diff --git a/codex-rs/app-server/src/request_processors/command_exec_processor.rs b/codex-rs/app-server/src/request_processors/command_exec_processor.rs index d2606e033e..b96b4187d3 100644 --- a/codex-rs/app-server/src/request_processors/command_exec_processor.rs +++ b/codex-rs/app-server/src/request_processors/command_exec_processor.rs @@ -1,5 +1,6 @@ use super::*; use codex_core::exec_env::inject_apply_patch_env; +use codex_core::windows_sandbox::local_binding_policy_for_sandbox; use codex_core::windows_sandbox::managed_proxy_routing_for_windows_sandbox; use codex_protocol::shell_environment::is_non_inheritable_env_var; @@ -266,6 +267,10 @@ impl CommandExecRequestProcessor { managed_proxy_routing_for_windows_sandbox( self.config.permissions.windows_sandbox_type, ), + local_binding_policy_for_sandbox( + self.config.permissions.windows_sandbox_type, + Some(std::env::consts::OS), + ), /*policy_decider*/ None, /*blocked_request_observer*/ None, managed_network_requirements_enabled, diff --git a/codex-rs/cli/src/debug_sandbox.rs b/codex-rs/cli/src/debug_sandbox.rs index f216e4df2e..1d906306e1 100644 --- a/codex-rs/cli/src/debug_sandbox.rs +++ b/codex-rs/cli/src/debug_sandbox.rs @@ -19,6 +19,7 @@ use codex_core::exec_env::create_env; #[cfg(target_os = "macos")] use codex_core::spawn::CODEX_SANDBOX_ENV_VAR; use codex_core::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR; +use codex_core::windows_sandbox::local_binding_policy_for_sandbox; use codex_core::windows_sandbox::managed_proxy_routing_for_windows_sandbox; use codex_protocol::config_types::SandboxMode; use codex_protocol::models::PermissionProfile; @@ -356,6 +357,10 @@ async fn run_command_under_sandbox( spec.start_proxy( &permission_profile, managed_proxy_routing_for_windows_sandbox(config.permissions.windows_sandbox_type), + local_binding_policy_for_sandbox( + config.permissions.windows_sandbox_type, + Some(std::env::consts::OS), + ), /*policy_decider*/ None, /*blocked_request_observer*/ None, managed_network_requirements_enabled, diff --git a/codex-rs/config/src/config_requirements.rs b/codex-rs/config/src/config_requirements.rs index 25ba174363..d1f6ab1033 100644 --- a/codex-rs/config/src/config_requirements.rs +++ b/codex-rs/config/src/config_requirements.rs @@ -435,6 +435,7 @@ pub struct NetworkRequirementsToml { /// network enforcement is active. User allowlist entries are ignored. pub managed_allowed_domains_only: Option, pub unix_sockets: Option, + /// MXC requires true when managed networking is enabled; an explicit false is rejected. pub allow_local_binding: Option, /// Requirements-only header injections. These annotate matching requests /// without changing whether non-matching requests are allowed. diff --git a/codex-rs/config/src/permissions_toml.rs b/codex-rs/config/src/permissions_toml.rs index 48c54ef71c..8853f73db2 100644 --- a/codex-rs/config/src/permissions_toml.rs +++ b/codex-rs/config/src/permissions_toml.rs @@ -340,6 +340,9 @@ pub struct NetworkToml { pub mode: Option, pub domains: Option, pub unix_sockets: Option, + /// Permits local servers and direct host-loopback connections and skips the proxy's + /// additional private-network destination checks. Proxy domain rules still apply. + /// Defaults to true for MXC, which cannot enforce false; otherwise defaults to false. pub allow_local_binding: Option, pub mitm: Option, } @@ -515,7 +518,7 @@ impl NetworkToml { config.unix_sockets = Some(proxy_unix_sockets); } if let Some(allow_local_binding) = self.allow_local_binding { - config.allow_local_binding = allow_local_binding; + config.allow_local_binding = Some(allow_local_binding); } if let Some(mitm) = self.mitm.as_ref() { config.mitm_hooks = mitm.to_runtime_hooks(mitm.actions.as_ref()); diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index be933571f5..164ef7093e 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -2817,6 +2817,7 @@ "additionalProperties": false, "properties": { "allow_local_binding": { + "description": "Permits local servers and direct host-loopback connections and skips the proxy's additional private-network destination checks. Proxy domain rules still apply. Defaults to true for MXC, which cannot enforce false; otherwise defaults to false.", "type": "boolean" }, "allow_upstream_proxy": { @@ -2895,6 +2896,7 @@ "additionalProperties": false, "properties": { "allow_local_binding": { + "description": "Permits local servers and direct host-loopback connections and skips the proxy's additional private-network destination checks. Proxy domain rules still apply. Defaults to true for MXC, which cannot enforce false; otherwise defaults to false.", "type": "boolean" }, "allow_upstream_proxy": { diff --git a/codex-rs/core/src/config/network_config.rs b/codex-rs/core/src/config/network_config.rs index 653717e5d3..6de02ee17f 100644 --- a/codex-rs/core/src/config/network_config.rs +++ b/codex-rs/core/src/config/network_config.rs @@ -67,6 +67,7 @@ pub fn validate_environment_network_policy( policy, permission_profile, &Policy::empty(), + codex_network_proxy::LocalBindingPolicy::DefaultFalse, ) .and_then(|spec| spec.build_config_state_for_spec(executor_os).map(|_| ())) .map_err(|_| EnvironmentNetworkConfigError) diff --git a/codex-rs/core/src/config/network_proxy_spec.rs b/codex-rs/core/src/config/network_proxy_spec.rs index d324226d4d..f2a95fdb0d 100644 --- a/codex-rs/core/src/config/network_proxy_spec.rs +++ b/codex-rs/core/src/config/network_proxy_spec.rs @@ -5,6 +5,7 @@ use codex_network_proxy::ConfigReloader; use codex_network_proxy::ConfigReloaderFuture; use codex_network_proxy::ConfigState; use codex_network_proxy::EnvironmentNetworkPolicy; +use codex_network_proxy::LocalBindingPolicy; use codex_network_proxy::ManagedProxyRouting; use codex_network_proxy::NetworkDecision; use codex_network_proxy::NetworkPolicyDecider; @@ -111,7 +112,7 @@ impl NetworkProxySpec { #[cfg(any(target_os = "windows", test))] pub(crate) fn allow_local_binding(&self) -> bool { - self.config.allow_local_binding + self.config.allow_local_binding() } /// Returns the firewall settings and listener identities used by the Windows provisioning service. @@ -145,7 +146,7 @@ impl NetworkProxySpec { Ok(( codex_windows_sandbox::WindowsSandboxProvisioningSettings { proxy_ports, - allow_local_binding: self.config.allow_local_binding, + allow_local_binding: self.config.allow_local_binding(), }, codex_windows_sandbox::WindowsSandboxProxyListeners { http_ports: vec![http_port], @@ -188,16 +189,22 @@ impl NetworkProxySpec { }) } + #[allow(clippy::too_many_arguments)] pub async fn start_proxy( &self, permission_profile: &PermissionProfile, managed_proxy_routing: ManagedProxyRouting, + local_binding_policy: LocalBindingPolicy, policy_decider: Option>, blocked_request_observer: Option>, enable_network_approval_flow: bool, audit_metadata: NetworkProxyAuditMetadata, ) -> std::io::Result { - let state = self.build_state_with_audit_metadata(audit_metadata, Platform::native())?; + let state = self.build_state_with_audit_metadata( + audit_metadata, + Platform::native(), + local_binding_policy, + )?; let mut builder = NetworkProxy::builder() .state(Arc::new(state)) .managed_proxy_routing(managed_proxy_routing); @@ -248,6 +255,7 @@ impl NetworkProxySpec { policy: &EnvironmentNetworkPolicy, permission_profile: &PermissionProfile, exec_policy: &Policy, + local_binding_policy: LocalBindingPolicy, ) -> std::io::Result { if matches!(permission_profile, PermissionProfile::Disabled) { return Err(std::io::Error::new( @@ -268,13 +276,14 @@ impl NetworkProxySpec { enabled: true, // Without a controller, the owner supplies the entire permission ceiling. dangerously_allow_all_unix_sockets: Some(true), - allow_local_binding: true, + allow_local_binding: Some(true), ..NetworkProxyConfig::default() }, /*requirements*/ None, permission_profile, )?, }; + spec.config.allow_local_binding = Some(local_binding_policy.resolve(&spec.config)); policy.apply_to(&mut spec.config); let protected_denials = spec.config.denied_domains().unwrap_or_default(); @@ -353,6 +362,7 @@ impl NetworkProxySpec { &self, audit_metadata: NetworkProxyAuditMetadata, executor_os: Platform, + local_binding_policy: LocalBindingPolicy, ) -> std::io::Result { let state = self.build_config_state_for_spec(executor_os)?; let reloader = Arc::new(StaticNetworkProxyReloader::new(state.clone())); @@ -360,6 +370,7 @@ impl NetworkProxySpec { state, reloader, audit_metadata, + local_binding_policy, )) } @@ -468,7 +479,7 @@ impl NetworkProxySpec { constraints.allow_unix_sockets = Some(allow_unix_sockets); } if let Some(allow_local_binding) = requirements.allow_local_binding { - config.allow_local_binding = allow_local_binding; + config.allow_local_binding = Some(allow_local_binding); constraints.allow_local_binding = Some(allow_local_binding); } diff --git a/codex-rs/core/src/config/network_proxy_spec_tests.rs b/codex-rs/core/src/config/network_proxy_spec_tests.rs index 996c86743d..16ea7de91e 100644 --- a/codex-rs/core/src/config/network_proxy_spec_tests.rs +++ b/codex-rs/core/src/config/network_proxy_spec_tests.rs @@ -5,6 +5,7 @@ use codex_config::NetworkDomainPermissionToml; use codex_config::NetworkDomainPermissionsToml; use codex_execpolicy::Decision::Allow; use codex_execpolicy::NetworkRuleProtocol::Https; +use codex_network_proxy::LocalBindingPolicy::DefaultFalse; use codex_network_proxy::NetworkDomainPermission; use codex_network_proxy::NetworkUnixSocketPermission; use codex_network_proxy::NetworkUnixSocketPermissions; @@ -78,15 +79,18 @@ async fn attachment_socket_grants_respect_configured_restrictions_at_remote_laun &owner, &profile, &Policy::empty(), + DefaultFalse, )?; assert_eq!( composed.config.dangerously_allow_all_unix_sockets, Some(expected), "{name}" ); - let state = Arc::new( - controller.build_state_with_audit_metadata(Default::default(), Platform::Linux)?, - ); + let state = Arc::new(controller.build_state_with_audit_metadata( + Default::default(), + Platform::Linux, + DefaultFalse, + )?); let proxy = NetworkProxy::builder() .state(Arc::clone(&state)) .managed_by_codex(false) @@ -99,7 +103,7 @@ async fn attachment_socket_grants_respect_configured_restrictions_at_remote_laun Some(composed.environment_policy()), /*fallback_policy_decider*/ None, )?; - let launch = scoped.remote_launch_config().await?; + let launch = scoped.remote_launch_config(DefaultFalse).await?; assert_eq!( launch.proxy.dangerously_allow_all_unix_sockets, expected, "{name}" @@ -115,7 +119,7 @@ async fn attachment_socket_grants_respect_configured_restrictions_at_remote_laun // Inheritance must not grant sockets to ordinary controller-only commands. assert!( !proxy - .remote_launch_config() + .remote_launch_config(DefaultFalse) .await? .proxy .dangerously_allow_all_unix_sockets @@ -123,7 +127,7 @@ async fn attachment_socket_grants_respect_configured_restrictions_at_remote_laun state.add_allowed_domain("granted.example").await?; assert!( scoped - .remote_launch_config() + .remote_launch_config(DefaultFalse) .await? .proxy .dangerously_allow_all_unix_sockets @@ -144,7 +148,7 @@ async fn attachment_socket_grants_respect_configured_restrictions_at_remote_laun .await?; assert!( !scoped - .remote_launch_config() + .remote_launch_config(DefaultFalse) .await? .proxy .dangerously_allow_all_unix_sockets @@ -171,7 +175,7 @@ fn build_state_with_audit_metadata_threads_metadata_to_state() { }; let state = spec - .build_state_with_audit_metadata(metadata.clone(), Platform::Linux) + .build_state_with_audit_metadata(metadata.clone(), Platform::Linux, DefaultFalse) .expect("state should build"); assert_eq!(state.audit_metadata(), &metadata); } @@ -184,7 +188,7 @@ fn windows_sandbox_proxy_listeners_preserve_effective_protocol_roles() { enabled: true, proxy_url: "http://127.0.0.1:48081".to_string(), socks_url: "socks5h://127.0.0.1:3128".to_string(), - allow_local_binding: true, + allow_local_binding: Some(true), ..NetworkProxyConfig::default() }, /*requirements*/ None, @@ -250,7 +254,7 @@ fn environment_policy_replaces_soft_controller_allowlist_and_preserves_denials() "/tmp/allowed.sock".to_string(), ]); owner.dangerously_allow_all_unix_sockets = Some(true); - owner.allow_local_binding = true; + owner.allow_local_binding = Some(true); let owner_policy = EnvironmentNetworkPolicy::from_config(&owner, /*managed_allowed_domains_only*/ false); assert_eq!( @@ -265,7 +269,12 @@ fn environment_policy_replaces_soft_controller_allowlist_and_preserves_denials() ), Err(EnvironmentNetworkConfigError) ); - let compose = NetworkProxySpec::for_environment; + let compose = |controller: Option<&NetworkProxySpec>, + policy: &EnvironmentNetworkPolicy, + profile: &PermissionProfile, + rules: &Policy| { + NetworkProxySpec::for_environment(controller, policy, profile, rules, DefaultFalse) + }; let empty = Policy::empty(); let disabled_controller = NetworkProxySpec::from_config_and_constraints( NetworkProxyConfig::default(), @@ -297,7 +306,7 @@ fn environment_policy_replaces_soft_controller_allowlist_and_preserves_denials() owner.unix_sockets.clone_from(&spec.config.unix_sockets); owner.allow_upstream_proxy = false; owner.dangerously_allow_all_unix_sockets = Some(false); - owner.allow_local_binding = false; + owner.allow_local_binding = Some(false); assert_eq!( restricted.environment_policy(), EnvironmentNetworkPolicy::from_config(&owner, /*managed_allowed_domains_only*/ false) diff --git a/codex-rs/core/src/session/environment.rs b/codex-rs/core/src/session/environment.rs index 77e3217ac3..d77d9df3da 100644 --- a/codex-rs/core/src/session/environment.rs +++ b/codex-rs/core/src/session/environment.rs @@ -67,6 +67,7 @@ fn validate_environment_config( policy, config.permission_profile.permission_profile(), &Policy::empty(), + codex_network_proxy::LocalBindingPolicy::DefaultFalse, ) .map_err(|error| { CodexErr::InvalidRequest(format!("invalid environment network policy: {error}")) diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index cd5d7b50b6..7aa25b9fe4 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -335,6 +335,7 @@ use crate::turn_timing::TurnTimingState; use crate::turn_timing::record_turn_ttfm_metric; use crate::unified_exec::UnifiedExecProcessManager; use crate::windows_sandbox::WindowsSandboxLevelExt; +use crate::windows_sandbox::local_binding_policy_for_sandbox; use crate::windows_sandbox::managed_proxy_routing_for_windows_sandbox; use codex_core_plugins::PluginCommandAttribution; use codex_core_plugins::PluginsManager; @@ -1221,6 +1222,7 @@ impl Session { .start_proxy( permission_profile, managed_proxy_routing_for_windows_sandbox(windows_sandbox_type), + local_binding_policy_for_sandbox(windows_sandbox_type, Some(std::env::consts::OS)), network_policy_decider, blocked_request_observer, managed_network_requirements_enabled, diff --git a/codex-rs/core/src/shell_snapshot_tests.rs b/codex-rs/core/src/shell_snapshot_tests.rs index 8c5681e022..f27d8e4980 100644 --- a/codex-rs/core/src/shell_snapshot_tests.rs +++ b/codex-rs/core/src/shell_snapshot_tests.rs @@ -239,6 +239,7 @@ async fn inactive_profiles_keep_snapshots_but_active_brokers_require_sandbox() - .start_proxy( &permission_profile, codex_network_proxy::ManagedProxyRouting::SharedIngress, + codex_network_proxy::LocalBindingPolicy::DefaultFalse, /*policy_decider*/ None, /*blocked_request_observer*/ None, /*enable_network_approval_flow*/ false, @@ -627,7 +628,7 @@ async fn snapshot_discovers_and_redacts_shell_initialized_credentials() -> Resul let mut network_config = NetworkProxyConfig::default(); network_config.set_credential_broker_enabled(/*enabled*/ true); - network_config.allow_local_binding = true; + network_config.allow_local_binding = Some(true); network_config.credential_providers.insert( "local".to_string(), CredentialProviderConfig { @@ -670,6 +671,7 @@ async fn snapshot_discovers_and_redacts_shell_initialized_credentials() -> Resul .start_proxy( &permission_profile, codex_network_proxy::ManagedProxyRouting::SharedIngress, + codex_network_proxy::LocalBindingPolicy::DefaultFalse, /*policy_decider*/ None, /*blocked_request_observer*/ None, /*enable_network_approval_flow*/ false, diff --git a/codex-rs/core/src/tools/network_approval.rs b/codex-rs/core/src/tools/network_approval.rs index 3b03c31831..71db3b5695 100644 --- a/codex-rs/core/src/tools/network_approval.rs +++ b/codex-rs/core/src/tools/network_approval.rs @@ -1117,6 +1117,9 @@ pub(crate) async fn begin_network_approval( } let controller = turn.config.permissions.network.as_ref(); + let environment = environments + .turn_environments() + .find(|environment| environment.selection.environment_id == environment_id); let owner_spec = network_policy .as_ref() .map(|policy| { @@ -1126,6 +1129,15 @@ pub(crate) async fn begin_network_approval( policy, &permission_profile, session.services.exec_policy.current().as_ref(), + environment.map_or( + codex_network_proxy::LocalBindingPolicy::DefaultFalse, + |environment| { + crate::windows_sandbox::local_binding_policy_for_sandbox( + environment.config().windows_sandbox_type, + environment.executor_platform_os.as_deref(), + ) + }, + ), ) }) .transpose() @@ -1153,15 +1165,13 @@ pub(crate) async fn begin_network_approval( } else { // This carrier never listens: the executor starts the real per-command proxy. let executor_os = Platform::from_platform_os( - environments - .turn_environments() - .find(|environment| environment.selection.environment_id == environment_id) - .and_then(|environment| environment.executor_platform_os.as_deref()), + environment.and_then(|environment| environment.executor_platform_os.as_deref()), ); let state = owner_spec .build_state_with_audit_metadata( session.services.network_proxy_audit_metadata.clone(), executor_os, + codex_network_proxy::LocalBindingPolicy::DefaultFalse, ) .map_err(|error| { ToolError::Rejected(format!( diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index 1ca70bc579..1e9cfb6b31 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -354,13 +354,21 @@ impl<'a> ToolRuntime for UnifiedExecRunt }; let (mut env, managed_network_context, network_proxy_launch) = match managed_network { Some(network) if environment_is_remote => { - let mut launch = network.remote_launch_config().await.map_err(|err| { - ToolError::Codex(CodexErr::Io(io::Error::other(err.to_string()))) - })?; + let mut launch = network + .remote_launch_config(crate::windows_sandbox::local_binding_policy_for_sandbox( + req.turn_environment.config().windows_sandbox_type, + req.turn_environment.executor_platform_os.as_deref(), + )) + .await + .map_err(|err| { + ToolError::Codex(CodexErr::Io(io::Error::other(err.to_string()))) + })?; if routes_approval_policy_to_guardian( ctx.step_context.settings.approval_policy(), ctx.step_context.settings.approvals_reviewer(), - ) && network.remote_policy_decider().is_some() + ) && network + .remote_policy_decider(launch.proxy.allow_local_binding) + .is_some() { let timeout = ctx .session diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index 015f7c9c60..ef18be3447 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -1278,7 +1278,7 @@ impl UnifiedExecProcessManager { let network_policy_decider = network_proxy_launch .as_ref() .filter(|launch| launch.policy_decision_timeout_ms.is_some()) - .and_then(|_| network.and_then(NetworkProxy::remote_policy_decider)); + .and_then(|launch| network?.remote_policy_decider(launch.proxy.allow_local_binding)); if environment.is_remote() && network.is_some() && network_proxy_launch.is_none() { request.exec_server_enforce_managed_network = false; } diff --git a/codex-rs/core/src/windows_sandbox.rs b/codex-rs/core/src/windows_sandbox.rs index 859f405284..5213e3d1ba 100644 --- a/codex-rs/core/src/windows_sandbox.rs +++ b/codex-rs/core/src/windows_sandbox.rs @@ -18,6 +18,18 @@ use std::path::Path; use std::path::PathBuf; use std::time::Instant; +/// Selects local binding policy from the sandbox and executor OS. +pub fn local_binding_policy_for_sandbox( + sandbox_type: SandboxType, + platform_os: Option<&str>, +) -> codex_network_proxy::LocalBindingPolicy { + if sandbox_type == SandboxType::WindowsMxc && platform_os == Some("windows") { + codex_network_proxy::LocalBindingPolicy::RequireTrue + } else { + codex_network_proxy::LocalBindingPolicy::DefaultFalse + } +} + pub fn managed_proxy_routing_for_windows_sandbox( sandbox_type: SandboxType, ) -> codex_network_proxy::ManagedProxyRouting { diff --git a/codex-rs/core/src/windows_sandbox_tests.rs b/codex-rs/core/src/windows_sandbox_tests.rs index a5c69db559..41b3b4afe0 100644 --- a/codex-rs/core/src/windows_sandbox_tests.rs +++ b/codex-rs/core/src/windows_sandbox_tests.rs @@ -125,7 +125,7 @@ fn provisioning_settings_omit_the_disabled_socks_proxy() { proxy_url: "http://127.0.0.1:43128".to_string(), enable_socks5: false, socks_url: "socks5h://127.0.0.1:48081".to_string(), - allow_local_binding: true, + allow_local_binding: Some(true), ..Default::default() }; let spec = crate::config::NetworkProxySpec::from_config_and_constraints( diff --git a/codex-rs/core/tests/suite/network_approval.rs b/codex-rs/core/tests/suite/network_approval.rs index ea324fb717..6dd6895ebf 100644 --- a/codex-rs/core/tests/suite/network_approval.rs +++ b/codex-rs/core/tests/suite/network_approval.rs @@ -1858,7 +1858,7 @@ async fn thread_turnover_closes_managed_proxy_tunnels() -> Result<()> { let mut network = NetworkProxyConfig { enabled: true, mode: codex_network_proxy::NetworkMode::Full, - allow_local_binding: true, + allow_local_binding: Some(true), allow_upstream_proxy: false, ..NetworkProxyConfig::default() }; @@ -2413,7 +2413,7 @@ async fn owner_network_policy_follows_the_selected_remote_command() -> Result<() NetworkProxySpec::from_config_and_constraints( NetworkProxyConfig { enabled: true, - allow_local_binding: true, + allow_local_binding: Some(true), ..NetworkProxyConfig::default() }, /*requirements*/ None, @@ -2458,7 +2458,7 @@ async fn owner_network_policy_follows_the_selected_remote_command() -> Result<() } let marker = format!("{scenario}_OWNER_{suffix}"); let mut proxy_config = NetworkProxyConfig { - allow_local_binding: true, + allow_local_binding: Some(true), ..NetworkProxyConfig::default() }; proxy_config.set_allowed_domains(vec![allowed_domain.to_string()]); diff --git a/codex-rs/core/tests/suite/network_approval_completion.rs b/codex-rs/core/tests/suite/network_approval_completion.rs index c1299b0677..d9067dd94c 100644 --- a/codex-rs/core/tests/suite/network_approval_completion.rs +++ b/codex-rs/core/tests/suite/network_approval_completion.rs @@ -1,14 +1,13 @@ -//! Exercises process cleanup while a remote network review is still pending. +//! Exercises remote network review startup and cleanup, including MXC's local-binding default. -use super::PushedExecScenario; use super::accept_initialized_exec_server; use super::read_exec_server_json; -use super::respond_environment_info; use super::send_exec_server_json; use anyhow::Context; use anyhow::Result; use base64::Engine; use base64::engine::general_purpose::STANDARD as BASE64_STANDARD; +use codex_config::test_support::CloudConfigBundleFixture; use codex_core::TurnInputRequest; use codex_protocol::config_types::ApprovalsReviewer; use codex_protocol::items::CommandExecutionStatus; @@ -16,8 +15,8 @@ use codex_protocol::items::TurnItem; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::ThreadSettingsOverrides; +use codex_protocol::sandbox::SandboxType; use codex_protocol::user_input::UserInput; -use core_test_support::managed_network_requirements_loader; use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; use core_test_support::responses::ev_function_call; @@ -34,13 +33,22 @@ use serde_json::Value; use serde_json::json; use std::fs; use std::time::Duration; +use test_case::test_case; use tokio::net::TcpListener; use tokio::net::TcpStream; use tokio::time::timeout; use tokio_tungstenite::WebSocketStream; +#[test_case(SandboxType::WindowsRestrictedToken, "allow_local_binding = true", Ok(true); "explicit_local_binding")] +#[test_case(SandboxType::WindowsMxc, "", Ok(true); "mxc_default_local_binding")] +#[test_case(SandboxType::WindowsMxc, "allow_local_binding = false", Err("MXC cannot enforce allow_local_binding=false"); "mxc_rejects_false")] +#[test_case(SandboxType::WindowsRestrictedToken, "", Ok(false); "non_mxc_default_local_binding")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn completed_remote_process_withdraws_pending_network_review() -> Result<()> { +async fn remote_local_binding_policy_and_review_cleanup( + sandbox_type: SandboxType, + local_binding: &'static str, + expected_local_binding: Result, +) -> Result<()> { let server = start_mock_server().await; let listener = TcpListener::bind("127.0.0.1:0").await?; let exec_server_url = format!("ws://{}", listener.local_addr()?); @@ -99,15 +107,26 @@ async fn completed_remote_process_withdraws_pending_network_review() -> Result<( .as_u64() .is_some() ); + let allow_local_binding = + expected_local_binding.expect("invalid MXC config must not launch"); + assert_eq!( + start["params"]["networkProxy"]["proxy"]["allowLocalBinding"], + allow_local_binding + ); let process_id = &start["params"]["processId"]; send_exec_server_json( &mut websocket, json!({"id": start["id"], "result": {"processId": process_id}}), ) .await; + let host = if allow_local_binding { + "10.0.0.1" + } else { + "8.8.8.8" + }; send_exec_server_json(&mut websocket, json!({ "id": 1, "method": "network/policyRequest", - "params": {"processId": process_id, "request": {"protocol": "https_connect", "host": "review.example", "port": 443}} + "params": {"processId": process_id, "request": {"protocol": "https_connect", "host": host, "port": 443}} })).await; timeout(Duration::from_secs(/*secs*/ 10), async { while !pending_review.requests().iter().any(|request| { @@ -155,24 +174,31 @@ async fn completed_remote_process_withdraws_pending_network_review() -> Result<( }); let test = test_codex() .with_exec_server_url(exec_server_url) - .with_cloud_config_bundle(managed_network_requirements_loader()) - .with_pre_build_hook(|home| { + .with_cloud_config_bundle( + CloudConfigBundleFixture::loader_with_enterprise_requirement(format!( + "[experimental_network]\nenabled = true\n{local_binding}" + )), + ) + .with_pre_build_hook(move |home| { fs::write( home.join("config.toml"), - r#"default_permissions = "workspace" + format!( + r#"default_permissions = "workspace" [permissions.workspace.filesystem] ":minimal" = "read" [permissions.workspace.network] enabled = true mode = "full" -allow_local_binding = true -"#, +{local_binding} +"# + ), ) .expect("managed network config"); }) - .with_config(|config| { + .with_config(move |config| { config.project_doc_max_bytes = 0; config.approvals_reviewer = ApprovalsReviewer::AutoReview; + config.permissions.windows_sandbox_type = sandbox_type; #[cfg(windows)] config.set_windows_sandbox_enabled(/*value*/ true); }) @@ -198,6 +224,21 @@ allow_local_binding = true }), ) .await?; + timeout(Duration::from_secs(/*secs*/ 15), async { + while final_response.function_call_output_text(call_id).is_none() { + tokio::time::sleep(Duration::from_millis(/*millis*/ 10)).await; + } + }) + .await?; + let output = final_response + .function_call_output_text(call_id) + .context("tool output")?; + if let Err(expected_error) = expected_local_binding { + assert!(output.contains(expected_error), "{output}"); + exec_server.abort(); + assert!(exec_server.await.is_err_and(|err| err.is_cancelled())); + return Ok(()); + } let completed = timeout(Duration::from_secs(/*secs*/ 15), async { loop { match test.codex.next_event().await?.msg { @@ -225,15 +266,6 @@ allow_local_binding = true Some("build complete\n") ) ); - timeout(Duration::from_secs(/*secs*/ 10), async { - while final_response.function_call_output_text(call_id).is_none() { - tokio::time::sleep(Duration::from_millis(/*millis*/ 10)).await; - } - }) - .await?; - let output = final_response - .function_call_output_text(call_id) - .context("tool output")?; assert!(output.contains("Process exited with code 0"), "{output}"); assert!(output.contains("build complete"), "{output}"); assert_eq!(parent.requests().len(), 1); @@ -257,7 +289,14 @@ fn review_body(request: &wiremock::Request) -> Value { async fn respond_setup_request(websocket: &mut WebSocketStream, request: &Value) { match request["method"].as_str() { Some("environment/info") => { - respond_environment_info(websocket, &request["id"], PushedExecScenario::Complete).await; + send_exec_server_json(websocket, json!({ + "id": request["id"], + "result": { + "shell": {"name": "powershell", "path": "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe"}, + "platformOs": "windows", + "capabilities": {"networkProxyLaunch": true} + } + })).await; } Some("fs/getMetadata" | "fs/readFile" | "fs/readDirectory") => { send_exec_server_json( @@ -289,6 +328,6 @@ async fn respond_setup_request(websocket: &mut WebSocketStream, reque ) .await; } - method => panic!("unexpected setup request: {method:?}"), + _ => panic!("unexpected setup request: {request}"), } } diff --git a/codex-rs/core/tests/suite/shell_snapshot.rs b/codex-rs/core/tests/suite/shell_snapshot.rs index 14030dd1df..f97dd9aeef 100644 --- a/codex-rs/core/tests/suite/shell_snapshot.rs +++ b/codex-rs/core/tests/suite/shell_snapshot.rs @@ -574,7 +574,7 @@ async fn shell_snapshot_v2_prewarm_stops_on_shutdown() -> Result<()> { }; "untrusted")] #[test_case::test_case(|config| { config.permissions.network = Some(NetworkProxySpec::from_config_and_constraints( - NetworkProxyConfig { enabled: true, allow_local_binding: true, ..Default::default() }, + NetworkProxyConfig { enabled: true, allow_local_binding: Some(true), ..Default::default() }, /*requirements*/ None, config.permissions.permission_profile(), ).expect("configure managed network")); diff --git a/codex-rs/features/src/feature_configs.rs b/codex-rs/features/src/feature_configs.rs index 1ee3f69d8d..b3c5a4bdc7 100644 --- a/codex-rs/features/src/feature_configs.rs +++ b/codex-rs/features/src/feature_configs.rs @@ -480,6 +480,9 @@ pub struct NetworkProxyConfigToml { pub domains: Option>, #[serde(skip_serializing_if = "Option::is_none")] pub unix_sockets: Option>, + /// Permits local servers and direct host-loopback connections and skips the proxy's + /// additional private-network destination checks. Proxy domain rules still apply. + /// Defaults to true for MXC, which cannot enforce false; otherwise defaults to false. #[serde(skip_serializing_if = "Option::is_none")] pub allow_local_binding: Option, #[serde(skip_serializing_if = "Option::is_none")] diff --git a/codex-rs/mxc-sandbox/README.md b/codex-rs/mxc-sandbox/README.md index b864424e4c..5c2d8c691e 100644 --- a/codex-rs/mxc-sandbox/README.md +++ b/codex-rs/mxc-sandbox/README.md @@ -46,10 +46,14 @@ to 4096 bytes; the helper removes them before native process creation. Use semantics and scan limits as the existing Windows sandbox. - Native deny paths depend on the host's capability probe. An installed Windows update alone is not treated as evidence that every policy feature is enabled. -- This adapter rejects managed networking with `allow_local_binding=false`: - its host-loopback permission is bidirectional. MXC's proxy-peer identity mode - is not integrated here. With local binding enabled, direct DNS remains denied, - matching the existing Windows sandbox. +- When MXC is the executor's selected backend, managed networking defaults + `allow_local_binding` to `true`. An effective `false` after applying managed + requirements is a configuration error: native host-loopback access + is bidirectional, and MXC's proxy-peer identity mode is not integrated here. + `true` permits local servers and direct host-loopback connections and removes + the proxy's additional private-network destination checks. Proxy domain rules + still apply to proxied traffic; direct DNS remains denied. This default also + applies to remote Windows executors and does not enable disabled networking. - Windows volume-root grants do not recurse. The adapter grants the root and its immediate children; directories added or newly mounted during a running command are not implicitly granted. diff --git a/codex-rs/network-proxy/README.md b/codex-rs/network-proxy/README.md index 75cd3966f2..a83a0df0d7 100644 --- a/codex-rs/network-proxy/README.md +++ b/codex-rs/network-proxy/README.md @@ -47,7 +47,8 @@ mode = "full" # default when unset; use "limited" for read-only mode # Hostnames that resolve to local/private IPs are still blocked even if allowlisted. # Clients that always bypass proxies for loopback, such as Go's `net/http`, remain blocked by # the operating-system sandbox when local binding is disabled. -allow_local_binding = false +# Omitted: true for MXC, false elsewhere. MXC rejects an effective false. +# allow_local_binding = false # DANGEROUS (macOS-only): bypasses unix socket allowlisting and permits any # absolute socket path from `x-unix-socket`. diff --git a/codex-rs/network-proxy/src/brokered_tunnel_tests.rs b/codex-rs/network-proxy/src/brokered_tunnel_tests.rs index b416e59ed2..7ffd36c6e5 100644 --- a/codex-rs/network-proxy/src/brokered_tunnel_tests.rs +++ b/codex-rs/network-proxy/src/brokered_tunnel_tests.rs @@ -159,7 +159,7 @@ impl TestProxy { mode, mitm: true, credential_broker: true, - allow_local_binding: true, + allow_local_binding: Some(true), credential_providers: BTreeMap::from([( "local".to_string(), CredentialProviderConfig { diff --git a/codex-rs/network-proxy/src/config.rs b/codex-rs/network-proxy/src/config.rs index 5e479cd0d0..dfbdaa1b91 100644 --- a/codex-rs/network-proxy/src/config.rs +++ b/codex-rs/network-proxy/src/config.rs @@ -139,7 +139,9 @@ pub struct NetworkProxyConfig { pub domains: Option, #[serde(default)] pub unix_sockets: Option, - pub allow_local_binding: bool, + /// Omission is resolved for the selected sandbox; ordinary proxy execution defaults to false. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub allow_local_binding: Option, #[serde(default)] pub mitm: bool, #[serde(default)] @@ -175,7 +177,7 @@ impl Default for NetworkProxyConfig { mode: NetworkMode::default(), domains: None, unix_sockets: None, - allow_local_binding: false, + allow_local_binding: None, mitm: false, credential_broker: false, credential_broker_enabled_mitm: false, @@ -189,6 +191,10 @@ impl Default for NetworkProxyConfig { } impl NetworkProxyConfig { + pub fn allow_local_binding(&self) -> bool { + self.allow_local_binding.unwrap_or(false) + } + pub fn set_credential_broker_enabled(&mut self, enabled: bool) { self.credential_broker = enabled; if enabled { @@ -710,7 +716,7 @@ mod tests { mode: NetworkMode::Full, domains: None, unix_sockets: None, - allow_local_binding: false, + allow_local_binding: None, mitm: false, credential_broker: false, credential_broker_enabled_mitm: false, @@ -905,7 +911,6 @@ mod tests { "example.com": "deny", }, "unix_sockets": null, - "allow_local_binding": false, "mitm": false, "credential_broker": false, "dangerously_allow_plaintext_credential_injection": false, diff --git a/codex-rs/network-proxy/src/connect_policy.rs b/codex-rs/network-proxy/src/connect_policy.rs index 47430a1929..eab3df9831 100644 --- a/codex-rs/network-proxy/src/connect_policy.rs +++ b/codex-rs/network-proxy/src/connect_policy.rs @@ -178,7 +178,7 @@ mod tests { let target = listener.local_addr().expect("local addr"); let connector = TargetCheckedTcpConnector::new(Arc::new(network_proxy_state_for_policy( NetworkProxyConfig { - allow_local_binding: true, + allow_local_binding: Some(true), ..NetworkProxyConfig::default() }, ))); diff --git a/codex-rs/network-proxy/src/connection_lifecycle/lifecycle_tests.rs b/codex-rs/network-proxy/src/connection_lifecycle/lifecycle_tests.rs index 60be13c30f..09c4606b62 100644 --- a/codex-rs/network-proxy/src/connection_lifecycle/lifecycle_tests.rs +++ b/codex-rs/network-proxy/src/connection_lifecycle/lifecycle_tests.rs @@ -71,7 +71,7 @@ async fn canceling_wait_closes_proxy_connections() -> Result<()> { async fn assert_stop_closes_connections(stop: StopProxy) -> Result<()> { let mut config = NetworkProxyConfig { - allow_local_binding: true, + allow_local_binding: Some(true), allow_upstream_proxy: false, enable_socks5_udp: false, ..NetworkProxyConfig::default() diff --git a/codex-rs/network-proxy/src/credential_broker.rs b/codex-rs/network-proxy/src/credential_broker.rs index 39b545f33c..bc3e2ffda5 100644 --- a/codex-rs/network-proxy/src/credential_broker.rs +++ b/codex-rs/network-proxy/src/credential_broker.rs @@ -287,14 +287,14 @@ impl CredentialBroker { .then(|| active_credential_sources(&state, &HashMap::new())); if state.enabled != config.credential_broker || state.openai_api_host != config.credential_broker_openai_host - || state.allow_local_binding != config.allow_local_binding + || state.allow_local_binding != config.allow_local_binding() || state.configured_provider_configs != config.credential_providers || previous_context_sources.is_some() { state.config_revision += 1; } state.context.clone_from(&config.credential_broker_context); - state.allow_local_binding = config.allow_local_binding; + state.allow_local_binding = config.allow_local_binding(); if state.enabled != config.credential_broker { state.enabled = config.credential_broker; state.credentials.clear(); diff --git a/codex-rs/network-proxy/src/credential_broker/configured_tests.rs b/codex-rs/network-proxy/src/credential_broker/configured_tests.rs index 4578e55d27..f23f0f6c11 100644 --- a/codex-rs/network-proxy/src/credential_broker/configured_tests.rs +++ b/codex-rs/network-proxy/src/credential_broker/configured_tests.rs @@ -280,7 +280,7 @@ fn local_proxy_bypass_preserves_credentials_and_aliases_across_reload() { assert_ne!(dummy, token); assert_ne!(github_dummy, github); - config.allow_local_binding = true; + config.allow_local_binding = Some(true); let revision = broker.config_revision(); broker.configure(&config); assert_eq!(broker.config_revision(), revision + 1); @@ -305,7 +305,7 @@ fn local_proxy_bypass_preserves_credentials_and_aliases_across_reload() { } ); } - config.allow_local_binding = false; + config.allow_local_binding = Some(false); let mut snapshot_env = env.clone(); broker.virtualize_snapshot_env(&mut snapshot_env, /*environment_id*/ None); assert_eq!(snapshot_env["VENDOR_TOKEN"], dummy); @@ -325,7 +325,7 @@ fn child_alias_identity_survives_scoped_dummies_and_partial_direct_restoration() let broker = CredentialBroker::new(/*enabled*/ true); broker.configure(&NetworkProxyConfig { credential_broker: true, - allow_local_binding, + allow_local_binding: Some(allow_local_binding), credential_providers: BTreeMap::from([( "local".to_string(), CredentialProviderConfig { @@ -386,7 +386,7 @@ fn local_proxy_bypass_is_scoped_for_inherited_credentials_and_aliases() { let broker = CredentialBroker::new(/*enabled*/ true); broker.configure(&NetworkProxyConfig { credential_broker: true, - allow_local_binding: true, + allow_local_binding: Some(true), credential_providers: BTreeMap::from([( "vendor".to_string(), CredentialProviderConfig { diff --git a/codex-rs/network-proxy/src/environment_policy.rs b/codex-rs/network-proxy/src/environment_policy.rs index c375a90553..ea5d4c8507 100644 --- a/codex-rs/network-proxy/src/environment_policy.rs +++ b/codex-rs/network-proxy/src/environment_policy.rs @@ -30,7 +30,7 @@ impl EnvironmentNetworkPolicy { dangerously_allow_all_unix_sockets: config .dangerously_allow_all_unix_sockets .unwrap_or(false), - allow_local_binding: config.allow_local_binding, + allow_local_binding: config.allow_local_binding(), managed_allowed_domains_only, } } @@ -87,6 +87,6 @@ impl EnvironmentNetworkPolicy { config.dangerously_allow_all_unix_sockets = Some(inherited_permits_all && owner_permits_all); config.allow_upstream_proxy &= self.allow_upstream_proxy; - config.allow_local_binding &= self.allow_local_binding; + config.allow_local_binding = Some(config.allow_local_binding() && self.allow_local_binding); } } diff --git a/codex-rs/network-proxy/src/http_proxy.rs b/codex-rs/network-proxy/src/http_proxy.rs index 52263eafcf..96163abf3c 100644 --- a/codex-rs/network-proxy/src/http_proxy.rs +++ b/codex-rs/network-proxy/src/http_proxy.rs @@ -1264,7 +1264,7 @@ mod tests { async fn http_connect_accept_allows_allowlisted_host_in_full_mode() { let policy = { let mut policy = NetworkProxyConfig { - allow_local_binding: true, + allow_local_binding: Some(true), ..NetworkProxyConfig::default() }; policy.set_allowed_domains(vec!["example.com".to_string()]); @@ -1562,7 +1562,7 @@ mod tests { ..NetworkProxyConfig::default() }; network.set_allowed_domains(vec!["127.0.0.1".to_string()]); - network.allow_local_binding = true; + network.allow_local_binding = Some(true); network })); let mut env = HashMap::from([ @@ -1639,7 +1639,7 @@ mod tests { let state = Arc::new(network_proxy_state_for_policy({ let mut network = NetworkProxyConfig { - allow_local_binding: true, + allow_local_binding: Some(true), mitm: true, mitm_hooks: vec![crate::mitm_hook::MitmHookConfig { host: "127.0.0.1".to_string(), diff --git a/codex-rs/network-proxy/src/lib.rs b/codex-rs/network-proxy/src/lib.rs index 3e7a9d6c4d..bd4307f34b 100644 --- a/codex-rs/network-proxy/src/lib.rs +++ b/codex-rs/network-proxy/src/lib.rs @@ -114,6 +114,7 @@ pub use runtime::BlockedRequestObserverFuture; pub use runtime::ConfigReloader; pub use runtime::ConfigReloaderFuture; pub use runtime::ConfigState; +pub use runtime::LocalBindingPolicy; pub use runtime::NetworkProxyState; pub use state::NetworkProxyAuditMetadata; pub use state::NetworkProxyConstraintError; diff --git a/codex-rs/network-proxy/src/mitm_tests.rs b/codex-rs/network-proxy/src/mitm_tests.rs index ce02f0e013..ccbf006bd8 100644 --- a/codex-rs/network-proxy/src/mitm_tests.rs +++ b/codex-rs/network-proxy/src/mitm_tests.rs @@ -124,7 +124,7 @@ async fn mitm_policy_rechecks_local_private_target_after_connect() { let app_state = Arc::new(network_proxy_state_for_policy({ let mut network = NetworkProxyConfig::default(); network.set_allowed_domains(vec!["example.com".to_string()]); - network.allow_local_binding = false; + network.allow_local_binding = Some(false); network })); let ctx = policy_ctx( diff --git a/codex-rs/network-proxy/src/network_policy.rs b/codex-rs/network-proxy/src/network_policy.rs index 49ba623123..1af3d48075 100644 --- a/codex-rs/network-proxy/src/network_policy.rs +++ b/codex-rs/network-proxy/src/network_policy.rs @@ -683,7 +683,12 @@ mod tests { let reloader = Arc::new(StaticReloader { state: state.clone(), }); - NetworkProxyState::with_reloader_and_audit_metadata(state, reloader, metadata) + NetworkProxyState::with_reloader_and_audit_metadata( + state, + reloader, + metadata, + crate::LocalBindingPolicy::DefaultFalse, + ) } fn is_rfc3339_utc_millis(timestamp: &str) -> bool { @@ -1078,7 +1083,7 @@ mod tests { let state = network_proxy_state_for_policy({ let mut network = NetworkProxyConfig::default(); network.set_allowed_domains(vec!["example.com".to_string()]); - network.allow_local_binding = false; + network.allow_local_binding = Some(false); network }); let request = NetworkPolicyRequest::new(NetworkPolicyRequestArgs { diff --git a/codex-rs/network-proxy/src/proxy.rs b/codex-rs/network-proxy/src/proxy.rs index db5da744c8..16d01ed16f 100644 --- a/codex-rs/network-proxy/src/proxy.rs +++ b/codex-rs/network-proxy/src/proxy.rs @@ -225,6 +225,7 @@ impl NetworkProxyBuilder { .set_blocked_request_observer(self.blocked_request_observer.clone()) .await; let (current_cfg, _, executor_os) = state.current_cfg_with_brokerage_provenance().await?; + let current_cfg = state.local_binding_policy.resolved_config(¤t_cfg); #[cfg(target_os = "windows")] let (current_cfg, runtime_settings, mut windows_ingress) = { let current_cfg = config::NetworkProxyConfig { @@ -454,7 +455,7 @@ impl NetworkProxyRuntimeSettings { None }; Ok(Self { - allow_local_binding: config.allow_local_binding, + allow_local_binding: config.allow_local_binding(), allow_unix_sockets: if cfg!(target_os = "windows") { Arc::default() } else { @@ -925,12 +926,16 @@ impl NetworkProxy { } /// Captures the static inputs needed to launch a matching executor-local proxy. - pub async fn remote_launch_config(&self) -> Result { + pub async fn remote_launch_config( + &self, + local_binding_policy: crate::LocalBindingPolicy, + ) -> Result { let (mut config, brokerage_created_default_allowlist, _) = self.state.current_cfg_with_brokerage_provenance().await?; // Proxy enablement and credential brokerage remain controller-owned. let mut broker_only_config = config::NetworkProxyConfig { enabled: config.enabled, + allow_local_binding: config.allow_local_binding.filter(|binding| !binding), credential_providers: config.credential_providers.clone(), credential_broker_openai_host: config.credential_broker_openai_host.clone(), credential_broker_context: config.credential_broker_context.clone(), @@ -960,9 +965,6 @@ impl NetworkProxy { .execution_scope .as_ref() .and_then(|scope| scope.environment_policy.as_ref()); - if let Some(policy) = environment_policy { - policy.apply_to(&mut config); - } if config.credential_broker { config.enabled &= !broker_only; config.credential_broker = false; @@ -971,10 +973,20 @@ impl NetworkProxy { config.mitm = false; } } + config.allow_local_binding = Some(local_binding_policy.resolve(&config)); + if let Some(policy) = environment_policy { + policy.apply_to(&mut config); + } anyhow::ensure!( environment_policy.is_none() || config.enabled, "environment network policy requires an enabled executor proxy" ); + anyhow::ensure!( + !config.enabled + || local_binding_policy != crate::LocalBindingPolicy::RequireTrue + || config.allow_local_binding(), + "MXC cannot enforce allow_local_binding=false: native host-loopback access is bidirectional; use true or a different sandbox backend" + ); let proxy = crate::RemoteNetworkProxyConfig::from_effective_config(&config)?; tracing::debug!(network_config = ?proxy, "resolved remote execution network configuration"); let (environment_id, execution_id) = self @@ -996,8 +1008,11 @@ impl NetworkProxy { }) } - /// Returns the policy decider with trusted execution attribution restored. - pub fn remote_policy_decider(&self) -> Option> { + /// Returns the policy decider with executor local-binding policy and trusted attribution. + pub fn remote_policy_decider( + &self, + allow_local_binding: bool, + ) -> Option> { let scope = self.execution_scope.as_ref()?; self.state.for_execution_token(&scope.attribution_token)?; let decider = Arc::clone(self.policy_decider.as_ref()?); @@ -1019,7 +1034,9 @@ impl NetworkProxy { crate::NetworkDecision::deny(crate::reasons::REASON_NOT_ALLOWED) } decision = async { - match state.host_blocked(&request.host, request.port).await { + match state.host_blocked_with_local_binding( + &request.host, request.port, Some(allow_local_binding), + ).await { // Controller approval alone cannot bypass attachment policy. Ok(HostBlockDecision::Allowed) if !environment_policy_applies => { NetworkDecision::Allow @@ -1527,7 +1544,11 @@ impl NetworkProxy { new_state.config.enable_socks5_udp == current_cfg.enable_socks5_udp, "cannot update network.enable_socks5_udp on a running proxy" ); - let settings = NetworkProxyRuntimeSettings::from_config(&new_state.config)?; + let config = self + .state + .local_binding_policy + .resolved_config(&new_state.config); + let settings = NetworkProxyRuntimeSettings::from_config(&config)?; self.state.replace_config_state(new_state).await?; let mut guard = self .runtime_settings @@ -1745,6 +1766,8 @@ impl Drop for NetworkProxyHandle { #[cfg(test)] mod tests { use super::*; + use crate::LocalBindingPolicy::DefaultFalse; + use crate::LocalBindingPolicy::RequireTrue; use crate::config::NetworkProxyConfig; use crate::state::network_proxy_state_for_policy; use pretty_assertions::assert_eq; @@ -1838,7 +1861,7 @@ mod tests { ); assert_eq!(proxy.current_cfg().await?, config); - let remote = proxy.remote_launch_config().await?; + let remote = proxy.remote_launch_config(DefaultFalse).await?; assert_eq!( ( remote.proxy.unix_sockets, @@ -1862,7 +1885,7 @@ mod tests { let captured = Arc::new(Mutex::new(None)); let captured_request = Arc::clone(&captured); let mut config = NetworkProxyConfig { - allow_local_binding: true, + allow_local_binding: Some(true), ..NetworkProxyConfig::default() }; config.set_allowed_domains(vec!["controller.example".to_string()]); @@ -1895,7 +1918,7 @@ mod tests { Some(Arc::clone(&fallback_policy_decider)), )?; let decider = scoped - .remote_policy_decider() + .remote_policy_decider(/*allow_local_binding*/ true) .expect("execution-scoped proxy should expose its policy decider"); let mut request = https_request("controller.example"); request.environment_id = Some("forged-environment".to_string()); @@ -1934,7 +1957,11 @@ mod tests { owner_config.set_denied_domains(vec!["denied.example".to_string()]); assert_eq!( - scoped.remote_launch_config().await?.proxy.domains, + scoped + .remote_launch_config(DefaultFalse) + .await? + .proxy + .domains, owner_config.domains ); let strict_scoped = proxy.for_execution( @@ -1947,7 +1974,11 @@ mod tests { )), Some(fallback_policy_decider), )?; - assert!(strict_scoped.remote_policy_decider().is_none()); + assert!( + strict_scoped + .remote_policy_decider(/*allow_local_binding*/ true) + .is_none() + ); Ok(()) } @@ -1956,7 +1987,7 @@ mod tests { let decision_started = Arc::new(tokio::sync::Notify::new()); let decision_started_for_decider = Arc::clone(&decision_started); let config = NetworkProxyConfig { - allow_local_binding: true, + allow_local_binding: Some(true), ..NetworkProxyConfig::default() }; let proxy = NetworkProxy::builder() @@ -1979,7 +2010,7 @@ mod tests { /*fallback_policy_decider*/ None, )?; let decider = scoped - .remote_policy_decider() + .remote_policy_decider(/*allow_local_binding*/ true) .expect("execution-scoped proxy should expose its policy decider"); let request = https_request("pending.example"); let decision = tokio::spawn(async move { decider.decide(request).await }); @@ -2067,7 +2098,7 @@ mod tests { enabled: true, proxy_url: "http://127.0.0.1:1".to_string(), socks_url: "http://127.0.0.1:2".to_string(), - allow_local_binding: true, + allow_local_binding: Some(true), ..NetworkProxyConfig::default() })); let third = NetworkProxy::builder() @@ -2087,7 +2118,7 @@ mod tests { enabled: true, proxy_url: format!("http://{http_addr}"), socks_url: format!("http://{socks_addr}"), - allow_local_binding: true, + allow_local_binding: Some(true), ..NetworkProxyConfig::default() }, Default::default(), @@ -2149,7 +2180,7 @@ mod tests { enabled: true, proxy_url: "http://127.0.0.1:43128".to_string(), socks_url: "http://127.0.0.1:48081".to_string(), - allow_local_binding: true, + allow_local_binding: Some(true), unix_sockets: Some(crate::config::NetworkUnixSocketPermissions { entries: [ ( @@ -2335,6 +2366,7 @@ mod tests { let mut config = NetworkProxyConfig { dangerously_allow_plaintext_credential_injection: true, dangerously_allow_all_unix_sockets: Some(false), + allow_local_binding: Some(false), ..NetworkProxyConfig::default() }; config.set_allow_unix_sockets(Vec::new()); @@ -2364,7 +2396,7 @@ mod tests { /*environment_policy*/ None, /*fallback_policy_decider*/ None, )?; - let launch = scoped.remote_launch_config().await?; + let launch = scoped.remote_launch_config(RequireTrue).await?; let prepared = scoped.prepare_for_optional_environment( HashMap::from([( PROXY_ATTRIBUTION_TOKEN_ENV_KEY.to_string(), @@ -2389,7 +2421,12 @@ mod tests { )), /*fallback_policy_decider*/ None, )?; - assert!(owner_scoped.remote_launch_config().await.is_err()); + assert!( + owner_scoped + .remote_launch_config(DefaultFalse) + .await + .is_err() + ); for (enabled, mode, allowed_domain, proxy_url) in [ (false, config::NetworkMode::Full, Some("*"), None), (false, config::NetworkMode::Full, Some("example.com"), None), @@ -2418,7 +2455,7 @@ mod tests { .state(Arc::new(network_proxy_state_for_policy(config))) .build() .await?; - let launch = proxy.remote_launch_config().await?; + let launch = proxy.remote_launch_config(DefaultFalse).await?; assert!(launch.proxy.enabled); assert_eq!(launch.proxy.mode, mode); } diff --git a/codex-rs/network-proxy/src/remote_config.rs b/codex-rs/network-proxy/src/remote_config.rs index a230699489..36d313031d 100644 --- a/codex-rs/network-proxy/src/remote_config.rs +++ b/codex-rs/network-proxy/src/remote_config.rs @@ -93,7 +93,7 @@ impl RemoteNetworkProxyConfig { mode: config.mode, domains: config.domains.clone(), unix_sockets: config.unix_sockets.clone(), - allow_local_binding: config.allow_local_binding, + allow_local_binding: config.allow_local_binding(), }) } @@ -107,7 +107,7 @@ impl RemoteNetworkProxyConfig { mode: self.mode, domains: self.domains, unix_sockets: self.unix_sockets, - allow_local_binding: self.allow_local_binding, + allow_local_binding: Some(self.allow_local_binding), ..NetworkProxyConfig::default() } } diff --git a/codex-rs/network-proxy/src/remote_config_tests.rs b/codex-rs/network-proxy/src/remote_config_tests.rs index a21163478f..dbb2cdc293 100644 --- a/codex-rs/network-proxy/src/remote_config_tests.rs +++ b/codex-rs/network-proxy/src/remote_config_tests.rs @@ -2,6 +2,8 @@ use pretty_assertions::assert_eq; use super::RemoteNetworkProxyConfig; use super::RemoteNetworkProxyLaunchConfig; +use crate::LocalBindingPolicy::DefaultFalse; +use crate::LocalBindingPolicy::RequireTrue; use crate::MitmHookConfig; use crate::NetworkMode; use crate::NetworkProxy; @@ -37,6 +39,7 @@ fn optional_socket_policy_preserves_input_and_resolves_at_remote_boundary() { allow_all.unwrap_or(false) ); config.dangerously_allow_all_unix_sockets = Some(allow_all.unwrap_or(false)); + config.allow_local_binding = Some(false); assert_eq!(remote.into_network_proxy_config(), config); } } @@ -56,7 +59,7 @@ async fn round_trip_preserves_supported_effective_settings() { allow_upstream_proxy: false, dangerously_allow_all_unix_sockets: Some(true), mode: NetworkMode::Limited, - allow_local_binding: true, + allow_local_binding: Some(true), ..NetworkProxyConfig::default() }; config.set_allowed_domains(vec!["example.com".into()]); @@ -92,7 +95,7 @@ async fn round_trip_preserves_supported_effective_settings() { ); assert_eq!( proxy - .remote_launch_config() + .remote_launch_config(DefaultFalse) .await .unwrap() .proxy @@ -210,3 +213,36 @@ fn policy_decision_callback_timeout_round_trips() { launch ); } + +#[tokio::test] +async fn local_binding_is_resolved_for_each_executor() -> anyhow::Result<()> { + for (binding, policy, expected) in [ + (None, DefaultFalse, Some(false)), + (None, RequireTrue, Some(true)), + (Some(false), DefaultFalse, Some(false)), + (Some(false), RequireTrue, None), + (Some(true), DefaultFalse, Some(true)), + ] { + let config = NetworkProxyConfig { + enabled: true, + allow_local_binding: binding, + ..Default::default() + }; + let mut state = crate::runtime::network_proxy_state_for_policy(config); + state.local_binding_policy = RequireTrue; + let proxy = NetworkProxy::builder() + .state(Arc::new(state)) + .managed_by_codex(false) + .build() + .await?; + assert_eq!( + proxy + .remote_launch_config(policy) + .await + .ok() + .map(|launch| launch.proxy.allow_local_binding), + expected, + ); + } + Ok(()) +} diff --git a/codex-rs/network-proxy/src/runtime.rs b/codex-rs/network-proxy/src/runtime.rs index 04da5a3612..8cc6adac1a 100644 --- a/codex-rs/network-proxy/src/runtime.rs +++ b/codex-rs/network-proxy/src/runtime.rs @@ -232,8 +232,36 @@ where } } +/// How the executor resolves and validates local binding when managed networking is enabled. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub enum LocalBindingPolicy { + /// Omission defaults to false; either explicit value is accepted. + #[default] + DefaultFalse, + /// Omission defaults to true; explicit false is rejected. + RequireTrue, +} + +impl LocalBindingPolicy { + /// Resolves omission without changing the configured value. + pub fn resolve(self, config: &NetworkProxyConfig) -> bool { + config + .allow_local_binding + .unwrap_or(config.enabled && self == Self::RequireTrue) + } + + pub(crate) fn resolved_config(self, config: &NetworkProxyConfig) -> NetworkProxyConfig { + NetworkProxyConfig { + allow_local_binding: Some(self.resolve(config)), + ..config.clone() + } + } +} + pub struct NetworkProxyState { state: Arc>, + /// Belongs to this proxy; config retains omission for launches on other executors. + pub(crate) local_binding_policy: LocalBindingPolicy, reloader: Arc, blocked_request_observer: Arc>>>, pub(crate) policy_audit_observer: Option, @@ -271,6 +299,7 @@ impl Clone for NetworkProxyState { fn clone(&self) -> Self { Self { state: self.state.clone(), + local_binding_policy: self.local_binding_policy, reloader: self.reloader.clone(), blocked_request_observer: self.blocked_request_observer.clone(), policy_audit_observer: self.policy_audit_observer.clone(), @@ -312,6 +341,7 @@ impl NetworkProxyState { state, Arc::new(StaticConfigReloader), audit_metadata, + LocalBindingPolicy::DefaultFalse, ) }) } @@ -321,6 +351,7 @@ impl NetworkProxyState { state, reloader, NetworkProxyAuditMetadata::default(), + LocalBindingPolicy::DefaultFalse, ) } @@ -333,6 +364,7 @@ impl NetworkProxyState { state, reloader, NetworkProxyAuditMetadata::default(), + LocalBindingPolicy::DefaultFalse, blocked_request_observer, ) } @@ -341,11 +373,13 @@ impl NetworkProxyState { state: ConfigState, reloader: Arc, audit_metadata: NetworkProxyAuditMetadata, + local_binding_policy: LocalBindingPolicy, ) -> Self { Self::with_reloader_and_audit_metadata_and_blocked_observer( state, reloader, audit_metadata, + local_binding_policy, /*blocked_request_observer*/ None, ) } @@ -354,13 +388,15 @@ impl NetworkProxyState { state: ConfigState, reloader: Arc, audit_metadata: NetworkProxyAuditMetadata, + local_binding_policy: LocalBindingPolicy, blocked_request_observer: Option>, ) -> Self { let credential_broker = CredentialBroker::new(state.config.credential_broker); - credential_broker.configure(&state.config); + credential_broker.configure(&local_binding_policy.resolved_config(&state.config)); Self { credential_broker, state: Arc::new(RwLock::new(state)), + local_binding_policy, reloader, blocked_request_observer: Arc::new(RwLock::new(blocked_request_observer)), policy_audit_observer: None, @@ -616,7 +652,8 @@ impl NetworkProxyState { Ok(mut new_state) => { { let mut guard = self.state.write().await; - self.credential_broker.configure(&new_state.config); + self.credential_broker + .configure(&self.local_binding_policy.resolved_config(&new_state.config)); // Log policy diffs without dumping potentially sensitive config values. log_policy_changes(&guard.config, &new_state.config); new_state.blocked = guard.blocked.clone(); @@ -637,7 +674,8 @@ impl NetworkProxyState { pub async fn replace_config_state(&self, mut new_state: ConfigState) -> Result<()> { self.reload_if_needed().await?; let mut guard = self.state.write().await; - self.credential_broker.configure(&new_state.config); + self.credential_broker + .configure(&self.local_binding_policy.resolved_config(&new_state.config)); log_policy_changes(&guard.config, &new_state.config); new_state.blocked = guard.blocked.clone(); new_state.blocked_total = guard.blocked_total; @@ -647,6 +685,16 @@ impl NetworkProxyState { } pub async fn host_blocked(&self, host: &str, port: u16) -> Result { + self.host_blocked_with_local_binding(host, port, /*allow_local_binding*/ None) + .await + } + + pub(crate) async fn host_blocked_with_local_binding( + &self, + host: &str, + port: u16, + allow_local_binding: Option, + ) -> Result { self.reload_if_needed().await?; let host = match Host::parse(host) { Ok(host) => host, @@ -658,7 +706,10 @@ impl NetworkProxyState { ( guard.deny_set.clone(), guard.allow_set.clone(), - guard.config.allow_local_binding, + // Keep explicit controller restrictions even if the executor launched with true. + allow_local_binding + .unwrap_or_else(|| self.local_binding_policy.resolve(&guard.config)) + && guard.config.allow_local_binding != Some(false), allowed_domains, ) }; @@ -848,7 +899,7 @@ impl NetworkProxyState { pub async fn allow_local_binding(&self) -> Result { self.reload_if_needed().await?; let guard = self.state.read().await; - Ok(guard.config.allow_local_binding) + Ok(self.local_binding_policy.resolve(&guard.config)) } pub async fn network_mode(&self) -> Result { @@ -867,7 +918,8 @@ impl NetworkProxyState { (candidate, guard.constraints.clone()) }; - validate_policy_against_constraints(&candidate, &constraints) + let resolved = self.local_binding_policy.resolved_config(&candidate); + validate_policy_against_constraints(&resolved, &constraints) .map_err(NetworkProxyConstraintError::into_anyhow) .context("network.mode constrained by managed config")?; @@ -969,7 +1021,8 @@ impl NetworkProxyState { normalize_host, ); - validate_policy_against_constraints(&candidate, &constraints) + let resolved = self.local_binding_policy.resolved_config(&candidate); + validate_policy_against_constraints(&resolved, &constraints) .map_err(NetworkProxyConstraintError::into_anyhow) .with_context(|| format!("{constraint_field} constrained by managed config"))?; @@ -1001,7 +1054,8 @@ impl NetworkProxyState { Some(mut new_state) => { { let mut guard = self.state.write().await; - self.credential_broker.configure(&new_state.config); + self.credential_broker + .configure(&self.local_binding_policy.resolved_config(&new_state.config)); log_policy_changes(&guard.config, &new_state.config); new_state.blocked = guard.blocked.clone(); new_state.blocked_total = guard.blocked_total; @@ -1733,7 +1787,7 @@ mod tests { #[tokio::test] async fn host_blocked_requires_exact_scoped_ipv6_allowlist_match() { let state = network_proxy_state_for_policy(NetworkProxyConfig { - allow_local_binding: true, + allow_local_binding: Some(true), ..network_settings(&["fe80::1%eth0"], &[]) }); @@ -1756,7 +1810,7 @@ mod tests { #[tokio::test] async fn host_blocked_denies_scoped_ipv6_literal_before_local_binding() { let state = network_proxy_state_for_policy(NetworkProxyConfig { - allow_local_binding: true, + allow_local_binding: Some(true), ..network_settings(&["*"], &["fd00::1"]) }); @@ -1772,7 +1826,7 @@ mod tests { #[tokio::test] async fn host_blocked_requires_exact_scoped_ipv6_denylist_match() { let state = network_proxy_state_for_policy(NetworkProxyConfig { - allow_local_binding: true, + allow_local_binding: Some(true), ..network_settings(&["*"], &["fd00::1%eth0"]) }); @@ -2049,7 +2103,7 @@ mod tests { let config = NetworkProxyConfig { enabled: true, - allow_local_binding: true, + allow_local_binding: Some(true), ..NetworkProxyConfig::default() }; diff --git a/codex-rs/network-proxy/src/state.rs b/codex-rs/network-proxy/src/state.rs index 918b5ade2a..86c4c9135a 100644 --- a/codex-rs/network-proxy/src/state.rs +++ b/codex-rs/network-proxy/src/state.rs @@ -228,7 +228,7 @@ pub fn validate_policy_against_constraints( )?; if let Some(allow_local_binding) = constraints.allow_local_binding { - validate(config.allow_local_binding, move |candidate| { + validate(config.allow_local_binding(), move |candidate| { if *candidate && !allow_local_binding { Err(invalid_value( "network.allow_local_binding", diff --git a/codex-rs/network-proxy/src/system_dns_tests.rs b/codex-rs/network-proxy/src/system_dns_tests.rs index 4ad4990a74..9d74646bcd 100644 --- a/codex-rs/network-proxy/src/system_dns_tests.rs +++ b/codex-rs/network-proxy/src/system_dns_tests.rs @@ -41,7 +41,7 @@ async fn connector_reaches_native_hostname_over_ipv4_and_ipv6() { let target = listener.local_addr().expect("local addr"); let connector = TargetCheckedTcpConnector::new(Arc::new(network_proxy_state_for_policy( NetworkProxyConfig { - allow_local_binding: true, + allow_local_binding: Some(true), ..NetworkProxyConfig::default() }, ))); diff --git a/codex-rs/network-proxy/tests/windows_stable_ingress.rs b/codex-rs/network-proxy/tests/windows_stable_ingress.rs index b3bd79b7ee..be8ae20b8f 100644 --- a/codex-rs/network-proxy/tests/windows_stable_ingress.rs +++ b/codex-rs/network-proxy/tests/windows_stable_ingress.rs @@ -297,7 +297,7 @@ async fn build_proxy( socks_url: format!("socks5://{socks_addr}"), enable_socks5, enable_socks5_udp: false, - allow_local_binding: true, + allow_local_binding: Some(true), mode: NetworkMode::Full, ..NetworkProxyConfig::default() };