diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index b8175ee36b..b465e9187a 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1218,6 +1218,7 @@ dependencies = [ "serde_json", "shlex", "starlark", + "tempfile", "thiserror 2.0.17", ] diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index d4b00e67f9..ff6ebc563d 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -153,6 +153,7 @@ pub(crate) async fn apply_bespoke_event_handling( cwd, reason, risk, + allow_prefix: _allow_prefix, parsed_cmd, }) => match api_version { ApiVersion::V1 => { @@ -610,6 +611,7 @@ async fn on_exec_approval_response( .submit(Op::ExecApproval { id: event_id, decision: response.decision, + allow_prefix: None, }) .await { @@ -783,6 +785,7 @@ async fn on_command_execution_request_approval_response( .submit(Op::ExecApproval { id: event_id, decision, + allow_prefix: None, }) .await { diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 4d9285dd24..f038030ce7 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -68,6 +68,7 @@ use crate::error::CodexErr; use crate::error::Result as CodexResult; #[cfg(test)] use crate::exec::StreamOutput; +use crate::exec_policy::ExecPolicyUpdateError; use crate::mcp::auth::compute_auth_statuses; use crate::mcp_connection_manager::McpConnectionManager; use crate::model_family::find_family_for_model; @@ -845,11 +846,43 @@ impl Session { .await } + pub(crate) async fn persist_command_allow_prefix( + &self, + prefix: &[String], + ) -> Result<(), ExecPolicyUpdateError> { + let (features, codex_home) = { + let state = self.state.lock().await; + ( + state.session_configuration.features.clone(), + state + .session_configuration + .original_config_do_not_use + .codex_home + .clone(), + ) + }; + + let policy = + crate::exec_policy::append_allow_prefix_rule_and_reload(&features, &codex_home, prefix) + .await?; + + let mut state = self.state.lock().await; + state.session_configuration.exec_policy = policy; + + Ok(()) + } + + pub(crate) async fn current_exec_policy(&self) -> Arc { + let state = self.state.lock().await; + state.session_configuration.exec_policy.clone() + } + /// Emit an exec approval request event and await the user's decision. /// /// The request is keyed by `sub_id`/`call_id` so matching responses are delivered /// to the correct in-flight turn. If the task is aborted, this returns the /// default `ReviewDecision` (`Denied`). + #[allow(clippy::too_many_arguments)] pub async fn request_command_approval( &self, turn_context: &TurnContext, @@ -858,6 +891,7 @@ impl Session { cwd: PathBuf, reason: Option, risk: Option, + allow_prefix: Option>, ) -> ReviewDecision { let sub_id = turn_context.sub_id.clone(); // Add the tx_approve callback to the map before sending the request. @@ -885,6 +919,7 @@ impl Session { cwd, reason, risk, + allow_prefix, parsed_cmd, }); self.send_event(turn_context, event).await; @@ -1383,8 +1418,12 @@ async fn submission_loop(sess: Arc, config: Arc, rx_sub: Receiv handlers::user_input_or_turn(&sess, sub.id.clone(), sub.op, &mut previous_context) .await; } - Op::ExecApproval { id, decision } => { - handlers::exec_approval(&sess, id, decision).await; + Op::ExecApproval { + id, + decision, + allow_prefix, + } => { + handlers::exec_approval(&sess, id, decision, allow_prefix).await; } Op::PatchApproval { id, decision } => { handlers::patch_approval(&sess, id, decision).await; @@ -1453,6 +1492,7 @@ mod handlers { use codex_protocol::protocol::ReviewDecision; use codex_protocol::protocol::ReviewRequest; use codex_protocol::protocol::TurnAbortReason; + use codex_protocol::protocol::WarningEvent; use codex_protocol::user_input::UserInput; use std::sync::Arc; @@ -1538,7 +1578,28 @@ mod handlers { *previous_context = Some(turn_context); } - pub async fn exec_approval(sess: &Arc, id: String, decision: ReviewDecision) { + pub async fn exec_approval( + sess: &Arc, + id: String, + decision: ReviewDecision, + allow_prefix: Option>, + ) { + if let Some(prefix) = allow_prefix + && matches!( + decision, + ReviewDecision::Approved | ReviewDecision::ApprovedForSession + ) + && let Err(err) = sess.persist_command_allow_prefix(&prefix).await + { + let message = format!("Failed to update execpolicy allow list: {err}"); + tracing::warn!("{message}"); + let warning = EventMsg::Warning(WarningEvent { message }); + sess.send_event_raw(Event { + id: id.clone(), + msg: warning, + }) + .await; + } match decision { ReviewDecision::Abort => { sess.interrupt_task().await; diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index 4cb4d4a06a..110848b157 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -235,6 +235,7 @@ async fn handle_exec_approval( event.cwd, event.reason, event.risk, + event.allow_prefix, ); let decision = await_approval_with_cancel( approval_fut, @@ -244,7 +245,13 @@ async fn handle_exec_approval( ) .await; - let _ = codex.submit(Op::ExecApproval { id, decision }).await; + let _ = codex + .submit(Op::ExecApproval { + id, + decision, + allow_prefix: None, + }) + .await; } /// Handle an ApplyPatchApprovalRequest by consulting the parent session and replying. diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index fcd2f5b0c9..c86ef71957 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -297,6 +297,7 @@ impl ShellHandler { let event_ctx = ToolEventCtx::new(session.as_ref(), turn.as_ref(), &call_id, None); emitter.begin(event_ctx).await; + let exec_policy = session.current_exec_policy().await; let req = ShellRequest { command: exec_params.command.clone(), cwd: exec_params.cwd.clone(), @@ -305,7 +306,7 @@ impl ShellHandler { with_escalated_permissions: exec_params.with_escalated_permissions, justification: exec_params.justification.clone(), approval_requirement: create_approval_requirement_for_command( - &turn.exec_policy, + exec_policy.as_ref(), &exec_params.command, turn.approval_policy, &turn.sandbox_policy, diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index 7e8e152f67..e37e753018 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -63,7 +63,7 @@ impl ToolOrchestrator { ApprovalRequirement::Forbidden { reason } => { return Err(ToolError::Rejected(reason)); } - ApprovalRequirement::NeedsApproval { reason } => { + ApprovalRequirement::NeedsApproval { reason, .. } => { let mut risk = None; if let Some(metadata) = req.sandbox_retry_data() { diff --git a/codex-rs/core/src/tools/runtimes/apply_patch.rs b/codex-rs/core/src/tools/runtimes/apply_patch.rs index 0cdddd5087..cbc93af284 100644 --- a/codex-rs/core/src/tools/runtimes/apply_patch.rs +++ b/codex-rs/core/src/tools/runtimes/apply_patch.rs @@ -127,6 +127,7 @@ impl Approvable for ApplyPatchRuntime { cwd, Some(reason), risk, + None, ) .await } else if user_explicitly_approved { diff --git a/codex-rs/core/src/tools/runtimes/shell.rs b/codex-rs/core/src/tools/runtimes/shell.rs index d71c4498e6..db40476fa6 100644 --- a/codex-rs/core/src/tools/runtimes/shell.rs +++ b/codex-rs/core/src/tools/runtimes/shell.rs @@ -106,7 +106,15 @@ impl Approvable for ShellRuntime { Box::pin(async move { with_cached_approval(&session.services, key, move || async move { session - .request_command_approval(turn, call_id, command, cwd, reason, risk) + .request_command_approval( + turn, + call_id, + command, + cwd, + reason, + risk, + req.approval_requirement.allow_prefix().cloned(), + ) .await }) .await diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index 5b18476bfc..0c6cf0bc73 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -123,7 +123,15 @@ impl Approvable for UnifiedExecRuntime<'_> { Box::pin(async move { with_cached_approval(&session.services, key, || async move { session - .request_command_approval(turn, call_id, command, cwd, reason, risk) + .request_command_approval( + turn, + call_id, + command, + cwd, + reason, + risk, + req.approval_requirement.allow_prefix().cloned(), + ) .await }) .await diff --git a/codex-rs/core/src/tools/sandboxing.rs b/codex-rs/core/src/tools/sandboxing.rs index e694c7fbef..057bfc2dee 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -92,11 +92,26 @@ pub(crate) enum ApprovalRequirement { /// No approval required for this tool call Skip, /// Approval required for this tool call - NeedsApproval { reason: Option }, + NeedsApproval { + reason: Option, + allow_prefix: Option>, + }, /// Execution forbidden for this tool call Forbidden { reason: String }, } +impl ApprovalRequirement { + pub fn allow_prefix(&self) -> Option<&Vec> { + match self { + Self::NeedsApproval { + allow_prefix: Some(prefix), + .. + } => Some(prefix), + _ => None, + } + } +} + /// - Never, OnFailure: do not ask /// - OnRequest: ask unless sandbox policy is DangerFullAccess /// - UnlessTrusted: always ask @@ -111,7 +126,10 @@ pub(crate) fn default_approval_requirement( }; if needs_approval { - ApprovalRequirement::NeedsApproval { reason: None } + ApprovalRequirement::NeedsApproval { + reason: None, + allow_prefix: None, + } } else { ApprovalRequirement::Skip } diff --git a/codex-rs/core/src/unified_exec/session_manager.rs b/codex-rs/core/src/unified_exec/session_manager.rs index 93340bb2d4..527c38f23d 100644 --- a/codex-rs/core/src/unified_exec/session_manager.rs +++ b/codex-rs/core/src/unified_exec/session_manager.rs @@ -445,6 +445,7 @@ impl UnifiedExecSessionManager { ) -> Result { let mut orchestrator = ToolOrchestrator::new(); let mut runtime = UnifiedExecRuntime::new(self); + let exec_policy = context.session.current_exec_policy().await; let req = UnifiedExecToolRequest::new( command.to_vec(), cwd, @@ -452,7 +453,7 @@ impl UnifiedExecSessionManager { with_escalated_permissions, justification, create_approval_requirement_for_command( - &context.turn.exec_policy, + exec_policy.as_ref(), command, context.turn.approval_policy, &context.turn.sandbox_policy, diff --git a/codex-rs/core/tests/suite/approvals.rs b/codex-rs/core/tests/suite/approvals.rs index a106d2eae0..a09594ca8e 100644 --- a/codex-rs/core/tests/suite/approvals.rs +++ b/codex-rs/core/tests/suite/approvals.rs @@ -1524,6 +1524,7 @@ async fn run_scenario(scenario: &ScenarioSpec) -> Result<()> { .submit(Op::ExecApproval { id: "0".into(), decision: *decision, + allow_prefix: None, }) .await?; wait_for_completion(&test).await; diff --git a/codex-rs/core/tests/suite/codex_delegate.rs b/codex-rs/core/tests/suite/codex_delegate.rs index 6339bfa71a..50ff1df986 100644 --- a/codex-rs/core/tests/suite/codex_delegate.rs +++ b/codex-rs/core/tests/suite/codex_delegate.rs @@ -93,6 +93,7 @@ async fn codex_delegate_forwards_exec_approval_and_proceeds_on_approval() { .submit(Op::ExecApproval { id: "0".into(), decision: ReviewDecision::Approved, + allow_prefix: None, }) .await .expect("submit exec approval"); diff --git a/codex-rs/core/tests/suite/otel.rs b/codex-rs/core/tests/suite/otel.rs index 8665d3a8ea..045c42e941 100644 --- a/codex-rs/core/tests/suite/otel.rs +++ b/codex-rs/core/tests/suite/otel.rs @@ -843,6 +843,7 @@ async fn handle_container_exec_user_approved_records_tool_decision() { .submit(Op::ExecApproval { id: "0".into(), decision: ReviewDecision::Approved, + allow_prefix: None, }) .await .unwrap(); @@ -901,6 +902,7 @@ async fn handle_container_exec_user_approved_for_session_records_tool_decision() .submit(Op::ExecApproval { id: "0".into(), decision: ReviewDecision::ApprovedForSession, + allow_prefix: None, }) .await .unwrap(); @@ -959,6 +961,7 @@ async fn handle_sandbox_error_user_approves_retry_records_tool_decision() { .submit(Op::ExecApproval { id: "0".into(), decision: ReviewDecision::Approved, + allow_prefix: None, }) .await .unwrap(); @@ -1017,6 +1020,7 @@ async fn handle_container_exec_user_denies_records_tool_decision() { .submit(Op::ExecApproval { id: "0".into(), decision: ReviewDecision::Denied, + allow_prefix: None, }) .await .unwrap(); @@ -1075,6 +1079,7 @@ async fn handle_sandbox_error_user_approves_for_session_records_tool_decision() .submit(Op::ExecApproval { id: "0".into(), decision: ReviewDecision::ApprovedForSession, + allow_prefix: None, }) .await .unwrap(); @@ -1134,6 +1139,7 @@ async fn handle_sandbox_error_user_denies_records_tool_decision() { .submit(Op::ExecApproval { id: "0".into(), decision: ReviewDecision::Denied, + allow_prefix: None, }) .await .unwrap(); diff --git a/codex-rs/execpolicy/Cargo.toml b/codex-rs/execpolicy/Cargo.toml index bececed4b2..1af1bd1b8e 100644 --- a/codex-rs/execpolicy/Cargo.toml +++ b/codex-rs/execpolicy/Cargo.toml @@ -27,3 +27,4 @@ thiserror = { workspace = true } [dev-dependencies] pretty_assertions = { workspace = true } +tempfile = { workspace = true } diff --git a/codex-rs/execpolicy/src/amend.rs b/codex-rs/execpolicy/src/amend.rs new file mode 100644 index 0000000000..614e13d076 --- /dev/null +++ b/codex-rs/execpolicy/src/amend.rs @@ -0,0 +1,143 @@ +use std::fs::OpenOptions; +use std::io::Write; +use std::path::Path; +use std::path::PathBuf; + +use serde_json; +use thiserror::Error; + +#[derive(Debug, Error)] +pub enum AmendError { + #[error("prefix rule requires at least one token")] + EmptyPrefix, + #[error("policy path has no parent: {path}")] + MissingParent { path: PathBuf }, + #[error("failed to create policy directory {dir}: {source}")] + CreatePolicyDir { + dir: PathBuf, + source: std::io::Error, + }, + #[error("failed to format prefix token {token}: {source}")] + SerializeToken { + token: String, + source: serde_json::Error, + }, + #[error("failed to open policy file {path}: {source}")] + OpenPolicyFile { + path: PathBuf, + source: std::io::Error, + }, + #[error("failed to write to policy file {path}: {source}")] + WritePolicyFile { + path: PathBuf, + source: std::io::Error, + }, + #[error("failed to read metadata for policy file {path}: {source}")] + PolicyMetadata { + path: PathBuf, + source: std::io::Error, + }, +} + +pub fn append_allow_prefix_rule(policy_path: &Path, prefix: &[String]) -> Result<(), AmendError> { + if prefix.is_empty() { + return Err(AmendError::EmptyPrefix); + } + + let tokens: Vec = prefix + .iter() + .map(|token| { + serde_json::to_string(token).map_err(|source| AmendError::SerializeToken { + token: token.clone(), + source, + }) + }) + .collect::>()?; + let pattern = tokens.join(", "); + let rule = format!("prefix_rule(pattern=[{pattern}], decision=\"allow\")\n"); + + let dir = policy_path + .parent() + .ok_or_else(|| AmendError::MissingParent { + path: policy_path.to_path_buf(), + })?; + match std::fs::create_dir(dir) { + Ok(()) => {} + Err(ref source) if source.kind() == std::io::ErrorKind::AlreadyExists => {} + Err(source) => { + return Err(AmendError::CreatePolicyDir { + dir: dir.to_path_buf(), + source, + }); + } + } + let mut file = OpenOptions::new() + .create(true) + .append(true) + .open(policy_path) + .map_err(|source| AmendError::OpenPolicyFile { + path: policy_path.to_path_buf(), + source, + })?; + let needs_newline = file + .metadata() + .map(|metadata| metadata.len() > 0) + .map_err(|source| AmendError::PolicyMetadata { + path: policy_path.to_path_buf(), + source, + })?; + let final_rule = if needs_newline { + format!("\n{rule}") + } else { + rule + }; + + file.write_all(final_rule.as_bytes()) + .map_err(|source| AmendError::WritePolicyFile { + path: policy_path.to_path_buf(), + source, + }) +} + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + use tempfile::tempdir; + + #[test] + fn appends_rule_and_creates_directories() { + let tmp = tempdir().expect("create temp dir"); + let policy_path = tmp.path().join("policy").join("default.codexpolicy"); + + append_allow_prefix_rule(&policy_path, &[String::from("echo"), String::from("Hello, world!")]) + .expect("append rule"); + + let contents = + std::fs::read_to_string(&policy_path).expect("default.codexpolicy should exist"); + assert_eq!( + contents, + "prefix_rule(pattern=[\"echo\", \"Hello, world!\"], decision=\"allow\")\n" + ); + } + + #[test] + fn separates_rules_with_newlines_when_appending() { + let tmp = tempdir().expect("create temp dir"); + let policy_path = tmp.path().join("policy").join("default.codexpolicy"); + std::fs::create_dir_all(policy_path.parent().unwrap()).expect("create policy dir"); + std::fs::write( + &policy_path, + "prefix_rule(pattern=[\"ls\"], decision=\"allow\")\n", + ) + .expect("write seed rule"); + + append_allow_prefix_rule(&policy_path, &[String::from("echo"), String::from("Hello, world!")]).expect("append rule"); + + let contents = std::fs::read_to_string(&policy_path).expect("read policy"); + assert_eq!( + contents, + "prefix_rule(pattern=[\"ls\"], decision=\"allow\")\n\nprefix_rule(pattern=[\"echo\", \"Hello, world!\"], decision=\"allow\")\n" + ); + } +} diff --git a/codex-rs/execpolicy/src/lib.rs b/codex-rs/execpolicy/src/lib.rs index 1b789fd862..a95d678b5d 100644 --- a/codex-rs/execpolicy/src/lib.rs +++ b/codex-rs/execpolicy/src/lib.rs @@ -1,9 +1,12 @@ +pub mod amend; pub mod decision; pub mod error; pub mod parser; pub mod policy; pub mod rule; +pub use amend::AmendError; +pub use amend::append_allow_prefix_rule; pub use decision::Decision; pub use error::Error; pub use error::Result; diff --git a/codex-rs/mcp-server/src/codex_tool_runner.rs b/codex-rs/mcp-server/src/codex_tool_runner.rs index 8dccb51250..7cfad0bcb5 100644 --- a/codex-rs/mcp-server/src/codex_tool_runner.rs +++ b/codex-rs/mcp-server/src/codex_tool_runner.rs @@ -180,6 +180,7 @@ async fn run_codex_tool_session_inner( call_id, reason: _, risk, + allow_prefix: _, parsed_cmd, }) => { handle_exec_approval_request( diff --git a/codex-rs/mcp-server/src/exec_approval.rs b/codex-rs/mcp-server/src/exec_approval.rs index 033523ac0d..e5b2e00f45 100644 --- a/codex-rs/mcp-server/src/exec_approval.rs +++ b/codex-rs/mcp-server/src/exec_approval.rs @@ -150,6 +150,7 @@ async fn on_exec_approval_response( .submit(Op::ExecApproval { id: event_id, decision: response.decision, + allow_prefix: None, }) .await { diff --git a/codex-rs/protocol/src/approvals.rs b/codex-rs/protocol/src/approvals.rs index 25f5e90e9e..e6c51a4f81 100644 --- a/codex-rs/protocol/src/approvals.rs +++ b/codex-rs/protocol/src/approvals.rs @@ -50,6 +50,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. + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional, type = "Array")] + pub allow_prefix: Option>, pub parsed_cmd: Vec, } diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index f20e412831..338d5f03df 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -143,6 +143,9 @@ pub enum Op { id: String, /// The user's decision in response to the request. decision: ReviewDecision, + /// When set, persist this prefix to the execpolicy allow list. + #[serde(default, skip_serializing_if = "Option::is_none")] + allow_prefix: Option>, }, /// Approve a code patch diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index ef709f0051..7ad8168153 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -41,6 +41,7 @@ pub(crate) enum ApprovalRequest { command: Vec, reason: Option, risk: Option, + allow_prefix: Option>, }, ApplyPatch { id: String, @@ -97,8 +98,8 @@ impl ApprovalOverlay { header: Box, ) -> (Vec, SelectionViewParams) { let (options, title) = match &variant { - ApprovalVariant::Exec { .. } => ( - exec_options(), + ApprovalVariant::Exec { allow_prefix, .. } => ( + exec_options(allow_prefix.clone()), "Would you like to run the following command?".to_string(), ), ApprovalVariant::ApplyPatch { .. } => ( @@ -150,8 +151,8 @@ impl ApprovalOverlay { }; if let Some(variant) = self.current_variant.as_ref() { match (&variant, option.decision) { - (ApprovalVariant::Exec { id, command }, decision) => { - self.handle_exec_decision(id, command, decision); + (ApprovalVariant::Exec { id, command, .. }, decision) => { + self.handle_exec_decision(id, command, decision, option.allow_prefix.clone()); } (ApprovalVariant::ApplyPatch { id, .. }, decision) => { self.handle_patch_decision(id, decision); @@ -163,12 +164,19 @@ impl ApprovalOverlay { self.advance_queue(); } - fn handle_exec_decision(&self, id: &str, command: &[String], decision: ReviewDecision) { + fn handle_exec_decision( + &self, + id: &str, + command: &[String], + decision: ReviewDecision, + allow_prefix: Option>, + ) { let cell = history_cell::new_approval_decision_cell(command.to_vec(), decision); self.app_event_tx.send(AppEvent::InsertHistoryCell(cell)); self.app_event_tx.send(AppEvent::CodexOp(Op::ExecApproval { id: id.to_string(), decision, + allow_prefix, })); } @@ -238,8 +246,8 @@ impl BottomPaneView for ApprovalOverlay { && let Some(variant) = self.current_variant.as_ref() { match &variant { - ApprovalVariant::Exec { id, command } => { - self.handle_exec_decision(id, command, ReviewDecision::Abort); + ApprovalVariant::Exec { id, command, .. } => { + self.handle_exec_decision(id, command, ReviewDecision::Abort, None); } ApprovalVariant::ApplyPatch { id, .. } => { self.handle_patch_decision(id, ReviewDecision::Abort); @@ -291,6 +299,7 @@ impl From for ApprovalRequestState { command, reason, risk, + allow_prefix, } => { let reason = reason.filter(|item| !item.is_empty()); let has_reason = reason.is_some(); @@ -310,7 +319,11 @@ impl From for ApprovalRequestState { } header.extend(full_cmd_lines); Self { - variant: ApprovalVariant::Exec { id, command }, + variant: ApprovalVariant::Exec { + id, + command, + allow_prefix, + }, header: Box::new(Paragraph::new(header).wrap(Wrap { trim: false })), } } @@ -364,8 +377,14 @@ fn render_risk_lines(risk: &SandboxCommandAssessment) -> Vec> { #[derive(Clone)] enum ApprovalVariant { - Exec { id: String, command: Vec }, - ApplyPatch { id: String }, + Exec { + id: String, + command: Vec, + allow_prefix: Option>, + }, + ApplyPatch { + id: String, + }, } #[derive(Clone)] @@ -374,6 +393,7 @@ struct ApprovalOption { decision: ReviewDecision, display_shortcut: Option, additional_shortcuts: Vec, + allow_prefix: Option>, } impl ApprovalOption { @@ -384,27 +404,39 @@ impl ApprovalOption { } } -fn exec_options() -> Vec { +fn exec_options(allow_prefix: Option>) -> Vec { vec![ ApprovalOption { label: "Yes, proceed".to_string(), decision: ReviewDecision::Approved, display_shortcut: None, additional_shortcuts: vec![key_hint::plain(KeyCode::Char('y'))], + allow_prefix: None, }, ApprovalOption { label: "Yes, and don't ask again for this command".to_string(), decision: ReviewDecision::ApprovedForSession, display_shortcut: None, additional_shortcuts: vec![key_hint::plain(KeyCode::Char('a'))], - }, - ApprovalOption { - label: "No, and tell Codex what to do differently".to_string(), - decision: ReviewDecision::Abort, - display_shortcut: Some(key_hint::plain(KeyCode::Esc)), - additional_shortcuts: vec![key_hint::plain(KeyCode::Char('n'))], + allow_prefix: None, }, ] + .into_iter() + .chain(allow_prefix.map(|prefix| ApprovalOption { + label: "Yes, and don't ask again for commands with this prefix".to_string(), + decision: ReviewDecision::ApprovedForSession, + display_shortcut: None, + additional_shortcuts: vec![key_hint::plain(KeyCode::Char('p'))], + allow_prefix: Some(prefix), + })) + .chain([ApprovalOption { + label: "No, and tell Codex what to do differently".to_string(), + decision: ReviewDecision::Abort, + display_shortcut: Some(key_hint::plain(KeyCode::Esc)), + additional_shortcuts: vec![key_hint::plain(KeyCode::Char('n'))], + allow_prefix: None, + }]) + .collect() } fn patch_options() -> Vec { @@ -414,12 +446,14 @@ fn patch_options() -> Vec { decision: ReviewDecision::Approved, display_shortcut: None, additional_shortcuts: vec![key_hint::plain(KeyCode::Char('y'))], + allow_prefix: None, }, ApprovalOption { label: "No, and tell Codex what to do differently".to_string(), decision: ReviewDecision::Abort, display_shortcut: Some(key_hint::plain(KeyCode::Esc)), additional_shortcuts: vec![key_hint::plain(KeyCode::Char('n'))], + allow_prefix: None, }, ] } @@ -437,6 +471,7 @@ mod tests { command: vec!["echo".to_string(), "hi".to_string()], reason: Some("reason".to_string()), risk: None, + allow_prefix: None, } } @@ -469,6 +504,41 @@ mod tests { assert!(saw_op, "expected approval decision to emit an op"); } + #[test] + fn exec_prefix_option_emits_allow_prefix() { + 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, + risk: None, + allow_prefix: Some(vec!["echo".to_string()]), + }, + tx, + ); + view.handle_key_event(KeyEvent::new(KeyCode::Char('p'), KeyModifiers::NONE)); + let mut saw_op = false; + while let Ok(ev) = rx.try_recv() { + if let AppEvent::CodexOp(Op::ExecApproval { + allow_prefix, + decision, + .. + }) = ev + { + assert_eq!(decision, ReviewDecision::ApprovedForSession); + assert_eq!(allow_prefix, Some(vec!["echo".to_string()])); + saw_op = true; + break; + } + } + assert!( + saw_op, + "expected approval decision to emit an op with allow prefix" + ); + } + #[test] fn header_includes_command_snippet() { let (tx, _rx) = unbounded_channel::(); @@ -479,6 +549,7 @@ mod tests { command, reason: None, risk: None, + allow_prefix: 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 6738d7672d..0ddb9c03cb 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -540,6 +540,7 @@ mod tests { command: vec!["echo".into(), "ok".into()], reason: None, risk: None, + allow_prefix: None, } } diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index a5728ab14f..2127f2fa16 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -1012,6 +1012,7 @@ impl ChatWidget { command: ev.command, reason: ev.reason, risk: ev.risk, + allow_prefix: ev.allow_prefix, }; 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 1393e52886..ba520f349c 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -587,6 +587,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, parsed_cmd: vec![], }; chat.handle_codex_event(Event { @@ -631,6 +632,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, parsed_cmd: vec![], }; chat.handle_codex_event(Event { @@ -681,6 +683,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, parsed_cmd: vec![], }; chat.handle_codex_event(Event { @@ -1830,6 +1833,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()]), parsed_cmd: vec![], }; chat.handle_codex_event(Event { @@ -1876,6 +1880,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()]), parsed_cmd: vec![], }; chat.handle_codex_event(Event { @@ -2089,6 +2094,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()]), parsed_cmd: vec![], }; chat.handle_codex_event(Event {