diff --git a/workspace-server/internal/handlers/registry.go b/workspace-server/internal/handlers/registry.go index 50a254ae..19ca8006 100644 --- a/workspace-server/internal/handlers/registry.go +++ b/workspace-server/internal/handlers/registry.go @@ -142,13 +142,27 @@ func validateAgentURL(rawURL string) error { {"127.0.0.0/8", "loopback address"}, {"fe80::/10", "IPv6 link-local address (cloud metadata analogue)"}, {"::1/128", "IPv6 loopback address"}, + // Always-blocked regardless of deploy mode: these ranges are never valid + // agent URLs in any deployment. TEST-NET (RFC-5737) are documentation-only + // ranges. CGNAT (RFC-6598) is never used for VPC subnets on any cloud + // provider. IPv4 multicast is never a unicast endpoint. fc00::/8 is the + // non-routable prefix of IPv6 ULA (fd00::/8 is allowed in SaaS mode). + {"192.0.2.0/24", "TEST-NET-1 documentation range (RFC-5737)"}, + {"198.51.100.0/24", "TEST-NET-2 documentation range (RFC-5737)"}, + {"203.0.113.0/24", "TEST-NET-3 documentation range (RFC-5737)"}, + {"100.64.0.0/10", "carrier-grade NAT address (RFC-6598)"}, + {"224.0.0.0/4", "IPv4 multicast address"}, + {"fc00::/8", "IPv6 ULA non-routable prefix (fc00::/8)"}, } if !saasMode() { blockedRanges = append(blockedRanges, blockedRange{"10.0.0.0/8", "RFC-1918 private address"}, blockedRange{"172.16.0.0/12", "RFC-1918 private address"}, blockedRange{"192.168.0.0/16", "RFC-1918 private address"}, - blockedRange{"fc00::/7", "IPv6 ULA address (RFC-4193 private)"}, + // In SaaS mode fd00::/8 (common ULA prefix) is allowed for VPC-internal + // routing. fc00::/8 is already always-blocked above. In non-SaaS mode + // block the entire fc00::/7 supernet (covers both fd00 and fc00). + blockedRange{"fd00::/8", "IPv6 ULA address (RFC-4193 private)"}, ) } diff --git a/workspace-server/internal/handlers/registry_test.go b/workspace-server/internal/handlers/registry_test.go index 4d2cb904..a9ebc025 100644 --- a/workspace-server/internal/handlers/registry_test.go +++ b/workspace-server/internal/handlers/registry_test.go @@ -570,6 +570,91 @@ func TestValidateAgentURL(t *testing.T) { } } +// TestValidateAgentURL_SaaSMode_AllowsRFC1918 is the integration-level wrapper test +// for the SaaS-mode SSRF relaxation in validateAgentURL (used at registration). +// It exercises validateAgentURL as called by the Register handler, not just the +// inner blockedRanges slice. Regression guard for the same class of bug as +// isSafeURL (issue #1785). +func TestValidateAgentURL_SaaSMode_AllowsRFC1918(t *testing.T) { + t.Setenv("MOLECULE_DEPLOY_MODE", "saas") + t.Setenv("MOLECULE_ORG_ID", "") + for _, url := range []string{ + "http://10.1.2.3/agent", + "http://10.0.0.5:8000/a2a", + "http://172.16.0.1/agent", + "http://172.18.0.42:8000/a2a", + "http://172.31.44.78/agent", + "http://192.168.1.100/agent", + "http://192.168.255.254:9000/a2a", + "http://[fd00::1]/agent", + "http://[fd12:3456:789a::42]/a2a", + } { + if err := validateAgentURL(url); err != nil { + t.Errorf("validateAgentURL(%q) in saasMode: got %v, want nil", url, err) + } + } +} + +// TestValidateAgentURL_SaaSMode_StillBlocksMetadataEtAl verifies that even in +// SaaS mode the always-blocked ranges (metadata, loopback, TEST-NET, CGNAT, +// non-fd00 ULA) stay blocked. +func TestValidateAgentURL_SaaSMode_StillBlocksMetadataEtAl(t *testing.T) { + t.Setenv("MOLECULE_DEPLOY_MODE", "saas") + t.Setenv("MOLECULE_ORG_ID", "") + for _, url := range []string{ + "http://169.254.169.254/latest/meta-data/", + "http://169.254.0.1/", + "http://127.0.0.1:8080", + "http://[::1]:8080", + "http://192.0.2.5/agent", + "http://198.51.100.5/a2a", + "http://203.0.113.42/agent", + "http://100.64.0.1/agent", + "http://100.127.255.254:8000/a2a", + "http://[fc00::1]/agent", + "http://224.0.0.1/", + } { + if err := validateAgentURL(url); err == nil { + t.Errorf("validateAgentURL(%q) in saasMode: got nil, want block", url) + } + } +} + +// TestValidateAgentURL_StrictMode_BlocksRFC1918 is the strict-mode counterpart +// to TestValidateAgentURL_SaaSMode_AllowsRFC1918. +func TestValidateAgentURL_StrictMode_BlocksRFC1918(t *testing.T) { + t.Setenv("MOLECULE_DEPLOY_MODE", "self-hosted") + t.Setenv("MOLECULE_ORG_ID", "") + for _, url := range []string{ + "http://10.1.2.3/agent", + "http://172.16.0.1:8000/a2a", + "http://172.31.44.78/agent", + "http://192.168.1.100/agent", + "http://[fd00::1]/agent", + } { + if err := validateAgentURL(url); err == nil { + t.Errorf("validateAgentURL(%q) in strict mode: got nil, want block", url) + } + } +} + +// TestValidateAgentURL_SaaSMode_LegacyOrgID covers the legacy MOLECULE_ORG_ID +// signal (no MOLECULE_DEPLOY_MODE set) for validateAgentURL. +func TestValidateAgentURL_SaaSMode_LegacyOrgID(t *testing.T) { + t.Setenv("MOLECULE_DEPLOY_MODE", "") + t.Setenv("MOLECULE_ORG_ID", "7b2179dc-8cc6-4581-a3c6-c8bff4481086") + for _, url := range []string{ + "http://10.1.2.3/agent", + "http://172.18.0.42:8000/a2a", + "http://192.168.1.100/agent", + "http://[fd00::1]/agent", + } { + if err := validateAgentURL(url); err != nil { + t.Errorf("validateAgentURL(%q) with legacy MOLECULE_ORG_ID: got %v, want nil", url, err) + } + } +} + // ==================== C18 — Register ownership ==================== // TestRegister_C18_BootstrapAllowedNoTokens verifies that a workspace with NO diff --git a/workspace-server/internal/handlers/ssrf_test.go b/workspace-server/internal/handlers/ssrf_test.go index 85412760..37b2b358 100644 --- a/workspace-server/internal/handlers/ssrf_test.go +++ b/workspace-server/internal/handlers/ssrf_test.go @@ -326,4 +326,101 @@ func TestDevModeAllowsLoopback_Predicate(t *testing.T) { } }) } +} + +// TestIsSafeURL_SaaSMode_AllowsRFC1918 is the integration-level wrapper test +// for the SaaS-mode SSRF relaxation. It exercises isSafeURL (the public API), +// not isPrivateOrMetadataIP (the inner helper), ensuring the wrapper correctly +// propagates saasMode() to its helper. +// +// Regression guard: isSafeURL previously hardcoded RFC-1918 rejection and never +// called saasMode(), causing 502 on every A2A call from Docker-networked or VPC +// deployments (issue #1785 / PR #1785). The inner helper's TestIsPrivateOrMetadataIP_SaaSMode +// was green the whole time — classic "test the intent, not the integration" gap. +func TestIsSafeURL_SaaSMode_AllowsRFC1918(t *testing.T) { + t.Setenv("MOLECULE_DEPLOY_MODE", "saas") + t.Setenv("MOLECULE_ORG_ID", "") + for _, url := range []string{ + "http://10.1.2.3/agent", + "http://10.0.0.5:8000/a2a", + "http://172.16.0.1/agent", + "http://172.18.0.42:8000/a2a", + "http://172.31.44.78/agent", + "http://192.168.1.100/agent", + "http://192.168.255.254:9000/a2a", + "http://[fd00::1]/agent", + "http://[fd12:3456:789a::42]/a2a", + } { + if err := isSafeURL(url); err != nil { + t.Errorf("isSafeURL(%q) in saasMode: got %v, want nil", url, err) + } + } +} + +// TestIsSafeURL_SaaSMode_StillBlocksMetadataEtAl verifies that even in SaaS +// mode the always-blocked ranges (metadata, loopback, TEST-NET, CGNAT) stay blocked. +func TestIsSafeURL_SaaSMode_StillBlocksMetadataEtAl(t *testing.T) { + t.Setenv("MOLECULE_DEPLOY_MODE", "saas") + t.Setenv("MOLECULE_ORG_ID", "") + for _, url := range []string{ + // Cloud metadata — must stay blocked in every mode. + "http://169.254.169.254/latest/meta-data/", + "http://169.254.0.1/", + // Loopback — must stay blocked. + "http://127.0.0.1:8080", + "http://[::1]:8080", + // TEST-NET documentation ranges — must stay blocked. + "http://192.0.2.5/agent", + "http://198.51.100.5/a2a", + "http://203.0.113.42/agent", + // CGNAT — must stay blocked. + "http://100.64.0.1/agent", + "http://100.127.255.254:8000/a2a", + // ULA fc00::/8 (non-fd00 half) — must stay blocked in SaaS. + "http://[fc00::1]/agent", + // Non-RFC-1918 private ranges still blocked. + "http://224.0.0.1/", + } { + if err := isSafeURL(url); err == nil { + t.Errorf("isSafeURL(%q) in saasMode: got nil, want block", url) + } + } +} + +// TestIsSafeURL_StrictMode_BlocksRFC1918 is the strict-mode counterpart to +// TestIsSafeURL_SaaSMode_AllowsRFC1918. In self-hosted / single-container +// deployments there is no legitimate reason to reach RFC-1918 agents, so the +// wrapper must block them. +func TestIsSafeURL_StrictMode_BlocksRFC1918(t *testing.T) { + t.Setenv("MOLECULE_DEPLOY_MODE", "self-hosted") + t.Setenv("MOLECULE_ORG_ID", "") + for _, url := range []string{ + "http://10.1.2.3/agent", + "http://172.16.0.1:8000/a2a", + "http://172.31.44.78/agent", + "http://192.168.1.100/agent", + "http://[fd00::1]/agent", + } { + if err := isSafeURL(url); err == nil { + t.Errorf("isSafeURL(%q) in strict mode: got nil, want block", url) + } + } +} + +// TestIsSafeURL_SaasMode_LegacyOrgID covers the legacy MOLECULE_ORG_ID signal +// (no MOLECULE_DEPLOY_MODE set). An org ID alone is sufficient to activate SaaS +// mode per the saasMode() resolution ladder. +func TestIsSafeURL_SaasMode_LegacyOrgID(t *testing.T) { + t.Setenv("MOLECULE_DEPLOY_MODE", "") + t.Setenv("MOLECULE_ORG_ID", "7b2179dc-8cc6-4581-a3c6-c8bff4481086") + for _, url := range []string{ + "http://10.1.2.3/agent", + "http://172.18.0.42:8000/a2a", + "http://192.168.1.100/agent", + "http://[fd00::1]/agent", + } { + if err := isSafeURL(url); err != nil { + t.Errorf("isSafeURL(%q) with legacy MOLECULE_ORG_ID: got %v, want nil", url, err) + } + } } \ No newline at end of file