From 40746553457e3753cdebca17fd2bcfc5af6c8abd Mon Sep 17 00:00:00 2001 From: penpal Date: Sat, 30 May 2026 12:09:55 +0545 Subject: [PATCH] fix(proxy): reject shell snippets in single-arg form (#2163) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `rtk proxy` executes the target binary directly via `Command::spawn`, with no shell in between. When a user wrapped a compound shell snippet in a single quoted argument (`rtk proxy 'for i in 1 2; do echo $i; done'`), `shell_split` produced tokens like `["for", "i", "in", "1", "2;", ...]` and operators (`;`, `&&`, `|`, `$()`, `>`, `&`) silently became positional arguments to the first binary. Worst case: token-like fragments created unintended filesystem entries. Add `first_unquoted_shell_metachar` that walks the raw single-arg string with the same quote state machine as `shell_split`, returning the first metacharacter that sits outside `'…'` / `"…"`. The proxy single-arg branch consults it before invoking `shell_split` and bails with a clear error message pointing at `rtk proxy sh -c '…'` for users who genuinely want shell semantics. Quoted operators stay legitimate payloads (`--format="%H %s"`, commit messages containing `&&`, etc.) — only unquoted metachars trip the rejection. Multi-arg form (`rtk proxy git log -1`) is unaffected since it never goes through `shell_split`. Covered by four new unit tests in `tests`: - simple commands accepted - quoted metacharacters ignored - bare operators (`;`, `|`, `&&`, `>`, `<`, `$(`, backtick, `(`) rejected - the issue #2163 compound-loop reproducer rejected Signed-off-by: penpal --- src/main.rs | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 101 insertions(+) diff --git a/src/main.rs b/src/main.rs index 992f865a2..3693b7d3d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1314,6 +1314,38 @@ fn shell_split(input: &str) -> Vec { discover::lexer::shell_split(input) } +/// Returns the first unquoted shell metacharacter in `input`, or `None`. +/// +/// `rtk proxy` executes commands directly via `Command::spawn`, with no shell +/// in between. Snippets that rely on shell features (`;`, `&&`, `|`, `$()`, +/// redirects, loops, …) silently misbehave: operators get passed as positional +/// args to the first binary, which can produce surprising filesystem effects +/// (#2163). This helper walks the raw single-arg string with the same quote +/// state machine as [`shell_split`], so metacharacters that appear inside +/// `'…'` or `"…"` are intentionally ignored — those are legitimate argument +/// payloads (e.g. `--format="%H %s"`). +fn first_unquoted_shell_metachar(input: &str) -> Option { + let mut chars = input.chars().peekable(); + let mut in_single = false; + let mut in_double = false; + while let Some(c) = chars.next() { + match c { + '\\' if !in_single => { + chars.next(); + } + '\'' if !in_double => in_single = !in_single, + '"' if !in_single => in_double = !in_double, + _ if in_single || in_double => {} + ';' | '|' | '&' | '>' | '<' | '`' | '(' | ')' | '{' | '}' | '\n' => { + return Some(c); + } + '$' if chars.peek() == Some(&'(') => return Some('$'), + _ => {} + } + } + None +} + /// Merge pnpm global filters args with other ones for standard String-based commands fn merge_pnpm_args(filters: &[String], args: &[String]) -> Vec { filters @@ -2285,6 +2317,20 @@ fn run_cli() -> Result { // e.g. rtk proxy 'git log --format="%H %s"' → cmd=git, args=["log", "--format=%H %s"] let (cmd_name, cmd_args): (String, Vec) = if args.len() == 1 { let full = args[0].to_string_lossy(); + // Reject shell snippets (#2163). Proxy runs commands directly via + // exec — operators like `;`, `&&`, `|`, `$()`, `>` would otherwise + // be passed as positional args to the first binary, causing silent + // misbehavior or filesystem garbage from token-like fragments. + if let Some(meta) = first_unquoted_shell_metachar(&full) { + anyhow::bail!( + "proxy refuses shell snippet (unquoted '{}' in: {}).\n\ + rtk proxy executes a single command directly without a shell.\n\ + For pipes, &&, $(), redirects, or loops, wrap with:\n \ + rtk proxy sh -c ''", + meta, + full + ); + } let parts = shell_split(&full); if parts.len() > 1 { (parts[0].clone(), parts[1..].to_vec()) @@ -3272,4 +3318,59 @@ mod tests { _ => panic!("Expected Init command"), } } + + // #2163: proxy must refuse shell snippets that would otherwise have their + // operators silently re-interpreted as positional args to the first binary. + #[test] + fn test_proxy_metachar_detector_accepts_simple_commands() { + assert_eq!(first_unquoted_shell_metachar("head -50 file.php"), None); + assert_eq!(first_unquoted_shell_metachar("git status"), None); + assert_eq!(first_unquoted_shell_metachar("ls -la /tmp/foo"), None); + } + + // Quoted operators are legitimate payloads (e.g. `--format="%H %s"`, + // commit messages containing `&&`) and must not trigger the rejection. + #[test] + fn test_proxy_metachar_detector_ignores_quoted_metachars() { + assert_eq!( + first_unquoted_shell_metachar(r#"git log --format="%H %s""#), + None + ); + assert_eq!( + first_unquoted_shell_metachar(r#"git commit -m "fix && cleanup""#), + None + ); + assert_eq!(first_unquoted_shell_metachar("grep -r 'a | b' ."), None); + assert_eq!( + first_unquoted_shell_metachar(r#"echo "value=$(date)""#), + None + ); + // Backslash escapes the next char outside single quotes. + assert_eq!(first_unquoted_shell_metachar(r"echo a\&b"), None); + } + + #[test] + fn test_proxy_metachar_detector_rejects_unquoted_operators() { + assert_eq!(first_unquoted_shell_metachar("a; b"), Some(';')); + assert_eq!(first_unquoted_shell_metachar("a | b"), Some('|')); + assert_eq!(first_unquoted_shell_metachar("a && b"), Some('&')); + assert_eq!(first_unquoted_shell_metachar("cmd > out"), Some('>')); + assert_eq!(first_unquoted_shell_metachar("cmd < in"), Some('<')); + assert_eq!(first_unquoted_shell_metachar("echo $(date)"), Some('$')); + assert_eq!(first_unquoted_shell_metachar("echo `date`"), Some('`')); + assert_eq!(first_unquoted_shell_metachar("(a)"), Some('(')); + assert_eq!( + first_unquoted_shell_metachar("for i in 1 2; do echo $i; done"), + Some(';') + ); + } + + // The exact reproducer shape from #2163 — compound for-loop with command + // substitution, pipes, redirects, and background jobs — must be caught at + // the first unquoted metachar regardless of how deep the snippet runs. + #[test] + fn test_proxy_metachar_detector_rejects_issue_2163_repro() { + let snippet = "for i in $(seq 1 3); do echo $i | tee -a out.txt > /dev/null & done"; + assert!(first_unquoted_shell_metachar(snippet).is_some()); + } }