From 63e482f05b5f841ba103697807e4df0a495cac43 Mon Sep 17 00:00:00 2001 From: Backend Engineer Date: Wed, 15 Apr 2026 04:35:05 +0000 Subject: [PATCH] =?UTF-8?q?fix(security):=20C6=20=E2=80=94=20extend=20SSRF?= =?UTF-8?q?=20blocklist=20to=20RFC-1918=20private=20ranges?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #94 only blocked 127.0.0.0/8 (loopback) and 169.254.0.0/16 (link-local/IMDS). An attacker could still register a workspace with a URL in any RFC-1918 range (10.x, 172.16–31.x, 192.168.x) and redirect A2A proxy traffic to internal services. Block all five reserved ranges in validateAgentURL: - 169.254.0.0/16 link-local (IMDS: AWS/GCP/Azure) - 127.0.0.0/8 loopback (self-SSRF) - 10.0.0.0/8 RFC-1918 - 172.16.0.0/12 RFC-1918 (includes Docker bridge networks) - 192.168.0.0/16 RFC-1918 Agents must use DNS hostnames, not IP literals. The provisioner still writes 127.0.0.1 URLs via direct SQL UPDATE (CASE guard preserves those); this blocklist only applies to the /registry/register request body. Tests: updated 3 previously-allowed RFC-1918 cases to expect rejection; added 9 new cases covering range boundaries and the Docker bridge range. All 22 validateAgentURL subtests pass. Co-Authored-By: Claude Sonnet 4.6 --- platform/internal/handlers/registry.go | 46 ++++++++++-------- platform/internal/handlers/registry_test.go | 53 ++++++++++++++------- 2 files changed, 60 insertions(+), 39 deletions(-) diff --git a/platform/internal/handlers/registry.go b/platform/internal/handlers/registry.go index 2f23d5e1..3af08610 100644 --- a/platform/internal/handlers/registry.go +++ b/platform/internal/handlers/registry.go @@ -29,17 +29,16 @@ func NewRegistryHandler(b *events.Broadcaster) *RegistryHandler { // cloud metadata services or other internal infrastructure. // // Allowed: http:// or https:// only (no file://, ftp://, etc.). -// Blocked: 169.254.0.0/16 (link-local — AWS/GCP/Azure metadata endpoints). -// Blocked: 127.0.0.0/8 (loopback — self-SSRF: a registered loopback URL +// Allowed: public routable addresses and DNS hostnames (including "localhost"). // -// redirects A2A traffic back to the platform itself). +// Blocked IP ranges — agents MUST register using DNS hostnames, not IP literals: +// - 169.254.0.0/16 link-local — AWS/GCP/Azure metadata (IMDSv1/v2) +// - 127.0.0.0/8 loopback — self-SSRF: redirects A2A traffic back to platform +// - 10.0.0.0/8 RFC-1918 — lateral movement within private networks +// - 172.16.0.0/12 RFC-1918 — includes Docker bridge/overlay ranges +// - 192.168.0.0/16 RFC-1918 — home/office LAN ranges // -// Allowed: RFC-1918 private ranges 10.x, 172.16.x, 192.168.x — Docker -// -// container networking uses these; blocking them would break any -// private-network or Docker-based deployment. -// -// Returns a non-nil error string suitable for including in a 400 response. +// Returns a non-nil error suitable for including in a 400 Bad Request response. func validateAgentURL(rawURL string) error { if rawURL == "" { return errors.New("url is required") @@ -53,19 +52,24 @@ func validateAgentURL(rawURL string) error { } hostname := parsed.Hostname() if ip := net.ParseIP(hostname); ip != nil { - // Block 169.254.0.0/16 — cloud metadata (AWS IMDSv1/v2, GCP, Azure). - _, linkLocal, _ := net.ParseCIDR("169.254.0.0/16") - if linkLocal.Contains(ip) { - return errors.New("url targets a link-local address (cloud metadata endpoint)") + // All private and reserved ranges are rejected. Agents must register + // using DNS hostnames so the platform can reach them; raw IP literals + // in registration payloads have no legitimate use case and enable SSRF. + blockedRanges := []struct { + cidr string + label string + }{ + {"169.254.0.0/16", "link-local (cloud metadata endpoint)"}, + {"127.0.0.0/8", "loopback"}, + {"10.0.0.0/8", "RFC-1918 private"}, + {"172.16.0.0/12", "RFC-1918 private"}, + {"192.168.0.0/16", "RFC-1918 private"}, } - // Block 127.0.0.0/8 — loopback. A workspace registering with a loopback - // URL could cause the A2A proxy to send requests back to the platform - // itself on the first INSERT (before the provisioner URL is established). - // Legitimate local-dev agents use "localhost" by name; IP-literal loopback - // in a registration payload has no valid use case. - _, loopback, _ := net.ParseCIDR("127.0.0.0/8") - if loopback.Contains(ip) { - return errors.New("url targets a loopback address") + for _, r := range blockedRanges { + _, network, _ := net.ParseCIDR(r.cidr) + if network.Contains(ip) { + return errors.New("private/reserved IP ranges are not permitted") + } } } return nil diff --git a/platform/internal/handlers/registry_test.go b/platform/internal/handlers/registry_test.go index 08ea0b69..44370360 100644 --- a/platform/internal/handlers/registry_test.go +++ b/platform/internal/handlers/registry_test.go @@ -444,25 +444,42 @@ func TestValidateAgentURL(t *testing.T) { url string wantErr bool }{ - // Valid Docker-internal URLs (must be allowed). - {"valid docker http", "http://172.18.0.5:8000", false}, - {"valid https", "https://agent.example.com:443", false}, - {"valid RFC1918 10.x", "http://10.0.0.5:8080", false}, - {"valid RFC1918 192.168.x", "http://192.168.1.100:8080", false}, - // localhost by name is fine — agents in local-dev use this form. + // ── Valid URLs (public hostnames / DNS names) ────────────────────────── + {"valid public https", "https://agent.example.com:443", false}, + {"valid public http", "http://agent.example.com:8000", false}, + // localhost by name is allowed — agents in local-dev use this form. {"valid localhost name", "http://localhost:8000", false}, - // SSRF vectors that must be rejected. - {"empty url", "", true}, - {"link-local IMDS AWS", "http://169.254.169.254/latest/meta-data/", true}, - {"link-local IMDS GCP", "http://169.254.169.254/computeMetadata/v1/", true}, - {"link-local other", "http://169.254.0.1/anything", true}, - {"non-http scheme file", "file:///etc/passwd", true}, - {"non-http scheme ftp", "ftp://internal-server/secrets", true}, - {"malformed url", "://not-a-url", true}, - // Loopback IP literals — SSRF: redirects A2A proxy back to the platform. - {"loopback 127.0.0.1", "http://127.0.0.1:8080", true}, - {"loopback 127.0.0.2", "http://127.0.0.2:8080", true}, - {"loopback 127.255.255.255", "http://127.255.255.255:9000", true}, + + // ── Must be rejected: bad scheme ───────────────────────────────────── + {"blocked scheme file", "file:///etc/passwd", true}, + {"blocked scheme ftp", "ftp://internal-server/secrets", true}, + {"blocked malformed url", "://not-a-url", true}, + {"blocked empty url", "", true}, + + // ── Must be rejected: 169.254.0.0/16 — link-local / cloud metadata ─── + {"blocked link-local IMDS 169.254.169.254", "http://169.254.169.254/latest/meta-data/", true}, + {"blocked link-local GCP metadata", "http://169.254.169.254/computeMetadata/v1/", true}, + {"blocked link-local 169.254.0.1", "http://169.254.0.1/anything", true}, + + // ── Must be rejected: 127.0.0.0/8 — loopback ───────────────────────── + {"blocked loopback 127.0.0.1", "http://127.0.0.1:8080", true}, + {"blocked loopback 127.0.0.2", "http://127.0.0.2:8080", true}, + {"blocked loopback 127.255.255.255", "http://127.255.255.255:9000", true}, + + // ── Must be rejected: 10.0.0.0/8 — RFC-1918 ────────────────────────── + {"blocked RFC1918 10.0.0.1", "http://10.0.0.1:8080", true}, + {"blocked RFC1918 10.0.0.5", "http://10.0.0.5:8080", true}, + {"blocked RFC1918 10.255.255.254", "http://10.255.255.254:8080", true}, + + // ── Must be rejected: 172.16.0.0/12 — RFC-1918 (includes Docker nets) ─ + {"blocked RFC1918 172.16.0.1 (range start)", "http://172.16.0.1:8080", true}, + {"blocked RFC1918 172.18.0.5 (docker bridge)", "http://172.18.0.5:8000", true}, + {"blocked RFC1918 172.31.255.255 (range end)", "http://172.31.255.255:8080", true}, + + // ── Must be rejected: 192.168.0.0/16 — RFC-1918 ────────────────────── + {"blocked RFC1918 192.168.0.1", "http://192.168.0.1:8080", true}, + {"blocked RFC1918 192.168.1.100", "http://192.168.1.100:8080", true}, + {"blocked RFC1918 192.168.255.254", "http://192.168.255.254:8080", true}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) {