diff --git a/codex-rs/app-server/src/lib.rs b/codex-rs/app-server/src/lib.rs index 2766882b86..663687c5da 100644 --- a/codex-rs/app-server/src/lib.rs +++ b/codex-rs/app-server/src/lib.rs @@ -1383,9 +1383,7 @@ mod tests { #[test] fn windows_network_config_errors_are_unrecoverable() { for error in [ - codex_config::ConstraintError::ManagedNetworkRequiresElevatedWindowsSandbox { - requirement_source: codex_config::RequirementSource::Unknown, - }, + codex_config::ConstraintError::NetworkProxyRequiresElevatedWindowsSandboxRequirement, codex_config::ConstraintError::NetworkProxyIncompatibleWithUnelevatedWindowsSandbox, ] { let error = std::io::Error::from(error); diff --git a/codex-rs/config/src/config_requirements.rs b/codex-rs/config/src/config_requirements.rs index 4206c4a8bb..607665104b 100644 --- a/codex-rs/config/src/config_requirements.rs +++ b/codex-rs/config/src/config_requirements.rs @@ -754,6 +754,13 @@ impl WindowsRequirementsToml { pub fn is_empty(&self) -> bool { self.allowed_sandbox_implementations.is_none() } + + pub fn allows_only_elevated_sandbox(&self) -> bool { + matches!( + self.allowed_sandbox_implementations.as_deref(), + Some([WindowsSandboxModeToml::Elevated]) + ) + } } #[derive(Deserialize, Debug, Clone, Default, PartialEq, Eq)] @@ -881,6 +888,14 @@ pub struct ConfigRequirementsToml { pub guardian_policy_config: Option, } +impl ConfigRequirementsToml { + pub fn allows_only_elevated_windows_sandbox(&self) -> bool { + self.windows + .as_ref() + .is_some_and(WindowsRequirementsToml::allows_only_elevated_sandbox) + } +} + #[derive(Deserialize, Debug, Clone, Default, PartialEq, Eq)] pub struct ModelsRequirementsToml { pub new_thread: Option, @@ -1299,21 +1314,12 @@ impl TryFrom for ConfigRequirements { } } - if let Some(network) = &network { - let elevated_is_only_allowed_windows_sandbox = - windows.as_ref().is_some_and(|windows| { - matches!( - windows.value.allowed_sandbox_implementations.as_deref(), - Some([WindowsSandboxModeToml::Elevated]) - ) - }); - if !elevated_is_only_allowed_windows_sandbox { - return Err( - ConstraintError::ManagedNetworkRequiresElevatedWindowsSandbox { - requirement_source: network.source.clone(), - }, - ); - } + if network.is_some() + && !windows + .as_ref() + .is_some_and(|windows| windows.value.allows_only_elevated_sandbox()) + { + return Err(ConstraintError::NetworkProxyRequiresElevatedWindowsSandboxRequirement); } let approval_policy = match allowed_approval_policies { @@ -2836,11 +2842,7 @@ allowed_approvals_reviewers = ["user"] assert_eq!( ConfigRequirements::try_from(with_unknown_source(config)), - Err( - ConstraintError::ManagedNetworkRequiresElevatedWindowsSandbox { - requirement_source: RequirementSource::Unknown, - } - ) + Err(ConstraintError::NetworkProxyRequiresElevatedWindowsSandboxRequirement) ); } diff --git a/codex-rs/config/src/constraint.rs b/codex-rs/config/src/constraint.rs index b0c2da6763..5449e235e7 100644 --- a/codex-rs/config/src/constraint.rs +++ b/codex-rs/config/src/constraint.rs @@ -35,11 +35,9 @@ pub enum ConstraintError { }, #[error( - "managed network requirements (set by {requirement_source}) require `[windows] allowed_sandbox_implementations = [\"elevated\"]`" + "network proxy configuration requires `[windows] allowed_sandbox_implementations = [\"elevated\"]` in requirements" )] - ManagedNetworkRequiresElevatedWindowsSandbox { - requirement_source: RequirementSource, - }, + NetworkProxyRequiresElevatedWindowsSandboxRequirement, #[error( "the network proxy is incompatible with the unelevated Windows sandbox; set `windows.sandbox = \"elevated\"` or disable the network proxy" @@ -57,7 +55,7 @@ impl ConstraintError { pub fn is_windows_network_configuration_error(&self) -> bool { matches!( self, - Self::ManagedNetworkRequiresElevatedWindowsSandbox { .. } + Self::NetworkProxyRequiresElevatedWindowsSandboxRequirement | Self::NetworkProxyIncompatibleWithUnelevatedWindowsSandbox ) } diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index d39bdcb5c3..eb7e132f02 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -100,6 +100,77 @@ use tempfile::tempdir; use super::*; +async fn load_config_with_elevated_only_windows_sandbox_requirement( + cfg: ConfigToml, + overrides: ConfigOverrides, + codex_home: AbsolutePathBuf, +) -> std::io::Result { + let requirements_toml = ConfigRequirementsToml { + windows: Some(codex_config::WindowsRequirementsToml { + allowed_sandbox_implementations: Some(vec![WindowsSandboxModeToml::Elevated]), + }), + ..Default::default() + }; + let mut requirements_with_sources = codex_config::ConfigRequirementsWithSources::default(); + requirements_with_sources.merge_unset_fields( + codex_config::RequirementSource::Unknown, + requirements_toml.clone(), + ); + let requirements = ConfigRequirements::try_from(requirements_with_sources)?; + let config_layer_stack = ConfigLayerStack::new(Vec::new(), requirements, requirements_toml)?; + Config::load_config_with_layer_stack( + codex_exec_server::LOCAL_FS.as_ref(), + cfg, + overrides, + codex_home, + config_layer_stack, + ) + .await +} + +#[test] +fn windows_network_proxy_requires_elevated_only_sandbox_requirement() { + let err = validate_windows_network_proxy_requirements_for_platform( + /*is_windows*/ true, + &ConfigRequirementsToml::default(), + /*network_proxy_configured*/ true, + ) + .expect_err("Windows network proxy should require elevated-only sandbox requirements"); + + assert!(matches!( + err.get_ref() + .and_then(|source| source.downcast_ref::()), + Some(ConstraintError::NetworkProxyRequiresElevatedWindowsSandboxRequirement) + )); + + for (requirements, network_proxy_configured) in [ + (ConfigRequirementsToml::default(), false), + ( + ConfigRequirementsToml { + windows: Some(codex_config::WindowsRequirementsToml { + allowed_sandbox_implementations: Some(vec![WindowsSandboxModeToml::Elevated]), + }), + ..Default::default() + }, + true, + ), + ] { + validate_windows_network_proxy_requirements_for_platform( + /*is_windows*/ true, + &requirements, + network_proxy_configured, + ) + .expect("valid Windows network proxy requirements"); + } + + validate_windows_network_proxy_requirements_for_platform( + /*is_windows*/ false, + &ConfigRequirementsToml::default(), + /*network_proxy_configured*/ true, + ) + .expect("non-Windows network proxy should not require Windows sandbox policy"); +} + #[test] fn windows_network_proxy_requires_elevated_sandbox() { let err = validate_windows_sandbox_network_proxy_compatibility_for_platform( @@ -142,7 +213,8 @@ fn windows_network_proxy_compatibility_allows_valid_combinations() -> std::io::R #[cfg(target_os = "windows")] #[tokio::test] -async fn hand_edited_network_proxy_and_unelevated_sandbox_is_rejected() -> std::io::Result<()> { +async fn network_proxy_without_elevated_only_sandbox_requirement_is_rejected() -> std::io::Result<()> +{ let codex_home = TempDir::new()?; let cwd = TempDir::new()?; std::fs::write(cwd.path().join(".git"), "gitdir: nowhere")?; @@ -156,7 +228,7 @@ async fn hand_edited_network_proxy_and_unelevated_sandbox_is_rejected() -> std:: }), features: Some(toml::from_str("network_proxy = true").expect("valid features")), windows: Some(WindowsToml { - sandbox: Some(WindowsSandboxModeToml::Unelevated), + sandbox: Some(WindowsSandboxModeToml::Elevated), sandbox_private_desktop: None, }), ..Default::default() @@ -168,12 +240,12 @@ async fn hand_edited_network_proxy_and_unelevated_sandbox_is_rejected() -> std:: codex_home.abs(), ) .await - .expect_err("hand-edited incompatible network and sandbox settings should fail"); + .expect_err("network proxy without elevated-only sandbox requirements should fail"); assert!(matches!( err.get_ref() .and_then(|source| source.downcast_ref::()), - Some(ConstraintError::NetworkProxyIncompatibleWithUnelevatedWindowsSandbox) + Some(ConstraintError::NetworkProxyRequiresElevatedWindowsSandboxRequirement) )); Ok(()) } @@ -1571,7 +1643,7 @@ async fn network_proxy_feature_matrix_preserves_sandbox_network_semantics() -> s ..Default::default() }, }; - let config = Config::load_from_base_config_with_overrides( + let config = load_config_with_elevated_only_windows_sandbox_requirement( base_config, ConfigOverrides { cwd: Some(cwd.path().to_path_buf()), @@ -1616,6 +1688,13 @@ sandbox = "elevated" )?; let config = ConfigBuilder::without_managed_config_for_tests() .codex_home(codex_home.path().to_path_buf()) + .cloud_config_bundle( + CloudConfigBundleFixture::loader_with_enterprise_requirement( + r#"[windows] +allowed_sandbox_implementations = ["elevated"] +"#, + ), + ) .cli_overrides(vec![ ( "features.network_proxy.enabled".to_string(), @@ -1774,7 +1853,7 @@ allowed_sandbox_implementations = ["elevated"] async fn network_proxy_feature_uses_profile_network_proxy_settings() -> std::io::Result<()> { let codex_home = TempDir::new()?; let cwd = TempDir::new()?; - let config = Config::load_from_base_config_with_overrides( + let config = load_config_with_elevated_only_windows_sandbox_requirement( ConfigToml { features: Some(toml::from_str("network_proxy = true").expect("valid features")), default_permissions: Some("dev".to_string()), diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index c46f3737b1..2708d0efe1 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -622,6 +622,31 @@ fn validate_windows_sandbox_network_proxy_compatibility_for_platform( Ok(()) } +fn validate_windows_network_proxy_requirements( + requirements: &ConfigRequirementsToml, + network_proxy_configured: bool, +) -> std::io::Result<()> { + validate_windows_network_proxy_requirements_for_platform( + cfg!(target_os = "windows"), + requirements, + network_proxy_configured, + ) +} + +fn validate_windows_network_proxy_requirements_for_platform( + is_windows: bool, + requirements: &ConfigRequirementsToml, + network_proxy_configured: bool, +) -> std::io::Result<()> { + if is_windows + && network_proxy_configured + && !requirements.allows_only_elevated_windows_sandbox() + { + return Err(ConstraintError::NetworkProxyRequiresElevatedWindowsSandboxRequirement.into()); + } + Ok(()) +} + /// Configured thread persistence backend. #[derive(Debug, Clone, PartialEq, Eq, Default)] pub enum ThreadStoreConfig { @@ -3763,6 +3788,10 @@ impl Config { network_requirements, &network_permission_profile, )?; + validate_windows_network_proxy_requirements( + config_layer_stack.requirements_toml(), + network.is_some(), + )?; validate_windows_sandbox_network_proxy_compatibility( windows_sandbox_level, network.is_some(), @@ -4166,6 +4195,10 @@ impl Config { self.config_layer_stack.requirements().network.clone(), permission_profile, )?; + validate_windows_network_proxy_requirements( + self.config_layer_stack.requirements_toml(), + network.is_some(), + )?; validate_windows_sandbox_network_proxy_compatibility( WindowsSandboxLevel::from_config(self), network.is_some(), diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 5ebce9aa42..fe807cc931 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -4757,9 +4757,17 @@ async fn active_profile_update_rebuilds_network_proxy_config() -> std::io::Resul codex_home.path().join(codex_config::CONFIG_TOML_FILE), toml::to_string(&base_config).expect("serialize config"), )?; + let elevated_only_windows_sandbox_requirement = || { + codex_config::test_support::CloudConfigBundleFixture::loader_with_enterprise_requirement( + r#"[windows] +allowed_sandbox_implementations = ["elevated"] +"#, + ) + }; let locked_config = Arc::new( ConfigBuilder::default() .codex_home(codex_home.path().to_path_buf()) + .cloud_config_bundle(elevated_only_windows_sandbox_requirement()) .harness_overrides(ConfigOverrides { cwd: Some(cwd.path().to_path_buf()), ..Default::default() @@ -4778,6 +4786,7 @@ async fn active_profile_update_rebuilds_network_proxy_config() -> std::io::Resul ); let selected_config = ConfigBuilder::default() .codex_home(codex_home.path().to_path_buf()) + .cloud_config_bundle(elevated_only_windows_sandbox_requirement()) .harness_overrides(ConfigOverrides { cwd: Some(cwd.path().to_path_buf()), default_permissions: Some("web-enabled".to_string()),