fix: add CSP to netlfiy #747
Conversation
Signed-off-by: Paul Arah <[email protected]>
👷 Deploy Preview for cilium processing.
|
✅ Deploy Preview for cilium ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Signed-off-by: Paul Arah <[email protected]>
Signed-off-by: Paul Arah <[email protected]>
|
@xmulligan this adds the content security policies that fixes the header security issue |
| [[headers]] | ||
| for = "/*" | ||
| [headers.values] | ||
| Content-Security-Policy-Report-Only = "default-src 'self'; base-uri 'self'; object-src 'none'; frame-ancestors 'self'; form-action 'self'; script-src 'self' 'unsafe-inline' https://www.googletagmanager.com https://www.google-analytics.com; style-src 'self' 'unsafe-inline' https://fonts.googleapis.com; img-src 'self' data: https://www.google-analytics.com; font-src 'self' https://fonts.gstatic.com data:; connect-src 'self' https://www.google-analytics.com https://region1.google-analytics.com; frame-src 'self' https://www.youtube.com" |
There was a problem hiding this comment.
I have some serious doubts surrounding this header.
First off all, this is the -Report-Only variant of the header, which tells to browsers to run the policy, but not enforce it. Violations are normally reported to an endpoint via HTTP request. This makes sense if we want to try out a CSP before adopting it, but this does not specify a reporting endpoint with the report-to/report-uri directive, so this is a no-op. See docs.
Second. The script-src includes the unsafe-inline directive, which essentially defeats the whole purpose of a CSP since it allows any inline block javascript block to execute. If there are parts of our site that require inline scripting to work, then we should look into allowing specific blocks with hashes. But removing all inline JS and only accepting self plus trusted external sources is the best course of action.
There was a problem hiding this comment.
I had the "-Report-Only" variant of the header there until we're happy with the entire CSP.
We have code like this </Heading> <p className="max-w-full sm:max-w-[520px] [&>br]:hidden sm:[&>br]:block dark:text-gray-2 text-black" dangerouslySetInnerHTML={{ __html: description }} /> </div> <Button all over the codebase. It'd be a huge refactor to be complaint.
There was a problem hiding this comment.
I had the "-Report-Only" variant of the header there until we're happy with the entire CSP.
Yes, that part makes sense. But we are not sending the reports anywhere, so how do you plan on getting feedback on how well the policy performs? If I recall correctly, the report-only does throw up messages in dev tools, but how confident are you that you can check all edge cases manually?
Also, non of that is explained in commit messages or the PR.
We have code like this <p className="max-w-full sm:max-w-[520px] [&>br]:hidden sm:[&>br]:block dark:text-gray-2 text-black" dangerouslySetInnerHTML={{ __html: description }} /> <Button all over the codebase. It'd be a huge refactor to be complaint.
Right, so we already have an issue if ever description is derived from user input in some way. But that shouldn't necessarily be an issue for CSP if we never use <script></script>. If we move all usages of inline script to dedicated files hosted on the same domain, then the CSP would actually add some protection.
As long as we include unsafe-inline you add a CSP header, and disable its primary feature which is to prevent XSS. You might as well not have a CSP at all if you cannot effectively use it.
There was a problem hiding this comment.
It does indeed seem to locally create an error in the dev tools. But I don't see a reason to merge the PR with -report-only unless we actually have a backend that can collect the reports.
For testing CSP without the reporting feature we can use the previews.
Also, it appears that you missed https://static.hotjar.com, found this while not even trying...
No description provided.