Repository navigation
fix(permissions): match deny and ask rules past assignments and wrappers (#4281) - #4284
Conversation
📊 Automated PR Analysis
SummaryFixes RTK's permission gate so deny and ask rules match a command even when it's preceded by an environment variable assignment or wrapped in commands like timeout, nice, nohup, stdbuf, or a bare xargs, aligning it with Claude Code's own matching behavior. It adds a peel_for_matching function that reuses the rewrite's existing prefix-peeling logic plus a new match-only wrapper table for stdbuf/xargs, and applies it to both the deny and ask loops while intentionally leaving allow rules and git's… Review Checklist
Linked issues: #4281 Analyzed automatically by wshm · This is an automated analysis, not a human review. |
c8b1821 to
c9a3f6e
Compare
efff29f to
7f36883
Compare
aeppling
left a comment
There was a problem hiding this comment.
Approve after changes.
The fix is right. Deny and ask rules now use the rewrite's prefix stripping, and allow rules are unchanged. A differential fuzz run against develop found no regression.
Blocking
- Middle-prefix rule missed. Deny
HUSKY=0 git push *doesn't stopnohup HUSKY=0 git push x, and with allownohup *the hook answers allow. Please match against every stripping step, not just the final result. - Quadratic slowdown. Stripping is redone for every rule. 5,000 stacked prefixes with 12 rules take 46 s, against 0.02 s on develop. Compute it once per segment and cap the depth at 10.
Nits
- Docs not updated:
discover/README.md:95, the permission table inhooks/README.md, andsupported-agents.md:194, which still says rules match "the command as written". - Missing test: deny or ask winning over an allow rule, e.g. allow
timeout *plus denygit push *. Develop answered allow here.
7f36883 to
828c44f
Compare
|
Thanks, both blockers are fixed in the new push.
Nits: |
828c44f to
ceeb7b6
Compare
aeppling
left a comment
There was a problem hiding this comment.
Thanks, both blockers are fixed and verified (middle-prefix deny/ask, and 5000 stacked prefixes back to develop speed).
Nit: a quoted assignment with spaces that is a whole statement is read past its first space. With deny Bash(rm -rf *), MSG="chore: rm -rf build dir" && git commit -m "$MSG" is now denied (develop: ask). The assignment regex expects a space after the closing quote, so at the end of a statement it falls back to the unquoted branch. The same misread already makes the rewrite edit inside the string on develop (MSG="fix: git push x" becomes MSG="fix: rtk git push x"), so fixing the regex would cover both.
|
Thanks @aeppling. Confirmed: the quoted-value branch needs a blank after the closing quote, so at the end of a statement it falls back to the unquoted one, in the gate and in develop's rewrite alike. #4136 (in the lexing stack) replaces that regex with |
…ers (#4281) Claude Code matches a deny or ask rule past any leading assignment and past a fixed set of wrappers — `timeout`, `time`, `nice`, `nohup`, `stdbuf`, `command`, `builtin`, `noglob` and a bare `xargs` — so `Bash(git push *)` stops `HUSKY=0 git push` and `timeout 30 git push` as well as `git push`. RTK's gate matched the segment as written and nothing else, so every one of those forms fell through to Default; under a host that treats Default as its own decision, as #3909's OpenClaw does, they auto-apply. The rewrite already looked through the same prefixes: `timeout 30 git push` became `timeout 30 rtk git push` while the gate did not recognise `git push` inside it. That was two readings of which command a line runs, and the gap between them is this bug. The deny and ask loops now also try every reading `matching_readings` gives: the rewrite's own peels — its env peel, its keyword prefixes, its wrapper table — applied one prefix at a time, each assignment and each `env` word a prefix of its own, each intermediate command kept. Where both look through a prefix, the gate and the rewrite now read the same command behind it, and a rule may name any layer: `HUSKY=0 git push *` stops `env HUSKY=0 git push`, and `nohup HUSKY=0 git push` too, where denying is stricter than the shell, since nohup looks for a program named `HUSKY=0`. The readings start after the split's leftover grammar is gone, since `{ HUSKY=0 git push; }` reaches the gate as `{ HUSKY=0 git push`. They are computed once per segment, not once per rule, and stop after 32 prefixes; past that the command behind them is unread, so where a deny or ask rule exists the verdict is at least Ask and an allow rule on the outer wrapper cannot answer for it. With no deny or ask rule, none of this is read. Two wrappers the host strips were not in the rewrite's table, for a reason that still holds: `stdbuf` exists to unbuffer the wrapped command's output, which routing through rtk would buffer, and `xargs` runs whatever it is handed. Recognising a command is not the same decision as rewriting it, so they sit in a match-only table the rewrite never consults. `xargs` is peeled only in its bare form, as the host does; any option makes the wrapper grammar give up. The line is drawn at the shell. `env`, `exec` and `nocorrect` are peeled too, though the host's list stops short of them; each runs the next word as the command. A program's own arguments are not: `git -C . push` stays `git`, as the host documents, since reading past it would mean knowing every program's option grammar. Allow is deliberately left as it was. The host matches allow past known-safe variables only, and peeling a wrapper for it would let `timeout 30 <cmd>` inherit `<cmd>`'s allow rule — so the peel can only make a verdict stricter. The gate reads prefixes with the rewrite's own peels, so it stops where they stop; there it reads the command as before, so no verdict is looser than it was. A line setting more than 32 variables before its command now prompts where a deny or ask rule exists. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ceeb7b6 to
b465def
Compare
Claude Code matches a deny or ask rule past any leading assignment and past a fixed set of wrappers —
timeout,time,nice,nohup,stdbuf,command,builtin,nogloband a barexargs— soBash(git push *)stopsHUSKY=0 git pushandtimeout 30 git pushas well asgit push. RTK's gate matched the segment as written and nothing else, so every one of those forms fell through to Default; under a host that treats Default as its own decision, as #3909's OpenClaw does, they auto-apply.The rewrite already looked through the same prefixes:
timeout 30 git pushbecametimeout 30 rtk git pushwhile the gate did not recognisegit pushinside it. That was two readings of which command a line runs, and the gap between them is this bug. The deny and ask loops now also try every readingmatching_readingsgives: the rewrite's own peels — its env peel, its keyword prefixes, its wrapper table — applied one prefix at a time, each assignment and eachenvword a prefix of its own, each intermediate command kept. Where both look through a prefix, the gate and the rewrite read the same command behind it, and a rule may name any layer:HUSKY=0 git push *stopsenv HUSKY=0 git push, andnohup HUSKY=0 git pushtoo (stricter than the shell, since nohup looks for a program namedHUSKY=0). The readings start after the split's leftover grammar is gone, since{ HUSKY=0 git push; }reaches the gate as{ HUSKY=0 git push.The readings are computed once per segment, not once per rule, and only when a deny or ask rule exists. They stop after 32 prefixes (each assignment counts, so ordinary variable runs stay well inside); past that the command behind them is unread, so the verdict is at least Ask and an allow rule on the outer wrapper cannot answer for it.
Two wrappers the host strips were not in the rewrite's table, for a reason that still holds:
stdbufexists to unbuffer the wrapped command's output, which routing through rtk would buffer, andxargsruns whatever it is handed. Recognising a command is not the same decision as rewriting it, so they sit in a match-only table the rewrite never consults.xargsis peeled only in its bare form, as the host does; any option makes the wrapper grammar give up.The line is drawn at the shell.
env,execandnocorrectare peeled too, though the host's list stops short of them; each runs the next word as the command. A program's own arguments are not:git -C . pushstaysgit, as the host documents, since reading past it would mean knowing every program's option grammar.Allow is deliberately left as it was. The host matches allow past known-safe variables only, and peeling a wrapper for it would let
timeout 30 <cmd>inherit<cmd>'s allow rule — so the peel can only make a verdict stricter.The gate reads prefixes with the rewrite's own peels, so it stops where they stop, and there it reads the command as before: no verdict is looser than on develop. A second, gate-only reading would cover more forms sooner, but it is the same drift this PR removes; the env peel is replaced by one that follows bash's assignment rule in #4136, and the gate follows.
Test plan
cargo fmt --all --check && cargo clippy --all-targets && cargo test --all --no-fail-fastondevelopf53e60337— 4892 passing, 0 failing, clippy clean;wincheck.sh(cfg(windows) type-check) cleanHUSKY=0 nohup timeout 30 git push, empty and quoted values (FOO=,FOO="a b",FOO='a b'), andenv,env FOO=1,exec. Ask fires on three of the same shapes1A=b,=x,FOO-BAR=xand"FOO"=xare commands, not assignments;FOO="x git push y" echo hirunsechoHUSKY=0,timeout 30,niceorenvxargs -n1 git pushandgit -C . push origin mainstay Defaulttrue && { HUSKY=0 git push ; },! timeout 30 git push,( nice git push ); a rule naming the assignment,HUSKY=0 git push *, still matches{ HUSKY=0 git push x ; }nohup HUSKY=0 git push x,env HUSKY=0 git push x,CI=1 HUSKY=0 git push x; deny and ask win over allowtimeout *fortimeout 30 git pushFor #4281. Independent of the lexing stack (#4130–#4138); when the stack rebases over this,
matching_readingsfollows #4136's rename ofstrip_disabled_prefixtosplit_env_prefix.🤖 Generated with Claude Code