Skip to content

fix: add CSP to netlfiy #747

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix: add CSP to netlfiy #747

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e353e57
fix: add CSP to netlfiy
paularah Aug 18, 2025
1206c60
fix: enforce csp
paularah Aug 18, 2025
50c87b7
fix: format netlify.toml file
paularah Aug 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions netlify.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
[build]
publish = "public"
command = "gatsby build"

[[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"
Copy link
Member

@dylandreimerink dylandreimerink Aug 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Content-Security-Policy/script-src#unsafe_inline_script

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

@dylandreimerink dylandreimerink Aug 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot from 2025-08-20 11-26-16

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...

Permissions-Policy = "accelerometer=(), ambient-light-sensor=(), autoplay=(), camera=(), clipboard-read=(), clipboard-write=(), display-capture=(), encrypted-media=(), fullscreen=(self), geolocation=(), gyroscope=(), magnetometer=(), microphone=(), midi=(), payment=(), picture-in-picture=(self), usb=(), xr-spatial-tracking=(), browsing-topics=()"
Referrer-Policy = "strict-origin-when-cross-origin"
X-Content-Type-Options = "nosniff"
X-Frame-Options = "DENY"
Strict-Transport-Security = "max-age=31536000"