Compare commits
1 Commits
74a57ad237
...
99968231ad
| Author | SHA1 | Date | |
|---|---|---|---|
| 99968231ad |
459
README.md
459
README.md
@@ -11,14 +11,9 @@ with retry support, logging, and observability. Category: infrastructure
|
||||
|
||||
### Prerequisites
|
||||
|
||||
- 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)
|
||||
- `curl`, used by `script/fetch-assets` to download the third-party
|
||||
browser assets, which are not committed (`make bootstrap` installs
|
||||
it if missing)
|
||||
- Go 1.26+
|
||||
- golangci-lint v2.11+
|
||||
- Docker (for containerized deployment)
|
||||
|
||||
### Quick Start
|
||||
|
||||
@@ -27,18 +22,14 @@ with retry support, logging, and observability. Category: infrastructure
|
||||
git clone https://git.eeqj.de/sneak/webhooker.git
|
||||
cd webhooker
|
||||
|
||||
# Install Go dependencies, the pinned linter, and the third-party
|
||||
# browser assets. `make deps` alone is not enough: it only runs
|
||||
# go mod download/tidy, and the checks below need the fetched assets.
|
||||
make bootstrap
|
||||
# Install Go dependencies
|
||||
make deps
|
||||
|
||||
# Run all checks (test, lint, format check)
|
||||
# Run all checks (format, lint, test, build)
|
||||
make check
|
||||
|
||||
# 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
|
||||
# Run in development mode (uses SQLite in current directory)
|
||||
make dev
|
||||
|
||||
# Build Docker image
|
||||
make docker
|
||||
@@ -51,18 +42,13 @@ make bootstrap # Install all dependencies (idempotent)
|
||||
make setup # Bootstrap + install git pre-commit hook
|
||||
make assets # Fetch + verify third-party browser assets
|
||||
make fmt # Format code (gofmt + goimports)
|
||||
make fmt-check # Fail if gofmt would change anything (writes nothing)
|
||||
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
|
||||
@@ -104,7 +90,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` | Report `maintenanceMode: true` in the healthcheck JSON. It does not change how any request is served — no maintenance page exists | `false` |
|
||||
| `MAINTENANCE_MODE` | Serve the maintenance page | `false` |
|
||||
| `METRICS_USERNAME` | Basic auth username for `/metrics` | `""` |
|
||||
| `METRICS_PASSWORD` | Basic auth password for `/metrics` | `""` |
|
||||
| `SENTRY_DSN` | Sentry error reporting DSN | `""` |
|
||||
@@ -144,13 +130,8 @@ 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 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.
|
||||
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
|
||||
@@ -182,8 +163,6 @@ 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:
|
||||
|
||||
@@ -253,20 +232,17 @@ 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`), 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.
|
||||
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.
|
||||
|
||||
## Entrypoints
|
||||
|
||||
This repository adheres to the
|
||||
[Scripts to Rule Them All](https://github.com/github/scripts-to-rule-them-all)
|
||||
standard: normalized scripts in `script/` are the entrypoints for the
|
||||
development workflow. Ten of the Makefile's sixteen targets are thin
|
||||
shims that call them; `build`, `run`, `dev`, `deps`, `clean` and `css`
|
||||
are inline commands with no script behind them. We provide:
|
||||
development workflow, and the Makefile targets are thin shims that call
|
||||
them. We provide:
|
||||
|
||||
- `script/bootstrap` — install all dependencies (idempotent)
|
||||
- `script/setup` — make a fresh clone ready for development
|
||||
@@ -378,11 +354,7 @@ 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
|
||||
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))
|
||||
per-IP login rate limiting (sliding window counter)
|
||||
- **[Prometheus](https://prometheus.io)** for metrics, served at
|
||||
`/metrics` behind basic auth
|
||||
- **[Sentry](https://sentry.io)** for optional error reporting
|
||||
@@ -400,7 +372,7 @@ The codebase uses consistent naming throughout (rename completed in
|
||||
|
||||
### Data Model
|
||||
|
||||
webhooker's data model has nine entities organized into two tiers: the
|
||||
webhooker's data model has eight entities organized into two tiers: the
|
||||
**application tier** (user and webhook configuration) and the **event
|
||||
tier** (event ingestion, delivery, and logging).
|
||||
|
||||
@@ -492,25 +464,9 @@ 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.
|
||||
|
||||
Submitted `retention_days` values therefore fall into three bands, not
|
||||
two:
|
||||
|
||||
- `1` 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.
|
||||
- `0`, and `365000` or above, are accepted and mean retain forever,
|
||||
collapsing to the sentinel — `0` in `Webhook.BeforeSave`, the large
|
||||
values in `parseRetentionDays`. The large values are 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".
|
||||
|
||||
A negative value is in none of the three: `parseRetentionDays` rejects
|
||||
it with a 400 before `BeforeSave` ever sees it.
|
||||
|
||||
The cap is not arbitrary: the reaper computes its cutoff as a
|
||||
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
|
||||
`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
|
||||
@@ -528,7 +484,7 @@ the full request and creates an Event.
|
||||
| -------------- | ------- | ----------- |
|
||||
| `id` | UUID | Primary key |
|
||||
| `webhook_id` | UUID | Foreign key → Webhook |
|
||||
| `path` | string | Unique bare UUID, generated at creation. The `/webhook/` prefix is route only and is not stored: the receiver matches this column against the raw `{uuid}` path segment |
|
||||
| `path` | string | Unique URL path (UUID-based, e.g. `/webhook/{uuid}`) |
|
||||
| `description` | string | Optional description |
|
||||
| `active` | boolean | Whether this entrypoint accepts events (default: true) |
|
||||
|
||||
@@ -552,8 +508,8 @@ events should be forwarded.
|
||||
| `type` | TargetType | One of: `http`, `slack`, `database`, `log` |
|
||||
| `active` | boolean | Whether deliveries are enabled (default: true) |
|
||||
| `config` | JSON text | Type-specific configuration |
|
||||
| `max_retries` | integer | Maximum retry attempts for `http` and `slack` targets (0 = fire-and-forget, >0 = retries with backoff and a circuit breaker). Ignored by `database` and `log` targets |
|
||||
| `max_queue_size` | integer | Stored and shown on the target's detail view, but not enforced anywhere yet: nothing in the delivery engine consults it. Queue depth is set by the two fixed 10,000-entry channels |
|
||||
| `max_retries` | integer | Maximum retry attempts for HTTP targets (0 = fire-and-forget, >0 = retries with backoff) |
|
||||
| `max_queue_size` | integer | Maximum queued deliveries (for HTTP targets with retries) |
|
||||
|
||||
**Relations:** Belongs to Webhook. Has many Deliveries.
|
||||
|
||||
@@ -566,11 +522,6 @@ events should be forwarded.
|
||||
greater than 0, failed deliveries are retried with exponential backoff
|
||||
up to `max_retries` attempts, protected by a per-target circuit
|
||||
breaker.
|
||||
- **`slack`** — Post the event as a formatted message to a
|
||||
Slack-compatible incoming webhook URL (`webhookUrl` in `config`). It
|
||||
is built on the same HTTP core as `http` and honours `max_retries`
|
||||
identically, circuit breaker included. See the Slack target section
|
||||
under "Per-Webhook Event Databases" for the message format.
|
||||
- **`database`** — Archive the full event as a row into a separate
|
||||
per-webhook archive database (`archive-{webhookID}.db`) for long-term
|
||||
retention, with an optional creation-validated expiry (default: keep
|
||||
@@ -607,7 +558,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 of the captured request. Always `POST`: the receiver answers every other method with 405 before an Event is created |
|
||||
| `method` | string | HTTP method (POST, PUT, etc.) |
|
||||
| `headers` | JSON | Complete request headers |
|
||||
| `body` | text | Raw request body |
|
||||
| `content_type` | string | Content-Type header value |
|
||||
@@ -662,9 +613,7 @@ retries) is individually logged for full observability.
|
||||
|
||||
#### Common Fields
|
||||
|
||||
Every entity except `Setting` includes these fields from `BaseModel`.
|
||||
`Setting` is a bare key-value row with no `id`, no timestamps and no
|
||||
soft delete:
|
||||
All entities include these fields from `BaseModel`:
|
||||
|
||||
| Field | Type | Description |
|
||||
| ------------ | --------- | ----------- |
|
||||
@@ -710,7 +659,7 @@ handles connection pooling, lazy opening, migrations, and cleanup.
|
||||
This separation provides:
|
||||
|
||||
- **Isolation** — a high-volume webhook won't cause lock contention or
|
||||
journal growth affecting the main application or other webhooks.
|
||||
WAL bloat affecting the main application or other webhooks.
|
||||
- **Independent lifecycle** — event databases can be independently
|
||||
backed up, archived, rotated, or size-limited without impacting the
|
||||
application.
|
||||
@@ -720,12 +669,9 @@ This separation provides:
|
||||
- **Per-webhook retention** — the `retention_days` field on each webhook
|
||||
controls automatic cleanup of old events in that webhook's database
|
||||
only, or disables cleanup entirely when set to `0` (retain forever).
|
||||
- **Performance** — each webhook's database has its own page cache and
|
||||
its own lock, so concurrent event ingestion across webhooks won't
|
||||
contend. No write-ahead log is involved: both DSNs are
|
||||
`file:{path}?cache=shared&mode=rwc` and no `journal_mode` pragma is
|
||||
ever issued, so every database runs on SQLite's default rollback
|
||||
journal.
|
||||
- **Performance** — each webhook's database has its own WAL, its own
|
||||
page cache, and its own lock, so concurrent event ingestion across
|
||||
webhooks won't contend.
|
||||
|
||||
The **database target type** builds on this architecture to provide
|
||||
long-term archiving, separate from the per-webhook event database (which
|
||||
@@ -781,9 +727,8 @@ and other compatible services). Each message includes event metadata
|
||||
pretty-printed in a code block. JSON payloads are automatically
|
||||
formatted with indentation for readability; non-JSON payloads are shown
|
||||
as raw text. Large payloads are truncated to keep messages reasonable.
|
||||
Config stores `webhookUrl` — the Slack/Mattermost incoming webhook
|
||||
endpoint. That is the JSON key; the error text for a missing one reads
|
||||
`webhook_url is required`, which is the message, not the key.
|
||||
Config stores `webhook_url` — the Slack/Mattermost incoming webhook
|
||||
endpoint.
|
||||
|
||||
The database uses the
|
||||
[modernc.org/sqlite](https://pkg.go.dev/modernc.org/sqlite) driver at
|
||||
@@ -805,9 +750,8 @@ External Service
|
||||
1. Look up Entrypoint by UUID
|
||||
2. Capture full request as Event
|
||||
3. Create Delivery records for each active Target
|
||||
4. Build self-contained delivery.Task structs
|
||||
(target config + event data inline for
|
||||
bodies < 16 KiB)
|
||||
4. Build self-contained DeliveryTask structs
|
||||
(target config + event data inline for ≤16KB)
|
||||
5. Notify Engine via channel (no DB read needed)
|
||||
│
|
||||
▼
|
||||
@@ -842,7 +786,7 @@ at any time, preventing goroutine explosions regardless of queue depth.
|
||||
a delivery channel (new tasks from the webhook handler) and a retry
|
||||
channel (tasks from backoff timers). Both are buffered to 10,000.
|
||||
- **Fan-out via channel, not goroutines:** When an event arrives with
|
||||
multiple targets, each `delivery.Task` is sent to the delivery channel.
|
||||
multiple targets, each `DeliveryTask` is sent to the delivery channel.
|
||||
Workers pick them up and process them — no goroutine-per-target.
|
||||
- **Worker goroutines:** A fixed number of worker goroutines select from
|
||||
both channels. Each worker processes one task at a time, then picks up
|
||||
@@ -866,12 +810,7 @@ This means:
|
||||
- **Independent results** — each worker records its own delivery result
|
||||
in the per-webhook database without coordination.
|
||||
- **Graceful shutdown** — cancel the context, workers finish their
|
||||
current task and exit. The stop hook waits for the pool via
|
||||
`lifecycle.WaitForShutdown`, which bounds that wait by fx's stop
|
||||
timeout rather than blocking forever on a wedged worker. On timeout
|
||||
it logs at `ERROR` and returns an error, and the goroutines that
|
||||
did not finish are still running — an unclean shutdown is reported
|
||||
rather than hidden.
|
||||
current task and exit. `WaitGroup.Wait()` ensures clean shutdown.
|
||||
|
||||
**Recovery paths:**
|
||||
|
||||
@@ -899,13 +838,12 @@ remains stored in the per-webhook event database, there is no way to
|
||||
redeliver it: manual redelivery is planned, not implemented (see
|
||||
[TODO.md](TODO.md)).
|
||||
|
||||
### Circuit Breaker (HTTP and Slack Targets with Retries)
|
||||
### Circuit Breaker (HTTP Targets with Retries)
|
||||
|
||||
`http` and `slack` targets with `max_retries` > 0 are protected by a
|
||||
**per-target circuit breaker** that prevents hammering a down target
|
||||
with repeated failed delivery attempts. The circuit breaker is
|
||||
in-memory only and resets on restart (which is fine — startup recovery
|
||||
rescans the database anyway).
|
||||
HTTP targets with `max_retries` > 0 are protected by a **per-target circuit breaker** that
|
||||
prevents hammering a down target with repeated failed delivery attempts.
|
||||
The circuit breaker is in-memory only and resets on restart (which is
|
||||
fine — startup recovery rescans the database anyway).
|
||||
|
||||
**States:**
|
||||
|
||||
@@ -941,12 +879,10 @@ rescans the database anyway).
|
||||
- **Failure threshold:** 5 consecutive failures before opening
|
||||
- **Cooldown:** 30 seconds in open state before probing
|
||||
|
||||
**Scope:** Circuit breakers apply to **`http` and `slack` targets with
|
||||
`max_retries` > 0**. The Slack target is built on the same HTTP core
|
||||
and hands its own `max_retries` to the same retry path, so it gets a
|
||||
breaker with the same 5-failure / 30-second defaults. Fire-and-forget
|
||||
targets of either type (`max_retries` == 0), database targets (local
|
||||
operations), and log targets (stdout) do not use circuit breakers.
|
||||
**Scope:** Circuit breakers only apply to **HTTP targets with
|
||||
`max_retries` > 0**. Fire-and-forget HTTP targets (`max_retries` == 0),
|
||||
Slack targets, database targets (local operations), and log
|
||||
targets (stdout) do not use circuit breakers.
|
||||
|
||||
When a circuit is open and a new delivery arrives, the engine marks the
|
||||
delivery as `retrying` and schedules a retry timer for after the
|
||||
@@ -962,11 +898,9 @@ unpredictable rates, and blanket limits shared with other routes would
|
||||
cause legitimate deliveries to be dropped.
|
||||
|
||||
The receiver instead has its own dedicated abuse limit, scoped to the
|
||||
`/webhook/{uuid}` route only and keyed per client IP per request path
|
||||
(`httprate.KeyByEndpoint`): one misbehaving sender is throttled without
|
||||
affecting other senders of the same entrypoint or the same sender's
|
||||
other entrypoints. Keying on the path rather than on the entrypoint
|
||||
matters — see the aggregate limit below. The limit is
|
||||
`/webhook/{uuid}` route only and keyed per client IP per entrypoint: one
|
||||
misbehaving sender is throttled without affecting other senders of the
|
||||
same entrypoint or the same sender's other entrypoints. The limit is
|
||||
`RECEIVER_RATE_LIMIT` requests per minute (default 120, generous for
|
||||
legitimate webhook senders). Requests over the limit receive HTTP 429
|
||||
with a `Retry-After` header. A set-but-invalid `RECEIVER_RATE_LIMIT`
|
||||
@@ -1026,16 +960,14 @@ reading; an absent one is not. A cut value ends in `[truncated]`, which
|
||||
is charged on top of the budget rather than inside it.
|
||||
|
||||
Each budget is spent in _encoded_ bytes, not in the bytes the client
|
||||
sent. Every rune is charged what the wider of the two log handlers
|
||||
emits for it: two bytes for a quotation mark, a backslash or a tab; six
|
||||
for a non-printable rune below U+10000; ten for one at or above it,
|
||||
which the text handler spells `\UXXXXXXXX`. Go's header parser accepts
|
||||
all of them in a header value, so a budget counted raw would buy a
|
||||
field several times its nominal size — and the line, not the header, is
|
||||
what an operator has to store. Plain ASCII encodes one byte for one, so
|
||||
a real browser's `User-Agent` still fits whole; a value built out of
|
||||
escapes keeps a proportionally shorter prefix, which is the right
|
||||
trade.
|
||||
sent. The log handler escapes a quotation mark, a backslash and a tab
|
||||
to two bytes each and a non-printable rune to six, and Go's header
|
||||
parser accepts all of them in a header value, so a budget counted raw
|
||||
would buy a field twice its nominal size — and the line, not the
|
||||
header, is what an operator has to store. Plain ASCII encodes one byte
|
||||
for one, so a real browser's `User-Agent` still fits whole; a value
|
||||
built out of escapes keeps a proportionally shorter prefix, which is
|
||||
the right trade.
|
||||
|
||||
Net: **one `INFO` line per request, of at most 2,560 bytes.** That
|
||||
ceiling is arithmetic, not an observation: 3 × (512 + 11) for `url`,
|
||||
@@ -1046,13 +978,10 @@ a zone, the status and the latency) — 2,087 bytes, stated at 2,560 so
|
||||
the figure has headroom. `internal/middleware/accesslog_test.go`
|
||||
asserts it against 8 KB of client-chosen text in the path, in the
|
||||
query, and in each of `User-Agent`, `Referer` and `X-Request-Id`,
|
||||
including cases built from the characters the handlers escape, and
|
||||
including cases built from the characters the handler escapes, and
|
||||
against the widest line the service can be made to write: a 5xx that
|
||||
keeps its concrete path while all three header fields are also at their
|
||||
budget. Every case runs through both handlers `internal/logger` can
|
||||
select — the JSON one and the text one it installs on a tty — since the
|
||||
two do not escape alike and the ceiling is quoted unqualified. Measured
|
||||
over a real connection, the widest line is 1,972 bytes.
|
||||
budget. Measured over a real connection, that line is 1,972 bytes.
|
||||
|
||||
Multiply that ceiling by the request rate to size log storage. Note
|
||||
that the rate is not bounded by the limits above on every route:
|
||||
@@ -1063,14 +992,7 @@ 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. That address becomes a bucket by family: IPv4 keys on the full
|
||||
address, IPv6 on its `/64` prefix. A routed `/64` is the normal
|
||||
residential and mobile IPv6 allocation, so keying IPv6 per address would
|
||||
let one subscriber rotate source addresses and mint a fresh bucket per
|
||||
request, evading these limits at the network layer without spoofing
|
||||
anything; the cost is that distinct clients inside one `/64` share a
|
||||
bucket. IPv4-mapped addresses (`::ffff:1.2.3.4`) key as the IPv4 address
|
||||
they carry. See [Trusted proxies](#trusted-proxies). Deployed without that
|
||||
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
|
||||
@@ -1093,8 +1015,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 whenever it is
|
||||
empty, in any environment.
|
||||
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
|
||||
@@ -1106,17 +1028,17 @@ abuse limit later; they are tracked as future work.
|
||||
|
||||
| Method | Path | Description |
|
||||
| ------ | --------------------------- | ----------- |
|
||||
| `GET` | `/` | Root redirect, 303 (authenticated → `/sources`, unauthenticated → `/pages/login`) |
|
||||
| `GET` | `/.well-known/healthcheck` | Health check (JSON: `status`, `now`, `uptimeSeconds`, `uptimeHuman`, `version`, `appname`, `maintenanceMode`) |
|
||||
| any | `/s/*` | Static file serving (embedded CSS, JS). Mounted for every method, not just `GET`/`HEAD`: chi's `Mount` registers all methods and `http.FileServer` special-cases only `HEAD` (by omitting the body), so a `POST` or `DELETE` to an asset is answered `200` with the file. Pinned by `TestStaticServesEveryMethod` |
|
||||
| `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)) |
|
||||
| `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) |
|
||||
|
||||
#### Authentication Endpoints
|
||||
|
||||
| Method | Path | Description |
|
||||
| ------ | --------------- | ----------- |
|
||||
| `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) |
|
||||
| `GET` | `/pages/login` | Login page |
|
||||
| `POST` | `/pages/login` | Login form submission |
|
||||
| `POST` | `/pages/logout` | Logout (destroys session) |
|
||||
|
||||
#### Authenticated Endpoints
|
||||
@@ -1124,7 +1046,6 @@ 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 |
|
||||
@@ -1134,17 +1055,13 @@ 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, behind basic auth. The route is registered only when `METRICS_USERNAME` is set; otherwise it does not exist and returns 404 |
|
||||
| `GET` | `/metrics` | Prometheus metrics (requires basic auth) |
|
||||
|
||||
#### API (Planned)
|
||||
|
||||
@@ -1158,10 +1075,8 @@ 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 |
|
||||
|
||||
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.
|
||||
API authentication will use API keys passed via `Authorization: Bearer
|
||||
<key>` header.
|
||||
|
||||
### Package Layout
|
||||
|
||||
@@ -1189,28 +1104,16 @@ 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)
|
||||
│ │ ├── testing.go # NewTestDatabase: wrapper for tests, no fx lifecycle
|
||||
│ │ └── 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/slack 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
|
||||
│ │ ├── circuit_breaker.go # Per-target circuit breaker for HTTP targets with retries
|
||||
│ │ └── ssrf.go # SSRF prevention (IP validation, safe HTTP transport)
|
||||
│ ├── handlers/
|
||||
│ │ ├── handlers.go # Base handler struct, JSON helpers, template rendering
|
||||
│ │ ├── auth.go # Login, logout handlers
|
||||
│ │ ├── event_log_view.go # Event log projection, byte-capped in SQL
|
||||
│ │ ├── healthcheck.go # Health check handler
|
||||
│ │ ├── index.go # Index page handler
|
||||
│ │ ├── profile.go # User profile handler
|
||||
@@ -1218,34 +1121,25 @@ webhooker/
|
||||
│ │ └── webhook.go # Webhook receiver handler
|
||||
│ ├── healthcheck/
|
||||
│ │ └── healthcheck.go # Health check service (uptime, version)
|
||||
│ ├── lifecycle/
|
||||
│ │ └── lifecycle.go # Shared stop-hook waiter, bounded by the stop context
|
||||
│ ├── logger/
|
||||
│ │ └── logger.go # slog setup with TTY detection
|
||||
│ ├── middleware/
|
||||
│ │ ├── middleware.go # Logging, CORS, Auth, Metrics, MetricsAuth, SecurityHeaders, MaxBodySize
|
||||
│ │ ├── csrf.go # CSRF protection middleware (gorilla/csrf)
|
||||
│ │ ├── ratelimit.go # Per-IP rate limiting middleware (go-chi/httprate)
|
||||
│ │ └── testing.go # NewForTest: Middleware without the fx lifecycle
|
||||
│ │ └── ratelimit.go # Per-IP rate limiting middleware (go-chi/httprate)
|
||||
│ ├── server/
|
||||
│ │ ├── server.go # Server struct, fx lifecycle, signal handling
|
||||
│ │ ├── http.go # HTTP server setup with timeouts
|
||||
│ │ └── routes.go # All route definitions
|
||||
│ └── session/
|
||||
│ ├── session.go # Cookie-based session management
|
||||
│ └── testing.go # NewForTest: Session without the fx lifecycle
|
||||
│ └── session.go # Cookie-based session management
|
||||
├── static/
|
||||
│ ├── static.go # //go:embed directive
|
||||
│ ├── 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/app.js # Progressive-enhancement copy-to-clipboard
|
||||
│ ├── js/alpine.min.js # Alpine.js, fetched by script/fetch-assets, not committed
|
||||
│ └── vendor.sha256 # Pinned hashes the fetched assets are verified against
|
||||
├── templates/ # Go HTML templates (base, login, sources, etc.)
|
||||
├── script/ # Scripts to Rule Them All entrypoints
|
||||
├── Dockerfile # Three stages: lint, test+build, Alpine runtime
|
||||
├── Makefile # 10 of 16 targets shim script/; 6 are inline
|
||||
│ ├── 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
|
||||
├── go.mod / go.sum
|
||||
└── .golangci.yml # Linter configuration
|
||||
```
|
||||
@@ -1261,27 +1155,21 @@ Components are wired via Uber fx in this order:
|
||||
user seed
|
||||
5. `database.NewWebhookDBManager` — Per-webhook event database
|
||||
lifecycle manager
|
||||
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
|
||||
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
|
||||
|
||||
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
|
||||
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
|
||||
package dependency. Each task carries all target config and event data
|
||||
inline (for bodies under 16 KiB, `delivery.MaxInlineBodySize`), so the
|
||||
engine can deliver without reading from any database — it only writes
|
||||
to record results.
|
||||
inline (for bodies ≤16KB), so the engine can deliver without reading
|
||||
from any database — it only writes to record results.
|
||||
|
||||
### Middleware Stack
|
||||
|
||||
@@ -1307,32 +1195,16 @@ 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` 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 global entries listed above all run first, so
|
||||
such a request is still logged and given the security headers — and
|
||||
counted in the metrics, on a deployment where `METRICS_USERNAME` is
|
||||
set and the Metrics middleware is therefore registered at all. 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}`.
|
||||
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.
|
||||
|
||||
### Authentication
|
||||
|
||||
- **Web UI:** Cookie-based sessions using gorilla/sessions with
|
||||
encrypted cookies. Sessions are configured with HttpOnly, SameSite
|
||||
Lax, and Secure (in production). Absolute session lifetime is 7 days,
|
||||
with a sliding idle timeout on top of it (see
|
||||
[Sessions](#sessions)).
|
||||
Lax, and Secure (in production). Session lifetime is 7 days.
|
||||
- **API (planned):** API key authentication via `Authorization: Bearer`
|
||||
header. API keys are stored per-user with usage tracking
|
||||
(`last_used_at`).
|
||||
@@ -1364,125 +1236,33 @@ rather than global: **LoginRateLimit** on `/pages/login`,
|
||||
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. 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
|
||||
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)
|
||||
- Container runs as non-root user (UID 1000)
|
||||
- GORM soft deletes on every entity that carries `BaseModel`, which is
|
||||
all of them but `Setting` (data preserved for audit)
|
||||
|
||||
### Shutdown
|
||||
|
||||
On SIGINT or SIGTERM, fx runs the registered stop hooks in reverse
|
||||
dependency order under a **5 second budget** (`fx.StopTimeout` in
|
||||
`cmd/webhooker/main.go`). That budget covers the whole sequence, not
|
||||
each hook. The order, read off the fx stop-hook log:
|
||||
|
||||
1. `ArchiveSweeper`
|
||||
2. `RetentionReaper`
|
||||
3. `server` — the HTTP drain, bounded separately by
|
||||
`server.ShutdownTimeout` (**3 seconds**), then a Sentry flush if
|
||||
`SENTRY_DSN` is set
|
||||
4. `delivery.Engine`
|
||||
5. `healthcheck`
|
||||
6. `WebhookDBManager`
|
||||
7. the database close
|
||||
|
||||
The two components that can realistically hold the budget run
|
||||
first: a retention sweep or an archive prune caught mid-tick each
|
||||
waits on its `WaitGroup` bounded by the stop context, so a wedge
|
||||
there consumes the 5 seconds before the HTTP server hook is ever
|
||||
entered. The hooks after the server are microsecond-scale in normal
|
||||
operation.
|
||||
|
||||
The HTTP drain budget is deliberately **shorter** than the sequence
|
||||
budget. Were the two equal, a drain that used its whole budget would
|
||||
exhaust the sequence budget at the instant it finished, and every
|
||||
later hook — the delivery engine, the healthcheck, the webhook DB
|
||||
manager and the database close — would be skipped in exactly the
|
||||
case where the drain mattered. 3 seconds leaves 2 seconds
|
||||
(`server.TailHookReserve`) for the tail, which is far more than the
|
||||
microseconds it needs.
|
||||
|
||||
That reserve belongs to the tail hooks, not to the server hook, and
|
||||
the Sentry flush is what could take it: it runs after the drain
|
||||
**inside the same hook**, and `sentry.Flush` takes a bare duration
|
||||
and honours no context, so an unreachable Sentry endpoint would add
|
||||
its own timeout on top of a full-length drain and consume the whole
|
||||
sequence budget by itself. It is therefore clamped to whatever is
|
||||
left on the stop context minus the reserve, and skipped when that
|
||||
leaves too little to be worth attempting — so a full-length drain
|
||||
means Sentry events are dropped rather than the database close being
|
||||
skipped.
|
||||
|
||||
This does not make the database close unconditional: a wedged
|
||||
`ArchiveSweeper` or `RetentionReaper` still runs first and can
|
||||
consume the whole budget on its own.
|
||||
|
||||
The value is chosen to sit inside the container stop grace period.
|
||||
Docker's default `docker stop` grace is 10 seconds and the Dockerfile
|
||||
sets no `STOPSIGNAL` or grace override, so the process must be gone
|
||||
before that. fx's own default is 15 seconds, which is past the grace:
|
||||
the container would be SIGKILLed (exit 137) before the bound could
|
||||
fire, and nothing that depends on it — including the
|
||||
`shutdown timed out, goroutines still running` error log that tells
|
||||
an operator a component is wedged — would ever be reached.
|
||||
|
||||
Two operational consequences follow from bounding the sequence:
|
||||
|
||||
- **A wedged component aborts the rest of the shutdown.** fx checks
|
||||
the stop context before each remaining hook and returns outright
|
||||
once it has expired, skipping the hooks it has not reached. If the
|
||||
first-stopped component consumes the whole budget, the later hooks
|
||||
never run — **the database close among them**. SQLite is crash-safe,
|
||||
so this is not corruption, but it is not a clean close either.
|
||||
- **Lowering the grace below 5 seconds reintroduces the silent
|
||||
truncation.** `docker stop --time`, Compose's `stop_grace_period`,
|
||||
or Kubernetes' `terminationGracePeriodSeconds` set under 5 seconds
|
||||
put SIGKILL back in front of the bound, and the process dies with
|
||||
no shutdown diagnostics at all. Keep the deployment's grace above
|
||||
the stop timeout.
|
||||
- GORM soft deletes on all entities (data preserved for audit)
|
||||
|
||||
### Docker
|
||||
|
||||
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:
|
||||
The Dockerfile uses a multi-stage build:
|
||||
|
||||
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), runs `script/fetch-assets`
|
||||
to download and verify the third-party browser assets, 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`.
|
||||
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.
|
||||
|
||||
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.
|
||||
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.
|
||||
|
||||
`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. Only `script/cibuild` and
|
||||
`script/docker` involve Docker. `script/lint`, and therefore
|
||||
`make lint` and `make check`, run whatever `golangci-lint` is on the
|
||||
host, which can be a different version from the pinned one — so the
|
||||
container is the authoritative lint result
|
||||
([issue #109](https://git.eeqj.de/sneak/webhooker/issues/109) tracks
|
||||
routing local linting through it as well).
|
||||
`docker build .` is the CI gate — if it passes, the code is formatted,
|
||||
linted, tested, and compiled.
|
||||
|
||||
#### CI gate honesty
|
||||
|
||||
@@ -1498,17 +1278,16 @@ 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`, `LICENSE` and `.editorconfig` from the context
|
||||
anyway — so the image replays from cache and costs seconds.
|
||||
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.
|
||||
|
||||
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
|
||||
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.
|
||||
|
||||
22
TODO.md
22
TODO.md
@@ -25,17 +25,13 @@ 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 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.
|
||||
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 Step
|
||||
|
||||
@@ -194,9 +190,7 @@ rate-limit keys should bucket by `/64`).
|
||||
- OpenAPI specification
|
||||
- Analytics dashboard: success rates, response times, volume
|
||||
- A remember-me option at login
|
||||
- Password reset flow for a forgotten password. The authenticated
|
||||
password *change* flow already landed on `main` (#65); reset does not
|
||||
exist
|
||||
- Password change and reset flow
|
||||
- Later, nice to have
|
||||
- email delivery target type
|
||||
- SNS and S3 delivery targets
|
||||
|
||||
@@ -2,8 +2,6 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"time"
|
||||
|
||||
"go.uber.org/fx"
|
||||
"sneak.berlin/go/webhooker/internal/config"
|
||||
"sneak.berlin/go/webhooker/internal/database"
|
||||
@@ -17,33 +15,6 @@ import (
|
||||
"sneak.berlin/go/webhooker/internal/session"
|
||||
)
|
||||
|
||||
// stopTimeout bounds the whole fx stop sequence, not each hook.
|
||||
//
|
||||
// fx defaults to 15s, which is longer than Docker's 10s default
|
||||
// stop grace: the container would be SIGKILLed before the bound
|
||||
// could fire, so nothing bounded by it would ever be observed.
|
||||
// 5s leaves headroom inside that grace for signal delivery and
|
||||
// process exit; the observed wedge case already exits at ~5.3s,
|
||||
// so a larger bound would trade a rare skipped database close for
|
||||
// a more common hard kill.
|
||||
//
|
||||
// The server's stop hook must fit inside it with room to spare: a
|
||||
// hook that used the whole budget would exhaust it at that instant,
|
||||
// and fx would skip every hook after the server — the delivery
|
||||
// engine, the healthcheck, the webhook DB manager and the database
|
||||
// close. That hook is the 3s HTTP drain plus the Sentry flush that
|
||||
// follows it in the same hook, so the flush is clamped to the stop
|
||||
// context's remaining time less server.TailHookReserve rather than
|
||||
// running for its own fixed 2s; the reserve is what the tail hooks
|
||||
// live on, and they are microsecond-scale in normal operation.
|
||||
// TestStopTimeout_LeavesHeadroomForTailHooks pins the arithmetic
|
||||
// across every drain length.
|
||||
//
|
||||
// This does not make the database close unconditional: the
|
||||
// ArchiveSweeper and RetentionReaper hooks run before the server
|
||||
// and can still consume the whole budget on their own.
|
||||
const stopTimeout = 5 * time.Second
|
||||
|
||||
// Build-time variables set via -ldflags.
|
||||
//
|
||||
//nolint:gochecknoglobals // Build-time variables injected by the linker.
|
||||
@@ -56,14 +27,7 @@ func main() {
|
||||
globals.Appname = appname
|
||||
globals.Version = version
|
||||
|
||||
newApp().Run()
|
||||
}
|
||||
|
||||
// newApp builds the application graph. It is separate from main so
|
||||
// a test can assert the options it carries.
|
||||
func newApp() *fx.App {
|
||||
return fx.New(
|
||||
fx.StopTimeout(stopTimeout),
|
||||
fx.New(
|
||||
fx.Provide(
|
||||
globals.New,
|
||||
logger.New,
|
||||
@@ -96,5 +60,5 @@ func newApp() *fx.App {
|
||||
) {
|
||||
},
|
||||
),
|
||||
)
|
||||
).Run()
|
||||
}
|
||||
|
||||
@@ -1,75 +0,0 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
"sneak.berlin/go/webhooker/internal/server"
|
||||
)
|
||||
|
||||
// dockerStopGrace is Docker's default `docker stop` grace period.
|
||||
// The Dockerfile sets no STOPSIGNAL or grace override, so this is
|
||||
// the deadline the container is actually held to, and the fx stop
|
||||
// timeout has to fit inside it with room for signal delivery and
|
||||
// process exit.
|
||||
const dockerStopGrace = 10 * time.Second
|
||||
|
||||
// TestNewApp_StopTimeout pins the fx stop timeout. Without the
|
||||
// explicit fx.StopTimeout option the app reads fx's 15s
|
||||
// DefaultTimeout, which exceeds dockerStopGrace: the container is
|
||||
// SIGKILLed before the bound fires and every shutdown hook bounded
|
||||
// by it — including the operator-facing timeout log — becomes
|
||||
// unreachable in the image this repo produces.
|
||||
//
|
||||
// fx.New applies options before it executes invokes, so the timeout
|
||||
// is set whether or not the graph itself can be constructed here.
|
||||
func TestNewApp_StopTimeout(t *testing.T) {
|
||||
t.Setenv("DATA_DIR", t.TempDir())
|
||||
|
||||
got := newApp().StopTimeout()
|
||||
|
||||
require.Equal(t, stopTimeout, got)
|
||||
require.Less(t, got, dockerStopGrace)
|
||||
}
|
||||
|
||||
// tailHeadroom is the slack the fx stop budget must keep beyond the
|
||||
// server stop hook. The hooks that run after the server — the
|
||||
// delivery engine, the healthcheck, the webhook DB manager and the
|
||||
// database close — are microsecond-scale in normal operation, so
|
||||
// this is generous for them.
|
||||
const tailHeadroom = 2 * time.Second
|
||||
|
||||
// TestStopTimeout_LeavesHeadroomForTailHooks pins the relationship
|
||||
// between the server's stop hook and the fx stop budget. fx bounds
|
||||
// the whole stop sequence, and returns without running its
|
||||
// remaining hooks once the stop context has expired. If the hook
|
||||
// could use the entire budget, every later hook — the database close
|
||||
// included — would be skipped in exactly the case where the drain
|
||||
// mattered.
|
||||
//
|
||||
// The hook is not just the HTTP drain: a Sentry flush follows it in
|
||||
// the same hook, and sentry.Flush honours no context, so both halves
|
||||
// have to be counted. The sweep walks every drain length the hook
|
||||
// can produce, since a shorter drain leaves the flush more room and
|
||||
// the worst case is not necessarily at either extreme.
|
||||
//
|
||||
// Shrinking either budget, or unbounding the flush again, must fail
|
||||
// here rather than silently recreating a hook that swallows the
|
||||
// whole sequence.
|
||||
func TestStopTimeout_LeavesHeadroomForTailHooks(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
require.Less(t, server.ShutdownTimeout, stopTimeout)
|
||||
|
||||
const step = 10 * time.Millisecond
|
||||
|
||||
for drain := time.Duration(0); drain <= server.ShutdownTimeout; drain += step {
|
||||
hook := drain + server.SentryFlushBudget(stopTimeout-drain)
|
||||
|
||||
require.LessOrEqual(
|
||||
t, hook+tailHeadroom, stopTimeout,
|
||||
"a %s drain leaves the tail hooks short", drain,
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -422,43 +422,33 @@ func loadFromEnv() (*Config, error) {
|
||||
}, nil
|
||||
}
|
||||
|
||||
// warnSharedRateLimitBucket logs a startup warning whenever
|
||||
// TRUSTED_PROXIES is empty, in any environment.
|
||||
// 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. 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
|
||||
// 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 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.
|
||||
// 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 len(c.TrustedProxies) > 0 {
|
||||
if !c.IsProd() || len(c.TrustedProxies) > 0 {
|
||||
return
|
||||
}
|
||||
|
||||
log.Warn(
|
||||
"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.",
|
||||
"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),
|
||||
)
|
||||
@@ -501,10 +491,6 @@ 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 != "",
|
||||
|
||||
@@ -628,12 +628,10 @@ func testTrustedProxiesSuccess(
|
||||
}
|
||||
|
||||
// TestSharedRateLimitBucketWarning covers the startup warning that
|
||||
// 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.
|
||||
// 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
|
||||
@@ -653,19 +651,12 @@ func TestSharedRateLimitBucketWarning(t *testing.T) {
|
||||
expectWarning: false,
|
||||
},
|
||||
{
|
||||
// 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",
|
||||
// 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: true,
|
||||
},
|
||||
{
|
||||
name: "dev with trusted proxies is quiet",
|
||||
environment: config.EnvironmentDev,
|
||||
trustedProxies: cidrPrivateV4,
|
||||
expectWarning: false,
|
||||
expectWarning: false,
|
||||
},
|
||||
}
|
||||
|
||||
@@ -706,14 +697,8 @@ func TestSharedRateLimitBucketWarning(t *testing.T) {
|
||||
|
||||
assert.Contains(t, logged, `"level":"WARN"`)
|
||||
assert.Contains(t, logged, "TRUSTED_PROXIES")
|
||||
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",
|
||||
)
|
||||
assert.Contains(t, logged, "shares one bucket")
|
||||
assert.Contains(t, logged, "deny the admin login")
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,21 +0,0 @@
|
||||
package lifecycle
|
||||
|
||||
import (
|
||||
"context"
|
||||
"log/slog"
|
||||
)
|
||||
|
||||
// WaitDone exposes waitDone to the external test package. Only the
|
||||
// unexported waiter can be handed a channel that is already closed
|
||||
// before the call, which is the state the preamble exists for;
|
||||
// through WaitForShutdown the waiter goroutine may or may not have
|
||||
// closed the channel yet, so the case is not reachable
|
||||
// deterministically from outside.
|
||||
func WaitDone(
|
||||
ctx context.Context,
|
||||
log *slog.Logger,
|
||||
component string,
|
||||
done <-chan struct{},
|
||||
) error {
|
||||
return waitDone(ctx, log, component, done)
|
||||
}
|
||||
@@ -38,29 +38,6 @@ func WaitForShutdown(
|
||||
wg.Wait()
|
||||
}()
|
||||
|
||||
return waitDone(ctx, log, component, done)
|
||||
}
|
||||
|
||||
// waitDone waits for done to close, bounded by ctx.
|
||||
//
|
||||
// The non-blocking preamble is load-bearing. When the component has
|
||||
// already drained and ctx has already expired, both cases of the
|
||||
// bounded select are ready and Go picks between them uniformly at
|
||||
// random, so a clean shutdown would be reported as a timeout about
|
||||
// half the time. Draining wins: the goroutines are gone, and there
|
||||
// is nothing left for the operator to act on.
|
||||
func waitDone(
|
||||
ctx context.Context,
|
||||
log *slog.Logger,
|
||||
component string,
|
||||
done <-chan struct{},
|
||||
) error {
|
||||
select {
|
||||
case <-done:
|
||||
return nil
|
||||
default:
|
||||
}
|
||||
|
||||
select {
|
||||
case <-done:
|
||||
return nil
|
||||
|
||||
@@ -37,57 +37,6 @@ func TestWaitForShutdown_DrainedGroup(t *testing.T) {
|
||||
)
|
||||
}
|
||||
|
||||
// racePasses is how many times the both-cases-ready race is run.
|
||||
// Without the preamble each pass is an independent coin flip, so
|
||||
// the probability of the whole loop passing by luck is 2^-N: at
|
||||
// this N the test is deterministic in practice, and it involves no
|
||||
// wall-clock waiting at all.
|
||||
const racePasses = 1000
|
||||
|
||||
// TestWaitDone_DrainedBeforeExpiredContext covers the case where a
|
||||
// component drained cleanly but the stop context had already
|
||||
// expired. Both select cases are ready, and Go chooses among ready
|
||||
// cases uniformly at random, so the drained case must be settled by
|
||||
// the preamble before the bounded select ever runs.
|
||||
func TestWaitDone_DrainedBeforeExpiredContext(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
done := make(chan struct{})
|
||||
close(done)
|
||||
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
cancel()
|
||||
|
||||
for pass := range racePasses {
|
||||
require.NoErrorf(
|
||||
t,
|
||||
lifecycle.WaitDone(
|
||||
ctx, discardLogger(), "test component", done,
|
||||
),
|
||||
"pass %d reported a timeout for a drained component",
|
||||
pass,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// TestWaitDone_ExpiredContext pins the other side of the preamble:
|
||||
// an expired context with a component that has not drained is still
|
||||
// a timeout.
|
||||
func TestWaitDone_ExpiredContext(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
cancel()
|
||||
|
||||
err := lifecycle.WaitDone(
|
||||
ctx, discardLogger(), "test component",
|
||||
make(chan struct{}),
|
||||
)
|
||||
|
||||
require.ErrorIs(t, err, context.Canceled)
|
||||
require.ErrorContains(t, err, "test component")
|
||||
}
|
||||
|
||||
func TestWaitForShutdown_ContextExpires(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
@@ -55,7 +55,6 @@ const tailMarker = "QQTRUNCATEDTAILQQ"
|
||||
const (
|
||||
maxFieldBytes = 512
|
||||
maxRequestIDBytes = 128
|
||||
maxMethodBytes = 32
|
||||
truncationSuffix = "[truncated]"
|
||||
unmatchedRouteLiteral = "(unmatched)"
|
||||
)
|
||||
@@ -77,27 +76,6 @@ func capturingMiddleware(t *testing.T) (*middleware.Middleware, *bytes.Buffer) {
|
||||
return middleware.NewForTest(log, cfg, nil), buf
|
||||
}
|
||||
|
||||
// capturingTextMiddleware is capturingMiddleware for the other handler
|
||||
// internal/logger can select: slog's text handler, which
|
||||
// internal/logger/logger.go installs when stderr is a tty. It escapes
|
||||
// differently from the JSON one, so the line bound has to be asserted
|
||||
// against both.
|
||||
func capturingTextMiddleware(
|
||||
t *testing.T,
|
||||
) (*middleware.Middleware, *bytes.Buffer) {
|
||||
t.Helper()
|
||||
|
||||
buf := new(bytes.Buffer)
|
||||
log := slog.New(slog.NewTextHandler(
|
||||
buf,
|
||||
&slog.HandlerOptions{Level: slog.LevelInfo},
|
||||
))
|
||||
|
||||
cfg := &config.Config{Environment: config.EnvironmentDev}
|
||||
|
||||
return middleware.NewForTest(log, cfg, nil), buf
|
||||
}
|
||||
|
||||
// accessLogRouter mirrors the production route shapes that an
|
||||
// unauthenticated client can reach: the public receiver, the
|
||||
// authenticated profile route (which redirects to login rather than
|
||||
@@ -380,23 +358,14 @@ func lineSizeCases() map[string]sizeCase {
|
||||
longPath := "/boom/" + strings.Repeat("x", oversizedSegmentBytes)
|
||||
wantLongURL := longPath[:maxFieldBytes] + truncationSuffix
|
||||
|
||||
// escapeChars are the runes Go's header parser accepts in a header
|
||||
// value and the log handler then escapes, coming out wider than
|
||||
// they went in. A budget counted in raw bytes lets any of them buy
|
||||
// a field several times its nominal size, so every one of them
|
||||
// gets a case.
|
||||
//
|
||||
// The astral one is the case the JSON handler alone does not
|
||||
// reach: U+1000C is unassigned, so it is non-printable, and
|
||||
// strconv.Quote spells a non-printable rune at or above U+10000
|
||||
// as a ten-byte \UXXXXXXXX. The JSON handler passes it through as
|
||||
// its four UTF-8 bytes, so only the text-handler shape of this
|
||||
// test holds the ten-byte charge honest.
|
||||
// escapeChars are the bytes Go's header parser accepts in a header
|
||||
// value and the log handler then escapes, one byte in and two or
|
||||
// more out. A budget counted in raw bytes lets any of them buy a
|
||||
// field twice its nominal size, so every one of them gets a case.
|
||||
escapeChars := map[string]string{
|
||||
"quote": `"`,
|
||||
"backslash": `\`,
|
||||
"tab": "\t",
|
||||
"astral": "\U0001000C",
|
||||
}
|
||||
|
||||
for kind, char := range escapeChars {
|
||||
@@ -475,91 +444,6 @@ func TestAccessLog_LineSizeDoesNotTrackInputSize(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestAccessLog_LineSizeDoesNotTrackInputSizeOnTheTextHandler runs the
|
||||
// same cases through slog's text handler, which internal/logger
|
||||
// selects on a tty.
|
||||
//
|
||||
// MaxAccessLogLineBytes is quoted to operators unqualified, so it has
|
||||
// to hold for whichever handler is installed — and the two do not
|
||||
// escape alike. The astral case is the one that separates them: the
|
||||
// JSON handler emits U+1000C as its four UTF-8 bytes, while
|
||||
// strconv.Quote spells it \U0001000C at ten. Charging six for it, as
|
||||
// this code did, put a real 2,676-byte line on the wire here while
|
||||
// every JSON case stayed comfortably inside the bound.
|
||||
//
|
||||
// Only the size bound is asserted; the url field's contents are the
|
||||
// JSON shape's business above.
|
||||
func TestAccessLog_LineSizeDoesNotTrackInputSizeOnTheTextHandler(
|
||||
t *testing.T,
|
||||
) {
|
||||
t.Parallel()
|
||||
|
||||
for name, tc := range lineSizeCases() {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
m, buf := capturingTextMiddleware(t)
|
||||
router := accessLogRouter(m)
|
||||
|
||||
assert.Equal(
|
||||
t,
|
||||
tc.wantStatus,
|
||||
getWithHeaders(t, router, tc.target, tc.headers),
|
||||
)
|
||||
|
||||
line := strings.TrimSpace(buf.String())
|
||||
|
||||
require.NotEmpty(t, line)
|
||||
assert.NotContains(
|
||||
t, line, "\n", "expected exactly one log line",
|
||||
)
|
||||
require.LessOrEqual(
|
||||
t, len(line), tc.bound,
|
||||
"access log line exceeded its bound",
|
||||
)
|
||||
assert.Contains(t, line, "url=")
|
||||
assert.NotContains(
|
||||
t, line, attackerMarker,
|
||||
"access log carried attacker-chosen text",
|
||||
)
|
||||
assert.NotContains(
|
||||
t, line, tailMarker,
|
||||
"access log carried an untruncated client field",
|
||||
)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestAccessLog_OversizedMethodIsTruncated covers the last term in the
|
||||
// MaxAccessLogLineBytes arithmetic that the size cases above cannot
|
||||
// reach: Go accepts any RFC 7230 token as a method, and getWithHeaders
|
||||
// only ever sends GET.
|
||||
func TestAccessLog_OversizedMethodIsTruncated(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
m, buf := capturingMiddleware(t)
|
||||
router := accessLogRouter(m)
|
||||
|
||||
method := strings.Repeat("M", oversizedSegmentBytes) + attackerMarker
|
||||
req := httptest.NewRequestWithContext(
|
||||
context.Background(), method, "/"+attackerMarker, nil,
|
||||
)
|
||||
rec := httptest.NewRecorder()
|
||||
router.ServeHTTP(rec, req)
|
||||
|
||||
entries := accessLogEntriesWithin(t, buf, maxLineBytes)
|
||||
require.Len(t, entries, 1)
|
||||
assert.Equal(
|
||||
t,
|
||||
strings.Repeat("M", maxMethodBytes)+truncationSuffix,
|
||||
entries[0]["method"],
|
||||
)
|
||||
assert.NotContains(
|
||||
t, buf.String(), attackerMarker,
|
||||
"access log carried attacker-chosen text",
|
||||
)
|
||||
}
|
||||
|
||||
// TestAccessLog_OversizedHeadersKeepATruncatedPrefix checks the other
|
||||
// half of the header cap: the fields are cut, not dropped, so a
|
||||
// truncated User-Agent is still worth reading.
|
||||
|
||||
@@ -90,13 +90,9 @@ const (
|
||||
// than sitting on the arithmetic.
|
||||
//
|
||||
// The tty text handler in internal/logger is covered by the same
|
||||
// figure. encodedLogFieldBytes charges every rune at least what
|
||||
// the wider of the two handlers emits for it — including the ten
|
||||
// bytes strconv.Quote spends on a non-printable rune at or above
|
||||
// U+10000, which is four more than the JSON handler ever spends —
|
||||
// so each budget bounds the encoded field under either handler.
|
||||
// The text handler's fixed portion is 286, the smaller of the two,
|
||||
// which puts its worst case at 2037.
|
||||
// figure: encodedLogFieldBytes charges the worse of the two
|
||||
// handlers' escapes, and the text handler's fixed portion is the
|
||||
// smaller of the two.
|
||||
MaxAccessLogLineBytes = 2560
|
||||
)
|
||||
|
||||
@@ -173,41 +169,24 @@ func (lrw *loggingResponseWriter) WriteHeader(code int) {
|
||||
//
|
||||
// slog's JSON handler escapes quote, backslash, newline, carriage
|
||||
// return and tab to two bytes each, and every other C0 control plus
|
||||
// LINE SEPARATOR and PARAGRAPH SEPARATOR to a six-byte \u escape; it
|
||||
// passes every other rune through as its own UTF-8. Its text handler
|
||||
// quotes with strconv.Quote, which spells a non-printable rune below
|
||||
// U+10000 as \uXXXX but one at or above U+10000 as \UXXXXXXXX — ten
|
||||
// bytes, not six. The text handler is therefore the worse of the two
|
||||
// for every non-printable rune, and by four bytes apiece for the
|
||||
// 955,086 unassigned, private-use and format code points on planes 1
|
||||
// to 16.
|
||||
//
|
||||
// Charging ten there is what makes MaxAccessLogLineBytes hold for the
|
||||
// tty handler as well: U+1000C encodes as F0 90 80 8C, every byte
|
||||
// >= 0x80, which httpguts.ValidHeaderFieldValue accepts and
|
||||
// net/textproto does not strip, so a header can be filled with them.
|
||||
//
|
||||
// Both handlers pass printable runes through as their own UTF-8, so
|
||||
// unicode.IsPrint separates the escaped cases from the plain ones for
|
||||
// either handler.
|
||||
// LINE SEPARATOR and PARAGRAPH SEPARATOR to a six-byte \u escape. Its
|
||||
// text handler quotes with strconv.Quote, which spells any
|
||||
// non-printable rune the same six-byte way. Both pass printable runes
|
||||
// through as their own UTF-8, so unicode.IsPrint separates the two
|
||||
// cases for either handler. Go's header parser accepts quote,
|
||||
// backslash, tab and non-printable multi-byte runes in a header value,
|
||||
// so every one of these is reachable from a request.
|
||||
func encodedLogFieldBytes(r rune) int {
|
||||
const (
|
||||
// A backslash and the character itself.
|
||||
shortEscapeBytes = 2
|
||||
// \uXXXX, which is also the width of \u00XX.
|
||||
escapedRuneBytes = 6
|
||||
// \UXXXXXXXX, strconv.Quote's spelling of a non-printable
|
||||
// rune outside the basic multilingual plane.
|
||||
escapedAstralRuneBytes = 10
|
||||
// The first code point strconv.Quote spells with \U.
|
||||
firstAstralRune = 0x10000
|
||||
)
|
||||
|
||||
switch {
|
||||
case r == '"' || r == '\\' || r == '\n' || r == '\r' || r == '\t':
|
||||
return shortEscapeBytes
|
||||
case !unicode.IsPrint(r) && r >= firstAstralRune:
|
||||
return escapedAstralRuneBytes
|
||||
case !unicode.IsPrint(r):
|
||||
return escapedRuneBytes
|
||||
default:
|
||||
|
||||
@@ -48,12 +48,6 @@ const (
|
||||
// bound every request pays a walk proportional to whatever the
|
||||
// client sent.
|
||||
maxForwardedHops = 64
|
||||
|
||||
// ipv6BucketBits is the prefix length IPv6 clients are bucketed
|
||||
// on. A routed /64 is the normal residential and mobile
|
||||
// allocation, so it is the unit an attacker gets addresses in
|
||||
// and therefore the unit worth limiting.
|
||||
ipv6BucketBits = 64
|
||||
)
|
||||
|
||||
// normalizeAddr strips the IPv4-in-IPv6 wrapper and any zone from
|
||||
@@ -62,40 +56,6 @@ func normalizeAddr(addr netip.Addr) netip.Addr {
|
||||
return addr.Unmap().WithZone("")
|
||||
}
|
||||
|
||||
// bucketKey is the rate-limit bucket identity of a client address.
|
||||
// IPv4 keys on the full address; IPv6 keys on its /64 prefix,
|
||||
// because keying IPv6 per /128 lets one ordinary subscriber rotate
|
||||
// source addresses inside its own routed /64 and mint a fresh bucket
|
||||
// per request — evading every limiter here at the network layer,
|
||||
// with no spoofing and nothing to detect.
|
||||
//
|
||||
// An IPv4-mapped address (::ffff:1.2.3.4) is keyed as the IPv4
|
||||
// address it carries, never masked to a /64: mapped form all shares
|
||||
// the ::ffff:0:0/96 prefix, so masking would collapse every IPv4
|
||||
// client reaching a proxy that emits it into one bucket. Callers
|
||||
// pass addresses through normalizeAddr, which already unmaps; the
|
||||
// unmap here keeps the property true of the key function itself.
|
||||
//
|
||||
// The two families cannot collide: an IPv4 key is a bare dotted
|
||||
// quad, and an IPv6 key always carries a "/64" suffix.
|
||||
func bucketKey(addr netip.Addr) string {
|
||||
addr = addr.Unmap()
|
||||
|
||||
if addr.Is4() {
|
||||
return addr.String()
|
||||
}
|
||||
|
||||
// Prefix errors only on a negative bit count, on over 32 bits
|
||||
// for an IPv4 address, or on over 128 for IPv6. The count here
|
||||
// is the constant 64 and the IPv4 case returned above, so the
|
||||
// error is unreachable. (The zero Addr does not error either: it
|
||||
// yields the zero Prefix. Neither call site can produce one,
|
||||
// since both parse the address first.)
|
||||
prefix, _ := addr.Prefix(ipv6BucketBits)
|
||||
|
||||
return prefix.String()
|
||||
}
|
||||
|
||||
// isTrustedProxy reports whether addr belongs to a network the
|
||||
// operator listed in TRUSTED_PROXIES. The list is empty by default,
|
||||
// so by default nothing is trusted.
|
||||
@@ -183,9 +143,6 @@ func (m *Middleware) forwardedClientAddr(
|
||||
// another client's bucket, by picking an X-Forwarded-For value —
|
||||
// which makes every limit here decorative against a deliberate
|
||||
// attacker.
|
||||
//
|
||||
// The address that identifies the client is then reduced to a bucket
|
||||
// by bucketKey: full address for IPv4, /64 prefix for IPv6.
|
||||
func (m *Middleware) rateLimitKey(r *http.Request) (string, error) {
|
||||
return m.clientKey(r), nil
|
||||
}
|
||||
@@ -195,25 +152,23 @@ func (m *Middleware) clientKey(r *http.Request) string {
|
||||
peer, err := netip.ParseAddr(ipFromHostPort(r.RemoteAddr))
|
||||
if err != nil {
|
||||
// Not an address we can reason about; key on the raw
|
||||
// value, the most specific identity left. Distinct
|
||||
// RemoteAddr values stay in distinct buckets, so this
|
||||
// path cannot silently collapse unrelated clients
|
||||
// together. On a Unix-socket listener every peer
|
||||
// carries the same RemoteAddr and so shares one bucket,
|
||||
// which is the fail-closed direction.
|
||||
// value, the most specific identity left. On a
|
||||
// Unix-socket listener every peer carries the same
|
||||
// RemoteAddr and so shares one bucket, which is the
|
||||
// fail-closed direction.
|
||||
return r.RemoteAddr
|
||||
}
|
||||
|
||||
peer = normalizeAddr(peer)
|
||||
if !m.isTrustedProxy(peer) {
|
||||
return bucketKey(peer)
|
||||
return peer.String()
|
||||
}
|
||||
|
||||
if addr, ok := m.forwardedClientAddr(r); ok {
|
||||
return bucketKey(addr)
|
||||
return addr.String()
|
||||
}
|
||||
|
||||
return bucketKey(peer)
|
||||
return peer.String()
|
||||
}
|
||||
|
||||
// tooManyRequests returns the 429 handler used by the login,
|
||||
|
||||
@@ -15,7 +15,6 @@ import (
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"sneak.berlin/go/webhooker/internal/config"
|
||||
"sneak.berlin/go/webhooker/internal/middleware"
|
||||
)
|
||||
@@ -371,30 +370,6 @@ const (
|
||||
headerXFF = "X-Forwarded-For"
|
||||
headerReal = "X-Real-IP"
|
||||
headerTrue = "True-Client-IP"
|
||||
|
||||
// clientIPv4 is the sample IPv4 client address these tests key
|
||||
// on, both directly and in IPv4-mapped form. clientIPv4Alt is
|
||||
// its neighbour, used to show the two do not share a bucket.
|
||||
clientIPv4 = "198.51.100.7"
|
||||
clientIPv4Alt = "198.51.100.8"
|
||||
|
||||
// clientIPv6 and clientIPv6Same are two addresses inside one
|
||||
// routed /64, so both must key on clientBucketV6.
|
||||
// clientIPv6Other is a different allocation and must key on
|
||||
// clientOtherBucketV6.
|
||||
clientIPv6 = "2001:db8:1:2:3:4:5:6"
|
||||
clientIPv6Same = "2001:db8:1:2:aaaa:bbbb:cccc:dddd"
|
||||
clientIPv6Other = "2001:db8:1:3::1"
|
||||
clientBucketV6 = "2001:db8:1:2::/64"
|
||||
clientOtherBucketV6 = "2001:db8:1:3::/64"
|
||||
|
||||
// trustedProxyCIDR is the proxy network the forwarded-path
|
||||
// tests configure, and trustedPeer an address inside it. A
|
||||
// production deployment is required to run behind a reverse
|
||||
// proxy with TRUSTED_PROXIES set, so this is the shape the
|
||||
// bucketing has to hold in.
|
||||
trustedProxyCIDR = "10.0.0.0/8"
|
||||
trustedPeer = "10.0.0.1:44444"
|
||||
)
|
||||
|
||||
// assertSharedBucket drives the login limiter from peer with the
|
||||
@@ -483,8 +458,8 @@ func TestRateLimitKey_SingleValuedHeadersIgnoredFromTrustedPeer(
|
||||
t.Parallel()
|
||||
|
||||
assertSharedBucket(
|
||||
t, trustedProxies(trustedProxyCIDR),
|
||||
trustedPeer,
|
||||
t, trustedProxies("10.0.0.0/8"),
|
||||
"10.0.0.1:44444",
|
||||
func(i int) map[string]string {
|
||||
return map[string]string{
|
||||
header: fmt.Sprintf(
|
||||
@@ -520,8 +495,8 @@ func TestRateLimitKey_MalformedRightmostHopFallsBackToPeer(
|
||||
t.Parallel()
|
||||
|
||||
assertSharedBucket(
|
||||
t, trustedProxies(trustedProxyCIDR),
|
||||
trustedPeer,
|
||||
t, trustedProxies("10.0.0.0/8"),
|
||||
"10.0.0.1:44444",
|
||||
func(i int) map[string]string {
|
||||
return map[string]string{
|
||||
headerXFF: fmt.Sprintf(
|
||||
@@ -547,13 +522,13 @@ func TestRateLimitKey_ForwardedHonouredFromTrustedPeer(
|
||||
t.Parallel()
|
||||
|
||||
m := rateLimitMiddleware(t, &config.Config{
|
||||
TrustedProxies: trustedProxies(trustedProxyCIDR),
|
||||
TrustedProxies: trustedProxies("10.0.0.0/8"),
|
||||
})
|
||||
handler := m.LoginRateLimit()(okHandler())
|
||||
|
||||
const peer = trustedPeer
|
||||
const peer = "10.0.0.1:44444"
|
||||
|
||||
first := map[string]string{headerXFF: clientIPv4}
|
||||
first := map[string]string{headerXFF: "198.51.100.7"}
|
||||
|
||||
for range middleware.LoginRateLimitConst {
|
||||
postWithHeaders(handler, peer, loginPath, first)
|
||||
@@ -567,7 +542,7 @@ func TestRateLimitKey_ForwardedHonouredFromTrustedPeer(
|
||||
|
||||
w = postWithHeaders(
|
||||
handler, peer, loginPath,
|
||||
map[string]string{headerXFF: clientIPv4Alt},
|
||||
map[string]string{headerXFF: "198.51.100.8"},
|
||||
)
|
||||
assert.Equal(
|
||||
t, http.StatusOK, w.Code,
|
||||
@@ -584,7 +559,7 @@ func TestRateLimitKey_ChainWalkSkipsClientPrepended(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
assertSharedBucket(
|
||||
t, trustedProxies(trustedProxyCIDR), trustedPeer,
|
||||
t, trustedProxies("10.0.0.0/8"), "10.0.0.1:44444",
|
||||
func(i int) map[string]string {
|
||||
return map[string]string{
|
||||
headerXFF: fmt.Sprintf(
|
||||
@@ -619,7 +594,7 @@ func TestRateLimitKey_LongChainCapsWalkAndFallsBackToPeer(
|
||||
start := time.Now()
|
||||
|
||||
assertSharedBucket(
|
||||
t, trustedProxies(trustedProxyCIDR), trustedPeer,
|
||||
t, trustedProxies("10.0.0.0/8"), "10.0.0.1:44444",
|
||||
func(i int) map[string]string {
|
||||
return map[string]string{
|
||||
headerXFF: fmt.Sprintf("9.9.9.%d%s", i+1, padding),
|
||||
@@ -658,13 +633,13 @@ func TestRateLimitKey_LongChainAllocationIsBounded(t *testing.T) {
|
||||
)
|
||||
|
||||
m := rateLimitMiddleware(t, &config.Config{
|
||||
TrustedProxies: trustedProxies(trustedProxyCIDR),
|
||||
TrustedProxies: trustedProxies("10.0.0.0/8"),
|
||||
})
|
||||
|
||||
req := httptest.NewRequestWithContext(
|
||||
context.Background(), http.MethodPost, loginPath, nil,
|
||||
)
|
||||
req.RemoteAddr = trustedPeer
|
||||
req.RemoteAddr = "10.0.0.1:44444"
|
||||
req.Header.Set(
|
||||
headerXFF, "9.9.9.9"+strings.Repeat(", 10.0.0.2", hops),
|
||||
)
|
||||
@@ -860,369 +835,3 @@ func TestReceiverRateLimit_IgnoresForwardedFromUntrustedPeer(
|
||||
"not mint a fresh receiver bucket",
|
||||
)
|
||||
}
|
||||
|
||||
// clientKeyFor returns the bucket key m computes for a request whose
|
||||
// direct peer is remoteAddr and which carries no forwarded headers.
|
||||
func clientKeyFor(
|
||||
t *testing.T, m *middleware.Middleware, remoteAddr string,
|
||||
) string {
|
||||
t.Helper()
|
||||
|
||||
req := httptest.NewRequestWithContext(
|
||||
context.Background(), http.MethodPost, loginPath, nil,
|
||||
)
|
||||
req.RemoteAddr = remoteAddr
|
||||
|
||||
return middleware.ClientKeyForTest(m, req)
|
||||
}
|
||||
|
||||
// TestRateLimitKey_IPv6BucketsByPrefix pins the key function's
|
||||
// address-family behaviour. IPv6 clients must bucket by /64 — a
|
||||
// routed /64 is the normal residential and mobile allocation, so
|
||||
// per-/128 keying lets one subscriber rotate source addresses and
|
||||
// mint a fresh bucket per request — while IPv4 keeps keying on the
|
||||
// full address and IPv4-mapped form is keyed as the IPv4 address it
|
||||
// carries.
|
||||
func TestRateLimitKey_IPv6BucketsByPrefix(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
m := rateLimitMiddleware(t, &config.Config{})
|
||||
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
peer string
|
||||
want string
|
||||
about string
|
||||
}{{
|
||||
name: "ipv6",
|
||||
peer: "[" + clientIPv6 + "]:44444",
|
||||
want: clientBucketV6,
|
||||
about: "an IPv6 peer must key on its /64",
|
||||
}, {
|
||||
name: "ipv6-other-in-same-64",
|
||||
peer: "[" + clientIPv6Same + "]:1",
|
||||
want: clientBucketV6,
|
||||
about: "another address in the same /64 must key the same",
|
||||
}, {
|
||||
name: "ipv6-different-64",
|
||||
peer: "[" + clientIPv6Other + "]:44444",
|
||||
want: clientOtherBucketV6,
|
||||
about: "a different /64 must key differently",
|
||||
}, {
|
||||
name: "ipv4",
|
||||
peer: clientIPv4 + ":44444",
|
||||
want: clientIPv4,
|
||||
about: "IPv4 must keep keying on the full address",
|
||||
}, {
|
||||
name: "ipv4-neighbour",
|
||||
peer: clientIPv4Alt + ":44444",
|
||||
want: clientIPv4Alt,
|
||||
about: "adjacent IPv4 addresses must not share a bucket",
|
||||
}, {
|
||||
name: "ipv4-mapped",
|
||||
peer: "[::ffff:" + clientIPv4 + "]:44444",
|
||||
want: clientIPv4,
|
||||
about: "IPv4-mapped form must key as the IPv4 address, " +
|
||||
"not be masked to a /64: mapped addresses all share " +
|
||||
"::ffff:0:0/96, so masking would collapse every IPv4 " +
|
||||
"client behind a mapping proxy into one bucket",
|
||||
}} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
assert.Equal(
|
||||
t, tc.want, clientKeyFor(t, m, tc.peer), tc.about,
|
||||
)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestRateLimitKey_FamiliesDoNotCollide pins the structure the
|
||||
// no-collision property rests on, rather than one sample pair: every
|
||||
// IPv4 key is a bare address and every IPv6 key is a /64 in CIDR
|
||||
// form, so the two name spaces are disjoint by shape. Dropping the
|
||||
// masking strips the suffix that guarantees it, which is why this
|
||||
// asserts the form of each key and not just that two of them differ.
|
||||
func TestRateLimitKey_FamiliesDoNotCollide(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Restated here rather than imported from the package under
|
||||
// test, so that changing the production bucket width fails this
|
||||
// test instead of silently moving with it.
|
||||
const wantBits = 64
|
||||
|
||||
m := rateLimitMiddleware(t, &config.Config{})
|
||||
|
||||
v4Keys := map[string]bool{}
|
||||
|
||||
for _, peer := range []string{
|
||||
clientIPv4 + ":44444",
|
||||
clientIPv4Alt + ":44444",
|
||||
"[::ffff:" + clientIPv4 + "]:44444",
|
||||
} {
|
||||
key := clientKeyFor(t, m, peer)
|
||||
|
||||
addr, err := netip.ParseAddr(key)
|
||||
require.NoError(
|
||||
t, err, "%s: an IPv4 key must be a bare address", peer,
|
||||
)
|
||||
assert.True(
|
||||
t, addr.Is4(),
|
||||
"%s: an IPv4 key must be a dotted quad, got %q", peer, key,
|
||||
)
|
||||
|
||||
v4Keys[key] = true
|
||||
}
|
||||
|
||||
for _, peer := range []string{
|
||||
"[" + clientIPv6 + "]:44444",
|
||||
"[" + clientIPv6Same + "]:44444",
|
||||
"[" + clientIPv6Other + "]:44444",
|
||||
"[2001:db8::" + clientIPv4 + "]:44444",
|
||||
} {
|
||||
key := clientKeyFor(t, m, peer)
|
||||
|
||||
prefix, err := netip.ParsePrefix(key)
|
||||
require.NoError(
|
||||
t, err, "%s: an IPv6 key must be a CIDR prefix", peer,
|
||||
)
|
||||
assert.Equal(
|
||||
t, wantBits, prefix.Bits(),
|
||||
"%s: an IPv6 key must name a /64", peer,
|
||||
)
|
||||
assert.False(
|
||||
t, v4Keys[key],
|
||||
"%s: an IPv6 key must never equal an IPv4 key", peer,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRateLimitKey_UnparseablePeerKeepsDistinctBuckets covers the
|
||||
// fallback path. A RemoteAddr that is not an address must not panic,
|
||||
// and must not drop unrelated clients into one shared bucket by
|
||||
// accident: the raw value is the most specific identity left, so
|
||||
// distinct values stay in distinct buckets.
|
||||
func TestRateLimitKey_UnparseablePeerKeepsDistinctBuckets(
|
||||
t *testing.T,
|
||||
) {
|
||||
t.Parallel()
|
||||
|
||||
m := rateLimitMiddleware(t, &config.Config{})
|
||||
|
||||
first := clientKeyFor(t, m, "not-an-address")
|
||||
second := clientKeyFor(t, m, "also-not-an-address:1234")
|
||||
|
||||
assert.NotEmpty(t, first)
|
||||
assert.NotEqual(
|
||||
t, first, second,
|
||||
"unparseable peers must not collapse into one bucket",
|
||||
)
|
||||
}
|
||||
|
||||
// TestLoginRateLimit_IPv6SharesBucketWithinSlash64 is the behavioural
|
||||
// half, and the regression test for the bypass itself: a client that
|
||||
// rotates source addresses inside its own routed /64 must stay in one
|
||||
// bucket. Reverting the masking makes this test fail, because each
|
||||
// rotated address would mint a fresh bucket and nothing would be
|
||||
// rejected.
|
||||
func TestLoginRateLimit_IPv6SharesBucketWithinSlash64(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
m := rateLimitMiddleware(t, &config.Config{})
|
||||
handler := m.LoginRateLimit()(okHandler())
|
||||
|
||||
for i := range middleware.LoginRateLimitConst {
|
||||
w := postWithHeaders(
|
||||
handler,
|
||||
fmt.Sprintf("[2001:db8:1:2::%d]:44444", i+1),
|
||||
loginPath, nil,
|
||||
)
|
||||
assert.Equal(
|
||||
t, http.StatusOK, w.Code, "request %d should pass", i,
|
||||
)
|
||||
}
|
||||
|
||||
w := postWithHeaders(
|
||||
handler, "[2001:db8:1:2::ffff]:44444", loginPath, nil,
|
||||
)
|
||||
assert.Equal(
|
||||
t, http.StatusTooManyRequests, w.Code,
|
||||
"rotating source addresses inside one routed /64 must not "+
|
||||
"mint fresh buckets",
|
||||
)
|
||||
}
|
||||
|
||||
// TestLoginRateLimit_IPv6IndependentAcrossSlash64 is the other side
|
||||
// of the trade: bucketing by /64 must not merge separate allocations,
|
||||
// so a client in a different /64 keeps its own limit.
|
||||
func TestLoginRateLimit_IPv6IndependentAcrossSlash64(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
m := rateLimitMiddleware(t, &config.Config{})
|
||||
handler := m.LoginRateLimit()(okHandler())
|
||||
|
||||
for range middleware.LoginRateLimitConst + 1 {
|
||||
postWithHeaders(
|
||||
handler, "[2001:db8:1:2::1]:44444", loginPath, nil,
|
||||
)
|
||||
}
|
||||
|
||||
w := postWithHeaders(
|
||||
handler, "[2001:db8:1:3::1]:44444", loginPath, nil,
|
||||
)
|
||||
assert.Equal(
|
||||
t, http.StatusOK, w.Code,
|
||||
"a different /64 must have its own bucket",
|
||||
)
|
||||
}
|
||||
|
||||
// TestLoginRateLimit_IPv4IndependentPerAddress guards against the
|
||||
// masking leaking into IPv4: two addresses one apart must still hold
|
||||
// separate buckets.
|
||||
func TestLoginRateLimit_IPv4IndependentPerAddress(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
m := rateLimitMiddleware(t, &config.Config{})
|
||||
handler := m.LoginRateLimit()(okHandler())
|
||||
|
||||
for range middleware.LoginRateLimitConst + 1 {
|
||||
postWithHeaders(
|
||||
handler, clientIPv4+":44444", loginPath, nil,
|
||||
)
|
||||
}
|
||||
|
||||
w := postWithHeaders(
|
||||
handler, clientIPv4Alt+":44444", loginPath, nil,
|
||||
)
|
||||
assert.Equal(
|
||||
t, http.StatusOK, w.Code,
|
||||
"a second IPv4 address must have its own bucket",
|
||||
)
|
||||
}
|
||||
|
||||
// forwardedKeyFor returns the bucket key m computes for a request
|
||||
// that arrives from trustedPeer — a configured trusted proxy — and
|
||||
// names forwarded as its client in X-Forwarded-For. That is the
|
||||
// production path: a deployment is required to run behind a reverse
|
||||
// proxy with TRUSTED_PROXIES set, so the forwarded address, not the
|
||||
// peer, is what the limiters bucket on there.
|
||||
func forwardedKeyFor(
|
||||
t *testing.T, m *middleware.Middleware, forwarded string,
|
||||
) string {
|
||||
t.Helper()
|
||||
|
||||
req := httptest.NewRequestWithContext(
|
||||
context.Background(), http.MethodPost, loginPath, nil,
|
||||
)
|
||||
req.RemoteAddr = trustedPeer
|
||||
req.Header.Set(headerXFF, forwarded)
|
||||
|
||||
return middleware.ClientKeyForTest(m, req)
|
||||
}
|
||||
|
||||
// TestRateLimitKey_ForwardedIPv6BucketsByPrefix pins the /64
|
||||
// bucketing on the trusted-proxy branch. The direct-peer tests above
|
||||
// cannot reach it, so without this the masking could be reverted for
|
||||
// forwarded clients alone — the only shape a production deployment
|
||||
// runs in — and the rest of the suite would stay green.
|
||||
func TestRateLimitKey_ForwardedIPv6BucketsByPrefix(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
m := rateLimitMiddleware(t, &config.Config{
|
||||
TrustedProxies: trustedProxies(trustedProxyCIDR),
|
||||
})
|
||||
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
forwarded string
|
||||
want string
|
||||
about string
|
||||
}{{
|
||||
name: "ipv6",
|
||||
forwarded: clientIPv6,
|
||||
want: clientBucketV6,
|
||||
about: "a forwarded IPv6 client must key on its /64",
|
||||
}, {
|
||||
name: "ipv6-other-in-same-64",
|
||||
forwarded: clientIPv6Same,
|
||||
want: clientBucketV6,
|
||||
about: "another forwarded address in the same /64 must " +
|
||||
"key the same",
|
||||
}, {
|
||||
name: "ipv6-different-64",
|
||||
forwarded: clientIPv6Other,
|
||||
want: clientOtherBucketV6,
|
||||
about: "a forwarded address in another /64 must differ",
|
||||
}, {
|
||||
name: "ipv4",
|
||||
forwarded: clientIPv4,
|
||||
want: clientIPv4,
|
||||
about: "a forwarded IPv4 client must key on the address",
|
||||
}, {
|
||||
name: "ipv4-mapped",
|
||||
forwarded: "::ffff:" + clientIPv4,
|
||||
want: clientIPv4,
|
||||
about: "a proxy that forwards IPv4-mapped form must key as " +
|
||||
"the IPv4 address it carries, not be masked to a /64: " +
|
||||
"mapped addresses all share ::ffff:0:0/96",
|
||||
}} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
assert.Equal(
|
||||
t, tc.want,
|
||||
forwardedKeyFor(t, m, tc.forwarded), tc.about,
|
||||
)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestLoginRateLimit_ForwardedIPv6SharesBucketWithinSlash64 is the
|
||||
// behavioural half on the production path: behind a trusted proxy, a
|
||||
// client rotating source addresses inside its own routed /64 must
|
||||
// stay in one bucket.
|
||||
func TestLoginRateLimit_ForwardedIPv6SharesBucketWithinSlash64(
|
||||
t *testing.T,
|
||||
) {
|
||||
t.Parallel()
|
||||
|
||||
assertSharedBucket(
|
||||
t, trustedProxies(trustedProxyCIDR), trustedPeer,
|
||||
func(i int) map[string]string {
|
||||
return map[string]string{
|
||||
headerXFF: fmt.Sprintf("2001:db8:1:2::%d", i+1),
|
||||
}
|
||||
},
|
||||
"rotating forwarded source addresses inside one routed /64 "+
|
||||
"must not mint fresh buckets",
|
||||
)
|
||||
}
|
||||
|
||||
// TestLoginRateLimit_ForwardedIPv6IndependentAcrossSlash64 is the
|
||||
// other side of that trade on the same path: bucketing by /64 must
|
||||
// not merge two allocations reaching the proxy.
|
||||
func TestLoginRateLimit_ForwardedIPv6IndependentAcrossSlash64(
|
||||
t *testing.T,
|
||||
) {
|
||||
t.Parallel()
|
||||
|
||||
m := rateLimitMiddleware(t, &config.Config{
|
||||
TrustedProxies: trustedProxies(trustedProxyCIDR),
|
||||
})
|
||||
handler := m.LoginRateLimit()(okHandler())
|
||||
|
||||
spent := map[string]string{headerXFF: clientIPv6}
|
||||
for range middleware.LoginRateLimitConst + 1 {
|
||||
postWithHeaders(handler, trustedPeer, loginPath, spent)
|
||||
}
|
||||
|
||||
w := postWithHeaders(
|
||||
handler, trustedPeer, loginPath,
|
||||
map[string]string{headerXFF: clientIPv6Other},
|
||||
)
|
||||
assert.Equal(
|
||||
t, http.StatusOK, w.Code,
|
||||
"a forwarded client in a different /64 must have its own "+
|
||||
"bucket",
|
||||
)
|
||||
}
|
||||
|
||||
@@ -24,7 +24,6 @@ import (
|
||||
"sneak.berlin/go/webhooker/internal/middleware"
|
||||
"sneak.berlin/go/webhooker/internal/server"
|
||||
"sneak.berlin/go/webhooker/internal/session"
|
||||
"sneak.berlin/go/webhooker/static"
|
||||
)
|
||||
|
||||
// csrfCookieName is the cookie gorilla/csrf issues when it runs. Its
|
||||
@@ -247,56 +246,6 @@ func (e *testEnv) storedHash(t *testing.T, username string) string {
|
||||
return user.Password
|
||||
}
|
||||
|
||||
// --- /s static group ---
|
||||
|
||||
// TestStaticServesEveryMethod pins what the static mount actually
|
||||
// answers. chi's Mount registers the handler for all methods and
|
||||
// http.FileServer only special-cases HEAD (by suppressing the body),
|
||||
// so a POST or a DELETE to an asset is served the file rather than
|
||||
// refused. The README documents this; the test is what keeps the two
|
||||
// from drifting.
|
||||
func TestStaticServesEveryMethod(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
env := newTestEnv(t)
|
||||
|
||||
body, err := static.Static.ReadFile("js/app.js")
|
||||
require.NoError(t, err)
|
||||
require.NotEmpty(t, body)
|
||||
|
||||
for _, method := range []string{
|
||||
http.MethodGet,
|
||||
http.MethodHead,
|
||||
http.MethodPost,
|
||||
http.MethodPut,
|
||||
http.MethodDelete,
|
||||
} {
|
||||
t.Run(method, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
req := httptest.NewRequestWithContext(
|
||||
context.Background(), method,
|
||||
"/s/js/app.js", nil,
|
||||
)
|
||||
w := httptest.NewRecorder()
|
||||
env.router.ServeHTTP(w, req)
|
||||
|
||||
assert.Equal(t, http.StatusOK, w.Code,
|
||||
"static mount answers every method")
|
||||
|
||||
if method == http.MethodHead {
|
||||
assert.Empty(t, w.Body.Bytes(),
|
||||
"HEAD must not carry a body")
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
assert.Equal(t, body, w.Body.Bytes(),
|
||||
"the asset itself is returned")
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// --- /pages group ---
|
||||
|
||||
// TestPagesLogin_OversizeBody_RejectedBeforeCSRF proves the cap runs
|
||||
|
||||
@@ -24,48 +24,15 @@ import (
|
||||
)
|
||||
|
||||
const (
|
||||
// ShutdownTimeout is the maximum time to wait for the HTTP
|
||||
// shutdownTimeout is the maximum time to wait for the HTTP
|
||||
// server to finish in-flight requests during shutdown.
|
||||
//
|
||||
// It must stay strictly below the fx stop timeout in
|
||||
// cmd/webhooker, which bounds the whole stop sequence: a drain
|
||||
// that used the entire sequence budget would leave nothing for
|
||||
// the hooks that run after the server, including the database
|
||||
// close. It is exported so that relationship can be tested.
|
||||
ShutdownTimeout = 3 * time.Second
|
||||
shutdownTimeout = 5 * time.Second
|
||||
|
||||
// TailHookReserve is the share of the fx stop budget this hook
|
||||
// refuses to spend, leaving it for the hooks that run after the
|
||||
// server: the delivery engine, the healthcheck, the webhook DB
|
||||
// manager and the database close.
|
||||
TailHookReserve = 2 * time.Second
|
||||
|
||||
// sentryFlushTimeout is the longest wait for Sentry to flush
|
||||
// pending events during shutdown, before the remaining stop
|
||||
// budget is taken into account.
|
||||
// sentryFlushTimeout is the maximum time to wait for Sentry
|
||||
// to flush pending events during shutdown.
|
||||
sentryFlushTimeout = 2 * time.Second
|
||||
|
||||
// minSentryFlush is the shortest flush worth attempting. Below
|
||||
// it the remaining budget goes to the tail hooks instead.
|
||||
minSentryFlush = 250 * time.Millisecond
|
||||
)
|
||||
|
||||
// SentryFlushBudget reports how long the Sentry flush may run when
|
||||
// remaining is the time left on the fx stop context after the HTTP
|
||||
// drain. sentry.Flush takes a bare duration and honours no context,
|
||||
// so this clamp is the only thing keeping a stalled flush from
|
||||
// spending the tail hooks' share of the budget on top of a
|
||||
// full-length drain. TailHookReserve is held back, and anything
|
||||
// under minSentryFlush is skipped rather than attempted uselessly.
|
||||
func SentryFlushBudget(remaining time.Duration) time.Duration {
|
||||
budget := min(remaining-TailHookReserve, sentryFlushTimeout)
|
||||
if budget < minSentryFlush {
|
||||
return 0
|
||||
}
|
||||
|
||||
return budget
|
||||
}
|
||||
|
||||
//nolint:revive // ServerParams is a standard fx naming convention.
|
||||
type ServerParams struct {
|
||||
fx.In
|
||||
@@ -197,7 +164,7 @@ func (s *Server) cleanShutdown(ctx context.Context) {
|
||||
s.exitCode = 0
|
||||
|
||||
ctxShutdown, shutdownCancel := context.WithTimeout(
|
||||
ctx, ShutdownTimeout,
|
||||
ctx, shutdownTimeout,
|
||||
)
|
||||
defer shutdownCancel()
|
||||
|
||||
@@ -211,31 +178,10 @@ func (s *Server) cleanShutdown(ctx context.Context) {
|
||||
s.cleanupForExit()
|
||||
|
||||
if s.sentryEnabled {
|
||||
s.flushSentry(ctx)
|
||||
sentry.Flush(sentryFlushTimeout)
|
||||
}
|
||||
}
|
||||
|
||||
// flushSentry drains Sentry's queue inside what is left of the fx
|
||||
// stop budget. A context carrying no deadline — a caller outside the
|
||||
// fx lifecycle — gets the full timeout.
|
||||
func (s *Server) flushSentry(ctx context.Context) {
|
||||
flush := sentryFlushTimeout
|
||||
|
||||
if deadline, ok := ctx.Deadline(); ok {
|
||||
flush = SentryFlushBudget(time.Until(deadline))
|
||||
}
|
||||
|
||||
if flush <= 0 {
|
||||
s.log.Warn(
|
||||
"skipping sentry flush, stop budget exhausted",
|
||||
)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
sentry.Flush(flush)
|
||||
}
|
||||
|
||||
func (s *Server) configure() {
|
||||
// identify ourselves in the logs
|
||||
s.params.Logger.Identify()
|
||||
|
||||
@@ -1,59 +0,0 @@
|
||||
package server_test
|
||||
|
||||
import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
"sneak.berlin/go/webhooker/internal/server"
|
||||
)
|
||||
|
||||
// TestSentryFlushBudget covers the clamp that keeps the Sentry flush
|
||||
// from spending the tail hooks' share of the fx stop budget.
|
||||
// sentry.Flush ignores the stop context, so without the clamp a
|
||||
// stalled flush adds its whole timeout on top of the HTTP drain.
|
||||
func TestSentryFlushBudget(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
remaining time.Duration
|
||||
want time.Duration
|
||||
}{
|
||||
{
|
||||
name: "full drain leaves only the reserve",
|
||||
remaining: server.TailHookReserve,
|
||||
want: 0,
|
||||
},
|
||||
{
|
||||
name: "expired budget",
|
||||
remaining: -time.Second,
|
||||
want: 0,
|
||||
},
|
||||
{
|
||||
name: "sliver above the reserve is not worth it",
|
||||
remaining: server.TailHookReserve + 10*time.Millisecond,
|
||||
want: 0,
|
||||
},
|
||||
{
|
||||
name: "partial flush when some room is left",
|
||||
remaining: server.TailHookReserve + time.Second,
|
||||
want: time.Second,
|
||||
},
|
||||
{
|
||||
name: "capped at the nominal timeout",
|
||||
remaining: time.Hour,
|
||||
want: 2 * time.Second,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
require.Equal(
|
||||
t, tt.want, server.SentryFlushBudget(tt.remaining),
|
||||
)
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user