From 5221575f2336b17e2ec13f943d93356138d2590f Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 23 Feb 2026 09:28:17 -0800 Subject: [PATCH] refactor: normalize unix module layout for exec-server and shell-escalation (#12556) ## Why Shell execution refactoring in `exec-server` had become split between duplicated code paths, which blocked a clean introduction of the new reusable shell escalation flow. This commit creates a dedicated foundation crate so later shell tooling changes can share one implementation. ## What changed - Added the `codex-shell-escalation` crate and moved the core escalation pieces (`mcp` protocol/socket/session flow, policy glue) that were previously in `exec-server` into it. - Normalized `exec-server` Unix structure under a dedicated `unix` module layout and kept non-Unix builds narrow. - Wired crate/build metadata so `shell-escalation` is a first-class workspace dependency for follow-on integration work. ## Verification - Built and linted the stack at this commit point with `just clippy`. [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/12556). * #12584 * #12583 * __->__ #12556 --- codex-rs/Cargo.lock | 27 ++++- codex-rs/Cargo.toml | 2 + codex-rs/exec-server/Cargo.toml | 25 ++-- codex-rs/exec-server/src/lib.rs | 10 +- .../exec-server/src/{posix.rs => unix.rs} | 11 +- .../exec-server/src/{posix => unix}/mcp.rs | 105 ++++++---------- .../{posix => unix}/mcp_escalation_policy.rs | 14 +-- codex-rs/shell-escalation/BUILD.bazel | 6 + codex-rs/shell-escalation/Cargo.toml | 30 +++++ codex-rs/shell-escalation/src/lib.rs | 21 ++++ .../src/unix/core_shell_escalation.rs | 71 +++++++++++ .../src/unix}/escalate_client.rs | 22 ++-- .../src/unix}/escalate_protocol.rs | 30 ++--- .../src/unix}/escalate_server.rs | 113 ++++++++++++------ .../src/unix}/escalation_policy.rs | 6 +- codex-rs/shell-escalation/src/unix/mod.rs | 7 ++ .../src/unix}/socket.rs | 0 .../src/unix}/stopwatch.rs | 8 +- 18 files changed, 327 insertions(+), 181 deletions(-) rename codex-rs/exec-server/src/{posix.rs => unix.rs} (97%) rename codex-rs/exec-server/src/{posix => unix}/mcp.rs (75%) rename codex-rs/exec-server/src/{posix => unix}/mcp_escalation_policy.rs (92%) create mode 100644 codex-rs/shell-escalation/BUILD.bazel create mode 100644 codex-rs/shell-escalation/Cargo.toml create mode 100644 codex-rs/shell-escalation/src/lib.rs create mode 100644 codex-rs/shell-escalation/src/unix/core_shell_escalation.rs rename codex-rs/{exec-server/src/posix => shell-escalation/src/unix}/escalate_client.rs (85%) rename codex-rs/{exec-server/src/posix => shell-escalation/src/unix}/escalate_protocol.rs (66%) rename codex-rs/{exec-server/src/posix => shell-escalation/src/unix}/escalate_server.rs (76%) rename codex-rs/{exec-server/src/posix => shell-escalation/src/unix}/escalation_policy.rs (62%) create mode 100644 codex-rs/shell-escalation/src/unix/mod.rs rename codex-rs/{exec-server/src/posix => shell-escalation/src/unix}/socket.rs (100%) rename codex-rs/{exec-server/src/posix => shell-escalation/src/unix}/stopwatch.rs (96%) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index a4bd8e47e..b5e5cd5ac 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1786,21 +1786,19 @@ dependencies = [ "codex-execpolicy", "codex-protocol", "codex-shell-command", + "codex-shell-escalation", "codex-utils-cargo-bin", "core_test_support", "exec_server_test_support", - "libc", "maplit", - "path-absolutize", "pretty_assertions", "rmcp", + "schemars 1.2.1", "serde", "serde_json", "shlex", - "socket2 0.6.2", "tempfile", "tokio", - "tokio-util", "tracing", "tracing-subscriber", ] @@ -2201,6 +2199,27 @@ dependencies = [ "which", ] +[[package]] +name = "codex-shell-escalation" +version = "0.0.0" +dependencies = [ + "anyhow", + "async-trait", + "codex-core", + "codex-execpolicy", + "codex-protocol", + "libc", + "path-absolutize", + "pretty_assertions", + "serde", + "serde_json", + "socket2 0.6.2", + "tempfile", + "tokio", + "tokio-util", + "tracing", +] + [[package]] name = "codex-skills" version = "0.0.0" diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index e22dba12c..56fe6d4d5 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -17,6 +17,7 @@ members = [ "cli", "config", "shell-command", + "shell-escalation", "skills", "core", "hooks", @@ -113,6 +114,7 @@ codex-responses-api-proxy = { path = "responses-api-proxy" } codex-rmcp-client = { path = "rmcp-client" } codex-secrets = { path = "secrets" } codex-shell-command = { path = "shell-command" } +codex-shell-escalation = { path = "shell-escalation" } codex-skills = { path = "skills" } codex-state = { path = "state" } codex-stdio-to-uds = { path = "stdio-to-uds" } diff --git a/codex-rs/exec-server/Cargo.toml b/codex-rs/exec-server/Cargo.toml index 9b40cd35d..a6a0721b9 100644 --- a/codex-rs/exec-server/Cargo.toml +++ b/codex-rs/exec-server/Cargo.toml @@ -1,8 +1,8 @@ [package] name = "codex-exec-server" -version.workspace = true edition.workspace = true license.workspace = true +version.workspace = true [[bin]] name = "codex-execve-wrapper" @@ -19,6 +19,11 @@ path = "src/lib.rs" [lints] workspace = true +[package.metadata.cargo-shear] +# This appears to be due to #[derive(rmcp::schemars::JsonSchema)], which +# requires use of schemars via a macro that shear cannot detect. +ignored = ["schemars"] + [dependencies] anyhow = { workspace = true } async-trait = { workspace = true } @@ -27,8 +32,7 @@ codex-core = { workspace = true } codex-execpolicy = { workspace = true } codex-protocol = { workspace = true } codex-shell-command = { workspace = true } -libc = { workspace = true } -path-absolutize = { workspace = true } +codex-shell-escalation = { workspace = true } rmcp = { workspace = true, default-features = false, features = [ "auth", "elicitation", @@ -42,25 +46,18 @@ rmcp = { workspace = true, default-features = false, features = [ "transport-streamable-http-server", "transport-io", ] } +schemars = { version = "1.2.1" } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } shlex = { workspace = true } -socket2 = { workspace = true } -tokio = { workspace = true, features = [ - "io-std", - "macros", - "process", - "rt-multi-thread", - "signal", -] } -tokio-util = { workspace = true } +tokio = { workspace = true, features = ["macros", "rt-multi-thread", "signal"] } tracing = { workspace = true } tracing-subscriber = { workspace = true, features = ["env-filter", "fmt"] } [dev-dependencies] -core_test_support = { workspace = true } -codex-utils-cargo-bin = { workspace = true } codex-protocol = { workspace = true } +codex-utils-cargo-bin = { workspace = true } +core_test_support = { workspace = true } exec_server_test_support = { workspace = true } maplit = { workspace = true } pretty_assertions = { workspace = true } diff --git a/codex-rs/exec-server/src/lib.rs b/codex-rs/exec-server/src/lib.rs index 62f7bbccc..cd55b46ce 100644 --- a/codex-rs/exec-server/src/lib.rs +++ b/codex-rs/exec-server/src/lib.rs @@ -1,11 +1,5 @@ #[cfg(unix)] -mod posix; +mod unix; #[cfg(unix)] -pub use posix::main_execve_wrapper; - -#[cfg(unix)] -pub use posix::main_mcp_server; - -#[cfg(unix)] -pub use posix::ExecResult; +pub use unix::*; diff --git a/codex-rs/exec-server/src/posix.rs b/codex-rs/exec-server/src/unix.rs similarity index 97% rename from codex-rs/exec-server/src/posix.rs rename to codex-rs/exec-server/src/unix.rs index 907ec14b6..0e0355443 100644 --- a/codex-rs/exec-server/src/posix.rs +++ b/codex-rs/exec-server/src/unix.rs @@ -67,21 +67,16 @@ use codex_execpolicy::Decision; use codex_execpolicy::Policy; use codex_execpolicy::RuleMatch; use codex_shell_command::is_dangerous_command::command_might_be_dangerous; +use codex_shell_escalation as shell_escalation; use rmcp::ErrorData as McpError; use tokio::sync::RwLock; use tracing_subscriber::EnvFilter; use tracing_subscriber::{self}; -use crate::posix::mcp_escalation_policy::ExecPolicyOutcome; +use crate::unix::mcp_escalation_policy::ExecPolicyOutcome; -mod escalate_client; -mod escalate_protocol; -mod escalate_server; -mod escalation_policy; mod mcp; mod mcp_escalation_policy; -mod socket; -mod stopwatch; pub use mcp::ExecResult; @@ -165,7 +160,7 @@ pub async fn main_execve_wrapper() -> anyhow::Result<()> { .init(); let ExecveWrapperCli { file, argv } = ExecveWrapperCli::parse(); - let exit_code = escalate_client::run(file, argv).await?; + let exit_code = shell_escalation::run(file, argv).await?; std::process::exit(exit_code); } diff --git a/codex-rs/exec-server/src/posix/mcp.rs b/codex-rs/exec-server/src/unix/mcp.rs similarity index 75% rename from codex-rs/exec-server/src/posix/mcp.rs rename to codex-rs/exec-server/src/unix/mcp.rs index f413fed21..547d055c1 100644 --- a/codex-rs/exec-server/src/posix/mcp.rs +++ b/codex-rs/exec-server/src/unix/mcp.rs @@ -1,4 +1,3 @@ -use std::path::Path; use std::path::PathBuf; use std::sync::Arc; use std::time::Duration; @@ -10,6 +9,8 @@ use codex_core::MCP_SANDBOX_STATE_METHOD; use codex_core::SandboxState; use codex_execpolicy::Policy; use codex_protocol::protocol::SandboxPolicy; +use codex_shell_escalation::EscalationPolicyFactory; +use codex_shell_escalation::run_escalate_server; use rmcp::ErrorData as McpError; use rmcp::RoleServer; use rmcp::ServerHandler; @@ -19,7 +20,6 @@ use rmcp::handler::server::wrapper::Parameters; use rmcp::model::CustomRequest; use rmcp::model::CustomResult; use rmcp::model::*; -use rmcp::schemars; use rmcp::service::RequestContext; use rmcp::service::RunningService; use rmcp::tool; @@ -29,11 +29,7 @@ use rmcp::transport::stdio; use serde_json::json; use tokio::sync::RwLock; -use crate::posix::escalate_server::EscalateServer; -use crate::posix::escalate_server::{self}; -use crate::posix::escalation_policy::EscalationPolicy; -use crate::posix::mcp_escalation_policy::McpEscalationPolicy; -use crate::posix::stopwatch::Stopwatch; +use crate::unix::mcp_escalation_policy::McpEscalationPolicy; /// Path to our patched bash. const CODEX_BASH_PATH_ENV_VAR: &str = "CODEX_BASH_PATH"; @@ -46,19 +42,7 @@ pub(crate) fn get_bash_path() -> Result { .context(format!("{CODEX_BASH_PATH_ENV_VAR} must be set")) } -#[derive(Debug, serde::Deserialize, schemars::JsonSchema)] -pub struct ExecParams { - /// The bash string to execute. - pub command: String, - /// The working directory to execute the command in. Must be an absolute path. - pub workdir: String, - /// The timeout for the command in milliseconds. - pub timeout_ms: Option, - /// Launch Bash with -lc instead of -c: defaults to true. - pub login: Option, -} - -#[derive(Debug, serde::Serialize, serde::Deserialize, schemars::JsonSchema)] +#[derive(Debug, serde::Serialize, serde::Deserialize)] pub struct ExecResult { pub exit_code: i32, pub output: String, @@ -66,8 +50,8 @@ pub struct ExecResult { pub timed_out: bool, } -impl From for ExecResult { - fn from(result: escalate_server::ExecResult) -> Self { +impl From for ExecResult { + fn from(result: codex_shell_escalation::ExecResult) -> Self { Self { exit_code: result.exit_code, output: result.output, @@ -87,10 +71,27 @@ pub struct ExecTool { sandbox_state: Arc>>, } -trait EscalationPolicyFactory { - type Policy: EscalationPolicy + Send + Sync + 'static; +#[derive(Debug, serde::Serialize, serde::Deserialize, rmcp::schemars::JsonSchema)] +pub struct ExecParams { + /// The bash string to execute. + pub command: String, + /// The working directory to execute the command in. Must be an absolute path. + pub workdir: String, + /// The timeout for the command in milliseconds. + pub timeout_ms: Option, + /// Launch Bash with -lc instead of -c: defaults to true. + pub login: Option, +} - fn create_policy(&self, policy: Arc>, stopwatch: Stopwatch) -> Self::Policy; +impl From for codex_shell_escalation::ExecParams { + fn from(inner: ExecParams) -> Self { + Self { + command: inner.command, + workdir: inner.workdir, + timeout_ms: inner.timeout_ms, + login: inner.login, + } + } } struct McpEscalationPolicyFactory { @@ -101,7 +102,11 @@ struct McpEscalationPolicyFactory { impl EscalationPolicyFactory for McpEscalationPolicyFactory { type Policy = McpEscalationPolicy; - fn create_policy(&self, policy: Arc>, stopwatch: Stopwatch) -> Self::Policy { + fn create_policy( + &self, + policy: Arc>, + stopwatch: codex_shell_escalation::Stopwatch, + ) -> Self::Policy { McpEscalationPolicy::new( policy, self.context.clone(), @@ -153,8 +158,8 @@ impl ExecTool { use_linux_sandbox_bwrap: false, }); let result = run_escalate_server( - params, - sandbox_state, + params.into(), + &sandbox_state, &self.bash_path, &self.execve_wrapper, self.policy.clone(), @@ -172,48 +177,6 @@ impl ExecTool { } } -/// Runs the escalate server to execute a shell command with potential -/// escalation of execve calls. -/// -/// - `exec_params` defines the shell command to run -/// - `sandbox_state` is the sandbox to use to run the shell program -/// - `shell_program` is the path to the shell program to run (e.g. /bin/bash) -/// - `execve_wrapper` is the path to the execve wrapper binary to use for -/// handling execve calls from the shell program. This is likely a symlink to -/// Codex using a special name. -/// - `policy` is the exec policy to use for deciding whether to allow or deny -/// execve calls from the shell program. -/// - `escalation_policy_factory` is a factory for creating an -/// `EscalationPolicy` to use for deciding whether to allow, deny, or prompt -/// the user for execve calls from the shell program. We use a factory here -/// because the `EscalationPolicy` may need to capture request-specific -/// context (e.g. the MCP request context) that is not available at the time -/// we create the `ExecTool`. -/// - `effective_timeout` is the timeout to use for running the shell command. -/// Implementations are encouraged to excludeany time spent prompting the -/// user. -async fn run_escalate_server( - exec_params: ExecParams, - sandbox_state: SandboxState, - shell_program: impl AsRef, - execve_wrapper: impl AsRef, - policy: Arc>, - escalation_policy_factory: impl EscalationPolicyFactory, - effective_timeout: Duration, -) -> anyhow::Result { - let stopwatch = Stopwatch::new(effective_timeout); - let cancel_token = stopwatch.cancellation_token(); - let escalate_server = EscalateServer::new( - shell_program.as_ref().to_path_buf(), - execve_wrapper.as_ref().to_path_buf(), - escalation_policy_factory.create_policy(policy, stopwatch), - ); - - escalate_server - .exec(exec_params, cancel_token, &sandbox_state) - .await -} - #[derive(Default)] pub struct CodexSandboxStateUpdateMethod; @@ -307,7 +270,7 @@ mod tests { /// `timeout_ms` fields are optional. #[test] fn exec_params_json_schema_matches_expected() { - let schema = schemars::schema_for!(ExecParams); + let schema = rmcp::schemars::schema_for!(ExecParams); let actual = serde_json::to_value(schema).expect("schema should serialize"); assert_eq!( diff --git a/codex-rs/exec-server/src/posix/mcp_escalation_policy.rs b/codex-rs/exec-server/src/unix/mcp_escalation_policy.rs similarity index 92% rename from codex-rs/exec-server/src/posix/mcp_escalation_policy.rs rename to codex-rs/exec-server/src/unix/mcp_escalation_policy.rs index 5e6b97f1a..986381826 100644 --- a/codex-rs/exec-server/src/posix/mcp_escalation_policy.rs +++ b/codex-rs/exec-server/src/unix/mcp_escalation_policy.rs @@ -2,6 +2,9 @@ use std::path::Path; use codex_core::sandboxing::SandboxPermissions; use codex_execpolicy::Policy; +use codex_shell_escalation::EscalateAction; +use codex_shell_escalation::EscalationPolicy; +use codex_shell_escalation::Stopwatch; use rmcp::ErrorData as McpError; use rmcp::RoleServer; use rmcp::model::CreateElicitationRequestParams; @@ -9,10 +12,7 @@ use rmcp::model::CreateElicitationResult; use rmcp::model::ElicitationAction; use rmcp::model::ElicitationSchema; use rmcp::service::RequestContext; - -use crate::posix::escalate_protocol::EscalateAction; -use crate::posix::escalation_policy::EscalationPolicy; -use crate::posix::stopwatch::Stopwatch; +use shlex::try_join; use std::sync::Arc; use tokio::sync::RwLock; @@ -59,7 +59,7 @@ impl McpEscalationPolicy { workdir: &Path, context: RequestContext, ) -> Result { - let args = shlex::try_join(argv.iter().skip(1).map(String::as_str)).unwrap_or_default(); + let args = try_join(argv.iter().skip(1).map(String::as_str)).unwrap_or_default(); let command = if args.is_empty() { file.display().to_string() } else { @@ -104,10 +104,10 @@ impl EscalationPolicy for McpEscalationPolicy { file: &Path, argv: &[String], workdir: &Path, - ) -> Result { + ) -> anyhow::Result { let policy = self.policy.read().await; let outcome = - crate::posix::evaluate_exec_policy(&policy, file, argv, self.preserve_program_paths)?; + crate::unix::evaluate_exec_policy(&policy, file, argv, self.preserve_program_paths)?; let action = match outcome { ExecPolicyOutcome::Allow { sandbox_permissions, diff --git a/codex-rs/shell-escalation/BUILD.bazel b/codex-rs/shell-escalation/BUILD.bazel new file mode 100644 index 000000000..5092f3332 --- /dev/null +++ b/codex-rs/shell-escalation/BUILD.bazel @@ -0,0 +1,6 @@ +load("//:defs.bzl", "codex_rust_crate") + +codex_rust_crate( + name = "shell-escalation", + crate_name = "codex_shell_escalation", +) diff --git a/codex-rs/shell-escalation/Cargo.toml b/codex-rs/shell-escalation/Cargo.toml new file mode 100644 index 000000000..49b8f10bd --- /dev/null +++ b/codex-rs/shell-escalation/Cargo.toml @@ -0,0 +1,30 @@ +[package] +name = "codex-shell-escalation" +version.workspace = true +edition.workspace = true +license.workspace = true + +[dependencies] +anyhow = { workspace = true } +async-trait = { workspace = true } +codex-core = { workspace = true } +codex-execpolicy = { workspace = true } +codex-protocol = { workspace = true } +libc = { workspace = true } +serde_json = { workspace = true } +path-absolutize = { workspace = true } +serde = { workspace = true, features = ["derive"] } +socket2 = { workspace = true } +tokio = { workspace = true, features = [ + "io-std", + "macros", + "process", + "rt-multi-thread", + "signal", +] } +tokio-util = { workspace = true } +tracing = { workspace = true } + +[dev-dependencies] +pretty_assertions = { workspace = true } +tempfile = { workspace = true } diff --git a/codex-rs/shell-escalation/src/lib.rs b/codex-rs/shell-escalation/src/lib.rs new file mode 100644 index 000000000..555d0f89e --- /dev/null +++ b/codex-rs/shell-escalation/src/lib.rs @@ -0,0 +1,21 @@ +#[cfg(unix)] +mod unix { + mod escalate_client; + mod escalate_protocol; + mod escalate_server; + mod escalation_policy; + mod socket; + mod stopwatch; + + pub use self::escalate_client::run; + pub use self::escalate_protocol::EscalateAction; + pub use self::escalate_server::EscalationPolicyFactory; + pub use self::escalate_server::ExecParams; + pub use self::escalate_server::ExecResult; + pub use self::escalate_server::run_escalate_server; + pub use self::escalation_policy::EscalationPolicy; + pub use self::stopwatch::Stopwatch; +} + +#[cfg(unix)] +pub use unix::*; diff --git a/codex-rs/shell-escalation/src/unix/core_shell_escalation.rs b/codex-rs/shell-escalation/src/unix/core_shell_escalation.rs new file mode 100644 index 000000000..0be7af28f --- /dev/null +++ b/codex-rs/shell-escalation/src/unix/core_shell_escalation.rs @@ -0,0 +1,71 @@ +use async_trait::async_trait; +use std::path::Path; +use std::sync::Arc; +use tokio::sync::RwLock; + +use crate::escalate_protocol::EscalateAction; +use crate::escalation_policy::EscalationPolicy; +use crate::stopwatch::Stopwatch; +use crate::unix::escalate_server::EscalationPolicyFactory; +use codex_execpolicy::Policy; + +#[async_trait] +pub trait ShellActionProvider: Send + Sync { + async fn determine_action( + &self, + file: &Path, + argv: &[String], + workdir: &Path, + stopwatch: &Stopwatch, + ) -> anyhow::Result; +} + +#[derive(Clone)] +pub struct ShellPolicyFactory { + provider: Arc, +} + +impl ShellPolicyFactory { + pub fn new

(provider: P) -> Self + where + P: ShellActionProvider + 'static, + { + Self { + provider: Arc::new(provider), + } + } + + pub fn with_provider(provider: Arc) -> Self { + Self { provider } + } +} + +struct ShellEscalationPolicy { + provider: Arc, + stopwatch: Stopwatch, +} + +#[async_trait] +impl EscalationPolicy for ShellEscalationPolicy { + async fn determine_action( + &self, + file: &Path, + argv: &[String], + workdir: &Path, + ) -> anyhow::Result { + self.provider + .determine_action(file, argv, workdir, &self.stopwatch) + .await + } +} + +impl EscalationPolicyFactory for ShellPolicyFactory { + type Policy = ShellEscalationPolicy; + + fn create_policy(&self, _policy: Arc>, stopwatch: Stopwatch) -> Self::Policy { + ShellEscalationPolicy { + provider: Arc::clone(&self.provider), + stopwatch, + } + } +} diff --git a/codex-rs/exec-server/src/posix/escalate_client.rs b/codex-rs/shell-escalation/src/unix/escalate_client.rs similarity index 85% rename from codex-rs/exec-server/src/posix/escalate_client.rs rename to codex-rs/shell-escalation/src/unix/escalate_client.rs index 1cb0c5908..17d8e6577 100644 --- a/codex-rs/exec-server/src/posix/escalate_client.rs +++ b/codex-rs/shell-escalation/src/unix/escalate_client.rs @@ -5,16 +5,16 @@ use std::os::fd::OwnedFd; use anyhow::Context as _; -use crate::posix::escalate_protocol::ESCALATE_SOCKET_ENV_VAR; -use crate::posix::escalate_protocol::EXEC_WRAPPER_ENV_VAR; -use crate::posix::escalate_protocol::EscalateAction; -use crate::posix::escalate_protocol::EscalateRequest; -use crate::posix::escalate_protocol::EscalateResponse; -use crate::posix::escalate_protocol::LEGACY_BASH_EXEC_WRAPPER_ENV_VAR; -use crate::posix::escalate_protocol::SuperExecMessage; -use crate::posix::escalate_protocol::SuperExecResult; -use crate::posix::socket::AsyncDatagramSocket; -use crate::posix::socket::AsyncSocket; +use crate::unix::escalate_protocol::ESCALATE_SOCKET_ENV_VAR; +use crate::unix::escalate_protocol::EXEC_WRAPPER_ENV_VAR; +use crate::unix::escalate_protocol::EscalateAction; +use crate::unix::escalate_protocol::EscalateRequest; +use crate::unix::escalate_protocol::EscalateResponse; +use crate::unix::escalate_protocol::LEGACY_BASH_EXEC_WRAPPER_ENV_VAR; +use crate::unix::escalate_protocol::SuperExecMessage; +use crate::unix::escalate_protocol::SuperExecResult; +use crate::unix::socket::AsyncDatagramSocket; +use crate::unix::socket::AsyncSocket; fn get_escalate_client() -> anyhow::Result { // TODO: we should defensively require only calling this once, since AsyncSocket will take ownership of the fd. @@ -27,7 +27,7 @@ fn get_escalate_client() -> anyhow::Result { Ok(unsafe { AsyncDatagramSocket::from_raw_fd(client_fd) }?) } -pub(crate) async fn run(file: String, argv: Vec) -> anyhow::Result { +pub async fn run(file: String, argv: Vec) -> anyhow::Result { let handshake_client = get_escalate_client()?; let (server, client) = AsyncSocket::pair()?; const HANDSHAKE_MESSAGE: [u8; 1] = [0]; diff --git a/codex-rs/exec-server/src/posix/escalate_protocol.rs b/codex-rs/shell-escalation/src/unix/escalate_protocol.rs similarity index 66% rename from codex-rs/exec-server/src/posix/escalate_protocol.rs rename to codex-rs/shell-escalation/src/unix/escalate_protocol.rs index 7245f88fb..3bf2f712d 100644 --- a/codex-rs/exec-server/src/posix/escalate_protocol.rs +++ b/codex-rs/shell-escalation/src/unix/escalate_protocol.rs @@ -6,33 +6,33 @@ use serde::Deserialize; use serde::Serialize; /// 'exec-server escalate' reads this to find the inherited FD for the escalate socket. -pub(super) const ESCALATE_SOCKET_ENV_VAR: &str = "CODEX_ESCALATE_SOCKET"; +pub const ESCALATE_SOCKET_ENV_VAR: &str = "CODEX_ESCALATE_SOCKET"; /// Patched shells use this to wrap exec() calls. -pub(super) const EXEC_WRAPPER_ENV_VAR: &str = "EXEC_WRAPPER"; +pub const EXEC_WRAPPER_ENV_VAR: &str = "EXEC_WRAPPER"; /// Compatibility alias for older patched bash builds. -pub(super) const LEGACY_BASH_EXEC_WRAPPER_ENV_VAR: &str = "BASH_EXEC_WRAPPER"; +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(super) struct EscalateRequest { +pub struct EscalateRequest { /// The absolute path to the executable to run, i.e. the first arg to exec. - pub(super) file: PathBuf, + pub file: PathBuf, /// The argv, including the program name (argv[0]). - pub(super) argv: Vec, - pub(super) workdir: PathBuf, - pub(super) env: HashMap, + pub argv: Vec, + pub workdir: PathBuf, + pub env: HashMap, } /// The server sends this to the client to respond to an exec() request. #[derive(Clone, Serialize, Deserialize, Debug, PartialEq, Eq)] -pub(super) struct EscalateResponse { - pub(super) action: EscalateAction, +pub struct EscalateResponse { + pub action: EscalateAction, } #[derive(Clone, Serialize, Deserialize, Debug, PartialEq, Eq)] -pub(super) enum EscalateAction { +pub enum EscalateAction { /// The command should be run directly by the client. Run, /// The command should be escalated to the server for execution. @@ -43,12 +43,12 @@ pub(super) enum EscalateAction { /// The client sends this to the server to forward its open FDs. #[derive(Clone, Serialize, Deserialize, Debug)] -pub(super) struct SuperExecMessage { - pub(super) fds: Vec, +pub struct SuperExecMessage { + pub fds: Vec, } /// The server responds when the exec()'d command has exited. #[derive(Clone, Serialize, Deserialize, Debug)] -pub(super) struct SuperExecResult { - pub(super) exit_code: i32, +pub struct SuperExecResult { + pub exit_code: i32, } diff --git a/codex-rs/exec-server/src/posix/escalate_server.rs b/codex-rs/shell-escalation/src/unix/escalate_server.rs similarity index 76% rename from codex-rs/exec-server/src/posix/escalate_server.rs rename to codex-rs/shell-escalation/src/unix/escalate_server.rs index de4bdfc2c..0ee5fc27c 100644 --- a/codex-rs/exec-server/src/posix/escalate_server.rs +++ b/codex-rs/shell-escalation/src/unix/escalate_server.rs @@ -1,35 +1,54 @@ 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 _; -use path_absolutize::Absolutize as _; - use codex_core::SandboxState; -use codex_core::exec::process_exec_tool_call; -use codex_core::sandboxing::SandboxPermissions; -use codex_protocol::config_types::WindowsSandboxLevel; +use codex_execpolicy::Policy; +use path_absolutize::Absolutize as _; use tokio::process::Command; +use tokio::sync::RwLock; use tokio_util::sync::CancellationToken; -use crate::posix::escalate_protocol::ESCALATE_SOCKET_ENV_VAR; -use crate::posix::escalate_protocol::EXEC_WRAPPER_ENV_VAR; -use crate::posix::escalate_protocol::EscalateAction; -use crate::posix::escalate_protocol::EscalateRequest; -use crate::posix::escalate_protocol::EscalateResponse; -use crate::posix::escalate_protocol::LEGACY_BASH_EXEC_WRAPPER_ENV_VAR; -use crate::posix::escalate_protocol::SuperExecMessage; -use crate::posix::escalate_protocol::SuperExecResult; -use crate::posix::escalation_policy::EscalationPolicy; -use crate::posix::mcp::ExecParams; -use crate::posix::socket::AsyncDatagramSocket; -use crate::posix::socket::AsyncSocket; -use codex_core::exec::ExecExpiration; +use crate::unix::escalate_protocol::ESCALATE_SOCKET_ENV_VAR; +use crate::unix::escalate_protocol::EXEC_WRAPPER_ENV_VAR; +use crate::unix::escalate_protocol::EscalateAction; +use crate::unix::escalate_protocol::EscalateRequest; +use crate::unix::escalate_protocol::EscalateResponse; +use crate::unix::escalate_protocol::LEGACY_BASH_EXEC_WRAPPER_ENV_VAR; +use crate::unix::escalate_protocol::SuperExecMessage; +use crate::unix::escalate_protocol::SuperExecResult; +use crate::unix::escalation_policy::EscalationPolicy; +use crate::unix::socket::AsyncDatagramSocket; +use crate::unix::socket::AsyncSocket; +use crate::unix::stopwatch::Stopwatch; -pub(crate) struct EscalateServer { +#[derive(Debug, serde::Deserialize, serde::Serialize)] +pub struct ExecParams { + /// The bash string to execute. + pub command: String, + /// The working directory to execute the command in. Must be an absolute path. + pub workdir: String, + /// The timeout for the command in milliseconds. + pub timeout_ms: Option, + /// Launch Bash with -lc instead of -c: defaults to true. + pub login: Option, +} + +#[derive(Debug, serde::Serialize, serde::Deserialize)] +pub struct ExecResult { + pub exit_code: i32, + pub output: String, + pub duration: Duration, + pub timed_out: bool, +} + +#[allow(clippy::module_name_repetitions)] +pub struct EscalateServer { bash_path: PathBuf, execve_wrapper: PathBuf, policy: Arc, @@ -78,7 +97,7 @@ impl EscalateServer { timeout_ms: _, login, } = params; - let result = process_exec_tool_call( + let result = codex_core::exec::process_exec_tool_call( codex_core::exec::ExecParams { command: vec![ self.bash_path.to_string_lossy().to_string(), @@ -90,11 +109,11 @@ impl EscalateServer { command, ], cwd: PathBuf::from(&workdir), - expiration: ExecExpiration::Cancellation(cancel_rx), + expiration: codex_core::exec::ExecExpiration::Cancellation(cancel_rx), env, network: None, - sandbox_permissions: SandboxPermissions::UseDefault, - windows_sandbox_level: WindowsSandboxLevel::Disabled, + sandbox_permissions: codex_core::sandboxing::SandboxPermissions::UseDefault, + windows_sandbox_level: codex_protocol::config_types::WindowsSandboxLevel::Disabled, justification: None, arg0: None, }, @@ -106,16 +125,45 @@ impl EscalateServer { ) .await?; escalate_task.abort(); - let result = ExecResult { + + Ok(ExecResult { exit_code: result.exit_code, output: result.aggregated_output.text, duration: result.duration, timed_out: result.timed_out, - }; - Ok(result) + }) } } +/// Factory for creating escalation policy instances for a single shell run. +pub trait EscalationPolicyFactory { + type Policy: EscalationPolicy + Send + Sync + 'static; + + fn create_policy(&self, policy: Arc>, stopwatch: Stopwatch) -> Self::Policy; +} + +pub async fn run_escalate_server( + exec_params: ExecParams, + sandbox_state: &SandboxState, + shell_program: impl AsRef, + execve_wrapper: impl AsRef, + policy: Arc>, + escalation_policy_factory: impl EscalationPolicyFactory, + effective_timeout: Duration, +) -> anyhow::Result { + let stopwatch = Stopwatch::new(effective_timeout); + let cancel_token = stopwatch.cancellation_token(); + let escalate_server = EscalateServer::new( + shell_program.as_ref().to_path_buf(), + execve_wrapper.as_ref().to_path_buf(), + escalation_policy_factory.create_policy(policy, stopwatch), + ); + + escalate_server + .exec(exec_params, cancel_token, sandbox_state) + .await +} + async fn escalate_task( socket: AsyncDatagramSocket, policy: Arc, @@ -136,14 +184,6 @@ async fn escalate_task( } } -#[derive(Debug)] -pub(crate) struct ExecResult { - pub(crate) exit_code: i32, - pub(crate) output: String, - pub(crate) duration: Duration, - pub(crate) timed_out: bool, -} - async fn handle_escalate_session_with_policy( socket: AsyncSocket, policy: Arc, @@ -158,7 +198,8 @@ async fn handle_escalate_session_with_policy( let workdir = PathBuf::from(&workdir).absolutize()?.into_owned(); let action = policy .determine_action(file.as_path(), &argv, &workdir) - .await?; + .await + .context("failed to determine escalation action")?; tracing::debug!("decided {action:?} for {file:?} {argv:?} {workdir:?}"); @@ -253,7 +294,7 @@ mod tests { _file: &Path, _argv: &[String], _workdir: &Path, - ) -> Result { + ) -> anyhow::Result { Ok(self.action.clone()) } } diff --git a/codex-rs/exec-server/src/posix/escalation_policy.rs b/codex-rs/shell-escalation/src/unix/escalation_policy.rs similarity index 62% rename from codex-rs/exec-server/src/posix/escalation_policy.rs rename to codex-rs/shell-escalation/src/unix/escalation_policy.rs index a7fcc4f62..09e74e271 100644 --- a/codex-rs/exec-server/src/posix/escalation_policy.rs +++ b/codex-rs/shell-escalation/src/unix/escalation_policy.rs @@ -1,14 +1,14 @@ use std::path::Path; -use crate::posix::escalate_protocol::EscalateAction; +use crate::unix::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 { +pub trait EscalationPolicy: Send + Sync { async fn determine_action( &self, file: &Path, argv: &[String], workdir: &Path, - ) -> Result; + ) -> anyhow::Result; } diff --git a/codex-rs/shell-escalation/src/unix/mod.rs b/codex-rs/shell-escalation/src/unix/mod.rs new file mode 100644 index 000000000..0ae7941da --- /dev/null +++ b/codex-rs/shell-escalation/src/unix/mod.rs @@ -0,0 +1,7 @@ +pub mod escalate_client; +pub mod escalate_protocol; +pub mod escalate_server; +pub mod escalation_policy; +pub mod socket; +pub mod core_shell_escalation; +pub mod stopwatch; diff --git a/codex-rs/exec-server/src/posix/socket.rs b/codex-rs/shell-escalation/src/unix/socket.rs similarity index 100% rename from codex-rs/exec-server/src/posix/socket.rs rename to codex-rs/shell-escalation/src/unix/socket.rs diff --git a/codex-rs/exec-server/src/posix/stopwatch.rs b/codex-rs/shell-escalation/src/unix/stopwatch.rs similarity index 96% rename from codex-rs/exec-server/src/posix/stopwatch.rs rename to codex-rs/shell-escalation/src/unix/stopwatch.rs index de29a4568..f56443481 100644 --- a/codex-rs/exec-server/src/posix/stopwatch.rs +++ b/codex-rs/shell-escalation/src/unix/stopwatch.rs @@ -8,7 +8,7 @@ use tokio::sync::Notify; use tokio_util::sync::CancellationToken; #[derive(Clone, Debug)] -pub(crate) struct Stopwatch { +pub struct Stopwatch { limit: Duration, inner: Arc>, notify: Arc, @@ -22,7 +22,7 @@ struct StopwatchState { } impl Stopwatch { - pub(crate) fn new(limit: Duration) -> Self { + pub fn new(limit: Duration) -> Self { Self { inner: Arc::new(Mutex::new(StopwatchState { elapsed: Duration::ZERO, @@ -34,7 +34,7 @@ impl Stopwatch { } } - pub(crate) fn cancellation_token(&self) -> CancellationToken { + pub fn cancellation_token(&self) -> CancellationToken { let limit = self.limit; let token = CancellationToken::new(); let cancel = token.clone(); @@ -80,7 +80,7 @@ impl Stopwatch { /// resumes automatically when the future completes. Nested/overlapping /// calls are reference-counted so the stopwatch only resumes when every /// pause is lifted. - pub(crate) async fn pause_for(&self, fut: F) -> T + pub async fn pause_for(&self, fut: F) -> T where F: Future, {