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(