diff --git a/codex-rs/core/src/shell.rs b/codex-rs/core/src/shell.rs index ac115facb6..2cb9bfd245 100644 --- a/codex-rs/core/src/shell.rs +++ b/codex-rs/core/src/shell.rs @@ -1,7 +1,16 @@ +use crate::bash::parse_shell_lc_plain_commands; use serde::Deserialize; use serde::Serialize; +use std::path::Path; use std::path::PathBuf; +/// Plain commands (no redirects/subshell/etc.) that are safe to run without +/// login-shell initialization. +const NON_LOGIN_SHELL_ALLOWLIST: &[&str] = &[ + "cat", "cd", "echo", "false", "find", "grep", "head", "ls", "nl", "pwd", "rm", + "rmdir", "sed", "tail", "true", "wc", +]; + #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)] pub enum ShellType { Zsh, @@ -58,6 +67,32 @@ impl Shell { } } } + + /// Builds shell exec args for `script`, defaulting to login-shell semantics + /// unless we detect a safe, allowlisted script. An explicit `login` flag + /// overrides the default. + pub fn build_command(&self, script: &str, login: Option) -> Vec { + let use_login_shell = login.unwrap_or_else(|| match self.shell_type { + ShellType::PowerShell | ShellType::Cmd => true, + ShellType::Zsh | ShellType::Bash | ShellType::Sh => { + !uses_allowlisted_plain_commands(&self.derive_exec_args(script, true)) + } + }); + self.derive_exec_args(script, use_login_shell) + } +} + +fn uses_allowlisted_plain_commands(command: &[String]) -> bool { + parse_shell_lc_plain_commands(command) + .filter(|commands| !commands.is_empty()) + .is_some_and(|commands| { + commands.iter().all(|cmd| { + cmd.first() + .and_then(|cmd0| Path::new(cmd0).file_name()) + .and_then(|name| name.to_str()) + .is_some_and(|cmd0| NON_LOGIN_SHELL_ALLOWLIST.contains(&cmd0)) + }) + }) } #[cfg(unix)] @@ -374,6 +409,41 @@ mod tests { ); } + #[test] + fn allowlisted_bash_script_skips_login_shell() { + let shell = Shell { + shell_type: ShellType::Bash, + shell_path: PathBuf::from("/bin/bash"), + }; + let command = shell.build_command("cd foo && ls", None); + assert_eq!(command[1], "-c"); + let command = shell.build_command("cd foo && cargo build", None); + assert_eq!(command[1], "-lc"); + } + + #[test] + fn non_allowlisted_bash_script_keeps_login_shell() { + let shell = Shell { + shell_type: ShellType::Bash, + shell_path: PathBuf::from("/bin/bash"), + }; + let command = shell.build_command("python -c 'print(1)'", None); + assert_eq!(command[1], "-lc"); + } + + #[test] + fn explicit_login_flag_is_respected() { + let shell = Shell { + shell_type: ShellType::Bash, + shell_path: PathBuf::from("/bin/bash"), + }; + let login_command = shell.build_command("echo hello", Some(true)); + assert_eq!(login_command[1], "-lc"); + + let non_login_command = shell.build_command("echo hello", Some(false)); + assert_eq!(non_login_command[1], "-c"); + } + #[test] fn can_run_on_shell_test() { let cmd = "echo \"Works\""; diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index cd05d126bf..c63a5e0f99 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -49,8 +49,7 @@ impl ShellCommandHandler { turn_context: &TurnContext, ) -> ExecParams { let shell = session.user_shell(); - let use_login_shell = true; - let command = shell.derive_exec_args(¶ms.command, use_login_shell); + let command = shell.build_command(¶ms.command, None); ExecParams { command, diff --git a/codex-rs/core/src/tools/handlers/unified_exec.rs b/codex-rs/core/src/tools/handlers/unified_exec.rs index 4c943c6285..c2a3c37a30 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec.rs @@ -34,8 +34,8 @@ struct ExecCommandArgs { workdir: Option, #[serde(default)] shell: Option, - #[serde(default = "default_login")] - login: bool, + #[serde(default)] + login: Option, #[serde(default = "default_exec_yield_time_ms")] yield_time_ms: u64, #[serde(default)] @@ -66,10 +66,6 @@ fn default_write_stdin_yield_time_ms() -> u64 { 250 } -fn default_login() -> bool { - true -} - #[async_trait] impl ToolHandler for UnifiedExecHandler { fn kind(&self) -> ToolKind { @@ -260,7 +256,7 @@ fn get_command(args: &ExecCommandArgs) -> Vec { default_user_shell() }; - shell.derive_exec_args(&args.cmd, args.login) + shell.build_command(&args.cmd, args.login) } fn format_response(response: &UnifiedExecResponse) -> String { @@ -304,6 +300,7 @@ mod tests { serde_json::from_str(json).expect("deserialize ExecCommandArgs"); assert!(args.shell.is_none()); + assert!(args.login.is_none()); let command = get_command(&args); @@ -325,6 +322,42 @@ mod tests { assert_eq!(command[2], "echo hello"); } + #[test] + fn test_get_command_defaults_to_non_login_shell_for_allowlisted_commands() { + let json = r#"{"cmd": "echo hello", "shell": "/bin/bash"}"#; + + let args: ExecCommandArgs = + serde_json::from_str(json).expect("deserialize ExecCommandArgs"); + + let command = get_command(&args); + + assert_eq!(command[1], "-c"); + } + + #[test] + fn test_get_command_keeps_login_shell_for_non_allowlisted_commands() { + let json = r#"{"cmd": "python -c 'print(1)'", "shell": "/bin/bash"}"#; + + let args: ExecCommandArgs = + serde_json::from_str(json).expect("deserialize ExecCommandArgs"); + + let command = get_command(&args); + + assert_eq!(command[1], "-lc"); + } + + #[test] + fn test_get_command_respects_explicit_login_flag() { + let json = r#"{"cmd": "echo hello", "shell": "/bin/bash", "login": true}"#; + + let args: ExecCommandArgs = + serde_json::from_str(json).expect("deserialize ExecCommandArgs"); + + let command = get_command(&args); + + assert_eq!(command[1], "-lc"); + } + #[test] fn test_get_command_respects_explicit_powershell_shell() { let json = r#"{"cmd": "echo hello", "shell": "powershell"}"#; diff --git a/codex-rs/core/tests/common/lib.rs b/codex-rs/core/tests/common/lib.rs index e7b1e71efa..8bb6ec89d8 100644 --- a/codex-rs/core/tests/common/lib.rs +++ b/codex-rs/core/tests/common/lib.rs @@ -173,7 +173,7 @@ pub fn sandbox_network_env_var() -> &'static str { } pub fn format_with_current_shell(command: &str) -> Vec { - codex_core::shell::default_user_shell().derive_exec_args(command, true) + codex_core::shell::default_user_shell().build_command(command, None) } pub fn format_with_current_shell_display(command: &str) -> String { diff --git a/codex-rs/core/tests/suite/unified_exec.rs b/codex-rs/core/tests/suite/unified_exec.rs index dc7bdb6b10..009947493d 100644 --- a/codex-rs/core/tests/suite/unified_exec.rs +++ b/codex-rs/core/tests/suite/unified_exec.rs @@ -337,7 +337,7 @@ async fn unified_exec_emits_exec_command_begin_event() -> Result<()> { }) .await; - assert_command(&begin_event.command, "-lc", "/bin/echo hello unified exec"); + assert_command(&begin_event.command, "-c", "/bin/echo hello unified exec"); assert_eq!(begin_event.cwd, cwd.path());