Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 97 additions & 13 deletions scripts/codex.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ const APP_RUNTIME_HELPER_OWNER_PID_ENV =
"CODEX_MULTI_AUTH_APP_ROTATION_OWNER_PID";
const APP_RUNTIME_HELPER_REAL_CODEX_HOME_ENV =
"CODEX_MULTI_AUTH_REAL_CODEX_HOME";
const APP_RUNTIME_HELPER_USE_CANONICAL_HOME_ENV =
"CODEX_MULTI_AUTH_APP_ROTATION_USE_CANONICAL_HOME";
const APP_SERVER_CONFIG_ARGS_ENV =
"CODEX_MULTI_AUTH_APP_SERVER_CONFIG_ARGS_JSON";
const APP_RUNTIME_HELPER_STATUS_FILE =
RUNTIME_CONSTANTS.APP_RUNTIME_HELPER_STATUS_FILE;
const DEFAULT_APP_RUNTIME_HELPER_IDLE_MS = 12 * 60 * 60 * 1000;
Expand Down Expand Up @@ -3490,6 +3494,38 @@ function createRuntimeRotationProxyCodexHome(
};
}

function createRuntimeRotationProxyCanonicalCodexHome(
baseEnv,
proxyBaseUrl,
clientApiKey,
configTomlModule,
) {
const originalCodexHome = resolveRuntimeRotationProxyOriginalCodexHome(baseEnv);
const providerTable = `model_providers.${RUNTIME_ROTATION_PROXY_PROVIDER_ID}`;
const configArgs = [
`${providerTable}.name=${configTomlModule.tomlStringLiteral(APP_SERVER_ACCOUNT_DISPLAY_NAME)}`,
`${providerTable}.base_url=${configTomlModule.tomlStringLiteral(proxyBaseUrl)}`,
`${providerTable}.env_key=${configTomlModule.tomlStringLiteral("OPENAI_API_KEY")}`,
`${providerTable}.requires_openai_auth=false`,
`${providerTable}.wire_api=${configTomlModule.tomlStringLiteral("responses")}`,
"disable_response_storage=false",
].flatMap((assignment) => ["-c", assignment]);

return {
args: configArgs,
env: {
...baseEnv,
CODEX_HOME: originalCodexHome,
OPENAI_API_KEY: clientApiKey,
CODEX_MULTI_AUTH_DIR: resolveRuntimeRotationOriginalMultiAuthDir(
originalCodexHome,
baseEnv,
),
},
cleanup: () => {},
};
}

Comment on lines +3497 to +3528

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'repo files matching codex.js/codex-bin-wrapper.test.ts:\n'
fd -a 'codex\.js$|codex-bin-wrapper\.test\.ts$' . | sed 's#^\./##'

printf '\noutline scripts/codex.js around target functions:\n'
ast-grep outline scripts/codex.js --match createRuntimeRotationProxyCanonicalCodexHome --view expanded || true
ast-grep outline scripts/codex.js --match createRuntimeRotationProxyCodexHome --view expanded || true

printf '\nrelevant scripts/codex.js lines:\n'
nl -ba scripts/codex.js | sed -n '3360,3560p'

printf '\nlock/guard terms in scripts/codex.js:\n'
rg -n "lock|single|instance|FILE_LOCK|tryLock|flock|lockfile|fs\\.lock|fs\\.open|mkdtemp|tmpdir|ORIGINAL_CODEX_HOME|CODEX_HOME=|CODEX_MULTI_AUTH_REAL_CODEX_BIN|createRuntimeRotation" scripts/codex.js || true

printf '\ntest lines:\n'
nl -ba test/codex-bin-wrapper.test.ts | sed -n '2700,2875p'

Repository: ndycode/codex-multi-auth

Length of output: 614


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'scripts/codex.js relevant lines:\n'
awk 'NR>=3360 && NR<=3560 {printf "%6d\t%s\n", NR, $0}' scripts/codex.js

printf '\nlock/guard terms in scripts/codex.js:\n'
rg -n "lock|single|instance|FILE_LOCK|tryLock|flock|lockfile|fs\\.lock|fs\\.open|mkdtemp|tmpdir|ORIGINAL_CODEX_HOME|CODEX_HOME=|CODEX_MULTI_AUTH_REAL_CODEX_BIN|createRuntimeRotation" scripts/codex.js || true

printf '\ncreateRuntimeRotationProxyCanonicalCodexHome call sites in scripts/codex.js:\n'
rg -n "createRuntimeRotationProxyCanonicalCodexHome|createRuntimeRotationProxyCodexHome|app-bridge|app-server|--canonical-shim" scripts/codex.js || true

printf '\ntest/codex-bin-wrapper.test.ts relevant lines:\n'
awk 'NR>=2700 && NR<=2875 {printf "%6d\t%s\n", NR, $0}' test/codex-bin-wrapper.test.ts

printf '\nconcurrency-related terms in test/codex-bin-wrapper.test.ts:\n'
rg -n "concurrent|concurren|Promise\\s*<|all\\(|race\\(|spawnSync|fork\\(|parallel|simultan|same fixture home|same home" test/codex-bin-wrapper.test.ts || true

Repository: ndycode/codex-multi-auth

Length of output: 28523


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'codex.js lines around canonical context creation and app helper startup:\n'
awk 'NR>=3870 && NR<=3940 {printf "%6d\t%s\n", NR, $0}' scripts/codex.js
awk 'NR>=4010 && NR<=4155 {printf "%6d\t%s\n", NR, $0}' scripts/codex.js
awk 'NR>=4210 && NR'=4295' {printf "%6d\t%s\n", NR, $0}' scripts/codex.js

printf '\nsearch canonical home tests near usages:\n'
rg -n "canonical Codex home|canonical-shim|canonical.*home|app-server --canonical-shim|createRuntimeRotationProxyCanonicalCodexHome" test/codex-bin-wrapper.test.ts scripts/codex.js || true

printf '\nPython source scan for lock/single-instance around canonical home env:\n'
python3 - <<'PY'
from pathlib import Path
p=Path('scripts/codex.js')
s=p.read_text()
for name in ['createRuntimeRotationProxyCanonicalCodexHome','createRuntimeRotationProxyContextIfEnabled','createRuntimeRotationAppHelperContext']:
    idx=s.find(f'function {name}')
    print(f'{name}: found={idx!=-1}')
print()
for needle in [
    'FILE_LOCK', 'single-instance', 'tryLock', 'flock', 'lockfile', 'mkdtempSync(join(shadowRoot',
    'CODEX_HOME: originalCodexHome', 'cleanup: () => {}',
]:
    print(needle, '=>', s.find(needle))
PY

Repository: ndycode/codex-multi-auth

Length of output: 41998


don’t give interactive TUI/app-helper modes unguarded access to the real codex home.

scripts/codex.js:3903-3938 selects createRuntimeRotationProxyCanonicalCodexHome for TUI and scripts/codex.js:4078-4124 gives the helper the same env; it sets CODEX_HOME to originalCodexHome with cleanup: () => {}, while scripts/codex.js:3419-3495 previously used a shadow home and sync-back. no lock/single-instance guard is in play, so two concurrent canonical codex-multi-auth sessions using the same Codex home can race on session/sqlite state. add a regression in test/codex-bin-wrapper.test.ts:2763-2857 that launches two canonical sessions concurrently and asserts no state corruption/loss, or enforce a single-instance/flock/backoff design.

📍 Affects 2 files
  • scripts/codex.js#L3497-L3528 (this comment)
  • test/codex-bin-wrapper.test.ts#L2763-L2857
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/codex.js` around lines 3497 - 3528, Prevent interactive TUI and
app-helper modes from concurrently using the real Codex home without protection:
update createRuntimeRotationProxyCanonicalCodexHome to restore isolated
shadow-home behavior with safe sync-back, or enforce an equivalent
single-instance lock/flock with backoff around canonical sessions. In
test/codex-bin-wrapper.test.ts lines 2763-2857, add a regression that launches
two canonical sessions concurrently and verifies session/SQLite state is
preserved without corruption or loss.

Source: Path instructions

function appendNodeImportOption(nodeOptions, preloadPath) {
const importOption = `--import=${pathToFileURL(preloadPath).href}`;
const trimmed = (nodeOptions ?? "").trim();
Expand All @@ -3504,15 +3540,26 @@ function createRuntimeRotationAppServerPreloadSource(wrapperScriptPath) {
"",
`const wrapperScriptPath = ${JSON.stringify(wrapperScriptPath)};`,
`const accountLabelEnv = ${JSON.stringify(APP_SERVER_ACCOUNT_LABEL_ENV)};`,
`const configArgsEnv = ${JSON.stringify(APP_SERVER_CONFIG_ARGS_ENV)};`,
"let configArgs = [];",
"try {",
' const parsed = JSON.parse(process.env[configArgsEnv] ?? "[]");',
" if (Array.isArray(parsed)) {",
' configArgs = parsed.filter((arg) => typeof arg === "string");',
" }",
"} catch {",
" // Ignore malformed internal metadata and preserve app-server startup.",
"}",
"const rawArgs = process.argv.slice(1);",
"const firstArg = rawArgs[0] ?? \"\";",
'if (basename(firstArg).toLowerCase() === "app-server") {',
' const args = ["app-server", ...rawArgs.slice(1)];',
' const args = ["app-server", ...rawArgs.slice(1), ...configArgs];',
" const env = {",
" ...process.env,",
' CODEX_MULTI_AUTH_RUNTIME_ROTATION_PROXY: "0",',
" [accountLabelEnv]: \"1\",",
" };",
" delete env[configArgsEnv];",
" const child = spawn(process.execPath, [wrapperScriptPath, ...args], {",
" stdio: \"inherit\",",
" env,",
Expand Down Expand Up @@ -3558,7 +3605,7 @@ function sweepStaleRuntimeRotationAppServerShimDirs(shimRootDir) {
}
}

function installRuntimeRotationAppServerCliShim(forwardedEnv) {
function installRuntimeRotationAppServerCliShim(forwardedEnv, configArgs = []) {
const shadowCodexHome = forwardedEnv.CODEX_HOME;
if (!shadowCodexHome) {
throw new Error("runtime app-server shim requires CODEX_HOME");
Expand Down Expand Up @@ -3615,6 +3662,11 @@ function installRuntimeRotationAppServerCliShim(forwardedEnv) {
);
forwardedEnv.CODEX_MULTI_AUTH_RUNTIME_ROTATION_PROXY = "0";
forwardedEnv[APP_SERVER_ACCOUNT_LABEL_ENV] = "1";
if (configArgs.length > 0) {
forwardedEnv[APP_SERVER_CONFIG_ARGS_ENV] = JSON.stringify(configArgs);
} else {
delete forwardedEnv[APP_SERVER_CONFIG_ARGS_ENV];
}
return shimDir;
}

Expand Down Expand Up @@ -3715,6 +3767,7 @@ function pickRuntimeRotationAppHelperEnv(env) {
"CODEX_MULTI_AUTH_REAL_CODEX_BIN",
"CODEX_MULTI_AUTH_RUNTIME_ROTATION_PROXY",
APP_SERVER_ACCOUNT_LABEL_ENV,
APP_SERVER_CONFIG_ARGS_ENV,
]) {
if (env[name]) {
picked[name] = env[name];
Expand Down Expand Up @@ -3776,7 +3829,7 @@ function createRuntimeRotationAppHelperStatus({

async function runRuntimeRotationAppHelper() {
let proxyServer = null;
let shadowContext = null;
let runtimeContext = null;
let appServerShimDir = null;
let statusTimer = null;
let closing = false;
Expand Down Expand Up @@ -3813,7 +3866,7 @@ async function runRuntimeRotationAppHelper() {
}
appServerShimDir = null;
}
shadowContext?.cleanup?.();
runtimeContext?.cleanup?.();
} finally {
try {
await proxyServer?.close?.();
Expand Down Expand Up @@ -3844,13 +3897,33 @@ async function runRuntimeRotationAppHelper() {
}
const clientApiKey = createRuntimeRotationProxyClientApiKey();
proxyServer = await proxyModule.startRuntimeRotationProxy({ clientApiKey });
shadowContext = createRuntimeRotationProxyCodexHome(
process.env,
proxyServer.baseUrl,
clientApiKey,
configTomlModule,
const useCanonicalHome =
(process.env[APP_RUNTIME_HELPER_USE_CANONICAL_HOME_ENV] ?? "").trim() ===
"1";
runtimeContext = useCanonicalHome
? createRuntimeRotationProxyCanonicalCodexHome(
process.env,
proxyServer.baseUrl,
clientApiKey,
configTomlModule,
)
: createRuntimeRotationProxyCodexHome(
process.env,
proxyServer.baseUrl,
clientApiKey,
configTomlModule,
);
const appServerConfigArgs = useCanonicalHome
? [
...(runtimeContext.args ?? []),
"-c",
`model_provider=${configTomlModule.tomlStringLiteral(RUNTIME_ROTATION_PROXY_PROVIDER_ID)}`,
]
: [];
appServerShimDir = installRuntimeRotationAppServerCliShim(
runtimeContext.env,
appServerConfigArgs,
);
appServerShimDir = installRuntimeRotationAppServerCliShim(shadowContext.env);
lastRequestCount = proxyServer.getStatus?.().totalRequests ?? 0;
publishStatus("running");
process.stdout.write(
Expand All @@ -3859,7 +3932,8 @@ async function runRuntimeRotationAppHelper() {
pid: process.pid,
baseUrl: proxyServer.baseUrl,
statusPath: resolveRuntimeRotationAppHelperStatusPath(),
env: pickRuntimeRotationAppHelperEnv(shadowContext.env),
args: runtimeContext.args ?? [],
env: pickRuntimeRotationAppHelperEnv(runtimeContext.env),
})}\n`,
);

Expand Down Expand Up @@ -3920,7 +3994,7 @@ function stopRuntimeRotationAppHelper(helper) {
return waitForRuntimeRotationAppHelperExit(helper);
}

function startRuntimeRotationAppHelper(baseContext) {
function startRuntimeRotationAppHelper(baseContext, options = {}) {
const realCodexHome =
baseContext.originalCodexHome ??
resolveRuntimeRotationProxyOriginalCodexHome(baseContext.env);
Expand All @@ -3940,6 +4014,8 @@ function startRuntimeRotationAppHelper(baseContext) {
),
[APP_RUNTIME_HELPER_OWNER_PID_ENV]: String(process.pid),
[APP_RUNTIME_HELPER_REAL_CODEX_HOME_ENV]: realCodexHome,
[APP_RUNTIME_HELPER_USE_CANONICAL_HOME_ENV]:
options.useCanonicalHome === true ? "1" : "0",
},
stdio: ["ignore", "pipe", "pipe"],
detached: true,
Expand Down Expand Up @@ -4005,8 +4081,14 @@ async function createRuntimeRotationAppHelperContext(
options = {},
) {
const startedAt = Date.now();
const { helper, message } = await startRuntimeRotationAppHelper(baseContext);
const { helper, message } = await startRuntimeRotationAppHelper(
baseContext,
options,
);
const helperEnv = message.env ?? {};
const helperArgs = Array.isArray(message.args)
? message.args.filter((arg) => typeof arg === "string")
: [];
const detachGraceMs = resolveRuntimeRotationAppHelperDetachGraceMs(baseContext.env);

const cleanup = async ({ exitCode } = {}) => {
Expand All @@ -4023,6 +4105,7 @@ async function createRuntimeRotationAppHelperContext(
return {
args: [
...baseContext.args,
...helperArgs,
"-c",
`model_provider=${configTomlModule.tomlStringLiteral(RUNTIME_ROTATION_PROXY_PROVIDER_ID)}`,
],
Expand Down Expand Up @@ -4063,6 +4146,7 @@ async function createRuntimeRotationProxyContextIfEnabled(
if (isCodexInteractiveTuiCommand(rawArgs)) {
return createRuntimeRotationAppHelperContext(baseContext, configTomlModule, {
detachOnExit: true,
useCanonicalHome: true,
});
}

Expand Down
95 changes: 95 additions & 0 deletions test/codex-bin-wrapper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2760,6 +2760,101 @@ describe("codex bin wrapper", () => {
expect(marker).toContain("close\n");
});

it("uses the canonical Codex home for interactive TUI runtime routing", async () => {
const fixtureRoot = createWrapperFixture();
createRuntimeRotationProxyFixtureModule(fixtureRoot);
const fakeBin = createCustomFakeCodexBin(fixtureRoot, [
"#!/usr/bin/env node",
'const { spawnSync } = require("node:child_process");',
'const fs = require("node:fs");',
'const path = require("node:path");',
'const args = process.argv.slice(2);',
'if (args[0] === "app-server") {',
' console.log(`APP_SERVER_FORWARDED:${args.join(" ")}`);',
" process.exit(0);",
"}",
'const statePath = path.join(process.env.CODEX_HOME ?? "", "state_5.sqlite");',
'const originalState = path.join(process.env.ORIGINAL_CODEX_HOME ?? "", "state_5.sqlite");',
'console.log(`TUI_HOME_IS_ORIGINAL:${process.env.CODEX_HOME === process.env.ORIGINAL_CODEX_HOME}`);',
'console.log(`TUI_STATE_EXISTED:${fs.existsSync(statePath)}`);',
'fs.writeFileSync(statePath, "first-run-state\\n", "utf8");',
'console.log(`TUI_STATE_PERSISTED:${fs.readFileSync(originalState, "utf8").includes("first-run-state")}`);',
'console.log(`TUI_HAS_BASE_URL_OVERRIDE:${args.some((arg) => arg.includes("model_providers.codex-multi-auth-runtime-proxy.base_url="))}`);',
'console.log(`TUI_HAS_ENV_KEY_OVERRIDE:${args.includes(\'model_providers.codex-multi-auth-runtime-proxy.env_key="OPENAI_API_KEY"\')}`);',
'console.log(`TUI_HAS_AUTH_OVERRIDE:${args.includes("model_providers.codex-multi-auth-runtime-proxy.requires_openai_auth=false")}`);',
'console.log(`TUI_HAS_WIRE_OVERRIDE:${args.includes(\'model_providers.codex-multi-auth-runtime-proxy.wire_api="responses"\')}`);',
'console.log(`TUI_HAS_STORAGE_OVERRIDE:${args.includes("disable_response_storage=false")}`);',
'console.log(`TUI_KEY_IN_ARGS:${args.includes(process.env.OPENAI_API_KEY ?? "__missing__")}`);',
'const shimExe = path.join(process.env.CODEX_CLI_PATH ?? "", process.platform === "win32" ? "codex.exe" : "codex");',
'const shimResult = spawnSync(shimExe, ["app-server", "--canonical-shim"], { encoding: "utf8", env: process.env });',
'console.log(`TUI_SHIM_STATUS:${shimResult.status}`);',
'console.log(`TUI_SHIM_STDOUT:${(shimResult.stdout ?? "").trim()}`);',
"process.exit(0);",
]);
const originalHome = join(fixtureRoot, "codex-home");
const markerPath = join(fixtureRoot, "proxy-marker.txt");
mkdirSync(originalHome, { recursive: true });
writeFileSync(
join(originalHome, "config.toml"),
'model_provider = "openai"\n',
"utf8",
);

const result = runWrapper(fixtureRoot, [], {
CODEX_MULTI_AUTH_REAL_CODEX_BIN: fakeBin,
CODEX_HOME: originalHome,
ORIGINAL_CODEX_HOME: originalHome,
CODEX_MULTI_AUTH_RUNTIME_ROTATION_PROXY: "1",
CODEX_MULTI_AUTH_APP_ROTATION_IDLE_MS: "200",
CODEX_MULTI_AUTH_TEST_PROXY_MARKER: markerPath,
OPENAI_API_KEY: undefined,
});

const output = combinedOutput(result);
if (result.status !== 0) {
throw new Error(output);
}
expect(output).toContain("TUI_HOME_IS_ORIGINAL:true");
expect(output).toContain("TUI_STATE_EXISTED:false");
expect(output).toContain("TUI_STATE_PERSISTED:true");
expect(output).toContain("TUI_HAS_BASE_URL_OVERRIDE:true");
expect(output).toContain("TUI_HAS_ENV_KEY_OVERRIDE:true");
expect(output).toContain("TUI_HAS_AUTH_OVERRIDE:true");
expect(output).toContain("TUI_HAS_WIRE_OVERRIDE:true");
expect(output).toContain("TUI_HAS_STORAGE_OVERRIDE:true");
expect(output).toContain("TUI_KEY_IN_ARGS:false");
expect(output).toContain("TUI_SHIM_STATUS:0");
expect(output).toContain(
"APP_SERVER_FORWARDED:app-server --canonical-shim",
);
expect(output).toContain(
"model_providers.codex-multi-auth-runtime-proxy.base_url=",
);
expect(output).toContain(
'model_providers.codex-multi-auth-runtime-proxy.env_key="OPENAI_API_KEY"',
);
expect(output).toContain(
"model_providers.codex-multi-auth-runtime-proxy.requires_openai_auth=false",
);
expect(output).toContain(
'model_providers.codex-multi-auth-runtime-proxy.wire_api="responses"',
);
expect(output).toContain("disable_response_storage=false");
expect(output).toContain(
'model_provider="codex-multi-auth-runtime-proxy"',
);
expect(readFileSync(join(originalHome, "state_5.sqlite"), "utf8")).toBe(
"first-run-state\n",
);
expect(readFileSync(join(originalHome, "config.toml"), "utf8")).toBe(
'model_provider = "openai"\n',
);
await waitForFileText(
markerPath,
"start:http://127.0.0.1:4567\nclose\n",
);
});

Comment on lines +2763 to +2857

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

solid single-session coverage, but missing a concurrency regression case for canonical-home sharing.

see scripts/codex.js:3497-3528 for the root cause — canonical mode now shares the live codex home across processes with no isolation. as per path instructions, test/** should "demand regression cases that reproduce concurrency bugs" and this new test only exercises one TUI session at a time.

🧰 Tools
🪛 ast-grep (0.44.1)

[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { type SpawnSyncReturns, spawn, spawnSync } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(detect-child-process-typescript)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/codex-bin-wrapper.test.ts` around lines 2763 - 2857, The canonical Codex
home test covers only one interactive session and does not reproduce concurrent
access. Extend the test around “uses the canonical Codex home for interactive
TUI runtime routing” to launch overlapping TUI processes sharing the same
canonical home, coordinate their execution, and assert both complete without
state/config corruption. Preserve the existing routing and persistence
assertions while adding the concurrency regression coverage.

Source: Path instructions

it("stops detached TUI app helpers after failed launches", async () => {
const fixtureRoot = createWrapperFixture();
createRuntimeRotationProxyFixtureModule(fixtureRoot);
Expand Down