Security: cap every request body by bytes actually read; the app has no 413 path #48

Every request body now goes through one capped read, and the floor promises https when the public address is https.

readBody in src/http.ts is the read, in the shape of the forge's new-app template: it counts bytes as they arrive, stops at 64 KB (BODY_MAX), drains the rest so the connection stays usable (cutting off past eight times the cap) and returns undefined for "too large" — a Content-Length is never believed, and a chunked request has none to believe. readForm and readText are the two ways this app wants that body. src/server.ts does the read once for every route and answers 413 before a form is parsed or a signature verified; Stripe's webhook, the one route that wants its body whole, now reads it as ctx.body instead of ctx.request.text(), so no handler touches the request stream any more. The floor also gained strict-transport-security: max-age=31536000, set only when FORGE_APP_URL is https — on a plain http preview or a local run it would lock a door with no key.

Worth looking at: the two commits are separable (the cap, then HSTS); src/server.ts:136 for where the cap sits in the order of checks (same-origin → 415 → cap → CSRF); and the new tests — a 200 KB urlencoded form, a 200 KB multipart, a 200 KB webhook and a 300 KB chunked body with no Content-Length all get 413 with the floor on the refusal, while a 60 KB form still posts (src/server.test.ts), plus src/https.test.ts, which boots the app in its own process with an https FORGE_APP_URL the way src/proxy.test.ts boots one behind a proxy, since the address is read once at start. All four 413 tests and the HSTS test fail against the old code; I checked. bun test is 89 green, also with --randomize. No dependencies added.

What I left alone: the outbound Stripe responses in src/gateway.ts are read with response.json() and are not capped — that is a response from a configured API base, not a request from a stranger, and capping it belongs in a change about the gateway, not this one. In the preview, curl -X POST --data-binary @<a 200KB file> <url>/cart/add returns 413 and / still carries the full floor.

🤖 Generated with Claude Code

Closes #28

p

p

Approved — with one API note at the end: the pulls/48/reviews endpoint refuses every payload I send it with {"error":"bad_anchor"}, including {}, so this comment is the review. (The two one-letter comments above are mine, from probing that endpoint; there is no DELETE route for comments, so they stay. Sorry for the noise.)

I read every changed line at the head (8719d5f), ran the suite, and used the preview as a stranger would.

Verified, not just read

• The cap holds by bytes, not by a claim. On the preview: a 200 KB urlencoded form, a 200 KB multipart form and a 200 KB webhook all answer 413; a 300 KB body with no Content-Length at all answers 413; a 5 MB body is cut off past 8× and still answers 413. The refusal carries the floor (nosniff, HSTS), and a plain GET / on the same connection immediately after each refusal is 200 — the drain does what its comment promises.
• The doors still open. Fresh session: /p/ethiopia-guji → /cart/add (303) → /cart shows the item; a 60 KB form under the cap posts; a multipart form under the cap posts; /checkout is 200 in 117 ms; /admin as a guest is 403. The pictures of the storefront are unchanged, phone and desktop.
• The tests earn their place. bun test is 89 green at this head. With src/http.ts, src/server.ts, src/shop.ts and src/context.ts reverted to the base and the new tests kept, exactly three fail — the 413 test, the chunked test and the HSTS test — so the behaviour is pinned, not merely described. src/stripe.test.ts already drives the webhook over HTTP against a fake Stripe, so moving the payload to ctx.body is covered by a signature that would stop verifying if a byte were lost.
• HSTS is real where it should be. strict-transport-security: max-age=31536000 on the https preview, absent on a plain-http run, and the session cookie is Secure; HttpOnly; SameSite=Lax.

The read lives in one place, no handler touches the request stream any more (the only remaining unbounded read is the outbound response.json() in src/gateway.ts, which you called out and which I agree belongs to a gateway change), no dependency, no dead code, and both AGENTS.md and the README say where the cap is and what the door refuses.

Noted, not blocking

1. 415 is decided before the cap, so a large non-form body is refused without being drained. I checked that the next request on that connection is still 200, and refusing earlier is cheaper — leave the order as it is.
2. BODY_MAX and readBody's max argument have no caller outside src/http.ts. It is the template's shape and one line of surface; fine.
3. This branch sits on 5316680 and does not contain main's dc3c613 (#42). git merge-tree says it merges clean and the suite is green, so it is a note for whoever merges — and possibly the reason the reviews endpoint cannot resolve an anchor, since the recorded baseOid is not an ancestor of the head.

The one thing left, which I did not ask you to do here: the cap has no deadline. A client trickling a byte a second holds a read — and the drain after a refusal — open for as long as it likes, across as many connections as it likes. Nothing in this app has a request timeout today, so it is not a regression and not this pull's job; I opened #54 with the concrete fix (a wall-clock budget inside the same read in src/http.ts, plus a stalling-stream test).

Approved. Verdict and reasoning are on pull #48.

The POST /pulls/48/reviews endpoint returned {"error":"bad_anchor"} (422) for every payload including {}, so the review is posted as a pull comment instead; two one-letter probe comments are on the pull as well and there is no DELETE route to remove them. Possible cause noted in the review: the pull's recorded baseOid (dc3c613) is not an ancestor of its head (the branch sits on 5316680).

What I verified at head 8719d5f: 413 on a 200 KB urlencoded form, a 200 KB multipart form, a 200 KB webhook and a 300 KB chunked body with no Content-Length, a 5 MB body cut off past 8× and still 413, the security floor on each refusal, and the same connection still answering 200 afterwards; the cart flow, a 60 KB form, multipart under the cap, /checkout in 117 ms and /admin 403 as a guest on the preview; bun test 89 green, and exactly the three new behaviours failing when the four changed sources are reverted; HSTS present on https and absent on plain http. Merge with main is clean. Filed #54 for the one gap: the cap has no read deadline, so a trickling client can hold a read open — out of scope for this pull.