diff --git a/docs-site/src/content/docs/reference/configuration/providers.md b/docs-site/src/content/docs/reference/configuration/providers.md index f7e8f16abe..fd72f9ab61 100644 --- a/docs-site/src/content/docs/reference/configuration/providers.md +++ b/docs-site/src/content/docs/reference/configuration/providers.md @@ -45,6 +45,7 @@ After GUI registration or OAuth login, the confirmation dialog lets you open the | `upstreamFailoverThreshold?` | `number` | `3` | Consecutive transient failures before future new sessions fail over. Set `0` to disable. For regular Responses and native compact sends, proven pre-connection DNS/TCP reachability failures are tracked at the provider-host level: they never affect account health, account cooldowns, thread/session affinity, active-account selection, or Pool routing, and never count toward this threshold. | | `upstreamHostCircuitThreshold?` | `number` | `0` | Opt-in circuit threshold for proven pre-connection DNS/TCP failures on native OpenAI forward Responses and compact sends. `0` disables it; `1`–`20` opens a 30-second provider-origin cooldown after that many terminal logical requests. While open, requests receive `503` with `Retry-After` before account selection or upstream send; after cooldown, one half-open request is admitted. Timeouts and HTTP responses never count, and any HTTP response closes the circuit. Applies only to Codex Pool routing with no pinned account; it is inert for `codexAccountMode: "direct"` and account-qualified selectors. | | `maxUpstreamBodyBytes?` | `number` | `0` | Opt-in ceiling, in bytes, on a serialized native Responses **passthrough** body. `0` or omitted disables it — no limit is inferred for any destination. When set, a built body above the ceiling is refused locally before the send: streaming turns receive a terminal `response.failed` / `context_length_exceeded` so the client compacts instead of resending, and non-streaming turns receive a `413` naming the size, the number of embedded `input_image` items, and roughly how many megabytes of image data they represent. Checked at every build and rebuild point, including OAuth-refresh replay and alternate-account retry. Translated adapter paths are not covered. There is deliberately no default: the only measured ceiling here belongs to the WebSocket transport, which already falls back to HTTP for oversized turns, so a default would refuse requests that currently succeed. Set it when your gateway has a known request-size limit and you would rather see an actionable local error than an opaque upstream failure. | +| `maxInboundBodyBytes?` | `number` | `0` | Opt-in ceiling, in bytes, on a decompressed **inbound** data-plane request body — the mirror of `maxUpstreamBodyBytes` above. `0` or omitted keeps the built-in 256 MiB default. Raise it when a large-context session can no longer compact: Codex replays the whole history to the compaction model, so on the 922k-token opt-in window the compaction request is itself the one that crosses the limit, and the session is stuck at the only operation that would have shrunk it. Clamped to 1 MiB–512 MiB. The ceiling is not negotiable: the reader materializes the body several times over (wire bytes, decoded bytes, the decoded string, and the parsed object graph), so peak memory is a multiple of whatever is admitted, and an unbounded value would be a memory exhaustion lever. Applies to `/v1/responses`, `/v1/responses/compact`, `/v1/chat/completions`, and `/v1/messages`. The listener's accept size is fixed when the proxy binds, so a change takes effect on restart. A body above the limit is refused locally with HTTP 413 and `code: "inbound_body_too_large"`, which is deliberately distinct from the `context_length_exceeded` 413 a provider size refusal produces. | | `modelCacheTtlMs?` | `number` | `300000` | Freshness window for the per-provider `/models` cache. | | `cacheRetention?` | `"none" \| "short" \| "long"` | `"short"` | Anthropic prompt-cache policy: disabled, 5-minute ephemeral, or 1-hour extended. | | `tokenGuardian?` | `OcxTokenGuardianConfig` | off | Optional proactive OAuth refresh and Codex-account warmup policy. | diff --git a/src/config.ts b/src/config.ts index 8cfaf63391..d6a3b7ecd4 100644 --- a/src/config.ts +++ b/src/config.ts @@ -1118,6 +1118,15 @@ const configSchema = z.object({ .min(0) .optional() .catch(undefined), + // Opt-in inbound body ceiling (#3573). An invalid hand edit degrades to the 256 MiB default + // rather than failing the parse, matching the outbound guard above: a malformed number must + // not change what the proxy admits. The hard ceiling is NOT enforced here — because of that + // `.catch`, and because a config object can be built without this schema at all — but in + // `resolveInboundBodyLimitBytes()`, which every reader goes through. + maxInboundBodyBytes: z.number().int() + .min(0) + .optional() + .catch(undefined), appOwnedMemoryBudgetMb: z.number().int() .min(MIN_APP_OWNED_MEMORY_BUDGET_MB) .max(MAX_APP_OWNED_MEMORY_BUDGET_MB) diff --git a/src/lib/errors.ts b/src/lib/errors.ts index 8fb5126487..46b2fa3ed2 100644 --- a/src/lib/errors.ts +++ b/src/lib/errors.ts @@ -209,6 +209,14 @@ export function classifyError(status: number, type: string, message: string): Oc if (type === "input_admission_refused") { return { message, type: "invalid_request_error", code: "input_admission_refused" }; } + // A LOCAL inbound admission refusal (#3573) keeps its own code for the same reason as the + // preflight refusal above. #4112 gave the UPSTREAM 413 on this surface + // `context_length_exceeded`; without a distinct code here a client cannot tell a body the + // proxy never read from a turn the provider itself rejected, and only one of the two is + // fixed by raising `maxInboundBodyBytes`. + if (type === "inbound_body_too_large") { + return { message, type: "invalid_request_error", code: "inbound_body_too_large" }; + } if ( text.includes("context_length_exceeded") || text.includes("context window") || diff --git a/src/server/chat-completions.ts b/src/server/chat-completions.ts index 8ee5f52a49..20c29856ec 100644 --- a/src/server/chat-completions.ts +++ b/src/server/chat-completions.ts @@ -28,7 +28,7 @@ import { resolveWireProtocolOverride } from "./adapter-resolve"; import { resolveOpenCodeGoTransport } from "../providers/opencode-go-transport"; import { normalizeLogConversationId, sessionLaneIdFromRequest } from "./request-log-conversation"; import type { OcxConfig } from "../types"; -import { readJsonRequestBody } from "./request-decompress"; +import { readJsonRequestBody, resolveInboundBodyLimitBytes } from "./request-decompress"; import { addFinalRequestLog, httpStatusForRequestLogTerminal, @@ -62,9 +62,9 @@ type Rec = Record; function isRec(v: unknown): v is Rec { return !!v && typeof v === "object" && !Array.isArray(v); } -async function readChatBody(req: Request, budget: TranslatorBudget): Promise { +async function readChatBody(req: Request, budget: TranslatorBudget, maxBytes: number): Promise { try { - return await readJsonRequestBody(req, budget); + return await readJsonRequestBody(req, budget, maxBytes); } catch (err) { if (isTranslatorBudgetExceededError(err)) throw err; throw new ChatCompletionsRequestError(err instanceof Error && err.message ? err.message : "Invalid JSON body"); @@ -106,7 +106,7 @@ async function handleChatCompletionsWithBudget( ): Promise { let chatBody: Rec; try { - const rawBody = await readChatBody(req, translatorBudget); + const rawBody = await readChatBody(req, translatorBudget, resolveInboundBodyLimitBytes(config.maxInboundBodyBytes)); assertChatCompletionsRoutingBody(rawBody); chatBody = rawBody; } catch (err) { diff --git a/src/server/claude-messages.ts b/src/server/claude-messages.ts index 7ff76f0f2e..a16466459b 100644 --- a/src/server/claude-messages.ts +++ b/src/server/claude-messages.ts @@ -34,7 +34,7 @@ import { registryEntryForProviderDestination } from "../providers/registry"; import { evidenceFromBody } from "../routing/request-evidence"; import { resolveWireProtocolOverride } from "./adapter-resolve"; import type { OcxConfig } from "../types"; -import { readJsonRequestBody } from "./request-decompress"; +import { readJsonRequestBody, resolveInboundBodyLimitBytes } from "./request-decompress"; import { addFinalRequestLog, httpStatusForRequestLogTerminal, recordFirstOutput, type RequestLogContext, type RequestLogEntry } from "./request-log"; import { conversationIdFromClaudeMetadata, normalizeLogConversationId, sessionLaneIdFromRequest } from "./request-log-conversation"; import { responseWithDeferredRequestLog } from "./relay"; @@ -129,9 +129,9 @@ function claudeInboundDisabled(config: OcxConfig): Response | null { return null; } -async function readAnthropicBody(req: Request, budget: TranslatorBudget): Promise { +async function readAnthropicBody(req: Request, budget: TranslatorBudget, maxBytes: number): Promise { try { - return await readJsonRequestBody(req, budget); + return await readJsonRequestBody(req, budget, maxBytes); } catch (err) { if (isTranslatorBudgetExceededError(err)) throw err; throw new AnthropicRequestError(err instanceof Error && err.message ? err.message : "Invalid JSON body"); @@ -655,7 +655,7 @@ async function handleClaudeMessagesWithBudget( let fastRow: ParsedFastRowId | null = null; let requestedModel = ""; try { - anthropicBody = await readAnthropicBody(req, translatorBudget); + anthropicBody = await readAnthropicBody(req, translatorBudget, resolveInboundBodyLimitBytes(config.maxInboundBodyBytes)); // Defensive [1m] strip (devlog 138): clients normally remove the context-variant // marker themselves; the 1M signal we act on is the anthropic-beta header. // Case-insensitive — the CLI matches /\[1m\]/i (audit 021 #7). @@ -1154,7 +1154,7 @@ export async function handleClaudeCountTokens( let body: unknown; const translatorBudget = createTranslatorBudget(); try { - body = await readAnthropicBody(req, translatorBudget); + body = await readAnthropicBody(req, translatorBudget, resolveInboundBodyLimitBytes(config.maxInboundBodyBytes)); } catch (err) { if (err instanceof DesktopModelMappingUnavailableError) return desktopMappingUnavailableResponse(err); if (err instanceof AnthropicRequestError) return anthropicErrorResponse(400, err.message); diff --git a/src/server/images.ts b/src/server/images.ts index 02e56fcacf..5642038474 100644 --- a/src/server/images.ts +++ b/src/server/images.ts @@ -29,7 +29,7 @@ import { sidecarEnter } from "../lib/sidecar-tracker"; import type { OcxConfig } from "../types"; import { resolveFirstUsableOpenAiSidecar, selectImagesProvider } from "../providers/openai-sidecar"; import { getProviderRegistryEntry } from "../providers/registry"; -import { readJsonRequestBody } from "./request-decompress"; +import { readJsonRequestBody, resolveInboundBodyLimitBytes } from "./request-decompress"; import { ForwardAdmissionCredentialError, validateForwardAdmissionCredential } from "./auth-cors"; import type { RequestLogContext } from "./request-log"; import { codexLogAccountId, decodeRequestErrorResponse } from "./responses"; @@ -602,7 +602,7 @@ export async function handleImages( ): Promise { let body: unknown; try { - body = await readJsonRequestBody(req); + body = await readJsonRequestBody(req, undefined, resolveInboundBodyLimitBytes(config.maxInboundBodyBytes)); } catch (err) { return decodeRequestErrorResponse(err, "images"); } diff --git a/src/server/index.ts b/src/server/index.ts index 8d46c30ab6..dc3bc2561d 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -63,7 +63,11 @@ import { runModelRenameStartupMigration } from "../providers/model-rename-startu import { isCanonicalOpenAiForwardProvider, OPENAI_CODEX_PROVIDER_ID } from "../providers/openai-tiers"; import { providerCodexAccountMode } from "../providers/registry"; import type { StorageCleanupPolicy } from "../types"; -import { MAX_DECOMPRESSED_BODY_BYTES } from "./request-decompress"; +import { + MAX_CONFIGURABLE_INBOUND_BODY_BYTES, + MIN_CONFIGURABLE_INBOUND_BODY_BYTES, + resolveInboundBodyLimitBytes, +} from "./request-decompress"; import { CodexAccountCooldownError, cooldownErrorMessage, @@ -1023,6 +1027,22 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server | null = null; let managementIngressServer: Server | null = null; + // Resolved once, before any listener binds. The clamp is silent inside the resolver so it + // stays pure and per-request cheap; the operator is told here instead, once, because a + // config value that was quietly reduced is exactly the thing they would otherwise debug + // against the wrong limit. + const inboundBodyLimitBytes = resolveInboundBodyLimitBytes(config.maxInboundBodyBytes); + const requestedInboundBodyLimit = config.maxInboundBodyBytes; + if (requestedInboundBodyLimit !== undefined + && requestedInboundBodyLimit > 0 + && requestedInboundBodyLimit !== inboundBodyLimitBytes) { + console.warn( + `[server] maxInboundBodyBytes=${requestedInboundBodyLimit} is outside the supported range ` + + `[${MIN_CONFIGURABLE_INBOUND_BODY_BYTES}, ${MAX_CONFIGURABLE_INBOUND_BODY_BYTES}]; ` + + `using ${inboundBodyLimitBytes} bytes.`, + ); + } + type ServerIngress = "public" | "unauthenticated-loopback" | "hub-management"; function ingressForServer(requestServer: Server): ServerIngress { if (requestServer === loopbackServer) return "unauthenticated-loopback"; @@ -1042,7 +1062,10 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server): Promise { const ingress = ingressForServer(requestServer); // The unauthenticated loopback listener (#1102) serves a fixed allowlist and nothing diff --git a/src/server/request-decompress.ts b/src/server/request-decompress.ts index 297c77a9d1..1939479429 100644 --- a/src/server/request-decompress.ts +++ b/src/server/request-decompress.ts @@ -21,6 +21,58 @@ import type { TranslatorBudget } from "../lib/translator-budget"; */ export const MAX_DECOMPRESSED_BODY_BYTES = 256 * 1024 * 1024; +/** + * Hard ceiling on the opt-in `maxInboundBodyBytes` (#3573). + * + * The opt-in exists because a 922k-token session serializes past the 256 MiB default, and the + * request that crosses it is the compaction request itself — so the session can no longer + * shrink and is stuck. An UNBOUNDED inbound cap is not an acceptable answer: this admission + * limit is the only thing standing between one request and the process heap, and + * `readBoundedJsonRequestBody` materializes the body several times over (retained wire bytes, + * decoded bytes, the decoded string, the re-encoded measurement copies, and the parsed object + * graph), so peak RSS is a MULTIPLE of whatever is admitted here. 512 MiB is the largest value + * that keeps that multiple survivable on an ordinary machine, and it is what #3573 asked for. + */ +export const MAX_CONFIGURABLE_INBOUND_BODY_BYTES = 512 * 1024 * 1024; + +/** Floor for the opt-in. Below this an ordinary multi-image turn cannot be admitted at all. */ +export const MIN_CONFIGURABLE_INBOUND_BODY_BYTES = 1024 * 1024; + +/** + * Resolve the configured inbound admission limit, clamped to the supported range. + * + * Pure and total on purpose: the schema in `src/config.ts` degrades an invalid hand edit to + * `undefined` rather than failing the parse, so the schema cannot be the place the ceiling is + * enforced. Every caller resolves through here, which makes this the single auditable bound + * regardless of how the config object was produced. + * + * Omitted, zero, or non-finite = the 256 MiB default, so an unconfigured proxy admits exactly + * what it admits today. + */ +export function resolveInboundBodyLimitBytes(configured: number | undefined): number { + if (configured === undefined || !Number.isFinite(configured) || configured <= 0) { + return MAX_DECOMPRESSED_BODY_BYTES; + } + return Math.min( + Math.max(Math.floor(configured), MIN_CONFIGURABLE_INBOUND_BODY_BYTES), + MAX_CONFIGURABLE_INBOUND_BODY_BYTES, + ); +} + +/** + * Render a byte count, or nothing at all. `DecompressedBodyTooLargeError` accepts non-finite + * and untyped values from legacy callers and deliberately keeps them out of its own message; + * the client-facing message inherits that rule rather than printing `NaN MB`. + */ +function megabytes(bytes: number): string | null { + return Number.isFinite(bytes) && bytes >= 0 && bytes <= Number.MAX_SAFE_INTEGER + ? (bytes / (1024 * 1024)).toFixed(1) + : null; +} + +const INBOUND_CEILING_MB = (MAX_CONFIGURABLE_INBOUND_BODY_BYTES / (1024 * 1024)).toFixed(1); + + export class UnsupportedContentEncodingError extends Error { constructor(readonly encoding: string) { super(`Unsupported content-encoding: ${encoding}`); @@ -54,6 +106,33 @@ export class DecompressedBodyTooLargeError extends Error { } } +/** + * Name OpenCodex as the refuser, and name the lever. + * + * #4112 gave the UPSTREAM context refusal on `/v1/responses` its own HTTP 413 with + * `context_length_exceeded`. That makes the two 413s on this surface look alike to a client + * while having opposite remedies: the upstream one means the provider will not take the turn, + * this one means the proxy never read it and a config key would have let it through. The + * wording deliberately avoids "context window"/"context length", which `classifyError` treats + * as evidence of an upstream context verdict. + */ +export function describeInboundBodyRefusal(error: DecompressedBodyTooLargeError): string { + // A lower-bound measurement stopped counting at the cap; reporting it as exact would be a lie. + const approximate = error.measurement === "declared_wire" || error.measurement === "decoded_exact" + ? "" : "at least "; + const observed = megabytes(error.bytes); + const limit = megabytes(error.limit); + const sizes = limit === null + ? "the body is above the inbound admission limit" + : observed === null + ? `the body is above the ${limit} MB inbound admission limit` + : `the body is ${approximate}${observed} MB, above the ${limit} MB inbound admission limit`; + return `OpenCodex refused this request before reading it: ${sizes}. ` + + "This is a local proxy limit, not a provider refusal. Raise \"maxInboundBodyBytes\" in " + + `config.json (ceiling ${INBOUND_CEILING_MB} MB) and restart the proxy, or compact the ` + + "conversation earlier."; +} + function assertBodySizeWithinLimit( body: Uint8Array, maxBytes: number, @@ -259,7 +338,16 @@ export async function readBoundedJsonRequestBody( } } -/** Parse a JSON data-plane body using the shared 256 MiB admission cap. */ -export function readJsonRequestBody(req: Request, budget?: TranslatorBudget): Promise { - return readBoundedJsonRequestBody(req, MAX_DECOMPRESSED_BODY_BYTES, budget); +/** + * Parse a JSON data-plane body using the shared admission cap. + * + * `maxBytes` is the resolved per-deployment limit from `resolveInboundBodyLimitBytes()`; + * omitting it keeps the 256 MiB default for callers with no config in scope. + */ +export function readJsonRequestBody( + req: Request, + budget?: TranslatorBudget, + maxBytes: number = MAX_DECOMPRESSED_BODY_BYTES, +): Promise { + return readBoundedJsonRequestBody(req, maxBytes, budget); } diff --git a/src/server/responses/compact.ts b/src/server/responses/compact.ts index 567012a0f5..4a019c2f36 100644 --- a/src/server/responses/compact.ts +++ b/src/server/responses/compact.ts @@ -104,7 +104,12 @@ import { fastPolicyForModel } from "../../providers/service-tier"; import { parseFastOnlyRowId } from "../fast-row"; import { applyOpenAiVirtualModel, resolveOpenAiCompactModel } from "../../providers/openai-virtual-models"; import { isUsageDebugEnabled } from "../../usage/debug"; -import { readJsonRequestBody, DecompressedBodyTooLargeError, UnsupportedContentEncodingError } from "../request-decompress"; +import { + readJsonRequestBody, + resolveInboundBodyLimitBytes, + DecompressedBodyTooLargeError, + UnsupportedContentEncodingError, +} from "../request-decompress"; import { resolveAdapter, resolveWireProtocolOverride } from "../adapter-resolve"; import { hasKeyPoolFailover, rotateProviderTransportOn429 } from "../../providers/key-failover"; import { shouldAttemptImageTierRetry } from "../image-retry"; @@ -522,7 +527,7 @@ export async function handleResponsesCompact( ): Promise { let body: unknown; try { - body = await readJsonRequestBody(req); + body = await readJsonRequestBody(req, undefined, resolveInboundBodyLimitBytes(config.maxInboundBodyBytes)); } catch (err) { return decodeRequestErrorResponse(err, "responses-compact"); } diff --git a/src/server/responses/core.ts b/src/server/responses/core.ts index bbae23157c..e4d22a1275 100644 --- a/src/server/responses/core.ts +++ b/src/server/responses/core.ts @@ -243,7 +243,13 @@ import { hasPassiveAccountQuota, recordAnthropicAccountQuotaFromHeaders, recordP import { captureConfigGeneration } from "../../lib/state-store-sweeper"; import { applyOpenAiVirtualModel, resolveOpenAiCompactModel } from "../../providers/openai-virtual-models"; import { isUsageDebugEnabled } from "../../usage/debug"; -import { readJsonRequestBody, DecompressedBodyTooLargeError, UnsupportedContentEncodingError } from "../request-decompress"; +import { + readJsonRequestBody, + describeInboundBodyRefusal, + resolveInboundBodyLimitBytes, + DecompressedBodyTooLargeError, + UnsupportedContentEncodingError, +} from "../request-decompress"; import { resolveAdapter, resolveWireProtocolOverride } from "../adapter-resolve"; import { providerModelResponsesTerminalRepair, @@ -1607,7 +1613,7 @@ export function decodeRequestErrorResponse(err: unknown, label: string): Respons return formatErrorResponse(415, "invalid_request_error", err.message); } if (err instanceof DecompressedBodyTooLargeError) { - return formatErrorResponse(413, "invalid_request_error", err.message); + return formatErrorResponse(413, "inbound_body_too_large", describeInboundBodyRefusal(err)); } console.warn(`[${label}] request body decode/parse failed: ${err instanceof Error ? `${err.name}: ${err.message}` : String(err)}`); return formatErrorResponse(400, "invalid_request_error", "Invalid JSON body"); @@ -3231,7 +3237,7 @@ async function handleResponsesInner( const agentTaskRecovery = agentTaskRecoveryConfig(config); let body: unknown; try { - body = await readJsonRequestBody(req, translatorBudget); + body = await readJsonRequestBody(req, translatorBudget, resolveInboundBodyLimitBytes(config.maxInboundBodyBytes)); } catch (err) { if (options.abortSignal?.aborted || req.signal.aborted) { return clientCancelledResponse(); diff --git a/src/server/responses/policy-fallback.ts b/src/server/responses/policy-fallback.ts index a6f1d425d8..13c90bdcee 100644 --- a/src/server/responses/policy-fallback.ts +++ b/src/server/responses/policy-fallback.ts @@ -1,6 +1,6 @@ import { comboFailureDecision } from "../../combos/failover"; import { readBoundedResponseBody } from "../../lib/bounded-body"; -import { readJsonRequestBody } from "../request-decompress"; +import { readJsonRequestBody, resolveInboundBodyLimitBytes } from "../request-decompress"; import { finishRequestAttempt, type RequestLogContext } from "../request-log"; import type { OcxConfig } from "../../types"; import type { RouteCandidateTrace, RouteDecisionTraceV1 } from "../../routing/trace"; @@ -145,7 +145,11 @@ export async function handleResponsesWithPolicyFallback( }; let rawBody: Record | null = null; try { - const parsed = await readJsonRequestBody(req.clone()); + const parsed = await readJsonRequestBody( + req.clone(), + undefined, + resolveInboundBodyLimitBytes(config.maxInboundBodyBytes), + ); if (parsed && typeof parsed === "object" && !Array.isArray(parsed)) rawBody = parsed as Record; } catch { // Core owns the client-facing parse/decompression error. diff --git a/src/server/search.ts b/src/server/search.ts index e09d15fc25..808bcd86c6 100644 --- a/src/server/search.ts +++ b/src/server/search.ts @@ -33,7 +33,7 @@ import { type ExactOpenAiSidecarAccount, } from "../providers/openai-sidecar"; import { routeModel } from "../router"; -import { readJsonRequestBody } from "./request-decompress"; +import { readJsonRequestBody, resolveInboundBodyLimitBytes } from "./request-decompress"; import { ForwardAdmissionCredentialError, validateForwardAdmissionCredential } from "./auth-cors"; import type { RequestLogContext } from "./request-log"; import { codexLogAccountId, decodeRequestErrorResponse } from "./responses"; @@ -64,7 +64,7 @@ export async function handleSearch( } let body: unknown; try { - body = await readJsonRequestBody(req); + body = await readJsonRequestBody(req, undefined, resolveInboundBodyLimitBytes(config.maxInboundBodyBytes)); } catch (err) { return decodeRequestErrorResponse(err, "search"); } diff --git a/src/types/config.ts b/src/types/config.ts index 7baadbfadf..0437cb42b4 100644 --- a/src/types/config.ts +++ b/src/types/config.ts @@ -801,6 +801,20 @@ export interface OcxConfig { * that work today — on Azure and custom Responses gateways as well, whose limits are unknown. */ maxUpstreamBodyBytes?: number; + /** + * Opt-in ceiling, in bytes, on a decompressed INBOUND data-plane request body (#3573). + * + * Omitted or 0 = the built-in 256 MiB default. The lever exists because a session on the + * 922k-token opt-in window serializes its full history past that default, and the request + * that crosses it is Codex's own remote-compaction request — so the session hits 413 on the + * one operation that would have shrunk it and cannot recover. + * + * Bounded on purpose. `resolveInboundBodyLimitBytes()` clamps to + * [1 MiB, 512 MiB]; an unbounded inbound cap is a memory DoS because the reader materializes + * the body several times over. The Bun listener's own `maxRequestBodySize` is fixed when the + * server starts, so raising this takes effect on restart. + */ + maxInboundBodyBytes?: number; /** * Opt-in Anthropic OAuth PROACTIVE routing (#294). Default OFF. * Sticky session affinity; new sessions may pick lowest known 5h usage. diff --git a/tests/server/server-request-body-size.test.ts b/tests/server/server-request-body-size.test.ts index 14b37c0f47..26dec9a691 100644 --- a/tests/server/server-request-body-size.test.ts +++ b/tests/server/server-request-body-size.test.ts @@ -1,6 +1,7 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { existsSync, mkdirSync} from "node:fs"; import { join } from "node:path"; +import { getDefaultConfig, saveConfig } from "../../src/config"; import { startServer } from "../../src/server"; import { MAX_DECOMPRESSED_BODY_BYTES } from "../../src/server/request-decompress"; import { installIsolatedCodexHome, type IsolatedCodexHome } from "../helpers/isolated-codex-home"; @@ -51,3 +52,52 @@ describe("server maxRequestBodySize (Issue #1601)", () => { } }); }); + +describe("configurable listener body size (Issue #3573)", () => { + const BODY_BYTES = 2 * 1024 * 1024; + + // Bun refuses an oversized body BEFORE fetch() runs, so a listener pinned to the 256 MiB + // default would silently cap the opt-in no matter what the handlers do with it. Proving the + // listener moved is cheaper downward than upward: the same 2 MiB body is admitted under the + // default and refused under a 1 MiB configured limit. + async function postFixedBody(port: number): Promise<{ refused: boolean; status: number | null }> { + // Bun answers 413 and stops reading while the client is still uploading, so the write side + // can surface the refusal as a transport error instead of a response. Both shapes mean the + // listener refused the body; neither can be produced by admitting it. + const res = await fetch(`http://127.0.0.1:${port}/v1/responses`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: Buffer.alloc(BODY_BYTES, 0x20), + }).catch(() => null); + if (!res) return { refused: true, status: null }; + const status = res.status; + await res.text().catch(() => ""); + return { refused: status === 413, status }; + } + + test("a body under the configured limit still reaches the handler", async () => { + saveConfig({ ...getDefaultConfig(), maxInboundBodyBytes: 8 * 1024 * 1024 }); + const server = startServer(0); + try { + const result = await postFixedBody(server.port); + // Unparseable JSON, so the handler answers 4xx — the point is that it answered at all. + expect(result.refused).toBe(false); + expect(result.status).not.toBeNull(); + } finally { + void server.stop(true); + } + }); + + test("the listener refuses above maxInboundBodyBytes instead of the fixed default", async () => { + // The old listener was pinned to MAX_DECOMPRESSED_BODY_BYTES, so this body reached the + // handler regardless of config. It must now be refused before the handler runs. + saveConfig({ ...getDefaultConfig(), maxInboundBodyBytes: 1024 * 1024 }); + expect(1024 * 1024).toBeLessThan(BODY_BYTES); + const server = startServer(0); + try { + expect((await postFixedBody(server.port)).refused).toBe(true); + } finally { + void server.stop(true); + } + }); +}); diff --git a/tests/usage/request-decompress.test.ts b/tests/usage/request-decompress.test.ts index 96a8f5a67a..cb88dd29e1 100644 --- a/tests/usage/request-decompress.test.ts +++ b/tests/usage/request-decompress.test.ts @@ -3,9 +3,13 @@ import { deflateRawSync, deflateSync } from "node:zlib"; import { DecompressedBodyTooLargeError, decodeRequestBody, + describeInboundBodyRefusal, MAX_DECOMPRESSED_BODY_BYTES, + MAX_CONFIGURABLE_INBOUND_BODY_BYTES, + MIN_CONFIGURABLE_INBOUND_BODY_BYTES, readBoundedJsonRequestBody, readJsonRequestBody, + resolveInboundBodyLimitBytes, UnsupportedContentEncodingError, } from "../../src/server/request-decompress"; import { MANAGEMENT_JSON_BODY_MAX_BYTES } from "../../src/server/management/body"; @@ -29,12 +33,15 @@ async function captureBodyTooLarge(run: () => unknown): Promise { expect(error.message).toBe(message); expect(message.length).toBeLessThan(200); + // The thrown message carries measurement provenance for the log; the client-facing message + // is the operator-directed one, and the two are deliberately not the same string (#3573). + const clientMessage = describeInboundBodyRefusal(error); for (const label of ["responses", "responses-compact"]) { const response = decodeRequestErrorResponse(error, label); expect(response.status).toBe(413); expect(response.headers.get("retry-after")).toBeNull(); expect(await response.json()).toEqual({ - error: { message, type: "invalid_request_error", code: "invalid_request_error" }, + error: { message: clientMessage, type: "invalid_request_error", code: "inbound_body_too_large" }, }); } } @@ -213,6 +220,100 @@ describe("decodeRequestBody", () => { }); }); +describe("configurable inbound body limit (Issue #3573)", () => { + test("an unconfigured proxy keeps the 256 MiB default", () => { + expect(resolveInboundBodyLimitBytes(undefined)).toBe(MAX_DECOMPRESSED_BODY_BYTES); + expect(resolveInboundBodyLimitBytes(0)).toBe(MAX_DECOMPRESSED_BODY_BYTES); + // A hand edit the schema degraded, or a config built without the schema at all. + expect(resolveInboundBodyLimitBytes(-1)).toBe(MAX_DECOMPRESSED_BODY_BYTES); + expect(resolveInboundBodyLimitBytes(Number.NaN)).toBe(MAX_DECOMPRESSED_BODY_BYTES); + expect(resolveInboundBodyLimitBytes(Number.POSITIVE_INFINITY)).toBe(MAX_DECOMPRESSED_BODY_BYTES); + }); + + test("the opt-in raises the limit for the 922k-context case", () => { + // The value #3573 asked for: 512 MiB, which is also the ceiling. + expect(resolveInboundBodyLimitBytes(512 * 1024 * 1024)).toBe(512 * 1024 * 1024); + expect(resolveInboundBodyLimitBytes(300 * 1024 * 1024)).toBe(300 * 1024 * 1024); + expect(resolveInboundBodyLimitBytes(300 * 1024 * 1024)).toBeGreaterThan(MAX_DECOMPRESSED_BODY_BYTES); + }); + + test("the ceiling is a hard bound, not a suggestion", () => { + // An unbounded inbound cap is a memory DoS: the reader materializes the body several + // times over, so no configured value may exceed the ceiling. + for (const requested of [ + MAX_CONFIGURABLE_INBOUND_BODY_BYTES + 1, + 4 * 1024 * 1024 * 1024, + Number.MAX_SAFE_INTEGER, + ]) { + expect(resolveInboundBodyLimitBytes(requested)).toBe(MAX_CONFIGURABLE_INBOUND_BODY_BYTES); + } + expect(MAX_CONFIGURABLE_INBOUND_BODY_BYTES).toBe(512 * 1024 * 1024); + }); + + test("a floor keeps a fat-fingered small value from refusing ordinary turns", () => { + expect(resolveInboundBodyLimitBytes(1)).toBe(MIN_CONFIGURABLE_INBOUND_BODY_BYTES); + expect(resolveInboundBodyLimitBytes(1024)).toBe(MIN_CONFIGURABLE_INBOUND_BODY_BYTES); + expect(resolveInboundBodyLimitBytes(1.9 * 1024 * 1024)).toBe(Math.floor(1.9 * 1024 * 1024)); + }); + + test("readJsonRequestBody admits and refuses against the resolved limit, not the default", async () => { + const body = JSON.stringify(PAYLOAD); + const request = () => new Request("http://localhost/v1/responses", { method: "POST", body }); + + expect(await readJsonRequestBody(request(), undefined, resolveInboundBodyLimitBytes(1024 * 1024))) + .toEqual(PAYLOAD); + + // Proves the limit is threaded through rather than ignored, without allocating 256 MiB. + const error = await captureBodyTooLarge(() => readJsonRequestBody(request(), undefined, 8)); + expect(error).toMatchObject({ limit: 8 }); + }); + + test("an inbound refusal is distinguishable from the upstream 413 of #4112", async () => { + const error = new DecompressedBodyTooLargeError(300 * 1024 * 1024, MAX_DECOMPRESSED_BODY_BYTES, "declared_wire"); + const response = decodeRequestErrorResponse(error, "responses"); + expect(response.status).toBe(413); + const payload = await response.json() as { error: { message: string; code: string } }; + // #4112 classifies the UPSTREAM 413 on this same surface as context_length_exceeded. + expect(payload.error.code).toBe("inbound_body_too_large"); + expect(payload.error.code).not.toBe("context_length_exceeded"); + // The diagnostic has to say whose limit it is and which key moves it, or the operator + // cannot tell the two 413s apart or find the lever. + expect(payload.error.message).toContain("maxInboundBodyBytes"); + expect(payload.error.message).toContain("local proxy limit"); + expect(payload.error.message).toContain("300.0 MB"); + expect(payload.error.message).toContain("256.0 MB"); + }); + + test("a lower-bound measurement is not reported as an exact size", () => { + const exact = new DecompressedBodyTooLargeError(600, 500, "decoded_exact"); + expect(describeInboundBodyRefusal(exact)).not.toContain("at least"); + for (const measurement of ["observed_wire_lower_bound", "decoded_lower_bound"] as const) { + const lower = new DecompressedBodyTooLargeError(600, 500, measurement); + expect(describeInboundBodyRefusal(lower)).toContain("at least"); + } + }); + + test("non-finite and untyped inputs stay out of the client-facing diagnostic", () => { + // Same rule the thrown message already follows: legacy callers can supply anything. + for (const bytes of [Number.NaN, Infinity, -Infinity, -1, Number.MAX_VALUE]) { + const message = describeInboundBodyRefusal(new DecompressedBodyTooLargeError(bytes, 500, "declared_wire")); + expect(message).not.toContain("NaN"); + expect(message).not.toContain("Infinity"); + expect(message).toContain("maxInboundBodyBytes"); + } + for (const limit of [Number.NaN, Infinity, -Infinity]) { + const message = describeInboundBodyRefusal(new DecompressedBodyTooLargeError(600, limit, "declared_wire")); + expect(message).not.toContain("NaN"); + expect(message).not.toContain("Infinity"); + expect(message).toContain("inbound admission limit"); + } + const untyped: DecompressedBodyTooLargeError = Reflect.construct(DecompressedBodyTooLargeError, [ + 600, 500, "private-header-context window".repeat(100), + ]); + expect(describeInboundBodyRefusal(untyped)).not.toContain("private-header"); + }); +}); + describe("readJsonRequestBody", () => { test("reports a compressed declaration without reading or echoing request metadata", async () => { const { body, stats } = trackedBodyStream([Bun.gzipSync(PAYLOAD_BYTES)]);