diff --git a/apps/api/src/body-limits.ts b/apps/api/src/body-limits.ts index 12e4ea9..f4551f0 100644 --- a/apps/api/src/body-limits.ts +++ b/apps/api/src/body-limits.ts @@ -1,6 +1,10 @@ -import { MODULE_ROUTE_MAX_BODY_BYTES_CEILING } from "@ekklesia/contracts"; -import type { Context } from "hono"; -import { bodyLimit } from "hono/body-limit"; +import { + MODULE_ROUTE_ABSOLUTE_MAX_BODY_BYTES, + MODULE_ROUTE_MAX_BODY_BYTES_CEILING, + MODULE_ROUTE_STREAMED_MAX_BODY_BYTES_CEILING, + type ModuleRouteBodyMode, +} from "@ekklesia/contracts"; +import type { Context, MiddlewareHandler } from "hono"; /** * The platform's one request-body policy, centralized here rather than scattered as magic numbers @@ -19,15 +23,20 @@ import { bodyLimit } from "hono/body-limit"; * field individually bounded well under 1 KiB combined; 16 KiB leaves headroom without inviting * abuse. * - `MODULE_ROUTE_MAX_BODY_BYTES_CEILING` (1 MiB, re-exported from `@ekklesia/contracts` — the same - * constant a module's own route declaration is validated against at registration) is the hard - * ceiling no module route may exceed and doubles as the platform's overall request-body ceiling: - * nothing in this platform ever declares, or enforces, a larger single request body than this. - * Large media transfer is explicitly out of scope for ordinary module HTTP bodies — it belongs to - * a future dedicated storage design, not to widening this ceiling. + * constant a module's own **buffered** route declaration is validated against at registration) is + * the ceiling for every body the platform materializes before a handler runs. It has not moved. + * - `MODULE_ROUTE_STREAMED_MAX_BODY_BYTES_CEILING` (32 MiB) is what a route may declare when it + * explicitly opts into `bodyMode: "streamed"` (ADR 0042), and doubles as + * `MODULE_ROUTE_ABSOLUTE_MAX_BODY_BYTES`: no route, in any mode, may declare or accept a larger + * single request body, and no unbounded option exists anywhere in the contract. */ export const AUTH_REQUEST_MAX_BODY_BYTES = 8 * 1024; export const CORE_MUTATION_MAX_BODY_BYTES = 16 * 1024; -export { MODULE_ROUTE_MAX_BODY_BYTES_CEILING }; +export { + MODULE_ROUTE_ABSOLUTE_MAX_BODY_BYTES, + MODULE_ROUTE_MAX_BODY_BYTES_CEILING, + MODULE_ROUTE_STREAMED_MAX_BODY_BYTES_CEILING, +}; /** The one stable, sanitized public contract for every request-body-size rejection across the * platform — auth, Core, and module routes alike. Never leaks the configured limit, the offending @@ -37,12 +46,173 @@ export function requestBodyTooLargeResponse(c: Context): Response { return c.json({ code: "REQUEST_BODY_TOO_LARGE" }, 413); } -/** A single shared `hono/body-limit` factory so every call site uses the identical sanitized 413 - * contract rather than redefining `onError` per route. Covers both a declared oversized - * `Content-Length` and an oversized chunked/streamed body with no declared length — see - * `hono/body-limit`'s pinned implementation, which checks `Content-Length` synchronously before - * ever reading the stream, and otherwise reads and counts the stream incrementally, rejecting as - * soon as the running total exceeds `maxSize` rather than buffering the whole body first. */ -export function platformBodyLimit(maxSize: number) { - return bodyLimit({ maxSize, onError: requestBodyTooLargeResponse }); +/** + * A declared `Content-Length`, or `undefined` when there is not exactly one well-formed one. + * + * A malformed or contradictory value is treated as *absent* rather than as a rejection, deliberately: + * the byte counting below bounds the request either way, so refusing here could only ever turn a + * legitimate request away without making an illegitimate one any less bounded. Node's own HTTP + * parser rejects genuinely malformed framing before this code sees the request at all. + */ +function declaredContentLength(request: Request): number | undefined { + const header = request.headers.get("content-length"); + if (header === null || !/^\d{1,15}$/u.test(header.trim())) { + return undefined; + } + return Number.parseInt(header.trim(), 10); +} + +/** Releases a body nothing consumed, so an abandoned or oversized upload does not hold a socket + * open waiting to be read. Safe on a stream that is locked, already drained, or already errored. */ +function releaseUnreadBody(request: Request): void { + const body = request.body; + if (!body || body.locked) { + return; + } + void body.cancel().catch(() => undefined); +} + +/** + * Reads at most `maxSize` bytes, stopping and cancelling the source the moment the running total + * exceeds it. Never allocates beyond what it has already accepted, so an over-limit body costs one + * chunk past the limit rather than the whole upload. + */ +async function readBounded( + body: ReadableStream, + maxSize: number, +): Promise { + const reader = body.getReader(); + const chunks: Uint8Array[] = []; + let size = 0; + try { + for (;;) { + const { done, value } = await reader.read(); + if (done) { + return chunks; + } + size += value.byteLength; + if (size > maxSize) { + await reader.cancel().catch(() => undefined); + return "over-limit"; + } + chunks.push(value); + } + } finally { + reader.releaseLock(); + } +} + +function streamOf(chunks: readonly Uint8Array[]): ReadableStream { + return new ReadableStream({ + start(controller) { + for (const chunk of chunks) { + controller.enqueue(chunk); + } + controller.close(); + }, + }); +} + +/** + * The platform's one request-body limiter, for every route in the application. + * + * ## Both modes reject a declared oversize before anything else happens + * + * A `Content-Length` above the limit is refused synchronously, before authorization, before the + * handler, and before a single body byte is read. That is the cheap rejection an ordinary + * too-large upload takes. + * + * ## `buffered` — the body is materialized, bounded, before the handler runs + * + * The exact behaviour every route had before streaming existed, now with the counting done here + * rather than by a dependency. Bytes are counted as they arrive and the request is rejected the + * moment the running total passes the limit, so a lying or absent `Content-Length` is bounded by + * what actually arrived rather than by what was claimed. The handler then receives a request whose + * body is fully available and provably within its declared allowance — which is what makes + * `await request.json()` safe in a module handler. + * + * The platform holds at most the route's own declared allowance in memory, and a buffered route may + * declare at most 1 MiB. That bound is the whole reason the buffered ceiling has not moved. + * + * ## `streamed` — the body is counted as the handler consumes it, and never materialized + * + * Nothing is buffered. The body is wrapped in a counting transform and handed to the handler, which + * reads it as a stream; the moment the running total passes the limit the stream errors, the source + * is cancelled, and the response is replaced with the same sanitized `413` — even if the handler + * caught the error and tried to answer anyway. That last part matters: a limit a handler can swallow + * is not a limit. + * + * The consequence a module author must know is that in this mode a `413` can arrive **after** the + * handler has started, so a handler that persists as it reads has to be prepared for a partially + * consumed upload. Writing to a temporary key and committing only after the read completes is the + * pattern; the platform cannot do it for you, because only the module knows what a partial write + * means to its own data. + * + * ## Ordering + * + * This runs before permission evaluation for both modes, so a request that is *declared* too large + * never reaches authorization work. In `streamed` mode no body byte is read until the handler reads + * it, and the handler only runs after authorization succeeds — so an unauthorized caller can never + * push a large upload through the platform at all. + * + * ## Timeouts + * + * Body reading is bounded in time by the server's own `headersTimeout`/`requestTimeout` + * (`apps/api/src/server.ts`), not by anything here. A slow upload is a slow *request*, and the + * platform already has one answer for that rather than a second per-route one. + */ +export function platformBodyLimit( + maxSize: number, + mode: ModuleRouteBodyMode = "buffered", +): MiddlewareHandler { + return async function enforceBodyLimit(c, next) { + const request = c.req.raw; + const declared = declaredContentLength(request); + if (declared !== undefined && declared > maxSize) { + return requestBodyTooLargeResponse(c); + } + if (!request.body) { + return next(); + } + + if (mode === "buffered") { + const chunks = await readBounded(request.body, maxSize); + if (chunks === "over-limit") { + return requestBodyTooLargeResponse(c); + } + c.req.raw = new Request(request, { + body: streamOf(chunks), + // Required by undici whenever a `Request` is constructed from a stream. + duplex: "half", + } as RequestInit); + return next(); + } + + let overLimit = false; + let total = 0; + const counted = request.body.pipeThrough( + new TransformStream({ + transform(chunk, controller) { + total += chunk.byteLength; + if (total > maxSize) { + overLimit = true; + // Erroring the transform propagates back to the source and cancels it, so the socket is + // released rather than left draining an upload nobody will read. + controller.error(new Error("REQUEST_BODY_TOO_LARGE")); + return; + } + controller.enqueue(chunk); + }, + }), + ); + c.req.raw = new Request(request, { body: counted, duplex: "half" } as RequestInit); + + await next(); + + if (overLimit) { + c.res = requestBodyTooLargeResponse(c); + } + releaseUnreadBody(c.req.raw); + return; + }; } diff --git a/apps/api/src/module-http-routes.ts b/apps/api/src/module-http-routes.ts index d58253c..3bbd4bc 100644 --- a/apps/api/src/module-http-routes.ts +++ b/apps/api/src/module-http-routes.ts @@ -235,9 +235,12 @@ export function mountModuleHttpRoutes( const requireRoutePermission = requirePermission(middlewareOptions, route.permission); // The route's own conscious body contract (`route.maxBodyBytes` — 0 means no body accepted at - // all) is enforced before permission evaluation ever runs, so an oversized request never - // reaches authorization work, let alone the handler or the module database. - const routeBodyLimit = platformBodyLimit(route.maxBodyBytes); + // all) is enforced before permission evaluation ever runs, so a request *declared* oversized + // never reaches authorization work, let alone the handler or the module database. In + // `streamed` mode no body byte is read until the handler reads it, and the handler runs only + // after authorization succeeds, so an unauthorized caller can never push an upload through + // (ADR 0042). + const routeBodyLimit = platformBodyLimit(route.maxBodyBytes, route.bodyMode); const path = moduleRoutePath(route.moduleId, route.path); const finalHandler = async (c: Context): Promise => { const context: ModuleRouteHandlerContext = Object.freeze({ diff --git a/apps/api/tests/api-http.integration.ts b/apps/api/tests/api-http.integration.ts index e509faf..7855e76 100644 --- a/apps/api/tests/api-http.integration.ts +++ b/apps/api/tests/api-http.integration.ts @@ -432,6 +432,39 @@ async function createFixtureModule( async handler(request, context) { return Response.json({ created: true, principal: context.principal }, { status: 201 }); }, + }); + // An image-sized streamed upload (ADR 0042). The handler counts bytes as they + // arrive and never materializes the body, which is exactly what a real module + // storing an upload would do. + http.route({ + method: "POST", + path: "/uploads", + permission: "calendar.events.read", + maxBodyBytes: 33554432, + bodyMode: "streamed", + async handler(request) { + let bytes = 0; + if (request.body) { + const reader = request.body.getReader(); + for (;;) { + const chunk = await reader.read(); + if (chunk.done) { break; } + bytes += chunk.value.byteLength; + } + } + return Response.json({ bytes }, { status: 201 }); + }, + }); + http.route({ + method: "POST", + path: "/small-uploads", + permission: "calendar.events.read", + maxBodyBytes: 64, + bodyMode: "streamed", + async handler(request) { + await request.arrayBuffer().catch(() => undefined); + return Response.json({ swallowed: true }, { status: 201 }); + }, });` : "" } @@ -2685,6 +2718,73 @@ describe.sequential("live API — installed module permission end-to-end", () => expect(await trustedResponse.json()).toMatchObject({ created: true }); }); + it("streams one image-sized upload end to end over real HTTP", async () => { + // 25 MiB — an ordinary photograph, and the size the streamed ceiling exists to accommodate. It + // travels through a real socket, a real Node server, and the platform's byte counter, and the + // platform holds none of it (ADR 0042). + const image = new Uint8Array(25 * 1024 * 1024).fill(0x41); + + const response = await fetch(`${origin}/api/v1/modules/calendar/uploads`, { + body: image, + headers: { + "content-type": "application/octet-stream", + cookie: ownerCookie, + origin: dashboardOrigin, + }, + method: "POST", + }); + + expect(response.status).toBe(201); + expect(await response.json()).toEqual({ bytes: image.byteLength }); + }, 60_000); + + it("refuses a declared oversize on a streamed route with a sanitized 413", async () => { + const response = await fetch(`${origin}/api/v1/modules/calendar/small-uploads`, { + body: "a".repeat(65), + headers: { + "content-type": "text/plain", + cookie: ownerCookie, + origin: dashboardOrigin, + }, + method: "POST", + }); + + expect(response.status).toBe(413); + expect(await response.json()).toEqual({ code: "REQUEST_BODY_TOO_LARGE" }); + }); + + it("terminates a chunked streamed body that exceeds the limit, even when the handler swallowed it", async () => { + // No declared `Content-Length`, so only the running byte count can bound this — and the + // handler deliberately ignores the stream error and answers `201`. The platform's `413` still + // wins: a limit a handler can swallow is not a limit. + const oversize = new ReadableStream({ + pull(controller) { + controller.enqueue(new Uint8Array(512).fill(0x41)); + }, + }); + + const response = await fetch(`${origin}/api/v1/modules/calendar/small-uploads`, { + body: oversize, + duplex: "half", + headers: { cookie: ownerCookie, origin: dashboardOrigin }, + method: "POST", + } as RequestInit); + + expect(response.status).toBe(413); + expect(await response.json()).toEqual({ code: "REQUEST_BODY_TOO_LARGE" }); + }, 30_000); + + it("keeps the server healthy after an oversized upload", async () => { + // The socket was released rather than left draining, so the very next request is served + // normally. + const response = await fetch(`${origin}/api/v1/modules/calendar/events/evt-43`, { + headers: { cookie: ownerCookie }, + }); + + expect(response.status).toBe(200); + expect(await response.json()).toMatchObject({ eventId: "evt-43" }); + }); + it("rejects a well-formed permission absent from the composed registry, writing no grant or audit row", async () => { const admin = await openAdministrativeClient(database.url); try { diff --git a/apps/api/tests/module-http-routes.test.ts b/apps/api/tests/module-http-routes.test.ts index f892330..c7d1b3d 100644 --- a/apps/api/tests/module-http-routes.test.ts +++ b/apps/api/tests/module-http-routes.test.ts @@ -58,14 +58,17 @@ function fakeStore(): AuthorizationStore { }; } -/** Defaults `maxBodyBytes` to a generous value for tests that are not themselves exercising the +/** Defaults `maxBodyBytes` and `bodyMode` for tests that are not themselves exercising the * body-size contract, so the many pre-existing routing/authorization/CORS scenarios below do not * each need to restate an unrelated body-size choice; the dedicated "module route body limits" - * suite further down overrides it explicitly to prove the contract itself. */ + * suite further down overrides them explicitly to prove the contract itself. */ function moduleRoute( - fields: Omit & { maxBodyBytes?: number }, + fields: Omit & { + bodyMode?: RegisteredModuleHttpRoute["bodyMode"]; + maxBodyBytes?: number; + }, ): RegisteredModuleHttpRoute { - return Object.freeze({ maxBodyBytes: 1024, ...fields }); + return Object.freeze({ bodyMode: "buffered", maxBodyBytes: 1024, ...fields }); } /** A small typed fake `ModuleRuntimeServices` — these tests exercise route mounting, @@ -1634,3 +1637,375 @@ describe("module route overlap can never reach mounting", () => { expect(moduleRoutesOverlap(paramRoute, staticRoute)).toBe(true); }); }); + +/** + * Explicit, bounded streaming uploads (ADR 0042). + * + * The distinction being proven throughout is *when* the platform knows a body is too large, and what + * it holds while finding out. In `buffered` mode the answer is "before the handler runs, holding at + * most the route's own declared allowance". In `streamed` mode it is "as the handler consumes it, + * holding nothing" — and the `413` still wins, even when the handler tried to answer anyway. + */ +describe("module route streamed body limits", () => { + const oneMebibyte = 1024 * 1024; + + /** A body of `size` bytes delivered in realistic chunks, with no declared `Content-Length` — the + * chunked-transfer shape a browser `fetch` of a `ReadableStream` actually produces. */ + function chunkedBody(size: number, chunkSize = 64 * 1024): ReadableStream { + let sent = 0; + return new ReadableStream({ + pull(controller) { + if (sent >= size) { + controller.close(); + return; + } + const next = Math.min(chunkSize, size - sent); + sent += next; + controller.enqueue(new Uint8Array(next).fill(97)); + }, + }); + } + + function streamingApp(options: { + readonly handler: ModuleRouteHandler; + readonly maxBodyBytes: number; + readonly evaluator?: AuthorizationEvaluator; + }) { + return buildModuleApp({ + ...(options.evaluator ? { evaluator: options.evaluator } : {}), + registry: permissionRegistry([{ id: writePermission, baseline: "public" }]), + routes: [ + moduleRoute({ + bodyMode: "streamed", + handler: options.handler, + maxBodyBytes: options.maxBodyBytes, + method: "POST", + moduleId: "calendar", + path: "/uploads", + permission: writePermission, + }), + ], + }); + } + + /** Counts what the handler actually received, which is the only honest way to tell "streamed" from + * "buffered then handed over". */ + async function countingHandler(received: { bytes: number }): Promise { + return async (request) => { + const body = request.body; + if (!body) { + return Response.json({ bytes: 0 }); + } + const reader = body.getReader(); + for (;;) { + const { done, value } = await reader.read(); + if (done) { + break; + } + received.bytes += value.byteLength; + } + return Response.json({ bytes: received.bytes }); + }; + } + + it("accepts a body of exactly the declared limit", async () => { + const received = { bytes: 0 }; + const { app } = streamingApp({ + handler: await countingHandler(received), + maxBodyBytes: oneMebibyte, + }); + + const response = await app.request("/api/v1/modules/calendar/uploads", { + body: chunkedBody(oneMebibyte), + duplex: "half", + headers: { origin: trustedOrigin }, + method: "POST", + } as RequestInit); + + expect(response.status).toBe(200); + expect(await response.json()).toEqual({ bytes: oneMebibyte }); + }); + + it("rejects a declared Content-Length over the limit before the handler runs", async () => { + const received = { bytes: 0 }; + const counting = await countingHandler(received); + const handler = vi.fn(counting); + const { app } = streamingApp({ handler, maxBodyBytes: 1024 }); + + const response = await app.request("/api/v1/modules/calendar/uploads", { + body: chunkedBody(2048, 16), + duplex: "half", + headers: { "content-length": String(1025), origin: trustedOrigin }, + method: "POST", + } as RequestInit); + + expect(response.status).toBe(413); + expect(await response.json()).toEqual({ code: "REQUEST_BODY_TOO_LARGE" }); + // The cheap rejection: the declared size alone decided it, so no handler work happened and no + // byte of the upload reached module code. + expect(handler).not.toHaveBeenCalled(); + expect(received.bytes).toBe(0); + }); + + it("terminates a body whose actual size exceeds a smaller declared Content-Length", async () => { + // A misleading `Content-Length` buys nothing: the running byte count is what bounds the request, + // so a header claiming 8 bytes cannot smuggle a megabyte past the limit. + const received = { bytes: 0 }; + const { app } = streamingApp({ + handler: await countingHandler(received), + maxBodyBytes: 64, + }); + + const response = await app.request("/api/v1/modules/calendar/uploads", { + body: chunkedBody(4096, 32), + duplex: "half", + headers: { "content-length": "8", origin: trustedOrigin }, + method: "POST", + } as RequestInit); + + expect(response.status).toBe(413); + expect(await response.json()).toEqual({ code: "REQUEST_BODY_TOO_LARGE" }); + // It stopped promptly rather than draining the whole upload first. + expect(received.bytes).toBeLessThanOrEqual(96); + }); + + it("answers 413 even when the handler swallowed the stream error and tried to respond", async () => { + // A limit a handler can swallow is not a limit. + const handler = vi.fn(async (request) => { + try { + await request.arrayBuffer(); + } catch { + // Deliberately ignored, exactly as a careless module might. + } + return Response.json({ ok: "handler won" }); + }); + const { app } = streamingApp({ handler, maxBodyBytes: 64 }); + + const response = await app.request("/api/v1/modules/calendar/uploads", { + body: chunkedBody(4096, 32), + duplex: "half", + headers: { origin: trustedOrigin }, + method: "POST", + } as RequestInit); + + expect(handler).toHaveBeenCalledTimes(1); + expect(response.status).toBe(413); + expect(await response.json()).toEqual({ code: "REQUEST_BODY_TOO_LARGE" }); + }); + + it("never buffers the body: the handler sees the first bytes before the last are sent", async () => { + let released: (() => void) | undefined; + const finished = new Promise((resolve) => { + released = resolve; + }); + let firstChunkSeen = false; + let secondChunkQueued = false; + const body = new ReadableStream({ + async pull(controller) { + if (!secondChunkQueued) { + secondChunkQueued = true; + controller.enqueue(new Uint8Array(16).fill(97)); + return; + } + // Only produce the rest once the handler has demonstrably consumed the first chunk. A + // buffering platform would deadlock here rather than reach the assertion below. + await finished; + controller.close(); + }, + }); + const handler = vi.fn(async (request) => { + const reader = (request.body as ReadableStream).getReader(); + const first = await reader.read(); + firstChunkSeen = !first.done; + released?.(); + for (;;) { + const next = await reader.read(); + if (next.done) { + break; + } + } + return Response.json({ ok: true }); + }); + const { app } = streamingApp({ handler, maxBodyBytes: oneMebibyte }); + + const response = await app.request("/api/v1/modules/calendar/uploads", { + body, + duplex: "half", + headers: { origin: trustedOrigin }, + method: "POST", + } as RequestInit); + + expect(firstChunkSeen).toBe(true); + expect(response.status).toBe(200); + }); + + it("releases an aborted upload without taking the platform down", async () => { + const aborted = new ReadableStream({ + start(controller) { + controller.enqueue(new Uint8Array(16).fill(97)); + controller.error(new Error("client went away")); + }, + }); + const handler = vi.fn(async (request) => { + await request.arrayBuffer(); + return Response.json({ ok: true }); + }); + const { app } = streamingApp({ handler, maxBodyBytes: oneMebibyte }); + + const response = await app.request("/api/v1/modules/calendar/uploads", { + body: aborted, + duplex: "half", + headers: { origin: trustedOrigin }, + method: "POST", + } as RequestInit); + + // An abort is a failed request, never an unhandled rejection or a 413: nothing exceeded a limit. + expect(response.status).toBe(500); + expect(await response.json()).toEqual({ code: "MODULE_HANDLER_FAILED" }); + }); + + it("releases a body the handler never read", async () => { + let cancelled = false; + const body = new ReadableStream({ + cancel() { + cancelled = true; + }, + pull(controller) { + controller.enqueue(new Uint8Array(16).fill(97)); + }, + }); + const { app } = streamingApp({ + handler: async () => Response.json({ ok: true }), + maxBodyBytes: oneMebibyte, + }); + + const response = await app.request("/api/v1/modules/calendar/uploads", { + body, + duplex: "half", + headers: { origin: trustedOrigin }, + method: "POST", + } as RequestInit); + + expect(response.status).toBe(200); + expect(cancelled).toBe(true); + }); + + it("refuses a declared oversize before authorization, and never streams for a denied caller", async () => { + const evaluator: AuthorizationEvaluator = { + authorize: vi.fn().mockResolvedValue({ allowed: false }), + }; + const handler = vi.fn(async () => Response.json({ ok: true })); + const { app } = buildModuleApp({ + evaluator, + registry: permissionRegistry([{ id: writePermission, baseline: "restricted" }]), + routes: [ + moduleRoute({ + bodyMode: "streamed", + handler, + maxBodyBytes: 64, + method: "POST", + moduleId: "calendar", + path: "/uploads", + permission: writePermission, + }), + ], + }); + + const declaredOversize = await app.request("/api/v1/modules/calendar/uploads", { + body: "a".repeat(65), + headers: { "content-length": "65", origin: trustedOrigin }, + method: "POST", + }); + expect(declaredOversize.status).toBe(413); + expect(evaluator.authorize).not.toHaveBeenCalled(); + + // A within-limit body from a denied caller is refused by authorization, and the handler — the + // only thing that ever reads a byte in streamed mode — never runs. + const response = await app.request("/api/v1/modules/calendar/uploads", { + body: chunkedBody(32, 8), + duplex: "half", + headers: { origin: trustedOrigin }, + method: "POST", + } as RequestInit); + expect(response.status).toBe(403); + expect(handler).not.toHaveBeenCalled(); + }); + + it("still rejects an untrusted Origin before any body handling", async () => { + const handler = vi.fn(async () => Response.json({ ok: true })); + const { app } = streamingApp({ handler, maxBodyBytes: oneMebibyte }); + + const response = await app.request("/api/v1/modules/calendar/uploads", { + body: chunkedBody(4096), + duplex: "half", + headers: { origin: untrustedOrigin }, + method: "POST", + } as RequestInit); + + expect(response.status).toBe(403); + expect(await response.json()).toEqual({ code: "UNTRUSTED_ORIGIN" }); + expect(handler).not.toHaveBeenCalled(); + }); + + it("keeps every rejection sanitized, naming no limit, size, or body content", async () => { + const { app } = streamingApp({ + handler: async () => Response.json({ ok: true }), + maxBodyBytes: 64, + }); + + const response = await app.request("/api/v1/modules/calendar/uploads", { + body: "secret-payload-".repeat(64), + headers: { "content-length": String(15 * 64), origin: trustedOrigin }, + method: "POST", + }); + const body = await response.text(); + + expect(response.status).toBe(413); + expect(body).toBe('{"code":"REQUEST_BODY_TOO_LARGE"}'); + expect(body).not.toContain("secret-payload"); + expect(body).not.toContain("64"); + expect(response.headers.get("cache-control")).toBe("no-store"); + }); + + it("still materializes a buffered body before the handler runs", async () => { + // The pre-existing promise, unchanged: a buffered handler may call `request.json()` and know the + // body is already there and already within its declared allowance. + const handler = vi.fn(async (request) => { + const text = await request.text(); + return Response.json({ length: text.length }); + }); + const { app } = buildModuleApp({ + registry: permissionRegistry([{ id: writePermission, baseline: "public" }]), + routes: [ + moduleRoute({ + bodyMode: "buffered", + handler, + maxBodyBytes: 128, + method: "POST", + moduleId: "calendar", + path: "/events", + permission: writePermission, + }), + ], + }); + + const withinLimit = await app.request("/api/v1/modules/calendar/events", { + body: chunkedBody(128, 16), + duplex: "half", + headers: { origin: trustedOrigin }, + method: "POST", + } as RequestInit); + expect(withinLimit.status).toBe(200); + expect(await withinLimit.json()).toEqual({ length: 128 }); + + const overLimit = await app.request("/api/v1/modules/calendar/events", { + body: chunkedBody(129, 16), + duplex: "half", + headers: { origin: trustedOrigin }, + method: "POST", + } as RequestInit); + expect(overLimit.status).toBe(413); + // A buffered route's handler never sees an over-limit request at all. + expect(handler).toHaveBeenCalledTimes(1); + }); +}); diff --git a/core/src/modules/runtime.ts b/core/src/modules/runtime.ts index 93908ba..810caea 100644 --- a/core/src/modules/runtime.ts +++ b/core/src/modules/runtime.ts @@ -60,6 +60,7 @@ export type ModuleRuntimeIssueCode = | "MODULE_ROLLBACK_TIMEOUT" | "MODULE_ROUTE_DEFINITION_INVALID" | "MODULE_ROUTE_DUPLICATE" + | "MODULE_ROUTE_BODY_MODE_INVALID" | "MODULE_ROUTE_MAX_BODY_BYTES_INVALID" | "MODULE_ROUTE_METHOD_INVALID" | "MODULE_ROUTE_OVERLAP" @@ -467,6 +468,7 @@ class StagedModuleHttpRoutes { shapes.add(shapeKey); local.push( Object.freeze({ + bodyMode: parsed.data.bodyMode, handler: parsed.data.handler, maxBodyBytes: parsed.data.maxBodyBytes, method: parsed.data.method, diff --git a/core/tests/module-runtime.test.ts b/core/tests/module-runtime.test.ts index c731c99..00a3059 100644 --- a/core/tests/module-runtime.test.ts +++ b/core/tests/module-runtime.test.ts @@ -861,6 +861,7 @@ describe("module HTTP routes", () => { expect(runtime.routes).toHaveLength(3); for (const route of runtime.routes) { expect(Object.keys(route).sort()).toEqual([ + "bodyMode", "handler", "maxBodyBytes", "method", diff --git a/docs/architecture/decisions/0042-bounded-streaming-request-bodies.md b/docs/architecture/decisions/0042-bounded-streaming-request-bodies.md new file mode 100644 index 0000000..95f218a --- /dev/null +++ b/docs/architecture/decisions/0042-bounded-streaming-request-bodies.md @@ -0,0 +1,160 @@ +# 0042: Explicit, bounded streaming request bodies + +- Status: Accepted +- Date: 2026-08-27 +- Supersedes: none +- Related: [ADR 0015](0015-static-module-http-routing-and-runtime-lifecycle.md), + [ADR 0017](0017-public-request-perimeter-hardening.md), + [ADR 0030](0030-host-provided-module-object-storage.md), + [ADR 0031](0031-direct-object-delivery.md) + +## Context + +ADR 0017 gave every public route an enforced request-body ceiling and capped a module route at +1 MiB, with an explicit note that "large media transfer belongs to a future dedicated storage +design, never to widening this ceiling." + +The storage design arrived — ADR 0030 gave modules a scoped object store, ADR 0031 took Ekklesia out +of the *read* path with signed delivery URLs — and it did not solve the *write* path. ADR 0031 +deliberately declined to offer signed write URLs, so the only way bytes reach a module's bucket is +through that module's own HTTP route. That route is capped at 1 MiB, and an ordinary photograph is +not 1 MiB. A church uploading a slide for a lobby screen cannot, today, get the file in. + +Two properties of the existing implementation also need naming, because they are what the change has +to preserve and what it has to fix: + +- The pinned body-limit middleware **fully buffers** any body with no declared `Content-Length` + before deciding. At 1 MiB that is a bounded, acceptable cost. At image sizes it would mean the + platform holding every concurrent upload in memory. +- With a declared `Content-Length` within the limit it *counts nothing at all*, trusting the header. + Node's HTTP framing makes that safe today, but it is trust the platform does not need to extend. + +## Decision + +Extend the existing route contract. There is no second HTTP system, no upload endpoint, no signed +write URL, and no new middleware stack — one route declaration, one limiter, one `413`. + +### A route declares how its body reaches it + +`ModuleRouteDefinitionInput` gains one optional field: + +```ts +http.route({ + method: "POST", + path: "/uploads", + permission: "signage.slides.write", + maxBodyBytes: 33_554_432, + bodyMode: "streamed", + handler, +}); +``` + +- **`buffered`** (the default, and what every route written before this meant) — the platform reads + and counts the whole body **before** the handler runs, so the handler is promised a fully + available, bounded body and an oversized request is rejected before any authorization or handler + work happens. Ceiling: 1 MiB, unchanged. +- **`streamed`** — the platform counts bytes **as the handler consumes them** and never materializes + the body. Ceiling: 32 MiB. + +The mode is **explicit rather than inferred from the size**. Inferring it — "over 1 MiB means +streamed" — would mean an author who raised a limit silently changed how their own handler receives +its body, and when a `413` can arrive relative to their own code. That is exactly the class of +invisible change this repository refuses. + +Omitting the field means `buffered`, so no existing route changed behaviour, and the canonical parsed +form always carries a mode so nothing downstream re-applies the default or disagrees about it. + +### The ceilings, and why 32 MiB + +`MODULE_ROUTE_MAX_BODY_BYTES_CEILING` stays at 1 MiB for buffered bodies: a body the platform +materializes is a body the platform is holding, and 1 MiB is the most a JSON request has any business +being. + +`MODULE_ROUTE_STREAMED_MAX_BODY_BYTES_CEILING` is 32 MiB, sized to hold one 25 MiB image plus bounded +overhead — a multipart envelope, its boundaries and part headers, and room for a client that pads. It +is also `MODULE_ROUTE_ABSOLUTE_MAX_BODY_BYTES`: **no route, in any mode, may declare or accept a +larger single request body, and no unbounded option exists anywhere in the contract.** A route +declaring more than its mode's ceiling fails registration, exactly as an over-ceiling value always +has. + +The unit is deliberately **one file per request**. A UI offering twenty images issues twenty +individually bounded requests, each of which the platform can reject cheaply and each of which fails +alone; a 500 MiB aggregate request would make one bad byte waste everything already sent, and nothing +in this contract can express one. + +### Enforcement, in one limiter + +The platform's own limiter replaces the buffering dependency. It is the same code for both modes and +differs only in *when* it counts: + +1. A declared `Content-Length` above the limit is refused synchronously — before authorization, + before the handler, before a single body byte is read. That is the cheap rejection an ordinary + too-large upload takes. A malformed or contradictory header is treated as *absent* rather than as + a rejection, because the byte counting bounds the request either way and refusing could only turn + a legitimate request away. +2. `buffered`: bytes are counted as they arrive and the request is rejected the moment the running + total passes the limit — so a lying or absent `Content-Length` is bounded by what actually + arrived rather than by what was claimed, which the previous implementation did not do. At most + the route's own declared allowance is ever held. +3. `streamed`: the body is wrapped in a counting transform and handed to the handler. The moment the + total passes the limit the stream errors, the source is cancelled, and the response is replaced + with the same sanitized `413` — **even if the handler caught the error and answered anyway**. A + limit a handler can swallow is not a limit. + +A body no handler read is cancelled after the request, so an abandoned upload releases its socket +rather than draining. An aborted upload surfaces as an ordinary failed request, never a `413`: +nothing exceeded a limit. + +Ordering is unchanged and remains deliberate: trusted-origin check, then the body contract, then +permission evaluation, then the handler. In `streamed` mode no body byte is read until the handler +reads it and the handler runs only after authorization succeeds, so an unauthorized caller can never +push an upload through the platform at all — strictly better than the buffered path. + +Timeouts stay where they were: the server's own `headersTimeout`/`requestTimeout`. A slow upload is a +slow *request*, and the platform already has one answer for that rather than a second per-route one. + +### What a module author must know + +In `streamed` mode a `413` can arrive **after** the handler has started. A handler that persists as +it reads must be prepared for a partially consumed upload — write to a temporary key and commit only +after the read completes. The platform cannot do that for the module, because only the module knows +what a partial write means to its own data. + +## Consequences + +- An ordinary image upload works, through the route contract a module already had, with the platform + holding none of it. +- The buffered path now counts bytes it previously trusted a header for, closing a gap rather than + opening one. +- Every existing route and every existing test is unchanged: the default is the old behaviour, and + the `413` contract, ordering, and sanitization are identical. +- `apps/api` owns ~60 lines of stream plumbing it previously delegated. That is the price of a + limiter that neither buffers a large body nor can be swallowed by a handler, and it is enforced by + tests rather than by reading the dependency's source. +- Media larger than 32 MiB — a video — still has no path in. That is deliberate and named below. + +## Alternatives + +- **Raise the single ceiling to 32 MiB.** Rejected: every existing route would silently gain the + ability to have 32 MiB buffered in memory on its behalf, and `await request.json()` in a module + handler would stop being a bounded operation. +- **Infer streaming from `maxBodyBytes`.** Rejected: it makes a size change silently change delivery + semantics and error timing for the author's own code. +- **A separate upload endpoint or second HTTP system.** Rejected: a second route table, a second + authorization path, a second CORS and origin policy, and a second failure vocabulary, to solve + something the existing route contract can express with one field. +- **Signed write URLs to object storage.** Deliberately still deferred (ADR 0031). A signed write URL + is a bearer capability to *create* an object, which needs quota, content-type, and lifecycle + answers this pass does not have — and a module that wants to validate, transform, or authorize per + byte cannot use one anyway. +- **Let the handler enforce its own limit.** Rejected: a limit enforced by the party being protected + against is not a limit, and every module would reimplement it slightly differently. +- **Trust `Content-Length` in streamed mode.** Rejected: the running byte count costs nothing and + removes the need to reason about framing at all. + +## Deferred + +Signed write URLs, resumable or chunked multi-request uploads, per-module or per-installation upload +quotas, content-type or magic-byte validation, virus scanning, image transformation, progress +reporting, and any body larger than the absolute ceiling — a video pipeline is its own design, not a +larger number here. diff --git a/docs/architecture/decisions/README.md b/docs/architecture/decisions/README.md index 625c34a..e87d890 100644 --- a/docs/architecture/decisions/README.md +++ b/docs/architecture/decisions/README.md @@ -50,3 +50,4 @@ ADRs capture durable choices that affect more than one package or phase. - [0039: Shared design tokens and foundational elements](0039-shared-design-tokens-and-elements.md) — Accepted - [0040: Module stylesheets, and every browser artifact gets an isolated root](0040-module-stylesheets-and-artifact-isolation.md) — Accepted - [0041: One module may contribute a browser renderer to another module's capability](0041-module-browser-contributions.md) — Accepted +- [0042: Explicit, bounded streaming request bodies](0042-bounded-streaming-request-bodies.md) — Accepted diff --git a/docs/development/api.md b/docs/development/api.md index 29ade94..76c079d 100644 --- a/docs/development/api.md +++ b/docs/development/api.md @@ -408,16 +408,28 @@ protection across restarts or replicas. ## Request body limits -Every public body-consuming route has an explicit, platform-enforced byte ceiling -(`apps/api/src/body-limits.ts`, built on Hono's pinned `hono/body-limit` middleware — verified -directly against its source to reject both an oversized declared `Content-Length` and an oversized -declared-length-free streamed body without ever buffering the whole payload first): +Every public body-consuming route has an explicit, platform-enforced byte ceiling, enforced by the +platform's own limiter (`apps/api/src/body-limits.ts`): | Surface | Limit | | --- | --- | | `/api/auth/*` (every Better Auth route) | 8 KiB | | `POST /api/v1/grants`, `POST /api/v1/grants/:grantId/revoke` | 16 KiB | -| A module route (`http.route(...)`'s own `maxBodyBytes`) | Up to 1 MiB, the platform ceiling (`MODULE_ROUTE_MAX_BODY_BYTES_CEILING`, `@ekklesia/contracts`) | +| A module route declaring `bodyMode: "buffered"` (the default) | Up to 1 MiB (`MODULE_ROUTE_MAX_BODY_BYTES_CEILING`, `@ekklesia/contracts`) | +| A module route declaring `bodyMode: "streamed"` | Up to 32 MiB (`MODULE_ROUTE_STREAMED_MAX_BODY_BYTES_CEILING`), which is also `MODULE_ROUTE_ABSOLUTE_MAX_BODY_BYTES` | + +A declared `Content-Length` over the limit is refused synchronously, before authorization and before +a single body byte is read. Beyond that the two module modes differ only in *when* the platform +counts ([ADR 0042](../architecture/decisions/0042-bounded-streaming-request-bodies.md)): + +- **`buffered`** materializes the body before the handler runs, counting as it arrives, so a lying or + absent `Content-Length` is bounded by what actually arrived. The handler is promised a fully + available body within its declared allowance, and the platform holds at most that allowance. +- **`streamed`** never materializes the body: it is counted as the handler consumes it, and the + moment the total passes the limit the stream errors, the source is cancelled, and the response + becomes the same `413` — even if the handler caught the error and answered anyway. A `413` can + therefore arrive *after* a streamed handler has started, which is the one consequence a module + author must design for. An oversized request always receives the identical stable, sanitized contract — `413 {"code":"REQUEST_BODY_TOO_LARGE"}` with `Cache-Control: no-store` — and never invokes Better @@ -425,10 +437,20 @@ Auth's business logic, a Core mutation, permission evaluation (where avoidable), or module database behavior. For a state-changing route the body limit is enforced after `requireTrustedOrigin` but before permission evaluation, so a rejected cross-origin mutation's body is never read for size at all, while an oversized same-origin request still fails before any -authorization work runs. See [Module HTTP routes](#module-http-routes) for the module-route body -contract itself, and [Module development](../module-development/README.md#http-routes) for the -module-authoring side of it. Large media transfer is explicitly out of scope for ordinary module -HTTP bodies — that belongs to a future dedicated storage design, never to widening this ceiling. +authorization work runs. In `streamed` mode no body byte is read until the handler reads it, and the +handler runs only after authorization succeeds, so an unauthorized caller can never push an upload +through the platform at all. + +Body reading is bounded in time by the server's own `headersTimeout`/`requestTimeout` below, not by a +second per-route timeout. A body no handler read is cancelled after the request, so an abandoned +upload releases its socket rather than draining; an aborted upload surfaces as an ordinary failed +request rather than a `413`, because nothing exceeded a limit. + +See [Module HTTP routes](#module-http-routes) for the module-route body contract itself, and +[Module development](../module-development/README.md#http-routes) for the module-authoring side of +it. The absolute ceiling is one image-sized file per request: aggregate uploads and anything larger +than 32 MiB remain out of scope, and signed write URLs are still deliberately not offered +([ADR 0031](../architecture/decisions/0031-direct-object-delivery.md)). ## Node request and header timeouts diff --git a/docs/module-development/README.md b/docs/module-development/README.md index a134bd9..cc637cc 100644 --- a/docs/module-development/README.md +++ b/docs/module-development/README.md @@ -1005,8 +1005,8 @@ export default defineModuleRuntime({ }); ``` -A route definition has exactly five fields — `method`, `path`, `permission`, `maxBodyBytes`, -`handler` — and nothing else; an unknown field (a baseline, a mount prefix, CORS/origin policy, +A route definition has exactly five required fields — `method`, `path`, `permission`, +`maxBodyBytes`, `handler` — plus one optional `bodyMode`, and nothing else; an unknown field (a baseline, a mount prefix, CORS/origin policy, middleware, audit metadata, authentication behavior, or another module's namespace) fails registration outright rather than being ignored. `method` is one of `GET`, `POST`, `PUT`, `PATCH`, `DELETE`, always uppercase; there is no `OPTIONS` (the platform owns CORS preflight) and no @@ -1071,15 +1071,61 @@ ones. The platform continues to own authentication, authorization, `Origin`, COR cache behavior, and uncaught-handler sanitization, and permission enforcement always completes before your handler runs — an unauthorized caller can never reach your validation at all. +### Request bodies: `maxBodyBytes` and `bodyMode` + `maxBodyBytes` is a conscious, required request-body contract, not an optional or defaulted field: `0` means the route accepts no body at all, and a non-empty declared (`Content-Length`) or streamed body is rejected before the handler ever runs; any other value is the exact maximum number of bytes -accepted. It must be a non-negative integer no greater than the platform ceiling, -`MODULE_ROUTE_MAX_BODY_BYTES_CEILING` (`@ekklesia/contracts`, currently 1 MiB) — a negative, -non-integer, non-finite, or over-ceiling value fails registration with -`MODULE_ROUTE_MAX_BODY_BYTES_INVALID`. The platform enforces this limit itself, ahead of permission -evaluation and the handler, using the identical sanitized `413 {"code":"REQUEST_BODY_TOO_LARGE"}` -contract every other body-consuming route in the platform uses — see +accepted. A negative, non-integer, non-finite, or over-ceiling value fails registration with +`MODULE_ROUTE_MAX_BODY_BYTES_INVALID`. + +`bodyMode` chooses how that body reaches you, and therefore which ceiling applies +([ADR 0042](../architecture/decisions/0042-bounded-streaming-request-bodies.md)): + +| `bodyMode` | Ceiling | What the platform does | +| --- | --- | --- | +| `"buffered"` (default) | 1 MiB | Reads and counts the whole body **before** your handler runs | +| `"streamed"` | 32 MiB | Counts bytes **as you consume them**, and never materializes the body | + +```ts +http.route({ + method: "POST", + path: "/slides", + permission: "signage.slides.write", + maxBodyBytes: 33_554_432, + bodyMode: "streamed", + async handler(request, context) { /* read request.body as a stream */ }, +}); +``` + +Omitting `bodyMode` means `buffered`, which is what every route written before this existed meant. + +**Memory.** In `buffered` mode the platform holds at most your declared allowance, which is why that +ceiling is small and why `await request.json()` is safe there. In `streamed` mode the platform holds +nothing: read `request.body` as a stream. Calling `request.arrayBuffer()` on a streamed route works +but throws away the entire point — you have just buffered 32 MiB yourself. + +**Streaming.** A declared `Content-Length` over your limit is refused synchronously, before +authorization and before a byte is read. Otherwise the platform counts what actually arrives, so a +misleading or absent `Content-Length` bounds nothing. + +**A `413` can arrive after your handler starts.** This is the one consequence of `streamed` mode you +must design for. When the running total passes your limit the stream errors, the source is cancelled, +and the platform replaces your response with the sanitized `413` — even if you caught the error and +answered anyway. If you persist as you read, write to a temporary key and commit only once the read +completes; the platform cannot do that for you, because only your module knows what a partial write +means to its own data. + +**Timeouts.** Body reading is bounded by the server's own header and request timeouts, not by a +per-route setting. A slow upload is a slow request, and the platform has one answer for that. + +**One file per request.** 32 MiB is the platform's absolute ceiling in every mode — there is no +unbounded option anywhere in the contract. A UI uploading twenty images issues twenty individually +bounded requests; nothing here can express a 500 MiB aggregate upload, deliberately, because one bad +byte in one would waste everything already sent. + +Every rejection uses the identical sanitized `413 {"code":"REQUEST_BODY_TOO_LARGE"}` contract every +other body-consuming route in the platform uses — see [API](../development/api.md#request-body-limits). `path` is local to the module, never the path a caller actually sends. The platform alone owns the diff --git a/packages/contracts/src/module-http.ts b/packages/contracts/src/module-http.ts index 8354be1..c1f9769 100644 --- a/packages/contracts/src/module-http.ts +++ b/packages/contracts/src/module-http.ts @@ -25,21 +25,72 @@ export function isModuleHttpMethod(value: unknown): value is ModuleHttpMethod { } /** - * The platform-owned hard ceiling on any single module route's declared request-body allowance, in - * bytes. No module route may declare a `maxBodyBytes` above this value — validated at registration - * (`safeParseModuleRouteDefinition`, below), not merely documented. This is a security contract, - * not a manifest capability: it does not gate on `manifestVersion` and is enforced identically for - * every installed module regardless of its declared compatibility range. Large media transfer - * belongs to a future dedicated storage design, never to widening this ceiling. + * How a route's request body reaches its handler, and therefore which ceiling applies to it + * (ADR 0042). + * + * - `buffered` (the default, and everything that existed before): the platform reads and counts the + * whole body **before** the handler runs, so the handler is promised a fully available, bounded + * body and an oversized request is rejected before any authorization or handler work happens. + * Bounded by {@link MODULE_ROUTE_MAX_BODY_BYTES_CEILING}, deliberately small. + * - `streamed`: the platform counts bytes **as the handler consumes them** and never materializes + * the body. That is what makes an ordinary image upload possible without the platform holding it + * in memory, and it is the whole reason the larger ceiling exists. + * + * Selecting the mode is an explicit, conscious decision rather than something inferred from the size + * a route asked for. Inferring it would mean an author who raised a limit silently changed how their + * own handler receives its body — and when a `413` can arrive relative to their own code. + */ +export const MODULE_HTTP_BODY_MODES = ["buffered", "streamed"] as const; + +export type ModuleRouteBodyMode = (typeof MODULE_HTTP_BODY_MODES)[number]; + +const moduleRouteBodyModes: ReadonlySet = new Set(MODULE_HTTP_BODY_MODES); + +export function isModuleRouteBodyMode(value: unknown): value is ModuleRouteBodyMode { + return typeof value === "string" && moduleRouteBodyModes.has(value); +} + +/** + * The platform-owned hard ceiling on a **buffered** module route's declared request-body allowance, + * in bytes. No buffered route may declare a `maxBodyBytes` above this value — validated at + * registration (`safeParseModuleRouteDefinition`, below), not merely documented. This is a security + * contract, not a manifest capability: it does not gate on `manifestVersion` and is enforced + * identically for every installed module regardless of its declared compatibility range. + * + * It stays exactly where it was. A body the platform materializes before a handler runs is a body + * the platform is holding in memory, and 1 MiB is the most a JSON request has any business being. */ export const MODULE_ROUTE_MAX_BODY_BYTES_CEILING = 1_048_576; +/** + * The platform-owned hard ceiling on a **streamed** module route's declared allowance, in bytes, + * and the platform's absolute request-body ceiling: nothing in Ekklesia may declare or accept a + * larger single request body, and there is no unbounded option anywhere in the contract. + * + * 32 MiB, chosen to hold one 25 MiB image plus bounded request overhead — a multipart envelope, its + * boundaries and part headers, and a modest allowance for a client that pads. The unit is + * deliberately *one file*: a UI offering twenty images issues twenty individually bounded requests, + * each of which the platform can reject cheaply, rather than one 500 MiB request whose failure + * wastes everything already sent. Nothing in this contract can express an aggregate upload. + */ +export const MODULE_ROUTE_STREAMED_MAX_BODY_BYTES_CEILING = 33_554_432; + +/** The largest body any module route may declare, in any mode. */ +export const MODULE_ROUTE_ABSOLUTE_MAX_BODY_BYTES = MODULE_ROUTE_STREAMED_MAX_BODY_BYTES_CEILING; + +/** The ceiling that applies to one delivery mode. */ +export function moduleRouteBodyCeiling(mode: ModuleRouteBodyMode): number { + return mode === "streamed" + ? MODULE_ROUTE_STREAMED_MAX_BODY_BYTES_CEILING + : MODULE_ROUTE_MAX_BODY_BYTES_CEILING; +} + const moduleRouteMaxBodyBytesSchema = z .number() .int() .finite() .min(0) - .max(MODULE_ROUTE_MAX_BODY_BYTES_CEILING); + .max(MODULE_ROUTE_ABSOLUTE_MAX_BODY_BYTES); const MODULE_ROUTE_PATH_MAX_LENGTH = 256; const staticSegmentPattern = /^[a-z0-9]+(?:-[a-z0-9]+)*$/u; @@ -265,6 +316,12 @@ export interface ModuleRouteDefinitionInput { readonly path: string; readonly permission: string; readonly maxBodyBytes: number; + /** + * How the body reaches the handler, and therefore which ceiling `maxBodyBytes` is checked + * against (ADR 0042). Omitted means `buffered`, which is what every route written before this + * existed meant, so adding this field changed no existing route's behaviour. + */ + readonly bodyMode?: ModuleRouteBodyMode; readonly handler: ModuleRouteHandler; } @@ -275,6 +332,9 @@ export interface ModuleRouteDefinition { readonly path: ModuleRoutePath; readonly permission: PermissionId; readonly maxBodyBytes: number; + /** Always present in the canonical form, defaulted to `buffered`, so no consumer downstream has + * to re-apply the default and none of them can disagree about it. */ + readonly bodyMode: ModuleRouteBodyMode; readonly handler: ModuleRouteHandler; } @@ -294,10 +354,20 @@ export interface RegisteredModuleHttpRoute { readonly path: ModuleRoutePath; readonly permission: PermissionId; readonly maxBodyBytes: number; + readonly bodyMode: ModuleRouteBodyMode; readonly handler: ModuleRouteHandler; } const moduleRouteDefinitionKeys = new Set([ + "method", + "path", + "permission", + "maxBodyBytes", + "bodyMode", + "handler", +]); + +const requiredModuleRouteDefinitionKeys = new Set([ "method", "path", "permission", @@ -314,15 +384,18 @@ function hasModuleRouteDefinitionShape(value: unknown): value is { path: unknown; permission: unknown; maxBodyBytes: unknown; + bodyMode: unknown; handler: unknown; } { if (!isRecord(value)) { return false; } const keys = Object.keys(value); + // Every required field present, `bodyMode` optional, and nothing else at all: a route definition + // still cannot carry a baseline, a mount prefix, middleware, or another module's namespace. return ( - keys.length === moduleRouteDefinitionKeys.size && keys.every((key) => moduleRouteDefinitionKeys.has(key)) && + [...requiredModuleRouteDefinitionKeys].every((key) => keys.includes(key)) && typeof value.handler === "function" ); } @@ -332,6 +405,7 @@ export type ModuleRouteDefinitionIssueCode = | "MODULE_ROUTE_METHOD_INVALID" | "MODULE_ROUTE_PATH_INVALID" | "MODULE_ROUTE_PERMISSION_INVALID" + | "MODULE_ROUTE_BODY_MODE_INVALID" | "MODULE_ROUTE_MAX_BODY_BYTES_INVALID"; export type ModuleRouteDefinitionParseResult = @@ -360,13 +434,23 @@ export function safeParseModuleRouteDefinition(value: unknown): ModuleRouteDefin if (!parsedPermission.success) { return { success: false, code: "MODULE_ROUTE_PERMISSION_INVALID" }; } + if (value.bodyMode !== undefined && !isModuleRouteBodyMode(value.bodyMode)) { + return { success: false, code: "MODULE_ROUTE_BODY_MODE_INVALID" }; + } + const bodyMode: ModuleRouteBodyMode = isModuleRouteBodyMode(value.bodyMode) + ? value.bodyMode + : "buffered"; const parsedMaxBodyBytes = moduleRouteMaxBodyBytesSchema.safeParse(value.maxBodyBytes); - if (!parsedMaxBodyBytes.success) { + // The ceiling is chosen by the declared mode, so a route asking for more than a buffered body may + // hold has to say `streamed` — and thereby accept that its handler consumes a stream and that a + // `413` can arrive after its own code has started running. + if (!parsedMaxBodyBytes.success || parsedMaxBodyBytes.data > moduleRouteBodyCeiling(bodyMode)) { return { success: false, code: "MODULE_ROUTE_MAX_BODY_BYTES_INVALID" }; } return { success: true, data: Object.freeze({ + bodyMode, method: value.method, path: parsedPath.data, permission: parsedPermission.data, diff --git a/packages/contracts/tests/module-http.test.ts b/packages/contracts/tests/module-http.test.ts index 4f2d3c2..533383b 100644 --- a/packages/contracts/tests/module-http.test.ts +++ b/packages/contracts/tests/module-http.test.ts @@ -2,7 +2,10 @@ import { describe, expect, it } from "vitest"; import { isModuleHttpMethod, MODULE_HTTP_METHODS, + MODULE_ROUTE_ABSOLUTE_MAX_BODY_BYTES, MODULE_ROUTE_MAX_BODY_BYTES_CEILING, + MODULE_ROUTE_STREAMED_MAX_BODY_BYTES_CEILING, + moduleRouteBodyCeiling, type ModuleRouteDefinitionInput, moduleRoutePathSchema, moduleRouteShapeKey, @@ -153,6 +156,7 @@ describe("safeParseModuleRouteDefinition", () => { expect(result.success).toBe(true); if (result.success) { expect(result.data).toEqual({ + bodyMode: "buffered", method: "GET", path: "/events/:eventId", permission: "calendar.events.read", @@ -258,6 +262,81 @@ describe("safeParseModuleRouteDefinition", () => { }); }); + it("defaults an omitted bodyMode to buffered, so an existing route means what it always meant", () => { + const parsed = safeParseModuleRouteDefinition(validDefinition()); + + expect(parsed.success && parsed.data.bodyMode).toBe("buffered"); + }); + + it("accepts either declared body mode and carries it through", () => { + for (const bodyMode of ["buffered", "streamed"] as const) { + const parsed = safeParseModuleRouteDefinition({ ...validDefinition(), bodyMode }); + + expect(parsed.success && parsed.data.bodyMode).toBe(bodyMode); + } + }); + + it("rejects a body mode outside the closed set", () => { + for (const bodyMode of ["stream", "chunked", "", 1, null]) { + expect(safeParseModuleRouteDefinition({ ...validDefinition(), bodyMode })).toEqual({ + success: false, + code: "MODULE_ROUTE_BODY_MODE_INVALID", + }); + } + }); + + it("checks maxBodyBytes against the ceiling the declared mode selects", () => { + // A route asking for more than a buffered body may hold must say `streamed` — and thereby accept + // that its handler consumes a stream (ADR 0042). + expect( + safeParseModuleRouteDefinition( + validDefinition({ maxBodyBytes: MODULE_ROUTE_MAX_BODY_BYTES_CEILING + 1 }), + ), + ).toEqual({ success: false, code: "MODULE_ROUTE_MAX_BODY_BYTES_INVALID" }); + expect( + safeParseModuleRouteDefinition({ + ...validDefinition({ maxBodyBytes: MODULE_ROUTE_MAX_BODY_BYTES_CEILING + 1 }), + bodyMode: "streamed", + }).success, + ).toBe(true); + }); + + it("holds one 25 MiB image plus bounded overhead, and refuses one byte more", () => { + const image = 25 * 1024 * 1024; + + expect(MODULE_ROUTE_STREAMED_MAX_BODY_BYTES_CEILING).toBeGreaterThan(image); + expect( + safeParseModuleRouteDefinition({ + ...validDefinition({ maxBodyBytes: MODULE_ROUTE_STREAMED_MAX_BODY_BYTES_CEILING }), + bodyMode: "streamed", + }).success, + ).toBe(true); + expect( + safeParseModuleRouteDefinition({ + ...validDefinition({ maxBodyBytes: MODULE_ROUTE_STREAMED_MAX_BODY_BYTES_CEILING + 1 }), + bodyMode: "streamed", + }), + ).toEqual({ success: false, code: "MODULE_ROUTE_MAX_BODY_BYTES_INVALID" }); + }); + + it("has one absolute ceiling and no unbounded option in any mode", () => { + expect(MODULE_ROUTE_ABSOLUTE_MAX_BODY_BYTES).toBe(MODULE_ROUTE_STREAMED_MAX_BODY_BYTES_CEILING); + expect(moduleRouteBodyCeiling("buffered")).toBe(MODULE_ROUTE_MAX_BODY_BYTES_CEILING); + expect(moduleRouteBodyCeiling("streamed")).toBe(MODULE_ROUTE_ABSOLUTE_MAX_BODY_BYTES); + for (const bodyMode of ["buffered", "streamed"] as const) { + for (const maxBodyBytes of [ + Number.POSITIVE_INFINITY, + Number.MAX_SAFE_INTEGER, + MODULE_ROUTE_ABSOLUTE_MAX_BODY_BYTES + 1, + ]) { + expect( + safeParseModuleRouteDefinition({ ...validDefinition({ maxBodyBytes }), bodyMode }) + .success, + ).toBe(false); + } + } + }); + it("does not retain the caller's mutable definition object: mutating it afterward cannot change the parsed result", () => { const source = { ...validDefinition() }; const result = safeParseModuleRouteDefinition(source);