The SSRF blocklist had no escape hatch, so the thing webhooker is
mostly for — taking a public webhook and forwarding it to something
on your own network — could not be configured at all. Every private
address, Docker sibling and loopback service was permanently
unreachable as a delivery destination.
ALLOWED_EGRESS_CIDRS (default empty) names blocks that delivery
targets may reach despite the default blocklist. It is an allowlist
and only ever adds destinations: there is no boolean, and no value
disables SSRF protection wholesale. Empty, the guard behaves exactly
as before.
A fixed set of addresses is refused before the allowlist is
consulted, so no supplied CIDR opens one — not the exact address,
not a supernet, not 0.0.0.0/0 or ::/0. It is the two link-local
blocks (169.254.0.0/16, fe80::/10) plus host routes for the cloud
metadata endpoints that sit outside them: AWS's IPv6 IMDS at
fd00:ec2::254, which lives in ordinary ULA space, and Alibaba's
100.100.100.200, which lives in CGNAT. Allowlisting fd00::/8 or
100.64.0.0/10 (Tailscale's range) is an ordinary thing for an
operator to do and must not reopen instance-credential theft. The
IPv4-compatible (::a9fe:a9fe) and NAT64 (64:ff9b::a9fe:a9fe)
spellings of 169.254.169.254 are listed too, because To4() does not
normalise them into the link-local block the way it does the
IPv4-mapped form. Reaching any of these is credential theft rather
than delivery to an internal service.
The policy now lives in one function, Guard.checkIP, which both
target-creation validation and the delivery dialer call. The two
paths previously decided separately, which is how they came to
disagree about a destination. The guard is built once from config
and injected via fx into both the handlers and the delivery engine,
so there is a single instance and a single answer.
A set-but-unparseable value aborts startup naming the variable,
reusing the existing envPrefixList parser. A non-empty list is
logged at startup with the blocks spelled out, not counted, so the
hole is visible in the log of any deployment that has one.
Tests: an allowlisted loopback CIDR both validates and delivers to a
live server (and the same URL still fails without the allowlist); a
private address outside the listed block stays refused on both
paths; every unconditionally blocked address stays refused on both
paths under an allowlist that covers it, and the set itself is
pinned entry by entry; public addresses are unaffected either way;
and config coverage for parsing, startup abort, and the warning's
contents.
In the shipped default, any stranger denied the operator the only
administrative path at 5 requests per minute: TRUSTED_PROXIES is empty,
the README requires a reverse proxy, so every login POST shared one
bucket keyed on the proxy.
Credentials are now verified first and only a FAILED attempt spends
budget, so a correct password is never throttled. Failures are counted
per (client bucket, submitted username), bounded. Concurrent Argon2id
verifications are capped at two, and the queue for them at 16 — because
verifying first lets an attacker force a 64 MB hash per request, and
bounding the wait alone bounds nothing.
The issue's own recommendation was insufficient and is rejected here:
keying by username stops an attacker locking out a DIFFERENT account,
but this is a single-admin product with a predictable bootstrap
username, so flooding the operator's own name still locks them out.
This is speculative — it implements a corrected recommendation ahead of
the owner's ruling so the decision can be made by merging or reverting.
Three things are disclosed rather than glossed: online guessing rises
from 5/min to roughly 27/s, because the 429 is a label on the response
and not a gate in front of the hash; the residual exposure is a loss of
login AVAILABILITY, not latency, and a determined flood still denies
login while it runs, at ~400x the cost and clearing the moment it
stops; and the endpoint should be provisioned for ~400 MB resident, not
the 203 MB of live commitment it itemises.
Independently reviewed four times. Reviewers disproved the suspected
FIFO starvation by measurement, then caught two successive memory
bounds the code did not have — the second by parking waiters and
reading the heap rather than checking the arithmetic.
The 8 KB render cap from #135 left storage untouched but no route served
the rest, so a body over the cap was reachable only with filesystem
access to the SQLite files — in a product whose purpose is storing
webhooks so they can be inspected.
GET /source/{sourceID}/logs/{eventID}/body serves the whole body to the
webhook's owner, as application/octet-stream with an attachment
disposition and nosniff. Those are a security control, not formatting:
the bytes come from the public receiver and are handed back inside the
operator's authenticated origin, and the existing CSP would not stop a
stored HTML payload executing there. The truncation marker links to it
only when a body was actually cut.
Accepted deviation, documented rather than glossed: #157's definition of
done asks the route to stream from the row. It buffers whole instead,
because database/sql exposes no incremental handle on a SQLite BLOB and
substr range reads re-materialise the entire column per call — an
earlier revision chunked at 64 KiB and was 11-15x slower for a worse
bound. Three independent reviewers confirmed no streaming path exists.
Independently reviewed three times. Two earlier revisions each asserted
a memory bound the code did not have; the final reviewer measured
2.057x at the ingest cap and pinned the two overlapping allocations from
source — the driver's column buffer and database/sql's convertAssign
clone — confirming the stated "roughly two bodies, and 2x is a floor
not a ceiling" is now accurate, since SQLite's own materialisation sits
outside the Go heap.
Publishing this README would have shipped false statements about the
product. Corrects the eight items on the issue plus everything a full
sweep turned up: the Slack circuit-breaker scope, a nonexistent WAL, the
wrong config key for slack targets, six undocumented routes, the
conditional /metrics registration, wrong retention bands, wrong shutdown
mechanism, and a Quick Start that led a new contributor into a red
build.
The lockout warning now fires whenever TRUSTED_PROXIES is empty rather
than only in production, since the variable it was gated on defaults to
dev. Rate-limit keying, the limits and the TRUSTED_PROXIES default are
untouched — those belong to #150.
What /s/* actually serves was settled empirically rather than by
reading: all five of GET/HEAD/POST/PUT/DELETE return 200, pinned by
TestStaticServesEveryMethod. Restricting it is filed separately.
Independently reviewed after three prior rounds. The reviewer
re-derived all fifteen claim-table rows against the code, including
every row a previous revision had marked "correct, left alone" and got
wrong, and found zero false; then verified every route method-by-method,
all twelve environment variables, all nine entity tables, and the
package tree against git ls-files. The Quick Start was confirmed by
running it in a fresh clone.
check / check (push) Superseded by a newer commit; never tested
CSRF ran before MaxBodySize, so the CSRF middleware parsed the form body
before any cap applied and an oversized request was read in full before
being rejected. MaxBodySize is now the first middleware in all four route
groups that parse forms, ahead of CSRF and RequireAuth.
An oversize request therefore gets 413 without the handler running and
without state changing, including the password-change route.
Note the ordering trade: an unauthenticated client now receives 413 rather
than an auth redirect on /user/{username}/password.
