From 2d3fe69e1ab8bf3d8f12de605289c4d893eaffb4 Mon Sep 17 00:00:00 2001 From: Yazan-O <154028854+Yazan-O@users.noreply.github.com> Date: Fri, 24 Jul 2026 09:29:32 -0500 Subject: [PATCH] fix(http): bound the size of buffered JSON responses response.json() buffers the entire body before parsing with no upper bound, so a large test result --history page or a run with a long steps[] array grows the heap in proportion to the payload. Read the typed JSON response bounded by a configurable cap (HttpClientOptions.maxResponseBytes, default 64 MiB): reject an over-cap Content-Length up front, count bytes while streaming chunked bodies, and throw a typed PAYLOAD_TOO_LARGE (exit 5) with guidance to narrow --page-size / --since. --- src/lib/http.test.ts | 60 ++++++++++++++++++++- src/lib/http.ts | 125 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 183 insertions(+), 2 deletions(-) diff --git a/src/lib/http.test.ts b/src/lib/http.test.ts index e8f4c35..c22f1dc 100644 --- a/src/lib/http.test.ts +++ b/src/lib/http.test.ts @@ -33,6 +33,7 @@ function makeClient( apiKey?: string | null; onDebug?: (e: DebugEvent) => void; onServerVersion?: (info: { minVersion?: string }) => void; + maxResponseBytes?: number; } = {}, ): HttpClient { const apiKey = 'apiKey' in options ? (options.apiKey ?? undefined) : 'sk-test'; @@ -44,9 +45,61 @@ function makeClient( random: () => 0, onDebug: options.onDebug, onServerVersion: options.onServerVersion, + maxResponseBytes: options.maxResponseBytes, }); } +describe('response size guard (maxResponseBytes)', () => { + it('rejects a response whose Content-Length exceeds the cap', async () => { + // jsonResponse sets a real Content-Length; a 50-byte cap is well under it. + const fetchImpl = vi.fn().mockResolvedValue(jsonResponse({ blob: 'x'.repeat(500) })); + const client = makeClient(fetchImpl as unknown as typeof fetch, { maxResponseBytes: 50 }); + + const err = await client.get('/tests').catch((e: unknown) => e); + + expect(err).toBeInstanceOf(ApiError); + expect((err as ApiError).code).toBe('PAYLOAD_TOO_LARGE'); + expect((err as ApiError).exitCode).toBe(5); + expect((err as ApiError).getDetail('maxBytes')).toBe(50); + }); + + it('rejects an over-cap chunked response that has no Content-Length', async () => { + // A body built from a ReadableStream carries no Content-Length, so only the + // streaming byte-counter can catch it. + const encoder = new TextEncoder(); + const stream = new ReadableStream({ + start(controller) { + controller.enqueue(encoder.encode('{"data":"')); + controller.enqueue(encoder.encode('y'.repeat(500))); + controller.enqueue(encoder.encode('"}')); + controller.close(); + }, + }); + const fetchImpl = vi + .fn() + .mockResolvedValue( + new Response(stream, { status: 200, headers: { 'content-type': 'application/json' } }), + ); + const client = makeClient(fetchImpl as unknown as typeof fetch, { maxResponseBytes: 50 }); + + const err = await client.get('/tests').catch((e: unknown) => e); + + expect(err).toBeInstanceOf(ApiError); + expect((err as ApiError).code).toBe('PAYLOAD_TOO_LARGE'); + }); + + it('reads a within-cap response unchanged', async () => { + const fetchImpl = vi.fn().mockResolvedValue(jsonResponse({ ok: true, items: [1, 2, 3] })); + const client = makeClient(fetchImpl as unknown as typeof fetch, { + maxResponseBytes: 1_000_000, + }); + + const body = await client.get<{ ok: boolean; items: number[] }>('/tests'); + + expect(body).toEqual({ ok: true, items: [1, 2, 3] }); + }); +}); + describe('CLIENT_TOO_OLD (426)', () => { it('is not retried — fails fast with the typed error', async () => { const fetchImpl = vi.fn().mockResolvedValue(errorEnvelopeResponse(426, 'CLIENT_TOO_OLD')); @@ -717,7 +770,12 @@ describe('HttpClient per-request timeout', () => { return { ok: true, status: 200, - json: () => Promise.reject(timeoutErr), + headers: new Headers(), + body: new ReadableStream({ + start(controller) { + controller.error(timeoutErr); + }, + }), } as unknown as Response; }); const client = new HttpClient({ diff --git a/src/lib/http.ts b/src/lib/http.ts index 21369de..b33395d 100644 --- a/src/lib/http.ts +++ b/src/lib/http.ts @@ -106,6 +106,14 @@ export interface HttpClientOptions { * `globalShutdown.signal` via the client factory. */ shutdownSignal?: AbortSignal; + /** + * Upper bound (bytes) on a successful JSON response body the client will + * buffer before parsing. A response whose declared `Content-Length` — or + * actual streamed size — exceeds this fails fast with a typed + * `PAYLOAD_TOO_LARGE` error instead of growing the heap without limit. + * Defaults to {@link MAX_RESPONSE_BYTES_DEFAULT} (64 MiB). + */ + maxResponseBytes?: number; } export interface RequestOptions { @@ -168,6 +176,15 @@ const MAX_RATE_LIMITED_DELAY_MS = 60_000; const CONFLICT_DELAY_MS = 1000; const INTERNAL_DELAY_MS = 500; +// Upper bound on the size of a successful JSON response body the client will +// buffer into memory. `response.json()` reads the ENTIRE body before parsing, +// with no limit, so a large `test result --history` page or a run with a long +// `steps[]` array (getRun `includeSteps`) would grow the heap in proportion to +// the payload. 64 MiB sits far above any legitimate metadata/history/steps +// response yet still bounds a pathological — or hostile — one. Override per +// client via `HttpClientOptions.maxResponseBytes`. +const MAX_RESPONSE_BYTES_DEFAULT = 64 * 1024 * 1024; + /** * Result of a successful HTTP request, including the parsed body and the * `x-request-id` that was sent (useful for surfacing in happy-path output). @@ -189,6 +206,7 @@ export class HttpClient { private readonly onServerVersion?: (info: { minVersion?: string }) => void; private readonly requestTimeoutMs: number; private readonly shutdownSignal?: AbortSignal; + private readonly maxResponseBytes: number; constructor(options: HttpClientOptions) { this.baseUrl = trimTrailingSlash(options.baseUrl); @@ -201,6 +219,7 @@ export class HttpClient { this.onTransition = options.onTransition; this.onServerVersion = options.onServerVersion; this.requestTimeoutMs = options.requestTimeoutMs ?? REQUEST_TIMEOUT_DEFAULT_MS; + this.maxResponseBytes = options.maxResponseBytes ?? MAX_RESPONSE_BYTES_DEFAULT; } /** @@ -595,10 +614,15 @@ export class HttpClient { durationMs, }); try { - return { body: (await response.json()) as T, requestId, status: response.status }; + const text = await readBoundedText(response, this.maxResponseBytes, requestId); + return { body: JSON.parse(text) as T, requestId, status: response.status }; } catch (err) { // Interrupt passthrough (see the fetch catch above). if (err instanceof InterruptError) throw err; + // A bounded-read rejection (PAYLOAD_TOO_LARGE) — or any typed ApiError — + // is a real, actionable outcome; surface it unchanged rather than + // masking it as a malformed-body error below. + if (err instanceof ApiError) throw err; // A timeout/abort can fire mid-body-read (headers received, stream stalls). this.rethrowIfAbort(err, timeoutSignal, options.signal, requestId, effectiveSignal); // Otherwise the successful response body was not valid JSON — a @@ -919,6 +943,105 @@ export function malformedResponseError( ); } +/** + * Read a successful response body as text, bounded to `maxBytes`. + * + * `response.json()` buffers the ENTIRE body into memory before parsing, with no + * upper limit — a large `test result --history` page, or a run with a long + * `steps[]` array (getRun `includeSteps`), grows the heap in proportion to the + * payload. This reads the body incrementally and stops once the accumulated + * size crosses `maxBytes`, so a pathological (or hostile) response fails fast + * with a typed PAYLOAD_TOO_LARGE error instead of exhausting memory. + * + * A declared `Content-Length` over the cap is rejected before any body is read. + * Chunked bodies (no `Content-Length`) are bounded by counting bytes as they + * stream. Decoding happens once, at the end, so multi-byte UTF-8 sequences that + * straddle a chunk boundary still decode correctly. + */ +async function readBoundedText( + response: Response, + maxBytes: number, + requestId: string, +): Promise { + const declared = Number(response.headers.get('content-length')); + if (Number.isFinite(declared) && declared > maxBytes) { + throw responseTooLargeError(requestId, maxBytes, declared); + } + const stream = response.body; + if (stream === null) { + // No readable stream exposed (some runtimes / test doubles): fall back to a + // buffered read, then enforce the cap on the materialized text. + const text = await response.text(); + if (new TextEncoder().encode(text).length > maxBytes) { + throw responseTooLargeError(requestId, maxBytes); + } + return text; + } + const reader = stream.getReader(); + const chunks: Uint8Array[] = []; + let total = 0; + try { + for (;;) { + const { done, value } = await reader.read(); + if (done) break; + if (value === undefined) continue; + total += value.byteLength; + if (total > maxBytes) { + await reader.cancel(); + throw responseTooLargeError(requestId, maxBytes, total); + } + chunks.push(value); + } + } finally { + try { + reader.releaseLock(); + } catch { + // The reader may already be released after cancel(); releasing twice is a + // no-op we don't want surfacing over the original error. + } + } + return new TextDecoder('utf-8').decode(concatChunks(chunks, total)); +} + +/** Concatenate byte chunks into a single `Uint8Array` of known total length. */ +function concatChunks(chunks: readonly Uint8Array[], total: number): Uint8Array { + const out = new Uint8Array(total); + let offset = 0; + for (const chunk of chunks) { + out.set(chunk, offset); + offset += chunk.byteLength; + } + return out; +} + +/** + * Typed error for a response body that exceeds the client-side buffering cap. + * Reuses PAYLOAD_TOO_LARGE (exit 5, validation family) — the same code the + * backend returns for oversized request bodies — so machine consumers route on + * it uniformly. `nextAction` names the knobs that shrink the result set. + */ +function responseTooLargeError( + requestId: string, + maxBytes: number, + observedBytes?: number, +): ApiError { + const limitMiB = Math.round(maxBytes / (1024 * 1024)); + return new ApiError({ + code: 'PAYLOAD_TOO_LARGE', + message: + `The server response exceeded the client-side ${limitMiB} MiB limit and was not ` + + `buffered, to avoid unbounded memory use.`, + nextAction: + 'Narrow the result set and retry — e.g. a smaller --page-size, a tighter --since ' + + 'window, or scope --history to a single run.', + requestId, + details: { + maxBytes, + ...(observedBytes !== undefined ? { observedBytes } : {}), + }, + }); +} + export function parseRetryAfter(headerValue: string | null): number | undefined { if (!headerValue) return undefined; const numeric = Number(headerValue);