Keep the SSRF-safe transport in clientForConfig (closes #69) (#74)

`clientForConfig()` in `internal/delivery/engine.go` built a fresh `http.Client` without a Transport when a per-target timeout was configured, dropping the request-time private-IP guard for that path.

It now reuses the shared client's SSRF-safe transport (`e.client.Transport`, the same `NewSSRFSafeTransport` instance), overriding only the `Timeout`. Behaviour is unchanged when no per-target timeout is set (the shared client is returned as before), so no engine code path makes an outbound target request with a client lacking the SSRF-safe transport.

Adds a delivery-package test proving a client from `clientForConfig()` with a per-target timeout still refuses private/reserved/link-local destinations, that the timeout is applied, that the SSRF-safe transport is reused (not duplicated), and that the no-timeout path returns the shared client unchanged.

Confined to `internal/delivery/` only; handlers and server code untouched.

Closes #69

Co-authored-by: sneak <sneak@sneak.berlin>
Co-authored-by: Jeffrey Paul <sneak@noreply.example.org>
Reviewed-on: #74
Co-authored-by: clawbot <clawbot@noreply.example.org>
Co-committed-by: clawbot <clawbot@noreply.example.org>

internal/delivery/client_ssrf_test.go

@@ -0,0 +1,112 @@
+package delivery_test
+
+import (
+	"context"
+	"log/slog"
+	"net/http"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/webhooker/internal/delivery"
+)
+
+// newSSRFTestEngine builds an Engine whose shared client
+// carries the SSRF-safe transport, mirroring production.
+func newSSRFTestEngine() *delivery.Engine {
+	log := slog.New(slog.DiscardHandler)
+
+	client := &http.Client{
+		Timeout:   30 * time.Second,
+		Transport: delivery.NewSSRFSafeTransport(),
+	}
+
+	return delivery.NewTestEngine(log, client, 1)
+}
+
+// TestClientForConfig_TimeoutKeepsSSRFGuard asserts that a
+// client returned by clientForConfig for a config with a
+// per-target timeout still refuses connections to
+// private/reserved addresses (the timeout must not drop the
+// SSRF-safe transport).
+func TestClientForConfig_TimeoutKeepsSSRFGuard(t *testing.T) {
+	t.Parallel()
+
+	engine := newSSRFTestEngine()
+
+	blocked := []string{
+		"http://127.0.0.1/hook",
+		"http://169.254.169.254/latest/meta-data/",
+		"http://[fe80::1]/hook",
+	}
+
+	for _, target := range blocked {
+		t.Run(target, func(t *testing.T) {
+			t.Parallel()
+
+			cfg := &delivery.HTTPTargetConfig{
+				URL:     target,
+				Timeout: 5,
+			}
+
+			client := engine.ExportClientForConfig(cfg)
+
+			require.NotSame(t, engine.ExportClient(), client,
+				"a per-target timeout must yield a "+
+					"distinct client",
+			)
+
+			assert.Equal(t,
+				5*time.Second, client.Timeout,
+				"the per-target timeout must be applied",
+			)
+
+			assert.Same(t,
+				engine.ExportClient().Transport,
+				client.Transport,
+				"the SSRF-safe transport must be reused, "+
+					"not dropped",
+			)
+
+			req, err := http.NewRequestWithContext(
+				context.Background(),
+				http.MethodPost, target, nil,
+			)
+			require.NoError(t, err)
+
+			resp, doErr := client.Do(req)
+			if resp != nil {
+				_ = resp.Body.Close()
+			}
+
+			require.Error(t, doErr,
+				"request to %s must be blocked", target,
+			)
+
+			assert.Contains(t, doErr.Error(), "blocked",
+				"error must come from the SSRF guard",
+			)
+		})
+	}
+}
+
+// TestClientForConfig_NoTimeoutUnchanged asserts that with
+// no per-target timeout the shared SSRF-safe client is
+// returned unchanged.
+func TestClientForConfig_NoTimeoutUnchanged(t *testing.T) {
+	t.Parallel()
+
+	engine := newSSRFTestEngine()
+
+	cfg := &delivery.HTTPTargetConfig{
+		URL: "https://example.com/hook",
+	}
+
+	client := engine.ExportClientForConfig(cfg)
+
+	assert.Same(t, engine.ExportClient(), client,
+		"without a per-target timeout the shared client "+
+			"must be returned unchanged",
+	)
+}

internal/delivery/engine.go

@@ -1713,10 +1713,15 @@ func (e *Engine) clientForConfig(
 	cfg *HTTPTargetConfig,
 ) *http.Client {
 	if cfg.Timeout > 0 {
+		// Reuse the shared client's SSRF-safe transport so
+		// a per-target timeout does not drop the
+		// request-time private-IP guard. Only the timeout
+		// is overridden.
 		return &http.Client{
 			Timeout: time.Duration(
 				cfg.Timeout,
 			) * time.Second,
+			Transport: e.client.Transport,
 		}
 	}
 

internal/delivery/export_test.go

@@ -126,6 +126,18 @@ func (e *Engine) ExportDoHTTPRequest(
 	return e.doHTTPRequest(ctx, cfg, event)
 }
 
+// ExportClientForConfig exposes clientForConfig.
+func (e *Engine) ExportClientForConfig(
+	cfg *HTTPTargetConfig,
+) *http.Client {
+	return e.clientForConfig(cfg)
+}
+
+// ExportClient returns the engine's shared HTTP client.
+func (e *Engine) ExportClient() *http.Client {
+	return e.client
+}
+
 // ExportScheduleRetry exposes scheduleRetry.
 func (e *Engine) ExportScheduleRetry(
 	task Task, delay time.Duration,