diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 91932b30f3..6b57b020f9 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2241,8 +2241,8 @@ dependencies = [ "anyhow", "async-trait", "clap", + "codex-utils-absolute-path", "libc", - "path-absolutize", "pretty_assertions", "serde", "serde_json", diff --git a/codex-rs/shell-escalation/Cargo.toml b/codex-rs/shell-escalation/Cargo.toml index f608b96083..85075cfb51 100644 --- a/codex-rs/shell-escalation/Cargo.toml +++ b/codex-rs/shell-escalation/Cargo.toml @@ -12,8 +12,8 @@ path = "src/bin/main_execve_wrapper.rs" anyhow = { workspace = true } async-trait = { workspace = true } clap = { workspace = true, features = ["derive"] } +codex-utils-absolute-path = { workspace = true } libc = { workspace = true } -path-absolutize = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } socket2 = { workspace = true, features = ["all"] } diff --git a/codex-rs/shell-escalation/src/unix/escalate_client.rs b/codex-rs/shell-escalation/src/unix/escalate_client.rs index 5c363f7d6a..f9c866df4e 100644 --- a/codex-rs/shell-escalation/src/unix/escalate_client.rs +++ b/codex-rs/shell-escalation/src/unix/escalate_client.rs @@ -4,6 +4,7 @@ use std::os::fd::FromRawFd as _; use std::os::fd::OwnedFd; use anyhow::Context as _; +use codex_utils_absolute_path::AbsolutePathBuf; use crate::unix::escalate_protocol::ESCALATE_SOCKET_ENV_VAR; use crate::unix::escalate_protocol::EXEC_WRAPPER_ENV_VAR; @@ -50,7 +51,7 @@ pub async fn run_shell_escalation_execve_wrapper( .send(EscalateRequest { file: file.clone().into(), argv: argv.clone(), - workdir: std::env::current_dir()?, + workdir: AbsolutePathBuf::current_dir()?, env, }) .await diff --git a/codex-rs/shell-escalation/src/unix/escalate_protocol.rs b/codex-rs/shell-escalation/src/unix/escalate_protocol.rs index 6be00fe4b2..6002fdc57b 100644 --- a/codex-rs/shell-escalation/src/unix/escalate_protocol.rs +++ b/codex-rs/shell-escalation/src/unix/escalate_protocol.rs @@ -2,6 +2,7 @@ use std::collections::HashMap; use std::os::fd::RawFd; use std::path::PathBuf; +use codex_utils_absolute_path::AbsolutePathBuf; use serde::Deserialize; use serde::Serialize; @@ -17,11 +18,14 @@ pub const LEGACY_BASH_EXEC_WRAPPER_ENV_VAR: &str = "BASH_EXEC_WRAPPER"; /// The client sends this to the server to request an exec() call. #[derive(Clone, Serialize, Deserialize, Debug, PartialEq, Eq)] pub struct EscalateRequest { - /// The absolute path to the executable to run, i.e. the first arg to exec. + /// The executable path from the intercepted exec call. + /// + /// This may be relative, in which case it should be resolved against + /// `workdir`. pub file: PathBuf, /// The argv, including the program name (argv[0]). pub argv: Vec, - pub workdir: PathBuf, + pub workdir: AbsolutePathBuf, pub env: HashMap, } diff --git a/codex-rs/shell-escalation/src/unix/escalate_server.rs b/codex-rs/shell-escalation/src/unix/escalate_server.rs index 56db822ebb..0d25a9081e 100644 --- a/codex-rs/shell-escalation/src/unix/escalate_server.rs +++ b/codex-rs/shell-escalation/src/unix/escalate_server.rs @@ -6,7 +6,7 @@ use std::sync::Arc; use std::time::Duration; use anyhow::Context as _; -use path_absolutize::Absolutize as _; +use codex_utils_absolute_path::AbsolutePathBuf; use tokio::process::Command; use tokio_util::sync::CancellationToken; @@ -114,8 +114,9 @@ impl EscalateServer { }, params.command, ]; + let workdir = AbsolutePathBuf::try_from(params.workdir)?; let result = command_executor - .run(command, PathBuf::from(¶ms.workdir), env, cancel_rx) + .run(command, workdir.to_path_buf(), env, cancel_rx) .await?; escalate_task.abort(); Ok(result) @@ -152,10 +153,9 @@ async fn handle_escalate_session_with_policy( workdir, env, } = socket.receive::().await?; - let file = PathBuf::from(&file).absolutize()?.into_owned(); - let workdir = PathBuf::from(&workdir).absolutize()?.into_owned(); + let file = AbsolutePathBuf::resolve_path_against_base(file, workdir.as_path())?; let action = policy - .determine_action(file.as_path(), &argv, &workdir) + .determine_action(file.as_path(), &argv, workdir.as_path()) .await .context("failed to determine escalation action")?; @@ -197,7 +197,7 @@ async fn handle_escalate_session_with_policy( )); } - let mut command = Command::new(file); + let mut command = Command::new(file.as_path()); command .args(&argv[1..]) .arg0(argv[0].clone()) @@ -236,6 +236,7 @@ async fn handle_escalate_session_with_policy( #[cfg(test)] mod tests { use super::*; + use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use std::collections::HashMap; use std::path::Path; @@ -257,6 +258,25 @@ mod tests { } } + struct AssertingEscalationPolicy { + expected_file: AbsolutePathBuf, + expected_workdir: AbsolutePathBuf, + } + + #[async_trait::async_trait] + impl EscalationPolicy for AssertingEscalationPolicy { + async fn determine_action( + &self, + file: &Path, + _argv: &[String], + workdir: &Path, + ) -> anyhow::Result { + assert_eq!(file, self.expected_file.as_path()); + assert_eq!(workdir, self.expected_workdir.as_path()); + Ok(EscalateAction::Run) + } + } + #[tokio::test] async fn handle_escalate_session_respects_run_in_sandbox_decision() -> anyhow::Result<()> { let (server, client) = AsyncSocket::pair()?; @@ -277,7 +297,7 @@ mod tests { .send(EscalateRequest { file: PathBuf::from("/bin/echo"), argv: vec!["echo".to_string()], - workdir: PathBuf::from("/tmp"), + workdir: AbsolutePathBuf::try_from(PathBuf::from("/tmp"))?, env, }) .await?; @@ -292,6 +312,42 @@ mod tests { server_task.await? } + #[tokio::test] + async fn handle_escalate_session_resolves_relative_file_against_request_workdir() + -> anyhow::Result<()> { + let (server, client) = AsyncSocket::pair()?; + let tmp = tempfile::TempDir::new()?; + let workdir = tmp.path().join("workspace"); + std::fs::create_dir(&workdir)?; + let workdir = AbsolutePathBuf::try_from(workdir)?; + let expected_file = workdir.join("bin/tool")?; + let server_task = tokio::spawn(handle_escalate_session_with_policy( + server, + Arc::new(AssertingEscalationPolicy { + expected_file, + expected_workdir: workdir.clone(), + }), + )); + + client + .send(EscalateRequest { + file: PathBuf::from("./bin/tool"), + argv: vec!["./bin/tool".to_string()], + workdir, + env: HashMap::new(), + }) + .await?; + + let response = client.receive::().await?; + assert_eq!( + EscalateResponse { + action: EscalateAction::Run, + }, + response + ); + server_task.await? + } + #[tokio::test] async fn handle_escalate_session_executes_escalated_command() -> anyhow::Result<()> { let (server, client) = AsyncSocket::pair()?; @@ -310,7 +366,7 @@ mod tests { "-c".to_string(), r#"if [ "$KEY" = VALUE ]; then exit 42; else exit 1; fi"#.to_string(), ], - workdir: std::env::current_dir()?, + workdir: AbsolutePathBuf::current_dir()?, env: HashMap::from([("KEY".to_string(), "VALUE".to_string())]), }) .await?;