diff --git a/sdk/typescript/README.md b/sdk/typescript/README.md index caaae2e60..a31c5abe6 100644 --- a/sdk/typescript/README.md +++ b/sdk/typescript/README.md @@ -215,9 +215,11 @@ npx @openai/codex-security scan C:\code\repository Login, logout, and scans share a private credential home: `$CODEX_SECURITY_STATE_DIR/codex-home`, or `$CODEX_HOME/state/plugins/codex-security/codex-home`. Codex uses the configured -file or keyring storage and managed-device policies. If this home has no -credentials, it imports an existing file-based Codex sign-in. Logout disables -imports until you log in again. +file or keyring storage and managed-device policies. Without an overriding +environment API key, scans and status checks import existing file-based Codex +credentials when this home is empty. Import errors make `login status` exit with +code 2 and SDK `account()` reject its promise. Logout disables imports until you +log in again. Finish operations using older versions before upgrading. Runtime preparation holds the credential-home lock through pauses; exit or crash releases it. diff --git a/sdk/typescript/src/api.ts b/sdk/typescript/src/api.ts index 3226c514e..206be5781 100644 --- a/sdk/typescript/src/api.ts +++ b/sdk/typescript/src/api.ts @@ -1850,11 +1850,20 @@ export class CodexSecurity { const ambientHome = environmentValue(this.#dependencies.environment, "CODEX_HOME") ?? join(homedir(), ".codex"); - await initialCredentialsAvailable( - this.#dependencies.environment, - ambientHome, - authentication.codexHome, - ); + const releaseCredentialHome = + await acquireCodexSecurityCredentialHomeLock( + authentication.codexHome, + this.#abortController.signal, + ); + try { + await initialCredentialsAvailable( + this.#dependencies.environment, + ambientHome, + authentication.codexHome, + ); + } finally { + await releaseCredentialHome(); + } return await accountStatus( this.#codexCommand(), authentication.environment, @@ -1867,16 +1876,28 @@ export class CodexSecurity { await this.#trackOperation(async () => { const authentication = await this.#authentication(); this.#requireOpen(); - await codexLogout( - this.#codexCommand(), - authentication.environment, - this.#abortController.signal, - ); - if ( - this.#runtime === null || - this.#runtime.persistentCredentialHome === true - ) { - await setCodexSecurityCredentialLogout(authentication.codexHome, true); + const releaseCredentialHome = + await acquireCodexSecurityCredentialHomeLock( + authentication.codexHome, + this.#abortController.signal, + ); + try { + await codexLogout( + this.#codexCommand(), + authentication.environment, + this.#abortController.signal, + ); + if ( + this.#runtime === null || + this.#runtime.persistentCredentialHome === true + ) { + await setCodexSecurityCredentialLogout( + authentication.codexHome, + true, + ); + } + } finally { + await releaseCredentialHome(); } if (this.#runtime !== null) this.#runtime.credentialsAvailable = false; this.#runtimeCredentialSource = null; diff --git a/sdk/typescript/src/cli.ts b/sdk/typescript/src/cli.ts index f97806ea0..1e16c5d75 100644 --- a/sdk/typescript/src/cli.ts +++ b/sdk/typescript/src/cli.ts @@ -132,6 +132,7 @@ import { } from "./publish.js"; import type { ScanResult } from "./result.js"; import { + acquireCodexSecurityCredentialHomeLock, bundledPluginRoot, canonicalizeModelSafePath, codexSecurityCredentialHome, @@ -4268,15 +4269,25 @@ export async function main( : await prepareCodexSecurityCredentialHome( dependencies.environment, ); - if (args.action === "status" && existsSync(credentialHome)) { + if ( + args.action === "status" && + existsSync(credentialHome) && + scanAuthentication(dependencies.environment).method !== "api_key" + ) { const ambientHome = environmentValue(dependencies.environment, "CODEX_HOME") ?? join(homedir(), ".codex"); - await initialCredentialsAvailable( - dependencies.environment, - ambientHome, - credentialHome, - ); + const releaseCredentialHome = + await acquireCodexSecurityCredentialHomeLock(credentialHome); + try { + await initialCredentialsAvailable( + dependencies.environment, + ambientHome, + credentialHome, + ); + } finally { + await releaseCredentialHome(); + } } const authenticationEnvironment = { ...dependencies.environment, @@ -4360,16 +4371,22 @@ export async function main( ...dependencies.environment, CODEX_HOME: credentialHome, }; - exitCode = await dependencies.runCodex( - ["logout"], - undefined, - authenticationEnvironment, - ); - if ( - exitCode === 0 && - dependencies.prepareAuthenticationHome !== undefined - ) { - await setCodexSecurityCredentialLogout(credentialHome, true); + const releaseCredentialHome = + await acquireCodexSecurityCredentialHomeLock(credentialHome); + try { + exitCode = await dependencies.runCodex( + ["logout"], + undefined, + authenticationEnvironment, + ); + if ( + exitCode === 0 && + dependencies.prepareAuthenticationHome !== undefined + ) { + await setCodexSecurityCredentialLogout(credentialHome, true); + } + } finally { + await releaseCredentialHome(); } }, }) diff --git a/sdk/typescript/tests-ts/api-credentials.test.ts b/sdk/typescript/tests-ts/api-credentials.test.ts index fb4cb089e..ba7cc347e 100644 --- a/sdk/typescript/tests-ts/api-credentials.test.ts +++ b/sdk/typescript/tests-ts/api-credentials.test.ts @@ -1,12 +1,15 @@ import { execFileSync } from "node:child_process"; import { existsSync } from "node:fs"; -import { mkdir, readFile, stat, writeFile } from "node:fs/promises"; +import { chmod, mkdir, readFile, stat, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { pathToFileURL } from "node:url"; import type { CodexOptions } from "@openai/codex-sdk"; import { afterEach, describe, expect, test } from "bun:test"; import { parse as parseToml } from "smol-toml"; -import { initialCredentialsAvailable } from "../src/api.js"; +import { + initialCredentialsAvailable, + selectedScanEnvironment, +} from "../src/api.js"; import { setCodexSecurityCredentialLogout } from "../src/runtime.js"; import { PLUGIN_ROOT } from "./plugin-root.js"; import { shellEnvironmentReference, TestClient } from "./support/api-client.js"; @@ -30,10 +33,7 @@ describe("CodexSecurity orchestration", () => { await mkdir(scanDir, { mode: 0o700 }); await writeFile(join(ambientHome, "auth.json"), "{}\n"); const interpreter = - process.env["PYTHON"] ?? - Bun.which("python") ?? - Bun.which("py") ?? - Bun.which("python3"); + Bun.which("python3") ?? Bun.which("python") ?? Bun.which("py"); expect(interpreter).not.toBeNull(); let capturedConfigPath: string | undefined; let capturedCodexHome: string | undefined; @@ -478,6 +478,37 @@ describe("CodexSecurity orchestration", () => { ).resolves.toBe(true); }); + test.skipIf(process.platform === "win32" || process.geteuid?.() === 0)( + "reports unreadable ambient credentials during account()", + async () => { + const root = await temporaryDirectory(); + const ambientHome = join(root, "ambient-home"); + const authPath = join(ambientHome, "auth.json"); + await mkdir(ambientHome); + await writeFile(authPath, '{"auth_mode":"chatgpt"}\n', { mode: 0o000 }); + const client = new TestClient( + {}, + { + environment: { + CODEX_HOME: ambientHome, + CODEX_SECURITY_STATE_DIR: join(root, "state"), + }, + resolveCodexCommand: () => { + throw new Error("Must not query Codex after an import failure"); + }, + }, + ); + try { + await expect(client.account()).rejects.toThrow( + "Unable to copy ambient Codex authentication.", + ); + } finally { + await chmod(authPath, 0o600); + await client.close(); + } + }, + ); + test("recognizes ambient credentials during account() on a fresh instance", async () => { const root = await temporaryDirectory(); const ambientHome = join(root, "ambient-home"); @@ -513,7 +544,7 @@ process.exit(process.exitCode ?? 0); { pluginPath: PLUGIN_ROOT }, { environment: { - ...process.env, + ...selectedScanEnvironment(process.env, "chatgpt"), NODE_OPTIONS: `--import=${pathToFileURL(script).href}`, CODEX_HOME: ambientHome, CODEX_SECURITY_STATE_DIR: stateDir, diff --git a/sdk/typescript/tests-ts/api.test.ts b/sdk/typescript/tests-ts/api.test.ts index 54252be8c..e408d3904 100644 --- a/sdk/typescript/tests-ts/api.test.ts +++ b/sdk/typescript/tests-ts/api.test.ts @@ -6021,6 +6021,7 @@ describe("CodexSecurity orchestration", () => { { pluginPath: join(root, "missing-plugin") }, { environment: { + CODEX_HOME: join(root, "ambient-codex-home"), CODEX_SECURITY_STATE_DIR: stateDirectory, ...fakeCommand.environment, }, diff --git a/sdk/typescript/tests-ts/auth-status-concurrency.test.ts b/sdk/typescript/tests-ts/auth-status-concurrency.test.ts new file mode 100644 index 000000000..5c17a008b --- /dev/null +++ b/sdk/typescript/tests-ts/auth-status-concurrency.test.ts @@ -0,0 +1,146 @@ +import { existsSync } from "node:fs"; +import * as fs from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { expect, mock, test } from "bun:test"; +import { runTestInSubprocess } from "./support/test-subprocess.js"; + +for (const surface of ["CLI", "SDK"] as const) { + for (const first of ["status", "logout"] as const) { + const name = `${surface} keeps credentials removed when ${first} starts before a concurrent ${first === "status" ? "logout" : "status"}`; + test(name, async () => { + if (runTestInSubprocess(import.meta.filename, name)) return; + + const originalFs = { ...fs }; + const root = await fs.realpath( + await fs.mkdtemp(join(tmpdir(), "codex-security-auth-concurrency-")), + ); + const ambientHome = join(root, "ambient"); + const source = join(ambientHome, "auth.json"); + await fs.mkdir(ambientHome, { mode: 0o700 }); + await fs.writeFile(source, '{"auth_mode":"chatgpt"}\n', { mode: 0o600 }); + const paused = Promise.withResolvers(); + const resume = Promise.withResolvers(); + const contending = Promise.withResolvers(); + let reachedPause = false; + mock.module("node:fs/promises", () => ({ + ...originalFs, + copyFile: async (...args: Parameters) => { + if (first === "status" && String(args[0]) === source) { + reachedPause = true; + paused.resolve(); + await resume.promise; + } + return await originalFs.copyFile(...args); + }, + })); + const runtime = { ...(await import("../src/runtime.js")) }; + let lockRequests = 0; + mock.module("../src/runtime.js", () => ({ + ...runtime, + acquireCodexSecurityCredentialHomeLock: ( + ...args: Parameters< + typeof runtime.acquireCodexSecurityCredentialHomeLock + > + ) => { + if (++lockRequests === 2) contending.resolve(); + return runtime.acquireCodexSecurityCredentialHomeLock(...args); + }, + })); + const environment = { + CODEX_HOME: ambientHome, + CODEX_SECURITY_STATE_DIR: join(root, "state"), + }; + const home = + await runtime.prepareCodexSecurityCredentialHome(environment); + const credentials = join(home, "auth.json"); + if (first === "logout") { + await fs.writeFile(credentials, '{"auth_mode":"chatgpt"}\n', { + mode: 0o600, + }); + } + const removeCredentials = async (): Promise => { + await fs.rm(credentials, { force: true }); + if (first === "logout") { + reachedPause = true; + paused.resolve(); + await resume.promise; + } + }; + let operations: Record<"status" | "logout", () => Promise>; + const clients: Array<{ close(): Promise }> = []; + if (surface === "CLI") { + const { main } = await import("../src/cli.js"); + const { capture, dependencies } = await import("./cli-fixtures.js"); + const run = async (args: string[]): Promise => + await main(args, capture().stream, capture().stream, { + ...dependencies({ environment }), + prepareAuthenticationHome: + runtime.prepareCodexSecurityCredentialHome, + runCodex: async (command) => { + if (command[0] === "logout") { + await removeCredentials(); + return 0; + } + return existsSync(credentials) ? 0 : 1; + }, + }); + operations = { + status: () => run(["login", "status"]), + logout: async () => { + expect(await run(["logout"])).toBe(0); + }, + }; + } else { + const auth = { ...(await import("../src/auth.js")) }; + mock.module("../src/auth.js", () => ({ + ...auth, + accountStatus: async () => ({ + authenticated: existsSync(credentials), + details: "Synthetic credential status", + }), + logout: removeCredentials, + })); + const { TestClient } = await import("./support/api-client.js"); + const createClient = () => + new TestClient( + {}, + { + environment, + resolveCodexCommand: () => ({ command: process.execPath }), + }, + ); + const statusClient = createClient(); + const logoutClient = createClient(); + clients.push(statusClient, logoutClient); + operations = { + status: () => statusClient.account(), + logout: () => logoutClient.logout(), + }; + } + let firstOperation: Promise | undefined; + let secondOperation: Promise | undefined; + try { + firstOperation = operations[first](); + await Promise.race([paused.promise, firstOperation]); + expect(reachedPause).toBe(true); + secondOperation = + operations[first === "status" ? "logout" : "status"](); + // Continue once the other operation finishes or queues for the lock, + // without depending on a particular scheduler delay. + await Promise.race([secondOperation, contending.promise]); + resume.resolve(); + await Promise.all([firstOperation, secondOperation]); + expect(existsSync(credentials)).toBe(false); + expect( + await runtime.codexSecurityCredentialAllowsAmbientImport(home), + ).toBe(false); + } finally { + resume.resolve(); + await Promise.allSettled([firstOperation, secondOperation]); + await Promise.all(clients.map((client) => client.close())); + await fs.rm(root, { recursive: true, force: true }); + } + }); + } +} diff --git a/sdk/typescript/tests-ts/cli-authentication.test.ts b/sdk/typescript/tests-ts/cli-authentication.test.ts index 38fa2b49e..af003bef0 100644 --- a/sdk/typescript/tests-ts/cli-authentication.test.ts +++ b/sdk/typescript/tests-ts/cli-authentication.test.ts @@ -1,6 +1,7 @@ import { spawnSync } from "node:child_process"; import { existsSync } from "node:fs"; import { + chmod, mkdir, mkdtemp, realpath, @@ -45,6 +46,7 @@ function dependencies( return cliDependencies({ ...options, environment: { + CODEX_HOME: join(stateDirectory, "ambient-codex"), CODEX_SECURITY_STATE_DIR: stateDirectory, ...options.environment, }, @@ -65,8 +67,7 @@ describe("CLI authentication", () => { const stdout = capture(); const stderr = capture(); const deps = dependencies(); - deps.prepareAuthenticationHome = async () => - join(stateDirectory, "codex-home"); + deps.prepareAuthenticationHome = prepareCodexSecurityCredentialHome; let forwarded: readonly string[] | undefined; deps.createSecurity = () => { throw new Error("must not initialize Codex Security"); @@ -1070,6 +1071,36 @@ describe("CLI authentication", () => { expect(`${stdout.text()}${stderr.text()}`).not.toContain("synthetic"); }); + test.skipIf(process.platform === "win32" || process.geteuid?.() === 0)( + "reports unreadable ambient credentials during login status", + async () => { + const ambientHome = join(stateDirectory, "ambient-codex"); + const authPath = join(ambientHome, "auth.json"); + await mkdir(ambientHome); + await writeFile(authPath, '{"auth_mode":"chatgpt"}\n', { mode: 0o000 }); + const deps = dependencies({ environment: { CODEX_HOME: ambientHome } }); + deps.runCodex = async () => { + throw new Error("Must not query Codex after an import failure"); + }; + const stderr = capture(); + try { + expect( + await main( + ["login", "status"], + capture().stream, + stderr.stream, + deps, + ), + ).toBe(2); + expect(stderr.text()).toContain( + "Unable to copy ambient Codex authentication.", + ); + } finally { + await chmod(authPath, 0o600); + } + }, + ); + test("recognizes existing ambient Codex authentication on a fresh state directory during login status", async () => { const root = await realpath( await mkdtemp(join(tmpdir(), "codex-security-cli-ambient-auth-")),