diff --git a/test/mocks/index.ts b/test/mocks/index.ts index 0a17f837..2a696c2d 100644 --- a/test/mocks/index.ts +++ b/test/mocks/index.ts @@ -7,5 +7,5 @@ export { MockSession, createMockSession, terminalOutputs } from './mock-session.js'; export { MockStateStore } from './mock-state-store.js'; -export { waitForEvent, createDeferred } from './test-helpers.js'; +export { waitForEvent, createDeferred, safeRmHomeTree, isUnderTestHome } 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 cc64a787..6e68f022 100644 --- a/test/mocks/test-helpers.ts +++ b/test/mocks/test-helpers.ts @@ -2,6 +2,9 @@ * Reusable async test helpers. */ +import { rmSync } from 'node:fs'; +import { resolve } from 'node:path'; + /** Wait for an EventEmitter to emit a specific event, with timeout */ export function waitForEvent( emitter: { once: (event: string, listener: (...args: unknown[]) => void) => void }, @@ -34,3 +37,39 @@ export function createDeferred(): { }); return { promise, resolve, reject }; } + +/** + * 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, but code that resolves paths via `os.homedir()` does NOT + * follow that redirect on every Linux build/Node version — some read + * /etc/passwd instead of $HOME. A test that `rmSync(CASES_DIR, recursive)` can + * therefore delete the PRODUCTION `~/codeman-cases` (or any home-anchored + * tree) on those platforms. This gate refuses to delete anything not under the + * (redirected) `process.env.HOME`. Lexical `resolve()` is used because the leaf + * often does not exist and `realpathSync` would throw. + */ +export function safeRmHomeTree(path: string): void { + 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 }); + } +} + +/** + * 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 never touch prod state on + * platforms where `os.homedir()` ignores `$HOME`. + */ +export function isUnderTestHome(path: string): boolean { + const home = process.env.HOME; + if (!home) return false; + const target = resolve(path); + const root = resolve(home); + return target === root || target.startsWith(root + '/'); +} diff --git a/test/routes/session-routes-workspace-hooks.test.ts b/test/routes/session-routes-workspace-hooks.test.ts index 39c4763e..a7820cda 100644 --- a/test/routes/session-routes-workspace-hooks.test.ts +++ b/test/routes/session-routes-workspace-hooks.test.ts @@ -26,7 +26,7 @@ import { mkdtemp, rm, readFile, mkdir, writeFile } from 'node:fs/promises'; import { existsSync } from 'node:fs'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; -import { createMockRouteContext, type MockRouteContext } from '../mocks/index.js'; +import { createMockRouteContext, safeRmHomeTree, type MockRouteContext } from '../mocks/index.js'; import { installRouteErrorHandler } from '../../src/web/route-error-handler.js'; import { registerSessionRoutes } from '../../src/web/routes/session-routes.js'; import { generateHooksConfig, applyWorkspaceHooks } from '../../src/hooks-config.js'; @@ -259,7 +259,11 @@ describe('POST /api/quick-start workspace hooks', () => { // Docker fixtures + case dirs must not leak into the next test. await rm(join(getDataDir(), 'docker-hosts.json'), { force: true }); await rm(join(getDataDir(), 'docker-cases.json'), { force: true }); - await rm(CASES_DIR, { recursive: true, force: true }); + // SAFETY (2026-08-29): CASES_DIR is `join(homedir(), 'codeman-cases')`, and + // on environments where `os.homedir()` ignores `$HOME` it resolves to the + // PROD case tree. `safeRmHomeTree` refuses to delete anything not under the + // redirected test HOME, so a run can never nuke the real `~/codeman-cases`. + safeRmHomeTree(CASES_DIR); }); it('installs hooks into an EXISTING case directory (a linked case / cloned repo)', async () => { diff --git a/test/setup.ts b/test/setup.ts index e5135457..1b24e5ee 100644 --- a/test/setup.ts +++ b/test/setup.ts @@ -20,6 +20,7 @@ const originalVitest = process.env.VITEST; const originalPlaywrightBrowsersPath = process.env.PLAYWRIGHT_BROWSERS_PATH; const originalCodemanDataDir = process.env.CODEMAN_DATA_DIR; const testHome = mkdtempSync(join(tmpdir(), 'codeman-vitest-')); +const testDataDir = join(tmpdir(), `codeman-vitest-data-${process.pid}`); if (originalPlaywrightBrowsersPath === undefined && originalHome) { process.env.PLAYWRIGHT_BROWSERS_PATH = @@ -41,7 +42,7 @@ process.env.VITEST = 'true'; // overwrote prod `remote-hosts.json` with an `h1/box/10.0.0.5` fixture during a // bare full-suite run, wiping every user-defined remote host and emptying the // launch case dropdown). Point every test at a throwaway data dir instead. -process.env.CODEMAN_DATA_DIR = join(tmpdir(), `codeman-vitest-data-${process.pid}`); +process.env.CODEMAN_DATA_DIR = testDataDir; delete process.env.CODEMAN_PASSWORD; delete process.env.CODEMAN_USERNAME; @@ -61,7 +62,9 @@ afterAll(async () => { // "onUserConsoleLog" call is still pending, and that single unhandled // EnvironmentTeardownError fails the run after every test has passed // (observed twice on the PR #175/#176 merge commit; never locally). - await new Promise((resolve) => setTimeout(resolve, 50)); + const { promise: drained, resolve: drainDone } = Promise.withResolvers(); + setTimeout(drainDone, 50); + await drained; if (originalHome === undefined) delete process.env.HOME; else process.env.HOME = originalHome; @@ -79,7 +82,7 @@ afterAll(async () => { else process.env.CODEMAN_DATA_DIR = originalCodemanDataDir; rmSync(testHome, { recursive: true, force: true }); - rmSync(process.env.CODEMAN_DATA_DIR ?? '', { recursive: true, force: true }); + rmSync(testDataDir, { recursive: true, force: true }); }); // afterAll never fires for a fully-skipped test file (no tests execute), which @@ -87,4 +90,5 @@ afterAll(async () => { // with force is a no-op when afterAll already removed it. process.on('exit', () => { rmSync(testHome, { recursive: true, force: true }); + rmSync(testDataDir, { recursive: true, force: true }); });