mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
feat: pass helper executable paths via Arg0DispatchPaths (#12719)
## Why `codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs` previously located `codex-execve-wrapper` by scanning `PATH` and sibling directories. That lookup is brittle and can select the wrong binary when the runtime environment differs from startup assumptions. We already pass `codex-linux-sandbox` from `codex-arg0`; `codex-execve-wrapper` should use the same startup-driven path plumbing. ## What changed - Introduced `Arg0DispatchPaths` in `codex-arg0` to carry both helper executable paths: - `codex_linux_sandbox_exe` - `main_execve_wrapper_exe` - Updated `arg0_dispatch_or_else()` to pass `Arg0DispatchPaths` to top-level binaries and preserve helper paths created in `prepend_path_entry_for_codex_aliases()`. - Threaded `Arg0DispatchPaths` through entrypoints in `cli`, `exec`, `tui`, `app-server`, and `mcp-server`. - Added `main_execve_wrapper_exe` to core configuration plumbing (`Config`, `ConfigOverrides`, and `SessionServices`). - Updated zsh-fork shell escalation to consume the configured `main_execve_wrapper_exe` and removed path-sniffing fallback logic. - Updated app-server config reload paths so reloaded configs keep the same startup-provided helper executable paths. ## References - [`Arg0DispatchPaths` definition](https://github.com/openai/codex/blob/e355b43d5c2a771f045296a6deae10d7c9c36ec6/codex-rs/arg0/src/lib.rs#L20-L24) - [`arg0_dispatch_or_else()` forwarding both paths](https://github.com/openai/codex/blob/e355b43d5c2a771f045296a6deae10d7c9c36ec6/codex-rs/arg0/src/lib.rs#L145-L176) - [zsh-fork escalation using configured wrapper path](https://github.com/openai/codex/blob/e355b43d5c2a771f045296a6deae10d7c9c36ec6/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs#L109-L150) ## Testing - `cargo check -p codex-arg0 -p codex-core -p codex-exec -p codex-tui -p codex-mcp-server -p codex-app-server` - `cargo test -p codex-arg0` - `cargo test -p codex-core tools::runtimes::shell::unix_escalation:: -- --nocapture`
This commit is contained in:
@@ -87,6 +87,7 @@ pub(crate) async fn apply_role_to_config(
|
||||
ConfigOverrides {
|
||||
cwd: Some(config.cwd.clone()),
|
||||
codex_linux_sandbox_exe: config.codex_linux_sandbox_exe.clone(),
|
||||
main_execve_wrapper_exe: config.main_execve_wrapper_exe.clone(),
|
||||
js_repl_node_path: config.js_repl_node_path.clone(),
|
||||
..Default::default()
|
||||
},
|
||||
@@ -340,6 +341,8 @@ mod tests {
|
||||
TomlValue::String("base-model".to_string()),
|
||||
)])
|
||||
.await;
|
||||
config.codex_linux_sandbox_exe = Some(PathBuf::from("/tmp/codex-linux-sandbox"));
|
||||
config.main_execve_wrapper_exe = Some(PathBuf::from("/tmp/codex-execve-wrapper"));
|
||||
let role_path = write_role_config(
|
||||
&home,
|
||||
"effort-only.toml",
|
||||
@@ -360,6 +363,14 @@ mod tests {
|
||||
|
||||
assert_eq!(config.model.as_deref(), Some("base-model"));
|
||||
assert_eq!(config.model_reasoning_effort, Some(ReasoningEffort::High));
|
||||
assert_eq!(
|
||||
config.codex_linux_sandbox_exe,
|
||||
Some(PathBuf::from("/tmp/codex-linux-sandbox"))
|
||||
);
|
||||
assert_eq!(
|
||||
config.main_execve_wrapper_exe,
|
||||
Some(PathBuf::from("/tmp/codex-execve-wrapper"))
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
@@ -1331,6 +1331,7 @@ impl Session {
|
||||
config.background_terminal_max_timeout,
|
||||
),
|
||||
shell_zsh_path: config.zsh_path.clone(),
|
||||
main_execve_wrapper_exe: config.main_execve_wrapper_exe.clone(),
|
||||
analytics_events_client: AnalyticsEventsClient::new(
|
||||
Arc::clone(&config),
|
||||
Arc::clone(&auth_manager),
|
||||
@@ -8201,6 +8202,7 @@ mod tests {
|
||||
config.background_terminal_max_timeout,
|
||||
),
|
||||
shell_zsh_path: None,
|
||||
main_execve_wrapper_exe: config.main_execve_wrapper_exe.clone(),
|
||||
analytics_events_client: AnalyticsEventsClient::new(
|
||||
Arc::clone(&config),
|
||||
Arc::clone(&auth_manager),
|
||||
@@ -8356,6 +8358,7 @@ mod tests {
|
||||
config.background_terminal_max_timeout,
|
||||
),
|
||||
shell_zsh_path: None,
|
||||
main_execve_wrapper_exe: config.main_execve_wrapper_exe.clone(),
|
||||
analytics_events_client: AnalyticsEventsClient::new(
|
||||
Arc::clone(&config),
|
||||
Arc::clone(&auth_manager),
|
||||
|
||||
@@ -399,6 +399,11 @@ pub struct Config {
|
||||
/// When this program is invoked, arg0 will be set to `codex-linux-sandbox`.
|
||||
pub codex_linux_sandbox_exe: Option<PathBuf>,
|
||||
|
||||
/// Path to the `codex-execve-wrapper` executable used for shell
|
||||
/// escalation. This cannot be set in the config file: it must be set in
|
||||
/// code via [`ConfigOverrides`].
|
||||
pub main_execve_wrapper_exe: Option<PathBuf>,
|
||||
|
||||
/// Optional absolute path to the Node runtime used by `js_repl`.
|
||||
pub js_repl_node_path: Option<PathBuf>,
|
||||
|
||||
@@ -646,7 +651,8 @@ impl Config {
|
||||
/// designed to use [AskForApproval::Never] exclusively.
|
||||
///
|
||||
/// Further, [ConfigOverrides] contains some options that are not supported
|
||||
/// in [ConfigToml], such as `cwd` and `codex_linux_sandbox_exe`.
|
||||
/// in [ConfigToml], such as `cwd`, `codex_linux_sandbox_exe`, and
|
||||
/// `main_execve_wrapper_exe`.
|
||||
pub async fn load_with_cli_overrides_and_harness_overrides(
|
||||
cli_overrides: Vec<(String, TomlValue)>,
|
||||
harness_overrides: ConfigOverrides,
|
||||
@@ -1536,6 +1542,7 @@ pub struct ConfigOverrides {
|
||||
pub model_provider: Option<String>,
|
||||
pub config_profile: Option<String>,
|
||||
pub codex_linux_sandbox_exe: Option<PathBuf>,
|
||||
pub main_execve_wrapper_exe: Option<PathBuf>,
|
||||
pub js_repl_node_path: Option<PathBuf>,
|
||||
pub js_repl_node_module_dirs: Option<Vec<PathBuf>>,
|
||||
pub zsh_path: Option<PathBuf>,
|
||||
@@ -1665,6 +1672,7 @@ impl Config {
|
||||
model_provider,
|
||||
config_profile: config_profile_key,
|
||||
codex_linux_sandbox_exe,
|
||||
main_execve_wrapper_exe,
|
||||
js_repl_node_path: js_repl_node_path_override,
|
||||
js_repl_node_module_dirs: js_repl_node_module_dirs_override,
|
||||
zsh_path: zsh_path_override,
|
||||
@@ -2151,6 +2159,7 @@ impl Config {
|
||||
ephemeral: ephemeral.unwrap_or_default(),
|
||||
file_opener: cfg.file_opener.unwrap_or(UriBasedFileOpener::VsCode),
|
||||
codex_linux_sandbox_exe,
|
||||
main_execve_wrapper_exe,
|
||||
js_repl_node_path,
|
||||
js_repl_node_module_dirs,
|
||||
zsh_path,
|
||||
@@ -4765,6 +4774,7 @@ model_verbosity = "high"
|
||||
ephemeral: false,
|
||||
file_opener: UriBasedFileOpener::VsCode,
|
||||
codex_linux_sandbox_exe: None,
|
||||
main_execve_wrapper_exe: None,
|
||||
js_repl_node_path: None,
|
||||
js_repl_node_module_dirs: Vec::new(),
|
||||
zsh_path: None,
|
||||
@@ -4891,6 +4901,7 @@ model_verbosity = "high"
|
||||
ephemeral: false,
|
||||
file_opener: UriBasedFileOpener::VsCode,
|
||||
codex_linux_sandbox_exe: None,
|
||||
main_execve_wrapper_exe: None,
|
||||
js_repl_node_path: None,
|
||||
js_repl_node_module_dirs: Vec::new(),
|
||||
zsh_path: None,
|
||||
@@ -5015,6 +5026,7 @@ model_verbosity = "high"
|
||||
ephemeral: false,
|
||||
file_opener: UriBasedFileOpener::VsCode,
|
||||
codex_linux_sandbox_exe: None,
|
||||
main_execve_wrapper_exe: None,
|
||||
js_repl_node_path: None,
|
||||
js_repl_node_module_dirs: Vec::new(),
|
||||
zsh_path: None,
|
||||
@@ -5125,6 +5137,7 @@ model_verbosity = "high"
|
||||
ephemeral: false,
|
||||
file_opener: UriBasedFileOpener::VsCode,
|
||||
codex_linux_sandbox_exe: None,
|
||||
main_execve_wrapper_exe: None,
|
||||
js_repl_node_path: None,
|
||||
js_repl_node_module_dirs: Vec::new(),
|
||||
zsh_path: None,
|
||||
|
||||
@@ -29,6 +29,8 @@ pub(crate) struct SessionServices {
|
||||
pub(crate) unified_exec_manager: UnifiedExecProcessManager,
|
||||
#[cfg_attr(not(unix), allow(dead_code))]
|
||||
pub(crate) shell_zsh_path: Option<PathBuf>,
|
||||
#[cfg_attr(not(unix), allow(dead_code))]
|
||||
pub(crate) main_execve_wrapper_exe: Option<PathBuf>,
|
||||
pub(crate) analytics_events_client: AnalyticsEventsClient,
|
||||
pub(crate) hooks: Hooks,
|
||||
pub(crate) rollout: Mutex<Option<RolloutRecorder>>,
|
||||
|
||||
@@ -106,15 +106,22 @@ pub(super) async fn try_run_zsh_fork(
|
||||
justification,
|
||||
arg0,
|
||||
};
|
||||
|
||||
let main_execve_wrapper_exe = ctx
|
||||
.session
|
||||
.services
|
||||
.main_execve_wrapper_exe
|
||||
.clone()
|
||||
.ok_or_else(|| {
|
||||
ToolError::Rejected(
|
||||
"zsh fork feature enabled, but execve wrapper is not configured".to_string(),
|
||||
)
|
||||
})?;
|
||||
let exec_params = ExecParams {
|
||||
command: script,
|
||||
workdir: req.cwd.to_string_lossy().to_string(),
|
||||
timeout_ms: Some(effective_timeout.as_millis() as u64),
|
||||
login: Some(login),
|
||||
};
|
||||
let execve_wrapper =
|
||||
shell_execve_wrapper().map_err(|err| ToolError::Rejected(format!("{err}")))?;
|
||||
|
||||
// Note that Stopwatch starts immediately upon creation, so currently we try
|
||||
// to minimize the time between creating the Stopwatch and starting the
|
||||
@@ -132,8 +139,11 @@ pub(super) async fn try_run_zsh_fork(
|
||||
stopwatch: stopwatch.clone(),
|
||||
};
|
||||
|
||||
let escalate_server =
|
||||
EscalateServer::new(shell_zsh_path.clone(), execve_wrapper, escalation_policy);
|
||||
let escalate_server = EscalateServer::new(
|
||||
shell_zsh_path.clone(),
|
||||
main_execve_wrapper_exe,
|
||||
escalation_policy,
|
||||
);
|
||||
|
||||
let exec_result = escalate_server
|
||||
.exec(exec_params, cancel_token, &command_executor)
|
||||
@@ -342,34 +352,6 @@ impl ShellCommandExecutor for CoreShellCommandExecutor {
|
||||
}
|
||||
}
|
||||
|
||||
// TODO(mbolin): This should be passed down from codex-arg0 like codex_linux_sandbox_exe.
|
||||
fn shell_execve_wrapper() -> anyhow::Result<PathBuf> {
|
||||
const EXECVE_WRAPPER: &str = "codex-execve-wrapper";
|
||||
|
||||
if let Some(path) = std::env::var_os("PATH") {
|
||||
for dir in std::env::split_paths(&path) {
|
||||
let candidate = dir.join(EXECVE_WRAPPER);
|
||||
if candidate.is_file() {
|
||||
return Ok(candidate);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
let exe = std::env::current_exe()?;
|
||||
let sibling = exe
|
||||
.parent()
|
||||
.map(|parent| parent.join(EXECVE_WRAPPER))
|
||||
.ok_or_else(|| anyhow::anyhow!("failed to determine codex-execve-wrapper path"))?;
|
||||
if sibling.is_file() {
|
||||
return Ok(sibling);
|
||||
}
|
||||
|
||||
Err(anyhow::anyhow!(
|
||||
"failed to locate {EXECVE_WRAPPER} in PATH or next to current executable ({})",
|
||||
exe.display()
|
||||
))
|
||||
}
|
||||
|
||||
#[derive(Debug, Eq, PartialEq)]
|
||||
struct ParsedShellCommand {
|
||||
script: String,
|
||||
|
||||
Reference in New Issue
Block a user