From d7d3e1fbf7598eaebbee3b1ed96dd15952c94c8e Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Fri, 18 Sep 2026 03:58:28 +0900 Subject: [PATCH 1/3] fix(responses): scope compact handoff cache to authenticated admission principal The compactHandoffRoutes map is process-global but was keyed only by the caller-supplied lane header, so any two admitted clients sending the same lane header shared one fallback route. Namespace the lane by the admitted principal (contextPrincipalId when minted, configured key id or environment kind otherwise); loopback and missing admissions have no authenticated identity and are ineligible rather than trusted. --- src/server/responses/compact.ts | 46 ++++- ...esponses-compact-handoff-admission.test.ts | 180 ++++++++++++++++++ .../responses-compaction-routing.test.ts | 12 +- 3 files changed, 222 insertions(+), 16 deletions(-) create mode 100644 tests/responses/responses-compact-handoff-admission.test.ts diff --git a/src/server/responses/compact.ts b/src/server/responses/compact.ts index 6d428194b94..0b467545e27 100644 --- a/src/server/responses/compact.ts +++ b/src/server/responses/compact.ts @@ -107,6 +107,7 @@ import { } from "../../codex/upstream-host-health"; import { ForwardAdmissionCredentialError, + contextPrincipalIdOf, hasForwardableCodexBearer, validateForwardAdmissionCredential, } from "../auth-cors"; @@ -214,8 +215,28 @@ function pruneCompactHandoffRoutes(now: number): void { } } -function rememberCompactHandoffRoute(req: Request, model: string, now = Date.now()): void { - const key = sessionLaneIdFromRequest(req.headers); +/** + * The handoff map is process-global, so a caller-controlled lane header alone + * cannot be the key: two authenticated clients sending the same lane header + * would share one fallback route. Namespace the lane by the admitted principal. + * A loopback or missing admission has no authenticated identity to bind this + * cross-request state to, so it is ineligible rather than trusted. + */ +function compactHandoffRouteKey(req: Request, admission: DataPlaneAdmission | undefined): string | null { + const lane = sessionLaneIdFromRequest(req.headers); + if (!lane || !admission || admission.kind === "loopback") return null; + const principal = contextPrincipalIdOf(admission) + ?? (admission.kind === "configured" ? `configured:${admission.keyId}` : "environment"); + return `${principal}\u0000${lane}`; +} + +function rememberCompactHandoffRoute( + req: Request, + admission: DataPlaneAdmission | undefined, + model: string, + now = Date.now(), +): void { + const key = compactHandoffRouteKey(req, admission); if (!key || model.length > COMPACT_HANDOFF_MODEL_MAX_LENGTH) return; pruneCompactHandoffRoutes(now); compactHandoffRoutes.delete(key); @@ -223,13 +244,18 @@ function rememberCompactHandoffRoute(req: Request, model: string, now = Date.now pruneCompactHandoffRoutes(now); } -function forgetCompactHandoffRoute(req: Request): void { - const key = sessionLaneIdFromRequest(req.headers); +function forgetCompactHandoffRoute(req: Request, admission?: DataPlaneAdmission): void { + const key = compactHandoffRouteKey(req, admission); if (key) compactHandoffRoutes.delete(key); } -function compactHandoffRoute(req: Request, previousModel: string, now = Date.now()): string | null { - const key = sessionLaneIdFromRequest(req.headers); +function compactHandoffRoute( + req: Request, + admission: DataPlaneAdmission | undefined, + previousModel: string, + now = Date.now(), +): string | null { + const key = compactHandoffRouteKey(req, admission); if (!key) return null; pruneCompactHandoffRoutes(now); const entry = compactHandoffRoutes.get(key); @@ -1272,10 +1298,10 @@ export async function handleResponsesCompact( // synthetic buffer errors are not upstream bodies and stay uninspected. if (buffered.ok) { inspectResponseLogJson(logCtx, await buffered.clone().text()); - forgetCompactHandoffRoute(req); + forgetCompactHandoffRoute(req, admission); rememberServingConversationStateIssuer(outcomeCtx, codexPoolAffinityKey(req.headers)); } else if (quotaFailure && !storedPool401ReplayAttempted) { - const fallbackModel = compactHandoffRoute(req, raw.model); + const fallbackModel = compactHandoffRoute(req, admission, raw.model); if (fallbackModel && !req.signal.aborted) { const fallbackReq = new Request(req.url, { method: "POST", @@ -1407,7 +1433,7 @@ export async function handleResponsesCompact( const result = new Response(JSON.stringify({ output: compactionItems }), { headers: { "Content-Type": "application/json" }, }); - rememberCompactHandoffRoute(req, raw.model); + rememberCompactHandoffRoute(req, admission, raw.model); return result; } const encrypted = compactionItems[0]!.encrypted_content; @@ -1418,6 +1444,6 @@ export async function handleResponsesCompact( } const summary = decoded; const output = buildCompactV1Output(extractCompactUserMessages(inputItems), summary); - rememberCompactHandoffRoute(req, raw.model); + rememberCompactHandoffRoute(req, admission, raw.model); return new Response(JSON.stringify({ output }), { headers: { "Content-Type": "application/json" } }); } diff --git a/tests/responses/responses-compact-handoff-admission.test.ts b/tests/responses/responses-compact-handoff-admission.test.ts new file mode 100644 index 00000000000..a556471e6dc --- /dev/null +++ b/tests/responses/responses-compact-handoff-admission.test.ts @@ -0,0 +1,180 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { mkdtempSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { saveCodexAccountCredential } from "../../src/codex/account-store"; +import { clearAccountQuota, updateAccountQuota } from "../../src/codex/auth-api"; +import { clearCodexUpstreamHealth } from "../../src/codex/routing"; +import { clearUpstreamHostHealth } from "../../src/codex/upstream-host-health"; +import { handleResponsesCompact } from "../../src/server/responses"; +import type { OcxConfig } from "../../src/types"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; + +const originalFetch = globalThis.fetch; + +afterEach(() => { + globalThis.fetch = originalFetch; +}); + +/** + * The compact handoff map is process-global while its lane key arrives in a + * caller-controlled header. These tests pin the admission-principal namespacing + * that keeps one authenticated client from claiming another client's remembered + * fallback route by re-sending the same lane header. + */ +describe("compact handoff route admission namespacing", () => { + function poolConfig(): OcxConfig { + return { + defaultProvider: "openai", + activeCodexAccountId: "pool-a", + providers: { + openai: { + adapter: "openai-responses", + baseUrl: "https://chatgpt.com/backend-api/codex", + authMode: "forward", + codexAccountMode: "pool", + }, + }, + codexAccounts: [{ + id: "pool-a", + email: "pool@example.test", + isMain: false, + chatgptAccountId: "pool_acc", + }], + } as OcxConfig; + } + + function withPoolEnv(run: (config: OcxConfig) => Promise): Promise { + const testDir = mkdtempSync(join(tmpdir(), "ocx-compact-handoff-admission-")); + const previousOpencodexHome = process.env.OPENCODEX_HOME; + const previousCodexHome = process.env.CODEX_HOME; + process.env.OPENCODEX_HOME = testDir; + process.env.CODEX_HOME = testDir; + clearCodexUpstreamHealth(); + clearUpstreamHostHealth(); + clearAccountQuota(); + saveCodexAccountCredential("pool-a", { + accessToken: "pool-a-access-token", + refreshToken: "pool-a-refresh-token", + expiresAt: Date.now() + 300_000, + chatgptAccountId: "pool_acc", + }); + updateAccountQuota("pool-a", 10); + return run(poolConfig()).finally(() => { + globalThis.fetch = originalFetch; + clearCodexUpstreamHealth(); + clearUpstreamHostHealth(); + clearAccountQuota(); + removeTreeWithRetry(testDir); + if (previousOpencodexHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousOpencodexHome; + if (previousCodexHome === undefined) delete process.env.CODEX_HOME; + else process.env.CODEX_HOME = previousCodexHome; + }); + } + + function compactionRequest( + body: Record, + extraHeaders: Record, + ): Request { + return new Request("http://localhost/v1/responses", { + method: "POST", + headers: { "content-type": "application/json", ...extraHeaders }, + body: JSON.stringify(body), + }); + } + + function compactionBody(model: string): Record { + return { + model, + stream: false, + input: [ + { type: "message", role: "user", content: [{ type: "input_text", text: "earlier turn" }] }, + { type: "compaction_trigger" }, + ], + tools: [{ type: "function", name: "shell" }], + tool_choice: "auto", + parallel_tool_calls: true, + }; + } + + test("a remembered route is claimed only by the principal that stored it", async () => { + await withPoolEnv(async config => { + config.providers.deepseek = { + adapter: "openai-chat", + baseUrl: "https://api.deepseek.com", + authMode: "key", + apiKey: "deepseek-test-key", + models: ["deepseek-v4-flash"], + }; + config.providers["openai-apikey"] = { + adapter: "openai-responses", + baseUrl: "https://api.openai.com/v1", + authMode: "key", + apiKey: "openai-test-key", + models: ["gpt-5.6-sol"], + }; + const headers = { "x-codex-parent-thread-id": "compact-handoff-admission-thread" }; + const owner = { kind: "configured", keyId: "compact-client", source: "dedicated" } as const; + const calls: Array<{ model: string; nativeCompact: boolean }> = []; + globalThis.fetch = (async (input: string | URL | Request, init?: RequestInit) => { + const url = typeof input === "string" + ? input + : input instanceof URL + ? input.toString() + : input.url; + const body = JSON.parse(String(init?.body ?? "{}")) as { model?: string }; + const nativeCompact = url.endsWith("/responses/compact"); + calls.push({ model: body.model ?? "", nativeCompact }); + if (nativeCompact) { + return Response.json({ error: { message: "The usage limit has been reached" } }, { + status: 502, + }); + } + return new Response(JSON.stringify({ + id: "resp_1", + status: "completed", + output: [{ type: "message", role: "assistant", content: [{ type: "output_text", text: "DeepSeek handoff summary" }] }], + usage: { input_tokens: 10, output_tokens: 5, total_tokens: 15 }, + }), { status: 200, headers: { "content-type": "application/json" } }); + }) as typeof fetch; + + const compact = (model: string, admission: Parameters[4]) => + handleResponsesCompact( + compactionRequest(compactionBody(model), headers), + config, + { model: "", provider: "" }, + undefined, + admission, + ); + + // The owner stores the deepseek route under (its principal, this lane). + const stored = await compact("deepseek/deepseek-v4-flash", owner); + expect(stored.status).toBe(200); + expect(calls).toEqual([{ model: "deepseek-v4-flash", nativeCompact: false }]); + + // A different admitted principal re-sending the same lane header must not + // claim it: every attempt stays on the requested model's native compact. + for (const intruder of [ + { kind: "configured", keyId: "different-client", source: "dedicated" }, + { kind: "environment", source: "bearer" }, + { kind: "loopback", source: "loopback" }, + undefined, + ] as const) { + calls.length = 0; + const res = await compact("openai-apikey/gpt-5.6-sol", intruder); + expect(res.status).toBe(502); + expect(calls.length).toBeGreaterThan(0); + expect(calls.every(call => call.model === "gpt-5.6-sol" && call.nativeCompact)).toBe(true); + } + + // The owner's own quota-blocked retry still finds the route and hands off. + calls.length = 0; + const handoff = await compact("openai-apikey/gpt-5.6-sol", owner); + expect(handoff.status).toBe(200); + expect(calls.at(-1)).toEqual({ model: "deepseek-v4-flash", nativeCompact: false }); + }); + // Five request sequences ride the transient-502 retry ladder; the default + // 5s budget is not enough on a contended host. + }, 20000); +}); diff --git a/tests/responses/responses-compaction-routing.test.ts b/tests/responses/responses-compaction-routing.test.ts index c612aed502a..57a5c82d319 100644 --- a/tests/responses/responses-compaction-routing.test.ts +++ b/tests/responses/responses-compaction-routing.test.ts @@ -1481,6 +1481,9 @@ describe("compact alternate-account attempt (#913)", () => { models: ["gpt-5.6-sol"], }; const headers = { "x-codex-parent-thread-id": "compact-routed-handoff-thread" }; + // The remembered route is keyed by the admitted principal, so every call in + // this scenario authenticates as the same configured client. + const admission = { kind: "configured", keyId: "compact-client", source: "dedicated" } as const; const calls: Array<{ model: string; nativeCompact: boolean }> = []; globalThis.fetch = (async (input: string | URL | Request, init?: RequestInit) => { const url = typeof input === "string" @@ -1505,8 +1508,7 @@ describe("compact alternate-account attempt (#913)", () => { undefined, headers, ), - config, - { model: "", provider: "" }, + config, { model: "", provider: "" }, undefined, admission, ); expect(manual.status).toBe(200); expect(calls).toEqual([{ model: "deepseek-v4-flash", nativeCompact: false }]); @@ -1518,8 +1520,7 @@ describe("compact alternate-account attempt (#913)", () => { undefined, { "x-codex-parent-thread-id": "different-compact-thread" }, ), - config, - { model: "", provider: "" }, + config, { model: "", provider: "" }, undefined, admission, ); expect(unrelated.status).toBe(502); expect(calls.length).toBeGreaterThan(0); @@ -1533,8 +1534,7 @@ describe("compact alternate-account attempt (#913)", () => { undefined, headers, ), - config, - logCtx, + config, logCtx, undefined, admission, ); expect(automatic.status).toBe(200); From 338d8fffdded025bfe9ca261c514dd39074b8a80 Mon Sep 17 00:00:00 2001 From: lidge-jun Date: Fri, 18 Sep 2026 09:44:26 +0900 Subject: [PATCH 2/3] fix(responses): fail closed when an admission mints no context principal compactHandoffRouteKey substituted "configured:" plus the key id, or the constant "environment", when contextPrincipalIdOf returned nothing. Both substitutes reintroduce the collision the key exists to prevent: a key id survives rotation, so a replaced secret inherited the previous holder's route, and every identity-less environment admission shared one bucket. Production admission always mints a principal for configured and environment holders, so refusing the cache without one costs no real caller anything. The new test file was also missing from both test-layout registries, which tests/test-layout-tooling.test.ts asserts independently. Register it, and replace the different-key intruder with a same-key-id rotation so the case the old fallback collapsed is the one under test. --- scripts/test-layout/layout.json | 1 + src/server/responses/compact.ts | 9 +++++++-- tests/fixtures/test-layout-expected.json | 1 + .../responses-compact-handoff-admission.test.ts | 12 ++++++++++-- 4 files changed, 19 insertions(+), 4 deletions(-) diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 162b4ba20fb..07f71ce4d9f 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -1204,6 +1204,7 @@ "response-model-identity.test.ts": "server", "responses-account-label.test.ts": "responses", "responses-canonical-only-top-level-fields.test.ts": "responses", + "responses-compact-handoff-admission.test.ts": "responses", "responses-compaction-routing.test.ts": "responses", "responses-compaction.test.ts": "responses", "responses-console-go-upload-retry.test.ts": "responses", diff --git a/src/server/responses/compact.ts b/src/server/responses/compact.ts index 0b467545e27..ab16ce31b6e 100644 --- a/src/server/responses/compact.ts +++ b/src/server/responses/compact.ts @@ -225,8 +225,13 @@ function pruneCompactHandoffRoutes(now: number): void { function compactHandoffRouteKey(req: Request, admission: DataPlaneAdmission | undefined): string | null { const lane = sessionLaneIdFromRequest(req.headers); if (!lane || !admission || admission.kind === "loopback") return null; - const principal = contextPrincipalIdOf(admission) - ?? (admission.kind === "configured" ? `configured:${admission.keyId}` : "environment"); + // Fail closed rather than substituting a weaker identity. `keyId` survives a + // rotation and every identity-less environment admission would collapse into + // one bucket, which is the collision this key exists to prevent. Production + // admission always mints `contextPrincipalId` for configured and environment + // holders, so no real authenticated caller loses the route. + const principal = contextPrincipalIdOf(admission); + if (!principal) return null; return `${principal}\u0000${lane}`; } diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index efdee7fadc8..456f3ff967f 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -1031,6 +1031,7 @@ "responses-account-label.test.ts": "responses", "responses-bare-echo-helper-fence.test.ts": "responses", "responses-canonical-only-top-level-fields.test.ts": "responses", + "responses-compact-handoff-admission.test.ts": "responses", "responses-compaction-routing.test.ts": "responses", "responses-compaction.test.ts": "responses", "responses-console-go-upload-retry.test.ts": "responses", diff --git a/tests/responses/responses-compact-handoff-admission.test.ts b/tests/responses/responses-compact-handoff-admission.test.ts index a556471e6dc..5fcb2aa67e3 100644 --- a/tests/responses/responses-compact-handoff-admission.test.ts +++ b/tests/responses/responses-compact-handoff-admission.test.ts @@ -115,7 +115,12 @@ describe("compact handoff route admission namespacing", () => { models: ["gpt-5.6-sol"], }; const headers = { "x-codex-parent-thread-id": "compact-handoff-admission-thread" }; - const owner = { kind: "configured", keyId: "compact-client", source: "dedicated" } as const; + const owner = { + kind: "configured", + keyId: "compact-client", + source: "dedicated", + contextPrincipalId: "principal-owner", + } as const; const calls: Array<{ model: string; nativeCompact: boolean }> = []; globalThis.fetch = (async (input: string | URL | Request, init?: RequestInit) => { const url = typeof input === "string" @@ -156,7 +161,10 @@ describe("compact handoff route admission namespacing", () => { // A different admitted principal re-sending the same lane header must not // claim it: every attempt stays on the requested model's native compact. for (const intruder of [ - { kind: "configured", keyId: "different-client", source: "dedicated" }, + // Same key id, rotated secret. Admission mints a new principal, and this + // is precisely the pair a keyId-derived key would have collapsed. + { kind: "configured", keyId: "compact-client", source: "dedicated", contextPrincipalId: "principal-rotated" }, + // Authenticated but carrying no minted principal: ineligible, not pooled. { kind: "environment", source: "bearer" }, { kind: "loopback", source: "loopback" }, undefined, From 0def3347530d24b14717f7e8f4b8b9fce52ed9b9 Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Fri, 18 Sep 2026 12:57:18 +0900 Subject: [PATCH 3/3] test(responses): include the minted principal in the routed-handoff admission fixture --- tests/responses/responses-compaction-routing.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/responses/responses-compaction-routing.test.ts b/tests/responses/responses-compaction-routing.test.ts index 57a5c82d319..3250e696c6a 100644 --- a/tests/responses/responses-compaction-routing.test.ts +++ b/tests/responses/responses-compaction-routing.test.ts @@ -1483,7 +1483,7 @@ describe("compact alternate-account attempt (#913)", () => { const headers = { "x-codex-parent-thread-id": "compact-routed-handoff-thread" }; // The remembered route is keyed by the admitted principal, so every call in // this scenario authenticates as the same configured client. - const admission = { kind: "configured", keyId: "compact-client", source: "dedicated" } as const; + const admission = { kind: "configured", keyId: "compact-client", source: "dedicated", contextPrincipalId: "compact-client-principal" } as const; const calls: Array<{ model: string; nativeCompact: boolean }> = []; globalThis.fetch = (async (input: string | URL | Request, init?: RequestInit) => { const url = typeof input === "string"