This repository has no description
0

Configure Feed

Select the types of activity you want to include in your feed.

test: scrub ambient PTY_ROOT so the suite is hermetic inside a pty session (#62)

When the vitest process itself runs inside a pty session, its ambient
PTY_ROOT / PTY_SESSION / PTY_SESSION_DIR leaked into every spawned `pty`.
getSessionDir() prefers PTY_ROOT over the per-test PTY_SESSION_DIR that the
helpers pass, so the spawned CLI read the developer's REAL live session dir
instead of the test tmpdir — non-deterministic failures, and a risk of
mutating live sessions.

- tests/setup/isolate-env.ts: new per-worker setupFile (registered in
vitest.config) that scrubs the three ambient vars before any test module
runs — the hard guard.
- Session.spawn: extract buildSpawnEnv() (pure, exported) and also scrub
ambient PTY_ROOT / PTY_SESSION_DIR there, so external users of the shipped
testing library get the same isolation. Guarded: an explicit root passed via
opts.env still wins.
- tests/env-isolation.test.ts: unit tests for buildSpawnEnv + a guard test
asserting the setupFile scrubbed the worker env.
- gc-flap-clear-badge-root-len: the restart test was passing only because an
ambient PTY_SESSION made `pty restart` skip its attach; set PTY_SESSION
explicitly so it no longer depends on the leak.

Verified: full suite green (1213 passed) WITH ambient PTY_ROOT set and no
env -u / shim workarounds; unchanged in clean CI (scrub is a no-op there).

authored by

Nathan and committed by
GitHub
(Jul 8, 2026, 7:26 PM +0200) 773fadbc d0317498

+126 -12
+33 -10
src/testing/session.ts
··· 26 26 return `test-${process.pid}-${Date.now()}-${++nameCounter}`; 27 27 } 28 28 29 + /** 30 + * Build the environment for a spawned `pty` process: the caller's base env 31 + * merged with `optsEnv`, minus the harness's own pty-internal context. 32 + * 33 + * Two hazards this guards against when the test harness ITSELF runs inside a 34 + * pty session: 35 + * - PTY_SESSION / PTY_SERVER_CONFIG leaking in would trip the spawned CLI's 36 + * nesting-prevention guard (or hand it a bogus server config). Always 37 + * scrubbed. 38 + * - PTY_ROOT / PTY_SESSION_DIR leaking in would override the caller's 39 + * intended isolation, because getSessionDir() prefers ambient PTY_ROOT over 40 + * the per-call PTY_SESSION_DIR — so the spawned `pty` would read the real 41 + * live session dir. Scrubbed ONLY when the caller didn't set them 42 + * explicitly via `optsEnv`, so a deliberate root override still wins. 43 + * 44 + * Pure and exported for unit testing. 45 + */ 46 + export function buildSpawnEnv( 47 + base: Record<string, string | undefined>, 48 + optsEnv?: Record<string, string>, 49 + ): Record<string, string> { 50 + const env: Record<string, string> = { 51 + ...(base as Record<string, string>), 52 + ...optsEnv, 53 + }; 54 + delete env.PTY_SERVER_CONFIG; 55 + delete env.PTY_SESSION; 56 + if (optsEnv?.PTY_ROOT === undefined) delete env.PTY_ROOT; 57 + if (optsEnv?.PTY_SESSION_DIR === undefined) delete env.PTY_SESSION_DIR; 58 + return env; 59 + } 60 + 29 61 export class Session { 30 62 private terminal: Terminal; 31 63 private serialize: SerializeAddon; ··· 77 109 const serialize = new xtermSerialize.SerializeAddon(); 78 110 terminal.loadAddon(serialize); 79 111 80 - const env: Record<string, string> = { 81 - ...(process.env as Record<string, string>), 82 - ...opts.env, 83 - }; 84 - // Scrub pty-internal env vars so the spawned CLI sees a clean user 85 - // environment, not the test harness's own pty context. Without this, 86 - // any test that runs `pty` inside a Session inherits the harness's 87 - // PTY_SESSION and trips the nesting-prevention guard. 88 - delete env.PTY_SERVER_CONFIG; 89 - delete env.PTY_SESSION; 112 + const env = buildSpawnEnv(process.env as Record<string, string | undefined>, opts.env); 90 113 91 114 const proc = pty.spawn(command, args, { 92 115 name: "xterm-256color",
+58
tests/env-isolation.test.ts
··· 1 + import { describe, it, expect } from "vitest"; 2 + import { buildSpawnEnv } from "../src/testing/session.ts"; 3 + 4 + // Regression coverage for the test-isolation gap: when the harness itself runs 5 + // inside a pty session, ambient PTY_ROOT (which getSessionDir() prefers over 6 + // PTY_SESSION_DIR) leaked into spawned `pty`s and made them read the real live 7 + // session dir instead of the per-test tmpdir. 8 + 9 + describe("buildSpawnEnv (Session.spawn env isolation)", () => { 10 + it("always scrubs PTY_SESSION and PTY_SERVER_CONFIG", () => { 11 + const env = buildSpawnEnv({ PTY_SESSION: "silber.pty", PTY_SERVER_CONFIG: "{}", HOME: "/h" }); 12 + expect(env.PTY_SESSION).toBeUndefined(); 13 + expect(env.PTY_SERVER_CONFIG).toBeUndefined(); 14 + expect(env.HOME).toBe("/h"); // unrelated vars pass through 15 + }); 16 + 17 + it("scrubs ambient PTY_ROOT / PTY_SESSION_DIR when the caller didn't set them", () => { 18 + const env = buildSpawnEnv({ PTY_ROOT: "/real/root", PTY_SESSION_DIR: "/real/dir" }); 19 + expect(env.PTY_ROOT).toBeUndefined(); 20 + expect(env.PTY_SESSION_DIR).toBeUndefined(); 21 + }); 22 + 23 + it("does NOT let ambient PTY_ROOT override an explicit per-call PTY_SESSION_DIR", () => { 24 + // The core failure mode: caller isolates via PTY_SESSION_DIR, harness has an 25 + // ambient PTY_ROOT. The child must see PTY_SESSION_DIR and no PTY_ROOT, so 26 + // getSessionDir() resolves to the caller's dir. 27 + const env = buildSpawnEnv( 28 + { PTY_ROOT: "/real/root", HOME: "/h" }, 29 + { PTY_SESSION_DIR: "/tmp/isolated" }, 30 + ); 31 + expect(env.PTY_ROOT).toBeUndefined(); 32 + expect(env.PTY_SESSION_DIR).toBe("/tmp/isolated"); 33 + }); 34 + 35 + it("keeps a PTY_ROOT the caller set explicitly (deliberate override wins)", () => { 36 + const env = buildSpawnEnv( 37 + { PTY_ROOT: "/ambient/root" }, 38 + { PTY_ROOT: "/wanted/root" }, 39 + ); 40 + expect(env.PTY_ROOT).toBe("/wanted/root"); 41 + }); 42 + 43 + it("keeps a PTY_SESSION_DIR the caller set explicitly", () => { 44 + const env = buildSpawnEnv({}, { PTY_SESSION_DIR: "/wanted/dir" }); 45 + expect(env.PTY_SESSION_DIR).toBe("/wanted/dir"); 46 + }); 47 + }); 48 + 49 + describe("isolation hard guard (tests/setup/isolate-env.ts)", () => { 50 + it("has scrubbed ambient PTY_ROOT / PTY_SESSION / PTY_SESSION_DIR from the worker", () => { 51 + // The setupFile runs once per worker before any test module. If this fails, 52 + // the suite is running with a leaked ambient pty context and any test that 53 + // spawns `pty` may read the developer's real live session dir. 54 + expect(process.env.PTY_ROOT).toBeUndefined(); 55 + expect(process.env.PTY_SESSION).toBeUndefined(); 56 + expect(process.env.PTY_SESSION_DIR).toBeUndefined(); 57 + }); 58 + });
+10 -2
tests/gc-flap-clear-badge-root-len.test.ts
··· 88 88 89 89 // Restart it. -y skips the prompt; command is "sh -c 'exit 1'" which 90 90 // will exit almost immediately — that's fine, we care about the tag 91 - // snapshot right after spawn. 92 - const r = runCli(dir, "restart", "-y", name); 91 + // snapshot right after spawn. PTY_SESSION is set so restart takes its 92 + // "already inside a session, don't attach" branch and returns 0 instead of 93 + // attaching to the just-exited session (a non-TTY attach here would inherit 94 + // that session's exit code 1). Setting it explicitly rather than relying on 95 + // an ambient PTY_SESSION leaking from the harness. 96 + const r = spawnSync(nodeBin, [cliPath, "restart", "-y", name], { 97 + env: { ...process.env, PTY_SESSION_DIR: dir, PTY_SESSION: "outer" }, 98 + encoding: "utf-8", 99 + timeout: 15000, 100 + }); 93 101 expect(r.status).toBe(0); 94 102 95 103 // Give the daemon a moment to write its metadata.
+21
tests/setup/isolate-env.ts
··· 1 + // Per-worker isolation hard guard. 2 + // 3 + // The vitest process may itself be running INSIDE a pty session (this repo is 4 + // often developed/tested from within a `pty` session), which means the ambient 5 + // environment carries PTY_ROOT / PTY_SESSION / PTY_SESSION_DIR. Tests isolate by 6 + // passing a per-test PTY_SESSION_DIR to each spawned `pty`, but getSessionDir() 7 + // prefers PTY_ROOT over PTY_SESSION_DIR — so an ambient PTY_ROOT (inherited via 8 + // `{ ...process.env }` in the test helpers) silently WINS and every spawned 9 + // `pty` reads the developer's REAL live session dir instead of the tmpdir. That 10 + // turns the suite non-deterministic and can even mutate live sessions. 11 + // 12 + // This file is registered as a vitest `setupFiles` entry, so it runs once in 13 + // every worker BEFORE any test module executes. Scrubbing here guarantees that 14 + // neither the worker nor any child it spawns (Session.spawn, raw spawn/spawnSync 15 + // in the test helpers) inherits the ambient pty context. Tests that WANT a 16 + // specific root still pass it explicitly per-child via `opts.env` / 17 + // `PTY_SESSION_DIR`, which is unaffected — we only clear the process-level 18 + // ambient values. 19 + delete process.env.PTY_ROOT; 20 + delete process.env.PTY_SESSION; 21 + delete process.env.PTY_SESSION_DIR;
+4
vitest.config.ts
··· 6 6 "node_modules/**", 7 7 ], 8 8 globalSetup: ["./tests/setup/vitest-global.ts"], 9 + // Runs once per worker before any test module — scrubs ambient 10 + // PTY_ROOT/PTY_SESSION/PTY_SESSION_DIR so a suite launched from inside a 11 + // pty session can't leak the real live session dir into spawned `pty`s. 12 + setupFiles: ["./tests/setup/isolate-env.ts"], 9 13 }, 10 14 });