diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index eb4206b7..42960b27 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -405,7 +405,7 @@ impl Session { let rollout_fut = RolloutRecorder::new(&config, rollout_params); let mcp_fut = McpConnectionManager::new(config.mcp_servers.clone()); - let default_shell_fut = shell::default_user_shell(); + let default_shell_fut = shell::default_user_shell(conversation_id.0, &config.codex_home); let history_meta_fut = crate::message_history::history_metadata(&config); // Join all independent futures. @@ -477,6 +477,7 @@ impl Session { shell_environment_policy: config.shell_environment_policy.clone(), cwd, }; + let sess = Arc::new(Session { conversation_id, tx_event: tx_event.clone(), @@ -2323,13 +2324,25 @@ pub struct ExecInvokeArgs<'a> { pub stdout_stream: Option, } +fn should_translate_shell_command( + shell: &crate::shell::Shell, + shell_policy: &ShellEnvironmentPolicy, +) -> bool { + matches!(shell, crate::shell::Shell::PowerShell(_)) + || shell_policy.use_profile + || matches!( + shell, + crate::shell::Shell::Posix(shell) if shell.shell_snapshot.is_some() + ) +} + fn maybe_translate_shell_command( params: ExecParams, sess: &Session, turn_context: &TurnContext, ) -> ExecParams { - let should_translate = matches!(sess.user_shell, crate::shell::Shell::PowerShell(_)) - || turn_context.shell_environment_policy.use_profile; + let should_translate = + should_translate_shell_command(&sess.user_shell, &turn_context.shell_environment_policy); if should_translate && let Some(command) = sess @@ -2946,10 +2959,15 @@ fn convert_call_tool_result_to_function_call_output_payload( #[cfg(test)] mod tests { use super::*; + use crate::config_types::ShellEnvironmentPolicyInherit; use mcp_types::ContentBlock; use mcp_types::TextContent; use pretty_assertions::assert_eq; use serde_json::json; + use shell::ShellSnapshot; + use std::collections::HashMap; + use std::path::PathBuf; + use std::sync::Arc; use std::time::Duration as StdDuration; fn text_block(s: &str) -> ContentBlock { @@ -2960,6 +2978,48 @@ mod tests { }) } + fn shell_policy_with_profile(use_profile: bool) -> ShellEnvironmentPolicy { + ShellEnvironmentPolicy { + inherit: ShellEnvironmentPolicyInherit::All, + ignore_default_excludes: false, + exclude: Vec::new(), + r#set: HashMap::new(), + include_only: Vec::new(), + use_profile, + } + } + + fn zsh_shell(shell_snapshot: Option>) -> shell::Shell { + shell::Shell::Posix(shell::PosixShell { + shell_path: "/bin/zsh".to_string(), + rc_path: "/Users/example/.zshrc".to_string(), + shell_snapshot, + }) + } + + #[test] + fn translates_commands_when_shell_policy_requests_profile() { + let policy = shell_policy_with_profile(true); + let shell = zsh_shell(None); + assert!(should_translate_shell_command(&shell, &policy)); + } + + #[test] + fn translates_commands_for_zsh_with_snapshot() { + let policy = shell_policy_with_profile(false); + let shell = zsh_shell(Some(Arc::new(ShellSnapshot::new(PathBuf::from( + "/tmp/snapshot", + ))))); + assert!(should_translate_shell_command(&shell, &policy)); + } + + #[test] + fn bypasses_translation_for_zsh_without_snapshot_or_profile() { + let policy = shell_policy_with_profile(false); + let shell = zsh_shell(None); + assert!(!should_translate_shell_command(&shell, &policy)); + } + #[test] fn prefers_structured_content_when_present() { let ctr = CallToolResult { diff --git a/codex-rs/core/src/shell.rs b/codex-rs/core/src/shell.rs index 7ef79076..d1f3b2cc 100644 --- a/codex-rs/core/src/shell.rs +++ b/codex-rs/core/src/shell.rs @@ -1,18 +1,36 @@ use serde::Deserialize; use serde::Serialize; use shlex; +use std::path::Path; use std::path::PathBuf; +use std::sync::Arc; +use tracing::trace; +use uuid::Uuid; -#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)] -pub struct ZshShell { - shell_path: String, - zshrc_path: String, +#[derive(Debug, PartialEq, Eq, Serialize, Deserialize)] +/// This structure cannot derive Clone or this will break the Drop implementation. +pub struct ShellSnapshot { + pub(crate) path: PathBuf, +} + +impl ShellSnapshot { + pub fn new(path: PathBuf) -> Self { + Self { path } + } +} + +impl Drop for ShellSnapshot { + fn drop(&mut self) { + delete_shell_snapshot(&self.path); + } } #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)] -pub struct BashShell { - shell_path: String, - bashrc_path: String, +pub struct PosixShell { + pub(crate) shell_path: String, + pub(crate) rc_path: String, + #[serde(skip_serializing, skip_deserializing)] + pub(crate) shell_snapshot: Option>, } #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)] @@ -23,8 +41,7 @@ pub struct PowerShellConfig { #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)] pub enum Shell { - Zsh(ZshShell), - Bash(BashShell), + Posix(PosixShell), PowerShell(PowerShellConfig), Unknown, } @@ -32,11 +49,27 @@ pub enum Shell { impl Shell { pub fn format_default_shell_invocation(&self, command: Vec) -> Option> { match self { - Shell::Zsh(zsh) => { - format_shell_invocation_with_rc(&command, &zsh.shell_path, &zsh.zshrc_path) - } - Shell::Bash(bash) => { - format_shell_invocation_with_rc(&command, &bash.shell_path, &bash.bashrc_path) + Shell::Posix(shell) => { + let joined = strip_bash_lc(&command) + .or_else(|| shlex::try_join(command.iter().map(|s| s.as_str())).ok())?; + + let mut source_path = Path::new(&shell.rc_path); + + let session_cmd = if let Some(shell_snapshot) = &shell.shell_snapshot + && shell_snapshot.path.exists() + { + source_path = shell_snapshot.path.as_path(); + "-c".to_string() + } else { + "-lc".to_string() + }; + + let source_path_str = source_path.to_string_lossy().to_string(); + let quoted_source_path = shlex::try_quote(&source_path_str).ok()?; + let rc_command = + format!("[ -f {quoted_source_path} ] && . {quoted_source_path}; ({joined})"); + + Some(vec![shell.shell_path.clone(), session_cmd, rc_command]) } Shell::PowerShell(ps) => { // If model generated a bash command, prefer a detected bash fallback @@ -89,33 +122,20 @@ impl Shell { pub fn name(&self) -> Option { match self { - Shell::Zsh(zsh) => std::path::Path::new(&zsh.shell_path) - .file_name() - .map(|s| s.to_string_lossy().to_string()), - Shell::Bash(bash) => std::path::Path::new(&bash.shell_path) + Shell::Posix(shell) => Path::new(&shell.shell_path) .file_name() .map(|s| s.to_string_lossy().to_string()), Shell::PowerShell(ps) => Some(ps.exe.clone()), Shell::Unknown => None, } } -} -fn format_shell_invocation_with_rc( - command: &Vec, - shell_path: &str, - rc_path: &str, -) -> Option> { - let joined = strip_bash_lc(command) - .or_else(|| shlex::try_join(command.iter().map(|s| s.as_str())).ok())?; - - let rc_command = if std::path::Path::new(rc_path).exists() { - format!("source {rc_path} && ({joined})") - } else { - joined - }; - - Some(vec![shell_path.to_string(), "-lc".to_string(), rc_command]) + pub fn get_snapshot(&self) -> Option> { + match self { + Shell::Posix(shell) => shell.shell_snapshot.clone(), + _ => None, + } + } } fn strip_bash_lc(command: &Vec) -> Option { @@ -132,7 +152,7 @@ fn strip_bash_lc(command: &Vec) -> Option { } #[cfg(unix)] -fn detect_default_user_shell() -> Shell { +async fn detect_default_user_shell(session_id: Uuid, codex_home: &Path) -> Shell { use libc::getpwuid; use libc::getuid; use std::ffi::CStr; @@ -147,31 +167,45 @@ fn detect_default_user_shell() -> Shell { .into_owned(); let home_path = CStr::from_ptr((*pw).pw_dir).to_string_lossy().into_owned(); - if shell_path.ends_with("/zsh") { - return Shell::Zsh(ZshShell { - shell_path, - zshrc_path: format!("{home_path}/.zshrc"), - }); - } + let rc_path = if shell_path.ends_with("/zsh") { + format!("{home_path}/.zshrc") + } else if shell_path.ends_with("/bash") { + format!("{home_path}/.bashrc") + } else { + return Shell::Unknown; + }; - if shell_path.ends_with("/bash") { - return Shell::Bash(BashShell { - shell_path, - bashrc_path: format!("{home_path}/.bashrc"), - }); + let snapshot_path = snapshots::ensure_posix_snapshot( + &shell_path, + &rc_path, + Path::new(&home_path), + codex_home, + session_id, + ) + .await; + if snapshot_path.is_none() { + trace!("failed to prepare posix snapshot; using live profile"); } + let shell_snapshot = + snapshot_path.map(|snapshot| Arc::new(ShellSnapshot::new(snapshot))); + + return Shell::Posix(PosixShell { + shell_path, + rc_path, + shell_snapshot, + }); } } Shell::Unknown } #[cfg(unix)] -pub async fn default_user_shell() -> Shell { - detect_default_user_shell() +pub async fn default_user_shell(session_id: Uuid, codex_home: &Path) -> Shell { + detect_default_user_shell(session_id, codex_home).await } #[cfg(target_os = "windows")] -pub async fn default_user_shell() -> Shell { +pub async fn default_user_shell(_session_id: Uuid, _codex_home: &Path) -> Shell { use tokio::process::Command; // Prefer PowerShell 7+ (`pwsh`) if available, otherwise fall back to Windows PowerShell. @@ -211,42 +245,158 @@ pub async fn default_user_shell() -> Shell { } #[cfg(all(not(target_os = "windows"), not(unix)))] -pub async fn default_user_shell() -> Shell { +pub async fn default_user_shell(_session_id: Uuid, _codex_home: &Path) -> Shell { Shell::Unknown } +#[cfg(unix)] +mod snapshots { + use super::*; + + fn zsh_profile_paths(home: &Path) -> Vec { + [".zshenv", ".zprofile", ".zshrc", ".zlogin"] + .into_iter() + .map(|name| home.join(name)) + .collect() + } + + fn posix_profile_source_script(home: &Path) -> String { + zsh_profile_paths(home) + .into_iter() + .map(|profile| { + let profile_string = profile.to_string_lossy().into_owned(); + let quoted = shlex::try_quote(&profile_string) + .map(|cow| cow.into_owned()) + .unwrap_or(profile_string.clone()); + + format!("[ -f {quoted} ] && . {quoted}") + }) + .collect::>() + .join("; ") + } + + pub(crate) async fn ensure_posix_snapshot( + shell_path: &str, + rc_path: &str, + home: &Path, + codex_home: &Path, + session_id: Uuid, + ) -> Option { + let snapshot_path = codex_home.join(format!("shell_snapshots/snapshot_{session_id}.zsh")); + + // Check if an update in the profile requires to re-generate the snapshot. + let snapshot_is_stale = async { + let snapshot_metadata = tokio::fs::metadata(&snapshot_path).await.ok()?; + let snapshot_modified = snapshot_metadata.modified().ok()?; + + for profile in zsh_profile_paths(home) { + let Ok(profile_metadata) = tokio::fs::metadata(&profile).await else { + continue; + }; + + let Ok(profile_modified) = profile_metadata.modified() else { + return Some(true); + }; + + if profile_modified > snapshot_modified { + return Some(true); + } + } + + Some(false) + } + .await + .unwrap_or(true); + + if !snapshot_is_stale { + return Some(snapshot_path); + } + + match regenerate_posix_snapshot(shell_path, rc_path, home, &snapshot_path).await { + Ok(()) => Some(snapshot_path), + Err(err) => { + tracing::warn!("failed to generate posix snapshot: {err}"); + None + } + } + } + + async fn regenerate_posix_snapshot( + shell_path: &str, + rc_path: &str, + home: &Path, + snapshot_path: &Path, + ) -> std::io::Result<()> { + // Use `emulate -L sh` instead of `set -o posix` so we work on zsh builds + // that disable that option. Guard `alias -p` with `|| true` so the script + // keeps a zero exit status even if aliases are disabled. + let mut capture_script = String::new(); + let profile_sources = posix_profile_source_script(home); + if !profile_sources.is_empty() { + capture_script.push_str(&format!("{profile_sources}; ")); + } + + let zshrc = home.join(rc_path); + + capture_script.push_str( + &format!(". {}; setopt posixbuiltins; export -p; {{ alias | sed 's/^/alias /'; }} 2>/dev/null || true", zshrc.display()), + ); + let output = tokio::process::Command::new(shell_path) + .arg("-lc") + .arg(capture_script) + .env("HOME", home) + .output() + .await?; + + if !output.status.success() { + return Err(std::io::Error::other(format!( + "snapshot capture exited with status {}", + output.status + ))); + } + + let mut contents = String::from("# Generated by Codex. Do not edit.\n"); + + contents.push_str(&String::from_utf8_lossy(&output.stdout)); + contents.push('\n'); + + if let Some(parent) = snapshot_path.parent() { + tokio::fs::create_dir_all(parent).await?; + } + + let tmp_path = snapshot_path.with_extension("tmp"); + tokio::fs::write(&tmp_path, contents).await?; + + // Restrict the snapshot to user read/write so that environment variables or aliases + // that may contain secrets are not exposed to other users on the system. + use std::os::unix::fs::PermissionsExt; + let permissions = std::fs::Permissions::from_mode(0o600); + tokio::fs::set_permissions(&tmp_path, permissions).await?; + + tokio::fs::rename(&tmp_path, snapshot_path).await?; + Ok(()) + } +} + +pub(crate) fn delete_shell_snapshot(path: &Path) { + if let Err(err) = std::fs::remove_file(path) { + trace!("failed to delete shell snapshot {path:?}: {err}"); + } +} + #[cfg(test)] #[cfg(unix)] mod tests { use super::*; - use std::process::Command; - #[tokio::test] - async fn test_current_shell_detects_zsh() { - let shell = Command::new("sh") - .arg("-c") - .arg("echo $SHELL") - .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: shell_path.to_string(), - zshrc_path: format!("{home}/.zshrc",), - }) - ); - } - } + use std::path::PathBuf; #[tokio::test] async fn test_run_with_profile_zshrc_not_exists() { - let shell = Shell::Zsh(ZshShell { + let shell = Shell::Posix(PosixShell { shell_path: "/bin/zsh".to_string(), - zshrc_path: "/does/not/exist/.zshrc".to_string(), + rc_path: "/does/not/exist/.zshrc".to_string(), + shell_snapshot: None, }); let actual_cmd = shell.format_default_shell_invocation(vec!["myecho".to_string()]); assert_eq!( @@ -254,24 +404,7 @@ mod tests { Some(vec![ "/bin/zsh".to_string(), "-lc".to_string(), - "myecho".to_string() - ]) - ); - } - - #[tokio::test] - async fn test_run_with_profile_bashrc_not_exists() { - let shell = Shell::Bash(BashShell { - shell_path: "/bin/bash".to_string(), - bashrc_path: "/does/not/exist/.bashrc".to_string(), - }); - let actual_cmd = shell.format_default_shell_invocation(vec!["myecho".to_string()]); - assert_eq!( - actual_cmd, - Some(vec![ - "/bin/bash".to_string(), - "-lc".to_string(), - "myecho".to_string() + "[ -f /does/not/exist/.zshrc ] && . /does/not/exist/.zshrc; (myecho)".to_string(), ]) ); } @@ -283,7 +416,11 @@ mod tests { let cases = vec![ ( vec!["myecho"], - vec![shell_path, "-lc", "source BASHRC_PATH && (myecho)"], + vec![ + shell_path, + "-lc", + "[ -f BASHRC_PATH ] && . BASHRC_PATH; (myecho)", + ], Some("It works!\n"), ), ( @@ -291,7 +428,7 @@ mod tests { vec![ shell_path, "-lc", - "source BASHRC_PATH && (echo 'single' \"double\")", + "[ -f BASHRC_PATH ] && . BASHRC_PATH; (echo 'single' \"double\")", ], Some("single double\n"), ), @@ -317,9 +454,10 @@ mod tests { "#, ) .unwrap(); - let shell = Shell::Bash(BashShell { + let shell = Shell::Posix(PosixShell { shell_path: shell_path.to_string(), - bashrc_path: bashrc_path.to_str().unwrap().to_string(), + rc_path: bashrc_path.to_str().unwrap().to_string(), + shell_snapshot: None, }); let actual_cmd = shell @@ -369,6 +507,82 @@ mod tests { #[cfg(target_os = "macos")] mod macos_tests { use super::*; + use crate::shell::snapshots::ensure_posix_snapshot; + + #[tokio::test] + async fn test_snapshot_generation_uses_session_id_and_cleanup() { + let shell_path = "/bin/zsh"; + + let temp_home = tempfile::tempdir().unwrap(); + let codex_home = tempfile::tempdir().unwrap(); + std::fs::write( + temp_home.path().join(".zshrc"), + "export SNAPSHOT_TEST_VAR=1\nalias snapshot_test_alias='echo hi'\n", + ) + .unwrap(); + + let session_id = Uuid::new_v4(); + let snapshot_path = ensure_posix_snapshot( + shell_path, + ".zshrc", + temp_home.path(), + codex_home.path(), + session_id, + ) + .await + .expect("snapshot path"); + + let filename = snapshot_path + .file_name() + .unwrap() + .to_string_lossy() + .to_string(); + assert!(filename.contains(&session_id.to_string())); + assert!(snapshot_path.exists()); + + let snapshot_path_second = ensure_posix_snapshot( + shell_path, + ".zshrc", + temp_home.path(), + codex_home.path(), + session_id, + ) + .await + .expect("snapshot path"); + assert_eq!(snapshot_path, snapshot_path_second); + + let contents = std::fs::read_to_string(&snapshot_path).unwrap(); + assert!(contents.contains("alias snapshot_test_alias='echo hi'")); + assert!(contents.contains("SNAPSHOT_TEST_VAR=1")); + + delete_shell_snapshot(&snapshot_path); + assert!(!snapshot_path.exists()); + } + + #[test] + fn format_default_shell_invocation_prefers_snapshot_when_available() { + let temp_dir = tempfile::tempdir().unwrap(); + let snapshot_path = temp_dir.path().join("snapshot.zsh"); + std::fs::write(&snapshot_path, "export SNAPSHOT_READY=1").unwrap(); + + let shell = Shell::Posix(PosixShell { + shell_path: "/bin/zsh".to_string(), + rc_path: { + let path = temp_dir.path().join(".zshrc"); + std::fs::write(&path, "# test zshrc").unwrap(); + path.to_string_lossy().to_string() + }, + shell_snapshot: Some(Arc::new(ShellSnapshot::new(snapshot_path.clone()))), + }); + + let invocation = shell.format_default_shell_invocation(vec!["echo".to_string()]); + let expected_command = vec!["/bin/zsh".to_string(), "-c".to_string(), { + let snapshot_path = snapshot_path.to_string_lossy(); + format!("[ -f {snapshot_path} ] && . {snapshot_path}; (echo)") + }]; + + assert_eq!(invocation, Some(expected_command)); + } #[tokio::test] async fn test_run_with_profile_escaping_and_execution() { @@ -377,12 +591,20 @@ mod macos_tests { let cases = vec![ ( vec!["myecho"], - vec![shell_path, "-lc", "source ZSHRC_PATH && (myecho)"], + vec![ + shell_path, + "-lc", + "[ -f ZSHRC_PATH ] && . ZSHRC_PATH; (myecho)", + ], Some("It works!\n"), ), ( vec!["myecho"], - vec![shell_path, "-lc", "source ZSHRC_PATH && (myecho)"], + vec![ + shell_path, + "-lc", + "[ -f ZSHRC_PATH ] && . ZSHRC_PATH; (myecho)", + ], Some("It works!\n"), ), ( @@ -390,7 +612,7 @@ mod macos_tests { vec![ shell_path, "-lc", - "source ZSHRC_PATH && (bash -c \"echo 'single' \\\"double\\\"\")", + "[ -f ZSHRC_PATH ] && . ZSHRC_PATH; (bash -c \"echo 'single' \\\"double\\\"\")", ], Some("single double\n"), ), @@ -399,7 +621,7 @@ mod macos_tests { vec![ shell_path, "-lc", - "source ZSHRC_PATH && (echo 'single' \"double\")", + "[ -f ZSHRC_PATH ] && . ZSHRC_PATH; (echo 'single' \"double\")", ], Some("single double\n"), ), @@ -426,9 +648,10 @@ mod macos_tests { "#, ) .unwrap(); - let shell = Shell::Zsh(ZshShell { + let shell = Shell::Posix(PosixShell { shell_path: shell_path.to_string(), - zshrc_path: zshrc_path.to_str().unwrap().to_string(), + rc_path: zshrc_path.to_str().unwrap().to_string(), + shell_snapshot: None, }); let actual_cmd = shell diff --git a/codex-rs/core/tests/suite/prompt_caching.rs b/codex-rs/core/tests/suite/prompt_caching.rs index 4625841a..4c30c864 100644 --- a/codex-rs/core/tests/suite/prompt_caching.rs +++ b/codex-rs/core/tests/suite/prompt_caching.rs @@ -17,6 +17,7 @@ use core_test_support::load_default_config_for_test; use core_test_support::load_sse_fixture_with_id; use core_test_support::wait_for_event; use tempfile::TempDir; +use uuid::Uuid; use wiremock::Mock; use wiremock::MockServer; use wiremock::ResponseTemplate; @@ -269,7 +270,7 @@ async fn prefixes_context_and_instructions_once_and_consistently_across_requests let requests = server.received_requests().await.unwrap(); assert_eq!(requests.len(), 2, "expected two POST requests"); - let shell = default_user_shell().await; + let shell = default_user_shell(Uuid::new_v4(), codex_home.path()).await; let expected_env_text = format!( r#"