Gate forwarded-header trust behind trusted-proxy config (closes #88)

Every rate limiter keyed on httprate.KeyByRealIP, which believes
True-Client-IP, X-Real-IP and the first X-Forwarded-For entry from
any peer. A client could therefore mint a fresh bucket per request
by rotating a spoofed header, or drain another client's bucket by
claiming its address, which left the receiver, login and password
change limits with no value against a deliberate attacker.

The receiver, login and password change limiters now share one key
function: the connection's own address, unless the direct peer is
inside a network listed in the new TRUSTED_PROXIES CIDR list, in
which case the forwarded client address is used. The list is empty
by default, so nothing is trusted until an operator names their
proxy; a set-but-unparseable value aborts startup, matching the
handling of the other parsed variables. Within a trusted request
X-Forwarded-For is walked right to left and the first hop that is
not itself a trusted proxy wins, so client-prepended entries cannot
be selected.

Also folds in two cleanups from the same review: the 429 responder
shared by all three limiters is extracted, and the
RECEIVER_RATE_LIMIT error-path tests now assert that the failure
names the variable and wraps ErrNonPositiveValue rather than only
that some error occurred.

README.md

@@ -95,6 +95,28 @@ TTY detection, and security headers are always applied.
 | `SENTRY_DSN`            | Sentry error reporting DSN          | `""`     |
 | `SESSION_IDLE_TIMEOUT`  | Idle session timeout (Go duration)  | `24h`    |
 | `RECEIVER_RATE_LIMIT`   | Receiver requests/minute per IP per entrypoint | `120` |
+| `TRUSTED_PROXIES`       | CIDRs whose forwarded headers are trusted | `""` (none) |
+
+#### Trusted proxies
+
+`TRUSTED_PROXIES` is a comma-separated list of CIDR blocks (a bare
+address such as `10.0.0.1` is accepted and treated as a single host),
+for example `10.0.0.0/8, 192.168.1.7, 2001:db8::/32`. It decides whose
+`X-Forwarded-For`, `X-Real-IP`, and `True-Client-IP` headers the rate
+limiters believe.
+
+Forwarded headers are honoured **only** when the connecting peer is
+inside one of these blocks; for every other peer the client identity is
+the connection's own address and the headers are ignored. The default
+is the empty list, which trusts nobody — anything else would let any
+client pick its own rate limit bucket, minting a fresh one per request
+or draining someone else's. Set it to the address of your reverse
+proxy, and to nothing wider.
+
+Within a trusted request, `X-Forwarded-For` is read right to left and
+the first hop that is not itself a trusted proxy wins, so entries a
+client prepended before reaching the proxy cannot be selected. A set
+but unparseable value aborts startup.
 
 Sessions are bounded by two independent clocks, and end at whichever
 one runs out first:
@@ -124,8 +146,9 @@ fatal configuration error: webhooker logs the offending variable and
 its value and refuses to start, rather than silently running with a
 substituted default. `PORT=eighty`, `DEBUG=ture`, and
 `RETENTION_SWEEP_INTERVAL=1 hour` all abort startup. `PORT` must
-additionally be a number in the range 1–65535, and
+additionally be a number in the range 1–65535,
-`RECEIVER_RATE_LIMIT` must be at least 1.
+`RECEIVER_RATE_LIMIT` must be at least 1, and every entry in
+`TRUSTED_PROXIES` must be a CIDR block or a bare IP address.
 
 Boolean variables (`DEBUG`, `MAINTENANCE_MODE`) accept exactly the
 spellings Go's `strconv.ParseBool` accepts — `1`, `t`, `T`, `TRUE`,
@@ -802,6 +825,16 @@ legitimate webhook senders). Requests over the limit receive HTTP 429
 with a `Retry-After` header. A set-but-invalid `RECEIVER_RATE_LIMIT`
 value aborts startup rather than silently falling back to the default.
 
+Every limiter here — receiver, login, and password change — identifies
+the client the same way, through one shared key function: the
+connection's own address, unless the peer is listed in
+`TRUSTED_PROXIES`, in which case the forwarded client address is used
+instead. See [Trusted proxies](#trusted-proxies). Deployed without that
+variable set, a client behind a reverse proxy shares one bucket with
+every other client behind the same proxy, which is the safe direction
+to be wrong in: set `TRUSTED_PROXIES` to the proxy's address to get
+per-client limits back.
+
 Finer-grained per-webhook rate limits (configured in the web UI and
 enforced in the webhook handler) can layer on top of this env-level
 abuse limit later; they are tracked as future work.

TODO.md

@@ -26,10 +26,6 @@ capability in the README rationale).
 
 # Completed Steps
 
-- 2026-08-11 Web UI cleanup: nav terminology unified on Webhooks, the
-  Profile settings placeholder removed, a progressive-enhancement copy
-  button for the entrypoint URL, and retention form copy that states the
-  actual policy (deletion by the reaper, 0 retains forever) (#57)
 - 2026-08-09 Inactivity-based session timeout: sliding idle expiry
   (`SESSION_IDLE_TIMEOUT`, default `24h`) refreshed on authenticated
   requests, with the 7-day absolute cap kept as an independent

internal/config/config.go

@@ -5,8 +5,10 @@ import (
 	"errors"
 	"fmt"
 	"log/slog"
+	"net/netip"
 	"os"
 	"strconv"
+	"strings"
 	"time"
 
 	"go.uber.org/fx"
@@ -59,6 +61,11 @@ var ErrNonPositiveValue = errors.New("value must be positive")
 // TCP port number is set above the valid port range.
 var ErrInvalidPort = errors.New("invalid port")
 
+// ErrInvalidCIDR is returned when an environment variable holding a
+// list of CIDR blocks contains an entry that is neither a CIDR block
+// nor a bare IP address.
+var ErrInvalidCIDR = errors.New("invalid CIDR")
+
 //nolint:revive // ConfigParams is a standard fx naming convention.
 type ConfigParams struct {
 	fx.In
@@ -90,6 +97,14 @@ type Config struct {
 	// client IP may send to a single webhook receiver entrypoint.
 	ReceiverRateLimit int
 
+	// TrustedProxies is the set of networks whose members are
+	// allowed to speak for the client with forwarded headers
+	// (X-Forwarded-For, X-Real-IP, True-Client-IP). It is empty
+	// unless TRUSTED_PROXIES is set, and empty means no peer is
+	// trusted: forwarded headers are then ignored entirely and
+	// clients are identified by the connection's own address.
+	TrustedProxies []netip.Prefix
+
 	params *ConfigParams
 	log    *slog.Logger
 }
@@ -212,6 +227,60 @@ func envDuration(
 	return d, nil
 }
 
+// parseCIDR parses one trusted-proxy list entry, which may be a
+// CIDR block ("10.0.0.0/8") or a bare address ("10.0.0.1", treated
+// as a single-host block).
+func parseCIDR(entry string) (netip.Prefix, error) {
+	if strings.Contains(entry, "/") {
+		prefix, err := netip.ParsePrefix(entry)
+		if err != nil {
+			return netip.Prefix{}, err //nolint:wrapcheck // wrapped by caller
+		}
+
+		return prefix.Masked(), nil
+	}
+
+	addr, err := netip.ParseAddr(entry)
+	if err != nil {
+		return netip.Prefix{}, err //nolint:wrapcheck // wrapped by caller
+	}
+
+	return netip.PrefixFrom(addr.Unmap(), addr.Unmap().BitLen()), nil
+}
+
+// envPrefixList returns the value of the named environment variable
+// parsed as a comma-separated list of CIDR blocks (bare addresses
+// allowed). An unset, empty, or blank value yields an empty list. A
+// set value containing an unparseable entry is a hard error naming
+// the key and the bad entry, so startup fails loudly rather than
+// silently running with a list the operator did not intend.
+func envPrefixList(key string) ([]netip.Prefix, error) {
+	v := strings.TrimSpace(os.Getenv(key))
+	if v == "" {
+		return nil, nil
+	}
+
+	var prefixes []netip.Prefix
+
+	for entry := range strings.SplitSeq(v, ",") {
+		entry = strings.TrimSpace(entry)
+		if entry == "" {
+			continue
+		}
+
+		prefix, err := parseCIDR(entry)
+		if err != nil {
+			return nil, fmt.Errorf(
+				"%w: %s: %q: %w", ErrInvalidCIDR, key, entry, err,
+			)
+		}
+
+		prefixes = append(prefixes, prefix)
+	}
+
+	return prefixes, nil
+}
+
 // resolveEnvironment reads WEBHOOKER_ENVIRONMENT, defaulting to
 // dev, and rejects unrecognised values.
 func resolveEnvironment() (string, error) {
@@ -282,6 +351,11 @@ func loadFromEnv() (*Config, error) {
 		return nil, err
 	}
 
+	trustedProxies, err := envPrefixList("TRUSTED_PROXIES")
+	if err != nil {
+		return nil, err
+	}
+
 	return &Config{
 		DataDir:                envString("DATA_DIR"),
 		Debug:                  debug,
@@ -294,6 +368,7 @@ func loadFromEnv() (*Config, error) {
 		RetentionSweepInterval: retentionSweepInterval,
 		SessionIdleTimeout:     sessionIdleTimeout,
 		ReceiverRateLimit:      receiverRateLimit,
+		TrustedProxies:         trustedProxies,
 	}, nil
 }
 
@@ -335,6 +410,7 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
 		"dataDir", s.DataDir,
 		"retentionSweepInterval", s.RetentionSweepInterval.String(),
 		"receiverRateLimit", s.ReceiverRateLimit,
+		"trustedProxies", len(s.TrustedProxies),
 		"hasSentryDSN", s.SentryDSN != "",
 		"hasMetricsAuth",
 		s.MetricsUsername != "" && s.MetricsPassword != "",

internal/config/config_test.go

@@ -179,9 +179,10 @@ func TestRetentionSweepInterval(t *testing.T) {
 	}
 }
 
-// expectStartupError asserts that fx refuses to build the app,
+// startupError builds the app config.New belongs to and returns
-// which is what a set-but-invalid environment value must cause.
+// the error fx reports, which is non-nil whenever an environment
-func expectStartupError(t *testing.T) {
+// value is set but invalid.
+func startupError(t *testing.T) error {
 	t.Helper()
 
 	var cfg *config.Config
@@ -196,7 +197,33 @@ func expectStartupError(t *testing.T) {
 		fx.Populate(&cfg),
 	)
 
-	assert.Error(t, app.Err())
+	return app.Err()
+}
+
+// expectStartupError asserts that fx refuses to build the app,
+// which is what a set-but-invalid environment value must cause.
+func expectStartupError(t *testing.T) {
+	t.Helper()
+
+	assert.Error(t, startupError(t))
+}
+
+// expectStartupErrorFor asserts that startup fails, that the error
+// names the offending variable so an operator can find it, and,
+// when sentinel is non-nil, that it wraps that sentinel.
+func expectStartupErrorFor(
+	t *testing.T,
+	key string,
+	sentinel error,
+) {
+	t.Helper()
+
+	err := startupError(t)
+	require.ErrorContains(t, err, key)
+
+	if sentinel != nil {
+		require.ErrorIs(t, err, sentinel)
+	}
 }
 
 func testRetentionSweepIntervalSuccess(
@@ -351,6 +378,10 @@ func TestReceiverRateLimit(t *testing.T) {
 		set         bool
 		value       string
 		expectError bool
+		// sentinel, when set, must be wrapped by the startup
+		// error; every error case must additionally name the
+		// variable in its message.
+		sentinel error
 		expected int
 	}{
 		{
@@ -375,12 +406,14 @@ func TestReceiverRateLimit(t *testing.T) {
 			set:         true,
 			value:       "0",
 			expectError: true,
+			sentinel:    config.ErrNonPositiveValue,
 		},
 		{
 			name:        "negative fails startup",
 			set:         true,
 			value:       "-5",
 			expectError: true,
+			sentinel:    config.ErrNonPositiveValue,
 		},
 	}
 
@@ -399,7 +432,9 @@ func TestReceiverRateLimit(t *testing.T) {
 			}
 
 			if tt.expectError {
-				expectStartupError(t)
+				expectStartupErrorFor(
+					t, "RECEIVER_RATE_LIMIT", tt.sentinel,
+				)
 			} else {
 				testReceiverRateLimitSuccess(t, tt.expected)
 			}
@@ -432,3 +467,107 @@ func testReceiverRateLimitSuccess(
 
 	assert.Equal(t, expected, cfg.ReceiverRateLimit)
 }
+
+func TestTrustedProxies(t *testing.T) {
+	tests := []struct {
+		name        string
+		set         bool
+		value       string
+		expectError bool
+		expected    []string
+	}{
+		{
+			// The default must be "trust nobody": an empty list
+			// means forwarded headers are ignored, never that
+			// every peer may speak for the client.
+			name:     caseUnsetUsesDefault,
+			set:      false,
+			expected: []string{},
+		},
+		{
+			name:     "blank value trusts nothing",
+			set:      true,
+			value:    "  ",
+			expected: []string{},
+		},
+		{
+			name:  caseValidValueParsed,
+			set:   true,
+			value: "10.0.0.0/8, 192.168.1.7 ,2001:db8::/32",
+			expected: []string{
+				"10.0.0.0/8", "192.168.1.7/32", "2001:db8::/32",
+			},
+		},
+		{
+			name:     "host bits are masked off",
+			set:      true,
+			value:    "10.1.2.3/8",
+			expected: []string{"10.0.0.0/8"},
+		},
+		{
+			name:        caseUnparseableFails,
+			set:         true,
+			value:       "10.0.0.0/8,not-an-address",
+			expectError: true,
+		},
+		{
+			name:        "out-of-range prefix length fails startup",
+			set:         true,
+			value:       "10.0.0.0/33",
+			expectError: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Cannot use t.Parallel() here because t.Setenv
+			// is incompatible with parallel subtests.
+			t.Setenv("WEBHOOKER_ENVIRONMENT", "dev")
+
+			if tt.set {
+				t.Setenv("TRUSTED_PROXIES", tt.value)
+			} else {
+				require.NoError(t, os.Unsetenv("TRUSTED_PROXIES"))
+			}
+
+			if tt.expectError {
+				expectStartupErrorFor(
+					t, "TRUSTED_PROXIES", config.ErrInvalidCIDR,
+				)
+			} else {
+				testTrustedProxiesSuccess(t, tt.expected)
+			}
+		})
+	}
+}
+
+func testTrustedProxiesSuccess(
+	t *testing.T,
+	expected []string,
+) {
+	t.Helper()
+
+	var cfg *config.Config
+
+	app := fxtest.New(
+		t,
+		fx.Provide(
+			globals.New,
+			logger.New,
+			config.New,
+		),
+		fx.Populate(&cfg),
+	)
+	require.NoError(t, app.Err())
+
+	app.RequireStart()
+
+	defer app.RequireStop()
+
+	got := make([]string, 0, len(cfg.TrustedProxies))
+	for _, prefix := range cfg.TrustedProxies {
+		got = append(got, prefix.String())
+	}
+
+	assert.Equal(t, expected, got)
+}

internal/delivery/ssrf.go

@@ -92,12 +92,7 @@ func ValidateTargetURL(
 ) error {
 	parsed, err := url.Parse(targetURL)
 	if err != nil {
-		// url.Parse embeds the whole URL in its error, and
+		return fmt.Errorf("invalid URL: %w", err)
-		// this one is logged and shown; mask it. Every other
-		// branch below reports only the hostname.
-		return fmt.Errorf(
-			"invalid URL: %w", maskURLError(err),
-		)
 	}
 
 	err = validateScheme(parsed.Scheme)

internal/delivery/target_config_view.go

@@ -3,6 +3,7 @@ package delivery
 import (
 	"encoding/json"
 	"fmt"
+	"net/url"
 	"strconv"
 
 	"sneak.berlin/go/webhooker/internal/database"
@@ -16,6 +17,9 @@ import (
 // browser history, screenshots and screen shares.
 const configUnavailable = "(unavailable)"
 
+// urlPathElision stands in for a URL's elided path.
+const urlPathElision = "/..."
+
 // ConfigField is one labelled, display-safe value derived
 // from a target's stored configuration.
 type ConfigField struct {
@@ -198,5 +202,23 @@ func databaseConfigFields(configJSON string) []ConfigField {
 // parse into a scheme and host yields the neutral
 // placeholder, never the raw string.
 func (c *SlackTargetConfig) MaskedWebhookURL() string {
-	return MaskURL(c.WebhookURL)
+	return maskURL(c.WebhookURL)
+}
+
+// maskURL renders a URL as scheme plus host with everything
+// that can carry a secret removed.
+func maskURL(raw string) string {
+	parsed, err := url.Parse(raw)
+	if err != nil || parsed.Scheme == "" ||
+		parsed.Host == "" {
+		return configUnavailable
+	}
+
+	masked := parsed.Scheme + "://" + parsed.Host
+
+	if parsed.Path != "" && parsed.Path != "/" {
+		masked += urlPathElision
+	}
+
+	return masked
 }

internal/delivery/target_http.go

@@ -363,8 +363,7 @@ func (t *httpTarget) doHTTPRequest(
 	)
 	if reqErr != nil {
 		return 0, "", 0, fmt.Errorf(
-			"creating request: %w",
+			"creating request: %w", reqErr,
-			maskURLError(reqErr),
 		)
 	}
 
@@ -493,19 +492,8 @@ func applyRequestHeaders(
 // executeHTTPRequest sends an HTTP request using the provided
 // client. URLs are validated by the config parsers and the
 // SSRF-safe transport before reaching here.
-//
-// Transport failures are masked here, at the single point
-// where every target's request errors are born, because the
-// caller stores them in DeliveryResult.Error: an unmasked
-// *url.Error would write the target URL — the credential for
-// a Slack incoming webhook — into the per-webhook database.
 func executeHTTPRequest(
 	client *http.Client, req *http.Request,
 ) (*http.Response, error) {
-	resp, err := client.Do(req) //#nosec G704 -- validated URL, SSRF-safe transport
+	return client.Do(req) //#nosec G704 -- validated URL, SSRF-safe transport
-	if err != nil {
-		return nil, maskURLError(err)
-	}
-
-	return resp, nil
 }

internal/delivery/target_slack.go

@@ -125,7 +125,7 @@ func (t *slackTarget) attempt(
 	if err != nil {
 		return attemptResult{
 			success: false,
-			errMsg:  maskURLError(err).Error(),
+			errMsg:  err.Error(),
 		}
 	}
 

internal/delivery/url_mask.go

@@ -1,61 +0,0 @@
-package delivery
-
-import (
-	"errors"
-	"net/url"
-)
-
-// urlPathElision stands in for a URL's elided path.
-const urlPathElision = "/..."
-
-// MaskURL renders a URL as scheme plus host with everything
-// that can carry a secret removed. A delivery target URL is
-// itself a credential — a Slack incoming webhook URL is a
-// bearer token — so the path, query and userinfo are never
-// reproduced, in a page, a log line or a stored error. A URL
-// that does not parse into a scheme and host yields the
-// neutral placeholder, never the raw string.
-func MaskURL(raw string) string {
-	parsed, err := url.Parse(raw)
-	if err != nil || parsed.Scheme == "" ||
-		parsed.Host == "" {
-		return configUnavailable
-	}
-
-	masked := parsed.Scheme + "://" + parsed.Host
-
-	if parsed.Path != "" && parsed.Path != "/" {
-		masked += urlPathElision
-	}
-
-	return masked
-}
-
-// maskURLError strips the credential from an error raised
-// against a request URL. The net/http and net/url packages
-// embed the full request URL in every *url.Error they return,
-// so an unmodified transport error persisted into
-// DeliveryResult.Error writes the credential to disk.
-//
-// The masked error keeps the operation and the wrapped cause,
-// so a DNS failure still reads differently from a refused
-// connection, a TLS handshake failure or a timeout, and Is,
-// As, Timeout and Temporary keep working on it. Only the
-// path, query and userinfo of the URL are dropped. Errors
-// that carry no URL are returned unchanged.
-//
-// Call it where the error is raised, before any wrapping: it
-// replaces the *url.Error itself, so any context wrapped
-// around it first would be discarded.
-func maskURLError(err error) error {
-	var urlErr *url.Error
-	if !errors.As(err, &urlErr) {
-		return err
-	}
-
-	return &url.Error{
-		Op:  urlErr.Op,
-		URL: MaskURL(urlErr.URL),
-		Err: urlErr.Err,
-	}
-}

internal/delivery/url_mask_test.go

@@ -1,196 +0,0 @@
-package delivery_test
-
-import (
-	"context"
-	"encoding/json"
-	"net/http"
-	"net/http/httptest"
-	"testing"
-
-	"github.com/google/uuid"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"gorm.io/gorm"
-	"sneak.berlin/go/webhooker/internal/database"
-	"sneak.berlin/go/webhooker/internal/delivery"
-)
-
-// The path of a Slack incoming webhook URL is the credential:
-// whoever holds these segments can post to the channel
-// forever. None of them may reach a stored delivery error,
-// which lives on disk in the per-webhook database and is
-// serialized by the JSON tag on DeliveryResult.Error.
-const (
-	maskSecretPath = "/services/T00000000/B00000000/" +
-		"XXXXXXXXXXXXXXXXXXXXXXXX"
-)
-
-// assertNoCredential fails if the whole path or any single
-// segment of it survived into the message, so a partial leak
-// fails the test too.
-func assertNoCredential(t *testing.T, msg string) {
-	t.Helper()
-
-	segments := []string{
-		maskSecretPath,
-		"services",
-		"T00000000",
-		"B00000000",
-		"XXXXXXXXXXXXXXXXXXXXXXXX",
-	}
-
-	for _, segment := range segments {
-		assert.NotContains(t, msg, segment)
-	}
-}
-
-// storedDeliveryError returns the error string persisted for a
-// delivery, which is what an operator and any future API read.
-func storedDeliveryError(
-	t *testing.T, db *gorm.DB, deliveryID string,
-) string {
-	t.Helper()
-
-	var result database.DeliveryResult
-
-	require.NoError(t, db.Where(
-		"delivery_id = ?", deliveryID,
-	).First(&result).Error)
-
-	return result.Error
-}
-
-// deliverSlackTo runs a Slack delivery against webhookURL and
-// returns the error string it persisted.
-func deliverSlackTo(
-	t *testing.T, webhookURL string,
-) string {
-	t.Helper()
-
-	db := testWebhookDB(t)
-	e := testEngine(t, 1)
-	targetID := uuid.New().String()
-
-	slackCfg, err := json.Marshal(
-		delivery.SlackTargetConfig{
-			WebhookURL: webhookURL,
-		},
-	)
-	require.NoError(t, err)
-
-	event := seedEvent(t, db, `{"test":true}`)
-
-	dlv := seedDelivery(
-		t, db, event.ID, targetID,
-		database.DeliveryStatusPending,
-	)
-
-	d := buildSlackDelivery(
-		dlv, event, targetID,
-		"test-slack-mask", string(slackCfg),
-	)
-
-	e.ExportDeliverSlack(context.TODO(), db, d)
-
-	assertDeliveryStatus(t, db, dlv.ID,
-		database.DeliveryStatusFailed,
-	)
-
-	return storedDeliveryError(t, db, dlv.ID)
-}
-
-// TestDeliverSlack_TransportErrorMasksWebhookURL is the
-// load-bearing regression test: a transport failure must not
-// persist the webhook URL's credential into the database, and
-// must still say what went wrong and where.
-func TestDeliverSlack_TransportErrorMasksWebhookURL(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	// A server closed before use gives a deterministic
-	// transport failure against a known host.
-	ts := httptest.NewServer(http.NewServeMux())
-	host := ts.URL
-
-	ts.Close()
-
-	errMsg := deliverSlackTo(t, host+maskSecretPath)
-
-	require.NotEmpty(t, errMsg)
-	assertNoCredential(t, errMsg)
-
-	// The diagnostic value survives: the operation, the host
-	// and the transport failure are all still reported, and
-	// only the path is elided.
-	assert.Contains(t, errMsg, "sending request")
-	assert.Contains(t, errMsg, "Post")
-	assert.Contains(t, errMsg, host+"/...")
-	assert.Contains(t, errMsg, "connection refused")
-}
-
-// TestDeliverSlack_UnparsableURLMasksWebhookURL covers the
-// other error path out of a Slack attempt: url.Parse also
-// embeds the whole URL in the error it returns.
-func TestDeliverSlack_UnparsableURLMasksWebhookURL(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	errMsg := deliverSlackTo(
-		t,
-		"https://hooks.slack.com"+maskSecretPath+"\n",
-	)
-
-	require.NotEmpty(t, errMsg)
-	assertNoCredential(t, errMsg)
-	assert.Contains(t, errMsg, "invalid control character")
-}
-
-// TestDoHTTPRequest_TransportErrorMasksURL proves the HTTP
-// target's transport errors are masked too; its destination
-// URL can carry a token in a query string.
-func TestDoHTTPRequest_TransportErrorMasksURL(t *testing.T) {
-	t.Parallel()
-
-	ts := httptest.NewServer(http.NewServeMux())
-	host := ts.URL
-
-	ts.Close()
-
-	e := testEngine(t, 1)
-
-	cfg, err := e.ExportParseHTTPConfig(
-		newHTTPTargetConfig(host + maskSecretPath),
-	)
-	require.NoError(t, err)
-
-	statusCode, _, _, reqErr := e.ExportDoHTTPRequest(
-		context.TODO(), cfg,
-		&database.Event{Body: `{"test":true}`},
-	)
-	require.Error(t, reqErr)
-	assert.Zero(t, statusCode)
-
-	assertNoCredential(t, reqErr.Error())
-	assert.Contains(t, reqErr.Error(), host+"/...")
-	assert.Contains(
-		t, reqErr.Error(), "connection refused",
-	)
-}
-
-// TestValidateTargetURL_UnparsableURLIsMasked proves the SSRF
-// validator's error does not carry the submitted URL, which
-// the handler both logs and shows.
-func TestValidateTargetURL_UnparsableURLIsMasked(t *testing.T) {
-	t.Parallel()
-
-	err := delivery.ValidateTargetURL(
-		context.TODO(),
-		"https://hooks.slack.com"+maskSecretPath+"\n",
-	)
-	require.Error(t, err)
-
-	assertNoCredential(t, err.Error())
-	assert.Contains(t, err.Error(), "invalid URL")
-}

internal/handlers/source_detail_test.go

@@ -26,14 +26,14 @@ const (
 )
 
 // seedConfiguredTarget inserts a target with a stored config
-// blob and returns it.
+// blob.
 func seedConfiguredTarget(
 	t *testing.T,
 	db *database.Database,
 	webhookID string,
 	targetType database.TargetType,
 	config string,
-) *database.Target {
+) {
 	t.Helper()
 
 	tgt := &database.Target{
@@ -48,8 +48,6 @@ func seedConfiguredTarget(
 		t,
 		db.DB().Omit(clause.Associations).Create(tgt).Error,
 	)
-
-	return tgt
 }
 
 // renderSourceDetailPage runs the real source detail handler

internal/handlers/source_logs_test.go

@@ -1,134 +0,0 @@
-package handlers_test
-
-import (
-	"context"
-	"net/http"
-	"net/http/httptest"
-	"testing"
-
-	"github.com/go-chi/chi"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"gorm.io/gorm/clause"
-	"sneak.berlin/go/webhooker/internal/database"
-	"sneak.berlin/go/webhooker/internal/handlers"
-	"sneak.berlin/go/webhooker/internal/session"
-)
-
-// seedDeliveredEvent records an event and a delivery for it in
-// the webhook's own database, so the log page has a delivery
-// to render against the target.
-func seedDeliveredEvent(
-	t *testing.T,
-	dbMgr *database.WebhookDBManager,
-	webhookID, targetID string,
-) {
-	t.Helper()
-
-	webhookDB, err := dbMgr.GetDB(webhookID)
-	require.NoError(t, err)
-
-	event := &database.Event{
-		WebhookID:   webhookID,
-		Method:      http.MethodPost,
-		Body:        `{"test":true}`,
-		ContentType: "application/json",
-	}
-
-	require.NoError(t, webhookDB.Omit(
-		clause.Associations,
-	).Create(event).Error)
-
-	dlv := &database.Delivery{
-		EventID:  event.ID,
-		TargetID: targetID,
-		Status:   database.DeliveryStatusDelivered,
-	}
-
-	require.NoError(t, webhookDB.Omit(
-		clause.Associations,
-	).Create(dlv).Error)
-}
-
-// renderSourceLogsPage runs the real event log handler for a
-// webhook and returns the rendered HTML.
-func renderSourceLogsPage(
-	t *testing.T,
-	h *handlers.Handlers,
-	sess *session.Session,
-	webhookID string,
-) string {
-	t.Helper()
-
-	req := httptest.NewRequestWithContext(
-		context.Background(),
-		http.MethodGet,
-		"/source/"+webhookID+"/logs",
-		nil,
-	)
-
-	for _, c := range authenticatedCookies(
-		t, sess, deleteTestUserID, deleteTestUsername,
-	) {
-		req.AddCookie(c)
-	}
-
-	rctx := chi.NewRouteContext()
-	rctx.URLParams.Add(paramSourceID, webhookID)
-
-	req = req.WithContext(
-		context.WithValue(
-			req.Context(), chi.RouteCtxKey, rctx,
-		),
-	)
-
-	w := httptest.NewRecorder()
-	h.HandleSourceLogs().ServeHTTP(w, req)
-
-	require.Equal(t, http.StatusOK, w.Code)
-
-	return w.Body.String()
-}
-
-// TestHandleSourceLogs_MasksSlackWebhookURL proves the event
-// log page is handed a display-safe projection of each target
-// rather than the stored row, so the credential cannot be
-// rendered from its template data.
-func TestHandleSourceLogs_MasksSlackWebhookURL(t *testing.T) {
-	t.Parallel()
-
-	var (
-		h     *handlers.Handlers
-		sess  *session.Session
-		db    *database.Database
-		dbMgr *database.WebhookDBManager
-	)
-
-	app := newTestApp(t, &h, &sess, &db, &dbMgr)
-	app.RequireStart()
-
-	t.Cleanup(app.RequireStop)
-
-	wh := seedWebhook(t, db)
-	tgt := seedConfiguredTarget(
-		t, db, wh.ID,
-		database.TargetTypeSlack,
-		`{"webhookUrl":"`+slackWebhookURL+`"}`,
-	)
-
-	seedDeliveredEvent(t, dbMgr, wh.ID, tgt.ID)
-
-	body := renderSourceLogsPage(t, h, sess, wh.ID)
-
-	assert.NotContains(t, body, slackSecretPath)
-	assert.NotContains(t, body, "T00000000")
-	assert.NotContains(t, body, "B00000000")
-	assert.NotContains(
-		t, body, "XXXXXXXXXXXXXXXXXXXXXXXX",
-	)
-	assert.NotContains(t, body, "webhookUrl")
-
-	// The page still identifies the delivery's target.
-	assert.Contains(t, body, tgt.Name)
-	assert.Contains(t, body, "delivered")
-}

internal/handlers/source_management.go

@@ -96,17 +96,7 @@ func parseRetentionDays(raw string, fallback int) (int, error) {
 type EventWithDeliveries struct {
 	database.Event
 
-	Deliveries []DeliveryView
+	Deliveries []database.Delivery
-}
-
-// DeliveryView is the display-safe projection of a delivery
-// for the event log page. Its target is a TargetView, so the
-// stored configuration blob — which holds the target's
-// credential — has no path to the template.
-type DeliveryView struct {
-	ID     string
-	Status database.DeliveryStatus
-	Target delivery.TargetView
 }
 
 // HandleSourceList shows a list of user's webhooks.
@@ -774,27 +764,22 @@ func (h *Handlers) HandleSourceLogs() http.HandlerFunc {
 	}
 }
 
-// loadTargetMap loads targets into a map of display-safe
+// loadTargetMap loads targets into a map keyed by target ID.
-// views keyed by target ID. The projection happens here so
-// that no caller can hand a raw target, configuration blob
-// and all, to a template.
 func (h *Handlers) loadTargetMap(
 	webhookID string,
-) map[string]delivery.TargetView {
+) map[string]database.Target {
 	var targets []database.Target
 
 	h.db.DB().Where(
 		"webhook_id = ?", webhookID,
 	).Find(&targets)
 
-	views := delivery.NewTargetViews(targets)
-
 	targetMap := make(
-		map[string]delivery.TargetView, len(views),
+		map[string]database.Target, len(targets),
 	)
 
-	for _, v := range views {
+	for _, t := range targets {
-		targetMap[v.ID] = v
+		targetMap[t.ID] = t
 	}
 
 	return targetMap
@@ -819,7 +804,7 @@ func (h *Handlers) parsePage(r *http.Request) int {
 func (h *Handlers) loadEventsWithDeliveries(
 	w http.ResponseWriter,
 	webhook database.Webhook,
-	targetMap map[string]delivery.TargetView,
+	targetMap map[string]database.Target,
 	page int,
 ) ([]EventWithDeliveries, int64) {
 	var totalEvents int64
@@ -858,39 +843,22 @@ func (h *Handlers) loadEventsWithDeliveries(
 	for i := range events {
 		result[i].Event = events[i]
 
-		var deliveries []database.Delivery
-
 		webhookDB.Where(
 			"event_id = ?", events[i].ID,
-		).Find(&deliveries)
+		).Find(&result[i].Deliveries)
 
-		result[i].Deliveries = newDeliveryViews(
+		for j := range result[i].Deliveries {
-			deliveries, targetMap,
+			tid := result[i].Deliveries[j].TargetID
-		)
+
+			if target, ok := targetMap[tid]; ok {
+				result[i].Deliveries[j].Target = target
+			}
+		}
 	}
 
 	return result, totalEvents
 }
 
-// newDeliveryViews projects deliveries for rendering,
-// resolving each one's target to its display-safe view.
-func newDeliveryViews(
-	deliveries []database.Delivery,
-	targetMap map[string]delivery.TargetView,
-) []DeliveryView {
-	views := make([]DeliveryView, len(deliveries))
-
-	for i := range deliveries {
-		views[i] = DeliveryView{
-			ID:     deliveries[i].ID,
-			Status: deliveries[i].Status,
-			Target: targetMap[deliveries[i].TargetID],
-		}
-	}
-
-	return views
-}
-
 // HandleEntrypointCreate handles adding a new entrypoint.
 func (h *Handlers) HandleEntrypointCreate() http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
@@ -1134,12 +1102,9 @@ func (h *Handlers) buildURLTargetConfig(
 		r.Context(), targetURL,
 	)
 	if err != nil {
-		// The submitted URL can be a credential (a Slack
-		// incoming webhook URL is a bearer token), so the log
-		// records only its scheme and host.
 		h.log.Warn(
 			"target URL blocked by SSRF protection",
-			"url", delivery.MaskURL(targetURL),
+			"url", targetURL,
 			"error", err,
 		)
 		http.Error(

internal/handlers/ui_copy_test.go

@@ -1,299 +0,0 @@
-package handlers_test
-
-import (
-	"context"
-	"net/http"
-	"net/http/httptest"
-	"strconv"
-	"testing"
-	"time"
-
-	"github.com/stretchr/testify/assert"
-	"sneak.berlin/go/webhooker/internal/database"
-	"sneak.berlin/go/webhooker/internal/delivery"
-	"sneak.berlin/go/webhooker/internal/handlers"
-	"sneak.berlin/go/webhooker/internal/session"
-)
-
-// Template data keys the page templates read. The handlers package has
-// its own unexported constants for these; this is the external test
-// package, so it needs its own.
-const (
-	dataKeyWebhook = "Webhook"
-	dataKeyError   = "Error"
-)
-
-// testWebhookID is the identifier given to the webhook under test on
-// pages that render one.
-const testWebhookID = "wh-1"
-
-// renderPage renders a page template through the real template set as
-// an authenticated user and returns the resulting HTML.
-func renderPage(
-	t *testing.T,
-	h *handlers.Handlers,
-	sess *session.Session,
-	page string,
-	data map[string]any,
-) string {
-	t.Helper()
-
-	cookies := authenticatedCookies(t, sess, "test-user-id", "testuser")
-
-	req := httptest.NewRequestWithContext(
-		context.Background(), http.MethodGet, "/", nil,
-	)
-	for _, c := range cookies {
-		req.AddCookie(c)
-	}
-
-	w := httptest.NewRecorder()
-	h.RenderTemplateForTest(w, req, page, data)
-
-	return w.Body.String()
-}
-
-// TestNavbarUsesWebhookTerminology pins the user-visible navigation
-// label to "Webhooks". The /sources route is deliberately unchanged, so
-// the assertion targets the link text rather than the href.
-func TestNavbarUsesWebhookTerminology(t *testing.T) {
-	t.Parallel()
-
-	var h *handlers.Handlers
-
-	var sess *session.Session
-
-	app := newTestApp(t, &h, &sess)
-	app.RequireStart()
-
-	t.Cleanup(app.RequireStop)
-
-	// One item, so the list body renders too: it calls
-	// WebhookListItem.RetentionLabel, promoted from the embedded
-	// Webhook and therefore a pointer method. An empty list would
-	// skip that call and hide a template error behind the
-	// navigation assertions below.
-	item := handlers.WebhookListItem{}
-	item.Name = "wh"
-	item.ID = testWebhookID
-	item.RetentionDays = 14
-
-	body := renderPage(t, h, sess, "sources_list.html", map[string]any{
-		"Webhooks": []handlers.WebhookListItem{item},
-	})
-
-	assert.Contains(t, body, "Retention: 14 days")
-	assert.Contains(t, body, `class="btn-text">Webhooks</a>`)
-	assert.Contains(
-		t, body, `class="btn-text w-full text-left">Webhooks</a>`,
-	)
-	assert.Contains(
-		t, body,
-		`<h1 class="text-2xl font-medium text-gray-900">Webhooks</h1>`,
-	)
-	assert.NotContains(
-		t, body, ">Sources<",
-		"no user-visible element may still be labelled Sources",
-	)
-	assert.Contains(
-		t, body, `href="/sources"`,
-		"the /sources route itself must not change",
-	)
-}
-
-// TestEditPageUsesWebhookTerminology pins the edit page's heading and
-// its back link. The link's href still points at /source/{id}, which is
-// intentional: only user-visible copy changes.
-func TestEditPageUsesWebhookTerminology(t *testing.T) {
-	t.Parallel()
-
-	var h *handlers.Handlers
-
-	var sess *session.Session
-
-	app := newTestApp(t, &h, &sess)
-	app.RequireStart()
-
-	t.Cleanup(app.RequireStop)
-
-	// The webhook goes in as a pointer because source_edit.html calls
-	// Webhook.RetentionLabel, a pointer method: a map element is not
-	// addressable, so a value here renders an error instead of the
-	// page.
-	webhook := &database.Webhook{Name: "wh", RetentionDays: 14}
-	webhook.ID = testWebhookID
-
-	body := renderPage(t, h, sess, "source_edit.html", map[string]any{
-		dataKeyWebhook: webhook,
-		dataKeyError:   "",
-	})
-
-	assert.Contains(t, body, "Edit Webhook")
-	assert.NotContains(t, body, ">Sources<")
-	assert.Contains(t, body, `href="/source/wh-1"`)
-}
-
-// TestCreateFormRetentionCopyMatchesBehaviour pins the create form's
-// retention copy to what the code does: the reaper permanently deletes
-// events past the cutoff, an empty field falls back to
-// DefaultRetentionDays, and 0 is rewritten to the retain-forever
-// sentinel by Webhook.BeforeSave.
-func TestCreateFormRetentionCopyMatchesBehaviour(t *testing.T) {
-	t.Parallel()
-
-	var h *handlers.Handlers
-
-	var sess *session.Session
-
-	app := newTestApp(t, &h, &sess)
-	app.RequireStart()
-
-	t.Cleanup(app.RequireStop)
-
-	body := renderPage(t, h, sess, "sources_new.html", map[string]any{
-		"Name":                 "",
-		"Description":          "",
-		"DefaultRetentionDays": database.DefaultRetentionDays,
-		dataKeyError:           "",
-	})
-
-	assert.Contains(
-		t, body,
-		"permanently deletes events older than this",
-		"the form must say retention is enforced by deletion",
-	)
-	assert.Contains(t, body, "Enter 0 to retain events forever")
-	assert.Contains(
-		t, body,
-		"leave blank to use the default of "+
-			strconv.Itoa(database.DefaultRetentionDays)+" days",
-		"blank means the default, not forever",
-	)
-}
-
-// TestEditFormRetentionCopyMatchesBehaviour pins the edit form's
-// retention copy, including that it states the stored policy via
-// RetentionLabel and that an empty field leaves that policy unchanged
-// rather than meaning forever.
-func TestEditFormRetentionCopyMatchesBehaviour(t *testing.T) {
-	t.Parallel()
-
-	var h *handlers.Handlers
-
-	var sess *session.Session
-
-	app := newTestApp(t, &h, &sess)
-	app.RequireStart()
-
-	t.Cleanup(app.RequireStop)
-
-	finite := &database.Webhook{Name: "wh", RetentionDays: 14}
-	finite.ID = testWebhookID
-
-	body := renderPage(t, h, sess, "source_edit.html", map[string]any{
-		dataKeyWebhook: finite,
-		dataKeyError:   "",
-	})
-
-	assert.Contains(t, body, "Currently 14 days.")
-	assert.Contains(
-		t, body,
-		"permanently deletes events older than this",
-	)
-	assert.Contains(t, body, "Enter 0 to retain events forever")
-	assert.Contains(
-		t, body,
-		"leave blank to keep the current setting",
-		"blank means unchanged, not forever",
-	)
-
-	forever := &database.Webhook{
-		Name:          "wh",
-		RetentionDays: database.RetentionForeverDays,
-	}
-	forever.ID = "wh-2"
-
-	foreverBody := renderPage(
-		t, h, sess, "source_edit.html", map[string]any{
-			dataKeyWebhook: forever,
-			dataKeyError:   "",
-		},
-	)
-
-	assert.Contains(
-		t, foreverBody, "Currently forever.",
-		"a retain-forever webhook must not read as a day count",
-	)
-	assert.Contains(
-		t, foreverBody,
-		"No events are deleted while retention is set to forever",
-	)
-	assert.NotContains(
-		t, foreverBody,
-		"permanently deletes events older than this",
-		"the reaper skips retain-forever webhooks, so the form "+
-			"must not claim it deletes their events",
-	)
-}
-
-// TestEntrypointCopyButtonIsProgressiveEnhancement proves the copy
-// affordance degrades: the button ships with the hidden attribute, so a
-// browser that never runs app.js shows no dead control, and the URL is
-// rendered as ordinary selectable text either way.
-func TestEntrypointCopyButtonIsProgressiveEnhancement(t *testing.T) {
-	t.Parallel()
-
-	var h *handlers.Handlers
-
-	var sess *session.Session
-
-	app := newTestApp(t, &h, &sess)
-	app.RequireStart()
-
-	t.Cleanup(app.RequireStop)
-
-	entrypoint := database.Entrypoint{Path: "abc123"}
-	entrypoint.ID = "ep-1"
-
-	// The webhook goes in as a pointer because source_detail.html
-	// calls Webhook.RetentionLabel, a pointer method: a map element
-	// is not addressable, so a value here aborts execution partway
-	// down the page, after the copy button has already been flushed
-	// to the response.
-	webhook := &database.Webhook{Name: "wh", RetentionDays: 14}
-	webhook.ID = testWebhookID
-	webhook.CreatedAt = time.Date(
-		2026, time.January, 2, 3, 4, 5, 0, time.UTC,
-	)
-
-	body := renderPage(t, h, sess, "source_detail.html", map[string]any{
-		dataKeyWebhook: webhook,
-		"Entrypoints":  []database.Entrypoint{entrypoint},
-		// The handler passes delivery.NewTargetViews(targets), never
-		// raw targets, so the test data has to have that same shape.
-		"Targets": delivery.NewTargetViews(nil),
-		"Events":  []database.Event{},
-		"BaseURL": "https://hooks.example.com",
-	})
-
-	assert.Contains(
-		t, body,
-		`<code id="entrypoint-url-ep-1"`,
-	)
-	assert.Contains(t, body, "https://hooks.example.com/webhook/abc123")
-	assert.Contains(
-		t, body,
-		`hidden data-copy-target="entrypoint-url-ep-1"`,
-		"the button must start hidden and be revealed by script",
-	)
-
-	// renderTemplate streams to the ResponseWriter, so an abort
-	// midway still leaves everything above it in the body. This pins
-	// content from the last line of the template, which is below the
-	// assertions above: without it, a page that renders the copy
-	// button and then 500s passes.
-	assert.Contains(
-		t, body, "Retention: 14 days",
-		"the page must render to completion, not abort partway",
-	)
-}

internal/middleware/ratelimit.go

@@ -2,6 +2,9 @@ package middleware
 
 import (
 	"net/http"
+	"net/netip"
+	"slices"
+	"strings"
 	"time"
 
 	"github.com/go-chi/httprate"
@@ -31,13 +34,118 @@ const (
 	receiverRateInterval = 1 * time.Minute
 )
 
+// normalizeAddr strips the IPv4-in-IPv6 wrapper and any zone from
+// addr so that comparisons and bucket keys are canonical.
+func normalizeAddr(addr netip.Addr) netip.Addr {
+	return addr.Unmap().WithZone("")
+}
+
+// isTrustedProxy reports whether addr belongs to a network the
+// operator listed in TRUSTED_PROXIES. The list is empty by default,
+// so by default nothing is trusted.
+func (m *Middleware) isTrustedProxy(addr netip.Addr) bool {
+	for _, prefix := range m.params.Config.TrustedProxies {
+		if prefix.Contains(addr) {
+			return true
+		}
+	}
+
+	return false
+}
+
+// forwardedClientAddr returns the client address named by this
+// request's forwarded headers. It is consulted only for requests
+// whose direct peer is a trusted proxy.
+//
+// True-Client-IP and X-Real-IP are single-valued, and a trusted
+// proxy is expected to overwrite whatever the client sent, so they
+// are taken as given. X-Forwarded-For is a chain the client can
+// prepend to, so it is walked right to left and the first hop that
+// is not itself a trusted proxy wins: entries the client inserted
+// sit to the left of the proxies' own appends and cannot be picked
+// while the chain is intact.
+func (m *Middleware) forwardedClientAddr(
+	r *http.Request,
+) (netip.Addr, bool) {
+	for _, header := range []string{"True-Client-IP", "X-Real-IP"} {
+		addr, err := netip.ParseAddr(
+			strings.TrimSpace(r.Header.Get(header)),
+		)
+		if err == nil {
+			return normalizeAddr(addr), true
+		}
+	}
+
+	hops := strings.Split(
+		strings.Join(r.Header.Values("X-Forwarded-For"), ","), ",",
+	)
+
+	for _, hop := range slices.Backward(hops) {
+		addr, err := netip.ParseAddr(strings.TrimSpace(hop))
+		if err != nil {
+			continue
+		}
+
+		if addr = normalizeAddr(addr); !m.isTrustedProxy(addr) {
+			return addr, true
+		}
+	}
+
+	return netip.Addr{}, false
+}
+
+// rateLimitKey is the client identity every rate limiter in this
+// package buckets on. Forwarded headers are honoured only when the
+// direct peer (RemoteAddr) is inside the configured trusted-proxy
+// set; otherwise the peer address itself is the key. Without that
+// gate any client could mint a fresh bucket per request, or starve
+// another client's bucket, by picking an X-Forwarded-For value —
+// which makes every limit here decorative against a deliberate
+// attacker.
+func (m *Middleware) rateLimitKey(r *http.Request) (string, error) {
+	return m.clientKey(r), nil
+}
+
+// clientKey computes the bucket key described on rateLimitKey.
+func (m *Middleware) clientKey(r *http.Request) string {
+	peer, err := netip.ParseAddr(ipFromHostPort(r.RemoteAddr))
+	if err != nil {
+		// Not an address we can reason about; key on the raw
+		// value rather than collapsing such peers into one
+		// shared bucket.
+		return r.RemoteAddr
+	}
+
+	peer = normalizeAddr(peer)
+	if !m.isTrustedProxy(peer) {
+		return peer.String()
+	}
+
+	if addr, ok := m.forwardedClientAddr(r); ok {
+		return addr.String()
+	}
+
+	return peer.String()
+}
+
+// tooManyRequests returns the 429 handler shared by every limiter:
+// it logs the rejection with logMessage and answers with
+// responseMessage. httprate adds the Retry-After header (RFC 6585).
+func (m *Middleware) tooManyRequests(
+	logMessage, responseMessage string,
+) http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
+		m.log.Warn(logMessage, "path", r.URL.Path)
+		http.Error(w, responseMessage, http.StatusTooManyRequests)
+	}
+}
+
 // LoginRateLimit returns middleware that enforces per-IP rate
 // limiting on login attempts using go-chi/httprate. Only POST
 // requests are rate-limited; GET requests (rendering the login
 // form) pass through unaffected. When the rate limit is exceeded,
-// a 429 Too Many Requests response is returned. IP extraction
+// a 429 Too Many Requests response is returned. Clients are
-// honours X-Forwarded-For, X-Real-IP, and True-Client-IP headers
+// identified by rateLimitKey.
-// for reverse-proxy setups.
 func (m *Middleware) LoginRateLimit() func(http.Handler) http.Handler {
 	return m.postRateLimit(
 		loginRateLimit,
@@ -66,9 +174,7 @@ func (m *Middleware) PasswordChangeRateLimit() func(http.Handler) http.Handler {
 // limit on POST requests only; all other methods pass through
 // unaffected. Requests over the limit receive a 429 with the
 // given response message, and each rejection is logged with the
-// given log message. IP extraction honours X-Forwarded-For,
+// given log message. Clients are identified by rateLimitKey.
-// X-Real-IP, and True-Client-IP headers for reverse-proxy
-// setups.
 func (m *Middleware) postRateLimit(
 	limit int,
 	interval time.Duration,
@@ -77,19 +183,10 @@ func (m *Middleware) postRateLimit(
 	limiter := httprate.Limit(
 		limit,
 		interval,
-		httprate.WithKeyFuncs(httprate.KeyByRealIP),
+		httprate.WithKeyFuncs(m.rateLimitKey),
-		httprate.WithLimitHandler(http.HandlerFunc(
+		httprate.WithLimitHandler(
-			func(w http.ResponseWriter, r *http.Request) {
+			m.tooManyRequests(logMessage, responseMessage),
-				m.log.Warn(logMessage,
+		),
-					"path", r.URL.Path,
-				)
-				http.Error(
-					w,
-					responseMessage,
-					http.StatusTooManyRequests,
-				)
-			},
-		)),
 	)
 
 	return func(next http.Handler) http.Handler {
@@ -116,31 +213,19 @@ func (m *Middleware) postRateLimit(
 // path (the path contains the entrypoint UUID, so each sender
 // is limited per entrypoint without affecting other senders or
 // other entrypoints). The limit is Config.ReceiverRateLimit
-// requests per minute. Requests over the limit receive a 429;
+// requests per minute. Requests over the limit receive a 429.
-// httprate adds the Retry-After header (RFC 6585). IP
+// Clients are identified by rateLimitKey.
-// extraction honours X-Forwarded-For, X-Real-IP, and
-// True-Client-IP headers for reverse-proxy setups.
 func (m *Middleware) ReceiverRateLimit() func(http.Handler) http.Handler {
 	return httprate.Limit(
 		m.params.Config.ReceiverRateLimit,
 		receiverRateInterval,
 		httprate.WithKeyFuncs(
-			httprate.KeyByRealIP,
+			m.rateLimitKey,
 			httprate.KeyByEndpoint,
 		),
-		httprate.WithLimitHandler(http.HandlerFunc(
+		httprate.WithLimitHandler(m.tooManyRequests(
-			func(w http.ResponseWriter, r *http.Request) {
-				m.log.Warn(
 			"webhook receiver rate limit exceeded",
-					"path", r.URL.Path,
+			"Too many requests. Please slow down.",
-				)
-				http.Error(
-					w,
-					"Too many requests. "+
-						"Please slow down.",
-					http.StatusTooManyRequests,
-				)
-			},
 		)),
 	)
 }

internal/middleware/ratelimit_test.go

@@ -2,9 +2,11 @@ package middleware_test
 
 import (
 	"context"
+	"fmt"
 	"log/slog"
 	"net/http"
 	"net/http/httptest"
+	"net/netip"
 	"os"
 	"testing"
 
@@ -182,11 +184,22 @@ func TestLoginRateLimit_IndependentPerIP(t *testing.T) {
 	)
 }
 
-// receiverLimitedHandler builds a ReceiverRateLimit-wrapped
+// okHandler is the terminal handler the limiter middleware wraps
-// handler with the given per-minute limit.
+// in these tests: it answers 200 to anything that reaches it.
-func receiverLimitedHandler(
+func okHandler() http.Handler {
-	t *testing.T, limit int,
+	return http.HandlerFunc(
-) http.Handler {
+		func(w http.ResponseWriter, _ *http.Request) {
+			w.WriteHeader(http.StatusOK)
+		},
+	)
+}
+
+// rateLimitMiddleware builds a Middleware around cfg, whose
+// TrustedProxies field is what the rate limit key function gates
+// forwarded-header trust on.
+func rateLimitMiddleware(
+	t *testing.T, cfg *config.Config,
+) *middleware.Middleware {
 	t.Helper()
 
 	log := slog.New(slog.NewTextHandler(
@@ -194,17 +207,53 @@ func receiverLimitedHandler(
 		&slog.HandlerOptions{Level: slog.LevelDebug},
 	))
 
-	m := middleware.NewForTest(
+	return middleware.NewForTest(log, cfg, nil)
-		log,
+}
-		&config.Config{ReceiverRateLimit: limit},
+
-		nil,
+// trustedProxies parses CIDR strings for a test Config.
+func trustedProxies(cidrs ...string) []netip.Prefix {
+	prefixes := make([]netip.Prefix, 0, len(cidrs))
+	for _, cidr := range cidrs {
+		prefixes = append(prefixes, netip.MustParsePrefix(cidr))
+	}
+
+	return prefixes
+}
+
+// postWithHeaders sends one POST to the handler from peer with the
+// given headers set and returns the recorder.
+func postWithHeaders(
+	handler http.Handler,
+	peer, path string,
+	headers map[string]string,
+) *httptest.ResponseRecorder {
+	req := httptest.NewRequestWithContext(
+		context.Background(), http.MethodPost, path, nil,
+	)
+	req.RemoteAddr = peer
+
+	for name, value := range headers {
+		req.Header.Set(name, value)
+	}
+
+	w := httptest.NewRecorder()
+	handler.ServeHTTP(w, req)
+
+	return w
+}
+
+// receiverLimitedHandler builds a ReceiverRateLimit-wrapped
+// handler with the given per-minute limit and no trusted proxies.
+func receiverLimitedHandler(
+	t *testing.T, limit int,
+) http.Handler {
+	t.Helper()
+
+	m := rateLimitMiddleware(
+		t, &config.Config{ReceiverRateLimit: limit},
 	)
 
-	return m.ReceiverRateLimit()(http.HandlerFunc(
+	return m.ReceiverRateLimit()(okHandler())
-		func(w http.ResponseWriter, _ *http.Request) {
-			w.WriteHeader(http.StatusOK)
-		},
-	))
 }
 
 // receiverPost sends one POST to the handler from the given IP
@@ -311,3 +360,182 @@ func TestReceiverRateLimit_CountsEveryMethod(t *testing.T) {
 		"a GET over the limit must be rate-limited",
 	)
 }
+
+const (
+	loginPath  = "/pages/login"
+	headerXFF  = "X-Forwarded-For"
+	headerReal = "X-Real-IP"
+	headerTrue = "True-Client-IP"
+)
+
+// TestRateLimitKey_SpoofedForwardedFromUntrustedPeer is the test
+// this gating exists for: with no trusted proxies configured (the
+// default), a client that rotates a forwarded header on every
+// request must stay in one bucket. If forwarded headers were
+// trusted unconditionally, each spoofed value would mint a fresh
+// bucket and the limit would stop no one.
+func TestRateLimitKey_SpoofedForwardedFromUntrustedPeer(
+	t *testing.T,
+) {
+	t.Parallel()
+
+	for _, header := range []string{
+		headerXFF, headerReal, headerTrue,
+	} {
+		t.Run(header, func(t *testing.T) {
+			t.Parallel()
+
+			m := rateLimitMiddleware(t, &config.Config{})
+			handler := m.LoginRateLimit()(okHandler())
+
+			const peer = "203.0.113.9:44444"
+
+			for i := range middleware.LoginRateLimitConst {
+				w := postWithHeaders(
+					handler, peer, loginPath,
+					map[string]string{
+						header: fmt.Sprintf(
+							"198.51.100.%d", i+1,
+						),
+					},
+				)
+				assert.Equal(
+					t, http.StatusOK, w.Code,
+					"request %d should pass", i,
+				)
+			}
+
+			w := postWithHeaders(
+				handler, peer, loginPath,
+				map[string]string{header: "198.51.100.200"},
+			)
+			assert.Equal(
+				t, http.StatusTooManyRequests, w.Code,
+				"a spoofed %s from an untrusted peer must "+
+					"not mint a fresh bucket", header,
+			)
+		})
+	}
+}
+
+// TestRateLimitKey_ForwardedHonouredFromTrustedPeer checks the
+// other half: when the direct peer is a configured trusted proxy,
+// the forwarded client address is what buckets are keyed on, so
+// one sender behind the proxy cannot exhaust another's limit.
+func TestRateLimitKey_ForwardedHonouredFromTrustedPeer(
+	t *testing.T,
+) {
+	t.Parallel()
+
+	m := rateLimitMiddleware(t, &config.Config{
+		TrustedProxies: trustedProxies("10.0.0.0/8"),
+	})
+	handler := m.LoginRateLimit()(okHandler())
+
+	const peer = "10.0.0.1:44444"
+
+	first := map[string]string{headerXFF: "198.51.100.7"}
+
+	for range middleware.LoginRateLimitConst {
+		postWithHeaders(handler, peer, loginPath, first)
+	}
+
+	w := postWithHeaders(handler, peer, loginPath, first)
+	assert.Equal(
+		t, http.StatusTooManyRequests, w.Code,
+		"the forwarded client's own bucket must fill up",
+	)
+
+	w = postWithHeaders(
+		handler, peer, loginPath,
+		map[string]string{headerXFF: "198.51.100.8"},
+	)
+	assert.Equal(
+		t, http.StatusOK, w.Code,
+		"a forwarded header from a trusted peer must be honoured",
+	)
+}
+
+// TestRateLimitKey_ChainWalkSkipsClientPrepended covers the
+// residual spoofing route behind a trusted proxy: the client
+// controls the leftmost X-Forwarded-For entries, so the key is the
+// rightmost hop that is not itself trusted. Rotating the prepended
+// entry must not create new buckets.
+func TestRateLimitKey_ChainWalkSkipsClientPrepended(t *testing.T) {
+	t.Parallel()
+
+	m := rateLimitMiddleware(t, &config.Config{
+		TrustedProxies: trustedProxies("10.0.0.0/8"),
+	})
+	handler := m.LoginRateLimit()(okHandler())
+
+	const peer = "10.0.0.1:44444"
+
+	chain := func(spoof string) map[string]string {
+		return map[string]string{
+			headerXFF: spoof + ", 198.51.100.7, 10.0.0.2",
+		}
+	}
+
+	for i := range middleware.LoginRateLimitConst {
+		w := postWithHeaders(
+			handler, peer, loginPath,
+			chain(fmt.Sprintf("9.9.9.%d", i+1)),
+		)
+		assert.Equal(
+			t, http.StatusOK, w.Code,
+			"request %d should pass", i,
+		)
+	}
+
+	w := postWithHeaders(
+		handler, peer, loginPath, chain("9.9.9.200"),
+	)
+	assert.Equal(
+		t, http.StatusTooManyRequests, w.Code,
+		"a client-prepended X-Forwarded-For entry must not "+
+			"mint a fresh bucket",
+	)
+}
+
+// TestReceiverRateLimit_IgnoresForwardedFromUntrustedPeer proves
+// the receiver limiter uses the same gated key function as the
+// POST limiters.
+func TestReceiverRateLimit_IgnoresForwardedFromUntrustedPeer(
+	t *testing.T,
+) {
+	t.Parallel()
+
+	const (
+		limit = 3
+		peer  = "203.0.113.10:44444"
+		path  = "/webhook/uuid-d"
+	)
+
+	handler := receiverLimitedHandler(t, limit)
+
+	for i := range limit {
+		w := postWithHeaders(
+			handler, peer, path,
+			map[string]string{
+				headerXFF: fmt.Sprintf(
+					"198.51.100.%d", i+1,
+				),
+			},
+		)
+		assert.Equal(
+			t, http.StatusOK, w.Code,
+			"request %d should pass", i,
+		)
+	}
+
+	w := postWithHeaders(
+		handler, peer, path,
+		map[string]string{headerXFF: "198.51.100.200"},
+	)
+	assert.Equal(
+		t, http.StatusTooManyRequests, w.Code,
+		"a spoofed X-Forwarded-For from an untrusted peer must "+
+			"not mint a fresh receiver bucket",
+	)
+}

static/js/app.js

@@ -1,60 +1,2 @@
 // Webhooker client-side JavaScript
 console.log("Webhooker loaded");
-
-// Copy-to-clipboard, as progressive enhancement.
-//
-// Markup renders each copy button with the `hidden` attribute and a
-// `data-copy-target` pointing at the id of the element holding the
-// text. This script reveals a button only once it has both a resolvable
-// target and a usable Clipboard API, so a browser without either shows
-// no button at all and the text stays selectable.
-(function () {
-    "use strict";
-
-    const revertDelayMs = 2000;
-
-    function flash(button, message) {
-        const original = button.getAttribute("data-copy-label");
-        button.textContent = message;
-        window.setTimeout(function () {
-            button.textContent = original;
-        }, revertDelayMs);
-    }
-
-    function wire(button) {
-        const target = document.getElementById(
-            button.getAttribute("data-copy-target")
-        );
-        if (!target) {
-            return;
-        }
-
-        button.setAttribute("data-copy-label", button.textContent);
-        button.addEventListener("click", function () {
-            navigator.clipboard.writeText(target.textContent.trim()).then(
-                function () {
-                    flash(button, "Copied");
-                },
-                function () {
-                    flash(button, "Copy failed");
-                }
-            );
-        });
-        button.removeAttribute("hidden");
-    }
-
-    function init() {
-        if (!navigator.clipboard || !navigator.clipboard.writeText) {
-            return;
-        }
-
-        const buttons = document.querySelectorAll("[data-copy-target]");
-        buttons.forEach(wire);
-    }
-
-    if (document.readyState === "loading") {
-        document.addEventListener("DOMContentLoaded", init);
-    } else {
-        init();
-    }
-})();

templates/navbar.html

@@ -16,7 +16,7 @@
         <!-- Desktop navigation -->
         <div class="hidden md:flex items-center gap-4">
             {{if .User}}
-            <a href="/sources" class="btn-text">Webhooks</a>
+            <a href="/sources" class="btn-text">Sources</a>
             <a href="/user/{{.User.Username}}" class="btn-text">
                 <svg class="w-5 h-5 mr-1" fill="currentColor" viewBox="0 0 16 16">
                     <path d="M11 6a3 3 0 1 1-6 0 3 3 0 0 1 6 0z"/>
@@ -38,7 +38,7 @@
     <div x-show="open" x-cloak x-transition class="md:hidden mt-4 pt-4 border-t border-gray-200">
         <div class="flex flex-col gap-2">
             {{if .User}}
-            <a href="/sources" class="btn-text w-full text-left">Webhooks</a>
+            <a href="/sources" class="btn-text w-full text-left">Sources</a>
             <a href="/user/{{.User.Username}}" class="btn-text w-full text-left">Profile</a>
             <form method="POST" action="/pages/logout">
                 <input type="hidden" name="csrf_token" value="{{.CSRFToken}}">

templates/profile.html

@@ -34,6 +34,7 @@
 
         <hr class="border-gray-200 mb-6">
 
+        <div class="grid grid-cols-1 md:grid-cols-2 gap-8">
             <div>
                 <h3 class="text-lg font-medium text-gray-900 mb-3">Account Information</h3>
                 <dl class="space-y-3">
@@ -47,6 +48,11 @@
                     </div>
                 </dl>
             </div>
+            <div>
+                <h3 class="text-lg font-medium text-gray-900 mb-3">Settings</h3>
+                <p class="text-sm text-gray-500">Profile settings and preferences will be available here.</p>
+            </div>
+        </div>
     </div>
 
     <div class="card p-6 mt-6">

templates/source_detail.html

@@ -69,12 +69,7 @@
                             </form>
                         </div>
                     </div>
-                    <div class="flex items-start gap-2 mt-1">
+                    <code class="text-xs text-gray-500 break-all block mt-1">{{$.BaseURL}}/webhook/{{.Path}}</code>
-                        <code id="entrypoint-url-{{.ID}}" class="text-xs text-gray-500 break-all block flex-1">{{$.BaseURL}}/webhook/{{.Path}}</code>
-                        <!-- Hidden until app.js reveals it; without the
-                             script the URL above stays selectable. -->
-                        <button type="button" hidden data-copy-target="entrypoint-url-{{.ID}}" class="text-xs text-gray-500 hover:text-primary-600">Copy</button>
-                    </div>
                 </div>
                 {{else}}
                 <div class="p-4 text-sm text-gray-500">No entrypoints configured.</div>

templates/source_edit.html

@@ -29,7 +29,7 @@
             <div class="form-group">
                 <label for="retention_days" class="label">Retention (days)</label>
                 <input type="number" id="retention_days" name="retention_days" value="{{.Webhook.RetentionDays}}" min="0" class="input">
-                <p class="text-xs text-gray-500 mt-1">Currently {{.Webhook.RetentionLabel}}.{{if .Webhook.RetainsForever}} No events are deleted while retention is set to forever.{{else}} A periodic cleanup permanently deletes events older than this, along with their delivery records.{{end}} Enter 0 to retain events forever; leave blank to keep the current setting.</p>
+                <p class="text-xs text-gray-500 mt-1">Currently {{.Webhook.RetentionLabel}}. Enter 0 to retain events forever.</p>
             </div>
 
             <div class="flex gap-3">

templates/sources_list.html

@@ -1,6 +1,6 @@
 {{template "base" .}}
 
-{{define "title"}}Webhooks - Webhooker{{end}}
+{{define "title"}}Sources - Webhooker{{end}}
 
 {{define "content"}}
 <div class="max-w-6xl mx-auto px-6 py-8">

templates/sources_new.html

@@ -29,7 +29,7 @@
             <div class="form-group">
                 <label for="retention_days" class="label">Retention (days)</label>
                 <input type="number" id="retention_days" name="retention_days" value="{{.DefaultRetentionDays}}" min="0" class="input">
-                <p class="text-xs text-gray-500 mt-1">A periodic cleanup permanently deletes events older than this, along with their delivery records. Enter 0 to retain events forever; leave blank to use the default of {{.DefaultRetentionDays}} days.</p>
+                <p class="text-xs text-gray-500 mt-1">How long to keep event data. Enter 0 to retain events forever.</p>
             </div>
 
             <div class="flex gap-3">