diff --git a/crates/goose/src/agents/extension_manager.rs b/crates/goose/src/agents/extension_manager.rs index 93cfa8c04..67b65a305 100644 --- a/crates/goose/src/agents/extension_manager.rs +++ b/crates/goose/src/agents/extension_manager.rs @@ -172,6 +172,7 @@ pub struct ExtensionManagerCapabilities { #[serde(rename_all = "camelCase")] pub struct GooseMcpAppToolAttachment { pub tool_name: String, + pub tool_name_is_actual: bool, pub extension_name: String, pub resource_uri: String, #[serde(skip_serializing_if = "Option::is_none")] @@ -396,7 +397,6 @@ pub fn is_hidden_extension(name: &str) -> bool { /// Result of resolving a tool call to its owning extension struct ResolvedTool { - tool_name: String, extension_name: String, actual_tool_name: String, client: McpClientBox, @@ -1385,7 +1385,8 @@ impl ExtensionManager { let resource_uri = resolved_tool.resource_uri.clone()?; let mut attachment = GooseMcpAppToolAttachment { - tool_name: resolved_tool.tool_name.clone(), + tool_name: resolved_tool.actual_tool_name.clone(), + tool_name_is_actual: true, extension_name: resolved_tool.extension_name.clone(), resource_uri: resource_uri.clone(), tool_meta: resolved_tool.tool_meta.clone(), @@ -1763,7 +1764,6 @@ impl ExtensionManager { })?; return Ok(ResolvedTool { - tool_name: tool.name.to_string(), extension_name: owner, actual_tool_name, client, @@ -1776,7 +1776,6 @@ impl ExtensionManager { let owner = name_to_key(prefix); if let Some(client) = self.get_server_client(&owner).await { return Ok(ResolvedTool { - tool_name: name.to_string(), extension_name: owner, actual_tool_name: actual.to_string(), client, @@ -3005,7 +3004,8 @@ mod tests { .resolve_tool("test-session-id", "test_client.tool") .await .expect("mangled dotted name should resolve to the real tool"); - assert_eq!(resolved.tool_name, "test_client__tool"); + assert_eq!(resolved.extension_name, "test_client"); + assert_eq!(resolved.actual_tool_name, "tool"); } #[tokio::test] @@ -3021,7 +3021,8 @@ mod tests { .resolve_tool("test-session-id", "functions.test_client__tool") .await .expect("functions-prefixed name should resolve to the real tool"); - assert_eq!(resolved.tool_name, "test_client__tool"); + assert_eq!(resolved.extension_name, "test_client"); + assert_eq!(resolved.actual_tool_name, "tool"); } #[tokio::test] @@ -3037,7 +3038,7 @@ mod tests { .resolve_tool("test-session-id", "dotted__db.query") .await .expect("exact dotted tool name must resolve"); - assert_eq!(resolved.tool_name, "dotted__db.query"); + assert_eq!(resolved.extension_name, "dotted"); assert_eq!(resolved.actual_tool_name, "db.query"); } @@ -3054,7 +3055,7 @@ mod tests { .resolve_tool("test-session-id", "dotted.db.query") .await .expect("mangled extension separator should resolve"); - assert_eq!(resolved.tool_name, "dotted__db.query"); + assert_eq!(resolved.extension_name, "dotted"); assert_eq!(resolved.actual_tool_name, "db.query"); } @@ -3129,7 +3130,8 @@ mod tests { fn test_insert_trusted_tool_update_meta_stores_backend_payload() { let mut result = CallToolResult::success(vec![]); let attachment = GooseMcpAppToolAttachment { - tool_name: "weather__render".to_string(), + tool_name: "render__secret".to_string(), + tool_name_is_actual: true, extension_name: "weather".to_string(), resource_uri: "ui://weather/app".to_string(), tool_meta: None, @@ -3152,7 +3154,8 @@ mod tests { meta.0.get(TRUSTED_TOOL_UPDATE_META_KEY), Some(&serde_json::json!({ "mcpApp": { - "toolName": "weather__render", + "toolName": "render__secret", + "toolNameIsActual": true, "extensionName": "weather", "resourceUri": "ui://weather/app", "resourceResult": { diff --git a/ui/desktop/src/acp/__tests__/sessionNotificationAdapter.test.ts b/ui/desktop/src/acp/__tests__/sessionNotificationAdapter.test.ts index 55f221ba0..aef953b5f 100644 --- a/ui/desktop/src/acp/__tests__/sessionNotificationAdapter.test.ts +++ b/ui/desktop/src/acp/__tests__/sessionNotificationAdapter.test.ts @@ -456,6 +456,7 @@ describe('createAcpSessionNotificationAdapter', () => { resourceUri: 'ui://app/resource', extensionName: 'developer', toolName: 'read_file', + toolNameIsActual: true, }, }, }, @@ -477,6 +478,7 @@ describe('createAcpSessionNotificationAdapter', () => { ui: { resourceUri: 'ui://app/resource' }, extensionName: 'developer', toolName: 'read_file', + toolNameIsActual: true, }, }, }, diff --git a/ui/desktop/src/acp/adapter/tools.ts b/ui/desktop/src/acp/adapter/tools.ts index c0bcb2473..021f5519d 100644 --- a/ui/desktop/src/acp/adapter/tools.ts +++ b/ui/desktop/src/acp/adapter/tools.ts @@ -337,6 +337,7 @@ interface DesktopMcpAppMeta extends Record { }; extensionName?: string; toolName?: string; + toolNameIsActual?: boolean; } type ToolResultValue = { @@ -368,5 +369,9 @@ function mcpAppMetadata(update: ToolCallUpdate): DesktopMcpAppMeta | undefined { extensionName: typeof goose.mcpApp.extensionName === 'string' ? goose.mcpApp.extensionName : undefined, toolName: typeof goose.mcpApp.toolName === 'string' ? goose.mcpApp.toolName : undefined, + toolNameIsActual: + typeof goose.mcpApp.toolNameIsActual === 'boolean' + ? goose.mcpApp.toolNameIsActual + : undefined, }; } diff --git a/ui/desktop/src/components/ToolCallWithResponse.security.test.ts b/ui/desktop/src/components/ToolCallWithResponse.security.test.ts new file mode 100644 index 000000000..6759d39dd --- /dev/null +++ b/ui/desktop/src/components/ToolCallWithResponse.security.test.ts @@ -0,0 +1,60 @@ +import { describe, expect, it } from 'vitest'; +import { resolveMcpAppMetadata } from './ToolCallWithResponse'; + +describe('MCP app metadata binding', () => { + it('preserves authoritative ownership when a tool name contains the delimiter', () => { + const metadata = resolveMcpAppMetadata({ + ui: { resourceUri: 'ui://victim/render' }, + extensionName: 'victim', + toolName: 'victim__render', + toolNameIsActual: true, + }); + + expect(metadata).toEqual({ + resourceUri: 'ui://victim/render', + extensionName: 'victim', + toolName: 'victim__render', + }); + }); + + it('normalizes the exact owner prefix from trusted legacy replay metadata', () => { + const metadata = resolveMcpAppMetadata({ + ui: { resourceUri: 'ui://victim/render' }, + extensionName: 'victim', + toolName: 'victim__render__secret', + }); + + expect(metadata).toEqual({ + resourceUri: 'ui://victim/render', + extensionName: 'victim', + toolName: 'render__secret', + }); + }); + + it('does not infer ownership from incomplete metadata', () => { + const metadata = resolveMcpAppMetadata({ + ui: { resourceUri: 'ui://victim/render' }, + }); + + expect(metadata).toBeNull(); + }); + + it('rejects untrusted request metadata when authenticated response metadata is absent', () => { + expect(resolveMcpAppMetadata(undefined)).toBeNull(); + }); + + it('uses complete authenticated response metadata', () => { + const metadata = resolveMcpAppMetadata({ + ui: { resourceUri: 'ui://victim/render' }, + extensionName: 'victim', + toolName: 'render__secret', + toolNameIsActual: true, + }); + + expect(metadata).toEqual({ + resourceUri: 'ui://victim/render', + extensionName: 'victim', + toolName: 'render__secret', + }); + }); +}); diff --git a/ui/desktop/src/components/ToolCallWithResponse.tsx b/ui/desktop/src/components/ToolCallWithResponse.tsx index f47e4d9f5..7c4f23650 100644 --- a/ui/desktop/src/components/ToolCallWithResponse.tsx +++ b/ui/desktop/src/components/ToolCallWithResponse.tsx @@ -69,6 +69,9 @@ type UiMeta = { ui?: { resourceUri?: string; }; + extensionName?: string; + toolName?: string; + toolNameIsActual?: boolean; subagent_session_id?: string; }; @@ -155,6 +158,27 @@ interface McpAppWrapperProps { append?: (value: string) => void; } +export function resolveMcpAppMetadata( + responseMeta: UiMeta | undefined +): { resourceUri: string; extensionName: string; toolName: string } | null { + const resourceUri = responseMeta?.ui?.resourceUri; + const extensionName = responseMeta?.extensionName; + const toolName = responseMeta?.toolName; + if (resourceUri && extensionName && toolName) { + const legacyPrefix = `${extensionName}__`; + const actualToolName = responseMeta.toolNameIsActual + ? toolName + : toolName.startsWith(legacyPrefix) + ? toolName.slice(legacyPrefix.length) + : toolName; + if (actualToolName) { + return { resourceUri, extensionName, toolName: actualToolName }; + } + } + + return null; +} + function McpAppWrapper({ toolRequest, toolResponse, @@ -162,25 +186,12 @@ function McpAppWrapper({ append, }: McpAppWrapperProps): React.ReactNode { const requestWithMeta = toolRequest as ToolRequestWithMeta; - let resourceUri = requestWithMeta._meta?.ui?.resourceUri; - - if (!resourceUri && toolResponse) { - const resultWithMeta = toolResponse.toolResult as ToolResultWithMeta; - if (resultWithMeta?.status === 'success' && resultWithMeta.value) { - resourceUri = resultWithMeta.value._meta?.ui?.resourceUri; - } - } - - // Tool names are formatted as "{extension_name}__{tool_name}". - // Extension names can contain underscores (special chars like parentheses are normalized to "_"), - // so we must use lastIndexOf to find the delimiter. - // e.g., "my_server(local)" -> "my_server_local_" -> "my_server_local___get_time" - const toolCallName = - requestWithMeta.toolCall.status === 'success' ? requestWithMeta.toolCall.value.name : ''; - const delimiterIndex = toolCallName.lastIndexOf('__'); - const extensionName = delimiterIndex === -1 ? '' : toolCallName.substring(0, delimiterIndex); - const toolName = - delimiterIndex === -1 ? toolCallName : toolCallName.substring(delimiterIndex + 2); + const resultWithMeta = toolResponse?.toolResult as ToolResultWithMeta | undefined; + const responseMeta = + resultWithMeta?.status === 'success' && resultWithMeta.value + ? resultWithMeta.value._meta + : undefined; + const appMetadata = resolveMcpAppMetadata(responseMeta); const toolArguments = requestWithMeta.toolCall.status === 'success' @@ -189,15 +200,16 @@ function McpAppWrapper({ const toolInput = { arguments: toolArguments || {} }; - const resultWithMeta = toolResponse?.toolResult as ToolResultWithMeta | undefined; const toolResult = resultWithMeta?.status === 'success' && resultWithMeta.value ? (resultWithMeta.value as unknown as CallToolResult) : undefined; - if (!resourceUri) return null; + if (!appMetadata) return null; if (requestWithMeta.toolCall.status !== 'success') return null; + const { resourceUri, extensionName, toolName } = appMetadata; + return (