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

The floor on every door, including the ones that stream #29

Merged opened this

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

Merged

`GET /events` was the one door outside the floor because `events()` built its own `Response`; it now gets its headers from `eventStream()` in `src/http.ts`, and the MCP 202 gets them from `empty()`, so every answer this app makes carries `SECURITY`. The guard is a table in `src/server.test.ts` built from `routes` itself plus the answers that aren't rows (a versioned asset, a 404, a refused cross-s…

shipped production is serving it deploy feb7d92

opened this

GET /events was the one door outside the floor because events() built its own Response; it now gets its headers from eventStream() in src/http.ts, and the MCP 202 gets them from empty(), so every answer this app makes carries SECURITY. The guard is a table in src/server.test.ts built from routes itself plus the answers that aren't rows (a versioned asset, a 404, a refused cross-site write, a 413) — I checked it fails on both a bare streaming response and a bodiless 202 before it passed.

The CSP now names script-src 'self' and connect-src 'self' ws(s)://<this host>, and style-src 'unsafe-inline' is gone: nothing in the tree renders an inline style or an inline script (only a linked app.css and a type="module" script), so #7 is free to rewrite the CSS as long as it keeps styling in the stylesheet. On ws:/wss: the decision is in the comment above SECURITY: 'self' should cover the socket under CSP3 but browsers disagreed for years, so the one socket address is named in full and a bare ws: — which would permit a socket to any host — is not written.

socketOrigin(request) sits beside sameOrigin for #9 to call rather than reinvent: Origin present and equal to publicOrigin. A missing Origin passes sameOrigin (a script judged by its forge identity, on a POST a browser can't forge cross-origin) and fails socketOrigin, because an upgrade has no preflight behind it; the comment and the test both state the difference. It is tested as a predicate, since the socket door itself lands in #9.

/events is rationed against the same bucket as a write, and bucketOf now returns { key, shared } because the numbers cannot be the same: six streams per person, sixty-four for one address, since behind the forge every guest arrives from the forge's address and one small cap would lock out every stranger at once. Over the cap is a 429 with the floor on it, not another socket held open. The listener and the slot are released exactly once — abort, cancel or a failed write — listenerCount() is exported, and src/live.test.ts opens N streams, hangs up and asserts zero; a HEAD of a stream now cancels the body it doesn't get. A Last-Event-ID or ?since= that isn't a whole count replays from the start instead of nothing.

To look at: the header table and socketOrigin in src/server.test.ts, the release path in events(), and the preview's / and /events headers — locally both carry the identical policy and / answers warm in under a millisecond. I did not add a tsconfig or typecheck the tree (that is #22; bunx tsc finds no bun types today), and I did not touch the page's CSS, leaving that to #7.

🤖 Generated with Claude Code

Closes #17

Changes requested — before the latest push

Full review is the anchored comment on src/server.test.ts:272: rebase onto main (three-file conflict, and the too-large door 404s once /notes is gone, so the 413 is uncovered), and add a test for the HEAD-of-a-stream release.

commented

Review submitted on #29 as changes requested (one review, anchored to src/server.test.ts:272; the pull now shows changesRequested: 1). Nothing was pushed and the clone is exactly as I found it.

What I found:

Blocking — the branch is not based on the tree it will land on. Head 1b24617 branches from f60d5fd, before #26 retired the notes scaffold. git merge main conflicts in README.md, src/routes.ts and src/server.test.ts, and the worst hunk interleaves main's existing floor test with four new ones. Worse than the conflict: once resolved, the centrepiece weakens silently — BODY["POST /notes"] and PARAM.id are dead on main, and the two hand-pushed doors point at /notes, which main answers 404. The cross-site door survives (sameOrigin runs before match, so still 403), but the body over the cap door 404s before a byte is read, so the 413 the description claims as covered is not covered. Fix: send the oversized body to POST /mcp. The preview confirms the staleness from the front — it still renders the notes page.

Blocking — untested behaviour. The HEAD-of-a-stream release is new and has no test; the only HEAD test is HEAD /. I verified both directions against a running copy of the head: with the cancel, listenerCount() and streamCount() are 0 after eight HEAD /events; with it removed, HEAD /events never completes at all.

Non-blocking notes. If server.requestIP is the forge's address for everyone, STREAM_MAX_SHARED = 64 is one bucket for all guests, so one script holding 64 EventSources denies the live world to every other guest — still better than the unbounded status quo, but worth an issue. And socketOrigin currently has no caller.

What is genuinely right: the floor is on every door in the preview (/events carries the identical CSP), 'unsafe-inline' is safe to drop — main's arena page and static/live.js use no inline style, no inline script, no eval, no innerHTML — the after() fix for Last-Event-ID is real, and the release-exactly-once path is correct. bun test is green on the branch: 28 pass.

Two artifacts I could not clean: the reviews endpoint rejects a state-only body with bad_anchor, and probing its shape left two comments reading p on src/live.ts:76; there is no delete route. My review body says they are mine and to ignore them. The state itself had to go through the forge's own review-state route, since the API exposes no route for it.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs