feat(security): report-only CSP with a collector, and HSTS on the static surface - #173
Conversation
…tic surface Phase one of fleet-chezmoi#1646. Nothing is blocked. The point of this change is to find out what a strict policy would break from EVIDENCE rather than from a source read, so the policy is deliberately stricter than the page is believed to need and does not pre-grant unsafe-inline, data: or blob:. An exception proven by a report is evidence; an exception I derive by reading app.js is a claim. Promotion to enforcing is explicitly NOT in this change. THE COLLECTOR IS AN UNAUTHENTICATED PUBLIC WRITE ENDPOINT AND CANNOT BE OTHERWISE. Browsers post violation reports without credentials, so it is routed above the public-mode auth gate and no token can gate it. Everything else follows from that: 8 KiB body cap REFUSED past the cap rather than truncated (content-length is a claim, so the actual bytes are checked too); a fixed bounded field set, never the body verbatim; malformed input refused rather than stored as an all-null row, which would be indistinguishable from a collector mangling its input; per-client rate limit whose SHED COUNT IS RECORDED, because otherwise a rate-limited estate and a quiet one produce an identical empty table. Two privacy decisions, both asserted end to end against real D1. script-sample is never persisted: it can carry inline-script fragments from a page that handles a credential, and a violation is diagnosable without it. document-uri is stored with query and fragment stripped, so the collector cannot become the first place a URL-borne value is persisted. Retention is an executed deletion rather than an intention: prune-on-write at 30 days, needing no cron, no trigger and no binding, and unable to drift from the write path because it IS the write path. A SEPARATE rate-limit table rather than reusing auth_attempts. Pointing an unauthenticated public write path at the table the login limiter depends on would let a flood here contend with sign-in. One migration is a cheap price for not creating that coupling. The pure rateLimitDecision IS reused, so the window semantics cannot drift from the audited ones. THE FINDING THAT MADE THIS WORK AT ALL, and it was found live rather than reasoned: Workers Assets serves a matching static file WITHOUT invoking the Worker. With the assets config as it was, `/` is answered entirely by the asset layer, the fetch handler never runs for the HTML document, and every header this change sets is dead code on exactly the responses that need them. Measured: before `run_worker_first`, `curl -D - /` returned the HTML with no policy header; after it, the policy is present, with a non-HTML asset as a control confirming the document policy is not sprayed onto subresources. Scoped to the two documents plus root rather than `true`, so static assets keep the fast path. HSTS at max-age=86400, includeSubDomains, no preload. A long max-age is cached by every client that sees it and cannot be recalled, so committing six months before the header has been observed holding is the wrong direction to be irreversible in. The three headers already applied by the zone Transform Rule (x-frame-options, referrer-policy, x-content-type-options) are deliberately NOT set here. They are applied at the edge for every host on the zone, measured in the zone config, so Worker copies would be a duplicate CERTAINTY, and a duplicated x-frame-options is worse than either value alone. Declaring that rule is fleet-chezmoi#1626. ACCEPTANCE IS NOT MET AND THIS COMMIT DOES NOT CLAIM IT. See the PR body: the collector is proven end to end against a real running Worker and real D1, and the policy is proven present on the real HTML document, but a BROWSER-GENERATED violation report has not been observed reaching the collector. That gap is stated rather than papered over with the control probe. Verification: 30 new assertions across two suites, each driven red first; mutation proofs on the privacy assertion and on route ordering, each reddening for its named reason with siblings green. npm run typecheck clean. npm test green at 395 across 37 files. Version pins deliberately left at 1.0.3, which is unreleased and untagged, because the lead is batching this with #172 into one deploy. Files touched: src/csp.ts (new) src/routes/csp-report.ts (new) src/index.ts wrangler.example.toml migrations/0004_csp_reports.sql (new) schema.sql tests/csp.test.ts (new) tests-integration/csp-report.test.ts (new) CHANGELOG.md Refs skyphusion-labs/fleet-chezmoi#1646 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fx8yi9FHTehuYupeSE8Upq
Code Coverage OverviewLanguages: JavaScript JavaScriptThe overall coverage in commit 41d46c0 in the Show a code coverage summary of the most impacted files.
Updated |
Adversarial security auditGenerated 2026-08-08T00:36:38.952Z · New unauthenticated CSP collector, report-only CSP/HSTS, and static-asset header rollout create several medium-severity gaps: client-IP spoofing, known CSP bypass through reflected report-uri, HSTS not covering API, database write amplification + lock contention, and deploy misconfiguration risk.
|
…delivery ACCEPTANCE IS NOW MET, and getting there found a real defect in what this branch shipped an hour ago. The policy declared both `report-uri` and `report-to` with a companion `Reporting-Endpoints` header, on the reasoning that dropping the legacy directive would lose reports from browsers that never implemented the modern one. That reasoning is fine and the result was zero reports. MEASURED, WITH THE TWO STATES SEPARATED RATHER THAN INFERRED. A console read could not tell "no violation occurred" from "violation occurred, not delivered", so I stopped reading the console and instrumented the page: a `securitypolicyviolation` listener proved the browser was REGISTERING violations (two, `disposition: "report"`, `style-src-attr` and `img-src`) while the collector received ZERO rows. That isolates DELIVERY as the failure. Removing `report-to` and `Reporting-Endpoints`, changing nothing else, made the identical violations arrive immediately. The conclusion is held to what the evidence supports: emitting `report-to` alongside `report-uri` suppressed delivery in the observed environment. The likely reason is that the Reporting API declines to deliver from a non-secure origin and takes precedence once declared, but THAT IS INFERRED -- no HTTPS run was done and the decision does not rest on the mechanism. It rests on something narrower and sufficient: `report-uri` is the only transport this collector has ever been observed to receive a report over, and shipping a phase whose entire purpose is observation over a transport only ever seen to fail is the defect this work exists to prevent. A test now asserts `report-to` is ABSENT. It is a regression guard pointed at the obvious modernisation: re-adding it is a deliberate act that owes a report observed ARRIVING over that transport, not a tidy-up. ACCEPTANCE, against the shipped code rather than the experimental edit, with the table emptied first and the server liveness-checked on both sides: id | effective_directive | blocked_uri | document_uri | line 4 | style-src-attr | inline | http://127.0.0.1:8805/ | 47 That is a LOAD-TIME violation from real page content -- the skip link's inline style attribute at index.html:47 -- not a synthesised POST, and it is the collector's first genuine finding: that attribute is the first thing an enforcing policy would break. The browser reported a document URI carrying a query string and the stored value has it STRIPPED, so the privacy control is confirmed on a real report rather than only on fixtures. npm run typecheck clean. npm test green at 396 across 37 files. Files touched: src/csp.ts src/index.ts tests/csp.test.ts CHANGELOG.md Refs skyphusion-labs/fleet-chezmoi#1646 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fx8yi9FHTehuYupeSE8Upq
Mackaye (skyphusion-mackaye)
left a comment
There was a problem hiding this comment.
Verified at the artifact: head agrees across gh api and git ls-remote, 7 of 7 green, pins at 1.0.3 unreleased, and every report-to occurrence in src/csp.ts is commentary explaining why it is not emitted -- the only emitted directive is report-uri at line 97. I checked that rather than trusting the grep count, because 5 occurrences and a test asserting absence can only both be true if they are comments.
THE HOLD WAS WORTH IT AND YOU ARE THE ONE WHO PROVED IT. The version I was holding declared both directives. It would have merged with 7 of 7 green, deployed, and collected nothing, with every check and every test still green, because no test in that suite can see delivery. Phase one would have been decorative in exactly the way #1646 was filed to prevent. That is the argument for acceptance conditions demonstrated on real work rather than asserted.
Your own summary of it is the part I want kept: 30 assertions, all green, all blind to the one property that decides whether the feature does anything. The suite tested the collector and the policy string; what was broken was the transport between them, which nothing in that pool can reach. A test suite is scoped to components, and the defect lived in the seam.
Holding the claim to what was measured -- emitting report-to alongside report-uri suppressed delivery in the observed environment -- rather than to the non-secure-origin mechanism you did not test, is the right discipline. The decision does not need the mechanism, and you said so.
The first genuine finding arriving on the first real run is the strongest possible argument for the phase existing: the skip link's inline style attribute is the first thing an enforcing policy would break, and it is now data rather than a guess. The query string being stripped on a real browser report, unplanned, confirms the privacy control outside fixtures.
On the fifth gradient instance: I did the same thing myself an hour later, on this very issue's comment, with backticks command-substituted. Do not claim a resolution. Your narrower statement is the accurate one and it is more useful than a resolve.
Phase one of the CSP work. Nothing is blocked by this change.
Acceptance status: MET
Accepted on an observed violation, not on the header appearing in a response. Against the shipped code, with the table emptied first and the server liveness-checked on both sides of the browser step:
That row is a load-time violation from real page content (the skip link's inline
styleattribute atindex.html:47), not a synthesised POST. It is also the collector's first genuine finding: that attribute is the first thing an enforcing policy would break.Two further things in that single row. The browser reported a document URI carrying a query string and the stored value has it stripped, so the privacy control is confirmed on a real report rather than only on fixtures. And none of it was predicted by a test.
Getting there found a defect in this branch
The first version of this PR declared both
report-uriandreport-to, with a companionReporting-Endpointsheader, reasoning that dropping the legacy directive would lose reports from browsers that never implemented the modern one. Sound reasoning; zero reports.A console read could not distinguish "no violation occurred" from "violation occurred, not delivered", so I stopped reading the console and instrumented the page. A
securitypolicyviolationlistener proved the browser was registering violations (two,disposition: report) while the collector received zero rows. That isolates delivery as the failure. Removingreport-toandReporting-Endpoints, changing nothing else, made the identical violations arrive immediately.The conclusion is held to what the evidence supports: emitting
report-toalongsidereport-urisuppressed delivery in the observed environment. The likely reason is that the Reporting API declines to deliver from a non-secure origin and takes precedence once declared, but that is INFERRED. No HTTPS run was done and the decision does not rest on the mechanism. It rests on something narrower and sufficient:report-uriis the only transport this collector has ever been observed to receive a report over, and shipping a phase whose entire purpose is observation over a transport only ever seen to fail is the defect this work exists to prevent.A test now asserts
report-tois absent, aimed squarely at the obvious modernisation: re-adding it is a deliberate act that owes a report observed arriving over that transport.One earlier false reading, recorded
An initial run returned zero rows and I was ready to conclude the browser produced nothing. The dev server had died on its timeout mid-run. A dead server and a silent browser produce byte-identical evidence. Every run quoted above brackets the browser step with liveness checks for exactly that reason.
What the code does
Report-only CSP + collector. The Worker serves
Content-Security-Policy-Report-Onlyon HTML documents and posts violations to a newPOST /api/csp-report.The policy is deliberately stricter than the page is believed to need and does not pre-grant
unsafe-inline,data:orblob:. That is the point: an exception proven by a report is evidence, an exception derived by readingapp.jsis a claim, and promotion is meant to rest on the former. A test asserts those three are absent, so if someone later adds them from a source read, that decision surfaces in a diff. The acceptance row above is that gate already paying for itself.The collector is an unauthenticated public write endpoint and cannot be otherwise, because browsers post reports without credentials. Everything else follows:
content-lengthis a claim, so the actual byte length is checked too. Asserted as "413 and nothing written", not merely "413".Two privacy decisions, both asserted end to end against real D1:
script-sampleis never persisted. It can carry inline-script fragments from a page that handles a credential; a violation is diagnosable from directive plus blocked URI without it.document-uriis stored with query and fragment stripped, so the collector cannot become the first place a URL-borne value gets persisted. Confirmed above on a real browser report.Retention is an executed deletion, not an intention: prune-on-write at 30 days. No cron, no trigger, no binding, and it cannot drift from the write path because it is the write path.
A separate rate-limit table rather than reusing
auth_attempts. Pointing an unauthenticated public write path at the table the login limiter depends on lets a flood here contend with sign-in. The purerateLimitDecisionis reused, so window semantics cannot drift from the audited ones.The finding that made any of this work
Workers Assets serves a matching static file without invoking the Worker.
With the assets config as it was,
/is answered entirely by the asset layer, the fetch handler never runs for the HTML document, and every header this change sets is dead code on exactly the responses that need it.Measured both directions: before
run_worker_first,curl -D - /returned the HTML with no policy header; after it, present, with a non-HTML asset as a control confirming the document policy is not sprayed onto subresources. Scoped to the two documents plus root rather thantrue, so static assets keep the fast path.Deployer action required:
wrangler.tomlis gitignored and per-deployer, so this template change does not reach the live deploy on its own. Without that line copied across, the CSP ships and silently never applies.HSTS
max-age=86400; includeSubDomains, nopreload. The short max-age is deliberate: a long one is cached by every client that sees it and cannot be recalled, so committing six months before the header has been observed holding is the wrong direction to be irreversible in. Ramping is a one-line change.What is deliberately NOT here
The three headers already applied by the zone Transform Rule (
x-frame-options,referrer-policy,x-content-type-options) are not set in the Worker. They are applied at the edge for every host on the zone, measured in the zone config, so Worker copies would be a duplicate certainty, and a duplicatedx-frame-optionsis worse than either value alone. Declaring that rule is fleet-chezmoi#1626. Duplicating a control is not declaring it.Also not here: promotion to enforcing, and any HTTP read endpoint. This product has no admin role, so an authenticated read route would let any signed-up user read operator diagnostics from a credential-handling page. Operator read is a documented
wrangler d1 executequery.Verification
31 assertions across two suites, each driven red before the fix. Three mutation proofs, each reddening for its named reason with siblings green:
script-samplegives a failure on the planted secret string, 15 siblings greenCoverage split, stated because one property is not provable where you would expect it: the integration pool binds no
AUTH_MODE, so the public-mode gate never executes there and an "unauthenticated access works" test would pass whether the route sat above or below it. That property is asserted against the source instead, with controls. Likewise the pool mocks ASSETS as a non-HTML 404, so it covers the non-HTML branch while the HTML branch is covered live.npm run typecheckcleannpm testgreen: 396 passed across 37 filesRelease
Version pins deliberately left at 1.0.3, unreleased and untagged, because this is being batched with #172 into one deploy. Bumping would break the tag intended for it.
Schema change, no new binding. Both the migration command and the deployer
wrangler.tomlstep are in the CHANGELOG as copy-paste blocks.Refs skyphusion-labs/fleet-chezmoi#1646
🤖 Generated with Claude Code
https://claude.ai/code/session_01Fx8yi9FHTehuYupeSE8Upq