From cfde11a24cc0d709a3a6d6c7542470c29704b94f Mon Sep 17 00:00:00 2001 From: Sean Huang Date: Sun, 13 Sep 2026 06:21:41 +0000 Subject: [PATCH] Extract shared network configuration and environment policy helpers (#45180) ## What changed - Introduce `PreparedNetworkConfig` to separate proxy preparation from applying managed network requirements, preserving preparation before permission fallback in local configuration loading. - Add helpers to build portable environment policies, retain selected or managed policies even when the proxy is disabled, and validate policies against the final permission profile using the execution resolver. - Strip listener addresses from environment network configuration and reject unsupported settings, malformed domain patterns, and invalid Unix socket paths. - Expose supporting managed-feature and permission-profile configuration helpers. ## Testing Add coverage for policy retention, listener removal, unsupported and malformed policies, and validation against permission profiles. Strengthen configuration tests to compare rebuilt proxy specifications and verify proxy configuration survives permission fallback. GitOrigin-RevId: 444bf02d8543eaebdbd2ebc14b56d92ce6d2ad2f --- .../core/src/config/config_loader_tests.rs | 6 +- codex-rs/core/src/config/config_tests.rs | 15 ++ codex-rs/core/src/config/managed_features.rs | 4 +- codex-rs/core/src/config/mod.rs | 61 ++---- codex-rs/core/src/config/network_config.rs | 184 ++++++++++++++++++ .../core/src/config/network_config_tests.rs | 99 ++++++++++ .../core/src/config/network_proxy_spec.rs | 2 +- .../src/config/network_proxy_spec_tests.rs | 21 ++ .../src/config/permission_profile_catalog.rs | 5 +- codex-rs/core/src/config/permissions.rs | 2 +- 10 files changed, 350 insertions(+), 49 deletions(-) create mode 100644 codex-rs/core/src/config/network_config.rs create mode 100644 codex-rs/core/src/config/network_config_tests.rs diff --git a/codex-rs/core/src/config/config_loader_tests.rs b/codex-rs/core/src/config/config_loader_tests.rs index 49d4b27105..1257a40c0f 100644 --- a/codex-rs/core/src/config/config_loader_tests.rs +++ b/codex-rs/core/src/config/config_loader_tests.rs @@ -1999,11 +1999,7 @@ extends = "first" config.permissions.permission_profile(), )? .unwrap(); - assert_eq!( - rebuilt.credential_broker_enabled(), - expected_enabled, - "{key}" - ); + assert_eq!(Some(&rebuilt), config.permissions.network.as_ref(), "{key}"); } Ok(()) } diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 99c91f2f30..31fcbfc906 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -10917,6 +10917,10 @@ default_permissions = "dev" async fn permission_profile_override_falls_back_when_disallowed_by_requirements() -> std::io::Result<()> { let codex_home = TempDir::new()?; + std::fs::write( + codex_home.path().join(CONFIG_TOML_FILE), + "[features.network_proxy]\nenabled = true\nproxy_url = 'http://127.0.0.1:43128'\n", + )?; let config = ConfigBuilder::without_managed_config_for_tests() .codex_home(codex_home.path().to_path_buf()) .fallback_cwd(Some(codex_home.path().to_path_buf())) @@ -10938,6 +10942,17 @@ async fn permission_profile_override_falls_back_when_disallowed_by_requirements( config.permissions.effective_permission_profile(), PermissionProfile::read_only() ); + // Prepare the proxy while the candidate permits network, before fallback. + let expected_network = NetworkProxySpec::from_config_and_constraints( + NetworkProxyConfig { + enabled: true, + proxy_url: "http://127.0.0.1:43128".to_string(), + ..Default::default() + }, + /*requirements*/ None, + &PermissionProfile::read_only(), + )?; + assert_eq!(config.permissions.network, Some(expected_network)); Ok(()) } diff --git a/codex-rs/core/src/config/managed_features.rs b/codex-rs/core/src/config/managed_features.rs index 1a2b970021..645bccd0e4 100644 --- a/codex-rs/core/src/config/managed_features.rs +++ b/codex-rs/core/src/config/managed_features.rs @@ -50,7 +50,9 @@ impl ManagedFeatures { ) } - pub(crate) fn from_configured_with_warnings( + /// Applies managed feature requirements and dependency normalization to + /// configured features, collecting warnings for unsupported requirements. + pub fn from_configured_with_warnings( configured_features: Features, feature_requirements: Option>, startup_warnings: &mut Vec, diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index f1aa685185..e1e00a2445 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -165,6 +165,7 @@ mod auth_keyring; pub mod edit; mod managed_features; mod metrics; +mod network_config; mod network_proxy_spec; mod otel; mod permission_path; @@ -189,19 +190,25 @@ use codex_sandboxing::compatibility_sandbox_policy_for_permission_profile; pub use codex_sandboxing::system_bwrap_warning; pub use managed_features::ManagedFeatures; pub(crate) use metrics::emit_session_start_metrics; +pub use network_config::EnvironmentNetworkConfigError; +pub use network_config::NetworkConfigInputs; +pub use network_config::PreparedNetworkConfig; +pub use network_config::project_environment_profile_network; +pub use network_config::validate_environment_network_policy; pub use network_proxy_spec::NetworkProxySpec; pub use network_proxy_spec::StartedNetworkProxy; pub use permission_profile_catalog::PermissionProfileCatalogEntry; pub use permission_profile_catalog::permission_profile_catalog; use permission_profile_catalog::permission_profile_catalog_from_permissions; use permission_profile_catalog::permission_profile_is_allowed; -use permission_profile_catalog::validate_permission_profile_for_deny_read; +pub use permission_profile_catalog::validate_permission_profile_for_deny_read; pub use permission_profile_selection::ResolvedPermissionProfileSelection; pub use permission_profile_selection::resolve_permission_profile_selection; pub use permissions::CompiledPermissionProfile; pub use permissions::WorkspaceWriteSettings; pub use permissions::compile_permission_profile; pub(crate) use permissions::is_builtin_permission_profile_name; +pub use permissions::network_proxy_config_from_profile_network; pub use permissions::resolve_permission_profile; pub(crate) use resolved_permission_profile::PermissionProfileState; pub use token_budget_startup::TokenBudgetStartupConfig; @@ -574,32 +581,10 @@ fn build_network_proxy_spec( environment_overrides: &HashMap, ) -> std::io::Result> { configured_network_proxy_config.configure_credential_broker_environment(environment_overrides); - let (network_requirements, network_requirements_source) = match network_requirements { - Some(Sourced { value, source }) => (Some(value), Some(source)), - None => (None, None), - }; - let has_network_requirements = network_requirements.is_some(); - let network = NetworkProxySpec::from_config_and_constraints( - configured_network_proxy_config, - network_requirements, - permission_profile, - ) - .map_err(|err| { - if let Some(source) = network_requirements_source.as_ref() { - std::io::Error::new( - err.kind(), - format!("failed to build managed network proxy from {source}: {err}"), - ) - } else { - err - } - })?; - - Ok(if has_network_requirements { - Some(network) - } else { - network.enabled().then_some(network) - }) + PreparedNetworkConfig { + configured_proxy: configured_network_proxy_config, + } + .build(network_requirements, permission_profile) } /// Configured thread persistence backend. @@ -3479,7 +3464,7 @@ impl Config { }; dedupe_absolute_paths(&mut workspace_roots); let ( - mut configured_network_proxy_config, + configured_network_proxy_config, permission_profile, file_system_sandbox_policy, mut active_permission_profile, @@ -3617,17 +3602,13 @@ impl Config { Vec::new(), ) }; - if enable_network_proxy && permission_profile.network_sandbox_policy().is_enabled() { - if let Some(network_proxy) = network_proxy_toml_config(cfg.features.as_ref()) { - apply_network_proxy_feature_config( - &mut configured_network_proxy_config, - network_proxy, - ); - } - configured_network_proxy_config - .set_credential_broker_openai_base_url(cfg.openai_base_url.as_deref()); - configured_network_proxy_config.enabled = true; - } + let prepared_network = PreparedNetworkConfig::from_inputs(NetworkConfigInputs { + configured_proxy: configured_network_proxy_config, + feature_enabled: enable_network_proxy, + features: cfg.features.as_ref(), + candidate_permission_profile: &permission_profile, + credential_broker_base_url: cfg.openai_base_url.as_deref(), + }); if cfg.approval_policy == Some(AskForApproval::UnlessTrusted) { return Err(std::io::Error::new( ErrorKind::InvalidData, @@ -4033,7 +4014,7 @@ impl Config { let network_permission_profile = constrained_permission_profile.get().clone(); let network = build_network_proxy_spec( - configured_network_proxy_config, + prepared_network.configured_proxy, network_requirements, &network_permission_profile, &shell_environment_policy.r#set, diff --git a/codex-rs/core/src/config/network_config.rs b/codex-rs/core/src/config/network_config.rs new file mode 100644 index 0000000000..4f5797cc96 --- /dev/null +++ b/codex-rs/core/src/config/network_config.rs @@ -0,0 +1,184 @@ +//! Network configuration materialization from explicit, already-selected inputs. +//! +//! Preparing the configured proxy precedes permission requirements fallback in +//! local configuration loading. Keep that phase separate from applying network +//! requirements to the final permission profile so both callers share the same +//! ordering without reading a Config or any host state here. +//! Environment policy conversion drops listener addresses, and final validation +//! uses the same policy resolver as command execution. + +use codex_config::NetworkConstraints; +use codex_config::Sourced; +use codex_config::permissions_toml::NetworkToml; +use codex_execpolicy::Policy; +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::build_config_state; +use codex_protocol::models::PermissionProfile; + +use super::NetworkProxySpec; +use super::permissions::apply_network_proxy_feature_config; + +#[derive(Debug, thiserror::Error, PartialEq, Eq)] +#[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. +pub fn project_environment_profile_network( + network: Option, +) -> Result, EnvironmentNetworkConfigError> { + let Some(mut network) = network else { + return Ok(None); + }; + network.proxy_url = None; + network.socks_url = None; + let supported = NetworkToml { + enabled: network.enabled, + allow_upstream_proxy: network.allow_upstream_proxy, + dangerously_allow_all_unix_sockets: network.dangerously_allow_all_unix_sockets, + domains: network.domains.clone(), + unix_sockets: network.unix_sockets.clone(), + allow_local_binding: network.allow_local_binding, + ..Default::default() + }; + let invalid_socket = network.unix_sockets.as_ref().is_some_and(|sockets| { + sockets + .entries + .keys() + .any(|path| !path.starts_with('/') || path.contains('\0')) + }); + if network != supported || invalid_socket { + return Err(EnvironmentNetworkConfigError); + } + // MITM and credential settings have been excluded. This only validates + // patterns and lexical socket paths; it does not discover executor files. + build_config_state( + network.to_network_proxy_config(), + NetworkProxyConstraints::default(), + ) + .map_err(|_| EnvironmentNetworkConfigError)?; + Ok(Some(network)) +} + +/// Validates the owner policy for its final permission profile using the native +/// environment resolver. Execution also applies controller and saved exec rules. +pub fn validate_environment_network_policy( + policy: &EnvironmentNetworkPolicy, + permission_profile: &PermissionProfile, +) -> Result<(), EnvironmentNetworkConfigError> { + NetworkProxySpec::for_environment( + /*controller*/ None, + policy, + permission_profile, + &Policy::empty(), + ) + .and_then(|spec| spec.build_config_state_for_spec().map(|_| ())) + .map_err(|_| EnvironmentNetworkConfigError) +} + +pub struct NetworkConfigInputs<'a> { + pub configured_proxy: NetworkProxyConfig, + pub feature_enabled: bool, + pub features: Option<&'a FeaturesToml>, + pub candidate_permission_profile: &'a PermissionProfile, + pub credential_broker_base_url: Option<&'a str>, +} + +/// The configured proxy before managed network requirements are applied. +pub struct PreparedNetworkConfig { + pub(super) configured_proxy: NetworkProxyConfig, +} + +impl PreparedNetworkConfig { + pub fn from_inputs(inputs: NetworkConfigInputs<'_>) -> Self { + let NetworkConfigInputs { + mut configured_proxy, + feature_enabled, + features, + candidate_permission_profile, + credential_broker_base_url, + } = inputs; + if feature_enabled + && candidate_permission_profile + .network_sandbox_policy() + .is_enabled() + { + if let Some(FeatureToml::Config(config)) = + features.and_then(|features| features.network_proxy.as_ref()) + { + apply_network_proxy_feature_config(&mut configured_proxy, config); + } + configured_proxy.set_credential_broker_openai_base_url(credential_broker_base_url); + configured_proxy.enabled = true; + } + Self { configured_proxy } + } + + /// Applies managed network requirements after permission fallback has run. + pub fn build( + self, + requirements: Option>, + effective_permission_profile: &PermissionProfile, + ) -> std::io::Result> { + let has_requirements = requirements.is_some(); + let network = self.build_spec(requirements, effective_permission_profile)?; + Ok(if has_requirements { + Some(network) + } else { + network.enabled().then_some(network) + }) + } + + /// An attachment's portable policy can be enforced without a local proxy + /// controller. Its presence must not depend on the controller feature gate. + pub fn build_environment_policy( + self, + requirements: Option>, + effective_permission_profile: &PermissionProfile, + has_selected_policy: bool, + ) -> std::io::Result> { + let has_requirements = requirements.is_some(); + let network = self.build_spec(requirements, effective_permission_profile)?; + Ok( + (has_selected_policy || has_requirements || network.enabled()) + .then(|| network.environment_policy()), + ) + } + + fn build_spec( + self, + requirements: Option>, + effective_permission_profile: &PermissionProfile, + ) -> std::io::Result { + let (requirements, source) = match requirements { + Some(Sourced { value, source }) => (Some(value), Some(source)), + None => (None, None), + }; + let network = NetworkProxySpec::from_config_and_constraints( + self.configured_proxy, + requirements, + effective_permission_profile, + ) + .map_err(|error| { + if let Some(source) = source.as_ref() { + std::io::Error::new( + error.kind(), + format!("failed to build managed network proxy from {source}: {error}"), + ) + } else { + error + } + })?; + + Ok(network) + } +} + +#[cfg(test)] +#[path = "network_config_tests.rs"] +mod tests; diff --git a/codex-rs/core/src/config/network_config_tests.rs b/codex-rs/core/src/config/network_config_tests.rs new file mode 100644 index 0000000000..b931545918 --- /dev/null +++ b/codex-rs/core/src/config/network_config_tests.rs @@ -0,0 +1,99 @@ +//! Covers policy presence and the environment-only configuration projection. + +use super::*; +use codex_config::RequirementSource; +use pretty_assertions::assert_eq; + +#[test] +fn environment_policy_presence_keeps_selected_and_managed_denials() { + let mut configured_proxy = NetworkProxyConfig::default(); + configured_proxy.set_denied_domains(vec!["blocked.example".to_string()]); + let expected = EnvironmentNetworkPolicy::from_config( + &configured_proxy, + /*managed_allowed_domains_only*/ false, + ); + for (has_selected_policy, requirements, has_policy) in [ + (false, None, false), + (true, None, true), + ( + false, + Some(NetworkConstraints { + enabled: Some(false), + ..Default::default() + }), + true, + ), + ] { + let actual = PreparedNetworkConfig { + configured_proxy: configured_proxy.clone(), + } + .build_environment_policy( + requirements.map(|value| Sourced::new(value, RequirementSource::Unknown)), + &PermissionProfile::read_only(), + has_selected_policy, + ) + .unwrap(); + assert_eq!(actual, has_policy.then(|| expected.clone())); + } +} + +#[test] +fn attachment_projection_preserves_policy_and_drops_listener_addresses() { + for raw in [ + "", + r#" +enabled = false +allow_upstream_proxy = false +dangerously_allow_all_unix_sockets = false +allow_local_binding = false +[domains] +'allowed.example' = 'allow' +'blocked.example' = 'deny' +[unix_sockets] +'/tmp/allowed.sock' = 'allow' +'/tmp/blocked.sock' = 'deny' +"#, + ] { + let expected: NetworkToml = toml::from_str(raw).unwrap(); + let network = NetworkToml { + proxy_url: Some("not a listener URL".to_string()), + socks_url: Some("socks5://127.0.0.1:1080".to_string()), + ..expected.clone() + }; + assert_eq!( + project_environment_profile_network(Some(network)), + Ok(Some(expected)) + ); + } + assert_eq!( + project_environment_profile_network(/*network*/ None), + Ok(None) + ); +} + +#[test] +fn attachment_projection_rejects_unsupported_restrictions_and_invalid_policy() { + for raw in [ + "mode = 'full'", + "mode = 'limited'", + r#" +[mitm.actions.redact] +strip_request_headers = ['Authorization'] +[mitm.hooks.api] +host = 'api.example' +methods = ['GET'] +path_prefixes = ['/'] +action = ['redact'] +"#, + "[unix_sockets]\n'relative.sock' = 'allow'", + "[unix_sockets]\n'relative.sock' = 'deny'", + "[domains]\n'[' = 'allow'", + ] { + let network: NetworkToml = toml::from_str(raw).unwrap(); + assert_eq!( + project_environment_profile_network(Some(network)), + Err(EnvironmentNetworkConfigError), + "{raw}" + ); + } +} diff --git a/codex-rs/core/src/config/network_proxy_spec.rs b/codex-rs/core/src/config/network_proxy_spec.rs index f802e402da..d7eb787ff0 100644 --- a/codex-rs/core/src/config/network_proxy_spec.rs +++ b/codex-rs/core/src/config/network_proxy_spec.rs @@ -334,7 +334,7 @@ impl NetworkProxySpec { )) } - fn build_config_state_for_spec(&self) -> std::io::Result { + pub(super) fn build_config_state_for_spec(&self) -> std::io::Result { build_config_state(self.config.clone(), self.constraints.clone()).map_err(|err| { std::io::Error::other(format!("failed to build network proxy state: {err}")) }) 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 4afa5cc3cf..0f3506cac0 100644 --- a/codex-rs/core/src/config/network_proxy_spec_tests.rs +++ b/codex-rs/core/src/config/network_proxy_spec_tests.rs @@ -1,4 +1,6 @@ use super::*; +use crate::config::EnvironmentNetworkConfigError; +use crate::config::validate_environment_network_policy; use codex_config::NetworkDomainPermissionToml; use codex_config::NetworkDomainPermissionsToml; use codex_execpolicy::Decision::Allow; @@ -121,6 +123,14 @@ fn environment_policy_replaces_soft_controller_allowlist_and_preserves_denials() owner.allow_local_binding = true; let owner_policy = EnvironmentNetworkPolicy::from_config(&owner, /*managed_allowed_domains_only*/ false); + assert_eq!( + validate_environment_network_policy(&owner_policy, &profile), + Ok(()) + ); + assert_eq!( + validate_environment_network_policy(&owner_policy, &PermissionProfile::Disabled), + Err(EnvironmentNetworkConfigError) + ); let compose = NetworkProxySpec::for_environment; let empty = Policy::empty(); let disabled_controller = NetworkProxySpec::from_config_and_constraints( @@ -185,6 +195,17 @@ fn environment_policy_replaces_soft_controller_allowlist_and_preserves_denials() let wildcard_policy = EnvironmentNetworkPolicy::from_config(&owner, /*managed_allowed_domains_only*/ false); assert!(compose(Some(&spec), &wildcard_policy, &profile, &empty).is_err()); + assert_eq!( + validate_environment_network_policy(&wildcard_policy, &profile), + Err(EnvironmentNetworkConfigError) + ); + owner.set_allowed_domains(vec!["[".to_string()]); + let malformed_policy = + EnvironmentNetworkPolicy::from_config(&owner, /*managed_allowed_domains_only*/ false); + assert_eq!( + validate_environment_network_policy(&malformed_policy, &profile), + Err(EnvironmentNetworkConfigError) + ); } #[test] diff --git a/codex-rs/core/src/config/permission_profile_catalog.rs b/codex-rs/core/src/config/permission_profile_catalog.rs index 34262a3bcf..224465ce7d 100644 --- a/codex-rs/core/src/config/permission_profile_catalog.rs +++ b/codex-rs/core/src/config/permission_profile_catalog.rs @@ -115,7 +115,10 @@ pub(super) fn permission_profile_is_allowed( allowed_by_id && allowed_by_sandbox_mode && allowed_by_filesystem } -pub(super) fn validate_permission_profile_for_deny_read( +/// Validates that a permission candidate can enforce managed filesystem denials. +/// Add this validator before applying profile requirements so the native +/// constrained fallback can replace full-access or external candidates. +pub fn validate_permission_profile_for_deny_read( permission_profile: &PermissionProfile, requirement_source: &RequirementSource, ) -> ConstraintResult<()> { diff --git a/codex-rs/core/src/config/permissions.rs b/codex-rs/core/src/config/permissions.rs index 2207c8f0fb..75612b2806 100644 --- a/codex-rs/core/src/config/permissions.rs +++ b/codex-rs/core/src/config/permissions.rs @@ -120,7 +120,7 @@ pub(crate) fn validate_user_permission_profile_names( Ok(()) } -pub(crate) fn network_proxy_config_from_profile_network( +pub fn network_proxy_config_from_profile_network( network: Option<&NetworkToml>, ) -> NetworkProxyConfig { let mut config = network.map_or_else(