From 1fed948c66277fd100ca31bd3ade227a3acab136 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 28 Apr 2026 17:18:19 -0700 Subject: [PATCH 1/2] core tests: migrate apply patch turns to profiles (#20040) ## Summary - Removes `SandboxPolicy` from the apply-patch CLI test suite. - Uses the harness' profile-backed submit helper for danger/no-sandbox turns instead of constructing `Op::UserTurn` manually with legacy fields. - Converts the workspace-write traversal cases to submit `PermissionProfile::workspace_write_with(...)` directly. ## Verification - `cargo check -p codex-core --tests` - `just fmt` --- codex-rs/core/tests/suite/apply_patch_cli.rs | 219 +++---------------- 1 file changed, 25 insertions(+), 194 deletions(-) diff --git a/codex-rs/core/tests/suite/apply_patch_cli.rs b/codex-rs/core/tests/suite/apply_patch_cli.rs index 4bb3be6631..2d931efb38 100644 --- a/codex-rs/core/tests/suite/apply_patch_cli.rs +++ b/codex-rs/core/tests/suite/apply_patch_cli.rs @@ -13,11 +13,9 @@ use std::sync::atomic::Ordering; use std::time::Duration; use codex_features::Feature; -use codex_protocol::protocol::AskForApproval; +use codex_protocol::models::PermissionProfile; +use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::EventMsg; -use codex_protocol::protocol::Op; -use codex_protocol::protocol::SandboxPolicy; -use codex_protocol::user_input::UserInput; #[cfg(target_os = "linux")] use codex_sandboxing::landlock::CODEX_LINUX_SANDBOX_ARG0; use core_test_support::assert_regex_match; @@ -59,6 +57,15 @@ async fn apply_patch_harness_with( Box::pin(TestCodexHarness::with_remote_aware_builder(builder)).await } +fn restrictive_workspace_write_profile() -> PermissionProfile { + PermissionProfile::workspace_write_with( + &[], + NetworkSandboxPolicy::Restricted, + /*exclude_tmpdir_env_var*/ true, + /*exclude_slash_tmp*/ true, + ) +} + pub async fn mount_apply_patch( harness: &TestCodexHarness, call_id: &str, @@ -354,28 +361,7 @@ async fn apply_patch_cli_move_without_content_change_has_no_turn_diff( let call_id = "apply-move-no-change"; mount_apply_patch(&harness, call_id, patch, "ok", model_output).await; - let model = test.session_configured.model.clone(); - codex - .submit(Op::UserTurn { - environments: None, - items: vec![UserInput::Text { - text: "rename without content change".into(), - text_elements: Vec::new(), - }], - final_output_json_schema: None, - cwd: harness.cwd().to_path_buf(), - approval_policy: AskForApproval::Never, - approvals_reviewer: None, - sandbox_policy: SandboxPolicy::DangerFullAccess, - permission_profile: None, - model, - effort: None, - summary: None, - service_tier: None, - collaboration_mode: None, - personality: None, - }) - .await?; + harness.submit("rename without content change").await?; let mut saw_turn_diff = false; wait_for_event(&codex, |event| match event { @@ -641,16 +627,10 @@ async fn apply_patch_cli_rejects_path_traversal_outside_workspace( let call_id = "apply-path-traversal"; mount_apply_patch(&harness, call_id, patch, "fail", model_output).await; - let sandbox_policy = SandboxPolicy::WorkspaceWrite { - writable_roots: vec![], - network_access: false, - exclude_tmpdir_env_var: true, - exclude_slash_tmp: true, - }; harness - .submit_with_policy( + .submit_with_permission_profile( "attempt to escape workspace via apply_patch", - sandbox_policy, + restrictive_workspace_write_profile(), ) .await?; @@ -696,14 +676,11 @@ async fn apply_patch_cli_rejects_move_path_traversal_outside_workspace( let call_id = "apply-move-traversal"; mount_apply_patch(&harness, call_id, patch, "fail", model_output).await; - let sandbox_policy = SandboxPolicy::WorkspaceWrite { - writable_roots: vec![], - network_access: false, - exclude_tmpdir_env_var: true, - exclude_slash_tmp: true, - }; harness - .submit_with_policy("attempt move traversal via apply_patch", sandbox_policy) + .submit_with_permission_profile( + "attempt move traversal via apply_patch", + restrictive_workspace_write_profile(), + ) .await?; let out = harness.apply_patch_output(call_id, model_output).await; @@ -992,27 +969,7 @@ async fn apply_patch_custom_tool_streaming_emits_updated_changes() -> Result<()> ) .await; - codex - .submit(Op::UserTurn { - environments: None, - items: vec![UserInput::Text { - text: "create streamed file".into(), - text_elements: Vec::new(), - }], - final_output_json_schema: None, - cwd: harness.cwd().to_path_buf(), - approval_policy: AskForApproval::Never, - approvals_reviewer: None, - sandbox_policy: SandboxPolicy::DangerFullAccess, - permission_profile: None, - model: test.session_configured.model.clone(), - effort: None, - summary: None, - service_tier: None, - collaboration_mode: None, - personality: None, - }) - .await?; + harness.submit("create streamed file").await?; let mut updates = Vec::new(); wait_for_event(&codex, |event| match event { @@ -1090,28 +1047,7 @@ async fn apply_patch_shell_command_heredoc_with_cd_emits_turn_diff() -> Result<( ]; mount_sse_sequence(harness.server(), bodies).await; - let model = test.session_configured.model.clone(); - codex - .submit(Op::UserTurn { - environments: None, - items: vec![UserInput::Text { - text: "apply via shell heredoc with cd".into(), - text_elements: Vec::new(), - }], - final_output_json_schema: None, - cwd: harness.cwd().to_path_buf(), - approval_policy: AskForApproval::Never, - approvals_reviewer: None, - sandbox_policy: SandboxPolicy::DangerFullAccess, - permission_profile: None, - model, - effort: None, - summary: None, - service_tier: None, - collaboration_mode: None, - personality: None, - }) - .await?; + harness.submit("apply via shell heredoc with cd").await?; let mut saw_turn_diff = None; let mut saw_patch_begin = false; @@ -1176,28 +1112,7 @@ async fn apply_patch_shell_command_failure_propagates_error_and_skips_diff() -> ]; mount_sse_sequence(harness.server(), bodies).await; - let model = test.session_configured.model.clone(); - codex - .submit(Op::UserTurn { - environments: None, - items: vec![UserInput::Text { - text: "apply patch via shell".into(), - text_elements: Vec::new(), - }], - final_output_json_schema: None, - cwd: harness.cwd().to_path_buf(), - approval_policy: AskForApproval::Never, - approvals_reviewer: None, - sandbox_policy: SandboxPolicy::DangerFullAccess, - permission_profile: None, - model, - effort: None, - summary: None, - service_tier: None, - collaboration_mode: None, - personality: None, - }) - .await?; + harness.submit("apply patch via shell").await?; let mut saw_turn_diff = false; wait_for_event(&codex, |event| match event { @@ -1333,28 +1248,7 @@ async fn apply_patch_emits_turn_diff_event_with_unified_diff( let patch = format!("*** Begin Patch\n*** Add File: {file}\n+hello\n*** End Patch\n"); mount_apply_patch(&harness, call_id, patch.as_str(), "ok", model_output).await; - let model = test.session_configured.model.clone(); - codex - .submit(Op::UserTurn { - environments: None, - items: vec![UserInput::Text { - text: "emit diff".into(), - text_elements: Vec::new(), - }], - final_output_json_schema: None, - cwd: harness.cwd().to_path_buf(), - approval_policy: AskForApproval::Never, - approvals_reviewer: None, - sandbox_policy: SandboxPolicy::DangerFullAccess, - permission_profile: None, - model, - effort: None, - summary: None, - service_tier: None, - collaboration_mode: None, - personality: None, - }) - .await?; + harness.submit("emit diff").await?; let mut saw_turn_diff = None; wait_for_event(&codex, |event| match event { @@ -1402,28 +1296,7 @@ async fn apply_patch_turn_diff_for_rename_with_content_change( let patch = "*** Begin Patch\n*** Update File: old.txt\n*** Move to: new.txt\n@@\n-old\n+new\n*** End Patch"; mount_apply_patch(&harness, call_id, patch, "ok", model_output).await; - let model = test.session_configured.model.clone(); - codex - .submit(Op::UserTurn { - environments: None, - items: vec![UserInput::Text { - text: "rename with change".into(), - text_elements: Vec::new(), - }], - final_output_json_schema: None, - cwd: harness.cwd().to_path_buf(), - approval_policy: AskForApproval::Never, - approvals_reviewer: None, - sandbox_policy: SandboxPolicy::DangerFullAccess, - permission_profile: None, - model, - effort: None, - summary: None, - service_tier: None, - collaboration_mode: None, - personality: None, - }) - .await?; + harness.submit("rename with change").await?; let mut last_diff: Option = None; wait_for_event(&codex, |event| match event { @@ -1480,28 +1353,7 @@ async fn apply_patch_aggregates_diff_across_multiple_tool_calls() -> Result<()> ]); mount_sse_sequence(harness.server(), vec![s1, s2, s3]).await; - let model = test.session_configured.model.clone(); - codex - .submit(Op::UserTurn { - environments: None, - items: vec![UserInput::Text { - text: "aggregate diffs".into(), - text_elements: Vec::new(), - }], - final_output_json_schema: None, - cwd: harness.cwd().to_path_buf(), - approval_policy: AskForApproval::Never, - approvals_reviewer: None, - sandbox_policy: SandboxPolicy::DangerFullAccess, - permission_profile: None, - model, - effort: None, - summary: None, - service_tier: None, - collaboration_mode: None, - personality: None, - }) - .await?; + harness.submit("aggregate diffs").await?; let mut last_diff: Option = None; wait_for_event(&codex, |event| match event { @@ -1558,28 +1410,7 @@ async fn apply_patch_aggregates_diff_preserves_success_after_failure() -> Result ]; mount_sse_sequence(harness.server(), responses).await; - let model = test.session_configured.model.clone(); - codex - .submit(Op::UserTurn { - environments: None, - items: vec![UserInput::Text { - text: "apply patch twice with failure".into(), - text_elements: Vec::new(), - }], - final_output_json_schema: None, - cwd: harness.cwd().to_path_buf(), - approval_policy: AskForApproval::Never, - approvals_reviewer: None, - sandbox_policy: SandboxPolicy::DangerFullAccess, - permission_profile: None, - model, - effort: None, - summary: None, - service_tier: None, - collaboration_mode: None, - personality: None, - }) - .await?; + harness.submit("apply patch twice with failure").await?; let mut last_diff: Option = None; wait_for_event_with_timeout( From 846e541e4321de22edfc09ea691f619323e70e2c Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 28 Apr 2026 17:18:22 -0700 Subject: [PATCH 2/2] core tests: migrate hook turns to profiles --- codex-rs/core/tests/suite/hooks.rs | 68 +++++++++++++++++------------- 1 file changed, 39 insertions(+), 29 deletions(-) diff --git a/codex-rs/core/tests/suite/hooks.rs b/codex-rs/core/tests/suite/hooks.rs index 6d232afd21..daf3053adc 100644 --- a/codex-rs/core/tests/suite/hooks.rs +++ b/codex-rs/core/tests/suite/hooks.rs @@ -7,13 +7,14 @@ use codex_core::config::Constrained; use codex_features::Feature; use codex_protocol::items::parse_hook_prompt_fragment; use codex_protocol::models::ContentItem; +use codex_protocol::models::PermissionProfile; use codex_protocol::models::ResponseItem; +use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::Op; use codex_protocol::protocol::RolloutItem; use codex_protocol::protocol::RolloutLine; -use codex_protocol::protocol::SandboxPolicy; use codex_protocol::user_input::UserInput; use core_test_support::managed_network_requirements_loader; use core_test_support::responses::ev_apply_patch_function_call; @@ -48,6 +49,24 @@ const BLOCKED_PROMPT_CONTEXT: &str = "Remember the blocked lighthouse note."; const PERMISSION_REQUEST_HOOK_MATCHER: &str = "^Bash$"; const PERMISSION_REQUEST_ALLOW_REASON: &str = "should not be used for allow"; +fn restrictive_workspace_write_profile() -> PermissionProfile { + PermissionProfile::workspace_write_with( + &[], + NetworkSandboxPolicy::Restricted, + /*exclude_tmpdir_env_var*/ true, + /*exclude_slash_tmp*/ true, + ) +} + +fn network_workspace_write_profile() -> PermissionProfile { + PermissionProfile::workspace_write_with( + &[], + NetworkSandboxPolicy::Enabled, + /*exclude_tmpdir_env_var*/ false, + /*exclude_slash_tmp*/ false, + ) +} + fn write_stop_hook(home: &Path, block_prompts: &[&str]) -> Result<()> { let script_path = home.join("stop_hook.py"); let log_path = home.join("stop_hook_log.jsonl"); @@ -1322,10 +1341,10 @@ async fn permission_request_hook_allows_shell_command_without_user_approval() -> fs::write(&marker, "seed").context("create permission request marker")?; - test.submit_turn_with_policies( + test.submit_turn_with_approval_and_permission_profile( "run the shell command after hook approval", AskForApproval::OnRequest, - codex_protocol::protocol::SandboxPolicy::DangerFullAccess, + PermissionProfile::Disabled, ) .await?; @@ -1407,15 +1426,10 @@ async fn permission_request_hook_allows_apply_patch_with_write_alias() -> Result let test = builder.build(&server).await?; let target_path = test.workspace_path(&patch_path); - test.submit_turn_with_policies( + test.submit_turn_with_approval_and_permission_profile( "apply the patch after hook approval", AskForApproval::OnRequest, - SandboxPolicy::WorkspaceWrite { - writable_roots: vec![], - network_access: false, - exclude_tmpdir_env_var: true, - exclude_slash_tmp: true, - }, + restrictive_workspace_write_profile(), ) .await?; @@ -1494,10 +1508,10 @@ async fn permission_request_hook_sees_raw_exec_command_input() -> Result<()> { fs::write(&marker, "seed").context("create exec command permission request marker")?; - test.submit_turn_with_policies( + test.submit_turn_with_approval_and_permission_profile( "run the exec command after hook approval", AskForApproval::OnRequest, - codex_protocol::protocol::SandboxPolicy::new_read_only_policy(), + PermissionProfile::read_only(), ) .await?; @@ -1558,13 +1572,8 @@ allow_local_binding = true .await; let approval_policy = AskForApproval::OnFailure; - let sandbox_policy = SandboxPolicy::WorkspaceWrite { - writable_roots: vec![], - network_access: true, - exclude_tmpdir_env_var: false, - exclude_slash_tmp: false, - }; - let sandbox_policy_for_config = sandbox_policy.clone(); + let permission_profile = network_workspace_write_profile(); + let permission_profile_for_config = permission_profile.clone(); let test = test_codex() .with_home(Arc::clone(&home)) .with_pre_build_hook(|home| { @@ -1580,8 +1589,9 @@ allow_local_binding = true .expect("test config should allow feature update"); config.permissions.approval_policy = Constrained::allow_any(approval_policy); config - .set_legacy_sandbox_policy(sandbox_policy_for_config) - .expect("set sandbox policy"); + .permissions + .set_permission_profile(permission_profile_for_config) + .expect("set permission profile"); }) .build(&server) .await?; @@ -1598,10 +1608,10 @@ allow_local_binding = true .as_ref() .expect("expected runtime managed network proxy addresses"); - test.submit_turn_with_policies( + test.submit_turn_with_approval_and_permission_profile( "run the shell command after network hook approval", approval_policy, - sandbox_policy, + permission_profile, ) .await?; @@ -1695,10 +1705,10 @@ async fn permission_request_hook_sees_retry_context_after_sandbox_denial() -> Re let marker_path = test.workspace_path(marker); let _ = fs::remove_file(&marker_path); - test.submit_turn_with_policies( + test.submit_turn_with_approval_and_permission_profile( "retry the shell command after sandbox denial", AskForApproval::OnFailure, - codex_protocol::protocol::SandboxPolicy::new_read_only_policy(), + PermissionProfile::read_only(), ) .await?; @@ -1769,9 +1779,9 @@ async fn pre_tool_use_blocks_shell_command_before_execution() -> Result<()> { fs::remove_file(&marker).context("remove leftover pre tool use marker")?; } - test.submit_turn_with_policy( + test.submit_turn_with_permission_profile( "run the blocked shell command", - codex_protocol::protocol::SandboxPolicy::DangerFullAccess, + PermissionProfile::Disabled, ) .await?; @@ -2013,9 +2023,9 @@ async fn pre_tool_use_blocks_shell_when_defined_in_config_toml() -> Result<()> { fs::remove_file(&marker).context("remove leftover config.toml marker")?; } - test.submit_turn_with_policy( + test.submit_turn_with_permission_profile( "run the blocked shell command from config toml", - codex_protocol::protocol::SandboxPolicy::DangerFullAccess, + PermissionProfile::Disabled, ) .await?;