Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Forbid unsafe-inline for style attributes in CSP #291

Merged
merged 2 commits into from
Jun 2, 2023

Conversation

kevindew
Copy link
Member

@kevindew kevindew commented Apr 27, 2023

This continues the work from #279 to remove risky properties from our Content Security Policy (CSP) by removing unsafe-inline from style properties.

We have been able to resolve the need for this property by updating Govspeak 1.

I've marked this as a breaking change as it forbids something fundamental, however this is really quite a soft breaking change as a) there has been lots of prep work across GOV.UK to prepare for it and b) the CSP is in report only mode across GOV.UK so won't actually block any usages.

Prior to merging this we need to represent any content that uses tables with text alignment to the Publishing API to remove their inline styles. I've opened this as a draft while this is completed.
Edit: 2 June, I've been through and removed nearly all usages of style. I'm now going to utilise the CSP reports to catch other ones.

@kevindew kevindew force-pushed the forbid-inline-styles branch from 8e81376 to 55b2547 Compare April 27, 2023 15:58
kevindew added a commit to alphagov/govuk_publishing_components that referenced this pull request May 23, 2023
This removes setting list-style on ordered lists to instead allow the
use of the type attribute on an ordered list to specify the list-style.

I couldn't determine why this style was needed. Current browsers style
this as list-style decimal by default and I can't find any code this
undoes. It could be that previously this was to undo an earlier
override, but it would be very unconventional for current GOV.UK to
style an `ol` without a class so this seems unlikely.

The motivation for making this change is to support some niche behaviour
used by publishers that will become disabled. Currently some advanced
published have managed to published ordered lists that are alphabetical
or roman numeral prefixed by use of an `ol` element with a style
attribute. This will not be available with the role out of the GOV.UK
Content Security Policy which forbids unsafe inline styles [1].

[1]: alphagov/govuk_app_config#291
kevindew added a commit to alphagov/govuk_publishing_components that referenced this pull request May 23, 2023
This removes setting list-style on ordered lists to instead allow the
use of the type attribute on an ordered list to specify the list-style.

I couldn't determine why this style was needed. Current browsers style
this as list-style decimal by default and I can't find any code this
undoes. It could be that previously this was to undo an earlier
override, but it would be very unconventional for current GOV.UK to
style an `ol` without a class so this seems unlikely.

The motivation for making this change is to support some niche behaviour
used by publishers that will become disabled. Currently some advanced
published have managed to published ordered lists that are alphabetical
or roman numeral prefixed by use of an `ol` element with a style
attribute. This will not be available with the role out of the GOV.UK
Content Security Policy which forbids unsafe inline styles [1].

[1]: alphagov/govuk_app_config#291
kevindew added a commit to alphagov/govuk_publishing_components that referenced this pull request Jun 2, 2023
This removes setting list-style on ordered lists to instead allow the
use of the type attribute on an ordered list to specify the list-style.

I couldn't determine why this style was needed. Current browsers style
this as list-style decimal by default and I can't find any code this
undoes. It could be that previously this was to undo an earlier
override, but it would be very unconventional for current GOV.UK to
style an `ol` without a class so this seems unlikely.

The motivation for making this change is to support some niche behaviour
used by publishers that will become disabled. Currently some advanced
published have managed to published ordered lists that are alphabetical
or roman numeral prefixed by use of an `ol` element with a style
attribute. This will not be available with the role out of the GOV.UK
Content Security Policy which forbids unsafe inline styles [1].

[1]: alphagov/govuk_app_config#291
This continues the work from
#279 to remove risky
properties from our Content Security Policy (CSP) by removing
unsafe-inline from style properties.

We have been able to resolve the need for this property by updating Govspeak
[1]

[1]: alphagov/govspeak#268
@kevindew kevindew force-pushed the forbid-inline-styles branch from 55b2547 to 864b3e8 Compare June 2, 2023 08:24
@kevindew kevindew marked this pull request as ready for review June 2, 2023 08:28
Copy link
Contributor

@ChrisBAshton ChrisBAshton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@kevindew kevindew merged commit 565d07a into main Jun 2, 2023
@kevindew kevindew deleted the forbid-inline-styles branch June 2, 2023 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants