Correct release-blocking README and startup-warning inaccuracies (closes #151)
All checks were successful
check / check (push) Successful in 3m10s

The empty-TRUSTED_PROXIES warning was gated on IsProd(), but
WEBHOOKER_ENVIRONMENT defaults to dev, so an internet-exposed
deployment whose operator never set it got no warning at all — the
exact operator error the warning exists to catch. It now fires whenever
the list is empty, in any environment, and its text is accurate both
behind a reverse proxy (shared buckets, remotely deniable admin login)
and with nothing in front of the process (harmless). The startup
configuration summary also now logs sessionIdleTimeout, the one value
where a valid setting silently disables a security control.

The README documented a two-stage Docker build on golang:1.24 running
"make check" (the tree has three stages: a golangci-lint lint stage
running fmt-check and lint, a golang:1.26.1-bookworm builder running
test and build, then the Alpine runtime), advertised the public
receiver as accepting all methods (it answers 405 to everything but
POST), claimed unqualified per-IP login rate limiting, and left the
session-expiry prose orphaned inside the trusted-proxy subsection.

The rest of the README was swept against the code rather than only the
reported lines: every documented route checked method-by-method against
internal/server/routes.go (adding the password-change, entrypoint and
target routes that were missing), every environment variable checked
against internal/config/config.go (MAINTENANCE_MODE serves no
maintenance page — it only sets a healthcheck field), the fx wiring,
package tree, prerequisites and dev commands brought back in line with
the tree, and two statements known false from other reviews corrected:
the body-size limit does not reject before "any other middleware" (the
eight global ones run first), and a retention value at or above the
retain-forever sentinel is accepted rather than 400ed.

TODO.md drops the unsupported half of its CI claim, keeping the
cache-defeated container runs, and splits the landed password change
away from the unimplemented reset flow.
This commit is contained in:
2026-08-17 20:42:42 +00:00
parent 2ee720a9af
commit 9ac8ff28ec
4 changed files with 231 additions and 106 deletions

234
README.md
View File

@@ -11,9 +11,11 @@ with retry support, logging, and observability. Category: infrastructure
### Prerequisites
- Go 1.26+
- golangci-lint v2.11+
- Docker (for containerized deployment)
- Go 1.26.1+ (the version in `go.mod`)
- golangci-lint v2.12.2 (the version pinned in `script/bootstrap` and
in the `Dockerfile`'s lint stage; `make bootstrap` installs it)
- Docker (for containerized deployment, and for the lint and test
stages of the CI gate)
### Quick Start
@@ -28,8 +30,10 @@ make deps
# Run all checks (format, lint, test, build)
make check
# Run in development mode (uses SQLite in current directory)
make dev
# Run in development mode. DATA_DIR defaults to /var/lib/webhooker in
# every environment, so set it (in .env or the shell) to a writable
# directory when running from a clone.
DATA_DIR=./data make dev
# Build Docker image
make docker
@@ -45,9 +49,13 @@ make lint # Run golangci-lint
make test # Run tests with race detection
make check # test + lint + fmt-check (CI gate)
make build # Build binary to bin/webhooker
make run # build, then run ./bin/webhooker
make dev # go run ./cmd/webhooker
make deps # go mod download + go mod tidy
make docker # Build Docker image
make hooks # Install git pre-commit hook that runs script/precommit
make css # Regenerate static/css/tailwind.css (needs tailwindcss)
make clean # Remove bin/
```
### Configuration
@@ -89,7 +97,7 @@ TTY detection, and security headers are always applied.
| `PORT` | HTTP listen port | `8080` |
| `DATA_DIR` | Directory for all SQLite databases | `/var/lib/webhooker` |
| `DEBUG` | Enable debug logging | `false` |
| `MAINTENANCE_MODE` | Serve the maintenance page | `false` |
| `MAINTENANCE_MODE` | Report `maintenanceMode: true` in the healthcheck JSON. It does not change how any request is served — no maintenance page exists | `false` |
| `METRICS_USERNAME` | Basic auth username for `/metrics` | `""` |
| `METRICS_PASSWORD` | Basic auth password for `/metrics` | `""` |
| `SENTRY_DSN` | Sentry error reporting DSN | `""` |
@@ -129,8 +137,13 @@ 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.
at startup whenever `TRUSTED_PROXIES` is empty, in every environment —
not only when `WEBHOOKER_ENVIRONMENT=prod`, because that variable
defaults to `dev` and an operator who never set it is precisely the
one at risk. The warning is informational when nothing proxies to the
process: with no proxy in front, the peer address is the client's own
and the buckets are already per-client. 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
@@ -162,6 +175,8 @@ Two operator requirements follow:
makes all three limits, including the unauthenticated webhook
receiver, silently bypassable by every client in the block.
#### Sessions
Sessions are bounded by two independent clocks, and end at whichever
one runs out first:
@@ -231,9 +246,11 @@ docker run -d \
The container runs as a non-root user (`webhooker`, UID 1000), exposes
port 8080, and includes a health check against
`/.well-known/healthcheck`. The `/var/lib/webhooker` volume holds all
SQLite databases: the main application database (`webhooker.db`) and
the per-webhook event databases (`events-{uuid}.db`). Mount this as a
persistent volume to preserve data across container restarts.
SQLite databases: the main application database (`webhooker.db`), the
per-webhook event databases (`events-{uuid}.db`), and any archive
databases written by `database` targets (`archive-{uuid}.db`). Mount
this as a persistent volume to preserve data across container
restarts.
## Entrypoints
@@ -330,7 +347,11 @@ It uses:
- **[gorilla/csrf](https://github.com/gorilla/csrf)** for CSRF
protection (cookie-based double-submit tokens)
- **[go-chi/httprate](https://github.com/go-chi/httprate)** for
per-IP login rate limiting (sliding window counter)
sliding-window rate limiting of the login, password-change and
webhook receiver endpoints. The bucket is per client IP only when
`TRUSTED_PROXIES` names the reverse proxy; unset, every client
behind that proxy shares one bucket per limit (see
[Rate Limiting](#rate-limiting))
- **[Prometheus](https://prometheus.io)** for metrics, served at
`/metrics` behind basic auth
- **[Sentry](https://sentry.io)** for optional error reporting
@@ -440,9 +461,20 @@ days (`database.RetentionForeverDays`). The retention reaper recognises
that sentinel and skips the webhook entirely, and the web UI displays
such a webhook's retention as "forever" rather than as a day count.
A *finite* retention is capped at `database.MaxFiniteRetentionDays`
(106751 days, about 292 years), and a larger one is rejected with a
400. The cap is not arbitrary: the reaper computes its cutoff as a
Submitted `retention_days` values therefore fall into three bands, not
two:
- `0` up to `database.MaxFiniteRetentionDays` (106751 days, about 292
years) is accepted as a finite retention.
- Above that ceiling but below the retain-forever sentinel of 365000
(`database.RetentionForeverDays`) is rejected with a 400. This is the
band the cap exists for.
- `365000` or above is accepted and means retain forever, collapsing to
the sentinel. It is not out of range: the edit form pre-fills the
sentinel for a retain-forever webhook, so submitting that form back
unchanged has to keep meaning "forever".
The cap is not arbitrary: the reaper computes its cutoff as a
`time.Duration`, an int64 nanosecond count, and a longer period
overflows it. An overflowed cutoff lands in the future, where it
matches every row, so the sweep would delete every event the webhook
@@ -534,7 +566,7 @@ data for auditing and for the planned replay capability.
| `id` | UUID | Primary key |
| `webhook_id` | UUID | Foreign key → Webhook |
| `entrypoint_id` | UUID | Foreign key → Entrypoint |
| `method` | string | HTTP method (POST, PUT, etc.) |
| `method` | string | HTTP method of the captured request. Always `POST`: the receiver answers every other method with 405 before an Event is created |
| `headers` | JSON | Complete request headers |
| `body` | text | Raw request body |
| `content_type` | string | Content-Type header value |
@@ -934,8 +966,8 @@ opposite directions:
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`.
must set `TRUSTED_PROXIES`; webhooker warns at startup whenever it is
empty, in any environment.
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
@@ -947,17 +979,17 @@ abuse limit later; they are tracked as future work.
| Method | Path | Description |
| ------ | --------------------------- | ----------- |
| `GET` | `/` | Root redirect (authenticated → `/sources`, unauthenticated → `/pages/login`) |
| `GET` | `/.well-known/healthcheck` | Health check (JSON: status, uptime, version) |
| `GET` | `/s/*` | Static file serving (embedded CSS, JS) |
| `ANY` | `/webhook/{uuid}` | Webhook receiver endpoint (accepts all methods) |
| `GET` | `/` | Root redirect, 303 (authenticated → `/sources`, unauthenticated → `/pages/login`) |
| `GET` | `/.well-known/healthcheck` | Health check (JSON: `status`, `now`, `uptimeSeconds`, `uptimeHuman`, `version`, `appname`, `maintenanceMode`) |
| `GET` | `/s/*` | Static file serving (embedded CSS, JS; `GET` and `HEAD`) |
| `POST` | `/webhook/{uuid}` | Webhook receiver endpoint. `POST` only — every other method is answered `405 Method Not Allowed` with `Allow: POST`. Rate limited (see [Rate Limiting](#rate-limiting)) |
#### Authentication Endpoints
| Method | Path | Description |
| ------ | --------------- | ----------- |
| `GET` | `/pages/login` | Login page |
| `POST` | `/pages/login` | Login form submission |
| `GET` | `/pages/login` | Login page (not rate limited; the limiter applies to POST only) |
| `POST` | `/pages/login` | Login form submission (5 per minute per bucket, then 429) |
| `POST` | `/pages/logout` | Logout (destroys session) |
#### Authenticated Endpoints
@@ -965,6 +997,7 @@ abuse limit later; they are tracked as future work.
| Method | Path | Description |
| ------ | ------------------------ | ----------- |
| `GET` | `/user/{username}` | User profile page |
| `POST` | `/user/{username}/password` | Change the user's password (5 per minute per bucket, then 429) |
| `GET` | `/sources` | List user's webhooks |
| `GET` | `/sources/new` | Create webhook form |
| `POST` | `/sources/new` | Create webhook submission |
@@ -974,13 +1007,17 @@ abuse limit later; they are tracked as future work.
| `POST` | `/source/{id}/delete` | Delete webhook |
| `GET` | `/source/{id}/logs` | Webhook event logs |
| `POST` | `/source/{id}/entrypoints` | Add entrypoint to webhook |
| `POST` | `/source/{id}/entrypoints/{entrypointID}/delete` | Delete an entrypoint |
| `POST` | `/source/{id}/entrypoints/{entrypointID}/toggle` | Enable or disable an entrypoint |
| `POST` | `/source/{id}/targets` | Add target to webhook |
| `POST` | `/source/{id}/targets/{targetID}/delete` | Delete a target |
| `POST` | `/source/{id}/targets/{targetID}/toggle` | Enable or disable a target |
#### Infrastructure Endpoints
| Method | Path | Description |
| ------ | ---------- | ----------- |
| `GET` | `/metrics` | Prometheus metrics (requires basic auth) |
| `GET` | `/metrics` | Prometheus metrics, behind basic auth. The route is registered only when `METRICS_USERNAME` is set; otherwise it does not exist and returns 404 |
#### API (Planned)
@@ -994,8 +1031,10 @@ abuse limit later; they are tracked as future work.
| `GET` | `/api/v1/webhooks/{id}/events` | List events for webhook |
| `POST` | `/api/v1/events/{id}/redeliver`| Redeliver an event |
API authentication will use API keys passed via `Authorization: Bearer
<key>` header.
None of these exist yet. `/api/v1` is mounted with no routes, so every
path under it returns 404 today. API authentication will use API keys
passed via `Authorization: Bearer <key>` header; no Bearer middleware
is implemented either.
### Package Layout
@@ -1023,13 +1062,25 @@ webhooker/
│ │ ├── model_delivery_result.go # DeliveryResult entity (per-webhook DB)
│ │ ├── model_apikey.go # APIKey entity
│ │ ├── password.go # Argon2id hashing and verification
│ │ ├── retention.go # Retention reaper (per-webhook event expiry)
│ │ └── webhook_db_manager.go # Per-webhook DB lifecycle manager
│ ├── globals/
│ │ └── globals.go # Build-time variables (appname, version, arch)
│ ├── delivery/
│ │ ├── engine.go # Event-driven delivery engine (channel + timer based)
│ │ ├── circuit_breaker.go # Per-target circuit breaker for HTTP targets with retries
│ │ ├── target.go # Target interface, Task, Scheduler
│ │ ├── target_http.go # HTTP target (retries, circuit breaker)
│ │ ├── target_slack.go # Slack/Mattermost incoming-webhook target
│ │ ├── target_database.go # Database archive target
│ │ ├── target_database_archive.go # Archive file lifecycle and pruning
│ │ ├── target_log.go # Log target (stdout)
│ │ ├── target_config_view.go # Masked target config for templates
│ │ ├── archive_sweeper.go # Periodic pruning of idle archives
│ │ ├── url_mask.go # Strips credentials from *url.Error
│ │ └── ssrf.go # SSRF prevention (IP validation, safe HTTP transport)
│ ├── lifecycle/
│ │ └── lifecycle.go # Shared fx start/stop hook helpers
│ ├── handlers/
│ │ ├── handlers.go # Base handler struct, JSON helpers, template rendering
│ │ ├── auth.go # Login, logout handlers
@@ -1054,11 +1105,15 @@ webhooker/
│ └── session.go # Cookie-based session management
├── static/
│ ├── static.go # //go:embed directive
│ ├── css/style.css # Custom stylesheet (system font stack, card effects, layout)
── js/app.js # Client-side JavaScript (minimal bootstrap)
├── templates/ # Go HTML templates (base, index, login, etc.)
├── Dockerfile # Multi-stage: lint, build+test, then Alpine runtime
├── Makefile # fmt, lint, test, check, build, docker targets
│ ├── css/input.css # Tailwind input, source for tailwind.css (make css)
── css/tailwind.css # Generated stylesheet the pages load
│ ├── css/style.css # Older hand-written stylesheet, no longer loaded
│ ├── js/alpine.min.js # Alpine.js, served locally (no CDN)
│ └── js/app.js # Progressive-enhancement copy-to-clipboard
├── templates/ # Go HTML templates (base, login, sources, etc.)
├── script/ # Scripts to Rule Them All entrypoints
├── Dockerfile # Three stages: lint, test+build, Alpine runtime
├── Makefile # Thin shims over script/
├── go.mod / go.sum
└── .golangci.yml # Linter configuration
```
@@ -1074,21 +1129,27 @@ Components are wired via Uber fx in this order:
user seed
5. `database.NewWebhookDBManager` — Per-webhook event database
lifecycle manager
6. `healthcheck.New` — Health check service
7. `session.New` — Cookie-based session manager (key from database)
8. `handlers.New`HTTP handlers
9. `middleware.New` — HTTP middleware
10. `delivery.New` — Event-driven delivery engine
11. `delivery.Engine``handlers.DeliveryNotifier` — interface bridge
12. `server.New` — HTTP server and router
6. `database.NewRetentionReaper` — Per-webhook event retention sweep
7. `healthcheck.New` — Health check service
8. `session.New`Cookie-based session manager (key from database)
9. `handlers.New` — HTTP handlers
10. `middleware.New` — HTTP middleware
11. `delivery.New` — Event-driven delivery engine
12. `delivery.NewArchiveSweeper` — Periodic pruning of idle archives
13. `delivery.Engine``delivery.Notifier` — interface bridge
14. `delivery.Engine``delivery.WebhookEvictor` — interface bridge so
deleting a webhook releases its archive writer
15. `server.New` — HTTP server and router
The server starts via `fx.Invoke(func(*server.Server, *delivery.Engine)
{})` which triggers the fx lifecycle hooks in dependency order. The
`DeliveryNotifier` interface allows the webhook handler to send
self-contained `DeliveryTask` slices to the engine without a direct
The server starts via `fx.Invoke(func(*server.Server, *delivery.Engine,
*database.RetentionReaper, *delivery.ArchiveSweeper) {})`, which
triggers the fx lifecycle hooks in dependency order. The
`delivery.Notifier` interface allows the webhook handler to send
self-contained `delivery.Task` slices to the engine without a direct
package dependency. Each task carries all target config and event data
inline (for bodies ≤16KB), so the engine can deliver without reading
from any database — it only writes to record results.
inline (for bodies ≤16KB, `delivery.MaxInlineBodySize`), so the engine
can deliver without reading from any database — it only writes to
record results.
### Middleware Stack
@@ -1114,16 +1175,30 @@ CSRF middleware in every one of those route groups, because
gorilla/csrf parses the form; if the cap were installed after it, form
parsing would run under net/http's 10 MB default and the 1 MB limit
would never apply. A request that declares a `Content-Length` over the
limit is answered with `413 Request Entity Too Large` before any other
middleware or handler runs; a chunked request, or one that lies about
its length, is hard-capped by `http.MaxBytesReader` and fails
downstream at form-parse time.
limit is answered with `413 Request Entity Too Large` without its body
being read and without reaching CSRF, the route group's remaining
middleware, or the handler. It is not rejected before *any* other
middleware, though: the eight global entries listed above all run
first, so such a request is still logged, counted in the metrics, and
given the security headers, and the rejection itself is logged at
`WARN` with the method, path and declared length. A chunked request, or
one that lies about its length, is hard-capped by
`http.MaxBytesReader` and fails downstream at form-parse time.
Those same four route groups then apply **CSRF** and **NoCache**
(`Cache-Control: no-store`, `Pragma: no-cache`), and every group except
`/pages` applies **RequireAuth**. The rate limiters are per-route
rather than global: **LoginRateLimit** on `/pages/login`,
**PasswordChangeRateLimit** on `/user/{username}/password`, and
**ReceiverRateLimit** on `/webhook/{uuid}`.
### Authentication
- **Web UI:** Cookie-based sessions using gorilla/sessions with
encrypted cookies. Sessions are configured with HttpOnly, SameSite
Lax, and Secure (in production). Session lifetime is 7 days.
Lax, and Secure (in production). Absolute session lifetime is 7 days,
with a sliding idle timeout on top of it (see
[Sessions](#sessions)).
- **API (planned):** API key authentication via `Authorization: Bearer`
header. API keys are stored per-user with usage tracking
(`last_used_at`).
@@ -1155,10 +1230,13 @@ downstream at form-parse time.
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 minute per bucket, to slow brute-force attacks. GET requests to
the login page are not limited. The password-change endpoint carries
the same 5-per-minute limit. 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)). webhooker warns at startup
whenever `TRUSTED_PROXIES` is empty
- Prometheus metrics behind basic auth
- Static assets embedded in binary (no filesystem access needed at
runtime)
@@ -1167,21 +1245,32 @@ downstream at form-parse time.
### Docker
The Dockerfile uses a multi-stage build:
The Dockerfile uses a three-stage build. Each stage is pinned by
digest, and the two check stages are separate images so the linter's
version is fixed independently of the compiler's:
1. **Builder stage** (Debian-based `golang:1.24`) — installs
golangci-lint, downloads dependencies, copies source, runs `make
check` (format verification, linting, tests, compilation).
2. **Runtime stage** (`alpine:3.21`) — copies the binary, creates the
`/var/lib/webhooker` directory for all SQLite databases, runs as
non-root user, exposes port 8080, includes a health check.
1. **Lint stage** (`golangci/golangci-lint:v2.12.2`, Debian-based) —
installs `make`, downloads dependencies, copies the source, and runs
`make fmt-check` then `make lint`.
2. **Builder stage** (`golang:1.26.1-bookworm`) — depends on the lint
stage passing (it copies a file from it), then runs `make test` and
`make build`, and finally rebuilds the binary with
`CGO_ENABLED=1` and static linking so it runs on musl.
3. **Runtime stage** (`alpine:3.21`) — copies the static binary,
creates the `/var/lib/webhooker` directory for all SQLite databases,
runs as the non-root `webhooker` user (UID 1000), exposes port 8080,
and includes a health check against `/.well-known/healthcheck`.
The builder uses Debian rather than Alpine because GORM's SQLite
dialect pulls in CGO-dependent headers at compile time. The runtime
binary is statically linked and runs on Alpine.
Both check stages use Debian rather than Alpine because
`gorm.io/driver/sqlite` pulls in `mattn/go-sqlite3`, which needs CGO
and does not compile against musl. Only the final binary is statically
linked, which is what lets it run on the Alpine runtime image.
`docker build .` is the CI gate — if it passes, the code is formatted,
linted, tested, and compiled.
`script/cibuild``docker build .` is the CI gate: the four check
targets run inside the image, so a build that succeeds is a repo that
is formatted, linted, tested and compiled. Local linting goes through
the same container rather than a host golangci-lint, because a host
binary can be a different version from the pinned one.
#### CI gate honesty
@@ -1197,16 +1286,17 @@ the hash of the last commit that touched the build context, so:
`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.
excludes `*.md`, `LICENSE` and `.editorconfig` 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
A separate workflow step, run before the fingerprint is written, 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.

22
TODO.md
View File

@@ -25,13 +25,17 @@ 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.
and it is verified green by cache-defeated container runs
(`docker build --no-cache-filter=lint --no-cache-filter=builder`). The
CI status is not independently claimed here: a superseded run is
recorded as `skipped` and still rolls up green, so a commit status on
`next` does not by itself evidence an executed check (#152). Before
#119, a warm layer cache also 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 Step
@@ -190,7 +194,9 @@ rate-limit keys should bucket by `/64`).
- OpenAPI specification
- Analytics dashboard: success rates, response times, volume
- A remember-me option at login
- Password change and reset flow
- Password reset flow for a forgotten password. The authenticated
password *change* flow already landed on `main` (#65); reset does not
exist
- Later, nice to have
- email delivery target type
- SNS and S3 delivery targets

View File

@@ -422,33 +422,43 @@ func loadFromEnv() (*Config, error) {
}, nil
}
// warnSharedRateLimitBucket logs a startup warning when a production
// deployment leaves TRUSTED_PROXIES empty.
// warnSharedRateLimitBucket logs a startup warning whenever
// TRUSTED_PROXIES is empty, in any environment.
//
// 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
// peer's address. Whether that is harmless or dangerous depends on
// what is in front of the process, which this code cannot observe:
// with nothing in front, the peer is the client and the limits are
// per-client as intended; behind a reverse proxy the peer is the 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.
// keep it full, which denies the only administrative login to everyone
// until the process restarts.
//
// The warning is deliberately not gated on WEBHOOKER_ENVIRONMENT. That
// variable defaults to dev, so gating on it would silence the warning
// for exactly the operator who forgot to configure the deployment —
// the case it exists to catch.
//
// 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 {
if 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.",
"TRUSTED_PROXIES is empty: every rate limit keys on the "+
"connecting peer's address. With nothing proxying to "+
"this process that is the client itself and the limits "+
"are per-client as intended. Behind a reverse proxy the "+
"peer is the proxy on every request, so all clients "+
"share one bucket per limit and any remote client can "+
"keep the login limit full, denying the admin login — "+
"the only administrative path — until restart. If "+
"anything proxies to this process, set TRUSTED_PROXIES "+
"to its address.",
"environment", c.Environment,
"trustedProxies", len(c.TrustedProxies),
)
@@ -491,6 +501,10 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
"maintenanceMode", s.MaintenanceMode,
"dataDir", s.DataDir,
"retentionSweepInterval", s.RetentionSweepInterval.String(),
// Logged because a perfectly valid non-positive value here
// disables idle expiry entirely, and that is worth showing
// back to the operator.
"sessionIdleTimeout", s.SessionIdleTimeout.String(),
"receiverRateLimit", s.ReceiverRateLimit,
"trustedProxies", len(s.TrustedProxies),
"hasSentryDSN", s.SentryDSN != "",

View File

@@ -628,10 +628,12 @@ func testTrustedProxiesSuccess(
}
// 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.
// tells an operator a deployment behind a reverse proxy shares one
// rate-limit bucket between every client, which makes the admin login
// remotely deniable. It must fire whenever TRUSTED_PROXIES is empty,
// in any environment: WEBHOOKER_ENVIRONMENT defaults to dev, so gating
// on it would silence the warning for exactly the operator who never
// configured the deployment. It stays quiet once proxies are named.
func TestSharedRateLimitBucketWarning(t *testing.T) {
tests := []struct {
name string
@@ -651,11 +653,18 @@ func TestSharedRateLimitBucketWarning(t *testing.T) {
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",
// The default environment. An internet-exposed
// deployment whose operator never set
// WEBHOOKER_ENVIRONMENT lands here and has exactly
// the exposure the warning announces.
name: "dev without trusted proxies warns",
environment: config.EnvironmentDev,
expectWarning: true,
},
{
name: "dev with trusted proxies is quiet",
environment: config.EnvironmentDev,
trustedProxies: cidrPrivateV4,
expectWarning: false,
},
}
@@ -697,8 +706,14 @@ func TestSharedRateLimitBucketWarning(t *testing.T) {
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")
assert.Contains(t, logged, "share one bucket")
assert.Contains(t, logged, "denying the admin login")
// The text must stay accurate for a developer with
// nothing in front of the process, where an empty
// list costs nothing.
assert.Contains(
t, logged, "nothing proxying to this process",
)
})
}
}