From 4d37aa8b9e5c4aa6cf7f3a086eaeebb8e698e9ea Mon Sep 17 00:00:00 2001 From: Adrien EPPLING Date: Mon, 12 Oct 2026 21:04:26 +0200 Subject: [PATCH] fix(hook): defer when a prefixed shell wrapper's script hits a deny rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #4126 made the rewrite descend through prefixes (env, assignments, timeout, nice, nohup, time, exec, command, uv run, configured transparent_prefixes) to a quoted shell wrapper, but the permission gate still looked for the wrapper at the segment start only. A denied inner command rode the outer allow: `env bash -c 'git push'` under `allow env:*` + `deny git push:*` was rewritten and auto-allowed. The gate now checks deny rules against the wrapper's script at every position the rewrite reaches (`registry::shell_wrapper_positions`, the rewrite's own prefix walk), on the text the rewrite reads: continuations joined, segments not cut at a redirect (`lexer::split_for_wrappers`). Inside the script the same walk is applied before matching, so a prefix there (`exec git push`, `timeout 5 git push`, `FOO=1 git push`) reaches the rule too. A match returns the new `PermissionVerdict::Defer`: no rewrite, no decision, the host sees the command as typed — master's behaviour, instead of #4126's Deny. Nothing else changes: a prefixed wrapper otherwise keeps the outer command's verdict, and a wrapper at the segment start keeps develop's rule. Fixes the release blocker reported on #4399. Co-Authored-By: Claude Fable 5.1 --- src/discover/lexer.rs | 10 +- src/discover/mod.rs | 21 ++- src/discover/registry.rs | 113 +++++++++++- src/hooks/README.md | 20 +- src/hooks/decision.rs | 21 ++- src/hooks/permissions.rs | 386 +++++++++++++++++++++++++++++++++++---- src/hooks/rewrite_cmd.rs | 13 +- src/main.rs | 5 +- 8 files changed, 542 insertions(+), 47 deletions(-) diff --git a/src/discover/lexer.rs b/src/discover/lexer.rs index 1140564bec..823cb6bc0d 100644 --- a/src/discover/lexer.rs +++ b/src/discover/lexer.rs @@ -682,6 +682,14 @@ pub(crate) fn redirect_has_file_target(tokens: &[ParsedToken], i: usize) -> bool /// hiding behind a construct another segmenter would leave intact. /// Callers must still gate on [`contains_unattestable_construct`] first. pub fn split_for_permissions(cmd: &str) -> Vec<&str> { + split_segments(cmd, true) +} + +pub fn split_for_wrappers(cmd: &str) -> Vec<&str> { + split_segments(cmd, false) +} + +fn split_segments(cmd: &str, truncate_at_redirect: bool) -> Vec<&str> { let trimmed = cmd.trim(); if trimmed.is_empty() { return vec![]; @@ -734,7 +742,7 @@ pub fn split_for_permissions(cmd: &str) -> Vec<&str> { seg_start = end; i = next; continue; - } else if seg_end.is_none() { + } else if truncate_at_redirect && seg_end.is_none() { seg_end = Some(tok.offset); } } else if tok.kind == TokenKind::Arg diff --git a/src/discover/mod.rs b/src/discover/mod.rs index 469046aff9..5e655d6df8 100644 --- a/src/discover/mod.rs +++ b/src/discover/mod.rs @@ -164,11 +164,12 @@ fn estimate_hook_coverage(permission_cmd: &str, rewrite_cmd: &str, ctx: &Coverag if !ctx.hook_installed { return false; } - let verdict = permissions::check_command_with_rules( + let verdict = permissions::check_command_with_rules_and_prefixes( permission_cmd, &ctx.rules.deny, &ctx.rules.ask, &ctx.rules.allow, + &ctx.normalized_transparent_prefixes, ); estimate_hook_coverage_with_verdict(permission_cmd, rewrite_cmd, verdict, ctx) } @@ -186,7 +187,7 @@ fn estimate_hook_coverage_with_verdict( ctx: &CoverageContext, ) -> bool { ctx.hook_installed - && verdict != PermissionVerdict::Deny + && !matches!(verdict, PermissionVerdict::Deny | PermissionVerdict::Defer) && !lexer::contains_unattestable_construct(permission_cmd) && registry::rewrite_command_precompiled( rewrite_cmd, @@ -730,6 +731,22 @@ mod tests { )); } + #[test] + fn test_estimate_hook_coverage_false_for_deferred_wrapper() { + assert!(!estimate_hook_coverage_with_verdict( + "env bash -c 'git push'", + "env bash -c 'git push'", + PermissionVerdict::Defer, + &test_ctx(true), + )); + assert!(estimate_hook_coverage_with_verdict( + "env bash -c 'git push'", + "env bash -c 'git push'", + PermissionVerdict::Ask, + &test_ctx(true), + )); + } + #[test] fn test_estimate_hook_coverage_true_for_rewritable_command() { let ctx = test_ctx(true); diff --git a/src/discover/registry.rs b/src/discover/registry.rs index 47416d44bc..e4d2d5381a 100644 --- a/src/discover/registry.rs +++ b/src/discover/registry.rs @@ -733,7 +733,7 @@ static BASH_JOIN_RE: LazyLock = LazyLock::new(|| Regex::new(r"\\\r?\n").u /// Replace every bash line continuation with a single space, mirroring what /// bash does before dispatching the command. Returns a borrowed `&str` when the /// input contains no continuations, so the common fast path allocates nothing. -fn collapse_line_continuations(s: &str) -> std::borrow::Cow<'_, str> { +pub(crate) fn collapse_line_continuations(s: &str) -> std::borrow::Cow<'_, str> { LINE_CONTINUATION_RE.replace_all(s, " ") } @@ -1669,6 +1669,74 @@ fn is_excluded(cmd: &str, excluded: &[ExcludePattern]) -> bool { }) } +/// Every position in `seg` where [`rewrite_segment_inner`] would try a quoted +/// shell wrapper: the segment itself and each tail reached by stripping one +/// prefix the rewriter is transparent to, following the same branches and +/// depth bound. The permission gate checks exactly these, so a wrapper a +/// rewrite can reach is never auto-allowed on the outer rule, and a shell name +/// that is merely an argument (`grep -rn bash -c src`) is never a wrapper. +pub(crate) fn shell_wrapper_positions<'a>( + seg: &'a str, + normalized_transparent_prefixes: &[String], +) -> Vec<&'a str> { + let mut positions = Vec::new(); + collect_shell_wrapper_positions(seg, normalized_transparent_prefixes, 0, &mut positions); + positions +} + +fn collect_shell_wrapper_positions<'a>( + seg: &'a str, + transparent_prefixes: &[String], + depth: usize, + positions: &mut Vec<&'a str>, +) { + let trimmed = seg.trim(); + if trimmed.is_empty() { + return; + } + positions.push(trimmed); + if depth >= MAX_PREFIX_DEPTH { + return; + } + + let (env_prefix, rest_after_env) = strip_disabled_prefix(trimmed); + if !env_prefix.is_empty() { + if env_prefix.contains("RTK_DISABLED=") { + return; + } + collect_shell_wrapper_positions(rest_after_env, transparent_prefixes, depth + 1, positions); + return; + } + + for (prefix, routable) in builtin_transparent_prefixes() { + if let Some(rest) = strip_word_prefix(trimmed, prefix) { + if rest.is_empty() { + return; + } + collect_shell_wrapper_positions(rest, transparent_prefixes, depth + 1, positions); + if !routable { + return; + } + break; + } + } + + if let Some((_, rest)) = strip_process_wrapper_prefix(trimmed) { + collect_shell_wrapper_positions(rest, transparent_prefixes, depth + 1, positions); + return; + } + + for prefix in transparent_prefixes { + if let Some(rest) = strip_word_prefix(trimmed, prefix) { + if rest.is_empty() { + return; + } + collect_shell_wrapper_positions(rest, transparent_prefixes, depth + 1, positions); + return; + } + } +} + fn rewrite_segment_inner( seg: &str, excluded: &[ExcludePattern], @@ -2114,6 +2182,49 @@ mod tests { super::rewrite_command(cmd, excluded, &[]) } + #[test] + fn test_shell_wrapper_positions_match_where_the_rewrite_reaches() { + let prefixes = normalize_transparent_prefixes(&["direnv exec .".to_string()]); + for (cmd, reached) in [ + ("bash -c 'git status'", true), + ("env bash -c 'git status'", true), + ("FOO=1 bash -c 'git status'", true), + ("env FOO=1 timeout 5 bash -c 'git status'", true), + ("nice -n 5 bash -c 'git status'", true), + ("time bash -c 'git status'", true), + ("nohup sh -c 'git status'", true), + ("command bash -c 'git status'", true), + ("exec bash -c 'git status'", true), + ("uv run bash -c 'git status'", true), + ("direnv exec . bash -c 'git status'", true), + ("echo bash -c 'git status'", false), + ("ssh host bash -c 'git status'", false), + ("find . -exec bash -c 'git status' \\;", false), + ("sudo bash -c 'git status'", false), + ("RTK_DISABLED=1 bash -c 'git status'", false), + ("docker exec c bash -c 'git status'", false), + ] { + let rewritten = rewrite_command(cmd, &[], &prefixes) + .is_some_and(|rewritten| rewritten.contains("-c 'rtk git status'")); + let seen = shell_wrapper_positions(cmd, &prefixes) + .into_iter() + .any(|position| parse_shell_wrapper(position).is_some()); + assert_eq!(rewritten, reached, "rewrite {cmd:?}"); + assert_eq!(seen, reached, "positions {cmd:?}"); + } + } + + #[test] + fn test_shell_wrapper_positions_stop_at_the_command_word() { + let cmd = format!("echo {}bash -c 'git status'", "a ".repeat(50_000)); + assert_eq!(shell_wrapper_positions(&cmd, &[]), vec![cmd.as_str()]); + let nested = format!("{}bash -c 'git status'", "command ".repeat(15)); + assert_eq!( + shell_wrapper_positions(&nested, &[]).len(), + MAX_PREFIX_DEPTH + 1 + ); + } + // Three compound-command segmenters look at the same kind of input for // different, deliberate purposes — split_for_permissions (the permission // gate, most conservative), split_on_operators/split_command_chain diff --git a/src/hooks/README.md b/src/hooks/README.md index 8544f535fd..57d8ecc56a 100644 --- a/src/hooks/README.md +++ b/src/hooks/README.md @@ -72,17 +72,26 @@ All file operations use atomic writes (tempfile + rename) to prevent corruption RTK enforces a permission precedence that matches Claude Code's least-privilege default: ``` -Deny > Ask > Allow (explicit) > Default (ask) +Deny > Defer > Ask > Allow (explicit) > Default (ask) ``` Rules are loaded from all Claude Code `settings.json` files (project + global, including `.local` variants). Only `Bash(...)` rules are extracted; other scopes (Read, Write) are ignored. Quoted `sh -c`, `bash -c`, `zsh -c` and `fish -c` scripts add a second command-parsing boundary. RTK checks deny rules against both the outer -wrapper and the parsed inner segments. A wrapper rewrite is never auto-allowed: -its strongest RTK verdict is `Ask`. Ask-capable hosts prompt, while other -hosts retain their existing native permission flow. Unsupported or ambiguous -wrapper syntax is left unchanged for the host to evaluate. +wrapper and the parsed inner segments, including a wrapper behind a prefix the +rewrite strips (`env bash -c '…'`, `timeout 5 sh -c '…'`, a configured +`transparent_prefixes` entry — `registry::shell_wrapper_positions`). A shell +name that is merely an argument (`grep -rn bash -c src`) is not a wrapper. A +deny rule matching an inner segment yields `Defer`: RTK neither rewrites nor +decides, so the host sees the command exactly as typed. +A wrapper at the start of a segment is never auto-allowed: its strongest RTK +verdict is `Ask`. A wrapper behind a prefix is only checked for deny rules and +otherwise keeps the outer command's verdict, as on master. Ask-capable hosts +prompt, while other hosts retain their existing native permission flow. +Unsupported or ambiguous wrapper syntax is left unchanged for the host to +evaluate. Line continuations are joined before the check, as the rewrite joins +them. | Verdict | Trigger | rewrite_cmd exit | Hook behavior | |---------|---------|-----------------|---------------| @@ -90,6 +99,7 @@ wrapper syntax is left unchanged for the host to evaluate. | Ask | `permissions.ask` rule matched | 3 | Rewrite + let host tool prompt user | | Allow | `permissions.allow` rule matched | 0 | Rewrite + auto-allow | | Default | No rule matched | 3 | Rewrite + let host tool prompt user | +| Defer | `permissions.deny` rule matched inside a shell wrapper | 1 | Passthrough — command runs as without RTK | A delegate that shells out to `rtk rewrite` and applies its own exec policy to the result can set `RTK_REWRITE_HOST=` on that subprocess. For an agent diff --git a/src/hooks/decision.rs b/src/hooks/decision.rs index 2dba83b1d1..ad833b8e08 100644 --- a/src/hooks/decision.rs +++ b/src/hooks/decision.rs @@ -122,7 +122,8 @@ impl ApprovalOwner { /// The gate order is load-bearing: /// /// 1. **Deny wins outright.** Checked before anything else so a denied command -/// is never even considered for rewriting. +/// is never even considered for rewriting. A deny rule matched inside a +/// shell wrapper's script yields `Defer` instead: no rewrite, no decision. /// 2. **Unattestable constructs are refused.** Command substitution and /// file-target redirects can't be decomposed into segments the permission /// gate can check individually, so a rewrite could smuggle an unchecked @@ -146,7 +147,7 @@ pub(crate) fn decide(cmd: &str, verdict: PermissionVerdict) -> HookDecision { } /// [`decide`] with the rewrite parameters supplied by the caller, mirroring -/// [`check_command_with_rules`](super::permissions::check_command_with_rules). +/// [`check_command_with_rules_and_prefixes`](super::permissions::check_command_with_rules_and_prefixes). /// /// `hook_rewrite_params` reads `config.toml`, which in a test build is the /// calling test's own. Taking the parameters lets a test state the @@ -163,6 +164,10 @@ pub(crate) fn decide_with_params( return HookDecision::Deny; } + if verdict == PermissionVerdict::Defer { + return HookDecision::Defer; + } + if crate::discover::lexer::contains_unattestable_construct(cmd) { return HookDecision::Defer; } @@ -392,6 +397,18 @@ mod tests { ); } + #[test] + fn defer_verdict_defers_a_rewritable_command() { + assert_eq!( + decide_with_params("env bash -c 'git push'", PermissionVerdict::Defer, &[], &[]), + HookDecision::Defer + ); + assert!(matches!( + decide_with_params("env bash -c 'git push'", PermissionVerdict::Ask, &[], &[]), + HookDecision::AskRewrite(_) + )); + } + /// Refused whatever the verdict: the permission gate can't decompose these /// into segments it can check, so a rewrite could carry an unchecked /// command past an allow rule. diff --git a/src/hooks/permissions.rs b/src/hooks/permissions.rs index f54164d2dc..2a72449419 100644 --- a/src/hooks/permissions.rs +++ b/src/hooks/permissions.rs @@ -5,7 +5,10 @@ use super::constants::{ use super::init::resolve_claude_dir; use crate::core::user_dirs; use crate::core::user_env; -use crate::discover::lexer::{is_word_boundary_whitespace, split_for_permissions}; +use crate::discover::lexer::{ + is_word_boundary_whitespace, split_for_permissions, split_for_wrappers, +}; +use crate::discover::registry; use crate::discover::shell_wrapper::{is_shell_wrapper_candidate, parse_shell_wrapper}; use serde_json::Value; use std::path::PathBuf; @@ -21,11 +24,13 @@ pub enum PermissionVerdict { Ask, /// No rule matched — default to ask (matches Claude Code's least-privilege default). Default, + /// A deny rule matched inside a shell wrapper's script — RTK neither rewrites nor decides. + Defer, } /// Check `cmd` against Claude Code's deny/ask/allow permission rules. /// -/// Precedence: Deny > Ask > Allow > Default (ask). +/// Precedence: Deny > Defer > Ask > Allow > Default (ask). /// Returns `Default` when no rules match — callers should treat this as ask /// to match Claude Code's least-privilege default. pub fn check_command(cmd: &str) -> PermissionVerdict { @@ -56,7 +61,14 @@ pub fn check_command_for_agent(cmd: &str, host: Host, agent: Option<&str>) -> Pe return super::permissions_opencode::check_command_with_opencode_rules(cmd, &rules); } let (deny_rules, ask_rules, allow_rules) = load_rules_for(host); - check_command_with_rules(cmd, &deny_rules, &ask_rules, &allow_rules) + let (_, transparent_prefixes) = crate::core::config::hook_rewrite_params(); + check_command_with_rules_and_prefixes( + cmd, + &deny_rules, + &ask_rules, + &allow_rules, + &transparent_prefixes, + ) } /// Load `host`'s deny/ask/allow Bash rules from disk, doing the settings-file I/O @@ -83,11 +95,24 @@ pub(crate) fn load_rules_for(host: Host) -> (Vec, Vec, Vec PermissionVerdict { + check_command_with_rules_and_prefixes(cmd, deny_rules, ask_rules, allow_rules, &[]) +} + +/// `check_command_with_rules` with the `transparent_prefixes` the rewrite +/// strips, so a wrapper behind one of them is gated where the rewrite reaches it. +pub(crate) fn check_command_with_rules_and_prefixes( + cmd: &str, + deny_rules: &[String], + ask_rules: &[String], + allow_rules: &[String], + transparent_prefixes: &[String], ) -> PermissionVerdict { let segments = split_compound_command(cmd); @@ -103,7 +128,12 @@ pub(crate) fn check_command_with_rules( } } - if let Some(verdict) = check_shell_wrapper_permissions(&segments, deny_rules) { + let transparent_prefixes = registry::normalize_transparent_prefixes(transparent_prefixes); + let joined = registry::collapse_line_continuations(cmd); + let wrapper_segments = split_for_wrappers(&joined); + if let Some(verdict) = + check_shell_wrapper_permissions(&wrapper_segments, deny_rules, &transparent_prefixes) + { return verdict; } @@ -150,7 +180,7 @@ pub(crate) fn check_command_with_rules( } } - // Precedence: Deny > Ask > Allow > Default (ask). + // Precedence: Deny > Defer > Ask > Allow > Default (ask). // Allow requires (1) at least one segment seen, (2) all segments matched, (3) non-empty rules. if any_ask { PermissionVerdict::Ask @@ -164,30 +194,46 @@ pub(crate) fn check_command_with_rules( fn check_shell_wrapper_permissions( segments: &[&str], deny_rules: &[String], + transparent_prefixes: &[String], ) -> Option { let mut contains_shell_wrapper = false; for segment in segments { - let segment = segment.trim(); - let Some(wrapper) = parse_shell_wrapper(segment) else { - contains_shell_wrapper |= is_shell_wrapper_candidate(segment); - continue; - }; - let Some(script) = wrapper.script(segment) else { - return Some(PermissionVerdict::Ask); - }; - contains_shell_wrapper = true; - // Both spellings, exactly as the outer loop above: a segment can open - // with grammar that is not part of the command (`! rm …`, `{ rm …`), - // and a rule naming the command has to reach it there too. - let inner_denied = split_for_permissions(script).iter().any(|inner_segment| { - let inner_segment = inner_segment.trim(); - deny_rules.iter().any(|pattern| { - command_matches_pattern(inner_segment, pattern) - || command_matches_pattern(strip_grammar_residue(inner_segment), pattern) - }) - }); - if inner_denied { - return Some(PermissionVerdict::Deny); + for (depth, position) in registry::shell_wrapper_positions(segment, transparent_prefixes) + .into_iter() + .enumerate() + { + let Some(wrapper) = parse_shell_wrapper(position) else { + if depth == 0 { + contains_shell_wrapper |= is_shell_wrapper_candidate(position); + } + continue; + }; + if depth == 0 { + contains_shell_wrapper = true; + } + let Some(script) = wrapper.script(position) else { + continue; + }; + // Both spellings, exactly as the outer loop above: a segment can open + // with grammar that is not part of the command (`! rm …`, `{ rm …`), + // and a rule naming the command has to reach it there too. + let inner_denied = split_for_permissions(script).iter().any(|inner_segment| { + let inner_segment = inner_segment.trim(); + let readings = registry::shell_wrapper_positions( + strip_grammar_residue(inner_segment), + transparent_prefixes, + ); + std::iter::once(inner_segment) + .chain(readings) + .any(|reading| { + deny_rules + .iter() + .any(|pattern| command_matches_pattern(reading, pattern)) + }) + }); + if inner_denied { + return Some(PermissionVerdict::Defer); + } } } @@ -1434,7 +1480,7 @@ mod tests { } #[test] - fn test_shell_wrapper_inner_deny_wins() { + fn test_shell_wrapper_inner_deny_defers() { let deny = vec!["rm:*".to_string()]; let allow = vec!["*".to_string()]; assert_eq!( @@ -1444,7 +1490,7 @@ mod tests { &[], &allow ), - PermissionVerdict::Deny + PermissionVerdict::Defer ); } @@ -1464,15 +1510,15 @@ mod tests { ] { assert_eq!( check_command_with_rules(cmd, &deny, &[], &allow), - PermissionVerdict::Deny, + PermissionVerdict::Defer, "{cmd:?}" ); } // The grammar itself names no command, so a script without a denied // one is unaffected. - assert_ne!( + assert_eq!( check_command_with_rules("bash -c 'git status; ! cargo test'", &deny, &[], &allow), - PermissionVerdict::Deny + PermissionVerdict::Ask ); } @@ -1635,7 +1681,7 @@ mod tests { } #[test] - fn test_shell_wrapper_inner_deny_wins_inside_outer_compound() { + fn test_shell_wrapper_inner_deny_defers_inside_outer_compound() { let deny = vec!["rm:*".to_string()]; let allow = vec!["*".to_string()]; assert_eq!( @@ -1645,7 +1691,281 @@ mod tests { &[], &allow ), - PermissionVerdict::Deny + PermissionVerdict::Defer + ); + } + + #[test] + fn test_prefixed_shell_wrapper_inner_deny_defers() { + let deny = vec!["git push:*".to_string()]; + let allow = vec!["*".to_string()]; + for cmd in [ + "env bash -c 'git push'", + "env FOO=1 bash -c 'git push'", + "FOO=1 bash -c 'git push'", + "timeout 5 bash -c 'git push'", + "nice -n 5 bash -c 'git push'", + "time bash -c 'git push'", + "nohup sh -c 'git push'", + "command bash -c 'git push'", + "exec bash -c 'git push'", + "uv run bash -c 'git push'", + "env FOO=1 timeout 5 bash -c 'git push'", + "env zsh -c 'git status; git push'", + "ls && env bash -c 'git push'", + ] { + assert_eq!( + check_command_with_rules(cmd, &deny, &[], &allow), + PermissionVerdict::Defer, + "{cmd:?}" + ); + } + } + + #[test] + fn test_prefixed_shell_wrapper_keeps_the_outer_verdict() { + let allow = vec![ + "env:*".to_string(), + "timeout:*".to_string(), + "git status:*".to_string(), + ]; + for cmd in [ + "env bash -c 'git status'", + "timeout 5 bash -c 'git status'", + "env bash -lc 'git status'", + ] { + assert_eq!( + check_command_with_rules(cmd, &[], &[], &allow), + PermissionVerdict::Allow, + "{cmd:?}" + ); + } + assert_eq!( + check_command_with_rules("bash -c 'git status'", &[], &[], &allow), + PermissionVerdict::Ask + ); + } + + #[test] + fn test_line_continuation_is_joined_before_the_wrapper_check() { + let deny = vec!["git push:*".to_string()]; + let allow = vec!["*".to_string()]; + for cmd in [ + "bash -c 'git \\\npush'", + "bash -c \\\n'git push'", + "bash \\\n-c 'git push'", + "env bash -c 'git \\\npush' && ls", + "timeout 5 bash -c 'git status; git \\\npush'", + ] { + assert_eq!( + check_command_with_rules(cmd, &deny, &[], &allow), + PermissionVerdict::Defer, + "{cmd:?}" + ); + } + assert_eq!( + check_command_with_rules( + "bash -c 'git status' && ls 'git\\\n status'", + &deny, + &[], + &allow + ), + PermissionVerdict::Ask + ); + } + + #[test] + fn test_wrapper_reached_by_the_rewrite_is_gated_for_deny_rules() { + let deny = vec!["git push:*".to_string()]; + let allow = vec!["*".to_string()]; + let ten = format!("{}bash -c 'git push'", "exec ".repeat(10)); + let eleven = format!("{}bash -c 'git push'", "exec ".repeat(11)); + for cmd in [ + "FOO=a>/dev/null bash -c 'git push'", + "FOO=a&>/dev/null timeout 5 bash -c 'git push'", + "bash -c \\\n'git push'", + "bash \\\n-c 'git push'", + "timeout 5 \\\nbash -c 'git push'", + "true &&\\\nbash -c 'git push'", + "ls && FOO=a>/dev/null bash -c 'git push'", + ten.as_str(), + ] { + assert!( + registry::rewrite_command(cmd, &[], &[]).is_some(), + "{cmd:?}" + ); + assert_eq!( + check_command_with_rules(cmd, &deny, &[], &allow), + PermissionVerdict::Defer, + "{cmd:?}" + ); + } + assert!(registry::rewrite_command(&eleven, &[], &[]).is_none()); + assert_eq!( + check_command_with_rules(&eleven, &deny, &[], &allow), + PermissionVerdict::Allow + ); + } + + #[test] + fn test_inner_deny_sees_past_a_prefix_inside_the_script() { + let deny = vec!["git push:*".to_string()]; + let allow = vec!["*".to_string()]; + for cmd in [ + "env bash -c 'exec git push'", + "env bash -c 'command git push'", + "env bash -c 'env git push'", + "env bash -c 'timeout 5 git push'", + "env bash -c 'FOO=1 git push'", + "env bash -c 'git status; ! exec git push'", + "bash -c 'exec git push'", + "bash -c 'nohup FOO=1 git push'", + ] { + assert_eq!( + check_command_with_rules(cmd, &deny, &[], &allow), + PermissionVerdict::Defer, + "{cmd:?}" + ); + } + for cmd in ["env bash -c 'echo git push'", "env bash -c 'git -C . push'"] { + assert_eq!( + check_command_with_rules(cmd, &deny, &[], &allow), + PermissionVerdict::Allow, + "{cmd:?}" + ); + } + } + + #[test] + fn test_untouched_prefixed_wrapper_keeps_the_outer_verdict() { + let deny = vec!["git push:*".to_string(), "rm:*".to_string()]; + let allow = vec!["*".to_string()]; + for cmd in [ + "uv run env sh -ec \"git status\"", + "\tenv zsh -ec 'git status' && ls", + "( nice fish -lc '{ git status; }' && ls", + "time -p sh -lc \"command git push\" ; ls", + "env zsh -c '{ git status; }' && ls", + "uv run bash -c \"git status\" | grep -rn x src", + ] { + assert_eq!( + check_command_with_rules(cmd, &deny, &[], &allow), + PermissionVerdict::Allow, + "{cmd:?}" + ); + } + } + + #[test] + fn test_user_transparent_prefix_wrapper_inner_deny_defers() { + let deny = vec!["git push:*".to_string()]; + let allow = vec!["*".to_string()]; + let prefixes = vec!["direnv exec .".to_string()]; + assert_eq!( + check_command_with_rules_and_prefixes( + "direnv exec . bash -c 'git push'", + &deny, + &[], + &allow, + &prefixes + ), + PermissionVerdict::Defer + ); + assert_eq!( + check_command_with_rules_and_prefixes( + "direnv exec . bash -c 'git status'", + &[], + &[], + &allow, + &prefixes + ), + PermissionVerdict::Allow + ); + assert_eq!( + check_command_with_rules("direnv exec . bash -c 'git push'", &deny, &[], &allow), + PermissionVerdict::Allow + ); + } + + #[test] + fn test_shell_wrapper_word_as_argument_is_not_a_wrapper() { + let deny = vec!["git push:*".to_string()]; + let allow = vec!["*".to_string()]; + for cmd in [ + r#"git commit -m "bash -c 'git push'""#, + r#"git commit -m "/bin/sh -c 'git push' fix""#, + "grep -rn bash -c src", + "rg -t sh -c TODO src", + "git log --oneline sh -c", + "ssh host bash -c 'git status'", + "docker run img sh -c 'ls'", + r#"find . -name '*.rs' -exec sh -c 'wc -l "$1"' _ {} \;"#, + r"git status \# sh -c x", + "echo bash -c 'git push'", + ] { + assert_eq!( + check_command_with_rules(cmd, &deny, &[], &allow), + PermissionVerdict::Allow, + "{cmd:?}" + ); + } + } + + #[test] + fn test_deny_rule_in_argument_text_keeps_the_chain_rewritable() { + let deny = vec!["rm:*".to_string()]; + let cmd = "git status && grep sh -c 'rm' f"; + assert_eq!( + check_command_with_rules(cmd, &deny, &[], &["*".to_string()]), + PermissionVerdict::Allow + ); + assert_eq!( + check_command_with_rules(cmd, &deny, &[], &[]), + PermissionVerdict::Default + ); + } + + #[test] + fn test_wrapper_suffix_arguments_are_not_a_second_wrapper() { + let deny = vec!["git push:*".to_string()]; + let allow = vec!["*".to_string()]; + assert_eq!( + check_command_with_rules( + "bash -c 'git status' -- sh -c 'git push'", + &deny, + &[], + &allow + ), + PermissionVerdict::Ask + ); + } + + #[test] + fn test_commented_shell_wrapper_text_is_not_a_wrapper() { + let deny = vec!["git push:*".to_string()]; + let allow = vec!["*".to_string()]; + assert_eq!( + check_command_with_rules("git status # bash -c 'git push'", &deny, &[], &allow), + PermissionVerdict::Allow + ); + } + + #[test] + fn test_unreadable_wrapper_does_not_hide_a_later_inner_deny() { + let deny = vec!["git push:*".to_string()]; + let allow = vec!["*".to_string()]; + assert_eq!( + check_command_with_rules( + r#"bash -c "$SCRIPT"; bash -c 'git push'"#, + &deny, + &[], + &allow + ), + PermissionVerdict::Defer + ); + assert_eq!( + check_command_with_rules(r#"bash -c "$SCRIPT""#, &deny, &[], &allow), + PermissionVerdict::Ask ); } } diff --git a/src/hooks/rewrite_cmd.rs b/src/hooks/rewrite_cmd.rs index 3efa7ad068..1216c4f5f9 100644 --- a/src/hooks/rewrite_cmd.rs +++ b/src/hooks/rewrite_cmd.rs @@ -71,7 +71,7 @@ pub(crate) fn track_tee_read(cmd: &str) { /// | Exit | Stdout | Meaning | /// |------|----------|--------------------------------------------------------------------------| /// | 0 | rewritten| Allow rules cover every command — hook may auto-allow the rewrite. | -/// | 1 | (none) | No rewrite ([`decision::decide`] deferred) — hook passes through. | +/// | 1 | (none) | No rewrite, or a deny rule matched inside a shell wrapper's script ([`decision::decide`] deferred) — hook passes through. | /// | 2 | (none) | Deny rule matched — hook defers to the host's native deny. | /// | 3 | rewritten| Ask rule, or allow rules short of every command (also with no rules) — hook rewrites, host prompts. | /// @@ -224,12 +224,14 @@ mod tests { /// Ask → 3 (process::exit(3)) /// Default→ 3 (process::exit(3)) — grouped with Ask /// Deny → 2 (process::exit(2)) — handled before rewrite match + /// Defer → 1 (process::exit(1)) — passthrough, no rewrite fn expected_exit_code(verdict: &PermissionVerdict) -> i32 { match verdict { PermissionVerdict::Allow => 0, PermissionVerdict::Deny => 2, PermissionVerdict::Ask => 3, PermissionVerdict::Default => 3, // MUST be 3, not 0! + PermissionVerdict::Defer => 1, } } @@ -245,6 +247,15 @@ mod tests { ); } + #[test] + fn test_defer_verdict_maps_to_passthrough_exit_code() { + let deny = vec!["git push:*".to_string()]; + let allow = vec!["*".to_string()]; + let verdict = check_command_with_rules("env bash -c 'git push'", &deny, &[], &allow); + assert_eq!(verdict, PermissionVerdict::Defer); + assert_eq!(expected_exit_code(&verdict), 1); + } + #[test] fn test_allow_verdict_maps_to_allow_exit_code() { let allow = vec!["git *".to_string()]; diff --git a/src/main.rs b/src/main.rs index 49d6854262..0b3e5ad08d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -985,8 +985,9 @@ enum Commands { /// auto-allows on 0 runs rtk with that variable unset. /// /// 1 - nothing printed, no rewrite (no RTK equivalent, nothing left to - /// rewrite, a command excluded in config, or a construct RTK does not - /// rewrite such as `$(...)` or a file redirect): run the command unchanged. + /// rewrite, a command excluded in config, a construct RTK does not + /// rewrite such as `$(...)` or a file redirect, or a deny rule matched + /// inside a shell wrapper's script): run the command unchanged. /// /// 2 - nothing printed, a deny rule matched (checked before anything /// else): defer to the agent's own deny.