diff --git a/codex-rs/app-server/src/config_manager.rs b/codex-rs/app-server/src/config_manager.rs index d3d7609d6535..ce6c7d66739f 100644 --- a/codex-rs/app-server/src/config_manager.rs +++ b/codex-rs/app-server/src/config_manager.rs @@ -148,6 +148,17 @@ impl ConfigManager { .await } + pub(crate) async fn load_config_for_cloud_config_bootstrap(&self) -> std::io::Result { + self.load_with_cli_overrides_inner( + &self.current_cli_overrides(), + /*request_overrides*/ None, + ConfigOverrides::default(), + /*fallback_cwd*/ None, + /*defer_windows_network_validation*/ true, + ) + .await + } + pub(crate) async fn load_latest_config_for_thread( &self, thread_config: &Config, @@ -215,11 +226,29 @@ impl ConfigManager { #[instrument(level = "trace", skip_all)] pub(crate) async fn load_with_cli_overrides( + &self, + cli_overrides: &[(String, TomlValue)], + request_overrides: Option>, + typesafe_overrides: ConfigOverrides, + fallback_cwd: Option, + ) -> std::io::Result { + self.load_with_cli_overrides_inner( + cli_overrides, + request_overrides, + typesafe_overrides, + fallback_cwd, + /*defer_windows_network_validation*/ false, + ) + .await + } + + async fn load_with_cli_overrides_inner( &self, cli_overrides: &[(String, TomlValue)], request_overrides: Option>, mut typesafe_overrides: ConfigOverrides, fallback_cwd: Option, + defer_windows_network_validation: bool, ) -> std::io::Result { let mut request_overrides = request_overrides.unwrap_or_default(); if let Some(value) = request_overrides.remove("bypass_hook_trust") { @@ -240,7 +269,7 @@ impl ConfigManager { ) .collect::>(); - let mut config = codex_core::config::ConfigBuilder::default() + let mut builder = codex_core::config::ConfigBuilder::default() .codex_home(self.codex_home.clone()) .cli_overrides(merged_cli_overrides) .loader_overrides(self.loader_overrides.clone()) @@ -248,9 +277,11 @@ impl ConfigManager { .harness_overrides(typesafe_overrides) .fallback_cwd(fallback_cwd) .cloud_config_bundle(self.current_cloud_config_bundle()) - .thread_config_loader(self.current_thread_config_loader()) - .build() - .await?; + .thread_config_loader(self.current_thread_config_loader()); + if defer_windows_network_validation { + builder = builder.defer_windows_network_validation_for_bootstrap(); + } + let mut config = builder.build().await?; self.apply_runtime_feature_enablement(&mut config); self.apply_arg0_paths(&mut config); Ok(config) diff --git a/codex-rs/app-server/src/lib.rs b/codex-rs/app-server/src/lib.rs index fa6b6cd811f7..5b89b22d9b01 100644 --- a/codex-rs/app-server/src/lib.rs +++ b/codex-rs/app-server/src/lib.rs @@ -54,6 +54,7 @@ use codex_app_server_protocol::TextPosition as AppTextPosition; use codex_app_server_protocol::TextRange as AppTextRange; use codex_config::ConfigLayerSource; use codex_config::ConfigLoadError; +use codex_config::ConstraintError; use codex_config::TextRange as CoreTextRange; use codex_core::ExecPolicyError; use codex_core::check_execpolicy_for_warnings; @@ -298,6 +299,12 @@ fn config_error_location(err: &std::io::Error) -> Option<(String, AppTextRange)> }) } +fn is_unrecoverable_windows_network_config_error(err: &std::io::Error) -> bool { + err.get_ref() + .and_then(|source| source.downcast_ref::()) + .is_some_and(ConstraintError::is_windows_network_configuration_error) +} + fn exec_policy_warning_location(err: &ExecPolicyError) -> (Option, Option) { match err { ExecPolicyError::ParsePolicy { path, source } => { @@ -495,7 +502,7 @@ pub async fn run_main_with_transport_options( Arc::new(NoopThreadConfigLoader), ); match config_manager - .load_latest_config(/*fallback_cwd*/ None) + .load_config_for_cloud_config_bootstrap() .await { Ok(config) => { @@ -521,7 +528,7 @@ pub async fn run_main_with_transport_options( { Ok(config) => (config, true), Err(err) => { - if strict_config { + if strict_config || is_unrecoverable_windows_network_config_error(&err) { return Err(err); } @@ -1364,14 +1371,62 @@ fn analytics_rpc_transport(transport: &AppServerTransport) -> AppServerRpcTransp #[cfg(test)] mod tests { use super::LogFormat; + use super::is_unrecoverable_windows_network_config_error; #[cfg(debug_assertions)] use super::loader_overrides_with_test_user_config_file; + #[cfg(target_os = "windows")] + use crate::config_manager::ConfigManager; #[cfg(debug_assertions)] use codex_config::LoaderOverrides; #[cfg(debug_assertions)] use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; + #[cfg(target_os = "windows")] + #[tokio::test] + async fn cloud_config_bootstrap_defers_network_validation_and_preserves_cli_overrides() { + let codex_home = tempfile::tempdir().expect("create Codex home"); + std::fs::write( + codex_home.path().join(codex_config::CONFIG_TOML_FILE), + r#" +sandbox_mode = "workspace-write" + +[sandbox_workspace_write] +network_access = true + +[features] +network_proxy = true + +[windows] +sandbox = "elevated" +"#, + ) + .expect("write config"); + let chatgpt_base_url = "https://cloud-config.example.test".to_string(); + let config_manager = ConfigManager::new_for_tests( + codex_home.path().to_path_buf(), + vec![( + "chatgpt_base_url".to_string(), + toml::Value::String(chatgpt_base_url.clone()), + )], + codex_config::LoaderOverrides::without_managed_config_for_tests(), + codex_config::CloudConfigBundleLoader::default(), + ); + + let config = config_manager + .load_config_for_cloud_config_bootstrap() + .await + .expect("bootstrap config should defer Windows network validation"); + assert_eq!(config.chatgpt_base_url, chatgpt_base_url); + assert!(config.permissions.network.is_some()); + + let err = config_manager + .load_latest_config(/*fallback_cwd*/ None) + .await + .expect_err("authoritative config should enforce Windows network validation"); + assert!(is_unrecoverable_windows_network_config_error(&err)); + } + #[test] fn log_format_from_env_value_matches_json_values_case_insensitively() { assert_eq!(LogFormat::from_env_value(Some("json")), LogFormat::Json); diff --git a/codex-rs/app-server/src/request_processors/command_exec_processor.rs b/codex-rs/app-server/src/request_processors/command_exec_processor.rs index dfe7555d5852..78f5ba62d62b 100644 --- a/codex-rs/app-server/src/request_processors/command_exec_processor.rs +++ b/codex-rs/app-server/src/request_processors/command_exec_processor.rs @@ -204,6 +204,7 @@ impl CommandExecRequestProcessor { network_proxy_permission_profile, managed_network_requirements_enabled, windows_sandbox_workspace_roots, + windows_sandbox_level, ) = if let Some(permission_profile) = permission_profile { let overrides = ConfigOverrides { cwd: Some(cwd.to_path_buf()), @@ -232,6 +233,7 @@ impl CommandExecRequestProcessor { config.permissions.permission_profile().clone(), config.managed_network_requirements_enabled(), config.effective_workspace_roots(), + WindowsSandboxLevel::from_config(&config), ) } else if let Some(policy) = sandbox_policy.map(|policy| policy.to_core()) { self.config @@ -258,6 +260,7 @@ impl CommandExecRequestProcessor { self.config.permissions.permission_profile().clone(), self.config.managed_network_requirements_enabled(), self.config.effective_workspace_roots(), + windows_sandbox_level, ) } else { ( @@ -266,9 +269,13 @@ impl CommandExecRequestProcessor { self.config.permissions.permission_profile().clone(), self.config.managed_network_requirements_enabled(), self.config.effective_workspace_roots(), + windows_sandbox_level, ) }; - let started_network_proxy = match network_proxy_spec.as_ref() { + let started_network_proxy = match network_proxy_spec + .as_ref() + .filter(|spec| !cfg!(target_os = "windows") || spec.enabled()) + { Some(spec) => match spec .start_proxy( &network_proxy_permission_profile, diff --git a/codex-rs/app-server/src/request_processors/windows_sandbox_processor.rs b/codex-rs/app-server/src/request_processors/windows_sandbox_processor.rs index c0917d933bf6..800ed0143929 100644 --- a/codex-rs/app-server/src/request_processors/windows_sandbox_processor.rs +++ b/codex-rs/app-server/src/request_processors/windows_sandbox_processor.rs @@ -63,6 +63,19 @@ impl WindowsSandboxRequestProcessor { config.config_layer_stack.requirements(), params.mode, )?; + let target_windows_sandbox_level = match setup_mode { + CoreWindowsSandboxSetupMode::Elevated => WindowsSandboxLevel::Elevated, + CoreWindowsSandboxSetupMode::Unelevated => WindowsSandboxLevel::RestrictedToken, + }; + codex_core::config::validate_windows_sandbox_network_proxy_compatibility( + target_windows_sandbox_level, + config + .permissions + .network + .as_ref() + .is_some_and(codex_core::config::NetworkProxySpec::enabled), + ) + .map_err(|err| invalid_request(err.to_string()))?; self.outgoing .send_response( diff --git a/codex-rs/config/src/config_requirements.rs b/codex-rs/config/src/config_requirements.rs index 00af97233641..fd19c79a7879 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, diff --git a/codex-rs/config/src/constraint.rs b/codex-rs/config/src/constraint.rs index 8628f3909c1d..5449e235e7e6 100644 --- a/codex-rs/config/src/constraint.rs +++ b/codex-rs/config/src/constraint.rs @@ -33,6 +33,16 @@ pub enum ConstraintError { requirement_source: RequirementSource, reason: String, }, + + #[error( + "network proxy configuration requires `[windows] allowed_sandbox_implementations = [\"elevated\"]` in requirements" + )] + NetworkProxyRequiresElevatedWindowsSandboxRequirement, + + #[error( + "the network proxy is incompatible with the unelevated Windows sandbox; set `windows.sandbox = \"elevated\"` or disable the network proxy" + )] + NetworkProxyIncompatibleWithUnelevatedWindowsSandbox, } impl ConstraintError { @@ -41,6 +51,14 @@ impl ConstraintError { field_name: field_name.into(), } } + + pub fn is_windows_network_configuration_error(&self) -> bool { + matches!( + self, + Self::NetworkProxyRequiresElevatedWindowsSandboxRequirement + | Self::NetworkProxyIncompatibleWithUnelevatedWindowsSandbox + ) + } } pub type ConstraintResult = Result; diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 40ebc64a0b77..3865c16e10dc 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -99,6 +99,92 @@ use serde::Deserialize; 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_validation() { + let elevated_only = ConfigRequirementsToml { + windows: Some(codex_config::WindowsRequirementsToml { + allowed_sandbox_implementations: Some(vec![WindowsSandboxModeToml::Elevated]), + }), + ..Default::default() + }; + for (is_windows, requirements, network_proxy_enabled, allowed) in [ + (true, ConfigRequirementsToml::default(), true, false), + (true, ConfigRequirementsToml::default(), false, true), + (true, elevated_only, true, true), + ( + true, + ConfigRequirementsToml { + windows: Some(codex_config::WindowsRequirementsToml { + allowed_sandbox_implementations: Some(vec![ + WindowsSandboxModeToml::Elevated, + WindowsSandboxModeToml::Unelevated, + ]), + }), + ..Default::default() + }, + true, + false, + ), + (false, ConfigRequirementsToml::default(), true, true), + ] { + assert_eq!( + validate_windows_network_proxy_requirements_for_platform( + is_windows, + &requirements, + network_proxy_enabled, + ) + .is_ok(), + allowed + ); + } + + for (is_windows, sandbox_level, network_proxy_enabled, allowed) in [ + (true, WindowsSandboxLevel::Disabled, true, false), + (true, WindowsSandboxLevel::RestrictedToken, true, false), + (true, WindowsSandboxLevel::Elevated, true, true), + (true, WindowsSandboxLevel::RestrictedToken, false, true), + (false, WindowsSandboxLevel::RestrictedToken, true, true), + ] { + assert_eq!( + validate_windows_sandbox_network_proxy_compatibility_for_platform( + is_windows, + sandbox_level, + network_proxy_enabled, + ) + .is_ok(), + allowed + ); + } +} use core_test_support::PathBufExt; use core_test_support::PathExt; use core_test_support::TempDirExt; @@ -1493,7 +1579,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()), @@ -1538,6 +1624,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(), @@ -1670,6 +1763,9 @@ async fn experimental_network_requirements_enable_proxy_without_feature() -> std r#" [experimental_network] enabled = true + +[windows] +allowed_sandbox_implementations = ["elevated"] "#, ), ) @@ -1693,7 +1789,7 @@ enabled = true 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 8c0a165564db..260ea9c0c970 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -597,6 +597,56 @@ fn build_network_proxy_spec( }) } +pub fn validate_windows_sandbox_network_proxy_compatibility( + windows_sandbox_level: WindowsSandboxLevel, + network_proxy_configured: bool, +) -> std::io::Result<()> { + validate_windows_sandbox_network_proxy_compatibility_for_platform( + cfg!(target_os = "windows"), + windows_sandbox_level, + network_proxy_configured, + ) +} + +fn validate_windows_sandbox_network_proxy_compatibility_for_platform( + is_windows: bool, + windows_sandbox_level: WindowsSandboxLevel, + network_proxy_configured: bool, +) -> std::io::Result<()> { + if is_windows + && network_proxy_configured + && windows_sandbox_level != WindowsSandboxLevel::Elevated + { + return Err(ConstraintError::NetworkProxyIncompatibleWithUnelevatedWindowsSandbox.into()); + } + 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 { @@ -1237,6 +1287,7 @@ pub struct ConfigBuilder { cloud_config_bundle: CloudConfigBundleLoader, thread_config_loader: Option>, fallback_cwd: Option, + defer_windows_network_validation: bool, } impl ConfigBuilder { @@ -1283,6 +1334,13 @@ impl ConfigBuilder { self } + /// Defers Windows network compatibility checks until an authoritative + /// config load that includes cloud requirements. + pub fn defer_windows_network_validation_for_bootstrap(mut self) -> Self { + self.defer_windows_network_validation = true; + self + } + pub async fn build(self) -> std::io::Result { // Keep the large config-loading future off small runtime thread stacks. Box::pin(self.build_inner()).await @@ -1298,6 +1356,7 @@ impl ConfigBuilder { cloud_config_bundle, thread_config_loader, fallback_cwd, + defer_windows_network_validation, } = self; let codex_home = match codex_home { Some(codex_home) => AbsolutePathBuf::from_absolute_path(codex_home)?, @@ -1373,12 +1432,13 @@ impl ConfigBuilder { config_layer_stack.requirements().clone(), config_layer_stack.requirements_toml().clone(), )?; - let mut config = Config::load_config_with_layer_stack( + let mut config = Config::load_config_with_layer_stack_and_options( LOCAL_FS.as_ref(), lock_config_toml, harness_overrides, codex_home, lock_config_layer_stack, + defer_windows_network_validation, ) .await?; config.config_lock_toml = Some(Arc::new(expected_lock_config)); @@ -1387,12 +1447,13 @@ impl ConfigBuilder { save_fields_resolved_from_model_catalog; return Ok(config); } - Config::load_config_with_layer_stack( + Config::load_config_with_layer_stack_and_options( LOCAL_FS.as_ref(), config_toml, harness_overrides, codex_home, config_layer_stack, + defer_windows_network_validation, ) .await } @@ -2941,6 +3002,25 @@ impl Config { overrides: ConfigOverrides, codex_home: AbsolutePathBuf, config_layer_stack: ConfigLayerStack, + ) -> std::io::Result { + Self::load_config_with_layer_stack_and_options( + fs, + cfg, + overrides, + codex_home, + config_layer_stack, + /*defer_windows_network_validation*/ false, + ) + .await + } + + async fn load_config_with_layer_stack_and_options( + fs: &dyn ExecutorFileSystem, + cfg: ConfigToml, + overrides: ConfigOverrides, + codex_home: AbsolutePathBuf, + config_layer_stack: ConfigLayerStack, + defer_windows_network_validation: bool, ) -> std::io::Result { // Keep the large config-construction future off small test thread stacks. Box::pin(async move { @@ -3738,6 +3818,17 @@ impl Config { network_requirements, &network_permission_profile, )?; + if !defer_windows_network_validation { + let network_proxy_enabled = network.as_ref().is_some_and(NetworkProxySpec::enabled); + validate_windows_network_proxy_requirements( + config_layer_stack.requirements_toml(), + network_proxy_enabled, + )?; + validate_windows_sandbox_network_proxy_compatibility( + windows_sandbox_level, + network_proxy_enabled, + )?; + } let mut helper_readable_roots = get_readable_roots_required_for_codex_runtime( &codex_home, zsh_path.as_ref(), @@ -4132,11 +4223,21 @@ impl Config { NetworkProxyConfig::default() }; - build_network_proxy_spec( + let network = build_network_proxy_spec( configured_network_proxy_config, self.config_layer_stack.requirements().network.clone(), permission_profile, - ) + )?; + let network_proxy_enabled = network.as_ref().is_some_and(NetworkProxySpec::enabled); + validate_windows_network_proxy_requirements( + self.config_layer_stack.requirements_toml(), + network_proxy_enabled, + )?; + validate_windows_sandbox_network_proxy_compatibility( + WindowsSandboxLevel::from_config(self), + network_proxy_enabled, + )?; + Ok(network) } pub fn bundled_skills_enabled(&self) -> bool { diff --git a/codex-rs/core/src/config/network_proxy_spec.rs b/codex-rs/core/src/config/network_proxy_spec.rs index 653e6987f308..0f9dd9f4cade 100644 --- a/codex-rs/core/src/config/network_proxy_spec.rs +++ b/codex-rs/core/src/config/network_proxy_spec.rs @@ -73,7 +73,7 @@ impl ConfigReloader for StaticNetworkProxyReloader { } impl NetworkProxySpec { - pub(crate) fn enabled(&self) -> bool { + pub fn enabled(&self) -> bool { self.config.network.enabled } diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 015b20715af4..81f38eb0e9f6 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -17,6 +17,7 @@ use tokio::io::BufReader; use tokio::process::Child; use tokio_util::sync::CancellationToken; +use crate::config::validate_windows_sandbox_network_proxy_compatibility; use crate::sandboxing::ExecOptions; use crate::sandboxing::ExecRequest; use crate::sandboxing::SandboxPermissions; @@ -411,10 +412,13 @@ pub fn build_exec_request( ) }) .map_err(CodexErr::from)?; - let use_windows_elevated_backend = windows_sandbox_uses_elevated_backend( + validate_windows_sandbox_network_proxy_compatibility( exec_req.windows_sandbox_level, exec_req.network.is_some(), - ); + ) + .map_err(CodexErr::Io)?; + let use_windows_elevated_backend = + windows_sandbox_uses_elevated_backend(exec_req.windows_sandbox_level); exec_req.windows_sandbox_filesystem_overrides = if use_windows_elevated_backend { resolve_windows_elevated_filesystem_overrides( exec_req.sandbox, @@ -658,7 +662,9 @@ async fn exec_windows_sandbox( let command_path = command.first().cloned(); let sandbox_level = windows_sandbox_level; let proxy_enforced = network.is_some(); - let use_elevated = windows_sandbox_uses_elevated_backend(sandbox_level, proxy_enforced); + validate_windows_sandbox_network_proxy_compatibility(sandbox_level, proxy_enforced) + .map_err(CodexErr::Io)?; + let use_elevated = windows_sandbox_uses_elevated_backend(sandbox_level); let additional_deny_write_paths = windows_sandbox_filesystem_overrides .map(|overrides| overrides.additional_deny_write_paths.clone()) .unwrap_or_default(); diff --git a/codex-rs/core/src/exec_tests.rs b/codex-rs/core/src/exec_tests.rs index 1a4477599de3..9987b7fb7945 100644 --- a/codex-rs/core/src/exec_tests.rs +++ b/codex-rs/core/src/exec_tests.rs @@ -397,18 +397,12 @@ fn windows_restricted_token_supports_read_only_profiles() { } #[test] -fn windows_proxy_enforcement_uses_elevated_backend() { +fn windows_backend_selection_uses_configured_sandbox_level() { assert!(!windows_sandbox_uses_elevated_backend( WindowsSandboxLevel::RestrictedToken, - /*proxy_enforced*/ false, - )); - assert!(windows_sandbox_uses_elevated_backend( - WindowsSandboxLevel::RestrictedToken, - /*proxy_enforced*/ true, )); assert!(windows_sandbox_uses_elevated_backend( WindowsSandboxLevel::Elevated, - /*proxy_enforced*/ false, )); } diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index fa240b890ba8..62fc453b352f 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -1127,6 +1127,10 @@ impl Session { spec } }; + if cfg!(target_os = "windows") && !spec.enabled() { + self.services.network_proxy.store(None); + return; + } if let Some(started_proxy) = self.services.network_proxy.load_full() { if let Err(err) = spec.apply_to_started_proxy(started_proxy.as_ref()).await { warn!("failed to refresh managed network proxy for sandbox change: {err}"); diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 08abb4637749..c2bf6b9cff6a 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -971,7 +971,12 @@ impl Session { ) }); let (network_proxy, session_network_proxy) = - if let Some(spec) = config.permissions.network.as_ref() { + if let Some(spec) = config + .permissions + .network + .as_ref() + .filter(|spec| !cfg!(target_os = "windows") || spec.enabled()) + { let current_exec_policy = exec_policy.current(); let (network_proxy, session_network_proxy) = Self::start_managed_network_proxy( spec, diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 5ebce9aa4253..548f8226ab33 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -913,6 +913,7 @@ async fn new_turn_refreshes_managed_network_proxy_for_sandbox_change() -> anyhow let initial_permission_profile = PermissionProfile::workspace_write(); let mut network_config = NetworkProxyConfig::default(); + network_config.network.enabled = true; network_config .network .set_allowed_domains(vec!["evil.com".to_string()]); @@ -1177,6 +1178,34 @@ async fn workspace_write_turns_continue_to_expose_managed_network_proxy() -> any Ok(()) } +#[cfg(target_os = "windows")] +#[tokio::test] +async fn workspace_write_turns_do_not_expose_disabled_managed_network_proxy() -> anyhow::Result<()> +{ + let permission_profile = PermissionProfile::workspace_write(); + let network_spec = crate::config::NetworkProxySpec::from_config_and_constraints( + NetworkProxyConfig::default(), + Some(NetworkConstraints { + enabled: Some(false), + ..Default::default() + }), + &permission_profile, + )?; + + let session = make_session_with_config(move |config| { + config + .permissions + .set_permission_profile(permission_profile) + .expect("test setup should allow permission profile"); + config.permissions.network = Some(network_spec); + }) + .await?; + + let turn_context = session.new_default_turn().await; + assert!(turn_context.network.is_none()); + Ok(()) +} + #[tokio::test] async fn user_shell_commands_do_not_inherit_managed_network_proxy() -> anyhow::Result<()> { let permission_profile = PermissionProfile::workspace_write(); @@ -4757,9 +4786,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 +4815,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()), diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index fe3ec45759bb..3c4c0754b0cd 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -955,6 +955,15 @@ impl UnifiedExecProcessManager { mut spawn_lifecycle: SpawnLifecycleHandle, environment: &codex_exec_server::Environment, ) -> Result { + #[cfg(target_os = "windows")] + if !environment.is_remote() { + crate::config::validate_windows_sandbox_network_proxy_compatibility( + request.windows_sandbox_level, + request.network.is_some(), + ) + .map_err(|err| UnifiedExecError::create_process(err.to_string()))?; + } + let inherited_fds = spawn_lifecycle.inherited_fds(); #[cfg(target_os = "windows")] diff --git a/codex-rs/core/tests/common/lib.rs b/codex-rs/core/tests/common/lib.rs index 29ed4875e067..47c4a6042f80 100644 --- a/codex-rs/core/tests/common/lib.rs +++ b/codex-rs/core/tests/common/lib.rs @@ -223,6 +223,9 @@ pub fn managed_network_requirements_loader() -> CloudConfigBundleLoader { [experimental_network] enabled = true allow_local_binding = true + +[windows] +allowed_sandbox_implementations = ["elevated"] "#, ) } diff --git a/codex-rs/sandboxing/src/manager.rs b/codex-rs/sandboxing/src/manager.rs index 51d2d6c98e59..070cb32b1dda 100644 --- a/codex-rs/sandboxing/src/manager.rs +++ b/codex-rs/sandboxing/src/manager.rs @@ -532,8 +532,7 @@ fn wrap_windows_sandbox_exec_request_for_direct_spawn( let inner_command = std::mem::take(&mut request.command); let proxy_enforced = request.network.is_some(); - let use_elevated = - windows_sandbox_uses_elevated_backend(request.windows_sandbox_level, proxy_enforced); + let use_elevated = windows_sandbox_uses_elevated_backend(request.windows_sandbox_level); let overrides = if use_elevated { resolve_windows_elevated_filesystem_overrides( request.sandbox, diff --git a/codex-rs/sandboxing/src/windows.rs b/codex-rs/sandboxing/src/windows.rs index a57281145c25..eec3213668d2 100644 --- a/codex-rs/sandboxing/src/windows.rs +++ b/codex-rs/sandboxing/src/windows.rs @@ -29,14 +29,8 @@ pub struct WindowsSandboxFilesystemOverrides { pub additional_deny_write_paths: Vec, } -pub fn windows_sandbox_uses_elevated_backend( - sandbox_level: WindowsSandboxLevel, - proxy_enforced: bool, -) -> bool { - // Windows firewall enforcement is tied to the logon-user sandbox identities, so - // proxy-enforced sessions must use that backend even when the configured mode is - // the default restricted-token sandbox. - proxy_enforced || matches!(sandbox_level, WindowsSandboxLevel::Elevated) +pub fn windows_sandbox_uses_elevated_backend(sandbox_level: WindowsSandboxLevel) -> bool { + matches!(sandbox_level, WindowsSandboxLevel::Elevated) } pub fn permission_profile_supports_windows_restricted_token_sandbox( diff --git a/codex-rs/windows-sandbox-rs/src/unified_exec/mod.rs b/codex-rs/windows-sandbox-rs/src/unified_exec/mod.rs index 84ccd0d8cc28..c9ef21246259 100644 --- a/codex-rs/windows-sandbox-rs/src/unified_exec/mod.rs +++ b/codex-rs/windows-sandbox-rs/src/unified_exec/mod.rs @@ -10,6 +10,7 @@ mod backends; use anyhow::Result; +use anyhow::bail; use codex_protocol::config_types::WindowsSandboxLevel; use codex_protocol::models::PermissionProfile; use codex_utils_absolute_path::AbsolutePathBuf; @@ -47,8 +48,12 @@ pub async fn spawn_windows_sandbox_session_for_level( request: WindowsSandboxSessionRequest<'_>, ) -> Result { if request.proxy_enforced - || matches!(request.windows_sandbox_level, WindowsSandboxLevel::Elevated) + && !matches!(request.windows_sandbox_level, WindowsSandboxLevel::Elevated) { + bail!("network proxy enforcement requires the elevated Windows sandbox backend"); + } + + if matches!(request.windows_sandbox_level, WindowsSandboxLevel::Elevated) { backends::elevated::spawn_windows_sandbox_session_elevated_for_permission_profile( request.permission_profile, request.workspace_roots,