CSP

[Rich-Harris]

#93.

Lots to do here. CSP is a big topic and we might not nail it in a single PR, but I think this is a good starting point (let me know if I've overlooked or messed up anything):

• add types for CSP directives
• update options.js to expect new csp config option (including mode: 'nonce' | 'hash' | 'auto')
• generate hashes (probably easiest to just use sha256 everywhere?) from static assets not necessary, it turns out
• if default-src or script-src or style-src doesn't specify unsafe-inline, generate nonces or hashes (depending on the value of mode — defaults to 'auto', which means 'nonce for dynamically rendered stuff, hash for prerendered stuff'). Realistically, many apps will need style-src: 'unsafe-inline' until Svelte switches over to using WAAPI for transitions
• if default-src or script-src or style-src specifies strict-dynamic, include nonces/hashes for everything Kit controls also not necessary
• generate header, and add to response (or use http-equiv, for prerendering)
• replace %svelte.nonce% in src/app.html, but not during prerendering
• in dev, add style-src 'unsafe-inline' if necessary, skip nonce, add connect-src: ws://*
• always generate a nonce if %svelte.nonce% is present in the template (unless prerendering, obvs)

Stuff that's important but should probably stay out of scope for this PR:

• Externalise scripts and styles when prerendering #3555
• CSP report only mode #3556
• CSP support for AMP pages #3557
• strict-dynamic CSP support #3558

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests

• Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

• If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpx changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

[changeset-bot]

🦋 Changeset detected

Latest commit: a1d5b97

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@sveltejs/kit	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[netlify]

✔️ Deploy Preview for kit-demo canceled.

🔨 Explore the source changes: a1d5b97

🔍 Inspect the deploy log: https://app.netlify.com/sites/kit-demo/deploys/61f1d150cdf15a0008625196

[Karlinator]

I've been experimenting a bit, both with hashing at compile time and general mayhem.

Conclusion: Browsers are weird and inconsistent.

When using strict-dynamic, Chrome will happily load all script files as long as the inline init script is hashed (it appears Chrome sees the preloading as safe, which I suppose is accurate, as long as CSP is satisfied when they are applied). Firefox, however, refuses to load the script imported from the inline init script even though that inline script is hashed. This happens when I remove the modulepreload links as well, so might just be an issue with Firefox and strict-dynamic. Firefox has no issues when the init script is nonced, however, even if the hashes or nonces for the preload links are not present (just like Chrome).

I think this genuinely is a bug in Firefox's impllementation of strict-dynamic.

Without strict-dynamic, Chrome allows external scripts to be hashed (but only if the link has an integrity attribute with the hash), Firefox does not (a known deficiency). Both function with either hashes or nonces covering the init script as long as the rest are allowed with script-src 'self'

Other than that I was able to get Kit to hash the client js files on render and put the hashes in the manifest (though I'm not sure I'm happy with where I put it), and then the runtime fetches those hashes and insert them into integrity attributes. It occurs to me that there's no real reason to not include the integrity attribute? Even if it isn't required/useful for your specific CSP setup it's still an anti-tamper check I guess. Code: Karlinator@a74901e

What's the best way for me to contribute code to this PR btw, if you want it? Should I just make PRs into the csp branch?

[Rich-Harris]

Conclusion: Browsers are weird and inconsistent.

Yikes. Thanks for investigating, that does indeed sound like a Firefox bug. The best we can do is probably support strict-dynamic to the best of our understanding, and note the bug in the documentation (and find/create the corresponding issue on bugzilla).

What's the best way for me to contribute code to this PR btw, if you want it? Should I just make PRs into the csp branch?

I have some work locally that would likely conflict if we were both working in src (mostly moving code around in render.js, e.g. meta tags need to be prepended after scripts have been generated) — will push it up later when I'm back at my desk. Might be easiest to hold off until then so we don't find ourselves in merge conflict hell. I haven't started thinking about tests though, so that would be super helpful if you get a chance!

[Karlinator]

Yikes. Thanks for investigating, that does indeed sound like a Firefox bug. The best we can do is probably support strict-dynamic to the best of our understanding, and note the bug in the documentation (and find/create the corresponding issue on bugzilla).

I'm trying to narrow down the exact issue with reproduction so I can report it. But yeah, we'll just have to do our best with it, and advise that using nonce mode seems more consistent.

Might be easiest to hold off until then so we don't find ourselves in merge conflict hell

Sure. I was mostly experimenting, trying to get a flow of compile-time hashing (since the chunk files are completely static).

I might try to look at tests—or I'll just make more abstract contributions (read: find bugs in browsers) some more

[Karlinator]

Bugzilla report: https://bugzilla.mozilla.org/show_bug.cgi?id=1751573

[Karlinator]

A couple of edge cases with the dev server:

Vite inserts a whole bunch of inline style tags in the dev server (for perfectly good reasons). The SSR can nonce the style tag it sees as much as it wants; Vite will promptly replace it with un-nonced tags after hydration. Since this only affects dev mode it's probably fine to insert style-src 'unsafe-inline'? We also have to remove the nonce from style-src then in dev mode otherwise unsafe-inline is ignored.

Vite connects over WebSocket in dev mode, so if default-src or connect-src is specified, it needs to add connect-src ws://localhost:* (or ws://* if you're hosting over the network?), else Vite can't do hot reloading.

I'd be fine with treating the dev server a little differently and noting it in the docs in case anyone is confused. If you're testing your CSP then you should do it in the adapter's emulator or something anyway.

[Rich-Harris]

Alright, I think this is done. There's some follow-up work to do, as mentioned at the top, but I believe this covers the majority of what we need.

[Karlinator]

Ooo, I've found more CSP weirdness.

'strict-dynamic' with hashes currently doesn't work in dev mode but does work in production builds (in Chrome at least), because when using hashes with strict-dynamic the imported script apparently needs to be preloaded (which it isn't in the dev server). When the inline script is nonced there is no such constraint.

[Rich-Harris]

struggling to get it to work in dev or prod with strict-dynamic, which is weird because i could have sworn it was working fine earlier. might just need to get some sleep

[Rich-Harris]

I'm going to admit defeat on strict-dynamic for now, in the interests of getting this merged. We can add support in a follow-up PR