Bound the receiver rate limit per client IP across /webhook/* (closes #139)

The receiver limiter keyed buckets on (client IP, request path). The
route pattern /webhook/{uuid} matches any single segment, so a client
that invented a fresh path per request minted a fresh bucket per
request and never refilled one: its aggregate rate against the only
unauthenticated, internet-exposed endpoint was unbounded, and every
one of those requests reached an entrypoint lookup before it 404ed.

Put a second limiter in front of it, keyed on the client IP alone and
covering the whole route at ten times the configured per-entrypoint
limit (1200/min by default). The per-entrypoint limit is unchanged and
still wanted; it just bounds nothing in aggregate on its own. Ten
entrypoints' worth of headroom lets one sender address drive several
entrypoints at full rate while still capping what one address costs
the receiver. The multiplication saturates rather than wrapping, since
nothing bounds RECEIVER_RATE_LIMIT from above and a negative limit
would reject every request.

The aggregate limiter is the outer one, so it counts the requests the
per-entrypoint limiter rejects; a test pins that order by exhausting
one path against the inner limit and then requiring a request to an
unused path to be rejected.

Move the handler's INFO line for an incoming webhook below the
entrypoint lookup. The UUID is attacker-controlled path text, so
logging it first let a client write an INFO line per invented path; a
miss is already logged at DEBUG and the request is already in the
access log.

Give the aggregate limiter its own 429 handler that logs at DEBUG and
without the path, rather than the shared one that logs at WARN with
it. Its rejections are one line per request of the very flood it
exists to bound, so the shared handler would have let a client write
its own text into the operator's log at an alerting level once per
request. What this limiter bounds is the database work an invented
path costs; the access log still records every request once at INFO,
and the README now says so instead of claiming DEBUG-only logging.

.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

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

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
+# Copy source code
-# 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 . .
 
 # 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
+# Copy source code
-# the lint stage above.
 COPY . .
 
 # Run tests and build

README.md

@@ -93,10 +93,10 @@ TTY detection, and security headers are always applied.
 | `METRICS_USERNAME`      | Basic auth username for `/metrics`  | `""`     |
 | `METRICS_PASSWORD`      | Basic auth password for `/metrics`  | `""`     |
 | `SENTRY_DSN`            | Sentry error reporting DSN          | `""`     |
-| `RETENTION_SWEEP_INTERVAL` | How often the retention reaper and archive sweeper run (Go duration, must be positive) | `1h` |
+| `RETENTION_SWEEP_INTERVAL` | Retention reaper period (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) |
+| `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
@@ -282,10 +265,9 @@ webhooker solves this by acting as a durable intermediary:
    targets simultaneously. This enables patterns like forwarding a
    GitHub webhook to both a deployment service and a Slack channel.
 
-5. **Replay** (not yet implemented) — Every received event is stored in
+5. **Replay** — Stored events can be manually redelivered for debugging
-   full, which is what manual redelivery for debugging or testing will
+   or testing, without requiring the original sender to fire the webhook
-   be built on. No redelivery exists today, in the web UI or the API;
+   again.
-   see [TODO.md](TODO.md).
 
 ### Use Cases
 
@@ -295,7 +277,6 @@ webhooker solves this by acting as a durable intermediary:
   size, and delivery performance
 - **Debugging** and introspection of webhook payloads in the web UI
 - **Replay** of webhook events for application testing and development
-  (planned; not yet implemented)
 - **Fan-out** delivery of a single webhook to multiple downstream
   targets
 - **High-availability ingestion** for delivery to less reliable backend
@@ -521,7 +502,7 @@ A programmatic access credential for API authentication.
 #### Event
 
 A captured incoming webhook request. Stores the complete HTTP request
-data for auditing and for the planned replay capability.
+data for replay and auditing.
 
 | Field          | Type   | Description |
 | -------------- | ------ | ----------- |
@@ -803,10 +784,9 @@ unknown) one while one of its deliveries is still `retrying`, both
 recovery paths above terminally mark that delivery `failed` and record a
 `DeliveryResult` naming the current target type as the reason, logging it
 at warn level. The delivery is not re-dispatched under the new type — the
-operator never asked for that delivery — and while the event itself
+operator never asked for that delivery — and the event itself remains
-remains stored in the per-webhook event database, there is no way to
+stored in the per-webhook event database, so it can be redelivered
-redeliver it: manual redelivery is planned, not implemented (see
+manually.
-[TODO.md](TODO.md)).
 
 ### Circuit Breaker (HTTP Targets with Retries)
 
@@ -887,19 +867,14 @@ 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
+costs, not the number of log lines it produces. The path is
-entrypoint is recorded by the handler at `DEBUG`, and the aggregate
+attacker-controlled, so nothing on this route writes it to the log
-limiter logs its own rejections at `DEBUG` and without the path, so
+above `DEBUG`: a path that names no entrypoint is recorded by the
-neither appears at all under the default level. The per-entrypoint
+handler at `DEBUG`, and the aggregate limiter logs its rejections at
-limiter is the loud one: it still logs every rejection at `WARN` with
+`DEBUG` and without the path. Every request is still recorded once by
-the request path, which on this route is attacker-controlled text. A
+the access log, at `INFO`, with its full URL, whether it was served or
-client hammering a single invented path is served `RECEIVER_RATE_LIMIT`
+rejected — so a flood of invented paths still writes one `INFO` line
-requests and has the rest of its aggregate budget rejected there, so
+per request.
-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
@@ -907,29 +882,16 @@ 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
+every other client behind the same proxy, which is the safe direction
-proxy's address to get per-client limits back. What the shared bucket
+to be wrong in: set `TRUSTED_PROXIES` to the proxy's address to get
-costs is not the same for every limiter, and the two cases pull in
+per-client limits back. That shared bucket matters more for the
-opposite directions:
+aggregate limit than for the per-entrypoint one: with `TRUSTED_PROXIES`
-
+unset behind the reverse proxy a production deployment is required to
-- For the **receiver** limits it costs throughput, which is the safe
+run behind, every request keys on the proxy, so the aggregate limit
-  direction to be wrong in: sharing can only make a limit bind sooner,
+becomes a service-wide ceiling of 1200 requests per minute across all
-  never let a sender past it. It matters more for the aggregate limit
+senders and all entrypoints, where the per-entrypoint limit's capacity
-  than for the per-entrypoint one: with `TRUSTED_PROXIES` unset behind
+still grows with the number of entrypoints. Any deployment with more
-  the reverse proxy a production deployment is required to run behind,
+than a handful of busy entrypoints must set `TRUSTED_PROXIES`.
-  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`.
 
 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 +1110,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-IP sliding-window rate limiter on the login endpoint (5 POST
-  per minute per bucket, to slow brute-force attacks. The bucket is per
+  attempts per minute per IP) to prevent brute-force attacks
-  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))
 - Prometheus metrics behind basic auth
 - Static assets embedded in binary (no filesystem access needed at
   runtime)
@@ -1177,34 +1136,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,
+`next` (9bfd033) holds the completed 1.0.0 milestone: every issue in it
-and it is verified green both by CI and by cache-defeated container
+is closed, and it is verified green by cache-defeated container runs
-runs. The two were only made to mean the same thing this cycle — before
+rather than by the CI badge, which can pass without executing anything
-#119, a warm layer cache let the gate report success without executing
+(#119). Note: TODO.md was deliberately deleted from this repo in f9a9569
-anything, and replayed the previous build's console log so the lie
+(2026-03-01, #6); its content was folded into the README TODO section,
-looked like a real run. Note: TODO.md was deliberately deleted from this
+which this draft reconstructs as of 2026-07-06.
-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.
+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
-Two decisions are open and belong to the owner, neither blocking the
+success without running is the one thing every other guarantee here
-tag: #115 (mask the `http` target's destination URL, implemented
+rests on.
-speculatively and awaiting a yes or no) and #125 (whether IPv6
-rate-limit keys should bucket by `/64`).
 
 # 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
@@ -91,34 +61,6 @@ rate-limit keys should bucket by `/64`).
   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-11 Mask the webhook credential in delivery errors and logs:
-  Go embeds the request URL in `*url.Error`, so every transport failure
-  persisted the full Slack webhook URL into the per-webhook event
-  database via `DeliveryResult.Error`, a field a future REST API would
-  have served. `maskURLError` drops path, query and userinfo while
-  preserving the wrapped cause, so `errors.Is`/`As` and `Timeout()`
-  still work and DNS, TLS and timeout failures still read differently
-  (#118)
-- 2026-08-11 Rate-limit the public webhook receiver endpoint
-  (`RECEIVER_RATE_LIMIT`, default 120/min), keyed on client IP plus
-  entrypoint path so one entrypoint cannot exhaust another's budget;
-  over-limit requests get 429 with `Retry-After`. It was the one
-  unauthenticated, internet-facing endpoint with no limit at all (#64)
-- 2026-08-11 Enforce the body size limit before CSRF parses the form:
-  `MaxBodySize` is now first in all four form-parsing route groups, so
-  an oversized request is rejected with 413 instead of being read in
-  full by the CSRF middleware before any cap applied (#90)
-- 2026-08-11 Mask target config on the source detail page, which
-  rendered the stored blob verbatim and so exposed the Slack
-  incoming-webhook URL — a bearer credential that cannot be revoked
-  per-holder. Config reaches the template only as a `TargetView` of
-  labelled fields, and header values are rendered as a count (#113)
-- 2026-08-11 Allow `retention_days` of 0 to mean retain forever, via a
-  sentinel written in `BeforeSave` so the GORM column default cannot
-  win the race. Also bounds the reaper's cutoff arithmetic: day counts
-  above 106751 overflowed `time.Duration` and wrapped the cutoff into
-  the future, where every row matched and the sweep deleted everything
-  (#79)
 - 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
@@ -173,9 +115,6 @@ rate-limit keys should bucket by `/64`).
 
 # Future Steps
 
-- Manual event redelivery from the web UI — the "Replay" capability the
-  README describes as planned. No redelivery code exists anywhere in the
-  tree; events are stored in full, which is all it would be built on
 - Delivery status and retry management UI
 - Per-webhook rate limiting in the receiver handler (per-webhook config
   plus handler enforcement; global limits must not apply to receiver

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/delivery/engine.go

@@ -785,9 +785,9 @@ func (e *Engine) sweepSingleRetry(
 // status retrying themselves. Re-dispatching under the new type
 // would be a delivery the operator never asked for, and leaving
 // the row retrying strands it forever, so the delivery is
-// failed with a recorded reason. The event stays stored, but
+// failed with a recorded reason and can be redelivered
-// nothing redelivers it today. Logged at warn, not error: this
+// manually. Logged at warn, not error: this is operator-caused
-// is operator-caused state, not a system fault.
+// state, not a system fault.
 func (e *Engine) failUnretryableRetry(
 	webhookDB *gorm.DB,
 	webhookID string,

internal/middleware/ratelimit.go

@@ -171,11 +171,9 @@ func (m *Middleware) clientKey(r *http.Request) string {
 	return peer.String()
 }
 
-// tooManyRequests returns the 429 handler used by the login,
+// tooManyRequests returns the 429 handler shared by every limiter:
-// password-change and per-entrypoint receiver limiters: it logs the
+// it logs the rejection with logMessage and answers with
-// rejection with logMessage and answers with responseMessage.
+// responseMessage. httprate adds the Retry-After header (RFC 6585).
-// httprate adds the Retry-After header (RFC 6585). The aggregate
-// receiver limiter uses floodTooManyRequests instead.
 func (m *Middleware) tooManyRequests(
 	logMessage, responseMessage string,
 ) http.HandlerFunc {

static/js/app.js

@@ -1,4 +1,5 @@
 // Webhooker client-side JavaScript
+console.log("Webhooker loaded");
 
 // Copy-to-clipboard, as progressive enhancement.
 //