From e963aa6bde54522c883b57da0dfd073bc4022cfa Mon Sep 17 00:00:00 2001 From: t Date: Mon, 7 Sep 2026 07:35:45 +0900 Subject: [PATCH 1/3] docs: plan platform validation follow-up Record scoped roadmap and independent plan audit. Local suites are excluded by owner instruction; final remote CI will verify the combined candidate. --- .../260907_platform_validation/000_plan.md | 31 +++++++++++++++++++ .../001_plan_audit.md | 5 +++ .../010_oauth_teardown.md | 14 +++++++++ .../020_container_smoke.md | 25 +++++++++++++++ .../030_container_ci.md | 21 +++++++++++++ .../035_body_diagnostics.md | 17 ++++++++++ .../040_residual_evidence.md | 15 +++++++++ 7 files changed, 128 insertions(+) create mode 100644 devlog/_plan/260907_platform_validation/000_plan.md create mode 100644 devlog/_plan/260907_platform_validation/001_plan_audit.md create mode 100644 devlog/_plan/260907_platform_validation/010_oauth_teardown.md create mode 100644 devlog/_plan/260907_platform_validation/020_container_smoke.md create mode 100644 devlog/_plan/260907_platform_validation/030_container_ci.md create mode 100644 devlog/_plan/260907_platform_validation/035_body_diagnostics.md create mode 100644 devlog/_plan/260907_platform_validation/040_residual_evidence.md diff --git a/devlog/_plan/260907_platform_validation/000_plan.md b/devlog/_plan/260907_platform_validation/000_plan.md new file mode 100644 index 0000000000..d1b3cb632f --- /dev/null +++ b/devlog/_plan/260907_platform_validation/000_plan.md @@ -0,0 +1,31 @@ +# Platform verification follow-up + +Baseline: dev `137d6a7270e7ecfb1c791993800a17c0e30022d9` (2026-09-07). + +## Objective and authority + +Satisfy the existing platform contracts for #3383, #3449, #3522 and #3573. The owner requested ordinary manual PRs, top-of-stack CI first, lower-layer CI only to diagnose a failed final run, no local test suites, push with --no-verify, admin merge after verification, and original contributor credit in commit trailers. No native GitHub stack registration. No publish, release, global settings changes, admission-limit increases, ACL relaxation, or speculative recovery policy. + +The initial assigned checkout contains unrelated dirty work and is preserved. Work lives in an isolated worktree. No SessionStart FSM binding is available in the supplied context; this record documents the work without claiming automatic loop continuation is armed. + +## Evidence and scope + +Dockerfile, compose.yaml, docker/bootstrap-token.ts and the source-build guide already exist. Cross-platform CI has no real image build/start/recreate check. #3522 requires same-process Windows recovery evidence; #3573 requires actual rejected compact-byte evidence. Existing diagnostics must be checked before adding anything. PR #3383 is a mixed historical source: only Windows temp/teardown residuals are in scope, not picker controls. + +Original Docker contributor: Buseong Kim , verified from original #3421 commit metadata. Carry this identity in commit trailers. + +## Dependency map + +1. `010_oauth_teardown.md`: drain the asynchronous ACL fixture before deletion. +2. `020_container_smoke.md`: executable isolated container acceptance probe. +3. `030_container_ci.md`: CI consumes that probe and gates its result. +4. `035_body_diagnostics.md`: distinguish declared size, observed lower bound, and decoded size without changing admission. +5. `040_residual_evidence.md`: settle the Windows/spill/compact residuals; implement only a proven narrow gap through a plan amendment, otherwise preserve open status. + +The manual review chain contains the independent OAuth fixture carry, bounded body diagnostics, the container probe, then its dependent CI integration. Independent code is prepared in disjoint files; the top CI validates their combined tree. Existing workflow triggers remain honest: final branch workflow_dispatch supplies the complete integration result; lower PR runs are not represented as passed if skipped/cancelled. Every implemented layer is reviewed, and final head is pinned before CI. After successful final CI, merge bottom-up using merge commits so reviewed commit ancestry survives. Revalidate the resulting integration and distinguish unrelated concurrent dev changes. + +## Verification and completion + +Local suites and typecheck are NOT RUN by owner instruction. Syntax and read-only diff checks are allowed. The real verifier is GitHub Cross-platform CI on the final branch, including the new Docker job. A failed final run is diagnosed on the smallest affected scope; do not repeatedly run passing gates. Independent Astra high review covers functionality and workflow/security boundaries. Security working notes remain in scratch, not this public unit. + +Completion means verified deliverable PRs merged with commit attribution, plus explicit no-op/blocked disposition for unavailable field evidence. It does not mean every original issue is fixed. New product/security policy choices remain outside scope. Evidence and final outcome are appended to this unit; workflow run URLs and SHAs are preserved. diff --git a/devlog/_plan/260907_platform_validation/001_plan_audit.md b/devlog/_plan/260907_platform_validation/001_plan_audit.md new file mode 100644 index 0000000000..96ac86c266 --- /dev/null +++ b/devlog/_plan/260907_platform_validation/001_plan_audit.md @@ -0,0 +1,5 @@ +# Plan audit disposition + +Independent Astra high reviewer: NEAR-PASS. OAuth teardown and bounded body diagnostics passed within scope. Three Docker/CI conditions were incorporated before implementation: explicit final lane=all executed-job inventory; isolated project/image/port and bounded cleanup; concrete readiness/admission/catalog/persistence checks before and after actual replacement. + +Main judgment: pass with those amendments. Scope remains unchanged: existing Docker contract verification, test-fixture teardown, bounded diagnostics. Live spill recovery and exact historical compact-body proof remain deferred. No local suites or typecheck were run. diff --git a/devlog/_plan/260907_platform_validation/010_oauth_teardown.md b/devlog/_plan/260907_platform_validation/010_oauth_teardown.md new file mode 100644 index 0000000000..d4ee405020 --- /dev/null +++ b/devlog/_plan/260907_platform_validation/010_oauth_teardown.md @@ -0,0 +1,14 @@ +# OAuth fixture teardown carry + +Original source: #3383 commit 51726d2c7c58146defdd6088aefa2b95a1e58553. +Original contributor: x3M3x (Git commit metadata). + +## Concrete delta + +MODIFY `tests/oauth/oauth-store-multi.test.ts` only: import flushConfigDirHardeningForTests and the async ICACLS test runner; stub synchronous and asynchronous runners consistently in setup. Change teardown to await the tracked hardening work before resetting runners/caches, restoring OPENCODEX_HOME, or removing the fixture. Preserve removeTreeWithRetry and all production semantics. Add a deterministic held-async-runner regression against the actual cleanup routine if the existing fixture seams allow it without a new production test API. + +Production path proof: store reads call hardenConfigDir; config/paths tracks asynchronous directory hardening; resetHardenedStateForTests clears caches but does not drain those jobs. Deletion retries alone do not ensure ordering. The prior carry #3258 only replaced the removal function. + +## Acceptance + +No real asynchronous ICACLS escapes the fixture runner. Cleanup waits while a controlled ACL flight is unresolved and only deletes/restores environment after completion. The same OAuth test file passes in final Linux/macOS/Windows CI. Local tests/typecheck are NOT RUN by owner instruction. No numeric-open-flags change is included without current Bun reproduction. No new API/auth policy, credentials, or production runtime change. diff --git a/devlog/_plan/260907_platform_validation/020_container_smoke.md b/devlog/_plan/260907_platform_validation/020_container_smoke.md new file mode 100644 index 0000000000..17d7139d35 --- /dev/null +++ b/devlog/_plan/260907_platform_validation/020_container_smoke.md @@ -0,0 +1,25 @@ +# Container smoke executable + +## File delta + +NEW `scripts/ci/docker-smoke.ts`: bounded Bun-native TypeScript probe for the existing source-build Compose contract. Reuse the canonical compatibility generator and docker/bootstrap-token.ts; do not add an alternative token writer or deployment configuration. The probe creates a unique temporary Compose project and image, builds the actual Dockerfile, bootstraps a freshly generated throwaway token through stdin, starts the hub, verifies health and data-plane admission, recreates the container on the same named volumes, and verifies persistent state again. Cleanup is limited to the unique test project and its generated artifacts. Never use an operator project, host home, provider credentials, global docker prune, or real upstream inference. + +MODIFY owning documentation only as needed to explain the CI acceptance scope and its limits; no claim of upstream-provider validation. + +## Acceptance + +- Real image builds from the checkout with a generated compatibility manifest. +- Read-only/non-root Compose service becomes healthy; requests without a token are refused. +- A synthetic catalog in the separate Codex volume is served with the throwaway token, proving admission and persistence without provider access. +- /readyz succeeds separately from liveness, token reinitialization fails without replacement, and effective container restrictions are verified. +- Token/config/catalog persist across an actual container replacement (different container id, same volumes). +- Failures and cleanup are bounded; token/body contents never appear in logs. +- Existing Docker settings and defaults remain unchanged. + +Run only in final remote CI. Locally perform source/static inspection, not the smoke or a test suite. Read the current lifecycle/API contracts before implementing assertions. + +## Audit amendments + +Use explicit unique project on every Compose command, unique image tag via a temporary override, controlled Compose environment, and loopback ephemeral host port. Preserve pre-existing generated files; cleanup must fail the probe if it cannot remove its own project resources. Bound every child, output capture and cleanup; terminate/reap timed-out children. Never print raw runtime logs or complete inspect output. + +Before/after replacement: require readyz 200 with status ready; authenticated catalog 200 with exact synthetic fixture; missing/wrong token 401 for catalog, Responses and compact. Second bootstrap must fail and preserve the original token while rejecting the proposed replacement. Verify different container IDs, identical named-volume identities and persistent config/catalog evidence without reseeding; check effective non-root UID and read-only root. diff --git a/devlog/_plan/260907_platform_validation/030_container_ci.md b/devlog/_plan/260907_platform_validation/030_container_ci.md new file mode 100644 index 0000000000..28f3e08fc9 --- /dev/null +++ b/devlog/_plan/260907_platform_validation/030_container_ci.md @@ -0,0 +1,21 @@ +# Container CI integration + +Depends on the committed probe from phase 1. + +## File delta + +MODIFY `.github/workflows/ci.yml`: include Dockerfile, compose.yaml, .dockerignore and docker/** in relevant scope detection; add an ubuntu-latest Docker smoke job using the existing pinned checkout and setup-project-bun action; invoke the script after installing required project dependencies if the generator needs them. Preserve read-only workflow permissions and persist-credentials false. Add the job to aggregate ci needs so failures cannot silently pass. No registry publishing, credentials, native stack integration or changes to existing suite retry/concurrency policy. + +MODIFY `tests/ci-workflows/ci-workflows.test.ts`: extend the existing source-oracle checks for scope paths, direct aggregate dependency, pinned actions, and actual probe invocation. Keep existing domain/layout registration unchanged by using the owning test file. + +MODIFY `docs-site/src/content/docs/guides/remote-hub.md`: describe image lifecycle validation and separate readiness/provider-auth limitations. + +## Acceptance and verifier + +Final-branch Cross-platform CI workflow_dispatch must run the smoke and the existing platform gates. The Docker job's failures must reach ci. Local suite/typecheck NOT RUN per owner. Independent review checks full workflow event, permission, input, credential, and cleanup boundaries before publishing. Existing source-oracle tests execute remotely in CI. + +Publish branches with --no-verify; do not claim lower-layer CI if only the final tree was tested. Final failure permits narrower runs. User authorized admin merge of verified layers; original author names/emails come from source commit metadata and are included as Co-authored-by trailers. + +## Final execution inventory + +Dispatch existing Cross-platform CI with lane=all on the immutable final head. Record each expected job and actual conclusion: Docker, four Linux shards, storage-policy, api-usage, gates, two macOS shards, macos-control, six Windows shards, keyring jobs, any selected npm packaging jobs, and ci. Aggregate green alone does not prove Windows or Docker ran. Explain legitimate scope skips instead of counting them as tests. diff --git a/devlog/_plan/260907_platform_validation/035_body_diagnostics.md b/devlog/_plan/260907_platform_validation/035_body_diagnostics.md new file mode 100644 index 0000000000..5165526583 --- /dev/null +++ b/devlog/_plan/260907_platform_validation/035_body_diagnostics.md @@ -0,0 +1,17 @@ +# Bounded inbound-body diagnostic semantics + +Issue #3573 requests usable size evidence. The existing error stores a byte value but returns only the admission limit; the byte value currently mixes declared length, observed wire bytes, an artificial limit+1 lower bound, and exact decoded length. + +## File delta + +MODIFY `src/server/request-decompress.ts`: extend DecompressedBodyTooLargeError with a closed measurement category and retained limit, preserving existing constructor call compatibility. Annotate existing throw sites: declared_wire, observed_wire_lower_bound, decoded_exact, decoded_lower_bound. Append a bounded numeric/category suffix to the current message so existing core.ts error mapping carries it. No request body, path, headers, item counts, further inflate/read, admission-limit changes, or new retry semantics. + +MODIFY `tests/usage/request-decompress.test.ts`: extend small-cap fixtures to verify identity/gzip/zstd/deflate and declared/fragmented input semantics. In particular, limit+1 remains a lower bound, never exact size. Verify HTTP 413 and existing error code/type through existing handler mapping. Preserve stream cancellation. + +MODIFY `docs-site/src/content/docs/reference/proxy-formats.md`: explain wire declared length vs measured/lower-bound diagnostics, separately from compact-response limits. State that Bun listener rejection may happen before application diagnostics and that this does not measure the exact historical compact payload. + +## Acceptance + +Unchanged 256 MiB listener/decoder limit and rejection classification. No context-window wording that causes errors.ts to reclassify the failure. Message remains bounded, only fixed categories and finite numeric values. Negative tests run in final remote CI; no local test/typecheck. Keep #3573 open pending exact real compact evidence. + +This is a new diagnostic refinement of an issue, not a carry of a new contributor PR. Credit reporter @nowhere1975 in commit prose without inventing name/email. Any borrowed existing PR patches must additionally retain their actual git author trailers. diff --git a/devlog/_plan/260907_platform_validation/040_residual_evidence.md b/devlog/_plan/260907_platform_validation/040_residual_evidence.md new file mode 100644 index 0000000000..f5466a4d38 --- /dev/null +++ b/devlog/_plan/260907_platform_validation/040_residual_evidence.md @@ -0,0 +1,15 @@ +# Windows and request diagnostic residuals + +## Read-only targets + +- #3383: inspect current PR and merged descendants for Windows temp creation and OAuth teardown. Confirm current source behavior and test coverage before proposing a residual patch. No picker UI changes. +- #3522: inspect response spill telemetry and fresh-versus-memoized timeout handling. The acceptance is recovery within the same affected Windows process; generic synthetic success does not prove the reported process recovered. +- #3573: inspect decompression rejection diagnostics and exact latest issue measurements. Serialized journal size and normal requests after raising a cap do not prove the rejected compact payload size or compact success. + +## Conditional delta + +No production edit is pre-approved by this document without a source-grounded residual. If the existing code covers the measurement, record the missing field evidence and leave the issue open. If a specific content-free diagnostic is missing, amend with exact files, field flow and negative assertions before implementation. Never change admission caps, parse a rejected body to count items, relax ACLs, clear memo state, or choose a new recovery/retry policy. + +## Completion + +Record source/commit evidence, original contributor attribution where code is carried, and a separate status per candidate: already implemented, proven patch delivered, or blocked on field evidence. Do not close an original feature PR or issue merely because one residual probe passes. From 6a51f048e7e4eea7861f331f84dc241e61db035a Mon Sep 17 00:00:00 2001 From: t Date: Mon, 7 Sep 2026 07:38:22 +0900 Subject: [PATCH 2/3] test(oauth): drain ACL flights before fixture teardown [skip ci] Carry the remaining teardown fix from #3383 (51726d2c7) and verify cleanup ordering with a held asynchronous runner. Final combined CI will validate the stack; no local suite was run. Co-authored-by: x3M3x --- tests/oauth/oauth-store-multi.test.ts | 86 +++++++++++++++++++++++---- 1 file changed, 73 insertions(+), 13 deletions(-) diff --git a/tests/oauth/oauth-store-multi.test.ts b/tests/oauth/oauth-store-multi.test.ts index 6cd3f21dbe..02d8f8024f 100644 --- a/tests/oauth/oauth-store-multi.test.ts +++ b/tests/oauth/oauth-store-multi.test.ts @@ -4,10 +4,14 @@ import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import * as atomicWrite from "../../src/config/atomic-write"; import * as oauthStore from "../../src/oauth/store"; +import { flushConfigDirHardeningForTests } from "../../src/config/paths"; import { resetHardenedStateForTests, + setAsyncIcaclsRunnerForTests, setIcaclsRunnerForTests, + setPlatformForTests, } from "../../src/lib/windows-secret-acl"; +import { setSyntheticWindowsPrincipalForTests } from "../../src/lib/windows-user-principal"; import { getAccountCredential, getAccountSet, @@ -35,6 +39,17 @@ import { removeTreeWithRetry } from "../helpers/remove-tree"; const TEST_DIR = join(import.meta.dir, ".tmp-oauth-store-multi-test"); let previousOpencodexHome: string | undefined; +const ICACLS_OK = { success: true, exitCode: 0, timedOut: false, stdout: "" }; + +async function cleanupOAuthStoreFixture(): Promise { + await flushConfigDirHardeningForTests(); + setIcaclsRunnerForTests(null); + setAsyncIcaclsRunnerForTests(null); + resetHardenedStateForTests(); + if (previousOpencodexHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousOpencodexHome; + if (existsSync(TEST_DIR)) removeTreeWithRetry(TEST_DIR); +} const cred = (over: Partial = {}): OAuthCredentials => ({ access: "access-1", @@ -61,21 +76,66 @@ describe("multi-account auth store", () => { mkdirSync(TEST_DIR, { recursive: true }); process.env.OPENCODEX_HOME = TEST_DIR; resetHardenedStateForTests(); - setIcaclsRunnerForTests(() => ({ - success: true, - exitCode: 0, - timedOut: false, - stdout: "", - })); + setIcaclsRunnerForTests(() => ICACLS_OK); + setAsyncIcaclsRunnerForTests(async () => ICACLS_OK); }); - afterEach(() => { - setIcaclsRunnerForTests(null); - resetHardenedStateForTests(); - if (previousOpencodexHome === undefined) delete process.env.OPENCODEX_HOME; - else process.env.OPENCODEX_HOME = previousOpencodexHome; - if (existsSync(TEST_DIR)) removeTreeWithRetry(TEST_DIR); - }); + afterEach(cleanupOAuthStoreFixture); + + test("fixture cleanup waits for a held config-directory ACL flight before restoring home or deleting files", async () => { + let release!: () => void; + const held = new Promise(resolve => { release = resolve; }); + let markStarted!: () => void; + const started = new Promise(resolve => { markStarted = resolve; }); + let deadlineTimer: ReturnType | undefined; + let cleaning: Promise | undefined; + let cleanupSettled = false; + setPlatformForTests("win32"); + // Keep SID discovery hermetic on Windows as well as on forced POSIX lanes. + setSyntheticWindowsPrincipalForTests("*S-1-5-21-1-2-3-1001"); + setAsyncIcaclsRunnerForTests(async () => { + markStarted(); + await held; + return ICACLS_OK; + }); + try { + // A real store read starts the production-tracked directory hardening flight. + expect(getAccountSet("xai")).toBeNull(); + await Promise.race([ + started, + new Promise((_, reject) => { + deadlineTimer = setTimeout(() => reject(new Error("ACL runner did not start")), INTERNAL_DEADLINE_MS); + }), + ]); + clearTimeout(deadlineTimer); + cleaning = cleanupOAuthStoreFixture().then( + () => { cleanupSettled = true; return null; }, + (error: unknown) => { cleanupSettled = true; return error; }, + ); + // An event-loop checkpoint lets an incorrectly unawaited cleanup finish; no sleep oracle. + await new Promise(resolve => setImmediate(resolve)); + expect(cleanupSettled).toBe(false); + expect(process.env.OPENCODEX_HOME).toBe(TEST_DIR); + expect(existsSync(TEST_DIR)).toBe(true); + + release(); + expect(await cleaning).toBeNull(); + expect(cleanupSettled).toBe(true); + expect(process.env.OPENCODEX_HOME).toBe(previousOpencodexHome); + expect(existsSync(TEST_DIR)).toBe(false); + } finally { + if (deadlineTimer !== undefined) clearTimeout(deadlineTimer); + // Even a broken cleanup must not release the held flight into the real runner. + setAsyncIcaclsRunnerForTests(async () => ICACLS_OK); + release(); + try { + await cleaning; + await flushConfigDirHardeningForTests(); + } finally { + setPlatformForTests(null); + } + } + }, STORE_BUDGET_MS); test("legacy single-credential auth.json normalizes and round-trips without losing login", async () => { const authPath = join(TEST_DIR, "auth.json"); From cb8ac02b96eb7ae055c1faab9d7ba85fac1f9412 Mon Sep 17 00:00:00 2001 From: t Date: Mon, 7 Sep 2026 07:39:36 +0900 Subject: [PATCH 3/3] fix(diagnostics): distinguish inbound size measurement provenance [skip ci] Report declared wire length, observed lower bounds, and decoded sizes without reading rejected payloads further or changing admission and retry policy. Follow up on #3573; thanks to @nowhere1975 for the content-free incident measurements. The issue remains open for exact compact-attempt evidence. Independent source review passed; final remote CI pending. --- .../content/docs/reference/proxy-formats.md | 25 +++- src/server/request-decompress.ts | 42 ++++-- tests/usage/request-decompress.test.ts | 127 ++++++++++++++++-- 3 files changed, 172 insertions(+), 22 deletions(-) diff --git a/docs-site/src/content/docs/reference/proxy-formats.md b/docs-site/src/content/docs/reference/proxy-formats.md index 7008194e19..cb97ad7076 100644 --- a/docs-site/src/content/docs/reference/proxy-formats.md +++ b/docs-site/src/content/docs/reference/proxy-formats.md @@ -409,7 +409,30 @@ default provider is enabled and is not itself an OpenAI-family entry; account-qu such as `side/gpt-5.6-sol` still fail closed. The proxy logs one notice per provider when this fallback engages. Configurations with an enabled canonical `openai` provider are unchanged. -Native compact responses are buffered with a 32 MiB maximum, including responses whose declared +Inbound bodies on both `/v1/responses` and `/v1/responses/compact` retain the shared 256 MiB +wire/decompression admission limit. Application-level size rejection returns HTTP 413 with +`type` and `code` both `invalid_request_error`. Its message includes a bounded diagnostic suffix, +for example: + +```text +Decompressed request body exceeds 268435456 bytes [measurement=decoded_lower_bound; bytes=268435457] +``` + +| Measurement | Meaning of `bytes` | +| --- | --- | +| `declared_wire` | Numeric `Content-Length` declared by the sender; rejected before reading, not a measured decoded size | +| `observed_wire_lower_bound` | Wire bytes encountered when reading stopped; the complete body may be larger | +| `decoded_exact` | Exact size of the buffer supplied to the identity decoder or returned by a decoder | +| `decoded_lower_bound` | Admission limit plus one after inflation aborts; a lower bound, never the exact decoded size | + +The suffix contains only a fixed category and a finite numeric byte value. Rejected bodies are +not read or inflated further, parsed for item counts, or retained for diagnostics. Legacy errors +without measurement provenance retain the limit-only message. Bun's listener can reject an +oversized wire body before application diagnostics run, so not every 413 carries this suffix. +A lower-bound diagnostic cannot establish the complete compact payload size. The admission +limit and retry behavior are unchanged. + +Native compact responses are buffered with a separate 32 MiB maximum, including responses whose declared `Content-Length` already exceeds the limit. The compact-specific failures include: | Status | Type or code | Meaning | diff --git a/src/server/request-decompress.ts b/src/server/request-decompress.ts index 0710470346..297c77a9d1 100644 --- a/src/server/request-decompress.ts +++ b/src/server/request-decompress.ts @@ -27,14 +27,39 @@ export class UnsupportedContentEncodingError extends Error { } } +export type BodySizeMeasurement = + | "declared_wire" + | "observed_wire_lower_bound" + | "decoded_exact" + | "decoded_lower_bound"; + export class DecompressedBodyTooLargeError extends Error { - constructor(readonly bytes: number, limit: number = MAX_DECOMPRESSED_BODY_BYTES) { - super(`Decompressed request body exceeds ${limit} bytes`); + readonly measurement: BodySizeMeasurement | null; + + constructor( + readonly bytes: number, + readonly limit: number = MAX_DECOMPRESSED_BODY_BYTES, + measurement: BodySizeMeasurement | null = null, + ) { + // Legacy callers supply no provenance. Only fixed categories and finite + // numbers may reach the public message, including calls from untyped code. + const category = measurement === "declared_wire" || measurement === "observed_wire_lower_bound" + || measurement === "decoded_exact" || measurement === "decoded_lower_bound" + ? measurement : null; + const suffix = category !== null && Number.isFinite(bytes) && bytes >= 0 + && Number.isFinite(limit) && limit >= 0 + ? ` [measurement=${category}; bytes=${bytes}]` : ""; + super(`Decompressed request body exceeds ${Number.isFinite(limit) ? limit : "unknown"} bytes${suffix}`); + this.measurement = category; } } -function assertBodySizeWithinLimit(body: Uint8Array, maxBytes: number): Uint8Array { - if (body.byteLength > maxBytes) throw new DecompressedBodyTooLargeError(body.byteLength, maxBytes); +function assertBodySizeWithinLimit( + body: Uint8Array, + maxBytes: number, + measurement: BodySizeMeasurement = "decoded_exact", +): Uint8Array { + if (body.byteLength > maxBytes) throw new DecompressedBodyTooLargeError(body.byteLength, maxBytes, measurement); return body; } @@ -112,7 +137,7 @@ async function readRequestBodyBytesCapped( if (!value || value.byteLength === 0) continue; if (value.byteLength > maxBytes - retainedBytes) { - const error = new DecompressedBodyTooLargeError(retainedBytes + value.byteLength, maxBytes); + const error = new DecompressedBodyTooLargeError(retainedBytes + value.byteLength, maxBytes, "observed_wire_lower_bound"); cancel(error); throw error; } @@ -173,7 +198,8 @@ export function decodeRequestBody( else throw new UnsupportedContentEncodingError(encoding); } catch (err) { if ((err as NodeJS.ErrnoException | null)?.code === "ERR_BUFFER_TOO_LARGE") { - throw new DecompressedBodyTooLargeError(maxBytes + 1, maxBytes); + // Inflation stopped at the cap; the full decoded size was never measured. + throw new DecompressedBodyTooLargeError(maxBytes + 1, maxBytes, "decoded_lower_bound"); } throw err; } @@ -198,7 +224,7 @@ export async function readBoundedJsonRequestBody( // Reject an honest oversized declaration before reading. Missing, malformed, // and dishonest declarations remain bounded by the streaming reader below. if (declaredLength !== null && declaredLength > maxBytes) { - const error = new DecompressedBodyTooLargeError(declaredLength, maxBytes); + const error = new DecompressedBodyTooLargeError(declaredLength, maxBytes, "declared_wire"); cancelStreamWithoutWaiting(req.body, error); throw error; } @@ -211,7 +237,7 @@ export async function readBoundedJsonRequestBody( } finally { releaseReservation?.(); } - assertBodySizeWithinLimit(raw, maxBytes); + assertBodySizeWithinLimit(raw, maxBytes, "observed_wire_lower_bound"); const releaseRaw = budget?.observeAcceptedRequestCopy(raw.byteLength); let releaseDecoded: (() => void) | undefined; let releaseText: (() => void) | undefined; diff --git a/tests/usage/request-decompress.test.ts b/tests/usage/request-decompress.test.ts index 7a536600cc..96a8f5a67a 100644 --- a/tests/usage/request-decompress.test.ts +++ b/tests/usage/request-decompress.test.ts @@ -1,4 +1,5 @@ import { describe, expect, test } from "bun:test"; +import { deflateRawSync, deflateSync } from "node:zlib"; import { DecompressedBodyTooLargeError, decodeRequestBody, @@ -9,11 +10,35 @@ import { } from "../../src/server/request-decompress"; import { MANAGEMENT_JSON_BODY_MAX_BYTES } from "../../src/server/management/body"; import { handleManagementAPI } from "../../src/server/management-api"; +import { decodeRequestErrorResponse } from "../../src/server/responses/core"; import type { OcxConfig } from "../../src/types"; const PAYLOAD = { model: "gpt-5.5", input: "hello", stream: true }; const PAYLOAD_BYTES = new TextEncoder().encode(JSON.stringify(PAYLOAD)); +async function captureBodyTooLarge(run: () => unknown): Promise { + try { + await run(); + } catch (error) { + if (!(error instanceof DecompressedBodyTooLargeError)) throw error; + return error; + } + throw new Error("Expected body admission to reject"); +} + +async function expectBodyLimitResponse(error: DecompressedBodyTooLargeError, message: string): Promise { + expect(error.message).toBe(message); + expect(message.length).toBeLessThan(200); + 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" }, + }); + } +} + interface TrackedBodyStats { pulls: number; cancelled: number; @@ -48,6 +73,36 @@ function trackedBodyStream( return { body, stats }; } +describe("DecompressedBodyTooLargeError", () => { + test("preserves one- and two-argument constructors without guessing measurement provenance", async () => { + const legacy = new DecompressedBodyTooLargeError(268435457); + expect(legacy).toMatchObject({ bytes: 268435457, limit: 268435456, measurement: null }); + await expectBodyLimitResponse(legacy, "Decompressed request body exceeds 268435456 bytes"); + const custom = new DecompressedBodyTooLargeError(6, 5); + expect(custom).toMatchObject({ bytes: 6, limit: 5, measurement: null }); + await expectBodyLimitResponse(custom, "Decompressed request body exceeds 5 bytes"); + }); + + test("keeps untyped categories and non-finite numbers out of the message", async () => { + const untyped: DecompressedBodyTooLargeError = Reflect.construct(DecompressedBodyTooLargeError, [ + 6, 5, "private-header-context window".repeat(100), + ]); + expect(untyped.measurement).toBeNull(); + await expectBodyLimitResponse(untyped, "Decompressed request body exceeds 5 bytes"); + for (const bytes of [NaN, Infinity, -Infinity, -1]) { + const error = new DecompressedBodyTooLargeError(bytes, 5, "declared_wire"); + await expectBodyLimitResponse(error, "Decompressed request body exceeds 5 bytes"); + } + for (const limit of [NaN, Infinity, -Infinity]) { + const error = new DecompressedBodyTooLargeError(6, limit, "declared_wire"); + await expectBodyLimitResponse(error, "Decompressed request body exceeds unknown bytes"); + } + const huge = new DecompressedBodyTooLargeError(Number.MAX_VALUE, 5, "declared_wire"); + await expectBodyLimitResponse(huge, + "Decompressed request body exceeds 5 bytes [measurement=declared_wire; bytes=1.7976931348623157e+308]"); + }); +}); + describe("decodeRequestBody", () => { test("passes identity and absent encodings through untouched", () => { expect(decodeRequestBody(PAYLOAD_BYTES, null)).toBe(PAYLOAD_BYTES); @@ -78,10 +133,11 @@ describe("decodeRequestBody", () => { expect(new TextDecoder().decode(decodeRequestBody(compressed, "x-gzip"))).toBe(JSON.stringify(PAYLOAD)); }); - test("round-trips deflate", () => { - const compressed = Bun.deflateSync(PAYLOAD_BYTES); - expect(new TextDecoder().decode(decodeRequestBody(compressed, "deflate"))).toBe(JSON.stringify(PAYLOAD)); - }); + for (const [label, compress] of [["wrapped", deflateSync], ["raw", deflateRawSync], ["Bun raw", Bun.deflateSync]] as const) { + test(`round-trips ${label} deflate`, () => { + expect(new TextDecoder().decode(decodeRequestBody(compress(PAYLOAD_BYTES), "deflate"))).toBe(JSON.stringify(PAYLOAD)); + }); + } test("is case/whitespace tolerant on the encoding token", () => { const compressed = Bun.zstdCompressSync(PAYLOAD_BYTES); @@ -104,15 +160,39 @@ describe("decodeRequestBody", () => { expect(() => decodeRequestBody(compressed, "zstd")).toThrow(DecompressedBodyTooLargeError); }); - test("aborts DURING inflation via maxOutputLength — activation per codec (injected cap)", () => { + test("reports exact identity size at the decoder boundary", async () => { + for (const encoding of [null, "", "identity"]) { + const error = await captureBodyTooLarge(() => decodeRequestBody(Uint8Array.of(1, 2, 3, 4, 5, 6), encoding, 5)); + expect(error).toMatchObject({ bytes: 6, limit: 5, measurement: "decoded_exact" }); + await expectBodyLimitResponse(error, "Decompressed request body exceeds 5 bytes [measurement=decoded_exact; bytes=6]"); + } + }); + + test("aborts DURING inflation and reports only a decoded lower bound for every codec", async () => { // Review finding (PR #96): the cap must fire inside zlib, not after full allocation. // A small injected cap keeps the test cheap while exercising the exact // ERR_BUFFER_TOO_LARGE -> DecompressedBodyTooLargeError path. const CAP = 1024; const inflates64k = new Uint8Array(64 * 1024); - expect(() => decodeRequestBody(Bun.zstdCompressSync(inflates64k), "zstd", CAP)).toThrow(DecompressedBodyTooLargeError); - expect(() => decodeRequestBody(Bun.gzipSync(inflates64k), "gzip", CAP)).toThrow(DecompressedBodyTooLargeError); - expect(() => decodeRequestBody(Bun.deflateSync(inflates64k), "deflate", CAP)).toThrow(DecompressedBodyTooLargeError); + for (const [encoding, compressed] of [ + ["zstd", Bun.zstdCompressSync(inflates64k)], + ["gzip", Bun.gzipSync(inflates64k)], + ["x-gzip", Bun.gzipSync(inflates64k)], + ["deflate", deflateSync(inflates64k)], + ["deflate", deflateRawSync(inflates64k)], + ["deflate", Bun.deflateSync(inflates64k)], + ] as const) { + expect(compressed.byteLength).toBeLessThan(CAP); + // Exercise the streaming reader too: these invalid-JSON bytes must be + // rejected by inflation before text decoding or JSON parsing. + const req = new Request("http://localhost/v1/responses/compact", { + method: "POST", headers: { "content-encoding": encoding }, body: compressed, + }); + const error = await captureBodyTooLarge(() => readBoundedJsonRequestBody(req, CAP)); + expect(error).toMatchObject({ bytes: 1025, limit: 1024, measurement: "decoded_lower_bound" }); + await expectBodyLimitResponse(error, + "Decompressed request body exceeds 1024 bytes [measurement=decoded_lower_bound; bytes=1025]"); + } }); test("injected cap still admits bodies within the limit", () => { @@ -134,6 +214,20 @@ describe("decodeRequestBody", () => { }); describe("readJsonRequestBody", () => { + test("reports a compressed declaration without reading or echoing request metadata", async () => { + const { body, stats } = trackedBodyStream([Bun.gzipSync(PAYLOAD_BYTES)]); + const req = new Request("http://localhost/v1/responses/compact?private-query", { + method: "POST", + headers: { "content-length": "00001025", "content-encoding": "gzip", "x-private-marker": "private-header" }, + body, + }); + const error = await captureBodyTooLarge(() => readBoundedJsonRequestBody(req, 1024)); + expect(error).toMatchObject({ bytes: 1025, limit: 1024, measurement: "declared_wire" }); + await expectBodyLimitResponse(error, + "Decompressed request body exceeds 1024 bytes [measurement=declared_wire; bytes=1025]"); + expect(stats).toEqual({ pulls: 0, cancelled: 1, sentinelPulled: false }); + }); + test("rejects and cancels declared over-cap bodies before reading", async () => { const { body, stats } = trackedBodyStream([PAYLOAD_BYTES]); const req = new Request("http://localhost/v1/responses", { @@ -142,7 +236,10 @@ describe("readJsonRequestBody", () => { body, }); - await expect(readJsonRequestBody(req)).rejects.toBeInstanceOf(DecompressedBodyTooLargeError); + const error = await captureBodyTooLarge(() => readJsonRequestBody(req)); + expect(error).toMatchObject({ bytes: 268435457, limit: 268435456, measurement: "declared_wire" }); + await expectBodyLimitResponse(error, + "Decompressed request body exceeds 268435456 bytes [measurement=declared_wire; bytes=268435457]"); expect(stats.pulls).toBe(0); expect(stats.cancelled).toBe(1); }); @@ -160,8 +257,10 @@ describe("readJsonRequestBody", () => { ], { sentinel }); const req = new Request("http://localhost/api/optional", { method: "POST", headers, body }); - await expect(readBoundedJsonRequestBody(req, 5, undefined, { emptyBodyFallback: {} })) - .rejects.toBeInstanceOf(DecompressedBodyTooLargeError); + const error = await captureBodyTooLarge(() => readBoundedJsonRequestBody(req, 5, undefined, { emptyBodyFallback: {} })); + expect(error).toMatchObject({ bytes: 6, limit: 5, measurement: "observed_wire_lower_bound" }); + await expectBodyLimitResponse(error, + "Decompressed request body exceeds 5 bytes [measurement=observed_wire_lower_bound; bytes=6]"); expect(stats).toEqual({ pulls: 2, cancelled: 1, sentinelPulled: false }); }); } @@ -252,8 +351,10 @@ describe("readJsonRequestBody", () => { body: oversizedWireBody, }); expect(req.headers.get("content-length")).toBeNull(); - await expect(readBoundedJsonRequestBody(req, 1024, undefined, { emptyBodyFallback: {} })) - .rejects.toBeInstanceOf(DecompressedBodyTooLargeError); + const error = await captureBodyTooLarge(() => readBoundedJsonRequestBody(req, 1024, undefined, { emptyBodyFallback: {} })); + expect(error).toMatchObject({ bytes: oversizedWireBody.byteLength, limit: 1024, measurement: "observed_wire_lower_bound" }); + await expectBodyLimitResponse(error, + `Decompressed request body exceeds 1024 bytes [measurement=observed_wire_lower_bound; bytes=${oversizedWireBody.byteLength}]`); }); test("parses an uncompressed request without touching arrayBuffer path", async () => {