Add base tag to html attachments view #5764
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
karlbaker02
force-pushed
the
add-base-tag-to-html-attachments-preview
branch
from
d784b5f to
d1ea6f0
Compare
|
Super, this looks like a good approach. Nice one working this out. Have you tried it on integration? Might also be worth checking it works ok on draft stack, but I assume Plek is fine for that. Implementation wise I'd suggest moving this from setting it on all html_publications (as the vast majority of them don't need it) and localising this to just csv_preview since that is the quirky bit of the stack. You could do that by setting |
|
Makes sense, thanks @kevindew. Haven't tested on integration yet as I'm waiting on the tests to pass before deploying the branch. I wasn't sure how wide to scope this change originally, which is why I added it into |
|
cool cool - probably worth giving it a spin on integration as is anyway just to verify it does fix the problem |
karlbaker02
force-pushed
the
add-base-tag-to-html-attachments-preview
branch
4 times, most recently
from
7a13849 to
5a36a48
Compare
karlbaker02
force-pushed
the
add-base-tag-to-html-attachments-preview
branch
from
5a36a48 to
84f246d
Compare
This commit fixes a long-standing bug with Whitehall, where links in the header and footer of the CSV preview page (rendered by the `CsvPreviewController`) contain relative links that point to the incorrect site root (`assets.publishing.service.gov.uk` rather than `www.gov.uk`), resulting in `404`s to any of those links which are followed. The footer itself is obtained through Slimmer, using the `chromeless` view that's part of `Static` where most URLs in the footer are set as relative. The fix is achieved through adding a `base` tag to the `frontend_base` partial, which is set from the `CsvPreviewController` and rendered via the `html_attachments` view. By adding a `base` tag, all relative URLs on the page will be prefixed with the `href` attribute specified, which is the GOV.UK website root. The `base` tag can only appear in the `head` of a page, which is the reason for adding it into the partial `frontend_base`.
karlbaker02
force-pushed
the
add-base-tag-to-html-attachments-preview
branch
from
84f246d to
5b96d9f
Compare
kevindew
added a commit
to alphagov/govuk_app_config
that referenced
this pull request
Jan 12, 2023
A base element can be used to change the destination of relative paths, this can be used as part of XSS to include a script file on a host an attacker controls. To prevent this we disable all uses of the base element as it is not used on all GOV.UK views, bar one exception. The exception is for CSV previews which are rendered by Whitehall [1] on a different hostname (assets). As this is only for one app the convention would be to modify the CSP in app. It is also unclear at this point in time when or whether we will enable the CSP on Whitehall frontend. [1]: alphagov/whitehall#5764
kevindew
added a commit
to alphagov/govuk_app_config
that referenced
this pull request
Jan 20, 2023
A base element can be used to change the destination of relative paths, this can be used as part of XSS to include a script file on a host an attacker controls. To prevent this we disable all uses of the base element as it is not used on all GOV.UK views, bar one exception. The exception is for CSV previews which are rendered by Whitehall [1] on a different hostname (assets). As this is only for one app the convention would be to modify the CSP in app. It is also unclear at this point in time when or whether we will enable the CSP on Whitehall frontend. [1]: alphagov/whitehall#5764
kevindew
added a commit
to alphagov/govuk_app_config
that referenced
this pull request
Jan 20, 2023
A base element can be used to change the destination of relative paths, this can be used as part of XSS to include a script file on a host an attacker controls. To prevent this we disable all uses of the base element as it is not used on all GOV.UK views, bar one exception. The exception is for CSV previews which are rendered by Whitehall [1] on a different hostname (assets). As this is only for one app the convention would be to modify the CSP in app. It is also unclear at this point in time when or whether we will enable the CSP on Whitehall frontend. [1]: alphagov/whitehall#5764
kevindew
added a commit
to alphagov/govuk_app_config
that referenced
this pull request
Jan 23, 2023
A base element can be used to change the destination of relative paths, this can be used as part of XSS to include a script file on a host an attacker controls. To prevent this we disable all uses of the base element as it is not used on all GOV.UK views, bar one exception. The exception is for CSV previews which are rendered by Whitehall [1] on a different hostname (assets). As this is only for one app the convention would be to modify the CSP in app. It is also unclear at this point in time when or whether we will enable the CSP on Whitehall frontend. [1]: alphagov/whitehall#5764
kevindew
added a commit
to alphagov/govuk_app_config
that referenced
this pull request
Jan 23, 2023
A base element can be used to change the destination of relative paths, this can be used as part of XSS to include a script file on a host an attacker controls. To prevent this we disable all uses of the base element as it is not used on all GOV.UK views, bar one exception. The exception is for CSV previews which are rendered by Whitehall [1] on a different hostname (assets). As this is only for one app the convention would be to modify the CSP in app. It is also unclear at this point in time when or whether we will enable the CSP on Whitehall frontend. [1]: alphagov/whitehall#5764
kevindew
added a commit
to alphagov/govuk_app_config
that referenced
this pull request
Jan 25, 2023
A base element can be used to change the destination of relative paths, this can be used as part of XSS to include a script file on a host an attacker controls. To prevent this we disable all uses of the base element as it is not used on all GOV.UK views, bar one exception. The exception is for CSV previews which are rendered by Whitehall [1] on a different hostname (assets). As this is only for one app the convention would be to modify the CSP in app. It is also unclear at this point in time when or whether we will enable the CSP on Whitehall frontend. [1]: alphagov/whitehall#5764
kevindew
added a commit
to alphagov/govuk_app_config
that referenced
this pull request
Jan 26, 2023
A base element can be used to change the destination of relative paths, this can be used as part of XSS to include a script file on a host an attacker controls. To prevent this we disable all uses of the base element as it is not used on all GOV.UK views, bar one exception. The exception is for CSV previews which are rendered by Whitehall [1] on a different hostname (assets). As this is only for one app the convention would be to modify the CSP in app. It is also unclear at this point in time when or whether we will enable the CSP on Whitehall frontend. [1]: alphagov/whitehall#5764
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a
basetag to thehtml_attachmentsview. This view is used by thecsv_previewcontroller and links currently rendered in the footer on this page currently point to the wrong URL (assets rather than GOV.UK); as a result, they return 404s. The footer itself is obtained through Slimmer, using thechromelessview that's part ofStaticwhere most URLs in the footer are relative.By adding a
basetag, all relative URLs on the page will be prefixed with thehrefattribute specified, which is the GOV.UK website root. Thebasetag can only appear in theheadof a page, which is the reason for adding it into the partialfrontend_base.