fix(execpolicy): shell grammar and command classification fixes - #6732
Merged
Merged
Conversation
…fter -c `bash -c -e 'cmd'`, `sh -c -- 'cmd'` and `bash -c -o pipefail 'cmd'` run `cmd`: options may follow `-c`, and the command string is the first operand once option parsing ends. The expander took the word right after `-c`, so a deny rule was matched against `-e` / `--` and the real command went unchecked. `shell_input` now keeps scanning options after `-c` (honouring `-o`/`-O` values, `--` and `-`) and reports the first operand, plus the word right after `-c` for shells that read it directly. The command-safety classifier's `-c` peel uses the same rule. Tests: execpolicy unit + integration suites green (unit 222 passed; integration 1 + 5 + 7 passed; doc 1 passed); the new shell_parse_policy entries fail on origin/main. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014ZwqatxgVFxHvovngywnks
…time input `bash /dev/stdin <<< 'cmd'`, `echo cmd | sh /proc/self/fd/0` and `. /dev/stdin <<< 'cmd'` read their script from stdin exactly like `bash <<< 'cmd'` or `bash -s`, which the expander already reports as dynamic. With a script operand present it was treated as an opaque file, so deny rules were never consulted. A shell or `source`/`.` operand naming `/dev/stdin`, `/dev/fd/N` or `/proc/<pid|self>/fd/N` is now dynamic, unless it is stdin and a heredoc body (which is expanded as the script) feeds it. Tests: execpolicy suites green (unit 223 passed; integration 1 + 5 + 7 passed; doc 1 passed); the new shell_parse_policy entries fail without the change. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014ZwqatxgVFxHvovngywnks
Deny rules were matched against the launcher instead of the command it runs for `pkexec`, `run0`, `fakeroot`, `taskset`, `chrt`, `prlimit`, `strace`, `ltrace`, `systemd-run`, `numactl`, `firejail`, `xvfb-run`, `dbus-launch`, `dbus-run-session`, `sg`, `proxychains`, `torsocks`, `eatmydata`, `cpulimit`, `setpriv`, `trickle` and the macOS GNU coreutils spellings (`gtimeout`, `gnice`, `gnohup`, `gstdbuf`, `gchroot`), so `pkexec rm -rf /` or `taskset -c 0 git push` passed a matching deny rule. They are now entries in the existing wrapper table (with their option and operand grammar; unknown options are still read both ways), and `sg GROUP [-c] 'cmd'` hands its joined operands to the shell parser like `watch`. Tests: execpolicy suites green (unit 223 passed; integration 1 + 5 + 7 passed; doc 1 passed); the 23 new shell_parse_policy entries fail without the change. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014ZwqatxgVFxHvovngywnks
…y matcher A typed `action = deny` rule that is not promoted to `denied_prefixes` (for example one scoped with `workspace = ...`) was matched with the allow-direction arity matcher, which requires the subcommand to be spelled literally at the front. `git -C . push`, `git -c a=b push` and `git --no-pager push` therefore did not meet a `git push` deny rule. Typed Deny rules now also match via `denied_prefix_matches`, the matcher the engine already uses for denied prefixes; Ask and Allow rules are unchanged and the workspace scope still applies. Tests: execpolicy suites green (unit 223 passed; integration 1 + 5 + 8 passed; doc 1 passed); the new typed_deny_rule_skips_global_options_before_the_subcommand fails without the change. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014ZwqatxgVFxHvovngywnks
`deny = ["git push --force"]` only denied the exact text `git push --force`: deny patterns were whole-command anchored globs, so `git push --force origin main` and `rm -rf / --no-preserve-root` fell through to AskUser, which auto-approving postures may grant. Deny patterns now also match through `denied_prefix_matches`, the permission engine's deny matcher (prefix at a word boundary, global options skipped), with the glob kept for patterns that spell `*`. That matcher also lets a rule's option appear after positionals, since most CLIs permute arguments: a `git push --force` rule now covers `git push origin main --force`, while the command word and the rule's own positionals stay anchored. The `compiled_glob` doc claimed `*` crosses newlines; it does not, and the doc now says so (behavior unchanged, so allow globs do not widen). Tests: execpolicy suites green (unit 225 passed; integration 1 + 5 + 8 passed; doc 1 passed); deny_pattern_is_a_command_prefix fails without the change. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014ZwqatxgVFxHvovngywnks
… the default registry The agent read-only grammar admitted `npm view|show|info` with any arguments, while `readonly_network_reads` reports the host as `registry.npmjs.org`. `npm view x --registry=https://other.example/` was therefore approved and checked against the wrong host, and `--cache` / `--userconfig` pointed npm at arbitrary files. The grammar now admits only `--json` as an option and refuses package specs that name a URL, a git or file source, or a local path; a refused npm read is reported as an option refusal. Tests: execpolicy suites green (unit 225 passed; integration 1 + 5 + 8 passed; doc 1 passed); the new refusal and network-read assertions fail without the change. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014ZwqatxgVFxHvovngywnks
…ad-only arguments
`analyze_command` rated a command `Safe` when its text merely started with
an entry of the safe table, so `cdk deploy`, `psql -c ...`, `setsid curl`
and `topgrade -y` read as `cd`, `ps`, `set` and `top`, and entries that run
their arguments or write files (`env CMD`, `awk 'BEGIN{system(...)}'`,
`sed -i`, `find -exec`, `git branch -D`, `git remote set-url`,
`git diff --output=...`) were `Safe` with any arguments. Auto-Review treats
`Safe`/`WorkspaceSafe` as routine and approval offers a persistent grant on
that verdict.
The safe and workspace-safe tables now match word for word, and each entry
that can execute or write is limited to its read-only forms: `env` lists
only, `find`/`sed` reuse the agent read-only grammar, `awk` leaves the
table, `rg --pre`, `fd -x/--exec`, `man -P/-H/-C`, `less +cmd`, a second
`uniq` operand and `hostname NAME` are refused, `git branch`/`tag`/`remote`
admit listing forms only, and `git diff`/`log`/`show` refuse `--output`.
`touch` and `mkdir` are workspace-safe only for workspace-relative
operands, as `cp`/`mv` already were. Refused forms fall back to
RequiresApproval.
Tests: execpolicy suites green (unit 226 passed; integration 1 + 5 + 8
passed; doc 1 passed); clippy -D warnings clean for the crate;
safe_classification_needs_whole_words_and_read_only_arguments fails
without the change.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014ZwqatxgVFxHvovngywnks
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
npm view/show/info cannot be admitted as registry-only reads from argv: project/user config and environment can redirect registry traffic, and npm package specs can name Git, remote archives, or local files. Remove their automatic Safe/read-only admission while preserving ordinary explicitly approved shell execution. Keep potential npm network detection independent of admission so network-denied children continue to fail closed. Extend classifier, shell, registry, and subagent regressions for ordinary, scoped, --json, Git, file, and configured-registry forms. Preserve the original PR #6732 commits and their author/session attribution. Verified by the coordinating root on this frozen four-file source: - codewhale-execpolicy: 227 unit + 14 integration + 1 doc = 242 passed, 0 failed. - Five focused TUI policy/network consumer tests: 5 passed, 0 failed. - npm test: 636 passed (68 + 16 + 50 + 502), 0 failed. - npm run check:web: passed, including 839 generated pages. - cargo fmt --all -- --check and git diff --check: passed. Logs: /private/tmp/cw-6732-{execpolicy,tui,npm,web}-root.log. Hosted CI must qualify the updated PR head separately; no provider calls.
Remove the stale npm view allowance from subagent and Fleet guidance. Explain why a network grant does not authorize npm executable helpers. Validation: npm test636passed/0failed; check:web passed. The already qualified Rust implementation and test inputs are unchanged. Signed-off-by: CodeWhale Bot <bot@codewhale.net>
pull Bot
pushed a commit
to jw5812018/DeepSeek-TUI
that referenced
this pull request
Sep 29, 2026
Prepare a bounded five-path execpolicy follow-up to PR Hmbown#6732. Source implementation and validation remain pending; no passing gate is claimed.
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.
No-Issue: verified bug-hunt findings
Shell-grammar and command-classification fixes in
crates/execpolicy. Each finding was first re-checked onorigin/main(1cdec0f) with a scratch harness callingExecPolicyEngine::check,ExecPolicyConfig::evaluate,analyze_commandandagent_readonly_verdict. All of them reproduced. Every regression test below also fails when its source change is reverted.-cis the first operand once options end:bash -c -e 'x',sh -c -- 'x',bash -c -o pipefail 'x'. The fish-style next word is still considered too. The command-safety-cpeel follows the same rule.shell_expand::options_after_dash_c_do_not_hide_the_command_string, newHIDDEN_RMentries intests/shell_parse_policy.rs, and newwrappers_and_path_spelled_binaries_are_unwrappedcases/dev/stdin,/dev/fd/Nand/proc/<pid|self>/fd/N(for shells andsource/.) are read at run time and marked dynamic. The exception is a heredoc on stdin, whose body is already expanded as the script.shell_expand::script_operand_naming_a_descriptor_is_read_at_run_time,HIDDEN_RMentriessg GROUP [-c] 'cmd'is parsed likewatch.HIDDEN_RMentriesaction = denyrules, including workspace-scoped ones, now match throughdenied_prefix_matches. That meansgit -C . push,git -c a=b pushandgit --no-pager pushmeet agit pushrule.typed_deny_rule_skips_global_options_before_the_subcommandexecpolicy.tomldeny patterns are command prefixes (git push --forcealso coversgit push --force origin main). A rule option may come after positionals (git push origin main --force). Thecompiled_globdoc now describes its real newline behavior.toml_rules::deny_pattern_is_a_command_prefix,deny_rule_options_may_follow_positionalsnpm view,show, andinforequire ordinary shell approval. Their registry can come from project/user config or environment, and package specs can resolve to Git, archives, or local paths. Potential npm network detection remains active for network-denied children.SafeandWorkspaceSafetables match whole words, socdk,psql,setsidandtopgradeno longer pass as safe words. Entries that can run their arguments or write files are limited to read-only forms (env, find, sed, rg --pre, fd -x, man -P, less +, uniq, hostname, git branch/tag/remote/diff/log/show).awkis removed from the table.touch/mkdirneed workspace-relative operands.safe_classification_needs_whole_words_and_read_only_argumentsLatest local verification for the four-file npm authority repair (coordinating root, frozen source at bbb2cb5):
npm test: 636 passed (68 + 16 + 50 + 502), 0 failed.npm run check:web: passed, including 839 generated pages.Original-author verification before the follow-up repair remains recorded in the original commits: execpolicy 241 tests, clippy, dead-code budget, and 647 TUI consumer tests passed (1 ignored). Those broader TUI and clippy checks were not repeated after the npm repair. Hosted CI must qualify the final PR head separately; no provider calls were made.
Scope notes:
lib.rs(typed Deny matcher, rule-option permutation) is shared by denied prefixes. It only affects deny matching.askrules still use the allow-direction matcher.bwrap, whose options take two values, is not in the wrapper table.🤖 Generated with Claude Code
https://claude.ai/code/session_014ZwqatxgVFxHvovngywnks
Documentation follow-up: Subagents and Fleet now describe the implemented refusal of
npm view, including why a network grant does not authorize npm executable helpers. Root npm test636passed/0failed and check:web passed. This follow-up changes only those two guides; previously recorded Rust implementation and test inputs are unchanged.