feat: use the same permission flow for enable extensions (#2302)
This commit is contained in:
@@ -6,14 +6,11 @@ use crate::providers::base::Provider;
|
||||
use chrono::Utc;
|
||||
use indoc::indoc;
|
||||
use mcp_core::tool::ToolAnnotations;
|
||||
use mcp_core::ToolCall;
|
||||
use mcp_core::{tool::Tool, TextContent};
|
||||
use serde_json::{json, Value};
|
||||
use std::collections::HashSet;
|
||||
use std::sync::Arc;
|
||||
|
||||
use super::permission_confirmation::PrincipalType;
|
||||
|
||||
/// Creates the tool definition for checking read-only permissions.
|
||||
fn create_read_only_tool() -> Tool {
|
||||
Tool::new(
|
||||
@@ -154,36 +151,6 @@ pub async fn detect_read_only_tools(
|
||||
}
|
||||
}
|
||||
|
||||
/// Gets the boolean value whether the message is enable extension related and
|
||||
/// the cconfirmation message based on the tool call
|
||||
pub fn get_confirmation_message(request_id: &str, tool_call: ToolCall) -> (PrincipalType, Message) {
|
||||
if tool_call.name == PLATFORM_ENABLE_EXTENSION_TOOL_NAME {
|
||||
(
|
||||
PrincipalType::Extension,
|
||||
Message::user().with_extension_request(
|
||||
request_id,
|
||||
tool_call
|
||||
.arguments
|
||||
.get("extension_name")
|
||||
.and_then(|v| v.as_str())
|
||||
.unwrap_or("")
|
||||
.to_string(),
|
||||
tool_call.name.clone(),
|
||||
),
|
||||
)
|
||||
} else {
|
||||
(
|
||||
PrincipalType::Tool,
|
||||
Message::user().with_tool_confirmation_request(
|
||||
request_id,
|
||||
tool_call.name.clone(),
|
||||
tool_call.arguments.clone(),
|
||||
Some("Goose would like to call the above tool. Allow? (y/n):".to_string()),
|
||||
),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// Define return structure
|
||||
pub struct PermissionCheckResult {
|
||||
pub approved: Vec<ToolRequest>,
|
||||
@@ -198,26 +165,24 @@ pub async fn check_tool_permissions(
|
||||
tools_without_annotation: HashSet<String>,
|
||||
permission_manager: &mut PermissionManager,
|
||||
provider: Arc<dyn Provider>,
|
||||
) -> PermissionCheckResult {
|
||||
) -> (PermissionCheckResult, Vec<String>) {
|
||||
let mut approved = vec![];
|
||||
let mut needs_approval = vec![];
|
||||
let mut denied = vec![];
|
||||
let mut llm_detect_candidates = vec![];
|
||||
let mut enable_extension_request_ids = vec![];
|
||||
|
||||
for request in candidate_requests {
|
||||
if let Ok(tool_call) = request.tool_call.clone() {
|
||||
// Always ask approval for enable extension tool.
|
||||
if tool_call.name == PLATFORM_ENABLE_EXTENSION_TOOL_NAME {
|
||||
// Insert at the front of the list so that enable extension can be run before other tools.
|
||||
needs_approval.insert(0, request.clone());
|
||||
continue;
|
||||
}
|
||||
|
||||
if mode == "chat" {
|
||||
continue;
|
||||
} else if mode == "auto" {
|
||||
approved.push(request.clone());
|
||||
} else {
|
||||
if tool_call.name == PLATFORM_ENABLE_EXTENSION_TOOL_NAME {
|
||||
enable_extension_request_ids.push(request.id.clone());
|
||||
}
|
||||
|
||||
// 1. Check user-defined permission
|
||||
if let Some(level) = permission_manager.get_user_permission(&tool_call.name) {
|
||||
match level {
|
||||
@@ -284,11 +249,14 @@ pub async fn check_tool_permissions(
|
||||
}
|
||||
}
|
||||
|
||||
PermissionCheckResult {
|
||||
approved,
|
||||
needs_approval,
|
||||
denied,
|
||||
}
|
||||
(
|
||||
PermissionCheckResult {
|
||||
approved,
|
||||
needs_approval,
|
||||
denied,
|
||||
},
|
||||
enable_extension_request_ids,
|
||||
)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
@@ -471,7 +439,7 @@ mod tests {
|
||||
vec![tool_request_1, tool_request_2, enable_extension];
|
||||
|
||||
// Call the function under test
|
||||
let result = check_tool_permissions(
|
||||
let (result, enable_extension_request_ids) = check_tool_permissions(
|
||||
&candidate_requests,
|
||||
"smart_approve",
|
||||
tools_with_readonly_annotation,
|
||||
@@ -485,21 +453,13 @@ mod tests {
|
||||
assert_eq!(result.approved.len(), 1); // file_reader should be approved
|
||||
assert_eq!(result.needs_approval.len(), 2); // data_fetcher should need approval
|
||||
assert_eq!(result.denied.len(), 0); // No tool should be denied in this test
|
||||
assert_eq!(enable_extension_request_ids.len(), 1);
|
||||
|
||||
// Ensure the right tools are in the approved and needs_approval lists
|
||||
assert!(result.approved.iter().any(|req| req.id == "tool_1"));
|
||||
assert!(result.needs_approval.iter().any(|req| req.id == "tool_2"));
|
||||
|
||||
let tool_0 = result.needs_approval.get(0);
|
||||
assert!(
|
||||
tool_0.is_some(),
|
||||
"Expected at least one tool in needs_approval"
|
||||
);
|
||||
assert_eq!(
|
||||
tool_0.unwrap().id,
|
||||
"tool_3",
|
||||
"PLATFORM_ENABLE_EXTENSION_TOOL_NAME should be the first in needs_approval"
|
||||
);
|
||||
assert!(result.needs_approval.iter().any(|req| req.id == "tool_3"));
|
||||
assert!(enable_extension_request_ids.iter().any(|id| id == "tool_3"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -538,7 +498,7 @@ mod tests {
|
||||
let candidate_requests: Vec<ToolRequest> = vec![tool_request_1, tool_request_2];
|
||||
|
||||
// Call the function under test
|
||||
let result = check_tool_permissions(
|
||||
let (result, _) = check_tool_permissions(
|
||||
&candidate_requests,
|
||||
"auto",
|
||||
tools_with_readonly_annotation,
|
||||
|
||||
Reference in New Issue
Block a user