Author SHA1 Message Date
sneak 1647b43aa6 deploy (#365)
check / check (push) Waiting to run
Reviewed-on: #365
2026-09-29 13:06:27 +02:00
sneakandclawbot f7151f0168 deployment to prod (#357)
check / check (push) Successful in 10s
Co-authored-by: clawbot <35+clawbot@noreply.example.org>
Reviewed-on: #357
2026-09-29 12:01:56 +02:00
sneak 3cc05a36eb deployment (#355)
check / check (push) Successful in 4m11s
Reviewed-on: #355
2026-09-29 11:54:06 +02:00
sneak 7316f0a7e2 Merge branch 'main' into prod
check / check (push) Successful in 7s
2026-09-29 11:49:18 +02:00
clawbotandsneak 5f84d891cf Deploy: main into prod (#343)
check / check (push) Successful in 3m32s
Brings `prod`, which upaas deploys, up to `main` at `9cf9cdd`, the merge of #321. `prod` was cut from `main` at `251cb3d` (1.0.0b1).

What it deploys is everything listed in #321. For running it:

- With `WEBHOOKER_ENVIRONMENT` unset, the instance runs as `prod` and sends no `Access-Control-Allow-Origin: *`.
- Each event database gains its new indexes the first time it is opened after the upgrade.
- `webhooker_delivery_retries_total` no longer counts a circuit breaker holding back a delivery that is already `retrying`.

Not in this PR yet: #340, in which the container sets its own data directory owner and mode before start. It is in progress on `next`. Once it reaches `main`, this PR carries it, because the PR follows `main`.

Model: opus-5-5
Co-authored-by: Jeffrey Paul <1+sneak@noreply.example.org>
Reviewed-on: #343
2026-09-29 02:48:03 -07:00
sneak 9cf9cdd8eb 1.0.0 milestone: next into main (#321)
check / check (push) Successful in 8s
Reviewed-on: #321
2026-09-29 11:04:58 +02:00
17 changed files with 539 additions and 248 deletions
+9 -2
View File
@@ -88,7 +88,9 @@ RUN CGO_ENABLED=1 make build VERSION="$VERSION" GO_LDFLAGS='-extldflags "-static
# alpine:3.21, 2026-03-17
FROM alpine:3.21@sha256:c3f8e73fdb79deaebaa2037150150191b9dcbfba68b4a46d70103204c53f4709
RUN apk --no-cache add ca-certificates
# su-exec 0.2-r3 (Alpine 3.21), 2026-09-29: the entrypoint runs the app
# as webhooker with it.
RUN apk --no-cache add ca-certificates su-exec=0.2-r3
# Create non-root user
RUN addgroup -g 1000 -S webhooker && \
@@ -99,13 +101,17 @@ WORKDIR /app
# Copy binary from builder
COPY --from=builder /build/bin/webhooker /app/webhooker
# Not under /app, which belongs to webhooker: this script runs as root.
COPY deploy/docker-entrypoint.sh /usr/local/bin/docker-entrypoint.sh
# Create data directory for all SQLite databases (main app DB +
# per-webhook event DBs). DATA_DIR defaults to /var/lib/webhooker.
RUN mkdir -p /var/lib/webhooker
RUN chown -R webhooker:webhooker /app /var/lib/webhooker
USER webhooker
# No USER: the entrypoint starts as root to make the data directory
# webhooker's, then runs the app as webhooker.
EXPOSE 8080
@@ -124,4 +130,5 @@ ENV BIND_ADDRESS=0.0.0.0
HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \
CMD wget --no-verbose --tries=1 --spider http://localhost:8080/.well-known/healthcheck || exit 1
ENTRYPOINT ["/usr/local/bin/docker-entrypoint.sh"]
CMD ["/app/webhooker"]
+114 -155
View File
@@ -147,7 +147,7 @@ TTY detection, and security headers are always applied.
| `RETENTION_SWEEP_INTERVAL` | How often the retention reaper and archive sweeper run (Go duration, must be positive) | `1h` |
| `SESSION_IDLE_TIMEOUT` | Idle session timeout (Go duration) | `24h` |
| `RECEIVER_RATE_LIMIT` | Receiver requests/minute per IP per entrypoint (10x that per IP across the route) | `120` |
| `TRUSTED_PROXIES` | CIDRs whose forwarded headers are trusted. A set value replaces the default. Under the default, any client with a private address, whether it connects directly or through the proxy, can choose its own rate-limit key by sending its own `X-Forwarded-For`; if any clients have private addresses, set it to the proxy's address alone. See [Trusted proxies](#trusted-proxies) | `10.0.0.0/8,172.16.0.0/12,192.168.0.0/16` (RFC 1918) |
| `TRUSTED_PROXIES` | CIDRs whose forwarded headers are trusted (unset: all clients behind a proxy share one rate-limit bucket; a correct login password is never throttled either way) | `""` (none) |
| `ALLOWED_EGRESS_CIDRS` | CIDRs that delivery targets may reach despite the SSRF blocklist. Read [Allowing egress to your own network](#allowing-egress-to-your-own-network) before setting it | `""` (none) |
#### Allowing egress to your own network
@@ -379,48 +379,41 @@ unlocked.
`TRUSTED_PROXIES` is a comma-separated list of CIDR blocks (a bare
address such as `192.168.1.7` is accepted and treated as a single
host), for example `192.168.1.7, 2001:db8::5`. It decides whose
`X-Forwarded-For` header the rate limiters believe, so it should cover
the addresses of your reverse proxies.
`X-Forwarded-For` header the rate limiters believe, so it should name
the addresses of your reverse proxies and nothing else.
`X-Forwarded-For` is honoured **only** when the connecting peer is
inside one of these blocks; for every other peer the client identity is
the connection's own address and the header is ignored. Unset (or
empty), the list is the RFC 1918 private ranges: `10.0.0.0/8`,
`172.16.0.0/12` and `192.168.0.0/16`. That covers a reverse proxy
reaching webhooker over a Docker network or a private LAN without
anything set. A set value replaces the default entirely. A set but
unparseable value aborts startup.
the connection's own address and the header is ignored. The default is
the empty list, which trusts nobody — anything else would let any
client pick its own rate limit bucket, minting a fresh one per request
or draining someone else's. Set it to the address of your reverse
proxy, and to nothing wider. A set but unparseable value aborts
startup.
Trusting those ranges has two consequences for clients with private
addresses:
- Any such client, whether it connects directly or through the proxy,
can choose its own rate-limit key by sending its own
`X-Forwarded-For`. A direct client's header is walked because the
client is itself trusted; behind the proxy, the client's own address
is skipped as a trusted hop when the chain is walked (below), so the
entry it wrote is taken as the client. If any of your clients have
private addresses, you must set `TRUSTED_PROXIES` to the proxy's
address alone.
- A client behind the proxy that sends no `X-Forwarded-For` of its own
shares the proxy's bucket, because its own address is skipped too.
Setting the list to the proxy's address alone gives each its own
bucket.
A proxy the list does not cover, such as nginx on the same host
reaching webhooker over loopback, is not trusted: every request through
it keys on the proxy's own address and all clients share a single
That default is safe against forged headers, but leaving it unset in
production has a cost you must know about. Production runs behind a
TLS-terminating reverse proxy, so with `TRUSTED_PROXIES` unset every
request keys on the proxy's own address and all clients share a single
bucket per limit. The receiver limits become service-wide ceilings,
and the login endpoint's failure counting collapses onto one key, so a
stranger's wrong passwords throttle every other client's wrong
passwords. Set `TRUSTED_PROXIES` to that proxy's address to restore
per-client buckets.
passwords.
What it cannot do is lock the operator out. The login endpoint
verifies credentials **before** it consults any limit and charges only
failures, so a correct password is never throttled no matter how full
the bucket is. See [Rate Limiting](#rate-limiting).
The remedy is to set `TRUSTED_PROXIES` to your reverse proxy's
address, which restores per-client buckets. webhooker logs a warning
at startup whenever `TRUSTED_PROXIES` is empty, in every environment,
because behind a proxy every client shares one bucket in `dev` and
`prod` alike. The warning is informational when nothing proxies to the
process: with no proxy in front, the peer address is the client's own
and the buckets are already per-client. See
[Rate Limiting](#rate-limiting) for what each limit shares.
`X-Real-IP` and `True-Client-IP` are **never** read, from any peer.
Reverse proxies append to `X-Forwarded-For` but forward other client
headers verbatim, so a single-valued header is client-controlled even
@@ -442,14 +435,14 @@ Two operator requirements follow:
(nginx `$proxy_add_x_forwarded_for`, HAProxy `option forwardfor`,
Caddy and AWS ALB by default), and must append a bare address with
no port.
- Keep clients out of the list. Any address inside `TRUSTED_PROXIES`
- List proxy hosts **only**. Any address inside `TRUSTED_PROXIES`
chooses its own rate-limit key: its `X-Forwarded-For` is walked, so
it can name a different address on every request to get a fresh
bucket each time, or name another client's address to drain that
client's bucket. A block that also covers clients — the default, on
a network where clients have private addresses — makes all three
limits, including the unauthenticated webhook receiver, silently
bypassable by every client in the block.
client's bucket. Never list a block that also covers clients — a
broad `10.0.0.0/8` on a network where clients live in the same range
makes all three limits, including the unauthenticated webhook
receiver, silently bypassable by every client in the block.
#### Sessions
@@ -545,6 +538,12 @@ its Argon2id hash. There is no second account and no forgot-password
flow, so the banner and the reset command below are the only two ways
in.
A start that finds no `webhooker.db` in `DATA_DIR` also logs
`created a new, empty database` at `WARN`, with the file's path,
shortly before the banner. On a deployment that has run before, that
line means `DATA_DIR` was empty, most often because its volume is not
mounted.
#### Recovering a lost admin password
`webhooker resetpw` sets an existing account's password from the
@@ -559,8 +558,9 @@ printf '%s' "$NEW_PASSWORD" | \
DATA_DIR=/var/lib/webhooker webhooker resetpw admin
```
In a container it is the same binary, which the image sets as `CMD`
rather than `ENTRYPOINT`, so the whole command has to be given:
In a container it is the same binary. The image's `CMD` is
`/app/webhooker`, and a command given to `docker run` replaces all of
it, so the whole command has to be given:
```bash
docker run --rm -v webhooker-data:/var/lib/webhooker \
@@ -697,38 +697,22 @@ 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
SQLite databases: the main application database (`webhooker.db`), the
per-webhook event databases (`events-{uuid}.db`), and any archive
databases written by `database` targets (`archive-{uuid}.db`). Mount
this as a persistent volume to preserve data across container
restarts.
The app runs as a non-root user (`webhooker`, UID 1000), exposes port
8080, and includes a health check against `/.well-known/healthcheck`.
The `/var/lib/webhooker` volume holds all SQLite databases: the main
application database (`webhooker.db`), the per-webhook event databases
(`events-{uuid}.db`), and any archive databases written by `database`
targets (`archive-{uuid}.db`). Mount this as a persistent volume to
preserve data across container restarts.
**The bind-mounted directory must be owned by UID 1000, or the
container does not start.** Docker creates a `-v` source path that
does not exist yet as `root:root`, and the process runs as UID 1000,
so it cannot take its `DATA_DIR` lock:
```
webhooker: locking data directory /var/lib/webhooker: open
/var/lib/webhooker/webhooker.lock: permission denied
```
It exits non-zero at that point, before opening any database. Create
the directory ahead of the first `docker run`:
```bash
mkdir -p /path/to/data
chown 1000:1000 /path/to/data
chmod 750 /path/to/data
```
The same `chown` is what a restore needs — see step 4 of
[Restore](#restore). A **named volume** does not have this problem:
Docker copies the image's ownership onto a volume it initializes, and
the image creates `/var/lib/webhooker` owned by `webhooker`.
**The container sets its data directory's owner and mode itself
before the app starts**, so a host directory can be mounted as it is,
whoever owns it. The image's `ENTRYPOINT`,
`deploy/docker-entrypoint.sh`, starts as root, creates `DATA_DIR` if
it is missing, gives the directory and anything in it that belongs to
another user to `webhooker`, sets the directory to `0750`, and only
then runs the app as `webhooker`. Started with `--user`, it changes
nothing and runs the app as that user.
**The file modes are not yours to set, and do not depend on the
directory.** `webhooker.db` holds target configuration in plaintext —
@@ -736,13 +720,10 @@ bearer tokens, API keys, Slack webhook URLs — along with the session
encryption key, so webhooker creates every SQLite file it owns `0600`:
each database and both of its `-wal` and `-shm` sidecars, across all
three tiers. Files an earlier build left `0644` are tightened when
they are opened. A `DATA_DIR` webhooker creates itself is `0750`, but
a bind mount supplies its own directory and Docker's default for one
it creates is `0755`; the `0600` files hold there regardless. The
`chmod 750` above is defence in depth — it stops other local users
listing the directory and learning your webhook UUIDs from the
`events-{uuid}.db` filenames — not the barrier protecting the
credentials.
they are opened. The directory's `0750` is defence in depth — it stops
other local users listing the directory and learning your webhook
UUIDs from the `events-{uuid}.db` filenames — not the barrier
protecting the credentials.
### Running under upaas
@@ -758,29 +739,12 @@ repository's `Dockerfile` and runs it. The app needs:
app name, port `8080`. Leave `PORT` unset: the image's health check
probes `8080`.
- **Volume:** one host directory mounted at `/var/lib/webhooker`.
upaas bind-mounts the host path it is given and does not create it,
and the container does not start unless UID 1000 owns it (see
[Running with Docker](#running-with-docker)). Create it before the
first deploy:
```bash
mkdir -p /path/to/data
chown 1000:1000 /path/to/data
chmod 750 /path/to/data
```
- **Environment variables:**
- `WEBHOOKER_ENVIRONMENT=prod`
- `TRUSTED_PROXIES`: Docker networks use private addresses, so the
default covers your reverse proxy on that network. Under the
default, any client with a private address, whether it connects
directly or through the proxy, can choose its own rate-limit key
by sending its own `X-Forwarded-For`. If any clients have private
addresses, or the network's addresses are outside the RFC 1918
ranges, set it to the proxy's address there. The `remoteIP` field
of the `http request` log line for a request that came through
the proxy shows it; the health check's own lines show `::1`. See
[Trusted proxies](#trusted-proxies).
- `TRUSTED_PROXIES`: your reverse proxy's address on that Docker
network. The `remoteIP` field of the `http request` log line for a
request that came through the proxy shows it; the health check's
own lines show `::1`. See [Trusted proxies](#trusted-proxies).
- Leave `BIND_ADDRESS` and `DATA_DIR` unset: the image sets
`BIND_ADDRESS` to `0.0.0.0`, and `DATA_DIR` defaults to
`/var/lib/webhooker`.
@@ -843,14 +807,12 @@ reports.
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. **Make sure `TRUSTED_PROXIES` covers the proxy's address.** Unset,
it covers the RFC 1918 private ranges, so a proxy on a Docker
network or a private LAN is covered and one on loopback is not. For
a proxy it does not cover, every rate limiter keys on the connecting
peer, which 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). If
any clients have private addresses, list the proxy and nothing else.
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
@@ -1035,12 +997,12 @@ done
`.backup` reads through the WAL and writes a single consistent file with
no sidecars of its own, so the destination is complete as it stands.
Two caveats. First, the runtime image is `alpine:3.21` with only
`ca-certificates` added — the `sqlite3` CLI is **not** in it, so run
this on the host against the volume path, or from a throwaway container
that mounts the volume. Second, each file is captured at its own
instant, so a webhook created or an event delivered between two files
being copied lands in one and not the other. If you need the whole set
coherent as of a single moment, stop the service.
`ca-certificates` and `su-exec` added — the `sqlite3` CLI is **not** in
it, so run this on the host against the volume path, or from a
throwaway container that mounts the volume. Second, each file is
captured at its own instant, so a webhook created or an event delivered
between two files being copied lands in one and not the other. If you
need the whole set coherent as of a single moment, stop the service.
Note that `sqlite3 <db> .dump` is **not** one of these procedures: it is
an export, it holds a read transaction open for as long as it runs, and
@@ -1094,21 +1056,11 @@ with any `-wal`/`-shm` beside it, or wait until there are none.
archive not opened since a crash. A copy salvaged from a crashed
instance has them for everything, and needs all of them.
4. **Fix ownership.** The container runs as the non-root `webhooker`
user, UID 1000 / GID 1000. Restored files must be owned by (or
writable by) that UID, and so must the directory itself — SQLite
creates the `-wal` and `-shm` sidecars beside the database, so a
writable file inside a directory it cannot write is not enough:
```bash
chown -R 1000:1000 /path/to/data
```
Restoring as `root` on the host and forgetting this step is the
usual way a restore fails.
5. Start the service. `AutoMigrate` runs against each restored database
as it is opened.
4. Start the service. The container gives the directory and the
restored files to the `webhooker` user before the app starts,
whoever restored them (see
[Running with Docker](#running-with-docker)). `AutoMigrate` runs
against each restored database as it is opened.
### Upgrades
@@ -1411,11 +1363,10 @@ It uses:
- **[go-chi/httprate](https://github.com/go-chi/httprate)** for
sliding-window rate limiting of the password-change and webhook
receiver endpoints. The bucket is per client IP only when
`TRUSTED_PROXIES` covers the reverse proxy (by default it covers the
RFC 1918 private ranges); otherwise every client behind that proxy
shares one bucket per limit. The login endpoint counts failed
attempts itself instead, so that a correct password is never
throttled (see [Rate Limiting](#rate-limiting))
`TRUSTED_PROXIES` names the reverse proxy; unset, every client
behind that proxy shares one bucket per limit. The login endpoint
counts failed attempts itself instead, so that a correct password is
never throttled (see [Rate Limiting](#rate-limiting))
- **[Prometheus](https://prometheus.io)** for metrics, served at
`/metrics` behind basic auth
- **[Sentry](https://sentry.io)** for optional error reporting
@@ -2596,30 +2547,30 @@ let one subscriber rotate source addresses and mint a fresh bucket per
request, evading these limits at the network layer without spoofing
anything; the cost is that distinct clients inside one `/64` share a
bucket. IPv4-mapped addresses (`::ffff:1.2.3.4`) key as the IPv4 address
they carry. See [Trusted proxies](#trusted-proxies). When that variable
does not cover the reverse proxy, a client behind it shares one bucket
with every other client behind the same proxy. Set `TRUSTED_PROXIES` to
the proxy's address to get per-client limits back. What the shared bucket
they carry. See [Trusted proxies](#trusted-proxies). Deployed without that
variable set, a client behind a reverse proxy shares one bucket with
every other client behind the same proxy. Set `TRUSTED_PROXIES` to the
proxy's address to get per-client limits back. What the shared bucket
costs is not the same for every limiter, and the two cases pull in
opposite directions:
- For the **receiver** limits it costs throughput, which is the safe
direction to be wrong in: sharing can only make a limit bind sooner,
never let a sender past it. It matters more for the aggregate limit
than for the per-entrypoint one: when `TRUSTED_PROXIES` does not
cover the reverse proxy a production deployment is required to run
behind, every request keys on the proxy, so the aggregate limit
becomes a service-wide ceiling of 1200 requests per minute across all
senders and all entrypoints, where the per-entrypoint limit's
capacity still grows with the number of entrypoints. Any deployment
with more than a handful of busy entrypoints must make sure
`TRUSTED_PROXIES` covers its proxy.
than for the per-entrypoint one: with `TRUSTED_PROXIES` unset behind
the reverse proxy a production deployment is required to run behind,
every request keys on the proxy, so the aggregate limit becomes a
service-wide ceiling of 1200 requests per minute across all senders
and all entrypoints, where the per-entrypoint limit's capacity still
grows with the number of entrypoints. Any deployment with more than a
handful of busy entrypoints must set `TRUSTED_PROXIES`.
- For the **login and password-change** limits it costs precision, not
availability. Login failures from every client land in one counter,
so a stranger's wrong passwords make the operator's own wrong
passwords answer `429` sooner; the operator's _correct_ password is
never affected, because it is never counted. Production deployments
should still make sure `TRUSTED_PROXIES` covers their proxy.
should still set `TRUSTED_PROXIES`; webhooker warns at startup
whenever it is empty, in any environment.
#### The login endpoint
@@ -2722,10 +2673,8 @@ re-fills both verification slots on its first two requests. The
remedies are to block the source at the reverse proxy, or to
rate-limit `POST /pages/login` there — the one place a limit can be
applied without reintroducing the lockout, because the proxy sees the
real client address. `TRUSTED_PROXIES` does not stop the saturation.
The flood's source is in the proxy's access log: webhooker's own logs
record the proxy's address, not the client's (see
[Deployment behind a reverse proxy](#deployment-behind-a-reverse-proxy)).
real client address. Setting `TRUSTED_PROXIES` does not stop the
saturation, but it makes the source visible in the failure logs.
Finer-grained per-webhook rate limits (configured in the web UI and
enforced in the webhook handler) can layer on top of this env-level
@@ -2739,7 +2688,7 @@ abuse limit later; they are tracked as future work.
| ------ | --------------------------- | ----------- |
| `GET` | `/` | Root redirect, 303 (authenticated → `/sources`, unauthenticated → `/pages/login`) |
| `GET` | `/.well-known/healthcheck` | Health check (JSON: `status`, `now`, `uptimeSeconds`, `uptimeHuman`, `version`, `appname`, `maintenanceMode`) |
| any | `/s/*` | Static file serving (embedded CSS, JS). Mounted for every method, not just `GET`/`HEAD`: chi's `Mount` registers all methods and `http.FileServer` special-cases only `HEAD` (by omitting the body), so a `POST` or `DELETE` to an asset is answered `200` with the file. Pinned by `TestStaticServesEveryMethod` |
| `GET`, `HEAD` | `/s/*` | Static file serving (embedded CSS, JS). `GET` and `HEAD` only — `POST`, `PUT`, `PATCH`, `DELETE`, `OPTIONS`, `TRACE` and `CONNECT` are answered `405 Method Not Allowed` with `Allow: GET, HEAD`. Any other method (such as `PROPFIND`) is refused by chi before it reaches this route, and gets `405` without an `Allow` header. Pinned by `TestStaticServesOnlyGetAndHead` |
| `POST` | `/webhook/{uuid}` | Webhook receiver endpoint. `POST` only — every other method is answered `405 Method Not Allowed` with `Allow: POST`. Rate limited (see [Rate Limiting](#rate-limiting)) |
#### Authentication Endpoints
@@ -3082,13 +3031,18 @@ check, see [The login endpoint](#the-login-endpoint).
It runs behind session auth, so only a client already holding a
valid session reaches it, and an operator throttled out of changing
a password can still log in. The bucket is per client IP only when
`TRUSTED_PROXIES` covers the reverse proxy; otherwise every client
`TRUSTED_PROXIES` names the reverse proxy; unset, every client
shares one bucket, which costs precision rather than availability
(see [Rate Limiting](#rate-limiting))
(see [Rate Limiting](#rate-limiting)). webhooker warns at startup
whenever `TRUSTED_PROXIES` is empty
- Prometheus metrics behind basic auth
- Static assets embedded in binary (no filesystem access needed at
runtime)
- Container runs as non-root user (UID 1000)
- The app runs as the non-root `webhooker` user (UID 1000) in the
container. The image sets no `USER`, so these run as root: the
`ENTRYPOINT` script, which sets the data directory's owner and mode
before the app starts; the image's health check; and `docker exec`,
unless given `--user`
- GORM soft deletes on every entity that carries `BaseModel`, which is
all of them but `Setting` (data preserved for audit)
@@ -3215,10 +3169,13 @@ version is fixed independently of the compiler's:
`GO_LDFLAGS`, so neither can drop the `-X` that stamps the version.
The version arrives as the `VERSION` build arg, since the context
has no `.git` (see [Version stamping](#version-stamping)).
3. **Runtime stage** (`alpine:3.21`) — copies the static binary,
creates the `/var/lib/webhooker` directory for all SQLite databases,
runs as the non-root `webhooker` user (UID 1000), exposes port 8080,
and includes a health check against `/.well-known/healthcheck`.
3. **Runtime stage** (`alpine:3.21`) — copies the static binary and
`deploy/docker-entrypoint.sh`, creates the `/var/lib/webhooker`
directory for all SQLite databases, exposes port 8080, and includes
a health check against `/.well-known/healthcheck`. It sets no
`USER`: the `ENTRYPOINT` script starts as root, sets the data
directory's owner and mode, and runs the app as the non-root
`webhooker` user (UID 1000) through `su-exec`.
The lint stage invokes `golangci-lint` directly rather than `make lint`:
it is already the pinned linter image, and `make lint` builds
@@ -3297,3 +3254,5 @@ MIT
## Author
[@sneak](https://sneak.berlin)
+22
View File
@@ -0,0 +1,22 @@
#!/bin/sh
# deploy/docker-entrypoint.sh: the image's ENTRYPOINT. A bind-mounted
# data directory keeps its owner from the host, often root, and the app
# could not write to it. Started as root, this creates DATA_DIR if
# needed, gives it and everything in it to webhooker, sets its mode, and
# runs the command as webhooker, so the app never runs as root. Started
# as another user, it only runs the command.
set -eu
main() {
if [ "$(id -u)" != 0 ]; then
exec "$@"
fi
dir="${DATA_DIR:-/var/lib/webhooker}"
mkdir -p "$dir"
find "$dir" ! -user webhooker -exec chown -h webhooker:webhooker {} +
chmod 750 "$dir"
exec su-exec webhooker "$@"
}
main "$@"
+60 -22
View File
@@ -75,11 +75,6 @@ const (
// internet-exposed endpoint.
defaultReceiverRateLimit = 120
// defaultTrustedProxies is TRUSTED_PROXIES when it is unset: the
// RFC 1918 private ranges, which a reverse proxy reaching the
// process over a Docker network or a private LAN connects from.
defaultTrustedProxies = "10.0.0.0/8,172.16.0.0/12,192.168.0.0/16"
// maxPort is the highest valid TCP port number. The lower
// bound (at least 1) is enforced by envPositiveInt.
maxPort = 65535
@@ -177,14 +172,13 @@ type Config struct {
// TrustedProxies is the set of networks whose members are
// allowed to speak for the client with X-Forwarded-For, the
// only forwarded header read. Unless TRUSTED_PROXIES is set it
// is the RFC 1918 private ranges (defaultTrustedProxies).
// Other peers' forwarded headers are ignored and they are
// identified by the connection's own address. Under the
// default any client with a private address, directly or
// through a proxy, can choose its own rate-limit key, so
// where any clients have private addresses this must be set
// to the proxy hosts alone.
// only forwarded header read. It is empty unless
// TRUSTED_PROXIES is set, and empty means no peer is
// trusted: forwarded headers are then ignored entirely and
// clients are identified by the connection's own address.
// Members can choose their own rate-limit key, so this must
// name proxy hosts only, never a block that also covers
// clients.
TrustedProxies []netip.Prefix
// AllowedEgressCIDRs is the set of networks a delivery target
@@ -466,15 +460,14 @@ func parseCIDR(entry string) (netip.Prefix, error) {
// envPrefixList returns the value of the named environment variable
// parsed as a comma-separated list of CIDR blocks (bare addresses
// allowed). An unset, empty, or blank value is read as defaultValue
// instead. A set value containing an unparseable entry is a hard
// error naming the key and the bad entry, so startup fails loudly
// rather than silently running with a list the operator did not
// intend.
func envPrefixList(key, defaultValue string) ([]netip.Prefix, error) {
// allowed). An unset, empty, or blank value yields an empty list. A
// set value containing an unparseable entry is a hard error naming
// the key and the bad entry, so startup fails loudly rather than
// silently running with a list the operator did not intend.
func envPrefixList(key string) ([]netip.Prefix, error) {
v := strings.TrimSpace(os.Getenv(key))
if v == "" {
v = defaultValue
return nil, nil
}
var prefixes []netip.Prefix
@@ -688,12 +681,12 @@ func loadFromEnv() (*Config, error) {
return nil, err
}
trustedProxies, err := envPrefixList("TRUSTED_PROXIES", defaultTrustedProxies)
trustedProxies, err := envPrefixList("TRUSTED_PROXIES")
if err != nil {
return nil, err
}
allowedEgressCIDRs, err := envPrefixList("ALLOWED_EGRESS_CIDRS", "")
allowedEgressCIDRs, err := envPrefixList("ALLOWED_EGRESS_CIDRS")
if err != nil {
return nil, err
}
@@ -767,6 +760,50 @@ func (c *Config) warnEgressAllowlist(log *slog.Logger) {
)
}
// warnSharedRateLimitBucket logs a startup warning whenever
// TRUSTED_PROXIES is empty, in any environment.
//
// With no trusted proxies every rate limiter keys on the connecting
// peer's address. Whether that is harmless or dangerous depends on
// what is in front of the process, which this code cannot observe:
// with nothing in front, the peer is the client and the limits are
// per-client as intended; behind a reverse proxy the peer is the proxy
// for every request, so all clients share one bucket per limiter.
//
// The login endpoint no longer spends budget on arrival — it verifies
// credentials first and charges only failures — so a shared bucket
// cannot deny the operator a correct password. What it does collapse
// is the failure counting: one client's wrong passwords throttle
// everyone else's wrong passwords, and the receiver's limits become
// service-wide ceilings.
//
// The warning is deliberately not gated on WEBHOOKER_ENVIRONMENT:
// behind a proxy every client shares one bucket in dev and prod alike.
//
// The default of trusting nobody is deliberate — trusting forwarded
// headers from arbitrary peers lets any client choose its own bucket —
// so this warns rather than failing startup or changing the key.
func (c *Config) warnSharedRateLimitBucket(log *slog.Logger) {
if len(c.TrustedProxies) > 0 {
return
}
log.Warn(
"TRUSTED_PROXIES is empty: every rate limit keys on the "+
"connecting peer's address. With nothing proxying to "+
"this process that is the client itself and the limits "+
"are per-client as intended. Behind a reverse proxy the "+
"peer is the proxy on every request, so all clients "+
"share one bucket per limit: the receiver limits become "+
"service-wide ceilings, and one client's failed logins "+
"throttle every other client's failed logins — a "+
"correct password still gets in. If anything proxies to "+
"this process, set TRUSTED_PROXIES to its address.",
"environment", c.Environment,
"trustedProxies", len(c.TrustedProxies),
)
}
// New creates a Config by reading environment variables.
//
//nolint:revive // lc parameter is required by fx even if unused.
@@ -812,6 +849,7 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
"hasMetricsAuth", s.MetricsAuthEnabled(),
)
s.warnSharedRateLimitBucket(log)
s.warnEgressAllowlist(log)
return s, nil
+101 -14
View File
@@ -551,11 +551,6 @@ func testReceiverRateLimitSuccess(
}
func TestTrustedProxies(t *testing.T) {
// Unset, the RFC 1918 private ranges are trusted, so a reverse
// proxy on a Docker network or a private LAN is covered without
// configuration.
defaultProxies := []string{cidrPrivateV4, "172.16.0.0/12", "192.168.0.0/16"}
tests := []struct {
name string
set bool
@@ -564,21 +559,18 @@ func TestTrustedProxies(t *testing.T) {
expected []string
}{
{
// The default must be "trust nobody": an empty list
// means forwarded headers are ignored, never that
// every peer may speak for the client.
name: caseUnsetUsesDefault,
set: false,
expected: defaultProxies,
expected: []string{},
},
{
name: "blank value uses default",
name: "blank value trusts nothing",
set: true,
value: " ",
expected: defaultProxies,
},
{
name: "set value replaces the default entirely",
set: true,
value: "203.0.113.7",
expected: []string{"203.0.113.7/32"},
expected: []string{},
},
{
name: caseValidValueParsed,
@@ -853,6 +845,101 @@ func TestEgressAllowlistWarning(t *testing.T) {
}
}
// TestSharedRateLimitBucketWarning covers the startup warning that
// tells an operator a deployment behind a reverse proxy shares one
// rate-limit bucket between every client, which turns the receiver
// limits into service-wide ceilings and collapses login failure
// counting. It must fire whenever TRUSTED_PROXIES is empty, in any
// environment, because behind a proxy every client shares one bucket
// in dev and prod alike. It stays quiet once proxies are named.
func TestSharedRateLimitBucketWarning(t *testing.T) {
tests := []struct {
name string
environment string
trustedProxies string
expectWarning bool
}{
{
name: "prod without trusted proxies warns",
environment: config.EnvironmentProd,
expectWarning: true,
},
{
name: "prod with trusted proxies is quiet",
environment: config.EnvironmentProd,
trustedProxies: cidrPrivateV4,
expectWarning: false,
},
{
name: "dev without trusted proxies warns",
environment: config.EnvironmentDev,
expectWarning: true,
},
{
name: "dev with trusted proxies is quiet",
environment: config.EnvironmentDev,
trustedProxies: cidrPrivateV4,
expectWarning: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Cannot use t.Parallel() here because t.Setenv
// is incompatible with parallel subtests.
t.Setenv("WEBHOOKER_ENVIRONMENT", tt.environment)
if tt.trustedProxies == "" {
require.NoError(
t, os.Unsetenv("TRUSTED_PROXIES"),
)
} else {
t.Setenv("TRUSTED_PROXIES", tt.trustedProxies)
}
var buf bytes.Buffer
log := slog.New(slog.NewJSONHandler(
&buf, &slog.HandlerOptions{
Level: slog.LevelDebug,
},
))
require.NoError(
t,
config.WarnSharedRateLimitBucketForTest(log),
)
if !tt.expectWarning {
assert.Empty(t, buf.String())
return
}
logged := buf.String()
assert.Contains(t, logged, `"level":"WARN"`)
assert.Contains(t, logged, "TRUSTED_PROXIES")
assert.Contains(t, logged, "share one bucket")
assert.Contains(
t, logged, "throttle every other client's failed logins",
)
// The warning must not claim a lockout the login
// endpoint no longer permits: credentials are verified
// before any budget is spent.
assert.Contains(
t, logged, "a correct password still gets in",
)
// The text must stay accurate for a developer with
// nothing in front of the process, where an empty
// list costs nothing.
assert.Contains(
t, logged, "nothing proxying to this process",
)
})
}
}
// metricsEnv describes what one subtest below puts in the
// environment for a single METRICS_ variable. A variable that is
// set to the empty string and one that is not set at all are
+15
View File
@@ -6,6 +6,21 @@ import "log/slog"
// the external config_test package so each helper can be covered by
// its own table-driven test without weakening the package API.
// WarnSharedRateLimitBucketForTest loads a Config from the current
// environment and emits its startup warnings to log. The real logger
// writes to stdout, so this lets the warning's firing condition be
// asserted against a handler the test controls.
func WarnSharedRateLimitBucketForTest(log *slog.Logger) error {
c, err := loadFromEnv()
if err != nil {
return err
}
c.warnSharedRateLimitBucket(log)
return nil
}
// WarnEgressAllowlistForTest loads a Config from the current
// environment and emits its egress-allowlist startup warning to
// log, so a test can assert both that the warning fires only when
@@ -3,6 +3,8 @@ package database_test
import (
"bytes"
"context"
"log/slog"
"path/filepath"
"strings"
"testing"
@@ -83,3 +85,37 @@ func TestFirstBoot_PrintsTheAdminPasswordAsABanner(t *testing.T) {
t, ok, "the printed password must open the seeded account",
)
}
// TestNewDatabase_IsLoggedWithItsPath is the log half of
// https://git.eeqj.de/sneak/webhooker/issues/359. A DATA_DIR that is
// unexpectedly empty boots exactly like a first start, so the start
// that creates the database must say so, and where. Opening that
// database again must not.
func TestNewDatabase_IsLoggedWithItsPath(t *testing.T) {
t.Parallel()
dir := t.TempDir()
open := func() string {
var out bytes.Buffer
db, err := database.Open(dir, slog.New(slog.NewTextHandler(&out, nil)))
require.NoError(t, err)
require.NoError(t, db.Close())
return out.String()
}
const created = `level=WARN msg="created a new, empty database"`
first := open()
second := open()
assert.Contains(
t, first,
created+" path="+filepath.Join(dir, database.MainDBFileName),
)
assert.NotContains(
t, second, created, "an existing database is not new",
)
}
+13 -1
View File
@@ -8,6 +8,7 @@ import (
"errors"
"fmt"
"io"
"io/fs"
"log/slog"
"os"
"path/filepath"
@@ -199,6 +200,12 @@ func (d *Database) connectTo(dataDir string) error {
// Construct the main application database path inside DATA_DIR.
dbPath := filepath.Join(dataDir, MainDBFileName)
// Checked before opening, which creates the file. A DATA_DIR that
// is unexpectedly empty -- its volume not mounted, say -- looks
// exactly like a first start, so a new database is a warning.
_, statErr := os.Stat(dbPath)
created := errors.Is(statErr, fs.ErrNotExist)
// Opened through OpenSQLite so this handle carries the same WAL
// journaling, busy timeout, immediate-transaction locking, and pool
// bounds as every other database file. See sqlite_open.go.
@@ -229,7 +236,12 @@ func (d *Database) connectTo(dataDir string) error {
}
d.db = db
d.log.Info("connected to database", "path", dbPath)
if created {
d.log.Warn("created a new, empty database", "path", dbPath)
} else {
d.log.Info("connected to database", "path", dbPath)
}
// Run migrations
return d.migrate()
+3 -4
View File
@@ -103,10 +103,9 @@ func (h *Handlers) renderLoginError(
// The credential check runs BEFORE any rate-limit budget is
// consulted, and only a failed check spends budget. That is what
// keeps the single administrative path reachable: behind the reverse
// proxy this deployment requires, when TRUSTED_PROXIES does not cover
// it, every client shares one bucket, so a limiter spent on arrival
// lets any stranger deny the operator's own correct password
// indefinitely.
// proxy this deployment requires, with TRUSTED_PROXIES unset, every
// client shares one bucket, so a limiter spent on arrival lets any
// stranger deny the operator's own correct password indefinitely.
//
// Verifying first means every login POST costs an Argon2id hash, so
// the work is taken under a bounded number of verification slots.
+6 -6
View File
@@ -25,7 +25,7 @@ const (
// sharedProxyPeer is the whole point of this file. Production is
// required to run behind a TLS-terminating reverse proxy, and
// when TRUSTED_PROXIES does not cover it every client — attacker
// TRUSTED_PROXIES defaults to empty, so every client — attacker
// and operator alike — reaches the process from the proxy's
// address and shares one rate-limit bucket. Both parties in
// these tests therefore use the same RemoteAddr.
@@ -115,11 +115,11 @@ func floodFailures(
// done-criterion of https://git.eeqj.de/sneak/webhooker/issues/150.
//
// The attacker and the operator share one rate-limit bucket, because
// behind the mandated reverse proxy, when TRUSTED_PROXIES does not
// cover it, every client keys on the proxy's address. The attacker
// floods the operator's own username — a single-admin product has a
// predictable one — far past the failure limit. The operator must
// still be able to log in with the correct password.
// behind the mandated reverse proxy with TRUSTED_PROXIES unset every
// client keys on the proxy's address. The attacker floods the
// operator's own username — a single-admin product has a predictable
// one — far past the failure limit. The operator must still be able
// to log in with the correct password.
//
// This fails if credentials stop being verified ahead of the limiter.
func TestLogin_StrangersFloodCannotLockOutTheOperator(t *testing.T) {
+4 -4
View File
@@ -108,10 +108,10 @@ type failureWindow struct {
//
// A limiter that spends budget on arrival cannot protect a
// single-admin product: behind the reverse proxy the deployment
// requires, when TRUSTED_PROXIES does not cover it, every client
// keys on the proxy, so a stranger trickling five POSTs a minute
// keeps the one bucket full and the operator's own correct password
// is answered 429 forever. There is no second administrative path.
// requires, with TRUSTED_PROXIES unset, every client keys on the
// proxy, so a stranger trickling five POSTs a minute keeps the one
// bucket full and the operator's own correct password is answered 429
// forever. There is no second administrative path.
//
// So budget is spent only by a FAILED verification. A correct
// password is never throttled, whatever the counters say, which is
+3 -2
View File
@@ -123,8 +123,9 @@ func bucketKey(addr netip.Addr) string {
return prefix.String()
}
// isTrustedProxy reports whether addr belongs to a network in
// TRUSTED_PROXIES, which by default is the RFC 1918 private ranges.
// isTrustedProxy reports whether addr belongs to a network the
// operator listed in TRUSTED_PROXIES. The list is empty by default,
// so by default nothing is trusted.
func (m *Middleware) isTrustedProxy(addr netip.Addr) bool {
for _, prefix := range m.params.Config.TrustedProxies {
if prefix.Contains(addr) {
+2 -2
View File
@@ -426,8 +426,8 @@ func assertSharedBucket(
}
// TestRateLimitKey_SpoofedForwardedFromUntrustedPeer is the test
// this gating exists for: from a peer that is not a trusted
// proxy, a client that rotates a forwarded header on every
// this gating exists for: with no trusted proxies configured (the
// default), a client that rotates a forwarded header on every
// request must stay in one bucket. If forwarded headers were
// trusted unconditionally, each spoofed value would mint a fresh
// bucket and the limit would stop no one.
+22 -9
View File
@@ -92,11 +92,25 @@ func (s *Server) setupGlobalMiddleware() {
func (s *Server) setupRoutes() {
s.router.Get("/", s.h.HandleIndex())
s.router.Mount(
"/s",
http.StripPrefix("/s", http.FileServer(http.FS(static.Static))),
// Static assets answer GET and HEAD only. chi's default 405
// carries no Allow header, so this group supplies its own.
staticFiles := http.StripPrefix(
"/s", http.FileServer(http.FS(static.Static)),
)
s.router.Route("/s", func(r chi.Router) {
r.MethodNotAllowed(func(w http.ResponseWriter, _ *http.Request) {
w.Header().Set("Allow", "GET, HEAD")
http.Error(
w,
"Method Not Allowed",
http.StatusMethodNotAllowed,
)
})
r.Method(http.MethodGet, "/*", staticFiles)
r.Method(http.MethodHead, "/*", staticFiles)
})
s.router.Route("/api/v1", func(_ chi.Router) {
// API routes will be added here.
})
@@ -140,12 +154,11 @@ func (s *Server) setupPageRoutes() {
r.Use(s.mw.NoCache())
// The login POST carries no pre-emptive rate limiter. Behind
// the reverse proxy production requires, when TRUSTED_PROXIES
// does not cover it, every client shares one bucket, so a
// limiter spent on arrival lets any stranger deny the operator
// the only administrative path. The handler verifies
// credentials first and charges only failures; see
// Handlers.authenticateUser.
// the reverse proxy production requires, with TRUSTED_PROXIES
// unset, every client shares one bucket, so a limiter spent
// on arrival lets any stranger deny the operator the only
// administrative path. The handler verifies credentials first
// and charges only failures; see Handlers.authenticateUser.
r.Get("/login", s.h.HandleLoginPage())
r.Post("/login", s.h.HandleLoginSubmit())
+108 -19
View File
@@ -7,6 +7,7 @@ import (
"net/http/httptest"
"net/url"
"regexp"
"slices"
"strconv"
"strings"
"testing"
@@ -220,9 +221,21 @@ func (e *testEnv) csrfFrom(
// out of the markup has to be unescaped before it is submitted.
token := html.UnescapeString(match[1])
combined := make([]*http.Cookie, 0, len(cookies))
combined = append(combined, cookies...)
combined = append(combined, w.Result().Cookies()...)
// A cookie the page sets replaces the one of the same name, as in
// a browser. Sent both, the server would read the first, older one.
set := w.Result().Cookies()
combined := make([]*http.Cookie, 0, len(cookies)+len(set))
for _, c := range cookies {
replaced := slices.ContainsFunc(set, func(n *http.Cookie) bool {
return n.Name == c.Name
})
if !replaced {
combined = append(combined, c)
}
}
combined = append(combined, set...)
return token, combined
}
@@ -396,13 +409,15 @@ func (e *testEnv) storedHash(t *testing.T, username string) string {
// --- /s static group ---
// TestStaticServesEveryMethod pins what the static mount actually
// answers. chi's Mount registers the handler for all methods and
// http.FileServer only special-cases HEAD (by suppressing the body),
// so a POST or a DELETE to an asset is served the file rather than
// refused. The README documents this; the test is what keeps the two
// from drifting.
func TestStaticServesEveryMethod(t *testing.T) {
// TestStaticServesOnlyGetAndHead pins the methods the static group
// answers: GET and HEAD are served the asset, and the other methods
// chi routes (POST, PUT, DELETE and the rest) are refused with 405
// and an Allow header naming those two. A method chi does not route,
// such as PROPFIND, is refused with 405 by the top-level router
// before it reaches the static group, so it gets no Allow header.
// The README documents this; the test is what keeps the two from
// drifting.
func TestStaticServesOnlyGetAndHead(t *testing.T) {
t.Parallel()
env := newTestEnv(t)
@@ -417,6 +432,7 @@ func TestStaticServesEveryMethod(t *testing.T) {
http.MethodPost,
http.MethodPut,
http.MethodDelete,
"PROPFIND",
} {
t.Run(method, func(t *testing.T) {
t.Parallel()
@@ -428,18 +444,38 @@ func TestStaticServesEveryMethod(t *testing.T) {
w := httptest.NewRecorder()
env.router.ServeHTTP(w, req)
assert.Equal(t, http.StatusOK, w.Code,
"static mount answers every method")
if method == http.MethodHead {
switch method {
case http.MethodGet:
assert.Equal(t, http.StatusOK, w.Code)
assert.Equal(t, body, w.Body.Bytes(),
"the asset itself is returned")
case http.MethodHead:
assert.Equal(t, http.StatusOK, w.Code)
assert.Empty(t, w.Body.Bytes(),
"HEAD must not carry a body")
return
case "PROPFIND":
assert.Equal(
t, http.StatusMethodNotAllowed, w.Code,
)
assert.Empty(t, w.Header().Get("Allow"),
"chi refuses a method it does not route "+
"before the static group runs")
assert.NotContains(
t, w.Body.String(), string(body),
"a refused method must not get the asset",
)
default:
assert.Equal(
t, http.StatusMethodNotAllowed, w.Code,
)
assert.Equal(
t, "GET, HEAD", w.Header().Get("Allow"),
)
assert.NotContains(
t, w.Body.String(), string(body),
"a refused method must not get the asset",
)
}
assert.Equal(t, body, w.Body.Bytes(),
"the asset itself is returned")
})
}
}
@@ -591,6 +627,59 @@ func TestPagesLogin_CorrectPasswordSurvivesASpentBudget(
)
}
// TestPagesLogin_CookiesFromAnEarlierDatabase is
// https://git.eeqj.de/sneak/webhooker/issues/359. A new database
// brings a new session key, and the operator's browser still holds
// the session and CSRF cookies signed with the old one. Logging in
// must work as from a fresh browser and leave cookies the new key
// accepts.
func TestPagesLogin_CookiesFromAnEarlierDatabase(t *testing.T) {
t.Parallel()
const (
username = "operator"
password = "correct-horse-battery-staple"
)
earlier := newTestEnv(t)
earlierID, _ := earlier.seedUser(t, username, password)
_, stale := earlier.csrfFrom(t, "/pages/login", nil)
stale = append(stale, earlier.authCookies(t, earlierID, username)...)
env := newTestEnv(t)
env.seedUser(t, username, password)
token, cookies := env.csrfFrom(t, "/pages/login", stale)
form := url.Values{}
form.Set("csrf_token", token)
form.Set("username", username)
form.Set("password", password)
w := env.post("/pages/login", form, cookies)
require.Equal(
t, http.StatusSeeOther, w.Code,
"a session cookie from another key must not fail the login",
)
// The response deletes the old session cookie and then sets the
// new one; a browser keeps the last.
var fresh *http.Cookie
for _, c := range w.Result().Cookies() {
if c.Name == session.SessionName {
fresh = c
}
}
require.NotNil(t, fresh, "login must set a session cookie")
assert.Equal(
t, "/sources",
env.get("/", []*http.Cookie{fresh}).Header().Get("Location"),
"the new session cookie must authenticate",
)
}
// --- /user/{username} group ---
// TestPasswordChange_OversizeBody_RejectedAndPasswordUnchanged
+8 -7
View File
@@ -19,8 +19,8 @@ import (
)
// The tests below exercise the securecookie codecs underneath the
// store and nothing else: Session.Get only decodes, so no server-side
// expiry check takes part in the result. They exist because
// store and nothing else: they decode through the store itself, so no
// server-side expiry check takes part in the result. They exist because
// NewCookieStore gives its codecs a 30-day max age that assigning
// store.Options does not override, which would let the codec accept a
// cookie weeks past the cap the cookie attribute advertises.
@@ -75,10 +75,11 @@ func restamp(
return base64.URLEncoding.EncodeToString(payload)
}
// decodeCookie feeds value back through the store's decode path.
// decodeCookie feeds value back through the store's decode path. It
// asks the store rather than Session.Get, which treats a cookie that
// does not decode as absent and so hides the codec's reason.
func decodeCookie(
t *testing.T,
s *session.Session,
value string,
) (*sessions.Session, error) {
t.Helper()
@@ -94,7 +95,7 @@ func decodeCookie(
SameSite: http.SameSiteLaxMode,
})
sess, err := s.Get(req)
sess, err := session.NewStore(testKey()).Get(req, session.SessionName)
require.NotNil(t, sess)
return sess, err
@@ -105,7 +106,7 @@ func TestCodec_AcceptsCookieInsideAbsoluteCap(t *testing.T) {
s := testSession(t)
sess, err := decodeCookie(t, s, restamp(
sess, err := decodeCookie(t, restamp(
t,
issuedCookie(t, s),
time.Now().Add(-(testAbsoluteMaxAge-time.Hour)),
@@ -126,7 +127,7 @@ func TestCodec_RejectsCookiePastAbsoluteCap(t *testing.T) {
s := testSession(t)
sess, err := decodeCookie(t, s, restamp(
sess, err := decodeCookie(t, restamp(
t,
issuedCookie(t, s),
time.Now().Add(-(testAbsoluteMaxAge+time.Hour)),
+13 -1
View File
@@ -224,10 +224,22 @@ func New(
}
// Get retrieves a session for the request.
//
// A session cookie that does not decode -- one signed with an earlier
// session key, say, because the database was made anew -- is treated
// as absent: the caller gets a new, empty session and no error, and
// the next save replaces the cookie.
func (s *Session) Get(
r *http.Request,
) (*sessions.Session, error) {
return s.store.Get(r, SessionName)
sess, err := s.store.Get(r, SessionName)
if sess == nil {
return nil, err
}
// For a cookie that does not decode, gorilla/sessions returns a
// new, empty session alongside the error that is dropped here.
return sess, nil
}
// GetKey returns the raw 32-byte authentication key used for