fix: prepare ExecPolicy in exec-server for execpolicy2 cutover (#6888)

This PR introduces an extra layer of abstraction to prepare us for the
migration to execpolicy2:

- introduces a new trait, `EscalationPolicy`, whose `determine_action()`
method is responsible for producing the `EscalateAction`
- the existing `ExecPolicy` typedef is changed to return an intermediate
`ExecPolicyOutcome` instead of `EscalateAction`
- the default implementation of `EscalationPolicy`,
`McpEscalationPolicy`, composes `ExecPolicy`
- the `ExecPolicyOutcome` includes `codex_execpolicy2::Decision`, which
has a `Prompt` variant
- when `McpEscalationPolicy` gets `Decision::Prompt` back from
`ExecPolicy`, it prompts the user via an MCP elicitation and maps the
result into an `ElicitationAction`
- now that the end user can reply to an elicitation with `Decline` or
`Cancel`, we introduce a new variant, `EscalateAction::Deny`, which the
client handles by returning exit code `1` without running anything

Note the way the elicitation is created is still not quite right, but I
will fix that once we have things running end-to-end for real in a
follow-up PR.

Author: bolinfest

codex-rs/Cargo.lock

@@ -12,6 +12,7 @@ workspace = true
 
 [dependencies]
 anyhow = { workspace = true }
+async-trait = { workspace = true }
 clap = { workspace = true, features = ["derive"] }
 codex-core = { workspace = true }
 libc = { workspace = true }
@@ -31,6 +32,7 @@ rmcp = { workspace = true, default-features = false, features = [
 ] }
 serde = { workspace = true, features = ["derive"] }
 serde_json = { workspace = true }
+shlex = { workspace = true }
 socket2 = { workspace = true }
 tokio = { workspace = true, features = [
     "io-std",

codex-rs/exec-server/Cargo.toml

@@ -64,25 +64,17 @@ use tracing_subscriber::{self};
 
 use crate::posix::escalate_protocol::EscalateAction;
 use crate::posix::escalate_server::EscalateServer;
+use crate::posix::escalation_policy::EscalationPolicy;
+use crate::posix::mcp_escalation_policy::ExecPolicyOutcome;
 
 mod escalate_client;
 mod escalate_protocol;
 mod escalate_server;
+mod escalation_policy;
 mod mcp;
+mod mcp_escalation_policy;
 mod socket;
 
-fn dummy_exec_policy(file: &Path, argv: &[String], _workdir: &Path) -> EscalateAction {
-    // TODO: execpolicy
-    if file == Path::new("/opt/homebrew/bin/gh")
-        && let [_, arg1, arg2, ..] = argv
-        && arg1 == "issue"
-        && arg2 == "list"
-    {
-        return EscalateAction::Escalate;
-    }
-    EscalateAction::Run
-}
-
 #[derive(Parser)]
 #[command(version)]
 pub struct Cli {
@@ -135,7 +127,7 @@ pub async fn main() -> anyhow::Result<()> {
         }
         Some(Commands::ShellExec(args)) => {
             let bash_path = mcp::get_bash_path()?;
-            let escalate_server = EscalateServer::new(bash_path, dummy_exec_policy);
+            let escalate_server = EscalateServer::new(bash_path, DummyEscalationPolicy {});
             let result = escalate_server
                 .exec(
                     args.command.clone(),
@@ -162,3 +154,59 @@ pub async fn main() -> anyhow::Result<()> {
         }
     }
 }
+
+// TODO: replace with execpolicy2
+
+struct DummyEscalationPolicy;
+
+#[async_trait::async_trait]
+impl EscalationPolicy for DummyEscalationPolicy {
+    async fn determine_action(
+        &self,
+        file: &Path,
+        argv: &[String],
+        workdir: &Path,
+    ) -> Result<EscalateAction, rmcp::ErrorData> {
+        let outcome = dummy_exec_policy(file, argv, workdir);
+        let action = match outcome {
+            ExecPolicyOutcome::Allow {
+                run_with_escalated_permissions,
+            } => {
+                if run_with_escalated_permissions {
+                    EscalateAction::Escalate
+                } else {
+                    EscalateAction::Run
+                }
+            }
+            ExecPolicyOutcome::Forbidden => EscalateAction::Deny {
+                reason: Some("Execution forbidden by policy".to_string()),
+            },
+            ExecPolicyOutcome::Prompt { .. } => EscalateAction::Deny {
+                reason: Some("Could not prompt user for permission".to_string()),
+            },
+        };
+        Ok(action)
+    }
+}
+
+fn dummy_exec_policy(file: &Path, argv: &[String], _workdir: &Path) -> ExecPolicyOutcome {
+    if file.ends_with("/rm") {
+        ExecPolicyOutcome::Forbidden
+    } else if file.ends_with("/git") {
+        ExecPolicyOutcome::Prompt {
+            run_with_escalated_permissions: false,
+        }
+    } else if file == Path::new("/opt/homebrew/bin/gh")
+        && let [_, arg1, arg2, ..] = argv
+        && arg1 == "issue"
+        && arg2 == "list"
+    {
+        ExecPolicyOutcome::Allow {
+            run_with_escalated_permissions: true,
+        }
+    } else {
+        ExecPolicyOutcome::Allow {
+            run_with_escalated_permissions: false,
+        }
+    }
+}

codex-rs/exec-server/src/posix.rs

@@ -98,5 +98,12 @@ pub(crate) async fn run(file: String, argv: Vec<String>) -> anyhow::Result<i32>
 
             Err(err.into())
         }
+        EscalateAction::Deny { reason } => {
+            match reason {
+                Some(reason) => eprintln!("Execution denied: {reason}"),
+                None => eprintln!("Execution denied"),
+            }
+            Ok(1)
+        }
     }
 }

codex-rs/exec-server/src/posix/escalate_client.rs

@@ -34,6 +34,8 @@ pub(super) enum EscalateAction {
     Run,
     /// The command should be escalated to the server for execution.
     Escalate,
+    /// The command should not be executed.
+    Deny { reason: Option<String> },
 }
 
 /// The client sends this to the server to forward its open FDs.

codex-rs/exec-server/src/posix/escalate_protocol.rs

@@ -1,8 +1,8 @@
 use std::collections::HashMap;
 use std::os::fd::AsRawFd;
-use std::path::Path;
 use std::path::PathBuf;
 use std::process::Stdio;
+use std::sync::Arc;
 use std::time::Duration;
 
 use anyhow::Context as _;
@@ -21,25 +21,24 @@ use crate::posix::escalate_protocol::EscalateRequest;
 use crate::posix::escalate_protocol::EscalateResponse;
 use crate::posix::escalate_protocol::SuperExecMessage;
 use crate::posix::escalate_protocol::SuperExecResult;
+use crate::posix::escalation_policy::EscalationPolicy;
 use crate::posix::socket::AsyncDatagramSocket;
 use crate::posix::socket::AsyncSocket;
 
-/// This is the policy which decides how to handle an exec() call.
-///
-/// `file` is the absolute, canonical path to the executable to run, i.e. the first arg to exec.
-/// `argv` is the argv, including the program name (`argv[0]`).
-/// `workdir` is the absolute, canonical path to the working directory in which to execute the
-/// command.
-pub(crate) type ExecPolicy = fn(file: &Path, argv: &[String], workdir: &Path) -> EscalateAction;
-
 pub(crate) struct EscalateServer {
     bash_path: PathBuf,
-    policy: ExecPolicy,
+    policy: Arc<dyn EscalationPolicy>,
 }
 
 impl EscalateServer {
-    pub fn new(bash_path: PathBuf, policy: ExecPolicy) -> Self {
-        Self { bash_path, policy }
+    pub fn new<P>(bash_path: PathBuf, policy: P) -> Self
+    where
+        P: EscalationPolicy + Send + Sync + 'static,
+    {
+        Self {
+            bash_path,
+            policy: Arc::new(policy),
+        }
     }
 
     pub async fn exec(
@@ -53,7 +52,7 @@ impl EscalateServer {
         let client_socket = escalate_client.into_inner();
         client_socket.set_cloexec(false)?;
 
-        let escalate_task = tokio::spawn(escalate_task(escalate_server, self.policy));
+        let escalate_task = tokio::spawn(escalate_task(escalate_server, self.policy.clone()));
         let mut env = env.clone();
         env.insert(
             ESCALATE_SOCKET_ENV_VAR.to_string(),
@@ -96,14 +95,18 @@ impl EscalateServer {
     }
 }
 
-async fn escalate_task(socket: AsyncDatagramSocket, policy: ExecPolicy) -> anyhow::Result<()> {
+async fn escalate_task(
+    socket: AsyncDatagramSocket,
+    policy: Arc<dyn EscalationPolicy>,
+) -> anyhow::Result<()> {
     loop {
         let (_, mut fds) = socket.receive_with_fds().await?;
         if fds.len() != 1 {
             tracing::error!("expected 1 fd in datagram handshake, got {}", fds.len());
             continue;
         }
         let stream_socket = AsyncSocket::from_fd(fds.remove(0))?;
+        let policy = policy.clone();
         tokio::spawn(async move {
             if let Err(err) = handle_escalate_session_with_policy(stream_socket, policy).await {
                 tracing::error!("escalate session failed: {err:?}");
@@ -122,7 +125,7 @@ pub(crate) struct ExecResult {
 
 async fn handle_escalate_session_with_policy(
     socket: AsyncSocket,
-    policy: ExecPolicy,
+    policy: Arc<dyn EscalationPolicy>,
 ) -> anyhow::Result<()> {
     let EscalateRequest {
         file,
@@ -132,8 +135,12 @@ async fn handle_escalate_session_with_policy(
     } = socket.receive::<EscalateRequest>().await?;
     let file = PathBuf::from(&file).absolutize()?.into_owned();
     let workdir = PathBuf::from(&workdir).absolutize()?.into_owned();
-    let action = policy(file.as_path(), &argv, &workdir);
+    let action = policy
+        .determine_action(file.as_path(), &argv, &workdir)
+        .await?;
+
     tracing::debug!("decided {action:?} for {file:?} {argv:?} {workdir:?}");
+
     match action {
         EscalateAction::Run => {
             socket
@@ -195,6 +202,13 @@ async fn handle_escalate_session_with_policy(
                 })
                 .await?;
         }
+        EscalateAction::Deny { reason } => {
+            socket
+                .send(EscalateResponse {
+                    action: EscalateAction::Deny { reason },
+                })
+                .await?;
+        }
     }
     Ok(())
 }
@@ -204,14 +218,33 @@ mod tests {
     use super::*;
     use pretty_assertions::assert_eq;
     use std::collections::HashMap;
+    use std::path::Path;
     use std::path::PathBuf;
 
+    struct DeterministicEscalationPolicy {
+        action: EscalateAction,
+    }
+
+    #[async_trait::async_trait]
+    impl EscalationPolicy for DeterministicEscalationPolicy {
+        async fn determine_action(
+            &self,
+            _file: &Path,
+            _argv: &[String],
+            _workdir: &Path,
+        ) -> Result<EscalateAction, rmcp::ErrorData> {
+            Ok(self.action.clone())
+        }
+    }
+
     #[tokio::test]
     async fn handle_escalate_session_respects_run_in_sandbox_decision() -> anyhow::Result<()> {
         let (server, client) = AsyncSocket::pair()?;
         let server_task = tokio::spawn(handle_escalate_session_with_policy(
             server,
-            |_file, _argv, _workdir| EscalateAction::Run,
+            Arc::new(DeterministicEscalationPolicy {
+                action: EscalateAction::Run,
+            }),
         ));
 
         client
@@ -238,7 +271,9 @@ mod tests {
         let (server, client) = AsyncSocket::pair()?;
         let server_task = tokio::spawn(handle_escalate_session_with_policy(
             server,
-            |_file, _argv, _workdir| EscalateAction::Escalate,
+            Arc::new(DeterministicEscalationPolicy {
+                action: EscalateAction::Escalate,
+            }),
         ));
 
         client

codex-rs/exec-server/src/posix/escalate_server.rs

@@ -0,0 +1,14 @@
+use std::path::Path;
+
+use crate::posix::escalate_protocol::EscalateAction;
+
+/// Decides what action to take in response to an execve request from a client.
+#[async_trait::async_trait]
+pub(crate) trait EscalationPolicy: Send + Sync {
+    async fn determine_action(
+        &self,
+        file: &Path,
+        argv: &[String],
+        workdir: &Path,
+    ) -> Result<EscalateAction, rmcp::ErrorData>;
+}

codex-rs/exec-server/src/posix/escalation_policy.rs

@@ -18,9 +18,10 @@ use rmcp::tool_handler;
 use rmcp::tool_router;
 use rmcp::transport::stdio;
 
-use crate::posix::escalate_server;
 use crate::posix::escalate_server::EscalateServer;
-use crate::posix::escalate_server::ExecPolicy;
+use crate::posix::escalate_server::{self};
+use crate::posix::mcp_escalation_policy::ExecPolicy;
+use crate::posix::mcp_escalation_policy::McpEscalationPolicy;
 
 /// Path to our patched bash.
 const CODEX_BASH_PATH_ENV_VAR: &str = "CODEX_BASH_PATH";
@@ -81,10 +82,13 @@ impl ExecTool {
     #[tool]
     async fn shell(
         &self,
-        _context: RequestContext<RoleServer>,
+        context: RequestContext<RoleServer>,
         Parameters(params): Parameters<ExecParams>,
     ) -> Result<CallToolResult, McpError> {
-        let escalate_server = EscalateServer::new(self.bash_path.clone(), self.policy);
+        let escalate_server = EscalateServer::new(
+            self.bash_path.clone(),
+            McpEscalationPolicy::new(self.policy, context),
+        );
         let result = escalate_server
             .exec(
                 params.command,
@@ -99,27 +103,6 @@ impl ExecTool {
             ExecResult::from(result),
         )?]))
     }
-
-    #[allow(dead_code)]
-    async fn prompt(
-        &self,
-        command: String,
-        workdir: String,
-        context: RequestContext<RoleServer>,
-    ) -> Result<CreateElicitationResult, McpError> {
-        context
-            .peer
-            .create_elicitation(CreateElicitationRequestParam {
-                message: format!("Allow Codex to run `{command:?}` in `{workdir:?}`?"),
-                #[allow(clippy::expect_used)]
-                requested_schema: ElicitationSchema::builder()
-                    .property("dummy", PrimitiveSchema::String(StringSchema::new()))
-                    .build()
-                    .expect("failed to build elicitation schema"),
-            })
-            .await
-            .map_err(|e| McpError::internal_error(e.to_string(), None))
-    }
 }
 
 #[tool_handler]