From 762d3b8b2cf97f60f8c70f01f685a36527cbc2c5 Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Sun, 26 Apr 2026 10:32:33 -0700 Subject: [PATCH] test(ssrf): pin dev-mode RFC-1918 allow contract (follow-up to #2103) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #2103 widened the SSRF saasMode branch to also relax RFC-1918 + ULA under MOLECULE_ENV=development (so the docker-compose dev pattern stops rejecting workspace registrations on 172.18.x.x bridge IPs). The existing TestIsSafeURL_DevMode_StillBlocksOtherRanges covered the security floor (metadata / TEST-NET / CGNAT stay blocked), but no test asserted the positive side — that 10.x / 172.x / 192.168.x / fd00:: ARE now allowed under dev mode. Without this test, a future refactor that quietly drops the `|| devModeAllowsLoopback()` from isPrivateOrMetadataIP wouldn't trip any assertion, and the docker-compose dev loop would silently re-break. Adds TestIsSafeURL_DevMode_AllowsRFC1918 — table of 4 URLs covering the three RFC-1918 IPv4 ranges + IPv6 ULA fd00::/8. Sets MOLECULE_DEPLOY_MODE=self-hosted explicitly so the test exercises the devMode branch, not a SaaS-mode pass. Closes the Optional finding I left on PR #2103. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../internal/handlers/ssrf_test.go | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/workspace-server/internal/handlers/ssrf_test.go b/workspace-server/internal/handlers/ssrf_test.go index 37b2b358..8e246ebc 100644 --- a/workspace-server/internal/handlers/ssrf_test.go +++ b/workspace-server/internal/handlers/ssrf_test.go @@ -387,6 +387,37 @@ func TestIsSafeURL_SaaSMode_StillBlocksMetadataEtAl(t *testing.T) { } } +// TestIsSafeURL_DevMode_AllowsRFC1918 pins the dev-mode RFC-1918 + ULA +// relaxation that #2103 widened. The dev-host docker-compose pattern +// puts the platform + workspaces on the same docker bridge (172.18.0.0/16), +// so workspace registration via 172.18.x.x must NOT be rejected in dev. +// SaaS already allowed this; dev mode now matches via +// `saas := saasMode() || devModeAllowsLoopback()` in isPrivateOrMetadataIP. +// +// Without this test, a future refactor that quietly drops the +// `|| devModeAllowsLoopback()` from line 130 wouldn't trip any test — +// the existing `TestIsSafeURL_DevMode_StillBlocksOtherRanges` only +// pins the security floor (metadata / TEST-NET / CGNAT), not the +// behavior change. +func TestIsSafeURL_DevMode_AllowsRFC1918(t *testing.T) { + // Make sure saasMode() returns false so the test exercises the + // devModeAllowsLoopback() branch specifically — not a SaaS-mode pass. + t.Setenv("MOLECULE_DEPLOY_MODE", "self-hosted") + t.Setenv("MOLECULE_ORG_ID", "") + t.Setenv("MOLECULE_ENV", "development") + + for _, url := range []string{ + "http://10.1.2.3/agent", + "http://172.18.0.42:8000/a2a", // the docker-compose case from the issue + "http://192.168.1.100/agent", + "http://[fd00::1]/agent", // IPv6 ULA fd00::/8 also relaxed + } { + if err := isSafeURL(url); err != nil { + t.Errorf("isSafeURL(%q) in dev mode: got %v, want nil", url, err) + } + } +} + // 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