diff --git a/codex-rs/app-server/src/config_manager.rs b/codex-rs/app-server/src/config_manager.rs index d3d7609d65..75193c6f23 100644 --- a/codex-rs/app-server/src/config_manager.rs +++ b/codex-rs/app-server/src/config_manager.rs @@ -24,6 +24,12 @@ use toml::Value as TomlValue; use tracing::instrument; use tracing::warn; +#[derive(Clone, Copy)] +enum ConfigLoadPhase { + CloudConfigBootstrap, + Authoritative, +} + /// Shared app-server entry point for loading effective Codex configuration. #[derive(Clone)] pub(crate) struct ConfigManager { @@ -148,6 +154,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, + ConfigLoadPhase::CloudConfigBootstrap, + ) + .await + } + pub(crate) async fn load_latest_config_for_thread( &self, thread_config: &Config, @@ -215,11 +232,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, + ConfigLoadPhase::Authoritative, + ) + .await + } + + async fn load_with_cli_overrides_inner( &self, cli_overrides: &[(String, TomlValue)], request_overrides: Option>, mut typesafe_overrides: ConfigOverrides, fallback_cwd: Option, + load_phase: ConfigLoadPhase, ) -> std::io::Result { let mut request_overrides = request_overrides.unwrap_or_default(); if let Some(value) = request_overrides.remove("bypass_hook_trust") { @@ -240,7 +275,7 @@ impl ConfigManager { ) .collect::>(); - let mut config = codex_core::config::ConfigBuilder::default() + let 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 +283,13 @@ 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()); + let mut config = match load_phase { + ConfigLoadPhase::CloudConfigBootstrap => { + builder.build_for_cloud_config_bootstrap().await? + } + ConfigLoadPhase::Authoritative => 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 9b7b26406a..80fe28771f 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; @@ -300,6 +301,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 } => { @@ -497,7 +504,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) => { @@ -523,7 +530,7 @@ pub async fn run_main_with_transport_options( { Ok(config) => config, Err(err) => { - if strict_config { + if strict_config || is_unrecoverable_windows_network_config_error(&err) { return Err(err); } @@ -1323,14 +1330,63 @@ fn analytics_rpc_transport(transport: &AppServerTransport) -> AppServerRpcTransp #[cfg(test)] mod tests { use super::LogFormat; + #[cfg(target_os = "windows")] + 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 dfe7555d58..78f5ba62d6 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 c0917d933b..800ed01439 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 00af972336..45058ef82a 100644 --- a/codex-rs/config/src/config_requirements.rs +++ b/codex-rs/config/src/config_requirements.rs @@ -754,6 +754,16 @@ impl WindowsRequirementsToml { pub fn is_empty(&self) -> bool { self.allowed_sandbox_implementations.is_none() } + + pub fn allows_only_elevated_sandbox(&self) -> bool { + let Some(modes) = self.allowed_sandbox_implementations.as_deref() else { + return false; + }; + !modes.is_empty() + && modes + .iter() + .all(|&mode| mode == WindowsSandboxModeToml::Elevated) + } } #[derive(Deserialize, Debug, Clone, Default, PartialEq, Eq)] @@ -881,6 +891,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 8628f3909c..5449e235e7 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 89f8c23ad8..c314780955 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 0bc79a88e0..646407cb72 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -601,6 +601,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 { @@ -1230,6 +1280,12 @@ impl AuthManagerConfig for Config { } } +#[derive(Clone, Copy)] +enum ConfigBuildPhase { + CloudConfigBootstrap, + Authoritative, +} + #[derive(Clone, Default)] pub struct ConfigBuilder { codex_home: Option, @@ -1288,10 +1344,15 @@ impl ConfigBuilder { 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 + Box::pin(self.build_inner(ConfigBuildPhase::Authoritative)).await } - async fn build_inner(self) -> std::io::Result { + /// Builds the preliminary config needed to initialize cloud config loading. + pub async fn build_for_cloud_config_bootstrap(self) -> std::io::Result { + Box::pin(self.build_inner(ConfigBuildPhase::CloudConfigBootstrap)).await + } + + async fn build_inner(self, build_phase: ConfigBuildPhase) -> std::io::Result { let Self { codex_home, cli_overrides, @@ -1376,12 +1437,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, + build_phase, ) .await?; config.config_lock_toml = Some(Arc::new(expected_lock_config)); @@ -1390,12 +1452,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, + build_phase, ) .await } @@ -2967,6 +3030,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, + ConfigBuildPhase::Authoritative, + ) + .await + } + + async fn load_config_with_layer_stack_and_options( + fs: &dyn ExecutorFileSystem, + cfg: ConfigToml, + overrides: ConfigOverrides, + codex_home: AbsolutePathBuf, + config_layer_stack: ConfigLayerStack, + build_phase: ConfigBuildPhase, ) -> std::io::Result { // Keep the large config-construction future off small test thread stacks. Box::pin(async move { @@ -3764,6 +3846,17 @@ impl Config { network_requirements, &network_permission_profile, )?; + if matches!(build_phase, ConfigBuildPhase::Authoritative) { + 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(), @@ -4157,11 +4250,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 a5cef5ca72..9551c34af0 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.enabled } diff --git a/codex-rs/core/src/context/environment_context.rs b/codex-rs/core/src/context/environment_context.rs index 2f795a8ea3..cae2d00ea2 100644 --- a/codex-rs/core/src/context/environment_context.rs +++ b/codex-rs/core/src/context/environment_context.rs @@ -212,22 +212,26 @@ pub(crate) fn push_xml_escaped_text(rendered: &mut String, value: &str) { #[derive(Debug, Clone, PartialEq, Eq, Default)] pub(crate) struct NetworkContext { + enabled: bool, allowed_domains: Vec, denied_domains: Vec, } impl NetworkContext { - pub(crate) fn new(allowed_domains: Vec, denied_domains: Vec) -> Self { + pub(crate) fn new(enabled: bool, allowed: Vec, denied: Vec) -> Self { Self { - allowed_domains, - denied_domains, + enabled, + allowed_domains: allowed, + denied_domains: denied, } } pub(super) fn render(&self) -> String { - let mut rendered = "".to_string(); - Self::push_rendered_domain_element(&mut rendered, "allowed", &self.allowed_domains); - Self::push_rendered_domain_element(&mut rendered, "denied", &self.denied_domains); + let mut rendered = format!("", self.enabled); + if self.enabled { + Self::push_rendered_domain_element(&mut rendered, "allowed", &self.allowed_domains); + Self::push_rendered_domain_element(&mut rendered, "denied", &self.denied_domains); + } rendered.push_str(""); rendered } diff --git a/codex-rs/core/src/context/world_state/environment.rs b/codex-rs/core/src/context/world_state/environment.rs index d34124ecf5..c2fd170e30 100644 --- a/codex-rs/core/src/context/world_state/environment.rs +++ b/codex-rs/core/src/context/world_state/environment.rs @@ -369,6 +369,7 @@ fn network_from_turn_context(turn_context: &TurnContext) -> Option Result<()> { current_date: Some("2026-06-19".to_string()), timezone: Some("UTC".to_string()), network: Some(NetworkContext::new( + /*enabled*/ true, vec!["old.example.com".to_string()], vec![], )), @@ -131,6 +132,7 @@ fn persisted_turn_context_values_render_a_diff() -> Result<()> { current_date: Some("2026-06-20".to_string()), timezone: Some("America/Los_Angeles".to_string()), network: Some(NetworkContext::new( + /*enabled*/ true, vec!["new.example.com".to_string()], vec!["blocked.example.com".to_string()], )), diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 015b20715a..81f38eb0e9 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 1a4477599d..9987b7fb79 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 b5093d7d01..06d90ada0c 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -1131,6 +1131,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 a1103eb08b..fc28d575f8 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -963,7 +963,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 8d01d264db..98ecb5fff8 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -936,7 +936,10 @@ async fn new_turn_refreshes_managed_network_proxy_for_sandbox_change() -> anyhow let (session, _turn_context) = make_session_and_context().await; let initial_permission_profile = PermissionProfile::workspace_write(); - let mut network_config = NetworkProxyConfig::default(); + let mut network_config = NetworkProxyConfig { + enabled: true, + ..Default::default() + }; network_config.set_allowed_domains(vec!["evil.com".to_string()]); let requirements = NetworkConstraints { domains: Some(NetworkDomainPermissionsToml { @@ -1190,6 +1193,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(); @@ -4839,9 +4870,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() @@ -4860,6 +4899,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()), @@ -8012,17 +8052,32 @@ async fn spawn_task_does_not_update_previous_turn_settings_for_non_run_turn_task } #[tokio::test] -async fn record_context_updates_emits_environment_item_for_network_changes() { - let (session, previous_context) = make_session_and_context().await; - let previous_context = Arc::new(previous_context); - let mut current_context = previous_context - .with_model( - previous_context.model_info.slug.clone(), - &session.services.models_manager, +async fn record_context_updates_emits_disabled_network_transition() { + let (session, mut previous_context) = make_session_and_context().await; + let permission_profile = PermissionProfile::workspace_write(); + let network_config = NetworkProxyConfig { + enabled: true, + ..Default::default() + }; + let network_spec = crate::config::NetworkProxySpec::from_config_and_constraints( + network_config, + /*requirements*/ None, + &permission_profile, + ) + .expect("build network proxy spec"); + let started_proxy = network_spec + .start_proxy( + &permission_profile, + /*policy_decider*/ None, + /*blocked_request_observer*/ None, + /*enable_network_approval_flow*/ false, + crate::config::NetworkProxyAuditMetadata::default(), ) - .await; + .await + .expect("start network proxy"); + previous_context.network = Some(started_proxy.proxy()); - let mut config = (*current_context.config).clone(); + let mut config = (*previous_context.config).clone(); let mut requirements = config.config_layer_stack.requirements().clone(); requirements.network = Some(Sourced::new( NetworkConstraints { @@ -8057,7 +8112,15 @@ async fn record_context_updates_emits_environment_item_for_network_changes() { config.config_layer_stack.requirements_toml().clone(), ) .expect("rebuild config layer stack with network requirements"); - current_context.config = Arc::new(config); + previous_context.config = Arc::new(config); + let previous_context = Arc::new(previous_context); + let mut current_context = previous_context + .with_model( + previous_context.model_info.slug.clone(), + &session.services.models_manager, + ) + .await; + current_context.network = None; let update_items = record_context_update_items(&session, previous_context, current_context).await; @@ -8066,9 +8129,7 @@ async fn record_context_updates_emits_environment_item_for_network_changes() { .into_iter() .find(|text| text.contains("")) .expect("environment update item should be emitted"); - assert!(environment_update.contains( - "api.example.comblocked.example.com" - )); + assert!(environment_update.contains("")); } #[tokio::test] diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index db233397b0..f5112dce3b 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -980,6 +980,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 29ed4875e0..47c4a6042f 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 51d2d6c98e..070cb32b1d 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 a57281145c..eec3213668 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 84ccd0d8cc..c9ef212462 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,