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