fix(relay): authorize reserve routes before reading request bodies (#36346)

* fix(relay): bound internal reserve request bodies

* fix(relay): authorize before reading reserve bodies

Hono reads a chunked body inside the body limit before next(). A caller who fails the director check can no longer stall the refusal, and an oversized body from that caller is not reported as 413.

* test(relay): cover an unavailable demote body

The missing-route check only posted to /internal/reserve. Demote now gets the same oversized request and must stay 404.

---------

Co-authored-by: Neil <neil@stably.ai>

cloud/apps/relay/src/assignment/public-admission/public-assignment-overload.test.ts

@@ -135,12 +135,14 @@ describe('public assignment overload', () => {
     expect(refused.status).toBe(413)
     expect(await refused.json()).toEqual({ error: 'request_too_large' })
     expect(resolve).not.toHaveBeenCalled()
-    // Reserve bodies may reach 16 KiB, so the 4 KiB app limit must not answer for them: this one
-    // reaches the route itself, which a director answers as unavailable.
+    // Reserve bodies may reach 16 KiB, so the 4 KiB app limit must not answer for them. An
+    // unavailable route refuses before its own limit, including a body past 16 KiB.
     const reserve = await app.request('/internal/reserve', chunked(6 * 1024))
     expect(reserve.status).toBe(404)
     expect(await reserve.json()).toEqual({ error: 'reserve_unavailable' })
-    expect((await app.request('/internal/reserve', chunked(20 * 1024))).status).toBe(413)
+    const oversized = await app.request('/internal/reserve', chunked(20 * 1024))
+    expect(oversized.status).toBe(404)
+    expect(await oversized.json()).toEqual({ error: 'reserve_unavailable' })
   })
 
   it('reserves one bounded resolve lane while assignment work is saturated', async () => {

cloud/apps/relay/src/cell/reservations/cell-reserve-routes.test.ts

@@ -25,7 +25,19 @@ function app(config: RelayConfig = cell) {
       headers: { authorization: `Bearer ${token}`, 'content-type': 'application/json' },
       body: JSON.stringify(body)
     })
-  return { post, reserve, demote }
+  const postChunked = (path: string, body: string) =>
+    routes.request(path, {
+      method: 'POST',
+      headers: { authorization: 'Bearer director-token', 'content-type': 'application/json' },
+      body: new ReadableStream<Uint8Array>({
+        start(controller) {
+          controller.enqueue(new TextEncoder().encode(body))
+          controller.close()
+        }
+      }),
+      duplex: 'half'
+    } as RequestInit)
+  return { post, postChunked, reserve, demote, routes }
 }
 
 const booking = {
@@ -59,4 +71,82 @@ describe('cell reserve routes', () => {
     expect(await reply.json()).toEqual({ v: 1, outcome: 'demoted' })
     expect(demote).toHaveBeenCalledWith({ ...seat, joinedAt: 1_000 })
   })
+
+  it.each(['/internal/reserve', '/internal/demote'])(
+    'rejects an oversized chunked body on %s without calling the handler',
+    async (path) => {
+      const { postChunked, reserve, demote } = app()
+      const response = await postChunked(path, ' '.repeat(16 * 1024 + 1))
+
+      expect(response.status).toBe(413)
+      expect(await response.json()).toEqual({ error: 'request_too_large' })
+      expect(reserve).not.toHaveBeenCalled()
+      expect(demote).not.toHaveBeenCalled()
+    }
+  )
+
+  it.each(['/internal/reserve', '/internal/demote'])(
+    'refuses %s without waiting for a chunked body',
+    async (path) => {
+      const { routes, reserve, demote } = app()
+      const response = await Promise.race([
+        routes.request(path, {
+          method: 'POST',
+          headers: { authorization: 'Bearer someone-else', 'content-type': 'application/json' },
+          body: new ReadableStream<Uint8Array>({
+            pull: () => new Promise(() => {})
+          }),
+          duplex: 'half'
+        } as RequestInit),
+        new Promise<Response>((_, reject) => {
+          setTimeout(() => reject(new Error('waited for the body')), 200)
+        })
+      ])
+
+      expect(response.status).toBe(401)
+      expect(await response.json()).toEqual({ error: 'invalid_token' })
+      expect(reserve).not.toHaveBeenCalled()
+      expect(demote).not.toHaveBeenCalled()
+    }
+  )
+
+  it.each(['/internal/reserve', '/internal/demote'])(
+    'refuses an unauthorized oversized %s before the size check',
+    async (path) => {
+      const { routes, reserve, demote } = app()
+      const response = await routes.request(path, {
+        method: 'POST',
+        headers: {
+          authorization: 'Bearer someone-else',
+          'content-type': 'application/json',
+          'content-length': String(16 * 1024 + 1)
+        },
+        body: '{}'
+      })
+
+      expect(response.status).toBe(401)
+      expect(await response.json()).toEqual({ error: 'invalid_token' })
+      expect(reserve).not.toHaveBeenCalled()
+      expect(demote).not.toHaveBeenCalled()
+    }
+  )
+
+  it.each(['/internal/reserve', '/internal/demote'])(
+    'hides a missing %s route before measuring an oversized body',
+    async (path) => {
+      const { routes } = app({ ...cell, role: 'director' })
+      const response = await routes.request(path, {
+        method: 'POST',
+        headers: {
+          authorization: 'Bearer director-token',
+          'content-type': 'application/json',
+          'content-length': String(16 * 1024 + 1)
+        },
+        body: '{}'
+      })
+
+      expect(response.status).toBe(404)
+      expect(await response.json()).toEqual({ error: 'reserve_unavailable' })
+    }
+  )
 })

cloud/apps/relay/src/cell/reservations/cell-reserve-routes.ts

@@ -1,4 +1,4 @@
-import type { Hono } from 'hono'
+import type { Hono, MiddlewareHandler } from 'hono'
 import { bodyLimit } from 'hono/body-limit'
 import {
   CELL_DEMOTE_PATH,
@@ -18,10 +18,6 @@ export const CELL_RESERVE_ROUTE_PATHS: ReadonlySet<string> = new Set([
   CELL_RESERVE_PATH,
   CELL_DEMOTE_PATH
 ])
-const limitBody = bodyLimit({
-  maxSize: MAX_BODY_BYTES,
-  onError: (context) => context.json({ error: 'request_too_large' }, 413)
-})
 
 // Reuse director rehome credentials for writes; directors can already place any host via DB, so these grant no new authority.
 export function registerCellReserveRoutes(
@@ -41,24 +37,24 @@ export function registerCellReserveRoutes(
   }
   const unauthorized = (status: 404 | 401) =>
     status === 404 ? { error: 'reserve_unavailable' } : { error: 'invalid_token' }
-
-  app.post(CELL_RESERVE_PATH, limitBody, async (context) => {
+  const limitBody = bodyLimit({
+    maxSize: MAX_BODY_BYTES,
+    onError: (context) => context.json({ error: 'request_too_large' }, 413)
+  })
+  // Hono reads a chunked body inside bodyLimit before next(). Check the caller first.
+  const requireDirector: MiddlewareHandler = async (context, next) => {
     const auth = await authorize(context.req.header('authorization'))
     if (auth !== 'ok') return context.json(unauthorized(auth), auth)
-    if (Number(context.req.header('content-length') ?? 0) > MAX_BODY_BYTES) {
-      return context.json({ error: 'request_too_large' }, 413)
-    }
+    await next()
+  }
+
+  app.post(CELL_RESERVE_PATH, requireDirector, limitBody, async (context) => {
     const body = ReserveRequestSchema.safeParse(await context.req.json().catch(() => null))
     if (!body.success) return context.json({ error: 'invalid_request' }, 400)
     return context.json({ v: 1, results: input.reserve!(body.data) })
   })
 
-  app.post(CELL_DEMOTE_PATH, limitBody, async (context) => {
-    const auth = await authorize(context.req.header('authorization'))
-    if (auth !== 'ok') return context.json(unauthorized(auth), auth)
-    if (Number(context.req.header('content-length') ?? 0) > MAX_BODY_BYTES) {
-      return context.json({ error: 'request_too_large' }, 413)
-    }
+  app.post(CELL_DEMOTE_PATH, requireDirector, limitBody, async (context) => {
     const body = DemoteRequestSchema.safeParse(await context.req.json().catch(() => null))
     if (!body.success) return context.json({ error: 'invalid_request' }, 400)
     return context.json({ v: 1, outcome: input.demote!(body.data) })