From a0182574873e28b0f6d4216c0cc4de43d6921eab Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sat, 18 Apr 2026 08:18:58 -0700 Subject: [PATCH] fix(auth): harden codex oauth bridge security --- .../.generated/plugin-sdk-api-baseline.sha256 | 4 +- .../codex/src/app-server/auth-bridge.test.ts | 65 ++++++++-------- .../codex/src/app-server/auth-bridge.ts | 66 ++-------------- .../openai/openai-codex-cli-bridge.test.ts | 65 +++++++++++----- extensions/openai/openai-codex-cli-bridge.ts | 68 +++------------- .../openai/openai-codex-provider.test.ts | 70 ++++++++++++++++- extensions/openai/openai-codex-provider.ts | 2 +- package.json | 4 + scripts/lib/plugin-sdk-entrypoints.json | 1 + .../auth-profiles/oauth-manager.test.ts | 29 ++++++- src/agents/auth-profiles/oauth-manager.ts | 14 ++-- src/agents/auth-profiles/oauth-shared.ts | 13 ++++ src/agents/auth-profiles/oauth.ts | 1 - src/agents/cli-runner/prepare.ts | 1 - src/agents/codex-auth-bridge.ts | 77 +++++++++++++++++++ src/infra/secret-file.test.ts | 64 +++++++++++---- src/infra/secret-file.ts | 57 +++++++++++--- src/plugin-sdk/codex-auth-bridge-runtime.ts | 8 ++ src/plugins/cli-backend.types.ts | 1 - 19 files changed, 397 insertions(+), 213 deletions(-) create mode 100644 src/agents/codex-auth-bridge.ts create mode 100644 src/plugin-sdk/codex-auth-bridge-runtime.ts diff --git a/docs/.generated/plugin-sdk-api-baseline.sha256 b/docs/.generated/plugin-sdk-api-baseline.sha256 index f9a41b9dc56..b68604f8016 100644 --- a/docs/.generated/plugin-sdk-api-baseline.sha256 +++ b/docs/.generated/plugin-sdk-api-baseline.sha256 @@ -1,2 +1,2 @@ -57d6ea3acdbaff64d59293bb42224be02ca975c5554afacebb78677642355b6f plugin-sdk-api-baseline.json -faff507780c473574e22e73af63dcdc4f4043cea72fa3475606cd4eaf20c1b17 plugin-sdk-api-baseline.jsonl +3447b4257e1eeaf3388b2c148e0086c534ba43147004100e9baa7cb662835c79 plugin-sdk-api-baseline.json +78590addd53ec7db1ef5a56eecc93c39303344ccfad8349a14e8f97480fe64b2 plugin-sdk-api-baseline.jsonl diff --git a/extensions/codex/src/app-server/auth-bridge.test.ts b/extensions/codex/src/app-server/auth-bridge.test.ts index 1d99a5456fa..f759029b275 100644 --- a/extensions/codex/src/app-server/auth-bridge.test.ts +++ b/extensions/codex/src/app-server/auth-bridge.test.ts @@ -2,15 +2,8 @@ import crypto from "node:crypto"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; -import { afterEach, beforeAll, describe, expect, it, vi } from "vitest"; - -const mocks = vi.hoisted(() => ({ - ensureAuthProfileStore: vi.fn(), -})); - -vi.mock("openclaw/plugin-sdk/provider-auth", () => ({ - ensureAuthProfileStore: mocks.ensureAuthProfileStore, -})); +import { saveAuthProfileStore } from "openclaw/plugin-sdk/agent-runtime"; +import { afterEach, beforeAll, describe, expect, it } from "vitest"; let bridgeCodexAppServerStartOptions: typeof import("./auth-bridge.js").bridgeCodexAppServerStartOptions; @@ -29,7 +22,6 @@ describe("bridgeCodexAppServerStartOptions", () => { }); afterEach(async () => { - mocks.ensureAuthProfileStore.mockReset(); await Promise.all( tempDirs.splice(0).map((dir) => fs.rm(dir, { recursive: true, force: true })), ); @@ -38,19 +30,22 @@ describe("bridgeCodexAppServerStartOptions", () => { it("bridges canonical OpenClaw oauth into an isolated CODEX_HOME", async () => { const agentDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-codex-app-server-")); tempDirs.push(agentDir); - mocks.ensureAuthProfileStore.mockReturnValue({ - version: 1, - profiles: { - "openai-codex:default": { - type: "oauth", - provider: "openai-codex", - access: "access-token", - refresh: "refresh-token", - expires: Date.now() + 60_000, - accountId: "acct-123", + saveAuthProfileStore( + { + version: 1, + profiles: { + "openai-codex:default": { + type: "oauth", + provider: "openai-codex", + access: "access-token", + refresh: "refresh-token", + expires: Date.now() + 60_000, + accountId: "acct-123", + }, }, }, - }); + agentDir, + ); const result = await bridgeCodexAppServerStartOptions({ startOptions: { @@ -98,10 +93,7 @@ describe("bridgeCodexAppServerStartOptions", () => { args: ["app-server"], headers: { authorization: "Bearer dev-token" }, }; - mocks.ensureAuthProfileStore.mockReturnValue({ - version: 1, - profiles: {}, - }); + saveAuthProfileStore({ version: 1, profiles: {} }, agentDir); await expect( bridgeCodexAppServerStartOptions({ @@ -115,18 +107,21 @@ describe("bridgeCodexAppServerStartOptions", () => { it("refuses to overwrite a symlinked auth bridge file", async () => { const agentDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-codex-app-server-")); tempDirs.push(agentDir); - mocks.ensureAuthProfileStore.mockReturnValue({ - version: 1, - profiles: { - "openai-codex:default": { - type: "oauth", - provider: "openai-codex", - access: "access-token", - refresh: "refresh-token", - expires: Date.now() + 60_000, + saveAuthProfileStore( + { + version: 1, + profiles: { + "openai-codex:default": { + type: "oauth", + provider: "openai-codex", + access: "access-token", + refresh: "refresh-token", + expires: Date.now() + 60_000, + }, }, }, - }); + agentDir, + ); const codexHome = resolveHashedCodexHome(agentDir, "openai-codex:default"); await fs.mkdir(codexHome, { recursive: true }); diff --git a/extensions/codex/src/app-server/auth-bridge.ts b/extensions/codex/src/app-server/auth-bridge.ts index 3883390e270..3c68f7ce5e4 100644 --- a/extensions/codex/src/app-server/auth-bridge.ts +++ b/extensions/codex/src/app-server/auth-bridge.ts @@ -1,49 +1,7 @@ -import crypto from "node:crypto"; -import path from "node:path"; -import { ensureAuthProfileStore, type OAuthCredential } from "openclaw/plugin-sdk/provider-auth"; -import { writePrivateSecretFileAtomic } from "openclaw/plugin-sdk/secret-file-runtime"; +import { prepareCodexAuthBridgeFromProfile } from "openclaw/plugin-sdk/codex-auth-bridge-runtime"; import type { CodexAppServerStartOptions } from "./config.js"; const DEFAULT_CODEX_AUTH_PROFILE_ID = "openai-codex:default"; -const CODEX_AUTH_ENV_CLEAR_KEYS = ["OPENAI_API_KEY"] as const; - -function isBridgeableCodexOAuthCredential(value: unknown): value is OAuthCredential { - return Boolean( - value && - typeof value === "object" && - value !== null && - "type" in value && - "provider" in value && - "access" in value && - "refresh" in value && - value.type === "oauth" && - value.provider === "openai-codex" && - typeof value.access === "string" && - value.access.trim().length > 0 && - typeof value.refresh === "string" && - value.refresh.trim().length > 0, - ); -} - -function resolveCodexBridgeHome(agentDir: string, profileId: string): string { - const digest = crypto.createHash("sha256").update(profileId).digest("hex").slice(0, 16); - return path.join(agentDir, "harness-auth", "codex", digest); -} - -function buildCodexAuthFile(credential: OAuthCredential): string { - return `${JSON.stringify( - { - auth_mode: "chatgpt", - tokens: { - access_token: credential.access, - refresh_token: credential.refresh, - ...(credential.accountId ? { account_id: credential.accountId } : {}), - }, - }, - null, - 2, - )}\n`; -} export async function bridgeCodexAppServerStartOptions(params: { startOptions: CodexAppServerStartOptions; @@ -51,29 +9,21 @@ export async function bridgeCodexAppServerStartOptions(params: { authProfileId?: string; }): Promise { const profileId = params.authProfileId?.trim() || DEFAULT_CODEX_AUTH_PROFILE_ID; - const store = ensureAuthProfileStore(params.agentDir, { - allowKeychainPrompt: false, + const bridge = await prepareCodexAuthBridgeFromProfile({ + agentDir: params.agentDir, + authProfileId: profileId, + bridgeRoot: "harness-auth", }); - const credential = store.profiles[profileId]; - if (!isBridgeableCodexOAuthCredential(credential)) { + if (!bridge) { return params.startOptions; } - const codexHome = resolveCodexBridgeHome(params.agentDir, profileId); - await writePrivateSecretFileAtomic({ - rootDir: params.agentDir, - filePath: path.join(codexHome, "auth.json"), - content: buildCodexAuthFile(credential), - }); - return { ...params.startOptions, env: { ...params.startOptions.env, - CODEX_HOME: codexHome, + CODEX_HOME: bridge.codexHome, }, - clearEnv: Array.from( - new Set([...(params.startOptions.clearEnv ?? []), ...CODEX_AUTH_ENV_CLEAR_KEYS]), - ), + clearEnv: Array.from(new Set([...(params.startOptions.clearEnv ?? []), ...bridge.clearEnv])), }; } diff --git a/extensions/openai/openai-codex-cli-bridge.test.ts b/extensions/openai/openai-codex-cli-bridge.test.ts index ba003510dac..80f21b630b2 100644 --- a/extensions/openai/openai-codex-cli-bridge.test.ts +++ b/extensions/openai/openai-codex-cli-bridge.test.ts @@ -2,6 +2,7 @@ import crypto from "node:crypto"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; +import { saveAuthProfileStore } from "openclaw/plugin-sdk/agent-runtime"; import { afterEach, describe, expect, it } from "vitest"; import { prepareOpenAICodexCliExecution } from "./openai-codex-cli-bridge.js"; @@ -24,6 +25,22 @@ describe("prepareOpenAICodexCliExecution", () => { it("writes a private CODEX_HOME bridge from canonical OpenClaw oauth", async () => { const agentDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-codex-cli-bridge-")); tempDirs.push(agentDir); + saveAuthProfileStore( + { + version: 1, + profiles: { + "openai-codex:default": { + type: "oauth", + provider: "openai-codex", + access: "access-token", + refresh: "refresh-token", + expires: Date.now() + 60_000, + accountId: "acct-123", + }, + }, + }, + agentDir, + ); const result = await prepareOpenAICodexCliExecution({ config: undefined, @@ -32,14 +49,6 @@ describe("prepareOpenAICodexCliExecution", () => { provider: "codex-cli", modelId: "gpt-5.4", authProfileId: "openai-codex:default", - authCredential: { - type: "oauth", - provider: "openai-codex", - access: "access-token", - refresh: "refresh-token", - expires: Date.now() + 60_000, - accountId: "acct-123", - }, }); expect(result).toMatchObject({ @@ -69,6 +78,19 @@ describe("prepareOpenAICodexCliExecution", () => { it("returns null when there is no bridgeable canonical oauth credential", async () => { const agentDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-codex-cli-bridge-")); tempDirs.push(agentDir); + saveAuthProfileStore( + { + version: 1, + profiles: { + "openai-codex:default": { + type: "api_key", + provider: "openai-codex", + key: "sk-test", + }, + }, + }, + agentDir, + ); await expect( prepareOpenAICodexCliExecution({ @@ -78,11 +100,6 @@ describe("prepareOpenAICodexCliExecution", () => { provider: "codex-cli", modelId: "gpt-5.4", authProfileId: "openai-codex:default", - authCredential: { - type: "api_key", - provider: "openai-codex", - key: "sk-test", - }, }), ).resolves.toBeNull(); }); @@ -93,6 +110,21 @@ describe("prepareOpenAICodexCliExecution", () => { const codexHome = resolveHashedCodexHome(agentDir, "openai-codex:default"); await fs.mkdir(codexHome, { recursive: true }); await fs.symlink(path.join(agentDir, "outside.txt"), path.join(codexHome, "auth.json")); + saveAuthProfileStore( + { + version: 1, + profiles: { + "openai-codex:default": { + type: "oauth", + provider: "openai-codex", + access: "access-token", + refresh: "refresh-token", + expires: Date.now() + 60_000, + }, + }, + }, + agentDir, + ); await expect( prepareOpenAICodexCliExecution({ @@ -102,13 +134,6 @@ describe("prepareOpenAICodexCliExecution", () => { provider: "codex-cli", modelId: "gpt-5.4", authProfileId: "openai-codex:default", - authCredential: { - type: "oauth", - provider: "openai-codex", - access: "access-token", - refresh: "refresh-token", - expires: Date.now() + 60_000, - }, }), ).rejects.toThrow("must not be a symlink"); }); diff --git a/extensions/openai/openai-codex-cli-bridge.ts b/extensions/openai/openai-codex-cli-bridge.ts index a77ba1e3abc..25d22cb5a19 100644 --- a/extensions/openai/openai-codex-cli-bridge.ts +++ b/extensions/openai/openai-codex-cli-bridge.ts @@ -1,75 +1,29 @@ -import crypto from "node:crypto"; -import path from "node:path"; import type { CliBackendPreparedExecution, CliBackendPrepareExecutionContext, } from "openclaw/plugin-sdk/cli-backend"; -import type { OAuthCredential } from "openclaw/plugin-sdk/provider-auth"; -import { writePrivateSecretFileAtomic } from "openclaw/plugin-sdk/secret-file-runtime"; - -const OPENAI_CODEX_PROVIDER_ID = "openai-codex"; -const CODEX_AUTH_ENV_CLEAR_KEYS = ["OPENAI_API_KEY"] as const; - -function isCodexBridgeableOAuthCredential(value: unknown): value is OAuthCredential { - return Boolean( - value && - typeof value === "object" && - value !== null && - "type" in value && - "provider" in value && - "access" in value && - "refresh" in value && - value.type === "oauth" && - value.provider === OPENAI_CODEX_PROVIDER_ID && - typeof value.access === "string" && - value.access.trim().length > 0 && - typeof value.refresh === "string" && - value.refresh.trim().length > 0, - ); -} - -function resolveCodexBridgeHome(agentDir: string, profileId: string): string { - const digest = crypto.createHash("sha256").update(profileId).digest("hex").slice(0, 16); - return path.join(agentDir, "cli-auth", "codex", digest); -} - -function buildCodexAuthFile(credential: OAuthCredential): string { - return `${JSON.stringify( - { - auth_mode: "chatgpt", - tokens: { - access_token: credential.access, - refresh_token: credential.refresh, - ...(credential.accountId ? { account_id: credential.accountId } : {}), - }, - }, - null, - 2, - )}\n`; -} +import { prepareCodexAuthBridgeFromProfile } from "openclaw/plugin-sdk/codex-auth-bridge-runtime"; export async function prepareOpenAICodexCliExecution( ctx: CliBackendPrepareExecutionContext, ): Promise { - if ( - !ctx.agentDir || - !ctx.authProfileId || - !isCodexBridgeableOAuthCredential(ctx.authCredential) - ) { + if (!ctx.agentDir || !ctx.authProfileId) { return null; } - const codexHome = resolveCodexBridgeHome(ctx.agentDir, ctx.authProfileId); - await writePrivateSecretFileAtomic({ - rootDir: ctx.agentDir, - filePath: path.join(codexHome, "auth.json"), - content: buildCodexAuthFile(ctx.authCredential), + const bridge = await prepareCodexAuthBridgeFromProfile({ + agentDir: ctx.agentDir, + authProfileId: ctx.authProfileId, + bridgeRoot: "cli-auth", }); + if (!bridge) { + return null; + } return { env: { - CODEX_HOME: codexHome, + CODEX_HOME: bridge.codexHome, }, - clearEnv: [...CODEX_AUTH_ENV_CLEAR_KEYS], + clearEnv: bridge.clearEnv, }; } diff --git a/extensions/openai/openai-codex-provider.test.ts b/extensions/openai/openai-codex-provider.test.ts index e56e53f5d0a..2cb5644642b 100644 --- a/extensions/openai/openai-codex-provider.test.ts +++ b/extensions/openai/openai-codex-provider.test.ts @@ -1,12 +1,25 @@ -import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; const refreshOpenAICodexTokenMock = vi.hoisted(() => vi.fn()); +const readOpenAICodexCliOAuthProfileMock = vi.hoisted(() => vi.fn()); vi.mock("./openai-codex-provider.runtime.js", () => ({ refreshOpenAICodexToken: refreshOpenAICodexTokenMock, })); +vi.mock("./openai-codex-cli-auth.js", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + readOpenAICodexCliOAuthProfile: readOpenAICodexCliOAuthProfileMock, + }; +}); + let buildOpenAICodexProviderPlugin: typeof import("./openai-codex-provider.js").buildOpenAICodexProviderPlugin; +const tempDirs: string[] = []; describe("openai codex provider", () => { beforeAll(async () => { @@ -15,6 +28,13 @@ describe("openai codex provider", () => { beforeEach(() => { refreshOpenAICodexTokenMock.mockReset(); + readOpenAICodexCliOAuthProfileMock.mockReset(); + }); + + afterEach(async () => { + await Promise.all( + tempDirs.splice(0).map((dir) => fs.rm(dir, { recursive: true, force: true })), + ); }); it("falls back to the cached credential when accountId extraction fails", async () => { @@ -98,6 +118,54 @@ describe("openai codex provider", () => { }); }); + it("uses the provider auth context env when importing Codex CLI auth", async () => { + const provider = buildOpenAICodexProviderPlugin(); + const importMethod = provider.auth?.find((method) => method.id === "import-codex-cli"); + const agentDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-openai-codex-provider-")); + tempDirs.push(agentDir); + readOpenAICodexCliOAuthProfileMock.mockImplementationOnce(({ env }) => { + expect(env).toMatchObject({ + CODEX_HOME: "/sandboxed/codex-home", + }); + return { + profileId: "openai-codex:default", + credential: { + type: "oauth", + provider: "openai-codex", + access: "access-token", + refresh: "refresh-token", + expires: Date.now() + 60_000, + email: "codex@example.com", + displayName: "Codex User", + accountId: "acct-123", + }, + }; + }); + + await expect( + importMethod?.run({ + config: {}, + env: { CODEX_HOME: "/sandboxed/codex-home" }, + agentDir, + prompter: {} as never, + runtime: {} as never, + isRemote: false, + openUrl: async () => {}, + oauth: { createVpsAwareHandlers: (() => ({})) as never }, + }), + ).resolves.toMatchObject({ + profiles: [ + { + profileId: "openai-codex:default", + credential: expect.objectContaining({ + provider: "openai-codex", + access: "access-token", + }), + }, + ], + }); + }); + it("owns native reasoning output mode for Codex responses", () => { const provider = buildOpenAICodexProviderPlugin(); diff --git a/extensions/openai/openai-codex-provider.ts b/extensions/openai/openai-codex-provider.ts index 925fdccfc29..1a311f46c88 100644 --- a/extensions/openai/openai-codex-provider.ts +++ b/extensions/openai/openai-codex-provider.ts @@ -284,7 +284,7 @@ async function runOpenAICodexOAuth(ctx: ProviderAuthContext) { async function runImportOpenAICodexCliAuth(ctx: ProviderAuthContext) { const profile = readOpenAICodexCliOAuthProfile({ - env: process.env, + env: ctx.env ?? process.env, store: ensureAuthProfileStore(ctx.agentDir, { allowKeychainPrompt: false, }), diff --git a/package.json b/package.json index 14de46be320..08bc7a3eff1 100644 --- a/package.json +++ b/package.json @@ -305,6 +305,10 @@ "types": "./dist/plugin-sdk/agent-runtime.d.ts", "default": "./dist/plugin-sdk/agent-runtime.js" }, + "./plugin-sdk/codex-auth-bridge-runtime": { + "types": "./dist/plugin-sdk/codex-auth-bridge-runtime.d.ts", + "default": "./dist/plugin-sdk/codex-auth-bridge-runtime.js" + }, "./plugin-sdk/simple-completion-runtime": { "types": "./dist/plugin-sdk/simple-completion-runtime.d.ts", "default": "./dist/plugin-sdk/simple-completion-runtime.js" diff --git a/scripts/lib/plugin-sdk-entrypoints.json b/scripts/lib/plugin-sdk-entrypoints.json index fc22e141391..27b73cc8133 100644 --- a/scripts/lib/plugin-sdk-entrypoints.json +++ b/scripts/lib/plugin-sdk-entrypoints.json @@ -62,6 +62,7 @@ "text-runtime", "text-chunking", "agent-runtime", + "codex-auth-bridge-runtime", "simple-completion-runtime", "speech-core", "plugin-runtime", diff --git a/src/agents/auth-profiles/oauth-manager.test.ts b/src/agents/auth-profiles/oauth-manager.test.ts index 8dd9c6cf751..02797859f6d 100644 --- a/src/agents/auth-profiles/oauth-manager.test.ts +++ b/src/agents/auth-profiles/oauth-manager.test.ts @@ -5,6 +5,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { createOAuthManager, isSafeToAdoptBootstrapOAuthIdentity, + isSafeToAdoptMainStoreOAuthIdentity, isSafeToOverwriteStoredOAuthIdentity, OAuthManagerRefreshError, } from "./oauth-manager.js"; @@ -73,6 +74,33 @@ describe("isSafeToOverwriteStoredOAuthIdentity", () => { }); }); +describe("isSafeToAdoptMainStoreOAuthIdentity", () => { + it("requires positive identity binding before adopting from the main store", () => { + expect( + isSafeToAdoptMainStoreOAuthIdentity( + createCredential({ + access: "sub-access", + refresh: "sub-refresh", + }), + createCredential({ + access: "main-access", + refresh: "main-refresh", + accountId: "acct-main", + }), + ), + ).toBe(false); + }); + + it("accepts matching account identities", () => { + expect( + isSafeToAdoptMainStoreOAuthIdentity( + createCredential({ accountId: "acct-123" }), + createCredential({ access: "main-access", refresh: "main-refresh", accountId: "acct-123" }), + ), + ).toBe(true); + }); +}); + describe("OAuthManagerRefreshError", () => { it("serializes without leaking credential or store secrets", () => { const refreshedStore: AuthProfileStore = { @@ -140,7 +168,6 @@ describe("createOAuthManager", () => { expires: Date.now() - 30_000, }), isRefreshTokenReusedError: () => false, - isSafeToCopyOAuthIdentity: () => true, }); const result = await manager.resolveOAuthAccess({ diff --git a/src/agents/auth-profiles/oauth-manager.ts b/src/agents/auth-profiles/oauth-manager.ts index 4e30c02d423..565dfe87132 100644 --- a/src/agents/auth-profiles/oauth-manager.ts +++ b/src/agents/auth-profiles/oauth-manager.ts @@ -10,6 +10,7 @@ import { areOAuthCredentialsEquivalent, hasUsableOAuthCredential, isSafeToAdoptBootstrapOAuthIdentity, + isSafeToAdoptMainStoreOAuthIdentity, isSafeToOverwriteStoredOAuthIdentity, overlayRuntimeExternalOAuthProfiles, shouldBootstrapFromExternalCliCredential, @@ -34,10 +35,6 @@ export type OAuthManagerAdapter = { credential: OAuthCredential; }) => OAuthCredential | null; isRefreshTokenReusedError: (error: unknown) => boolean; - isSafeToCopyOAuthIdentity: ( - existing: Pick, - incoming: Pick, - ) => boolean; }; export type ResolvedOAuthAccess = { @@ -90,6 +87,7 @@ export { areOAuthCredentialsEquivalent, hasUsableOAuthCredential, isSafeToAdoptBootstrapOAuthIdentity, + isSafeToAdoptMainStoreOAuthIdentity, isSafeToOverwriteStoredOAuthIdentity, overlayRuntimeExternalOAuthProfiles, shouldBootstrapFromExternalCliCredential, @@ -199,7 +197,7 @@ export function createOAuthManager(adapter: OAuthManagerAdapter) { Number.isFinite(mainCred.expires) && (!Number.isFinite(params.credential.expires) || mainCred.expires > params.credential.expires) && - adapter.isSafeToCopyOAuthIdentity(params.credential, mainCred) + isSafeToAdoptMainStoreOAuthIdentity(params.credential, mainCred) ) { params.store.profiles[params.profileId] = { ...mainCred }; saveAuthProfileStore(params.store, params.agentDir); @@ -327,7 +325,7 @@ export function createOAuthManager(adapter: OAuthManagerAdapter) { mainCred?.type === "oauth" && mainCred.provider === cred.provider && hasUsableOAuthCredential(mainCred) && - adapter.isSafeToCopyOAuthIdentity(cred, mainCred) + isSafeToAdoptMainStoreOAuthIdentity(cred, mainCred) ) { store.profiles[params.profileId] = { ...mainCred }; saveAuthProfileStore(store, params.agentDir); @@ -344,7 +342,7 @@ export function createOAuthManager(adapter: OAuthManagerAdapter) { mainCred?.type === "oauth" && mainCred.provider === cred.provider && hasUsableOAuthCredential(mainCred) && - !adapter.isSafeToCopyOAuthIdentity(cred, mainCred) + !isSafeToAdoptMainStoreOAuthIdentity(cred, mainCred) ) { log.warn("refused to adopt fresh main-store OAuth credential: identity mismatch", { profileId: params.profileId, @@ -537,7 +535,7 @@ export function createOAuthManager(adapter: OAuthManagerAdapter) { mainCred?.type === "oauth" && mainCred.provider === params.credential.provider && hasUsableOAuthCredential(mainCred) && - adapter.isSafeToCopyOAuthIdentity(params.credential, mainCred) + isSafeToAdoptMainStoreOAuthIdentity(params.credential, mainCred) ) { refreshedStore.profiles[params.profileId] = { ...mainCred }; saveAuthProfileStore(refreshedStore, params.agentDir); diff --git a/src/agents/auth-profiles/oauth-shared.ts b/src/agents/auth-profiles/oauth-shared.ts index 95ff67bbdad..c03d77214e5 100644 --- a/src/agents/auth-profiles/oauth-shared.ts +++ b/src/agents/auth-profiles/oauth-shared.ts @@ -131,6 +131,19 @@ export function isSafeToAdoptBootstrapOAuthIdentity( return hasMatchingOAuthIdentity(existing, incoming); } +export function isSafeToAdoptMainStoreOAuthIdentity( + existing: OAuthCredential | undefined, + incoming: OAuthCredential, +): boolean { + if (!existing || existing.type !== "oauth") { + return false; + } + if (existing.provider !== incoming.provider) { + return false; + } + return hasMatchingOAuthIdentity(existing, incoming); +} + export function shouldBootstrapFromExternalCliCredential(params: { existing: OAuthCredential | undefined; imported: OAuthCredential; diff --git a/src/agents/auth-profiles/oauth.ts b/src/agents/auth-profiles/oauth.ts index a0191ede27b..549d2315d22 100644 --- a/src/agents/auth-profiles/oauth.ts +++ b/src/agents/auth-profiles/oauth.ts @@ -288,7 +288,6 @@ const oauthManager = createOAuthManager({ credential, }), isRefreshTokenReusedError, - isSafeToCopyOAuthIdentity, }); export function resetOAuthRefreshQueuesForTest(): void { diff --git a/src/agents/cli-runner/prepare.ts b/src/agents/cli-runner/prepare.ts index 3f06d5f43df..28b17ad5010 100644 --- a/src/agents/cli-runner/prepare.ts +++ b/src/agents/cli-runner/prepare.ts @@ -185,7 +185,6 @@ export async function prepareCliRunContext( provider: params.provider, modelId, authProfileId: effectiveAuthProfileId, - authCredential, }); const authEpoch = await resolveCliAuthEpoch({ provider: params.provider, diff --git a/src/agents/codex-auth-bridge.ts b/src/agents/codex-auth-bridge.ts new file mode 100644 index 00000000000..82aa087a042 --- /dev/null +++ b/src/agents/codex-auth-bridge.ts @@ -0,0 +1,77 @@ +import crypto from "node:crypto"; +import path from "node:path"; +import { writePrivateSecretFileAtomic } from "../infra/secret-file.js"; +import { loadAuthProfileStoreForSecretsRuntime } from "./auth-profiles/store.js"; +import type { OAuthCredential } from "./auth-profiles/types.js"; + +export const OPENAI_CODEX_PROVIDER_ID = "openai-codex"; +export const CODEX_AUTH_ENV_CLEAR_KEYS = ["OPENAI_API_KEY"] as const; + +export function isCodexBridgeableOAuthCredential(value: unknown): value is OAuthCredential { + return Boolean( + value && + typeof value === "object" && + value !== null && + "type" in value && + "provider" in value && + "access" in value && + "refresh" in value && + value.type === "oauth" && + value.provider === OPENAI_CODEX_PROVIDER_ID && + typeof value.access === "string" && + value.access.trim().length > 0 && + typeof value.refresh === "string" && + value.refresh.trim().length > 0, + ); +} + +export function resolveCodexBridgeHome( + agentDir: string, + profileId: string, + bridgeRoot: "cli-auth" | "harness-auth", +): string { + const digest = crypto.createHash("sha256").update(profileId).digest("hex").slice(0, 16); + return path.join(agentDir, bridgeRoot, "codex", digest); +} + +export function buildCodexAuthFile(credential: OAuthCredential): string { + return `${JSON.stringify( + { + auth_mode: "chatgpt", + tokens: { + access_token: credential.access, + refresh_token: credential.refresh, + ...(credential.accountId ? { account_id: credential.accountId } : {}), + }, + }, + null, + 2, + )}\n`; +} + +export async function prepareCodexAuthBridgeFromProfile(params: { + agentDir: string; + authProfileId: string; + bridgeRoot: "cli-auth" | "harness-auth"; +}): Promise<{ codexHome: string; clearEnv: string[] } | null> { + const store = loadAuthProfileStoreForSecretsRuntime(params.agentDir); + const credential = store.profiles[params.authProfileId]; + if (!isCodexBridgeableOAuthCredential(credential)) { + return null; + } + + const codexHome = resolveCodexBridgeHome( + params.agentDir, + params.authProfileId, + params.bridgeRoot, + ); + await writePrivateSecretFileAtomic({ + rootDir: params.agentDir, + filePath: path.join(codexHome, "auth.json"), + content: buildCodexAuthFile(credential), + }); + return { + codexHome, + clearEnv: [...CODEX_AUTH_ENV_CLEAR_KEYS], + }; +} diff --git a/src/infra/secret-file.test.ts b/src/infra/secret-file.test.ts index 8aea56c52fd..3f585bd41fc 100644 --- a/src/infra/secret-file.test.ts +++ b/src/infra/secret-file.test.ts @@ -1,4 +1,4 @@ -import { mkdir, stat, symlink, writeFile } from "node:fs/promises"; +import * as fsPromises from "node:fs/promises"; import path from "node:path"; import { afterEach, describe, expect, it } from "vitest"; import { createTrackedTempDirs } from "../test-utils/tracked-temp-dirs.js"; @@ -47,7 +47,7 @@ describe("readSecretFileSync", () => { it("reads and trims a regular secret file", async () => { const dir = await createTempDir(); const file = path.join(dir, "secret.txt"); - await writeFile(file, " top-secret \n", "utf8"); + await fsPromises.writeFile(file, " top-secret \n", "utf8"); expect(readSecretFileSync(file, "Gateway password")).toBe("top-secret"); expect(tryReadSecretFileSync(file, "Gateway password")).toBe("top-secret"); @@ -90,7 +90,7 @@ describe("readSecretFileSync", () => { name: "rejects files larger than the secret-file limit", setup: async (dir: string) => { const file = path.join(dir, "secret.txt"); - await writeFile(file, "x".repeat(DEFAULT_SECRET_FILE_MAX_BYTES + 1), "utf8"); + await fsPromises.writeFile(file, "x".repeat(DEFAULT_SECRET_FILE_MAX_BYTES + 1), "utf8"); return file; }, expectedMessage: (file: string) => @@ -100,7 +100,7 @@ describe("readSecretFileSync", () => { name: "rejects non-regular files", setup: async (dir: string) => { const nestedDir = path.join(dir, "secret-dir"); - await mkdir(nestedDir); + await fsPromises.mkdir(nestedDir); return nestedDir; }, expectedMessage: (file: string) => `Gateway password file at ${file} must be a regular file.`, @@ -110,8 +110,8 @@ describe("readSecretFileSync", () => { setup: async (dir: string) => { const target = path.join(dir, "target.txt"); const link = path.join(dir, "secret-link.txt"); - await writeFile(target, "top-secret\n", "utf8"); - await symlink(target, link); + await fsPromises.writeFile(target, "top-secret\n", "utf8"); + await fsPromises.symlink(target, link); return link; }, options: { rejectSymlink: true }, @@ -121,7 +121,7 @@ describe("readSecretFileSync", () => { name: "rejects empty secret files after trimming", setup: async (dir: string) => { const file = path.join(dir, "secret.txt"); - await writeFile(file, " \n\t ", "utf8"); + await fsPromises.writeFile(file, " \n\t ", "utf8"); return file; }, expectedMessage: (file: string) => `Gateway password file at ${file} is empty.`, @@ -136,7 +136,7 @@ describe("readSecretFileSync", () => { pathValue: async () => createSecretPath(async (dir) => { const file = path.join(dir, "secret.txt"); - await writeFile(file, " \n\t ", "utf8"); + await fsPromises.writeFile(file, " \n\t ", "utf8"); return file; }), label: "Gateway password", @@ -154,8 +154,8 @@ describe("readSecretFileSync", () => { createSecretPath(async (dir) => { const target = path.join(dir, "target.txt"); const link = path.join(dir, "secret-link.txt"); - await writeFile(target, "top-secret\n", "utf8"); - await symlink(target, link); + await fsPromises.writeFile(target, "top-secret\n", "utf8"); + await fsPromises.symlink(target, link); return link; }), label: "Telegram bot token", @@ -207,8 +207,8 @@ describe("writePrivateSecretFileAtomic", () => { secret: '{"ok":true}', }); if (process.platform !== "win32") { - const dirStat = await stat(path.dirname(file)); - const fileStat = await stat(file); + const dirStat = await fsPromises.stat(path.dirname(file)); + const fileStat = await fsPromises.stat(file); expect(dirStat.mode & 0o777).toBe(PRIVATE_SECRET_DIR_MODE); expect(fileStat.mode & 0o777).toBe(PRIVATE_SECRET_FILE_MODE); } @@ -219,9 +219,9 @@ describe("writePrivateSecretFileAtomic", () => { const nestedDir = path.join(dir, "nested"); const target = path.join(dir, "outside.txt"); const link = path.join(nestedDir, "auth.json"); - await mkdir(nestedDir); - await writeFile(target, "outside", "utf8"); - await symlink(target, link); + await fsPromises.mkdir(nestedDir); + await fsPromises.writeFile(target, "outside", "utf8"); + await fsPromises.symlink(target, link); await expect( writePrivateSecretFileAtomic({ @@ -235,8 +235,8 @@ describe("writePrivateSecretFileAtomic", () => { it("rejects symlinked path components", async () => { const dir = await createTempDir(); const targetDir = path.join(dir, "outside-dir"); - await mkdir(targetDir); - await symlink(targetDir, path.join(dir, "linked")); + await fsPromises.mkdir(targetDir); + await fsPromises.symlink(targetDir, path.join(dir, "linked")); await expect( writePrivateSecretFileAtomic({ @@ -246,4 +246,34 @@ describe("writePrivateSecretFileAtomic", () => { }), ).rejects.toThrow("must not be a symlink"); }); + + it("tightens an existing world-readable directory before writing secrets", async () => { + const dir = await createTempDir(); + const nestedDir = path.join(dir, "nested"); + await fsPromises.mkdir(nestedDir, { mode: 0o777 }); + if (process.platform !== "win32") { + await writePrivateSecretFileAtomic({ + rootDir: dir, + filePath: path.join(nestedDir, "auth.json"), + content: '{"ok":true}\n', + }); + const dirStat = await fsPromises.stat(nestedDir); + expect(dirStat.mode & 0o777).toBe(PRIVATE_SECRET_DIR_MODE); + } + }); + + it("rejects a parent directory symlink before it can escape the private root", async () => { + const dir = await createTempDir(); + const targetDir = await createTempDir(); + const aliasDir = path.join(dir, "nested"); + await fsPromises.symlink(targetDir, aliasDir); + + await expect( + writePrivateSecretFileAtomic({ + rootDir: dir, + filePath: path.join(aliasDir, "auth.json"), + content: '{"ok":true}\n', + }), + ).rejects.toThrow("must not be a symlink"); + }); }); diff --git a/src/infra/secret-file.ts b/src/infra/secret-file.ts index e4d6e337b82..75a24998f50 100644 --- a/src/infra/secret-file.ts +++ b/src/infra/secret-file.ts @@ -151,6 +151,31 @@ function assertPathWithinRoot(rootDir: string, targetPath: string): void { } } +function assertRealPathWithinRoot(rootDir: string, targetPath: string): void { + const relative = path.relative(rootDir, targetPath); + if (relative.startsWith("..") || path.isAbsolute(relative)) { + throw new Error(`Private secret path must stay under ${rootDir}.`); + } +} + +async function enforcePrivatePathMode( + resolvedPath: string, + expectedMode: number, + kind: "directory" | "file", +): Promise { + if (process.platform === "win32") { + return; + } + await fsp.chmod(resolvedPath, expectedMode); + const stat = await fsp.stat(resolvedPath); + const actualMode = stat.mode & 0o777; + if (actualMode !== expectedMode) { + throw new Error( + `Private secret ${kind} ${resolvedPath} has insecure permissions ${actualMode.toString(8)}.`, + ); + } +} + async function ensurePrivateDirectory(rootDir: string, targetDir: string): Promise { const resolvedRoot = path.resolve(rootDir); const resolvedTarget = path.resolve(targetDir); @@ -163,12 +188,13 @@ async function ensurePrivateDirectory(rootDir: string, targetDir: string): Promi if (!rootStat.isDirectory()) { throw new Error(`Private secret root ${resolvedRoot} must be a directory.`); } - await fsp.chmod(resolvedRoot, PRIVATE_SECRET_DIR_MODE).catch(() => undefined); + await enforcePrivatePathMode(resolvedRoot, PRIVATE_SECRET_DIR_MODE, "directory"); return; } assertPathWithinRoot(resolvedRoot, resolvedTarget); await ensurePrivateDirectory(resolvedRoot, resolvedRoot); + const resolvedRootReal = await fsp.realpath(resolvedRoot); let current = resolvedRoot; for (const segment of path @@ -190,7 +216,9 @@ async function ensurePrivateDirectory(rootDir: string, targetDir: string): Promi } await fsp.mkdir(current, { mode: PRIVATE_SECRET_DIR_MODE }); } - await fsp.chmod(current, PRIVATE_SECRET_DIR_MODE).catch(() => undefined); + const currentReal = await fsp.realpath(current); + assertRealPathWithinRoot(resolvedRootReal, currentReal); + await enforcePrivatePathMode(currentReal, PRIVATE_SECRET_DIR_MODE, "directory"); } } @@ -202,16 +230,21 @@ export async function writePrivateSecretFileAtomic(params: { const resolvedRoot = path.resolve(params.rootDir); const resolvedFile = path.resolve(params.filePath); assertPathWithinRoot(resolvedRoot, resolvedFile); - const parentDir = path.dirname(resolvedFile); - await ensurePrivateDirectory(resolvedRoot, parentDir); + const intendedParentDir = path.dirname(resolvedFile); + await ensurePrivateDirectory(resolvedRoot, intendedParentDir); + const resolvedRootReal = await fsp.realpath(resolvedRoot); + const parentDir = await fsp.realpath(intendedParentDir); + assertRealPathWithinRoot(resolvedRootReal, parentDir); + const fileName = path.basename(resolvedFile); + const finalFilePath = path.join(parentDir, fileName); try { - const stat = await fsp.lstat(resolvedFile); + const stat = await fsp.lstat(finalFilePath); if (stat.isSymbolicLink()) { - throw new Error(`Private secret file ${resolvedFile} must not be a symlink.`); + throw new Error(`Private secret file ${finalFilePath} must not be a symlink.`); } if (!stat.isFile()) { - throw new Error(`Private secret file ${resolvedFile} must be a regular file.`); + throw new Error(`Private secret file ${finalFilePath} must be a regular file.`); } } catch (error) { if (!error || typeof error !== "object" || !("code" in error) || error.code !== "ENOENT") { @@ -232,10 +265,14 @@ export async function writePrivateSecretFileAtomic(params: { } finally { await handle.close(); } - await fsp.chmod(tempPath, PRIVATE_SECRET_FILE_MODE).catch(() => undefined); - await fsp.rename(tempPath, resolvedFile); + await enforcePrivatePathMode(tempPath, PRIVATE_SECRET_FILE_MODE, "file"); + const refreshedParentReal = await fsp.realpath(intendedParentDir); + if (refreshedParentReal !== parentDir) { + throw new Error(`Private secret parent directory changed during write for ${finalFilePath}.`); + } + await fsp.rename(tempPath, finalFilePath); createdTemp = false; - await fsp.chmod(resolvedFile, PRIVATE_SECRET_FILE_MODE).catch(() => undefined); + await enforcePrivatePathMode(finalFilePath, PRIVATE_SECRET_FILE_MODE, "file"); } finally { if (createdTemp) { await fsp.unlink(tempPath).catch(() => undefined); diff --git a/src/plugin-sdk/codex-auth-bridge-runtime.ts b/src/plugin-sdk/codex-auth-bridge-runtime.ts new file mode 100644 index 00000000000..fd1269fac20 --- /dev/null +++ b/src/plugin-sdk/codex-auth-bridge-runtime.ts @@ -0,0 +1,8 @@ +export { + buildCodexAuthFile, + CODEX_AUTH_ENV_CLEAR_KEYS, + isCodexBridgeableOAuthCredential, + OPENAI_CODEX_PROVIDER_ID, + prepareCodexAuthBridgeFromProfile, + resolveCodexBridgeHome, +} from "../agents/codex-auth-bridge.js"; diff --git a/src/plugins/cli-backend.types.ts b/src/plugins/cli-backend.types.ts index 4169d614ce9..9862a130a42 100644 --- a/src/plugins/cli-backend.types.ts +++ b/src/plugins/cli-backend.types.ts @@ -25,7 +25,6 @@ export type CliBackendPrepareExecutionContext = { provider: string; modelId: string; authProfileId?: string; - authCredential?: unknown; }; export type CliBackendPreparedExecution = {