fix(session): cap daemon HTTP, websocket, and registration payload sizes

[benvinegar]

Summary

Closes item #4 of the session-daemon security review: no body / WebSocket message size limits. The broker buffered HTTP bodies, websocket frames, and session registration payloads with no ceiling, so a single oversized request could drive the daemon to unbounded memory / OOM — a local DoS, or a remote DoS when HUNK_MCP_UNSAFE_ALLOW_REMOTE=1.

This builds on the Host/Origin/content-type hardening from #329.

Changes

New shared packages/session-broker-core/src/limits.ts with hard caps, enforced as early as each transport allows:

• HTTP body — readRequestTextWithLimit rejects an oversized Content-Length before reading, and stream-aborts a missing/lying Content-Length mid-read so the body is never buffered unbounded. Over-limit → 413. Applied to both the generic daemon and the Hunk session API. Portable across the Bun and Node serve layers.
• WebSocket — set Bun's native maxPayloadLength (protocol-layer rejection) plus a decoded-string guard as defense-in-depth.
• Registration / snapshot — wire parsers reject payloads with too many files, hunks-per-file, live comments, or review notes, or an oversized per-file patch; rejection closes the socket as an incompatible payload.

Caps (named constants, easy to tune): HTTP body 4 MiB, WS message 8 MiB, 5k files, 10k hunks/file, 2 MiB/patch, 10k comments, 10k notes — all far above any realistic review.

Testing

• bun run typecheck, bun run lint — clean
• New limits.test.ts: Content-Length early-reject, streaming abort with no Content-Length, under-limit pass-through, empty body, utf8ByteLength
• daemon.test.ts / brokerServer.test.ts: 413 on oversized body for both API surfaces
• wire.test.ts: file / hunk / patch / live-comment / review-note caps reject the payload

🤖 Generated with Claude Code

[greptile-apps]

Greptile Summary

This PR adds hard size ceilings to every inbound data path the session broker processes: HTTP request bodies (4 MiB), WebSocket frames (8 MiB), and registration/snapshot payloads (files, hunks, patch text, comments, notes). The limits are centralized in a new limits.ts module and enforced as early as each transport allows, with tests covering all enforcement points.

• limits.ts introduces readRequestTextWithLimit, which rejects on Content-Length before reading and stream-aborts mid-read to prevent buffering beyond the cap; both daemon.ts and brokerServer.ts map PayloadTooLargeError to HTTP 413.
• serve.ts sets Bun's native maxPayloadLength for protocol-layer rejection and adds a decoded-string guard as defense-in-depth; wire.ts adds per-field guards so oversized registrations are rejected during parsing rather than stored.

Confidence Score: 4/5

Safe to merge; the enforcement logic is correct across all three transport paths and is well-tested.

Two small cleanup items in limits.ts: reader.releaseLock() is not called after reader.cancel() on the over-limit path, and utf8ByteLength allocates a fresh TextEncoder per call rather than reusing a module-level instance.

packages/session-broker-core/src/limits.ts — the streaming-read over-limit path and the utf8ByteLength helper.

Important Files Changed

Filename	Overview
packages/session-broker-core/src/limits.ts	New core limits module with size constants, PayloadTooLargeError, utf8ByteLength, and readRequestTextWithLimit. Content-Length early-reject and streaming abort are well-implemented; one minor cleanup issue: releaseLock() is not called on the cancel+throw path.
packages/session-broker-bun/src/serve.ts	Adds Bun's native maxPayloadLength at the protocol layer plus a decoded-string guard in the message handler. Defense-in-depth layering is correct; WS close code 1009 (Message Too Large) is semantically right.
packages/session-broker/src/daemon.ts	parseJsonRequest now goes through readRequestTextWithLimit before JSON.parse; PayloadTooLargeError is caught and mapped to HTTP 413 cleanly.
src/session-broker/brokerServer.ts	Same HTTP body limit pattern as daemon.ts applied to the Hunk session API path; 413 handling is consistent.
src/hunk-session/wire.ts	Files, hunks-per-file, patch bytes, live comments, and review notes all guarded with named constants; reviewNotes cap is correctly applied only when the field is an array.
packages/session-broker-core/src/limits.test.ts	Good coverage: Content-Length early-reject, streaming abort, under-limit pass-through, empty body, and utf8ByteLength multi-byte character counts all tested.
src/hunk-session/wire.test.ts	Wire cap tests cover all five new limits (files, hunks/file, patch bytes, live comments, review notes).

Prompt To Fix All With AI

Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
packages/session-broker-core/src/limits.ts:98-103
The reader lock is not released before throwing on the over-limit path. `reader.cancel()` cancels the underlying source but does not release the lock per the WHATWG Streams spec — only `releaseLock()` does that. In practice the lock is freed when the reader is GC'd (since the request object goes out of scope after the 413 response), but the lock should be released explicitly to match the normal-exit path and avoid holding it for the GC cycle.

```suggestion
    total += value.byteLength;
    if (total > maxBytes) {
      // Stop pulling from the stream immediately so the body cannot grow past the cap.
      await reader.cancel().catch(() => {});
      reader.releaseLock();
      throw new PayloadTooLargeError(maxBytes);
    }
```

### Issue 2 of 2
packages/session-broker-core/src/limits.ts:39-42
`utf8ByteLength` allocates a fresh `TextEncoder` on every invocation. It is called per WebSocket message, per HTTP body, and per file patch. Hoisting a single module-level encoder avoids the per-call allocation entirely.

```suggestion
const _encoder = new TextEncoder();

/** UTF-8 byte length of a string without allocating a Buffer in non-Node runtimes. */
export function utf8ByteLength(value: string): number {
  return _encoder.encode(value).length;
}
```

_{Reviews (1): Last reviewed commit: "fix(session): cap daemon HTTP, websocket..." | Re-trigger Greptile}

[greptile-apps]

The reader lock is not released before throwing on the over-limit path. reader.cancel() cancels the underlying source but does not release the lock per the WHATWG Streams spec — only releaseLock() does that. In practice the lock is freed when the reader is GC'd (since the request object goes out of scope after the 413 response), but the lock should be released explicitly to match the normal-exit path and avoid holding it for the GC cycle.

Suggested change

	total += value.byteLength;
	if (total > maxBytes) {
	// Stop pulling from the stream immediately so the body cannot grow past the cap.
	await reader.cancel().catch(() => {});
	throw new PayloadTooLargeError(maxBytes);
	}
	total += value.byteLength;
	if (total > maxBytes) {
	// Stop pulling from the stream immediately so the body cannot grow past the cap.
	await reader.cancel().catch(() => {});
	reader.releaseLock();
	throw new PayloadTooLargeError(maxBytes);
	}

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/session-broker-core/src/limits.ts
Line: 98-103

Comment:
The reader lock is not released before throwing on the over-limit path. `reader.cancel()` cancels the underlying source but does not release the lock per the WHATWG Streams spec — only `releaseLock()` does that. In practice the lock is freed when the reader is GC'd (since the request object goes out of scope after the 413 response), but the lock should be released explicitly to match the normal-exit path and avoid holding it for the GC cycle.

```suggestion
    total += value.byteLength;
    if (total > maxBytes) {
      // Stop pulling from the stream immediately so the body cannot grow past the cap.
      await reader.cancel().catch(() => {});
      reader.releaseLock();
      throw new PayloadTooLargeError(maxBytes);
    }
```

How can I resolve this? If you propose a fix, please make it concise.

[benvinegar]

Done — added an explicit reader.releaseLock() after cancel() on the over-limit path so it matches the normal-exit path instead of waiting for GC.

_{Responded by Claude Code using claude-opus-4-7.}

[greptile-apps]

utf8ByteLength allocates a fresh TextEncoder on every invocation. It is called per WebSocket message, per HTTP body, and per file patch. Hoisting a single module-level encoder avoids the per-call allocation entirely.

Suggested change

	/** UTF-8 byte length of a string without allocating a Buffer in non-Node runtimes. */
	export function utf8ByteLength(value: string): number {
	return new TextEncoder().encode(value).length;
	}
	const _encoder = new TextEncoder();
	/** UTF-8 byte length of a string without allocating a Buffer in non-Node runtimes. */
	export function utf8ByteLength(value: string): number {
	return _encoder.encode(value).length;
	}

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/session-broker-core/src/limits.ts
Line: 39-42

Comment:
`utf8ByteLength` allocates a fresh `TextEncoder` on every invocation. It is called per WebSocket message, per HTTP body, and per file patch. Hoisting a single module-level encoder avoids the per-call allocation entirely.

```suggestion
const _encoder = new TextEncoder();

/** UTF-8 byte length of a string without allocating a Buffer in non-Node runtimes. */
export function utf8ByteLength(value: string): number {
  return _encoder.encode(value).length;
}
```

How can I resolve this? If you propose a fix, please make it concise.

[benvinegar]

Done — hoisted a single module-level TextEncoder (sharedTextEncoder) and reuse it in utf8ByteLength.

_{Responded by Claude Code using claude-opus-4-7.}