From 9d174acad84466c65e727f1cfdc80479a20266aa Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Tue, 28 Apr 2026 18:39:34 -0700 Subject: [PATCH 1/2] feat(cli): add sandbox profile config controls Co-authored-by: Codex noreply@openai.com --- codex-rs/cli/src/debug_sandbox.rs | 313 +++++++++--------- codex-rs/cli/src/lib.rs | 52 +++ codex-rs/cli/src/main.rs | 35 ++ codex-rs/config/src/loader/mod.rs | 51 +-- codex-rs/config/src/state.rs | 2 + .../core/src/config/config_loader_tests.rs | 46 +++ 6 files changed, 327 insertions(+), 172 deletions(-) diff --git a/codex-rs/cli/src/debug_sandbox.rs b/codex-rs/cli/src/debug_sandbox.rs index 852af4d21683..e334faa49328 100644 --- a/codex-rs/cli/src/debug_sandbox.rs +++ b/codex-rs/cli/src/debug_sandbox.rs @@ -6,6 +6,7 @@ mod seatbelt; use std::path::PathBuf; use std::process::Stdio; +use codex_config::LoaderOverrides; use codex_core::config::Config; use codex_core::config::ConfigBuilder; use codex_core::config::ConfigOverrides; @@ -42,20 +43,29 @@ pub async fn run_command_under_seatbelt( codex_linux_sandbox_exe: Option, ) -> anyhow::Result<()> { let SeatbeltCommand { + full_auto, permissions_profile, + cwd, + include_managed_config, allow_unix_sockets, log_denials, config_overrides, command, } = command; run_command_under_sandbox( - permissions_profile, + DebugSandboxConfigOptions { + full_auto, + permissions_profile, + cwd, + include_managed_config, + }, command, config_overrides, codex_linux_sandbox_exe, - SandboxType::Seatbelt, - log_denials, - &allow_unix_sockets, + SandboxType::Seatbelt { + allow_unix_sockets, + log_denials, + }, ) .await } @@ -73,18 +83,24 @@ pub async fn run_command_under_landlock( codex_linux_sandbox_exe: Option, ) -> anyhow::Result<()> { let LandlockCommand { + full_auto, permissions_profile, + cwd, + include_managed_config, config_overrides, command, } = command; run_command_under_sandbox( - permissions_profile, + DebugSandboxConfigOptions { + full_auto, + permissions_profile, + cwd, + include_managed_config, + }, command, config_overrides, codex_linux_sandbox_exe, SandboxType::Landlock, - /*log_denials*/ false, - &[], ) .await } @@ -94,45 +110,65 @@ pub async fn run_command_under_windows( codex_linux_sandbox_exe: Option, ) -> anyhow::Result<()> { let WindowsCommand { + full_auto, permissions_profile, + cwd, + include_managed_config, config_overrides, command, } = command; run_command_under_sandbox( - permissions_profile, + DebugSandboxConfigOptions { + full_auto, + permissions_profile, + cwd, + include_managed_config, + }, command, config_overrides, codex_linux_sandbox_exe, SandboxType::Windows, - /*log_denials*/ false, - &[], ) .await } enum SandboxType { #[cfg(target_os = "macos")] - Seatbelt, + Seatbelt { + allow_unix_sockets: Vec, + log_denials: bool, + }, Landlock, Windows, } -async fn run_command_under_sandbox( +#[derive(Debug)] +struct DebugSandboxConfigOptions { + full_auto: bool, permissions_profile: Option, + cwd: Option, + include_managed_config: bool, +} + +#[derive(Debug, Clone, Copy)] +enum ManagedRequirementsMode { + Include, + Ignore, +} + +async fn run_command_under_sandbox( + config_options: DebugSandboxConfigOptions, command: Vec, config_overrides: CliConfigOverrides, codex_linux_sandbox_exe: Option, sandbox_type: SandboxType, - log_denials: bool, - #[cfg_attr(not(target_os = "macos"), allow(unused_variables))] - allow_unix_sockets: &[AbsolutePathBuf], ) -> anyhow::Result<()> { let config = load_debug_sandbox_config( config_overrides .parse_overrides() .map_err(anyhow::Error::msg)?, codex_linux_sandbox_exe, - permissions_profile, + config_options, ) .await?; @@ -162,9 +198,10 @@ async fn run_command_under_sandbox( } #[cfg(target_os = "macos")] - let mut denial_logger = log_denials.then(DenialLogger::new).flatten(); - #[cfg(not(target_os = "macos"))] - let _ = log_denials; + let mut denial_logger = match &sandbox_type { + SandboxType::Seatbelt { log_denials, .. } => log_denials.then(DenialLogger::new).flatten(), + SandboxType::Landlock | SandboxType::Windows => None, + }; let managed_network_requirements_enabled = config.managed_network_requirements_enabled(); @@ -189,7 +226,9 @@ async fn run_command_under_sandbox( let mut child = match sandbox_type { #[cfg(target_os = "macos")] - SandboxType::Seatbelt => { + SandboxType::Seatbelt { + allow_unix_sockets, .. + } => { let file_system_sandbox_policy = config.permissions.file_system_sandbox_policy(); let network_sandbox_policy = config.permissions.network_sandbox_policy(); let args = create_seatbelt_command_args(CreateSeatbeltCommandArgsParams { @@ -199,7 +238,7 @@ async fn run_command_under_sandbox( sandbox_policy_cwd: sandbox_policy_cwd.as_path(), enforce_managed_network: false, network: network.as_ref(), - extra_allow_unix_sockets: allow_unix_sockets, + extra_allow_unix_sockets: &allow_unix_sockets, }); spawn_debug_sandbox_child( PathBuf::from("/usr/bin/sandbox-exec"), @@ -402,6 +441,14 @@ async fn run_command_under_windows_session( std::process::exit(exit_code); } +pub fn create_sandbox_mode(full_auto: bool) -> SandboxMode { + if full_auto { + SandboxMode::WorkspaceWrite + } else { + SandboxMode::ReadOnly + } +} + async fn spawn_debug_sandbox_child( program: PathBuf, args: Vec, @@ -571,12 +618,12 @@ mod windows_stdio_bridge { async fn load_debug_sandbox_config( cli_overrides: Vec<(String, TomlValue)>, codex_linux_sandbox_exe: Option, - permissions_profile: Option, + options: DebugSandboxConfigOptions, ) -> anyhow::Result { load_debug_sandbox_config_with_codex_home( cli_overrides, codex_linux_sandbox_exe, - permissions_profile, + options, /*codex_home*/ None, ) .await @@ -585,9 +632,22 @@ async fn load_debug_sandbox_config( async fn load_debug_sandbox_config_with_codex_home( mut cli_overrides: Vec<(String, TomlValue)>, codex_linux_sandbox_exe: Option, - permissions_profile: Option, + options: DebugSandboxConfigOptions, codex_home: Option, ) -> anyhow::Result { + let DebugSandboxConfigOptions { + full_auto, + permissions_profile, + cwd, + include_managed_config, + } = options; + + let managed_requirements_mode = if permissions_profile.is_some() && !include_managed_config { + ManagedRequirementsMode::Ignore + } else { + ManagedRequirementsMode::Include + }; + if let Some(permissions_profile) = permissions_profile { cli_overrides.push(( "default_permissions".to_string(), @@ -595,34 +655,37 @@ async fn load_debug_sandbox_config_with_codex_home( )); } - // For legacy configs, `codex sandbox` historically defaulted to read-only - // instead of inheriting ambient `sandbox_mode` settings from user/system - // config. Keep that behavior unless this invocation explicitly passes a - // legacy `sandbox_mode` CLI override, which is now the documented writable - // replacement for the removed `--full-auto` flag. - let uses_legacy_sandbox_mode_override = cli_overrides_use_legacy_sandbox_mode(&cli_overrides); let config = build_debug_sandbox_config( cli_overrides.clone(), ConfigOverrides { + cwd: cwd.clone(), codex_linux_sandbox_exe: codex_linux_sandbox_exe.clone(), ..Default::default() }, codex_home.clone(), + managed_requirements_mode, ) .await?; - if config_uses_permission_profiles(&config) || uses_legacy_sandbox_mode_override { + if config_uses_permission_profiles(&config) { + if full_auto { + anyhow::bail!( + "`codex sandbox --full-auto` is only supported for legacy `sandbox_mode` configs; choose a writable `[permissions]` profile instead" + ); + } return Ok(config); } build_debug_sandbox_config( cli_overrides, ConfigOverrides { - sandbox_mode: Some(SandboxMode::ReadOnly), + sandbox_mode: Some(create_sandbox_mode(full_auto)), + cwd, codex_linux_sandbox_exe, ..Default::default() }, codex_home, + managed_requirements_mode, ) .await .map_err(Into::into) @@ -632,10 +695,17 @@ async fn build_debug_sandbox_config( cli_overrides: Vec<(String, TomlValue)>, harness_overrides: ConfigOverrides, codex_home: Option, + managed_requirements_mode: ManagedRequirementsMode, ) -> std::io::Result { let mut builder = ConfigBuilder::default() .cli_overrides(cli_overrides) .harness_overrides(harness_overrides); + if let ManagedRequirementsMode::Ignore = managed_requirements_mode { + builder = builder.loader_overrides(LoaderOverrides { + ignore_managed_requirements: true, + ..Default::default() + }); + } if let Some(codex_home) = codex_home { builder = builder .codex_home(codex_home.clone()) @@ -652,14 +722,9 @@ fn config_uses_permission_profiles(config: &Config) -> bool { .is_some() } -fn cli_overrides_use_legacy_sandbox_mode(cli_overrides: &[(String, TomlValue)]) -> bool { - cli_overrides.iter().any(|(key, _)| key == "sandbox_mode") -} - #[cfg(test)] mod tests { use super::*; - use pretty_assertions::assert_eq; use tempfile::TempDir; fn escape_toml_path(path: &std::path::Path) -> String { @@ -701,22 +766,29 @@ mod tests { Vec::new(), ConfigOverrides::default(), Some(codex_home_path.clone()), + ManagedRequirementsMode::Include, ) .await?; let legacy_config = build_debug_sandbox_config( Vec::new(), ConfigOverrides { - sandbox_mode: Some(SandboxMode::ReadOnly), + sandbox_mode: Some(create_sandbox_mode(/*full_auto*/ false)), ..Default::default() }, Some(codex_home_path.clone()), + ManagedRequirementsMode::Include, ) .await?; let config = load_debug_sandbox_config_with_codex_home( Vec::new(), /*codex_linux_sandbox_exe*/ None, - /*permissions_profile*/ None, + DebugSandboxConfigOptions { + full_auto: false, + permissions_profile: None, + cwd: None, + include_managed_config: false, + }, Some(codex_home_path), ) .await?; @@ -740,134 +812,48 @@ mod tests { } #[tokio::test] - async fn debug_sandbox_honors_explicit_legacy_sandbox_mode() -> anyhow::Result<()> { + async fn debug_sandbox_rejects_full_auto_for_permission_profiles() -> anyhow::Result<()> { let codex_home = TempDir::new()?; - let codex_home_path = codex_home.path().to_path_buf(); - let cli_overrides = vec![( - "sandbox_mode".to_string(), - TomlValue::String("workspace-write".to_string()), - )]; + let sandbox_paths = TempDir::new()?; + let docs = sandbox_paths.path().join("docs"); + let private = docs.join("private"); + write_permissions_profile_config(&codex_home, &docs, &private)?; - let workspace_write_config = build_debug_sandbox_config( - cli_overrides.clone(), - ConfigOverrides::default(), - Some(codex_home_path.clone()), - ) - .await?; - let read_only_config = build_debug_sandbox_config( + let err = load_debug_sandbox_config_with_codex_home( Vec::new(), - ConfigOverrides { - sandbox_mode: Some(SandboxMode::ReadOnly), - ..Default::default() - }, - Some(codex_home_path.clone()), - ) - .await?; - - let config = load_debug_sandbox_config_with_codex_home( - cli_overrides, /*codex_linux_sandbox_exe*/ None, - /*permissions_profile*/ None, - Some(codex_home_path), - ) - .await?; - - if cfg!(target_os = "windows") { - assert_eq!( - workspace_write_config - .permissions - .file_system_sandbox_policy(), - read_only_config.permissions.file_system_sandbox_policy(), - "workspace-write downgrades to read-only when the Windows sandbox is disabled" - ); - } else { - assert_ne!( - workspace_write_config - .permissions - .file_system_sandbox_policy(), - read_only_config.permissions.file_system_sandbox_policy(), - "test fixture should distinguish explicit workspace-write from read-only" - ); - } - assert_eq!( - config.permissions.file_system_sandbox_policy(), - workspace_write_config - .permissions - .file_system_sandbox_policy(), - ); - - Ok(()) - } - - #[tokio::test] - async fn debug_sandbox_defaults_legacy_configs_to_read_only() -> anyhow::Result<()> { - let codex_home = TempDir::new()?; - let codex_home_path = codex_home.path().to_path_buf(); - - let read_only_config = build_debug_sandbox_config( - Vec::new(), - ConfigOverrides { - sandbox_mode: Some(SandboxMode::ReadOnly), - ..Default::default() + DebugSandboxConfigOptions { + full_auto: true, + permissions_profile: None, + cwd: None, + include_managed_config: false, }, - Some(codex_home_path.clone()), - ) - .await?; - - let config = load_debug_sandbox_config_with_codex_home( - Vec::new(), - /*codex_linux_sandbox_exe*/ None, - /*permissions_profile*/ None, - Some(codex_home_path), - ) - .await?; - - assert!(!config_uses_permission_profiles(&config)); - assert_eq!( - config.permissions.file_system_sandbox_policy(), - read_only_config.permissions.file_system_sandbox_policy(), - ); - - Ok(()) - } - - #[tokio::test] - async fn debug_sandbox_honors_explicit_builtin_permission_profile() -> anyhow::Result<()> { - let codex_home = TempDir::new()?; - - let config = load_debug_sandbox_config_with_codex_home( - Vec::new(), - /*codex_linux_sandbox_exe*/ None, - Some(":workspace".to_string()), Some(codex_home.path().to_path_buf()), ) - .await?; + .await + .expect_err("full-auto should be rejected for active permission profiles"); - assert_eq!( - config.permissions.file_system_sandbox_policy(), - codex_protocol::models::PermissionProfile::workspace_write() - .file_system_sandbox_policy() + assert!( + err.to_string().contains("--full-auto"), + "unexpected error: {err}" ); Ok(()) } #[tokio::test] - async fn explicit_permission_profile_overrides_active_profile_sandbox_mode() - -> anyhow::Result<()> { + async fn debug_sandbox_honors_explicit_builtin_permission_profile() -> anyhow::Result<()> { let codex_home = TempDir::new()?; - std::fs::write( - codex_home.path().join("config.toml"), - "profile = \"legacy\"\n\ - \n\ - [profiles.legacy]\n\ - sandbox_mode = \"danger-full-access\"\n", - )?; let config = load_debug_sandbox_config_with_codex_home( Vec::new(), /*codex_linux_sandbox_exe*/ None, - Some(":workspace".to_string()), + DebugSandboxConfigOptions { + full_auto: false, + permissions_profile: Some(":workspace".to_string()), + cwd: None, + include_managed_config: false, + }, Some(codex_home.path().to_path_buf()), ) .await?; @@ -892,7 +878,12 @@ mod tests { let config = load_debug_sandbox_config_with_codex_home( Vec::new(), /*codex_linux_sandbox_exe*/ None, - Some("limited-read-test".to_string()), + DebugSandboxConfigOptions { + full_auto: false, + permissions_profile: Some("limited-read-test".to_string()), + cwd: None, + include_managed_config: false, + }, Some(codex_home.path().to_path_buf()), ) .await?; @@ -904,6 +895,7 @@ mod tests { )], ConfigOverrides::default(), Some(codex_home.path().to_path_buf()), + ManagedRequirementsMode::Include, ) .await?; @@ -914,4 +906,27 @@ mod tests { Ok(()) } + + #[tokio::test] + async fn debug_sandbox_uses_explicit_profile_cwd() -> anyhow::Result<()> { + let codex_home = TempDir::new()?; + let cwd = TempDir::new()?; + + let config = load_debug_sandbox_config_with_codex_home( + Vec::new(), + /*codex_linux_sandbox_exe*/ None, + DebugSandboxConfigOptions { + full_auto: false, + permissions_profile: Some(":workspace".to_string()), + cwd: Some(cwd.path().to_path_buf()), + include_managed_config: false, + }, + Some(codex_home.path().to_path_buf()), + ) + .await?; + + assert_eq!(config.cwd.as_path(), cwd.path()); + + Ok(()) + } } diff --git a/codex-rs/cli/src/lib.rs b/codex-rs/cli/src/lib.rs index f78afa0fa372..1aacdc6fd3b9 100644 --- a/codex-rs/cli/src/lib.rs +++ b/codex-rs/cli/src/lib.rs @@ -5,6 +5,7 @@ pub(crate) mod login; use clap::Parser; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_cli::CliConfigOverrides; +use std::path::PathBuf; pub use debug_sandbox::run_command_under_landlock; pub use debug_sandbox::run_command_under_seatbelt; @@ -25,6 +26,23 @@ pub struct SeatbeltCommand { #[arg(long = "permissions-profile", value_name = "NAME")] pub permissions_profile: Option, + /// Working directory used for profile resolution and command execution. + #[arg( + short = 'C', + long = "cd", + value_name = "DIR", + requires = "permissions_profile" + )] + pub cwd: Option, + + /// Include managed requirements while resolving an explicit permissions profile. + #[arg( + long = "include-managed-config", + default_value_t = false, + requires = "permissions_profile" + )] + pub include_managed_config: bool, + /// Allow the sandboxed command to bind/connect AF_UNIX sockets rooted at this path. Relative paths are resolved against the current directory. Repeat to allow multiple paths. #[arg(long = "allow-unix-socket", value_parser = parse_allow_unix_socket_path)] pub allow_unix_sockets: Vec, @@ -52,6 +70,23 @@ pub struct LandlockCommand { #[arg(long = "permissions-profile", value_name = "NAME")] pub permissions_profile: Option, + /// Working directory used for profile resolution and command execution. + #[arg( + short = 'C', + long = "cd", + value_name = "DIR", + requires = "permissions_profile" + )] + pub cwd: Option, + + /// Include managed requirements while resolving an explicit permissions profile. + #[arg( + long = "include-managed-config", + default_value_t = false, + requires = "permissions_profile" + )] + pub include_managed_config: bool, + #[clap(skip)] pub config_overrides: CliConfigOverrides, @@ -66,6 +101,23 @@ pub struct WindowsCommand { #[arg(long = "permissions-profile", value_name = "NAME")] pub permissions_profile: Option, + /// Working directory used for profile resolution and command execution. + #[arg( + short = 'C', + long = "cd", + value_name = "DIR", + requires = "permissions_profile" + )] + pub cwd: Option, + + /// Include managed requirements while resolving an explicit permissions profile. + #[arg( + long = "include-managed-config", + default_value_t = false, + requires = "permissions_profile" + )] + pub include_managed_config: bool, + #[clap(skip)] pub config_overrides: CliConfigOverrides, diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index ab33d9f948f0..bb7a15f90e7a 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -1946,6 +1946,41 @@ mod tests { assert_eq!(command.command, vec!["echo"]); } + #[test] + fn sandbox_macos_parses_explicit_profile_controls() { + let cli = MultitoolCli::try_parse_from([ + "codex", + "sandbox", + "macos", + "--permissions-profile", + ":workspace", + "-C", + "/tmp", + "--include-managed-config", + "--", + "echo", + ]) + .expect("parse"); + + let Some(Subcommand::Sandbox(SandboxArgs { + cmd: SandboxCommand::Macos(command), + })) = cli.subcommand + else { + panic!("expected sandbox macos command"); + }; + + assert_eq!(command.cwd.as_deref(), Some(std::path::Path::new("/tmp"))); + assert!(command.include_managed_config); + } + + #[test] + fn sandbox_macos_rejects_explicit_profile_controls_without_profile() { + let err = MultitoolCli::try_parse_from(["codex", "sandbox", "macos", "-C", "/tmp"]) + .expect_err("parse should fail"); + + assert_eq!(err.kind(), clap::error::ErrorKind::MissingRequiredArgument); + } + #[test] fn plugin_marketplace_remove_parses_under_plugin() { let cli = diff --git a/codex-rs/config/src/loader/mod.rs b/codex-rs/config/src/loader/mod.rs index 28e5ff34267f..74dd258eb9c7 100644 --- a/codex-rs/config/src/loader/mod.rs +++ b/codex-rs/config/src/loader/mod.rs @@ -92,41 +92,46 @@ pub async fn load_config_layers_state( cloud_requirements: CloudRequirementsLoader, thread_config_loader: &dyn ThreadConfigLoader, ) -> io::Result { + let ignore_managed_requirements = overrides.ignore_managed_requirements; let ignore_user_config = overrides.ignore_user_config; let ignore_user_and_project_exec_policy_rules = overrides.ignore_user_and_project_exec_policy_rules; let mut config_requirements_toml = ConfigRequirementsWithSources::default(); - if let Some(requirements) = cloud_requirements.get().await.map_err(io::Error::other)? { - merge_requirements_with_remote_sandbox_config( - &mut config_requirements_toml, - RequirementSource::CloudRequirements, - requirements, - ); - } + if !ignore_managed_requirements { + if let Some(requirements) = cloud_requirements.get().await.map_err(io::Error::other)? { + merge_requirements_with_remote_sandbox_config( + &mut config_requirements_toml, + RequirementSource::CloudRequirements, + requirements, + ); + } - #[cfg(target_os = "macos")] - macos::load_managed_admin_requirements_toml( - &mut config_requirements_toml, - overrides - .macos_managed_config_requirements_base64 - .as_deref(), - ) - .await?; + #[cfg(target_os = "macos")] + macos::load_managed_admin_requirements_toml( + &mut config_requirements_toml, + overrides + .macos_managed_config_requirements_base64 + .as_deref(), + ) + .await?; - // Honor the system requirements.toml location. - let requirements_toml_file = system_requirements_toml_file_with_overrides(&overrides)?; - load_requirements_toml(fs, &mut config_requirements_toml, &requirements_toml_file).await?; + // Honor the system requirements.toml location. + let requirements_toml_file = system_requirements_toml_file_with_overrides(&overrides)?; + load_requirements_toml(fs, &mut config_requirements_toml, &requirements_toml_file).await?; + } // Make a best-effort to support the legacy `managed_config.toml` as a // requirements specification. let loaded_config_layers = layer_io::load_config_layers_internal(fs, codex_home, overrides.clone()).await?; - load_requirements_from_legacy_scheme( - &mut config_requirements_toml, - loaded_config_layers.clone(), - ) - .await?; + if !ignore_managed_requirements { + load_requirements_from_legacy_scheme( + &mut config_requirements_toml, + loaded_config_layers.clone(), + ) + .await?; + } let thread_config_context = ThreadConfigContext { thread_id: None, diff --git a/codex-rs/config/src/state.rs b/codex-rs/config/src/state.rs index 6bb846edd92f..08ddbca95e26 100644 --- a/codex-rs/config/src/state.rs +++ b/codex-rs/config/src/state.rs @@ -20,6 +20,7 @@ pub struct LoaderOverrides { pub managed_config_path: Option, pub system_config_path: Option, pub system_requirements_path: Option, + pub ignore_managed_requirements: bool, pub ignore_user_config: bool, pub ignore_user_and_project_exec_policy_rules: bool, //TODO(gt): Add a macos_ prefix to this field and remove the target_os check. @@ -38,6 +39,7 @@ impl LoaderOverrides { managed_config_path: Some(base.join("managed_config.toml")), system_config_path: Some(base.join("config.toml")), system_requirements_path: Some(base.join("requirements.toml")), + ignore_managed_requirements: false, ignore_user_config: false, ignore_user_and_project_exec_policy_rules: false, #[cfg(target_os = "macos")] diff --git a/codex-rs/core/src/config/config_loader_tests.rs b/codex-rs/core/src/config/config_loader_tests.rs index cc465d42b123..b0c9670ae4ce 100644 --- a/codex-rs/core/src/config/config_loader_tests.rs +++ b/codex-rs/core/src/config/config_loader_tests.rs @@ -1084,6 +1084,52 @@ async fn load_config_layers_includes_cloud_requirements() -> anyhow::Result<()> Ok(()) } +#[tokio::test] +async fn load_config_layers_can_ignore_managed_requirements() -> anyhow::Result<()> { + let tmp = tempdir()?; + let codex_home = tmp.path().join("home"); + tokio::fs::create_dir_all(&codex_home).await?; + let cwd = AbsolutePathBuf::from_absolute_path(tmp.path())?; + + let managed_config_path = tmp.path().join("managed_config.toml"); + tokio::fs::write(&managed_config_path, "approval_policy = \"never\"\n").await?; + let system_requirements_path = tmp.path().join("requirements.toml"); + tokio::fs::write( + &system_requirements_path, + "allowed_sandbox_modes = [\"read-only\"]\n", + ) + .await?; + + let mut overrides = LoaderOverrides::with_managed_config_path_for_tests(managed_config_path); + overrides.system_requirements_path = Some(system_requirements_path); + overrides.ignore_managed_requirements = true; + + let cloud_requirements = CloudRequirementsLoader::new(async { + Ok(Some(ConfigRequirementsToml { + allowed_approval_policies: Some(vec![AskForApproval::Never]), + ..Default::default() + })) + }); + + let layers = load_config_layers_state( + LOCAL_FS.as_ref(), + &codex_home, + Some(cwd), + &[] as &[(String, TomlValue)], + overrides, + cloud_requirements, + &codex_config::NoopThreadConfigLoader, + ) + .await?; + + assert_eq!( + layers.requirements_toml(), + &ConfigRequirementsToml::default() + ); + + Ok(()) +} + #[tokio::test] async fn load_config_layers_includes_cloud_hook_requirements() -> anyhow::Result<()> { let tmp = tempdir()?; From 3ec0fbd8a2f7a3ddfa6b5d9fc66db32efac111ee Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Tue, 28 Apr 2026 23:10:44 -0700 Subject: [PATCH 2/2] fix(cli): address sandbox config review comments Co-authored-by: Codex noreply@openai.com --- codex-rs/cli/src/debug_sandbox.rs | 257 +++++++++++++----- codex-rs/cli/src/lib.rs | 2 + codex-rs/cli/src/main.rs | 27 -- .../core/src/config/config_loader_tests.rs | 32 ++- 4 files changed, 205 insertions(+), 113 deletions(-) diff --git a/codex-rs/cli/src/debug_sandbox.rs b/codex-rs/cli/src/debug_sandbox.rs index e334faa49328..e9bc6a046ebc 100644 --- a/codex-rs/cli/src/debug_sandbox.rs +++ b/codex-rs/cli/src/debug_sandbox.rs @@ -43,7 +43,6 @@ pub async fn run_command_under_seatbelt( codex_linux_sandbox_exe: Option, ) -> anyhow::Result<()> { let SeatbeltCommand { - full_auto, permissions_profile, cwd, include_managed_config, @@ -52,20 +51,22 @@ pub async fn run_command_under_seatbelt( config_overrides, command, } = command; + let managed_requirements_mode = ManagedRequirementsMode::for_profile_invocation( + &permissions_profile, + include_managed_config, + ); run_command_under_sandbox( DebugSandboxConfigOptions { - full_auto, permissions_profile, cwd, - include_managed_config, + managed_requirements_mode, }, command, config_overrides, codex_linux_sandbox_exe, - SandboxType::Seatbelt { - allow_unix_sockets, - log_denials, - }, + SandboxType::Seatbelt, + log_denials, + &allow_unix_sockets, ) .await } @@ -83,24 +84,28 @@ pub async fn run_command_under_landlock( codex_linux_sandbox_exe: Option, ) -> anyhow::Result<()> { let LandlockCommand { - full_auto, permissions_profile, cwd, include_managed_config, config_overrides, command, } = command; + let managed_requirements_mode = ManagedRequirementsMode::for_profile_invocation( + &permissions_profile, + include_managed_config, + ); run_command_under_sandbox( DebugSandboxConfigOptions { - full_auto, permissions_profile, cwd, - include_managed_config, + managed_requirements_mode, }, command, config_overrides, codex_linux_sandbox_exe, SandboxType::Landlock, + /*log_denials*/ false, + &[], ) .await } @@ -110,44 +115,44 @@ pub async fn run_command_under_windows( codex_linux_sandbox_exe: Option, ) -> anyhow::Result<()> { let WindowsCommand { - full_auto, permissions_profile, cwd, include_managed_config, config_overrides, command, } = command; + let managed_requirements_mode = ManagedRequirementsMode::for_profile_invocation( + &permissions_profile, + include_managed_config, + ); run_command_under_sandbox( DebugSandboxConfigOptions { - full_auto, permissions_profile, cwd, - include_managed_config, + managed_requirements_mode, }, command, config_overrides, codex_linux_sandbox_exe, SandboxType::Windows, + /*log_denials*/ false, + &[], ) .await } enum SandboxType { #[cfg(target_os = "macos")] - Seatbelt { - allow_unix_sockets: Vec, - log_denials: bool, - }, + Seatbelt, Landlock, Windows, } #[derive(Debug)] struct DebugSandboxConfigOptions { - full_auto: bool, permissions_profile: Option, cwd: Option, - include_managed_config: bool, + managed_requirements_mode: ManagedRequirementsMode, } #[derive(Debug, Clone, Copy)] @@ -156,12 +161,28 @@ enum ManagedRequirementsMode { Ignore, } +impl ManagedRequirementsMode { + fn for_profile_invocation( + permissions_profile: &Option, + include_managed_config: bool, + ) -> Self { + if permissions_profile.is_some() && !include_managed_config { + Self::Ignore + } else { + Self::Include + } + } +} + async fn run_command_under_sandbox( config_options: DebugSandboxConfigOptions, command: Vec, config_overrides: CliConfigOverrides, codex_linux_sandbox_exe: Option, sandbox_type: SandboxType, + log_denials: bool, + #[cfg_attr(not(target_os = "macos"), allow(unused_variables))] + allow_unix_sockets: &[AbsolutePathBuf], ) -> anyhow::Result<()> { let config = load_debug_sandbox_config( config_overrides @@ -198,10 +219,9 @@ async fn run_command_under_sandbox( } #[cfg(target_os = "macos")] - let mut denial_logger = match &sandbox_type { - SandboxType::Seatbelt { log_denials, .. } => log_denials.then(DenialLogger::new).flatten(), - SandboxType::Landlock | SandboxType::Windows => None, - }; + let mut denial_logger = log_denials.then(DenialLogger::new).flatten(); + #[cfg(not(target_os = "macos"))] + let _ = log_denials; let managed_network_requirements_enabled = config.managed_network_requirements_enabled(); @@ -226,9 +246,7 @@ async fn run_command_under_sandbox( let mut child = match sandbox_type { #[cfg(target_os = "macos")] - SandboxType::Seatbelt { - allow_unix_sockets, .. - } => { + SandboxType::Seatbelt => { let file_system_sandbox_policy = config.permissions.file_system_sandbox_policy(); let network_sandbox_policy = config.permissions.network_sandbox_policy(); let args = create_seatbelt_command_args(CreateSeatbeltCommandArgsParams { @@ -238,7 +256,7 @@ async fn run_command_under_sandbox( sandbox_policy_cwd: sandbox_policy_cwd.as_path(), enforce_managed_network: false, network: network.as_ref(), - extra_allow_unix_sockets: &allow_unix_sockets, + extra_allow_unix_sockets: allow_unix_sockets, }); spawn_debug_sandbox_child( PathBuf::from("/usr/bin/sandbox-exec"), @@ -441,14 +459,6 @@ async fn run_command_under_windows_session( std::process::exit(exit_code); } -pub fn create_sandbox_mode(full_auto: bool) -> SandboxMode { - if full_auto { - SandboxMode::WorkspaceWrite - } else { - SandboxMode::ReadOnly - } -} - async fn spawn_debug_sandbox_child( program: PathBuf, args: Vec, @@ -636,18 +646,11 @@ async fn load_debug_sandbox_config_with_codex_home( codex_home: Option, ) -> anyhow::Result { let DebugSandboxConfigOptions { - full_auto, permissions_profile, cwd, - include_managed_config, + managed_requirements_mode, } = options; - let managed_requirements_mode = if permissions_profile.is_some() && !include_managed_config { - ManagedRequirementsMode::Ignore - } else { - ManagedRequirementsMode::Include - }; - if let Some(permissions_profile) = permissions_profile { cli_overrides.push(( "default_permissions".to_string(), @@ -655,6 +658,12 @@ async fn load_debug_sandbox_config_with_codex_home( )); } + // For legacy configs, `codex sandbox` historically defaulted to read-only + // instead of inheriting ambient `sandbox_mode` settings from user/system + // config. Keep that behavior unless this invocation explicitly passes a + // legacy `sandbox_mode` CLI override, which is now the documented writable + // replacement for the removed `--full-auto` flag. + let uses_legacy_sandbox_mode_override = cli_overrides_use_legacy_sandbox_mode(&cli_overrides); let config = build_debug_sandbox_config( cli_overrides.clone(), ConfigOverrides { @@ -667,19 +676,14 @@ async fn load_debug_sandbox_config_with_codex_home( ) .await?; - if config_uses_permission_profiles(&config) { - if full_auto { - anyhow::bail!( - "`codex sandbox --full-auto` is only supported for legacy `sandbox_mode` configs; choose a writable `[permissions]` profile instead" - ); - } + if config_uses_permission_profiles(&config) || uses_legacy_sandbox_mode_override { return Ok(config); } build_debug_sandbox_config( cli_overrides, ConfigOverrides { - sandbox_mode: Some(create_sandbox_mode(full_auto)), + sandbox_mode: Some(SandboxMode::ReadOnly), cwd, codex_linux_sandbox_exe, ..Default::default() @@ -722,9 +726,14 @@ fn config_uses_permission_profiles(config: &Config) -> bool { .is_some() } +fn cli_overrides_use_legacy_sandbox_mode(cli_overrides: &[(String, TomlValue)]) -> bool { + cli_overrides.iter().any(|(key, _)| key == "sandbox_mode") +} + #[cfg(test)] mod tests { use super::*; + use pretty_assertions::assert_eq; use tempfile::TempDir; fn escape_toml_path(path: &std::path::Path) -> String { @@ -772,7 +781,7 @@ mod tests { let legacy_config = build_debug_sandbox_config( Vec::new(), ConfigOverrides { - sandbox_mode: Some(create_sandbox_mode(/*full_auto*/ false)), + sandbox_mode: Some(SandboxMode::ReadOnly), ..Default::default() }, Some(codex_home_path.clone()), @@ -784,10 +793,9 @@ mod tests { Vec::new(), /*codex_linux_sandbox_exe*/ None, DebugSandboxConfigOptions { - full_auto: false, permissions_profile: None, cwd: None, - include_managed_config: false, + managed_requirements_mode: ManagedRequirementsMode::Include, }, Some(codex_home_path), ) @@ -812,30 +820,103 @@ mod tests { } #[tokio::test] - async fn debug_sandbox_rejects_full_auto_for_permission_profiles() -> anyhow::Result<()> { + async fn debug_sandbox_honors_explicit_legacy_sandbox_mode() -> anyhow::Result<()> { let codex_home = TempDir::new()?; - let sandbox_paths = TempDir::new()?; - let docs = sandbox_paths.path().join("docs"); - let private = docs.join("private"); - write_permissions_profile_config(&codex_home, &docs, &private)?; + let codex_home_path = codex_home.path().to_path_buf(); + let cli_overrides = vec![( + "sandbox_mode".to_string(), + TomlValue::String("workspace-write".to_string()), + )]; - let err = load_debug_sandbox_config_with_codex_home( + let workspace_write_config = build_debug_sandbox_config( + cli_overrides.clone(), + ConfigOverrides::default(), + Some(codex_home_path.clone()), + ManagedRequirementsMode::Include, + ) + .await?; + let read_only_config = build_debug_sandbox_config( Vec::new(), + ConfigOverrides { + sandbox_mode: Some(SandboxMode::ReadOnly), + ..Default::default() + }, + Some(codex_home_path.clone()), + ManagedRequirementsMode::Include, + ) + .await?; + + let config = load_debug_sandbox_config_with_codex_home( + cli_overrides, /*codex_linux_sandbox_exe*/ None, DebugSandboxConfigOptions { - full_auto: true, permissions_profile: None, cwd: None, - include_managed_config: false, + managed_requirements_mode: ManagedRequirementsMode::Include, }, - Some(codex_home.path().to_path_buf()), + Some(codex_home_path), ) - .await - .expect_err("full-auto should be rejected for active permission profiles"); + .await?; - assert!( - err.to_string().contains("--full-auto"), - "unexpected error: {err}" + if cfg!(target_os = "windows") { + assert_eq!( + workspace_write_config + .permissions + .file_system_sandbox_policy(), + read_only_config.permissions.file_system_sandbox_policy(), + "workspace-write downgrades to read-only when the Windows sandbox is disabled" + ); + } else { + assert_ne!( + workspace_write_config + .permissions + .file_system_sandbox_policy(), + read_only_config.permissions.file_system_sandbox_policy(), + "test fixture should distinguish explicit workspace-write from read-only" + ); + } + assert_eq!( + config.permissions.file_system_sandbox_policy(), + workspace_write_config + .permissions + .file_system_sandbox_policy(), + ); + + Ok(()) + } + + #[tokio::test] + async fn debug_sandbox_defaults_legacy_configs_to_read_only() -> anyhow::Result<()> { + let codex_home = TempDir::new()?; + let codex_home_path = codex_home.path().to_path_buf(); + + let read_only_config = build_debug_sandbox_config( + Vec::new(), + ConfigOverrides { + sandbox_mode: Some(SandboxMode::ReadOnly), + ..Default::default() + }, + Some(codex_home_path.clone()), + ManagedRequirementsMode::Include, + ) + .await?; + + let config = load_debug_sandbox_config_with_codex_home( + Vec::new(), + /*codex_linux_sandbox_exe*/ None, + DebugSandboxConfigOptions { + permissions_profile: None, + cwd: None, + managed_requirements_mode: ManagedRequirementsMode::Include, + }, + Some(codex_home_path), + ) + .await?; + + assert!(!config_uses_permission_profiles(&config)); + assert_eq!( + config.permissions.file_system_sandbox_policy(), + read_only_config.permissions.file_system_sandbox_policy(), ); Ok(()) @@ -849,10 +930,42 @@ mod tests { Vec::new(), /*codex_linux_sandbox_exe*/ None, DebugSandboxConfigOptions { - full_auto: false, permissions_profile: Some(":workspace".to_string()), cwd: None, - include_managed_config: false, + managed_requirements_mode: ManagedRequirementsMode::Ignore, + }, + Some(codex_home.path().to_path_buf()), + ) + .await?; + + assert_eq!( + config.permissions.file_system_sandbox_policy(), + codex_protocol::models::PermissionProfile::workspace_write() + .file_system_sandbox_policy() + ); + + Ok(()) + } + + #[tokio::test] + async fn explicit_permission_profile_overrides_active_profile_sandbox_mode() + -> anyhow::Result<()> { + let codex_home = TempDir::new()?; + std::fs::write( + codex_home.path().join("config.toml"), + "profile = \"legacy\"\n\ + \n\ + [profiles.legacy]\n\ + sandbox_mode = \"danger-full-access\"\n", + )?; + + let config = load_debug_sandbox_config_with_codex_home( + Vec::new(), + /*codex_linux_sandbox_exe*/ None, + DebugSandboxConfigOptions { + permissions_profile: Some(":workspace".to_string()), + cwd: None, + managed_requirements_mode: ManagedRequirementsMode::Ignore, }, Some(codex_home.path().to_path_buf()), ) @@ -879,10 +992,9 @@ mod tests { Vec::new(), /*codex_linux_sandbox_exe*/ None, DebugSandboxConfigOptions { - full_auto: false, permissions_profile: Some("limited-read-test".to_string()), cwd: None, - include_managed_config: false, + managed_requirements_mode: ManagedRequirementsMode::Ignore, }, Some(codex_home.path().to_path_buf()), ) @@ -916,10 +1028,9 @@ mod tests { Vec::new(), /*codex_linux_sandbox_exe*/ None, DebugSandboxConfigOptions { - full_auto: false, permissions_profile: Some(":workspace".to_string()), cwd: Some(cwd.path().to_path_buf()), - include_managed_config: false, + managed_requirements_mode: ManagedRequirementsMode::Ignore, }, Some(codex_home.path().to_path_buf()), ) diff --git a/codex-rs/cli/src/lib.rs b/codex-rs/cli/src/lib.rs index 1aacdc6fd3b9..6750cbf39e38 100644 --- a/codex-rs/cli/src/lib.rs +++ b/codex-rs/cli/src/lib.rs @@ -20,6 +20,8 @@ pub use login::run_login_with_device_code; pub use login::run_login_with_device_code_fallback_to_browser; pub use login::run_logout; +// TODO: Deduplicate these shared sandbox options if we remove the explicit +// `codex sandbox ` platform subcommands. #[derive(Debug, Parser)] pub struct SeatbeltCommand { /// Named permissions profile to apply from the active configuration stack. diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index bb7a15f90e7a..988187855412 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -1946,33 +1946,6 @@ mod tests { assert_eq!(command.command, vec!["echo"]); } - #[test] - fn sandbox_macos_parses_explicit_profile_controls() { - let cli = MultitoolCli::try_parse_from([ - "codex", - "sandbox", - "macos", - "--permissions-profile", - ":workspace", - "-C", - "/tmp", - "--include-managed-config", - "--", - "echo", - ]) - .expect("parse"); - - let Some(Subcommand::Sandbox(SandboxArgs { - cmd: SandboxCommand::Macos(command), - })) = cli.subcommand - else { - panic!("expected sandbox macos command"); - }; - - assert_eq!(command.cwd.as_deref(), Some(std::path::Path::new("/tmp"))); - assert!(command.include_managed_config); - } - #[test] fn sandbox_macos_rejects_explicit_profile_controls_without_profile() { let err = MultitoolCli::try_parse_from(["codex", "sandbox", "macos", "-C", "/tmp"]) diff --git a/codex-rs/core/src/config/config_loader_tests.rs b/codex-rs/core/src/config/config_loader_tests.rs index b0c9670ae4ce..06d6f9129425 100644 --- a/codex-rs/core/src/config/config_loader_tests.rs +++ b/codex-rs/core/src/config/config_loader_tests.rs @@ -1111,21 +1111,27 @@ async fn load_config_layers_can_ignore_managed_requirements() -> anyhow::Result< })) }); - let layers = load_config_layers_state( - LOCAL_FS.as_ref(), - &codex_home, - Some(cwd), - &[] as &[(String, TomlValue)], - overrides, - cloud_requirements, - &codex_config::NoopThreadConfigLoader, - ) - .await?; + let mut config = ConfigBuilder::default() + .codex_home(codex_home) + .fallback_cwd(Some(cwd.to_path_buf())) + .loader_overrides(overrides) + .cloud_requirements(cloud_requirements) + .build() + .await?; - assert_eq!( - layers.requirements_toml(), - &ConfigRequirementsToml::default() + assert!( + config + .permissions + .approval_policy + .can_set(&AskForApproval::OnRequest) + .is_ok(), + "ignoring managed requirements should leave on-request approval allowed" ); + config + .permissions + .approval_policy + .set(AskForApproval::OnRequest) + .expect("ignoring managed requirements should allow setting on-request approval"); Ok(()) }