Repository navigation
build(csp): the hash scan reads documents with a parser, not a tag walker - #658
Conversation
github-actions
Bot
added
area: infra
Wire-size diff vs merge base
Brotli q11, the quality the edge serves. Every number here is deterministic: Client assetsNo change. 25 files, 86.35 KiB Brotli total. PagesNo change. 54 files, 571.62 KiB Brotli total. Worker bundleNo change. 240.26 KiB gzip. Dictionary deltas (
|
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
aadhar-sh | f26e09a | Aug 28 2026, 07:53 PM |
…lker Step 7c writes the sha256 map the enforcing `script-src` is built from. It read HTML with a hand-rolled tag walker, correct only because it had been patched three times against the served bytes. That is the last hand-rolled scanner in the HTML passes, and this build has already caught three naive ones on its own minified output: a `<script` search that read /garage/horizon's XSS demo payload as two real scripts, a link scanner that found 33 refs where there were 2645, and the attribute scanner in this very step, which had to learn quotes for the same reason. minify-html unquotes attributes and decodes character references inside quoted values, so anything written against the AUTHORED HTML reads the MINIFIED HTML wrong. HTMLRewriter is lol-html, the SAME engine src/worker/ runs, so the build now reads a document the way the Worker does. lib/link-integrity.ts made this exact move on 2026-08-20; this finishes it. Salvaged from the parked bun branch (#443) and re-pointed at the current layout. BYTE-IDENTICAL, which is the bar rather than a nice result. csp-hashes.ts feeds the Worker bundle and /a/ URLs are content-addressed, so one moved byte re-mints assets and orphans dictionary snapshots. Building both scanners against the same tree: 110 inline blocks across 54 documents, 110 hashes either way, csp-hashes.ts identical, and 1570 of 1570 staged files identical. 19ms for the 54 documents. 107 lines of walker out, a 130-line module with its argument in the header in. The tools type ratchet drops 504 to 501, since three of build.ts's errors were the walker's untyped locals. WHAT THE TESTS PIN, each with a control that fails without it: - only executable scripts are hashed. Bare, module and speculationrules yes; application/json and ld+json no, because they are parsed as data and never reach script-src, and 31 of this site's inline blocks are data. Control: making every type executable fails it. - an empty <script></script> still hashes the empty string. It has no text node, which is why the capture closes on the END TAG. Control: hashing only blocks that produced text fails it. - a body is hashed as SOURCE bytes, so `&` is not decoded first. Control: decoding the chunk fails it. - a srcdoc document's scripts hash against the EMBEDDING page, identically to the same script in the parent. Control: dropping the recursion fails it. - document order, an unterminated script throwing, and the parser requirement naming HTMLRewriter rather than dying on an undefined symbol. The handler test carries the trap as an ASSERTION rather than a comment: the naive /\son\w+=/ finds three handlers in the fixture and the parser finds one, and the test fails if that stops being true, so the fixture cannot quietly stop testing anything. lint, typecheck, 519 tests, tools:check, pages:check, photos:check, perf-budget, 149 routes and a wrangler dry-run at 708.52 KiB / 239.88 KiB gzip.
95c114d to
92c2cde
Compare
|
Blocked on one thing, and it needs the dashboard: What is ruled out:
The artifact is identical, so the failure cannot be in what gets uploaded. It is in the Top hypothesis: the image's bun is older than Could you open the build and paste the error? One line of it will confirm or kill this. If it is |
Attempt at the Workers Builds failure, which is the one red check on this PR and the only one nothing here can reproduce. WHAT IS RULED OUT. `validate` runs the same `bun run build` on a runner and passes. perf-diff builds BOTH the merge base and HEAD on linux-x64 and reports no change in any category, so it is not the platform. A pristine clone plus `bun install --frozen-lockfile` plus `bun tools/build.ts` exits 0. The artifact is byte-identical to the old walker's, 0 of 1570 staged files differing, so it cannot be what gets uploaded. Re-running the identical tree (amend, force-push, new build id) failed again, so it is not a flake. That leaves the build step inside Cloudflare's image, whose log needs the dashboard and whose bun version is recorded nowhere in this repo. `wrangler` has no builds subcommand. THE HYPOTHESIS, stated as one because it is not verified: el.onEndTag() was the newest HTMLRewriter API this module used, and the only part of the surface that lib/link-integrity.ts — the other parser caller, which builds fine in that image on main — does not exercise. So it goes. The replacement reserves a slot at the script's OPEN tag and lets the text handler fill it, which keeps document order exact without needing to know when the element closed. An empty <script></script> emits no text chunk at all, probed rather than assumed, so reserving at open is also what keeps it from being dropped. The text handler is scoped to `script` rather than `*`, and that is load-bearing rather than tidy: with a `*` handler an empty script is handed the next text the stream produces. Measured on `<script></script><p>following text</p>`, the body comes back as "following text", so the page would ship a hash for a script that does not exist while the real empty block stayed blocked. THAT ONE WAS UNTESTED UNTIL THE CONTROL SAID SO. Five mutations were run against the new implementation and four failed a test; scoping the handler to `*` failed nothing, because no fixture had an empty script followed by text. The comment was asserting something no test proved, which is the shape this repo keeps catching. There is a fixture now, and the control fails with it. ONE BEHAVIOUR CHANGES, and toward the browser. The old walker THREW on an unterminated <script>, because it went looking for a close tag and found none. That was a property of the walker: lol-html closes the text node at EOF with the body a browser would run, probed here, so hashing it is right and throwing would refuse to cover a block that really does execute. The test pins the new answer rather than dropping the assertion. Still byte-identical: 110 blocks across 54 documents, csp-hashes.ts unchanged, 0 of 1570 staged files differing. lint, typecheck, 520 tests, pages:check, tools:check, perf-budget, photos:check, 149 routes, dry-run 708.52 KiB / 239.88 KiB gzip.
|
Fixed, and the hypothesis above is confirmed: it was The A/B is clean, since the only thing that moved in code the image executes was that one API:
The general shape is worth more than the fix. The replacement reserves the hash slot at the script's OPEN tag and lets the text handler fill it, so document order stays exact and an empty One thing the fix turned up on its own. Scoping the text handler to One behaviour changed, toward the browser: an unterminated |
1 participant
Salvaged from the parked bun branch (#443) before it is retired, re-pointed at
the current layout.
The change
Step 7c writes the sha256 map the enforcing
script-srcis built from, and itread HTML with a hand-rolled tag walker that was correct only because it had
been patched three times against the served bytes.
That was the last hand-rolled scanner in the HTML passes, and this build has
already caught three naive ones on its own minified output: a
<scriptsearchthat read
/garage/horizon's XSS demo payload as two real scripts, a linkscanner that found 33 refs where there were 2645, and the attribute scanner in
this very step, which had to learn quotes for the same reason. minify-html
unquotes attributes and decodes character references inside quoted values, so
anything written against the authored HTML reads the minified HTML wrong.
HTMLRewriter is lol-html, the same engine
src/worker/runs, so the build nowreads a document the way the Worker does.
lib/link-integrity.tsmade thisexact move on 2026-08-20; this finishes it.
107 lines of walker out, a 130-line module with its argument in the header in.
Byte-identical, which is the bar
csp-hashes.tsfeeds the Worker bundle and/a/URLs are content-addressed, soone moved byte re-mints assets and orphans dictionary snapshots. Building both
scanners against the same tree:
csp-hashes.ts19ms for the 54 documents. The tools type ratchet drops 504 → 501, since three
of
build.ts's errors were the walker's untyped locals.What the tests pin, and the control for each
Every one was run as a mutation against the module and fails without the
behaviour it names:
speculationrulesyes;application/jsonandld+jsonno)<script></script>still hashes the empty string&is not decoded firstsrcdocdocument's scripts hash against the EMBEDDING pageThe data-block one is worth the most: 31 of this site's inline blocks are data,
so a scanner wrong there is wrong 31 times. The
srcdocone is the liveregression from 2026-08-16, where
/garage/horizon's#mb-frameuptime counterwould have frozen at 0 with nothing logged.
Document order, an unterminated script throwing, and the parser requirement
naming HTMLRewriter rather than dying on an undefined symbol are pinned too.
The handler test carries the trap as an assertion rather than a comment: the
naive
/\son\w+=/finds three handlers in the fixture and the parser finds one,and the test fails if that stops being true, so the fixture cannot quietly stop
testing anything.
Validation
lint,typecheck, 519 tests,tools:check,pages:check,photos:check,perf-budget(hard checks green), 149 routes, and a wranglerdry-run at 708.52 KiB / 239.88 KiB gzip.
Stacks cleanly with #657 — that one touches step 5 and adds a block after
7b-links, this one replaces the body of 7c.