test: Add secure webhook client tests for redirect handling

Tests SSRF protection in webhook client:
- Redirect limit enforcement (max 3)
- Blocking redirects to private networks (10.x, 192.168.x, 172.16.x)
- Blocking redirects to link-local addresses (169.254.x)
- Allowing valid redirects to allowlisted servers

Coverage: createSecureWebhookClient 18.2% -> 100%

internal/notifications/webhook_client_test.go

@@ -0,0 +1,186 @@
+package notifications
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+)
+
+// createTestNotificationManager creates a NotificationManager with localhost allowlisted
+// for testing purposes (since httptest servers run on localhost).
+func createTestNotificationManager(t *testing.T) *NotificationManager {
+	t.Helper()
+	nm := &NotificationManager{
+		lastNotified:      make(map[string]notificationRecord),
+		webhookRateLimits: make(map[string]*webhookRateLimit),
+	}
+	// Allowlist localhost for test servers
+	if err := nm.UpdateAllowedPrivateCIDRs("127.0.0.0/8"); err != nil {
+		t.Fatalf("failed to allowlist localhost: %v", err)
+	}
+	return nm
+}
+
+// TestSecureWebhookClientRedirectLimit verifies that the client stops following
+// redirects after exceeding WebhookMaxRedirects.
+func TestSecureWebhookClientRedirectLimit(t *testing.T) {
+	redirectCount := 0
+	// Create a server that always redirects to itself
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		redirectCount++
+		http.Redirect(w, r, r.URL.String()+"x", http.StatusFound)
+	}))
+	defer server.Close()
+
+	nm := createTestNotificationManager(t)
+
+	client := nm.createSecureWebhookClient(WebhookTimeout)
+	_, err := client.Get(server.URL)
+
+	if err == nil {
+		t.Fatal("expected error from too many redirects, got nil")
+	}
+	if !strings.Contains(err.Error(), "stopped after") {
+		t.Errorf("expected 'stopped after' error, got: %v", err)
+	}
+	// CheckRedirect is called when len(via) >= WebhookMaxRedirects.
+	// via includes previous requests, so with WebhookMaxRedirects=3:
+	// - Initial request (not in via yet)
+	// - Redirect 1: via has 1 entry (initial) - allowed
+	// - Redirect 2: via has 2 entries - allowed
+	// - Redirect 3: via has 3 entries - blocked (len(via) >= 3)
+	// Total server hits: initial + 2 successful redirects = 3
+	expectedRequests := WebhookMaxRedirects
+	if redirectCount != expectedRequests {
+		t.Errorf("expected %d requests, got %d", expectedRequests, redirectCount)
+	}
+}
+
+// TestSecureWebhookClientBlocksUnsafeRedirect verifies that redirects to
+// localhost/private networks are blocked when not in allowlist.
+func TestSecureWebhookClientBlocksUnsafeRedirect(t *testing.T) {
+	// Create a server that redirects to localhost (allowlist 127.0.0.1 only, not 127.0.0.2)
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// Redirect to a different localhost IP that's not in our allowlist
+		http.Redirect(w, r, "http://127.0.0.2:8080/evil", http.StatusFound)
+	}))
+	defer server.Close()
+
+	// Create manager with narrow allowlist (only the test server's IP)
+	nm := &NotificationManager{
+		lastNotified:      make(map[string]notificationRecord),
+		webhookRateLimits: make(map[string]*webhookRateLimit),
+	}
+	// Allowlist only 127.0.0.1/32, so 127.0.0.2 will be blocked
+	if err := nm.UpdateAllowedPrivateCIDRs("127.0.0.1/32"); err != nil {
+		t.Fatalf("failed to set allowlist: %v", err)
+	}
+
+	client := nm.createSecureWebhookClient(WebhookTimeout)
+	_, err := client.Get(server.URL)
+
+	if err == nil {
+		t.Fatal("expected error from unsafe redirect, got nil")
+	}
+	if !strings.Contains(err.Error(), "redirect to unsafe URL blocked") {
+		t.Errorf("expected 'redirect to unsafe URL blocked' error, got: %v", err)
+	}
+}
+
+// TestSecureWebhookClientBlocksPrivateNetworkRedirect verifies that redirects to
+// private network IPs (10.x, 192.168.x, 172.16-31.x) are blocked.
+func TestSecureWebhookClientBlocksPrivateNetworkRedirect(t *testing.T) {
+	privateIPs := []string{
+		"http://10.0.0.1/webhook",
+		"http://192.168.1.1/webhook",
+		"http://172.16.0.1/webhook",
+	}
+
+	for _, privateURL := range privateIPs {
+		t.Run(privateURL, func(t *testing.T) {
+			server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				http.Redirect(w, r, privateURL, http.StatusFound)
+			}))
+			defer server.Close()
+
+			// Allowlist localhost for test server, but not private network ranges
+			nm := &NotificationManager{
+				lastNotified:      make(map[string]notificationRecord),
+				webhookRateLimits: make(map[string]*webhookRateLimit),
+			}
+			if err := nm.UpdateAllowedPrivateCIDRs("127.0.0.0/8"); err != nil {
+				t.Fatalf("failed to set allowlist: %v", err)
+			}
+
+			client := nm.createSecureWebhookClient(WebhookTimeout)
+			_, err := client.Get(server.URL)
+
+			if err == nil {
+				t.Fatalf("expected error from redirect to %s, got nil", privateURL)
+			}
+			if !strings.Contains(err.Error(), "redirect to unsafe URL blocked") {
+				t.Errorf("expected 'redirect to unsafe URL blocked' error for %s, got: %v", privateURL, err)
+			}
+		})
+	}
+}
+
+// TestSecureWebhookClientAllowsValidRedirects verifies that valid redirects
+// to safe URLs are followed successfully.
+func TestSecureWebhookClientAllowsValidRedirects(t *testing.T) {
+	// Create a chain of servers for redirect testing
+	// Final server returns 200 OK
+	finalServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+		w.Write([]byte("success"))
+	}))
+	defer finalServer.Close()
+
+	// Middle server redirects to final
+	middleServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		http.Redirect(w, r, finalServer.URL, http.StatusFound)
+	}))
+	defer middleServer.Close()
+
+	// First server redirects to middle
+	firstServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		http.Redirect(w, r, middleServer.URL, http.StatusFound)
+	}))
+	defer firstServer.Close()
+
+	nm := createTestNotificationManager(t)
+
+	client := nm.createSecureWebhookClient(WebhookTimeout)
+	resp, err := client.Get(firstServer.URL)
+
+	if err != nil {
+		t.Fatalf("expected successful redirect chain, got error: %v", err)
+	}
+	defer resp.Body.Close()
+
+	if resp.StatusCode != http.StatusOK {
+		t.Errorf("expected status 200, got %d", resp.StatusCode)
+	}
+}
+
+// TestSecureWebhookClientBlocksLinkLocalRedirect verifies that redirects to
+// link-local addresses (169.254.x.x) are blocked.
+func TestSecureWebhookClientBlocksLinkLocalRedirect(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		http.Redirect(w, r, "http://169.254.169.254/latest/meta-data/", http.StatusFound)
+	}))
+	defer server.Close()
+
+	nm := createTestNotificationManager(t)
+
+	client := nm.createSecureWebhookClient(WebhookTimeout)
+	_, err := client.Get(server.URL)
+
+	if err == nil {
+		t.Fatal("expected error from redirect to link-local/metadata address, got nil")
+	}
+	if !strings.Contains(err.Error(), "redirect to unsafe URL blocked") {
+		t.Errorf("expected 'redirect to unsafe URL blocked' error, got: %v", err)
+	}
+}