Repository navigation
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
Conversation
…x user through sudo, allowlist env, sudo kill, fail-closed preflight (TASK-651)
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
VERDICT: REQUEST-CHANGES Defects
Not verified
Notes[pass A (hunk)]
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. |
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 ininternal/executor/backend_claudecode.go, epic planning ininternal/executor/epic.go, the judge and reviewers ininternal/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 ininternal/executor/parallel.go, the summary and evidence calls ininternal/executor/runner.go, and the other backends (internal/executor/backend_opencode.go,internal/executor/backend_qwencode.go). Each takes its environment from modelSubprocessEnvFor ininternal/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 ininternal/executor/process_group.go. The per-run guard directory and settings file come from setupRunGuards ininternal/executor/read_guard.gowith modes 0700 and 0600; the gh shim directory comes frominternal/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
internal/executor/backend.go, two string fields next to ReadGuard: run_as_user and run_as_group (yaml names; empty by default). Document both inconfigs/pilot.example.yamlwith the sentence "Linux only; leave empty until the agent workspace legs of TASK-651 are released".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.--.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.sudo -n -u <user> trueexits 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.Acceptance
go test -race -count=1 ./internal/executor/ ./cmd/pilot/ -run 'AgentCommand|RunAsUser|ModelEnv|ReadGuard'passes; paste the output into the PR body.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:
Not-verified: