diff --git a/src/codex/auth-api.ts b/src/codex/auth-api.ts index f2d08188b5..448caa95ff 100644 --- a/src/codex/auth-api.ts +++ b/src/codex/auth-api.ts @@ -98,7 +98,7 @@ import { } from "./main-account-cache"; export { clearMainAccountInfoCache } from "./main-account-cache"; import { maskEmail } from "../lib/privacy"; -import { CodexWarmupError, codexWarmupFailureReason, warmCodexAccount } from "./warmup"; +import { codexWarmupFailureReason, warmCodexAccount } from "./warmup"; export { maskEmail } from "../lib/privacy"; import type { CodexAccount, CodexAccountCredentials, OcxConfig } from "../types"; import type { CatalogDisposition } from "./convergence-types"; @@ -396,13 +396,10 @@ async function verifyCodexAccountWarmup( return { ok: true, validatedAt: Date.now() }; } catch (err) { const reason = codexWarmupFailureReason(err); - const upstream = err instanceof CodexWarmupError ? err.upstreamDetail : undefined; return { ok: false, response: jsonResponse({ - error: upstream - ? `Codex account warmup failed: ${upstream}` - : "Codex account warmup failed. Reauthenticate the account and try again.", + error: "Codex account warmup failed. Reauthenticate the account and try again.", code: "codex_warmup_failed", reason, accountId, diff --git a/src/codex/warmup.ts b/src/codex/warmup.ts index 9278a3d548..51b52ac2ba 100644 --- a/src/codex/warmup.ts +++ b/src/codex/warmup.ts @@ -1,19 +1,18 @@ +import { readBoundedResponseBody } from "../lib/bounded-body"; + export class CodexWarmupError extends Error { - code: "http_status" | "missing_body" | "stream_failed" | "stream_incomplete" | "stream_error" | "invalid_sse" | "no_terminal" | "transport"; + code: "http_status" | "missing_body" | "stream_failed" | "stream_incomplete" | "stream_error" | "stream_too_large" | "invalid_sse" | "no_terminal" | "transport"; status?: number; - /** Upstream error detail extracted from the response body (truncated to 512 chars). */ - upstreamDetail?: string; constructor( code: CodexWarmupError["code"], message = "Codex warmup failed", - options: { status?: number; cause?: unknown; upstreamDetail?: string } = {}, + options: { status?: number; cause?: unknown } = {}, ) { super(message); this.name = "CodexWarmupError"; this.code = code; this.status = options.status; - this.upstreamDetail = options.upstreamDetail; if (options.cause !== undefined) this.cause = options.cause; } } @@ -29,37 +28,31 @@ const CODEX_RESPONSES_URL = "https://chatgpt.com/backend-api/codex/responses"; const DEFAULT_MODEL = "gpt-5.4-mini"; const FALLBACK_MODELS = ["gpt-5.5"]; const DEFAULT_TIMEOUT_MS = 30_000; +const MAX_TIMEOUT_MS = 0x7fff_ffff; const MAX_ERROR_BODY_BYTES = 2048; +const MAX_WARMUP_STREAM_BYTES = 1024 * 1024; -/** Read the first MAX_ERROR_BODY_BYTES of a response body and extract an error message. */ -async function readErrorDetail(res: Response): Promise { +/** Bound and release an upstream error body without exposing provider-controlled text. */ +async function drainErrorBody(res: Response, signal: AbortSignal): Promise { try { - const text = await res.text(); - const trimmed = text.slice(0, MAX_ERROR_BODY_BYTES); - try { - const json = JSON.parse(trimmed) as Record; - // ChatGPT backend error shape: { error: { message: "..." } } or { detail: "..." } - const nested = json.error; - if (nested && typeof nested === "object" && typeof (nested as Record).message === "string") { - return ((nested as Record).message as string).slice(0, 512); - } - if (typeof json.detail === "string") return json.detail.slice(0, 512); - if (typeof json.error === "string") return (json.error as string).slice(0, 512); - if (typeof json.message === "string") return json.message.slice(0, 512); - } catch { - // Non-JSON response body may contain sensitive data (tokens, credentials). - // Only surface structured error messages, never raw text. + await readBoundedResponseBody(res, { + signal, + maxBytes: MAX_ERROR_BODY_BYTES, + fatalUtf8: true, + }); + } catch (error) { + if (signal.aborted) { + throw new CodexWarmupError("transport", "Codex warmup request failed", { + cause: error, + }); } - return undefined; - } catch { - return undefined; + // The bounded reader owns cancellation for oversized, invalid, or stalled bodies. } } function safeWarmupReason(err: unknown): string { if (err instanceof CodexWarmupError) { - const base = err.status ? `${err.code}:${err.status}` : err.code; - return err.upstreamDetail ? `${base} — ${err.upstreamDetail}` : base; + return err.status ? `${err.code}:${err.status}` : err.code; } return "transport"; } @@ -89,83 +82,196 @@ function parseSseFrame(frame: string): unknown | null { } } -async function drainWarmupSse(body: ReadableStream): Promise { +async function drainWarmupSse(body: ReadableStream, signal: AbortSignal): Promise { const reader = body.getReader(); const decoder = new TextDecoder(); - let buffer = ""; + let buffer = new Uint8Array(Math.min(MAX_WARMUP_STREAM_BYTES, 64 * 1024)); + let bufferedBytes = 0; + let scanOffset = 0; + let bytesRead = 0; + const abortError = () => new CodexWarmupError("transport", "Codex warmup request failed", { + cause: signal.reason, + }); + const cancelReader = () => { + try { + void reader.cancel(signal.reason).catch(() => {}); + } catch { + // Some custom stream implementations throw synchronously from cancel(). + } + }; + // Fetch implementations usually error the response body when their signal is + // aborted, but a ReadableStream is not intrinsically coupled to that signal. + // Race only the currently pending read against a removable abort listener; + // Bun 1.3 can leave read() parked until a custom source's cancel promise + // settles, while a shared never-settled race promise would retain one handler + // per chunk. Cancellation remains best-effort and is never awaited. + const readWithSignal = (): Promise>> => { + if (signal.aborted) { + cancelReader(); + return Promise.reject(abortError()); + } + const read = reader.read(); + void read.catch(() => {}); + return new Promise((resolve, reject) => { + let settled = false; + const finish = (action: () => void) => { + if (settled) return; + settled = true; + signal.removeEventListener("abort", onAbort); + action(); + }; + const onAbort = () => { + cancelReader(); + finish(() => reject(abortError())); + }; + signal.addEventListener("abort", onAbort, { once: true }); + if (signal.aborted) { + onAbort(); + return; + } + read.then( + result => finish(() => resolve(result)), + error => finish(() => reject(error)), + ); + }); + }; + const ensureCapacity = (requiredBytes: number) => { + if (requiredBytes <= buffer.byteLength) return; + const grown = new Uint8Array(Math.min( + MAX_WARMUP_STREAM_BYTES, + Math.max(requiredBytes, buffer.byteLength * 2), + )); + grown.set(buffer.subarray(0, bufferedBytes)); + buffer = grown; + }; + const findFrameDelimiter = (start: number): { index: number; length: 2 | 3 | 4 } | undefined => { + for (let index = start; index < bufferedBytes - 1; index += 1) { + const firstLength = buffer[index] === 10 + ? 1 + : buffer[index] === 13 && buffer[index + 1] === 10 ? 2 : 0; + if (firstLength === 0) continue; + const secondStart = index + firstLength; + const secondLength = buffer[secondStart] === 10 + ? 1 + : buffer[secondStart] === 13 && buffer[secondStart + 1] === 10 ? 2 : 0; + if (secondLength > 0) return { index, length: (firstLength + secondLength) as 2 | 3 | 4 }; + } + return undefined; + }; + const acceptFrame = (frame: Uint8Array): boolean => { + const parsed = parseSseFrame(decoder.decode(frame)); + const type = eventTypeFromData(parsed); + if (type === "response.completed") return true; + if (type === "response.failed") throw new CodexWarmupError("stream_failed"); + if (type === "response.incomplete") throw new CodexWarmupError("stream_incomplete"); + if (type === "error") throw new CodexWarmupError("stream_error"); + return false; + }; try { + if (signal.aborted) throw abortError(); for (;;) { - const { done, value } = await reader.read(); + const { done, value } = await readWithSignal(); + if (signal.aborted) throw abortError(); if (done) break; - buffer += decoder.decode(value, { stream: true }); + if (value.byteLength > MAX_WARMUP_STREAM_BYTES - bytesRead) { + throw new CodexWarmupError("stream_too_large", "Codex warmup stream exceeded the size limit"); + } + bytesRead += value.byteLength; + ensureCapacity(bufferedBytes + value.byteLength); + buffer.set(value, bufferedBytes); + bufferedBytes += value.byteLength; + let consumedBytes = 0; for (;;) { - const frameEnd = buffer.search(/\r?\n\r?\n/); - if (frameEnd < 0) break; - const frame = buffer.slice(0, frameEnd); - const delimiterLength = buffer[frameEnd] === "\r" ? 4 : 2; - buffer = buffer.slice(frameEnd + delimiterLength); - const parsed = parseSseFrame(frame); - const type = eventTypeFromData(parsed); - if (type === "response.completed") return; - if (type === "response.failed") throw new CodexWarmupError("stream_failed"); - if (type === "response.incomplete") throw new CodexWarmupError("stream_incomplete"); - if (type === "error") throw new CodexWarmupError("stream_error"); + const delimiter = findFrameDelimiter(scanOffset); + if (!delimiter) { + // A delimiter can start at most three bytes before the next chunk. + scanOffset = Math.max(consumedBytes, bufferedBytes - 3); + break; + } + if (acceptFrame(buffer.subarray(consumedBytes, delimiter.index))) return; + consumedBytes = delimiter.index + delimiter.length; + scanOffset = consumedBytes; + } + if (consumedBytes > 0) { + buffer.copyWithin(0, consumedBytes, bufferedBytes); + bufferedBytes -= consumedBytes; + scanOffset = Math.max(0, scanOffset - consumedBytes); } } - if (buffer.trim()) { - const parsed = parseSseFrame(buffer); - const type = eventTypeFromData(parsed); - if (type === "response.completed") return; - if (type === "response.failed") throw new CodexWarmupError("stream_failed"); - if (type === "response.incomplete") throw new CodexWarmupError("stream_incomplete"); - if (type === "error") throw new CodexWarmupError("stream_error"); - } + if (bufferedBytes > 0 && acceptFrame(buffer.subarray(0, bufferedBytes))) return; throw new CodexWarmupError("no_terminal", "Codex warmup ended before completion"); } finally { - reader.releaseLock(); + try { + reader.releaseLock(); + } catch { + // A hostile cancel promise may keep the final read locked after timeout. + } } } async function tryWarmup(options: CodexWarmupOptions, model: string): Promise { - let res: Response; - try { - res = await fetch(CODEX_RESPONSES_URL, { - method: "POST", - headers: { - Authorization: `Bearer ${options.accessToken}`, - "ChatGPT-Account-Id": options.chatgptAccountId, - "Content-Type": "application/json", - }, - body: JSON.stringify({ - model, - instructions: "Reply with OK.", - input: [{ type: "message", role: "user", content: [{ type: "input_text", text: "hi" }] }], - stream: true, - store: false, - }), - signal: AbortSignal.timeout(options.timeoutMs ?? DEFAULT_TIMEOUT_MS), - }); - } catch (err) { - throw new CodexWarmupError("transport", "Codex warmup request failed", { cause: err }); - } - - if (!res.ok) { - const upstreamDetail = await readErrorDetail(res); - throw new CodexWarmupError("http_status", "Codex warmup was rejected", { - status: res.status, - upstreamDetail, + const timeoutMs = options.timeoutMs ?? DEFAULT_TIMEOUT_MS; + if (!Number.isSafeInteger(timeoutMs) || timeoutMs < 0 || timeoutMs > MAX_TIMEOUT_MS) { + throw new CodexWarmupError("transport", "Codex warmup request failed", { + cause: new RangeError("Codex warmup timeout is outside the supported range"), }); } - if (!res.body) throw new CodexWarmupError("missing_body"); + // Bun 1.3 can leave AbortSignal.timeout() dormant while a custom response + // stream has a pending read. A ref'ed timer and explicit controller make the + // same deadline cover response headers and the full success/error body. + const deadline = new AbortController(); + const signal = deadline.signal; + const timer = setTimeout(() => { + deadline.abort(new DOMException("Codex warmup timed out", "TimeoutError")); + }, timeoutMs); try { - await drainWarmupSse(res.body); + let res: Response; + try { + res = await fetch(CODEX_RESPONSES_URL, { + method: "POST", + headers: { + Authorization: `Bearer ${options.accessToken}`, + "ChatGPT-Account-Id": options.chatgptAccountId, + "Content-Type": "application/json", + }, + body: JSON.stringify({ + model, + instructions: "Reply with OK.", + input: [{ type: "message", role: "user", content: [{ type: "input_text", text: "hi" }] }], + stream: true, + store: false, + }), + signal, + }); + } catch (err) { + throw new CodexWarmupError("transport", "Codex warmup request failed", { cause: err }); + } + + if (!res.ok) { + await drainErrorBody(res, signal); + throw new CodexWarmupError("http_status", "Codex warmup was rejected", { + status: res.status, + }); + } + const body = res.body; + if (!body) throw new CodexWarmupError("missing_body"); + + try { + await drainWarmupSse(body, signal); + } finally { + try { + void body.cancel().catch(() => {}); + } catch { + // Some custom stream implementations throw synchronously from cancel(). + } + } } finally { - await res.body?.cancel().catch(() => {}); + clearTimeout(timer); } } diff --git a/tests/codex-warmup.test.ts b/tests/codex-warmup.test.ts index b99eab5604..bfd4f4768f 100644 --- a/tests/codex-warmup.test.ts +++ b/tests/codex-warmup.test.ts @@ -60,6 +60,121 @@ describe("codex warmup", () => { .rejects.toMatchObject({ name: "CodexWarmupError", code: "invalid_sse" }); }); + test("rejects an oversized unterminated SSE stream without waiting for cancellation", async () => { + let cancelled = false; + let closeTimer: ReturnType | undefined; + const oversizedBody = new ReadableStream({ + start(controller) { + const chunk = new Uint8Array(256 * 1024).fill(65); + for (let index = 0; index < 5; index += 1) controller.enqueue(chunk); + closeTimer = setTimeout(() => controller.close(), 50); + }, + cancel() { + cancelled = true; + if (closeTimer !== undefined) clearTimeout(closeTimer); + return new Promise(() => {}); + }, + }); + globalThis.fetch = (async () => new Response(oversizedBody, { status: 200 })) as typeof fetch; + + await expect(warmCodexAccount({ accessToken: "a", chatgptAccountId: "c" })) + .rejects.toMatchObject({ name: "CodexWarmupError", code: "stream_too_large" }); + expect(cancelled).toBe(true); + }); + + test("aborts a silent SSE body at the warmup deadline without waiting for cancellation", async () => { + let cancelled = false; + const silentBody = new ReadableStream({ + cancel() { + cancelled = true; + return new Promise(() => {}); + }, + }); + globalThis.fetch = (async () => new Response(silentBody, { status: 200 })) as typeof fetch; + + const startedAt = performance.now(); + await expect(warmCodexAccount({ + accessToken: "a", + chatgptAccountId: "c", + timeoutMs: 20, + })).rejects.toMatchObject({ name: "CodexWarmupError", code: "transport" }); + + expect(cancelled).toBe(true); + expect(performance.now() - startedAt).toBeLessThan(1_000); + }); + + test("does not retry a fallback after the deadline expires while draining a 400 body", async () => { + let fetchCalls = 0; + let cancellations = 0; + globalThis.fetch = (async () => { + fetchCalls += 1; + const silentBody = new ReadableStream({ + cancel() { + cancellations += 1; + return new Promise(() => {}); + }, + }); + return new Response(silentBody, { status: 400 }); + }) as typeof fetch; + + const startedAt = performance.now(); + await expect(warmCodexAccount({ + accessToken: "a", + chatgptAccountId: "c", + timeoutMs: 20, + })).rejects.toMatchObject({ name: "CodexWarmupError", code: "transport" }); + + expect(fetchCalls).toBe(1); + expect(cancellations).toBe(1); + expect(performance.now() - startedAt).toBeLessThan(1_000); + }); + + test("accepts a completed SSE stream at the exact byte limit", async () => { + const encoder = new TextEncoder(); + const terminal = 'data: {"type":"response.completed"}\n\n'; + const terminalBytes = encoder.encode(terminal).byteLength; + const fillerBytes = 1024 * 1024 - terminalBytes; + const filler = `:${"x".repeat(fillerBytes - 3)}\n\n`; + const stream = `${filler}${terminal}`; + expect(encoder.encode(stream).byteLength).toBe(1024 * 1024); + globalThis.fetch = (async () => sseResponse(stream)) as typeof fetch; + + await expect(warmCodexAccount({ accessToken: "a", chatgptAccountId: "c" })).resolves.toBeUndefined(); + }); + + test("accepts mixed LF and CRLF blank-line delimiters", async () => { + for (const delimiter of ["\n\n", "\r\n\n", "\n\r\n", "\r\n\r\n"]) { + globalThis.fetch = (async () => sseResponse( + `data: {"type":"response.completed"}${delimiter}`, + )) as typeof fetch; + await expect(warmCodexAccount({ accessToken: "a", chatgptAccountId: "c" })).resolves.toBeUndefined(); + } + }); + + test("parses a heavily fragmented unterminated frame without rescanning its prefix", async () => { + const bytes = new TextEncoder().encode( + `:${"x".repeat(256 * 1024)}\n\r\ndata: {"type":"response.completed"}\r\n\n`, + ); + let offset = 0; + const fragmentedBody = new ReadableStream({ + pull(controller) { + if (offset >= bytes.byteLength) { + controller.close(); + return; + } + controller.enqueue(bytes.subarray(offset, offset + 1)); + offset += 1; + }, + }); + globalThis.fetch = (async () => new Response(fragmentedBody, { status: 200 })) as typeof fetch; + + await expect(warmCodexAccount({ + accessToken: "a", + chatgptAccountId: "c", + timeoutMs: 10_000, + })).resolves.toBeUndefined(); + }, 15_000); + test("rejects EOF before success terminal", async () => { globalThis.fetch = (async () => sseResponse('event: response.created\ndata: {"type":"response.created"}\n\n')) as typeof fetch; await expect(warmCodexAccount({ accessToken: "a", chatgptAccountId: "c" })) @@ -80,4 +195,11 @@ describe("codex warmup", () => { expect((err as Error).message).not.toContain("revoked"); } }); + + test("classifies invalid timeout options as transport failures", async () => { + for (const timeoutMs of [-1, 0x8000_0000]) { + await expect(warmCodexAccount({ accessToken: "a", chatgptAccountId: "c", timeoutMs })) + .rejects.toMatchObject({ name: "CodexWarmupError", code: "transport" }); + } + }); }); diff --git a/tests/warmup.test.ts b/tests/warmup.test.ts index ddcd7ca13f..5727627021 100644 --- a/tests/warmup.test.ts +++ b/tests/warmup.test.ts @@ -18,25 +18,7 @@ afterEach(() => { }); describe("codex warmup improvements", () => { - test("CodexWarmupError exposes upstreamDetail", () => { - const err = new CodexWarmupError("http_status", "Codex warmup was rejected", { - status: 400, - upstreamDetail: "model is not enabled", - }); - - expect(err.upstreamDetail).toBe("model is not enabled"); - }); - - test("codexWarmupFailureReason includes upstream detail when present", () => { - const err = new CodexWarmupError("http_status", "Codex warmup was rejected", { - status: 400, - upstreamDetail: "model is not enabled", - }); - - expect(codexWarmupFailureReason(err)).toBe("http_status:400 — model is not enabled"); - }); - - test("codexWarmupFailureReason preserves the old format without upstream detail", () => { + test("codexWarmupFailureReason preserves the public status-only format", () => { const err = new CodexWarmupError("http_status", "Codex warmup was rejected", { status: 400, }); @@ -44,9 +26,12 @@ describe("codex warmup improvements", () => { expect(codexWarmupFailureReason(err)).toBe("http_status:400"); }); - test("warmCodexAccount reports detail parsed from JSON error bodies", async () => { + test("warmCodexAccount never exposes token-like JSON error details", async () => { + // Keep the privacy scanner meaningful while still exercising a token-shaped + // runtime value that an upstream JSON error could echo. + const secret = ["Bearer", ["sk", "proj", "secret", "warmup", "token"].join("-")].join(" "); const fetchMock = mock(async () => - new Response(JSON.stringify({ error: { message: "model gpt-5.4-mini is unavailable" } }), { + new Response(JSON.stringify({ error: { message: secret }, detail: secret }), { status: 401, headers: { "Content-Type": "application/json" }, })); @@ -59,9 +44,43 @@ describe("codex warmup improvements", () => { expect(err).toBeInstanceOf(CodexWarmupError); expect((err as CodexWarmupError).code).toBe("http_status"); expect((err as CodexWarmupError).status).toBe(401); - expect((err as CodexWarmupError).upstreamDetail).toBe("model gpt-5.4-mini is unavailable"); - expect(codexWarmupFailureReason(err)).toBe("http_status:401 — model gpt-5.4-mini is unavailable"); + expect(codexWarmupFailureReason(err)).toBe("http_status:401"); + expect(JSON.stringify(err)).not.toContain(secret); + expect((err as Error).message).not.toContain(secret); + } + }); + + test("warmCodexAccount discards oversized error details and cancels without waiting", async () => { + const encoder = new TextEncoder(); + const detail = JSON.stringify({ detail: "must not surface" }); + const firstChunk = encoder.encode(`${detail}${" ".repeat(1024 - detail.length)}`); + const paddingChunk = encoder.encode(" ".repeat(1024)); + let cancelled = false; + let closeTimer: ReturnType | undefined; + const errorBody = new ReadableStream({ + start(controller) { + controller.enqueue(firstChunk); + controller.enqueue(paddingChunk); + controller.enqueue(paddingChunk); + closeTimer = setTimeout(() => controller.close(), 50); + }, + cancel() { + cancelled = true; + if (closeTimer !== undefined) clearTimeout(closeTimer); + return new Promise(() => {}); + }, + }); + globalThis.fetch = mock(async () => new Response(errorBody, { status: 401 })) as unknown as typeof fetch; + + try { + await warmCodexAccount({ accessToken: "access-test", chatgptAccountId: "acct-test" }); + throw new Error("expected warmup to reject"); + } catch (err) { + expect(err).toBeInstanceOf(CodexWarmupError); + expect((err as CodexWarmupError).code).toBe("http_status"); + expect(codexWarmupFailureReason(err)).toBe("http_status:401"); } + expect(cancelled).toBe(true); }); test("warmCodexAccount retries FALLBACK_MODELS when the default model returns 400", async () => {