diff --git a/codex-rs/app-server/src/config_manager_service.rs b/codex-rs/app-server/src/config_manager_service.rs index 94980bf8e012..d2a639e42c6e 100644 --- a/codex-rs/app-server/src/config_manager_service.rs +++ b/codex-rs/app-server/src/config_manager_service.rs @@ -326,6 +326,21 @@ impl ConfigManager { format!("Invalid configuration: {err}"), ) })?; + let materialized_config_validation = + codex_core::config::validate_materialized_config_from_layer_stack( + self.codex_home().to_path_buf(), + updated_layers.clone(), + codex_core::config::ConfigOverrides::default(), + ) + .await; + if let Err(err) = materialized_config_validation + && crate::is_unrecoverable_config_error(&err) + { + return Err(ConfigManagerError::write( + ConfigWriteErrorCode::ConfigValidationError, + err.to_string(), + )); + } if !config_edits.is_empty() { ConfigEditsBuilder::for_config_path(provided_path.as_path()) diff --git a/codex-rs/app-server/src/config_manager_service_tests.rs b/codex-rs/app-server/src/config_manager_service_tests.rs index 1ec79a749b3c..287b3a4025b4 100644 --- a/codex-rs/app-server/src/config_manager_service_tests.rs +++ b/codex-rs/app-server/src/config_manager_service_tests.rs @@ -743,6 +743,88 @@ personality = true ); } +#[tokio::test] +async fn write_value_ignores_unrelated_materialized_config_errors() -> Result<()> { + let tmp = tempdir().expect("tempdir"); + let config_path = tmp.path().join(CONFIG_TOML_FILE); + std::fs::write(&config_path, "model_provider = \"missing\"\n")?; + + let service = ConfigManager::without_managed_config_for_tests(tmp.path().to_path_buf()); + service + .write_value(ConfigValueWriteParams { + file_path: Some(config_path.display().to_string()), + key_path: "features.personality".to_string(), + value: serde_json::json!(true), + merge_strategy: MergeStrategy::Replace, + expected_version: None, + }) + .await?; + + let actual: TomlValue = toml::from_str(&std::fs::read_to_string(&config_path)?)?; + let expected: TomlValue = toml::from_str( + r#" +model_provider = "missing" + +[features] +personality = true +"#, + )?; + assert_eq!(actual, expected); + + Ok(()) +} + +#[cfg(target_os = "windows")] +#[tokio::test] +async fn batch_write_rejects_unelevated_windows_sandbox_with_network_proxy() -> Result<()> { + let tmp = tempdir().expect("tempdir"); + let config_path = tmp.path().join(CONFIG_TOML_FILE); + let original_config = r#" +default_permissions = "networked" + +[features] +network_proxy = true + +[windows] +sandbox = "elevated" + +[permissions.networked.filesystem] +":root" = "read" + +[permissions.networked.network] +enabled = true +"#; + std::fs::write(&config_path, original_config)?; + + let service = ConfigManager::without_managed_config_for_tests(tmp.path().to_path_buf()); + let error = service + .batch_write(ConfigBatchWriteParams { + edits: vec![codex_app_server_protocol::ConfigEdit { + key_path: "windows.sandbox".to_string(), + value: serde_json::json!("unelevated"), + merge_strategy: MergeStrategy::Replace, + }], + file_path: Some(config_path.display().to_string()), + expected_version: None, + reload_user_config: false, + }) + .await + .expect_err("unelevated Windows sandbox should be rejected with network proxy"); + + assert_eq!( + error.write_error_code(), + Some(ConfigWriteErrorCode::ConfigValidationError) + ); + assert!( + error + .to_string() + .contains("The network proxy requires the elevated Windows sandbox backend"), + "{error}" + ); + assert_eq!(std::fs::read_to_string(&config_path)?, original_config); + Ok(()) +} + #[tokio::test] async fn read_reports_managed_overrides_user_and_session_flags() { let tmp = tempdir().expect("tempdir"); diff --git a/codex-rs/app-server/src/lib.rs b/codex-rs/app-server/src/lib.rs index 0edb57d8a214..0066ab7547ea 100644 --- a/codex-rs/app-server/src/lib.rs +++ b/codex-rs/app-server/src/lib.rs @@ -298,6 +298,10 @@ fn config_error_location(err: &std::io::Error) -> Option<(String, AppTextRange)> }) } +pub(crate) fn is_unrecoverable_config_error(err: &std::io::Error) -> bool { + codex_core::config::is_windows_sandbox_network_proxy_incompatible_error(err) +} + fn exec_policy_warning_location(err: &ExecPolicyError) -> (Option, Option) { match err { ExecPolicyError::ParsePolicy { path, source } => { 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..d23db6dbf922 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.as_ref(), + ) + .map_err(|err| invalid_request(err.to_string()))?; self.outgoing .send_response( diff --git a/codex-rs/app-server/tests/suite/strict_config.rs b/codex-rs/app-server/tests/suite/strict_config.rs index 93784c975272..3bcc2535b9fa 100644 --- a/codex-rs/app-server/tests/suite/strict_config.rs +++ b/codex-rs/app-server/tests/suite/strict_config.rs @@ -31,3 +31,53 @@ foo = "bar" Ok(()) } + +#[cfg(target_os = "windows")] +#[test] +fn non_strict_config_falls_back_for_unelevated_windows_sandbox_with_network_proxy() -> Result<()> { + let codex_home = TempDir::new()?; + std::fs::write( + codex_home.path().join("config.toml"), + r#" +default_permissions = "networked" + +[features] +network_proxy = true + +[windows] +sandbox = "unelevated" + +[permissions.networked.filesystem] +":minimal" = "read" + +[permissions.networked.network] +enabled = true +"#, + )?; + + let output = Command::new(codex_utils_cargo_bin::cargo_bin("codex-app-server")?) + .env("CODEX_HOME", codex_home.path()) + .env( + "CODEX_APP_SERVER_MANAGED_CONFIG_PATH", + codex_home.path().join("managed_config.toml"), + ) + .args(["--listen", "off"]) + .output()?; + + assert!(!output.status.success()); + let stderr = String::from_utf8(output.stderr)?; + assert!( + stderr.contains("Invalid configuration; using defaults."), + "expected invalid config warning in stderr, got: {stderr}" + ); + assert!( + stderr.contains("The network proxy requires the elevated Windows sandbox backend"), + "expected Windows sandbox compatibility warning in stderr, got: {stderr}" + ); + assert!( + stderr.contains("no transport configured; use --listen or enable remote control"), + "expected app server to reach transport validation after falling back, got: {stderr}" + ); + + Ok(()) +} diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index c34447554196..f678f8962dd2 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -1685,6 +1685,191 @@ enabled = true Ok(()) } +#[test] +fn windows_sandbox_network_proxy_compatibility_rejects_network_proxy_with_unelevated_windows() { + 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_eq!(err.kind(), std::io::ErrorKind::InvalidInput); + assert_eq!( + err.to_string(), + WINDOWS_SANDBOX_NETWORK_PROXY_INCOMPATIBLE_ERROR + ); +} + +#[test] +fn windows_sandbox_network_proxy_compatibility_allows_compatible_combinations() { + for (windows_sandbox_level, network_proxy_configured) in [ + (WindowsSandboxLevel::Disabled, true), + (WindowsSandboxLevel::Elevated, true), + (WindowsSandboxLevel::RestrictedToken, false), + ] { + validate_windows_sandbox_network_proxy_compatibility_for_platform( + /*is_windows*/ true, + windows_sandbox_level, + network_proxy_configured, + ) + .expect("compatible Windows sandbox and network proxy settings"); + } + validate_windows_sandbox_network_proxy_compatibility_for_platform( + /*is_windows*/ false, + WindowsSandboxLevel::RestrictedToken, + /*network_proxy_configured*/ true, + ) + .expect("Windows sandbox compatibility should only be enforced on Windows"); +} + +#[cfg(target_os = "windows")] +#[tokio::test] +async fn disabled_experimental_network_requirements_reject_unelevated_windows_sandbox() +-> std::io::Result<()> { + let codex_home = TempDir::new()?; + std::fs::write( + codex_home.path().join(CONFIG_TOML_FILE), + r#" +[windows] +sandbox = "unelevated" +"#, + )?; + + let err = ConfigBuilder::without_managed_config_for_tests() + .codex_home(codex_home.path().to_path_buf()) + .fallback_cwd(Some(codex_home.path().to_path_buf())) + .cloud_config_bundle( + CloudConfigBundleFixture::loader_with_enterprise_requirement( + r#" +[experimental_network] +enabled = false +"#, + ), + ) + .build() + .await + .expect_err("disabled experimental_network requirements still configure network proxy"); + + assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput); + assert_eq!( + err.to_string(), + WINDOWS_SANDBOX_NETWORK_PROXY_INCOMPATIBLE_ERROR + ); + Ok(()) +} + +#[cfg(target_os = "windows")] +#[tokio::test] +async fn network_proxy_feature_rejects_unelevated_windows_sandbox() -> 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 { + features: Some(toml::from_str("network_proxy = true").expect("valid features")), + default_permissions: Some("dev".to_string()), + permissions: Some(PermissionsToml { + entries: BTreeMap::from([( + "dev".to_string(), + PermissionProfileToml { + description: None, + extends: None, + workspace_roots: None, + filesystem: Some(FilesystemPermissionsToml { + glob_scan_max_depth: None, + entries: BTreeMap::from([( + ":minimal".to_string(), + FilesystemPermissionToml::Access(FileSystemAccessMode::Read), + )]), + }), + network: Some(NetworkToml { + enabled: Some(true), + ..Default::default() + }), + }, + )]), + }), + 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("network_proxy feature should reject unelevated Windows sandbox"); + + assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput); + assert_eq!( + err.to_string(), + WINDOWS_SANDBOX_NETWORK_PROXY_INCOMPATIBLE_ERROR + ); + Ok(()) +} + +#[cfg(target_os = "windows")] +#[tokio::test] +async fn active_profile_network_proxy_rejects_unelevated_windows_sandbox() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cwd = TempDir::new()?; + std::fs::write(cwd.path().join(".git"), "gitdir: nowhere")?; + + std::fs::write( + codex_home.path().join(CONFIG_TOML_FILE), + r#" +default_permissions = ":read-only" + +[features] +network_proxy = true + +[windows] +sandbox = "unelevated" + +[permissions.dev.filesystem] +":minimal" = "read" + +[permissions.dev.network] +enabled = true +"#, + )?; + let config = ConfigBuilder::without_managed_config_for_tests() + .codex_home(codex_home.path().to_path_buf()) + .harness_overrides(ConfigOverrides { + cwd: Some(cwd.path().to_path_buf()), + ..Default::default() + }) + .build() + .await?; + + assert_eq!(config.permissions.network, None); + let err = config + .network_proxy_spec_for_active_permission_profile( + &ActivePermissionProfile::new("dev"), + &PermissionProfile::Managed { + file_system: ManagedFileSystemPermissions::Restricted { + entries: Vec::new(), + glob_scan_max_depth: None, + }, + network: NetworkSandboxPolicy::Enabled, + }, + ) + .expect_err("inactive network proxy profile should be rejected with unelevated sandbox"); + + assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput); + assert_eq!( + err.to_string(), + WINDOWS_SANDBOX_NETWORK_PROXY_INCOMPATIBLE_ERROR + ); + Ok(()) +} + #[tokio::test] async fn network_proxy_feature_uses_profile_network_proxy_settings() -> std::io::Result<()> { let codex_home = TempDir::new()?; diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index d90d010102d5..e55b997d5410 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -594,6 +594,41 @@ fn build_network_proxy_spec( }) } +const WINDOWS_SANDBOX_NETWORK_PROXY_INCOMPATIBLE_ERROR: &str = "The network proxy requires the elevated Windows sandbox backend, but Codex is using the unelevated backend. Set windows.sandbox = \"elevated\" or disable the network proxy. If these settings are managed by your organization, contact your administrator."; + +pub fn validate_windows_sandbox_network_proxy_compatibility( + windows_sandbox_level: WindowsSandboxLevel, + network_proxy: Option<&NetworkProxySpec>, +) -> std::io::Result<()> { + validate_windows_sandbox_network_proxy_compatibility_for_platform( + cfg!(target_os = "windows"), + windows_sandbox_level, + network_proxy.is_some(), + ) +} + +pub fn is_windows_sandbox_network_proxy_incompatible_error(err: &std::io::Error) -> bool { + err.kind() == std::io::ErrorKind::InvalidInput + && err.to_string() == WINDOWS_SANDBOX_NETWORK_PROXY_INCOMPATIBLE_ERROR +} + +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(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + WINDOWS_SANDBOX_NETWORK_PROXY_INCOMPATIBLE_ERROR, + )); + } + Ok(()) +} + /// Configured thread persistence backend. #[derive(Debug, Clone, PartialEq, Eq, Default)] pub enum ThreadStoreConfig { @@ -1398,6 +1433,34 @@ impl ConfigBuilder { } } +/// Validate an in-memory layer stack after callers have edited it but before +/// they persist the edited layer. +/// +/// `ConfigLayerStack::effective_config()` materializes the merged TOML. The +/// final `Config` also applies feature and requirement constraints, permission +/// profile resolution, and network proxy construction. The normal builders load +/// layers from disk, so config writes use this to validate the pending stack. +pub async fn validate_materialized_config_from_layer_stack( + codex_home: PathBuf, + config_layer_stack: ConfigLayerStack, + overrides: ConfigOverrides, +) -> std::io::Result<()> { + let cfg: ConfigToml = config_layer_stack + .effective_config() + .try_into() + .map_err(|err| std::io::Error::new(std::io::ErrorKind::InvalidData, err))?; + let codex_home = AbsolutePathBuf::from_absolute_path_checked(codex_home)?; + Config::load_config_with_layer_stack( + LOCAL_FS.as_ref(), + cfg, + overrides, + codex_home, + config_layer_stack, + ) + .await + .map(|_| ()) +} + impl Config { pub(crate) fn multi_agent_version_from_features(&self) -> MultiAgentVersion { if self.features.enabled(Feature::MultiAgentV2) { @@ -3988,6 +4051,10 @@ impl Config { .unwrap_or_default(), otel, }; + validate_windows_sandbox_network_proxy_compatibility( + WindowsSandboxLevel::from_config(&config), + config.permissions.network.as_ref(), + )?; Ok(config) }) .await @@ -4104,11 +4171,16 @@ impl Config { NetworkProxyConfig::default() }; - build_network_proxy_spec( + let network_proxy = 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_proxy.as_ref(), + )?; + Ok(network_proxy) } pub fn bundled_skills_enabled(&self) -> bool { diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 015b20715af4..73601949202f 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -411,10 +411,8 @@ pub fn build_exec_request( ) }) .map_err(CodexErr::from)?; - let use_windows_elevated_backend = windows_sandbox_uses_elevated_backend( - exec_req.windows_sandbox_level, - exec_req.network.is_some(), - ); + 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 +656,7 @@ 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); + 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..d4cfce7d376f 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_sandbox_level_selects_elevated_backend() { assert!(!windows_sandbox_uses_elevated_backend( - WindowsSandboxLevel::RestrictedToken, - /*proxy_enforced*/ false, + WindowsSandboxLevel::RestrictedToken )); assert!(windows_sandbox_uses_elevated_backend( - WindowsSandboxLevel::RestrictedToken, - /*proxy_enforced*/ true, - )); - assert!(windows_sandbox_uses_elevated_backend( - WindowsSandboxLevel::Elevated, - /*proxy_enforced*/ false, + WindowsSandboxLevel::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..01f1f6fc3ddf 100644 --- a/codex-rs/sandboxing/src/windows.rs +++ b/codex-rs/sandboxing/src/windows.rs @@ -29,14 +29,10 @@ 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 { + // Compatibility validation rejects proxy-enforced sessions in unelevated mode. + // Backend selection must still honor the explicitly configured sandbox level. + matches!(sandbox_level, WindowsSandboxLevel::Elevated) } pub fn permission_profile_supports_windows_restricted_token_sandbox(