Repository navigation
Conversation
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.
Problem
As reported in #185:
server/securityHeaders.js:136-138mapped any unrecognisedCSP_MODEto'report-only'without saying anything, so a typo such asCSP_MODE=enforcleft the legacy'unsafe-inline'policy enforcing and thehash-based strict policy stuck in
Content-Security-Policy-Report-Onlyforever,with no log line to explain why.
cloudbuild.yamlanddeploy-with-secret.shalready reject bad values, but anout-of-band
gcloud run services update --update-env-vars CSP_MODE=...bypassesboth scripts and lands straight in this module.
Change
resolveCspMode(raw, warn = console.warn):'report-only'(unchanged default, no warning);report-only/enforce→ returned unchanged, no warning;csp.mode.invalidwarning naming the rejected value andthe effect, then
'report-only'.CSP_MODEnow comes fromresolveCspMode(process.env.CSP_MODE).tests/securityHeaders.test.mjscover default, happy pathand the typo case (asserting the warning fires exactly once).
I kept the fallback as
report-onlyrather than flipping toenforce: silentlyjumping to hash-based enforcement on a typo could brick a deploy (server.js
refuses to boot under
enforcewhen no inline hashes are available), so the fixmakes the downgrade loud instead of changing which policy wins.
Verification (local toolchain)
Node 22.14.0 / npm 10.9.2,
npm test(node:test):The 28 failures are all in unrelated suites that this PR does not touch — zstd/cache
codecs, rar listing via
bsdtar, WinDBG polling, data-use agreement/client — and comefrom missing external binaries/network in this sandbox. Every
securityHeaderstestpasses, including the three new ones.