Skip to content

Gate unattended cmd sources at the subcommand level, not just the binary (git reset / find -delete / gh api -X POST) #5808

Description

@atomantic

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.

Evidenceserver/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

  1. 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.
  2. 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.)
  3. 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.
  4. 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

  • git reset --hard, find . -delete, and gh api -X POST … are rejected by validateUnattendedCommand and dropped by runShellCommand with 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/execute behavior is byte-identical.
  • settings.layeredIntelligence.trustShellSources === true still restores full-shell behavior unchanged.
  • cd server && npm test passes.

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.

Metadata

Metadata

Assignees

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions