1.0.0 milestone: next into main #321

Merged
sneak merged 16 commits from next into main 2026-09-29 11:04:58 +02:00
Collaborator

Milestone branch next for 1.0.0 (https://git.eeqj.de/sneak/webhooker/milestone/9), ready to merge into main. It completes the upaas readiness work (#323).

On the branch since main (1.0.0b1):

  • README: a "Running under upaas" section (#323); the UUID-is-the-credential rule (#301); max_retries counts all attempts (#316); make check needs make bootstrap once (#282).
  • WEBHOOKER_ENVIRONMENT defaults to prod (#307).
  • Delivery fixes: no second send of a delivery restart recovery already sent (#299); a half-open circuit breaker no longer re-queues held deliveries in a tight loop (#306); a pending delivery whose target was deleted is failed with a reason instead of stranded (#293); one Content-Type per delivery (#246).
  • Security: Azure WireServer (168.63.129.16) is blocked by default and can be reopened through ALLOWED_EGRESS_CIDRS (#245); a request that panics before responding no longer hands the client a Set-Cookie (#193).
  • Storage: event database indexes (#314); each event database opened once under racing callers (#291); archive databases closed at a clean stop, so each is one file again (#280); the data directory's mode set in one place (#288).

To deploy: a normal image rebuild. With WEBHOOKER_ENVIRONMENT unset, an instance stops sending Access-Control-Allow-Origin: *. Indexes are added on each event database's first open; no manual step. webhooker_delivery_retries_total no longer counts a breaker holding back a delivery already retrying.

For upaas: prod exists, cut from main at 251cb3d. After this merge, a reviewed main to prod PR brings these changes there.

The 1.0 tag waits on your production run, separately from this merge.

Model: fable-5-1 (earlier text); opus-5-5 (update)

Milestone branch `next` for 1.0.0 (https://git.eeqj.de/sneak/webhooker/milestone/9), ready to merge into `main`. It completes the upaas readiness work (https://git.eeqj.de/sneak/webhooker/issues/323). On the branch since `main` (1.0.0b1): - README: a "Running under upaas" section (https://git.eeqj.de/sneak/webhooker/issues/323); the UUID-is-the-credential rule (https://git.eeqj.de/sneak/webhooker/issues/301); `max_retries` counts all attempts (https://git.eeqj.de/sneak/webhooker/issues/316); `make check` needs `make bootstrap` once (https://git.eeqj.de/sneak/webhooker/issues/282). - `WEBHOOKER_ENVIRONMENT` defaults to `prod` (https://git.eeqj.de/sneak/webhooker/issues/307). - Delivery fixes: no second send of a delivery restart recovery already sent (https://git.eeqj.de/sneak/webhooker/issues/299); a half-open circuit breaker no longer re-queues held deliveries in a tight loop (https://git.eeqj.de/sneak/webhooker/issues/306); a pending delivery whose target was deleted is failed with a reason instead of stranded (https://git.eeqj.de/sneak/webhooker/issues/293); one `Content-Type` per delivery (https://git.eeqj.de/sneak/webhooker/issues/246). - Security: Azure WireServer (`168.63.129.16`) is blocked by default and can be reopened through `ALLOWED_EGRESS_CIDRS` (https://git.eeqj.de/sneak/webhooker/issues/245); a request that panics before responding no longer hands the client a `Set-Cookie` (https://git.eeqj.de/sneak/webhooker/issues/193). - Storage: event database indexes (https://git.eeqj.de/sneak/webhooker/issues/314); each event database opened once under racing callers (https://git.eeqj.de/sneak/webhooker/issues/291); archive databases closed at a clean stop, so each is one file again (https://git.eeqj.de/sneak/webhooker/issues/280); the data directory's mode set in one place (https://git.eeqj.de/sneak/webhooker/issues/288). To deploy: a normal image rebuild. With `WEBHOOKER_ENVIRONMENT` unset, an instance stops sending `Access-Control-Allow-Origin: *`. Indexes are added on each event database's first open; no manual step. `webhooker_delivery_retries_total` no longer counts a breaker holding back a delivery already `retrying`. For upaas: `prod` exists, cut from `main` at `251cb3d`. After this merge, a reviewed `main` to `prod` PR brings these changes there. The 1.0 tag waits on your production run, separately from this merge. Model: fable-5-1 (earlier text); opus-5-5 (update)
clawbot self-assigned this 2026-09-21 14:56:25 +02:00
clawbot added 2 commits 2026-09-21 14:56:25 +02:00
Closes #301. Docs-only apart from one test comment; no behaviour change.

The receiver has authenticated on the entrypoint UUID alone since inbound signature verification was removed in #279. The README described that as the current state. It did not say it is the decision, which leaves a future contributor free to propose HMAC as an improvement rather than as a reversal.

What changed:

- `## The entrypoint URL is the authentication secret` now states the rule: the v4 UUID at `/webhook/{uuid}` is the credential and the only one; no shared secret, HMAC signature, bearer token or second factor will be added, including as defence in depth. It names the removal that settled it, and it says explicitly that signature headers a sender sends anyway are stored and forwarded but never checked — the previous text left that ambiguous.
- The same section carries the two consequences an operator has to act on: the URL is a capability, so keep it out of logs, tickets and screenshots; and rotation means minting a new entrypoint, not changing a key.
- It also handles the case the rule will next be argued from: a sender that only supports signed payloads to a well-known URL is a constraint on that integration, to be raised on its own terms, not grounds to reintroduce shared secrets.
- The rule is reachable without scrolling 1,100 lines: a pointer in the intro, a new first bullet under Authentication (which previously listed the web UI, the API and `/metrics` and said nothing about the receiver at all), and a sharpened bullet under Security.

Stale language found and corrected: one, in `internal/delivery/redirect_test.go`. Its comment justified same-origin header retention partly by "the inbound signature the receiver verifies" — in this repo's vocabulary "the receiver" is `/webhook/{uuid}`, which verifies nothing. The endpoint that verifies it is the delivery target's, and the comment now says so.

Two places that read like stale signing language were checked and left alone as accurate: `internal/delivery/redirect.go` and `internal/server/sentry.go` describe signature headers senders put on the receiver route, which do arrive and are forwarded — neither claims webhooker checks them.

`REPO_POLICIES.md` was deliberately not touched. It is the cross-project policy document synced from `sneak/prompts` and carries `last_modified` front matter for that purpose, so a webhooker-specific carve-out does not belong in it. Worth knowing: its hardening section ends "if a standard security hardening measure exists for HTTP services and is not listed here, it is still expected. When in doubt, harden" — that is the sentence a future HMAC proposal will cite, and only the README now answers it.

`TODO.md` is untouched per its own Workflow section (issue branches do not touch it).

Co-authored-by: sneak <sneak@sneak.berlin>
Reviewed-on: #302
Co-authored-by: clawbot <clawbot@noreply.example.org>
Co-committed-by: clawbot <clawbot@noreply.example.org>
Two packages each declared the 0o750 mode for DATA_DIR and both created the directory. internal/datadir now exports DirPerm as the single definition, and internal/database uses it in both places it creates the directory. The value is unchanged, so existing deployments see no permission change. datadir owns it because guarding and creating DATA_DIR is that package's whole purpose and it imports nothing that would form a cycle.

Model: opus-4-8 (implementation and review); fable-5-1 (merge)
clawbot added 1 commit 2026-09-21 18:33:11 +02:00
The help text under the field on both target forms and the max_retries rows in the README now say the number is the total number of delivery attempts: 0 is a single attempt with no retries and no circuit breaker, and N is N attempts in total. The delivery code already worked this way; only the wording was wrong, so an operator wanting one try plus two retries would have entered 2 instead of 3. A UI copy test renders both forms and pins the wording. Delivery behaviour is unchanged.

Model: opus-4-8 (implementation); fable-5-1 (merge)
clawbot added 1 commit 2026-09-28 12:30:34 +02:00
Adds a "Running under upaas" section to the README: add no port
mapping, since upaas publishes mapped ports on every host interface,
and put the app on the reverse proxy's Docker network instead; one
data volume at /var/lib/webhooker, created owned by UID 1000 before
the first deploy; WEBHOOKER_ENVIRONMENT and TRUSTED_PROXIES; the
health check upaas reads 60 seconds after a deploy; and where the
first-run admin password appears and how to reset it.

upaas bind-mounts a host directory it never creates, and one made by
root stops the container at its data directory lock. The documented
creation step removes that; the image is unchanged.

Model: opus-5-5
clawbot added 1 commit 2026-09-28 12:47:34 +02:00
Default WEBHOOKER_ENVIRONMENT to prod (closes #307)
check / check (push) Successful in 3m14s
237f131367
An unset WEBHOOKER_ENVIRONMENT now means prod, not dev. The only
thing dev still changes is CORS, which then answers every origin with
Access-Control-Allow-Origin: *, so an operator who forgets the
variable is no longer silently permissive; dev must be set
explicitly. Cookie Secure and CSRF strictness follow each request's
transport and are unaffected.

The README, comments and tests no longer describe dev as the default:
the deployment checklist asks only that the environment is not dev,
the Docker and nginx examples drop the now-redundant setting, and the
TRUSTED_PROXIES warning gives its real reason for firing in every
environment.

Model: opus-4-8 (implementation); opus-5-5 (rework)
clawbot added 1 commit 2026-09-28 14:13:24 +02:00
The per-webhook event databases had no secondary indexes, so startup
recovery, the retry and pending sweeps, the queue-depth sampler, the
event log and retention each read whole tables. Indexes declared in
the GORM model tags now serve them, and AutoMigrate adds them to new
and existing databases alike.

Each index also covers deleted_at: GORM adds deleted_at IS NULL to
these queries, and SQLite, with no table statistics, otherwise
prefers the existing deleted_at index. A test checks SQLite's plan
for each statement as GORM builds it.

Rule suppressed: lll on the three event-tier model structs, whose
struct tags cannot wrap.
The resubmitted_from_id scan is left to
#325.

Model: opus-4-8 (implementation); opus-5-5 (rework)
clawbot removed their assignment 2026-09-28 14:17:53 +02:00
sneak was assigned by clawbot 2026-09-28 14:17:53 +02:00
clawbot added the merge-ready label 2026-09-28 14:17:53 +02:00
sneak added 1 commit 2026-09-29 03:13:59 +02:00
Merge branch 'main' into next
check / check (push) Successful in 9s
aeeeca5ea1
clawbot added 1 commit 2026-09-29 04:12:16 +02:00
Restart recovery could find a just-written delivery pending, send it
and release it before the receiver's Notify queued the same delivery.
Notify's claim then succeeded on the released id, and the worker sent
it again because the new-task path never read the delivery's row.

Before sending a new task the worker now reads the delivery's status
by primary key and skips the task unless the row still says pending,
as the retry path already does for retrying. Nothing else can change
the row while the worker owns the delivery. A row left pending by a
failed bookkeeping write is still sent again.

loadRetryDelivery is renamed loadDelivery now that both paths use it.

Model: opus-5-5
clawbot added 1 commit 2026-09-29 04:30:16 +02:00
The third-party browser assets are not committed, so on a fresh clone
make check fails in the tests until make bootstrap (or make assets) has
fetched them. The Entrypoints section now says so up front, and why the
check does not fetch them itself: it must not change files in the repo.

Model: opus-5-5
clawbot added 1 commit 2026-09-29 04:37:32 +02:00
GetDB opened the database on a cache miss and then tried to cache it,
so callers racing on a webhook's first use could each open the file,
and the losers closed their copies. On a new file the parallel opens
also create its tables at the same time, and one caller can fail with
"table already exists".

A mutex now covers the open: GetDB looks in the cache again under it,
then opens and caches. DeleteDB and CloseAll take the same mutex, so
neither runs while an open is under way. Reading an already cached
database takes no lock.

The new test starts many callers on one webhook at once and checks
that exactly one open happened.

Model: opus-5-5
clawbot added 1 commit 2026-09-29 05:48:22 +02:00
While the breaker was half-open, Allow refused every delivery but the
probe and CooldownRemaining returned zero, so each queued task for the
target went straight back onto the retry channel and rewrote its status
on every pass until the probe finished.

CooldownRemaining now returns the whole cooldown while half-open, so a
refused delivery waits that long. A refused delivery already at
retrying is not written again, so the retry counter now moves only
when a refusal moves a delivery into retrying.

Model: opus-5-5
clawbot added 1 commit 2026-09-29 06:48:21 +02:00
Restart recovery and the pending sweep skipped a pending delivery
whose target was missing from the batch's target map, every minute,
for the life of the database. A miss now asks loadTarget: no row
fails the delivery terminally with a recorded reason; any other error
leaves it pending, since the map is also empty when its query failed;
a target found there is used.

The failure goes through the ownership-gated function the retrying
paths already used, now failMissingTarget. Once it owns the delivery
it re-reads the row and fails it only if the status is unchanged, so
a delivery sent and settled in between is left alone.

Model: opus-5-5
clawbot added 1 commit 2026-09-29 07:11:57 +02:00
Send Content-Type once on a delivery (closes #246)
check / check (push) Successful in 3m20s
d4f4ddf51f
A delivery set Content-Type from the event's ContentType and then
added the inbound Content-Type from the event's stored headers, so a
target could receive two values. The inbound Content-Type is no
longer forwarded from the stored headers; the receiver already saves
it as the event's ContentType.

Which value is sent is now stated at applyRequestHeaders: a
Content-Type configured on the target, otherwise the event's
ContentType, otherwise none. A configured one still survives a
cross-origin 307/308 with its body.

Model: opus-5-5
clawbot added 1 commit 2026-09-29 08:30:26 +02:00
The engine cached archive writers and never closed them at shutdown,
so after a clean stop an archive's rows could sit in its -wal while
the .db held no table. The engine's stop hook now evicts every cached
writer once its workers have returned, the same way deleting a webhook
does, so a clean stop leaves each archive as one file and a late write
is refused. If the workers do not return within the stop budget, the
writers are left open as a kill would leave them: closing would wait
on a write in progress, and a still-running worker would open new
ones.

The README no longer says archives keep their sidecars across a clean
stop.

Model: opus-5-5
clawbot added 1 commit 2026-09-29 10:22:08 +02:00
Add 168.63.129.16 (Azure WireServer) to blockedNetworks, the default
blocklist, not alwaysBlockedNetworks: it is public unicast, so an
operator who lists it in ALLOWED_EGRESS_CIDRS can reach it again. The
refusal message, the allowlist startup warning, the README and the
comments no longer call every blocked address private/reserved, and
no longer claim the allowlist cannot open any metadata endpoint.

Sources:
- https://learn.microsoft.com/en-us/azure/virtual-network/what-is-ip-address-168-63-129-16
- https://learn.microsoft.com/en-us/azure/virtual-machines/metadata-security-protocol/overview

Deviation: 147.75.207.243 (Equinix Metal) is not added; Equinix
documents only a hostname, and the service was sunset on 2026-06-30.

Model: opus-5-5
clawbot added 1 commit 2026-09-29 10:30:29 +02:00
Drop Set-Cookie from the recovered 500 (closes #193)
check / check (push) Successful in 3m35s
f0adeafde3
When a handler sets a cookie and then panics before sending
anything, the recover middleware now deletes Set-Cookie before
writing its 500, so a request that failed never hands the client
a credential. Every other header, Location included, is left as
http.Error leaves it, matching chi's Recoverer. A response that
was already sent is untouched.

Tests cover the uncommitted case (no cookie, Location kept) and
assert the cookie still reaches the client when the response was
committed before the panic.

Model: opus-5-5
sneak merged commit 9cf9cdd8eb into main 2026-09-29 11:04:58 +02:00
sneak deleted branch next 2026-09-29 11:04:58 +02:00
sneak referenced this issue from a commit 2026-09-29 11:46:51 +02:00
sneak referenced this issue from a commit 2026-09-29 11:48:16 +02:00
Sign in to join this conversation.
No Reviewers
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/webhooker#321