Restart recovery and the pending sweep skipped a pending delivery
whose target was missing from the batch's target map, and the sweep
did so again every minute for the life of the database. A miss now
asks loadTarget: no row fails the delivery with a recorded reason,
through the ownership-gated function the retrying paths already use,
renamed failMissingTarget with its log line and reason text made to
fit both statuses. Any other error leaves the delivery pending,
because the map is also empty when its query failed.
Model: opus-5-5
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
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
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
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)
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)
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
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)
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)
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>
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).
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.
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.
check / check (push) Superseded by a newer commit; never tested
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.
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.
check / check (push) Superseded by a newer commit; never tested
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.
clawbot
sneak