fix(hooks): honor permission rules for already-rtk-prefixed commands

Fixes rtk-ai#3152. Claude Code sometimes issues the already-rewritten
form of a command directly (e.g. `rtk grep foo` instead of `grep foo`) once
it has seen it once in an earlier turn. Permission rules like
`Bash(grep *)` are written against the underlying tool, but neither RTK's
own matcher nor the host's native one recognized `rtk grep foo` as a match
for that pattern — RTK's hook fell back to Defer (nothing to rewrite), and
the host's native check independently failed the same way, so both allow
and deny rules silently stopped applying once a command arrived pre-rewritten.

- permissions.rs: strip a leading `rtk ` from each compound-command segment
  before matching against deny/ask/allow patterns, so rules match regardless
  of which form the command arrives in.
- hook_cmd.rs (decide_from_verdict): an Allow verdict with nothing left to
  rewrite is no longer silently discarded as Defer — it's asserted directly,
  since the corrected matching above means it's a real Allow.
- hook_cmd.rs (process_claude_payload): a Deny verdict on an already-rtk
  segment is now asserted explicitly instead of silently deferring to the
  host's native check — deferring only works when the host evaluates text
  that still matches the user's original pattern, which isn't true once the
  segment carries the `rtk ` prefix. This closes a real bypass: a deny rule
  could previously be sidestepped just by the command arriving pre-rewritten.
  The original (non-rtk) command path is untouched — same defer-to-native
  behavior as before.

Verified live against the reporter's exact settings.local.json + rtk-prefixed
grep/ls repro, the already-rtk deny case, and a compound already-rtk `||`
chain. Added regression tests in both files; full suite (2498), clippy, and
fmt all clean.

Author: albatrossflyon-coder

src/hooks/hook_cmd.rs

@@ -266,6 +266,7 @@ fn get_rewritten(cmd: &str) -> Option<String> {
     Some(rewritten)
 }
 
+#[derive(Debug)]
 enum HookDecision {
     AllowRewrite(String),
     AskRewrite(String),
@@ -283,6 +284,14 @@ fn decide_from_verdict(cmd: &str, verdict: PermissionVerdict) -> HookDecision {
     match get_rewritten(cmd) {
         Some(r) if verdict == PermissionVerdict::Allow => HookDecision::AllowRewrite(r),
         Some(r) => HookDecision::AskRewrite(r),
+        // No text to rewrite — most commonly because `cmd` is already in
+        // `rtk …` form (Claude issued it directly after seeing it once).
+        // `verdict` was computed with #3152's already-rtk-aware matching,
+        // so an Allow here is real: assert it instead of silently
+        // deferring to the host's native check, which won't recognize the
+        // rewritten form either and would prompt for something the user
+        // already allowlisted.
+        None if verdict == PermissionVerdict::Allow => HookDecision::AllowRewrite(cmd.to_string()),
         None => HookDecision::Defer,
     }
 }
@@ -617,6 +626,16 @@ enum PayloadAction {
         reason: &'static str,
         cmd: String,
     },
+    /// A deny rule matched a segment that's already in `rtk …` form. Unlike
+    /// the ordinary Skip-on-deny path (which relies on the host's own native
+    /// check catching it), the host's native check evaluates the raw text
+    /// unchanged and has no concept of `rtk` as an alias — it would not
+    /// recognize `rtk rm -rf …` as matching a `Bash(rm:*)` deny rule. Assert
+    /// the deny explicitly instead of silently stepping aside. See #3152.
+    Deny {
+        cmd: String,
+        output: Value,
+    },
     Ignore,
 }
 
@@ -636,6 +655,21 @@ fn claude_payload_input(v: &Value) -> Option<(&Value, &str)> {
     None
 }
 
+/// True if any segment of `cmd` is already in `rtk …` form. Used to decide
+/// whether a Deny verdict is safe to leave to the host's own native
+/// permission check (works for the original, un-rewritten command text)
+/// or must be asserted explicitly (the host's native check evaluates the
+/// raw text unchanged and has no concept of `rtk` as an alias for the
+/// underlying tool). See #3152.
+fn contains_already_rtk_segment(cmd: &str) -> bool {
+    crate::discover::lexer::split_for_permissions(cmd)
+        .iter()
+        .any(|segment| {
+            let segment = segment.trim();
+            segment == "rtk" || segment.starts_with("rtk ")
+        })
+}
+
 fn process_claude_payload(v: &Value) -> PayloadAction {
     let (input, cmd) = match claude_payload_input(v) {
         Some((input, cmd)) => (input, cmd),
@@ -644,10 +678,23 @@ fn process_claude_payload(v: &Value) -> PayloadAction {
 
     let (rewritten, allow) = match decide_hook_action(cmd, permissions::Host::Claude) {
         HookDecision::Deny => {
+            if contains_already_rtk_segment(cmd) {
+                let output = json!({
+                    "hookSpecificOutput": {
+                        "hookEventName": PRE_TOOL_USE_KEY,
+                        "permissionDecision": "deny",
+                        "permissionDecisionReason": "RTK: matches a configured deny rule"
+                    }
+                });
+                return PayloadAction::Deny {
+                    cmd: cmd.to_string(),
+                    output,
+                };
+            }
             return PayloadAction::Skip {
                 reason: "skip:deny_rule",
                 cmd: cmd.to_string(),
-            }
+            };
         }
         HookDecision::Defer => {
             return PayloadAction::Skip {
@@ -721,6 +768,10 @@ pub fn run_claude() -> Result<()> {
         PayloadAction::Skip { reason, cmd } => {
             audit_log(reason, &cmd, "");
         }
+        PayloadAction::Deny { cmd, output } => {
+            audit_log("deny:already_rtk", &cmd, "");
+            let _ = writeln!(io::stdout(), "{output}");
+        }
         PayloadAction::Ignore => {}
     }
 
@@ -733,6 +784,7 @@ fn run_claude_inner(input: &str) -> Option<String> {
     let v: Value = serde_json::from_str(input).ok()?;
     match process_claude_payload(&v) {
         PayloadAction::Rewrite { output, .. } => Some(output.to_string()),
+        PayloadAction::Deny { output, .. } => Some(output.to_string()),
         _ => None,
     }
 }
@@ -1565,6 +1617,26 @@ mod tests {
         assert!(run_claude_inner(&claude_input("rtk git status")).is_none());
     }
 
+    // --- contains_already_rtk_segment (used by the active-deny path, #3152) ---
+
+    #[test]
+    fn test_contains_already_rtk_segment_detects_prefix() {
+        assert!(contains_already_rtk_segment("rtk rm -rf /"));
+        assert!(contains_already_rtk_segment("rtk"));
+        assert!(contains_already_rtk_segment(
+            "git status && rtk rm -rf /tmp/x"
+        ));
+    }
+
+    #[test]
+    fn test_contains_already_rtk_segment_ignores_original_form() {
+        assert!(!contains_already_rtk_segment("rm -rf /"));
+        assert!(!contains_already_rtk_segment("git status && cargo test"));
+        // A tool name that merely starts with "rtk" as a substring, not a
+        // whole segment/prefix, must not false-positive.
+        assert!(!contains_already_rtk_segment("rtkinit --help"));
+    }
+
     #[test]
     fn test_claude_empty_command_passthrough() {
         let input = json!({
@@ -1977,6 +2049,40 @@ mod tests {
         ));
     }
 
+    // --- Regression tests for #3152 ---
+    // An already-rtk command with no text left to rewrite must still assert
+    // Allow when the (now already-rtk-aware) verdict says so, instead of
+    // deferring to the host's native check, which doesn't recognize the
+    // rewritten form and would prompt for something the user allowlisted.
+
+    #[test]
+    fn test_decide_allow_for_already_rtk_command() {
+        let allow = vec!["grep *".to_string()];
+        match decide_with_rules("rtk grep -n foo bar.py", &[], &[], &allow) {
+            HookDecision::AllowRewrite(r) => assert_eq!(r, "rtk grep -n foo bar.py"),
+            other => panic!("expected AllowRewrite(cmd unchanged), got {other:?}"),
+        }
+    }
+
+    #[test]
+    fn test_decide_deny_for_already_rtk_command() {
+        let deny = vec!["rm:*".to_string()];
+        assert!(matches!(
+            decide_with_rules("rtk rm -rf /", &deny, &[], &all_allowed()),
+            HookDecision::Deny
+        ));
+    }
+
+    #[test]
+    fn test_decide_defer_for_already_rtk_command_with_no_matching_rule() {
+        // No allow rule matches "ls" — must stay Defer (ask), not silently allow.
+        let allow = vec!["grep *".to_string()];
+        assert!(matches!(
+            decide_with_rules("rtk ls -la", &[], &[], &allow),
+            HookDecision::Defer
+        ));
+    }
+
     // --- Gemini rendering ---
 
     fn gemini_render(cmd: &str, deny: &[String], ask: &[String], allow: &[String]) -> String {

src/hooks/permissions.rs

@@ -61,7 +61,7 @@ pub(crate) fn check_command_with_rules(
 
     // Deny takes highest priority and pre-empts every other construct.
     for segment in &segments {
-        let segment = segment.trim();
+        let segment = normalize_for_matching(segment.trim());
         for pattern in deny_rules {
             if command_matches_pattern(segment, pattern) {
                 return PermissionVerdict::Deny;
@@ -82,7 +82,7 @@ pub(crate) fn check_command_with_rules(
     let mut saw_segment = false;
 
     for segment in &segments {
-        let segment = segment.trim();
+        let segment = normalize_for_matching(segment.trim());
         if segment.is_empty() {
             continue;
         }
@@ -375,6 +375,20 @@ pub(crate) fn extract_bash_pattern(rule: &str) -> &str {
     rule
 }
 
+/// Strips a leading `rtk ` invocation from a permission-check segment.
+///
+/// Permission rules (e.g. `Bash(grep *)`) are written against the
+/// underlying tool the user actually typed, but once an agent host has
+/// seen RTK's rewritten form it sometimes issues `rtk grep …` directly on
+/// a later turn — a segment that no longer looks like the original tool
+/// invocation to either RTK's own matcher or the host's native one, so
+/// deny/ask/allow rules alike silently stop applying (rtk-ai/rtk#3152).
+/// Normalizing here — right before matching — keeps the deny/ask/allow
+/// checks meaningful regardless of which form the command arrives in.
+fn normalize_for_matching(segment: &str) -> &str {
+    segment.strip_prefix("rtk ").unwrap_or(segment)
+}
+
 /// Check if `cmd` matches a Claude Code permission pattern.
 ///
 /// Pattern forms:
@@ -1166,6 +1180,61 @@ mod tests {
         assert_eq!(out, vec!["git", "npm test", "*"]);
     }
 
+    // --- Regression tests for #3152 ---
+    // Allow/ask/deny rules are written against the underlying tool (e.g.
+    // `Bash(grep *)`), but Claude sometimes issues the already-rewritten
+    // `rtk grep …` form directly on a later turn. Rules must still apply.
+
+    #[test]
+    fn test_already_rtk_matches_allow_rule() {
+        let allow = vec!["grep *".to_string(), "ls *".to_string()];
+        assert_eq!(
+            check_command_with_rules("rtk grep -n foo bar.py", &[], &[], &allow),
+            PermissionVerdict::Allow
+        );
+        assert_eq!(
+            check_command_with_rules("rtk ls -la", &[], &[], &allow),
+            PermissionVerdict::Allow
+        );
+    }
+
+    #[test]
+    fn test_already_rtk_still_matches_deny_rule() {
+        // A deny rule must not be bypassable just because the command
+        // arrives pre-rewritten — this is the security-relevant half of #3152.
+        let deny = vec!["rm:*".to_string()];
+        let allow = vec!["*".to_string()];
+        assert_eq!(
+            check_command_with_rules("rtk rm -rf /", &deny, &[], &allow),
+            PermissionVerdict::Deny
+        );
+    }
+
+    #[test]
+    fn test_already_rtk_still_matches_ask_rule() {
+        let ask = vec!["git push".to_string()];
+        assert_eq!(
+            check_command_with_rules("rtk git push origin main", &[], &ask, &[]),
+            PermissionVerdict::Ask
+        );
+    }
+
+    #[test]
+    fn test_already_rtk_compound_all_segments_checked() {
+        // Mirrors #1213's per-segment requirement: every segment of an
+        // already-rtk compound command must independently match.
+        let allow = vec!["git status *".to_string(), "git status".to_string()];
+        assert_eq!(
+            check_command_with_rules("rtk git status && rtk git add .", &[], &[], &allow),
+            PermissionVerdict::Default,
+            "unallowed second segment must demote the whole chain, even pre-rewritten"
+        );
+        assert_eq!(
+            check_command_with_rules("rtk git status && rtk git status -s", &[], &[], &allow),
+            PermissionVerdict::Allow
+        );
+    }
+
     #[test]
     fn test_wrapped_rules_extracted_patterns_match() {
         let mut allow = Vec::new();