Skip to content

Conversation

@thebanjomatic
Copy link
Contributor

Description

Split off from #16415. This PR will set the nonce attribute for any inline <style> tags in the html if html.cspNonce is set. This probably shouldn't be merged until #16415 gets merged to avoid writing a second nonce attribute for any existing <style nonce=""> tags, otherwise this change would be breaking.

See comment from @sapphi-red

While I was working on #16052, I was thinking hard about what tags should have the nonce attribute injected. The first idea was to only inject tags that cannot be achieved by chaning the input HTML (e.g. <script type="module">). The second idea was to inject it into all tags that CSP allows using nonce (e.g. <script>, <style>). When I did that commit, I decided to go with the first idea. The reason was because I wasn't sure the whole list of tags that needs the nonce attribute to be set and another reason was because I thought it's easy to add it later rather than removing it. But it seems I forgot to skip injecting nonce attributes to script tags without type=module.

On second thought, I think it makes sense to go with the second idea. Vite already injects the nonce to <script>, , , . If I understand the CSP spec correctly, the only missing tag is the <style> tag.

@bolt-new-by-stackblitz
Copy link

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@sapphi-red sapphi-red added feat: html p2-nice-to-have Not breaking anything but nice to have (priority) labels Apr 13, 2024
sapphi-red
sapphi-red previously approved these changes Apr 14, 2024
@sapphi-red
Copy link
Member

/ecosystem-ci run

@vite-ecosystem-ci
Copy link

bluwy
bluwy previously approved these changes Apr 18, 2024
patak-dev
patak-dev previously approved these changes Apr 18, 2024
@patak-dev
Copy link
Member

@thebanjomatic would you resolve the conflicts? We discussed about this in the last team meeting and it was decided to move forward with it in a patch.

@thebanjomatic thebanjomatic dismissed stale reviews from patak-dev, bluwy, and sapphi-red via 16b9a8c April 18, 2024 10:27
@thebanjomatic
Copy link
Contributor Author

@patak-dev conflicts are resolved now. Thanks!

@patak-dev patak-dev enabled auto-merge (squash) April 18, 2024 10:51
@patak-dev patak-dev merged commit 8e54bbd into vitejs:main Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feat: html p2-nice-to-have Not breaking anything but nice to have (priority)

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants