From 710ed73585f3aa3141423543e6ef8dd7fc09003f Mon Sep 17 00:00:00 2001 From: Devvyn <22340871+opticon454@users.noreply.github.com> Date: Mon, 12 Oct 2026 16:58:16 +0800 Subject: [PATCH] fix(test): a real home is never "the test HOME" to the deletion guards safeRmHomeTree and isUnderTestHome answered "is this path under process.env.HOME?". With test/setup.ts skipped (a bare vitest) HOME is the developer's real home, so every real file passed, which is how the real linked-cases registry was deleted on 2026-10-10. They now also require HOME to be the throwaway codeman-vitest-* directory setup.ts creates, and the shared route harness throws when it is not, so every route test refuses to start against a real ~/.codeman rather than just the one file that checked. --- test/mocks/index.ts | 10 ++- test/mocks/test-helpers.ts | 79 ++++++++++++++------- test/routes/_route-test-utils.ts | 6 +- test/routes/case-custom-path-routes.test.ts | 4 +- test/test-home-guard.test.ts | 78 ++++++++++++++++++++ 5 files changed, 149 insertions(+), 28 deletions(-) create mode 100644 test/test-home-guard.test.ts diff --git a/test/mocks/index.ts b/test/mocks/index.ts index 2a696c2d..29bc0c8e 100644 --- a/test/mocks/index.ts +++ b/test/mocks/index.ts @@ -7,5 +7,13 @@ export { MockSession, createMockSession, terminalOutputs } from './mock-session.js'; export { MockStateStore } from './mock-state-store.js'; -export { waitForEvent, createDeferred, safeRmHomeTree, isUnderTestHome } from './test-helpers.js'; +export { + waitForEvent, + createDeferred, + safeRmHomeTree, + isUnderTestHome, + isSandboxedHome, + assertSandboxedHome, + TEST_HOME_PREFIX, +} from './test-helpers.js'; export { createMockRouteContext, type MockRouteContext } from './mock-route-context.js'; diff --git a/test/mocks/test-helpers.ts b/test/mocks/test-helpers.ts index edf13015..c930e0a2 100644 --- a/test/mocks/test-helpers.ts +++ b/test/mocks/test-helpers.ts @@ -3,7 +3,7 @@ */ import { rmSync } from 'node:fs'; -import { resolve } from 'node:path'; +import { basename, resolve } from 'node:path'; /** Wait for an EventEmitter to emit a specific event, with timeout */ export function waitForEvent( @@ -39,37 +39,68 @@ export function createDeferred(): { } /** - * Delete a directory tree, but ONLY when it lives inside the test HOME. - * - * SAFETY (2026-08-29): `test/setup.ts` redirects `process.env.HOME` to a - * throwaway dir and `os.homedir()` follows it, so a `rmSync(CASES_DIR, - * recursive)` normally lands inside the fixture. This gate is defense in depth - * for the day that stops being true (a test that runs outside setup.ts, an - * env override that anchors a path elsewhere): it refuses to delete anything - * not under the redirected `process.env.HOME`, so the failure mode is a - * leftover temp dir rather than a deleted PRODUCTION `~/codeman-cases`. - * Lexical `resolve()` is used because the leaf often does not exist and - * `realpathSync` would throw. + * Prefix of the throwaway HOME `test/setup.ts` creates (`mkdtempSync(join(tmpdir(), 'codeman-vitest-'))`). + * Pinned by test/test-env-isolation.test.ts, so renaming one without the other fails there. */ -export function safeRmHomeTree(path: string): void { +export const TEST_HOME_PREFIX = 'codeman-vitest-'; + +/** + * True only when `process.env.HOME` is the throwaway directory `test/setup.ts` made. + * + * ⚠️ This is what the helpers below hang on, and it exists because "is the path under HOME" + * is a vacuous question when setup never ran: a bare `vitest` invocation has no setupFiles, + * HOME is the developer's REAL home, and every real file is then "under the test HOME". That is + * how a bare run on 2026-10-10 deleted the real linked-cases registry through a guard that was + * written to prevent exactly that. The temp directory's name is the only thing a real home + * cannot have. + */ +export function isSandboxedHome(): boolean { const home = process.env.HOME; - if (!home) return; - const target = resolve(path); - const root = resolve(home); - if (target === root || target.startsWith(root + '/')) { - rmSync(target, { recursive: true, force: true }); + return !!home && basename(resolve(home)).startsWith(TEST_HOME_PREFIX); +} + +/** + * Throw unless the suite is running under `test/setup.ts`'s temp HOME. The shared route harness + * calls this, so every route test refuses to start against a real `~/.codeman`. + */ +export function assertSandboxedHome(what = 'this test'): void { + if (!isSandboxedHome()) { + throw new Error( + `${what} reads and writes Codeman's data files and must run under test/setup.ts's temp HOME ` + + `(HOME=${process.env.HOME ?? '(unset)'}). Run it with \`npm test\` or ` + + `\`npx vitest run --config config/vitest.ci.config.ts \`, never with a bare vitest.` + ); } } /** - * True when `path` resolves strictly inside `process.env.HOME` (or to it). - * Same rationale as `safeRmHomeTree`; use for guarded non-recursive deletes - * (single files like `linked-cases.json`) so they can never touch prod state. + * Delete a directory tree, but ONLY when it lives inside the test HOME. + * + * SAFETY (2026-08-29, tightened 2026-10-12): `test/setup.ts` redirects `process.env.HOME` to a + * throwaway dir and `os.homedir()` follows it, so a `rmSync(CASES_DIR, recursive)` normally + * lands inside the fixture. This gate is defense in depth for the day that stops being true + * (a test that runs outside setup.ts, an env override that anchors a path elsewhere): it + * refuses to delete anything unless HOME IS the throwaway dir ({@link isSandboxedHome}) and the + * target is under it, so the failure mode is a leftover temp dir rather than a deleted + * PRODUCTION `~/codeman-cases`. The first version only checked "under HOME", which a run + * without setup.ts satisfied for the user's real home. + * Lexical `resolve()` is used because the leaf often does not exist and + * `realpathSync` would throw. + */ +export function safeRmHomeTree(path: string): void { + if (!isUnderTestHome(path)) return; + rmSync(resolve(path), { recursive: true, force: true }); +} + +/** + * True when `path` resolves strictly inside the throwaway test HOME (or to it), and false + * whenever HOME is anything else. Same rationale as `safeRmHomeTree`; use for guarded + * non-recursive deletes (single files like `linked-cases.json`) so they can never touch + * prod state. */ export function isUnderTestHome(path: string): boolean { - const home = process.env.HOME; - if (!home) return false; + if (!isSandboxedHome()) return false; + const root = resolve(process.env.HOME as string); const target = resolve(path); - const root = resolve(home); return target === root || target.startsWith(root + '/'); } diff --git a/test/routes/_route-test-utils.ts b/test/routes/_route-test-utils.ts index 42a4c291..d254bfa0 100644 --- a/test/routes/_route-test-utils.ts +++ b/test/routes/_route-test-utils.ts @@ -6,7 +6,7 @@ */ import Fastify, { type FastifyInstance } from 'fastify'; import fastifyCookie from '@fastify/cookie'; -import { createMockRouteContext, type MockRouteContext } from '../mocks/index.js'; +import { assertSandboxedHome, createMockRouteContext, type MockRouteContext } from '../mocks/index.js'; import { installRouteErrorHandler } from '../../src/web/route-error-handler.js'; import { MAX_ROUTE_PARAM_LENGTH } from '../../src/web/route-helpers.js'; @@ -31,6 +31,10 @@ export async function createRouteTestHarness( registerFn: (app: FastifyInstance, ctx: any) => void, ctxOptions?: { sessionId?: string; authUser?: { username: string; role: 'admin' | 'user' } } ): Promise { + // Route tests write the linked-cases, docker and settings files. Refuse to build a harness against + // a real home (a bare vitest has no setupFiles), instead of finding out from a wiped registry. + assertSandboxedHome('A route test'); + const app = Fastify({ logger: false, routerOptions: { maxParamLength: MAX_ROUTE_PARAM_LENGTH } }); // Register cookie plugin — some routes access req.cookies diff --git a/test/routes/case-custom-path-routes.test.ts b/test/routes/case-custom-path-routes.test.ts index 9db507d1..b49d703e 100644 --- a/test/routes/case-custom-path-routes.test.ts +++ b/test/routes/case-custom-path-routes.test.ts @@ -24,12 +24,12 @@ import { basename, join } from 'node:path'; import { createRouteTestHarness } from './_route-test-utils.js'; import { registerCaseRoutes } from '../../src/web/routes/case-routes.js'; import { dataPath } from '../../src/config/instance.js'; -import { safeRmHomeTree } from '../mocks/index.js'; +import { isSandboxedHome, safeRmHomeTree } from '../mocks/index.js'; const LINKED = () => dataPath('linked-cases.json'); const CASES_DIR = () => join(homedir(), 'codeman-cases'); /** test/setup.ts's temp HOME (its prefix is pinned by test/test-env-isolation.test.ts). */ -const sandboxed = () => basename(homedir()).startsWith('codeman-vitest-'); +const sandboxed = () => isSandboxedHome(); let workDir = ''; const work = () => workDir; const linked = (): Record => (existsSync(LINKED()) ? JSON.parse(readFileSync(LINKED(), 'utf8')) : {}); diff --git a/test/test-home-guard.test.ts b/test/test-home-guard.test.ts new file mode 100644 index 00000000..6481df99 --- /dev/null +++ b/test/test-home-guard.test.ts @@ -0,0 +1,78 @@ +/** + * @fileoverview The test-only deletion guards must treat a REAL home as "not the test HOME". + * + * `safeRmHomeTree` and `isUnderTestHome` were written so a test could never delete production + * state, and they asked "is this path under `process.env.HOME`?". With `test/setup.ts` skipped + * (a bare `vitest`), HOME is the developer's real home, so that answer is "yes" for every real + * file, and on 2026-10-10 they approved wiping the real linked-cases registry. They now also + * require HOME to be the throwaway directory setup.ts made. + * + * Port: none. Everything happens in a temp tree; HOME is stubbed for the duration of a test. + */ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { + TEST_HOME_PREFIX, + assertSandboxedHome, + isSandboxedHome, + isUnderTestHome, + safeRmHomeTree, +} from './mocks/index.js'; +import { createRouteTestHarness } from './routes/_route-test-utils.js'; + +const made: string[] = []; +/** A stand-in for a developer's REAL home: a real directory with real files, not a sandbox name. */ +function fakeRealHome(): string { + const dir = mkdtempSync(join(tmpdir(), 'not-the-sandbox-')); + made.push(dir); + mkdirSync(join(dir, '.codeman')); + writeFileSync(join(dir, '.codeman', 'linked-cases.json'), '{"keep":"/me"}'); + return dir; +} + +afterEach(() => { + vi.unstubAllEnvs(); + for (const d of made.splice(0)) rmSync(d, { recursive: true, force: true }); +}); + +describe('test HOME guards', () => { + it('this suite really runs under the sandboxed HOME', () => { + expect(isSandboxedHome()).toBe(true); + expect(() => assertSandboxedHome()).not.toThrow(); + }); + + it('pins the prefix test/setup.ts creates', () => { + expect(TEST_HOME_PREFIX).toBe('codeman-vitest-'); + }); + + it('safeRmHomeTree does not delete anything under a real home', () => { + const home = fakeRealHome(); + vi.stubEnv('HOME', home); + safeRmHomeTree(join(home, '.codeman')); + expect(existsSync(join(home, '.codeman', 'linked-cases.json'))).toBe(true); + }); + + it('isUnderTestHome is false for a real home, even for a file inside it', () => { + const home = fakeRealHome(); + vi.stubEnv('HOME', home); + expect(isSandboxedHome()).toBe(false); + expect(isUnderTestHome(join(home, '.codeman', 'linked-cases.json'))).toBe(false); + }); + + it('still deletes inside the real sandbox', () => { + const inside = mkdtempSync(join(process.env.HOME as string, 'guard-')); + writeFileSync(join(inside, 'f'), 'x'); + expect(isUnderTestHome(inside)).toBe(true); + safeRmHomeTree(inside); + expect(existsSync(inside)).toBe(false); + }); + + it('the route harness refuses to start against a real home', async () => { + const home = fakeRealHome(); + vi.stubEnv('HOME', home); + await expect(createRouteTestHarness(() => undefined)).rejects.toThrow(/temp HOME/); + expect(existsSync(join(home, '.codeman', 'linked-cases.json'))).toBe(true); + }); +});