-
Notifications
You must be signed in to change notification settings - Fork 65
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
fix(csp): inline script/style have whitespace character #478
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
97fbbb4
to
7a10605
Compare
Hey @hlhc Thanks for this PR. @vejja @GalacticHypernova what are your thoughts on this? :) |
Looks great |
Just checked, works as expected. The wildcard appears to have a limitation so that was a good spot (ideally any character should whitespace, as per the definition of any character). I don't see a point in adding this to all regexes besides inline scripts. Maybe inline styles but no one ever reported an issue on it so I'll first check it and whether it accepts it or not. Links for sure don't need it because it's a one liner. Though I am wondering whether or not it's even required, because the code is transformed and minified by Nuxt before the runtime plugins apply, as part of the build time. @hlhc could you please share a reproduction with such inline scripts not being given a csp hash so I could experiment with some things? |
inline styles can have newlines too |
Oh I know they can have newlines, I'm asking whether the nuxt minimization process takes care of these newlines or not |
I'll provide the reproduction later.
Based on my research in unjs repos, there shouldn't be any minimization process inside unhead.
Yes, I missed that in this PR since I only having problem in inline scripts. I think it should also added to inline styles in case any linebreaks or whitespace inside. |
It could also be handled inside vite or directly in nuxt, or maybe nitro if it's during the ssr render. That's why I would like to experiment with it
Sounds good! |
The HTML should only be handled inside Nuxt with Nitro. Inline styles will be built with Vite (with |
I assume that would differ between projects and specific nuxt configurations, maybe with sourcemaps enabled/disabled, which is why the best way to check it would be the reproduction of the issue that you encountered |
Reproduction: https://github.com/hlhc/csp-reproduce |
7a10605
to
4aede1b
Compare
any timeframe for this merge as currently this break ui pro with default csp |
Hey, I am up for releasing it even tomorrow as a next RC version if you agree :) |
Looks good on my end as well (sorry, was quite busy lately) |
Published in https://github.com/Baroshem/nuxt-security/releases/tag/v2.0.0-rc.7. Thanks guys for the amazing work! |
Types of changes
Description
This PR updates the regular expressions in the
30-cspSsgHashes.ts
file. The previous regular expression was not correctly capturing the content of inline script and style tags in all scenarios.The old regular expression for inline scripts:
The updated regular expression:
The change from
(.*?)
to([\s\S]*?)
ensures that the regular expression matches any character, including newlines, between the<script>
and</script>
tags. This change improves the accuracy of inline script content capture, ensuring that our CSP security hashes are correctly generated for all inline scripts.Same changes are made in inline styles.
Checklist: