diff --git a/src/lib/test-home-guard.ts b/src/lib/test-home-guard.ts index d218b248370..f0843fb59d7 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 samePathOnPlatform(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 (!samePathOnPlatform(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 (!samePathOnPlatform(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 (!samePathOnPlatform(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,16 @@ function foldPlatformCase(path: string): string { return process.platform === "darwin" || process.platform === "win32" ? path.toLowerCase() : path; } +/** + * Path equality under the guard's conservative platform policy: fold on darwin/win32 (as the + * Claude-tree checks already do), a plain string compare elsewhere. This is an OS-level + * policy, not a probe of actual filesystem identity — on a case-sensitive volume it can + * over-refuse, which is the safe direction for a guard. + */ +function samePathOnPlatform(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); @@ -325,7 +335,19 @@ export function protectedRemovalTreesForTests(): readonly string[] { /** Whether `child` sits strictly below `parent`, both already canonicalized. */ function isInside(parent: string, child: string): boolean { const rel = relative(parent, child); - return rel !== "" && !rel.startsWith("..") && !isAbsolute(rel); + // Only a leading ".." COMPONENT leaves the root: a child named "..backup" is inside it. + return rel !== "" && !isAbsolute(rel) && rel !== ".." && !rel.startsWith(`..${sep}`); +} + +/** + * The case-folding counterpart of {@link isInside}, used ONLY on the guard's deny side. + * Refusing too broadly is the guard's safe direction, so protected-tree comparisons fold + * platform case. The checkout-content exemption must never use this: a folded ALLOW would + * call a distinct same-letters-different-case sibling "checkout content" on a case-sensitive + * volume, turning a refusal into permission. + */ +function isInsideIgnoringPlatformCase(parent: string, child: string): boolean { + return isInside(foldPlatformCase(parent), foldPlatformCase(child)); } let checkoutRootSpellings: readonly string[] | undefined; @@ -387,16 +409,16 @@ 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 (samePathOnPlatform(candidate, PROTECTED_REAL_HOME) || samePathOnPlatform(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 (isInside(protectedPath, candidate) && !isOwnCheckoutContent(protectedPath, candidate)) { + if (samePathOnPlatform(candidate, protectedPath)) return `${tree.label} (${protectedPath})`; + if (isInsideIgnoringPlatformCase(protectedPath, candidate) && !isOwnCheckoutContent(protectedPath, candidate)) { return `a path inside ${tree.label} (${protectedPath})`; } - if (isInside(candidate, protectedPath)) return `an ancestor of ${tree.label} (${protectedPath})`; + if (isInsideIgnoringPlatformCase(candidate, protectedPath)) return `an ancestor of ${tree.label} (${protectedPath})`; } } // The Claude directory is judged like its writer guard: roots resolved now, case folded diff --git a/structure/ops/test-sandbox-cleanup.md b/structure/ops/test-sandbox-cleanup.md index ccc50a7057a..6c0639b9426 100644 --- a/structure/ops/test-sandbox-cleanup.md +++ b/structure/ops/test-sandbox-cleanup.md @@ -89,7 +89,12 @@ gateway-cache writes, served-catalog invalidation and intercept `settings.json` real Claude config directory, its default and the developer's override alike. Writers pass every directory they touch, so a sandbox child linked into the real directory is judged by its target; protected roots resolve at check time and compare case-insensitively on macOS and Windows. That -directory is also a protected removal tree. +directory is also a protected removal tree. Removal-tree containment and equality use the same +component-aware, platform-case-folded comparison: only a leading `..` path component leaves a +protected tree (a child named `..backup` stays inside), and case variants compare equal on +macOS and Windows. The running-checkout content exemption is the one comparison that does NOT +fold case: a folded ALLOW would call a distinct same-letters-different-case sibling checkout +content on a case-sensitive volume, turning a refusal into permission. `tests/claude-integration/claude-config-home-isolation.test.ts` pins each layer. The write guard arms without the preload too. Bun reads `bunfig.toml` from the current working diff --git a/tests/ci-workflows/test-home-guard.test.ts b/tests/ci-workflows/test-home-guard.test.ts index 2cbe3f79e44..8940577ba76 100644 --- a/tests/ci-workflows/test-home-guard.test.ts +++ b/tests/ci-workflows/test-home-guard.test.ts @@ -49,6 +49,23 @@ const REPO_ROOT = repoRoot(); */ const REPO_ROOT_URL = pathToFileURL(repoRoot() + "/").href; +/** + * Whether this host's temp volume treats case-flipped names as distinct. The case-distinct + * checkout-sibling regression needs two same-letters-different-case directories to coexist; + * on a case-insensitive volume the fold makes them one directory, the exemption legitimately + * applies, and the scenario cannot exist, so that one test skips there. The mock only + * substitutes process.platform — it never simulates the filesystem. + */ +const CASE_SENSITIVE_TMP = (() => { + const probe = mkdtempSync(join(tmpdir(), "ocx-case-probe-")); + try { + mkdirSync(join(probe, "checkout")); + return !existsSync(join(probe, "CHECKOUT")); + } finally { + removeTreeWithRetry(probe); + } +})(); + // Scale only execution: cleanup retains room below CI's existing 60-second ceiling. const PROBE_EXECUTION_MS = watchdogMs(5_000); const PROBE_TERM_MS = 5_000; @@ -749,6 +766,102 @@ const canSymlink = (() => { } }); + test("a protected-tree child whose name begins with two dots is still inside the tree (#7853)", () => { + // `relative(home, join(home, "..backup"))` is "..backup": a string-prefix `..` check read + // that as an escape and judged the child OUTSIDE the tree, so the removal guard did not + // refuse it. Only a leading ".." path component leaves the root. + const home = protectedHomeForTests(); + expect(protectedRemovalReason(join(home, "..backup"))).not.toBeNull(); + expect(protectedRemovalReason(join(home, "..backup", "config.json"))).not.toBeNull(); + // A real escape still resolves outside and stays unrefused. + expect(protectedRemovalReason(join(home, "sub", "..", "..", "ocx-guard-outside"))).toBeNull(); + }); + + test("case variants of protected trees and homes are refused where the filesystem folds case (#7853)", async () => { + // The fold branch only runs on darwin/win32, so the probe mocks process.platform before the + // guard module loads (module state is captured at import). That exercises the comparison + // logic on any host, but it is NOT a real macOS or Windows filesystem run. + const probeId = beginProbe("16-case-fold-refusal"); + const realHome = mkdtempSync(join(tmpdir(), "ocx-sentinel-casehome-")); + try { + mkdirSync(join(realHome, ".opencodex"), { recursive: true }); + mkdirSync(join(realHome, ".codex"), { recursive: true }); + mkdirSync(join(realHome, "Library", "LaunchAgents"), { recursive: true }); + const probe = await runProbe(probeId, ` + Object.defineProperty(process, "platform", { value: "darwin", configurable: true }); + const { join, dirname, basename } = await import("node:path"); + const guard = await import(${JSON.stringify(REPO_ROOT_URL + "src/lib/test-home-guard")}); + const home = guard.protectedHomeForTests(); + const variant = join(dirname(home), basename(home).toUpperCase()); + const codexHome = guard.protectedCodexHomeForTests(); + const codexVariant = join(dirname(codexHome), basename(codexHome).toUpperCase()); + const launchAgents = guard.protectedLaunchAgentsDirForTests(); + const launchVariant = join(dirname(launchAgents), basename(launchAgents).toUpperCase()); + const throws = fn => { try { fn(); return false; } catch { return true; } }; + console.log(JSON.stringify({ + variantDiffers: variant !== home && codexVariant !== codexHome && launchVariant !== launchAgents, + removalVariant: guard.protectedRemovalReason(variant) !== null, + removalVariantChild: guard.protectedRemovalReason(join(variant, "config.json")) !== null, + writeHome: throws(() => guard.assertNotRealHomeUnderTest(variant)), + isProtectedHome: guard.isProtectedHomeUnderTest(variant), + writeCodex: throws(() => guard.assertNotRealCodexHomeUnderTest(codexVariant)), + writeLaunch: throws(() => guard.assertNotRealLaunchAgentsUnderTest(launchVariant)), + ordinary: guard.protectedRemovalReason(join(${JSON.stringify(realHome)}, "ordinary-dir")) === null, + })); + `, { OCX_REAL_HOME: realHome, OCX_TEST_HOME_GUARD: "1" }); + + expect(JSON.parse(probe.stdout.trim())).toEqual({ + variantDiffers: true, + removalVariant: true, + removalVariantChild: true, + writeHome: true, + isProtectedHome: true, + writeCodex: true, + writeLaunch: true, + ordinary: true, + }); + } finally { + removeTreeWithRetry(realHome); + } + }); + + test.skipIf(!CASE_SENSITIVE_TMP)("a case-distinct sibling of the running checkout inside a protected tree stays refused (#7853)", async () => { + // Regression: the deny side folds platform case, but the own-checkout ALLOW exemption must + // stay exact. A folded exemption would call `.opencodex/CHECKOUT` (a distinct directory on a + // case-sensitive volume) content of the running checkout at `.opencodex/checkout` and permit + // its removal. The probe imports a COPY of the guard from inside the synthetic home so the + // running checkout genuinely sits inside the protected tree; a symlink into the tree would + // give the candidate separate canonical and lexical spellings and mask the exemption path. + const probeId = beginProbe("17-case-sibling-not-checkout"); + const realHome = mkdtempSync(join(tmpdir(), "ocx-sentinel-casehome-")); + try { + const moduleDir = join(realHome, ".opencodex", "checkout", "src", "lib"); + mkdirSync(moduleDir, { recursive: true }); + mkdirSync(join(realHome, ".opencodex", "CHECKOUT"), { recursive: true }); + const modulePath = join(moduleDir, "test-home-guard.ts"); + writeFileSync(modulePath, readFileSync(repoPath("src/lib/test-home-guard.ts"), "utf8"), "utf8"); + const probe = await runProbe(probeId, ` + Object.defineProperty(process, "platform", { value: "darwin", configurable: true }); + const { join, dirname } = await import("node:path"); + const { fileURLToPath } = await import("node:url"); + const moduleUrl = ${JSON.stringify("__MODULE_URL_PLACEHOLDER__")}; + const guard = await import(moduleUrl); + const checkout = dirname(dirname(dirname(fileURLToPath(moduleUrl)))); + console.log(JSON.stringify({ + siblingRefused: guard.protectedRemovalReason(join(checkout, "..", "CHECKOUT", "private-state")) !== null, + ownContentAllowed: guard.protectedRemovalReason(join(checkout, "src", "private-file.ts")) === null, + })); + `.replace("__MODULE_URL_PLACEHOLDER__", pathToFileURL(modulePath).href), { OCX_REAL_HOME: realHome, OCX_TEST_HOME_GUARD: "1" }); + + expect(JSON.parse(probe.stdout.trim())).toEqual({ + siblingRefused: true, + ownContentAllowed: true, + }); + } finally { + removeTreeWithRetry(realHome); + } + }); + test.skipIf(!canSymlink)("a symlink pointing at a protected tree is refused through its target", async () => { const probeId = beginProbe("13-symlink-removal"); const { realHome, opencodexHome } = sentinelHome();