From 8f8be17db46dcae12613ccd3e00d2c518686cd59 Mon Sep 17 00:00:00 2001 From: Molecule AI Core Platform Lead Date: Tue, 21 Apr 2026 17:03:36 +0000 Subject: [PATCH] =?UTF-8?q?fix(core):=20resolve=20main=20build=20=E2=80=94?= =?UTF-8?q?=20remove=20duplicate=20SSRF=20function=20declarations?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Build on origin/main (38e9eba) will fail go build with duplicate function declarations: ssrf.go:15 isSafeURL redeclared (a2a_proxy.go:741) ssrf.go:58 isPrivateOrMetadataIP redeclared (a2a_proxy.go:795) ssrf.go:84 validateRelPath redeclared (templates.go:65) a2a_proxy.go:14 "fmt" imported and not used Root cause: main was fast-forwarded to a CWE-22 fix commit that incorporated ssrf.go from the staging handler-split (PR #1457), but ssrf.go declares isSafeURL/isPrivateOrMetadataIP that already exist in a2a_proxy.go, and validateRelPath that already exists in templates.go. Fix: - Delete ssrf.go entirely — its isSafeURL/isPrivateOrMetadataIP are already in a2a_proxy.go; its validateRelPath is in templates.go. - Remove unused "fmt" import from a2a_proxy.go. - Add t.Setenv cleanup in TestIsPrivateOrMetadataIP and TestIsSafeURL so MOLECULE_DEPLOY_MODE=saas from TestIsPrivateOrMetadataIP_SaaSMode cannot leak into sibling tests. - Update stale file-location comments in ssrf_test.go. Co-Authored-By: Claude Sonnet 4.6 --- .../internal/handlers/a2a_proxy.go | 1 - workspace-server/internal/handlers/ssrf.go | 90 ------------------- .../internal/handlers/ssrf_test.go | 8 +- 3 files changed, 6 insertions(+), 93 deletions(-) delete mode 100644 workspace-server/internal/handlers/ssrf.go diff --git a/workspace-server/internal/handlers/a2a_proxy.go b/workspace-server/internal/handlers/a2a_proxy.go index fd6cd50f..6b9df237 100644 --- a/workspace-server/internal/handlers/a2a_proxy.go +++ b/workspace-server/internal/handlers/a2a_proxy.go @@ -6,7 +6,6 @@ import ( "database/sql" "encoding/json" "errors" - "fmt" "io" "log" "net" diff --git a/workspace-server/internal/handlers/ssrf.go b/workspace-server/internal/handlers/ssrf.go deleted file mode 100644 index 09bb2774..00000000 --- a/workspace-server/internal/handlers/ssrf.go +++ /dev/null @@ -1,90 +0,0 @@ -package handlers - -import ( - "fmt" - "net" - "net/url" - "path/filepath" - "strings" -) - -// 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 -} - -// validateRelPath checks that a file path is relative and does not escape -// the destination via absolute paths or ".." traversal. Used by -// copyFilesToContainer and deleteViaEphemeral as a defence-in-depth measure. -func validateRelPath(filePath string) error { - clean := filepath.Clean(filePath) - if filepath.IsAbs(clean) || strings.Contains(clean, "..") { - return fmt.Errorf("path traversal or absolute path not allowed: %s", filePath) - } - return nil -} \ No newline at end of file diff --git a/workspace-server/internal/handlers/ssrf_test.go b/workspace-server/internal/handlers/ssrf_test.go index 7a48deba..1185f85b 100644 --- a/workspace-server/internal/handlers/ssrf_test.go +++ b/workspace-server/internal/handlers/ssrf_test.go @@ -5,8 +5,8 @@ import ( "testing" ) -// isSafeURL is defined in mcp.go. -// isPrivateOrMetadataIP is defined in mcp.go. +// isSafeURL is defined in a2a_proxy.go. +// isPrivateOrMetadataIP is defined in a2a_proxy.go. // saasMode is defined in registry.go. // TestSaasMode covers the env-resolution ladder so a self-hosted @@ -127,6 +127,8 @@ func TestIsPrivateOrMetadataIP_IPv6(t *testing.T) { } func TestIsPrivateOrMetadataIP(t *testing.T) { + t.Setenv("MOLECULE_DEPLOY_MODE", "") + t.Setenv("MOLECULE_ORG_ID", "") cases := []struct { name string ipStr string @@ -173,6 +175,8 @@ func TestIsPrivateOrMetadataIP(t *testing.T) { } func TestIsSafeURL(t *testing.T) { + t.Setenv("MOLECULE_DEPLOY_MODE", "") + t.Setenv("MOLECULE_ORG_ID", "") cases := []struct { name string rawURL string