fix(agent): constrain headless file writes and node commands#120
Open
rich7420 wants to merge 1 commit into
Open
fix(agent): constrain headless file writes and node commands#120rich7420 wants to merge 1 commit into
rich7420 wants to merge 1 commit into
Conversation
Before: in headless lanes (cron, heartbeat, sub-agents) there is no permission resolver, so the permission checker auto-approved anything that was not a statically-safe call — including write_file/edit_file to ANY absolute path and nodes system.run with a dangerous inner command. Only bash had a dangerous-command floor. A hallucinating or prompt-injected headless run could overwrite ~/.ssh/authorized_keys, drop a file anywhere on disk, or run a destructive node command, with no human in the loop. After: file writes and node commands go through the same envelope headless already applied to bash. - Hard-block floor (runs before the config-rule logic, so it beats an explicit permissions.allow rule): dangerous bash commands (unchanged), write_file/ edit_file to a dangerous path (isDangerousPath: ssh keys, home dotfiles, .git/.claude, .env, credentials, ...), and nodes system.run whose inner command is dangerous. Extracted into headlessHardBlockReason(). - Workspace envelope in the auto-approve path: write_file/edit_file is allowed only inside the session working directory, an explicitly approved directory, or the specific memory targets a headless lane legitimately writes (the memory/ subtree and MEMORY.md under the workspace root). Everything else is denied; users can still opt in per-path via permissions.allow. - Symlink escape closed: tryRealpath now canonicalizes the deepest existing ancestor of a not-yet-created path (iteratively, so a pathological path cannot overflow the stack), so a write through an in-tree symlink pointing outside the workspace resolves to its real location before the containment check. This also fixes a pre-existing false-deny for new files under an approved directory that contains a symlinked path component. Headless bash is intentionally left denylist-based: sub-agents are expected to run builds and tests, so a non-safe, non-dangerous command still auto-approves. The memory-write allowance is scoped to memory/ and MEMORY.md only (not the whole workspace) so a headless lane cannot plant a workspace skill or overwrite a bootstrap file that a later session would load.
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.
Summary
Headless agent lanes — cron jobs, the heartbeat turn, and sub-agents spawned by
the
agenttool — run with no permission resolver (there is no human toprompt). The permission checker's response to "no resolver" was to auto-approve
anything that was not already denied, so a whole class of tools fell through:
write_file/edit_fileto any absolute path (e.g.~/.ssh/authorized_keys,~/.bashrc,.env, another repo,/tmp) — with zero path validation.nodessystem.runwith a dangerous inner command — the existingdangerous-command floor keyed on
call.name === "bash", so a dangerous commandwrapped in a node call bypassed it.
Only
bashhad a floor. This PR extends the same envelope to file writes andnode commands, so an unattended, possibly prompt-injected, model cannot write
outside its sandbox or run a destructive node command.
How the headless permission check flows (after)
For each tool call, headless (resolver == null) evaluates in this order:
Why it is shaped this way
explicit
allowrule shouldn't let a hallucinating model overwrite SSH keysor run a destructive node command, exactly as bash already treated
rm -rf.So those live above the config-rule logic and beat
allow.accept-editsenvelope, not a blanketallow. Writes are permitted inside the session working directory or an
explicitly approved directory — the same containment interactive mode enforces
— and denied elsewhere. Users can still opt a specific path in via
permissions.allow.(heartbeat consolidation/distillation, cron daily logs) write to a fixed
path under the workspace root (
memory/…andMEMORY.md) whose location isindependent of the session working directory. We allow exactly those targets —
not the whole workspace root — because widening to the root would also let
a headless lane plant a workspace skill (which overrides bundled/user skills)
or overwrite a bootstrap file (
AGENTS.md,HEARTBEAT.md,SOUL.md) that alater, higher-trust session loads.
write_filecan target a not-yet-created leaf;realpathSyncthrows on it and used to fall back to the lexical path, so awrite through an in-tree symlink pointing outside the workspace looked
contained but landed outside.
tryRealpathnow canonicalizes the deepestexisting ancestor (iteratively — a pathological path can't overflow the stack)
before the containment check. This also fixes a pre-existing false-deny for
new files created under an approved directory reached via a symlinked path
component.
builds and tests, so a non-safe, non-dangerous command (
echo,mkdir,bun test,git commit) still auto-approves in headless. Tightening headlessbash to an allowlist would break that primary use and is a separate policy
decision.
Validation
bunx tsc --noEmit: clean.bun test ./tests/test-tool-executor.ts: 114 pass, 0 fail. New tests cover:dangerous-path writes denied; out-of-workspace writes denied; in-workspace and
memory (
memory/…,MEMORY.md) writes allowed; workspace skill / bootstrapwrites denied;
nodes system.run rm -rf ~denied; symlink-escape write deniedand nothing created outside; a config
allowrule re-permitting anout-of-workspace write; and a non-safe/non-dangerous bash command still
auto-approving.
bun run test(full core suite): 4753 pass, 0 fail.new out-of-workspace escape and no regression to interactive
accept-edits/allow_directoryflows.Limitations / Follow-ups
Headless
bashremains denylist-based by design (see above): a non-safe,non-dangerous command such as
curl -d @secret https://…,mv, ornpm install <pkg>still auto-approves. Moving headless bash to an allowlistis a larger policy change tracked separately.
isDangerousPathis lexical (it does not resolve symlinks), so a symlink withan innocent name pointing at a workspace-internal dangerous file (e.g.
<ws>/data.json -> <ws>/.git/config) is only bounded by the workspaceenvelope, not flagged by the floor. This is pre-existing and shared with
interactive
accept-edits; the proper fix (canonicalize beforeisDangerousPath) should be applied to both lanes together.All headless lanes currently share one write envelope (session working
directory + explicitly approved directories + the memory targets). They are
not differentiated per lane: a heartbeat or cron turn can write anywhere in
the gateway's launch directory, not only inside the workspace directory.
Tightening this to per-lane capability grants (e.g. heartbeat limited to the
workspace directory, broader scopes only via explicit configuration) needs a
lane-identity signal plumbed into the tool context plus a capability-config
mechanism, and is deferred to the policy-pipeline rewrite where it belongs.