This repository has no description
fix(restart): scrub the restarter's bus-identity env from the re-exec (#75)
Incident follow-up #3 — the deeper root cause. `pty restart` (and the dead-
session "Restart? [Y/n]" path) re-run a session's stored command under the
RESTARTER's shell environment. When cos was restarted from smalltalk's shell,
that shell's ST_AGENT=smalltalk-claude (and ST_ROOT) leaked into the re-exec,
so cos came back under the wrong bus identity and died (exit 129).
Strip the bus-identity vars (ST_AGENT/ST_ROOT) from an operator-initiated
restart's environment so a session re-exec'd from a different shell can never
inherit that shell's identity. Mechanism: a new `scrubEnv?: string[]` on
spawnDaemon deletes the named keys from the daemon's env before it spawns — and
therefore before the session child inherits them (spawnViaNode path). The two
operator-restart call sites (cmdRestart, handleDeadSession) pass
RESTART_SCRUBBED_ENV.
Scoped deliberately to restart: a fresh `pty run` is unaffected, because a
convoy-launched create legitimately inherits its own identity. And this keeps
`pty restart` appropriately un-blessed for agents (consistent with the #73
guardrail): it's now safe — it won't resurrect a session under the wrong
identity — but the correct way to restart an agent with a valid identity is
still its supervisor (convoy). pty itself reads neither var, so scrubbing them
changes only what the child inherits.
tests/restart-env-scrub.test.ts: a restart run from a shell carrying
ST_AGENT=smalltalk-claude / ST_ROOT=/leaked yields a child that records
UNSET|UNSET (identity scrubbed, not leaked); a fresh `pty run` still inherits
its creator's ST_AGENT/ST_ROOT (create path unaffected).
+119
+14
src/cli.ts
+14
src/cli.ts
···
1473
1473
await spawnDaemon({
1474
1474
name: session.name, command: meta.command, args: meta.args, displayCommand: meta.displayCommand, cwd: meta.cwd, tags: meta.tags,
1475
1475
...(meta.displayName ? { displayName: meta.displayName } : {}),
1476
+
scrubEnv: RESTART_SCRUBBED_ENV,
1476
1477
});
1477
1478
console.log(`Session "${session.name}" restarted.`);
1478
1479
doAttach(session.name);
···
2999
3000
return null;
3000
3001
}
3001
3002
3003
+
/** Bus-identity env vars stripped from an operator-initiated restart's re-exec.
3004
+
* `pty restart` (and the dead-session "Restart? [Y/n]" path) re-run a stored
3005
+
* command under the RESTARTER's shell env. If that shell belongs to a different
3006
+
* convoy agent, its ST_AGENT/ST_ROOT leak into the re-exec'd session and it
3007
+
* comes back under the wrong bus identity — the cos-restart incident, where
3008
+
* restarting cos from smalltalk's shell brought cos back as smalltalk-claude
3009
+
* (exit 129). Scrubbing them means a restarted session never inherits the
3010
+
* restarter's identity; the blessed way to restart an agent with a correct
3011
+
* identity is its supervisor (convoy). Fresh `pty run` is unaffected — a
3012
+
* convoy-launched create legitimately inherits its own identity. */
3013
+
const RESTART_SCRUBBED_ENV = ["ST_AGENT", "ST_ROOT"];
3014
+
3002
3015
async function cmdRestart(
3003
3016
name: string,
3004
3017
yes = false,
···
3062
3075
await spawnDaemon({
3063
3076
name, command: meta.command, args: meta.args, displayCommand: meta.displayCommand, cwd: meta.cwd, tags: restartTags,
3064
3077
...(meta.displayName ? { displayName: meta.displayName } : {}),
3078
+
scrubEnv: RESTART_SCRUBBED_ENV,
3065
3079
});
3066
3080
console.log(`Session "${name}" restarted.`);
3067
3081
+14
src/spawn.ts
+14
src/spawn.ts
···
84
84
* handler still surfaces immediate failures within milliseconds, so
85
85
* this only governs the "alive but slow" case. */
86
86
startTimeoutMs?: number;
87
+
/** Env var names to DELETE from the daemon's inherited environment before it
88
+
* spawns — and therefore before the session child inherits it. Used by the
89
+
* operator-initiated restart paths to strip the *restarter's* ambient
90
+
* bus-identity vars (ST_AGENT/ST_ROOT), so a session re-exec'd from a
91
+
* different shell can't come back under that shell's identity. See the
92
+
* cos-restart incident. Applied on the spawnViaNode path; the CLI-fallback
93
+
* path can't express it (a bundled consumer that hits the fallback also
94
+
* isn't the operator-restart context this guards). */
95
+
scrubEnv?: string[];
87
96
}
88
97
89
98
/** Default time we wait for a daemon's Unix socket to appear after
···
151
160
// when its spawner is gone. Off by default — opt in via
152
161
// `bindToSpawnerLifetime` when the caller owns the daemon's lifetime.
153
162
const env: Record<string, string> = { ...process.env, PTY_SERVER_CONFIG: config };
163
+
// Strip caller-requested vars (e.g. the restarter's leaked bus identity)
164
+
// before the daemon — and thus the session child — can inherit them.
165
+
if (options.scrubEnv) {
166
+
for (const key of options.scrubEnv) delete env[key];
167
+
}
154
168
if (options.bindToSpawnerLifetime) env.PTY_SPAWNER_PID = String(process.pid);
155
169
const child = spawn(launcherCmd, [...launcherArgs, serverModule], {
156
170
detached: true,
+91
tests/restart-env-scrub.test.ts
+91
tests/restart-env-scrub.test.ts
···
1
+
import { describe, it, expect, afterAll } from "vitest";
2
+
import * as fs from "node:fs";
3
+
import * as os from "node:os";
4
+
import * as path from "node:path";
5
+
import { fileURLToPath } from "node:url";
6
+
import { spawnSync } from "node:child_process";
7
+
8
+
const __dirname = path.dirname(fileURLToPath(import.meta.url));
9
+
const nodeBin = process.execPath;
10
+
const cliPath = path.join(__dirname, "..", "dist", "cli.js");
11
+
const testRoot = fs.mkdtempSync(path.join(os.tmpdir(), "pty-envscrub-"));
12
+
const bgPids: number[] = [];
13
+
afterAll(() => {
14
+
for (const pid of bgPids) { try { process.kill(pid, "SIGKILL"); } catch {} }
15
+
fs.rmSync(testRoot, { recursive: true, force: true, maxRetries: 3, retryDelay: 100 });
16
+
});
17
+
18
+
interface RunOpts { env?: Record<string, string>; unset?: string[]; timeout?: number }
19
+
20
+
// PTY_SESSION set => restart takes its "already inside a session, not attaching"
21
+
// branch and returns instead of hanging on a non-TTY attach.
22
+
function runCli(dir: string, args: string[], opts: RunOpts = {}) {
23
+
const env: Record<string, string> = {
24
+
...(process.env as Record<string, string>),
25
+
PTY_SESSION_DIR: dir, PTY_ROOT_LEGACY_SILENT: "1", PTY_SESSION: "outer",
26
+
...(opts.env ?? {}),
27
+
};
28
+
for (const k of opts.unset ?? []) delete env[k];
29
+
return spawnSync(nodeBin, [cliPath, ...args], { env, encoding: "utf8", timeout: opts.timeout ?? 15000 });
30
+
}
31
+
32
+
/** A command that records the ST_AGENT/ST_ROOT it was actually launched with,
33
+
* then stays alive. `-` default => "UNSET" when the var is absent. Re-runs on
34
+
* every (re)start, so the file always reflects the current child's env. */
35
+
function recorderCmd(outFile: string): string[] {
36
+
return ["sh", "-c", `printf '%s|%s' "\${ST_AGENT-UNSET}" "\${ST_ROOT-UNSET}" > "${outFile}"; exec sleep 300`];
37
+
}
38
+
39
+
function createSession(dir: string, name: string, outFile: string, opts: RunOpts): void {
40
+
const r = runCli(dir, ["run", "-d", "--id", name, "--", ...recorderCmd(outFile)], opts);
41
+
expect(r.status).toBe(0);
42
+
try {
43
+
bgPids.push(Number(fs.readFileSync(path.join(dir, `${name}.pid`), "utf8").trim()));
44
+
} catch {}
45
+
}
46
+
47
+
const sleepSync = (ms: number) => Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms);
48
+
49
+
function waitForContent(p: string, timeoutMs = 4000): string {
50
+
const start = Date.now();
51
+
while (Date.now() - start < timeoutMs) {
52
+
try {
53
+
const s = fs.readFileSync(p, "utf8");
54
+
if (s.length > 0) return s;
55
+
} catch {}
56
+
sleepSync(50);
57
+
}
58
+
try { return fs.readFileSync(p, "utf8"); } catch { return ""; }
59
+
}
60
+
61
+
describe("restart scrubs the restarter's bus-identity env", () => {
62
+
it("does NOT leak the restarter's ST_AGENT/ST_ROOT into the re-exec'd session", () => {
63
+
const dir = fs.mkdtempSync(path.join(testRoot, "d-"));
64
+
const outFile = path.join(dir, "child.env");
65
+
// Create with no ambient identity so the first launch records UNSET|UNSET.
66
+
createSession(dir, "s", outFile, { unset: ["ST_AGENT", "ST_ROOT"] });
67
+
expect(waitForContent(outFile)).toBe("UNSET|UNSET");
68
+
69
+
// Restart from a DIFFERENT agent's shell: its identity must not leak in.
70
+
fs.rmSync(outFile, { force: true });
71
+
const r = runCli(dir, ["restart", "-y", "s"], {
72
+
env: { ST_AGENT: "smalltalk-claude", ST_ROOT: "/leaked/convoy" },
73
+
});
74
+
expect(r.status).toBe(0);
75
+
expect(r.stdout).toContain("restarted");
76
+
77
+
const recorded = waitForContent(outFile);
78
+
expect(recorded).toBe("UNSET|UNSET");
79
+
expect(recorded).not.toContain("smalltalk-claude");
80
+
expect(recorded).not.toContain("/leaked/convoy");
81
+
}, 25000);
82
+
83
+
it("still inherits the creator's ST_AGENT on a fresh `pty run` (create path unaffected)", () => {
84
+
const dir = fs.mkdtempSync(path.join(testRoot, "d-"));
85
+
const outFile = path.join(dir, "child.env");
86
+
createSession(dir, "fresh", outFile, {
87
+
env: { ST_AGENT: "creator-abc", ST_ROOT: "/creator/convoy" },
88
+
});
89
+
expect(waitForContent(outFile)).toBe("creator-abc|/creator/convoy");
90
+
}, 20000);
91
+
});