From 0a83353ca3f7889efbf322b74476c8ebba0a2d36 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Sun, 17 May 2026 08:39:41 -0700 Subject: [PATCH] test: reduce core sandbox policy test setup (#23036) ## Why `SandboxPolicy` is a legacy compatibility shape, but several core tests still used it for ordinary turn setup even when the runtime path now carries `PermissionProfile`. With the first cleanup PR merged, this follow-up trims more core test scaffolding so remaining `SandboxPolicy` matches are easier to classify as production compatibility, legacy-boundary coverage, or explicit conversion tests. ## What Changed - Updated apply-patch handler and runtime tests to pass `PermissionProfile` directly. - Changed sandboxing test helpers to build permission profiles without first creating `SandboxPolicy` values. - Converted request-permissions integration turns to pass `PermissionProfile` through the test helper, leaving legacy sandbox projection at the `Op::UserTurn` boundary. - Converted unified exec integration helpers and direct turn submissions to use `PermissionProfile` values instead of `SandboxPolicy` setup. - Removed now-unused `SandboxPolicy` imports from the touched core tests. ## Test Plan - `just fmt` - `cargo test -p codex-core --lib tools::sandboxing::tests` - `cargo test -p codex-core --lib tools::runtimes::apply_patch::tests` - `cargo test -p codex-core --lib tools::handlers::apply_patch::tests` - `cargo test -p codex-core --lib unified_exec::process_manager::tests` - `cargo test -p codex-core --test all request_permissions::` - `cargo test -p codex-core --test all unified_exec::` - `just fix -p codex-core` --- .../src/tools/handlers/apply_patch_tests.rs | 23 ++-- .../src/tools/runtimes/apply_patch_tests.rs | 4 +- codex-rs/core/src/tools/sandboxing.rs | 2 - codex-rs/core/src/tools/sandboxing_tests.rs | 15 +- .../src/unified_exec/process_manager_tests.rs | 10 +- .../core/tests/suite/request_permissions.rs | 97 ++++++------- codex-rs/core/tests/suite/unified_exec.rs | 130 +++++++++--------- 7 files changed, 127 insertions(+), 154 deletions(-) diff --git a/codex-rs/core/src/tools/handlers/apply_patch_tests.rs b/codex-rs/core/src/tools/handlers/apply_patch_tests.rs index 99854d23ea..e9a118641a 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch_tests.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch_tests.rs @@ -3,7 +3,6 @@ use codex_apply_patch::MaybeApplyPatchVerified; use codex_exec_server::LOCAL_FS; use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::protocol::FileChange; -use codex_protocol::protocol::SandboxPolicy; use core_test_support::PathBufExt; use core_test_support::PathExt; use pretty_assertions::assert_eq; @@ -251,12 +250,11 @@ fn write_permissions_for_paths_skip_dirs_already_writable_under_workspace_root() std::fs::create_dir_all(&nested).expect("create nested dir"); let file_path = AbsolutePathBuf::try_from(nested.join("file.txt")) .expect("nested file path should be absolute"); - let sandbox_policy = FileSystemSandboxPolicy::from(&SandboxPolicy::WorkspaceWrite { - writable_roots: vec![], - network_access: false, - exclude_tmpdir_env_var: true, - exclude_slash_tmp: false, - }); + let sandbox_policy = FileSystemSandboxPolicy::workspace_write( + &[], + /*exclude_tmpdir_env_var*/ true, + /*exclude_slash_tmp*/ false, + ); let permissions = write_permissions_for_paths(&[file_path], &sandbox_policy, &cwd); @@ -273,12 +271,11 @@ fn write_permissions_for_paths_keep_dirs_outside_workspace_root() { let file_path = AbsolutePathBuf::try_from(outside.join("file.txt")) .expect("outside file path should be absolute"); let cwd_abs = cwd.abs(); - let sandbox_policy = FileSystemSandboxPolicy::from(&SandboxPolicy::WorkspaceWrite { - writable_roots: vec![], - network_access: false, - exclude_tmpdir_env_var: true, - exclude_slash_tmp: true, - }); + let sandbox_policy = FileSystemSandboxPolicy::workspace_write( + &[], + /*exclude_tmpdir_env_var*/ true, + /*exclude_slash_tmp*/ true, + ); let permissions = write_permissions_for_paths(&[file_path], &sandbox_policy, &cwd_abs); let expected_outside = diff --git a/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs b/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs index ff2c3d1e89..f4d8d461b2 100644 --- a/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs +++ b/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs @@ -7,7 +7,6 @@ use codex_protocol::models::PermissionProfile; use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::GranularApprovalConfig; -use codex_protocol::protocol::SandboxPolicy; use codex_sandboxing::SandboxManager; use codex_sandboxing::SandboxType; use codex_sandboxing::policy_transforms::effective_file_system_sandbox_policy; @@ -201,8 +200,7 @@ async fn file_system_sandbox_context_uses_active_attempt() { additional_permissions: Some(additional_permissions.clone()), permissions_preapproved: false, }; - let sandbox_policy = SandboxPolicy::new_read_only_policy(); - let file_system_policy = FileSystemSandboxPolicy::from(&sandbox_policy); + let file_system_policy = FileSystemSandboxPolicy::default(); let permissions = PermissionProfile::from_runtime_permissions( &file_system_policy, NetworkSandboxPolicy::Restricted, diff --git a/codex-rs/core/src/tools/sandboxing.rs b/codex-rs/core/src/tools/sandboxing.rs index 656615d4d1..1ca589a5f9 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -19,8 +19,6 @@ use codex_protocol::permissions::FileSystemSandboxKind; use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::ReviewDecision; -#[cfg(test)] -use codex_protocol::protocol::SandboxPolicy; use codex_sandboxing::SandboxCommand; use codex_sandboxing::SandboxManager; use codex_sandboxing::SandboxTransformError; diff --git a/codex-rs/core/src/tools/sandboxing_tests.rs b/codex-rs/core/src/tools/sandboxing_tests.rs index 3989862ab5..99b43699a9 100644 --- a/codex-rs/core/src/tools/sandboxing_tests.rs +++ b/codex-rs/core/src/tools/sandboxing_tests.rs @@ -5,7 +5,6 @@ use codex_protocol::permissions::FileSystemAccessMode; use codex_protocol::permissions::FileSystemPath; use codex_protocol::permissions::FileSystemSandboxEntry; use codex_protocol::protocol::GranularApprovalConfig; -use codex_protocol::protocol::NetworkAccess; use pretty_assertions::assert_eq; use serde_json::json; @@ -39,13 +38,10 @@ fn bash_permission_request_payload_includes_description_when_present() { #[test] fn external_sandbox_skips_exec_approval_on_request() { - let sandbox_policy = SandboxPolicy::ExternalSandbox { - network_access: NetworkAccess::Restricted, - }; assert_eq!( default_exec_approval_requirement( AskForApproval::OnRequest, - &FileSystemSandboxPolicy::from(&sandbox_policy), + &FileSystemSandboxPolicy::external_sandbox(), ), ExecApprovalRequirement::Skip { bypass_sandbox: false, @@ -56,11 +52,10 @@ fn external_sandbox_skips_exec_approval_on_request() { #[test] fn restricted_sandbox_requires_exec_approval_on_request() { - let sandbox_policy = SandboxPolicy::new_read_only_policy(); assert_eq!( default_exec_approval_requirement( AskForApproval::OnRequest, - &FileSystemSandboxPolicy::from(&sandbox_policy) + &FileSystemSandboxPolicy::default() ), ExecApprovalRequirement::NeedsApproval { reason: None, @@ -79,9 +74,8 @@ fn default_exec_approval_requirement_rejects_sandbox_prompt_when_granular_disabl mcp_elicitations: true, }); - let sandbox_policy = SandboxPolicy::new_read_only_policy(); let requirement = - default_exec_approval_requirement(policy, &FileSystemSandboxPolicy::from(&sandbox_policy)); + default_exec_approval_requirement(policy, &FileSystemSandboxPolicy::default()); assert_eq!( requirement, @@ -101,9 +95,8 @@ fn default_exec_approval_requirement_keeps_prompt_when_granular_allows_sandbox_a mcp_elicitations: false, }); - let sandbox_policy = SandboxPolicy::new_read_only_policy(); let requirement = - default_exec_approval_requirement(policy, &FileSystemSandboxPolicy::from(&sandbox_policy)); + default_exec_approval_requirement(policy, &FileSystemSandboxPolicy::default()); assert_eq!( requirement, diff --git a/codex-rs/core/src/unified_exec/process_manager_tests.rs b/codex-rs/core/src/unified_exec/process_manager_tests.rs index c554a2a4e4..cd83ce7a9c 100644 --- a/codex-rs/core/src/unified_exec/process_manager_tests.rs +++ b/codex-rs/core/src/unified_exec/process_manager_tests.rs @@ -71,16 +71,10 @@ fn exec_server_params_use_env_policy_overlay_contract() { .expect("current dir") .try_into() .expect("absolute path"); - let sandbox_policy = codex_protocol::protocol::SandboxPolicy::DangerFullAccess; let file_system_sandbox_policy = - codex_protocol::permissions::FileSystemSandboxPolicy::from(&sandbox_policy); + codex_protocol::permissions::FileSystemSandboxPolicy::unrestricted(); let network_sandbox_policy = codex_protocol::permissions::NetworkSandboxPolicy::Restricted; - let permission_profile = - codex_protocol::models::PermissionProfile::from_runtime_permissions_with_enforcement( - codex_protocol::models::SandboxEnforcement::from_legacy_sandbox_policy(&sandbox_policy), - &file_system_sandbox_policy, - network_sandbox_policy, - ); + let permission_profile = codex_protocol::models::PermissionProfile::Disabled; let request = ExecRequest { command: vec!["bash".to_string(), "-lc".to_string(), "true".to_string()], cwd: cwd.clone(), diff --git a/codex-rs/core/tests/suite/request_permissions.rs b/codex-rs/core/tests/suite/request_permissions.rs index 0463ea3e2b..cf021f29e4 100644 --- a/codex-rs/core/tests/suite/request_permissions.rs +++ b/codex-rs/core/tests/suite/request_permissions.rs @@ -15,7 +15,6 @@ use codex_protocol::protocol::ExecApprovalRequestEvent; use codex_protocol::protocol::GranularApprovalConfig; use codex_protocol::protocol::Op; use codex_protocol::protocol::ReviewDecision; -use codex_protocol::protocol::SandboxPolicy; use codex_protocol::request_permissions::PermissionGrantScope; use codex_protocol::request_permissions::RequestPermissionProfile; use codex_protocol::request_permissions::RequestPermissionsResponse; @@ -33,6 +32,7 @@ use core_test_support::skip_if_no_network; use core_test_support::skip_if_sandbox; use core_test_support::test_codex::TestCodex; use core_test_support::test_codex::test_codex; +use core_test_support::test_codex::turn_permission_fields; use core_test_support::wait_for_event; use pretty_assertions::assert_eq; use regex_lite::Regex; @@ -184,9 +184,11 @@ async fn submit_turn( test: &TestCodex, prompt: &str, approval_policy: AskForApproval, - sandbox_policy: SandboxPolicy, + permission_profile: CorePermissionProfile, ) -> Result<()> { let session_model = test.session_configured.model.clone(); + let (sandbox_policy, permission_profile) = + turn_permission_fields(permission_profile, test.cwd.path()); test.codex .submit(Op::UserTurn { environments: None, @@ -199,7 +201,7 @@ async fn submit_turn( approval_policy, approvals_reviewer: Some(ApprovalsReviewer::User), sandbox_policy, - permission_profile: None, + permission_profile, model: session_model, effort: None, summary: None, @@ -285,16 +287,7 @@ async fn expect_request_permissions_event( } } -fn workspace_write_excluding_tmp() -> SandboxPolicy { - SandboxPolicy::WorkspaceWrite { - writable_roots: vec![], - network_access: false, - exclude_tmpdir_env_var: true, - exclude_slash_tmp: true, - } -} - -fn workspace_write_excluding_tmp_profile() -> CorePermissionProfile { +fn workspace_write_excluding_tmp() -> CorePermissionProfile { CorePermissionProfile::workspace_write_with( &[], NetworkSandboxPolicy::Restricted, @@ -330,7 +323,7 @@ async fn with_additional_permissions_requires_approval_under_on_request() -> Res let server = start_mock_server().await; let approval_policy = AskForApproval::OnRequest; - let sandbox_policy = SandboxPolicy::new_read_only_policy(); + let permission_profile = CorePermissionProfile::read_only(); let permission_profile_for_config = CorePermissionProfile::read_only(); let mut builder = test_codex().with_config(move |config| { @@ -384,7 +377,7 @@ async fn with_additional_permissions_requires_approval_under_on_request() -> Res ) .await; - submit_turn(&test, call_id, approval_policy, sandbox_policy.clone()).await?; + submit_turn(&test, call_id, approval_policy, permission_profile.clone()).await?; let approval = expect_exec_approval(&test, command).await; assert_eq!( approval.additional_permissions, @@ -428,7 +421,7 @@ async fn request_permissions_tool_is_auto_denied_when_granular_request_permissio request_permissions: false, mcp_elicitations: true, }); - let sandbox_policy = SandboxPolicy::new_read_only_policy(); + let permission_profile = CorePermissionProfile::read_only(); let permission_profile_for_config = CorePermissionProfile::read_only(); let mut builder = test_codex().with_config(move |config| { @@ -476,7 +469,7 @@ async fn request_permissions_tool_is_auto_denied_when_granular_request_permissio &test, "request permissions under granular.request_permissions = false", approval_policy, - sandbox_policy, + permission_profile, ) .await?; @@ -514,7 +507,7 @@ async fn relative_additional_permissions_resolve_against_tool_workdir() -> Resul let server = start_mock_server().await; let approval_policy = AskForApproval::OnRequest; - let sandbox_policy = SandboxPolicy::new_read_only_policy(); + let permission_profile = CorePermissionProfile::read_only(); let permission_profile_for_config = CorePermissionProfile::read_only(); let mut builder = test_codex().with_config(move |config| { @@ -578,7 +571,7 @@ async fn relative_additional_permissions_resolve_against_tool_workdir() -> Resul ) .await; - submit_turn(&test, call_id, approval_policy, sandbox_policy.clone()).await?; + submit_turn(&test, call_id, approval_policy, permission_profile.clone()).await?; let approval = expect_exec_approval(&test, command).await; assert_eq!( @@ -618,7 +611,7 @@ async fn read_only_with_additional_permissions_does_not_widen_to_unrequested_cwd let server = start_mock_server().await; let approval_policy = AskForApproval::OnRequest; - let sandbox_policy = SandboxPolicy::new_read_only_policy(); + let permission_profile = CorePermissionProfile::read_only(); let permission_profile_for_config = CorePermissionProfile::read_only(); let mut builder = test_codex().with_config(move |config| { @@ -675,7 +668,7 @@ async fn read_only_with_additional_permissions_does_not_widen_to_unrequested_cwd ) .await; - submit_turn(&test, call_id, approval_policy, sandbox_policy.clone()).await?; + submit_turn(&test, call_id, approval_policy, permission_profile.clone()).await?; let approval = expect_exec_approval(&test, &command).await; assert_eq!( @@ -721,7 +714,7 @@ async fn read_only_with_additional_permissions_does_not_widen_to_unrequested_tmp let server = start_mock_server().await; let approval_policy = AskForApproval::OnRequest; - let sandbox_policy = SandboxPolicy::new_read_only_policy(); + let permission_profile = CorePermissionProfile::read_only(); let permission_profile_for_config = CorePermissionProfile::read_only(); let mut builder = test_codex().with_config(move |config| { @@ -779,7 +772,7 @@ async fn read_only_with_additional_permissions_does_not_widen_to_unrequested_tmp ) .await; - submit_turn(&test, call_id, approval_policy, sandbox_policy.clone()).await?; + submit_turn(&test, call_id, approval_policy, permission_profile.clone()).await?; let approval = expect_exec_approval(&test, &command).await; assert_eq!( @@ -823,8 +816,8 @@ async fn workspace_write_with_additional_permissions_can_write_outside_cwd() -> let server = start_mock_server().await; let approval_policy = AskForApproval::OnRequest; - let sandbox_policy = workspace_write_excluding_tmp(); - let permission_profile_for_config = workspace_write_excluding_tmp_profile(); + let permission_profile = workspace_write_excluding_tmp(); + let permission_profile_for_config = workspace_write_excluding_tmp(); let mut builder = test_codex().with_config(move |config| { config.permissions.approval_policy = Constrained::allow_any(approval_policy); @@ -890,7 +883,7 @@ async fn workspace_write_with_additional_permissions_can_write_outside_cwd() -> ) .await; - submit_turn(&test, call_id, approval_policy, sandbox_policy.clone()).await?; + submit_turn(&test, call_id, approval_policy, permission_profile.clone()).await?; let approval = expect_exec_approval(&test, &command).await; assert_eq!( @@ -930,8 +923,8 @@ async fn with_additional_permissions_denied_approval_blocks_execution() -> Resul skip_if_no_network!(Ok(())); let server = start_mock_server().await; let approval_policy = AskForApproval::OnRequest; - let sandbox_policy = workspace_write_excluding_tmp(); - let permission_profile_for_config = workspace_write_excluding_tmp_profile(); + let permission_profile = workspace_write_excluding_tmp(); + let permission_profile_for_config = workspace_write_excluding_tmp(); let mut builder = test_codex().with_config(move |config| { config.permissions.approval_policy = Constrained::allow_any(approval_policy); @@ -995,7 +988,7 @@ async fn with_additional_permissions_denied_approval_blocks_execution() -> Resul ) .await; - submit_turn(&test, call_id, approval_policy, sandbox_policy.clone()).await?; + submit_turn(&test, call_id, approval_policy, permission_profile.clone()).await?; let approval = expect_exec_approval(&test, &command).await; assert_eq!( @@ -1038,8 +1031,8 @@ async fn request_permissions_grants_apply_to_later_exec_command_calls() -> Resul let server = start_mock_server().await; let approval_policy = AskForApproval::OnRequest; - let sandbox_policy = workspace_write_excluding_tmp(); - let permission_profile_for_config = workspace_write_excluding_tmp_profile(); + let permission_profile = workspace_write_excluding_tmp(); + let permission_profile_for_config = workspace_write_excluding_tmp(); let mut builder = test_codex().with_config(move |config| { config.permissions.approval_policy = Constrained::allow_any(approval_policy); @@ -1110,7 +1103,7 @@ async fn request_permissions_grants_apply_to_later_exec_command_calls() -> Resul &test, "write outside the workspace", approval_policy, - sandbox_policy, + permission_profile, ) .await?; @@ -1165,8 +1158,8 @@ async fn request_permissions_preapprove_explicit_exec_permissions_outside_on_req let server = start_mock_server().await; let approval_policy = AskForApproval::OnRequest; - let sandbox_policy = workspace_write_excluding_tmp(); - let permission_profile_for_config = workspace_write_excluding_tmp_profile(); + let permission_profile = workspace_write_excluding_tmp(); + let permission_profile_for_config = workspace_write_excluding_tmp(); let mut builder = test_codex().with_config(move |config| { config.permissions.approval_policy = Constrained::allow_any(approval_policy); @@ -1228,7 +1221,7 @@ async fn request_permissions_preapprove_explicit_exec_permissions_outside_on_req &test, "write outside the workspace", approval_policy, - sandbox_policy, + permission_profile, ) .await?; @@ -1286,8 +1279,8 @@ async fn request_permissions_grants_apply_to_later_shell_command_calls() -> Resu let server = start_mock_server().await; let approval_policy = AskForApproval::OnRequest; - let sandbox_policy = workspace_write_excluding_tmp(); - let permission_profile_for_config = workspace_write_excluding_tmp_profile(); + let permission_profile = workspace_write_excluding_tmp(); + let permission_profile_for_config = workspace_write_excluding_tmp(); let mut builder = test_codex().with_config(move |config| { config.permissions.approval_policy = Constrained::allow_any(approval_policy); @@ -1345,7 +1338,7 @@ async fn request_permissions_grants_apply_to_later_shell_command_calls() -> Resu &test, "write outside the workspace", approval_policy, - sandbox_policy, + permission_profile, ) .await?; @@ -1401,8 +1394,8 @@ async fn request_permissions_grants_apply_to_later_shell_command_calls_without_i let server = start_mock_server().await; let approval_policy = AskForApproval::OnRequest; - let sandbox_policy = workspace_write_excluding_tmp(); - let permission_profile_for_config = workspace_write_excluding_tmp_profile(); + let permission_profile = workspace_write_excluding_tmp(); + let permission_profile_for_config = workspace_write_excluding_tmp(); let mut builder = test_codex().with_config(move |config| { config.permissions.approval_policy = Constrained::allow_any(approval_policy); @@ -1458,7 +1451,7 @@ async fn request_permissions_grants_apply_to_later_shell_command_calls_without_i &test, "write outside the workspace without inline permission feature", approval_policy, - sandbox_policy, + permission_profile, ) .await?; @@ -1516,8 +1509,8 @@ async fn partial_request_permissions_grants_do_not_preapprove_new_permissions() let server = start_mock_server().await; let approval_policy = AskForApproval::OnRequest; - let sandbox_policy = workspace_write_excluding_tmp(); - let permission_profile_for_config = workspace_write_excluding_tmp_profile(); + let permission_profile = workspace_write_excluding_tmp(); + let permission_profile_for_config = workspace_write_excluding_tmp(); let mut builder = test_codex().with_config(move |config| { config.permissions.approval_policy = Constrained::allow_any(approval_policy); @@ -1611,7 +1604,7 @@ async fn partial_request_permissions_grants_do_not_preapprove_new_permissions() &test, "write outside the workspace", approval_policy, - sandbox_policy, + permission_profile, ) .await?; @@ -1683,8 +1676,8 @@ async fn request_permissions_grants_do_not_carry_across_turns() -> Result<()> { let server = start_mock_server().await; let approval_policy = AskForApproval::OnRequest; - let sandbox_policy = workspace_write_excluding_tmp(); - let permission_profile_for_config = workspace_write_excluding_tmp_profile(); + let permission_profile = workspace_write_excluding_tmp(); + let permission_profile_for_config = workspace_write_excluding_tmp(); let mut builder = test_codex().with_config(move |config| { config.permissions.approval_policy = Constrained::allow_any(approval_policy); @@ -1733,7 +1726,7 @@ async fn request_permissions_grants_do_not_carry_across_turns() -> Result<()> { &test, "request permissions for later use", approval_policy, - sandbox_policy.clone(), + permission_profile.clone(), ) .await?; @@ -1778,7 +1771,7 @@ async fn request_permissions_grants_do_not_carry_across_turns() -> Result<()> { &test, "try to reuse permissions in a later turn", approval_policy, - sandbox_policy, + permission_profile, ) .await?; wait_for_completion(&test).await; @@ -1799,8 +1792,8 @@ async fn request_permissions_session_grants_carry_across_turns() -> Result<()> { let server = start_mock_server().await; let approval_policy = AskForApproval::OnRequest; - let sandbox_policy = workspace_write_excluding_tmp(); - let permission_profile_for_config = workspace_write_excluding_tmp_profile(); + let permission_profile = workspace_write_excluding_tmp(); + let permission_profile_for_config = workspace_write_excluding_tmp(); let mut builder = test_codex().with_config(move |config| { config.permissions.approval_policy = Constrained::allow_any(approval_policy); @@ -1854,7 +1847,7 @@ async fn request_permissions_session_grants_carry_across_turns() -> Result<()> { &test, "request session permissions for later use", approval_policy, - sandbox_policy.clone(), + permission_profile.clone(), ) .await?; @@ -1896,7 +1889,7 @@ async fn request_permissions_session_grants_carry_across_turns() -> Result<()> { &test, "reuse session permissions in a later turn", approval_policy, - sandbox_policy, + permission_profile, ) .await?; diff --git a/codex-rs/core/tests/suite/unified_exec.rs b/codex-rs/core/tests/suite/unified_exec.rs index 92b8aa9047..6b962116af 100644 --- a/codex-rs/core/tests/suite/unified_exec.rs +++ b/codex-rs/core/tests/suite/unified_exec.rs @@ -15,7 +15,6 @@ use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::ExecCommandSource; use codex_protocol::protocol::ExecCommandStatus; use codex_protocol::protocol::Op; -use codex_protocol::protocol::SandboxPolicy; use codex_protocol::user_input::UserInput; use core_test_support::assert_regex_match; use core_test_support::managed_network_requirements_loader; @@ -35,6 +34,7 @@ use core_test_support::skip_if_windows; use core_test_support::test_codex::TestCodex; use core_test_support::test_codex::TestCodexHarness; use core_test_support::test_codex::test_codex; +use core_test_support::test_codex::turn_permission_fields; use core_test_support::wait_for_event; use core_test_support::wait_for_event_match; use core_test_support::wait_for_event_with_timeout; @@ -185,9 +185,11 @@ async fn wait_for_raw_unified_exec_output( async fn submit_unified_exec_turn( test: &TestCodex, prompt: &str, - sandbox_policy: SandboxPolicy, + permission_profile: PermissionProfile, ) -> Result<()> { let session_model = test.session_configured.model.clone(); + let (sandbox_policy, permission_profile) = + turn_permission_fields(permission_profile, test.config.cwd.as_path()); test.codex .submit(Op::UserTurn { @@ -201,7 +203,7 @@ async fn submit_unified_exec_turn( approval_policy: AskForApproval::Never, approvals_reviewer: None, sandbox_policy, - permission_profile: None, + permission_profile, model: session_model, effort: None, summary: None, @@ -272,6 +274,8 @@ async fn unified_exec_intercepts_apply_patch_exec_command() -> Result<()> { let codex = test.codex.clone(); let cwd = test.cwd_path().to_path_buf(); let session_model = test.session_configured.model.clone(); + let (sandbox_policy, permission_profile) = + turn_permission_fields(PermissionProfile::Disabled, &cwd); codex .submit(Op::UserTurn { @@ -284,8 +288,8 @@ async fn unified_exec_intercepts_apply_patch_exec_command() -> Result<()> { cwd, approval_policy: AskForApproval::Never, approvals_reviewer: None, - sandbox_policy: SandboxPolicy::DangerFullAccess, - permission_profile: None, + sandbox_policy, + permission_profile, model: session_model, effort: None, summary: None, @@ -403,7 +407,7 @@ async fn unified_exec_emits_exec_command_begin_event() -> Result<()> { ]; mount_sse_sequence(&server, responses).await; - submit_unified_exec_turn(&test, "emit begin event", SandboxPolicy::DangerFullAccess).await?; + submit_unified_exec_turn(&test, "emit begin event", PermissionProfile::Disabled).await?; let begin_event = wait_for_event_match(&test.codex, |msg| match msg { EventMsg::ExecCommandBegin(event) if event.call_id == call_id => Some(event.clone()), @@ -467,7 +471,7 @@ async fn unified_exec_resolves_relative_workdir() -> Result<()> { submit_unified_exec_turn( &test, "run relative workdir test", - SandboxPolicy::DangerFullAccess, + PermissionProfile::Disabled, ) .await?; @@ -532,7 +536,7 @@ async fn unified_exec_respects_workdir_override() -> Result<()> { ]; let request_log = mount_sse_sequence(&server, responses).await; - submit_unified_exec_turn(&test, "run workdir test", SandboxPolicy::DangerFullAccess).await?; + submit_unified_exec_turn(&test, "run workdir test", PermissionProfile::Disabled).await?; let begin_event = wait_for_event_match(&test.codex, |msg| match msg { EventMsg::ExecCommandBegin(event) if event.call_id == call_id => Some(event.clone()), @@ -609,7 +613,7 @@ async fn unified_exec_emits_exec_command_end_event() -> Result<()> { ]; mount_sse_sequence(&server, responses).await; - submit_unified_exec_turn(&test, "emit end event", SandboxPolicy::DangerFullAccess).await?; + submit_unified_exec_turn(&test, "emit end event", PermissionProfile::Disabled).await?; let end_event = wait_for_event_match(&test.codex, |msg| match msg { EventMsg::ExecCommandEnd(ev) if ev.call_id == call_id => Some(ev.clone()), @@ -667,7 +671,7 @@ async fn unified_exec_emits_output_delta_for_exec_command() -> Result<()> { ]; mount_sse_sequence(&server, responses).await; - submit_unified_exec_turn(&test, "emit delta", SandboxPolicy::DangerFullAccess).await?; + submit_unified_exec_turn(&test, "emit delta", PermissionProfile::Disabled).await?; let event = wait_for_event_match(&test.codex, |msg| match msg { EventMsg::ExecCommandEnd(ev) if ev.call_id == call_id => Some(ev.clone()), @@ -729,7 +733,7 @@ async fn unified_exec_full_lifecycle_with_background_end_event() -> Result<()> { submit_unified_exec_turn( &test, "exercise full unified exec lifecycle", - SandboxPolicy::DangerFullAccess, + PermissionProfile::Disabled, ) .await?; @@ -872,7 +876,7 @@ async fn unified_exec_short_lived_network_denial_emits_failed_end_event() -> Res #[allow(clippy::expect_used)] async fn unified_exec_network_denial_test( server: &wiremock::MockServer, -) -> Result<(TestCodex, SandboxPolicy)> { +) -> Result<(TestCodex, PermissionProfile)> { use codex_config::Constrained; use std::sync::Arc; use tempfile::TempDir; @@ -897,10 +901,7 @@ allow_local_binding = true /*exclude_tmpdir_env_var*/ false, /*exclude_slash_tmp*/ false, ); - let sandbox_policy = permission_profile_for_config - .clone() - .to_legacy_sandbox_policy(home.path()) - .expect("workspace-write profile should project to legacy policy"); + let permission_profile = permission_profile_for_config.clone(); let mut builder = test_codex() .with_home(home) .with_cloud_requirements(managed_network_requirements_loader()) @@ -922,7 +923,7 @@ allow_local_binding = true "expected managed network proxy config to be present" ); - Ok((test, sandbox_policy)) + Ok((test, permission_profile)) } async fn mount_unified_exec_network_denial_responses( @@ -1040,7 +1041,7 @@ async fn unified_exec_emits_terminal_interaction_for_write_stdin() -> Result<()> ]; mount_sse_sequence(&server, responses).await; - submit_unified_exec_turn(&test, "stdin delta", SandboxPolicy::DangerFullAccess).await?; + submit_unified_exec_turn(&test, "stdin delta", PermissionProfile::Disabled).await?; let mut terminal_interaction = None; @@ -1160,7 +1161,7 @@ async fn unified_exec_terminal_interaction_captures_delayed_output() -> Result<( submit_unified_exec_turn( &test, "delayed terminal interaction output", - SandboxPolicy::DangerFullAccess, + PermissionProfile::Disabled, ) .await?; @@ -1305,7 +1306,7 @@ async fn unified_exec_emits_one_begin_and_one_end_event() -> Result<()> { submit_unified_exec_turn( &test, "check poll event behavior", - SandboxPolicy::DangerFullAccess, + PermissionProfile::Disabled, ) .await?; @@ -1395,7 +1396,7 @@ async fn exec_command_reports_chunk_and_exit_metadata() -> Result<()> { ]; let request_log = mount_sse_sequence(&server, responses).await; - submit_unified_exec_turn(&test, "run metadata test", SandboxPolicy::DangerFullAccess).await?; + submit_unified_exec_turn(&test, "run metadata test", PermissionProfile::Disabled).await?; wait_for_event(&test.codex, |event| { matches!(event, EventMsg::TurnComplete(_)) @@ -1494,7 +1495,7 @@ async fn exec_command_clamps_model_requested_max_output_tokens_to_policy() -> Re submit_unified_exec_turn( &test, "run clamped max output test", - SandboxPolicy::DangerFullAccess, + PermissionProfile::Disabled, ) .await?; @@ -1577,7 +1578,7 @@ async fn write_stdin_clamps_model_requested_max_output_tokens_to_policy() -> Res submit_unified_exec_turn( &test, "run clamped write_stdin output test", - SandboxPolicy::DangerFullAccess, + PermissionProfile::Disabled, ) .await?; @@ -1642,7 +1643,7 @@ async fn unified_exec_defaults_to_pipe() -> Result<()> { submit_unified_exec_turn( &test, "check default pipe mode", - SandboxPolicy::DangerFullAccess, + PermissionProfile::Disabled, ) .await?; @@ -1709,7 +1710,7 @@ async fn unified_exec_can_enable_tty() -> Result<()> { ]; let request_log = mount_sse_sequence(&server, responses).await; - submit_unified_exec_turn(&test, "check tty enabled", SandboxPolicy::DangerFullAccess).await?; + submit_unified_exec_turn(&test, "check tty enabled", PermissionProfile::Disabled).await?; wait_for_event(&test.codex, |event| { matches!(event, EventMsg::TurnComplete(_)) @@ -1776,7 +1777,7 @@ async fn unified_exec_respects_early_exit_notifications() -> Result<()> { submit_unified_exec_turn( &test, "watch early exit timing", - SandboxPolicy::DangerFullAccess, + PermissionProfile::Disabled, ) .await?; @@ -1895,7 +1896,7 @@ async fn write_stdin_returns_exit_metadata_and_clears_session() -> Result<()> { submit_unified_exec_turn( &test, "test write_stdin exit behavior", - SandboxPolicy::DangerFullAccess, + PermissionProfile::Disabled, ) .await?; @@ -2048,7 +2049,7 @@ async fn unified_exec_emits_end_event_when_session_dies_via_stdin() -> Result<() ]; mount_sse_sequence(&server, responses).await; - submit_unified_exec_turn(&test, "end on exit", SandboxPolicy::DangerFullAccess).await?; + submit_unified_exec_turn(&test, "end on exit", PermissionProfile::Disabled).await?; // We expect the ExecCommandEnd event to match the initial exec_command call_id. let end_event = wait_for_event_match(&test.codex, |msg| match msg { @@ -2114,6 +2115,9 @@ async fn unified_exec_keeps_long_running_session_after_turn_end() -> Result<()> mount_sse_sequence(&server, responses).await; let session_model = session_configured.model.clone(); + let turn_cwd = cwd.path().to_path_buf(); + let (sandbox_policy, permission_profile) = + turn_permission_fields(PermissionProfile::Disabled, &turn_cwd); codex .submit(Op::UserTurn { @@ -2123,11 +2127,11 @@ async fn unified_exec_keeps_long_running_session_after_turn_end() -> Result<()> text_elements: Vec::new(), }], final_output_json_schema: None, - cwd: cwd.path().to_path_buf(), + cwd: turn_cwd, approval_policy: AskForApproval::Never, approvals_reviewer: None, - sandbox_policy: SandboxPolicy::DangerFullAccess, - permission_profile: None, + sandbox_policy, + permission_profile, model: session_model, effort: None, summary: None, @@ -2209,6 +2213,9 @@ async fn unified_exec_interrupt_preserves_long_running_session() -> Result<()> { mount_sse_sequence(&server, responses).await; let session_model = session_configured.model.clone(); + let turn_cwd = cwd.path().to_path_buf(); + let (sandbox_policy, permission_profile) = + turn_permission_fields(PermissionProfile::Disabled, &turn_cwd); codex .submit(Op::UserTurn { @@ -2218,11 +2225,11 @@ async fn unified_exec_interrupt_preserves_long_running_session() -> Result<()> { text_elements: Vec::new(), }], final_output_json_schema: None, - cwd: cwd.path().to_path_buf(), + cwd: turn_cwd, approval_policy: AskForApproval::Never, approvals_reviewer: None, - sandbox_policy: SandboxPolicy::DangerFullAccess, - permission_profile: None, + sandbox_policy, + permission_profile, model: session_model, effort: None, summary: None, @@ -2314,7 +2321,7 @@ async fn unified_exec_reuses_session_via_stdin() -> Result<()> { ]; let request_log = mount_sse_sequence(&server, responses).await; - submit_unified_exec_turn(&test, "run unified exec", SandboxPolicy::DangerFullAccess).await?; + submit_unified_exec_turn(&test, "run unified exec", PermissionProfile::Disabled).await?; wait_for_event(&test.codex, |event| { matches!(event, EventMsg::TurnComplete(_)) @@ -2432,12 +2439,7 @@ PY ]; let request_log = mount_sse_sequence(&server, responses).await; - submit_unified_exec_turn( - &test, - "exercise lag handling", - SandboxPolicy::DangerFullAccess, - ) - .await?; + submit_unified_exec_turn(&test, "exercise lag handling", PermissionProfile::Disabled).await?; // This is a worst case scenario for the truncate logic, and CI can spend a // while draining the lagged tail before the follow-up tool call completes. wait_for_event_with_timeout( @@ -2532,7 +2534,7 @@ async fn unified_exec_timeout_and_followup_poll() -> Result<()> { ]; let request_log = mount_sse_sequence(&server, responses).await; - submit_unified_exec_turn(&test, "check timeout", SandboxPolicy::DangerFullAccess).await?; + submit_unified_exec_turn(&test, "check timeout", PermissionProfile::Disabled).await?; loop { let event = test.codex.next_event().await.expect("event"); @@ -2608,12 +2610,7 @@ PY ]; let request_log = mount_sse_sequence(&server, responses).await; - submit_unified_exec_turn( - &test, - "summarize large output", - SandboxPolicy::DangerFullAccess, - ) - .await?; + submit_unified_exec_turn(&test, "summarize large output", PermissionProfile::Disabled).await?; wait_for_event(&test.codex, |event| { matches!(event, EventMsg::TurnComplete(_)) @@ -2683,6 +2680,9 @@ async fn unified_exec_runs_under_sandbox() -> Result<()> { let request_log = mount_sse_sequence(&server, responses).await; let session_model = session_configured.model.clone(); + let turn_cwd = cwd.path().to_path_buf(); + let (sandbox_policy, permission_profile) = + turn_permission_fields(PermissionProfile::read_only(), &turn_cwd); codex .submit(Op::UserTurn { @@ -2692,12 +2692,12 @@ async fn unified_exec_runs_under_sandbox() -> Result<()> { text_elements: Vec::new(), }], final_output_json_schema: None, - cwd: cwd.path().to_path_buf(), + cwd: turn_cwd, approval_policy: AskForApproval::Never, approvals_reviewer: None, // Important! - sandbox_policy: SandboxPolicy::new_read_only_policy(), - permission_profile: None, + sandbox_policy, + permission_profile, model: session_model, effort: None, summary: None, @@ -2799,7 +2799,9 @@ async fn unified_exec_enforces_glob_deny_read_policy() -> Result<()> { let request_log = mount_sse_sequence(&server, responses).await; let session_model = session_configured.model.clone(); - let read_only_policy = SandboxPolicy::new_read_only_policy(); + let turn_cwd = cwd.path().to_path_buf(); + let (sandbox_policy, permission_profile) = + turn_permission_fields(PermissionProfile::read_only(), &turn_cwd); codex .submit(Op::UserTurn { environments: None, @@ -2808,11 +2810,11 @@ async fn unified_exec_enforces_glob_deny_read_policy() -> Result<()> { text_elements: Vec::new(), }], final_output_json_schema: None, - cwd: cwd.path().to_path_buf(), + cwd: turn_cwd, approval_policy: AskForApproval::Never, approvals_reviewer: None, - sandbox_policy: read_only_policy, - permission_profile: None, + sandbox_policy, + permission_profile, model: session_model, effort: None, summary: None, @@ -2929,6 +2931,9 @@ async fn unified_exec_python_prompt_under_seatbelt() -> Result<()> { let request_log = mount_sse_sequence(&server, responses).await; let session_model = session_configured.model.clone(); + let turn_cwd = cwd.path().to_path_buf(); + let (sandbox_policy, permission_profile) = + turn_permission_fields(PermissionProfile::read_only(), &turn_cwd); codex .submit(Op::UserTurn { @@ -2938,11 +2943,11 @@ async fn unified_exec_python_prompt_under_seatbelt() -> Result<()> { text_elements: Vec::new(), }], final_output_json_schema: None, - cwd: cwd.path().to_path_buf(), + cwd: turn_cwd, approval_policy: AskForApproval::Never, approvals_reviewer: None, - sandbox_policy: SandboxPolicy::new_read_only_policy(), - permission_profile: None, + sandbox_policy, + permission_profile, model: session_model, effort: None, summary: None, @@ -3025,12 +3030,7 @@ async fn unified_exec_runs_on_all_platforms() -> Result<()> { ]; let request_log = mount_sse_sequence(&server, responses).await; - submit_unified_exec_turn( - &test, - "summarize large output", - SandboxPolicy::DangerFullAccess, - ) - .await?; + submit_unified_exec_turn(&test, "summarize large output", PermissionProfile::Disabled).await?; wait_for_event(&test.codex, |event| { matches!(event, EventMsg::TurnComplete(_)) @@ -3148,7 +3148,7 @@ async fn unified_exec_prunes_exited_sessions_first() -> Result<()> { let response_mock = mount_sse_sequence(&server, vec![first_response, completion_response]).await; - submit_unified_exec_turn(&test, "fill session cache", SandboxPolicy::DangerFullAccess).await?; + submit_unified_exec_turn(&test, "fill session cache", PermissionProfile::Disabled).await?; wait_for_event(&test.codex, |event| { matches!(event, EventMsg::TurnComplete(_))