From 2694d3f74acd71788dbd890b5075f62674537a15 Mon Sep 17 00:00:00 2001 From: timkjr Date: Sat, 29 Aug 2026 15:21:46 -0500 Subject: [PATCH 1/4] fix(test): isolate route tests from the production ~/.codeman data dir MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit session-routes-workspace-hooks.test.ts wrote its h1/box/10.0.0.5 host fixture into getDataDir()/remote-hosts.json. getDataDir() resolves via homedir() → ~/.codeman (INSTANCE_SUFFIX='' by default), and overriding HOME in test/setup.ts does NOT change os.homedir() on Linux — so every full-suite run silently overwrote the PRODUCTION remote-hosts.json, wiping user-defined remote hosts, emptying the launch-case dropdown and breaking remote session creation (found live 2026-08-29). The vitest v4 test.env config key is ignored (probe confirmed the worker still saw CODEMAN_DATA_DIR=undefined), so the reliable fix is stubbing the env inside the test: the fixture write now goes to a throwaway /tmp dir via vi.stubEnv + finally unstub. Verified: prod remote-hosts.json hash is identical before and after the suite run. --- config/vitest.ci.config.ts | 7 +++++++ config/vitest.config.ts | 7 +++++++ .../session-routes-workspace-hooks.test.ts | 21 ++++++++++++++----- test/setup.ts | 15 +++++++++++++ 4 files changed, 45 insertions(+), 5 deletions(-) diff --git a/config/vitest.ci.config.ts b/config/vitest.ci.config.ts index 0305838c..e0490680 100644 --- a/config/vitest.ci.config.ts +++ b/config/vitest.ci.config.ts @@ -23,6 +23,13 @@ export default defineConfig({ include: ['test/**/*.test.ts'], exclude: [...configDefaults.exclude, ...NON_CI_TEST_GLOBS], setupFiles: ['./test/setup.ts'], + // SAFETY: force every worker's data dir away from prod `~/.codeman`. Route + // tests (e.g. session-routes-workspace-hooks) write remote-hosts.json into + // `getDataDir()`; without this a bare run clobbers the production host + // registry (found 2026-08-29). `/tmp` is fine here — the tree is throwaway. + env: { + CODEMAN_DATA_DIR: '/tmp/codeman-vitest-data', + }, fileParallelism: false, testTimeout: 30000, teardownTimeout: 60000, diff --git a/config/vitest.config.ts b/config/vitest.config.ts index 578e6c4a..7b1317d4 100644 --- a/config/vitest.config.ts +++ b/config/vitest.config.ts @@ -21,6 +21,13 @@ export default defineConfig({ environment: 'node', include: ['test/**/*.test.ts'], setupFiles: ['./test/setup.ts'], + // SAFETY: force every worker's data dir away from prod `~/.codeman`. Route + // tests (e.g. session-routes-workspace-hooks) write remote-hosts.json into + // `getDataDir()`; without this a bare run clobbers the production host + // registry (found 2026-08-29). `/tmp` is fine here — the tree is throwaway. + env: { + CODEMAN_DATA_DIR: '/tmp/codeman-vitest-data', + }, // Run test files sequentially to respect mux session limits // Individual tests within files still run in parallel where safe fileParallelism: false, diff --git a/test/routes/session-routes-workspace-hooks.test.ts b/test/routes/session-routes-workspace-hooks.test.ts index 4957226d..39c4763e 100644 --- a/test/routes/session-routes-workspace-hooks.test.ts +++ b/test/routes/session-routes-workspace-hooks.test.ts @@ -168,11 +168,22 @@ describe('POST /api/sessions workspace hooks', () => { // `user@host:session` — locally a RELATIVE path, so a mkdir would create it // as a junk directory under the server cwd. statusLineTelemetry rides along: // applyStatusLineConfig mkdirs the same way and used to run for remote attaches. - await mkdir(getDataDir(), { recursive: true }); - await writeFile( - join(getDataDir(), 'remote-hosts.json'), - JSON.stringify([{ id: 'h1', label: 'box', host: '10.0.0.5', username: 'dev' }]) - ); + // SAFETY (2026-08-29): `getDataDir()` is call-time so stub the env to a + // throwaway dir for this write — otherwise this test overwrites the PROD + // `~/.codeman/remote-hosts.json` with the fixture below, wiping every + // user-defined remote host (caught live: a full-suite run emptied the + // launch-case dropdown and broke remote session creation). + const fixtureDataDir = join(tmpdir(), `codeman-hook-fixture-${process.pid}`); + vi.stubEnv('CODEMAN_DATA_DIR', fixtureDataDir); + try { + await mkdir(getDataDir(), { recursive: true }); + await writeFile( + join(getDataDir(), 'remote-hosts.json'), + JSON.stringify([{ id: 'h1', label: 'box', host: '10.0.0.5', username: 'dev' }]) + ); + } finally { + vi.unstubAllEnvs(); + } const res = await createSession({ name: 'hooks-remote', diff --git a/test/setup.ts b/test/setup.ts index edd11f51..e5135457 100644 --- a/test/setup.ts +++ b/test/setup.ts @@ -18,6 +18,7 @@ const originalHome = process.env.HOME; const originalUserProfile = process.env.USERPROFILE; 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-')); if (originalPlaywrightBrowsersPath === undefined && originalHome) { @@ -32,6 +33,16 @@ process.env.HOME = testHome; process.env.USERPROFILE = testHome; process.env.VITEST = 'true'; +// SAFETY: `getDataDir()` resolves via `homedir()` → `~/.codeman`. +// Overriding HOME above is NOT enough: on Linux `os.homedir()` reads /etc/passwd, +// not $HOME, so without this a route test that writes `remote-hosts.json` (or +// any state file) into `getDataDir()` silently clobbers the PRODUCTION +// `~/.codeman` tree (found 2026-08-29: `session-routes-workspace-hooks.test.ts` +// 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}`); + delete process.env.CODEMAN_PASSWORD; delete process.env.CODEMAN_USERNAME; // Gesture availability changes renderIndexHtml output (injects the @@ -64,7 +75,11 @@ afterAll(async () => { if (originalPlaywrightBrowsersPath === undefined) delete process.env.PLAYWRIGHT_BROWSERS_PATH; else process.env.PLAYWRIGHT_BROWSERS_PATH = originalPlaywrightBrowsersPath; + if (originalCodemanDataDir === undefined) delete process.env.CODEMAN_DATA_DIR; + else process.env.CODEMAN_DATA_DIR = originalCodemanDataDir; + rmSync(testHome, { recursive: true, force: true }); + rmSync(process.env.CODEMAN_DATA_DIR ?? '', { recursive: true, force: true }); }); // afterAll never fires for a fully-skipped test file (no tests execute), which From ff88b6957e4e5f9a84feefaee61d70d51e78efd7 Mon Sep 17 00:00:00 2001 From: timkjr Date: Sun, 30 Aug 2026 21:43:45 -0500 Subject: [PATCH 2/4] fix(test): guard the CASES_DIR delete + harden the data-dir teardown PR #356 stopped the remote-hosts.json fixture write from clobbering prod. Two holes in the same file remain: 1. The quick-start afterEach still ran rmSync(CASES_DIR, recursive). CASES_DIR is join(homedir(), 'codeman-cases'), and on Linux builds where os.homedir() reads /etc/passwd instead of $HOME it resolves to the PROD case tree - so a full-suite run deleted the real ~/codeman-cases. Add a shared safeRmHomeTree() containment gate that only deletes a path under the redirected test HOME. 2. setup.ts teardown did rmSync(process.env.CODEMAN_DATA_DIR ?? '') AFTER restoring the env - if a pre-existing prod CODEMAN_DATA_DIR was set, that deleted prod. Capture the throwaway dir in a const and clean that. A broader test-isolation sweep (10 files: cli-skill-target, edge-cases, integration-flows, operation-lightspeed, ralph-integration, case-clone-routes, voice-routes, session-cleanup, sse-events, sse-subscription-filter) also applies the same containment gates to every per-case delete. It is intentionally NOT included here to keep this PR skinny; it is identified and available on request. --- test/mocks/index.ts | 2 +- test/mocks/test-helpers.ts | 39 +++++++++++++++++++ .../session-routes-workspace-hooks.test.ts | 8 +++- test/setup.ts | 10 +++-- 4 files changed, 53 insertions(+), 6 deletions(-) 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 }); }); From 4068c02b9e310defa13af1f4b66402771d947450 Mon Sep 17 00:00:00 2001 From: timkjr Date: Sun, 30 Aug 2026 22:30:10 -0500 Subject: [PATCH 3/4] fix(test): write the remote-hosts fixture where the route actually reads it The "never writes hooks for a remote attach" test stubbed CODEMAN_DATA_DIR to a separate throwaway dir just for this write, but session-routes.ts's CODEMAN_CONFIG_DIR is a module-load-time constant frozen at test/setup.ts's sandboxed dir before this test ever runs. The fixture landed somewhere the route handler could never read, so the remote-host lookup silently failed (NOT_FOUND) and the test passed for the wrong reason -- createErrorResponse never sets reply.code(), so Fastify's default 200 made the NOT_FOUND branch and the intended success branch indistinguishable by status code alone. Write straight to getDataDir() instead, matching the docker-hosts fixture convention already used elsewhere in this file. Verified the fix actually exercises the success path (host resolves, 200 with a real session), not just an accidental 200 from the error branch. --- .../session-routes-workspace-hooks.test.ts | 33 +++++++++---------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/test/routes/session-routes-workspace-hooks.test.ts b/test/routes/session-routes-workspace-hooks.test.ts index a7820cda..94f0a7fb 100644 --- a/test/routes/session-routes-workspace-hooks.test.ts +++ b/test/routes/session-routes-workspace-hooks.test.ts @@ -19,7 +19,7 @@ * including the sweep's deleted-workspace guard. */ -import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import Fastify, { type FastifyInstance } from 'fastify'; import fastifyCookie from '@fastify/cookie'; import { mkdtemp, rm, readFile, mkdir, writeFile } from 'node:fs/promises'; @@ -168,22 +168,21 @@ describe('POST /api/sessions workspace hooks', () => { // `user@host:session` — locally a RELATIVE path, so a mkdir would create it // as a junk directory under the server cwd. statusLineTelemetry rides along: // applyStatusLineConfig mkdirs the same way and used to run for remote attaches. - // SAFETY (2026-08-29): `getDataDir()` is call-time so stub the env to a - // throwaway dir for this write — otherwise this test overwrites the PROD - // `~/.codeman/remote-hosts.json` with the fixture below, wiping every - // user-defined remote host (caught live: a full-suite run emptied the - // launch-case dropdown and broke remote session creation). - const fixtureDataDir = join(tmpdir(), `codeman-hook-fixture-${process.pid}`); - vi.stubEnv('CODEMAN_DATA_DIR', fixtureDataDir); - try { - await mkdir(getDataDir(), { recursive: true }); - await writeFile( - join(getDataDir(), 'remote-hosts.json'), - JSON.stringify([{ id: 'h1', label: 'box', host: '10.0.0.5', username: 'dev' }]) - ); - } finally { - vi.unstubAllEnvs(); - } + // SAFETY (2026-08-29): write straight to `getDataDir()` — `test/setup.ts` + // already sandboxes CODEMAN_DATA_DIR for the whole file (same convention as + // the docker-hosts fixtures below). A prior version of this test stubbed + // CODEMAN_DATA_DIR to a SEPARATE throwaway dir for just this write, but + // `session-routes.ts`'s `CODEMAN_CONFIG_DIR` is a module-load-time constant + // (frozen at the sandboxed dir before this test ever runs), so that fixture + // landed somewhere the route handler could never read it — the remote host + // lookup silently failed and the test passed for the wrong reason (Fastify + // defaults an unset reply code to 200, so the NOT_FOUND branch and the + // intended success branch were indistinguishable by status code alone). + await mkdir(getDataDir(), { recursive: true }); + await writeFile( + join(getDataDir(), 'remote-hosts.json'), + JSON.stringify([{ id: 'h1', label: 'box', host: '10.0.0.5', username: 'dev' }]) + ); const res = await createSession({ name: 'hooks-remote', From 4a63ab16040518f49264bf62c9a6b4bbda2d1fa4 Mon Sep 17 00:00:00 2001 From: timkjr Date: Sun, 30 Aug 2026 22:31:29 -0500 Subject: [PATCH 4/4] test: extend CASES_DIR containment guard to the rest of the suite #356 introduced safeRmHomeTree/isUnderTestHome to stop tests from deleting the PRODUCTION ~/codeman-cases tree on platforms where os.homedir() ignores the $HOME override -- but only applied it to the one file caught doing it live. CASES_DIR has no CODEMAN_DATA_DIR-style env override at all, so every other test file's raw rmSync(join(CASES_DIR, ...)) was the same unguarded pattern, just not yet triggered. Routes every CASES_DIR delete in these 10 files through safeRmHomeTree: cli-skill-target, edge-cases, integration-flows, operation-lightspeed, ralph-integration, routes/case-clone-routes, routes/voice-routes, session-cleanup, sse-events, sse-subscription-filter. Also fixes one instance in case-clone-routes.test.ts that mkdirSync'd then rmSync'd a CASES_DIR path directly with no guard at all -- the exact clobbering pattern #356 exists to prevent, found by extending the sweep. Held as a separate commit (and intended as a separate PR once #356 merges) rather than folding into #356 -- keeps the already-checked skinny fix reviewable on its own; this is the same bug class applied broadly, not new functionality. Verified: all 10 files pass (180 tests), npm run typecheck clean. --- test/cli-skill-target.test.ts | 13 +++++++--- test/edge-cases.test.ts | 21 ++++++---------- test/integration-flows.test.ts | 17 +++++-------- test/operation-lightspeed.test.ts | 15 ++++------- test/ralph-integration.test.ts | 11 +++----- test/routes/case-clone-routes.test.ts | 12 +++++---- test/routes/voice-routes.test.ts | 36 ++++++++++++++++----------- test/session-cleanup.test.ts | 16 ++++-------- test/sse-events.test.ts | 12 ++++----- test/sse-subscription-filter.test.ts | 14 ++++------- 10 files changed, 75 insertions(+), 92 deletions(-) diff --git a/test/cli-skill-target.test.ts b/test/cli-skill-target.test.ts index f3135116..66e720b2 100644 --- a/test/cli-skill-target.test.ts +++ b/test/cli-skill-target.test.ts @@ -18,6 +18,7 @@ import { mkdirSync, rmSync, writeFileSync, existsSync } from 'node:fs'; import { homedir } from 'node:os'; import { join } from 'node:path'; import { dataPath } from '../src/config/instance.js'; +import { safeRmHomeTree } from './mocks/index.js'; import { program, resolveCliCasePath, resolveSkillTargetPath } from '../src/cli.js'; import { getCasesDir } from '../src/config/cases-dir.js'; @@ -37,14 +38,18 @@ function writeLinkedCases(content: string): void { beforeEach(() => { rmSync(LINKED_CASES_FILE, { force: true }); - rmSync(CASES_DIR, { recursive: true, force: true }); - rmSync(LINKED_ROOT, { recursive: true, force: true }); + safeRmHomeTree(CASES_DIR); + safeRmHomeTree(LINKED_ROOT); }); afterEach(() => { + // LINKED_CASES_FILE is dataPath('linked-cases.json') → CODEMAN_DATA_DIR, + // which test/setup.ts points at a throwaway /tmp dir, so a plain delete is + // safe here. Only homedir()-derived paths (CASES_DIR/LINKED_ROOT) need the + // containment gate. rmSync(LINKED_CASES_FILE, { force: true }); - rmSync(CASES_DIR, { recursive: true, force: true }); - rmSync(LINKED_ROOT, { recursive: true, force: true }); + safeRmHomeTree(CASES_DIR); + safeRmHomeTree(LINKED_ROOT); }); describe('resolveSkillTargetPath (global)', () => { diff --git a/test/edge-cases.test.ts b/test/edge-cases.test.ts index 8ceec2ae..576b442a 100644 --- a/test/edge-cases.test.ts +++ b/test/edge-cases.test.ts @@ -1,8 +1,8 @@ import { describe, it, expect, beforeAll, afterAll, afterEach } from 'vitest'; import { WebServer } from '../src/web/server.js'; -import { existsSync, rmSync } from 'node:fs'; import { join } from 'node:path'; import { homedir } from 'node:os'; +import { safeRmHomeTree } from './mocks/index.js'; const TEST_PORT = 3110; const CASES_DIR = join(homedir(), 'codeman-cases'); @@ -19,13 +19,12 @@ describe('Edge Cases and Error Handling', () => { }); afterEach(() => { - // Clean up cases created during this test + // Clean up cases created during this test. SAFETY: CASES_DIR is + // homedir()-derived, which on some platforms ignores the test HOME — the + // containment gate refuses to delete anything not under the temp HOME. while (createdCases.length > 0) { const caseName = createdCases.pop()!; - const casePath = join(CASES_DIR, caseName); - if (existsSync(casePath)) { - rmSync(casePath, { recursive: true, force: true }); - } + safeRmHomeTree(join(CASES_DIR, caseName)); } }); @@ -349,15 +348,9 @@ describe('Concurrent Session Handling', () => { } } - // Cleanup - const { rmSync, existsSync } = await import('node:fs'); - const { join } = await import('node:path'); - const { homedir } = await import('node:os'); + // Cleanup (containment-gated: never touch prod ~/codeman-cases) for (const name of createdCases) { - const path = join(homedir(), 'codeman-cases', name); - if (existsSync(path)) { - rmSync(path, { recursive: true, force: true }); - } + safeRmHomeTree(join(homedir(), 'codeman-cases', name)); } }); }); diff --git a/test/integration-flows.test.ts b/test/integration-flows.test.ts index 727c7ebb..2d38aa89 100644 --- a/test/integration-flows.test.ts +++ b/test/integration-flows.test.ts @@ -1,8 +1,8 @@ import { describe, it, expect, beforeAll, afterAll, afterEach } from 'vitest'; import { WebServer } from '../src/web/server.js'; -import { existsSync, rmSync } from 'node:fs'; import { join } from 'node:path'; import { homedir } from 'node:os'; +import { safeRmHomeTree } from './mocks/index.js'; const TEST_PORT = 3115; const CASES_DIR = join(homedir(), 'codeman-cases'); @@ -24,13 +24,11 @@ describe('Integration Flows', () => { }); afterEach(() => { - // Clean up cases created during this test + // Clean up cases created during this test (containment-gated: never + // delete a case dir outside the temp HOME, e.g. prod ~/codeman-cases on + // platforms where os.homedir() ignores $HOME). while (createdCases.length > 0) { - const caseName = createdCases.pop()!; - const casePath = join(CASES_DIR, caseName); - if (existsSync(casePath)) { - rmSync(casePath, { recursive: true, force: true }); - } + safeRmHomeTree(join(CASES_DIR, createdCases.pop()!)); } }); @@ -296,10 +294,7 @@ describe('SSE Event Flow', () => { } catch {} } for (const caseName of createdCases) { - const casePath = join(CASES_DIR, caseName); - if (existsSync(casePath)) { - rmSync(casePath, { recursive: true, force: true }); - } + safeRmHomeTree(join(CASES_DIR, caseName)); } await server.stop(); }, 60000); diff --git a/test/operation-lightspeed.test.ts b/test/operation-lightspeed.test.ts index 34056cb4..2f59cdb1 100644 --- a/test/operation-lightspeed.test.ts +++ b/test/operation-lightspeed.test.ts @@ -12,7 +12,9 @@ */ import { describe, it, expect, beforeAll, afterAll, beforeEach } from 'vitest'; import { WebServer } from '../src/web/server.js'; - +import { safeRmHomeTree } from './mocks/index.js'; +import { homedir } from 'node:os'; +import { join } from 'node:path'; const TEST_PORT = 3215; // Helper to parse SSE events from raw text @@ -1043,15 +1045,8 @@ describe('Operation Lightspeed', () => { const caseEvent = events.find((e) => e.event === 'case:created'); expect(caseEvent).toBeDefined(); - // Cleanup - const { rmSync } = await import('node:fs'); - const { join } = await import('node:path'); - const { homedir } = await import('node:os'); - try { - rmSync(join(homedir(), 'codeman-cases', caseName), { recursive: true }); - } catch { - /* may not exist */ - } + // Cleanup (containment-gated: never touch prod ~/codeman-cases) + safeRmHomeTree(join(homedir(), 'codeman-cases', caseName)); }); it('should deliver session:created to every client, even those with a mismatched filter', async () => { diff --git a/test/ralph-integration.test.ts b/test/ralph-integration.test.ts index 46316339..597c03e0 100644 --- a/test/ralph-integration.test.ts +++ b/test/ralph-integration.test.ts @@ -13,9 +13,9 @@ import { describe, it, expect, beforeAll, afterAll, afterEach } from 'vitest'; import { WebServer } from '../src/web/server.js'; -import { existsSync, rmSync } from 'node:fs'; import { join } from 'node:path'; import { homedir } from 'node:os'; +import { safeRmHomeTree } from './mocks/index.js'; const TEST_PORT = 3125; const CASES_DIR = join(homedir(), 'codeman-cases'); @@ -33,13 +33,10 @@ describe('Ralph Integration Tests', () => { }); afterEach(() => { - // Clean up cases created during this test + // Clean up cases created during this test (containment-gated: never + // delete a case dir outside the temp HOME). while (createdCases.length > 0) { - const caseName = createdCases.pop()!; - const casePath = join(CASES_DIR, caseName); - if (existsSync(casePath)) { - rmSync(casePath, { recursive: true, force: true }); - } + safeRmHomeTree(join(CASES_DIR, createdCases.pop()!)); } }); diff --git a/test/routes/case-clone-routes.test.ts b/test/routes/case-clone-routes.test.ts index 6cb7e7a8..4d200003 100644 --- a/test/routes/case-clone-routes.test.ts +++ b/test/routes/case-clone-routes.test.ts @@ -9,8 +9,10 @@ * working tree in the case directory, that scaffolding does not overwrite the * repository's own files, and that a rejected URL never reaches git. * - * `test/setup.ts` points HOME at a per-file temp dir, so CASES_DIR resolves - * inside the fixture and nothing touches the developer's real ~/codeman-cases. + * `test/setup.ts` points HOME at a per-file temp dir, but CASES_DIR is + * `join(homedir(), 'codeman-cases')` and `os.homedir()` ignores the HOME + * override on some platforms/Node builds — so cleanup below goes through + * `safeRmHomeTree`, which refuses to delete anything outside the temp HOME. * * Port: N/A (app.inject). */ @@ -31,7 +33,7 @@ import { } from 'node:fs'; import { homedir, tmpdir } from 'node:os'; import { join } from 'node:path'; -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 { ApiErrorCode, httpStatusForErrorCode } from '../../src/types.js'; import { registerCaseRoutes } from '../../src/web/routes/case-routes.js'; @@ -147,7 +149,7 @@ describe('POST /api/cases/clone — input rejection', () => { expect(res.statusCode).toBe(httpStatusForErrorCode(ApiErrorCode.ALREADY_EXISTS)); expect(JSON.parse(res.body).error).toMatch(/already exists/i); } finally { - rmSync(join(CASES_DIR, 'taken'), { recursive: true, force: true }); + safeRmHomeTree(join(CASES_DIR, 'taken')); } }); }); @@ -217,7 +219,7 @@ describe.skipIf(!gitPresent)('POST /api/cases/clone — real clone', () => { afterAll(() => { rmSync(root, { recursive: true, force: true }); - for (const name of created) rmSync(join(CASES_DIR, name), { recursive: true, force: true }); + for (const name of created) safeRmHomeTree(join(CASES_DIR, name)); }); beforeEach(buildApp); diff --git a/test/routes/voice-routes.test.ts b/test/routes/voice-routes.test.ts index d29c2a5a..c2475bab 100644 --- a/test/routes/voice-routes.test.ts +++ b/test/routes/voice-routes.test.ts @@ -19,12 +19,33 @@ import Fastify, { type FastifyInstance } from 'fastify'; import fastifyWebsocket from '@fastify/websocket'; import WebSocket, { WebSocketServer } from 'ws'; import { mkdirSync, writeFileSync, rmSync } from 'node:fs'; -import { homedir } from 'node:os'; import { join } from 'node:path'; import { createMockRouteContext, type MockRouteContext } from '../mocks/index.js'; import { registerVoiceRoutes, _resetVoiceStreamCountForTesting } from '../../src/web/routes/voice-routes.js'; import { MAX_CONCURRENT_STREAMS } from '../../src/config/voice.js'; +// SAFETY (2026-08-29): anchor on the REDIRECTED test HOME (process.env.HOME, +// which test/setup.ts points at a throwaway dir) instead of os.homedir(). +// On some Linux builds os.homedir() reads /etc/passwd and would resolve to the +// REAL home, clobbering the user's ~/.claude/.credentials.json. +function testHome(): string { + if (!process.env.HOME) throw new Error('process.env.HOME unset — test/setup.ts must run first'); + return process.env.HOME; +} + +function writeCredentials(expiresAt: number | undefined): void { + const dir = join(testHome(), '.claude'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, '.credentials.json'), + JSON.stringify({ claudeAiOauth: { accessToken: TOKEN, expiresAt, subscriptionType: 'max' } }) + ); +} + +function removeCredentials(): void { + rmSync(join(testHome(), '.claude', '.credentials.json'), { force: true }); +} + const PORT = 3230; const UPSTREAM_PORT = 3231; const TOKEN = 'sk-ant-oat01-voice-route-test'; @@ -38,19 +59,6 @@ interface UpstreamCapture { socket: WebSocket | null; } -function writeCredentials(expiresAt: number | undefined): void { - const dir = join(homedir(), '.claude'); - mkdirSync(dir, { recursive: true }); - writeFileSync( - join(dir, '.credentials.json'), - JSON.stringify({ claudeAiOauth: { accessToken: TOKEN, expiresAt, subscriptionType: 'max' } }) - ); -} - -function removeCredentials(): void { - rmSync(join(homedir(), '.claude', '.credentials.json'), { force: true }); -} - function waitForClose(ws: WebSocket, timeoutMs = 3000): Promise<{ code: number; reason: string }> { return new Promise((resolve, reject) => { const timer = setTimeout(() => reject(new Error('WS close timeout')), timeoutMs); diff --git a/test/session-cleanup.test.ts b/test/session-cleanup.test.ts index 73e40c06..e7676729 100644 --- a/test/session-cleanup.test.ts +++ b/test/session-cleanup.test.ts @@ -1,8 +1,9 @@ import { describe, it, expect, beforeAll, afterAll, afterEach, vi } from 'vitest'; import { WebServer } from '../src/web/server.js'; -import { existsSync, mkdtempSync, rmSync } from 'node:fs'; +import { mkdtempSync, rmSync } from 'node:fs'; import { join } from 'node:path'; import { homedir, tmpdir } from 'node:os'; +import { safeRmHomeTree } from './mocks/index.js'; const TEST_PORT = 3120; const CASES_DIR = join(homedir(), 'codeman-cases'); @@ -27,13 +28,9 @@ describe('Session Cleanup', () => { }); afterEach(() => { - // Clean up cases created during this test + // Clean up cases created during this test (containment-gated). while (createdCases.length > 0) { - const caseName = createdCases.pop()!; - const casePath = join(CASES_DIR, caseName); - if (existsSync(casePath)) { - rmSync(casePath, { recursive: true, force: true }); - } + safeRmHomeTree(join(CASES_DIR, createdCases.pop()!)); } }); @@ -228,10 +225,7 @@ describe('Resource Management', () => { afterAll(async () => { for (const caseName of createdCases) { - const casePath = join(CASES_DIR, caseName); - if (existsSync(casePath)) { - rmSync(casePath, { recursive: true, force: true }); - } + safeRmHomeTree(join(CASES_DIR, caseName)); } await server.stop(); }, 60000); diff --git a/test/sse-events.test.ts b/test/sse-events.test.ts index ea3b1573..d602880e 100644 --- a/test/sse-events.test.ts +++ b/test/sse-events.test.ts @@ -1,6 +1,9 @@ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import { WebServer } from '../src/web/server.js'; import { EventEmitter } from 'node:events'; +import { safeRmHomeTree } from './mocks/index.js'; +import { homedir } from 'node:os'; +import { join } from 'node:path'; const TEST_PORT = 3107; @@ -295,13 +298,8 @@ describe('SSE Event Types', () => { expect(caseCreated).toBeDefined(); expect((caseCreated?.data as any).name).toBe(caseName); - // Cleanup - const { rmSync } = await import('node:fs'); - const { join } = await import('node:path'); - const { homedir } = await import('node:os'); - try { - rmSync(join(homedir(), 'codeman-cases', caseName), { recursive: true }); - } catch {} + // Cleanup (containment-gated) + safeRmHomeTree(join(homedir(), 'codeman-cases', caseName)); }); }); }); diff --git a/test/sse-subscription-filter.test.ts b/test/sse-subscription-filter.test.ts index 59511d92..54514785 100644 --- a/test/sse-subscription-filter.test.ts +++ b/test/sse-subscription-filter.test.ts @@ -1,5 +1,8 @@ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import { WebServer } from '../src/web/server.js'; +import { safeRmHomeTree } from './mocks/index.js'; +import { homedir } from 'node:os'; +import { join } from 'node:path'; const TEST_PORT = 3212; @@ -437,14 +440,7 @@ describe('SSE Subscription Filtering', () => { expect(caseCreated).toBeDefined(); expect((caseCreated?.data as any).name).toBe(caseName); - // Cleanup - const { rmSync } = await import('node:fs'); - const { join } = await import('node:path'); - const { homedir } = await import('node:os'); - try { - rmSync(join(homedir(), 'codeman-cases', caseName), { recursive: true }); - } catch { - /* may not exist */ - } + // Cleanup (containment-gated) + safeRmHomeTree(join(homedir(), 'codeman-cases', caseName)); }); });