fix: found subtle bug in path resolution of execve program arg in codex-shell-escalation

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",

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"] }

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

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<String>,
-    pub workdir: PathBuf,
+    pub workdir: AbsolutePathBuf,
     pub env: HashMap<String, String>,
 }
 

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(&params.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::<EscalateRequest>().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<EscalateAction> {
+            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::<EscalateResponse>().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?;