From 20f724281a7c33a0d832e6597bb4e64a021b8d73 Mon Sep 17 00:00:00 2001 From: comfuture Date: Tue, 11 Aug 2026 08:59:33 +0900 Subject: [PATCH 01/16] fix(codex): stop recursive launcher shims --- src/codex/shim.ts | 9 +++++ tests/codex-shim.test.ts | 80 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+) diff --git a/src/codex/shim.ts b/src/codex/shim.ts index c6a995655..c915ad2df 100644 --- a/src/codex/shim.ts +++ b/src/codex/shim.ts @@ -383,6 +383,15 @@ export function buildUnixCodexShim(realCodexPath: string, bunPath: string, cliPa const valueOptions = CODEX_GLOBAL_OPTIONS_WITH_VALUE.join("|"); return `#!/usr/bin/env sh # ${SHIM_MARKER} +if [ "\${OCX_SHIM_ACTIVE_PID:-}" = "$$" ]; then + printf '%s\n' 'opencodex: saved Codex launcher resolved back to the autostart shim; run ocx restore and reinstall Codex before enabling codexAutoStart.' >&2 + exit 126 +fi +# Dynamic launchers such as mise exec -- codex may resolve the command name +# back to this wrapper. An exec chain keeps the same PID; a legitimate nested +# Codex invocation starts a new process and is allowed to establish a new guard. +OCX_SHIM_ACTIVE_PID=$$ +export OCX_SHIM_ACTIVE_PID if [ -z "$OPENCODEX_API_AUTH_TOKEN" ] && [ -f ${shQuote(tokenFile)} ]; then OPENCODEX_API_AUTH_TOKEN="$(cat ${shQuote(tokenFile)})" export OPENCODEX_API_AUTH_TOKEN diff --git a/tests/codex-shim.test.ts b/tests/codex-shim.test.ts index 31137b245..180ad64a4 100644 --- a/tests/codex-shim.test.ts +++ b/tests/codex-shim.test.ts @@ -214,6 +214,86 @@ describe("Codex autostart shim", () => { expect(script).toContain("OCX_SHIM_BYPASS"); }); + test("Unix shim stops same-process re-entry through a dynamic launcher", () => { + if (process.platform === "win32") return; + + const dir = mkdtempSync(join(tmpdir(), "ocx-shim-reentry-")); + const bunPath = join(dir, "bun"); + const misePath = join(dir, "mise"); + const realCodexPath = join(dir, "codex.opencodex-real"); + const shimPath = join(dir, "codex"); + try { + writeFileSync(bunPath, "#!/usr/bin/env sh\nexit 0\n", "utf8"); + writeFileSync(misePath, `#!/usr/bin/env sh +if [ "$1" = exec ] && [ "$2" = -- ] && [ "$3" = codex ]; then + shift 3 + exec codex "$@" +fi +exit 64 +`, "utf8"); + writeFileSync(realCodexPath, `#!/usr/bin/env sh\nexec "${misePath}" exec -- codex "$@"\n`, "utf8"); + writeFileSync(shimPath, buildUnixCodexShim(realCodexPath, bunPath, "/cli.ts", "bundled"), "utf8"); + chmodSync(bunPath, 0o755); + chmodSync(misePath, 0o755); + chmodSync(realCodexPath, 0o755); + chmodSync(shimPath, 0o755); + const env = { ...process.env, PATH: `${dir}:${process.env.PATH ?? ""}`, OCX_SHIM_BYPASS: "1" }; + delete env.OCX_SHIM_ACTIVE_PID; + + const result = spawnSync(shimPath, ["--help"], { + encoding: "utf8", + env, + timeout: 2_000, + }); + + expect(result.error).toBeUndefined(); + expect(result.status).toBe(126); + expect(result.stderr).toContain("saved Codex launcher resolved back to the autostart shim"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + test("Unix shim permits a real Codex process to start a new child invocation", () => { + if (process.platform === "win32") return; + + const dir = mkdtempSync(join(tmpdir(), "ocx-shim-child-")); + const bunPath = join(dir, "bun"); + const realCodexPath = join(dir, "codex.opencodex-real"); + const shimPath = join(dir, "codex"); + try { + writeFileSync(bunPath, "#!/usr/bin/env sh\nexit 0\n", "utf8"); + writeFileSync(realCodexPath, `#!/usr/bin/env sh +if [ -z "$OCX_TEST_CHILD" ]; then + OCX_TEST_CHILD=1 + export OCX_TEST_CHILD + "${shimPath}" --version + exit $? +fi +printf '%s\\n' child-codex +`, "utf8"); + writeFileSync(shimPath, buildUnixCodexShim(realCodexPath, bunPath, "/cli.ts", "bundled"), "utf8"); + chmodSync(bunPath, 0o755); + chmodSync(realCodexPath, 0o755); + chmodSync(shimPath, 0o755); + const env = { ...process.env, OCX_SHIM_BYPASS: "1" }; + delete env.OCX_SHIM_ACTIVE_PID; + + const result = spawnSync(shimPath, ["--help"], { + encoding: "utf8", + env, + timeout: 2_000, + }); + + expect(result.error).toBeUndefined(); + expect(result.status).toBe(0); + expect(result.stdout).toBe("child-codex\n"); + expect(result.stderr).toBe(""); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + test("Windows shim uses bypass env var to skip proxy start", () => { const script = buildWindowsCodexShim("C:\\codex.exe", "C:\\bun.exe", "C:\\cli.ts", "bundled"); expect(script).toContain("OCX_SHIM_BYPASS"); From 9246d25e6215eda0874d3f1ade4cb4f4bba31832 Mon Sep 17 00:00:00 2001 From: comfuture Date: Tue, 11 Aug 2026 09:19:40 +0900 Subject: [PATCH 02/16] fix(codex): validate shim installs before commit --- src/codex/shim.ts | 55 +++++++++++++++++++++++++++-- tests/codex-shim.test.ts | 75 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 128 insertions(+), 2 deletions(-) diff --git a/src/codex/shim.ts b/src/codex/shim.ts index c915ad2df..b1f522ef6 100644 --- a/src/codex/shim.ts +++ b/src/codex/shim.ts @@ -1,4 +1,5 @@ import { randomUUID } from "node:crypto"; +import { spawnSync } from "node:child_process"; import { delimiter, dirname, extname, join, posix } from "node:path"; import { chmodSync, @@ -33,6 +34,9 @@ const CODEX_SHIM_PROBE_BYTES = 16 * 1024; export const CODEX_SHIM_REPLACEMENT_STABLE_MS = 100; export const CODEX_SHIM_STATE_MAX_BYTES = 1024 * 1024; const CODEX_SHIM_RESTORE_LOCK_STALE_MS = 30_000; +const CODEX_SHIM_INSTALL_PROBE_TIMEOUT_MS = 5_000; +const CODEX_SHIM_REENTRY_EXIT_CODE = 126; +const CODEX_SHIM_REENTRY_DIAGNOSTIC = "opencodex: saved Codex launcher resolved back to the autostart shim; run ocx restore and reinstall Codex before enabling codexAutoStart."; const MAX_DIAGNOSTIC_VALUE_BYTES = 8 * 1024; let lastShimDiscoveryError: string | null = null; /** Last human-readable reason discovery returned null (exposed for doctor/tests). */ @@ -384,8 +388,8 @@ export function buildUnixCodexShim(realCodexPath: string, bunPath: string, cliPa return `#!/usr/bin/env sh # ${SHIM_MARKER} if [ "\${OCX_SHIM_ACTIVE_PID:-}" = "$$" ]; then - printf '%s\n' 'opencodex: saved Codex launcher resolved back to the autostart shim; run ocx restore and reinstall Codex before enabling codexAutoStart.' >&2 - exit 126 + printf '%s\n' ${shQuote(CODEX_SHIM_REENTRY_DIAGNOSTIC)} >&2 + exit ${CODEX_SHIM_REENTRY_EXIT_CODE} fi # Dynamic launchers such as mise exec -- codex may resolve the command name # back to this wrapper. An exec chain keeps the same PID; a legitimate nested @@ -435,6 +439,40 @@ exec ${shQuote(realCodexPath)} "$@" `; } +function probeUnixShimInstall(wrapperPath: string): "recursive" | "timeout" | null { + if (process.platform === "win32") return null; + const env: NodeJS.ProcessEnv = { ...process.env, OCX_SHIM_BYPASS: "1" }; + delete env.OCX_SHIM_ACTIVE_PID; + const result = spawnSync("/bin/sh", [wrapperPath, "--version"], { + encoding: "utf8", + env, + timeout: CODEX_SHIM_INSTALL_PROBE_TIMEOUT_MS, + killSignal: "SIGKILL", + }); + if ((result.error as NodeJS.ErrnoException | undefined)?.code === "ETIMEDOUT") return "timeout"; + if (result.status === CODEX_SHIM_REENTRY_EXIT_CODE && result.stderr.includes(CODEX_SHIM_REENTRY_DIAGNOSTIC)) { + return "recursive"; + } + return null; +} + +function rollbackFreshShimInstall(targets: readonly ShimFileState[]): void { + const errors: Error[] = []; + for (const target of [...targets].reverse()) { + try { + if (!target.preserveOnly && existsSync(target.wrapperPath) && isShim(target.wrapperPath)) unlinkSync(target.wrapperPath); + } catch (error) { + errors.push(error instanceof Error ? error : new Error(String(error))); + } + try { + if (existsSync(target.backupPath) && !existsSync(target.originalPath)) renameSync(target.backupPath, target.originalPath); + } catch (error) { + errors.push(error instanceof Error ? error : new Error(String(error))); + } + } + if (errors.length > 0) throw new AggregateError(errors, "Codex shim install validation rollback failed"); +} + function windowsBatchValue(value: string): string { return value .replace(/%/g, "%%") @@ -1073,6 +1111,19 @@ function installCodexShimInternal(options: InstallCodexShimInternalOptions): { i if (existsSync(target.originalPath)) renameSync(target.originalPath, target.backupPath); if (!target.preserveOnly) writeShim(target.wrapperPath, target.realPath ?? target.backupPath); } + if (process.platform !== "win32") { + const unsafe = targets.map(target => probeUnixShimInstall(target.wrapperPath)).find(result => result !== null); + if (unsafe) { + rollbackFreshShimInstall(targets); + const reason = unsafe === "recursive" + ? "the saved launcher resolved back to the generated shim" + : `the saved launcher did not finish --version within ${CODEX_SHIM_INSTALL_PROBE_TIMEOUT_MS}ms`; + return { + installed: false, + message: `Refusing Codex autostart shim because ${reason}. The original launcher was restored; reinstall Codex as a concrete executable before enabling codexAutoStart.`, + }; + } + } writeState(primaryState(targets)); return { installed: true, diff --git a/tests/codex-shim.test.ts b/tests/codex-shim.test.ts index 180ad64a4..46b79e74a 100644 --- a/tests/codex-shim.test.ts +++ b/tests/codex-shim.test.ts @@ -254,6 +254,81 @@ exit 64 } }); + test("Unix install rejects a recursive dynamic launcher and restores the original", () => { + if (process.platform === "win32") return; + + const binDir = mkdtempSync(join(tmpdir(), "ocx-shim-install-reentry-bin-")); + const home = mkdtempSync(join(tmpdir(), "ocx-shim-install-reentry-home-")); + const oldPath = process.env.PATH; + const oldHome = process.env.OPENCODEX_HOME; + const codexPath = join(binDir, "codex"); + const misePath = join(binDir, "mise"); + const original = `#!/bin/sh\nexec "${misePath}" exec -- codex "$@"\n`; + try { + process.env.PATH = `${binDir}:${oldPath ?? ""}`; + process.env.OPENCODEX_HOME = home; + writeFileSync(misePath, `#!/bin/sh +if [ "$1" = exec ] && [ "$2" = -- ] && [ "$3" = codex ]; then + shift 3 + exec codex "$@" +fi +exit 64 +`, "utf8"); + writeFileSync(codexPath, original, "utf8"); + chmodSync(misePath, 0o755); + chmodSync(codexPath, 0o755); + + const installed = installCodexShim(); + + expect(installed.installed).toBe(false); + expect(installed.message).toContain("saved launcher resolved back to the generated shim"); + expect(installed.message).toContain("original launcher was restored"); + expect(readFileSync(codexPath, "utf8")).toBe(original); + expect(existsSync(`${codexPath}.opencodex-real`)).toBe(false); + expect(existsSync(join(home, "codex-shim.json"))).toBe(false); + } finally { + if (oldPath === undefined) delete process.env.PATH; + else process.env.PATH = oldPath; + if (oldHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = oldHome; + rmSync(binDir, { recursive: true, force: true }); + rmSync(home, { recursive: true, force: true }); + } + }); + + test("Unix install rolls back when launcher validation times out", () => { + if (process.platform === "win32") return; + + const binDir = mkdtempSync(join(tmpdir(), "ocx-shim-install-timeout-bin-")); + const home = mkdtempSync(join(tmpdir(), "ocx-shim-install-timeout-home-")); + const oldPath = process.env.PATH; + const oldHome = process.env.OPENCODEX_HOME; + const codexPath = join(binDir, "codex"); + const original = "#!/bin/sh\nexec /bin/sleep 30\n"; + try { + process.env.PATH = `${binDir}:${oldPath ?? ""}`; + process.env.OPENCODEX_HOME = home; + writeFileSync(codexPath, original, "utf8"); + chmodSync(codexPath, 0o755); + + const installed = installCodexShim(); + + expect(installed.installed).toBe(false); + expect(installed.message).toContain("did not finish --version within 5000ms"); + expect(installed.message).toContain("original launcher was restored"); + expect(readFileSync(codexPath, "utf8")).toBe(original); + expect(existsSync(`${codexPath}.opencodex-real`)).toBe(false); + expect(existsSync(join(home, "codex-shim.json"))).toBe(false); + } finally { + if (oldPath === undefined) delete process.env.PATH; + else process.env.PATH = oldPath; + if (oldHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = oldHome; + rmSync(binDir, { recursive: true, force: true }); + rmSync(home, { recursive: true, force: true }); + } + }, 10_000); + test("Unix shim permits a real Codex process to start a new child invocation", () => { if (process.platform === "win32") return; From 4655b02e3a5e5b98e0f5d84510d510790b729d53 Mon Sep 17 00:00:00 2001 From: comfuture Date: Tue, 11 Aug 2026 09:38:53 +0900 Subject: [PATCH 03/16] fix(codex): terminate recursive probe trees --- src/codex/shim.ts | 109 ++++++++++++++++++++++++++++++++++----- tests/codex-shim.test.ts | 103 +++++++++++++++++++++++++++++++++++- 2 files changed, 198 insertions(+), 14 deletions(-) diff --git a/src/codex/shim.ts b/src/codex/shim.ts index b1f522ef6..73abea0e9 100644 --- a/src/codex/shim.ts +++ b/src/codex/shim.ts @@ -1,5 +1,6 @@ import { randomUUID } from "node:crypto"; import { spawnSync } from "node:child_process"; +import { tmpdir } from "node:os"; import { delimiter, dirname, extname, join, posix } from "node:path"; import { chmodSync, @@ -35,8 +36,41 @@ export const CODEX_SHIM_REPLACEMENT_STABLE_MS = 100; export const CODEX_SHIM_STATE_MAX_BYTES = 1024 * 1024; const CODEX_SHIM_RESTORE_LOCK_STALE_MS = 30_000; const CODEX_SHIM_INSTALL_PROBE_TIMEOUT_MS = 5_000; +const CODEX_SHIM_INSTALL_PROBE_EXIT_TIMEOUT_MS = 1_000; const CODEX_SHIM_REENTRY_EXIT_CODE = 126; -const CODEX_SHIM_REENTRY_DIAGNOSTIC = "opencodex: saved Codex launcher resolved back to the autostart shim; run ocx restore and reinstall Codex before enabling codexAutoStart."; +const CODEX_SHIM_REENTRY_DIAGNOSTIC = "opencodex: saved Codex launcher resolved back to the autostart shim; run ocx codex-shim uninstall and reinstall Codex before enabling codexAutoStart."; +const CODEX_SHIM_INSTALL_PROBE_SCRIPT = ` +set -m +marker=$1 +group_file=$2 +wrapper=$3 +timeout_seconds=$4 + +"$wrapper" --version & +launcher_pid=$! +printf '%s\n' "$launcher_pid" > "$group_file" +( + /bin/sleep "$timeout_seconds" + printf '%s\n' timeout > "$marker" + kill -KILL -"$launcher_pid" 2>/dev/null || true +) & +watchdog_pid=$! + +wait "$launcher_pid" +launcher_status=$? +if [ -f "$marker" ]; then + wait "$watchdog_pid" 2>/dev/null || true + exit 124 +fi +if kill -0 -"$launcher_pid" 2>/dev/null; then + printf '%s\n' descendants > "$marker" + kill -KILL -"$launcher_pid" 2>/dev/null || true +fi +kill -KILL -"$watchdog_pid" 2>/dev/null || true +wait "$watchdog_pid" 2>/dev/null || true +if [ -f "$marker" ]; then exit 125; fi +exit "$launcher_status" +`; const MAX_DIAGNOSTIC_VALUE_BYTES = 8 * 1024; let lastShimDiscoveryError: string | null = null; /** Last human-readable reason discovery returned null (exposed for doctor/tests). */ @@ -439,21 +473,68 @@ exec ${shQuote(realCodexPath)} "$@" `; } -function probeUnixShimInstall(wrapperPath: string): "recursive" | "timeout" | null { +function probeUnixShimInstall(wrapperPath: string): "descendants" | "recursive" | "timeout" | null { if (process.platform === "win32") return null; const env: NodeJS.ProcessEnv = { ...process.env, OCX_SHIM_BYPASS: "1" }; delete env.OCX_SHIM_ACTIVE_PID; - const result = spawnSync("/bin/sh", [wrapperPath, "--version"], { - encoding: "utf8", - env, - timeout: CODEX_SHIM_INSTALL_PROBE_TIMEOUT_MS, - killSignal: "SIGKILL", - }); - if ((result.error as NodeJS.ErrnoException | undefined)?.code === "ETIMEDOUT") return "timeout"; - if (result.status === CODEX_SHIM_REENTRY_EXIT_CODE && result.stderr.includes(CODEX_SHIM_REENTRY_DIAGNOSTIC)) { - return "recursive"; + const probeId = `${process.pid}-${randomUUID()}`; + const markerPath = join(tmpdir(), `opencodex-shim-probe-${probeId}.result`); + const groupPath = join(tmpdir(), `opencodex-shim-probe-${probeId}.group`); + try { + const result = spawnSync("/bin/sh", [ + "-c", + CODEX_SHIM_INSTALL_PROBE_SCRIPT, + "opencodex-shim-probe", + markerPath, + groupPath, + wrapperPath, + String(CODEX_SHIM_INSTALL_PROBE_TIMEOUT_MS / 1_000), + ], { + encoding: "utf8", + env, + timeout: CODEX_SHIM_INSTALL_PROBE_TIMEOUT_MS + CODEX_SHIM_INSTALL_PROBE_EXIT_TIMEOUT_MS, + killSignal: "SIGKILL", + }); + const timedOut = (result.error as NodeJS.ErrnoException | undefined)?.code === "ETIMEDOUT"; + const marker = existsSync(markerPath) ? readFileSync(markerPath, "utf8").trim() : ""; + const groupId = existsSync(groupPath) ? Number.parseInt(readFileSync(groupPath, "utf8").trim(), 10) : 0; + if ((timedOut || marker) && Number.isInteger(groupId) && groupId > 0) terminateUnixProcessGroup(groupId); + if (timedOut || marker === "timeout") return "timeout"; + if (marker === "descendants") return "descendants"; + if (result.status === CODEX_SHIM_REENTRY_EXIT_CODE && result.stderr.includes(CODEX_SHIM_REENTRY_DIAGNOSTIC)) { + return "recursive"; + } + return null; + } finally { + for (const path of [markerPath, groupPath]) { + try { + if (existsSync(path)) unlinkSync(path); + } catch { /* best-effort cleanup of non-sensitive probe metadata */ } + } + } +} + +function unixProcessGroupAlive(groupId: number): boolean { + try { + process.kill(-groupId, 0); + return true; + } catch (error) { + return (error as NodeJS.ErrnoException).code !== "ESRCH"; + } +} + +function terminateUnixProcessGroup(groupId: number): void { + try { + process.kill(-groupId, "SIGKILL"); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ESRCH") throw error; + } + const deadline = Date.now() + CODEX_SHIM_INSTALL_PROBE_EXIT_TIMEOUT_MS; + const waiter = new Int32Array(new SharedArrayBuffer(4)); + while (Date.now() < deadline && unixProcessGroupAlive(groupId)) Atomics.wait(waiter, 0, 0, 10); + if (unixProcessGroupAlive(groupId)) { + throw new Error(`Codex shim install probe process group ${groupId} did not terminate`); } - return null; } function rollbackFreshShimInstall(targets: readonly ShimFileState[]): void { @@ -1117,7 +1198,9 @@ function installCodexShimInternal(options: InstallCodexShimInternalOptions): { i rollbackFreshShimInstall(targets); const reason = unsafe === "recursive" ? "the saved launcher resolved back to the generated shim" - : `the saved launcher did not finish --version within ${CODEX_SHIM_INSTALL_PROBE_TIMEOUT_MS}ms`; + : unsafe === "timeout" + ? `the saved launcher did not finish --version within ${CODEX_SHIM_INSTALL_PROBE_TIMEOUT_MS}ms` + : "the saved launcher left background descendants running after --version"; return { installed: false, message: `Refusing Codex autostart shim because ${reason}. The original launcher was restored; reinstall Codex as a concrete executable before enabling codexAutoStart.`, diff --git a/tests/codex-shim.test.ts b/tests/codex-shim.test.ts index 46b79e74a..9be10f958 100644 --- a/tests/codex-shim.test.ts +++ b/tests/codex-shim.test.ts @@ -249,6 +249,7 @@ exit 64 expect(result.error).toBeUndefined(); expect(result.status).toBe(126); expect(result.stderr).toContain("saved Codex launcher resolved back to the autostart shim"); + expect(result.stderr).toContain("ocx codex-shim uninstall"); } finally { rmSync(dir, { recursive: true, force: true }); } @@ -296,6 +297,56 @@ exit 64 } }); + test("Unix install rejects child-process redispatch and restores the original", () => { + if (process.platform === "win32") return; + + const binDir = mkdtempSync(join(tmpdir(), "ocx-shim-install-child-reentry-bin-")); + const home = mkdtempSync(join(tmpdir(), "ocx-shim-install-child-reentry-home-")); + const oldPath = process.env.PATH; + const oldHome = process.env.OPENCODEX_HOME; + const codexPath = join(binDir, "codex"); + const launcherPath = join(binDir, "dynamic-launcher"); + const childPidPath = join(home, "child-reentry.pid"); + const original = `#!/bin/sh\nexec "${launcherPath}" "$@"\n`; + try { + process.env.PATH = `${binDir}:${oldPath ?? ""}`; + process.env.OPENCODEX_HOME = home; + writeFileSync(launcherPath, `#!/bin/sh +if [ -n "$OCX_TEST_CHILD_REENTRY" ]; then + /bin/sleep 30 + exec codex "$@" +fi +OCX_TEST_CHILD_REENTRY=1 +export OCX_TEST_CHILD_REENTRY +codex "$@" & +child=$! +printf '%s\\n' "$child" > "${childPidPath}" +exit 0 +`, "utf8"); + writeFileSync(codexPath, original, "utf8"); + chmodSync(launcherPath, 0o755); + chmodSync(codexPath, 0o755); + + const installed = installCodexShim(); + + expect(installed.installed).toBe(false); + expect(installed.message).toContain("left background descendants running after --version"); + expect(readFileSync(codexPath, "utf8")).toBe(original); + expect(existsSync(`${codexPath}.opencodex-real`)).toBe(false); + expect(existsSync(join(home, "codex-shim.json"))).toBe(false); + const childPid = Number.parseInt(readFileSync(childPidPath, "utf8").trim(), 10); + const childState = spawnSync("/bin/ps", ["-o", "stat=", "-p", String(childPid)], { encoding: "utf8" }).stdout.trim(); + expect(childState === "" || childState.startsWith("Z")).toBe(true); + } finally { + if (oldPath === undefined) delete process.env.PATH; + else process.env.PATH = oldPath; + if (oldHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = oldHome; + rmSync(binDir, { recursive: true, force: true }); + rmSync(home, { recursive: true, force: true }); + } + }); + test("Unix install rolls back when launcher validation times out", () => { if (process.platform === "win32") return; @@ -304,7 +355,15 @@ exit 64 const oldPath = process.env.PATH; const oldHome = process.env.OPENCODEX_HOME; const codexPath = join(binDir, "codex"); - const original = "#!/bin/sh\nexec /bin/sleep 30\n"; + const childPidPath = join(home, "probe-child.pid"); + const groupIdPath = join(home, "probe-group.pid"); + const original = `#!/bin/sh +/bin/sleep 30 & +child=$! +printf '%s\\n' "$child" > "${childPidPath}" +printf '%s\\n' "$$" > "${groupIdPath}" +wait "$child" +`; try { process.env.PATH = `${binDir}:${oldPath ?? ""}`; process.env.OPENCODEX_HOME = home; @@ -319,6 +378,13 @@ exit 64 expect(readFileSync(codexPath, "utf8")).toBe(original); expect(existsSync(`${codexPath}.opencodex-real`)).toBe(false); expect(existsSync(join(home, "codex-shim.json"))).toBe(false); + const childPid = Number.parseInt(readFileSync(childPidPath, "utf8").trim(), 10); + const groupId = Number.parseInt(readFileSync(groupIdPath, "utf8").trim(), 10); + expect(Number.isInteger(childPid)).toBe(true); + expect(Number.isInteger(groupId)).toBe(true); + expect(() => process.kill(-groupId, 0)).toThrow(); + const childState = spawnSync("/bin/ps", ["-o", "stat=", "-p", String(childPid)], { encoding: "utf8" }).stdout.trim(); + expect(childState === "" || childState.startsWith("Z")).toBe(true); } finally { if (oldPath === undefined) delete process.env.PATH; else process.env.PATH = oldPath; @@ -329,6 +395,41 @@ exit 64 } }, 10_000); + test("Unix install preserves an existing backup without probing or mutation", () => { + if (process.platform === "win32") return; + + const binDir = mkdtempSync(join(tmpdir(), "ocx-shim-install-backup-bin-")); + const home = mkdtempSync(join(tmpdir(), "ocx-shim-install-backup-home-")); + const oldPath = process.env.PATH; + const oldHome = process.env.OPENCODEX_HOME; + const codexPath = join(binDir, "codex"); + const backupPath = `${codexPath}.opencodex-real`; + const original = "#!/bin/sh\nprintf '%s\\n' original\n"; + const backup = "#!/bin/sh\nprintf '%s\\n' preserved-backup\n"; + try { + process.env.PATH = `${binDir}:${oldPath ?? ""}`; + process.env.OPENCODEX_HOME = home; + writeFileSync(codexPath, original, "utf8"); + writeFileSync(backupPath, backup, "utf8"); + chmodSync(codexPath, 0o755); + chmodSync(backupPath, 0o755); + + const installed = installCodexShim(); + + expect(installed).toEqual({ installed: false, message: `Refusing to overwrite existing backup: ${backupPath}` }); + expect(readFileSync(codexPath, "utf8")).toBe(original); + expect(readFileSync(backupPath, "utf8")).toBe(backup); + expect(existsSync(join(home, "codex-shim.json"))).toBe(false); + } finally { + if (oldPath === undefined) delete process.env.PATH; + else process.env.PATH = oldPath; + if (oldHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = oldHome; + rmSync(binDir, { recursive: true, force: true }); + rmSync(home, { recursive: true, force: true }); + } + }); + test("Unix shim permits a real Codex process to start a new child invocation", () => { if (process.platform === "win32") return; From e647c13406f8acfb9da976061ce17a77f9915bb4 Mon Sep 17 00:00:00 2001 From: comfuture Date: Tue, 11 Aug 2026 10:13:55 +0900 Subject: [PATCH 04/16] fix(codex): validate shim refresh transactions --- src/codex/shim.ts | 53 +++++++++++++++++++++------ tests/codex-shim.test.ts | 79 +++++++++++++++++++++++++++++++++++++--- 2 files changed, 116 insertions(+), 16 deletions(-) diff --git a/src/codex/shim.ts b/src/codex/shim.ts index 73abea0e9..03b7a8847 100644 --- a/src/codex/shim.ts +++ b/src/codex/shim.ts @@ -46,11 +46,11 @@ group_file=$2 wrapper=$3 timeout_seconds=$4 -"$wrapper" --version & +/bin/sh "$wrapper" --version & launcher_pid=$! printf '%s\n' "$launcher_pid" > "$group_file" ( - /bin/sleep "$timeout_seconds" + /bin/sleep "$timeout_seconds" || exit 0 printf '%s\n' timeout > "$marker" kill -KILL -"$launcher_pid" 2>/dev/null || true ) & @@ -473,7 +473,9 @@ exec ${shQuote(realCodexPath)} "$@" `; } -function probeUnixShimInstall(wrapperPath: string): "descendants" | "recursive" | "timeout" | null { +type UnixShimProbeResult = "cleanup" | "descendants" | "recursive" | "timeout" | null; + +function probeUnixShimInstall(wrapperPath: string): UnixShimProbeResult { if (process.platform === "win32") return null; const env: NodeJS.ProcessEnv = { ...process.env, OCX_SHIM_BYPASS: "1" }; delete env.OCX_SHIM_ACTIVE_PID; @@ -498,7 +500,13 @@ function probeUnixShimInstall(wrapperPath: string): "descendants" | "recursive" const timedOut = (result.error as NodeJS.ErrnoException | undefined)?.code === "ETIMEDOUT"; const marker = existsSync(markerPath) ? readFileSync(markerPath, "utf8").trim() : ""; const groupId = existsSync(groupPath) ? Number.parseInt(readFileSync(groupPath, "utf8").trim(), 10) : 0; - if ((timedOut || marker) && Number.isInteger(groupId) && groupId > 0) terminateUnixProcessGroup(groupId); + if ((timedOut || marker) && Number.isInteger(groupId) && groupId > 0) { + try { + terminateUnixProcessGroup(groupId); + } catch { + return "cleanup"; + } + } if (timedOut || marker === "timeout") return "timeout"; if (marker === "descendants") return "descendants"; if (result.status === CODEX_SHIM_REENTRY_EXIT_CODE && result.stderr.includes(CODEX_SHIM_REENTRY_DIAGNOSTIC)) { @@ -514,6 +522,14 @@ function probeUnixShimInstall(wrapperPath: string): "descendants" | "recursive" } } +function probeUnixShimFiles(files: readonly ShimFileState[]): UnixShimProbeResult { + if (process.platform === "win32") return null; + return files + .filter(file => !file.preserveOnly) + .map(file => probeUnixShimInstall(file.wrapperPath)) + .find(result => result !== null) ?? null; +} + function unixProcessGroupAlive(groupId: number): boolean { try { process.kill(-groupId, 0); @@ -812,12 +828,20 @@ function refreshShimFile(file: ShimFileState): boolean { } if (existsSync(file.wrapperPath) && !isShim(file.wrapperPath)) { if (file.wrapperPath !== file.originalPath) return false; - replaceOwnedBackup(file.wrapperPath, file.backupPath); - writeShim(file.wrapperPath, file.realPath ?? file.backupPath); - return true; + const replacement = stableShimPathProbe(file.wrapperPath); + if (!replacement) return false; + return applyGuardedRefreshTransaction([{ + file, + expectedReplacement: replacement.fingerprint, + sourcePath: file.wrapperPath, + }]); } if (!existsSync(file.wrapperPath) && existsSync(file.backupPath)) { writeShim(file.wrapperPath, file.realPath ?? file.backupPath); + if (probeUnixShimFiles([file]) !== null) { + if (existsSync(file.wrapperPath) && isShim(file.wrapperPath)) unlinkSync(file.wrapperPath); + return false; + } return true; } if (file.originalPath !== file.wrapperPath && existsSync(file.originalPath) && existsSync(file.wrapperPath) && isShim(file.wrapperPath)) { @@ -1056,6 +1080,7 @@ function applyGuardedRefreshTransaction( const journal: GuardedRefreshJournalEntry[] = []; let applyError: Error | null = null; let fingerprintMismatch = false; + let unsafeLauncher = false; const transactionId = `${process.pid}-${++guardedRefreshTransactionId}`; for (const [index, operation] of operations.entries()) { @@ -1087,7 +1112,11 @@ function applyGuardedRefreshTransaction( } } - if (!fingerprintMismatch && !applyError && commitState) { + if (!fingerprintMismatch && !applyError) { + unsafeLauncher = probeUnixShimFiles(operations.map(operation => operation.file)) !== null; + } + + if (!fingerprintMismatch && !applyError && !unsafeLauncher && commitState) { try { commitState(); } catch (error) { @@ -1095,7 +1124,7 @@ function applyGuardedRefreshTransaction( } } - if (fingerprintMismatch || applyError) { + if (fingerprintMismatch || applyError || unsafeLauncher) { const rollbackErrors = rollbackGuardedRefresh(journal); if (applyError || rollbackErrors.length > 0) { throw new AggregateError( @@ -1193,14 +1222,16 @@ function installCodexShimInternal(options: InstallCodexShimInternalOptions): { i if (!target.preserveOnly) writeShim(target.wrapperPath, target.realPath ?? target.backupPath); } if (process.platform !== "win32") { - const unsafe = targets.map(target => probeUnixShimInstall(target.wrapperPath)).find(result => result !== null); + const unsafe = probeUnixShimFiles(targets); if (unsafe) { rollbackFreshShimInstall(targets); const reason = unsafe === "recursive" ? "the saved launcher resolved back to the generated shim" : unsafe === "timeout" ? `the saved launcher did not finish --version within ${CODEX_SHIM_INSTALL_PROBE_TIMEOUT_MS}ms` - : "the saved launcher left background descendants running after --version"; + : unsafe === "descendants" + ? "the saved launcher left background descendants running after --version" + : "the saved launcher's probe process group could not be terminated cleanly"; return { installed: false, message: `Refusing Codex autostart shim because ${reason}. The original launcher was restored; reinstall Codex as a concrete executable before enabling codexAutoStart.`, diff --git a/tests/codex-shim.test.ts b/tests/codex-shim.test.ts index 9be10f958..3bd7cd666 100644 --- a/tests/codex-shim.test.ts +++ b/tests/codex-shim.test.ts @@ -8,6 +8,20 @@ import { autoRestoreCodexShim, buildUnixCodexShim, buildWindowsCodexShim, buildW const SHIM_MARKER = "opencodex codex autostart shim"; const skipStabilityWait = () => {}; +function processState(pid: number): string { + return spawnSync("/bin/ps", ["-o", "stat=", "-p", String(pid)], { encoding: "utf8" }).stdout.trim(); +} + +function expectProcessGroupMissing(groupId: number): void { + let code: string | undefined; + try { + process.kill(-groupId, 0); + } catch (error) { + code = (error as NodeJS.ErrnoException).code; + } + expect(code).toBe("ESRCH"); +} + function withInstalledShim(run: (paths: { binDir: string; home: string; @@ -23,7 +37,7 @@ function withInstalledShim(run: (paths: { ? [join(binDir, "codex.cmd"), join(binDir, "codex.ps1"), join(binDir, "codex")] : [join(binDir, "codex")]; try { - process.env.PATH = binDir; + process.env.PATH = `${binDir}:${oldPath ?? ""}`; process.env.OPENCODEX_HOME = home; for (const wrapper of wrappers) { writeFileSync(wrapper, process.platform === "win32" ? `real ${wrapper}\n` : "#!/bin/sh\necho real\n", "utf8"); @@ -307,20 +321,27 @@ exit 64 const codexPath = join(binDir, "codex"); const launcherPath = join(binDir, "dynamic-launcher"); const childPidPath = join(home, "child-reentry.pid"); + const grandchildPidPath = join(home, "child-reentry-grandchild.pid"); + const groupIdPath = join(home, "child-reentry-group.pid"); const original = `#!/bin/sh\nexec "${launcherPath}" "$@"\n`; try { process.env.PATH = `${binDir}:${oldPath ?? ""}`; process.env.OPENCODEX_HOME = home; writeFileSync(launcherPath, `#!/bin/sh if [ -n "$OCX_TEST_CHILD_REENTRY" ]; then - /bin/sleep 30 + /bin/sleep 30 & + grandchild=$! + printf '%s\\n' "$grandchild" > "${grandchildPidPath}" + wait "$grandchild" exec codex "$@" fi OCX_TEST_CHILD_REENTRY=1 export OCX_TEST_CHILD_REENTRY +printf '%s\\n' "$$" > "${groupIdPath}" codex "$@" & child=$! printf '%s\\n' "$child" > "${childPidPath}" +while [ ! -f "${grandchildPidPath}" ]; do /bin/sleep 0.01; done exit 0 `, "utf8"); writeFileSync(codexPath, original, "utf8"); @@ -335,8 +356,13 @@ exit 0 expect(existsSync(`${codexPath}.opencodex-real`)).toBe(false); expect(existsSync(join(home, "codex-shim.json"))).toBe(false); const childPid = Number.parseInt(readFileSync(childPidPath, "utf8").trim(), 10); - const childState = spawnSync("/bin/ps", ["-o", "stat=", "-p", String(childPid)], { encoding: "utf8" }).stdout.trim(); + const grandchildPid = Number.parseInt(readFileSync(grandchildPidPath, "utf8").trim(), 10); + const groupId = Number.parseInt(readFileSync(groupIdPath, "utf8").trim(), 10); + expectProcessGroupMissing(groupId); + const childState = processState(childPid); + const grandchildState = processState(grandchildPid); expect(childState === "" || childState.startsWith("Z")).toBe(true); + expect(grandchildState === "" || grandchildState.startsWith("Z")).toBe(true); } finally { if (oldPath === undefined) delete process.env.PATH; else process.env.PATH = oldPath; @@ -382,8 +408,8 @@ wait "$child" const groupId = Number.parseInt(readFileSync(groupIdPath, "utf8").trim(), 10); expect(Number.isInteger(childPid)).toBe(true); expect(Number.isInteger(groupId)).toBe(true); - expect(() => process.kill(-groupId, 0)).toThrow(); - const childState = spawnSync("/bin/ps", ["-o", "stat=", "-p", String(childPid)], { encoding: "utf8" }).stdout.trim(); + expectProcessGroupMissing(groupId); + const childState = processState(childPid); expect(childState === "" || childState.startsWith("Z")).toBe(true); } finally { if (oldPath === undefined) delete process.env.PATH; @@ -701,6 +727,49 @@ printf '%s\\n' child-codex }); }); + test("guarded auto-restore rejects a recursive replacement and restores both launcher generations", () => { + if (process.platform === "win32") return; + withInstalledShim(({ binDir, wrappers, backups, statePath }) => { + const dynamicLauncher = join(binDir, "dynamic-codex-launcher"); + const replacement = `#!/bin/sh\nexec "${dynamicLauncher}" "$@"\n`; + const oldBackup = readFileSync(backups[0]); + const oldState = readFileSync(statePath); + writeFileSync(dynamicLauncher, "#!/bin/sh\nexec codex \"$@\"\n", "utf8"); + writeFileSync(wrappers[0], replacement, "utf8"); + chmodSync(dynamicLauncher, 0o755); + chmodSync(wrappers[0], 0o755); + + const result = autoRestoreCodexShim({ enabled: () => true, stabilitySleep: skipStabilityWait }); + + expect(result.status).toBe("deferred"); + expect(readFileSync(wrappers[0], "utf8")).toBe(replacement); + expect(readFileSync(backups[0])).toEqual(oldBackup); + expect(readFileSync(statePath)).toEqual(oldState); + }); + }); + + test("direct refresh rejects a recursive replacement without replacing the owned backup", () => { + if (process.platform === "win32") return; + withInstalledShim(({ binDir, wrappers, backups, statePath }) => { + const dynamicLauncher = join(binDir, "dynamic-codex-launcher"); + const replacement = `#!/bin/sh\nexec "${dynamicLauncher}" "$@"\n`; + const oldBackup = readFileSync(backups[0]); + const oldState = readFileSync(statePath); + writeFileSync(dynamicLauncher, "#!/bin/sh\nexec codex \"$@\"\n", "utf8"); + writeFileSync(wrappers[0], replacement, "utf8"); + chmodSync(dynamicLauncher, 0o755); + chmodSync(wrappers[0], 0o755); + + const result = installCodexShim(); + + expect(result.installed).toBe(false); + expect(result.message).toContain("Refusing to overwrite existing backup"); + expect(readFileSync(wrappers[0], "utf8")).toBe(replacement); + expect(readFileSync(backups[0])).toEqual(oldBackup); + expect(readFileSync(statePath)).toEqual(oldState); + }); + }); + test("an aged lock held by a live restore owner is never reclaimed", async () => { const binDir = mkdtempSync(join(tmpdir(), "ocx-shim-concurrent-bin-")); const home = mkdtempSync(join(tmpdir(), "ocx-shim-concurrent-home-")); From 7c61ef85332db10e2d824912de7664876a588e43 Mon Sep 17 00:00:00 2001 From: comfuture Date: Tue, 11 Aug 2026 10:28:09 +0900 Subject: [PATCH 05/16] fix(codex): reject failed launcher probes --- src/codex/shim.ts | 7 +++-- tests/codex-shim.test.ts | 67 ++++++++++++++++++++++++++++++++-------- 2 files changed, 59 insertions(+), 15 deletions(-) diff --git a/src/codex/shim.ts b/src/codex/shim.ts index 03b7a8847..a6a086ec5 100644 --- a/src/codex/shim.ts +++ b/src/codex/shim.ts @@ -473,7 +473,7 @@ exec ${shQuote(realCodexPath)} "$@" `; } -type UnixShimProbeResult = "cleanup" | "descendants" | "recursive" | "timeout" | null; +type UnixShimProbeResult = "cleanup" | "descendants" | "failed" | "recursive" | "timeout" | null; function probeUnixShimInstall(wrapperPath: string): UnixShimProbeResult { if (process.platform === "win32") return null; @@ -512,6 +512,7 @@ function probeUnixShimInstall(wrapperPath: string): UnixShimProbeResult { if (result.status === CODEX_SHIM_REENTRY_EXIT_CODE && result.stderr.includes(CODEX_SHIM_REENTRY_DIAGNOSTIC)) { return "recursive"; } + if (result.status !== 0) return "failed"; return null; } finally { for (const path of [markerPath, groupPath]) { @@ -1231,7 +1232,9 @@ function installCodexShimInternal(options: InstallCodexShimInternalOptions): { i ? `the saved launcher did not finish --version within ${CODEX_SHIM_INSTALL_PROBE_TIMEOUT_MS}ms` : unsafe === "descendants" ? "the saved launcher left background descendants running after --version" - : "the saved launcher's probe process group could not be terminated cleanly"; + : unsafe === "cleanup" + ? "the saved launcher's probe process group could not be terminated cleanly" + : "the saved launcher failed its --version probe"; return { installed: false, message: `Refusing Codex autostart shim because ${reason}. The original launcher was restored; reinstall Codex as a concrete executable before enabling codexAutoStart.`, diff --git a/tests/codex-shim.test.ts b/tests/codex-shim.test.ts index 3bd7cd666..bea7d027f 100644 --- a/tests/codex-shim.test.ts +++ b/tests/codex-shim.test.ts @@ -1,13 +1,21 @@ import { describe, expect, test } from "bun:test"; import { spawnSync } from "node:child_process"; import { chmodSync, mkdirSync, mkdtempSync, writeFileSync, readFileSync, existsSync, readdirSync, rmSync, statSync, symlinkSync, utimesSync } from "node:fs"; -import { dirname, join } from "node:path"; +import { delimiter, dirname, join } from "node:path"; import { tmpdir } from "node:os"; import { autoRestoreCodexShim, buildUnixCodexShim, buildWindowsCodexShim, buildWindowsPowerShellCodexShim, diagnoseCodexShim, findCodexOnPath, installCodexShim, isWindowsInteropDir, lastCodexDiscoveryError, uninstallCodexShim } from "../src/codex/shim"; const SHIM_MARKER = "opencodex codex autostart shim"; const skipStabilityWait = () => {}; +function prependPath(dir: string, current: string | undefined): string { + return [dir, current].filter(Boolean).join(delimiter); +} + +function successfulLauncher(label: string): string { + return process.platform === "win32" ? `${label}\r\n` : `#!/bin/sh\n# ${label}\nexit 0\n`; +} + function processState(pid: number): string { return spawnSync("/bin/ps", ["-o", "stat=", "-p", String(pid)], { encoding: "utf8" }).stdout.trim(); } @@ -37,7 +45,7 @@ function withInstalledShim(run: (paths: { ? [join(binDir, "codex.cmd"), join(binDir, "codex.ps1"), join(binDir, "codex")] : [join(binDir, "codex")]; try { - process.env.PATH = `${binDir}:${oldPath ?? ""}`; + process.env.PATH = prependPath(binDir, oldPath); process.env.OPENCODEX_HOME = home; for (const wrapper of wrappers) { writeFileSync(wrapper, process.platform === "win32" ? `real ${wrapper}\n` : "#!/bin/sh\necho real\n", "utf8"); @@ -251,13 +259,13 @@ exit 64 chmodSync(misePath, 0o755); chmodSync(realCodexPath, 0o755); chmodSync(shimPath, 0o755); - const env = { ...process.env, PATH: `${dir}:${process.env.PATH ?? ""}`, OCX_SHIM_BYPASS: "1" }; + const env = { ...process.env, PATH: prependPath(dir, process.env.PATH), OCX_SHIM_BYPASS: "1" }; delete env.OCX_SHIM_ACTIVE_PID; const result = spawnSync(shimPath, ["--help"], { encoding: "utf8", env, - timeout: 2_000, + timeout: 4_000, }); expect(result.error).toBeUndefined(); @@ -280,7 +288,7 @@ exit 64 const misePath = join(binDir, "mise"); const original = `#!/bin/sh\nexec "${misePath}" exec -- codex "$@"\n`; try { - process.env.PATH = `${binDir}:${oldPath ?? ""}`; + process.env.PATH = prependPath(binDir, oldPath); process.env.OPENCODEX_HOME = home; writeFileSync(misePath, `#!/bin/sh if [ "$1" = exec ] && [ "$2" = -- ] && [ "$3" = codex ]; then @@ -325,7 +333,7 @@ exit 64 const groupIdPath = join(home, "child-reentry-group.pid"); const original = `#!/bin/sh\nexec "${launcherPath}" "$@"\n`; try { - process.env.PATH = `${binDir}:${oldPath ?? ""}`; + process.env.PATH = prependPath(binDir, oldPath); process.env.OPENCODEX_HOME = home; writeFileSync(launcherPath, `#!/bin/sh if [ -n "$OCX_TEST_CHILD_REENTRY" ]; then @@ -391,7 +399,7 @@ printf '%s\\n' "$$" > "${groupIdPath}" wait "$child" `; try { - process.env.PATH = `${binDir}:${oldPath ?? ""}`; + process.env.PATH = prependPath(binDir, oldPath); process.env.OPENCODEX_HOME = home; writeFileSync(codexPath, original, "utf8"); chmodSync(codexPath, 0o755); @@ -433,7 +441,7 @@ wait "$child" const original = "#!/bin/sh\nprintf '%s\\n' original\n"; const backup = "#!/bin/sh\nprintf '%s\\n' preserved-backup\n"; try { - process.env.PATH = `${binDir}:${oldPath ?? ""}`; + process.env.PATH = prependPath(binDir, oldPath); process.env.OPENCODEX_HOME = home; writeFileSync(codexPath, original, "utf8"); writeFileSync(backupPath, backup, "utf8"); @@ -456,6 +464,38 @@ wait "$child" } }); + test("Unix install rolls back when the saved launcher fails its version probe", () => { + if (process.platform === "win32") return; + + const binDir = mkdtempSync(join(tmpdir(), "ocx-shim-install-failed-bin-")); + const home = mkdtempSync(join(tmpdir(), "ocx-shim-install-failed-home-")); + const oldPath = process.env.PATH; + const oldHome = process.env.OPENCODEX_HOME; + const codexPath = join(binDir, "codex"); + const original = "#!/bin/sh\nexit 127\n"; + try { + process.env.PATH = prependPath(binDir, oldPath); + process.env.OPENCODEX_HOME = home; + writeFileSync(codexPath, original, "utf8"); + chmodSync(codexPath, 0o755); + + const installed = installCodexShim(); + + expect(installed.installed).toBe(false); + expect(installed.message).toContain("saved launcher failed its --version probe"); + expect(readFileSync(codexPath, "utf8")).toBe(original); + expect(existsSync(`${codexPath}.opencodex-real`)).toBe(false); + expect(existsSync(join(home, "codex-shim.json"))).toBe(false); + } finally { + if (oldPath === undefined) delete process.env.PATH; + else process.env.PATH = oldPath; + if (oldHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = oldHome; + rmSync(binDir, { recursive: true, force: true }); + rmSync(home, { recursive: true, force: true }); + } + }); + test("Unix shim permits a real Codex process to start a new child invocation", () => { if (process.platform === "win32") return; @@ -716,7 +756,7 @@ printf '%s\\n' child-codex test("stable shim replacement restores through the shared install transaction", () => { withInstalledShim(({ wrappers, backups }) => { - const replacements = wrappers.map((wrapper, index) => `replacement-${index}\n`); + const replacements = wrappers.map((wrapper, index) => successfulLauncher(`replacement-${index}`)); wrappers.forEach((wrapper, index) => writeFileSync(wrapper, replacements[index], "utf8")); const result = autoRestoreCodexShim({ enabled: () => true, stabilitySleep: skipStabilityWait }); @@ -778,7 +818,7 @@ printf '%s\\n' child-codex const restoreLockPath = join(home, "codex-shim.autorestore.lock"); const wrapper = join(binDir, process.platform === "win32" ? "codex.cmd" : "codex"); const backup = join(binDir, process.platform === "win32" ? "codex.opencodex-real.cmd" : "codex.opencodex-real"); - const replacement = "concurrent replacement launcher\n"; + const replacement = successfulLauncher("concurrent replacement launcher"); const oldPath = process.env.PATH; const oldHome = process.env.OPENCODEX_HOME; let first: ReturnType | undefined; @@ -890,7 +930,7 @@ printf '%s\\n' child-codex withInstalledShim(({ home, wrappers, backups }) => { const lockPath = join(home, "codex-shim.autorestore.lock"); const ownerPath = join(lockPath, "dead-owner.json"); - const replacements = wrappers.map((_, index) => `dead-owner-replacement-${index}\n`); + const replacements = wrappers.map((_, index) => successfulLauncher(`dead-owner-replacement-${index}`)); wrappers.forEach((path, index) => writeFileSync(path, replacements[index], "utf8")); mkdirSync(lockPath); writeFileSync(ownerPath, `${JSON.stringify({ @@ -958,10 +998,11 @@ printf '%s\\n' child-codex test("opt-out set -> no restore and explicit install remains available", () => { withInstalledShim(({ wrappers }) => { - wrappers.forEach((wrapper, index) => writeFileSync(wrapper, `disabled-${index}\n`, "utf8")); + const replacements = wrappers.map((_, index) => successfulLauncher(`disabled-${index}`)); + wrappers.forEach((wrapper, index) => writeFileSync(wrapper, replacements[index], "utf8")); expect(autoRestoreCodexShim({ enabled: () => false, stabilitySleep: skipStabilityWait })).toEqual({ status: "disabled" }); - wrappers.forEach((wrapper, index) => expect(readFileSync(wrapper, "utf8")).toBe(`disabled-${index}\n`)); + wrappers.forEach((wrapper, index) => expect(readFileSync(wrapper, "utf8")).toBe(replacements[index])); expect(installCodexShim().installed).toBe(true); wrappers.forEach(wrapper => expect(readFileSync(wrapper, "utf8")).toContain(SHIM_MARKER)); }); From 568190d2103b613e3f891d9de2e04dc45277f2b5 Mon Sep 17 00:00:00 2001 From: comfuture Date: Tue, 11 Aug 2026 10:50:38 +0900 Subject: [PATCH 06/16] test(codex): cover failed refresh probes --- tests/codex-shim.test.ts | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tests/codex-shim.test.ts b/tests/codex-shim.test.ts index bea7d027f..d6da2d186 100644 --- a/tests/codex-shim.test.ts +++ b/tests/codex-shim.test.ts @@ -788,6 +788,23 @@ printf '%s\\n' child-codex }); }); + test("guarded auto-restore rejects a replacement that fails its version probe", () => { + if (process.platform === "win32") return; + withInstalledShim(({ wrappers, backups, statePath }) => { + const replacement = "#!/bin/sh\nexit 127\n"; + const oldBackup = readFileSync(backups[0]); + const oldState = readFileSync(statePath); + writeFileSync(wrappers[0], replacement, "utf8"); + + const result = autoRestoreCodexShim({ enabled: () => true, stabilitySleep: skipStabilityWait }); + + expect(result.status).toBe("deferred"); + expect(readFileSync(wrappers[0], "utf8")).toBe(replacement); + expect(readFileSync(backups[0])).toEqual(oldBackup); + expect(readFileSync(statePath)).toEqual(oldState); + }); + }); + test("direct refresh rejects a recursive replacement without replacing the owned backup", () => { if (process.platform === "win32") return; withInstalledShim(({ binDir, wrappers, backups, statePath }) => { @@ -810,6 +827,24 @@ printf '%s\\n' child-codex }); }); + test("direct refresh rejects a replacement that fails its version probe", () => { + if (process.platform === "win32") return; + withInstalledShim(({ wrappers, backups, statePath }) => { + const replacement = "#!/bin/sh\nexit 127\n"; + const oldBackup = readFileSync(backups[0]); + const oldState = readFileSync(statePath); + writeFileSync(wrappers[0], replacement, "utf8"); + + const result = installCodexShim(); + + expect(result.installed).toBe(false); + expect(result.message).toContain("Refusing to overwrite existing backup"); + expect(readFileSync(wrappers[0], "utf8")).toBe(replacement); + expect(readFileSync(backups[0])).toEqual(oldBackup); + expect(readFileSync(statePath)).toEqual(oldState); + }); + }); + test("an aged lock held by a live restore owner is never reclaimed", async () => { const binDir = mkdtempSync(join(tmpdir(), "ocx-shim-concurrent-bin-")); const home = mkdtempSync(join(tmpdir(), "ocx-shim-concurrent-home-")); From 60b9a3634ce93b2e5d0af67da0ef29086c20e45a Mon Sep 17 00:00:00 2001 From: comfuture Date: Tue, 11 Aug 2026 11:26:02 +0900 Subject: [PATCH 07/16] fix(codex): harden launcher probe transactions --- src/codex/shim.ts | 249 +++++++++++++++++++++++++++++++-------- tests/codex-shim.test.ts | 168 +++++++++++++++++++++++++- 2 files changed, 370 insertions(+), 47 deletions(-) diff --git a/src/codex/shim.ts b/src/codex/shim.ts index a6a086ec5..e13272e40 100644 --- a/src/codex/shim.ts +++ b/src/codex/shim.ts @@ -40,36 +40,88 @@ const CODEX_SHIM_INSTALL_PROBE_EXIT_TIMEOUT_MS = 1_000; const CODEX_SHIM_REENTRY_EXIT_CODE = 126; const CODEX_SHIM_REENTRY_DIAGNOSTIC = "opencodex: saved Codex launcher resolved back to the autostart shim; run ocx codex-shim uninstall and reinstall Codex before enabling codexAutoStart."; const CODEX_SHIM_INSTALL_PROBE_SCRIPT = ` -set -m -marker=$1 -group_file=$2 -wrapper=$3 -timeout_seconds=$4 - -/bin/sh "$wrapper" --version & -launcher_pid=$! -printf '%s\n' "$launcher_pid" > "$group_file" -( - /bin/sleep "$timeout_seconds" || exit 0 - printf '%s\n' timeout > "$marker" - kill -KILL -"$launcher_pid" 2>/dev/null || true -) & -watchdog_pid=$! - -wait "$launcher_pid" -launcher_status=$? -if [ -f "$marker" ]; then - wait "$watchdog_pid" 2>/dev/null || true - exit 124 -fi -if kill -0 -"$launcher_pid" 2>/dev/null; then - printf '%s\n' descendants > "$marker" - kill -KILL -"$launcher_pid" 2>/dev/null || true -fi -kill -KILL -"$watchdog_pid" 2>/dev/null || true -wait "$watchdog_pid" 2>/dev/null || true -if [ -f "$marker" ]; then exit 125; fi -exit "$launcher_status" +const { spawn } = require("node:child_process"); +const { writeFileSync } = require("node:fs"); +const [markerPath, groupPath, stderrPath, launcherShellPath, wrapperPath, timeoutRaw, stderrLimitRaw] = process.argv.slice(1); +const timeoutMs = Number.parseInt(timeoutRaw, 10); +const stderrLimit = Number.parseInt(stderrLimitRaw, 10); +const stderrChunks = []; +let stderrBytes = 0; +let launcher; +let timer; +let marker = ""; +let finished = false; + +function writeExclusive(path, value) { + writeFileSync(path, value, { flag: "wx", mode: 0o600 }); +} + +function appendStderr(value) { + if (stderrBytes >= stderrLimit) return; + const bytes = Buffer.from(value); + const retained = bytes.subarray(0, stderrLimit - stderrBytes); + stderrChunks.push(retained); + stderrBytes += retained.byteLength; +} + +function groupAlive() { + if (!launcher || !launcher.pid) return false; + try { + process.kill(-launcher.pid, 0); + return true; + } catch (error) { + return error && error.code !== "ESRCH"; + } +} + +function killGroup() { + if (!launcher || !launcher.pid) return; + try { process.kill(-launcher.pid, "SIGKILL"); } catch (error) { + if (!error || error.code !== "ESRCH") appendStderr(String(error)); + } +} + +function setMarker(value) { + if (marker) return; + marker = value; + try { writeExclusive(markerPath, value + "\\n"); } catch (error) { appendStderr(String(error)); } +} + +function finish(status) { + if (finished) return; + finished = true; + if (timer) clearTimeout(timer); + if (!marker && groupAlive()) { + setMarker("descendants"); + killGroup(); + } + try { writeExclusive(stderrPath, Buffer.concat(stderrChunks)); } catch { /* parent fails closed */ } + process.exit(marker === "timeout" ? 124 : marker === "descendants" ? 125 : status); +} + +try { + launcher = spawn(launcherShellPath, [wrapperPath, "--version"], { + detached: true, + env: process.env, + stdio: ["ignore", "ignore", "pipe"], + }); + if (!launcher.pid) throw new Error("Codex shim probe launcher has no pid"); + writeExclusive(groupPath, String(launcher.pid) + "\\n"); + launcher.stderr.on("data", appendStderr); + launcher.once("error", error => { + appendStderr(String(error)); + finish(127); + }); + launcher.once("exit", code => finish(Number.isInteger(code) ? code : 127)); + timer = setTimeout(() => { + setMarker("timeout"); + killGroup(); + }, timeoutMs); +} catch (error) { + appendStderr(String(error)); + killGroup(); + finish(127); +} `; const MAX_DIAGNOSTIC_VALUE_BYTES = 8 * 1024; let lastShimDiscoveryError: string | null = null; @@ -475,6 +527,30 @@ exec ${shQuote(realCodexPath)} "$@" type UnixShimProbeResult = "cleanup" | "descendants" | "failed" | "recursive" | "timeout" | null; +let codexShimProbeHookForTests: (() => void) | null = null; +let codexShimProbeShellForTests: string | null = null; + +/** Narrow deterministic seam for transaction rollback tests. */ +export function setCodexShimProbeHookForTests(hook: (() => void) | null): void { + codexShimProbeHookForTests = hook; +} + +/** Selects a POSIX shell only for cross-shell probe regression tests. */ +export function setCodexShimProbeShellForTests(path: string | null): void { + codexShimProbeShellForTests = path; +} + +function readProbeMetadata(path: string, maxBytes: number): string | null { + try { + if (!existsSync(path)) return ""; + const stat = lstatSync(path); + if (!stat.isFile() || stat.size > maxBytes) return null; + return readFileSync(path, "utf8").trim(); + } catch { + return null; + } +} + function probeUnixShimInstall(wrapperPath: string): UnixShimProbeResult { if (process.platform === "win32") return null; const env: NodeJS.ProcessEnv = { ...process.env, OCX_SHIM_BYPASS: "1" }; @@ -482,15 +558,19 @@ function probeUnixShimInstall(wrapperPath: string): UnixShimProbeResult { const probeId = `${process.pid}-${randomUUID()}`; const markerPath = join(tmpdir(), `opencodex-shim-probe-${probeId}.result`); const groupPath = join(tmpdir(), `opencodex-shim-probe-${probeId}.group`); + const stderrPath = join(tmpdir(), `opencodex-shim-probe-${probeId}.stderr`); + let groupId = 0; try { - const result = spawnSync("/bin/sh", [ - "-c", + const result = spawnSync(process.execPath, [ + "-e", CODEX_SHIM_INSTALL_PROBE_SCRIPT, - "opencodex-shim-probe", markerPath, groupPath, + stderrPath, + codexShimProbeShellForTests ?? "/bin/sh", wrapperPath, - String(CODEX_SHIM_INSTALL_PROBE_TIMEOUT_MS / 1_000), + String(CODEX_SHIM_INSTALL_PROBE_TIMEOUT_MS), + String(MAX_DIAGNOSTIC_VALUE_BYTES), ], { encoding: "utf8", env, @@ -498,24 +578,36 @@ function probeUnixShimInstall(wrapperPath: string): UnixShimProbeResult { killSignal: "SIGKILL", }); const timedOut = (result.error as NodeJS.ErrnoException | undefined)?.code === "ETIMEDOUT"; - const marker = existsSync(markerPath) ? readFileSync(markerPath, "utf8").trim() : ""; - const groupId = existsSync(groupPath) ? Number.parseInt(readFileSync(groupPath, "utf8").trim(), 10) : 0; - if ((timedOut || marker) && Number.isInteger(groupId) && groupId > 0) { + const marker = readProbeMetadata(markerPath, 64); + const groupText = readProbeMetadata(groupPath, 64); + const launcherStderr = readProbeMetadata(stderrPath, MAX_DIAGNOSTIC_VALUE_BYTES); + groupId = groupText === null ? 0 : Number.parseInt(groupText, 10); + if (marker === null || groupText === null || launcherStderr === null + || !Number.isInteger(groupId) || groupId <= 0) return "cleanup"; + const groupSurvived = unixProcessGroupAlive(groupId); + if (timedOut || marker || groupSurvived) { try { terminateUnixProcessGroup(groupId); } catch { return "cleanup"; } } + if (result.error && !timedOut) return "cleanup"; if (timedOut || marker === "timeout") return "timeout"; if (marker === "descendants") return "descendants"; - if (result.status === CODEX_SHIM_REENTRY_EXIT_CODE && result.stderr.includes(CODEX_SHIM_REENTRY_DIAGNOSTIC)) { + if (groupSurvived) return "cleanup"; + if (result.status === CODEX_SHIM_REENTRY_EXIT_CODE && launcherStderr.includes(CODEX_SHIM_REENTRY_DIAGNOSTIC)) { return "recursive"; } if (result.status !== 0) return "failed"; return null; + } catch { + if (Number.isInteger(groupId) && groupId > 0) { + try { terminateUnixProcessGroup(groupId); } catch { /* cleanup classification below */ } + } + return "cleanup"; } finally { - for (const path of [markerPath, groupPath]) { + for (const path of [markerPath, groupPath, stderrPath]) { try { if (existsSync(path)) unlinkSync(path); } catch { /* best-effort cleanup of non-sensitive probe metadata */ } @@ -525,6 +617,7 @@ function probeUnixShimInstall(wrapperPath: string): UnixShimProbeResult { function probeUnixShimFiles(files: readonly ShimFileState[]): UnixShimProbeResult { if (process.platform === "win32") return null; + codexShimProbeHookForTests?.(); return files .filter(file => !file.preserveOnly) .map(file => probeUnixShimInstall(file.wrapperPath)) @@ -839,8 +932,16 @@ function refreshShimFile(file: ShimFileState): boolean { } if (!existsSync(file.wrapperPath) && existsSync(file.backupPath)) { writeShim(file.wrapperPath, file.realPath ?? file.backupPath); - if (probeUnixShimFiles([file]) !== null) { + let unsafe: UnixShimProbeResult = null; + let probeError: Error | null = null; + try { + unsafe = probeUnixShimFiles([file]); + } catch (error) { + probeError = error instanceof Error ? error : new Error(String(error)); + } + if (unsafe !== null || probeError) { if (existsSync(file.wrapperPath) && isShim(file.wrapperPath)) unlinkSync(file.wrapperPath); + if (probeError) throw probeError; return false; } return true; @@ -862,7 +963,9 @@ interface GuardedRefreshOperation { interface GuardedRefreshJournalEntry { operation: GuardedRefreshOperation; stagedOldBackupPath?: string; + movedReplacementFingerprint?: ShimPathFingerprint; replacementMovedToBackup: boolean; + writtenWrapperFingerprint?: ShimPathFingerprint; wrapperWriteStarted: boolean; } @@ -1054,14 +1157,33 @@ function rollbackGuardedRefresh(journal: readonly GuardedRefreshJournalEntry[]): } }; for (const entry of [...journal].reverse()) { + let sourceOccupied = false; attempt(() => { - if (entry.wrapperWriteStarted && existsSync(entry.operation.file.wrapperPath)) { + const wrapper = stableShimPathProbe(entry.operation.file.wrapperPath); + const ownsWrapper = entry.wrapperWriteStarted + && entry.writtenWrapperFingerprint !== undefined + && wrapper !== null + && sameFingerprint(wrapper.fingerprint, entry.writtenWrapperFingerprint); + if (ownsWrapper) { unlinkSync(entry.operation.file.wrapperPath); + } else { + try { + lstatSync(entry.operation.sourcePath); + sourceOccupied = true; + } catch (error) { + if (fileErrorCode(error) !== "ENOENT") sourceOccupied = true; + } } }); attempt(() => { if (entry.replacementMovedToBackup && existsSync(entry.operation.file.backupPath)) { - renameSync(entry.operation.file.backupPath, entry.operation.sourcePath); + const movedReplacement = stableShimPathProbe(entry.operation.file.backupPath); + if (!movedReplacement || !entry.movedReplacementFingerprint + || !sameFingerprint(movedReplacement.fingerprint, entry.movedReplacementFingerprint)) { + throw new Error("Codex shim guarded refresh backup changed during rollback"); + } + if (sourceOccupied) unlinkSync(entry.operation.file.backupPath); + else renameSync(entry.operation.file.backupPath, entry.operation.sourcePath); } }); attempt(() => { @@ -1082,6 +1204,7 @@ function applyGuardedRefreshTransaction( let applyError: Error | null = null; let fingerprintMismatch = false; let unsafeLauncher = false; + let wrapperChangedDuringProbe = false; const transactionId = `${process.pid}-${++guardedRefreshTransactionId}`; for (const [index, operation] of operations.entries()) { @@ -1105,8 +1228,16 @@ function applyGuardedRefreshTransaction( } renameSync(operation.sourcePath, operation.file.backupPath); entry.replacementMovedToBackup = true; + const movedReplacement = stableShimPathProbe(operation.file.backupPath); + if (!movedReplacement) throw new Error("Codex shim guarded refresh could not fingerprint the staged launcher"); + entry.movedReplacementFingerprint = movedReplacement.fingerprint; entry.wrapperWriteStarted = true; writeShim(operation.file.wrapperPath, operation.file.realPath ?? operation.file.backupPath); + const writtenWrapper = stableShimPathProbe(operation.file.wrapperPath); + if (!writtenWrapper || !writtenWrapper.prefix.includes(SHIM_MARKER)) { + throw new Error("Codex shim guarded refresh could not fingerprint the generated wrapper"); + } + entry.writtenWrapperFingerprint = writtenWrapper.fingerprint; } catch (error) { applyError = error instanceof Error ? error : new Error(String(error)); break; @@ -1114,10 +1245,22 @@ function applyGuardedRefreshTransaction( } if (!fingerprintMismatch && !applyError) { - unsafeLauncher = probeUnixShimFiles(operations.map(operation => operation.file)) !== null; + try { + unsafeLauncher = probeUnixShimFiles(operations.map(operation => operation.file)) !== null; + } catch (error) { + applyError = error instanceof Error ? error : new Error(String(error)); + } + } + + if (!fingerprintMismatch && !applyError && !unsafeLauncher) { + wrapperChangedDuringProbe = journal.some(entry => { + const wrapper = stableShimPathProbe(entry.operation.file.wrapperPath); + return !wrapper || !entry.writtenWrapperFingerprint + || !sameFingerprint(wrapper.fingerprint, entry.writtenWrapperFingerprint); + }); } - if (!fingerprintMismatch && !applyError && !unsafeLauncher && commitState) { + if (!fingerprintMismatch && !applyError && !unsafeLauncher && !wrapperChangedDuringProbe && commitState) { try { commitState(); } catch (error) { @@ -1125,7 +1268,7 @@ function applyGuardedRefreshTransaction( } } - if (fingerprintMismatch || applyError || unsafeLauncher) { + if (fingerprintMismatch || applyError || unsafeLauncher || wrapperChangedDuringProbe) { const rollbackErrors = rollbackGuardedRefresh(journal); if (applyError || rollbackErrors.length > 0) { throw new AggregateError( @@ -1223,7 +1366,21 @@ function installCodexShimInternal(options: InstallCodexShimInternalOptions): { i if (!target.preserveOnly) writeShim(target.wrapperPath, target.realPath ?? target.backupPath); } if (process.platform !== "win32") { - const unsafe = probeUnixShimFiles(targets); + let unsafe: UnixShimProbeResult = null; + let probeError: Error | null = null; + try { + unsafe = probeUnixShimFiles(targets); + } catch (error) { + probeError = error instanceof Error ? error : new Error(String(error)); + } + if (probeError) { + try { + rollbackFreshShimInstall(targets); + } catch (rollbackError) { + throw new AggregateError([probeError, rollbackError], "Codex shim probe and install rollback failed"); + } + throw probeError; + } if (unsafe) { rollbackFreshShimInstall(targets); const reason = unsafe === "recursive" diff --git a/tests/codex-shim.test.ts b/tests/codex-shim.test.ts index d6da2d186..b2fed6165 100644 --- a/tests/codex-shim.test.ts +++ b/tests/codex-shim.test.ts @@ -3,7 +3,7 @@ import { spawnSync } from "node:child_process"; import { chmodSync, mkdirSync, mkdtempSync, writeFileSync, readFileSync, existsSync, readdirSync, rmSync, statSync, symlinkSync, utimesSync } from "node:fs"; import { delimiter, dirname, join } from "node:path"; import { tmpdir } from "node:os"; -import { autoRestoreCodexShim, buildUnixCodexShim, buildWindowsCodexShim, buildWindowsPowerShellCodexShim, diagnoseCodexShim, findCodexOnPath, installCodexShim, isWindowsInteropDir, lastCodexDiscoveryError, uninstallCodexShim } from "../src/codex/shim"; +import { autoRestoreCodexShim, buildUnixCodexShim, buildWindowsCodexShim, buildWindowsPowerShellCodexShim, diagnoseCodexShim, findCodexOnPath, installCodexShim, isWindowsInteropDir, lastCodexDiscoveryError, setCodexShimProbeHookForTests, setCodexShimProbeShellForTests, uninstallCodexShim } from "../src/codex/shim"; const SHIM_MARKER = "opencodex codex autostart shim"; const skipStabilityWait = () => {}; @@ -319,6 +319,38 @@ exit 64 } }); + test("Unix install accepts a valid launcher when the probe shell is dash", () => { + if (process.platform === "win32" || !existsSync("/bin/dash")) return; + + const binDir = mkdtempSync(join(tmpdir(), "ocx-shim-install-dash-bin-")); + const home = mkdtempSync(join(tmpdir(), "ocx-shim-install-dash-home-")); + const oldPath = process.env.PATH; + const oldHome = process.env.OPENCODEX_HOME; + const codexPath = join(binDir, "codex"); + try { + process.env.PATH = prependPath(binDir, oldPath); + process.env.OPENCODEX_HOME = home; + writeFileSync(codexPath, successfulLauncher("dash-valid-launcher"), "utf8"); + chmodSync(codexPath, 0o755); + setCodexShimProbeShellForTests("/bin/dash"); + + const installed = installCodexShim(); + + expect(installed.installed).toBe(true); + expect(readFileSync(codexPath, "utf8")).toContain(SHIM_MARKER); + expect(readFileSync(`${codexPath}.opencodex-real`, "utf8")).toBe(successfulLauncher("dash-valid-launcher")); + expect(existsSync(join(home, "codex-shim.json"))).toBe(true); + } finally { + setCodexShimProbeShellForTests(null); + if (oldPath === undefined) delete process.env.PATH; + else process.env.PATH = oldPath; + if (oldHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = oldHome; + rmSync(binDir, { recursive: true, force: true }); + rmSync(home, { recursive: true, force: true }); + } + }); + test("Unix install rejects child-process redispatch and restores the original", () => { if (process.platform === "win32") return; @@ -496,6 +528,38 @@ wait "$child" } }); + test("Unix install rolls back when probe infrastructure throws", () => { + if (process.platform === "win32") return; + + const binDir = mkdtempSync(join(tmpdir(), "ocx-shim-install-probe-error-bin-")); + const home = mkdtempSync(join(tmpdir(), "ocx-shim-install-probe-error-home-")); + const oldPath = process.env.PATH; + const oldHome = process.env.OPENCODEX_HOME; + const codexPath = join(binDir, "codex"); + const original = successfulLauncher("probe-error-original"); + try { + process.env.PATH = prependPath(binDir, oldPath); + process.env.OPENCODEX_HOME = home; + writeFileSync(codexPath, original, "utf8"); + chmodSync(codexPath, 0o755); + setCodexShimProbeHookForTests(() => { throw new Error("synthetic probe infrastructure failure"); }); + + expect(() => installCodexShim()).toThrow("synthetic probe infrastructure failure"); + + expect(readFileSync(codexPath, "utf8")).toBe(original); + expect(existsSync(`${codexPath}.opencodex-real`)).toBe(false); + expect(existsSync(join(home, "codex-shim.json"))).toBe(false); + } finally { + setCodexShimProbeHookForTests(null); + if (oldPath === undefined) delete process.env.PATH; + else process.env.PATH = oldPath; + if (oldHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = oldHome; + rmSync(binDir, { recursive: true, force: true }); + rmSync(home, { recursive: true, force: true }); + } + }); + test("Unix shim permits a real Codex process to start a new child invocation", () => { if (process.platform === "win32") return; @@ -805,6 +869,57 @@ printf '%s\\n' child-codex }); }); + test("guarded auto-restore rolls back when probe infrastructure throws", () => { + if (process.platform === "win32") return; + withInstalledShim(({ wrappers, backups, statePath }) => { + const replacement = successfulLauncher("guarded-probe-error-replacement"); + const oldBackup = readFileSync(backups[0]); + const oldState = readFileSync(statePath); + writeFileSync(wrappers[0], replacement, "utf8"); + setCodexShimProbeHookForTests(() => { throw new Error("synthetic guarded probe failure"); }); + + let failure: unknown; + try { + autoRestoreCodexShim({ enabled: () => true, stabilitySleep: skipStabilityWait }); + } catch (error) { + failure = error; + } finally { + setCodexShimProbeHookForTests(null); + } + + expect(failure).toBeInstanceOf(AggregateError); + expect((failure as AggregateError).errors.map(error => String(error))).toContain("Error: synthetic guarded probe failure"); + expect(readFileSync(wrappers[0], "utf8")).toBe(replacement); + expect(readFileSync(backups[0])).toEqual(oldBackup); + expect(readFileSync(statePath)).toEqual(oldState); + }); + }); + + test("guarded auto-restore preserves a concurrent wrapper replacement after a successful probe", () => { + if (process.platform === "win32") return; + withInstalledShim(({ binDir, wrappers, backups, statePath }) => { + const concurrent = successfulLauncher("concurrent updater replacement"); + const replacement = `#!/bin/sh +if [ "$1" = "--version" ]; then + printf '%s\\n' '#!/bin/sh' '# concurrent updater replacement' 'exit 0' > "${wrappers[0]}" + chmod 755 "${wrappers[0]}" +fi +exit 0 +`; + const oldBackup = readFileSync(backups[0]); + const oldState = readFileSync(statePath); + writeFileSync(wrappers[0], replacement, "utf8"); + + const result = autoRestoreCodexShim({ enabled: () => true, stabilitySleep: skipStabilityWait }); + + expect(result.status).toBe("deferred"); + expect(readFileSync(wrappers[0], "utf8")).toBe(concurrent); + expect(readFileSync(backups[0])).toEqual(oldBackup); + expect(readFileSync(statePath)).toEqual(oldState); + expect(readdirSync(binDir).filter(name => name.includes(".autorestore-"))).toEqual([]); + }); + }); + test("direct refresh rejects a recursive replacement without replacing the owned backup", () => { if (process.platform === "win32") return; withInstalledShim(({ binDir, wrappers, backups, statePath }) => { @@ -845,6 +960,57 @@ printf '%s\\n' child-codex }); }); + test("direct refresh rolls back when probe infrastructure throws", () => { + if (process.platform === "win32") return; + withInstalledShim(({ wrappers, backups, statePath }) => { + const replacement = successfulLauncher("direct-probe-error-replacement"); + const oldBackup = readFileSync(backups[0]); + const oldState = readFileSync(statePath); + writeFileSync(wrappers[0], replacement, "utf8"); + setCodexShimProbeHookForTests(() => { throw new Error("synthetic direct probe failure"); }); + + let failure: unknown; + try { + installCodexShim(); + } catch (error) { + failure = error; + } finally { + setCodexShimProbeHookForTests(null); + } + + expect(failure).toBeInstanceOf(AggregateError); + expect((failure as AggregateError).errors.map(error => String(error))).toContain("Error: synthetic direct probe failure"); + expect(readFileSync(wrappers[0], "utf8")).toBe(replacement); + expect(readFileSync(backups[0])).toEqual(oldBackup); + expect(readFileSync(statePath)).toEqual(oldState); + }); + }); + + test("direct refresh preserves a concurrent wrapper replacement after an unsafe probe", () => { + if (process.platform === "win32") return; + withInstalledShim(({ binDir, wrappers, backups, statePath }) => { + const concurrent = successfulLauncher("concurrent unsafe updater replacement"); + const replacement = `#!/bin/sh +if [ "$1" = "--version" ]; then + printf '%s\\n' '#!/bin/sh' '# concurrent unsafe updater replacement' 'exit 0' > "${wrappers[0]}" + chmod 755 "${wrappers[0]}" +fi +exit 127 +`; + const oldBackup = readFileSync(backups[0]); + const oldState = readFileSync(statePath); + writeFileSync(wrappers[0], replacement, "utf8"); + + const result = installCodexShim(); + + expect(result.installed).toBe(false); + expect(readFileSync(wrappers[0], "utf8")).toBe(concurrent); + expect(readFileSync(backups[0])).toEqual(oldBackup); + expect(readFileSync(statePath)).toEqual(oldState); + expect(readdirSync(binDir).filter(name => name.includes(".autorestore-"))).toEqual([]); + }); + }); + test("an aged lock held by a live restore owner is never reclaimed", async () => { const binDir = mkdtempSync(join(tmpdir(), "ocx-shim-concurrent-bin-")); const home = mkdtempSync(join(tmpdir(), "ocx-shim-concurrent-home-")); From d7d2a6456bff36f70661f5573f7bf9e5b87d424d Mon Sep 17 00:00:00 2001 From: comfuture Date: Tue, 11 Aug 2026 11:33:20 +0900 Subject: [PATCH 08/16] fix(codex): revalidate fresh shim installs --- src/codex/shim.ts | 109 +++++++++++++++++++++++++++++++++++---- tests/codex-shim.test.ts | 39 ++++++++++++++ 2 files changed, 137 insertions(+), 11 deletions(-) diff --git a/src/codex/shim.ts b/src/codex/shim.ts index e13272e40..965ede9d1 100644 --- a/src/codex/shim.ts +++ b/src/codex/shim.ts @@ -647,16 +647,52 @@ function terminateUnixProcessGroup(groupId: number): void { } } -function rollbackFreshShimInstall(targets: readonly ShimFileState[]): void { +interface FreshShimInstallJournalEntry { + target: ShimFileState; + movedOriginalFingerprint?: ShimPathFingerprint; + originalMovedToBackup: boolean; + writtenWrapperFingerprint?: ShimPathFingerprint; + wrapperWriteStarted: boolean; +} + +function rollbackFreshShimInstall(journal: readonly FreshShimInstallJournalEntry[]): void { const errors: Error[] = []; - for (const target of [...targets].reverse()) { + for (const entry of [...journal].reverse()) { + const target = entry.target; + let sourceOccupied = false; try { - if (!target.preserveOnly && existsSync(target.wrapperPath) && isShim(target.wrapperPath)) unlinkSync(target.wrapperPath); + const wrapper = stableShimPathProbe(target.wrapperPath); + const ownsWrapper = !target.preserveOnly + && entry.wrapperWriteStarted + && entry.writtenWrapperFingerprint !== undefined + && wrapper !== null + && sameFingerprint(wrapper.fingerprint, entry.writtenWrapperFingerprint); + if (ownsWrapper) unlinkSync(target.wrapperPath); + else { + try { + lstatSync(target.originalPath); + sourceOccupied = true; + } catch (error) { + if (fileErrorCode(error) !== "ENOENT") sourceOccupied = true; + } + } } catch (error) { errors.push(error instanceof Error ? error : new Error(String(error))); } try { - if (existsSync(target.backupPath) && !existsSync(target.originalPath)) renameSync(target.backupPath, target.originalPath); + if (entry.originalMovedToBackup && existsSync(target.backupPath)) { + const movedOriginal = stableShimPathProbe(target.backupPath); + if (!movedOriginal || !entry.movedOriginalFingerprint + || !sameFingerprint(movedOriginal.fingerprint, entry.movedOriginalFingerprint)) { + throw new Error("Codex shim fresh-install backup changed during rollback"); + } + if (sourceOccupied) { + if (!entry.writtenWrapperFingerprint) { + throw new Error("Codex shim fresh-install wrapper ownership changed during rollback"); + } + unlinkSync(target.backupPath); + } else renameSync(target.backupPath, target.originalPath); + } } catch (error) { errors.push(error instanceof Error ? error : new Error(String(error))); } @@ -1361,11 +1397,50 @@ function installCodexShimInternal(options: InstallCodexShimInternalOptions): { i for (const target of targets) { if (existsSync(target.backupPath)) return { installed: false, message: `Refusing to overwrite existing backup: ${target.backupPath}` }; } + const freshJournal: FreshShimInstallJournalEntry[] = []; + let freshApplyError: Error | null = null; for (const target of targets) { - if (existsSync(target.originalPath)) renameSync(target.originalPath, target.backupPath); - if (!target.preserveOnly) writeShim(target.wrapperPath, target.realPath ?? target.backupPath); + const entry: FreshShimInstallJournalEntry = { + target, + originalMovedToBackup: false, + wrapperWriteStarted: false, + }; + freshJournal.push(entry); + try { + if (existsSync(target.originalPath)) { + renameSync(target.originalPath, target.backupPath); + entry.originalMovedToBackup = true; + if (process.platform !== "win32") { + const movedOriginal = stableShimPathProbe(target.backupPath); + if (!movedOriginal) throw new Error("Codex shim fresh install could not fingerprint the staged launcher"); + entry.movedOriginalFingerprint = movedOriginal.fingerprint; + } + } + if (!target.preserveOnly) { + entry.wrapperWriteStarted = true; + writeShim(target.wrapperPath, target.realPath ?? target.backupPath); + if (process.platform !== "win32") { + const writtenWrapper = stableShimPathProbe(target.wrapperPath); + if (!writtenWrapper || !writtenWrapper.prefix.includes(SHIM_MARKER)) { + throw new Error("Codex shim fresh install could not fingerprint the generated wrapper"); + } + entry.writtenWrapperFingerprint = writtenWrapper.fingerprint; + } + } + } catch (error) { + freshApplyError = error instanceof Error ? error : new Error(String(error)); + break; + } } if (process.platform !== "win32") { + if (freshApplyError) { + try { + rollbackFreshShimInstall(freshJournal); + } catch (rollbackError) { + throw new AggregateError([freshApplyError, rollbackError], "Codex shim installation and rollback failed"); + } + throw freshApplyError; + } let unsafe: UnixShimProbeResult = null; let probeError: Error | null = null; try { @@ -1375,15 +1450,23 @@ function installCodexShimInternal(options: InstallCodexShimInternalOptions): { i } if (probeError) { try { - rollbackFreshShimInstall(targets); + rollbackFreshShimInstall(freshJournal); } catch (rollbackError) { throw new AggregateError([probeError, rollbackError], "Codex shim probe and install rollback failed"); } throw probeError; } - if (unsafe) { - rollbackFreshShimInstall(targets); - const reason = unsafe === "recursive" + const wrapperChangedDuringProbe = freshJournal.some(entry => { + if (entry.target.preserveOnly) return false; + const wrapper = stableShimPathProbe(entry.target.wrapperPath); + return !wrapper || !entry.writtenWrapperFingerprint + || !sameFingerprint(wrapper.fingerprint, entry.writtenWrapperFingerprint); + }); + if (unsafe || wrapperChangedDuringProbe) { + rollbackFreshShimInstall(freshJournal); + const reason = wrapperChangedDuringProbe + ? "the generated wrapper changed during its validation probe" + : unsafe === "recursive" ? "the saved launcher resolved back to the generated shim" : unsafe === "timeout" ? `the saved launcher did not finish --version within ${CODEX_SHIM_INSTALL_PROBE_TIMEOUT_MS}ms` @@ -1394,9 +1477,13 @@ function installCodexShimInternal(options: InstallCodexShimInternalOptions): { i : "the saved launcher failed its --version probe"; return { installed: false, - message: `Refusing Codex autostart shim because ${reason}. The original launcher was restored; reinstall Codex as a concrete executable before enabling codexAutoStart.`, + message: wrapperChangedDuringProbe + ? `Refusing Codex autostart shim because ${reason}. The concurrent launcher was preserved; retry after the Codex update finishes.` + : `Refusing Codex autostart shim because ${reason}. The original launcher was restored; reinstall Codex as a concrete executable before enabling codexAutoStart.`, }; } + } else if (freshApplyError) { + throw freshApplyError; } writeState(primaryState(targets)); return { diff --git a/tests/codex-shim.test.ts b/tests/codex-shim.test.ts index b2fed6165..50003fc68 100644 --- a/tests/codex-shim.test.ts +++ b/tests/codex-shim.test.ts @@ -560,6 +560,45 @@ wait "$child" } }); + test("Unix fresh install preserves a concurrent wrapper replacement after a successful probe", () => { + if (process.platform === "win32") return; + + const binDir = mkdtempSync(join(tmpdir(), "ocx-shim-install-concurrent-bin-")); + const home = mkdtempSync(join(tmpdir(), "ocx-shim-install-concurrent-home-")); + const oldPath = process.env.PATH; + const oldHome = process.env.OPENCODEX_HOME; + const codexPath = join(binDir, "codex"); + const concurrent = successfulLauncher("fresh concurrent updater replacement"); + const original = `#!/bin/sh +if [ "$1" = "--version" ]; then + printf '%s\\n' '#!/bin/sh' '# fresh concurrent updater replacement' 'exit 0' > "${codexPath}" + chmod 755 "${codexPath}" +fi +exit 0 +`; + try { + process.env.PATH = prependPath(binDir, oldPath); + process.env.OPENCODEX_HOME = home; + writeFileSync(codexPath, original, "utf8"); + chmodSync(codexPath, 0o755); + + const installed = installCodexShim(); + + expect(installed.installed).toBe(false); + expect(installed.message).toContain("generated wrapper changed during its validation probe"); + expect(readFileSync(codexPath, "utf8")).toBe(concurrent); + expect(existsSync(`${codexPath}.opencodex-real`)).toBe(false); + expect(existsSync(join(home, "codex-shim.json"))).toBe(false); + } finally { + if (oldPath === undefined) delete process.env.PATH; + else process.env.PATH = oldPath; + if (oldHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = oldHome; + rmSync(binDir, { recursive: true, force: true }); + rmSync(home, { recursive: true, force: true }); + } + }); + test("Unix shim permits a real Codex process to start a new child invocation", () => { if (process.platform === "win32") return; From 7200dd04424b753d63c495c4ca55f6005eed491a Mon Sep 17 00:00:00 2001 From: comfuture Date: Tue, 11 Aug 2026 11:45:04 +0900 Subject: [PATCH 09/16] fix(codex): harden probe cleanup edges --- src/codex/shim.ts | 55 +++++++++++++------ tests/codex-shim.test.ts | 114 +++++++++++++++++++++++++++++++++++---- 2 files changed, 143 insertions(+), 26 deletions(-) diff --git a/src/codex/shim.ts b/src/codex/shim.ts index 965ede9d1..69a1e894b 100644 --- a/src/codex/shim.ts +++ b/src/codex/shim.ts @@ -9,11 +9,13 @@ import { fstatSync, lstatSync, mkdirSync, + mkdtempSync, openSync, readFileSync, readdirSync, readSync, renameSync, + rmSync, rmdirSync, statSync, type Stats, @@ -42,13 +44,15 @@ const CODEX_SHIM_REENTRY_DIAGNOSTIC = "opencodex: saved Codex launcher resolved const CODEX_SHIM_INSTALL_PROBE_SCRIPT = ` const { spawn } = require("node:child_process"); const { writeFileSync } = require("node:fs"); -const [markerPath, groupPath, stderrPath, launcherShellPath, wrapperPath, timeoutRaw, stderrLimitRaw] = process.argv.slice(1); +const [markerPath, groupPath, stderrPath, launcherShellPath, wrapperPath, timeoutRaw, stderrLimitRaw, stderrDrainRaw] = process.argv.slice(1); const timeoutMs = Number.parseInt(timeoutRaw, 10); const stderrLimit = Number.parseInt(stderrLimitRaw, 10); +const stderrDrainMs = Number.parseInt(stderrDrainRaw, 10); const stderrChunks = []; let stderrBytes = 0; let launcher; let timer; +let stderrDrainTimer; let marker = ""; let finished = false; @@ -91,6 +95,7 @@ function finish(status) { if (finished) return; finished = true; if (timer) clearTimeout(timer); + if (stderrDrainTimer) clearTimeout(stderrDrainTimer); if (!marker && groupAlive()) { setMarker("descendants"); killGroup(); @@ -99,6 +104,20 @@ function finish(status) { process.exit(marker === "timeout" ? 124 : marker === "descendants" ? 125 : status); } +function finishAfterStderr(status) { + if (finished) return; + if (timer) { + clearTimeout(timer); + timer = undefined; + } + if (!launcher || !launcher.stderr || launcher.stderr.readableEnded) { + finish(status); + return; + } + launcher.stderr.once("end", () => finish(status)); + stderrDrainTimer = setTimeout(() => finish(status), stderrDrainMs); +} + try { launcher = spawn(launcherShellPath, [wrapperPath, "--version"], { detached: true, @@ -110,9 +129,9 @@ try { launcher.stderr.on("data", appendStderr); launcher.once("error", error => { appendStderr(String(error)); - finish(127); + finishAfterStderr(127); }); - launcher.once("exit", code => finish(Number.isInteger(code) ? code : 127)); + launcher.once("exit", code => finishAfterStderr(Number.isInteger(code) ? code : 127)); timer = setTimeout(() => { setMarker("timeout"); killGroup(); @@ -529,6 +548,7 @@ type UnixShimProbeResult = "cleanup" | "descendants" | "failed" | "recursive" | let codexShimProbeHookForTests: (() => void) | null = null; let codexShimProbeShellForTests: string | null = null; +let codexShimGuardedWriteHookForTests: (() => void) | null = null; /** Narrow deterministic seam for transaction rollback tests. */ export function setCodexShimProbeHookForTests(hook: (() => void) | null): void { @@ -540,6 +560,11 @@ export function setCodexShimProbeShellForTests(path: string | null): void { codexShimProbeShellForTests = path; } +/** Narrow deterministic seam for guarded partial-write rollback tests. */ +export function setCodexShimGuardedWriteHookForTests(hook: (() => void) | null): void { + codexShimGuardedWriteHookForTests = hook; +} + function readProbeMetadata(path: string, maxBytes: number): string | null { try { if (!existsSync(path)) return ""; @@ -555,12 +580,13 @@ function probeUnixShimInstall(wrapperPath: string): UnixShimProbeResult { if (process.platform === "win32") return null; const env: NodeJS.ProcessEnv = { ...process.env, OCX_SHIM_BYPASS: "1" }; delete env.OCX_SHIM_ACTIVE_PID; - const probeId = `${process.pid}-${randomUUID()}`; - const markerPath = join(tmpdir(), `opencodex-shim-probe-${probeId}.result`); - const groupPath = join(tmpdir(), `opencodex-shim-probe-${probeId}.group`); - const stderrPath = join(tmpdir(), `opencodex-shim-probe-${probeId}.stderr`); + const probeDir = mkdtempSync(join(tmpdir(), "opencodex-shim-probe-")); + const markerPath = join(probeDir, "result"); + const groupPath = join(probeDir, "group"); + const stderrPath = join(probeDir, "stderr"); let groupId = 0; try { + chmodSync(probeDir, 0o700); const result = spawnSync(process.execPath, [ "-e", CODEX_SHIM_INSTALL_PROBE_SCRIPT, @@ -571,6 +597,7 @@ function probeUnixShimInstall(wrapperPath: string): UnixShimProbeResult { wrapperPath, String(CODEX_SHIM_INSTALL_PROBE_TIMEOUT_MS), String(MAX_DIAGNOSTIC_VALUE_BYTES), + String(CODEX_SHIM_INSTALL_PROBE_EXIT_TIMEOUT_MS), ], { encoding: "utf8", env, @@ -595,7 +622,7 @@ function probeUnixShimInstall(wrapperPath: string): UnixShimProbeResult { if (result.error && !timedOut) return "cleanup"; if (timedOut || marker === "timeout") return "timeout"; if (marker === "descendants") return "descendants"; - if (groupSurvived) return "cleanup"; + if (groupSurvived) return "descendants"; if (result.status === CODEX_SHIM_REENTRY_EXIT_CODE && launcherStderr.includes(CODEX_SHIM_REENTRY_DIAGNOSTIC)) { return "recursive"; } @@ -607,11 +634,7 @@ function probeUnixShimInstall(wrapperPath: string): UnixShimProbeResult { } return "cleanup"; } finally { - for (const path of [markerPath, groupPath, stderrPath]) { - try { - if (existsSync(path)) unlinkSync(path); - } catch { /* best-effort cleanup of non-sensitive probe metadata */ } - } + try { rmSync(probeDir, { recursive: true, force: true }); } catch { /* best-effort cleanup */ } } } @@ -1197,9 +1220,10 @@ function rollbackGuardedRefresh(journal: readonly GuardedRefreshJournalEntry[]): attempt(() => { const wrapper = stableShimPathProbe(entry.operation.file.wrapperPath); const ownsWrapper = entry.wrapperWriteStarted - && entry.writtenWrapperFingerprint !== undefined && wrapper !== null - && sameFingerprint(wrapper.fingerprint, entry.writtenWrapperFingerprint); + && (entry.writtenWrapperFingerprint !== undefined + ? sameFingerprint(wrapper.fingerprint, entry.writtenWrapperFingerprint) + : wrapper.prefix.includes(SHIM_MARKER)); if (ownsWrapper) { unlinkSync(entry.operation.file.wrapperPath); } else { @@ -1269,6 +1293,7 @@ function applyGuardedRefreshTransaction( entry.movedReplacementFingerprint = movedReplacement.fingerprint; entry.wrapperWriteStarted = true; writeShim(operation.file.wrapperPath, operation.file.realPath ?? operation.file.backupPath); + codexShimGuardedWriteHookForTests?.(); const writtenWrapper = stableShimPathProbe(operation.file.wrapperPath); if (!writtenWrapper || !writtenWrapper.prefix.includes(SHIM_MARKER)) { throw new Error("Codex shim guarded refresh could not fingerprint the generated wrapper"); diff --git a/tests/codex-shim.test.ts b/tests/codex-shim.test.ts index 50003fc68..c782afb17 100644 --- a/tests/codex-shim.test.ts +++ b/tests/codex-shim.test.ts @@ -3,7 +3,7 @@ import { spawnSync } from "node:child_process"; import { chmodSync, mkdirSync, mkdtempSync, writeFileSync, readFileSync, existsSync, readdirSync, rmSync, statSync, symlinkSync, utimesSync } from "node:fs"; import { delimiter, dirname, join } from "node:path"; import { tmpdir } from "node:os"; -import { autoRestoreCodexShim, buildUnixCodexShim, buildWindowsCodexShim, buildWindowsPowerShellCodexShim, diagnoseCodexShim, findCodexOnPath, installCodexShim, isWindowsInteropDir, lastCodexDiscoveryError, setCodexShimProbeHookForTests, setCodexShimProbeShellForTests, uninstallCodexShim } from "../src/codex/shim"; +import { autoRestoreCodexShim, buildUnixCodexShim, buildWindowsCodexShim, buildWindowsPowerShellCodexShim, diagnoseCodexShim, findCodexOnPath, installCodexShim, isWindowsInteropDir, lastCodexDiscoveryError, setCodexShimGuardedWriteHookForTests, setCodexShimProbeHookForTests, setCodexShimProbeShellForTests, uninstallCodexShim } from "../src/codex/shim"; const SHIM_MARKER = "opencodex codex autostart shim"; const skipStabilityWait = () => {}; @@ -319,27 +319,93 @@ exit 64 } }); - test("Unix install accepts a valid launcher when the probe shell is dash", () => { - if (process.platform === "win32" || !existsSync("/bin/dash")) return; + test("Unix install drains an immediate recursive diagnostic before classifying the probe", () => { + if (process.platform === "win32") return; - const binDir = mkdtempSync(join(tmpdir(), "ocx-shim-install-dash-bin-")); - const home = mkdtempSync(join(tmpdir(), "ocx-shim-install-dash-home-")); + const binDir = mkdtempSync(join(tmpdir(), "ocx-shim-install-immediate-reentry-bin-")); + const home = mkdtempSync(join(tmpdir(), "ocx-shim-install-immediate-reentry-home-")); const oldPath = process.env.PATH; const oldHome = process.env.OPENCODEX_HOME; const codexPath = join(binDir, "codex"); + const original = `#!/bin/sh +printf '%s\\n' 'opencodex: saved Codex launcher resolved back to the autostart shim; run ocx codex-shim uninstall and reinstall Codex before enabling codexAutoStart.' >&2 +exit 126 +`; try { process.env.PATH = prependPath(binDir, oldPath); process.env.OPENCODEX_HOME = home; - writeFileSync(codexPath, successfulLauncher("dash-valid-launcher"), "utf8"); + writeFileSync(codexPath, original, "utf8"); chmodSync(codexPath, 0o755); - setCodexShimProbeShellForTests("/bin/dash"); const installed = installCodexShim(); - expect(installed.installed).toBe(true); - expect(readFileSync(codexPath, "utf8")).toContain(SHIM_MARKER); - expect(readFileSync(`${codexPath}.opencodex-real`, "utf8")).toBe(successfulLauncher("dash-valid-launcher")); - expect(existsSync(join(home, "codex-shim.json"))).toBe(true); + expect(installed.installed).toBe(false); + expect(installed.message).toContain("saved launcher resolved back to the generated shim"); + expect(readFileSync(codexPath, "utf8")).toBe(original); + } finally { + if (oldPath === undefined) delete process.env.PATH; + else process.env.PATH = oldPath; + if (oldHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = oldHome; + rmSync(binDir, { recursive: true, force: true }); + rmSync(home, { recursive: true, force: true }); + } + }); + + test.skipIf(process.platform === "win32" || !existsSync("/bin/dash"))( + "Unix install accepts a valid launcher when the probe shell is dash", + () => { + const binDir = mkdtempSync(join(tmpdir(), "ocx-shim-install-dash-bin-")); + const home = mkdtempSync(join(tmpdir(), "ocx-shim-install-dash-home-")); + const oldPath = process.env.PATH; + const oldHome = process.env.OPENCODEX_HOME; + const codexPath = join(binDir, "codex"); + try { + process.env.PATH = prependPath(binDir, oldPath); + process.env.OPENCODEX_HOME = home; + writeFileSync(codexPath, successfulLauncher("dash-valid-launcher"), "utf8"); + chmodSync(codexPath, 0o755); + setCodexShimProbeShellForTests("/bin/dash"); + + const installed = installCodexShim(); + + expect(installed.installed).toBe(true); + expect(readFileSync(codexPath, "utf8")).toContain(SHIM_MARKER); + expect(readFileSync(`${codexPath}.opencodex-real`, "utf8")).toBe(successfulLauncher("dash-valid-launcher")); + expect(existsSync(join(home, "codex-shim.json"))).toBe(true); + } finally { + setCodexShimProbeShellForTests(null); + if (oldPath === undefined) delete process.env.PATH; + else process.env.PATH = oldPath; + if (oldHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = oldHome; + rmSync(binDir, { recursive: true, force: true }); + rmSync(home, { recursive: true, force: true }); + } + }, + ); + + test("Unix install honors the injected probe shell path", () => { + if (process.platform === "win32") return; + + const binDir = mkdtempSync(join(tmpdir(), "ocx-shim-install-missing-shell-bin-")); + const home = mkdtempSync(join(tmpdir(), "ocx-shim-install-missing-shell-home-")); + const oldPath = process.env.PATH; + const oldHome = process.env.OPENCODEX_HOME; + const codexPath = join(binDir, "codex"); + const original = successfulLauncher("missing-probe-shell"); + try { + process.env.PATH = prependPath(binDir, oldPath); + process.env.OPENCODEX_HOME = home; + writeFileSync(codexPath, original, "utf8"); + chmodSync(codexPath, 0o755); + setCodexShimProbeShellForTests(join(binDir, "does-not-exist")); + + const installed = installCodexShim(); + + expect(installed.installed).toBe(false); + expect(readFileSync(codexPath, "utf8")).toBe(original); + expect(existsSync(`${codexPath}.opencodex-real`)).toBe(false); } finally { setCodexShimProbeShellForTests(null); if (oldPath === undefined) delete process.env.PATH; @@ -934,6 +1000,32 @@ printf '%s\\n' child-codex }); }); + test("guarded auto-restore removes its unfingerprinted partial wrapper before rollback", () => { + if (process.platform === "win32") return; + withInstalledShim(({ wrappers, backups, statePath }) => { + const replacement = successfulLauncher("guarded-partial-write-replacement"); + const oldBackup = readFileSync(backups[0]); + const oldState = readFileSync(statePath); + writeFileSync(wrappers[0], replacement, "utf8"); + setCodexShimGuardedWriteHookForTests(() => { throw new Error("synthetic failure after wrapper write"); }); + + let failure: unknown; + try { + autoRestoreCodexShim({ enabled: () => true, stabilitySleep: skipStabilityWait }); + } catch (error) { + failure = error; + } finally { + setCodexShimGuardedWriteHookForTests(null); + } + + expect(failure).toBeInstanceOf(AggregateError); + expect((failure as AggregateError).errors.map(error => String(error))).toContain("Error: synthetic failure after wrapper write"); + expect(readFileSync(wrappers[0], "utf8")).toBe(replacement); + expect(readFileSync(backups[0])).toEqual(oldBackup); + expect(readFileSync(statePath)).toEqual(oldState); + }); + }); + test("guarded auto-restore preserves a concurrent wrapper replacement after a successful probe", () => { if (process.platform === "win32") return; withInstalledShim(({ binDir, wrappers, backups, statePath }) => { From f72d1cfa664c07661c722780f9f68ecf28009757 Mon Sep 17 00:00:00 2001 From: comfuture Date: Tue, 11 Aug 2026 12:59:24 +0900 Subject: [PATCH 10/16] fix(codex): detect detached probe reentry --- .../docs/ja/reference/cli/lifecycle.md | 2 + .../docs/ko/reference/cli/lifecycle.md | 7 + .../content/docs/reference/cli/lifecycle.md | 7 + .../docs/ru/reference/cli/lifecycle.md | 8 ++ .../docs/zh-cn/reference/cli/lifecycle.md | 2 + src/codex/shim.ts | 85 ++++++++++-- tests/codex-shim.test.ts | 122 ++++++++++++++---- 7 files changed, 195 insertions(+), 38 deletions(-) diff --git a/docs-site/src/content/docs/ja/reference/cli/lifecycle.md b/docs-site/src/content/docs/ja/reference/cli/lifecycle.md index 447720170..17e387da6 100644 --- a/docs-site/src/content/docs/ja/reference/cli/lifecycle.md +++ b/docs-site/src/content/docs/ja/reference/cli/lifecycle.md @@ -181,6 +181,8 @@ Windows では、タスク スケジューラ エントリを作成するには 軽量の自動起動スクリプトを使用して、スクリプトベースの `codex` ランチャーを PATH 上にラップします。実際の `codex.exe` ターゲットは、正確な実行可能呼び出しの破損を避けるため、変更されないまま残されます。 +インストールまたは修復を確定する前に、OpenCodex はサービス起動をバイパスした状態で、保存済みランチャーを `--version` 付きで実行します。ランチャーが `codex` を再び shim に解決する、0 以外で終了する、5 秒を超える、子プロセスを残す、または安全に検証・クリーンアップできない場合、変更を拒否してロールバックします。したがって `codex-shim install` は無条件のインストールではありません。拒否された場合は、PATH エントリが具体的な実行ファイルまたはランチャーを指すよう Codex を再インストールしてから再試行してください。動的コマンドマネージャーのランチャーがこれらの検証を満たせない場合は、代わりに `ocx service install` を使用してください。 + 完了した外部 Codex アップデートがインストールされている shim を上書きした場合、次の通常の `ocx` コマンドは安定した新しいランチャーをバックアップし、ディスパッチ前に shim を復元します。まだ変更中のランチャーは変更されず、後で再試行されます。修復の失敗は、要求されたコマンドを失敗させることなく警告します。手動フォールバック: `ocx codex-shim install`。 `codexShimAutoRestore` を `false` に設定するか、プロセス レベルのオプトアウトの場合は `OPENCODEX_CODEX_SHIM_AUTO_RESTORE=0` を設定します。 |サブコマンド |アクション | diff --git a/docs-site/src/content/docs/ko/reference/cli/lifecycle.md b/docs-site/src/content/docs/ko/reference/cli/lifecycle.md index 881dc9bc2..d76c8f96a 100644 --- a/docs-site/src/content/docs/ko/reference/cli/lifecycle.md +++ b/docs-site/src/content/docs/ko/reference/cli/lifecycle.md @@ -235,6 +235,13 @@ Windows에서 Task Scheduler 항목을 만들려면 권한 상승이 필요합 PATH 위의 스크립트 기반 `codex` 런처를 가벼운 자동 시작 스크립트로 감쌉니다. 정확한 실행 파일 호출을 깨지 않도록 실제 `codex.exe` 대상은 손대지 않습니다. +설치나 복구를 확정하기 전에 OpenCodex는 서비스 시작을 우회한 상태에서 저장된 런처를 +`--version`으로 실행합니다. 런처가 `codex`를 shim으로 다시 해석해 재귀하거나, 0이 아닌 코드로 +종료하거나, 5초를 초과하거나, 실행 중인 자식 프로세스를 남기거나, 안전하게 검증·정리할 수 없으면 +변경을 거부하고 롤백합니다. 따라서 `codex-shim install`은 무조건 성공하는 명령이 아닙니다. 거부되면 +PATH 항목이 구체적인 실행 파일 또는 런처를 가리키도록 Codex를 다시 설치한 뒤 재시도하세요. 동적 +명령 관리자의 런처가 이 검증을 충족할 수 없다면 대신 `ocx service install`을 사용하세요. + 완료된 외부 Codex 업데이트가 설치된 shim을 덮어쓰면, 다음 일반 `ocx` 명령이 안정적인 새 런처를 백업하고 명령을 처리하기 전에 shim을 복원합니다. 아직 변경 중인 런처는 건드리지 않고 나중에 다시 시도합니다. 복구 실패는 요청한 명령을 실패시키지 않고 경고만 표시합니다. 수동 대체 수단은 `ocx codex-shim install` diff --git a/docs-site/src/content/docs/reference/cli/lifecycle.md b/docs-site/src/content/docs/reference/cli/lifecycle.md index ccf2bc18e..12780c1a1 100644 --- a/docs-site/src/content/docs/reference/cli/lifecycle.md +++ b/docs-site/src/content/docs/reference/cli/lifecycle.md @@ -287,6 +287,13 @@ dashboard UAC prompt or rerun `ocx service install` in an elevated PowerShell wi Wrap a script-based `codex` launcher on PATH with a lightweight autostart script. Real `codex.exe` targets are left untouched to avoid breaking exact executable invocations. +Before an install or repair is committed, OpenCodex runs the saved launcher with `--version` while +service startup is bypassed. It refuses the change and rolls back when the launcher resolves +`codex` back to the shim, exits nonzero, exceeds five seconds, leaves descendants running, or +cannot be validated and cleaned up safely. Therefore `codex-shim install` is not unconditional. If +it is refused, reinstall Codex so the PATH entry is a concrete executable or launcher and retry; +use `ocx service install` instead when a dynamic command-manager launcher cannot meet these checks. + Launcher installation alone does not prove that Codex requests will use OpenCodex. After a healthy install, the command checks the current Codex routing and reports a warning instead of a green result when routing is external, user-owned, or unverifiable. It also warns when outbound proxy variables diff --git a/docs-site/src/content/docs/ru/reference/cli/lifecycle.md b/docs-site/src/content/docs/ru/reference/cli/lifecycle.md index 8d82fead7..be01664eb 100644 --- a/docs-site/src/content/docs/ru/reference/cli/lifecycle.md +++ b/docs-site/src/content/docs/ru/reference/cli/lifecycle.md @@ -253,6 +253,14 @@ scheduler-error. Чужие задачи и чужие операции нико Обернуть script-based launcher `codex` на `PATH` лёгким автозапусковым скриптом. Настоящие target'ы `codex.exe` не трогаются, чтобы не ломать точные вызовы исполняемого файла. +Перед фиксацией установки или repair OpenCodex запускает сохранённый launcher с `--version`, +не запуская сервис. Изменение отклоняется и откатывается, если launcher снова разрешает `codex` +в shim, завершается с ненулевым кодом, работает дольше пяти секунд, оставляет дочерние процессы +или не может быть безопасно проверен и очищен. Поэтому `codex-shim install` не является +безусловной установкой. После отказа переустановите Codex так, чтобы запись в `PATH` указывала на +конкретный исполняемый файл или launcher, и повторите попытку. Если динамический launcher +менеджера команд не проходит эти проверки, используйте вместо него `ocx service install`. + Если завершённое внешнее обновление Codex перезаписало установленный shim, следующая обычная команда `ocx` сохранит новый стабильный launcher и восстановит shim перед выполнением запроса. Launcher, который всё ещё меняется, не трогается, а попытка откладывается до следующего раза. diff --git a/docs-site/src/content/docs/zh-cn/reference/cli/lifecycle.md b/docs-site/src/content/docs/zh-cn/reference/cli/lifecycle.md index 81bc03406..b7d976bb9 100644 --- a/docs-site/src/content/docs/zh-cn/reference/cli/lifecycle.md +++ b/docs-site/src/content/docs/zh-cn/reference/cli/lifecycle.md @@ -178,6 +178,8 @@ ocx service uninstall 在 PATH 上把基于脚本的 `codex` 启动器包装为一个轻量自启动脚本。真实的 `codex.exe` 目标会保持不变，以避免破坏精确的可执行文件调用。 +提交安装或修复前，OpenCodex 会在跳过服务启动的情况下，用 `--version` 运行已保存的启动器。如果启动器把 `codex` 再次解析到 shim、以非零状态退出、运行超过五秒、留下仍在运行的子进程，或无法被安全验证和清理，OpenCodex 会拒绝并回滚更改。因此 `codex-shim install` 并不是无条件安装。若被拒绝，请重新安装 Codex，使 PATH 条目指向具体的可执行文件或启动器，然后重试；如果动态命令管理器的启动器无法满足这些检查，请改用 `ocx service install`。 + 仅安装启动器并不能证明 Codex 请求会经过 OpenCodex。完成健康安装后，命令会检查当前 Codex 路由；当路由由外部配置、用户自有网关管理或无法验证时，会显示警告而不是绿色成功。若出站代理变量只存在于当前进程，而 `config.proxy` 未设置或无法解析，也会给出警告，因为 Codex 启动器和后台服务未必继承该环境。这些检查只读且绝不会打印代理值；在依赖自动启动前，请先处理提示的交接配置并运行 `ocx doctor`。 如果已完成的外部 Codex 更新覆盖了已安装的 shim，下一次普通的 `ocx` 命令会先备份稳定的新启动器，再在分发前恢复 shim。仍在变动中的启动器会保持不动，并在稍后重试。修复失败只会警告，不会让所请求的命令失败；手动回退：`ocx codex-shim install`。将 `codexShimAutoRestore` 设为 `false`，或设置 `OPENCODEX_CODEX_SHIM_AUTO_RESTORE=0`，即可在进程级别关闭自动恢复。 diff --git a/src/codex/shim.ts b/src/codex/shim.ts index 69a1e894b..5d040bc3b 100644 --- a/src/codex/shim.ts +++ b/src/codex/shim.ts @@ -39,20 +39,24 @@ export const CODEX_SHIM_STATE_MAX_BYTES = 1024 * 1024; const CODEX_SHIM_RESTORE_LOCK_STALE_MS = 30_000; const CODEX_SHIM_INSTALL_PROBE_TIMEOUT_MS = 5_000; const CODEX_SHIM_INSTALL_PROBE_EXIT_TIMEOUT_MS = 1_000; +const CODEX_SHIM_INSTALL_PROBE_REENTRY_GRACE_MS = 100; const CODEX_SHIM_REENTRY_EXIT_CODE = 126; const CODEX_SHIM_REENTRY_DIAGNOSTIC = "opencodex: saved Codex launcher resolved back to the autostart shim; run ocx codex-shim uninstall and reinstall Codex before enabling codexAutoStart."; const CODEX_SHIM_INSTALL_PROBE_SCRIPT = ` const { spawn } = require("node:child_process"); -const { writeFileSync } = require("node:fs"); -const [markerPath, groupPath, stderrPath, launcherShellPath, wrapperPath, timeoutRaw, stderrLimitRaw, stderrDrainRaw] = process.argv.slice(1); +const { readFileSync, writeFileSync } = require("node:fs"); +const [markerPath, reentryPath, groupPath, stderrPath, launcherShellPath, wrapperPath, timeoutRaw, stderrLimitRaw, stderrDrainRaw, reentryGraceRaw] = process.argv.slice(1); const timeoutMs = Number.parseInt(timeoutRaw, 10); const stderrLimit = Number.parseInt(stderrLimitRaw, 10); const stderrDrainMs = Number.parseInt(stderrDrainRaw, 10); +const reentryGraceMs = Number.parseInt(reentryGraceRaw, 10); const stderrChunks = []; let stderrBytes = 0; let launcher; let timer; let stderrDrainTimer; +let reentryGraceTimer; +let reentryPollTimer; let marker = ""; let finished = false; @@ -91,17 +95,31 @@ function setMarker(value) { try { writeExclusive(markerPath, value + "\\n"); } catch (error) { appendStderr(String(error)); } } +function reentryDetected() { + try { return readFileSync(reentryPath, "utf8").trim() === "recursive"; } catch { return false; } +} + +function checkReentry() { + if (finished || !reentryDetected()) return; + setMarker("recursive"); + killGroup(); + finish(126); +} + function finish(status) { if (finished) return; finished = true; if (timer) clearTimeout(timer); if (stderrDrainTimer) clearTimeout(stderrDrainTimer); + if (reentryGraceTimer) clearTimeout(reentryGraceTimer); + if (reentryPollTimer) clearInterval(reentryPollTimer); + if (!marker && reentryDetected()) setMarker("recursive"); if (!marker && groupAlive()) { setMarker("descendants"); killGroup(); } try { writeExclusive(stderrPath, Buffer.concat(stderrChunks)); } catch { /* parent fails closed */ } - process.exit(marker === "timeout" ? 124 : marker === "descendants" ? 125 : status); + process.exit(marker === "timeout" ? 124 : marker === "descendants" ? 125 : marker === "recursive" ? 126 : status); } function finishAfterStderr(status) { @@ -110,11 +128,23 @@ function finishAfterStderr(status) { clearTimeout(timer); timer = undefined; } - if (!launcher || !launcher.stderr || launcher.stderr.readableEnded) { + if (!launcher || !launcher.stderr) { finish(status); return; } - launcher.stderr.once("end", () => finish(status)); + let stderrEnded = launcher.stderr.readableEnded; + let graceElapsed = false; + const finishWhenReady = () => { + if (stderrEnded && graceElapsed) finish(status); + }; + launcher.stderr.once("end", () => { + stderrEnded = true; + finishWhenReady(); + }); + reentryGraceTimer = setTimeout(() => { + graceElapsed = true; + finishWhenReady(); + }, reentryGraceMs); stderrDrainTimer = setTimeout(() => finish(status), stderrDrainMs); } @@ -127,6 +157,7 @@ try { if (!launcher.pid) throw new Error("Codex shim probe launcher has no pid"); writeExclusive(groupPath, String(launcher.pid) + "\\n"); launcher.stderr.on("data", appendStderr); + reentryPollTimer = setInterval(checkReentry, 10); launcher.once("error", error => { appendStderr(String(error)); finishAfterStderr(127); @@ -492,6 +523,17 @@ export function buildUnixCodexShim(realCodexPath: string, bunPath: string, cliPa const valueOptions = CODEX_GLOBAL_OPTIONS_WITH_VALUE.join("|"); return `#!/usr/bin/env sh # ${SHIM_MARKER} +if [ "\${OCX_SHIM_PROBE:-}" = "1" ]; then + if [ "\${OCX_SHIM_PROBE_ACTIVE:-}" = "1" ]; then + if [ -n "\${OCX_SHIM_PROBE_REENTRY_PATH:-}" ]; then + (umask 077; printf '%s\n' recursive > "$OCX_SHIM_PROBE_REENTRY_PATH") 2>/dev/null || true + fi + printf '%s\n' ${shQuote(CODEX_SHIM_REENTRY_DIAGNOSTIC)} >&2 + exit ${CODEX_SHIM_REENTRY_EXIT_CODE} + fi + OCX_SHIM_PROBE_ACTIVE=1 + export OCX_SHIM_PROBE_ACTIVE +fi if [ "\${OCX_SHIM_ACTIVE_PID:-}" = "$$" ]; then printf '%s\n' ${shQuote(CODEX_SHIM_REENTRY_DIAGNOSTIC)} >&2 exit ${CODEX_SHIM_REENTRY_EXIT_CODE} @@ -578,12 +620,19 @@ function readProbeMetadata(path: string, maxBytes: number): string | null { function probeUnixShimInstall(wrapperPath: string): UnixShimProbeResult { if (process.platform === "win32") return null; - const env: NodeJS.ProcessEnv = { ...process.env, OCX_SHIM_BYPASS: "1" }; - delete env.OCX_SHIM_ACTIVE_PID; const probeDir = mkdtempSync(join(tmpdir(), "opencodex-shim-probe-")); const markerPath = join(probeDir, "result"); + const reentryPath = join(probeDir, "reentry"); const groupPath = join(probeDir, "group"); const stderrPath = join(probeDir, "stderr"); + const env: NodeJS.ProcessEnv = { + ...process.env, + OCX_SHIM_BYPASS: "1", + OCX_SHIM_PROBE: "1", + OCX_SHIM_PROBE_REENTRY_PATH: reentryPath, + }; + delete env.OCX_SHIM_ACTIVE_PID; + delete env.OCX_SHIM_PROBE_ACTIVE; let groupId = 0; try { chmodSync(probeDir, 0o700); @@ -591,6 +640,7 @@ function probeUnixShimInstall(wrapperPath: string): UnixShimProbeResult { "-e", CODEX_SHIM_INSTALL_PROBE_SCRIPT, markerPath, + reentryPath, groupPath, stderrPath, codexShimProbeShellForTests ?? "/bin/sh", @@ -598,6 +648,7 @@ function probeUnixShimInstall(wrapperPath: string): UnixShimProbeResult { String(CODEX_SHIM_INSTALL_PROBE_TIMEOUT_MS), String(MAX_DIAGNOSTIC_VALUE_BYTES), String(CODEX_SHIM_INSTALL_PROBE_EXIT_TIMEOUT_MS), + String(CODEX_SHIM_INSTALL_PROBE_REENTRY_GRACE_MS), ], { encoding: "utf8", env, @@ -606,13 +657,14 @@ function probeUnixShimInstall(wrapperPath: string): UnixShimProbeResult { }); const timedOut = (result.error as NodeJS.ErrnoException | undefined)?.code === "ETIMEDOUT"; const marker = readProbeMetadata(markerPath, 64); + const reentryMarker = readProbeMetadata(reentryPath, 64); const groupText = readProbeMetadata(groupPath, 64); const launcherStderr = readProbeMetadata(stderrPath, MAX_DIAGNOSTIC_VALUE_BYTES); groupId = groupText === null ? 0 : Number.parseInt(groupText, 10); - if (marker === null || groupText === null || launcherStderr === null + if (marker === null || reentryMarker === null || groupText === null || launcherStderr === null || !Number.isInteger(groupId) || groupId <= 0) return "cleanup"; const groupSurvived = unixProcessGroupAlive(groupId); - if (timedOut || marker || groupSurvived) { + if (timedOut || marker || reentryMarker || groupSurvived) { try { terminateUnixProcessGroup(groupId); } catch { @@ -621,6 +673,8 @@ function probeUnixShimInstall(wrapperPath: string): UnixShimProbeResult { } if (result.error && !timedOut) return "cleanup"; if (timedOut || marker === "timeout") return "timeout"; + if (marker === "recursive" || reentryMarker === "recursive") return "recursive"; + if (reentryMarker !== "") return "cleanup"; if (marker === "descendants") return "descendants"; if (groupSurvived) return "descendants"; if (result.status === CODEX_SHIM_REENTRY_EXIT_CODE && launcherStderr.includes(CODEX_SHIM_REENTRY_DIAGNOSTIC)) { @@ -991,6 +1045,10 @@ function refreshShimFile(file: ShimFileState): boolean { } if (!existsSync(file.wrapperPath) && existsSync(file.backupPath)) { writeShim(file.wrapperPath, file.realPath ?? file.backupPath); + const writtenWrapper = stableShimPathProbe(file.wrapperPath); + if (!writtenWrapper || !writtenWrapper.prefix.includes(SHIM_MARKER)) { + return false; + } let unsafe: UnixShimProbeResult = null; let probeError: Error | null = null; try { @@ -998,8 +1056,13 @@ function refreshShimFile(file: ShimFileState): boolean { } catch (error) { probeError = error instanceof Error ? error : new Error(String(error)); } - if (unsafe !== null || probeError) { - if (existsSync(file.wrapperPath) && isShim(file.wrapperPath)) unlinkSync(file.wrapperPath); + const currentWrapper = stableShimPathProbe(file.wrapperPath); + const wrapperChangedDuringProbe = !currentWrapper + || !sameFingerprint(currentWrapper.fingerprint, writtenWrapper.fingerprint); + if (unsafe !== null || probeError || wrapperChangedDuringProbe) { + if (currentWrapper && sameFingerprint(currentWrapper.fingerprint, writtenWrapper.fingerprint)) { + unlinkSync(file.wrapperPath); + } if (probeError) throw probeError; return false; } diff --git a/tests/codex-shim.test.ts b/tests/codex-shim.test.ts index c782afb17..c2894c5e6 100644 --- a/tests/codex-shim.test.ts +++ b/tests/codex-shim.test.ts @@ -7,6 +7,9 @@ import { autoRestoreCodexShim, buildUnixCodexShim, buildWindowsCodexShim, buildW const SHIM_MARKER = "opencodex codex autostart shim"; const skipStabilityWait = () => {}; +const python3Path = process.platform === "win32" + ? "" + : spawnSync("/bin/sh", ["-c", "command -v python3"], { encoding: "utf8" }).stdout.trim(); function prependPath(dir: string, current: string | undefined): string { return [dir, current].filter(Boolean).join(delimiter); @@ -20,6 +23,17 @@ function processState(pid: number): string { return spawnSync("/bin/ps", ["-o", "stat=", "-p", String(pid)], { encoding: "utf8" }).stdout.trim(); } +function waitForProcessStop(pid: number, timeoutMs = 1_000): string { + const deadline = Date.now() + timeoutMs; + const waiter = new Int32Array(new SharedArrayBuffer(4)); + let state = processState(pid); + while (state !== "" && !state.startsWith("Z") && Date.now() < deadline) { + Atomics.wait(waiter, 0, 0, 10); + state = processState(pid); + } + return state; +} + function expectProcessGroupMissing(groupId: number): void { let code: string | undefined; try { @@ -417,7 +431,7 @@ exit 126 } }); - test("Unix install rejects child-process redispatch and restores the original", () => { + test("Unix install rejects a launcher that leaves a background descendant", () => { if (process.platform === "win32") return; const binDir = mkdtempSync(join(tmpdir(), "ocx-shim-install-child-reentry-bin-")); @@ -425,33 +439,19 @@ exit 126 const oldPath = process.env.PATH; const oldHome = process.env.OPENCODEX_HOME; const codexPath = join(binDir, "codex"); - const launcherPath = join(binDir, "dynamic-launcher"); - const childPidPath = join(home, "child-reentry.pid"); - const grandchildPidPath = join(home, "child-reentry-grandchild.pid"); - const groupIdPath = join(home, "child-reentry-group.pid"); - const original = `#!/bin/sh\nexec "${launcherPath}" "$@"\n`; - try { - process.env.PATH = prependPath(binDir, oldPath); - process.env.OPENCODEX_HOME = home; - writeFileSync(launcherPath, `#!/bin/sh -if [ -n "$OCX_TEST_CHILD_REENTRY" ]; then - /bin/sleep 30 & - grandchild=$! - printf '%s\\n' "$grandchild" > "${grandchildPidPath}" - wait "$grandchild" - exec codex "$@" -fi -OCX_TEST_CHILD_REENTRY=1 -export OCX_TEST_CHILD_REENTRY -printf '%s\\n' "$$" > "${groupIdPath}" -codex "$@" & + const childPidPath = join(home, "background-child.pid"); + const groupIdPath = join(home, "background-child-group.pid"); + const original = `#!/bin/sh +/bin/sleep 30 & child=$! printf '%s\\n' "$child" > "${childPidPath}" -while [ ! -f "${grandchildPidPath}" ]; do /bin/sleep 0.01; done +printf '%s\\n' "$$" > "${groupIdPath}" exit 0 -`, "utf8"); +`; + try { + process.env.PATH = prependPath(binDir, oldPath); + process.env.OPENCODEX_HOME = home; writeFileSync(codexPath, original, "utf8"); - chmodSync(launcherPath, 0o755); chmodSync(codexPath, 0o755); const installed = installCodexShim(); @@ -462,13 +462,10 @@ exit 0 expect(existsSync(`${codexPath}.opencodex-real`)).toBe(false); expect(existsSync(join(home, "codex-shim.json"))).toBe(false); const childPid = Number.parseInt(readFileSync(childPidPath, "utf8").trim(), 10); - const grandchildPid = Number.parseInt(readFileSync(grandchildPidPath, "utf8").trim(), 10); const groupId = Number.parseInt(readFileSync(groupIdPath, "utf8").trim(), 10); expectProcessGroupMissing(groupId); const childState = processState(childPid); - const grandchildState = processState(grandchildPid); expect(childState === "" || childState.startsWith("Z")).toBe(true); - expect(grandchildState === "" || grandchildState.startsWith("Z")).toBe(true); } finally { if (oldPath === undefined) delete process.env.PATH; else process.env.PATH = oldPath; @@ -479,6 +476,52 @@ exit 0 } }); + test.skipIf(process.platform === "win32" || !python3Path)( + "Unix install rejects recursive redispatch that escapes into a detached process group", + () => { + const binDir = mkdtempSync(join(tmpdir(), "ocx-shim-install-detached-reentry-bin-")); + const home = mkdtempSync(join(tmpdir(), "ocx-shim-install-detached-reentry-home-")); + const oldPath = process.env.PATH; + const oldHome = process.env.OPENCODEX_HOME; + const codexPath = join(binDir, "codex"); + const childPidPath = join(home, "detached-reentry.pid"); + const original = `#!${python3Path} +import os +pid = os.fork() +if pid == 0: + os.setsid() + os.execvpe("codex", ["codex", "--version"], os.environ) +with open(${JSON.stringify(childPidPath)}, "w", encoding="utf-8") as handle: + handle.write(str(pid)) +os._exit(0) +`; + try { + process.env.PATH = prependPath(binDir, oldPath); + process.env.OPENCODEX_HOME = home; + writeFileSync(codexPath, original, "utf8"); + chmodSync(codexPath, 0o755); + + const installed = installCodexShim(); + + expect(installed.installed).toBe(false); + expect(installed.message).toContain("saved launcher resolved back to the generated shim"); + expect(readFileSync(codexPath, "utf8")).toBe(original); + expect(existsSync(`${codexPath}.opencodex-real`)).toBe(false); + expect(existsSync(join(home, "codex-shim.json"))).toBe(false); + const childPid = Number.parseInt(readFileSync(childPidPath, "utf8"), 10); + const childState = waitForProcessStop(childPid); + expect(childState === "" || childState.startsWith("Z")).toBe(true); + } finally { + if (oldPath === undefined) delete process.env.PATH; + else process.env.PATH = oldPath; + if (oldHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = oldHome; + rmSync(binDir, { recursive: true, force: true }); + rmSync(home, { recursive: true, force: true }); + } + }, + ); + test("Unix install rolls back when launcher validation times out", () => { if (process.platform === "win32") return; @@ -1051,6 +1094,31 @@ exit 0 }); }); + test("missing-wrapper repair preserves a concurrent replacement after a successful probe", () => { + if (process.platform === "win32") return; + withInstalledShim(({ wrappers, backups, statePath }) => { + const concurrent = successfulLauncher("missing-wrapper concurrent replacement"); + const mutatingBackup = `#!/bin/sh +if [ "$1" = "--version" ]; then + printf '%s\\n' '#!/bin/sh' '# missing-wrapper concurrent replacement' 'exit 0' > "${wrappers[0]}" + chmod 755 "${wrappers[0]}" +fi +exit 0 +`; + const oldState = readFileSync(statePath); + writeFileSync(backups[0], mutatingBackup, "utf8"); + chmodSync(backups[0], 0o755); + rmSync(wrappers[0], { force: true }); + + const result = installCodexShim(); + + expect(result.installed).toBe(false); + expect(readFileSync(wrappers[0], "utf8")).toBe(concurrent); + expect(readFileSync(backups[0], "utf8")).toBe(mutatingBackup); + expect(readFileSync(statePath)).toEqual(oldState); + }); + }); + test("direct refresh rejects a recursive replacement without replacing the owned backup", () => { if (process.platform === "win32") return; withInstalledShim(({ binDir, wrappers, backups, statePath }) => { From e91d722ef2e8cd753ede9e8e9f8e0c02e6d5611c Mon Sep 17 00:00:00 2001 From: comfuture Date: Tue, 11 Aug 2026 13:15:32 +0900 Subject: [PATCH 11/16] fix(codex): close shim upgrade safety gaps --- .../docs/ja/reference/cli/lifecycle.md | 2 + .../docs/ko/reference/cli/lifecycle.md | 3 + .../content/docs/reference/cli/lifecycle.md | 3 + .../docs/ru/reference/cli/lifecycle.md | 3 + .../docs/zh-cn/reference/cli/lifecycle.md | 2 + src/codex/shim.ts | 242 ++++++++++++++++-- tests/codex-shim.test.ts | 109 ++++++++ 7 files changed, 344 insertions(+), 20 deletions(-) diff --git a/docs-site/src/content/docs/ja/reference/cli/lifecycle.md b/docs-site/src/content/docs/ja/reference/cli/lifecycle.md index 17e387da6..1f8c593e9 100644 --- a/docs-site/src/content/docs/ja/reference/cli/lifecycle.md +++ b/docs-site/src/content/docs/ja/reference/cli/lifecycle.md @@ -183,6 +183,8 @@ Windows では、タスク スケジューラ エントリを作成するには インストールまたは修復を確定する前に、OpenCodex はサービス起動をバイパスした状態で、保存済みランチャーを `--version` 付きで実行します。ランチャーが `codex` を再び shim に解決する、0 以外で終了する、5 秒を超える、子プロセスを残す、または安全に検証・クリーンアップできない場合、変更を拒否してロールバックします。したがって `codex-shim install` は無条件のインストールではありません。拒否された場合は、PATH エントリが具体的な実行ファイルまたはランチャーを指すよう Codex を再インストールしてから再試行してください。動的コマンドマネージャーのランチャーがこれらの検証を満たせない場合は、代わりに `ocx service install` を使用してください。 +アップグレード時には、現在の検証ガードを持たない既存の Unix shim を再生成して検証します。保存済みランチャーが安全でない場合、OpenCodex は危険な wrapper を残さず、古い shim を削除して元のランチャーを復元します。 + 完了した外部 Codex アップデートがインストールされている shim を上書きした場合、次の通常の `ocx` コマンドは安定した新しいランチャーをバックアップし、ディスパッチ前に shim を復元します。まだ変更中のランチャーは変更されず、後で再試行されます。修復の失敗は、要求されたコマンドを失敗させることなく警告します。手動フォールバック: `ocx codex-shim install`。 `codexShimAutoRestore` を `false` に設定するか、プロセス レベルのオプトアウトの場合は `OPENCODEX_CODEX_SHIM_AUTO_RESTORE=0` を設定します。 |サブコマンド |アクション | diff --git a/docs-site/src/content/docs/ko/reference/cli/lifecycle.md b/docs-site/src/content/docs/ko/reference/cli/lifecycle.md index d76c8f96a..14d8db2cf 100644 --- a/docs-site/src/content/docs/ko/reference/cli/lifecycle.md +++ b/docs-site/src/content/docs/ko/reference/cli/lifecycle.md @@ -241,6 +241,9 @@ PATH 위의 스크립트 기반 `codex` 런처를 가벼운 자동 시작 스크 변경을 거부하고 롤백합니다. 따라서 `codex-shim install`은 무조건 성공하는 명령이 아닙니다. 거부되면 PATH 항목이 구체적인 실행 파일 또는 런처를 가리키도록 Codex를 다시 설치한 뒤 재시도하세요. 동적 명령 관리자의 런처가 이 검증을 충족할 수 없다면 대신 `ocx service install`을 사용하세요. +업그레이드할 때 현재 검증 가드가 없는 기존 Unix shim은 다시 생성하고 검증합니다. 저장된 런처가 +안전하지 않으면 OpenCodex는 위험한 wrapper를 그대로 두지 않고 구버전 shim을 제거한 뒤 원래 +런처를 복원합니다. 완료된 외부 Codex 업데이트가 설치된 shim을 덮어쓰면, 다음 일반 `ocx` 명령이 안정적인 새 런처를 백업하고 명령을 처리하기 전에 shim을 복원합니다. 아직 변경 중인 런처는 건드리지 않고 나중에 다시 시도합니다. diff --git a/docs-site/src/content/docs/reference/cli/lifecycle.md b/docs-site/src/content/docs/reference/cli/lifecycle.md index 12780c1a1..1b0ed9044 100644 --- a/docs-site/src/content/docs/reference/cli/lifecycle.md +++ b/docs-site/src/content/docs/reference/cli/lifecycle.md @@ -293,6 +293,9 @@ service startup is bypassed. It refuses the change and rolls back when the launc cannot be validated and cleaned up safely. Therefore `codex-shim install` is not unconditional. If it is refused, reinstall Codex so the PATH entry is a concrete executable or launcher and retry; use `ocx service install` instead when a dynamic command-manager launcher cannot meet these checks. +During upgrades, an installed Unix shim that lacks the current validation guard is regenerated and +probed. If its saved launcher is unsafe, OpenCodex removes the obsolete shim and restores the +original launcher instead of leaving the unsafe wrapper installed. Launcher installation alone does not prove that Codex requests will use OpenCodex. After a healthy install, the command checks the current Codex routing and reports a warning instead of a green result diff --git a/docs-site/src/content/docs/ru/reference/cli/lifecycle.md b/docs-site/src/content/docs/ru/reference/cli/lifecycle.md index be01664eb..2696370cc 100644 --- a/docs-site/src/content/docs/ru/reference/cli/lifecycle.md +++ b/docs-site/src/content/docs/ru/reference/cli/lifecycle.md @@ -260,6 +260,9 @@ target'ы `codex.exe` не трогаются, чтобы не ломать то безусловной установкой. После отказа переустановите Codex так, чтобы запись в `PATH` указывала на конкретный исполняемый файл или launcher, и повторите попытку. Если динамический launcher менеджера команд не проходит эти проверки, используйте вместо него `ocx service install`. +При обновлении установленный Unix shim без текущей validation-защиты пересоздаётся и проверяется. +Если сохранённый launcher небезопасен, OpenCodex удаляет устаревший shim и восстанавливает исходный +launcher, а не оставляет небезопасный wrapper установленным. Если завершённое внешнее обновление Codex перезаписало установленный shim, следующая обычная команда `ocx` сохранит новый стабильный launcher и восстановит shim перед выполнением запроса. diff --git a/docs-site/src/content/docs/zh-cn/reference/cli/lifecycle.md b/docs-site/src/content/docs/zh-cn/reference/cli/lifecycle.md index b7d976bb9..4d103f0f0 100644 --- a/docs-site/src/content/docs/zh-cn/reference/cli/lifecycle.md +++ b/docs-site/src/content/docs/zh-cn/reference/cli/lifecycle.md @@ -180,6 +180,8 @@ ocx service uninstall 提交安装或修复前，OpenCodex 会在跳过服务启动的情况下，用 `--version` 运行已保存的启动器。如果启动器把 `codex` 再次解析到 shim、以非零状态退出、运行超过五秒、留下仍在运行的子进程，或无法被安全验证和清理，OpenCodex 会拒绝并回滚更改。因此 `codex-shim install` 并不是无条件安装。若被拒绝，请重新安装 Codex，使 PATH 条目指向具体的可执行文件或启动器，然后重试；如果动态命令管理器的启动器无法满足这些检查，请改用 `ocx service install`。 +升级时，缺少当前验证保护的已安装 Unix shim 会被重新生成并接受探测。如果保存的启动器不安全，OpenCodex 会移除旧 shim 并恢复原始启动器，而不是保留不安全的 wrapper。 + 仅安装启动器并不能证明 Codex 请求会经过 OpenCodex。完成健康安装后，命令会检查当前 Codex 路由；当路由由外部配置、用户自有网关管理或无法验证时，会显示警告而不是绿色成功。若出站代理变量只存在于当前进程，而 `config.proxy` 未设置或无法解析，也会给出警告，因为 Codex 启动器和后台服务未必继承该环境。这些检查只读且绝不会打印代理值；在依赖自动启动前，请先处理提示的交接配置并运行 `ocx doctor`。 如果已完成的外部 Codex 更新覆盖了已安装的 shim，下一次普通的 `ocx` 命令会先备份稳定的新启动器，再在分发前恢复 shim。仍在变动中的启动器会保持不动，并在稍后重试。修复失败只会警告，不会让所请求的命令失败；手动回退：`ocx codex-shim install`。将 `codexShimAutoRestore` 设为 `false`，或设置 `OPENCODEX_CODEX_SHIM_AUTO_RESTORE=0`，即可在进程级别关闭自动恢复。 diff --git a/src/codex/shim.ts b/src/codex/shim.ts index 5d040bc3b..1f02c088a 100644 --- a/src/codex/shim.ts +++ b/src/codex/shim.ts @@ -33,29 +33,28 @@ import { isWslRuntime, wslAutomountRoot } from "./home"; import { truncateRetainedUtf8 } from "../lib/admission"; const SHIM_MARKER = "opencodex codex autostart shim"; +const UNIX_SHIM_REVISION_MARKER = "opencodex unix codex shim revision 2"; const CODEX_SHIM_PROBE_BYTES = 16 * 1024; export const CODEX_SHIM_REPLACEMENT_STABLE_MS = 100; export const CODEX_SHIM_STATE_MAX_BYTES = 1024 * 1024; const CODEX_SHIM_RESTORE_LOCK_STALE_MS = 30_000; const CODEX_SHIM_INSTALL_PROBE_TIMEOUT_MS = 5_000; const CODEX_SHIM_INSTALL_PROBE_EXIT_TIMEOUT_MS = 1_000; -const CODEX_SHIM_INSTALL_PROBE_REENTRY_GRACE_MS = 100; const CODEX_SHIM_REENTRY_EXIT_CODE = 126; const CODEX_SHIM_REENTRY_DIAGNOSTIC = "opencodex: saved Codex launcher resolved back to the autostart shim; run ocx codex-shim uninstall and reinstall Codex before enabling codexAutoStart."; const CODEX_SHIM_INSTALL_PROBE_SCRIPT = ` const { spawn } = require("node:child_process"); const { readFileSync, writeFileSync } = require("node:fs"); -const [markerPath, reentryPath, groupPath, stderrPath, launcherShellPath, wrapperPath, timeoutRaw, stderrLimitRaw, stderrDrainRaw, reentryGraceRaw] = process.argv.slice(1); +const [markerPath, reentryPath, groupPath, stderrPath, launcherShellPath, wrapperPath, timeoutRaw, stderrLimitRaw, stderrDrainRaw] = process.argv.slice(1); const timeoutMs = Number.parseInt(timeoutRaw, 10); const stderrLimit = Number.parseInt(stderrLimitRaw, 10); const stderrDrainMs = Number.parseInt(stderrDrainRaw, 10); -const reentryGraceMs = Number.parseInt(reentryGraceRaw, 10); const stderrChunks = []; let stderrBytes = 0; let launcher; +let probeLease; let timer; let stderrDrainTimer; -let reentryGraceTimer; let reentryPollTimer; let marker = ""; let finished = false; @@ -111,7 +110,6 @@ function finish(status) { finished = true; if (timer) clearTimeout(timer); if (stderrDrainTimer) clearTimeout(stderrDrainTimer); - if (reentryGraceTimer) clearTimeout(reentryGraceTimer); if (reentryPollTimer) clearInterval(reentryPollTimer); if (!marker && reentryDetected()) setMarker("recursive"); if (!marker && groupAlive()) { @@ -124,37 +122,45 @@ function finish(status) { function finishAfterStderr(status) { if (finished) return; - if (timer) { - clearTimeout(timer); - timer = undefined; - } - if (!launcher || !launcher.stderr) { + if (!launcher || !launcher.stderr || !probeLease) { finish(status); return; } let stderrEnded = launcher.stderr.readableEnded; - let graceElapsed = false; + let leaseEnded = probeLease.readableEnded; const finishWhenReady = () => { - if (stderrEnded && graceElapsed) finish(status); + if (stderrEnded && leaseEnded) finish(status); }; launcher.stderr.once("end", () => { stderrEnded = true; finishWhenReady(); }); - reentryGraceTimer = setTimeout(() => { - graceElapsed = true; + probeLease.once("end", () => { + leaseEnded = true; + finishWhenReady(); + }); + stderrDrainTimer = setTimeout(() => { + stderrEnded = true; + if (!marker && groupAlive()) { + setMarker("descendants"); + killGroup(); + finish(125); + return; + } finishWhenReady(); - }, reentryGraceMs); - stderrDrainTimer = setTimeout(() => finish(status), stderrDrainMs); + }, stderrDrainMs); + finishWhenReady(); } try { launcher = spawn(launcherShellPath, [wrapperPath, "--version"], { detached: true, env: process.env, - stdio: ["ignore", "ignore", "pipe"], + stdio: ["ignore", "ignore", "pipe", "pipe"], }); if (!launcher.pid) throw new Error("Codex shim probe launcher has no pid"); + probeLease = launcher.stdio[3]; + if (!probeLease) throw new Error("Codex shim probe launcher has no descendant lease pipe"); writeExclusive(groupPath, String(launcher.pid) + "\\n"); launcher.stderr.on("data", appendStderr); reentryPollTimer = setInterval(checkReentry, 10); @@ -166,6 +172,7 @@ try { timer = setTimeout(() => { setMarker("timeout"); killGroup(); + finish(124); }, timeoutMs); } catch (error) { appendStderr(String(error)); @@ -288,6 +295,7 @@ function isHealthyShim(path: string, platform: NodeJS.Platform): boolean { try { const content = readFileSync(path, "utf8"); if (content.length < 180 || !content.includes(SHIM_MARKER) || !content.includes("ensure")) return false; + if (platform !== "win32" && !content.includes(UNIX_SHIM_REVISION_MARKER)) return false; if (platform !== "win32" && (lstatSync(path).mode & 0o111) === 0) return false; return true; } catch { @@ -379,6 +387,10 @@ function isHealthyShimProbe(probe: StableShimPathProbe, platform: NodeJS.Platfor return platform === "win32" || (mode & 0o111) !== 0; } +function isCurrentUnixShimProbe(probe: StableShimPathProbe): boolean { + return probe.prefix.includes(UNIX_SHIM_REVISION_MARKER); +} + function hasUsableBackingPath(file: ShimFileState): boolean { return [existsSync(file.backupPath) ? file.backupPath : undefined, file.realPath] .some(path => { @@ -523,6 +535,7 @@ export function buildUnixCodexShim(realCodexPath: string, bunPath: string, cliPa const valueOptions = CODEX_GLOBAL_OPTIONS_WITH_VALUE.join("|"); return `#!/usr/bin/env sh # ${SHIM_MARKER} +# ${UNIX_SHIM_REVISION_MARKER} if [ "\${OCX_SHIM_PROBE:-}" = "1" ]; then if [ "\${OCX_SHIM_PROBE_ACTIVE:-}" = "1" ]; then if [ -n "\${OCX_SHIM_PROBE_REENTRY_PATH:-}" ]; then @@ -648,7 +661,6 @@ function probeUnixShimInstall(wrapperPath: string): UnixShimProbeResult { String(CODEX_SHIM_INSTALL_PROBE_TIMEOUT_MS), String(MAX_DIAGNOSTIC_VALUE_BYTES), String(CODEX_SHIM_INSTALL_PROBE_EXIT_TIMEOUT_MS), - String(CODEX_SHIM_INSTALL_PROBE_REENTRY_GRACE_MS), ], { encoding: "utf8", env, @@ -1415,10 +1427,184 @@ function applyGuardedRefreshTransaction( return true; } +interface ObsoleteUnixShimJournalEntry { + file: ShimFileState; + stagedWrapperPath: string; + priorWrapperFingerprint: ShimPathFingerprint; + backingFingerprint: ShimPathFingerprint; + writtenWrapperFingerprint?: ShimPathFingerprint; + wrapperWriteStarted: boolean; +} + +function rollbackObsoleteUnixShimRefresh(journal: readonly ObsoleteUnixShimJournalEntry[]): Error[] { + const errors: Error[] = []; + const attempt = (operation: () => void): void => { + try { + operation(); + } catch (error) { + errors.push(error instanceof Error ? error : new Error(String(error))); + } + }; + for (const entry of [...journal].reverse()) { + attempt(() => { + const wrapper = stableShimPathProbe(entry.file.wrapperPath); + const ownsWrapper = entry.wrapperWriteStarted + && wrapper !== null + && (entry.writtenWrapperFingerprint + ? sameFingerprint(wrapper.fingerprint, entry.writtenWrapperFingerprint) + : wrapper.prefix.includes(UNIX_SHIM_REVISION_MARKER)); + if (ownsWrapper) unlinkSync(entry.file.wrapperPath); + }); + attempt(() => { + if (!existsSync(entry.stagedWrapperPath)) return; + if (existsSync(entry.file.wrapperPath)) unlinkSync(entry.stagedWrapperPath); + else renameSync(entry.stagedWrapperPath, entry.file.wrapperPath); + }); + } + return errors; +} + +function refreshObsoleteUnixShims(files: readonly ShimFileState[]): { installed: boolean; message: string } { + if (process.platform === "win32") { + return { installed: false, message: "Codex autostart shim is already current." }; + } + const candidates = files.filter(file => { + if (file.preserveOnly || file.wrapperPath !== file.originalPath || !existsSync(file.wrapperPath)) return false; + const probe = stableShimPathProbe(file.wrapperPath); + return probe !== null && probe.prefix.includes(SHIM_MARKER) && !isCurrentUnixShimProbe(probe); + }); + if (candidates.length === 0) { + return { installed: false, message: "Codex autostart shim upgrade deferred because tracked launchers changed." }; + } + + const journal: ObsoleteUnixShimJournalEntry[] = []; + const transactionId = `${process.pid}-${randomUUID()}`; + let applyError: Error | null = null; + for (const [index, file] of candidates.entries()) { + const wrapper = stableShimPathProbe(file.wrapperPath); + const backing = stableShimPathProbe(file.backupPath); + if (!wrapper || !wrapper.prefix.includes(SHIM_MARKER) || isCurrentUnixShimProbe(wrapper) || !backing) { + applyError = new Error("Codex autostart shim upgrade inputs changed before regeneration"); + break; + } + const entry: ObsoleteUnixShimJournalEntry = { + file, + stagedWrapperPath: `${file.wrapperPath}.upgrade-${transactionId}-${index}`, + priorWrapperFingerprint: wrapper.fingerprint, + backingFingerprint: backing.fingerprint, + wrapperWriteStarted: false, + }; + journal.push(entry); + try { + renameSync(file.wrapperPath, entry.stagedWrapperPath); + const stagedWrapper = stableShimPathProbe(entry.stagedWrapperPath); + if (!stagedWrapper + || stagedWrapper.fingerprint.dev !== entry.priorWrapperFingerprint.dev + || stagedWrapper.fingerprint.ino !== entry.priorWrapperFingerprint.ino + || stagedWrapper.fingerprint.kind !== entry.priorWrapperFingerprint.kind + || stagedWrapper.fingerprint.mode !== entry.priorWrapperFingerprint.mode + || stagedWrapper.fingerprint.size !== entry.priorWrapperFingerprint.size + || stagedWrapper.fingerprint.mtimeMs !== entry.priorWrapperFingerprint.mtimeMs) { + throw new Error("Codex autostart shim upgrade could not fingerprint the staged wrapper"); + } + entry.wrapperWriteStarted = true; + writeShim(file.wrapperPath, file.realPath ?? file.backupPath); + const writtenWrapper = stableShimPathProbe(file.wrapperPath); + if (!writtenWrapper || !isCurrentUnixShimProbe(writtenWrapper)) { + throw new Error("Codex autostart shim upgrade could not fingerprint the regenerated wrapper"); + } + entry.writtenWrapperFingerprint = writtenWrapper.fingerprint; + } catch (error) { + applyError = error instanceof Error ? error : new Error(String(error)); + break; + } + } + + let unsafe: UnixShimProbeResult = null; + let probeError: Error | null = null; + if (!applyError) { + try { + unsafe = probeUnixShimFiles(candidates); + } catch (error) { + probeError = error instanceof Error ? error : new Error(String(error)); + } + } + const changedDuringProbe = !applyError && !probeError && journal.some(entry => { + const wrapper = stableShimPathProbe(entry.file.wrapperPath); + const backing = stableShimPathProbe(entry.file.backupPath); + return !wrapper || !entry.writtenWrapperFingerprint + || !sameFingerprint(wrapper.fingerprint, entry.writtenWrapperFingerprint) + || !backing || !sameFingerprint(backing.fingerprint, entry.backingFingerprint); + }); + + if (applyError || probeError || changedDuringProbe) { + const rollbackErrors = rollbackObsoleteUnixShimRefresh(journal); + if (applyError || probeError || rollbackErrors.length > 0) { + throw new AggregateError( + [...(applyError ? [applyError] : []), ...(probeError ? [probeError] : []), ...rollbackErrors], + "Codex autostart shim upgrade failed", + ); + } + return { installed: false, message: "Codex autostart shim upgrade deferred because tracked launchers changed." }; + } + + if (unsafe) { + const cleanupErrors: Error[] = []; + for (const entry of [...journal].reverse()) { + try { + const wrapper = stableShimPathProbe(entry.file.wrapperPath); + if (!wrapper || !entry.writtenWrapperFingerprint + || !sameFingerprint(wrapper.fingerprint, entry.writtenWrapperFingerprint)) { + throw new Error("Codex autostart shim upgrade lost wrapper ownership before removal"); + } + const backing = stableShimPathProbe(entry.file.backupPath); + if (!backing || !sameFingerprint(backing.fingerprint, entry.backingFingerprint)) { + throw new Error("Codex autostart shim upgrade backing launcher changed before restoration"); + } + unlinkSync(entry.file.wrapperPath); + renameSync(entry.file.backupPath, entry.file.originalPath); + if (existsSync(entry.stagedWrapperPath)) unlinkSync(entry.stagedWrapperPath); + } catch (error) { + cleanupErrors.push(error instanceof Error ? error : new Error(String(error))); + } + } + if (cleanupErrors.length > 0) { + throw new AggregateError(cleanupErrors, "Codex autostart shim upgrade safety removal failed"); + } + if (existsSync(statePath())) unlinkSync(statePath()); + return { + installed: false, + message: "Removed an obsolete Codex autostart shim because its saved launcher failed current validation. The original launcher was restored; reinstall Codex as a concrete executable before enabling codexAutoStart.", + }; + } + + const cleanupErrors: Error[] = []; + for (const entry of journal) { + try { + if (existsSync(entry.stagedWrapperPath)) unlinkSync(entry.stagedWrapperPath); + } catch (error) { + cleanupErrors.push(error instanceof Error ? error : new Error(String(error))); + } + } + if (cleanupErrors.length > 0) throw new AggregateError(cleanupErrors, "Codex autostart shim upgrade cleanup failed"); + return { + installed: true, + message: `Upgraded Codex autostart shim at ${candidates.map(file => file.wrapperPath).join(", ")} and validated the saved launcher.`, + }; +} + function installCodexShimInternal(options: InstallCodexShimInternalOptions): { installed: boolean; message: string } { const existing = readState(); if (existing) { const files = stateFiles(existing); + if (!options.expectedReplacements && process.platform !== "win32") { + const hasObsoleteShim = files.some(file => { + if (file.preserveOnly) return false; + const probe = stableShimPathProbe(file.wrapperPath); + return probe !== null && probe.prefix.includes(SHIM_MARKER) && !isCurrentUnixShimProbe(probe); + }); + if (hasObsoleteShim) return refreshObsoleteUnixShims(files); + } if (options.expectedReplacements) { const operations = planGuardedRefreshTransaction(files, options.expectedReplacements); if (!operations || operations.length === 0) { @@ -1604,6 +1790,7 @@ export function autoRestoreCodexShim(options: { const files = stateFiles(state); const replacementProbes = new Map(); + const obsoleteShimProbes = new Map(); const seen = new Set(); let healthyCount = 0; for (const file of files) { @@ -1618,15 +1805,19 @@ export function autoRestoreCodexShim(options: { if (!probe) return { status: "deferred" }; if (probe.prefix.includes(SHIM_MARKER)) { if (!isHealthyShimProbe(probe, state.platform)) return { status: "ineligible" }; + if (state.platform !== "win32" && !isCurrentUnixShimProbe(probe)) { + obsoleteShimProbes.set(file.wrapperPath, probe); + continue; + } healthyCount += 1; continue; } replacementProbes.set(file.wrapperPath, probe); } - if (replacementProbes.size === 0) return { status: "healthy" }; + if (replacementProbes.size === 0 && obsoleteShimProbes.size === 0) return { status: "healthy" }; if (!options.enabled()) return { status: "disabled" }; - if (files.length > 1 && healthyCount > 0) { + if (files.length > 1 && (healthyCount > 0 || (replacementProbes.size > 0 && obsoleteShimProbes.size > 0))) { return { status: "deferred", message: "Codex shim auto-restore deferred because tracked launcher siblings are in a mixed shim/replacement state.", @@ -1638,6 +1829,17 @@ export function autoRestoreCodexShim(options: { try { options.afterRestoreLockAcquired?.(); (options.stabilitySleep ?? Bun.sleepSync)(CODEX_SHIM_REPLACEMENT_STABLE_MS); + if (obsoleteShimProbes.size > 0) { + for (const [path, firstProbe] of obsoleteShimProbes) { + const secondProbe = stableShimPathProbe(path); + if (!secondProbe || isCurrentUnixShimProbe(secondProbe) + || !sameStableShimPathProbe(firstProbe, secondProbe)) return { status: "deferred" }; + } + const result = installCodexShimInternal({ allowFreshInstall: false }); + return result.installed + ? { status: "restored", message: result.message } + : { status: "ineligible", message: result.message }; + } const expectedReplacements = new Map(); for (const [path, firstProbe] of replacementProbes) { const secondProbe = stableShimPathProbe(path); diff --git a/tests/codex-shim.test.ts b/tests/codex-shim.test.ts index c2894c5e6..7ec2e1ee4 100644 --- a/tests/codex-shim.test.ts +++ b/tests/codex-shim.test.ts @@ -6,6 +6,7 @@ import { tmpdir } from "node:os"; import { autoRestoreCodexShim, buildUnixCodexShim, buildWindowsCodexShim, buildWindowsPowerShellCodexShim, diagnoseCodexShim, findCodexOnPath, installCodexShim, isWindowsInteropDir, lastCodexDiscoveryError, setCodexShimGuardedWriteHookForTests, setCodexShimProbeHookForTests, setCodexShimProbeShellForTests, uninstallCodexShim } from "../src/codex/shim"; const SHIM_MARKER = "opencodex codex autostart shim"; +const UNIX_SHIM_REVISION_MARKER = "opencodex unix codex shim revision 2"; const skipStabilityWait = () => {}; const python3Path = process.platform === "win32" ? "" @@ -487,9 +488,15 @@ exit 0 const childPidPath = join(home, "detached-reentry.pid"); const original = `#!${python3Path} import os +import time +os.set_inheritable(3, True) pid = os.fork() if pid == 0: os.setsid() + stderr_fd = os.open(os.devnull, os.O_WRONLY) + os.dup2(stderr_fd, 2) + os.close(stderr_fd) + time.sleep(0.25) os.execvpe("codex", ["codex", "--version"], os.environ) with open(${JSON.stringify(childPidPath)}, "w", encoding="utf-8") as handle: handle.write(str(pid)) @@ -966,6 +973,108 @@ printf '%s\\n' child-codex }); }); + test("auto-restore upgrades an obsolete Unix shim and validates its saved launcher", () => { + if (process.platform === "win32") return; + withInstalledShim(({ wrappers, backups, statePath }) => { + const current = readFileSync(wrappers[0], "utf8"); + const obsolete = current.replace(`# ${UNIX_SHIM_REVISION_MARKER}\n`, ""); + const oldBackup = readFileSync(backups[0]); + const oldState = readFileSync(statePath); + expect(obsolete).not.toBe(current); + writeFileSync(wrappers[0], obsolete, "utf8"); + + const result = autoRestoreCodexShim({ enabled: () => true, stabilitySleep: skipStabilityWait }); + + expect(result.status).toBe("restored"); + expect(result.message).toContain("Upgraded Codex autostart shim"); + expect(readFileSync(wrappers[0], "utf8")).toContain(UNIX_SHIM_REVISION_MARKER); + expect(readFileSync(backups[0])).toEqual(oldBackup); + expect(readFileSync(statePath)).toEqual(oldState); + expect(diagnoseCodexShim()).toMatchObject({ installed: true, healthy: true }); + }); + }); + + test("manual install removes an obsolete Unix shim when its saved launcher recurses", () => { + if (process.platform === "win32") return; + withInstalledShim(({ binDir, wrappers, backups, statePath }) => { + const current = readFileSync(wrappers[0], "utf8"); + const obsolete = current.replace(`# ${UNIX_SHIM_REVISION_MARKER}\n`, ""); + const dynamicLauncher = join(binDir, "obsolete-dynamic-launcher"); + const recursiveLauncher = `#!/bin/sh\nexec "${dynamicLauncher}" "$@"\n`; + writeFileSync(dynamicLauncher, "#!/bin/sh\nexec codex \"$@\"\n", "utf8"); + writeFileSync(backups[0], recursiveLauncher, "utf8"); + writeFileSync(wrappers[0], obsolete, "utf8"); + chmodSync(dynamicLauncher, 0o755); + chmodSync(backups[0], 0o755); + + const result = installCodexShim(); + + expect(result.installed).toBe(false); + expect(result.message).toContain("Removed an obsolete Codex autostart shim"); + expect(result.message).toContain("original launcher was restored"); + expect(readFileSync(wrappers[0], "utf8")).toBe(recursiveLauncher); + expect(existsSync(backups[0])).toBe(false); + expect(existsSync(statePath)).toBe(false); + expect(diagnoseCodexShim()).toMatchObject({ installed: false, healthy: false }); + }); + }); + + test("obsolete Unix shim upgrade rolls back when probe infrastructure throws", () => { + if (process.platform === "win32") return; + withInstalledShim(({ wrappers, backups, statePath }) => { + const current = readFileSync(wrappers[0], "utf8"); + const obsolete = current.replace(`# ${UNIX_SHIM_REVISION_MARKER}\n`, ""); + const oldBackup = readFileSync(backups[0]); + const oldState = readFileSync(statePath); + writeFileSync(wrappers[0], obsolete, "utf8"); + setCodexShimProbeHookForTests(() => { throw new Error("synthetic obsolete upgrade probe failure"); }); + + let failure: unknown; + try { + installCodexShim(); + } catch (error) { + failure = error; + } finally { + setCodexShimProbeHookForTests(null); + } + + expect(failure).toBeInstanceOf(AggregateError); + expect(readFileSync(wrappers[0], "utf8")).toBe(obsolete); + expect(readFileSync(backups[0])).toEqual(oldBackup); + expect(readFileSync(statePath)).toEqual(oldState); + }); + }); + + test("obsolete Unix shim upgrade preserves a concurrent wrapper replacement", () => { + if (process.platform === "win32") return; + withInstalledShim(({ binDir, wrappers, backups, statePath }) => { + const current = readFileSync(wrappers[0], "utf8"); + const obsolete = current.replace(`# ${UNIX_SHIM_REVISION_MARKER}\n`, ""); + const concurrent = successfulLauncher("obsolete upgrade concurrent replacement"); + const oldBackup = readFileSync(backups[0]); + const oldState = readFileSync(statePath); + writeFileSync(wrappers[0], obsolete, "utf8"); + setCodexShimProbeHookForTests(() => { + writeFileSync(wrappers[0], concurrent, "utf8"); + chmodSync(wrappers[0], 0o755); + }); + + let result!: ReturnType; + try { + result = installCodexShim(); + } finally { + setCodexShimProbeHookForTests(null); + } + + expect(result.installed).toBe(false); + expect(result.message).toContain("upgrade deferred because tracked launchers changed"); + expect(readFileSync(wrappers[0], "utf8")).toBe(concurrent); + expect(readFileSync(backups[0])).toEqual(oldBackup); + expect(readFileSync(statePath)).toEqual(oldState); + expect(readdirSync(binDir).some(name => name.includes(".upgrade-"))).toBe(false); + }); + }); + test("stable shim replacement restores through the shared install transaction", () => { withInstalledShim(({ wrappers, backups }) => { const replacements = wrappers.map((wrapper, index) => successfulLauncher(`replacement-${index}`)); From ae1d5423e65e7a1bce4c0d143a3db64dca0ac100 Mon Sep 17 00:00:00 2001 From: comfuture Date: Tue, 11 Aug 2026 13:26:02 +0900 Subject: [PATCH 12/16] fix(codex): observe closed-fd probe reentry --- src/codex/shim.ts | 31 +++++++++++++++++++++++++++++-- tests/codex-shim.test.ts | 20 +++++++++++++------- 2 files changed, 42 insertions(+), 9 deletions(-) diff --git a/src/codex/shim.ts b/src/codex/shim.ts index 1f02c088a..67d5b7241 100644 --- a/src/codex/shim.ts +++ b/src/codex/shim.ts @@ -45,16 +45,19 @@ const CODEX_SHIM_REENTRY_DIAGNOSTIC = "opencodex: saved Codex launcher resolved const CODEX_SHIM_INSTALL_PROBE_SCRIPT = ` const { spawn } = require("node:child_process"); const { readFileSync, writeFileSync } = require("node:fs"); -const [markerPath, reentryPath, groupPath, stderrPath, launcherShellPath, wrapperPath, timeoutRaw, stderrLimitRaw, stderrDrainRaw] = process.argv.slice(1); +const [markerPath, reentryPath, groupPath, stderrPath, launcherShellPath, wrapperPath, timeoutRaw, stderrLimitRaw, stderrDrainRaw, observationRaw] = process.argv.slice(1); const timeoutMs = Number.parseInt(timeoutRaw, 10); const stderrLimit = Number.parseInt(stderrLimitRaw, 10); const stderrDrainMs = Number.parseInt(stderrDrainRaw, 10); +const observationMs = Number.parseInt(observationRaw, 10); +const probeStartedAt = Date.now(); const stderrChunks = []; let stderrBytes = 0; let launcher; let probeLease; let timer; let stderrDrainTimer; +let observationTimer; let reentryPollTimer; let marker = ""; let finished = false; @@ -110,6 +113,7 @@ function finish(status) { finished = true; if (timer) clearTimeout(timer); if (stderrDrainTimer) clearTimeout(stderrDrainTimer); + if (observationTimer) clearTimeout(observationTimer); if (reentryPollTimer) clearInterval(reentryPollTimer); if (!marker && reentryDetected()) setMarker("recursive"); if (!marker && groupAlive()) { @@ -122,14 +126,19 @@ function finish(status) { function finishAfterStderr(status) { if (finished) return; + if (timer) { + clearTimeout(timer); + timer = undefined; + } if (!launcher || !launcher.stderr || !probeLease) { finish(status); return; } let stderrEnded = launcher.stderr.readableEnded; let leaseEnded = probeLease.readableEnded; + let observationElapsed = false; const finishWhenReady = () => { - if (stderrEnded && leaseEnded) finish(status); + if (stderrEnded && leaseEnded && observationElapsed) finish(status); }; launcher.stderr.once("end", () => { stderrEnded = true; @@ -149,6 +158,17 @@ function finishAfterStderr(status) { } finishWhenReady(); }, stderrDrainMs); + const remainingObservationMs = Math.max(0, observationMs - (Date.now() - probeStartedAt)); + observationTimer = setTimeout(() => { + observationElapsed = true; + if (!leaseEnded) { + setMarker(groupAlive() ? "descendants" : "timeout"); + killGroup(); + finish(marker === "descendants" ? 125 : 124); + return; + } + finishWhenReady(); + }, remainingObservationMs); finishWhenReady(); } @@ -604,6 +624,7 @@ type UnixShimProbeResult = "cleanup" | "descendants" | "failed" | "recursive" | let codexShimProbeHookForTests: (() => void) | null = null; let codexShimProbeShellForTests: string | null = null; let codexShimGuardedWriteHookForTests: (() => void) | null = null; +let codexShimProbeObservationMs = CODEX_SHIM_INSTALL_PROBE_TIMEOUT_MS; /** Narrow deterministic seam for transaction rollback tests. */ export function setCodexShimProbeHookForTests(hook: (() => void) | null): void { @@ -615,6 +636,11 @@ export function setCodexShimProbeShellForTests(path: string | null): void { codexShimProbeShellForTests = path; } +/** Shortens the successful-launcher observation window only for focused tests. */ +export function setCodexShimProbeObservationMsForTests(value: number | null): void { + codexShimProbeObservationMs = value ?? CODEX_SHIM_INSTALL_PROBE_TIMEOUT_MS; +} + /** Narrow deterministic seam for guarded partial-write rollback tests. */ export function setCodexShimGuardedWriteHookForTests(hook: (() => void) | null): void { codexShimGuardedWriteHookForTests = hook; @@ -661,6 +687,7 @@ function probeUnixShimInstall(wrapperPath: string): UnixShimProbeResult { String(CODEX_SHIM_INSTALL_PROBE_TIMEOUT_MS), String(MAX_DIAGNOSTIC_VALUE_BYTES), String(CODEX_SHIM_INSTALL_PROBE_EXIT_TIMEOUT_MS), + String(codexShimProbeObservationMs), ], { encoding: "utf8", env, diff --git a/tests/codex-shim.test.ts b/tests/codex-shim.test.ts index 7ec2e1ee4..7b7670d8e 100644 --- a/tests/codex-shim.test.ts +++ b/tests/codex-shim.test.ts @@ -1,9 +1,9 @@ -import { describe, expect, test } from "bun:test"; +import { afterAll, describe, expect, test } from "bun:test"; import { spawnSync } from "node:child_process"; import { chmodSync, mkdirSync, mkdtempSync, writeFileSync, readFileSync, existsSync, readdirSync, rmSync, statSync, symlinkSync, utimesSync } from "node:fs"; import { delimiter, dirname, join } from "node:path"; import { tmpdir } from "node:os"; -import { autoRestoreCodexShim, buildUnixCodexShim, buildWindowsCodexShim, buildWindowsPowerShellCodexShim, diagnoseCodexShim, findCodexOnPath, installCodexShim, isWindowsInteropDir, lastCodexDiscoveryError, setCodexShimGuardedWriteHookForTests, setCodexShimProbeHookForTests, setCodexShimProbeShellForTests, uninstallCodexShim } from "../src/codex/shim"; +import { autoRestoreCodexShim, buildUnixCodexShim, buildWindowsCodexShim, buildWindowsPowerShellCodexShim, diagnoseCodexShim, findCodexOnPath, installCodexShim, isWindowsInteropDir, lastCodexDiscoveryError, setCodexShimGuardedWriteHookForTests, setCodexShimProbeHookForTests, setCodexShimProbeObservationMsForTests, setCodexShimProbeShellForTests, uninstallCodexShim } from "../src/codex/shim"; const SHIM_MARKER = "opencodex codex autostart shim"; const UNIX_SHIM_REVISION_MARKER = "opencodex unix codex shim revision 2"; @@ -11,6 +11,8 @@ const skipStabilityWait = () => {}; const python3Path = process.platform === "win32" ? "" : spawnSync("/bin/sh", ["-c", "command -v python3"], { encoding: "utf8" }).stdout.trim(); +setCodexShimProbeObservationMsForTests(20); +afterAll(() => setCodexShimProbeObservationMsForTests(null)); function prependPath(dir: string, current: string | undefined): string { return [dir, current].filter(Boolean).join(delimiter); @@ -478,7 +480,7 @@ exit 0 }); test.skipIf(process.platform === "win32" || !python3Path)( - "Unix install rejects recursive redispatch that escapes into a detached process group", + "Unix install rejects delayed detached redispatch after the launcher closes its lease fd", () => { const binDir = mkdtempSync(join(tmpdir(), "ocx-shim-install-detached-reentry-bin-")); const home = mkdtempSync(join(tmpdir(), "ocx-shim-install-detached-reentry-home-")); @@ -489,14 +491,14 @@ exit 0 const original = `#!${python3Path} import os import time -os.set_inheritable(3, True) +os.close(3) pid = os.fork() if pid == 0: os.setsid() stderr_fd = os.open(os.devnull, os.O_WRONLY) os.dup2(stderr_fd, 2) os.close(stderr_fd) - time.sleep(0.25) + time.sleep(0.5) os.execvpe("codex", ["codex", "--version"], os.environ) with open(${JSON.stringify(childPidPath)}, "w", encoding="utf-8") as handle: handle.write(str(pid)) @@ -505,6 +507,7 @@ os._exit(0) try { process.env.PATH = prependPath(binDir, oldPath); process.env.OPENCODEX_HOME = home; + setCodexShimProbeObservationMsForTests(1_500); writeFileSync(codexPath, original, "utf8"); chmodSync(codexPath, 0o755); @@ -519,6 +522,7 @@ os._exit(0) const childState = waitForProcessStop(childPid); expect(childState === "" || childState.startsWith("Z")).toBe(true); } finally { + setCodexShimProbeObservationMsForTests(20); if (oldPath === undefined) delete process.env.PATH; else process.env.PATH = oldPath; if (oldHome === undefined) delete process.env.OPENCODEX_HOME; @@ -1344,7 +1348,8 @@ exit 127 const firstScript = ` import { existsSync, readFileSync, readdirSync, utimesSync, writeFileSync } from "node:fs"; import { join } from "node:path"; - const { autoRestoreCodexShim } = await import(${JSON.stringify(shimModule)}); + const { autoRestoreCodexShim, setCodexShimProbeObservationMsForTests } = await import(${JSON.stringify(shimModule)}); + setCodexShimProbeObservationMsForTests(20); const result = autoRestoreCodexShim({ enabled: () => true, stabilitySleep: () => {}, @@ -1361,7 +1366,8 @@ exit 127 console.log(JSON.stringify(result)); `; const secondScript = ` - const { autoRestoreCodexShim } = await import(${JSON.stringify(shimModule)}); + const { autoRestoreCodexShim, setCodexShimProbeObservationMsForTests } = await import(${JSON.stringify(shimModule)}); + setCodexShimProbeObservationMsForTests(20); console.log(JSON.stringify(autoRestoreCodexShim({ enabled: () => true, stabilitySleep: () => {} }))); `; const childEnv = { ...process.env, PATH: binDir, OPENCODEX_HOME: home }; From 71ec54747ee1cddbca85c43c031e0b780bf87e1a Mon Sep 17 00:00:00 2001 From: comfuture Date: Tue, 11 Aug 2026 13:31:16 +0900 Subject: [PATCH 13/16] fix(codex): close rollback review gaps --- src/codex/shim.ts | 47 +++++++++----- tests/codex-shim.test.ts | 128 +++++++++++++++++++++++++++++++++++---- 2 files changed, 148 insertions(+), 27 deletions(-) diff --git a/src/codex/shim.ts b/src/codex/shim.ts index 67d5b7241..9fe3633e4 100644 --- a/src/codex/shim.ts +++ b/src/codex/shim.ts @@ -372,6 +372,12 @@ function sameFingerprint( : false); } +function sameFingerprintAfterRename(left: ShimPathFingerprint, right: ShimPathFingerprint): boolean { + // rename changes the outer directory entry ctime on macOS; every other field, + // including a symlink target fingerprint, must remain identical. + return sameFingerprint({ ...left, ctimeMs: 0 }, { ...right, ctimeMs: 0 }); +} + function stableShimPathProbe(path: string): StableShimPathProbe | null { const before = statFingerprint(path, false); if (!before) return null; @@ -624,6 +630,7 @@ type UnixShimProbeResult = "cleanup" | "descendants" | "failed" | "recursive" | let codexShimProbeHookForTests: (() => void) | null = null; let codexShimProbeShellForTests: string | null = null; let codexShimGuardedWriteHookForTests: (() => void) | null = null; +let codexShimFreshWriteHookForTests: (() => void) | null = null; let codexShimProbeObservationMs = CODEX_SHIM_INSTALL_PROBE_TIMEOUT_MS; /** Narrow deterministic seam for transaction rollback tests. */ @@ -646,6 +653,11 @@ export function setCodexShimGuardedWriteHookForTests(hook: (() => void) | null): codexShimGuardedWriteHookForTests = hook; } +/** Narrow deterministic seam for fresh-install partial-write rollback tests. */ +export function setCodexShimFreshWriteHookForTests(hook: (() => void) | null): void { + codexShimFreshWriteHookForTests = hook; +} + function readProbeMetadata(path: string, maxBytes: number): string | null { try { if (!existsSync(path)) return ""; @@ -756,8 +768,7 @@ function terminateUnixProcessGroup(groupId: number): void { if ((error as NodeJS.ErrnoException).code !== "ESRCH") throw error; } const deadline = Date.now() + CODEX_SHIM_INSTALL_PROBE_EXIT_TIMEOUT_MS; - const waiter = new Int32Array(new SharedArrayBuffer(4)); - while (Date.now() < deadline && unixProcessGroupAlive(groupId)) Atomics.wait(waiter, 0, 0, 10); + while (Date.now() < deadline && unixProcessGroupAlive(groupId)) Bun.sleepSync(10); if (unixProcessGroupAlive(groupId)) { throw new Error(`Codex shim install probe process group ${groupId} did not terminate`); } @@ -780,9 +791,10 @@ function rollbackFreshShimInstall(journal: readonly FreshShimInstallJournalEntry const wrapper = stableShimPathProbe(target.wrapperPath); const ownsWrapper = !target.preserveOnly && entry.wrapperWriteStarted - && entry.writtenWrapperFingerprint !== undefined && wrapper !== null - && sameFingerprint(wrapper.fingerprint, entry.writtenWrapperFingerprint); + && (entry.writtenWrapperFingerprint + ? sameFingerprint(wrapper.fingerprint, entry.writtenWrapperFingerprint) + : wrapper.prefix.includes(SHIM_MARKER)); if (ownsWrapper) unlinkSync(target.wrapperPath); else { try { @@ -1491,9 +1503,13 @@ function rollbackObsoleteUnixShimRefresh(journal: readonly ObsoleteUnixShimJourn return errors; } -function refreshObsoleteUnixShims(files: readonly ShimFileState[]): { installed: boolean; message: string } { +type ObsoleteUnixShimRefreshResult = + | { installed: true; message: string } + | { installed: false; deferred: boolean; message: string }; + +function refreshObsoleteUnixShims(files: readonly ShimFileState[]): ObsoleteUnixShimRefreshResult { if (process.platform === "win32") { - return { installed: false, message: "Codex autostart shim is already current." }; + return { installed: false, deferred: false, message: "Codex autostart shim is already current." }; } const candidates = files.filter(file => { if (file.preserveOnly || file.wrapperPath !== file.originalPath || !existsSync(file.wrapperPath)) return false; @@ -1501,7 +1517,7 @@ function refreshObsoleteUnixShims(files: readonly ShimFileState[]): { installed: return probe !== null && probe.prefix.includes(SHIM_MARKER) && !isCurrentUnixShimProbe(probe); }); if (candidates.length === 0) { - return { installed: false, message: "Codex autostart shim upgrade deferred because tracked launchers changed." }; + return { installed: false, deferred: true, message: "Codex autostart shim upgrade deferred because tracked launchers changed." }; } const journal: ObsoleteUnixShimJournalEntry[] = []; @@ -1526,12 +1542,7 @@ function refreshObsoleteUnixShims(files: readonly ShimFileState[]): { installed: renameSync(file.wrapperPath, entry.stagedWrapperPath); const stagedWrapper = stableShimPathProbe(entry.stagedWrapperPath); if (!stagedWrapper - || stagedWrapper.fingerprint.dev !== entry.priorWrapperFingerprint.dev - || stagedWrapper.fingerprint.ino !== entry.priorWrapperFingerprint.ino - || stagedWrapper.fingerprint.kind !== entry.priorWrapperFingerprint.kind - || stagedWrapper.fingerprint.mode !== entry.priorWrapperFingerprint.mode - || stagedWrapper.fingerprint.size !== entry.priorWrapperFingerprint.size - || stagedWrapper.fingerprint.mtimeMs !== entry.priorWrapperFingerprint.mtimeMs) { + || !sameFingerprintAfterRename(stagedWrapper.fingerprint, entry.priorWrapperFingerprint)) { throw new Error("Codex autostart shim upgrade could not fingerprint the staged wrapper"); } entry.wrapperWriteStarted = true; @@ -1572,7 +1583,7 @@ function refreshObsoleteUnixShims(files: readonly ShimFileState[]): { installed: "Codex autostart shim upgrade failed", ); } - return { installed: false, message: "Codex autostart shim upgrade deferred because tracked launchers changed." }; + return { installed: false, deferred: true, message: "Codex autostart shim upgrade deferred because tracked launchers changed." }; } if (unsafe) { @@ -1601,6 +1612,7 @@ function refreshObsoleteUnixShims(files: readonly ShimFileState[]): { installed: if (existsSync(statePath())) unlinkSync(statePath()); return { installed: false, + deferred: false, message: "Removed an obsolete Codex autostart shim because its saved launcher failed current validation. The original launcher was restored; reinstall Codex as a concrete executable before enabling codexAutoStart.", }; } @@ -1720,6 +1732,7 @@ function installCodexShimInternal(options: InstallCodexShimInternalOptions): { i if (!target.preserveOnly) { entry.wrapperWriteStarted = true; writeShim(target.wrapperPath, target.realPath ?? target.backupPath); + codexShimFreshWriteHookForTests?.(); if (process.platform !== "win32") { const writtenWrapper = stableShimPathProbe(target.wrapperPath); if (!writtenWrapper || !writtenWrapper.prefix.includes(SHIM_MARKER)) { @@ -1862,10 +1875,12 @@ export function autoRestoreCodexShim(options: { if (!secondProbe || isCurrentUnixShimProbe(secondProbe) || !sameStableShimPathProbe(firstProbe, secondProbe)) return { status: "deferred" }; } - const result = installCodexShimInternal({ allowFreshInstall: false }); + const result = refreshObsoleteUnixShims(files); return result.installed ? { status: "restored", message: result.message } - : { status: "ineligible", message: result.message }; + : result.deferred + ? { status: "deferred", message: result.message } + : { status: "ineligible", message: result.message }; } const expectedReplacements = new Map(); for (const [path, firstProbe] of replacementProbes) { diff --git a/tests/codex-shim.test.ts b/tests/codex-shim.test.ts index 7b7670d8e..ad9d09200 100644 --- a/tests/codex-shim.test.ts +++ b/tests/codex-shim.test.ts @@ -1,9 +1,9 @@ import { afterAll, describe, expect, test } from "bun:test"; import { spawnSync } from "node:child_process"; -import { chmodSync, mkdirSync, mkdtempSync, writeFileSync, readFileSync, existsSync, readdirSync, rmSync, statSync, symlinkSync, utimesSync } from "node:fs"; +import { chmodSync, copyFileSync, existsSync, lstatSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, rmSync, statSync, symlinkSync, utimesSync, writeFileSync } from "node:fs"; import { delimiter, dirname, join } from "node:path"; import { tmpdir } from "node:os"; -import { autoRestoreCodexShim, buildUnixCodexShim, buildWindowsCodexShim, buildWindowsPowerShellCodexShim, diagnoseCodexShim, findCodexOnPath, installCodexShim, isWindowsInteropDir, lastCodexDiscoveryError, setCodexShimGuardedWriteHookForTests, setCodexShimProbeHookForTests, setCodexShimProbeObservationMsForTests, setCodexShimProbeShellForTests, uninstallCodexShim } from "../src/codex/shim"; +import { autoRestoreCodexShim, buildUnixCodexShim, buildWindowsCodexShim, buildWindowsPowerShellCodexShim, diagnoseCodexShim, findCodexOnPath, installCodexShim, isWindowsInteropDir, lastCodexDiscoveryError, setCodexShimFreshWriteHookForTests, setCodexShimGuardedWriteHookForTests, setCodexShimProbeHookForTests, setCodexShimProbeObservationMsForTests, setCodexShimProbeShellForTests, uninstallCodexShim } from "../src/codex/shim"; const SHIM_MARKER = "opencodex codex autostart shim"; const UNIX_SHIM_REVISION_MARKER = "opencodex unix codex shim revision 2"; @@ -13,6 +13,7 @@ const python3Path = process.platform === "win32" : spawnSync("/bin/sh", ["-c", "command -v python3"], { encoding: "utf8" }).stdout.trim(); setCodexShimProbeObservationMsForTests(20); afterAll(() => setCodexShimProbeObservationMsForTests(null)); +const psPath = process.platform !== "win32" && existsSync("/bin/ps") ? "/bin/ps" : ""; function prependPath(dir: string, current: string | undefined): string { return [dir, current].filter(Boolean).join(delimiter); @@ -23,7 +24,18 @@ function successfulLauncher(label: string): string { } function processState(pid: number): string { - return spawnSync("/bin/ps", ["-o", "stat=", "-p", String(pid)], { encoding: "utf8" }).stdout.trim(); + if (!psPath) throw new Error("/bin/ps is required for process-state assertions"); + const result = spawnSync(psPath, ["-o", "stat=", "-p", String(pid)], { encoding: "utf8" }); + if (result.error || typeof result.stdout !== "string") { + throw new Error(`/bin/ps failed during process-state assertion: ${String(result.error ?? "missing stdout")}`); + } + return result.stdout.trim(); +} + +function obsoleteUnixShim(current: string): string { + const obsolete = current.replace(`# ${UNIX_SHIM_REVISION_MARKER}\n`, ""); + expect(obsolete).not.toBe(current); + return obsolete; } function waitForProcessStop(pid: number, timeoutMs = 1_000): string { @@ -434,6 +446,65 @@ exit 126 } }); + test.skipIf(process.platform === "win32" || !existsSync("/usr/bin/true"))( + "Unix install probes a concrete native executable through the generated wrapper", + () => { + const binDir = mkdtempSync(join(tmpdir(), "ocx-shim-install-native-bin-")); + const home = mkdtempSync(join(tmpdir(), "ocx-shim-install-native-home-")); + const oldPath = process.env.PATH; + const oldHome = process.env.OPENCODEX_HOME; + const codexPath = join(binDir, "codex"); + try { + process.env.PATH = prependPath(binDir, oldPath); + process.env.OPENCODEX_HOME = home; + copyFileSync("/usr/bin/true", codexPath); + chmodSync(codexPath, 0o755); + + const installed = installCodexShim(); + + expect(installed.installed).toBe(true); + expect(readFileSync(codexPath, "utf8")).toContain(SHIM_MARKER); + expect(lstatSync(`${codexPath}.opencodex-real`).isFile()).toBe(true); + } finally { + if (oldPath === undefined) delete process.env.PATH; + else process.env.PATH = oldPath; + if (oldHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = oldHome; + rmSync(binDir, { recursive: true, force: true }); + rmSync(home, { recursive: true, force: true }); + } + }, + ); + + test.skipIf(process.platform === "win32" || !existsSync("/usr/bin/true"))( + "Unix install probes a symlinked native executable through the generated wrapper", + () => { + const binDir = mkdtempSync(join(tmpdir(), "ocx-shim-install-native-link-bin-")); + const home = mkdtempSync(join(tmpdir(), "ocx-shim-install-native-link-home-")); + const oldPath = process.env.PATH; + const oldHome = process.env.OPENCODEX_HOME; + const codexPath = join(binDir, "codex"); + try { + process.env.PATH = prependPath(binDir, oldPath); + process.env.OPENCODEX_HOME = home; + symlinkSync("/usr/bin/true", codexPath); + + const installed = installCodexShim(); + + expect(installed.installed).toBe(true); + expect(readFileSync(codexPath, "utf8")).toContain(SHIM_MARKER); + expect(lstatSync(`${codexPath}.opencodex-real`).isSymbolicLink()).toBe(true); + } finally { + if (oldPath === undefined) delete process.env.PATH; + else process.env.PATH = oldPath; + if (oldHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = oldHome; + rmSync(binDir, { recursive: true, force: true }); + rmSync(home, { recursive: true, force: true }); + } + }, + ); + test("Unix install rejects a launcher that leaves a background descendant", () => { if (process.platform === "win32") return; @@ -680,6 +751,41 @@ wait "$child" } }); + test("Unix fresh install removes its marker-bearing partial wrapper before rollback", () => { + if (process.platform === "win32") return; + + const binDir = mkdtempSync(join(tmpdir(), "ocx-shim-install-partial-write-bin-")); + const home = mkdtempSync(join(tmpdir(), "ocx-shim-install-partial-write-home-")); + const oldPath = process.env.PATH; + const oldHome = process.env.OPENCODEX_HOME; + const codexPath = join(binDir, "codex"); + const original = successfulLauncher("partial-write-original"); + try { + process.env.PATH = prependPath(binDir, oldPath); + process.env.OPENCODEX_HOME = home; + writeFileSync(codexPath, original, "utf8"); + chmodSync(codexPath, 0o755); + setCodexShimFreshWriteHookForTests(() => { + writeFileSync(codexPath, `#!/bin/sh\n# ${SHIM_MARKER}\n`, "utf8"); + throw new Error("synthetic fresh partial write failure"); + }); + + expect(() => installCodexShim()).toThrow("synthetic fresh partial write failure"); + + expect(readFileSync(codexPath, "utf8")).toBe(original); + expect(existsSync(`${codexPath}.opencodex-real`)).toBe(false); + expect(existsSync(join(home, "codex-shim.json"))).toBe(false); + } finally { + setCodexShimFreshWriteHookForTests(null); + if (oldPath === undefined) delete process.env.PATH; + else process.env.PATH = oldPath; + if (oldHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = oldHome; + rmSync(binDir, { recursive: true, force: true }); + rmSync(home, { recursive: true, force: true }); + } + }); + test("Unix fresh install preserves a concurrent wrapper replacement after a successful probe", () => { if (process.platform === "win32") return; @@ -981,7 +1087,7 @@ printf '%s\\n' child-codex if (process.platform === "win32") return; withInstalledShim(({ wrappers, backups, statePath }) => { const current = readFileSync(wrappers[0], "utf8"); - const obsolete = current.replace(`# ${UNIX_SHIM_REVISION_MARKER}\n`, ""); + const obsolete = obsoleteUnixShim(current); const oldBackup = readFileSync(backups[0]); const oldState = readFileSync(statePath); expect(obsolete).not.toBe(current); @@ -1002,7 +1108,7 @@ printf '%s\\n' child-codex if (process.platform === "win32") return; withInstalledShim(({ binDir, wrappers, backups, statePath }) => { const current = readFileSync(wrappers[0], "utf8"); - const obsolete = current.replace(`# ${UNIX_SHIM_REVISION_MARKER}\n`, ""); + const obsolete = obsoleteUnixShim(current); const dynamicLauncher = join(binDir, "obsolete-dynamic-launcher"); const recursiveLauncher = `#!/bin/sh\nexec "${dynamicLauncher}" "$@"\n`; writeFileSync(dynamicLauncher, "#!/bin/sh\nexec codex \"$@\"\n", "utf8"); @@ -1027,7 +1133,7 @@ printf '%s\\n' child-codex if (process.platform === "win32") return; withInstalledShim(({ wrappers, backups, statePath }) => { const current = readFileSync(wrappers[0], "utf8"); - const obsolete = current.replace(`# ${UNIX_SHIM_REVISION_MARKER}\n`, ""); + const obsolete = obsoleteUnixShim(current); const oldBackup = readFileSync(backups[0]); const oldState = readFileSync(statePath); writeFileSync(wrappers[0], obsolete, "utf8"); @@ -1053,7 +1159,7 @@ printf '%s\\n' child-codex if (process.platform === "win32") return; withInstalledShim(({ binDir, wrappers, backups, statePath }) => { const current = readFileSync(wrappers[0], "utf8"); - const obsolete = current.replace(`# ${UNIX_SHIM_REVISION_MARKER}\n`, ""); + const obsolete = obsoleteUnixShim(current); const concurrent = successfulLauncher("obsolete upgrade concurrent replacement"); const oldBackup = readFileSync(backups[0]); const oldState = readFileSync(statePath); @@ -1063,15 +1169,15 @@ printf '%s\\n' child-codex chmodSync(wrappers[0], 0o755); }); - let result!: ReturnType; + let result!: ReturnType; try { - result = installCodexShim(); + result = autoRestoreCodexShim({ enabled: () => true, stabilitySleep: skipStabilityWait }); } finally { setCodexShimProbeHookForTests(null); } - expect(result.installed).toBe(false); - expect(result.message).toContain("upgrade deferred because tracked launchers changed"); + expect(result.status).toBe("deferred"); + expect("message" in result && result.message).toContain("upgrade deferred because tracked launchers changed"); expect(readFileSync(wrappers[0], "utf8")).toBe(concurrent); expect(readFileSync(backups[0])).toEqual(oldBackup); expect(readFileSync(statePath)).toEqual(oldState); From fdf4fb2a31e819e56462764448d9036785f41dd1 Mon Sep 17 00:00:00 2001 From: comfuture Date: Tue, 11 Aug 2026 13:35:42 +0900 Subject: [PATCH 14/16] fix(codex): bound child-process reentry depth --- src/codex/shim.ts | 20 +++++++++++++++++--- tests/codex-shim.test.ts | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/src/codex/shim.ts b/src/codex/shim.ts index 9fe3633e4..fd1f54893 100644 --- a/src/codex/shim.ts +++ b/src/codex/shim.ts @@ -577,11 +577,24 @@ if [ "\${OCX_SHIM_ACTIVE_PID:-}" = "$$" ]; then printf '%s\n' ${shQuote(CODEX_SHIM_REENTRY_DIAGNOSTIC)} >&2 exit ${CODEX_SHIM_REENTRY_EXIT_CODE} fi +case "\${OCX_SHIM_ACTIVE_DEPTH:-0}" in + 0) + OCX_SHIM_ACTIVE_DEPTH=1 + ;; + 1) + OCX_SHIM_ACTIVE_DEPTH=2 + ;; + *) + printf '%s\n' ${shQuote(CODEX_SHIM_REENTRY_DIAGNOSTIC)} >&2 + exit ${CODEX_SHIM_REENTRY_EXIT_CODE} + ;; +esac # Dynamic launchers such as mise exec -- codex may resolve the command name -# back to this wrapper. An exec chain keeps the same PID; a legitimate nested -# Codex invocation starts a new process and is allowed to establish a new guard. +# back to this wrapper. An exec chain keeps the same PID. A legitimate nested +# Codex invocation may enter once with a new PID; repeated child-process +# redispatch reaches depth 2 and is rejected before it can form an infinite chain. OCX_SHIM_ACTIVE_PID=$$ -export OCX_SHIM_ACTIVE_PID +export OCX_SHIM_ACTIVE_PID OCX_SHIM_ACTIVE_DEPTH if [ -z "$OPENCODEX_API_AUTH_TOKEN" ] && [ -f ${shQuote(tokenFile)} ]; then OPENCODEX_API_AUTH_TOKEN="$(cat ${shQuote(tokenFile)})" export OPENCODEX_API_AUTH_TOKEN @@ -683,6 +696,7 @@ function probeUnixShimInstall(wrapperPath: string): UnixShimProbeResult { OCX_SHIM_PROBE_REENTRY_PATH: reentryPath, }; delete env.OCX_SHIM_ACTIVE_PID; + delete env.OCX_SHIM_ACTIVE_DEPTH; delete env.OCX_SHIM_PROBE_ACTIVE; let groupId = 0; try { diff --git a/tests/codex-shim.test.ts b/tests/codex-shim.test.ts index ad9d09200..90f3fb447 100644 --- a/tests/codex-shim.test.ts +++ b/tests/codex-shim.test.ts @@ -348,6 +348,46 @@ exit 64 } }); + test("Unix runtime guard stops argument-dependent child-process redispatch", () => { + if (process.platform === "win32") return; + + const binDir = mkdtempSync(join(tmpdir(), "ocx-shim-runtime-child-reentry-bin-")); + const home = mkdtempSync(join(tmpdir(), "ocx-shim-runtime-child-reentry-home-")); + const oldPath = process.env.PATH; + const oldHome = process.env.OPENCODEX_HOME; + const codexPath = join(binDir, "codex"); + const original = `#!/bin/sh +if [ "$1" = "--version" ]; then + exit 0 +fi +codex "$@" +`; + try { + process.env.PATH = prependPath(binDir, oldPath); + process.env.OPENCODEX_HOME = home; + writeFileSync(codexPath, original, "utf8"); + chmodSync(codexPath, 0o755); + + expect(installCodexShim().installed).toBe(true); + + const result = spawnSync(codexPath, ["--help"], { + encoding: "utf8", + env: { ...process.env, OCX_SHIM_BYPASS: "1" }, + timeout: 3_000, + }); + expect(result.error).toBeUndefined(); + expect(result.status).toBe(126); + expect(result.stderr).toContain("saved Codex launcher resolved back to the autostart shim"); + } finally { + if (oldPath === undefined) delete process.env.PATH; + else process.env.PATH = oldPath; + if (oldHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = oldHome; + rmSync(binDir, { recursive: true, force: true }); + rmSync(home, { recursive: true, force: true }); + } + }); + test("Unix install drains an immediate recursive diagnostic before classifying the probe", () => { if (process.platform === "win32") return; From 34c851e278f126d206fb2d637d466f19871a735c Mon Sep 17 00:00:00 2001 From: comfuture Date: Tue, 11 Aug 2026 15:45:35 +0900 Subject: [PATCH 15/16] test(codex): allow shim probe observation window --- tests/codex-shim-autorestore.test.ts | 2 +- tests/codex-shim-readiness.test.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/codex-shim-autorestore.test.ts b/tests/codex-shim-autorestore.test.ts index 3b9b0b0ac..f6bd4b2b5 100644 --- a/tests/codex-shim-autorestore.test.ts +++ b/tests/codex-shim-autorestore.test.ts @@ -161,5 +161,5 @@ describe("Codex shim CLI auto-restore policy", () => { rmSync(binDir, { recursive: true, force: true }); rmSync(home, { recursive: true, force: true }); } - }); + }, 20_000); }); diff --git a/tests/codex-shim-readiness.test.ts b/tests/codex-shim-readiness.test.ts index d81e941ef..18f0b9e59 100644 --- a/tests/codex-shim-readiness.test.ts +++ b/tests/codex-shim-readiness.test.ts @@ -140,7 +140,7 @@ describe("Codex shim install readiness", () => { } finally { rmSync(root, { recursive: true, force: true }); } - }); + }, 10_000); test("keeps install advisory when the Codex config cannot be read", () => { const root = mkdtempSync(join(tmpdir(), "ocx-shim-unreadable-config-")); @@ -177,5 +177,5 @@ describe("Codex shim install readiness", () => { } finally { rmSync(root, { recursive: true, force: true }); } - }); + }, 10_000); }); From 90cac9b42d206aeca13377969f5c8eeb7c6c647d Mon Sep 17 00:00:00 2001 From: comfuture Date: Tue, 11 Aug 2026 18:19:53 +0900 Subject: [PATCH 16/16] docs(codex): record shim validation decision --- structure/01_runtime.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/structure/01_runtime.md b/structure/01_runtime.md index e0977229a..6d5527709 100644 --- a/structure/01_runtime.md +++ b/structure/01_runtime.md @@ -58,6 +58,14 @@ tracked sibling before mutation and rolls back earlier siblings in reverse order Failures warn without changing the requested command's exit behavior. The probe uses read-only config diagnostics only for a confirmed candidate and never reads adjacent auth state. +[Decision Log] +- 목적과 의도: Prevent a saved command-name launcher such as a mise trampoline from resolving back to the newly installed Codex shim and recursing indefinitely, while keeping native Codex usable after every failed install or repair. +- 기존 구현 및 제약 조건: Unix installation accepted any non-OpenCodex PATH launcher and saved it behind the shim. Shell scripts, native executables, symlinks, and dynamic launchers are all valid, and static launcher contents cannot prove what a command-name redispatch will resolve to after the shim is installed. +- 검토한 주요 대안: Reject launchers by content or known tool signatures; resolve one more executable path before installation; run an unbounded validation command; supervise every descendant across new sessions with platform-specific process enumeration. +- 선택한 방식: Validate the generated wrapper with a bounded behavioral `--version` probe. Run it in an isolated process group, use a probe-only re-entry sentinel plus an inherited lease descriptor as evidence of recursion or surviving descendants, terminate the owned process group on an unsafe result, and commit state only while the generated wrapper still matches its recorded fingerprint. Probe failures and owned-wrapper changes roll back in reverse order while preserving an observed external replacement. +- 다른 대안 대신 이 방식을 선택한 이유: Exercising the installed resolution path catches mise-style and other dynamic redispatch without parsing provider-specific launcher formats. A five-second observation window bounds installation latency and cleanup, while ownership fingerprints make rollback safe for the concurrent replacements the transaction can observe. +- 장점, 단점 및 영향: Direct executables and symlinks remain supported, deterministic shim re-entry is rejected, and failed validation restores the prior launcher instead of leaving a committed recursive shim. Installation can take up to five additional seconds. Containment is deliberately process-group scoped: a descendant that intentionally creates a new session can escape group termination, so this probe is not a general OS process supervisor; sentinel evidence still rejects an escaped descendant that re-enters the generated shim during the bounded window. + The bridge enforces a heartbeat stall deadline. It defaults to 300 seconds sampled on a 2 s tick (`src/stall-timeout.ts`) and is configurable, so treat the number as a default rather than an invariant; sidecars keep their own clocks. On expiry the stream is closed and the upstream request