From dda8199b7336133baa1612203dbf3483ebabffbb Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Sun, 26 Apr 2026 15:30:40 -0700 Subject: [PATCH] permissions: migrate approval and sandbox consumers to profiles (#19393) ## Why Runtime decisions should not infer permissions from the lossy legacy sandbox projection once `PermissionProfile` is available. In particular, `Disabled` and `External` need to remain distinct, and managed profiles with split filesystem or deny-read rules should not be collapsed before approval, network, safety, or analytics code makes decisions. ## What Changed - Changes managed network proxy setup and network approval logic to use `PermissionProfile` when deciding whether a managed sandbox is active. - Migrates patch safety, Guardian/user-shell approval paths, Landlock helper setup, analytics sandbox classification, and selected turn/session code to profile-backed permissions. - Validates command-level profile overrides against the constrained `PermissionProfile` rather than a strict `SandboxPolicy` round trip. - Preserves configured deny-read restrictions when command profiles are narrowed. - Adds coverage for profile-backed trust, network proxy/approval behavior, patch safety, analytics classification, and command-profile narrowing. ## Verification - `cargo test -p codex-core direct_write_roots` - `cargo test -p codex-core runtime_roots_to_legacy_projection` - `cargo test -p codex-app-server requested_permissions_trust_project_uses_permission_profile_intent` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/19393). * #19395 * #19394 * __->__ #19393 --- .../analytics/src/analytics_client_tests.rs | 5 +- codex-rs/analytics/src/facts.rs | 5 +- codex-rs/analytics/src/reducer.rs | 28 +++++--- .../app-server/src/codex_message_processor.rs | 32 ++++++--- codex-rs/cli/src/debug_sandbox.rs | 2 +- codex-rs/core/src/apply_patch.rs | 3 +- codex-rs/core/src/config/config_tests.rs | 57 ++++++++++++++++ codex-rs/core/src/config/mod.rs | 9 ++- .../core/src/config/network_proxy_spec.rs | 43 ++++++------ .../src/config/network_proxy_spec_tests.rs | 67 +++++++++++++++---- codex-rs/core/src/guardian/review_session.rs | 2 +- codex-rs/core/src/guardian/tests.rs | 9 ++- codex-rs/core/src/landlock.rs | 26 +++---- codex-rs/core/src/safety.rs | 44 ++++++++---- codex-rs/core/src/safety_tests.rs | 22 +++--- codex-rs/core/src/session/mod.rs | 30 +++++---- codex-rs/core/src/session/session.rs | 6 +- codex-rs/core/src/session/tests.rs | 62 ++++++++++------- codex-rs/core/src/session/turn.rs | 3 +- codex-rs/core/src/session/turn_context.rs | 20 +++--- codex-rs/core/src/tasks/user_shell.rs | 20 +++++- codex-rs/core/src/tools/network_approval.rs | 12 ++-- .../core/src/tools/network_approval_tests.rs | 19 ++++-- codex-rs/exec/tests/suite/sandbox.rs | 5 +- 24 files changed, 367 insertions(+), 164 deletions(-) diff --git a/codex-rs/analytics/src/analytics_client_tests.rs b/codex-rs/analytics/src/analytics_client_tests.rs index c0465ca7d..352acbe1f 100644 --- a/codex-rs/analytics/src/analytics_client_tests.rs +++ b/codex-rs/analytics/src/analytics_client_tests.rs @@ -315,7 +315,10 @@ fn sample_turn_resolved_config(turn_id: &str) -> TurnResolvedConfigFact { session_source: SessionSource::Exec, model: "gpt-5".to_string(), model_provider: "openai".to_string(), - sandbox_policy: SandboxPolicy::new_read_only_policy(), + permission_profile: CorePermissionProfile::from_legacy_sandbox_policy( + &SandboxPolicy::new_read_only_policy(), + ), + permission_profile_cwd: PathBuf::from("/tmp"), reasoning_effort: None, reasoning_summary: None, service_tier: None, diff --git a/codex-rs/analytics/src/facts.rs b/codex-rs/analytics/src/facts.rs index 1d371acb1..8ebff278c 100644 --- a/codex-rs/analytics/src/facts.rs +++ b/codex-rs/analytics/src/facts.rs @@ -13,12 +13,12 @@ use codex_protocol::config_types::ModeKind; use codex_protocol::config_types::Personality; use codex_protocol::config_types::ReasoningSummary; use codex_protocol::config_types::ServiceTier; +use codex_protocol::models::PermissionProfile; use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::HookEventName; use codex_protocol::protocol::HookRunStatus; use codex_protocol::protocol::HookSource; -use codex_protocol::protocol::SandboxPolicy; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::SkillScope; use codex_protocol::protocol::SubAgentSource; @@ -62,7 +62,8 @@ pub struct TurnResolvedConfigFact { pub session_source: SessionSource, pub model: String, pub model_provider: String, - pub sandbox_policy: SandboxPolicy, + pub permission_profile: PermissionProfile, + pub permission_profile_cwd: PathBuf, pub reasoning_effort: Option, pub reasoning_summary: Option, pub service_tier: Option, diff --git a/codex-rs/analytics/src/reducer.rs b/codex-rs/analytics/src/reducer.rs index a6ce3fc83..681c25483 100644 --- a/codex-rs/analytics/src/reducer.rs +++ b/codex-rs/analytics/src/reducer.rs @@ -61,6 +61,7 @@ use codex_login::default_client::originator; use codex_protocol::config_types::ModeKind; use codex_protocol::config_types::Personality; use codex_protocol::config_types::ReasoningSummary; +use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::SandboxPolicy; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::SkillScope; @@ -884,7 +885,8 @@ fn codex_turn_event_params( session_source: _session_source, model, model_provider, - sandbox_policy, + permission_profile, + permission_profile_cwd, reasoning_effort, reasoning_summary, service_tier, @@ -909,7 +911,10 @@ fn codex_turn_event_params( parent_thread_id: thread_metadata.parent_thread_id.clone(), model: Some(model), model_provider, - sandbox_policy: Some(sandbox_policy_mode(&sandbox_policy)), + sandbox_policy: Some(sandbox_policy_mode( + &permission_profile, + permission_profile_cwd.as_path(), + )), reasoning_effort: reasoning_effort.map(|value| value.to_string()), reasoning_summary: reasoning_summary_mode(reasoning_summary), service_tier: service_tier @@ -954,12 +959,19 @@ fn codex_turn_event_params( } } -fn sandbox_policy_mode(sandbox_policy: &SandboxPolicy) -> &'static str { - match sandbox_policy { - SandboxPolicy::DangerFullAccess => "full_access", - SandboxPolicy::ReadOnly { .. } => "read_only", - SandboxPolicy::WorkspaceWrite { .. } => "workspace_write", - SandboxPolicy::ExternalSandbox { .. } => "external_sandbox", +fn sandbox_policy_mode(permission_profile: &PermissionProfile, cwd: &Path) -> &'static str { + match permission_profile { + PermissionProfile::Disabled => "full_access", + PermissionProfile::External { .. } => "external_sandbox", + PermissionProfile::Managed { .. } => { + match permission_profile.to_legacy_sandbox_policy(cwd) { + Ok(SandboxPolicy::DangerFullAccess) => "full_access", + Ok(SandboxPolicy::ReadOnly { .. }) => "read_only", + Ok(SandboxPolicy::WorkspaceWrite { .. }) => "workspace_write", + Ok(SandboxPolicy::ExternalSandbox { .. }) => "external_sandbox", + Err(_) => "workspace_write", + } + } } } diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 2c6e172f7..44b9a398c 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -2209,7 +2209,7 @@ impl CodexMessageProcessor { let started_network_proxy = match self.config.permissions.network.as_ref() { Some(spec) => match spec .start_proxy( - self.config.permissions.sandbox_policy.get(), + self.config.permissions.permission_profile.get(), /*policy_decider*/ None, /*blocked_request_observer*/ None, managed_network_requirements_enabled, @@ -2290,17 +2290,11 @@ impl CodexMessageProcessor { &file_system_sandbox_policy, network_sandbox_policy, ); - let sandbox_policy = compatibility_sandbox_policy_for_permission_profile( - &effective_permission_profile, - &file_system_sandbox_policy, - network_sandbox_policy, - sandbox_cwd.as_path(), - ); match self .config .permissions - .sandbox_policy - .can_set(&sandbox_policy) + .permission_profile + .can_set(&effective_permission_profile) { Ok(()) => effective_permission_profile, Err(err) => { @@ -2320,13 +2314,29 @@ impl CodexMessageProcessor { codex_protocol::permissions::FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(&policy, &sandbox_cwd); let network_sandbox_policy = codex_protocol::permissions::NetworkSandboxPolicy::from(&policy); - codex_protocol::models::PermissionProfile::from_runtime_permissions_with_enforcement( + let permission_profile = + codex_protocol::models::PermissionProfile::from_runtime_permissions_with_enforcement( codex_protocol::models::SandboxEnforcement::from_legacy_sandbox_policy( &policy, ), &file_system_sandbox_policy, network_sandbox_policy, - ) + ); + if let Err(err) = self + .config + .permissions + .permission_profile + .can_set(&permission_profile) + { + let error = JSONRPCErrorError { + code: INVALID_REQUEST_ERROR_CODE, + message: format!("invalid sandbox policy: {err}"), + data: None, + }; + self.outgoing.send_error(request, error).await; + return; + } + permission_profile } Err(err) => { let error = JSONRPCErrorError { diff --git a/codex-rs/cli/src/debug_sandbox.rs b/codex-rs/cli/src/debug_sandbox.rs index a59ce31d5..a6cd07699 100644 --- a/codex-rs/cli/src/debug_sandbox.rs +++ b/codex-rs/cli/src/debug_sandbox.rs @@ -171,7 +171,7 @@ async fn run_command_under_sandbox( let network_proxy = match config.permissions.network.as_ref() { Some(spec) => Some( spec.start_proxy( - config.permissions.sandbox_policy.get(), + config.permissions.permission_profile.get(), /*policy_decider*/ None, /*blocked_request_observer*/ None, managed_network_requirements_enabled, diff --git a/codex-rs/core/src/apply_patch.rs b/codex-rs/core/src/apply_patch.rs index d31b4f034..d5ebe4fe1 100644 --- a/codex-rs/core/src/apply_patch.rs +++ b/codex-rs/core/src/apply_patch.rs @@ -35,11 +35,10 @@ pub(crate) async fn apply_patch( file_system_sandbox_policy: &FileSystemSandboxPolicy, action: ApplyPatchAction, ) -> InternalApplyPatchInvocation { - let sandbox_policy = turn_context.sandbox_policy(); match assess_patch_safety( &action, turn_context.approval_policy.value(), - &sandbox_policy, + &turn_context.permission_profile(), file_system_sandbox_policy, &turn_context.cwd, turn_context.windows_sandbox_level, diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 38dce5df3..1d1a60e13 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -58,6 +58,7 @@ use codex_model_provider_info::WireApi; use codex_models_manager::bundled_models_response; use codex_protocol::models::ManagedFileSystemPermissions; use codex_protocol::models::PermissionProfile; +use codex_protocol::models::SandboxEnforcement; use codex_protocol::permissions::FileSystemAccessMode; use codex_protocol::permissions::FileSystemPath; use codex_protocol::permissions::FileSystemSandboxEntry; @@ -6775,6 +6776,62 @@ async fn permission_profile_override_falls_back_when_disallowed_by_requirements( Ok(()) } +#[tokio::test] +async fn permission_profile_override_preserves_split_write_roots() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cwd = codex_home.path().join("workspace"); + let outside_root = codex_home.path().join("outside-write"); + std::fs::create_dir_all(&cwd)?; + std::fs::create_dir_all(&outside_root)?; + let outside_root = + AbsolutePathBuf::from_absolute_path(outside_root).expect("outside root is absolute"); + let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + access: FileSystemAccessMode::Read, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: outside_root.clone(), + }, + access: FileSystemAccessMode::Write, + }, + ]); + let permission_profile = PermissionProfile::from_runtime_permissions_with_enforcement( + SandboxEnforcement::Managed, + &file_system_sandbox_policy, + NetworkSandboxPolicy::Restricted, + ); + + let config = ConfigBuilder::without_managed_config_for_tests() + .codex_home(codex_home.path().to_path_buf()) + .fallback_cwd(Some(cwd)) + .harness_overrides(ConfigOverrides { + permission_profile: Some(permission_profile), + ..Default::default() + }) + .build() + .await?; + + assert!( + config + .permissions + .file_system_sandbox_policy() + .can_write_path_with_cwd(outside_root.as_path(), config.cwd.as_path()) + ); + assert!(matches!( + config.permissions.sandbox_policy.get(), + SandboxPolicy::WorkspaceWrite { .. } + )); + assert_eq!( + config.permissions.network_sandbox_policy(), + NetworkSandboxPolicy::Restricted + ); + Ok(()) +} + #[tokio::test] async fn requirements_web_search_mode_overrides_danger_full_access_default() -> std::io::Result<()> { diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index e8c83fe95..099569f5e 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -2396,10 +2396,17 @@ impl Config { None => (None, None), }; let has_network_requirements = network_requirements.is_some(); + let network_permission_profile = if *constrained_sandbox_policy.get() + == original_sandbox_policy + { + permission_profile.clone() + } else { + PermissionProfile::from_legacy_sandbox_policy(constrained_sandbox_policy.get()) + }; let network = NetworkProxySpec::from_config_and_constraints( configured_network_proxy_config, network_requirements, - constrained_sandbox_policy.get(), + &network_permission_profile, ) .map_err(|err| { if let Some(source) = network_requirements_source.as_ref() { diff --git a/codex-rs/core/src/config/network_proxy_spec.rs b/codex-rs/core/src/config/network_proxy_spec.rs index 1bb5e1c9f..631a826ac 100644 --- a/codex-rs/core/src/config/network_proxy_spec.rs +++ b/codex-rs/core/src/config/network_proxy_spec.rs @@ -16,7 +16,7 @@ use codex_network_proxy::build_config_state; use codex_network_proxy::host_and_port_from_network_addr; use codex_network_proxy::normalize_host; use codex_network_proxy::validate_policy_against_constraints; -use codex_protocol::protocol::SandboxPolicy; +use codex_protocol::models::PermissionProfile; use std::collections::HashSet; use std::sync::Arc; @@ -89,7 +89,7 @@ impl NetworkProxySpec { pub(crate) fn from_config_and_constraints( config: NetworkProxyConfig, requirements: Option, - sandbox_policy: &SandboxPolicy, + permission_profile: &PermissionProfile, ) -> std::io::Result { let base_config = config.clone(); let hard_deny_allowlist_misses = requirements @@ -99,7 +99,7 @@ impl NetworkProxySpec { Self::apply_requirements( config, requirements, - sandbox_policy, + permission_profile, hard_deny_allowlist_misses, ) } else { @@ -122,7 +122,7 @@ impl NetworkProxySpec { pub async fn start_proxy( &self, - sandbox_policy: &SandboxPolicy, + permission_profile: &PermissionProfile, policy_decider: Option>, blocked_request_observer: Option>, enable_network_approval_flow: bool, @@ -133,10 +133,7 @@ impl NetworkProxySpec { if enable_network_approval_flow && !self.hard_deny_allowlist_misses { if let Some(policy_decider) = policy_decider { builder = builder.policy_decider_arc(policy_decider); - } else if matches!( - sandbox_policy, - SandboxPolicy::ReadOnly { .. } | SandboxPolicy::WorkspaceWrite { .. } - ) { + } else if Self::managed_sandbox_active(permission_profile) { builder = builder .policy_decider(|_request| async { NetworkDecision::ask("not_allowed") }); } @@ -154,14 +151,14 @@ impl NetworkProxySpec { Ok(StartedNetworkProxy::new(proxy, handle)) } - pub(crate) fn recompute_for_sandbox_policy( + pub(crate) fn recompute_for_permission_profile( &self, - sandbox_policy: &SandboxPolicy, + permission_profile: &PermissionProfile, ) -> std::io::Result { Self::from_config_and_constraints( self.base_config.clone(), self.requirements.clone(), - sandbox_policy, + permission_profile, ) } @@ -216,13 +213,13 @@ impl NetworkProxySpec { fn apply_requirements( mut config: NetworkProxyConfig, requirements: &NetworkConstraints, - sandbox_policy: &SandboxPolicy, + permission_profile: &PermissionProfile, hard_deny_allowlist_misses: bool, ) -> (NetworkProxyConfig, NetworkProxyConstraints) { let mut constraints = NetworkProxyConstraints::default(); let allowlist_expansion_enabled = - Self::allowlist_expansion_enabled(sandbox_policy, hard_deny_allowlist_misses); - let denylist_expansion_enabled = Self::denylist_expansion_enabled(sandbox_policy); + Self::allowlist_expansion_enabled(permission_profile, hard_deny_allowlist_misses); + let denylist_expansion_enabled = Self::denylist_expansion_enabled(permission_profile); if let Some(enabled) = requirements.enabled { config.network.enabled = enabled; @@ -322,24 +319,22 @@ impl NetworkProxySpec { } fn allowlist_expansion_enabled( - sandbox_policy: &SandboxPolicy, + permission_profile: &PermissionProfile, hard_deny_allowlist_misses: bool, ) -> bool { - matches!( - sandbox_policy, - SandboxPolicy::ReadOnly { .. } | SandboxPolicy::WorkspaceWrite { .. } - ) && !hard_deny_allowlist_misses + Self::managed_sandbox_active(permission_profile) && !hard_deny_allowlist_misses } fn managed_allowed_domains_only(requirements: &NetworkConstraints) -> bool { requirements.managed_allowed_domains_only.unwrap_or(false) } - fn denylist_expansion_enabled(sandbox_policy: &SandboxPolicy) -> bool { - matches!( - sandbox_policy, - SandboxPolicy::ReadOnly { .. } | SandboxPolicy::WorkspaceWrite { .. } - ) + fn denylist_expansion_enabled(permission_profile: &PermissionProfile) -> bool { + Self::managed_sandbox_active(permission_profile) + } + + fn managed_sandbox_active(permission_profile: &PermissionProfile) -> bool { + matches!(permission_profile, PermissionProfile::Managed { .. }) } fn merge_domain_lists(mut managed: Vec, user_entries: &[String]) -> Vec { diff --git a/codex-rs/core/src/config/network_proxy_spec_tests.rs b/codex-rs/core/src/config/network_proxy_spec_tests.rs index fb4231aca..14b7c1c33 100644 --- a/codex-rs/core/src/config/network_proxy_spec_tests.rs +++ b/codex-rs/core/src/config/network_proxy_spec_tests.rs @@ -2,8 +2,16 @@ use super::*; use codex_config::NetworkDomainPermissionToml; use codex_config::NetworkDomainPermissionsToml; use codex_network_proxy::NetworkDomainPermission; +use codex_protocol::models::ManagedFileSystemPermissions; +use codex_protocol::models::PermissionProfile; +use codex_protocol::permissions::NetworkSandboxPolicy; +use codex_protocol::protocol::SandboxPolicy; use pretty_assertions::assert_eq; +fn permission_profile_for_sandbox_policy(sandbox_policy: &SandboxPolicy) -> PermissionProfile { + PermissionProfile::from_legacy_sandbox_policy(sandbox_policy) +} + fn domain_permissions( entries: impl IntoIterator, ) -> NetworkDomainPermissionsToml { @@ -54,7 +62,7 @@ fn requirements_allowed_domains_are_a_baseline_for_user_allowlist() { let spec = NetworkProxySpec::from_config_and_constraints( config, Some(requirements), - &SandboxPolicy::new_read_only_policy(), + &permission_profile_for_sandbox_policy(&SandboxPolicy::new_read_only_policy()), ) .expect("config should stay within the managed allowlist"); @@ -89,7 +97,7 @@ fn requirements_allowed_domains_do_not_override_user_denies_for_same_pattern() { let spec = NetworkProxySpec::from_config_and_constraints( config, Some(requirements), - &SandboxPolicy::new_workspace_write_policy(), + &permission_profile_for_sandbox_policy(&SandboxPolicy::new_workspace_write_policy()), ) .expect("managed allowlist should not erase a user deny"); @@ -121,7 +129,7 @@ fn requirements_allowlist_expansion_keeps_user_entries_mutable() { let spec = NetworkProxySpec::from_config_and_constraints( config, Some(requirements), - &SandboxPolicy::new_workspace_write_policy(), + &permission_profile_for_sandbox_policy(&SandboxPolicy::new_workspace_write_policy()), ) .expect("managed baseline should still allow user edits"); @@ -144,6 +152,41 @@ fn requirements_allowlist_expansion_keeps_user_entries_mutable() { .expect("user allowlist entries should not become managed constraints"); } +#[test] +fn managed_unrestricted_profile_allows_domain_expansion() { + let mut config = NetworkProxyConfig::default(); + config + .network + .set_allowed_domains(vec!["api.example.com".to_string()]); + let requirements = NetworkConstraints { + domains: Some(domain_permissions([( + "*.example.com", + NetworkDomainPermissionToml::Allow, + )])), + ..Default::default() + }; + let permission_profile = PermissionProfile::Managed { + file_system: ManagedFileSystemPermissions::Unrestricted, + network: NetworkSandboxPolicy::Restricted, + }; + + let spec = NetworkProxySpec::from_config_and_constraints( + config, + Some(requirements), + &permission_profile, + ) + .expect("managed unrestricted filesystem should still use managed network constraints"); + + assert_eq!( + spec.config.network.allowed_domains(), + Some(vec![ + "*.example.com".to_string(), + "api.example.com".to_string() + ]) + ); + assert_eq!(spec.constraints.allowlist_expansion_enabled, Some(true)); +} + #[test] fn danger_full_access_keeps_managed_allowlist_and_denylist_fixed() { let mut config = NetworkProxyConfig::default(); @@ -164,7 +207,7 @@ fn danger_full_access_keeps_managed_allowlist_and_denylist_fixed() { let spec = NetworkProxySpec::from_config_and_constraints( config, Some(requirements), - &SandboxPolicy::DangerFullAccess, + &permission_profile_for_sandbox_policy(&SandboxPolicy::DangerFullAccess), ) .expect("yolo mode should pin the effective policy to the managed baseline"); @@ -198,7 +241,7 @@ fn managed_allowed_domains_only_disables_default_mode_allowlist_expansion() { let spec = NetworkProxySpec::from_config_and_constraints( config, Some(requirements), - &SandboxPolicy::new_workspace_write_policy(), + &permission_profile_for_sandbox_policy(&SandboxPolicy::new_workspace_write_policy()), ) .expect("managed baseline should still load"); @@ -227,7 +270,7 @@ fn managed_allowed_domains_only_ignores_user_allowlist_and_hard_denies_misses() let spec = NetworkProxySpec::from_config_and_constraints( config, Some(requirements), - &SandboxPolicy::new_workspace_write_policy(), + &permission_profile_for_sandbox_policy(&SandboxPolicy::new_workspace_write_policy()), ) .expect("managed-only allowlist should still load"); @@ -257,7 +300,7 @@ fn managed_allowed_domains_only_without_managed_allowlist_blocks_all_user_domain let spec = NetworkProxySpec::from_config_and_constraints( config, Some(requirements), - &SandboxPolicy::new_workspace_write_policy(), + &permission_profile_for_sandbox_policy(&SandboxPolicy::new_workspace_write_policy()), ) .expect("managed-only mode should treat missing managed allowlist as empty"); @@ -281,7 +324,7 @@ fn managed_allowed_domains_only_blocks_all_user_domains_in_full_access_without_m let spec = NetworkProxySpec::from_config_and_constraints( config, Some(requirements), - &SandboxPolicy::DangerFullAccess, + &permission_profile_for_sandbox_policy(&SandboxPolicy::DangerFullAccess), ) .expect("managed-only mode should treat missing managed allowlist as empty"); @@ -308,7 +351,7 @@ fn deny_only_requirements_do_not_create_allow_constraints_in_full_access() { let spec = NetworkProxySpec::from_config_and_constraints( config, Some(requirements), - &SandboxPolicy::DangerFullAccess, + &permission_profile_for_sandbox_policy(&SandboxPolicy::DangerFullAccess), ) .expect("deny-only requirements should not constrain the allowlist"); @@ -341,7 +384,7 @@ fn allow_only_requirements_do_not_create_deny_constraints_in_full_access() { let spec = NetworkProxySpec::from_config_and_constraints( config, Some(requirements), - &SandboxPolicy::DangerFullAccess, + &permission_profile_for_sandbox_policy(&SandboxPolicy::DangerFullAccess), ) .expect("allow-only requirements should not constrain the denylist"); @@ -374,7 +417,7 @@ fn requirements_denied_domains_are_a_baseline_for_default_mode() { let spec = NetworkProxySpec::from_config_and_constraints( config, Some(requirements), - &SandboxPolicy::new_workspace_write_policy(), + &permission_profile_for_sandbox_policy(&SandboxPolicy::new_workspace_write_policy()), ) .expect("default mode should merge managed and user deny entries"); @@ -409,7 +452,7 @@ fn requirements_denylist_expansion_keeps_user_entries_mutable() { let spec = NetworkProxySpec::from_config_and_constraints( config, Some(requirements), - &SandboxPolicy::new_workspace_write_policy(), + &permission_profile_for_sandbox_policy(&SandboxPolicy::new_workspace_write_policy()), ) .expect("managed baseline should still allow user edits"); diff --git a/codex-rs/core/src/guardian/review_session.rs b/codex-rs/core/src/guardian/review_session.rs index 754cb43af..fac589c58 100644 --- a/codex-rs/core/src/guardian/review_session.rs +++ b/codex-rs/core/src/guardian/review_session.rs @@ -874,7 +874,7 @@ pub(crate) fn build_guardian_review_session_config( guardian_config.permissions.network = Some(NetworkProxySpec::from_config_and_constraints( live_network_config, network_constraints, - &SandboxPolicy::new_read_only_policy(), + guardian_config.permissions.permission_profile.get(), )?); } for feature in [ diff --git a/codex-rs/core/src/guardian/tests.rs b/codex-rs/core/src/guardian/tests.rs index 76b4a8464..641e24c01 100644 --- a/codex-rs/core/src/guardian/tests.rs +++ b/codex-rs/core/src/guardian/tests.rs @@ -27,6 +27,7 @@ use codex_protocol::ThreadId; use codex_protocol::approvals::NetworkApprovalProtocol; use codex_protocol::config_types::ApprovalsReviewer; use codex_protocol::models::ContentItem; +use codex_protocol::models::PermissionProfile; use codex_protocol::models::ResponseItem; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; @@ -1942,7 +1943,7 @@ async fn guardian_review_session_config_preserves_parent_network_proxy() { }), ..Default::default() }), - parent_config.permissions.sandbox_policy.get(), + parent_config.permissions.permission_profile.get(), ) .expect("network proxy spec"); parent_config.permissions.network = Some(network.clone()); @@ -2007,7 +2008,7 @@ async fn guardian_review_session_config_uses_live_network_proxy_state() { NetworkProxySpec::from_config_and_constraints( parent_network, /*requirements*/ None, - parent_config.permissions.sandbox_policy.get(), + parent_config.permissions.permission_profile.get(), ) .expect("parent network proxy spec"), ); @@ -2032,7 +2033,9 @@ async fn guardian_review_session_config_uses_live_network_proxy_state() { NetworkProxySpec::from_config_and_constraints( live_network, /*requirements*/ None, - &SandboxPolicy::new_read_only_policy(), + &PermissionProfile::from_legacy_sandbox_policy( + &SandboxPolicy::new_read_only_policy(), + ), ) .expect("live network proxy spec") ) diff --git a/codex-rs/core/src/landlock.rs b/codex-rs/core/src/landlock.rs index 7e2de35e8..56059f8ee 100644 --- a/codex-rs/core/src/landlock.rs +++ b/codex-rs/core/src/landlock.rs @@ -2,9 +2,8 @@ use crate::spawn::SpawnChildRequest; use crate::spawn::StdioPolicy; use crate::spawn::spawn_child_async; use codex_network_proxy::NetworkProxy; -use codex_protocol::permissions::FileSystemSandboxPolicy; -use codex_protocol::permissions::NetworkSandboxPolicy; -use codex_protocol::protocol::SandboxPolicy; +use codex_protocol::models::PermissionProfile; +use codex_sandboxing::compatibility_sandbox_policy_for_permission_profile; use codex_sandboxing::landlock::CODEX_LINUX_SANDBOX_ARG0; use codex_sandboxing::landlock::allow_network_for_proxy; use codex_sandboxing::landlock::create_linux_sandbox_command_args_for_policies; @@ -18,15 +17,15 @@ use tokio::process::Child; /// isolation plus seccomp for network restrictions. /// /// Unlike macOS Seatbelt where we directly embed the policy text, the Linux -/// helper is a separate executable. We pass the legacy [`SandboxPolicy`] plus -/// split filesystem/network policies as JSON so the helper can migrate -/// incrementally without breaking older call sites. +/// helper is a separate executable. We pass both the canonical split +/// filesystem/network policies and a compatibility legacy projection as JSON +/// until the helper protocol no longer needs the legacy field. #[allow(clippy::too_many_arguments)] pub async fn spawn_command_under_linux_sandbox

( codex_linux_sandbox_exe: P, command: Vec, command_cwd: AbsolutePathBuf, - sandbox_policy: &SandboxPolicy, + permission_profile: &PermissionProfile, sandbox_policy_cwd: &AbsolutePathBuf, use_legacy_landlock: bool, stdio_policy: StdioPolicy, @@ -36,15 +35,18 @@ pub async fn spawn_command_under_linux_sandbox

( where P: AsRef, { - let file_system_sandbox_policy = FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd( - sandbox_policy, - sandbox_policy_cwd, + let (file_system_sandbox_policy, network_sandbox_policy) = + permission_profile.to_runtime_permissions(); + let sandbox_policy = compatibility_sandbox_policy_for_permission_profile( + permission_profile, + &file_system_sandbox_policy, + network_sandbox_policy, + sandbox_policy_cwd.as_path(), ); - let network_sandbox_policy = NetworkSandboxPolicy::from(sandbox_policy); let args = create_linux_sandbox_command_args_for_policies( command, command_cwd.as_path(), - sandbox_policy, + &sandbox_policy, &file_system_sandbox_policy, network_sandbox_policy, sandbox_policy_cwd, diff --git a/codex-rs/core/src/safety.rs b/codex-rs/core/src/safety.rs index 843145de3..c8c85a681 100644 --- a/codex-rs/core/src/safety.rs +++ b/codex-rs/core/src/safety.rs @@ -6,9 +6,9 @@ use crate::util::resolve_path; use codex_apply_patch::ApplyPatchAction; use codex_apply_patch::ApplyPatchFileChange; use codex_protocol::config_types::WindowsSandboxLevel; +use codex_protocol::models::PermissionProfile; use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::protocol::AskForApproval; -use codex_protocol::protocol::SandboxPolicy; use codex_sandboxing::SandboxType; use codex_sandboxing::get_platform_sandbox; use codex_utils_absolute_path::AbsolutePathBuf; @@ -33,7 +33,7 @@ pub enum SafetyCheck { pub fn assess_patch_safety( action: &ApplyPatchAction, policy: AskForApproval, - sandbox_policy: &SandboxPolicy, + permission_profile: &PermissionProfile, file_system_sandbox_policy: &FileSystemSandboxPolicy, cwd: &AbsolutePathBuf, windows_sandbox_level: WindowsSandboxLevel, @@ -71,10 +71,11 @@ pub fn assess_patch_safety( || matches!(policy, AskForApproval::OnFailure) { if matches!( - sandbox_policy, - SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } + permission_profile, + PermissionProfile::Disabled | PermissionProfile::External { .. } ) { - // DangerFullAccess is intended to bypass sandboxing entirely. + // Disabled and External profiles intentionally do not apply an + // outer Codex filesystem sandbox. SafetyCheck::AutoApprove { sandbox_type: SandboxType::None, user_explicitly_approved: false, @@ -91,7 +92,12 @@ pub fn assess_patch_safety( None => { if rejects_sandbox_approval { SafetyCheck::Reject { - reason: patch_rejection_reason(sandbox_policy).to_string(), + reason: patch_rejection_reason( + permission_profile, + file_system_sandbox_policy, + cwd, + ) + .to_string(), } } else { SafetyCheck::AskUser @@ -101,19 +107,31 @@ pub fn assess_patch_safety( } } else if rejects_sandbox_approval { SafetyCheck::Reject { - reason: patch_rejection_reason(sandbox_policy).to_string(), + reason: patch_rejection_reason(permission_profile, file_system_sandbox_policy, cwd) + .to_string(), } } else { SafetyCheck::AskUser } } -fn patch_rejection_reason(sandbox_policy: &SandboxPolicy) -> &'static str { - match sandbox_policy { - SandboxPolicy::ReadOnly { .. } => PATCH_REJECTED_READ_ONLY_REASON, - SandboxPolicy::WorkspaceWrite { .. } - | SandboxPolicy::DangerFullAccess - | SandboxPolicy::ExternalSandbox { .. } => PATCH_REJECTED_OUTSIDE_PROJECT_REASON, +fn patch_rejection_reason( + permission_profile: &PermissionProfile, + file_system_sandbox_policy: &FileSystemSandboxPolicy, + cwd: &AbsolutePathBuf, +) -> &'static str { + match permission_profile { + PermissionProfile::Managed { .. } + if !file_system_sandbox_policy.has_full_disk_write_access() + && file_system_sandbox_policy + .get_writable_roots_with_cwd(cwd.as_path()) + .is_empty() => + { + PATCH_REJECTED_READ_ONLY_REASON + } + PermissionProfile::Managed { .. } + | PermissionProfile::Disabled + | PermissionProfile::External { .. } => PATCH_REJECTED_OUTSIDE_PROJECT_REASON, } } diff --git a/codex-rs/core/src/safety_tests.rs b/codex-rs/core/src/safety_tests.rs index 774673f88..0ca10e66e 100644 --- a/codex-rs/core/src/safety_tests.rs +++ b/codex-rs/core/src/safety_tests.rs @@ -1,14 +1,20 @@ use super::*; +use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::FileSystemAccessMode; use codex_protocol::protocol::FileSystemPath; use codex_protocol::protocol::FileSystemSandboxEntry; use codex_protocol::protocol::FileSystemSpecialPath; use codex_protocol::protocol::GranularApprovalConfig; +use codex_protocol::protocol::SandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; use core_test_support::PathExt; use pretty_assertions::assert_eq; use tempfile::TempDir; +fn permission_profile_for_policy(sandbox_policy: &SandboxPolicy) -> PermissionProfile { + PermissionProfile::from_legacy_sandbox_policy(sandbox_policy) +} + #[test] fn test_writable_roots_constraint() { // Use a temporary directory as our workspace to avoid touching @@ -75,7 +81,7 @@ fn external_sandbox_auto_approves_in_on_request() { assess_patch_safety( &add_inside, AskForApproval::OnRequest, - &policy, + &permission_profile_for_policy(&policy), &FileSystemSandboxPolicy::from(&policy), &cwd, WindowsSandboxLevel::Disabled @@ -105,7 +111,7 @@ fn granular_with_all_flags_true_matches_on_request_for_out_of_root_patch() { assess_patch_safety( &add_outside, AskForApproval::OnRequest, - &policy_workspace_only, + &permission_profile_for_policy(&policy_workspace_only), &FileSystemSandboxPolicy::from(&policy_workspace_only), &cwd, WindowsSandboxLevel::Disabled, @@ -122,7 +128,7 @@ fn granular_with_all_flags_true_matches_on_request_for_out_of_root_patch() { request_permissions: true, mcp_elicitations: true, }), - &policy_workspace_only, + &permission_profile_for_policy(&policy_workspace_only), &FileSystemSandboxPolicy::from(&policy_workspace_only), &cwd, WindowsSandboxLevel::Disabled, @@ -155,7 +161,7 @@ fn granular_sandbox_approval_false_rejects_out_of_root_patch() { request_permissions: true, mcp_elicitations: true, }), - &policy_workspace_only, + &permission_profile_for_policy(&policy_workspace_only), &FileSystemSandboxPolicy::from(&policy_workspace_only), &cwd, WindowsSandboxLevel::Disabled, @@ -185,7 +191,7 @@ fn read_only_policy_rejects_patch_with_read_only_reason() { assess_patch_safety( &action, AskForApproval::Never, - &sandbox_policy, + &permission_profile_for_policy(&sandbox_policy), &file_system_sandbox_policy, &cwd, WindowsSandboxLevel::Disabled, @@ -229,7 +235,7 @@ fn explicit_unreadable_paths_prevent_auto_approval_for_external_sandbox() { assess_patch_safety( &action, AskForApproval::OnRequest, - &sandbox_policy, + &permission_profile_for_policy(&sandbox_policy), &file_system_sandbox_policy, &cwd, WindowsSandboxLevel::Disabled, @@ -273,7 +279,7 @@ fn explicit_read_only_subpaths_prevent_auto_approval_for_external_sandbox() { assess_patch_safety( &action, AskForApproval::OnRequest, - &sandbox_policy, + &permission_profile_for_policy(&sandbox_policy), &file_system_sandbox_policy, &cwd, WindowsSandboxLevel::Disabled, @@ -306,7 +312,7 @@ fn missing_project_dot_codex_config_requires_approval() { assess_patch_safety( &action, AskForApproval::OnRequest, - &sandbox_policy, + &permission_profile_for_policy(&sandbox_policy), &file_system_sandbox_policy, &cwd, WindowsSandboxLevel::Disabled, diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 866458a2c..d3f365cc2 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -845,8 +845,10 @@ impl Session { } } - fn managed_network_proxy_active_for_sandbox_policy(sandbox_policy: &SandboxPolicy) -> bool { - !matches!(sandbox_policy, SandboxPolicy::DangerFullAccess) + fn managed_network_proxy_active_for_permission_profile( + permission_profile: &PermissionProfile, + ) -> bool { + !matches!(permission_profile, PermissionProfile::Disabled) } /// Builds the `x-codex-beta-features` header value for this session. @@ -879,7 +881,7 @@ impl Session { async fn start_managed_network_proxy( spec: &crate::config::NetworkProxySpec, exec_policy: &codex_execpolicy::Policy, - sandbox_policy: &SandboxPolicy, + permission_profile: &PermissionProfile, network_policy_decider: Option>, blocked_request_observer: Option>, managed_network_requirements_enabled: bool, @@ -896,7 +898,7 @@ impl Session { .unwrap_or_else(|_| spec.clone()); let network_proxy = spec .start_proxy( - sandbox_policy, + permission_profile, network_policy_decider, blocked_request_observer, managed_network_requirements_enabled, @@ -914,7 +916,7 @@ impl Session { Ok((network_proxy, session_network_proxy)) } - async fn refresh_managed_network_proxy_for_current_sandbox_policy(&self) { + async fn refresh_managed_network_proxy_for_current_permission_profile(&self) { let Some(started_proxy) = self.services.network_proxy.as_ref() else { return; }; @@ -935,7 +937,8 @@ impl Session { return; }; - let spec = match spec.recompute_for_sandbox_policy(&session_configuration.sandbox_policy()) + let spec = match spec + .recompute_for_permission_profile(&session_configuration.permission_profile()) { Ok(spec) => spec, Err(err) => { @@ -1285,7 +1288,7 @@ impl Session { &self, updates: SessionSettingsUpdate, ) -> ConstraintResult<()> { - let (previous_cwd, sandbox_policy_changed, next_cwd, codex_home, session_source) = { + let (previous_cwd, permission_profile_changed, next_cwd, codex_home, session_source) = { let mut state = self.state.lock().await; let updated = match state.session_configuration.apply(&updates) { Ok(updated) => updated, @@ -1296,16 +1299,17 @@ impl Session { }; let previous_cwd = state.session_configuration.cwd.clone(); - let previous_sandbox_policy = state.session_configuration.sandbox_policy(); - let updated_sandbox_policy = updated.sandbox_policy(); - let sandbox_policy_changed = previous_sandbox_policy != updated_sandbox_policy; + let previous_permission_profile = state.session_configuration.permission_profile(); + let updated_permission_profile = updated.permission_profile(); + let permission_profile_changed = + previous_permission_profile != updated_permission_profile; let next_cwd = updated.cwd.clone(); let codex_home = updated.codex_home.clone(); let session_source = updated.session_source.clone(); state.session_configuration = updated; ( previous_cwd, - sandbox_policy_changed, + permission_profile_changed, next_cwd, codex_home, session_source, @@ -1318,8 +1322,8 @@ impl Session { &codex_home, &session_source, ); - if sandbox_policy_changed { - self.refresh_managed_network_proxy_for_current_sandbox_policy() + if permission_profile_changed { + self.refresh_managed_network_proxy_for_current_permission_profile() .await; } diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 1c725745f..bf2e36a27 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -730,7 +730,7 @@ impl Session { let (network_proxy, session_network_proxy) = Self::start_managed_network_proxy( spec, current_exec_policy.as_ref(), - config.permissions.sandbox_policy.get(), + config.permissions.permission_profile.get(), network_policy_decider.as_ref().map(Arc::clone), blocked_request_observer.as_ref().map(Arc::clone), managed_network_requirements_configured, @@ -885,8 +885,8 @@ impl Session { history_entry_count, initial_messages, network_proxy: session_network_proxy.filter(|_| { - Self::managed_network_proxy_active_for_sandbox_policy( - &session_sandbox_policy, + Self::managed_network_proxy_active_for_permission_profile( + session_configuration.permission_profile.get(), ) }), rollout_path, diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 6b1bddbb8..286ed0695 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -159,6 +159,10 @@ use std::time::Duration as StdDuration; mod guardian_tests; +fn permission_profile_for_sandbox_policy(sandbox_policy: &SandboxPolicy) -> PermissionProfile { + PermissionProfile::from_legacy_sandbox_policy(sandbox_policy) +} + struct InstructionsTestCase { slug: &'static str, expects_apply_patch_description: bool, @@ -593,7 +597,7 @@ async fn start_managed_network_proxy_applies_execpolicy_network_rules() -> anyho let spec = crate::config::NetworkProxySpec::from_config_and_constraints( NetworkProxyConfig::default(), /*requirements*/ None, - &SandboxPolicy::new_workspace_write_policy(), + &permission_profile_for_sandbox_policy(&SandboxPolicy::new_workspace_write_policy()), )?; let mut exec_policy = Policy::empty(); exec_policy.add_network_rule( @@ -606,7 +610,7 @@ async fn start_managed_network_proxy_applies_execpolicy_network_rules() -> anyho let (started_proxy, _) = Session::start_managed_network_proxy( &spec, &exec_policy, - &SandboxPolicy::new_workspace_write_policy(), + &permission_profile_for_sandbox_policy(&SandboxPolicy::new_workspace_write_policy()), /*network_policy_decider*/ None, /*blocked_request_observer*/ None, /*managed_network_requirements_enabled*/ false, @@ -637,7 +641,7 @@ async fn start_managed_network_proxy_ignores_invalid_execpolicy_network_rules() managed_allowed_domains_only: Some(true), ..Default::default() }), - &SandboxPolicy::new_workspace_write_policy(), + &permission_profile_for_sandbox_policy(&SandboxPolicy::new_workspace_write_policy()), )?; let mut exec_policy = Policy::empty(); exec_policy.add_network_rule( @@ -650,7 +654,7 @@ async fn start_managed_network_proxy_ignores_invalid_execpolicy_network_rules() let (started_proxy, _) = Session::start_managed_network_proxy( &spec, &exec_policy, - &SandboxPolicy::new_workspace_write_policy(), + &permission_profile_for_sandbox_policy(&SandboxPolicy::new_workspace_write_policy()), /*network_policy_decider*/ None, /*blocked_request_observer*/ None, /*managed_network_requirements_enabled*/ false, @@ -674,7 +678,7 @@ async fn managed_network_proxy_decider_survives_full_access_start() -> anyhow::R enabled: Some(true), ..Default::default() }), - &SandboxPolicy::DangerFullAccess, + &permission_profile_for_sandbox_policy(&SandboxPolicy::DangerFullAccess), )?; let exec_policy = Policy::empty(); let decider_calls = Arc::new(std::sync::atomic::AtomicUsize::new(0)); @@ -689,7 +693,7 @@ async fn managed_network_proxy_decider_survives_full_access_start() -> anyhow::R let (started_proxy, _) = Session::start_managed_network_proxy( &spec, &exec_policy, - &SandboxPolicy::DangerFullAccess, + &permission_profile_for_sandbox_policy(&SandboxPolicy::DangerFullAccess), Some(network_policy_decider), /*blocked_request_observer*/ None, /*managed_network_requirements_enabled*/ true, @@ -697,7 +701,9 @@ async fn managed_network_proxy_decider_survives_full_access_start() -> anyhow::R ) .await?; - let spec = spec.recompute_for_sandbox_policy(&SandboxPolicy::new_workspace_write_policy())?; + let spec = spec.recompute_for_permission_profile(&permission_profile_for_sandbox_policy( + &SandboxPolicy::new_workspace_write_policy(), + ))?; spec.apply_to_started_proxy(&started_proxy).await?; let current_cfg = started_proxy.proxy().current_cfg().await?; assert_eq!(current_cfg.network.allowed_domains(), None); @@ -754,12 +760,12 @@ async fn new_turn_refreshes_managed_network_proxy_for_sandbox_change() -> anyhow let spec = crate::config::NetworkProxySpec::from_config_and_constraints( network_config, Some(requirements), - &initial_policy, + &permission_profile_for_sandbox_policy(&initial_policy), )?; let (started_proxy, _) = Session::start_managed_network_proxy( &spec, &Policy::empty(), - &initial_policy, + &permission_profile_for_sandbox_policy(&initial_policy), /*network_policy_decider*/ None, /*blocked_request_observer*/ None, /*managed_network_requirements_enabled*/ false, @@ -832,14 +838,15 @@ async fn danger_full_access_turns_do_not_expose_managed_network_proxy() -> anyho enabled: Some(true), ..Default::default() }), - &SandboxPolicy::DangerFullAccess, + &permission_profile_for_sandbox_policy(&SandboxPolicy::DangerFullAccess), )?; let session = make_session_with_config(move |config| { - config.permissions.sandbox_policy = - codex_config::Constrained::allow_any(SandboxPolicy::DangerFullAccess); - config.permissions.permission_profile = - codex_config::Constrained::allow_any(PermissionProfile::Disabled); + let cwd = config.cwd.clone(); + config + .permissions + .set_legacy_sandbox_policy(SandboxPolicy::DangerFullAccess, cwd.as_path()) + .expect("test setup should allow sandbox policy"); config.permissions.network = Some(network_spec); }) .await?; @@ -897,14 +904,15 @@ async fn danger_full_access_tool_attempts_do_not_enforce_managed_network() -> an enabled: Some(true), ..Default::default() }), - &SandboxPolicy::DangerFullAccess, + &permission_profile_for_sandbox_policy(&SandboxPolicy::DangerFullAccess), )?; let session = make_session_with_config(move |config| { - config.permissions.sandbox_policy = - codex_config::Constrained::allow_any(SandboxPolicy::DangerFullAccess); - config.permissions.permission_profile = - codex_config::Constrained::allow_any(PermissionProfile::Disabled); + let cwd = config.cwd.clone(); + config + .permissions + .set_legacy_sandbox_policy(SandboxPolicy::DangerFullAccess, cwd.as_path()) + .expect("test setup should allow sandbox policy"); config.permissions.network = Some(network_spec); let layers = config @@ -971,11 +979,15 @@ async fn workspace_write_turns_continue_to_expose_managed_network_proxy() -> any enabled: Some(true), ..Default::default() }), - &sandbox_policy, + &permission_profile_for_sandbox_policy(&sandbox_policy), )?; let session = make_session_with_config(move |config| { - config.permissions.sandbox_policy = codex_config::Constrained::allow_any(sandbox_policy); + let cwd = config.cwd.clone(); + config + .permissions + .set_legacy_sandbox_policy(sandbox_policy, cwd.as_path()) + .expect("test setup should allow sandbox policy"); config.permissions.network = Some(network_spec); }) .await?; @@ -994,11 +1006,15 @@ async fn user_shell_commands_do_not_inherit_managed_network_proxy() -> anyhow::R enabled: Some(true), ..Default::default() }), - &sandbox_policy, + &permission_profile_for_sandbox_policy(&sandbox_policy), )?; let (session, rx) = make_session_with_config_and_rx(move |config| { - config.permissions.sandbox_policy = codex_config::Constrained::allow_any(sandbox_policy); + let cwd = config.cwd.clone(); + config + .permissions + .set_legacy_sandbox_policy(sandbox_policy, cwd.as_path()) + .expect("test setup should allow sandbox policy"); config.permissions.network = Some(network_spec); }) .await?; diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index 827053f0f..6383b892e 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -692,7 +692,8 @@ async fn track_turn_resolved_config_analytics( session_source: thread_config.session_source, model: turn_context.model_info.slug.clone(), model_provider: turn_context.config.model_provider_id.clone(), - sandbox_policy: turn_context.sandbox_policy(), + permission_profile: turn_context.permission_profile(), + permission_profile_cwd: turn_context.cwd.to_path_buf(), reasoning_effort: turn_context.reasoning_effort, reasoning_summary: Some(turn_context.reasoning_summary), service_tier: turn_context.config.service_tier, diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index 3cdaad2b4..b9b553926 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -538,16 +538,18 @@ impl Session { let turn_environments = self.resolve_turn_environments(&effective_environments)?; let previous_cwd = state.session_configuration.cwd.clone(); - let previous_sandbox_policy = state.session_configuration.sandbox_policy(); - let next_sandbox_policy = next.sandbox_policy(); - let sandbox_policy_changed = previous_sandbox_policy != next_sandbox_policy; + let previous_permission_profile = + state.session_configuration.permission_profile(); + let next_permission_profile = next.permission_profile(); + let permission_profile_changed = + previous_permission_profile != next_permission_profile; let codex_home = next.codex_home.clone(); let session_source = next.session_source.clone(); state.session_configuration = next.clone(); Ok(( next, turn_environments, - sandbox_policy_changed, + permission_profile_changed, previous_cwd, codex_home, session_source, @@ -560,7 +562,7 @@ impl Session { let ( session_configuration, turn_environments, - sandbox_policy_changed, + permission_profile_changed, previous_cwd, codex_home, session_source, @@ -587,8 +589,8 @@ impl Session { &session_source, ); - if sandbox_policy_changed { - self.refresh_managed_network_proxy_for_current_sandbox_policy() + if permission_profile_changed { + self.refresh_managed_network_proxy_for_current_permission_profile() .await; } @@ -691,8 +693,8 @@ impl Session { .network_proxy .as_ref() .and_then(|started_proxy| { - Self::managed_network_proxy_active_for_sandbox_policy( - &session_configuration.sandbox_policy(), + Self::managed_network_proxy_active_for_permission_profile( + &session_configuration.permission_profile(), ) .then(|| started_proxy.proxy()) }), diff --git a/codex-rs/core/src/tasks/user_shell.rs b/codex-rs/core/src/tasks/user_shell.rs index f12200e54..61e7bc15a 100644 --- a/codex-rs/core/src/tasks/user_shell.rs +++ b/codex-rs/core/src/tasks/user_shell.rs @@ -3,6 +3,10 @@ use std::time::Duration; use codex_async_utils::CancelErr; use codex_async_utils::OrCancelExt; +use codex_network_proxy::PROXY_ACTIVE_ENV_KEY; +use codex_network_proxy::PROXY_ENV_KEYS; +#[cfg(target_os = "macos")] +use codex_network_proxy::PROXY_GIT_SSH_COMMAND_ENV_KEY; use codex_protocol::user_input::UserInput; use tokio_util::sync::CancellationToken; use tracing::error; @@ -123,10 +127,24 @@ pub(crate) async fn execute_user_shell_command( let use_login_shell = true; let session_shell = session.user_shell(); let display_command = session_shell.derive_exec_args(&command, use_login_shell); - let exec_env_map = create_env( + let mut exec_env_map = create_env( &turn_context.shell_environment_policy, Some(session.conversation_id), ); + if exec_env_map.contains_key(PROXY_ACTIVE_ENV_KEY) { + for key in PROXY_ENV_KEYS { + exec_env_map.remove(*key); + } + #[cfg(target_os = "macos")] + if exec_env_map + .get(PROXY_GIT_SSH_COMMAND_ENV_KEY) + .is_some_and(|value| { + value.starts_with(codex_network_proxy::CODEX_PROXY_GIT_SSH_COMMAND_MARKER) + }) + { + exec_env_map.remove(PROXY_GIT_SSH_COMMAND_ENV_KEY); + } + } let exec_command = maybe_wrap_shell_lc_with_snapshot( &display_command, session_shell.as_ref(), diff --git a/codex-rs/core/src/tools/network_approval.rs b/codex-rs/core/src/tools/network_approval.rs index af0331700..1264e809b 100644 --- a/codex-rs/core/src/tools/network_approval.rs +++ b/codex-rs/core/src/tools/network_approval.rs @@ -21,11 +21,11 @@ use codex_network_proxy::NetworkProxy; use codex_protocol::approvals::NetworkApprovalContext; use codex_protocol::approvals::NetworkApprovalProtocol; use codex_protocol::approvals::NetworkPolicyRuleAction; +use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::Event; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::ReviewDecision; -use codex_protocol::protocol::SandboxPolicy; use codex_protocol::protocol::WarningEvent; use indexmap::IndexMap; use std::collections::HashMap; @@ -127,11 +127,8 @@ fn allows_network_approval_flow(policy: AskForApproval) -> bool { !matches!(policy, AskForApproval::Never) } -fn sandbox_policy_allows_network_approval_flow(policy: &SandboxPolicy) -> bool { - matches!( - policy, - SandboxPolicy::ReadOnly { .. } | SandboxPolicy::WorkspaceWrite { .. } - ) +fn permission_profile_allows_network_approval_flow(permission_profile: &PermissionProfile) -> bool { + matches!(permission_profile, PermissionProfile::Managed { .. }) } impl PendingApprovalDecision { @@ -359,8 +356,7 @@ impl NetworkApprovalService { .await; return NetworkDecision::deny(REASON_NOT_ALLOWED); }; - let sandbox_policy = turn_context.sandbox_policy(); - if !sandbox_policy_allows_network_approval_flow(&sandbox_policy) { + if !permission_profile_allows_network_approval_flow(&turn_context.permission_profile()) { pending.set_decision(PendingApprovalDecision::Deny).await; self.pending_host_approvals.lock().await.remove(&key); self.record_outcome_for_single_active_call(NetworkApprovalOutcome::DeniedByPolicy( diff --git a/codex-rs/core/src/tools/network_approval_tests.rs b/codex-rs/core/src/tools/network_approval_tests.rs index ac0046228..683c8c753 100644 --- a/codex-rs/core/src/tools/network_approval_tests.rs +++ b/codex-rs/core/src/tools/network_approval_tests.rs @@ -1,6 +1,8 @@ use super::*; use crate::sandboxing::SandboxPermissions; use codex_network_proxy::BlockedRequestArgs; +use codex_protocol::models::PermissionProfile; +use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::SandboxPolicy; use core_test_support::PathBufExt; @@ -185,14 +187,19 @@ fn only_never_policy_disables_network_approval_flow() { #[test] fn network_approval_flow_is_limited_to_restricted_sandbox_modes() { - assert!(sandbox_policy_allows_network_approval_flow( - &SandboxPolicy::new_read_only_policy() + assert!(permission_profile_allows_network_approval_flow( + &PermissionProfile::from_legacy_sandbox_policy(&SandboxPolicy::new_read_only_policy()) )); - assert!(sandbox_policy_allows_network_approval_flow( - &SandboxPolicy::new_workspace_write_policy() + assert!(permission_profile_allows_network_approval_flow( + &PermissionProfile::from_legacy_sandbox_policy(&SandboxPolicy::new_workspace_write_policy()) )); - assert!(!sandbox_policy_allows_network_approval_flow( - &SandboxPolicy::DangerFullAccess + assert!(!permission_profile_allows_network_approval_flow( + &PermissionProfile::Disabled + )); + assert!(!permission_profile_allows_network_approval_flow( + &PermissionProfile::External { + network: NetworkSandboxPolicy::Restricted, + } )); } diff --git a/codex-rs/exec/tests/suite/sandbox.rs b/codex-rs/exec/tests/suite/sandbox.rs index 84d4a6beb..feb1a7b8c 100644 --- a/codex-rs/exec/tests/suite/sandbox.rs +++ b/codex-rs/exec/tests/suite/sandbox.rs @@ -89,13 +89,16 @@ async fn spawn_command_under_sandbox( env: HashMap, ) -> std::io::Result { use codex_core::spawn_command_under_linux_sandbox; + use codex_protocol::models::PermissionProfile; + let codex_linux_sandbox_exe = core_test_support::find_codex_linux_sandbox_exe() .map_err(|err| io::Error::new(io::ErrorKind::NotFound, err))?; + let permission_profile = PermissionProfile::from_legacy_sandbox_policy(sandbox_policy); spawn_command_under_linux_sandbox( codex_linux_sandbox_exe, command, command_cwd, - sandbox_policy, + &permission_profile, sandbox_cwd, /*use_legacy_landlock*/ false, stdio_policy,