Skip to content

Commit d8b707e

Browse files
committed
fix: fold the wp6 re-audit round (house bounded-body primitive, exact UTF-8 cap)
Re-audit findings on 9794e24: - readBoundedResponseText awaited reader.cancel(), so a broken stream whose cancellation never settles would hang the overflow path instead of returning the documented 502. The custom helper is replaced by the house primitive readBoundedResponseBody, which cancels fire-and-forget with synchronous-throw protection and adds total (180s) and inactivity (30s) transfer deadlines on top of the byte ceiling; oversize and stalls both fail closed, and a partial body is never parsed. - bounded-body.ts gains a maxBytes option (default unchanged at 64 KiB) and accumulates into a geometrically growing single buffer, so per-chunk metadata cannot amplify beyond the payload budget on large ceilings. - the repair byte cap measured UTF-16 code units; astral text could enter the parse attempts at 4x the intended bytes. utf8BytesExceed measures the exact UTF-8 length with early exit and no allocation; regression test covers a 600k-code-unit, 1.2 MB input that a length check would have admitted.
1 parent 9794e24 commit d8b707e

5 files changed

Lines changed: 75 additions & 52 deletions

File tree

src/adapters/anthropic.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,26 @@ function usableToolUseId(id: unknown): string {
288288
*/
289289
const MAX_REPAIRABLE_TOOL_ARGUMENT_BYTES = 1024 * 1024;
290290

291+
/**
292+
* Whether `input` encodes to more than `max` UTF-8 bytes, with an early exit so the check
293+
* itself never allocates a copy of a hostile string. `string.length` counts UTF-16 code
294+
* units, which undercounts astral text by 2x against a byte budget.
295+
*/
296+
function utf8BytesExceed(input: string, max: number): boolean {
297+
let bytes = 0;
298+
for (let i = 0; i < input.length; i++) {
299+
const code = input.charCodeAt(i);
300+
if (code < 0x80) bytes += 1;
301+
else if (code < 0x800) bytes += 2;
302+
else if (code >= 0xd800 && code <= 0xdbff && i + 1 < input.length) {
303+
bytes += 4;
304+
i++;
305+
} else bytes += 3; // lone surrogates encode as U+FFFD (3 bytes)
306+
if (bytes > max) return true;
307+
}
308+
return false;
309+
}
310+
291311
function lastValidJsonObject(input: string, maxCandidates: number): string | undefined {
292312
const tryParseObject = (candidate: string): string | undefined => {
293313
try {
@@ -323,7 +343,7 @@ function toolUseArguments(input: unknown, lenient = false): string {
323343
JSON.parse(trimmed);
324344
return trimmed;
325345
} catch {
326-
if (lenient && trimmed.length <= MAX_REPAIRABLE_TOOL_ARGUMENT_BYTES) {
346+
if (lenient && !utf8BytesExceed(trimmed, MAX_REPAIRABLE_TOOL_ARGUMENT_BYTES)) {
327347
const repaired = lastValidJsonObject(trimmed, 32);
328348
if (repaired !== undefined) return repaired;
329349
}

src/lib/bounded-body.ts

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,12 @@ export const BOUNDED_BODY_TIMEOUT_MS = 5_000;
77
export interface BoundedBodyOptions {
88
/** Abort the read with this signal. Its reason is rethrown by identity. */
99
signal?: AbortSignal;
10+
/**
11+
* Byte ceiling for retained body data. Defaults to BOUNDED_BODY_MAX_BYTES (64 KiB),
12+
* which suits error bodies; callers materializing whole success payloads (e.g. a
13+
* non-streaming upstream JSON completion) pass a larger explicit budget.
14+
*/
15+
maxBytes?: number;
1016
/** Total wall-clock deadline. Exposed for focused tests. */
1117
totalTimeoutMs?: number;
1218
/** Deadline between non-empty raw chunks. Exposed for focused tests. */
@@ -93,7 +99,11 @@ export async function readBoundedResponseBody(
9399
}
94100

95101
const reader = body.getReader();
96-
const chunks: Uint8Array[] = [];
102+
const maxBytes = options.maxBytes ?? BOUNDED_BODY_MAX_BYTES;
103+
// Geometrically growing single buffer: per-chunk arrays would retain one object per
104+
// transport chunk, which a hostile peer could inflate into metadata amplification far
105+
// beyond the payload ceiling on large budgets.
106+
let retained = new Uint8Array(Math.min(maxBytes, 64 * 1024));
97107
let retainedBytes = 0;
98108
let mustCancel = false;
99109
let cancelReason: unknown;
@@ -134,7 +144,7 @@ export async function readBoundedResponseBody(
134144
"TimeoutError",
135145
);
136146
return {
137-
text: decodeUtf8(chunks),
147+
text: decodeUtf8([retained.subarray(0, retainedBytes)]),
138148
truncated: true,
139149
timedOut: true,
140150
totalTimedOut: outcome === TOTAL_TIMEOUT,
@@ -147,7 +157,7 @@ export async function readBoundedResponseBody(
147157
const { value, done } = outcome as ReadableStreamReadResult<Uint8Array>;
148158
if (done) {
149159
return {
150-
text: decodeUtf8(chunks),
160+
text: decodeUtf8([retained.subarray(0, retainedBytes)]),
151161
truncated: false,
152162
timedOut: false,
153163
totalTimedOut: false,
@@ -165,10 +175,10 @@ export async function readBoundedResponseBody(
165175
INACTIVITY_TIMEOUT,
166176
);
167177

168-
if (value.byteLength > BOUNDED_BODY_MAX_BYTES - retainedBytes) {
178+
if (value.byteLength > maxBytes - retainedBytes) {
169179
mustCancel = true;
170180
cancelReason = new DOMException("Error body size limit reached", "QuotaExceededError");
171-
chunks.length = 0;
181+
retained = new Uint8Array(0);
172182
retainedBytes = 0;
173183
return {
174184
text: "",
@@ -181,7 +191,14 @@ export async function readBoundedResponseBody(
181191
};
182192
}
183193

184-
chunks.push(value);
194+
if (retainedBytes + value.byteLength > retained.length) {
195+
const grown = new Uint8Array(
196+
Math.min(maxBytes, Math.max(retained.length * 2, retainedBytes + value.byteLength)),
197+
);
198+
grown.set(retained.subarray(0, retainedBytes));
199+
retained = grown;
200+
}
201+
retained.set(value, retainedBytes);
185202
retainedBytes += value.byteLength;
186203
}
187204
} catch (error) {

src/server/relay.ts

Lines changed: 0 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -20,48 +20,6 @@ export const MAX_INSPECTION_SSE_FRAME_BYTES = 4 * 1024 * 1024;
2020
export const MAX_COMPLETED_OUTPUT_ITEMS = 256;
2121
export const MAX_COMPLETED_OUTPUT_ITEM_SOURCE_BYTES = 8 * 1024 * 1024;
2222
export const MAX_TAIL_ERROR_MESSAGE_CHARS = 512;
23-
// Whole-body ceiling for a non-streaming upstream JSON response. The caller materializes
24-
// the body for logging and (on the WebSocket bridge) reframing, so an unbounded `.text()`
25-
// read would let a hostile or broken upstream grow proxy memory without limit. 32 MiB
26-
// matches the continuation snapshot read bound and is far above any legitimate
27-
// non-streaming completion (including base64 image payloads).
28-
export const MAX_UPSTREAM_JSON_BODY_BYTES = 32 * 1024 * 1024;
29-
30-
/**
31-
* Read an entire upstream body as text with a hard byte ceiling. Returns `truncated: true`
32-
* (with the body already cancelled) when the body exceeds `maxBytes`; callers must treat
33-
* truncation as an upstream failure, never parse the partial text. A null body reads as "".
34-
*/
35-
export async function readBoundedResponseText(
36-
body: ReadableStream<Uint8Array> | null,
37-
maxBytes: number = MAX_UPSTREAM_JSON_BODY_BYTES,
38-
): Promise<{ text: string; truncated: boolean }> {
39-
if (!body) return { text: "", truncated: false };
40-
const reader = body.getReader();
41-
const chunks: Uint8Array[] = [];
42-
let total = 0;
43-
try {
44-
for (;;) {
45-
const { done, value } = await reader.read();
46-
if (done) break;
47-
total += value.byteLength;
48-
if (total > maxBytes) {
49-
await reader.cancel("upstream body exceeded the safe byte limit").catch(() => {});
50-
return { text: "", truncated: true };
51-
}
52-
chunks.push(value);
53-
}
54-
} finally {
55-
reader.releaseLock();
56-
}
57-
const merged = new Uint8Array(total);
58-
let offset = 0;
59-
for (const chunk of chunks) {
60-
merged.set(chunk, offset);
61-
offset += chunk.byteLength;
62-
}
63-
return { text: new TextDecoder().decode(merged), truncated: false };
64-
}
6523

6624
export type InspectionCounters = {
6725
frameBufferHighWaterBytes: number;

src/server/responses/core.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,6 @@ import {
139139
isNativePassthroughSseResponse,
140140
markEagerRelaySseResponse,
141141
markNativePassthroughSseResponse,
142-
readBoundedResponseText,
143142
relaySseWithFailedTail,
144143
relayWithAbort,
145144
sanitizePassthroughHeaders,
@@ -688,6 +687,15 @@ export function buildComboChildHeaders(parentHeaders: HeadersInit): Headers {
688687
const UNREADABLE_ENCRYPTED_AGENT_TASK_MESSAGE =
689688
"Routed V2 worker task is encrypted for the native ChatGPT backend and cannot be read by the selected provider. Use plaintext V2 agent-message delivery or select a native ChatGPT model.";
690689

690+
// Whole-body policy for non-streaming upstream JSON responses (see the application/json
691+
// branch of the passthrough return path). 32 MiB matches the continuation snapshot read
692+
// bound and is far above any legitimate non-streaming completion, including base64 image
693+
// payloads. The stall deadlines only govern the body transfer — generation time before
694+
// the response headers is untouched.
695+
const MAX_UPSTREAM_JSON_BODY_BYTES = 32 * 1024 * 1024;
696+
const UPSTREAM_JSON_BODY_TOTAL_TIMEOUT_MS = 180_000;
697+
const UPSTREAM_JSON_BODY_INACTIVITY_TIMEOUT_MS = 30_000;
698+
691699
function unreadableEncryptedAgentTaskResponse(): Response {
692700
return new Response(
693701
JSON.stringify({
@@ -1969,10 +1977,18 @@ async function handleResponsesInner(
19691977
// so an unbounded .text() would let a hostile or stuck upstream grow proxy memory
19701978
// without limit. This path is no longer rare — WebSocket turns for models whose
19711979
// streaming terminal event is unreliable are deliberately answered with bounded JSON.
1972-
const bounded = await readBoundedResponseText(upstreamResponse.body);
1973-
if (bounded.truncated) {
1980+
// Oversize and stall deadlines both fail closed; a partial body is never parsed.
1981+
const bounded = await readBoundedResponseBody(upstreamResponse, {
1982+
maxBytes: MAX_UPSTREAM_JSON_BODY_BYTES,
1983+
totalTimeoutMs: UPSTREAM_JSON_BODY_TOTAL_TIMEOUT_MS,
1984+
inactivityTimeoutMs: UPSTREAM_JSON_BODY_INACTIVITY_TIMEOUT_MS,
1985+
});
1986+
if (bounded.oversized) {
19741987
return formatErrorResponse(502, "upstream_error", "upstream JSON response exceeded the safe body limit");
19751988
}
1989+
if (bounded.truncated) {
1990+
return formatErrorResponse(502, "upstream_error", "upstream JSON response stalled before completing");
1991+
}
19761992
const text = bounded.text;
19771993
inspectResponseLogJson(logCtx, text);
19781994
if (rememberPassthroughResponse) {

tests/anthropic-eof-tolerance.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,4 +178,16 @@ describe("AgentRouter Anthropic EOF tolerance (#658)", () => {
178178
const events = await createAnthropicAdapter(tolerant).parseResponse(new Response(payload));
179179
expect(events).toContainEqual({ type: "tool_call_delta", arguments: "{}" });
180180
});
181+
182+
test("the repair cap measures UTF-8 bytes, not UTF-16 code units", async () => {
183+
// 600k 2-byte characters: 1.2 MB on the wire but only 600k code units, which a
184+
// `string.length` check would wrongly admit into the parse attempts.
185+
const oversized = `{${"é".repeat(600_000)}`;
186+
const payload = JSON.stringify({
187+
content: [{ type: "tool_use", id: "toolu_1", name: "get_weather", input: oversized }],
188+
});
189+
190+
const events = await createAnthropicAdapter(tolerant).parseResponse(new Response(payload));
191+
expect(events).toContainEqual({ type: "tool_call_delta", arguments: "{}" });
192+
});
181193
});

0 commit comments

Comments
 (0)