diff --git a/codex-rs/app-server/src/lib.rs b/codex-rs/app-server/src/lib.rs index c558233413..ad049ad305 100644 --- a/codex-rs/app-server/src/lib.rs +++ b/codex-rs/app-server/src/lib.rs @@ -278,9 +278,7 @@ pub async fn run_main_with_transport( } }; - if let Ok(Some(err)) = - check_execpolicy_for_warnings(&config.features, &config.config_layer_stack).await - { + if let Ok(Some(err)) = check_execpolicy_for_warnings(&config.config_layer_stack).await { let (path, range) = exec_policy_warning_location(&err); let message = ConfigWarningNotification { summary: "Error parsing rules; custom rules not applied.".to_string(), diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index ed99fda825..b944a4bfec 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -175,9 +175,6 @@ "enable_request_compression": { "type": "boolean" }, - "exec_policy": { - "type": "boolean" - }, "experimental_use_freeform_apply_patch": { "type": "boolean" }, @@ -1209,9 +1206,6 @@ "enable_request_compression": { "type": "boolean" }, - "exec_policy": { - "type": "boolean" - }, "experimental_use_freeform_apply_patch": { "type": "boolean" }, diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index cd35b256ae..68c99c599c 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -291,7 +291,7 @@ impl Codex { let enabled_skills = loaded_skills.enabled_skills(); let user_instructions = get_user_instructions(&config, Some(&enabled_skills)).await; - let exec_policy = ExecPolicyManager::load(&config.features, &config.config_layer_stack) + let exec_policy = ExecPolicyManager::load(&config.config_layer_stack) .await .map_err(|err| CodexErr::Fatal(format!("failed to load rules: {err}")))?; @@ -1706,7 +1706,6 @@ impl Session { &self, amendment: &ExecPolicyAmendment, ) -> Result<(), ExecPolicyUpdateError> { - let features = self.features.clone(); let codex_home = self .state .lock() @@ -1715,11 +1714,6 @@ impl Session { .codex_home() .clone(); - if !features.enabled(Feature::ExecPolicy) { - error!("attempted to append execpolicy rule while execpolicy feature is disabled"); - return Err(ExecPolicyUpdateError::FeatureDisabled); - } - self.services .exec_policy .append_amendment_and_update(&codex_home, amendment) diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 2ae5a08e4d..5e73fb0735 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -25,8 +25,6 @@ use tokio::fs; use tokio::task::spawn_blocking; use crate::bash::parse_shell_lc_plain_commands; -use crate::features::Feature; -use crate::features::Features; use crate::sandboxing::SandboxPermissions; use crate::tools::sandboxing::ExecApprovalRequirement; use shlex::try_join as shlex_try_join; @@ -78,9 +76,6 @@ pub enum ExecPolicyUpdateError { #[from] source: ExecPolicyRuleError, }, - - #[error("cannot append rule because rules feature is disabled")] - FeatureDisabled, } pub(crate) struct ExecPolicyManager { @@ -88,7 +83,6 @@ pub(crate) struct ExecPolicyManager { } pub(crate) struct ExecApprovalRequest<'a> { - pub(crate) features: &'a Features, pub(crate) command: &'a [String], pub(crate) approval_policy: AskForApproval, pub(crate) sandbox_policy: &'a SandboxPolicy, @@ -103,12 +97,8 @@ impl ExecPolicyManager { } } - pub(crate) async fn load( - features: &Features, - config_stack: &ConfigLayerStack, - ) -> Result { - let (policy, warning) = - load_exec_policy_for_features_with_warning(features, config_stack).await?; + pub(crate) async fn load(config_stack: &ConfigLayerStack) -> Result { + let (policy, warning) = load_exec_policy_with_warning(config_stack).await?; if let Some(err) = warning.as_ref() { tracing::warn!("failed to parse rules: {err}"); } @@ -124,7 +114,6 @@ impl ExecPolicyManager { req: ExecApprovalRequest<'_>, ) -> ExecApprovalRequirement { let ExecApprovalRequest { - features, command, approval_policy, sandbox_policy, @@ -144,11 +133,8 @@ impl ExecPolicyManager { }; let evaluation = exec_policy.check_multiple(commands.iter(), &exec_policy_fallback); - let requested_amendment = derive_requested_execpolicy_amendment( - features, - prefix_rule.as_ref(), - &evaluation.matched_rules, - ); + let requested_amendment = + derive_requested_execpolicy_amendment(prefix_rule.as_ref(), &evaluation.matched_rules); match evaluation.decision { Decision::Forbidden => ExecApprovalRequirement::Forbidden { @@ -162,15 +148,11 @@ impl ExecPolicyManager { } else { ExecApprovalRequirement::NeedsApproval { reason: derive_prompt_reason(command, &evaluation), - proposed_execpolicy_amendment: if features.enabled(Feature::ExecPolicy) { - requested_amendment.or_else(|| { - try_derive_execpolicy_amendment_for_prompt_rules( - &evaluation.matched_rules, - ) - }) - } else { - None - }, + proposed_execpolicy_amendment: requested_amendment.or_else(|| { + try_derive_execpolicy_amendment_for_prompt_rules( + &evaluation.matched_rules, + ) + }), } } } @@ -179,11 +161,9 @@ impl ExecPolicyManager { bypass_sandbox: evaluation.matched_rules.iter().any(|rule_match| { is_policy_match(rule_match) && rule_match.decision() == Decision::Allow }), - proposed_execpolicy_amendment: if features.enabled(Feature::ExecPolicy) { - try_derive_execpolicy_amendment_for_allow_rules(&evaluation.matched_rules) - } else { - None - }, + proposed_execpolicy_amendment: try_derive_execpolicy_amendment_for_allow_rules( + &evaluation.matched_rules, + ), }, } } @@ -221,21 +201,15 @@ impl Default for ExecPolicyManager { } pub async fn check_execpolicy_for_warnings( - features: &Features, config_stack: &ConfigLayerStack, ) -> Result, ExecPolicyError> { - let (_, warning) = load_exec_policy_for_features_with_warning(features, config_stack).await?; + let (_, warning) = load_exec_policy_with_warning(config_stack).await?; Ok(warning) } -async fn load_exec_policy_for_features_with_warning( - features: &Features, +async fn load_exec_policy_with_warning( config_stack: &ConfigLayerStack, ) -> Result<(Policy, Option), ExecPolicyError> { - if !features.enabled(Feature::ExecPolicy) { - return Ok((Policy::empty(), None)); - } - match load_exec_policy(config_stack).await { Ok(policy) => Ok((policy, None)), Err(err @ ExecPolicyError::ParsePolicy { .. }) => Ok((Policy::empty(), Some(err))), @@ -413,14 +387,9 @@ fn try_derive_execpolicy_amendment_for_allow_rules( } fn derive_requested_execpolicy_amendment( - features: &Features, prefix_rule: Option<&Vec>, matched_rules: &[RuleMatch], ) -> Option { - if !features.enabled(Feature::ExecPolicy) { - return None; - } - let prefix_rule = prefix_rule?; if prefix_rule.is_empty() { return None; @@ -589,13 +558,11 @@ mod tests { } #[tokio::test] - async fn returns_empty_policy_when_feature_disabled() { - let mut features = Features::with_defaults(); - features.disable(Feature::ExecPolicy); + async fn returns_empty_policy_when_no_policy_files_exist() { let temp_dir = tempdir().expect("create temp dir"); let config_stack = config_stack_for_dot_codex_folder(temp_dir.path()); - let manager = ExecPolicyManager::load(&features, &config_stack) + let manager = ExecPolicyManager::load(&config_stack) .await .expect("manager result"); let policy = manager.current(); @@ -809,7 +776,6 @@ prefix_rule(pattern=["rm"], decision="forbidden") let manager = ExecPolicyManager::new(policy); let requirement = manager .create_exec_approval_requirement_for_command(ExecApprovalRequest { - features: &Features::with_defaults(), command: &forbidden_script, approval_policy: AskForApproval::OnRequest, sandbox_policy: &SandboxPolicy::DangerFullAccess, @@ -844,7 +810,6 @@ prefix_rule( let manager = ExecPolicyManager::new(policy); let requirement = manager .create_exec_approval_requirement_for_command(ExecApprovalRequest { - features: &Features::with_defaults(), command: &[ "rm".to_string(), "-rf".to_string(), @@ -878,7 +843,6 @@ prefix_rule( let manager = ExecPolicyManager::new(policy); let requirement = manager .create_exec_approval_requirement_for_command(ExecApprovalRequest { - features: &Features::with_defaults(), command: &command, approval_policy: AskForApproval::OnRequest, sandbox_policy: &SandboxPolicy::DangerFullAccess, @@ -909,7 +873,6 @@ prefix_rule( let manager = ExecPolicyManager::new(policy); let requirement = manager .create_exec_approval_requirement_for_command(ExecApprovalRequest { - features: &Features::with_defaults(), command: &command, approval_policy: AskForApproval::Never, sandbox_policy: &SandboxPolicy::DangerFullAccess, @@ -933,7 +896,6 @@ prefix_rule( let manager = ExecPolicyManager::default(); let requirement = manager .create_exec_approval_requirement_for_command(ExecApprovalRequest { - features: &Features::with_defaults(), command: &command, approval_policy: AskForApproval::UnlessTrusted, sandbox_policy: &SandboxPolicy::ReadOnly, @@ -964,7 +926,6 @@ prefix_rule( let requirement = manager .create_exec_approval_requirement_for_command(ExecApprovalRequest { - features: &features, command: &command, approval_policy: AskForApproval::OnRequest, sandbox_policy: &SandboxPolicy::ReadOnly, @@ -1002,7 +963,6 @@ prefix_rule( assert_eq!( ExecPolicyManager::new(policy) .create_exec_approval_requirement_for_command(ExecApprovalRequest { - features: &Features::with_defaults(), command: &command, approval_policy: AskForApproval::UnlessTrusted, sandbox_policy: &SandboxPolicy::DangerFullAccess, @@ -1077,7 +1037,6 @@ prefix_rule( let manager = ExecPolicyManager::default(); let requirement = manager .create_exec_approval_requirement_for_command(ExecApprovalRequest { - features: &Features::with_defaults(), command: &command, approval_policy: AskForApproval::UnlessTrusted, sandbox_policy: &SandboxPolicy::ReadOnly, @@ -1095,34 +1054,6 @@ prefix_rule( ); } - #[tokio::test] - async fn proposed_execpolicy_amendment_is_disabled_when_execpolicy_feature_disabled() { - let command = vec!["cargo".to_string(), "build".to_string()]; - - let mut features = Features::with_defaults(); - features.disable(Feature::ExecPolicy); - - let manager = ExecPolicyManager::default(); - let requirement = manager - .create_exec_approval_requirement_for_command(ExecApprovalRequest { - features: &features, - command: &command, - approval_policy: AskForApproval::UnlessTrusted, - sandbox_policy: &SandboxPolicy::ReadOnly, - sandbox_permissions: SandboxPermissions::UseDefault, - prefix_rule: None, - }) - .await; - - assert_eq!( - requirement, - ExecApprovalRequirement::NeedsApproval { - reason: None, - proposed_execpolicy_amendment: None, - } - ); - } - #[tokio::test] async fn proposed_execpolicy_amendment_is_omitted_when_policy_prompts() { let policy_src = r#"prefix_rule(pattern=["rm"], decision="prompt")"#; @@ -1136,7 +1067,6 @@ prefix_rule( let manager = ExecPolicyManager::new(policy); let requirement = manager .create_exec_approval_requirement_for_command(ExecApprovalRequest { - features: &Features::with_defaults(), command: &command, approval_policy: AskForApproval::OnRequest, sandbox_policy: &SandboxPolicy::DangerFullAccess, @@ -1164,7 +1094,6 @@ prefix_rule( let manager = ExecPolicyManager::default(); let requirement = manager .create_exec_approval_requirement_for_command(ExecApprovalRequest { - features: &Features::with_defaults(), command: &command, approval_policy: AskForApproval::UnlessTrusted, sandbox_policy: &SandboxPolicy::ReadOnly, @@ -1203,7 +1132,6 @@ prefix_rule( assert_eq!( ExecPolicyManager::new(policy) .create_exec_approval_requirement_for_command(ExecApprovalRequest { - features: &Features::with_defaults(), command: &command, approval_policy: AskForApproval::UnlessTrusted, sandbox_policy: &SandboxPolicy::ReadOnly, @@ -1227,7 +1155,6 @@ prefix_rule( let manager = ExecPolicyManager::default(); let requirement = manager .create_exec_approval_requirement_for_command(ExecApprovalRequest { - features: &Features::with_defaults(), command: &command, approval_policy: AskForApproval::OnRequest, sandbox_policy: &SandboxPolicy::ReadOnly, @@ -1258,7 +1185,6 @@ prefix_rule( let manager = ExecPolicyManager::new(policy); let requirement = manager .create_exec_approval_requirement_for_command(ExecApprovalRequest { - features: &Features::with_defaults(), command: &command, approval_policy: AskForApproval::OnRequest, sandbox_policy: &SandboxPolicy::ReadOnly, @@ -1282,7 +1208,6 @@ prefix_rule( let manager = ExecPolicyManager::default(); let requirement = manager .create_exec_approval_requirement_for_command(ExecApprovalRequest { - features: &Features::with_defaults(), command: &command, approval_policy: AskForApproval::OnRequest, sandbox_policy: &SandboxPolicy::DangerFullAccess, @@ -1316,7 +1241,6 @@ prefix_rule( } let policy = ExecPolicyManager::new(Arc::new(Policy::empty())); - let features = Features::with_defaults(); let permissions = SandboxPermissions::UseDefault; // This command should not be run without user approval unless there is @@ -1350,7 +1274,6 @@ prefix_rule( expected_req, policy .create_exec_approval_requirement_for_command(ExecApprovalRequest { - features: &features, command: &sneaky_command, approval_policy: AskForApproval::OnRequest, sandbox_policy: &SandboxPolicy::ReadOnly, @@ -1374,7 +1297,6 @@ prefix_rule( }, policy .create_exec_approval_requirement_for_command(ExecApprovalRequest { - features: &features, command: &dangerous_command, approval_policy: AskForApproval::OnRequest, sandbox_policy: &SandboxPolicy::ReadOnly, @@ -1394,7 +1316,6 @@ prefix_rule( }, policy .create_exec_approval_requirement_for_command(ExecApprovalRequest { - features: &features, command: &dangerous_command, approval_policy: AskForApproval::Never, sandbox_policy: &SandboxPolicy::ReadOnly, diff --git a/codex-rs/core/src/features.rs b/codex-rs/core/src/features.rs index 22cb88f998..2ea28d321a 100644 --- a/codex-rs/core/src/features.rs +++ b/codex-rs/core/src/features.rs @@ -87,8 +87,6 @@ pub enum Feature { /// Allow the model to request web searches that fetch cached content. /// Takes precedence over `WebSearchRequest`. WebSearchCached, - /// Gate the execpolicy enforcement for shell/unified exec. - ExecPolicy, /// Use the bubblewrap-based Linux sandbox pipeline. UseLinuxSandboxBwrap, /// Allow the model to request approval and propose exec rules. @@ -467,12 +465,6 @@ pub const FEATURES: &[FeatureSpec] = &[ stage: Stage::UnderDevelopment, default_enabled: false, }, - FeatureSpec { - id: Feature::ExecPolicy, - key: "exec_policy", - stage: Stage::UnderDevelopment, - default_enabled: true, - }, FeatureSpec { id: Feature::UseLinuxSandboxBwrap, key: "use_linux_sandbox_bwrap", diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index cd1aa9056a..1d4f01356f 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -298,7 +298,6 @@ impl ShellHandler { .services .exec_policy .create_exec_approval_requirement_for_command(ExecApprovalRequest { - features: &features, command: &exec_params.command, approval_policy: turn.approval_policy, sandbox_policy: &turn.sandbox_policy, diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index 6b44926df4..1659ba6951 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -487,7 +487,6 @@ impl UnifiedExecProcessManager { &context.turn.shell_environment_policy, Some(context.session.conversation_id), )); - let features = context.session.features(); let mut orchestrator = ToolOrchestrator::new(); let mut runtime = UnifiedExecRuntime::new(self); let exec_approval_requirement = context @@ -495,7 +494,6 @@ impl UnifiedExecProcessManager { .services .exec_policy .create_exec_approval_requirement_for_command(ExecApprovalRequest { - features: &features, command: &request.command, approval_policy: context.turn.approval_policy, sandbox_policy: &context.turn.sandbox_policy, diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index 15f252e86f..7ec39cedbc 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -16,7 +16,6 @@ use crate::key_hint::KeyBinding; use crate::render::highlight::highlight_bash_to_lines; use crate::render::renderable::ColumnRenderable; use crate::render::renderable::Renderable; -use codex_core::features::Feature; use codex_core::features::Features; use codex_core::protocol::ElicitationAction; use codex_core::protocol::ExecPolicyAmendment; @@ -105,14 +104,14 @@ impl ApprovalOverlay { fn build_options( variant: ApprovalVariant, header: Box, - features: &Features, + _features: &Features, ) -> (Vec, SelectionViewParams) { let (options, title) = match &variant { ApprovalVariant::Exec { proposed_execpolicy_amendment, .. } => ( - exec_options(proposed_execpolicy_amendment.clone(), features), + exec_options(proposed_execpolicy_amendment.clone()), "Would you like to run the following command?".to_string(), ), ApprovalVariant::ApplyPatch { .. } => ( @@ -447,10 +446,7 @@ impl ApprovalOption { } } -fn exec_options( - proposed_execpolicy_amendment: Option, - features: &Features, -) -> Vec { +fn exec_options(proposed_execpolicy_amendment: Option) -> Vec { vec![ApprovalOption { label: "Yes, proceed".to_string(), decision: ApprovalDecision::Review(ReviewDecision::Approved), @@ -458,29 +454,23 @@ fn exec_options( additional_shortcuts: vec![key_hint::plain(KeyCode::Char('y'))], }] .into_iter() - .chain( - proposed_execpolicy_amendment - .filter(|_| features.enabled(Feature::ExecPolicy)) - .and_then(|prefix| { - let rendered_prefix = strip_bash_lc_and_escape(prefix.command()); - if rendered_prefix.contains('\n') || rendered_prefix.contains('\r') { - return None; - } + .chain(proposed_execpolicy_amendment.and_then(|prefix| { + let rendered_prefix = strip_bash_lc_and_escape(prefix.command()); + if rendered_prefix.contains('\n') || rendered_prefix.contains('\r') { + return None; + } - Some(ApprovalOption { - label: format!( - "Yes, and don't ask again for commands that start with `{rendered_prefix}`" - ), - decision: ApprovalDecision::Review( - ReviewDecision::ApprovedExecpolicyAmendment { - proposed_execpolicy_amendment: prefix, - }, - ), - display_shortcut: None, - additional_shortcuts: vec![key_hint::plain(KeyCode::Char('p'))], - }) + Some(ApprovalOption { + label: format!( + "Yes, and don't ask again for commands that start with `{rendered_prefix}`" + ), + decision: ApprovalDecision::Review(ReviewDecision::ApprovedExecpolicyAmendment { + proposed_execpolicy_amendment: prefix, }), - ) + display_shortcut: None, + additional_shortcuts: vec![key_hint::plain(KeyCode::Char('p'))], + }) + })) .chain([ApprovalOption { label: "No, and tell Codex what to do differently".to_string(), decision: ApprovalDecision::Review(ReviewDecision::Abort), @@ -619,32 +609,6 @@ mod tests { ); } - #[test] - fn exec_prefix_option_hidden_when_execpolicy_disabled() { - let (tx, mut rx) = unbounded_channel::(); - let tx = AppEventSender::new(tx); - let mut view = ApprovalOverlay::new( - ApprovalRequest::Exec { - id: "test".to_string(), - command: vec!["echo".to_string()], - reason: None, - proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![ - "echo".to_string(), - ])), - }, - tx, - { - let mut features = Features::with_defaults(); - features.disable(Feature::ExecPolicy); - features - }, - ); - assert_eq!(view.options.len(), 2); - view.handle_key_event(KeyEvent::new(KeyCode::Char('p'), KeyModifiers::NONE)); - assert!(!view.is_complete()); - assert!(rx.try_recv().is_err()); - } - #[test] fn header_includes_command_snippet() { let (tx, _rx) = unbounded_channel::();