Bind the app port deliberately and document the proxy deployment (closes #268)

The plaintext listener bound `:PORT`, so it answered on every
interface with no way to say otherwise. That published the admin UI
and the unauthenticated receiver in cleartext beside whatever TLS
proxy was in front of them, reachable from any host that could route
to the machine.

BIND_ADDRESS now selects the address. The binary defaults to
127.0.0.1, which is the safe answer for a bare host: reaching
webhooker from elsewhere becomes a deliberate act. The image sets
0.0.0.0, which is the correct answer inside a container, where the
network namespace is already the boundary and exposure is decided by
the publish flag instead — so `-p 127.0.0.1:8080:8080` is what the
README shows. Existing container deployments are unaffected. Only IP
address literals are accepted: hostnames, host:port and CIDR blocks
abort startup naming the variable and the value, and a literal that is
not an address of this host fails at listen and exits non-zero.

The http.Server is now built in New rather than in the serving
goroutine, and sentryEnabled is atomic. Both fields were written by
the serving goroutine and read by the fx stop hook with nothing
ordering them, and the OnStart hook returns before that goroutine has
necessarily run: cleanShutdown could dereference a nil httpServer on
an early SIGTERM, and both reads raced. No test started and stopped
the server, so nothing observed it.

Closes #226.

README gains a "Deployment behind a reverse proxy" section: a working
nginx server block, and the five things that are silent when wrong —
bind or firewall the app port, WEBHOOKER_ENVIRONMENT=prod,
TRUSTED_PROXIES, Host as $http_host rather than $host, and keeping the
proxy's access log because webhooker's own records only the proxy.

Dockerfile

@@ -61,14 +61,28 @@ RUN script/fetch-assets
 
 # Run tests and build
 RUN make test
-RUN make build
+
+# Version stamped into the binary. .dockerignore excludes .git/, so
+# nothing in this stage can derive it: script/docker resolves it on the
+# 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 and asset steps, so a changed version
+# does not invalidate their cached layers.
+ARG VERSION=unknown
+
+RUN make build VERSION="$VERSION"
 
 # Rebuild with static linking for Alpine runtime.
 # make build already verified compilation.
 # The CGO binary from `make build` is dynamically linked against glibc,
 # which doesn't exist on Alpine (musl). Rebuild with static linking so
 # the binary runs on Alpine without glibc.
-RUN CGO_ENABLED=1 go build -ldflags '-extldflags "-static"' -o bin/webhooker ./cmd/webhooker
+#
+# The static flags go in through GO_LDFLAGS rather than a -ldflags of
+# their own: the build target composes them with the -X that stamps the
+# version, so this relink cannot silently drop the stamp.
+RUN CGO_ENABLED=1 make build VERSION="$VERSION" GO_LDFLAGS='-extldflags "-static"'
 
 # Runtime stage
 # alpine:3.21, 2026-03-17
@@ -95,6 +109,18 @@ USER webhooker
 
 EXPOSE 8080
 
+# The binary defaults BIND_ADDRESS to 127.0.0.1, which is right for a
+# bare host: the cleartext listener serves the admin UI and the
+# unauthenticated receiver, so it must not appear on every interface
+# of a machine that configured nothing. A container is the other case.
+# Its network namespace is already the isolation boundary, so binding
+# every address inside it exposes nothing; what decides exposure is
+# the publish flag, and `-p 127.0.0.1:8080:8080` is the operator's
+# control there. Shipping the image on loopback would buy no security
+# and would make the process unreachable through its own published
+# port.
+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
 

Makefile

@@ -1,8 +1,26 @@
-.PHONY: bootstrap setup assets test lint fmt fmt-check check build run dev deps docker clean hooks css
+.PHONY: bootstrap setup assets test lint fmt fmt-check check build run dev deps docker clean hooks css version
 
 # Default target
 .DEFAULT_GOAL := check
 
+# Version stamped into the binary. Derived from git by script/version;
+# override it (`make build VERSION=v1.2.3`) where git metadata is
+# unavailable, which is how the Dockerfile passes its build arg in.
+VERSION ?= $(shell script/version)
+
+# An empty override (`make build VERSION=`, or a `--build-arg VERSION=`
+# landing on the Dockerfile's `make build VERSION="$VERSION"`) means unset,
+# exactly as it does in script/version -- stamping "" would leave the binary
+# reporting no version and the footer back on its "dev" fallback. `override`
+# is required: a plain assignment loses to the command-line definition it
+# exists to correct.
+override VERSION := $(or $(strip $(VERSION)),$(shell script/version))
+
+# Extra linker flags for the build target. The static relink in the
+# Dockerfile adds -extldflags here rather than passing its own -ldflags,
+# so composing flags cannot drop the version stamp.
+GO_LDFLAGS ?=
+
 bootstrap:
 	@script/bootstrap
 
@@ -28,7 +46,7 @@ check:
 	@script/check
 
 build:
-	go build -o bin/webhooker ./cmd/webhooker
+	go build -ldflags '$(strip -X main.version=$(VERSION) $(GO_LDFLAGS))' -o bin/webhooker ./cmd/webhooker
 
 run: build
 	./bin/webhooker
@@ -40,6 +58,9 @@ deps:
 	go mod download
 	go mod tidy
 
+version:
+	@echo $(VERSION)
+
 docker:
 	@script/docker
 

README.md

@@ -57,7 +57,8 @@ make fmt-check  # Fail if gofmt would change anything (writes nothing)
 make lint       # Run golangci-lint in Docker (Dockerfile.lint)
 make test       # Run tests with race detection
 make check      # test + lint + fmt-check (CI gate)
-make build      # Build binary to bin/webhooker
+make build      # Build binary to bin/webhooker (version-stamped)
+make version    # Print the version this checkout would stamp
 make run        # build, then run ./bin/webhooker
 make dev        # go run ./cmd/webhooker
 make deps       # go mod download + go mod tidy
@@ -74,27 +75,41 @@ you can place variables in a `.env` file in the project root (loaded
 automatically via `godotenv/autoload`).
 
 The environment is selected by setting `WEBHOOKER_ENVIRONMENT` to `dev`
-or `prod` (default: `dev`). The setting controls several behaviors:
+or `prod` (default: `dev`). The setting controls exactly one behavior:
 
-| Behavior              | `dev`                            | `prod`                          |
+| Behavior | `dev`                   | `prod`           |
-| --------------------- | -------------------------------- | ------------------------------- |
+| -------- | ----------------------- | ---------------- |
-| CORS                  | Allows any origin (`*`)          | Disabled (no-op)                |
+| CORS     | Allows any origin (`*`) | Disabled (no-op) |
-| Session cookie Secure | `false` (works over plain HTTP)  | `true` (requires HTTPS)         |
 
-The CSRF cookie's `Secure` flag and Origin/Referer validation mode are
+The environment setting does **not** control cookie security. Both the
-determined per-request based on the actual transport protocol, not the
+session cookie and the CSRF cookie get their `Secure` flag, and the
-environment setting. The middleware checks `r.TLS` (direct TLS) and the
+CSRF middleware its Origin/Referer validation mode, from the transport
-`X-Forwarded-Proto` header (TLS-terminating reverse proxy) to decide:
+of each individual request, decided by one predicate —
+`internal/reqtls.IsTLS`. It reports TLS for a direct TLS connection
+(`r.TLS`) or for a TLS-terminating reverse proxy that reports one in
+`X-Forwarded-Proto`:
 
 - **Direct TLS or `X-Forwarded-Proto: https`**: Secure cookies, strict
   Origin/Referer validation.
 - **Plaintext HTTP**: Non-Secure cookies, relaxed Origin/Referer
   checks (token validation still enforced).
 
-This means CSRF protection works correctly in all deployment scenarios:
+The `X-Forwarded-Proto` value is matched case-insensitively on its
-behind a TLS-terminating reverse proxy, with direct TLS, or over plain
+first comma-separated element, trimmed, so `HTTPS` and the appended
-HTTP during development. When running behind a reverse proxy, ensure it
+chains a proxy behind another proxy emits (`https, http`) are all read
-sets the `X-Forwarded-Proto: https` header.
+as TLS.
+
+This means both cookie security and CSRF protection work correctly in
+all deployment scenarios: behind a TLS-terminating reverse proxy, with
+direct TLS, or over plain HTTP during development — a plain-HTTP local
+run gets non-`Secure` cookies and remains usable, and a proxied
+deployment gets `Secure` ones without the operator setting anything.
+When running behind a reverse proxy, ensure it sets the
+`X-Forwarded-Proto: https` header. Unlike `X-Forwarded-For`, this
+header is read from any peer and is **not** gated by
+`TRUSTED_PROXIES`; a correctly configured proxy overwrites whatever a
+client sent. On a listener exposed directly to clients, any client can
+assert it, so do not run one without a proxy in front.
 
 All other differences (log format, security headers, etc.) are
 independent of the environment setting — log format is determined by
@@ -104,6 +119,7 @@ TTY detection, and security headers are always applied.
 | ----------------------- | ----------------------------------- | -------- |
 | `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` |
 | `DEBUG`                 | Enable debug logging                | `false`  |
 | `MAINTENANCE_MODE`      | Report `maintenanceMode: true` in the healthcheck JSON. It does not change how any request is served — no maintenance page exists | `false`  |
@@ -225,6 +241,68 @@ A set but unparseable value aborts startup. When the list is non-empty
 webhooker logs it at startup, blocks and all, so the hole is visible in
 the log of any deployment that has one.
 
+#### Bind address
+
+`BIND_ADDRESS` is the IP address the HTTP listener binds. The binary
+defaults to `127.0.0.1`, so a bare webhooker is reachable only from the
+host it runs on. The Docker image ships `ENV BIND_ADDRESS=0.0.0.0`
+instead — see below for why the two differ.
+
+That listener speaks **cleartext**, and it serves both the admin UI and
+the unauthenticated webhook receiver. webhooker terminates no TLS
+itself; a production deployment puts a reverse proxy in front of it
+(see
+[Deployment behind a reverse proxy](#deployment-behind-a-reverse-proxy)),
+and the proxy reaches it over loopback. A default that bound every
+interface would leave that cleartext port answering the internet
+alongside the proxy — the admin login form and the receiver, in the
+clear, on a port nobody chose to publish. Reaching webhooker from
+another host is therefore something you configure, not something you
+get by default.
+
+**In a container the answer is `0.0.0.0`, which is why the image ships
+that.** A container's network namespace is already the boundary the
+loopback default is reaching for: nothing outside the container gets to
+`0.0.0.0:8080` because of the namespace, whatever the process bound.
+Exposure is decided at the publish flag instead — `-p
+127.0.0.1:8080:8080` rather than `-p 8080:8080` — which is the
+operator's to choose and is what
+[Running with Docker](#running-with-docker) shows. A loopback bind
+inside a container buys nothing and makes the process unreachable
+through its own published port.
+
+The value must be an IP address literal:
+
+- `127.0.0.1` — loopback only (the binary's default). Use this with a
+  reverse proxy on the same host.
+- `0.0.0.0` — every IPv4 address. The image's default; on a bare host,
+  only behind a firewall on the port.
+- `::` — every address, IPv6 and (on Linux, with the default
+  `net.ipv6.bindv6only=0`) IPv4 as well.
+- A specific address such as `10.0.0.5` — that interface only.
+
+An **empty** value is treated as unset, as everywhere else here, and
+takes the default. In a container that matters: `BIND_ADDRESS=` throws
+away the image's `0.0.0.0` and falls back to the binary's
+`127.0.0.1`, which is the one quiet failure this setting has — see
+[Running with Docker](#running-with-docker).
+
+Hostnames are **not** accepted. `localhost` aborts startup rather than
+being resolved: which of `127.0.0.1` and `::1` it means differs by
+host, a name can resolve to several addresses of which only one could
+be bound, and the answer can change under a running process. A value
+carrying a port (`127.0.0.1:8080`) is likewise rejected — the port is
+`PORT`'s business. Any unparseable value aborts startup; see
+[Invalid values abort startup](#invalid-values-abort-startup).
+
+An address that parses but is not assigned to this host — say
+`10.0.0.5` on a machine that has no such interface — is a valid
+literal, so it reaches the listener and fails there. The process logs
+the bind error and exits non-zero rather than staying up with nothing
+listening. The effective value is in the `bindAddress` field of the
+startup log line, which is the way to check what a running deployment
+actually bound.
+
 #### Metrics credentials
 
 `METRICS_USERNAME` and `METRICS_PASSWORD` are set together or not at
@@ -382,8 +460,11 @@ additionally be a number in the range 1–65535,
 `RECEIVER_RATE_LIMIT` must be at least 1,
 `RETENTION_SWEEP_INTERVAL` must be greater than zero (it is a ticker
 period, so `0s` or a negative value would crash the reaper after
-startup), and every entry in `TRUSTED_PROXIES` and
+startup), every entry in `TRUSTED_PROXIES` and
-`ALLOWED_EGRESS_CIDRS` must be a CIDR block or a bare IP address.
+`ALLOWED_EGRESS_CIDRS` must be a CIDR block or a bare IP address, and
+`BIND_ADDRESS` must be an IP address literal — `localhost`,
+`127.0.0.1:8080` and `10.0.0.0/8` are each rejected rather than
+resolved, split, or narrowed to something they do not say.
 `SESSION_IDLE_TIMEOUT` is the exception: a
 non-positive value there means idle expiry is disabled, not invalid.
 
@@ -528,12 +609,54 @@ decision:
 
 ```bash
 docker run -d \
-  -p 8080:8080 \
+  -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
 ```
 
+**The image and the bare binary default `BIND_ADDRESS` differently, on
+purpose.** The binary defaults to `127.0.0.1`; the image ships
+`ENV BIND_ADDRESS=0.0.0.0`, so the `-e BIND_ADDRESS=0.0.0.0` above is
+belt-and-braces and the command works without it.
+
+The two cases are not the same question. On a bare host, `0.0.0.0`
+puts the cleartext admin UI and the unauthenticated receiver on every
+interface of the machine, which is what the loopback default exists to
+prevent. In a container, the network namespace is already that
+boundary: nothing outside reaches `0.0.0.0:8080` because of the
+namespace, not because of the bind. What decides exposure there is the
+**publish flag**, and that is the line to get right.
+
+So publish to `127.0.0.1:8080` rather than `8080`. A bare
+`-p 8080:8080` opens the port on every interface of the host — through
+firewall rules too, since Docker's forwarding rules are inserted ahead
+of most host firewalls. Publish to the host address your reverse proxy
+connects from, and nothing wider.
+
+**An empty `BIND_ADDRESS` is treated as unset**, like every other
+variable here, so `-e BIND_ADDRESS=` does not mean "keep the image
+default" — it discards the image's `0.0.0.0` and falls back to the
+_binary's_ `127.0.0.1`. In a container that is the failure below, and
+nothing in the logs names the variable. A templated Compose file or a
+`.env` line with an empty value is the usual way in. Either set a
+literal or leave the variable out entirely.
+
+Overriding `BIND_ADDRESS` to a loopback address in a container — by
+that route or deliberately — makes the container unreachable from
+outside its namespace even with `-p`. The published port answers
+nothing, and the health check fails too: it requests
+`http://localhost:8080`, `localhost` resolves to `::1` first, and a
+`127.0.0.1` bind is not listening there. The container then goes
+`unhealthy` about **65 seconds** after start — from `HEALTHCHECK
+--start-period=5s --interval=30s --retries=3`, so failing probes at
+5s, 35s and 65s, and `unhealthy` on the third. (Docker's probe cadence
+during the start period has changed between versions; re-derive from
+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 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
@@ -543,6 +666,151 @@ 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
+
+webhooker terminates no TLS of its own. It serves plaintext HTTP and
+expects a reverse proxy in front of it, which is the deployment it is
+built for: the proxy holds the certificate, and webhooker binds
+loopback where only the proxy can reach it.
+
+Five things have to be right. Each one is silent when it is wrong —
+the service comes up, serves pages, and is broken in a way nothing
+reports.
+
+1. **Bind or firewall the app port.** The binary binds `127.0.0.1` by
+   default, so the cleartext listener is not published beside the
+   proxy. The image binds `0.0.0.0` inside its own network namespace
+   and relies on the publish address instead —
+   `-p 127.0.0.1:8080:8080`. Either way the port must reach the proxy
+   and nothing else; widen it only with a firewall or a publish
+   address in front of it. A cleartext port answering the internet
+   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. **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. 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
+   per limit and the receiver's per-IP limits become service-wide
+   ceilings. See [Trusted proxies](#trusted-proxies). List the proxy
+   and nothing else.
+4. **Send `Host` as `$http_host`, not `$host`.** `$host` strips the
+   port. webhooker's Origin/Referer check compares against the host it
+   was given, so on any port other than 443 `$host` makes every form
+   POST — including login — fail with `403 origin invalid`, with
+   nothing in the error naming the cause.
+5. **Keep the proxy's access log.** webhooker's own access log records
+   the peer address, which behind a proxy is always the proxy. The
+   proxy's log is the only record of which client sent what. nginx's
+   default `combined` format already logs `$remote_addr`; do not
+   replace it with one that drops the client address, and retain those
+   logs as long as you would want to answer a question about traffic.
+
+### nginx
+
+Complete server block. Replace the `server_name` and the two
+certificate paths.
+
+```nginx
+server {
+    listen 443 ssl;
+    listen [::]:443 ssl;
+    http2 on;  # nginx 1.25.1+; older: listen 443 ssl http2;
+
+    server_name webhooker.example.com;
+
+    ssl_certificate     /etc/ssl/certs/webhooker.example.com.crt;
+    ssl_certificate_key /etc/ssl/private/webhooker.example.com.key;
+    ssl_protocols       TLSv1.2 TLSv1.3;
+
+    # webhooker caps form POST bodies at 1 MB. nginx's default happens
+    # to match, so leaving this out breaks nothing today — but if you
+    # ever raise webhooker's cap, this is the limit you will still be
+    # hitting, and the rejection is nginx's HTML page rather than
+    # webhooker's message.
+    client_max_body_size 1m;
+
+    # $remote_addr is the client. webhooker's own log records this
+    # proxy and nothing else, so this file is the only place the
+    # client's address is written down.
+    access_log /var/log/nginx/webhooker.access.log combined;
+
+    location / {
+        # A literal address, not localhost: with BIND_ADDRESS at its
+        # 127.0.0.1 default, a localhost that resolves to ::1 first
+        # gets connection refused.
+        proxy_pass http://127.0.0.1:8080;
+
+        # $http_host, NOT $host. $host drops the port and every form
+        # POST fails with 403 origin invalid on any port but 443.
+        proxy_set_header Host              $http_host;
+        proxy_set_header X-Forwarded-For   $proxy_add_x_forwarded_for;
+        proxy_set_header X-Forwarded-Proto $scheme;
+
+        # Above webhooker's own 60s request timeout, so its 503
+        # reaches the client instead of nginx cutting the connection
+        # first and answering 504.
+        proxy_read_timeout 70s;
+    }
+}
+
+server {
+    listen 80;
+    listen [::]:80;
+    server_name webhooker.example.com;
+    return 308 https://$host$request_uri;
+}
+```
+
+`X-Forwarded-For` must be **appended** to, which
+`$proxy_add_x_forwarded_for` does. webhooker reads no other forwarded
+client header: `X-Real-IP` and `True-Client-IP` are ignored from every
+peer, so setting them has no effect. See
+[Trusted proxies](#trusted-proxies) for how the chain is walked.
+
+`X-Forwarded-Proto: https` is what tells webhooker the request arrived
+over TLS, which decides the `Secure` flag on both the session and CSRF
+cookies and the strict Origin/Referer mode. Without it, requests are
+treated as plaintext and the cookies ship without `Secure`. Unlike
+`X-Forwarded-For`, this header is read from any peer and is not gated
+by `TRUSTED_PROXIES`, so the proxy must overwrite whatever a client
+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
+```
+
+If nginx runs on another host, `BIND_ADDRESS` becomes the address it
+connects to, `TRUSTED_PROXIES` becomes nginx's address, and the port
+must be firewalled to that address — the traffic between them is
+cleartext.
+
+### HSTS is always sent, and is not configurable
+
+Every response carries
+`Strict-Transport-Security: max-age=63072000; includeSubDomains; preload`.
+Two years, every subdomain, and a `preload` token. There is no setting
+that changes or suppresses it.
+
+This is worth knowing before the first request reaches a browser: a
+client that sees it once will refuse plaintext HTTP to that hostname —
+and to every subdomain of it — for two years, whatever else is served
+there. Terminate TLS on a hostname you are prepared to keep on HTTPS.
+
 ## Backup, Restore, and Upgrades
 
 ### What to back up
@@ -677,6 +945,21 @@ Upgrade procedure:
 3. Pull the new image and start it.
 4. Confirm `database migrations completed` in the logs before putting
    traffic back on it.
+5. Confirm the new build is the one running:
+   `curl -s http://host:8080/.well-known/healthcheck` reports the
+   version it was stamped with (see [Version stamping](#version-stamping)).
+
+**Upgrading past the introduction of `BIND_ADDRESS`:** earlier versions
+always bound every interface. **Container deployments are unaffected**
+— the image ships `ENV BIND_ADDRESS=0.0.0.0`, so a `docker run` or
+Compose service that worked before still works with nothing changed.
+
+A **bare binary** is the case that changes: the listener now binds
+`127.0.0.1` unless `BIND_ADDRESS` says otherwise, so a deployment that
+relied on reaching it from another host becomes unreachable until it
+sets the address the proxy connects to. Check the `bindAddress` field
+of the startup log to see what a running process bound. See
+[Bind address](#bind-address).
 
 **Downgrade is unsupported.** Once a newer binary has migrated the files
 there is no way to move them back. `AutoMigrate` is additive — it adds
@@ -687,6 +970,42 @@ silent divergence, not a startup error. The only supported way back to
 an older version is restoring the pre-upgrade backup, which discards
 everything received since that backup was taken.
 
+### Version stamping
+
+The binary reports its version at `/.well-known/healthcheck` (the
+`version` field), in the UI footer, and in the startup log line
+(`msg=starting`, `version=...`). It is also the Sentry release name,
+as `webhooker-{version}`. The value is stamped in at build time by the
+linker; it is not read from a file at runtime, so it identifies the
+build itself.
+
+`script/version` produces the value and both build paths use it:
+
+| Build | What it reports |
+| --- | --- |
+| Clean checkout at a tag | exactly that tag, e.g. `v1.0.0` |
+| Commits past a tag | `v1.0.0-3-g1a2b3c4` — tag, commits since, short SHA |
+| No tag reachable | the short SHA, e.g. `1a2b3c4` |
+| Uncommitted changes | the above with a `-dirty` suffix |
+| No git metadata | `unknown` |
+
+`unknown` is what a source tarball or a `docker build .` with no
+`--build-arg VERSION=...` reports. `.dockerignore` excludes `.git/`, so
+the build context carries no git metadata and the image cannot derive
+the version itself: `script/docker` (and so `make docker`) resolves it
+on the host and passes it in as the `VERSION` build arg. A build that
+reports `unknown` is a build nobody told what it was; it is not a
+failure, but it cannot be traced back to a commit.
+
+`make version` prints what the current checkout would stamp, and
+`make build VERSION=v1.2.3` overrides it. An empty override — from
+`make build VERSION=` or from `--build-arg VERSION=` — means unset
+rather than `""`, and resolves the way an absent one does.
+
+Nothing that varies between two builds of the same commit is stamped —
+no timestamp, no hostname, no builder identity — so two builds of one
+commit still produce a byte-identical binary.
+
 ### Backups contain secrets
 
 Treat a backup with the same care as the credentials inside it. Encrypt
@@ -719,119 +1038,29 @@ backups at rest and restrict who can read them.
   credential that was in a backup you cannot account for.
 - `webhooker.db` stores target config **unencrypted**, tracked at
   [issue #212](https://git.eeqj.de/sneak/webhooker/issues/212), next to
-  the session encryption key and the Argon2id password hashes. It also
+  the session encryption key and the Argon2id password hashes.
-  holds each entrypoint's inbound signature secret in the clear, for
-  the reason given under
-  [Inbound Signature Verification](#inbound-signature-verification):
-  HMAC verification needs the key itself, so it cannot be hashed.
 
-## Inbound Signature Verification
+## The entrypoint URL is the authentication secret
 
-A receiver URL is a bare v4 UUID in a path. That is unguessable, but it
+The receiver verifies nothing about an inbound request. The UUID in an
-is not a credential: anyone who learns it — from a browser history, a
+entrypoint's URL is its credential: anyone who holds that URL can
-proxy log, a screenshot, a copy-pasted support ticket — can post events
+submit events to it, and the receiver checks nothing else about the
-that webhooker stores and forwards, and the inbound headers are passed
+sender. Treat an entrypoint URL the way you would treat an API token.
-on to your targets almost verbatim, so they also choose what the
-downstream service sees. Verification is how an entrypoint stops
-accepting anything that reaches its URL.
 
-It is optional and configured per entrypoint. An entrypoint with no
+There is no way to rotate the UUID in place. To retire one, delete the
-scheme selected is not verified, which is what every entrypoint was
+entrypoint (or deactivate it, which answers `410`) and create a new
-before this existed and what every entrypoint remains after an
+one, then point the sender at the new URL.
-upgrade — enabling verification is always a deliberate act, and no
-existing deployment is locked out of its own receivers by installing a
-new version.
-
-When a scheme **is** selected, a request whose signature is missing,
-malformed or wrong is answered `401` and **nothing is stored**: no
-event row, no delivery row, no delivery attempt. Rejection happens
-after the body is read (the signature covers it) and before the first
-write.
-
-### Supported schemes
-
-| Scheme   | Header                | Check |
-| -------- | --------------------- | ----- |
-| `github` | `X-Hub-Signature-256` | HMAC-SHA256 of the raw request body under the shared secret, hex-encoded, prefixed `sha256=` |
-| `gitlab` | `X-Gitlab-Token`      | The header is the shared secret itself, compared as-is |
-
-Both comparisons run in constant time (`hmac.Equal`). The HMAC is
-computed over the request body exactly as received, before any parsing,
-and under the same 1 MB body cap every other request obeys — an
-unsigned sender cannot make webhooker buffer more than a signed one.
-
-GitHub's older SHA-1 `X-Hub-Signature` is **not** accepted. Neither is
-a GitHub digest sent without its `sha256=` prefix.
-
-### Configuring a sender
-
-The secret is a value you choose and enter in two places: at the sender
-and in webhooker. webhooker never generates or displays one, so there
-is no stored credential the UI can be made to reveal.
-
-1. Generate a secret, e.g. `openssl rand -hex 32`.
-2. In webhooker, open the webhook's page, find the entrypoint, and
-   click **Configure** (or **Rotate**, if it already has one). Select
-   the scheme and paste the secret. Surrounding whitespace is stripped,
-   so a value pasted with a trailing space still works; a secret whose
-   own first or last character is a space cannot be stored.
-3. At the sender:
-    - **GitHub** — repository (or organization) → Settings → Webhooks →
-      the hook → **Secret**. GitHub then signs every delivery with
-      `X-Hub-Signature-256`.
-    - **GitLab** — project → Settings → Webhooks → the hook → **Secret
-      token**. GitLab sends it verbatim as `X-Gitlab-Token`.
-
-**Rotation** is the same form: submit the new secret. Deliveries signed
-with the old secret are rejected from that moment, so change it at the
-sender in the same sitting. Selecting **None** removes verification and
-deletes the stored secret with it.
-
-The page shows which scheme an entrypoint uses and which header it
-reads, never the secret. The value is stored in the clear — HMAC
-verification needs the key itself, and a hash of it cannot recompute a
-sender's digest — so it is handled like the other credentials
-webhooker holds: excluded from JSON, kept out of templates by a
-projection (`handlers.EntrypointView`), and absent from every log line,
-including the ones written when verification fails.
-
-### The credential is not stored or forwarded
-
-Under the `gitlab` scheme the signature header **is** the secret. An
-accepted request's headers are persisted on the event and forwarded to
-every delivery target, so `X-Gitlab-Token` is removed from that copy
-before the event is written — otherwise every target operator, every
-backup and everyone with read access to `events-*.db` would hold the
-value needed to forge signed requests to the entrypoint it protects.
-The sender's other headers are untouched, and the request the receiver
-itself verifies against is not modified.
-
-The stripping is driven by the scheme's own description rather than by
-a header name, and a scheme is stripped unless it declares that its
-header carries a digest. `github` declares it: `X-Hub-Signature-256` is
-an HMAC over the body, from which the key cannot be recovered, so it is
-stored and forwarded intact. A scheme added later is stripped by
-default.
-
-### When configuration is broken
-
-An entrypoint whose stored scheme this build does not recognise, or
-which has one half of the scheme/secret pair and not the other, is
-answered `500` and stores nothing. It is not treated as unverified. The
-UI cannot create such a row — it rejects an unknown scheme with a `400`
-— so this covers a hand-edited database or a downgrade to a build that
-predates a scheme. Failing closed is the point: an entrypoint the
-operator believes is protected must never quietly go back to accepting
-anything.
 
 ## Entrypoints
 
 This repository adheres to the
 [Scripts to Rule Them All](https://github.com/github/scripts-to-rule-them-all)
 standard: normalized scripts in `script/` are the entrypoints for the
-development workflow. Ten of the Makefile's sixteen targets are thin
+development workflow. Ten of the Makefile's seventeen targets are thin
-shims that call them; `build`, `run`, `dev`, `deps`, `clean` and `css`
+shims that call them; `build`, `run`, `dev`, `deps`, `clean`, `css` and
-are inline commands with no script behind them. We provide:
+`version` are inline commands with no script behind them, though
+`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
@@ -844,7 +1073,11 @@ are inline commands with no script behind them. We provide:
 - `script/fmt` — format all code (writes)
 - `script/fmt-check` — check formatting (read-only)
 - `script/check` — run test, lint, and fmt-check
-- `script/docker` — build the Docker image tagged via `script/projectname`
+- `script/version` — output the version to stamp into the binary (see
+  [Version stamping](#version-stamping))
+- `script/docker` — build the Docker image tagged via
+  `script/projectname`, passing `script/version`'s output in as the
+  `VERSION` build arg
 - `script/cibuild` — CI entrypoint: `docker build .` (the Dockerfile
   runs the checks, so a green build implies a green repo)
 - `script/ci-mark-superseded` — CI helper: mark the commits whose run a
@@ -1102,18 +1335,12 @@ the full request and creates an Event.
 | -------------- | ------- | ----------- |
 | `id`           | UUID    | Primary key |
 | `webhook_id`   | UUID    | Foreign key → Webhook |
-| `path`         | string  | Unique bare UUID, generated at creation. The `/webhook/` prefix is route only and is not stored: the receiver matches this column against the raw `{uuid}` path segment |
+| `path`         | string  | Unique bare UUID, generated at creation. The `/webhook/` prefix is route only and is not stored: the receiver matches this column against the raw `{uuid}` path segment. It is also the entrypoint's credential; see [The entrypoint URL is the authentication secret](#the-entrypoint-url-is-the-authentication-secret) |
 | `description`  | string  | Optional description |
 | `active`       | boolean | Whether this entrypoint accepts events (default: true) |
-| `signature_scheme` | string | How inbound requests are authenticated: `github`, `gitlab`, or empty for no verification (default: empty). See [Inbound Signature Verification](#inbound-signature-verification) |
-| `signature_secret` | string | The secret shared with the sender, stored in the clear because HMAC verification needs the key itself. Never marshalled to JSON, never rendered, never logged. Empty when no scheme is set |
 
 **Relations:** Belongs to Webhook.
 
-Both signature columns arrive through `AutoMigrate` with an empty
-default, so every entrypoint written before they existed migrates to
-"not configured" and keeps accepting the traffic it already accepted.
-
 A webhook can have multiple entrypoints. This allows separate URLs for
 different event sources that all feed into the same processing pipeline
 (e.g., one entrypoint for GitHub, another for Stripe, both routing to
@@ -1307,10 +1534,6 @@ fired twenty times at a backend stays traceable. Resubmitting the same
 event repeatedly is supported and is the point of the action — there is
 no in-flight refusal; the route's own rate limit is what bounds it.
 
-Inbound signature verification is not re-run on a resubmit. There is no
-inbound signature to check on a copy the operator submits, and the
-route is authenticated and CSRF-protected as an operator action.
-
 #### DeliveryResult
 
 The result of a single delivery attempt. Every attempt (including
@@ -1471,17 +1694,15 @@ External Service
 │             │     │  Stack       │     │   Handler    │
 └─────────────┘     └──────────────┘     └──────┬───────┘
                                                 │
-                     1. Look up Entrypoint by UUID
+                     1. Look up Entrypoint by UUID — 404 if unknown,
+                        410 if inactive
                      2. Read the body under the 1 MB cap
-                     3. Verify the signature, if the entrypoint has
+                     3. Capture full request as Event
-                        one configured — 401 and no writes if it
+                     4. Create Delivery records for each active Target
-                        fails (see Inbound Signature Verification)
+                     5. Build self-contained delivery.Task structs
-                     4. Capture full request as Event
-                     5. Create Delivery records for each active Target
-                     6. Build self-contained delivery.Task structs
                         (target config + event data inline for
                          bodies < 16 KiB)
-                     7. Notify Engine via channel (no DB read needed)
+                     6. Notify Engine via channel (no DB read needed)
                                                 │
                                                 ▼
                                         ┌──────────────┐
@@ -1853,13 +2074,16 @@ the rest. Nothing dropped is needed for the likeliest use, debugging a
 CSRF rejection. Its three inputs are the TLS decision, `Origin` and
 `Referer`; the latter two are kept, and the first is the scheme of the
 retained URL, because the SDK derives that scheme from
-`r.TLS != nil || r.Header.Get("X-Forwarded-Proto") == "https"` — byte
+`r.TLS != nil || r.Header.Get("X-Forwarded-Proto") == "https"`. That
-for byte the predicate `internal/middleware/csrf.go` uses to choose
+predicate is the SDK's own and is stricter than `internal/reqtls.IsTLS`,
-between the `csrf.Secure(true)` and `csrf.Secure(false)` handlers.
+which this service now uses everywhere it decides transport: the SDK
-That is what the rewrite above preserves it for, and it is why
+reports `http` for the `HTTPS` and `https, http` spellings `reqtls`
-dropping `X-Forwarded-Proto` costs nothing. The dropped provider
+accepts. Only a reported scheme is affected, no decision is, so it is
-headers (`X-GitHub-Event`, `X-Gitlab-Event` and the like) are real
+left to the SDK rather than reimplemented. That is what the rewrite
-signal but are recorded locally on the event, and
+above preserves it for, and it is why dropping `X-Forwarded-Proto`
+costs nothing. The dropped provider headers (`X-GitHub-Event`,
+`X-Gitlab-Event` and the like) are real signal but are recorded
+locally on the event, and
 `Sentry-Trace`/`Baggage` are already reflected in the event's trace
 context.
 
@@ -2283,7 +2507,6 @@ abuse limit later; they are tracked as future work.
 | `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}/entrypoints/{entrypointID}/secret` | Set, rotate or remove the entrypoint's inbound signature scheme and secret (see [Inbound Signature Verification](#inbound-signature-verification)) |
 | `POST` | `/source/{id}/targets`   | Add target to webhook |
 | `POST` | `/source/{id}/targets/{targetID}/delete` | Delete a target |
 | `POST` | `/source/{id}/targets/{targetID}/toggle` | Enable or disable a target |
@@ -2334,7 +2557,7 @@ webhooker/
 │   │   ├── model_setting.go   # Setting entity (key-value app config)
 │   │   ├── model_user.go      # User entity
 │   │   ├── model_webhook.go    # Webhook entity
-│   │   ├── model_entrypoint.go # Entrypoint entity and SignatureScheme enum
+│   │   ├── model_entrypoint.go # Entrypoint entity
 │   │   ├── model_target.go    # Target entity and TargetType enum
 │   │   ├── model_event.go     # Event entity (per-webhook DB)
 │   │   ├── model_delivery.go  # Delivery entity (per-webhook DB)
@@ -2396,11 +2619,9 @@ webhooker/
 │   │   ├── server.go          # Server struct, fx lifecycle, signal handling
 │   │   ├── http.go            # HTTP server setup with timeouts
 │   │   └── routes.go          # All route definitions
-│   ├── session/
+│   └── session/
-│   │   ├── session.go         # Cookie-based session management
+│       ├── session.go         # Cookie-based session management
-│   │   └── testing.go         # NewForTest: Session without the fx lifecycle
+│       └── testing.go         # NewForTest: Session without the fx lifecycle
-│   └── signature/
-│       └── signature.go       # Inbound signature verification (GitHub, GitLab)
 ├── static/
 │   ├── static.go              # //go:embed directive
 │   ├── css/input.css          # Tailwind input, source for tailwind.css (make css)
@@ -2413,7 +2634,7 @@ webhooker/
 ├── script/                    # Scripts to Rule Them All entrypoints
 ├── Dockerfile                 # Three stages: lint, test+build, Alpine runtime
 ├── Dockerfile.lint            # Lint-only image built by script/lint
-├── Makefile                   # 10 of 16 targets shim script/; 6 are inline
+├── Makefile                   # 10 of 17 targets shim script/; 7 are inline
 ├── go.mod / go.sum
 └── .golangci.yml              # Linter configuration
 ```
@@ -2515,8 +2736,9 @@ check, see [The login endpoint](#the-login-endpoint).
 
 - **Web UI:** Cookie-based sessions using gorilla/sessions with
   encrypted cookies. Sessions are configured with HttpOnly, SameSite
-  Lax, and Secure (in production). Absolute session lifetime is 7 days,
+  Lax, and Secure whenever the request is on TLS — the flag follows the
-  with a sliding idle timeout on top of it (see
+  request's transport, not the environment. Absolute session lifetime
+  is 7 days, with a sliding idle timeout on top of it (see
   [Sessions](#sessions)).
 - **API (planned):** API key authentication via `Authorization: Bearer`
   header. API keys are stored per-user with usage tracking
@@ -2529,7 +2751,10 @@ check, see [The login endpoint](#the-login-endpoint).
 ### Security
 
 - Passwords hashed with Argon2id (64 MB memory cost)
-- Session cookies are HttpOnly, SameSite Lax, Secure (prod only)
+- Session cookies are HttpOnly, SameSite Lax, and Secure on any request
+  that arrived over TLS (directly or through a reverse proxy reporting
+  it), decided per-request by `internal/reqtls.IsTLS` rather than by the
+  configured environment
 - Session regeneration on login to prevent session fixation attacks
 - Session key is a 32-byte value auto-generated on first startup and
   stored in the database
@@ -2542,18 +2767,14 @@ check, see [The login endpoint](#the-login-endpoint).
   on all state-changing forms (cookie-based double-submit tokens with
   HMAC authentication). Applied to `/pages`, `/sources`, `/source`, and
   `/user` routes. Excluded from `/webhook` (inbound webhook POSTs) and
-  `/api` (stateless API). The middleware auto-detects TLS status
+  `/api` (stateless API). The middleware detects TLS per-request through
-  per-request (via `r.TLS` and `X-Forwarded-Proto`) to set appropriate
+  `internal/reqtls.IsTLS` — the same predicate the session cookie uses —
-  cookie security flags and Origin/Referer validation mode
+  to set appropriate cookie security flags and Origin/Referer validation
-- **Optional inbound signature verification** per entrypoint (GitHub
+  mode
-  `X-Hub-Signature-256`, GitLab `X-Gitlab-Token`). Off by default and
+- **The entrypoint URL is the receiver's only credential.** Nothing
-  off after an upgrade, so behaviour is unchanged until an operator
+  about an inbound request is verified; possession of the UUID
-  turns it on. Where it is on, an unsigned or wrongly signed request
+  authorises submission (see
-  is `401` and is not persisted, and a configuration the receiver
+  [The entrypoint URL is the authentication secret](#the-entrypoint-url-is-the-authentication-secret))
-  cannot apply fails closed rather than reverting to unverified. The
-  comparison is constant time and the secret never reaches a template,
-  a JSON response or a log line (see
-  [Inbound Signature Verification](#inbound-signature-verification))
 - **SSRF prevention** for HTTP delivery targets: private/reserved IP
   ranges (RFC 1918, loopback, link-local, cloud metadata) are blocked
   both at target creation time (URL validation) and at delivery time
@@ -2713,7 +2934,11 @@ version is fixed independently of the compiler's:
    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.
+   `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,

internal/config/config.go

@@ -33,6 +33,34 @@ const (
 	// defaultPort is the default HTTP listen port.
 	defaultPort = 8080
 
+	// defaultBindAddress is the interface the plaintext HTTP
+	// listener claims when BIND_ADDRESS is unset.
+	//
+	// Loopback, because the listener speaks cleartext and serves
+	// both the admin UI and the unauthenticated receiver: a
+	// wildcard default publishes them on every interface of every
+	// host that never configured anything, which is the failure
+	// this default exists to prevent. Reaching webhooker from off
+	// the host is then a deliberate act — a reverse proxy in front
+	// of it, or an explicit BIND_ADDRESS.
+	//
+	// This is the binary's default only. The Dockerfile ships
+	// ENV BIND_ADDRESS=0.0.0.0, so a container deployment needs
+	// nothing set and is unaffected by this constant. The two
+	// differ because they answer different questions: a container's
+	// network namespace is already the boundary this default is
+	// reaching for, so binding every address inside it exposes
+	// nothing, and what decides exposure there is the publish flag
+	// (-p 127.0.0.1:8080:8080). A loopback bind inside a container
+	// buys no security and makes the process unreachable through
+	// its own published port.
+	//
+	// The split is expressed as two explicit defaults rather than
+	// container auto-detection, because a heuristic that guesses
+	// wrong opens the cleartext port exactly where nobody is
+	// looking.
+	defaultBindAddress = "127.0.0.1"
+
 	// defaultRetentionSweepInterval is how often the retention
 	// reaper deletes events older than each webhook's RetentionDays.
 	defaultRetentionSweepInterval = time.Hour
@@ -75,6 +103,10 @@ var ErrInvalidPort = errors.New("invalid port")
 // nor a bare IP address.
 var ErrInvalidCIDR = errors.New("invalid CIDR")
 
+// ErrInvalidBindAddress is returned when BIND_ADDRESS is set to
+// something that is not an IP address literal.
+var ErrInvalidBindAddress = errors.New("invalid bind address")
+
 // ErrIncompleteMetricsAuth is returned when exactly one of
 // METRICS_USERNAME and METRICS_PASSWORD carries a value. Neither
 // fallback is acceptable: serving /metrics on the username alone
@@ -105,6 +137,13 @@ type Config struct {
 	Port            int
 	SentryDSN       string
 
+	// BindAddress is the IP address the plaintext HTTP listener
+	// binds, as an address literal. It defaults to
+	// defaultBindAddress and is never empty: an empty string would
+	// mean the wildcard to net.Listen, which is the opposite of the
+	// default this ships.
+	BindAddress string
+
 	// RetentionSweepInterval is how often the retention reaper runs.
 	// Always positive: it becomes a time.NewTicker period.
 	RetentionSweepInterval time.Duration
@@ -387,6 +426,42 @@ func envPrefixList(key string) ([]netip.Prefix, error) {
 	return prefixes, nil
 }
 
+// envBindAddress returns the value of the named environment variable
+// parsed as an IP address literal. An unset (or empty, or
+// whitespace-only) value yields defaultValue.
+//
+// Only literals are accepted: no hostname is resolved, so `localhost`
+// is an error rather than a DNS lookup at startup whose answer could
+// be either loopback family, could change under the process, and
+// could return several addresses of which only one would be bound. A
+// value with a port in it (`127.0.0.1:8080`) is likewise an error —
+// the port is PORT's business, and silently accepting it would bind
+// something other than what was asked for.
+//
+// A set value that is not a literal is a hard error naming the key
+// and the bad value, so startup fails loudly rather than falling back
+// to a default the operator plainly did not want. A literal that is
+// not an address of this host parses here and fails at listen time
+// instead, which ends the process non-zero.
+func envBindAddress(key, defaultValue string) (string, error) {
+	v := strings.TrimSpace(os.Getenv(key))
+	if v == "" {
+		return defaultValue, nil
+	}
+
+	addr, err := netip.ParseAddr(v)
+	if err != nil {
+		return "", fmt.Errorf(
+			"%w: %s: %q must be an IP address literal such as "+
+				"127.0.0.1, 0.0.0.0 or ::, not a hostname and not "+
+				"host:port: %w",
+			ErrInvalidBindAddress, key, v, err,
+		)
+	}
+
+	return addr.String(), nil
+}
+
 // resolveMetricsAuth reads the /metrics basic-auth credentials and
 // rejects a half-set pair, naming both variables either way. The
 // error carries neither value: the password is a secret.
@@ -431,6 +506,27 @@ func resolveEnvironment() (string, error) {
 	return environment, nil
 }
 
+// resolveListener reads the two variables that describe the HTTP
+// listener: which port it claims and which address it claims it on.
+// They are read together because neither is meaningful alone, and
+// because a validation failure in either has to abort startup before
+// anything binds.
+func resolveListener() (int, string, error) {
+	port, err := envPort("PORT", defaultPort)
+	if err != nil {
+		return 0, "", err
+	}
+
+	bindAddress, err := envBindAddress(
+		"BIND_ADDRESS", defaultBindAddress,
+	)
+	if err != nil {
+		return 0, "", err
+	}
+
+	return port, bindAddress, nil
+}
+
 // loadFromEnv builds a Config from the environment. Every value that
 // needs parsing fails loudly when it is set but unparseable: the
 // documented defaults apply only to variables that are unset (or
@@ -442,7 +538,7 @@ func loadFromEnv() (*Config, error) {
 		return nil, err
 	}
 
-	port, err := envPort("PORT", defaultPort)
+	port, bindAddress, err := resolveListener()
 	if err != nil {
 		return nil, err
 	}
@@ -506,6 +602,7 @@ func loadFromEnv() (*Config, error) {
 		MetricsUsername:        metricsUsername,
 		MetricsPassword:        metricsPassword,
 		Port:                   port,
+		BindAddress:            bindAddress,
 		SentryDSN:              envString("SENTRY_DSN"),
 		RetentionSweepInterval: retentionSweepInterval,
 		SessionIdleTimeout:     sessionIdleTimeout,
@@ -625,6 +722,11 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
 	log.Info("Configuration loaded",
 		"environment", s.Environment,
 		"port", s.Port,
+		// Logged because which interfaces the cleartext listener
+		// answers on is not otherwise observable from inside a
+		// container, and it decides whether anything but the local
+		// host can reach the admin UI.
+		"bindAddress", s.BindAddress,
 		"debug", s.Debug,
 		"maintenanceMode", s.MaintenanceMode,
 		"dataDir", s.DataDir,

internal/config/env_test.go

@@ -21,6 +21,22 @@ const (
 	envKeyPort            = "PORT"
 	envKeyDebug           = "DEBUG"
 	envKeyMaintenanceMode = "MAINTENANCE_MODE"
+	envKeyBindAddress     = "BIND_ADDRESS"
+)
+
+// Sample BIND_ADDRESS values used by the tables below.
+const (
+	// bindAddressDefault is the shipped default. It is asserted
+	// against the package's own constant in
+	// TestNewUsesDefaultsWhenUnset, so the two cannot drift.
+	bindAddressDefault = "127.0.0.1"
+
+	// bindAddressWildcard is the value a container deployment sets.
+	bindAddressWildcard = "0.0.0.0"
+
+	// bindAddressSample is an arbitrary specific address, standing
+	// for "one interface of several".
+	bindAddressSample = "10.1.2.3"
 )
 
 // envBoolCase is one row of the envBool table.
@@ -291,6 +307,160 @@ func TestEnvPort(t *testing.T) {
 	}
 }
 
+// TestEnvBindAddress covers BIND_ADDRESS parsing.
+//
+// Only IP address literals are accepted. Every rejection below is a
+// value an operator plausibly writes — a hostname, a host:port, a
+// CIDR block — and each has to abort startup rather than fall back to
+// the default, because falling back would bind an address other than
+// the one asked for and, in the wildcard-default case this setting
+// exists to end, publish cleartext on every interface.
+func TestEnvBindAddress(t *testing.T) {
+	for _, tt := range envBindAddressCases() {
+		t.Run(tt.name, func(t *testing.T) {
+			// Cannot use t.Parallel() here because t.Setenv
+			// is incompatible with parallel subtests.
+			if tt.set {
+				t.Setenv(testEnvKey, tt.value)
+			} else {
+				require.NoError(t, os.Unsetenv(testEnvKey))
+			}
+
+			got, err := config.EnvBindAddressForTest(
+				testEnvKey, bindAddressDefault,
+			)
+
+			if tt.expectError {
+				require.Error(t, err)
+				require.ErrorIs(t, err, config.ErrInvalidBindAddress)
+				assert.Contains(t, err.Error(), testEnvKey)
+				assert.Contains(t, err.Error(), tt.value)
+
+				return
+			}
+
+			require.NoError(t, err)
+			assert.Equal(t, tt.expected, got)
+		})
+	}
+}
+
+// envBindAddressCase is one row of the envBindAddress table.
+type envBindAddressCase struct {
+	name        string
+	set         bool
+	value       string
+	expectError bool
+	expected    string
+}
+
+// envBindAddressCases is the envBindAddress table, kept out of the
+// test body so the test itself stays readable.
+func envBindAddressCases() []envBindAddressCase {
+	return append(
+		envBindAddressAcceptedCases(),
+		envBindAddressRejectedCases()...,
+	)
+}
+
+// envBindAddressAcceptedCases are the values that parse: the three
+// spellings of "unset" that take the default, and the literals.
+func envBindAddressAcceptedCases() []envBindAddressCase {
+	return []envBindAddressCase{
+		{
+			name:     "unset returns the default",
+			expected: bindAddressDefault,
+		},
+		{
+			name:     "empty returns the default",
+			set:      true,
+			value:    "",
+			expected: bindAddressDefault,
+		},
+		{
+			name:     "whitespace returns the default",
+			set:      true,
+			value:    "   ",
+			expected: bindAddressDefault,
+		},
+		{
+			name:     "ipv4 wildcard is parsed",
+			set:      true,
+			value:    bindAddressWildcard,
+			expected: bindAddressWildcard,
+		},
+		{
+			name:     "ipv4 literal is parsed",
+			set:      true,
+			value:    bindAddressSample,
+			expected: bindAddressSample,
+		},
+		{
+			name:     "surrounding whitespace is trimmed",
+			set:      true,
+			value:    "  " + bindAddressSample + "  ",
+			expected: bindAddressSample,
+		},
+		{
+			name:     "ipv6 wildcard is parsed",
+			set:      true,
+			value:    "::",
+			expected: "::",
+		},
+		{
+			name:     "ipv6 literal is parsed",
+			set:      true,
+			value:    "2001:db8::5",
+			expected: "2001:db8::5",
+		},
+	}
+}
+
+// envBindAddressRejectedCases are the values that abort startup.
+// Each is something an operator plausibly writes, and none may fall
+// back to the default: the default is loopback, so a silent fallback
+// would bind somewhere other than what was asked for.
+func envBindAddressRejectedCases() []envBindAddressCase {
+	return []envBindAddressCase{
+		{
+			name:        "garbage is rejected",
+			set:         true,
+			value:       "not-an-address",
+			expectError: true,
+		},
+		{
+			name:        "hostname is rejected",
+			set:         true,
+			value:       "localhost",
+			expectError: true,
+		},
+		{
+			name:        "unresolvable hostname is rejected",
+			set:         true,
+			value:       "no-such-host.invalid",
+			expectError: true,
+		},
+		{
+			name:        "host and port is rejected",
+			set:         true,
+			value:       bindAddressDefault + ":8080",
+			expectError: true,
+		},
+		{
+			name:        "bracketed ipv6 is rejected",
+			set:         true,
+			value:       "[::1]",
+			expectError: true,
+		},
+		{
+			name:        "CIDR block is rejected",
+			set:         true,
+			value:       "10.0.0.0/8",
+			expectError: true,
+		},
+	}
+}
+
 // buildConfig constructs a Config through fx exactly as the
 // application does, returning the config and any construction error.
 func buildConfig(t *testing.T) (*config.Config, error) {
@@ -312,13 +482,45 @@ func buildConfig(t *testing.T) (*config.Config, error) {
 }
 
 func TestNewRejectsBadEnvValues(t *testing.T) {
-	tests := []struct {
+	for _, tt := range badEnvValueCases() {
-		name        string
+		t.Run(tt.name, func(t *testing.T) {
-		key         string
+			// Cannot use t.Parallel() here because t.Setenv
-		value       string
+			// is incompatible with parallel subtests.
-		expectError bool
+			t.Setenv("WEBHOOKER_ENVIRONMENT", "dev")
-		check       func(t *testing.T, cfg *config.Config)
+			t.Setenv(tt.key, tt.value)
-	}{
+
+			cfg, err := buildConfig(t)
+
+			if tt.expectError {
+				require.Error(t, err)
+				assert.Contains(t, err.Error(), tt.key)
+				assert.Contains(t, err.Error(), tt.value)
+
+				return
+			}
+
+			require.NoError(t, err)
+			require.NotNil(t, cfg)
+			tt.check(t, cfg)
+		})
+	}
+}
+
+// badEnvValueCase is one row of the config.New table: a variable, the
+// value it is set to, and either the assertion that startup fails
+// naming both, or a check on the Config that resulted.
+type badEnvValueCase struct {
+	name        string
+	key         string
+	value       string
+	expectError bool
+	check       func(t *testing.T, cfg *config.Config)
+}
+
+// badEnvValueCases is the config.New table, kept out of the test body
+// so the test itself stays readable.
+func badEnvValueCases() []badEnvValueCase {
+	return []badEnvValueCase{
 		{
 			name:  "valid PORT is used",
 			key:   envKeyPort,
@@ -361,29 +563,35 @@ func TestNewRejectsBadEnvValues(t *testing.T) {
 			value:       "sometimes",
 			expectError: true,
 		},
-	}
+		{
-
+			name:  "valid BIND_ADDRESS is used",
-	for _, tt := range tests {
+			key:   envKeyBindAddress,
-		t.Run(tt.name, func(t *testing.T) {
+			value: bindAddressWildcard,
-			// Cannot use t.Parallel() here because t.Setenv
+			check: func(t *testing.T, cfg *config.Config) {
-			// is incompatible with parallel subtests.
+				t.Helper()
-			t.Setenv("WEBHOOKER_ENVIRONMENT", "dev")
+				assert.Equal(
-			t.Setenv(tt.key, tt.value)
+					t, bindAddressWildcard, cfg.BindAddress,
-
+				)
-			cfg, err := buildConfig(t)
+			},
-
+		},
-			if tt.expectError {
+		{
-				require.Error(t, err)
+			name:        "unparseable BIND_ADDRESS aborts startup",
-				assert.Contains(t, err.Error(), tt.key)
+			key:         envKeyBindAddress,
-				assert.Contains(t, err.Error(), tt.value)
+			value:       "not-an-address",
-
+			expectError: true,
-				return
+		},
-			}
+		{
-
+			name:        "hostname BIND_ADDRESS aborts startup",
-			require.NoError(t, err)
+			key:         envKeyBindAddress,
-			require.NotNil(t, cfg)
+			value:       "localhost",
-			tt.check(t, cfg)
+			expectError: true,
-		})
+		},
+		{
+			name:        "BIND_ADDRESS with a port aborts startup",
+			key:         envKeyBindAddress,
+			value:       bindAddressDefault + ":8080",
+			expectError: true,
+		},
 	}
 }
 
@@ -395,6 +603,7 @@ func TestNewUsesDefaultsWhenUnset(t *testing.T) {
 
 	for _, key := range []string{
 		envKeyPort, envKeyDebug, envKeyMaintenanceMode,
+		envKeyBindAddress,
 	} {
 		require.NoError(t, os.Unsetenv(key))
 	}
@@ -406,4 +615,15 @@ func TestNewUsesDefaultsWhenUnset(t *testing.T) {
 	assert.Equal(t, 8080, cfg.Port)
 	assert.False(t, cfg.Debug)
 	assert.False(t, cfg.MaintenanceMode)
+
+	// Loopback, not the wildcard: the default must not publish the
+	// cleartext admin UI and the unauthenticated receiver on every
+	// interface of a host that configured nothing. The value is read
+	// from the package rather than repeated, so the README's
+	// documented default and the compiled-in one are pinned to the
+	// same constant.
+	assert.Equal(
+		t, config.DefaultBindAddressForTest, cfg.BindAddress,
+	)
+	assert.Equal(t, bindAddressDefault, cfg.BindAddress)
 }

internal/config/export_test.go

@@ -50,3 +50,13 @@ func EnvPositiveIntForTest(key string, defaultValue int) (int, error) {
 func EnvPortForTest(key string, defaultValue int) (int, error) {
 	return envPort(key, defaultValue)
 }
+
+// EnvBindAddressForTest exposes envBindAddress.
+func EnvBindAddressForTest(key, defaultValue string) (string, error) {
+	return envBindAddress(key, defaultValue)
+}
+
+// DefaultBindAddressForTest exposes the compiled-in BIND_ADDRESS
+// default, so a test pins the documented value rather than repeating
+// a literal that could drift from it.
+const DefaultBindAddressForTest = defaultBindAddress

internal/database/migration_entrypoint_test.go

@@ -1,85 +0,0 @@
-package database_test
-
-import (
-	"net/http"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"sneak.berlin/go/webhooker/internal/database"
-	"sneak.berlin/go/webhooker/internal/signature"
-)
-
-// TestEntrypointSignatureColumnsMigrateToUnconfigured pins the
-// upgrade path for a deployment that already has entrypoints.
-//
-// The signature columns arrive through GORM's AutoMigrate, so every
-// row written before they existed acquires them with no value. That
-// has to land on "not configured", because the alternative is an
-// upgrade that rejects the traffic the operator was already
-// receiving — a self-inflicted outage on a receiver whose senders
-// cannot be told to start signing.
-//
-// The legacy schema is reproduced by dropping the columns from a
-// migrated database and writing a row through the old shape, so the
-// row really predates them rather than merely being blank.
-func TestEntrypointSignatureColumnsMigrateToUnconfigured(t *testing.T) {
-	t.Parallel()
-
-	db, lc := setupTestDB(t)
-	lc.RequireStart()
-
-	t.Cleanup(lc.RequireStop)
-
-	for _, column := range []string{
-		"signature_scheme", "signature_secret",
-	} {
-		require.NoError(
-			t,
-			db.DB().Exec(
-				"ALTER TABLE entrypoints DROP COLUMN "+column,
-			).Error,
-			"dropping %s to reproduce the pre-upgrade schema",
-			column,
-		)
-	}
-
-	const legacyID = "legacy-entrypoint"
-
-	require.NoError(
-		t,
-		db.DB().Exec(
-			`INSERT INTO entrypoints
-			 (id, created_at, updated_at, webhook_id, path,
-			  description, active)
-			 VALUES (?, ?, ?, ?, ?, ?, ?)`,
-			legacyID, "2026-01-01 00:00:00", "2026-01-01 00:00:00",
-			"legacy-webhook", "legacy-path", "predates signatures",
-			true,
-		).Error,
-	)
-
-	// The upgrade.
-	require.NoError(t, db.Migrate())
-
-	var ep database.Entrypoint
-
-	require.NoError(
-		t,
-		db.DB().Where("id = ?", legacyID).First(&ep).Error,
-		"the migrated row must still load; a NULL landing in a "+
-			"string column would fail here",
-	)
-
-	assert.Equal(t, database.SignatureSchemeNone, ep.SignatureScheme)
-	assert.Empty(t, ep.SignatureSecret)
-	assert.False(t, ep.SignatureConfigured())
-	assert.True(t, ep.Active, "the row's other columns survive")
-
-	// The behaviour that actually matters: an unsigned request to
-	// this entrypoint is still accepted.
-	assert.NoError(
-		t,
-		signature.Verify(&ep, http.Header{}, []byte(`{"a":1}`)),
-	)
-}

internal/database/model_entrypoint.go

@@ -1,71 +1,20 @@
 package database
 
-// SignatureScheme names the way an entrypoint authenticates inbound
-// requests. A scheme fixes both the header the signature arrives in
-// and the algorithm used to check it, so an operator cannot pair one
-// sender's header with another sender's comparison.
-type SignatureScheme string
-
-// Signature scheme values. The empty scheme means the entrypoint
-// performs no inbound verification: it is the default, and it is the
-// state every entrypoint created before this column existed migrates
-// to, so an existing deployment keeps accepting the requests it
-// accepted before.
-const (
-	SignatureSchemeNone   SignatureScheme = ""
-	SignatureSchemeGitHub SignatureScheme = "github"
-	SignatureSchemeGitLab SignatureScheme = "gitlab"
-)
-
 // Entrypoint represents an inbound URL endpoint that feeds into a webhook
 type Entrypoint struct {
 	BaseModel
 
 	WebhookID string `gorm:"type:uuid;not null" json:"webhookId"`
 
-	// Path is the URL path for this entrypoint.
+	// Path is the URL path for this entrypoint. It is the
+	// entrypoint's only credential: possession of the UUID
+	// authorises submission, so the receiver checks nothing else
+	// about the sender.
 	Path string `gorm:"uniqueIndex;not null" json:"path"`
 
 	Description string `json:"description"`
 	Active      bool   `gorm:"default:true" json:"active"`
 
-	// SignatureScheme selects how inbound requests to this
-	// entrypoint are authenticated. Empty means unauthenticated,
-	// which is what a UUID-only entrypoint has always been.
-	SignatureScheme SignatureScheme `gorm:"default:''" json:"signatureScheme"`
-
-	// SignatureSecret is the secret shared with the sender.
-	//
-	// It is stored in the clear because HMAC verification needs the
-	// key itself: a hash of it cannot recompute the sender's digest.
-	// It is therefore a live credential, and json:"-" keeps it out of
-	// any handler that marshals the model, the way APIKey.Key and
-	// Target.Config are kept out. handlers.EntrypointView is the
-	// matching barrier for the HTML path.
-	SignatureSecret string `gorm:"default:''" json:"-"`
-
 	// Relations
 	Webhook Webhook `json:"webhook,omitzero"`
 }
-
-// SignatureConfigured reports whether this entrypoint verifies
-// inbound requests. Both halves must be present: a scheme without a
-// secret, or a secret without a scheme, is a broken configuration
-// rather than a configured one, and signature.Verify fails those
-// closed rather than treating them as "off".
-func (e *Entrypoint) SignatureConfigured() bool {
-	return e.SignatureScheme != SignatureSchemeNone &&
-		e.SignatureSecret != ""
-}
-
-// SignatureHalfConfigured reports whether exactly one half of the
-// scheme/secret pair is present. The receiver refuses such a row on
-// every request, so the UI must not describe it as unverified. It
-// reports the state without exposing the secret, which is why it
-// lives here rather than in the display projection.
-func (e *Entrypoint) SignatureHalfConfigured() bool {
-	hasScheme := e.SignatureScheme != SignatureSchemeNone
-	hasSecret := e.SignatureSecret != ""
-
-	return hasScheme != hasSecret
-}

internal/database/model_secrets_test.go

@@ -34,8 +34,6 @@ func marshalModel(t *testing.T, v any) string {
 //   - APIKey.Key is a bearer token outright.
 //   - Setting.Value holds the session encryption key.
 //   - User.Password holds the Argon2 hash, and was already tagged.
-//   - Entrypoint.SignatureSecret is the secret its senders sign with,
-//     stored in the clear because HMAC verification needs the key.
 func TestModelsDoNotMarshalTheirSecrets(t *testing.T) {
 	t.Parallel()
 
@@ -74,14 +72,6 @@ func TestModelsDoNotMarshalTheirSecrets(t *testing.T) {
 				Password: marker,
 			},
 		},
-		{
-			name: "entrypoint signature secret",
-			model: database.Entrypoint{
-				Description:     keptField,
-				SignatureScheme: database.SignatureSchemeGitHub,
-				SignatureSecret: marker,
-			},
-		},
 	}
 
 	for _, tc := range cases {
@@ -115,24 +105,3 @@ func TestWebhookMarshalsNoTargetConfig(t *testing.T) {
 	assert.NotContains(t, encoded, marker)
 	assert.Contains(t, encoded, keptField)
 }
-
-// TestWebhookMarshalsNoEntrypointSecret covers the same nested case
-// for the entrypoint's inbound signature secret, which reaches a
-// marshalled webhook through the Entrypoints association.
-func TestWebhookMarshalsNoEntrypointSecret(t *testing.T) {
-	t.Parallel()
-
-	const marker = "QQENTRYPOINTMARKERQQ"
-
-	encoded := marshalModel(t, database.Webhook{
-		Name: keptField,
-		Entrypoints: []database.Entrypoint{{
-			Path:            "some-uuid",
-			SignatureScheme: database.SignatureSchemeGitLab,
-			SignatureSecret: marker,
-		}},
-	})
-
-	assert.NotContains(t, encoded, marker)
-	assert.Contains(t, encoded, keptField)
-}

internal/delivery/target_http_secret_test.go

@@ -1,142 +0,0 @@
-package delivery_test
-
-import (
-	"context"
-	"encoding/json"
-	"net/http"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"sneak.berlin/go/webhooker/internal/database"
-	"sneak.berlin/go/webhooker/internal/delivery"
-	"sneak.berlin/go/webhooker/internal/signature"
-)
-
-// gitlabDeliverySecret is the shared secret the entrypoint in these
-// tests is configured with. No outbound request may contain it.
-const gitlabDeliverySecret = "QQDELIVERYSECRETQQ"
-
-// receivedEventHeaders builds the Event.Headers value the receiver
-// stores for an inbound request, by running the request's headers
-// through the same sanitizer the receive path uses. Going through
-// signature.SanitizeHeaders rather than a literal is the point of
-// the test: it joins the two egresses at the field they share, so a
-// regression at either end shows up here.
-func receivedEventHeaders(
-	t *testing.T,
-	scheme database.SignatureScheme,
-	inbound http.Header,
-) string {
-	t.Helper()
-
-	ep := &database.Entrypoint{
-		SignatureScheme: scheme,
-		SignatureSecret: gitlabDeliverySecret,
-	}
-
-	encoded, err := json.Marshal(
-		signature.SanitizeHeaders(ep, inbound),
-	)
-	require.NoError(t, err)
-
-	return string(encoded)
-}
-
-// TestApplyRequestHeadersDropsInboundCredential proves a delivery to
-// an HTTP target does not carry the GitLab shared secret.
-//
-// isForwardableHeader is a blocklist of hop-by-hop names, so it
-// forwards X-Gitlab-Token like any other header; what keeps the
-// secret out of the outbound request is that the receiver never
-// stored it. Handing a target operator the token would hand them the
-// ability to forge requests to the entrypoint it authenticates,
-// which is the one control the receiver has.
-func TestApplyRequestHeadersDropsInboundCredential(t *testing.T) {
-	t.Parallel()
-
-	inbound := http.Header{}
-	inbound.Set(signature.HeaderGitLab, gitlabDeliverySecret)
-	inbound.Set("X-Gitlab-Event", "Push Hook")
-
-	event := &database.Event{
-		Headers: receivedEventHeaders(
-			t, database.SignatureSchemeGitLab, inbound,
-		),
-		ContentType: "application/json",
-	}
-
-	req, err := http.NewRequestWithContext(
-		context.Background(),
-		http.MethodPost,
-		"https://target.example.com/hook",
-		http.NoBody,
-	)
-	require.NoError(t, err)
-
-	delivery.ExportApplyRequestHeaders(
-		req, event, &delivery.HTTPTargetConfig{},
-	)
-
-	assert.Empty(
-		t,
-		req.Header.Values(signature.HeaderGitLab),
-		"the shared secret header must not reach a target",
-	)
-
-	// Header.Values canonicalises, so a differently-cased spelling
-	// would be caught above; this catches the value arriving under
-	// some other name.
-	for name, values := range req.Header {
-		for _, v := range values {
-			assert.NotContains(
-				t, v, gitlabDeliverySecret,
-				"secret present in outbound header %s", name,
-			)
-		}
-	}
-
-	// The rest of the sender's headers still arrive. A fix that
-	// dropped everything would pass the assertions above while
-	// breaking delivery.
-	assert.Equal(
-		t,
-		"Push Hook",
-		req.Header.Get("X-Gitlab-Event"),
-	)
-}
-
-// TestApplyRequestHeadersKeepsGitHubDigest proves the stripping is
-// scoped to headers that carry the secret itself. GitHub's
-// X-Hub-Signature-256 is an HMAC over the body, so a target can be
-// shown it without being handed the key.
-func TestApplyRequestHeadersKeepsGitHubDigest(t *testing.T) {
-	t.Parallel()
-
-	const digest = "sha256=deadbeef"
-
-	inbound := http.Header{}
-	inbound.Set(signature.HeaderGitHub, digest)
-
-	event := &database.Event{
-		Headers: receivedEventHeaders(
-			t, database.SignatureSchemeGitHub, inbound,
-		),
-	}
-
-	req, err := http.NewRequestWithContext(
-		context.Background(),
-		http.MethodPost,
-		"https://target.example.com/hook",
-		http.NoBody,
-	)
-	require.NoError(t, err)
-
-	delivery.ExportApplyRequestHeaders(
-		req, event, &delivery.HTTPTargetConfig{},
-	)
-
-	assert.Equal(
-		t, digest, req.Header.Get(signature.HeaderGitHub),
-	)
-}

internal/handlers/entrypoint_secret_test.go

@@ -1,342 +0,0 @@
-package handlers_test
-
-import (
-	"net/http"
-	"net/http/httptest"
-	"net/url"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"sneak.berlin/go/webhooker/internal/database"
-	"sneak.berlin/go/webhooker/internal/handlers"
-	"sneak.berlin/go/webhooker/internal/session"
-)
-
-// submitEntrypointSecret posts the signature configuration form for
-// an entrypoint and returns the recorder.
-func submitEntrypointSecret(
-	t *testing.T,
-	h *handlers.Handlers,
-	cookies []*http.Cookie,
-	webhookID, entrypointID, scheme, secret string,
-) *httptest.ResponseRecorder {
-	t.Helper()
-
-	form := url.Values{}
-	form.Set("signature_scheme", scheme)
-	form.Set("secret", secret)
-
-	req := formRequest(
-		"/source/"+webhookID+"/entrypoints/"+
-			entrypointID+"/secret",
-		cookies,
-		form,
-		map[string]string{
-			paramSourceID:     webhookID,
-			entrypointIDParam: entrypointID,
-		},
-	)
-	w := httptest.NewRecorder()
-
-	h.HandleEntrypointSecret().ServeHTTP(w, req)
-
-	return w
-}
-
-// reloadEntrypoint reads an entrypoint back from the database,
-// including the columns the model keeps out of JSON.
-func reloadEntrypoint(
-	t *testing.T,
-	db *database.Database,
-	id string,
-) database.Entrypoint {
-	t.Helper()
-
-	var ep database.Entrypoint
-
-	require.NoError(
-		t, db.DB().Where("id = ?", id).First(&ep).Error,
-	)
-
-	return ep
-}
-
-// TestEntrypointSecretSetRotateAndRemove walks the whole lifecycle
-// the UI has to support: turning verification on, rotating the secret
-// to a new value, and turning it back off.
-func TestEntrypointSecretSetRotateAndRemove(t *testing.T) {
-	t.Parallel()
-
-	var (
-		h    *handlers.Handlers
-		sess *session.Session
-		db   *database.Database
-	)
-
-	app := newTestApp(t, &h, &sess, &db)
-	app.RequireStart()
-
-	t.Cleanup(app.RequireStop)
-
-	cookies := authenticatedCookies(
-		t, sess, deleteTestUserID, deleteTestUsername,
-	)
-	wh := seedWebhook(t, db)
-	ep := seedSignedEntrypoint(
-		t, db, wh.ID, database.SignatureSchemeNone, "",
-	)
-
-	// Set.
-	w := submitEntrypointSecret(
-		t, h, cookies, wh.ID, ep.ID, "github", inboundSecret,
-	)
-	require.Equal(t, http.StatusSeeOther, w.Code)
-
-	stored := reloadEntrypoint(t, db, ep.ID)
-	assert.Equal(
-		t, database.SignatureSchemeGitHub, stored.SignatureScheme,
-	)
-	assert.Equal(t, inboundSecret, stored.SignatureSecret)
-	assert.True(t, stored.SignatureConfigured())
-
-	// Rotate: a new secret and a different scheme in one submission.
-	// The new value is submitted with surrounding whitespace, the way
-	// a secret pasted out of a password manager arrives; storing that
-	// verbatim would make every later request fail verification with
-	// nothing visible on either side to explain it.
-	const rotated = "QQROTATEDSECRETQQ"
-
-	w = submitEntrypointSecret(
-		t, h, cookies, wh.ID, ep.ID, "gitlab", "  "+rotated+"\t",
-	)
-	require.Equal(t, http.StatusSeeOther, w.Code)
-
-	stored = reloadEntrypoint(t, db, ep.ID)
-	assert.Equal(
-		t, database.SignatureSchemeGitLab, stored.SignatureScheme,
-	)
-	assert.Equal(t, rotated, stored.SignatureSecret)
-
-	// Remove. The secret has to go with the scheme: a stored
-	// credential nothing reads is one more copy to leak.
-	w = submitEntrypointSecret(t, h, cookies, wh.ID, ep.ID, "", "")
-	require.Equal(t, http.StatusSeeOther, w.Code)
-
-	stored = reloadEntrypoint(t, db, ep.ID)
-	assert.Equal(
-		t, database.SignatureSchemeNone, stored.SignatureScheme,
-	)
-	assert.Empty(t, stored.SignatureSecret)
-	assert.False(t, stored.SignatureConfigured())
-}
-
-// TestEntrypointSecretRejectsBadInput proves the form cannot create a
-// row the receiver would later have to refuse. Both rejections leave
-// the stored configuration untouched rather than half-applied.
-func TestEntrypointSecretRejectsBadInput(t *testing.T) {
-	t.Parallel()
-
-	cases := []struct {
-		name   string
-		scheme string
-		secret string
-	}{
-		{
-			name:   "unsupported scheme",
-			scheme: "stripe",
-			secret: inboundSecret,
-		},
-		{
-			name:   "scheme with no secret",
-			scheme: "github",
-			secret: "",
-		},
-		{
-			// Whitespace is stripped, so a secret of spaces is an
-			// empty one.
-			name:   "scheme with blank secret",
-			scheme: "github",
-			secret: "   ",
-		},
-	}
-
-	var (
-		h    *handlers.Handlers
-		sess *session.Session
-		db   *database.Database
-	)
-
-	app := newTestApp(t, &h, &sess, &db)
-	app.RequireStart()
-
-	t.Cleanup(app.RequireStop)
-
-	cookies := authenticatedCookies(
-		t, sess, deleteTestUserID, deleteTestUsername,
-	)
-
-	for _, tc := range cases {
-		wh := seedWebhook(t, db)
-		ep := seedSignedEntrypoint(
-			t, db, wh.ID,
-			database.SignatureSchemeGitLab, inboundSecret,
-		)
-
-		w := submitEntrypointSecret(
-			t, h, cookies, wh.ID, ep.ID, tc.scheme, tc.secret,
-		)
-
-		assert.Equal(
-			t, http.StatusBadRequest, w.Code, "case %s", tc.name,
-		)
-
-		stored := reloadEntrypoint(t, db, ep.ID)
-		assert.Equal(
-			t,
-			database.SignatureSchemeGitLab,
-			stored.SignatureScheme,
-			"case %s", tc.name,
-		)
-		assert.Equal(
-			t, inboundSecret, stored.SignatureSecret,
-			"case %s", tc.name,
-		)
-	}
-}
-
-// TestEntrypointSecretRequiresOwnership proves the configuration
-// endpoint is bound by the same ownership check as the rest of the
-// webhook's pages: another user's entrypoint is a 404, and the secret
-// is not touched.
-func TestEntrypointSecretRequiresOwnership(t *testing.T) {
-	t.Parallel()
-
-	var (
-		h    *handlers.Handlers
-		sess *session.Session
-		db   *database.Database
-	)
-
-	app := newTestApp(t, &h, &sess, &db)
-	app.RequireStart()
-
-	t.Cleanup(app.RequireStop)
-
-	wh := seedWebhook(t, db)
-	ep := seedSignedEntrypoint(
-		t, db, wh.ID,
-		database.SignatureSchemeGitLab, inboundSecret,
-	)
-
-	stranger := authenticatedCookies(
-		t, sess, "someone-else", "someoneelse",
-	)
-
-	w := submitEntrypointSecret(
-		t, h, stranger, wh.ID, ep.ID, "github", "hijacked",
-	)
-
-	assert.Equal(t, http.StatusNotFound, w.Code)
-	assert.Equal(
-		t,
-		inboundSecret,
-		reloadEntrypoint(t, db, ep.ID).SignatureSecret,
-	)
-}
-
-// TestHandleSourceDetail_MasksEntrypointSecret is the regression test
-// for the credential on the entrypoint: the page has to say that
-// verification is configured and which header carries it, without the
-// secret itself ever reaching the rendered HTML.
-func TestHandleSourceDetail_MasksEntrypointSecret(t *testing.T) {
-	t.Parallel()
-
-	var (
-		h    *handlers.Handlers
-		sess *session.Session
-		db   *database.Database
-	)
-
-	app := newTestApp(t, &h, &sess, &db)
-	app.RequireStart()
-
-	t.Cleanup(app.RequireStop)
-
-	wh := seedWebhook(t, db)
-	seedSignedEntrypoint(
-		t, db, wh.ID,
-		database.SignatureSchemeGitHub, inboundSecret,
-	)
-
-	body := renderSourceDetailPage(t, h, sess, wh.ID)
-
-	assert.NotContains(t, body, inboundSecret)
-	assert.Contains(t, body, "GitHub")
-	assert.Contains(t, body, "X-Hub-Signature-256")
-}
-
-// TestEntrypointViewsDropTheSecret pins the projection itself, so the
-// barrier survives a template rewrite that stops rendering the field
-// the page test above looks at.
-func TestEntrypointViewsDropTheSecret(t *testing.T) {
-	t.Parallel()
-
-	views := handlers.NewEntrypointViews([]database.Entrypoint{
-		{
-			Path:            "p1",
-			Active:          true,
-			SignatureScheme: database.SignatureSchemeGitHub,
-			SignatureSecret: inboundSecret,
-		},
-		{
-			Path: "p2",
-		},
-		{
-			// Half a configuration. The receiver 500s every request
-			// to this row, so the UI must not call it unverified.
-			Path:            "p2a",
-			SignatureScheme: database.SignatureSchemeGitLab,
-		},
-		{
-			// The other half.
-			Path:            "p2b",
-			SignatureSecret: inboundSecret,
-		},
-		{
-			// A scheme this build does not know: described as
-			// unavailable, never echoed back.
-			Path:            "p3",
-			SignatureScheme: database.SignatureScheme("stripe"),
-			SignatureSecret: inboundSecret,
-		},
-	})
-
-	require.Len(t, views, 5)
-
-	assert.True(t, views[0].Configured)
-	assert.Equal(t, "GitHub", views[0].SchemeLabel)
-	assert.Equal(t, "X-Hub-Signature-256", views[0].SchemeHeader)
-
-	assert.False(t, views[1].Configured)
-	assert.Equal(t, "not verified", views[1].SchemeLabel)
-	assert.Empty(t, views[1].SchemeHeader)
-
-	for _, v := range []handlers.EntrypointView{views[2], views[3]} {
-		assert.False(t, v.Configured)
-		assert.Equal(t, "misconfigured", v.SchemeLabel)
-		assert.Empty(t, v.SchemeHeader)
-	}
-
-	assert.True(t, views[4].Configured)
-	assert.Equal(t, "(unavailable)", views[4].SchemeLabel)
-
-	// The struct has no field that could carry the secret, so this
-	// fails to compile rather than fails at runtime if one is added
-	// and populated. The assertion covers the labels it derives.
-	for _, v := range views {
-		assert.NotContains(t, v.SchemeLabel, inboundSecret)
-		assert.NotContains(t, v.SchemeHeader, inboundSecret)
-		assert.NotContains(t, string(v.Scheme), inboundSecret)
-	}
-}

internal/handlers/entrypoint_view.go

@@ -2,56 +2,18 @@ package handlers
 
 import (
 	"sneak.berlin/go/webhooker/internal/database"
-	"sneak.berlin/go/webhooker/internal/signature"
 )
 
-// signatureUnavailable is what an entrypoint's scheme renders as when
-// the stored value is not one this build supports. The stored string
-// is never echoed as a fallback: it is operator-supplied and the row
-// is already in a state the receiver refuses, so the UI says so
-// rather than inventing a description for it.
-const signatureUnavailable = "(unavailable)"
-
-// signatureNotVerified is the label for an entrypoint that performs
-// no inbound verification.
-const signatureNotVerified = "not verified"
-
-// signatureMisconfigured is the label for a row holding one half of
-// the scheme/secret pair. The receiver answers every request to such
-// an entrypoint 500, so calling it "not verified" would describe a
-// receiver that is refusing everything as one that is accepting
-// everything. The form cannot create the state; a hand-edited
-// database or a downgrade past a scheme can.
-const signatureMisconfigured = "misconfigured"
-
 // EntrypointView is the display-safe projection of an entrypoint for
-// the UI. It deliberately has no secret field, so no template —
+// the UI, in the same way delivery.TargetView is one for a target.
-// present or future — can render the shared secret, in the same way
-// delivery.TargetView keeps a target's stored credential away from
-// one.
 type EntrypointView struct {
 	ID          string
 	Path        string
 	Description string
 	Active      bool
-
-	// Configured reports whether inbound requests to this entrypoint
-	// are verified.
-	Configured bool
-
-	// Scheme is the stored scheme, carried so the form can preselect
-	// it. It names an algorithm, not a secret.
-	Scheme database.SignatureScheme
-
-	// SchemeLabel and SchemeHeader describe the configured scheme for
-	// display: the sender's name, and the header its signature
-	// arrives in.
-	SchemeLabel  string
-	SchemeHeader string
 }
 
-// NewEntrypointViews projects entrypoints for rendering, dropping the
+// NewEntrypointViews projects entrypoints for rendering.
-// shared secret on the way.
 func NewEntrypointViews(
 	entrypoints []database.Entrypoint,
 ) []EntrypointView {
@@ -60,31 +22,12 @@ func NewEntrypointViews(
 	for i := range entrypoints {
 		e := &entrypoints[i]
 
-		view := EntrypointView{
+		views = append(views, EntrypointView{
-			ID:           e.ID,
+			ID:          e.ID,
-			Path:         e.Path,
+			Path:        e.Path,
-			Description:  e.Description,
+			Description: e.Description,
-			Active:       e.Active,
+			Active:      e.Active,
-			Configured:   e.SignatureConfigured(),
+		})
-			Scheme:       e.SignatureScheme,
-			SchemeLabel:  signatureNotVerified,
-			SchemeHeader: "",
-		}
-
-		switch {
-		case view.Configured:
-			view.SchemeLabel = signatureUnavailable
-
-			info, ok := signature.Info(e.SignatureScheme)
-			if ok {
-				view.SchemeLabel = info.Label
-				view.SchemeHeader = info.Header
-			}
-		case e.SignatureHalfConfigured():
-			view.SchemeLabel = signatureMisconfigured
-		}
-
-		views = append(views, view)
 	}
 
 	return views

internal/handlers/event_resubmit.go

@@ -84,10 +84,6 @@ const resubmitColumns = "id, entrypoint_id, method, headers, " +
 // is the stored EVENT. The response bodies and headers the original
 // deliveries received stay where they are.
 //
-// Inbound signature verification is deliberately not re-run. There is
-// no inbound signature to check on a copy the operator submits; the
-// route is authenticated and CSRF-protected as an operator action.
-//
 // Resubmitting the same event repeatedly is supported and is the point
 // of the feature, so replay's in-flight refusal is deliberately not
 // applied here. The route's rate limit is what bounds a held-down

internal/handlers/footer_version_test.go

@@ -0,0 +1,38 @@
+package handlers_test
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"sneak.berlin/go/webhooker/internal/globals"
+	"sneak.berlin/go/webhooker/internal/handlers"
+	"sneak.berlin/go/webhooker/internal/session"
+)
+
+// The footer in base.html falls back to the literal "dev" when the
+// template data carries no version, which is what every page rendered
+// while nothing supplied one. The operator uses the footer to tell
+// which build is live, so it has to carry the stamped value.
+func TestFooterReportsStampedVersion(t *testing.T) {
+	t.Parallel()
+
+	var (
+		h    *handlers.Handlers
+		sess *session.Session
+		g    *globals.Globals
+	)
+
+	app := newTestApp(t, &h, &sess, &g)
+	app.RequireStart()
+
+	t.Cleanup(app.RequireStop)
+
+	g.Version = "v9.9.9-test"
+
+	html := renderPage(t, h, sess, "login.html", map[string]any{
+		dataKeyError: "",
+	})
+
+	assert.Contains(t, html, "<span>v9.9.9-test</span>")
+	assert.NotContains(t, html, "<span>dev</span>")
+}

internal/handlers/handlers.go

@@ -184,6 +184,7 @@ type UserInfo struct {
 type templateDataWrapper struct {
 	User      *UserInfo
 	CSRFToken string
+	Version   string
 	Data      any
 }
 
@@ -234,9 +235,16 @@ func (s *Handlers) renderTemplate(
 	userInfo := s.getUserInfo(r)
 	csrfToken := middleware.CSRFToken(r)
 
+	// The footer in base.html renders .Version. Every page reaches it
+	// through here, so this is the one place that has to supply it;
+	// left unset, the footer falls back to its literal "dev" and the
+	// UI reports a build that is not the one running.
+	version := s.params.Globals.Version
+
 	if m, ok := data.(map[string]any); ok {
 		m["User"] = userInfo
 		m["CSRFToken"] = csrfToken
+		m["Version"] = version
 		s.executeTemplate(w, tmpl, m)
 
 		return
@@ -245,6 +253,7 @@ func (s *Handlers) renderTemplate(
 	wrapper := templateDataWrapper{
 		User:      userInfo,
 		CSRFToken: csrfToken,
+		Version:   version,
 		Data:      data,
 	}
 

internal/handlers/source_delete_test.go

@@ -2,6 +2,7 @@ package handlers_test
 
 import (
 	"context"
+	"errors"
 	"net/http"
 	"net/http/httptest"
 	"os"
@@ -11,6 +12,7 @@ import (
 	"github.com/go-chi/chi"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"gorm.io/gorm"
 	"gorm.io/gorm/clause"
 	"sneak.berlin/go/webhooker/internal/database"
 	"sneak.berlin/go/webhooker/internal/handlers"
@@ -73,6 +75,77 @@ func seedTarget(
 	return tgt
 }
 
+// errInjectedDelete is the failure failDeleteOnTable reports
+// from a delete statement.
+var errInjectedDelete = errors.New("injected delete failure")
+
+// seedEntrypoint inserts an entrypoint for a webhook.
+func seedEntrypoint(
+	t *testing.T,
+	db *database.Database,
+	webhookID string,
+) {
+	t.Helper()
+
+	ep := &database.Entrypoint{
+		WebhookID: webhookID,
+		Path:      "ep-" + webhookID,
+		Active:    true,
+	}
+
+	require.NoError(
+		t,
+		db.DB().Omit(clause.Associations).Create(ep).Error,
+	)
+}
+
+// countRows counts the live (not soft-deleted) rows of a model
+// matching column = value.
+func countRows(
+	t *testing.T,
+	db *database.Database,
+	model any,
+	column, value string,
+) int64 {
+	t.Helper()
+
+	var n int64
+
+	require.NoError(
+		t,
+		db.DB().Model(model).
+			Where(column+" = ?", value).
+			Count(&n).Error,
+	)
+
+	return n
+}
+
+// failDeleteOnTable makes every delete against the named table
+// fail the way a database-level error does: the statement
+// reports an error but leaves the surrounding transaction
+// usable, so a caller that does not check it can go on to
+// commit the statements that did succeed.
+func failDeleteOnTable(
+	t *testing.T,
+	db *database.Database,
+	table string,
+) {
+	t.Helper()
+
+	require.NoError(t, db.DB().Callback().Delete().
+		Before("gorm:delete").
+		Register(
+			"test:fail_delete_"+table,
+			func(tx *gorm.DB) {
+				if tx.Statement.Table == table {
+					_ = tx.AddError(errInjectedDelete)
+				}
+			},
+		),
+	)
+}
+
 // archivePathFor returns the archive database path the
 // delivery engine would use for a webhook: beside the webhook's
 // event database in the data directory.
@@ -209,6 +282,159 @@ func TestHandleSourceDelete_KeepsArchiveFile(t *testing.T) {
 	)
 }
 
+// TestHandleSourceDelete_FailedDeleteKeepsEverything proves
+// that a failing delete statement loses nothing: the
+// configuration is rolled back whole, the event database
+// survives, and the operator is told the deletion failed
+// instead of being redirected as though it worked.
+func TestHandleSourceDelete_FailedDeleteKeepsEverything(
+	t *testing.T,
+) {
+	t.Parallel()
+
+	var (
+		h    *handlers.Handlers
+		sess *session.Session
+		db   *database.Database
+		mgr  *database.WebhookDBManager
+	)
+
+	app := newTestApp(t, &h, &sess, &db, &mgr)
+	app.RequireStart()
+
+	t.Cleanup(app.RequireStop)
+
+	wh := seedWebhook(t, db)
+	seedEntrypoint(t, db, wh.ID)
+	seedTarget(t, db, wh.ID, database.TargetTypeDatabase)
+
+	require.NoError(t, mgr.CreateDB(wh.ID))
+
+	eventDBPath := mgr.DBPath(wh.ID)
+	require.FileExists(t, eventDBPath)
+
+	// The entrypoint delete runs first and succeeds; the target
+	// delete then fails, which is what the whole transaction has
+	// to be rolled back over.
+	failDeleteOnTable(t, db, "targets")
+
+	cookies := authenticatedCookies(
+		t, sess, deleteTestUserID, deleteTestUsername,
+	)
+
+	req := postRequest(
+		"/source/"+wh.ID+"/delete",
+		cookies,
+		map[string]string{paramSourceID: wh.ID},
+	)
+	w := httptest.NewRecorder()
+
+	h.HandleSourceDelete().ServeHTTP(w, req)
+
+	assert.Equal(
+		t, http.StatusInternalServerError, w.Code,
+		"a failed deletion must be reported, not redirected",
+	)
+	assert.Empty(
+		t, w.Header().Get("Location"),
+		"a failed deletion must not redirect to /sources",
+	)
+
+	assert.Equal(
+		t, int64(1),
+		countRows(t, db, &database.Webhook{}, "id", wh.ID),
+		"the webhook must survive a failed deletion",
+	)
+	assert.Equal(
+		t, int64(1),
+		countRows(
+			t, db, &database.Entrypoint{}, "webhook_id", wh.ID,
+		),
+		"the entrypoint delete must be rolled back",
+	)
+	assert.Equal(
+		t, int64(1),
+		countRows(
+			t, db, &database.Target{}, "webhook_id", wh.ID,
+		),
+		"the target must survive a failed deletion",
+	)
+
+	assert.FileExists(
+		t, eventDBPath,
+		"event history must not be destroyed when the "+
+			"configuration delete did not commit",
+	)
+}
+
+// TestHandleSourceDelete_RemovesConfigAndEventDatabase is the
+// positive control for the rollback above: an ordinary deletion
+// still removes the webhook, its children and its event
+// database.
+func TestHandleSourceDelete_RemovesConfigAndEventDatabase(
+	t *testing.T,
+) {
+	t.Parallel()
+
+	var (
+		h    *handlers.Handlers
+		sess *session.Session
+		db   *database.Database
+		mgr  *database.WebhookDBManager
+	)
+
+	app := newTestApp(t, &h, &sess, &db, &mgr)
+	app.RequireStart()
+
+	t.Cleanup(app.RequireStop)
+
+	wh := seedWebhook(t, db)
+	seedEntrypoint(t, db, wh.ID)
+	seedTarget(t, db, wh.ID, database.TargetTypeDatabase)
+
+	require.NoError(t, mgr.CreateDB(wh.ID))
+
+	eventDBPath := mgr.DBPath(wh.ID)
+	require.FileExists(t, eventDBPath)
+
+	cookies := authenticatedCookies(
+		t, sess, deleteTestUserID, deleteTestUsername,
+	)
+
+	req := postRequest(
+		"/source/"+wh.ID+"/delete",
+		cookies,
+		map[string]string{paramSourceID: wh.ID},
+	)
+	w := httptest.NewRecorder()
+
+	h.HandleSourceDelete().ServeHTTP(w, req)
+
+	require.Equal(t, http.StatusSeeOther, w.Code)
+	assert.Equal(t, "/sources", w.Header().Get("Location"))
+
+	assert.Equal(
+		t, int64(0),
+		countRows(t, db, &database.Webhook{}, "id", wh.ID),
+	)
+	assert.Equal(
+		t, int64(0),
+		countRows(
+			t, db, &database.Entrypoint{}, "webhook_id", wh.ID,
+		),
+	)
+	assert.Equal(
+		t, int64(0),
+		countRows(
+			t, db, &database.Target{}, "webhook_id", wh.ID,
+		),
+	)
+	assert.NoFileExists(
+		t, eventDBPath,
+		"a successful deletion removes the event database",
+	)
+}
+
 // TestHandleTargetDelete_EvictsWhenLastDatabaseTargetGone
 // proves that removing the last database target releases the
 // archive writer.

internal/handlers/source_management.go

@@ -13,7 +13,6 @@ import (
 	"gorm.io/gorm"
 	"sneak.berlin/go/webhooker/internal/database"
 	"sneak.berlin/go/webhooker/internal/delivery"
-	"sneak.berlin/go/webhooker/internal/signature"
 )
 
 // WebhookListItem holds data for the webhook list view.
@@ -443,16 +442,13 @@ func (h *Handlers) renderSourceDetail(
 	// receivers; html/template cannot address a value stored in a map.
 	data := map[string]any{
 		tmplKeyWebhook: &webhook,
-		// Entrypoints and targets are both projected to
+		// Targets are projected to a display-safe view: a
-		// display-safe views: an entrypoint carries the shared
+		// target's stored config blob holds a credential, and it
-		// secret its senders sign with and a target's stored
+		// must never reach a template.
-		// config blob holds a credential, and neither must ever
+		"Entrypoints": NewEntrypointViews(entrypoints),
-		// reach a template.
+		"Targets":     delivery.NewTargetViews(targets),
-		"Entrypoints":      NewEntrypointViews(entrypoints),
+		"Events":      events,
-		"Targets":          delivery.NewTargetViews(targets),
+		"BaseURL":     scheme + "://" + host,
-		"SignatureSchemes": signature.Schemes(),
-		"Events":           events,
-		"BaseURL":          scheme + "://" + host,
 	}
 
 	h.renderTemplate(w, r, "source_detail.html", data)
@@ -625,43 +621,27 @@ func (h *Handlers) deleteWebhookResources(
 	webhook database.Webhook,
 	userID string,
 ) {
-	tx := h.db.DB().Begin()
+	// The configuration delete commits before the event database
-	if tx.Error != nil {
+	// is touched. No transaction spans the main database and the
-		h.log.Error(
+	// filesystem, so one side has to go first: committing the
-			"failed to begin transaction",
+	// configuration first means a later failure leaves an unused
-			"error", tx.Error,
+	// event database file on disk, while removing the event
-		)
+	// database first would mean a failed commit destroys the
-		http.Error(
+	// history of a webhook that still exists. A leftover file can
-			w, "Internal server error",
+	// be removed by hand; deleted history cannot be recovered.
-			http.StatusInternalServerError,
+	err := h.commitWebhookDeletion(&webhook)
-		)
-
-		return
-	}
-
-	tx.Where(
-		"webhook_id = ?", webhook.ID,
-	).Delete(&database.Entrypoint{})
-
-	tx.Where(
-		"webhook_id = ?", webhook.ID,
-	).Delete(&database.Target{})
-
-	tx.Delete(&webhook)
-
-	err := tx.Commit().Error
 	if err != nil {
-		h.log.Error(
+		h.serverError(w, "failed to delete webhook", err)
-			"failed to commit deletion", "error", err,
-		)
-		http.Error(
-			w, "Internal server error",
-			http.StatusInternalServerError,
-		)
 
 		return
 	}
 
+	h.log.Info(
+		"webhook deleted",
+		"webhook_id", webhook.ID,
+		"user_id", userID,
+	)
+
 	// Release the delivery engine's per-webhook archiving state
 	// so a deleted webhook's archive writer (and any handle open
 	// within its debounce window) does not linger for the
@@ -671,22 +651,63 @@ func (h *Handlers) deleteWebhookResources(
 
 	err = h.dbMgr.DeleteDB(webhook.ID)
 	if err != nil {
-		h.log.Error(
+		// The configuration is committed, so the webhook is gone,
-			"failed to delete webhook event database",
+		// but its event database file is still on disk with
-			"webhook_id", webhook.ID,
+		// nothing referencing it. Report the failure rather than
-			"error", err,
+		// redirecting as though everything succeeded: the file
+		// needs removing by hand, and the logged error names it.
+		h.serverError(
+			w, "failed to delete webhook event database", err,
 		)
+
+		return
 	}
 
-	h.log.Info(
-		"webhook deleted",
-		"webhook_id", webhook.ID,
-		"user_id", userID,
-	)
-
 	http.Redirect(w, r, "/sources", http.StatusSeeOther)
 }
 
+// commitWebhookDeletion soft-deletes a webhook's entrypoints,
+// targets and the webhook row in one transaction. Every
+// statement is checked and any failure rolls the whole
+// transaction back, so a caller that gets an error knows the
+// configuration is untouched and the event database must be
+// left alone.
+func (h *Handlers) commitWebhookDeletion(
+	webhook *database.Webhook,
+) error {
+	tx := h.db.DB().Begin()
+	if tx.Error != nil {
+		return tx.Error
+	}
+
+	err := tx.Where(
+		"webhook_id = ?", webhook.ID,
+	).Delete(&database.Entrypoint{}).Error
+	if err != nil {
+		tx.Rollback()
+
+		return err
+	}
+
+	err = tx.Where(
+		"webhook_id = ?", webhook.ID,
+	).Delete(&database.Target{}).Error
+	if err != nil {
+		tx.Rollback()
+
+		return err
+	}
+
+	err = tx.Delete(webhook).Error
+	if err != nil {
+		tx.Rollback()
+
+		return err
+	}
+
+	return tx.Commit().Error
+}
+
 // evictArchiveWriter asks the delivery engine to drop its
 // cached archive writer for a webhook, closing the archive file
 // handle.
@@ -1240,145 +1261,6 @@ func (h *Handlers) HandleEntrypointCreate() http.HandlerFunc {
 	}
 }
 
-// HandleEntrypointSecret sets, rotates or removes the shared secret
-// an entrypoint verifies inbound requests with.
-//
-// Setting and rotating are the same operation: the form always takes
-// the secret afresh and the stored value is never sent to the browser
-// to be edited, so there is no path by which the page can display a
-// credential it holds. Rotation is therefore "submit the new secret",
-// and the operator already has that value — both supported senders
-// require them to enter the same string on the sender's side, so
-// there is no generated value for webhooker to reveal once.
-func (h *Handlers) HandleEntrypointSecret() http.HandlerFunc {
-	return func(w http.ResponseWriter, r *http.Request) {
-		webhook, ok := h.ownedWebhook(w, r)
-		if !ok {
-			return
-		}
-
-		// The body size cap is enforced by the MaxBodySize
-		// middleware, which runs before CSRF parses the form.
-		err := r.ParseForm()
-		if err != nil {
-			http.Error(
-				w, "Bad request", http.StatusBadRequest,
-			)
-
-			return
-		}
-
-		var entrypoint database.Entrypoint
-
-		err = h.db.DB().Where(
-			"id = ? AND webhook_id = ?",
-			chi.URLParam(r, "entrypointID"), webhook.ID,
-		).First(&entrypoint).Error
-		if err != nil {
-			http.NotFound(w, r)
-
-			return
-		}
-
-		h.applyEntrypointSecret(w, r, &entrypoint)
-	}
-}
-
-// applyEntrypointSecret validates the submitted scheme and secret and
-// stores them.
-//
-// A scheme this build does not support is a 400, never a stored value
-// the receiver would later have to interpret: the receiver fails such
-// a row closed, so letting one be created would take the entrypoint
-// offline through a form that reported success.
-func (h *Handlers) applyEntrypointSecret(
-	w http.ResponseWriter,
-	r *http.Request,
-	entrypoint *database.Entrypoint,
-) {
-	// PostFormValue, not FormValue: a credential must come from the
-	// body. FormValue falls back to the query string, and the request
-	// line — unlike the body — is what logs, proxies, Referer headers
-	// and error trackers record.
-	scheme := database.SignatureScheme(
-		r.PostFormValue("signature_scheme"),
-	)
-
-	// Surrounding whitespace is stripped, because a secret pasted from
-	// a password manager routinely carries some and the resulting
-	// mismatch is undiagnosable from the sender's side. A secret whose
-	// own first or last character is a space cannot be stored; the
-	// README says so.
-	secret := strings.TrimSpace(r.PostFormValue("secret"))
-
-	if !signature.Supported(scheme) {
-		http.Error(
-			w, "Invalid signature scheme",
-			http.StatusBadRequest,
-		)
-
-		return
-	}
-
-	if scheme == database.SignatureSchemeNone {
-		// Turning verification off drops the secret with it: a stored
-		// credential nothing reads is one more copy to leak, and
-		// Verify refuses that pairing in any case.
-		secret = ""
-	} else if secret == "" {
-		http.Error(
-			w,
-			"A shared secret is required for this signature scheme.",
-			http.StatusBadRequest,
-		)
-
-		return
-	}
-
-	h.storeEntrypointSecret(w, r, entrypoint, scheme, secret)
-}
-
-// storeEntrypointSecret writes a validated scheme and secret to an
-// entrypoint and returns the operator to the webhook page.
-func (h *Handlers) storeEntrypointSecret(
-	w http.ResponseWriter,
-	r *http.Request,
-	entrypoint *database.Entrypoint,
-	scheme database.SignatureScheme,
-	secret string,
-) {
-	// Updates with a map rather than a struct: a struct update skips
-	// zero values, and the empty pair is exactly what has to be
-	// written when verification is being turned off.
-	err := h.db.DB().Model(entrypoint).Updates(map[string]any{
-		"signature_scheme": scheme,
-		"signature_secret": secret,
-	}).Error
-	if err != nil {
-		// The error is logged by serverError; GORM's error text
-		// carries the statement, not the bound values, so the secret
-		// does not travel with it.
-		h.serverError(
-			w, "failed to update entrypoint signature", err,
-		)
-
-		return
-	}
-
-	h.log.Info(
-		"entrypoint signature configuration updated",
-		"entrypoint_id", entrypoint.ID,
-		"webhook_id", entrypoint.WebhookID,
-		"scheme", string(scheme),
-	)
-
-	http.Redirect(
-		w, r,
-		"/source/"+entrypoint.WebhookID,
-		http.StatusSeeOther,
-	)
-}
-
 // HandleTargetCreate handles adding a new target to a webhook.
 func (h *Handlers) HandleTargetCreate() http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {

internal/handlers/ui_copy_test.go

@@ -13,7 +13,6 @@ import (
 	"sneak.berlin/go/webhooker/internal/delivery"
 	"sneak.berlin/go/webhooker/internal/handlers"
 	"sneak.berlin/go/webhooker/internal/session"
-	"sneak.berlin/go/webhooker/internal/signature"
 )
 
 // Template data keys the page templates read. The handlers package has
@@ -269,16 +268,15 @@ func TestEntrypointCopyButtonIsProgressiveEnhancement(t *testing.T) {
 
 	body := renderPage(t, h, sess, "source_detail.html", map[string]any{
 		dataKeyWebhook: webhook,
-		// The handler passes projected views, never raw rows — an
+		// The handler passes projected views, never raw rows — a
-		// entrypoint carries its shared secret and a target its
+		// target carries its stored credential — so the test data
-		// stored credential — so the test data has that same shape.
+		// has that same shape.
 		"Entrypoints": handlers.NewEntrypointViews(
 			[]database.Entrypoint{entrypoint},
 		),
-		"Targets":          delivery.NewTargetViews(nil),
+		"Targets": delivery.NewTargetViews(nil),
-		"SignatureSchemes": signature.Schemes(),
+		"Events":  []database.Event{},
-		"Events":           []database.Event{},
+		"BaseURL": "https://hooks.example.com",
-		"BaseURL":          "https://hooks.example.com",
 	})
 
 	assert.Contains(

internal/handlers/webhook.go

@@ -2,7 +2,6 @@ package handlers
 
 import (
 	"encoding/json"
-	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -12,7 +11,6 @@ import (
 	"sneak.berlin/go/webhooker/internal/database"
 	"sneak.berlin/go/webhooker/internal/delivery"
 	"sneak.berlin/go/webhooker/internal/logfield"
-	"sneak.berlin/go/webhooker/internal/signature"
 )
 
 const (
@@ -72,8 +70,12 @@ func (h *Handlers) HandleWebhook() http.HandlerFunc {
 	}
 }
 
-// processWebhookRequest reads the body, verifies the sender,
+// processWebhookRequest reads the body, serializes headers, loads
-// serializes headers, loads targets, and delivers the event.
+// targets, and delivers the event.
+//
+// Nothing about the request itself is authenticated: the entrypoint
+// UUID in the path is the credential, and reaching here means it
+// matched an active entrypoint.
 func (h *Handlers) processWebhookRequest(
 	w http.ResponseWriter,
 	r *http.Request,
@@ -84,26 +86,7 @@ func (h *Handlers) processWebhookRequest(
 		return
 	}
 
-	// Before anything is written. An unverified request must leave no
+	headersJSON, err := json.Marshal(r.Header)
-	// event row, no delivery row and no delivery task behind, so this
-	// sits above every write rather than inside the transaction that
-	// performs them. It has to sit below the body read because the
-	// signature is computed over the body; readWebhookBody is what
-	// bounds that read, so an unauthenticated sender still cannot make
-	// the process hold more than the 1 MB cap.
-	if !h.verifyInboundSignature(w, entrypoint, r.Header, body) {
-		return
-	}
-
-	// These headers are about to be stored verbatim and handed to
-	// every delivery target, so the scheme's credential comes out
-	// first. Under GitLab's scheme the header is the shared secret
-	// itself, and leaving it in would hand the ability to forge
-	// signed requests to exactly the parties the signature is meant
-	// to exclude.
-	headersJSON, err := json.Marshal(
-		signature.SanitizeHeaders(&entrypoint, r.Header),
-	)
 	if err != nil {
 		h.serverError(w, "failed to serialize headers", err)
 
@@ -122,63 +105,6 @@ func (h *Handlers) processWebhookRequest(
 	)
 }
 
-// verifyInboundSignature authenticates the request against the
-// entrypoint's configured secret, reporting false once it has written
-// the response.
-//
-// An entrypoint with no secret configured is not checked and this
-// returns true, which is the unchanged behaviour every existing
-// entrypoint keeps.
-//
-// A configuration that cannot be applied — an unknown scheme, or one
-// half of the pair missing — is a 500, not a 401: the request may well
-// be authentic, and calling it unauthorized would tell a legitimate
-// sender to go fix its own signing. Either way it is refused. Failing
-// open here would mean an entrypoint the operator has protected
-// quietly accepting anything.
-func (h *Handlers) verifyInboundSignature(
-	w http.ResponseWriter,
-	entrypoint database.Entrypoint,
-	header http.Header,
-	body []byte,
-) bool {
-	err := signature.Verify(&entrypoint, header, body)
-	if err == nil {
-		return true
-	}
-
-	if errors.Is(err, signature.ErrConfig) {
-		h.log.Error(
-			"entrypoint signature configuration cannot be applied",
-			"entrypoint_id", entrypoint.ID,
-			"webhook_id", entrypoint.WebhookID,
-			"error", err,
-		)
-		http.Error(
-			w, "Internal server error",
-			http.StatusInternalServerError,
-		)
-
-		return false
-	}
-
-	// Every field here is bounded and none is client-chosen: the ids
-	// are ours, the scheme is one of a fixed set, and the error is a
-	// static string carrying no part of the secret or of what the
-	// client presented. Reaching this line also requires a real
-	// entrypoint UUID, so it is not a line a stranger can drive.
-	h.log.Warn(
-		"inbound signature verification failed",
-		"entrypoint_id", entrypoint.ID,
-		"webhook_id", entrypoint.WebhookID,
-		"scheme", string(entrypoint.SignatureScheme),
-		"error", err,
-	)
-	http.Error(w, "Unauthorized", http.StatusUnauthorized)
-
-	return false
-}
-
 // loadActiveTargets returns all active targets for a webhook.
 func (h *Handlers) loadActiveTargets(
 	webhookID string,

internal/handlers/webhook_signature_test.go

@@ -1,468 +0,0 @@
-package handlers_test
-
-import (
-	"bytes"
-	"context"
-	"crypto/hmac"
-	"crypto/sha256"
-	"encoding/hex"
-	"log/slog"
-	"net/http"
-	"net/http/httptest"
-	"strings"
-	"testing"
-
-	"github.com/go-chi/chi"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"gorm.io/gorm/clause"
-	"sneak.berlin/go/webhooker/internal/database"
-	"sneak.berlin/go/webhooker/internal/handlers"
-	"sneak.berlin/go/webhooker/internal/signature"
-)
-
-const (
-	// inboundSecret is the shared secret the signed-receiver tests
-	// configure on their entrypoint. It doubles as a marker: no log
-	// line and no rendered page may contain it.
-	inboundSecret = "QQINBOUNDSECRETQQ"
-
-	// inboundBody is the payload the sender signs.
-	inboundBody = `{"zen":"Non-blocking is better than blocking."}`
-
-	// entrypointIDParam is the chi URL parameter naming an entrypoint.
-	entrypointIDParam = "entrypointID"
-)
-
-// hubSignature returns the X-Hub-Signature-256 value a GitHub sender
-// holding secret would send for inboundBody.
-func hubSignature(secret string) string {
-	mac := hmac.New(sha256.New, []byte(secret))
-	_, _ = mac.Write([]byte(inboundBody))
-
-	return "sha256=" + hex.EncodeToString(mac.Sum(nil))
-}
-
-// seedSignedEntrypoint inserts an active entrypoint for a webhook
-// with the given signature configuration and returns it.
-func seedSignedEntrypoint(
-	t *testing.T,
-	db *database.Database,
-	webhookID string,
-	scheme database.SignatureScheme,
-	secret string,
-) *database.Entrypoint {
-	t.Helper()
-
-	ep := &database.Entrypoint{
-		WebhookID:       webhookID,
-		Path:            "path-" + webhookID,
-		Description:     "signed",
-		Active:          true,
-		SignatureScheme: scheme,
-		SignatureSecret: secret,
-	}
-
-	require.NoError(
-		t,
-		db.DB().Omit(clause.Associations).Create(ep).Error,
-	)
-
-	return ep
-}
-
-// postToEntrypoint drives the real receiver handler at an
-// entrypoint's path with one optional header set.
-func postToEntrypoint(
-	t *testing.T,
-	h *handlers.Handlers,
-	path, body, headerName, headerValue string,
-) *httptest.ResponseRecorder {
-	t.Helper()
-
-	req := httptest.NewRequestWithContext(
-		context.Background(),
-		http.MethodPost,
-		"/webhook/"+path,
-		strings.NewReader(body),
-	)
-	req.Header.Set("Content-Type", "application/json")
-
-	if headerName != "" {
-		req.Header.Set(headerName, headerValue)
-	}
-
-	rctx := chi.NewRouteContext()
-	rctx.URLParams.Add("uuid", path)
-
-	req = req.WithContext(
-		context.WithValue(req.Context(), chi.RouteCtxKey, rctx),
-	)
-
-	w := httptest.NewRecorder()
-	h.HandleWebhook().ServeHTTP(w, req)
-
-	return w
-}
-
-// storedEvents counts the event rows a webhook's per-webhook database
-// holds. A database that was never opened holds none, which is the
-// state a rejected request has to leave behind.
-func storedEvents(
-	t *testing.T,
-	mgr *database.WebhookDBManager,
-	webhookID string,
-) int64 {
-	t.Helper()
-
-	if !mgr.DBExists(webhookID) {
-		return 0
-	}
-
-	db, err := mgr.GetDB(webhookID)
-	require.NoError(t, err)
-
-	var count int64
-
-	require.NoError(
-		t,
-		db.Model(&database.Event{}).
-			Where("webhook_id = ?", webhookID).
-			Count(&count).Error,
-	)
-
-	return count
-}
-
-// storedEventHeaders reads back the Headers column of the single
-// event row a webhook's per-webhook database holds.
-//
-// It reads the database rather than an in-memory struct on purpose:
-// what matters is what an operator, a backup or the reaper's archive
-// would find on disk, not what the handler passed around.
-func storedEventHeaders(
-	t *testing.T,
-	mgr *database.WebhookDBManager,
-	webhookID string,
-) string {
-	t.Helper()
-
-	require.True(t, mgr.DBExists(webhookID))
-
-	db, err := mgr.GetDB(webhookID)
-	require.NoError(t, err)
-
-	var events []database.Event
-
-	require.NoError(
-		t,
-		db.Where("webhook_id = ?", webhookID).
-			Find(&events).Error,
-	)
-	require.Len(t, events, 1)
-
-	return events[0].Headers
-}
-
-// signedReceiverCase is one inbound request against an entrypoint
-// with a given stored signature configuration.
-type signedReceiverCase struct {
-	name        string
-	scheme      database.SignatureScheme
-	secret      string
-	headerName  string
-	headerValue string
-	body        string
-	wantStatus  int
-}
-
-// signedReceiverCases covers each supported scheme with a valid
-// signature, an invalid one and none at all, plus the two states that
-// are not "a client got it wrong": an entrypoint with nothing
-// configured, and one whose stored configuration cannot be applied.
-func signedReceiverCases() []signedReceiverCase {
-	return append(
-		schemeReceiverCases(), unverifiedReceiverCases()...,
-	)
-}
-
-// schemeReceiverCases covers the two supported schemes.
-func schemeReceiverCases() []signedReceiverCase {
-	return []signedReceiverCase{
-		{
-			name:        "github valid",
-			scheme:      database.SignatureSchemeGitHub,
-			secret:      inboundSecret,
-			headerName:  signature.HeaderGitHub,
-			headerValue: hubSignature(inboundSecret),
-			body:        inboundBody,
-			wantStatus:  http.StatusOK,
-		},
-		{
-			name:        "github wrong secret",
-			scheme:      database.SignatureSchemeGitHub,
-			secret:      inboundSecret,
-			headerName:  signature.HeaderGitHub,
-			headerValue: hubSignature("wrong"),
-			body:        inboundBody,
-			wantStatus:  http.StatusUnauthorized,
-		},
-		{
-			// A digest that was valid for a different body: the
-			// check is over the bytes as received.
-			name:        "github body tampered",
-			scheme:      database.SignatureSchemeGitHub,
-			secret:      inboundSecret,
-			headerName:  signature.HeaderGitHub,
-			headerValue: hubSignature(inboundSecret),
-			body:        inboundBody + " ",
-			wantStatus:  http.StatusUnauthorized,
-		},
-		{
-			name:       "github unsigned",
-			scheme:     database.SignatureSchemeGitHub,
-			secret:     inboundSecret,
-			body:       inboundBody,
-			wantStatus: http.StatusUnauthorized,
-		},
-		{
-			name:        "gitlab valid",
-			scheme:      database.SignatureSchemeGitLab,
-			secret:      inboundSecret,
-			headerName:  signature.HeaderGitLab,
-			headerValue: inboundSecret,
-			body:        inboundBody,
-			wantStatus:  http.StatusOK,
-		},
-		{
-			name:        "gitlab wrong token",
-			scheme:      database.SignatureSchemeGitLab,
-			secret:      inboundSecret,
-			headerName:  signature.HeaderGitLab,
-			headerValue: "wrong",
-			body:        inboundBody,
-			wantStatus:  http.StatusUnauthorized,
-		},
-		{
-			name:       "gitlab unsigned",
-			scheme:     database.SignatureSchemeGitLab,
-			secret:     inboundSecret,
-			body:       inboundBody,
-			wantStatus: http.StatusUnauthorized,
-		},
-	}
-}
-
-// unverifiedReceiverCases covers the two entrypoint states that are
-// not about a client getting its signature wrong: nothing configured
-// at all, and a configuration the receiver cannot apply.
-func unverifiedReceiverCases() []signedReceiverCase {
-	return []signedReceiverCase{
-		{
-			// The pass-through case. An entrypoint with nothing
-			// configured is what every deployment already has, and
-			// it must keep accepting unsigned requests so that an
-			// upgrade does not lock an operator out of their own
-			// receivers.
-			name:       "unconfigured accepts unsigned",
-			scheme:     database.SignatureSchemeNone,
-			body:       inboundBody,
-			wantStatus: http.StatusOK,
-		},
-		{
-			// A stray signature header changes nothing when nothing
-			// is configured to check it.
-			name:        "unconfigured ignores a stray header",
-			scheme:      database.SignatureSchemeNone,
-			headerName:  signature.HeaderGitHub,
-			headerValue: "sha256=deadbeef",
-			body:        inboundBody,
-			wantStatus:  http.StatusOK,
-		},
-		{
-			// A scheme this build cannot apply, reachable only by
-			// editing the database: refused, not waved through as
-			// unverified.
-			name:        "unknown scheme fails closed",
-			scheme:      database.SignatureScheme("stripe"),
-			secret:      inboundSecret,
-			headerName:  signature.HeaderGitHub,
-			headerValue: hubSignature(inboundSecret),
-			body:        inboundBody,
-			wantStatus:  http.StatusInternalServerError,
-		},
-	}
-}
-
-// TestReceiverVerifiesConfiguredEntrypoints is the load-bearing test
-// for the feature: for each supported scheme a correctly signed
-// request is accepted and stored, and an incorrectly signed or
-// unsigned one is answered 401 having stored nothing.
-//
-// The event count is the half that matters most. A rejection that
-// still wrote a row would leave the receiver a place for a stranger
-// who knows a URL to deposit content, which is exactly what the
-// signature is there to prevent.
-//
-// The cases share one application and take a webhook each, rather
-// than each standing up its own: every newTestApp seeds an admin user
-// and so pays an Argon2id hash at 64 MB, and this package's test
-// budget is not large enough to spend one per table row.
-func TestReceiverVerifiesConfiguredEntrypoints(t *testing.T) {
-	t.Parallel()
-
-	var (
-		h   *handlers.Handlers
-		db  *database.Database
-		mgr *database.WebhookDBManager
-	)
-
-	app := newTestApp(t, &h, &db, &mgr)
-	app.RequireStart()
-
-	t.Cleanup(app.RequireStop)
-
-	for _, tc := range signedReceiverCases() {
-		wh := seedWebhook(t, db)
-		ep := seedSignedEntrypoint(
-			t, db, wh.ID, tc.scheme, tc.secret,
-		)
-
-		w := postToEntrypoint(
-			t, h, ep.Path, tc.body,
-			tc.headerName, tc.headerValue,
-		)
-
-		assert.Equal(t, tc.wantStatus, w.Code, "case %s", tc.name)
-
-		want := int64(0)
-		if tc.wantStatus == http.StatusOK {
-			want = 1
-		}
-
-		assert.Equal(
-			t, want, storedEvents(t, mgr, wh.ID),
-			"case %s: stored event rows after a %d response",
-			tc.name, w.Code,
-		)
-	}
-}
-
-// TestReceiverLogsNoSecret proves the rejection path does not write
-// the shared secret, or what the client presented, into the log. A
-// GitLab token arrives as the credential itself, so echoing the
-// header value would put a live secret in the log of every deployment
-// whose sender is briefly misconfigured.
-func TestReceiverLogsNoSecret(t *testing.T) {
-	t.Parallel()
-
-	const presented = "QQPRESENTEDVALUEQQ"
-
-	var (
-		h  *handlers.Handlers
-		db *database.Database
-	)
-
-	app := newTestApp(t, &h, &db)
-	app.RequireStart()
-
-	t.Cleanup(app.RequireStop)
-
-	var buf bytes.Buffer
-
-	h.SetLogForTest(slog.New(slog.NewJSONHandler(&buf, nil)))
-
-	wh := seedWebhook(t, db)
-	ep := seedSignedEntrypoint(
-		t, db, wh.ID,
-		database.SignatureSchemeGitLab, inboundSecret,
-	)
-
-	w := postToEntrypoint(
-		t, h, ep.Path, inboundBody,
-		signature.HeaderGitLab, presented,
-	)
-	require.Equal(t, http.StatusUnauthorized, w.Code)
-
-	// The rejection is recorded at all — a silent 401 leaves an
-	// operator no way to see a sender failing to authenticate.
-	assert.Contains(t, buf.String(), "verification failed")
-	assert.NotContains(t, buf.String(), inboundSecret)
-	assert.NotContains(t, buf.String(), presented)
-}
-
-// TestReceiverDoesNotStoreInboundCredential proves an accepted
-// request leaves no copy of the shared secret in the event store.
-//
-// GitLab's X-Gitlab-Token is the credential itself, not a digest
-// over the request. Stored headers are read back by the UI, copied
-// into every backup and archive, and handed verbatim to every
-// delivery target, so a stored token is the entrypoint's only
-// authentication control disclosed to precisely the parties it
-// exists to exclude.
-//
-// The two cases share one application: every newTestApp seeds an
-// admin user and pays an Argon2id hash at 64 MB, and this package's
-// test budget does not stretch to one per case.
-func TestReceiverDoesNotStoreInboundCredential(t *testing.T) {
-	t.Parallel()
-
-	var (
-		h   *handlers.Handlers
-		db  *database.Database
-		mgr *database.WebhookDBManager
-	)
-
-	app := newTestApp(t, &h, &db, &mgr)
-	app.RequireStart()
-
-	t.Cleanup(app.RequireStop)
-
-	gitlab := seedWebhook(t, db)
-	gitlabEP := seedSignedEntrypoint(
-		t, db, gitlab.ID,
-		database.SignatureSchemeGitLab, inboundSecret,
-	)
-
-	w := postToEntrypoint(
-		t, h, gitlabEP.Path, inboundBody,
-		signature.HeaderGitLab, inboundSecret,
-	)
-	require.Equal(t, http.StatusOK, w.Code)
-
-	stored := storedEventHeaders(t, mgr, gitlab.ID)
-
-	assert.NotContains(
-		t, stored, inboundSecret,
-		"the shared secret must not be persisted",
-	)
-	assert.NotContains(
-		t, stored, signature.HeaderGitLab,
-		"the credential header must not be persisted at all",
-	)
-
-	// Everything else the sender set is still there. A fix that
-	// stored no headers would satisfy the assertions above while
-	// discarding the record the receiver exists to keep.
-	assert.Contains(t, stored, "Content-Type")
-
-	// A GitHub digest is an HMAC over the body, so the key cannot be
-	// recovered from it and it stays: the stripping is scoped to
-	// what actually carries the secret.
-	github := seedWebhook(t, db)
-	githubEP := seedSignedEntrypoint(
-		t, db, github.ID,
-		database.SignatureSchemeGitHub, inboundSecret,
-	)
-
-	w = postToEntrypoint(
-		t, h, githubEP.Path, inboundBody,
-		signature.HeaderGitHub, hubSignature(inboundSecret),
-	)
-	require.Equal(t, http.StatusOK, w.Code)
-
-	stored = storedEventHeaders(t, mgr, github.ID)
-
-	assert.Contains(t, stored, signature.HeaderGitHub)
-	assert.NotContains(t, stored, inboundSecret)
-}

internal/middleware/csrf.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/gorilla/csrf"
 	"sneak.berlin/go/webhooker/internal/logfield"
+	"sneak.berlin/go/webhooker/internal/reqtls"
 )
 
 // CSRFToken retrieves the CSRF token from the request context.
@@ -13,13 +14,6 @@ func CSRFToken(r *http.Request) string {
 	return csrf.Token(r)
 }
 
-// isClientTLS reports whether the client-facing connection uses TLS.
-// It checks for a direct TLS connection (r.TLS) or a TLS-terminating
-// reverse proxy that sets the standard X-Forwarded-Proto header.
-func isClientTLS(r *http.Request) bool {
-	return r.TLS != nil || r.Header.Get("X-Forwarded-Proto") == "https"
-}
-
 // CSRF returns middleware that provides CSRF protection using the
 // gorilla/csrf library. The middleware uses the session authentication
 // key to sign a CSRF cookie and validates a masked token submitted via
@@ -27,9 +21,10 @@ func isClientTLS(r *http.Request) bool {
 // POST/PUT/PATCH/DELETE requests. Requests with an invalid or missing
 // token receive a 403 Forbidden response.
 //
-// The middleware detects the client-facing transport protocol per-request
+// The middleware detects the client-facing transport protocol
-// using r.TLS and the X-Forwarded-Proto header. This allows correct
+// per-request via reqtls.IsTLS, the single TLS predicate the session
-// behavior in all deployment scenarios:
+// cookie also uses. This allows correct behavior in all deployment
+// scenarios:
 //
 //   - Direct HTTPS: strict Referer/Origin checks, Secure cookies.
 //   - Behind a TLS-terminating reverse proxy: strict checks (the
@@ -83,7 +78,7 @@ func (m *Middleware) CSRF() func(http.Handler) http.Handler {
 		httpCSRF := httpProtect(next)
 
 		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-			if isClientTLS(r) {
+			if reqtls.IsTLS(r) {
 				// Client is on TLS (directly or via reverse proxy).
 				// Use Secure cookies and strict Origin/Referer checks.
 				tlsCSRF.ServeHTTP(w, r)

internal/middleware/csrf_test.go

@@ -297,55 +297,176 @@ func TestCSRFToken_NoMiddleware(t *testing.T) {
 }
 
 // --- TLS Detection Tests ---
+//
+// The predicate itself is tested in internal/reqtls. What is tested
+// here is the consequence that actually matters: which of the two
+// gorilla/csrf instances a request is routed to.
+//
+// The two are told apart behaviourally rather than by inspection. On
+// the STRICT (TLS) instance, a state-changing request carrying no
+// Origin header must supply a Referer -- gorilla/csrf rejects it with
+// ErrNoReferer before it ever looks at the token, to defend a
+// TLS site against an HTTP machine-in-the-middle injecting a form. On
+// the RELAXED (plaintext) instance that check is skipped and a valid
+// token is enough. So: valid token, no Origin, no Referer, and the
+// outcome names the instance.
+//
+// Landing on the relaxed instance for a genuinely-HTTPS deployment is
+// the defect: an exact == "https" comparison did exactly that for the
+// uppercase and comma-appended spellings below.
 
-func TestIsClientTLS_DirectTLS(t *testing.T) {
+// csrfTookStrictPath reports whether the CSRF middleware routed a
+// request with the given transport to the strict instance. It also
+// asserts the CSRF cookie's Secure attribute agrees, since the two are
+// set by the same choice and must never disagree.
+func csrfTookStrictPath(
+	t *testing.T,
+	env string,
+	directTLS bool,
+	fwdProto string,
+) bool {
+	t.Helper()
+
+	m, _ := testMiddleware(t, env)
+	csrfMW := m.CSRF()
+
+	newReq := func(method string) *http.Request {
+		r := httptest.NewRequestWithContext(
+			context.Background(), method,
+			"http://example.com/form", nil,
+		)
+
+		if directTLS {
+			r.TLS = &tls.ConnectionState{}
+		}
+
+		if fwdProto != "" {
+			r.Header.Set("X-Forwarded-Proto", fwdProto)
+		}
+
+		return r
+	}
+
+	token, cookies := csrfGetToken(t, csrfMW, newReq(http.MethodGet))
+
+	// Deliberately no Origin and no Referer: that is what makes the
+	// two instances distinguishable.
+	called, code := csrfPostWithToken(
+		t, csrfMW, newReq(http.MethodPost), token, cookies,
+	)
+
+	strict := !called
+
+	if strict {
+		assert.Equal(
+			t, http.StatusForbidden, code,
+			"the strict instance rejects a Referer-less POST",
+		)
+	}
+
+	for _, c := range cookies {
+		if c.Name == csrfCookieName {
+			assert.Equal(
+				t, strict, c.Secure,
+				"the CSRF cookie's Secure attribute and the "+
+					"chosen instance come from one decision "+
+					"and must agree",
+			)
+		}
+	}
+
+	return strict
+}
+
+// TestCSRF_ForwardedProtoSpellingsTakeStrictPath runs the header
+// spellings a real proxy emits through the middleware. The environment
+// 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()
 
-	r := httptest.NewRequestWithContext(
+	cases := []struct {
-		context.Background(), http.MethodGet, "/", nil)
+		name   string
-	r.TLS = &tls.ConnectionState{}
+		header string
+		strict bool
+		why    string
+	}{
+		{
+			name:   "lowercase",
+			header: "https",
+			strict: true,
+			why:    "the ordinary spelling",
+		},
+		{
+			name:   "uppercase",
+			header: "HTTPS",
+			strict: true,
+			why:    "the header value is a case-insensitive token",
+		},
+		{
+			name:   "chain with plaintext inner hop",
+			header: "https, http",
+			strict: true,
+			why: "a chained proxy appends its hop; the leftmost " +
+				"element is the browser's connection",
+		},
+		{
+			name:   "chain of two TLS hops",
+			header: "https,https",
+			strict: true,
+			why:    "appended chain with no space after the comma",
+		},
+		{
+			name:   "trailing space",
+			header: "https ",
+			strict: true,
+			why:    "whitespace is not part of the token",
+		},
+		{
+			name:   "plaintext",
+			header: "http",
+			strict: false,
+			why: "the negative control: the proxy reports a " +
+				"plaintext client connection",
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			assert.Equal(
+				t, tc.strict,
+				csrfTookStrictPath(
+					t, config.EnvironmentDev, false, tc.header,
+				),
+				"X-Forwarded-Proto %q: %s", tc.header, tc.why,
+			)
+		})
+	}
+}
+
+// TestCSRF_DirectTLSTakesStrictPath covers the no-proxy TLS
+// deployment, and TestCSRF_PlaintextTakesRelaxedPath the no-proxy
+// plaintext one -- the local development case that must keep working.
+func TestCSRF_DirectTLSTakesStrictPath(t *testing.T) {
+	t.Parallel()
 
 	assert.True(
-		t, middleware.IsClientTLS(r),
+		t,
-		"should detect direct TLS connection",
+		csrfTookStrictPath(t, config.EnvironmentDev, true, ""),
+		"a request that arrived over TLS takes the strict path",
 	)
 }
 
-func TestIsClientTLS_XForwardedProto(t *testing.T) {
+func TestCSRF_PlaintextTakesRelaxedPath(t *testing.T) {
 	t.Parallel()
 
-	r := httptest.NewRequestWithContext(
-		context.Background(), http.MethodGet, "/", nil)
-	r.Header.Set("X-Forwarded-Proto", "https")
-
-	assert.True(
-		t, middleware.IsClientTLS(r),
-		"should detect TLS via X-Forwarded-Proto",
-	)
-}
-
-func TestIsClientTLS_PlaintextHTTP(t *testing.T) {
-	t.Parallel()
-
-	r := httptest.NewRequestWithContext(
-		context.Background(), http.MethodGet, "/", nil)
-
 	assert.False(
-		t, middleware.IsClientTLS(r),
+		t,
-		"should detect plaintext HTTP",
+		csrfTookStrictPath(t, config.EnvironmentProd, false, ""),
-	)
+		"no TLS and no proxy header is plaintext, in any environment",
-}
-
-func TestIsClientTLS_XForwardedProtoHTTP(t *testing.T) {
-	t.Parallel()
-
-	r := httptest.NewRequestWithContext(
-		context.Background(), http.MethodGet, "/", nil)
-	r.Header.Set("X-Forwarded-Proto", "http")
-
-	assert.False(
-		t, middleware.IsClientTLS(r),
-		"should detect plaintext when X-Forwarded-Proto is http",
 	)
 }
 

internal/middleware/export_test.go

@@ -56,11 +56,6 @@ func ClientKeyForTest(m *Middleware, r *http.Request) string {
 	return m.clientKey(r)
 }
 
-// IsClientTLS exposes isClientTLS for testing.
-func IsClientTLS(r *http.Request) bool {
-	return isClientTLS(r)
-}
-
 // LoginRateLimitConst exposes the loginRateLimit constant: the
 // number of FAILED login attempts one client may make against one
 // submitted username per interval.

internal/reqtls/reqtls.go

@@ -0,0 +1,59 @@
+// Package reqtls answers one question, in one place, for the whole
+// application: did this request reach the service over TLS?
+//
+// It exists because that question used to be answered independently in
+// 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 default environment emitted one
+// Secure cookie and one non-Secure cookie on the same response.
+// Everything kept working, which is exactly why nobody noticed.
+//
+// Any code that needs a scheme or a Secure flag must call IsTLS rather
+// than reading the request itself.
+package reqtls
+
+import (
+	"net/http"
+	"strings"
+)
+
+// forwardedProtoHeader is the de-facto standard header by which a
+// TLS-terminating reverse proxy reports the protocol the CLIENT used.
+const forwardedProtoHeader = "X-Forwarded-Proto"
+
+// IsTLS reports whether the client-facing connection uses TLS: either
+// the request arrived over TLS directly, or a reverse proxy terminated
+// TLS and said so in X-Forwarded-Proto.
+//
+// The header is only as trustworthy as whatever sits in front of the
+// listener. A proxy that overwrites it -- which is what the deployment
+// documentation requires -- makes it authoritative; a listener exposed
+// directly to clients lets any client assert it. That is the same
+// exposure every X-Forwarded-* consumer carries.
+func IsTLS(r *http.Request) bool {
+	return r.TLS != nil || forwardedProto(r) == "https"
+}
+
+// forwardedProto reduces X-Forwarded-Proto to a bare, comparable
+// protocol token, or "" when the header is absent or blank.
+//
+// Two shapes that real infrastructure emits do not survive an exact
+// comparison against "https", and both name a TLS client connection:
+//
+//   - "HTTPS", because the header value is a case-insensitive token and
+//     nothing obliges a proxy to emit it lowercased.
+//   - "https, http", because a proxy chained behind another proxy
+//     APPENDS its own hop instead of replacing the value. As with
+//     X-Forwarded-For, the leftmost element is the one nearest the
+//     client, so it is the element that describes the browser's
+//     connection -- the only hop a cookie's Secure attribute is about.
+//
+// Landing on the plaintext path for either of those spellings is not a
+// cosmetic error: it stops gorilla/csrf enforcing the strict Referer
+// check on a site that genuinely is HTTPS.
+func forwardedProto(r *http.Request) string {
+	first, _, _ := strings.Cut(r.Header.Get(forwardedProtoHeader), ",")
+
+	return strings.ToLower(strings.TrimSpace(first))
+}

internal/reqtls/reqtls_test.go

@@ -0,0 +1,209 @@
+package reqtls_test
+
+import (
+	"context"
+	"crypto/tls"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"sneak.berlin/go/webhooker/internal/reqtls"
+)
+
+// newReq builds a plaintext request with no forwarding headers.
+func newReq(t *testing.T) *http.Request {
+	t.Helper()
+
+	return httptest.NewRequestWithContext(
+		context.Background(), http.MethodGet, "/", nil,
+	)
+}
+
+func TestIsTLS_DirectTLS(t *testing.T) {
+	t.Parallel()
+
+	r := newReq(t)
+	r.TLS = &tls.ConnectionState{}
+
+	assert.True(
+		t, reqtls.IsTLS(r),
+		"a request that arrived over TLS is TLS",
+	)
+}
+
+func TestIsTLS_PlaintextNoHeader(t *testing.T) {
+	t.Parallel()
+
+	assert.False(
+		t, reqtls.IsTLS(newReq(t)),
+		"no TLS connection and no header means plaintext",
+	)
+}
+
+// protoCase is one X-Forwarded-Proto spelling and the answer IsTLS
+// owes it.
+type protoCase struct {
+	name   string
+	header string
+	want   bool
+	why    string
+}
+
+// protoCases enumerates the header values real infrastructure emits.
+func protoCases() []protoCase {
+	return append(protoTLSCases(), protoPlaintextCases()...)
+}
+
+// protoTLSCases are the spellings that name a TLS client connection.
+// Every one but the first is a spelling an exact == "https"
+// comparison used to miss, silently downgrading a genuinely-HTTPS
+// deployment to the plaintext path.
+func protoTLSCases() []protoCase {
+	return []protoCase{
+		{
+			name:   "lowercase",
+			header: "https",
+			want:   true,
+			why:    "the ordinary spelling",
+		},
+		{
+			name:   "uppercase",
+			header: "HTTPS",
+			want:   true,
+			why: "the value is a case-insensitive token; " +
+				"nothing obliges a proxy to lowercase it",
+		},
+		{
+			name:   "mixed case",
+			header: "HttpS",
+			want:   true,
+			why:    "case folding must be total, not just the two extremes",
+		},
+		{
+			name:   "chain with plaintext inner hop",
+			header: "https, http",
+			want:   true,
+			why: "a chained proxy appends its hop; the leftmost " +
+				"element is the client-facing one",
+		},
+		{
+			name:   "chain of two TLS hops",
+			header: "https,https",
+			want:   true,
+			why:    "appended chain with no space after the comma",
+		},
+		{
+			name:   "trailing space",
+			header: "https ",
+			want:   true,
+			why:    "surrounding whitespace is not part of the token",
+		},
+		{
+			name:   "leading space",
+			header: " https",
+			want:   true,
+			why:    "surrounding whitespace is not part of the token",
+		},
+		{
+			name:   "uppercase chain",
+			header: "HTTPS, HTTP",
+			want:   true,
+			why:    "case folding and chain splitting must compose",
+		},
+	}
+}
+
+// protoPlaintextCases are the values that must NOT be read as TLS.
+func protoPlaintextCases() []protoCase {
+	return []protoCase{
+		{
+			name:   "plaintext",
+			header: "http",
+			want:   false,
+			why:    "the negative control: the proxy reports plaintext",
+		},
+		{
+			name:   "plaintext chain with TLS inner hop",
+			header: "http, https",
+			want:   false,
+			why: "the client-facing hop is plaintext even though " +
+				"an inner hop used TLS",
+		},
+		{
+			name:   "empty",
+			header: "",
+			want:   false,
+			why:    "an empty header asserts nothing",
+		},
+		{
+			name:   "whitespace only",
+			header: "   ",
+			want:   false,
+			why:    "a blank header asserts nothing",
+		},
+		{
+			name:   "unrelated token",
+			header: "ftp",
+			want:   false,
+			why:    "only https means TLS",
+		},
+		{
+			name:   "https as a substring",
+			header: "nothttps",
+			want:   false,
+			why:    "matching must be on the whole token, not a substring",
+		},
+	}
+}
+
+func TestIsTLS_ForwardedProtoSpellings(t *testing.T) {
+	t.Parallel()
+
+	for _, tc := range protoCases() {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			r := newReq(t)
+			r.Header.Set("X-Forwarded-Proto", tc.header)
+
+			assert.Equal(
+				t, tc.want, reqtls.IsTLS(r),
+				"X-Forwarded-Proto %q: %s", tc.header, tc.why,
+			)
+		})
+	}
+}
+
+// TestIsTLS_DirectTLSBeatsPlaintextHeader pins the precedence: a
+// connection this process itself terminated with TLS is a fact, and a
+// header claiming otherwise does not override it.
+func TestIsTLS_DirectTLSBeatsPlaintextHeader(t *testing.T) {
+	t.Parallel()
+
+	r := newReq(t)
+	r.TLS = &tls.ConnectionState{}
+	r.Header.Set("X-Forwarded-Proto", "http")
+
+	assert.True(
+		t, reqtls.IsTLS(r),
+		"an actual TLS connection outranks a header claiming plaintext",
+	)
+}
+
+// TestIsTLS_FirstHeaderValueWins covers a proxy that adds a second
+// header line rather than appending to the existing one. net/http
+// keeps them as separate values; the first is the client-facing hop,
+// matching how the comma-separated form is read.
+func TestIsTLS_FirstHeaderValueWins(t *testing.T) {
+	t.Parallel()
+
+	r := newReq(t)
+	r.Header.Add("X-Forwarded-Proto", "https")
+	r.Header.Add("X-Forwarded-Proto", "http")
+
+	assert.True(
+		t, reqtls.IsTLS(r),
+		"the first header line is the client-facing hop",
+	)
+}

internal/server/bind_address_test.go

@@ -0,0 +1,336 @@
+package server_test
+
+import (
+	"context"
+	"net"
+	"net/http"
+	"strconv"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"go.uber.org/fx"
+	"sneak.berlin/go/webhooker/internal/config"
+	"sneak.berlin/go/webhooker/internal/globals"
+	"sneak.berlin/go/webhooker/internal/server"
+)
+
+const (
+	// loopbackV4 is the shipped BIND_ADDRESS default.
+	loopbackV4 = "127.0.0.1"
+
+	// wildcardV4 is the value a container deployment must set,
+	// where a loopback-bound process is unreachable from outside
+	// its network namespace even with a published port.
+	wildcardV4 = "0.0.0.0"
+
+	// unavailableAddr is a TEST-NET-1 address (RFC 5737). It is a
+	// well-formed literal that no host is assigned, so binding it
+	// fails with EADDRNOTAVAIL rather than succeeding somewhere
+	// unexpected.
+	unavailableAddr = "192.0.2.1"
+
+	// listenReadyTimeout bounds the wait for the listener to accept
+	// connections. The bind itself is immediate; this only covers
+	// goroutine scheduling.
+	listenReadyTimeout = 3 * time.Second
+
+	// listenPollInterval is how often the readiness wait retries.
+	listenPollInterval = 10 * time.Millisecond
+
+	// dialTimeout bounds a single connection attempt in these
+	// tests. Everything dialled here is on this host, so a dial
+	// that is not answered immediately is a failure, not slowness.
+	dialTimeout = time.Second
+)
+
+// freePort returns a TCP port that is free on every local address at
+// the moment it returns, by taking one on the wildcard and releasing
+// it. The window between release and re-bind is the standard one
+// every "pick a free port" helper carries.
+func freePort(t *testing.T) int {
+	t.Helper()
+
+	var listenCfg net.ListenConfig
+
+	l, err := listenCfg.Listen(t.Context(), "tcp", "0.0.0.0:0")
+	require.NoError(t, err)
+
+	addr, ok := l.Addr().(*net.TCPAddr)
+	require.True(t, ok, "listener is not TCP")
+	require.NoError(t, l.Close())
+
+	return addr.Port
+}
+
+// otherLocalAddr returns a local IPv4 address that is not
+// loopbackV4, or skips the test when the host has none.
+//
+// The bind-address tests need a second address of this host to stand
+// in for "another interface": what a wildcard bind claims and a
+// loopback bind does not. 127.0.0.2 is that address on Linux, where
+// the whole 127.0.0.0/8 is local; elsewhere an interface address is
+// used instead. Each candidate is proven bindable before it is
+// returned, so a host that offers neither skips rather than fails on
+// something that was never about the code under test.
+func otherLocalAddr(t *testing.T) string {
+	t.Helper()
+
+	candidates := []string{"127.0.0.2"}
+
+	ifaceAddrs, err := net.InterfaceAddrs()
+	require.NoError(t, err)
+
+	for _, a := range ifaceAddrs {
+		ipNet, ok := a.(*net.IPNet)
+		if !ok {
+			continue
+		}
+
+		ip4 := ipNet.IP.To4()
+		if ip4 == nil || ip4.String() == loopbackV4 {
+			continue
+		}
+
+		candidates = append(candidates, ip4.String())
+	}
+
+	var listenCfg net.ListenConfig
+
+	for _, candidate := range candidates {
+		l, listenErr := listenCfg.Listen(
+			t.Context(), "tcp", net.JoinHostPort(candidate, "0"),
+		)
+		if listenErr != nil {
+			continue
+		}
+
+		require.NoError(t, l.Close())
+
+		return candidate
+	}
+
+	t.Skip("host has no second local IPv4 address to bind")
+
+	return ""
+}
+
+// startBoundServer starts the wired app with the given bind address
+// on a free port and returns that port. The app is stopped on
+// cleanup.
+func startBoundServer(t *testing.T, bindAddress string) int {
+	t.Helper()
+
+	port := freePort(t)
+
+	env := newTestEnv(t)
+	env.cfg.BindAddress = bindAddress
+	env.cfg.Port = port
+
+	app := fx.New(
+		fx.NopLogger,
+		fx.Supply(env.log, env.cfg, env.mw, env.hnd),
+		fx.Provide(globals.New, server.New),
+		fx.Invoke(func(*server.Server) {}),
+	)
+
+	startCtx, cancelStart := context.WithTimeout(
+		context.Background(), lifecycleTimeout,
+	)
+	defer cancelStart()
+
+	require.NoError(t, app.Start(startCtx))
+
+	t.Cleanup(func() {
+		stopCtx, cancelStop := context.WithTimeout(
+			context.Background(), lifecycleTimeout,
+		)
+		defer cancelStop()
+
+		require.NoError(t, app.Stop(stopCtx))
+	})
+
+	return port
+}
+
+// dialable reports whether a TCP connection to addr succeeds.
+func dialable(ctx context.Context, addr string) bool {
+	dialer := net.Dialer{Timeout: dialTimeout}
+
+	conn, err := dialer.DialContext(ctx, "tcp", addr)
+	if err != nil {
+		return false
+	}
+
+	_ = conn.Close()
+
+	return true
+}
+
+// requireDialable waits for addr to accept connections, failing the
+// test if it never does.
+func requireDialable(t *testing.T, addr string) {
+	t.Helper()
+
+	deadline := time.Now().Add(listenReadyTimeout)
+	for time.Now().Before(deadline) {
+		if dialable(t.Context(), addr) {
+			return
+		}
+
+		time.Sleep(listenPollInterval)
+	}
+
+	t.Fatalf("nothing accepted connections on %s", addr)
+}
+
+// TestListenAddr pins how BindAddress and Port are rendered into the
+// listen address.
+//
+// The defect this covers was a bare fmt.Sprintf(":%d", port), which
+// binds every interface with no way to say otherwise. The IPv6 rows
+// are here because an unbracketed IPv6 host would produce an address
+// net.Listen rejects, turning a valid configuration into a startup
+// failure.
+func TestListenAddr(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name        string
+		bindAddress string
+		port        int
+		expected    string
+	}{
+		{
+			name:        "loopback default",
+			bindAddress: loopbackV4,
+			port:        8080,
+			expected:    "127.0.0.1:8080",
+		},
+		{
+			name:        "ipv4 wildcard",
+			bindAddress: wildcardV4,
+			port:        8080,
+			expected:    "0.0.0.0:8080",
+		},
+		{
+			name:        "ipv6 wildcard is bracketed",
+			bindAddress: "::",
+			port:        8080,
+			expected:    "[::]:8080",
+		},
+		{
+			name:        "ipv6 literal is bracketed",
+			bindAddress: "2001:db8::5",
+			port:        9001,
+			expected:    "[2001:db8::5]:9001",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			assert.Equal(t, tt.expected, server.ListenAddrForTest(
+				&config.Config{
+					BindAddress: tt.bindAddress,
+					Port:        tt.port,
+				},
+			))
+		})
+	}
+}
+
+// TestBindAddress_LoopbackIsNotOnOtherAddresses proves the fix end to
+// end: with BIND_ADDRESS at its loopback default, the cleartext
+// listener answers on loopback and has not claimed any other address
+// of this host.
+//
+// The second address is proven free by binding it on the same port
+// while the server runs. That is the assertion that fails against the
+// old wildcard bind — a wildcard listener owns the port on every
+// address, so this bind would return EADDRINUSE. Dialling from
+// another machine is what the operator cares about, and this is the
+// in-process form of it: the socket the remote host would connect to
+// does not exist.
+func TestBindAddress_LoopbackIsNotOnOtherAddresses(t *testing.T) {
+	t.Parallel()
+
+	other := otherLocalAddr(t)
+	port := startBoundServer(t, loopbackV4)
+
+	// Positive control: the service really is up and serving.
+	requireDialable(t, net.JoinHostPort(loopbackV4, strconv.Itoa(port)))
+
+	var listenCfg net.ListenConfig
+
+	l, err := listenCfg.Listen(
+		t.Context(), "tcp",
+		net.JoinHostPort(other, strconv.Itoa(port)),
+	)
+	require.NoError(
+		t, err,
+		"port %d on %s is taken while bound to %s: the listener "+
+			"claimed more than its configured address",
+		port, other, loopbackV4,
+	)
+
+	require.NoError(t, l.Close())
+}
+
+// TestBindAddress_WildcardReachesOtherAddresses is the counterpart:
+// the value a container deployment sets does reach the addresses the
+// default withholds. Without this, a loopback-only bind would pass
+// the test above by never listening at all.
+func TestBindAddress_WildcardReachesOtherAddresses(t *testing.T) {
+	t.Parallel()
+
+	other := otherLocalAddr(t)
+	port := startBoundServer(t, wildcardV4)
+
+	requireDialable(t, net.JoinHostPort(other, strconv.Itoa(port)))
+}
+
+// TestBindAddress_ServesRequestsOnConfiguredAddress proves the bound
+// listener serves the application rather than merely accepting TCP,
+// so a bind address that is honoured cannot be mistaken for one that
+// is honoured and broken.
+func TestBindAddress_ServesRequestsOnConfiguredAddress(t *testing.T) {
+	t.Parallel()
+
+	port := startBoundServer(t, loopbackV4)
+	addr := net.JoinHostPort(loopbackV4, strconv.Itoa(port))
+	requireDialable(t, addr)
+
+	req, err := http.NewRequestWithContext(
+		t.Context(), http.MethodGet,
+		"http://"+addr+"/.well-known/healthcheck", nil,
+	)
+	require.NoError(t, err)
+
+	client := &http.Client{Timeout: dialTimeout}
+
+	resp, err := client.Do(req)
+	require.NoError(t, err)
+
+	defer func() { _ = resp.Body.Close() }()
+
+	assert.Equal(t, http.StatusOK, resp.StatusCode)
+}
+
+// TestBindAddress_UnavailableAddressShutsDownTheApp covers the half
+// of the fail-loud rule that configuration parsing cannot reach. A
+// syntactically valid address that is not assigned to this host
+// parses fine and fails at bind time, after fx has already reported
+// RUNNING. It must end the process non-zero rather than leave it
+// alive with nothing listening.
+func TestBindAddress_UnavailableAddressShutsDownTheApp(t *testing.T) {
+	t.Parallel()
+
+	env := newTestEnv(t)
+	env.cfg.BindAddress = unavailableAddr
+	env.cfg.Port = freePort(t)
+
+	requireListenFailureExit(t, env)
+}

internal/server/early_shutdown_test.go

@@ -0,0 +1,91 @@
+package server_test
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"go.uber.org/fx"
+	"sneak.berlin/go/webhooker/internal/globals"
+	"sneak.berlin/go/webhooker/internal/server"
+)
+
+// earlyStopIterations is how many start/stop cycles the race test
+// runs. The window it aims at is the gap between the OnStart hook
+// returning and the serving goroutine reaching its first field
+// access, which is microseconds wide. The race detector reports an
+// unsynchronised pair whenever it observes one, but it has to observe
+// one, so a single cycle can miss purely on scheduling. Repetition
+// makes the observation reliable; the collaborators are built once,
+// so the cycles themselves are cheap.
+const earlyStopIterations = 25
+
+// TestEarlyShutdown_NoPanicAndNoRace stops the application
+// immediately after starting it, before the serving goroutine has
+// necessarily run at all.
+//
+// Two defects live in that window. The OnStart hook returns as soon
+// as it has spawned the serving goroutine, so fx runs the stop
+// sequence against a Server whose serving goroutine may not have
+// executed a single line. cleanShutdown called Shutdown on an
+// httpServer that goroutine was supposed to assign, which was a nil
+// dereference on an early SIGTERM; and it read httpServer and
+// sentryEnabled with nothing ordering those reads against the
+// goroutine's writes, which is a data race that only surfaces once
+// something both starts and stops the server. Nothing did before this
+// test: the listen-failure test never binds, and the router tests
+// bypass the lifecycle entirely.
+//
+// httpServer is now built in New, on the constructing goroutine, so
+// it is written before any hook exists and can never be nil.
+// sentryEnabled is atomic. This test is what catches either one
+// coming back — under -race, which is how the suite runs.
+func TestEarlyShutdown_NoPanicAndNoRace(t *testing.T) {
+	t.Parallel()
+
+	// Built once: the collaborators are not what is under test, and
+	// standing up a database per iteration would make repetition too
+	// expensive to be worth having.
+	env := newTestEnv(t)
+	env.cfg.BindAddress = loopbackV4
+
+	for range earlyStopIterations {
+		requireStartStopIsClean(t, env)
+	}
+}
+
+// requireStartStopIsClean runs one start/stop cycle with no wait in
+// between, failing the test if either half errors.
+//
+// Each cycle gets a fresh fx app, so the Server under test is
+// constructed anew every time — that construction is where the
+// httpServer write now happens, and reusing one Server would test it
+// only once.
+func requireStartStopIsClean(t *testing.T, env *testEnv) {
+	t.Helper()
+
+	env.cfg.Port = freePort(t)
+
+	app := fx.New(
+		fx.NopLogger,
+		fx.Supply(env.log, env.cfg, env.mw, env.hnd),
+		fx.Provide(globals.New, server.New),
+		fx.Invoke(func(*server.Server) {}),
+	)
+
+	startCtx, cancelStart := context.WithTimeout(
+		context.Background(), lifecycleTimeout,
+	)
+	defer cancelStart()
+
+	require.NoError(t, app.Start(startCtx))
+
+	// No sleep and no readiness wait: stopping while the serving
+	// goroutine is still in flight is the whole point.
+	stopCtx, cancelStop := context.WithTimeout(
+		context.Background(), lifecycleTimeout,
+	)
+	defer cancelStop()
+
+	require.NoError(t, app.Stop(stopCtx))
+}

internal/server/export_test.go

@@ -55,6 +55,16 @@ func NewRouterForTest(
 	return s.router
 }
 
+// ListenAddrForTest exposes the address the HTTP listener binds for
+// a given Config, so the rendering of host and port — IPv6
+// bracketing above all — can be pinned without standing up a
+// listener.
+func ListenAddrForTest(cfg *config.Config) string {
+	s := &Server{params: ServerParams{Config: cfg}}
+
+	return s.listenAddr()
+}
+
 // ProbePattern is the route NewRouterWithProbeForTest adds to the
 // production route tree.
 const ProbePattern = "/probe"
@@ -80,12 +90,12 @@ func NewRouterWithProbeForTest(
 	probe http.HandlerFunc,
 ) http.Handler {
 	s := &Server{
-		log:           log,
+		log:    log,
-		mw:            mw,
+		mw:     mw,
-		h:             h,
+		h:      h,
-		params:        ServerParams{Config: cfg},
+		params: ServerParams{Config: cfg},
-		sentryEnabled: sentryEnabled,
 	}
+	s.sentryEnabled.Store(sentryEnabled)
 	s.SetupRoutes()
 	s.router.Handle(ProbePattern, probe)
 

internal/server/http.go

@@ -2,8 +2,9 @@ package server
 
 import (
 	"errors"
-	"fmt"
+	"net"
 	"net/http"
+	"strconv"
 	"time"
 )
 
@@ -24,21 +25,47 @@ const (
 	httpMaxHeaderBytes = 1 << 20
 )
 
-func (s *Server) serveUntilShutdown() {
+// listenAddr renders the address the HTTP listener binds.
-	listenAddr := fmt.Sprintf(":%d", s.params.Config.Port)
+//
-	s.httpServer = &http.Server{
+// The host half is always present: an empty host would be the
-		Addr:           listenAddr,
+// wildcard, and the whole point of BIND_ADDRESS is that binding every
+// interface is a choice the operator makes rather than one the
+// process makes for them. Config guarantees a literal, so
+// JoinHostPort's bracketing is enough to make IPv6 well formed.
+func (s *Server) listenAddr() string {
+	return net.JoinHostPort(
+		s.params.Config.BindAddress,
+		strconv.Itoa(s.params.Config.Port),
+	)
+}
+
+// newHTTPServer builds the HTTP server for this Server's
+// configuration.
+//
+// It is called from New, on the constructing goroutine, rather than
+// from the serving goroutine that used to assign s.httpServer
+// directly. Two goroutines reach that field — the serving goroutine
+// and the fx stop hook, which calls Shutdown on it — with nothing
+// ordering them. Constructing it during New puts the write before
+// every hook fx will later run, which both removes the race and rules
+// out the nil dereference a stop that arrived before the serving
+// goroutine had run would have caused.
+func (s *Server) newHTTPServer() *http.Server {
+	return &http.Server{
+		Addr:           s.listenAddr(),
 		ReadTimeout:    httpReadTimeout,
 		WriteTimeout:   httpWriteTimeout,
 		MaxHeaderBytes: httpMaxHeaderBytes,
 		Handler:        s,
 	}
+}
 
+func (s *Server) serveUntilShutdown() {
 	// add routes
 	// this does any necessary setup in each handler
 	s.SetupRoutes()
 
-	s.log.Info("http begin listen", "listenaddr", listenAddr)
+	s.log.Info("http begin listen", "listenaddr", s.httpServer.Addr)
 
 	err := s.httpServer.ListenAndServe()
 	if err != nil && !errors.Is(err, http.ErrServerClosed) {

internal/server/listen_failure_test.go

@@ -36,9 +36,9 @@ const lifecycleTimeout = 15 * time.Second
 //
 // The port is occupied by a listener this test holds open, on a
 // kernel-chosen port, so the failure is the real EADDRINUSE the
-// operator hits when a second instance starts. Loopback is enough to
+// operator hits when a second instance starts. The server is pointed
-// collide with the server's wildcard bind: a listening socket on a
+// at the same loopback address, so the collision is a direct one on
-// specific address blocks the wildcard from claiming the same port.
+// the exact address it asks the kernel for.
 func TestListenFailure_ShutsDownTheApp(t *testing.T) {
 	t.Parallel()
 
@@ -55,11 +55,27 @@ func TestListenFailure_ShutsDownTheApp(t *testing.T) {
 	require.True(t, ok, "listener is not TCP")
 
 	// The collaborators come from the wired graph rather than stubs,
-	// so the Server under test is the one that ships. Only the port
+	// so the Server under test is the one that ships. Only the
-	// is test-specific.
+	// listen address is test-specific.
 	env := newTestEnv(t)
+	env.cfg.BindAddress = loopbackV4
 	env.cfg.Port = addr.Port
 
+	requireListenFailureExit(t, env)
+}
+
+// requireListenFailureExit starts the wired app over env and asserts
+// that it gives up on its own with the listen-failure status, then
+// completes its stop sequence.
+//
+// Two different listen failures share it — a port already in use and
+// an address that is not on this host — because what has to hold for
+// both is the same: the failure is discovered after fx has already
+// reported RUNNING, so the only thing that can turn it into a visible
+// exit is the shutdown path under test.
+func requireListenFailureExit(t *testing.T, env *testEnv) {
+	t.Helper()
+
 	app := fx.New(
 		fx.NopLogger,
 		fx.Supply(env.log, env.cfg, env.mw, env.hnd),

internal/server/routes.go

@@ -81,7 +81,7 @@ func (s *Server) setupGlobalMiddleware() {
 	// Sentry error reporting (if SENTRY_DSN is set). Repanic is
 	// true so panics still bubble up to the Recoverer middleware
 	// registered immediately above.
-	if s.sentryEnabled {
+	if s.sentryEnabled.Load() {
 		sentryHandler := sentryhttp.New(sentryhttp.Options{
 			Repanic: true,
 		})
@@ -237,10 +237,6 @@ func (s *Server) setupSourceRoutes() {
 			"/entrypoints/{entrypointID}/toggle",
 			s.h.HandleEntrypointToggle(),
 		)
-		r.Post(
-			"/entrypoints/{entrypointID}/secret",
-			s.h.HandleEntrypointSecret(),
-		)
 		r.Post("/targets", s.h.HandleTargetCreate())
 		// The edit form is the one page that renders a target's
 		// destination URL and header values in full; see

internal/server/sentry.go

@@ -147,10 +147,13 @@ func sentryRoutePattern(hint *sentry.EventHint) string {
 //
 // The scheme is load-bearing and is kept: the SDK derives it from
 // r.TLS != nil || r.Header.Get("X-Forwarded-Proto") == "https"
-// (interfaces.go:180), byte for byte the predicate
+// (interfaces.go:180), which is the reason dropping X-Forwarded-Proto
-// internal/middleware/csrf.go uses, so it is the CSRF TLS decision and
+// from the header allowlist costs nothing. That predicate is the SDK's
-// the reason dropping X-Forwarded-Proto from the header allowlist
+// own and is stricter than reqtls.IsTLS, which this service now uses
-// costs nothing. The host is parsed.Host of the SDK's
+// everywhere it decides transport: the SDK reports "http" for the
+// "HTTPS" and "https, http" spellings reqtls accepts. Only a reported
+// scheme is affected, no decision is, so it is left to the SDK rather
+// than reimplemented. The host is parsed.Host of the SDK's
 // scheme://r.Host/path, so it is whatever the client's Host header
 // carried: this service validates no hostname. It is kept because that
 // same header is on the allowlist, so scrubbing it here would withhold

internal/server/server.go

@@ -9,6 +9,7 @@ import (
 	"net/http"
 	"os"
 	"os/signal"
+	"sync/atomic"
 	"syscall"
 	"time"
 
@@ -88,8 +89,15 @@ type ServerParams struct {
 // Server is the main HTTP server that wires up routes and manages
 // graceful shutdown.
 type Server struct {
-	startupTime   time.Time
+	startupTime time.Time
-	sentryEnabled bool
+
+	// sentryEnabled is written by the serving goroutine, in
+	// enableSentry, and read by the fx stop hook in cleanShutdown.
+	// Nothing orders those two: the OnStart hook returns as soon as
+	// the goroutine is spawned, so a stop can be running while
+	// enableSentry is still deciding. It is atomic to supply the
+	// edge the goroutines do not.
+	sentryEnabled atomic.Bool
 	log           *slog.Logger
 	cancelFunc    context.CancelFunc
 	httpServer    *http.Server
@@ -107,6 +115,7 @@ func New(lc fx.Lifecycle, params ServerParams) (*Server, error) {
 	s.mw = params.Middleware
 	s.h = params.Handlers
 	s.log = params.Logger.Get()
+	s.httpServer = s.newHTTPServer()
 
 	lc.Append(fx.Hook{
 		OnStart: func(_ context.Context) error {
@@ -142,7 +151,7 @@ func (s *Server) MaintenanceMode() bool {
 }
 
 func (s *Server) enableSentry() {
-	s.sentryEnabled = false
+	s.sentryEnabled.Store(false)
 
 	if s.params.Config.SentryDSN == "" {
 		return
@@ -163,7 +172,7 @@ func (s *Server) enableSentry() {
 	}
 
 	s.log.Info("sentry error reporting activated")
-	s.sentryEnabled = true
+	s.sentryEnabled.Store(true)
 }
 
 // serve installs the signal watcher, starts the listener and blocks
@@ -242,7 +251,7 @@ func (s *Server) cleanShutdown(ctx context.Context) {
 
 	s.cleanupForExit()
 
-	if s.sentryEnabled {
+	if s.sentryEnabled.Load() {
 		s.flushSentry(ctx)
 	}
 }

internal/session/export_test.go

@@ -5,6 +5,6 @@ import "github.com/gorilla/sessions"
 // NewStore exposes the production cookie-store constructor so tests
 // exercise the store the application actually runs with, rather than a
 // lookalike assembled in the test.
-func NewStore(key []byte, secure bool) *sessions.CookieStore {
+func NewStore(key []byte) *sessions.CookieStore {
-	return newStore(key, secure)
+	return newStore(key)
 }

internal/session/session.go

@@ -17,6 +17,7 @@ import (
 	"sneak.berlin/go/webhooker/internal/config"
 	"sneak.berlin/go/webhooker/internal/database"
 	"sneak.berlin/go/webhooker/internal/logger"
+	"sneak.berlin/go/webhooker/internal/reqtls"
 )
 
 const (
@@ -84,10 +85,9 @@ type Params struct {
 
 // Session manages encrypted session storage.
 type Session struct {
-	store  *sessions.CookieStore
+	store *sessions.CookieStore
-	key    []byte // raw 32-byte auth key, also used for CSRF cookie signing
+	key   []byte // raw 32-byte auth key, also used for CSRF cookie signing
-	log    *slog.Logger
+	log   *slog.Logger
-	config *config.Config
 
 	// idleTimeout is the sliding inactivity window. A session that
 	// sees no authenticated request within this window expires,
@@ -104,6 +104,10 @@ type Session struct {
 // cookie. MaxAge is deliberately left at its zero value: for a store
 // it is set through CookieStore.MaxAge (see newStore), and for a
 // single session it is copied from the store's options.
+//
+// Secure is a parameter rather than a constant because it is the one
+// attribute here that is not a policy -- it is a fact about the
+// connection carrying this particular response. See applyTransport.
 func cookieOptions(secure bool) *sessions.Options {
 	return &sessions.Options{
 		Path:     "/",
@@ -121,14 +125,52 @@ func cookieOptions(secure bool) *sessions.Options {
 // Options never touches Codecs -- so a store configured that way still
 // decodes a 30-day-old cookie, leaving the cookie attribute and the
 // codec disagreeing about the same policy. store.MaxAge sets both.
-func newStore(key []byte, secure bool) *sessions.CookieStore {
+//
+// The store's Secure is fixed at true, and is only a template: every
+// write path overwrites it for the request in hand (applyTransport).
+// It is true rather than false so that a write path added later which
+// forgets to call applyTransport fails loudly -- the browser drops the
+// cookie over plaintext HTTP and the developer sees it immediately --
+// instead of silently shipping the authentication credential without
+// Secure, which is the exact failure this store already had once.
+func newStore(key []byte) *sessions.CookieStore {
 	store := sessions.NewCookieStore(key)
-	store.Options = cookieOptions(secure)
+	store.Options = cookieOptions(true)
 	store.MaxAge(secondsPerDay * sessionMaxAgeDays)
 
 	return store
 }
 
+// applyTransport sets the session cookie's Secure attribute from the
+// transport of the request being answered.
+//
+// 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" 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.
+//
+// gorilla/sessions makes this cheap and local: CookieStore.New gives
+// every session its own copy of the store's Options, and
+// CookieStore.Save renders the cookie from that copy rather than from
+// the store. So the flag is set on the one session being saved,
+// without a second store and without reaching across concurrent
+// requests.
+//
+// The flag tracks the transport in BOTH directions rather than being
+// latched on once seen. Secure on a plaintext response is worse than
+// useless: the browser discards such a cookie without any error, so a
+// latched flag would make a plain-HTTP local run impossible to log
+// into. It is also why every write path must call this, including the
+// deletion cookies in Destroy and Regenerate -- a Secure deletion
+// cookie sent over plaintext is dropped too, leaving the session the
+// caller believed it had just revoked.
+func applyTransport(r *http.Request, sess *sessions.Session) {
+	sess.Options.Secure = reqtls.IsTLS(r)
+}
+
 // New creates a new session manager. The cookie store is
 // initialized during the fx OnStart phase after the database is
 // connected, using a session key that is auto-generated and stored
@@ -139,7 +181,6 @@ func New(
 ) (*Session, error) {
 	s := &Session{
 		log:         params.Logger.Get(),
-		config:      params.Config,
 		idleTimeout: params.Config.SessionIdleTimeout,
 		now:         time.Now,
 	}
@@ -172,7 +213,7 @@ func New(
 			}
 
 			s.key = keyBytes
-			s.store = newStore(keyBytes, !params.Config.IsDev())
+			s.store = newStore(keyBytes)
 			s.log.Info("session manager initialized")
 
 			return nil
@@ -196,12 +237,16 @@ func (s *Session) GetKey() []byte {
 	return s.key
 }
 
-// Save saves the session.
+// Save saves the session. Every session-cookie write in the
+// application goes through here or through Regenerate, which is what
+// makes applyTransport a complete answer rather than a best effort.
 func (s *Session) Save(
 	r *http.Request,
 	w http.ResponseWriter,
 	sess *sessions.Session,
 ) error {
+	applyTransport(r, sess)
+
 	return sess.Save(r, w)
 }
 
@@ -340,6 +385,7 @@ func (s *Session) Regenerate(
 	// Destroy the old session
 	oldSess.Options.MaxAge = -1
 	s.ClearUser(oldSess)
+	applyTransport(r, oldSess)
 
 	err := oldSess.Save(r, w)
 	if err != nil {
@@ -368,7 +414,7 @@ func (s *Session) Regenerate(
 	// Apply the standard session options (the destroyed old
 	// session had MaxAge = -1, which store.New might inherit
 	// from the cookie).
-	newSess.Options = cookieOptions(!s.config.IsDev())
+	newSess.Options = cookieOptions(reqtls.IsTLS(r))
 	newSess.Options.MaxAge = secondsPerDay * sessionMaxAgeDays
 
 	return newSess, nil

internal/session/session_test.go

@@ -2,6 +2,7 @@ package session_test
 
 import (
 	"context"
+	"crypto/tls"
 	"log/slog"
 	"net/http"
 	"net/http/httptest"
@@ -73,7 +74,7 @@ func testSessionWithClock(
 	t.Helper()
 
 	key := testKey()
-	store := session.NewStore(key, false)
+	store := session.NewStore(key)
 
 	cfg := &config.Config{
 		Environment:        config.EnvironmentDev,
@@ -880,3 +881,264 @@ func TestDestroy_ThenSave_DeletesCookie(t *testing.T) {
 		"destroyed session cookie should have negative MaxAge",
 	)
 }
+
+// --- Secure Attribute / Transport Tests ---
+
+// transportCase describes one client-facing transport and the Secure
+// attribute the session cookie must carry for it.
+type transportCase struct {
+	name   string
+	tls    bool
+	header string
+	want   bool
+	why    string
+}
+
+// transportCases enumerates the transports the session cookie has to
+// get right. Every https spelling here is one a real proxy emits.
+func transportCases() []transportCase {
+	return []transportCase{
+		{
+			name: "direct TLS",
+			tls:  true,
+			want: true,
+			why:  "this process terminated TLS itself",
+		},
+		{
+			name:   "proxy reports https",
+			header: "https",
+			want:   true,
+			why:    "the ordinary reverse-proxy deployment",
+		},
+		{
+			name:   "proxy reports HTTPS",
+			header: "HTTPS",
+			want:   true,
+			why:    "the header value is a case-insensitive token",
+		},
+		{
+			name:   "appended chain https, http",
+			header: "https, http",
+			want:   true,
+			why:    "the leftmost hop is the browser's connection",
+		},
+		{
+			name:   "appended chain https,https",
+			header: "https,https",
+			want:   true,
+			why:    "two TLS hops, no space after the comma",
+		},
+		{
+			name:   "trailing space",
+			header: "https ",
+			want:   true,
+			why:    "whitespace is not part of the token",
+		},
+		{
+			name:   "proxy reports http",
+			header: "http",
+			want:   false,
+			why: "the negative control: Secure over plaintext is " +
+				"dropped by the browser without a word",
+		},
+		{
+			name: "plaintext, no proxy",
+			want: false,
+			why:  "a plain local run must stay loggable-in",
+		},
+	}
+}
+
+// transportRequest builds a request carrying the case's transport.
+func (tc transportCase) request(t *testing.T) *http.Request {
+	t.Helper()
+
+	r := httptest.NewRequestWithContext(
+		context.Background(), http.MethodGet,
+		"http://example.com/", nil,
+	)
+
+	if tc.tls {
+		r.TLS = &tls.ConnectionState{}
+	}
+
+	if tc.header != "" {
+		r.Header.Set("X-Forwarded-Proto", tc.header)
+	}
+
+	return r
+}
+
+// sessionCookieFrom returns the session cookie from a response, or
+// fails the test if there is none.
+func sessionCookieFrom(
+	t *testing.T,
+	w *httptest.ResponseRecorder,
+) *http.Cookie {
+	t.Helper()
+
+	for _, c := range w.Result().Cookies() {
+		if c.Name == session.SessionName {
+			return c
+		}
+	}
+
+	require.FailNow(t, "no session cookie in response")
+
+	return nil
+}
+
+// TestSave_SecureFollowsRequestTransport is the regression test for
+// the defect this replaces: Secure was fixed at startup from the
+// 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.
+//
+// testSession builds its config with EnvironmentDev precisely so that
+// the https cases below fail against the old startup-fixed behaviour.
+func TestSave_SecureFollowsRequestTransport(t *testing.T) {
+	t.Parallel()
+
+	for _, tc := range transportCases() {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			s := testSession(t)
+			r := tc.request(t)
+			w := httptest.NewRecorder()
+
+			sess, err := s.Get(r)
+			require.NoError(t, err)
+
+			s.SetUser(sess, "user-1", "alice")
+			require.NoError(t, s.Save(r, w, sess))
+
+			assert.Equal(
+				t, tc.want, sessionCookieFrom(t, w).Secure,
+				"session cookie Secure for %q: %s",
+				tc.name, tc.why,
+			)
+		})
+	}
+}
+
+// TestSave_SecureTracksTransportBothWays pins that the flag is not
+// latched. One store serves every request, so a Secure cookie set for
+// a proxied request must not leak into a later plaintext response --
+// the browser would silently discard that one, and a local run would
+// become impossible to log into.
+func TestSave_SecureTracksTransportBothWays(t *testing.T) {
+	t.Parallel()
+
+	s := testSession(t)
+
+	secureReq := httptest.NewRequestWithContext(
+		context.Background(), http.MethodGet,
+		"http://example.com/", nil,
+	)
+	secureReq.Header.Set("X-Forwarded-Proto", "https")
+
+	secureW := httptest.NewRecorder()
+
+	secureSess, err := s.Get(secureReq)
+	require.NoError(t, err)
+	require.NoError(t, s.Save(secureReq, secureW, secureSess))
+	require.True(
+		t, sessionCookieFrom(t, secureW).Secure,
+		"proxied request should produce a Secure cookie",
+	)
+
+	plainReq := httptest.NewRequestWithContext(
+		context.Background(), http.MethodGet,
+		"http://example.com/", nil,
+	)
+	plainW := httptest.NewRecorder()
+
+	plainSess, err := s.Get(plainReq)
+	require.NoError(t, err)
+	require.NoError(t, s.Save(plainReq, plainW, plainSess))
+
+	assert.False(
+		t, sessionCookieFrom(t, plainW).Secure,
+		"a later plaintext request must not inherit Secure from "+
+			"the earlier proxied one",
+	)
+}
+
+// TestDestroy_DeletionCookieFollowsTransport covers the trap in the
+// deletion path. The store's template Secure is true, so a logout over
+// plaintext that failed to track the transport would emit a Secure
+// deletion cookie -- which the browser drops, leaving the session the
+// user just tried to end still sitting in the jar.
+func TestDestroy_DeletionCookieFollowsTransport(t *testing.T) {
+	t.Parallel()
+
+	s := testSession(t)
+
+	r := httptest.NewRequestWithContext(
+		context.Background(), http.MethodGet,
+		"http://example.com/", nil,
+	)
+	w := httptest.NewRecorder()
+
+	sess, err := s.Get(r)
+	require.NoError(t, err)
+
+	s.Destroy(sess)
+	require.NoError(t, s.Save(r, w, sess))
+
+	cookie := sessionCookieFrom(t, w)
+
+	require.Negative(
+		t, cookie.MaxAge,
+		"Destroy then Save should emit a deletion cookie",
+	)
+	assert.False(
+		t, cookie.Secure,
+		"a deletion cookie sent over plaintext must not be Secure, "+
+			"or the browser discards it and the session survives",
+	)
+}
+
+// TestRegenerate_BothCookiesFollowTransport covers the login path.
+// Regenerate writes two cookies -- a deletion for the pre-login
+// session and the new authenticated one -- and both have to match the
+// transport or one of them is silently dropped.
+func TestRegenerate_BothCookiesFollowTransport(t *testing.T) {
+	t.Parallel()
+
+	for _, tc := range transportCases() {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			s := testSession(t)
+			r := tc.request(t)
+			w := httptest.NewRecorder()
+
+			oldSess, err := s.Get(r)
+			require.NoError(t, err)
+
+			newSess, err := s.Regenerate(r, w, oldSess)
+			require.NoError(t, err)
+
+			s.SetUser(newSess, "user-1", "alice")
+			require.NoError(t, s.Save(r, w, newSess))
+
+			cookies := w.Result().Cookies()
+			require.Len(
+				t, cookies, 2,
+				"Regenerate then Save writes a deletion cookie "+
+					"and a replacement",
+			)
+
+			for _, c := range cookies {
+				assert.Equal(
+					t, tc.want, c.Secure,
+					"cookie %d Secure for %q: %s",
+					c.MaxAge, tc.name, tc.why,
+				)
+			}
+		})
+	}
+}

internal/session/testing.go

@@ -32,7 +32,6 @@ func NewForTest(
 	return &Session{
 		store:       store,
 		key:         key,
-		config:      cfg,
 		log:         log,
 		idleTimeout: cfg.SessionIdleTimeout,
 		now:         now,

internal/signature/signature.go

@@ -1,283 +0,0 @@
-// Package signature verifies that an inbound webhook request really
-// came from the sender an entrypoint was configured for.
-//
-// Verification is optional and per entrypoint. An entrypoint with no
-// scheme configured is not verified at all, which is what every
-// entrypoint was before this package existed. An entrypoint whose
-// configuration is present but incoherent is failed closed, never
-// treated as unverified: the whole point of the feature is that
-// turning it on cannot silently turn itself back off.
-package signature
-
-import (
-	"crypto/hmac"
-	"crypto/sha256"
-	"encoding/hex"
-	"errors"
-	"fmt"
-	"net/http"
-	"strings"
-
-	"sneak.berlin/go/webhooker/internal/database"
-)
-
-// Header names each supported scheme reads its signature from.
-const (
-	// HeaderGitHub is GitHub's HMAC-SHA256 signature header. GitHub
-	// also sends the older SHA-1 X-Hub-Signature; it is not accepted.
-	HeaderGitHub = "X-Hub-Signature-256"
-
-	// HeaderGitLab is GitLab's plain shared-token header.
-	HeaderGitLab = "X-Gitlab-Token"
-)
-
-// githubPrefix is the algorithm label GitHub puts in front of the hex
-// digest. It is required, not optional: accepting a bare digest too
-// would mean accepting a spelling no supported sender produces.
-const githubPrefix = "sha256="
-
-// ErrConfig marks a failure caused by the entrypoint's stored
-// configuration rather than by the request. A caller must fail these
-// closed — refuse the request — because the alternative is an
-// entrypoint the operator believes is verified silently accepting
-// anything.
-var ErrConfig = errors.New("entrypoint signature configuration invalid")
-
-// ErrUnauthorized marks a request that failed verification. A caller
-// answers these 401.
-var ErrUnauthorized = errors.New("inbound signature verification failed")
-
-// Configuration failures. None of these carry any part of the secret.
-var (
-	errSchemeUnknown = fmt.Errorf(
-		"%w: unsupported scheme", ErrConfig,
-	)
-	errSecretMissing = fmt.Errorf(
-		"%w: scheme set with no secret", ErrConfig,
-	)
-	errSchemeMissing = fmt.Errorf(
-		"%w: secret set with no scheme", ErrConfig,
-	)
-)
-
-// Request failures. These are logged, so none of them carries the
-// value the client sent: under the GitLab scheme that value is a
-// guess at the token, and under either scheme a misconfigured sender
-// could be presenting the real one.
-var (
-	errHeaderMissing = fmt.Errorf(
-		"%w: signature header absent", ErrUnauthorized,
-	)
-	errHeaderMalformed = fmt.Errorf(
-		"%w: signature header malformed", ErrUnauthorized,
-	)
-	errSignatureMismatch = fmt.Errorf(
-		"%w: signature does not match", ErrUnauthorized,
-	)
-)
-
-// SchemeInfo describes one supported scheme for the UI.
-type SchemeInfo struct {
-	Scheme database.SignatureScheme
-	Label  string
-	Header string
-
-	// HeaderIsDigest reports that Header carries a value derived from
-	// the request rather than the shared secret itself, and so may be
-	// kept when the request is stored and forwarded.
-	//
-	// The polarity is deliberate: false — the zero value — means the
-	// header is the credential and must be stripped. A scheme added
-	// later is therefore stripped unless whoever adds it positively
-	// declares the header safe to keep.
-	HeaderIsDigest bool
-}
-
-// Schemes returns the supported schemes in the order the UI offers
-// them. It returns a fresh slice per call so no caller can edit the
-// set out from under another.
-func Schemes() []SchemeInfo {
-	return []SchemeInfo{
-		{
-			Scheme: database.SignatureSchemeGitHub,
-			Label:  "GitHub",
-			Header: HeaderGitHub,
-			// An HMAC over the body, not the key. Keeping it lets an
-			// operator see what the sender sent.
-			HeaderIsDigest: true,
-		},
-		{
-			Scheme: database.SignatureSchemeGitLab,
-			Label:  "GitLab",
-			Header: HeaderGitLab,
-			// X-Gitlab-Token is the shared secret in plaintext.
-			HeaderIsDigest: false,
-		},
-	}
-}
-
-// Info returns the description of a supported scheme. It reports
-// false for the empty scheme and for anything unrecognised, which is
-// what a row hand-edited in the database could hold.
-func Info(scheme database.SignatureScheme) (SchemeInfo, bool) {
-	for _, s := range Schemes() {
-		if s.Scheme == scheme {
-			return s, true
-		}
-	}
-
-	return SchemeInfo{}, false
-}
-
-// Supported reports whether a scheme may be stored on an entrypoint.
-// The empty scheme is supported: it means no verification.
-func Supported(scheme database.SignatureScheme) bool {
-	if scheme == database.SignatureSchemeNone {
-		return true
-	}
-
-	_, ok := Info(scheme)
-
-	return ok
-}
-
-// SanitizeHeaders returns a copy of an accepted request's headers
-// with the entrypoint's credential removed.
-//
-// Under a scheme whose header is the shared secret itself — GitLab's
-// X-Gitlab-Token — every downstream use of the inbound headers is a
-// disclosure of the credential: they are persisted verbatim in the
-// per-webhook event store and forwarded to every delivery target, so
-// a target operator or anyone who reads the event database could
-// forge signed requests to the very entrypoint the secret protects.
-// Stripping happens here, once, above the first write, rather than
-// at each egress, so a new consumer of Event.Headers cannot reopen
-// the leak by forgetting to filter.
-//
-// header is never modified; the caller's request keeps its headers
-// intact for anything that still needs the original.
-//
-// An entrypoint with no scheme, or one whose stored scheme this
-// build does not know, is returned unchanged: there is no configured
-// credential to remove, and the unknown case is refused by Verify
-// before a request reaches storage.
-func SanitizeHeaders(
-	entrypoint *database.Entrypoint,
-	header http.Header,
-) http.Header {
-	clone := header.Clone()
-	if clone == nil {
-		return header
-	}
-
-	info, ok := Info(entrypoint.SignatureScheme)
-	if !ok || info.HeaderIsDigest {
-		return clone
-	}
-
-	clone.Del(info.Header)
-
-	return clone
-}
-
-// Verify checks an inbound request against an entrypoint's
-// configuration and returns nil when the request may be accepted.
-//
-// body must be the raw bytes exactly as received, before any parsing
-// or normalisation: the sender computed its digest over those bytes,
-// so anything that re-encodes them produces a different digest and a
-// spurious rejection. The caller is also responsible for bounding
-// that read; this package hashes what it is handed.
-//
-// Every non-nil error is either ErrConfig or ErrUnauthorized, so a
-// caller can tell "the server is misconfigured" from "the client did
-// not authenticate" with errors.Is.
-func Verify(
-	entrypoint *database.Entrypoint,
-	header http.Header,
-	body []byte,
-) error {
-	scheme := entrypoint.SignatureScheme
-	secret := entrypoint.SignatureSecret
-
-	if scheme == database.SignatureSchemeNone {
-		// A secret with no scheme names no header and no algorithm,
-		// so there is nothing to check it with. Accepting the request
-		// would make a half-applied configuration indistinguishable
-		// from no configuration at all.
-		if secret != "" {
-			return errSchemeMissing
-		}
-
-		return nil
-	}
-
-	if secret == "" {
-		return errSecretMissing
-	}
-
-	switch scheme {
-	case database.SignatureSchemeGitHub:
-		return verifyGitHub(secret, header.Get(HeaderGitHub), body)
-	case database.SignatureSchemeGitLab:
-		return verifyGitLab(secret, header.Get(HeaderGitLab))
-	case database.SignatureSchemeNone:
-		// Handled above; restated so the switch stays exhaustive and
-		// adding a scheme has to be decided here.
-		return nil
-	default:
-		return errSchemeUnknown
-	}
-}
-
-// verifyGitHub checks a GitHub-style X-Hub-Signature-256: the string
-// "sha256=" followed by the hex HMAC-SHA256 of the raw body under the
-// shared secret.
-func verifyGitHub(secret, provided string, body []byte) error {
-	if provided == "" {
-		return errHeaderMissing
-	}
-
-	encoded, ok := strings.CutPrefix(provided, githubPrefix)
-	if !ok {
-		return errHeaderMalformed
-	}
-
-	got, err := hex.DecodeString(encoded)
-	if err != nil {
-		return errHeaderMalformed
-	}
-
-	mac := hmac.New(sha256.New, []byte(secret))
-
-	// hash.Hash.Write is documented never to return an error.
-	_, _ = mac.Write(body)
-
-	// hmac.Equal, never ==: string comparison stops at the first
-	// differing byte, which tells a client how much of a forged
-	// digest it got right and turns forgery into a per-byte search.
-	if !hmac.Equal(mac.Sum(nil), got) {
-		return errSignatureMismatch
-	}
-
-	return nil
-}
-
-// verifyGitLab checks a GitLab-style X-Gitlab-Token, which is the
-// shared secret itself rather than a digest over the body.
-//
-// The comparison is constant time in the same way as the HMAC one.
-// hmac.Equal returns early for unequal lengths, so the length of the
-// token is not hidden; its contents are, and length alone does not
-// let a client search for the value.
-func verifyGitLab(secret, provided string) error {
-	if provided == "" {
-		return errHeaderMissing
-	}
-
-	if !hmac.Equal([]byte(provided), []byte(secret)) {
-		return errSignatureMismatch
-	}
-
-	return nil
-}

internal/signature/signature_test.go

@@ -1,340 +0,0 @@
-package signature_test
-
-import (
-	"crypto/hmac"
-	"crypto/sha256"
-	"encoding/hex"
-	"net/http"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"sneak.berlin/go/webhooker/internal/database"
-	"sneak.berlin/go/webhooker/internal/signature"
-)
-
-const (
-	// testSharedKey is the shared secret under test. It is not named
-	// "secret": gosec reads a credential-shaped name bound to a
-	// high-entropy literal as a leaked credential, which is the right
-	// rule and the wrong finding here.
-	testSharedKey = "s3kr1t-shared-value"
-	testBody      = `{"action":"opened","number":1}`
-)
-
-// githubSignature returns the X-Hub-Signature-256 value GitHub would
-// send for testBody signed with secret.
-func githubSignature(secret string) string {
-	mac := hmac.New(sha256.New, []byte(secret))
-	_, _ = mac.Write([]byte(testBody))
-
-	return "sha256=" + hex.EncodeToString(mac.Sum(nil))
-}
-
-// headerWith builds a request header carrying one value.
-func headerWith(name, value string) http.Header {
-	h := http.Header{}
-	if name != "" {
-		h.Set(name, value)
-	}
-
-	return h
-}
-
-// entrypoint builds an entrypoint with a signature configuration.
-func entrypoint(
-	scheme database.SignatureScheme, secret string,
-) *database.Entrypoint {
-	return &database.Entrypoint{
-		SignatureScheme: scheme,
-		SignatureSecret: secret,
-	}
-}
-
-// TestVerifyUnconfiguredAcceptsAnything pins the pass-through case:
-// an entrypoint with no scheme is the entrypoint every deployment
-// already has, and it must keep accepting requests that carry no
-// signature at all.
-func TestVerifyUnconfiguredAcceptsAnything(t *testing.T) {
-	t.Parallel()
-
-	ep := entrypoint(database.SignatureSchemeNone, "")
-
-	require.NoError(
-		t, signature.Verify(ep, http.Header{}, []byte(testBody)),
-	)
-	require.NoError(
-		t,
-		signature.Verify(
-			ep,
-			headerWith(signature.HeaderGitHub, "sha256=deadbeef"),
-			[]byte(testBody),
-		),
-	)
-}
-
-// githubCase is one inbound request against a GitHub-scheme
-// entrypoint.
-type githubCase struct {
-	name   string
-	header string
-	value  string
-	body   string
-	want   error
-}
-
-// githubCases enumerates the shapes a GitHub signature can arrive in.
-func githubCases() []githubCase {
-	valid := githubSignature(testSharedKey)
-
-	return []githubCase{
-		{
-			name:   "valid",
-			header: signature.HeaderGitHub,
-			value:  valid,
-			body:   testBody,
-			want:   nil,
-		},
-		{
-			name:   "absent header",
-			header: "",
-			body:   testBody,
-			want:   signature.ErrUnauthorized,
-		},
-		{
-			name:   "wrong secret",
-			header: signature.HeaderGitHub,
-			value:  githubSignature("not-the-shared-value"),
-			body:   testBody,
-			want:   signature.ErrUnauthorized,
-		},
-		{
-			// The digest is valid for a different body: the check
-			// has to be over the bytes actually received.
-			name:   "body altered in flight",
-			header: signature.HeaderGitHub,
-			value:  valid,
-			body:   testBody + " ",
-			want:   signature.ErrUnauthorized,
-		},
-		{
-			name:   "missing algorithm prefix",
-			header: signature.HeaderGitHub,
-			value:  valid[len("sha256="):],
-			body:   testBody,
-			want:   signature.ErrUnauthorized,
-		},
-		{
-			name:   "not hex",
-			header: signature.HeaderGitHub,
-			value:  "sha256=zzzz",
-			body:   testBody,
-			want:   signature.ErrUnauthorized,
-		},
-		{
-			name:   "empty digest",
-			header: signature.HeaderGitHub,
-			value:  "sha256=",
-			body:   testBody,
-			want:   signature.ErrUnauthorized,
-		},
-		{
-			// GitLab's header does not authenticate a GitHub
-			// entrypoint, even holding the right secret.
-			name:   "wrong header for the scheme",
-			header: signature.HeaderGitLab,
-			value:  testSharedKey,
-			body:   testBody,
-			want:   signature.ErrUnauthorized,
-		},
-	}
-}
-
-func TestVerifyGitHub(t *testing.T) {
-	t.Parallel()
-
-	for _, tc := range githubCases() {
-		t.Run(tc.name, func(t *testing.T) {
-			t.Parallel()
-
-			err := signature.Verify(
-				entrypoint(
-					database.SignatureSchemeGitHub, testSharedKey,
-				),
-				headerWith(tc.header, tc.value),
-				[]byte(tc.body),
-			)
-
-			if tc.want == nil {
-				require.NoError(t, err)
-
-				return
-			}
-
-			require.ErrorIs(t, err, tc.want)
-		})
-	}
-}
-
-func TestVerifyGitLab(t *testing.T) {
-	t.Parallel()
-
-	cases := []struct {
-		name   string
-		header string
-		value  string
-		want   error
-	}{
-		{
-			name:   "valid",
-			header: signature.HeaderGitLab,
-			value:  testSharedKey,
-			want:   nil,
-		},
-		{
-			name:   "absent header",
-			header: "",
-			want:   signature.ErrUnauthorized,
-		},
-		{
-			name:   "wrong token",
-			header: signature.HeaderGitLab,
-			value:  "not-the-shared-value",
-			want:   signature.ErrUnauthorized,
-		},
-		{
-			name:   "token prefix only",
-			header: signature.HeaderGitLab,
-			value:  testSharedKey[:5],
-			want:   signature.ErrUnauthorized,
-		},
-		{
-			name:   "wrong header for the scheme",
-			header: signature.HeaderGitHub,
-			value:  githubSignature(testSharedKey),
-			want:   signature.ErrUnauthorized,
-		},
-	}
-
-	for _, tc := range cases {
-		t.Run(tc.name, func(t *testing.T) {
-			t.Parallel()
-
-			err := signature.Verify(
-				entrypoint(
-					database.SignatureSchemeGitLab, testSharedKey,
-				),
-				headerWith(tc.header, tc.value),
-				[]byte(testBody),
-			)
-
-			if tc.want == nil {
-				require.NoError(t, err)
-
-				return
-			}
-
-			require.ErrorIs(t, err, tc.want)
-		})
-	}
-}
-
-// TestVerifyBrokenConfigurationFailsClosed covers the rows a caller
-// must refuse rather than wave through. Each is a state an operator
-// could only reach outside the UI, and each one would otherwise be
-// indistinguishable from "verification is off".
-func TestVerifyBrokenConfigurationFailsClosed(t *testing.T) {
-	t.Parallel()
-
-	cases := []struct {
-		name   string
-		scheme database.SignatureScheme
-		secret string
-	}{
-		{
-			name:   "unknown scheme",
-			scheme: database.SignatureScheme("stripe"),
-			secret: testSharedKey,
-		},
-		{
-			name:   "scheme without secret",
-			scheme: database.SignatureSchemeGitHub,
-			secret: "",
-		},
-		{
-			name:   "secret without scheme",
-			scheme: database.SignatureSchemeNone,
-			secret: testSharedKey,
-		},
-	}
-
-	for _, tc := range cases {
-		t.Run(tc.name, func(t *testing.T) {
-			t.Parallel()
-
-			err := signature.Verify(
-				entrypoint(tc.scheme, tc.secret),
-				headerWith(
-					signature.HeaderGitHub,
-					githubSignature(testSharedKey),
-				),
-				[]byte(testBody),
-			)
-
-			require.ErrorIs(t, err, signature.ErrConfig)
-			assert.NotErrorIs(t, err, signature.ErrUnauthorized)
-		})
-	}
-}
-
-// TestErrorsCarryNoSecret proves the strings that reach the log hold
-// no part of the shared secret or of what the client presented.
-func TestErrorsCarryNoSecret(t *testing.T) {
-	t.Parallel()
-
-	const presented = "QQPRESENTEDTOKENQQ"
-
-	for _, scheme := range []database.SignatureScheme{
-		database.SignatureSchemeGitHub,
-		database.SignatureSchemeGitLab,
-	} {
-		for _, header := range []string{
-			signature.HeaderGitHub, signature.HeaderGitLab,
-		} {
-			err := signature.Verify(
-				entrypoint(scheme, testSharedKey),
-				headerWith(header, presented),
-				[]byte(testBody),
-			)
-			require.Error(t, err)
-			assert.NotContains(t, err.Error(), testSharedKey)
-			assert.NotContains(t, err.Error(), presented)
-		}
-	}
-}
-
-func TestSchemeMetadata(t *testing.T) {
-	t.Parallel()
-
-	assert.True(t, signature.Supported(database.SignatureSchemeNone))
-	assert.True(t, signature.Supported(database.SignatureSchemeGitHub))
-	assert.True(t, signature.Supported(database.SignatureSchemeGitLab))
-	assert.False(
-		t, signature.Supported(database.SignatureScheme("stripe")),
-	)
-
-	// The empty scheme describes no sender, so it has no info even
-	// though it is a storable value.
-	_, ok := signature.Info(database.SignatureSchemeNone)
-	assert.False(t, ok)
-
-	info, ok := signature.Info(database.SignatureSchemeGitHub)
-	require.True(t, ok)
-	assert.Equal(t, "GitHub", info.Label)
-	assert.Equal(t, signature.HeaderGitHub, info.Header)
-
-	info, ok = signature.Info(database.SignatureSchemeGitLab)
-	require.True(t, ok)
-	assert.Equal(t, "GitLab", info.Label)
-	assert.Equal(t, signature.HeaderGitLab, info.Header)
-}

internal/versionscript/doc.go

@@ -0,0 +1,12 @@
+// Package versionscript holds the tests for script/version and for the
+// build files that consume it. It carries no runtime code: the version
+// string is produced by a shell script at build time and reaches the
+// binary through a linker flag, so nothing in the Go build graph can
+// assert it, but the behaviour still has to be verified by the test
+// suite.
+//
+// The files under test are outside the Go build graph, so `go test`'s
+// result cache serves a stale PASS when only script/version, the
+// Makefile or the Dockerfile changed: run the container build, or
+// GOFLAGS=-count=1, to trust a result here after editing them.
+package versionscript

internal/versionscript/version_script_test.go

@@ -0,0 +1,345 @@
+package versionscript_test
+
+import (
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+const (
+	repoRoot       = "../.."
+	scriptPath     = "../../script/version"
+	makefilePath   = "../../Makefile"
+	dockerfilePath = "../../Dockerfile"
+
+	// unknown is what a tree with no git metadata and no $VERSION must
+	// report: a source tarball has no way to know its version, and the
+	// one thing it must not do is name a tag it may not be at.
+	unknown = "unknown"
+
+	// scriptMode keeps the copied script runnable; dirMode and fileMode
+	// are the ordinary permissions for the throwaway tree around it.
+	scriptMode = 0o755
+	dirMode    = 0o750
+	fileMode   = 0o600
+)
+
+// checkout is a throwaway working tree carrying a copy of the script
+// under test at the same path it lives at in this repository, since the
+// script resolves the checkout root from its own location.
+type checkout struct {
+	dir  string
+	head string
+}
+
+func TestVersion_CleanTagReportsExactlyTheTag(t *testing.T) {
+	t.Parallel()
+
+	c := newCheckout(t)
+	c.git(t, "tag", "v1.2.3")
+
+	require.Equal(t, "v1.2.3", c.version(t))
+}
+
+func TestVersion_CommitAfterTagCarriesDistanceAndSHA(t *testing.T) {
+	t.Parallel()
+
+	c := newCheckout(t)
+	c.git(t, "tag", "v1.2.3")
+	head := c.commit(t, "after the tag")
+
+	got := c.version(t)
+
+	require.NotEqual(t, "v1.2.3", got,
+		"a commit past the tag must not claim to be the tag")
+	require.True(t, strings.HasPrefix(got, "v1.2.3-1-g"),
+		"want describe form v1.2.3-1-g<sha>, got %q", got)
+	require.True(t, strings.HasPrefix(head, strings.TrimPrefix(
+		got, "v1.2.3-1-g")),
+		"%q must carry the abbreviated head SHA of %q", got, head)
+}
+
+func TestVersion_UntaggedHistoryReportsShortSHA(t *testing.T) {
+	t.Parallel()
+
+	c := newCheckout(t)
+
+	got := c.version(t)
+
+	require.NotEmpty(t, got)
+	require.NotEqual(t, unknown, got)
+	require.True(t, strings.HasPrefix(c.head, got),
+		"%q must be an abbreviation of head %q", got, c.head)
+}
+
+func TestVersion_UncommittedChangesAreMarkedDirty(t *testing.T) {
+	t.Parallel()
+
+	c := newCheckout(t)
+	c.git(t, "tag", "v1.2.3")
+
+	require.NoError(t, os.WriteFile(
+		filepath.Join(c.dir, "tracked.txt"), []byte("edited\n"), fileMode,
+	))
+
+	require.Equal(t, "v1.2.3-dirty", c.version(t))
+}
+
+// A source tarball, or any build context without .git, still has to
+// build. It reports "unknown" rather than failing or naming a tag.
+func TestVersion_NoGitMetadataReportsUnknown(t *testing.T) {
+	t.Parallel()
+
+	c := newCheckout(t)
+	require.NoError(t, os.RemoveAll(filepath.Join(c.dir, ".git")))
+
+	require.Equal(t, unknown, c.version(t))
+}
+
+// An unpacked tarball can sit inside an unrelated working copy. The
+// enclosing repository's version is not this tree's version.
+func TestVersion_EnclosingRepositoryIsNotUsed(t *testing.T) {
+	t.Parallel()
+
+	outer := newCheckout(t)
+	outer.git(t, "tag", "v9.9.9")
+
+	inner := filepath.Join(outer.dir, "unpacked")
+	require.NoError(t, os.MkdirAll(filepath.Join(inner, "script"), dirMode))
+	copyScript(t, inner)
+
+	require.Equal(t, unknown, runScript(t, inner, nil))
+}
+
+// The Docker build has no git metadata, so the version arrives as an
+// environment override. It wins over anything derivable.
+func TestVersion_EnvironmentOverrideWins(t *testing.T) {
+	t.Parallel()
+
+	c := newCheckout(t)
+	c.git(t, "tag", "v1.2.3")
+
+	require.Equal(t, "v4.5.6",
+		runScript(t, c.dir, []string{"VERSION=v4.5.6"}))
+}
+
+// An empty VERSION is treated as unset rather than stamping an empty
+// string: the Dockerfile's build arg has a non-empty default, but a
+// caller exporting VERSION= must not produce a binary reporting "".
+func TestVersion_EmptyOverrideFallsBackToGit(t *testing.T) {
+	t.Parallel()
+
+	c := newCheckout(t)
+	c.git(t, "tag", "v1.2.3")
+
+	require.Equal(t, "v1.2.3", runScript(t, c.dir, []string{"VERSION="}))
+}
+
+// Two builds of the same commit must produce a byte-identical binary,
+// which they cannot if the stamped value moves between invocations.
+func TestVersion_IsStableAcrossInvocations(t *testing.T) {
+	t.Parallel()
+
+	c := newCheckout(t)
+	c.git(t, "tag", "v1.2.3")
+
+	first := c.version(t)
+	second := c.version(t)
+
+	require.Equal(t, first, second)
+}
+
+// The build target is the only place composing linker flags. If a
+// future edit drops either half, the binary silently reports "dev"
+// again (the defect this package exists for) or fails to link on
+// Alpine.
+func TestMakefile_BuildComposesVersionAndExtraFlags(t *testing.T) {
+	t.Parallel()
+
+	makefile := read(t, makefilePath)
+
+	require.Contains(t, makefile, "-X main.version=$(VERSION)")
+	require.Contains(t, makefile, "$(GO_LDFLAGS)")
+	require.Contains(t, makefile, "VERSION ?= $(shell script/version)")
+}
+
+// A caller can define VERSION as the empty string -- `make build
+// VERSION=`, or a `--build-arg VERSION=` reaching the Dockerfile's `make
+// build VERSION="$VERSION"`. script/version's own guard does not cover
+// that: the value never passes through the script. Stamping "" would
+// leave the binary reporting no version and the footer on "dev", which
+// is the defect this package exists for.
+func TestMakefile_EmptyOverrideResolvesLikeAnUnsetOne(t *testing.T) {
+	t.Parallel()
+
+	// A plain assignment would be ignored here: a command-line
+	// definition outranks it, and that is the case being corrected.
+	require.Contains(t, read(t, makefilePath), "override VERSION :=")
+
+	requireMake(t)
+
+	derived := makeVersion(t)
+	require.NotEmpty(t, derived)
+
+	require.Equal(t, derived, makeVersion(t, "VERSION="),
+		"an empty VERSION must resolve the way an unset one does")
+	require.Equal(t, "v9.9.9", makeVersion(t, "VERSION=v9.9.9"),
+		"the empty guard must not clobber a real override")
+}
+
+// makeVersion runs this repository's `version` target, which prints the
+// value `make build` would stamp, with the given command-line
+// definitions.
+func makeVersion(t *testing.T, defs ...string) string {
+	t.Helper()
+
+	//nolint:gosec // fixed argv, arguments are test constants
+	cmd := exec.CommandContext(t.Context(), "make",
+		append([]string{"--no-print-directory", "version"}, defs...)...)
+	cmd.Dir = repoRoot
+
+	// Only the command-line definitions may decide the outcome: an
+	// inherited VERSION would change what an unset one resolves to, and
+	// an inherited MAKEFLAGS carries the parent's jobserver.
+	cmd.Env = append(os.Environ(), "VERSION=", "MAKEFLAGS=", "MAKELEVEL=")
+
+	out, err := cmd.CombinedOutput()
+	require.NoError(t, err, string(out))
+
+	return strings.TrimSpace(string(out))
+}
+
+func requireMake(t *testing.T) {
+	t.Helper()
+
+	_, err := exec.LookPath("make")
+	if err != nil {
+		t.Skipf("make is not installed: %v", err)
+	}
+}
+
+// Every compile in the image goes through the build target, so the
+// static relink cannot replace the flags that carry the stamp.
+func TestDockerfile_BuildsThroughTheMakeTarget(t *testing.T) {
+	t.Parallel()
+
+	dockerfile := read(t, dockerfilePath)
+
+	require.NotContains(t, dockerfile, "go build",
+		"a raw go build bypasses the Makefile's -X flag")
+	require.Contains(t, dockerfile, "ARG VERSION=")
+	require.Contains(t, dockerfile,
+		`make build VERSION="$VERSION" GO_LDFLAGS='-extldflags "-static"'`)
+}
+
+func read(t *testing.T, path string) string {
+	t.Helper()
+
+	//nolint:gosec // repo-local build file under test, fixed path
+	b, err := os.ReadFile(path)
+	require.NoError(t, err)
+
+	return string(b)
+}
+
+// version runs the script in this checkout with no overrides.
+func (c checkout) version(t *testing.T) string {
+	t.Helper()
+
+	return runScript(t, c.dir, nil)
+}
+
+func runScript(t *testing.T, dir string, env []string) string {
+	t.Helper()
+
+	//nolint:gosec // fixed argv, repo-local script under test
+	cmd := exec.CommandContext(t.Context(), "sh",
+		filepath.Join(dir, "script", "version"))
+	cmd.Dir = dir
+
+	cmd.Env = append(os.Environ(), env...)
+
+	out, err := cmd.CombinedOutput()
+	require.NoError(t, err, string(out))
+
+	return strings.TrimSpace(string(out))
+}
+
+func (c checkout) git(t *testing.T, args ...string) string {
+	t.Helper()
+
+	//nolint:gosec // fixed argv, arguments are test constants
+	cmd := exec.CommandContext(t.Context(), "git", args...)
+	cmd.Dir = c.dir
+
+	out, err := cmd.CombinedOutput()
+	require.NoError(t, err, string(out))
+
+	return strings.TrimSpace(string(out))
+}
+
+func (c checkout) commit(t *testing.T, message string) string {
+	t.Helper()
+
+	c.git(t,
+		"-c", "user.email=ci@example.invalid",
+		"-c", "user.name=ci",
+		"-c", "commit.gpgsign=false",
+		"commit", "-q", "--allow-empty", "-m", message,
+	)
+
+	return c.git(t, "rev-parse", "HEAD")
+}
+
+// newCheckout builds a one-commit repository with a tracked file, so a
+// later edit to that file makes the tree dirty, and with a copy of the
+// script at the path it occupies in this repository.
+func newCheckout(t *testing.T) checkout {
+	t.Helper()
+
+	requireGit(t)
+
+	dir := t.TempDir()
+	c := checkout{dir: dir}
+
+	require.NoError(t, os.MkdirAll(filepath.Join(dir, "script"), dirMode))
+	copyScript(t, dir)
+
+	require.NoError(t, os.WriteFile(
+		filepath.Join(dir, "tracked.txt"), []byte("original\n"), fileMode,
+	))
+
+	c.git(t, "init", "-q", "-b", "main")
+	c.git(t, "add", "tracked.txt")
+	c.head = c.commit(t, "initial")
+
+	return c
+}
+
+func copyScript(t *testing.T, dir string) {
+	t.Helper()
+
+	body, err := os.ReadFile(scriptPath)
+	require.NoError(t, err)
+
+	//nolint:gosec // the copy has to stay executable to be run
+	err = os.WriteFile(
+		filepath.Join(dir, "script", "version"), body, scriptMode,
+	)
+	require.NoError(t, err)
+}
+
+func requireGit(t *testing.T) {
+	t.Helper()
+
+	for _, tool := range []string{"sh", "git"} {
+		_, err := exec.LookPath(tool)
+		if err != nil {
+			t.Skipf("%s is not installed: %v", tool, err)
+		}
+	}
+}

script/docker

@@ -1,7 +1,10 @@
 #!/bin/sh
 # script/docker: build the Docker image tagged with the project name.
-# Identical in all repos; the tag comes from script/projectname.
+# The tag comes from script/projectname.
-# Generic: needs no adaptation.
+#
+# .dockerignore excludes .git/, so the builder stage cannot derive the
+# version itself. It is resolved here, where the checkout is, and passed
+# in as a build arg; without it the image would stamp itself "unknown".
 set -eu
 
 SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"
@@ -9,7 +12,9 @@ ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)"
 
 main() {
     cd "$ROOT"
-    docker build -t "$("$SCRIPT_DIR/projectname")" .
+    docker build \
+        --build-arg VERSION="$("$SCRIPT_DIR/version")" \
+        -t "$("$SCRIPT_DIR/projectname")" .
 }
 
 main "$@"

script/version

@@ -0,0 +1,65 @@
+#!/bin/sh
+# script/version: output the version string the binary is stamped with.
+# Our own extension to scripts-to-rule-them-all. The Makefile's build
+# target and script/docker both take the value from here, so a `make
+# build` binary and a `make docker` image built from the same checkout
+# report the same thing.
+#
+# Order of precedence:
+#
+#   1. $VERSION, if set and non-empty. This is how the value reaches a
+#      build that cannot derive it: .dockerignore excludes .git/, so the
+#      builder stage has no git metadata and the Dockerfile takes the
+#      value as a build arg instead.
+#   2. `git describe --tags --always --dirty` against this checkout. At
+#      a clean tagged commit that is exactly the tag; otherwise it
+#      carries the short SHA, the commit distance when a tag is
+#      reachable, and a -dirty suffix for uncommitted changes.
+#   3. "unknown", for a tree with no git metadata and no $VERSION -- a
+#      source tarball, or `docker build .` with no --build-arg. That
+#      case must not fail the build and must not name a tag the tree may
+#      not be at, so it names nothing.
+#
+# The git step insists the enclosing repository is this checkout, not
+# merely some repository above it: an unpacked tarball sitting inside an
+# unrelated working copy would otherwise be stamped with that copy's
+# version.
+#
+# Nothing here may vary between two builds of the same commit: the
+# release gate asserts the binary is byte-identical across builds. That
+# rules out a build timestamp, a hostname, and a builder identity.
+set -eu
+
+ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
+
+# in_this_checkout succeeds when git can read metadata for a repository
+# whose work tree root is $ROOT.
+in_this_checkout() {
+    command -v git >/dev/null 2>&1 || return 1
+
+    top="$(git rev-parse --show-toplevel 2>/dev/null)" || return 1
+    [ -n "$top" ] || return 1
+
+    top="$(cd "$top" 2>/dev/null && pwd -P)" || return 1
+    [ "$top" = "$ROOT" ]
+}
+
+main() {
+    if [ -n "${VERSION:-}" ]; then
+        echo "$VERSION"
+
+        return 0
+    fi
+
+    cd "$ROOT"
+
+    if in_this_checkout; then
+        # --always keeps an untagged history from failing the build: it
+        # falls back to the bare short SHA.
+        git describe --tags --always --dirty 2>/dev/null && return 0
+    fi
+
+    echo "unknown"
+}
+
+main "$@"

templates/source_detail.html

@@ -48,7 +48,7 @@
 
             <div class="divide-y divide-gray-100">
                 {{range .Entrypoints}}
-                <div class="p-4" x-data="{ showSecret: false }">
+                <div class="p-4">
                     <div class="flex items-center justify-between mb-1">
                         <span class="text-sm font-medium text-gray-900">{{if .Description}}{{.Description}}{{else}}Entrypoint{{end}}</span>
                         <div class="flex items-center gap-2">
@@ -75,38 +75,8 @@
                              script the URL above stays selectable. -->
                         <button type="button" hidden data-copy-target="entrypoint-url-{{.ID}}" class="text-xs text-gray-500 hover:text-primary-600">Copy</button>
                     </div>
-                    <div class="flex items-center gap-2 mt-2">
+                    <!-- The URL above is the entrypoint's credential:
-                        <span class="text-xs text-gray-500">
+                         anyone holding it can submit events. -->
-                            Signature: {{.SchemeLabel}}{{if .SchemeHeader}} ({{.SchemeHeader}}){{end}}
-                        </span>
-                        <button type="button" @click="showSecret = !showSecret" class="text-xs text-gray-500 hover:text-primary-600">
-                            {{if .Configured}}Rotate{{else}}Configure{{end}}
-                        </button>
-                    </div>
-                    <!-- The stored secret is never sent to the browser: the
-                         form takes a new one every time, so setting and
-                         rotating are the same submission. -->
-                    <div x-show="showSecret" x-cloak class="mt-2">
-                        <form method="POST" action="/source/{{$.Webhook.ID}}/entrypoints/{{.ID}}/secret" class="flex gap-2">
-                            <input type="hidden" name="csrf_token" value="{{$.CSRFToken}}">
-                            <select name="signature_scheme" class="input text-sm w-28">
-                                <!-- Selection follows the stored scheme, not
-                                     whether the pair is complete: a row with a
-                                     scheme and no secret would otherwise mark
-                                     both this option and its own selected. -->
-                                <option value="" {{if not .Scheme}}selected{{end}}>None</option>
-                                {{$current := .Scheme}}
-                                {{range $.SignatureSchemes}}
-                                <option value="{{.Scheme}}" {{if eq .Scheme $current}}selected{{end}}>{{.Label}}</option>
-                                {{end}}
-                            </select>
-                            <input type="password" name="secret" autocomplete="new-password" placeholder="Shared secret" class="input text-sm flex-1">
-                            <button type="submit" class="btn-primary text-sm">Save</button>
-                        </form>
-                        <p class="text-xs text-gray-500 mt-1">
-                            Enter the same secret you configured at the sender. Selecting None removes verification.
-                        </p>
-                    </div>
                 </div>
                 {{else}}
                 <div class="p-4 text-sm text-gray-500">No entrypoints configured.</div>