Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1473,6 +1473,7 @@ async function handleDeadSession(
await spawnDaemon({
name: session.name, command: meta.command, args: meta.args, displayCommand: meta.displayCommand, cwd: meta.cwd, tags: meta.tags,
...(meta.displayName ? { displayName: meta.displayName } : {}),
scrubEnv: RESTART_SCRUBBED_ENV,
});
console.log(`Session "${session.name}" restarted.`);
doAttach(session.name);
Expand Down Expand Up @@ -2999,6 +3000,18 @@ function statefulAgentReason(meta: SessionMetadata): string | null {
return null;
}

/** Bus-identity env vars stripped from an operator-initiated restart's re-exec.
* `pty restart` (and the dead-session "Restart? [Y/n]" path) re-run a stored
* command under the RESTARTER's shell env. If that shell belongs to a different
* convoy agent, its ST_AGENT/ST_ROOT leak into the re-exec'd session and it
* comes back under the wrong bus identity — the cos-restart incident, where
* restarting cos from smalltalk's shell brought cos back as smalltalk-claude
* (exit 129). Scrubbing them means a restarted session never inherits the
* restarter's identity; the blessed way to restart an agent with a correct
* identity is its supervisor (convoy). Fresh `pty run` is unaffected — a
* convoy-launched create legitimately inherits its own identity. */
const RESTART_SCRUBBED_ENV = ["ST_AGENT", "ST_ROOT"];

async function cmdRestart(
name: string,
yes = false,
Expand Down Expand Up @@ -3062,6 +3075,7 @@ async function cmdRestart(
await spawnDaemon({
name, command: meta.command, args: meta.args, displayCommand: meta.displayCommand, cwd: meta.cwd, tags: restartTags,
...(meta.displayName ? { displayName: meta.displayName } : {}),
scrubEnv: RESTART_SCRUBBED_ENV,
});
console.log(`Session "${name}" restarted.`);

Expand Down
14 changes: 14 additions & 0 deletions src/spawn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,15 @@ export interface SpawnDaemonOptions {
* handler still surfaces immediate failures within milliseconds, so
* this only governs the "alive but slow" case. */
startTimeoutMs?: number;
/** Env var names to DELETE from the daemon's inherited environment before it
* spawns — and therefore before the session child inherits it. Used by the
* operator-initiated restart paths to strip the *restarter's* ambient
* bus-identity vars (ST_AGENT/ST_ROOT), so a session re-exec'd from a
* different shell can't come back under that shell's identity. See the
* cos-restart incident. Applied on the spawnViaNode path; the CLI-fallback
* path can't express it (a bundled consumer that hits the fallback also
* isn't the operator-restart context this guards). */
scrubEnv?: string[];
}

/** Default time we wait for a daemon's Unix socket to appear after
Expand Down Expand Up @@ -151,6 +160,11 @@ async function spawnViaNode(options: SpawnDaemonOptions, serverModule: string):
// when its spawner is gone. Off by default — opt in via
// `bindToSpawnerLifetime` when the caller owns the daemon's lifetime.
const env: Record<string, string> = { ...process.env, PTY_SERVER_CONFIG: config };
// Strip caller-requested vars (e.g. the restarter's leaked bus identity)
// before the daemon — and thus the session child — can inherit them.
if (options.scrubEnv) {
for (const key of options.scrubEnv) delete env[key];
}
if (options.bindToSpawnerLifetime) env.PTY_SPAWNER_PID = String(process.pid);
const child = spawn(launcherCmd, [...launcherArgs, serverModule], {
detached: true,
Expand Down
91 changes: 91 additions & 0 deletions tests/restart-env-scrub.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
import { describe, it, expect, afterAll } from "vitest";
import * as fs from "node:fs";
import * as os from "node:os";
import * as path from "node:path";
import { fileURLToPath } from "node:url";
import { spawnSync } from "node:child_process";

const __dirname = path.dirname(fileURLToPath(import.meta.url));
const nodeBin = process.execPath;
const cliPath = path.join(__dirname, "..", "dist", "cli.js");
const testRoot = fs.mkdtempSync(path.join(os.tmpdir(), "pty-envscrub-"));
const bgPids: number[] = [];
afterAll(() => {
for (const pid of bgPids) { try { process.kill(pid, "SIGKILL"); } catch {} }
fs.rmSync(testRoot, { recursive: true, force: true, maxRetries: 3, retryDelay: 100 });
});

interface RunOpts { env?: Record<string, string>; unset?: string[]; timeout?: number }

// PTY_SESSION set => restart takes its "already inside a session, not attaching"
// branch and returns instead of hanging on a non-TTY attach.
function runCli(dir: string, args: string[], opts: RunOpts = {}) {
const env: Record<string, string> = {
...(process.env as Record<string, string>),
PTY_SESSION_DIR: dir, PTY_ROOT_LEGACY_SILENT: "1", PTY_SESSION: "outer",
...(opts.env ?? {}),
};
for (const k of opts.unset ?? []) delete env[k];
return spawnSync(nodeBin, [cliPath, ...args], { env, encoding: "utf8", timeout: opts.timeout ?? 15000 });
}

/** A command that records the ST_AGENT/ST_ROOT it was actually launched with,
* then stays alive. `-` default => "UNSET" when the var is absent. Re-runs on
* every (re)start, so the file always reflects the current child's env. */
function recorderCmd(outFile: string): string[] {
return ["sh", "-c", `printf '%s|%s' "\${ST_AGENT-UNSET}" "\${ST_ROOT-UNSET}" > "${outFile}"; exec sleep 300`];
}

function createSession(dir: string, name: string, outFile: string, opts: RunOpts): void {
const r = runCli(dir, ["run", "-d", "--id", name, "--", ...recorderCmd(outFile)], opts);
expect(r.status).toBe(0);
try {
bgPids.push(Number(fs.readFileSync(path.join(dir, `${name}.pid`), "utf8").trim()));
} catch {}
}

const sleepSync = (ms: number) => Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms);

function waitForContent(p: string, timeoutMs = 4000): string {
const start = Date.now();
while (Date.now() - start < timeoutMs) {
try {
const s = fs.readFileSync(p, "utf8");
if (s.length > 0) return s;
} catch {}
sleepSync(50);
}
try { return fs.readFileSync(p, "utf8"); } catch { return ""; }
}

describe("restart scrubs the restarter's bus-identity env", () => {
it("does NOT leak the restarter's ST_AGENT/ST_ROOT into the re-exec'd session", () => {
const dir = fs.mkdtempSync(path.join(testRoot, "d-"));
const outFile = path.join(dir, "child.env");
// Create with no ambient identity so the first launch records UNSET|UNSET.
createSession(dir, "s", outFile, { unset: ["ST_AGENT", "ST_ROOT"] });
expect(waitForContent(outFile)).toBe("UNSET|UNSET");

// Restart from a DIFFERENT agent's shell: its identity must not leak in.
fs.rmSync(outFile, { force: true });
const r = runCli(dir, ["restart", "-y", "s"], {
env: { ST_AGENT: "smalltalk-claude", ST_ROOT: "/leaked/convoy" },
});
expect(r.status).toBe(0);
expect(r.stdout).toContain("restarted");

const recorded = waitForContent(outFile);
expect(recorded).toBe("UNSET|UNSET");
expect(recorded).not.toContain("smalltalk-claude");
expect(recorded).not.toContain("/leaked/convoy");
}, 25000);

it("still inherits the creator's ST_AGENT on a fresh `pty run` (create path unaffected)", () => {
const dir = fs.mkdtempSync(path.join(testRoot, "d-"));
const outFile = path.join(dir, "child.env");
createSession(dir, "fresh", outFile, {
env: { ST_AGENT: "creator-abc", ST_ROOT: "/creator/convoy" },
});
expect(waitForContent(outFile)).toBe("creator-abc|/creator/convoy");
}, 20000);
});
Loading