Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Added

- Preserved standard MCP tool metadata and call results end to end, including
output schemas, annotations, icons, `_meta`, `structuredContent`, decoded
images, embedded resources, and bounded content-addressed artifacts.

### Security

- Made MCP confirmation annotations escalation-only: tool metadata can require
HITL but cannot weaken a host Allow/Ask/Deny decision.
- Allowed an explicitly scoped delegated worker to see a parent-hidden tool
while keeping both parent and worker execution policies authoritative.

## [5.3.5] - 2026-07-17

### Added
Expand Down
11 changes: 10 additions & 1 deletion core/src/agent/tool_invoker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@ impl ScopedToolInvoker {
tool_name: &invocation.name,
args: &invocation.args,
pre_tool_block,
tool_requires_confirmation: self
.agent
.tool_executor
.registry()
.requires_confirmation(&invocation.name, &invocation.args),
})
.await
}
Expand Down Expand Up @@ -395,7 +400,11 @@ impl ToolInvoker for ScopedToolInvoker {
}

fn available_tools(&self) -> Vec<String> {
self.agent.tool_executor.registry().list()
let mut tools = self.agent.tool_executor.registry().list();
if let Some(permission_checker) = &self.agent.config.permission_checker {
tools.retain(|tool| permission_checker.expose_to_model(tool));
}
tools
}

fn capabilities(&self, name: &str, args: &serde_json::Value) -> Option<ToolCapabilities> {
Expand Down
150 changes: 145 additions & 5 deletions core/src/child_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
use crate::agent::AgentConfig;
use crate::hitl::ConfirmationProvider;
use crate::hooks::HookExecutor;
use crate::permissions::{PermissionChecker, PermissionPolicy};
use crate::permissions::{PermissionChecker, PermissionDecision, PermissionPolicy};
use crate::security::SecurityProvider;
use crate::skills::SkillRegistry;
use std::sync::Arc;
Expand Down Expand Up @@ -60,6 +60,56 @@ pub struct ChildRunContext {
pub budget_guard: Option<Arc<dyn crate::budget::BudgetGuard>>,
}

struct DelegatedPermissionChecker {
child: Arc<dyn PermissionChecker>,
child_policy: Option<PermissionPolicy>,
parent: Arc<dyn PermissionChecker>,
parent_policy: Option<PermissionPolicy>,
}

impl PermissionChecker for DelegatedPermissionChecker {
fn expose_to_model(&self, tool_name: &str) -> bool {
if !self.child.expose_to_model(tool_name) {
return false;
}

if self
.child_policy
.as_ref()
.is_some_and(|policy| policy.declares_tool_access(tool_name))
{
// A worker's explicitly declared capability may cross a host's
// ordinary parent-only visibility filter. A serializable parent
// deny remains authoritative and keeps the tool hidden.
return self
.parent_policy
.as_ref()
.map(|policy| policy.expose_to_model(tool_name))
.unwrap_or_else(|| self.parent.expose_to_model(tool_name));
}

self.parent.expose_to_model(tool_name)
}

fn check(&self, tool_name: &str, args: &serde_json::Value) -> PermissionDecision {
stricter_decision(
self.child.check(tool_name, args),
self.parent.check(tool_name, args),
)
}
}

const fn stricter_decision(
left: PermissionDecision,
right: PermissionDecision,
) -> PermissionDecision {
match (left, right) {
(PermissionDecision::Deny, _) | (_, PermissionDecision::Deny) => PermissionDecision::Deny,
(PermissionDecision::Ask, _) | (_, PermissionDecision::Ask) => PermissionDecision::Ask,
(PermissionDecision::Allow, PermissionDecision::Allow) => PermissionDecision::Allow,
}
}

impl ChildRunContext {
/// Apply inherited capabilities to a child AgentConfig.
///
Expand All @@ -75,10 +125,26 @@ impl ChildRunContext {
if config.skill_registry.is_none() {
config.skill_registry = self.skill_registry.clone();
}
if config.permission_checker.is_none() {
config.permission_checker = self.permission_checker.clone();
config.permission_policy = self.permission_policy.clone();
} else if config.permission_policy.is_none() {
match (
config.permission_checker.take(),
self.permission_checker.clone(),
) {
(Some(child), Some(parent)) => {
config.permission_checker = Some(Arc::new(DelegatedPermissionChecker {
child,
child_policy: config.permission_policy.clone(),
parent,
parent_policy: self.permission_policy.clone(),
}));
}
(Some(child), None) => config.permission_checker = Some(child),
(None, Some(parent)) => {
config.permission_checker = Some(parent);
config.permission_policy = self.permission_policy.clone();
}
(None, None) => {}
}
if config.permission_policy.is_none() {
config.permission_policy = self.permission_policy.clone();
}
if config.tool_timeout_ms.is_none() {
Expand Down Expand Up @@ -110,3 +176,77 @@ impl ChildRunContext {
}
}
}

#[cfg(test)]
mod tests {
use super::*;

#[derive(Clone)]
struct ParentVisibility {
policy: PermissionPolicy,
hide_use_from_primary: bool,
}

impl PermissionChecker for ParentVisibility {
fn expose_to_model(&self, tool_name: &str) -> bool {
!(self.hide_use_from_primary && tool_name.starts_with("mcp__use_"))
&& self.policy.expose_to_model(tool_name)
}

fn check(&self, tool_name: &str, args: &serde_json::Value) -> PermissionDecision {
self.policy.check(tool_name, args)
}
}

fn delegated(
child_policy: PermissionPolicy,
parent_policy: PermissionPolicy,
) -> DelegatedPermissionChecker {
DelegatedPermissionChecker {
child: Arc::new(child_policy.clone()),
child_policy: Some(child_policy),
parent: Arc::new(ParentVisibility {
policy: parent_policy.clone(),
hide_use_from_primary: true,
}),
parent_policy: Some(parent_policy),
}
}

#[test]
fn explicitly_scoped_worker_can_see_parent_hidden_tool() {
let mut child = PermissionPolicy::new().allow("mcp__use_*");
child.default_decision = PermissionDecision::Deny;
let parent = PermissionPolicy::new().allow("mcp__use_*");
let checker = delegated(child, parent);

assert!(checker.expose_to_model("mcp__use_browser__browser_snapshot"));
assert_eq!(
checker.check("mcp__use_browser__browser_snapshot", &serde_json::json!({})),
PermissionDecision::Allow
);
}

#[test]
fn unrelated_worker_does_not_inherit_parent_hidden_use_tools() {
let child = PermissionPolicy::new().allow("read(*)");
let parent = PermissionPolicy::new().allow("mcp__use_*");
let checker = delegated(child, parent);

assert!(!checker.expose_to_model("mcp__use_browser__browser_snapshot"));
}

#[test]
fn parent_deny_remains_authoritative_for_explicit_worker_capability() {
let mut child = PermissionPolicy::new().allow("mcp__use_*");
child.default_decision = PermissionDecision::Deny;
let parent = PermissionPolicy::new().deny("mcp__use_ocr__ocr_extract");
let checker = delegated(child, parent);

assert!(!checker.expose_to_model("mcp__use_ocr__ocr_extract"));
assert_eq!(
checker.check("mcp__use_ocr__ocr_extract", &serde_json::json!({})),
PermissionDecision::Deny
);
}
}
31 changes: 3 additions & 28 deletions core/src/mcp/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
use crate::mcp::client::McpClient;
use crate::mcp::oauth;
use crate::mcp::protocol::{
CallToolResult, McpServerConfig, McpTool, McpTransportConfig, OAuthConfig, ToolContent,
CallToolResult, McpServerConfig, McpTool, McpTransportConfig, OAuthConfig,
};
use crate::mcp::transport::http_sse::HttpSseTransport;
use crate::mcp::transport::stdio::StdioTransport;
Expand All @@ -16,6 +16,8 @@ use std::collections::HashMap;
use std::sync::Arc;
use tokio::sync::RwLock;

pub use crate::mcp::result::tool_result_to_string;

/// MCP server status
#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)]
pub struct McpServerStatus {
Expand Down Expand Up @@ -489,33 +491,6 @@ fn now_epoch_ms() -> u64 {
.unwrap_or(0)
}

/// Convert MCP tool result to string output
pub fn tool_result_to_string(result: &CallToolResult) -> String {
let mut output = String::new();

for content in &result.content {
match content {
ToolContent::Text { text } => {
output.push_str(text);
output.push('\n');
}
ToolContent::Image { data: _, mime_type } => {
output.push_str(&format!("[Image: {}]\n", mime_type));
}
ToolContent::Resource { resource } => {
if let Some(text) = &resource.text {
output.push_str(text);
output.push('\n');
} else {
output.push_str(&format!("[Resource: {}]\n", resource.uri));
}
}
}
}

output.trim_end().to_string()
}

#[cfg(test)]
#[path = "manager/tests.rs"]
mod tests;
8 changes: 8 additions & 0 deletions core/src/mcp/manager/tests.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use super::*;
use crate::mcp::protocol::ToolContent;

#[test]
fn test_parse_tool_name() {
Expand Down Expand Up @@ -32,6 +33,7 @@ fn test_tool_result_to_string() {
},
],
is_error: false,
..CallToolResult::default()
};

let output = tool_result_to_string(&result);
Expand Down Expand Up @@ -136,6 +138,7 @@ fn test_tool_result_to_string_single_text() {
text: "Hello World".to_string(),
}],
is_error: false,
..CallToolResult::default()
};
let output = tool_result_to_string(&result);
assert_eq!(output, "Hello World");
Expand All @@ -153,6 +156,7 @@ fn test_tool_result_to_string_multiple_text() {
},
],
is_error: false,
..CallToolResult::default()
};
let output = tool_result_to_string(&result);
assert!(output.contains("First line"));
Expand All @@ -164,6 +168,7 @@ fn test_tool_result_to_string_empty() {
let result = CallToolResult {
content: vec![],
is_error: false,
..CallToolResult::default()
};
let output = tool_result_to_string(&result);
assert_eq!(output, "");
Expand All @@ -177,6 +182,7 @@ fn test_tool_result_to_string_image() {
mime_type: "image/png".to_string(),
}],
is_error: false,
..CallToolResult::default()
};
let output = tool_result_to_string(&result);
assert!(output.contains("[Image: image/png]"));
Expand All @@ -195,6 +201,7 @@ fn test_tool_result_to_string_resource() {
},
}],
is_error: false,
..CallToolResult::default()
};
let output = tool_result_to_string(&result);
assert!(output.contains("Resource content"));
Expand Down Expand Up @@ -222,6 +229,7 @@ fn test_tool_result_to_string_mixed_content() {
},
],
is_error: false,
..CallToolResult::default()
};
let output = tool_result_to_string(&result);
assert!(output.contains("Text content"));
Expand Down
8 changes: 5 additions & 3 deletions core/src/mcp/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,15 @@ pub mod client;
pub mod manager;
pub mod oauth;
pub mod protocol;
mod result;
pub mod tools;
pub mod transport;

pub use client::McpClient;
pub use manager::{tool_result_to_string, McpManager, McpServerStatus};
pub use manager::{McpManager, McpServerStatus};
pub use protocol::{
CallToolResult, McpNotification, McpResource, McpServerConfig, McpTool, McpTransportConfig,
OAuthConfig, ServerCapabilities, ToolContent,
CallToolResult, McpNotification, McpResource, McpServerConfig, McpTool, McpToolAnnotations,
McpTransportConfig, OAuthConfig, ServerCapabilities, ToolContent,
};
pub use result::tool_result_to_string;
pub use tools::{create_mcp_tools, McpToolWrapper};
Loading
Loading