agent/codex
1
0
Fork 0
mirror of https://github.com/pchuan98/codex.git synced 2026-07-01 00:31:56 +08:00
Code Issues Packages Projects Releases Wiki Activity

Revert "Overhaul shell detection and centralize command generation for unified exec" (#6606)

Browse Source 
Reverts openai/codex#6577
This commit is contained in:
pakrym-oai
2025-11-13 08:43:00 -08:00
committed by GitHub
Unverified
parent d28e912214
commit e6995174c1
11 changed files with 411 additions and 283 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/bash.rs
+13 -11
View File
@@ -1,13 +1,8 @@
use std::path::PathBuf;
use tree_sitter::Node;
use tree_sitter::Parser;
use tree_sitter::Tree;
use tree_sitter_bash::LANGUAGE as BASH;
use crate::shell::ShellType;
use crate::shell::detect_shell_type;
/// Parse the provided bash source using tree-sitter-bash, returning a Tree on
/// success or None if parsing failed.
pub fn try_parse_shell(shell_lc_arg: &str) -> Option<Tree> {
@@ -93,16 +88,23 @@ pub fn try_parse_word_only_commands_sequence(tree: &Tree, src: &str) -> Option<V
Some(commands)
}
pub fn is_well_known_sh_shell(shell: &str) -> bool {
if shell == "bash" || shell == "zsh" {
return true;
}
let shell_name = std::path::Path::new(shell)
.file_name()
.and_then(|s| s.to_str())
.unwrap_or(shell);
matches!(shell_name, "bash" | "zsh")
}
pub fn extract_bash_command(command: &[String]) -> Option<(&str, &str)> {
let [shell, flag, script] = command else {
return None;
};
if !matches!(flag.as_str(), "-lc" | "-c")
|| !matches!(
detect_shell_type(&PathBuf::from(shell)),
Some(ShellType::Zsh) | Some(ShellType::Bash)
)
{
if !matches!(flag.as_str(), "-lc" | "-c") || !is_well_known_sh_shell(shell) {
return None;
}
Some((shell, script))
codex-rs/core/src/command_safety/is_safe_command.rs
+6 -4
View File
@@ -1,5 +1,4 @@
use crate::bash::parse_shell_lc_plain_commands;
use crate::command_safety::windows_safe_commands::is_safe_command_windows;
pub fn is_known_safe_command(command: &[String]) -> bool {
let command: Vec<String> = command
@@ -12,9 +11,12 @@ pub fn is_known_safe_command(command: &[String]) -> bool {
}
})
.collect();
if is_safe_command_windows(&command) {
return true;
#[cfg(target_os = "windows")]
{
use super::windows_safe_commands::is_safe_command_windows;
if is_safe_command_windows(&command) {
return true;
}
}
if is_safe_to_call_with_exec(&command) {
codex-rs/core/src/command_safety/mod.rs
+1
View File
@@ -1,3 +1,4 @@
pub mod is_dangerous_command;
pub mod is_safe_command;
#[cfg(target_os = "windows")]
pub mod windows_safe_commands;
codex-rs/core/src/environment_context.rs
+2
View File
@@ -329,6 +329,7 @@ mod tests {
Some(workspace_write_policy(vec!["/repo"], false)),
Some(Shell::Bash(BashShell {
shell_path: "/bin/bash".into(),
bashrc_path: "/home/user/.bashrc".into(),
})),
);
let context2 = EnvironmentContext::new(
@@ -337,6 +338,7 @@ mod tests {
Some(workspace_write_policy(vec!["/repo"], false)),
Some(Shell::Zsh(ZshShell {
shell_path: "/bin/zsh".into(),
zshrc_path: "/home/user/.zshrc".into(),
})),
);
codex-rs/core/src/shell.rs
+337 -214
View File
@@ -2,26 +2,22 @@ use serde::Deserialize;
use serde::Serialize;
use std::path::PathBuf;
#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)]
pub enum ShellType {
Zsh,
Bash,
PowerShell,
}
#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)]
pub struct ZshShell {
pub(crate) shell_path: PathBuf,
pub(crate) shell_path: String,
pub(crate) zshrc_path: String,
}
#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)]
pub struct BashShell {
pub(crate) shell_path: PathBuf,
pub(crate) shell_path: String,
pub(crate) bashrc_path: String,
}
#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)]
pub struct PowerShellConfig {
pub(crate) shell_path: PathBuf, // Executable name or path, e.g. "pwsh" or "powershell.exe".
pub(crate) exe: String, // Executable name or path, e.g. "pwsh" or "powershell.exe".
pub(crate) bash_exe_fallback: Option<PathBuf>, // In case the model generates a bash command.
}
#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)]
@@ -40,10 +36,7 @@ impl Shell {
.file_name()
.map(|s| s.to_string_lossy().to_string())
}
Shell::PowerShell(ps) => ps
.shell_path
.file_stem()
.map(|s| s.to_string_lossy().to_string()),
Shell::PowerShell(ps) => Some(ps.exe.clone()),
Shell::Unknown => None,
}
}
@@ -54,17 +47,10 @@ impl Shell {
match self {
Shell::Zsh(ZshShell { shell_path, .. }) | Shell::Bash(BashShell { shell_path, .. }) => {
let arg = if use_login_shell { "-lc" } else { "-c" };
vec![
shell_path.to_string_lossy().to_string(),
arg.to_string(),
command.to_string(),
]
vec![shell_path.clone(), arg.to_string(), command.to_string()]
}
Shell::PowerShell(ps) => {
let mut args = vec![
ps.shell_path.to_string_lossy().to_string(),
"-NoLogo".to_string(),
];
let mut args = vec![ps.exe.clone(), "-NoLogo".to_string()];
if !use_login_shell {
args.push("-NoProfile".to_string());
}
@@ -79,7 +65,7 @@ impl Shell {
}
#[cfg(unix)]
fn get_user_shell_path() -> Option<PathBuf> {
fn detect_default_user_shell() -> Shell {
use libc::getpwuid;
use libc::getuid;
use std::ffi::CStr;
@@ -92,164 +78,75 @@ fn get_user_shell_path() -> Option<PathBuf> {
let shell_path = CStr::from_ptr((*pw).pw_shell)
.to_string_lossy()
.into_owned();
Some(PathBuf::from(shell_path))
} else {
None
}
}
}
let home_path = CStr::from_ptr((*pw).pw_dir).to_string_lossy().into_owned();
#[cfg(not(unix))]
fn get_user_shell_path() -> Option<PathBuf> {
None
}
if shell_path.ends_with("/zsh") {
return Shell::Zsh(ZshShell {
shell_path,
zshrc_path: format!("{home_path}/.zshrc"),
});
}
fn file_exists(path: &PathBuf) -> Option<PathBuf> {
if std::fs::metadata(path).is_ok_and(|metadata| metadata.is_file()) {
Some(PathBuf::from(path))
} else {
None
}
}
fn get_shell_path(
shell_type: ShellType,
binary_name: &str,
fallback_paths: Vec<&str>,
) -> Option<PathBuf> {
// Check if the shell we are trying to load is user's default shell
// if just use it
let default_shell_path = get_user_shell_path();
if let Some(default_shell_path) = default_shell_path
&& detect_shell_type(&default_shell_path) == Some(shell_type)
{
return Some(default_shell_path);
}
if let Ok(path) = which::which(binary_name) {
return Some(path);
}
for path in fallback_paths {
//check exists
if let Some(path) = file_exists(&PathBuf::from(path)) {
return Some(path);
}
}
None
}
fn get_zsh_shell() -> Option<ZshShell> {
let shell_path = get_shell_path(ShellType::Zsh, "zsh", vec!["/bin/zsh"]);
shell_path.map(|shell_path| ZshShell { shell_path })
}
fn get_bash_shell() -> Option<BashShell> {
let shell_path = get_shell_path(ShellType::Bash, "bash", vec!["/bin/bash"]);
shell_path.map(|shell_path| BashShell { shell_path })
}
fn get_powershell_shell() -> Option<PowerShellConfig> {
let shell_path = get_shell_path(ShellType::PowerShell, "pwsh", vec!["/usr/local/bin/pwsh"])
.or_else(|| get_shell_path(ShellType::PowerShell, "powershell", vec![]));
shell_path.map(|shell_path| PowerShellConfig { shell_path })
}
pub fn get_shell_by_model_provided_path(shell_path: &PathBuf) -> Shell {
detect_shell_type(shell_path)
.and_then(get_shell)
.unwrap_or(Shell::Unknown)
}
pub fn get_shell(shell_type: ShellType) -> Option<Shell> {
match shell_type {
ShellType::Zsh => get_zsh_shell().map(Shell::Zsh),
ShellType::Bash => get_bash_shell().map(Shell::Bash),
ShellType::PowerShell => get_powershell_shell().map(Shell::PowerShell),
}
}
pub fn detect_shell_type(shell_path: &PathBuf) -> Option<ShellType> {
match shell_path.as_os_str().to_str() {
Some("zsh") => Some(ShellType::Zsh),
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
&& shell_name != shell_path
{
detect_shell_type(&PathBuf::from(shell_name))
} else {
None
if shell_path.ends_with("/bash") {
return Shell::Bash(BashShell {
shell_path,
bashrc_path: format!("{home_path}/.bashrc"),
});
}
}
}
Shell::Unknown
}
#[cfg(unix)]
pub async fn default_user_shell() -> Shell {
if cfg!(windows) {
get_shell(ShellType::PowerShell).unwrap_or(Shell::Unknown)
detect_default_user_shell()
}
#[cfg(target_os = "windows")]
pub async fn default_user_shell() -> Shell {
use tokio::process::Command;
// Prefer PowerShell 7+ (`pwsh`) if available, otherwise fall back to Windows PowerShell.
let has_pwsh = Command::new("pwsh")
.arg("-NoLogo")
.arg("-NoProfile")
.arg("-Command")
.arg("$PSVersionTable.PSVersion.Major")
.output()
.await
.map(|o| o.status.success())
.unwrap_or(false);
let bash_exe = if Command::new("bash.exe")
.arg("--version")
.stdin(std::process::Stdio::null())
.output()
.await
.ok()
.map(|o| o.status.success())
.unwrap_or(false)
{
which::which("bash.exe").ok()
} else {
get_user_shell_path()
.and_then(|shell| detect_shell_type(&shell))
.and_then(get_shell)
.unwrap_or(Shell::Unknown)
None
};
if has_pwsh {
Shell::PowerShell(PowerShellConfig {
exe: "pwsh.exe".to_string(),
bash_exe_fallback: bash_exe,
})
} else {
Shell::PowerShell(PowerShellConfig {
exe: "powershell.exe".to_string(),
bash_exe_fallback: bash_exe,
})
}
}
#[cfg(test)]
mod detect_shell_type_tests {
use super::*;
#[test]
fn test_detect_shell_type() {
assert_eq!(
detect_shell_type(&PathBuf::from("zsh")),
Some(ShellType::Zsh)
);
assert_eq!(
detect_shell_type(&PathBuf::from("bash")),
Some(ShellType::Bash)
);
assert_eq!(
detect_shell_type(&PathBuf::from("pwsh")),
Some(ShellType::PowerShell)
);
assert_eq!(
detect_shell_type(&PathBuf::from("powershell")),
Some(ShellType::PowerShell)
);
assert_eq!(detect_shell_type(&PathBuf::from("fish")), None);
assert_eq!(detect_shell_type(&PathBuf::from("other")), None);
assert_eq!(
detect_shell_type(&PathBuf::from("/bin/zsh")),
Some(ShellType::Zsh)
);
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("pwsh.exe")),
Some(ShellType::PowerShell)
);
assert_eq!(
detect_shell_type(&PathBuf::from("/usr/local/bin/pwsh")),
Some(ShellType::PowerShell)
);
}
#[cfg(all(not(target_os = "windows"), not(unix)))]
pub async fn default_user_shell() -> Shell {
Shell::Unknown
}
#[cfg(test)]
@@ -259,30 +156,6 @@ mod tests {
use std::path::PathBuf;
use std::process::Command;
#[test]
#[cfg(target_os = "macos")]
fn detects_zsh() {
let zsh_shell = get_shell(ShellType::Zsh).unwrap();
let ZshShell { shell_path } = match zsh_shell {
Shell::Zsh(zsh_shell) => zsh_shell,
_ => panic!("expected zsh shell"),
};
assert_eq!(shell_path, PathBuf::from("/bin/zsh"));
}
#[test]
fn detects_bash() {
let bash_shell = get_shell(ShellType::Bash).unwrap();
let BashShell { shell_path } = match bash_shell {
Shell::Bash(bash_shell) => bash_shell,
_ => panic!("expected bash shell"),
};
assert_eq!(shell_path, PathBuf::from("/bin/bash"));
}
#[tokio::test]
async fn test_current_shell_detects_zsh() {
let shell = Command::new("sh")
@@ -291,42 +164,292 @@ mod tests {
.output()
.unwrap();
let home = std::env::var("HOME").unwrap();
let shell_path = String::from_utf8_lossy(&shell.stdout).trim().to_string();
if shell_path.ends_with("/zsh") {
assert_eq!(
default_user_shell().await,
Shell::Zsh(ZshShell {
shell_path: PathBuf::from(shell_path),
shell_path: shell_path.to_string(),
zshrc_path: format!("{home}/.zshrc",),
})
);
}
}
#[tokio::test]
async fn test_run_with_profile_bash_escaping_and_execution() {
let shell_path = "/bin/bash";
let cases = vec![
(
vec!["myecho"],
vec![shell_path, "-lc", "source BASHRC_PATH && (myecho)"],
Some("It works!\n"),
),
(
vec!["bash", "-lc", "echo 'single' \"double\""],
vec![
shell_path,
"-lc",
"source BASHRC_PATH && (echo 'single' \"double\")",
],
Some("single double\n"),
),
];
for (input, expected_cmd, expected_output) in cases {
use std::collections::HashMap;
use crate::exec::ExecParams;
use crate::exec::SandboxType;
use crate::exec::process_exec_tool_call;
use crate::protocol::SandboxPolicy;
let temp_home = tempfile::tempdir().unwrap();
let bashrc_path = temp_home.path().join(".bashrc");
std::fs::write(
&bashrc_path,
r#"
set -x
function myecho {
echo 'It works!'
}
"#,
)
.unwrap();
let command = expected_cmd
.iter()
.map(|s| s.replace("BASHRC_PATH", bashrc_path.to_str().unwrap()))
.collect::<Vec<_>>();
let output = process_exec_tool_call(
ExecParams {
command: command.clone(),
cwd: PathBuf::from(temp_home.path()),
timeout_ms: None,
env: HashMap::from([(
"HOME".to_string(),
temp_home.path().to_str().unwrap().to_string(),
)]),
with_escalated_permissions: None,
justification: None,
arg0: None,
},
SandboxType::None,
&SandboxPolicy::DangerFullAccess,
temp_home.path(),
&None,
None,
)
.await
.unwrap();
assert_eq!(output.exit_code, 0, "input: {input:?} output: {output:?}");
if let Some(expected) = expected_output {
assert_eq!(
output.stdout.text, expected,
"input: {input:?} output: {output:?}"
);
}
}
}
}
#[cfg(test)]
#[cfg(windows)]
mod tests {
use super::*;
#[cfg(target_os = "macos")]
mod macos_tests {
use std::path::PathBuf;
#[tokio::test]
async fn detects_powershell_as_default() {
let powershell_shell = default_user_shell().await;
let PowerShellConfig { shell_path } = match powershell_shell {
Shell::PowerShell(powershell_shell) => powershell_shell,
_ => panic!("expected powershell shell"),
};
async fn test_run_with_profile_escaping_and_execution() {
let shell_path = "/bin/zsh";
assert!(shell_path.ends_with("pwsh.exe") || shell_path.ends_with("powershell.exe"));
}
let cases = vec![
(
vec!["myecho"],
vec![shell_path, "-lc", "source ZSHRC_PATH && (myecho)"],
Some("It works!\n"),
),
(
vec!["myecho"],
vec![shell_path, "-lc", "source ZSHRC_PATH && (myecho)"],
Some("It works!\n"),
),
(
vec!["bash", "-c", "echo 'single' \"double\""],
vec![
shell_path,
"-lc",
"source ZSHRC_PATH && (bash -c \"echo 'single' \\\"double\\\"\")",
],
Some("single double\n"),
),
(
vec!["bash", "-lc", "echo 'single' \"double\""],
vec![
shell_path,
"-lc",
"source ZSHRC_PATH && (echo 'single' \"double\")",
],
Some("single double\n"),
),
];
for (input, expected_cmd, expected_output) in cases {
use std::collections::HashMap;
#[test]
fn finds_poweshell() {
let powershell_shell = get_shell(ShellType::PowerShell).unwrap();
let PowerShellConfig { shell_path } = match powershell_shell {
Shell::PowerShell(powershell_shell) => powershell_shell,
_ => panic!("expected powershell shell"),
};
use crate::exec::ExecParams;
use crate::exec::SandboxType;
use crate::exec::process_exec_tool_call;
use crate::protocol::SandboxPolicy;
assert!(shell_path.ends_with("pwsh.exe") || shell_path.ends_with("powershell.exe"));
let temp_home = tempfile::tempdir().unwrap();
let zshrc_path = temp_home.path().join(".zshrc");
std::fs::write(
&zshrc_path,
r#"
set -x
function myecho {
echo 'It works!'
}
"#,
)
.unwrap();
let command = expected_cmd
.iter()
.map(|s| s.replace("ZSHRC_PATH", zshrc_path.to_str().unwrap()))
.collect::<Vec<_>>();
let output = process_exec_tool_call(
ExecParams {
command: command.clone(),
cwd: PathBuf::from(temp_home.path()),
timeout_ms: None,
env: HashMap::from([(
"HOME".to_string(),
temp_home.path().to_str().unwrap().to_string(),
)]),
with_escalated_permissions: None,
justification: None,
arg0: None,
},
SandboxType::None,
&SandboxPolicy::DangerFullAccess,
temp_home.path(),
&None,
None,
)
.await
.unwrap();
assert_eq!(output.exit_code, 0, "input: {input:?} output: {output:?}");
if let Some(expected) = expected_output {
assert_eq!(
output.stdout.text, expected,
"input: {input:?} output: {output:?}"
);
}
}
}
}
#[cfg(test)]
#[cfg(target_os = "windows")]
mod tests_windows {
use super::*;
#[test]
fn test_format_default_shell_invocation_powershell() {
use std::path::PathBuf;
let cases = vec![
(
PowerShellConfig {
exe: "pwsh.exe".to_string(),
bash_exe_fallback: None,
},
vec!["bash", "-lc", "echo hello"],
vec!["pwsh.exe", "-NoProfile", "-Command", "echo hello"],
),
(
PowerShellConfig {
exe: "powershell.exe".to_string(),
bash_exe_fallback: None,
},
vec!["bash", "-lc", "echo hello"],
vec!["powershell.exe", "-NoProfile", "-Command", "echo hello"],
),
(
PowerShellConfig {
exe: "pwsh.exe".to_string(),
bash_exe_fallback: Some(PathBuf::from("bash.exe")),
},
vec!["bash", "-lc", "echo hello"],
vec!["bash.exe", "-lc", "echo hello"],
),
(
PowerShellConfig {
exe: "pwsh.exe".to_string(),
bash_exe_fallback: Some(PathBuf::from("bash.exe")),
},
vec![
"bash",
"-lc",
"apply_patch <<'EOF'\n*** Begin Patch\n*** Update File: destination_file.txt\n-original content\n+modified content\n*** End Patch\nEOF",
],
vec![
"bash.exe",
"-lc",
"apply_patch <<'EOF'\n*** Begin Patch\n*** Update File: destination_file.txt\n-original content\n+modified content\n*** End Patch\nEOF",
],
),
(
PowerShellConfig {
exe: "pwsh.exe".to_string(),
bash_exe_fallback: Some(PathBuf::from("bash.exe")),
},
vec!["echo", "hello"],
vec!["pwsh.exe", "-NoProfile", "-Command", "echo hello"],
),
(
PowerShellConfig {
exe: "pwsh.exe".to_string(),
bash_exe_fallback: Some(PathBuf::from("bash.exe")),
},
vec!["pwsh.exe", "-NoProfile", "-Command", "echo hello"],
vec!["pwsh.exe", "-NoProfile", "-Command", "echo hello"],
),
(
PowerShellConfig {
exe: "powershell.exe".to_string(),
bash_exe_fallback: Some(PathBuf::from("bash.exe")),
},
vec![
"codex-mcp-server.exe",
"--codex-run-as-apply-patch",
"*** Begin Patch\n*** Update File: C:\\Users\\person\\destination_file.txt\n-original content\n+modified content\n*** End Patch",
],
vec![
"codex-mcp-server.exe",
"--codex-run-as-apply-patch",
"*** Begin Patch\n*** Update File: C:\\Users\\person\\destination_file.txt\n-original content\n+modified content\n*** End Patch",
],
),
];
for (config, input, expected_cmd) in cases {
let command = expected_cmd
.iter()
.map(|s| (*s).to_string())
.collect::<Vec<_>>();
// These tests assert the final command for each scenario now that the helper
// has been removed. The inputs remain to document the original coverage.
let expected = expected_cmd
.iter()
.map(|s| (*s).to_string())
.collect::<Vec<_>>();
assert_eq!(command, expected, "input: {input:?} config: {config:?}");
}
}
}
codex-rs/core/src/tools/events.rs
+4 -4
View File
@@ -87,7 +87,7 @@ pub(crate) enum ToolEmitter {
auto_approved: bool,
},
UnifiedExec {
command: Vec<String>,
command: String,
cwd: PathBuf,
// True for `exec_command` and false for `write_stdin`.
#[allow(dead_code)]
@@ -111,9 +111,9 @@ impl ToolEmitter {
}
}
pub fn unified_exec(command: &[String], cwd: PathBuf, is_startup_command: bool) -> Self {
pub fn unified_exec(command: String, cwd: PathBuf, is_startup_command: bool) -> Self {
Self::UnifiedExec {
command: command.to_vec(),
command,
cwd,
is_startup_command,
}
@@ -218,7 +218,7 @@ impl ToolEmitter {
emit_patch_end(ctx, String::new(), (*message).to_string(), false).await;
}
(Self::UnifiedExec { command, cwd, .. }, ToolEventStage::Begin) => {
emit_exec_command_begin(ctx, command, cwd.as_path(), false).await;
emit_exec_command_begin(ctx, &[command.to_string()], cwd.as_path(), false).await;
}
(Self::UnifiedExec { .. }, ToolEventStage::Success(output)) => {
emit_exec_end(
codex-rs/core/src/tools/handlers/shell.rs
+14 -9
View File
@@ -324,11 +324,8 @@ impl ShellHandler {
#[cfg(test)]
mod tests {
use std::path::PathBuf;
use crate::is_safe_command::is_known_safe_command;
use crate::shell::BashShell;
use crate::shell::PowerShellConfig;
use crate::shell::Shell;
use crate::shell::ZshShell;
@@ -338,19 +335,27 @@ mod tests {
#[test]
fn commands_generated_by_shell_command_handler_can_be_matched_by_is_known_safe_command() {
let bash_shell = Shell::Bash(BashShell {
shell_path: PathBuf::from("/bin/bash"),
shell_path: "/bin/bash".to_string(),
bashrc_path: "/home/user/.bashrc".to_string(),
});
assert_safe(&bash_shell, "ls -la");
let zsh_shell = Shell::Zsh(ZshShell {
shell_path: PathBuf::from("/bin/zsh"),
shell_path: "/bin/zsh".to_string(),
zshrc_path: "/home/user/.zshrc".to_string(),
});
assert_safe(&zsh_shell, "ls -la");
let powershell = Shell::PowerShell(PowerShellConfig {
shell_path: PathBuf::from("pwsh.exe"),
});
assert_safe(&powershell, "ls -Name");
#[cfg(target_os = "windows")]
{
use crate::shell::PowerShellConfig;
let powershell = Shell::PowerShell(PowerShellConfig {
exe: "pwsh.exe".to_string(),
bash_exe_fallback: None,
});
assert_safe(&powershell, "ls -Name");
}
}
fn assert_safe(shell: &Shell, command: &str) {
codex-rs/core/src/tools/handlers/unified_exec.rs
+8 -14
View File
@@ -5,7 +5,6 @@ use crate::is_safe_command::is_known_safe_command;
use crate::protocol::EventMsg;
use crate::protocol::ExecCommandOutputDeltaEvent;
use crate::protocol::ExecOutputStream;
use crate::shell::get_shell_by_model_provided_path;
use crate::tools::context::ToolInvocation;
use crate::tools::context::ToolOutput;
use crate::tools::context::ToolPayload;
@@ -85,8 +84,7 @@ impl ToolHandler for UnifiedExecHandler {
let Ok(params) = serde_json::from_str::<ExecCommandArgs>(arguments) else {
return true;
};
let command = get_command(&params);
!is_known_safe_command(&command)
!is_known_safe_command(&["bash".to_string(), "-lc".to_string(), params.cmd])
}
async fn handle(&self, invocation: ToolInvocation) -> Result<ToolOutput, FunctionCallError> {
@@ -119,15 +117,15 @@ impl ToolHandler for UnifiedExecHandler {
"failed to parse exec_command arguments: {err:?}"
))
})?;
let command = get_command(&args);
let ExecCommandArgs {
cmd,
workdir,
shell,
login,
yield_time_ms,
max_output_tokens,
with_escalated_permissions,
justification,
..
} = args;
if with_escalated_permissions.unwrap_or(false)
@@ -154,14 +152,15 @@ impl ToolHandler for UnifiedExecHandler {
&context.call_id,
None,
);
let emitter = ToolEmitter::unified_exec(&command, cwd.clone(), true);
let emitter = ToolEmitter::unified_exec(cmd.clone(), cwd.clone(), true);
emitter.emit(event_ctx, ToolEventStage::Begin).await;
manager
.exec_command(
ExecCommandRequest {
command,
command: &cmd,
shell: &shell,
login,
yield_time_ms,
max_output_tokens,
workdir,
@@ -222,11 +221,6 @@ impl ToolHandler for UnifiedExecHandler {
}
}
fn get_command(args: &ExecCommandArgs) -> Vec<String> {
let shell = get_shell_by_model_provided_path(&PathBuf::from(args.shell.clone()));
shell.derive_exec_args(&args.cmd, args.login)
}
fn format_response(response: &UnifiedExecResponse) -> String {
let mut sections = Vec::new();
codex-rs/core/src/unified_exec/mod.rs
+8 -4
View File
@@ -64,8 +64,10 @@ impl UnifiedExecContext {
}
#[derive(Debug)]
pub(crate) struct ExecCommandRequest {
pub command: Vec<String>,
pub(crate) struct ExecCommandRequest<'a> {
pub command: &'a str,
pub shell: &'a str,
pub login: bool,
pub yield_time_ms: Option<u64>,
pub max_output_tokens: Option<usize>,
pub workdir: Option<PathBuf>,
@@ -103,7 +105,7 @@ struct SessionEntry {
session_ref: Arc<Session>,
turn_ref: Arc<TurnContext>,
call_id: String,
command: Vec<String>,
command: String,
cwd: PathBuf,
started_at: tokio::time::Instant,
}
@@ -195,7 +197,9 @@ mod tests {
.unified_exec_manager
.exec_command(
ExecCommandRequest {
command: vec!["bash".to_string(), "-lc".to_string(), cmd.to_string()],
command: cmd,
shell: "/bin/bash",
login: true,
yield_time_ms,
max_output_tokens: None,
workdir: None,
codex-rs/core/src/unified_exec/session_manager.rs
+16 -10
View File
@@ -36,17 +36,23 @@ use super::truncate_output_to_tokens;
impl UnifiedExecSessionManager {
pub(crate) async fn exec_command(
&self,
request: ExecCommandRequest,
request: ExecCommandRequest<'_>,
context: &UnifiedExecContext,
) -> Result<UnifiedExecResponse, UnifiedExecError> {
let cwd = request
.workdir
.clone()
.unwrap_or_else(|| context.turn.cwd.clone());
let shell_flag = if request.login { "-lc" } else { "-c" };
let command = vec![
request.shell.to_string(),
shell_flag.to_string(),
request.command.to_string(),
];
let session = self
.open_session_with_sandbox(
&request.command,
command,
cwd.clone(),
request.with_escalated_permissions,
request.justification,
@@ -73,7 +79,7 @@ impl UnifiedExecSessionManager {
None
} else {
Some(
self.store_session(session, context, &request.command, cwd.clone(), start)
self.store_session(session, context, request.command, cwd.clone(), start)
.await,
)
};
@@ -93,7 +99,7 @@ impl UnifiedExecSessionManager {
let exit = response.exit_code.unwrap_or(-1);
Self::emit_exec_end_from_context(
context,
&request.command,
request.command.to_string(),
cwd,
response.output.clone(),
exit,
@@ -218,7 +224,7 @@ impl UnifiedExecSessionManager {
&self,
session: UnifiedExecSession,
context: &UnifiedExecContext,
command: &[String],
command: &str,
cwd: PathBuf,
started_at: Instant,
) -> i32 {
@@ -230,7 +236,7 @@ impl UnifiedExecSessionManager {
session_ref: Arc::clone(&context.session),
turn_ref: Arc::clone(&context.turn),
call_id: context.call_id.clone(),
command: command.to_vec(),
command: command.to_string(),
cwd,
started_at,
};
@@ -258,7 +264,7 @@ impl UnifiedExecSessionManager {
&entry.call_id,
None,
);
let emitter = ToolEmitter::unified_exec(&entry.command, entry.cwd, true);
let emitter = ToolEmitter::unified_exec(entry.command, entry.cwd, true);
emitter
.emit(event_ctx, ToolEventStage::Success(output))
.await;
@@ -266,7 +272,7 @@ impl UnifiedExecSessionManager {
async fn emit_exec_end_from_context(
context: &UnifiedExecContext,
command: &[String],
command: String,
cwd: PathBuf,
aggregated_output: String,
exit_code: i32,
@@ -315,7 +321,7 @@ impl UnifiedExecSessionManager {
pub(super) async fn open_session_with_sandbox(
&self,
command: &[String],
command: Vec<String>,
cwd: PathBuf,
with_escalated_permissions: Option<bool>,
justification: Option<String>,
@@ -324,7 +330,7 @@ impl UnifiedExecSessionManager {
let mut orchestrator = ToolOrchestrator::new();
let mut runtime = UnifiedExecRuntime::new(self);
let req = UnifiedExecToolRequest::new(
command.to_vec(),
command,
cwd,
create_env(&context.turn.shell_environment_policy),
with_escalated_permissions,
codex-rs/core/tests/suite/unified_exec.rs
+2 -13
View File
@@ -214,11 +214,7 @@ async fn unified_exec_emits_exec_command_begin_event() -> Result<()> {
assert_eq!(
begin_event.command,
vec![
"/bin/bash".to_string(),
"-lc".to_string(),
"/bin/echo hello unified exec".to_string()
]
vec!["/bin/echo hello unified exec".to_string()]
);
assert_eq!(begin_event.cwd, cwd.path());
@@ -658,14 +654,7 @@ async fn unified_exec_skips_begin_event_for_empty_input() -> Result<()> {
"expected only the initial command to emit begin event"
);
assert_eq!(begin_events[0].call_id, open_call_id);
assert_eq!(
begin_events[0].command,
vec![
"/bin/bash".to_string(),
"-lc".to_string(),
"/bin/sh -c echo ready".to_string()
]
);
assert_eq!(begin_events[0].command[0], "/bin/sh -c echo ready");
Ok(())
}