From fc8054eb0b357d32cb0457094d142de52c3db0e7 Mon Sep 17 00:00:00 2001 From: revopsrocks Date: Fri, 24 Jul 2026 15:37:23 +0930 Subject: [PATCH 1/6] feat(rewrite): rewrite each line of multi-line Bash blocks Coding agents routinely emit multi-line Bash blocks of sequential commands. The rewriter tokenized newlines as plain whitespace, so a block was treated as one command: the rtk prefix landed on the first line and every following line ran raw and unfiltered. Split multi-line input at the newline tokens the quote-aware lexer emits (newlines inside quoted strings are never split points) and rewrite each line through the existing single-line path. Blank lines, comment lines, indentation, and CRLF separators are preserved verbatim. Per-line rewriting only applies when every line is an independent command. The whole block passes through untouched when: - a shell keyword opens control flow (for/if/while/case/...) - a list or pipeline continues across the line break (&&, ||, |, |& at a line edge) - a subshell or group spans lines - the block contains a heredoc (existing gate) Ref #1243 --- src/discover/lexer.rs | 20 ++++ src/discover/registry.rs | 237 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 255 insertions(+), 2 deletions(-) diff --git a/src/discover/lexer.rs b/src/discover/lexer.rs index 6cca2a39e1..9a3805599d 100644 --- a/src/discover/lexer.rs +++ b/src/discover/lexer.rs @@ -26,6 +26,13 @@ pub fn tokenize(input: &str) -> Vec { tokenize_inner(input, false) } +/// Like [`tokenize`] but emits a `\n` operator token for each newline that +/// sits outside quotes. Newlines inside quoted strings stay part of their +/// argument, so callers can use the emitted offsets as safe line-split points. +pub fn tokenize_with_newlines(input: &str) -> Vec { + tokenize_inner(input, true) +} + fn tokenize_inner(input: &str, emit_newline: bool) -> Vec { let mut tokens = Vec::new(); let mut current = String::new(); @@ -1342,4 +1349,17 @@ mod tests { assert!(split_for_permissions("").is_empty()); assert!(split_for_permissions(" ").is_empty()); } + + #[test] + fn test_tokenize_with_newlines_emits_operator_outside_quotes_only() { + let newline_ops = |input: &str| { + tokenize_with_newlines(input) + .iter() + .filter(|t| t.kind == TokenKind::Operator && t.value == "\n") + .count() + }; + assert_eq!(newline_ops("git status\ngit log"), 1); + assert_eq!(newline_ops("echo 'line1\nline2'"), 0); + assert_eq!(newline_ops("git status\r\ngit log"), 2); + } } diff --git a/src/discover/registry.rs b/src/discover/registry.rs index 81b170785a..b7827482b1 100644 --- a/src/discover/registry.rs +++ b/src/discover/registry.rs @@ -5,7 +5,10 @@ use regex::{Regex, RegexSet}; use std::path::Path; use std::sync::LazyLock; -use super::lexer::{shell_split, split_on_operators, tokenize, ParsedToken, PipeKind, TokenKind}; +use super::lexer::{ + shell_split, split_on_operators, tokenize, tokenize_with_newlines, ParsedToken, PipeKind, + TokenKind, +}; use super::rules::{IGNORED_EXACT, IGNORED_PREFIXES, RULES}; const PHP_TOOL_NAMES: [&str; 6] = ["phpunit", "phpstan", "ecs", "pest", "paratest", "pint"]; @@ -583,6 +586,19 @@ pub fn rewrite_command( let compiled = compile_exclude_patterns(excluded); let normalized_prefixes = normalize_transparent_prefixes(transparent_prefixes); + if trimmed.contains('\n') { + return rewrite_multiline_block(trimmed, &compiled, &normalized_prefixes); + } + + rewrite_single(trimmed, &compiled, &normalized_prefixes) +} + +/// Rewrite one logical command line (no unquoted newlines). +fn rewrite_single( + trimmed: &str, + excluded: &[ExcludePattern], + transparent_prefixes: &[String], +) -> Option { // Simple (non-compound) already-RTK command — return as-is. // For compound commands that start with "rtk" (e.g. "rtk git add . && cargo test"), // fall through to rewrite_compound so the remaining segments get rewritten. @@ -595,7 +611,103 @@ pub fn rewrite_command( return Some(trimmed.to_string()); } - rewrite_compound(trimmed, &compiled, &normalized_prefixes) + rewrite_compound(trimmed, excluded, transparent_prefixes) +} + +/// Shell keywords that open or close a multi-line construct. A line inside a +/// loop, conditional, case arm, function body, or group is not an independent +/// command, so the whole block passes through untouched. +const BLOCK_KEYWORDS: &[&str] = &[ + "for", "while", "until", "if", "then", "else", "elif", "fi", "do", "done", "case", "esac", + "select", "function", "coproc", "{", "}", "(", ")", +]; + +/// A list or pipeline that continues across a line break makes adjacent lines +/// one logical command; grouping chars at a line edge mean a construct spans +/// lines. Either way, per-line rewriting is unsafe. +fn line_breaks_independence(line: &str) -> bool { + let first = line.split_whitespace().next().unwrap_or(""); + if BLOCK_KEYWORDS.contains(&first) { + return true; + } + ["&&", "||", "|&", "|"] + .iter() + .any(|op| line.ends_with(op) || line.starts_with(op)) + || line.ends_with('(') + || line.ends_with('{') + || line.starts_with(')') + || line.starts_with('}') +} + +/// Rewrite each line of a multi-line block independently (issue #1243). +/// +/// Split points are the newline tokens the quote-aware lexer emits, so a +/// newline inside a quoted string (e.g. a multi-line commit message) never +/// becomes a boundary. The whole block passes through unchanged unless every +/// line is an independent command (see [`line_breaks_independence`]); blank +/// lines and comment lines are preserved verbatim, as is indentation and the +/// original separator bytes (`\n` vs `\r\n`). +fn rewrite_multiline_block( + cmd: &str, + excluded: &[ExcludePattern], + transparent_prefixes: &[String], +) -> Option { + let newline_offsets: Vec = tokenize_with_newlines(cmd) + .iter() + .filter(|t| t.kind == TokenKind::Operator && t.value == "\n") + .map(|t| t.offset) + .collect(); + + // Newlines only inside quotes — a single logical command. + if newline_offsets.is_empty() { + return rewrite_single(cmd, excluded, transparent_prefixes); + } + + let mut segments = Vec::with_capacity(newline_offsets.len() + 1); + let mut start = 0; + for &off in &newline_offsets { + segments.push(&cmd[start..off]); + start = off + 1; + } + segments.push(&cmd[start..]); + + if segments + .iter() + .map(|seg| seg.trim()) + .filter(|line| !line.is_empty() && !line.starts_with('#')) + .any(line_breaks_independence) + { + return None; + } + + let mut any_changed = false; + let mut result = String::with_capacity(cmd.len() + 32); + for (i, seg) in segments.iter().enumerate() { + if i > 0 { + let off = newline_offsets[i - 1]; + result.push_str(&cmd[off..off + 1]); + } + let line = seg.trim(); + if line.is_empty() || line.starts_with('#') { + result.push_str(seg); + continue; + } + match rewrite_single(line, excluded, transparent_prefixes) { + Some(rewritten) if rewritten != line => { + any_changed = true; + let indent = &seg[..seg.len() - seg.trim_start().len()]; + result.push_str(indent); + result.push_str(&rewritten); + } + _ => result.push_str(seg), + } + } + + if any_changed { + Some(result) + } else { + None + } } /// Pipeline boundaries used to rewrite its final stage. @@ -1159,6 +1271,127 @@ mod tests { super::rewrite_command(cmd, excluded, &[]) } + mod multiline_blocks { + use super::rewrite_command_no_prefixes; + + #[test] + fn test_rewrites_each_line() { + assert_eq!( + rewrite_command_no_prefixes("git status\ngit log --oneline -3", &[]), + Some("rtk git status\nrtk git log --oneline -3".into()) + ); + } + + #[test] + fn test_preserves_blank_lines_comments_and_indentation() { + assert_eq!( + rewrite_command_no_prefixes("git status\n\n# check history\n git log -3", &[]), + Some("rtk git status\n\n# check history\n rtk git log -3".into()) + ); + } + + #[test] + fn test_compound_line_inside_block() { + assert_eq!( + rewrite_command_no_prefixes("cd /tmp && git status\ngrep -rn foo src", &[]), + Some("cd /tmp && rtk git status\nrtk grep -rn foo src".into()) + ); + } + + #[test] + fn test_crlf_separators_preserved() { + assert_eq!( + rewrite_command_no_prefixes("git status\r\ngit log -3", &[]), + Some("rtk git status\r\nrtk git log -3".into()) + ); + } + + #[test] + fn test_newline_inside_quotes_is_not_a_split_point() { + // The quoted body must never be treated as a command line of its own. + let result = + rewrite_command_no_prefixes("git commit -m \"subject\ngit status in body\"", &[]); + if let Some(rewritten) = result { + assert!(!rewritten.contains("rtk git status")); + } + } + + #[test] + fn test_no_rewritable_line_passes_through() { + assert_eq!(rewrite_command_no_prefixes("echo one\necho two", &[]), None); + } + + #[test] + fn test_already_rtk_lines_count_as_unchanged() { + assert_eq!( + rewrite_command_no_prefixes("rtk git status\necho done", &[]), + None + ); + } + + #[test] + fn test_mixed_rtk_and_rewritable_line() { + assert_eq!( + rewrite_command_no_prefixes("rtk git status\ngit log -3", &[]), + Some("rtk git status\nrtk git log -3".into()) + ); + } + + #[test] + fn test_for_loop_block_passes_through() { + assert_eq!( + rewrite_command_no_prefixes("for f in a b; do\n grep -n foo $f\ndone", &[]), + None + ); + } + + #[test] + fn test_if_block_passes_through() { + assert_eq!( + rewrite_command_no_prefixes("if [ -d src ]; then\n git status\nfi", &[]), + None + ); + } + + #[test] + fn test_cross_line_and_list_passes_through() { + assert_eq!( + rewrite_command_no_prefixes("git status &&\ngit log -3", &[]), + None + ); + } + + #[test] + fn test_cross_line_pipeline_passes_through() { + assert_eq!( + rewrite_command_no_prefixes("git log |\ngrep feat", &[]), + None + ); + assert_eq!( + rewrite_command_no_prefixes("cargo test |&\ngrep FAILED", &[]), + None + ); + } + + #[test] + fn test_subshell_spanning_lines_passes_through() { + assert_eq!(rewrite_command_no_prefixes("(\n git status\n)", &[]), None); + } + + #[test] + fn test_group_spanning_lines_passes_through() { + assert_eq!(rewrite_command_no_prefixes("{\n git status\n}", &[]), None); + } + + #[test] + fn test_heredoc_block_passes_through() { + assert_eq!( + rewrite_command_no_prefixes("git status\ncat < PipelineAnalysis { let tokens = tokenize(cmd); let first_pipe_offset = tokens From ac5febe3bb1c57195d3bdbe73d029c6aad34a64d Mon Sep 17 00:00:00 2001 From: revopsrocks Date: Fri, 24 Jul 2026 16:14:42 +0930 Subject: [PATCH 2/6] fix(rewrite): bail on swallowed newlines and cross-line arithmetic Hardening from adversarial review of the multi-line rewrite: - If any newline byte was swallowed by quote state (raw \n/\r count vs emitted newline tokens), pass the whole block through. The lexer has no comment awareness, so an apostrophe in a # comment opens quote state and hides subsequent lines; rewriting such a block would act on lines no permission verdict accounted for. Passthrough hands the original command to native permission handling. Quoted multi-line strings (commit messages) forgo their rewrite as the safe trade. - Bail when (( or )) sits at a line edge: arithmetic spanning lines must not get a command prefix spliced into arithmetic context. --- src/discover/registry.rs | 73 ++++++++++++++++++++++++++++++++++------ 1 file changed, 63 insertions(+), 10 deletions(-) diff --git a/src/discover/registry.rs b/src/discover/registry.rs index b7827482b1..78597cb048 100644 --- a/src/discover/registry.rs +++ b/src/discover/registry.rs @@ -637,6 +637,9 @@ fn line_breaks_independence(line: &str) -> bool { || line.ends_with('{') || line.starts_with(')') || line.starts_with('}') + || line.starts_with("((") + || line.ends_with("))") + || line.ends_with("((") } /// Rewrite each line of a multi-line block independently (issue #1243). @@ -647,6 +650,14 @@ fn line_breaks_independence(line: &str) -> bool { /// line is an independent command (see [`line_breaks_independence`]); blank /// lines and comment lines are preserved verbatim, as is indentation and the /// original separator bytes (`\n` vs `\r\n`). +/// +/// If any newline byte was swallowed by quote state, the block passes through +/// untouched. The lexer has no comment awareness, so an apostrophe in a `#` +/// comment opens quote state and hides the rest of the block — rewriting (or +/// prefixing) such a block would act on lines no permission verdict was +/// computed for. Passthrough hands the original command to the agent's native +/// permission handling instead. Genuine quoted newlines (multi-line commit +/// messages) also land here; forgoing that rewrite is the safe trade. fn rewrite_multiline_block( cmd: &str, excluded: &[ExcludePattern], @@ -658,9 +669,9 @@ fn rewrite_multiline_block( .map(|t| t.offset) .collect(); - // Newlines only inside quotes — a single logical command. - if newline_offsets.is_empty() { - return rewrite_single(cmd, excluded, transparent_prefixes); + let raw_breaks = cmd.chars().filter(|c| matches!(c, '\n' | '\r')).count(); + if raw_breaks != newline_offsets.len() { + return None; } let mut segments = Vec::with_capacity(newline_offsets.len() + 1); @@ -1307,13 +1318,55 @@ mod tests { } #[test] - fn test_newline_inside_quotes_is_not_a_split_point() { - // The quoted body must never be treated as a command line of its own. - let result = - rewrite_command_no_prefixes("git commit -m \"subject\ngit status in body\"", &[]); - if let Some(rewritten) = result { - assert!(!rewritten.contains("rtk git status")); - } + fn test_newline_inside_quotes_passes_through() { + // A swallowed newline means the block can't be split safely — + // the quoted body must never be treated as a command line of its own. + assert_eq!( + rewrite_command_no_prefixes("git commit -m \"subject\ngit status in body\"", &[]), + None + ); + } + + #[test] + fn test_comment_apostrophe_swallowing_newline_passes_through() { + // The lexer has no comment state: the apostrophe in `don't` opens + // a quote that swallows the newline and hides the next line. The + // block must pass through so native permission handling sees the + // original command — never a partially rewritten one. + assert_eq!( + rewrite_command_no_prefixes("git status # don't\nrm -rf /tmp/x", &[]), + None + ); + } + + #[test] + fn test_comment_apostrophe_hidden_in_later_segment_passes_through() { + // Same hazard when a clean split point precedes the contaminated + // line: the swallowed-newline check is global, not per-segment. + assert_eq!( + rewrite_command_no_prefixes("git log -3\ngit status # don't\nrm -rf /tmp/x", &[]), + None + ); + } + + #[test] + fn test_comment_with_balanced_quotes_still_rewrites() { + // Both apostrophes close before the newline, so the split is safe + // and the trailing comment rides along untouched. + assert_eq!( + rewrite_command_no_prefixes( + "git status # isn't it what's expected\ngit log -3", + &[] + ), + Some("rtk git status # isn't it what's expected\nrtk git log -3".into()) + ); + } + + #[test] + fn test_arithmetic_spanning_lines_passes_through() { + // `(( x = ls ))` is arithmetic evaluation; injecting `rtk` before + // `ls` would splice a command into arithmetic context. + assert_eq!(rewrite_command_no_prefixes("(( x =\nls ))", &[]), None); } #[test] From 93aab9a29c940c7adcbf98b25f28d9b60020302d Mon Sep 17 00:00:00 2001 From: revopsrocks Date: Fri, 24 Jul 2026 20:55:12 +0930 Subject: [PATCH 3/6] fix(rewrite): harden line-independence checks from second review round MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Strip trailing unquoted comments before the independence checks: 'git log | # keep pipeline' continues the pipeline across the newline even though the line ends in comment text, and rewriting the next line would rebuild a pipeline whose final stage the pipeline-safety guard may specifically reject (e.g. grep -f). - Bail when a line's unquoted ()/{} don't balance: array literals (arr=(one), function bodies (foo() {), and groups span lines, so surrounding lines are not independent commands. Subsumes the previous single-char edge checks. - Bail on any block containing $'...': inside ANSI-C quoting bash treats \' as a literal quote that does not close the string, but the lexer thinks it does — emitting a split point bash would never honor, the inverse of the swallowed-newline case the count check catches. --- src/discover/registry.rs | 149 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 138 insertions(+), 11 deletions(-) diff --git a/src/discover/registry.rs b/src/discover/registry.rs index 78597cb048..29c2c9c03c 100644 --- a/src/discover/registry.rs +++ b/src/discover/registry.rs @@ -622,24 +622,81 @@ const BLOCK_KEYWORDS: &[&str] = &[ "select", "function", "coproc", "{", "}", "(", ")", ]; +/// Byte offset where an unquoted `#` at the start of a word begins a trailing +/// comment, if any. The lexer has no comment state, so the independence checks +/// must ignore comment text themselves: `git log | # keep pipeline` continues +/// the pipeline across the newline even though the line ends in comment text. +fn comment_start(line: &str) -> Option { + let bytes = line.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 && !in_double && (i == 0 || bytes[i - 1].is_ascii_whitespace()) => { + return Some(i) + } + _ => {} + } + i += 1; + } + None +} + +/// Unquoted `(`/`)` or `{`/`}` that don't balance within the line: an array +/// literal (`arr=(one`), function body (`foo() {`), or group spans lines, so +/// the lines around it are not independent commands. +fn line_has_unbalanced_grouping(code: &str) -> bool { + let bytes = code.as_bytes(); + let mut in_single = false; + let mut in_double = false; + let mut paren = 0i32; + let mut brace = 0i32; + 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 && !in_double => paren += 1, + b')' if !in_single && !in_double => paren -= 1, + b'{' if !in_single && !in_double => brace += 1, + b'}' if !in_single && !in_double => brace -= 1, + _ => {} + } + if paren < 0 || brace < 0 { + return true; + } + i += 1; + } + paren != 0 || brace != 0 +} + /// A list or pipeline that continues across a line break makes adjacent lines -/// one logical command; grouping chars at a line edge mean a construct spans -/// lines. Either way, per-line rewriting is unsafe. +/// one logical command; grouping that spans lines means no line stands alone. +/// Either way, per-line rewriting is unsafe. All checks run against the line +/// with any trailing comment stripped. fn line_breaks_independence(line: &str) -> bool { - let first = line.split_whitespace().next().unwrap_or(""); + let code = comment_start(line).map_or(line, |i| line[..i].trim_end()); + let first = code.split_whitespace().next().unwrap_or(""); if BLOCK_KEYWORDS.contains(&first) { return true; } ["&&", "||", "|&", "|"] .iter() - .any(|op| line.ends_with(op) || line.starts_with(op)) - || line.ends_with('(') - || line.ends_with('{') - || line.starts_with(')') - || line.starts_with('}') - || line.starts_with("((") - || line.ends_with("))") - || line.ends_with("((") + .any(|op| code.ends_with(op) || code.starts_with(op)) + || code.starts_with("((") + || code.ends_with("))") + || line_has_unbalanced_grouping(code) } /// Rewrite each line of a multi-line block independently (issue #1243). @@ -669,6 +726,14 @@ fn rewrite_multiline_block( .map(|t| t.offset) .collect(); + // ANSI-C quoting is the inverse hazard: inside $'...' bash treats \' as a + // literal quote that does NOT close the string, but the lexer thinks it + // does — emitting a split point bash would never honor, which the + // swallowed-newline count check below cannot see. Forgo the rewrite. + if cmd.contains("$'") { + return None; + } + let raw_breaks = cmd.chars().filter(|c| matches!(c, '\n' | '\r')).count(); if raw_breaks != newline_offsets.len() { return None; @@ -1369,6 +1434,68 @@ mod tests { assert_eq!(rewrite_command_no_prefixes("(( x =\nls ))", &[]), None); } + #[test] + fn test_array_assignment_spanning_lines_passes_through() { + // The inner line is an array element, not a command; rewriting it + // would mutate the array's contents. + assert_eq!( + rewrite_command_no_prefixes("arr=(one\ngit status\ntwo)", &[]), + None + ); + } + + #[test] + fn test_function_definition_spanning_lines_passes_through() { + assert_eq!( + rewrite_command_no_prefixes("foo() {\n git status\n}", &[]), + None + ); + } + + #[test] + fn test_continuation_operator_behind_comment_passes_through() { + // Bash continues the pipeline across the newline even though the + // line ends in comment text; the next line is a pipeline stage, + // not an independent command. + assert_eq!( + rewrite_command_no_prefixes("git log | # keep pipeline\ngrep -f patterns.txt", &[]), + None + ); + assert_eq!( + rewrite_command_no_prefixes("git status && # continue\ngit log -3", &[]), + None + ); + } + + #[test] + fn test_ansi_c_quoting_passes_through() { + // Inside $'...' bash treats \' as a literal quote that does not + // close the string, so the second line is string content — the + // lexer can't see that, so any $' in the block forgoes the rewrite. + assert_eq!( + rewrite_command_no_prefixes("x=$'foo\\'\ngit status\n'", &[]), + None + ); + assert_eq!( + rewrite_command_no_prefixes("echo $'a\\tb'\ngit status", &[]), + None + ); + } + + #[test] + fn test_balanced_grouping_within_a_line_still_rewrites() { + // `${HOME}` braces (quoted or not) must not trip the + // unbalanced-grouping bail. + assert_eq!( + rewrite_command_no_prefixes("echo ${HOME}\ngit status", &[]), + Some("echo ${HOME}\nrtk git status".into()) + ); + assert_eq!( + rewrite_command_no_prefixes("echo \"${HOME}\"\ngit status", &[]), + Some("echo \"${HOME}\"\nrtk git status".into()) + ); + } + #[test] fn test_no_rewritable_line_passes_through() { assert_eq!(rewrite_command_no_prefixes("echo one\necho two", &[]), None); From d1714494914b003d6456c1dae0afa579d2bef4c4 Mon Sep 17 00:00:00 2001 From: Adrien Eppling Date: Thu, 30 Jul 2026 14:13:44 +0200 Subject: [PATCH 4/6] fix(rewrite): narrow multiline passthrough bails from #3188 review --- src/discover/registry.rs | 238 ++++++++++++++++++++++++++++++++------- 1 file changed, 200 insertions(+), 38 deletions(-) diff --git a/src/discover/registry.rs b/src/discover/registry.rs index 29c2c9c03c..a48610ae03 100644 --- a/src/discover/registry.rs +++ b/src/discover/registry.rs @@ -538,6 +538,8 @@ fn strip_trailing_redirects(cmd: &str) -> (&str, &str) { static LINE_CONTINUATION_RE: LazyLock = LazyLock::new(|| Regex::new(r"(?m)[ \t\x0B\x0C]*\\\r?\n[ \t\x0B\x0C]*").unwrap()); +static BASH_JOIN_RE: LazyLock = LazyLock::new(|| Regex::new(r"\\\r?\n").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. @@ -569,6 +571,15 @@ pub fn rewrite_command( excluded: &[String], transparent_prefixes: &[String], ) -> Option { + // Bash joins `\` with nothing, so `<<` or `$((` can arrive split across + // a continuation; the space-join below would erase them (#3188 review). + if cmd.contains('\\') { + let joined = BASH_JOIN_RE.replace_all(cmd, ""); + if has_heredoc(&joined) || joined.contains("$((") { + return None; + } + } + // 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. @@ -639,7 +650,14 @@ fn comment_start(line: &str) -> Option { } b'\'' if !in_double => in_single = !in_single, b'"' if !in_single => in_double = !in_double, - b'#' if !in_single && !in_double && (i == 0 || bytes[i - 1].is_ascii_whitespace()) => { + // `#` starts a comment at any word start, incl. after an operator + // byte — but not after `{`: `${#var}` is an expansion (#3188 review). + b'#' if !in_single + && !in_double + && (i == 0 + || bytes[i - 1].is_ascii_whitespace() + || matches!(bytes[i - 1], b'|' | b'&' | b';' | b'(' | b')')) => + { return Some(i) } _ => {} @@ -681,32 +699,92 @@ fn line_has_unbalanced_grouping(code: &str) -> bool { paren != 0 || brace != 0 } -/// A list or pipeline that continues across a line break makes adjacent lines -/// one logical command; grouping that spans lines means no line stands alone. -/// Either way, per-line rewriting is unsafe. All checks run against the line -/// with any trailing comment stripped. -fn line_breaks_independence(line: &str) -> bool { - let code = comment_start(line).map_or(line, |i| line[..i].trim_end()); +// Only `\'` inside `$'…'` diverges: bash keeps the string open, the lexer +// closes it — an extra split point the newline-count check can't see (#3188). +fn ansi_c_quote_defeats_lexer(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 && !in_double && bytes.get(i + 1) == Some(&b'\'') => { + i += 2; + while i < bytes.len() { + match bytes[i] { + b'\\' => { + if bytes.get(i + 1) == Some(&b'\'') { + return true; + } + i += 2; + continue; + } + b'\'' => break, + _ => {} + } + i += 1; + } + } + _ => {} + } + i += 1; + } + false +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum LineRole { + Passive, + Independent, + ContinuesNext, + Unsafe, +} + +fn classify_line(line: &str) -> LineRole { + if line.is_empty() || line.starts_with('#') { + return LineRole::Passive; + } + let comment = comment_start(line); + let code = comment.map_or(line, |i| line[..i].trim_end()); let first = code.split_whitespace().next().unwrap_or(""); if BLOCK_KEYWORDS.contains(&first) { - return true; + return LineRole::Unsafe; } - ["&&", "||", "|&", "|"] - .iter() - .any(|op| code.ends_with(op) || code.starts_with(op)) + const CONTINUATION_OPS: [&str; 4] = ["&&", "||", "|&", "|"]; + if CONTINUATION_OPS.iter().any(|op| code.starts_with(op)) || code.starts_with("((") || code.ends_with("))") || line_has_unbalanced_grouping(code) + { + return LineRole::Unsafe; + } + if CONTINUATION_OPS.iter().any(|op| code.ends_with(op)) { + // An operator behind a trailing comment can't be joined textually: + // the comment-blind tokenizer would read the comment as command words. + return if comment.is_some() { + LineRole::Unsafe + } else { + LineRole::ContinuesNext + }; + } + LineRole::Independent } /// Rewrite each line of a multi-line block independently (issue #1243). /// /// Split points are the newline tokens the quote-aware lexer emits, so a /// newline inside a quoted string (e.g. a multi-line commit message) never -/// becomes a boundary. The whole block passes through unchanged unless every -/// line is an independent command (see [`line_breaks_independence`]); blank -/// lines and comment lines are preserved verbatim, as is indentation and the -/// original separator bytes (`\n` vs `\r\n`). +/// becomes a boundary. Lines continued by a trailing `&&`/`||`/`|`/`|&` are +/// joined and rewritten as one logical command through the single-line path; +/// any line [`classify_line`] marks unsafe passes the whole block through. +/// Blank lines and comment lines are preserved verbatim, as is indentation +/// and the original separator bytes (`\n` vs `\r\n`). /// /// If any newline byte was swallowed by quote state, the block passes through /// untouched. The lexer has no comment awareness, so an apostrophe in a `#` @@ -726,11 +804,7 @@ fn rewrite_multiline_block( .map(|t| t.offset) .collect(); - // ANSI-C quoting is the inverse hazard: inside $'...' bash treats \' as a - // literal quote that does NOT close the string, but the lexer thinks it - // does — emitting a split point bash would never honor, which the - // swallowed-newline count check below cannot see. Forgo the rewrite. - if cmd.contains("$'") { + if ansi_c_quote_defeats_lexer(cmd) { return None; } @@ -742,32 +816,59 @@ fn rewrite_multiline_block( let mut segments = Vec::with_capacity(newline_offsets.len() + 1); let mut start = 0; for &off in &newline_offsets { - segments.push(&cmd[start..off]); + segments.push((start, &cmd[start..off])); start = off + 1; } - segments.push(&cmd[start..]); + segments.push((start, &cmd[start..])); - if segments + let roles: Vec = segments .iter() - .map(|seg| seg.trim()) - .filter(|line| !line.is_empty() && !line.starts_with('#')) - .any(line_breaks_independence) - { + .map(|(_, seg)| classify_line(seg.trim())) + .collect(); + if roles.contains(&LineRole::Unsafe) { return None; } let mut any_changed = false; let mut result = String::with_capacity(cmd.len() + 32); - for (i, seg) in segments.iter().enumerate() { + let mut i = 0; + while i < segments.len() { if i > 0 { let off = newline_offsets[i - 1]; result.push_str(&cmd[off..off + 1]); } - let line = seg.trim(); - if line.is_empty() || line.starts_with('#') { + let (seg_off, seg) = segments[i]; + + if roles[i] == LineRole::Passive { result.push_str(seg); + i += 1; continue; } + + let mut end = i; + while roles[end] == LineRole::ContinuesNext { + let mut next = end + 1; + while next < segments.len() && segments[next].1.trim().is_empty() { + next += 1; + } + if next >= segments.len() { + break; + } + if roles[next] == LineRole::Passive { + // Comment line inside a continuation: the comment-blind + // tokenizer would join it as command words (#3188 review). + return None; + } + end = next; + } + + let unit = if end == i { + seg + } else { + let (last_off, last_seg) = segments[end]; + &cmd[seg_off..last_off + last_seg.len()] + }; + let line = unit.trim(); match rewrite_single(line, excluded, transparent_prefixes) { Some(rewritten) if rewritten != line => { any_changed = true; @@ -775,8 +876,9 @@ fn rewrite_multiline_block( result.push_str(indent); result.push_str(&rewritten); } - _ => result.push_str(seg), + _ => result.push_str(unit), } + i = end + 1; } if any_changed { @@ -1468,17 +1570,21 @@ mod tests { } #[test] - fn test_ansi_c_quoting_passes_through() { + fn test_ansi_c_escaped_quote_passes_through() { // Inside $'...' bash treats \' as a literal quote that does not // close the string, so the second line is string content — the - // lexer can't see that, so any $' in the block forgoes the rewrite. + // lexer can't see that, so the block forgoes the rewrite. assert_eq!( rewrite_command_no_prefixes("x=$'foo\\'\ngit status\n'", &[]), None ); + } + + #[test] + fn test_ansi_c_without_escaped_quote_still_rewrites() { assert_eq!( rewrite_command_no_prefixes("echo $'a\\tb'\ngit status", &[]), - None + Some("echo $'a\\tb'\nrtk git status".into()) ); } @@ -1534,18 +1640,18 @@ mod tests { } #[test] - fn test_cross_line_and_list_passes_through() { + fn test_cross_line_and_list_joins_and_rewrites() { assert_eq!( rewrite_command_no_prefixes("git status &&\ngit log -3", &[]), - None + Some("rtk git status && rtk git log -3".into()) ); } #[test] - fn test_cross_line_pipeline_passes_through() { + fn test_cross_line_pipeline_joins_and_rewrites() { assert_eq!( rewrite_command_no_prefixes("git log |\ngrep feat", &[]), - None + Some("git log | rtk grep feat".into()) ); assert_eq!( rewrite_command_no_prefixes("cargo test |&\ngrep FAILED", &[]), @@ -1553,6 +1659,46 @@ mod tests { ); } + #[test] + fn test_cross_line_pipeline_unsafe_final_stage_passes_through() { + assert_eq!( + rewrite_command_no_prefixes("git log |\ngrep -f patterns.txt", &[]), + None + ); + } + + #[test] + fn test_mixed_independent_and_continued_lines() { + assert_eq!( + rewrite_command_no_prefixes("grep -rn foo src\ngit status &&\ngit log -3", &[]), + Some("rtk grep -rn foo src\nrtk git status && rtk git log -3".into()) + ); + } + + #[test] + fn test_blank_line_inside_continuation_joins() { + assert_eq!( + rewrite_command_no_prefixes("git status &&\n\ngit log -3", &[]), + Some("rtk git status && rtk git log -3".into()) + ); + } + + #[test] + fn test_comment_line_inside_continuation_passes_through() { + assert_eq!( + rewrite_command_no_prefixes("git status &&\n# note\ngit log -3", &[]), + None + ); + } + + #[test] + fn test_comment_directly_after_operator_passes_through() { + assert_eq!( + rewrite_command_no_prefixes("git log |# keep pipeline\ngrep -f patterns.txt", &[]), + None + ); + } + #[test] fn test_subshell_spanning_lines_passes_through() { assert_eq!(rewrite_command_no_prefixes("(\n git status\n)", &[]), None); @@ -1570,6 +1716,22 @@ mod tests { None ); } + + #[test] + fn test_heredoc_split_by_line_continuation_passes_through() { + assert_eq!( + rewrite_command_no_prefixes("cat <\\\n PipelineAnalysis { From 3c8892f5c08d23d857643ff2c8dc4ee2dfeee549 Mon Sep 17 00:00:00 2001 From: Adrien Eppling Date: Thu, 30 Jul 2026 14:54:55 +0200 Subject: [PATCH 5/6] fix(rewrite): keep rewriting quoted multi-line strings as one command --- src/discover/registry.rs | 44 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 41 insertions(+), 3 deletions(-) diff --git a/src/discover/registry.rs b/src/discover/registry.rs index a48610ae03..0030a30f25 100644 --- a/src/discover/registry.rs +++ b/src/discover/registry.rs @@ -738,6 +738,26 @@ fn ansi_c_quote_defeats_lexer(cmd: &str) -> bool { false } +fn quotes_balanced(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, + _ => {} + } + i += 1; + } + !in_single && !in_double +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum LineRole { Passive, @@ -810,6 +830,12 @@ fn rewrite_multiline_block( let raw_breaks = cmd.chars().filter(|c| matches!(c, '\n' | '\r')).count(); if raw_breaks != newline_offsets.len() { + // Every newline swallowed by quote state with quotes balanced at EOF + // is one logical command (a multi-line commit message), not a hidden + // extra line; rewrite it whole, as develop always did (#3319 fuzz). + if newline_offsets.is_empty() && quotes_balanced(cmd) { + return rewrite_single(cmd, excluded, transparent_prefixes); + } return None; } @@ -1485,11 +1511,23 @@ mod tests { } #[test] - fn test_newline_inside_quotes_passes_through() { - // A swallowed newline means the block can't be split safely — - // the quoted body must never be treated as a command line of its own. + fn test_newline_inside_quotes_rewrites_as_one_command() { + // The quoted body is never treated as a command line of its own; + // the whole thing is one logical command and gets one prefix. assert_eq!( rewrite_command_no_prefixes("git commit -m \"subject\ngit status in body\"", &[]), + Some("rtk git commit -m \"subject\ngit status in body\"".into()) + ); + assert_eq!( + rewrite_command_no_prefixes("git commit -m 'multi\nline\nmessage'", &[]), + Some("rtk git commit -m 'multi\nline\nmessage'".into()) + ); + } + + #[test] + fn test_unbalanced_swallowed_newline_passes_through() { + assert_eq!( + rewrite_command_no_prefixes("git commit -m \"subject\ngit status", &[]), None ); } From b09da2d0f804cdb61e9f7f1a452803d028212408 Mon Sep 17 00:00:00 2001 From: Adrien Eppling Date: Sat, 1 Aug 2026 14:31:54 +0200 Subject: [PATCH 6/6] fix(rewrite): bail on cross-line [[ ]], share quote-state walker --- src/discover/registry.rs | 226 ++++++++++++++++++++++++--------------- 1 file changed, 137 insertions(+), 89 deletions(-) diff --git a/src/discover/registry.rs b/src/discover/registry.rs index 0030a30f25..6469b178d8 100644 --- a/src/discover/registry.rs +++ b/src/discover/registry.rs @@ -633,129 +633,150 @@ const BLOCK_KEYWORDS: &[&str] = &[ "select", "function", "coproc", "{", "}", "(", ")", ]; +/// Shared quote-state byte walker used by all line scanners. Yields +/// `(offset, byte, in_single_before, in_double_before)`, skipping backslash +/// escape pairs outside single quotes and toggling quote state — the same +/// model the lexer applies. +struct QuoteScan<'a> { + bytes: &'a [u8], + i: usize, + in_single: bool, + in_double: bool, +} + +impl<'a> QuoteScan<'a> { + fn new(s: &'a str) -> Self { + Self { + bytes: s.as_bytes(), + i: 0, + in_single: false, + in_double: false, + } + } + + fn balanced(&self) -> bool { + !self.in_single && !self.in_double + } +} + +impl Iterator for QuoteScan<'_> { + type Item = (usize, u8, bool, bool); + + fn next(&mut self) -> Option { + while self.i < self.bytes.len() { + let i = self.i; + let b = self.bytes[i]; + if b == b'\\' && !self.in_single { + self.i += 2; + continue; + } + let item = (i, b, self.in_single, self.in_double); + match b { + b'\'' if !self.in_double => self.in_single = !self.in_single, + b'"' if !self.in_single => self.in_double = !self.in_double, + _ => {} + } + self.i += 1; + return Some(item); + } + None + } +} + /// Byte offset where an unquoted `#` at the start of a word begins a trailing /// comment, if any. The lexer has no comment state, so the independence checks /// must ignore comment text themselves: `git log | # keep pipeline` continues /// the pipeline across the newline even though the line ends in comment text. fn comment_start(line: &str) -> Option { let bytes = line.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, - // `#` starts a comment at any word start, incl. after an operator - // byte — but not after `{`: `${#var}` is an expansion (#3188 review). - b'#' if !in_single - && !in_double - && (i == 0 - || bytes[i - 1].is_ascii_whitespace() - || matches!(bytes[i - 1], b'|' | b'&' | b';' | b'(' | b')')) => - { - return Some(i) - } - _ => {} - } - i += 1; - } - None + // `#` starts a comment at any word start, incl. after an operator + // byte — but not after `{`: `${#var}` is an expansion (#3188 review). + QuoteScan::new(line).find_map(|(i, b, in_single, in_double)| { + (b == b'#' + && !in_single + && !in_double + && (i == 0 + || bytes[i - 1].is_ascii_whitespace() + || matches!(bytes[i - 1], b'|' | b'&' | b';' | b'(' | b')'))) + .then_some(i) + }) } /// Unquoted `(`/`)` or `{`/`}` that don't balance within the line: an array /// literal (`arr=(one`), function body (`foo() {`), or group spans lines, so /// the lines around it are not independent commands. fn line_has_unbalanced_grouping(code: &str) -> bool { - let bytes = code.as_bytes(); - let mut in_single = false; - let mut in_double = false; let mut paren = 0i32; let mut brace = 0i32; - 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 && !in_double => paren += 1, - b')' if !in_single && !in_double => paren -= 1, - b'{' if !in_single && !in_double => brace += 1, - b'}' if !in_single && !in_double => brace -= 1, + for (_, b, in_single, in_double) in QuoteScan::new(code) { + if in_single || in_double { + continue; + } + match b { + b'(' => paren += 1, + b')' => paren -= 1, + b'{' => brace += 1, + b'}' => brace -= 1, _ => {} } if paren < 0 || brace < 0 { return true; } - i += 1; } paren != 0 || brace != 0 } +/// Unquoted `[[` / `]]` words that don't balance within the line: bash allows +/// a conditional expression to span lines (`[[ -f a &&` / `-f b ]]`), so the +/// surrounding lines are not independent commands. +fn line_has_unbalanced_test_brackets(code: &str) -> bool { + let bytes = code.as_bytes(); + let mut depth = 0i32; + for (i, b, in_single, in_double) in QuoteScan::new(code) { + if in_single || in_double || !matches!(b, b'[' | b']') { + continue; + } + let word_start = i == 0 || bytes[i - 1].is_ascii_whitespace(); + let word_end = bytes.get(i + 2).is_none_or(|c| c.is_ascii_whitespace()); + if bytes.get(i + 1) == Some(&b) && word_start && word_end { + depth += if b == b'[' { 1 } else { -1 }; + if depth < 0 { + return true; + } + } + } + depth != 0 +} + // Only `\'` inside `$'…'` diverges: bash keeps the string open, the lexer // closes it — an extra split point the newline-count check can't see (#3188). fn ansi_c_quote_defeats_lexer(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; + let mut ansi_span = false; + let mut backslash_run = 0u32; + for (i, b, in_single, in_double) in QuoteScan::new(cmd) { + if b == b'\'' && !in_double { + if !in_single { + ansi_span = i > 0 && bytes[i - 1] == b'$'; + backslash_run = 0; + } else if ansi_span && backslash_run % 2 == 1 { + return true; } - b'\'' if !in_double => in_single = !in_single, - b'"' if !in_single => in_double = !in_double, - b'$' if !in_single && !in_double && bytes.get(i + 1) == Some(&b'\'') => { - i += 2; - while i < bytes.len() { - match bytes[i] { - b'\\' => { - if bytes.get(i + 1) == Some(&b'\'') { - return true; - } - i += 2; - continue; - } - b'\'' => break, - _ => {} - } - i += 1; - } + } else if in_single { + if b == b'\\' { + backslash_run += 1; + } else { + backslash_run = 0; } - _ => {} } - i += 1; } false } fn quotes_balanced(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, - _ => {} - } - i += 1; - } - !in_single && !in_double + let mut scan = QuoteScan::new(cmd); + scan.by_ref().for_each(drop); + scan.balanced() } #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -781,6 +802,7 @@ fn classify_line(line: &str) -> LineRole { || code.starts_with("((") || code.ends_with("))") || line_has_unbalanced_grouping(code) + || line_has_unbalanced_test_brackets(code) { return LineRole::Unsafe; } @@ -801,7 +823,9 @@ fn classify_line(line: &str) -> LineRole { /// Split points are the newline tokens the quote-aware lexer emits, so a /// newline inside a quoted string (e.g. a multi-line commit message) never /// becomes a boundary. Lines continued by a trailing `&&`/`||`/`|`/`|&` are -/// joined and rewritten as one logical command through the single-line path; +/// joined and rewritten as one logical command through the single-line path — +/// joining is not byte-preserving: separators inside a joined unit collapse +/// to single spaces (see `test_blank_line_inside_continuation_joins`); /// any line [`classify_line`] marks unsafe passes the whole block through. /// Blank lines and comment lines are preserved verbatim, as is indentation /// and the original separator bytes (`\n` vs `\r\n`). @@ -828,6 +852,8 @@ fn rewrite_multiline_block( return None; } + // The lexer emits one newline token per `\r` and per `\n` (CRLF = two + // tokens), so the parity check must count both bytes individually. let raw_breaks = cmd.chars().filter(|c| matches!(c, '\n' | '\r')).count(); if raw_breaks != newline_offsets.len() { // Every newline swallowed by quote state with quotes balanced at EOF @@ -888,6 +914,8 @@ fn rewrite_multiline_block( end = next; } + // A joined unit is rebuilt through the single-line path: interior + // newlines and blank lines collapse to single spaces, not preserved. let unit = if end == i { seg } else { @@ -1737,6 +1765,26 @@ mod tests { ); } + #[test] + fn test_conditional_expression_spanning_lines_passes_through() { + assert_eq!( + rewrite_command_no_prefixes("[[ -f a &&\n-f b ]]\ngit status", &[]), + None + ); + assert_eq!( + rewrite_command_no_prefixes("git status\n[[\n-f a ]]", &[]), + None + ); + } + + #[test] + fn test_balanced_conditional_line_still_rewrites() { + assert_eq!( + rewrite_command_no_prefixes("[[ -x foo ]] &&\ngit status", &[]), + Some("[[ -x foo ]] && rtk git status".into()) + ); + } + #[test] fn test_subshell_spanning_lines_passes_through() { assert_eq!(rewrite_command_no_prefixes("(\n git status\n)", &[]), None);