fix(shell): demote destructive flags on allowlisted commands#258
Merged
Conversation
Allowlist matched on leading tokens only, so `git branch -D`, `find -delete`, `git diff --output=...`, `npx eslint --fix`, etc. flowed through without confirmation. Add a per-prefix RISKY_ARGS table that demotes specific tail tokens back to the confirm gate; both `--flag value` and `--flag=value` forms are caught. Chains inherit via the existing per-segment isAllowed call.
Follow-up audit caught two more write-anywhere paths on otherwise- allowlisted inspection tools: `tree -o FILE` writes the tree output, and `find -fprint0 FILE` writes NUL-separated paths. Same risk class as the `git diff --output=` family already covered.
This was referenced May 5, 2026
ChasLui
pushed a commit
to ChasLui/DeepSeek-Reasonix
that referenced
this pull request
May 23, 2026
…e#258) * fix(shell): demote destructive flags on allowlisted commands (esengine#257) Allowlist matched on leading tokens only, so `git branch -D`, `find -delete`, `git diff --output=...`, `npx eslint --fix`, etc. flowed through without confirmation. Add a per-prefix RISKY_ARGS table that demotes specific tail tokens back to the confirm gate; both `--flag value` and `--flag=value` forms are caught. Chains inherit via the existing per-segment isAllowed call. * fix(shell): also demote tree -o and find -fprint0 (esengine#257) Follow-up audit caught two more write-anywhere paths on otherwise- allowlisted inspection tools: `tree -o FILE` writes the tree output, and `find -fprint0 FILE` writes NUL-separated paths. Same risk class as the `git diff --output=` family already covered.
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
1 participant
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
git branch -dbypasses review mode #257 —git branch -D(and similar destructive flags on otherwise-allowlisted commands) skipped review mode because the allowlist match looked at the leading tokens only.RISKY_ARGStable insrc/tools/shell.ts. When an allowlisted prefix matches but a tail token is in that prefix's risky list, the call falls back to the confirm gate. Read-only forms (git branch,git branch -v,find -name '*.ts',npx eslint src, …) stay on the fast path.BUILTIN_ALLOWLISTfor the same class of bypass while in there. New rules cover:git branch,git remote,git diff/log/show(--output,--ext-diff),find(-delete,-exec*,-ok*,-fprint*),npx eslint --fix,npx biome check --write/--apply*,ruff --fix/--unsafe-fixes/format.--flag valueand--flag=valueforms are caught, sogit diff --output=x.patchandgit diff --output x.patchboth demote.a && b,a | b) inherit automatically —chainAllowedalready callsisAllowedper segment.npm test/pytest/cargo testetc. are intentionally not gated further: the contract of these allowlist entries is "run user code", and no flag-level filter would meaningfully constrain that.Test plan
npm run verify(build + lint + typecheck + 2260 tests, all pass)risky-arg demotiondescribe block intests/shell-tools.test.ts— 5 groups covering eachRISKY_ARGSprefix with happy + sad paths, including the--flag=valueformgit branch -D feature/foonow hits the confirm gate instead of executing immediately