feat: record whether a skill script is approved for the session

This commit is contained in:
Michael Bolin
2026-02-25 00:41:50 -08:00
parent c4ec6be4ab
commit 433fbd85ff
3 changed files with 60 additions and 3 deletions

View File

@@ -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,

View File

@@ -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,7 @@ pub(crate) struct SessionServices {
pub(crate) models_manager: Arc<ModelsManager>,
pub(crate) otel_manager: OtelManager,
pub(crate) tool_approvals: Mutex<ApprovalStore>,
pub(crate) execve_session_approvals: RwLock<HashSet<AbsolutePathBuf>>,
pub(crate) skills_manager: Arc<SkillsManager>,
pub(crate) file_watcher: Arc<FileWatcher>,
pub(crate) agent_control: AgentControl,

View File

@@ -165,6 +165,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| {
@@ -238,6 +245,7 @@ impl CoreShellActionProvider {
program: &AbsolutePathBuf,
argv: &[String],
workdir: &AbsolutePathBuf,
decision_source: DecisionSource,
) -> anyhow::Result<EscalateAction> {
let action = match decision {
Decision::Forbidden => EscalateAction::Deny {
@@ -255,8 +263,26 @@ impl CoreShellActionProvider {
} else {
match self.prompt(program, argv, workdir, &self.stopwatch).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 {
@@ -325,8 +351,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, needs_escalation, program, argv, workdir)
.process_decision(
decision,
needs_escalation,
program,
argv,
workdir,
DecisionSource::SkillScript,
)
.await;
}
@@ -360,12 +405,18 @@ 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,
program,
argv,
workdir,
decision_source,
)
.await
}