Clarify web UI terminology, copy, and the entrypoint URL (closes #57)

Terminology: the nav labelled its link Sources while every heading said
Webhooks. Both nav links and the sources list page title now say
Webhooks, matching the product name and the database.Webhook model. The
/sources and /source/{id} routes are deliberately unchanged; renaming
them would break existing bookmarks for no gain, since the URL is not
what a user reads.

Profile: the "Settings" column held only placeholder copy promising
settings that would appear later. Password change is the only real
account setting and it already has its own card below, so the column is
removed and the two-column grid collapses to the single remaining one.

Retention copy on the create and edit forms now describes what the code
does rather than what the field implies. The reaper permanently deletes
events past the cutoff along with their deliveries and delivery
results, so the hint says so instead of "how long to keep event data".
Both forms state that an empty field is not a way to ask for forever:
the create path falls back to DefaultRetentionDays and the edit path
leaves the stored policy alone, while 0 is what BeforeSave rewrites to
the retain-forever sentinel.

Entrypoint URL copy button as progressive enhancement. The button is
rendered with the hidden attribute and a data-copy-target naming the
element that holds the URL. app.js reveals it only after confirming
both a resolvable target and a usable Clipboard API, so a browser
without either shows no dead control, and the URL is plain selectable
text in every case. The script uses const throughout, per the JS
styleguide the repo policies bind this repo to; nothing in the repo's
checks covers JavaScript, so that is enforced by reading rather than by
the gate.

Template rendering assertions cover the nav labels, the absence of any
remaining user-visible "Sources", the button's hidden-by-default
markup, and both forms' retention copy including the forever label, so
the copy cannot drift back silently. The edit-page assertions pass the
webhook as a pointer because RetentionLabel is a pointer method and a
map element is not addressable.

README.md

@@ -95,54 +95,6 @@ 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 `192.168.1.7` is accepted and treated as a single
-host), for example `192.168.1.7, 2001:db8::5`. It decides whose
-`X-Forwarded-For` header the rate limiters believe, so it should name
-the addresses of your reverse proxies and nothing else.
-
-`X-Forwarded-For` is 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 header is 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. A set but unparseable value aborts
-startup.
-
-`X-Real-IP` and `True-Client-IP` are **never** read, from any peer.
-Reverse proxies append to `X-Forwarded-For` but forward other client
-headers verbatim, so a single-valued header is client-controlled even
-behind a trusted proxy.
-
-Within a trusted request, `X-Forwarded-For` is read right to left,
-because the rightmost entry is the one the nearest proxy appended and
-everything left of it may have been written by the client. The first
-hop that is not itself a trusted proxy is taken as the client. A hop
-that is not a bare IP address — `ip:port`, a bracketed IPv6 literal,
-the token `unknown` — ends the walk and the peer address is used
-instead, since past such an entry the chain is not the shape assumed
-here. The peer address is likewise used when the header is absent or
-every hop in it is a trusted proxy.
-
-Two operator requirements follow:
-
-- Your proxy must **append** the peer address to `X-Forwarded-For`
-  (nginx `$proxy_add_x_forwarded_for`, HAProxy `option forwardfor`,
-  Caddy and AWS ALB by default), and must append a bare address with
-  no port.
-- List proxy hosts **only**. Any address inside `TRUSTED_PROXIES`
-  chooses its own rate-limit key: its `X-Forwarded-For` is walked, so
-  it can name a different address on every request to get a fresh
-  bucket each time, or name another client's address to drain that
-  client's bucket. Never list a block that also covers clients — a
-  broad `10.0.0.0/8` on a network where clients live in the same range
-  makes all three limits, including the unauthenticated webhook
-  receiver, silently bypassable by every client in the block.
 
 Sessions are bounded by two independent clocks, and end at whichever
 one runs out first:
@@ -172,9 +124,8 @@ 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,
+additionally be a number in the range 1–65535, and
-`RECEIVER_RATE_LIMIT` must be at least 1, and every entry in
+`RECEIVER_RATE_LIMIT` must be at least 1.
-`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`,
@@ -851,16 +802,6 @@ 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.

internal/config/config.go

@@ -5,10 +5,8 @@ import (
 	"errors"
 	"fmt"
 	"log/slog"
-	"net/netip"
 	"os"
 	"strconv"
-	"strings"
 	"time"
 
 	"go.uber.org/fx"
@@ -47,11 +45,6 @@ const (
 	// maxPort is the highest valid TCP port number. The lower
 	// bound (at least 1) is enforced by envPositiveInt.
 	maxPort = 65535
-
-	// mappedV4Offset is the number of leading bits an IPv4-mapped
-	// IPv6 prefix spends on the ::ffff:0:0/96 wrapper, so a /104
-	// covers the same addresses as an IPv4 /8.
-	mappedV4Offset = 96
 )
 
 // ErrInvalidEnvironment is returned when WEBHOOKER_ENVIRONMENT
@@ -66,11 +59,6 @@ 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
@@ -102,17 +90,6 @@ 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 X-Forwarded-For, the
-	// only forwarded header read. 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.
-	// Members can choose their own rate-limit key, so this must
-	// name proxy hosts only, never a block that also covers
-	// clients.
-	TrustedProxies []netip.Prefix
-
 	params *ConfigParams
 	log    *slog.Logger
 }
@@ -235,71 +212,6 @@ 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).
-//
-// Both forms are unmapped, because peer addresses are unmapped
-// before they are matched against the list: an IPv4-mapped prefix
-// left in that form would silently never match.
-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
-		}
-
-		if addr := prefix.Addr(); addr.Is4In6() &&
-			prefix.Bits() >= mappedV4Offset {
-			prefix = netip.PrefixFrom(
-				addr.Unmap(), prefix.Bits()-mappedV4Offset,
-			)
-		}
-
-		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) {
@@ -370,11 +282,6 @@ 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,
@@ -387,7 +294,6 @@ func loadFromEnv() (*Config, error) {
 		RetentionSweepInterval: retentionSweepInterval,
 		SessionIdleTimeout:     sessionIdleTimeout,
 		ReceiverRateLimit:      receiverRateLimit,
-		TrustedProxies:         trustedProxies,
 	}, nil
 }
 
@@ -429,7 +335,6 @@ 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

@@ -20,10 +20,6 @@ const (
 	caseUnsetUsesDefault = "unset uses default"
 	caseValidValueParsed = "valid value is parsed"
 	caseUnparseableFails = "unparseable value fails startup"
-
-	// cidrPrivateV4 is the sample trusted-proxy block the
-	// TRUSTED_PROXIES cases are built from.
-	cidrPrivateV4 = "10.0.0.0/8"
 )
 
 func TestEnvironmentConfig(t *testing.T) {
@@ -183,10 +179,9 @@ func TestRetentionSweepInterval(t *testing.T) {
 	}
 }
 
-// startupError builds the app config.New belongs to and returns
+// expectStartupError asserts that fx refuses to build the app,
-// the error fx reports, which is non-nil whenever an environment
+// which is what a set-but-invalid environment value must cause.
-// value is set but invalid.
+func expectStartupError(t *testing.T) {
-func startupError(t *testing.T) error {
 	t.Helper()
 
 	var cfg *config.Config
@@ -201,33 +196,7 @@ func startupError(t *testing.T) error {
 		fx.Populate(&cfg),
 	)
 
-	return app.Err()
+	assert.Error(t, 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(
@@ -382,11 +351,7 @@ func TestReceiverRateLimit(t *testing.T) {
 		set         bool
 		value       string
 		expectError bool
-		// sentinel, when set, must be wrapped by the startup
+		expected    int
-		// error; every error case must additionally name the
-		// variable in its message.
-		sentinel error
-		expected int
 	}{
 		{
 			name:     caseUnsetUsesDefault,
@@ -410,14 +375,12 @@ 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,
 		},
 	}
 
@@ -436,9 +399,7 @@ func TestReceiverRateLimit(t *testing.T) {
 			}
 
 			if tt.expectError {
-				expectStartupErrorFor(
+				expectStartupError(t)
-					t, "RECEIVER_RATE_LIMIT", tt.sentinel,
-				)
 			} else {
 				testReceiverRateLimitSuccess(t, tt.expected)
 			}
@@ -471,116 +432,3 @@ 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: cidrPrivateV4 + ", 192.168.1.7 ,2001:db8::/32",
-			expected: []string{
-				cidrPrivateV4, "192.168.1.7/32", "2001:db8::/32",
-			},
-		},
-		{
-			name:     "host bits are masked off",
-			set:      true,
-			value:    "10.1.2.3/8",
-			expected: []string{cidrPrivateV4},
-		},
-		{
-			// Peer addresses are unmapped before they are
-			// matched, so an IPv4-mapped prefix kept in that
-			// form could never match anything.
-			name:     "IPv4-mapped prefix is unmapped",
-			set:      true,
-			value:    "::ffff:10.0.0.0/104",
-			expected: []string{cidrPrivateV4},
-		},
-		{
-			name:        caseUnparseableFails,
-			set:         true,
-			value:       cidrPrivateV4 + ",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/middleware/ratelimit.go

@@ -2,9 +2,6 @@ package middleware
 
 import (
 	"net/http"
-	"net/netip"
-	"slices"
-	"strings"
 	"time"
 
 	"github.com/go-chi/httprate"
@@ -34,120 +31,13 @@ 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 X-Forwarded-For chain. It is consulted only for requests
-// whose direct peer is a trusted proxy.
-//
-// X-Forwarded-For is the only header read. X-Real-IP and
-// True-Client-IP are deliberately ignored: the reverse proxies in
-// common use append to X-Forwarded-For and pass any other header the
-// client sent through untouched, so believing a single-valued header
-// would let a client behind the trusted proxy name its own bucket —
-// the very bypass this gating exists to close.
-//
-// The chain is walked right to left, because the rightmost entry is
-// the one the nearest proxy appended and everything to its left may
-// have been written by the client. The first hop that is not itself
-// a trusted proxy is the client. A hop that cannot be read as a bare
-// address ends the walk: past it the chain is not the shape assumed
-// here, so the caller falls back to the peer address.
-func (m *Middleware) forwardedClientAddr(
-	r *http.Request,
-) (netip.Addr, bool) {
-	hops := strings.Split(
-		strings.Join(r.Header.Values("X-Forwarded-For"), ","), ",",
-	)
-
-	for _, hop := range slices.Backward(hops) {
-		hop = strings.TrimSpace(hop)
-		if hop == "" {
-			continue
-		}
-
-		addr, err := netip.ParseAddr(hop)
-		if err != nil {
-			return netip.Addr{}, false
-		}
-
-		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. Clients are
+// a 429 Too Many Requests response is returned. IP extraction
-// identified by rateLimitKey.
+// honours X-Forwarded-For, X-Real-IP, and True-Client-IP headers
+// for reverse-proxy setups.
 func (m *Middleware) LoginRateLimit() func(http.Handler) http.Handler {
 	return m.postRateLimit(
 		loginRateLimit,
@@ -176,7 +66,9 @@ 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. Clients are identified by rateLimitKey.
+// given log message. IP extraction honours X-Forwarded-For,
+// X-Real-IP, and True-Client-IP headers for reverse-proxy
+// setups.
 func (m *Middleware) postRateLimit(
 	limit int,
 	interval time.Duration,
@@ -185,10 +77,19 @@ func (m *Middleware) postRateLimit(
 	limiter := httprate.Limit(
 		limit,
 		interval,
-		httprate.WithKeyFuncs(m.rateLimitKey),
+		httprate.WithKeyFuncs(httprate.KeyByRealIP),
-		httprate.WithLimitHandler(
+		httprate.WithLimitHandler(http.HandlerFunc(
-			m.tooManyRequests(logMessage, responseMessage),
+			func(w http.ResponseWriter, r *http.Request) {
-		),
+				m.log.Warn(logMessage,
+					"path", r.URL.Path,
+				)
+				http.Error(
+					w,
+					responseMessage,
+					http.StatusTooManyRequests,
+				)
+			},
+		)),
 	)
 
 	return func(next http.Handler) http.Handler {
@@ -215,19 +116,31 @@ 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;
-// Clients are identified by rateLimitKey.
+// httprate adds the Retry-After header (RFC 6585). IP
+// 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(
-			m.rateLimitKey,
+			httprate.KeyByRealIP,
 			httprate.KeyByEndpoint,
 		),
-		httprate.WithLimitHandler(m.tooManyRequests(
+		httprate.WithLimitHandler(http.HandlerFunc(
-			"webhook receiver rate limit exceeded",
+			func(w http.ResponseWriter, r *http.Request) {
-			"Too many requests. Please slow down.",
+				m.log.Warn(
+					"webhook receiver rate limit exceeded",
+					"path", r.URL.Path,
+				)
+				http.Error(
+					w,
+					"Too many requests. "+
+						"Please slow down.",
+					http.StatusTooManyRequests,
+				)
+			},
 		)),
 	)
 }

internal/middleware/ratelimit_test.go

@@ -2,11 +2,9 @@ package middleware_test
 
 import (
 	"context"
-	"fmt"
 	"log/slog"
 	"net/http"
 	"net/http/httptest"
-	"net/netip"
 	"os"
 	"testing"
 
@@ -184,22 +182,11 @@ func TestLoginRateLimit_IndependentPerIP(t *testing.T) {
 	)
 }
 
-// okHandler is the terminal handler the limiter middleware wraps
+// receiverLimitedHandler builds a ReceiverRateLimit-wrapped
-// in these tests: it answers 200 to anything that reaches it.
+// handler with the given per-minute limit.
-func okHandler() http.Handler {
+func receiverLimitedHandler(
-	return http.HandlerFunc(
+	t *testing.T, limit int,
-		func(w http.ResponseWriter, _ *http.Request) {
+) http.Handler {
-			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(
@@ -207,53 +194,17 @@ func rateLimitMiddleware(
 		&slog.HandlerOptions{Level: slog.LevelDebug},
 	))
 
-	return middleware.NewForTest(log, cfg, nil)
+	m := middleware.NewForTest(
-}
+		log,
-
+		&config.Config{ReceiverRateLimit: limit},
-// trustedProxies parses CIDR strings for a test Config.
+		nil,
-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()(okHandler())
+	return m.ReceiverRateLimit()(http.HandlerFunc(
+		func(w http.ResponseWriter, _ *http.Request) {
+			w.WriteHeader(http.StatusOK)
+		},
+	))
 }
 
 // receiverPost sends one POST to the handler from the given IP
@@ -360,252 +311,3 @@ 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"
-)
-
-// assertSharedBucket drives the login limiter from peer with the
-// trusted-proxy set proxies, sending one more request than the limit
-// allows and varying the headers on each with headers(i). Every
-// request must land in the same bucket, so the last one is rejected:
-// if any of the varying header values reached the key, the run would
-// have minted fresh buckets and nothing would be rejected.
-func assertSharedBucket(
-	t *testing.T,
-	proxies []netip.Prefix,
-	peer string,
-	headers func(i int) map[string]string,
-	msg string,
-) {
-	t.Helper()
-
-	m := rateLimitMiddleware(
-		t, &config.Config{TrustedProxies: proxies},
-	)
-	handler := m.LoginRateLimit()(okHandler())
-
-	for i := range middleware.LoginRateLimitConst {
-		w := postWithHeaders(handler, peer, loginPath, headers(i))
-		assert.Equal(
-			t, http.StatusOK, w.Code,
-			"request %d should pass", i,
-		)
-	}
-
-	w := postWithHeaders(
-		handler, peer, loginPath,
-		headers(middleware.LoginRateLimitConst),
-	)
-	assert.Equal(t, http.StatusTooManyRequests, w.Code, msg)
-}
-
-// 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()
-
-			assertSharedBucket(
-				t, nil, "203.0.113.9:44444",
-				func(i int) map[string]string {
-					return map[string]string{
-						header: fmt.Sprintf(
-							"198.51.100.%d", i+1,
-						),
-					}
-				},
-				"a spoofed "+header+" from an untrusted peer "+
-					"must not mint a fresh bucket",
-			)
-		})
-	}
-}
-
-// TestRateLimitKey_SingleValuedHeadersIgnoredFromTrustedPeer is the
-// regression test for the bypass hiding inside the trusted case.
-// Real reverse proxies (nginx, HAProxy, Caddy, ALB) set only
-// X-Forwarded-For and pass every other client header through
-// verbatim, so a client behind the configured proxy can send its own
-// X-Real-IP or True-Client-IP. Reading either would hand that client
-// a fresh bucket per request from inside exactly the deployment
-// TRUSTED_PROXIES exists to serve, so neither header is read at all.
-func TestRateLimitKey_SingleValuedHeadersIgnoredFromTrustedPeer(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	for _, header := range []string{headerReal, headerTrue} {
-		t.Run(header, func(t *testing.T) {
-			t.Parallel()
-
-			assertSharedBucket(
-				t, trustedProxies("10.0.0.0/8"),
-				"10.0.0.1:44444",
-				func(i int) map[string]string {
-					return map[string]string{
-						header: fmt.Sprintf(
-							"198.51.100.%d", i+1,
-						),
-					}
-				},
-				header+" from a trusted peer must not mint a "+
-					"fresh bucket: only X-Forwarded-For is read",
-			)
-		})
-	}
-}
-
-// TestRateLimitKey_MalformedRightmostHopFallsBackToPeer covers the
-// other end of the chain walk. The rightmost X-Forwarded-For entry
-// is the one the trusted proxy appended; if it cannot be read as an
-// address the chain is not the shape the walk assumes, and every
-// entry to its left may have come from the client. The walk must
-// stop and fall back to the peer rather than select one of them.
-func TestRateLimitKey_MalformedRightmostHopFallsBackToPeer(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	// Forms seen in the wild: host:port (Azure Application
-	// Gateway, IIS ARR), a bracketed IPv6 literal, and the
-	// RFC 7239 placeholder token.
-	for _, tail := range []string{
-		"198.51.100.7:1234", "[2001:db8::1]", "unknown",
-	} {
-		t.Run(tail, func(t *testing.T) {
-			t.Parallel()
-
-			assertSharedBucket(
-				t, trustedProxies("10.0.0.0/8"),
-				"10.0.0.1:44444",
-				func(i int) map[string]string {
-					return map[string]string{
-						headerXFF: fmt.Sprintf(
-							"9.9.9.%d, %s", i+1, tail,
-						),
-					}
-				},
-				"an unparseable rightmost hop must fall back "+
-					"to the peer address, not select a "+
-					"client-controlled entry",
-			)
-		})
-	}
-}
-
-// 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()
-
-	assertSharedBucket(
-		t, trustedProxies("10.0.0.0/8"), "10.0.0.1:44444",
-		func(i int) map[string]string {
-			return map[string]string{
-				headerXFF: fmt.Sprintf(
-					"9.9.9.%d, 198.51.100.7, 10.0.0.2", i+1,
-				),
-			}
-		},
-		"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",
-	)
-}