diff --git a/PR.md b/PR.md deleted file mode 100644 index c87df4523a..0000000000 --- a/PR.md +++ /dev/null @@ -1,6 +0,0 @@ -## Summary -- thread fallback evaluation through execpolicy so pipelines evaluate the first unmatched command instead of stopping at the first allow match -- update core exec_policy integration, execpolicycheck CLI, and docs to use the new fallback path and avoid skipping heuristics on later piped commands - -## Testing -- not run (not requested) diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index 3eb0519761..9469b99b44 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -179,7 +179,7 @@ pub(crate) async fn apply_bespoke_event_handling( cwd, reason, risk, - allow_prefix: _allow_prefix, + proposed_execpolicy_amendment: _proposed_execpolicy_amendment, parsed_cmd, }) => match api_version { ApiVersion::V1 => { diff --git a/codex-rs/core/src/apply_patch.rs b/codex-rs/core/src/apply_patch.rs index 5cf0ecee70..67433303e5 100644 --- a/codex-rs/core/src/apply_patch.rs +++ b/codex-rs/core/src/apply_patch.rs @@ -71,7 +71,7 @@ pub(crate) async fn apply_patch( .await; match rx_approve.await.unwrap_or_default() { ReviewDecision::Approved - | ReviewDecision::ApprovedAllowPrefix { .. } + | ReviewDecision::ApprovedExecpolicyAmendment { .. } | ReviewDecision::ApprovedForSession => { InternalApplyPatchInvocation::DelegateToExec(ApplyPatchExec { action, diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 5c7e409423..ac44920b08 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -862,13 +862,9 @@ impl Session { .await } - /// Adds a prefix rule to the exec policy - /// - /// This mutates the in-memory execpolicy so the current conversation can use the new - /// prefix and persists the change in default.execpolicy so new conversations will also allow the new prefix. - pub(crate) async fn persist_command_allow_prefix( + pub(crate) async fn persist_execpolicy_amendment( &self, - prefix: &[String], + amendment: &[String], ) -> Result<(), ExecPolicyUpdateError> { let (features, codex_home, current_policy) = { let state = self.state.lock().await; @@ -888,10 +884,10 @@ impl Session { return Err(ExecPolicyUpdateError::FeatureDisabled); } - crate::exec_policy::append_allow_prefix_rule_and_update( + crate::exec_policy::append_execpolicy_amendment_and_update( &codex_home, ¤t_policy, - prefix, + amendment, ) .await?; @@ -912,7 +908,7 @@ impl Session { cwd: PathBuf, reason: Option, risk: Option, - allow_prefix: Option>, + proposed_execpolicy_amendment: Option>, ) -> ReviewDecision { let sub_id = turn_context.sub_id.clone(); // Add the tx_approve callback to the map before sending the request. @@ -940,7 +936,7 @@ impl Session { cwd, reason, risk, - allow_prefix, + proposed_execpolicy_amendment, parsed_cmd, }); self.send_event(turn_context, event).await; @@ -1672,13 +1668,17 @@ mod handlers { } } - /// Propagate a user's exec approval decision to the session - /// Also optionally whitelists command in execpolicy + /// Propagate a user's exec approval decision to the session. + /// Also optionally applies an execpolicy amendment. pub async fn exec_approval(sess: &Arc, id: String, decision: ReviewDecision) { - if let ReviewDecision::ApprovedAllowPrefix { allow_prefix } = &decision - && let Err(err) = sess.persist_command_allow_prefix(allow_prefix).await + if let ReviewDecision::ApprovedExecpolicyAmendment { + proposed_execpolicy_amendment, + } = &decision + && let Err(err) = sess + .persist_execpolicy_amendment(proposed_execpolicy_amendment) + .await { - let message = format!("Failed to update execpolicy allow list: {err}"); + let message = format!("Failed to apply execpolicy amendment: {err}"); tracing::warn!("{message}"); let warning = EventMsg::Warning(WarningEvent { message }); sess.send_event_raw(Event { diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index 0e1140febd..9c9a9d777b 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -275,7 +275,7 @@ async fn handle_exec_approval( event.cwd, event.reason, event.risk, - event.allow_prefix, + event.proposed_execpolicy_amendment, ); let decision = await_approval_with_cancel( approval_fut, diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 9e1faca14f..9946a5a336 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -23,7 +23,7 @@ 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 crate::tools::sandboxing::ApprovalRequirement; const FORBIDDEN_REASON: &str = "execpolicy forbids this command"; const PROMPT_CONFLICT_REASON: &str = @@ -115,7 +115,7 @@ pub(crate) fn default_policy_path(codex_home: &Path) -> PathBuf { codex_home.join(POLICY_DIR_NAME).join(DEFAULT_POLICY_FILE) } -pub(crate) async fn append_allow_prefix_rule_and_update( +pub(crate) async fn append_execpolicy_amendment_and_update( codex_home: &Path, current_policy: &Arc>, prefix: &[String], @@ -142,59 +142,25 @@ pub(crate) async fn append_allow_prefix_rule_and_update( Ok(()) } -pub(crate) async fn create_exec_approval_requirement_for_command( - exec_policy: &Arc>, - features: &Features, - command: &[String], - approval_policy: AskForApproval, - sandbox_policy: &SandboxPolicy, - sandbox_permissions: SandboxPermissions, -) -> ExecApprovalRequirement { - let commands = parse_shell_lc_plain_commands(command).unwrap_or_else(|| vec![command.to_vec()]); - let heuristics_fallback = |cmd: &[String]| { - if requires_initial_appoval(approval_policy, sandbox_policy, cmd, sandbox_permissions) { - Decision::Prompt - } else { - Decision::Allow - } - }; - let policy = exec_policy.read().await; - let evaluation = policy.check_multiple(commands.iter(), &heuristics_fallback); - let has_policy_allow = evaluation.matched_rules.iter().any(|rule_match| { - !matches!(rule_match, RuleMatch::HeuristicsRuleMatch { .. }) - && rule_match.decision() == Decision::Allow - }); - - match evaluation.decision { - Decision::Forbidden => ExecApprovalRequirement::Forbidden { - reason: FORBIDDEN_REASON.to_string(), - }, - Decision::Prompt => { - let prompt_reason = derive_prompt_reason(&evaluation); - if matches!(approval_policy, AskForApproval::Never) { - ExecApprovalRequirement::Forbidden { - reason: prompt_reason.unwrap_or_else(|| PROMPT_CONFLICT_REASON.to_string()), - } - } else { - ExecApprovalRequirement::NeedsApproval { - reason: prompt_reason.or_else(|| Some(PROMPT_REASON.to_string())), - allow_prefix: allow_prefix_if_applicable(&evaluation, &commands, features), - } - } - } - Decision::Allow => ExecApprovalRequirement::Skip { - bypass_sandbox: has_policy_allow, - }, - } -} - -/// Return an allow-prefix option when a command needs approval and execpolicy did not drive the decision -fn allow_prefix_if_applicable( +/// Returns a proposed execpolicy amendment only when heuristics caused +/// the prompt decision, so we can offer to apply that amendment for future runs. +/// +/// The amendment uses the first command heuristics marked as `Prompt`. If any explicit +/// execpolicy rule also prompts, we return `None` because applying the amendment would not +/// skip that policy requirement. +/// +/// 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`. If heuristics allow `cd` but prompt +/// on `prog1`, we return `Some(vec!["prog1", "--option1", "arg1"])`. +/// - 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 ["prog1", "--option1", "arg1"]. +fn proposed_execpolicy_amendment( evaluation: &Evaluation, - commands: &[Vec], features: &Features, ) -> Option> { - if !features.enabled(Feature::ExecPolicy) || commands.len() != 1 { + if !features.enabled(Feature::ExecPolicy) { return None; } @@ -220,6 +186,55 @@ fn allow_prefix_if_applicable( first_prompt_from_heuristics } +pub(crate) async fn create_approval_requirement_for_command( + exec_policy: &Arc>, + features: &Features, + command: &[String], + approval_policy: AskForApproval, + sandbox_policy: &SandboxPolicy, + sandbox_permissions: SandboxPermissions, +) -> ApprovalRequirement { + let commands = parse_shell_lc_plain_commands(command).unwrap_or_else(|| vec![command.to_vec()]); + let heuristics_fallback = |cmd: &[String]| { + if requires_initial_appoval(approval_policy, sandbox_policy, cmd, sandbox_permissions) { + Decision::Prompt + } else { + Decision::Allow + } + }; + let policy = exec_policy.read().await; + let evaluation = policy.check_multiple(commands.iter(), &heuristics_fallback); + let has_policy_allow = evaluation.matched_rules.iter().any(|rule_match| { + !matches!(rule_match, RuleMatch::HeuristicsRuleMatch { .. }) + && rule_match.decision() == Decision::Allow + }); + + match evaluation.decision { + Decision::Forbidden => ApprovalRequirement::Forbidden { + reason: FORBIDDEN_REASON.to_string(), + }, + Decision::Prompt => { + let prompt_reason = derive_prompt_reason(&evaluation); + if matches!(approval_policy, AskForApproval::Never) { + ApprovalRequirement::Forbidden { + reason: PROMPT_CONFLICT_REASON.to_string(), + } + } else { + ApprovalRequirement::NeedsApproval { + reason: prompt_reason, + proposed_execpolicy_amendment: proposed_execpolicy_amendment( + &evaluation, + features, + ), + } + } + } + Decision::Allow => ApprovalRequirement::Skip { + bypass_sandbox: has_policy_allow, + }, + } +} + /// Only return PROMPT_REASON when an execpolicy rule drove the prompt decision fn derive_prompt_reason(evaluation: &Evaluation) -> Option { evaluation.matched_rules.iter().find_map(|rule_match| { @@ -405,7 +420,7 @@ prefix_rule(pattern=["rm"], decision="forbidden") "rm -rf /tmp".to_string(), ]; - let requirement = create_exec_approval_requirement_for_command( + let requirement = create_approval_requirement_for_command( &policy, &Features::with_defaults(), &forbidden_script, @@ -417,14 +432,14 @@ prefix_rule(pattern=["rm"], decision="forbidden") assert_eq!( requirement, - ExecApprovalRequirement::Forbidden { + ApprovalRequirement::Forbidden { reason: FORBIDDEN_REASON.to_string() } ); } #[tokio::test] - async fn exec_approval_requirement_prefers_execpolicy_match() { + async fn approval_requirement_prefers_execpolicy_match() { let policy_src = r#"prefix_rule(pattern=["rm"], decision="prompt")"#; let mut parser = PolicyParser::new(); parser @@ -433,7 +448,7 @@ prefix_rule(pattern=["rm"], decision="forbidden") let policy = Arc::new(RwLock::new(parser.build())); let command = vec!["rm".to_string()]; - let requirement = create_exec_approval_requirement_for_command( + let requirement = create_approval_requirement_for_command( &policy, &Features::with_defaults(), &command, @@ -445,15 +460,15 @@ prefix_rule(pattern=["rm"], decision="forbidden") assert_eq!( requirement, - ExecApprovalRequirement::NeedsApproval { + ApprovalRequirement::NeedsApproval { reason: Some(PROMPT_REASON.to_string()), - allow_prefix: None, + proposed_execpolicy_amendment: None, } ); } #[tokio::test] - async fn exec_approval_requirement_respects_approval_policy() { + async fn approval_requirement_respects_approval_policy() { let policy_src = r#"prefix_rule(pattern=["rm"], decision="prompt")"#; let mut parser = PolicyParser::new(); parser @@ -462,7 +477,7 @@ prefix_rule(pattern=["rm"], decision="forbidden") let policy = Arc::new(RwLock::new(parser.build())); let command = vec!["rm".to_string()]; - let requirement = create_exec_approval_requirement_for_command( + let requirement = create_approval_requirement_for_command( &policy, &Features::with_defaults(), &command, @@ -474,18 +489,18 @@ prefix_rule(pattern=["rm"], decision="forbidden") assert_eq!( requirement, - ExecApprovalRequirement::Forbidden { - reason: PROMPT_REASON.to_string() + ApprovalRequirement::Forbidden { + reason: PROMPT_CONFLICT_REASON.to_string() } ); } #[tokio::test] - async fn exec_approval_requirement_falls_back_to_heuristics() { - let command = vec!["cargo".to_string(), "build".to_string()]; + async fn approval_requirement_falls_back_to_heuristics() { + let command = vec!["python".to_string()]; let empty_policy = Arc::new(RwLock::new(Policy::empty())); - let requirement = create_exec_approval_requirement_for_command( + let requirement = create_approval_requirement_for_command( &empty_policy, &Features::with_defaults(), &command, @@ -497,9 +512,9 @@ prefix_rule(pattern=["rm"], decision="forbidden") assert_eq!( requirement, - ExecApprovalRequirement::NeedsApproval { + ApprovalRequirement::NeedsApproval { reason: None, - allow_prefix: Some(command) + proposed_execpolicy_amendment: Some(command) } ); } @@ -519,7 +534,7 @@ prefix_rule(pattern=["rm"], decision="forbidden") ]; assert_eq!( - create_exec_approval_requirement_for_command( + create_approval_requirement_for_command( &policy, &Features::with_defaults(), &command, @@ -528,20 +543,20 @@ prefix_rule(pattern=["rm"], decision="forbidden") SandboxPermissions::UseDefault, ) .await, - ExecApprovalRequirement::NeedsApproval { + ApprovalRequirement::NeedsApproval { reason: None, - allow_prefix: None + proposed_execpolicy_amendment: Some(vec!["orange".to_string()]) } ); } #[tokio::test] - async fn append_allow_prefix_rule_updates_policy_and_file() { + async fn append_execpolicy_amendment_updates_policy_and_file() { let codex_home = tempdir().expect("create temp dir"); let current_policy = Arc::new(RwLock::new(Policy::empty())); let prefix = vec!["echo".to_string(), "hello".to_string()]; - append_allow_prefix_rule_and_update(codex_home.path(), ¤t_policy, &prefix) + append_execpolicy_amendment_and_update(codex_home.path(), ¤t_policy, &prefix) .await .expect("update policy"); @@ -567,12 +582,12 @@ prefix_rule(pattern=["rm"], decision="forbidden") } #[tokio::test] - async fn append_allow_prefix_rule_rejects_empty_prefix() { + async fn append_execpolicy_amendment_rejects_empty_prefix() { let codex_home = tempdir().expect("create temp dir"); let current_policy = Arc::new(RwLock::new(Policy::empty())); let result = - append_allow_prefix_rule_and_update(codex_home.path(), ¤t_policy, &[]).await; + append_execpolicy_amendment_and_update(codex_home.path(), ¤t_policy, &[]).await; assert!(matches!( result, @@ -584,11 +599,11 @@ prefix_rule(pattern=["rm"], decision="forbidden") } #[tokio::test] - async fn allow_prefix_is_present_for_single_command_without_policy_match() { - let command = vec!["cargo".to_string(), "build".to_string()]; + async fn proposed_execpolicy_amendment_is_present_for_single_command_without_policy_match() { + let command = vec!["python".to_string()]; let empty_policy = Arc::new(RwLock::new(Policy::empty())); - let requirement = create_exec_approval_requirement_for_command( + let requirement = create_approval_requirement_for_command( &empty_policy, &Features::with_defaults(), &command, @@ -600,21 +615,21 @@ prefix_rule(pattern=["rm"], decision="forbidden") assert_eq!( requirement, - ExecApprovalRequirement::NeedsApproval { + ApprovalRequirement::NeedsApproval { reason: None, - allow_prefix: Some(command) + proposed_execpolicy_amendment: Some(command) } ); } #[tokio::test] - async fn allow_prefix_is_disabled_when_execpolicy_feature_disabled() { - let command = vec!["cargo".to_string(), "build".to_string()]; + async fn proposed_execpolicy_amendment_is_disabled_when_execpolicy_feature_disabled() { + let command = vec!["python".to_string()]; let mut features = Features::with_defaults(); features.disable(Feature::ExecPolicy); - let requirement = create_exec_approval_requirement_for_command( + let requirement = create_approval_requirement_for_command( &Arc::new(RwLock::new(Policy::empty())), &features, &command, @@ -626,15 +641,15 @@ prefix_rule(pattern=["rm"], decision="forbidden") assert_eq!( requirement, - ExecApprovalRequirement::NeedsApproval { + ApprovalRequirement::NeedsApproval { reason: None, - allow_prefix: None, + proposed_execpolicy_amendment: None, } ); } #[tokio::test] - async fn allow_prefix_is_omitted_when_policy_prompts() { + async fn proposed_execpolicy_amendment_is_omitted_when_policy_prompts() { let policy_src = r#"prefix_rule(pattern=["rm"], decision="prompt")"#; let mut parser = PolicyParser::new(); parser @@ -643,7 +658,7 @@ prefix_rule(pattern=["rm"], decision="forbidden") let policy = Arc::new(RwLock::new(parser.build())); let command = vec!["rm".to_string()]; - let requirement = create_exec_approval_requirement_for_command( + let requirement = create_approval_requirement_for_command( &policy, &Features::with_defaults(), &command, @@ -655,42 +670,16 @@ prefix_rule(pattern=["rm"], decision="forbidden") assert_eq!( requirement, - ExecApprovalRequirement::NeedsApproval { + ApprovalRequirement::NeedsApproval { reason: Some(PROMPT_REASON.to_string()), - allow_prefix: None, + proposed_execpolicy_amendment: None, } ); } #[tokio::test] - async fn allow_prefix_is_omitted_for_multi_command_scripts() { - let command = vec![ - "bash".to_string(), - "-lc".to_string(), - "cargo build && echo ok".to_string(), - ]; - let requirement = create_exec_approval_requirement_for_command( - &Arc::new(RwLock::new(Policy::empty())), - &Features::with_defaults(), - &command, - AskForApproval::UnlessTrusted, - &SandboxPolicy::ReadOnly, - SandboxPermissions::UseDefault, - ) - .await; - - assert_eq!( - requirement, - ExecApprovalRequirement::NeedsApproval { - reason: None, - allow_prefix: None, - } - ); - } - - #[tokio::test] - async fn allow_prefix_uses_first_no_match_in_multi_command_scripts() { - let policy_src = r#"prefix_rule(pattern=["python"], decision="allow")"#; + async fn proposed_execpolicy_amendment_uses_first_no_match_in_multi_command_scripts() { + let policy_src = r#"prefix_rule(pattern=["cat"], decision="allow")"#; let mut parser = PolicyParser::new(); parser .parse("test.codexpolicy", policy_src) @@ -700,11 +689,11 @@ prefix_rule(pattern=["rm"], decision="forbidden") let command = vec![ "bash".to_string(), "-lc".to_string(), - "python && echo ok".to_string(), + "cat && apple".to_string(), ]; assert_eq!( - create_exec_approval_requirement_for_command( + create_approval_requirement_for_command( &policy, &Features::with_defaults(), &command, @@ -713,8 +702,9 @@ prefix_rule(pattern=["rm"], decision="forbidden") SandboxPermissions::UseDefault, ) .await, - ExecApprovalRequirement::Skip { - bypass_sandbox: true + ApprovalRequirement::NeedsApproval { + reason: None, + proposed_execpolicy_amendment: Some(vec!["apple".to_string()]), } ); } diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index 76d7e30a2e..3928ee6b47 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -6,7 +6,7 @@ use std::sync::Arc; use crate::codex::TurnContext; use crate::exec::ExecParams; use crate::exec_env::create_env; -use crate::exec_policy::create_exec_approval_requirement_for_command; +use crate::exec_policy::create_approval_requirement_for_command; use crate::function_tool::FunctionCallError; use crate::is_safe_command::is_known_safe_command; use crate::protocol::ExecCommandSource; @@ -232,7 +232,7 @@ impl ShellHandler { emitter.begin(event_ctx).await; let features = session.features().await; - let exec_approval_requirement = create_exec_approval_requirement_for_command( + let approval_requirement = create_approval_requirement_for_command( &turn.exec_policy, &features, &exec_params.command, @@ -241,6 +241,7 @@ impl ShellHandler { SandboxPermissions::from(exec_params.with_escalated_permissions.unwrap_or(false)), ) .await; + let req = ShellRequest { command: exec_params.command.clone(), cwd: exec_params.cwd.clone(), @@ -248,7 +249,7 @@ impl ShellHandler { env: exec_params.env.clone(), with_escalated_permissions: exec_params.with_escalated_permissions, justification: exec_params.justification.clone(), - exec_approval_requirement, + approval_requirement, }; let mut orchestrator = ToolOrchestrator::new(); let mut runtime = ShellRuntime::new(); diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index 5ac3c63509..2fd0e8f64f 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -11,14 +11,14 @@ use crate::error::get_error_message_ui; use crate::exec::ExecToolCallOutput; use crate::sandboxing::SandboxManager; use crate::tools::sandboxing::ApprovalCtx; -use crate::tools::sandboxing::ExecApprovalRequirement; +use crate::tools::sandboxing::ApprovalRequirement; use crate::tools::sandboxing::ProvidesSandboxRetryData; use crate::tools::sandboxing::SandboxAttempt; use crate::tools::sandboxing::SandboxOverride; use crate::tools::sandboxing::ToolCtx; use crate::tools::sandboxing::ToolError; use crate::tools::sandboxing::ToolRuntime; -use crate::tools::sandboxing::default_exec_approval_requirement; +use crate::tools::sandboxing::default_approval_requirement; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::ReviewDecision; @@ -54,17 +54,17 @@ impl ToolOrchestrator { // 1) Approval let mut already_approved = false; - let requirement = tool.exec_approval_requirement(req).unwrap_or_else(|| { - default_exec_approval_requirement(approval_policy, &turn_ctx.sandbox_policy) + let requirement = tool.approval_requirement(req).unwrap_or_else(|| { + default_approval_requirement(approval_policy, &turn_ctx.sandbox_policy) }); match requirement { - ExecApprovalRequirement::Skip { .. } => { + ApprovalRequirement::Skip { .. } => { otel.tool_decision(otel_tn, otel_ci, &ReviewDecision::Approved, otel_cfg); } - ExecApprovalRequirement::Forbidden { reason } => { + ApprovalRequirement::Forbidden { reason } => { return Err(ToolError::Rejected(reason)); } - ExecApprovalRequirement::NeedsApproval { reason, .. } => { + ApprovalRequirement::NeedsApproval { reason, .. } => { let mut risk = None; if let Some(metadata) = req.sandbox_retry_data() { @@ -95,7 +95,7 @@ impl ToolOrchestrator { return Err(ToolError::Rejected("rejected by user".to_string())); } ReviewDecision::Approved - | ReviewDecision::ApprovedAllowPrefix { .. } + | ReviewDecision::ApprovedExecpolicyAmendment { .. } | ReviewDecision::ApprovedForSession => {} } already_approved = true; @@ -120,87 +120,50 @@ impl ToolOrchestrator { codex_linux_sandbox_exe: turn_ctx.codex_linux_sandbox_exe.as_ref(), }; - match tool.run(req, &initial_attempt, tool_ctx).await { - Ok(out) => { - // We have a successful initial result - Ok(out) - } - Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { output }))) => { - if !tool.escalate_on_failure() { - return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { - output, - }))); - } - // Under `Never` or `OnRequest`, do not retry without sandbox; surface a concise - // sandbox denial that preserves the original output. - if !tool.wants_no_sandbox_approval(approval_policy) { - return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { - output, - }))); - } + let mut out = tool.run(req, &initial_attempt, tool_ctx).await; - // Ask for approval before retrying without sandbox. - if !tool.should_bypass_approval(approval_policy, already_approved) { - let mut risk = None; + if matches!(initial_sandbox, crate::exec::SandboxType::None) { + // If we already skipped sandboxing on the first attempt, there's no + // fallback path. + return out; + } - if let Some(metadata) = req.sandbox_retry_data() { - let err = SandboxErr::Denied { - output: output.clone(), - }; - let friendly = get_error_message_ui(&CodexErr::Sandbox(err)); - let failure_summary = format!("failed in sandbox: {friendly}"); - - risk = tool_ctx - .session - .assess_sandbox_command( - turn_ctx, - &tool_ctx.call_id, - &metadata.command, - Some(failure_summary.as_str()), - ) - .await; - } - - let reason_msg = build_denial_reason_from_output(output.as_ref()); - let approval_ctx = ApprovalCtx { - session: tool_ctx.session, - turn: turn_ctx, - call_id: &tool_ctx.call_id, - retry_reason: Some(reason_msg), - risk, - }; - - let decision = tool.start_approval_async(req, approval_ctx).await; - otel.tool_decision(otel_tn, otel_ci, &decision, otel_user); - - match decision { - ReviewDecision::Denied | ReviewDecision::Abort => { - return Err(ToolError::Rejected("rejected by user".to_string())); - } - ReviewDecision::Approved - | ReviewDecision::ApprovedAllowPrefix { .. } - | ReviewDecision::ApprovedForSession => {} - } - } - - let escalated_attempt = SandboxAttempt { + // 3) Retry without sandbox on approval. + if matches!(&out, Err(ToolError::Codex(CodexErr::Sandbox(_)))) + && tool.sandbox_preference() != crate::tools::sandboxing::SandboxablePreference::Require + && tool.escalate_on_failure() + && tool.should_bypass_approval(approval_policy, already_approved) + { + // Attempt a retry without sandbox. + out = tool.run( + req, + &SandboxAttempt { sandbox: crate::exec::SandboxType::None, - policy: &turn_ctx.sandbox_policy, + policy: &crate::protocol::SandboxPolicy::DangerFullAccess, manager: &self.sandbox, sandbox_cwd: &turn_ctx.cwd, - codex_linux_sandbox_exe: None, - }; - - // Second attempt. - (*tool).run(req, &escalated_attempt, tool_ctx).await + codex_linux_sandbox_exe: turn_ctx.codex_linux_sandbox_exe.as_ref(), + }, + tool_ctx, + ) + .await; + if let Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { output }))) = &out { + return Err(ToolError::Rejected(format!( + "sandbox denied the command, even after approving it without sandbox: {}", + get_error_message_ui(output) + ))); } - other => other, + } + + out + } + + /// Translate result from tool runner to library level result (errors not in ToolError become CodexErr). + pub fn translate_response(result: Result) -> Result<(), CodexErr> { + match result { + Ok(_) => Ok(()), + Err(ToolError::Codex(err)) => Err(err), + Err(ToolError::Rejected(reason)) => Err(CodexErr::Rejected(reason)), } } } - -fn build_denial_reason_from_output(_output: &ExecToolCallOutput) -> String { - // Keep approval reason terse and stable for UX/tests, but accept the - // output so we can evolve heuristics later without touching call sites. - "command failed; retry without sandbox?".to_string() -} diff --git a/codex-rs/core/src/tools/runtimes/shell.rs b/codex-rs/core/src/tools/runtimes/shell.rs index 48dc5b9990..4ff2ea5886 100644 --- a/codex-rs/core/src/tools/runtimes/shell.rs +++ b/codex-rs/core/src/tools/runtimes/shell.rs @@ -9,7 +9,7 @@ use crate::sandboxing::execute_env; use crate::tools::runtimes::build_command_spec; use crate::tools::sandboxing::Approvable; use crate::tools::sandboxing::ApprovalCtx; -use crate::tools::sandboxing::ExecApprovalRequirement; +use crate::tools::sandboxing::ApprovalRequirement; use crate::tools::sandboxing::ProvidesSandboxRetryData; use crate::tools::sandboxing::SandboxAttempt; use crate::tools::sandboxing::SandboxOverride; @@ -32,7 +32,7 @@ pub struct ShellRequest { pub env: std::collections::HashMap, pub with_escalated_permissions: Option, pub justification: Option, - pub exec_approval_requirement: ExecApprovalRequirement, + pub approval_requirement: ApprovalRequirement, } impl ProvidesSandboxRetryData for ShellRequest { @@ -114,7 +114,9 @@ impl Approvable for ShellRuntime { cwd, reason, risk, - req.exec_approval_requirement.allow_prefix().cloned(), + req.approval_requirement + .proposed_execpolicy_amendment() + .cloned(), ) .await }) @@ -122,15 +124,15 @@ impl Approvable for ShellRuntime { }) } - fn exec_approval_requirement(&self, req: &ShellRequest) -> Option { - Some(req.exec_approval_requirement.clone()) + fn approval_requirement(&self, req: &ShellRequest) -> Option { + Some(req.approval_requirement.clone()) } fn sandbox_mode_for_first_attempt(&self, req: &ShellRequest) -> SandboxOverride { if req.with_escalated_permissions.unwrap_or(false) || matches!( - req.exec_approval_requirement, - ExecApprovalRequirement::Skip { + req.approval_requirement, + ApprovalRequirement::Skip { bypass_sandbox: true } ) diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index 45d804d688..98acc651a1 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -10,7 +10,7 @@ use crate::exec::ExecExpiration; use crate::tools::runtimes::build_command_spec; use crate::tools::sandboxing::Approvable; use crate::tools::sandboxing::ApprovalCtx; -use crate::tools::sandboxing::ExecApprovalRequirement; +use crate::tools::sandboxing::ApprovalRequirement; use crate::tools::sandboxing::ProvidesSandboxRetryData; use crate::tools::sandboxing::SandboxAttempt; use crate::tools::sandboxing::SandboxOverride; @@ -36,7 +36,7 @@ pub struct UnifiedExecRequest { pub env: HashMap, pub with_escalated_permissions: Option, pub justification: Option, - pub exec_approval_requirement: ExecApprovalRequirement, + pub approval_requirement: ApprovalRequirement, } impl ProvidesSandboxRetryData for UnifiedExecRequest { @@ -66,7 +66,7 @@ impl UnifiedExecRequest { env: HashMap, with_escalated_permissions: Option, justification: Option, - exec_approval_requirement: ExecApprovalRequirement, + approval_requirement: ApprovalRequirement, ) -> Self { Self { command, @@ -74,7 +74,7 @@ impl UnifiedExecRequest { env, with_escalated_permissions, justification, - exec_approval_requirement, + approval_requirement, } } } @@ -132,7 +132,9 @@ impl Approvable for UnifiedExecRuntime<'_> { cwd, reason, risk, - req.exec_approval_requirement.allow_prefix().cloned(), + req.approval_requirement + .proposed_execpolicy_amendment() + .cloned(), ) .await }) @@ -140,18 +142,15 @@ impl Approvable for UnifiedExecRuntime<'_> { }) } - fn exec_approval_requirement( - &self, - req: &UnifiedExecRequest, - ) -> Option { - Some(req.exec_approval_requirement.clone()) + fn approval_requirement(&self, req: &UnifiedExecRequest) -> Option { + Some(req.approval_requirement.clone()) } fn sandbox_mode_for_first_attempt(&self, req: &UnifiedExecRequest) -> SandboxOverride { if req.with_escalated_permissions.unwrap_or(false) || matches!( - req.exec_approval_requirement, - ExecApprovalRequirement::Skip { + req.approval_requirement, + ApprovalRequirement::Skip { bypass_sandbox: true } ) diff --git a/codex-rs/core/src/tools/sandboxing.rs b/codex-rs/core/src/tools/sandboxing.rs index 793c162936..cf28983d85 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -88,28 +88,29 @@ pub(crate) struct ApprovalCtx<'a> { // Specifies what tool orchestrator should do with a given tool call. #[derive(Clone, Debug, PartialEq, Eq)] -pub(crate) enum ExecApprovalRequirement { +pub(crate) enum ApprovalRequirement { /// No approval required for this tool call. Skip { /// The first attempt should skip sandboxing (e.g., when explicitly /// greenlit by policy). bypass_sandbox: bool, }, - /// Approval required for this tool call. + /// Approval required for this tool call NeedsApproval { reason: Option, - /// Prefix that can be whitelisted via execpolicy to skip future approvals for similar commands - allow_prefix: Option>, + /// Proposed execpolicy amendment to skip future approvals for similar commands + /// See core/src/exec_policy.rs for more details on how proposed_execpolicy_amendment is determined. + proposed_execpolicy_amendment: Option>, }, - /// Execution forbidden for this tool call. + /// Execution forbidden for this tool call Forbidden { reason: String }, } -impl ExecApprovalRequirement { - pub fn allow_prefix(&self) -> Option<&Vec> { +impl ApprovalRequirement { + pub fn proposed_execpolicy_amendment(&self) -> Option<&Vec> { match self { Self::NeedsApproval { - allow_prefix: Some(prefix), + proposed_execpolicy_amendment: Some(prefix), .. } => Some(prefix), _ => None, @@ -120,10 +121,10 @@ impl ExecApprovalRequirement { /// - Never, OnFailure: do not ask /// - OnRequest: ask unless sandbox policy is DangerFullAccess /// - UnlessTrusted: always ask -pub(crate) fn default_exec_approval_requirement( +pub(crate) fn default_approval_requirement( policy: AskForApproval, sandbox_policy: &SandboxPolicy, -) -> ExecApprovalRequirement { +) -> ApprovalRequirement { let needs_approval = match policy { AskForApproval::Never | AskForApproval::OnFailure => false, AskForApproval::OnRequest => !matches!(sandbox_policy, SandboxPolicy::DangerFullAccess), @@ -131,12 +132,12 @@ pub(crate) fn default_exec_approval_requirement( }; if needs_approval { - ExecApprovalRequirement::NeedsApproval { + ApprovalRequirement::NeedsApproval { reason: None, - allow_prefix: None, + proposed_execpolicy_amendment: None, } } else { - ExecApprovalRequirement::Skip { + ApprovalRequirement::Skip { bypass_sandbox: false, } } @@ -168,9 +169,10 @@ pub(crate) trait Approvable { matches!(policy, AskForApproval::Never) } - /// Return `Some(_)` to specify a custom exec approval requirement, or `None` - /// to fall back to policy-based default. - fn exec_approval_requirement(&self, _req: &Req) -> Option { + /// Override the default approval requirement. Return `Some(_)` to specify + /// a custom requirement, or `None` to fall back to + /// policy-based default. + fn approval_requirement(&self, _req: &Req) -> Option { None } diff --git a/codex-rs/core/src/unified_exec/session_manager.rs b/codex-rs/core/src/unified_exec/session_manager.rs index c46de7a87a..fba8108103 100644 --- a/codex-rs/core/src/unified_exec/session_manager.rs +++ b/codex-rs/core/src/unified_exec/session_manager.rs @@ -15,7 +15,7 @@ use crate::codex::TurnContext; use crate::exec::ExecToolCallOutput; use crate::exec::StreamOutput; use crate::exec_env::create_env; -use crate::exec_policy::create_exec_approval_requirement_for_command; +use crate::exec_policy::create_approval_requirement_for_command; use crate::protocol::BackgroundEventEvent; use crate::protocol::EventMsg; use crate::protocol::ExecCommandSource; @@ -555,7 +555,7 @@ impl UnifiedExecSessionManager { let features = context.session.features().await; let mut orchestrator = ToolOrchestrator::new(); let mut runtime = UnifiedExecRuntime::new(self); - let exec_approval_requirement = create_exec_approval_requirement_for_command( + let approval_requirement = create_approval_requirement_for_command( &context.turn.exec_policy, &features, command, @@ -570,7 +570,7 @@ impl UnifiedExecSessionManager { env, with_escalated_permissions, justification, - exec_approval_requirement, + approval_requirement, ); let tool_ctx = ToolCtx { session: context.session.as_ref(), diff --git a/codex-rs/core/tests/suite/approvals.rs b/codex-rs/core/tests/suite/approvals.rs index 8ac31fb378..e3282c74e6 100644 --- a/codex-rs/core/tests/suite/approvals.rs +++ b/codex-rs/core/tests/suite/approvals.rs @@ -1559,29 +1559,28 @@ async fn run_scenario(scenario: &ScenarioSpec) -> Result<()> { } #[tokio::test(flavor = "current_thread")] -#[cfg(unix)] -async fn approving_allow_prefix_persists_policy_and_skips_future_prompts() -> Result<()> { +async fn approving_execpolicy_amendment_persists_policy_and_skips_future_prompts() -> Result<()> { let server = start_mock_server().await; let approval_policy = AskForApproval::UnlessTrusted; - let sandbox_policy = SandboxPolicy::ReadOnly; + let sandbox_policy = SandboxPolicy::DangerFullAccess; let sandbox_policy_for_config = sandbox_policy.clone(); let mut builder = test_codex().with_config(move |config| { config.approval_policy = approval_policy; config.sandbox_policy = sandbox_policy_for_config; }); let test = builder.build(&server).await?; - let allow_prefix_path = test.cwd.path().join("allow-prefix.txt"); - let _ = fs::remove_file(&allow_prefix_path); - let call_id_first = "allow-prefix-first"; let (first_event, expected_command) = ActionKind::RunCommand { - command: "touch allow-prefix.txt", + command: "printf allow-prefix-ok", } .prepare(&test, &server, call_id_first, false) .await?; let expected_command = - expected_command.expect("allow prefix scenario should produce a shell command"); - let expected_allow_prefix = vec!["touch".to_string(), "allow-prefix.txt".to_string()]; + expected_command.expect("execpolicy amendment scenario should produce a shell command"); + let expected_execpolicy_amendment = expected_command + .split_whitespace() + .map(ToString::to_string) + .collect::>(); let _ = mount_sse_once( &server, @@ -1610,13 +1609,16 @@ async fn approving_allow_prefix_persists_policy_and_skips_future_prompts() -> Re .await?; let approval = expect_exec_approval(&test, expected_command.as_str()).await; - assert_eq!(approval.allow_prefix, Some(expected_allow_prefix.clone())); + assert_eq!( + approval.proposed_execpolicy_amendment, + Some(expected_execpolicy_amendment.clone()) + ); test.codex .submit(Op::ExecApproval { id: "0".into(), - decision: ReviewDecision::ApprovedAllowPrefix { - allow_prefix: expected_allow_prefix.clone(), + decision: ReviewDecision::ApprovedExecpolicyAmendment { + proposed_execpolicy_amendment: expected_execpolicy_amendment.clone(), }, }) .await?; @@ -1626,7 +1628,7 @@ async fn approving_allow_prefix_persists_policy_and_skips_future_prompts() -> Re let policy_contents = fs::read_to_string(&policy_path)?; assert!( policy_contents - .contains(r#"prefix_rule(pattern=["touch", "allow-prefix.txt"], decision="allow")"#), + .contains(r#"prefix_rule(pattern=["printf", "allow-prefix-ok"], decision="allow")"#), "unexpected policy contents: {policy_contents}" ); @@ -1637,19 +1639,14 @@ async fn approving_allow_prefix_persists_policy_and_skips_future_prompts() -> Re ); assert_eq!(first_output.exit_code.unwrap_or(0), 0); assert!( - first_output.stdout.is_empty(), + first_output.stdout.contains("allow-prefix-ok"), "unexpected stdout: {}", first_output.stdout ); - assert_eq!( - fs::read_to_string(&allow_prefix_path)?, - "", - "unexpected file contents after first run" - ); let call_id_second = "allow-prefix-second"; let (second_event, second_command) = ActionKind::RunCommand { - command: "touch allow-prefix.txt", + command: "printf allow-prefix-ok", } .prepare(&test, &server, call_id_second, false) .await?; @@ -1690,15 +1687,10 @@ async fn approving_allow_prefix_persists_policy_and_skips_future_prompts() -> Re ); assert_eq!(second_output.exit_code.unwrap_or(0), 0); assert!( - second_output.stdout.is_empty(), + second_output.stdout.contains("allow-prefix-ok"), "unexpected stdout: {}", second_output.stdout ); - assert_eq!( - fs::read_to_string(&allow_prefix_path)?, - "", - "unexpected file contents after second run" - ); Ok(()) } diff --git a/codex-rs/mcp-server/src/codex_tool_runner.rs b/codex-rs/mcp-server/src/codex_tool_runner.rs index cbee0fe0e7..aa895d8dd3 100644 --- a/codex-rs/mcp-server/src/codex_tool_runner.rs +++ b/codex-rs/mcp-server/src/codex_tool_runner.rs @@ -180,7 +180,7 @@ async fn run_codex_tool_session_inner( call_id, reason: _, risk, - allow_prefix: _, + proposed_execpolicy_amendment: _, parsed_cmd, }) => { handle_exec_approval_request( diff --git a/codex-rs/protocol/src/approvals.rs b/codex-rs/protocol/src/approvals.rs index 54a9efca9f..6d4b0c76b4 100644 --- a/codex-rs/protocol/src/approvals.rs +++ b/codex-rs/protocol/src/approvals.rs @@ -51,10 +51,10 @@ pub struct ExecApprovalRequestEvent { /// Optional model-provided risk assessment describing the blocked command. #[serde(skip_serializing_if = "Option::is_none")] pub risk: Option, - /// Prefix rule that can be added to the user's execpolicy to allow future runs. + /// Proposed execpolicy amendment that can be applied to allow future runs. #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional, type = "Array")] - pub allow_prefix: Option>, + pub proposed_execpolicy_amendment: Option>, pub parsed_cmd: Vec, } diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 78cce24cd4..1a3662a227 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -1652,9 +1652,11 @@ pub enum ReviewDecision { /// User has approved this command and the agent should execute it. Approved, - /// User has approved this command and wants to add the command prefix to - /// the execpolicy allow list so future matching commands are permitted. - ApprovedAllowPrefix { allow_prefix: Vec }, + /// User has approved this command and wants to apply the proposed execpolicy + /// amendment so future matching commands are permitted. + ApprovedExecpolicyAmendment { + proposed_execpolicy_amendment: Vec, + }, /// User has approved this command and wants to automatically approve any /// future identical instances (`command` and `cwd` match exactly) for the diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index 3defdbf9e3..4ca47bc720 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -43,7 +43,7 @@ pub(crate) enum ApprovalRequest { command: Vec, reason: Option, risk: Option, - allow_prefix: Option>, + proposed_execpolicy_amendment: Option>, }, ApplyPatch { id: String, @@ -105,8 +105,11 @@ impl ApprovalOverlay { header: Box, ) -> (Vec, SelectionViewParams) { let (options, title) = match &variant { - ApprovalVariant::Exec { allow_prefix, .. } => ( - exec_options(allow_prefix.clone()), + ApprovalVariant::Exec { + proposed_execpolicy_amendment, + .. + } => ( + exec_options(proposed_execpolicy_amendment.clone()), "Would you like to run the following command?".to_string(), ), ApprovalVariant::ApplyPatch { .. } => ( @@ -337,7 +340,7 @@ impl From for ApprovalRequestState { command, reason, risk, - allow_prefix, + proposed_execpolicy_amendment, } => { let reason = reason.filter(|item| !item.is_empty()); let has_reason = reason.is_some(); @@ -360,7 +363,7 @@ impl From for ApprovalRequestState { variant: ApprovalVariant::Exec { id, command, - allow_prefix, + proposed_execpolicy_amendment, }, header: Box::new(Paragraph::new(header).wrap(Wrap { trim: false })), } @@ -437,7 +440,7 @@ enum ApprovalVariant { Exec { id: String, command: Vec, - allow_prefix: Option>, + proposed_execpolicy_amendment: Option>, }, ApplyPatch { id: String, @@ -470,7 +473,7 @@ impl ApprovalOption { } } -fn exec_options(allow_prefix: Option>) -> Vec { +fn exec_options(proposed_execpolicy_amendment: Option>) -> Vec { vec![ ApprovalOption { label: "Yes, proceed".to_string(), @@ -486,10 +489,10 @@ fn exec_options(allow_prefix: Option>) -> Vec { }, ] .into_iter() - .chain(allow_prefix.map(|prefix| ApprovalOption { + .chain(proposed_execpolicy_amendment.map(|prefix| ApprovalOption { label: "Yes, and don't ask again for commands with this prefix".to_string(), - decision: ApprovalDecision::Review(ReviewDecision::ApprovedAllowPrefix { - allow_prefix: prefix, + decision: ApprovalDecision::Review(ReviewDecision::ApprovedExecpolicyAmendment { + proposed_execpolicy_amendment: prefix, }), display_shortcut: None, additional_shortcuts: vec![key_hint::plain(KeyCode::Char('p'))], @@ -556,7 +559,7 @@ mod tests { command: vec!["echo".to_string(), "hi".to_string()], reason: Some("reason".to_string()), risk: None, - allow_prefix: None, + proposed_execpolicy_amendment: None, } } @@ -590,7 +593,7 @@ mod tests { } #[test] - fn exec_prefix_option_emits_allow_prefix() { + fn exec_prefix_option_emits_execpolicy_amendment() { let (tx, mut rx) = unbounded_channel::(); let tx = AppEventSender::new(tx); let mut view = ApprovalOverlay::new( @@ -599,7 +602,7 @@ mod tests { command: vec!["echo".to_string()], reason: None, risk: None, - allow_prefix: Some(vec!["echo".to_string()]), + proposed_execpolicy_amendment: Some(vec!["echo".to_string()]), }, tx, ); @@ -609,8 +612,8 @@ mod tests { if let AppEvent::CodexOp(Op::ExecApproval { decision, .. }) = ev { assert_eq!( decision, - ReviewDecision::ApprovedAllowPrefix { - allow_prefix: vec!["echo".to_string()] + ReviewDecision::ApprovedExecpolicyAmendment { + proposed_execpolicy_amendment: vec!["echo".to_string()] } ); saw_op = true; @@ -633,7 +636,7 @@ mod tests { command, reason: None, risk: None, - allow_prefix: None, + proposed_execpolicy_amendment: None, }; let view = ApprovalOverlay::new(exec_request, tx); diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 4c5a8ea258..2a4fcbc514 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -563,7 +563,7 @@ mod tests { command: vec!["echo".into(), "ok".into()], reason: None, risk: None, - allow_prefix: None, + proposed_execpolicy_amendment: None, } } diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index cd4eedfda1..9b9ccdd217 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -1069,7 +1069,7 @@ impl ChatWidget { command: ev.command, reason: ev.reason, risk: ev.risk, - allow_prefix: ev.allow_prefix, + proposed_execpolicy_amendment: ev.proposed_execpolicy_amendment, }; self.bottom_pane.push_approval_request(request); self.request_redraw(); diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index c25355a759..28c8497958 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -677,7 +677,7 @@ fn exec_approval_emits_proposed_command_and_decision_history() { "this is a test reason such as one that would be produced by the model".into(), ), risk: None, - allow_prefix: None, + proposed_execpolicy_amendment: None, parsed_cmd: vec![], }; chat.handle_codex_event(Event { @@ -722,7 +722,7 @@ fn exec_approval_decision_truncates_multiline_and_long_commands() { "this is a test reason such as one that would be produced by the model".into(), ), risk: None, - allow_prefix: None, + proposed_execpolicy_amendment: None, parsed_cmd: vec![], }; chat.handle_codex_event(Event { @@ -773,7 +773,7 @@ fn exec_approval_decision_truncates_multiline_and_long_commands() { cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), reason: None, risk: None, - allow_prefix: None, + proposed_execpolicy_amendment: None, parsed_cmd: vec![], }; chat.handle_codex_event(Event { @@ -1972,7 +1972,7 @@ fn approval_modal_exec_snapshot() { "this is a test reason such as one that would be produced by the model".into(), ), risk: None, - allow_prefix: Some(vec!["echo".into(), "hello".into(), "world".into()]), + proposed_execpolicy_amendment: Some(vec!["echo".into(), "hello".into(), "world".into()]), parsed_cmd: vec![], }; chat.handle_codex_event(Event { @@ -2019,7 +2019,7 @@ fn approval_modal_exec_without_reason_snapshot() { cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), reason: None, risk: None, - allow_prefix: Some(vec!["echo".into(), "hello".into(), "world".into()]), + proposed_execpolicy_amendment: Some(vec!["echo".into(), "hello".into(), "world".into()]), parsed_cmd: vec![], }; chat.handle_codex_event(Event { @@ -2233,7 +2233,7 @@ fn status_widget_and_approval_modal_snapshot() { "this is a test reason such as one that would be produced by the model".into(), ), risk: None, - allow_prefix: Some(vec!["echo".into(), "hello world".into()]), + proposed_execpolicy_amendment: Some(vec!["echo".into(), "hello world".into()]), parsed_cmd: vec![], }; chat.handle_codex_event(Event { diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index eac163b8fb..22eb24bee3 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -408,7 +408,7 @@ pub fn new_approval_decision_cell( ], ) } - ApprovedAllowPrefix { .. } => { + ApprovedExecpolicyAmendment { .. } => { let snippet = Span::from(exec_snippet(&command)).dim(); ( "✔ ".green(), @@ -417,7 +417,7 @@ pub fn new_approval_decision_cell( "approved".bold(), " codex to run ".into(), snippet, - " and added its prefix to your allow list".bold(), + " and applied the execpolicy amendment".bold(), ], ) } diff --git a/docs/config.md b/docs/config.md index 6aa1bde8c5..3b06d73019 100644 --- a/docs/config.md +++ b/docs/config.md @@ -603,7 +603,7 @@ metadata above): - `codex.tool_decision` - `tool_name` - `call_id` - - `decision` (`approved`, `approved_allow_prefix`, `approved_for_session`, `denied`, or `abort`) + - `decision` (`approved`, `approved_execpolicy_amendment`, `approved_for_session`, `denied`, or `abort`) - `source` (`config` or `user`) - `codex.tool_result` - `tool_name` diff --git a/docs/execpolicy.md b/docs/execpolicy.md index 77d8ed0e7b..d543a414e7 100644 --- a/docs/execpolicy.md +++ b/docs/execpolicy.md @@ -53,10 +53,10 @@ When no rules match, `matchedRules` is an empty array and `decision` is omitted. ```json { - "matchedRules": [], + "matchedRules": [] } +``` ## Status `execpolicy` commands are still in preview. The API may have breaking changes in the future. -```