From d462cda2a75aaa7183ff51e832864130aa838acd Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 22 May 2026 17:30:20 -0700 Subject: [PATCH] runtime: prepend zsh fork bin dir to PATH Summary: - prepend the configured zsh fork executable directory to PATH for zsh-fork shell-command and unified-exec launches - preserve that PATH value through shell snapshot wrapping so #!/usr/bin/env zsh can resolve to the packaged fork - cover duplicate PATH entries and snapshot preservation in codex-core runtime tests Test Plan: - just fmt - cargo test -p codex-core apply_zsh_fork_path_prepend - cargo test -p codex-core maybe_wrap_shell_lc_with_snapshot_preserves_zsh_fork_path_prepend - just fix -p codex-core Notes: - cargo test -p codex-core currently aborts in unrelated thread_manager::tests::resume_and_fork_do_not_restore_thread_environments_from_rollout with a stack overflow; rerunning that single test reproduces the same stack overflow. --- codex-rs/core/src/tools/runtimes/mod.rs | 25 +++++ codex-rs/core/src/tools/runtimes/mod_tests.rs | 93 +++++++++++++++++++ codex-rs/core/src/tools/runtimes/shell.rs | 17 +++- .../tools/runtimes/shell/unix_escalation.rs | 5 +- .../tools/runtimes/shell/zsh_fork_backend.rs | 10 +- .../core/src/tools/runtimes/unified_exec.rs | 13 ++- 6 files changed, 153 insertions(+), 10 deletions(-) diff --git a/codex-rs/core/src/tools/runtimes/mod.rs b/codex-rs/core/src/tools/runtimes/mod.rs index bba1c572ef..71a3fd0cbb 100644 --- a/codex-rs/core/src/tools/runtimes/mod.rs +++ b/codex-rs/core/src/tools/runtimes/mod.rs @@ -22,6 +22,8 @@ use codex_sandboxing::SandboxCommand; use codex_sandboxing::SandboxType; use codex_utils_absolute_path::AbsolutePathBuf; use std::collections::HashMap; +#[cfg(unix)] +use std::path::Path; pub(crate) mod apply_patch; pub(crate) mod shell; @@ -70,6 +72,29 @@ pub(crate) fn exec_env_for_sandbox_permissions( env } +#[cfg(unix)] +pub(crate) fn apply_zsh_fork_path_prepend( + env: &mut HashMap, + explicit_env_overrides: &mut HashMap, + shell_zsh_path: &Path, +) { + let Some(zsh_bin_dir) = shell_zsh_path.parent() else { + return; + }; + let zsh_bin_dir = zsh_bin_dir.to_string_lossy().to_string(); + let updated_path = match env.get("PATH") { + Some(path) if !path.is_empty() => std::iter::once(zsh_bin_dir.as_str()) + .chain(path.split(':').filter(|entry| *entry != zsh_bin_dir)) + .collect::>() + .join(":"), + _ => zsh_bin_dir, + }; + env.insert("PATH".to_string(), updated_path.clone()); + // Snapshot wrapping restores explicit overrides after sourcing the shell + // snapshot, so capture this PATH override there as well. + explicit_env_overrides.insert("PATH".to_string(), updated_path); +} + pub(crate) fn disable_powershell_profile_for_elevated_windows_sandbox( command: &[String], shell_type: Option<&ShellType>, diff --git a/codex-rs/core/src/tools/runtimes/mod_tests.rs b/codex-rs/core/src/tools/runtimes/mod_tests.rs index fd10f22242..539bbb90de 100644 --- a/codex-rs/core/src/tools/runtimes/mod_tests.rs +++ b/codex-rs/core/src/tools/runtimes/mod_tests.rs @@ -143,6 +143,48 @@ async fn explicit_escalation_prepares_exec_without_managed_network() -> anyhow:: Ok(()) } +#[cfg(unix)] +#[test] +fn apply_zsh_fork_path_prepend_uses_shell_parent() { + let mut env = HashMap::from([("PATH".to_string(), "/usr/bin:/bin".to_string())]); + let mut explicit_env_overrides = HashMap::new(); + + apply_zsh_fork_path_prepend( + &mut env, + &mut explicit_env_overrides, + PathBuf::from("/package/codex-resources/zsh/bin/zsh").as_path(), + ); + + let expected = "/package/codex-resources/zsh/bin:/usr/bin:/bin"; + assert_eq!(env.get("PATH").map(String::as_str), Some(expected)); + assert_eq!( + explicit_env_overrides.get("PATH").map(String::as_str), + Some(expected) + ); +} + +#[cfg(unix)] +#[test] +fn apply_zsh_fork_path_prepend_moves_existing_shell_parent_to_front() { + let mut env = HashMap::from([( + "PATH".to_string(), + "/usr/bin:/package/codex-resources/zsh/bin:/bin:/package/codex-resources/zsh/bin" + .to_string(), + )]); + let mut explicit_env_overrides = HashMap::new(); + + apply_zsh_fork_path_prepend( + &mut env, + &mut explicit_env_overrides, + PathBuf::from("/package/codex-resources/zsh/bin/zsh").as_path(), + ); + + assert_eq!( + env.get("PATH").map(String::as_str), + Some("/package/codex-resources/zsh/bin:/usr/bin:/bin") + ); +} + #[test] fn explicit_escalation_keeps_user_proxy_env_without_codex_marker() { let env = HashMap::from([ @@ -818,6 +860,57 @@ fn maybe_wrap_shell_lc_with_snapshot_applies_explicit_path_override() { assert_eq!(String::from_utf8_lossy(&output.stdout), "/worktree/bin"); } +#[cfg(unix)] +#[test] +fn maybe_wrap_shell_lc_with_snapshot_preserves_zsh_fork_path_prepend() { + let dir = tempdir().expect("create temp dir"); + let snapshot_path = dir.path().join("snapshot.sh"); + std::fs::write( + &snapshot_path, + "# Snapshot file\nexport PATH='/snapshot/bin'\n", + ) + .expect("write snapshot"); + let session_shell = shell_with_snapshot( + ShellType::Bash, + "/bin/bash", + snapshot_path.abs(), + dir.path().abs(), + ); + let command = vec![ + "/bin/bash".to_string(), + "-lc".to_string(), + "printf '%s' \"$PATH\"".to_string(), + ]; + let zsh_path = dir + .path() + .join("codex-resources") + .join("zsh") + .join("bin") + .join("zsh"); + let zsh_bin_dir = zsh_path.parent().expect("zsh path should have parent"); + let mut env = HashMap::from([("PATH".to_string(), "/worktree/bin".to_string())]); + let mut explicit_env_overrides = HashMap::new(); + apply_zsh_fork_path_prepend(&mut env, &mut explicit_env_overrides, zsh_path.as_path()); + let rewritten = maybe_wrap_shell_lc_with_snapshot( + &command, + &session_shell, + &dir.path().abs(), + &explicit_env_overrides, + &env, + ); + let output = Command::new(&rewritten[0]) + .args(&rewritten[1..]) + .env("PATH", env.get("PATH").expect("PATH should be set")) + .output() + .expect("run rewritten command"); + + assert!(output.status.success(), "command failed: {output:?}"); + assert_eq!( + String::from_utf8_lossy(&output.stdout), + format!("{}:/worktree/bin", zsh_bin_dir.display()) + ); +} + #[test] fn maybe_wrap_shell_lc_with_snapshot_does_not_embed_override_values_in_argv() { let dir = tempdir().expect("create temp dir"); diff --git a/codex-rs/core/src/tools/runtimes/shell.rs b/codex-rs/core/src/tools/runtimes/shell.rs index 6251267773..7de15b3672 100644 --- a/codex-rs/core/src/tools/runtimes/shell.rs +++ b/codex-rs/core/src/tools/runtimes/shell.rs @@ -20,6 +20,8 @@ use crate::shell::ShellType; use crate::tools::flat_tool_name; use crate::tools::network_approval::NetworkApprovalMode; use crate::tools::network_approval::NetworkApprovalSpec; +#[cfg(unix)] +use crate::tools::runtimes::apply_zsh_fork_path_prepend; use crate::tools::runtimes::build_sandbox_command; use crate::tools::runtimes::disable_powershell_profile_for_elevated_windows_sandbox; use crate::tools::runtimes::exec_env_for_sandbox_permissions; @@ -231,12 +233,19 @@ impl ToolRuntime for ShellRuntime { let session_shell = ctx.session.user_shell(); let managed_network = managed_network_for_sandbox_permissions(req.network.as_ref(), req.sandbox_permissions); - let env = exec_env_for_sandbox_permissions(&req.env, req.sandbox_permissions); + let mut env = exec_env_for_sandbox_permissions(&req.env, req.sandbox_permissions); + let mut explicit_env_overrides = req.explicit_env_overrides.clone(); + #[cfg(unix)] + if self.backend == ShellRuntimeBackend::ShellCommandZshFork + && let Some(shell_zsh_path) = ctx.session.services.shell_zsh_path.as_deref() + { + apply_zsh_fork_path_prepend(&mut env, &mut explicit_env_overrides, shell_zsh_path); + } let command = maybe_wrap_shell_lc_with_snapshot( &req.command, session_shell.as_ref(), &req.cwd, - &req.explicit_env_overrides, + &explicit_env_overrides, &env, ); let command = disable_powershell_profile_for_elevated_windows_sandbox( @@ -252,7 +261,9 @@ impl ToolRuntime for ShellRuntime { }; if self.backend == ShellRuntimeBackend::ShellCommandZshFork { - match zsh_fork_backend::maybe_run_shell_command(req, attempt, ctx, &command).await? { + match zsh_fork_backend::maybe_run_shell_command(req, attempt, ctx, &command, &env) + .await? + { Some(out) => return Ok(out), None => { tracing::warn!( diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs index fef8db5ca9..597ab78dbe 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -15,7 +15,6 @@ use crate::sandboxing::ExecRequest; use crate::sandboxing::SandboxPermissions; use crate::shell::ShellType; use crate::tools::runtimes::build_sandbox_command; -use crate::tools::runtimes::exec_env_for_sandbox_permissions; use crate::tools::sandboxing::PermissionRequestPayload; use crate::tools::sandboxing::SandboxAttempt; use crate::tools::sandboxing::ToolCtx; @@ -102,6 +101,7 @@ pub(super) async fn try_run_zsh_fork( attempt: &SandboxAttempt<'_>, ctx: &ToolCtx, command: &[String], + env: &HashMap, ) -> Result, ToolError> { let Some(shell_zsh_path) = ctx.session.services.shell_zsh_path.as_ref() else { tracing::warn!("ZshFork backend specified, but shell_zsh_path is not configured."); @@ -116,9 +116,8 @@ pub(super) async fn try_run_zsh_fork( return Ok(None); } - let env = exec_env_for_sandbox_permissions(&req.env, req.sandbox_permissions); let command = - build_sandbox_command(command, &req.cwd, &env, req.additional_permissions.clone())?; + build_sandbox_command(command, &req.cwd, env, req.additional_permissions.clone())?; let options = ExecOptions { expiration: req.timeout_ms.into(), capture_policy: ExecCapturePolicy::ShellTool, diff --git a/codex-rs/core/src/tools/runtimes/shell/zsh_fork_backend.rs b/codex-rs/core/src/tools/runtimes/shell/zsh_fork_backend.rs index 819658ecca..093a60e038 100644 --- a/codex-rs/core/src/tools/runtimes/shell/zsh_fork_backend.rs +++ b/codex-rs/core/src/tools/runtimes/shell/zsh_fork_backend.rs @@ -7,6 +7,7 @@ use crate::tools::sandboxing::ToolError; use crate::unified_exec::SpawnLifecycleHandle; use codex_protocol::exec_output::ExecToolCallOutput; use codex_tools::ZshForkConfig; +use std::collections::HashMap; pub(crate) struct PreparedUnifiedExecSpawn { pub(crate) exec_request: ExecRequest, @@ -23,8 +24,9 @@ pub(crate) async fn maybe_run_shell_command( attempt: &SandboxAttempt<'_>, ctx: &ToolCtx, command: &[String], + env: &HashMap, ) -> Result, ToolError> { - imp::maybe_run_shell_command(req, attempt, ctx, command).await + imp::maybe_run_shell_command(req, attempt, ctx, command, env).await } /// Prepares unified exec to launch through the zsh-fork backend when the @@ -76,8 +78,9 @@ mod imp { attempt: &SandboxAttempt<'_>, ctx: &ToolCtx, command: &[String], + env: &HashMap, ) -> Result, ToolError> { - unix_escalation::try_run_zsh_fork(req, attempt, ctx, command).await + unix_escalation::try_run_zsh_fork(req, attempt, ctx, command, env).await } pub(super) async fn maybe_prepare_unified_exec( @@ -118,8 +121,9 @@ mod imp { attempt: &SandboxAttempt<'_>, ctx: &ToolCtx, command: &[String], + env: &HashMap, ) -> Result, ToolError> { - let _ = (req, attempt, ctx, command); + let _ = (req, attempt, ctx, command, env); Ok(None) } diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index c613f198e0..ec9de38679 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -17,6 +17,8 @@ use crate::shell::ShellType; use crate::tools::flat_tool_name; use crate::tools::network_approval::NetworkApprovalMode; use crate::tools::network_approval::NetworkApprovalSpec; +#[cfg(unix)] +use crate::tools::runtimes::apply_zsh_fork_path_prepend; use crate::tools::runtimes::build_sandbox_command; use crate::tools::runtimes::disable_powershell_profile_for_elevated_windows_sandbox; use crate::tools::runtimes::exec_env_for_sandbox_permissions; @@ -261,6 +263,15 @@ impl<'a> ToolRuntime for UnifiedExecRunt if let Some(network) = managed_network { network.apply_to_env(&mut env); } + let mut explicit_env_overrides = req.explicit_env_overrides.clone(); + #[cfg(unix)] + if let UnifiedExecShellMode::ZshFork(zsh_fork_config) = &self.shell_mode { + apply_zsh_fork_path_prepend( + &mut env, + &mut explicit_env_overrides, + zsh_fork_config.shell_zsh_path.as_path(), + ); + } let environment_is_remote = req.environment.is_remote(); let command = if environment_is_remote { base_command.to_vec() @@ -269,7 +280,7 @@ impl<'a> ToolRuntime for UnifiedExecRunt base_command, session_shell.as_ref(), &req.cwd, - &req.explicit_env_overrides, + &explicit_env_overrides, &env, ) };