From 68e3a1a136f453fdc60c8fe15692fe2163dce049 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Sat, 20 Jun 2026 13:21:55 -0700 Subject: [PATCH] fix(codex-local): preserve resolved auth for secret-bound keys on reconcile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Startup reconciliation could not resolve a secret-bound OPENAI_API_KEY, so it passed apiKey: null and re-seeded the home with the shared subscription symlink — silently downgrading an already-resolved API-key auth.json on every boot while reporting already_seeded. Classify the persisted OPENAI_API_KEY binding into plain / secret / none and pass apiKeySecretBound through to reconcileManagedCodexHome, which now preserves existing usable auth when the key is secret-bound. Adds regression tests at the adapter and server layers. Co-Authored-By: Paperclip --- .../codex-local/src/server/codex-home.test.ts | 47 +++++++++++++++++++ .../codex-local/src/server/codex-home.ts | 19 ++++++++ .../codex-auth-reconciliation.test.ts | 36 ++++++++++++++ .../src/services/codex-auth-reconciliation.ts | 30 +++++++++++- 4 files changed, 130 insertions(+), 2 deletions(-) diff --git a/packages/adapters/codex-local/src/server/codex-home.test.ts b/packages/adapters/codex-local/src/server/codex-home.test.ts index 038974d604e..df97de16334 100644 --- a/packages/adapters/codex-local/src/server/codex-home.test.ts +++ b/packages/adapters/codex-local/src/server/codex-home.test.ts @@ -405,6 +405,53 @@ describe("reconcileManagedCodexHome", () => { } }); + it("preserves an existing API-key auth.json when the key is secret-bound", async () => { + const fx = await makeFixture(); + try { + // A prior execute-time run resolved the secret and wrote a regular-file + // auth.json containing the key. + await fs.mkdir(fx.agentHome, { recursive: true }); + await fs.writeFile( + fx.agentAuth, + JSON.stringify({ OPENAI_API_KEY: "sk-secret-resolved" }), + { mode: 0o600 }, + ); + + const result = await reconcileManagedCodexHome({ + companyId: "company-1", + configuredCodexHome: fx.agentHome, + apiKeySecretBound: true, + env: fx.env, + }); + + expect(result.status).toBe("already_seeded"); + expect((await fs.lstat(fx.agentAuth)).isSymbolicLink()).toBe(false); + expect(JSON.parse(await fs.readFile(fx.agentAuth, "utf8"))).toEqual({ + OPENAI_API_KEY: "sk-secret-resolved", + }); + } finally { + await fs.rm(fx.root, { recursive: true, force: true }); + } + }); + + it("seeds the shared symlink for a secret-bound key when no auth exists yet", async () => { + const fx = await makeFixture(); + try { + const result = await reconcileManagedCodexHome({ + companyId: "company-1", + configuredCodexHome: fx.agentHome, + apiKeySecretBound: true, + env: fx.env, + }); + + expect(result.status).toBe("seeded"); + expect((await fs.lstat(fx.agentAuth)).isSymbolicLink()).toBe(true); + expect(await fs.realpath(fx.agentAuth)).toBe(await fs.realpath(fx.sharedAuth)); + } finally { + await fs.rm(fx.root, { recursive: true, force: true }); + } + }); + it("writes an API-key auth.json into a managed home when an apiKey is supplied", async () => { const fx = await makeFixture(); try { diff --git a/packages/adapters/codex-local/src/server/codex-home.ts b/packages/adapters/codex-local/src/server/codex-home.ts index 2f78a6b5ddb..89c175a7996 100644 --- a/packages/adapters/codex-local/src/server/codex-home.ts +++ b/packages/adapters/codex-local/src/server/codex-home.ts @@ -230,6 +230,14 @@ export interface ReconcileManagedCodexHomeInput { companyId: string | undefined; configuredCodexHome: string | null | undefined; apiKey?: string | null; + /** + * Set when the agent's persisted `OPENAI_API_KEY` is a secret binding that + * could not be resolved in this context (e.g. startup reconciliation, which + * never resolves secrets). When true and the home already has usable auth, + * reconciliation preserves that auth instead of downgrading it to the shared + * subscription symlink. + */ + apiKeySecretBound?: boolean; env?: NodeJS.ProcessEnv; onLog?: AdapterExecutionContext["onLog"]; } @@ -265,6 +273,17 @@ export async function reconcileManagedCodexHome( const apiKey = nonEmpty(input.apiKey ?? undefined); const hadUsableAuth = await codexHomeHasUsableAuth(resolved); + + // A secret-bound OPENAI_API_KEY cannot be resolved here, so we cannot rewrite + // it into auth.json. If the home already has usable auth — typically an + // API-key auth.json written at execute time when the secret WAS resolved — + // preserve it. Re-seeding without the key would delete that file and restore + // the shared subscription symlink, silently changing the agent's credentials + // on every boot while the persisted config still says "use the secret key". + if (input.apiKeySecretBound && hadUsableAuth) { + return { status: "already_seeded", home: resolved }; + } + await seedManagedCodexHome(resolved, env, input.onLog ?? noopOnLog, { apiKey }); // Seeding always (re)writes when an API key is supplied; otherwise it only diff --git a/server/src/__tests__/codex-auth-reconciliation.test.ts b/server/src/__tests__/codex-auth-reconciliation.test.ts index 10dcc066112..16f75c8fdce 100644 --- a/server/src/__tests__/codex-auth-reconciliation.test.ts +++ b/server/src/__tests__/codex-auth-reconciliation.test.ts @@ -129,6 +129,42 @@ describe("reconcileCodexLocalManagedHomesOnStartup", () => { ); }); + it("preserves a preexisting API-key auth.json when the key is secret-bound", async () => { + const agentHome = managedAgentHome("company-4", "agent-secret"); + // Simulate an agent that previously ran with a RESOLVED secret API key: + // execute-time seeding wrote a regular-file auth.json containing the key. + await fs.mkdir(agentHome, { recursive: true }); + await fs.writeFile( + path.join(agentHome, "auth.json"), + JSON.stringify({ OPENAI_API_KEY: "sk-secret-resolved" }), + { mode: 0o600 }, + ); + const rows: AgentRow[] = [ + { + id: "agent-secret", + companyId: "company-4", + adapterConfig: { + env: { + CODEX_HOME: agentHome, + // Canonical secret binding shape; unresolvable at startup. + OPENAI_API_KEY: { type: "secret_ref", secretId: "11111111-1111-1111-1111-111111111111" }, + }, + }, + }, + ]; + + const summary = await reconcileCodexLocalManagedHomesOnStartup(makeDb(rows)); + expect(summary).toMatchObject({ scanned: 1, seeded: 0, alreadySeeded: 1, failed: 0 }); + + const agentAuth = path.join(agentHome, "auth.json"); + // The resolved-key file must remain a regular file, not be downgraded to the + // shared subscription symlink. + expect((await fs.lstat(agentAuth)).isSymbolicLink()).toBe(false); + expect(JSON.parse(await fs.readFile(agentAuth, "utf8"))).toEqual({ + OPENAI_API_KEY: "sk-secret-resolved", + }); + }); + it("writes a plain OPENAI_API_KEY into the managed home", async () => { const agentHome = managedAgentHome("company-3", "agent-5"); const rows: AgentRow[] = [ diff --git a/server/src/services/codex-auth-reconciliation.ts b/server/src/services/codex-auth-reconciliation.ts index b617245b0eb..b941fae95de 100644 --- a/server/src/services/codex-auth-reconciliation.ts +++ b/server/src/services/codex-auth-reconciliation.ts @@ -37,6 +37,31 @@ function readPlainEnvValue(value: unknown): string | null { return readPlainEnvValue(record.value); } +type ApiKeyBinding = + | { kind: "plain"; value: string } + | { kind: "secret" } + | { kind: "none" }; + +/** + * Classifies an `OPENAI_API_KEY` env binding so reconciliation can tell the + * difference between three states it must treat differently: + * - `plain`: a literal value we can write into auth.json. + * - `secret`: a secret binding (e.g. `{ type: "secret_ref", ... }`) we cannot + * resolve at startup; the resolved value may already exist on disk from a + * prior execute-time run, so reconciliation must not clobber it. + * - `none`: no key configured (chatgpt-subscription mode); seed the shared + * auth symlink. + */ +function classifyApiKeyBinding(value: unknown): ApiKeyBinding { + const plain = readPlainEnvValue(value); + if (plain) return { kind: "plain", value: plain }; + const record = asRecord(value); + if (record && typeof record.type === "string" && record.type !== "plain") { + return { kind: "secret" }; + } + return { kind: "none" }; +} + /** * Startup backfill for PAPA-910 Phase 2: seed `auth.json` into any already- * isolated `codex_local` managed home that was created (by the #8272 isolation @@ -71,13 +96,14 @@ export async function reconcileCodexLocalManagedHomesOnStartup( summary.scanned += 1; const env = asRecord(asRecord(row.adapterConfig)?.env); const configuredCodexHome = env ? readPlainEnvValue(env.CODEX_HOME) : null; - const apiKey = env ? readPlainEnvValue(env.OPENAI_API_KEY) : null; + const apiKeyBinding = classifyApiKeyBinding(env?.OPENAI_API_KEY); try { const result = await reconcileManagedCodexHome({ companyId: row.companyId, configuredCodexHome, - apiKey, + apiKey: apiKeyBinding.kind === "plain" ? apiKeyBinding.value : null, + apiKeySecretBound: apiKeyBinding.kind === "secret", }); switch (result.status) { case "seeded":