diff --git a/bunfig.toml b/bunfig.toml index 318845b44a..1a6ced2c5d 100644 --- a/bunfig.toml +++ b/bunfig.toml @@ -9,8 +9,15 @@ # The npm script already uses `bun test ./tests/`; this makes a bare `bun test` behave the same. [test] root = "tests" -# Sandboxes HOME/OPENCODEX_HOME/CODEX_HOME and arms the real-home write guard for EVERY -# invocation, including a bare `bun test ` that skips `scripts/test.ts`. A test -# once overwrote a real user config through that unwrapped path; see -# devlog/_fin/260730_codex_rs_upstream_v2_live_handoff/070. +# Sandboxes HOME/OPENCODEX_HOME/CODEX_HOME and arms the real-home write guard for every +# invocation THAT READS THIS FILE, including a bare `bun test ` that skips +# `scripts/test.ts`. A test once overwrote a real user config through that unwrapped path; +# see devlog/_fin/260730_codex_rs_upstream_v2_live_handoff/070. +# +# That qualifier is load-bearing. Bun resolves bunfig.toml from the CURRENT WORKING +# DIRECTORY, so `cd /tmp && bun test /tests/x.test.ts` loads no preload at all: nothing +# is sandboxed, the guard stays disarmed, and every `getConfigDir()` in that process returns +# the developer's real `~/.opencodex`. On 2026-09-15 such a run deleted one. A test that needs +# a config directory pins its own OPENCODEX_HOME instead of trusting this line, and +# `tests/ci-workflows/test-home-guard.test.ts` enforces that for the destructive case. preload = ["./tests/preload.ts"] diff --git a/tests/ci-workflows/test-home-guard.test.ts b/tests/ci-workflows/test-home-guard.test.ts index 47369ed663..acc8661c54 100644 --- a/tests/ci-workflows/test-home-guard.test.ts +++ b/tests/ci-workflows/test-home-guard.test.ts @@ -535,4 +535,56 @@ const canSymlink = (() => { expect(JSON.parse(probe.stdout.trim())).toEqual({ armed: true, rejected: true }); }); + /* + * The guard covers WRITERS, so a test that removes the config directory outright never + * reaches it: rmSync is plain node:fs, not a guarded writer. And the sandbox that would + * otherwise make the removal harmless is not universal — Bun resolves bunfig.toml, and with + * it the preload, from the CURRENT WORKING DIRECTORY. A run started outside the repository + * arms nothing, leaves OPENCODEX_HOME unset, and getConfigDir() then returns the developer's + * real ~/.opencodex. On 2026-09-15 a test did exactly that and deleted a live home: every + * OAuth login, the Codex account store, the service tokens and a 372MB usage ledger. + * + * Nothing runtime can be asserted here — the directory is gone before any guarded call runs + * — so the invariant is asserted on the test sources. A test that needs a config directory + * pins its own OPENCODEX_HOME and names that directory; none may hand the process-global one + * to a destructive fs call. + */ + test("no test file hands the process-global config directory to a destructive fs call", async () => { + const DESTRUCTIVE = "rmSync|rmdirSync|unlinkSync|renameSync|cpSync"; + const direct = new RegExp("\\b(?:" + DESTRUCTIVE + ")\\(\\s*getConfigDir\\(\\)"); + const bound = new RegExp("\\bconst\\s+([A-Za-z_$][\\w$]*)\\s*=\\s*getConfigDir\\(\\)"); + + // A line that merely NAMES the pattern is not a call: tests/cli/uninstall.test.ts asserts + // the CLI does not contain it, and the oracle at the end of this test is a literal. Both + // carry a quote on the line; a destructive call on a directory variable does not. + const isCode = (line: string): boolean => { + const trimmed = line.trim(); + if (trimmed.startsWith("//") || trimmed.startsWith("*") || trimmed.startsWith("/*")) return false; + return !trimmed.includes('"') && !trimmed.includes("'"); + }; + + const offenders = new Set(); + const testsDir = join(repoRoot(), "tests"); + for await (const relative of new Bun.Glob("**/*.test.ts").scan({ cwd: testsDir })) { + const lines = (await Bun.file(join(testsDir, relative)).text()).split("\n"); + const names = new Set(); + for (const line of lines) { + const found = bound.exec(line); + if (found) names.add(found[1]); + } + for (const line of lines) { + if (!isCode(line)) continue; + if (direct.test(line)) offenders.add(relative + ": getConfigDir() passed directly"); + for (const name of names) { + const viaName = new RegExp("\\b(?:" + DESTRUCTIVE + ")\\(\\s*" + name + "\\b"); + if (viaName.test(line)) offenders.add(relative + ": config dir removed via " + name); + } + } + } + + // The matcher must be able to see the shape it looks for, so an empty result is evidence + // rather than a silently broken regex. + expect(direct.test("rmSync(getConfigDir(), { recursive: true })")).toBe(true); + expect([...offenders].sort()).toEqual([]); + }); }); diff --git a/tests/preload.ts b/tests/preload.ts index a848a4aaf0..b8b0a83245 100644 --- a/tests/preload.ts +++ b/tests/preload.ts @@ -4,8 +4,16 @@ * `bun run test` already sandboxes HOME/OPENCODEX_HOME/CODEX_HOME through * `scripts/test.ts`. The incident this file prevents happened under a bare * `bun test ` — the command anyone reaches for while iterating on one test — - * which gets no wrapper and therefore had no isolation at all. A preload runs for - * EVERY invocation, so the protection no longer depends on remembering the wrapper. + * which gets no wrapper and therefore had no isolation at all. A preload runs for every + * invocation that READS bunfig.toml, so the protection no longer depends on remembering + * the wrapper. + * + * It does still depend on WHERE the run starts. Bun resolves bunfig.toml from the current + * working directory, so a run launched outside the repository never loads this file: no + * sandbox, no arming, and getConfigDir() resolves the real ~/.opencodex. On 2026-09-15 a + * run of that shape deleted a live home. Nothing here can close that hole from inside, so + * a test that needs a config directory pins its own OPENCODEX_HOME rather than inheriting + * one, and tests/ci-workflows/test-home-guard.test.ts enforces it for destructive calls. * (devlog `_plan/260730_codex_rs_upstream_v2_live_handoff/070`.) * * Import order below is load-bearing: importing the guard captures the real home at diff --git a/tests/usage/quota-reset-seen-store.test.ts b/tests/usage/quota-reset-seen-store.test.ts index 6689c26326..7594ed13b1 100644 --- a/tests/usage/quota-reset-seen-store.test.ts +++ b/tests/usage/quota-reset-seen-store.test.ts @@ -1,5 +1,6 @@ -import { beforeEach, describe, expect, test } from "bun:test"; -import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { afterAll, beforeEach, describe, expect, test } from "bun:test"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; import { join } from "node:path"; import { getConfigDir } from "../../src/config"; import type { QuotaResetEvent } from "../../src/quota/reset-detector"; @@ -15,6 +16,36 @@ import { swapLastObservedWindows, } from "../../src/quota/reset-seen-store"; +/** + * This file owns its config directory instead of inheriting one. + * + * Every case here resolves the process-global config home, and one of them DELETES it to + * force a write failure. That is bounded only while OPENCODEX_HOME points at a sandbox, and + * the preload that normally guarantees it does not cover every way this file can be run: Bun + * resolves `bunfig.toml` — and therefore its `preload = ["./tests/preload.ts"]` — from the + * CURRENT WORKING DIRECTORY. A run started outside the repository loads no preload, leaves + * OPENCODEX_HOME unset and the guard disarmed, and `getConfigDir()` then resolves the + * developer's real `~/.opencodex`. + * + * On 2026-09-15 exactly that invocation ran this file and deleted a live home. auth.json, + * codex-accounts.json, the service tokens and a 372MB usage ledger went with it; every OAuth + * login on the machine was gone, and only an unrelated three-week-old copy made any of it + * recoverable. The write guard could not help: `assertNotRealHomeUnderTest` covers + * writers, and `rmSync` is not one. + * + * Pinning the home here is what makes the deletion below safe under EITHER invocation. The + * previous value is restored afterwards because Bun reuses one process for several files. + */ +const PREVIOUS_OPENCODEX_HOME = process.env.OPENCODEX_HOME; +const ISOLATED_HOME = mkdtempSync(join(tmpdir(), "quota-reset-seen-store-")); +process.env.OPENCODEX_HOME = ISOLATED_HOME; + +afterAll(() => { + if (PREVIOUS_OPENCODEX_HOME === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = PREVIOUS_OPENCODEX_HOME; + rmSync(ISOLATED_HOME, { recursive: true, force: true }); +}); + const DAY = 24 * 60 * 60_000; /** * Real wall clock, not a fixed constant. @@ -75,7 +106,10 @@ describe("quota reset claim store", () => { // still reported a durable claim and the next start re-notified. // atomicWriteFile writes a sibling temp file in the config dir, so replacing that // directory with a regular file makes the real write fail without touching the module. - const configDir = getConfigDir(); + // This file's OWN directory, named directly: the store resolves the same path, and a + // destructive call must never be able to follow a config home it did not create. + const configDir = ISOLATED_HOME; + expect(getConfigDir()).toBe(configDir); rmSync(configDir, { recursive: true, force: true }); writeFileSync(configDir, "not a directory"); try {