sneak/webhooker
1
0
Fork 0
Code Issues 11 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

Compare commits

base: sneak/webhooker:main
Branches Tags
sneak/webhooker:main sneak/webhooker:next sneak/webhooker:prod sneak/webhooker:issue-70-log-target
..
compare: sneak/webhooker:918533d897e66ab18a6b3acd31b44a0d89ed046d
Branches Tags
sneak/webhooker:next sneak/webhooker:main sneak/webhooker:prod sneak/webhooker:issue-70-log-target
1 Commits
main .. 918533d897
Author SHA1 Message Date
sneak 918533d897 Close the two remaining delivery terminal-state gaps (closes #107)
check / check (push) Has been cancelled
Details 
A delivery could reach a bad end without the engine recording why, and
a retrying delivery could fail to reach an end at all.

An unknown target type marked the delivery failed and wrote no
DeliveryResult, so the event log showed "failed" with no attempts and
the only account of why was one line in the server log. It now records
a result naming the type before failing the delivery.

A deleted target left its retrying deliveries stranded. Both recovery
and the sweep began with a scoped loadTarget, which cannot see a soft
deleted row, so both logged and returned: the delivery stayed retrying
for the life of the database while the sweep repeated the same error
every minute. Both now terminalise it with a recorded reason.

Deleting a target also did not stop deliveries to it. A scheduled
retry is a time.AfterFunc holding the target's configuration from when
the chain began, and nothing on that path read the target row, so the
timer kept firing and kept sending to the destination the operator had
removed for the rest of the backoff chain; terminalising in recovery
and the sweep alone would only have caught it after a restart.
processRetryTask now confirms the target still exists before it
attempts, and abandons the chain when it does not.

Only a target confirmed gone stops anything. A lookup that fails for
any other reason is the main database being unreadable, which is
transient, and every path leaves the delivery exactly as it was rather
than failing it.

The reason text comes from one Unscoped lookup confined to these
terminal paths, because a soft deleted row is what distinguishes a
target the operator deleted from an id that never named one. The
engine's normal target loading stays scoped, or deleting a target
would stop nothing.

Terminal writes reached from recovery keep going through the existing
retainIdle ownership gate; the retry path writes directly, as a
target's own Deliver does, because the worker already holds that
delivery. Nothing was added to either sweep dispatch arm.

Retry fixtures that drove processRetryTask for a target id with no row
in the main database now create one. That state is not reachable in
service: the handler reads the target to build the task.
 2026-08-24 02:30:53 +00:00
52 changed files with 327 additions and 2831 deletions
Expand all files Collapse all files
Dockerfile
+2 -9
View File
@@ -88,9 +88,7 @@ RUN CGO_ENABLED=1 make build VERSION="$VERSION" GO_LDFLAGS='-extldflags "-static
# alpine:3.21, 2026-03-17
FROM alpine:3.21@sha256:c3f8e73fdb79deaebaa2037150150191b9dcbfba68b4a46d70103204c53f4709
# su-exec 0.2-r3 (Alpine 3.21), 2026-09-29: the entrypoint runs the app
# as webhooker with it.
RUN apk --no-cache add ca-certificates su-exec=0.2-r3
RUN apk --no-cache add ca-certificates
# Create non-root user
RUN addgroup -g 1000 -S webhooker && \
@@ -101,17 +99,13 @@ WORKDIR /app
# Copy binary from builder
COPY --from=builder /build/bin/webhooker /app/webhooker
# Not under /app, which belongs to webhooker: this script runs as root.
COPY deploy/docker-entrypoint.sh /usr/local/bin/docker-entrypoint.sh
# Create data directory for all SQLite databases (main app DB +
# per-webhook event DBs). DATA_DIR defaults to /var/lib/webhooker.
RUN mkdir -p /var/lib/webhooker
RUN chown -R webhooker:webhooker /app /var/lib/webhooker
# No USER: the entrypoint starts as root to make the data directory
# webhooker's, then runs the app as webhooker.
USER webhooker
EXPOSE 8080
@@ -130,5 +124,4 @@ ENV BIND_ADDRESS=0.0.0.0
HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \
CMD wget --no-verbose --tries=1 --spider http://localhost:8080/.well-known/healthcheck || exit 1
ENTRYPOINT ["/usr/local/bin/docker-entrypoint.sh"]
CMD ["/app/webhooker"]
README.md
+110 -274
View File
@@ -7,13 +7,6 @@ services, durably stores them, and delivers them to configured targets
with retry support, logging, and observability. Category: infrastructure
/ web service. License: MIT.
Each entrypoint is a version 4 UUID served at `/webhook/{uuid}`, and
that UUID is the entrypoint's only credential. webhooker does not use
shared secrets, HMAC signatures or token headers on the receiver, and
will not add them — read
[The entrypoint URL is the authentication secret](#the-entrypoint-url-is-the-authentication-secret)
before deploying one.
## Getting Started
### Prerequisites
@@ -44,9 +37,9 @@ make bootstrap
# Run all checks (test, lint, format check)
make check
# Run the server from the clone. DATA_DIR defaults to
# /var/lib/webhooker in every environment, so set it (in .env or the
# shell) to a writable directory.
# Run in development mode. DATA_DIR defaults to /var/lib/webhooker in
# every environment, so set it (in .env or the shell) to a writable
# directory when running from a clone.
DATA_DIR=./data make dev
# Build Docker image
@@ -92,8 +85,7 @@ them at once. A variable already present in the real environment wins
over the file's value for the same name.
The environment is selected by setting `WEBHOOKER_ENVIRONMENT` to `dev`
or `prod` (default: `prod`; `dev` must be set explicitly). The setting
controls exactly one behavior:
or `prod` (default: `dev`). The setting controls exactly one behavior:
| Behavior | `dev` | `prod` |
| -------- | ----------------------- | ---------------- |
@@ -135,7 +127,7 @@ TTY detection, and security headers are always applied.
| Variable | Description | Default |
| ----------------------- | ----------------------------------- | -------- |
| `WEBHOOKER_ENVIRONMENT` | `dev` or `prod` | `prod` |
| `WEBHOOKER_ENVIRONMENT` | `dev` or `prod` | `dev` |
| `PORT` | HTTP listen port | `8080` |
| `BIND_ADDRESS` | IP address the HTTP listener binds. Loopback by default, so the cleartext listener is not published on every interface. The Docker image ships `0.0.0.0` instead. See [Bind address](#bind-address) | `127.0.0.1` (image: `0.0.0.0`) |
| `DATA_DIR` | Directory for all SQLite databases | `/var/lib/webhooker` |
@@ -157,11 +149,6 @@ private and reserved ranges — RFC 1918, loopback, CGNAT, link-local and
the rest — are refused, which stops a target from being used to make
webhooker probe the network it sits in.
Besides the private and reserved ranges, the default blocklist refuses
public cloud metadata addresses: currently only `168.63.129.16`, Azure's
WireServer, which serves an Azure VM its credentials. Because it is a
public address, listing it in `ALLOWED_EGRESS_CIDRS` reopens it.
That default is also inconvenient for the thing webhooker is mostly
for: taking a public webhook and forwarding it to something on your own
network. A container on the same Docker network, a box on `10.x`, a
@@ -200,16 +187,15 @@ Two things this setting cannot do:
the list is always an allowlist; an empty list (the default) means
every private and reserved range stays refused. Note that
`0.0.0.0/0` gets you most of the way there anyway, per above.
- **It cannot open link-local, or a cloud metadata endpoint at a
non-public address that discloses credentials or user data.** An
address is on the list below when it is not a public address and both
of these hold: the provider fixes it, so it cannot collide with
anything you run; and reaching it hands out credentials, user data or
bootstrap material. Those stay blocked no matter what you list,
including when you list them outright or list a supernet such as
`0.0.0.0/0`, `::/0`, `fd00::/8` or `100.64.0.0/10`. Treat this as best
effort rather than a guarantee — it is a hand-maintained list and the
caveat below the table applies:
- **It cannot open link-local, or a cloud metadata endpoint that
discloses credentials or user data.** An address is on the list below
when both of these hold: the provider fixes it, so it cannot collide
with anything you run; and reaching it hands out credentials, user
data or bootstrap material. Those stay blocked no matter what you
list, including when you list them outright or list a supernet such
as `0.0.0.0/0`, `::/0`, `fd00::/8` or `100.64.0.0/10`. Treat this as
best effort rather than a guarantee — it is a hand-maintained list
and the caveat below the table applies:
| Blocked unconditionally | What it is |
| ----------------------- | ---------- |
@@ -248,8 +234,7 @@ Two things this setting cannot do:
encodings, which the default blocklist does not match. A publicly
routable metadata address is not listed here, because nothing on this
list can be reopened and blocking one that way would leave you no
escape hatch at all; Azure's `168.63.129.16` is refused by the default
blocklist instead, as described above.
escape hatch at all.
This list is not exhaustive of every cloud's metadata address — if
yours is not here, do not allowlist the block that contains it.
@@ -407,9 +392,10 @@ the bucket is. See [Rate Limiting](#rate-limiting).
The remedy is to set `TRUSTED_PROXIES` to your reverse proxy's
address, which restores per-client buckets. webhooker logs a warning
at startup whenever `TRUSTED_PROXIES` is empty, in every environment,
because behind a proxy every client shares one bucket in `dev` and
`prod` alike. The warning is informational when nothing proxies to the
at startup whenever `TRUSTED_PROXIES` is empty, in every environment —
not only when `WEBHOOKER_ENVIRONMENT=prod`, because that variable
defaults to `dev` and an operator who never set it is precisely the
one at risk. The warning is informational when nothing proxies to the
process: with no proxy in front, the peer address is the client's own
and the buckets are already per-client. See
[Rate Limiting](#rate-limiting) for what each limit shares.
@@ -538,12 +524,6 @@ its Argon2id hash. There is no second account and no forgot-password
flow, so the banner and the reset command below are the only two ways
in.
A start that finds no `webhooker.db` in `DATA_DIR` also logs
`created a new, empty database` at `WARN`, with the file's path,
shortly before the banner. On a deployment that has run before, that
line means `DATA_DIR` was empty, most often because its volume is not
mounted.
#### Recovering a lost admin password
`webhooker resetpw` sets an existing account's password from the
@@ -558,9 +538,8 @@ printf '%s' "$NEW_PASSWORD" | \
DATA_DIR=/var/lib/webhooker webhooker resetpw admin
```
In a container it is the same binary. The image's `CMD` is
`/app/webhooker`, and a command given to `docker run` replaces all of
it, so the whole command has to be given:
In a container it is the same binary, which the image sets as `CMD`
rather than `ENTRYPOINT`, so the whole command has to be given:
```bash
docker run --rm -v webhooker-data:/var/lib/webhooker \
@@ -652,6 +631,7 @@ decision:
docker run -d \
-p 127.0.0.1:8080:8080 \
-v /path/to/data:/var/lib/webhooker \
-e WEBHOOKER_ENVIRONMENT=prod \
-e BIND_ADDRESS=0.0.0.0 \
webhooker:latest
```
@@ -697,82 +677,14 @@ those three values rather than trusting the figure. Measured at 65s on
Docker 29.7.2.) A container `unhealthy` with `connection refused` in
its health log, or a published port that resets connections, is this.
The app runs as a non-root user (`webhooker`, UID 1000), exposes port
8080, and includes a health check against `/.well-known/healthcheck`.
The `/var/lib/webhooker` volume holds all SQLite databases: the main
application database (`webhooker.db`), the per-webhook event databases
(`events-{uuid}.db`), and any archive databases written by `database`
targets (`archive-{uuid}.db`). Mount this as a persistent volume to
preserve data across container restarts.
**The container sets its data directory's owner and mode itself
before the app starts**, so a host directory can be mounted as it is,
whoever owns it. The image's `ENTRYPOINT`,
`deploy/docker-entrypoint.sh`, starts as root, creates `DATA_DIR` if
it is missing, gives the directory and anything in it that belongs to
another user to `webhooker`, sets the directory to `0750`, and only
then runs the app as `webhooker`. Started with `--user`, it changes
nothing and runs the app as that user.
**The file modes are not yours to set, and do not depend on the
directory.** `webhooker.db` holds target configuration in plaintext —
bearer tokens, API keys, Slack webhook URLs — along with the session
encryption key, so webhooker creates every SQLite file it owns `0600`:
each database and both of its `-wal` and `-shm` sidecars, across all
three tiers. Files an earlier build left `0644` are tightened when
they are opened. The directory's `0750` is defence in depth — it stops
other local users listing the directory and learning your webhook
UUIDs from the `events-{uuid}.db` filenames — not the barrier
protecting the credentials.
### Running under upaas
[upaas](https://git.eeqj.de/sneak/upaas) builds the image from this
repository's `Dockerfile` and runs it. The app needs:
- **Network and port:** add no port mapping in upaas. upaas publishes
every mapped port on all interfaces of the host
([upaas issue 113](https://git.eeqj.de/sneak/upaas/issues/113)),
which would put the plain-HTTP admin UI and receiver there. Instead,
set the app's Docker Network in upaas to your reverse proxy's Docker
network; the proxy then reaches the app at `upaas-` followed by the
app name, port `8080`. Leave `PORT` unset: the image's health check
probes `8080`.
- **Volume:** one host directory mounted at `/var/lib/webhooker`.
- **Environment variables:**
- `WEBHOOKER_ENVIRONMENT=prod`
- `TRUSTED_PROXIES`: your reverse proxy's address on that Docker
network. The `remoteIP` field of the `http request` log line for a
request that came through the proxy shows it; the health check's
own lines show `::1`. See [Trusted proxies](#trusted-proxies).
- Leave `BIND_ADDRESS` and `DATA_DIR` unset: the image sets
`BIND_ADDRESS` to `0.0.0.0`, and `DATA_DIR` defaults to
`/var/lib/webhooker`.
- Everything else is optional; see [Configuration](#configuration).
- **Health check:** the image's own, which requests
`/.well-known/healthcheck`. upaas reads the container's health 60
seconds after a deploy and marks the deploy failed unless it is
`healthy`.
- **First run:** the first start prints the `admin` password once, in
the banner described under [The admin account](#the-admin-account),
to the container's log. upaas names the container `upaas-` followed
by the app name, so for an app named `webhooker`:
```bash
docker logs upaas-webhooker
```
If the password is lost, stop the container, set a new password with
the app's own image and volume, and start it again (see
[Recovering a lost admin password](#recovering-a-lost-admin-password)):
```bash
docker stop upaas-webhooker
docker run --rm --volumes-from upaas-webhooker \
"$(docker inspect -f '{{.Image}}' upaas-webhooker)" \
/app/webhooker resetpw -generate admin
docker start upaas-webhooker
```
The container runs as a non-root user (`webhooker`, UID 1000), exposes
port 8080, and includes a health check against
`/.well-known/healthcheck`. The `/var/lib/webhooker` volume holds all
SQLite databases: the main application database (`webhooker.db`), the
per-webhook event databases (`events-{uuid}.db`), and any archive
databases written by `database` targets (`archive-{uuid}.db`). Mount
this as a persistent volume to preserve data across container
restarts.
## Deployment behind a reverse proxy
@@ -795,18 +707,17 @@ reports.
serves the admin login form and the unauthenticated receiver with
no TLS at all, and the proxy in front of it changes nothing about
that.
2. **Make sure the environment is not `dev` (leave
`WEBHOOKER_ENVIRONMENT` unset or set it to `prod`), and make sure
the proxy sends `X-Forwarded-Proto`.** These are two requirements,
not one. The environment setting decides CORS and nothing else:
2. **Set `WEBHOOKER_ENVIRONMENT=prod`, and make sure the proxy sends
`X-Forwarded-Proto`.** These are two requirements, not one. The
environment setting decides CORS and nothing else: the default
`dev` answers every origin with `Access-Control-Allow-Origin: *`
(without credentials), which a server-rendered production
deployment has no use for, and `prod` — the default — disables it.
Cookie `Secure` and the strict Origin/Referer mode are **not** tied
to it — they are decided per request from the transport, which
behind a proxy means the `X-Forwarded-Proto` header. The block below
sets it; without it every request is read as plaintext and cookies
ship without `Secure`. See [Configuration](#configuration).
deployment has no use for. Cookie `Secure` and the strict
Origin/Referer mode are **not** tied to it — they are decided per
request from the transport, which behind a proxy means the
`X-Forwarded-Proto` header. The block below sets it; without it
every request is read as plaintext and cookies ship without
`Secure`. See [Configuration](#configuration).
3. **Set `TRUSTED_PROXIES` to the proxy's address.** Unset, every rate
limiter keys on the connecting peer, which behind a proxy is the
proxy on every request: all clients collapse into one global bucket
@@ -898,6 +809,7 @@ sent — `$scheme` above does.
With that block, webhooker's environment is:
```sh
WEBHOOKER_ENVIRONMENT=prod
BIND_ADDRESS=127.0.0.1 # the default; stated here to be explicit
TRUSTED_PROXIES=127.0.0.1
```
@@ -952,10 +864,15 @@ scratch file**: it holds committed transactions that are not yet in the
have no readable schema at all. `-shm` is regenerable, but there is no
reason to separate the two — copy the directory and you have them.
A clean shutdown closes every database, which checkpoints and removes
its sidecars; a killed or crashed instance leaves them, and they must be
carried with the `.db`. An archive the service has not opened since a
crash keeps that crash's sidecars, even across a later clean stop.
A clean shutdown closes `webhooker.db` and every `events-*.db`, which
checkpoints and removes their sidecars; a killed or crashed instance
leaves them, and they must be carried with the `.db`. **Archive
databases are different**: their handle is not closed at shutdown, so
`archive-*.db-wal` and `-shm` normally survive a clean stop and the
`-wal` can hold every row the archive has. Measured on a stopped
instance: `archive-….db` 4096 bytes with no table, its `-wal` 157 KB
holding all 8 archived events. Copying `DATA_DIR` in full is what makes
this a non-issue; copying `.db` files out of it by name is not.
Configuration is **not** in `DATA_DIR` — it comes from the environment
and from a `.env` file read out of the process working directory. Back
@@ -997,12 +914,12 @@ done
`.backup` reads through the WAL and writes a single consistent file with
no sidecars of its own, so the destination is complete as it stands.
Two caveats. First, the runtime image is `alpine:3.21` with only
`ca-certificates` and `su-exec` added — the `sqlite3` CLI is **not** in
it, so run this on the host against the volume path, or from a
throwaway container that mounts the volume. Second, each file is
captured at its own instant, so a webhook created or an event delivered
between two files being copied lands in one and not the other. If you
need the whole set coherent as of a single moment, stop the service.
`ca-certificates` added — the `sqlite3` CLI is **not** in it, so run
this on the host against the volume path, or from a throwaway container
that mounts the volume. Second, each file is captured at its own
instant, so a webhook created or an event delivered between two files
being copied lands in one and not the other. If you need the whole set
coherent as of a single moment, stop the service.
Note that `sqlite3 <db> .dump` is **not** one of these procedures: it is
an export, it holds a read transaction open for as long as it runs, and
@@ -1030,9 +947,10 @@ The file becomes self-contained again when the handle closes, which
happens on the next write past the debounce window, when the connection
pool retires the idle connection (about a minute after the last write),
or at the idle archive sweep — measured, the same file was a complete
20 KB `.db` with no sidecars about a minute after its last write. A
clean stop closes it too. So either move `archive-{uuid}.db` together
with any `-wal`/`-shm` beside it, or wait until there are none.
20 KB `.db` with no sidecars about a minute after its last write.
Shutdown is **not** on that list: the archive handle is not closed when
the service stops. So either move `archive-{uuid}.db` together with any
`-wal`/`-shm` beside it, or wait until there are none.
### Restore
@@ -1051,16 +969,28 @@ with any `-wal`/`-shm` beside it, or wait until there are none.
They are part of the database, and dropping a `-wal` silently
discards every transaction it still holds. An `.backup` set will not
contain any: it writes a single consolidated file per database. A
stop-and-copy set normally has none, because a clean stop closes
every database and checkpoints its sidecars away; the exception is an
archive not opened since a crash. A copy salvaged from a crashed
instance has them for everything, and needs all of them.
stop-and-copy set has none for `webhooker.db` or the `events-*.db`,
because a clean stop closes those and checkpoints their sidecars
away — but it will normally have them for `archive-*.db`, whose
handle stays open across shutdown, and those carry the archive's
rows. A copy salvaged from a crashed instance has them for
everything, and needs all of them.
4. Start the service. The container gives the directory and the
restored files to the `webhooker` user before the app starts,
whoever restored them (see
[Running with Docker](#running-with-docker)). `AutoMigrate` runs
against each restored database as it is opened.
4. **Fix ownership.** The container runs as the non-root `webhooker`
user, UID 1000 / GID 1000. Restored files must be owned by (or
writable by) that UID, and so must the directory itself — SQLite
creates the `-wal` and `-shm` sidecars beside the database, so a
writable file inside a directory it cannot write is not enough:
```bash
chown -R 1000:1000 /path/to/data
```
Restoring as `root` on the host and forgetting this step is the
usual way a restore fails.
5. Start the service. `AutoMigrate` runs against each restored database
as it is opened.
### Upgrades
@@ -1181,38 +1111,14 @@ backups at rest and restrict who can read them.
## The entrypoint URL is the authentication secret
**The entrypoint UUID is the credential, and it is the only one.**
webhooker mints a version 4 UUID per entrypoint and serves it at
`/webhook/{uuid}`. Possession of that URL is the authentication:
anyone who holds it can submit events to the entrypoint, and the
receiver verifies nothing else about the sender.
The receiver verifies nothing about an inbound request. The UUID in an
entrypoint's URL is its credential: anyone who holds that URL can
submit events to it, and the receiver checks nothing else about the
sender. Treat an entrypoint URL the way you would treat an API token.
There is no shared secret, no HMAC signature, no bearer token and no
second factor on the receiver, and none will be added. This was
considered and rejected; the implementation that existed was removed
in [PR #279](https://git.eeqj.de/sneak/webhooker/pulls/279), closing
[issue #67](https://git.eeqj.de/sneak/webhooker/issues/67) and
[issue #241](https://git.eeqj.de/sneak/webhooker/issues/241). A
proposal to reintroduce any of them — including as "defence in depth"
alongside the UUID — is answered by this section. Inbound signature
headers a sender sends anyway (`X-Hub-Signature` and its
per-provider equivalents) are stored and forwarded as ordinary
headers; nothing checks them.
What that means for an operator:
- **The URL is a capability, so treat it as a secret.** Keep it out of
logs, ticket bodies, chat messages and screenshots. Anyone who reads
it anywhere can post events as that sender.
- **Rotating means minting a new entrypoint, not changing a key.**
There is no way to rotate the UUID in place. To retire one, delete
the entrypoint (or deactivate it, which answers `410`) and create a
new one, then point the sender at the new URL.
- **A sender that cannot be given a secret URL is a constraint on that
integration, not a reason to change this.** If a service only
supports signed payloads to a well-known URL, raise it as its own
problem — pick a different integration path, or accept that it
cannot be used. It is not grounds to reintroduce shared secrets.
There is no way to rotate the UUID in place. To retire one, delete the
entrypoint (or deactivate it, which answers `410`) and create a new
one, then point the sender at the new URL.
## Entrypoints
@@ -1222,15 +1128,8 @@ standard: normalized scripts in `script/` are the entrypoints for the
development workflow. Ten of the Makefile's seventeen targets are thin
shims that call them; `build`, `run`, `dev`, `deps`, `clean`, `css` and
`version` are inline commands with no script behind them, though
`build` and `version` both take their value from `script/version`.
`make check` needs the third-party browser assets in `static/`, which
are not committed, so run `make bootstrap` (or just `make assets`) once
after cloning. Without them the tests fail with a message naming that
remedy. `make check` does not fetch them itself because it must not
change any files in the repo.
We provide:
`build` and `version` both take their value from `script/version`. We
provide:
- `script/bootstrap` — install all dependencies (idempotent)
- `script/setup` — make a fresh clone ready for development
@@ -1297,16 +1196,6 @@ webhooker solves this by acting as a durable intermediary:
backoff. Every delivery attempt is logged with status codes, response
bodies, and timing.
**That guarantee is at-least-once, not exactly-once.** When a send
reaches its target but the write recording that outcome fails, the
delivery is deliberately left in a recoverable state rather than
marked done — losing a delivery is the worse failure — so the
pending sweep picks it up about fifteen minutes later, or the next
restart does, and the target receives a payload it already got.
webhooker adds no delivery identifier of its own to an outbound
request, so **make your receiver idempotent** against whatever the
payload itself carries.
3. **Observability** — Full request/response logging for every webhook
received and every delivery attempted. Prometheus metrics expose
volume, latency, and error rates. The web UI provides real-time
@@ -1539,7 +1428,7 @@ events should be forwarded.
| `type` | TargetType | One of: `http`, `slack`, `database`, `log` |
| `active` | boolean | Whether deliveries are enabled (default: true) |
| `config` | JSON text | Type-specific configuration |
| `max_retries` | integer | Total delivery attempts for `http` and `slack` targets, not retries on top of the first: 0 is a single fire-and-forget attempt with no retries and no circuit breaker, and a value of N makes N attempts in all, with exponential backoff and a per-target circuit breaker. Ignored by `database` and `log` targets |
| `max_retries` | integer | Maximum retry attempts for `http` and `slack` targets (0 = fire-and-forget, >0 = retries with backoff and a circuit breaker). Ignored by `database` and `log` targets |
| `max_queue_size` | integer | Stored and shown on the target's detail view, but not enforced anywhere yet: nothing in the delivery engine consults it. Queue depth is set by the two fixed 10,000-entry channels |
**Relations:** Belongs to Webhook. Has many Deliveries.
@@ -1547,12 +1436,12 @@ events should be forwarded.
**Target types:**
- **`http`** — Forward the event as an HTTP POST to a configured URL.
`max_retries` is the total number of delivery attempts, not retries on
top of the first: when `max_retries` is 0 (the default), the target
operates in fire-and-forget mode, a single attempt with no retries and
no circuit breaker; a value of N makes up to N attempts in all,
retrying failed deliveries with exponential backoff and protecting them
with a per-target circuit breaker.
Behavior depends on `max_retries`: when `max_retries` is 0 (the
default), the target operates in fire-and-forget mode — a single
attempt with no retries and no circuit breaker. When `max_retries` is
greater than 0, failed deliveries are retried with exponential backoff
up to `max_retries` attempts, protected by a per-target circuit
breaker.
- **`slack`** — Post the event as a formatted message to a
Slack-compatible incoming webhook URL (`webhookUrl` in `config`). It
is built on the same HTTP core as `http` and honours `max_retries`
@@ -1732,28 +1621,6 @@ retries) is individually logged for full observability.
**Relations:** Belongs to Delivery.
#### Event-tier indexes
These indexes on the per-webhook event databases are declared in the model
tags, so `AutoMigrate` creates them on a fresh and on an existing database:
| Table | Columns | Serves |
| ------------------ | --------------------------- | ------ |
| `deliveries` | `status`, `deleted_at` | Startup recovery, the retry and pending sweeps every 60 seconds and the queue-depth sampler every 30 seconds, which select deliveries by status |
| `deliveries` | `event_id`, `deleted_at` | The event log, which loads each event's deliveries, and retention, which selects and deletes the deliveries of expired events |
| `delivery_results` | `delivery_id`, `deleted_at` | The event log, which loads the attempts of a page's deliveries, and retention, which deletes the attempts of expired events |
| `events` | `deleted_at`, `created_at` | Retention, which selects expired events by age |
| `events` | `created_at` | Retention's delete of the expired events themselves |
GORM's soft delete adds `deleted_at IS NULL` to these queries; retention's
deletes leave it out, but their lookups of expired rows keep it. SQLite keeps
no statistics on these tables, and without them it rates the `deleted_at`
index, which every live row matches, above an index on a column matched
against several values or compared with `<`. So every index but the last also
covers `deleted_at`. It comes second, so that retention's deletes can use the
index without it, except in `events`, where `created_at` is compared with `<`
and SQLite narrows by a `<` only on the last column it uses.
#### Common Fields
Every entity except `Setting` includes these fields from `BaseModel`.
@@ -1775,11 +1642,6 @@ webhooker uses **separate SQLite database files**: a main application
database for configuration data and per-webhook databases for event
storage. All database files live in the `DATA_DIR` directory.
Every one of them is created `0600`, and so is each `-wal` and `-shm`
sidecar. See
[Running with Docker](#running-with-docker) for what that does and
does not protect.
**Main Application Database** (`{DATA_DIR}/webhooker.db`) — stores
configuration and application state:
@@ -2017,7 +1879,7 @@ rescans the database anyway).
| ----------- | -------- |
| **Closed** | Normal operation. Deliveries flow through. Consecutive failures are counted. |
| **Open** | Target appears down. Deliveries are skipped and rescheduled for after the cooldown. |
| **Half-Open** | Cooldown expired. One probe delivery is allowed to test if the target has recovered. Other deliveries are rescheduled for one whole cooldown later. |
| **Half-Open** | Cooldown expired. One probe delivery is allowed to test if the target has recovered. |
**Transitions:**
@@ -2055,9 +1917,7 @@ operations), and log targets (stdout) do not use circuit breakers.
When a circuit is open and a new delivery arrives, the engine marks the
delivery as `retrying` and schedules a retry timer for after the
remaining cooldown period. This ensures no deliveries are lost — they're
just delayed until the target is healthy again. A delivery already in
`retrying` keeps that status without another database write each time
the breaker turns it away.
just delayed until the target is healthy again.
### Metrics
@@ -2071,7 +1931,7 @@ arriving and being stored, they are just not getting anywhere.
| Metric | Type | Meaning |
| ------ | ---- | ------- |
| `webhooker_events_received_total` | counter | Events received and durably stored. Compare against the delivery counters on one dashboard |
| `webhooker_delivery_attempts_total` | counter | Delivery attempts actually dispatched to a target. A delivery a circuit breaker refused is not one: it is counted as a retry instead, but only when the refusal moves it into `retrying` |
| `webhooker_delivery_attempts_total` | counter | Delivery attempts actually dispatched to a target. A delivery an open circuit breaker refused is not one: it is counted as a retry instead |
| `webhooker_deliveries_succeeded_total` | counter | Deliveries that reached `delivered` |
| `webhooker_deliveries_failed_total` | counter | Deliveries that failed terminally and will not be retried |
| `webhooker_delivery_retries_total` | counter | Deliveries put back into `retrying` |
@@ -2688,7 +2548,7 @@ abuse limit later; they are tracked as future work.
| ------ | --------------------------- | ----------- |
| `GET` | `/` | Root redirect, 303 (authenticated → `/sources`, unauthenticated → `/pages/login`) |
| `GET` | `/.well-known/healthcheck` | Health check (JSON: `status`, `now`, `uptimeSeconds`, `uptimeHuman`, `version`, `appname`, `maintenanceMode`) |
| `GET`, `HEAD` | `/s/*` | Static file serving (embedded CSS, JS). `GET` and `HEAD` only — `POST`, `PUT`, `PATCH`, `DELETE`, `OPTIONS`, `TRACE` and `CONNECT` are answered `405 Method Not Allowed` with `Allow: GET, HEAD`. Any other method (such as `PROPFIND`) is refused by chi before it reaches this route, and gets `405` without an `Allow` header. Pinned by `TestStaticServesOnlyGetAndHead` |
| any | `/s/*` | Static file serving (embedded CSS, JS). Mounted for every method, not just `GET`/`HEAD`: chi's `Mount` registers all methods and `http.FileServer` special-cases only `HEAD` (by omitting the body), so a `POST` or `DELETE` to an asset is answered `200` with the file. Pinned by `TestStaticServesEveryMethod` |
| `POST` | `/webhook/{uuid}` | Webhook receiver endpoint. `POST` only — every other method is answered `405 Method Not Allowed` with `Allow: POST`. Rate limited (see [Rate Limiting](#rate-limiting)) |
#### Authentication Endpoints
@@ -2713,15 +2573,12 @@ abuse limit later; they are tracked as future work.
| `POST` | `/source/{id}/edit` | Edit webhook submission |
| `POST` | `/source/{id}/delete` | Delete webhook |
| `GET` | `/source/{id}/logs` | Webhook event logs |
| `GET` | `/source/{id}/logs/{eventID}/body` | Download an event's full stored body. The log page renders each body only up to its cap, so this is the only route that serves a whole one; it is offered wherever a body is shown truncated |
| `POST` | `/source/{id}/deliveries/{deliveryID}/replay` | Replay a finished delivery: creates a new delivery for the same event against the target's current configuration (30 per minute per bucket, then `429`) |
| `POST` | `/source/{id}/events/{eventID}/resubmit` | Resubmit a stored event: creates a new event copying it and fans that out to every currently active target (30 per minute per bucket, then `429`) |
| `POST` | `/source/{id}/entrypoints` | Add entrypoint to webhook |
| `POST` | `/source/{id}/entrypoints/{entrypointID}/delete` | Delete an entrypoint |
| `POST` | `/source/{id}/entrypoints/{entrypointID}/toggle` | Enable or disable an entrypoint |
| `POST` | `/source/{id}/targets` | Add target to webhook |
| `GET` | `/source/{id}/targets/{targetID}/edit` | Edit target form. The one page that renders a target's destination URL and header values in full, rather than masked |
| `POST` | `/source/{id}/targets/{targetID}/edit` | Edit target submission |
| `POST` | `/source/{id}/targets/{targetID}/delete` | Delete a target |
| `POST` | `/source/{id}/targets/{targetID}/toggle` | Enable or disable a target |
@@ -2760,8 +2617,6 @@ webhooker/
├── internal/
│ ├── banner/
│ │ └── banner.go # Ruled block for the one credential shown in the clear
│ ├── ciscript/
│ │ └── doc.go # Tests for the CI shell scripts in script/; no runtime code
│ ├── resetpw/
│ │ └── resetpw.go # `webhooker resetpw`: set an account's password, stopped deployments only
│ ├── config/
@@ -2831,17 +2686,13 @@ webhooker/
│ │ ├── ratelimit.go # Per-IP rate limiting middleware (go-chi/httprate)
│ │ ├── loginguard.go # Login failure counters and the Argon2id verification semaphore
│ │ └── testing.go # NewForTest: Middleware without the fx lifecycle
│ ├── reqtls/
│ │ └── reqtls.go # IsTLS: the one TLS predicate, r.TLS or X-Forwarded-Proto
│ ├── server/
│ │ ├── server.go # Server struct, fx lifecycle, signal handling
│ │ ├── http.go # HTTP server setup with timeouts
│ │ └── routes.go # All route definitions
│ ├── session/
│ │ ├── session.go # Cookie-based session management
│ │ └── testing.go # NewForTest: Session without the fx lifecycle
│ └── versionscript/
│ └── doc.go # Tests for script/version and the build files that use it
│ └── session/
│ ├── session.go # Cookie-based session management
│ └── testing.go # NewForTest: Session without the fx lifecycle
├── static/
│ ├── static.go # //go:embed directive
│ ├── css/input.css # Tailwind input, source for tailwind.css (make css)
@@ -2954,10 +2805,6 @@ check, see [The login endpoint](#the-login-endpoint).
### Authentication
- **Webhook receiver:** the entrypoint UUID in the URL, and nothing
else. No shared secret, no HMAC signature, no token header, and none
will be added — see
[The entrypoint URL is the authentication secret](#the-entrypoint-url-is-the-authentication-secret).
- **Web UI:** Cookie-based sessions using gorilla/sessions with
encrypted cookies. Sessions are configured with HttpOnly, SameSite
Lax, and Secure whenever the request is on TLS — the flag follows the
@@ -2997,8 +2844,7 @@ check, see [The login endpoint](#the-login-endpoint).
mode
- **The entrypoint URL is the receiver's only credential.** Nothing
about an inbound request is verified; possession of the UUID
authorises submission, and no shared secret or signature check will
be added alongside it (see
authorises submission (see
[The entrypoint URL is the authentication secret](#the-entrypoint-url-is-the-authentication-secret))
- **SSRF prevention** for HTTP delivery targets: private/reserved IP
ranges (RFC 1918, loopback, link-local, cloud metadata) are blocked
@@ -3038,11 +2884,7 @@ check, see [The login endpoint](#the-login-endpoint).
- Prometheus metrics behind basic auth
- Static assets embedded in binary (no filesystem access needed at
runtime)
- The app runs as the non-root `webhooker` user (UID 1000) in the
container. The image sets no `USER`, so these run as root: the
`ENTRYPOINT` script, which sets the data directory's owner and mode
before the app starts; the image's health check; and `docker exec`,
unless given `--user`
- Container runs as non-root user (UID 1000)
- GORM soft deletes on every entity that carries `BaseModel`, which is
all of them but `Setting` (data preserved for audit)
@@ -3058,8 +2900,7 @@ each hook. The order, read off the fx stop-hook log:
3. `server` — the HTTP drain, bounded separately by
`server.ShutdownTimeout` (**3 seconds**), then a Sentry flush if
`SENTRY_DSN` is set
4. `delivery.Engine` — waits for its workers, then closes the archive
databases
4. `delivery.Engine`
5. `healthcheck`
6. `WebhookDBManager`
7. the database close
@@ -3169,13 +3010,10 @@ version is fixed independently of the compiler's:
`GO_LDFLAGS`, so neither can drop the `-X` that stamps the version.
The version arrives as the `VERSION` build arg, since the context
has no `.git` (see [Version stamping](#version-stamping)).
3. **Runtime stage** (`alpine:3.21`) — copies the static binary and
`deploy/docker-entrypoint.sh`, creates the `/var/lib/webhooker`
directory for all SQLite databases, exposes port 8080, and includes
a health check against `/.well-known/healthcheck`. It sets no
`USER`: the `ENTRYPOINT` script starts as root, sets the data
directory's owner and mode, and runs the app as the non-root
`webhooker` user (UID 1000) through `su-exec`.
3. **Runtime stage** (`alpine:3.21`) — copies the static binary,
creates the `/var/lib/webhooker` directory for all SQLite databases,
runs as the non-root `webhooker` user (UID 1000), exposes port 8080,
and includes a health check against `/.well-known/healthcheck`.
The lint stage invokes `golangci-lint` directly rather than `make lint`:
it is already the pinned linter image, and `make lint` builds
@@ -3254,5 +3092,3 @@ MIT
## Author
[@sneak](https://sneak.berlin)
TODO.md
+16 -25
View File
@@ -18,27 +18,18 @@ Issue branches do NOT touch this file — the manager maintains it on
# Status
The milestone (https://git.eeqj.de/sneak/webhooker/milestone/9) is the
authoritative list, and the only place to read a count or a state of
play from. This file records where the project is, not what is in
flight: a sentence whose truth depends on a branch being unmerged is
wrong the moment it merges, and this file has been wrong that way
before.
1.0.0 is open, with work remaining. The milestone
(https://git.eeqj.de/sneak/webhooker/milestone/9) is the authoritative
list, and the only place to read a count or a state of play from. This
file records where the project is, not what is in flight: a sentence
whose truth depends on a branch being unmerged is wrong the moment it
merges, and this file has been wrong that way before.
The durability defect that held the tag has landed
(https://git.eeqj.de/sneak/webhooker/issues/256, commit `8d64259`).
Every SQLite handle opens with WAL journaling and a busy timeout, a
bookkeeping write that fails leaves its delivery in a recoverable
state rather than a lying one, and recovery skips a delivery that
already has a successful result row. Final pre-tag verification
exercised it and confirmed it holds. Whatever the milestone still
shows open is what remains before `v1.0.0`.
Delivery is at-least-once by design, not by accident: a send whose
result row does not land is attempted again, so a receiver can see a
duplicate. That is deliberate — the alternative is a silent lost
delivery — and the README says so under Rationale. It is not a defect
to re-file.
The tag is held on a durability defect
(https://git.eeqj.de/sneak/webhooker/issues/256): a concurrent reader
of a per-webhook event database strands delivered webhooks at
`pending`, and the next restart re-delivers them. That issue gates
`v1.0.0`, and is where the fix's own state is tracked.
One caveat on reading a green check: a docs-only commit deliberately
replays from the layer cache
@@ -48,11 +39,11 @@ commit invalidates the `COPY` layer and genuinely executes.
# Next Step
Clear the rest of the open 1.0.0 milestone
(https://git.eeqj.de/sneak/webhooker/milestone/9) and tag `v1.0.0`.
Merging `next` into `main` is a separate act from tagging and waits on
neither of those: `next` is kept mergeable at all times, which is the
point of the branch.
Land https://git.eeqj.de/sneak/webhooker/issues/256, then clear the
rest of the open 1.0.0 milestone and tag `v1.0.0`. Merging `next` into
`main` is a separate act from tagging and waits on neither of those:
`next` is kept mergeable at all times, which is the point of the
branch.
# Completed Steps
deploy/docker-entrypoint.sh
-22
View File
@@ -1,22 +0,0 @@
#!/bin/sh
# deploy/docker-entrypoint.sh: the image's ENTRYPOINT. A bind-mounted
# data directory keeps its owner from the host, often root, and the app
# could not write to it. Started as root, this creates DATA_DIR if
# needed, gives it and everything in it to webhooker, sets its mode, and
# runs the command as webhooker, so the app never runs as root. Started
# as another user, it only runs the command.
set -eu
main() {
if [ "$(id -u)" != 0 ]; then
exec "$@"
fi
dir="${DATA_DIR:-/var/lib/webhooker}"
mkdir -p "$dir"
find "$dir" ! -user webhooker -exec chown -h webhooker:webhooker {} +
chmod 750 "$dir"
exec su-exec webhooker "$@"
}
main "$@"
internal/config/config.go
+16 -19
View File
@@ -192,10 +192,9 @@ type Config struct {
// alwaysBlockedNetworks stays blocked no matter what is listed
// here. That set is link-local plus the cloud metadata
// endpoints outside it that disclose credentials or user data
// at a provider-fixed, non-public address; it is not
// exhaustive of every cloud's metadata address. See
// alwaysBlockedNetworks for the authoritative list and the
// criterion it is built from.
// at a provider-fixed address; it is not exhaustive of every
// cloud's metadata address. See alwaysBlockedNetworks for the
// authoritative list and the criterion it is built from.
AllowedEgressCIDRs []netip.Prefix
params *ConfigParams
@@ -586,14 +585,12 @@ func resolveMetricsAuth() (string, string, error) {
)
}
// resolveEnvironment reads WEBHOOKER_ENVIRONMENT, defaulting to prod
// when it is unset so a deployment that forgets the variable is not
// silently permissive; dev must be set explicitly. It rejects
// unrecognised values.
// resolveEnvironment reads WEBHOOKER_ENVIRONMENT, defaulting to
// dev, and rejects unrecognised values.
func resolveEnvironment() (string, error) {
environment := os.Getenv("WEBHOOKER_ENVIRONMENT")
if environment == "" {
environment = EnvironmentProd
environment = EnvironmentDev
}
if environment != EnvironmentDev &&
@@ -747,14 +744,12 @@ func (c *Config) warnEgressAllowlist(log *slog.Logger) {
log.Warn(
"ALLOWED_EGRESS_CIDRS lets delivery targets reach these "+
"otherwise-blocked networks. Anyone who can create a "+
"delivery target can now make this process issue "+
"requests into them, and read back the response. Only "+
"the addresses the README lists as blocked "+
"unconditionally stay blocked regardless of what is "+
"listed here; a public cloud metadata address such as "+
"168.63.129.16 is reachable once it, or a block "+
"covering it, is listed.",
"otherwise-blocked private/reserved networks. Anyone "+
"who can create a delivery target can now make this "+
"process issue requests into them, and read back the "+
"response. Link-local and the known cloud instance "+
"metadata endpoints outside it stay blocked "+
"regardless of what is listed here.",
"allowedEgressCIDRs",
strings.Join(PrefixStrings(c.AllowedEgressCIDRs), ","),
)
@@ -777,8 +772,10 @@ func (c *Config) warnEgressAllowlist(log *slog.Logger) {
// everyone else's wrong passwords, and the receiver's limits become
// service-wide ceilings.
//
// The warning is deliberately not gated on WEBHOOKER_ENVIRONMENT:
// behind a proxy every client shares one bucket in dev and prod alike.
// The warning is deliberately not gated on WEBHOOKER_ENVIRONMENT. That
// variable defaults to dev, so gating on it would silence the warning
// for exactly the operator who forgot to configure the deployment —
// the case it exists to catch.
//
// The default of trusting nobody is deliberate — trusting forwarded
// headers from arbitrary peers lets any client choose its own bucket —
internal/config/config_test.go
+17 -13
View File
@@ -44,9 +44,9 @@ func TestEnvironmentConfig(t *testing.T) {
isProd bool
}{
{
name: "default is prod",
isDev: false,
isProd: true,
name: "default is dev",
isDev: true,
isProd: false,
},
{
name: "explicit dev",
@@ -834,13 +834,12 @@ func TestEgressAllowlistWarning(t *testing.T) {
// to be able to read back which networks are open.
assert.Contains(t, logged, "10.0.0.0/8")
assert.Contains(t, logged, "127.0.0.0/8")
// What stays shut is the whole unconditional set, not
// link-local alone; a public metadata address is not in
// it, so a listed block covering it opens it.
assert.Contains(t, logged, "blocked unconditionally")
assert.Contains(t, logged, "168.63.129.16 is reachable")
// The listed blocks need not be private or reserved.
assert.NotContains(t, logged, "private/reserved")
// What stays shut. Asserted on the clause naming the
// wider set rather than on "Link-local" alone, so the
// string cannot narrow back to link-local only while
// the always-blocked set covers ULA, CGNAT and two
// public metadata addresses as well.
assert.Contains(t, logged, "metadata endpoints outside it")
})
}
}
@@ -849,9 +848,10 @@ func TestEgressAllowlistWarning(t *testing.T) {
// tells an operator a deployment behind a reverse proxy shares one
// rate-limit bucket between every client, which turns the receiver
// limits into service-wide ceilings and collapses login failure
// counting. It must fire whenever TRUSTED_PROXIES is empty, in any
// environment, because behind a proxy every client shares one bucket
// in dev and prod alike. It stays quiet once proxies are named.
// counting. It must fire whenever TRUSTED_PROXIES is empty,
// in any environment: WEBHOOKER_ENVIRONMENT defaults to dev, so gating
// on it would silence the warning for exactly the operator who never
// configured the deployment. It stays quiet once proxies are named.
func TestSharedRateLimitBucketWarning(t *testing.T) {
tests := []struct {
name string
@@ -871,6 +871,10 @@ func TestSharedRateLimitBucketWarning(t *testing.T) {
expectWarning: false,
},
{
// The default environment. An internet-exposed
// deployment whose operator never set
// WEBHOOKER_ENVIRONMENT lands here and has exactly
// the exposure the warning announces.
name: "dev without trusted proxies warns",
environment: config.EnvironmentDev,
expectWarning: true,
internal/database/bootstrap_banner_test.go
-36
View File
@@ -3,8 +3,6 @@ package database_test
import (
"bytes"
"context"
"log/slog"
"path/filepath"
"strings"
"testing"
@@ -85,37 +83,3 @@ func TestFirstBoot_PrintsTheAdminPasswordAsABanner(t *testing.T) {
t, ok, "the printed password must open the seeded account",
)
}
// TestNewDatabase_IsLoggedWithItsPath is the log half of
// https://git.eeqj.de/sneak/webhooker/issues/359. A DATA_DIR that is
// unexpectedly empty boots exactly like a first start, so the start
// that creates the database must say so, and where. Opening that
// database again must not.
func TestNewDatabase_IsLoggedWithItsPath(t *testing.T) {
t.Parallel()
dir := t.TempDir()
open := func() string {
var out bytes.Buffer
db, err := database.Open(dir, slog.New(slog.NewTextHandler(&out, nil)))
require.NoError(t, err)
require.NoError(t, db.Close())
return out.String()
}
const created = `level=WARN msg="created a new, empty database"`
first := open()
second := open()
assert.Contains(
t, first,
created+" path="+filepath.Join(dir, database.MainDBFileName),
)
assert.NotContains(
t, second, created, "an existing database is not new",
)
}
internal/database/database.go
+3 -17
View File
@@ -8,7 +8,6 @@ import (
"errors"
"fmt"
"io"
"io/fs"
"log/slog"
"os"
"path/filepath"
@@ -18,12 +17,12 @@ import (
"gorm.io/gorm"
"sneak.berlin/go/webhooker/internal/banner"
"sneak.berlin/go/webhooker/internal/config"
"sneak.berlin/go/webhooker/internal/datadir"
"sneak.berlin/go/webhooker/internal/gormlog"
"sneak.berlin/go/webhooker/internal/logger"
)
const (
dataDirPerm = 0750
randomPasswordLen = 16
sessionKeyLen = 32
)
@@ -186,9 +185,7 @@ func (d *Database) connect() error {
// caller's decision.
func (d *Database) connectTo(dataDir string) error {
// Ensure the data directory exists before opening the database.
// datadir.DirPerm is the single source of the directory mode; this
// package creates the directory too, since either may run first.
err := os.MkdirAll(dataDir, datadir.DirPerm)
err := os.MkdirAll(dataDir, dataDirPerm)
if err != nil {
return fmt.Errorf(
"creating data directory %s: %w",
@@ -200,12 +197,6 @@ func (d *Database) connectTo(dataDir string) error {
// Construct the main application database path inside DATA_DIR.
dbPath := filepath.Join(dataDir, MainDBFileName)
// Checked before opening, which creates the file. A DATA_DIR that
// is unexpectedly empty -- its volume not mounted, say -- looks
// exactly like a first start, so a new database is a warning.
_, statErr := os.Stat(dbPath)
created := errors.Is(statErr, fs.ErrNotExist)
// Opened through OpenSQLite so this handle carries the same WAL
// journaling, busy timeout, immediate-transaction locking, and pool
// bounds as every other database file. See sqlite_open.go.
@@ -236,12 +227,7 @@ func (d *Database) connectTo(dataDir string) error {
}
d.db = db
if created {
d.log.Warn("created a new, empty database", "path", dbPath)
} else {
d.log.Info("connected to database", "path", dbPath)
}
d.log.Info("connected to database", "path", dbPath)
// Run migrations
return d.migrate()
internal/database/event_tier_indexes_test.go
-166
View File
@@ -1,166 +0,0 @@
package database_test
import (
"context"
"fmt"
"testing"
"time"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gorm.io/gorm"
"sneak.berlin/go/webhooker/internal/database"
)
// TestWebhookDBManager_OpenAddsEventTierIndexes verifies that opening a
// per-webhook database that predates these indexes creates them. It
// stands in for an older database file by dropping the indexes
// AutoMigrate just created, then reopening the same file.
func TestWebhookDBManager_OpenAddsEventTierIndexes(t *testing.T) {
t.Parallel()
indexes := []struct {
model any
name string
}{
{&database.Delivery{}, "idx_deliveries_status"},
{&database.Delivery{}, "idx_deliveries_event_id"},
{&database.DeliveryResult{}, "idx_delivery_results_delivery_id"},
{&database.Event{}, "idx_events_deleted_at_created_at"},
{&database.Event{}, "idx_events_created_at"},
}
mgr, lc := setupTestWebhookDBManager(t)
ctx := context.Background()
require.NoError(t, lc.Start(ctx))
defer func() { require.NoError(t, lc.Stop(ctx)) }()
webhookID := uuid.New().String()
db, err := mgr.GetDB(webhookID)
require.NoError(t, err)
// A fresh database has them.
for _, ix := range indexes {
require.True(t, db.Migrator().HasIndex(ix.model, ix.name))
}
// Stand in for a database file created before the indexes existed.
for _, ix := range indexes {
require.NoError(t, db.Migrator().DropIndex(ix.model, ix.name))
require.False(t, db.Migrator().HasIndex(ix.model, ix.name))
}
// Drop the cached connection so the next open reopens the file and
// runs AutoMigrate against it, as a restart would.
require.NoError(t, mgr.CloseAll())
db, err = mgr.GetDB(webhookID)
require.NoError(t, err)
for _, ix := range indexes {
assert.True(t, db.Migrator().HasIndex(ix.model, ix.name),
"opening the existing database should create %s", ix.name)
}
}
// TestEventTierQueriesUseTheirIndexes verifies that the statements the
// indexes are for use them. GORM builds each statement in a dry run as
// the code named above it does, soft-delete condition included, and
// SQLite, which keeps no statistics on these tables, must plan to seek
// on each index listed by the columns in parentheses.
func TestEventTierQueriesUseTheirIndexes(t *testing.T) {
t.Parallel()
mgr, lc := setupTestWebhookDBManager(t)
ctx := context.Background()
require.NoError(t, lc.Start(ctx))
defer func() { require.NoError(t, lc.Stop(ctx)) }()
db, err := mgr.GetDB(uuid.New().String())
require.NoError(t, err)
dry := db.Session(&gorm.Session{DryRun: true})
ids := []string{
uuid.New().String(), uuid.New().String(), uuid.New().String(),
}
cutoff := time.Now()
var (
deliveries []database.Delivery
results []database.DeliveryResult
depths []struct{ Depth int }
)
byStatus := "idx_deliveries_status (status=? AND deleted_at=?)"
byEvent := "idx_deliveries_event_id (event_id=? AND deleted_at=?)"
byAge := "idx_events_deleted_at_created_at (deleted_at=? AND created_at<?)"
// The delivery engine: recovery and the retry sweep, the sweep for
// stranded pending deliveries, and the queue depth count.
assertPlanUses(t, db, dry.Where(
"status = ?", database.DeliveryStatusRetrying,
).Find(&deliveries), byStatus)
assertPlanUses(t, db, dry.Where(
"status = ? AND updated_at < ?",
database.DeliveryStatusPending, cutoff,
).Limit(500).Find(&deliveries), byStatus)
assertPlanUses(t, db, dry.Model(&database.Delivery{}).
Select("target_id", "status", "count(*) as depth").
Where("status IN ?", []database.DeliveryStatus{
database.DeliveryStatusPending,
database.DeliveryStatusRetrying,
}).Group("target_id, status").Find(&depths), byStatus)
// The event log: each event's deliveries, then their attempts
// (loadEventsWithDeliveries, loadDeliveryResults).
assertPlanUses(t, db, dry.Where("event_id = ?", ids[0]).
Find(&deliveries), byEvent)
assertPlanUses(t, db, dry.Where("delivery_id IN ?", ids).
Order("attempt_num ASC").Find(&results),
"idx_delivery_results_delivery_id (delivery_id=? AND deleted_at=?)")
// Retention's three deletes (reapExpired), whose subqueries are built
// afresh for each statement as it builds them.
expiredEventIDs := func() *gorm.DB {
return dry.Model(&database.Event{}).Select("id").
Where("created_at < ?", cutoff)
}
assertPlanUses(t, db, dry.Unscoped().Where(
"delivery_id IN (?)", dry.Model(&database.Delivery{}).
Select("id").Where("event_id IN (?)", expiredEventIDs()),
).Delete(&database.DeliveryResult{}),
"idx_delivery_results_delivery_id (delivery_id=?)", byEvent, byAge)
assertPlanUses(t, db, dry.Unscoped().Where(
"event_id IN (?)", expiredEventIDs(),
).Delete(&database.Delivery{}),
"idx_deliveries_event_id (event_id=?)", byAge)
assertPlanUses(t, db, dry.Unscoped().Where(
"created_at < ?", cutoff,
).Delete(&database.Event{}), "idx_events_created_at (created_at<?)")
}
// assertPlanUses asserts that SQLite's plan for a statement GORM built
// in a dry run, run with the same SQL and arguments GORM would send,
// names each of the given indexes.
func assertPlanUses(
t *testing.T, db, built *gorm.DB, indexes ...string,
) {
t.Helper()
var plan []struct{ Detail string }
require.NoError(t, db.Raw(
"EXPLAIN QUERY PLAN "+built.Statement.SQL.String(),
built.Statement.Vars...,
).Scan(&plan).Error)
for _, index := range indexes {
assert.Contains(t, fmt.Sprint(plan), index,
built.Statement.SQL.String())
}
}
internal/database/export_test.go
-12
View File
@@ -5,7 +5,6 @@ import (
"io"
"log/slog"
"os"
"testing"
"time"
"go.uber.org/fx"
@@ -80,14 +79,3 @@ func (d *Database) ExportSetBannerOut(w io.Writer) {
func DummyPasswordHashForTest() string {
return dummyPasswordHash()
}
// HashAtShippedCostForTest makes HashPassword hash at the shipped
// memory cost until t ends. t must not run in parallel with other
// tests, which would hash at that cost alongside it.
func HashAtShippedCostForTest(t *testing.T) {
t.Helper()
hashAtShippedCostInTest = true
t.Cleanup(func() { hashAtShippedCostInTest = false })
}
internal/database/model_delivery.go
+3 -12
View File
@@ -1,7 +1,5 @@
package database
import "gorm.io/gorm"
// DeliveryStatus represents the status of a delivery
type DeliveryStatus string
@@ -31,19 +29,12 @@ func (s DeliveryStatus) Terminal() bool {
}
// Delivery represents a delivery attempt for an event to a target
//
//nolint:lll // a struct tag cannot wrap
type Delivery struct {
BaseModel
EventID string `gorm:"type:uuid;not null;index:idx_deliveries_event_id,priority:1" json:"eventId"`
TargetID string `gorm:"type:uuid;not null" json:"targetId"`
Status DeliveryStatus `gorm:"not null;default:'pending';index:idx_deliveries_status,priority:1" json:"status"`
// DeletedAt repeats the BaseModel field only to be the second column
// of the event_id and status indexes, for the reason DeliveryResult
// gives.
DeletedAt gorm.DeletedAt `gorm:"index:idx_deliveries_event_id,priority:2;index:idx_deliveries_status,priority:2" json:"deletedAt,omitzero"`
EventID string `gorm:"type:uuid;not null" json:"eventId"`
TargetID string `gorm:"type:uuid;not null" json:"targetId"`
Status DeliveryStatus `gorm:"not null;default:'pending'" json:"status"`
// Relations
Event Event `json:"event,omitzero"`
internal/database/model_delivery_result.go
+1 -12
View File
@@ -1,21 +1,10 @@
package database
import "gorm.io/gorm"
// DeliveryResult represents the result of a delivery attempt
//
//nolint:lll // a struct tag cannot wrap
type DeliveryResult struct {
BaseModel
// DeliveryID and DeletedAt make up one index, in that order.
// DeletedAt repeats the BaseModel field only to join it: GORM adds
// "deleted_at IS NULL" to almost every query, and where a column is
// matched against several values SQLite otherwise reads through the
// deleted_at index, which every live row matches.
DeliveryID string `gorm:"type:uuid;not null;index:idx_delivery_results_delivery_id,priority:1" json:"deliveryId"`
DeletedAt gorm.DeletedAt `gorm:"index:idx_delivery_results_delivery_id,priority:2" json:"deletedAt,omitzero"`
DeliveryID string `gorm:"type:uuid;not null" json:"deliveryId"`
AttemptNum int `gorm:"not null" json:"attemptNum"`
Success bool `json:"success"`
StatusCode int `json:"statusCode,omitempty"`
internal/database/model_event.go
-18
View File
@@ -1,27 +1,9 @@
package database
import (
"time"
"gorm.io/gorm"
)
// Event represents a captured webhook event
//
//nolint:lll // a struct tag cannot wrap
type Event struct {
BaseModel
// CreatedAt and DeletedAt repeat the BaseModel fields only to index
// them for retention, which finds events by age. Its lookups carry
// GORM's "deleted_at IS NULL" (see DeliveryResult) and compare
// created_at with <, so their index has deleted_at first: SQLite
// narrows by a < only on the last column it uses. Its final delete
// has no deleted_at condition and uses the index on created_at
// alone. The other tables keep the unindexed BaseModel created_at.
CreatedAt time.Time `gorm:"index;index:idx_events_deleted_at_created_at,priority:2" json:"createdAt"`
DeletedAt gorm.DeletedAt `gorm:"index:idx_events_deleted_at_created_at,priority:1" json:"deletedAt,omitzero"`
WebhookID string `gorm:"type:uuid;not null" json:"webhookId"`
EntrypointID string `gorm:"type:uuid;not null" json:"entrypointId"`
internal/database/password.go
+1 -22
View File
@@ -9,7 +9,6 @@ import (
"math/big"
"strings"
"sync"
"testing"
"golang.org/x/crypto/argon2"
)
@@ -64,30 +63,10 @@ func DefaultPasswordConfig() *PasswordConfig {
}
}
// testArgon2Memory is the Argon2id memory cost, in KiB, that a test
// binary hashes with: 1 MB instead of the shipped 64 MB. Every test
// that starts a database hashes the bootstrap admin password, dozens
// of them run in parallel, and under the race detector each 64 MB hash
// holds about 150 MB. VerifyPassword reads the cost from the hash it
// checks, so verification follows.
const testArgon2Memory = 1024
// hashAtShippedCostInTest makes a test binary hash at the shipped
// memory cost. Only TestHashPassword_ShippedParameters sets it.
//
//nolint:gochecknoglobals // set by one test, see above
var hashAtShippedCostInTest bool
// HashPassword generates an Argon2id hash of the password. A binary
// built by go test hashes at testArgon2Memory; one built by go build
// always hashes at the defaults.
// HashPassword generates an Argon2id hash of the password
func HashPassword(password string) (string, error) {
config := DefaultPasswordConfig()
if testing.Testing() && !hashAtShippedCostInTest {
config.Memory = testArgon2Memory
}
// Generate a salt
salt := make([]byte, config.SaltLen)
internal/database/password_test.go
-33
View File
@@ -192,39 +192,6 @@ func TestHashPasswordUniqueness(t *testing.T) {
}
}
// TestHashPassword_ShippedParameters hashes and verifies through
// HashPassword at the shipped Argon2id parameters. Every other test
// hashes at the lower memory cost a test binary uses, so this is the
// one that keeps production hashing covered. One hash and one
// verification: each costs 64 MB.
//
//nolint:paralleltest // changes the hashing cost for the whole binary
func TestHashPassword_ShippedParameters(t *testing.T) {
database.HashAtShippedCostForTest(t)
password := "correct horse battery staple"
hash, err := database.HashPassword(password)
if err != nil {
t.Fatalf("hashing with the shipped parameters: %v", err)
}
const shipped = "$argon2id$v=19$m=65536,t=1,p=4$"
if !strings.HasPrefix(hash, shipped) {
t.Errorf("hash = %q, want prefix %q", hash, shipped)
}
valid, err := database.VerifyPassword(password, hash)
if err != nil {
t.Fatalf("VerifyPassword() error = %v", err)
}
if !valid {
t.Error("VerifyPassword() returned false for correct password")
}
}
// TestVerifyDummyPassword_DoesRealWork covers the anti-enumeration
// path. Login charges an unknown username a verification against a
// dummy hash so that a nonexistent account is not answered in
internal/database/sqlite_mode_test.go
-240
View File
@@ -1,240 +0,0 @@
package database_test
import (
"context"
"io/fs"
"net/http"
"os"
"path/filepath"
"testing"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/fx/fxtest"
"sneak.berlin/go/webhooker/internal/config"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/globals"
"sneak.berlin/go/webhooker/internal/logger"
)
// ownerOnly is the mode every SQLite file the service owns must have.
// Spelled out rather than referencing database.SQLiteFilePerm so the
// test fails if the constant itself is loosened.
const ownerOnly fs.FileMode = 0o600
// requireOwnerOnly asserts that path exists and is readable and
// writable by its owner and by nobody else.
func requireOwnerOnly(t *testing.T, path string) {
t.Helper()
info, err := os.Stat(path)
require.NoError(t, err, "%s must exist", path)
assert.Equal(
t,
ownerOnly,
info.Mode().Perm(),
"%s holds credentials and must not be readable by "+
"anyone but its owner",
path,
)
}
// requireDatabaseSetOwnerOnly asserts the mode of a database file and
// of both WAL sidecars. The sidecars carry the same rows as the
// database, so tightening only the main file fixes nothing.
func requireDatabaseSetOwnerOnly(t *testing.T, dbPath string) {
t.Helper()
requireOwnerOnly(t, dbPath)
requireOwnerOnly(t, dbPath+"-wal")
requireOwnerOnly(t, dbPath+"-shm")
}
// TestMainDatabaseFilesAreOwnerOnly covers the tier the defect was
// reported against: webhooker.db holds targets.config in plaintext —
// bearer tokens, API keys, Slack webhook URLs — and the session
// encryption key.
func TestMainDatabaseFilesAreOwnerOnly(t *testing.T) {
t.Parallel()
lc := fxtest.NewLifecycle(t)
l, err := logger.New(lc, logger.LoggerParams{
Globals: &globals.Globals{
Appname: testAppname,
Version: testVersion,
},
})
require.NoError(t, err)
// A directory the application creates itself, not one t.TempDir
// made at 0700, so the mode below is the application's.
dataDir := filepath.Join(t.TempDir(), "data")
db, err := database.New(lc, database.DatabaseParams{
Config: &config.Config{DataDir: dataDir},
Logger: l,
})
require.NoError(t, err)
ctx := context.Background()
require.NoError(t, lc.Start(ctx))
defer func() { require.NoError(t, lc.Stop(ctx)) }()
// Write through the real model so the WAL is populated and both
// sidecars are on disk while the handle is open.
require.NoError(t, db.DB().Create(&database.Webhook{
Name: testWebhookName,
}).Error)
requireDatabaseSetOwnerOnly(
t, filepath.Join(dataDir, database.MainDBFileName),
)
// The data directory grants nothing to `other`. Asserted as a
// property rather than as an exact 0750, because MkdirAll applies
// the ambient umask: the exact mode is the developer's umask as
// much as the application's request, and pinning it would make
// `make check` pass or fail on where it is run. The group bits are
// deliberately left unasserted — deployments may rely on them.
info, err := os.Stat(dataDir)
require.NoError(t, err)
assert.Zero(
t,
info.Mode().Perm()&0o007,
"the data directory must not be world-accessible",
)
}
// TestPerWebhookEventDatabaseFilesAreOwnerOnly covers the events-*.db
// tier. These carry no credential canaries since
// https://git.eeqj.de/sneak/webhooker/issues/206, but they hold every
// received request body and header.
func TestPerWebhookEventDatabaseFilesAreOwnerOnly(t *testing.T) {
t.Parallel()
mgr, lc := setupTestWebhookDBManager(t)
ctx := context.Background()
require.NoError(t, lc.Start(ctx))
defer func() { require.NoError(t, lc.Stop(ctx)) }()
webhookID := uuid.New().String()
db, err := mgr.GetDB(webhookID)
require.NoError(t, err)
require.NoError(t, db.Create(&database.Event{
WebhookID: webhookID,
EntrypointID: uuid.New().String(),
Method: http.MethodPost,
Body: "{}",
}).Error)
requireDatabaseSetOwnerOnly(t, mgr.DBPath(webhookID))
}
// TestArchiveDatabaseFilesAreOwnerOnly covers the archive-*.db tier.
// internal/delivery builds that path and opens it through OpenSQLite,
// the same single open path exercised here, so the mode is settled for
// all three tiers in one place.
func TestArchiveDatabaseFilesAreOwnerOnly(t *testing.T) {
t.Parallel()
ctx := context.Background()
path := filepath.Join(
t.TempDir(), "archive-"+uuid.New().String()+".db",
)
sqlDB, err := database.OpenSQLite(path, database.SQLiteModeCreate)
require.NoError(t, err)
defer func() { require.NoError(t, sqlDB.Close()) }()
_, err = sqlDB.ExecContext(ctx, "create table t (id integer)")
require.NoError(t, err)
requireDatabaseSetOwnerOnly(t, path)
}
// TestOpenSQLiteTightensFilesLeftWorldReadable is the upgrade case: a
// data directory an earlier build left at 0644, including a
// developer's own scratch directory, is fixed when it is opened rather
// than staying exposed until it is recreated.
func TestOpenSQLiteTightensFilesLeftWorldReadable(t *testing.T) {
t.Parallel()
dir := t.TempDir()
path := filepath.Join(dir, database.MainDBFileName)
// A database and both sidecars as the pre-fix build left them.
for _, p := range []string{path, path + "-wal", path + "-shm"} {
require.NoError(t, os.WriteFile(p, nil, 0o644)) //nolint:gosec // the mode under test
}
sqlDB, err := database.OpenSQLite(path, database.SQLiteModeCreate)
require.NoError(t, err)
require.NoError(t, sqlDB.Close())
requireDatabaseSetOwnerOnly(t, path)
}
// TestOpenSQLiteExistingModeDoesNotCreateTheFile guards the mechanism
// the fix uses: OpenSQLite now creates the database file itself, and
// must not do so for a caller that asked for an existing database. An
// empty file materialized here would turn a missing-database error
// into a silently empty one.
func TestOpenSQLiteExistingModeDoesNotCreateTheFile(t *testing.T) {
t.Parallel()
ctx := context.Background()
path := filepath.Join(t.TempDir(), "absent.db")
sqlDB, err := database.OpenSQLite(path, database.SQLiteModeExisting)
if err == nil {
// sql.Open is lazy: force the connection that fails.
require.Error(t, sqlDB.PingContext(ctx))
require.NoError(t, sqlDB.Close())
}
_, statErr := os.Stat(path)
assert.ErrorIs(t, statErr, fs.ErrNotExist)
}
// TestReopenAfterRestartKeepsFilesOwnerOnly is the restart case: a
// process that closed its files must be able to open them again at
// 0600, including through a gorm handle, and the sidecars must come
// back at 0600 too rather than at SQLite's own default.
func TestReopenAfterRestartKeepsFilesOwnerOnly(t *testing.T) {
t.Parallel()
ctx := context.Background()
dir := t.TempDir()
path := filepath.Join(dir, database.MainDBFileName)
first, err := database.OpenSQLite(path, database.SQLiteModeCreate)
require.NoError(t, err)
_, err = first.ExecContext(ctx, "create table t (id integer)")
require.NoError(t, err)
require.NoError(t, first.Close())
second, err := database.OpenSQLite(path, database.SQLiteModeCreate)
require.NoError(t, err)
defer func() { require.NoError(t, second.Close()) }()
_, err = second.ExecContext(ctx, "insert into t (id) values (1)")
require.NoError(t, err)
requireDatabaseSetOwnerOnly(t, path)
var got int
require.NoError(t,
second.QueryRowContext(ctx, "select id from t").Scan(&got))
assert.Equal(t, 1, got)
}
internal/database/sqlite_open.go
-95
View File
@@ -2,11 +2,8 @@ package database
import (
"database/sql"
"errors"
"fmt"
"io/fs"
"net/url"
"os"
"time"
_ "modernc.org/sqlite" // Pure Go SQLite driver
@@ -75,90 +72,6 @@ const (
sqliteConnMaxIdleTime = time.Minute
)
// SQLiteFilePerm is the mode every SQLite file this service owns is
// created with and held at: owner read/write, nothing for group or
// other.
//
// These files hold credentials in plaintext. The main database stores
// `targets.config` — bearer tokens, API keys, Slack webhook URLs — and
// the session encryption key. SQLite left to itself creates them 0644
// (see reserveSQLiteFile), which made the 0750 data directory the only
// barrier; a bind-mounted directory supplied at 0755 removes it and
// every local user on the host can read every stored credential.
//
// This is a file-mode fix and not encryption at rest. An unattended
// process needs a key it can read without a human, so the key lands
// beside the data and an attacker who can read the database can read
// it too. See https://git.eeqj.de/sneak/webhooker/issues/212.
const SQLiteFilePerm fs.FileMode = 0o600
// reserveSQLiteFile puts path at SQLiteFilePerm before the driver ever
// touches it, and tightens any sidecar already on disk.
//
// The mode has to be settled here rather than by a chmod after opening,
// because SQLite picks it: robust_open substitutes
// SQLITE_DEFAULT_FILE_PERMISSIONS (0644) whenever it is handed mode 0,
// and findCreateFileMode yields 0 for a main database opened by URI
// with no `modeof` parameter. A chmod afterwards would leave a window
// in which the credentials are on disk world-readable.
//
// Creating the file ourselves also settles the sidecars, which is the
// half that could quietly not work. SQLite does not create those at a
// mode we choose — it derives both from the main database file:
// `-wal` through findCreateFileMode, which stats the path with the
// suffix stripped, and `-shm` in unixOpenSharedMemory from an fstat of
// the already-open database descriptor. A main file at 0600 therefore
// produces sidecars at 0600. A zero-length file is a valid empty
// database, so reserving it changes nothing else.
//
// create says whether the caller is opening in a mode that may create
// the database. When it is false a missing file is left missing, so
// SQLite still reports the absence rather than this function
// materializing an empty database the caller asked not to create.
//
// Chmod of a file that already exists is what tightens a data
// directory an earlier build left at 0644 — including a developer's
// own scratch directory — without any migration machinery.
func reserveSQLiteFile(path string, create bool) error {
if create {
// gosec G304: the path is the database file the caller asked
// to open, and the driver is about to open the same path
// anyway. Creating it here is what fixes its mode.
f, err := os.OpenFile( //nolint:gosec // see above
path, os.O_RDWR|os.O_CREATE, SQLiteFilePerm,
)
if err != nil {
return fmt.Errorf("creating %s: %w", path, err)
}
err = f.Close()
if err != nil {
return fmt.Errorf("closing %s: %w", path, err)
}
}
// O_CREATE leaves an existing file's mode alone, and umask can only
// have narrowed a new one. Chmod settles both cases at exactly
// SQLiteFilePerm.
for _, p := range append(
[]string{path}, sqliteSidecarPaths(path)...,
) {
err := os.Chmod(p, SQLiteFilePerm)
if err != nil && !errors.Is(err, fs.ErrNotExist) {
return fmt.Errorf("securing %s: %w", p, err)
}
}
return nil
}
// sqliteSidecarPaths returns the files SQLite maintains beside a
// database under WAL. They carry the same rows as the database itself,
// so a fix that tightens only the main file has fixed nothing.
func sqliteSidecarPaths(path string) []string {
return []string{path + "-wal", path + "-shm"}
}
// SQLiteDSN builds the connection string for one database file.
//
// mode is the SQLite URI open mode: "rwc" to create the file when it
@@ -225,17 +138,9 @@ func SQLiteDSN(path, mode string) string {
// durability settings and pool bounds applied. mode is the SQLite URI
// open mode ("rwc" or "rw").
//
// The file and its WAL sidecars are settled at SQLiteFilePerm before
// the driver sees the path; see reserveSQLiteFile.
//
// The handle is returned rather than a *gorm.DB because the callers
// wrap it in gorm themselves with their own logger.
func OpenSQLite(path, mode string) (*sql.DB, error) {
err := reserveSQLiteFile(path, mode == SQLiteModeCreate)
if err != nil {
return nil, err
}
sqlDB, err := sql.Open("sqlite", SQLiteDSN(path, mode))
if err != nil {
return nil, fmt.Errorf(
internal/database/webhook_db_manager.go
+29 -42
View File
@@ -13,7 +13,6 @@ import (
"gorm.io/driver/sqlite"
"gorm.io/gorm"
"sneak.berlin/go/webhooker/internal/config"
"sneak.berlin/go/webhooker/internal/datadir"
"sneak.berlin/go/webhooker/internal/gormlog"
"sneak.berlin/go/webhooker/internal/logger"
)
@@ -41,11 +40,6 @@ type WebhookDBManager struct {
dataDir string
dbs sync.Map // map[webhookID]*gorm.DB
log *slog.Logger
// mu is held while a database is opened, deleted, or closed, so
// each file has at most one open handle. Reading an already cached
// handle does not take it.
mu sync.Mutex
}
// NewWebhookDBManager creates a new WebhookDBManager and
@@ -59,9 +53,8 @@ func NewWebhookDBManager(
log: params.Logger.Get(),
}
// Create data directory if it doesn't exist. datadir.DirPerm is the
// single source of the directory mode; either package may run first.
err := os.MkdirAll(m.dataDir, datadir.DirPerm)
// Create data directory if it doesn't exist
err := os.MkdirAll(m.dataDir, dataDirPerm)
if err != nil {
return nil, fmt.Errorf(
"creating data directory %s: %w",
@@ -91,39 +84,43 @@ func (m *WebhookDBManager) GetDB(
) (*gorm.DB, error) {
// Fast path: already open
if val, ok := m.dbs.Load(webhookID); ok {
return asGormDB(val, webhookID)
}
cachedDB, castOK := val.(*gorm.DB)
if !castOK {
return nil, fmt.Errorf(
"%w for webhook %s",
errInvalidCachedDBType,
webhookID,
)
}
// Slow path: open the database under the lock, looking in the
// cache again first. A caller that raced another one here then
// waits for its handle instead of opening a second one.
m.mu.Lock()
defer m.mu.Unlock()
if val, ok := m.dbs.Load(webhookID); ok {
return asGormDB(val, webhookID)
return cachedDB, nil
}
// Slow path: open/create the database
db, err := m.openDB(webhookID)
if err != nil {
return nil, err
}
m.dbs.Store(webhookID, db)
// Store it; if another goroutine beat us, close ours
actual, loaded := m.dbs.LoadOrStore(webhookID, db)
if loaded {
// Another goroutine created it first; close our duplicate
sqlDB, closeErr := db.DB()
if closeErr == nil {
_ = sqlDB.Close()
}
return db, nil
}
existingDB, castOK := actual.(*gorm.DB)
if !castOK {
return nil, fmt.Errorf(
"%w for webhook %s",
errInvalidCachedDBType,
webhookID,
)
}
// asGormDB returns a value read from the cache as the database
// handle it is.
func asGormDB(val any, webhookID string) (*gorm.DB, error) {
db, ok := val.(*gorm.DB)
if !ok {
return nil, fmt.Errorf(
"%w for webhook %s",
errInvalidCachedDBType,
webhookID,
)
return existingDB, nil
}
return db, nil
@@ -154,11 +151,6 @@ func (m *WebhookDBManager) DBExists(
func (m *WebhookDBManager) DeleteDB(
webhookID string,
) error {
// Held until the files are gone, so GetDB cannot open the file
// again between the close and the removal.
m.mu.Lock()
defer m.mu.Unlock()
// Close and remove from cache
if val, ok := m.dbs.LoadAndDelete(webhookID); ok {
if gormDB, castOK := val.(*gorm.DB); castOK {
@@ -192,11 +184,6 @@ func (m *WebhookDBManager) DeleteDB(
// CloseAll closes all open per-webhook database connections.
// Called during application shutdown.
func (m *WebhookDBManager) CloseAll() error {
// An open already under way finishes and is cached first, so it
// is closed here rather than cached after this loop has passed.
m.mu.Lock()
defer m.mu.Unlock()
var lastErr error
m.dbs.Range(func(key, value any) bool {
internal/database/webhook_db_manager_test.go
-52
View File
@@ -1,14 +1,10 @@
package database_test
import (
"bytes"
"context"
"log/slog"
"net/http"
"os"
"path/filepath"
"strings"
"sync"
"testing"
"github.com/google/uuid"
@@ -108,54 +104,6 @@ func TestWebhookDBManager_CreateAndGetDB(t *testing.T) {
assert.Equal(t, `{"test": true}`, readEvent.Body)
}
// Many callers ask for one webhook's database at the same moment,
// before it is cached. Only one of them may open the file; the others
// must wait for its handle. openDB logs one "opened per-webhook
// database" line per open, and those lines are what is counted.
func TestWebhookDBManager_ConcurrentFirstTouchOpensOnce(t *testing.T) {
t.Parallel()
var logs bytes.Buffer
mgr := database.NewTestWebhookDBManagerWithLogger(
t.TempDir(),
slog.New(slog.NewTextHandler(&logs, nil)),
)
t.Cleanup(func() { assert.NoError(t, mgr.CloseAll()) })
webhookID := uuid.New().String()
const callers = 16
start := make(chan struct{})
handles := make([]*gorm.DB, callers)
errs := make([]error, callers)
var wg sync.WaitGroup
for i := range callers {
wg.Go(func() {
<-start
handles[i], errs[i] = mgr.GetDB(webhookID)
})
}
close(start)
wg.Wait()
for i := range callers {
require.NoError(t, errs[i])
assert.Same(t, handles[0], handles[i])
}
assert.Equal(
t, 1,
strings.Count(logs.String(), "opened per-webhook database"),
)
}
func TestWebhookDBManager_DeleteDB(t *testing.T) {
t.Parallel()
internal/datadir/lock.go
+4 -6
View File
@@ -29,11 +29,9 @@ import (
// process that was killed with SIGKILL blocks nothing.
const LockFileName = "webhooker.lock"
// DirPerm is the mode DATA_DIR is created with. It is the single
// source of that mode: internal/database consumes it rather than
// keeping its own copy, so the two packages that both create the
// directory cannot drift into disagreeing about its permissions.
const DirPerm = 0o750
// dirPerm is the mode Acquire creates DATA_DIR with. It matches what
// internal/database uses, since whichever runs first creates it.
const dirPerm = 0o750
// ErrLocked reports that another live process holds the data
// directory. Callers that need to know whether a deployment is running
@@ -66,7 +64,7 @@ func Acquire(dir string) (*Lock, error) {
return nil, ErrNoDir
}
err := os.MkdirAll(dir, DirPerm)
err := os.MkdirAll(dir, dirPerm)
if err != nil {
return nil, fmt.Errorf(
"creating data directory %s: %w", dir, err,
internal/delivery/circuit_breaker.go
+2 -10
View File
@@ -76,20 +76,12 @@ func (cb *CircuitBreaker) Allow() bool {
}
}
// CooldownRemaining returns how long a delivery that Allow refused
// should wait before it is tried again. Closed, it returns zero.
// Open, it returns what is left of the cooldown, or zero once that
// has passed. Half-open, it returns the whole cooldown: the one
// probe delivery is still in flight, and if it fails the circuit
// reopens for that long.
// CooldownRemaining returns how much time is left before
// an open circuit transitions to half-open.
func (cb *CircuitBreaker) CooldownRemaining() time.Duration {
cb.mu.Lock()
defer cb.mu.Unlock()
if cb.state == CircuitHalfOpen {
return cb.cooldown
}
if cb.state != CircuitOpen {
return 0
}
internal/delivery/circuit_breaker_test.go
+3 -5
View File
@@ -267,7 +267,7 @@ func TestCircuitBreaker_CooldownRemaining_ClosedReturnsZero(
)
}
func TestCircuitBreaker_CooldownRemaining_HalfOpenReturnsCooldown(
func TestCircuitBreaker_CooldownRemaining_HalfOpenReturnsZero(
t *testing.T,
) {
t.Parallel()
@@ -282,11 +282,9 @@ func TestCircuitBreaker_CooldownRemaining_HalfOpenReturnsCooldown(
require.True(t, cb.Allow())
// The cooldown newShortCooldownCB gives the breaker.
assert.Equal(t, 50*time.Millisecond,
assert.Equal(t, time.Duration(0),
cb.CooldownRemaining(),
"a delivery refused while half-open should wait "+
"a whole cooldown",
"half-open circuit should have zero cooldown remaining",
)
}
internal/delivery/engine.go
+38 -153
View File
@@ -362,15 +362,6 @@ func (e *Engine) start() {
// stop cancels the worker pool's context and waits for the pool
// to drain, bounded by the stop hook's context: a wedged worker
// must not hang the process past fx's stop timeout.
//
// Once the pool has drained it closes the archive writers, so a
// clean stop leaves no archive -wal behind. Nothing else holds a
// writer for long by then: the archive sweeper stops before the
// engine, and deleting a webhook only closes one. If the pool did
// not drain in time, the writers are left open, as a kill would
// leave them. Closing them would wait for any write in progress,
// and a worker still running would then open new writers that
// nothing closes, so it gains nothing over a kill.
func (e *Engine) stop(ctx context.Context) error {
e.log.Info("delivery engine stopping")
@@ -385,8 +376,6 @@ func (e *Engine) stop(ctx context.Context) error {
return err
}
e.dbTarget.evictAll()
e.log.Info("delivery engine stopped")
return nil
@@ -449,34 +438,9 @@ func (e *Engine) processNewTask(
return
}
// Restart recovery can send and release this delivery before the
// receiver's Notify queues it. Ownership cannot refuse a delivery
// nobody holds, so the row decides whether it still needs sending.
row, err := e.loadDelivery(webhookDB, task.DeliveryID)
if err != nil {
e.log.Error(
"failed to load delivery",
"delivery_id", task.DeliveryID,
"error", err,
)
return
}
if row.Status != database.DeliveryStatusPending {
e.log.Info(
"delivery already handled, not sent again",
"delivery_id", task.DeliveryID,
"event_id", task.EventID,
"status", row.Status,
)
return
}
event := buildEventFromTask(task)
event, err = e.hydrateEvent(
event, err = e.resolveEventBody(
webhookDB, event, task,
)
if err != nil {
@@ -518,7 +482,7 @@ func (e *Engine) processRetryTask(
return
}
d, err := e.loadDelivery(
d, err := e.loadRetryDelivery(
webhookDB, task.DeliveryID,
)
if err != nil {
@@ -548,7 +512,7 @@ func (e *Engine) processRetryTask(
event := buildEventFromTask(task)
event, err = e.hydrateEvent(
event, err = e.resolveEventBody(
webhookDB, event, task,
)
if err != nil {
@@ -739,7 +703,9 @@ func (e *Engine) recoverSingleRetry(
// webhook on one bad read would be a far larger fault than
// the strand it is meant to clear.
if errors.Is(err, gorm.ErrRecordNotFound) {
e.failMissingTarget(webhookDB, webhookID, d)
e.failMissingTargetRetry(
webhookDB, webhookID, d,
)
return
}
@@ -1142,7 +1108,9 @@ func (e *Engine) sweepSingleRetry(
// Deleted is terminal, unreadable is not; see
// recoverSingleRetry.
if errors.Is(err, gorm.ErrRecordNotFound) {
e.failMissingTarget(webhookDB, webhookID, d)
e.failMissingTargetRetry(
webhookDB, webhookID, d,
)
return
}
@@ -1256,19 +1224,19 @@ func (e *Engine) failUnretryableRetry(
e.failDelivery(webhookDB, d, target.Type, reason)
}
// failMissingTarget terminally fails a recovered delivery, pending or
// retrying, whose target row is gone. Restart recovery and the periodic
// sweep call it for both statuses, so the transition exists once.
// failMissingTargetRetry terminally fails an orphaned retrying
// delivery whose target row is gone. Both restart recovery and the
// periodic sweep call it, so the transition exists once.
//
// Until it existed those paths logged the failed lookup and moved on,
// which left the delivery where it was for the life of the database and
// Until it existed both paths logged the failed lookup and returned,
// which left the delivery retrying for the life of the database and
// the sweep repeating the same error every minute forever. Failing it
// with a recorded reason is the treatment the other orphaned-retry
// cases already get, so all of them read alike in the event log.
//
// Logged at warn rather than error: a deleted target is an operator
// action, not a system fault.
func (e *Engine) failMissingTarget(
func (e *Engine) failMissingTargetRetry(
webhookDB *gorm.DB,
webhookID string,
d *database.Delivery,
@@ -1281,37 +1249,13 @@ func (e *Engine) failMissingTarget(
defer e.inflight.release(d.ID)
// The batch was read before ownership was taken, and a worker may
// have settled the delivery and let it go in between. Only a row
// still in the status the batch read is failed.
row, err := e.loadDelivery(webhookDB, d.ID)
if err != nil {
e.log.Error(
"failed to load delivery",
"delivery_id", d.ID,
"error", err,
)
return
}
if row.Status != d.Status {
e.log.Debug(
"delivery already handled, not failed",
"delivery_id", d.ID,
"status", row.Status,
)
return
}
targetType, reason := e.missingTargetReason(d.TargetID)
e.log.Warn(
"failing recovered delivery: its target no longer exists",
"failing orphaned retrying delivery: "+
"its target no longer exists",
"webhook_id", webhookID,
"delivery_id", d.ID,
"status", d.Status,
"target_id", d.TargetID,
"target_type", targetType,
)
@@ -1345,14 +1289,15 @@ func (e *Engine) missingTargetReason(
if err != nil {
return "", fmt.Sprintf(
"target %s no longer exists; the delivery "+
"has been failed terminally",
"cannot be retried and has been failed "+
"terminally",
targetID,
)
}
return target.Type, fmt.Sprintf(
"target %q (type %s) was deleted; the delivery "+
"has been failed terminally",
"cannot be retried and has been failed terminally",
target.Name, target.Type,
)
}
@@ -1602,11 +1547,6 @@ func truncate(s string, maxLen int) string {
// --- Helper functions ---
// buildEventFromTask reconstructs the event a Task describes, as far
// as the Task itself goes. The fields it cannot fill — the body when
// it was too large to inline, and the receipt time, which no Task
// carries — come from the stored row in hydrateEvent, which every
// caller of this function runs next.
func buildEventFromTask(task *Task) database.Event {
event := database.Event{
EntrypointID: task.EntrypointID,
@@ -1634,71 +1574,33 @@ func buildTargetFromTask(task *Task) database.Target {
return target
}
// hydrateEvent fills in the event fields a Task does not carry, by
// reading the stored event row.
//
// CreatedAt is the event's receipt time and lives only in that row.
// The Slack target renders it into every message it sends, so an
// unhydrated event puts the zero time in front of a human on every
// notification the product delivers. See
// https://git.eeqj.de/sneak/webhooker/issues/257.
//
// The body comes from the same row when the Task did not inline it,
// which is the case for a body at or above MaxInlineBodySize.
//
// A read failure is fatal to the delivery only when the body depended
// on it. When the Task inlined the body, the delivery has everything
// it needs to be sent and goes ahead with the timestamp unset: the row
// can be gone under a retention reap while a queued delivery still
// holds its body, and dropping a deliverable event to protect one
// metadata field would be a worse failure than the one it prevents.
func (e *Engine) hydrateEvent(
func (e *Engine) resolveEventBody(
webhookDB *gorm.DB,
event database.Event,
task *Task,
) (database.Event, error) {
columns := []string{"created_at"}
if task.Body == nil {
columns = append(columns, "body")
}
var dbEvent database.Event
err := webhookDB.Select(columns).
First(&dbEvent, "id = ?", task.EventID).Error
if err != nil {
if task.Body == nil {
return event, fmt.Errorf(
"fetching event body: %w", err,
)
}
e.log.Warn(
"could not read the stored event; delivering "+
"the inlined body without its receipt time",
"event_id", task.EventID,
"delivery_id", task.DeliveryID,
"error", err,
)
if task.Body != nil {
event.Body = *task.Body
return event, nil
}
event.CreatedAt = dbEvent.CreatedAt
var dbEvent database.Event
if task.Body != nil {
event.Body = *task.Body
} else {
event.Body = dbEvent.Body
err := webhookDB.Select("body").
First(&dbEvent, "id = ?", task.EventID).Error
if err != nil {
return event, fmt.Errorf(
"fetching event body: %w", err,
)
}
event.Body = dbEvent.Body
return event, nil
}
func (e *Engine) loadDelivery(
func (e *Engine) loadRetryDelivery(
webhookDB *gorm.DB, deliveryID string,
) (*database.Delivery, error) {
var d database.Delivery
@@ -2051,30 +1953,13 @@ func (e *Engine) sendRecoveredDeliveries(
target, ok := targetMap[deliveries[i].TargetID]
if !ok {
// A missing entry does not mean the target is gone: the
// map is also empty when its query failed. Only a lookup
// that finds no row ends the delivery; any other error
// leaves it pending for the next sweep. See
// recoverSingleRetry.
var err error
e.log.Error(
"target not found for delivery",
"delivery_id", deliveries[i].ID,
"target_id", deliveries[i].TargetID,
)
target, err = e.loadTarget(deliveries[i].TargetID)
if errors.Is(err, gorm.ErrRecordNotFound) {
e.failMissingTarget(webhookDB, webhookID, &deliveries[i])
continue
}
if err != nil {
e.log.Error(
"failed to load target for recovered delivery",
"delivery_id", deliveries[i].ID,
"target_id", deliveries[i].TargetID,
"error", err,
)
continue
}
continue
}
if !e.takeForRedispatch(
internal/delivery/engine_lifecycle_test.go
-87
View File
@@ -2,8 +2,6 @@ package delivery_test
import (
"context"
"fmt"
"path/filepath"
"testing"
"time"
@@ -271,88 +269,3 @@ func TestEngine_StopHookHonoursStopTimeout(t *testing.T) {
requireStopHookExpires(t, lc.hooks[0], "delivery engine")
}
// deliverToArchive runs one delivery to a database target through
// the running engine and returns the webhook's archive file path.
// The archive writer holds the file open afterwards.
func deliverToArchive(t *testing.T, s iSetup) string {
t.Helper()
deliveryID, task := seedLogTask(t, s)
task.TargetType = database.TargetTypeDatabase
s.Engine.Notify([]delivery.Task{task})
iWaitForDelivered(t, s.WebhookDB, deliveryID)
return filepath.Join(
filepath.Dir(s.DBMgr.DBPath(s.WebhookID)),
fmt.Sprintf("archive-%s.db", s.WebhookID),
)
}
// TestEngine_StopHookClosesArchives is the regression test for an
// archive split across two files by a clean stop. The engine never
// closed its archive writers, so after a stop the archived rows
// could sit in archive-{id}.db-wal while archive-{id}.db held no
// table at all, and copying the .db on its own gave an empty
// database.
func TestEngine_StopHookClosesArchives(t *testing.T) {
t.Parallel()
s := newISetup(t)
lc := startEngineViaHook(t, s.Engine)
path := deliverToArchive(t, s)
require.FileExists(
t, path+"-wal",
"an open archive should have a -wal for the stop to remove",
)
require.NoError(t, lc.hooks[0].OnStop(context.Background()))
wals, err := filepath.Glob(
filepath.Join(filepath.Dir(path), "archive-*.db-wal"),
)
require.NoError(t, err)
require.Empty(
t, wals, "a clean stop must leave no archive -wal behind",
)
// With no -wal beside it, the row can only be in the .db.
count, err := countArchivedRows(path)
require.NoError(t, err)
require.Equal(t, int64(1), count)
}
// TestEngine_StopHookTimeoutLeavesArchivesOpen covers a stop whose
// budget runs out while a worker is still running. The archive
// writers are left open, as a kill would leave them: closing them
// would wait for any write in progress, and that worker would then
// open new writers that nothing closes.
func TestEngine_StopHookTimeoutLeavesArchivesOpen(t *testing.T) {
t.Parallel()
s := newISetup(t)
lc := startEngineViaHook(t, s.Engine)
deliverToArchive(t, s)
release := make(chan struct{})
t.Cleanup(func() {
close(release)
s.Engine.EvictWebhook(s.WebhookID)
})
s.Engine.ExportWedgeWorker(release)
requireStopHookExpires(t, lc.hooks[0], "delivery engine")
require.True(
t, s.Engine.ExportArchiveHandleOpen(s.WebhookID),
"a stop that timed out must not close archive writers",
)
}
internal/delivery/engine_test.go
-175
View File
@@ -17,7 +17,6 @@ import (
"time"
"github.com/google/uuid"
"github.com/prometheus/client_golang/prometheus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gorm.io/driver/sqlite"
@@ -25,7 +24,6 @@ import (
_ "modernc.org/sqlite"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
"sneak.berlin/go/webhooker/internal/metrics"
)
// testContentType is the event content type used in tests.
@@ -896,100 +894,6 @@ func TestDeliverHTTP_CircuitBreakerBlocks(t *testing.T) {
)
}
// recordingScheduler keeps the delay of every retry it is asked to
// schedule, and schedules nothing.
type recordingScheduler struct {
delays []time.Duration
}
func (s *recordingScheduler) ScheduleRetry(
_ delivery.Task, delay time.Duration,
) {
s.delays = append(s.delays, delay)
}
// TestDeliverHTTP_HalfOpenBreakerDelaysQueuedTasks proves that while a
// half-open breaker's one probe delivery is in flight, every other task
// for the target is put back with a whole cooldown as its delay rather
// than none, and that its status is written the first time the breaker
// turns it away and not on each pass after that.
func TestDeliverHTTP_HalfOpenBreakerDelaysQueuedTasks(t *testing.T) {
t.Parallel()
db := testWebhookDB(t)
e := testEngine(t, 1)
// Every write of retrying moves the retry counter, so on a registry
// this test owns the counter is the number of those writes.
reg := prometheus.NewRegistry()
e.ExportSetMetrics(metrics.New(reg))
targetID := uuid.New().String()
cb := newShortCooldownCB(t)
e.ExportSetCircuitBreaker(targetID, cb)
for range delivery.ExportDefaultFailureThreshold {
cb.RecordFailure()
}
time.Sleep(60 * time.Millisecond)
require.True(t, cb.Allow(), "the probe delivery should go through")
require.Equal(t, delivery.CircuitHalfOpen, cb.State())
cfg := newHTTPTargetConfig(
"http://will-not-be-called.invalid",
)
sched := &recordingScheduler{}
const queued, passes = 3, 4
for range queued {
event := seedEvent(t, db, `{"cb":"half-open"}`)
dlv := seedDelivery(
t, db, event.ID, targetID,
database.DeliveryStatusPending,
)
for range passes {
// Each pass starts from the stored row, as a retry does.
var row database.Delivery
require.NoError(t, db.First(
&row, "id = ?", dlv.ID,
).Error)
fix := buildHTTPFixture(
row, event, targetID,
"test-cb-half-open", cfg, 5, 1,
)
e.ExportDeliverHTTPWithScheduler(
context.TODO(), db, fix.Delivery, fix.Task, sched,
)
}
assertDeliveryStatus(t, db, dlv.ID,
database.DeliveryStatusRetrying,
)
}
require.Len(t, sched.delays, queued*passes)
for _, delay := range sched.delays {
// The cooldown newShortCooldownCB gives the breaker.
assert.Equal(t, 50*time.Millisecond, delay,
"a task turned away while half-open should wait "+
"a whole cooldown",
)
}
assert.InDelta(t, float64(queued),
mCounter(t, reg, mRetries, mTypeHTTP), 0,
"status should be written once per task, not once per pass",
)
}
func TestGetCircuitBreaker_CreatesOnDemand(t *testing.T) {
t.Parallel()
@@ -1166,10 +1070,6 @@ func TestIsForwardableHeader(t *testing.T) {
assert.False(t,
delivery.ExportIsForwardableHeader("Content-Length"),
)
assert.False(t,
delivery.ExportIsForwardableHeader("Content-Type"),
)
}
func TestTruncate(t *testing.T) {
@@ -1251,81 +1151,6 @@ func TestDoHTTPRequest_ForwardsHeaders(t *testing.T) {
)
}
// The event's stored inbound headers carry the same Content-Type the
// receiver saved as the event's ContentType, so a delivery could send
// it twice. It must go out exactly once, with a Content-Type configured
// on the target winning, then the event's ContentType.
func TestApplyRequestHeaders_SendsOneContentType(t *testing.T) {
t.Parallel()
cases := map[string]struct {
inbound string
event string
configured string
want []string
}{
"inbound and event agree": {
inbound: testContentType,
event: testContentType,
want: []string{testContentType},
},
"inbound and event disagree": {
inbound: "text/plain",
event: testContentType,
want: []string{testContentType},
},
"event has none": {
inbound: testContentType,
want: nil,
},
"target configures its own": {
inbound: testContentType,
event: testContentType,
configured: "application/xml",
want: []string{"application/xml"},
},
}
for name, tc := range cases {
t.Run(name, func(t *testing.T) {
t.Parallel()
inbound, err := json.Marshal(map[string][]string{
headerContentType: {tc.inbound},
})
require.NoError(t, err)
cfg := &delivery.HTTPTargetConfig{}
if tc.configured != "" {
cfg.Headers = map[string]string{
headerContentType: tc.configured,
}
}
req, err := http.NewRequestWithContext(
context.Background(),
http.MethodPost,
"https://target.example.com/hook",
http.NoBody,
)
require.NoError(t, err)
delivery.ExportApplyRequestHeaders(
req,
&database.Event{
Headers: string(inbound),
ContentType: tc.event,
},
cfg,
)
assert.Equal(t,
tc.want, req.Header.Values(headerContentType),
)
})
}
}
func TestProcessDelivery_RoutesToCorrectHandler(
t *testing.T,
) {
internal/delivery/event_timestamp_test.go
-442
View File
@@ -1,442 +0,0 @@
package delivery_test
import (
"context"
"encoding/json"
"io"
"net/http"
"net/http/httptest"
"testing"
"time"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gorm.io/gorm"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
)
// tsEventCreatedAt is the receipt time seeded on the events these
// tests deliver. It is far enough from both the zero time and from
// now that neither can be mistaken for it.
func tsEventCreatedAt() time.Time {
return time.Date(
2026, time.March, 4, 5, 6, 7, 0, time.UTC,
)
}
// tsZeroStamp is what a Slack message renders when the event handed
// to FormatSlackMessage carries no CreatedAt.
const tsZeroStamp = "*Timestamp:* `0001-01-01T00:00:00Z`"
// tsEventBody is the body seeded on every event in this file. It is
// small enough that a Task can inline it.
const tsEventBody = `{"hello":"world"}`
// tsUndeliverableHook stands in for a Slack incoming webhook on the
// tests that never send: the config parser requires a URL, but no
// request is made.
const tsUndeliverableHook = "https://hooks.slack.com/services/T/B/x"
// tsSink is a stand-in Slack incoming webhook that records the raw
// body posted to it.
type tsSink struct {
*httptest.Server
bodies chan []byte
}
func newTSSink(t *testing.T) *tsSink {
t.Helper()
s := &tsSink{bodies: make(chan []byte, 8)}
s.Server = httptest.NewServer(http.HandlerFunc(
func(w http.ResponseWriter, r *http.Request) {
body, _ := io.ReadAll(r.Body)
select {
case s.bodies <- body:
default:
}
w.WriteHeader(http.StatusOK)
},
))
t.Cleanup(s.Close)
return s
}
// text returns the Slack message text from the single payload the
// sink received.
func (s *tsSink) text(t *testing.T) string {
t.Helper()
select {
case raw := <-s.bodies:
t.Logf("raw slack payload: %s", raw)
var payload struct {
Text string `json:"text"`
}
require.NoError(t, json.Unmarshal(raw, &payload))
return payload.Text
case <-time.After(5 * time.Second):
t.Fatal("slack sink received no payload")
return ""
}
}
func tsSlackConfig(t *testing.T, url string) string {
t.Helper()
data, err := json.Marshal(
delivery.SlackTargetConfig{WebhookURL: url},
)
require.NoError(t, err)
return string(data)
}
// tsSeedEvent writes an event whose CreatedAt is tsEventCreatedAt
// rather than the write time, so an assertion on the rendered
// timestamp cannot pass by accident against "roughly now".
func tsSeedEvent(
t *testing.T, db *gorm.DB, webhookID string,
) database.Event {
t.Helper()
event := database.Event{
WebhookID: webhookID,
EntrypointID: uuid.New().String(),
Method: http.MethodPost,
Headers: `{}`,
Body: tsEventBody,
ContentType: "application/json",
}
event.ID = uuid.New().String()
event.CreatedAt = tsEventCreatedAt()
event.UpdatedAt = tsEventCreatedAt()
require.NoError(t, db.Create(&event).Error)
var stored database.Event
require.NoError(t,
db.First(&stored, "id = ?", event.ID).Error,
)
require.Equal(t,
tsEventCreatedAt().UTC(), stored.CreatedAt.UTC(),
"seeded created_at did not round-trip",
)
return event
}
// tsSeedTarget writes the slack target row into the main database.
// The retry path confirms the target still exists before sending.
func tsSeedTarget(
t *testing.T, mainDB *gorm.DB, webhookID, config string,
) database.Target {
t.Helper()
target := database.Target{
WebhookID: webhookID,
Name: "slack-sink",
Type: database.TargetTypeSlack,
Config: config,
Active: true,
}
require.NoError(t, mainDB.Create(&target).Error)
return target
}
func tsTask(
d database.Delivery,
event database.Event,
webhookID string,
target database.Target,
attemptNum int,
body *string,
) delivery.Task {
return delivery.Task{
DeliveryID: d.ID,
EventID: event.ID,
WebhookID: webhookID,
EntrypointID: event.EntrypointID,
TargetID: target.ID,
TargetName: target.Name,
TargetType: database.TargetTypeSlack,
TargetConfig: target.Config,
MaxRetries: 0,
Method: event.Method,
Headers: event.Headers,
ContentType: event.ContentType,
Body: body,
AttemptNum: attemptNum,
}
}
func tsAssertRealTimestamp(t *testing.T, text string) {
t.Helper()
assert.NotContains(t, text, tsZeroStamp,
"slack message carries the zero timestamp",
)
assert.Contains(t, text,
"*Timestamp:* `"+
tsEventCreatedAt().UTC().Format(time.RFC3339)+"`",
"slack message does not carry the event's receipt time",
)
}
// tsCase is one end-to-end delivery of a seeded event to a slack
// sink, over whichever engine path `process` names.
type tsCase struct {
// status is the delivery row's status before the engine runs.
// The retry path refuses a delivery that is not retrying.
status database.DeliveryStatus
// inlineBody mirrors a Task built for a body under
// MaxInlineBodySize. When false the engine reads the body back
// from the stored row.
inlineBody bool
attemptNum int
process func(
ctx context.Context, e *delivery.Engine, task *delivery.Task,
)
}
// run delivers one event through the named path and returns the
// Slack message text the sink received.
func (c tsCase) run(t *testing.T) (iSetup, database.Delivery, string) {
t.Helper()
s := newISetup(t)
sink := newTSSink(t)
cfg := tsSlackConfig(t, sink.URL)
target := tsSeedTarget(t, s.MainDB, s.WebhookID, cfg)
event := tsSeedEvent(t, s.WebhookDB, s.WebhookID)
d := iSeedDelivery(
t, s.WebhookDB, event.ID, target.ID, c.status,
)
var body *string
if c.inlineBody {
bodyStr := event.Body
body = &bodyStr
}
task := tsTask(
d, event, s.WebhookID, target, c.attemptNum, body,
)
c.process(context.TODO(), s.Engine, &task)
return s, d, sink.text(t)
}
// TestSlackFirstAttemptCarriesEventTimestamp covers the path an
// event takes on its first delivery: the task comes from the
// receiver and the engine reconstructs the event from it.
func TestSlackFirstAttemptCarriesEventTimestamp(t *testing.T) {
t.Parallel()
s, d, text := tsCase{
status: database.DeliveryStatusPending,
inlineBody: true,
attemptNum: 1,
process: func(
ctx context.Context,
e *delivery.Engine,
task *delivery.Task,
) {
e.ExportProcessNewTask(ctx, task)
},
}.run(t)
tsAssertRealTimestamp(t, text)
iAssertStatus(t, s.WebhookDB, d.ID,
database.DeliveryStatusDelivered,
)
}
// TestSlackFirstAttemptLargeBodyCarriesEventTimestamp covers the
// first-attempt path for an event whose body exceeded
// MaxInlineBodySize, so the task carries no body and the engine
// reads it back from the stored row.
func TestSlackFirstAttemptLargeBodyCarriesEventTimestamp(
t *testing.T,
) {
t.Parallel()
_, _, text := tsCase{
status: database.DeliveryStatusPending,
inlineBody: false,
attemptNum: 1,
process: func(
ctx context.Context,
e *delivery.Engine,
task *delivery.Task,
) {
e.ExportProcessNewTask(ctx, task)
},
}.run(t)
tsAssertRealTimestamp(t, text)
}
// TestSlackRetryCarriesEventTimestamp covers the retry path, which
// reconstructs the event from the same task the first attempt used.
func TestSlackRetryCarriesEventTimestamp(t *testing.T) {
t.Parallel()
s, d, text := tsCase{
status: database.DeliveryStatusRetrying,
inlineBody: true,
attemptNum: 2,
process: func(
ctx context.Context,
e *delivery.Engine,
task *delivery.Task,
) {
e.ExportProcessRetryTask(ctx, task)
},
}.run(t)
tsAssertRealTimestamp(t, text)
iAssertStatus(t, s.WebhookDB, d.ID,
database.DeliveryStatusDelivered,
)
}
// TestFormatSlackMessageOverTaskReconstructedEvent asserts on the
// formatted message directly, over the event the delivery paths
// reconstruct from a Task. It is the unit-level guard under the
// end-to-end tests: revert the CreatedAt population in hydrateEvent
// and this fails on the zero timestamp.
func TestFormatSlackMessageOverTaskReconstructedEvent(
t *testing.T,
) {
t.Parallel()
s := newISetup(t)
cfg := tsSlackConfig(t, tsUndeliverableHook)
target := tsSeedTarget(t, s.MainDB, s.WebhookID, cfg)
event := tsSeedEvent(t, s.WebhookDB, s.WebhookID)
d := iSeedDelivery(
t, s.WebhookDB, event.ID, target.ID,
database.DeliveryStatusPending,
)
bodyStr := event.Body
task := tsTask(d, event, s.WebhookID, target, 1, &bodyStr)
rebuilt, err := s.Engine.ExportEventForTask(
s.WebhookDB, &task,
)
require.NoError(t, err)
assert.False(t, rebuilt.CreatedAt.IsZero(),
"reconstructed event carries the zero time",
)
assert.Equal(t,
tsEventCreatedAt().UTC(), rebuilt.CreatedAt.UTC(),
)
tsAssertRealTimestamp(
t, delivery.FormatSlackMessage(&rebuilt),
)
}
// TestFormatSlackMessageZeroTimestamp asserts the rendering choice
// directly, without going through the engine: a zero CreatedAt (the
// shape a reaped-row fallback produces) renders as "unknown" rather
// than the year-1 zero time, while a real CreatedAt still renders as
// RFC3339.
func TestFormatSlackMessageZeroTimestamp(t *testing.T) {
t.Parallel()
zeroEvent := database.Event{
Method: http.MethodPost,
ContentType: testContentType,
Body: tsEventBody,
}
zeroText := delivery.FormatSlackMessage(&zeroEvent)
assert.NotContains(t, zeroText, "0001-01-01",
"slack message carries the zero-time year",
)
assert.Contains(t, zeroText, "*Timestamp:* `unknown`",
"slack message does not mark an unset receipt time as unknown",
)
nonZeroEvent := zeroEvent
nonZeroEvent.CreatedAt = tsEventCreatedAt()
nonZeroText := delivery.FormatSlackMessage(&nonZeroEvent)
assert.Contains(t, nonZeroText,
"*Timestamp:* `"+
tsEventCreatedAt().UTC().Format(time.RFC3339)+"`",
"slack message does not render a real receipt time as RFC3339",
)
}
// TestEventReconstructionSurvivesAReapedRow pins the fallback: an
// event row reaped by retention while its delivery still holds the
// body inline is still delivered, with the receipt time unset,
// rather than dropped.
func TestEventReconstructionSurvivesAReapedRow(t *testing.T) {
t.Parallel()
s := newISetup(t)
cfg := tsSlackConfig(t, tsUndeliverableHook)
target := tsSeedTarget(t, s.MainDB, s.WebhookID, cfg)
event := tsSeedEvent(t, s.WebhookDB, s.WebhookID)
d := iSeedDelivery(
t, s.WebhookDB, event.ID, target.ID,
database.DeliveryStatusPending,
)
bodyStr := event.Body
task := tsTask(d, event, s.WebhookID, target, 1, &bodyStr)
require.NoError(t, s.WebhookDB.Unscoped().Delete(
&database.Event{}, "id = ?", event.ID,
).Error)
rebuilt, err := s.Engine.ExportEventForTask(
s.WebhookDB, &task,
)
require.NoError(t, err)
assert.Equal(t, bodyStr, rebuilt.Body)
assert.True(t, rebuilt.CreatedAt.IsZero())
// A task with no inlined body has nothing left to deliver, so
// the same reaped row is an error there.
noBody := task
noBody.Body = nil
_, err = s.Engine.ExportEventForTask(s.WebhookDB, &noBody)
require.Error(t, err)
}
internal/delivery/export_test.go
-56
View File
@@ -101,19 +101,6 @@ func (e *Engine) ExportDeliverHTTP(
e.httpTarget.Deliver(ctx, webhookDB, d, task, e)
}
// ExportDeliverHTTPWithScheduler delivers via the http target, handing
// any retry to sched instead of the engine, so a test can see the
// delay each retry is given.
func (e *Engine) ExportDeliverHTTPWithScheduler(
ctx context.Context,
webhookDB *gorm.DB,
d *database.Delivery,
task *Task,
sched Scheduler,
) {
e.httpTarget.Deliver(ctx, webhookDB, d, task, sched)
}
// ExportDeliverDatabase delivers via the database target.
func (e *Engine) ExportDeliverDatabase(
webhookDB *gorm.DB, d *database.Delivery,
@@ -164,16 +151,6 @@ func (e *Engine) ExportProcessRetryTask(
e.processRetryTask(ctx, task)
}
// ExportEventForTask exposes the event reconstruction the delivery
// paths run: buildEventFromTask followed by hydrateEvent.
func (e *Engine) ExportEventForTask(
webhookDB *gorm.DB, task *Task,
) (database.Event, error) {
return e.hydrateEvent(
webhookDB, buildEventFromTask(task), task,
)
}
// ExportProcessDelivery exposes processDelivery.
func (e *Engine) ExportProcessDelivery(
ctx context.Context,
@@ -192,14 +169,6 @@ func (e *Engine) ExportGetCircuitBreaker(
return e.httpTarget.getCircuitBreaker(targetID)
}
// ExportSetCircuitBreaker makes cb the http target's circuit breaker
// for targetID, so a test can use one with a short cooldown.
func (e *Engine) ExportSetCircuitBreaker(
targetID string, cb *CircuitBreaker,
) {
e.httpTarget.circuitBreakers.Store(targetID, cb)
}
// ExportParseHTTPConfig exposes parseHTTPConfig.
func (e *Engine) ExportParseHTTPConfig(
configJSON string,
@@ -342,31 +311,6 @@ func (e *Engine) ExportRecoverRetryingDeliveries(
e.recoverRetryingDeliveries(webhookDB, webhookID)
}
// ExportFailMissingTarget exposes failMissingTarget, so a test can hand
// it a delivery as a batch read it earlier.
func (e *Engine) ExportFailMissingTarget(
webhookDB *gorm.DB,
webhookID string,
d *database.Delivery,
) {
e.failMissingTarget(webhookDB, webhookID, d)
}
// ExportSendRecoveredDeliveries exposes sendRecoveredDeliveries, so a
// test can hand it a target map that lacks a delivery's target.
func (e *Engine) ExportSendRecoveredDeliveries(
ctx context.Context,
webhookDB *gorm.DB,
deliveries []database.Delivery,
webhookID string,
targetMap map[string]database.Target,
settled map[string]struct{},
) {
e.sendRecoveredDeliveries(
ctx, webhookDB, deliveries, webhookID, targetMap, settled,
)
}
// ExportDeliveryCh returns the delivery channel.
func (e *Engine) ExportDeliveryCh() chan Task {
return e.deliveryCh
internal/delivery/inflight_test.go
-67
View File
@@ -273,73 +273,6 @@ func TestOwnershipIsReleasedAfterDelivery(t *testing.T) {
)
}
// TestNotifyAfterRecoveryDoesNotSendAgain is the startup race of
// https://git.eeqj.de/sneak/webhooker/issues/299. The receiver has
// written a delivery, restart recovery finds it pending, sends it and
// releases it, and only then does the receiver's Notify for it arrive.
// Nothing owns the delivery by then, so Notify takes it.
func TestNotifyAfterRecoveryDoesNotSendAgain(t *testing.T) {
t.Parallel()
targetID := uuid.New().String()
s := fSweepSetup(t, targetID, "recovered")
event := iSeedEvent(
t, s.WebhookDB, s.WebhookID, `{"recovered":true}`,
)
d := iSeedDelivery(
t, s.WebhookDB, event.ID, targetID,
database.DeliveryStatusPending,
)
s.Engine.ExportStart()
defer func() {
require.NoError(
t, s.Engine.ExportStop(context.Background()),
)
}()
// Restart recovery sends the delivery and lets it go.
iWaitForDelivered(t, s.WebhookDB, d.ID)
require.Eventually(
t,
func() bool {
return s.Engine.ExportInflightHeld() == 0
},
5*time.Second, 20*time.Millisecond,
)
body := event.Body
s.Engine.Notify([]delivery.Task{{
DeliveryID: d.ID,
EventID: event.ID,
WebhookID: s.WebhookID,
TargetID: targetID,
TargetName: "recovered",
TargetType: database.TargetTypeLog,
Body: &body,
EntrypointID: event.EntrypointID,
}})
// Notify took the delivery, and a worker releases it once it has
// run the task.
require.Eventually(
t,
func() bool {
return s.Engine.ExportInflightHeld() == 0
},
5*time.Second, 20*time.Millisecond,
)
assert.Len(
t, iResults(t, s.WebhookDB, d.ID), 1,
"the delivery was sent a second time",
)
}
// TestRetryingRecoverySkipsASuccessfulResult is the retrying-side twin
// of the pending reconcile. A second attempt that reached the receiver
// and whose status write then failed sits at retrying holding a
internal/delivery/metrics_test.go
+3 -4
View File
@@ -412,10 +412,9 @@ func TestDeliveryMetrics_BreakerBlockedIsNotAnAttempt(
s.Engine.ExportProcessRetryTask(context.TODO(), &blocked)
// The breaker refused it: rescheduled without rewriting the
// retrying status it already had, so the retry counter did not
// move, and nothing was attempted or timed.
assert.InDelta(t, retriesBefore,
// The breaker refused it: rescheduled, so the retry counter
// moved, but nothing was attempted or timed.
assert.InDelta(t, retriesBefore+1,
mCounter(t, reg, mRetries, mTypeHTTP), 0)
assert.InDelta(t, threshold,
mCounter(t, reg, mAttempts, mTypeHTTP), 0)
internal/delivery/redirect_test.go
+7 -14
View File
@@ -170,8 +170,7 @@ func TestDelivery_CrossOriginRedirectDropsOriginScopedHeaders(
// Stripping must not fire within the configured origin, or every
// destination that redirects its own path would lose its
// credential and start answering 401 — and would lose the inbound
// signature header the target endpoint verifies. webhooker's own
// receiver verifies no signature; it only forwards the header.
// signature the receiver verifies.
func TestDelivery_SameOriginRedirectKeepsOriginScopedHeaders(
t *testing.T,
) {
@@ -339,11 +338,10 @@ func TestRedirectPolicy_StopsAtHopCap(t *testing.T) {
// The set the redirect policy strips is whatever the delivery path
// actually put on the wire, so a header added to the forward set is
// covered without a second edit. A header the event never carried
// is not in the set, and neither is the inbound Content-Type, because
// it is not forwarded. Two more are deliberately excluded: a
// Content-Type configured on the target describes the body, which a
// 307 carries across hosts, and the inbound User-Agent every real
// sender supplies is overwritten before the request goes out.
// is not in the set, and the delivery path's own two are deliberately
// excluded: Content-Type describes the body, which a 307 carries
// across hosts, and the inbound User-Agent every real sender supplies
// is overwritten before the request goes out.
func TestApplyRequestHeaders_ReportsOriginScopedNames(t *testing.T) {
t.Parallel()
@@ -372,7 +370,6 @@ func TestApplyRequestHeaders_ReportsOriginScopedNames(t *testing.T) {
&delivery.HTTPTargetConfig{
Headers: map[string]string{
probeHeaderName: probeHeaderValue,
"Content-Type": testContentType,
},
},
)
@@ -380,11 +377,7 @@ func TestApplyRequestHeaders_ReportsOriginScopedNames(t *testing.T) {
assert.Equal(t,
[]string{probeHeaderName, inboundHeaderName}, names,
"both header classes are reported, and only those: "+
"Host and the inbound Content-Type are never "+
"forwarded, User-Agent is the delivery path's own",
)
assert.NotContains(t, names, "Content-Type",
"a Content-Type configured on the target must survive "+
"a cross-origin 307/308 with the body it describes",
"Host is never forwarded, Content-Type and "+
"User-Agent are the delivery path's own",
)
}
internal/delivery/ssrf.go
+7 -10
View File
@@ -26,7 +26,7 @@ var (
"hostname resolved to no IP addresses",
)
errBlockedIP = errors.New(
"blocked private, reserved or cloud metadata address",
"blocked private/reserved IP range",
)
errBlockedMetadata = errors.New(
"blocked link-local or cloud instance metadata " +
@@ -37,10 +37,9 @@ var (
)
)
// blockedNetworks is the default blocklist: the private and
// reserved IP ranges, plus the public cloud metadata addresses,
// that are blocked to prevent SSRF attacks. An operator can
// permit specific blocks out of this set with
// blockedNetworks contains all private/reserved IP ranges
// that should be blocked to prevent SSRF attacks. An operator
// can permit specific blocks out of this set with
// ALLOWED_EGRESS_CIDRS; see Guard.
//
//nolint:gochecknoglobals // package-level network list is appropriate here
@@ -123,8 +122,6 @@ func init() {
"::1/128",
"fc00::/7",
"fe80::/10",
// Azure WireServer, a public address that serves VM credentials.
"168.63.129.16/32",
})
// Every entry is named. The set must not grow or shrink
@@ -219,8 +216,8 @@ func matchesAny(networks []*net.IPNet, ip net.IP) bool {
}
// isBlockedIP checks whether an IP address falls within
// the default blocklist, before any operator allowlist is
// considered.
// any blocked private/reserved network range, before any
// operator allowlist is considered.
func isBlockedIP(ip net.IP) bool {
return matchesAny(blockedNetworks, ip)
}
@@ -323,7 +320,7 @@ func (g *Guard) allows(ip net.IP) bool {
//
// 1. alwaysBlockedNetworks is refused before the allowlist is
// consulted, so no configured CIDR reaches link-local or a
// cloud metadata endpoint at a non-public address.
// cloud instance metadata endpoint.
// 2. The allowlist is consulted next, so a listed private
// network becomes reachable.
// 3. Everything else keeps the default blocklist's answer.
internal/delivery/ssrf_allowlist_test.go
-35
View File
@@ -390,41 +390,6 @@ func TestGuardAllowlist_PublicUnaffected(t *testing.T) {
}
}
// TestGuardAllowlist_AzureWireServerReopenable covers Azure's
// WireServer, a public address that serves VM credentials. The
// default guard refuses it, but because it is public it sits in
// the default blocklist rather than the unconditional set, so an
// operator who lists it can reach it.
func TestGuardAllowlist_AzureWireServerReopenable(t *testing.T) {
t.Parallel()
const wireServerIP = "168.63.129.16"
target := "http://" + wireServerIP + "/?comp=versions"
defaultGuard := delivery.NewTestGuard()
err := defaultGuard.ValidateTargetURL(context.Background(), target)
require.Error(t, err,
"WireServer must be refused with no allowlist set",
)
assert.NotContains(t, err.Error(), metadataRefusalClause,
"WireServer must be refused by the default blocklist, "+
"which an allowlist can override",
)
assertDialRefused(t, defaultGuard, target)
listed := delivery.NewTestGuard(
netip.MustParsePrefix(wireServerIP + "/32"),
)
assert.NoError(t,
listed.ValidateTargetURL(context.Background(), target),
"an operator who lists WireServer must be able to reach it",
)
}
// TestGuardCheckIP_BothPathsShareOneDecision asserts that the
// validator and the dialer are not two policies that happen to
// agree: both are defined in terms of checkIP, so the exported
internal/delivery/target_database.go
-18
View File
@@ -277,24 +277,6 @@ func (t *databaseTarget) evict(webhookID string) {
)
}
// evictAll evicts every cached archive writer, exactly as evict
// does for one webhook. The engine calls it at shutdown, once its
// workers have returned. Closing the last handle on an archive
// moves the contents of its -wal into the .db and removes the
// -wal, so a clean stop leaves each archive as a single file.
func (t *databaseTarget) evictAll() {
t.mu.Lock()
writers := t.writers
t.writers = nil
t.mu.Unlock()
for _, w := range writers {
w.evict()
}
}
// sweepWebhook prunes one webhook's archive of rows older than
// expiry, without requiring a write. It returns nil (nothing to
// do) when the archive file does not exist, so a sweep never
internal/delivery/target_database_evict_test.go
-44
View File
@@ -1,7 +1,6 @@
package delivery_test
import (
"context"
"errors"
"fmt"
"net/http"
@@ -362,46 +361,3 @@ func TestEvictWebhook_LaterDeliveryRecreatesWriter(t *testing.T) {
"a later delivery should recreate the writer",
)
}
// TestEngineStop_WriteAfterStopIsRefused proves the engine's stop
// closes each archive writer the way deleting its webhook does: a
// write that reaches a writer after the stop is refused, reopens
// nothing and adds no row.
func TestEngineStop_WriteAfterStopIsRefused(t *testing.T) {
t.Parallel()
eng, _ := evictTestEngine(t)
webhookDB := testWebhookDB(t)
event := seedEvent(t, webhookDB, `{"archived":true}`)
d := seedDatabaseTargetDelivery(t, webhookDB, event, "")
eng.ExportDeliverDatabase(webhookDB, d)
w := eng.ExportArchiveWriterFor(event.WebhookID)
require.NotNil(t, w)
require.True(t, w.HandleOpen())
require.NoError(t, eng.ExportStop(context.Background()))
err := w.Write(evictTestRow("ev-after-stop"), 0)
require.ErrorIs(
t, err, delivery.ErrExportArchiveWriterEvicted,
"a write after the stop must be refused",
)
assert.False(
t, w.HandleOpen(),
"a refused write must not reopen the archive",
)
assert.False(
t, eng.ExportHasArchiveWriter(event.WebhookID),
"the stop should empty the registry",
)
count, err := countArchivedRows(w.Path())
require.NoError(t, err)
assert.Equal(
t, int64(1), count, "the refused row must not be written",
)
}
internal/delivery/target_headers_test.go
+1 -2
View File
@@ -11,11 +11,10 @@ import (
"sneak.berlin/go/webhooker/internal/delivery"
)
// Literals these tests repeat, named so that the header names and the
// Literals these tests repeat, named so that the header name and the
// keep-forever archive config each have one definition.
const (
headerAuthorization = "Authorization"
headerContentType = "Content-Type"
bearerValue = "Bearer abc"
archiveConfigNever = "{\"expiry\":\"never\"}"
)
internal/delivery/target_http.go
+8 -21
View File
@@ -197,14 +197,10 @@ func (c *httpCore) circuitBreakerBlock(
"cooldown_remaining", remaining,
)
// A delivery already at retrying is left as it is, so a task
// the breaker keeps turning away writes nothing each time.
if d.Status != database.DeliveryStatusRetrying {
c.eng.settleStatus(
webhookDB, d, d.Target.Type,
database.DeliveryStatusRetrying,
)
}
c.eng.settleStatus(
webhookDB, d, d.Target.Type,
database.DeliveryStatusRetrying,
)
retryTask := *task
sched.ScheduleRetry(retryTask, remaining)
@@ -541,11 +537,6 @@ func isForwardableHeader(name string) bool {
"Upgrade", "Proxy-Authorization",
"Proxy-Connection", "Content-Length":
return false
case "Content-Type":
// applyRequestHeaders sets Content-Type itself. The receiver
// already stored this inbound value as the event's
// ContentType, so forwarding it too would send it twice.
return false
default:
return true
}
@@ -558,10 +549,6 @@ func isForwardableHeader(name string) bool {
// policy strips exactly that set on a hop that leaves the origin,
// so the forward set is decided here and only here — a header added
// to it is covered off-origin without a second edit elsewhere.
//
// Content-Type goes out once: a Content-Type configured on the target
// wins, otherwise the event's ContentType, otherwise none. The inbound
// Content-Type in the event's headers is never forwarded.
func applyRequestHeaders(
req *http.Request,
event *database.Event,
@@ -582,10 +569,10 @@ func applyRequestHeaders(
req.Header.Set("User-Agent", "webhooker/1.0")
// A Content-Type configured on the target describes the body
// being sent rather than the sender. A 307/308 preserves the
// body across hosts, so stripping it would send that body
// untyped.
// Content-Type describes the body being sent rather than the
// sender, and the delivery path sets it from the event itself.
// A 307/308 preserves the body across hosts, so stripping it
// would send that body untyped.
delete(originScoped, "Content-Type")
// User-Agent is overwritten just above, so an inbound one never
internal/delivery/target_slack.go
+1 -6
View File
@@ -231,15 +231,10 @@ func FormatSlackMessage(
event.ContentType,
)
timestamp := "unknown"
if !event.CreatedAt.IsZero() {
timestamp = event.CreatedAt.UTC().Format(time.RFC3339)
}
fmt.Fprintf(
&b,
"*Timestamp:* `%s`\n",
timestamp,
event.CreatedAt.UTC().Format(time.RFC3339),
)
fmt.Fprintf(
internal/delivery/terminal_state_test.go
+9 -270
View File
@@ -18,17 +18,16 @@ import (
// https://git.eeqj.de/sneak/webhooker/issues/107: a delivery failed
// with nothing in its event log to say why, and a retrying delivery
// whose target was deleted, which used to keep sending and then never
// terminalise. Section 4 is the same deleted-target gap for a pending
// delivery: https://git.eeqj.de/sneak/webhooker/issues/293.
// terminalise.
// tUnknownType is a target type no build implements. It stands in for
// a target whose type was written by a build that knew a type this one
// does not.
const tUnknownType = database.TargetType("pubsub")
// tSeedDeletedTarget creates a target, a delivery against it at the
// given status with one recorded failed attempt, and then deletes the
// target the way the source page does.
// tSeedDeletedTarget creates a target, a retrying delivery against it
// with one recorded failed attempt, and then deletes the target the
// way the source page does.
//
// It asserts the delete is soft, because that is the whole reason the
// engine could not tell a deleted target from a target id that never
@@ -37,7 +36,6 @@ func tSeedDeletedTarget(
t *testing.T,
s iSetup,
name, url string,
status database.DeliveryStatus,
) string {
t.Helper()
@@ -53,7 +51,8 @@ func tSeedDeletedTarget(
)
d := iSeedDelivery(
t, s.WebhookDB, event.ID, targetID, status,
t, s.WebhookDB, event.ID, targetID,
database.DeliveryStatusRetrying,
)
iSeedFailedResult(t, s.WebhookDB, d.ID)
@@ -174,7 +173,6 @@ func TestRecoverSingleRetry_TargetDeleted(t *testing.T) {
deliveryID := tSeedDeletedTarget(
t, s, "gone-on-recovery", "http://example.com/hook",
database.DeliveryStatusRetrying,
)
s.Engine.ExportRecoverWebhookDeliveries(
@@ -212,7 +210,6 @@ func TestSweepSingleRetry_TargetDeleted(t *testing.T) {
deliveryID := tSeedDeletedTarget(
t, s, "gone-on-sweep", "http://example.com/hook",
database.DeliveryStatusRetrying,
)
// Twice, because the bug was an error the sweep repeated every
@@ -281,11 +278,11 @@ func TestSweepSingleRetry_TargetNeverExisted(t *testing.T) {
)
}
// TestFailMissingTarget_WritesNoTargetRow holds the new terminal path
// to the same rule as the existing one: no target row, and so no
// TestFailMissingTargetRetry_WritesNoTargetRow holds the new terminal
// path to the same rule as the existing one: no target row, and so no
// plaintext target config, may be written into the per-webhook event
// database. See https://git.eeqj.de/sneak/webhooker/issues/206.
func TestFailMissingTarget_WritesNoTargetRow(
func TestFailMissingTargetRetry_WritesNoTargetRow(
t *testing.T,
) {
t.Parallel()
@@ -300,7 +297,6 @@ func TestFailMissingTarget_WritesNoTargetRow(
deliveryID := tSeedDeletedTarget(
t, s, "credential-bearing", hookURL,
database.DeliveryStatusRetrying,
)
s.Engine.ExportSweepWebhookRetries(
@@ -533,260 +529,3 @@ func TestRecoverSingleRetry_TargetUnreadable_LeavesDeliveryAlone(
assert.Zero(t, s.Engine.ExportInflightHeld())
}
// --- 4. A pending delivery whose target is gone ---
func TestRecoverPending_TargetDeleted(t *testing.T) {
t.Parallel()
s := newISetup(t)
deliveryID := tSeedDeletedTarget(
t, s, "gone-while-pending", "http://example.com/hook",
database.DeliveryStatusPending,
)
s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID,
)
iAssertStatus(
t, s.WebhookDB, deliveryID,
database.DeliveryStatusFailed,
)
last := tLastResult(t, s, deliveryID, 2)
assert.False(t, last.Success)
assert.Equal(t, 2, last.AttemptNum)
assert.Contains(t, last.Error, "gone-while-pending")
assert.Contains(t, last.Error, "was deleted")
assert.Empty(t, fDrain(s.Engine),
"a delivery whose target is gone was sent",
)
assert.Zero(t, s.Engine.ExportInflightHeld(),
"the terminal path leaked its ownership reference",
)
}
// TestRecoverPending_TargetDeleted_LeavesAnOwnedDeliveryAlone: the
// terminal write takes ownership like every other recovery write, so a
// delivery the engine still holds is not failed underneath its worker.
func TestRecoverPending_TargetDeleted_LeavesAnOwnedDeliveryAlone(
t *testing.T,
) {
t.Parallel()
s := newISetup(t)
deliveryID := tSeedDeletedTarget(
t, s, "gone-but-owned", "http://example.com/hook",
database.DeliveryStatusPending,
)
require.True(t, s.Engine.ExportRetainDelivery(deliveryID))
s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID,
)
iAssertStatus(
t, s.WebhookDB, deliveryID,
database.DeliveryStatusPending,
)
assert.Len(t, iResults(t, s.WebhookDB, deliveryID), 1,
"a delivery the engine owns was failed underneath it",
)
}
// TestFailMissingTarget_LeavesASettledDeliveryAlone: the recovery paths
// read their batch before taking ownership, and a worker may send a
// delivery and let it go in between. The terminal write goes by the row
// as it is now, not as the batch read it.
func TestFailMissingTarget_LeavesASettledDeliveryAlone(
t *testing.T,
) {
t.Parallel()
s := newISetup(t)
deliveryID := tSeedDeletedTarget(
t, s, "gone-after-sending", "http://example.com/hook",
database.DeliveryStatusPending,
)
var batch database.Delivery
require.NoError(t, s.WebhookDB.First(
&batch, "id = ?", deliveryID,
).Error)
// A worker settles the delivery after the batch was read.
require.NoError(t, s.WebhookDB.Model(&database.Delivery{}).
Where("id = ?", deliveryID).
Update("status", database.DeliveryStatusDelivered).Error)
s.Engine.ExportFailMissingTarget(
s.WebhookDB, s.WebhookID, &batch,
)
iAssertStatus(
t, s.WebhookDB, deliveryID,
database.DeliveryStatusDelivered,
)
assert.Len(t, iResults(t, s.WebhookDB, deliveryID), 1,
"a delivery settled after the batch read was then failed",
)
assert.Zero(t, s.Engine.ExportInflightHeld())
}
// TestSweepPending_TargetDeleted sweeps twice over a batch that also
// holds a healthy stranded delivery. The one whose target is gone is
// failed once and then left alone; the healthy one is queued by the
// first sweep and not again by the second.
func TestSweepPending_TargetDeleted(t *testing.T) {
t.Parallel()
liveTargetID := uuid.New().String()
s := fSweepSetup(t, liveTargetID, "still-there")
deliveryID := tSeedDeletedTarget(
t, s, "gone-on-pending-sweep", "http://example.com/hook",
database.DeliveryStatusPending,
)
rAgePending(t, s.WebhookDB, deliveryID)
event := iSeedEvent(
t, s.WebhookDB, s.WebhookID, `{"target":"live"}`,
)
healthy := iSeedDelivery(
t, s.WebhookDB, event.ID, liveTargetID,
database.DeliveryStatusPending,
)
rAgePending(t, s.WebhookDB, healthy.ID)
ctx := context.Background()
s.Engine.ExportSweepWebhookRetries(ctx, s.WebhookID)
tasks := fDrain(s.Engine)
require.Len(t, tasks, 1,
"the first sweep did not queue the healthy delivery",
)
assert.Equal(t, healthy.ID, tasks[0].DeliveryID)
s.Engine.ExportSweepWebhookRetries(ctx, s.WebhookID)
assert.Empty(t, fDrain(s.Engine),
"the second sweep queued a delivery again",
)
iAssertStatus(
t, s.WebhookDB, deliveryID,
database.DeliveryStatusFailed,
)
last := tLastResult(t, s, deliveryID, 2)
assert.Contains(t, last.Error, "gone-on-pending-sweep")
assert.Contains(t, last.Error, "was deleted")
}
// TestSendRecoveredDeliveries_TargetMissingFromMap: the batch's target
// map is empty when its query failed, so every delivery in the batch is
// looked up on its own. A healthy one is sent to the target that lookup
// finds.
func TestSendRecoveredDeliveries_TargetMissingFromMap(
t *testing.T,
) {
t.Parallel()
s := newISetup(t)
targetID := uuid.New().String()
iCreateTarget(
t, s.MainDB, targetID, s.WebhookID, "found-on-lookup",
database.TargetTypeLog, "", 0,
)
event := iSeedEvent(
t, s.WebhookDB, s.WebhookID, `{"map":"empty"}`,
)
d := iSeedDelivery(
t, s.WebhookDB, event.ID, targetID,
database.DeliveryStatusPending,
)
s.Engine.ExportSendRecoveredDeliveries(
context.Background(), s.WebhookDB,
[]database.Delivery{d}, s.WebhookID,
map[string]database.Target{}, nil,
)
tasks := fDrain(s.Engine)
require.Len(t, tasks, 1,
"the healthy delivery was not queued exactly once",
)
assert.Equal(t, d.ID, tasks[0].DeliveryID)
assert.Equal(t, targetID, tasks[0].TargetID)
assert.Equal(t, database.TargetTypeLog, tasks[0].TargetType)
iAssertStatus(
t, s.WebhookDB, d.ID,
database.DeliveryStatusPending,
)
}
// TestRecoverPending_TargetUnreadable_LeavesDeliveryAlone: a failed
// read of the main database is not a deleted target. Restart recovery
// holds every pending delivery of the webhook in one batch, so failing
// on this would fail all of them.
func TestRecoverPending_TargetUnreadable_LeavesDeliveryAlone(
t *testing.T,
) {
t.Parallel()
s := newISetup(t)
targetID := uuid.New().String()
iCreateTarget(
t, s.MainDB, targetID, s.WebhookID, "healthy",
database.TargetTypeLog, "", 0,
)
event := iSeedEvent(
t, s.WebhookDB, s.WebhookID, `{"still":"pending"}`,
)
d := iSeedDelivery(
t, s.WebhookDB, event.ID, targetID,
database.DeliveryStatusPending,
)
sqlDB, err := s.MainDB.DB()
require.NoError(t, err)
require.NoError(t, sqlDB.Close())
s.Engine.ExportRecoverPendingDeliveries(
context.Background(), s.WebhookDB, s.WebhookID,
)
iAssertStatus(
t, s.WebhookDB, d.ID,
database.DeliveryStatusPending,
)
assert.Empty(t, iResults(t, s.WebhookDB, d.ID),
"an unreadable main database produced a terminal "+
"failure row",
)
assert.Empty(t, fDrain(s.Engine))
assert.Zero(t, s.Engine.ExportInflightHeld())
}
internal/handlers/ui_copy_test.go
-77
View File
@@ -300,80 +300,3 @@ func TestEntrypointCopyButtonIsProgressiveEnhancement(t *testing.T) {
"the page must render to completion, not abort partway",
)
}
// maxRetriesHelp is the wording both target forms must carry. The
// delivery core makes max_retries attempts in total, not that many
// retries on top of a first try (a fresh delivery starts at attempt 1
// and target_http gives up once the attempt number reaches
// max_retries), and 0 is special-cased to a single fire-and-forget
// attempt with no circuit breaker.
const maxRetriesHelp = "This is the total number of delivery attempts, " +
"not retries on top of the first: a value of 3 makes three attempts " +
"in all. 0 means a single attempt with no retries and no circuit " +
"breaker."
// TestTargetFormMaxRetriesCopyMatchesBehaviour pins the max_retries
// help text on both the create form (the add-target form on the webhook
// detail page) and the edit form, so the copy cannot drift back to
// calling the number a retry count.
func TestTargetFormMaxRetriesCopyMatchesBehaviour(t *testing.T) {
t.Parallel()
var h *handlers.Handlers
var sess *session.Session
app := newTestApp(t, &h, &sess)
app.RequireStart()
t.Cleanup(app.RequireStop)
webhook := &database.Webhook{Name: "wh", RetentionDays: 14}
webhook.ID = testWebhookID
entrypoint := database.Entrypoint{Path: "abc123"}
entrypoint.ID = "ep-1"
createBody := renderPage(
t, h, sess, "source_detail.html", map[string]any{
dataKeyWebhook: webhook,
"Entrypoints": handlers.NewEntrypointViews(
[]database.Entrypoint{entrypoint},
),
"Targets": delivery.NewTargetViews(nil),
"Events": []database.Event{},
"BaseURL": "https://hooks.example.com",
},
)
assert.Contains(
t, createBody, maxRetriesHelp,
"the add-target form must explain max_retries as total attempts",
)
// A slack target exercises the same max_retries field while needing
// only Config.URL from the edit template, so the test data stays
// minimal. The Target key mirrors the field names the template reads
// off the handler's view value.
editBody := renderPage(
t, h, sess, "target_edit.html", map[string]any{
dataKeyWebhook: webhook,
"Target": map[string]any{
"ID": "tg-1",
"Name": "t",
"Type": "slack",
"Active": true,
"MaxRetries": 3,
"Config": map[string]any{
"URL": "https://hooks.slack.com/services/x",
},
},
dataKeyError: "",
},
)
assert.Contains(
t, editBody, maxRetriesHelp,
"the target edit form must explain max_retries as total attempts",
)
}
internal/middleware/csrf_test.go
+3 -2
View File
@@ -380,8 +380,9 @@ func csrfTookStrictPath(
// TestCSRF_ForwardedProtoSpellingsTakeStrictPath runs the header
// spellings a real proxy emits through the middleware. The environment
// is set to dev -- the permissive setting -- to pin that the routing is
// a per-request transport decision and owes nothing to configuration.
// is dev -- the DEFAULT when WEBHOOKER_ENVIRONMENT is unset -- to pin
// that the routing is a per-request transport decision and owes
// nothing to configuration.
func TestCSRF_ForwardedProtoSpellingsTakeStrictPath(t *testing.T) {
t.Parallel()
internal/middleware/recoverer.go
-7
View File
@@ -133,11 +133,6 @@ func (w *recoverResponseWriter) Unwrap() http.ResponseWriter {
// what the access log records and the metrics count, and outside the
// sentryhttp handler, whose Repanic option depends on something
// further out recovering what it re-raises.
//
// Unlike http.Error on its own, it deletes any Set-Cookie the handler
// set before panicking, because a request that failed must not hand
// the client a credential; every other header is left to http.Error.
// See https://git.eeqj.de/sneak/webhooker/issues/193.
func (s *Middleware) Recoverer() func(http.Handler) http.Handler {
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(
@@ -169,8 +164,6 @@ func (s *Middleware) Recoverer() func(http.Handler) http.Handler {
return
}
rw.Header().Del("Set-Cookie")
http.Error(
rw,
http.StatusText(
internal/middleware/recoverer_test.go
+2 -31
View File
@@ -304,44 +304,16 @@ func TestRecovererRepanicsErrAbortHandler(t *testing.T) {
)
}
// TestRecovererDropsSetCookieFromTheRecovered500 covers a handler that
// sets a cookie and a redirect target and then panics before sending
// anything. A request that failed must not hand the client a
// credential, so the 500 carries no cookie; Location is left alone.
func TestRecovererDropsSetCookieFromTheRecovered500(t *testing.T) {
t.Parallel()
probe := newRecovererProbe(
t, false,
func(w http.ResponseWriter, _ *http.Request) {
w.Header().Set("Set-Cookie", "session=x")
w.Header().Set("Location", "/after")
panic(panicMarker)
},
)
resp, err := probe.get(t)
require.NoError(t, err)
require.NoError(t, resp.Body.Close())
assert.Equal(t, http.StatusInternalServerError, resp.StatusCode)
assert.Empty(t, resp.Cookies())
assert.Equal(t, "/after", resp.Header.Get("Location"))
}
// TestRecovererKeepsAnAlreadyCommittedResponse covers a handler that
// panics after sending its status. The bytes are already on the wire,
// cookie included, so a second WriteHeader would change nothing the
// client sees and would draw net/http's "superfluous
// response.WriteHeader" report.
// so a second WriteHeader would change nothing the client sees and
// would draw net/http's "superfluous response.WriteHeader" report.
func TestRecovererKeepsAnAlreadyCommittedResponse(t *testing.T) {
t.Parallel()
probe := newRecovererProbe(
t, false,
func(w http.ResponseWriter, _ *http.Request) {
w.Header().Set("Set-Cookie", "session=x")
w.WriteHeader(committedStatus)
_, _ = w.Write([]byte("partial"))
@@ -359,7 +331,6 @@ func TestRecovererKeepsAnAlreadyCommittedResponse(t *testing.T) {
assert.Equal(t, committedStatus, resp.StatusCode)
assert.Equal(t, "partial", string(body))
assert.Len(t, resp.Cookies(), 1)
record := probe.panicRecord(t)
assert.Equal(t, panicMarker, record["panic"])
internal/reqtls/reqtls.go
+1 -1
View File
@@ -5,7 +5,7 @@
// several packages, by hand, and the answers disagreed. The session
// cookie's Secure attribute was decided at startup from the configured
// environment while the CSRF cookie's was decided per-request, so a
// deployment behind a TLS proxy in the dev environment emitted one
// deployment behind a TLS proxy in the default environment emitted one
// Secure cookie and one non-Secure cookie on the same response.
// Everything kept working, which is exactly why nobody noticed.
//
internal/resetpw/resetpw_test.go
+1 -1
View File
@@ -140,7 +140,7 @@ func (n *noopEvictor) EvictWebhook(string) {}
// and the database, exactly as internal/handlers builds them.
//
// One application per test function, not per case: every start that
// finds no account seeds one with an Argon2id hash, and this package's
// finds no account seeds one at 64 MB of Argon2id, and this package's
// budget is not the place to spend that repeatedly.
func newServerApp(
t *testing.T, dir string,
internal/server/routes.go
+3 -17
View File
@@ -92,25 +92,11 @@ func (s *Server) setupGlobalMiddleware() {
func (s *Server) setupRoutes() {
s.router.Get("/", s.h.HandleIndex())
// Static assets answer GET and HEAD only. chi's default 405
// carries no Allow header, so this group supplies its own.
staticFiles := http.StripPrefix(
"/s", http.FileServer(http.FS(static.Static)),
s.router.Mount(
"/s",
http.StripPrefix("/s", http.FileServer(http.FS(static.Static))),
)
s.router.Route("/s", func(r chi.Router) {
r.MethodNotAllowed(func(w http.ResponseWriter, _ *http.Request) {
w.Header().Set("Allow", "GET, HEAD")
http.Error(
w,
"Method Not Allowed",
http.StatusMethodNotAllowed,
)
})
r.Method(http.MethodGet, "/*", staticFiles)
r.Method(http.MethodHead, "/*", staticFiles)
})
s.router.Route("/api/v1", func(_ chi.Router) {
// API routes will be added here.
})
internal/server/routes_test.go
+19 -108
View File
@@ -7,7 +7,6 @@ import (
"net/http/httptest"
"net/url"
"regexp"
"slices"
"strconv"
"strings"
"testing"
@@ -221,21 +220,9 @@ func (e *testEnv) csrfFrom(
// out of the markup has to be unescaped before it is submitted.
token := html.UnescapeString(match[1])
// A cookie the page sets replaces the one of the same name, as in
// a browser. Sent both, the server would read the first, older one.
set := w.Result().Cookies()
combined := make([]*http.Cookie, 0, len(cookies)+len(set))
for _, c := range cookies {
replaced := slices.ContainsFunc(set, func(n *http.Cookie) bool {
return n.Name == c.Name
})
if !replaced {
combined = append(combined, c)
}
}
combined = append(combined, set...)
combined := make([]*http.Cookie, 0, len(cookies))
combined = append(combined, cookies...)
combined = append(combined, w.Result().Cookies()...)
return token, combined
}
@@ -409,15 +396,13 @@ func (e *testEnv) storedHash(t *testing.T, username string) string {
// --- /s static group ---
// TestStaticServesOnlyGetAndHead pins the methods the static group
// answers: GET and HEAD are served the asset, and the other methods
// chi routes (POST, PUT, DELETE and the rest) are refused with 405
// and an Allow header naming those two. A method chi does not route,
// such as PROPFIND, is refused with 405 by the top-level router
// before it reaches the static group, so it gets no Allow header.
// The README documents this; the test is what keeps the two from
// drifting.
func TestStaticServesOnlyGetAndHead(t *testing.T) {
// TestStaticServesEveryMethod pins what the static mount actually
// answers. chi's Mount registers the handler for all methods and
// http.FileServer only special-cases HEAD (by suppressing the body),
// so a POST or a DELETE to an asset is served the file rather than
// refused. The README documents this; the test is what keeps the two
// from drifting.
func TestStaticServesEveryMethod(t *testing.T) {
t.Parallel()
env := newTestEnv(t)
@@ -432,7 +417,6 @@ func TestStaticServesOnlyGetAndHead(t *testing.T) {
http.MethodPost,
http.MethodPut,
http.MethodDelete,
"PROPFIND",
} {
t.Run(method, func(t *testing.T) {
t.Parallel()
@@ -444,38 +428,18 @@ func TestStaticServesOnlyGetAndHead(t *testing.T) {
w := httptest.NewRecorder()
env.router.ServeHTTP(w, req)
switch method {
case http.MethodGet:
assert.Equal(t, http.StatusOK, w.Code)
assert.Equal(t, body, w.Body.Bytes(),
"the asset itself is returned")
case http.MethodHead:
assert.Equal(t, http.StatusOK, w.Code)
assert.Equal(t, http.StatusOK, w.Code,
"static mount answers every method")
if method == http.MethodHead {
assert.Empty(t, w.Body.Bytes(),
"HEAD must not carry a body")
case "PROPFIND":
assert.Equal(
t, http.StatusMethodNotAllowed, w.Code,
)
assert.Empty(t, w.Header().Get("Allow"),
"chi refuses a method it does not route "+
"before the static group runs")
assert.NotContains(
t, w.Body.String(), string(body),
"a refused method must not get the asset",
)
default:
assert.Equal(
t, http.StatusMethodNotAllowed, w.Code,
)
assert.Equal(
t, "GET, HEAD", w.Header().Get("Allow"),
)
assert.NotContains(
t, w.Body.String(), string(body),
"a refused method must not get the asset",
)
return
}
assert.Equal(t, body, w.Body.Bytes(),
"the asset itself is returned")
})
}
}
@@ -627,59 +591,6 @@ func TestPagesLogin_CorrectPasswordSurvivesASpentBudget(
)
}
// TestPagesLogin_CookiesFromAnEarlierDatabase is
// https://git.eeqj.de/sneak/webhooker/issues/359. A new database
// brings a new session key, and the operator's browser still holds
// the session and CSRF cookies signed with the old one. Logging in
// must work as from a fresh browser and leave cookies the new key
// accepts.
func TestPagesLogin_CookiesFromAnEarlierDatabase(t *testing.T) {
t.Parallel()
const (
username = "operator"
password = "correct-horse-battery-staple"
)
earlier := newTestEnv(t)
earlierID, _ := earlier.seedUser(t, username, password)
_, stale := earlier.csrfFrom(t, "/pages/login", nil)
stale = append(stale, earlier.authCookies(t, earlierID, username)...)
env := newTestEnv(t)
env.seedUser(t, username, password)
token, cookies := env.csrfFrom(t, "/pages/login", stale)
form := url.Values{}
form.Set("csrf_token", token)
form.Set("username", username)
form.Set("password", password)
w := env.post("/pages/login", form, cookies)
require.Equal(
t, http.StatusSeeOther, w.Code,
"a session cookie from another key must not fail the login",
)
// The response deletes the old session cookie and then sets the
// new one; a browser keeps the last.
var fresh *http.Cookie
for _, c := range w.Result().Cookies() {
if c.Name == session.SessionName {
fresh = c
}
}
require.NotNil(t, fresh, "login must set a session cookie")
assert.Equal(
t, "/sources",
env.get("/", []*http.Cookie{fresh}).Header().Get("Location"),
"the new session cookie must authenticate",
)
}
// --- /user/{username} group ---
// TestPasswordChange_OversizeBody_RejectedAndPasswordUnchanged
internal/session/codec_test.go
+7 -8
View File
@@ -19,8 +19,8 @@ import (
)
// The tests below exercise the securecookie codecs underneath the
// store and nothing else: they decode through the store itself, so no
// server-side expiry check takes part in the result. They exist because
// store and nothing else: Session.Get only decodes, so no server-side
// expiry check takes part in the result. They exist because
// NewCookieStore gives its codecs a 30-day max age that assigning
// store.Options does not override, which would let the codec accept a
// cookie weeks past the cap the cookie attribute advertises.
@@ -75,11 +75,10 @@ func restamp(
return base64.URLEncoding.EncodeToString(payload)
}
// decodeCookie feeds value back through the store's decode path. It
// asks the store rather than Session.Get, which treats a cookie that
// does not decode as absent and so hides the codec's reason.
// decodeCookie feeds value back through the store's decode path.
func decodeCookie(
t *testing.T,
s *session.Session,
value string,
) (*sessions.Session, error) {
t.Helper()
@@ -95,7 +94,7 @@ func decodeCookie(
SameSite: http.SameSiteLaxMode,
})
sess, err := session.NewStore(testKey()).Get(req, session.SessionName)
sess, err := s.Get(req)
require.NotNil(t, sess)
return sess, err
@@ -106,7 +105,7 @@ func TestCodec_AcceptsCookieInsideAbsoluteCap(t *testing.T) {
s := testSession(t)
sess, err := decodeCookie(t, restamp(
sess, err := decodeCookie(t, s, restamp(
t,
issuedCookie(t, s),
time.Now().Add(-(testAbsoluteMaxAge-time.Hour)),
@@ -127,7 +126,7 @@ func TestCodec_RejectsCookiePastAbsoluteCap(t *testing.T) {
s := testSession(t)
sess, err := decodeCookie(t, restamp(
sess, err := decodeCookie(t, s, restamp(
t,
issuedCookie(t, s),
time.Now().Add(-(testAbsoluteMaxAge+time.Hour)),
internal/session/session.go
+3 -15
View File
@@ -146,8 +146,8 @@ func newStore(key []byte) *sessions.CookieStore {
//
// This is decided per-request, not once at startup. Deciding it at
// startup from the configured environment is what this replaces, and
// it got the DEFAULT posture wrong: "dev" was then the environment when
// WEBHOOKER_ENVIRONMENT was unset, so a deployment terminating TLS at a
// it got the DEFAULT posture wrong: "dev" is the environment when
// WEBHOOKER_ENVIRONMENT is unset, so a deployment terminating TLS at a
// proxy without also setting the environment emitted the
// authentication cookie with no Secure attribute -- silently, and on
// the same response as a CSRF cookie that did have one.
@@ -224,22 +224,10 @@ func New(
}
// Get retrieves a session for the request.
//
// A session cookie that does not decode -- one signed with an earlier
// session key, say, because the database was made anew -- is treated
// as absent: the caller gets a new, empty session and no error, and
// the next save replaces the cookie.
func (s *Session) Get(
r *http.Request,
) (*sessions.Session, error) {
sess, err := s.store.Get(r, SessionName)
if sess == nil {
return nil, err
}
// For a cookie that does not decode, gorilla/sessions returns a
// new, empty session alongside the error that is dropped here.
return sess, nil
return s.store.Get(r, SessionName)
}
// GetKey returns the raw 32-byte authentication key used for
internal/session/session_test.go
+2 -2
View File
@@ -990,8 +990,8 @@ func sessionCookieFrom(
// TestSave_SecureFollowsRequestTransport is the regression test for
// the defect this replaces: Secure was fixed at startup from the
// configured environment, and "dev" was then the environment when
// WEBHOOKER_ENVIRONMENT was unset. A deployment behind a TLS proxy in
// configured environment, and "dev" is the environment when
// WEBHOOKER_ENVIRONMENT is unset. A deployment behind a TLS proxy in
// that DEFAULT posture shipped the authentication cookie with no
// Secure attribute and said nothing about it.
//
script/test
+1 -11
View File
@@ -22,23 +22,13 @@
# The one figure above 90s is GOMAXPROCS 1, a synthetic core floor rather than
# a condition CI runs under. If a CPU-limited runner ever puts a real run near
# 67s, that is the datum to revisit the org figure with.
#
# -p 4 -parallel 8 keep the run under 2 GB of memory: at most four test
# binaries build or run at once, each with at most eight parallel tests. Under
# -race every test binary and every link costs a few hundred MB, so the
# defaults (one per core) add up to several GB on a many-core host.
#
# No -v: the Docker build cuts each step's log off at 2 MiB, and verbose output
# from the whole suite passes that before a failure is printed. Without it, go
# test prints one result line per package and, for a package that fails,
# everything its tests wrote, application log lines included.
set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
main() {
cd "$ROOT"
go test -race -p 4 -parallel 8 -timeout 90s ./...
go test -v -race -timeout 90s ./...
}
main "$@"
templates/source_detail.html
+3 -6
View File
@@ -120,12 +120,9 @@
<label class="text-sm text-gray-700">Timeout (seconds, blank = default):</label>
<input type="number" name="timeout" min="0" max="300" :disabled="targetType !== 'http'" class="input text-sm w-24">
</div>
<div x-show="targetType === 'http'">
<div class="flex gap-2 items-center">
<label class="text-sm text-gray-700">Max retries:</label>
<input type="number" name="max_retries" value="0" min="0" max="20" class="input text-sm w-24">
</div>
<p class="text-xs text-gray-500 mt-1">This is the total number of delivery attempts, not retries on top of the first: a value of 3 makes three attempts in all. 0 means a single attempt with no retries and no circuit breaker.</p>
<div x-show="targetType === 'http'" class="flex gap-2 items-center">
<label class="text-sm text-gray-700">Max retries (0 = fire-and-forget):</label>
<input type="number" name="max_retries" value="0" min="0" max="20" class="input text-sm w-24">
</div>
<div x-show="targetType === 'slack'">
<input type="url" name="url" placeholder="https://hooks.slack.com/services/..." :disabled="targetType !== 'slack'" class="input text-sm">
templates/target_edit.html
+1 -1
View File
@@ -69,7 +69,7 @@
<div class="form-group">
<label for="max_retries" class="label">Max retries</label>
<input type="number" id="max_retries" name="max_retries" value="{{.Target.MaxRetries}}" min="0" max="20" class="input">
<p class="text-xs text-gray-500 mt-1">This is the total number of delivery attempts, not retries on top of the first: a value of 3 makes three attempts in all. 0 means a single attempt with no retries and no circuit breaker.</p>
<p class="text-xs text-gray-500 mt-1">0 is fire-and-forget: one attempt, no circuit breaker.</p>
</div>
{{end}}