From 8406bd7672f0e6b45d060f59e8094785c8a8e4d6 Mon Sep 17 00:00:00 2001 From: gt-oai Date: Tue, 3 Feb 2026 20:47:34 +0000 Subject: [PATCH] [codex] Default values from requirements if unset (#10531) If we don't set any explicit values for sandbox or approval policy, let's try to use a requirements-satisfying value. --- codex-rs/core/src/config/mod.rs | 281 ++++++++++++++++++++++++++++++-- 1 file changed, 267 insertions(+), 14 deletions(-) diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 3e2ed804a..949c08724 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -1094,7 +1094,11 @@ impl ConfigToml { profile_sandbox_mode: Option, windows_sandbox_level: WindowsSandboxLevel, resolved_cwd: &Path, + sandbox_policy_constraint: Option<&Constrained>, ) -> SandboxPolicyResolution { + let sandbox_mode_was_explicit = sandbox_mode_override.is_some() + || profile_sandbox_mode.is_some() + || self.sandbox_mode.is_some(); let resolved_sandbox_mode = sandbox_mode_override .or(profile_sandbox_mode) .or(self.sandbox_mode) @@ -1128,13 +1132,30 @@ impl ConfigToml { SandboxMode::DangerFullAccess => SandboxPolicy::DangerFullAccess, }; let mut forced_auto_mode_downgraded_on_windows = false; - if cfg!(target_os = "windows") - && matches!(resolved_sandbox_mode, SandboxMode::WorkspaceWrite) - // If the experimental Windows sandbox is enabled, do not force a downgrade. - && windows_sandbox_level == codex_protocol::config_types::WindowsSandboxLevel::Disabled + let mut downgrade_workspace_write_if_unsupported = |policy: &mut SandboxPolicy| { + if cfg!(target_os = "windows") + // If the experimental Windows sandbox is enabled, do not force a downgrade. + && windows_sandbox_level + == codex_protocol::config_types::WindowsSandboxLevel::Disabled + && matches!(&*policy, SandboxPolicy::WorkspaceWrite { .. }) + { + *policy = SandboxPolicy::new_read_only_policy(); + forced_auto_mode_downgraded_on_windows = true; + } + }; + if matches!(resolved_sandbox_mode, SandboxMode::WorkspaceWrite) { + downgrade_workspace_write_if_unsupported(&mut sandbox_policy); + } + if !sandbox_mode_was_explicit + && let Some(constraint) = sandbox_policy_constraint + && let Err(err) = constraint.can_set(&sandbox_policy) { - sandbox_policy = SandboxPolicy::new_read_only_policy(); - forced_auto_mode_downgraded_on_windows = true; + tracing::warn!( + error = %err, + "default sandbox policy is disallowed by requirements; falling back to required default" + ); + sandbox_policy = constraint.get().clone(); + downgrade_workspace_write_if_unsupported(&mut sandbox_policy); } SandboxPolicyResolution { policy: sandbox_policy, @@ -1364,6 +1385,9 @@ impl Config { let active_project = cfg .get_active_project(&resolved_cwd) .unwrap_or(ProjectConfig { trust_level: None }); + let sandbox_mode_was_explicit = sandbox_mode.is_some() + || config_profile.sandbox_mode.is_some() + || cfg.sandbox_mode.is_some(); let windows_sandbox_level = WindowsSandboxLevel::from_features(&features); let SandboxPolicyResolution { @@ -1374,6 +1398,7 @@ impl Config { config_profile.sandbox_mode, windows_sandbox_level, &resolved_cwd, + Some(&requirements.sandbox_policy), ); if let SandboxPolicy::WorkspaceWrite { writable_roots, .. } = &mut sandbox_policy { for path in additional_writable_roots { @@ -1382,7 +1407,10 @@ impl Config { } } } - let approval_policy = approval_policy_override + let approval_policy_was_explicit = approval_policy_override.is_some() + || config_profile.approval_policy.is_some() + || cfg.approval_policy.is_some(); + let mut approval_policy = approval_policy_override .or(config_profile.approval_policy) .or(cfg.approval_policy) .unwrap_or_else(|| { @@ -1394,16 +1422,20 @@ impl Config { AskForApproval::default() } }); + if !approval_policy_was_explicit + && let Err(err) = requirements.approval_policy.can_set(&approval_policy) + { + tracing::warn!( + error = %err, + "default approval policy is disallowed by requirements; falling back to required default" + ); + approval_policy = requirements.approval_policy.value(); + } let web_search_mode = resolve_web_search_mode(&cfg, &config_profile, &features); // TODO(dylan): We should be able to leverage ConfigLayerStack so that // we can reliably check this at every config level. - let did_user_set_custom_approval_policy_or_sandbox_mode = approval_policy_override - .is_some() - || config_profile.approval_policy.is_some() - || cfg.approval_policy.is_some() - || sandbox_mode.is_some() - || config_profile.sandbox_mode.is_some() - || cfg.sandbox_mode.is_some(); + let did_user_set_custom_approval_policy_or_sandbox_mode = + approval_policy_was_explicit || sandbox_mode_was_explicit; let mut model_providers = built_in_model_providers(); // Merge user-defined providers into the built-in list. @@ -1919,6 +1951,7 @@ network_access = false # This should be ignored. None, WindowsSandboxLevel::Disabled, &PathBuf::from("/tmp/test"), + None, ); assert_eq!( resolution, @@ -1943,6 +1976,7 @@ network_access = true # This should be ignored. None, WindowsSandboxLevel::Disabled, &PathBuf::from("/tmp/test"), + None, ); assert_eq!( resolution, @@ -1975,6 +2009,7 @@ exclude_slash_tmp = true None, WindowsSandboxLevel::Disabled, &PathBuf::from("/tmp/test"), + None, ); if cfg!(target_os = "windows") { assert_eq!( @@ -2024,6 +2059,7 @@ trust_level = "trusted" None, WindowsSandboxLevel::Disabled, &PathBuf::from("/tmp/test"), + None, ); if cfg!(target_os = "windows") { assert_eq!( @@ -4331,6 +4367,7 @@ trust_level = "untrusted" None, WindowsSandboxLevel::Disabled, &PathBuf::from("/tmp/test"), + None, ); // Verify that untrusted projects get WorkspaceWrite (or ReadOnly on Windows due to downgrade) @@ -4351,6 +4388,103 @@ trust_level = "untrusted" Ok(()) } + #[test] + fn derive_sandbox_policy_falls_back_to_constraint_value_for_implicit_defaults() + -> anyhow::Result<()> { + let project_dir = TempDir::new()?; + let project_path = project_dir.path().to_path_buf(); + let project_key = project_path.to_string_lossy().to_string(); + let cfg = ConfigToml { + projects: Some(HashMap::from([( + project_key, + ProjectConfig { + trust_level: Some(TrustLevel::Trusted), + }, + )])), + ..Default::default() + }; + let constrained = Constrained::new(SandboxPolicy::DangerFullAccess, |candidate| { + if matches!(candidate, SandboxPolicy::DangerFullAccess) { + Ok(()) + } else { + Err(ConstraintError::InvalidValue { + field_name: "sandbox_mode", + candidate: format!("{candidate:?}"), + allowed: "[DangerFullAccess]".to_string(), + requirement_source: RequirementSource::Unknown, + }) + } + })?; + + let resolution = cfg.derive_sandbox_policy( + None, + None, + WindowsSandboxLevel::Disabled, + &project_path, + Some(&constrained), + ); + + assert_eq!(resolution.policy, SandboxPolicy::DangerFullAccess); + Ok(()) + } + + #[test] + fn derive_sandbox_policy_preserves_windows_downgrade_for_unsupported_fallback() + -> anyhow::Result<()> { + let project_dir = TempDir::new()?; + let project_path = project_dir.path().to_path_buf(); + let project_key = project_path.to_string_lossy().to_string(); + let cfg = ConfigToml { + projects: Some(HashMap::from([( + project_key, + ProjectConfig { + trust_level: Some(TrustLevel::Trusted), + }, + )])), + ..Default::default() + }; + let constrained = + Constrained::new(SandboxPolicy::new_workspace_write_policy(), |candidate| { + if matches!(candidate, SandboxPolicy::WorkspaceWrite { .. }) { + Ok(()) + } else { + Err(ConstraintError::InvalidValue { + field_name: "sandbox_mode", + candidate: format!("{candidate:?}"), + allowed: "[WorkspaceWrite]".to_string(), + requirement_source: RequirementSource::Unknown, + }) + } + })?; + + let resolution = cfg.derive_sandbox_policy( + None, + None, + WindowsSandboxLevel::Disabled, + &project_path, + Some(&constrained), + ); + + if cfg!(target_os = "windows") { + assert_eq!( + resolution, + SandboxPolicyResolution { + policy: SandboxPolicy::ReadOnly, + forced_auto_mode_downgraded_on_windows: true, + } + ); + } else { + assert_eq!( + resolution, + SandboxPolicyResolution { + policy: SandboxPolicy::new_workspace_write_policy(), + forced_auto_mode_downgraded_on_windows: false, + } + ); + } + Ok(()) + } + #[test] fn test_resolve_oss_provider_explicit_override() { let config_toml = ConfigToml::default(); @@ -4506,6 +4640,125 @@ mcp_oauth_callback_port = 5678 Ok(()) } + + #[tokio::test] + async fn requirements_disallowing_default_sandbox_falls_back_to_required_default() + -> std::io::Result<()> { + let codex_home = TempDir::new()?; + + let config = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .cloud_requirements(CloudRequirementsLoader::new(async { + Some(crate::config_loader::ConfigRequirementsToml { + allowed_sandbox_modes: Some(vec![ + crate::config_loader::SandboxModeRequirement::ReadOnly, + ]), + ..Default::default() + }) + })) + .build() + .await?; + + assert_eq!(*config.sandbox_policy.get(), SandboxPolicy::ReadOnly); + Ok(()) + } + + #[tokio::test] + async fn explicit_sandbox_mode_still_errors_when_disallowed_by_requirements() + -> std::io::Result<()> { + let codex_home = TempDir::new()?; + std::fs::write( + codex_home.path().join(CONFIG_TOML_FILE), + r#"sandbox_mode = "danger-full-access" +"#, + )?; + + let requirements = crate::config_loader::ConfigRequirementsToml { + allowed_approval_policies: None, + allowed_sandbox_modes: Some(vec![ + crate::config_loader::SandboxModeRequirement::ReadOnly, + ]), + mcp_servers: None, + rules: None, + enforce_residency: None, + }; + + let err = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .fallback_cwd(Some(codex_home.path().to_path_buf())) + .cloud_requirements(CloudRequirementsLoader::new( + async move { Some(requirements) }, + )) + .build() + .await + .expect_err("explicit disallowed mode should still fail"); + assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput); + let message = err.to_string(); + assert!(message.contains("invalid value for `sandbox_mode`")); + assert!(message.contains("set by cloud requirements")); + Ok(()) + } + + #[tokio::test] + async fn requirements_disallowing_default_approval_falls_back_to_required_default() + -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let workspace = TempDir::new()?; + let workspace_key = workspace.path().to_string_lossy().replace('\\', "\\\\"); + std::fs::write( + codex_home.path().join(CONFIG_TOML_FILE), + format!( + r#" +[projects."{workspace_key}"] +trust_level = "untrusted" +"# + ), + )?; + + let config = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .fallback_cwd(Some(workspace.path().to_path_buf())) + .cloud_requirements(CloudRequirementsLoader::new(async { + Some(crate::config_loader::ConfigRequirementsToml { + allowed_approval_policies: Some(vec![AskForApproval::OnRequest]), + ..Default::default() + }) + })) + .build() + .await?; + + assert_eq!(config.approval_policy.value(), AskForApproval::OnRequest); + Ok(()) + } + + #[tokio::test] + async fn explicit_approval_policy_still_errors_when_disallowed_by_requirements() + -> std::io::Result<()> { + let codex_home = TempDir::new()?; + std::fs::write( + codex_home.path().join(CONFIG_TOML_FILE), + r#"approval_policy = "untrusted" +"#, + )?; + + let err = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .fallback_cwd(Some(codex_home.path().to_path_buf())) + .cloud_requirements(CloudRequirementsLoader::new(async { + Some(crate::config_loader::ConfigRequirementsToml { + allowed_approval_policies: Some(vec![AskForApproval::OnRequest]), + ..Default::default() + }) + })) + .build() + .await + .expect_err("explicit disallowed approval policy should fail"); + assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput); + let message = err.to_string(); + assert!(message.contains("invalid value for `approval_policy`")); + assert!(message.contains("set by cloud requirements")); + Ok(()) + } } #[cfg(test)]