Problem — #5669 (PR #5806) split the unattended Layered Intelligence cmd lane onto its own UNATTENDED_READONLY_COMMANDS allowlist, which removed every remote-code fetch/exec verb (npx, node, python, pip, curl, wget, go, cargo, make, brew). That closed the RCE class. It did not close the mutation class, because the gate is binary-level, not subcommand-level: four of the admitted binaries are multi-purpose and still accept destructive subcommands from persistent, attacker-reachable config on an unattended schedule.
Evidence — server/lib/commandSecurity.js, UNATTENDED_READONLY_COMMANDS. All of these pass validateUnattendedCommand today (no shell metacharacter, base binary on the list):
git reset --hard, git checkout ., git clean -fd — destroys uncommitted work in the app repo (cwd is the repo).
find . -delete / find . -exec … — arbitrary deletion, and -exec is arbitrary execution that bypasses the whole point of the allowlist.
gh api -X POST … / gh pr merge / glab mr merge — authenticated writes against the tracker using the operator's own credentials.
Impact is lower than #5669's (destructive/mutating, not "fetch and run attacker code"), which is why it was deliberately scoped out rather than rolled in — but find -exec in particular re-opens arbitrary execution, so this is not purely a data-loss concern.
Plan
- Add a per-binary subcommand/flag check to the unattended path in
server/lib/commandSecurity.js, keyed off check.baseCommand the same way validateCommand already keys the pm2 check off its base command — so the shape matches the existing precedent rather than inventing a second mechanism.
- Decisions (made, not deferred):
git — allow a read-only subcommand set: log, show, status, diff, blame, describe, rev-parse, branch, tag, remote, config --get, ls-files, shortlog. Reject everything else, including commit, reset, checkout, clean, push, fetch, pull. Rejecting unknown subcommands (allowlist, not denylist) is required — a denylist rots as git grows verbs.
find — reject the action flags -delete, -exec, -execdir, -ok, -okdir, -fls, -fprint, -fprintf anywhere in the args. Everything else (-name, -type, -maxdepth, -print) is inspection.
gh / glab — allow only <noun> list, <noun> view, and api with no -X/--method other than GET. Reject -X POST/PUT/PATCH/DELETE, merge, close, create, edit, delete, comment.
- Leave
ls, cat, head, tail, grep, wc, pwd, echo binary-level — none of them mutates. (grep has no write mode; tail -f only blocks, and the existing timeout covers it.)
- Keep this on the unattended path ONLY.
validateCommand and POST /api/commands/execute stay byte-identical — a human triggers and watches those, and git commit there is legitimate.
- Update the scope note in the
UNATTENDED_READONLY_COMMANDS comment (it currently says subcommand gating is tracked separately — point it at the implementation instead) and the commandSecurity.js row in server/lib/README.md.
Tests — extend server/lib/commandSecurity.test.js:
- Rejects
git reset --hard, git checkout ., git clean -fd, git push, and an unknown-to-the-allowlist subcommand.
- Accepts
git log --oneline -20, git status --short, git diff HEAD~1.
- Rejects
find . -delete and find . -exec rm {} ; (note: the metacharacter filter already catches the {}/; form, so use a form that reaches the new check — e.g. find . -exec rm -f -- + — or the test proves nothing new).
- Rejects
gh api -X POST /repos/x/y/issues, gh pr merge 1, glab mr merge 1; accepts gh pr list, gh issue view 1, gh api /rate_limit.
- Pins that
validateCommand('git commit -m "x") and validateCommand('gh pr merge 1') still pass — the operator lane is untouched.
Acceptance criteria
Out of scope — Do not change ALLOWED_COMMANDS, DANGEROUS_SHELL_CHARS, redactOutput, or server/routes/commands.js; do not remove the trustShellSources escape hatch; do not add binaries to UNATTENDED_READONLY_COMMANDS.
Follow-up to #5669 / #5806, raised by the local review round on that PR.
Problem — #5669 (PR #5806) split the unattended Layered Intelligence
cmdlane onto its ownUNATTENDED_READONLY_COMMANDSallowlist, which removed every remote-code fetch/exec verb (npx,node,python,pip,curl,wget,go,cargo,make,brew). That closed the RCE class. It did not close the mutation class, because the gate is binary-level, not subcommand-level: four of the admitted binaries are multi-purpose and still accept destructive subcommands from persistent, attacker-reachable config on an unattended schedule.Evidence —
server/lib/commandSecurity.js,UNATTENDED_READONLY_COMMANDS. All of these passvalidateUnattendedCommandtoday (no shell metacharacter, base binary on the list):git reset --hard,git checkout .,git clean -fd— destroys uncommitted work in the app repo (cwdis the repo).find . -delete/find . -exec …— arbitrary deletion, and-execis arbitrary execution that bypasses the whole point of the allowlist.gh api -X POST …/gh pr merge/glab mr merge— authenticated writes against the tracker using the operator's own credentials.Impact is lower than #5669's (destructive/mutating, not "fetch and run attacker code"), which is why it was deliberately scoped out rather than rolled in — but
find -execin particular re-opens arbitrary execution, so this is not purely a data-loss concern.Plan
server/lib/commandSecurity.js, keyed offcheck.baseCommandthe same wayvalidateCommandalready keys thepm2check off its base command — so the shape matches the existing precedent rather than inventing a second mechanism.git— allow a read-only subcommand set:log,show,status,diff,blame,describe,rev-parse,branch,tag,remote,config --get,ls-files,shortlog. Reject everything else, includingcommit,reset,checkout,clean,push,fetch,pull. Rejecting unknown subcommands (allowlist, not denylist) is required — a denylist rots as git grows verbs.find— reject the action flags-delete,-exec,-execdir,-ok,-okdir,-fls,-fprint,-fprintfanywhere in the args. Everything else (-name,-type,-maxdepth,-print) is inspection.gh/glab— allow only<noun> list,<noun> view, andapiwith no-X/--methodother thanGET. Reject-X POST/PUT/PATCH/DELETE,merge,close,create,edit,delete,comment.ls,cat,head,tail,grep,wc,pwd,echobinary-level — none of them mutates. (grephas no write mode;tail -fonly blocks, and the existing timeout covers it.)validateCommandandPOST /api/commands/executestay byte-identical — a human triggers and watches those, andgit committhere is legitimate.UNATTENDED_READONLY_COMMANDScomment (it currently says subcommand gating is tracked separately — point it at the implementation instead) and thecommandSecurity.jsrow inserver/lib/README.md.Tests — extend
server/lib/commandSecurity.test.js:git reset --hard,git checkout .,git clean -fd,git push, and an unknown-to-the-allowlist subcommand.git log --oneline -20,git status --short,git diff HEAD~1.find . -deleteandfind . -exec rm {} ;(note: the metacharacter filter already catches the{}/;form, so use a form that reaches the new check — e.g.find . -exec rm -f -- +— or the test proves nothing new).gh api -X POST /repos/x/y/issues,gh pr merge 1,glab mr merge 1; acceptsgh pr list,gh issue view 1,gh api /rate_limit.validateCommand('git commit -m "x")andvalidateCommand('gh pr merge 1')still pass — the operator lane is untouched.Acceptance criteria
git reset --hard,find . -delete, andgh api -X POST …are rejected byvalidateUnattendedCommandand dropped byrunShellCommandwith a warning.git log,gh pr list, and the other read-only forms still run and their stdout still reaches the reasoner.validateCommand/POST /api/commands/executebehavior is byte-identical.settings.layeredIntelligence.trustShellSources === truestill restores full-shell behavior unchanged.cd server && npm testpasses.Out of scope — Do not change
ALLOWED_COMMANDS,DANGEROUS_SHELL_CHARS,redactOutput, orserver/routes/commands.js; do not remove thetrustShellSourcesescape hatch; do not add binaries toUNATTENDED_READONLY_COMMANDS.Follow-up to #5669 / #5806, raised by the local review round on that PR.