Skip to content

GH-6321: feat(executor): run_as_user — start every model process as a separate Unix user through sudo, with an allowlist environment, a kill through sudo and a fail-closed preflight (off by default) - #6335

Merged
alekspetrov merged 1 commit into
mainfrom
pilot/GH-6321
Nov 12, 2026

Conversation

@alekspetrov

Copy link
Copy Markdown
Collaborator

Summary

Automated PR created by Pilot for task GH-6321.

Closes #6321

Changes

GitHub Issue GH-6321: feat(executor): run_as_user — start every model process as a separate Unix user through sudo, with an allowlist environment, a kill through sudo and a fail-closed preflight (off by default)

Context

Navigator TASK-651 (design in .agent/system/agent-isolation-design.md, sections 4.1 and 4.3). Today every model process is built inline with exec.CommandContext and runs as the daemon user: the main backend in internal/executor/backend_claudecode.go, epic planning in internal/executor/epic.go, the judge and reviewers in internal/executor/intent_judge.go, the parsers and classifiers (internal/executor/subtask_parser.go, internal/executor/effort_classifier.go, internal/executor/complexity_classifier.go), research subagents in internal/executor/parallel.go, the summary and evidence calls in internal/executor/runner.go, and the other backends (internal/executor/backend_opencode.go, internal/executor/backend_qwencode.go). Each takes its environment from modelSubprocessEnvFor in internal/executor/model_env.go, a deny-list scrub that keeps the GitHub and Claude names. The process group is set by configureProcessGroup and killed by killProcessGroup in internal/executor/process_group.go. The per-run guard directory and settings file come from setupRunGuards in internal/executor/read_guard.go with modes 0700 and 0600; the gh shim directory comes from internal/executor/ghguard_spawn.go.

This leg adds the option and the mechanics only. With the option empty nothing changes. The workspace, the gates and the GitHub token follow in later legs, so the option must stay empty on every host until they merge; say so in the config example.

Scope

  1. Config: in the ClaudeCodeConfig struct of internal/executor/backend.go, two string fields next to ReadGuard: run_as_user and run_as_group (yaml names; empty by default). Document both in configs/pilot.example.yaml with the sentence "Linux only; leave empty until the agent workspace legs of TASK-651 are released".
  2. One constructor, new file, plain text: internal/executor/agent_command.go. A function agentCommand(ctx, name, args...) returns the exec.Cmd for a model process. With the option empty it returns exactly exec.CommandContext(ctx, name, args...). With the option set, on Linux, it returns the command sudo -n -u <user> [-g <group>] --preserve-env=<comma list of the names in cmd.Env> -- <name> <args...>; the list is filled by a second function that the caller uses to set the environment, so that the names in the flag and the entries of cmd.Env always match. No value of an environment variable is ever placed in argv. On a platform other than Linux a set option fails the preflight (item 6) and the constructor behaves as with an empty option.
  3. Use the constructor at every model spawn site named in Context. A site that is missed is a defect: add one test per site that sets the option and asserts the first argv element is sudo and that the model binary follows the --.
  4. Environment with the option set: an allowlist, not the deny-list. Keep: PATH, LANG, LC_ALL, TERM, TZ, TMPDIR, the Go cache and proxy names (GOCACHE, GOMODCACHE, GOFLAGS, GOTOOLCHAIN, GOPROXY, GONOSUMDB, GOPRIVATE), NODE_OPTIONS, PILOT_EXECUTOR, every name with the prefix CLAUDE_ or ANTHROPIC_ that the present function keeps or injects (the billing credential of the execution included), and HOME and CLAUDE_CONFIG_DIR set to the home of the agent user (resolve it once with os/user). Drop everything else, the GitHub names included; the GitHub token for the agent comes in a later leg. With the option empty the present deny-list result is unchanged, byte for byte.
  5. Kill: with the option set, killProcessGroup runs sudo -n -u <user> kill -s <signal> -- -<pgid> with a 5 second timeout, because the daemon user may not signal the agent user or sudo. Keep the direct kill for the empty option. When the sudo kill fails, log one Error line with the pgid.
  6. Preflight, called where PreflightReadGuard is called at daemon start: with the option set, check Linux, the user exists and is not the user of the daemon, sudo -n -u <user> true exits 0 within 5 seconds, and the home of the user exists. On any failure log one Error line that names the failed check and set a process-wide flag; while the flag is set, every model spawn returns an error "run_as_user preflight failed: " and no process starts. It must not fall back to a run as the daemon user. With the option empty the preflight does nothing.
  7. Guard files with the option set: the directory from setupRunGuards and the gh shim directory get mode 0750 and the files in them 0640 (scripts 0750), and the group of run_as_group when it is set (os.Chown with the gid; the daemon user is a member). They stay owned by the daemon user. With the option empty the modes stay 0700 and 0600.
  8. Tests, table-driven, no root needed, in new test files next to the code: the constructor with the option empty returns the plain command; with it set the argv is as in item 2 and holds no environment value; the preserve list equals the names in cmd.Env; the allowlist drops GITHUB_TOKEN, GH_TOKEN, AWS names and an arbitrary FOO_TOKEN and keeps the names of item 4; the kill builds the sudo command; the preflight fails closed for a missing user, for the user of the daemon, and for a sudo that exits non-zero (use a PATH with a mock sudo script), and a spawn after a failed preflight returns the error and starts nothing; the guard file modes for both states of the option.

Acceptance

  • go test -race -count=1 ./internal/executor/ ./cmd/pilot/ -run 'AgentCommand|RunAsUser|ModelEnv|ReadGuard' passes; paste the output into the PR body.
  • Use exec.CommandContext directly at one spawn site -> the test of that site fails.
  • Let a failed preflight fall back to the plain command -> the fail-closed test fails.
  • Add GITHUB_TOKEN to the allowlist -> the allowlist test fails.
  • Do not re-run make build, make test or make lint as evidence.

Out of scope

The workspace and git operations (still a worktree owned by the daemon user, so the option must stay empty on hosts). Quality gates, evidence commands and workflow hooks. The GitHub token for the agent. A per-project override. Any change of behaviour with the option empty.

Not verified

  • go test -race -count=1 ./internal/executor/ ./cmd/pilot/ -run 'AgentCommand|RunAsUser|ModelEnv|ReadGuard' passes; paste the output into the PR body.
    Reason: Not verified: shell operators are not allowed in evidence commands

  • Use exec.CommandContext directly at one spawn site -> the test of that site fails.
    Reason: freeform mutation, run manually

  • Let a failed preflight fall back to the plain command -> the fail-closed test fails.
    Reason: freeform mutation, run manually

  • Add GITHUB_TOKEN to the allowlist -> the allowlist test fails.
    Reason: freeform mutation, run manually

Pre-PR review

Mode: enforce
Verdict: APPROVE-WITH-NOTES
Fix: none

Defects:

  • [minor] at internal/executor/backend_opencode.go:702 — With run_as_user set, StopServer still kills the OpenCode server directly, not through sudo. The process it holds is the root sudo, which the daemon user may not signal, so the server agentCommand started at :119 outlives StopServer
  • [minor] at internal/executor/effort_classifier.go:117 — None of the sites without a cmd.Cancel override send a context cancel or timeout through sudo: effort/complexity classifier, subtask parser, intent judge runCmdInHome (intent_judge.go:184), and runner.go summary/evidence. os/exec's default Cancel is Process.Kill on the sudo pid, which the daemon user cannot signal, so a timed-out model call keeps running. Only backend_claudecode.go:651, backend_qwencode.go:268 and parallel.go:287 set cmd.Cancel = func() error { return killProcessGroup(cmd, syscall.SIGKILL) }.
  • [minor] at internal/executor/agent_command.go:264 — With the option set, agentEnvFor drops HOME and CLAUDE_CONFIG_DIR and puts the agent's own back. That breaks the post-merge reviewer's review HOME, whose contract is written at intent_judge.go:173-174: "a non-empty home replaces the subprocess HOME and config dir". postMergeReviewEnviron (post_merge_confine.go:175) appends "HOME="+home, "CLAUDE_CONFIG_DIR="+configDir, and the allowlist silently replaces both, so the review settings written at post_merge_confine.go:127 stop applying. Only reachable with the option set, and issue item 4 asks for the agent HOME.
  • [minor] at internal/executor/agent_command.go:222 — CLAUDE_CONFIG_DIR is set to /.claude. Issue item 4 says "HOME and CLAUDE_CONFIG_DIR set to the home of the agent user". This reading is reasonable but differs from the literal text.
  • [minor] at PR — body
  • [minor] at internal/executor/intent_judge.go:191 — With run_as_user set, the post-merge review loses its own HOME and config dir. setAgentEnv runs after postMergeReviewEnviron, and agentEnvFor drops the input CLAUDE_CONFIG_DIR and forces HOME=. This goes against the contract at post_merge_confine.go:136-138 ("with HOME and the CLI's config dir pointed at the review's own home"). The user-level confinement settings at post_merge_confine.go:127 (filepath.Join(env.home, ".claude", "settings.json")) and the copied login file are no longer seen. The worktree copy at :126 still applies. Item 4 of the issue asks for exactly this override, so it is not blocking, but a later leg has to reconcile the two.
  • [minor] at internal/executor/epic.go:301 — PlanEpic, the classifiers, the subtask parser, the judge (intent_judge.go:204-211) and runner summary/evidence never set cmd.Cancel or a process group. On ctx cancel, exec's default Process.Kill signals the sudo pid directly. Item 5 says the daemon user may not do this, so a cancelled sudo-run model process can outlive its context. The sudo kill covers only the paths that already called killProcessGroup.
  • [minor] at internal/executor/backend_claudecode.go:805 — A failed sudo kill logs two Error lines, not the one in item 5. killAgentProcessGroup logs its own Error, then this pre-existing caller logs "Failed to kill process group after result-exit grace". The same applies to the callers at :908 and :954.
  • [minor] at internal/executor/agent_command.go:367 — With run_as_user set and run_as_group empty, applyGuardPerm opens the guard files to the daemon's primary group only. The agent user is normally not in that group, so it cannot read --settings or run the hook scripts. This follows item 7 literally, but the config example (pilot.example.yaml) does not say a group is effectively required.

Not-verified:

  • The required go test -race ... -run 'AgentCommand|RunAsUser|ModelEnv|ReadGuard' output: not in the PR body, and I could not run it (read-only checkout).
  • The three mutation acceptance bullets: no evidence in the PR body, and I could not run mutations.
  • That a real sudo accepts --preserve-env= for these names under the host's sudoers (env_reset, secure_path): this depends on the host and I could not check it from the checkout.
  • … 3 more not-verified items; 2 requirements marked met but not traced to a file read omitted (capped at 3)

…x user through sudo, allowlist env, sudo kill, fail-closed preflight (TASK-651)
@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 79.83539% with 49 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/executor/agent_command.go 80.21% 27 Missing and 9 partials ⚠️
internal/executor/ghguard_spawn.go 0.00% 4 Missing and 2 partials ⚠️
internal/executor/read_guard.go 0.00% 3 Missing and 3 partials ⚠️
internal/executor/intent_judge.go 83.33% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@alekspetrov

Copy link
Copy Markdown
Collaborator Author

VERDICT: REQUEST-CHANGES

Defects

  • [blocking] at internal/executor/agent_command.go:263 — with run_as_user set, the allowlist drops PILOT_GH_GUARD_SHIM_DIR, PILOT_GH_REAL, PILOT_TASK_* and PILOT_GH_GUARD_JOURNAL but keeps PATH, which still starts with the shim dir. That breaks the contract at internal/executor/ghguard/policy.go:96-100 ("ResolveFallbackGh excludes this directory when EnvRealGh is unset, so the fallback PATH search cannot recurse into the shim").
    Evidence: backend_claudecode.go:709 calls env = prependPathEnv(env, shimDir) and :710 appends ghGuardTaskEnv(...). At ghguard_spawn.go:98-103 those are the PILOT_* names. In the agent process, cmd/pilot/ghguard.go:103-105 reads realGh := id.RealGh (now empty) and calls ResolveFallbackGh(getenv(ghguard.EnvShimDir)) with "". At policy.go:938 and :943, excludeDir = filepath.Clean(excludeDir) turns "" into ".", which excludes nothing. So the shim's own gh is the first match, and ghguard.go:113 exec.Command(realGh, args...) re-enters the shim without end on every allowed gh call. Task identity and the denial journal are lost too. The test at agent_command_test.go (allowlist case) asserts PILOT_GH_REAL is dropped, which pins this behaviour.
  • [minor] at internal/executor/backend_opencode.go:702 — return b.serverCmd.Process.Kill() signals the sudo process directly, and item 5 says the daemon user may not do that. So with the option set, StopServer cannot stop the OpenCode server that agentCommand started at :119.
    Evidence: killProcessGroup is never called on this path
  • [minor] at internal/executor/effort_classifier.go:117 — the effort and complexity classifiers, the subtask parser, epic.go:301, intent_judge.go:184 and runner.go:9288/9350 never set cmd.Cancel. On a ctx timeout, exec's default Process.Kill hits the sudo pid, which fails as item 5 describes, so a timed-out model call can outlive its context. Only backend_claudecode.go:651, backend_qwencode.go:268 and parallel.go:287 route through killProcessGroup.
  • [minor] at internal/executor/agent_command.go:367 — with run_as_group empty, s.gid stays -1 and the files stay in the daemon's primary group at 0750/0640. The agent user is normally not in that group, so it cannot read --settings (read_guard.go:178) or run the hook scripts (read_guard.go:146). The config example at configs/pilot.example.yaml:948 does not say a group is effectively required.
  • [minor] at internal/executor/ghguard_spawn.go:45 — the shim dir is 0750, so the agent user has no write access to journal.jsonl (:72), and every deny entry that cmd/pilot/ghguard.go:96 writes through AppendJournal is silently lost.
  • [minor] at internal/executor/intent_judge.go:195 — setAgentEnv runs after postMergeReviewEnviron, which appends at post_merge_confine.go:175 (append(out, "HOME="+home, "CLAUDE_CONFIG_DIR="+configDir)). agentEnvFor drops CLAUDE_CONFIG_DIR and forces HOME to the agent home, so the post-merge review home stops applying. Item 4 asks for this override, so it is not blocking, but a later leg has to reconcile the two.
  • [minor] at internal/executor/agent_command.go:129 — if uid, err := currentUID(); err == nil && uid == u.Uid lets the "not the daemon user" check pass when user.Current fails. That leaves a fail-open path in a check that is meant to be fail-closed.
  • [minor] at internal/executor/agent_command_test.go:497 — the option-set mode test only exercises os.Chown when the daemon has a supplementary group (if g != os.Getegid()). Otherwise s.gid stays -1 and the group half of item 7 goes untested.
  • [minor] at internal/executor/backend_claudecode.go:805 — a failed sudo kill logs two Error lines: killAgentProcessGroup logs one at agent_command.go:347, then this caller logs its own. Item 5 asks for one. The same applies at :908 and :954.
  • [minor] at internal/executor/intent_judge.go:184 — Several sites keep os/exec's default Cancel (Process.Kill on the sudo pid) and set no process group or WaitDelay: intent judge runCmdInHome (intent_judge.go:204 cmd.Start() / :211 cmd.Wait()), effort and complexity classifiers, subtask parser, PlanEpic, and the runner summary and evidence calls. When ctx times out, the kill of the sudo pid fails, and cmd.Output()/Wait blocks until the model exits by itself.
    Evidence: only backend_claudecode.go:651, backend_qwencode.go:268 and parallel.go:287 route Cancel through killProcessGroup
  • [minor] at internal/executor/agent_command.go:290 — In setAgentEnv, if the settings were cleared between agentCommand and setAgentEnv, cmd.Env gets the full deny-list env (GitHub names included) while argv still carries --preserve-env=HOME,CLAUDE_CONFIG_DIR from envBase. sudo would then forward the daemon's HOME. The comment "keep it closed" does not match this. Only a reload race can reach it.
  • [minor] at internal/executor/backend_anthropic.go:218 — execBash runs model-chosen commands with exec.CommandContext(ctx, "bash", "-c", command) as the daemon user and with the full ambient env. This site is not named in the issue's Context, so it is not a scope miss, but "every model process" does not hold for the anthropic backend.

Not verified

  • The go test -race output and the three mutation acceptance bullets: the PR body's Not verified section says none of them were run, and this checkout is read-only.
  • Whether the host sudo (env_reset, secure_path) honours --preserve-env for PATH: this depends on the host's sudoers and cannot be seen from the checkout.
  • Callers of killProcessGroup at runner.go:8071/8092: not traced to the command they kill.
  • … 3 more not-verified items; 2 requirements marked met but not traced to a file read omitted (capped at 3)

Notes

[pass A (hunk)]
1 Config: met. backend.go:1340-1341 adds RunAsUser/RunAsGroup with yaml tags run_as_user/run_as_group, and pilot.example.yaml:948-950 carries the required sentence. An absent key unmarshals to "", which takes the empty-option path at agent_command.go:149.
2 Constructor: met. agent_command.go:208-217 returns plain exec.CommandContext when s is nil, and otherwise sudo plus preserve-env plus "--". setAgentEnv at :300-308 rewrites the flag from the final env, so names and values cannot drift. A cmd not built by agentCommand fails agentCmdUser at :315 and gets a plain assignment.
3 Spawn sites: met. A Grep of exec.Command in internal/executor (non-test) finds no remaining model-CLI spawn. Only gh, git, gates, hooks, dmesg and the billing command remain, and the issue scopes those out. agent_command_sites_test.go:30-71 covers all 11 named sites through the real code paths with a mock sudo.
4 Allowlist: met as written (agent_command.go:65-75 and :258-280). The billing names ANTHROPIC_API_KEY and ANTHROPIC_AUTH_TOKEN (model_env.go:213-216) pass through the ANTHROPIC_ prefix. With the option empty, setAgentEnv assigns env unchanged (:291-294). The env it drops breaks the gh guard; see the blocking defect.
5 Kill: met for killProcessGroup callers. process_group.go:23 routes to killAgentProcessGroup with the 5s timeout at agent_command.go:342, and TestRunAsUserKillProcessGroupGoesThroughSudo covers it. Paths that call Process.Kill directly are listed as minor defects.
6 Preflight: met. main.go:460 runs it inside applyReadGuardConfig, which is reached from PersistentPreRun at :398 and from start at :568. A failure sets failed, and agentCommand returns the error at :205-207 before building anything. TestReadGuardConfigRunsRunAsUserPreflight pins the wiring.
7 Guard modes: met for mode and group as specified (read_guard.go:136, :146, :178; ghguard_spawn.go:45, :67). With the option empty, applyGuardPerm returns at agent_command.go:368, so modes stay 0700/0600.
8 Tests: present and table-driven. They run the real spawn code against a mock sudo, not a mock of the adapter under test.
Memories: the autopilot YAML pitfall does not apply, since run_as_user binds under executor.claude_code (main.go:473). The founder-token learning has no bearing on this diff.
[pass B (adjacent)]

  • Item 1 met. The fields are at internal/executor/backend.go:1340-1341, and the example sentence is at configs/pilot.example.yaml:948-953. With the fields unset, PreflightRunAsUser resets the state at agent_command.go:148ff, so nothing changes.
  • Item 2 met. The plain path is agent_command.go:213 and the sudo argv is at :207ff. The names come from setAgentEnv (:282) and are rewritten from the same out slice, so the flag and cmd.Env cannot diverge, except in the reload race above.
  • Item 4 met. The allowlist is at agent_command.go:65-74 and is applied at :250ff. HOME and CLAUDE_CONFIG_DIR are forced by envBase (:221). CLAUDE_CONFIG_DIR is <home>/.claude, which is a reasonable reading of the issue text. With the option empty, cmd.Env is a plain assignment (:284).
  • Item 5 met. process_group.go:23 hands commands built by agentCommand to killAgentProcessGroup with the 5 s bound (agent_command.go:340). The empty option keeps procgroup.Kill.
  • Item 6 met. The preflight is wired at cmd/pilot/main.go:460, which every entrypoint reaches through :483 and :568. It checks Linux, the user, the daemon uid, sudo and home at agent_command.go:164-171. Spawns are refused through runAsSpawnError at :208, with no fallback to the daemon user.
  • Item 7 met. The run guard dir, scripts and settings are set at read_guard.go:136, :146 and :178, the shim dir and script at ghguard_spawn.go:45 and :67, and the group at agent_command.go:377.
  • Migrations: none. Out of scope: no change of behaviour with the option empty was found.
  • Memories: the autopilot YAML pitfall and the founder-token review learning do not apply to this diff.

Diff: 53361 bytes, 20 files, trimmed: no, memories: 2, checkout: yes, files read: 18

Automated post-merge review — an input to human review of this merge, not a substitute for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants