diff --git a/CLAUDE.md b/CLAUDE.md index c647f7aa..e6a3e4b4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -407,7 +407,7 @@ Raw `npx vitest` skips the config (and with it `setup.ts`); always use `npm test **Config**: Vitest with `globals: true`, `fileParallelism: false`. Timeout 30s, teardown 60s. `config/vitest.config.ts` is the everything-config behind `test:all`; `config/vitest.ci.config.ts` is the gate and derives its excludes from `config/test-suites.ts`, which is also what `vitest.browser.config.ts` and `vitest.perf.config.ts` derive their includes from — so the exclusions and the runners cannot drift apart. Keep shared options in sync across them. -**Tmux safety**: under vitest (`VITEST` env var, set automatically), `TmuxManager` no-ops ALL shell commands and becomes a pure in-memory mock — tests physically cannot create/kill/attach real tmux sessions (`IS_TEST_MODE` in `src/tmux-manager.ts`). Every docker IO path is no-op'd the same way. `Session` is test-gated too: instead of attaching a real tmux client, it spawns a raw-mode echo PTY (`TEST_PTY_SCRIPT` in `src/session.ts`), so integration tests get a live input/output loop that echoes each byte exactly once. `test/setup.ts` gives every test file a temporary `HOME`/`USERPROFILE` (all `homedir()`-derived state, `~/.codeman` and `~/codeman-cases` included, resolves into a per-file fixture; the Playwright browser cache path is preserved), and additionally strips `CODEMAN_PASSWORD`/`CODEMAN_USERNAME` (so auth state from the running instance can't leak into tests) and `CODEMAN_GESTURE` (a shell-exported gesture flag would flip render-injection assertions). ⚠️ Raw `npx vitest` without `--config` skips `setup.ts` and with it the temp-HOME isolation. +**Tmux safety**: under vitest (`VITEST` env var, set automatically), `TmuxManager` no-ops ALL shell commands and becomes a pure in-memory mock — tests physically cannot create/kill/attach real tmux sessions (`IS_TEST_MODE` in `src/tmux-manager.ts`). Every docker IO path is no-op'd the same way. `Session` is test-gated too: instead of attaching a real tmux client, it spawns a raw-mode echo PTY (`TEST_PTY_SCRIPT` in `src/session.ts`), so integration tests get a live input/output loop that echoes each byte exactly once. `test/setup.ts` gives every test file a temporary `HOME`/`USERPROFILE` (all `homedir()`-derived state, `~/.codeman` and `~/codeman-cases` included, resolves into a per-file fixture; the Playwright browser cache path is preserved), and additionally strips `CODEMAN_PASSWORD`/`CODEMAN_USERNAME` (so auth state from the running instance can't leak into tests) and `CODEMAN_GESTURE` (a shell-exported gesture flag would flip render-injection assertions), and points `CODEMAN_DATA_DIR` at a throwaway dir (#356). ⚠️ That last one is what actually protects `~/.codeman`: `getDataDir()` reads `CODEMAN_DATA_DIR` as an ABSOLUTE override before it ever looks at `homedir()`, so one inherited from the shell (a second instance, a beta run) bypasses the temp HOME entirely, and a bare suite run once overwrote the real `remote-hosts.json` with a route test's fixture. `os.homedir()` itself DOES follow `$HOME`, so the temp HOME is what redirects everything else. Tests that delete case trees go through `safeRmHomeTree()` (`test/mocks`), which refuses any path outside the temp HOME, so a wrong anchor leaves a temp dir behind instead of deleting `~/codeman-cases`. ⚠️ Raw `npx vitest` without `--config` skips `setup.ts` and with it the temp-HOME isolation. **Ports**: Pick unique ports manually, 3150+. Search `const PORT =` before adding new tests. Never 3000 (the live instance). diff --git a/test/mocks/test-helpers.ts b/test/mocks/test-helpers.ts index 6e68f022..edf13015 100644 --- a/test/mocks/test-helpers.ts +++ b/test/mocks/test-helpers.ts @@ -42,13 +42,14 @@ 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, 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. + * 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. */ export function safeRmHomeTree(path: string): void { const home = process.env.HOME; @@ -63,8 +64,7 @@ export function safeRmHomeTree(path: string): void { /** * 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`. + * (single files like `linked-cases.json`) so they can never touch prod state. */ export function isUnderTestHome(path: string): boolean { const home = process.env.HOME; diff --git a/test/routes/case-clone-routes.test.ts b/test/routes/case-clone-routes.test.ts index 4d200003..ab0c3414 100644 --- a/test/routes/case-clone-routes.test.ts +++ b/test/routes/case-clone-routes.test.ts @@ -9,10 +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, 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. + * `test/setup.ts` points HOME at a per-file temp dir, so CASES_DIR + * (`join(homedir(), 'codeman-cases')`) resolves inside the fixture; cleanup + * below still goes through `safeRmHomeTree`, which refuses to delete anything + * outside the temp HOME, so a wrong anchor can never reach the real tree. * * Port: N/A (app.inject). */ diff --git a/test/routes/voice-routes.test.ts b/test/routes/voice-routes.test.ts index c2475bab..d1cd8886 100644 --- a/test/routes/voice-routes.test.ts +++ b/test/routes/voice-routes.test.ts @@ -25,9 +25,10 @@ import { registerVoiceRoutes, _resetVoiceStreamCountForTesting } from '../../src 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. +// which test/setup.ts points at a throwaway dir). `os.homedir()` follows it too, +// but this file writes and deletes `~/.claude/.credentials.json`, the one file +// where a wrong anchor would sign the developer out of their own CLI, so it +// fails loudly if setup.ts did not run rather than trusting any fallback. function testHome(): string { if (!process.env.HOME) throw new Error('process.env.HOME unset — test/setup.ts must run first'); return process.env.HOME; diff --git a/test/setup.ts b/test/setup.ts index 1b24e5ee..3c6e3ee9 100644 --- a/test/setup.ts +++ b/test/setup.ts @@ -34,14 +34,15 @@ 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. +// SAFETY: `getDataDir()` is `process.env.CODEMAN_DATA_DIR || join(homedir(), '.codeman')`. +// The temp HOME above already redirects the second half (`os.homedir()` follows +// `$HOME`; libuv checks the env var before the passwd entry), but the first half +// is an ABSOLUTE override: a `CODEMAN_DATA_DIR` inherited from the shell (a +// second instance, a beta run) bypasses the temp HOME entirely, and a bare suite +// run then reads and writes the REAL data dir (found 2026-08-29: +// `session-routes-workspace-hooks.test.ts` overwrote the production +// `remote-hosts.json` with an `h1/box/10.0.0.5` fixture, wiping every user-defined +// remote host and emptying the launch case dropdown). Point it at a throwaway dir. process.env.CODEMAN_DATA_DIR = testDataDir; delete process.env.CODEMAN_PASSWORD;