From c124f243545b2abf3442fe4326f38cb6a6906c50 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 25 Oct 2025 18:52:26 -0500 Subject: [PATCH] Added support for `sandbox_mode` in profiles (#5686) Currently, `approval_policy` is supported in profiles, but `sandbox_mode` is not. This PR adds support for `sandbox_mode`. Note: a fix for this was submitted in [this PR](https://github.com/openai/codex/pull/2397), but the underlying code has changed significantly since then. This addresses issue #3034 --- codex-rs/core/src/config.rs | 104 +++++++++++++++++++++++++--- codex-rs/core/src/config_profile.rs | 2 + 2 files changed, 96 insertions(+), 10 deletions(-) diff --git a/codex-rs/core/src/config.rs b/codex-rs/core/src/config.rs index b04aba5d..c5406b1d 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -1027,9 +1027,11 @@ impl ConfigToml { fn derive_sandbox_policy( &self, sandbox_mode_override: Option, + profile_sandbox_mode: Option, resolved_cwd: &Path, ) -> SandboxPolicy { let resolved_sandbox_mode = sandbox_mode_override + .or(profile_sandbox_mode) .or(self.sandbox_mode) .or_else(|| { // if no sandbox_mode is set, but user has marked directory as trusted, use WorkspaceWrite @@ -1219,7 +1221,8 @@ impl Config { .get_active_project(&resolved_cwd) .unwrap_or(ProjectConfig { trust_level: None }); - let mut sandbox_policy = cfg.derive_sandbox_policy(sandbox_mode, &resolved_cwd); + let mut sandbox_policy = + cfg.derive_sandbox_policy(sandbox_mode, config_profile.sandbox_mode, &resolved_cwd); if let SandboxPolicy::WorkspaceWrite { writable_roots, .. } = &mut sandbox_policy { for path in additional_writable_roots { if !writable_roots.iter().any(|existing| existing == &path) { @@ -1242,8 +1245,8 @@ impl Config { .is_some() || config_profile.approval_policy.is_some() || cfg.approval_policy.is_some() - // TODO(#3034): profile.sandbox_mode is not implemented || sandbox_mode.is_some() + || config_profile.sandbox_mode.is_some() || cfg.sandbox_mode.is_some(); let mut model_providers = built_in_model_providers(); @@ -1603,8 +1606,11 @@ network_access = false # This should be ignored. let sandbox_mode_override = None; assert_eq!( SandboxPolicy::DangerFullAccess, - sandbox_full_access_cfg - .derive_sandbox_policy(sandbox_mode_override, &PathBuf::from("/tmp/test")) + sandbox_full_access_cfg.derive_sandbox_policy( + sandbox_mode_override, + None, + &PathBuf::from("/tmp/test") + ) ); let sandbox_read_only = r#" @@ -1619,8 +1625,11 @@ network_access = true # This should be ignored. let sandbox_mode_override = None; assert_eq!( SandboxPolicy::ReadOnly, - sandbox_read_only_cfg - .derive_sandbox_policy(sandbox_mode_override, &PathBuf::from("/tmp/test")) + sandbox_read_only_cfg.derive_sandbox_policy( + sandbox_mode_override, + None, + &PathBuf::from("/tmp/test") + ) ); let sandbox_workspace_write = r#" @@ -1644,8 +1653,11 @@ exclude_slash_tmp = true exclude_tmpdir_env_var: true, exclude_slash_tmp: true, }, - sandbox_workspace_write_cfg - .derive_sandbox_policy(sandbox_mode_override, &PathBuf::from("/tmp/test")) + sandbox_workspace_write_cfg.derive_sandbox_policy( + sandbox_mode_override, + None, + &PathBuf::from("/tmp/test") + ) ); let sandbox_workspace_write = r#" @@ -1672,8 +1684,11 @@ trust_level = "trusted" exclude_tmpdir_env_var: true, exclude_slash_tmp: true, }, - sandbox_workspace_write_cfg - .derive_sandbox_policy(sandbox_mode_override, &PathBuf::from("/tmp/test")) + sandbox_workspace_write_cfg.derive_sandbox_policy( + sandbox_mode_override, + None, + &PathBuf::from("/tmp/test") + ) ); } @@ -1765,6 +1780,75 @@ trust_level = "trusted" Ok(()) } + #[test] + fn profile_sandbox_mode_overrides_base() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let mut profiles = HashMap::new(); + profiles.insert( + "work".to_string(), + ConfigProfile { + sandbox_mode: Some(SandboxMode::DangerFullAccess), + ..Default::default() + }, + ); + let cfg = ConfigToml { + profiles, + profile: Some("work".to_string()), + sandbox_mode: Some(SandboxMode::ReadOnly), + ..Default::default() + }; + + let config = Config::load_from_base_config_with_overrides( + cfg, + ConfigOverrides::default(), + codex_home.path().to_path_buf(), + )?; + + assert!(matches!( + config.sandbox_policy, + SandboxPolicy::DangerFullAccess + )); + assert!(config.did_user_set_custom_approval_policy_or_sandbox_mode); + + Ok(()) + } + + #[test] + fn cli_override_takes_precedence_over_profile_sandbox_mode() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let mut profiles = HashMap::new(); + profiles.insert( + "work".to_string(), + ConfigProfile { + sandbox_mode: Some(SandboxMode::DangerFullAccess), + ..Default::default() + }, + ); + let cfg = ConfigToml { + profiles, + profile: Some("work".to_string()), + ..Default::default() + }; + + let overrides = ConfigOverrides { + sandbox_mode: Some(SandboxMode::WorkspaceWrite), + ..Default::default() + }; + + let config = Config::load_from_base_config_with_overrides( + cfg, + overrides, + codex_home.path().to_path_buf(), + )?; + + assert!(matches!( + config.sandbox_policy, + SandboxPolicy::WorkspaceWrite { .. } + )); + + Ok(()) + } + #[test] fn feature_table_overrides_legacy_flags() -> std::io::Result<()> { let codex_home = TempDir::new()?; diff --git a/codex-rs/core/src/config_profile.rs b/codex-rs/core/src/config_profile.rs index 84b90f5a..7b7cafed 100644 --- a/codex-rs/core/src/config_profile.rs +++ b/codex-rs/core/src/config_profile.rs @@ -4,6 +4,7 @@ use std::path::PathBuf; use crate::protocol::AskForApproval; use codex_protocol::config_types::ReasoningEffort; use codex_protocol::config_types::ReasoningSummary; +use codex_protocol::config_types::SandboxMode; use codex_protocol::config_types::Verbosity; /// Collection of common configuration options that a user can define as a unit @@ -15,6 +16,7 @@ pub struct ConfigProfile { /// [`ModelProviderInfo`] to use. pub model_provider: Option, pub approval_policy: Option, + pub sandbox_mode: Option, pub model_reasoning_effort: Option, pub model_reasoning_summary: Option, pub model_verbosity: Option,