Bug: Support nonce for streaming scripts

[kentcdodds]

React version: 18.2.0

Steps To Reproduce

1. Have a server-rendered React app with a CSP that specifies a nonce for scripts
2. Try to use suspense + streaming

The current behavior

As noted in @sebmarkbage's comment, nonce support should be added to the scripts that React injects for suspense + streaming. That doesn't appear to be happening. I can't find where in the source code those scripts are generated, but in the built code I notice there's no nonce support of any kind:

var startInlineScript = stringToPrecomputedChunk('<script>');
var endInlineScript = stringToPrecomputedChunk('</script>');
var startScriptSrc = stringToPrecomputedChunk('<script src="');
var startModuleSrc = stringToPrecomputedChunk('<script type="module" src="');
var endAsyncScript = stringToPrecomputedChunk('" async=""></script>');

That appears above a function called escapeBootstrapScriptContent which for the life of me I can't find in the react repo 🤷‍♂️

The expected behavior

I should be able to provide a nonce to React (presumably at the renderToPipeableStream and hydrateRoot calls?) so React can include those in the scripts it creates.

[kentcdodds]

This already exists on renderToPipeableStream: https://beta.reactjs.org/reference/react-dom/server/renderToPipeableStream

Sorry for the noise.

[danieltott]

To be clear, this only exists for inline scripts via bootstrapScriptContent at the moment. The nonce attribute should be added to bootstrapScripts and bootstrapModules to support the strict-dynamic source list keyword.

[gaearon]

@danieltott Mind filing a new issue with more details?

[sebmarkbage]

Seems reasonable. PR?

[danieltott]

Updated issue and PR in progress - will update later tonight 👍

[danieltott]

Would adding nonce support to the other script types be a bug or a feature?

[sebmarkbage]

External runtime should have it too.

<script> rendered inside the tree I'm not sure about. On one hand, if you can render JSX, you can probably own the page anyway but maybe not. In general we consider JSX safe but it might be a little too broad to grant access to every script tag. You really probably should be providing it manually in user space at that point.

[danieltott]

Actually I think #24883 covers it (with my comment additions). I'm not sure if adding a new issue is appropriate/useful, since it'll be pretty much the same.

@sebmarkbage As far as scripts rendered inside the tree, it's actually a non-issue when using strict-dynamic. A script approved by strict-dynamic initially is allowed to add more "non-parser-inserted scripts" which I believe will cover anything React will do.

If you're not using strict-dynamic, you can add whatever you need to either the script or the CSP manually.

[danieltott]

External runtime should have it too.

@sebmarkbage can you clarify? I'm adjusting the code in

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

Line 198 in 540bab0

export function createResponseState(

, which is used by all of the non-legacy server files (although the static ones don't make use of nonce, which makes sense).

I'm new to the React codebase so just want to make sure I cover my bases.

[sebmarkbage]

You can add a startScriptSrc similar to startInlineScript in response state which contains the string to emit as the start tag. Then use responseState.startScriptSrc instead of startScriptSrc everywhere.

However, we also might need to add it to the code that adds a script tag to load the external runtime.

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

Line 4069 in 540bab0

internalPreinitScript(resources, src, integrity);

However, I'm not sure if it need it for all preinit or if preinit should accept nonce as an argument. cc @gnoff

[danieltott]

@sebmarkbage I filed a PR here: #26738

Pretty sure I handled all the cases we'd need to cover.

[gnoff]

@sebmarkbage generally I'm of the mind that we should not auto-nonce preinits or user scripts. I opened #26744 to add the ability to provide a nonce while preiniting