mirror of
https://github.com/openai/codex.git
synced 2026-09-05 15:18:41 +00:00
Merge d462cda2a7 into sapling-pr-archive-bolinfest
This commit is contained in:
@@ -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<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(
|
||||
command: &[String],
|
||||
shell_type: Option<&ShellType>,
|
||||
|
||||
@@ -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");
|
||||
|
||||
@@ -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<ShellRequest, ExecToolCallOutput> 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<ShellRequest, ExecToolCallOutput> 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!(
|
||||
|
||||
@@ -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<String, String>,
|
||||
) -> Result<Option<ExecToolCallOutput>, 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,
|
||||
|
||||
@@ -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<String, String>,
|
||||
) -> 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
|
||||
@@ -76,8 +78,9 @@ mod imp {
|
||||
attempt: &SandboxAttempt<'_>,
|
||||
ctx: &ToolCtx,
|
||||
command: &[String],
|
||||
env: &HashMap<String, String>,
|
||||
) -> 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(
|
||||
@@ -118,8 +121,9 @@ mod imp {
|
||||
attempt: &SandboxAttempt<'_>,
|
||||
ctx: &ToolCtx,
|
||||
command: &[String],
|
||||
env: &HashMap<String, String>,
|
||||
) -> Result<Option<ExecToolCallOutput>, ToolError> {
|
||||
let _ = (req, attempt, ctx, command);
|
||||
let _ = (req, attempt, ctx, command, env);
|
||||
Ok(None)
|
||||
}
|
||||
|
||||
|
||||
@@ -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<UnifiedExecRequest, UnifiedExecProcess> 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<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
base_command,
|
||||
session_shell.as_ref(),
|
||||
&req.cwd,
|
||||
&req.explicit_env_overrides,
|
||||
&explicit_env_overrides,
|
||||
&env,
|
||||
)
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user