mirror of
https://github.com/openai/codex.git
synced 2026-10-26 18:38:47 +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 - keep Unix-only zsh-fork PATH mutation out of Windows clippy-visible mutability - cover duplicate PATH entries and snapshot preservation in codex-core runtime tests Test Plan: - just fmt - just fix -p codex-core - cargo test -p codex-core zsh_fork_path_prepend 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_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,44 @@ pub(crate) fn exec_env_for_sandbox_permissions(
|
||||
env
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
fn prepend_path_entry(env: &mut HashMap<String, String>, path_entry: &str) -> String {
|
||||
let updated_path = match env.get("PATH") {
|
||||
Some(path) if !path.is_empty() => std::iter::once(path_entry)
|
||||
.chain(path.split(':').filter(|entry| *entry != path_entry))
|
||||
.collect::<Vec<_>>()
|
||||
.join(":"),
|
||||
_ => path_entry.to_string(),
|
||||
};
|
||||
env.insert("PATH".to_string(), updated_path.clone());
|
||||
updated_path
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
pub(crate) fn prepend_zsh_fork_bin_to_path(
|
||||
env: &mut HashMap<String, String>,
|
||||
shell_zsh_path: &Path,
|
||||
) -> Option<String> {
|
||||
let zsh_bin_dir = shell_zsh_path
|
||||
.parent()
|
||||
.map(|path| path.to_string_lossy().to_string())?;
|
||||
Some(prepend_path_entry(env, &zsh_bin_dir))
|
||||
}
|
||||
|
||||
#[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(updated_path) = prepend_zsh_fork_bin_to_path(env, shell_zsh_path) else {
|
||||
return;
|
||||
};
|
||||
// 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;
|
||||
@@ -234,11 +236,23 @@ impl ToolRuntime<ShellRequest, ExecToolCallOutput> for ShellRuntime {
|
||||
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 explicit_env_overrides = req.explicit_env_overrides.clone();
|
||||
#[cfg(unix)]
|
||||
let (env, explicit_env_overrides) = {
|
||||
let mut env = env;
|
||||
let mut explicit_env_overrides = explicit_env_overrides;
|
||||
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);
|
||||
}
|
||||
(env, explicit_env_overrides)
|
||||
};
|
||||
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(
|
||||
|
||||
@@ -16,6 +16,7 @@ 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::runtimes::prepend_zsh_fork_bin_to_path;
|
||||
use crate::tools::sandboxing::PermissionRequestPayload;
|
||||
use crate::tools::sandboxing::SandboxAttempt;
|
||||
use crate::tools::sandboxing::ToolCtx;
|
||||
@@ -116,7 +117,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 mut env = exec_env_for_sandbox_permissions(&req.env, req.sandbox_permissions);
|
||||
prepend_zsh_fork_bin_to_path(&mut env, shell_zsh_path);
|
||||
let command =
|
||||
build_sandbox_command(command, &req.cwd, &env, req.additional_permissions.clone())?;
|
||||
let options = ExecOptions {
|
||||
|
||||
@@ -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,19 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
if let Some(network) = managed_network {
|
||||
network.apply_to_env(&mut env);
|
||||
}
|
||||
let explicit_env_overrides = req.explicit_env_overrides.clone();
|
||||
#[cfg(unix)]
|
||||
let explicit_env_overrides = {
|
||||
let mut explicit_env_overrides = explicit_env_overrides;
|
||||
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(),
|
||||
);
|
||||
}
|
||||
explicit_env_overrides
|
||||
};
|
||||
let environment_is_remote = req.environment.is_remote();
|
||||
let command = if environment_is_remote {
|
||||
base_command.to_vec()
|
||||
@@ -269,7 +284,7 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
base_command,
|
||||
session_shell.as_ref(),
|
||||
&req.cwd,
|
||||
&req.explicit_env_overrides,
|
||||
&explicit_env_overrides,
|
||||
&env,
|
||||
)
|
||||
};
|
||||
|
||||
@@ -46,6 +46,8 @@ use serde_json::Value;
|
||||
use serde_json::json;
|
||||
use std::env;
|
||||
use std::fs;
|
||||
#[cfg(unix)]
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
@@ -2528,6 +2530,166 @@ async fn spawned_subagent_execpolicy_amendment_propagates_to_parent_session() ->
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[cfg(unix)]
|
||||
async fn env_zsh_script_spawned_by_python_can_request_escalation_under_zsh_fork() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let Some(runtime) = zsh_fork_runtime("zsh-fork env zsh nested escalation test")? else {
|
||||
return Ok(());
|
||||
};
|
||||
|
||||
let approval_policy = AskForApproval::OnRequest;
|
||||
let permission_profile = restrictive_workspace_write_profile();
|
||||
let outside_dir = tempfile::tempdir_in(std::env::current_dir()?)?;
|
||||
let outside_path = outside_dir.path().join("zsh-fork-env-zsh-escalated.txt");
|
||||
let outside_path_arg = shlex::try_join([outside_path.to_string_lossy().as_ref()])?;
|
||||
let rules = r#"prefix_rule(pattern=["touch"], decision="prompt")"#.to_string();
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let outside_path_for_hook = outside_path.clone();
|
||||
let test = build_zsh_fork_test(
|
||||
&server,
|
||||
runtime,
|
||||
approval_policy,
|
||||
permission_profile.clone(),
|
||||
move |home| {
|
||||
let _ = fs::remove_file(&outside_path_for_hook);
|
||||
let rules_dir = home.join("rules");
|
||||
fs::create_dir_all(&rules_dir).unwrap();
|
||||
fs::write(rules_dir.join("default.rules"), &rules).unwrap();
|
||||
},
|
||||
)
|
||||
.await?;
|
||||
|
||||
let script_path = test.cwd.path().join("runs-under-env-zsh");
|
||||
fs::write(
|
||||
&script_path,
|
||||
format!(
|
||||
"#!/usr/bin/env zsh\ntouch {outside_path_arg}\nprint -r -- nested-env-zsh-complete\n"
|
||||
),
|
||||
)?;
|
||||
let mut script_permissions = fs::metadata(&script_path)?.permissions();
|
||||
script_permissions.set_mode(0o755);
|
||||
fs::set_permissions(&script_path, script_permissions)?;
|
||||
|
||||
let script_literal = serde_json::to_string(script_path.to_string_lossy().as_ref())?;
|
||||
let python_script = format!(
|
||||
"import subprocess; subprocess.run([{script_literal}], check=True, close_fds=False)"
|
||||
);
|
||||
let command = shlex::try_join(["python3", "-c", python_script.as_str()])?;
|
||||
|
||||
let call_id = "zsh-fork-env-zsh-nested-escalation";
|
||||
let event = shell_event(
|
||||
call_id,
|
||||
&command,
|
||||
/*timeout_ms*/ 30_000,
|
||||
SandboxPermissions::UseDefault,
|
||||
)?;
|
||||
let _ = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_response_created("resp-zsh-fork-env-zsh-1"),
|
||||
event,
|
||||
ev_completed("resp-zsh-fork-env-zsh-1"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
let results = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_assistant_message("msg-zsh-fork-env-zsh-1", "done"),
|
||||
ev_completed("resp-zsh-fork-env-zsh-2"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
|
||||
let session_model = test.session_configured.model.clone();
|
||||
let (sandbox_policy, permission_profile) =
|
||||
turn_permission_fields(permission_profile, test.cwd.path());
|
||||
test.codex
|
||||
.submit(Op::UserInput {
|
||||
items: vec![UserInput::Text {
|
||||
text: "run nested env zsh script through python".into(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
environments: None,
|
||||
final_output_json_schema: None,
|
||||
responsesapi_client_metadata: None,
|
||||
additional_context: Default::default(),
|
||||
thread_settings: codex_protocol::protocol::ThreadSettingsOverrides {
|
||||
cwd: Some(test.cwd.path().to_path_buf()),
|
||||
approval_policy: Some(approval_policy),
|
||||
approvals_reviewer: Some(ApprovalsReviewer::User),
|
||||
sandbox_policy: Some(sandbox_policy),
|
||||
permission_profile,
|
||||
collaboration_mode: Some(codex_protocol::config_types::CollaborationMode {
|
||||
mode: codex_protocol::config_types::ModeKind::Default,
|
||||
settings: codex_protocol::config_types::Settings {
|
||||
model: session_model,
|
||||
reasoning_effort: None,
|
||||
developer_instructions: None,
|
||||
},
|
||||
}),
|
||||
..Default::default()
|
||||
},
|
||||
})
|
||||
.await?;
|
||||
|
||||
let approval_event = wait_for_event_with_timeout(
|
||||
&test.codex,
|
||||
|event| {
|
||||
matches!(
|
||||
event,
|
||||
EventMsg::ExecApprovalRequest(_) | EventMsg::TurnComplete(_)
|
||||
)
|
||||
},
|
||||
Duration::from_secs(10),
|
||||
)
|
||||
.await;
|
||||
let EventMsg::ExecApprovalRequest(approval) = approval_event else {
|
||||
panic!("expected nested zsh script to request approval before completion");
|
||||
};
|
||||
assert!(
|
||||
approval.command.iter().any(|arg| arg.ends_with("/touch"))
|
||||
&& approval
|
||||
.command
|
||||
.iter()
|
||||
.any(|arg| arg == outside_path.to_string_lossy().as_ref()),
|
||||
"expected approval for nested touch command, got: {:?}",
|
||||
approval.command
|
||||
);
|
||||
|
||||
test.codex
|
||||
.submit(Op::ExecApproval {
|
||||
id: approval.effective_approval_id(),
|
||||
turn_id: None,
|
||||
decision: ReviewDecision::Approved,
|
||||
})
|
||||
.await?;
|
||||
|
||||
wait_for_completion(&test).await;
|
||||
|
||||
let result = parse_result(&results.single_request().function_call_output(call_id));
|
||||
assert_eq!(
|
||||
result.exit_code.unwrap_or(0),
|
||||
0,
|
||||
"nested env zsh script should complete successfully: {}",
|
||||
result.stdout
|
||||
);
|
||||
assert!(
|
||||
result.stdout.contains("nested-env-zsh-complete"),
|
||||
"nested script did not report completion: {}",
|
||||
result.stdout
|
||||
);
|
||||
assert!(
|
||||
outside_path.exists(),
|
||||
"approved nested touch should create the out-of-workspace file"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[cfg(unix)]
|
||||
async fn matched_prefix_rule_runs_unsandboxed_under_zsh_fork() -> Result<()> {
|
||||
|
||||
Reference in New Issue
Block a user