diff --git a/test/setup.ts b/test/setup.ts index edd11f51..2bb09dbf 100644 --- a/test/setup.ts +++ b/test/setup.ts @@ -5,8 +5,11 @@ * mode before application modules load. Tests therefore cannot touch the real * Codeman state/cases tree or launch external tmux-backed agent sessions. * - * This setup file strips shell-level auth configuration that can leak from a - * running Codeman instance, then handles mock/timer cleanup between tests. + * This setup file strips shell-level configuration that can leak from a running + * Codeman instance — auth (`CODEMAN_PASSWORD`/`CODEMAN_USERNAME`), the gesture + * flag, and the three INSTANCE-selection vars that would otherwise point the + * suite at a real data dir or tmux socket — then handles mock/timer cleanup + * between tests. */ import { mkdtempSync, rmSync } from 'node:fs'; @@ -39,6 +42,33 @@ delete process.env.CODEMAN_USERNAME; // (test/server-index-title.test.ts) when the shell exports CODEMAN_GESTURE=1. delete process.env.CODEMAN_GESTURE; +// Instance selection is PROCESS-WIDE and is what `src/config/instance.ts` derives +// both the data dir and the tmux socket from, so a shell that exports any of these +// three reaches straight past the temp HOME above and undoes the isolation this +// file exists to provide: +// +// - CODEMAN_DATA_DIR is the dangerous one. It is an ABSOLUTE override read in +// `getDataDir()`, so it bypasses HOME entirely: a developer who exports it +// (or a shell left over from `codeman web -d`) has the suite reading and +// WRITING their real `state.json`, `users.json`, `intents.json` and +// `hook-secret` instead of a throwaway tree. +// - CODEMAN_INSTANCE moves the data dir to `~/.codeman-` and the socket to +// `codeman-`. Inside the temp HOME that is not a data-loss risk, but it +// silently changes the paths tests assert on — and `scripts/run-beta.sh` +// exports it, so any shell that has run a beta carries it. +// - CODEMAN_TMUX_SOCKET renames the socket `resolveTmuxSocketName()` returns. +// `TmuxManager` no-ops its shell commands under vitest, so this is assertion +// drift rather than a stray `tmux -L` against prod — but it is the same class +// of leak and the same one-line fix. +// +// ⚠️ These must be deleted HERE rather than in a test, because `CODEMAN_INSTANCE` +// is captured into a module-level const the first time `config/instance.ts` is +// imported. A setup file runs before any application module loads; a beforeEach +// would already be too late. +delete process.env.CODEMAN_INSTANCE; +delete process.env.CODEMAN_DATA_DIR; +delete process.env.CODEMAN_TMUX_SOCKET; + afterEach(() => { vi.clearAllMocks(); vi.useRealTimers(); diff --git a/test/test-env-isolation.test.ts b/test/test-env-isolation.test.ts new file mode 100644 index 00000000..07bc2e8b --- /dev/null +++ b/test/test-env-isolation.test.ts @@ -0,0 +1,73 @@ +/** + * @fileoverview Pins the environment isolation `test/setup.ts` provides. + * + * The suite's hermeticity rests on a temp `HOME` plus a short list of env vars that are + * deleted before any application module loads. That list is easy to under-maintain: it grew + * once for auth (`CODEMAN_PASSWORD`/`CODEMAN_USERNAME`) and once for `CODEMAN_GESTURE`, both + * times only after a leak had already produced a confusing failure, and it was still missing + * the three INSTANCE-selection vars. + * + * Those three matter more than the ones already on the list, because `src/config/instance.ts` + * derives BOTH the data dir and the tmux socket from them, and `CODEMAN_DATA_DIR` is an + * absolute path that bypasses `HOME` entirely — so a developer who exports it has the suite + * reading and writing their real `state.json` rather than a throwaway tree. + * + * ⚠️ The runtime half of this file cannot fail on a machine where the vars were never set, so + * it is not enough on its own: a `delete` line removed from `setup.ts` would still pass here + * on almost every developer's box and on CI. The STATIC half is what actually guards the + * list — it reads `setup.ts` and asserts each name is deleted there, which fails wherever the + * suite runs. Both halves are deliberate; do not drop the static one as redundant. + * + * Port: none (pure, over process.env and one source file). + */ + +import { describe, expect, it } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; + +/** Every env var `setup.ts` must strip, with why it would otherwise leak. */ +const STRIPPED_ENV_VARS: Array<[name: string, why: string]> = [ + ['CODEMAN_PASSWORD', 'auth from a running instance would make protected routes behave differently'], + ['CODEMAN_USERNAME', 'same, and it changes which owner scoping resolves to'], + ['CODEMAN_GESTURE', 'flips renderIndexHtml output and breaks byte-identity assertions'], + ['CODEMAN_INSTANCE', 'moves the data dir to ~/.codeman- and the tmux socket to codeman-'], + ['CODEMAN_DATA_DIR', 'ABSOLUTE override: bypasses the temp HOME and points the suite at a real data dir'], + ['CODEMAN_TMUX_SOCKET', 'renames the socket resolveTmuxSocketName() returns'], +]; + +const SETUP_SOURCE = readFileSync(fileURLToPath(new URL('./setup.ts', import.meta.url)), 'utf-8'); + +/** + * Just the top-of-file STRIP section, cut at the first hook. + * + * The teardown below it restores HOME/USERPROFILE/VITEST/PLAYWRIGHT_BROWSERS_PATH with the + * same `delete` syntax, and those are the opposite of a strip — counting them would make the + * anti-drift check demand a reason for a var the suite deliberately puts back. + */ +const SETUP_STRIP_SECTION = SETUP_SOURCE.split(/^afterEach\(/m)[0]; + +describe('test environment isolation', () => { + it.each(STRIPPED_ENV_VARS)('%s is unset while the suite runs', (name) => { + expect(process.env[name], `${name} leaked into the test environment`).toBeUndefined(); + }); + + it.each(STRIPPED_ENV_VARS)('setup.ts deletes %s (%s)', (name) => { + // The half that fails everywhere, not just on a machine that happens to export the var. + expect(SETUP_SOURCE, `setup.ts no longer deletes ${name}`).toContain(`delete process.env.${name};`); + }); + + it('runs against a throwaway HOME, not the real one', () => { + // The property every other test's isolation is built on: `~/.codeman` and `~/codeman-cases` + // both resolve under here, so a test that writes state cannot reach the developer's own. + const home = process.env.HOME ?? process.env.USERPROFILE; + expect(home).toBeTruthy(); + expect(home).toContain('codeman-vitest-'); + }); + + it('lists every name the setup file strips (anti-drift)', () => { + // Catches the other direction: a var added to setup.ts but never given a reason here, so + // the next person cannot tell whether it is load-bearing or left over. + const deleted = [...SETUP_STRIP_SECTION.matchAll(/delete process\.env\.([A-Z0-9_]+);/g)].map((m) => m[1]).sort(); + expect(deleted).toEqual(STRIPPED_ENV_VARS.map(([name]) => name).sort()); + }); +});