fix: address chat MCP UI widgets PR review (#2046)

dimetron · dimetron · commit 4bc1353bb84c · 2026-06-18T19:58:38.000+02:00
- Use the typed MCP CallToolResult (modelcontextprotocol/go-sdk) instead of
  hand-built map[string]any when compacting MCP App tool results for the model
  (peterj review).
- Remove the accidentally committed go/build.err build-log artifact.
- ChatMinimap: add a type-only React import so React.RefObject/React.PointerEvent
  resolve (fixes "Cannot find namespace 'React'").
- sandbox_proxy.html: restrict postMessage to the expected parent origin (passed
  via ?parentOrigin from McpAppRenderer) instead of accepting/posting to "*",
  preventing arbitrary HTML/script injection from other origins.

Signed-off-by: Dmytro Rashko &lt;dmitriy.rashko@amdocs.com&gt;
diff --git a/go/adk/pkg/agent/mcp_apps.go b/go/adk/pkg/agent/mcp_apps.go
@@ -1,6 +1,9 @@
 package agent
 
 import (
+	"encoding/json"
+
+	mcpsdk "github.com/modelcontextprotocol/go-sdk/mcp"
 	"google.golang.org/adk/agent"
 	"google.golang.org/adk/agent/llmagent"
 	adkmodel "google.golang.org/adk/model"
@@ -39,34 +42,61 @@ func MakeMCPAppModelResultCallback(appToolNames map[string]bool) llmagent.Before
 	}
 }
 
+// compactMCPAppModelResponse rewrites an MCP App tool result for the model.
+//
+// The model exchanges tool results as a generic map (genai
+// FunctionResponse.Response), but the payload is really an MCP
+// [mcpsdk.CallToolResult]. We decode it into that typed result so the logic
+// works against real fields (IsError, Content, Meta, StructuredContent) rather
+// than poking at string keys. If the payload isn't a recognizable MCP result we
+// leave it untouched.
 func compactMCPAppModelResponse(response map[string]any) map[string]any {
-	// On error, keep the original content so the model can diagnose/recover.
-	if isErr, ok := response["isError"].(bool); ok && isErr {
-		out := map[string]any{"isError": true}
-		if content, ok := response["content"]; ok {
-			out["content"] = content
-		}
-		if meta, ok := response["_meta"]; ok {
-			out["_meta"] = meta
-		}
-		return out
+	result, err := decodeCallToolResult(response)
+	if err != nil {
+		return response
+	}
+
+	if result.IsError {
+		// On error, keep the original content/meta so the model can
+		// diagnose and recover; only drop the heavy structured payload.
+		result.StructuredContent = nil
+		return encodeCallToolResult(result, response)
 	}
 
 	// On success, collapse the render payload into a terminal directive so the
-	// model stops re-invoking the rendering tool.
-	out := map[string]any{
-		"content": []any{
-			map[string]any{
-				"type": "text",
-				"text": mcpAppRenderedNotice,
-			},
-		},
+	// model stops re-invoking the rendering tool. Preserve _meta (e.g.
+	// resourceUri) in case downstream tooling relies on it.
+	compact := &mcpsdk.CallToolResult{
+		Meta:    result.Meta,
+		Content: []mcpsdk.Content{&mcpsdk.TextContent{Text: mcpAppRenderedNotice}},
 	}
+	return encodeCallToolResult(compact, response)
+}
 
-	// Preserve _meta (e.g. resourceUri) in case downstream tooling relies on it.
-	if meta, ok := response["_meta"]; ok {
-		out["_meta"] = meta
+// decodeCallToolResult interprets a generic model-facing response map as a typed
+// MCP CallToolResult.
+func decodeCallToolResult(response map[string]any) (*mcpsdk.CallToolResult, error) {
+	raw, err := json.Marshal(response)
+	if err != nil {
+		return nil, err
+	}
+	var result mcpsdk.CallToolResult
+	if err := json.Unmarshal(raw, &result); err != nil {
+		return nil, err
 	}
+	return &result, nil
+}
 
+// encodeCallToolResult converts a typed CallToolResult back into the generic map
+// the model expects, falling back to the original response if conversion fails.
+func encodeCallToolResult(result *mcpsdk.CallToolResult, fallback map[string]any) map[string]any {
+	raw, err := json.Marshal(result)
+	if err != nil {
+		return fallback
+	}
+	var out map[string]any
+	if err := json.Unmarshal(raw, &out); err != nil {
+		return fallback
+	}
 	return out
 }
diff --git a/go/build.err b/go/build.err
diff --git a/ui/public/sandbox_proxy.html b/ui/public/sandbox_proxy.html
@@ -15,7 +15,17 @@
 </head>
 <body>
   <script>
+    // The host page passes its origin via ?parentOrigin=...; fall back to this
+    // document's own origin (the proxy is served from the host's origin). We
+    // never trust "*" — only messages from (and posts to) the expected parent.
+    var params = new URLSearchParams(window.location.search);
+    var parentOrigin = params.get("parentOrigin") || window.location.origin;
+
     window.addEventListener("message", (event) => {
+      // Reject messages from any origin other than the expected parent so a
+      // page embedding this iframe cannot inject arbitrary HTML/script.
+      if (event.origin !== parentOrigin) return;
+
       const data = event.data;
       if (!data || typeof data !== "object") return;
 
@@ -32,7 +42,7 @@
     window.parent.postMessage({
       method: "ui/notifications/sandbox-proxy-ready",
       params: {}
-    }, "*");
+    }, parentOrigin);
   </script>
 </body>
 </html>
diff --git a/ui/src/components/chat/ChatMinimap.tsx b/ui/src/components/chat/ChatMinimap.tsx
@@ -1,5 +1,6 @@
 "use client";
 
+import type React from "react";
 import { useCallback, useEffect, useRef, useState } from "react";
 import { cn } from "@/lib/utils";
 
diff --git a/ui/src/components/mcp-apps/McpAppRenderer.tsx b/ui/src/components/mcp-apps/McpAppRenderer.tsx
@@ -88,7 +88,11 @@ export function McpAppRenderer({
 
   useEffect(() => {
     const id = requestAnimationFrame(() => {
-      setSandboxUrl(new URL("/sandbox_proxy.html", window.location.origin));
+      // Tell the sandbox proxy which origin to trust for postMessage, so it can
+      // reject messages from any other origin instead of accepting "*".
+      const url = new URL("/sandbox_proxy.html", window.location.origin);
+      url.searchParams.set("parentOrigin", window.location.origin);
+      setSandboxUrl(url);
     });
     return () => cancelAnimationFrame(id);
   }, []);
