Skip to content

Commit 9794e24

Browse files
committed
fix: fold the wp6 reviewer round (bounded upstream JSON body, backward-scan repair, strict EOF predicates)
Review findings on the stacked contributor fixes, all red-green verified: - core.ts upstream JSON branch read the whole body with an unbounded .text(); every non-streaming upstream — now including WebSocket turns deliberately answered with bounded JSON — could grow proxy memory without limit. The read goes through relay.readBoundedResponseText (32 MiB ceiling, body cancelled on overflow) and fails closed with a 502 instead of parsing a partial body. - anthropic lastValidJsonObject collected every brace offset into two arrays before its candidate cap, so brace-dense hostile input cost O(n) index storage. It now scans backwards from the end with lastIndexOf and never materializes an index; inputs above 1 MiB are not repaired at all. - an empty text_delta marked sawVisibleText, letting a cut-off tolerant stream complete as a successful empty answer; only non-empty text authorizes tolerant completion now. - a translator-budget overflow could be followed by tool_call_end/done when the generator was fully drained; the budget error now returns immediately as the single terminal event.
1 parent 9469422 commit 9794e24

5 files changed

Lines changed: 171 additions & 21 deletions

File tree

src/adapters/anthropic.ts

Lines changed: 32 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -281,30 +281,36 @@ function usableToolUseId(id: unknown): string {
281281
* Bound repair for a malformed tool-arguments string under the compatibility profile (#658):
282282
* a gateway such as AgentRouter can concatenate JSON objects (`{}{"value":42}`). Find the
283283
* last parseable JSON object by scanning suffixes from each object-open brace and prefixes
284-
* ending at each object-close brace, bounded so hostile input cannot cost unbounded time.
284+
* ending at each object-close brace. Both scans walk backwards from the end trying at most
285+
* `maxCandidates` positions, so no offset index is ever materialized: a brace-dense hostile
286+
* input costs at most 2 × maxCandidates bounded JSON.parse attempts and no extra storage.
287+
* Inputs above MAX_REPAIRABLE_TOOL_ARGUMENT_BYTES are not repaired at all.
285288
*/
289+
const MAX_REPAIRABLE_TOOL_ARGUMENT_BYTES = 1024 * 1024;
290+
286291
function lastValidJsonObject(input: string, maxCandidates: number): string | undefined {
287-
const opens: number[] = [];
288-
const closes: number[] = [];
289-
for (let i = 0; i < input.length; i++) {
290-
if (input[i] === "{") opens.push(i);
291-
else if (input[i] === "}") closes.push(i);
292-
}
293-
let tried = 0;
294-
for (let i = opens.length - 1; i >= 0 && tried < maxCandidates; i--, tried++) {
295-
const candidate = input.slice(opens[i]);
292+
const tryParseObject = (candidate: string): string | undefined => {
296293
try {
297294
const parsed = JSON.parse(candidate) as unknown;
298295
if (parsed && typeof parsed === "object" && !Array.isArray(parsed)) return candidate;
299296
} catch { /* keep scanning */ }
297+
return undefined;
298+
};
299+
let scanFrom = input.length - 1;
300+
for (let tried = 0; tried < maxCandidates && scanFrom >= 0; tried++) {
301+
const open = input.lastIndexOf("{", scanFrom);
302+
if (open === -1) break;
303+
const repaired = tryParseObject(input.slice(open));
304+
if (repaired !== undefined) return repaired;
305+
scanFrom = open - 1;
300306
}
301-
tried = 0;
302-
for (let i = closes.length - 1; i >= 0 && tried < maxCandidates; i--, tried++) {
303-
const candidate = input.slice(0, closes[i] + 1);
304-
try {
305-
const parsed = JSON.parse(candidate) as unknown;
306-
if (parsed && typeof parsed === "object" && !Array.isArray(parsed)) return candidate;
307-
} catch { /* keep scanning */ }
307+
scanFrom = input.length - 1;
308+
for (let tried = 0; tried < maxCandidates && scanFrom >= 0; tried++) {
309+
const close = input.lastIndexOf("}", scanFrom);
310+
if (close === -1) break;
311+
const repaired = tryParseObject(input.slice(0, close + 1));
312+
if (repaired !== undefined) return repaired;
313+
scanFrom = close - 1;
308314
}
309315
return undefined;
310316
}
@@ -317,7 +323,7 @@ function toolUseArguments(input: unknown, lenient = false): string {
317323
JSON.parse(trimmed);
318324
return trimmed;
319325
} catch {
320-
if (lenient) {
326+
if (lenient && trimmed.length <= MAX_REPAIRABLE_TOOL_ARGUMENT_BYTES) {
321327
const repaired = lastValidJsonObject(trimmed, 32);
322328
if (repaired !== undefined) return repaired;
323329
}
@@ -890,7 +896,10 @@ export function createAnthropicAdapter(provider: OcxProviderConfig, cacheRetenti
890896
const delta = data.delta as Record<string, unknown> | undefined;
891897
if (!delta) break;
892898
if (delta.type === "text_delta" && typeof delta.text === "string") {
893-
sawVisibleText = true;
899+
// Only non-empty text proves the upstream produced usable output; an empty
900+
// delta followed by EOF must stay a truncation error even on the tolerant
901+
// profile, or a cut-off turn would surface as a successful empty answer.
902+
if (delta.text.length > 0) sawVisibleText = true;
894903
yield { type: "text_delta", text: delta.text };
895904
} else if (delta.type === "thinking_delta" && typeof delta.thinking === "string") {
896905
yield { type: "thinking_delta", thinking: delta.thinking };
@@ -972,6 +981,10 @@ export function createAnthropicAdapter(provider: OcxProviderConfig, cacheRetenti
972981
code: "translation_buffer_limit",
973982
message: "upstream translation buffer exceeded the safe limit",
974983
};
984+
// The budget error IS the terminal event for this stream. Falling through to the
985+
// EOF handling below could append tool_call_end/done after it, violating the
986+
// one-terminal-event contract for consumers that keep draining the generator.
987+
return;
975988
} finally {
976989
if (currentToolCallId) budget.closeCall(currentToolCallId);
977990
}

src/server/relay.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,48 @@ 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+
}
2365

2466
export type InspectionCounters = {
2567
frameBufferHighWaterBytes: number;

src/server/responses/core.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ import {
139139
isNativePassthroughSseResponse,
140140
markEagerRelaySseResponse,
141141
markNativePassthroughSseResponse,
142+
readBoundedResponseText,
142143
relaySseWithFailedTail,
143144
relayWithAbort,
144145
sanitizePassthroughHeaders,
@@ -1963,7 +1964,16 @@ async function handleResponsesInner(
19631964
}));
19641965
}
19651966
if (headers.get("content-type")?.toLowerCase().includes("application/json")) {
1966-
const text = await upstreamResponse.text();
1967+
// Bounded whole-body read: a non-streaming upstream JSON body is fully materialized
1968+
// here (and again by the request-log finalizer and the WebSocket bridge's reframing),
1969+
// so an unbounded .text() would let a hostile or stuck upstream grow proxy memory
1970+
// without limit. This path is no longer rare — WebSocket turns for models whose
1971+
// streaming terminal event is unreliable are deliberately answered with bounded JSON.
1972+
const bounded = await readBoundedResponseText(upstreamResponse.body);
1973+
if (bounded.truncated) {
1974+
return formatErrorResponse(502, "upstream_error", "upstream JSON response exceeded the safe body limit");
1975+
}
1976+
const text = bounded.text;
19671977
inspectResponseLogJson(logCtx, text);
19681978
if (rememberPassthroughResponse) {
19691979
try {

tests/anthropic-eof-tolerance.test.ts

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { describe, expect, test } from "bun:test";
22
import { createAnthropicAdapter as createAnthropicAdapterProduction } from "../src/adapters/anthropic";
33
import { FREE_PROVIDER_DIRECTORY } from "../src/providers/free-directory";
44
import type { AdapterEvent, OcxProviderConfig } from "../src/types";
5-
import { withTestTranslatorBudget } from "./helpers/translator-budget";
5+
import { createTestTranslatorBudget, withTestTranslatorBudget } from "./helpers/translator-budget";
66

77
/**
88
* #658: AgentRouter's Anthropic-compatible endpoint can close the stream before
@@ -119,4 +119,63 @@ describe("AgentRouter Anthropic EOF tolerance (#658)", () => {
119119
const row = FREE_PROVIDER_DIRECTORY.find(provider => provider.id === "agentrouter");
120120
expect(row?.anthropicEofTolerance).toBe(true);
121121
});
122+
123+
test("an empty text delta at EOF stays a truncation error even when tolerant", async () => {
124+
// Review finding: `sawVisibleText` must require non-empty text, or a cut-off turn
125+
// surfaces as a successful empty answer.
126+
const events = await collect(tolerant, [
127+
'event: message_start\ndata: {"type":"message_start","message":{"usage":{"input_tokens":2}}}',
128+
'event: content_block_start\ndata: {"type":"content_block_start","index":0,"content_block":{"type":"text","text":""}}',
129+
'event: content_block_delta\ndata: {"type":"content_block_delta","index":0,"delta":{"type":"text_delta","text":""}}',
130+
]);
131+
132+
expect(events.at(-1)).toEqual({ type: "error", message: TRUNCATION });
133+
expect(events.some(event => event.type === "done")).toBe(false);
134+
});
135+
136+
test("a budget overflow is the single terminal event on the tolerant path", async () => {
137+
// Review finding: after the translation_buffer_limit error the generator must return,
138+
// not fall through to EOF tolerance and append tool_call_end/done behind the error.
139+
const adapter = createAnthropicAdapter(tolerant);
140+
const events: AdapterEvent[] = [];
141+
for await (const event of adapter.parseStream(
142+
sseResponse(toolEof('{"value":42}', "toolu_1").concat([
143+
`event: content_block_delta\ndata: ${JSON.stringify({ type: "content_block_delta", index: 0, delta: { type: "input_json_delta", partial_json: ',"extra":true' } })}`,
144+
])),
145+
createTestTranslatorBudget({ maxCallArgumentBytes: 16 }),
146+
)) {
147+
events.push(event);
148+
}
149+
150+
const terminal = events.at(-1);
151+
expect(terminal?.type).toBe("error");
152+
expect((terminal as { code?: string }).code).toBe("translation_buffer_limit");
153+
const firstError = events.findIndex(event => event.type === "error");
154+
expect(events.slice(firstError + 1).some(event => event.type === "done" || event.type === "tool_call_end")).toBe(false);
155+
});
156+
157+
test("repair never materializes a brace index over hostile input", async () => {
158+
// Review finding: the repair scan must stay backward and candidate-bounded. 4 MiB of
159+
// unmatched opens would have built two O(n) offset arrays in the original helper; the
160+
// result must still be the plain fallback.
161+
const hostile = "{".repeat(4 * 1024 * 1024);
162+
const payload = JSON.stringify({
163+
content: [{ type: "tool_use", id: "toolu_1", name: "get_weather", input: hostile }],
164+
});
165+
166+
const started = Date.now();
167+
const events = await createAnthropicAdapter(tolerant).parseResponse(new Response(payload));
168+
expect(Date.now() - started).toBeLessThan(10_000);
169+
expect(events).toContainEqual({ type: "tool_call_delta", arguments: "{}" });
170+
});
171+
172+
test("repair declines input above the byte cap", async () => {
173+
const oversized = `{"pad":"${"x".repeat(1024 * 1024 + 8)}`; // > 1 MiB, unparseable
174+
const payload = JSON.stringify({
175+
content: [{ type: "tool_use", id: "toolu_1", name: "get_weather", input: oversized }],
176+
});
177+
178+
const events = await createAnthropicAdapter(tolerant).parseResponse(new Response(payload));
179+
expect(events).toContainEqual({ type: "tool_call_delta", arguments: "{}" });
180+
});
122181
});

tests/deepseek-inbound-wire.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,32 @@ describe("the inbound scope survives the handleResponses replay", () => {
135135
test("ordinary HTTP Responses requests keep streaming upstream", async () => {
136136
expect((await drive("responses")).body.stream).toBe(true);
137137
});
138+
139+
test("an oversized upstream JSON body fails closed instead of buffering without limit", async () => {
140+
// Review finding: the WebSocket bounded-JSON path (and every non-streaming upstream)
141+
// materializes the whole body, so the read must have a hard byte ceiling. 33 MiB is
142+
// one MiB over MAX_UPSTREAM_JSON_BODY_BYTES.
143+
globalThis.fetch = (async () => new Response(" ".repeat(33 * 1024 * 1024), {
144+
status: 200,
145+
headers: { "content-type": "application/json" },
146+
})) as typeof fetch;
147+
const config = { providers: { deepseek: deepseekProvider() } } as unknown as OcxConfig;
148+
const response = await handleResponses(
149+
new Request("http://localhost/v1/responses", {
150+
method: "POST",
151+
headers: { "content-type": "application/json" },
152+
body: JSON.stringify({ model: MODEL, input: "ping", stream: true }),
153+
}),
154+
config,
155+
{ model: "", provider: "" },
156+
{ inboundWire: "responses", inboundTransport: "websocket" },
157+
);
158+
159+
expect(response.status).toBe(502);
160+
const payload = (await response.json()) as { error?: { code?: string; message?: string } };
161+
expect(payload.error?.code).toBe("upstream_server_error");
162+
expect(payload.error?.message).toContain("exceeded the safe body limit");
163+
});
138164
});
139165

140166
/**

0 commit comments

Comments
 (0)