Warn when TRUSTED_PROXIES is empty in production (closes #149)

With no trusted proxies configured, every client behind the reverse proxy production requires shares one rate-limit bucket per limit, so five POSTs per minute from anywhere holds the login limit full and denies the admin login until restart. The default is still correct; it was the consequence that was invisible. Startup now warns, and the README no longer claims the login limit is per-IP unconditionally.

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` | Retention reaper period (Go duration, must be positive) | `1h` |
+| `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 | `""` (none) |
+| `TRUSTED_PROXIES`       | CIDRs whose forwarded headers are trusted (unset: all clients behind a proxy share one rate-limit bucket) | `""` (none) |
 
 #### Trusted proxies
 
@@ -115,6 +115,23 @@ 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
@@ -265,9 +282,10 @@ 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** — Stored events can be manually redelivered for debugging
+5. **Replay** (not yet implemented) — Every received event is stored in
-   or testing, without requiring the original sender to fire the webhook
+   full, which is what manual redelivery for debugging or testing will
-   again.
+   be built on. No redelivery exists today, in the web UI or the API;
+   see [TODO.md](TODO.md).
 
 ### Use Cases
 
@@ -277,6 +295,7 @@ 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
@@ -502,7 +521,7 @@ A programmatic access credential for API authentication.
 #### Event
 
 A captured incoming webhook request. Stores the complete HTTP request
-data for replay and auditing.
+data for auditing and for the planned replay capability.
 
 | Field          | Type   | Description |
 | -------------- | ------ | ----------- |
@@ -784,9 +803,10 @@ 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 the event itself remains
+operator never asked for that delivery — and while the event itself
-stored in the per-webhook event database, so it can be redelivered
+remains stored in the per-webhook event database, there is no way to
-manually.
+redeliver it: manual redelivery is planned, not implemented (see
+[TODO.md](TODO.md)).
 
 ### Circuit Breaker (HTTP Targets with Retries)
 
@@ -887,16 +907,29 @@ 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
+every other client behind the same proxy. Set `TRUSTED_PROXIES` to the
-to be wrong in: set `TRUSTED_PROXIES` to the proxy's address to get
+proxy's address to get per-client limits back. What the shared bucket
-per-client limits back. That shared bucket matters more for the
+costs is not the same for every limiter, and the two cases pull in
-aggregate limit than for the per-entrypoint one: with `TRUSTED_PROXIES`
+opposite directions:
-unset behind the reverse proxy a production deployment is required to
+
-run behind, every request keys on the proxy, so the aggregate limit
+- For the **receiver** limits it costs throughput, which is the safe
-becomes a service-wide ceiling of 1200 requests per minute across all
+  direction to be wrong in: sharing can only make a limit bind sooner,
-senders and all entrypoints, where the per-entrypoint limit's capacity
+  never let a sender past it. It matters more for the aggregate limit
-still grows with the number of entrypoints. Any deployment with more
+  than for the per-entrypoint one: with `TRUSTED_PROXIES` unset behind
-than a handful of busy entrypoints must set `TRUSTED_PROXIES`.
+  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`.
 
 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
@@ -1115,8 +1148,11 @@ 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):
-  per-IP sliding-window rate limiter on the login endpoint (5 POST
+  sliding-window rate limiter on the login endpoint, 5 POST attempts
-  attempts per minute per IP) to prevent brute-force attacks
+  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))
 - Prometheus metrics behind basic auth
 - Static assets embedded in binary (no filesystem access needed at
   runtime)

TODO.md

@@ -24,22 +24,52 @@ 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` (9bfd033) holds the completed 1.0.0 milestone: every issue in it
+`next` holds the completed 1.0.0 milestone: every issue in it is closed,
-is closed, and it is verified green by cache-defeated container runs
+and it is verified green both by CI and by cache-defeated container
-rather than by the CI badge, which can pass without executing anything
+runs. The two were only made to mean the same thing this cycle — before
-(#119). Note: TODO.md was deliberately deleted from this repo in f9a9569
+#119, a warm layer cache let the gate report success without executing
-(2026-03-01, #6); its content was folded into the README TODO section,
+anything, and replayed the previous build's console log so the lie
-which this draft reconstructs as of 2026-07-06.
+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 Step
 
-Tag 1.0.0 from `main` once the milestone PR merges, then repair the CI
+Merge the milestone PR to `main` and tag 1.0.0 from it.
-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
+Two decisions are open and belong to the owner, neither blocking the
-rests on.
+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`).
 
 # 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
@@ -61,6 +91,34 @@ rests on.
   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
@@ -115,6 +173,9 @@ rests on.
 
 # 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,6 +422,38 @@ 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.
@@ -466,5 +498,7 @@ 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,6 +1,8 @@
 package config_test
 
 import (
+	"bytes"
+	"log/slog"
 	"os"
 	"testing"
 	"time"
@@ -624,3 +626,79 @@ 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,9 +1,26 @@
 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 and can be redelivered
+// failed with a recorded reason. The event stays stored, but
-// manually. Logged at warn, not error: this is operator-caused
+// nothing redelivers it today. Logged at warn, not error: this
-// state, not a system fault.
+// is operator-caused state, not a system fault.
 func (e *Engine) failUnretryableRetry(
 	webhookDB *gorm.DB,
 	webhookID string,

static/js/app.js

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