fix(hooks): add rtk run to wrappers; never assert for wrapped invocations

Round-4 review fixes on the rtk-ai#3195 adoption:

- COMMAND_WRAPPERS missed 'run' — the one subcommand that shells out its
  argument via sh -c — so 'rtk run rm -rf /' evaded a Bash(rm:*) deny
  while Bash(rtk:*) allow-asserted it. 'run' is now stripped like proxy,
  and the exhaustiveness doc is corrected ('check' lives under rtk hook,
  not top-level).
- Adding the token is not sufficient: wrapper arguments are arbitrary
  text ('rtk run -c "rm -rf /"', quoted args in any wrapper) that
  token-literal matching cannot decompose. New total rule: wrapped
  invocations are matched best-effort for deny/ask (over-firing is
  fail-safe) but are excluded from the hook's assert gates entirely
  (is_wrapped_invocation) — RTK never speaks for text it cannot attest.
- The unattestable-construct branch pre-empted the already-rtk ask
  assertion: 'rtk git push > f' silently deferred into a host-native
  rtk:* allowlist. Already-rtk unattestables now assert ask (forced
  prompt); plain ones keep the silent defer.
- test_claude_already_rtk_passthrough migrated to injected rules — since
  rtk-ai#3152 its outcome depends on permission rules, and reading the real
  on-disk settings made it environment-dependent.
- tests: run-wrapper deny, never-assert sweep across wrapper shapes,
  unattestable ask assertion, is_wrapped_invocation contract.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: kylehgc

src/hooks/hook_cmd.rs

@@ -279,6 +279,15 @@ fn decide_from_verdict(cmd: &str, verdict: PermissionVerdict) -> HookDecision {
         return HookDecision::Deny;
     }
     if crate::discover::lexer::contains_unattestable_construct(cmd) {
+        // RTK can't attest what a substitution / file redirect really does,
+        // so it never asserts allow here. For an already-rtk command a
+        // silent defer would hand the decision to a host-native
+        // `Bash(rtk:*)` allowlist that is blind to the underlying tool —
+        // force the prompt by asserting ask instead. Plain commands keep
+        // the pre-existing silent defer.
+        if contains_already_rtk_segment(cmd) {
+            return HookDecision::AskRewrite(cmd.to_string());
+        }
         return HookDecision::Defer;
     }
     match get_rewritten(cmd) {
@@ -703,7 +712,10 @@ fn contains_already_rtk_segment(cmd: &str) -> bool {
 }
 
 /// True when `cmd` has at least one non-empty segment and every non-empty
-/// segment is rtk-prefixed. See the quantifier note above.
+/// segment is rtk-prefixed AND directly names its tool (not a wrapper like
+/// `rtk proxy`/`rtk run`, whose arbitrary argument text RTK cannot fully
+/// attest — see `permissions::is_wrapped_invocation`). See the quantifier
+/// note above.
 fn all_segments_already_rtk(cmd: &str) -> bool {
     let segments = crate::discover::lexer::split_for_permissions(cmd);
     let mut saw_segment = false;
@@ -712,7 +724,7 @@ fn all_segments_already_rtk(cmd: &str) -> bool {
         if segment.is_empty() {
             continue;
         }
-        if !permissions::is_rtk_prefixed(segment) {
+        if !permissions::is_rtk_prefixed(segment) || permissions::is_wrapped_invocation(segment) {
             return false;
         }
         saw_segment = true;
@@ -1671,7 +1683,24 @@ mod tests {
 
     #[test]
     fn test_claude_already_rtk_passthrough() {
-        assert!(run_claude_inner(&claude_input("rtk git status")).is_none());
+        // No rules → Default verdict → already-rtk command still defers.
+        // Driven through the injected decision path: since #3152 the
+        // outcome depends on permission rules, so reading the real on-disk
+        // settings here would make the test environment-dependent.
+        let input = json!({"tool_name": "Bash", "tool_input": {"command": "rtk git status"}});
+        let action = process_claude_payload_impl(&input, |cmd| {
+            decide_from_verdict(
+                cmd,
+                permissions::check_command_with_rules(cmd, &[], &[], &[]),
+            )
+        });
+        assert!(matches!(
+            action,
+            PayloadAction::Skip {
+                reason: "skip:defer",
+                ..
+            }
+        ));
     }
 
     // --- contains_already_rtk_segment (used by the active-deny path, #3152) ---
@@ -2210,6 +2239,7 @@ mod tests {
         let allow = vec!["rtk:*".to_string()];
         for cmd in [
             "rtk proxy rm -rf /",
+            "rtk run rm -rf /",
             "rtk err rm -rf /",
             "rtk test rm -rf /",
             "rtk summary rm -rf /",
@@ -2225,15 +2255,60 @@ mod tests {
         }
     }
 
+    #[test]
+    fn test_wrapped_invocations_never_asserted() {
+        // Wrapper arguments are arbitrary (`rtk run` takes a whole shell
+        // string; quoting defeats token matching in any wrapper), so RTK
+        // must never speak for them: deny stays best-effort, allow/ask
+        // always defer — even under a catch-all allow rule.
+        let allow = vec!["*".to_string()];
+        for cmd in [
+            "rtk run rm -rf /",
+            "rtk run -c \"rm -rf /\"",
+            "rtk run \"rm -rf /\"",
+            "rtk proxy \"rm\" -rf /",
+            "rtk proxy rm -rf /",
+            "rtk err rm -rf /",
+        ] {
+            assert!(
+                matches!(
+                    decide_with_rules(cmd, &[], &[], &allow),
+                    HookDecision::Defer
+                ),
+                "{cmd} must defer, never be asserted"
+            );
+        }
+    }
+
+    #[test]
+    fn test_unattestable_already_rtk_asserts_ask() {
+        // A redirect/substitution on an already-rtk command can't be
+        // attested; silence would let a host-native `Bash(rtk:*)`
+        // allowlist auto-run it, so the ask must be asserted.
+        let allow = vec!["*".to_string()];
+        match decide_with_rules("rtk git log > /tmp/out.txt", &[], &[], &allow) {
+            HookDecision::AskRewrite(r) => assert_eq!(r, "rtk git log > /tmp/out.txt"),
+            other => panic!("expected AskRewrite(cmd unchanged), got {other:?}"),
+        }
+        // The plain (non-rtk) unattestable path keeps its silent defer.
+        assert!(matches!(
+            decide_with_rules("git log > /tmp/out.txt", &[], &[], &allow),
+            HookDecision::Defer
+        ));
+    }
+
     #[test]
     fn test_probe_shapes_fail_safe() {
         // Shapes that must never reach the already-rtk assert paths: each
         // stays out of the gates so the host's native matcher (or its
         // prompt) remains authoritative.
         let allow = vec!["*".to_string()];
+        // The lexer surfaces the rtk token inside the substitution, so this
+        // lands on the unattestable+already-rtk arm: an asserted ask
+        // (forced prompt) rather than a silent defer — never an allow.
         assert!(matches!(
             decide_with_rules("$(rtk rm -rf /)", &[], &[], &allow),
-            HookDecision::Defer
+            HookDecision::AskRewrite(_)
         ));
         assert!(!all_segments_already_rtk("FOO=1 rtk rm -rf /"));
         assert!(!all_segments_already_rtk("'rtk' rm -rf /"));

src/hooks/permissions.rs

@@ -410,15 +410,41 @@ pub(crate) fn normalize_for_matching(segment: &str) -> &str {
     }
 }
 
-/// RTK subcommands whose trailing arguments are themselves the executed
-/// command: `rtk proxy <cmd>` runs `<cmd>` raw; `rtk err <cmd>`,
-/// `rtk test <cmd>`, and `rtk summary <cmd>` run `<cmd>` and filter its
-/// output. For permission matching the effective command is the wrapped
-/// one, so a `Bash(rm:*)` deny must see through `rtk proxy rm -rf /`.
-/// This list is exhaustive over `main.rs`'s `Commands` enum: every other
-/// variant either proxies the tool it is named after (`rtk grep …` IS
-/// `grep …`) or never executes its argument (`rtk check`, `rtk rewrite`).
-const COMMAND_WRAPPERS: [&str; 4] = ["proxy", "err", "test", "summary"];
+/// RTK subcommands whose arguments are themselves the executed command:
+/// `rtk proxy <cmd>` runs `<cmd>` raw, `rtk run …` shells out its string
+/// via `sh -c`, and `rtk err/test/summary <cmd>` run `<cmd>` and filter
+/// its output. For permission matching the effective command is the
+/// wrapped one, so deny/ask rules are matched against the stripped form —
+/// best-effort only: matching is token-literal, and quoted or string-form
+/// arguments (`rtk run -c "rm -rf /"`, `rtk proxy "rm" -rf /`) may not
+/// match. That is why wrapped invocations are also excluded from the
+/// hook's assert gates via [`is_wrapped_invocation`]: over-firing a deny
+/// is fail-safe, asserting an allow for text RTK cannot decompose is not.
+/// This list is exhaustive over `main.rs`'s top-level `Commands` enum:
+/// every other variant either proxies the tool it is named after
+/// (`rtk grep …` IS `grep …`) or never executes its argument
+/// (`rtk rewrite`, `rtk hook check`).
+const COMMAND_WRAPPERS: [&str; 5] = ["proxy", "run", "err", "test", "summary"];
+
+/// True when `segment` reaches its executed command through one of
+/// [`COMMAND_WRAPPERS`]. Such segments are matched best-effort for
+/// deny/ask rules but must never be allow/ask-ASSERTED by the hook: the
+/// wrapper's argument is arbitrary text RTK cannot fully attest.
+pub(crate) fn is_wrapped_invocation(segment: &str) -> bool {
+    let mut s = segment;
+    loop {
+        let Some(after_rtk) = strip_token(s, "rtk") else {
+            return false;
+        };
+        if COMMAND_WRAPPERS
+            .iter()
+            .any(|wrapper| strip_token(after_rtk, wrapper).is_some())
+        {
+            return true;
+        }
+        s = after_rtk;
+    }
+}
 
 /// Strips `token` from the start of `s` when it is followed by at least
 /// one ASCII IFS whitespace character (space, tab, CR, LF) and more text;
@@ -1318,6 +1344,30 @@ mod tests {
         assert_eq!(normalize_for_matching("rtk proxy"), "proxy");
     }
 
+    #[test]
+    fn test_is_wrapped_invocation_contract() {
+        assert!(is_wrapped_invocation("rtk proxy rm -rf /"));
+        assert!(is_wrapped_invocation("rtk run -c \"rm -rf /\""));
+        assert!(is_wrapped_invocation("rtk rtk proxy rm"));
+        assert!(is_wrapped_invocation("rtk err cargo build"));
+        assert!(!is_wrapped_invocation("rtk grep foo"));
+        assert!(!is_wrapped_invocation("proxy rm -rf /"));
+        assert!(!is_wrapped_invocation("rtk proxy"));
+        assert!(!is_wrapped_invocation("rm -rf /"));
+        assert!(!is_wrapped_invocation("rtk proxyfoo bar"));
+    }
+
+    #[test]
+    fn test_run_wrapper_deny_rule_fires() {
+        // `rtk run <cmd>` shells out via sh -c — same class as proxy.
+        let deny = vec!["rm:*".to_string()];
+        let allow = vec!["rtk:*".to_string()];
+        assert_eq!(
+            check_command_with_rules("rtk run rm -rf /", &deny, &[], &allow),
+            PermissionVerdict::Deny
+        );
+    }
+
     #[test]
     fn test_wrapped_command_deny_rule_fires() {
         // Security half of the wrapper strip: a deny against the wrapped