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..d68146bac5 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -845,11 +845,43 @@ impl Session { .await } + pub(crate) async fn persist_command_allow_prefix( + &self, + prefix: &[String], + ) -> Result<(), crate::exec_policy::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 +890,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 +918,7 @@ impl Session { cwd, reason, risk, + allow_prefix, parsed_cmd, }); self.send_event(turn_context, event).await; @@ -1383,8 +1417,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 +1491,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 +1577,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/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 9525cc1acf..78ca60edba 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -4,10 +4,12 @@ use std::path::PathBuf; use std::sync::Arc; use crate::command_safety::is_dangerous_command::requires_initial_appoval; +use codex_execpolicy::AmendError; use codex_execpolicy::Decision; use codex_execpolicy::Evaluation; use codex_execpolicy::Policy; use codex_execpolicy::PolicyParser; +use codex_execpolicy::append_allow_prefix_rule; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::SandboxPolicy; use thiserror::Error; @@ -23,6 +25,7 @@ const FORBIDDEN_REASON: &str = "execpolicy forbids this command"; const PROMPT_REASON: &str = "execpolicy requires approval for this command"; const POLICY_DIR_NAME: &str = "policy"; const POLICY_EXTENSION: &str = "codexpolicy"; +const DEFAULT_POLICY_FILE: &str = "default.codexpolicy"; #[derive(Debug, Error)] pub enum ExecPolicyError { @@ -45,6 +48,15 @@ pub enum ExecPolicyError { }, } +#[derive(Debug, Error)] +pub enum ExecPolicyUpdateError { + #[error("failed to update execpolicy file {path}: {source}")] + AppendRule { path: PathBuf, source: AmendError }, + + #[error("failed to reload execpolicy after updating policy: {0}")] + Reload(#[from] ExecPolicyError), +} + pub(crate) async fn exec_policy_for( features: &Features, codex_home: &Path, @@ -84,33 +96,64 @@ pub(crate) async fn exec_policy_for( Ok(policy) } -fn evaluate_with_policy( - policy: &Policy, - command: &[String], - approval_policy: AskForApproval, -) -> Option { - let commands = parse_shell_lc_plain_commands(command).unwrap_or_else(|| vec![command.to_vec()]); - let evaluation = policy.check_multiple(commands.iter()); +pub(crate) fn default_policy_path(codex_home: &Path) -> PathBuf { + codex_home.join(POLICY_DIR_NAME).join(DEFAULT_POLICY_FILE) +} - match evaluation { - Evaluation::Match { decision, .. } => match decision { - Decision::Forbidden => Some(ApprovalRequirement::Forbidden { - reason: FORBIDDEN_REASON.to_string(), - }), - Decision::Prompt => { - let reason = PROMPT_REASON.to_string(); - if matches!(approval_policy, AskForApproval::Never) { - Some(ApprovalRequirement::Forbidden { reason }) - } else { - Some(ApprovalRequirement::NeedsApproval { - reason: Some(reason), - }) +pub(crate) async fn append_allow_prefix_rule_and_reload( + features: &Features, + codex_home: &Path, + prefix: &[String], +) -> Result, ExecPolicyUpdateError> { + let policy_path = default_policy_path(codex_home); + append_allow_prefix_rule(&policy_path, prefix).map_err(|source| { + ExecPolicyUpdateError::AppendRule { + path: policy_path, + source, + } + })?; + + exec_policy_for(features, codex_home) + .await + .map_err(ExecPolicyUpdateError::Reload) +} + +fn requirement_from_decision( + decision: Decision, + approval_policy: AskForApproval, +) -> ApprovalRequirement { + match decision { + Decision::Forbidden => ApprovalRequirement::Forbidden { + reason: FORBIDDEN_REASON.to_string(), + }, + Decision::Prompt => { + let reason = PROMPT_REASON.to_string(); + if matches!(approval_policy, AskForApproval::Never) { + ApprovalRequirement::Forbidden { reason } + } else { + ApprovalRequirement::NeedsApproval { + reason: Some(reason), + allow_prefix: None, } } - Decision::Allow => Some(ApprovalRequirement::Skip), - }, - Evaluation::NoMatch => None, + } + Decision::Allow => ApprovalRequirement::Skip, + } +} + +/// Return an allow-prefix option when a single plain command needs approval without +/// any matching policy rule. We only surface the prefix opt-in when execpolicy did +/// not already drive the decision (NoMatch) and when the command is a single +/// unrolled command (multi-part scripts shouldn’t be whitelisted via prefix). +fn allow_prefix_if_applicable( + commands: &[Vec], + evaluation: &Evaluation, +) -> Option> { + if matches!(evaluation, Evaluation::NoMatch) && commands.len() == 1 { + return Some(commands[0].clone()); } + + None } pub(crate) fn create_approval_requirement_for_command( @@ -120,19 +163,26 @@ pub(crate) fn create_approval_requirement_for_command( sandbox_policy: &SandboxPolicy, sandbox_permissions: SandboxPermissions, ) -> ApprovalRequirement { - if let Some(requirement) = evaluate_with_policy(policy, command, approval_policy) { - return requirement; - } + let commands = parse_shell_lc_plain_commands(command).unwrap_or_else(|| vec![command.to_vec()]); + let evaluation = policy.check_multiple(commands.iter()); - if requires_initial_appoval( - approval_policy, - sandbox_policy, - command, - sandbox_permissions, - ) { - ApprovalRequirement::NeedsApproval { reason: None } - } else { - ApprovalRequirement::Skip + match evaluation { + Evaluation::Match { decision, .. } => requirement_from_decision(decision, approval_policy), + Evaluation::NoMatch => { + if requires_initial_appoval( + approval_policy, + sandbox_policy, + command, + sandbox_permissions, + ) { + ApprovalRequirement::NeedsApproval { + reason: None, + allow_prefix: allow_prefix_if_applicable(&commands, &evaluation), + } + } else { + ApprovalRequirement::Skip + } + } } } @@ -280,9 +330,13 @@ prefix_rule(pattern=["rm"], decision="forbidden") "rm -rf /tmp".to_string(), ]; - let requirement = - evaluate_with_policy(&policy, &forbidden_script, AskForApproval::OnRequest) - .expect("expected match for forbidden command"); + let requirement = create_approval_requirement_for_command( + &policy, + &forbidden_script, + AskForApproval::OnRequest, + &SandboxPolicy::DangerFullAccess, + SandboxPermissions::UseDefault, + ); assert_eq!( requirement, @@ -313,7 +367,8 @@ prefix_rule(pattern=["rm"], decision="forbidden") assert_eq!( requirement, ApprovalRequirement::NeedsApproval { - reason: Some(PROMPT_REASON.to_string()) + reason: Some(PROMPT_REASON.to_string()), + allow_prefix: None, } ); } @@ -359,7 +414,83 @@ prefix_rule(pattern=["rm"], decision="forbidden") assert_eq!( requirement, - ApprovalRequirement::NeedsApproval { reason: None } + ApprovalRequirement::NeedsApproval { + reason: None, + allow_prefix: Some(command) + } + ); + } + + #[test] + fn allow_prefix_is_present_for_single_command_without_policy_match() { + let command = vec!["python".to_string()]; + + let empty_policy = Policy::empty(); + let requirement = create_approval_requirement_for_command( + &empty_policy, + &command, + AskForApproval::UnlessTrusted, + &SandboxPolicy::ReadOnly, + SandboxPermissions::UseDefault, + ); + + assert_eq!( + requirement, + ApprovalRequirement::NeedsApproval { + reason: None, + allow_prefix: Some(command) + } + ); + } + + #[test] + fn allow_prefix_is_omitted_when_policy_prompts() { + let policy_src = r#"prefix_rule(pattern=["rm"], decision="prompt")"#; + let mut parser = PolicyParser::new(); + parser + .parse("test.codexpolicy", policy_src) + .expect("parse policy"); + let policy = parser.build(); + let command = vec!["rm".to_string()]; + + let requirement = create_approval_requirement_for_command( + &policy, + &command, + AskForApproval::OnRequest, + &SandboxPolicy::DangerFullAccess, + SandboxPermissions::UseDefault, + ); + + assert_eq!( + requirement, + ApprovalRequirement::NeedsApproval { + reason: Some(PROMPT_REASON.to_string()), + allow_prefix: None, + } + ); + } + + #[test] + fn allow_prefix_is_omitted_for_multi_command_scripts() { + let command = vec![ + "bash".to_string(), + "-lc".to_string(), + "python && echo ok".to_string(), + ]; + let requirement = create_approval_requirement_for_command( + &Policy::empty(), + &command, + AskForApproval::UnlessTrusted, + &SandboxPolicy::ReadOnly, + SandboxPermissions::UseDefault, + ); + + assert_eq!( + requirement, + ApprovalRequirement::NeedsApproval { + reason: None, + allow_prefix: None, + } ); } } 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..3030a73683 100644 --- a/codex-rs/core/src/tools/runtimes/shell.rs +++ b/codex-rs/core/src/tools/runtimes/shell.rs @@ -100,13 +100,22 @@ impl Approvable for ShellRuntime { .clone() .or_else(|| req.justification.clone()); let risk = ctx.risk.clone(); + let allow_prefix = req.approval_requirement.allow_prefix().cloned(); let session = ctx.session; let turn = ctx.turn; let call_id = ctx.call_id.to_string(); 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, + allow_prefix, + ) .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..66f3e90806 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -120,10 +120,19 @@ impl Approvable for UnifiedExecRuntime<'_> { .clone() .or_else(|| req.justification.clone()); let risk = ctx.risk.clone(); + let allow_prefix = req.approval_requirement.allow_prefix().cloned(); 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, + allow_prefix, + ) .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..c1da6ee2e8 --- /dev/null +++ b/codex-rs/execpolicy/src/amend.rs @@ -0,0 +1,136 @@ +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 dir = policy_path + .parent() + .ok_or_else(|| AmendError::MissingParent { + path: policy_path.to_path_buf(), + })?; + std::fs::create_dir_all(dir).map_err(|source| AmendError::CreatePolicyDir { + dir: dir.to_path_buf(), + source, + })?; + + 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 needs_newline = policy_path + .metadata() + .map(|metadata| metadata.len() > 0) + .unwrap_or(false); + + let final_rule = if needs_newline { + format!("\n{rule}") + } else { + rule + }; + + let mut file = OpenOptions::new() + .create(true) + .append(true) + .open(policy_path) + .map_err(|source| AmendError::OpenPolicyFile { + path: policy_path.to_path_buf(), + source, + })?; + 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("bash"), String::from("-lc")]) + .expect("append rule"); + + let contents = + std::fs::read_to_string(&policy_path).expect("default.codexpolicy should exist"); + assert_eq!( + contents, + "prefix_rule(pattern=[\"bash\", \"-lc\"], 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("git")]).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=[\"git\"], 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..3aac56864e 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,43 @@ impl ApprovalOption { } } -fn exec_options() -> Vec { - vec![ - ApprovalOption { - label: "Yes, proceed".to_string(), - decision: ReviewDecision::Approved, - display_shortcut: None, - additional_shortcuts: vec![key_hint::plain(KeyCode::Char('y'))], - }, - ApprovalOption { - label: "Yes, and don't ask again for this command".to_string(), +fn exec_options(allow_prefix: Option>) -> Vec { + let mut options = Vec::new(); + options.push(ApprovalOption { + label: "Yes, proceed".to_string(), + decision: ReviewDecision::Approved, + display_shortcut: None, + additional_shortcuts: vec![key_hint::plain(KeyCode::Char('y'))], + allow_prefix: None, + }); + + options.push(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'))], + allow_prefix: None, + }); + + if let Some(prefix) = allow_prefix { + options.push(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('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'))], - }, - ] + additional_shortcuts: vec![key_hint::plain(KeyCode::Char('p'))], + allow_prefix: Some(prefix), + }); + } + + options.push(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, + }); + + options } fn patch_options() -> Vec { @@ -414,12 +450,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 +475,7 @@ mod tests { command: vec!["echo".to_string(), "hi".to_string()], reason: Some("reason".to_string()), risk: None, + allow_prefix: None, } } @@ -469,6 +508,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 +553,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/snapshots/codex_tui__chatwidget__tests__approval_modal_exec.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec.snap index b84588e337..9a0cf18f7f 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec.snap @@ -11,6 +11,7 @@ expression: terminal.backend().vt100().screen().contents() › 1. Yes, proceed (y) 2. Yes, and don't ask again for this command (a) - 3. No, and tell Codex what to do differently (esc) + 3. Yes, and don't ask again for commands with this prefix (p) + 4. No, and tell Codex what to do differently (esc) Press enter to confirm or esc to cancel diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 1393e52886..7a471c8abc 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: None, 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: None, parsed_cmd: vec![], }; chat.handle_codex_event(Event {