diff --git a/codex-rs/shell-command/src/bash.rs b/codex-rs/shell-command/src/bash.rs index 06b21f1415..fe7d5a2335 100644 --- a/codex-rs/shell-command/src/bash.rs +++ b/codex-rs/shell-command/src/bash.rs @@ -141,32 +141,35 @@ pub fn extract_bash_command_joined(command: &[String]) -> Option<(String, String if rest.len() < 2 || !is_recognised_bash_lc_invocation(shell, flag) { return None; } - Some((shell.clone(), join_flattened_shell_script(rest))) -} - -fn join_flattened_shell_script(tokens: &[String]) -> String { let mut script = String::new(); - for token in tokens { + for token in rest { if !script.is_empty() { script.push(' '); } - if is_shell_operator_token(token) { + let without_fd = token.trim_start_matches(|ch: char| ch.is_ascii_digit()); + let is_operator = token.chars().any(|ch| "<>|&;()".contains(ch)) + && token + .chars() + .all(|ch| ch.is_ascii_digit() || "<>|&;()".contains(ch)); + let is_attached_redirection = [ + "&>>", "<<<", ">>", "<<", "<>", ">|", ">&", "<&", "&>", ">", "<", + ] + .iter() + .any(|operator| { + without_fd + .strip_prefix(operator) + .is_some_and(|target| !target.is_empty()) + }); + if is_operator || is_attached_redirection { script.push_str(token); } else { let Ok(quoted) = shlex::try_quote(token) else { - return "".to_string(); + return Some((shell.clone(), "".to_string())); }; script.push_str("ed); } } - script -} - -fn is_shell_operator_token(token: &str) -> bool { - token.chars().any(|ch| "<>|&;()".contains(ch)) - && token - .chars() - .all(|ch| ch.is_ascii_digit() || "<>|&;()".contains(ch)) + Some((shell.clone(), script)) } /// Returns the sequence of plain commands within a `bash -lc "..."` or @@ -176,7 +179,7 @@ fn is_shell_operator_token(token: &str) -> bool { /// Uses the strict `extract_bash_command`, so flattened argv returns None /// rather than being rich-parsed: disambiguating "tail is script" from "tail /// is POSIX `$0`/`$1`" isn't safe here. Such argv instead lands in -/// `single_unknown_for_command`'s joined fallback as `Unknown { cmd }`. +/// `single_unknown_for_command` as `Unknown { cmd }` with the wrapper intact. pub fn parse_shell_lc_plain_commands(command: &[String]) -> Option>> { let (_, script) = extract_bash_command(command)?; @@ -426,6 +429,21 @@ mod tests { assert_eq!(script, "rg 'foo bar' | head > /tmp/out 2>&1 ; echo done"); } + #[test] + fn extract_bash_command_joined_preserves_attached_redirections() { + let (_, script) = extract_bash_command_joined(&[ + "zsh".to_string(), + "-lc".to_string(), + "command".to_string(), + "2>/tmp/stderr".to_string(), + ">output".to_string(), + ">>log".to_string(), + "&>/tmp/all".to_string(), + ]) + .expect("known shell + -lc + tail should match"); + assert_eq!(script, "command 2>/tmp/stderr >output >>log &>/tmp/all"); + } + #[test] fn extract_bash_command_joined_accepts_absolute_shell_path() { let (shell, script) = extract_bash_command_joined(&[ diff --git a/codex-rs/shell-command/src/parse_command.rs b/codex-rs/shell-command/src/parse_command.rs index fde617d321..546d1015fe 100644 --- a/codex-rs/shell-command/src/parse_command.rs +++ b/codex-rs/shell-command/src/parse_command.rs @@ -53,11 +53,6 @@ fn single_unknown_for_command(command: &[String]) -> ParsedCommand { cmd: shell_command.to_string(), }; } - // Fall back for argv flattened past `[shell, flag, script]` before - // leaking the wrapper into shlex_join below. - if let Some((_, script)) = crate::bash::extract_bash_command_joined(command) { - return ParsedCommand::Unknown { cmd: script }; - } ParsedCommand::Unknown { cmd: shlex_join(command), } @@ -81,14 +76,18 @@ mod tests { } #[test] - fn flattened_shell_wrapper_strips_to_script_only() { - // Flattened argv used to fall through to shlex_join, leaking the - // wrapper into Unknown.cmd; the joined fallback now stitches the tail. - let parsed = parse_command(&vec_str(&["zsh", "-lc", "touch", "/tmp/foo"])); + fn shell_wrapper_with_positional_args_remains_intact() { + let parsed = parse_command(&vec_str(&[ + "bash", + "-c", + "echo \"$0\" \"$1\"", + "tool", + "arg", + ])); assert_eq!( parsed, vec![ParsedCommand::Unknown { - cmd: "touch /tmp/foo".to_string(), + cmd: "bash -c 'echo \"$0\" \"$1\"' tool arg".to_string(), }] ); } @@ -131,22 +130,6 @@ mod tests { assert!(crate::bash::extract_bash_command_joined(&argv).is_none()); } - #[test] - fn flattened_shell_wrapper_handles_absolute_shell_path() { - let parsed = parse_command(&vec_str(&[ - "/opt/homebrew/bin/zsh", - "-lc", - "touch", - "/tmp/foo", - ])); - assert_eq!( - parsed, - vec![ParsedCommand::Unknown { - cmd: "touch /tmp/foo".to_string(), - }] - ); - } - fn assert_parsed(args: &[String], expected: Vec) { let out = parse_command(args); assert_eq!(out, expected);