fix: hook security + stderr redirects + version bump#807
Closed
pszymkowiak wants to merge 10 commits intomasterfrom
Closed
fix: hook security + stderr redirects + version bump#807pszymkowiak wants to merge 10 commits intomasterfrom
pszymkowiak wants to merge 10 commits intomasterfrom
Conversation
The PreToolUse hook was emitting `permissionDecision: "allow"` on every rewritten command, bypassing deny and ask rules in .claude/settings.json. - Add `src/permissions.rs`: loads Bash deny/ask rules from all 4 Claude Code settings files (project + global, settings.json + settings.local.json), checks commands (including compound && / || / | / ;) and returns Allow / Deny / Ask verdict. 16 unit tests. - Modify `src/rewrite_cmd.rs`: after finding a rewrite, check the original command against permissions. Exit 0 = allow (auto-approve rewrite), exit 2 = deny (passthrough, let CC native deny handle it), exit 3 = ask (print rewrite but no permissionDecision, CC prompts user). - Update both hook files to handle exit codes 2 and 3. Version bumped 2→3. - Bump `CURRENT_HOOK_VERSION` 2→3 in `hook_check.rs` so users with the old hook get the upgrade prompt. - Fix set -euo pipefail bug in .claude/hooks/rtk-rewrite.sh: capture exit code with `|| EXIT_CODE=$?` instead of bare assignment. Fixes #260 Signed-off-by: Florian BRUNIAUX <florian@bruniaux.com>
Bug 1 (Critical): check_command() was called inside Some(rewritten), so non-RTK commands (rm, kill, python3 -c) bypassed deny rules entirely. Move verdict check before registry::rewrite_command() so all commands are evaluated regardless of whether RTK has an equivalent. Bug 4 (Medium): print!() before process::exit() could leave stdout unflushed. Add explicit std::io::stdout().flush() after each print!(). Add Eq derive to PermissionVerdict (required for == comparison). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Florian BRUNIAUX <florian@bruniaux.com>
Bug 2 (Critical): *:* catch-all matched nothing. strip_suffix('*') left
"*:" which after trim became "*" (non-empty), so the branch returned
false instead of true. Fix: detect empty-or-star prefix after stripping.
Bug 3 (Medium): leading wildcards ("* --force"), middle wildcards
("git * main"), and multi-wildcard patterns ("git * --force *") fell
through to exact match, silently failing. Add glob_matches() with
character-level segment anchoring: first segment must be prefix, last
must be suffix, middle segments found via str::find in order.
Colon normalization in glob_matches(): "sudo:*" -> "sudo *" so both
fast path and glob path interpret colon syntax consistently.
New tests: test_star_colon_star_matches_everything,
test_leading_wildcard, test_leading_wildcard_no_partial,
test_middle_wildcard, test_middle_wildcard_no_match,
test_multiple_wildcards, test_deny_with_leading_wildcard,
test_deny_star_colon_star.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Florian BRUNIAUX <florian@bruniaux.com>
…ormat Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Florian BRUNIAUX <florian@bruniaux.com>
fix(hook): respect Claude Code deny/ask permission rules on rewrite
Commands like `git status 2>&1` or `cargo test 2>/dev/null` were not rewritten because the redirect suffix prevented pattern matching. Now strips redirects (2>&1, 2>/dev/null, etc.) before matching, then re-appends them to the rewritten command. Signed-off-by: Patrick szymkowiak <patrick.szymkowiak@innovtech.eu>
fix: strip trailing stderr redirects before rewrite matching (#530)
Sync manifest and Cargo.toml with released v0.33.0. Signed-off-by: Patrick szymkowiak <patrick.szymkowiak@innovtech.eu>
chore: bump version to 0.33.0
Signed-off-by: Patrick szymkowiak <patrick.szymkowiak@innovtech.eu>
FlorianBruniaux
approved these changes
Mar 25, 2026
Collaborator
FlorianBruniaux
left a comment
FlorianBruniaux
left a comment
There was a problem hiding this comment.
Reviewed and tested locally. Here's what I found.
Build + tests: clean build, 1114 tests pass, 0 clippy errors.
Security scan: no dangerous patterns (no shell injection, no network calls, no unsafe blocks, no obfuscated strings). Zero new dependencies — Cargo.toml is version bump only.
permissions.rs (the core of this PR): reads Claude Code settings.json files in the right priority order, extracts only Bash-scoped rules, checks deny before ask, handles wildcards and compound commands correctly. 30+ unit tests cover the edge cases well. One minor note: split_compound_command doesn't parse quotes, so git commit -m "fix || bug" would split on the || inside the message. This over-checks segments rather than under-checks — conservative failure mode, not a security issue.
Hook changes: the 4-exit-code protocol is correctly implemented. Exit 2 passes through without auto-allow, exit 3 rewrites but omits permissionDecision so Claude Code prompts. Solid.
Redirect stripping (registry.rs): regex handles quotes conservatively. False positive = no-strip, not a missed deny rule. Safe.
Approving. Good fix for a real gap.
pszymkowiak
added a commit
that referenced
this pull request
Mar 25, 2026
* fix(hook): respect Claude Code deny/ask permission rules on rewrite * fix(permissions): check deny rules before rewrite + flush stdout * fix(permissions): support *:* and leading/middle wildcards * fix: strip trailing stderr redirects before rewrite matching (#530) * chore: bump version to 0.33.0 Signed-off-by: Patrick szymkowiak <patrick.szymkowiak@innovtech.eu>
Collaborator
Author
|
Squash merged manually into master as 0649e97. Release-please will create v0.33.1. |
This was referenced Mar 25, 2026
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
2 participants
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
2>&1,2>/dev/nullbefore rewrite matchingImportant
Squash merge this PR — release-please needs a conventional commit title on master.
Test plan