fix(security): add dial-time IP guard to stop SSRF / DNS-rebinding (#31)

* fix(security): add dial-time IP guard to stop SSRF / DNS-rebinding

danger.ClassifyURL inspects only the literal hostname, so a domain whose
A/AAAA record resolves to 169.254.169.254, 10.x, 192.168.x, or fd00::/8
classified as plain NetworkEgress and the HTTP clients used the default
transport with no post-resolution check — an SSRF to cloud metadata /
internal services, plus a DNS-rebinding window between the gate's view and
the kernel's dial.

- Add danger.IsBlockedIP (single source of truth for internal ranges:
  loopback, RFC1918/RFC4193 private incl. IPv6 ULA, link-local incl. the
  metadata endpoint, unspecified) and HostIsImplicitlyInternal; refactor
  ClassifyURL to share them (behavior unchanged).
- Add a shared ssrfGuardedDial transport: for a host that presents as
  external it resolves the name, refuses if ANY answer is internal (fail
  closed), then pins the dial to a validated IP so the kernel cannot
  re-resolve to a rebound address. Hosts already internal by inspection
  are left to the policy gate (preserves explicitly-allowed localhost).
- Install the guarded transport on the browser and http_batch clients.
  Because every redirect hop dials through it, this also re-checks
  redirect targets at the IP layer.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* harden(ssrf): fail safe if http.DefaultTransport was globally replaced

vprotocol axis 2.4: ssrfGuardedTransport ran at startup with an unchecked
http.DefaultTransport.(*http.Transport) assertion. A third-party package
that swaps the global for a custom RoundTripper would panic odek at boot.
Use a checked assertion with a fresh-transport fallback.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

Author: jkyberneees

cmd/odek/browser_tool.go

@@ -66,7 +66,10 @@ func newBrowserTool(dc danger.DangerousConfig) *browserTool {
 		state:           &browserState{nextRef: 1},
 		dangerousConfig: dc,
 	}
-	t.client = &http.Client{CheckRedirect: t.checkRedirect}
+	t.client = &http.Client{
+		CheckRedirect: t.checkRedirect,
+		Transport:     ssrfGuardedTransport(),
+	}
 	return t
 }
 
@@ -162,7 +165,10 @@ func (t *browserTool) Call(argsJSON string) (string, error) {
 		t.state = &browserState{nextRef: 1}
 	}
 	if t.client == nil {
-		t.client = &http.Client{CheckRedirect: t.checkRedirect}
+		t.client = &http.Client{
+			CheckRedirect: t.checkRedirect,
+			Transport:     ssrfGuardedTransport(),
+		}
 	}
 
 	switch args.Action {

cmd/odek/perf_tools.go

@@ -426,6 +426,7 @@ func newHTTPBatchTool(dc danger.DangerousConfig) *httpBatchTool {
 	t.client = &http.Client{
 		Timeout:       30 * time.Second,
 		CheckRedirect: t.checkRedirect,
+		Transport:     ssrfGuardedTransport(),
 	}
 	return t
 }

cmd/odek/ssrf_guard.go

@@ -0,0 +1,100 @@
+package main
+
+import (
+	"context"
+	"fmt"
+	"net"
+	"net/http"
+	"time"
+
+	"github.com/BackendStack21/odek/internal/danger"
+)
+
+// SSRF / DNS-rebinding dial guard.
+//
+// danger.ClassifyURL inspects only the literal hostname of a URL, so a domain
+// whose A/AAAA record points at 169.254.169.254 (cloud metadata), 10.x,
+// 192.168.x, fd00::/8, etc. sails through the pre-request gate as plain
+// NetworkEgress. With the default HTTP transport the kernel then resolves and
+// connects with no second check — an SSRF. The same literal-only gate leaves a
+// DNS-rebinding window: the address the policy saw and the address actually
+// dialed can differ.
+//
+// ssrfGuardedDial closes both holes at the transport layer. For a host that
+// presented as external it resolves the name itself, refuses the connection if
+// ANY answer is an internal range (fail closed), and then pins the dial to an
+// already-validated IP so the kernel cannot re-resolve to a rebound address.
+// Because every redirect hop dials through the same transport, this also
+// re-checks redirect targets without any per-tool redirect logic.
+
+// dialFunc matches (*net.Dialer).DialContext and http.Transport.DialContext.
+type dialFunc func(ctx context.Context, network, addr string) (net.Conn, error)
+
+// ipLookupFunc matches (*net.Resolver).LookupIPAddr; injectable for testing.
+type ipLookupFunc func(ctx context.Context, host string) ([]net.IPAddr, error)
+
+// ssrfGuardedDial wraps a base dial function with a post-resolution IP check.
+// Hosts that are already internal by inspection (a literal internal IP or a
+// known-internal hostname) are dialed through unchanged: ClassifyURL already
+// surfaced them to the policy gate as SystemWrite, so honouring that decision
+// here preserves explicitly-allowed localhost access (and keeps httptest-backed
+// tests working). Every other host is resolved via lookup and validated.
+func ssrfGuardedDial(base dialFunc, lookup ipLookupFunc) dialFunc {
+	return func(ctx context.Context, network, addr string) (net.Conn, error) {
+		host, port, err := net.SplitHostPort(addr)
+		if err != nil {
+			return nil, err
+		}
+
+		if danger.HostIsImplicitlyInternal(host) {
+			return base(ctx, network, addr)
+		}
+
+		ips, err := lookup(ctx, host)
+		if err != nil {
+			return nil, err
+		}
+		if len(ips) == 0 {
+			return nil, fmt.Errorf("no addresses found for %q", host)
+		}
+		// Validate every answer before dialing any of them: an attacker can
+		// return a safe IP alongside an internal one hoping the dialer picks
+		// the internal address. Refuse the whole set if any is internal.
+		for _, ipa := range ips {
+			if danger.IsBlockedIP(ipa.IP) {
+				return nil, fmt.Errorf("blocked connection to %q: resolves to internal address %s (possible SSRF / DNS rebinding)", host, ipa.IP)
+			}
+		}
+		// Pin to validated IPs — never hand the hostname back to the kernel,
+		// which would resolve a second time and reopen the rebinding window.
+		// Try each in order so a single unreachable answer still fails over.
+		var dialErr error
+		for _, ipa := range ips {
+			conn, err := base(ctx, network, net.JoinHostPort(ipa.IP.String(), port))
+			if err == nil {
+				return conn, nil
+			}
+			dialErr = err
+		}
+		return nil, dialErr
+	}
+}
+
+// ssrfGuardedTransport returns an *http.Transport whose DialContext is the SSRF
+// guard above, backed by the real dialer and resolver. It clones the default
+// transport when possible so it inherits sane defaults (env proxy handling,
+// idle-conn limits, HTTP/2, TLS handshake timeout); if a third-party package
+// has swapped http.DefaultTransport for a non-*http.Transport RoundTripper, it
+// falls back to a fresh transport with explicit proxy handling rather than
+// panicking on the type assertion — this runs at startup, so it must fail safe.
+func ssrfGuardedTransport() *http.Transport {
+	base := &net.Dialer{Timeout: 30 * time.Second, KeepAlive: 30 * time.Second}
+	tr, ok := http.DefaultTransport.(*http.Transport)
+	if ok {
+		tr = tr.Clone()
+	} else {
+		tr = &http.Transport{Proxy: http.ProxyFromEnvironment}
+	}
+	tr.DialContext = ssrfGuardedDial(base.DialContext, net.DefaultResolver.LookupIPAddr)
+	return tr
+}

cmd/odek/ssrf_guard_test.go

@@ -0,0 +1,187 @@
+package main
+
+import (
+	"context"
+	"net"
+	"net/http"
+	"strings"
+	"testing"
+
+	"github.com/BackendStack21/odek/internal/danger"
+)
+
+// recordingDial returns a dialFunc that records the addr it was asked to dial
+// and never actually connects (returns nil, nil). Tests assert on *what* the
+// guard decided to dial, not on a live connection.
+func recordingDial(dialed *[]string) dialFunc {
+	return func(_ context.Context, _ string, addr string) (net.Conn, error) {
+		*dialed = append(*dialed, addr)
+		return nil, nil
+	}
+}
+
+func stubLookup(ips ...string) ipLookupFunc {
+	return func(_ context.Context, _ string) ([]net.IPAddr, error) {
+		out := make([]net.IPAddr, 0, len(ips))
+		for _, s := range ips {
+			out = append(out, net.IPAddr{IP: net.ParseIP(s)})
+		}
+		return out, nil
+	}
+}
+
+func TestSSRFGuardedDial_ImplicitlyInternalDialedDirect(t *testing.T) {
+	// A literal internal IP was already gated as SystemWrite by ClassifyURL;
+	// the guard must dial it through unchanged without a DNS lookup (this is
+	// what keeps httptest-on-127.0.0.1 tests working).
+	var dialed []string
+	lookupCalled := false
+	lookup := func(_ context.Context, _ string) ([]net.IPAddr, error) {
+		lookupCalled = true
+		return nil, nil
+	}
+	guard := ssrfGuardedDial(recordingDial(&dialed), lookup)
+
+	if _, err := guard(context.Background(), "tcp", "127.0.0.1:8080"); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if lookupCalled {
+		t.Error("lookup must NOT be called for an implicitly-internal literal IP")
+	}
+	if len(dialed) != 1 || dialed[0] != "127.0.0.1:8080" {
+		t.Errorf("dialed = %v, want [127.0.0.1:8080]", dialed)
+	}
+}
+
+func TestSSRFGuardedDial_ExternalResolvingInternalRefused(t *testing.T) {
+	// The SSRF / rebinding core case: a host that presents as external but
+	// resolves to an internal address must be refused, and no dial attempted.
+	cases := []struct {
+		name string
+		ip   string
+	}{
+		{"cloud metadata", "169.254.169.254"},
+		{"rfc1918", "10.1.2.3"},
+		{"loopback", "127.0.0.1"},
+		{"ipv6 ula", "fd00::1"},
+		{"unspecified", "0.0.0.0"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			var dialed []string
+			guard := ssrfGuardedDial(recordingDial(&dialed), stubLookup(tc.ip))
+
+			_, err := guard(context.Background(), "tcp", "evil.example.com:80")
+			if err == nil {
+				t.Fatal("expected the connection to be refused, got nil error")
+			}
+			if !strings.Contains(err.Error(), "internal address") {
+				t.Errorf("error %q should mention the internal address", err)
+			}
+			if len(dialed) != 0 {
+				t.Errorf("no dial must be attempted, dialed = %v", dialed)
+			}
+		})
+	}
+}
+
+func TestSSRFGuardedDial_MixedAnswersFailClosed(t *testing.T) {
+	// An answer set mixing a safe and an internal IP must be refused entirely —
+	// an attacker cannot smuggle an internal target past the guard by padding
+	// the DNS response with a public address.
+	var dialed []string
+	guard := ssrfGuardedDial(recordingDial(&dialed), stubLookup("93.184.216.34", "10.0.0.1"))
+
+	if _, err := guard(context.Background(), "tcp", "evil.example.com:80"); err == nil {
+		t.Fatal("expected refusal for mixed safe+internal answers")
+	}
+	if len(dialed) != 0 {
+		t.Errorf("no dial must be attempted, dialed = %v", dialed)
+	}
+}
+
+func TestSSRFGuardedDial_ExternalPinnedToValidatedIP(t *testing.T) {
+	// A genuinely external host is dialed — but pinned to the validated IP, not
+	// the hostname, so the kernel cannot re-resolve to a rebound address.
+	var dialed []string
+	guard := ssrfGuardedDial(recordingDial(&dialed), stubLookup("93.184.216.34"))
+
+	if _, err := guard(context.Background(), "tcp", "example.com:443"); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if len(dialed) != 1 || dialed[0] != "93.184.216.34:443" {
+		t.Errorf("dialed = %v, want [93.184.216.34:443] (pinned IP, not hostname)", dialed)
+	}
+}
+
+// TestBrowser_SSRF_ResolvesInternal exercises the guard through the real
+// browser navigate path: the hostname classifies as NetworkEgress (so the
+// policy gate lets it through) but resolves to the cloud-metadata IP, and the
+// dial guard refuses it.
+func TestBrowser_SSRF_ResolvesInternal(t *testing.T) {
+	b := &browserTool{state: &browserState{nextRef: 1}}
+	b.client = &http.Client{
+		CheckRedirect: b.checkRedirect,
+		Transport: &http.Transport{
+			DialContext: ssrfGuardedDial((&net.Dialer{}).DialContext, stubLookup("169.254.169.254")),
+		},
+	}
+
+	result := callJSON(t, b, `{"action":"navigate","url":"http://internal-disguised.example.com/"}`)
+	var r struct {
+		Error string `json:"error"`
+	}
+	mustUnmarshal(t, result, &r)
+	if r.Error == "" {
+		t.Fatal("expected navigate to be blocked by the dial guard")
+	}
+	if !strings.Contains(r.Error, "internal address") && !strings.Contains(r.Error, "SSRF") {
+		t.Errorf("error %q should explain the SSRF block", r.Error)
+	}
+}
+
+// TestHTTPBatch_SSRF_ResolvesInternal exercises the guard through the real
+// http_batch fetch path.
+func TestHTTPBatch_SSRF_ResolvesInternal(t *testing.T) {
+	tool := newHTTPBatchTool(danger.DangerousConfig{})
+	tool.client = &http.Client{
+		CheckRedirect: tool.checkRedirect,
+		Transport: &http.Transport{
+			DialContext: ssrfGuardedDial((&net.Dialer{}).DialContext, stubLookup("10.0.0.1")),
+		},
+	}
+
+	result := callJSON(t, tool, `{"requests":[{"url":"http://internal-disguised.example.com/"}]}`)
+	var r struct {
+		Results []struct {
+			Error string `json:"error"`
+		} `json:"results"`
+	}
+	mustUnmarshal(t, result, &r)
+	if len(r.Results) != 1 {
+		t.Fatalf("Results = %d, want 1", len(r.Results))
+	}
+	if r.Results[0].Error == "" {
+		t.Fatal("expected fetch to be blocked by the dial guard")
+	}
+}
+
+// TestSSRFGuardedTransport_Installed is a guard against regressions that would
+// silently drop the SSRF protection from the production constructors.
+func TestSSRFGuardedTransport_Installed(t *testing.T) {
+	b := newBrowserTool(danger.DangerousConfig{})
+	if b.client.Transport == nil {
+		t.Error("browser tool client has no Transport — SSRF guard not installed")
+	}
+	if tr, ok := b.client.Transport.(*http.Transport); !ok || tr.DialContext == nil {
+		t.Error("browser tool Transport is missing the guarded DialContext")
+	}
+
+	h := newHTTPBatchTool(danger.DangerousConfig{})
+	if h.client.Transport == nil {
+		t.Error("http_batch tool client has no Transport — SSRF guard not installed")
+	}
+	if tr, ok := h.client.Transport.(*http.Transport); !ok || tr.DialContext == nil {
+		t.Error("http_batch tool Transport is missing the guarded DialContext")
+	}
+}