Correct release-blocking documentation inaccuracies (closes #141)

Four defects found by the integration review, each of which would have
made the README or the release notes untrue at the moment of tagging.

RETENTION_SWEEP_INTERVAL was absent from the README env table while two
other passages referred to it as documented. Enumerated every variable
read by internal/config from the source (12 in total) rather than by
eye; that was the only one missing.

TODO.md omitted five of the units landed in this milestone (#64, #79,
#90, #113, #118), two of them credential-exposure fixes, which are
precisely the entries a reader of the release notes wants to find. The
list is now derived from git log origin/main..origin/next.

The README sold Replay in the present tense as a core capability while
no redelivery code exists anywhere in the tree, and the roadmap entry
for it had been dropped without it being implemented. Every mention of
replay or redelivery in the README is now either marked planned or
already under a Planned heading: the Rationale item, the Use Cases
bullet, the Event model's field description, and the delivery-semantics
passage on target-type edits, which told an operator that a terminally
failed delivery was recoverable by hand when nothing can recover it.
The comment on failUnretryableRetry made the same claim and is
corrected with it. The roadmap entry is back in TODO.md.

The production JS asset shipped a console.log on load. The rest of the
file and every other shipped asset were checked; that was the only one
(alpine.min.js is vendored and untouched).

No behavioural change: the only non-comment, non-documentation edit is
the deleted console.log.

.dockerignore

@@ -1,6 +1,3 @@
-# .ci-fingerprint is deliberately NOT excluded: it is the CI cache barrier
-# that keeps the check stages from replaying a cached pass. See the lint
-# stage of the Dockerfile.
 .git/
 bin/
 *.md

.gitea/workflows/check.yml

@@ -11,53 +11,5 @@ jobs:
     steps:
       - name: Checkout
         uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 2024-10-23
-        with:
-          # The fingerprint step below needs history to find the last commit
-          # that touched the Docker build context.
-          fetch-depth: 0
-
-      - name: Neutralize superseded run statuses
-        # Gitea cancels the in-flight run when another commit is pushed to the
-        # same branch and records the cancellation as `failure`, so a commit
-        # that was never tested reads red. The cancellation is unconditional
-        # server-side for push events and cannot be disabled from a workflow
-        # file, so the superseding run rewrites those statuses to `skipped`.
-        # Only the exact cancellation status is touched; a real failure is
-        # left alone.
-        env:
-          GITEA_TOKEN: ${{ secrets.GITEA_TOKEN }}
-        run: |
-          set -eu
-          api="${GITHUB_API_URL}/repos/${GITHUB_REPOSITORY}"
-          ctx='check / check (push)'
-          for sha in $(git rev-list --max-count=20 "${GITHUB_SHA}^" || true); do
-            latest="$(curl -sf "${api}/commits/${sha}/status" | jq -r \
-              --arg c "$ctx" \
-              '[.statuses[] | select(.context == $c)][0] // empty
-               | "\(.status)|\(.description)"')" || continue
-            [ "$latest" = 'failure|Has been cancelled' ] || continue
-            curl -sf -X POST "${api}/statuses/${sha}" \
-              -H "Authorization: token ${GITEA_TOKEN}" \
-              -H 'Content-Type: application/json' \
-              -d "$(jq -nc --arg c "$ctx" '{
-                context: $c,
-                state: "skipped",
-                description: "Superseded by a newer commit; never tested"
-              }')" >/dev/null
-            echo "neutralized superseded status on ${sha}"
-          done
-
-      - name: Fingerprint the build context
-        # `.dockerignore` keeps docs out of the build context, so a docs-only
-        # commit legitimately replays the whole image from cache and stays
-        # cheap. Every other commit writes a new fingerprint into the context,
-        # which invalidates the `COPY . .` layer of both check stages: a
-        # commit that was never linted, formatted-checked, tested and built
-        # cannot report success from cache.
-        run: |
-          set -eu
-          fp="$(git log -1 --format=%H -- . ':!*.md' ':!LICENSE' ':!.editorconfig')"
-          printf '%s\n' "${fp:-$GITHUB_SHA}" > .ci-fingerprint
-
       - name: Build Docker image (runs make check)
         run: script/cibuild

.gitignore

@@ -41,7 +41,4 @@ data/
 
 # Temporary files
 tmp/
-temp/
-
-# CI cache barrier, written into the build context by the check workflow
-.ci-fingerprint 
+temp/ 

Dockerfile

@@ -12,11 +12,7 @@ WORKDIR /src
 COPY go.mod go.sum ./
 RUN go mod download
 
-# Copy source code. In CI the context also carries .ci-fingerprint, whose
-# value changes with every commit that touches the build context (see
-# .gitea/workflows/check.yml). That invalidates this layer, so the checks
-# below cannot report success by replaying a cached pass. Do not add it to
-# .dockerignore.
+# Copy source code
 COPY . .
 
 # Run formatting check and linter
@@ -40,8 +36,7 @@ WORKDIR /build
 COPY go.mod go.sum ./
 RUN go mod download
 
-# Copy source code, including the .ci-fingerprint cache barrier described in
-# the lint stage above.
+# Copy source code
 COPY . .
 
 # Run tests and build

README.md

@@ -95,8 +95,8 @@ TTY detection, and security headers are always applied.
 | `SENTRY_DSN`            | Sentry error reporting DSN          | `""`     |
 | `RETENTION_SWEEP_INTERVAL` | How often the retention reaper and archive sweeper run (Go duration, must be positive) | `1h` |
 | `SESSION_IDLE_TIMEOUT`  | Idle session timeout (Go duration)  | `24h`    |
-| `RECEIVER_RATE_LIMIT`   | Receiver requests/minute per IP per entrypoint (10x that per IP across the route) | `120` |
-| `TRUSTED_PROXIES`       | CIDRs whose forwarded headers are trusted (unset: all clients behind a proxy share one rate-limit bucket) | `""` (none) |
+| `RECEIVER_RATE_LIMIT`   | Receiver requests/minute per IP per entrypoint | `120` |
+| `TRUSTED_PROXIES`       | CIDRs whose forwarded headers are trusted | `""` (none) |
 
 #### Trusted proxies
 
@@ -115,23 +115,6 @@ 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.
 
-That default is safe against forged headers, but leaving it unset in
-production has a cost you must know about. Production runs behind a
-TLS-terminating reverse proxy, so with `TRUSTED_PROXIES` unset every
-request keys on the proxy's own address and all clients share a single
-bucket per limit. For the login and password-change limits that is a
-denial of service anyone can perform: a steady five POSTs per minute
-from any address on the internet keeps the shared login bucket full,
-and the operator's own login then returns HTTP 429 for as long as the
-trickle continues. There is no second administrative path and no
-bypass. Restarting the service clears the in-memory buckets, but a
-sustained trickle re-locks them immediately.
-
-The remedy is to set `TRUSTED_PROXIES` to your reverse proxy's
-address, which restores per-client buckets. webhooker logs a warning
-at startup when `WEBHOOKER_ENVIRONMENT=prod` and `TRUSTED_PROXIES` is
-empty. See [Rate Limiting](#rate-limiting) for what each limit shares.
-
 `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
@@ -876,60 +859,15 @@ 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.
 
-A second limit sits in front of that one, keyed on the client IP alone
-and covering the whole route at ten times `RECEIVER_RATE_LIMIT` requests
-per minute (default 1200). The per-entrypoint limit needs it: the route
-pattern matches any single path segment, so a client that invents a
-fresh path per request gets a fresh per-entrypoint bucket every time and
-would otherwise have no aggregate limit at all — while each of those
-requests still costs an entrypoint lookup before it 404s. The aggregate
-limit leaves room for one address to drive several entrypoints at their
-full rate, and it is not configurable separately.
-
-What that aggregate limit bounds is the database work an invented path
-costs; log volume it caps rather than eliminates. A path that names no
-entrypoint is recorded by the handler at `DEBUG`, and the aggregate
-limiter logs its own rejections at `DEBUG` and without the path, so
-neither appears at all under the default level. The per-entrypoint
-limiter is the loud one: it still logs every rejection at `WARN` with
-the request path, which on this route is attacker-controlled text. A
-client hammering a single invented path is served `RECEIVER_RATE_LIMIT`
-requests and has the rest of its aggregate budget rejected there, so
-the aggregate limit is what bounds those `WARN` lines — to under ten
-times `RECEIVER_RATE_LIMIT` per minute per client IP, 1080 at the
-defaults, where before it there was no bound at all. The access log is
-bounded by neither limit: every request is recorded once at `INFO` with
-its full URL, served or rejected alike.
-
 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. Set `TRUSTED_PROXIES` to the
-proxy's address to get per-client limits back. What the shared bucket
-costs is not the same for every limiter, and the two cases pull in
-opposite directions:
-
-- For the **receiver** limits it costs throughput, which is the safe
-  direction to be wrong in: sharing can only make a limit bind sooner,
-  never let a sender past it. It matters more for the aggregate limit
-  than for the per-entrypoint one: with `TRUSTED_PROXIES` unset behind
-  the reverse proxy a production deployment is required to run behind,
-  every request keys on the proxy, so the aggregate limit becomes a
-  service-wide ceiling of 1200 requests per minute across all senders
-  and all entrypoints, where the per-entrypoint limit's capacity still
-  grows with the number of entrypoints. Any deployment with more than a
-  handful of busy entrypoints must set `TRUSTED_PROXIES`.
-- For the **login and password-change** limits it costs availability of
-  the only administrative path, which is not safe at all. Five POSTs
-  per minute from any address on the internet keeps the single shared
-  login bucket full, and the operator's own login returns HTTP 429 for
-  as long as that trickle continues. A restart clears the in-memory
-  buckets and a resumed trickle re-locks them. Production deployments
-  must set `TRUSTED_PROXIES`; webhooker warns at startup when it is
-  empty in `prod`.
+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
@@ -1148,11 +1086,8 @@ downstream at form-parse time.
   (custom HTTP transport with SSRF-safe dialer that validates resolved
   IPs before connecting, preventing DNS rebinding attacks)
 - **Login rate limiting** via [go-chi/httprate](https://github.com/go-chi/httprate):
-  sliding-window rate limiter on the login endpoint, 5 POST attempts
-  per minute per bucket, to slow brute-force attacks. The bucket is per
-  client IP only when `TRUSTED_PROXIES` names the reverse proxy;
-  unset, every client shares one bucket and the login becomes remotely
-  deniable (see [Rate Limiting](#rate-limiting))
+  per-IP sliding-window rate limiter on the login endpoint (5 POST
+  attempts per minute per IP) to prevent brute-force attacks
 - Prometheus metrics behind basic auth
 - Static assets embedded in binary (no filesystem access needed at
   runtime)
@@ -1177,34 +1112,6 @@ binary is statically linked and runs on Alpine.
 `docker build .` is the CI gate — if it passes, the code is formatted,
 linted, tested, and compiled.
 
-#### CI gate honesty
-
-A layer cache lets `docker build .` exit 0 in seconds with the lint and
-test stages replayed rather than executed, which would make a green
-check meaningless. The `check` workflow therefore writes
-`.ci-fingerprint` into the build context before building. Its value is
-the hash of the last commit that touched the build context, so:
-
-- Any commit that changes code (including a squash merge whose tree
-  matches an already-built branch) gets a new fingerprint, invalidates
-  the `COPY . .` layer of both check stages, and really runs
-  `make fmt-check`, `make lint`, `make test`, and `make build`. A run
-  that reports success ran them.
-- A docs-only commit leaves the fingerprint unchanged — `.dockerignore`
-  excludes `*.md` and `LICENSE` from the context anyway — so the image
-  replays from cache and costs seconds.
-
-The module download layer sits above `COPY . .` and stays cached either
-way.
-
-The workflow's first step covers a second way the gate lied: Gitea
-cancels an in-flight run when a newer commit lands on the same branch
-and records that cancellation as a `failure` status, marking a commit
-red that was never tested. Cancellation is unconditional server-side for
-push events, so the superseding run rewrites the exact
-`Has been cancelled` status to `skipped`. Genuine failures are never
-touched.
-
 ## TODO
 
 See [TODO.md](TODO.md).

TODO.md

@@ -24,52 +24,22 @@ event retention (#63), the database archiving target (#43), the admin
 password change flow (#65), policy compliance (#6), pinned lint tooling
 (#55), and fail-loud configuration parsing (#80).
 
-`next` holds the completed 1.0.0 milestone: every issue in it is closed,
-and it is verified green both by CI and by cache-defeated container
-runs. The two were only made to mean the same thing this cycle — before
-#119, a warm layer cache let the gate report success without executing
-anything, and replayed the previous build's console log so the lie
-looked like a real run. Note: TODO.md was deliberately deleted from this
-repo in f9a9569 (2026-03-01, #6); its content was folded into the README
-TODO section, which this draft reconstructs as of 2026-07-06.
+`next` (9bfd033) holds the completed 1.0.0 milestone: every issue in it
+is closed, and it is verified green by cache-defeated container runs
+rather than by the CI badge, which can pass without executing anything
+(#119). Note: TODO.md was deliberately deleted from this repo in f9a9569
+(2026-03-01, #6); its content was folded into the README TODO section,
+which this draft reconstructs as of 2026-07-06.
 
 # Next Step
 
-Merge the milestone PR to `main` and tag 1.0.0 from it.
-
-Two decisions are open and belong to the owner, neither blocking the
-tag: #115 (mask the `http` target's destination URL, implemented
-speculatively and awaiting a yes or no) and #125 (whether IPv6
-rate-limit keys should bucket by `/64`).
+Tag 1.0.0 from `main` once the milestone PR merges, then repair the CI
+gate (#119) before the next cycle's work lands — a gate that can report
+success without running is the one thing every other guarantee here
+rests on.
 
 # Completed Steps
 
-- 2026-08-12 Bound the receiver rate limit per client IP across the
-  whole `/webhook/*` route. The existing limiter keyed on the request
-  path and `/webhook/{uuid}` matches any single segment, so a client
-  that invented a fresh path per request minted a fresh bucket per
-  request: the limit on the only unauthenticated endpoint bounded
-  nothing in aggregate, and every request still cost an entrypoint
-  lookup before it 404ed. An outer limiter keyed on the client address
-  alone now bounds that, chained in front of the unchanged
-  per-entrypoint limiter (#139)
-- 2026-08-12 Correct release-blocking documentation inaccuracies: the
-  README promised manual redelivery in the present tense in three
-  places when nothing implements it (the same false claim also sat in
-  the doc comment that was its source text), the env table omitted
-  `RETENTION_SWEEP_INTERVAL`, and `TODO.md` itself omitted five landed
-  units (#141)
-- 2026-08-12 Make the CI gate execute the checks it reports on. The
-  workflow now writes a build-context fingerprint before calling
-  `script/cibuild`, so a code commit invalidates the `COPY` layer of
-  the lint and builder stages while a docs-only commit still replays
-  from cache; a superseding run also rewrites the `failure` status
-  Gitea leaves on commits it cancelled and never tested. Verified by
-  pushing a deliberately broken test and watching CI go red (#119)
-- 2026-08-12 Require a positive `RETENTION_SWEEP_INTERVAL`: a
-  non-positive value reached `time.NewTicker` in both the retention
-  reaper and the archive sweeper, panicking two goroutines with no
-  recover after startup had already reported success (#140)
 - 2026-08-12 Bound the `X-Forwarded-For` scan's allocation to the hop
   cap: the reverse walk cuts entries with `strings.LastIndexByte`
   instead of joining and splitting, so a 1 MB header allocates 16 bytes

internal/config/config.go

@@ -422,38 +422,6 @@ func loadFromEnv() (*Config, error) {
 	}, nil
 }
 
-// warnSharedRateLimitBucket logs a startup warning when a production
-// deployment leaves TRUSTED_PROXIES empty.
-//
-// With no trusted proxies every rate limiter keys on the connecting
-// peer's address. A production deployment is required to run behind a
-// TLS-terminating reverse proxy, and the peer is then that proxy for
-// every request, so all clients share one bucket per limiter. The
-// login limiter's bucket is the dangerous one: any remote client can
-// keep it full, which denies the only administrative login to
-// everyone until the process restarts.
-//
-// The default of trusting nobody is deliberate — trusting forwarded
-// headers from arbitrary peers lets any client choose its own bucket —
-// so this warns rather than failing startup or changing the key.
-func (c *Config) warnSharedRateLimitBucket(log *slog.Logger) {
-	if !c.IsProd() || len(c.TrustedProxies) > 0 {
-		return
-	}
-
-	log.Warn(
-		"TRUSTED_PROXIES is empty: rate limits key on the "+
-			"connecting peer, so behind the reverse proxy a "+
-			"production deployment runs behind, every client "+
-			"shares one bucket per limit. Any remote client can "+
-			"then keep the login limit full and deny the admin "+
-			"login, the only administrative path, until restart. "+
-			"Set TRUSTED_PROXIES to your reverse proxy's address.",
-		"environment", c.Environment,
-		"trustedProxies", len(c.TrustedProxies),
-	)
-}
-
 // New creates a Config by reading environment variables.
 //
 //nolint:revive // lc parameter is required by fx even if unused.
@@ -498,7 +466,5 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
 		s.MetricsUsername != "" && s.MetricsPassword != "",
 	)
 
-	s.warnSharedRateLimitBucket(log)
-
 	return s, nil
 }

internal/config/config_test.go

@@ -1,8 +1,6 @@
 package config_test
 
 import (
-	"bytes"
-	"log/slog"
 	"os"
 	"testing"
 	"time"
@@ -626,79 +624,3 @@ func testTrustedProxiesSuccess(
 
 	assert.Equal(t, expected, got)
 }
-
-// TestSharedRateLimitBucketWarning covers the startup warning that
-// tells an operator their production deployment shares one rate-limit
-// bucket between every client, which makes the admin login remotely
-// deniable. It must fire when TRUSTED_PROXIES is empty in production
-// and stay quiet otherwise.
-func TestSharedRateLimitBucketWarning(t *testing.T) {
-	tests := []struct {
-		name           string
-		environment    string
-		trustedProxies string
-		expectWarning  bool
-	}{
-		{
-			name:          "prod without trusted proxies warns",
-			environment:   config.EnvironmentProd,
-			expectWarning: true,
-		},
-		{
-			name:           "prod with trusted proxies is quiet",
-			environment:    config.EnvironmentProd,
-			trustedProxies: cidrPrivateV4,
-			expectWarning:  false,
-		},
-		{
-			// Development is not required to run behind a
-			// reverse proxy, so the shared bucket the warning
-			// describes is not the expected shape there.
-			name:          "dev without trusted proxies is quiet",
-			environment:   config.EnvironmentDev,
-			expectWarning: false,
-		},
-	}
-
-	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", tt.environment)
-
-			if tt.trustedProxies == "" {
-				require.NoError(
-					t, os.Unsetenv("TRUSTED_PROXIES"),
-				)
-			} else {
-				t.Setenv("TRUSTED_PROXIES", tt.trustedProxies)
-			}
-
-			var buf bytes.Buffer
-
-			log := slog.New(slog.NewJSONHandler(
-				&buf, &slog.HandlerOptions{
-					Level: slog.LevelDebug,
-				},
-			))
-
-			require.NoError(
-				t,
-				config.WarnSharedRateLimitBucketForTest(log),
-			)
-
-			if !tt.expectWarning {
-				assert.Empty(t, buf.String())
-
-				return
-			}
-
-			logged := buf.String()
-
-			assert.Contains(t, logged, `"level":"WARN"`)
-			assert.Contains(t, logged, "TRUSTED_PROXIES")
-			assert.Contains(t, logged, "shares one bucket")
-			assert.Contains(t, logged, "deny the admin login")
-		})
-	}
-}

internal/config/export_test.go

@@ -1,26 +1,9 @@
 package config
 
-import "log/slog"
-
 // This file exposes the unexported environment parsing helpers to
 // the external config_test package so each helper can be covered by
 // its own table-driven test without weakening the package API.
 
-// WarnSharedRateLimitBucketForTest loads a Config from the current
-// environment and emits its startup warnings to log. The real logger
-// writes to stdout, so this lets the warning's firing condition be
-// asserted against a handler the test controls.
-func WarnSharedRateLimitBucketForTest(log *slog.Logger) error {
-	c, err := loadFromEnv()
-	if err != nil {
-		return err
-	}
-
-	c.warnSharedRateLimitBucket(log)
-
-	return nil
-}
-
 // EnvBoolForTest exposes envBool.
 func EnvBoolForTest(key string, defaultValue bool) (bool, error) {
 	return envBool(key, defaultValue)

internal/handlers/webhook.go

@@ -39,6 +39,12 @@ func (h *Handlers) HandleWebhook() http.HandlerFunc {
 			return
 		}
 
+		h.log.Info("webhook request received",
+			"entrypoint_uuid", entrypointUUID,
+			"method", r.Method,
+			"remote_addr", r.RemoteAddr,
+		)
+
 		entrypoint, ok := h.lookupEntrypoint(
 			w, r, entrypointUUID,
 		)
@@ -46,18 +52,6 @@ func (h *Handlers) HandleWebhook() http.HandlerFunc {
 			return
 		}
 
-		// Logged only once the UUID is known to name a real
-		// entrypoint. The UUID comes straight out of the path on
-		// the one unauthenticated endpoint, so logging it before
-		// the lookup let a client write an INFO line per invented
-		// path; the request itself is already in the access log
-		// and a miss is already logged at DEBUG.
-		h.log.Info("webhook request received",
-			"entrypoint_uuid", entrypointUUID,
-			"method", r.Method,
-			"remote_addr", r.RemoteAddr,
-		)
-
 		if !entrypoint.Active {
 			http.Error(w, "Gone", http.StatusGone)
 

internal/middleware/export_test.go

@@ -41,13 +41,3 @@ const LoginRateLimitConst = loginRateLimit
 // PasswordChangeRateLimitConst exposes the
 // passwordChangeRateLimit constant.
 const PasswordChangeRateLimitConst = passwordChangeRateLimit
-
-// ReceiverAggregateMultiplierConst exposes the
-// receiverAggregateMultiplier constant.
-const ReceiverAggregateMultiplierConst = receiverAggregateMultiplier
-
-// ReceiverAggregateLimitForTest exposes receiverAggregateLimit for
-// testing.
-func ReceiverAggregateLimitForTest(perEntrypoint int) int {
-	return receiverAggregateLimit(perEntrypoint)
-}

internal/middleware/ratelimit.go

@@ -1,7 +1,6 @@
 package middleware
 
 import (
-	"math"
 	"net/http"
 	"net/netip"
 	"slices"
@@ -34,14 +33,6 @@ const (
 	// requests per minute.
 	receiverRateInterval = 1 * time.Minute
 
-	// receiverAggregateMultiplier scales the configured
-	// per-entrypoint receiver limit into the aggregate limit one
-	// client IP may spend across the whole /webhook/* route. Ten
-	// entrypoints' worth lets a single sender address drive several
-	// entrypoints at their full rate, while still capping what one
-	// address costs the unauthenticated receiver.
-	receiverAggregateMultiplier = 10
-
 	// maxForwardedHops bounds how many X-Forwarded-For entries the
 	// chain walk examines. Real chains are one to three hops, but a
 	// client can pad the header up to MaxHeaderBytes, so without a
@@ -171,11 +162,9 @@ func (m *Middleware) clientKey(r *http.Request) string {
 	return peer.String()
 }
 
-// tooManyRequests returns the 429 handler used by the login,
-// password-change and per-entrypoint receiver limiters: it logs the
-// rejection with logMessage and answers with responseMessage.
-// httprate adds the Retry-After header (RFC 6585). The aggregate
-// receiver limiter uses floodTooManyRequests instead.
+// 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 {
@@ -185,31 +174,6 @@ func (m *Middleware) tooManyRequests(
 	}
 }
 
-// floodTooManyRequests returns the 429 handler for a limiter whose
-// rejections are themselves the flood: it logs at DEBUG and without
-// the path, then answers with responseMessage.
-//
-// The aggregate receiver limiter trips exactly when one address is
-// sending faster than the receiver wants to serve, so its rejection
-// log is one line per request of that flood. At WARN with "path" that
-// hands a client a way to write its own text into the operator's log,
-// at a level that trips alerting, once per request — the log-volume
-// problem this limiter exists to bound. DEBUG is off in production by
-// default, so a flood costs nothing here; the path is dropped so that
-// turning DEBUG on to diagnose one does not restore the problem.
-//
-// This limiter bounds the database work an invented path costs, not
-// the number of log lines it produces: the access log in
-// middleware.go still records every request, served or rejected.
-func (m *Middleware) floodTooManyRequests(
-	logMessage, responseMessage string,
-) http.HandlerFunc {
-	return func(w http.ResponseWriter, _ *http.Request) {
-		m.log.Debug(logMessage)
-		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
@@ -278,26 +242,15 @@ func (m *Middleware) postRateLimit(
 	}
 }
 
-// ReceiverRateLimit returns middleware that rate-limits the public
-// webhook receiver endpoint with two limits in series.
-//
-// The inner limit is per client IP per request path: the path
-// contains the entrypoint UUID, so each sender is limited per
-// entrypoint without affecting other senders or other entrypoints.
-// It is Config.ReceiverRateLimit requests per minute.
-//
-// That limit alone bounds nothing in aggregate. The route pattern
-// /webhook/{uuid} matches any single segment, so a client that
-// invents a fresh path per request mints a fresh bucket per request
-// and never refills one — and every such request still reaches the
-// handler's entrypoint lookup before it 404s. The outer limit is
-// therefore keyed on the client IP alone, capping what one address
-// can spend across the whole route however it varies the path.
-//
-// Requests over either limit receive a 429. Clients are identified
-// by rateLimitKey.
+// ReceiverRateLimit returns middleware that rate-limits the
+// public webhook receiver endpoint per client IP per request
+// 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.
+// Clients are identified by rateLimitKey.
 func (m *Middleware) ReceiverRateLimit() func(http.Handler) http.Handler {
-	perEntrypoint := httprate.Limit(
+	return httprate.Limit(
 		m.params.Config.ReceiverRateLimit,
 		receiverRateInterval,
 		httprate.WithKeyFuncs(
@@ -309,31 +262,4 @@ func (m *Middleware) ReceiverRateLimit() func(http.Handler) http.Handler {
 			"Too many requests. Please slow down.",
 		)),
 	)
-
-	aggregate := httprate.Limit(
-		receiverAggregateLimit(m.params.Config.ReceiverRateLimit),
-		receiverRateInterval,
-		httprate.WithKeyFuncs(m.rateLimitKey),
-		httprate.WithLimitHandler(m.floodTooManyRequests(
-			"webhook receiver aggregate rate limit exceeded",
-			"Too many requests. Please slow down.",
-		)),
-	)
-
-	return func(next http.Handler) http.Handler {
-		return aggregate(perEntrypoint(next))
-	}
-}
-
-// receiverAggregateLimit is the per-IP aggregate limit derived from
-// the configured per-entrypoint limit. The operator sets the latter
-// and nothing bounds it from above, so the multiplication is
-// saturated rather than allowed to wrap into a negative limit that
-// would reject every request.
-func receiverAggregateLimit(perEntrypoint int) int {
-	if perEntrypoint > math.MaxInt/receiverAggregateMultiplier {
-		return math.MaxInt
-	}
-
-	return perEntrypoint * receiverAggregateMultiplier
 }

internal/middleware/ratelimit_test.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 	"log/slog"
-	"math"
 	"net/http"
 	"net/http/httptest"
 	"net/netip"
@@ -671,129 +670,6 @@ func TestRateLimitKey_LongChainAllocationIsBounded(t *testing.T) {
 	)
 }
 
-// TestReceiverRateLimit_LimitsAggregateAcrossInventedPaths is the
-// regression test for the per-path bucket key. The route pattern
-// matches any single segment, so a client that never reuses a path
-// never reuses a per-entrypoint bucket either, and its aggregate
-// rate against the receiver is whatever it likes — with every
-// request reaching an entrypoint lookup before it 404s. The IP-only
-// aggregate limiter is what bounds that, so this must fail if the
-// aggregate limiter is removed.
-func TestReceiverRateLimit_LimitsAggregateAcrossInventedPaths(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	const (
-		limit = 3
-		ip    = "6.6.6.6:1234"
-	)
-
-	aggregate := limit * middleware.ReceiverAggregateMultiplierConst
-
-	handler := receiverLimitedHandler(t, limit)
-
-	// Every request goes to a path this client has never used, so
-	// none of them shares a per-entrypoint bucket with another.
-	for i := range aggregate {
-		w := receiverPost(
-			handler, ip, fmt.Sprintf("/webhook/invented-%d", i),
-		)
-		assert.Equal(
-			t, http.StatusOK, w.Code,
-			"request %d to a distinct path should pass", i,
-		)
-	}
-
-	w := receiverPost(
-		handler, ip, fmt.Sprintf("/webhook/invented-%d", aggregate),
-	)
-	assert.Equal(
-		t, http.StatusTooManyRequests, w.Code,
-		"a client must not be able to raise its aggregate rate "+
-			"against /webhook/* by varying the path",
-	)
-
-	// The aggregate limit is still per client IP: exhausting one
-	// address must not throttle another.
-	w = receiverPost(handler, "6.6.6.7:1234", "/webhook/invented-0")
-	assert.Equal(
-		t, http.StatusOK, w.Code,
-		"a different client IP must not be affected",
-	)
-}
-
-// TestReceiverRateLimit_RejectedRequestsCountTowardAggregate pins the
-// order the two limiters are chained in. The aggregate limiter has to
-// be the outer one, so that it counts requests the per-entrypoint
-// limiter rejects: those requests still arrive, and the aggregate
-// limit exists to bound what one address can make the receiver do.
-//
-// One path is hammered past the per-entrypoint limit, which alone
-// would leave the aggregate budget almost untouched; then a path the
-// client has never used must be rejected, which only the aggregate
-// limiter can do. Swap the two limiters and that last request is
-// served, because the rejected ones never reached the aggregate
-// limiter to be counted.
-func TestReceiverRateLimit_RejectedRequestsCountTowardAggregate(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	const (
-		limit = 3
-		ip    = "6.6.6.8:1234"
-	)
-
-	aggregate := limit * middleware.ReceiverAggregateMultiplierConst
-
-	handler := receiverLimitedHandler(t, limit)
-
-	// Spend the whole aggregate budget on one path. Only the first
-	// limit requests are served; the rest are rejected by the
-	// per-entrypoint limiter but still count against the aggregate.
-	for i := range aggregate {
-		w := receiverPost(handler, ip, "/webhook/exhausted")
-
-		want := http.StatusTooManyRequests
-		if i < limit {
-			want = http.StatusOK
-		}
-
-		assert.Equal(
-			t, want, w.Code,
-			"request %d to the exhausted path", i,
-		)
-	}
-
-	w := receiverPost(handler, ip, "/webhook/never-used")
-	assert.Equal(
-		t, http.StatusTooManyRequests, w.Code,
-		"requests rejected per entrypoint must still count "+
-			"toward the aggregate limit, so the aggregate "+
-			"limiter has to run first",
-	)
-}
-
-// TestReceiverAggregateLimit_SaturatesOnOverflow covers the derived
-// aggregate limit for a configured per-entrypoint limit large enough
-// that multiplying it would wrap negative, which httprate would read
-// as a limit that rejects every request.
-func TestReceiverAggregateLimit_SaturatesOnOverflow(t *testing.T) {
-	t.Parallel()
-
-	assert.Equal(
-		t, 1200,
-		middleware.ReceiverAggregateLimitForTest(120),
-		"the default limit scales by the multiplier",
-	)
-	assert.Equal(
-		t, math.MaxInt,
-		middleware.ReceiverAggregateLimitForTest(math.MaxInt),
-		"an overflowing limit saturates instead of wrapping",
-	)
-}
-
 // TestReceiverRateLimit_IgnoresForwardedFromUntrustedPeer proves
 // the receiver limiter uses the same gated key function as the
 // POST limiters.