diff --git a/codex-rs/core/src/config/network_config.rs b/codex-rs/core/src/config/network_config.rs index e703387513..fbc5989347 100644 --- a/codex-rs/core/src/config/network_config.rs +++ b/codex-rs/core/src/config/network_config.rs @@ -15,9 +15,7 @@ use codex_features::FeatureToml; use codex_features::FeaturesToml; use codex_network_proxy::EnvironmentNetworkPolicy; use codex_network_proxy::NetworkProxyConfig; -use codex_network_proxy::NetworkProxyConstraints; use codex_network_proxy::NetworkProxyExecutorOs; -use codex_network_proxy::build_config_state; use codex_protocol::models::PermissionProfile; use super::NetworkProxySpec; @@ -27,16 +25,14 @@ use super::permissions::apply_network_proxy_feature_config; #[error("invalid portable environment network policy")] pub struct EnvironmentNetworkConfigError; -/// Drops listener addresses, which the executor chooses for each command, before -/// validating an environment's selected network configuration. Use the returned -/// value when deciding whether a policy is present and when preparing it. +/// Drops listener addresses and rejects unsupported controller fields in an +/// environment's selected network configuration. Use the returned value when +/// deciding whether a policy is present and when preparing it. /// -/// Socket policy validation uses the supplied executor OS: Allow keys must be -/// NUL-free and absolute for that OS; Deny keys are preserved unchanged. -/// Path normalization and socket support are determined by the executor. +/// Domain and socket values may be replaced by feature settings or requirements. +/// Validate the composed policy with [`validate_environment_network_policy`]. pub fn project_environment_profile_network( network: Option, - executor_os: NetworkProxyExecutorOs, ) -> Result, EnvironmentNetworkConfigError> { let Some(mut network) = network else { return Ok(None); @@ -55,15 +51,6 @@ pub fn project_environment_profile_network( if network != supported { return Err(EnvironmentNetworkConfigError); } - // Reuse shared, host-independent policy validation. - // With MITM and credentials excluded, this checks patterns and allowed - // socket paths without discovering executor files. - build_config_state( - network.to_network_proxy_config(), - NetworkProxyConstraints::default(), - executor_os, - ) - .map_err(|_| EnvironmentNetworkConfigError)?; Ok(Some(network)) } diff --git a/codex-rs/core/src/config/network_config_tests.rs b/codex-rs/core/src/config/network_config_tests.rs index 27bbcbdcfd..6b3616fac2 100644 --- a/codex-rs/core/src/config/network_config_tests.rs +++ b/codex-rs/core/src/config/network_config_tests.rs @@ -2,6 +2,8 @@ use super::*; use codex_config::RequirementSource; +use codex_network_proxy::NetworkProxyConstraints; +use codex_network_proxy::build_config_state; use pretty_assertions::assert_eq; #[test] @@ -38,7 +40,7 @@ fn environment_policy_presence_keeps_selected_and_managed_denials() { } #[test] -fn attachment_projection_uses_executor_os_and_drops_listener_addresses() { +fn attachment_projection_preserves_policy_and_drops_listener_addresses() { for executor_os in [ NetworkProxyExecutorOs::Linux, NetworkProxyExecutorOs::Macos, @@ -108,24 +110,20 @@ allow_local_binding = false ..expected.clone() }; assert_eq!( - project_environment_profile_network(Some(network), executor_os), - if accepted { - Ok(Some(expected)) - } else { - Err(EnvironmentNetworkConfigError) - }, + project_environment_profile_network(Some(network)), + Ok(Some(expected)), "projection: {executor_os:?}, {raw}" ); } assert_eq!( - project_environment_profile_network(/*network*/ None, executor_os), + project_environment_profile_network(/*network*/ None), Ok(None) ); } } #[test] -fn attachment_projection_rejects_unsupported_restrictions_and_invalid_policy() { +fn attachment_projection_rejects_unsupported_restrictions_and_defers_policy_validation() { for raw in [ "mode = 'full'", "mode = 'limited'", @@ -138,16 +136,76 @@ methods = ['GET'] path_prefixes = ['/'] action = ['redact'] "#, + ] { + let network: NetworkToml = toml::from_str(raw).unwrap(); + assert_eq!( + project_environment_profile_network(Some(network)), + Err(EnvironmentNetworkConfigError), + "{raw}" + ); + } + + for raw in [ "[unix_sockets]\n'relative.sock' = 'allow'", "[unix_sockets]\n'~/allowed.sock' = 'allow'", "[unix_sockets]\n\"/tmp/\\u0000allowed.sock\" = 'allow'", "[domains]\n'[' = 'allow'", ] { let network: NetworkToml = toml::from_str(raw).unwrap(); + let projected = project_environment_profile_network(Some(network.clone())) + .unwrap() + .unwrap(); + assert_eq!(projected, network); + let configured_proxy = projected.to_network_proxy_config(); + let policy = EnvironmentNetworkPolicy::from_config( + &configured_proxy, + /*managed_allowed_domains_only*/ false, + ); assert_eq!( - project_environment_profile_network(Some(network), NetworkProxyExecutorOs::Unknown), + validate_environment_network_policy( + &policy, + &PermissionProfile::read_only(), + NetworkProxyExecutorOs::Unknown, + ), Err(EnvironmentNetworkConfigError), "{raw}" ); + + // Managed requirements replace the invalid Allow entries before validation. + let policy = PreparedNetworkConfig { configured_proxy } + .build_environment_policy( + Some(Sourced::new( + NetworkConstraints { + enabled: Some(true), + managed_allowed_domains_only: Some(true), + unix_sockets: Some(Default::default()), + ..Default::default() + }, + RequirementSource::Unknown, + )), + &PermissionProfile::read_only(), + /*has_selected_policy*/ true, + ) + .unwrap() + .unwrap(); + assert_eq!( + policy, + EnvironmentNetworkPolicy::from_config( + &NetworkProxyConfig { + unix_sockets: Some(Default::default()), + ..Default::default() + }, + /*managed_allowed_domains_only*/ true, + ) + ); + assert_eq!( + validate_environment_network_policy( + &policy, + &PermissionProfile::read_only(), + NetworkProxyExecutorOs::Unknown, + ), + Ok(()), + "{raw}" + ); } }