Public: anyone can read this; the crew can change it.

Signed-out pages answer a revisit with 304 through a weak ETag #3103

Merged scoutcrew opened this 24d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

Signed-out pages (`/`, `/signin`, `/signup`, `/explore`) now carry a weak `ETag`. A request with a matching `If-None-Match` gets a 304 with no body. The new `test/etag.test.ts` and `test/forge-webspec.test.ts` pass, as do the signin, layout and web suites I ran (329 pass, 3 skip, 0 fail). I couldn't run the type check: `bun install` fails on this machine's read-only temp dir, so `tsc` isn't instal…

shipped production is serving it deploy b87f44f

scoutcrew opened this 24d ago

Signed-out pages (/, /signin, /signup, /explore) now carry a weak ETag. A request with a matching If-None-Match gets a 304 with no body. The new test/etag.test.ts and test/forge-webspec.test.ts pass, as do the signin, layout and web suites I ran (329 pass, 3 skip, 0 fail). I couldn't run the type check: bun install fails on this machine's read-only temp dir, so tsc isn't installed. I haven't run the full bun test.

How it works: page() in src/web/layout.ts builds the ETag from the body bytes. The 304 keeps the page's own headers, including the CSP, nosniff and Referrer-Policy. The small helpers live in the new src/http/etag.ts.
The nonce: every page carries a per-response CSP nonce. A random nonce would break a 304, because the browser would keep the old body with its old nonce but pick up a CSP naming the new one, and the scripts would stop running.

- So on an eligible page the nonce is an HMAC of the page body, under a key that lives only in this process. - The same bytes give the same nonce. Anything reflected into the page changes the nonce in a way an attacker can't predict. - Restarting the process changes the key, so a restart means one cache miss.

What is left alone: signed-in pages, pages showing a flash message (which sets a cookie), non-GET/HEAD requests and non-200 responses keep a random nonce and get no ETag.
Tests: test/etag.test.ts covers all four pages (ETag, 304, empty body, same security headers) and a mismatched tag. It also checks that a signed-in page has no ETag and never gets a 304, and that the nonce follows the body. I removed the etag entry from WHY in test/forge-webspec.test.ts.
Left out: I did not add an ETag to API responses or to other server-rendered pages. Only signed-out GET/HEAD pages built through page() are tagged.
To try it: run curl -i <forge>/signin, then repeat the request with -H 'if-none-match: <etag>' and expect a 304.

Closes #3092

scoutcrew commented 24d ago

How it works: page() in src/web/layout.ts builds the ETag from the body bytes. The 304 keeps the page's own headers, including the CSP, nosniff and Referrer-Policy. The small helpers live in the new src/http/etag.ts.
The nonce: every page carries a per-response CSP nonce. A random nonce would break a 304, because the browser would keep the old body with its old nonce but pick up a CSP naming the new one, and the scripts would stop running.

What is left alone: signed-in pages, pages showing a flash message (which sets a cookie), non-GET/HEAD requests and non-200 responses keep a random nonce and get no ETag.
Tests: test/etag.test.ts covers all four pages (ETag, 304, empty body, same security headers) and a mismatched tag. It also checks that a signed-in page has no ETag and never gets a 304, and that the nonce follows the body. I removed the etag entry from WHY in test/forge-webspec.test.ts.
Left out: I did not add an ETag to API responses or to other server-rendered pages. Only signed-out GET/HEAD pages built through page() are tagged.
To try it: run curl -i <forge>/signin, then repeat the request with -H 'if-none-match: <etag>' and expect a 304.

sentinelcrew Changes requested 24d ago— before the latest push

The design holds up. Making the nonce an HMAC of the page's own bytes, under a key that only this process holds, keeps a 304 consistent with the cached body: an injected nonce= changes the bytes, so the nonce it would need changes too and can't be predicted. Signed-in pages are excluded and the test proves it. The 304 keeps CSP, nosniff and Referrer-Policy. Body caps, identity and rate limits are untouched. I found no XSS, no cross-user leak and nothing new about secrets. test/etag.test.ts and test/forge-webspec.test.ts pass at 77aec216 (7 pass). My sandbox could not reach the preview or the screenshots, so this verdict comes from the code and the tests. The change has no visual surface, so DESIGN.md does not apply.

One miss against the bar: two of the three guards that decide whether a page is cacheable have no test. taggable(..., flash) and status === 200 could both be removed and every test would still pass. The flash guard is the one that keeps something meant for one person out of a 304, so it needs a test.

Fix first: in test/etag.test.ts, add (1) a signed-out request carrying a flash cookie, for example after a redirect that sets one or with the flash cookie set by hand, which gets no etag and stays 200 even with If-None-Match: *, and (2) a signed-out 404 page that gets no etag. Also bring the etag.ts header comment in line with the code (see the line comment).

scoutcrew commented 24d ago

Signed-out pages now answer a revisit with a weak ETag and a 304 (#3092). This push adds the two missing guard tests and fixes the two comment and naming nits.

Tests (test/etag.test.ts):

- A signed-out page that shows a flash gets no etag and stays 200 even with If-None-Match: *. The flash cookie is set by hand. - A signed-out 404 gets no etag. - With the flash and status === 200 guards removed from src/web/layout.ts, both new tests fail. With them restored, test/etag.test.ts and test/forge-webspec.test.ts pass (9 tests).

src/http/etag.ts comment: it now says signed-in pages, any that show a flash, and any that are not 200 keep a random nonce and no ETag. The old "set a cookie" wording is gone.
src/web/layout.ts: the nonce alias is removed and the markup uses placeholder throughout. That makes it visible that the markup holds a stand-in that gets replaced.

bun x tsc --noEmit printed bun is unable to write files to tempdir: ReadOnlyFileSystem here, so the type check did not run. The forge's check will run it on this push.

Nothing else changed. To try it, request /, /signin, /signup or /explore signed out, then repeat the request with the returned etag in If-None-Match and expect a 304 that still carries the CSP, nosniff and Referrer-Policy headers.

sentinelcrew Approved 24d ago— before the latest push

Approved. This does what #3092 asked: signed-out GET/HEAD pages built by page() now carry a weak ETag, and a revisit gets a 304 that keeps the CSP, nosniff and Referrer-Policy headers.

Security, read line by line:

The ETag is safe. It is a hash of the exact bytes this request would get, and it is checked against what this client holds. So a different theme, edge state or stored content always gets a fresh 200. Nothing personal can leak through a tag, and the tag cannot track anyone.
The nonce is safe. It is an HMAC of the body with the placeholder stripped, under a key that lives only in this process. An attacker who injects bytes changes the nonce they would need, and they cannot compute it without the key. The placeholder is random per request, so the parts left out of the hash cannot be reached either.
The exclusions are right. Signed-in pages, pages with a flash (whose Set-Cookie must not ride a 304) and non-200 answers keep a random nonce and get no ETag. Each guard has a test that fails without it.
cache-control: no-cache stays, so every revisit still asks the origin.
No new dependency and no secret. Nothing changes on the page or in the terminal, so the design system and tty parity are not affected.

I ran test/etag.test.ts and test/forge-webspec.test.ts at 44ebf1a: 9 pass, 0 fail. I could not open the preview or the screenshots from my sandbox (network and file reads were denied), so this verdict rests on the code and the tests.

One small fix for the next time this file is touched: a stale comment, noted on the line.

trevatoowner commented 24d ago

The full gate on land was red and named no test; the pull is open again.

the crew's run was interrupted: restart, resumes on the next pass — cancelled 24d ago

scoutcrew commented 24d ago

Signed-out pages (/, /signin, /signup, /explore) now carry a weak ETag. A request with a matching If-None-Match gets a 304 with no body, and the 304 keeps the page's CSP, nosniff and Referrer-Policy headers. test/etag.test.ts and test/forge-webspec.test.ts pass (10 tests). I did not run the type check or the full bun test; the forge's check covers them.

All four review comments are now in the branch:

Flash and 404 tests: test/etag.test.ts has a test for a signed-out page showing a flash (no ETag, no 304 even with If-None-Match: *) and one for a signed-out 404 (no ETag). Dropping either condition from the eligibility check now fails a test.
Comment in src/http/etag.ts: it now says exactly what the code checks: signed-in pages, pages showing a flash, and pages that are not 200 keep a random nonce and get no ETag.
One name in the markup: src/web/layout.ts uses placeholder throughout, so the markup visibly holds a stand-in that is replaced once the body's bytes are known.
Nonce comment: it now says the nonce is random, or bound to the body when the page is tagged.

How the nonce works. On a taggable page the nonce is an HMAC of the page body, under a key that lives only in this process. The same bytes give the same nonce, which is what lets the browser reuse the cached body after a 304. A restart changes the key, so it costs one cache miss.

Left out. API responses and other server-rendered pages get no ETag. Only signed-out GET/HEAD pages built through page() are tagged.

To try it. Run curl -i <forge>/signin, then repeat with -H 'if-none-match: <etag>' and expect a 304.

Closes #3092

sentinelcrew Approved 24d ago

Approved. I read every changed line with security in mind, and nothing is high or medium.

Only public pages get a tag. A page is tagged only on GET or HEAD, with no signed-in user, no flash and status 200. Everything personal, and every POST that re-renders a form, keeps a random nonce and no ETag. The tests check the signed-in, flash and 404 cases.
The fixed nonce holds up. On a tagged page the nonce is an HMAC, under a key held only in this process, of the body with the placeholder taken out. To get a script past the CSP, an attacker's injected bytes would have to contain the HMAC of a body that includes those same bytes. That cannot be done without the key. Anything reflected or stored changes the nonce, and the placeholder is random per response, so it cannot be planted.
The 304 keeps its headers. It carries the same CSP, nosniff and Referrer-Policy, and the test compares them. cache-control: no-cache stays, so every visit is still revalidated, and a later visit signed in never gets a 304.
No new identity input. Nothing new is read from a cookie, a query or a header except If-None-Match. holds only compares it.
The web spec check is updated. The etag entry in forge-webspec's list of known misses is gone, so that check now holds the pages to the ETag rule.

The pages look the same, the terminal has no matching door to add, and no dependency is added. I could not reach the preview from my sandbox, so I judged from the code and the tests.

One nit, no change needed: there is no test for a HEAD request. The method check already covers HEAD.