diff --git a/codex-rs/exec-server/src/posix/escalate_server.rs b/codex-rs/exec-server/src/posix/escalate_server.rs index 784562f2ff..d25409c839 100644 --- a/codex-rs/exec-server/src/posix/escalate_server.rs +++ b/codex-rs/exec-server/src/posix/escalate_server.rs @@ -23,6 +23,7 @@ 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::mcp::ExecParams; use crate::posix::socket::AsyncDatagramSocket; use crate::posix::socket::AsyncSocket; use codex_core::exec::ExecExpiration; @@ -47,9 +48,7 @@ impl EscalateServer { pub async fn exec( &self, - command: String, - env: HashMap, - workdir: PathBuf, + params: ExecParams, cancel_rx: CancellationToken, ) -> anyhow::Result { let (escalate_server, escalate_client) = AsyncDatagramSocket::pair()?; @@ -57,7 +56,7 @@ impl EscalateServer { client_socket.set_cloexec(false)?; let escalate_task = tokio::spawn(escalate_task(escalate_server, self.policy.clone())); - let mut env = env.clone(); + let mut env = std::env::vars().collect::>(); env.insert( ESCALATE_SOCKET_ENV_VAR.to_string(), client_socket.as_raw_fd().to_string(), @@ -73,11 +72,21 @@ impl EscalateServer { let sandbox_policy = SandboxPolicy::ReadOnly; let sandbox_cwd = PathBuf::from("/__NONEXISTENT__"); + let ExecParams { + command, + workdir, + timeout_ms: _, + login, + } = params; let result = process_exec_tool_call( codex_core::exec::ExecParams { command: vec![ self.bash_path.to_string_lossy().to_string(), - "-c".to_string(), + if login == Some(false) { + "-c".to_string() + } else { + "-lc".to_string() + }, command, ], cwd: PathBuf::from(&workdir), diff --git a/codex-rs/exec-server/src/posix/mcp.rs b/codex-rs/exec-server/src/posix/mcp.rs index b2f9b6de48..e4e7b25d92 100644 --- a/codex-rs/exec-server/src/posix/mcp.rs +++ b/codex-rs/exec-server/src/posix/mcp.rs @@ -41,6 +41,8 @@ pub struct ExecParams { 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, schemars::JsonSchema)] @@ -101,13 +103,7 @@ impl ExecTool { McpEscalationPolicy::new(self.policy, context, stopwatch.clone()), ); let result = escalate_server - .exec( - params.command, - // TODO: use ShellEnvironmentPolicy - std::env::vars().collect(), - PathBuf::from(¶ms.workdir), - cancel_token, - ) + .exec(params, cancel_token) .await .map_err(|e| McpError::internal_error(e.to_string(), None))?; Ok(CallToolResult::success(vec![Content::json( @@ -147,3 +143,50 @@ pub(crate) async fn serve( let tool = ExecTool::new(bash_path, execve_wrapper, policy); tool.serve(stdio()).await } + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + use serde_json::json; + + /// Verify that the way we use serde does not compromise the desired JSON + /// schema via schemars. In particular, ensure that the `login` and + /// `timeout_ms` fields are optional. + #[test] + fn exec_params_json_schema_matches_expected() { + let schema = schemars::schema_for!(ExecParams); + let actual = serde_json::to_value(schema).expect("schema should serialize"); + + assert_eq!( + actual, + json!({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "title": "ExecParams", + "type": "object", + "properties": { + "command": { + "description": "The bash string to execute.", + "type": "string" + }, + "login": { + "description": "Launch Bash with -lc instead of -c: defaults to true.", + "type": ["boolean", "null"] + }, + "timeout_ms": { + "description": "The timeout for the command in milliseconds.", + "format": "uint64", + "minimum": 0, + "type": ["integer", "null"] + }, + "workdir": { + "description": + "The working directory to execute the command in. Must be an absolute path.", + "type": "string" + } + }, + "required": ["command", "workdir"] + }) + ); + } +}