From d97c50168461aa1bbd5e6bead9c8ae83351d90d3 Mon Sep 17 00:00:00 2001 From: airenostars Date: Tue, 21 Apr 2026 12:26:16 -0700 Subject: [PATCH] fix(canvas): infinite render loop in ContextMenu + dedupe SSRF funcs ContextMenu: useCanvasStore selector returned .filter() (new array on every call), causing React 19's useSyncExternalStore to detect a reference change and re-render infinitely. Fixed by using .some() which returns a stable boolean. Also deduplicates isSafeURL, isPrivateOrMetadataIP, validateRelPath which existed in 3 files after PR merges collided. Canonical location is ssrf.go. Removed unused imports (fmt, net, net/url, database/sql, strings) from a2a_proxy.go, a2a_proxy_helpers.go, mcp_tools.go. Co-Authored-By: Claude Opus 4.6 (1M context) --- canvas/src/components/ContextMenu.tsx | 7 +- .../internal/handlers/a2a_proxy.go | 1 - .../internal/handlers/a2a_proxy_helpers.go | 124 +----------------- .../internal/handlers/mcp_tools.go | 72 +--------- .../internal/handlers/templates.go | 9 +- 5 files changed, 7 insertions(+), 206 deletions(-) diff --git a/canvas/src/components/ContextMenu.tsx b/canvas/src/components/ContextMenu.tsx index 3d869a81a..5e5ff11eb 100644 --- a/canvas/src/components/ContextMenu.tsx +++ b/canvas/src/components/ContextMenu.tsx @@ -23,10 +23,11 @@ export function ContextMenu() { const setPanelTab = useCanvasStore((s) => s.setPanelTab); const nestNode = useCanvasStore((s) => s.nestNode); const contextNodeId = contextMenu?.nodeId ?? null; - const children = useCanvasStore((s) => - contextNodeId ? s.nodes.filter((n) => n.data.parentId === contextNodeId) : [] + // Derive hasChildren with a stable boolean return (not a new array each call) + // to avoid infinite loop from useSyncExternalStore in React 19. + const hasChildren = useCanvasStore((s) => + contextNodeId ? s.nodes.some((n) => n.data.parentId === contextNodeId) : false ); - const hasChildren = children.length > 0; const setPendingDelete = useCanvasStore((s) => s.setPendingDelete); const ref = useRef(null); const [actionLoading, setActionLoading] = useState(false); diff --git a/workspace-server/internal/handlers/a2a_proxy.go b/workspace-server/internal/handlers/a2a_proxy.go index 18991f38b..d17070700 100644 --- a/workspace-server/internal/handlers/a2a_proxy.go +++ b/workspace-server/internal/handlers/a2a_proxy.go @@ -11,7 +11,6 @@ import ( "database/sql" "encoding/json" "errors" - "fmt" "io" "log" "net/http" diff --git a/workspace-server/internal/handlers/a2a_proxy_helpers.go b/workspace-server/internal/handlers/a2a_proxy_helpers.go index 1a87071a6..d1c243e10 100644 --- a/workspace-server/internal/handlers/a2a_proxy_helpers.go +++ b/workspace-server/internal/handlers/a2a_proxy_helpers.go @@ -5,16 +5,11 @@ package handlers import ( "context" - "database/sql" "encoding/json" "errors" - "fmt" "log" - "net" "net/http" - "net/url" "strconv" - "strings" "time" "github.com/Molecule-AI/molecule-monorepo/platform/internal/db" @@ -281,124 +276,7 @@ func parseUsageFromA2AResponse(body []byte) (inputTokens, outputTokens int64) { return 0, 0 } -// isSafeURL validates that a URL resolves to a publicly-routable address, -// preventing A2A requests from being redirected to internal/cloud-metadata -// infrastructure (SSRF, CWE-918). Workspace URLs come from DB/Redis caches -// so we validate before making any outbound HTTP call. -func isSafeURL(rawURL string) error { - u, err := url.Parse(rawURL) - if err != nil { - return fmt.Errorf("invalid URL: %w", err) - } - // Reject non-HTTP(S) schemes. - if u.Scheme != "http" && u.Scheme != "https" { - return fmt.Errorf("forbidden scheme: %s (only http/https allowed)", u.Scheme) - } - host := u.Hostname() - if host == "" { - return fmt.Errorf("empty hostname") - } - // Block direct IP addresses. - if ip := net.ParseIP(host); ip != nil { - if ip.IsLoopback() || ip.IsUnspecified() || ip.IsLinkLocalUnicast() { - return fmt.Errorf("forbidden loopback/unspecified IP: %s", ip) - } - if isPrivateOrMetadataIP(ip) { - return fmt.Errorf("forbidden private/metadata IP: %s", ip) - } - return nil - } - // For hostnames, resolve and validate each returned IP. - addrs, err := net.LookupHost(host) - if err != nil { - // DNS resolution failure — block it. Could be an internal hostname. - return fmt.Errorf("DNS resolution blocked for hostname: %s (%v)", host, err) - } - if len(addrs) == 0 { - return fmt.Errorf("DNS returned no addresses for: %s", host) - } - for _, addr := range addrs { - ip := net.ParseIP(addr) - if ip != nil && (ip.IsLoopback() || ip.IsUnspecified() || ip.IsLinkLocalUnicast() || isPrivateOrMetadataIP(ip)) { - return fmt.Errorf("hostname %s resolves to forbidden IP: %s", host, ip) - } - } - return nil -} - -// isPrivateOrMetadataIP returns true for cloud-metadata / loopback / link-local -// ranges (always) and RFC-1918 / IPv6 ULA ranges (self-hosted only). -// -// In SaaS cross-EC2 mode (see saasMode() in registry.go) the tenant platform -// and its workspaces share a VPC, so workspaces register with their -// VPC-private IP — typically 172.31.x.x on AWS default VPCs. Blocking RFC-1918 -// unconditionally would reject every legitimate registration. Cloud metadata -// (169.254.0.0/16, fe80::/10), loopback, and TEST-NET ranges stay blocked in -// both modes; they are never a legitimate agent URL. -// -// Both IPv4 and IPv6 are checked. The previous implementation returned false -// for every non-IPv4 input, which meant a registered [::1] or [fe80::…] -// URL would bypass the SSRF gate entirely. -func isPrivateOrMetadataIP(ip net.IP) bool { - // Always blocked — IPv4 cloud metadata + network-test ranges. - metadataRangesV4 := []string{ - "169.254.0.0/16", // link-local / IMDSv1-v2 - "100.64.0.0/10", // CGNAT — reachable via some VPC configs, not a legit agent URL - "192.0.2.0/24", // TEST-NET-1 - "198.51.100.0/24", // TEST-NET-2 - "203.0.113.0/24", // TEST-NET-3 - } - // Always blocked — IPv6 cloud-metadata / loopback equivalents. - metadataRangesV6 := []string{ - "::1/128", // loopback - "fe80::/10", // link-local (IMDS analogue) - "::ffff:0:0/96", // IPv4-mapped loopback (defence-in-depth; To4() below usually normalises first) - } - // RFC-1918 private — blocked in self-hosted, allowed in SaaS. - rfc1918RangesV4 := []string{ - "10.0.0.0/8", - "172.16.0.0/12", - "192.168.0.0/16", - } - // RFC-4193 ULA — IPv6 analogue of RFC-1918. Same SaaS-mode treatment. - ulaRangesV6 := []string{ - "fc00::/7", - } - - contains := func(cidrs []string, target net.IP) bool { - for _, c := range cidrs { - _, n, err := net.ParseCIDR(c) - if err != nil { - continue - } - if n.Contains(target) { - return true - } - } - return false - } - - // Prefer IPv4 semantics when the input is an IPv4 address encoded in any - // form (raw v4, ::ffff:a.b.c.d, etc.) — To4() normalises all of them. - if ip4 := ip.To4(); ip4 != nil { - if contains(metadataRangesV4, ip4) { - return true - } - if saasMode() { - return false - } - return contains(rfc1918RangesV4, ip4) - } - - // True IPv6 path. - if contains(metadataRangesV6, ip) { - return true - } - if saasMode() { - return false - } - return contains(ulaRangesV6, ip) -} +// isSafeURL and isPrivateOrMetadataIP live in ssrf.go (single source of truth). // readUsageMap extracts input_tokens / output_tokens from the "usage" key of m. // Returns (0, 0, false) when the key is absent or contains no non-zero values. diff --git a/workspace-server/internal/handlers/mcp_tools.go b/workspace-server/internal/handlers/mcp_tools.go index 26df4fdd7..10b61ff1d 100644 --- a/workspace-server/internal/handlers/mcp_tools.go +++ b/workspace-server/internal/handlers/mcp_tools.go @@ -14,9 +14,7 @@ import ( "fmt" "io" "log" - "net" "net/http" - "net/url" "os" "strings" "time" @@ -460,75 +458,7 @@ func (h *MCPHandler) toolRecallMemory(ctx context.Context, workspaceID string, a return string(b), nil } -// isSafeURL validates that a URL resolves to a publicly-routable address, -// preventing A2A requests from being redirected to internal/cloud-metadata -// infrastructure (SSRF, CWE-918). Workspace URLs come from DB/Redis caches -// so we validate before making any outbound HTTP call. -func isSafeURL(rawURL string) error { - u, err := url.Parse(rawURL) - if err != nil { - return fmt.Errorf("invalid URL: %w", err) - } - // Reject non-HTTP(S) schemes. - if u.Scheme != "http" && u.Scheme != "https" { - return fmt.Errorf("forbidden scheme: %s (only http/https allowed)", u.Scheme) - } - host := u.Hostname() - if host == "" { - return fmt.Errorf("empty hostname") - } - // Block direct IP addresses. - if ip := net.ParseIP(host); ip != nil { - if ip.IsLoopback() || ip.IsUnspecified() || ip.IsLinkLocalUnicast() { - return fmt.Errorf("forbidden loopback/unspecified IP: %s", ip) - } - if isPrivateOrMetadataIP(ip) { - return fmt.Errorf("forbidden private/metadata IP: %s", ip) - } - return nil - } - // For hostnames, resolve and validate each returned IP. - addrs, err := net.LookupHost(host) - if err != nil { - // DNS resolution failure — block it. Could be an internal hostname. - return fmt.Errorf("DNS resolution blocked for hostname: %s (%v)", host, err) - } - if len(addrs) == 0 { - return fmt.Errorf("DNS returned no addresses for: %s", host) - } - for _, addr := range addrs { - ip := net.ParseIP(addr) - if ip != nil && (ip.IsLoopback() || ip.IsUnspecified() || ip.IsLinkLocalUnicast() || isPrivateOrMetadataIP(ip)) { - return fmt.Errorf("hostname %s resolves to forbidden IP: %s", host, ip) - } - } - return nil -} - -// isPrivateOrMetadataIP returns true for RFC-1918 private, carrier-grade NAT, -// link-local, and cloud metadata ranges. -func isPrivateOrMetadataIP(ip net.IP) bool { - var privateRanges = []net.IPNet{ - {IP: net.ParseIP("10.0.0.0"), Mask: net.CIDRMask(8, 32)}, - {IP: net.ParseIP("172.16.0.0"), Mask: net.CIDRMask(12, 32)}, - {IP: net.ParseIP("192.168.0.0"), Mask: net.CIDRMask(16, 32)}, - {IP: net.ParseIP("169.254.0.0"), Mask: net.CIDRMask(16, 32)}, - {IP: net.ParseIP("100.64.0.0"), Mask: net.CIDRMask(10, 32)}, - {IP: net.ParseIP("192.0.2.0"), Mask: net.CIDRMask(24, 32)}, - {IP: net.ParseIP("198.51.100.0"), Mask: net.CIDRMask(24, 32)}, - {IP: net.ParseIP("203.0.113.0"), Mask: net.CIDRMask(24, 32)}, - } - ip = ip.To4() - if ip == nil { - return false - } - for _, r := range privateRanges { - if r.Contains(ip) { - return true - } - } - return false -} +// isSafeURL and isPrivateOrMetadataIP live in ssrf.go (single source of truth). // ───────────────────────────────────────────────────────────────────────────── // Helpers diff --git a/workspace-server/internal/handlers/templates.go b/workspace-server/internal/handlers/templates.go index 7e87ab2a4..fae258691 100644 --- a/workspace-server/internal/handlers/templates.go +++ b/workspace-server/internal/handlers/templates.go @@ -61,14 +61,7 @@ func (h *TemplatesHandler) resolveTemplateDir(wsName string) string { return "" } -// validateRelPath checks that a relative path doesn't escape the target directory. -func validateRelPath(relPath string) error { - clean := filepath.Clean(relPath) - if filepath.IsAbs(clean) || strings.HasPrefix(clean, "..") { - return fmt.Errorf("path traversal blocked: %s", relPath) - } - return nil -} +// validateRelPath lives in ssrf.go (single source of truth). // List handles GET /templates func (h *TemplatesHandler) List(c *gin.Context) {