From 794e8b71b327c64ff7e24a843d0ff82cf7d3b655 Mon Sep 17 00:00:00 2001 From: JUN <243035832+lidge-jun@users.noreply.github.com> Date: Wed, 28 Oct 2026 15:46:27 +0900 Subject: [PATCH 1/6] fix(test): judge protected-tree containment per path component (#7853) isInside() treated any relative path starting with ".." as outside the parent, so a direct child such as ~/.opencodex/..backup passed the removal guard. Equality checks for the protected homes were also case-sensitive on case-insensitive filesystems. Both now use the component-aware, case-folding comparison the Claude tree checks already used. Closes #7853 --- src/lib/test-home-guard.ts | 32 ++++++++++++++++------ tests/ci-workflows/test-home-guard.test.ts | 13 +++++++++ 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/src/lib/test-home-guard.ts b/src/lib/test-home-guard.ts index d218b248370..1f08bdbe8f4 100644 --- a/src/lib/test-home-guard.ts +++ b/src/lib/test-home-guard.ts @@ -191,7 +191,7 @@ function missingPreloadHint(): string { */ export function isProtectedHomeUnderTest(dir: string): boolean { if (!isTestHomeGuardArmed()) return false; - return canonicalize(dir) === PROTECTED_HOME; + return samePathIgnoringPlatformCase(canonicalize(dir), PROTECTED_HOME); } /** @@ -204,7 +204,7 @@ export function isProtectedHomeUnderTest(dir: string): boolean { */ export function assertNotRealHomeUnderTest(dir: string): void { if (!isTestHomeGuardArmed()) return; - if (canonicalize(dir) !== PROTECTED_HOME) return; + if (!samePathIgnoringPlatformCase(canonicalize(dir), PROTECTED_HOME)) return; throw new Error( `refusing to write the real OpenCodex home (${PROTECTED_HOME}) from a test process. ` + "Point OPENCODEX_HOME at a temp directory for this test, or inject persistence " @@ -227,7 +227,7 @@ export function protectedLaunchAgentsDirForTests(): string { */ export function assertNotRealLaunchAgentsUnderTest(dir: string): void { if (!isTestHomeGuardArmed()) return; - if (canonicalize(dir) !== PROTECTED_LAUNCH_AGENTS) return; + if (!samePathIgnoringPlatformCase(canonicalize(dir), PROTECTED_LAUNCH_AGENTS)) return; throw new Error( `refusing to write the real LaunchAgents directory (${PROTECTED_LAUNCH_AGENTS}) from a test ` + "process: os.homedir() ignores HOME, so rewriting HOME does not move this path. Pass an " @@ -238,7 +238,7 @@ export function assertNotRealLaunchAgentsUnderTest(dir: string): void { /** Throw when an armed test process is about to write the real native Codex home. */ export function assertNotRealCodexHomeUnderTest(dir: string): void { if (!isTestHomeGuardArmed()) return; - if (canonicalize(dir) !== PROTECTED_CODEX_HOME) return; + if (!samePathIgnoringPlatformCase(canonicalize(dir), PROTECTED_CODEX_HOME)) return; throw new Error( `refusing to write the real Codex home (${PROTECTED_CODEX_HOME}) from a test process. ` + "Point CODEX_HOME at a temp directory for this test before writing Codex files " @@ -285,6 +285,15 @@ function foldPlatformCase(path: string): string { return process.platform === "darwin" || process.platform === "win32" ? path.toLowerCase() : path; } +/** + * Path equality as the default filesystem on this platform sees it. A not-yet-existing + * `.OPENCODEX` canonicalizes through its parent and keeps its spelling, so a plain `===` + * against the canonical home calls it a different directory on Windows and macOS (#7853). + */ +function samePathIgnoringPlatformCase(a: string, b: string): boolean { + return foldPlatformCase(a) === foldPlatformCase(b); +} + function isSameOrInsideIgnoringPlatformCase(root: string, candidate: string): boolean { const parent = foldPlatformCase(root); const child = foldPlatformCase(candidate); @@ -322,10 +331,15 @@ export function protectedRemovalTreesForTests(): readonly string[] { return [PROTECTED_REAL_HOME, ...PROTECTED_TREES.map(tree => tree.path)]; } -/** Whether `child` sits strictly below `parent`, both already canonicalized. */ +/** + * Whether `child` sits strictly below `parent`, both already canonicalized. + * + * Containment is decided per path component, with case folded where the filesystem folds it. + * A prefix test on the relative path called a direct child named `..backup` "outside", so the + * removal guard let it through (#7853). + */ function isInside(parent: string, child: string): boolean { - const rel = relative(parent, child); - return rel !== "" && !rel.startsWith("..") && !isAbsolute(rel); + return !samePathIgnoringPlatformCase(parent, child) && isSameOrInsideIgnoringPlatformCase(parent, child); } let checkoutRootSpellings: readonly string[] | undefined; @@ -387,12 +401,12 @@ export function protectedRemovalReason(target: string): string | null { // form still names it. Upstream Codex makes the same distinction in its writable-root // handling, keeping logical and resolved forms side by side rather than collapsing to one. for (const candidate of [canonicalize(target), resolve(target)]) { - if (candidate === PROTECTED_REAL_HOME || candidate === LEXICAL_REAL_HOME) { + if (samePathIgnoringPlatformCase(candidate, PROTECTED_REAL_HOME) || samePathIgnoringPlatformCase(candidate, LEXICAL_REAL_HOME)) { return `the real home directory (${PROTECTED_REAL_HOME})`; } for (const tree of PROTECTED_TREES) { for (const protectedPath of [tree.path, tree.lexical]) { - if (candidate === protectedPath) return `${tree.label} (${protectedPath})`; + if (samePathIgnoringPlatformCase(candidate, protectedPath)) return `${tree.label} (${protectedPath})`; if (isInside(protectedPath, candidate) && !isOwnCheckoutContent(protectedPath, candidate)) { return `a path inside ${tree.label} (${protectedPath})`; } diff --git a/tests/ci-workflows/test-home-guard.test.ts b/tests/ci-workflows/test-home-guard.test.ts index 2cbe3f79e44..f9547bf2064 100644 --- a/tests/ci-workflows/test-home-guard.test.ts +++ b/tests/ci-workflows/test-home-guard.test.ts @@ -740,6 +740,19 @@ const canSymlink = (() => { // And the refusal is not armed-gated: the run that caused the incident armed nothing. expect(() => assertRemovalOutsideProtectedTrees(protectedHomeForTests())).toThrow("refusing to remove"); + // #7853: a child whose NAME starts with two dots is still inside the tree. Only a leading + // ".." component leaves it. The real home (first entry) is refused by equality only. + for (const tree of trees.slice(1)) { + expect(protectedRemovalReason(join(tree, "..backup"))).not.toBeNull(); + expect(protectedRemovalReason(join(tree, "..backup", "config.json"))).not.toBeNull(); + } + // And where the filesystem ignores case, a case variant names the same tree, existing or not. + if (process.platform === "win32" || process.platform === "darwin") { + const upper = join(dirname(protectedHomeForTests()), ".OPENCODEX"); + expect(protectedRemovalReason(upper)).not.toBeNull(); + expect(protectedRemovalReason(join(upper, "config.json"))).not.toBeNull(); + } + const ordinary = mkdtempSync(join(tmpdir(), "ocx-removal-allowed-")); try { expect(protectedRemovalReason(ordinary)).toBeNull(); From e3b0581c7c10ea29d9f2688b868ee3b785274693 Mon Sep 17 00:00:00 2001 From: JUN <243035832+lidge-jun@users.noreply.github.com> Date: Wed, 28 Oct 2026 18:28:04 +0900 Subject: [PATCH 2/6] fix(test): keep the checkout removal exemption case-exact (#7853) Review of the component-aware containment change found that case folding also reached isOwnCheckoutContent(). On macOS the guard folds case, so on a case-sensitive APFS volume a running checkout at ~/.codex/worktrees/repo exempted a distinct sibling ~/.codex/worktrees/REPO from the removal guard. Protection decisions keep folding, since comparing broadly only refuses more. The exemption, which lifts a refusal, now compares case exactly and per path component. The new probe reports darwin before loading the guard on a POSIX host, which models that volume; it fails on the previous head (caseSibling exempted) and passes here. Windows skips it because path.relative ignores case there regardless of the guard. --- src/lib/test-home-guard.ts | 16 ++++++++++- tests/ci-workflows/test-home-guard.test.ts | 31 +++++++++++++++++++++- 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/src/lib/test-home-guard.ts b/src/lib/test-home-guard.ts index 1f08bdbe8f4..d3c25a01558 100644 --- a/src/lib/test-home-guard.ts +++ b/src/lib/test-home-guard.ts @@ -342,6 +342,20 @@ function isInside(parent: string, child: string): boolean { return !samePathIgnoringPlatformCase(parent, child) && isSameOrInsideIgnoringPlatformCase(parent, child); } +/** + * Whether `child` sits strictly below `parent` with case PRESERVED, deciding per path component. + * + * Only for the checkout exemption below, which LIFTS a refusal. Folding case there would let a + * running checkout at `~/.codex/worktrees/repo` exempt a distinct `~/.codex/worktrees/REPO` on a + * case-sensitive macOS volume. Protection decisions fold, because comparing too broadly only refuses + * more; an exemption must not, because the same breadth removes more. + */ +function isInsidePreservingCase(parent: string, child: string): boolean { + if (parent === child) return false; + const rel = relative(parent, child); + return rel !== "" && !isAbsolute(rel) && rel !== ".." && !rel.startsWith(`..${sep}`); +} + let checkoutRootSpellings: readonly string[] | undefined; /** @@ -376,7 +390,7 @@ function checkoutRoots(): readonly string[] { * tree (one at `$HOME`, or the virtual root of a compiled build) never qualifies. */ function isOwnCheckoutContent(protectedPath: string, candidate: string): boolean { - return checkoutRoots().some(root => isInside(protectedPath, root) && isInside(root, candidate)); + return checkoutRoots().some(root => isInsidePreservingCase(protectedPath, root) && isInsidePreservingCase(root, candidate)); } /** diff --git a/tests/ci-workflows/test-home-guard.test.ts b/tests/ci-workflows/test-home-guard.test.ts index f9547bf2064..c752589a8ed 100644 --- a/tests/ci-workflows/test-home-guard.test.ts +++ b/tests/ci-workflows/test-home-guard.test.ts @@ -12,7 +12,7 @@ import { describe, expect, spyOn, test } from "bun:test"; import { existsSync, mkdtempSync, mkdirSync, readFileSync, statSync, symlinkSync, unlinkSync, writeFileSync } from "node:fs"; import { homedir, tmpdir } from "node:os"; -import { dirname, join } from "node:path"; +import { basename, dirname, join } from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; import { assertNotRealHomeUnderTest, @@ -823,6 +823,35 @@ const canSymlink = (() => { } }); + // Windows' path.relative ignores case whatever the guard decides, so this case cannot arise there. + test.skipIf(!canSymlink || process.platform === "win32")("on a case-folding platform the checkout exemption still compares case exactly", async () => { + // The guard folds case on macOS so protection refuses broadly, but a case-sensitive APFS volume + // can hold ~/.codex/worktrees/repo and a distinct ~/.codex/worktrees/REPO side by side. The + // child reports darwin before loading the guard, so the guard folds while path comparison stays + // exact, which is that volume. Folding the exemption too exempted the sibling (#7853 review). + const probeId = beginProbe("16-checkout-case-sibling"); + const realHome = mkdtempSync(join(tmpdir(), "ocx-sentinel-home-")); + const codexLink = join(realHome, ".codex"); + symlinkSync(dirname(REPO_ROOT), codexLink); + mkdirSync(join(realHome, ".opencodex"), { recursive: true }); + const caseSibling = join(dirname(REPO_ROOT), basename(REPO_ROOT).toUpperCase()); + try { + const probe = await runProbe(probeId, ` + Object.defineProperty(process, "platform", { value: "darwin" }); + const { protectedRemovalReason } = await import("${REPO_ROOT_URL}src/lib/test-home-guard"); + console.log(JSON.stringify({ + distinct: ${JSON.stringify(caseSibling)} !== ${JSON.stringify(REPO_ROOT)}, + fixture: protectedRemovalReason(${JSON.stringify(join(REPO_ROOT, "tests", ".tmp-guard-fixture"))}) === null, + caseSibling: protectedRemovalReason(${JSON.stringify(join(caseSibling, "config.json"))}) !== null, + })); + `, { OCX_REAL_HOME: realHome, OCX_TEST_HOME_GUARD: "1" }); + + expect(JSON.parse(probe.stdout.trim())).toEqual({ distinct: true, fixture: true, caseSibling: true }); + } finally { + unlinkSync(codexLink); + } + }); + test("a checkout that contains a protected tree gains no exemption", async () => { // The lift requires the checkout to sit INSIDE the tree. A checkout at the home directory, or // the virtual root a compiled build reports, contains ~/.codex instead and must stay guarded. From c969dd3a1d0db09d4ed8a9619565378b68487389 Mon Sep 17 00:00:00 2001 From: JUN <243035832+lidge-jun@users.noreply.github.com> Date: Wed, 28 Oct 2026 18:55:20 +0900 Subject: [PATCH 3/6] test(service): report compiler stderr when the native recovery fixture fails to build tests/service/service-desktop-recovery.test.ts compiles a std-only Rust program and a Bun executable before exercising native Desktop recovery. On #7884's full-selection `test changed` runs, rustc exited 1 in about 52ms with its stderr discarded, so the failure could not be told apart from a toolchain that never started. The assertions now carry the last 2,000 bytes of each compiler's stderr. Exit code 0 is still required. --- tests/service/service-desktop-recovery.test.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/service/service-desktop-recovery.test.ts b/tests/service/service-desktop-recovery.test.ts index bd137d8bc36..3efb33dc3eb 100644 --- a/tests/service/service-desktop-recovery.test.ts +++ b/tests/service/service-desktop-recovery.test.ts @@ -330,9 +330,13 @@ import { runDesktopColdStart, productionDesktopColdStartDeps } from ${JSON.strin const evidence = await inspectDesktopRecovery(); const result = await runDesktopColdStart({ ...productionDesktopColdStartDeps(async () => ({ fixture: true })), desktopRecovery: async () => evidence, info: () => {} }); console.log(JSON.stringify({ evidence, result }));`); const rustBuild = Bun.spawnSync([Bun.which("rustc")!, "--edition=2021", nativeSource, "-o", app], { stdout: "pipe", stderr: "pipe", timeout: 30_000 }); - expect(rustBuild.exitCode).toBe(0); + // The compiler's own words travel with a failure: a bare "Received: 1" in 52ms left nothing to + // tell a compile error from a toolchain that never started. + expect({ exitCode: rustBuild.exitCode, stderr: rustBuild.stderr.toString().slice(-2_000) }) + .toEqual({ exitCode: 0, stderr: expect.any(String) }); const build = Bun.spawnSync([process.execPath, "build", "--compile", entry, "--outfile", cli], { stdout: "pipe", stderr: "pipe", timeout: 30_000 }); - expect(build.exitCode).toBe(0); + expect({ exitCode: build.exitCode, stderr: build.stderr.toString().slice(-2_000) }) + .toEqual({ exitCode: 0, stderr: expect.any(String) }); const env = { ...process.env, XDG_CONFIG_HOME: join(root, "xdg"), OPENCODEX_HOME: config }; let child: ReturnType | undefined; const foreignSockets: Socket[] = []; From 0280f57550d8adae909419c1265d9db3a1f88ec5 Mon Sep 17 00:00:00 2001 From: JUN <243035832+lidge-jun@users.noreply.github.com> Date: Wed, 28 Oct 2026 19:13:58 +0900 Subject: [PATCH 4/6] test(service): resolve the rustup toolchain from the real home for the native recovery fixture With compiler stderr now reported, #7884's full-selection `test changed` run showed the cause of the 52-86 ms rustc failures: error: rustup could not choose a version of rustc to run, because one wasn't specified explicitly, and no default is configured. rustc on the runner is a rustup proxy. With RUSTUP_HOME and CARGO_HOME unset it resolves its toolchain under $HOME, and the test preload points HOME at a sandbox. The fixture's rustc spawn now sets both to the real home's .rustup and .cargo (OCX_REAL_HOME, captured by the harness before HOME is rewritten, falling back to the passwd home) unless they are already set. The compile and every assertion are unchanged. --- tests/service/service-desktop-recovery.test.ts | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/tests/service/service-desktop-recovery.test.ts b/tests/service/service-desktop-recovery.test.ts index 3efb33dc3eb..1fc1b6417a0 100644 --- a/tests/service/service-desktop-recovery.test.ts +++ b/tests/service/service-desktop-recovery.test.ts @@ -2,7 +2,7 @@ import { describe, expect, test } from "bun:test"; import { ptr, toArrayBuffer } from "bun:ffi"; import { chmodSync, copyFileSync, mkdirSync, mkdtempSync, realpathSync, rmSync, symlinkSync, unlinkSync, writeFileSync } from "node:fs"; import { connect, createServer, type Socket } from "node:net"; -import { tmpdir } from "node:os"; +import { tmpdir, userInfo } from "node:os"; import { join } from "node:path"; import { inspectDesktopRecovery, parseRecoveryAnswer, recoveryHomeIdentity, type DesktopRecoveryIo } from "../../src/service/desktop-recovery"; @@ -311,6 +311,17 @@ describe("native Desktop recovery evidence", () => { // The Rust unit tests run in the native CI job. This Linux integration additionally executes // the same producer and an actually compiled Bun consumer, without injecting any process proof. +// +// rustc is usually a rustup proxy, which finds its toolchain through $HOME when RUSTUP_HOME and +// CARGO_HOME are unset. The test preload points HOME at a sandbox, where the proxy finds no default +// toolchain and exits 1 ("rustup could not choose a version of rustc to run"). Resolve both the way +// rustup would under the real home, which the harness captures as OCX_REAL_HOME before rewriting HOME. +const realHome = process.env.OCX_REAL_HOME?.trim() || userInfo().homedir; +const rustEnv = { + ...process.env, + RUSTUP_HOME: process.env.RUSTUP_HOME || join(realHome, ".rustup"), + CARGO_HOME: process.env.CARGO_HOME || join(realHome, ".cargo"), +}; test.skipIf(process.platform !== "linux" || !Bun.which("rustc"))("compiled bundled CLI verifies real native recovery and ignores a protocol-speaking renamed outsider", async () => { const root = mkdtempSync(join(tmpdir(), "ocx-recovery-native-")); const appConfig = join(root, "xdg", "com.opencodex.desktop"), config = join(root, "config"); @@ -329,7 +340,7 @@ std::thread::park(); }`; import { runDesktopColdStart, productionDesktopColdStartDeps } from ${JSON.stringify(repoPath("src", "cli", "ensure-desktop-cold-start.ts"))}; const evidence = await inspectDesktopRecovery(); const result = await runDesktopColdStart({ ...productionDesktopColdStartDeps(async () => ({ fixture: true })), desktopRecovery: async () => evidence, info: () => {} }); console.log(JSON.stringify({ evidence, result }));`); - const rustBuild = Bun.spawnSync([Bun.which("rustc")!, "--edition=2021", nativeSource, "-o", app], { stdout: "pipe", stderr: "pipe", timeout: 30_000 }); + const rustBuild = Bun.spawnSync([Bun.which("rustc")!, "--edition=2021", nativeSource, "-o", app], { env: rustEnv, stdout: "pipe", stderr: "pipe", timeout: 30_000 }); // The compiler's own words travel with a failure: a bare "Received: 1" in 52ms left nothing to // tell a compile error from a toolchain that never started. expect({ exitCode: rustBuild.exitCode, stderr: rustBuild.stderr.toString().slice(-2_000) }) From e0d6fccef842eefa6d03ed04b893914aa1f98944 Mon Sep 17 00:00:00 2001 From: JUN <243035832+lidge-jun@users.noreply.github.com> Date: Wed, 28 Oct 2026 19:47:59 +0900 Subject: [PATCH 5/6] test(guard): flip every letter to build the case-colliding checkout sibling basename(REPO_ROOT).toUpperCase() returns the same name for a checkout that is already uppercase, so the probe reported distinct: false and failed even with a correct guard. Flip the case of every letter instead, and skip only when the name has no cased letter, where a case-colliding sibling cannot exist. Reported by CodeRabbit on #7884. --- tests/ci-workflows/test-home-guard.test.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/ci-workflows/test-home-guard.test.ts b/tests/ci-workflows/test-home-guard.test.ts index c752589a8ed..8b7e08f543c 100644 --- a/tests/ci-workflows/test-home-guard.test.ts +++ b/tests/ci-workflows/test-home-guard.test.ts @@ -824,7 +824,13 @@ const canSymlink = (() => { }); // Windows' path.relative ignores case whatever the guard decides, so this case cannot arise there. - test.skipIf(!canSymlink || process.platform === "win32")("on a case-folding platform the checkout exemption still compares case exactly", async () => { + // Every letter of the checkout name is flipped, so the sibling differs whichever case the name + // uses; a name with no cased letter has no case-colliding sibling at all. + const checkoutName = basename(REPO_ROOT); + const caseFlippedName = [...checkoutName] + .map(ch => (ch === ch.toUpperCase() ? ch.toLowerCase() : ch.toUpperCase())) + .join(""); + test.skipIf(!canSymlink || process.platform === "win32" || caseFlippedName === checkoutName)("on a case-folding platform the checkout exemption still compares case exactly", async () => { // The guard folds case on macOS so protection refuses broadly, but a case-sensitive APFS volume // can hold ~/.codex/worktrees/repo and a distinct ~/.codex/worktrees/REPO side by side. The // child reports darwin before loading the guard, so the guard folds while path comparison stays @@ -834,7 +840,7 @@ const canSymlink = (() => { const codexLink = join(realHome, ".codex"); symlinkSync(dirname(REPO_ROOT), codexLink); mkdirSync(join(realHome, ".opencodex"), { recursive: true }); - const caseSibling = join(dirname(REPO_ROOT), basename(REPO_ROOT).toUpperCase()); + const caseSibling = join(dirname(REPO_ROOT), caseFlippedName); try { const probe = await runProbe(probeId, ` Object.defineProperty(process, "platform", { value: "darwin" }); From 36c75708ab7bab11e734758a5b100b38c1a1562b Mon Sep 17 00:00:00 2001 From: JUN <243035832+lidge-jun@users.noreply.github.com> Date: Thu, 29 Oct 2026 13:29:53 +0900 Subject: [PATCH 6/6] chore: credit the parallel fix from #7879 #7879 by @lee3Q reached the same component-aware protected-tree check independently; this PR lands the maintainer version and closes it as a duplicate. Co-authored-by: Lee Sang Gyu