From 25cf5d7c60f9d28005b9139c2bf3da5d573e95ab Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Wed, 15 Apr 2026 14:04:07 -0700 Subject: [PATCH 1/2] fix: tighten shell wrapper detection Use exact matches for known shell wrappers so command display and approval keys do not treat arbitrary shell-like paths as trusted wrappers. Co-authored-by: Codex --- .../src/command_canonicalization_tests.rs | 11 +++ codex-rs/core/src/shell.rs | 9 ++ codex-rs/core/src/shell_detect.rs | 44 ++++++---- .../src/tools/handlers/unified_exec_tests.rs | 21 +++++ codex-rs/shell-command/src/bash.rs | 12 +++ codex-rs/shell-command/src/parse_command.rs | 2 +- codex-rs/shell-command/src/powershell.rs | 12 ++- codex-rs/shell-command/src/shell_detect.rs | 82 +++++++++++++++---- codex-rs/tui/src/exec_command.rs | 4 + 9 files changed, 161 insertions(+), 36 deletions(-) diff --git a/codex-rs/core/src/command_canonicalization_tests.rs b/codex-rs/core/src/command_canonicalization_tests.rs index c278dedafcc..dd545541d21 100644 --- a/codex-rs/core/src/command_canonicalization_tests.rs +++ b/codex-rs/core/src/command_canonicalization_tests.rs @@ -86,3 +86,14 @@ fn preserves_non_shell_commands() { let command = vec!["cargo".to_string(), "fmt".to_string()]; assert_eq!(canonicalize_command_for_approval(&command), command); } + +#[test] +fn preserves_shell_like_paths_in_approval_keys() { + let command = vec![ + ".poc/bash".to_string(), + "-lc".to_string(), + "cargo test -p codex-core".to_string(), + ]; + + assert_eq!(canonicalize_command_for_approval(&command), command); +} diff --git a/codex-rs/core/src/shell.rs b/codex-rs/core/src/shell.rs index 48c760d6677..501d693e1a2 100644 --- a/codex-rs/core/src/shell.rs +++ b/codex-rs/core/src/shell.rs @@ -367,6 +367,9 @@ mod detect_shell_type_tests { detect_shell_type(&PathBuf::from("/bin/bash")), Some(ShellType::Bash) ); + assert_eq!(detect_shell_type(&PathBuf::from(".poc/bash")), None); + assert_eq!(detect_shell_type(&PathBuf::from("/tmp/bash")), None); + assert_eq!(detect_shell_type(&PathBuf::from("/tmp/bash.evil")), None); assert_eq!( detect_shell_type(&PathBuf::from("powershell.exe")), Some(ShellType::PowerShell) @@ -401,6 +404,12 @@ mod detect_shell_type_tests { Some(ShellType::Cmd) ); } + + #[test] + fn model_provided_shell_does_not_accept_repo_local_shell_names() { + let shell = get_shell_by_model_provided_path(&PathBuf::from(".poc/bash")); + assert_ne!(shell.shell_path, PathBuf::from(".poc/bash")); + } } #[cfg(test)] diff --git a/codex-rs/core/src/shell_detect.rs b/codex-rs/core/src/shell_detect.rs index 3595ab3469b..fda8d8cf217 100644 --- a/codex-rs/core/src/shell_detect.rs +++ b/codex-rs/core/src/shell_detect.rs @@ -1,24 +1,34 @@ use crate::shell::ShellType; -use std::path::Path; use std::path::PathBuf; pub(crate) fn detect_shell_type(shell_path: &PathBuf) -> Option { - match shell_path.as_os_str().to_str() { - Some("zsh") => Some(ShellType::Zsh), - Some("sh") => Some(ShellType::Sh), - Some("cmd") => Some(ShellType::Cmd), - Some("bash") => Some(ShellType::Bash), - Some("pwsh") => Some(ShellType::PowerShell), - Some("powershell") => Some(ShellType::PowerShell), - _ => { - let shell_name = shell_path.file_stem(); - if let Some(shell_name) = shell_name { - let shell_name_path = Path::new(shell_name); - if shell_name_path != Path::new(shell_path) { - return detect_shell_type(&shell_name_path.to_path_buf()); - } - } - None + let shell_text = shell_path.as_os_str().to_str()?; + // Keep this exact: repo-local files named like shells must not inherit + // shell-wrapper trust in approval or display decisions. + match shell_text { + "zsh" | "/bin/zsh" | "/usr/bin/zsh" | "/usr/local/bin/zsh" | "/opt/homebrew/bin/zsh" => { + Some(ShellType::Zsh) } + "sh" | "/bin/sh" | "/usr/bin/sh" => Some(ShellType::Sh), + "bash" + | "/bin/bash" + | "/usr/bin/bash" + | "/usr/local/bin/bash" + | "/opt/homebrew/bin/bash" => Some(ShellType::Bash), + "pwsh" + | "powershell" + | "pwsh.exe" + | "powershell.exe" + | "/usr/local/bin/pwsh" + | "/usr/bin/pwsh" + | "/bin/pwsh" + | "/opt/homebrew/bin/pwsh" => Some(ShellType::PowerShell), + "cmd" | "cmd.exe" => Some(ShellType::Cmd), + _ => match shell_text.replace('\\', "/").to_ascii_lowercase().as_str() { + "c:/windows/system32/cmd.exe" => Some(ShellType::Cmd), + "c:/windows/system32/windowspowershell/v1.0/powershell.exe" + | "c:/program files/powershell/7/pwsh.exe" => Some(ShellType::PowerShell), + _ => None, + }, } } diff --git a/codex-rs/core/src/tools/handlers/unified_exec_tests.rs b/codex-rs/core/src/tools/handlers/unified_exec_tests.rs index b6418145166..6437bcd1a04 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec_tests.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec_tests.rs @@ -68,6 +68,27 @@ fn test_get_command_respects_explicit_bash_shell() -> anyhow::Result<()> { Ok(()) } +#[test] +fn test_get_command_does_not_execute_shell_like_repo_path() -> anyhow::Result<()> { + let json = r#"{"cmd": "echo hello", "shell": ".poc/bash"}"#; + + let args: ExecCommandArgs = parse_arguments(json)?; + + assert_eq!(args.shell.as_deref(), Some(".poc/bash")); + + let command = get_command( + &args, + Arc::new(default_user_shell()), + &UnifiedExecShellMode::Direct, + /*allow_login_shell*/ true, + ) + .map_err(anyhow::Error::msg)?; + + assert_ne!(command.first(), Some(&".poc/bash".to_string())); + assert_eq!(command.last(), Some(&"echo hello".to_string())); + Ok(()) +} + #[test] fn test_get_command_respects_explicit_powershell_shell() -> anyhow::Result<()> { let json = r#"{"cmd": "echo hello", "shell": "powershell"}"#; diff --git a/codex-rs/shell-command/src/bash.rs b/codex-rs/shell-command/src/bash.rs index 2fe299108c2..9797a68a569 100644 --- a/codex-rs/shell-command/src/bash.rs +++ b/codex-rs/shell-command/src/bash.rs @@ -453,6 +453,18 @@ mod tests { assert_eq!(parsed, vec![vec!["ls".to_string()]]); } + #[test] + fn extract_bash_command_rejects_shell_like_attacker_paths() { + for shell in [".poc/bash", "/tmp/bash", "/tmp/bash.evil"] { + let command = vec![ + shell.to_string(), + "-lc".to_string(), + "echo INNOCENT_COMMAND".to_string(), + ]; + assert_eq!(extract_bash_command(&command), None); + } + } + #[test] fn accepts_concatenated_flag_and_value() { // Test case: -g"*.py" (flag directly concatenated with quoted value) diff --git a/codex-rs/shell-command/src/parse_command.rs b/codex-rs/shell-command/src/parse_command.rs index 1c8ac994858..0099f0e6b3b 100644 --- a/codex-rs/shell-command/src/parse_command.rs +++ b/codex-rs/shell-command/src/parse_command.rs @@ -1260,7 +1260,7 @@ mod tests { let command = if cfg!(windows) { "C:\\windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe" } else { - "/usr/local/bin/powershell.exe" + "/usr/local/bin/pwsh" }; assert_parsed( diff --git a/codex-rs/shell-command/src/powershell.rs b/codex-rs/shell-command/src/powershell.rs index 0b118f72794..5c9010885d6 100644 --- a/codex-rs/shell-command/src/powershell.rs +++ b/codex-rs/shell-command/src/powershell.rs @@ -168,13 +168,23 @@ mod tests { let command = if cfg!(windows) { "C:\\windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe".to_string() } else { - "/usr/local/bin/powershell.exe".to_string() + "/usr/local/bin/pwsh".to_string() }; let cmd = vec![command, "-Command".to_string(), "Write-Host hi".to_string()]; let (_shell, script) = extract_powershell_command(&cmd).expect("extract"); assert_eq!(script, "Write-Host hi"); } + #[test] + fn rejects_shell_like_attacker_paths() { + let cmd = vec![ + r"C:\tmp\powershell.exe".to_string(), + "-Command".to_string(), + "Write-Host hi".to_string(), + ]; + assert_eq!(extract_powershell_command(&cmd), None); + } + #[test] fn extracts_with_noprofile_and_alias() { let cmd = vec![ diff --git a/codex-rs/shell-command/src/shell_detect.rs b/codex-rs/shell-command/src/shell_detect.rs index 34322a06d00..5dc8e21762f 100644 --- a/codex-rs/shell-command/src/shell_detect.rs +++ b/codex-rs/shell-command/src/shell_detect.rs @@ -1,4 +1,3 @@ -use std::path::Path; use std::path::PathBuf; #[derive(Debug, PartialEq, Eq, Clone, Copy)] @@ -11,22 +10,71 @@ pub(crate) enum ShellType { } pub(crate) fn detect_shell_type(shell_path: &PathBuf) -> Option { - match shell_path.as_os_str().to_str() { - Some("zsh") => Some(ShellType::Zsh), - Some("sh") => Some(ShellType::Sh), - Some("cmd") => Some(ShellType::Cmd), - Some("bash") => Some(ShellType::Bash), - Some("pwsh") => Some(ShellType::PowerShell), - Some("powershell") => Some(ShellType::PowerShell), - _ => { - let shell_name = shell_path.file_stem(); - if let Some(shell_name) = shell_name { - let shell_name_path = Path::new(shell_name); - if shell_name_path != Path::new(shell_path) { - return detect_shell_type(&shell_name_path.to_path_buf()); - } - } - None + let shell_text = shell_path.as_os_str().to_str()?; + // Keep this exact: repo-local files named like shells must not inherit + // shell-wrapper trust in approval or display decisions. + match shell_text { + "zsh" | "/bin/zsh" | "/usr/bin/zsh" | "/usr/local/bin/zsh" | "/opt/homebrew/bin/zsh" => { + Some(ShellType::Zsh) } + "sh" | "/bin/sh" | "/usr/bin/sh" => Some(ShellType::Sh), + "bash" + | "/bin/bash" + | "/usr/bin/bash" + | "/usr/local/bin/bash" + | "/opt/homebrew/bin/bash" => Some(ShellType::Bash), + "pwsh" + | "powershell" + | "pwsh.exe" + | "powershell.exe" + | "/usr/local/bin/pwsh" + | "/usr/bin/pwsh" + | "/bin/pwsh" + | "/opt/homebrew/bin/pwsh" => Some(ShellType::PowerShell), + "cmd" | "cmd.exe" => Some(ShellType::Cmd), + _ => match shell_text.replace('\\', "/").to_ascii_lowercase().as_str() { + "c:/windows/system32/cmd.exe" => Some(ShellType::Cmd), + "c:/windows/system32/windowspowershell/v1.0/powershell.exe" + | "c:/program files/powershell/7/pwsh.exe" => Some(ShellType::PowerShell), + _ => None, + }, + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn detects_exact_shell_names_and_system_paths() { + assert_eq!( + detect_shell_type(&PathBuf::from("bash")), + Some(ShellType::Bash) + ); + assert_eq!( + detect_shell_type(&PathBuf::from("/bin/bash")), + Some(ShellType::Bash) + ); + assert_eq!( + detect_shell_type(&PathBuf::from("powershell.exe")), + Some(ShellType::PowerShell) + ); + assert_eq!( + detect_shell_type(&PathBuf::from( + r"C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe" + )), + Some(ShellType::PowerShell) + ); + } + + #[test] + fn rejects_shell_like_attacker_controlled_paths() { + assert_eq!(detect_shell_type(&PathBuf::from(".poc/bash")), None); + assert_eq!(detect_shell_type(&PathBuf::from("/tmp/bash")), None); + assert_eq!(detect_shell_type(&PathBuf::from("/tmp/bash.evil")), None); + assert_eq!( + detect_shell_type(&PathBuf::from(r"C:\tmp\powershell.exe")), + None + ); } } diff --git a/codex-rs/tui/src/exec_command.rs b/codex-rs/tui/src/exec_command.rs index 0083e1df2bd..05dafa13ce6 100644 --- a/codex-rs/tui/src/exec_command.rs +++ b/codex-rs/tui/src/exec_command.rs @@ -82,6 +82,10 @@ mod tests { let args = vec!["/bin/bash".into(), "-lc".into(), "echo hello".into()]; let cmdline = strip_bash_lc_and_escape(&args); assert_eq!(cmdline, "echo hello"); + + let args = vec![".poc/bash".into(), "-lc".into(), "echo hello".into()]; + let cmdline = strip_bash_lc_and_escape(&args); + assert_eq!(cmdline, ".poc/bash -lc 'echo hello'"); } #[test] From 2bd0700b76bd37e15b35bd4a311b28a534004402 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Wed, 15 Apr 2026 14:09:45 -0700 Subject: [PATCH 2/2] refactor: share shell wrapper detection Expose the known-shell detector from codex-shell-command and map it into codex-core's runtime shell type. Co-authored-by: Codex --- codex-rs/core/src/lib.rs | 1 - codex-rs/core/src/shell.rs | 13 ++++++++- codex-rs/core/src/shell_detect.rs | 34 ---------------------- codex-rs/shell-command/src/lib.rs | 2 +- codex-rs/shell-command/src/shell_detect.rs | 5 ++-- 5 files changed, 16 insertions(+), 39 deletions(-) delete mode 100644 codex-rs/core/src/shell_detect.rs diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 8d463f3b519..6f2c68f1578 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -86,7 +86,6 @@ mod sandbox_tags; pub mod sandboxing; mod session_prefix; mod session_startup_prewarm; -mod shell_detect; pub mod skills; pub(crate) use skills::SkillError; pub(crate) use skills::SkillInjections; diff --git a/codex-rs/core/src/shell.rs b/codex-rs/core/src/shell.rs index 501d693e1a2..57548711470 100644 --- a/codex-rs/core/src/shell.rs +++ b/codex-rs/core/src/shell.rs @@ -1,5 +1,6 @@ -use crate::shell_detect::detect_shell_type; use crate::shell_snapshot::ShellSnapshot; +use codex_shell_command::shell_detect::ShellType as DetectedShellType; +use codex_shell_command::shell_detect::detect_shell_type as detect_known_shell_type; use serde::Deserialize; use serde::Serialize; use std::path::PathBuf; @@ -88,6 +89,16 @@ impl PartialEq for Shell { impl Eq for Shell {} +fn detect_shell_type(shell_path: &PathBuf) -> Option { + match detect_known_shell_type(shell_path)? { + DetectedShellType::Zsh => Some(ShellType::Zsh), + DetectedShellType::Bash => Some(ShellType::Bash), + DetectedShellType::PowerShell => Some(ShellType::PowerShell), + DetectedShellType::Sh => Some(ShellType::Sh), + DetectedShellType::Cmd => Some(ShellType::Cmd), + } +} + #[cfg(unix)] fn get_user_shell_path() -> Option { let uid = unsafe { libc::getuid() }; diff --git a/codex-rs/core/src/shell_detect.rs b/codex-rs/core/src/shell_detect.rs deleted file mode 100644 index fda8d8cf217..00000000000 --- a/codex-rs/core/src/shell_detect.rs +++ /dev/null @@ -1,34 +0,0 @@ -use crate::shell::ShellType; -use std::path::PathBuf; - -pub(crate) fn detect_shell_type(shell_path: &PathBuf) -> Option { - let shell_text = shell_path.as_os_str().to_str()?; - // Keep this exact: repo-local files named like shells must not inherit - // shell-wrapper trust in approval or display decisions. - match shell_text { - "zsh" | "/bin/zsh" | "/usr/bin/zsh" | "/usr/local/bin/zsh" | "/opt/homebrew/bin/zsh" => { - Some(ShellType::Zsh) - } - "sh" | "/bin/sh" | "/usr/bin/sh" => Some(ShellType::Sh), - "bash" - | "/bin/bash" - | "/usr/bin/bash" - | "/usr/local/bin/bash" - | "/opt/homebrew/bin/bash" => Some(ShellType::Bash), - "pwsh" - | "powershell" - | "pwsh.exe" - | "powershell.exe" - | "/usr/local/bin/pwsh" - | "/usr/bin/pwsh" - | "/bin/pwsh" - | "/opt/homebrew/bin/pwsh" => Some(ShellType::PowerShell), - "cmd" | "cmd.exe" => Some(ShellType::Cmd), - _ => match shell_text.replace('\\', "/").to_ascii_lowercase().as_str() { - "c:/windows/system32/cmd.exe" => Some(ShellType::Cmd), - "c:/windows/system32/windowspowershell/v1.0/powershell.exe" - | "c:/program files/powershell/7/pwsh.exe" => Some(ShellType::PowerShell), - _ => None, - }, - } -} diff --git a/codex-rs/shell-command/src/lib.rs b/codex-rs/shell-command/src/lib.rs index 1d9e302a4e7..947de5e645a 100644 --- a/codex-rs/shell-command/src/lib.rs +++ b/codex-rs/shell-command/src/lib.rs @@ -1,6 +1,6 @@ //! Command parsing and safety utilities shared across Codex crates. -mod shell_detect; +pub mod shell_detect; pub mod bash; pub(crate) mod command_safety; diff --git a/codex-rs/shell-command/src/shell_detect.rs b/codex-rs/shell-command/src/shell_detect.rs index 5dc8e21762f..902cabce00f 100644 --- a/codex-rs/shell-command/src/shell_detect.rs +++ b/codex-rs/shell-command/src/shell_detect.rs @@ -1,7 +1,8 @@ use std::path::PathBuf; +/// Shell executable families that Codex treats as known command wrappers. #[derive(Debug, PartialEq, Eq, Clone, Copy)] -pub(crate) enum ShellType { +pub enum ShellType { Zsh, Bash, PowerShell, @@ -9,7 +10,7 @@ pub(crate) enum ShellType { Cmd, } -pub(crate) fn detect_shell_type(shell_path: &PathBuf) -> Option { +pub fn detect_shell_type(shell_path: &PathBuf) -> Option { let shell_text = shell_path.as_os_str().to_str()?; // Keep this exact: repo-local files named like shells must not inherit // shell-wrapper trust in approval or display decisions.