Allow scripts to contain a CSP nonce

[nolanlawson]

This PR allows Sapper to use a nonce for inline scripts, using the Express and Helmet convention of res.locals.nonce.

CSP is a key feature for Pinafore because it's a webapp that can talk to multiple third-party servers (Mastodon instances), none of which may trust each other. The app also injects arbitrary HTML from these servers. This HTML is expected to be sanitized, but a malicious instance could inject inline <script>s to read data from other instances. To prevent this, Pinafore uses Helmet with CSP.

Unfortunately Sapper now injects inline <script>s, which are blocked by CSP. This PR would allow a Sapper app to use nonces instead, allowing Sapper to function correctly while still blocking arbitrary inline scripts.

[nolanlawson]

Documentation PR: sveltejs/sapper-legacy.svelte.dev#34

[kylecordes]

This makes me wonder: is it best to have the feature of "make it possible to be compatible with CSP if you configure and adjust for it"?

How about instead a feature like "Sapper automatically, always, by default emits output compatible with CSP"?

[Rich-Harris]

I love it, thanks @nolanlawson. I'm torn between the two approaches — on the one hand I'm fairly ignorant about a lot of this stuff and would love it if a framework would just do the right thing on my behalf as @kylecordes suggests, especially since it seems you need an unfortunate about of boilerplate to do very basic things; on the other, I'm not a fan of 'magic' to the extent that it can be avoided.

The second approach also poses a challenge if a CSP header already exists (but doesn't specify a nonce) — do we modify the header? That seems, well, rude. Though I suppose if no nonce was specified then it means we don't need to add one to the <script> tag, just like we currently don't.

Also worth noting: Rollup apps use Shimport in browsers that don't support (dynamic) import, which involves fetching modules and evaluating them. Does the JS inside a script-with-nonce 'inherit' the ability to eval those modules, or does that need to be added as a separate CSP rule?

In summary: I think I probably lean towards the approach in this PR, but I'd be interested to hear from anyone else who has opinions about how this should be tackled.

[Rich-Harris]

This script would need a nonce too — can you use the same nonce more than once in a document? Or does it then become a nmorethanonce?

[nolanlawson]

This script would need a nonce too — can you use the same nonce more than once in a document?

You can use the same nonce more than once (in fact you're supposed to), but AFAICT that script doesn't need a nonce because it's just <script src=main.js></script>, i.e. it's covered by the self directive. The nonce only applies to inline scripts.

Also worth noting: Rollup apps use Shimport in browsers that don't support (dynamic) import, which involves fetching modules and evaluating them. Does the JS inside a script-with-nonce 'inherit' the ability to eval those modules, or does that need to be added as a separate CSP rule?

Yes, this would break CSP where unsafe-eval isn't allowed, assuming you're using something like eval() or new Function(). A user could disallow unsafe-inline but allow unsafe-eval, however.

How about instead a feature like "Sapper automatically, always, by default emits output compatible with CSP"?

I think it would be taking on a lot for Sapper to try to "automatically" do CSP. Unfortunately there are just a lot of different ways of configuring it, and each site may have different needs. (Related: it's also possible to disable inline CSS, so presumably Sapper could also emit a nonce for that, but we can cover that in another PR.) If Sapper wanted to "do CSP" correctly, then it would basically need to contain all of helmet-csp and expose options for configuring it, at which point you're better off just letting users roll their own. 😃

So basically I'm in favor of the approach in this PR, unless there are other suggestions for how to go about it.

[nolanlawson]

How about instead a feature like "Sapper automatically, always, by default emits output compatible with CSP"?

Just realized I misunderstood this question. Yes, if Sapper used an actual script instead of an inline script, then that would not break CSP for the script-src: self directive. I think this would require generating a script file for each route, though?

[Rich-Harris]

The nonce only applies to inline scripts

ah, of course. D'oh!

I think it would be taking on a lot for Sapper to try to "automatically" do CSP

You make a good point!

Yes, this would break CSP where unsafe-eval isn't allowed, assuming you're using something like eval() or new Function().

That's correct — Shimport does (0,eval)(text). Is that a problem for you? I've kind of glossed over it so far because the alternative involves a lot more moving parts.

[Rich-Harris]

I think this would require generating a script file for each route, though?

Not just each route, but each request, in cases where there was preloaded data. Unless it were to do preload in the client as well as on the server, so that it could avoid serialization.

[nolanlawson]

Shimport does (0,eval)(text). Is that a problem for you?

If I were to use Shimport, then yeah, this would be a problem. Based on #397 it seems optional, though? Pinafore doesn't support IE, so I don't have to worry about differential builds. 😃

[Rich-Harris]

Shimport isn't related to differential bundling — it's automatic if you're using Rollup, because Rollup apps emit native ESM. I've found this makes a big difference to bundle size. Unfortunately, Firefox doesn't yet support import() so in order to load code-split chunks we have to do something slightly distasteful.

The alternative I refer to is creating AMD code-split chunks and using something like https://github.com/surma/rollup-plugin-loadz0r, but AMD bundles are a decent amount larger, unfortunately (basically the same as webpack builds).