Every content-bearing stage executed, confirmed per vertex rather than inferred:
stage
result
[lint 7/9] make fmt-check
DONE 4.9s
[lint 8/9] golangci-lint config verify
DONE 0.2s
[lint 9/9] golangci-lint run
DONE 52.1s, 0 issues.
[builder 8/11] script/fetch-assets
DONE 0.4s, hashes verified
[builder 9/11] make test
DONE 70.3s
[builder 10/11] make build
DONE 42.9s
[builder 11/11] static relink
DONE 3.3s
Five vertices were CACHED, not zero:#8 the digest-pinned golangci/golangci-lint:v2.12.2FROM resolve, #7 the digest-pinned golang:1.26.1-bookwormFROM resolve, and #28/#29/#30 the Alpine stage-2 runtime layers (apk add ca-certificates, adduser, WORKDIR). None carries a dependency on repository content, so none can mask a stale result.
One limit on that log: buildkit clipped the make test vertex at its 2 MiB output cap, so the per-package tally is not readable from it. script/test is go test -v -race -timeout 90s ./... under set -eu, so a failure would have failed the build — but the tally below comes from a separate run, not from that log.
Separate uncapped GOFLAGS=-count=1 make check on the same clone at the same commit: exit 0, 23/23 packages (21 ok + 2 no-test), 0 FAIL, 0 (cached), 0 data races.
Binary from the built image: sha256:1f81b6749a7d5ba102d6e5ecfd2a7b9f9025ef9ad72578ea453a20f01fca7339, 32217760 bytes.
A fresh clone requires make bootstrap before make check — #282.
No commit ahead of main carries an attribution trailer.
Landed since the last revision of this body
#257 — the event's receipt time now reaches every delivery. Every Slack/Mattermost message rendered *Timestamp:* 0001-01-01T00:00:00Z, because no delivery task carried created_at and nothing read it back. Verified live against a running instance at this head, not from tests: event POSTed at 15:44:00.27-15:44:00.29Z, sink received *Timestamp:* `2026-08-24T15:44:00Z`.
#298 — the reaped-row fallback no longer renders that same zero string. When retention hard-deletes an event row while a delivery still holds its inlined body, the delivery correctly goes out anyway with CreatedAt unset; it now renders `unknown` rather than a year-1 date, so an expected reap cannot be misread as the defect above returning.
#107 — the two remaining delivery terminal-state gaps.
#283 — an unparseable SENTRY_DSN or a malformed .env aborts startup instead of silently defaulting.
#256 — the durability defect that held the tag. WAL, a busy timeout, _txlock=immediate, no cache=shared, a bounded pool, and delivery re-dispatch gated by reference-counted ownership. An operator can now run sqlite3 .dump against a live database; that previously rejected 41 of 60 inbound webhooks and re-sent already-delivered events on restart. Verified across 1200 events / 7200 deliveries: 7200 POSTs, zero duplicates, zero new POSTs after restart.
Inbound signing removed — the UUID URL is the authentication secret.
Live entrypoint UUIDs in /metrics and Sentry are acceptable. No masking work done.
Pre-1.0, no installed base. No migrations, compatibility shims or upgrade tests required of any unit.
For you — none of it blocking the merge
next is not linear.9313b0f is a merge commit; the fast-forward is unaffected, but do not expect a clean rebase if you want linear history.
Static-CGO DNS. The static relink emits a glibc dlopen/getaddrinfo warning — your deliberate Debian-builder/Alpine-runtime design. Delivery by DNS name worked end to end in both audits.
#150 was decided by me, not you, rather than stalling the queue. Reverse it if that call was wrong.
#193 — NOT reproducible from outside; no panic path is externally reachable. The mechanism was shown on a genuine non-panic 500, so the question stands but the panic-specific claim is unverified.
#212 — narrowed and deferred. Encryption at rest is the wrong control for an unattended single-host process, since the key must live where the data lives. The real exposure was the file mode, fixed by #255.
#198 — 22.9s on an unloaded host, inside the 60s cap. The 90s figure came from a host at load 122-170.
#265 — script/docker diverges from the byte-identical model script to pass a version build arg, and no green CI run exercises a real stamp. Durable fix is upstream in sneak/prompts.
#215 — make fmt does not format markdown here; markdown is hand-wrapped.
#280 — archive writers are not closed at shutdown, so an archive database is a three-file artifact. No data loss under the documented cp -a procedure, but moving archive-*.db alone silently yields fewer rows.
#299 — startup recovery races the receiver and can deliver a fresh event twice. Filed, unmilestoned, consistent with the at-least-once contract the README already documents.
**Taggable.** Milestone 1.0.0 (https://git.eeqj.de/sneak/webhooker/milestone/9) is 0 open. `next` @ `d61d9dc`, 83 commits ahead of `main`, strict fast-forward (`merge-base` == `main` head).
## Verification at `d61d9dc`
Cache-defeated container build per https://git.eeqj.de/sneak/webhooker/issues/119, from a fresh clone:
`docker build --no-cache-filter=lint --no-cache-filter=builder --progress=plain .` — **exit 0.**
Every content-bearing stage executed, confirmed per vertex rather than inferred:
| stage | result |
| --- | --- |
| `[lint 7/9] make fmt-check` | DONE 4.9s |
| `[lint 8/9] golangci-lint config verify` | DONE 0.2s |
| `[lint 9/9] golangci-lint run` | DONE 52.1s, `0 issues.` |
| `[builder 8/11] script/fetch-assets` | DONE 0.4s, hashes verified |
| `[builder 9/11] make test` | DONE 70.3s |
| `[builder 10/11] make build` | DONE 42.9s |
| `[builder 11/11]` static relink | DONE 3.3s |
**Five vertices were CACHED, not zero:** `#8` the digest-pinned `golangci/golangci-lint:v2.12.2` `FROM` resolve, `#7` the digest-pinned `golang:1.26.1-bookworm` `FROM` resolve, and `#28`/`#29`/`#30` the Alpine `stage-2` runtime layers (`apk add ca-certificates`, `adduser`, `WORKDIR`). None carries a dependency on repository content, so none can mask a stale result.
One limit on that log: buildkit clipped the `make test` vertex at its 2 MiB output cap, so the per-package tally is not readable from it. `script/test` is `go test -v -race -timeout 90s ./...` under `set -eu`, so a failure would have failed the build — but the tally below comes from a separate run, not from that log.
Separate uncapped `GOFLAGS=-count=1 make check` on the same clone at the same commit: **exit 0, 23/23 packages (21 `ok` + 2 no-test), 0 `FAIL`, 0 `(cached)`, 0 data races.**
Binary from the built image: `sha256:1f81b6749a7d5ba102d6e5ecfd2a7b9f9025ef9ad72578ea453a20f01fca7339`, 32217760 bytes.
A fresh clone requires `make bootstrap` before `make check` — https://git.eeqj.de/sneak/webhooker/issues/282.
No commit ahead of `main` carries an attribution trailer.
## Landed since the last revision of this body
- https://git.eeqj.de/sneak/webhooker/issues/257 — **the event's receipt time now reaches every delivery.** Every Slack/Mattermost message rendered `*Timestamp:* 0001-01-01T00:00:00Z`, because no delivery task carried `created_at` and nothing read it back. Verified live against a running instance at this head, not from tests: event POSTed at `15:44:00.27-15:44:00.29Z`, sink received `` *Timestamp:* `2026-08-24T15:44:00Z` ``.
- https://git.eeqj.de/sneak/webhooker/issues/298 — **the reaped-row fallback no longer renders that same zero string.** When retention hard-deletes an event row while a delivery still holds its inlined body, the delivery correctly goes out anyway with `CreatedAt` unset; it now renders `` `unknown` `` rather than a year-1 date, so an expected reap cannot be misread as the defect above returning.
- https://git.eeqj.de/sneak/webhooker/issues/107 — the two remaining delivery terminal-state gaps.
- https://git.eeqj.de/sneak/webhooker/issues/255 — every SQLite file is created `0600`.
- https://git.eeqj.de/sneak/webhooker/issues/283 — an unparseable `SENTRY_DSN` or a malformed `.env` aborts startup instead of silently defaulting.
- https://git.eeqj.de/sneak/webhooker/issues/256 — **the durability defect that held the tag.** WAL, a busy timeout, `_txlock=immediate`, no `cache=shared`, a bounded pool, and delivery re-dispatch gated by reference-counted ownership. An operator can now run `sqlite3 .dump` against a live database; that previously rejected 41 of 60 inbound webhooks and re-sent already-delivered events on restart. Verified across 1200 events / 7200 deliveries: 7200 POSTs, zero duplicates, zero new POSTs after restart.
- https://git.eeqj.de/sneak/webhooker/issues/272, https://git.eeqj.de/sneak/webhooker/issues/268 + https://git.eeqj.de/sneak/webhooker/issues/226, https://git.eeqj.de/sneak/webhooker/issues/279, https://git.eeqj.de/sneak/webhooker/issues/253, https://git.eeqj.de/sneak/webhooker/issues/250, https://git.eeqj.de/sneak/webhooker/issues/254 + https://git.eeqj.de/sneak/webhooker/issues/261, https://git.eeqj.de/sneak/webhooker/issues/221, https://git.eeqj.de/sneak/webhooker/issues/211, https://git.eeqj.de/sneak/webhooker/issues/262, https://git.eeqj.de/sneak/webhooker/issues/269.
## Your scope decisions, recorded
- **Inbound signing removed** — the UUID URL is the authentication secret.
- **Live entrypoint UUIDs in `/metrics` and Sentry are acceptable.** No masking work done.
- **Pre-1.0, no installed base.** No migrations, compatibility shims or upgrade tests required of any unit.
## For you — none of it blocking the merge
- **`next` is not linear.** `9313b0f` is a merge commit; the fast-forward is unaffected, but do not expect a clean rebase if you want linear history.
- **Static-CGO DNS.** The static relink emits a glibc `dlopen`/`getaddrinfo` warning — your deliberate Debian-builder/Alpine-runtime design. Delivery by DNS name worked end to end in both audits.
- **https://git.eeqj.de/sneak/webhooker/issues/150 was decided by me**, not you, rather than stalling the queue. Reverse it if that call was wrong.
- **https://git.eeqj.de/sneak/webhooker/issues/193** — NOT reproducible from outside; no panic path is externally reachable. The mechanism was shown on a genuine non-panic 500, so the question stands but the panic-specific claim is unverified.
- **https://git.eeqj.de/sneak/webhooker/issues/212** — narrowed and deferred. Encryption at rest is the wrong control for an unattended single-host process, since the key must live where the data lives. The real exposure was the file mode, fixed by https://git.eeqj.de/sneak/webhooker/issues/255.
- **https://git.eeqj.de/sneak/webhooker/issues/198** — 22.9s on an unloaded host, inside the 60s cap. The 90s figure came from a host at load 122-170.
- **https://git.eeqj.de/sneak/webhooker/issues/265** — `script/docker` diverges from the byte-identical model script to pass a version build arg, and no green CI run exercises a real stamp. Durable fix is upstream in `sneak/prompts`.
- **https://git.eeqj.de/sneak/webhooker/issues/215** — `make fmt` does not format markdown here; markdown is hand-wrapped.
- **https://git.eeqj.de/sneak/webhooker/issues/280** — archive writers are not closed at shutdown, so an archive database is a three-file artifact. No data loss under the documented `cp -a` procedure, but moving `archive-*.db` alone silently yields fewer rows.
- **https://git.eeqj.de/sneak/webhooker/issues/299** — startup recovery races the receiver and can deliver a fresh event twice. Filed, unmilestoned, consistent with the at-least-once contract the README already documents.
The delivery engine worker pool and the retention reaper both rooted their
goroutines in the fx OnStart hook context, which fx cancels 15s into startup.
Both now use context.WithCancel(context.Background()), bounded by OnStop.
clawbot
self-assigned this 2026-08-10 15:45:42 +02:00
Per-webhook archive writers are now evicted when the webhook or its last
database target is deleted, and a background sweeper prunes expired rows from
idle archives that no longer receive writes. Archive files themselves are never
deleted.
A delivery left in `retrying` whose target type was edited to a fire-and-forget
or unknown type was skipped forever by both restart recovery and the retry
sweep. Both paths now record a result row and mark it `failed`.
Defaults now apply only to unset or empty environment variables; a set-but-
unparseable value aborts startup with an error naming the key and the value.
envInt is gone, envBool parses with strconv.ParseBool, and PORT is bounded.
Sessions now carry a server-enforced idle deadline (SESSION_IDLE_TIMEOUT,
default 24h) alongside the 7-day absolute cap, refreshed on authenticated
activity. Activity never extends the absolute cap.
Rewrites retention_days=0 to the RetentionForeverDays sentinel (365 * 1000)
in Webhook.BeforeSave, so the GORM column default cannot win the race. The
reaper skips retain-forever webhooks before building any query.
Also bounds the reaper's cutoff arithmetic: a time.Duration is int64
nanoseconds, so day counts above MaxFiniteRetentionDays (106751) overflowed
and wrapped the cutoff into the future, where created_at < cutoff matched
every row and the sweep deleted everything. parseRetentionDays now rejects
finite values above the ceiling, and retentionCutoff saturates so rows
written by older versions cannot reach it either.
Views render RetentionLabel() rather than the raw sentinel.
The page rendered the stored target config verbatim, exposing the Slack
incoming-webhook URL, which is a bearer credential: anyone holding it can
post to the channel indefinitely, and it cannot be scoped or revoked
per-holder.
Target config now reaches the template only as a TargetView carrying
labelled fields, so no code path can render the raw blob. maskURL keeps
scheme and host and elides the path, and drops query, fragment and
userinfo; every parse failure yields a neutral placeholder rather than
falling back to the stored string. HTTP header values are never rendered,
only a count.
Rendering change only: the stored config format and the delivery path are
unchanged.
CSRF ran before MaxBodySize, so the CSRF middleware parsed the form body
before any cap applied and an oversized request was read in full before
being rejected. MaxBodySize is now the first middleware in all four route
groups that parse forms, ahead of CSRF and RequireAuth.
An oversize request therefore gets 413 without the handler running and
without state changing, including the password-change route.
Note the ordering trade: an unauthenticated client now receives 413 rather
than an auth redirect on /user/{username}/password.
The receiver was the one unauthenticated, internet-facing endpoint with no
rate limit, so a misbehaving or hostile sender could flood a webhook
without bound. RECEIVER_RATE_LIMIT (default 120/min) now caps it, keyed on
client IP plus entrypoint path so one entrypoint cannot exhaust another's
budget. Over-limit requests get 429 with Retry-After.
The limiter deliberately does not reuse postRateLimit: that helper is
POST-only and keys on IP alone, whereas the receiver must count every
method. A test locks that property in.
Config parsing follows the fail-loudly idiom: a set-but-unparseable or
non-positive value aborts startup rather than falling back to the default.
Known limitation, tracked in #88: the key still trusts forwarded headers
unconditionally, so the limit is evadable by rotating X-Forwarded-For until
trusted-proxy gating lands.
Go embeds the request URL in *url.Error, so any transport failure — DNS,
TLS, refused, timeout, SSRF dial block — persisted the full Slack webhook
URL into the per-webhook SQLite database via DeliveryResult.Error. That
field is tagged json:"error,omitempty", so a future REST API would have
served it.
maskURLError rebuilds the error preserving Op and the wrapped cause, so DNS
vs TLS vs timeout still read differently and errors.Is/As and Timeout()
keep working; only path, query and userinfo are dropped. Applied where the
errors are born, which covers both the Slack and HTTP targets. url.Parse
embeds the URL too, so ValidateTargetURL's parse branch gets the same
treatment.
The SSRF rejection log now logs the masked URL, and source_logs.html
receives view types rather than raw rows, so no config blob is reachable
from that template.
MaskURL is now the single masker for the whole tree.
Unifies user-visible copy on "Webhook" (routes and URLs unchanged), drops
the placeholder Profile settings section, and adds a copy-to-clipboard
affordance for the entrypoint URL as progressive enhancement — the button
stays hidden unless both the target element and the Clipboard API resolve,
so no dead control appears without JavaScript and the URL stays selectable.
Retention copy now matches what the code does: deletion is permanent, 0
retains forever, and a blank field means the default on create or the
current value on edit. The permanent-deletion sentence is suppressed for a
retain-forever webhook, which the reaper exempts before computing a cutoff.
Template tests gained a render-completed assertion. Without it, a page that
aborted mid-render still satisfied assertions matching the already-flushed
prefix, because renderTemplate streams to the ResponseWriter (#123).
All three rate limiters (receiver, login, password change) now key on the connection's own address unless the direct peer is inside the new TRUSTED_PROXIES CIDR list, in which case X-Forwarded-For is walked right to left for the first non-proxy hop. Default is the empty list, which trusts nothing. A set-but-unparseable value aborts startup.
The walk now keeps only the rightmost 64 hops, so an attacker-supplied chain cannot burn unbounded CPU in the rate-limit key function. Running off the end of the truncated slice falls back to the peer address, the same fail-closed direction the rest of the function takes. Also corrects the unparseable-RemoteAddr comment, which overclaimed about Unix-socket peers.
forwardedClientAddr now walks the header values in reverse with strings.LastIndexByte instead of joining and splitting, so allocation is bounded by the 64-hop cap rather than by header length: 1.6 MB per call becomes 16 bytes for a 1 MB chain. Semantics are unchanged, verified by differential testing against the previous implementation.
Records the trusted-proxy gating, hop cap and bounded scan, and corrects the Workflow section, which still described branching from main and committing TODO.md alongside the work.
Integration-level review of origin/main..origin/next @ 543005c. FAIL — needs-rework.
Gate is genuinely green: docker build --no-cache-filter=lint,builder exit 0 with #15 RUN make fmt-check DONE 1.1s, #16 RUN make lint -> 0 issues. DONE 59.3s, #23 RUN make test DONE 66.1s (5.8k lines, zero (cached) markers, 9 ok package lines); host make check exit 0. main is an ancestor of next — fast-forwardable, no conflict. The Gitea status on 543005c says "Successful in 5s", i.e. a cache replay per #119 — the runs above are the evidence, not the badge. No Claude/Anthropic strings or attribution trailers in the tree or in any of the 15 commit messages; inclusive terminology clean.
Findings, ranked.
1. The receiver rate limit does not bound abuse of the receiver — internal/middleware/ratelimit.go:258 + internal/server/routes.go:174.
The limiter keys on (client IP, httprate.KeyByEndpoint), and KeyByEndpoint is literally r.URL.Path. The chi pattern /webhook/{uuid} matches any single path segment, so a client mints a fresh 120/min bucket for every distinct string it invents — /webhook/a, /webhook/b, ... — and the aggregate rate against the route is unlimited. Every one of those requests is admitted into HandleWebhook, which logs an INFO line carrying the attacker-supplied value (internal/handlers/webhook.go:42) and performs a DB lookup (internal/handlers/webhook.go:48) before returning 404. One IP therefore drives unbounded DB queries and unbounded log volume against the single unauthenticated, internet-exposed endpoint. That is precisely the property internal/config/config.go:38-41 claims ("bounding abuse of the one unauthenticated, internet-exposed endpoint") and that #64 was opened to obtain. Neither unit review could see this: #64 landed the per-endpoint key against a next without the trusted-proxy work, and #88 rewrote only the IP half of the same key. Acceptable: a second, coarser limiter over the whole /webhook/* group keyed on client IP alone at a higher ceiling, or resolve the entrypoint first and bucket on the resolved entrypoint ID rather than the raw path — so an unresolvable path cannot buy a bucket.
2. RETENTION_SWEEP_INTERVAL is not range-validated and now panics two loops — internal/config/config.go:219,349. envDuration accepts any parseable duration, including 0s and -1h. That value reaches time.NewTicker in internal/database/retention.go:122 and — new in this diff — internal/delivery/archive_sweeper.go:130. time.NewTicker panics on a non-positive duration, in goroutines with no recover, so RETENTION_SWEEP_INTERVAL=0 logs "Configuration loaded" and then aborts the process: a delayed crash, not a loud startup failure. PORT and RECEIVER_RATE_LIMIT both received lower-bound checks under #80; this variable did not, and #89 doubled its blast radius. The missing check predates this branch, but the second consumer does not. Acceptable: reject non-positive RETENTION_SWEEP_INTERVAL in loadFromEnv, as envPositiveInt does.
3. RETENTION_SWEEP_INTERVAL is missing from the README environment table — README.md:86-98.
The table gained MAINTENANCE_MODE, SESSION_IDLE_TIMEOUT, RECEIVER_RATE_LIMIT and TRUSTED_PROXIES, but two later sections use RETENTION_SWEEP_INTERVAL as if it were documented there (README.md:174 cites it as a fail-loud example; README.md:642 makes it the archive sweeper's interval). An operator reading the table cannot discover the variable that controls both sweepers.
4. TODO.md does not record five of the fifteen landed units.
The landing commit is titled "Update TODO.md for the completed 1.0.0 milestone" and its Status paragraph asserts next "holds the completed 1.0.0 milestone", but Completed Steps has no entry for #64, #79, #90, #113 or #118 — including both credential-exposure fixes. REPO_POLICIES.md requires the TODO be updated meticulously.
5. TODO.md:118 dropped "Manual event redelivery from the web UI" from Future Steps, but it is not implemented.
No redelivery handler, route or template exists anywhere in internal/ or templates/. The only surviving trace is the planned API endpoint (TODO.md:128, README.md:919). Meanwhile README.md:263 still advertises it in the present tense as a core capability: "Replay — Stored events can be manually redelivered for debugging or testing, without requiring the original sender to fire the webhook again." Tagging 1.0.0 would ship a README promising a feature the binary lacks, with the tracking item deleted in the same branch. Acceptable: restore the Future Steps entry (or file an issue) and soften README.md:263 to planned.
6. Minor — static/js/app.js:2: console.log("Webhooker loaded"); ships in the production asset. Pre-existing, but it is debug scaffolding in a 1.0.0 artifact.
7. Minor — templates/sources_list.html:3: the #57 copy change to {{define "title"}}Webhooks - Webhooker{{end}} is inert, because page title blocks never render (#117). Harmless, but the commit does not achieve what it claims for that file.
8. PR state. Still a draft with WIP: in the title, and the body's "Still open in the milestone" list names #57, #88 and #118, all of which landed; the milestone is now 0 open. Body and draft state need correcting before this can merge.
Probes worth recording, all of which passed for the right reason:
The codec/idle-timeout interaction is safe. securecookie's max-age on next is still gorilla's 30-day default while store.Options.MaxAge is 7 days (#108), but session.expired now rejects anything past created_at + 7d server-side and treats a session carrying no timestamps as expired, so the skew is not reachable. #132 remains worth landing on its own merits.
Consequence of that: every session issued before this branch is invalidated on upgrade — a forced re-login. Correct for a first release, and not stated in the README; worth one line there if any pre-1.0 deployment exists.
forwardedClientAddr hand-checked for hop splitting, empty hops, trailing commas, multi-value headers and the 64-hop cutoff falling back to the peer address: correct in each case.
The delete-vs-sweep race on archive writers is closed: sweepWriterFor / releaseSweepWriter / the evicted flag are all evaluated under the registry lock, and the sweep never creates an archive file.
maxBodyShift remains live via internal/handlers/webhook.go:17, so removing the handler-level MaxBytesReader calls did not disturb the receiver's own 1 MB cap.
Disclosure: 14 of 15 commits end with (closes #N); Update TODO.md for the completed 1.0.0 milestone closes no issue, which I judged acceptable for a housekeeping commit rather than a finding. The buffered-rendering interaction could not be assessed on this branch — renderTemplate still writes straight to the ResponseWriter (internal/handlers/handlers.go:233); #131 has not landed here.
Integration-level review of `origin/main..origin/next` @ 543005c. **FAIL — `needs-rework`.**
Gate is genuinely green: `docker build --no-cache-filter=lint,builder` exit 0 with `#15 RUN make fmt-check` DONE 1.1s, `#16 RUN make lint` -> `0 issues.` DONE 59.3s, `#23 RUN make test` DONE 66.1s (5.8k lines, zero `(cached)` markers, 9 `ok` package lines); host `make check` exit 0. `main` is an ancestor of `next` — fast-forwardable, no conflict. The Gitea status on 543005c says "Successful in 5s", i.e. a cache replay per https://git.eeqj.de/sneak/webhooker/issues/119 — the runs above are the evidence, not the badge. No Claude/Anthropic strings or attribution trailers in the tree or in any of the 15 commit messages; inclusive terminology clean.
Findings, ranked.
**1. The receiver rate limit does not bound abuse of the receiver — `internal/middleware/ratelimit.go:258` + `internal/server/routes.go:174`.**
The limiter keys on (client IP, `httprate.KeyByEndpoint`), and `KeyByEndpoint` is literally `r.URL.Path`. The chi pattern `/webhook/{uuid}` matches any single path segment, so a client mints a fresh 120/min bucket for every distinct string it invents — `/webhook/a`, `/webhook/b`, ... — and the aggregate rate against the route is unlimited. Every one of those requests is admitted into `HandleWebhook`, which logs an INFO line carrying the attacker-supplied value (`internal/handlers/webhook.go:42`) and performs a DB lookup (`internal/handlers/webhook.go:48`) before returning 404. One IP therefore drives unbounded DB queries and unbounded log volume against the single unauthenticated, internet-exposed endpoint. That is precisely the property `internal/config/config.go:38-41` claims ("bounding abuse of the one unauthenticated, internet-exposed endpoint") and that https://git.eeqj.de/sneak/webhooker/issues/64 was opened to obtain. Neither unit review could see this: https://git.eeqj.de/sneak/webhooker/issues/64 landed the per-endpoint key against a `next` without the trusted-proxy work, and https://git.eeqj.de/sneak/webhooker/issues/88 rewrote only the IP half of the same key. Acceptable: a second, coarser limiter over the whole `/webhook/*` group keyed on client IP alone at a higher ceiling, or resolve the entrypoint first and bucket on the resolved entrypoint ID rather than the raw path — so an unresolvable path cannot buy a bucket.
**2. `RETENTION_SWEEP_INTERVAL` is not range-validated and now panics two loops — `internal/config/config.go:219,349`.**
`envDuration` accepts any parseable duration, including `0s` and `-1h`. That value reaches `time.NewTicker` in `internal/database/retention.go:122` and — new in this diff — `internal/delivery/archive_sweeper.go:130`. `time.NewTicker` panics on a non-positive duration, in goroutines with no `recover`, so `RETENTION_SWEEP_INTERVAL=0` logs "Configuration loaded" and then aborts the process: a delayed crash, not a loud startup failure. `PORT` and `RECEIVER_RATE_LIMIT` both received lower-bound checks under https://git.eeqj.de/sneak/webhooker/issues/80; this variable did not, and https://git.eeqj.de/sneak/webhooker/issues/89 doubled its blast radius. The missing check predates this branch, but the second consumer does not. Acceptable: reject non-positive `RETENTION_SWEEP_INTERVAL` in `loadFromEnv`, as `envPositiveInt` does.
**3. `RETENTION_SWEEP_INTERVAL` is missing from the README environment table — `README.md:86-98`.**
The table gained `MAINTENANCE_MODE`, `SESSION_IDLE_TIMEOUT`, `RECEIVER_RATE_LIMIT` and `TRUSTED_PROXIES`, but two later sections use `RETENTION_SWEEP_INTERVAL` as if it were documented there (`README.md:174` cites it as a fail-loud example; `README.md:642` makes it the archive sweeper's interval). An operator reading the table cannot discover the variable that controls both sweepers.
**4. `TODO.md` does not record five of the fifteen landed units.**
The landing commit is titled "Update TODO.md for the completed 1.0.0 milestone" and its Status paragraph asserts `next` "holds the completed 1.0.0 milestone", but Completed Steps has no entry for https://git.eeqj.de/sneak/webhooker/issues/64, https://git.eeqj.de/sneak/webhooker/issues/79, https://git.eeqj.de/sneak/webhooker/issues/90, https://git.eeqj.de/sneak/webhooker/issues/113 or https://git.eeqj.de/sneak/webhooker/issues/118 — including both credential-exposure fixes. `REPO_POLICIES.md` requires the TODO be updated meticulously.
**5. `TODO.md:118` dropped "Manual event redelivery from the web UI" from Future Steps, but it is not implemented.**
No redelivery handler, route or template exists anywhere in `internal/` or `templates/`. The only surviving trace is the *planned* API endpoint (`TODO.md:128`, `README.md:919`). Meanwhile `README.md:263` still advertises it in the present tense as a core capability: "**Replay** — Stored events can be manually redelivered for debugging or testing, without requiring the original sender to fire the webhook again." Tagging 1.0.0 would ship a README promising a feature the binary lacks, with the tracking item deleted in the same branch. Acceptable: restore the Future Steps entry (or file an issue) and soften `README.md:263` to planned.
**6. Minor — `static/js/app.js:2`: `console.log("Webhooker loaded");`** ships in the production asset. Pre-existing, but it is debug scaffolding in a 1.0.0 artifact.
**7. Minor — `templates/sources_list.html:3`:** the https://git.eeqj.de/sneak/webhooker/issues/57 copy change to `{{define "title"}}Webhooks - Webhooker{{end}}` is inert, because page title blocks never render (https://git.eeqj.de/sneak/webhooker/issues/117). Harmless, but the commit does not achieve what it claims for that file.
**8. PR state.** Still a draft with `WIP:` in the title, and the body's "Still open in the milestone" list names https://git.eeqj.de/sneak/webhooker/issues/57, https://git.eeqj.de/sneak/webhooker/issues/88 and https://git.eeqj.de/sneak/webhooker/issues/118, all of which landed; the milestone is now 0 open. Body and draft state need correcting before this can merge.
Probes worth recording, all of which passed for the right reason:
- The codec/idle-timeout interaction is safe. `securecookie`'s max-age on `next` is still gorilla's 30-day default while `store.Options.MaxAge` is 7 days (https://git.eeqj.de/sneak/webhooker/issues/108), but `session.expired` now rejects anything past `created_at + 7d` server-side and treats a session carrying no timestamps as expired, so the skew is not reachable. https://git.eeqj.de/sneak/webhooker/pulls/132 remains worth landing on its own merits.
- Consequence of that: every session issued before this branch is invalidated on upgrade — a forced re-login. Correct for a first release, and not stated in the README; worth one line there if any pre-1.0 deployment exists.
- `forwardedClientAddr` hand-checked for hop splitting, empty hops, trailing commas, multi-value headers and the 64-hop cutoff falling back to the peer address: correct in each case.
- The delete-vs-sweep race on archive writers is closed: `sweepWriterFor` / `releaseSweepWriter` / the `evicted` flag are all evaluated under the registry lock, and the sweep never creates an archive file.
- `maxBodyShift` remains live via `internal/handlers/webhook.go:17`, so removing the handler-level `MaxBytesReader` calls did not disturb the receiver's own 1 MB cap.
Disclosure: 14 of 15 commits end with ` (closes #N)`; `Update TODO.md for the completed 1.0.0 milestone` closes no issue, which I judged acceptable for a housekeeping commit rather than a finding. The buffered-rendering interaction could not be assessed on this branch — `renderTemplate` still writes straight to the `ResponseWriter` (`internal/handlers/handlers.go:233`); https://git.eeqj.de/sneak/webhooker/pulls/131 has not landed here.
A non-positive value reached time.NewTicker in the retention reaper and the archive sweeper, panicking both goroutines after startup had already reported success. envPositiveDuration now rejects it in loadFromEnv, matching how PORT and RECEIVER_RATE_LIMIT fail. SESSION_IDLE_TIMEOUT keeps treating non-positive as disabled, which is guarded at every use site.
The workflow writes a build-context fingerprint before calling script/cibuild, so a code commit invalidates the COPY layer of the lint and builder stages and the checks really run, while a docs-only commit still replays from cache. A superseding run also rewrites the exact failure/Has been cancelled status left on commits that were never tested to skipped, so cancellation no longer reads as red. script/cibuild itself is untouched.
The README env table was missing RETENTION_SWEEP_INTERVAL, TODO.md omitted five landed units, and three passages sold manual redelivery in the present tense when nothing implements it. The same false claim was corrected in the doc comment on failUnretryableRetry, which was its source text. Also removes a console.log from the shipped static asset.
The receiver limiter keyed on the request path, and /webhook/{uuid} matches any single segment, so a client minted a fresh bucket per invented path and had unlimited aggregate rate against the only unauthenticated endpoint. An outer limiter keyed on the client address alone now bounds that, chained in front of the unchanged per-entrypoint limiter. Its rejections log at DEBUG without the path, and the README states what each limit does and does not bound.
Adds Completed Steps for the receiver aggregate rate limit, the documentation accuracy pass, the CI gate repair and the RETENTION_SWEEP_INTERVAL bound. Drops the commit hash that pinned the Status paragraph to a specific next head, and rewrites Next Step now that the gate repair it named has landed.
Gate, run by me on 339548d in a fresh clone: docker build --no-cache-filter=lint,builder exit 0 in 210s — #15 RUN make fmt-check DONE 0.8s, #16 RUN make lint -> 0 issues. DONE 59.0s, #23 RUN make test DONE 67.1s (9 ok package lines, zero (cached) markers), #24 RUN make build DONE 51.0s; host make check exit 0. main is an ancestor of next, fast-forwardable, no conflict. No Claude/Anthropic strings or attribution trailers in the tree or in any of the 20 commit messages; inclusive terminology clean; no non-test TODO/FIXME, commented-out blocks or debug code reachable from the production binary; external references hash-pinned. The five findings of #111 (comment) are all confirmed fixed and are not re-reported. None of the findings below duplicate the deferred set.
1. In the default configuration any unauthenticated client on the internet can lock the admin out of the web UI indefinitely — internal/middleware/ratelimit.go:151-172 + :219-226, undocumented at README.md:1120-1122.
LoginRateLimit is 5 POSTs/minute per bucket, and since #88 the bucket key is clientKey: the connection's own peer address unless that peer is in TRUSTED_PROXIES, which defaults to empty. README.md:79-80 and the prod TLS story require a TLS-terminating reverse proxy, so in the default deployment every login POST from every client keys on the proxy's address and shares one 5/minute bucket. An attacker sending 5 login POSTs per minute — about 0.08 req/s, from anywhere — keeps that bucket permanently full, and the operator's own login POST gets 429 forever. There is no second administrative path. PasswordChangeRateLimit is the same 5/min shared bucket.
This is not the trusted-proxy default being wrong: empty is correct. What is wrong is that the consequence is nowhere stated and one place states its opposite. README.md:1120-1122 still advertises the login limiter as "per-IP sliding-window rate limiter on the login endpoint (5 POST attempts per minute per IP)", which is false in the default deployment. README.md:891-895 does disclose the shared bucket but characterises it as "the safe direction to be wrong in" — true for the receiver, exactly inverted for login, where a shared bucket converts a per-attacker throttle into a global lockout. The #### Trusted proxies section at README.md:101-146, the one an operator actually reads while configuring, presents the empty default as having no downside at all.
Acceptable: state in README.md:101-146 that a production deployment behind a reverse proxy MUST set TRUSTED_PROXIES, and that leaving it unset makes the login and password-change limits a single global bucket that any remote client can hold full; and correct README.md:1120-1122 so it does not claim per-IP. A startup WARN when TRUSTED_PROXIES is empty and WEBHOOKER_ENVIRONMENT=prod would be better still.
2. The Docker section describes a build that does not exist, and contradicts the section 15 lines below it — README.md:1131-1135.
It says the Dockerfile is a two-stage build whose "Builder stage (Debian-based golang:1.24) — installs golangci-lint, downloads dependencies, copies source, runs make check". The actual Dockerfile is three stages: lint on golangci/golangci-lint:v2.12.2 running make fmt-check and make lint (Dockerfile:5,26-27), builder on golang:1.26.1-bookworm running make test and make build (Dockerfile:32,49-50), then the runtime stage. Wrong stage count, wrong Go version, wrong image, wrong commands — and README.md:1147-1159, added by #119 in this branch, correctly describes "both check stages" and the four separate targets, so the two paragraphs contradict each other. The described single-builder shape is also the one REPO_POLICIES.md:102-106 forbids. Acceptable: rewrite README.md:1131-1135 to the three stages actually in the tree.
3. The API table is false about the only public endpoint — README.md:917, and README.md:514.
README.md:917 lists ANY | /webhook/{uuid} | Webhook receiver endpoint (accepts all methods). internal/handlers/webhook.go:24-33 answers 405 with Allow: POST to everything except POST, and has since #20. README.md:514 compounds it, documenting Event.method as "HTTP method (POST, PUT, etc.)" when the column can only ever hold POST. Pre-existing on main, but this is the first line an integrator reads about the endpoint the release exists to serve, and the documentation-accuracy pass in this branch (0e397b3) did not catch it. Acceptable: POST and "accepts POST only; other methods get 405", and fix the method field description.
4. The gate status on the PR head is a cache replay and is not evidence — 339548d.
check / check (push) on 339548d reads success, description "Successful in 6s". 339548d is docs-only so the fingerprint mechanism replays legitimately — but the two commits under it, 0e397b3 and 95161c7 (the latter is the receiver aggregate rate-limit fix this PR was failed for last round), both carry status skipped, "Superseded by a newer commit; never tested", and Gitea's combined-status API returns "state":"success" for each of them. The newest status on next that describes an executed run is be57609, "Successful in 2m52s", whose tree predates both of those code commits. So no commit status on this branch evidences an executed check of the current tree, and TODO.md:28-31's claim that next "is verified green both by CI and by cache-defeated container runs" is unsupported on the CI half. The tree itself is fine — my cache-defeated build above is the actual evidence — so this is an evidence defect, not a code defect, but it means the repaired gate still turns "never tested" into a green rollup whenever a run is superseded. Distinct from #147, which covers script/cibuild drift and the hardcoded context string. Acceptable: push a no-op commit or re-run so the head carries a real run before tagging, and soften or substantiate the TODO.md claim. Note for the merge: if next is fast-forwarded rather than squashed, main inherits commit 339548d unchanged, so main's check will also be a seconds-long replay — expected, not a regression.
5. The session-expiry documentation is orphaned inside the trusted-proxy section — README.md:101 vs README.md:148-166.
#66 put the two-clock session prose directly under ### Configuration. #88 then inserted #### Trusted proxies above it without a closing heading, so SESSION_IDLE_TIMEOUT, the 7-day absolute cap and the 10% refresh lag now sit inside a subsection about X-Forwarded-For and are unreachable from the table of contents. Visible only once both units are combined. Acceptable: a #### Sessions heading at README.md:148.
6. Minor — internal/config/config.go:455-467: the startup summary omits sessionIdleTimeout. Every other value parsed in loadFromEnv is logged, including the three added in this milestone. SESSION_IDLE_TIMEOUT is the one variable where a valid setting silently turns off a security control (config.go:99, non-positive disables idle expiry), so it is the one most worth showing back to the operator.
7. Minor — TODO.md:193 lists "Password change and reset flow" under Future Steps while TODO.md:23-24 records the admin password change flow (#65) as already landed on main.
Probes that passed, recorded because they were the likely failure sites: the two chained receiver limiters compose correctly (aggregate outer, per-entrypoint inner, saturating multiply at ratelimit.go:333-339, no unbounded key growth since the outer limiter gates the inner one's key space); forwardedClientAddr hand-checked for hop splitting, empty hops, multi-value headers and the 64-hop cutoff falling back to the peer; the codec max-age / idle-timeout skew stays unreachable because session.expired treats a session with no timestamps as expired; the three background loops each root at context.Background() and each OnStop cancels then waits, with no shared state between the reaper's per-webhook DBs and the sweeper's archive files; renderTemplate still streams to the ResponseWriter (internal/handlers/handlers.go:233), so the buffered-rendering interaction does not exist on this branch; the .ci-fingerprint barrier is correctly excluded from .dockerignore and does not disturb the policy-mandated lint stage; no raw config blob reaches any template.
Disclosures: two of the 20 commit titles carry no (closes #N) — 543005c and 339548d, both TODO.md housekeeping — which I judged acceptable, consistent with the prior review. I could not independently determine whether run 167 or 168 completed their docker build before being cancelled, so finding 4 is stated as unsupported evidence rather than as a proven untested tree. internal/delivery/target_config_view.go:115-118 renders the HTTP target URL unmasked; that is #115 and is your call, not a finding here.
Independent integration review of `origin/main..origin/next` @ 339548d (20 commits, 65 files). **FAIL — `needs-rework`.**
Gate, run by me on 339548d in a fresh clone: `docker build --no-cache-filter=lint,builder` exit 0 in 210s — `#15 RUN make fmt-check` DONE 0.8s, `#16 RUN make lint` -> `0 issues.` DONE 59.0s, `#23 RUN make test` DONE 67.1s (9 `ok` package lines, zero `(cached)` markers), `#24 RUN make build` DONE 51.0s; host `make check` exit 0. `main` is an ancestor of `next`, fast-forwardable, no conflict. No Claude/Anthropic strings or attribution trailers in the tree or in any of the 20 commit messages; inclusive terminology clean; no non-test `TODO`/`FIXME`, commented-out blocks or debug code reachable from the production binary; external references hash-pinned. The five findings of https://git.eeqj.de/sneak/webhooker/pulls/111#issuecomment-58699 are all confirmed fixed and are not re-reported. None of the findings below duplicate the deferred set.
**1. In the default configuration any unauthenticated client on the internet can lock the admin out of the web UI indefinitely — `internal/middleware/ratelimit.go:151-172` + `:219-226`, undocumented at `README.md:1120-1122`.**
`LoginRateLimit` is 5 POSTs/minute per bucket, and since https://git.eeqj.de/sneak/webhooker/issues/88 the bucket key is `clientKey`: the connection's own peer address unless that peer is in `TRUSTED_PROXIES`, which defaults to empty. `README.md:79-80` and the prod TLS story require a TLS-terminating reverse proxy, so in the default deployment *every* login POST from *every* client keys on the proxy's address and shares one 5/minute bucket. An attacker sending 5 login POSTs per minute — about 0.08 req/s, from anywhere — keeps that bucket permanently full, and the operator's own login POST gets 429 forever. There is no second administrative path. `PasswordChangeRateLimit` is the same 5/min shared bucket.
This is not the trusted-proxy default being wrong: empty is correct. What is wrong is that the consequence is nowhere stated and one place states its opposite. `README.md:1120-1122` still advertises the login limiter as "per-IP sliding-window rate limiter on the login endpoint (5 POST attempts per minute per IP)", which is false in the default deployment. `README.md:891-895` does disclose the shared bucket but characterises it as "the safe direction to be wrong in" — true for the receiver, exactly inverted for login, where a shared bucket converts a per-attacker throttle into a global lockout. The `#### Trusted proxies` section at `README.md:101-146`, the one an operator actually reads while configuring, presents the empty default as having no downside at all.
Acceptable: state in `README.md:101-146` that a production deployment behind a reverse proxy MUST set `TRUSTED_PROXIES`, and that leaving it unset makes the login and password-change limits a single global bucket that any remote client can hold full; and correct `README.md:1120-1122` so it does not claim per-IP. A startup WARN when `TRUSTED_PROXIES` is empty and `WEBHOOKER_ENVIRONMENT=prod` would be better still.
**2. The Docker section describes a build that does not exist, and contradicts the section 15 lines below it — `README.md:1131-1135`.**
It says the Dockerfile is a two-stage build whose "**Builder stage** (Debian-based `golang:1.24`) — installs golangci-lint, downloads dependencies, copies source, runs `make check`". The actual `Dockerfile` is three stages: `lint` on `golangci/golangci-lint:v2.12.2` running `make fmt-check` and `make lint` (`Dockerfile:5,26-27`), `builder` on `golang:1.26.1-bookworm` running `make test` and `make build` (`Dockerfile:32,49-50`), then the runtime stage. Wrong stage count, wrong Go version, wrong image, wrong commands — and `README.md:1147-1159`, added by https://git.eeqj.de/sneak/webhooker/issues/119 in this branch, correctly describes "both check stages" and the four separate targets, so the two paragraphs contradict each other. The described single-builder shape is also the one `REPO_POLICIES.md:102-106` forbids. Acceptable: rewrite `README.md:1131-1135` to the three stages actually in the tree.
**3. The API table is false about the only public endpoint — `README.md:917`, and `README.md:514`.**
`README.md:917` lists `ANY | /webhook/{uuid} | Webhook receiver endpoint (accepts all methods)`. `internal/handlers/webhook.go:24-33` answers 405 with `Allow: POST` to everything except POST, and has since https://git.eeqj.de/sneak/webhooker/issues/20. `README.md:514` compounds it, documenting `Event.method` as "HTTP method (POST, PUT, etc.)" when the column can only ever hold `POST`. Pre-existing on `main`, but this is the first line an integrator reads about the endpoint the release exists to serve, and the documentation-accuracy pass in this branch (0e397b3) did not catch it. Acceptable: `POST` and "accepts POST only; other methods get 405", and fix the `method` field description.
**4. The gate status on the PR head is a cache replay and is not evidence — 339548d.**
`check / check (push)` on 339548d reads success, description "Successful in 6s". 339548d is docs-only so the fingerprint mechanism replays legitimately — but the two commits under it, 0e397b3 and 95161c7 (the latter is the receiver aggregate rate-limit fix this PR was failed for last round), both carry status `skipped`, "Superseded by a newer commit; never tested", and Gitea's combined-status API returns `"state":"success"` for each of them. The newest status on `next` that describes an executed run is be57609, "Successful in 2m52s", whose tree predates both of those code commits. So no commit status on this branch evidences an executed check of the current tree, and `TODO.md:28-31`'s claim that `next` "is verified green both by CI and by cache-defeated container runs" is unsupported on the CI half. The tree itself is fine — my cache-defeated build above is the actual evidence — so this is an evidence defect, not a code defect, but it means the repaired gate still turns "never tested" into a green rollup whenever a run is superseded. Distinct from https://git.eeqj.de/sneak/webhooker/issues/147, which covers `script/cibuild` drift and the hardcoded context string. Acceptable: push a no-op commit or re-run so the head carries a real run before tagging, and soften or substantiate the `TODO.md` claim. Note for the merge: if `next` is fast-forwarded rather than squashed, `main` inherits commit 339548d unchanged, so `main`'s check will also be a seconds-long replay — expected, not a regression.
**5. The session-expiry documentation is orphaned inside the trusted-proxy section — `README.md:101` vs `README.md:148-166`.**
https://git.eeqj.de/sneak/webhooker/issues/66 put the two-clock session prose directly under `### Configuration`. https://git.eeqj.de/sneak/webhooker/issues/88 then inserted `#### Trusted proxies` above it without a closing heading, so `SESSION_IDLE_TIMEOUT`, the 7-day absolute cap and the 10% refresh lag now sit inside a subsection about `X-Forwarded-For` and are unreachable from the table of contents. Visible only once both units are combined. Acceptable: a `#### Sessions` heading at `README.md:148`.
**6. Minor — `internal/config/config.go:455-467`: the startup summary omits `sessionIdleTimeout`.** Every other value parsed in `loadFromEnv` is logged, including the three added in this milestone. `SESSION_IDLE_TIMEOUT` is the one variable where a *valid* setting silently turns off a security control (`config.go:99`, non-positive disables idle expiry), so it is the one most worth showing back to the operator.
**7. Minor — `TODO.md:193` lists "Password change and reset flow" under Future Steps** while `TODO.md:23-24` records the admin password change flow (https://git.eeqj.de/sneak/webhooker/issues/65) as already landed on `main`.
Probes that passed, recorded because they were the likely failure sites: the two chained receiver limiters compose correctly (aggregate outer, per-entrypoint inner, saturating multiply at `ratelimit.go:333-339`, no unbounded key growth since the outer limiter gates the inner one's key space); `forwardedClientAddr` hand-checked for hop splitting, empty hops, multi-value headers and the 64-hop cutoff falling back to the peer; the codec max-age / idle-timeout skew stays unreachable because `session.expired` treats a session with no timestamps as expired; the three background loops each root at `context.Background()` and each `OnStop` cancels then waits, with no shared state between the reaper's per-webhook DBs and the sweeper's archive files; `renderTemplate` still streams to the `ResponseWriter` (`internal/handlers/handlers.go:233`), so the buffered-rendering interaction does not exist on this branch; the `.ci-fingerprint` barrier is correctly excluded from `.dockerignore` and does not disturb the policy-mandated lint stage; no raw config blob reaches any template.
Disclosures: two of the 20 commit titles carry no ` (closes #N)` — 543005c and 339548d, both `TODO.md` housekeeping — which I judged acceptable, consistent with the prior review. I could not independently determine whether run 167 or 168 completed their docker build before being cancelled, so finding 4 is stated as unsupported evidence rather than as a proven untested tree. `internal/delivery/target_config_view.go:115-118` renders the HTTP target URL unmasked; that is https://git.eeqj.de/sneak/webhooker/issues/115 and is your call, not a finding here.
With no trusted proxies configured, every client behind the reverse proxy production requires shares one rate-limit bucket per limit, so five POSTs per minute from anywhere holds the login limit full and denies the admin login until restart. The default is still correct; it was the consequence that was invisible. Startup now warns, and the README no longer claims the login limit is per-IP unconditionally.
The http target's destination URL can itself be a bearer credential, and
the source detail page rendered it in full. Render it through the
existing MaskURL instead, matching the rule already applied to slack
targets.
Independently reviewed: mutation-verified (reverting to the raw value
fails the absence assertions, not merely the masked-form ones), MaskURL
probed against userinfo, query, fragment, port, IPv6 literal and
non-http schemes, and every sibling path that surfaces target data
re-walked and found clean.
The event log rendered stored bodies untruncated. Since buffered
rendering landed (#123) that became resident memory per concurrent
viewer, up to tens of MB, driven by payloads unauthenticated clients
supply to the public receiver.
Bound in the query rather than the template, via
substr(cast(body as blob), 1, ?) plus length(cast(body as blob)), so an
oversized body never becomes a Go string at all. Adds an EventLogView
projection carrying the true byte count, and trims a partial UTF-8 tail
without rewriting bodies that are merely invalid UTF-8.
Independently reviewed. The generated SQL was dumped under GORM DryRun
to confirm the cap is a bound parameter, both casts are present, and no
other path selects the full column; soft-delete scope, ordering and
pagination are unchanged.
Correction to the PR body: its quoted mutation output was produced by
removing the bound from eventLogColumns, not by raising the cap to
1<<30 as the text claimed. The reviewer reproduced the real
mutation and confirmed the tests do catch removal of the bound.
Follow-up #157 restores in-app retrieval of bodies above the cap.
static/js/alpine.min.js was a committed minified bundle, which
REPO_POLICIES forbids, referenced by no content hash at all. A minified
blob is unreviewable, which is the shape a supply-chain compromise
takes.
script/fetch-assets now downloads Alpine 3.14.9 from the npm registry
and verifies sha256 on both the tarball and the extracted file, and
static/vendor_test.go re-hashes the bytes go:embed actually placed in
the binary. The shipped bytes are byte-identical to the blob that was
committed, so the served asset does not change.
Independently reviewed. Five negative controls reproduced by the
reviewer: flipped expected hash, repointed URL, post-fetch tampering,
asset absent, and manifest inconsistencies — each fails closed with
static/js/ left clean. Registry hashes confirmed against the pins, and
the runtime image was built, run and curled to confirm the asset is
still served and the login page still loads it.
Known gap, filed separately: static/static.go embeds the js directory
rather than named files, so a missing fetched asset is not a compile
error on ungated local build paths. Every gated path fails loudly, so
the release artifact is unaffected.
Publishing this README would have shipped false statements about the
product. Corrects the eight items on the issue plus everything a full
sweep turned up: the Slack circuit-breaker scope, a nonexistent WAL, the
wrong config key for slack targets, six undocumented routes, the
conditional /metrics registration, wrong retention bands, wrong shutdown
mechanism, and a Quick Start that led a new contributor into a red
build.
The lockout warning now fires whenever TRUSTED_PROXIES is empty rather
than only in production, since the variable it was gated on defaults to
dev. Rate-limit keying, the limits and the TRUSTED_PROXIES default are
untouched — those belong to #150.
What /s/* actually serves was settled empirically rather than by
reading: all five of GET/HEAD/POST/PUT/DELETE return 200, pinned by
TestStaticServesEveryMethod. Restricting it is filed separately.
Independently reviewed after three prior rounds. The reviewer
re-derived all fifteen claim-table rows against the code, including
every row a previous revision had marked "correct, left alone" and got
wrong, and found zero false; then verified every route method-by-method,
all twelve environment variables, all nine entity tables, and the
package tree against git ls-files. The Quick Start was confirmed by
running it in a fresh clone.
Milestone triage, recorded once. The 1.0.0 milestone stood at 22 closed / 1 open when I picked this up. It now has seven open, because six issues had been filed with an explicit "not milestoned 1.0.0" note that no longer held, and one is new. Every move is argued on its own issue; the summary:
Moved in, reversing the filer's original call
#146 — an unauthenticated client writes unbounded attacker-chosen text into the operator's logs via the public receiver. "Every access log does this" is true of authenticated admin surfaces, not of an internet-exposed endpoint that 404s invented paths for free.
#152 — 1.0 is where the repo starts being consumed programmatically, and a combined status that reports success for a commit nothing ever ran is a false green that branch protection or a release script will read as fact.
#135 — the original call said the page is authenticated. That bounds who TRIGGERS the render, not who SUPPLIES the payload; the bodies come from the public receiver. It also went from hypothetical to live when #123 landed buffered rendering. Since merged.
#145 — the 1.0 artifact would ship unreviewable, un-hash-pinned third-party JavaScript to the admin's browser, breaking two REPO_POLICIES.md rules the release claims to follow. "Pre-existing" describes when it started, not whether it belongs in the tag. Since merged.
#134 — #130 shipped a shutdown bound that is inert in the container this repo actually produces, so the database close never runs under a stock docker stop.
#125 — the receiver rate limit is a README-documented security control that any client with an ordinary residential or mobile IPv6 /64 bypasses completely, with no spoofing and nothing to detect.
#150 — the shipped default lets a stranger deny the operator the only administrative path at 5 requests per minute. #149 made that visible; visibility is not a control.
Opened during the work, and milestoned
#157 — the #135 cap left bodies over 8 KB unreachable through the product. Its definition of done permitted deferring this, but shipping a webhook store that cannot show you a 20 KB payload is a regression we introduced.
#160 — found by the review of #126. The remaining ingress path in the credential-exposure class that #113, #118 and #115 each closed one leg of.
Deliberately left out, though tempting: #107 (the strongest pure correctness defect in the backlog, but not unauthenticated-reachable), #117, #128, #127, #166, #168, #169, and the test-hygiene and tooling set (#93, #94, #99, #101, #103, #120, #154). The test applied throughout: can an unauthenticated attacker on the public internet exploit it, or does it make the shipped artifact wrong, untrustworthy or unverifiable?
Two decisions I took rather than parking, both reversible by closing the PR: #115 (mask unconditionally) and #125 (option 1, /64). Only #150 is genuinely sneak's, and it is assigned to him with a corrected recommendation — the option originally recommended there does not fix the reported attack, since flooding the operator's own predictable username still locks them out.
Milestone triage, recorded once. The 1.0.0 milestone stood at 22 closed / 1 open when I picked this up. It now has seven open, because six issues had been filed with an explicit "not milestoned 1.0.0" note that no longer held, and one is new. Every move is argued on its own issue; the summary:
**Moved in, reversing the filer's original call**
- https://git.eeqj.de/sneak/webhooker/issues/146 — an unauthenticated client writes unbounded attacker-chosen text into the operator's logs via the public receiver. "Every access log does this" is true of authenticated admin surfaces, not of an internet-exposed endpoint that 404s invented paths for free.
- https://git.eeqj.de/sneak/webhooker/issues/152 — 1.0 is where the repo starts being consumed programmatically, and a combined status that reports `success` for a commit nothing ever ran is a false green that branch protection or a release script will read as fact.
- https://git.eeqj.de/sneak/webhooker/issues/135 — the original call said the page is authenticated. That bounds who TRIGGERS the render, not who SUPPLIES the payload; the bodies come from the public receiver. It also went from hypothetical to live when https://git.eeqj.de/sneak/webhooker/issues/123 landed buffered rendering. Since merged.
- https://git.eeqj.de/sneak/webhooker/issues/145 — the 1.0 artifact would ship unreviewable, un-hash-pinned third-party JavaScript to the admin's browser, breaking two `REPO_POLICIES.md` rules the release claims to follow. "Pre-existing" describes when it started, not whether it belongs in the tag. Since merged.
- https://git.eeqj.de/sneak/webhooker/issues/134 — https://git.eeqj.de/sneak/webhooker/issues/130 shipped a shutdown bound that is inert in the container this repo actually produces, so the database close never runs under a stock `docker stop`.
- https://git.eeqj.de/sneak/webhooker/issues/125 — the receiver rate limit is a README-documented security control that any client with an ordinary residential or mobile IPv6 `/64` bypasses completely, with no spoofing and nothing to detect.
- https://git.eeqj.de/sneak/webhooker/issues/150 — the shipped default lets a stranger deny the operator the only administrative path at 5 requests per minute. https://git.eeqj.de/sneak/webhooker/issues/149 made that visible; visibility is not a control.
**Opened during the work, and milestoned**
- https://git.eeqj.de/sneak/webhooker/issues/157 — the https://git.eeqj.de/sneak/webhooker/issues/135 cap left bodies over 8 KB unreachable through the product. Its definition of done permitted deferring this, but shipping a webhook store that cannot show you a 20 KB payload is a regression we introduced.
- https://git.eeqj.de/sneak/webhooker/issues/160 — found by the review of https://git.eeqj.de/sneak/webhooker/pulls/126. The remaining ingress path in the credential-exposure class that https://git.eeqj.de/sneak/webhooker/issues/113, https://git.eeqj.de/sneak/webhooker/issues/118 and https://git.eeqj.de/sneak/webhooker/issues/115 each closed one leg of.
**Deliberately left out**, though tempting: https://git.eeqj.de/sneak/webhooker/issues/107 (the strongest pure correctness defect in the backlog, but not unauthenticated-reachable), https://git.eeqj.de/sneak/webhooker/issues/117, https://git.eeqj.de/sneak/webhooker/issues/128, https://git.eeqj.de/sneak/webhooker/issues/127, https://git.eeqj.de/sneak/webhooker/issues/166, https://git.eeqj.de/sneak/webhooker/issues/168, https://git.eeqj.de/sneak/webhooker/issues/169, and the test-hygiene and tooling set (https://git.eeqj.de/sneak/webhooker/issues/93, https://git.eeqj.de/sneak/webhooker/issues/94, https://git.eeqj.de/sneak/webhooker/issues/99, https://git.eeqj.de/sneak/webhooker/issues/101, https://git.eeqj.de/sneak/webhooker/issues/103, https://git.eeqj.de/sneak/webhooker/issues/120, https://git.eeqj.de/sneak/webhooker/issues/154). The test applied throughout: can an unauthenticated attacker on the public internet exploit it, or does it make the shipped artifact wrong, untrustworthy or unverifiable?
Two decisions I took rather than parking, both reversible by closing the PR: https://git.eeqj.de/sneak/webhooker/issues/115 (mask unconditionally) and https://git.eeqj.de/sneak/webhooker/issues/125 (option 1, `/64`). Only https://git.eeqj.de/sneak/webhooker/issues/150 is genuinely sneak's, and it is assigned to him with a corrected recommendation — the option originally recommended there does not fix the reported attack, since flooding the operator's own predictable username still locks them out.
Rate-limit keys were per-address, i.e. per /128 for IPv6. A routed /64
is the normal residential and mobile allocation, so a client rotated
source addresses inside its own prefix and minted a fresh bucket per
request — evading every limiter at the network layer, with no spoofing
and nothing to detect. #88 closed the header half of this control; this
is the network half.
IPv6 now keys on the /64, IPv4 on the full address, via stdlib
net/netip. IPv4-mapped form is unmapped rather than masked, so clients
behind a mapping proxy do not collapse into one bucket.
Independently reviewed twice. The first round found the trusted-proxy
forwarded path — the one carrying production traffic — had no coverage
at all, so a silent revert there was undetectable; that is now pinned.
The reviewer confirmed both branches are independently mutation-tested:
reverting either the direct-peer return or the forwarded return alone
fails only that branch's tests. The 18-site test-constant refactor was
verified byte-identical against next, with no pre-existing assertion
changed.
Known remaining coverage gap, judged not a defect: the fallback when the
peer is trusted but the forwarded address does not parse has no test.
Only operator-controlled addresses inside TRUSTED_PROXIES reach it, they
already share the proxy's single bucket, and masking there can only
merge operator proxies — fail-closed, nothing attacker-controlled.
fx defaults to a 15s stop timeout and the Dockerfile sets no grace
override, so Docker SIGKILLed at 10s and the bounded shutdown #130 built
— including the log line that tells an operator a component is wedged —
was unreachable in the image this repo produces.
Sets fx.StopTimeout to 5s, and lowers the HTTP drain to 3s so a
full-length drain no longer exhausts the whole sequence budget and skip
every later hook, database close included. The Sentry flush, which runs
in the same hook and honours no context, is clamped to the remaining
stop budget less a 2s tail reserve, so a stalled flush drops Sentry
events rather than the database close.
Also fixes a latent coin flip in the shared stop-hook waiter, which
reported "shutdown timed out" about half the time for a component that
drained cleanly against an already-expired context.
Independently reviewed three times. The final reviewer derived a
stronger invariant than the implementation claims — the server hook's
absolute end is bounded at stopTimeout minus the reserve regardless of
drain length or of time consumed by preceding hooks — and confirmed the
guard's 10ms sweep cannot step over the maximum, since both breakpoints
land on its grid. Both Sentry probe arms, the docker stop demo and every
mutation were reproduced independently.
Known residual, filed separately: the HTTP drain itself is not clamped
by the reserve, so slow preceding hooks can still jointly exhaust the
budget. Demonstrated with a 2.2s sweeper delay.
Gitea records a cancelled run as failure, and the #119 repair rewrote
that to skipped. Gitea's Combine() folds skipped into success, so a
commit nothing ever tested reported a combined green — observed on three
commits on next, including the very change a prior integration review
had failed a PR for.
Superseded commits are now marked failure with an honest description, so
never-tested no longer reads as passed and git bisect archaeology can
tell "passed", "failed" and "never ran" apart. Option 1, re-running the
superseded commit, was verified unreachable for automation on Gitea
1.25.4: no rerun endpoint, dispatches takes a ref not a SHA and lands
under a different context, and CancelPreviousJobs is unconditional.
The rewrite moves out of the workflow into script/ci-mark-superseded so
the tested artifact is the shipped one, and every failure path in it is
loud: an unparseable or empty ANCESTOR_LIMIT, an unreadable ancestor
status, and a shallow clone all abort rather than exiting 0 having
marked nothing. Each has a regression test. The status context is
derived rather than hardcoded, which also closes#147 item 2; item 1
remains open.
Independently reviewed four times. The final reviewer confirmed the
shallow-clone test is genuinely shallow — a file:// URL is load-bearing,
since git silently ignores --depth on a local path — and that deleting
the guard fails that one test out of 331 and cannot pass for the wrong
reason. They also reproduced deterministically that go test's cache
serves a stale PASS after a script-only edit, which internal/ciscript's
doc.go now records.
The access log wrote one INFO line per request carrying the full
attacker-controlled URL, on the unauthenticated public receiver, so a
client inventing paths wrote unbounded arbitrary text into the
operator's logs.
Rejected requests now log the chi route pattern instead of the concrete
URL — extended to 3xx as well as 4xx, because RequireAuth answers 303
and so /user/<anything> was an unauthenticated path-varying vector. The
query is redacted on the branches that keep a concrete path, and every
client-supplied field is capped: url, useragent and referer at 512
bytes, request_id at 128, method at 32. The caps are spent in ENCODED
bytes, so escaping cannot multiply them.
One INFO line per request, at most 2,560 bytes — a figure derived
arithmetically rather than observed, with the fixed portion measured at
336 (JSON) and 286 (text).
Independently reviewed four times, and broken three of those times on
the same class of defect: a stated bound the code did not have. Round 1
left the 2xx query and the headers unbounded; round 2 counted raw bytes
against an encoded ceiling and broke at 2,611; round 3 charged 6 bytes
for every non-printable when strconv.Quote spells astral ones as
\UXXXXXXXX, and broke at 2,676. Two independent exhaustive audits over
all 1,112,064 code points, built by different methods, now both report
zero undercharged runes on either handler. Measured worst case over a
real TCP socket is 1,972 bytes, 77% of the ceiling.
Follow-up filed to assert that charge against every code point in the
suite, so the ceiling defends itself rather than resting on one
hand-picked rune.
The 8 KB render cap from #135 left storage untouched but no route served
the rest, so a body over the cap was reachable only with filesystem
access to the SQLite files — in a product whose purpose is storing
webhooks so they can be inspected.
GET /source/{sourceID}/logs/{eventID}/body serves the whole body to the
webhook's owner, as application/octet-stream with an attachment
disposition and nosniff. Those are a security control, not formatting:
the bytes come from the public receiver and are handed back inside the
operator's authenticated origin, and the existing CSP would not stop a
stored HTML payload executing there. The truncation marker links to it
only when a body was actually cut.
Accepted deviation, documented rather than glossed: #157's definition of
done asks the route to stream from the row. It buffers whole instead,
because database/sql exposes no incremental handle on a SQLite BLOB and
substr range reads re-materialise the entire column per call — an
earlier revision chunked at 64 KiB and was 11-15x slower for a worse
bound. Three independent reviewers confirmed no streaming path exists.
Independently reviewed three times. Two earlier revisions each asserted
a memory bound the code did not have; the final reviewer measured
2.057x at the ingest cap and pinned the two overlapping allocations from
source — the driver's column buffer and database/sql's convertAssign
clone — confirming the stated "roughly two bodies, and 2x is a floor
not a ceiling" is now accurate, since SQLite's own materialisation sits
outside the Go heap.
golangci-lint no longer runs on the host. script/lint builds
Dockerfile.lint, which copies the repo into the digest-pinned linter
image, so the container holds only this repo and the cross-worktree
cache contamination of #106 becomes structurally impossible rather than
filtered after the fact. Five workers hit that contamination in one
evening, in both directions.
Three properties are load-bearing. --no-cache-filter=lint forces the
lint stage to re-execute while deps keeps its cache, because a cached
build lints nothing in 0.27s and exits 0. script/lint does not trust
that flag, since Docker silently ignores an unmatched stage name: it
asserts golangci-lint's own summary line appears, so no summary means no
lint whatever the exit code says. And both lint steps run
--network=none, which enforces rather than assumes that config verify
does not fetch its schema — verify is kept, because golangci-lint run
silently ignores unrecognized config keys and it is the only thing that
catches a typo that disables a setting.
Independently reviewed four times. Three passed the behaviour; the
remaining rounds were a README merge against #151, whose premise was
that the file contains no false statements. Six statements this change
falsified were found and corrected across those rounds — the last
reviewer re-derived every countable claim against the tree rather than
reading for plausibility, and found no seventh.
Supersedes #106.
In the shipped default, any stranger denied the operator the only
administrative path at 5 requests per minute: TRUSTED_PROXIES is empty,
the README requires a reverse proxy, so every login POST shared one
bucket keyed on the proxy.
Credentials are now verified first and only a FAILED attempt spends
budget, so a correct password is never throttled. Failures are counted
per (client bucket, submitted username), bounded. Concurrent Argon2id
verifications are capped at two, and the queue for them at 16 — because
verifying first lets an attacker force a 64 MB hash per request, and
bounding the wait alone bounds nothing.
The issue's own recommendation was insufficient and is rejected here:
keying by username stops an attacker locking out a DIFFERENT account,
but this is a single-admin product with a predictable bootstrap
username, so flooding the operator's own name still locks them out.
This is speculative — it implements a corrected recommendation ahead of
the owner's ruling so the decision can be made by merging or reverting.
Three things are disclosed rather than glossed: online guessing rises
from 5/min to roughly 27/s, because the 429 is a label on the response
and not a gate in front of the hash; the residual exposure is a loss of
login AVAILABILITY, not latency, and a determined flood still denies
login while it runs, at ~400x the cost and clearing the moment it
stops; and the endpoint should be provisioned for ~400 MB resident, not
the 203 MB of live commitment it itemises.
Independently reviewed four times. Reviewers disproved the suspected
FIFO starvation by measurement, then caught two successive memory
bounds the code did not have — the second by parking waiters and
reading the heap rather than checking the arithmetic.
r.FormValue falls back to the query string, so
POST /source/{id}/targets?url=<secret> created a working target from a
value carried on the request line — where proxy logs, browser history
and Referer all record it. Every form read is now r.PostFormValue,
including the login password and both password-change fields, which had
the same defect in a more acute form.
The Sentry leg needed more than the query string. sentryhttp attaches
the whole request to the scope, and ApplyToEvent copies the teed body
into Request.Data with no SendDefaultPII guard — so reading every field
from the body only pointed every credential this change protects at the
one field the first revision did not scrub. Body and query are now
redacted, Cookies and Env cleared, and Headers reduced to an allowlist,
because the SDK's own filter removes four names and would otherwise ship
X-Csrf-Token and the shared secrets senders put on the receiver route.
Also adds json:"-" to Target.Config, APIKey.Key and Setting.Value —
TargetView is the masking barrier for the HTML path only, and the first
handler to marshal a model would serialise a bearer token or the session
encryption key.
Independently reviewed three times. The second review found the Data
leak and proved it with a scratch module; the third disproved the
PR's own claim that BeforeSend gets no request, so the README now
records that redacting unconditionally is a deliberate choice rather
than a limitation — which is what makes #179 cheap to fix.
#160 scrubbed the Sentry body, query, cookies, env and headers but kept
Request.URL, which the SDK builds from the concrete path. On the
receiver that path is /webhook/<uuid> in full — a write capability, not
an identifier: anyone holding it can inject events the operator's
targets then deliver. #146's "2xx and 5xx keep the concrete path" ruling
was reasoned about a log the operator owns and does not transfer to a
tracker with its own retention and access control.
The chi route pattern now replaces the path on every route, reached via
the request the SDK carries on hint.Context. Unconditional, because a
route-conditional rule leaks on any route someone forgets to add, and on
a static route the pattern is the path anyway. The fallback is never the
concrete path.
Also rewrites event.Transaction, which carries the same UUID on the
sibling dispatch and which the issue did not name. Tracing is off today,
so that half is a floor rather than a live fix — and it is why enabling
tracing later needs #185 first, or every transaction collapses into one
bucket.
Independently reviewed. The reviewer ran fourteen adversarial probes —
404 and 405 panics, panics in middleware before and after routing,
direct CaptureException, mounted subrouters, wildcards, tracing on and
off — and found no path where the concrete URL survives, and no third
field carrying it.
Merge note: the final round was a two-comment documentation fix on an
already-passed review, correcting a rationale that called the host
operator configuration when it is the client's Host header. I verified
that amend is comment-only myself rather than spending a fifth review
round on it.
The Status section claimed next held the completed 1.0.0 milestone with
every issue closed. Four are open (#176, #178, #186, #187), so a merge
of next to main would have shipped that claim to main.
Next Step still named #115 and #125 as open owner decisions; both
landed. It now names the real open items, #150 and #112, and the forced
merge order for the remaining four.
Completed Steps was seventeen units behind, back to 2026-08-12.
clawbot
changed title from WIP: Milestone: delivery lifecycle, retention, config and session hardening to Milestone 1.0.0: internet-facing readiness2026-08-18 10:50:50 +02:00
clawbot
removed their assignment 2026-08-18 10:50:56 +02:00
sneak
was assigned by clawbot2026-08-18 10:50:56 +02:00
Held as WIP: and unassigned — not merge-ready. sneak, 2026-08-20: "webhooker must be to mvp before tagging 1.0. it is prerelease now and must be usable in low volume prod by me before a 1.0".
The 1.0.0 gate is no longer "milestone empty" but "deployable and usable in low-volume production". A code-level deployability audit is running now: first-run admin bootstrap, the blast radius of having no edit forms for targets and entrypoints (#127), inbound authentication on the receive endpoint (#67), retention end to end, and a live run pushing a webhook through to a real sink.
Its blockers get milestoned to 1.0.0. This PR reopens for merge when they are closed.
Held as `WIP:` and unassigned — not merge-ready. sneak, 2026-08-20: "webhooker must be to mvp before tagging 1.0. it is prerelease now and must be usable in low volume prod by me before a 1.0".
The 1.0.0 gate is no longer "milestone empty" but "deployable and usable in low-volume production". A code-level deployability audit is running now: first-run admin bootstrap, the blast radius of having no edit forms for targets and entrypoints (https://git.eeqj.de/sneak/webhooker/issues/127), inbound authentication on the receive endpoint (https://git.eeqj.de/sneak/webhooker/issues/67), retention end to end, and a live run pushing a webhook through to a real sink.
Its blockers get milestoned to 1.0.0. This PR reopens for merge when they are closed.
Both queue-depth reads used (*gorm.DB).Scan, which swaps GORM's own trace
recorder in for the logging adapter. That recorder does not implement
gorm.ParamsFilter, so those statements logged their bound values
interpolated, bypassing the suppression added for #207.
The scan guard from #222 and the queue-depth sampler from #224 each gated
green against a next that lacked the other; both landed and next went red.
Converted to Find. The emitted SQL is identical apart from placeholders,
and both paths parse the anonymous dest schema the same way, so the
queue-depth gauges are unchanged.
GORM's association upsert copied whole targets rows -- plaintext
credential-bearing config -- into the per-webhook event databases with an
empty webhook_id. The leak was in updateDeliveryStatus, not the create
path: Update leaves Statement.Model pointing at a Delivery whose Target
the engine populated, so save_before_associations upserts it.
A connection-level callback now appends clause.Associations to
Statement.Omits on the create and update chains of every per-webhook
connection, so every write path is covered rather than one call site.
Existing files are swept on first open: the leaked rows are deleted and
the file is VACUUMed, because DELETE alone only unlinks the pages and
leaves the credential recoverable in the file's free space. The sweep is
recorded in PRAGMA user_version only after the VACUUM returns, so a sweep
that fails or is interrupted fails the open and is retried on the next
one, rather than being marked done.
Encryption of target config at rest is deliberately out of scope and
deferred to #212.
The receiver had no inbound authentication of any kind: /webhook/{uuid}
was mounted behind a rate limiter alone, so the only thing protecting an
entrypoint was the secrecy of a v4 UUID in a URL path. Inbound headers are
forwarded almost verbatim to the target, so anyone who learned the URL
also chose the headers the downstream service received.
Adds an optional per-entrypoint secret with two schemes: github
(X-Hub-Signature-256, HMAC-SHA256 hex over the raw body) and gitlab
(X-Gitlab-Token, a plain shared token). Comparison is constant-time, the
HMAC is computed over the raw body before any parsing, and rejection
happens before persistence -- an unauthenticated request creates no event
row. An entrypoint with no secret behaves exactly as before, including
every row that predates this change.
The scheme's credential header is stripped from the header map before it
is marshalled into Event.Headers, so the GitLab token reaches neither the
event store nor any delivery target. SchemeInfo.HeaderIsDigest defaults to
false meaning strip, so a scheme added later is protected unless its
header is positively declared a digest.
The admin bootstrap password was printed once, as one line among roughly
45 fx lines, and under docker run -d went to container logs subject to
rotation. There was no reset path at all -- no subcommand, no forgot-password
flow, no env override -- so recovery meant hand-deleting the users row from
webhooker.db, which was documented nowhere.
Adds webhooker resetpw [-generate] <username>. The password is read from
stdin or generated with the existing crypto/rand helper, never taken from
argv where /proc would publish it. It reuses the existing Argon2id hashing
rather than reimplementing the parameters, and writes a single UPDATE only
after the hash is complete, so no failure can leave an account with no
usable password. An unknown username is a hard error and never creates an
account.
It refuses to run against a DATA_DIR held by a live instance, via the
exclusive lock from #201. DATA_DIR and webhooker.db are checked to exist
before the lock is acquired, so a mistyped path creates nothing -- neither
a directory tree nor a stray lock file.
The bootstrap password now appears exactly once, in a distinct banner
written straight to a caller-named writer rather than as an fx log line.
There was no redelivery path anywhere: once a delivery exhausted
max_retries it was failed permanently, even though the event body is
durably stored. Storing an event and being unable to re-send it defeats
the reason it is stored, and the ordinary case is a destination that was
down longer than the backoff ladder.
Adds POST /source/{sourceID}/deliveries/{deliveryID}/replay, inside the
authenticated group so it inherits MaxBodySize, CSRF, NoCache and
RequireAuth. Replay creates a NEW pending delivery against the target's
CURRENT config and hands it to the engine through the same notifier the
receiver uses, so it runs the normal path with the retry ladder, the
SSRF-guarded transport and the circuit breaker. The original delivery's
rows are never touched, and the stored event body is re-sent, never the
recorded response.
Replay is refused, with a distinct message, for a non-terminal delivery, a
deleted target, a deactivated target, and when an earlier replay of the
same event and target is still in flight. Bounded by a per-client rate
limit and by that in-flight check.
The new delivery row is written with Omit(clause.Associations) and with
neither Event nor Target populated, so it cannot upsert a targets row into
the per-webhook event database (#206).
Counted by webhooker_delivery_replays_total on the existing target_type
label. A replay also moves the ordinary attempt, outcome and duration
series, because it is a real delivery.
delivery_results stored status_code, response_body, error, duration and
attempt_num, and no template rendered any of it, so a failure read as
"target: failed" and diagnosing it meant opening the per-webhook SQLite
file by hand.
An expanded delivery now lists its attempts with attempt number, status
code, duration, error and response body. The body is bounded in the query
rather than read whole and truncated in Go (#135), and a body the engine
itself cut is no longer presented as complete.
The response body and error are untrusted remote content, so target
credentials are removed before rendering. Two cases needed care: a secret
severed by the 4096-byte cut matches nothing as a whole string, and the
engine's io.LimitReader cuts at the same constant the renderer uses, so
the guard keys on the body reaching the cap rather than on the stored size
exceeding it. Empty secrets are filtered where the secret list is built,
because an empty string passed to strings.ReplaceAll inserts the marker at
every byte boundary.
loadTargetMap builds the redactor half unscoped, so a soft-deleted
target's historical deliveries still render redacted.
Also regenerates static/css/tailwind.css, which had drifted from the
templates: hover:text-red-700, text-red-500, underline and w-28 were in
use but absent from the served stylesheet (#236).
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Taggable. Milestone 1.0.0 (https://git.eeqj.de/sneak/webhooker/milestone/9) is 0 open.
next@d61d9dc, 83 commits ahead ofmain, strict fast-forward (merge-base==mainhead).Verification at
d61d9dcCache-defeated container build per #119, from a fresh clone:
docker build --no-cache-filter=lint --no-cache-filter=builder --progress=plain .— exit 0.Every content-bearing stage executed, confirmed per vertex rather than inferred:
[lint 7/9] make fmt-check[lint 8/9] golangci-lint config verify[lint 9/9] golangci-lint run0 issues.[builder 8/11] script/fetch-assets[builder 9/11] make test[builder 10/11] make build[builder 11/11]static relinkFive vertices were CACHED, not zero:
#8the digest-pinnedgolangci/golangci-lint:v2.12.2FROMresolve,#7the digest-pinnedgolang:1.26.1-bookwormFROMresolve, and#28/#29/#30the Alpinestage-2runtime layers (apk add ca-certificates,adduser,WORKDIR). None carries a dependency on repository content, so none can mask a stale result.One limit on that log: buildkit clipped the
make testvertex at its 2 MiB output cap, so the per-package tally is not readable from it.script/testisgo test -v -race -timeout 90s ./...underset -eu, so a failure would have failed the build — but the tally below comes from a separate run, not from that log.Separate uncapped
GOFLAGS=-count=1 make checkon the same clone at the same commit: exit 0, 23/23 packages (21ok+ 2 no-test), 0FAIL, 0(cached), 0 data races.Binary from the built image:
sha256:1f81b6749a7d5ba102d6e5ecfd2a7b9f9025ef9ad72578ea453a20f01fca7339, 32217760 bytes.A fresh clone requires
make bootstrapbeforemake check— #282.No commit ahead of
maincarries an attribution trailer.Landed since the last revision of this body
*Timestamp:* 0001-01-01T00:00:00Z, because no delivery task carriedcreated_atand nothing read it back. Verified live against a running instance at this head, not from tests: event POSTed at15:44:00.27-15:44:00.29Z, sink received*Timestamp:* `2026-08-24T15:44:00Z`.CreatedAtunset; it now renders`unknown`rather than a year-1 date, so an expected reap cannot be misread as the defect above returning.0600.SENTRY_DSNor a malformed.envaborts startup instead of silently defaulting._txlock=immediate, nocache=shared, a bounded pool, and delivery re-dispatch gated by reference-counted ownership. An operator can now runsqlite3 .dumpagainst a live database; that previously rejected 41 of 60 inbound webhooks and re-sent already-delivered events on restart. Verified across 1200 events / 7200 deliveries: 7200 POSTs, zero duplicates, zero new POSTs after restart.Your scope decisions, recorded
/metricsand Sentry are acceptable. No masking work done.For you — none of it blocking the merge
nextis not linear.9313b0fis a merge commit; the fast-forward is unaffected, but do not expect a clean rebase if you want linear history.dlopen/getaddrinfowarning — your deliberate Debian-builder/Alpine-runtime design. Delivery by DNS name worked end to end in both audits.script/dockerdiverges from the byte-identical model script to pass a version build arg, and no green CI run exercises a real stamp. Durable fix is upstream insneak/prompts.make fmtdoes not format markdown here; markdown is hand-wrapped.cp -aprocedure, but movingarchive-*.dbalone silently yields fewer rows.CSRF ran before MaxBodySize, so the CSRF middleware parsed the form body before any cap applied and an oversized request was read in full before being rejected. MaxBodySize is now the first middleware in all four route groups that parse forms, ahead of CSRF and RequireAuth. An oversize request therefore gets 413 without the handler running and without state changing, including the password-change route. Note the ordering trade: an unauthenticated client now receives 413 rather than an auth redirect on /user/{username}/password.Integration-level review of
origin/main..origin/next@543005c. FAIL —needs-rework.Gate is genuinely green:
docker build --no-cache-filter=lint,builderexit 0 with#15 RUN make fmt-checkDONE 1.1s,#16 RUN make lint->0 issues.DONE 59.3s,#23 RUN make testDONE 66.1s (5.8k lines, zero(cached)markers, 9okpackage lines); hostmake checkexit 0.mainis an ancestor ofnext— fast-forwardable, no conflict. The Gitea status on543005csays "Successful in 5s", i.e. a cache replay per #119 — the runs above are the evidence, not the badge. No Claude/Anthropic strings or attribution trailers in the tree or in any of the 15 commit messages; inclusive terminology clean.Findings, ranked.
1. The receiver rate limit does not bound abuse of the receiver —
internal/middleware/ratelimit.go:258+internal/server/routes.go:174.The limiter keys on (client IP,
httprate.KeyByEndpoint), andKeyByEndpointis literallyr.URL.Path. The chi pattern/webhook/{uuid}matches any single path segment, so a client mints a fresh 120/min bucket for every distinct string it invents —/webhook/a,/webhook/b, ... — and the aggregate rate against the route is unlimited. Every one of those requests is admitted intoHandleWebhook, which logs an INFO line carrying the attacker-supplied value (internal/handlers/webhook.go:42) and performs a DB lookup (internal/handlers/webhook.go:48) before returning 404. One IP therefore drives unbounded DB queries and unbounded log volume against the single unauthenticated, internet-exposed endpoint. That is precisely the propertyinternal/config/config.go:38-41claims ("bounding abuse of the one unauthenticated, internet-exposed endpoint") and that #64 was opened to obtain. Neither unit review could see this: #64 landed the per-endpoint key against anextwithout the trusted-proxy work, and #88 rewrote only the IP half of the same key. Acceptable: a second, coarser limiter over the whole/webhook/*group keyed on client IP alone at a higher ceiling, or resolve the entrypoint first and bucket on the resolved entrypoint ID rather than the raw path — so an unresolvable path cannot buy a bucket.2.
RETENTION_SWEEP_INTERVALis not range-validated and now panics two loops —internal/config/config.go:219,349.envDurationaccepts any parseable duration, including0sand-1h. That value reachestime.NewTickerininternal/database/retention.go:122and — new in this diff —internal/delivery/archive_sweeper.go:130.time.NewTickerpanics on a non-positive duration, in goroutines with norecover, soRETENTION_SWEEP_INTERVAL=0logs "Configuration loaded" and then aborts the process: a delayed crash, not a loud startup failure.PORTandRECEIVER_RATE_LIMITboth received lower-bound checks under #80; this variable did not, and #89 doubled its blast radius. The missing check predates this branch, but the second consumer does not. Acceptable: reject non-positiveRETENTION_SWEEP_INTERVALinloadFromEnv, asenvPositiveIntdoes.3.
RETENTION_SWEEP_INTERVALis missing from the README environment table —README.md:86-98.The table gained
MAINTENANCE_MODE,SESSION_IDLE_TIMEOUT,RECEIVER_RATE_LIMITandTRUSTED_PROXIES, but two later sections useRETENTION_SWEEP_INTERVALas if it were documented there (README.md:174cites it as a fail-loud example;README.md:642makes it the archive sweeper's interval). An operator reading the table cannot discover the variable that controls both sweepers.4.
TODO.mddoes not record five of the fifteen landed units.The landing commit is titled "Update TODO.md for the completed 1.0.0 milestone" and its Status paragraph asserts
next"holds the completed 1.0.0 milestone", but Completed Steps has no entry for #64, #79, #90, #113 or #118 — including both credential-exposure fixes.REPO_POLICIES.mdrequires the TODO be updated meticulously.5.
TODO.md:118dropped "Manual event redelivery from the web UI" from Future Steps, but it is not implemented.No redelivery handler, route or template exists anywhere in
internal/ortemplates/. The only surviving trace is the planned API endpoint (TODO.md:128,README.md:919). MeanwhileREADME.md:263still advertises it in the present tense as a core capability: "Replay — Stored events can be manually redelivered for debugging or testing, without requiring the original sender to fire the webhook again." Tagging 1.0.0 would ship a README promising a feature the binary lacks, with the tracking item deleted in the same branch. Acceptable: restore the Future Steps entry (or file an issue) and softenREADME.md:263to planned.6. Minor —
static/js/app.js:2:console.log("Webhooker loaded");ships in the production asset. Pre-existing, but it is debug scaffolding in a 1.0.0 artifact.7. Minor —
templates/sources_list.html:3: the #57 copy change to{{define "title"}}Webhooks - Webhooker{{end}}is inert, because page title blocks never render (#117). Harmless, but the commit does not achieve what it claims for that file.8. PR state. Still a draft with
WIP:in the title, and the body's "Still open in the milestone" list names #57, #88 and #118, all of which landed; the milestone is now 0 open. Body and draft state need correcting before this can merge.Probes worth recording, all of which passed for the right reason:
securecookie's max-age onnextis still gorilla's 30-day default whilestore.Options.MaxAgeis 7 days (#108), butsession.expirednow rejects anything pastcreated_at + 7dserver-side and treats a session carrying no timestamps as expired, so the skew is not reachable. #132 remains worth landing on its own merits.forwardedClientAddrhand-checked for hop splitting, empty hops, trailing commas, multi-value headers and the 64-hop cutoff falling back to the peer address: correct in each case.sweepWriterFor/releaseSweepWriter/ theevictedflag are all evaluated under the registry lock, and the sweep never creates an archive file.maxBodyShiftremains live viainternal/handlers/webhook.go:17, so removing the handler-levelMaxBytesReadercalls did not disturb the receiver's own 1 MB cap.Disclosure: 14 of 15 commits end with
(closes #N);Update TODO.md for the completed 1.0.0 milestonecloses no issue, which I judged acceptable for a housekeeping commit rather than a finding. The buffered-rendering interaction could not be assessed on this branch —renderTemplatestill writes straight to theResponseWriter(internal/handlers/handlers.go:233); #131 has not landed here.The receiver limiter keyed on the request path, and /webhook/{uuid} matches any single segment, so a client minted a fresh bucket per invented path and had unlimited aggregate rate against the only unauthenticated endpoint. An outer limiter keyed on the client address alone now bounds that, chained in front of the unchanged per-entrypoint limiter. Its rejections log at DEBUG without the path, and the README states what each limit does and does not bound.Independent integration review of
origin/main..origin/next@339548d(20 commits, 65 files). FAIL —needs-rework.Gate, run by me on
339548din a fresh clone:docker build --no-cache-filter=lint,builderexit 0 in 210s —#15 RUN make fmt-checkDONE 0.8s,#16 RUN make lint->0 issues.DONE 59.0s,#23 RUN make testDONE 67.1s (9okpackage lines, zero(cached)markers),#24 RUN make buildDONE 51.0s; hostmake checkexit 0.mainis an ancestor ofnext, fast-forwardable, no conflict. No Claude/Anthropic strings or attribution trailers in the tree or in any of the 20 commit messages; inclusive terminology clean; no non-testTODO/FIXME, commented-out blocks or debug code reachable from the production binary; external references hash-pinned. The five findings of #111 (comment) are all confirmed fixed and are not re-reported. None of the findings below duplicate the deferred set.1. In the default configuration any unauthenticated client on the internet can lock the admin out of the web UI indefinitely —
internal/middleware/ratelimit.go:151-172+:219-226, undocumented atREADME.md:1120-1122.LoginRateLimitis 5 POSTs/minute per bucket, and since #88 the bucket key isclientKey: the connection's own peer address unless that peer is inTRUSTED_PROXIES, which defaults to empty.README.md:79-80and the prod TLS story require a TLS-terminating reverse proxy, so in the default deployment every login POST from every client keys on the proxy's address and shares one 5/minute bucket. An attacker sending 5 login POSTs per minute — about 0.08 req/s, from anywhere — keeps that bucket permanently full, and the operator's own login POST gets 429 forever. There is no second administrative path.PasswordChangeRateLimitis the same 5/min shared bucket.This is not the trusted-proxy default being wrong: empty is correct. What is wrong is that the consequence is nowhere stated and one place states its opposite.
README.md:1120-1122still advertises the login limiter as "per-IP sliding-window rate limiter on the login endpoint (5 POST attempts per minute per IP)", which is false in the default deployment.README.md:891-895does disclose the shared bucket but characterises it as "the safe direction to be wrong in" — true for the receiver, exactly inverted for login, where a shared bucket converts a per-attacker throttle into a global lockout. The#### Trusted proxiessection atREADME.md:101-146, the one an operator actually reads while configuring, presents the empty default as having no downside at all.Acceptable: state in
README.md:101-146that a production deployment behind a reverse proxy MUST setTRUSTED_PROXIES, and that leaving it unset makes the login and password-change limits a single global bucket that any remote client can hold full; and correctREADME.md:1120-1122so it does not claim per-IP. A startup WARN whenTRUSTED_PROXIESis empty andWEBHOOKER_ENVIRONMENT=prodwould be better still.2. The Docker section describes a build that does not exist, and contradicts the section 15 lines below it —
README.md:1131-1135.It says the Dockerfile is a two-stage build whose "Builder stage (Debian-based
golang:1.24) — installs golangci-lint, downloads dependencies, copies source, runsmake check". The actualDockerfileis three stages:lintongolangci/golangci-lint:v2.12.2runningmake fmt-checkandmake lint(Dockerfile:5,26-27),builderongolang:1.26.1-bookwormrunningmake testandmake build(Dockerfile:32,49-50), then the runtime stage. Wrong stage count, wrong Go version, wrong image, wrong commands — andREADME.md:1147-1159, added by #119 in this branch, correctly describes "both check stages" and the four separate targets, so the two paragraphs contradict each other. The described single-builder shape is also the oneREPO_POLICIES.md:102-106forbids. Acceptable: rewriteREADME.md:1131-1135to the three stages actually in the tree.3. The API table is false about the only public endpoint —
README.md:917, andREADME.md:514.README.md:917listsANY | /webhook/{uuid} | Webhook receiver endpoint (accepts all methods).internal/handlers/webhook.go:24-33answers 405 withAllow: POSTto everything except POST, and has since #20.README.md:514compounds it, documentingEvent.methodas "HTTP method (POST, PUT, etc.)" when the column can only ever holdPOST. Pre-existing onmain, but this is the first line an integrator reads about the endpoint the release exists to serve, and the documentation-accuracy pass in this branch (0e397b3) did not catch it. Acceptable:POSTand "accepts POST only; other methods get 405", and fix themethodfield description.4. The gate status on the PR head is a cache replay and is not evidence —
339548d.check / check (push)on339548dreads success, description "Successful in 6s".339548dis docs-only so the fingerprint mechanism replays legitimately — but the two commits under it,0e397b3and95161c7(the latter is the receiver aggregate rate-limit fix this PR was failed for last round), both carry statusskipped, "Superseded by a newer commit; never tested", and Gitea's combined-status API returns"state":"success"for each of them. The newest status onnextthat describes an executed run isbe57609, "Successful in 2m52s", whose tree predates both of those code commits. So no commit status on this branch evidences an executed check of the current tree, andTODO.md:28-31's claim thatnext"is verified green both by CI and by cache-defeated container runs" is unsupported on the CI half. The tree itself is fine — my cache-defeated build above is the actual evidence — so this is an evidence defect, not a code defect, but it means the repaired gate still turns "never tested" into a green rollup whenever a run is superseded. Distinct from #147, which coversscript/cibuilddrift and the hardcoded context string. Acceptable: push a no-op commit or re-run so the head carries a real run before tagging, and soften or substantiate theTODO.mdclaim. Note for the merge: ifnextis fast-forwarded rather than squashed,maininherits commit339548dunchanged, somain's check will also be a seconds-long replay — expected, not a regression.5. The session-expiry documentation is orphaned inside the trusted-proxy section —
README.md:101vsREADME.md:148-166.#66 put the two-clock session prose directly under
### Configuration. #88 then inserted#### Trusted proxiesabove it without a closing heading, soSESSION_IDLE_TIMEOUT, the 7-day absolute cap and the 10% refresh lag now sit inside a subsection aboutX-Forwarded-Forand are unreachable from the table of contents. Visible only once both units are combined. Acceptable: a#### Sessionsheading atREADME.md:148.6. Minor —
internal/config/config.go:455-467: the startup summary omitssessionIdleTimeout. Every other value parsed inloadFromEnvis logged, including the three added in this milestone.SESSION_IDLE_TIMEOUTis the one variable where a valid setting silently turns off a security control (config.go:99, non-positive disables idle expiry), so it is the one most worth showing back to the operator.7. Minor —
TODO.md:193lists "Password change and reset flow" under Future Steps whileTODO.md:23-24records the admin password change flow (#65) as already landed onmain.Probes that passed, recorded because they were the likely failure sites: the two chained receiver limiters compose correctly (aggregate outer, per-entrypoint inner, saturating multiply at
ratelimit.go:333-339, no unbounded key growth since the outer limiter gates the inner one's key space);forwardedClientAddrhand-checked for hop splitting, empty hops, multi-value headers and the 64-hop cutoff falling back to the peer; the codec max-age / idle-timeout skew stays unreachable becausesession.expiredtreats a session with no timestamps as expired; the three background loops each root atcontext.Background()and eachOnStopcancels then waits, with no shared state between the reaper's per-webhook DBs and the sweeper's archive files;renderTemplatestill streams to theResponseWriter(internal/handlers/handlers.go:233), so the buffered-rendering interaction does not exist on this branch; the.ci-fingerprintbarrier is correctly excluded from.dockerignoreand does not disturb the policy-mandated lint stage; no raw config blob reaches any template.Disclosures: two of the 20 commit titles carry no
(closes #N)—543005cand339548d, bothTODO.mdhousekeeping — which I judged acceptable, consistent with the prior review. I could not independently determine whether run 167 or 168 completed their docker build before being cancelled, so finding 4 is stated as unsupported evidence rather than as a proven untested tree.internal/delivery/target_config_view.go:115-118renders the HTTP target URL unmasked; that is #115 and is your call, not a finding here.Milestone triage, recorded once. The 1.0.0 milestone stood at 22 closed / 1 open when I picked this up. It now has seven open, because six issues had been filed with an explicit "not milestoned 1.0.0" note that no longer held, and one is new. Every move is argued on its own issue; the summary:
Moved in, reversing the filer's original call
successfor a commit nothing ever ran is a false green that branch protection or a release script will read as fact.REPO_POLICIES.mdrules the release claims to follow. "Pre-existing" describes when it started, not whether it belongs in the tag. Since merged.docker stop./64bypasses completely, with no spoofing and nothing to detect.Opened during the work, and milestoned
Deliberately left out, though tempting: #107 (the strongest pure correctness defect in the backlog, but not unauthenticated-reachable), #117, #128, #127, #166, #168, #169, and the test-hygiene and tooling set (#93, #94, #99, #101, #103, #120, #154). The test applied throughout: can an unauthenticated attacker on the public internet exploit it, or does it make the shipped artifact wrong, untrustworthy or unverifiable?
Two decisions I took rather than parking, both reversible by closing the PR: #115 (mask unconditionally) and #125 (option 1,
/64). Only #150 is genuinely sneak's, and it is assigned to him with a corrected recommendation — the option originally recommended there does not fix the reported attack, since flooding the operator's own predictable username still locks them out.r.FormValue falls back to the query string, so POST /source/{id}/targets?url=<secret> created a working target from a value carried on the request line — where proxy logs, browser history and Referer all record it. Every form read is now r.PostFormValue, including the login password and both password-change fields, which had the same defect in a more acute form. The Sentry leg needed more than the query string. sentryhttp attaches the whole request to the scope, and ApplyToEvent copies the teed body into Request.Data with no SendDefaultPII guard — so reading every field from the body only pointed every credential this change protects at the one field the first revision did not scrub. Body and query are now redacted, Cookies and Env cleared, and Headers reduced to an allowlist, because the SDK's own filter removes four names and would otherwise ship X-Csrf-Token and the shared secrets senders put on the receiver route. Also adds json:"-" to Target.Config, APIKey.Key and Setting.Value — TargetView is the masking barrier for the HTML path only, and the first handler to marshal a model would serialise a bearer token or the session encryption key. Independently reviewed three times. The second review found the Data leak and proved it with a scratch module; the third disproved the PR's own claim that BeforeSend gets no request, so the README now records that redacting unconditionally is a deliberate choice rather than a limitation — which is what makes #179 cheap to fix.WIP: Milestone: delivery lifecycle, retention, config and session hardeningto Milestone 1.0.0: internet-facing readinessHeld as
WIP:and unassigned — not merge-ready. sneak, 2026-08-20: "webhooker must be to mvp before tagging 1.0. it is prerelease now and must be usable in low volume prod by me before a 1.0".The 1.0.0 gate is no longer "milestone empty" but "deployable and usable in low-volume production". A code-level deployability audit is running now: first-run admin bootstrap, the blast radius of having no edit forms for targets and entrypoints (#127), inbound authentication on the receive endpoint (#67), retention end to end, and a live run pushing a webhook through to a real sink.
Its blockers get milestoned to 1.0.0. This PR reopens for merge when they are closed.
The receiver had no inbound authentication of any kind: /webhook/{uuid} was mounted behind a rate limiter alone, so the only thing protecting an entrypoint was the secrecy of a v4 UUID in a URL path. Inbound headers are forwarded almost verbatim to the target, so anyone who learned the URL also chose the headers the downstream service received. Adds an optional per-entrypoint secret with two schemes: github (X-Hub-Signature-256, HMAC-SHA256 hex over the raw body) and gitlab (X-Gitlab-Token, a plain shared token). Comparison is constant-time, the HMAC is computed over the raw body before any parsing, and rejection happens before persistence -- an unauthenticated request creates no event row. An entrypoint with no secret behaves exactly as before, including every row that predates this change. The scheme's credential header is stripped from the header map before it is marshalled into Event.Headers, so the GitLab token reaches neither the event store nor any delivery target. SchemeInfo.HeaderIsDigest defaults to false meaning strip, so a scheme added later is protected unless its header is positively declared a digest.There was no redelivery path anywhere: once a delivery exhausted max_retries it was failed permanently, even though the event body is durably stored. Storing an event and being unable to re-send it defeats the reason it is stored, and the ordinary case is a destination that was down longer than the backoff ladder. Adds POST /source/{sourceID}/deliveries/{deliveryID}/replay, inside the authenticated group so it inherits MaxBodySize, CSRF, NoCache and RequireAuth. Replay creates a NEW pending delivery against the target's CURRENT config and hands it to the engine through the same notifier the receiver uses, so it runs the normal path with the retry ladder, the SSRF-guarded transport and the circuit breaker. The original delivery's rows are never touched, and the stored event body is re-sent, never the recorded response. Replay is refused, with a distinct message, for a non-terminal delivery, a deleted target, a deactivated target, and when an earlier replay of the same event and target is still in flight. Bounded by a per-client rate limit and by that in-flight check. The new delivery row is written with Omit(clause.Associations) and with neither Event nor Target populated, so it cannot upsert a targets row into the per-webhook event database (#206). Counted by webhooker_delivery_replays_total on the existing target_type label. A replay also moves the ordinary attempt, outcome and duration series, because it is a real delivery.clawbot referenced this pull request2026-08-24 03:34:45 +02:00