Author SHA1 Message Date
sneak 306c2cb9b1 Send Content-Type once on a delivery (closes #246)
check / check (push) Successful in 4m3s
A delivery set Content-Type from the event's ContentType and then
added the inbound Content-Type from the event's stored headers, so
the target could receive two values. The inbound Content-Type is no
longer forwarded from the stored headers; the receiver already saved
it as the event's ContentType.

Which value wins is now stated at applyRequestHeaders: a Content-Type
configured on the target, otherwise the event's ContentType,
otherwise none.

Model: opus-5-5
2026-09-29 03:56:16 +00:00
28 changed files with 384 additions and 789 deletions
+4 -2
View File
@@ -3,8 +3,10 @@
# stage of the Dockerfile.
.git/
bin/
# Extracted from 3p/ by `make assets` inside the build; a host copy is not
# needed. The tarball in 3p/ must stay in the context.
# Third-party browser assets are fetched and hash-verified inside the build by
# script/fetch-assets. Excluding any host copy keeps a developer's working tree
# from supplying the bytes that get shipped. The script and its
# static/vendor.sha256 manifest stay in the context.
static/js/alpine.min.js
*.md
LICENSE
+3 -2
View File
@@ -46,6 +46,7 @@ temp/
# CI cache barrier, written into the build context by the check workflow
.ci-fingerprint
# Alpine.js, extracted by `make assets` from its tarball in 3p/, which is
# what is committed.
# Third-party browser assets, fetched and hash-verified by
# script/fetch-assets against static/vendor.sha256. Not committed:
# REPO_POLICIES.md forbids minified bundles in version control.
/static/js/alpine.min.js
Binary file not shown.
+11 -4
View File
@@ -51,8 +51,15 @@ RUN go mod download
# the lint stage above.
COPY . .
# Run tests and build. Both first run script/assets, which extracts Alpine.js
# from its tarball in 3p/.
# Fetch the third-party browser assets the UI serves. They are not committed
# (REPO_POLICIES.md forbids minified bundles in version control) and
# .dockerignore keeps any host copy out of the build context, so this step is
# the only way they enter the image. Each download is checked against a
# hardcoded sha256 and the build fails on mismatch; make test re-checks the
# hashes against the bytes go:embed actually put in the binary.
RUN script/fetch-assets
# Run tests and build
RUN make test
# Version stamped into the binary. .dockerignore excludes .git/, so
@@ -60,8 +67,8 @@ RUN make test
# host and passes it in. The default is what a bare `docker build .`
# with no --build-arg gets, and it names no tag the tree may not be at.
#
# Declared here, below the test step, so a changed version does not
# invalidate its cached layer.
# Declared here, below the test and asset steps, so a changed version
# does not invalidate their cached layers.
ARG VERSION=unknown
RUN make build VERSION="$VERSION"
+3 -3
View File
@@ -28,7 +28,7 @@ setup:
@script/setup
assets:
@script/assets
@script/fetch-assets
test:
@script/test
@@ -45,13 +45,13 @@ fmt-check:
check:
@script/check
build: assets
build:
go build -ldflags '$(strip -X main.version=$(VERSION) $(GO_LDFLAGS))' -o bin/webhooker ./cmd/webhooker
run: build
./bin/webhooker
dev: assets
dev:
go run ./cmd/webhooker
deps:
+73 -72
View File
@@ -21,6 +21,9 @@ before deploying one.
- Go 1.26.1+ (the version in `go.mod`)
- Docker (for linting, for the test stage of the CI gate, and for
containerized deployment)
- `curl`, used by `script/fetch-assets` to download the third-party
browser assets, which are not committed (`make bootstrap` installs
it if missing)
golangci-lint is not a prerequisite and must not be installed on the
host: `script/bootstrap` does not install it, and `make lint` runs the
@@ -33,7 +36,9 @@ digest-pinned linter image via `Dockerfile.lint`.
git clone https://git.eeqj.de/sneak/webhooker.git
cd webhooker
# Install the Go toolchain if missing, and the Go dependencies
# Install Go dependencies and the third-party browser assets.
# `make deps` alone is not enough: it only runs go mod download/tidy,
# and the checks below need the fetched assets.
make bootstrap
# Run all checks (test, lint, format check)
@@ -53,7 +58,7 @@ make docker
```bash
make bootstrap # Install all dependencies (idempotent)
make setup # Bootstrap + install git pre-commit hook
make assets # Extract Alpine.js from 3p/ (test, check, build, dev run it)
make assets # Fetch + verify third-party browser assets
make fmt # Format code (gofmt + goimports)
make fmt-check # Fail if gofmt would change anything (writes nothing)
make lint # Run golangci-lint in Docker (Dockerfile.lint)
@@ -152,11 +157,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
@@ -195,16 +195,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 |
| ----------------------- | ---------- |
@@ -243,8 +242,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.
@@ -970,10 +968,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
@@ -1048,9 +1051,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
@@ -1069,10 +1073,12 @@ 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. **Fix ownership.** The container runs as the non-root `webhooker`
user, UID 1000 / GID 1000. Restored files must be owned by (or
@@ -1249,14 +1255,14 @@ This repository adheres to the
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`.
`version` are inline commands with no script behind them, though
`build` and `version` both take their value from `script/version`.
`script/test`, `make build` and `make dev` each run `script/assets`
first, which writes the ignored `static/js/alpine.min.js` (see
[Third-party browser assets](#third-party-browser-assets)), so
`make test`, `make check` and the pre-commit hook work on a fresh clone
without a separate step.
`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:
@@ -1264,8 +1270,8 @@ We provide:
- `script/setup` — make a fresh clone ready for development
(bootstrap, then install-precommit)
- `script/projectname` — output the project name ("webhooker")
- `script/assets` — extract Alpine.js from its tarball in `3p/` (see
[Third-party browser assets](#third-party-browser-assets))
- `script/fetch-assets` — download the third-party browser assets into
`static/`, verifying each against its pinned sha256
- `script/test` — run the test suite
- `script/lint` — run golangci-lint in Docker (see Linting below)
- `script/fmt` — format all code (writes)
@@ -1287,25 +1293,24 @@ We provide:
## Third-party browser assets
The web UI serves one third-party script, Alpine.js. Its npm package tarball
is committed as `3p/alpinejs-3.14.9.tgz`, byte for byte as the npm registry
publishes it. It is a dependency, not this repo's build output, so
`REPO_POLICIES.md`'s rule against committed build artifacts does not apply.
The directory is `3p/` rather than `vendor/` because Go treats a root
`vendor/` directory as its module vendor directory.
The web UI serves one third-party script, Alpine.js. It is **not** committed:
a minified bundle in the tree is unreviewable, and `REPO_POLICIES.md` bars
both committed build artifacts and unpinned external references.
`script/assets` (`make assets`) extracts the browser build,
`package/dist/cdn.min.js`, from the tarball to `static/js/alpine.min.js`,
where `go:embed` picks it up. `script/test`, `make build` and `make dev` run
it first, and the Dockerfile builds through `make test` and `make build`, so
nothing downloads Alpine.js. The extracted file is not committed, and
`.dockerignore` keeps any host copy out of the build context.
Instead `script/fetch-assets` downloads it from a pinned URL, checks the
download against a hardcoded sha256, and installs it under `static/`. The
sha256 of every installed asset is recorded in `static/vendor.sha256`, and
`static/vendor_test.go` re-hashes the bytes `go:embed` put in the binary
against that manifest — so the pin is enforced on what actually ships, not
merely written down. Any mismatch fails the build.
To move to a new version: download
`https://registry.npmjs.org/alpinejs/-/alpinejs-<version>.tgz`, check it
against the `dist.integrity` hash listed at
`https://registry.npmjs.org/alpinejs/<version>`, replace the tarball in `3p/`
with it, update its file name in `script/assets`, and run `make check`.
`make bootstrap` runs the fetch for local development, and the Dockerfile
runs it in the build stage; `.gitignore` and `.dockerignore` keep the
artifact out of both the repo and the build context.
To move to a new version: update the version, URL, and tarball sha256 in
`script/fetch-assets` and the asset sha256 in `static/vendor.sha256`, then
run `make assets && make check`.
## Rationale
@@ -2717,7 +2722,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
@@ -2784,8 +2789,6 @@ imports. The entry point is `cmd/webhooker/main.go`.
```
webhooker/
├── 3p/
│ └── alpinejs-3.14.9.tgz # Alpine.js npm package, extracted by make assets
├── cmd/webhooker/
│ └── main.go # Entry point: subcommand dispatch; no args locks DATA_DIR and wires fx
├── internal/
@@ -2879,7 +2882,8 @@ webhooker/
│ ├── css/tailwind.css # Generated stylesheet the pages load
│ ├── css/style.css # Older hand-written stylesheet, no longer loaded
│ ├── js/app.js # Progressive-enhancement copy-to-clipboard
│ └── js/alpine.min.js # Alpine.js, extracted from 3p/ by make assets, not committed
│ ├── js/alpine.min.js # Alpine.js, fetched by script/fetch-assets, not committed
│ └── vendor.sha256 # Pinned hashes the fetched assets are verified against
├── templates/ # Go HTML templates (base, login, sources, etc.)
├── script/ # Scripts to Rule Them All entrypoints
├── Dockerfile # Three stages: lint, test+build, Alpine runtime
@@ -3084,8 +3088,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
@@ -3187,14 +3190,14 @@ version is fixed independently of the compiler's:
`make fmt-check`, then `golangci-lint config verify` and
`golangci-lint run`, both with `--network=none`.
2. **Builder stage** (`golang:1.26.1-bookworm`) — depends on the lint
stage passing (it copies a file from it), runs `make test` and
`make build` (both extract Alpine.js from `3p/` first), and finally
rebuilds the binary with `CGO_ENABLED=1` and static linking so it
runs on musl. Both builds go through `make build`, the relink adding
its `-extldflags` via `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)).
stage passing (it copies a file from it), runs `script/fetch-assets`
to download and verify the third-party browser assets, then runs
`make test` and `make build`, and finally rebuilds the binary with
`CGO_ENABLED=1` and static linking so it runs on musl. Both builds
go through `make build`, the relink adding its `-extldflags` via
`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,
creates the `/var/lib/webhooker` directory for all SQLite databases,
runs as the non-root `webhooker` user (UID 1000), exposes port 8080,
@@ -3277,5 +3280,3 @@ MIT
## Author
[@sneak](https://sneak.berlin)
+9 -12
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
@@ -747,14 +746,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), ","),
)
+6 -7
View File
@@ -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")
})
}
}
+18 -65
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
@@ -739,7 +728,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 +1133,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 +1249,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 +1274,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 +1314,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,
)
}
@@ -2051,31 +2021,14 @@ 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
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",
"target not found for delivery",
"delivery_id", deliveries[i].ID,
"target_id", deliveries[i].TargetID,
"error", err,
)
continue
}
}
if !e.takeForRedispatch(
webhookDB, deliveries[i].ID,
@@ -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",
)
}
-25
View File
@@ -342,31 +342,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
+6 -12
View File
@@ -339,11 +339,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 +371,6 @@ func TestApplyRequestHeaders_ReportsOriginScopedNames(t *testing.T) {
&delivery.HTTPTargetConfig{
Headers: map[string]string{
probeHeaderName: probeHeaderValue,
"Content-Type": testContentType,
},
},
)
@@ -380,11 +378,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",
)
}
+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.
-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
-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
@@ -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",
)
}
+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())
}
-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(
+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"])
+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.
})
+16 -39
View File
@@ -396,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)
@@ -419,7 +417,6 @@ func TestStaticServesOnlyGetAndHead(t *testing.T) {
http.MethodPost,
http.MethodPut,
http.MethodDelete,
"PROPFIND",
} {
t.Run(method, func(t *testing.T) {
t.Parallel()
@@ -431,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")
})
}
}
+3 -3
View File
@@ -13,9 +13,9 @@ import (
// TestBaseTemplateScriptsAreServed walks every /s/ script the base
// template loads on each page and fetches it through the real router.
// Alpine.js is extracted from its tarball in 3p/ at build time, so the
// file is not in the tree: this is the check that the page still gets
// the JavaScript it asks for.
// Alpine.js is fetched at build time rather than committed, so nothing
// in the repo guarantees it is present: this is the check that the page
// still gets the JavaScript it asks for.
func TestBaseTemplateScriptsAreServed(t *testing.T) {
t.Parallel()
-16
View File
@@ -1,16 +0,0 @@
#!/bin/sh
# script/assets: extract Alpine.js from its npm package tarball, committed
# in 3p/, to static/js/alpine.min.js, where go:embed reads it. The
# extracted file is not committed. script/test, make build and make dev run
# this first.
set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
main() {
cd "$ROOT"
tar -xzOf 3p/alpinejs-3.14.9.tgz package/dist/cdn.min.js \
>static/js/alpine.min.js
}
main "$@"
+8 -1
View File
@@ -4,7 +4,9 @@
# installed tools are skipped. Base tooling comes from nix, apt, brew,
# or apk (detected in that order); assumes NOTHING is present (not git,
# make, or go). golangci-lint is deliberately not installed: linting runs
# only in docker, via script/lint and Dockerfile.lint.
# only in docker, via script/lint and Dockerfile.lint. Finishes by running
# script/fetch-assets, which installs the hash-pinned third-party browser
# assets the repo does not commit.
set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
@@ -67,6 +69,11 @@ main() {
go mod download
# Third-party browser assets are not committed; fetch and verify them
# so a fresh clone can build and test.
if missing curl; then pkg_install curl curl curl curl; fi
"$ROOT/script/fetch-assets"
echo "bootstrap complete"
}
+104
View File
@@ -0,0 +1,104 @@
#!/bin/sh
# script/fetch-assets: download the third-party browser assets the web UI
# ships and install them under static/. Minified bundles are not committed
# (REPO_POLICIES.md: no build artifacts in version control), so the build
# fetches them here. Every download is verified against a hardcoded sha256
# before it is installed, and any mismatch aborts. Idempotent: an asset
# already present with its pinned hash is left alone.
set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
# The sha256 of each installed asset lives in static/vendor.sha256, in
# sha256sum(1) format, with paths relative to static/. That file is the
# single source of truth: this script verifies against it, and
# static/vendor_test.go asserts the bytes embedded into the binary match
# it, so the hash cannot rot into a value nothing checks.
MANIFEST="static/vendor.sha256"
# Alpine.js 3.14.9, 2026-08-17. Fetched from registry.npmjs.org, the
# publisher of record; the jsDelivr and unpkg copies are mirrors of this
# same tarball. dist/cdn.min.js is the browser build Alpine publishes for
# a <script> tag.
ALPINE_VERSION="3.14.9"
ALPINE_URL="https://registry.npmjs.org/alpinejs/-/alpinejs-${ALPINE_VERSION}.tgz"
# sha256 of alpinejs-3.14.9.tgz
ALPINE_TARBALL_SHA256="97dad7c0c81e659cfc8e7700055da9770f8186187cb9a8a76efb57e00d5ce52a"
ALPINE_MEMBER="package/dist/cdn.min.js"
ALPINE_DEST="js/alpine.min.js"
sha256_of() {
if command -v sha256sum >/dev/null 2>&1; then
sha256sum "$1" | cut -d' ' -f1
else
shasum -a 256 "$1" | cut -d' ' -f1
fi
}
# expected_sha256 <path-relative-to-static>
expected_sha256() {
awk -v want="$1" '$2 == want { print $1; found = 1 }
END { if (!found) exit 1 }' "$ROOT/$MANIFEST"
}
# verify <file> <expected-sha256> <what>
verify() {
actual="$(sha256_of "$1")"
if [ "$actual" != "$2" ]; then
echo "fetch-assets: sha256 mismatch for $3" >&2
echo " expected: $2" >&2
echo " actual: $actual" >&2
exit 1
fi
}
# up_to_date <path-relative-to-static> <expected-sha256>
up_to_date() {
[ -f "$ROOT/static/$1" ] || return 1
[ "$(sha256_of "$ROOT/static/$1")" = "$2" ]
}
fetch_alpine() {
want="$(expected_sha256 "$ALPINE_DEST")"
if up_to_date "$ALPINE_DEST" "$want"; then
echo "fetch-assets: static/$ALPINE_DEST already at $want"
return 0
fi
echo "fetch-assets: fetching Alpine.js $ALPINE_VERSION from $ALPINE_URL"
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT INT TERM
curl -fsSL -o "$tmp/alpine.tgz" "$ALPINE_URL"
verify "$tmp/alpine.tgz" "$ALPINE_TARBALL_SHA256" "alpinejs-${ALPINE_VERSION}.tgz"
tar -xzOf "$tmp/alpine.tgz" "$ALPINE_MEMBER" >"$tmp/alpine.min.js"
verify "$tmp/alpine.min.js" "$want" "$ALPINE_MEMBER from alpinejs-${ALPINE_VERSION}.tgz"
mkdir -p "$(dirname "$ROOT/static/$ALPINE_DEST")"
cp "$tmp/alpine.min.js" "$ROOT/static/$ALPINE_DEST"
rm -rf "$tmp"
trap - EXIT INT TERM
echo "fetch-assets: installed static/$ALPINE_DEST ($want)"
}
# Re-check every manifest entry against what is now on disk, so an entry
# no script installs fails loudly instead of passing silently.
verify_manifest() {
while read -r want path; do
case "$want" in '' | '#'*) continue ;; esac
if [ ! -f "$ROOT/static/$path" ]; then
echo "fetch-assets: $MANIFEST lists static/$path, which is missing" >&2
exit 1
fi
verify "$ROOT/static/$path" "$want" "static/$path"
done <"$ROOT/$MANIFEST"
}
main() {
cd "$ROOT"
fetch_alpine
verify_manifest
echo "fetch-assets: all assets in $MANIFEST verified"
}
main "$@"
-1
View File
@@ -28,7 +28,6 @@ ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
main() {
cd "$ROOT"
"$ROOT/script/assets"
go test -v -race -timeout 90s ./...
}
+1
View File
@@ -0,0 +1 @@
3ed1eed252488921df65e363d6715deb04d7f92aaedb9e52199fdf73cb1e0ad3 js/alpine.min.js
+92
View File
@@ -0,0 +1,92 @@
package static_test
import (
"bufio"
"crypto/sha256"
"encoding/hex"
"os"
"strings"
"testing"
"github.com/stretchr/testify/require"
"sneak.berlin/go/webhooker/static"
)
const manifestPath = "vendor.sha256"
// fetchHint is appended to every failure here: the assets the manifest
// covers are fetched by the build, not committed, so a fresh clone that
// has not run script/fetch-assets fails this test and should be told why.
const fetchHint = "run `script/fetch-assets` (or `make assets`) to install " +
"the pinned third-party assets"
// TestVendoredAssetsMatchManifest asserts that every asset listed in
// static/vendor.sha256 is embedded in the binary with exactly the pinned
// bytes. script/fetch-assets verifies the same hashes at download time;
// this test verifies them again on what actually ships, so a build that
// skipped, cached, or subverted the fetch cannot produce a binary serving
// unpinned third-party JavaScript.
func TestVendoredAssetsMatchManifest(t *testing.T) {
t.Parallel()
entries := readManifest(t)
require.NotEmpty(t, entries, "%s lists no assets", manifestPath)
for path, want := range entries {
t.Run(path, func(t *testing.T) {
t.Parallel()
data, err := static.Static.ReadFile(path)
require.NoErrorf(
t, err,
"%s is listed in %s but is not embedded; %s",
path, manifestPath, fetchHint,
)
sum := sha256.Sum256(data)
got := hex.EncodeToString(sum[:])
require.Equalf(
t, want, got,
"embedded %s does not match its pinned sha256 in %s; %s",
path, manifestPath, fetchHint,
)
})
}
}
// readManifest parses static/vendor.sha256, which is in sha256sum(1)
// format with paths relative to static/.
func readManifest(t *testing.T) map[string]string {
t.Helper()
f, err := os.Open(manifestPath)
require.NoError(t, err, "opening %s", manifestPath)
defer func() { require.NoError(t, f.Close()) }()
entries := make(map[string]string)
scanner := bufio.NewScanner(f)
for scanner.Scan() {
line := strings.TrimSpace(scanner.Text())
if line == "" || strings.HasPrefix(line, "#") {
continue
}
fields := strings.Fields(line)
require.Lenf(
t, fields, 2,
"%s: malformed entry %q, want \"<sha256> <path>\"",
manifestPath, line,
)
sum, path := fields[0], fields[1]
require.Lenf(t, sum, 64, "%s: %q is not a sha256", manifestPath, sum)
entries[path] = sum
}
require.NoError(t, scanner.Err(), "reading %s", manifestPath)
return entries
}