fix(server): cap every request body to prevent a JSON/MCP OOM#118
Merged
Conversation
The asset-upload fix bounded /api/assets, but every other write endpoint (/api/surfaces, /api/comments, /api/sessions, the trace ingest, /api/theme) and /mcp still read their body with an unbounded c.req.json() — so the same unauthenticated out-of-memory vector was reachable by POSTing a giant JSON body instead. The local default has no auth token, so those endpoints are reachable unauthenticated. Add a global bodyLimit that 413s any request body over a generous ceiling (sized to clear a base64 asset over MCP). It short-circuits on an oversize Content-Length and otherwise aborts the stream at the cap, so a chunked body can't slip past. It runs after auth — unauthenticated requests on a token board are refused before their body is read — and exempts /api/assets, which streams its own stricter cap. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…dyCapped The asset route had a hand-rolled streaming cap (readBodyCapped) plus a manual Content-Length check. Hono's bodyLimit does both — short-circuits an oversize Content-Length and aborts a chunked stream at the cap — so scope one to the route (at the asset limit, with the asset-specific 413 message) and read the now-bounded body with arrayBuffer(). Same behavior and limit, one primitive instead of a bespoke reader. /api/assets stays exempt from the global body cap so its tighter limit is the only one that applies. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
The previous PR (#117) bounded
POST /api/assets, but that was only the binary half of the same vector. Every other write endpoint —/api/surfaces,/api/snippets,/api/comments,/api/sessions,/api/sessions/:id/trace,/api/theme— and/mcpstill read their body with an unboundedc.req.json(). So the same unauthenticated out-of-memory flood was still reachable — an attacker just POSTs a multi-gigabyte JSON body to/api/surfaces(or/mcp). The local default ships with no auth token, so on any board reachable beyond localhost these endpoints are open. (On Cloudflare Workers the platform caps request bodies ~100 MB by plan; the Node self-hosted path has no such backstop — so this matters most there.)Fix
Use Hono's built-in
bodyLimiteverywhere, in two scopes:MAX_BODY_BYTES = 16 MiBglobal — clears the largest legitimate body (a base64-encoded asset over MCP, ~4/3 of the 5 MiB asset cap) while bounding a flood.bodyLimitshort-circuits on an oversizeContent-Lengthand otherwise aborts the stream at the cap, so a chunked body (noContent-Length) can't slip past — the gap fix(server): cap the asset-upload body while streaming to prevent an OOM #117 closed for assets./api/assetsis exempt from the global cap and instead carries its ownbodyLimitat the (stricter) asset limit, so the tighter bound is the only one that applies.Also folds in the asset route onto the primitive
This PR additionally replaces #117's hand-rolled
readBodyCapped(a custom streaming reader + manualContent-Lengthcheck) with the scopedbodyLimitabove — same behavior and limit, one framework primitive instead of a bespoke reader (net −37 lines). Done here rather than as a follow-up so we don't merge the custom reader only to delete it.Tests
test/api.test.tsaddsthe global body cap rejects oversize JSON and MCP bodies(over-capContent-Length→ 413 on a REST write endpoint and/mcp). The asset route's existing tests from #117 — oversizeContent-Length, chunked-stream cap (asserts the read stops early), and multi-chunk reassembly — all still pass against the scopedbodyLimit, confirming the swap preserved behavior. The streaming-abort path itself is Hono'sbodyLimit(tested upstream).npm run typecheck,npm run lint,npm run format:check,npm test(205/205) ✅🤖 Generated with Claude Code