fix(server): cap every request body to prevent a JSON/MCP OOM

.changeset/cap-request-body-size.md

@@ -0,0 +1,15 @@
+---
+"sideshow": patch
+---
+
+Cap every request body so an oversize JSON or MCP payload can't OOM the server.
+The previous 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). A global `bodyLimit` now rejects
+any request body over a generous ceiling with a 413, short-circuiting on an
+oversize `Content-Length` and otherwise aborting 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.

server/app.ts

@@ -1,4 +1,5 @@
 import { Hono, type Context } from "hono";
+import { bodyLimit } from "hono/body-limit";
 import { getCookie, setCookie } from "hono/cookie";
 import { streamSSE } from "hono/streaming";
 import { decodeBase64 } from "./base64.ts";
@@ -23,6 +24,14 @@ import { validateSurfaceParts } from "./surfaceParts.ts";
 
 const MAX_SURFACE_BYTES = 2 * 1024 * 1024;
 const MAX_WAIT_SECONDS = 300;
+// Hard ceiling on any request body, applied globally. Every write endpoint
+// reads its body with an unbounded `c.req.json()` (and /mcp likewise), so
+// without this a single oversize POST is an out-of-memory flood — and the local
+// default ships with no auth token, so those endpoints are reachable
+// unauthenticated. Sized to clear the largest legitimate body — a base64 asset
+// uploaded over MCP, ~4/3 of the 5 MiB asset cap — while still bounding a flood.
+// The /api/assets route's own 5 MiB streaming cap is stricter and still applies.
+const MAX_BODY_BYTES = 16 * 1024 * 1024;
 // Bound the session trace: each step's detail is truncated and the per-session
 // list rolls, so memory stays flat no matter how long the agent runs.
 const MAX_TRACE_STEPS = 2000;
@@ -77,41 +86,6 @@ const isAssetKind = (v: unknown): v is AssetKind => v === "image" || v === "trac
 // them with the real origin so a deployed instance shows copy-pasteable URLs.
 const LOCAL_ORIGIN = "http://localhost:8228";
 
-// Read a request body into one Uint8Array, stopping and returning null once it
-// exceeds `limit`. Lets an upload route bound memory before it knows the real
-// size: a chunked request sends no Content-Length, so a plain `arrayBuffer()`
-// would buffer the entire stream into memory first. Web-streams only
-// (`Request.body`), so it stays runtime-agnostic across Node and Workers.
-async function readBodyCapped(req: Request, limit: number): Promise<Uint8Array | null> {
-  const stream = req.body;
-  if (!stream) {
-    const buf = new Uint8Array(await req.arrayBuffer());
-    return buf.byteLength > limit ? null : buf;
-  }
-  const reader = stream.getReader();
-  const chunks: Uint8Array[] = [];
-  let total = 0;
-  for (;;) {
-    const { done, value } = await reader.read();
-    if (done) break;
-    if (!value) continue;
-    total += value.byteLength;
-    if (total > limit) {
-      await reader.cancel();
-      return null;
-    }
-    chunks.push(value);
-  }
-  if (chunks.length === 1) return chunks[0];
-  const out = new Uint8Array(total);
-  let offset = 0;
-  for (const chunk of chunks) {
-    out.set(chunk, offset);
-    offset += chunk.byteLength;
-  }
-  return out;
-}
-
 export type AuthenticateHook = (
   request: Request,
 ) => boolean | Response | Promise<boolean | Response>;
@@ -559,6 +533,26 @@ export function createApp({
     return c.text("unauthorized — open this page as /?key=<your token>", 401);
   });
 
+  // Cap every request body. Runs after auth, so an unauthenticated request on a
+  // token-protected board is rejected (401) before its body is ever read; on a
+  // no-token board it still bounds the body. bodyLimit short-circuits on an
+  // oversize Content-Length and otherwise streams-and-aborts at the cap, so a
+  // chunked body (no Content-Length) can't slip past either. /api/assets is
+  // exempt here because it applies its own, stricter cap (limitAssetBody below).
+  const limitBody = bodyLimit({
+    maxSize: MAX_BODY_BYTES,
+    onError: (c) => c.json({ error: "request body too large" }, 413),
+  });
+  app.use("*", (c, next) => (c.req.path === "/api/assets" ? next() : limitBody(c, next)));
+
+  // The asset route's own (tighter) body cap. Keeps the asset limit and its
+  // wording, and bounds the upload before it is read — bodyLimit refuses an
+  // oversize Content-Length outright and aborts a chunked stream at the cap.
+  const limitAssetBody = bodyLimit({
+    maxSize: MAX_ASSET_BYTES,
+    onError: (c) => c.json({ error: `asset exceeds ${MAX_ASSET_BYTES} bytes` }, 413),
+  });
+
   // --- pages and docs ---
 
   const withOrigin = (text: string, c: { req: { url: string } }) =>
@@ -918,20 +912,12 @@ export function createApp({
   // and a JSON client both work, and MCP can ride base64. The body is read once
   // and only treated as an envelope when it is application/json carrying a
   // base64 `data` string; a raw JSON asset (no top-level `data`) stays raw.
-  app.post("/api/assets", async (c) => {
+  app.post("/api/assets", limitAssetBody, async (c) => {
     const mime = (c.req.header("content-type") ?? "").split(";")[0].trim().toLowerCase();
-    // Bound the body twice. The Content-Length header is an early-out for
-    // honest clients, but a chunked upload sends no Content-Length — so
-    // readBodyCapped enforces the same cap while streaming, stopping before it
-    // buffers the whole body into memory (an unauthenticated OOM otherwise). The
-    // post-decode cap in uploadAsset still applies (a base64 envelope decodes to
-    // ~3/4), so the true asset limit is enforced however the bytes arrive.
-    const declaredLen = Number(c.req.header("content-length") ?? 0);
-    if (declaredLen > MAX_ASSET_BYTES) {
-      return c.json({ error: `asset exceeds ${MAX_ASSET_BYTES} bytes` }, 413);
-    }
-    const buf = await readBodyCapped(c.req.raw, MAX_ASSET_BYTES);
-    if (!buf) return c.json({ error: `asset exceeds ${MAX_ASSET_BYTES} bytes` }, 413);
+    // limitAssetBody has already bounded the body to MAX_ASSET_BYTES, so this
+    // read is safe. The post-decode cap in uploadAsset still applies (a base64
+    // envelope decodes to ~3/4), enforcing the true asset limit on the bytes.
+    const buf = new Uint8Array(await c.req.arrayBuffer());
     let envelope: any = null;
     if (mime === "application/json") {
       try {

test/api.test.ts

@@ -1370,9 +1370,9 @@ test("caps a chunked upload with no Content-Length instead of buffering it", asy
 
 test("assembles a valid multi-chunk streamed upload and stores it intact", async () => {
   const app = makeApp();
-  // A streamed body under the cap must be accepted, and readBodyCapped must
-  // stitch its chunks back together in order — every other upload test sends a
-  // single chunk, so this is the only cover for the concat path. We read the
+  // A streamed body under the cap must be accepted and its chunks reassembled
+  // in order — every other upload test sends a single chunk, so this is the only
+  // cover for a multi-chunk body surviving the bodyLimit re-wrap. We read the
   // asset back and compare bytes so a wrong offset/order would fail loudly.
   const stream = new ReadableStream({
     start(controller) {
@@ -1397,6 +1397,30 @@ test("assembles a valid multi-chunk streamed upload and stores it intact", async
   assert.deepEqual([...new Uint8Array(await served.arrayBuffer())], [1, 2, 3, 4, 5, 6, 7, 8, 9]);
 });
 
+test("the global body cap rejects oversize JSON and MCP bodies", async () => {
+  const app = makeApp();
+  // Every write endpoint reads its body with an unbounded c.req.json(); the
+  // global bodyLimit must refuse an oversize one with a 413 before it is read.
+  // An over-cap Content-Length is the cheap path (no body buffered) — assert it
+  // fires on a REST write endpoint and on /mcp, the two body-reading surfaces.
+  const oversize = {
+    "content-type": "application/json",
+    "content-length": String(17 * 1024 * 1024),
+  };
+  const surfaces = await app.request("/api/surfaces", {
+    method: "POST",
+    headers: oversize,
+    body: new Uint8Array(0), // no bytes sent — the Content-Length check fires first
+  });
+  assert.equal(surfaces.status, 413);
+  const mcp = await app.request("/mcp", {
+    method: "POST",
+    headers: oversize,
+    body: new Uint8Array(0),
+  });
+  assert.equal(mcp.status, 413);
+});
+
 test("uploading to an unknown session 404s; serving a missing asset 404s", async () => {
   const app = makeApp();
   const res = await app.request(