From 658bf9f740755c7f0c02512c4edc39530c94e45b Mon Sep 17 00:00:00 2001 From: David Wiesen Date: Tue, 24 Mar 2026 09:29:40 -0700 Subject: [PATCH] Block stdio MCP startup in Windows sandbox --- codex-rs/core/src/mcp_connection_manager.rs | 66 +++++++++++++++--- .../core/src/mcp_connection_manager_tests.rs | 69 +++++++++++++++++++ 2 files changed, 125 insertions(+), 10 deletions(-) diff --git a/codex-rs/core/src/mcp_connection_manager.rs b/codex-rs/core/src/mcp_connection_manager.rs index 938d6d0b2b..07762fee43 100644 --- a/codex-rs/core/src/mcp_connection_manager.rs +++ b/codex-rs/core/src/mcp_connection_manager.rs @@ -501,6 +501,20 @@ impl AsyncManagedClient { } } + fn failed(tool_plugin_provenance: Arc, error: String) -> Self { + let startup_complete = Arc::new(AtomicBool::new(true)); + let client = futures::future::ready(Err(StartupOutcomeError::Failed { error })) + .boxed() + .shared(); + + Self { + client, + startup_snapshot: None, + startup_complete, + tool_plugin_provenance, + } + } + async fn client(&self) -> Result { self.client.clone().await } @@ -671,16 +685,24 @@ impl McpConnectionManager { } else { None }; - let async_managed_client = AsyncManagedClient::new( - server_name.clone(), - cfg, - store_mode, - cancel_token.clone(), - tx_event.clone(), - elicitation_requests.clone(), - codex_apps_tools_cache_context, - Arc::clone(&tool_plugin_provenance), - ); + let async_managed_client = if let Some(error) = sandboxed_stdio_mcp_startup_error( + &cfg.transport, + &initial_sandbox_state, + cfg!(windows), + ) { + AsyncManagedClient::failed(Arc::clone(&tool_plugin_provenance), error) + } else { + AsyncManagedClient::new( + server_name.clone(), + cfg, + store_mode, + cancel_token.clone(), + tx_event.clone(), + elicitation_requests.clone(), + codex_apps_tools_cache_context, + Arc::clone(&tool_plugin_provenance), + ) + }; clients.insert(server_name.clone(), async_managed_client.clone()); let tx_event = tx_event.clone(); let auth_entry = auth_entries.get(&server_name).cloned(); @@ -1580,6 +1602,30 @@ fn transport_origin(transport: &McpServerTransportConfig) -> Option { } } +fn sandboxed_stdio_mcp_startup_error( + transport: &McpServerTransportConfig, + sandbox_state: &SandboxState, + is_windows_host: bool, +) -> Option { + if !is_windows_host { + return None; + } + + if !matches!( + sandbox_state.sandbox_policy, + SandboxPolicy::ReadOnly { .. } | SandboxPolicy::WorkspaceWrite { .. } + ) { + return None; + } + + match transport { + McpServerTransportConfig::Stdio { .. } => Some( + "Local stdio MCP servers are disabled while Windows sandboxing is enabled because they run outside the sandbox and can access host resources. Use a streamable HTTP MCP server, or disable sandboxing if you intend to grant host access.".to_string(), + ), + McpServerTransportConfig::StreamableHttp { .. } => None, + } +} + async fn list_tools_for_client_uncached( server_name: &str, client: &Arc, diff --git a/codex-rs/core/src/mcp_connection_manager_tests.rs b/codex-rs/core/src/mcp_connection_manager_tests.rs index c5f7fc4a40..5089008b87 100644 --- a/codex-rs/core/src/mcp_connection_manager_tests.rs +++ b/codex-rs/core/src/mcp_connection_manager_tests.rs @@ -642,3 +642,72 @@ fn transport_origin_is_stdio_for_stdio_transport() { assert_eq!(transport_origin(&transport), Some("stdio".to_string())); } + +#[test] +fn sandboxed_stdio_mcp_startup_error_blocks_windows_stdio_servers() { + let sandbox_state = SandboxState { + sandbox_policy: SandboxPolicy::new_read_only_policy(), + codex_linux_sandbox_exe: None, + sandbox_cwd: PathBuf::from("/tmp"), + use_legacy_landlock: false, + }; + let transport = McpServerTransportConfig::Stdio { + command: "bridge-server".to_string(), + args: Vec::new(), + env: None, + env_vars: Vec::new(), + cwd: None, + }; + + let error = sandboxed_stdio_mcp_startup_error(&transport, &sandbox_state, true); + + assert_eq!( + error, + Some( + "Local stdio MCP servers are disabled while Windows sandboxing is enabled because they run outside the sandbox and can access host resources. Use a streamable HTTP MCP server, or disable sandboxing if you intend to grant host access.".to_string() + ) + ); +} + +#[test] +fn sandboxed_stdio_mcp_startup_error_allows_http_servers() { + let sandbox_state = SandboxState { + sandbox_policy: SandboxPolicy::new_read_only_policy(), + codex_linux_sandbox_exe: None, + sandbox_cwd: PathBuf::from("/tmp"), + use_legacy_landlock: false, + }; + let transport = McpServerTransportConfig::StreamableHttp { + url: "https://example.com/mcp".to_string(), + bearer_token_env_var: None, + http_headers: None, + env_http_headers: None, + }; + + assert_eq!( + sandboxed_stdio_mcp_startup_error(&transport, &sandbox_state, true), + None + ); +} + +#[test] +fn sandboxed_stdio_mcp_startup_error_allows_stdio_without_windows_sandbox() { + let sandbox_state = SandboxState { + sandbox_policy: SandboxPolicy::DangerFullAccess, + codex_linux_sandbox_exe: None, + sandbox_cwd: PathBuf::from("/tmp"), + use_legacy_landlock: false, + }; + let transport = McpServerTransportConfig::Stdio { + command: "bridge-server".to_string(), + args: Vec::new(), + env: None, + env_vars: Vec::new(), + cwd: None, + }; + + assert_eq!( + sandboxed_stdio_mcp_startup_error(&transport, &sandbox_state, true), + None + ); +}