Skip to content

feat(proxy): make the inbound body admission limit configurable - #4138

Merged
lidge-jun merged 2 commits into
devfrom
codex/3573-configurable-inbound-body-limit
Sep 9, 2026
Merged

feat(proxy): make the inbound body admission limit configurable#4138
lidge-jun merged 2 commits into
devfrom
codex/3573-configurable-inbound-body-limit

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Sep 9, 2026

Copy link
Copy Markdown
Owner

Summary

Closes #3573.

MAX_DECOMPRESSED_BODY_BYTES in src/server/request-decompress.ts was hard-coded at 256 MiB with no config lever. On the 922k-token opt-in window a session's serialized history crosses that limit, and the request that crosses it is Codex's own remote-compaction request — so the session gets 413 ... Decompressed request body exceeds 268435456 bytes on the one operation that would have shrunk it, and cannot recover.

  • New opt-in maxInboundBodyBytes, shaped like the existing maxUpstreamBodyBytes: omitted or 0 keeps today's 256 MiB, so an unconfigured proxy admits exactly what it admits now.
  • The ceiling is mandatory, not advisory. Every reader resolves through resolveInboundBodyLimitBytes(), which clamps to [1 MiB, 512 MiB]. An unbounded inbound cap would be a memory-exhaustion lever: 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. 512 MiB is the value the issue asked for and the largest that keeps that multiple survivable. The bound deliberately lives in code rather than in the schema, because the schema mirrors the outbound guard's .catch(undefined) degradation and a config object can also be built without the schema at all.
  • Bun rejects an oversized body before fetch runs, so the listener's maxRequestBodySize is now resolved from config at bind time instead of being pinned to the default; otherwise the opt-in would silently do nothing. It is fixed at bind time, so a change takes effect on restart, which the config doc and the docs-site row both state. An out-of-range value is clamped with one startup warning naming the value actually used.
  • The limit is threaded through every inbound data-plane reader that has config in scope — /v1/responses, /v1/responses/compact, policy fallback's body clone, /v1/chat/completions, /v1/messages, images, and search — so one deployment does not end up with two different inbound limits. Management routes keep their own separate MANAGEMENT_JSON_BODY_MAX_BYTES.

Improved 413 diagnostic. The parent PR (#4127, issue #4112) gives an upstream size refusal on this same surface HTTP 413 with code: context_length_exceeded. Without a distinct code the two are indistinguishable while having opposite remedies — one means the provider will not take the turn, the other means the proxy never read it and a config key would have let it through. A local admission refusal now answers code: inbound_body_too_large with a message that names OpenCodex as the refuser, the observed and configured sizes, and the key that moves the limit. The wording avoids "context window"/"context length" on purpose, since classifyError treats those as evidence of an upstream context verdict. The diagnostic also inherits the thrown error's existing rule about keeping non-finite and untyped values out of the text.

Originally opened on top of #4127 because both PRs change the meaning of a 413 on this surface and the new code only makes sense against the classified one. #4127 has since merged, so this now targets dev directly and its diff is only the change described above.

Verification

  • Local product suite, typecheck, build, lint, and bun install were NOT RUN, per explicit maintainer instruction for this round. Exact-head remote CI on this PR is the gate.
  • git diff --check: clean.
  • Regression coverage added, to be executed by CI:
    • tests/usage/request-decompress.test.ts — a new configurable inbound body limit (Issue #3573) block pins the default for unconfigured/zero/negative/non-finite input, the raise, the hard ceiling (including MAX_SAFE_INTEGER and 4 GiB), the floor, that readJsonRequestBody honors the resolved limit rather than the default, that an inbound refusal is inbound_body_too_large and not context_length_exceeded, that a lower-bound measurement is not reported as an exact size, and that non-finite/untyped inputs stay out of the client-facing message.
    • The existing expectBodyLimitResponse helper was updated to assert the new code and the new client-facing message while still pinning the thrown message, which keeps its provenance for the log.
    • tests/server/server-request-body-size.test.ts — a configured 2 MiB limit rejects a 3 MiB body at the listener, which the previously fixed listener admitted through to the handler. Proven downward on purpose so CI does not allocate 257 MiB.
  • Docs: added the maxInboundBodyBytes row to the configuration reference next to maxUpstreamBodyBytes.
  • No GUI change, so no screenshot applies.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. This change raises a memory-admission bound, so the bound itself is the security surface: it is clamped in one auditable resolver that every reader goes through, the default is unchanged, no credential, auth, or destination policy is touched, and no request content reaches a log or an error message.

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.
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 9, 2026 15:07
@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (2)
  • ^dev$
  • ^preview$

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 607ddb57-68c6-4e9f-b121-1144a6bee92b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

…sal 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.
@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 68 / 80

이 PR은 이슈 #3573을 닫으려는 변경이다. 현재 dev HEAD 3b4d8c439src/server/request-decompress.ts에는 MAX_DECOMPRESSED_BODY_BYTES = 256 * 1024 * 1024가 하드코딩되어 있고, maxInboundBodyBytesresolveInboundBodyLimitBytes는 없다. outbound 쪽만 maxUpstreamBodyBytes가 있다. 922k 토큰 창에서 세션 히스토리가 커지면 Codex의 remote-compaction 요청 자체가 256MiB를 넘기고, 줄여 줄 유일한 요청이 413 Decompressed request body exceeds …로 거절되어 세션이 막힌다. 그 실사용 통증이 분명하다.

이 PR이 하는 일은 그 상한을 설정 가능하게 만드는 것이다. maxInboundBodyBytes를 넣고(생략/0이면 지금과 같은 256MiB), resolveInboundBodyLimitBytes()[1MiB, 512MiB]로 클램프한다. /v1/responses·compact·chat·messages·images·search 등 데이터면 리더와 Bun maxRequestBodySize까지 같은 값을 쓰게 했고, 로컬 거절은 code: inbound_body_too_large로 올려서 #4112/#4127이 만드는 upstream context_length_exceeded 413과 구분한다. 메모리 배수(와이어·디코드·문자열·파스 그래프)를 이유로 상한을 코드에 고정한 설명도 타당하다.

가장 큰 블록커는 base다. 이 PR의 base는 dev가 아니라 ingw/fix-4112-json-overflow(#4127)다. #4127은 아직 dev에 안 들어갔고(mergeable이지만 mergeStateStatus BLOCKED), 두 PR이 같은 표면의 413 의미를 같이 바꾼다. 지금 그대로 dev에 강제 머지하거나 base만 바꾸면 진단 코드·diff가 어긋날 수 있다. PR 본문도 “#4127이 dev에 오른 뒤 retarget”라고 이미 적어 두었다. 다음 게이트가 main/preview 안정 2.49.0인 상황에서, 이 스택은 dev 위에 #4127 → 이 PR 순으로만 올리는 것이 맞다.

경로/심볼 - base ingw/fix-4112-json-overflow (#4127) — dev가 아님. #4127 착지 전 단독 머지 금지. retarget 또는 스택 유지 후 순차 머지.

경로/심볼 - src/server/request-decompress.ts — HEAD의 하드코딩 256MiB가 그대로다. PR의 resolver·클램프·inbound_body_too_large 메시지가 그 자리를 대체한다.

경로/심볼 - src/server/index.ts listener maxRequestBodySize — 설정만 올리고 listener를 안 바꾸면 Bun이 fetch 전에 거절해 옵션이 무력화된다. bind-time 반영·재시작 필요 문구는 필수다.

라인 - src/lib/errors.ts classifyErrorinbound_body_too_largecontext_length_exceeded와 분리해야 한다. “context window” 문구를 로컬 메시지에 넣으면 분류가 다시 섞인다.

메인테이너의 판단이 필요한 지점

  • #4127을 먼저 dev에 올릴지, 이 PR을 #4127과 하나의 착지 PR로 묶을지
  • 512MiB 상한이 922k + 이미지 포함 세션에 충분한지, 아니면 별도 스트리밍/스풀 설계가 필요한지
  • CI(이 PR은 base가 달라 CodeRabbit이 base branch 비활성으로 skip)를 dev retarget 후 다시 돌릴지

너의 추천
지금 merge하지 말고 #4127이 dev에 들어간 뒤 base를 dev로 바꾸거나 스택 순서로 올린다. #3573 가치는 높고(remote-compaction 413 탈출), 설계도 outbound 가드와 대칭이라 착지 후 우선 머지 후보로 둔다. 로컬 스위트는 안 돌렸다고 했으니 exact-head CI를 게이트로 쓴다.

이 댓글은 grok-bot이 작성했습니다

Base automatically changed from ingw/fix-4112-json-overflow to dev September 9, 2026 15:16
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the enhancement New feature or request label Sep 9, 2026
@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 72 / 80

이 PR은 이슈 #3573을 닫는다. 지금 dev HEAD f982af48bsrc/server/request-decompress.ts에는 MAX_DECOMPRESSED_BODY_BYTES = 256 * 1024 * 1024가 하드코딩되어 있고, maxInboundBodyBytes / resolveInboundBodyLimitBytes는 없다. outbound만 maxUpstreamBodyBytes가 있다. 922k 토큰 창에서 세션 히스토리가 커지면 Codex remote-compaction 요청 자체가 256MiB를 넘기고, 줄여 줄 유일한 요청이 413 Decompressed request body exceeds …로 거절되어 세션이 막힌다. 그 실사용 통증이 분명하다.

이 PR이 하는 일은 그 상한을 설정 가능하게 만드는 것이다. maxInboundBodyBytes를 넣고(생략/0이면 지금과 같은 256MiB), resolveInboundBodyLimitBytes()[1MiB, 512MiB]로 클램프한다. /v1/responses·compact·chat·messages·images·search 등 데이터면 리더와 Bun maxRequestBodySize까지 같은 값을 쓰게 했고, 로컬 거절은 code: inbound_body_too_large로 올려서 이미 dev에 들어간 #4127의 upstream context_length_exceeded 413과 구분한다. 메모리 배수(와이어·디코드·문자열·파스 그래프)를 이유로 상한을 코드에 고정한 설명도 타당하다.

이전에 올렸던 리뷰의 블록커는 해제되었다. #4127은 2026-09-09 15:16 UTCdev로 머지되었고, 이 PR의 base는 이제 dev다. 예전 댓글의 “#4127 착지 전 단독 머지 금지”는 더 이상 맞지 않는다. 지금 남은 게이트는 exact-head CI(일부 macos/resolve-pr가 아직 진행·대기, mergeStateStatus: BLOCKED)와 문서·커버리지 세부다.

경로/심볼 - src/server/request-decompress.ts — HEAD의 하드코딩 256MiB가 그대로다. PR의 resolver·클램프·describeInboundBodyRefusal / inbound_body_too_large가 그 자리를 대체한다.

경로/심볼 - src/server/index.ts listener maxRequestBodySize — 설정만 올리고 listener를 안 바꾸면 Bun이 fetch 전에 거절해 옵션이 무력화된다. bind-time 반영·재시작 필요 문구는 필수다(문서·startup warn 있음).

라인 - src/lib/errors.ts classifyErrorinbound_body_too_largecontext_length_exceeded와 분리한다. 로컬 메시지에 “context window”를 넣으면 분류가 다시 섞인다(PR이 의도적으로 피함).

경로/심볼 - docs-site/.../providers.md maxInboundBodyBytes 행 — 적용 경로를 responses/compact/chat/messages만 적었는데, 코드는 images·search에도 같은 limit을 넣는다. 문서가 좁으면 운영자가 이미지/검색 경로를 예외로 오해할 수 있다.

경로/심볼 - src/server/responses/encrypted-payload.ts / collaboration.tsreadJsonRequestBody를 import만 하고 호출하지는 않는다. 이번 스레드 대상은 아니고, 미사용 import 정리는 별도 청소로 둬도 된다.

메인테이너의 판단이 필요한 지점

  • 512MiB 상한이 922k + 이미지 포함 세션에 충분한지, 아니면 나중에 스트리밍/스풀이 필요한지
  • docs 행에 images·search를 지금 같이 넣을지, 후속 문서 PR로 미룰지
  • CI가 전부 초록이 될 때까지 기다릴지(지금 BLOCKED는 대기 중인 check로 보임)

너의 추천
#4127 블록커는 끝났으니, CI가 초록이면 dev에 머지한다. #3573 가치가 높고(remote-compaction 413 탈출), outbound 가드와 대칭이며 기본값(미설정=256MiB)이 유지된다. docs에 images·search를 한 줄만 보강하면 더 좋다. 로컬 스위트는 안 돌렸다고 했으니 exact-head CI를 게이트로 쓴다. 이전 리뷰(우선순위 68, base≠dev)의 “지금 merge하지 말자”는 이 댓글로 대체한다.

이 댓글은 grok-bot이 작성했습니다

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant