From e734a1a5c1c6e51d7a28ec8ec6381d7ffb18e23b Mon Sep 17 00:00:00 2001 From: andrewgu-oai Date: Sat, 8 Aug 2026 00:53:30 +0000 Subject: [PATCH] Ignore reusable command approvals for cyber models (#37516) ## What changed - Filter saved `allow` prefix rules from the execution policy for cyber-specialized models and models listed in `auto_review.ignore_rules`, while preserving prompt, forbidden, network, and host-executable policy entries. - Apply the filtered policy consistently to permission instructions, shell commands, unified exec, and the zsh-fork backend. - Limit cyber-model command approvals to one-time decisions without proposing reusable policy amendments. General models continue to honor saved prefixes, including after switching models within a thread. ## Testing - Add policy-level coverage for filtering allow-prefix rules while retaining restrictive and network rules. - Add end-to-end coverage across shell and unified exec paths, zsh-fork execution, user and automatic review, and model switching. GitOrigin-RevId: f3bd3872424291b12354ec415f33986ec369a368 --- .../request_processors/config_processor.rs | 1 + codex-rs/config/src/config_requirements.rs | 17 +- .../config/src/requirements_layers/models.rs | 1 + .../config/src/requirements_layers/stack.rs | 3 +- codex-rs/core/src/exec_policy.rs | 17 +- codex-rs/core/src/exec_policy/model_policy.rs | 47 ++ .../src/exec_policy/model_policy_tests.rs | 177 ++++++++ codex-rs/core/src/exec_policy_tests.rs | 12 + codex-rs/core/src/session/tests.rs | 1 + codex-rs/core/src/session/turn_context.rs | 19 + codex-rs/core/src/session/world_state.rs | 6 +- codex-rs/core/src/tools/handlers/shell.rs | 1 + .../tools/runtimes/shell/unix_escalation.rs | 14 +- .../core/src/unified_exec/process_manager.rs | 1 + .../core/tests/suite/cyber_exec_policy.rs | 413 ++++++++++++++++++ codex-rs/core/tests/suite/mod.rs | 1 + 16 files changed, 717 insertions(+), 14 deletions(-) create mode 100644 codex-rs/core/src/exec_policy/model_policy.rs create mode 100644 codex-rs/core/src/exec_policy/model_policy_tests.rs create mode 100644 codex-rs/core/tests/suite/cyber_exec_policy.rs diff --git a/codex-rs/app-server/src/request_processors/config_processor.rs b/codex-rs/app-server/src/request_processors/config_processor.rs index 8f6750cd3e..a2501855c7 100644 --- a/codex-rs/app-server/src/request_processors/config_processor.rs +++ b/codex-rs/app-server/src/request_processors/config_processor.rs @@ -728,6 +728,7 @@ mod tests { let mapped = map_requirements_toml_to_api(ConfigRequirementsToml { auto_review: Some(AutoReviewRequirementsToml { required_on_models: Some(vec!["gpt-protected".to_string()]), + ignore_rules: None, }), models: Some(ModelsRequirementsToml { new_thread: Some(NewThreadModelDefaultsToml { diff --git a/codex-rs/config/src/config_requirements.rs b/codex-rs/config/src/config_requirements.rs index c0bc51ee86..2508338d49 100644 --- a/codex-rs/config/src/config_requirements.rs +++ b/codex-rs/config/src/config_requirements.rs @@ -964,6 +964,7 @@ pub struct ConfigRequirementsToml { #[derive(Deserialize, Debug, Clone, Default, PartialEq, Eq)] pub struct AutoReviewRequirementsToml { pub required_on_models: Option>, + pub ignore_rules: Option>, } #[derive(Deserialize, Debug, Clone, Default, PartialEq, Eq)] @@ -1174,6 +1175,12 @@ impl ConfigRequirementsWithSources { } } } + if existing_auto_review.value.ignore_rules.is_none() + && let Some(ignore_rules) = incoming_auto_review.ignore_rules + { + existing_auto_review.value.ignore_rules = Some(ignore_rules); + source_contributed = true; + } if source_contributed && existing_auto_review.source != source { existing_auto_review.source = RequirementSource::composite([ existing_auto_review.source.clone(), @@ -1392,10 +1399,11 @@ impl ConfigRequirementsToml { && self.network.is_none() && self.permissions.is_none() && self.auto_review.as_ref().is_none_or(|auto_review| { - auto_review - .required_on_models - .as_ref() - .is_none_or(Vec::is_empty) + auto_review.ignore_rules.as_ref().is_none_or(Vec::is_empty) + && auto_review + .required_on_models + .as_ref() + .is_none_or(Vec::is_empty) }) && self .models @@ -2350,6 +2358,7 @@ mod tests { }; let auto_review = AutoReviewRequirementsToml { required_on_models: Some(vec!["managed-model".to_string()]), + ignore_rules: None, }; let models = ModelsRequirementsToml { new_thread: Some(NewThreadModelDefaultsToml { diff --git a/codex-rs/config/src/requirements_layers/models.rs b/codex-rs/config/src/requirements_layers/models.rs index b48ea1488a..34b3bbc7e9 100644 --- a/codex-rs/config/src/requirements_layers/models.rs +++ b/codex-rs/config/src/requirements_layers/models.rs @@ -47,6 +47,7 @@ impl AutoReviewModelsMergeState { *target = Some(Sourced::new( AutoReviewRequirementsToml { required_on_models: Some(self.slugs), + ignore_rules: None, }, source, )); diff --git a/codex-rs/config/src/requirements_layers/stack.rs b/codex-rs/config/src/requirements_layers/stack.rs index 17aa9c0c06..ababe7c79f 100644 --- a/codex-rs/config/src/requirements_layers/stack.rs +++ b/codex-rs/config/src/requirements_layers/stack.rs @@ -242,7 +242,7 @@ fn populate_merged_regular_fields_with_sources( enforce_residency, network, permissions, - auto_review: _, + auto_review, models, guardian_policy_config, } = requirements; @@ -273,6 +273,7 @@ fn populate_merged_regular_fields_with_sources( set_sourced!(allow_managed_hooks_only, &["allow_managed_hooks_only"]); set_sourced!(allow_appshots, &["allow_appshots"]); set_sourced!(allow_remote_control, &["allow_remote_control"]); + set_sourced!(auto_review, &["auto_review"]); set_sourced!(computer_use, &["computer_use"]); set_sourced!(browser_use, &["browser_use"]); set_sourced!(windows, &["windows"]); diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index c065917d41..5de0593753 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -40,6 +40,10 @@ use codex_shell_command::bash::parse_shell_lc_single_command_prefix; use codex_utils_absolute_path::AbsolutePathBuf; use shlex::try_join as shlex_try_join; +mod model_policy; + +pub(crate) use model_policy::AllowPrefixRules; + const PROMPT_CONFLICT_REASON: &str = "approval required by policy, but AskForApproval is set to Never"; const REJECT_SANDBOX_APPROVAL_REASON: &str = @@ -281,6 +285,7 @@ pub(crate) struct ExecApprovalRequest<'a> { pub(crate) windows_sandbox_level: WindowsSandboxLevel, pub(crate) sandbox_permissions: SandboxPermissions, pub(crate) prefix_rule: Option>, + pub(crate) allow_prefix_rules: AllowPrefixRules, } impl ExecPolicyManager { @@ -315,17 +320,19 @@ impl ExecPolicyManager { windows_sandbox_level, sandbox_permissions, prefix_rule, + allow_prefix_rules, } = req; - let exec_policy = self.current(); + let exec_policy = self.current_for_prefix_rules(allow_prefix_rules); let ExecPolicyCommands { commands, used_complex_parsing, command_origin, } = commands_for_exec_policy(command); - // Keep heredoc prefix parsing for rule evaluation so existing - // allow/prompt/forbidden rules still apply, but avoid auto-derived - // amendments when only the heredoc fallback parser matched. - let auto_amendment_allowed = !used_complex_parsing; + // Keep heredoc prefix parsing for the rules that apply to this model, + // but avoid reusable approvals for cyber models or when only the + // heredoc fallback parser matched. + let auto_amendment_allowed = + !used_complex_parsing && allow_prefix_rules == AllowPrefixRules::Honor; let exec_policy_fallback = |cmd: &[String]| { render_decision_for_unmatched_command( cmd, diff --git a/codex-rs/core/src/exec_policy/model_policy.rs b/codex-rs/core/src/exec_policy/model_policy.rs new file mode 100644 index 0000000000..c1c2df1938 --- /dev/null +++ b/codex-rs/core/src/exec_policy/model_policy.rs @@ -0,0 +1,47 @@ +use super::ExecPolicyManager; +use codex_execpolicy::Decision; +use codex_execpolicy::Policy; +use codex_execpolicy::PrefixRule; +use std::sync::Arc; + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(crate) enum AllowPrefixRules { + Honor, + IgnoreForCyberModel, +} + +impl ExecPolicyManager { + pub(crate) fn current_for_prefix_rules( + &self, + allow_prefix_rules: AllowPrefixRules, + ) -> Arc { + let policy = self.current(); + if allow_prefix_rules == AllowPrefixRules::Honor { + return policy; + } + + let rules = policy + .rules() + .iter_all() + .flat_map(|(program, rules)| { + rules.iter().filter_map(move |rule| { + let is_allow_prefix = rule + .as_any() + .downcast_ref::() + .is_some_and(|prefix| prefix.decision == Decision::Allow); + (!is_allow_prefix).then(|| (program.clone(), Arc::clone(rule))) + }) + }) + .collect(); + + Arc::new(Policy::from_parts( + rules, + policy.network_rules().to_vec(), + policy.host_executables().clone(), + )) + } +} + +#[cfg(test)] +#[path = "model_policy_tests.rs"] +mod tests; diff --git a/codex-rs/core/src/exec_policy/model_policy_tests.rs b/codex-rs/core/src/exec_policy/model_policy_tests.rs new file mode 100644 index 0000000000..ae103a17ff --- /dev/null +++ b/codex-rs/core/src/exec_policy/model_policy_tests.rs @@ -0,0 +1,177 @@ +use super::AllowPrefixRules; +use super::ExecPolicyManager; +use crate::exec_policy::ExecApprovalRequest; +use crate::sandboxing::SandboxPermissions; +use crate::tools::sandboxing::ExecApprovalRequirement; +use codex_execpolicy::Decision; +use codex_execpolicy::MatchOptions; +use codex_execpolicy::Policy; +use codex_execpolicy::PolicyParser; +use codex_protocol::config_types::WindowsSandboxLevel; +use codex_protocol::models::PermissionProfile; +use codex_protocol::protocol::AskForApproval; +use pretty_assertions::assert_eq; +use std::sync::Arc; + +fn policy_with_broad_allow_prefix() -> (Arc, String) { + let program_name = if cfg!(windows) { "cargo.exe" } else { "cargo" }; + let program_path = std::env::temp_dir() + .join(program_name) + .to_string_lossy() + .into_owned(); + let escaped_program_path = program_path.replace('\\', "\\\\"); + let source = format!( + r#" +host_executable(name="cargo", paths=["{escaped_program_path}"]) +prefix_rule(pattern=["cargo"], decision="allow") +prefix_rule(pattern=["cargo", "publish"], decision="prompt") +prefix_rule(pattern=["rm"], decision="forbidden") +network_rule(host="example.com", protocol="https", decision="allow") +"# + ); + let mut parser = PolicyParser::new(); + parser.parse("test.rules", &source).expect("parse policy"); + (Arc::new(parser.build()), program_path) +} + +#[test] +fn cyber_policy_filters_allow_prefixes_but_preserves_restrictive_and_network_rules() { + let (policy, program_path) = policy_with_broad_allow_prefix(); + let manager = ExecPolicyManager::new(Arc::clone(&policy)); + + let standard_policy = manager.current_for_prefix_rules(AllowPrefixRules::Honor); + assert!(Arc::ptr_eq(&standard_policy, &policy)); + + let cyber_policy = manager.current_for_prefix_rules(AllowPrefixRules::IgnoreForCyberModel); + assert!(cyber_policy.get_allowed_prefixes().is_empty()); + assert_eq!(cyber_policy.network_rules(), policy.network_rules()); + assert_eq!(cyber_policy.host_executables(), policy.host_executables()); + + let cargo_install = vec!["cargo".to_string(), "install".to_string()]; + assert_eq!( + cyber_policy + .check(&cargo_install, &|_| Decision::Prompt) + .decision, + Decision::Prompt, + ); + + let cargo_publish = vec!["cargo".to_string(), "publish".to_string()]; + assert_eq!( + cyber_policy + .check(&cargo_publish, &|_| Decision::Allow) + .decision, + Decision::Prompt, + ); + + let resolved_cargo_publish = vec![program_path, "publish".to_string()]; + assert_eq!( + cyber_policy + .check_with_options( + &resolved_cargo_publish, + &|_| Decision::Allow, + &MatchOptions { + resolve_host_executables: true, + }, + ) + .decision, + Decision::Prompt, + ); + + let forbidden_command = vec!["rm".to_string(), "target".to_string()]; + assert_eq!( + cyber_policy + .check(&forbidden_command, &|_| Decision::Allow) + .decision, + Decision::Forbidden, + ); +} + +#[tokio::test] +async fn cyber_policy_requires_approval_for_broad_wrapped_and_resolved_prefixes() { + let (policy, program_path) = policy_with_broad_allow_prefix(); + let manager = ExecPolicyManager::new(policy); + let commands = [ + vec![ + "cargo".to_string(), + "install".to_string(), + "example".to_string(), + ], + vec![ + "bash".to_string(), + "-lc".to_string(), + "cargo install example".to_string(), + ], + vec![program_path, "install".to_string(), "example".to_string()], + ]; + + for command in commands { + let requirement = manager + .create_exec_approval_requirement_for_command(ExecApprovalRequest { + command: &command, + approval_policy: AskForApproval::OnRequest, + permission_profile: PermissionProfile::read_only(), + windows_sandbox_level: WindowsSandboxLevel::Disabled, + sandbox_permissions: SandboxPermissions::RequireEscalated, + prefix_rule: Some(vec!["cargo".to_string(), "install".to_string()]), + allow_prefix_rules: AllowPrefixRules::IgnoreForCyberModel, + }) + .await; + + assert_eq!( + requirement, + ExecApprovalRequirement::NeedsApproval { + reason: None, + proposed_execpolicy_amendment: None, + }, + "command {command:?} must not inherit a saved prefix approval", + ); + } +} + +#[tokio::test] +async fn cyber_policy_keeps_heuristically_safe_commands_inside_the_sandbox() { + let mut policy = Policy::empty(); + policy + .add_prefix_rule(&["echo".to_string()], Decision::Allow) + .expect("add broad allow prefix"); + let manager = ExecPolicyManager::new(Arc::new(policy)); + let command = vec!["echo".to_string(), "hello".to_string()]; + + let cyber_requirement = manager + .create_exec_approval_requirement_for_command(ExecApprovalRequest { + command: &command, + approval_policy: AskForApproval::OnRequest, + permission_profile: PermissionProfile::read_only(), + windows_sandbox_level: WindowsSandboxLevel::Disabled, + sandbox_permissions: SandboxPermissions::UseDefault, + prefix_rule: None, + allow_prefix_rules: AllowPrefixRules::IgnoreForCyberModel, + }) + .await; + assert_eq!( + cyber_requirement, + ExecApprovalRequirement::Skip { + bypass_sandbox: false, + proposed_execpolicy_amendment: None, + }, + ); + + let standard_requirement = manager + .create_exec_approval_requirement_for_command(ExecApprovalRequest { + command: &command, + approval_policy: AskForApproval::OnRequest, + permission_profile: PermissionProfile::read_only(), + windows_sandbox_level: WindowsSandboxLevel::Disabled, + sandbox_permissions: SandboxPermissions::UseDefault, + prefix_rule: None, + allow_prefix_rules: AllowPrefixRules::Honor, + }) + .await; + assert_eq!( + standard_requirement, + ExecApprovalRequirement::Skip { + bypass_sandbox: true, + proposed_execpolicy_amendment: None, + }, + ); +} diff --git a/codex-rs/core/src/exec_policy_tests.rs b/codex-rs/core/src/exec_policy_tests.rs index 518cf8d8af..209a34f4b3 100644 --- a/codex-rs/core/src/exec_policy_tests.rs +++ b/codex-rs/core/src/exec_policy_tests.rs @@ -1392,6 +1392,7 @@ async fn mixed_rule_and_sandbox_prompt_prioritizes_rule_for_rejection_decision() windows_sandbox_level: WindowsSandboxLevel::Disabled, sandbox_permissions: SandboxPermissions::RequireEscalated, prefix_rule: None, + allow_prefix_rules: AllowPrefixRules::Honor, }) .await; @@ -1429,6 +1430,7 @@ async fn forced_rm_preserves_rule_rejection_when_granular_rules_are_disabled() { windows_sandbox_level: WindowsSandboxLevel::Disabled, sandbox_permissions: SandboxPermissions::RequireEscalated, prefix_rule: None, + allow_prefix_rules: AllowPrefixRules::Honor, }) .await; @@ -1453,6 +1455,7 @@ async fn exec_approval_requirement_falls_back_to_heuristics() { windows_sandbox_level: WindowsSandboxLevel::Disabled, sandbox_permissions: SandboxPermissions::UseDefault, prefix_rule: None, + allow_prefix_rules: AllowPrefixRules::Honor, }) .await; @@ -1478,6 +1481,7 @@ async fn empty_bash_lc_script_falls_back_to_original_command() { windows_sandbox_level: WindowsSandboxLevel::Disabled, sandbox_permissions: SandboxPermissions::UseDefault, prefix_rule: None, + allow_prefix_rules: AllowPrefixRules::Honor, }) .await; @@ -1507,6 +1511,7 @@ async fn whitespace_bash_lc_script_falls_back_to_original_command() { windows_sandbox_level: WindowsSandboxLevel::Disabled, sandbox_permissions: SandboxPermissions::UseDefault, prefix_rule: None, + allow_prefix_rules: AllowPrefixRules::Honor, }) .await; @@ -1536,6 +1541,7 @@ async fn request_rule_uses_prefix_rule() { windows_sandbox_level: WindowsSandboxLevel::Disabled, sandbox_permissions: SandboxPermissions::RequireEscalated, prefix_rule: Some(vec!["cargo".to_string(), "install".to_string()]), + allow_prefix_rules: AllowPrefixRules::Honor, }) .await; @@ -1568,6 +1574,7 @@ async fn request_rule_falls_back_when_prefix_rule_does_not_approve_all_commands( windows_sandbox_level: WindowsSandboxLevel::Disabled, sandbox_permissions: SandboxPermissions::RequireEscalated, prefix_rule: Some(vec!["cargo".to_string(), "install".to_string()]), + allow_prefix_rules: AllowPrefixRules::Honor, }) .await; @@ -1607,6 +1614,7 @@ async fn heuristics_apply_when_other_commands_match_policy() { windows_sandbox_level: WindowsSandboxLevel::Disabled, sandbox_permissions: SandboxPermissions::UseDefault, prefix_rule: None, + allow_prefix_rules: AllowPrefixRules::Honor, }) .await, ExecApprovalRequirement::NeedsApproval { @@ -2085,6 +2093,7 @@ async fn forced_rm_requires_approval_or_specific_rejection_on_all_platforms() { windows_sandbox_level: WindowsSandboxLevel::Disabled, sandbox_permissions: permissions, prefix_rule: None, + allow_prefix_rules: AllowPrefixRules::Honor, }) .await, r#"On all platforms, a forbidden command should require approval @@ -2106,6 +2115,7 @@ async fn forced_rm_requires_approval_or_specific_rejection_on_all_platforms() { windows_sandbox_level: WindowsSandboxLevel::Disabled, sandbox_permissions: permissions, prefix_rule: None, + allow_prefix_rules: AllowPrefixRules::Honor, }) .await, r#"On all platforms, a forbidden command should require approval @@ -2164,6 +2174,7 @@ async fn verify_approval_requirement_for_unsafe_powershell_command() { windows_sandbox_level: WindowsSandboxLevel::Disabled, sandbox_permissions: permissions, prefix_rule: None, + allow_prefix_rules: AllowPrefixRules::Honor, }) .await, "{pwsh_approval_reason}" @@ -2256,6 +2267,7 @@ async fn exec_approval_requirement_for_command( windows_sandbox_level: WindowsSandboxLevel::RestrictedToken, sandbox_permissions, prefix_rule, + allow_prefix_rules: AllowPrefixRules::Honor, }) .await } diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 2ef1af21ba..7ebe44e3ee 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -11599,6 +11599,7 @@ async fn rejects_escalated_permissions_when_policy_not_on_request() { windows_sandbox_level: turn_context.windows_sandbox_level, sandbox_permissions: SandboxPermissions::UseDefault, prefix_rule: None, + allow_prefix_rules: turn_context.allow_prefix_rules(), }) .await; assert!(matches!( diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index b901b4c602..531285e9e4 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -1,5 +1,6 @@ use super::*; use crate::environment_selection::TurnEnvironmentSnapshot; +use crate::exec_policy::AllowPrefixRules; use crate::shell_snapshot::ShellSnapshotFile; use codex_core_plugins::PluginCommandAttribution; use codex_core_plugins::TrustedPluginRoots; @@ -8,6 +9,7 @@ use codex_model_provider::SharedModelProvider; use codex_protocol::SessionId; use codex_protocol::ThreadId; use codex_protocol::models::AdditionalPermissionProfile; +use codex_protocol::openai_models::MODEL_SPECIALTY_CYBER; use codex_protocol::openai_models::ModelInfo; use codex_protocol::protocol::ErrorEvent; use codex_protocol::protocol::MultiAgentVersion; @@ -206,6 +208,23 @@ impl TurnContext { self.config.permissions.approval_policy.value() } + pub(crate) fn allow_prefix_rules(&self) -> AllowPrefixRules { + let ignore_rules = self + .config + .config_layer_stack + .requirements_toml() + .auto_review + .as_ref() + .and_then(|auto_review| auto_review.ignore_rules.as_ref()) + .is_some_and(|models| models.contains(&self.model_info.slug)); + if self.model_info.model_specialty.as_deref() == Some(MODEL_SPECIALTY_CYBER) || ignore_rules + { + AllowPrefixRules::IgnoreForCyberModel + } else { + AllowPrefixRules::Honor + } + } + pub(crate) fn permission_profile(&self) -> PermissionProfile { self.config.permissions.effective_permission_profile() } diff --git a/codex-rs/core/src/session/world_state.rs b/codex-rs/core/src/session/world_state.rs index 2800edca11..6e72a69135 100644 --- a/codex-rs/core/src/session/world_state.rs +++ b/codex-rs/core/src/session/world_state.rs @@ -142,6 +142,10 @@ impl Session { .and_then(|instructions| instructions.end.as_deref()), )); world_state.add_section(AgentsMdState::new(step_context.loaded_agents_md.as_deref())); + let exec_policy = self + .services + .exec_policy + .current_for_prefix_rules(turn_context.allow_prefix_rules()); if turn_context.config.include_permissions_instructions { let environment = step_context.environments.primary(); let permission_profile = environment @@ -162,7 +166,6 @@ impl Session { .and_then(|environment| environment.cwd().to_abs_path().ok()) .unwrap_or_else(|| turn_context.cwd.clone()); let model_messages = turn_context.model_info.model_messages.as_ref(); - let exec_policy = self.services.exec_policy.current(); world_state.add_section(PermissionsState::new( &permission_profile, turn_context.approval_policy(), @@ -183,7 +186,6 @@ impl Session { .enabled(Feature::RequestPermissionsTool), )); } else { - let exec_policy = self.services.exec_policy.current(); world_state.add_section(CompactPermissionsState::new(exec_policy.as_ref())); } if turn_context.config.include_collaboration_mode_instructions { diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index 7275082989..20c6a5a359 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -186,6 +186,7 @@ async fn run_exec_like(args: RunExecLikeArgs) -> Result Result { + let (tool_name, command_key, timeout_key) = match command_tool { + CommandTool::ShellCommand => ("shell_command", "command", "timeout_ms"), + CommandTool::UnifiedExec => ("exec_command", "cmd", "yield_time_ms"), + }; + let mut args = json!({ + "sandbox_permissions": SandboxPermissions::RequireEscalated, + "justification": "Check whether a saved prefix bypasses Guardian.", + "prefix_rule": ["git", "version"], + }); + args[command_key] = json!(TEST_COMMAND); + args[timeout_key] = json!(1_000); + + Ok(sse(vec![ + ev_response_created(response_id), + ev_function_call(call_id, tool_name, &serde_json::to_string(&args)?), + ev_completed(response_id), + ])) +} + +fn guardian_allow_response(response_id: &str) -> String { + sse(vec![ + ev_response_created(response_id), + ev_assistant_message(&format!("{response_id}-message"), r#"{"outcome":"allow"}"#), + ev_completed(response_id), + ]) +} + +async fn submit_model_turn(test: &TestCodex, model: &str, prompt: &str) -> Result<()> { + submit_thread_settings( + &test.codex, + ThreadSettingsOverrides { + model: Some(model.to_string()), + approval_policy: Some(AskForApproval::OnRequest), + approvals_reviewer: Some(ApprovalsReviewer::AutoReview), + ..Default::default() + }, + ) + .await?; + test.submit_text_turn(prompt).await +} + +#[test_case(CommandTool::ShellCommand, ModelSpecialty::Cyber, ShellBackend::Standard; "cyber shell command is reviewed")] +#[test_case(CommandTool::UnifiedExec, ModelSpecialty::Cyber, ShellBackend::Standard; "cyber unified exec is reviewed")] +#[test_case(CommandTool::ShellCommand, ModelSpecialty::Cyber, ShellBackend::ZshFork; "cyber zsh shell command is reviewed")] +#[test_case(CommandTool::UnifiedExec, ModelSpecialty::Cyber, ShellBackend::ZshFork; "cyber zsh unified exec is reviewed")] +#[test_case(CommandTool::ShellCommand, ModelSpecialty::General, ShellBackend::Standard; "general shell command keeps saved approval")] +#[test_case(CommandTool::UnifiedExec, ModelSpecialty::General, ShellBackend::Standard; "general unified exec keeps saved approval")] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn saved_prefix_only_bypasses_guardian_for_general_models( + command_tool: CommandTool, + model_specialty: ModelSpecialty, + shell_backend: ShellBackend, +) -> Result<()> { + skip_if_no_network!(Ok(())); + skip_if_sandbox!(Ok(())); + skip_if_wine_exec!(Ok(()), "Guardian command reviews require host-native paths"); + + let server = start_mock_server().await; + let builder = match shell_backend { + ShellBackend::Standard => test_codex(), + ShellBackend::ZshFork => { + skip_if_host_windows!(Ok(())); + let Some(runtime) = zsh_fork_runtime("cyber model zsh-fork saved prefix")? else { + return Ok(()); + }; + zsh_fork_test_builder(runtime, AskForApproval::OnRequest) + } + }; + let mut builder = builder + .with_model_info_override("gpt-5.4", move |model| { + if model_specialty == ModelSpecialty::Cyber { + model.model_specialty = Some(MODEL_SPECIALTY_CYBER.to_string()); + } + }) + .with_config(move |config| { + configure_saved_prefix_and_guardian(config); + command_tool.configure(config); + if shell_backend == ShellBackend::ZshFork && command_tool == CommandTool::UnifiedExec { + config + .features + .enable(Feature::UnifiedExecZshFork) + .expect("enable unified-exec zsh fork"); + } + }); + let test = builder.build_with_auto_env(&server).await?; + let expected_guardian_review_count = match (model_specialty, shell_backend) { + (ModelSpecialty::General, _) => 0, + (ModelSpecialty::Cyber, ShellBackend::Standard) => 1, + (ModelSpecialty::Cyber, ShellBackend::ZshFork) => 2, + }; + + let mut response_bodies = vec![command_response( + "parent-saved-prefix-command", + "saved-prefix-command", + command_tool, + )?]; + for review_index in 0..expected_guardian_review_count { + response_bodies.push(guardian_allow_response(&format!( + "guardian-saved-prefix-review-{review_index}" + ))); + } + response_bodies.push(sse_completed("parent-saved-prefix-complete")); + let responses = mount_sse_sequence(&server, response_bodies).await; + + submit_model_turn(&test, "gpt-5.4", "run the saved-prefix command").await?; + + let requests = responses.requests(); + let guardian_request_count = requests + .iter() + .filter(|request| { + request.body_json()["client_metadata"]["x-openai-subagent"].as_str() == Some("guardian") + }) + .count(); + assert_eq!(guardian_request_count, expected_guardian_review_count); + + let advertised_saved_prefix = requests[0] + .message_input_texts("developer") + .iter() + .any(|message| message.contains(SAVED_PREFIX)); + assert_eq!( + advertised_saved_prefix, + model_specialty == ModelSpecialty::General, + ); + assert!( + responses + .function_call_output_text("saved-prefix-command") + .is_some_and(|output| output.contains(TEST_COMMAND)), + "approved test command should execute", + ); + + Ok(()) +} + +#[test_case(CommandTool::ShellCommand; "shell command")] +#[test_case(CommandTool::UnifiedExec; "unified exec")] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn cyber_model_user_approval_never_offers_a_reusable_prefix( + command_tool: CommandTool, +) -> Result<()> { + skip_if_no_network!(Ok(())); + skip_if_sandbox!(Ok(())); + skip_if_wine_exec!(Ok(()), "command approval requires host-native paths"); + + let server = start_mock_server().await; + let mut builder = test_codex() + .with_model_info_override("gpt-5.4", |model| { + model.model_specialty = Some(MODEL_SPECIALTY_CYBER.to_string()); + }) + .with_config(move |config| { + configure_saved_prefix_and_guardian(config); + command_tool.configure(config); + config.approvals_reviewer = ApprovalsReviewer::User; + }); + let test = builder.build_with_auto_env(&server).await?; + let policy_path = test.codex_home_path().join("rules/default.rules"); + let initial_policy = fs::read_to_string(&policy_path)?; + let responses = mount_sse_sequence( + &server, + vec![ + command_response( + "parent-one-time-approval", + "one-time-approval", + command_tool, + )?, + sse_completed("parent-one-time-complete"), + ], + ) + .await; + + test.codex + .submit(Op::UserInput { + items: vec![UserInput::Text { + text: "run a command with one-time approval".to_string(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + responsesapi_client_metadata: None, + additional_context: Default::default(), + thread_settings: ThreadSettingsOverrides { + approval_policy: Some(AskForApproval::OnRequest), + approvals_reviewer: Some(ApprovalsReviewer::User), + ..Default::default() + }, + }) + .await?; + + let EventMsg::ExecApprovalRequest(approval) = wait_for_event(&test.codex, |event| { + matches!( + event, + EventMsg::ExecApprovalRequest(_) | EventMsg::TurnComplete(_) + ) + }) + .await + else { + panic!("expected a one-time command approval request"); + }; + assert_eq!(approval.proposed_execpolicy_amendment, None); + assert_eq!( + approval.effective_available_decisions(), + vec![ReviewDecision::Approved, ReviewDecision::Abort], + ); + test.codex + .submit(Op::ExecApproval { + id: approval.effective_approval_id(), + turn_id: None, + decision: ReviewDecision::Approved, + }) + .await?; + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + + assert!( + responses + .function_call_output_text("one-time-approval") + .is_some_and(|output| output.contains(TEST_COMMAND)), + ); + assert_eq!(fs::read_to_string(policy_path)?, initial_policy); + assert_eq!(responses.requests().len(), 2); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn switching_models_suppresses_and_restores_saved_prefix_approvals() -> Result<()> { + skip_if_no_network!(Ok(())); + skip_if_sandbox!(Ok(())); + skip_if_wine_exec!(Ok(()), "Guardian command reviews require host-native paths"); + + let server = start_mock_server().await; + let mut builder = test_codex() + .with_model_info_override("gpt-5.4", |model| { + model.model_specialty = Some(MODEL_SPECIALTY_CYBER.to_string()); + }) + .with_model("gpt-5.2") + .with_config(configure_saved_prefix_and_guardian); + let test = builder.build_with_auto_env(&server).await?; + + let responses = mount_sse_sequence( + &server, + vec![ + command_response( + "parent-general-first-command", + "general-first-command", + CommandTool::ShellCommand, + )?, + sse_completed("parent-general-first-complete"), + command_response( + "parent-cyber-command", + "cyber-command", + CommandTool::ShellCommand, + )?, + guardian_allow_response("guardian-cyber-review"), + sse_completed("parent-cyber-complete"), + command_response( + "parent-general-last-command", + "general-last-command", + CommandTool::ShellCommand, + )?, + sse_completed("parent-general-last-complete"), + ], + ) + .await; + + submit_model_turn(&test, "gpt-5.2", "run the first general-model command").await?; + submit_model_turn(&test, "gpt-5.4", "run the cyber-model command").await?; + submit_model_turn(&test, "gpt-5.2", "run the final general-model command").await?; + + let requests = responses.requests(); + let guardian_requests = requests + .iter() + .filter(|request| { + request.body_json()["client_metadata"]["x-openai-subagent"].as_str() == Some("guardian") + }) + .collect::>(); + assert_eq!(guardian_requests.len(), 1); + assert!(guardian_requests[0].body_contains_text(TEST_COMMAND)); + + let cyber_request = requests + .iter() + .find(|request| request.body_json()["model"] == "gpt-5.4") + .expect("cyber-model request"); + let cyber_developer_messages = cyber_request.message_input_texts("developer"); + let cyber_permissions = cyber_developer_messages + .iter() + .rev() + .find(|message| message.contains("")) + .expect("refreshed cyber-model permissions instructions"); + assert!(!cyber_permissions.contains(SAVED_PREFIX)); + + let final_parent_request = requests + .iter() + .rev() + .find(|request| request.body_json()["model"] == "gpt-5.2") + .expect("final general-model request"); + assert!( + final_parent_request + .message_input_texts("developer") + .iter() + .any(|message| { + message.contains("Approved command prefix saved:") && message.contains(SAVED_PREFIX) + }), + "switching back should restore the original saved-prefix context", + ); + + for call_id in [ + "general-first-command", + "cyber-command", + "general-last-command", + ] { + assert!( + responses + .function_call_output_text(call_id) + .is_some_and(|output| output.contains(TEST_COMMAND)), + "approved {call_id} should execute", + ); + } + + Ok(()) +} diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index a1668778e4..0cb63eec73 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -57,6 +57,7 @@ mod compact_remote; mod compact_remote_parity; mod compact_resume_fork; mod current_time_reminder; +mod cyber_exec_policy; mod deprecation_notice; mod exec; mod exec_policy;