Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions codex-rs/core/src/command_canonicalization_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
1 change: 0 additions & 1 deletion codex-rs/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
22 changes: 21 additions & 1 deletion codex-rs/core/src/shell.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -88,6 +89,16 @@ impl PartialEq for Shell {

impl Eq for Shell {}

fn detect_shell_type(shell_path: &PathBuf) -> Option<ShellType> {
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<PathBuf> {
let uid = unsafe { libc::getuid() };
Expand Down Expand Up @@ -367,6 +378,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)
Expand Down Expand Up @@ -401,6 +415,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)]
Expand Down
24 changes: 0 additions & 24 deletions codex-rs/core/src/shell_detect.rs

This file was deleted.

21 changes: 21 additions & 0 deletions codex-rs/core/src/tools/handlers/unified_exec_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"}"#;
Expand Down
12 changes: 12 additions & 0 deletions codex-rs/shell-command/src/bash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion codex-rs/shell-command/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
2 changes: 1 addition & 1 deletion codex-rs/shell-command/src/parse_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
12 changes: 11 additions & 1 deletion codex-rs/shell-command/src/powershell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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![
Expand Down
87 changes: 68 additions & 19 deletions codex-rs/shell-command/src/shell_detect.rs
Original file line number Diff line number Diff line change
@@ -1,32 +1,81 @@
use std::path::Path;
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,
Sh,
Cmd,
}

pub(crate) fn detect_shell_type(shell_path: &PathBuf) -> Option<ShellType> {
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
pub fn detect_shell_type(shell_path: &PathBuf) -> Option<ShellType> {
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
);
}
}
4 changes: 4 additions & 0 deletions codex-rs/tui/src/exec_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
Loading