diff --git a/.github/workflows/test-scratch-lifecycle.yml b/.github/workflows/test-scratch-lifecycle.yml index f229a76b1..ebfb1b431 100644 --- a/.github/workflows/test-scratch-lifecycle.yml +++ b/.github/workflows/test-scratch-lifecycle.yml @@ -76,7 +76,20 @@ jobs: - name: Scoped strict TypeScript run: pnpm exec tsc --project scripts/test-scratch/tsconfig.json --pretty false + - name: Require bubblewrap + # The /tmp visibility probe runs with TEST_SANDBOX_REQUIRE=1. Without a working bubblewrap it must fail, + # never fall back to the host /tmp, so install it, lift Ubuntu's unprivileged-userns restriction and + # prove it here with the flags test-sandbox uses. + run: | + sudo apt-get update -qq + sudo apt-get install -y --no-install-recommends bubblewrap + sudo sysctl -w kernel.apparmor_restrict_unprivileged_userns=0 || true + bwrap --dev-bind / / --tmpfs /tmp --unshare-net true + echo "bubblewrap verified: $(bwrap --version)" + - name: Lifecycle owner oracle + # The whole file runs in well under a minute; a hung process must fail the step, not the job. + timeout-minutes: 5 run: pnpm exec tsx --test scripts/test-scratch/run-command.test.ts - name: Canonical-entrypoint ratchet diff --git a/scripts/test-sandbox/README.md b/scripts/test-sandbox/README.md index 46a2c7814..a4eb07158 100644 --- a/scripts/test-sandbox/README.md +++ b/scripts/test-sandbox/README.md @@ -10,7 +10,7 @@ names the enforcement that remains. `TEST_SANDBOX_REQUIRE=1` makes missing enforcement an error. `TEST_SANDBOX_DISABLE=1` runs the command directly while keeping scratch ownership and cleanup. -The executable is copied byte-for-byte from `ai/skills/local/test-architecture/scripts/test-sandbox` in Tim Nunamaker's dotfiles at commit `ce0d966`. Change and test that source first, then update this copy and digest: +The executable is copied from `ai/skills/local/test-architecture/scripts/test-sandbox` in Tim Nunamaker's dotfiles at commit `ce0d966`, plus one later upstream fix applied as-is: commit `e4b320a` makes `TEST_SANDBOX_REQUIRE=1` exit 3 on bash older than 5 instead of running the command without isolation. Change and test that source first, then update this copy and digest: ```sh cp /ai/skills/local/test-architecture/scripts/test-sandbox scripts/test-sandbox/test-sandbox diff --git a/scripts/test-sandbox/test-sandbox b/scripts/test-sandbox/test-sandbox index b2403cfce..e20b8cc23 100755 --- a/scripts/test-sandbox/test-sandbox +++ b/scripts/test-sandbox/test-sandbox @@ -20,6 +20,9 @@ # unless --receipt is given. # Old bash (macOS ships 3.2) cannot parse what follows: run the command directly. if [ -z "${BASH_VERSINFO:-}" ] || [ "${BASH_VERSINFO[0]}" -lt 5 ]; then + if [ "${TEST_SANDBOX_REQUIRE:-0}" = 1 ] && [ "${TEST_SANDBOX_DISABLE:-0}" != 1 ]; then + echo "test-sandbox: bash 5 not found; required enforcement layer unavailable" >&2; exit 3 + fi echo "test-sandbox: bash 5 not found; running without enforcement" >&2 # Drop our options (bash 3.2 syntax only): everything after "--" is the command. # The scratch contract still holds: TMPDIR is --scratch (created 0700 if diff --git a/scripts/test-sandbox/test-sandbox.sha256 b/scripts/test-sandbox/test-sandbox.sha256 index 55259c461..771a7d78e 100644 --- a/scripts/test-sandbox/test-sandbox.sha256 +++ b/scripts/test-sandbox/test-sandbox.sha256 @@ -1 +1 @@ -b51043419454d9836b9a90014fe7a077129711913751bf9c59cba8c747203941 test-sandbox +f4ede419008088fdaa751087b202098adfe5b65075edb9f057b7b26eb2c4284d test-sandbox diff --git a/scripts/test-scratch/fixtures/leaked-group-after-failure.ts b/scripts/test-scratch/fixtures/leaked-group-after-failure.ts new file mode 100644 index 000000000..7237f8a5f --- /dev/null +++ b/scripts/test-scratch/fixtures/leaked-group-after-failure.ts @@ -0,0 +1,69 @@ +// Copyright The PDP-Connect Contributors +// SPDX-License-Identifier: Apache-2.0 + +// Run only by run-command.test.ts, as a child `node --test`: kill a wrapper's owner while +// its command and a background child are alive, then fail an assertion. The parent checks +// the run still exits and both processes are gone. + +import assert from "node:assert/strict"; +import { readFile, writeFile } from "node:fs/promises"; +import test from "node:test"; +import { setTimeout as sleep } from "node:timers/promises"; +import { lifecycleFixture } from "./lifecycle-fixture.ts"; + +const wrapperPath = new URL("../run-command.ts", import.meta.url).pathname; +const PID_FILE = /^\d+\n$/; + +// Shorter than the parent's 60 s timeout, so a stuck setup fails here and runs this file's after-hooks. +const READY_DEADLINE_MS = 20_000; + +// Each program publishes its own pid from inside the running process, atomically, once it is up. +const PUBLISH = + 'const f=require("node:fs");const w=(p)=>{f.writeFileSync(p+".tmp",process.pid+"\\n");f.renameSync(p+".tmp",p)};'; +// The background keep-alive. It inherits the wrapper's pipes, as the background child did in the CI hang. +const KEEP_ALIVE = `${PUBLISH}w(process.argv[1]);setInterval(()=>{},1e6)`; +// The command (owner). It publishes only after its keep-alive child reports ready, and exits at once if that child exits. +const COMMAND = `${PUBLISH}const c=require("node:child_process").spawn(process.execPath,["-e",${JSON.stringify(KEEP_ALIVE)},process.argv[2]],{stdio:"inherit"});c.on("exit",()=>process.exit(1));c.on("error",()=>process.exit(1));const t=setInterval(()=>{if(f.existsSync(process.argv[2])){clearInterval(t);w(process.argv[1]);setInterval(()=>{},1e6)}},10)`; + +/** Wait for both self-reported pids: the command's and its background child's. */ +async function readyPids(ownerFile: string, childFile: string): Promise<[number, number]> { + const deadline = Date.now() + READY_DEADLINE_MS; + while (Date.now() < deadline) { + // biome-ignore lint/performance/noAwaitInLoops: bounded by READY_DEADLINE_MS. + const owner = await readFile(ownerFile, "utf8").catch(() => ""); + const child = await readFile(childFile, "utf8").catch(() => ""); + if (PID_FILE.test(owner) && PID_FILE.test(child)) { + return [Number(owner), Number(child)]; + } + await sleep(25); + } + throw new Error("setup failed: the command never reported its pids"); +} + +function alive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +} + +test("owner killed while its command and a background child run, then an assertion fails", async (t) => { + const fx = await lifecycleFixture(t); + const ownerFile = fx.path("owner-pid"); + const childFile = fx.path("child-pid"); + const wrapper = fx.spawn( + process.execPath, + ["--import", "tsx", wrapperPath, "--", process.execPath, "-e", COMMAND, ownerFile, childFile], + { helpers: [ownerFile, childFile] } + ); + const pids = await readyPids(ownerFile, childFile); + const gone = pids.filter((pid) => !alive(pid)); + if (gone.length > 0 || wrapper.exitCode !== null || wrapper.signalCode !== null) { + throw new Error(`setup failed: the command or its background child is not running (gone: ${gone.join(",")})`); + } + await writeFile(process.env.PIDS_OUT as string, `${pids.join(" ")}\n`); + wrapper.kill("SIGKILL"); + assert.fail("deliberate failure while an orphaned command holds the wrapper's pipes"); +}); diff --git a/scripts/test-scratch/fixtures/lifecycle-fixture.ts b/scripts/test-scratch/fixtures/lifecycle-fixture.ts new file mode 100644 index 000000000..76093b8e1 --- /dev/null +++ b/scripts/test-scratch/fixtures/lifecycle-fixture.ts @@ -0,0 +1,170 @@ +// Copyright The PDP-Connect Contributors +// SPDX-License-Identifier: Apache-2.0 + +// The one place the lifecycle oracle creates directories for, launches, and cleans up +// after a command that runs through the scratch wrapper. It guards against two mistakes that +// hung CI on a runner image with bubblewrap, so no test has to remember them: +// +// 1. A helper the command cannot see. `scripts/test-sandbox/test-sandbox` gives the command +// a private /tmp and binds back only the top directory of its scratch root. Fixture-created +// paths stay inside `runnerTemp`; declared outside helpers are rejected. A path a test +// builds by hand and does not declare is not checked. +// 2. A process the test leaves behind. A leftover command group keeps the wrapper's pipes +// open, so the test process never exits. Teardown closes the pipes and kills what the +// fixture launched. +// +// Ownership is the launch environment: `spawn` requires `RUNNER_TEMP` to be this fixture's +// unique `runnerTemp`, and every descendant, including one in the sandbox, inherits it. +// Teardown kills exactly the processes carrying that value. It never signals a pid or group +// id recorded earlier: each process is matched and signalled in one synchronous step. + +import { type ChildProcess, spawn as nodeSpawn, type SpawnOptions } from "node:child_process"; +import { readdirSync, readFileSync } from "node:fs"; +import { mkdir, mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { isAbsolute, join, relative, resolve } from "node:path"; +import type { TestContext } from "node:test"; + +export interface LifecycleFixture { + /** Create `name` inside `runnerTemp` and return its path. */ + dir: (name: string) => Promise; + /** `process.env` with `RUNNER_TEMP` pointing at `runnerTemp`, plus `overrides`. */ + env: (overrides?: NodeJS.ProcessEnv) => NodeJS.ProcessEnv; + /** A path inside `runnerTemp`; throws for anything that would resolve outside it. */ + path: (...segments: string[]) => string; + /** The only directory tree a helper handed to a launched command may live in. */ + readonly runnerTemp: string; + /** Where the wrapper allocates its per-run scratch roots. */ + readonly scratchParent: string; + /** Start a process, killed with its pipes closed when the test ends. Each of `helpers` (files the command uses) must be inside `runnerTemp`. */ + spawn: ( + file: string, + args: readonly string[], + options?: SpawnOptions & { helpers?: readonly string[] } + ) => ChildProcess; + /** Point this process's `RUNNER_TEMP` at `runnerTemp` until the test ends; repeated calls change nothing. */ + useRunnerTempForThisTest: () => void; +} + +const DIGITS = /^\d+$/; + +function isInside(root: string, candidate: string): boolean { + const rel = relative(root, candidate); + return rel === "" || !(rel.startsWith("..") || isAbsolute(rel)); +} + +/** Kill every other process whose environment carries `entry`, rescanning while a parent forks; false when `/proc` is unreadable. */ +function killCarriers(entry: string): boolean { + let killed = -1; + for (let scan = 0; scan < 6 && killed !== 0; scan += 1) { + killed = 0; + let names: string[]; + try { + names = readdirSync("/proc"); + } catch { + return false; + } + for (const name of names.filter((n) => DIGITS.test(n) && Number(n) !== process.pid)) { + try { + // Nothing is awaited between this read and the kill; that gap is the window Jenkins' ProcessTreeKiller accepts. + if (readFileSync(`/proc/${name}/environ`, "utf8").split("\0").includes(entry)) { + process.kill(Number(name), "SIGKILL"); + killed += 1; + } + } catch { + // The environment is unreadable or the process already exited. + } + } + } + return true; +} + +function killQuietly(pid: number): void { + try { + process.kill(pid, "SIGKILL"); + } catch { + // The process already exited. + } +} + +/** Create a fixture for one test; everything it handed out is cleaned up in the test's `after` hook. */ +export async function lifecycleFixture( + t: TestContext, + options: { /** Parent of `runnerTemp`; defaults to the host temporary directory. */ under?: string } = {} +): Promise { + const runnerTemp = await mkdtemp(join(options.under ?? tmpdir(), "pdpp-test-scratch-test-")); + const children: ChildProcess[] = []; + let restoreRunnerTemp: (() => void) | undefined; + + const inside = (candidate: string): string => { + if (!isInside(runnerTemp, candidate)) { + throw new Error( + `${candidate} is outside the bound runner temp ${runnerTemp}; the sandbox's private /tmp hides it from the command` + ); + } + return candidate; + }; + const path = (...segments: string[]): string => inside(resolve(runnerTemp, ...segments)); + const env = (overrides: NodeJS.ProcessEnv = {}): NodeJS.ProcessEnv => ({ + ...process.env, + ...overrides, + RUNNER_TEMP: runnerTemp, + }); + + t.after(async () => { + const mayHaveLaunched = children.length > 0 || restoreRunnerTemp !== undefined; + restoreRunnerTemp?.(); + for (const proc of children) { + proc.stdin?.destroy(); + proc.stdout?.destroy(); + proc.stderr?.destroy(); + // An unreaped child still owns its pid, so signalling it by handle cannot hit another process. + if (proc.pid !== undefined && proc.exitCode === null && proc.signalCode === null) { + killQuietly(proc.pid); + } + } + // A test that launched nothing and never adopted RUNNER_TEMP has nothing to reap. + if (mayHaveLaunched && !killCarriers(`RUNNER_TEMP=${runnerTemp}`)) { + t.diagnostic(`/proc is unreadable, so descendants of ${runnerTemp} cannot be proven ours; none were killed`); + } + await rm(runnerTemp, { force: true, recursive: true }); + }); + + return { + runnerTemp, + scratchParent: join(runnerTemp, "pdpp-test-scratch"), + path, + env, + async dir(name) { + const target = path(name); + await mkdir(target, { recursive: true }); + return target; + }, + spawn(file, args, { helpers = [], ...spawnOptions } = {}) { + const launchEnv = spawnOptions.env ?? env(); + if (launchEnv.RUNNER_TEMP !== runnerTemp) { + throw new Error(`RUNNER_TEMP must be the fixture's runner temp ${runnerTemp}; got ${launchEnv.RUNNER_TEMP}`); + } + for (const helper of helpers) { + inside(resolve(helper)); + } + const proc = nodeSpawn(file, args, { ...spawnOptions, env: launchEnv }); + children.push(proc); + return proc; + }, + useRunnerTempForThisTest() { + if (restoreRunnerTemp) { + return; + } + const previous = process.env.RUNNER_TEMP; + process.env.RUNNER_TEMP = runnerTemp; + restoreRunnerTemp = () => { + if (previous === undefined) { + delete process.env.RUNNER_TEMP; + } else { + process.env.RUNNER_TEMP = previous; + } + }; + }, + }; +} diff --git a/scripts/test-scratch/run-command.test.ts b/scripts/test-scratch/run-command.test.ts index 48f96c5bc..c7dfe3e2c 100644 --- a/scripts/test-scratch/run-command.test.ts +++ b/scripts/test-scratch/run-command.test.ts @@ -21,7 +21,8 @@ import { } from "node:fs/promises"; import { platform, tmpdir } from "node:os"; import { join, posix, win32 } from "node:path"; -import test from "node:test"; +import test, { type TestContext } from "node:test"; +import { type LifecycleFixture, lifecycleFixture } from "./fixtures/lifecycle-fixture.ts"; import { allocateScratchOwnership, cleanupScratchOwnership, @@ -51,26 +52,38 @@ const lifecycleWrapperPath = new URL("./fixtures/lifecycle-wrapper.ts", import.m const sandboxChildWrapperPath = new URL("./fixtures/sandbox-child-wrapper.ts", import.meta.url).pathname; const delayedPublishSandboxChild = new URL("./fixtures/delayed-publish-sandbox-child.ts", import.meta.url).pathname; const killAfterSpawnSandboxChild = new URL("./fixtures/kill-after-spawn-sandbox-child.ts", import.meta.url).pathname; +const leakedGroupAfterFailure = new URL("./fixtures/leaked-group-after-failure.ts", import.meta.url).pathname; interface ChildResult { code: number | null; signal: NodeJS.Signals | null; } -function startWrapper(args: string[], env: NodeJS.ProcessEnv = process.env) { - return spawn(process.execPath, ["--import", "tsx", wrapperPath, ...command(args)], { +// Every launch of the wrapper goes through a LifecycleFixture: it keeps the paths a +// sandboxed command touches inside the bound runner temp and reaps what the test leaves. +function startWrapper(fx: LifecycleFixture, args: string[], env: NodeJS.ProcessEnv = fx.env()) { + return fx.spawn(process.execPath, ["--import", "tsx", wrapperPath, ...command(args)], { env, stdio: ["ignore", "pipe", "ignore"], }); } -function startRawWrapper(argv: string[], env: NodeJS.ProcessEnv = process.env) { - return spawn(process.execPath, ["--import", "tsx", wrapperPath, "--", ...argv], { +function startRawWrapper(fx: LifecycleFixture, argv: string[], env: NodeJS.ProcessEnv = fx.env()) { + return fx.spawn(process.execPath, ["--import", "tsx", wrapperPath, "--", ...argv], { env, stdio: ["ignore", "pipe", "pipe"], }); } +// The launch must allocate its own owner, not join one this process inherited. +const NO_INHERITED_OWNER = { + PDPP_TEST_SCRATCH_ROOT: undefined, + PDPP_TEST_SCRATCH_SCHEMA: undefined, + PDPP_TEST_SCRATCH_MARKER: undefined, + PDPP_TEST_SCRATCH_NONCE: undefined, + PDPP_TEST_SCRATCH_OWNER_PID: undefined, +}; + // Each sandbox these tests spawn must start from its own defaults, whatever // TEST_SANDBOX_* knobs the caller's shell or an outer run has set. const FRESH_SANDBOX_CONTEXT = { @@ -85,43 +98,33 @@ const FRESH_SANDBOX_CONTEXT = { const sandboxBashMajor = Number(spawnSync("bash", ["-c", "echo $BASH_VERSINFO"], { encoding: "utf8" }).stdout.trim()); const sandboxContract = { skip: sandboxBashMajor >= 5 ? false : "test-sandbox needs bash 5 to enforce its contract" }; -test("owner routes children through the sandbox unless explicitly disabled", sandboxContract, async () => { - const runnerTemp = await temporaryParent(); +test("owner routes children through the sandbox unless explicitly disabled", sandboxContract, async (t) => { + const fx = await lifecycleFixture(t); const probe = [ process.execPath, "-e", "console.log(JSON.stringify({active:process.env.TEST_SANDBOX_ACTIVE||null,tmp:process.env.TMPDIR,mode:require('node:fs').statSync(process.env.TMPDIR).mode%0o1000}))", ]; - try { - for (const disabled of [false, true]) { - const env = { - ...process.env, - PDPP_TEST_SCRATCH_ROOT: undefined, - PDPP_TEST_SCRATCH_SCHEMA: undefined, - PDPP_TEST_SCRATCH_MARKER: undefined, - PDPP_TEST_SCRATCH_NONCE: undefined, - PDPP_TEST_SCRATCH_OWNER_PID: undefined, - RUNNER_TEMP: runnerTemp, - ...FRESH_SANDBOX_CONTEXT, - TEST_SANDBOX_DISABLE: disabled ? "1" : "0", - }; - const wrapper = startRawWrapper(probe, env); - let stdout = ""; - wrapper.stdout?.on("data", (chunk: Buffer) => { - stdout += chunk.toString(); - }); - // biome-ignore lint/performance/noAwaitInLoops: exercise enabled and disabled launches sequentially. - assert.deepEqual(await closed(wrapper), { code: 0, signal: null }); - const result = JSON.parse(stdout.trim()) as { active: string | null; tmp: string; mode: number }; - assert.equal(result.active, disabled ? null : "1"); - if (!disabled) { - assert.equal(result.mode, 0o700); - } - assert.ok(result.tmp.includes("pdpp-test-scratch")); - await assert.rejects(access(result.tmp)); + for (const disabled of [false, true]) { + const env = fx.env({ + ...NO_INHERITED_OWNER, + ...FRESH_SANDBOX_CONTEXT, + TEST_SANDBOX_DISABLE: disabled ? "1" : "0", + }); + const wrapper = startRawWrapper(fx, probe, env); + let stdout = ""; + wrapper.stdout?.on("data", (chunk: Buffer) => { + stdout += chunk.toString(); + }); + // biome-ignore lint/performance/noAwaitInLoops: exercise enabled and disabled launches sequentially. + assert.deepEqual(await closed(wrapper), { code: 0, signal: null }); + const result = JSON.parse(stdout.trim()) as { active: string | null; tmp: string; mode: number }; + assert.equal(result.active, disabled ? null : "1"); + if (!disabled) { + assert.equal(result.mode, 0o700); } - } finally { - await rm(runnerTemp, { force: true, recursive: true }); + assert.ok(result.tmp.includes("pdpp-test-scratch")); + await assert.rejects(access(result.tmp)); } }); @@ -154,48 +157,34 @@ test("sandbox creates a missing --scratch root 0700 once and never re-creates it } }); -test("sandbox preserves literal shell variable syntax in child argv", async () => { - const runnerTemp = await temporaryParent(); - try { - // biome-ignore lint/suspicious/noTemplateCurlyInString: exercise literal shell syntax across the launcher boundary. - const wrapper = startRawWrapper(["bash", "-c", 'X=ok; printf %s "${X}"'], { - ...process.env, - PDPP_TEST_SCRATCH_ROOT: undefined, - PDPP_TEST_SCRATCH_SCHEMA: undefined, - PDPP_TEST_SCRATCH_MARKER: undefined, - PDPP_TEST_SCRATCH_NONCE: undefined, - PDPP_TEST_SCRATCH_OWNER_PID: undefined, - RUNNER_TEMP: runnerTemp, - ...FRESH_SANDBOX_CONTEXT, - TEST_SANDBOX_DISABLE: "0", - }); - let output = ""; - wrapper.stdout?.on("data", (chunk: Buffer) => { - output += chunk.toString(); - }); - assert.deepEqual(await closed(wrapper), { code: 0, signal: null }); - assert.equal(output, "ok"); - } finally { - await rm(runnerTemp, { force: true, recursive: true }); - } +test("sandbox preserves literal shell variable syntax in child argv", async (t) => { + const fx = await lifecycleFixture(t); + // biome-ignore lint/suspicious/noTemplateCurlyInString: exercise literal shell syntax across the launcher boundary. + const wrapper = startRawWrapper(fx, ["bash", "-c", 'X=ok; printf %s "${X}"'], { + ...fx.env(), + ...NO_INHERITED_OWNER, + ...FRESH_SANDBOX_CONTEXT, + TEST_SANDBOX_DISABLE: "0", + }); + let output = ""; + wrapper.stdout?.on("data", (chunk: Buffer) => { + output += chunk.toString(); + }); + assert.deepEqual(await closed(wrapper), { code: 0, signal: null }); + assert.equal(output, "ok"); }); -test("normal sandbox exit stops a background command child before scratch cleanup", async () => { - const runnerTemp = await temporaryParent(); +test("normal sandbox exit stops a background command child before scratch cleanup", async (t) => { + const fx = await lifecycleFixture(t); const wrapper = startRawWrapper( + fx, ["bash", "-c", 'sleep 300 /dev/null 2>&1 & printf "%s\\n%s\\n" "$!" "$TMPDIR"'], - { - ...process.env, - PDPP_TEST_SCRATCH_ROOT: undefined, - PDPP_TEST_SCRATCH_SCHEMA: undefined, - PDPP_TEST_SCRATCH_MARKER: undefined, - PDPP_TEST_SCRATCH_NONCE: undefined, - PDPP_TEST_SCRATCH_OWNER_PID: undefined, - RUNNER_TEMP: runnerTemp, + fx.env({ + ...NO_INHERITED_OWNER, TEST_RECEIPT_ACTIVE: "1", TEST_SANDBOX_ACTIVE: "1", TEST_SANDBOX_DISABLE: "0", - } + }) ); let output = ""; wrapper.stdout?.on("data", (chunk: Buffer) => { @@ -212,56 +201,37 @@ test("normal sandbox exit stops a background command child before scratch cleanu await eventuallyAbsentProcess(backgroundPid); await assert.rejects(access(root)); } finally { - if (backgroundPid) { - try { - process.kill(backgroundPid, "SIGKILL"); - } catch { - // The launcher may already have stopped the child. - } - } - await rm(runnerTemp, { force: true, recursive: true }); + killQuietly(backgroundPid); } }); test("sandbox sees its owned scratch root below /tmp", { skip: process.platform !== "linux", -}, async () => { - const runnerTemp = await mkdtemp(join("/tmp", "pdpp-test-sandbox-tmp-")); - try { - const wrapper = startRawWrapper( - [ - process.execPath, - "-e", - 'const fs=require("node:fs");const root=process.env.TMPDIR;fs.writeFileSync(root+"/visible","ok");console.log(JSON.stringify({root,active:process.env.TEST_SANDBOX_ACTIVE}))', - ], - { - ...process.env, - PDPP_TEST_SCRATCH_ROOT: undefined, - PDPP_TEST_SCRATCH_SCHEMA: undefined, - PDPP_TEST_SCRATCH_MARKER: undefined, - PDPP_TEST_SCRATCH_NONCE: undefined, - PDPP_TEST_SCRATCH_OWNER_PID: undefined, - RUNNER_TEMP: runnerTemp, - ...FRESH_SANDBOX_CONTEXT, - TEST_SANDBOX_DISABLE: "0", - } - ); - let output = ""; - wrapper.stdout?.on("data", (chunk: Buffer) => { - output += chunk.toString(); - }); - assert.deepEqual(await closed(wrapper), { code: 0, signal: null }); - const result = JSON.parse(output.trim()) as { root: string; active: string }; - assert.equal(result.active, "1"); - assert.ok(result.root.startsWith(`${runnerTemp}/pdpp-test-scratch/run-`)); - await assert.rejects(access(result.root)); - } finally { - await rm(runnerTemp, { force: true, recursive: true }); - } +}, async (t) => { + // The case under test is a scratch root directly below /tmp, whatever TMPDIR is. + const fx = await lifecycleFixture(t, { under: "/tmp" }); + const wrapper = startRawWrapper( + fx, + [ + process.execPath, + "-e", + 'const fs=require("node:fs");const root=process.env.TMPDIR;fs.writeFileSync(root+"/visible","ok");console.log(JSON.stringify({root,active:process.env.TEST_SANDBOX_ACTIVE}))', + ], + fx.env({ ...NO_INHERITED_OWNER, ...FRESH_SANDBOX_CONTEXT, TEST_SANDBOX_DISABLE: "0" }) + ); + let output = ""; + wrapper.stdout?.on("data", (chunk: Buffer) => { + output += chunk.toString(); + }); + assert.deepEqual(await closed(wrapper), { code: 0, signal: null }); + const result = JSON.parse(output.trim()) as { root: string; active: string }; + assert.equal(result.active, "1"); + assert.ok(result.root.startsWith(`${fx.runnerTemp}/pdpp-test-scratch/run-`)); + await assert.rejects(access(result.root)); }); -function startLaunchingWrapper(env: NodeJS.ProcessEnv = process.env) { - return spawn(process.execPath, ["--import", "tsx", lifecycleWrapperPath], { +function startLaunchingWrapper(fx: LifecycleFixture, env: NodeJS.ProcessEnv = fx.env()) { + return fx.spawn(process.execPath, ["--import", "tsx", lifecycleWrapperPath], { env, stdio: ["ignore", "pipe", "ignore"], }); @@ -514,8 +484,8 @@ test("cleanup refuses a live recorded process group before quarantine", async () } }); -test("participant creates no nested owner", async () => { - const parent = await temporaryParent(); +test("participant creates no nested owner", async (t) => { + const { runnerTemp: parent } = await lifecycleFixture(t); const ownership = await allocateScratchOwnership({ parent }); const keys = Object.keys(ownership.env) as Array; const before = new Map(keys.map((key) => [key, process.env[key]])); @@ -532,90 +502,60 @@ test("participant creates no nested owner", async () => { } } await cleanupScratchOwnership(ownership); - await rm(parent, { force: true, recursive: true }); } }); -test("participant validation fails closed when any inherited temporary path is altered", async () => { - const parent = await temporaryParent(); - const outside = await temporaryParent(); - const ownership = await allocateScratchOwnership({ parent }); +test("participant validation fails closed when any inherited temporary path is altered", async (t) => { + const fx = await lifecycleFixture(t); + const outside = await fx.dir("outside"); + const ownership = await allocateScratchOwnership({ parent: await fx.dir("parent") }); try { const altered = { ...ownership.env, TMPDIR: outside }; await assert.rejects(inheritedScratchOwnership(altered), { name: "ScratchOwnershipError", reason: "invalid-inherited-ownership", }); - const nested = startRawWrapper([process.execPath, "--import", "tsx", child, "--exit=0"], altered); + const nested = startRawWrapper(fx, [process.execPath, "--import", "tsx", child, "--exit=0"], fx.env(altered)); assert.deepEqual(await closed(nested), { code: 1, signal: null }); await assert.rejects(access(join(outside, "child.txt"))); await access(ownership.allocation.root); } finally { await cleanupScratchOwnership(ownership); - await rm(parent, { force: true, recursive: true }); - await rm(outside, { force: true, recursive: true }); } }); -test("launch failure removes its allocated root and returns the infrastructure exit", async () => { - const runnerTemp = await temporaryParent(); - const parent = join(runnerTemp, "pdpp-test-scratch"); - try { - const wrapper = startRawWrapper(["/definitely-not-a-pdpp-command"], { ...process.env, RUNNER_TEMP: runnerTemp }); - assert.deepEqual(await closed(wrapper), { code: 74, signal: null }); - assert.deepEqual(await readdir(parent), []); - } finally { - await rm(runnerTemp, { force: true, recursive: true }); - } +test("launch failure removes its allocated root and returns the infrastructure exit", async (t) => { + const fx = await lifecycleFixture(t); + const wrapper = startRawWrapper(fx, ["/definitely-not-a-pdpp-command"]); + assert.deepEqual(await closed(wrapper), { code: 74, signal: null }); + assert.deepEqual(await readdir(fx.scratchParent), []); }); -test("an unproven group shutdown retains scratch and makes a successful command infrastructure-failed", async () => { - const runnerTemp = await temporaryParent(); - const parent = join(runnerTemp, "pdpp-test-scratch"); - const previousRunnerTemp = process.env.RUNNER_TEMP; - process.env.RUNNER_TEMP = runnerTemp; - try { - const result = await runScratchCommand(command(["--exit=0"]), { stopGroup: async () => false }); - assert.deepEqual(result, { code: 74, signal: null }); - const roots = (await readdir(parent)).filter((entry) => entry.startsWith("run-")); - assert.equal(roots.length, 1); - } finally { - if (previousRunnerTemp === undefined) { - delete process.env.RUNNER_TEMP; - } else { - process.env.RUNNER_TEMP = previousRunnerTemp; - } - await rm(runnerTemp, { force: true, recursive: true }); - } +test("an unproven group shutdown retains scratch and makes a successful command infrastructure-failed", async (t) => { + const fx = await lifecycleFixture(t); + fx.useRunnerTempForThisTest(); + const result = await runScratchCommand(command(["--exit=0"]), { stopGroup: async () => false }); + assert.deepEqual(result, { code: 74, signal: null }); + const roots = (await readdir(fx.scratchParent)).filter((entry) => entry.startsWith("run-")); + assert.equal(roots.length, 1); }); -test("a rejected group proof retains scratch but preserves the child result", async () => { - const runnerTemp = await temporaryParent(); - const parent = join(runnerTemp, "pdpp-test-scratch"); - const previousRunnerTemp = process.env.RUNNER_TEMP; - process.env.RUNNER_TEMP = runnerTemp; - try { - for (const exitCode of [0, 42]) { - // biome-ignore lint/performance/noAwaitInLoops: each case must retain and reap its own live group before the next case. - const result = await runScratchCommand(["--", "sh", "-c", `sleep 30 & exit ${exitCode}`], { - stopGroup: (pgid) => { - process.kill(-pgid, "SIGKILL"); - throw new Error("simulate rejected group proof"); - }, - }); - assert.deepEqual(result, { code: exitCode === 0 ? 74 : exitCode, signal: null }); - const roots = (await readdir(parent)).filter((entry) => entry.startsWith("run-")); - assert.equal(roots.length, 1); - const root = join(parent, roots[0] as string); - await rm(root, { force: true, recursive: true }); - } - } finally { - if (previousRunnerTemp === undefined) { - delete process.env.RUNNER_TEMP; - } else { - process.env.RUNNER_TEMP = previousRunnerTemp; - } - await rm(runnerTemp, { force: true, recursive: true }); +test("a rejected group proof retains scratch but preserves the child result", async (t) => { + const fx = await lifecycleFixture(t); + fx.useRunnerTempForThisTest(); + for (const exitCode of [0, 42]) { + // biome-ignore lint/performance/noAwaitInLoops: each case must retain and reap its own live group before the next case. + const result = await runScratchCommand(["--", "sh", "-c", `sleep 30 & exit ${exitCode}`], { + stopGroup: (pgid) => { + process.kill(-pgid, "SIGKILL"); + throw new Error("simulate rejected group proof"); + }, + }); + assert.deepEqual(result, { code: exitCode === 0 ? 74 : exitCode, signal: null }); + const roots = (await readdir(fx.scratchParent)).filter((entry) => entry.startsWith("run-")); + assert.equal(roots.length, 1); + const root = join(fx.scratchParent, roots[0] as string); + await rm(root, { force: true, recursive: true }); } }); @@ -927,28 +867,18 @@ test("cleanup rejects a forged nonce before constructing a quarantine target", a } }); -test("CLI child signal remains a signal to its parent", async () => { - const result = await new Promise<{ code: number | null; signal: NodeJS.Signals | null }>((resolve, reject) => { - const proc = spawn( - process.execPath, - ["--import", "tsx", new URL("./run-command.ts", import.meta.url).pathname, ...command(["--self-signal=SIGTERM"])], - { - stdio: "ignore", - } - ); - proc.once("error", reject); - proc.once("close", (code, signal) => resolve({ code, signal })); +test("CLI child signal remains a signal to its parent", async (t) => { + const fx = await lifecycleFixture(t); + const proc = fx.spawn(process.execPath, ["--import", "tsx", wrapperPath, ...command(["--self-signal=SIGTERM"])], { + stdio: "ignore", }); - assert.deepEqual(result, { code: null, signal: "SIGTERM" }); + assert.deepEqual(await closed(proc), { code: null, signal: "SIGTERM" }); }); -test("SIGTERM reaches the owned group and cleanup completes before wrapper termination", async () => { +test("SIGTERM reaches the owned group and cleanup completes before wrapper termination", async (t) => { + const fx = await lifecycleFixture(t); let output = ""; - const wrapper = spawn( - process.execPath, - ["--import", "tsx", new URL("./run-command.ts", import.meta.url).pathname, ...command(["--print-root", "--wait"])], - { stdio: ["ignore", "pipe", "ignore"] } - ); + const wrapper = startWrapper(fx, ["--print-root", "--wait"]); wrapper.stdout?.on("data", (chunk: Buffer) => { output += chunk.toString(); }); @@ -973,18 +903,10 @@ test("SIGTERM reaches the owned group and cleanup completes before wrapper termi await assert.rejects(access(root)); }); -test("SIGINT is forwarded unchanged to the owned group before cleanup", async () => { +test("SIGINT is forwarded unchanged to the owned group before cleanup", async (t) => { + const fx = await lifecycleFixture(t); let output = ""; - const wrapper = spawn( - process.execPath, - [ - "--import", - "tsx", - new URL("./run-command.ts", import.meta.url).pathname, - ...command(["--print-root", "--record-signals", "--wait"]), - ], - { stdio: ["ignore", "pipe", "ignore"] } - ); + const wrapper = startWrapper(fx, ["--print-root", "--record-signals", "--wait"]); wrapper.stdout?.on("data", (chunk: Buffer) => { output += chunk.toString(); }); @@ -1013,19 +935,14 @@ test("SIGINT is forwarded unchanged to the owned group before cleanup", async () }); /** Environment for a sandbox with its cgroup layer absent, as under test-receipt or without a systemd user manager. */ -function noCgroupEnv(runnerTemp: string): NodeJS.ProcessEnv { - return { - ...process.env, - PDPP_TEST_SCRATCH_ROOT: undefined, - PDPP_TEST_SCRATCH_SCHEMA: undefined, - PDPP_TEST_SCRATCH_MARKER: undefined, - PDPP_TEST_SCRATCH_NONCE: undefined, - PDPP_TEST_SCRATCH_OWNER_PID: undefined, - RUNNER_TEMP: runnerTemp, +function noCgroupEnv(fx: LifecycleFixture, overrides: NodeJS.ProcessEnv = {}): NodeJS.ProcessEnv { + return fx.env({ + ...NO_INHERITED_OWNER, TEST_RECEIPT_ACTIVE: "1", ...FRESH_SANDBOX_CONTEXT, TEST_SANDBOX_DISABLE: "0", - }; + ...overrides, + }); } function processAlive(pid: number): boolean { @@ -1043,9 +960,9 @@ function killQuietly(pid: number | undefined): void { } } -test("SIGINT waits for the sidecar to publish its group before the launcher cleans up", async () => { - const runnerTemp = await temporaryParent(); - const wrapper = spawn( +test("SIGINT waits for the sidecar to publish its group before the launcher cleans up", async (t) => { + const fx = await lifecycleFixture(t); + const wrapper = fx.spawn( process.execPath, [ "--import", @@ -1060,7 +977,7 @@ test("SIGINT waits for the sidecar to publish its group before the launcher clea "--record-signals", "--wait", ], - { env: noCgroupEnv(runnerTemp), stdio: ["ignore", "ignore", "ignore"] } + { env: noCgroupEnv(fx), stdio: ["ignore", "ignore", "ignore"] } ); const result = closed(wrapper); let settled = false; @@ -1068,11 +985,9 @@ test("SIGINT waits for the sidecar to publish its group before the launcher clea settled = true; }); let sidecarPgid: number | undefined; - let groupPath: string | undefined; try { - const parent = join(runnerTemp, "pdpp-test-scratch"); - const root = await eventuallyRunRoot(parent); - groupPath = join(root, ".sandbox-child-group"); + const root = await eventuallyRunRoot(fx.scratchParent); + const groupPath = join(root, ".sandbox-child-group"); sidecarPgid = Number((await eventuallyRead(`${groupPath}.held`, 20_000)).trim()); assert.ok(Number.isSafeInteger(sidecarPgid) && sidecarPgid > 0); await assert.rejects(access(groupPath)); @@ -1086,11 +1001,6 @@ test("SIGINT waits for the sidecar to publish its group before the launcher clea await eventuallyAbsentGroup(sidecarPgid); await assert.rejects(access(root)); } finally { - if (groupPath) { - await writeFile(`${groupPath}.release`, "release\n").catch(() => { - // The owner may already have removed its scratch root. - }); - } if (sidecarPgid) { try { process.kill(-sidecarPgid, "SIGKILL"); @@ -1098,145 +1008,110 @@ test("SIGINT waits for the sidecar to publish its group before the launcher clea // The group may have exited before this failure-path cleanup. } } - if (wrapper.exitCode === null && wrapper.signalCode === null) { - wrapper.kill("SIGTERM"); - await result; - } - await rm(runnerTemp, { force: true, recursive: true }); } }); -test("a sandbox that exits before publishing a command group leaves no scratch and propagates its status", async () => { - const runnerTemp = await temporaryParent(); - const stubs = await temporaryParent(); - try { - // A bwrap that cannot create namespaces makes TEST_SANDBOX_REQUIRE=1 exit 3 before the command starts. - await writeFile(join(stubs, "bwrap"), "#!/bin/sh\nexit 1\n", { mode: 0o755 }); - const wrapper = spawn(process.execPath, ["--import", "tsx", wrapperPath, ...command(["--exit=0"])], { - env: { - ...process.env, - PATH: `${stubs}:${process.env.PATH}`, - PDPP_TEST_SCRATCH_ROOT: undefined, - PDPP_TEST_SCRATCH_SCHEMA: undefined, - PDPP_TEST_SCRATCH_MARKER: undefined, - PDPP_TEST_SCRATCH_NONCE: undefined, - PDPP_TEST_SCRATCH_OWNER_PID: undefined, - RUNNER_TEMP: runnerTemp, - ...FRESH_SANDBOX_CONTEXT, - TEST_SANDBOX_DISABLE: "0", - TEST_SANDBOX_REQUIRE: "1", - }, - stdio: ["ignore", "ignore", "pipe"], - }); - let stderr = ""; - wrapper.stderr?.on("data", (chunk: Buffer) => { - stderr += chunk.toString(); - }); - const started = Date.now(); - assert.deepEqual(await closed(wrapper), { code: 3, signal: null }); - assert.doesNotMatch(stderr, GROUP_STILL_LIVE_OUTPUT); - assert.ok(Date.now() - started < 3000, "the launcher must not wait out the publish grace period"); - assert.deepEqual(await readdir(join(runnerTemp, "pdpp-test-scratch")), []); - } finally { - await rm(runnerTemp, { force: true, recursive: true }); - await rm(stubs, { force: true, recursive: true }); - } +test("a sandbox that exits before publishing a command group leaves no scratch and propagates its status", async (t) => { + const fx = await lifecycleFixture(t); + const stubs = await fx.dir("stubs"); + // A bwrap that cannot create namespaces makes TEST_SANDBOX_REQUIRE=1 exit 3 before the command starts. + await writeFile(join(stubs, "bwrap"), "#!/bin/sh\nexit 1\n", { mode: 0o755 }); + const wrapper = fx.spawn(process.execPath, ["--import", "tsx", wrapperPath, ...command(["--exit=0"])], { + env: fx.env({ + ...NO_INHERITED_OWNER, + ...FRESH_SANDBOX_CONTEXT, + PATH: `${stubs}:${process.env.PATH}`, + TEST_SANDBOX_DISABLE: "0", + TEST_SANDBOX_REQUIRE: "1", + }), + helpers: [stubs], + stdio: ["ignore", "ignore", "pipe"], + }); + let stderr = ""; + wrapper.stderr?.on("data", (chunk: Buffer) => { + stderr += chunk.toString(); + }); + const started = Date.now(); + assert.deepEqual(await closed(wrapper), { code: 3, signal: null }); + assert.doesNotMatch(stderr, GROUP_STILL_LIVE_OUTPUT); + assert.ok(Date.now() - started < 3000, "the launcher must not wait out the publish grace period"); + assert.deepEqual(await readdir(fx.scratchParent), []); }); -test("a sidecar killed right after spawning the command does not orphan the command under a deleted scratch", async () => { - const runnerTemp = await temporaryParent(); - const work = await temporaryParent(); - const recordFile = join(work, "command.record"); - let commandPid: number | undefined; - try { - const wrapper = spawn( - process.execPath, - ["--import", "tsx", sandboxChildWrapperPath, killAfterSpawnSandboxChild, "sh", "-c", "exec sleep 300"], - { - env: { ...noCgroupEnv(runnerTemp), KILL_AFTER_SPAWN_RECORD: recordFile }, - stdio: ["ignore", "ignore", "ignore"], - } - ); - await closed(wrapper); - // Written by the sidecar before the kill, so it exists even if cleanup stops the command before it runs. - const [pid, root] = (await eventuallyRead(recordFile, 20_000)).trim().split(" "); - commandPid = Number(pid); - assert.ok(Number.isSafeInteger(commandPid) && commandPid > 0); - assert.ok(root); - // Either the scratch is kept, or the command is gone: never a live command with its scratch deleted. - if ( - !(await access(root) - .then(() => true) - .catch(() => false)) - ) { - await eventuallyAbsentProcess(commandPid); +test("a sidecar killed right after spawning the command does not orphan the command under a deleted scratch", async (t) => { + const fx = await lifecycleFixture(t); + const recordFile = fx.path("work", "command.record"); + await fx.dir("work"); + const wrapper = fx.spawn( + process.execPath, + ["--import", "tsx", sandboxChildWrapperPath, killAfterSpawnSandboxChild, "sh", "-c", "exec sleep 300"], + { + env: noCgroupEnv(fx, { KILL_AFTER_SPAWN_RECORD: recordFile }), + helpers: [recordFile], + stdio: ["ignore", "ignore", "ignore"], } - } finally { - killQuietly(commandPid); - await rm(runnerTemp, { force: true, recursive: true }); - await rm(work, { force: true, recursive: true }); + ); + await closed(wrapper); + // Written by the sidecar before the kill, so it exists even if cleanup stops the command before it runs. + const [pid, root] = (await eventuallyRead(recordFile, 20_000)).trim().split(" "); + const commandPid = Number(pid); + assert.ok(Number.isSafeInteger(commandPid) && commandPid > 0); + assert.ok(root); + // Either the scratch is kept, or the command is gone: never a live command with its scratch deleted. + if ( + !(await access(root) + .then(() => true) + .catch(() => false)) + ) { + await eventuallyAbsentProcess(commandPid); } }); -test("owner SIGKILL then a command that leaves a background child does not let recovery delete a live group's scratch", async () => { - const runnerTemp = await temporaryParent(); - const work = await temporaryParent(); - const pidFile = join(work, "background.pid"); - const releaseFile = join(work, "release"); - let backgroundPid: number | undefined; - let wrapper: ReturnType | undefined; - try { - wrapper = startRawWrapper( - [ - "sh", - "-c", - 'sleep 300 /dev/null 2>&1 & echo "$! $PDPP_TEST_SCRATCH_ROOT" > "$1"; while [ ! -e "$2" ]; do sleep 0.05; done', - "sh", - pidFile, - releaseFile, - ], - noCgroupEnv(runnerTemp) - ); - const [pid, root] = (await eventuallyRead(pidFile, 20_000)).trim().split(" "); - backgroundPid = Number(pid); - assert.ok(root); - const markerPath = join(root, ".pdpp-test-scratch.json"); - const marker = await eventuallyRunningMarker(markerPath); - const close = exited(wrapper); - wrapper.kill("SIGKILL"); - await close; - // The command now exits, leaving its background child behind; no owner is left to stop it. - await writeFile(releaseFile, "release\n"); - await eventuallyAbsentGroup(marker.pgid); - await writeFile( - markerPath, - `${JSON.stringify({ ...marker, created_at: new Date(Date.now() - 120_000).toISOString() })}\n` - ); - const results = await recoverStaleScratch({ parent: join(runnerTemp, "pdpp-test-scratch") }); - // Either recovery keeps the scratch, or nothing of the command is left alive in it. - if (results.some((entry) => entry.path === root && entry.removed)) { - await eventuallyAbsentProcess(backgroundPid); - } - } finally { - // Release the command's wait loop and stop an owner a failed assertion left running. - await writeFile(releaseFile, "release\n").catch(() => undefined); - if (wrapper?.pid && wrapper.exitCode === null && wrapper.signalCode === null) { - wrapper.kill("SIGKILL"); - } - killQuietly(backgroundPid); - await rm(runnerTemp, { force: true, recursive: true }); - await rm(work, { force: true, recursive: true }); +test("owner SIGKILL then a command that leaves a background child does not let recovery delete a live group's scratch", async (t) => { + const fx = await lifecycleFixture(t); + const pidFile = fx.path("work", "background.pid"); + const releaseFile = fx.path("work", "release"); + await fx.dir("work"); + const wrapper = startRawWrapper( + fx, + [ + "sh", + "-c", + 'sleep 300 /dev/null 2>&1 & echo "$! $PDPP_TEST_SCRATCH_ROOT" > "$1"; while [ ! -e "$2" ]; do sleep 0.05; done', + "sh", + pidFile, + releaseFile, + ], + noCgroupEnv(fx) + ); + const [pid, root] = (await eventuallyRead(pidFile, 20_000)).trim().split(" "); + const backgroundPid = Number(pid); + assert.ok(root); + const markerPath = join(root, ".pdpp-test-scratch.json"); + const marker = await eventuallyRunningMarker(markerPath); + const close = exited(wrapper); + wrapper.kill("SIGKILL"); + await close; + // The command now exits, leaving its background child behind; no owner is left to stop it. + await writeFile(releaseFile, "release\n"); + await eventuallyAbsentGroup(marker.pgid); + await writeFile( + markerPath, + `${JSON.stringify({ ...marker, created_at: new Date(Date.now() - 120_000).toISOString() })}\n` + ); + const results = await recoverStaleScratch({ parent: fx.scratchParent }); + // Either recovery keeps the scratch, or nothing of the command is left alive in it. + if (results.some((entry) => entry.path === root && entry.removed)) { + await eventuallyAbsentProcess(backgroundPid); } }); -test("a sandbox killed before its sidecar publishes does not let recovery delete the scratch of a command that starts later", async () => { - const runnerTemp = await temporaryParent(); - const parent = join(runnerTemp, "pdpp-test-scratch"); +test("a sandbox killed before its sidecar publishes does not let recovery delete the scratch of a command that starts later", async (t) => { + const fx = await lifecycleFixture(t); + const parent = fx.scratchParent; let sidecarPgid: number | undefined; - let groupPath: string | undefined; - let wrapper: ReturnType | undefined; try { - wrapper = spawn( + const wrapper = fx.spawn( process.execPath, [ "--import", @@ -1247,11 +1122,11 @@ test("a sandbox killed before its sidecar publishes does not let recovery delete "-c", 'echo $$ > "$PDPP_TEST_SCRATCH_ROOT/command.pid"; exec sleep 300', ], - { env: noCgroupEnv(runnerTemp), stdio: ["ignore", "ignore", "ignore"] } + { env: noCgroupEnv(fx), stdio: ["ignore", "ignore", "ignore"] } ); const wrapperClosed = closed(wrapper); const root = await eventuallyRunRoot(parent); - groupPath = join(root, ".sandbox-child-group"); + const groupPath = join(root, ".sandbox-child-group"); const markerPath = join(root, ".pdpp-test-scratch.json"); const marker = await eventuallyRunningMarker(markerPath); sidecarPgid = Number((await eventuallyRead(`${groupPath}.held`, 20_000)).trim()); @@ -1277,9 +1152,6 @@ test("a sandbox killed before its sidecar publishes does not let recovery delete await access(root); assert.ok(processAlive(commandPid)); } finally { - if (groupPath) { - await writeFile(`${groupPath}.release`, "release\n").catch(() => undefined); - } if (sidecarPgid) { try { process.kill(-sidecarPgid, "SIGKILL"); @@ -1287,10 +1159,6 @@ test("a sandbox killed before its sidecar publishes does not let recovery delete // The group may have exited before this failure-path cleanup. } } - if (wrapper?.pid && wrapper.exitCode === null && wrapper.signalCode === null) { - wrapper.kill("SIGKILL"); - } - await rm(runnerTemp, { force: true, recursive: true }); } }); @@ -1355,17 +1223,13 @@ test("recovery keeps a scratch whose published sidecar group is live although th } }); -test("SIGTERM starts bounded escalation before a TERM-ignoring direct child closes", async () => { +test("SIGTERM starts bounded escalation before a TERM-ignoring direct child closes", async (t) => { + const fx = await lifecycleFixture(t); let output = ""; - const wrapper = spawn( - process.execPath, - [ - "--import", - "tsx", - new URL("./run-command.ts", import.meta.url).pathname, - ...command(["--print-root", "--ignore-term", "--grandchild", "--grandchild-ignore-term", "--wait"]), - ], - { env: { ...process.env, TEST_SANDBOX_DISABLE: "1" }, stdio: ["ignore", "pipe", "ignore"] } + const wrapper = startWrapper( + fx, + ["--print-root", "--ignore-term", "--grandchild", "--grandchild-ignore-term", "--wait"], + fx.env({ TEST_SANDBOX_DISABLE: "1" }) ); wrapper.stdout?.on("data", (chunk: Buffer) => { output += chunk.toString(); @@ -1397,13 +1261,13 @@ test("SIGTERM starts bounded escalation before a TERM-ignoring direct child clos await assert.rejects(access(root)); }); -test("an intentionally detached descendant is outside the process-group containment contract", async () => { - const runnerTemp = await temporaryParent(); - const wrapper = startWrapper(["--print-root", "--escaped-grandchild", "--wait"], { - ...process.env, - RUNNER_TEMP: runnerTemp, - TEST_SANDBOX_DISABLE: "1", - }); +test("an intentionally detached descendant is outside the process-group containment contract", async (t) => { + const fx = await lifecycleFixture(t); + const wrapper = startWrapper( + fx, + ["--print-root", "--escaped-grandchild", "--wait"], + fx.env({ TEST_SANDBOX_DISABLE: "1" }) + ); let escapedPid: number | undefined; try { const root = await reportedRoot(wrapper); @@ -1423,7 +1287,6 @@ test("an intentionally detached descendant is outside the process-group containm } await eventuallyAbsentGroup(escapedPid); } - await rm(runnerTemp, { force: true, recursive: true }); } }); @@ -1437,131 +1300,83 @@ test("a signal latched while the running transition is durable is forwarded afte assert.deepEqual(result, { code: null, signal: "SIGTERM" }); }); -test("SIGTERM latched before recovery creates no allocation or child", async () => { - const runnerTemp = await temporaryParent(); - const parent = join(runnerTemp, "pdpp-test-scratch"); - const previousRunnerTemp = process.env.RUNNER_TEMP; - process.env.RUNNER_TEMP = runnerTemp; - try { - const result = await runScratchCommand(command(["--print-root"]), { - afterSignalHandlersInstalled: async () => { - process.kill(process.pid, "SIGTERM"); - await new Promise((resolve) => setTimeout(resolve, 25)); - }, - }); - assert.deepEqual(result, { code: null, signal: "SIGTERM" }); - assert.deepEqual(await readdir(parent), []); - } finally { - if (previousRunnerTemp === undefined) { - delete process.env.RUNNER_TEMP; - } else { - process.env.RUNNER_TEMP = previousRunnerTemp; - } - await rm(runnerTemp, { force: true, recursive: true }); - } -}); - -test("SIGTERM during allocation cleanup prevents spawn", async () => { - const runnerTemp = await temporaryParent(); - const parent = join(runnerTemp, "pdpp-test-scratch"); - const previousRunnerTemp = process.env.RUNNER_TEMP; - process.env.RUNNER_TEMP = runnerTemp; - try { - const result = await runScratchCommand(command(["--print-root"]), { - afterAllocationBeforeSpawn: async () => { - process.kill(process.pid, "SIGTERM"); - await new Promise((resolve) => setTimeout(resolve, 25)); - }, - }); - assert.deepEqual(result, { code: null, signal: "SIGTERM" }); - assert.deepEqual(await readdir(parent), []); - } finally { - if (previousRunnerTemp === undefined) { - delete process.env.RUNNER_TEMP; - } else { - process.env.RUNNER_TEMP = previousRunnerTemp; - } - await rm(runnerTemp, { force: true, recursive: true }); - } +test("SIGTERM latched before recovery creates no allocation or child", async (t) => { + const fx = await lifecycleFixture(t); + fx.useRunnerTempForThisTest(); + const result = await runScratchCommand(command(["--print-root"]), { + afterSignalHandlersInstalled: async () => { + process.kill(process.pid, "SIGTERM"); + await new Promise((resolve) => setTimeout(resolve, 25)); + }, + }); + assert.deepEqual(result, { code: null, signal: "SIGTERM" }); + assert.deepEqual(await readdir(fx.scratchParent), []); }); -test("SIGTERM during cleanup remains observable after the root is removed", async () => { - const runnerTemp = await temporaryParent(); - const parent = join(runnerTemp, "pdpp-test-scratch"); - const previousRunnerTemp = process.env.RUNNER_TEMP; - process.env.RUNNER_TEMP = runnerTemp; - try { - const result = await runScratchCommand(command(["--exit=0"]), { - cleanupHooks: { - afterJournal: async () => { - process.kill(process.pid, "SIGTERM"); - await new Promise((resolve) => setTimeout(resolve, 25)); - }, - }, - }); - assert.deepEqual(result, { code: null, signal: "SIGTERM" }); - assert.deepEqual(await readdir(parent), []); - } finally { - if (previousRunnerTemp === undefined) { - delete process.env.RUNNER_TEMP; - } else { - process.env.RUNNER_TEMP = previousRunnerTemp; - } - await rm(runnerTemp, { force: true, recursive: true }); - } +test("SIGTERM during allocation cleanup prevents spawn", async (t) => { + const fx = await lifecycleFixture(t); + fx.useRunnerTempForThisTest(); + const result = await runScratchCommand(command(["--print-root"]), { + afterAllocationBeforeSpawn: async () => { + process.kill(process.pid, "SIGTERM"); + await new Promise((resolve) => setTimeout(resolve, 25)); + }, + }); + assert.deepEqual(result, { code: null, signal: "SIGTERM" }); + assert.deepEqual(await readdir(fx.scratchParent), []); }); -test("a latched signal plus a pre-launch error cleans the provably allocated root", async () => { - const runnerTemp = await temporaryParent(); - const parent = join(runnerTemp, "pdpp-test-scratch"); - const previousRunnerTemp = process.env.RUNNER_TEMP; - process.env.RUNNER_TEMP = runnerTemp; - try { - const result = await runScratchCommand(command(["--print-root"]), { - afterAllocationBeforeSpawn: async () => { +test("SIGTERM during cleanup remains observable after the root is removed", async (t) => { + const fx = await lifecycleFixture(t); + fx.useRunnerTempForThisTest(); + const result = await runScratchCommand(command(["--exit=0"]), { + cleanupHooks: { + afterJournal: async () => { process.kill(process.pid, "SIGTERM"); await new Promise((resolve) => setTimeout(resolve, 25)); - throw new Error("simulate pre-launch failure"); }, - }); - assert.deepEqual(result, { code: null, signal: "SIGTERM" }); - assert.deepEqual(await readdir(parent), []); - } finally { - if (previousRunnerTemp === undefined) { - delete process.env.RUNNER_TEMP; - } else { - process.env.RUNNER_TEMP = previousRunnerTemp; - } - await rm(runnerTemp, { force: true, recursive: true }); - } + }, + }); + assert.deepEqual(result, { code: null, signal: "SIGTERM" }); + assert.deepEqual(await readdir(fx.scratchParent), []); }); -test("SIGKILL during spawn-before-running retains a launching root even after its group dies", async () => { - const runnerTemp = await temporaryParent(); - const parent = join(runnerTemp, "pdpp-test-scratch"); - const proc = startLaunchingWrapper({ ...process.env, RUNNER_TEMP: runnerTemp, TEST_SANDBOX_DISABLE: "1" }); - try { - const root = await reportedRoot(proc); - const markerPath = join(root, ".pdpp-test-scratch.json"); - const marker = JSON.parse(await readFile(markerPath, "utf8")) as { state: string }; - assert.equal(marker.state, "launching"); - const childPid = Number.parseInt(await eventuallyRead(join(root, "child.txt")), 10); - const close = exited(proc); - proc.kill("SIGKILL"); - assert.deepEqual(await close, { code: null, signal: "SIGKILL" }); - const now = Date.now() + 120_000; - assert.deepEqual(await recoverStaleScratch({ now, parent }), [ - { path: root, reason: "launch-unknown", removed: false }, - ]); - process.kill(-childPid, "SIGKILL"); - await eventuallyAbsentGroup(childPid); - assert.deepEqual(await recoverStaleScratch({ now: now + 120_000, parent }), [ - { path: root, reason: "launch-unknown", removed: false }, - ]); - await access(root); - } finally { - await rm(runnerTemp, { force: true, recursive: true }); - } +test("a latched signal plus a pre-launch error cleans the provably allocated root", async (t) => { + const fx = await lifecycleFixture(t); + fx.useRunnerTempForThisTest(); + const result = await runScratchCommand(command(["--print-root"]), { + afterAllocationBeforeSpawn: async () => { + process.kill(process.pid, "SIGTERM"); + await new Promise((resolve) => setTimeout(resolve, 25)); + throw new Error("simulate pre-launch failure"); + }, + }); + assert.deepEqual(result, { code: null, signal: "SIGTERM" }); + assert.deepEqual(await readdir(fx.scratchParent), []); +}); + +test("SIGKILL during spawn-before-running retains a launching root even after its group dies", async (t) => { + const fx = await lifecycleFixture(t); + const parent = fx.scratchParent; + const proc = startLaunchingWrapper(fx, fx.env({ TEST_SANDBOX_DISABLE: "1" })); + const root = await reportedRoot(proc); + const markerPath = join(root, ".pdpp-test-scratch.json"); + const marker = JSON.parse(await readFile(markerPath, "utf8")) as { state: string }; + assert.equal(marker.state, "launching"); + const childPid = Number.parseInt(await eventuallyRead(join(root, "child.txt")), 10); + const close = exited(proc); + proc.kill("SIGKILL"); + assert.deepEqual(await close, { code: null, signal: "SIGKILL" }); + const now = Date.now() + 120_000; + assert.deepEqual(await recoverStaleScratch({ now, parent }), [ + { path: root, reason: "launch-unknown", removed: false }, + ]); + process.kill(-childPid, "SIGKILL"); + await eventuallyAbsentGroup(childPid); + assert.deepEqual(await recoverStaleScratch({ now: now + 120_000, parent }), [ + { path: root, reason: "launch-unknown", removed: false }, + ]); + await access(root); }); test("journal recovery resumes an interrupted rename and survives embedded-marker removal", async () => { @@ -1639,34 +1454,24 @@ test("a malformed nonempty boot identity cannot bypass the live-group proof", as } }); -test("recovery budgets bound inventory inspection and still permit the new owner", async () => { - const runnerTemp = await temporaryParent(); - const parent = join(runnerTemp, "pdpp-test-scratch"); - const previousRunnerTemp = process.env.RUNNER_TEMP; - process.env.RUNNER_TEMP = runnerTemp; - try { - await mkdir(parent, { mode: 0o700 }); - await Promise.all(Array.from({ length: 12 }, (_, index) => writeFile(join(parent, `foreign-${index}`), "defer"))); - const inspected: string[] = []; - const recovered = await recoverStaleScratch({ - hooks: { onInspect: (path) => inspected.push(path) }, - limits: { maxInspectedEntries: 3 }, - parent, - }); - assert.ok(inspected.length <= 3); - assert.deepEqual(recovered.at(-1), { path: parent, reason: "recovery-budget-exhausted", removed: false }); - assert.deepEqual(await runScratchCommand(command(["--exit=0"]), { recoveryLimits: { maxInspectedEntries: 1 } }), { - code: 0, - signal: null, - }); - } finally { - if (previousRunnerTemp === undefined) { - delete process.env.RUNNER_TEMP; - } else { - process.env.RUNNER_TEMP = previousRunnerTemp; - } - await rm(runnerTemp, { force: true, recursive: true }); - } +test("recovery budgets bound inventory inspection and still permit the new owner", async (t) => { + const fx = await lifecycleFixture(t); + const parent = fx.scratchParent; + fx.useRunnerTempForThisTest(); + await mkdir(parent, { mode: 0o700 }); + await Promise.all(Array.from({ length: 12 }, (_, index) => writeFile(join(parent, `foreign-${index}`), "defer"))); + const inspected: string[] = []; + const recovered = await recoverStaleScratch({ + hooks: { onInspect: (path) => inspected.push(path) }, + limits: { maxInspectedEntries: 3 }, + parent, + }); + assert.ok(inspected.length <= 3); + assert.deepEqual(recovered.at(-1), { path: parent, reason: "recovery-budget-exhausted", removed: false }); + assert.deepEqual(await runScratchCommand(command(["--exit=0"]), { recoveryLimits: { maxInspectedEntries: 1 } }), { + code: 0, + signal: null, + }); }); test("a durable lexical cursor eventually reaches a stale candidate beyond an initial recovery page", async () => { @@ -1755,15 +1560,15 @@ test("startup recovery quarantines a large verified root when recursive removal } }); -test("SIGKILL leaves a live orphan, then a later recovery removes its dead group", async () => { - const runnerTemp = await temporaryParent(); - const sibling = join(runnerTemp, "sibling-sentinel"); +test("SIGKILL leaves a live orphan, then a later recovery removes its dead group", async (t) => { + const fx = await lifecycleFixture(t); + const sibling = fx.path("sibling-sentinel"); await writeFile(sibling, "unchanged"); - const proc = startWrapper(["--print-root", "--grandchild", "--grandchild-ignore-term", "--wait"], { - ...process.env, - RUNNER_TEMP: runnerTemp, - TEST_SANDBOX_DISABLE: "1", - }); + const proc = startWrapper( + fx, + ["--print-root", "--grandchild", "--grandchild-ignore-term", "--wait"], + fx.env({ TEST_SANDBOX_DISABLE: "1" }) + ); const root = await reportedRoot(proc); const markerPath = join(root, ".pdpp-test-scratch.json"); const marker = await eventuallyRunningMarker(markerPath); @@ -1778,7 +1583,7 @@ test("SIGKILL leaves a live orphan, then a later recovery removes its dead group markerPath, `${JSON.stringify({ ...marker, created_at: new Date(Date.now() - 120_000).toISOString() })}\n` ); - const parent = join(runnerTemp, "pdpp-test-scratch"); + const parent = fx.scratchParent; assert.deepEqual(await recoverStaleScratch({ parent }), [{ path: root, reason: "group-live", removed: false }]); assert.equal(await readFile(join(root, "child-root.txt"), "utf8"), `${root}\n`); assert.equal(await readFile(sibling, "utf8"), "unchanged"); @@ -1787,7 +1592,6 @@ test("SIGKILL leaves a live orphan, then a later recovery removes its dead group await eventuallyRecover(parent, root); await assert.rejects(access(root)); assert.equal(await readFile(sibling, "utf8"), "unchanged"); - await rm(runnerTemp, { force: true, recursive: true }); }); test("recovery has stable fail-closed classifications for every stale candidate state", async () => { @@ -1904,11 +1708,11 @@ test("wrong-owner candidates have a stable conservative recovery reason", async } }); -test("parallel outer owners isolate participant worker trees under one inherited root each", async () => { - const runnerTemp = await temporaryParent(); - const env = { ...process.env, RUNNER_TEMP: runnerTemp, TEST_SANDBOX_DISABLE: "1" }; - const first = startWrapper(["--print-root", "--nested-participant", "--wait"], env); - const second = startWrapper(["--print-root", "--nested-participant", "--wait"], env); +test("parallel outer owners isolate participant worker trees under one inherited root each", async (t) => { + const fx = await lifecycleFixture(t); + const env = fx.env({ TEST_SANDBOX_DISABLE: "1" }); + const first = startWrapper(fx, ["--print-root", "--nested-participant", "--wait"], env); + const second = startWrapper(fx, ["--print-root", "--nested-participant", "--wait"], env); const [firstRoot, secondRoot] = await Promise.all([reportedRoot(first), reportedRoot(second)]); try { assert.notEqual(await realpath(firstRoot), await realpath(secondRoot)); @@ -1947,6 +1751,120 @@ test("parallel outer owners isolate participant worker trees under one inherited assert.deepEqual(await secondClose, { code: null, signal: "SIGTERM" }); await assert.rejects(access(firstRoot)); await assert.rejects(access(secondRoot)); - await rm(runnerTemp, { force: true, recursive: true }); } }); + +// The tests below protect the fixture that every launch above depends on. + +const OUTSIDE_BOUND_ROOT = /outside the bound runner temp/; +const RUNNER_TEMP_MISMATCH = /RUNNER_TEMP must be/; +const DELIBERATE_FAILURE = /deliberate failure while an orphaned command holds/; +const SETUP_FAILED = /setup failed/; + +test("a helper outside the bound runner temp is rejected before the command is launched", async (t) => { + const fx = await lifecycleFixture(t); + const sibling = await mkdtemp(join(tmpdir(), "pdpp-test-scratch-test-sibling-")); + t.after(() => rm(sibling, { force: true, recursive: true })); + assert.throws(() => fx.spawn("true", [], { helpers: [join(sibling, "f")] }), { message: OUTSIDE_BOUND_ROOT }); + assert.throws(() => fx.path("..", "elsewhere"), { message: OUTSIDE_BOUND_ROOT }); + assert.throws(() => fx.spawn("true", [], { env: { ...process.env, RUNNER_TEMP: sibling } }), { + message: RUNNER_TEMP_MISMATCH, + }); +}); + +test("a helper made with the fixture is readable through the sandboxed wrapper below /tmp", async (t) => { + if (process.env.TEST_SANDBOX_ACTIVE) { + t.skip("bind-back isolation is gated by the lifecycle job; bwrap cannot nest inside the outer sandbox"); + return; + } + // A broken /tmp bind-back fails here. TEST_SANDBOX_REQUIRE=1 makes a host without a usable bubblewrap fail + // instead of running the command on the unisolated host /tmp. + const fx = await lifecycleFixture(t, { under: "/tmp" }); + const record = join(await fx.dir("work"), "record"); + await writeFile(record, "from the fixture\n"); + const reader = fx.spawn(process.execPath, ["--import", "tsx", wrapperPath, "--", "cat", record], { + env: fx.env({ + ...NO_INHERITED_OWNER, + ...FRESH_SANDBOX_CONTEXT, + TEST_SANDBOX_DISABLE: "0", + TEST_SANDBOX_REQUIRE: "1", + }), + helpers: [record], + stdio: ["ignore", "pipe", "pipe"], + }); + let output = ""; + reader.stdout?.on("data", (chunk: Buffer) => { + output += chunk.toString(); + }); + assert.deepEqual(await closed(reader), { code: 0, signal: null }); + assert.equal(output, "from the fixture\n"); +}); + +test("fixture teardown kills what the fixture launched and leaves a process it did not launch alone", { + skip: process.platform !== "linux", +}, async () => { + const hooks: Array<() => Promise | void> = []; + const fx = await lifecycleFixture({ + after: (hook: () => Promise | void) => hooks.push(hook), + } as unknown as TestContext); + // Neither has a marker or a registered id: the fixture knows only the environment it gave the first. + const launched = fx.spawn("sh", ["-c", "exec sleep 300"], { detached: true }); + const stranger = spawn("sleep", ["300"], { + detached: true, + env: { ...process.env, RUNNER_TEMP: "/not/a/fixture" }, + stdio: "ignore", + }); + try { + assert.ok(launched.pid && stranger.pid); + for (const hook of hooks) { + // biome-ignore lint/performance/noAwaitInLoops: teardown hooks run in registration order. + await hook(); + } + await eventuallyAbsentGroup(launched.pid); + assert.equal(launched.stdout?.destroyed, true); + await assert.rejects(access(fx.runnerTemp)); + process.kill(-stranger.pid, 0); + } finally { + killQuietly(stranger.pid); + } +}); + +for (const sandboxDisabled of ["0", "1"]) { + test(`a failed assertion ends the test run and leaves no command process behind (TEST_SANDBOX_DISABLE=${sandboxDisabled})`, { + skip: process.platform !== "linux", + }, async (t) => { + const fx = await lifecycleFixture(t); + const pidsOut = fx.path("pids"); + // Before the fixture reaped what a test left behind, this run never exited. + const run = fx.spawn(process.execPath, ["--import", "tsx", "--test", leakedGroupAfterFailure], { + env: fx.env({ + ...NO_INHERITED_OWNER, + ...FRESH_SANDBOX_CONTEXT, + PIDS_OUT: pidsOut, + // The outer test runner's context would make the child run skip its test file. + NODE_TEST_CONTEXT: undefined, + TEST_SANDBOX_DISABLE: sandboxDisabled, + }), + helpers: [pidsOut], + killSignal: "SIGKILL", + stdio: ["ignore", "pipe", "pipe"], + timeout: 60_000, + }); + let output = ""; + for (const stream of [run.stdout, run.stderr]) { + stream?.on("data", (chunk: Buffer) => { + output += chunk.toString(); + }); + } + assert.deepEqual(await closed(run), { code: 1, signal: null }, output); + // The deliberate failure, not a setup failure, ended the run. + assert.doesNotMatch(output, SETUP_FAILED); + assert.match(output, DELIBERATE_FAILURE); + await Promise.all( + (await readFile(pidsOut, "utf8")) + .trim() + .split(" ") + .map((pid) => eventuallyAbsentProcess(Number(pid))) + ); + }); +} diff --git a/test-accounting.manifest.json b/test-accounting.manifest.json index 4efdfcce4..ffbfb15a2 100644 --- a/test-accounting.manifest.json +++ b/test-accounting.manifest.json @@ -44,7 +44,9 @@ { "id": "default", "required": true, - "skip_reasons": {} + "skip_reasons": { + "bind-back isolation is gated by the lifecycle job; bwrap cannot nest inside the outer sandbox": 1 + } } ], "environment_unset": [