fix(restart): scrub the restarter's bus-identity env from the re-exec - #75
Merged
Conversation
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).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Incident follow-up #3 — the deeper root cause (guardrail #73 blocked the accident; backstop #74 fixed the orphan-daemon; this fixes the wrong-identity mechanism itself).
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:So when cos was restarted from smalltalk's shell, that shell's
ST_AGENT=smalltalk-claude(andST_ROOT) leaked into the re-exec → cos came back under the wrong bus identity and died (exit 129).Fix
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.scrubEnv?: string[]onspawnDaemon— deletes the named keys from the daemon's env before it spawns, and therefore before the session child inherits them (spawnViaNode path).cmdRestart,handleDeadSession) passRESTART_SCRUBBED_ENV = ["ST_AGENT", "ST_ROOT"].Deliberately scoped to restart
pty runis untouched — a convoy-launched create legitimately inherits its own identity. (Verified by a control test.)pty restartappropriately un-blessed for agents (consistent with the feat(restart): refuse to restart a stateful agent session unless --force #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 remains its supervisor (convoy). I chose scrub over persist-and-restore precisely so pty restart doesn't become a convoy-competitor for agent lifecycle; scrub kills the footgun (wrong-identity collision) without new persisted state.grepconfirms the only refs are this PR's comment/const), so scrubbing changes only what the child inherits.Tests —
tests/restart-env-scrub.test.tsST_AGENT=smalltalk-claude/ST_ROOT=/leaked/convoyyields a child that recordsUNSET|UNSET— identity scrubbed, not leaked (asserts the leaked values are absent).pty runstill inherits its creator'sST_AGENT/ST_ROOT.Full suite green (1265 passed, 0 failed; scrollback load-flake did not recur). All spawn/restart/env-adjacent tests (restart-guardrail, spawn-options, spawn-bundle-fallback, env-isolation, up-down, up-name-decouple, nesting[-prevention] — 81 tests) pass. Build/typecheck clean.
Note / possible later follow-up
Only the operator-restart paths are scrubbed. The toml-batch spawn (
spawnDaemonat the pty-file path,commandWithEnvExports) is config-driven with its own env exports and isn't the incident path — left as-is. If we later want config-spawns to also refuse ambient identity leakage, that's a small separate change. The low-pri scrollback-fidelity flake hardening is still queued after this.