feat: exec policy integration in shell mcp

zhao-oai · zhao-oai · commit e6b51aa3e6c0 · 2025-12-04T22:10:40.000Z
diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock
diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs
@@ -73,7 +73,7 @@ pub enum ExecPolicyUpdateError {
     FeatureDisabled,
 }
 
-pub(crate) async fn exec_policy_for(
+pub async fn exec_policy_for(
     features: &Features,
     codex_home: &Path,
 ) -> Result<Arc<RwLock<Policy>>, ExecPolicyError> {
diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs
@@ -97,7 +97,10 @@ mod user_shell_command;
 pub mod util;
 
 pub use apply_patch::CODEX_APPLY_PATCH_ARG1;
+pub use command_safety::is_dangerous_command;
 pub use command_safety::is_safe_command;
+pub use exec_policy::ExecPolicyError;
+pub use exec_policy::exec_policy_for;
 pub use safety::get_platform_sandbox;
 pub use safety::set_windows_sandbox_enabled;
 // Re-export the protocol types from the standalone `codex-protocol` crate so existing
diff --git a/codex-rs/exec-server/Cargo.toml b/codex-rs/exec-server/Cargo.toml
@@ -24,6 +24,7 @@ anyhow = { workspace = true }
 async-trait = { workspace = true }
 clap = { workspace = true, features = ["derive"] }
 codex-core = { workspace = true }
+codex-execpolicy = { workspace = true }
 libc = { workspace = true }
 path-absolutize = { workspace = true }
 rmcp = { workspace = true, default-features = false, features = [
diff --git a/codex-rs/exec-server/src/posix.rs b/codex-rs/exec-server/src/posix.rs
@@ -57,8 +57,20 @@
 //!
 use std::path::Path;
 use std::path::PathBuf;
+use std::sync::Arc;
 
+use anyhow::Context as _;
 use clap::Parser;
+use codex_core::ExecPolicyError;
+use codex_core::bash::parse_shell_lc_plain_commands;
+use codex_core::config::find_codex_home;
+use codex_core::exec_policy_for;
+use codex_core::features::Features;
+use codex_core::is_dangerous_command::command_might_be_dangerous;
+use codex_execpolicy::Decision;
+use codex_execpolicy::Policy;
+use codex_execpolicy::RuleMatch;
+use tracing::debug;
 use tracing_subscriber::EnvFilter;
 use tracing_subscriber::{self};
 
@@ -76,6 +88,7 @@ mod stopwatch;
 /// Default value of --execve option relative to the current executable.
 /// Note this must match the name of the binary as specified in Cargo.toml.
 const CODEX_EXECVE_WRAPPER_EXE_NAME: &str = "codex-execve-wrapper";
+const POLICY_DIR_NAME: &str = "policy";
 
 #[derive(Parser)]
 #[clap(version)]
@@ -87,6 +100,10 @@ struct McpServerCli {
     /// Path to Bash that has been patched to support execve() wrapping.
     #[arg(long = "bash")]
     bash_path: Option<PathBuf>,
+
+    /// Strip program paths before applying execpolicy (e.g., /usr/bin/echo -> echo).
+    #[arg(long, default_value_t = true)]
+    strip_program_paths: bool,
 }
 
 #[tokio::main]
@@ -113,13 +130,19 @@ pub async fn main_mcp_server() -> anyhow::Result<()> {
         Some(path) => path,
         None => mcp::get_bash_path()?,
     };
+    let policy = load_exec_policy().await?;
 
     tracing::info!("Starting MCP server");
-    let service = mcp::serve(bash_path, execve_wrapper, dummy_exec_policy)
-        .await
-        .inspect_err(|e| {
-            tracing::error!("serving error: {:?}", e);
-        })?;
+    let service = mcp::serve(
+        bash_path,
+        execve_wrapper,
+        policy.clone(),
+        cli.strip_program_paths,
+    )
+    .await
+    .inspect_err(|e| {
+        tracing::error!("serving error: {:?}", e);
+    })?;
 
     service.waiting().await?;
     Ok(())
@@ -146,26 +169,64 @@ pub async fn main_execve_wrapper() -> anyhow::Result<()> {
     std::process::exit(exit_code);
 }
 
-// TODO: replace with execpolicy
-
-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,
+/// Decide how to handle an exec() call for a specific command.
+///
+/// `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]`).
+pub(crate) fn evaluate_exec_policy(
+    policy: &Policy,
+    file: &Path,
+    argv: &[String],
+    strip_program_paths: bool,
+) -> ExecPolicyOutcome {
+    let command: Vec<String> = std::iter::once(format_program_name(file, strip_program_paths))
+        // Use the normalized program name instead of argv[0].
+        .chain(argv.iter().skip(1).cloned())
+        .collect();
+    let commands = parse_shell_lc_plain_commands(&command).unwrap_or_else(|| vec![command]);
+    let evaluation = policy.check_multiple(commands.iter(), &|cmd| {
+        if command_might_be_dangerous(cmd) {
+            Decision::Prompt
+        } else {
+            Decision::Allow
         }
+    });
+
+    // decisions driven by policy should run outside sandbox
+    let decision_driven_by_policy = evaluation.matched_rules.iter().any(|rule_match| {
+        !matches!(rule_match, RuleMatch::HeuristicsRuleMatch { .. })
+            && rule_match.decision() == evaluation.decision
+    });
+
+    match evaluation.decision {
+        Decision::Forbidden => ExecPolicyOutcome::Forbidden,
+        Decision::Prompt => ExecPolicyOutcome::Prompt {
+            run_with_escalated_permissions: decision_driven_by_policy,
+        },
+        Decision::Allow => ExecPolicyOutcome::Allow {
+            run_with_escalated_permissions: decision_driven_by_policy,
+        },
+    }
+}
+
+fn format_program_name(path: &Path, strip_program_paths: bool) -> String {
+    if strip_program_paths {
+        path.file_name()
+            .and_then(|name| name.to_str())
+            .map(str::to_string)
+            .unwrap_or_else(|| path.display().to_string())
     } else {
-        ExecPolicyOutcome::Allow {
-            run_with_escalated_permissions: false,
-        }
+        path.display().to_string()
     }
 }
+
+async fn load_exec_policy() -> anyhow::Result<Arc<Policy>> {
+    let codex_home = find_codex_home().context("failed to resolve codex_home for execpolicy")?;
+    let policy_dir = codex_home.join(POLICY_DIR_NAME);
+    let policy = exec_policy_for(&Features::with_defaults(), &codex_home)
+        .await
+        .map_err(|err: ExecPolicyError| anyhow::anyhow!(err))?;
+    let policy = policy.read().await.clone();
+    debug!("loaded execpolicy from {}", policy_dir.display());
+    Ok(Arc::new(policy))
+}
diff --git a/codex-rs/exec-server/src/posix/mcp.rs b/codex-rs/exec-server/src/posix/mcp.rs
@@ -79,17 +79,24 @@ pub struct ExecTool {
     bash_path: PathBuf,
     execve_wrapper: PathBuf,
     policy: ExecPolicy,
+    strip_program_paths: bool,
     sandbox_state: Arc<RwLock<Option<SandboxState>>>,
 }
 
 #[tool_router]
 impl ExecTool {
-    pub fn new(bash_path: PathBuf, execve_wrapper: PathBuf, policy: ExecPolicy) -> Self {
+    pub fn new(
+        bash_path: PathBuf,
+        execve_wrapper: PathBuf,
+        policy: ExecPolicy,
+        strip_program_paths: bool,
+    ) -> Self {
         Self {
             tool_router: Self::tool_router(),
             bash_path,
             execve_wrapper,
             policy,
+            strip_program_paths,
             sandbox_state: Arc::new(RwLock::new(None)),
         }
     }
@@ -121,7 +128,12 @@ impl ExecTool {
         let escalate_server = EscalateServer::new(
             self.bash_path.clone(),
             self.execve_wrapper.clone(),
-            McpEscalationPolicy::new(self.policy, context, stopwatch.clone()),
+            McpEscalationPolicy::new(
+                self.policy.clone(),
+                context,
+                stopwatch.clone(),
+                self.strip_program_paths,
+            ),
         );
 
         let result = escalate_server
@@ -199,8 +211,9 @@ pub(crate) async fn serve(
     bash_path: PathBuf,
     execve_wrapper: PathBuf,
     policy: ExecPolicy,
+    strip_program_paths: bool,
 ) -> Result<RunningService<RoleServer, ExecTool>, rmcp::service::ServerInitializeError> {
-    let tool = ExecTool::new(bash_path, execve_wrapper, policy);
+    let tool = ExecTool::new(bash_path, execve_wrapper, policy, strip_program_paths);
     tool.serve(stdio()).await
 }
 
diff --git a/codex-rs/exec-server/src/posix/mcp_escalation_policy.rs b/codex-rs/exec-server/src/posix/mcp_escalation_policy.rs
@@ -1,5 +1,7 @@
 use std::path::Path;
+use std::sync::Arc;
 
+use codex_execpolicy::Policy;
 use rmcp::ErrorData as McpError;
 use rmcp::RoleServer;
 use rmcp::model::CreateElicitationRequestParam;
@@ -12,13 +14,8 @@ use crate::posix::escalate_protocol::EscalateAction;
 use crate::posix::escalation_policy::EscalationPolicy;
 use crate::posix::stopwatch::Stopwatch;
 
-/// 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) -> ExecPolicyOutcome;
+/// In-memory execpolicy rules that drive how to handle an exec() call.
+pub(crate) type ExecPolicy = Arc<Policy>;
 
 pub(crate) enum ExecPolicyOutcome {
     Allow {
@@ -36,18 +33,21 @@ pub(crate) struct McpEscalationPolicy {
     policy: ExecPolicy,
     context: RequestContext<RoleServer>,
     stopwatch: Stopwatch,
+    strip_program_paths: bool,
 }
 
 impl McpEscalationPolicy {
     pub(crate) fn new(
         policy: ExecPolicy,
         context: RequestContext<RoleServer>,
         stopwatch: Stopwatch,
+        strip_program_paths: bool,
     ) -> Self {
         Self {
             policy,
             context,
             stopwatch,
+            strip_program_paths,
         }
     }
 
@@ -103,7 +103,8 @@ impl EscalationPolicy for McpEscalationPolicy {
         argv: &[String],
         workdir: &Path,
     ) -> Result<EscalateAction, rmcp::ErrorData> {
-        let outcome = (self.policy)(file, argv, workdir);
+        let outcome =
+            crate::posix::evaluate_exec_policy(&self.policy, file, argv, self.strip_program_paths);
         let action = match outcome {
             ExecPolicyOutcome::Allow {
                 run_with_escalated_permissions,

Original file line number	Diff line number	Diff line change
`@@ -73,7 +73,7 @@ pub enum ExecPolicyUpdateError {`
`73`	`73`	`FeatureDisabled,`
`74`	`74`	`}`
`75`	`75`
`76`		`-pub(crate) async fn exec_policy_for(`
	`76`	`+pub async fn exec_policy_for(`
`77`	`77`	`features: &Features,`
`78`	`78`	`codex_home: &Path,`
`79`	`79`	`) -> Result<Arc<RwLock<Policy>>, ExecPolicyError> {`