From 026df712cc6f0a4825ef9cb957a7990908ad9e82 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 28 Apr 2026 17:15:58 -0700 Subject: [PATCH 1/2] core tests: migrate zsh-fork permissions to profiles (#20034) ## Summary - Updates the zsh-fork test helper to configure `PermissionProfile` directly instead of constructing a legacy `SandboxPolicy`. - Sends permission-profile-backed turns from the skill approval zsh-fork tests so the runtime and request path exercise the canonical permissions model. - Leaves the broader approvals suite on legacy policies for now, except for the zsh-fork test that shares this helper. ## Verification - `cargo check -p codex-core --tests` - `just fmt` --- codex-rs/core/tests/common/zsh_fork.rs | 28 ++++++++------- codex-rs/core/tests/suite/approvals.rs | 38 +++++++++++++++------ codex-rs/core/tests/suite/skill_approval.rs | 23 +++++++------ 3 files changed, 56 insertions(+), 33 deletions(-) diff --git a/codex-rs/core/tests/common/zsh_fork.rs b/codex-rs/core/tests/common/zsh_fork.rs index 448693e069..e58ebd81dd 100644 --- a/codex-rs/core/tests/common/zsh_fork.rs +++ b/codex-rs/core/tests/common/zsh_fork.rs @@ -5,8 +5,9 @@ use anyhow::Result; use codex_core::config::Config; use codex_core::config::Constrained; use codex_features::Feature; +use codex_protocol::models::PermissionProfile; +use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::AskForApproval; -use codex_protocol::protocol::SandboxPolicy; use crate::test_codex::TestCodex; use crate::test_codex::test_codex; @@ -22,7 +23,7 @@ impl ZshForkRuntime { &self, config: &mut Config, approval_policy: AskForApproval, - sandbox_policy: SandboxPolicy, + permission_profile: PermissionProfile, ) { config .features @@ -37,18 +38,19 @@ impl ZshForkRuntime { config.permissions.allow_login_shell = false; config.permissions.approval_policy = Constrained::allow_any(approval_policy); config - .set_legacy_sandbox_policy(sandbox_policy) - .expect("set sandbox policy"); + .permissions + .set_permission_profile(permission_profile) + .expect("set permission profile"); } } -pub fn restrictive_workspace_write_policy() -> SandboxPolicy { - SandboxPolicy::WorkspaceWrite { - writable_roots: Vec::new(), - network_access: false, - exclude_tmpdir_env_var: true, - exclude_slash_tmp: true, - } +pub fn restrictive_workspace_write_profile() -> PermissionProfile { + PermissionProfile::workspace_write_with( + &[], + NetworkSandboxPolicy::Restricted, + /*exclude_tmpdir_env_var*/ true, + /*exclude_slash_tmp*/ true, + ) } pub fn zsh_fork_runtime(test_name: &str) -> Result> { @@ -78,7 +80,7 @@ pub async fn build_zsh_fork_test( server: &wiremock::MockServer, runtime: ZshForkRuntime, approval_policy: AskForApproval, - sandbox_policy: SandboxPolicy, + permission_profile: PermissionProfile, pre_build_hook: F, ) -> Result where @@ -87,7 +89,7 @@ where let mut builder = test_codex() .with_pre_build_hook(pre_build_hook) .with_config(move |config| { - runtime.apply_to_config(config, approval_policy, sandbox_policy); + runtime.apply_to_config(config, approval_policy, permission_profile); }); builder.build(server).await } diff --git a/codex-rs/core/tests/suite/approvals.rs b/codex-rs/core/tests/suite/approvals.rs index ef22cfbfc1..79bf0f830f 100644 --- a/codex-rs/core/tests/suite/approvals.rs +++ b/codex-rs/core/tests/suite/approvals.rs @@ -32,10 +32,11 @@ use core_test_support::responses::start_mock_server; use core_test_support::skip_if_no_network; 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 core_test_support::wait_for_event_with_timeout; use core_test_support::zsh_fork::build_zsh_fork_test; -use core_test_support::zsh_fork::restrictive_workspace_write_policy; +use core_test_support::zsh_fork::restrictive_workspace_write_profile; use core_test_support::zsh_fork::zsh_fork_runtime; use pretty_assertions::assert_eq; use regex_lite::Regex; @@ -2318,7 +2319,7 @@ async fn matched_prefix_rule_runs_unsandboxed_under_zsh_fork() -> Result<()> { }; let approval_policy = AskForApproval::Never; - let sandbox_policy = restrictive_workspace_write_policy(); + let permission_profile = restrictive_workspace_write_profile(); let outside_dir = tempfile::tempdir_in(std::env::current_dir()?)?; let outside_path = outside_dir .path() @@ -2332,7 +2333,7 @@ async fn matched_prefix_rule_runs_unsandboxed_under_zsh_fork() -> Result<()> { &server, runtime, approval_policy, - sandbox_policy.clone(), + permission_profile.clone(), move |home| { let _ = fs::remove_file(&outside_path_for_hook); let rules_dir = home.join("rules"); @@ -2367,13 +2368,30 @@ async fn matched_prefix_rule_runs_unsandboxed_under_zsh_fork() -> Result<()> { ) .await; - submit_turn( - &test, - "run allowed touch under zsh fork", - approval_policy, - sandbox_policy, - ) - .await?; + 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, + items: vec![UserInput::Text { + text: "run allowed touch under zsh fork".into(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + cwd: test.cwd.path().to_path_buf(), + approval_policy, + approvals_reviewer: Some(ApprovalsReviewer::User), + sandbox_policy, + permission_profile, + model: session_model, + effort: None, + summary: None, + service_tier: None, + collaboration_mode: None, + personality: None, + }) + .await?; wait_for_completion_without_approval(&test).await; diff --git a/codex-rs/core/tests/suite/skill_approval.rs b/codex-rs/core/tests/suite/skill_approval.rs index 44c2fb7b68..72244d534e 100644 --- a/codex-rs/core/tests/suite/skill_approval.rs +++ b/codex-rs/core/tests/suite/skill_approval.rs @@ -2,21 +2,22 @@ #![cfg(unix)] use anyhow::Result; +use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::ExecApprovalRequestEvent; use codex_protocol::protocol::GranularApprovalConfig; use codex_protocol::protocol::Op; -use codex_protocol::protocol::SandboxPolicy; use codex_protocol::user_input::UserInput; use core_test_support::responses::mount_function_call_agent_response; use core_test_support::responses::start_mock_server; use core_test_support::skip_if_no_network; use core_test_support::test_codex::TestCodex; +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::zsh_fork::build_zsh_fork_test; -use core_test_support::zsh_fork::restrictive_workspace_write_policy; +use core_test_support::zsh_fork::restrictive_workspace_write_profile; use core_test_support::zsh_fork::zsh_fork_runtime; use std::fs; use std::path::Path; @@ -40,8 +41,10 @@ async fn submit_turn_with_policies( test: &TestCodex, prompt: &str, approval_policy: AskForApproval, - sandbox_policy: SandboxPolicy, + permission_profile: PermissionProfile, ) -> Result<()> { + let (sandbox_policy, permission_profile) = + turn_permission_fields(permission_profile, test.cwd_path()); test.codex .submit(Op::UserTurn { environments: None, @@ -54,7 +57,7 @@ async fn submit_turn_with_policies( approval_policy, approvals_reviewer: None, sandbox_policy, - permission_profile: None, + permission_profile, model: test.session_configured.model.clone(), effort: None, summary: None, @@ -144,7 +147,7 @@ async fn shell_zsh_fork_skill_scripts_ignore_declared_permissions() -> Result<() request_permissions: true, mcp_elicitations: true, }); - let workspace_write_policy = restrictive_workspace_write_policy(); + let workspace_write_profile = restrictive_workspace_write_profile(); let outside_dir = tempfile::tempdir_in(std::env::current_dir()?)?; let allowed_dir = outside_dir.path().join("allowed-output"); fs::create_dir_all(&allowed_dir)?; @@ -165,7 +168,7 @@ async fn shell_zsh_fork_skill_scripts_ignore_declared_permissions() -> Result<() &server, runtime, approval_policy, - workspace_write_policy.clone(), + workspace_write_profile.clone(), move |home| { let _ = fs::remove_file(&allowed_path_for_hook); write_skill_with_shell_script_contents( @@ -190,7 +193,7 @@ async fn shell_zsh_fork_skill_scripts_ignore_declared_permissions() -> Result<() &test, "use $mbolin-test-skill", approval_policy, - workspace_write_policy, + workspace_write_profile, ) .await?; @@ -235,13 +238,13 @@ async fn shell_zsh_fork_still_enforces_workspace_write_sandbox() -> Result<()> { let server = start_mock_server().await; let tool_call_id = "zsh-fork-workspace-write-deny"; let outside_path = "/tmp/codex-zsh-fork-workspace-write-deny.txt"; - let workspace_write_policy = restrictive_workspace_write_policy(); + let workspace_write_profile = restrictive_workspace_write_profile(); let _ = fs::remove_file(outside_path); let test = build_zsh_fork_test( &server, runtime, AskForApproval::Never, - workspace_write_policy.clone(), + workspace_write_profile.clone(), move |_| { let _ = fs::remove_file(outside_path); }, @@ -258,7 +261,7 @@ async fn shell_zsh_fork_still_enforces_workspace_write_sandbox() -> Result<()> { &test, "write outside workspace with zsh fork", AskForApproval::Never, - workspace_write_policy, + workspace_write_profile, ) .await?; From dc486f5cee33dec579bab1c352d09abb1f7f993d Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 28 Apr 2026 17:16:05 -0700 Subject: [PATCH 2/2] core tests: migrate compact turns to profiles --- codex-rs/core/tests/suite/compact.rs | 195 +++++++++------------------ 1 file changed, 61 insertions(+), 134 deletions(-) diff --git a/codex-rs/core/tests/suite/compact.rs b/codex-rs/core/tests/suite/compact.rs index ce9dfdf28c..629c370103 100644 --- a/codex-rs/core/tests/suite/compact.rs +++ b/codex-rs/core/tests/suite/compact.rs @@ -8,6 +8,7 @@ use codex_model_provider_info::ModelProviderInfo; use codex_model_provider_info::built_in_model_providers; use codex_models_manager::bundled_models_response; use codex_protocol::items::TurnItem; +use codex_protocol::models::PermissionProfile; use codex_protocol::openai_models::ModelInfo; use codex_protocol::openai_models::ModelsResponse; use codex_protocol::protocol::AskForApproval; @@ -17,7 +18,6 @@ use codex_protocol::protocol::ItemStartedEvent; use codex_protocol::protocol::Op; use codex_protocol::protocol::RolloutItem; use codex_protocol::protocol::RolloutLine; -use codex_protocol::protocol::SandboxPolicy; use codex_protocol::protocol::WarningEvent; use codex_protocol::user_input::UserInput; use core_test_support::context_snapshot; @@ -28,6 +28,7 @@ use core_test_support::responses::ev_reasoning_item; use core_test_support::responses::mount_models_once; use core_test_support::skip_if_no_network; 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 std::path::PathBuf; @@ -71,6 +72,30 @@ const PRETURN_CONTEXT_DIFF_CWD: &str = "/tmp/PRETURN_CONTEXT_DIFF_CWD"; pub(super) const COMPACT_WARNING_MESSAGE: &str = "Heads up: Long threads and multiple compactions can cause the model to be less accurate. Start a new thread when possible to keep threads small and targeted."; +fn disabled_permission_user_turn(text: impl Into, cwd: PathBuf, model: String) -> Op { + let (sandbox_policy, permission_profile) = + turn_permission_fields(PermissionProfile::Disabled, cwd.as_path()); + Op::UserTurn { + environments: None, + items: vec![UserInput::Text { + text: text.into(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + cwd, + approval_policy: AskForApproval::Never, + approvals_reviewer: None, + sandbox_policy, + permission_profile, + model, + effort: None, + summary: None, + service_tier: None, + collaboration_mode: None, + personality: None, + } +} + fn auto_summary(summary: &str) -> String { summary.to_string() } @@ -1675,25 +1700,11 @@ async fn auto_compact_runs_after_resume_when_token_usage_is_over_limit() { resumed .codex - .submit(Op::UserTurn { - environments: None, - items: vec![UserInput::Text { - text: follow_up_user.into(), - text_elements: Vec::new(), - }], - final_output_json_schema: None, - cwd: resumed.cwd.path().to_path_buf(), - approval_policy: AskForApproval::Never, - approvals_reviewer: None, - sandbox_policy: SandboxPolicy::DangerFullAccess, - permission_profile: None, - model: resumed.session_configured.model.clone(), - effort: None, - summary: None, - service_tier: None, - collaboration_mode: None, - personality: None, - }) + .submit(disabled_permission_user_turn( + follow_up_user, + resumed.cwd.path().to_path_buf(), + resumed.session_configured.model.clone(), + )) .await .unwrap(); @@ -1768,25 +1779,11 @@ async fn pre_sampling_compact_runs_on_switch_to_smaller_context_model() { let test = builder.build(&server).await.expect("build test codex"); test.codex - .submit(Op::UserTurn { - environments: None, - items: vec![UserInput::Text { - text: "before switch".into(), - text_elements: Vec::new(), - }], - final_output_json_schema: None, - cwd: test.cwd.path().to_path_buf(), - approval_policy: AskForApproval::Never, - approvals_reviewer: None, - sandbox_policy: SandboxPolicy::DangerFullAccess, - permission_profile: None, - model: previous_model.to_string(), - effort: None, - summary: None, - service_tier: None, - collaboration_mode: None, - personality: None, - }) + .submit(disabled_permission_user_turn( + "before switch", + test.cwd.path().to_path_buf(), + previous_model.to_string(), + )) .await .expect("submit first user turn"); wait_for_event(&test.codex, |event| { @@ -1795,25 +1792,11 @@ async fn pre_sampling_compact_runs_on_switch_to_smaller_context_model() { .await; test.codex - .submit(Op::UserTurn { - environments: None, - items: vec![UserInput::Text { - text: "after switch".into(), - text_elements: Vec::new(), - }], - final_output_json_schema: None, - cwd: test.cwd.path().to_path_buf(), - approval_policy: AskForApproval::Never, - approvals_reviewer: None, - sandbox_policy: SandboxPolicy::DangerFullAccess, - permission_profile: None, - model: next_model.to_string(), - effort: None, - summary: None, - service_tier: None, - collaboration_mode: None, - personality: None, - }) + .submit(disabled_permission_user_turn( + "after switch", + test.cwd.path().to_path_buf(), + next_model.to_string(), + )) .await .expect("submit second user turn"); assert_compaction_uses_turn_lifecycle_id(&test.codex).await; @@ -1908,25 +1891,11 @@ async fn pre_sampling_compact_runs_after_resume_and_switch_to_smaller_model() { initial .codex - .submit(Op::UserTurn { - environments: None, - items: vec![UserInput::Text { - text: "before resume".into(), - text_elements: Vec::new(), - }], - final_output_json_schema: None, - cwd: initial.cwd.path().to_path_buf(), - approval_policy: AskForApproval::Never, - approvals_reviewer: None, - sandbox_policy: SandboxPolicy::DangerFullAccess, - permission_profile: None, - model: previous_model.to_string(), - effort: None, - summary: None, - service_tier: None, - collaboration_mode: None, - personality: None, - }) + .submit(disabled_permission_user_turn( + "before resume", + initial.cwd.path().to_path_buf(), + previous_model.to_string(), + )) .await .expect("submit pre-resume turn"); wait_for_event(&initial.codex, |event| { @@ -1959,25 +1928,11 @@ async fn pre_sampling_compact_runs_after_resume_and_switch_to_smaller_model() { resumed .codex - .submit(Op::UserTurn { - environments: None, - items: vec![UserInput::Text { - text: "after resume".into(), - text_elements: Vec::new(), - }], - final_output_json_schema: None, - cwd: resumed.cwd.path().to_path_buf(), - approval_policy: AskForApproval::Never, - approvals_reviewer: None, - sandbox_policy: SandboxPolicy::DangerFullAccess, - permission_profile: None, - model: next_model.to_string(), - effort: None, - summary: None, - service_tier: None, - collaboration_mode: None, - personality: None, - }) + .submit(disabled_permission_user_turn( + "after resume", + resumed.cwd.path().to_path_buf(), + next_model.to_string(), + )) .await .expect("submit resumed user turn"); assert_compaction_uses_turn_lifecycle_id(&resumed.codex).await; @@ -3195,25 +3150,11 @@ async fn snapshot_request_shape_pre_turn_compaction_strips_incoming_model_switch .expect("build codex"); test.codex - .submit(Op::UserTurn { - environments: None, - items: vec![UserInput::Text { - text: "BEFORE_SWITCH_USER".into(), - text_elements: Vec::new(), - }], - final_output_json_schema: None, - cwd: test.cwd.path().to_path_buf(), - approval_policy: AskForApproval::Never, - approvals_reviewer: None, - sandbox_policy: SandboxPolicy::DangerFullAccess, - permission_profile: None, - model: previous_model.to_string(), - effort: None, - summary: None, - service_tier: None, - collaboration_mode: None, - personality: None, - }) + .submit(disabled_permission_user_turn( + "BEFORE_SWITCH_USER", + test.cwd.path().to_path_buf(), + previous_model.to_string(), + )) .await .expect("submit first user turn"); wait_for_event(&test.codex, |event| { @@ -3222,25 +3163,11 @@ async fn snapshot_request_shape_pre_turn_compaction_strips_incoming_model_switch .await; test.codex - .submit(Op::UserTurn { - environments: None, - items: vec![UserInput::Text { - text: "AFTER_SWITCH_USER".into(), - text_elements: Vec::new(), - }], - final_output_json_schema: None, - cwd: test.cwd.path().to_path_buf(), - approval_policy: AskForApproval::Never, - approvals_reviewer: None, - sandbox_policy: SandboxPolicy::DangerFullAccess, - permission_profile: None, - model: next_model.to_string(), - effort: None, - summary: None, - service_tier: None, - collaboration_mode: None, - personality: None, - }) + .submit(disabled_permission_user_turn( + "AFTER_SWITCH_USER", + test.cwd.path().to_path_buf(), + next_model.to_string(), + )) .await .expect("submit second user turn"); wait_for_event(&test.codex, |event| {