From e808f90724c0d20ed628ee2fcc38f61cf07c4228 Mon Sep 17 00:00:00 2001 From: rreichel3-oai Date: Thu, 19 Mar 2026 15:15:27 -0400 Subject: [PATCH] Gate enhanced execpolicy suggestions behind feature config In codex-rs/core/src/features.rs and codex-rs/core/config.schema.json, add the enhanced_exec_policy_suggestions feature flag as a disabled-by-default under-development config. In codex-rs/core/src/exec_policy.rs, codex-rs/core/src/tools/handlers/shell.rs, and codex-rs/core/src/unified_exec/process_manager.rs, route auto-generated execpolicy suggestion behavior through the new feature gate while preserving legacy suggestions when it is off. In codex-rs/core/src/exec_policy_tests.rs, cover both the legacy default-off behavior and the enhanced suggestion path explicitly. --- codex-rs/core/config.schema.json | 6 ++ codex-rs/core/src/exec_policy.rs | 82 +++++++++++--- codex-rs/core/src/exec_policy_tests.rs | 101 +++++++++++++----- codex-rs/core/src/features.rs | 8 ++ codex-rs/core/src/tools/handlers/shell.rs | 28 +++-- .../core/src/unified_exec/process_manager.rs | 29 +++-- 6 files changed, 191 insertions(+), 63 deletions(-) diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 056b4c4b79..597bf854eb 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -380,6 +380,9 @@ "enable_request_compression": { "type": "boolean" }, + "enhanced_exec_policy_suggestions": { + "type": "boolean" + }, "exec_permission_approvals": { "type": "boolean" }, @@ -1986,6 +1989,9 @@ "enable_request_compression": { "type": "boolean" }, + "enhanced_exec_policy_suggestions": { + "type": "boolean" + }, "exec_permission_approvals": { "type": "boolean" }, diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 733e9dd4ce..5104c0bb6e 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -223,9 +223,33 @@ impl ExecPolicyManager { self.policy.load_full() } + #[cfg(test)] pub(crate) async fn create_exec_approval_requirement_for_command( &self, req: ExecApprovalRequest<'_>, + ) -> ExecApprovalRequirement { + self.create_exec_approval_requirement_for_command_impl( + req, /*enhanced_exec_policy_suggestions*/ false, + ) + .await + } + + pub(crate) async fn create_exec_approval_requirement_for_command_with_enhanced_suggestions( + &self, + req: ExecApprovalRequest<'_>, + enhanced_exec_policy_suggestions: bool, + ) -> ExecApprovalRequirement { + self.create_exec_approval_requirement_for_command_impl( + req, + enhanced_exec_policy_suggestions, + ) + .await + } + + async fn create_exec_approval_requirement_for_command_impl( + &self, + req: ExecApprovalRequest<'_>, + enhanced_exec_policy_suggestions: bool, ) -> ExecApprovalRequirement { let ExecApprovalRequest { command, @@ -288,6 +312,7 @@ impl ExecPolicyManager { try_derive_execpolicy_amendment_for_prompt_rules( &evaluation.matched_rules, &commands, + enhanced_exec_policy_suggestions, ) } else { None @@ -305,6 +330,7 @@ impl ExecPolicyManager { try_derive_execpolicy_amendment_for_allow_rules( &evaluation.matched_rules, &commands, + enhanced_exec_policy_suggestions, ) } else { None @@ -644,13 +670,15 @@ fn commands_for_exec_policy(command: &[String]) -> (Vec>, bool) { /// - Examples: /// - execpolicy: empty. Command: `["python"]`. Heuristics prompt -> `Some(vec!["python"])`. /// - execpolicy: empty. Command: `["bash", "-c", "cd /some/folder && prog1 --option1 arg1 && prog2 --option2 arg2"]`. -/// Parsed commands include `cd /some/folder`, `prog1 --option1 arg1`, and `prog2 --option2 arg2`. For multi-command scripts, -/// we derive the suggestion from the first parsed segment, so this returns `Some(vec!["cd", "/some/folder"])`. +/// Parsed commands include `cd /some/folder`, `prog1 --option1 arg1`, and `prog2 --option2 arg2`. In enhanced mode for +/// multi-command scripts, we derive the suggestion from the first parsed segment, so this returns +/// `Some(vec!["cd", "/some/folder"])`. /// - execpolicy: contains a `prompt for prefix ["prog2"]` rule. For the same command as above, /// we return `None` because an execpolicy prompt still applies even if we amend execpolicy to allow ["cd", "/some/folder"]. fn try_derive_execpolicy_amendment_for_prompt_rules( matched_rules: &[RuleMatch], commands: &[Vec], + enhanced_exec_policy_suggestions: bool, ) -> Option { if matched_rules .iter() @@ -659,8 +687,11 @@ fn try_derive_execpolicy_amendment_for_prompt_rules( return None; } - if commands.len() > 1 { - return auto_derived_execpolicy_amendment(&commands[0]); + if enhanced_exec_policy_suggestions && commands.len() > 1 { + return auto_derived_execpolicy_amendment_for_mode( + &commands[0], + /*enhanced_exec_policy_suggestions*/ true, + ); } matched_rules @@ -669,7 +700,10 @@ fn try_derive_execpolicy_amendment_for_prompt_rules( RuleMatch::HeuristicsRuleMatch { command, decision: Decision::Prompt, - } => auto_derived_execpolicy_amendment(command), + } => auto_derived_execpolicy_amendment_for_mode( + command, + enhanced_exec_policy_suggestions, + ), _ => None, }) } @@ -680,13 +714,17 @@ fn try_derive_execpolicy_amendment_for_prompt_rules( fn try_derive_execpolicy_amendment_for_allow_rules( matched_rules: &[RuleMatch], commands: &[Vec], + enhanced_exec_policy_suggestions: bool, ) -> Option { if matched_rules.iter().any(is_policy_match) { return None; } - if commands.len() > 1 { - return auto_derived_execpolicy_amendment(&commands[0]); + if enhanced_exec_policy_suggestions && commands.len() > 1 { + return auto_derived_execpolicy_amendment_for_mode( + &commands[0], + /*enhanced_exec_policy_suggestions*/ true, + ); } matched_rules @@ -695,18 +733,32 @@ fn try_derive_execpolicy_amendment_for_allow_rules( RuleMatch::HeuristicsRuleMatch { command, decision: Decision::Allow, - } => auto_derived_execpolicy_amendment(command), + } => auto_derived_execpolicy_amendment_for_mode( + command, + enhanced_exec_policy_suggestions, + ), _ => None, }) } -/// Keep generated execpolicy suggestions broad enough to cover similar -/// invocations, but stop before the first flag so we do not bake incidental -/// option values into a persisted allow rule. +/// In enhanced mode, keep generated execpolicy suggestions broad enough to +/// cover similar invocations, but stop before the first flag so we do not bake +/// incidental option values into a persisted allow rule. /// /// If truncating before the first flag would leave fewer than two tokens, fall /// back to the whole command segment. -fn auto_derived_execpolicy_amendment(command: &[String]) -> Option { +fn auto_derived_execpolicy_amendment_for_mode( + command: &[String], + enhanced_exec_policy_suggestions: bool, +) -> Option { + if command.is_empty() { + return None; + } + + if !enhanced_exec_policy_suggestions { + return Some(ExecPolicyAmendment::from(command.to_vec())); + } + let prefix: Vec = command .iter() .take_while(|token| !token.starts_with('-')) @@ -717,11 +769,7 @@ fn auto_derived_execpolicy_amendment(command: &[String]) -> Option FileSystemSandboxPolicy { FileSystemSandboxPolicy::unrestricted() } +async fn create_exec_approval_requirement_with_enhanced_suggestions( + manager: &ExecPolicyManager, + req: ExecApprovalRequest<'_>, +) -> ExecApprovalRequirement { + manager + .create_exec_approval_requirement_for_command_with_enhanced_suggestions( + req, /*enhanced_exec_policy_suggestions*/ true, + ) + .await +} + async fn test_config() -> (TempDir, Config) { let home = TempDir::new().expect("create temp dir"); let config = ConfigBuilder::default() @@ -1127,16 +1138,18 @@ async fn request_rule_falls_back_to_first_segment_for_multi_command_scripts() { ]; let manager = ExecPolicyManager::default(); - let requirement = manager - .create_exec_approval_requirement_for_command(ExecApprovalRequest { + let requirement = create_exec_approval_requirement_with_enhanced_suggestions( + &manager, + ExecApprovalRequest { command: &command, approval_policy: AskForApproval::OnRequest, sandbox_policy: &SandboxPolicy::DangerFullAccess, file_system_sandbox_policy: &unrestricted_file_system_sandbox_policy(), sandbox_permissions: SandboxPermissions::RequireEscalated, prefix_rule: Some(vec!["cargo".to_string(), "install".to_string()]), - }) - .await; + }, + ) + .await; assert_eq!( requirement, @@ -1166,16 +1179,18 @@ async fn heuristics_apply_when_other_commands_match_policy() { ]; assert_eq!( - ExecPolicyManager::new(policy) - .create_exec_approval_requirement_for_command(ExecApprovalRequest { + create_exec_approval_requirement_with_enhanced_suggestions( + &ExecPolicyManager::new(policy), + ExecApprovalRequest { command: &command, approval_policy: AskForApproval::UnlessTrusted, sandbox_policy: &SandboxPolicy::DangerFullAccess, file_system_sandbox_policy: &unrestricted_file_system_sandbox_policy(), sandbox_permissions: SandboxPermissions::UseDefault, prefix_rule: None, - }) - .await, + }, + ) + .await, ExecApprovalRequirement::NeedsApproval { reason: None, proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![ @@ -1270,6 +1285,41 @@ async fn proposed_execpolicy_amendment_stops_before_first_flag_for_generated_sug "codex-core".to_string(), ]; + let manager = ExecPolicyManager::default(); + let requirement = create_exec_approval_requirement_with_enhanced_suggestions( + &manager, + ExecApprovalRequest { + command: &command, + approval_policy: AskForApproval::UnlessTrusted, + sandbox_policy: &SandboxPolicy::new_read_only_policy(), + file_system_sandbox_policy: &read_only_file_system_sandbox_policy(), + sandbox_permissions: SandboxPermissions::UseDefault, + prefix_rule: None, + }, + ) + .await; + + assert_eq!( + requirement, + ExecApprovalRequirement::NeedsApproval { + reason: None, + proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![ + "cargo".to_string(), + "test".to_string(), + ])) + } + ); +} + +#[tokio::test] +async fn proposed_execpolicy_amendment_preserves_full_command_when_enhanced_suggestions_disabled() { + let command = vec![ + "cargo".to_string(), + "test".to_string(), + "--package".to_string(), + "codex-core".to_string(), + ]; + let manager = ExecPolicyManager::default(); let requirement = manager .create_exec_approval_requirement_for_command(ExecApprovalRequest { @@ -1286,10 +1336,7 @@ async fn proposed_execpolicy_amendment_stops_before_first_flag_for_generated_sug requirement, ExecApprovalRequirement::NeedsApproval { reason: None, - proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![ - "cargo".to_string(), - "test".to_string(), - ])) + proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)) } ); } @@ -1299,16 +1346,18 @@ async fn proposed_execpolicy_amendment_falls_back_to_whole_command_if_flag_start let command = vec!["curl".to_string(), "-k".to_string(), "xyz.com".to_string()]; let manager = ExecPolicyManager::default(); - let requirement = manager - .create_exec_approval_requirement_for_command(ExecApprovalRequest { + let requirement = create_exec_approval_requirement_with_enhanced_suggestions( + &manager, + ExecApprovalRequest { command: &command, approval_policy: AskForApproval::UnlessTrusted, sandbox_policy: &SandboxPolicy::new_read_only_policy(), file_system_sandbox_policy: &read_only_file_system_sandbox_policy(), sandbox_permissions: SandboxPermissions::UseDefault, prefix_rule: None, - }) - .await; + }, + ) + .await; assert_eq!( requirement, @@ -1358,16 +1407,18 @@ async fn proposed_execpolicy_amendment_is_based_on_first_segment_for_multi_comma "cargo build && echo ok".to_string(), ]; let manager = ExecPolicyManager::default(); - let requirement = manager - .create_exec_approval_requirement_for_command(ExecApprovalRequest { + let requirement = create_exec_approval_requirement_with_enhanced_suggestions( + &manager, + ExecApprovalRequest { command: &command, approval_policy: AskForApproval::UnlessTrusted, sandbox_policy: &SandboxPolicy::new_read_only_policy(), file_system_sandbox_policy: &read_only_file_system_sandbox_policy(), sandbox_permissions: SandboxPermissions::UseDefault, prefix_rule: None, - }) - .await; + }, + ) + .await; assert_eq!( requirement, @@ -1398,16 +1449,18 @@ async fn proposed_execpolicy_amendment_uses_whole_one_token_first_segment_for_mu ]; assert_eq!( - ExecPolicyManager::new(policy) - .create_exec_approval_requirement_for_command(ExecApprovalRequest { + create_exec_approval_requirement_with_enhanced_suggestions( + &ExecPolicyManager::new(policy), + ExecApprovalRequest { command: &command, approval_policy: AskForApproval::UnlessTrusted, sandbox_policy: &SandboxPolicy::new_read_only_policy(), file_system_sandbox_policy: &read_only_file_system_sandbox_policy(), sandbox_permissions: SandboxPermissions::UseDefault, prefix_rule: None, - }) - .await, + }, + ) + .await, ExecApprovalRequirement::NeedsApproval { reason: None, proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec!["cat".to_string()])), diff --git a/codex-rs/core/src/features.rs b/codex-rs/core/src/features.rs index bcd064302b..81079439c9 100644 --- a/codex-rs/core/src/features.rs +++ b/codex-rs/core/src/features.rs @@ -118,6 +118,8 @@ pub enum Feature { UseLegacyLandlock, /// Allow the model to request approval and propose exec rules. RequestRule, + /// Use tighter generated execpolicy prefix suggestions for approval prompts. + EnhancedExecPolicySuggestions, /// Enable Windows sandbox (restricted token) on Windows. WindowsSandbox, /// Use the elevated Windows sandbox pipeline (setup + runner). @@ -677,6 +679,12 @@ pub const FEATURES: &[FeatureSpec] = &[ stage: Stage::Removed, default_enabled: false, }, + FeatureSpec { + id: Feature::EnhancedExecPolicySuggestions, + key: "enhanced_exec_policy_suggestions", + stage: Stage::UnderDevelopment, + default_enabled: false, + }, FeatureSpec { id: Feature::WindowsSandbox, key: "experimental_windows_sandbox", diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index 04b5c77c37..f780064656 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -428,18 +428,24 @@ impl ShellHandler { let exec_approval_requirement = session .services .exec_policy - .create_exec_approval_requirement_for_command(ExecApprovalRequest { - command: &exec_params.command, - approval_policy: turn.approval_policy.value(), - sandbox_policy: turn.sandbox_policy.get(), - file_system_sandbox_policy: &turn.file_system_sandbox_policy, - sandbox_permissions: if effective_additional_permissions.permissions_preapproved { - codex_protocol::models::SandboxPermissions::UseDefault - } else { - effective_additional_permissions.sandbox_permissions + .create_exec_approval_requirement_for_command_with_enhanced_suggestions( + ExecApprovalRequest { + command: &exec_params.command, + approval_policy: turn.approval_policy.value(), + sandbox_policy: turn.sandbox_policy.get(), + file_system_sandbox_policy: &turn.file_system_sandbox_policy, + sandbox_permissions: if effective_additional_permissions.permissions_preapproved + { + codex_protocol::models::SandboxPermissions::UseDefault + } else { + effective_additional_permissions.sandbox_permissions + }, + prefix_rule, }, - prefix_rule, - }) + session + .features() + .enabled(Feature::EnhancedExecPolicySuggestions), + ) .await; let req = ShellRequest { diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index 52d668c000..941388a500 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -15,6 +15,7 @@ use tokio_util::sync::CancellationToken; use crate::exec_env::create_env; use crate::exec_policy::ExecApprovalRequest; +use crate::features::Feature; use crate::protocol::ExecCommandSource; use crate::sandboxing::ExecRequest; use crate::tools::context::ExecCommandToolOutput; @@ -596,18 +597,24 @@ impl UnifiedExecProcessManager { .session .services .exec_policy - .create_exec_approval_requirement_for_command(ExecApprovalRequest { - command: &request.command, - approval_policy: context.turn.approval_policy.value(), - sandbox_policy: context.turn.sandbox_policy.get(), - file_system_sandbox_policy: &context.turn.file_system_sandbox_policy, - sandbox_permissions: if request.additional_permissions_preapproved { - crate::sandboxing::SandboxPermissions::UseDefault - } else { - request.sandbox_permissions + .create_exec_approval_requirement_for_command_with_enhanced_suggestions( + ExecApprovalRequest { + command: &request.command, + approval_policy: context.turn.approval_policy.value(), + sandbox_policy: context.turn.sandbox_policy.get(), + file_system_sandbox_policy: &context.turn.file_system_sandbox_policy, + sandbox_permissions: if request.additional_permissions_preapproved { + crate::sandboxing::SandboxPermissions::UseDefault + } else { + request.sandbox_permissions + }, + prefix_rule: request.prefix_rule.clone(), }, - prefix_rule: request.prefix_rule.clone(), - }) + context + .session + .features() + .enabled(Feature::EnhancedExecPolicySuggestions), + ) .await; let req = UnifiedExecToolRequest { command: request.command.clone(),