diff --git a/packages/plugin/src/features/magic-context/storage-db.ts b/packages/plugin/src/features/magic-context/storage-db.ts index 9fa00518a..6f3e88f06 100644 --- a/packages/plugin/src/features/magic-context/storage-db.ts +++ b/packages/plugin/src/features/magic-context/storage-db.ts @@ -5,13 +5,11 @@ import { type Dirent, existsSync, mkdirSync, - mkdtempSync, readdirSync, readFileSync, statSync, unlinkSync, } from "node:fs"; -import { tmpdir } from "node:os"; import { basename, dirname, join } from "node:path"; import { bootQuietRemainingMs, scheduleAfterBootQuiet } from "../../plugin/boot-quiet"; import { @@ -158,79 +156,14 @@ export function resolveDatabasePath(dbPathOverride?: string): { dbDir: string; d if (dbPathOverride) { return { dbDir: dirname(dbPathOverride), dbPath: dbPathOverride }; } - // Test-isolation guard. Under the test runner the preload - // (bunfig.toml `[test] preload`) sets MAGIC_CONTEXT_TEST_DATA_DIR to a - // throwaway temp dir AND XDG_DATA_HOME to the same dir. Tests that manage - // their OWN XDG_DATA_HOME (per-test temp dirs) keep working — we honor XDG - // below via getMagicContextStorageDir(). The guard fires ONLY when - // XDG_DATA_HOME is UNSET: that is the dangerous window, because - // getMagicContextStorageDir() would otherwise fall back to the REAL - // ~/.local/share and a bare openDatabase() would run migrations on the - // user's production DB. Some tests delete XDG_DATA_HOME to exercise - // path-fallback behavior (2026-06-01 incident: a dormant test migrated the - // live DB to v26 and fail-closed every running v25 binary); in that window - // we resolve into the dedicated test dir instead of the real path. No test - // mutates MAGIC_CONTEXT_TEST_DATA_DIR, so the guard cannot be defeated. It - // is never set in production. - const testDataDir = process.env.MAGIC_CONTEXT_TEST_DATA_DIR; - if (testDataDir && !process.env.XDG_DATA_HOME) { - const dbDir = join(testDataDir, "cortexkit", "magic-context"); - return { dbDir, dbPath: join(dbDir, "context.db") }; - } - // CWD-INDEPENDENT TEST BACKSTOP. The MAGIC_CONTEXT_TEST_DATA_DIR / XDG guard - // above only fires when the bunfig `[test] preload` ran — which depends on - // `bun test`'s CWD having a bunfig with `[test] preload`. A `bun test` from a - // dir WITHOUT that wiring (monorepo root, a package missing its bunfig, or a - // brand-new package) recursively runs every *.test.ts with NO preload, so a - // bare openDatabase() would resolve to the user's REAL shared DB and run - // migrations on it. That is exactly how the live DB was migrated to v41 by a - // worktree whose LATEST was 41 (a re-run of the 2026-06-01 v26 incident). - // - // Bun sets NODE_ENV=test for EVERY `bun test` regardless of CWD/bunfig (and - // it is never "test" in the plugin runtime — production never sets it). So if - // we are under the test runner with neither the test data dir nor an explicit - // override, we MUST NOT touch real storage: redirect into a throwaway temp dir - // so the live DB is physically unreachable. This makes it structurally - // impossible for ANY test, from ANY CWD, to read or migrate production data. - // Fire ONLY when XDG_DATA_HOME is unset: that is the dangerous window where - // getMagicContextStorageDir() below would otherwise resolve to the REAL - // ~/.local/share shared DB. When a test sets its own XDG_DATA_HOME (a - // per-test temp dir, e.g. to exercise path fallbacks or share a DB across - // helper calls), getMagicContextStorageDir() already points inside that - // controlled dir — honor it, do not override. - if (process.env.NODE_ENV === "test" && !process.env.XDG_DATA_HOME) { - // Memoized per-process so repeated openDatabase() calls in the same - // unisolated test resolve to the SAME path (openDatabase caches by path; - // a fresh temp dir per call would defeat the cache and hand back - // different DB handles). - const dbDir = getTestBackstopDbDir(); - if (!testBackstopWarned) { - testBackstopWarned = true; - log( - "[magic-context] TEST BACKSTOP: NODE_ENV=test with no MAGIC_CONTEXT_TEST_DATA_DIR " + - `— redirecting DB to a throwaway temp dir (${dbDir}) so no test can touch the ` + - "user's real shared database. Wire `[test] preload` in this package's bunfig.toml.", - ); - } - return { dbDir, dbPath: join(dbDir, "context.db") }; - } + // Test-isolation guards (MAGIC_CONTEXT_TEST_DATA_DIR + the CWD-independent + // NODE_ENV backstop) both live in getMagicContextStorageDir(), so this + // resolver and every direct caller of that helper are covered by one + // implementation. See its doc comment for the incident history. const dbDir = getMagicContextStorageDir(); return { dbDir, dbPath: join(dbDir, "context.db") }; } -let testBackstopDbDir: string | null = null; -let testBackstopWarned = false; -function getTestBackstopDbDir(): string { - if (!testBackstopDbDir) { - testBackstopDbDir = join( - mkdtempSync(join(tmpdir(), "mc-test-db-backstop-")), - "cortexkit", - "magic-context", - ); - } - return testBackstopDbDir; -} - export function getDatabasePath(db: Database): string | null { return pathByDatabase.get(db) ?? null; } diff --git a/packages/plugin/src/shared/data-path.test.ts b/packages/plugin/src/shared/data-path.test.ts index 3d7bfc5fd..a12be3bd1 100644 --- a/packages/plugin/src/shared/data-path.test.ts +++ b/packages/plugin/src/shared/data-path.test.ts @@ -20,6 +20,8 @@ const savedEnv = { XDG_DATA_HOME: process.env.XDG_DATA_HOME, LOCALAPPDATA: process.env.LOCALAPPDATA, MAGIC_CONTEXT_LOG_PATH: process.env.MAGIC_CONTEXT_LOG_PATH, + MAGIC_CONTEXT_TEST_DATA_DIR: process.env.MAGIC_CONTEXT_TEST_DATA_DIR, + NODE_ENV: process.env.NODE_ENV, }; describe("data-path", () => { @@ -36,14 +38,14 @@ describe("data-path", () => { }); afterEach(() => { - if (savedEnv.XDG_CACHE_HOME !== undefined) - process.env.XDG_CACHE_HOME = savedEnv.XDG_CACHE_HOME; - if (savedEnv.XDG_DATA_HOME !== undefined) - process.env.XDG_DATA_HOME = savedEnv.XDG_DATA_HOME; - if (savedEnv.LOCALAPPDATA !== undefined) process.env.LOCALAPPDATA = savedEnv.LOCALAPPDATA; - if (savedEnv.MAGIC_CONTEXT_LOG_PATH !== undefined) - process.env.MAGIC_CONTEXT_LOG_PATH = savedEnv.MAGIC_CONTEXT_LOG_PATH; - else delete process.env.MAGIC_CONTEXT_LOG_PATH; + // Restore-or-delete every var this suite touches. Several tests lift a + // guard (NODE_ENV, MAGIC_CONTEXT_TEST_DATA_DIR) to assert production + // shape, so a restore that skips the unset case would leak state into + // the next test and make results order-dependent. + for (const [key, value] of Object.entries(savedEnv)) { + if (value !== undefined) process.env[key] = value; + else delete process.env[key]; + } }); test("getCacheDir falls back to /.cache when XDG_CACHE_HOME is unset (all platforms)", () => { @@ -89,8 +91,62 @@ describe("data-path", () => { // Cross-harness shared path: both OpenCode and Pi plugins read/write here, // unlike the legacy opencode/storage/plugin/magic-context location which // was OpenCode-specific. See ARCHITECTURE_DECISIONS memory for rationale. + // Production shape, so both test-isolation guards (the preload's data + // dir and the NODE_ENV backstop bun sets for every `bun test`) are + // lifted for the duration of this assertion. + const savedTestDir = process.env.MAGIC_CONTEXT_TEST_DATA_DIR; + const savedNodeEnv = process.env.NODE_ENV; + delete process.env.MAGIC_CONTEXT_TEST_DATA_DIR; + delete process.env.NODE_ENV; + try { + expect(getMagicContextStorageDir()).toBe( + path.join(os.homedir(), ".local", "share", "cortexkit", "magic-context"), + ); + } finally { + if (savedTestDir !== undefined) process.env.MAGIC_CONTEXT_TEST_DATA_DIR = savedTestDir; + if (savedNodeEnv !== undefined) process.env.NODE_ENV = savedNodeEnv; + } + }); + + test("getMagicContextStorageDir backstops to a temp dir under NODE_ENV=test with no guard set", () => { + // CWD-independent backstop: a `bun test` from a dir whose bunfig has no + // `[test] preload` runs every suite with neither guard env var set. The + // DB resolver used to own this branch, so direct callers of this helper + // (the CLI doctors' own PRAGMA integrity_check) still reached the real + // shared DB. Memoized, so repeated calls must agree — openDatabase() + // caches by path. + const savedTestDir = process.env.MAGIC_CONTEXT_TEST_DATA_DIR; + delete process.env.MAGIC_CONTEXT_TEST_DATA_DIR; + process.env.NODE_ENV = "test"; + try { + const resolved = getMagicContextStorageDir(); + expect(resolved).not.toContain(path.join(os.homedir(), ".local", "share")); + expect(resolved.endsWith(path.join("cortexkit", "magic-context"))).toBe(true); + expect(getMagicContextStorageDir()).toBe(resolved); + } finally { + if (savedTestDir !== undefined) process.env.MAGIC_CONTEXT_TEST_DATA_DIR = savedTestDir; + } + }); + + test("getMagicContextStorageDir honors MAGIC_CONTEXT_TEST_DATA_DIR when XDG_DATA_HOME is unset", () => { + // The hole this closes: a test that deletes XDG_DATA_HOME to exercise + // path fallbacks used to resolve to the user's REAL shared storage, + // because bun caches os.homedir() and a mutated process.env.HOME cannot + // move getDataDir(). Callers that build their own context.db path (the + // CLI doctors) then ran integrity checks against production data. + process.env.MAGIC_CONTEXT_TEST_DATA_DIR = "/tmp/mc-test-isolation"; expect(getMagicContextStorageDir()).toBe( - path.join(os.homedir(), ".local", "share", "cortexkit", "magic-context"), + path.join("/tmp/mc-test-isolation", "cortexkit", "magic-context"), + ); + }); + + test("getMagicContextStorageDir prefers XDG_DATA_HOME over MAGIC_CONTEXT_TEST_DATA_DIR", () => { + // A test managing its own per-test data home is already controlled, and + // several suites depend on that dir being honored. + process.env.MAGIC_CONTEXT_TEST_DATA_DIR = "/tmp/mc-test-isolation"; + process.env.XDG_DATA_HOME = "/tmp/custom-data"; + expect(getMagicContextStorageDir()).toBe( + path.join("/tmp/custom-data", "cortexkit", "magic-context"), ); }); diff --git a/packages/plugin/src/shared/data-path.ts b/packages/plugin/src/shared/data-path.ts index e0f064cb4..78ec9ffc4 100644 --- a/packages/plugin/src/shared/data-path.ts +++ b/packages/plugin/src/shared/data-path.ts @@ -1,4 +1,4 @@ -import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, writeFileSync } from "node:fs"; import * as os from "node:os"; import * as path from "node:path"; import { getHarness, type HarnessId } from "./harness"; @@ -167,11 +167,76 @@ export function getOpenCodeStorageDir(): string { * - Future cross-harness session migration * * Layout: /cortexkit/magic-context/ + * + * TEST-ISOLATION GUARD. `openDatabase()` has been guarded in + * `resolveDatabasePath()` since the 2026-06-01 (v26) and 2026-06-19 (v41) + * incidents, in which unisolated tests migrated the user's REAL shared DB. + * Every OTHER caller bypassed that guard — notably the CLI doctors, which + * build `join(getMagicContextStorageDir(), "context.db")` themselves and run + * `PRAGMA integrity_check` against it. A test that deletes XDG_DATA_HOME to + * exercise path fallbacks cannot restore isolation by setting + * `process.env.HOME`: bun caches `os.homedir()` at startup, so `getDataDir()` + * still resolves to the real home. MAGIC_CONTEXT_TEST_DATA_DIR — set by + * `packages/plugin/test-preload.ts`, and restored by any test that touches it + * — is honored here, so the preload's "cannot be defeated" guarantee holds for + * every caller, not just `openDatabase()`. It is never set in production. + * + * CWD-INDEPENDENT BACKSTOP. The preload only runs when `bun test`'s CWD has a + * bunfig wiring `[test] preload`. A run from a dir WITHOUT that wiring (a + * package missing its bunfig, or a brand-new one) executes every *.test.ts with + * NO preload, and this resolver would hand back the REAL shared path — which is + * how the live DB reached v41. Bun sets NODE_ENV=test for EVERY `bun test` + * regardless of CWD, and production never sets it, so that window redirects to + * a memoized throwaway dir instead. It lives here rather than in + * `resolveDatabasePath()` so direct callers (the CLI doctors' own + * `PRAGMA integrity_check`, announcements, the models.dev cache) are covered + * too — they never go through the DB resolver. + * + * XDG_DATA_HOME still wins over both: a test that manages its own data home is + * already controlled, and production has no test dir set at all. */ export function getMagicContextStorageDir(): string { + if (!process.env.XDG_DATA_HOME) { + const testDataDir = process.env.MAGIC_CONTEXT_TEST_DATA_DIR; + if (testDataDir) { + return path.join(testDataDir, "cortexkit", "magic-context"); + } + if (process.env.NODE_ENV === "test") { + return getTestBackstopStorageDir(); + } + } return path.join(getDataDir(), "cortexkit", "magic-context"); } +let testBackstopStorageDir: string | null = null; +let testBackstopWarned = false; + +/** + * Memoized per process so repeated calls in the same unisolated test resolve to + * the SAME path — `openDatabase()` caches by path, and a fresh temp dir per call + * would defeat that cache and hand back different DB handles. + */ +function getTestBackstopStorageDir(): string { + if (!testBackstopStorageDir) { + testBackstopStorageDir = path.join( + mkdtempSync(path.join(os.tmpdir(), "mc-test-db-backstop-")), + "cortexkit", + "magic-context", + ); + } + if (!testBackstopWarned) { + testBackstopWarned = true; + // Deliberately console, not the logger: logger.ts imports this module. + console.warn( + "[magic-context] TEST BACKSTOP: NODE_ENV=test with no MAGIC_CONTEXT_TEST_DATA_DIR " + + `— redirecting storage to a throwaway temp dir (${testBackstopStorageDir}) so no ` + + "test can touch the user's real shared database. Wire `[test] preload` in this " + + "package's bunfig.toml.", + ); + } + return testBackstopStorageDir; +} + /** * Legacy magic-context storage directory used by the OpenCode plugin before the * shared cortexkit path. Used only for one-time migration of existing data into