From acfedae0af453889e294bec3f7c0b1c67131954a Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Mon, 17 Aug 2026 02:14:26 +0900 Subject: [PATCH] fix(auth): serve env_key bearer admission on Direct by substituting stored main auth #1686's second half. Admission already resolved HOW a credential was presented (`DataPlaneAdmission.source`), and `materializeCodexUpstreamAuth` already knew how to substitute the stored main credential -- but the two never met. The source was resolved at the door in `src/server/index.ts` and dropped one frame later, so `resolveResponsesCodexAuth` still ran `validateForwardAdmissionCredential` against a bearer it had just admitted and answered 401. That is the exact failure in the issue: a Codex client injected with `env_key` could not reach Direct at all. Thread the admission through every surface that replays into `handleResponses`: HTTP Responses, `/v1/responses/compact`, the Chat-translated path, and the WebSocket frame loop (which already retained it on `ws.data`). When the source is `bearer`, skip the forward guard and materialize with `substituteMainCredential`, so the stored main token and `chatgpt-account-id` overwrite the caller's headers before any upstream I/O. Widening admission without guaranteeing substitution would create the leak the guard prevents, so `CodexMainSubstitutionUnavailableError` maps to a 401 -- fail closed with nothing on the wire rather than forwarding our own secret. A dedicated-header caller is untouched: that bearer is the user's own ChatGPT credential and keeps its intentional passthrough. Pool and main-pool overwrite as before. Verification: new `tests/codex-envkey-admission-substitution.test.ts` drives real HTTP against a stubbed upstream and asserts the admission secret never appears in a forwarded header. Driven red by pinning `substituteMainCredential` to false, which reproduces the issue's 401 exactly. `bun x tsc --noEmit` clean; 73 tests green across the four auth suites. The one `codex-main-rotation` failure under a shared runner is a pre-existing environment bleed -- it fails identically with these changes stashed and passes under `--isolate`. --- src/server/chat-completions.ts | 8 +- src/server/index.ts | 6 +- src/server/responses/compact.ts | 13 +- src/server/responses/core.ts | 30 +++- ...odex-envkey-admission-substitution.test.ts | 159 ++++++++++++++++++ 5 files changed, 207 insertions(+), 9 deletions(-) create mode 100644 tests/codex-envkey-admission-substitution.test.ts diff --git a/src/server/chat-completions.ts b/src/server/chat-completions.ts index 7d39afa565..a8e160a206 100644 --- a/src/server/chat-completions.ts +++ b/src/server/chat-completions.ts @@ -37,6 +37,7 @@ import { import { responseWithDeferredRequestLog } from "./relay"; import { handleResponses } from "./responses"; import type { AdmissionLease } from "../lib/admission"; +import type { DataPlaneAdmission } from "./auth-cors"; import { tryClaimNativeMainProfileForTurn } from "../codex/native-main-admission"; import { createTranslatorBudget, @@ -64,7 +65,7 @@ export async function handleChatCompletions( req: Request, config: OcxConfig, logCtx: RequestLogContext, - logIds?: { requestId: string; start: number; turnAdmissionLease?: AdmissionLease }, + logIds?: { requestId: string; start: number; turnAdmissionLease?: AdmissionLease; admission?: DataPlaneAdmission }, ): Promise { const translatorBudget = createTranslatorBudget(); try { @@ -83,7 +84,7 @@ async function handleChatCompletionsWithBudget( config: OcxConfig, logCtx: RequestLogContext, translatorBudget: TranslatorBudget, - logIds?: { requestId: string; start: number; turnAdmissionLease?: AdmissionLease }, + logIds?: { requestId: string; start: number; turnAdmissionLease?: AdmissionLease; admission?: DataPlaneAdmission }, ): Promise { let chatBody: Rec; try { @@ -251,6 +252,9 @@ async function handleChatCompletionsWithBudget( }; const upstream = await handleResponses(internalReq, config, logCtx, { ...(logIds?.turnAdmissionLease ? { turnAdmissionLease: logIds.turnAdmissionLease } : {}), + // #1686: the Chat surface translates its body and replays here, so the admission fact has + // to ride along or a bearer-admitted Chat caller would still be refused by Direct. + ...(logIds?.admission ? { admission: logIds.admission } : {}), abortSignal: req.signal, // Body is Responses-shaped by now, but the client spoke Chat Completions. inboundWire: "chat", diff --git a/src/server/index.ts b/src/server/index.ts index 7241eea139..79a5208b43 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -1092,7 +1092,7 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server { let response: Response; try { - response = await handleResponsesCompact(req, config, logCtx, turnAdmissionLease); + response = await handleResponsesCompact(req, config, logCtx, turnAdmissionLease, admission); } catch { response = formatErrorResponse(500, "server_error", "Unexpected compact request failure"); } @@ -1214,6 +1214,7 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server { const response = await handleResponses(req, config, logCtx, { turnAdmissionLease, + admission, onRequestBodyRead: () => disableResponsesRequestTimeout(req, requestServer), abortSignal: req.signal, onFirstOutput: () => recordFirstOutput(logCtx, start), @@ -1302,7 +1303,7 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server withCors( - await handleChatCompletions(req, config, logCtx, { requestId, start, turnAdmissionLease }), + await handleChatCompletions(req, config, logCtx, { requestId, start, turnAdmissionLease, admission }), req, config, )); @@ -1569,6 +1570,7 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server void) | undefined; const response = await handleResponses(req, config, logCtx, { + ...(wsAdmission ? { admission: wsAdmission } : {}), forceEmptyResponseId: true, inboundTransport: "websocket", abortSignal: turnAbort.signal, diff --git a/src/server/responses/compact.ts b/src/server/responses/compact.ts index 589ba09196..258d4ece11 100644 --- a/src/server/responses/compact.ts +++ b/src/server/responses/compact.ts @@ -51,6 +51,8 @@ import { CodexPoolAuthenticationError, CodexThreadAffinityExpiredError, headersForCodexAuthContext, + materializeCodexUpstreamAuth, + CodexMainSubstitutionUnavailableError, isCodexAuthContextUsable, resolveCodexAuthContext, codexProbeLeaseId, @@ -81,6 +83,7 @@ import { type UpstreamHostAdmissionLease, } from "../../codex/upstream-host-health"; import { ForwardAdmissionCredentialError, validateForwardAdmissionCredential } from "../auth-cors"; +import type { DataPlaneAdmission } from "../auth-cors"; import { listOpenAiForwardSidecarCandidates, resolveFirstUsableOpenAiSidecar, type ResolvedOpenAiForwardSidecar } from "../../providers/openai-sidecar"; import { CODEX_FORWARD_BASE_URL, isCanonicalOpenAiForwardProvider, supportsNativeResponsesCompactEndpoint } from "../../providers/openai-tiers"; import { slugsEquivalent } from "../../providers/slug-codec"; @@ -269,6 +272,7 @@ export async function handleResponsesCompact( config: OcxConfig, logCtx: RequestLogContext, turnAdmissionLease?: AdmissionLease, + admission?: DataPlaneAdmission, ): Promise { let body: unknown; try { @@ -316,7 +320,10 @@ export async function handleResponsesCompact( logCtx.resolvedModel = route.modelId; } - if (route.codexAccountMode === "direct") { + // #1686: a bearer-presented admission secret is one of ours, so the stored main credential + // is substituted below instead of the caller bearer being forwarded. + const substituteMainCredential = admission?.source === "bearer"; + if (route.codexAccountMode === "direct" && !substituteMainCredential) { try { validateForwardAdmissionCredential(req.headers, config); } catch (err) { if (err instanceof ForwardAdmissionCredentialError) return formatErrorResponse(401, "authentication_error", err.message); @@ -364,7 +371,7 @@ export async function handleResponsesCompact( beginCodexAccountSelection: codexAccountSelectionForTurn(turnAdmissionLease), }); logCtx.accountLogLabel = codexAuthContextLogLabel(authCtx, config); - const selected = headersForCodexAuthContext(req.headers, authCtx); + const selected = materializeCodexUpstreamAuth(req.headers, authCtx, { substituteMainCredential }); compactProvider = applyCodexAuthContextToProvider(route.provider, authCtx, route.codexAccountMode); for (const name of FORWARD_HEADERS) { const value = selected.get(name); @@ -671,7 +678,7 @@ export async function handleResponsesCompact( headers: internalHeaders, body: JSON.stringify(internalBody), }); - const response = await handleResponses(internalReq, config, logCtx, { abortSignal: req.signal, turnAdmissionLease }); + const response = await handleResponses(internalReq, config, logCtx, { abortSignal: req.signal, turnAdmissionLease, ...(admission ? { admission } : {}) }); if (!response.ok) return response; let json: { output?: unknown[]; status?: unknown; error?: unknown }; try { diff --git a/src/server/responses/core.ts b/src/server/responses/core.ts index 45e74780b5..e061af218e 100644 --- a/src/server/responses/core.ts +++ b/src/server/responses/core.ts @@ -91,6 +91,8 @@ import { CodexPoolAuthenticationError, CodexThreadAffinityExpiredError, headersForCodexAuthContext, + materializeCodexUpstreamAuth, + CodexMainSubstitutionUnavailableError, isCodexAuthContextUsable, resolveCodexAuthContext, codexProbeLeaseId, @@ -114,6 +116,7 @@ import { prepareSameTarget429Wait, } from "../../lib/upstream-retry"; import { ForwardAdmissionCredentialError, validateForwardAdmissionCredential } from "../auth-cors"; +import type { DataPlaneAdmission } from "../auth-cors"; import { createTranslatorBudget, isTranslatorBudgetExceededError, type TranslatorBudget } from "../../lib/translator-budget"; import { listOpenAiForwardSidecarCandidates, resolveFirstUsableOpenAiSidecar, type ResolvedOpenAiForwardSidecar } from "../../providers/openai-sidecar"; import { isCanonicalOpenAiForwardProvider } from "../../providers/openai-tiers"; @@ -755,6 +758,15 @@ export interface ConsumedComboFailure { export interface HandleResponsesOptions { turnAdmissionLease?: AdmissionLease; + /** + * How the caller proved data-plane admission (#1686). + * + * A bearer-presented admission secret is one of OUR OWN secrets, so a Direct turn must + * SUBSTITUTE the stored main credential rather than forward it. Without this fact at the + * decision point, Direct cannot tell an admission bearer from the user own ChatGPT bearer, + * which is why it refused the whole env_key flow instead of serving it. + */ + admission?: DataPlaneAdmission; /** Called at most once after the complete client body is read and accepted for dispatch. */ onRequestBodyRead?: () => void; forceEmptyResponseId?: boolean; @@ -988,7 +1000,14 @@ async function resolveResponsesCodexAuth( options: HandleResponsesOptions, ): Promise { try { - if (route.codexAccountMode === "direct") validateForwardAdmissionCredential(req.headers, config); + // #1686: a caller that proved admission with a BEARER presented one of our own secrets. + // Refusing it here is what made the codex-cli `env_key` contract unusable against Direct. + // Admitting it is only safe because the stored main credential is substituted below, so + // the admission secret still never leaves this process. + const substituteMainCredential = options.admission?.source === "bearer"; + if (route.codexAccountMode === "direct" && !substituteMainCredential) { + validateForwardAdmissionCredential(req.headers, config); + } let authCtx: CodexAuthContext; if (route.codexAccountMode) { authCtx = await resolveCodexAuthContext(req.headers, config, route.codexAccountMode, { @@ -1011,7 +1030,7 @@ async function resolveResponsesCodexAuth( return { ok: true, authCtx, - headers: headersForCodexAuthContext(req.headers, authCtx), + headers: materializeCodexUpstreamAuth(req.headers, authCtx, { substituteMainCredential }), }; } catch (err) { if (err instanceof CodexAccountCooldownError) { @@ -1045,6 +1064,13 @@ async function resolveResponsesCodexAuth( if (err instanceof ForwardAdmissionCredentialError) { return { ok: false, response: formatErrorResponse(401, "authentication_error", err.message) }; } + if (err instanceof CodexMainSubstitutionUnavailableError) { + // Fail BEFORE any upstream I/O. The alternative is forwarding the admission secret. + return { + ok: false, + response: formatErrorResponse(401, "authentication_error", "No usable Codex main credential to serve this request"), + }; + } throw err; } } diff --git a/tests/codex-envkey-admission-substitution.test.ts b/tests/codex-envkey-admission-substitution.test.ts new file mode 100644 index 0000000000..08ab22245c --- /dev/null +++ b/tests/codex-envkey-admission-substitution.test.ts @@ -0,0 +1,159 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { saveConfig } from "../src/config"; +import { startServer } from "../src/server"; +import type { OcxConfig } from "../src/types"; + +/** + * #1686 end to end: a Codex client injected with `env_key` presents the proxy admission + * secret as `Authorization: Bearer`. Direct must ADMIT that request and substitute the + * stored main credential; the admission secret must never appear upstream. + * + * The unit matrix in tests/codex-auth-context.test.ts proves the materializer in isolation. + * It cannot prove the HTTP handler passes the presentation source down to it, which is the + * half that was missing: the source was resolved at the door and dropped one frame later. + */ + +const originalFetch = globalThis.fetch; +const previousOcxHome = process.env.OPENCODEX_HOME; +const previousCodexHome = process.env.CODEX_HOME; +const previousDataToken = process.env.OPENCODEX_API_AUTH_TOKEN; + +let ocxHome = ""; +let codexHome = ""; +let upstreamAuth: Array = []; + +const ADMISSION_SECRET = "ocx_data_envkeysecret"; + +/** A JWT whose `exp` is far in the future, so the stored main token reads as live. */ +function liveJwt(): string { + const payload = Buffer.from(JSON.stringify({ exp: Math.floor(Date.now() / 1000) + 86_400 })).toString("base64url"); + return `header.${payload}.signature`; +} + +function directConfig(): OcxConfig { + return { + port: 0, + // Remote bind, so admission is actually required rather than loopback-waived. + hostname: "0.0.0.0", + defaultProvider: "openai", + openaiProviderTierVersion: 2, + providers: { + openai: { + adapter: "openai-responses", + baseUrl: "https://chatgpt.com/backend-api/codex", + authMode: "forward", + codexAccountMode: "direct", + defaultModel: "gpt-5.6-luna", + }, + }, + apiKeys: [ + { id: "env-key", name: "env_key", key: ADMISSION_SECRET, createdAt: "2026-08-16T00:00:00.000Z" }, + ], + } as OcxConfig; +} + +function writeStoredMain(accessToken: string): void { + writeFileSync( + join(codexHome, "auth.json"), + JSON.stringify({ tokens: { access_token: accessToken, account_id: "stored_main_acc" } }), + ); +} + +beforeEach(() => { + ocxHome = mkdtempSync(join(tmpdir(), "ocx-1686-home-")); + codexHome = mkdtempSync(join(tmpdir(), "ocx-1686-codex-")); + process.env.OPENCODEX_HOME = ocxHome; + process.env.CODEX_HOME = codexHome; + delete process.env.OPENCODEX_API_AUTH_TOKEN; + upstreamAuth = []; + globalThis.fetch = (async (input, init) => { + const raw = input instanceof Request ? input.url : String(input); + const url = new URL(raw); + if (url.hostname === "chatgpt.com" || url.hostname === "api.openai.com") { + const headers = new Headers(input instanceof Request ? input.headers : init?.headers); + upstreamAuth.push(headers.get("authorization")); + return Response.json({ id: "resp_1686", object: "response", status: "completed", output: [] }); + } + return originalFetch(input, init); + }) as typeof fetch; +}); + +afterEach(() => { + globalThis.fetch = originalFetch; + if (previousOcxHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousOcxHome; + if (previousCodexHome === undefined) delete process.env.CODEX_HOME; + else process.env.CODEX_HOME = previousCodexHome; + if (previousDataToken === undefined) delete process.env.OPENCODEX_API_AUTH_TOKEN; + else process.env.OPENCODEX_API_AUTH_TOKEN = previousDataToken; + if (ocxHome) rmSync(ocxHome, { recursive: true, force: true }); + if (codexHome) rmSync(codexHome, { recursive: true, force: true }); + ocxHome = ""; + codexHome = ""; +}); + +async function postResponses(url: string | URL, authorization: string): Promise { + return originalFetch(new URL("/v1/responses", url), { + method: "POST", + headers: { "content-type": "application/json", authorization }, + body: JSON.stringify({ model: "gpt-5.6-luna", input: "hi", stream: false }), + }); +} + +describe("#1686 env_key bearer admission reaches Direct with substitution", () => { + test("an admission bearer is served and the stored main credential goes upstream", async () => { + saveConfig(directConfig()); + const stored = liveJwt(); + writeStoredMain(stored); + + const server = startServer(0); + try { + const response = await postResponses(server.url, `Bearer ${ADMISSION_SECRET}`); + + // Before this change the same request answered 401: admission accepted the bearer at the + // door, then Direct refused it because it could not tell it from a user's own credential. + expect(response.status).toBe(200); + expect(upstreamAuth).toEqual([`Bearer ${stored}`]); + // The proof that matters: our own secret never reached the wire. + expect(upstreamAuth.join("|")).not.toContain(ADMISSION_SECRET); + } finally { + await server.stop(true); + } + }); + + test("substitution fails closed before any upstream I/O when no main credential is stored", async () => { + saveConfig(directConfig()); + writeFileSync(join(codexHome, "auth.json"), JSON.stringify({ tokens: {} })); + + const server = startServer(0); + try { + const response = await postResponses(server.url, `Bearer ${ADMISSION_SECRET}`); + + expect(response.status).toBe(401); + // Falling through would have forwarded the admission secret, which is the leak the + // forward guard exists to prevent. Nothing may reach an upstream on this path. + expect(upstreamAuth).toHaveLength(0); + } finally { + await server.stop(true); + } + }); + + test("a foreign bearer is still Codex Direct passthrough, not admission", async () => { + saveConfig(directConfig()); + writeStoredMain(liveJwt()); + + const server = startServer(0); + try { + // A real ChatGPT credential is NOT one of our secrets, so it must not be admitted as one. + const response = await postResponses(server.url, "Bearer sk-user-chatgpt-token"); + expect(response.status).toBe(401); + expect(upstreamAuth).toHaveLength(0); + } finally { + await server.stop(true); + } + }); +}); +