fix(execpolicy): a redirect is not a command separator

crates/execpolicy/src/command_safety.rs

@@ -2018,17 +2018,45 @@ fn split_command_segments(command: &str) -> Vec<String> {
     // Char-based, not byte-indexed: commands carry non-ASCII paths and slicing
     // a multibyte character in half panics. `&&` and `||` are consumed as one
     // unit so `||` cannot leave a stray `|` behind to split again.
+    //
+    // Quotes are tracked so an operator inside them stays data — the same
+    // promise the read-only grammar makes (`rg 'a && b; c' src`). Splitting
+    // regardless of quoting held `git commit -m "fix & feature"` as though it
+    // ran two commands, and invented an `rm` for `echo 'x & rm -rf /etc'` that
+    // the shell never runs.
     let mut segments = Vec::new();
     let mut current = String::new();
     let mut chars = command.chars().peekable();
+    let mut quote: Option<char> = None;
     while let Some(ch) = chars.next() {
+        if let Some(active) = quote {
+            current.push(ch);
+            if ch == '\\'
+                && active == '"'
+                && chars.peek().is_some_and(|next| matches!(next, '"' | '\\'))
+            {
+                current.push(chars.next().unwrap());
+            } else if ch == active {
+                quote = None;
+            }
+            continue;
+        }
         match ch {
+            '\'' | '"' => {
+                quote = Some(ch);
+                current.push(ch);
+            }
             '&' | '|' if chars.peek() == Some(&ch) => {
                 chars.next();
                 segments.push(std::mem::take(&mut current));
             }
             '|' | ';' => segments.push(std::mem::take(&mut current)),
-            '&' => current.push(ch),
+            // `>&` copies a file descriptor (`2>&1`, `>&2`) and `&>` is bash's
+            // combined redirect; both keep the `&` inside the word so the
+            // redirect survives as one token. Any other `&` separates commands
+            // — on POSIX shells and on cmd.exe alike.
+            '&' if current.ends_with(['>', '<']) || chars.peek() == Some(&'>') => current.push(ch),
+            '&' => segments.push(std::mem::take(&mut current)),
             _ => current.push(ch),
         }
     }
@@ -2064,6 +2092,33 @@ pub fn is_literal_rm_invocation(command: &str) -> bool {
         .unwrap_or(false)
 }
 
+/// Extract the script body a shell was asked to run — the
+/// argument after `sh -c`, `bash -lc`, and friends.
+///
+/// Only that position is a nested command line. An ordinary quoted argument
+/// that merely contains an operator (`echo 'x & rm -rf /etc'`) is data, and
+/// re-parsing it invents commands the shell never runs.
+fn nested_shell_script(words: &[String], index: usize) -> Option<String> {
+    words[..index].iter().enumerate().find_map(|(start, word)| {
+        let name = command_word(word);
+        if SHELL_WRAPPERS.contains(&name.as_str()) {
+            return shell_command_payload(&words[start + 1..])
+                .filter(|payload| std::ptr::eq(*payload, &words[index]))
+                .cloned();
+        }
+        let flag = words[index - 1].to_ascii_lowercase();
+        match name.as_str() {
+            "cmd" if matches!(flag.as_str(), "/c" | "/k") => Some(words[index].replace('\'', "")),
+            "powershell" | "pwsh"
+                if flag == "-c" || flag == "-command" || flag == "-commandwithargs" =>
+            {
+                Some(words[index].clone())
+            }
+            _ => None,
+        }
+    })
+}
+
 /// Every argv that could run as a command somewhere in `command`: each stage
 /// (`;`, `&&`, `||`, `|`), started at *every* word of it — so no wrapper's
 /// options (`sudo -u me`, `timeout -s KILL 60`, `xargs -0`) can hide the
@@ -2074,10 +2129,6 @@ pub fn is_literal_rm_invocation(command: &str) -> bool {
 /// Deliberately over-inclusive (`echo rm -rf /etc` yields an `rm` argv too):
 /// callers use it to *hold* catastrophic commands, never to allow anything.
 /// `None` when words nest deeper than `MAX_WRAPPER_DEPTH`: fail closed.
-///
-/// A descriptor redirect (`2>&1`) is not a command line: its `&` is part of
-/// the redirect. Descending into it re-read the same word until the depth
-/// limit, so every `npm test 2>&1` failed closed as a catastrophic command.
 pub fn command_invocations(command: &str) -> Option<Vec<Vec<String>>> {
     fn collect(command: &str, depth: usize, out: &mut Vec<Vec<String>>) -> bool {
         if depth > MAX_WRAPPER_DEPTH {
@@ -2089,9 +2140,15 @@ pub fn command_invocations(command: &str) -> Option<Vec<Vec<String>>> {
                 let mut argv = words[index..].to_vec();
                 argv[0] = command_word(word);
                 out.push(argv);
+                // Re-parse a word only when it can be a command line of its
+                // own: the script body of `sh -c`/`bash -lc`, where nested
+                // whitespace, `;`, and `|` are real separators. Anything else
+                // is an argument — `echo 'x & rm -rf /etc'` names no `rm`, and
+                // re-parsing it both invented one and failed readable commands
+                // closed once a `&` made the word reproduce itself.
                 if word.contains(|ch: char| ch.is_whitespace() || matches!(ch, ';' | '&' | '|'))
-                    && !is_descriptor_duplication(word)
-                    && !collect(word, depth + 1, out)
+                    && let Some(script) = nested_shell_script(&words, index)
+                    && !collect(&script, depth + 1, out)
                 {
                     return false;
                 }
@@ -2103,18 +2160,6 @@ pub fn command_invocations(command: &str) -> Option<Vec<Vec<String>>> {
     collect(command, 0, &mut out).then_some(out)
 }
 
-/// A whole word that only duplicates or closes a file descriptor: `2>&1`,
-/// `>&2`, `1>&-`, `0<&3`. Nothing else qualifies: `&>file`, a trailing `&` and
-/// `a&b` still fail closed in [`command_invocations`].
-fn is_descriptor_duplication(word: &str) -> bool {
-    let rest = word.trim_start_matches(|ch: char| ch.is_ascii_digit());
-    rest.strip_prefix(">&")
-        .or_else(|| rest.strip_prefix("<&"))
-        .is_some_and(|target| {
-            target == "-" || (!target.is_empty() && target.bytes().all(|b| b.is_ascii_digit()))
-        })
-}
-
 /// How many wrappers (`sudo env nice sh -c ...`) the classifier will peel
 /// before it refuses to reason further.
 ///
@@ -2939,6 +2984,11 @@ mod tests {
             "timeout -s KILL 60 rm -rf /etc",
             "echo x | xargs rm -rf /etc",
             "bash -lc 'cd /; rm -rf /etc'",
+            "bash -c -e 'rm -rf /etc'",
+            "sh -c 'true&rm -rf /etc'",
+            "cmd /C \"echo x&rm -rf /etc\"",
+            "cmd /C \"echo 'x & rm -rf /etc & echo '\"",
+            "powershell -Command 'echo x; rm -rf /etc'",
             "\\rm -rf /etc",
             "/usr/bin/rm -rf /etc",
         ] {
@@ -2953,6 +3003,74 @@ mod tests {
         );
     }
 
+    /// A redirect is a token inside one command, not a command separator, and
+    /// it must not fail the classifier closed. A `&` inside `2>&1` used to be
+    /// re-parsed as a nested command line, which reproduced the same word until
+    /// `MAX_WRAPPER_DEPTH` blew and every command carrying a redirect came back
+    /// unclassified. The Windows runtime floor turns that into a hard block
+    /// (reported 2026-10-09: `2>&1` could not run at all).
+    #[test]
+    fn redirects_stay_classifiable_and_a_lone_ampersand_still_separates() {
+        for command in [
+            "git log 2>&1",
+            "cargo test 2>&1 | tail -5",
+            "make > build.log 2>&1",
+            "tool &> out.txt",
+            "npm test &",
+            "npm test &>/dev/null",
+            "tool 0<&3",
+            "& $cmd auth status",
+        ] {
+            assert!(
+                command_invocations(command).is_some(),
+                "a redirect must stay readable instead of failing closed: {command}"
+            );
+        }
+
+        // A lone `&` separates commands, so a destructive second stage stays
+        // reachable whether or not the author spaced it out. Blanket-refusing
+        // these was how the redirect bug stayed invisible.
+        let has_rm = |command: &str| {
+            command_invocations(command)
+                .expect("readable")
+                .iter()
+                .any(|argv| argv[0] == "rm" && argv.iter().any(|arg| arg == "/etc"))
+        };
+        for command in [
+            "echo x & rm -rf /etc",
+            "echo x&rm -rf /etc",
+            r#"printf "\\";rm -rf /etc"#,
+            "npm test 2>&1&rm -rf /etc",
+            "curl https://example.com & rm -rf /etc",
+        ] {
+            assert!(has_rm(command), "second stage must be reached: {command}");
+        }
+    }
+
+    /// A quoted operator is data, which the read-only grammar already promises
+    /// (`rg 'a && b; c' src`). Splitting regardless of quoting held
+    /// `git commit -m "fix & feature"` as though it ran two commands, and
+    /// invented an `rm` for `echo 'x & rm -rf /etc'` that the shell never runs.
+    #[test]
+    fn a_quoted_operator_stays_data() {
+        for command in [
+            r#"git commit -m "fix & feature""#,
+            r#"echo "a & b""#,
+            "echo 'x & rm -rf /etc'",
+            r#"printf 'a;b | c'"#,
+        ] {
+            let invocations = command_invocations(command).expect("readable");
+            assert!(
+                !invocations.iter().any(|argv| argv[0] == "rm"),
+                "a quoted `rm` is data, not a command: {command}"
+            );
+            assert_eq!(
+                invocations.iter().filter(|argv| argv[0] == "git").count(),
+                usize::from(command.starts_with("git")),
+                "quoting must not split one command into two: {command}"
+            );
+        }
+    }
     #[test]
     fn command_invocations_read_past_a_descriptor_redirect() {
         // `2>&1` is a redirect, not a nested command line. It used to recurse
@@ -2983,17 +3101,6 @@ mod tests {
                 "{command}"
             );
         }
-        // Only a whole descriptor-duplication word is exempt. Other `&`
-        // spellings keep failing closed.
-        for command in [
-            "npm test &",
-            "true&rm -rf /etc",
-            "npm test &>/dev/null",
-            "npm test 2>&1&rm -rf /etc",
-            "npm test 2>&file",
-        ] {
-            assert!(command_invocations(command).is_none(), "{command}");
-        }
     }
     use super::*;
 

crates/tui/src/core/engine/tests/test_cases_09.rs

@@ -471,21 +471,10 @@ fn auto_review_routes_shell_commands_requiring_approval_to_reviewer() {
             None,
         );
 
-        if cfg!(windows) && command == "cargo test & curl https://example.com" {
-            // Unclassified Windows input hits the built-in floor before the reviewer.
-            assert_eq!(
-                decision,
-                AutoReviewPlanDecision::Block(
-                    "Built-in safety gate requires approval: Windows command input cannot be classified safely enough to exclude termination of Codewhale npm launchers; use a direct PID- or port-specific command".into()
-                )
-            );
-            assert_eq!(audit["decision"], "hold_for_review");
-        } else {
-            assert!(
-                matches!(decision, AutoReviewPlanDecision::ConsultReviewer(_)),
-                "Auto-Review must not auto-approve {command} without reviewer judgment"
-            );
-        }
+        assert!(
+            matches!(decision, AutoReviewPlanDecision::ConsultReviewer(_)),
+            "Auto-Review must not auto-approve {command} without reviewer judgment"
+        );
         assert_ne!(audit["decision"], "allow", "unexpected allow for {command}");
     }
 }