From 2061dddaa0c5e08f3bfec71b0a6a5b7cfa2f10b4 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 25 Feb 2026 01:27:37 -0800 Subject: [PATCH] feat: record whether a skill script is approved for the session --- codex-rs/core/src/codex.rs | 3 + codex-rs/core/src/state/service.rs | 4 ++ .../tools/runtimes/shell/unix_escalation.rs | 64 +++++++++++++++++-- 3 files changed, 67 insertions(+), 4 deletions(-) diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 0fe5f28523..9d31184f72 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -1348,6 +1348,7 @@ impl Session { otel_manager, models_manager: Arc::clone(&models_manager), tool_approvals: Mutex::new(ApprovalStore::default()), + execve_session_approvals: RwLock::new(HashSet::new()), skills_manager, file_watcher, agent_control, @@ -8221,6 +8222,7 @@ mod tests { otel_manager: otel_manager.clone(), models_manager: Arc::clone(&models_manager), tool_approvals: Mutex::new(ApprovalStore::default()), + execve_session_approvals: RwLock::new(HashSet::new()), skills_manager, file_watcher, agent_control, @@ -8377,6 +8379,7 @@ mod tests { otel_manager: otel_manager.clone(), models_manager: Arc::clone(&models_manager), tool_approvals: Mutex::new(ApprovalStore::default()), + execve_session_approvals: RwLock::new(HashSet::new()), skills_manager, file_watcher, agent_control, diff --git a/codex-rs/core/src/state/service.rs b/codex-rs/core/src/state/service.rs index 2a541d38a9..0ec03b139e 100644 --- a/codex-rs/core/src/state/service.rs +++ b/codex-rs/core/src/state/service.rs @@ -1,3 +1,4 @@ +use std::collections::HashSet; use std::sync::Arc; use crate::AuthManager; @@ -17,6 +18,7 @@ use crate::tools::sandboxing::ApprovalStore; use crate::unified_exec::UnifiedExecProcessManager; use codex_hooks::Hooks; use codex_otel::OtelManager; +use codex_utils_absolute_path::AbsolutePathBuf; use std::path::PathBuf; use tokio::sync::Mutex; use tokio::sync::RwLock; @@ -42,6 +44,8 @@ pub(crate) struct SessionServices { pub(crate) models_manager: Arc, pub(crate) otel_manager: OtelManager, pub(crate) tool_approvals: Mutex, + #[cfg_attr(not(unix), allow(dead_code))] + pub(crate) execve_session_approvals: RwLock>, pub(crate) skills_manager: Arc, pub(crate) file_watcher: Arc, pub(crate) agent_control: AgentControl, diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs index 6e8bd138fe..cacdf45948 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -166,6 +166,13 @@ struct CoreShellActionProvider { stopwatch: Stopwatch, } +enum DecisionSource { + SkillScript, + PrefixRule, + /// Often, this is `is_safe_command()`. + UnmatchedCommandFallback, +} + impl CoreShellActionProvider { fn decision_driven_by_policy(matched_rules: &[RuleMatch], decision: Decision) -> bool { matched_rules.iter().any(|rule_match| { @@ -181,6 +188,7 @@ impl CoreShellActionProvider { workdir: &AbsolutePathBuf, stopwatch: &Stopwatch, additional_permissions: Option, + available_decisions: Option>, ) -> anyhow::Result { let command = join_program_and_argv(program, argv); let workdir = workdir.to_path_buf(); @@ -191,7 +199,7 @@ impl CoreShellActionProvider { Ok(stopwatch .pause_for(async move { session - .request_command_approval( + .request_command_approval_with_options( &turn, call_id, approval_id, @@ -201,6 +209,7 @@ impl CoreShellActionProvider { None, None, additional_permissions, + available_decisions, ) .await }) @@ -241,6 +250,7 @@ impl CoreShellActionProvider { argv: &[String], workdir: &AbsolutePathBuf, additional_permissions: Option, + decision_source: DecisionSource, ) -> anyhow::Result { let action = match decision { Decision::Forbidden => EscalateAction::Deny { @@ -256,6 +266,14 @@ impl CoreShellActionProvider { reason: Some("Execution forbidden by policy".to_string()), } } else { + let available_decisions = + matches!(decision_source, DecisionSource::SkillScript).then(|| { + vec![ + ReviewDecision::Approved, + ReviewDecision::ApprovedForSession, + ReviewDecision::Abort, + ] + }); match self .prompt( program, @@ -263,12 +281,31 @@ impl CoreShellActionProvider { workdir, &self.stopwatch, additional_permissions, + available_decisions, ) .await? { ReviewDecision::Approved - | ReviewDecision::ApprovedExecpolicyAmendment { .. } - | ReviewDecision::ApprovedForSession => { + | ReviewDecision::ApprovedExecpolicyAmendment { .. } => { + if needs_escalation { + EscalateAction::Escalate + } else { + EscalateAction::Run + } + } + ReviewDecision::ApprovedForSession => { + // Currently, we only add session approvals for + // skill scripts because we are storing only the + // `program` whereas prefix rules may be restricted by a longer prefix. + if matches!(decision_source, DecisionSource::SkillScript) { + self.session + .services + .execve_session_approvals + .write() + .await + .insert(program.clone()); + } + if needs_escalation { EscalateAction::Escalate } else { @@ -337,14 +374,27 @@ impl EscalationPolicy for CoreShellActionProvider { // EscalateAction::Run case, rather than always escalating when a // skill matches. let needs_escalation = true; + let is_approved_for_session = self + .session + .services + .execve_session_approvals + .read() + .await + .contains(program); + let decision = if is_approved_for_session { + Decision::Allow + } else { + Decision::Prompt + }; return self .process_decision( - Decision::Prompt, + decision, needs_escalation, program, argv, workdir, skill.permission_profile.clone(), + DecisionSource::SkillScript, ) .await; } @@ -379,6 +429,11 @@ impl EscalationPolicy for CoreShellActionProvider { let needs_escalation = self.sandbox_permissions.requires_escalated_permissions() || decision_driven_by_policy; + let decision_source = if decision_driven_by_policy { + DecisionSource::PrefixRule + } else { + DecisionSource::UnmatchedCommandFallback + }; self.process_decision( evaluation.decision, needs_escalation, @@ -386,6 +441,7 @@ impl EscalationPolicy for CoreShellActionProvider { argv, workdir, None, + decision_source, ) .await }