From 36e65799179f12ae8d0647d6ead50ebc59abd69f Mon Sep 17 00:00:00 2001 From: David Wiesen Date: Tue, 7 Jul 2026 10:08:21 -0700 Subject: [PATCH] Require elevated Windows policy for managed network --- codex-rs/app-server/src/lib.rs | 27 ++++++- .../windows_sandbox_processor.rs | 9 +++ codex-rs/config/src/config_requirements.rs | 80 ++++++++++++++++++ codex-rs/config/src/constraint.rs | 20 +++++ codex-rs/core/src/config/config_tests.rs | 81 +++++++++++++++++++ codex-rs/core/src/config/mod.rs | 38 ++++++++- codex-rs/core/src/exec.rs | 12 ++- codex-rs/core/src/exec_tests.rs | 8 +- codex-rs/core/tests/common/lib.rs | 3 + codex-rs/sandboxing/src/manager.rs | 3 +- codex-rs/sandboxing/src/windows.rs | 10 +-- .../src/unified_exec/mod.rs | 7 +- 12 files changed, 274 insertions(+), 24 deletions(-) diff --git a/codex-rs/app-server/src/lib.rs b/codex-rs/app-server/src/lib.rs index fa6b6cd811..2766882b86 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 } => { @@ -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,6 +1371,7 @@ 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(debug_assertions)] @@ -1372,6 +1380,23 @@ mod tests { use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; + #[test] + fn windows_network_config_errors_are_unrecoverable() { + for error in [ + codex_config::ConstraintError::ManagedNetworkRequiresElevatedWindowsSandbox { + requirement_source: codex_config::RequirementSource::Unknown, + }, + codex_config::ConstraintError::NetworkProxyIncompatibleWithUnelevatedWindowsSandbox, + ] { + let error = std::io::Error::from(error); + assert!(is_unrecoverable_windows_network_config_error(&error)); + } + + assert!(!is_unrecoverable_windows_network_config_error( + &std::io::Error::other("unrelated config error") + )); + } + #[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/windows_sandbox_processor.rs b/codex-rs/app-server/src/request_processors/windows_sandbox_processor.rs index c0917d933b..bd26c44c9e 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,15 @@ 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.is_some(), + ) + .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..4206c4a8bb 100644 --- a/codex-rs/config/src/config_requirements.rs +++ b/codex-rs/config/src/config_requirements.rs @@ -1299,6 +1299,23 @@ 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(), + }, + ); + } + } + let approval_policy = match allowed_approval_policies { Some(Sourced { value: policies, @@ -2795,6 +2812,63 @@ allowed_approvals_reviewers = ["user"] Ok(()) } + #[test] + fn managed_network_requires_elevated_only_windows_sandbox() -> Result<()> { + for windows_requirements in [ + "", + r#" + [windows] + allowed_sandbox_implementations = ["unelevated"] + "#, + r#" + [windows] + allowed_sandbox_implementations = ["elevated", "unelevated"] + "#, + ] { + let config: ConfigRequirementsToml = from_str(&format!( + r#" + [experimental_network] + enabled = false + + {windows_requirements} + "# + ))?; + + assert_eq!( + ConfigRequirements::try_from(with_unknown_source(config)), + Err( + ConstraintError::ManagedNetworkRequiresElevatedWindowsSandbox { + requirement_source: RequirementSource::Unknown, + } + ) + ); + } + + Ok(()) + } + + #[test] + fn managed_network_accepts_elevated_only_windows_sandbox() -> Result<()> { + let config: ConfigRequirementsToml = from_str( + r#" + [experimental_network] + enabled = true + + [windows] + allowed_sandbox_implementations = ["elevated"] + "#, + )?; + + let requirements = ConfigRequirements::try_from(with_unknown_source(config))?; + assert!(requirements.network.is_some()); + assert_eq!( + requirements.windows_sandbox_mode.value(), + Some(WindowsSandboxModeToml::Elevated) + ); + + Ok(()) + } + #[test] fn deserialize_legacy_allowed_approvals_reviewer() -> Result<()> { let toml_str = r#" @@ -3335,6 +3409,9 @@ command = "python3 /enterprise/hooks/pre.py" [experimental_network.unix_sockets] "/tmp/example.sock" = "allow" "/tmp/blocked.sock" = "deny" + + [windows] + allowed_sandbox_implementations = ["elevated"] "#; let source = RequirementSource::LegacyManagedConfigTomlFromMdm; @@ -3408,6 +3485,9 @@ command = "python3 /enterprise/hooks/pre.py" denied_domains = ["blocked.example.com"] allow_unix_sockets = ["/tmp/example.sock"] allow_local_binding = false + + [windows] + allowed_sandbox_implementations = ["elevated"] "#; let source = RequirementSource::LegacyManagedConfigTomlFromMdm; diff --git a/codex-rs/config/src/constraint.rs b/codex-rs/config/src/constraint.rs index 8628f3909c..b0c2da6763 100644 --- a/codex-rs/config/src/constraint.rs +++ b/codex-rs/config/src/constraint.rs @@ -33,6 +33,18 @@ pub enum ConstraintError { requirement_source: RequirementSource, reason: String, }, + + #[error( + "managed network requirements (set by {requirement_source}) require `[windows] allowed_sandbox_implementations = [\"elevated\"]`" + )] + ManagedNetworkRequiresElevatedWindowsSandbox { + requirement_source: RequirementSource, + }, + + #[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 +53,14 @@ impl ConstraintError { field_name: field_name.into(), } } + + pub fn is_windows_network_configuration_error(&self) -> bool { + matches!( + self, + Self::ManagedNetworkRequiresElevatedWindowsSandbox { .. } + | 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 40ebc64a0b..d39bdcb5c3 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -99,6 +99,84 @@ use serde::Deserialize; use tempfile::tempdir; use super::*; + +#[test] +fn windows_network_proxy_requires_elevated_sandbox() { + let err = validate_windows_sandbox_network_proxy_compatibility_for_platform( + /*is_windows*/ true, + WindowsSandboxLevel::RestrictedToken, + /*network_proxy_configured*/ true, + ) + .expect_err("unelevated Windows sandbox should reject configured network proxy"); + + assert!(matches!( + err.get_ref() + .and_then(|source| source.downcast_ref::()), + Some(ConstraintError::NetworkProxyIncompatibleWithUnelevatedWindowsSandbox) + )); +} + +#[test] +fn windows_network_proxy_compatibility_allows_valid_combinations() -> std::io::Result<()> { + for (windows_sandbox_level, network_proxy_configured) in [ + (WindowsSandboxLevel::Disabled, false), + (WindowsSandboxLevel::Disabled, true), + (WindowsSandboxLevel::RestrictedToken, false), + (WindowsSandboxLevel::Elevated, false), + (WindowsSandboxLevel::Elevated, true), + ] { + validate_windows_sandbox_network_proxy_compatibility_for_platform( + /*is_windows*/ true, + windows_sandbox_level, + network_proxy_configured, + )?; + } + validate_windows_sandbox_network_proxy_compatibility_for_platform( + /*is_windows*/ false, + WindowsSandboxLevel::RestrictedToken, + /*network_proxy_configured*/ true, + )?; + + Ok(()) +} + +#[cfg(target_os = "windows")] +#[tokio::test] +async fn hand_edited_network_proxy_and_unelevated_sandbox_is_rejected() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cwd = TempDir::new()?; + std::fs::write(cwd.path().join(".git"), "gitdir: nowhere")?; + + let err = Config::load_from_base_config_with_overrides( + ConfigToml { + sandbox_mode: Some(SandboxMode::WorkspaceWrite), + sandbox_workspace_write: Some(SandboxWorkspaceWrite { + network_access: true, + ..Default::default() + }), + features: Some(toml::from_str("network_proxy = true").expect("valid features")), + windows: Some(WindowsToml { + sandbox: Some(WindowsSandboxModeToml::Unelevated), + sandbox_private_desktop: None, + }), + ..Default::default() + }, + ConfigOverrides { + cwd: Some(cwd.path().to_path_buf()), + ..Default::default() + }, + codex_home.abs(), + ) + .await + .expect_err("hand-edited incompatible network and sandbox settings should fail"); + + assert!(matches!( + err.get_ref() + .and_then(|source| source.downcast_ref::()), + Some(ConstraintError::NetworkProxyIncompatibleWithUnelevatedWindowsSandbox) + )); + Ok(()) +} use core_test_support::PathBufExt; use core_test_support::PathExt; use core_test_support::TempDirExt; @@ -1670,6 +1748,9 @@ async fn experimental_network_requirements_enable_proxy_without_feature() -> std r#" [experimental_network] enabled = true + +[windows] +allowed_sandbox_implementations = ["elevated"] "#, ), ) diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 8c0a165564..c46f3737b1 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -597,6 +597,31 @@ 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::RestrictedToken + { + return Err(ConstraintError::NetworkProxyIncompatibleWithUnelevatedWindowsSandbox.into()); + } + Ok(()) +} + /// Configured thread persistence backend. #[derive(Debug, Clone, PartialEq, Eq, Default)] pub enum ThreadStoreConfig { @@ -3738,6 +3763,10 @@ impl Config { network_requirements, &network_permission_profile, )?; + validate_windows_sandbox_network_proxy_compatibility( + windows_sandbox_level, + network.is_some(), + )?; let mut helper_readable_roots = get_readable_roots_required_for_codex_runtime( &codex_home, zsh_path.as_ref(), @@ -4132,11 +4161,16 @@ 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, - ) + )?; + validate_windows_sandbox_network_proxy_compatibility( + WindowsSandboxLevel::from_config(self), + network.is_some(), + )?; + Ok(network) } pub fn bundled_skills_enabled(&self) -> bool { 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/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,