From f01e627b84c687befae5a8c453b78458cafdf129 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 20 Oct 2026 01:03:39 -0400 Subject: [PATCH] fix(run-claude-cli): reap every leg-1 process before the next leg starts After each leg, list every process whose real uid is the runner user's and that started after the launcher (excluding the launcher and its ancestors), stop and kill them, and start the next leg only once a listing comes back empty. A survivor fails the run. The listing does not depend on process group, session or parent, so it covers &, nohup, setsid and double forks. Closes #799 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Q6B1MWhz73WFRYyyrKjtjY --- .github/actions/run-claude-cli/lane.cjs | 91 +++++++++- .github/actions/run-claude-cli/lane.test.cjs | 177 ++++++++++++++++++- README.md | 6 +- 3 files changed, 269 insertions(+), 5 deletions(-) diff --git a/.github/actions/run-claude-cli/lane.cjs b/.github/actions/run-claude-cli/lane.cjs index 8a36a2d0..5db77015 100644 --- a/.github/actions/run-claude-cli/lane.cjs +++ b/.github/actions/run-claude-cli/lane.cjs @@ -35,6 +35,9 @@ // - The CLI runs in --restricted mode: code-running tools only when the // tools input names them, no user, project or local settings, file tools // confined to the working directories, no bypassPermissions. +// - After each leg, every process of the runner user started after this +// one (legProcesses) is stopped and killed, and the next leg starts only +// once a listing finds none; any survivor fails the run. // - The subscription leg runs only when the key leg is a credit stop // (classify.cjs isCreditStop), judged from structured fields. // - Secret names are derived from the role and never taken from input. @@ -621,14 +624,89 @@ function restoreModes(modes) { } } +// One process's state, parent and start time (clock ticks after boot) from +// /proc//stat; the fields after the command name start at field 3. +function procStat(pid, procRoot = "/proc") { + const text = fs.readFileSync(path.join(procRoot, String(pid), "stat"), "utf8"); + const fields = text.slice(text.lastIndexOf(")") + 2).split(" "); + return { + state: fields[0], + ppid: Number(fields[1]), + start: BigInt(fields[19]), + }; +} + +// Every live process whose real uid is `uid` and that started no earlier than +// `since`, other than this process and its ancestors. Neither a new session (setsid) nor a new parent (a +// double fork, nohup, `&`) changes a process's uid or start time, so this +// finds whatever a leg left running however it detached. +function legProcesses({ + since, + uid = process.getuid(), + self = process.pid, + procRoot = "/proc", +}) { + const ancestors = new Set(); + for (let pid = self; pid > 0 && !ancestors.has(pid); ) { + ancestors.add(pid); + pid = procStat(pid, procRoot).ppid; + } + const found = []; + for (const name of fs.readdirSync(procRoot)) { + if (!/^\d+$/u.test(name) || ancestors.has(Number(name))) continue; + try { + const stat = procStat(name, procRoot); + if (stat.state === "Z" || stat.state === "X" || stat.start < since) + continue; + // Only a privileged process can change its real uid. + const realUid = fs + .readFileSync(path.join(procRoot, name, "status"), "utf8") + .match(/^Uid:\s+(\d+)/mu)[1]; + if (Number(realUid) === uid) found.push(Number(name)); + } catch (error) { + // The process ended while it was read. + if (error.code !== "ENOENT" && error.code !== "ESRCH") throw error; + } + } + return found; +} + +// Stops, then kills, every process `list` returns, until a listing comes back +// empty or the rounds run out. Stopping first keeps a process from forking +// again before it is killed. Returns the processes still alive: an empty list +// confirms the leg left nothing running. +function reapLeg({ list, kill = process.kill, rounds = 50, pauseMs = 20 }) { + for (let round = 0; round < rounds; round += 1) { + const pids = list(); + if (pids.length === 0) return []; + for (const signal of ["SIGSTOP", "SIGKILL"]) { + for (const pid of pids) { + try { + kill(pid, signal); + } catch { + // Gone already, or not ours to kill: the next listing decides. + } + } + } + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, pauseMs); + } + return list(); +} + function runLegs( config, { baseEnv = process.env, spawn = spawnSync, now = () => new Date(), + listLegProcesses, + kill = process.kill, } = {}, ) { + // Anything of the runner user's started after this process is a leg's. + const list = + listLegProcesses ?? + (() => legProcesses({ since: procStat(process.pid).start })); const prompt = promptText(config); const active = config.legs.filter((leg) => !leg.skipped); const records = []; @@ -655,6 +733,12 @@ function runLegs( killSignal: "SIGINT", }); const endedAt = now().toISOString(); + const survivors = reapLeg({ list, kill }); + if (survivors.length > 0) { + throw new LaneError( + `leg ${index + 1} left ${survivors.length} process(es) running after it ended; no further leg runs`, + ); + } const timedOut = child.error?.code === "ETIMEDOUT"; const overflowed = child.error?.code === "ENOBUFS"; const exitCode = typeof child.status === "number" ? child.status : null; @@ -901,7 +985,7 @@ function install(env, { spawn = spawnSync } = {}) { }); } -function run(env) { +function run(env, legOptions = {}) { verifyEnvironment(env); const config = readConfig(env); for (const line of skipWarnings(config)) process.stdout.write(`${line}\n`); @@ -909,7 +993,7 @@ function run(env) { process.stdout.write(`::stop-commands::${token}\n`); let outcome; try { - outcome = runLegs(config, { baseEnv: env }); + outcome = runLegs(config, { ...legOptions, baseEnv: env }); } finally { process.stdout.write(`::${token}::\n`); } @@ -975,6 +1059,9 @@ module.exports = { install, judgeLeg, legEnv, + legProcesses, + procStat, + reapLeg, legRecord, linesToMessages, readConfig, diff --git a/.github/actions/run-claude-cli/lane.test.cjs b/.github/actions/run-claude-cli/lane.test.cjs index adcb59b4..d0c0f115 100644 --- a/.github/actions/run-claude-cli/lane.test.cjs +++ b/.github/actions/run-claude-cli/lane.test.cjs @@ -27,6 +27,8 @@ const { holdsCredential, install, legEnv, + legProcesses, + procStat, readConfig, resolveCliVersion, run, @@ -77,6 +79,20 @@ fs.appendFileSync(process.env.FAKE_CALLS, JSON.stringify({ leg, seen, fileComman wait: process.env.CLAUDE_CODE_RETRY_WATCHDOG_MAX_WAIT_MS, args: process.argv.slice(2), stdin: fs.readFileSync(0, "utf8") }) + "\\n"); +// FAKE_DETACH: leave one process running per way of detaching, each writing +// its own pid to a file in that directory first. +if (process.env.FAKE_DETACH && leg === "KEY") { + const dir = process.env.FAKE_DETACH; + require("node:child_process").spawnSync("sh", ["-c", [ + \`sleep 60 & echo $! > \${dir}/amp\`, + \`nohup sleep 60 > /dev/null 2>&1 & echo $! > \${dir}/nohup\`, + \`setsid -f sh -c 'echo $$ > \${dir}/setsid; exec sleep 60' > /dev/null 2>&1\`, + \`(sh -c 'echo $$ > \${dir}/double; exec sleep 60' > /dev/null 2>&1 &)\`, + ].join("\\n")], { stdio: "ignore" }); + for (let i = 0; i < 250 && fs.readdirSync(dir).length < 4; i += 1) { + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 20); + } +} if (process.env["FAKE_" + leg + "_SLEEP"]) { Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 5000); if (process.env.FAKE_SURVIVED) fs.writeFileSync(process.env.FAKE_SURVIVED, ""); @@ -89,6 +105,33 @@ fs.chmodSync(fakeCli, 0o755); let caseNumber = 0; +// legProcesses as runLegs lists them, narrowed to the processes whose +// environment holds this case's marker, so a test never signals a process +// another test or the user started meanwhile. +const testStart = procStat(process.pid).start; +function caseProcesses(marker) { + return () => + legProcesses({ since: testStart }).filter((pid) => { + try { + return fs + .readFileSync(`/proc/${pid}/environ`, "utf8") + .split("\0") + .includes(marker); + } catch { + return false; + } + }); +} + +// True while the process runs; a zombie has already ended. +function alive(pid) { + try { + return !["Z", "X"].includes(procStat(pid).state); + } catch { + return false; + } +} + // Runs what follows `--` when the legs ask for bwrap, so cases with a tree to // protect test the mode lock on hosts without bubblewrap. function withoutBwrap(command, args, options) { @@ -226,6 +269,7 @@ function runCase({ extraEnv = {}, prepare = () => ({}), spawn = withoutBwrap, + kill = process.kill, }) { caseNumber += 1; const dir = path.join(root, `case-${caseNumber}`); @@ -289,7 +333,12 @@ function runCase({ workDir: dir, runId: "123", }; - const outcome = runLegs(config, { baseEnv, spawn }); + const outcome = runLegs(config, { + baseEnv, + spawn, + kill, + listLegProcesses: caseProcesses(`FAKE_CALLS=${calls}`), + }); const callLines = fs.existsSync(calls) ? fs.readFileSync(calls, "utf8").trim().split("\n").filter(Boolean) : []; @@ -691,6 +740,127 @@ test("a CLI under bwrap does not outlive a timed-out leg", withBwrap, () => { assert.equal(fs.existsSync(survived), false); }); +const DETACHED = ["amp", "nohup", "setsid", "double"]; + +// The pid each detached process wrote for itself, by way of detaching. +function detachedPids(dir) { + return Object.fromEntries( + DETACHED.map((name) => [ + name, + Number(fs.readFileSync(path.join(dir, name), "utf8")), + ]), + ); +} + +// Cleanup when a case fails: kills whatever detached process is left. +function killAll(dir) { + for (const pid of Object.values(detachedPids(dir))) { + try { + process.kill(pid, "SIGKILL"); + } catch { + // Already gone. + } + } +} + +test("legProcesses finds a process left by &, nohup, setsid and a double fork, never this process or its parent", () => { + const dir = fs.mkdtempSync(path.join(root, "detach-")); + const since = procStat(process.pid).start; + fs.writeFileSync(`${dir}.jsonl`, ""); + spawnSync(fakeCli, [], { + env: { + ...process.env, + ANTHROPIC_API_KEY: "x", + FAKE_DETACH: dir, + FAKE_CALLS: `${dir}.calls`, + FAKE_KEY: `${dir}.jsonl`, + }, + input: "", + }); + const pids = detachedPids(dir); + try { + const found = legProcesses({ since }); + for (const [name, pid] of Object.entries(pids)) { + assert.ok(found.includes(pid), `${name} pid ${pid} not listed`); + } + assert.equal(found.includes(process.pid), false); + assert.equal(found.includes(process.ppid), false); + } finally { + killAll(dir); + } +}); + +test("what leg 1 detaches by &, nohup, setsid or a double fork is dead before leg 2 starts", () => { + let detach; + let atLeg2 = null; + const { calls, outputs } = runCase({ + key: CREDIT_STOP, + sub: SUCCESS, + prepare: (dir) => { + detach = path.join(dir, "detached"); + fs.mkdirSync(detach); + return { FAKE_DETACH: detach }; + }, + spawn: (command, args, options) => { + if (options.env.CLAUDE_CODE_OAUTH_TOKEN) { + atLeg2 = Object.fromEntries( + Object.entries(detachedPids(detach)).map(([name, pid]) => [ + name, + alive(pid), + ]), + ); + } + return withoutBwrap(command, args, options); + }, + }); + try { + assert.deepEqual(atLeg2, { + amp: false, + nohup: false, + setsid: false, + double: false, + }); + assert.equal(calls.length, 2); + assert.equal(outputs["lane-class"], ""); + } finally { + killAll(detach); + } +}); + +test("a detached process that cannot be killed fails the run before leg 2", () => { + let detach; + let dir; + try { + assert.throws( + () => + runCase({ + key: CREDIT_STOP, + sub: SUCCESS, + prepare: (caseDir) => { + dir = caseDir; + detach = path.join(caseDir, "detached"); + fs.mkdirSync(detach); + return { FAKE_DETACH: detach }; + }, + kill: () => {}, + }), + (error) => + error instanceof LaneError && + /^leg 1 left 4 process\(es\) running after it ended; no further leg runs$/u.test( + error.message, + ), + ); + const legs = fs + .readFileSync(path.join(dir, "calls.jsonl"), "utf8") + .trim() + .split("\n") + .map((line) => JSON.parse(line).leg); + assert.deepEqual(legs, ["KEY"]); + } finally { + killAll(detach); + } +}); + test("the install tree is writable again when a leg throws", asNonRoot, () => { let installed; assert.throws(() => @@ -1051,7 +1221,10 @@ test("run writes the outputs, the log and the summary, and fails on a lane class FAKE_SUB_EXIT: "1", }; assert.throws( - () => run(env), + () => + run(env, { + listLegProcesses: caseProcesses(`FAKE_CALLS=${env.FAKE_CALLS}`), + }), /intake ended class=rate-limit on AUTOMATIONS_CLAUDE_CODE_OAUTH_TOKEN/u, ); const outputs = Object.fromEntries( diff --git a/README.md b/README.md index 57a478d6..d14dd548 100644 --- a/README.md +++ b/README.md @@ -563,7 +563,11 @@ consumer to audit it. `GITHUB_PATH`, `GITHUB_OUTPUT`, `GITHUB_STEP_SUMMARY` or `GITHUB_STATE` file is cut off after the last leg, before the action writes its own outputs, and the installed CLI tree and `_work/_actions` have their write - bits removed while the legs run. Code a leg runs still runs as the runner + bits removed while the legs run. After each leg, every process of the + runner user started after the launcher is stopped and killed, whether it + detached with `&`, `nohup`, `setsid` or a double fork, and the next leg + starts only once none is left; a process that survives fails the step. + Code a leg runs still runs as the runner user, so a lane that gives the model unrestricted Bash, or any code-running tool beyond fixed read-only command rules, runs the action in a job that holds no other