mirror of
https://github.com/openai/codex.git
synced 2026-10-22 17:58:46 +00:00
support PreToolUse approval decisions
This commit is contained in:
@@ -150,7 +150,7 @@ pub(crate) async fn run_pre_tool_use_hooks(
|
||||
permission_mode: hook_permission_mode(turn_context),
|
||||
tool_name: tool_name.name().to_string(),
|
||||
matcher_aliases: tool_name.matcher_aliases().to_vec(),
|
||||
tool_use_id,
|
||||
tool_use_id: tool_use_id.clone(),
|
||||
tool_input: tool_input.clone(),
|
||||
};
|
||||
let hooks = sess.hooks();
|
||||
@@ -162,9 +162,13 @@ pub(crate) async fn run_pre_tool_use_hooks(
|
||||
should_block,
|
||||
block_reason,
|
||||
additional_contexts,
|
||||
permission_decision,
|
||||
} = hooks.run_pre_tool_use(request).await;
|
||||
emit_hook_completed_events(sess, turn_context, hook_events).await;
|
||||
record_additional_contexts(sess, turn_context, additional_contexts).await;
|
||||
if let Some(permission_decision) = permission_decision {
|
||||
turn_context.record_pre_tool_use_permission_decision(tool_use_id, permission_decision);
|
||||
}
|
||||
|
||||
if should_block {
|
||||
block_reason.map(|reason| {
|
||||
|
||||
@@ -39,6 +39,7 @@ use codex_analytics::build_track_events_context;
|
||||
use codex_config::types::AppToolApproval;
|
||||
use codex_features::Feature;
|
||||
use codex_hooks::PermissionRequestDecision;
|
||||
use codex_hooks::PreToolUsePermissionDecision;
|
||||
use codex_mcp::CODEX_APPS_MCP_SERVER_NAME;
|
||||
use codex_mcp::McpPermissionPromptAutoApproveContext;
|
||||
use codex_mcp::SandboxState;
|
||||
@@ -954,20 +955,29 @@ async fn maybe_request_mcp_tool_approval(
|
||||
metadata: Option<&McpToolApprovalMetadata>,
|
||||
approval_mode: AppToolApproval,
|
||||
) -> Option<McpToolApprovalDecision> {
|
||||
if mcp_permission_prompt_is_auto_approved(
|
||||
let pre_tool_use_permission_decision = turn_context.pre_tool_use_permission_decision(call_id);
|
||||
let pre_tool_use_asks_user = matches!(
|
||||
pre_tool_use_permission_decision.as_ref(),
|
||||
Some(PreToolUsePermissionDecision::Ask { .. })
|
||||
);
|
||||
let pre_tool_use_allows = matches!(
|
||||
pre_tool_use_permission_decision.as_ref(),
|
||||
Some(PreToolUsePermissionDecision::Allow { .. })
|
||||
);
|
||||
let auto_approved_by_permissions = mcp_permission_prompt_is_auto_approved(
|
||||
turn_context.approval_policy.value(),
|
||||
&turn_context.permission_profile(),
|
||||
McpPermissionPromptAutoApproveContext {
|
||||
approvals_reviewer: Some(turn_context.config.approvals_reviewer),
|
||||
tool_approval_mode: Some(approval_mode),
|
||||
},
|
||||
) {
|
||||
return None;
|
||||
}
|
||||
);
|
||||
|
||||
let annotations = metadata.and_then(|metadata| metadata.annotations.as_ref());
|
||||
let approval_required = requires_mcp_tool_approval(annotations);
|
||||
if !approval_required && approval_mode != AppToolApproval::Prompt {
|
||||
let approval_required = !auto_approved_by_permissions
|
||||
&& (approval_required || approval_mode == AppToolApproval::Prompt);
|
||||
if pre_tool_use_allows || (!approval_required && !pre_tool_use_asks_user) {
|
||||
return None;
|
||||
}
|
||||
|
||||
@@ -999,35 +1009,38 @@ async fn maybe_request_mcp_tool_approval(
|
||||
let session_approval_key = session_mcp_tool_approval_key(invocation, metadata, approval_mode);
|
||||
let persistent_approval_key =
|
||||
persistent_mcp_tool_approval_key(invocation, metadata, approval_mode);
|
||||
if let Some(key) = session_approval_key.as_ref()
|
||||
if pre_tool_use_permission_decision.is_none()
|
||||
&& let Some(key) = session_approval_key.as_ref()
|
||||
&& mcp_tool_approval_is_remembered(sess, key).await
|
||||
{
|
||||
return Some(McpToolApprovalDecision::Accept);
|
||||
}
|
||||
|
||||
match run_permission_request_hooks(
|
||||
sess,
|
||||
turn_context,
|
||||
call_id,
|
||||
PermissionRequestPayload {
|
||||
tool_name: HookToolName::new(hook_tool_name),
|
||||
tool_input: invocation
|
||||
.arguments
|
||||
.clone()
|
||||
.unwrap_or_else(|| serde_json::Value::Object(serde_json::Map::new())),
|
||||
},
|
||||
)
|
||||
.await
|
||||
{
|
||||
Some(PermissionRequestDecision::Allow) => {
|
||||
return Some(McpToolApprovalDecision::Accept);
|
||||
if pre_tool_use_permission_decision.is_none() {
|
||||
match run_permission_request_hooks(
|
||||
sess,
|
||||
turn_context,
|
||||
call_id,
|
||||
PermissionRequestPayload {
|
||||
tool_name: HookToolName::new(hook_tool_name),
|
||||
tool_input: invocation
|
||||
.arguments
|
||||
.clone()
|
||||
.unwrap_or_else(|| serde_json::Value::Object(serde_json::Map::new())),
|
||||
},
|
||||
)
|
||||
.await
|
||||
{
|
||||
Some(PermissionRequestDecision::Allow) => {
|
||||
return Some(McpToolApprovalDecision::Accept);
|
||||
}
|
||||
Some(PermissionRequestDecision::Deny { message }) => {
|
||||
return Some(McpToolApprovalDecision::Decline {
|
||||
message: Some(message),
|
||||
});
|
||||
}
|
||||
None => {}
|
||||
}
|
||||
Some(PermissionRequestDecision::Deny { message }) => {
|
||||
return Some(McpToolApprovalDecision::Decline {
|
||||
message: Some(message),
|
||||
});
|
||||
}
|
||||
None => {}
|
||||
}
|
||||
|
||||
let tool_call_mcp_elicitation_enabled = turn_context
|
||||
@@ -1035,7 +1048,7 @@ async fn maybe_request_mcp_tool_approval(
|
||||
.features
|
||||
.enabled(Feature::ToolCallMcpElicitation);
|
||||
|
||||
if routes_approval_to_guardian(turn_context) {
|
||||
if routes_approval_to_guardian(turn_context) && !pre_tool_use_asks_user {
|
||||
let review_id = new_guardian_review_id();
|
||||
let decision = review_approval_request(
|
||||
sess,
|
||||
@@ -1084,8 +1097,13 @@ async fn maybe_request_mcp_tool_approval(
|
||||
.as_ref()
|
||||
.map(|rendered_template| rendered_template.question.as_str()),
|
||||
);
|
||||
question.question =
|
||||
mcp_tool_approval_question_text(question.question, monitor_reason.as_deref());
|
||||
let prompt_reason = match &pre_tool_use_permission_decision {
|
||||
Some(PreToolUsePermissionDecision::Ask { reason }) => {
|
||||
reason.as_deref().or(monitor_reason.as_deref())
|
||||
}
|
||||
Some(PreToolUsePermissionDecision::Allow { .. }) | None => monitor_reason.as_deref(),
|
||||
};
|
||||
question.question = mcp_tool_approval_question_text(question.question, prompt_reason);
|
||||
if tool_call_mcp_elicitation_enabled {
|
||||
let request_id = rmcp::model::RequestId::String(
|
||||
format!("{MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX}_{call_id}").into(),
|
||||
@@ -1103,7 +1121,7 @@ async fn maybe_request_mcp_tool_approval(
|
||||
tool_params_display: tool_params_display.as_deref(),
|
||||
question,
|
||||
message_override: rendered_template.as_ref().and_then(|rendered_template| {
|
||||
monitor_reason
|
||||
prompt_reason
|
||||
.is_none()
|
||||
.then_some(rendered_template.elicitation_message.as_str())
|
||||
}),
|
||||
|
||||
@@ -146,6 +146,7 @@ pub(super) async fn spawn_review_thread(
|
||||
codex_linux_sandbox_exe: parent_turn_context.codex_linux_sandbox_exe.clone(),
|
||||
tool_call_gate: Arc::new(ReadinessFlag::new()),
|
||||
dynamic_tools: parent_turn_context.dynamic_tools.clone(),
|
||||
pre_tool_use_permission_decisions: Default::default(),
|
||||
truncation_policy: model_info.truncation_policy.into(),
|
||||
turn_metadata_state,
|
||||
turn_skills: TurnSkillsContext::new(parent_turn_context.turn_skills.outcome.clone()),
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
use super::*;
|
||||
use crate::config::GhostSnapshotConfig;
|
||||
use codex_hooks::PreToolUsePermissionDecision;
|
||||
use codex_model_provider::SharedModelProvider;
|
||||
use codex_model_provider::create_model_provider;
|
||||
use codex_protocol::models::AdditionalPermissionProfile;
|
||||
@@ -7,6 +8,8 @@ use codex_protocol::protocol::TurnEnvironmentSelection;
|
||||
use codex_sandboxing::compatibility_sandbox_policy_for_permission_profile;
|
||||
use codex_sandboxing::policy_transforms::effective_file_system_sandbox_policy;
|
||||
use codex_sandboxing::policy_transforms::effective_network_sandbox_policy;
|
||||
use std::collections::HashMap;
|
||||
use std::sync::Mutex as StdMutex;
|
||||
use std::sync::atomic::AtomicBool;
|
||||
use std::sync::atomic::Ordering;
|
||||
|
||||
@@ -86,6 +89,8 @@ pub(crate) struct TurnContext {
|
||||
pub(crate) tool_call_gate: Arc<ReadinessFlag>,
|
||||
pub(crate) truncation_policy: TruncationPolicy,
|
||||
pub(crate) dynamic_tools: Vec<DynamicToolSpec>,
|
||||
pub(crate) pre_tool_use_permission_decisions:
|
||||
Arc<StdMutex<HashMap<String, PreToolUsePermissionDecision>>>,
|
||||
pub(crate) turn_metadata_state: Arc<TurnMetadataState>,
|
||||
pub(crate) turn_skills: TurnSkillsContext,
|
||||
pub(crate) turn_timing_state: Arc<TurnTimingState>,
|
||||
@@ -105,6 +110,28 @@ impl TurnContext {
|
||||
self.permission_profile.network_sandbox_policy()
|
||||
}
|
||||
|
||||
pub(crate) fn record_pre_tool_use_permission_decision(
|
||||
&self,
|
||||
tool_use_id: String,
|
||||
decision: PreToolUsePermissionDecision,
|
||||
) {
|
||||
self.pre_tool_use_permission_decisions
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.insert(tool_use_id, decision);
|
||||
}
|
||||
|
||||
pub(crate) fn pre_tool_use_permission_decision(
|
||||
&self,
|
||||
tool_use_id: &str,
|
||||
) -> Option<PreToolUsePermissionDecision> {
|
||||
self.pre_tool_use_permission_decisions
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.get(tool_use_id)
|
||||
.cloned()
|
||||
}
|
||||
|
||||
pub(crate) fn primary_environment(&self) -> Option<&TurnEnvironment> {
|
||||
self.environments.first()
|
||||
}
|
||||
@@ -257,6 +284,7 @@ impl TurnContext {
|
||||
tool_call_gate: Arc::new(ReadinessFlag::new()),
|
||||
truncation_policy,
|
||||
dynamic_tools: self.dynamic_tools.clone(),
|
||||
pre_tool_use_permission_decisions: self.pre_tool_use_permission_decisions.clone(),
|
||||
turn_metadata_state: self.turn_metadata_state.clone(),
|
||||
turn_skills: self.turn_skills.clone(),
|
||||
turn_timing_state: Arc::clone(&self.turn_timing_state),
|
||||
@@ -547,6 +575,7 @@ impl Session {
|
||||
tool_call_gate: Arc::new(ReadinessFlag::new()),
|
||||
truncation_policy: model_info.truncation_policy.into(),
|
||||
dynamic_tools: session_configuration.dynamic_tools.clone(),
|
||||
pre_tool_use_permission_decisions: Arc::new(StdMutex::new(HashMap::new())),
|
||||
turn_metadata_state,
|
||||
turn_skills: TurnSkillsContext::new(skills_outcome),
|
||||
turn_timing_state: Arc::new(TurnTimingState::default()),
|
||||
|
||||
@@ -11,6 +11,7 @@ use crate::session::session::Session;
|
||||
use crate::tools::sandboxing::PermissionRequestPayload;
|
||||
use crate::tools::sandboxing::ToolError;
|
||||
use codex_hooks::PermissionRequestDecision;
|
||||
use codex_hooks::PreToolUsePermissionDecision;
|
||||
use codex_network_proxy::BlockedRequest;
|
||||
use codex_network_proxy::BlockedRequestObserver;
|
||||
use codex_network_proxy::NetworkDecision;
|
||||
@@ -51,6 +52,7 @@ pub(crate) struct NetworkApprovalSpec {
|
||||
pub mode: NetworkApprovalMode,
|
||||
pub trigger: GuardianNetworkAccessTrigger,
|
||||
pub command: String,
|
||||
pub pre_tool_use_permission_decision: Option<codex_hooks::PreToolUsePermissionDecision>,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug)]
|
||||
@@ -224,6 +226,7 @@ struct ActiveNetworkApprovalCall {
|
||||
turn_id: String,
|
||||
trigger: GuardianNetworkAccessTrigger,
|
||||
command: String,
|
||||
pre_tool_use_permission_decision: Option<codex_hooks::PreToolUsePermissionDecision>,
|
||||
cancellation_token: CancellationToken,
|
||||
}
|
||||
|
||||
@@ -267,6 +270,7 @@ impl NetworkApprovalService {
|
||||
turn_id: String,
|
||||
trigger: GuardianNetworkAccessTrigger,
|
||||
command: String,
|
||||
pre_tool_use_permission_decision: Option<codex_hooks::PreToolUsePermissionDecision>,
|
||||
cancellation_token: CancellationToken,
|
||||
) {
|
||||
let mut calls = self.calls.lock().await;
|
||||
@@ -278,6 +282,7 @@ impl NetworkApprovalService {
|
||||
turn_id,
|
||||
trigger,
|
||||
command,
|
||||
pre_tool_use_permission_decision,
|
||||
cancellation_token,
|
||||
}),
|
||||
);
|
||||
@@ -464,13 +469,25 @@ impl NetworkApprovalService {
|
||||
let command = owner_call
|
||||
.as_ref()
|
||||
.map_or_else(|| prompt_command.join(" "), |call| call.command.clone());
|
||||
if let Some(permission_request_decision) = run_permission_request_hooks(
|
||||
&session,
|
||||
&turn_context,
|
||||
&guardian_approval_id,
|
||||
PermissionRequestPayload::bash(command, Some(format!("network-access {target}"))),
|
||||
)
|
||||
.await
|
||||
let pre_tool_use_permission_decision = owner_call
|
||||
.as_ref()
|
||||
.and_then(|call| call.pre_tool_use_permission_decision.as_ref());
|
||||
let pre_tool_use_asks_user = matches!(
|
||||
pre_tool_use_permission_decision,
|
||||
Some(PreToolUsePermissionDecision::Ask { .. })
|
||||
);
|
||||
let pre_tool_use_allows = matches!(
|
||||
pre_tool_use_permission_decision,
|
||||
Some(PreToolUsePermissionDecision::Allow { .. })
|
||||
);
|
||||
if pre_tool_use_permission_decision.is_none()
|
||||
&& let Some(permission_request_decision) = run_permission_request_hooks(
|
||||
&session,
|
||||
&turn_context,
|
||||
&guardian_approval_id,
|
||||
PermissionRequestPayload::bash(command, Some(format!("network-access {target}"))),
|
||||
)
|
||||
.await
|
||||
{
|
||||
match permission_request_decision {
|
||||
PermissionRequestDecision::Allow => {
|
||||
@@ -496,8 +513,17 @@ impl NetworkApprovalService {
|
||||
}
|
||||
}
|
||||
}
|
||||
let use_guardian = routes_approval_to_guardian(&turn_context);
|
||||
let guardian_review_id = use_guardian.then(new_guardian_review_id);
|
||||
if pre_tool_use_allows {
|
||||
pending
|
||||
.set_decision(PendingApprovalDecision::AllowOnce)
|
||||
.await;
|
||||
let mut pending_approvals = self.pending_host_approvals.lock().await;
|
||||
pending_approvals.remove(&key);
|
||||
return NetworkDecision::Allow;
|
||||
}
|
||||
let guardian_review_id = (routes_approval_to_guardian(&turn_context)
|
||||
&& !pre_tool_use_asks_user)
|
||||
.then(new_guardian_review_id);
|
||||
let approval_decision = if let Some(review_id) = guardian_review_id.clone() {
|
||||
review_approval_request(
|
||||
&session,
|
||||
@@ -526,7 +552,14 @@ impl NetworkApprovalService {
|
||||
/*approval_id*/ None,
|
||||
prompt_command,
|
||||
turn_context.cwd.clone(),
|
||||
Some(prompt_reason),
|
||||
match pre_tool_use_permission_decision {
|
||||
Some(PreToolUsePermissionDecision::Ask { reason }) => {
|
||||
reason.clone().or(Some(prompt_reason))
|
||||
}
|
||||
Some(PreToolUsePermissionDecision::Allow { .. }) | None => {
|
||||
Some(prompt_reason)
|
||||
}
|
||||
},
|
||||
Some(network_approval_context.clone()),
|
||||
/*proposed_execpolicy_amendment*/ None,
|
||||
/*additional_permissions*/ None,
|
||||
@@ -710,6 +743,7 @@ pub(crate) async fn begin_network_approval(
|
||||
mode,
|
||||
trigger,
|
||||
command,
|
||||
pre_tool_use_permission_decision,
|
||||
} = spec?;
|
||||
if !managed_network_active || network.is_none() {
|
||||
return None;
|
||||
@@ -725,6 +759,7 @@ pub(crate) async fn begin_network_approval(
|
||||
turn_id.to_string(),
|
||||
trigger,
|
||||
command,
|
||||
pre_tool_use_permission_decision,
|
||||
cancellation_token.clone(),
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -238,6 +238,7 @@ async fn register_call_with_default_shell_trigger(
|
||||
tty: None,
|
||||
},
|
||||
"curl https://example.com".to_string(),
|
||||
/*pre_tool_use_permission_decision*/ None,
|
||||
cancellation_token.clone(),
|
||||
)
|
||||
.await;
|
||||
@@ -264,6 +265,7 @@ async fn active_call_preserves_triggering_command_context() {
|
||||
"turn-1".to_string(),
|
||||
expected.clone(),
|
||||
"curl https://example.com".to_string(),
|
||||
/*pre_tool_use_permission_decision*/ None,
|
||||
CancellationToken::new(),
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -27,6 +27,7 @@ use crate::tools::sandboxing::ToolError;
|
||||
use crate::tools::sandboxing::ToolRuntime;
|
||||
use crate::tools::sandboxing::default_exec_approval_requirement;
|
||||
use codex_hooks::PermissionRequestDecision;
|
||||
use codex_hooks::PreToolUsePermissionDecision;
|
||||
use codex_otel::ToolDecisionSource;
|
||||
use codex_protocol::error::CodexErr;
|
||||
use codex_protocol::error::SandboxErr;
|
||||
@@ -138,7 +139,23 @@ impl ToolOrchestrator {
|
||||
let otel_tn = &tool_ctx.tool_name;
|
||||
let otel_ci = &tool_ctx.call_id;
|
||||
let strict_auto_review = tool_ctx.session.strict_auto_review_enabled_for_turn().await;
|
||||
let use_guardian = routes_approval_to_guardian(turn_ctx) || strict_auto_review;
|
||||
let pre_tool_use_permission_decision = tool_ctx
|
||||
.turn
|
||||
.pre_tool_use_permission_decision(&tool_ctx.call_id);
|
||||
let pre_tool_use_asks_user = matches!(
|
||||
pre_tool_use_permission_decision.as_ref(),
|
||||
Some(PreToolUsePermissionDecision::Ask { .. })
|
||||
);
|
||||
let pre_tool_use_allows = matches!(
|
||||
pre_tool_use_permission_decision.as_ref(),
|
||||
Some(PreToolUsePermissionDecision::Allow { .. })
|
||||
) && !strict_auto_review;
|
||||
let pre_tool_use_reason = match &pre_tool_use_permission_decision {
|
||||
Some(PreToolUsePermissionDecision::Ask { reason }) => reason.clone(),
|
||||
Some(PreToolUsePermissionDecision::Allow { .. }) | None => None,
|
||||
};
|
||||
let use_guardian = (routes_approval_to_guardian(turn_ctx) || strict_auto_review)
|
||||
&& !pre_tool_use_asks_user;
|
||||
|
||||
// 1) Approval
|
||||
let mut already_approved = false;
|
||||
@@ -150,15 +167,23 @@ impl ToolOrchestrator {
|
||||
});
|
||||
match requirement {
|
||||
ExecApprovalRequirement::Skip { .. } => {
|
||||
if strict_auto_review {
|
||||
let guardian_review_id = Some(new_guardian_review_id());
|
||||
if !strict_auto_review && !pre_tool_use_asks_user {
|
||||
otel.tool_decision(
|
||||
otel_tn,
|
||||
otel_ci,
|
||||
&ReviewDecision::Approved,
|
||||
ToolDecisionSource::Config,
|
||||
);
|
||||
} else {
|
||||
let guardian_review_id = use_guardian.then(new_guardian_review_id);
|
||||
let approval_ctx = ApprovalCtx {
|
||||
session: &tool_ctx.session,
|
||||
turn: &tool_ctx.turn,
|
||||
call_id: &tool_ctx.call_id,
|
||||
guardian_review_id: guardian_review_id.clone(),
|
||||
retry_reason: None,
|
||||
retry_reason: pre_tool_use_reason.clone(),
|
||||
network_approval_context: None,
|
||||
bypass_approval_cache: pre_tool_use_asks_user,
|
||||
};
|
||||
let decision = Self::request_approval(
|
||||
tool,
|
||||
@@ -167,48 +192,50 @@ impl ToolOrchestrator {
|
||||
approval_ctx,
|
||||
tool_ctx,
|
||||
/*evaluate_permission_request_hooks*/ false,
|
||||
&otel,
|
||||
)
|
||||
.await?;
|
||||
Self::reject_if_not_approved(tool_ctx, guardian_review_id.as_deref(), decision)
|
||||
.await?;
|
||||
already_approved = true;
|
||||
} else {
|
||||
otel.tool_decision(
|
||||
otel_tn,
|
||||
otel_ci,
|
||||
&ReviewDecision::Approved,
|
||||
ToolDecisionSource::Config,
|
||||
);
|
||||
}
|
||||
}
|
||||
ExecApprovalRequirement::Forbidden { reason } => {
|
||||
return Err(ToolError::Rejected(reason));
|
||||
}
|
||||
ExecApprovalRequirement::NeedsApproval { reason, .. } => {
|
||||
let guardian_review_id = use_guardian.then(new_guardian_review_id);
|
||||
let approval_ctx = ApprovalCtx {
|
||||
session: &tool_ctx.session,
|
||||
turn: &tool_ctx.turn,
|
||||
call_id: &tool_ctx.call_id,
|
||||
guardian_review_id: guardian_review_id.clone(),
|
||||
retry_reason: reason,
|
||||
network_approval_context: None,
|
||||
};
|
||||
let decision = Self::request_approval(
|
||||
tool,
|
||||
req,
|
||||
tool_ctx.call_id.as_str(),
|
||||
approval_ctx,
|
||||
tool_ctx,
|
||||
/*evaluate_permission_request_hooks*/ !strict_auto_review,
|
||||
&otel,
|
||||
)
|
||||
.await?;
|
||||
|
||||
Self::reject_if_not_approved(tool_ctx, guardian_review_id.as_deref(), decision)
|
||||
if pre_tool_use_allows {
|
||||
otel.tool_decision(
|
||||
otel_tn,
|
||||
otel_ci,
|
||||
&ReviewDecision::Approved,
|
||||
ToolDecisionSource::Config,
|
||||
);
|
||||
} else {
|
||||
let guardian_review_id = use_guardian.then(new_guardian_review_id);
|
||||
let approval_ctx = ApprovalCtx {
|
||||
session: &tool_ctx.session,
|
||||
turn: &tool_ctx.turn,
|
||||
call_id: &tool_ctx.call_id,
|
||||
guardian_review_id: guardian_review_id.clone(),
|
||||
retry_reason: pre_tool_use_reason.clone().or(reason),
|
||||
network_approval_context: None,
|
||||
bypass_approval_cache: pre_tool_use_asks_user,
|
||||
};
|
||||
let decision = Self::request_approval(
|
||||
tool,
|
||||
req,
|
||||
tool_ctx.call_id.as_str(),
|
||||
approval_ctx,
|
||||
tool_ctx,
|
||||
/*evaluate_permission_request_hooks*/
|
||||
!strict_auto_review && !pre_tool_use_asks_user,
|
||||
)
|
||||
.await?;
|
||||
already_approved = true;
|
||||
|
||||
Self::reject_if_not_approved(tool_ctx, guardian_review_id.as_deref(), decision)
|
||||
.await?;
|
||||
already_approved = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -318,15 +345,16 @@ impl ToolOrchestrator {
|
||||
let bypass_retry_approval = !strict_auto_review
|
||||
&& tool.should_bypass_approval(approval_policy, already_approved)
|
||||
&& network_approval_context.is_none();
|
||||
if !bypass_retry_approval {
|
||||
if !bypass_retry_approval && !pre_tool_use_allows {
|
||||
let guardian_review_id = use_guardian.then(new_guardian_review_id);
|
||||
let approval_ctx = ApprovalCtx {
|
||||
session: &tool_ctx.session,
|
||||
turn: &tool_ctx.turn,
|
||||
call_id: &tool_ctx.call_id,
|
||||
guardian_review_id: guardian_review_id.clone(),
|
||||
retry_reason: Some(retry_reason),
|
||||
retry_reason: pre_tool_use_reason.clone().or(Some(retry_reason)),
|
||||
network_approval_context: network_approval_context.clone(),
|
||||
bypass_approval_cache: pre_tool_use_asks_user,
|
||||
};
|
||||
|
||||
let permission_request_run_id = format!("{}:retry", tool_ctx.call_id);
|
||||
@@ -336,8 +364,8 @@ impl ToolOrchestrator {
|
||||
&permission_request_run_id,
|
||||
approval_ctx,
|
||||
tool_ctx,
|
||||
/*evaluate_permission_request_hooks*/ !strict_auto_review,
|
||||
&otel,
|
||||
/*evaluate_permission_request_hooks*/
|
||||
!strict_auto_review && !pre_tool_use_asks_user,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -389,11 +417,11 @@ impl ToolOrchestrator {
|
||||
approval_ctx: ApprovalCtx<'_>,
|
||||
tool_ctx: &ToolCtx,
|
||||
evaluate_permission_request_hooks: bool,
|
||||
otel: &codex_otel::SessionTelemetry,
|
||||
) -> Result<ReviewDecision, ToolError>
|
||||
where
|
||||
T: ToolRuntime<Rq, Out>,
|
||||
{
|
||||
let otel = approval_ctx.turn.session_telemetry.clone();
|
||||
if evaluate_permission_request_hooks
|
||||
&& let Some(permission_request) = tool.permission_request_payload(req)
|
||||
{
|
||||
@@ -428,7 +456,6 @@ impl ToolOrchestrator {
|
||||
None => {}
|
||||
}
|
||||
}
|
||||
|
||||
let otel_source = if approval_ctx.guardian_review_id.is_some() {
|
||||
ToolDecisionSource::AutomatedReviewer
|
||||
} else {
|
||||
|
||||
@@ -135,20 +135,19 @@ impl Approvable<ApplyPatchRequest> for ApplyPatchRuntime {
|
||||
return rx_approve.await.unwrap_or_default();
|
||||
}
|
||||
|
||||
with_cached_approval(
|
||||
&session.services,
|
||||
"apply_patch",
|
||||
approval_keys,
|
||||
|| async move {
|
||||
let rx_approve = session
|
||||
.request_patch_approval(
|
||||
turn, call_id, changes, /*reason*/ None, /*grant_root*/ None,
|
||||
)
|
||||
.await;
|
||||
rx_approve.await.unwrap_or_default()
|
||||
},
|
||||
)
|
||||
.await
|
||||
let fetch = || async move {
|
||||
let rx_approve = session
|
||||
.request_patch_approval(
|
||||
turn, call_id, changes, /*reason*/ None, /*grant_root*/ None,
|
||||
)
|
||||
.await;
|
||||
rx_approve.await.unwrap_or_default()
|
||||
};
|
||||
if ctx.bypass_approval_cache {
|
||||
fetch().await
|
||||
} else {
|
||||
with_cached_approval(&session.services, "apply_patch", approval_keys, fetch).await
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -175,7 +175,7 @@ impl Approvable<ShellRequest> for ShellRuntime {
|
||||
)
|
||||
.await;
|
||||
}
|
||||
with_cached_approval(&session.services, "shell", keys, move || async move {
|
||||
let fetch = move || async move {
|
||||
let available_decisions = None;
|
||||
session
|
||||
.request_command_approval(
|
||||
@@ -193,8 +193,12 @@ impl Approvable<ShellRequest> for ShellRuntime {
|
||||
available_decisions,
|
||||
)
|
||||
.await
|
||||
})
|
||||
.await
|
||||
};
|
||||
if ctx.bypass_approval_cache {
|
||||
fetch().await
|
||||
} else {
|
||||
with_cached_approval(&session.services, "shell", keys, fetch).await
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
@@ -236,6 +240,9 @@ impl ToolRuntime<ShellRequest, ExecToolCallOutput> for ShellRuntime {
|
||||
tty: None,
|
||||
},
|
||||
command: req.hook_command.clone(),
|
||||
pre_tool_use_permission_decision: ctx
|
||||
.turn
|
||||
.pre_tool_use_permission_decision(&ctx.call_id),
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -28,6 +28,7 @@ use codex_execpolicy::Policy;
|
||||
use codex_execpolicy::RuleMatch;
|
||||
use codex_features::Feature;
|
||||
use codex_hooks::PermissionRequestDecision;
|
||||
use codex_hooks::PreToolUsePermissionDecision;
|
||||
use codex_protocol::config_types::WindowsSandboxLevel;
|
||||
use codex_protocol::error::CodexErr;
|
||||
use codex_protocol::error::SandboxErr;
|
||||
@@ -403,38 +404,61 @@ impl CoreShellActionProvider {
|
||||
let call_id = self.call_id.clone();
|
||||
let approval_id = Some(Uuid::new_v4().to_string());
|
||||
let source = self.tool_name;
|
||||
let guardian_review_id = routes_approval_to_guardian(&turn).then(new_guardian_review_id);
|
||||
let pre_tool_use_permission_decision =
|
||||
self.turn.pre_tool_use_permission_decision(&self.call_id);
|
||||
let pre_tool_use_asks_user = matches!(
|
||||
pre_tool_use_permission_decision.as_ref(),
|
||||
Some(PreToolUsePermissionDecision::Ask { .. })
|
||||
);
|
||||
let pre_tool_use_allows = matches!(
|
||||
pre_tool_use_permission_decision.as_ref(),
|
||||
Some(PreToolUsePermissionDecision::Allow { .. })
|
||||
);
|
||||
let guardian_review_id = (routes_approval_to_guardian(&turn) && !pre_tool_use_asks_user)
|
||||
.then(new_guardian_review_id);
|
||||
Ok(stopwatch
|
||||
.pause_for(async move {
|
||||
// 1) Run PermissionRequest hooks
|
||||
let permission_request = PermissionRequestPayload::bash(
|
||||
codex_shell_command::parse_command::shlex_join(&command),
|
||||
/*description*/ None,
|
||||
);
|
||||
let effective_approval_id = approval_id.clone().unwrap_or_else(|| call_id.clone());
|
||||
match run_permission_request_hooks(
|
||||
&session,
|
||||
&turn,
|
||||
&effective_approval_id,
|
||||
permission_request,
|
||||
)
|
||||
.await
|
||||
{
|
||||
Some(PermissionRequestDecision::Allow) => {
|
||||
return PromptDecision {
|
||||
decision: ReviewDecision::Approved,
|
||||
guardian_review_id: None,
|
||||
rejection_message: None,
|
||||
};
|
||||
if pre_tool_use_allows {
|
||||
return PromptDecision {
|
||||
decision: ReviewDecision::Approved,
|
||||
guardian_review_id: None,
|
||||
rejection_message: None,
|
||||
};
|
||||
}
|
||||
|
||||
// 1) Run PermissionRequest hooks when PreToolUse did not already
|
||||
// provide the approval directive for this tool call.
|
||||
if pre_tool_use_permission_decision.is_none() {
|
||||
let permission_request = PermissionRequestPayload::bash(
|
||||
codex_shell_command::parse_command::shlex_join(&command),
|
||||
/*description*/ None,
|
||||
);
|
||||
let effective_approval_id =
|
||||
approval_id.clone().unwrap_or_else(|| call_id.clone());
|
||||
match run_permission_request_hooks(
|
||||
&session,
|
||||
&turn,
|
||||
&effective_approval_id,
|
||||
permission_request,
|
||||
)
|
||||
.await
|
||||
{
|
||||
Some(PermissionRequestDecision::Allow) => {
|
||||
return PromptDecision {
|
||||
decision: ReviewDecision::Approved,
|
||||
guardian_review_id: None,
|
||||
rejection_message: None,
|
||||
};
|
||||
}
|
||||
Some(PermissionRequestDecision::Deny { message }) => {
|
||||
return PromptDecision {
|
||||
decision: ReviewDecision::Denied,
|
||||
guardian_review_id: None,
|
||||
rejection_message: Some(message),
|
||||
};
|
||||
}
|
||||
None => {}
|
||||
}
|
||||
Some(PermissionRequestDecision::Deny { message }) => {
|
||||
return PromptDecision {
|
||||
decision: ReviewDecision::Denied,
|
||||
guardian_review_id: None,
|
||||
rejection_message: Some(message),
|
||||
};
|
||||
}
|
||||
None => {}
|
||||
}
|
||||
|
||||
// 2) Route to Guardian if configured
|
||||
@@ -469,7 +493,10 @@ impl CoreShellActionProvider {
|
||||
approval_id,
|
||||
command,
|
||||
workdir.clone(),
|
||||
/*reason*/ None,
|
||||
match pre_tool_use_permission_decision {
|
||||
Some(PreToolUsePermissionDecision::Ask { reason }) => reason,
|
||||
Some(PreToolUsePermissionDecision::Allow { .. }) | None => None,
|
||||
},
|
||||
/*network_approval_context*/ None,
|
||||
/*proposed_execpolicy_amendment*/ None,
|
||||
additional_permissions,
|
||||
|
||||
@@ -168,7 +168,7 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
|
||||
)
|
||||
.await;
|
||||
}
|
||||
with_cached_approval(&session.services, "unified_exec", keys, || async move {
|
||||
let fetch = || async move {
|
||||
let available_decisions = None;
|
||||
session
|
||||
.request_command_approval(
|
||||
@@ -186,8 +186,12 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
|
||||
available_decisions,
|
||||
)
|
||||
.await
|
||||
})
|
||||
.await
|
||||
};
|
||||
if ctx.bypass_approval_cache {
|
||||
fetch().await
|
||||
} else {
|
||||
with_cached_approval(&session.services, "unified_exec", keys, fetch).await
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
@@ -235,6 +239,9 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
tty: Some(req.tty),
|
||||
},
|
||||
command: req.hook_command.clone(),
|
||||
pre_tool_use_permission_decision: ctx
|
||||
.turn
|
||||
.pre_tool_use_permission_decision(&ctx.call_id),
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -130,6 +130,7 @@ pub(crate) struct ApprovalCtx<'a> {
|
||||
pub guardian_review_id: Option<String>,
|
||||
pub retry_reason: Option<String>,
|
||||
pub network_approval_context: Option<NetworkApprovalContext>,
|
||||
pub bypass_approval_cache: bool,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, PartialEq, Eq)]
|
||||
|
||||
@@ -13,6 +13,7 @@ use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::Op;
|
||||
use codex_protocol::protocol::ReviewDecision;
|
||||
use codex_protocol::protocol::RolloutItem;
|
||||
use codex_protocol::protocol::RolloutLine;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
@@ -33,6 +34,7 @@ use core_test_support::skip_if_windows;
|
||||
use core_test_support::streaming_sse::StreamingSseChunk;
|
||||
use core_test_support::streaming_sse::start_streaming_sse_server;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use core_test_support::test_codex::turn_permission_fields;
|
||||
use core_test_support::wait_for_event;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::Value;
|
||||
@@ -67,6 +69,38 @@ fn network_workspace_write_profile() -> PermissionProfile {
|
||||
)
|
||||
}
|
||||
|
||||
async fn submit_turn_without_waiting(
|
||||
test: &core_test_support::test_codex::TestCodex,
|
||||
prompt: &str,
|
||||
approval_policy: AskForApproval,
|
||||
permission_profile: PermissionProfile,
|
||||
) -> Result<()> {
|
||||
let (sandbox_policy, permission_profile) =
|
||||
turn_permission_fields(permission_profile, test.cwd.path());
|
||||
test.codex
|
||||
.submit(Op::UserTurn {
|
||||
environments: None,
|
||||
items: vec![UserInput::Text {
|
||||
text: prompt.into(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
cwd: test.cwd.path().to_path_buf(),
|
||||
approval_policy,
|
||||
approvals_reviewer: None,
|
||||
sandbox_policy,
|
||||
permission_profile,
|
||||
model: test.session_configured.model.clone(),
|
||||
effort: None,
|
||||
summary: None,
|
||||
service_tier: None,
|
||||
collaboration_mode: None,
|
||||
personality: None,
|
||||
})
|
||||
.await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn write_stop_hook(home: &Path, block_prompts: &[&str]) -> Result<()> {
|
||||
let script_path = home.join("stop_hook.py");
|
||||
let log_path = home.join("stop_hook_log.jsonl");
|
||||
@@ -237,6 +271,22 @@ if mode == "json_deny":
|
||||
"permissionDecisionReason": reason
|
||||
}}
|
||||
}}))
|
||||
elif mode == "json_allow":
|
||||
print(json.dumps({{
|
||||
"hookSpecificOutput": {{
|
||||
"hookEventName": "PreToolUse",
|
||||
"permissionDecision": "allow",
|
||||
"permissionDecisionReason": reason
|
||||
}}
|
||||
}}))
|
||||
elif mode == "json_ask":
|
||||
print(json.dumps({{
|
||||
"hookSpecificOutput": {{
|
||||
"hookEventName": "PreToolUse",
|
||||
"permissionDecision": "ask",
|
||||
"permissionDecisionReason": reason
|
||||
}}
|
||||
}}))
|
||||
elif mode == "context":
|
||||
print(json.dumps({{
|
||||
"hookSpecificOutput": {{
|
||||
@@ -1390,6 +1440,207 @@ async fn permission_request_hook_allows_shell_command_without_user_approval() ->
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn pre_tool_use_allow_skips_shell_command_approval() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let call_id = "pretooluse-allow-shell-command";
|
||||
let marker = std::env::temp_dir().join("pretooluse-allow-shell-command-marker");
|
||||
let command = format!("rm -f {}", marker.display());
|
||||
let args = serde_json::json!({ "command": command });
|
||||
let responses = mount_sse_sequence(
|
||||
&server,
|
||||
vec![
|
||||
sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_function_call(call_id, "shell_command", &serde_json::to_string(&args)?),
|
||||
ev_completed("resp-1"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-2"),
|
||||
ev_assistant_message("msg-1", "pre tool use allowed it"),
|
||||
ev_completed("resp-2"),
|
||||
]),
|
||||
],
|
||||
)
|
||||
.await;
|
||||
|
||||
let mut builder = test_codex()
|
||||
.with_pre_build_hook(|home| {
|
||||
if let Err(error) =
|
||||
write_pre_tool_use_hook(home, Some("^Bash$"), "json_allow", "approved by hook")
|
||||
{
|
||||
panic!("failed to write pre tool use hook test fixture: {error}");
|
||||
}
|
||||
})
|
||||
.with_config(|config| {
|
||||
config
|
||||
.features
|
||||
.enable(Feature::CodexHooks)
|
||||
.expect("test config should allow feature update");
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
fs::write(&marker, "seed").context("create pre tool use marker")?;
|
||||
|
||||
test.submit_turn_with_approval_and_permission_profile(
|
||||
"run the shell command after pre tool use approval",
|
||||
AskForApproval::OnRequest,
|
||||
PermissionProfile::Disabled,
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert_eq!(responses.requests().len(), 2);
|
||||
assert!(
|
||||
!marker.exists(),
|
||||
"pre tool use allow should execute without prompting"
|
||||
);
|
||||
assert!(
|
||||
timeout(
|
||||
Duration::from_secs(2),
|
||||
wait_for_event(&test.codex, |event| matches!(
|
||||
event,
|
||||
EventMsg::ExecApprovalRequest(_)
|
||||
))
|
||||
)
|
||||
.await
|
||||
.is_err(),
|
||||
"pre tool use allow should bypass the approval prompt"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn pre_tool_use_ask_prompts_user_with_hook_reason() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let call_id = "pretooluse-ask-shell-command";
|
||||
let marker = std::env::temp_dir().join("pretooluse-ask-shell-command-marker");
|
||||
let command = format!("rm -f {}", marker.display());
|
||||
let args = serde_json::json!({ "command": command });
|
||||
let _responses = mount_sse_sequence(
|
||||
&server,
|
||||
vec![
|
||||
sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_function_call(call_id, "shell_command", &serde_json::to_string(&args)?),
|
||||
ev_completed("resp-1"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-2"),
|
||||
ev_assistant_message("msg-1", "pre tool use asked first"),
|
||||
ev_completed("resp-2"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-3"),
|
||||
ev_function_call(
|
||||
"pretooluse-ask-shell-command-again",
|
||||
"shell_command",
|
||||
&serde_json::to_string(&args)?,
|
||||
),
|
||||
ev_completed("resp-3"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-4"),
|
||||
ev_assistant_message("msg-2", "pre tool use asked again"),
|
||||
ev_completed("resp-4"),
|
||||
]),
|
||||
],
|
||||
)
|
||||
.await;
|
||||
|
||||
let mut builder = test_codex()
|
||||
.with_pre_build_hook(|home| {
|
||||
if let Err(error) =
|
||||
write_pre_tool_use_hook(home, Some("^Bash$"), "json_ask", "please confirm")
|
||||
{
|
||||
panic!("failed to write pre tool use hook test fixture: {error}");
|
||||
}
|
||||
})
|
||||
.with_config(|config| {
|
||||
config
|
||||
.features
|
||||
.enable(Feature::CodexHooks)
|
||||
.expect("test config should allow feature update");
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
fs::write(&marker, "seed").context("create pre tool use marker")?;
|
||||
|
||||
submit_turn_without_waiting(
|
||||
&test,
|
||||
"run the shell command after pre tool use asks",
|
||||
AskForApproval::OnRequest,
|
||||
PermissionProfile::Disabled,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let approval_event = wait_for_event(&test.codex, |event| {
|
||||
matches!(event, EventMsg::ExecApprovalRequest(_))
|
||||
})
|
||||
.await;
|
||||
let EventMsg::ExecApprovalRequest(approval) = approval_event else {
|
||||
panic!("expected pre tool use ask to prompt the user");
|
||||
};
|
||||
assert_eq!(approval.reason.as_deref(), Some("please confirm"));
|
||||
|
||||
test.codex
|
||||
.submit(Op::ExecApproval {
|
||||
id: approval.effective_approval_id(),
|
||||
turn_id: None,
|
||||
decision: ReviewDecision::ApprovedForSession,
|
||||
})
|
||||
.await?;
|
||||
wait_for_event(&test.codex, |event| {
|
||||
matches!(event, EventMsg::TurnComplete(_))
|
||||
})
|
||||
.await;
|
||||
assert!(
|
||||
!marker.exists(),
|
||||
"approved command should remove marker file"
|
||||
);
|
||||
|
||||
fs::write(&marker, "seed again").context("recreate pre tool use marker")?;
|
||||
|
||||
submit_turn_without_waiting(
|
||||
&test,
|
||||
"run the same shell command after pre tool use asks again",
|
||||
AskForApproval::OnRequest,
|
||||
PermissionProfile::Disabled,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let second_approval_event = wait_for_event(&test.codex, |event| {
|
||||
matches!(event, EventMsg::ExecApprovalRequest(_))
|
||||
})
|
||||
.await;
|
||||
let EventMsg::ExecApprovalRequest(second_approval) = second_approval_event else {
|
||||
panic!("expected pre tool use ask to bypass cached approval");
|
||||
};
|
||||
assert_eq!(second_approval.reason.as_deref(), Some("please confirm"));
|
||||
|
||||
test.codex
|
||||
.submit(Op::ExecApproval {
|
||||
id: second_approval.effective_approval_id(),
|
||||
turn_id: None,
|
||||
decision: ReviewDecision::Approved,
|
||||
})
|
||||
.await?;
|
||||
wait_for_event(&test.codex, |event| {
|
||||
matches!(event, EventMsg::TurnComplete(_))
|
||||
})
|
||||
.await;
|
||||
assert!(
|
||||
!marker.exists(),
|
||||
"second approved command should remove marker file"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn permission_request_hook_allows_apply_patch_with_write_alias() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
@@ -52,12 +52,29 @@
|
||||
"type": "object"
|
||||
},
|
||||
"PreToolUsePermissionDecisionWire": {
|
||||
"enum": [
|
||||
"allow",
|
||||
"deny",
|
||||
"ask"
|
||||
],
|
||||
"type": "string"
|
||||
"oneOf": [
|
||||
{
|
||||
"enum": [
|
||||
"allow",
|
||||
"deny"
|
||||
],
|
||||
"type": "string"
|
||||
},
|
||||
{
|
||||
"description": "Always requests explicit human-user confirmation.",
|
||||
"enum": [
|
||||
"ask"
|
||||
],
|
||||
"type": "string"
|
||||
},
|
||||
{
|
||||
"description": "Reserved for the Codex exec integration.\n\nCodex does not implement this decision yet.",
|
||||
"enum": [
|
||||
"defer"
|
||||
],
|
||||
"type": "string"
|
||||
}
|
||||
]
|
||||
}
|
||||
},
|
||||
"properties": {
|
||||
|
||||
@@ -16,10 +16,17 @@ pub(crate) struct SessionStartOutput {
|
||||
pub(crate) struct PreToolUseOutput {
|
||||
pub universal: UniversalOutput,
|
||||
pub block_reason: Option<String>,
|
||||
pub permission_decision: Option<PreToolUsePermissionDecision>,
|
||||
pub additional_context: Option<String>,
|
||||
pub invalid_reason: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub(crate) enum PreToolUsePermissionDecision {
|
||||
Allow { reason: Option<String> },
|
||||
Ask { reason: Option<String> },
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub(crate) enum PermissionRequestDecision {
|
||||
Allow,
|
||||
@@ -125,10 +132,16 @@ pub(crate) fn parse_pre_tool_use(stdout: &str) -> Option<PreToolUseOutput> {
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let permission_decision = if invalid_reason.is_none() {
|
||||
hook_specific_output.and_then(pre_tool_use_permission_decision)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
Some(PreToolUseOutput {
|
||||
universal,
|
||||
block_reason,
|
||||
permission_decision,
|
||||
additional_context,
|
||||
invalid_reason,
|
||||
})
|
||||
@@ -344,12 +357,8 @@ fn unsupported_pre_tool_use_hook_specific_output(
|
||||
Some("PreToolUse hook returned unsupported updatedInput".to_string())
|
||||
} else {
|
||||
match output.permission_decision {
|
||||
Some(PreToolUsePermissionDecisionWire::Allow) => {
|
||||
Some("PreToolUse hook returned unsupported permissionDecision:allow".to_string())
|
||||
}
|
||||
Some(PreToolUsePermissionDecisionWire::Ask) => {
|
||||
Some("PreToolUse hook returned unsupported permissionDecision:ask".to_string())
|
||||
}
|
||||
Some(PreToolUsePermissionDecisionWire::Allow)
|
||||
| Some(PreToolUsePermissionDecisionWire::Ask) => None,
|
||||
Some(PreToolUsePermissionDecisionWire::Deny) => {
|
||||
if output
|
||||
.permission_decision_reason
|
||||
@@ -362,6 +371,9 @@ fn unsupported_pre_tool_use_hook_specific_output(
|
||||
None
|
||||
}
|
||||
}
|
||||
Some(PreToolUsePermissionDecisionWire::Defer) => {
|
||||
Some("PreToolUse hook returned unsupported permissionDecision:defer".to_string())
|
||||
}
|
||||
None => {
|
||||
if output.permission_decision_reason.is_some() {
|
||||
Some("PreToolUse hook returned permissionDecisionReason without permissionDecision".to_string())
|
||||
@@ -403,6 +415,29 @@ fn invalid_pre_tool_use_reason_message() -> String {
|
||||
.to_string()
|
||||
}
|
||||
|
||||
fn pre_tool_use_permission_decision(
|
||||
output: &crate::schema::PreToolUseHookSpecificOutputWire,
|
||||
) -> Option<PreToolUsePermissionDecision> {
|
||||
match output.permission_decision {
|
||||
Some(PreToolUsePermissionDecisionWire::Allow) => {
|
||||
Some(PreToolUsePermissionDecision::Allow {
|
||||
reason: output
|
||||
.permission_decision_reason
|
||||
.as_deref()
|
||||
.and_then(trimmed_reason),
|
||||
})
|
||||
}
|
||||
Some(PreToolUsePermissionDecisionWire::Ask) => Some(PreToolUsePermissionDecision::Ask {
|
||||
reason: output
|
||||
.permission_decision_reason
|
||||
.as_deref()
|
||||
.and_then(trimmed_reason),
|
||||
}),
|
||||
Some(PreToolUsePermissionDecisionWire::Deny | PreToolUsePermissionDecisionWire::Defer)
|
||||
| None => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn trimmed_reason(reason: &str) -> Option<String> {
|
||||
let trimmed = reason.trim();
|
||||
if trimmed.is_empty() {
|
||||
|
||||
@@ -16,6 +16,7 @@ use crate::engine::ConfiguredHandler;
|
||||
use crate::engine::command_runner::CommandRunResult;
|
||||
use crate::engine::dispatcher;
|
||||
use crate::engine::output_parser;
|
||||
use crate::engine::output_parser::PreToolUsePermissionDecision as ParsedPermissionDecision;
|
||||
use crate::schema::PreToolUseCommandInput;
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
@@ -37,13 +38,25 @@ pub struct PreToolUseOutcome {
|
||||
pub hook_events: Vec<HookCompletedEvent>,
|
||||
pub should_block: bool,
|
||||
pub block_reason: Option<String>,
|
||||
pub permission_decision: Option<PreToolUsePermissionDecision>,
|
||||
pub additional_contexts: Vec<String>,
|
||||
}
|
||||
|
||||
/// Hook-authored approval guidance to apply later at the concrete permission
|
||||
/// boundary for the same tool call.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub enum PreToolUsePermissionDecision {
|
||||
/// Skip an otherwise-required permission prompt when policy allows it.
|
||||
Allow { reason: Option<String> },
|
||||
/// Require explicit human-user confirmation.
|
||||
Ask { reason: Option<String> },
|
||||
}
|
||||
|
||||
#[derive(Debug, Default, PartialEq, Eq)]
|
||||
struct PreToolUseHandlerData {
|
||||
should_block: bool,
|
||||
block_reason: Option<String>,
|
||||
permission_decision: Option<PreToolUsePermissionDecision>,
|
||||
additional_contexts_for_model: Vec<String>,
|
||||
}
|
||||
|
||||
@@ -80,6 +93,7 @@ pub(crate) async fn run(
|
||||
hook_events: Vec::new(),
|
||||
should_block: false,
|
||||
block_reason: None,
|
||||
permission_decision: None,
|
||||
additional_contexts: Vec::new(),
|
||||
};
|
||||
}
|
||||
@@ -111,6 +125,11 @@ pub(crate) async fn run(
|
||||
let block_reason = results
|
||||
.iter()
|
||||
.find_map(|result| result.data.block_reason.clone());
|
||||
let permission_decision = resolve_permission_decision(
|
||||
results
|
||||
.iter()
|
||||
.filter_map(|result| result.data.permission_decision.as_ref()),
|
||||
);
|
||||
let additional_contexts = common::flatten_additional_contexts(
|
||||
results
|
||||
.iter()
|
||||
@@ -126,10 +145,33 @@ pub(crate) async fn run(
|
||||
.collect(),
|
||||
should_block,
|
||||
block_reason,
|
||||
permission_decision,
|
||||
additional_contexts,
|
||||
}
|
||||
}
|
||||
|
||||
fn resolve_permission_decision<'a>(
|
||||
decisions: impl IntoIterator<Item = &'a PreToolUsePermissionDecision>,
|
||||
) -> Option<PreToolUsePermissionDecision> {
|
||||
let mut resolved_allow = None;
|
||||
let mut resolved_ask = None;
|
||||
for decision in decisions {
|
||||
match decision {
|
||||
PreToolUsePermissionDecision::Allow { reason } => {
|
||||
resolved_allow = Some(PreToolUsePermissionDecision::Allow {
|
||||
reason: reason.clone(),
|
||||
});
|
||||
}
|
||||
PreToolUsePermissionDecision::Ask { reason } => {
|
||||
resolved_ask = Some(PreToolUsePermissionDecision::Ask {
|
||||
reason: reason.clone(),
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
resolved_ask.or(resolved_allow)
|
||||
}
|
||||
|
||||
/// Serializes command stdin for a selected `PreToolUse` hook.
|
||||
///
|
||||
/// Handler selection may include internal matcher aliases, but hook stdin keeps
|
||||
@@ -160,6 +202,7 @@ fn parse_completed(
|
||||
let mut status = HookRunStatus::Completed;
|
||||
let mut should_block = false;
|
||||
let mut block_reason = None;
|
||||
let mut permission_decision = None;
|
||||
let mut additional_contexts_for_model = Vec::new();
|
||||
|
||||
match run_result.error.as_deref() {
|
||||
@@ -204,6 +247,29 @@ fn parse_completed(
|
||||
text: reason,
|
||||
});
|
||||
}
|
||||
if let Some(parsed_decision) = parsed.permission_decision {
|
||||
let (decision, reason) = match parsed_decision {
|
||||
ParsedPermissionDecision::Allow { reason } => (
|
||||
PreToolUsePermissionDecision::Allow {
|
||||
reason: reason.clone(),
|
||||
},
|
||||
reason,
|
||||
),
|
||||
ParsedPermissionDecision::Ask { reason } => (
|
||||
PreToolUsePermissionDecision::Ask {
|
||||
reason: reason.clone(),
|
||||
},
|
||||
reason,
|
||||
),
|
||||
};
|
||||
if let Some(reason) = reason {
|
||||
entries.push(HookOutputEntry {
|
||||
kind: HookOutputEntryKind::Feedback,
|
||||
text: reason,
|
||||
});
|
||||
}
|
||||
permission_decision = Some(decision);
|
||||
}
|
||||
}
|
||||
} else if trimmed_stdout.starts_with('{') || trimmed_stdout.starts_with('[') {
|
||||
status = HookRunStatus::Failed;
|
||||
@@ -257,6 +323,7 @@ fn parse_completed(
|
||||
data: PreToolUseHandlerData {
|
||||
should_block,
|
||||
block_reason,
|
||||
permission_decision,
|
||||
additional_contexts_for_model,
|
||||
},
|
||||
}
|
||||
@@ -267,6 +334,7 @@ fn serialization_failure_outcome(hook_events: Vec<HookCompletedEvent>) -> PreToo
|
||||
hook_events,
|
||||
should_block: false,
|
||||
block_reason: None,
|
||||
permission_decision: None,
|
||||
additional_contexts: Vec::new(),
|
||||
}
|
||||
}
|
||||
@@ -283,6 +351,7 @@ mod tests {
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
use super::PreToolUseHandlerData;
|
||||
use super::PreToolUsePermissionDecision;
|
||||
use super::command_input_json;
|
||||
use super::parse_completed;
|
||||
use super::preview;
|
||||
@@ -319,6 +388,7 @@ mod tests {
|
||||
PreToolUseHandlerData {
|
||||
should_block: true,
|
||||
block_reason: Some("do not run that".to_string()),
|
||||
permission_decision: None,
|
||||
additional_contexts_for_model: Vec::new(),
|
||||
}
|
||||
);
|
||||
@@ -349,6 +419,7 @@ mod tests {
|
||||
PreToolUseHandlerData {
|
||||
should_block: true,
|
||||
block_reason: Some("do not run that".to_string()),
|
||||
permission_decision: None,
|
||||
additional_contexts_for_model: Vec::new(),
|
||||
}
|
||||
);
|
||||
@@ -379,6 +450,7 @@ mod tests {
|
||||
PreToolUseHandlerData {
|
||||
should_block: true,
|
||||
block_reason: Some("do not run that".to_string()),
|
||||
permission_decision: None,
|
||||
additional_contexts_for_model: vec!["remember this".to_string()],
|
||||
}
|
||||
);
|
||||
@@ -399,7 +471,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unsupported_permission_decision_fails_open() {
|
||||
fn permission_decision_ask_is_user_visible_without_model_context() {
|
||||
let parsed = parse_completed(
|
||||
&handler(),
|
||||
run_result(
|
||||
@@ -415,6 +487,73 @@ mod tests {
|
||||
PreToolUseHandlerData {
|
||||
should_block: false,
|
||||
block_reason: None,
|
||||
permission_decision: Some(PreToolUsePermissionDecision::Ask {
|
||||
reason: Some("please confirm".to_string()),
|
||||
}),
|
||||
additional_contexts_for_model: Vec::new(),
|
||||
}
|
||||
);
|
||||
assert_eq!(parsed.completed.run.status, HookRunStatus::Completed);
|
||||
assert_eq!(
|
||||
parsed.completed.run.entries,
|
||||
vec![HookOutputEntry {
|
||||
kind: HookOutputEntryKind::Feedback,
|
||||
text: "please confirm".to_string(),
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn permission_decision_allow_is_user_visible_without_model_context() {
|
||||
let parsed = parse_completed(
|
||||
&handler(),
|
||||
run_result(
|
||||
Some(0),
|
||||
r#"{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"allow","permissionDecisionReason":"approved by policy"}}"#,
|
||||
"",
|
||||
),
|
||||
Some("turn-1".to_string()),
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
parsed.data,
|
||||
PreToolUseHandlerData {
|
||||
should_block: false,
|
||||
block_reason: None,
|
||||
permission_decision: Some(PreToolUsePermissionDecision::Allow {
|
||||
reason: Some("approved by policy".to_string()),
|
||||
}),
|
||||
additional_contexts_for_model: Vec::new(),
|
||||
}
|
||||
);
|
||||
assert_eq!(parsed.completed.run.status, HookRunStatus::Completed);
|
||||
assert_eq!(
|
||||
parsed.completed.run.entries,
|
||||
vec![HookOutputEntry {
|
||||
kind: HookOutputEntryKind::Feedback,
|
||||
text: "approved by policy".to_string(),
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn permission_decision_defer_stays_unimplemented() {
|
||||
let parsed = parse_completed(
|
||||
&handler(),
|
||||
run_result(
|
||||
Some(0),
|
||||
r#"{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"defer"}}"#,
|
||||
"",
|
||||
),
|
||||
Some("turn-1".to_string()),
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
parsed.data,
|
||||
PreToolUseHandlerData {
|
||||
should_block: false,
|
||||
block_reason: None,
|
||||
permission_decision: None,
|
||||
additional_contexts_for_model: Vec::new(),
|
||||
}
|
||||
);
|
||||
@@ -423,7 +562,7 @@ mod tests {
|
||||
parsed.completed.run.entries,
|
||||
vec![HookOutputEntry {
|
||||
kind: HookOutputEntryKind::Error,
|
||||
text: "PreToolUse hook returned unsupported permissionDecision:ask".to_string(),
|
||||
text: "PreToolUse hook returned unsupported permissionDecision:defer".to_string(),
|
||||
}]
|
||||
);
|
||||
}
|
||||
@@ -441,6 +580,7 @@ mod tests {
|
||||
PreToolUseHandlerData {
|
||||
should_block: false,
|
||||
block_reason: None,
|
||||
permission_decision: None,
|
||||
additional_contexts_for_model: Vec::new(),
|
||||
}
|
||||
);
|
||||
@@ -471,6 +611,7 @@ mod tests {
|
||||
PreToolUseHandlerData {
|
||||
should_block: true,
|
||||
block_reason: Some("do not run that".to_string()),
|
||||
permission_decision: None,
|
||||
additional_contexts_for_model: vec!["nope".to_string()],
|
||||
}
|
||||
);
|
||||
@@ -503,6 +644,7 @@ mod tests {
|
||||
PreToolUseHandlerData {
|
||||
should_block: false,
|
||||
block_reason: None,
|
||||
permission_decision: None,
|
||||
additional_contexts_for_model: Vec::new(),
|
||||
}
|
||||
);
|
||||
@@ -523,6 +665,7 @@ mod tests {
|
||||
PreToolUseHandlerData {
|
||||
should_block: false,
|
||||
block_reason: None,
|
||||
permission_decision: None,
|
||||
additional_contexts_for_model: Vec::new(),
|
||||
}
|
||||
);
|
||||
@@ -549,6 +692,7 @@ mod tests {
|
||||
PreToolUseHandlerData {
|
||||
should_block: true,
|
||||
block_reason: Some("blocked by policy".to_string()),
|
||||
permission_decision: None,
|
||||
additional_contexts_for_model: Vec::new(),
|
||||
}
|
||||
);
|
||||
|
||||
@@ -34,6 +34,7 @@ pub use events::permission_request::PermissionRequestRequest;
|
||||
pub use events::post_tool_use::PostToolUseOutcome;
|
||||
pub use events::post_tool_use::PostToolUseRequest;
|
||||
pub use events::pre_tool_use::PreToolUseOutcome;
|
||||
pub use events::pre_tool_use::PreToolUsePermissionDecision;
|
||||
pub use events::pre_tool_use::PreToolUseRequest;
|
||||
pub use events::session_start::SessionStartOutcome;
|
||||
pub use events::session_start::SessionStartRequest;
|
||||
|
||||
@@ -198,8 +198,14 @@ pub(crate) enum PreToolUsePermissionDecisionWire {
|
||||
Allow,
|
||||
#[serde(rename = "deny")]
|
||||
Deny,
|
||||
/// Always requests explicit human-user confirmation.
|
||||
#[serde(rename = "ask")]
|
||||
Ask,
|
||||
/// Reserved for the Codex exec integration.
|
||||
///
|
||||
/// Codex does not implement this decision yet.
|
||||
#[serde(rename = "defer")]
|
||||
Defer,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, PartialEq, Eq)]
|
||||
|
||||
Reference in New Issue
Block a user