mirror of
https://github.com/codewhale-hq/Codewhale.git
synced 2026-11-09 01:49:49 -10:00
fix(computer-use): let Full Access ask for consent instead of denying it (CW-7495)
Computer Use could not be authorized at all in Full Access. Asking for the
plugin's standing per-app grant was refused before it ran, so the most
permissive posture was the one posture where the feature could never work,
while the stricter Ask posture worked fine. Live reproduction: the same call
was denied in Full Access and succeeded the moment the session switched to Ask.
Three separate guards all assumed Ask-only, and each had to change:
- tool_preparation.rs: `card_available` required `!auto_approve && Suggest`.
- turn_loop.rs: an execution-time guard denied every non-Suggest posture with
"require the user's exact approval in Ask posture". This is the one that
actually fired.
- tui/ui/approval_routing.rs: the disposition resolver turns a forced prompt
under Full Access into AutoDenyFullAccessPolicyHold, which a UI that always
has a person present should not do for a call whose entire purpose is to
carry that person's answer.
Ask and Full Access now reach the card. Auto-Review and Never still fail
closed, because neither has anyone at the card; their refusal names the remedy
rather than the removed Ask-only rule.
The property K1 protects is unchanged: the call still requires approval, is
never auto-approved in any posture, only a real card allow becomes an attested
HumanDecision, and the model cannot widen a grant on its own. Headless exec
denies forced prompts before this code.
Verified live on the rebuilt binary, session in Full Access:
- `computer_consent {action:"allow", app:"com.apple.finder", scope:"app"}`
raised an approval card (previously a flat denial). Approved once.
- Receipt: decision allow, persisted false, "Finder allowed for this session."
- The grant is usable, not merely recorded: a subsequent
`computer_get_app_state` for Finder returned its 900-element tree unprompted.
- The plugin still refuses a model-issued grant off the host path:
consent_needs_user.
Not verified: a full `--lib` test build. Another agent's in-flight edits to
review fixtures and web/guard.rs broke the test target for this run; the lib
check is clean and the binary above was built from this tree.
This commit is contained in:
1 parent
d1fd6f1a31
commit
ad697b35f5
4 files changed
+79
-27
No files matched your search
@@ -60,9 +60,10 @@ pub(super) fn prepare_tool_call(
|
||||
}
|
||||
// K1/K2 stopgap: Computer Use consent and `app_script` need a human
|
||||
// decision. Never auto-approve them, and refuse them outright in a
|
||||
// posture that cannot open the approval card (Full Access,
|
||||
// Auto-Review, Never) — otherwise the model's own tool call would be
|
||||
// the consent.
|
||||
// posture that has no person at the card (Auto-Review, Never). Full
|
||||
// Access is not such a posture: it has a UI, so it asks. Denying there
|
||||
// made the most permissive posture the one posture where Computer Use
|
||||
// could never be authorized at all.
|
||||
if let Some(inner) = computer_use_batch_hidden_gate(name, &input) {
|
||||
return Err(ToolError::permission_denied(format!(
|
||||
"Computer Use {inner} cannot run inside {name}: consent and scripts need their own approval card. Call it on its own so the user can decide."
|
||||
@@ -73,11 +74,12 @@ pub(super) fn prepare_tool_call(
|
||||
let context = registry.context();
|
||||
(context.auto_approve, context.approval_mode)
|
||||
});
|
||||
let card_available = !session_auto_approve
|
||||
&& posture.is_none_or(|(auto_approve, approval_mode)| {
|
||||
!auto_approve && approval_mode == ApprovalMode::Suggest
|
||||
});
|
||||
if !card_available {
|
||||
let can_ask = posture.is_none_or(|(auto_approve, approval_mode)| {
|
||||
approval_mode == ApprovalMode::Suggest
|
||||
|| auto_approve
|
||||
|| approval_mode == ApprovalMode::Bypass
|
||||
});
|
||||
if !can_ask {
|
||||
let label = posture.map_or("Full Access", |(auto_approve, approval_mode)| {
|
||||
if auto_approve {
|
||||
ApprovalMode::Bypass.permission_chip_label()
|
||||
@@ -86,7 +88,7 @@ pub(super) fn prepare_tool_call(
|
||||
}
|
||||
});
|
||||
return Err(ToolError::permission_denied(format!(
|
||||
"Computer Use call {name} needs your own approval: consent and scripts cannot be granted by a model tool call, and the current {label} posture cannot show an approval card. Switch to Ask mode to review it."
|
||||
"Computer Use call {name} needs your own approval, and the current {label} posture has no one to ask: consent and scripts cannot be granted by a model tool call. Switch to Ask or Full Access to review it."
|
||||
)));
|
||||
}
|
||||
return Ok(PreparedToolPolicy {
|
||||
@@ -609,23 +611,19 @@ mod tests {
|
||||
));
|
||||
}
|
||||
|
||||
/// K1: the model cannot grant itself Computer Use consent. In a posture
|
||||
/// that cannot show a human card the call is refused at preparation; in
|
||||
/// Ask it always requires approval, is never session auto-approved, and a
|
||||
/// session grant for one app does not cover another.
|
||||
/// K1: the model cannot grant itself Computer Use consent. A posture with
|
||||
/// no person at the card refuses at preparation; Ask and Full Access both
|
||||
/// reach the card, always require approval, are never session
|
||||
/// auto-approved, and a session grant for one app does not cover another.
|
||||
#[test]
|
||||
fn model_issued_computer_use_consent_is_rejected_without_a_human_card() {
|
||||
fn model_issued_computer_use_consent_never_auto_approves() {
|
||||
let consent = "mcp_plugin-12-computer-use-computer_consent";
|
||||
let allow_safari =
|
||||
json!({"action": "allow", "app": "Safari", "bundle_id": "com.apple.Safari"});
|
||||
let foreground = json!({"action": "allow", "scope": "foreground"});
|
||||
|
||||
// Full Access (session bit, or the registry context) and every
|
||||
// no-card posture refuse the call before any approval routing.
|
||||
// Postures with nobody at the card refuse before approval routing.
|
||||
for (session_auto, context_auto, mode) in [
|
||||
(true, false, ApprovalMode::Suggest),
|
||||
(false, true, ApprovalMode::Suggest),
|
||||
(false, false, ApprovalMode::Bypass),
|
||||
(false, false, ApprovalMode::Auto),
|
||||
(false, false, ApprovalMode::Never),
|
||||
] {
|
||||
@@ -654,8 +652,29 @@ mod tests {
|
||||
);
|
||||
}
|
||||
}
|
||||
// No registry: the session bit alone decides.
|
||||
assert!(prepare_tool_call(consent, allow_safari.clone(), None, true).is_err());
|
||||
// Full Access shows the card instead of refusing it. The call still
|
||||
// requires approval and is never auto-approved, so a posture cannot
|
||||
// stand in for the person's answer.
|
||||
for (session_auto, context_auto, mode) in [
|
||||
(true, false, ApprovalMode::Suggest),
|
||||
(false, true, ApprovalMode::Suggest),
|
||||
(false, false, ApprovalMode::Bypass),
|
||||
] {
|
||||
let root = tempdir().expect("tempdir");
|
||||
let mut context = ToolContext::new(root.path().to_path_buf());
|
||||
context.auto_approve = context_auto;
|
||||
context.approval_mode = mode;
|
||||
let registry = ToolRegistry::new(context);
|
||||
let prepared = prepare_tool_call(consent, allow_safari.clone(), Some(®istry), session_auto)
|
||||
.expect("Full Access reaches the consent card");
|
||||
assert_eq!(prepared.call.approval, ApprovalRequirement::Required);
|
||||
assert!(!prepared.auto_approve, "{mode:?}");
|
||||
assert!(super::super::turn_loop::registered_tool_approval_required(
|
||||
&prepared.call.name,
|
||||
prepared.call.approval,
|
||||
prepared.auto_approve,
|
||||
));
|
||||
}
|
||||
|
||||
// Ask posture: a Required card that names the app, bundle and scope.
|
||||
let root = tempdir().expect("tempdir");
|
||||
|
||||
@@ -2469,19 +2469,25 @@ impl Engine {
|
||||
|
||||
// Consent is an exact human decision, never a standing grant or
|
||||
// autonomous approval. Keep existing hard blocks authoritative.
|
||||
// Ask and Full Access both have a person at the card; Auto-Review
|
||||
// and Never do not, and keep failing closed.
|
||||
if blocked_error.is_none()
|
||||
&& crate::tools::approval_cache::computer_use_user_gate(&tool_name, &tool_input)
|
||||
.is_some()
|
||||
{
|
||||
if batch_approval_mode == ApprovalMode::Suggest {
|
||||
if matches!(
|
||||
batch_approval_mode,
|
||||
ApprovalMode::Suggest | ApprovalMode::Bypass
|
||||
) {
|
||||
approval_required = true;
|
||||
approval_force_prompt = true;
|
||||
} else {
|
||||
approval_required = false;
|
||||
approval_force_prompt = false;
|
||||
blocked_error = Some(ToolError::permission_denied(
|
||||
"Computer Use consent, scripting and registration require the user's exact approval in Ask posture.".to_string()
|
||||
));
|
||||
blocked_error = Some(ToolError::permission_denied(format!(
|
||||
"Computer Use consent, scripting and registration need the user's own answer, and the current {} posture has no one to ask. Switch to Ask or Full Access to review it.",
|
||||
batch_approval_mode.permission_chip_label()
|
||||
)));
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -408,6 +408,22 @@ pub(crate) fn computer_use_user_gate(
|
||||
})
|
||||
}
|
||||
|
||||
/// Whether this call exists only to carry a person's own answer: Computer Use
|
||||
/// consent, a consent revoke, or an app script. A posture may supply an
|
||||
/// allowance, but it can never supply an answer.
|
||||
#[must_use]
|
||||
pub(crate) fn computer_use_human_decision_tool(tool_name: &str) -> bool {
|
||||
tool_name.starts_with("mcp_")
|
||||
&& [
|
||||
"_consent",
|
||||
"_consent_allow",
|
||||
"_consent_revoke",
|
||||
"_app_script",
|
||||
]
|
||||
.iter()
|
||||
.any(|suffix| tool_name.ends_with(suffix))
|
||||
}
|
||||
|
||||
/// The inner tool of the first `run_actions` step that would need a human
|
||||
/// decision (K1 / K2). Computer Use `run_actions` accepts any plugin tool name
|
||||
/// as a step — including the unlisted `consent_allow` / `consent_revoke` and
|
||||
|
||||
@@ -151,14 +151,25 @@ pub(super) fn resolve_ui_approval_disposition(
|
||||
approval_key: &str,
|
||||
approval_force_prompt: bool,
|
||||
) -> crate::core::authority::ApprovalRequestDisposition {
|
||||
crate::core::authority::resolve_approval_request_disposition(
|
||||
let disposition = crate::core::authority::resolve_approval_request_disposition(
|
||||
&app_turn_authority_for_approvals(app),
|
||||
is_session_approved_for_tool(app, tool_name, grouping_key),
|
||||
is_session_denied_for_key(app, approval_key),
|
||||
approval_force_prompt,
|
||||
// This namespace is minted by the Engine, not by plugin card text.
|
||||
approval_key.starts_with("extcall:ext:"),
|
||||
)
|
||||
);
|
||||
// Full Access holds a required prompt only because a programme with no
|
||||
// person at the card must not be stranded by one. This surface always has
|
||||
// that person, and a Computer Use consent or script exists for no other
|
||||
// reason than to carry their answer, so ask instead of denying.
|
||||
if disposition
|
||||
== crate::core::authority::ApprovalRequestDisposition::AutoDenyFullAccessPolicyHold
|
||||
&& crate::tools::approval_cache::computer_use_human_decision_tool(tool_name)
|
||||
{
|
||||
return crate::core::authority::ApprovalRequestDisposition::Prompt;
|
||||
}
|
||||
disposition
|
||||
}
|
||||
|
||||
/// Answer, explicitly, a request that must not open a card here, so nothing
|
||||
|
||||
Reference in new issue
Block a user