Add a CSP-nonce to style tag

[pgjones]

This follows on from e377515 which introduced a script nonce. The same nonce is now used for the inline styles, allowing a stronger CSP (nonce over unsafe-inline).

Closes #1231

[antony]

Looks like a nice idea, however this seems like something which could break sites if it were to be done incorrectly, so I think this PR is deserving of a test or two to ensure that the feature doesn't regress.

[pgjones]

I'd need some help to write tests for this - could you point me at a similar or example test?

[benmccann]

There are a bunch of integration tests in the test/apps directory. I'm not super familiar with CSP, but maybe you could add it to the existing test/apps/css/test.ts integration test?

[pgjones]

Thanks, I've added a test.

I can't run the tests locally though, I've a clean repo (apart from these changes) and I get,

> sapper@0.27.13 test /Users/pgjones/sapper
> mocha --opts mocha.opts

undefined:1
��AjǺ
^

SyntaxError: Unexpected token � in JSON at position 0
    at JSON.parse (<anonymous>)
    at Object.parseSourceMapInput (/Users/pgjones/sapper/node_modules/source-map/lib/util.js:433:15)
    at new SourceMapConsumer (/Users/pgjones/sapper/node_modules/source-map/lib/source-map-consumer.js:17:22)
    at mapSourcePosition (/Users/pgjones/sapper/node_modules/source-map-support/source-map-support.js:190:14)
    at wrapCallSite (/Users/pgjones/sapper/node_modules/source-map-support/source-map-support.js:358:20)
    at /Users/pgjones/sapper/node_modules/source-map-support/source-map-support.js:399:26
    at Array.map (<anonymous>)
    at Function.prepareStackTrace (/Users/pgjones/sapper/node_modules/source-map-support/source-map-support.js:398:30)
    at prepareStackTrace (internal/errors.js:30:29)
    at process.emit (/Users/pgjones/sapper/node_modules/source-map-support/source-map-support.js:457:52)
    at process._fatalException (internal/process/execution.js:150:25)

any ideas?

[benmccann]

Hmmm. Can you open a new issue for that and include the output of npx envinfo --system --npmPackages svelte,sapper --binaries --browsers?

[pgjones]

@benmccann I've figured out the test failure, a stray : instead of a ; in the test code.

Now fixed, with a test for the script nonce as well :)

[pgjones]

I believe the test cancellation (failure) is unrelated to this patch.

[pgjones]

@Conduitry I've fixed the issue, could you review?

[justinwerner]

This is something that I would love to have implemented as well (I don't feel comfortable deploying an application with unsafe-inline set for anything). Does anybody have a status for this? Just waiting on @Conduitry? Thanks!

[pgjones]

I don't think the test failures are related to this change - only failed after the rebase.

[benmccann]

@pgjones there actually is a problem here. The same line of code is duplicated on lines 297 and 311. Can you remove the second one?

sapper/runtime/src/server/middleware/get_page_handler.ts

Line 297 in 600b540

const nonce_attr = (res.locals && res.locals.nonce) ? ` nonce="${res.locals.nonce}"` : '';

[pgjones]

@benmccann ah yes - I've fixed that now. I don't understand why the tests no longer work though - I guess something has changed since I wrote them?

[benmccann]

I can't really think of anything that would make them fail. I think the version of Puppeteer and some dependencies were updated, so maybe that? Anyway, I'm not sure, but if you manage to fix the tests I'm happy to merge this when they're all passing

[pgjones]

@benmccann I think this commit bcb1c19 breaks this, how can build_info.css evaluate to anything other than truthy given the default?

[benmccann]

That's the type, not the default though, right? There's been a lot that's changed there and it's certainly possible that something changed unintentionally there. I'm open to changing it back if you can find something like that

[pgjones]

That's the type, not the default though, right?

Yep, sorry I'm not fully concentrating on this.

I've taken another look, and I think I've now fixed everything (by setting the css option to undefined, which I think makes sense - although I don't understand why setting css to false in the svelte part doesn't work?).

[benmccann]

@pgjones I think the issue you were running into here was fixed by #1533. Can you take another look and see if that unblocks your testing efforts? It should hopefully allow you to stop doing 'css': undefined with the replace plugin in the test rollup config

[pgjones]

@benmccann excellent, I've removed the 'css': undefined and everything looks to work correctly.

[benmccann]

Thanks for your patience and sticking with it to see it through!

[pgjones]

🎉 thank you!