build: WALLET-1364 — emit the CSP nonce only for Chrome production builds - #1429
Open
ost-ptk wants to merge 1 commit into
Open
build: WALLET-1364 — emit the CSP nonce only for Chrome production builds#1429ost-ptk wants to merge 1 commit into
ost-ptk wants to merge 1 commit into
Conversation
The nonce was generated per build and injected via DefinePlugin for every browser target, although only the Chrome-production CSP arm pins style-src to it. Both readers are browser-agnostic, so every Firefox and Safari bundle carried a fresh random base64 literal that nothing used — and AMO source review rebuilds the submitted source and compares the artifact byte for byte, which a per-build random literal makes impossible. The DefinePlugin key stays defined for all targets on purpose: nothing else defines `process` in these bundles, so dropping the key would leave process.env.CSP_NONCE unreplaced and throw ReferenceError at runtime. Only the value becomes null off the Chrome-production path, and both readers now skip the attribute entirely rather than stamping nonce="null". Deriving the nonce from the commit hash was considered and rejected: a publicly computable nonce adds nothing over a static one, while gating removes the value from the non-Chrome bundles altogether. Verified: two consecutive build:firefox runs are byte-identical after this change and differed before it; build/chrome still carries a real nonce in its manifest CSP.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ticket: https://make-software.atlassian.net/browse/WALLET-1364 (P1 item 4)
The problem
webpack.config.jsgenerated a random CSP nonce per build and injected it throughDefinePluginfor every browser target, even though only the Chrome-production CSP arm pinsstyle-srcto it. Both readers —src/set-webpack-nonce.ts(webpack's__webpack_nonce__, read by style-loader) andsrc/libs/ui/style-sheet-manager.tsx(styled-components) — are browser-agnostic, so every Firefox and Safari bundle carried a fresh random base64 literal that nothing ever used.AMO source review rebuilds the submitted source and compares the result to the uploaded artifact byte for byte. A per-build random literal makes that impossible.
The fix
The value is generated only when
isChrome && !isDev; every other target getsnull.The
DefinePluginkey stays defined for all targets on purpose. Nothing else definesprocessin these bundles, so dropping the key would leaveprocess.env.CSP_NONCEunreplaced in the output and throwReferenceError: process is not definedat runtime. Only the value changes.Both readers skip the attribute entirely rather than stamping a bogus one — verified against the installed library sources, not assumed: style-loader does
if (nonce) setAttribute("nonce", nonce), andStyleSheetManagertypes the prop asnonce?: undefined | string, sonull ?? undefinedis the correct falsy shape.Deriving the nonce from the commit hash was considered and rejected: a publicly computable nonce adds nothing over a static one, whereas gating removes the value from the non-Chrome bundles altogether.
Verification
npm run build:firefoxruns are byte-identical after this change and differed before it — the difference was confined to exactly the five bundles that prepend the nonce setter, which matches the entry wiring.build/chrome/manifest.jsonstill carries a realnonce-<base64>in its CSP, and the Chrome bundle still sets it at runtime.null/undefinedstring leaked into the Firefox output where the nonce used to be.npm run ci-checkpasses.