Compare commits

1 Commits

Author SHA1 Message Date
88f3e1d019 Mark superseded commits honestly instead of skipped (closes #152)
All checks were successful
check / check (push) Successful in 3m28s
Gitea cancels an in-flight run when a newer commit lands on the same
branch and records the cancellation as `failure` / "Has been
cancelled". The workflow rewrote that to `skipped`, but Gitea's
Combine() folds `skipped` into `success`, so the combined-status API
returned green for a commit nothing had ever tested. Rewrite it to
`failure` / "Superseded by a newer commit; never tested" instead:
red-but-honest, and never `pending`, which would block the commit
forever.

Re-running the superseded commit would have been better still, but is
not reachable on this Gitea (1.25.4): its API exposes no rerun
endpoint, workflow dispatch takes a ref rather than a SHA, and every
replay would be a full uncached build with no bound on how many pile
up behind a burst of merges.

The step also stops hardcoding its status context: the logic moves into
script/ci-mark-superseded, which derives the context from the workflow
name, job id and event, and fails loudly when no status on the commit
being built carries that context, so renaming the workflow or the job
cannot silently disable the rewrite. The derivation is not byte-exact
with Gitea's own rule -- Gitea uses the job's display `name:` where the
runner exports the job id -- so adding a `name:` to the job turns every
push red rather than quietly doing nothing; the script header says so,
because that loud failure is the point. That is item 2 of
#147; item 1 there is
untouched.

Nothing about the walk may fail quietly, since the script exists to
stop CI lying quietly. An ANCESTOR_LIMIT that is set but not a positive
integer aborts instead of passing an unusable value to git and
discarding the error; the root-commit case is detected explicitly so
every other rev-list failure aborts too; and per-ancestor status reads
carry the same `--retry 3 --max-time 30` as the head-commit read and
abort on failure rather than losing curl's exit status through a pipe.

Tests drive the script against a fake Gitea covering the cancelled,
laundered-skipped, genuinely-failed, passing and renamed cases, an
unparseable ANCESTOR_LIMIT and an ancestor whose status read answers
HTTP 500, so jq joins the builder image to run them.
2026-08-17 21:24:46 +00:00
7 changed files with 156 additions and 397 deletions

View File

@@ -13,8 +13,7 @@ jobs:
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 2024-10-23 uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 2024-10-23
with: with:
# The fingerprint step below needs history to find the last commit # The fingerprint step below needs history to find the last commit
# that touched the Docker build context, and the superseded-status # that touched the Docker build context.
# step needs it to walk ancestors (it aborts on a shallow clone).
fetch-depth: 0 fetch-depth: 0
- name: Mark superseded run statuses - name: Mark superseded run statuses

374
README.md
View File

@@ -11,14 +11,9 @@ with retry support, logging, and observability. Category: infrastructure
### Prerequisites ### Prerequisites
- Go 1.26.1+ (the version in `go.mod`) - Go 1.26+
- golangci-lint v2.12.2 (the version pinned in `script/bootstrap` and - golangci-lint v2.11+
in the `Dockerfile`'s lint stage; `make bootstrap` installs it) - Docker (for containerized deployment)
- 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)
### Quick Start ### Quick Start
@@ -27,18 +22,14 @@ with retry support, logging, and observability. Category: infrastructure
git clone https://git.eeqj.de/sneak/webhooker.git git clone https://git.eeqj.de/sneak/webhooker.git
cd webhooker cd webhooker
# Install Go dependencies, the pinned linter, and the third-party # Install Go dependencies
# browser assets. `make deps` alone is not enough: it only runs make deps
# go mod download/tidy, and the checks below need the fetched assets.
make bootstrap
# Run all checks (test, lint, format check) # Run all checks (format, lint, test, build)
make check make check
# Run in development mode. DATA_DIR defaults to /var/lib/webhooker in # Run in development mode (uses SQLite in current directory)
# every environment, so set it (in .env or the shell) to a writable make dev
# directory when running from a clone.
DATA_DIR=./data make dev
# Build Docker image # Build Docker image
make docker make docker
@@ -51,18 +42,13 @@ make bootstrap # Install all dependencies (idempotent)
make setup # Bootstrap + install git pre-commit hook make setup # Bootstrap + install git pre-commit hook
make assets # Fetch + verify third-party browser assets make assets # Fetch + verify third-party browser assets
make fmt # Format code (gofmt + goimports) make fmt # Format code (gofmt + goimports)
make fmt-check # Fail if gofmt would change anything (writes nothing)
make lint # Run golangci-lint make lint # Run golangci-lint
make test # Run tests with race detection make test # Run tests with race detection
make check # test + lint + fmt-check (CI gate) make check # test + lint + fmt-check (CI gate)
make build # Build binary to bin/webhooker make build # Build binary to bin/webhooker
make run # build, then run ./bin/webhooker
make dev # go run ./cmd/webhooker make dev # go run ./cmd/webhooker
make deps # go mod download + go mod tidy
make docker # Build Docker image make docker # Build Docker image
make hooks # Install git pre-commit hook that runs script/precommit 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 ### Configuration
@@ -104,7 +90,7 @@ TTY detection, and security headers are always applied.
| `PORT` | HTTP listen port | `8080` | | `PORT` | HTTP listen port | `8080` |
| `DATA_DIR` | Directory for all SQLite databases | `/var/lib/webhooker` | | `DATA_DIR` | Directory for all SQLite databases | `/var/lib/webhooker` |
| `DEBUG` | Enable debug logging | `false` | | `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_USERNAME` | Basic auth username for `/metrics` | `""` |
| `METRICS_PASSWORD` | Basic auth password for `/metrics` | `""` | | `METRICS_PASSWORD` | Basic auth password for `/metrics` | `""` |
| `SENTRY_DSN` | Sentry error reporting DSN | `""` | | `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 The remedy is to set `TRUSTED_PROXIES` to your reverse proxy's
address, which restores per-client buckets. webhooker logs a warning address, which restores per-client buckets. webhooker logs a warning
at startup whenever `TRUSTED_PROXIES` is empty, in every environment — at startup when `WEBHOOKER_ENVIRONMENT=prod` and `TRUSTED_PROXIES` is
not only when `WEBHOOKER_ENVIRONMENT=prod`, because that variable empty. See [Rate Limiting](#rate-limiting) for what each limit shares.
defaults to `dev` and an operator who never set it is precisely the
one at risk. The warning is informational when nothing proxies to the
process: with no proxy in front, the peer address is the client's own
and the buckets are already per-client. See
[Rate Limiting](#rate-limiting) for what each limit shares.
`X-Real-IP` and `True-Client-IP` are **never** read, from any peer. `X-Real-IP` and `True-Client-IP` are **never** read, from any peer.
Reverse proxies append to `X-Forwarded-For` but forward other client 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 makes all three limits, including the unauthenticated webhook
receiver, silently bypassable by every client in the block. receiver, silently bypassable by every client in the block.
#### Sessions
Sessions are bounded by two independent clocks, and end at whichever Sessions are bounded by two independent clocks, and end at whichever
one runs out first: one runs out first:
@@ -253,20 +232,17 @@ docker run -d \
The container runs as a non-root user (`webhooker`, UID 1000), exposes The container runs as a non-root user (`webhooker`, UID 1000), exposes
port 8080, and includes a health check against port 8080, and includes a health check against
`/.well-known/healthcheck`. The `/var/lib/webhooker` volume holds all `/.well-known/healthcheck`. The `/var/lib/webhooker` volume holds all
SQLite databases: the main application database (`webhooker.db`), the SQLite databases: the main application database (`webhooker.db`) and
per-webhook event databases (`events-{uuid}.db`), and any archive the per-webhook event databases (`events-{uuid}.db`). Mount this as a
databases written by `database` targets (`archive-{uuid}.db`). Mount persistent volume to preserve data across container restarts.
this as a persistent volume to preserve data across container
restarts.
## Entrypoints ## Entrypoints
This repository adheres to the This repository adheres to the
[Scripts to Rule Them All](https://github.com/github/scripts-to-rule-them-all) [Scripts to Rule Them All](https://github.com/github/scripts-to-rule-them-all)
standard: normalized scripts in `script/` are the entrypoints for the standard: normalized scripts in `script/` are the entrypoints for the
development workflow. Ten of the Makefile's sixteen targets are thin development workflow, and the Makefile targets are thin shims that call
shims that call them; `build`, `run`, `dev`, `deps`, `clean` and `css` them. We provide:
are inline commands with no script behind them. We provide:
- `script/bootstrap` — install all dependencies (idempotent) - `script/bootstrap` — install all dependencies (idempotent)
- `script/setup` — make a fresh clone ready for development - `script/setup` — make a fresh clone ready for development
@@ -380,11 +356,7 @@ It uses:
- **[gorilla/csrf](https://github.com/gorilla/csrf)** for CSRF - **[gorilla/csrf](https://github.com/gorilla/csrf)** for CSRF
protection (cookie-based double-submit tokens) protection (cookie-based double-submit tokens)
- **[go-chi/httprate](https://github.com/go-chi/httprate)** for - **[go-chi/httprate](https://github.com/go-chi/httprate)** for
sliding-window rate limiting of the login, password-change and per-IP login rate limiting (sliding window counter)
webhook receiver endpoints. The bucket is per client IP only when
`TRUSTED_PROXIES` names the reverse proxy; unset, every client
behind that proxy shares one bucket per limit (see
[Rate Limiting](#rate-limiting))
- **[Prometheus](https://prometheus.io)** for metrics, served at - **[Prometheus](https://prometheus.io)** for metrics, served at
`/metrics` behind basic auth `/metrics` behind basic auth
- **[Sentry](https://sentry.io)** for optional error reporting - **[Sentry](https://sentry.io)** for optional error reporting
@@ -402,7 +374,7 @@ The codebase uses consistent naming throughout (rename completed in
### Data Model ### 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 **application tier** (user and webhook configuration) and the **event
tier** (event ingestion, delivery, and logging). tier** (event ingestion, delivery, and logging).
@@ -494,25 +466,9 @@ days (`database.RetentionForeverDays`). The retention reaper recognises
that sentinel and skips the webhook entirely, and the web UI displays 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. such a webhook's retention as "forever" rather than as a day count.
Submitted `retention_days` values therefore fall into three bands, not A *finite* retention is capped at `database.MaxFiniteRetentionDays`
two: (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
- `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
`time.Duration`, an int64 nanosecond count, and a longer period `time.Duration`, an int64 nanosecond count, and a longer period
overflows it. An overflowed cutoff lands in the future, where it overflows it. An overflowed cutoff lands in the future, where it
matches every row, so the sweep would delete every event the webhook matches every row, so the sweep would delete every event the webhook
@@ -530,7 +486,7 @@ the full request and creates an Event.
| -------------- | ------- | ----------- | | -------------- | ------- | ----------- |
| `id` | UUID | Primary key | | `id` | UUID | Primary key |
| `webhook_id` | UUID | Foreign key → Webhook | | `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 | | `description` | string | Optional description |
| `active` | boolean | Whether this entrypoint accepts events (default: true) | | `active` | boolean | Whether this entrypoint accepts events (default: true) |
@@ -554,8 +510,8 @@ events should be forwarded.
| `type` | TargetType | One of: `http`, `slack`, `database`, `log` | | `type` | TargetType | One of: `http`, `slack`, `database`, `log` |
| `active` | boolean | Whether deliveries are enabled (default: true) | | `active` | boolean | Whether deliveries are enabled (default: true) |
| `config` | JSON text | Type-specific configuration | | `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_retries` | integer | Maximum retry attempts for HTTP targets (0 = fire-and-forget, >0 = retries with backoff) |
| `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_queue_size` | integer | Maximum queued deliveries (for HTTP targets with retries) |
**Relations:** Belongs to Webhook. Has many Deliveries. **Relations:** Belongs to Webhook. Has many Deliveries.
@@ -568,11 +524,6 @@ events should be forwarded.
greater than 0, failed deliveries are retried with exponential backoff greater than 0, failed deliveries are retried with exponential backoff
up to `max_retries` attempts, protected by a per-target circuit up to `max_retries` attempts, protected by a per-target circuit
breaker. 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 - **`database`** — Archive the full event as a row into a separate
per-webhook archive database (`archive-{webhookID}.db`) for long-term per-webhook archive database (`archive-{webhookID}.db`) for long-term
retention, with an optional creation-validated expiry (default: keep retention, with an optional creation-validated expiry (default: keep
@@ -609,7 +560,7 @@ data for auditing and for the planned replay capability.
| `id` | UUID | Primary key | | `id` | UUID | Primary key |
| `webhook_id` | UUID | Foreign key → Webhook | | `webhook_id` | UUID | Foreign key → Webhook |
| `entrypoint_id` | UUID | Foreign key → Entrypoint | | `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 | | `headers` | JSON | Complete request headers |
| `body` | text | Raw request body | | `body` | text | Raw request body |
| `content_type` | string | Content-Type header value | | `content_type` | string | Content-Type header value |
@@ -664,9 +615,7 @@ retries) is individually logged for full observability.
#### Common Fields #### Common Fields
Every entity except `Setting` includes these fields from `BaseModel`. All entities include these fields from `BaseModel`:
`Setting` is a bare key-value row with no `id`, no timestamps and no
soft delete:
| Field | Type | Description | | Field | Type | Description |
| ------------ | --------- | ----------- | | ------------ | --------- | ----------- |
@@ -712,7 +661,7 @@ handles connection pooling, lazy opening, migrations, and cleanup.
This separation provides: This separation provides:
- **Isolation** — a high-volume webhook won't cause lock contention or - **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 - **Independent lifecycle** — event databases can be independently
backed up, archived, rotated, or size-limited without impacting the backed up, archived, rotated, or size-limited without impacting the
application. application.
@@ -722,12 +671,9 @@ This separation provides:
- **Per-webhook retention** — the `retention_days` field on each webhook - **Per-webhook retention** — the `retention_days` field on each webhook
controls automatic cleanup of old events in that webhook's database controls automatic cleanup of old events in that webhook's database
only, or disables cleanup entirely when set to `0` (retain forever). only, or disables cleanup entirely when set to `0` (retain forever).
- **Performance** — each webhook's database has its own page cache and - **Performance** — each webhook's database has its own WAL, its own
its own lock, so concurrent event ingestion across webhooks won't page cache, and its own lock, so concurrent event ingestion across
contend. No write-ahead log is involved: both DSNs are webhooks won't contend.
`file:{path}?cache=shared&mode=rwc` and no `journal_mode` pragma is
ever issued, so every database runs on SQLite's default rollback
journal.
The **database target type** builds on this architecture to provide The **database target type** builds on this architecture to provide
long-term archiving, separate from the per-webhook event database (which long-term archiving, separate from the per-webhook event database (which
@@ -783,9 +729,8 @@ and other compatible services). Each message includes event metadata
pretty-printed in a code block. JSON payloads are automatically pretty-printed in a code block. JSON payloads are automatically
formatted with indentation for readability; non-JSON payloads are shown formatted with indentation for readability; non-JSON payloads are shown
as raw text. Large payloads are truncated to keep messages reasonable. as raw text. Large payloads are truncated to keep messages reasonable.
Config stores `webhookUrl` — the Slack/Mattermost incoming webhook Config stores `webhook_url` — the Slack/Mattermost incoming webhook
endpoint. That is the JSON key; the error text for a missing one reads endpoint.
`webhook_url is required`, which is the message, not the key.
The database uses the The database uses the
[modernc.org/sqlite](https://pkg.go.dev/modernc.org/sqlite) driver at [modernc.org/sqlite](https://pkg.go.dev/modernc.org/sqlite) driver at
@@ -807,9 +752,8 @@ External Service
1. Look up Entrypoint by UUID 1. Look up Entrypoint by UUID
2. Capture full request as Event 2. Capture full request as Event
3. Create Delivery records for each active Target 3. Create Delivery records for each active Target
4. Build self-contained delivery.Task structs 4. Build self-contained DeliveryTask structs
(target config + event data inline for (target config + event data inline for ≤16KB)
bodies < 16 KiB)
5. Notify Engine via channel (no DB read needed) 5. Notify Engine via channel (no DB read needed)
@@ -844,7 +788,7 @@ at any time, preventing goroutine explosions regardless of queue depth.
a delivery channel (new tasks from the webhook handler) and a retry a delivery channel (new tasks from the webhook handler) and a retry
channel (tasks from backoff timers). Both are buffered to 10,000. channel (tasks from backoff timers). Both are buffered to 10,000.
- **Fan-out via channel, not goroutines:** When an event arrives with - **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. Workers pick them up and process them — no goroutine-per-target.
- **Worker goroutines:** A fixed number of worker goroutines select from - **Worker goroutines:** A fixed number of worker goroutines select from
both channels. Each worker processes one task at a time, then picks up both channels. Each worker processes one task at a time, then picks up
@@ -868,12 +812,7 @@ This means:
- **Independent results** — each worker records its own delivery result - **Independent results** — each worker records its own delivery result
in the per-webhook database without coordination. in the per-webhook database without coordination.
- **Graceful shutdown** — cancel the context, workers finish their - **Graceful shutdown** — cancel the context, workers finish their
current task and exit. The stop hook waits for the pool via current task and exit. `WaitGroup.Wait()` ensures clean shutdown.
`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.
**Recovery paths:** **Recovery paths:**
@@ -901,13 +840,12 @@ remains stored in the per-webhook event database, there is no way to
redeliver it: manual redelivery is planned, not implemented (see redeliver it: manual redelivery is planned, not implemented (see
[TODO.md](TODO.md)). [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 HTTP targets with `max_retries` > 0 are protected by a **per-target circuit breaker** that
**per-target circuit breaker** that prevents hammering a down target prevents hammering a down target with repeated failed delivery attempts.
with repeated failed delivery attempts. The circuit breaker is The circuit breaker is in-memory only and resets on restart (which is
in-memory only and resets on restart (which is fine — startup recovery fine — startup recovery rescans the database anyway).
rescans the database anyway).
**States:** **States:**
@@ -943,12 +881,10 @@ rescans the database anyway).
- **Failure threshold:** 5 consecutive failures before opening - **Failure threshold:** 5 consecutive failures before opening
- **Cooldown:** 30 seconds in open state before probing - **Cooldown:** 30 seconds in open state before probing
**Scope:** Circuit breakers apply to **`http` and `slack` targets with **Scope:** Circuit breakers only apply to **HTTP targets with
`max_retries` > 0**. The Slack target is built on the same HTTP core `max_retries` > 0**. Fire-and-forget HTTP targets (`max_retries` == 0),
and hands its own `max_retries` to the same retry path, so it gets a Slack targets, database targets (local operations), and log
breaker with the same 5-failure / 30-second defaults. Fire-and-forget targets (stdout) do not use circuit breakers.
targets of either type (`max_retries` == 0), 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 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 delivery as `retrying` and schedules a retry timer for after the
@@ -964,11 +900,9 @@ unpredictable rates, and blanket limits shared with other routes would
cause legitimate deliveries to be dropped. cause legitimate deliveries to be dropped.
The receiver instead has its own dedicated abuse limit, scoped to the The receiver instead has its own dedicated abuse limit, scoped to the
`/webhook/{uuid}` route only and keyed per client IP per request path `/webhook/{uuid}` route only and keyed per client IP per entrypoint: one
(`httprate.KeyByEndpoint`): one misbehaving sender is throttled without misbehaving sender is throttled without affecting other senders of the
affecting other senders of the same entrypoint or the same sender's same entrypoint or the same sender's other entrypoints. The limit is
other entrypoints. Keying on the path rather than on the entrypoint
matters — see the aggregate limit below. The limit is
`RECEIVER_RATE_LIMIT` requests per minute (default 120, generous for `RECEIVER_RATE_LIMIT` requests per minute (default 120, generous for
legitimate webhook senders). Requests over the limit receive HTTP 429 legitimate webhook senders). Requests over the limit receive HTTP 429
with a `Retry-After` header. A set-but-invalid `RECEIVER_RATE_LIMIT` with a `Retry-After` header. A set-but-invalid `RECEIVER_RATE_LIMIT`
@@ -1026,8 +960,8 @@ opposite directions:
login bucket full, and the operator's own login returns HTTP 429 for 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 as long as that trickle continues. A restart clears the in-memory
buckets and a resumed trickle re-locks them. Production deployments buckets and a resumed trickle re-locks them. Production deployments
must set `TRUSTED_PROXIES`; webhooker warns at startup whenever it is must set `TRUSTED_PROXIES`; webhooker warns at startup when it is
empty, in any environment. empty in `prod`.
Finer-grained per-webhook rate limits (configured in the web UI and 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 enforced in the webhook handler) can layer on top of this env-level
@@ -1039,17 +973,17 @@ abuse limit later; they are tracked as future work.
| Method | Path | Description | | Method | Path | Description |
| ------ | --------------------------- | ----------- | | ------ | --------------------------- | ----------- |
| `GET` | `/` | Root redirect, 303 (authenticated → `/sources`, unauthenticated → `/pages/login`) | | `GET` | `/` | Root redirect (authenticated → `/sources`, unauthenticated → `/pages/login`) |
| `GET` | `/.well-known/healthcheck` | Health check (JSON: `status`, `now`, `uptimeSeconds`, `uptimeHuman`, `version`, `appname`, `maintenanceMode`) | | `GET` | `/.well-known/healthcheck` | Health check (JSON: status, uptime, version) |
| 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` | | `GET` | `/s/*` | Static file serving (embedded CSS, JS) |
| `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)) | | `ANY` | `/webhook/{uuid}` | Webhook receiver endpoint (accepts all methods) |
#### Authentication Endpoints #### Authentication Endpoints
| Method | Path | Description | | Method | Path | Description |
| ------ | --------------- | ----------- | | ------ | --------------- | ----------- |
| `GET` | `/pages/login` | Login page (not rate limited; the limiter applies to POST only) | | `GET` | `/pages/login` | Login page |
| `POST` | `/pages/login` | Login form submission (5 per minute per bucket, then 429) | | `POST` | `/pages/login` | Login form submission |
| `POST` | `/pages/logout` | Logout (destroys session) | | `POST` | `/pages/logout` | Logout (destroys session) |
#### Authenticated Endpoints #### Authenticated Endpoints
@@ -1057,7 +991,6 @@ abuse limit later; they are tracked as future work.
| Method | Path | Description | | Method | Path | Description |
| ------ | ------------------------ | ----------- | | ------ | ------------------------ | ----------- |
| `GET` | `/user/{username}` | User profile page | | `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` | List user's webhooks |
| `GET` | `/sources/new` | Create webhook form | | `GET` | `/sources/new` | Create webhook form |
| `POST` | `/sources/new` | Create webhook submission | | `POST` | `/sources/new` | Create webhook submission |
@@ -1067,17 +1000,13 @@ abuse limit later; they are tracked as future work.
| `POST` | `/source/{id}/delete` | Delete webhook | | `POST` | `/source/{id}/delete` | Delete webhook |
| `GET` | `/source/{id}/logs` | Webhook event logs | | `GET` | `/source/{id}/logs` | Webhook event logs |
| `POST` | `/source/{id}/entrypoints` | Add entrypoint to webhook | | `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` | 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 #### Infrastructure Endpoints
| Method | Path | Description | | 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) #### API (Planned)
@@ -1091,10 +1020,8 @@ abuse limit later; they are tracked as future work.
| `GET` | `/api/v1/webhooks/{id}/events` | List events for webhook | | `GET` | `/api/v1/webhooks/{id}/events` | List events for webhook |
| `POST` | `/api/v1/events/{id}/redeliver`| Redeliver an event | | `POST` | `/api/v1/events/{id}/redeliver`| Redeliver an event |
None of these exist yet. `/api/v1` is mounted with no routes, so every API authentication will use API keys passed via `Authorization: Bearer
path under it returns 404 today. API authentication will use API keys <key>` header.
passed via `Authorization: Bearer <key>` header; no Bearer middleware
is implemented either.
### Package Layout ### Package Layout
@@ -1122,30 +1049,16 @@ webhooker/
│ │ ├── model_delivery_result.go # DeliveryResult entity (per-webhook DB) │ │ ├── model_delivery_result.go # DeliveryResult entity (per-webhook DB)
│ │ ├── model_apikey.go # APIKey entity │ │ ├── model_apikey.go # APIKey entity
│ │ ├── password.go # Argon2id hashing and verification │ │ ├── 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 │ │ └── webhook_db_manager.go # Per-webhook DB lifecycle manager
│ ├── globals/ │ ├── globals/
│ │ └── globals.go # Build-time variables (appname, version, arch) │ │ └── globals.go # Build-time variables (appname, version, arch)
│ ├── delivery/ │ ├── delivery/
│ │ ├── engine.go # Event-driven delivery engine (channel + timer based) │ │ ├── engine.go # Event-driven delivery engine (channel + timer based)
│ │ ├── circuit_breaker.go # Per-target circuit breaker for http/slack targets with retries │ │ ├── circuit_breaker.go # Per-target circuit breaker for HTTP targets with retries
│ │ ├── target.go # Target interface, Task, Scheduler
│ │ ├── target_http.go # HTTP target (retries, circuit breaker)
│ │ ├── target_slack.go # Slack/Mattermost incoming-webhook target
│ │ ├── target_database.go # Database archive target
│ │ ├── target_database_archive.go # Archive file lifecycle and pruning
│ │ ├── target_log.go # Log target (stdout)
│ │ ├── target_config_view.go # Masked target config for templates
│ │ ├── archive_sweeper.go # Periodic pruning of idle archives
│ │ ├── url_mask.go # Strips credentials from *url.Error
│ │ └── ssrf.go # SSRF prevention (IP validation, safe HTTP transport) │ │ └── ssrf.go # SSRF prevention (IP validation, safe HTTP transport)
│ ├── lifecycle/
│ │ └── lifecycle.go # Shared fx start/stop hook helpers
│ ├── handlers/ │ ├── handlers/
│ │ ├── handlers.go # Base handler struct, JSON helpers, template rendering │ │ ├── handlers.go # Base handler struct, JSON helpers, template rendering
│ │ ├── auth.go # Login, logout handlers │ │ ├── auth.go # Login, logout handlers
│ │ ├── event_log_view.go # Event log projection, byte-capped in SQL
│ │ ├── healthcheck.go # Health check handler │ │ ├── healthcheck.go # Health check handler
│ │ ├── index.go # Index page handler │ │ ├── index.go # Index page handler
│ │ ├── profile.go # User profile handler │ │ ├── profile.go # User profile handler
@@ -1158,27 +1071,20 @@ webhooker/
│ ├── middleware/ │ ├── middleware/
│ │ ├── middleware.go # Logging, CORS, Auth, Metrics, MetricsAuth, SecurityHeaders, MaxBodySize │ │ ├── middleware.go # Logging, CORS, Auth, Metrics, MetricsAuth, SecurityHeaders, MaxBodySize
│ │ ├── csrf.go # CSRF protection middleware (gorilla/csrf) │ │ ├── csrf.go # CSRF protection middleware (gorilla/csrf)
│ │ ── ratelimit.go # Per-IP rate limiting middleware (go-chi/httprate) │ │ ── ratelimit.go # Per-IP rate limiting middleware (go-chi/httprate)
│ │ └── testing.go # NewForTest: Middleware without the fx lifecycle
│ ├── server/ │ ├── server/
│ │ ├── server.go # Server struct, fx lifecycle, signal handling │ │ ├── server.go # Server struct, fx lifecycle, signal handling
│ │ ├── http.go # HTTP server setup with timeouts │ │ ├── http.go # HTTP server setup with timeouts
│ │ └── routes.go # All route definitions │ │ └── routes.go # All route definitions
│ └── session/ │ └── session/
── session.go # Cookie-based session management ── session.go # Cookie-based session management
│ └── testing.go # NewForTest: Session without the fx lifecycle
├── static/ ├── static/
│ ├── static.go # //go:embed directive │ ├── static.go # //go:embed directive
│ ├── css/input.css # Tailwind input, source for tailwind.css (make css) │ ├── css/style.css # Custom stylesheet (system font stack, card effects, layout)
── css/tailwind.css # Generated stylesheet the pages load ── js/app.js # Client-side JavaScript (minimal bootstrap)
│ ├── css/style.css # Older hand-written stylesheet, no longer loaded ├── templates/ # Go HTML templates (base, index, login, etc.)
│ ├── js/app.js # Progressive-enhancement copy-to-clipboard ├── Dockerfile # Multi-stage: lint, build+test, then Alpine runtime
│ ├── js/alpine.min.js # Alpine.js, fetched by script/fetch-assets, not committed ├── Makefile # fmt, lint, test, check, build, docker targets
│ └── 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
├── go.mod / go.sum ├── go.mod / go.sum
└── .golangci.yml # Linter configuration └── .golangci.yml # Linter configuration
``` ```
@@ -1194,27 +1100,21 @@ Components are wired via Uber fx in this order:
user seed user seed
5. `database.NewWebhookDBManager` — Per-webhook event database 5. `database.NewWebhookDBManager` — Per-webhook event database
lifecycle manager lifecycle manager
6. `database.NewRetentionReaper` — Per-webhook event retention sweep 6. `healthcheck.New` — Health check service
7. `healthcheck.New` — Health check service 7. `session.New` — Cookie-based session manager (key from database)
8. `session.New`Cookie-based session manager (key from database) 8. `handlers.New`HTTP handlers
9. `handlers.New` — HTTP handlers 9. `middleware.New` — HTTP middleware
10. `middleware.New` — HTTP middleware 10. `delivery.New` — Event-driven delivery engine
11. `delivery.New` — Event-driven delivery engine 11. `delivery.Engine``handlers.DeliveryNotifier` — interface bridge
12. `delivery.NewArchiveSweeper` — Periodic pruning of idle archives 12. `server.New` — HTTP server and router
13. `delivery.Engine``delivery.Notifier` — interface bridge
14. `delivery.Engine``delivery.WebhookEvictor` — interface bridge so
deleting a webhook releases its archive writer
15. `server.New` — HTTP server and router
The server starts via `fx.Invoke(func(*server.Server, *delivery.Engine, The server starts via `fx.Invoke(func(*server.Server, *delivery.Engine)
*database.RetentionReaper, *delivery.ArchiveSweeper) {})`, which {})` which triggers the fx lifecycle hooks in dependency order. The
triggers the fx lifecycle hooks in dependency order. The `DeliveryNotifier` interface allows the webhook handler to send
`delivery.Notifier` interface allows the webhook handler to send self-contained `DeliveryTask` slices to the engine without a direct
self-contained `delivery.Task` slices to the engine without a direct
package dependency. Each task carries all target config and event data package dependency. Each task carries all target config and event data
inline (for bodies under 16 KiB, `delivery.MaxInlineBodySize`), so the inline (for bodies ≤16KB), so the engine can deliver without reading
engine can deliver without reading from any database — it only writes from any database — it only writes to record results.
to record results.
### Middleware Stack ### Middleware Stack
@@ -1240,32 +1140,16 @@ CSRF middleware in every one of those route groups, because
gorilla/csrf parses the form; if the cap were installed after it, form 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 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 would never apply. A request that declares a `Content-Length` over the
limit is answered with `413 Request Entity Too Large` without its body limit is answered with `413 Request Entity Too Large` before any other
being read and without reaching CSRF, the route group's remaining middleware or handler runs; a chunked request, or one that lies about
middleware, or the handler. It is not rejected before *any* other its length, is hard-capped by `http.MaxBytesReader` and fails
middleware, though: the global entries listed above all run first, so downstream at form-parse time.
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}`.
### Authentication ### Authentication
- **Web UI:** Cookie-based sessions using gorilla/sessions with - **Web UI:** Cookie-based sessions using gorilla/sessions with
encrypted cookies. Sessions are configured with HttpOnly, SameSite encrypted cookies. Sessions are configured with HttpOnly, SameSite
Lax, and Secure (in production). Absolute session lifetime is 7 days, Lax, and Secure (in production). Session lifetime is 7 days.
with a sliding idle timeout on top of it (see
[Sessions](#sessions)).
- **API (planned):** API key authentication via `Authorization: Bearer` - **API (planned):** API key authentication via `Authorization: Bearer`
header. API keys are stored per-user with usage tracking header. API keys are stored per-user with usage tracking
(`last_used_at`). (`last_used_at`).
@@ -1297,53 +1181,33 @@ rather than global: **LoginRateLimit** on `/pages/login`,
IPs before connecting, preventing DNS rebinding attacks) IPs before connecting, preventing DNS rebinding attacks)
- **Login rate limiting** via [go-chi/httprate](https://github.com/go-chi/httprate): - **Login rate limiting** via [go-chi/httprate](https://github.com/go-chi/httprate):
sliding-window rate limiter on the login endpoint, 5 POST attempts sliding-window rate limiter on the login endpoint, 5 POST attempts
per minute per bucket, to slow brute-force attacks. GET requests to per minute per bucket, to slow brute-force attacks. The bucket is per
the login page are not limited. The password-change endpoint carries client IP only when `TRUSTED_PROXIES` names the reverse proxy;
the same 5-per-minute limit. The bucket is per client IP only when unset, every client shares one bucket and the login becomes remotely
`TRUSTED_PROXIES` names the reverse proxy; unset, every client deniable (see [Rate Limiting](#rate-limiting))
shares one bucket and the login becomes remotely deniable (see
[Rate Limiting](#rate-limiting)). webhooker warns at startup
whenever `TRUSTED_PROXIES` is empty
- Prometheus metrics behind basic auth - Prometheus metrics behind basic auth
- Static assets embedded in binary (no filesystem access needed at - Static assets embedded in binary (no filesystem access needed at
runtime) runtime)
- Container runs as non-root user (UID 1000) - Container runs as non-root user (UID 1000)
- GORM soft deletes on every entity that carries `BaseModel`, which is - GORM soft deletes on all entities (data preserved for audit)
all of them but `Setting` (data preserved for audit)
### Docker ### Docker
The Dockerfile uses a three-stage build. Each stage is pinned by The Dockerfile uses a multi-stage build:
digest, and the two check stages are separate images so the linter's
version is fixed independently of the compiler's:
1. **Lint stage** (`golangci/golangci-lint:v2.12.2`, Debian-based) — 1. **Builder stage** (Debian-based `golang:1.24`) — installs
installs `make`, downloads dependencies, copies the source, and runs golangci-lint, downloads dependencies, copies source, runs `make
`make fmt-check` then `make lint`. check` (format verification, linting, tests, compilation).
2. **Builder stage** (`golang:1.26.1-bookworm`) — depends on the lint 2. **Runtime stage** (`alpine:3.21`) — copies the binary, creates the
stage passing (it copies a file from it), runs `script/fetch-assets` `/var/lib/webhooker` directory for all SQLite databases, runs as
to download and verify the third-party browser assets, then runs non-root user, exposes port 8080, includes a health check.
`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`.
Both check stages use Debian rather than Alpine because The builder uses Debian rather than Alpine because GORM's SQLite
`gorm.io/driver/sqlite` pulls in `mattn/go-sqlite3`, which needs CGO dialect pulls in CGO-dependent headers at compile time. The runtime
and does not compile against musl. Only the final binary is statically binary is statically linked and runs on Alpine.
linked, which is what lets it run on the Alpine runtime image.
`script/cibuild``docker build .` is the CI gate: the four check `docker build .` is the CI gate — if it passes, the code is formatted,
targets run inside the image, so a build that succeeds is a repo that linted, tested, and compiled.
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).
#### CI gate honesty #### CI gate honesty
@@ -1359,20 +1223,19 @@ the hash of the last commit that touched the build context, so:
`make fmt-check`, `make lint`, `make test`, and `make build`. A run `make fmt-check`, `make lint`, `make test`, and `make build`. A run
that reports success ran them. that reports success ran them.
- A docs-only commit leaves the fingerprint unchanged — `.dockerignore` - A docs-only commit leaves the fingerprint unchanged — `.dockerignore`
excludes `*.md`, `LICENSE` and `.editorconfig` from the context excludes `*.md` and `LICENSE` from the context anyway — so the image
anyway — so the image replays from cache and costs seconds. replays from cache and costs seconds.
The module download layer sits above `COPY . .` and stays cached either The module download layer sits above `COPY . .` and stays cached either
way. way.
A separate workflow step, run before the fingerprint is written, covers The workflow's first step covers a second way the gate lied: Gitea
a second way the gate lied: Gitea cancels an in-flight run when a newer cancels an in-flight run when a newer commit lands on the same branch
commit lands on the same branch and records that cancellation as a and records that cancellation as a `failure` status, so a commit nothing
`failure` status, so a commit nothing ever tested reads as a test ever tested reads as a test result. Cancellation is unconditional
result. Cancellation is unconditional server-side for push events, so server-side for push events, so the superseding run calls
the superseding run calls `script/ci-mark-superseded`, which rewrites `script/ci-mark-superseded`, which rewrites that exact status to
that exact status to `failure` / `failure` / `Superseded by a newer commit; never tested`.
`Superseded by a newer commit; never tested`.
The state stays `failure` on purpose: Gitea's combined status folds The state stays `failure` on purpose: Gitea's combined status folds
`skipped` into `success`, so marking a never-tested commit `skipped` `skipped` into `success`, so marking a never-tested commit `skipped`
@@ -1382,18 +1245,15 @@ that passed. Reading a commit's status on this repo therefore goes:
- `success` / `Successful in ...` — the checks ran and passed. - `success` / `Successful in ...` — the checks ran and passed.
- `failure` / `Failing after ...` — the checks ran and failed. - `failure` / `Failing after ...` — the checks ran and failed.
- `failure` / `Superseded by a newer commit; never tested` — the run was - `failure` / `Superseded by a newer commit; never tested` — the run was
cancelled, by a newer push or by hand, and nothing was verified about cancelled by a newer push, and nothing was verified about this commit.
this commit. Test the commit itself before concluding anything about Test the commit itself before concluding anything about it.
it.
Genuine failures and successes are never touched, and no status is left Genuine failures and successes are never touched, and no status is left
`pending`, which would block the commit indefinitely. The step derives `pending`, which would block the commit indefinitely. The step derives
its context string from the workflow name, the job **id** and the event. its context string from the workflow name, job name and event — the same
That is deliberately not byte-identical to Gitea's own rule, which uses three values Gitea builds the context from — and fails loudly when no
the job's display `name:` where the runner exports the id, so giving the status carries that context, so renaming the workflow or the job cannot
job a `name:` — or renaming the workflow — makes the derived context silently disable the rewrite.
stop matching. The step fails loudly when no status on the commit
carries that context, so no rename can silently disable the rewrite.
## TODO ## TODO

22
TODO.md
View File

@@ -25,17 +25,13 @@ password change flow (#65), policy compliance (#6), pinned lint tooling
(#55), and fail-loud configuration parsing (#80). (#55), and fail-loud configuration parsing (#80).
`next` holds the completed 1.0.0 milestone: every issue in it is closed, `next` holds the completed 1.0.0 milestone: every issue in it is closed,
and it is verified green by cache-defeated container runs and it is verified green both by CI and by cache-defeated container
(`docker build --no-cache-filter=lint --no-cache-filter=builder`). The runs. The two were only made to mean the same thing this cycle — before
CI status is not independently claimed here: a superseded run is #119, a warm layer cache let the gate report success without executing
recorded as `skipped` and still rolls up green, so a commit status on anything, and replayed the previous build's console log so the lie
`next` does not by itself evidence an executed check (#152). Before looked like a real run. Note: TODO.md was deliberately deleted from this
#119, a warm layer cache also let the gate report success without repo in f9a9569 (2026-03-01, #6); its content was folded into the README
executing anything, and replayed the previous build's console log so TODO section, which this draft reconstructs as of 2026-07-06.
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 # Next Step
@@ -194,9 +190,7 @@ rate-limit keys should bucket by `/64`).
- OpenAPI specification - OpenAPI specification
- Analytics dashboard: success rates, response times, volume - Analytics dashboard: success rates, response times, volume
- A remember-me option at login - A remember-me option at login
- Password reset flow for a forgotten password. The authenticated - Password change and reset flow
password *change* flow already landed on `main` (#65); reset does not
exist
- Later, nice to have - Later, nice to have
- email delivery target type - email delivery target type
- SNS and S3 delivery targets - SNS and S3 delivery targets

View File

@@ -422,43 +422,33 @@ func loadFromEnv() (*Config, error) {
}, nil }, nil
} }
// warnSharedRateLimitBucket logs a startup warning whenever // warnSharedRateLimitBucket logs a startup warning when a production
// TRUSTED_PROXIES is empty, in any environment. // deployment leaves TRUSTED_PROXIES empty.
// //
// With no trusted proxies every rate limiter keys on the connecting // With no trusted proxies every rate limiter keys on the connecting
// peer's address. Whether that is harmless or dangerous depends on // peer's address. A production deployment is required to run behind a
// what is in front of the process, which this code cannot observe: // TLS-terminating reverse proxy, and the peer is then that proxy for
// with nothing in front, the peer is the client and the limits are // every request, so all clients share one bucket per limiter. The
// per-client as intended; behind a reverse proxy the peer is the proxy
// for every request, so all clients share one bucket per limiter. The
// login limiter's bucket is the dangerous one: any remote client can // login limiter's bucket is the dangerous one: any remote client can
// keep it full, which denies the only administrative login to everyone // keep it full, which denies the only administrative login to
// until the process restarts. // everyone until the process restarts.
//
// The warning is deliberately not gated on WEBHOOKER_ENVIRONMENT. That
// variable defaults to dev, so gating on it would silence the warning
// for exactly the operator who forgot to configure the deployment —
// the case it exists to catch.
// //
// The default of trusting nobody is deliberate — trusting forwarded // The default of trusting nobody is deliberate — trusting forwarded
// headers from arbitrary peers lets any client choose its own bucket — // headers from arbitrary peers lets any client choose its own bucket —
// so this warns rather than failing startup or changing the key. // so this warns rather than failing startup or changing the key.
func (c *Config) warnSharedRateLimitBucket(log *slog.Logger) { func (c *Config) warnSharedRateLimitBucket(log *slog.Logger) {
if len(c.TrustedProxies) > 0 { if !c.IsProd() || len(c.TrustedProxies) > 0 {
return return
} }
log.Warn( log.Warn(
"TRUSTED_PROXIES is empty: every rate limit keys on the "+ "TRUSTED_PROXIES is empty: rate limits key on the "+
"connecting peer's address. With nothing proxying to "+ "connecting peer, so behind the reverse proxy a "+
"this process that is the client itself and the limits "+ "production deployment runs behind, every client "+
"are per-client as intended. Behind a reverse proxy the "+ "shares one bucket per limit. Any remote client can "+
"peer is the proxy on every request, so all clients "+ "then keep the login limit full and deny the admin "+
"share one bucket per limit and any remote client can "+ "login, the only administrative path, until restart. "+
"keep the login limit full, denying the admin login — "+ "Set TRUSTED_PROXIES to your reverse proxy's address.",
"the only administrative path — until restart. If "+
"anything proxies to this process, set TRUSTED_PROXIES "+
"to its address.",
"environment", c.Environment, "environment", c.Environment,
"trustedProxies", len(c.TrustedProxies), "trustedProxies", len(c.TrustedProxies),
) )
@@ -501,10 +491,6 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
"maintenanceMode", s.MaintenanceMode, "maintenanceMode", s.MaintenanceMode,
"dataDir", s.DataDir, "dataDir", s.DataDir,
"retentionSweepInterval", s.RetentionSweepInterval.String(), "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, "receiverRateLimit", s.ReceiverRateLimit,
"trustedProxies", len(s.TrustedProxies), "trustedProxies", len(s.TrustedProxies),
"hasSentryDSN", s.SentryDSN != "", "hasSentryDSN", s.SentryDSN != "",

View File

@@ -628,12 +628,10 @@ func testTrustedProxiesSuccess(
} }
// TestSharedRateLimitBucketWarning covers the startup warning that // TestSharedRateLimitBucketWarning covers the startup warning that
// tells an operator a deployment behind a reverse proxy shares one // tells an operator their production deployment shares one rate-limit
// rate-limit bucket between every client, which makes the admin login // bucket between every client, which makes the admin login remotely
// remotely deniable. It must fire whenever TRUSTED_PROXIES is empty, // deniable. It must fire when TRUSTED_PROXIES is empty in production
// in any environment: WEBHOOKER_ENVIRONMENT defaults to dev, so gating // and stay quiet otherwise.
// on it would silence the warning for exactly the operator who never
// configured the deployment. It stays quiet once proxies are named.
func TestSharedRateLimitBucketWarning(t *testing.T) { func TestSharedRateLimitBucketWarning(t *testing.T) {
tests := []struct { tests := []struct {
name string name string
@@ -653,19 +651,12 @@ func TestSharedRateLimitBucketWarning(t *testing.T) {
expectWarning: false, expectWarning: false,
}, },
{ {
// The default environment. An internet-exposed // Development is not required to run behind a
// deployment whose operator never set // reverse proxy, so the shared bucket the warning
// WEBHOOKER_ENVIRONMENT lands here and has exactly // describes is not the expected shape there.
// the exposure the warning announces. name: "dev without trusted proxies is quiet",
name: "dev without trusted proxies warns",
environment: config.EnvironmentDev, environment: config.EnvironmentDev,
expectWarning: true, expectWarning: false,
},
{
name: "dev with trusted proxies is quiet",
environment: config.EnvironmentDev,
trustedProxies: cidrPrivateV4,
expectWarning: false,
}, },
} }
@@ -706,14 +697,8 @@ func TestSharedRateLimitBucketWarning(t *testing.T) {
assert.Contains(t, logged, `"level":"WARN"`) assert.Contains(t, logged, `"level":"WARN"`)
assert.Contains(t, logged, "TRUSTED_PROXIES") assert.Contains(t, logged, "TRUSTED_PROXIES")
assert.Contains(t, logged, "share one bucket") assert.Contains(t, logged, "shares one bucket")
assert.Contains(t, logged, "denying the admin login") assert.Contains(t, logged, "deny 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",
)
}) })
} }
} }

View File

@@ -24,7 +24,6 @@ import (
"sneak.berlin/go/webhooker/internal/middleware" "sneak.berlin/go/webhooker/internal/middleware"
"sneak.berlin/go/webhooker/internal/server" "sneak.berlin/go/webhooker/internal/server"
"sneak.berlin/go/webhooker/internal/session" "sneak.berlin/go/webhooker/internal/session"
"sneak.berlin/go/webhooker/static"
) )
// csrfCookieName is the cookie gorilla/csrf issues when it runs. Its // 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 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 --- // --- /pages group ---
// TestPagesLogin_OversizeBody_RejectedBeforeCSRF proves the cap runs // TestPagesLogin_OversizeBody_RejectedBeforeCSRF proves the cap runs

View File

@@ -43,10 +43,7 @@ context() {
# git would print `fatal: not an integer` into a discarded exit status # git would print `fatal: not an integer` into a discarded exit status
# and mark nothing. # and mark nothing.
ancestor_limit() { ancestor_limit() {
# `-` and not `:-`: an explicitly empty value is set-but-unusable _limit="${ANCESTOR_LIMIT:-20}"
# config, so it aborts like any other bad value rather than silently
# running at the default.
_limit="${ANCESTOR_LIMIT-20}"
case "$_limit" in case "$_limit" in
'' | *[!0-9]* | 0*) '' | *[!0-9]* | 0*)
echo "ANCESTOR_LIMIT must be a positive integer," \ echo "ANCESTOR_LIMIT must be a positive integer," \
@@ -112,20 +109,9 @@ main() {
require_own_context "$_api" "$_ctx" require_own_context "$_api" "$_ctx"
# A shallow clone cannot resolve the parent, so it looks exactly like # A root commit legitimately has no ancestors; every other rev-list
# a root commit to rev-parse below and would exit 0 having walked # failure (a shallow clone, an unknown SHA) must abort, so the walk
# nothing (or, at depth > 1, only the ancestors that happen to be # itself carries no `|| true`.
# present). The workflow checks out with `fetch-depth: 0`; verify
# that here rather than depend on it silently.
if [ "$(git rev-parse --is-shallow-repository)" = 'true' ]; then
echo "shallow repository: the ancestor walk needs full history" >&2
return 1
fi
# A root commit legitimately has no ancestors and is not an error;
# every other rev-list failure (an unknown SHA) must abort, so the
# walk itself carries no `|| true`.
if ! git rev-parse -q --verify "${GITHUB_SHA}^" >/dev/null; then if ! git rev-parse -q --verify "${GITHUB_SHA}^" >/dev/null; then
echo "no ancestor of ${GITHUB_SHA} to check" echo "no ancestor of ${GITHUB_SHA} to check"