-
Notifications
You must be signed in to change notification settings - Fork 35
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
Add csp_nonce for inline script and gov_template #5325
Conversation
674ec4d
to
3b22955
Compare
missed
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a comment but also wanted to suggest this adds a test to this section of tests/conftest.py
for script tags that either contain inline code or reference a file not hosted on our domain. That would both identify any current scripts without the nonce
attribute and protect us from adding any in future.
Also to say this is great work and much needed. Thanks for doing it!
1480ec3
to
bbeb4fb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry to be asking for changes again, I'm not sure my last review was clear enough, but I think these changes would mean we could spot and guard against all unsafe script tags.
62596af
to
21d13d0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
based on #5325 (comment) I'm happy with this but it should wait for Tom as he requested changes
Previously we added `unsafe-inline` and `unsafe-eval` directives for `script-src` header. We can remove these directives as part of ITHC after removing `Hogan`. Additionally to fix CSP errors added `csp_nonce` for request to allow `govuk_frontend_jinja/template.html` and inline `script`.
21d13d0
to
0bf7ea6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for going through all the changes requested. Looks great now 👍🏻
Previously we added
unsafe-inline
andunsafe-eval
directives forscript-src
header. We can remove these directives as part of ITHC after removingHogan
. Additionally to fix CSP errors addedcsp_nonce
for request to allowgovuk_frontend_jinja/template.html
and inlinescript
. More details on this card.