From 346d2ace3067ff223fe8a031ef11bda018956b31 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Sat, 25 Apr 2026 16:34:19 -0700 Subject: [PATCH] permissions: remove core legacy policy round trips --- codex-rs/core/src/config/config_tests.rs | 17 +-- codex-rs/core/src/config/mod.rs | 4 +- codex-rs/core/src/exec_policy.rs | 33 +++-- codex-rs/core/src/exec_policy_tests.rs | 80 +++++++++-- codex-rs/core/src/session/tests.rs | 3 +- codex-rs/core/src/session/turn_context.rs | 6 +- codex-rs/core/src/tools/handlers/shell.rs | 3 +- .../tools/runtimes/shell/unix_escalation.rs | 38 ++--- .../runtimes/shell/unix_escalation_tests.rs | 61 ++++---- .../core/src/unified_exec/process_manager.rs | 3 +- codex-rs/protocol/src/approvals.rs | 5 - codex-rs/sandboxing/src/policy_transforms.rs | 134 ------------------ .../sandboxing/src/policy_transforms_tests.rs | 63 -------- 13 files changed, 147 insertions(+), 303 deletions(-) diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index e32abc6a89..85e3d3e185 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -2094,24 +2094,25 @@ fn web_search_mode_disabled_overrides_legacy_request() { #[test] fn web_search_mode_for_turn_uses_preference_for_read_only() { let web_search_mode = Constrained::allow_any(WebSearchMode::Cached); - let mode = - resolve_web_search_mode_for_turn(&web_search_mode, &SandboxPolicy::new_read_only_policy()); + let permission_profile = + PermissionProfile::from_legacy_sandbox_policy(&SandboxPolicy::new_read_only_policy()); + let mode = resolve_web_search_mode_for_turn(&web_search_mode, &permission_profile); assert_eq!(mode, WebSearchMode::Cached); } #[test] -fn web_search_mode_for_turn_prefers_live_for_danger_full_access() { +fn web_search_mode_for_turn_prefers_live_for_disabled_permissions() { let web_search_mode = Constrained::allow_any(WebSearchMode::Cached); - let mode = resolve_web_search_mode_for_turn(&web_search_mode, &SandboxPolicy::DangerFullAccess); + let mode = resolve_web_search_mode_for_turn(&web_search_mode, &PermissionProfile::Disabled); assert_eq!(mode, WebSearchMode::Live); } #[test] -fn web_search_mode_for_turn_respects_disabled_for_danger_full_access() { +fn web_search_mode_for_turn_respects_disabled_for_disabled_permissions() { let web_search_mode = Constrained::allow_any(WebSearchMode::Disabled); - let mode = resolve_web_search_mode_for_turn(&web_search_mode, &SandboxPolicy::DangerFullAccess); + let mode = resolve_web_search_mode_for_turn(&web_search_mode, &PermissionProfile::Disabled); assert_eq!(mode, WebSearchMode::Disabled); } @@ -2131,7 +2132,7 @@ fn web_search_mode_for_turn_falls_back_when_live_is_disallowed() -> anyhow::Resu }) } })?; - let mode = resolve_web_search_mode_for_turn(&web_search_mode, &SandboxPolicy::DangerFullAccess); + let mode = resolve_web_search_mode_for_turn(&web_search_mode, &PermissionProfile::Disabled); assert_eq!(mode, WebSearchMode::Cached); Ok(()) @@ -6695,7 +6696,7 @@ async fn requirements_web_search_mode_overrides_danger_full_access_default() -> assert_eq!( resolve_web_search_mode_for_turn( &config.web_search_mode, - config.permissions.sandbox_policy.get(), + &config.permissions.permission_profile(), ), WebSearchMode::Cached, ); diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index d8fe67b838..dee7216347 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -1606,11 +1606,11 @@ fn multi_agent_v2_toml_config(features: Option<&FeaturesToml>) -> Option<&MultiA pub(crate) fn resolve_web_search_mode_for_turn( web_search_mode: &Constrained, - sandbox_policy: &SandboxPolicy, + permission_profile: &PermissionProfile, ) -> WebSearchMode { let preferred = web_search_mode.value(); - if matches!(sandbox_policy, SandboxPolicy::DangerFullAccess) + if matches!(permission_profile, PermissionProfile::Disabled) && preferred != WebSearchMode::Disabled { for mode in [ diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 54ad8058d0..7f55ede63a 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -20,10 +20,10 @@ use codex_execpolicy::RuleMatch; use codex_execpolicy::blocking_append_allow_prefix_rule; use codex_execpolicy::blocking_append_network_rule; use codex_protocol::approvals::ExecPolicyAmendment; +use codex_protocol::models::PermissionProfile; use codex_protocol::permissions::FileSystemSandboxKind; use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::protocol::AskForApproval; -use codex_protocol::protocol::SandboxPolicy; use codex_shell_command::is_dangerous_command::command_might_be_dangerous; use codex_shell_command::is_safe_command::is_known_safe_command; use thiserror::Error; @@ -204,7 +204,7 @@ pub(crate) struct ExecPolicyManager { pub(crate) struct ExecApprovalRequest<'a> { pub(crate) command: &'a [String], pub(crate) approval_policy: AskForApproval, - pub(crate) sandbox_policy: &'a SandboxPolicy, + pub(crate) permission_profile: PermissionProfile, pub(crate) file_system_sandbox_policy: &'a FileSystemSandboxPolicy, pub(crate) sandbox_permissions: SandboxPermissions, pub(crate) prefix_rule: Option>, @@ -238,7 +238,7 @@ impl ExecPolicyManager { let ExecApprovalRequest { command, approval_policy, - sandbox_policy, + permission_profile, file_system_sandbox_policy, sandbox_permissions, prefix_rule, @@ -252,7 +252,7 @@ impl ExecPolicyManager { let exec_policy_fallback = |cmd: &[String]| { render_decision_for_unmatched_command( approval_policy, - sandbox_policy, + &permission_profile, file_system_sandbox_policy, cmd, sandbox_permissions, @@ -580,7 +580,7 @@ pub async fn load_exec_policy(config_stack: &ConfigLayerStack) -> Result { let sandbox_is_explicitly_disabled = matches!( - sandbox_policy, - SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } + permission_profile, + PermissionProfile::Disabled | PermissionProfile::External { .. } ); if sandbox_is_explicitly_disabled { // If the sandbox is explicitly disabled, we should allow the command to run @@ -670,6 +670,21 @@ pub fn render_decision_for_unmatched_command( } } +fn profile_is_managed_read_only( + permission_profile: &PermissionProfile, + file_system_sandbox_policy: &FileSystemSandboxPolicy, +) -> bool { + matches!(permission_profile, PermissionProfile::Managed { .. }) + && matches!( + file_system_sandbox_policy.kind, + FileSystemSandboxKind::Restricted + ) + && !file_system_sandbox_policy + .entries + .iter() + .any(|entry| entry.access.can_write()) +} + fn default_policy_path(codex_home: &Path) -> PathBuf { codex_home.join(RULES_DIR_NAME).join(DEFAULT_POLICY_FILE) } diff --git a/codex-rs/core/src/exec_policy_tests.rs b/codex-rs/core/src/exec_policy_tests.rs index fe4560a781..a5a062f4bd 100644 --- a/codex-rs/core/src/exec_policy_tests.rs +++ b/codex-rs/core/src/exec_policy_tests.rs @@ -15,10 +15,12 @@ use codex_config::RequirementsExecPolicy; use codex_config::config_toml::ConfigToml; use codex_config::config_toml::ProjectConfig; use codex_protocol::config_types::TrustLevel; +use codex_protocol::models::PermissionProfile; use codex_protocol::permissions::FileSystemAccessMode; use codex_protocol::permissions::FileSystemPath; use codex_protocol::permissions::FileSystemSandboxEntry; use codex_protocol::permissions::FileSystemSpecialPath; +use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::GranularApprovalConfig; use codex_protocol::protocol::SandboxPolicy; @@ -116,6 +118,10 @@ fn external_file_system_sandbox_policy() -> FileSystemSandboxPolicy { FileSystemSandboxPolicy::external_sandbox() } +fn permission_profile_from_sandbox_policy(sandbox_policy: &SandboxPolicy) -> PermissionProfile { + PermissionProfile::from_legacy_sandbox_policy(sandbox_policy) +} + async fn test_config() -> (TempDir, Config) { let home = TempDir::new().expect("create temp dir"); let config = ConfigBuilder::without_managed_config_for_tests() @@ -954,7 +960,7 @@ fn unmatched_granular_policy_still_prompts_for_restricted_sandbox_escalation() { request_permissions: true, mcp_elicitations: true, }), - &SandboxPolicy::new_read_only_policy(), + &permission_profile_from_sandbox_policy(&SandboxPolicy::new_read_only_policy()), &read_only_file_system_sandbox_policy(), &command, SandboxPermissions::RequireEscalated, @@ -972,7 +978,7 @@ fn unmatched_on_request_uses_split_filesystem_policy_for_escalation_prompts() { Decision::Prompt, render_decision_for_unmatched_command( AskForApproval::OnRequest, - &SandboxPolicy::DangerFullAccess, + &PermissionProfile::Disabled, &restricted_file_system_policy, &command, SandboxPermissions::RequireEscalated, @@ -981,6 +987,33 @@ fn unmatched_on_request_uses_split_filesystem_policy_for_escalation_prompts() { ); } +#[test] +fn managed_cwd_write_profile_is_not_read_only() { + let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + access: FileSystemAccessMode::Read, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: FileSystemAccessMode::Write, + }, + ]); + let permission_profile = PermissionProfile::from_runtime_permissions( + &file_system_sandbox_policy, + NetworkSandboxPolicy::Restricted, + ); + + assert!(!profile_is_managed_read_only( + &permission_profile, + &file_system_sandbox_policy + )); +} + #[tokio::test] async fn exec_approval_requirement_prompts_for_inline_additional_permissions_under_on_request() { assert_exec_approval_requirement_for_command( @@ -1058,7 +1091,9 @@ async fn mixed_rule_and_sandbox_prompt_prioritizes_rule_for_rejection_decision() request_permissions: true, mcp_elicitations: true, }), - sandbox_policy: &SandboxPolicy::new_read_only_policy(), + permission_profile: permission_profile_from_sandbox_policy( + &SandboxPolicy::new_read_only_policy(), + ), file_system_sandbox_policy: &read_only_file_system_sandbox_policy(), sandbox_permissions: SandboxPermissions::RequireEscalated, prefix_rule: None, @@ -1095,7 +1130,9 @@ async fn mixed_rule_and_sandbox_prompt_rejects_when_granular_rules_are_disabled( request_permissions: true, mcp_elicitations: true, }), - sandbox_policy: &SandboxPolicy::new_read_only_policy(), + permission_profile: permission_profile_from_sandbox_policy( + &SandboxPolicy::new_read_only_policy(), + ), file_system_sandbox_policy: &read_only_file_system_sandbox_policy(), sandbox_permissions: SandboxPermissions::RequireEscalated, prefix_rule: None, @@ -1119,7 +1156,9 @@ async fn exec_approval_requirement_falls_back_to_heuristics() { .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &command, approval_policy: AskForApproval::UnlessTrusted, - sandbox_policy: &SandboxPolicy::new_read_only_policy(), + permission_profile: permission_profile_from_sandbox_policy( + &SandboxPolicy::new_read_only_policy(), + ), file_system_sandbox_policy: &read_only_file_system_sandbox_policy(), sandbox_permissions: SandboxPermissions::UseDefault, prefix_rule: None, @@ -1144,7 +1183,9 @@ async fn empty_bash_lc_script_falls_back_to_original_command() { .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &command, approval_policy: AskForApproval::UnlessTrusted, - sandbox_policy: &SandboxPolicy::new_read_only_policy(), + permission_profile: permission_profile_from_sandbox_policy( + &SandboxPolicy::new_read_only_policy(), + ), file_system_sandbox_policy: &read_only_file_system_sandbox_policy(), sandbox_permissions: SandboxPermissions::UseDefault, prefix_rule: None, @@ -1173,7 +1214,9 @@ async fn whitespace_bash_lc_script_falls_back_to_original_command() { .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &command, approval_policy: AskForApproval::UnlessTrusted, - sandbox_policy: &SandboxPolicy::new_read_only_policy(), + permission_profile: permission_profile_from_sandbox_policy( + &SandboxPolicy::new_read_only_policy(), + ), file_system_sandbox_policy: &read_only_file_system_sandbox_policy(), sandbox_permissions: SandboxPermissions::UseDefault, prefix_rule: None, @@ -1202,7 +1245,9 @@ async fn request_rule_uses_prefix_rule() { .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &command, approval_policy: AskForApproval::OnRequest, - sandbox_policy: &SandboxPolicy::new_read_only_policy(), + permission_profile: permission_profile_from_sandbox_policy( + &SandboxPolicy::new_read_only_policy(), + ), file_system_sandbox_policy: &read_only_file_system_sandbox_policy(), sandbox_permissions: SandboxPermissions::RequireEscalated, prefix_rule: Some(vec!["cargo".to_string(), "install".to_string()]), @@ -1234,7 +1279,7 @@ async fn request_rule_falls_back_when_prefix_rule_does_not_approve_all_commands( .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &command, approval_policy: AskForApproval::OnRequest, - sandbox_policy: &SandboxPolicy::DangerFullAccess, + permission_profile: PermissionProfile::Disabled, file_system_sandbox_policy: &unrestricted_file_system_sandbox_policy(), sandbox_permissions: SandboxPermissions::RequireEscalated, prefix_rule: Some(vec!["cargo".to_string(), "install".to_string()]), @@ -1273,7 +1318,7 @@ async fn heuristics_apply_when_other_commands_match_policy() { .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &command, approval_policy: AskForApproval::UnlessTrusted, - sandbox_policy: &SandboxPolicy::DangerFullAccess, + permission_profile: PermissionProfile::Disabled, file_system_sandbox_policy: &unrestricted_file_system_sandbox_policy(), sandbox_permissions: SandboxPermissions::UseDefault, prefix_rule: None, @@ -1759,7 +1804,9 @@ async fn verify_approval_requirement_for_unsafe_powershell_command() { .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &sneaky_command, approval_policy: AskForApproval::OnRequest, - sandbox_policy: &SandboxPolicy::new_read_only_policy(), + permission_profile: permission_profile_from_sandbox_policy( + &SandboxPolicy::new_read_only_policy(), + ), file_system_sandbox_policy: &read_only_file_system_sandbox_policy(), sandbox_permissions: permissions, prefix_rule: None, @@ -1783,7 +1830,9 @@ async fn verify_approval_requirement_for_unsafe_powershell_command() { .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &dangerous_command, approval_policy: AskForApproval::OnRequest, - sandbox_policy: &SandboxPolicy::new_read_only_policy(), + permission_profile: permission_profile_from_sandbox_policy( + &SandboxPolicy::new_read_only_policy(), + ), file_system_sandbox_policy: &read_only_file_system_sandbox_policy(), sandbox_permissions: permissions, prefix_rule: None, @@ -1803,7 +1852,9 @@ async fn verify_approval_requirement_for_unsafe_powershell_command() { .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &dangerous_command, approval_policy: AskForApproval::Never, - sandbox_policy: &SandboxPolicy::new_read_only_policy(), + permission_profile: permission_profile_from_sandbox_policy( + &SandboxPolicy::new_read_only_policy(), + ), file_system_sandbox_policy: &read_only_file_system_sandbox_policy(), sandbox_permissions: permissions, prefix_rule: None, @@ -1897,11 +1948,12 @@ async fn assert_exec_approval_requirement_for_command( None => Arc::new(Policy::empty()), }; + let permission_profile = permission_profile_from_sandbox_policy(&sandbox_policy); let requirement = ExecPolicyManager::new(policy) .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &command, approval_policy, - sandbox_policy: &sandbox_policy, + permission_profile, file_system_sandbox_policy: &file_system_sandbox_policy, sandbox_permissions, prefix_rule, diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 1cb11f70bc..9e55206952 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -7849,14 +7849,13 @@ async fn rejects_escalated_permissions_when_policy_not_on_request() { turn_context_mut.permission_profile = PermissionProfile::Disabled; let file_system_sandbox_policy = turn_context.file_system_sandbox_policy(); - let sandbox_policy = turn_context.sandbox_policy(); let exec_approval_requirement = session .services .exec_policy .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: ¶ms.command, approval_policy: turn_context.approval_policy.value(), - sandbox_policy: &sandbox_policy, + permission_profile: turn_context.permission_profile(), file_system_sandbox_policy: &file_system_sandbox_policy, sandbox_permissions: SandboxPermissions::UseDefault, prefix_rule: None, diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index 018935c425..3a0fb81eea 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -384,10 +384,10 @@ impl Session { per_turn_config.permissions.permission_profile = session_configuration.permission_profile.clone(); let sandbox_policy = session_configuration.sandbox_policy(); - per_turn_config.permissions.sandbox_policy = - Constrained::allow_only(sandbox_policy.clone()); + per_turn_config.permissions.sandbox_policy = Constrained::allow_only(sandbox_policy); + let permission_profile = session_configuration.permission_profile(); let resolved_web_search_mode = - resolve_web_search_mode_for_turn(&per_turn_config.web_search_mode, &sandbox_policy); + resolve_web_search_mode_for_turn(&per_turn_config.web_search_mode, &permission_profile); if let Err(err) = per_turn_config .web_search_mode .set(resolved_web_search_mode) diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index b43fab30b4..835b6170f8 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -514,14 +514,13 @@ impl ShellHandler { emitter.begin(event_ctx).await; let file_system_sandbox_policy = turn.file_system_sandbox_policy(); - let sandbox_policy = turn.sandbox_policy(); let exec_approval_requirement = session .services .exec_policy .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &exec_params.command, approval_policy: turn.approval_policy.value(), - sandbox_policy: &sandbox_policy, + permission_profile: turn.permission_profile(), file_system_sandbox_policy: &file_system_sandbox_policy, sandbox_permissions: if effective_additional_permissions.permissions_preapproved { codex_protocol::models::SandboxPermissions::UseDefault diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs index b850a36b59..d0e4d0919c 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -34,7 +34,6 @@ use codex_protocol::exec_output::ExecToolCallOutput; use codex_protocol::exec_output::StreamOutput; use codex_protocol::models::AdditionalPermissionProfile; use codex_protocol::models::PermissionProfile; -use codex_protocol::models::SandboxEnforcement; use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::AskForApproval; @@ -206,9 +205,8 @@ pub(super) async fn try_run_zsh_fork( call_id: ctx.call_id.clone(), tool_name: GuardianCommandSource::Shell, approval_policy: ctx.turn.approval_policy.value(), - sandbox_policy: command_executor.sandbox_policy.clone(), + permission_profile: command_executor.permission_profile.clone(), file_system_sandbox_policy: command_executor.file_system_sandbox_policy.clone(), - network_sandbox_policy: command_executor.network_sandbox_policy, sandbox_permissions: req.sandbox_permissions, approval_sandbox_permissions, prompt_permissions: req.additional_permissions.clone(), @@ -279,9 +277,8 @@ pub(crate) async fn prepare_unified_exec_zsh_fork( call_id: ctx.call_id.clone(), tool_name: GuardianCommandSource::UnifiedExec, approval_policy: ctx.turn.approval_policy.value(), - sandbox_policy: exec_request.sandbox_policy.clone(), + permission_profile: exec_request.permission_profile.clone(), file_system_sandbox_policy: exec_request.file_system_sandbox_policy.clone(), - network_sandbox_policy: exec_request.network_sandbox_policy, sandbox_permissions: req.sandbox_permissions, approval_sandbox_permissions: approval_sandbox_permissions( req.sandbox_permissions, @@ -314,9 +311,8 @@ struct CoreShellActionProvider { call_id: String, tool_name: GuardianCommandSource, approval_policy: AskForApproval, - sandbox_policy: SandboxPolicy, + permission_profile: PermissionProfile, file_system_sandbox_policy: FileSystemSandboxPolicy, - network_sandbox_policy: NetworkSandboxPolicy, sandbox_permissions: SandboxPermissions, approval_sandbox_permissions: SandboxPermissions, prompt_permissions: Option, @@ -366,9 +362,7 @@ impl CoreShellActionProvider { fn shell_request_escalation_execution( sandbox_permissions: SandboxPermissions, - sandbox_policy: &SandboxPolicy, - file_system_sandbox_policy: &FileSystemSandboxPolicy, - network_sandbox_policy: NetworkSandboxPolicy, + permission_profile: &PermissionProfile, additional_permissions: Option<&AdditionalPermissionProfile>, ) -> EscalationExecution { match sandbox_permissions { @@ -381,15 +375,7 @@ impl CoreShellActionProvider { EscalationExecution::Permissions( EscalationPermissions::ResolvedPermissionProfile( ResolvedPermissionProfile { - permission_profile: - PermissionProfile::from_runtime_permissions_with_enforcement( - SandboxEnforcement::from_legacy_sandbox_policy( - sandbox_policy, - ), - file_system_sandbox_policy, - network_sandbox_policy, - ), - sandbox_policy: sandbox_policy.clone(), + permission_profile: permission_profile.clone(), }, ), ) @@ -608,7 +594,7 @@ impl EscalationPolicy for CoreShellActionProvider { argv, InterceptedExecPolicyContext { approval_policy: self.approval_policy, - sandbox_policy: &self.sandbox_policy, + permission_profile: self.permission_profile.clone(), file_system_sandbox_policy: &self.file_system_sandbox_policy, sandbox_permissions: self.approval_sandbox_permissions, enable_shell_wrapper_parsing: @@ -632,9 +618,7 @@ impl EscalationPolicy for CoreShellActionProvider { DecisionSource::PrefixRule => EscalationExecution::Unsandboxed, DecisionSource::UnmatchedCommandFallback => Self::shell_request_escalation_execution( self.sandbox_permissions, - &self.sandbox_policy, - &self.file_system_sandbox_policy, - self.network_sandbox_policy, + &self.permission_profile, self.prompt_permissions.as_ref(), ), }; @@ -660,7 +644,7 @@ fn evaluate_intercepted_exec_policy( ) -> Evaluation { let InterceptedExecPolicyContext { approval_policy, - sandbox_policy, + permission_profile, file_system_sandbox_policy, sandbox_permissions, enable_shell_wrapper_parsing, @@ -685,7 +669,7 @@ fn evaluate_intercepted_exec_policy( let fallback = |cmd: &[String]| { crate::exec_policy::render_decision_for_unmatched_command( approval_policy, - sandbox_policy, + &permission_profile, file_system_sandbox_policy, cmd, sandbox_permissions, @@ -702,10 +686,10 @@ fn evaluate_intercepted_exec_policy( ) } -#[derive(Clone, Copy)] +#[derive(Clone)] struct InterceptedExecPolicyContext<'a> { approval_policy: AskForApproval, - sandbox_policy: &'a SandboxPolicy, + permission_profile: PermissionProfile, file_system_sandbox_policy: &'a FileSystemSandboxPolicy, sandbox_permissions: SandboxPermissions, enable_shell_wrapper_parsing: bool, diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs index b02ad08775..192ae46031 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs @@ -66,6 +66,10 @@ fn read_only_file_system_sandbox_policy() -> FileSystemSandboxPolicy { }]) } +fn permission_profile_from_sandbox_policy(sandbox_policy: &SandboxPolicy) -> PermissionProfile { + PermissionProfile::from_legacy_sandbox_policy(sandbox_policy) +} + #[test] fn execve_prompt_rejection_keeps_prefix_rules_on_rules_flag() { assert_eq!( @@ -266,12 +270,6 @@ fn shell_request_escalation_execution_is_explicit() { )), ..Default::default() }; - let sandbox_policy = SandboxPolicy::WorkspaceWrite { - writable_roots: vec![AbsolutePathBuf::from_absolute_path("/tmp/original/output").unwrap()], - network_access: false, - exclude_tmpdir_env_var: false, - exclude_slash_tmp: false, - }; let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { path: FileSystemPath::Path { @@ -287,13 +285,15 @@ fn shell_request_escalation_execution_is_explicit() { }, ]); let network_sandbox_policy = NetworkSandboxPolicy::Restricted; + let permission_profile = PermissionProfile::from_runtime_permissions( + &file_system_sandbox_policy, + network_sandbox_policy, + ); assert_eq!( CoreShellActionProvider::shell_request_escalation_execution( crate::sandboxing::SandboxPermissions::UseDefault, - &sandbox_policy, - &file_system_sandbox_policy, - network_sandbox_policy, + &permission_profile, /*additional_permissions*/ None, ), EscalationExecution::TurnDefault, @@ -301,9 +301,7 @@ fn shell_request_escalation_execution_is_explicit() { assert_eq!( CoreShellActionProvider::shell_request_escalation_execution( crate::sandboxing::SandboxPermissions::RequireEscalated, - &sandbox_policy, - &file_system_sandbox_policy, - network_sandbox_policy, + &permission_profile, /*additional_permissions*/ None, ), EscalationExecution::Unsandboxed, @@ -311,19 +309,11 @@ fn shell_request_escalation_execution_is_explicit() { assert_eq!( CoreShellActionProvider::shell_request_escalation_execution( crate::sandboxing::SandboxPermissions::WithAdditionalPermissions, - &sandbox_policy, - &file_system_sandbox_policy, - network_sandbox_policy, + &permission_profile, Some(&requested_permissions), ), EscalationExecution::Permissions(EscalationPermissions::ResolvedPermissionProfile( - ResolvedPermissionProfile { - permission_profile: PermissionProfile::from_runtime_permissions( - &file_system_sandbox_policy, - network_sandbox_policy, - ), - sandbox_policy, - }, + ResolvedPermissionProfile { permission_profile }, )), ); } @@ -395,8 +385,6 @@ async fn execve_permission_request_hook_short_circuits_prompt() -> anyhow::Resul &read_only_file_system_sandbox_policy(), NetworkSandboxPolicy::Restricted, ); - let sandbox_policy = SandboxPolicy::new_read_only_policy(); - let workdir = AbsolutePathBuf::try_from(std::env::current_dir()?)?; let target = std::env::temp_dir().join("execve-hook-short-circuit.txt"); let target_str = target.display().to_string(); @@ -410,9 +398,10 @@ async fn execve_permission_request_hook_short_circuits_prompt() -> anyhow::Resul call_id: "execve-hook-call".to_string(), tool_name: GuardianCommandSource::Shell, approval_policy: AskForApproval::OnRequest, - sandbox_policy, + permission_profile: permission_profile_from_sandbox_policy( + &SandboxPolicy::new_read_only_policy(), + ), file_system_sandbox_policy: read_only_file_system_sandbox_policy(), - network_sandbox_policy: NetworkSandboxPolicy::Restricted, sandbox_permissions: SandboxPermissions::RequireEscalated, approval_sandbox_permissions: SandboxPermissions::RequireEscalated, prompt_permissions: None, @@ -476,7 +465,9 @@ fn evaluate_intercepted_exec_policy_uses_wrapper_command_when_shell_wrapper_pars ], InterceptedExecPolicyContext { approval_policy: AskForApproval::OnRequest, - sandbox_policy: &SandboxPolicy::new_read_only_policy(), + permission_profile: permission_profile_from_sandbox_policy( + &SandboxPolicy::new_read_only_policy(), + ), file_system_sandbox_policy: &read_only_file_system_sandbox_policy(), sandbox_permissions: SandboxPermissions::UseDefault, enable_shell_wrapper_parsing: enable_intercepted_exec_policy_shell_wrapper_parsing, @@ -527,7 +518,9 @@ fn evaluate_intercepted_exec_policy_matches_inner_shell_commands_when_enabled() ], InterceptedExecPolicyContext { approval_policy: AskForApproval::OnRequest, - sandbox_policy: &SandboxPolicy::new_read_only_policy(), + permission_profile: permission_profile_from_sandbox_policy( + &SandboxPolicy::new_read_only_policy(), + ), file_system_sandbox_policy: &read_only_file_system_sandbox_policy(), sandbox_permissions: SandboxPermissions::UseDefault, enable_shell_wrapper_parsing: enable_intercepted_exec_policy_shell_wrapper_parsing, @@ -569,7 +562,9 @@ host_executable(name = "git", paths = ["{git_path_literal}"]) &["git".to_string(), "status".to_string()], InterceptedExecPolicyContext { approval_policy: AskForApproval::OnRequest, - sandbox_policy: &SandboxPolicy::new_read_only_policy(), + permission_profile: permission_profile_from_sandbox_policy( + &SandboxPolicy::new_read_only_policy(), + ), file_system_sandbox_policy: &read_only_file_system_sandbox_policy(), sandbox_permissions: SandboxPermissions::UseDefault, enable_shell_wrapper_parsing: false, @@ -609,7 +604,7 @@ fn intercepted_exec_policy_treats_preapproved_additional_permissions_as_default( &argv, InterceptedExecPolicyContext { approval_policy, - sandbox_policy: &sandbox_policy, + permission_profile: permission_profile_from_sandbox_policy(&sandbox_policy), file_system_sandbox_policy: &file_system_sandbox_policy, sandbox_permissions: super::approval_sandbox_permissions( SandboxPermissions::WithAdditionalPermissions, @@ -624,7 +619,7 @@ fn intercepted_exec_policy_treats_preapproved_additional_permissions_as_default( &argv, InterceptedExecPolicyContext { approval_policy, - sandbox_policy: &sandbox_policy, + permission_profile: permission_profile_from_sandbox_policy(&sandbox_policy), file_system_sandbox_policy: &file_system_sandbox_policy, sandbox_permissions: SandboxPermissions::WithAdditionalPermissions, enable_shell_wrapper_parsing: false, @@ -657,7 +652,9 @@ host_executable(name = "git", paths = ["{allowed_git_literal}"]) &["git".to_string(), "status".to_string()], InterceptedExecPolicyContext { approval_policy: AskForApproval::OnRequest, - sandbox_policy: &SandboxPolicy::new_read_only_policy(), + permission_profile: permission_profile_from_sandbox_policy( + &SandboxPolicy::new_read_only_policy(), + ), file_system_sandbox_policy: &read_only_file_system_sandbox_policy(), sandbox_permissions: SandboxPermissions::UseDefault, enable_shell_wrapper_parsing: false, diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index bd4452ce1a..b2c83a363f 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -789,7 +789,6 @@ impl UnifiedExecProcessManager { context.turn.tools_config.unified_exec_shell_mode.clone(), ); let file_system_sandbox_policy = context.turn.file_system_sandbox_policy(); - let sandbox_policy = context.turn.sandbox_policy(); let exec_approval_requirement = context .session .services @@ -797,7 +796,7 @@ impl UnifiedExecProcessManager { .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &request.command, approval_policy: context.turn.approval_policy.value(), - sandbox_policy: &sandbox_policy, + permission_profile: context.turn.permission_profile(), file_system_sandbox_policy: &file_system_sandbox_policy, sandbox_permissions: if request.additional_permissions_preapproved { crate::sandboxing::SandboxPermissions::UseDefault diff --git a/codex-rs/protocol/src/approvals.rs b/codex-rs/protocol/src/approvals.rs index 6fc5e49b49..73283e3eb6 100644 --- a/codex-rs/protocol/src/approvals.rs +++ b/codex-rs/protocol/src/approvals.rs @@ -4,7 +4,6 @@ use crate::models::PermissionProfile; use crate::parse_command::ParsedCommand; use crate::protocol::FileChange; use crate::protocol::ReviewDecision; -use crate::protocol::SandboxPolicy; use crate::request_permissions::RequestPermissionProfile; use codex_utils_absolute_path::AbsolutePathBuf; use schemars::JsonSchema; @@ -16,13 +15,9 @@ use std::path::PathBuf; use ts_rs::TS; /// Fully resolved permissions for rerunning an intercepted child process. -/// -/// `permission_profile` is the canonical permission model. `sandbox_policy` -/// remains as the legacy adapter for sandbox backends that still require it. #[derive(Debug, Clone, PartialEq, Eq)] pub struct ResolvedPermissionProfile { pub permission_profile: PermissionProfile, - pub sandbox_policy: SandboxPolicy, } #[allow(clippy::large_enum_variant)] diff --git a/codex-rs/sandboxing/src/policy_transforms.rs b/codex-rs/sandboxing/src/policy_transforms.rs index 20efb4b900..fc352865a8 100644 --- a/codex-rs/sandboxing/src/policy_transforms.rs +++ b/codex-rs/sandboxing/src/policy_transforms.rs @@ -10,37 +10,12 @@ use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::permissions::ReadDenyMatcher; -use codex_protocol::protocol::NetworkAccess; -use codex_protocol::protocol::SandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::canonicalize_preserving_symlinks; -use std::collections::HashSet; use std::num::NonZeroUsize; use std::path::Path; use std::path::PathBuf; -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct EffectiveSandboxPermissions { - pub sandbox_policy: SandboxPolicy, -} - -impl EffectiveSandboxPermissions { - pub fn new( - sandbox_policy: &SandboxPolicy, - additional_permissions: Option<&AdditionalPermissionProfile>, - ) -> Self { - let Some(additional_permissions) = additional_permissions else { - return Self { - sandbox_policy: sandbox_policy.clone(), - }; - }; - - Self { - sandbox_policy: effective_sandbox_policy(sandbox_policy, Some(additional_permissions)), - } - } -} - pub fn normalize_additional_permissions( additional_permissions: AdditionalPermissionProfile, ) -> Result { @@ -446,48 +421,6 @@ fn merge_permission_entries( merged } -fn dedup_absolute_paths(paths: Vec) -> Vec { - let mut out = Vec::with_capacity(paths.len()); - let mut seen = HashSet::new(); - for path in paths { - if seen.insert(path.to_path_buf()) { - out.push(path); - } - } - out -} - -fn additional_permission_roots( - additional_permissions: &AdditionalPermissionProfile, -) -> (Vec, Vec) { - ( - dedup_absolute_paths( - additional_permissions - .file_system - .as_ref() - .map(|file_system| { - file_system - .explicit_path_entries() - .filter_map(|(path, access)| access.can_read().then_some(path.clone())) - .collect() - }) - .unwrap_or_default(), - ), - dedup_absolute_paths( - additional_permissions - .file_system - .as_ref() - .map(|file_system| { - file_system - .explicit_path_entries() - .filter_map(|(path, access)| access.can_write().then_some(path.clone())) - .collect() - }) - .unwrap_or_default(), - ), - ) -} - fn merge_file_system_policy_with_additional_permissions( file_system_policy: &FileSystemSandboxPolicy, additional_permissions: &FileSystemPermissions, @@ -578,73 +511,6 @@ pub fn effective_permission_profile( ) } -fn sandbox_policy_with_additional_permissions( - sandbox_policy: &SandboxPolicy, - additional_permissions: &AdditionalPermissionProfile, -) -> SandboxPolicy { - if additional_permissions.is_empty() { - return sandbox_policy.clone(); - } - - let (_extra_reads, extra_writes) = additional_permission_roots(additional_permissions); - - match sandbox_policy { - SandboxPolicy::DangerFullAccess => SandboxPolicy::DangerFullAccess, - SandboxPolicy::ExternalSandbox { network_access } => SandboxPolicy::ExternalSandbox { - network_access: if merge_network_access( - network_access.is_enabled(), - additional_permissions, - ) { - NetworkAccess::Enabled - } else { - NetworkAccess::Restricted - }, - }, - SandboxPolicy::WorkspaceWrite { - writable_roots, - network_access, - exclude_tmpdir_env_var, - exclude_slash_tmp, - } => { - let mut merged_writes = writable_roots.clone(); - merged_writes.extend(extra_writes); - SandboxPolicy::WorkspaceWrite { - writable_roots: dedup_absolute_paths(merged_writes), - network_access: merge_network_access(*network_access, additional_permissions), - exclude_tmpdir_env_var: *exclude_tmpdir_env_var, - exclude_slash_tmp: *exclude_slash_tmp, - } - } - SandboxPolicy::ReadOnly { network_access } => { - if extra_writes.is_empty() { - SandboxPolicy::ReadOnly { - network_access: merge_network_access(*network_access, additional_permissions), - } - } else { - // todo(dylan) - for now, this grants more access than the request. We should restrict this, - // but we should add a new SandboxPolicy variant to handle this. While the feature is still - // UnderDevelopment, it's a useful approximation of the desired behavior. - SandboxPolicy::WorkspaceWrite { - writable_roots: dedup_absolute_paths(extra_writes), - network_access: merge_network_access(*network_access, additional_permissions), - exclude_tmpdir_env_var: false, - exclude_slash_tmp: false, - } - } - } - } -} - -fn effective_sandbox_policy( - sandbox_policy: &SandboxPolicy, - additional_permissions: Option<&AdditionalPermissionProfile>, -) -> SandboxPolicy { - additional_permissions.map_or_else( - || sandbox_policy.clone(), - |permissions| sandbox_policy_with_additional_permissions(sandbox_policy, permissions), - ) -} - pub fn should_require_platform_sandbox( file_system_policy: &FileSystemSandboxPolicy, network_policy: NetworkSandboxPolicy, diff --git a/codex-rs/sandboxing/src/policy_transforms_tests.rs b/codex-rs/sandboxing/src/policy_transforms_tests.rs index 2894b29bb1..9b41205735 100644 --- a/codex-rs/sandboxing/src/policy_transforms_tests.rs +++ b/codex-rs/sandboxing/src/policy_transforms_tests.rs @@ -2,7 +2,6 @@ use super::effective_file_system_sandbox_policy; use super::intersect_permission_profiles; use super::merge_file_system_policy_with_additional_permissions; use super::normalize_additional_permissions; -use super::sandbox_policy_with_additional_permissions; use super::should_require_platform_sandbox; use codex_protocol::models::AdditionalPermissionProfile as PermissionProfile; use codex_protocol::models::FileSystemPermissions; @@ -13,8 +12,6 @@ use codex_protocol::permissions::FileSystemSandboxEntry; use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::permissions::NetworkSandboxPolicy; -use codex_protocol::protocol::NetworkAccess; -use codex_protocol::protocol::SandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; use dunce::canonicalize; use pretty_assertions::assert_eq; @@ -757,66 +754,6 @@ fn intersect_permission_profiles_uses_granted_unbounded_glob_scan_depth() { ); } -#[test] -fn read_only_additional_permissions_can_enable_network_without_writes() { - let temp_dir = TempDir::new().expect("create temp dir"); - let path = AbsolutePathBuf::from_absolute_path( - canonicalize(temp_dir.path()).expect("canonicalize temp dir"), - ) - .expect("absolute temp dir"); - let policy = sandbox_policy_with_additional_permissions( - &SandboxPolicy::ReadOnly { - network_access: false, - }, - &PermissionProfile { - network: Some(NetworkPermissions { - enabled: Some(true), - }), - file_system: Some(FileSystemPermissions::from_read_write_roots( - Some(vec![path]), - Some(Vec::new()), - )), - }, - ); - - assert_eq!( - policy, - SandboxPolicy::ReadOnly { - network_access: true, - } - ); -} - -#[test] -fn external_sandbox_additional_permissions_can_enable_network() { - let temp_dir = TempDir::new().expect("create temp dir"); - let path = AbsolutePathBuf::from_absolute_path( - canonicalize(temp_dir.path()).expect("canonicalize temp dir"), - ) - .expect("absolute temp dir"); - let policy = sandbox_policy_with_additional_permissions( - &SandboxPolicy::ExternalSandbox { - network_access: NetworkAccess::Restricted, - }, - &PermissionProfile { - network: Some(NetworkPermissions { - enabled: Some(true), - }), - file_system: Some(FileSystemPermissions::from_read_write_roots( - Some(vec![path]), - Some(Vec::new()), - )), - }, - ); - - assert_eq!( - policy, - SandboxPolicy::ExternalSandbox { - network_access: NetworkAccess::Enabled, - } - ); -} - #[test] fn merge_file_system_policy_with_additional_permissions_preserves_unreadable_roots() { let temp_dir = TempDir::new().expect("create temp dir");