mirror of
https://github.com/openai/codex.git
synced 2026-10-23 18:08:45 +00:00
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.
This commit is contained in:
@@ -22,6 +22,8 @@ use codex_sandboxing::SandboxCommand;
|
|||||||
use codex_sandboxing::SandboxType;
|
use codex_sandboxing::SandboxType;
|
||||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||||
use std::collections::HashMap;
|
use std::collections::HashMap;
|
||||||
|
#[cfg(unix)]
|
||||||
|
use std::path::Path;
|
||||||
|
|
||||||
pub(crate) mod apply_patch;
|
pub(crate) mod apply_patch;
|
||||||
pub(crate) mod shell;
|
pub(crate) mod shell;
|
||||||
@@ -70,6 +72,29 @@ pub(crate) fn exec_env_for_sandbox_permissions(
|
|||||||
env
|
env
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(unix)]
|
||||||
|
pub(crate) fn apply_zsh_fork_path_prepend(
|
||||||
|
env: &mut HashMap<String, String>,
|
||||||
|
explicit_env_overrides: &mut HashMap<String, String>,
|
||||||
|
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::<Vec<_>>()
|
||||||
|
.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(
|
pub(crate) fn disable_powershell_profile_for_elevated_windows_sandbox(
|
||||||
command: &[String],
|
command: &[String],
|
||||||
shell_type: Option<&ShellType>,
|
shell_type: Option<&ShellType>,
|
||||||
|
|||||||
@@ -143,6 +143,48 @@ async fn explicit_escalation_prepares_exec_without_managed_network() -> anyhow::
|
|||||||
Ok(())
|
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]
|
#[test]
|
||||||
fn explicit_escalation_keeps_user_proxy_env_without_codex_marker() {
|
fn explicit_escalation_keeps_user_proxy_env_without_codex_marker() {
|
||||||
let env = HashMap::from([
|
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");
|
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]
|
#[test]
|
||||||
fn maybe_wrap_shell_lc_with_snapshot_does_not_embed_override_values_in_argv() {
|
fn maybe_wrap_shell_lc_with_snapshot_does_not_embed_override_values_in_argv() {
|
||||||
let dir = tempdir().expect("create temp dir");
|
let dir = tempdir().expect("create temp dir");
|
||||||
|
|||||||
@@ -20,6 +20,8 @@ use crate::shell::ShellType;
|
|||||||
use crate::tools::flat_tool_name;
|
use crate::tools::flat_tool_name;
|
||||||
use crate::tools::network_approval::NetworkApprovalMode;
|
use crate::tools::network_approval::NetworkApprovalMode;
|
||||||
use crate::tools::network_approval::NetworkApprovalSpec;
|
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::build_sandbox_command;
|
||||||
use crate::tools::runtimes::disable_powershell_profile_for_elevated_windows_sandbox;
|
use crate::tools::runtimes::disable_powershell_profile_for_elevated_windows_sandbox;
|
||||||
use crate::tools::runtimes::exec_env_for_sandbox_permissions;
|
use crate::tools::runtimes::exec_env_for_sandbox_permissions;
|
||||||
@@ -231,12 +233,19 @@ impl ToolRuntime<ShellRequest, ExecToolCallOutput> for ShellRuntime {
|
|||||||
let session_shell = ctx.session.user_shell();
|
let session_shell = ctx.session.user_shell();
|
||||||
let managed_network =
|
let managed_network =
|
||||||
managed_network_for_sandbox_permissions(req.network.as_ref(), req.sandbox_permissions);
|
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(
|
let command = maybe_wrap_shell_lc_with_snapshot(
|
||||||
&req.command,
|
&req.command,
|
||||||
session_shell.as_ref(),
|
session_shell.as_ref(),
|
||||||
&req.cwd,
|
&req.cwd,
|
||||||
&req.explicit_env_overrides,
|
&explicit_env_overrides,
|
||||||
&env,
|
&env,
|
||||||
);
|
);
|
||||||
let command = disable_powershell_profile_for_elevated_windows_sandbox(
|
let command = disable_powershell_profile_for_elevated_windows_sandbox(
|
||||||
@@ -252,7 +261,9 @@ impl ToolRuntime<ShellRequest, ExecToolCallOutput> for ShellRuntime {
|
|||||||
};
|
};
|
||||||
|
|
||||||
if self.backend == ShellRuntimeBackend::ShellCommandZshFork {
|
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),
|
Some(out) => return Ok(out),
|
||||||
None => {
|
None => {
|
||||||
tracing::warn!(
|
tracing::warn!(
|
||||||
|
|||||||
@@ -15,7 +15,6 @@ use crate::sandboxing::ExecRequest;
|
|||||||
use crate::sandboxing::SandboxPermissions;
|
use crate::sandboxing::SandboxPermissions;
|
||||||
use crate::shell::ShellType;
|
use crate::shell::ShellType;
|
||||||
use crate::tools::runtimes::build_sandbox_command;
|
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::PermissionRequestPayload;
|
||||||
use crate::tools::sandboxing::SandboxAttempt;
|
use crate::tools::sandboxing::SandboxAttempt;
|
||||||
use crate::tools::sandboxing::ToolCtx;
|
use crate::tools::sandboxing::ToolCtx;
|
||||||
@@ -102,6 +101,7 @@ pub(super) async fn try_run_zsh_fork(
|
|||||||
attempt: &SandboxAttempt<'_>,
|
attempt: &SandboxAttempt<'_>,
|
||||||
ctx: &ToolCtx,
|
ctx: &ToolCtx,
|
||||||
command: &[String],
|
command: &[String],
|
||||||
|
env: &HashMap<String, String>,
|
||||||
) -> Result<Option<ExecToolCallOutput>, ToolError> {
|
) -> Result<Option<ExecToolCallOutput>, ToolError> {
|
||||||
let Some(shell_zsh_path) = ctx.session.services.shell_zsh_path.as_ref() else {
|
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.");
|
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);
|
return Ok(None);
|
||||||
}
|
}
|
||||||
|
|
||||||
let env = exec_env_for_sandbox_permissions(&req.env, req.sandbox_permissions);
|
|
||||||
let command =
|
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 {
|
let options = ExecOptions {
|
||||||
expiration: req.timeout_ms.into(),
|
expiration: req.timeout_ms.into(),
|
||||||
capture_policy: ExecCapturePolicy::ShellTool,
|
capture_policy: ExecCapturePolicy::ShellTool,
|
||||||
|
|||||||
@@ -7,6 +7,7 @@ use crate::tools::sandboxing::ToolError;
|
|||||||
use crate::unified_exec::SpawnLifecycleHandle;
|
use crate::unified_exec::SpawnLifecycleHandle;
|
||||||
use codex_protocol::exec_output::ExecToolCallOutput;
|
use codex_protocol::exec_output::ExecToolCallOutput;
|
||||||
use codex_tools::ZshForkConfig;
|
use codex_tools::ZshForkConfig;
|
||||||
|
use std::collections::HashMap;
|
||||||
|
|
||||||
pub(crate) struct PreparedUnifiedExecSpawn {
|
pub(crate) struct PreparedUnifiedExecSpawn {
|
||||||
pub(crate) exec_request: ExecRequest,
|
pub(crate) exec_request: ExecRequest,
|
||||||
@@ -23,8 +24,9 @@ pub(crate) async fn maybe_run_shell_command(
|
|||||||
attempt: &SandboxAttempt<'_>,
|
attempt: &SandboxAttempt<'_>,
|
||||||
ctx: &ToolCtx,
|
ctx: &ToolCtx,
|
||||||
command: &[String],
|
command: &[String],
|
||||||
|
env: &HashMap<String, String>,
|
||||||
) -> Result<Option<ExecToolCallOutput>, ToolError> {
|
) -> Result<Option<ExecToolCallOutput>, 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
|
/// Prepares unified exec to launch through the zsh-fork backend when the
|
||||||
@@ -76,8 +78,9 @@ mod imp {
|
|||||||
attempt: &SandboxAttempt<'_>,
|
attempt: &SandboxAttempt<'_>,
|
||||||
ctx: &ToolCtx,
|
ctx: &ToolCtx,
|
||||||
command: &[String],
|
command: &[String],
|
||||||
|
env: &HashMap<String, String>,
|
||||||
) -> Result<Option<ExecToolCallOutput>, ToolError> {
|
) -> Result<Option<ExecToolCallOutput>, 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(
|
pub(super) async fn maybe_prepare_unified_exec(
|
||||||
@@ -118,8 +121,9 @@ mod imp {
|
|||||||
attempt: &SandboxAttempt<'_>,
|
attempt: &SandboxAttempt<'_>,
|
||||||
ctx: &ToolCtx,
|
ctx: &ToolCtx,
|
||||||
command: &[String],
|
command: &[String],
|
||||||
|
env: &HashMap<String, String>,
|
||||||
) -> Result<Option<ExecToolCallOutput>, ToolError> {
|
) -> Result<Option<ExecToolCallOutput>, ToolError> {
|
||||||
let _ = (req, attempt, ctx, command);
|
let _ = (req, attempt, ctx, command, env);
|
||||||
Ok(None)
|
Ok(None)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -17,6 +17,8 @@ use crate::shell::ShellType;
|
|||||||
use crate::tools::flat_tool_name;
|
use crate::tools::flat_tool_name;
|
||||||
use crate::tools::network_approval::NetworkApprovalMode;
|
use crate::tools::network_approval::NetworkApprovalMode;
|
||||||
use crate::tools::network_approval::NetworkApprovalSpec;
|
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::build_sandbox_command;
|
||||||
use crate::tools::runtimes::disable_powershell_profile_for_elevated_windows_sandbox;
|
use crate::tools::runtimes::disable_powershell_profile_for_elevated_windows_sandbox;
|
||||||
use crate::tools::runtimes::exec_env_for_sandbox_permissions;
|
use crate::tools::runtimes::exec_env_for_sandbox_permissions;
|
||||||
@@ -261,6 +263,15 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
|||||||
if let Some(network) = managed_network {
|
if let Some(network) = managed_network {
|
||||||
network.apply_to_env(&mut env);
|
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 environment_is_remote = req.environment.is_remote();
|
||||||
let command = if environment_is_remote {
|
let command = if environment_is_remote {
|
||||||
base_command.to_vec()
|
base_command.to_vec()
|
||||||
@@ -269,7 +280,7 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
|||||||
base_command,
|
base_command,
|
||||||
session_shell.as_ref(),
|
session_shell.as_ref(),
|
||||||
&req.cwd,
|
&req.cwd,
|
||||||
&req.explicit_env_overrides,
|
&explicit_env_overrides,
|
||||||
&env,
|
&env,
|
||||||
)
|
)
|
||||||
};
|
};
|
||||||
|
|||||||
Reference in New Issue
Block a user