mirror of
https://github.com/openai/codex.git
synced 2026-09-08 15:50:34 +00:00
Add execpolicy match telemetry
Update exec approval requirements to carry whether a concrete execpolicy rule matched the command.\n\nEmit that metadata on codex.tool_decision so unmatched commands are queryable in telemetry, and update related tests and helper expectations.
This commit is contained in:
@@ -50,6 +50,7 @@ pub(crate) async fn apply_patch(
|
||||
action,
|
||||
auto_approved: !user_explicitly_approved,
|
||||
exec_approval_requirement: ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched: false,
|
||||
bypass_sandbox: false,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
@@ -62,6 +63,7 @@ pub(crate) async fn apply_patch(
|
||||
action,
|
||||
auto_approved: false,
|
||||
exec_approval_requirement: ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
|
||||
@@ -267,6 +267,7 @@ impl ExecPolicyManager {
|
||||
&exec_policy_fallback,
|
||||
&match_options,
|
||||
);
|
||||
let execpolicy_matched = evaluation.matched_rules.iter().any(is_policy_match);
|
||||
|
||||
let requested_amendment = derive_requested_execpolicy_amendment_from_prefix_rule(
|
||||
prefix_rule.as_ref(),
|
||||
@@ -279,6 +280,7 @@ impl ExecPolicyManager {
|
||||
|
||||
match evaluation.decision {
|
||||
Decision::Forbidden => ExecApprovalRequirement::Forbidden {
|
||||
execpolicy_matched,
|
||||
reason: derive_forbidden_reason(command, &evaluation),
|
||||
},
|
||||
Decision::Prompt => {
|
||||
@@ -287,9 +289,11 @@ impl ExecPolicyManager {
|
||||
});
|
||||
match prompt_is_rejected_by_policy(approval_policy, prompt_is_rule) {
|
||||
Some(reason) => ExecApprovalRequirement::Forbidden {
|
||||
execpolicy_matched,
|
||||
reason: reason.to_string(),
|
||||
},
|
||||
None => ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched,
|
||||
reason: derive_prompt_reason(command, &evaluation),
|
||||
proposed_execpolicy_amendment: requested_amendment.or_else(|| {
|
||||
if auto_amendment_allowed {
|
||||
@@ -304,6 +308,7 @@ impl ExecPolicyManager {
|
||||
}
|
||||
}
|
||||
Decision::Allow => ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched,
|
||||
// Bypass sandbox only when every parsed command segment is
|
||||
// explicitly allowed by execpolicy.
|
||||
bypass_sandbox: commands.iter().all(|command| {
|
||||
|
||||
@@ -636,6 +636,7 @@ async fn evaluates_bash_lc_inner_commands() {
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::Forbidden {
|
||||
execpolicy_matched: true,
|
||||
reason: "`bash -lc 'rm -rf /some/important/folder'` rejected: policy forbids commands starting with `rm`".to_string(),
|
||||
},
|
||||
)
|
||||
@@ -718,6 +719,7 @@ async fn evaluates_heredoc_script_against_prefix_rules() {
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched: true,
|
||||
bypass_sandbox: true,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
@@ -742,6 +744,7 @@ async fn omits_auto_amendment_for_heredoc_fallback_prompts() {
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
@@ -770,6 +773,7 @@ async fn drops_requested_amendment_for_heredoc_fallback_prompts_when_it_wont_mat
|
||||
]),
|
||||
},
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
@@ -803,6 +807,7 @@ prefix_rule(
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::Forbidden {
|
||||
execpolicy_matched: true,
|
||||
reason: "`rm -rf /some/important/folder` rejected: destructive command".to_string(),
|
||||
},
|
||||
)
|
||||
@@ -822,6 +827,7 @@ async fn exec_approval_requirement_prefers_execpolicy_match() {
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: true,
|
||||
reason: Some("`rm` requires approval by policy".to_string()),
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
@@ -850,6 +856,7 @@ prefix_rule(pattern=["git"], decision="allow")
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched: true,
|
||||
bypass_sandbox: true,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
@@ -884,6 +891,7 @@ prefix_rule(pattern=["git"], decision="prompt")
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched: false,
|
||||
bypass_sandbox: false,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![
|
||||
disallowed_git_path,
|
||||
@@ -911,6 +919,7 @@ async fn requested_prefix_rule_can_approve_absolute_path_commands() {
|
||||
prefix_rule: Some(vec!["cargo".to_string(), "install".to_string()]),
|
||||
},
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![
|
||||
"cargo".to_string(),
|
||||
@@ -934,6 +943,7 @@ async fn exec_approval_requirement_respects_approval_policy() {
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::Forbidden {
|
||||
execpolicy_matched: true,
|
||||
reason: PROMPT_CONFLICT_REASON.to_string(),
|
||||
},
|
||||
)
|
||||
@@ -998,6 +1008,7 @@ async fn exec_approval_requirement_prompts_for_inline_additional_permissions_und
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![
|
||||
"touch".to_string(),
|
||||
@@ -1028,6 +1039,7 @@ async fn exec_approval_requirement_rejects_unmatched_sandbox_escalation_when_gra
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::Forbidden {
|
||||
execpolicy_matched: false,
|
||||
reason: REJECT_SANDBOX_APPROVAL_REASON.to_string(),
|
||||
},
|
||||
)
|
||||
@@ -1105,6 +1117,7 @@ async fn mixed_rule_and_sandbox_prompt_rejects_when_granular_rules_are_disabled(
|
||||
assert_eq!(
|
||||
requirement,
|
||||
ExecApprovalRequirement::Forbidden {
|
||||
execpolicy_matched: true,
|
||||
reason: REJECT_RULES_APPROVAL_REASON.to_string(),
|
||||
}
|
||||
);
|
||||
@@ -1129,6 +1142,7 @@ async fn exec_approval_requirement_falls_back_to_heuristics() {
|
||||
assert_eq!(
|
||||
requirement,
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command))
|
||||
}
|
||||
@@ -1154,6 +1168,7 @@ async fn empty_bash_lc_script_falls_back_to_original_command() {
|
||||
assert_eq!(
|
||||
requirement,
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)),
|
||||
}
|
||||
@@ -1183,6 +1198,7 @@ async fn whitespace_bash_lc_script_falls_back_to_original_command() {
|
||||
assert_eq!(
|
||||
requirement,
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)),
|
||||
}
|
||||
@@ -1212,6 +1228,7 @@ async fn request_rule_uses_prefix_rule() {
|
||||
assert_eq!(
|
||||
requirement,
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![
|
||||
"cargo".to_string(),
|
||||
@@ -1244,6 +1261,7 @@ async fn request_rule_falls_back_when_prefix_rule_does_not_approve_all_commands(
|
||||
assert_eq!(
|
||||
requirement,
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![
|
||||
"rm".to_string(),
|
||||
@@ -1280,6 +1298,7 @@ async fn heuristics_apply_when_other_commands_match_policy() {
|
||||
})
|
||||
.await,
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: true,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![
|
||||
"orange".to_string()
|
||||
@@ -1354,6 +1373,7 @@ async fn proposed_execpolicy_amendment_is_present_for_single_command_without_pol
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)),
|
||||
},
|
||||
@@ -1374,6 +1394,7 @@ async fn proposed_execpolicy_amendment_is_omitted_when_policy_prompts() {
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: true,
|
||||
reason: Some("`rm` requires approval by policy".to_string()),
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
@@ -1398,6 +1419,7 @@ async fn proposed_execpolicy_amendment_is_present_for_multi_command_scripts() {
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![
|
||||
"cargo".to_string(),
|
||||
@@ -1428,6 +1450,7 @@ async fn proposed_execpolicy_amendment_uses_first_no_match_in_multi_command_scri
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: true,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![
|
||||
"apple".to_string(),
|
||||
@@ -1452,6 +1475,7 @@ async fn proposed_execpolicy_amendment_is_present_when_heuristics_allow() {
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched: false,
|
||||
bypass_sandbox: false,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)),
|
||||
},
|
||||
@@ -1472,6 +1496,7 @@ async fn proposed_execpolicy_amendment_is_suppressed_when_policy_matches_allow()
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched: true,
|
||||
bypass_sandbox: true,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
@@ -1503,6 +1528,7 @@ prefix_rule(pattern=["cat"], decision="allow")
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched: true,
|
||||
bypass_sandbox: false,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
@@ -1535,6 +1561,7 @@ prefix_rule(pattern=["bash"], decision="allow")
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched: true,
|
||||
bypass_sandbox: true,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
@@ -1740,6 +1767,7 @@ async fn verify_approval_requirement_for_unsafe_powershell_command() {
|
||||
that no sandbox is present, so anything that is not "provably
|
||||
safe" should require approval."#,
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: expected_amendment.clone(),
|
||||
},
|
||||
@@ -1748,6 +1776,7 @@ async fn verify_approval_requirement_for_unsafe_powershell_command() {
|
||||
(
|
||||
"On non-Windows, rely on the read-only sandbox to prevent harm.",
|
||||
ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched: false,
|
||||
bypass_sandbox: false,
|
||||
proposed_execpolicy_amendment: expected_amendment.clone(),
|
||||
},
|
||||
@@ -1772,6 +1801,7 @@ async fn verify_approval_requirement_for_unsafe_powershell_command() {
|
||||
let dangerous_command = vec_str(&["rm", "-rf", "/important/data"]);
|
||||
assert_eq!(
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec_str(&[
|
||||
"rm",
|
||||
@@ -1797,6 +1827,7 @@ async fn verify_approval_requirement_for_unsafe_powershell_command() {
|
||||
// AskForApproval::Never.
|
||||
assert_eq!(
|
||||
ExecApprovalRequirement::Forbidden {
|
||||
execpolicy_matched: false,
|
||||
reason: "`rm -rf /important/data` rejected: blocked by policy".to_string(),
|
||||
},
|
||||
policy
|
||||
@@ -1830,6 +1861,7 @@ async fn dangerous_command_allowed_when_sandbox_is_explicitly_disabled() {
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched: false,
|
||||
bypass_sandbox: false,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment {
|
||||
command: vec_str(&["rm", "-rf", "/tmp/nonexistent"]),
|
||||
@@ -1855,6 +1887,7 @@ async fn dangerous_command_forbidden_in_external_sandbox_when_policy_matches() {
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::Forbidden {
|
||||
execpolicy_matched: true,
|
||||
reason: "approval required by policy, but AskForApproval is set to Never".to_string(),
|
||||
},
|
||||
)
|
||||
|
||||
@@ -124,6 +124,7 @@ impl ToolOrchestrator {
|
||||
let requirement = tool.exec_approval_requirement(req).unwrap_or_else(|| {
|
||||
default_exec_approval_requirement(approval_policy, &turn_ctx.file_system_sandbox_policy)
|
||||
});
|
||||
let execpolicy_matched = requirement.execpolicy_matched();
|
||||
match requirement {
|
||||
ExecApprovalRequirement::Skip { .. } => {
|
||||
otel.tool_decision(
|
||||
@@ -131,9 +132,10 @@ impl ToolOrchestrator {
|
||||
otel_ci,
|
||||
&ReviewDecision::Approved,
|
||||
ToolDecisionSource::Config,
|
||||
Some(execpolicy_matched),
|
||||
);
|
||||
}
|
||||
ExecApprovalRequirement::Forbidden { reason } => {
|
||||
ExecApprovalRequirement::Forbidden { reason, .. } => {
|
||||
return Err(ToolError::Rejected(reason));
|
||||
}
|
||||
ExecApprovalRequirement::NeedsApproval { reason, .. } => {
|
||||
@@ -154,6 +156,7 @@ impl ToolOrchestrator {
|
||||
tool_ctx,
|
||||
use_guardian,
|
||||
&otel,
|
||||
Some(execpolicy_matched),
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -387,6 +390,7 @@ impl ToolOrchestrator {
|
||||
tool_ctx: &ToolCtx,
|
||||
use_guardian: bool,
|
||||
otel: &codex_otel::SessionTelemetry,
|
||||
execpolicy_matched: Option<bool>,
|
||||
) -> Result<ReviewDecision, ToolError>
|
||||
where
|
||||
T: ToolRuntime<Rq, Out>,
|
||||
@@ -407,6 +411,7 @@ impl ToolOrchestrator {
|
||||
&tool_ctx.call_id,
|
||||
&decision,
|
||||
ToolDecisionSource::Config,
|
||||
execpolicy_matched,
|
||||
);
|
||||
return Ok(decision);
|
||||
}
|
||||
@@ -417,6 +422,7 @@ impl ToolOrchestrator {
|
||||
&tool_ctx.call_id,
|
||||
&decision,
|
||||
ToolDecisionSource::Config,
|
||||
execpolicy_matched,
|
||||
);
|
||||
return Err(ToolError::Rejected(message));
|
||||
}
|
||||
@@ -435,6 +441,7 @@ impl ToolOrchestrator {
|
||||
&tool_ctx.call_id,
|
||||
&decision,
|
||||
otel_source,
|
||||
execpolicy_matched,
|
||||
);
|
||||
Ok(decision)
|
||||
}
|
||||
|
||||
@@ -58,6 +58,7 @@ fn guardian_review_request_includes_patch_context() {
|
||||
},
|
||||
)]),
|
||||
exec_approval_requirement: ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
@@ -95,6 +96,7 @@ fn file_system_sandbox_context_uses_active_attempt() {
|
||||
file_paths: vec![path.clone()],
|
||||
changes: HashMap::new(),
|
||||
exec_approval_requirement: ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched: false,
|
||||
bypass_sandbox: false,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
@@ -151,6 +153,7 @@ fn file_system_sandbox_context_omits_legacy_equivalent_policy() {
|
||||
file_paths: vec![path.clone()],
|
||||
changes: HashMap::new(),
|
||||
exec_approval_requirement: ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched: false,
|
||||
bypass_sandbox: false,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
@@ -192,6 +195,7 @@ fn no_sandbox_attempt_has_no_file_system_context() {
|
||||
file_paths: vec![path.clone()],
|
||||
changes: HashMap::new(),
|
||||
exec_approval_requirement: ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched: false,
|
||||
bypass_sandbox: false,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
|
||||
@@ -143,6 +143,8 @@ pub(crate) struct PermissionRequestPayload {
|
||||
pub(crate) enum ExecApprovalRequirement {
|
||||
/// No approval required for this tool call.
|
||||
Skip {
|
||||
/// Whether at least one concrete execpolicy rule matched this command.
|
||||
execpolicy_matched: bool,
|
||||
/// The first attempt should skip sandboxing (e.g., when explicitly
|
||||
/// greenlit by policy).
|
||||
bypass_sandbox: bool,
|
||||
@@ -152,13 +154,19 @@ pub(crate) enum ExecApprovalRequirement {
|
||||
},
|
||||
/// Approval required for this tool call.
|
||||
NeedsApproval {
|
||||
/// Whether at least one concrete execpolicy rule matched this command.
|
||||
execpolicy_matched: bool,
|
||||
reason: Option<String>,
|
||||
/// Proposed execpolicy amendment to skip future approvals for similar commands
|
||||
/// See core/src/exec_policy.rs for more details on how proposed_execpolicy_amendment is determined.
|
||||
proposed_execpolicy_amendment: Option<ExecPolicyAmendment>,
|
||||
},
|
||||
/// Execution forbidden for this tool call.
|
||||
Forbidden { reason: String },
|
||||
Forbidden {
|
||||
/// Whether at least one concrete execpolicy rule matched this command.
|
||||
execpolicy_matched: bool,
|
||||
reason: String,
|
||||
},
|
||||
}
|
||||
|
||||
impl ExecApprovalRequirement {
|
||||
@@ -175,6 +183,20 @@ impl ExecApprovalRequirement {
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
pub fn execpolicy_matched(&self) -> bool {
|
||||
match self {
|
||||
Self::Skip {
|
||||
execpolicy_matched, ..
|
||||
}
|
||||
| Self::NeedsApproval {
|
||||
execpolicy_matched, ..
|
||||
}
|
||||
| Self::Forbidden {
|
||||
execpolicy_matched, ..
|
||||
} => *execpolicy_matched,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// - Never, OnFailure: do not ask
|
||||
@@ -205,15 +227,18 @@ pub(crate) fn default_exec_approval_requirement(
|
||||
)
|
||||
{
|
||||
ExecApprovalRequirement::Forbidden {
|
||||
execpolicy_matched: false,
|
||||
reason: "approval policy disallowed sandbox approval prompt".to_string(),
|
||||
}
|
||||
} else if needs_approval {
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
}
|
||||
} else {
|
||||
ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched: false,
|
||||
bypass_sandbox: false,
|
||||
proposed_execpolicy_amendment: None,
|
||||
}
|
||||
|
||||
@@ -15,6 +15,7 @@ fn external_sandbox_skips_exec_approval_on_request() {
|
||||
&FileSystemSandboxPolicy::from(&sandbox_policy),
|
||||
),
|
||||
ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched: false,
|
||||
bypass_sandbox: false,
|
||||
proposed_execpolicy_amendment: None,
|
||||
}
|
||||
@@ -30,6 +31,7 @@ fn restricted_sandbox_requires_exec_approval_on_request() {
|
||||
&FileSystemSandboxPolicy::from(&sandbox_policy)
|
||||
),
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
}
|
||||
@@ -53,6 +55,7 @@ fn default_exec_approval_requirement_rejects_sandbox_prompt_when_granular_disabl
|
||||
assert_eq!(
|
||||
requirement,
|
||||
ExecApprovalRequirement::Forbidden {
|
||||
execpolicy_matched: false,
|
||||
reason: "approval policy disallowed sandbox approval prompt".to_string(),
|
||||
}
|
||||
);
|
||||
@@ -75,6 +78,7 @@ fn default_exec_approval_requirement_keeps_prompt_when_granular_allows_sandbox_a
|
||||
assert_eq!(
|
||||
requirement,
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
}
|
||||
@@ -87,6 +91,7 @@ fn additional_permissions_allow_bypass_sandbox_first_attempt_when_execpolicy_ski
|
||||
sandbox_override_for_first_attempt(
|
||||
SandboxPermissions::WithAdditionalPermissions,
|
||||
&ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched: false,
|
||||
bypass_sandbox: true,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
@@ -101,6 +106,7 @@ fn guardian_bypasses_sandbox_for_explicit_escalation_on_first_attempt() {
|
||||
sandbox_override_for_first_attempt(
|
||||
SandboxPermissions::RequireEscalated,
|
||||
&ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched: false,
|
||||
bypass_sandbox: false,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
|
||||
@@ -1040,10 +1040,12 @@ fn tool_decision_assertion<'a>(
|
||||
call_id: &'a str,
|
||||
expected_decision: &'a str,
|
||||
expected_source: &'a str,
|
||||
expected_execpolicy_matched: &'a str,
|
||||
) -> impl Fn(&[&str]) -> Result<(), String> + 'a {
|
||||
let call_id = call_id.to_string();
|
||||
let expected_decision = expected_decision.to_string();
|
||||
let expected_source = expected_source.to_string();
|
||||
let expected_execpolicy_matched = expected_execpolicy_matched.to_string();
|
||||
|
||||
move |lines: &[&str]| {
|
||||
let line = lines
|
||||
@@ -1063,6 +1065,11 @@ fn tool_decision_assertion<'a>(
|
||||
if !lower.contains(&format!("source={expected_source}")) {
|
||||
return Err(format!("unexpected source for {expected_source}"));
|
||||
}
|
||||
if !lower.contains(&format!("execpolicy_matched={expected_execpolicy_matched}")) {
|
||||
return Err(format!(
|
||||
"unexpected execpolicy_matched for {call_id}: expected {expected_execpolicy_matched}"
|
||||
));
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -1122,6 +1129,7 @@ async fn handle_container_exec_autoapprove_from_config_records_tool_decision() {
|
||||
"auto_config_call",
|
||||
"approved",
|
||||
"config",
|
||||
"false",
|
||||
));
|
||||
}
|
||||
|
||||
@@ -1189,6 +1197,7 @@ async fn handle_container_exec_user_approved_records_tool_decision() {
|
||||
"user_approved_call",
|
||||
"approved",
|
||||
"user",
|
||||
"false",
|
||||
));
|
||||
}
|
||||
|
||||
@@ -1256,6 +1265,7 @@ async fn handle_container_exec_user_approved_for_session_records_tool_decision()
|
||||
"user_approved_session_call",
|
||||
"approvedforsession",
|
||||
"user",
|
||||
"false",
|
||||
));
|
||||
}
|
||||
|
||||
@@ -1323,6 +1333,7 @@ async fn handle_sandbox_error_user_approves_retry_records_tool_decision() {
|
||||
"sandbox_retry_call",
|
||||
"approved",
|
||||
"user",
|
||||
"false",
|
||||
));
|
||||
}
|
||||
|
||||
@@ -1390,6 +1401,7 @@ async fn handle_container_exec_user_denies_records_tool_decision() {
|
||||
"user_denied_call",
|
||||
"denied",
|
||||
"user",
|
||||
"false",
|
||||
));
|
||||
}
|
||||
|
||||
@@ -1457,6 +1469,7 @@ async fn handle_sandbox_error_user_approves_for_session_records_tool_decision()
|
||||
"sandbox_session_call",
|
||||
"approvedforsession",
|
||||
"user",
|
||||
"false",
|
||||
));
|
||||
}
|
||||
|
||||
@@ -1525,5 +1538,6 @@ async fn handle_sandbox_error_user_denies_records_tool_decision() {
|
||||
"sandbox_deny_call",
|
||||
"denied",
|
||||
"user",
|
||||
"false",
|
||||
));
|
||||
}
|
||||
|
||||
@@ -867,6 +867,7 @@ impl SessionTelemetry {
|
||||
call_id: &str,
|
||||
decision: &ReviewDecision,
|
||||
source: ToolDecisionSource,
|
||||
execpolicy_matched: Option<bool>,
|
||||
) {
|
||||
log_event!(
|
||||
self,
|
||||
@@ -875,6 +876,9 @@ impl SessionTelemetry {
|
||||
call_id = %call_id,
|
||||
decision = %decision.clone().to_string().to_lowercase(),
|
||||
source = %source.to_string(),
|
||||
execpolicy_matched = %execpolicy_matched
|
||||
.map(|matched| matched.to_string())
|
||||
.unwrap_or_default(),
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user