fix(hooks/security): add SSRF-safe http client missed by phase 2 commit

Phase 2 (a587231b) advertised SSRF hardening for the http hook handler
but the supporting `internal/security` package was never created, so the
production HTTPHandler fell back to a bare http.Client and admin-config
webhooks could probe loopback / link-local / RFC1918 / cloud-metadata.

- internal/security/ssrf.go: Validate(rawURL) parses + resolves once,
  rejects loopback/link-local/private/multicast/unspecified + 169.254.169.254;
  NewSafeClient(timeout) returns an http.Client whose DialContext pins the
  resolved IP from context (defense-in-depth re-checks the dialed IP) and
  refuses redirects (CheckRedirect = ErrUseLastResponse)
- internal/hooks/handlers/http_handler.go: call Validate before each request,
  attach pinned IP via security.WithPinnedIP(ctx, ip)
- cmd/gateway_managed.go: construct HTTPHandler with
  Client: security.NewSafeClient(10*time.Second)
- tests: 13 new ssrf_test.go cases (every block category + redirect refused
  + dial pinned); existing http_test.go retrofitted with
  security.SetAllowLoopbackForTest helper for httptest.NewServer
- plan: phase-02 Step 2a marked done with reference to this commit

Refs: GitHub Issue #875

cmd/gateway_managed.go

@@ -7,6 +7,7 @@ import (
 	"log/slog"
 	"os"
 	"path/filepath"
+	"time"
 
 	"github.com/google/uuid"
 
@@ -27,6 +28,7 @@ import (
 	memorypkg "github.com/nextlevelbuilder/goclaw/internal/memory"
 	"github.com/nextlevelbuilder/goclaw/internal/providers"
 	"github.com/nextlevelbuilder/goclaw/internal/sandbox"
+	"github.com/nextlevelbuilder/goclaw/internal/security"
 	"github.com/nextlevelbuilder/goclaw/internal/skills"
 	"github.com/nextlevelbuilder/goclaw/internal/store"
 	"github.com/nextlevelbuilder/goclaw/internal/store/pg"
@@ -156,7 +158,10 @@ func wireExtras(
 			Audit: hooks.NewAuditWriter(hs, ""),
 			Handlers: map[hooks.HandlerType]hooks.Handler{
 				hooks.HandlerCommand: &hookhandlers.CommandHandler{Edition: edition.Current()},
-				hooks.HandlerHTTP:    &hookhandlers.HTTPHandler{EncryptKey: encryptKey},
+				hooks.HandlerHTTP: &hookhandlers.HTTPHandler{
+					EncryptKey: encryptKey,
+					Client:     security.NewSafeClient(10 * time.Second),
+				},
 			},
 		}
 		hookDispatcher = hooks.NewStdDispatcher(stdOpts)

internal/hooks/handlers/http_handler.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/nextlevelbuilder/goclaw/internal/crypto"
 	"github.com/nextlevelbuilder/goclaw/internal/hooks"
+	"github.com/nextlevelbuilder/goclaw/internal/security"
 )
 
 // HTTPHandler posts event data to an HTTP endpoint and interprets the JSON
@@ -43,6 +44,20 @@ func (h *HTTPHandler) Execute(ctx context.Context, cfg hooks.HookConfig, ev hook
 		return hooks.DecisionError, fmt.Errorf("hook: http handler: missing 'url' in config")
 	}
 
+	// SSRF validation: resolve host once and pin the IP.
+	// When a custom Client is set (e.g. in tests via httptest), the client's
+	// own transport handles routing; Validate is still called so that
+	// production-path URLs are always vetted. Tests that use httptest must
+	// call security.SetAllowLoopbackForTest(true) to bypass the loopback block.
+	parsedURL, pinnedIP, err := security.Validate(urlStr)
+	if err != nil {
+		return hooks.DecisionError, fmt.Errorf("hook: http handler: ssrf check: %w", err)
+	}
+	_ = parsedURL // URL string already validated; we keep urlStr for the request.
+
+	// Stash pinned IP into context so NewSafeClient's DialContext can read it.
+	ctx = security.WithPinnedIP(ctx, pinnedIP)
+
 	body, err := json.Marshal(ev)
 	if err != nil {
 		return hooks.DecisionError, fmt.Errorf("hook: http handler: marshal event: %w", err)

internal/hooks/handlers/http_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/nextlevelbuilder/goclaw/internal/crypto"
 	"github.com/nextlevelbuilder/goclaw/internal/hooks"
 	"github.com/nextlevelbuilder/goclaw/internal/hooks/handlers"
+	"github.com/nextlevelbuilder/goclaw/internal/security"
 )
 
 // makeHTTPCfg builds a minimal HookConfig with given URL.
@@ -23,6 +24,9 @@ func makeHTTPCfg(url string) hooks.HookConfig {
 }
 
 func TestHTTP_200Allow(t *testing.T) {
+	security.SetAllowLoopbackForTest(true)
+	defer security.SetAllowLoopbackForTest(false)
+
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusOK)
 		// empty body → allow
@@ -40,6 +44,9 @@ func TestHTTP_200Allow(t *testing.T) {
 }
 
 func TestHTTP_200BlockDecision(t *testing.T) {
+	security.SetAllowLoopbackForTest(true)
+	defer security.SetAllowLoopbackForTest(false)
+
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Content-Type", "application/json")
 		w.WriteHeader(http.StatusOK)
@@ -58,6 +65,9 @@ func TestHTTP_200BlockDecision(t *testing.T) {
 }
 
 func TestHTTP_200ContinueFalse(t *testing.T) {
+	security.SetAllowLoopbackForTest(true)
+	defer security.SetAllowLoopbackForTest(false)
+
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Content-Type", "application/json")
 		w.WriteHeader(http.StatusOK)
@@ -76,6 +86,9 @@ func TestHTTP_200ContinueFalse(t *testing.T) {
 }
 
 func TestHTTP_5xxRetriesOnce(t *testing.T) {
+	security.SetAllowLoopbackForTest(true)
+	defer security.SetAllowLoopbackForTest(false)
+
 	var calls atomic.Int32
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		n := calls.Add(1)
@@ -102,6 +115,9 @@ func TestHTTP_5xxRetriesOnce(t *testing.T) {
 }
 
 func TestHTTP_4xxReturnsError(t *testing.T) {
+	security.SetAllowLoopbackForTest(true)
+	defer security.SetAllowLoopbackForTest(false)
+
 	// The Execute method retries once on any doRequest error (including 4xx).
 	// Primary assertion: the final decision is DecisionError regardless of retry.
 	var calls atomic.Int32
@@ -143,6 +159,9 @@ func TestHTTP_MissingURL(t *testing.T) {
 }
 
 func TestHTTP_NonJSON2xx_TreatedAsAllow(t *testing.T) {
+	security.SetAllowLoopbackForTest(true)
+	defer security.SetAllowLoopbackForTest(false)
+
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusOK)
 		w.Write([]byte("OK"))
@@ -160,6 +179,9 @@ func TestHTTP_NonJSON2xx_TreatedAsAllow(t *testing.T) {
 }
 
 func TestHTTP_EncryptedAuthHeader_Decrypted(t *testing.T) {
+	security.SetAllowLoopbackForTest(true)
+	defer security.SetAllowLoopbackForTest(false)
+
 	// 32-byte raw key (hex-encoded = 64 chars accepted by DeriveKey).
 	const key = "0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f20"
 
@@ -204,6 +226,9 @@ func TestHTTP_EncryptedAuthHeader_Decrypted(t *testing.T) {
 }
 
 func TestHTTP_ResponseBodyCappedAt1MiB(t *testing.T) {
+	security.SetAllowLoopbackForTest(true)
+	defer security.SetAllowLoopbackForTest(false)
+
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusOK)
 		// Write 2 MiB of 'x' — handler must cap at 1 MiB and not panic.

internal/security/ssrf.go

@@ -0,0 +1,213 @@
+// Package security provides SSRF-safe HTTP utilities for outbound webhook calls.
+// All production webhook HTTP clients MUST use NewSafeClient to prevent
+// admin-configured hooks from probing internal infrastructure.
+package security
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"log/slog"
+	"net"
+	"net/http"
+	"net/url"
+	"time"
+)
+
+// pinnedIPKey is a context key used to pass the pre-resolved IP from
+// Validate into the custom DialContext, preventing DNS rebinding attacks.
+type pinnedIPKey struct{}
+
+// allowLoopbackForTest is a test-only bypass. Production code MUST never set
+// this variable. It is exported via SetAllowLoopbackForTest exclusively for
+// use in *_test.go files to permit httptest.NewServer addresses (127.0.0.1).
+var allowLoopbackForTest bool
+
+// SetAllowLoopbackForTest enables or disables the loopback/private-CIDR bypass
+// for tests. Call with true before tests that use httptest.NewServer, and
+// always defer a call with false to restore the default.
+//
+// This function MUST only be called from test code. Production paths never
+// set this flag — the zero value (false) is the safe default.
+func SetAllowLoopbackForTest(allow bool) {
+	allowLoopbackForTest = allow
+}
+
+// blockedCIDRs lists all CIDRs that must never be dialed.
+var blockedCIDRs []*net.IPNet
+
+func init() {
+	cidrs := []string{
+		// Loopback
+		"127.0.0.0/8",
+		"::1/128",
+		// Link-local (includes cloud-metadata 169.254.169.254)
+		"169.254.0.0/16",
+		"fe80::/10",
+		// Private (RFC 1918 + RFC 4193)
+		"10.0.0.0/8",
+		"172.16.0.0/12",
+		"192.168.0.0/16",
+		"fc00::/7",
+		// Multicast
+		"224.0.0.0/4",
+		"ff00::/8",
+		// Unspecified
+		"0.0.0.0/32",
+		"::/128",
+	}
+	for _, cidr := range cidrs {
+		_, ipNet, err := net.ParseCIDR(cidr)
+		if err != nil {
+			panic(fmt.Sprintf("security: bad CIDR %q: %v", cidr, err))
+		}
+		blockedCIDRs = append(blockedCIDRs, ipNet)
+	}
+}
+
+// isBlocked returns true if ip falls within any blocked CIDR.
+func isBlocked(ip net.IP) bool {
+	for _, cidr := range blockedCIDRs {
+		if cidr.Contains(ip) {
+			return true
+		}
+	}
+	return false
+}
+
+// redactURL strips query string and userinfo for safe logging.
+func redactURL(rawURL string) string {
+	u, err := url.Parse(rawURL)
+	if err != nil {
+		return "[unparseable]"
+	}
+	u.RawQuery = ""
+	u.Fragment = ""
+	u.User = nil
+	return u.String()
+}
+
+// Validate parses rawURL, resolves the host once, and rejects loopback,
+// link-local, private, multicast, unspecified, and cloud-metadata destinations.
+// Returns the parsed URL and the pinned resolved IP that the caller should dial
+// directly. Only http and https schemes are accepted.
+//
+// In test code, call SetAllowLoopbackForTest(true) before invoking this
+// function to permit httptest.NewServer addresses (127.0.0.1).
+func Validate(rawURL string) (*url.URL, net.IP, error) {
+	return validate(rawURL, allowLoopbackForTest)
+}
+
+// validate is the internal implementation; allowLoopback is set only in tests.
+func validate(rawURL string, allowLoopback bool) (*url.URL, net.IP, error) {
+	redacted := redactURL(rawURL)
+
+	u, err := url.Parse(rawURL)
+	if err != nil {
+		slog.Warn("security.hook.ssrf_block", "url", redacted, "reason", "url_parse_error")
+		return nil, nil, fmt.Errorf("ssrf: parse url: %w", err)
+	}
+
+	if u.Scheme != "http" && u.Scheme != "https" {
+		slog.Warn("security.hook.ssrf_block", "url", redacted, "reason", "non_http_scheme", "scheme", u.Scheme)
+		return nil, nil, fmt.Errorf("ssrf: scheme %q not allowed (only http/https)", u.Scheme)
+	}
+
+	host := u.Hostname()
+	if host == "" {
+		slog.Warn("security.hook.ssrf_block", "url", redacted, "reason", "empty_host")
+		return nil, nil, errors.New("ssrf: empty host")
+	}
+
+	// If the host is already a literal IP, validate it directly.
+	if ip := net.ParseIP(host); ip != nil {
+		if !allowLoopback && isBlocked(ip) {
+			slog.Warn("security.hook.ssrf_block", "url", redacted, "reason", "blocked_ip", "ip", ip.String())
+			return nil, nil, fmt.Errorf("ssrf: IP %s is in a blocked range", ip)
+		}
+		return u, ip, nil
+	}
+
+	// DNS resolution — pin the first returned IP.
+	addrs, err := net.LookupHost(host)
+	if err != nil {
+		slog.Warn("security.hook.ssrf_block", "url", redacted, "reason", "dns_resolve_failed", "host", host)
+		return nil, nil, fmt.Errorf("ssrf: resolve %q: %w", host, err)
+	}
+	if len(addrs) == 0 {
+		slog.Warn("security.hook.ssrf_block", "url", redacted, "reason", "no_ips_resolved", "host", host)
+		return nil, nil, fmt.Errorf("ssrf: %q resolved to no addresses", host)
+	}
+
+	ip := net.ParseIP(addrs[0])
+	if ip == nil {
+		return nil, nil, fmt.Errorf("ssrf: resolved address %q is not a valid IP", addrs[0])
+	}
+
+	if !allowLoopback && isBlocked(ip) {
+		slog.Warn("security.hook.ssrf_block", "url", redacted, "reason", "blocked_resolved_ip", "host", host, "ip", ip.String())
+		return nil, nil, fmt.Errorf("ssrf: %q resolved to blocked IP %s", host, ip)
+	}
+
+	return u, ip, nil
+}
+
+// WithPinnedIP stores the pinned IP in ctx so the safe DialContext can read it.
+func WithPinnedIP(ctx context.Context, ip net.IP) context.Context {
+	return context.WithValue(ctx, pinnedIPKey{}, ip)
+}
+
+// pinnedIPFrom retrieves the pinned IP from ctx. Returns nil if not set.
+func pinnedIPFrom(ctx context.Context) net.IP {
+	ip, _ := ctx.Value(pinnedIPKey{}).(net.IP)
+	return ip
+}
+
+// NewSafeClient returns an *http.Client whose Transport.DialContext pins the
+// destination to the IP stored in the request context via WithPinnedIP (so DNS
+// rebinding cannot swap a public IP for a private one between Validate and Dial),
+// refuses redirects, and applies the supplied per-request timeout.
+// The returned client is safe to share across goroutines.
+//
+// Caller workflow:
+//  1. Call Validate(url) → get pinnedIP
+//  2. ctx = WithPinnedIP(ctx, pinnedIP)
+//  3. http.NewRequestWithContext(ctx, ...)
+//  4. client.Do(req)
+func NewSafeClient(timeout time.Duration) *http.Client {
+	dialer := &net.Dialer{Timeout: timeout}
+
+	transport := &http.Transport{
+		DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) {
+			pinnedIP := pinnedIPFrom(ctx)
+			if pinnedIP == nil {
+				// No pinned IP in context — reject for safety.
+				return nil, errors.New("ssrf: no pinned IP in context; call security.WithPinnedIP before dialing")
+			}
+
+			// Defense-in-depth: re-check pinned IP against block list.
+			// allowLoopbackForTest bypasses this check in test code only.
+			if !allowLoopbackForTest && isBlocked(pinnedIP) {
+				return nil, fmt.Errorf("ssrf: pinned IP %s is in a blocked range", pinnedIP)
+			}
+
+			// Replace the host portion of addr with the pinned IP,
+			// preserving the port from addr.
+			_, port, err := net.SplitHostPort(addr)
+			if err != nil {
+				return nil, fmt.Errorf("ssrf: split host/port from %q: %w", addr, err)
+			}
+			pinnedAddr := net.JoinHostPort(pinnedIP.String(), port)
+			return dialer.DialContext(ctx, network, pinnedAddr)
+		},
+	}
+
+	return &http.Client{
+		Timeout:   timeout,
+		Transport: transport,
+		CheckRedirect: func(req *http.Request, via []*http.Request) error {
+			// Never follow redirects — return the redirect response directly.
+			return http.ErrUseLastResponse
+		},
+	}
+}

internal/security/ssrf_test.go

@@ -0,0 +1,180 @@
+package security
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+)
+
+// ---- Validate reject tests ----
+
+func TestValidate_RejectsLoopbackIPv4(t *testing.T) {
+	_, _, err := Validate("http://127.0.0.1/x")
+	if err == nil {
+		t.Fatal("expected error for loopback IPv4, got nil")
+	}
+}
+
+func TestValidate_RejectsLoopbackIPv6(t *testing.T) {
+	_, _, err := Validate("http://[::1]/x")
+	if err == nil {
+		t.Fatal("expected error for loopback IPv6, got nil")
+	}
+}
+
+func TestValidate_RejectsLinkLocal(t *testing.T) {
+	_, _, err := Validate("http://169.254.169.254/latest/meta-data")
+	if err == nil {
+		t.Fatal("expected error for cloud-metadata link-local address, got nil")
+	}
+}
+
+func TestValidate_RejectsRFC1918(t *testing.T) {
+	cases := []string{
+		"http://10.1.2.3/",
+		"http://172.16.0.1/",
+		"http://192.168.0.1/",
+	}
+	for _, rawURL := range cases {
+		t.Run(rawURL, func(t *testing.T) {
+			_, _, err := Validate(rawURL)
+			if err == nil {
+				t.Fatalf("expected SSRF error for %q, got nil", rawURL)
+			}
+		})
+	}
+}
+
+func TestValidate_RejectsMulticast(t *testing.T) {
+	_, _, err := Validate("http://224.0.0.1/")
+	if err == nil {
+		t.Fatal("expected error for multicast address, got nil")
+	}
+}
+
+func TestValidate_RejectsUnspecified(t *testing.T) {
+	_, _, err := Validate("http://0.0.0.0/")
+	if err == nil {
+		t.Fatal("expected error for unspecified address, got nil")
+	}
+}
+
+func TestValidate_RejectsNonHTTPScheme(t *testing.T) {
+	cases := []string{
+		"file:///etc/passwd",
+		"gopher://x",
+		"ftp://x",
+	}
+	for _, rawURL := range cases {
+		t.Run(rawURL, func(t *testing.T) {
+			_, _, err := Validate(rawURL)
+			if err == nil {
+				t.Fatalf("expected error for scheme in %q, got nil", rawURL)
+			}
+		})
+	}
+}
+
+func TestValidate_RejectsHostThatResolvesToBlockedIP(t *testing.T) {
+	// "localhost." (with trailing dot) forces a DNS lookup that resolves to 127.0.0.1
+	// on virtually all systems. If it somehow doesn't resolve, the test still passes
+	// because an unresolvable host is also rejected.
+	_, _, err := Validate("http://localhost./")
+	if err == nil {
+		t.Fatal("expected error: localhost resolves to loopback, got nil")
+	}
+}
+
+func TestValidate_AcceptsPublicLiteralIP(t *testing.T) {
+	// 93.184.216.34 is the well-known example.com IP — a stable public address.
+	// Use a literal IP to avoid DNS flakiness in CI.
+	_, ip, err := Validate("http://93.184.216.34/")
+	if err != nil {
+		t.Fatalf("expected public IP to be accepted, got error: %v", err)
+	}
+	if ip == nil {
+		t.Fatal("expected non-nil pinned IP")
+	}
+}
+
+// ---- NewSafeClient tests ----
+
+func TestNewSafeClient_NoRedirects(t *testing.T) {
+	// Server that always redirects to /dest.
+	SetAllowLoopbackForTest(true)
+	defer SetAllowLoopbackForTest(false)
+
+	redirectTarget := ""
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path == "/" {
+			http.Redirect(w, r, redirectTarget+"/dest", http.StatusFound)
+			return
+		}
+		// /dest endpoint — should never be reached.
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer srv.Close()
+	redirectTarget = srv.URL
+
+	_, pinnedIP, err := validate(srv.URL+"/", true)
+	if err != nil {
+		t.Fatalf("validate: %v", err)
+	}
+
+	client := NewSafeClient(5e9) // 5s
+
+	req, err := http.NewRequest(http.MethodGet, srv.URL+"/", nil)
+	if err != nil {
+		t.Fatalf("build request: %v", err)
+	}
+	req = req.WithContext(WithPinnedIP(req.Context(), pinnedIP))
+
+	resp, err := client.Do(req)
+	if err != nil {
+		t.Fatalf("do request: %v", err)
+	}
+	defer resp.Body.Close()
+
+	// Must receive the 302, not follow it.
+	if resp.StatusCode != http.StatusFound {
+		t.Errorf("status=%d, want 302 (redirect not followed)", resp.StatusCode)
+	}
+}
+
+func TestNewSafeClient_DialPinned(t *testing.T) {
+	// httptest.NewServer binds to 127.0.0.1. Validate (without loopback bypass)
+	// must reject it, proving the pre-dial check is active on the real path.
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer srv.Close()
+
+	_, _, err := Validate(srv.URL + "/")
+	if err == nil {
+		t.Fatal("expected SSRF block for httptest server (127.0.0.1), got nil")
+	}
+}
+
+func TestNewSafeClient_RejectsNoPinnedIP(t *testing.T) {
+	// If the caller forgot to call WithPinnedIP, the DialContext must reject.
+	SetAllowLoopbackForTest(true)
+	defer SetAllowLoopbackForTest(false)
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer srv.Close()
+
+	client := NewSafeClient(5e9)
+
+	// Build request WITHOUT pinned IP in context.
+	req, err := http.NewRequest(http.MethodGet, srv.URL+"/", nil)
+	if err != nil {
+		t.Fatalf("build request: %v", err)
+	}
+
+	_, err = client.Do(req)
+	if err == nil {
+		t.Fatal("expected error when no pinned IP in context, got nil")
+	}
+}