diff --git a/codex-rs/core/src/config/network_proxy_spec.rs b/codex-rs/core/src/config/network_proxy_spec.rs index 1957455d6f..fc052fa89c 100644 --- a/codex-rs/core/src/config/network_proxy_spec.rs +++ b/codex-rs/core/src/config/network_proxy_spec.rs @@ -149,7 +149,9 @@ impl NetworkProxySpec { let mut constraints = NetworkProxyConstraints::default(); if let Some(enabled) = requirements.enabled { - config.network.enabled = enabled; + if !enabled { + config.network.enabled = false; + } constraints.enabled = Some(enabled); } if let Some(http_port) = requirements.http_port { @@ -159,42 +161,194 @@ impl NetworkProxySpec { config.network.socks_url = format!("http://127.0.0.1:{socks_port}"); } if let Some(allow_upstream_proxy) = requirements.allow_upstream_proxy { - config.network.allow_upstream_proxy = allow_upstream_proxy; + if !allow_upstream_proxy { + config.network.allow_upstream_proxy = false; + } constraints.allow_upstream_proxy = Some(allow_upstream_proxy); } if let Some(dangerously_allow_non_loopback_proxy) = requirements.dangerously_allow_non_loopback_proxy { - config.network.dangerously_allow_non_loopback_proxy = - dangerously_allow_non_loopback_proxy; + if !dangerously_allow_non_loopback_proxy { + config.network.dangerously_allow_non_loopback_proxy = false; + } constraints.dangerously_allow_non_loopback_proxy = Some(dangerously_allow_non_loopback_proxy); } if let Some(dangerously_allow_non_loopback_admin) = requirements.dangerously_allow_non_loopback_admin { - config.network.dangerously_allow_non_loopback_admin = - dangerously_allow_non_loopback_admin; + if !dangerously_allow_non_loopback_admin { + config.network.dangerously_allow_non_loopback_admin = false; + } constraints.dangerously_allow_non_loopback_admin = Some(dangerously_allow_non_loopback_admin); } if let Some(allowed_domains) = requirements.allowed_domains.clone() { - config.network.allowed_domains = allowed_domains.clone(); + // Keep the user-configured allowlist if it is narrower. Managed + // requirements act as a subset constraint and are enforced by + // `validate_policy_against_constraints`. constraints.allowed_domains = Some(allowed_domains); } if let Some(denied_domains) = requirements.denied_domains.clone() { - config.network.denied_domains = denied_domains.clone(); + // Managed denied domains are a floor. Preserve user-added entries, + // but ensure all managed entries are present. + for denied_domain in &denied_domains { + if !config + .network + .denied_domains + .iter() + .any(|existing| existing.eq_ignore_ascii_case(denied_domain)) + { + config.network.denied_domains.push(denied_domain.clone()); + } + } constraints.denied_domains = Some(denied_domains); } if let Some(allow_unix_sockets) = requirements.allow_unix_sockets.clone() { - config.network.allow_unix_sockets = allow_unix_sockets.clone(); + // Keep the user-configured socket allowlist if it is narrower. + // Managed requirements are enforced as a subset constraint during + // validation. constraints.allow_unix_sockets = Some(allow_unix_sockets); } if let Some(allow_local_binding) = requirements.allow_local_binding { - config.network.allow_local_binding = allow_local_binding; + if !allow_local_binding { + config.network.allow_local_binding = false; + } constraints.allow_local_binding = Some(allow_local_binding); } (config, constraints) } } + +#[cfg(test)] +mod tests { + use super::*; + + use codex_network_proxy::NetworkMode; + use pretty_assertions::assert_eq; + + #[test] + fn managed_requirements_preserve_more_restrictive_user_network_settings() -> std::io::Result<()> + { + let mut user_config = NetworkProxyConfig::default(); + user_config.network.enabled = false; + user_config.network.mode = NetworkMode::Limited; + user_config.network.allow_upstream_proxy = false; + user_config.network.dangerously_allow_non_loopback_proxy = false; + user_config.network.dangerously_allow_non_loopback_admin = false; + user_config.network.allowed_domains = vec!["api.openai.com".to_string()]; + user_config.network.denied_domains = + vec!["tracker.com".to_string(), "evil.com".to_string()]; + user_config.network.allow_unix_sockets = vec!["/tmp/a.sock".to_string()]; + user_config.network.allow_local_binding = false; + + let requirements = NetworkConstraints { + enabled: Some(true), + http_port: Some(43128), + socks_port: Some(43129), + allow_upstream_proxy: Some(true), + dangerously_allow_non_loopback_proxy: Some(true), + dangerously_allow_non_loopback_admin: Some(true), + allowed_domains: Some(vec!["*.openai.com".to_string()]), + denied_domains: Some(vec!["evil.com".to_string()]), + allow_unix_sockets: Some(vec!["/tmp/a.sock".to_string(), "/tmp/b.sock".to_string()]), + allow_local_binding: Some(true), + }; + + let spec = NetworkProxySpec::from_config_and_constraints(user_config, Some(requirements))?; + + let mut expected_config = NetworkProxyConfig::default(); + expected_config.network.enabled = false; + expected_config.network.proxy_url = "http://127.0.0.1:43128".to_string(); + expected_config.network.mode = NetworkMode::Limited; + expected_config.network.allow_upstream_proxy = false; + expected_config.network.dangerously_allow_non_loopback_proxy = false; + expected_config.network.dangerously_allow_non_loopback_admin = false; + expected_config.network.enable_socks5 = true; + expected_config.network.socks_url = "http://127.0.0.1:43129".to_string(); + expected_config.network.enable_socks5_udp = true; + expected_config.network.allowed_domains = vec!["api.openai.com".to_string()]; + expected_config.network.denied_domains = + vec!["tracker.com".to_string(), "evil.com".to_string()]; + expected_config.network.allow_unix_sockets = vec!["/tmp/a.sock".to_string()]; + expected_config.network.allow_local_binding = false; + + assert_eq!(spec.config, expected_config); + assert_eq!( + spec.constraints, + NetworkProxyConstraints { + enabled: Some(true), + mode: None, + allow_upstream_proxy: Some(true), + dangerously_allow_non_loopback_proxy: Some(true), + dangerously_allow_non_loopback_admin: Some(true), + allowed_domains: Some(vec!["*.openai.com".to_string()]), + denied_domains: Some(vec!["evil.com".to_string()]), + allow_unix_sockets: Some(vec![ + "/tmp/a.sock".to_string(), + "/tmp/b.sock".to_string() + ]), + allow_local_binding: Some(true), + } + ); + + Ok(()) + } + + #[test] + fn managed_requirements_clamp_and_union_user_network_settings() -> std::io::Result<()> { + let mut user_config = NetworkProxyConfig::default(); + user_config.network.enabled = true; + user_config.network.mode = NetworkMode::Full; + user_config.network.allow_upstream_proxy = true; + user_config.network.dangerously_allow_non_loopback_proxy = true; + user_config.network.dangerously_allow_non_loopback_admin = true; + user_config.network.denied_domains = vec!["tracker.com".to_string()]; + user_config.network.allow_local_binding = true; + + let requirements = NetworkConstraints { + enabled: Some(false), + http_port: None, + socks_port: None, + allow_upstream_proxy: Some(false), + dangerously_allow_non_loopback_proxy: Some(false), + dangerously_allow_non_loopback_admin: Some(false), + allowed_domains: None, + denied_domains: Some(vec!["evil.com".to_string()]), + allow_unix_sockets: None, + allow_local_binding: Some(false), + }; + + let spec = NetworkProxySpec::from_config_and_constraints(user_config, Some(requirements))?; + + let mut expected_config = NetworkProxyConfig::default(); + expected_config.network.enabled = false; + expected_config.network.mode = NetworkMode::Full; + expected_config.network.allow_upstream_proxy = false; + expected_config.network.dangerously_allow_non_loopback_proxy = false; + expected_config.network.dangerously_allow_non_loopback_admin = false; + expected_config.network.denied_domains = + vec!["tracker.com".to_string(), "evil.com".to_string()]; + expected_config.network.allow_local_binding = false; + + assert_eq!(spec.config, expected_config); + assert_eq!( + spec.constraints, + NetworkProxyConstraints { + enabled: Some(false), + mode: None, + allow_upstream_proxy: Some(false), + dangerously_allow_non_loopback_proxy: Some(false), + dangerously_allow_non_loopback_admin: Some(false), + allowed_domains: None, + denied_domains: Some(vec!["evil.com".to_string()]), + allow_unix_sockets: None, + allow_local_binding: Some(false), + } + ); + + Ok(()) + } +} diff --git a/codex-rs/core/src/config/permissions.rs b/codex-rs/core/src/config/permissions.rs index a03b5273da..8f20726225 100644 --- a/codex-rs/core/src/config/permissions.rs +++ b/codex-rs/core/src/config/permissions.rs @@ -26,8 +26,17 @@ pub struct NetworkToml { pub dangerously_allow_non_loopback_admin: Option, #[schemars(with = "Option")] pub mode: Option, + /// When `requirements.toml` sets `experimental_network.allowed_domains`, + /// values here may further narrow access but must stay within the managed + /// allowlist (subset semantics). pub allowed_domains: Option>, + /// When `requirements.toml` sets `experimental_network.denied_domains`, + /// managed entries are always enforced. Values here can add more denied + /// domains but cannot remove the managed ones (floor semantics). pub denied_domains: Option>, + /// When `requirements.toml` sets `experimental_network.allow_unix_sockets`, + /// values here may further narrow access but must stay within the managed + /// socket allowlist (subset semantics). pub allow_unix_sockets: Option>, pub allow_local_binding: Option, }