From c1224369ae385fbddc88045150c5e01b2f6506c5 Mon Sep 17 00:00:00 2001 From: JUN Date: Thu, 10 Sep 2026 00:02:39 +0900 Subject: [PATCH 1/2] feat(proxy): make the inbound body admission limit configurable MAX_DECOMPRESSED_BODY_BYTES was hard-coded at 256 MiB, and on the 922k-token opt-in window a session's serialized history crosses it. The request that crosses it is Codex's own remote-compaction request, so the session 413s on the one operation that would have shrunk it and cannot recover. Adds the opt-in `maxInboundBodyBytes`, shaped like `maxUpstreamBodyBytes`: omitted or 0 keeps today's 256 MiB. Every reader resolves through resolveInboundBodyLimitBytes(), which clamps to [1 MiB, 512 MiB]. The ceiling is mandatory rather than advisory: readBoundedJsonRequestBody materializes the body several times over, so peak memory is a multiple of whatever is admitted and an unbounded inbound cap would be a memory exhaustion lever. The schema keeps the outbound guard's `.catch(undefined)` degradation, which is exactly why the bound cannot live there. Bun's listener rejects an oversized body before fetch() runs, so maxRequestBodySize is now resolved from config at bind time instead of being pinned to the default; an out-of-range value is clamped with one startup warning. Also separates the two 413s a client can now see on this surface. #4112 gives an upstream size refusal `context_length_exceeded`; a local admission refusal now answers `inbound_body_too_large` with a message that names OpenCodex as the refuser, the observed and configured sizes, and the config key that moves the limit. The diagnostic inherits the thrown error's rule about keeping non-finite and untyped values out of the text. --- .../docs/reference/configuration/providers.md | 1 + src/config.ts | 9 ++ src/lib/errors.ts | 8 ++ src/server/chat-completions.ts | 8 +- src/server/claude-messages.ts | 10 +- src/server/images.ts | 4 +- src/server/index.ts | 27 ++++- src/server/request-decompress.ts | 94 +++++++++++++++- src/server/responses/compact.ts | 9 +- src/server/responses/core.ts | 12 +- src/server/responses/policy-fallback.ts | 8 +- src/server/search.ts | 4 +- src/types/config.ts | 14 +++ tests/server/server-request-body-size.test.ts | 23 ++++ tests/usage/request-decompress.test.ts | 103 +++++++++++++++++- 15 files changed, 308 insertions(+), 26 deletions(-) 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..36e2097f33 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,25 @@ describe("server maxRequestBodySize (Issue #1601)", () => { } }); }); + +describe("configurable listener body size (Issue #3573)", () => { + test("the Bun listener follows maxInboundBodyBytes instead of the fixed default", async () => { + // Bun refuses an oversized body BEFORE fetch() runs, so a listener pinned to the 256 MiB + // default would silently cap the opt-in. Proving the listener moved is cheaper downward: + // a 2 MiB configured limit rejects a 3 MiB body, which the old fixed listener admitted + // (it reached the handler and answered 400 for unparseable JSON). + saveConfig({ ...getDefaultConfig(), maxInboundBodyBytes: 2 * 1024 * 1024 }); + const server = startServer(0); + try { + const res = await fetch(`http://127.0.0.1:${server.port}/v1/responses`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: Buffer.alloc(3 * 1024 * 1024, 0x20), + }); + expect(res.status).toBe(413); + await res.text(); + } 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)]); From 92e6f5a9934bf4b5e10317c46cb0f0d96c66fefc Mon Sep 17 00:00:00 2001 From: JUN Date: Thu, 10 Sep 2026 00:10:36 +0900 Subject: [PATCH 2/2] test(server): make the listener-limit regression robust to Bun's refusal shape Bun answers 413 and stops reading while the client is still uploading, so the write side can surface the refusal as a transport error rather than a response. Accept either shape for the refusal, and pair it with the admitted case at the same body size so the assertion stays non-vacuous: the old listener, pinned to MAX_DECOMPRESSED_BODY_BYTES, admitted this body under every configuration. --- tests/server/server-request-body-size.test.ts | 53 ++++++++++++++----- 1 file changed, 40 insertions(+), 13 deletions(-) diff --git a/tests/server/server-request-body-size.test.ts b/tests/server/server-request-body-size.test.ts index 36e2097f33..26dec9a691 100644 --- a/tests/server/server-request-body-size.test.ts +++ b/tests/server/server-request-body-size.test.ts @@ -54,21 +54,48 @@ describe("server maxRequestBodySize (Issue #1601)", () => { }); describe("configurable listener body size (Issue #3573)", () => { - test("the Bun listener follows maxInboundBodyBytes instead of the fixed default", async () => { - // Bun refuses an oversized body BEFORE fetch() runs, so a listener pinned to the 256 MiB - // default would silently cap the opt-in. Proving the listener moved is cheaper downward: - // a 2 MiB configured limit rejects a 3 MiB body, which the old fixed listener admitted - // (it reached the handler and answered 400 for unparseable JSON). - saveConfig({ ...getDefaultConfig(), maxInboundBodyBytes: 2 * 1024 * 1024 }); + 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 res = await fetch(`http://127.0.0.1:${server.port}/v1/responses`, { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: Buffer.alloc(3 * 1024 * 1024, 0x20), - }); - expect(res.status).toBe(413); - await res.text(); + 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); }