docs(test): name the real reason the suite could reach ~/.codeman

#356 stopped a bare suite run from overwriting the production
`remote-hosts.json` by pointing `CODEMAN_DATA_DIR` at a throwaway dir, and it
gated every case-tree delete on the temp HOME. Both changes are right; the
explanation written next to them is not. It says `os.homedir()` reads
/etc/passwd rather than `$HOME` on Linux, which would mean the temp HOME in
test/setup.ts never worked. It does: libuv checks the env var before the passwd
entry (measured: `HOME=/tmp/x node -e 'console.log(os.homedir())'` prints
/tmp/x), and CLAUDE.md's testing section relies on exactly that.

What bypasses the temp HOME is `CODEMAN_DATA_DIR` itself. `getDataDir()` reads
it as an absolute override before it looks at `homedir()`, so one inherited from
the shell (a second instance, a beta run) sends the whole suite at the real data
dir. That is the case setup.ts now closes, and #371 names the same variable from
the other direction.

The comments in setup.ts, the `safeRmHomeTree` helper, the voice-routes and
case-clone tests now say that, and the containment gate is described as what it
is: defense in depth. CLAUDE.md's testing paragraph gets the same note so the
next reader does not chase a homedir() bug that does not exist.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qg6bcATm1pNNY4kQWGwzgu
This commit is contained in:
Codeman maintainer
2026-09-04 13:50:22 +02:00
parent 28b44237ae
commit 2e0129f1f8
5 changed files with 27 additions and 25 deletions
+1 -1
View File
@@ -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).
+9 -9
View File
@@ -42,13 +42,14 @@ export function createDeferred<T = void>(): {
* 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;
+4 -4
View File
@@ -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).
*/
+4 -3
View File
@@ -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;
+9 -8
View File
@@ -34,14 +34,15 @@ process.env.HOME = testHome;
process.env.USERPROFILE = testHome;
process.env.VITEST = 'true';
// SAFETY: `getDataDir()` resolves via `homedir()` → `~/.codeman<INSTANCE_SUFFIX>`.
// 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<suffix>')`.
// 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;