Add nonce support to bootstrap scripts and external runtime #26738

danieltott · 2023-04-28T15:26:09Z

Summary

When using a Content Security Policy with the strict-dynamic source list keyword, all other non-nonce keywords and sources are ignored, specifically 'self'. This means that scripts that are inserted via <script src=... /> into the HTML violate the content security policy.

This fix also supports CSPs that are not using strict-dynamic, but are using nonce-xxx sources

Right now the render functions (renderToReadableStream, renderToPipeableStream etc) via createResponseState add the nonce attribute to inline scripts set in bootstrapScriptContent, but not scripts via bootstrapScripts and bootstrapModules. Also missing the nonce value are external runtime scripts, as well as scripts rendered at runtime that could be rendered on the server.

Adding nonce support to those scripts will allow them to pass a content-security-policy that uses strict-dynamic.

Related issues:

Changes:

Add nonce to bootstrap scripts/modules in ReactFizzConfigDOM -> createResponseState()
Add nonce property to the return object for createResponseState()
Pass nonce prop to internalPreinitScript() (external runtime scripts)
~~Inject nonce prop in pushStartInstance() where type=='script' if it exists in responseState~~ (removed based on Add nonce support to bootstrap scripts and external runtime #26738 (comment))
Updated FizzTestUtils.js to re-add nonce attribute after executing inline scripts

How did you test this change?

I added the following tests:

ReactDOMFizzServer-test - it should render scripts with nonce added
Test that nonce values are applied correctly in rendered scripts
ReactDOMFizzServer-test - it should support nonce for bootstrap and runtime scripts
Update the existing nonce test to test all bootstrap scripts, as well as the runtime script

react-sizebot · 2023-04-28T15:31:07Z

Comparing: 18282f8...d2d41c4

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js	=	164.04 kB	164.04 kB	=	51.72 kB	51.72 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	170.34 kB	170.34 kB	=	53.63 kB	53.63 kB
facebook-www/ReactDOM-prod.classic.js	=	567.04 kB	567.04 kB	=	100.16 kB	100.16 kB
facebook-www/ReactDOM-prod.modern.js	=	550.78 kB	550.78 kB	=	97.36 kB	97.36 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against d2d41c4

gaearon · 2023-04-28T16:09:01Z

Seems like PROD tests fail?

danieltott · 2023-04-28T16:10:16Z

@gaearon I see that! Looking into it now...

Test in —prod actually executes the boostrapScriptContent code

danieltott · 2023-04-28T16:38:12Z

@gaearon all fixed 👍

gnoff · 2023-04-28T17:56:24Z

@danieltott what’s the rationale for automatically applying the nonce to all scripts? Also there are hydration implications because attributes are validated in dev. From a. Security angle it seems good that you need to opt your scripts into being nonced then again if you have a supply chain attack taking advantage of this auto nonce then they can also do a lot of other bad things

gnoff · 2023-04-28T17:58:13Z

packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js

+    const startScriptWithNonce =
+      nonce === undefined
+        ? startScriptSrc
+        : stringToPrecomputedChunk(


Precomputed chunks are generally constructed in module scope. The idiomatic way of doing this is to break these chunks up in a static-dynamic-static sandwich

@gnoff I updated the code to sandwich style 🥪

I had originally based my code on the pre-existing nonce setup for inline scripts:

react/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js

Lines 210 to 215 in b1b1940

const inlineScriptWithNonce =

nonce === undefined

? startInlineScript

: stringToPrecomputedChunk(

'<script nonce="' + escapeTextForBrowser(nonce) + '">',

);

If the updated code looks good, should I apply it there as well?

This is different. We use this repeatedly when we emit our fizz runtime scripts. The idea with precomputed chunks is you can do the text encoding once (if configured for the host runtime) so we only gain a benefit if the value is reused. pulling the chunk creation to module scope is our way of reusing the chunk which is why I suggested the sandwhich style for your original use case. Here we are getting reuse by referencing this precomputed chunk on each instruction flush but we can't make it in module scope because we embed the nonce within it

OK - if I understand correctly, this is because you return startInlineScript from createResponseState and use it other places. Since we don't have to do that with src/module scripts, it's actually more efficient to construct them the way you suggested, since we can precompile the nonce= chunk once inside the module. 👍

That was a tough one to wrap my head around but I'm pretty sure I have it - thank you for your explanations 🙌

danieltott · 2023-04-28T19:23:20Z

what’s the rationale for automatically applying the nonce to all scripts?

@gnoff Honestly I might have gotten a little overzealous here. I was concerned with server-rendered script tags since that is the important thing in strict-dynamic scenarios, but I think you're right that users are free to add nonce=... to their own scripts in the body, and react just needs to make sure automatically-created scripts are handled properly. I'll remove those additions here shortly.

danieltott · 2023-04-28T19:46:34Z

@gnoff Updated 👍

gnoff

Nits but looks good

gnoff · 2023-04-28T22:07:44Z

packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js

+      if (nonce === undefined) {
+        bootstrapChunks.push(
+          startModuleSrc,
+          stringToChunk(escapeTextForBrowser(src)),
+        );
+      } else {
+        bootstrapChunks.push(
+          startModuleSrcWithNonce,
+          stringToChunk(escapeTextForBrowser(nonce)),
+          middleModuleSrcWithNonce,
+          stringToChunk(escapeTextForBrowser(src)),
+        );
+      }


I would maybe just do this like the integrity attribute

Suggested change

if (nonce === undefined) {

bootstrapChunks.push(

startModuleSrc,

stringToChunk(escapeTextForBrowser(src)),

);

} else {

bootstrapChunks.push(

startModuleSrcWithNonce,

stringToChunk(escapeTextForBrowser(nonce)),

middleModuleSrcWithNonce,

stringToChunk(escapeTextForBrowser(src)),

);

}

bootstrapChunks.push(

startModuleSrc,

stringToChunk(escapeTextForBrowser(src))

);

if (nonce) {

bootstrapChunks.push(

scriptNonce,

stringToChunk(escapeTextForBrowser(nonce)),

);

}

gnoff · 2023-04-28T22:07:55Z

packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js

+      if (nonce === undefined) {
+        bootstrapChunks.push(
+          startScriptSrc,
+          stringToChunk(escapeTextForBrowser(src)),
+        );
+      } else {
+        bootstrapChunks.push(
+          startScriptSrcWithNonce,
+          stringToChunk(escapeTextForBrowser(nonce)),
+          middleScriptSrcWithNonce,
+          stringToChunk(escapeTextForBrowser(src)),
+        );
+      }


same recc as for modules

gnoff · 2023-04-28T22:58:55Z

@danieltott I'll merge once you make those last changes. Or I can make them for you if preferred

danieltott · 2023-04-28T23:09:13Z

@gnoff makes sense! will update here in a minute

danieltott · 2023-04-28T23:21:48Z

@gnoff this is all set

gnoff · 2023-05-01T16:19:55Z

@danieltott congrats on landing! Thanks for making the PR

Adds support for nonce on `bootstrapScripts`, `bootstrapModules` and the external fizz runtime DiffTrain build for [9545e48](9545e48)

danieltott · 2023-05-01T16:25:32Z

@gnoff thank you!! And thank you for your time helping me work through it a bit 🙌

Any idea on release scheduling? Not sure how that sort of thing works with the React codebase/package

gnoff · 2023-05-01T17:01:04Z

Any idea on release scheduling? Not sure how that sort of thing works with the React codebase/package

It's been a while since we've done a release and we're due for one soon. You'll be able to test our the changes in the next and experimental channels that get published every night and unless this gets reverted it will be included in the next stable however I cannot say when that will be

danieltott · 2023-05-01T17:02:20Z

@gnoff awesome - thanks again!

in #26738 we added nonce to the ResponseState. Initially it was used in a variety of places but the version that got merged only included it with the external fizz runtime. This PR updates the config for the external fizz runtime so that the nonce is encoded into the script chunks at request creation time. The rationale is that for live-requests, streaming is more likely than not so doing the encoding work at the start is better than during flush. For cases such as SSG where the runtime is not required the extra encoding is tolerable (not a live request). Bots are an interesting case because if you want fastest TTFB you will end up requiring the runtime but if you are withholding until the stream is done you have already sacrificed fastest TTFB and the marginal slowdown of the extraneous encoding is hopefully neglibible I'm writing this so later if we learn that this tradeoff isn't worth it we at least understand why I made the change in the first place.

in #26738 we added nonce to the ResponseState. Initially it was used in a variety of places but the version that got merged only included it with the external fizz runtime. This PR updates the config for the external fizz runtime so that the nonce is encoded into the script chunks at request creation time. The rationale is that for live-requests, streaming is more likely than not so doing the encoding work at the start is better than during flush. For cases such as SSG where the runtime is not required the extra encoding is tolerable (not a live request). Bots are an interesting case because if you want fastest TTFB you will end up requiring the runtime but if you are withholding until the stream is done you have already sacrificed fastest TTFB and the marginal slowdown of the extraneous encoding is hopefully neglibible I'm writing this so later if we learn that this tradeoff isn't worth it we at least understand why I made the change in the first place. DiffTrain build for [8ea96ef](8ea96ef)

Includes the following upstream changes: - [5dd90c562](https://github.com/facebook/react/commits/5dd90c562) Use content hash for react-native builds ([vercel#26734](facebook/react#26734)) (Samuel Susla) - [559e83aeb](https://github.com/facebook/react/commits/559e83aeb) [Fizz] Allow an action provide a custom set of props to use for progressive enhancement ([vercel#26749](facebook/react#26749)) (Sebastian Markbåge) - [67f4fb021](https://github.com/facebook/react/commits/67f4fb021) Allow forms to skip hydration of hidden inputs ([vercel#26735](facebook/react#26735)) (Sebastian Markbåge) - [8ea96ef84](https://github.com/facebook/react/commits/8ea96ef84) [Fizz] Encode external fizz runtime into chunks eagerly ([vercel#26752](facebook/react#26752)) (Josh Story) - [491aec5d6](https://github.com/facebook/react/commits/491aec5d6) Implement experimental_useOptimisticState ([vercel#26740](facebook/react#26740)) (Andrew Clark) - [9545e4810](https://github.com/facebook/react/commits/9545e4810) Add nonce support to bootstrap scripts and external runtime ([vercel#26738](facebook/react#26738)) (Dan Ott) - [86b0e9199](https://github.com/facebook/react/commits/86b0e9199) Gate DevTools test to fix CI ([#26742](facebook/react#26742)) (Andrew Clark) - [b12bea62d](https://github.com/facebook/react/commits/b12bea62d) Preinits should support a nonce option ([#26744](facebook/react#26744)) (Josh Story) - [efbd68511](https://github.com/facebook/react/commits/efbd68511) Remove unused `initialStatus` parameter from `useHostTransitionStatus` ([vercel#26743](facebook/react#26743)) (Sebastian Silbermann) - [18282f881](https://github.com/facebook/react/commits/18282f881) Fix: Update while suspended fails to interrupt ([vercel#26739](facebook/react#26739)) (Andrew Clark) - [540bab085](https://github.com/facebook/react/commits/540bab085) Implement experimental_useFormStatus ([#26722](facebook/react#26722)) (Andrew Clark)

Includes the following upstream changes: - [b7972822b](https://github.com/facebook/react/commits/b7972822b) useOptimisticState -> useOptimistic ([#26772](facebook/react#26772)) (Andrew Clark) - [388686f29](https://github.com/facebook/react/commits/388686f29) Add "canary" to list of allowed npm dist tags ([#26767](facebook/react#26767)) (Andrew Clark) - [8a25302c6](https://github.com/facebook/react/commits/8a25302c6) fix[dynamic-scripts-injection]: unregister content scripts before registration ([#26765](facebook/react#26765)) (Ruslan Lesiutin) - [2c2476834](https://github.com/facebook/react/commits/2c2476834) Rename "next" prerelease channel to "canary" ([#26761](facebook/react#26761)) (Andrew Clark) - [fa4314841](https://github.com/facebook/react/commits/fa4314841) Remove deprecated workflow key from Circle config ([#26762](facebook/react#26762)) (Andrew Clark) - [5dd90c562](https://github.com/facebook/react/commits/5dd90c562) Use content hash for react-native builds ([#26734](facebook/react#26734)) (Samuel Susla) - [559e83aeb](https://github.com/facebook/react/commits/559e83aeb) [Fizz] Allow an action provide a custom set of props to use for progressive enhancement ([#26749](facebook/react#26749)) (Sebastian Markbåge) - [67f4fb021](https://github.com/facebook/react/commits/67f4fb021) Allow forms to skip hydration of hidden inputs ([#26735](facebook/react#26735)) (Sebastian Markbåge) - [8ea96ef84](https://github.com/facebook/react/commits/8ea96ef84) [Fizz] Encode external fizz runtime into chunks eagerly ([#26752](facebook/react#26752)) (Josh Story) - [491aec5d6](https://github.com/facebook/react/commits/491aec5d6) Implement experimental_useOptimisticState ([#26740](facebook/react#26740)) (Andrew Clark) - [9545e4810](https://github.com/facebook/react/commits/9545e4810) Add nonce support to bootstrap scripts and external runtime ([#26738](facebook/react#26738)) (Dan Ott) - [86b0e9199](https://github.com/facebook/react/commits/86b0e9199) Gate DevTools test to fix CI ([#26742](facebook/react#26742)) (Andrew Clark) - [b12bea62d](https://github.com/facebook/react/commits/b12bea62d) Preinits should support a nonce option ([#26744](facebook/react#26744)) (Josh Story) - [efbd68511](https://github.com/facebook/react/commits/efbd68511) Remove unused `initialStatus` parameter from `useHostTransitionStatus` ([#26743](facebook/react#26743)) (Sebastian Silbermann) - [18282f881](https://github.com/facebook/react/commits/18282f881) Fix: Update while suspended fails to interrupt ([#26739](facebook/react#26739)) (Andrew Clark) - [540bab085](https://github.com/facebook/react/commits/540bab085) Implement experimental_useFormStatus ([#26722](facebook/react#26722)) (Andrew Clark) ---------

Summary: in facebook/react#26738 we added nonce to the ResponseState. Initially it was used in a variety of places but the version that got merged only included it with the external fizz runtime. This PR updates the config for the external fizz runtime so that the nonce is encoded into the script chunks at request creation time. The rationale is that for live-requests, streaming is more likely than not so doing the encoding work at the start is better than during flush. For cases such as SSG where the runtime is not required the extra encoding is tolerable (not a live request). Bots are an interesting case because if you want fastest TTFB you will end up requiring the runtime but if you are withholding until the stream is done you have already sacrificed fastest TTFB and the marginal slowdown of the extraneous encoding is hopefully neglibible I'm writing this so later if we learn that this tradeoff isn't worth it we at least understand why I made the change in the first place. DiffTrain build for commit facebook/react@8ea96ef. Changelog: [Internal] << DO NOT EDIT BELOW THIS LINE >> Reviewed By: sammy-SC Differential Revision: D45449587 Pulled By: tyao1 fbshipit-source-id: 76a5f34e42db5e9ad5a78b4b8f0d4e3dad2e7fcd

gaearon · 2023-05-05T16:04:09Z

@danieltott

Re: releases, see https://react.dev/blog/2023/05/03/react-canaries

Summary: in facebook/react#26738 we added nonce to the ResponseState. Initially it was used in a variety of places but the version that got merged only included it with the external fizz runtime. This PR updates the config for the external fizz runtime so that the nonce is encoded into the script chunks at request creation time. The rationale is that for live-requests, streaming is more likely than not so doing the encoding work at the start is better than during flush. For cases such as SSG where the runtime is not required the extra encoding is tolerable (not a live request). Bots are an interesting case because if you want fastest TTFB you will end up requiring the runtime but if you are withholding until the stream is done you have already sacrificed fastest TTFB and the marginal slowdown of the extraneous encoding is hopefully neglibible I'm writing this so later if we learn that this tradeoff isn't worth it we at least understand why I made the change in the first place. DiffTrain build for commit facebook/react@8ea96ef. Changelog: [Internal] << DO NOT EDIT BELOW THIS LINE >> Reviewed By: sammy-SC Differential Revision: D45449587 Pulled By: tyao1 fbshipit-source-id: 76a5f34e42db5e9ad5a78b4b8f0d4e3dad2e7fcd

in facebook/react#26738 we added nonce to the ResponseState. Initially it was used in a variety of places but the version that got merged only included it with the external fizz runtime. This PR updates the config for the external fizz runtime so that the nonce is encoded into the script chunks at request creation time. The rationale is that for live-requests, streaming is more likely than not so doing the encoding work at the start is better than during flush. For cases such as SSG where the runtime is not required the extra encoding is tolerable (not a live request). Bots are an interesting case because if you want fastest TTFB you will end up requiring the runtime but if you are withholding until the stream is done you have already sacrificed fastest TTFB and the marginal slowdown of the extraneous encoding is hopefully neglibible I'm writing this so later if we learn that this tradeoff isn't worth it we at least understand why I made the change in the first place. DiffTrain build for [8ea96ef84d8f08ed1846dec9e8ed20d2225db0d3](facebook/react@8ea96ef)

…#26738) Adds support for nonce on `bootstrapScripts`, `bootstrapModules` and the external fizz runtime

in facebook#26738 we added nonce to the ResponseState. Initially it was used in a variety of places but the version that got merged only included it with the external fizz runtime. This PR updates the config for the external fizz runtime so that the nonce is encoded into the script chunks at request creation time. The rationale is that for live-requests, streaming is more likely than not so doing the encoding work at the start is better than during flush. For cases such as SSG where the runtime is not required the extra encoding is tolerable (not a live request). Bots are an interesting case because if you want fastest TTFB you will end up requiring the runtime but if you are withholding until the stream is done you have already sacrificed fastest TTFB and the marginal slowdown of the extraneous encoding is hopefully neglibible I'm writing this so later if we learn that this tradeoff isn't worth it we at least understand why I made the change in the first place.

Adds support for nonce on `bootstrapScripts`, `bootstrapModules` and the external fizz runtime DiffTrain build for commit 9545e48.

danieltott added 12 commits April 27, 2023 16:01

Add nonce support for src and module scripts

715fe00

prettier

3eb1605

clarify test title

1568a43

update attribute order

1f96d21

add nonce test to FizzServerBrowser test

90a10e5

make sure to add nonce back to script if it exists

0031469

pass nonce prop to the runtime script

2647b84

add test for nonce support

f3d98d3

Update legacy typing

5b76f96

Add nonce for rendered scripts

89ce5ad

reorder test

afb6d38

Update test to use async for some scripts

d5c6bd2

facebook-github-bot added the CLA Signed label Apr 28, 2023

This was referenced Apr 28, 2023

Bug: Support nonce for streaming scripts #26026

Closed

Bug: React 18 renderToPipeableStream missing support for nonce for bootstrapScripts and bootstrapModules #24883

Closed

sebmarkbage requested a review from gnoff April 28, 2023 15:34

Fix inline script content

b1b1940

Test in —prod actually executes the boostrapScriptContent code

danieltott mentioned this pull request Apr 28, 2023

Inline scripts generated as a result of appDir preventing use of strict CSP vercel/next.js#43743

Closed

1 task

gnoff requested changes Apr 28, 2023

View reviewed changes

use static-dynamic-static sandwich style 🥪

dc3ceba

danieltott added 2 commits April 28, 2023 15:41

Remove automatic nonce-adding to rendered scripts

bb21ba7

Merge branch 'facebook:main' into nonce-scripts

ceb387b

gnoff approved these changes Apr 28, 2023

View reviewed changes

gnoff mentioned this pull request Apr 28, 2023

Preinits should support a nonce option #26744

Merged

Update computation of script chunks

d2d41c4

gnoff merged commit 9545e48 into facebook:main May 1, 2023
36 checks passed

danieltott deleted the nonce-scripts branch May 1, 2023 16:19

gnoff mentioned this pull request May 1, 2023

[Fizz] Encode external fizz runtime into chunks eagerly #26752

Merged

acdlite mentioned this pull request May 3, 2023

Upgrade React to 18.3.0-canary-b7972822b-20230503 vercel/next.js#49158

Merged

wizardlyhel mentioned this pull request Aug 17, 2023

Add a default content security policy to the skeleton template Shopify/hydrogen#1235

Merged

EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024

Add nonce support to bootstrap scripts and external runtime (facebook…

4d621c1

…#26738) Adds support for nonce on `bootstrapScripts`, `bootstrapModules` and the external fizz runtime

bigfootjon pushed a commit that referenced this pull request Apr 18, 2024

Add nonce support to bootstrap scripts and external runtime (#26738)

5f70437

Adds support for nonce on `bootstrapScripts`, `bootstrapModules` and the external fizz runtime DiffTrain build for commit 9545e48.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add nonce support to bootstrap scripts and external runtime #26738

Add nonce support to bootstrap scripts and external runtime #26738

danieltott commented Apr 28, 2023 •

edited

react-sizebot commented Apr 28, 2023 •

edited

gaearon commented Apr 28, 2023

danieltott commented Apr 28, 2023

danieltott commented Apr 28, 2023

gnoff commented Apr 28, 2023

gnoff Apr 28, 2023

danieltott Apr 28, 2023

gnoff Apr 28, 2023

danieltott Apr 28, 2023

danieltott commented Apr 28, 2023

danieltott commented Apr 28, 2023

gnoff left a comment

gnoff Apr 28, 2023

gnoff Apr 28, 2023

gnoff commented Apr 28, 2023

danieltott commented Apr 28, 2023

danieltott commented Apr 28, 2023

gnoff commented May 1, 2023 •

edited

danieltott commented May 1, 2023

gnoff commented May 1, 2023

danieltott commented May 1, 2023

gaearon commented May 5, 2023

	const inlineScriptWithNonce =
	nonce === undefined
	? startInlineScript
	: stringToPrecomputedChunk(
	'<script nonce="' + escapeTextForBrowser(nonce) + '">',
	);

Add nonce support to bootstrap scripts and external runtime #26738

Add nonce support to bootstrap scripts and external runtime #26738

Conversation

danieltott commented Apr 28, 2023 • edited

Summary

Related issues:

Changes:

How did you test this change?

react-sizebot commented Apr 28, 2023 • edited

Critical size changes

Significant size changes

gaearon commented Apr 28, 2023

danieltott commented Apr 28, 2023

danieltott commented Apr 28, 2023

gnoff commented Apr 28, 2023

gnoff Apr 28, 2023

Choose a reason for hiding this comment

danieltott Apr 28, 2023

Choose a reason for hiding this comment

gnoff Apr 28, 2023

Choose a reason for hiding this comment

danieltott Apr 28, 2023

Choose a reason for hiding this comment

danieltott commented Apr 28, 2023

danieltott commented Apr 28, 2023

gnoff left a comment

Choose a reason for hiding this comment

gnoff Apr 28, 2023

Choose a reason for hiding this comment

gnoff Apr 28, 2023

Choose a reason for hiding this comment

gnoff commented Apr 28, 2023

danieltott commented Apr 28, 2023

danieltott commented Apr 28, 2023

gnoff commented May 1, 2023 • edited

danieltott commented May 1, 2023

gnoff commented May 1, 2023

danieltott commented May 1, 2023

gaearon commented May 5, 2023

danieltott commented Apr 28, 2023 •

edited

react-sizebot commented Apr 28, 2023 •

edited

gnoff commented May 1, 2023 •

edited