Compare commits

2 Commits

Author SHA1 Message Date
clawbot
6f37d05ab6 Serve an event's full stored body over HTTP (closes #157)
All checks were successful
check / check (push) Successful in 3m23s
Capping the event log page at 8 KB of body per event left no
in-app way to see a larger one: storage keeps it, but no route
served it, so a payload over the cap was reachable only by an
operator with filesystem access. GitHub pull_request and
multi-commit push payloads, expanded Stripe events and Shopify
orders all routinely clear 8 KB, which is exactly when the tool
is supposed to be useful.

GET /source/{sourceID}/logs/{eventID}/body now serves one whole
body, and the truncation marker links to it when — and only
when — a body was actually cut.

The response is deliberately inert. Its bytes are chosen by
whoever can reach the public receiver and it hands them back
inside the operator's own authenticated origin, so it goes out
as application/octet-stream with Content-Disposition: attachment
and nosniff, and the filename is built from a parsed uuid rather
than from anything in the request. The application CSP is no
help on this path: script-src allows 'unsafe-inline' from
'self', so a document served from this origin could run its own
script.

The body is read in one query and held whole while it is
written. There is no cheaper bound to take. database/sql
exposes no incremental handle on a SQLite blob, and reading
byte ranges with substr does not avoid the cost either: SQLite
materialises the entire column value to evaluate each substr
call, so range reads pay for the whole body once per range
rather than once per download. Measured over a 1 MiB body,
64 KiB ranges cost 11-15x a single read to move the same bytes.
The bound is therefore the one the issue asks for: the route is
owner-authenticated and ingest is capped at 1 MB, so peak is
one body per concurrent download. Nothing goes through
renderTemplate, which buffers a whole response before writing
it.

Reading the body before the first header is written also means
an event reaped mid-request cannot produce a torn response: it
is either served whole or 404s cleanly, and both are tested.

The ownership check the log page applies is extracted as
ownedWebhook and shared with the download, so the two cannot
drift apart. A webhook owned by someone else and one that does
not exist are the same 404.

The route registration and the link the template emits are
covered end to end through the production router, so a typo in
either fails the suite rather than leaving the feature dead
behind green handler tests.
2026-08-17 21:54:57 +00:00
c3b6623be1 Bucket IPv6 rate-limit keys by /64 (closes #125)
All checks were successful
check / check (push) Successful in 3m5s
Rate-limit keys were per-address, i.e. per /128 for IPv6. A routed /64
is the normal residential and mobile allocation, so a client rotated
source addresses inside its own prefix and minted a fresh bucket per
request — evading every limiter at the network layer, with no spoofing
and nothing to detect. #88 closed the header half of this control; this
is the network half.

IPv6 now keys on the /64, IPv4 on the full address, via stdlib
net/netip. IPv4-mapped form is unmapped rather than masked, so clients
behind a mapping proxy do not collapse into one bucket.

Independently reviewed twice. The first round found the trusted-proxy
forwarded path — the one carrying production traffic — had no coverage
at all, so a silent revert there was undetectable; that is now pinned.
The reviewer confirmed both branches are independently mutation-tested:
reverting either the direct-peer return or the forwarded return alone
fails only that branch's tests. The 18-site test-constant refactor was
verified byte-identical against next, with no pre-existing assertion
changed.

Known remaining coverage gap, judged not a defect: the fallback when the
peer is trusted but the forwarded address does not parse has no test.
Only operator-controlled addresses inside TRUSTED_PROXIES reach it, they
already share the proxy's single bucket, and masking there can only
merge operator proxies — fail-closed, nothing attacker-controlled.
2026-08-17 23:52:15 +02:00
3 changed files with 463 additions and 20 deletions

View File

@@ -1001,7 +1001,14 @@ Every limiter here — receiver, login, and password change — identifies
the client the same way, through one shared key function: the
connection's own address, unless the peer is listed in
`TRUSTED_PROXIES`, in which case the forwarded client address is used
instead. See [Trusted proxies](#trusted-proxies). Deployed without that
instead. That address becomes a bucket by family: IPv4 keys on the full
address, IPv6 on its `/64` prefix. A routed `/64` is the normal
residential and mobile IPv6 allocation, so keying IPv6 per address would
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). 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

View File

@@ -48,6 +48,12 @@ const (
// bound every request pays a walk proportional to whatever the
// client sent.
maxForwardedHops = 64
// ipv6BucketBits is the prefix length IPv6 clients are bucketed
// on. A routed /64 is the normal residential and mobile
// allocation, so it is the unit an attacker gets addresses in
// and therefore the unit worth limiting.
ipv6BucketBits = 64
)
// normalizeAddr strips the IPv4-in-IPv6 wrapper and any zone from
@@ -56,6 +62,40 @@ func normalizeAddr(addr netip.Addr) netip.Addr {
return addr.Unmap().WithZone("")
}
// bucketKey is the rate-limit bucket identity of a client address.
// IPv4 keys on the full address; IPv6 keys on its /64 prefix,
// because keying IPv6 per /128 lets one ordinary subscriber rotate
// source addresses inside its own routed /64 and mint a fresh bucket
// per request — evading every limiter here at the network layer,
// with no spoofing and nothing to detect.
//
// An IPv4-mapped address (::ffff:1.2.3.4) is keyed as the IPv4
// address it carries, never masked to a /64: mapped form all shares
// the ::ffff:0:0/96 prefix, so masking would collapse every IPv4
// client reaching a proxy that emits it into one bucket. Callers
// pass addresses through normalizeAddr, which already unmaps; the
// unmap here keeps the property true of the key function itself.
//
// The two families cannot collide: an IPv4 key is a bare dotted
// quad, and an IPv6 key always carries a "/64" suffix.
func bucketKey(addr netip.Addr) string {
addr = addr.Unmap()
if addr.Is4() {
return addr.String()
}
// Prefix errors only on a negative bit count, on over 32 bits
// for an IPv4 address, or on over 128 for IPv6. The count here
// is the constant 64 and the IPv4 case returned above, so the
// error is unreachable. (The zero Addr does not error either: it
// yields the zero Prefix. Neither call site can produce one,
// since both parse the address first.)
prefix, _ := addr.Prefix(ipv6BucketBits)
return prefix.String()
}
// 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.
@@ -143,6 +183,9 @@ func (m *Middleware) forwardedClientAddr(
// another client's bucket, by picking an X-Forwarded-For value —
// which makes every limit here decorative against a deliberate
// attacker.
//
// The address that identifies the client is then reduced to a bucket
// by bucketKey: full address for IPv4, /64 prefix for IPv6.
func (m *Middleware) rateLimitKey(r *http.Request) (string, error) {
return m.clientKey(r), nil
}
@@ -152,23 +195,25 @@ func (m *Middleware) clientKey(r *http.Request) string {
peer, err := netip.ParseAddr(ipFromHostPort(r.RemoteAddr))
if err != nil {
// Not an address we can reason about; key on the raw
// value, the most specific identity left. On a
// Unix-socket listener every peer carries the same
// RemoteAddr and so shares one bucket, which is the
// fail-closed direction.
// value, the most specific identity left. Distinct
// RemoteAddr values stay in distinct buckets, so this
// path cannot silently collapse unrelated clients
// together. On a Unix-socket listener every peer
// carries the same RemoteAddr and so shares one bucket,
// which is the fail-closed direction.
return r.RemoteAddr
}
peer = normalizeAddr(peer)
if !m.isTrustedProxy(peer) {
return peer.String()
return bucketKey(peer)
}
if addr, ok := m.forwardedClientAddr(r); ok {
return addr.String()
return bucketKey(addr)
}
return peer.String()
return bucketKey(peer)
}
// tooManyRequests returns the 429 handler used by the login,

View File

@@ -15,6 +15,7 @@ import (
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/webhooker/internal/config"
"sneak.berlin/go/webhooker/internal/middleware"
)
@@ -370,6 +371,30 @@ const (
headerXFF = "X-Forwarded-For"
headerReal = "X-Real-IP"
headerTrue = "True-Client-IP"
// clientIPv4 is the sample IPv4 client address these tests key
// on, both directly and in IPv4-mapped form. clientIPv4Alt is
// its neighbour, used to show the two do not share a bucket.
clientIPv4 = "198.51.100.7"
clientIPv4Alt = "198.51.100.8"
// clientIPv6 and clientIPv6Same are two addresses inside one
// routed /64, so both must key on clientBucketV6.
// clientIPv6Other is a different allocation and must key on
// clientOtherBucketV6.
clientIPv6 = "2001:db8:1:2:3:4:5:6"
clientIPv6Same = "2001:db8:1:2:aaaa:bbbb:cccc:dddd"
clientIPv6Other = "2001:db8:1:3::1"
clientBucketV6 = "2001:db8:1:2::/64"
clientOtherBucketV6 = "2001:db8:1:3::/64"
// trustedProxyCIDR is the proxy network the forwarded-path
// tests configure, and trustedPeer an address inside it. A
// production deployment is required to run behind a reverse
// proxy with TRUSTED_PROXIES set, so this is the shape the
// bucketing has to hold in.
trustedProxyCIDR = "10.0.0.0/8"
trustedPeer = "10.0.0.1:44444"
)
// assertSharedBucket drives the login limiter from peer with the
@@ -458,8 +483,8 @@ func TestRateLimitKey_SingleValuedHeadersIgnoredFromTrustedPeer(
t.Parallel()
assertSharedBucket(
t, trustedProxies("10.0.0.0/8"),
"10.0.0.1:44444",
t, trustedProxies(trustedProxyCIDR),
trustedPeer,
func(i int) map[string]string {
return map[string]string{
header: fmt.Sprintf(
@@ -495,8 +520,8 @@ func TestRateLimitKey_MalformedRightmostHopFallsBackToPeer(
t.Parallel()
assertSharedBucket(
t, trustedProxies("10.0.0.0/8"),
"10.0.0.1:44444",
t, trustedProxies(trustedProxyCIDR),
trustedPeer,
func(i int) map[string]string {
return map[string]string{
headerXFF: fmt.Sprintf(
@@ -522,13 +547,13 @@ func TestRateLimitKey_ForwardedHonouredFromTrustedPeer(
t.Parallel()
m := rateLimitMiddleware(t, &config.Config{
TrustedProxies: trustedProxies("10.0.0.0/8"),
TrustedProxies: trustedProxies(trustedProxyCIDR),
})
handler := m.LoginRateLimit()(okHandler())
const peer = "10.0.0.1:44444"
const peer = trustedPeer
first := map[string]string{headerXFF: "198.51.100.7"}
first := map[string]string{headerXFF: clientIPv4}
for range middleware.LoginRateLimitConst {
postWithHeaders(handler, peer, loginPath, first)
@@ -542,7 +567,7 @@ func TestRateLimitKey_ForwardedHonouredFromTrustedPeer(
w = postWithHeaders(
handler, peer, loginPath,
map[string]string{headerXFF: "198.51.100.8"},
map[string]string{headerXFF: clientIPv4Alt},
)
assert.Equal(
t, http.StatusOK, w.Code,
@@ -559,7 +584,7 @@ func TestRateLimitKey_ChainWalkSkipsClientPrepended(t *testing.T) {
t.Parallel()
assertSharedBucket(
t, trustedProxies("10.0.0.0/8"), "10.0.0.1:44444",
t, trustedProxies(trustedProxyCIDR), trustedPeer,
func(i int) map[string]string {
return map[string]string{
headerXFF: fmt.Sprintf(
@@ -594,7 +619,7 @@ func TestRateLimitKey_LongChainCapsWalkAndFallsBackToPeer(
start := time.Now()
assertSharedBucket(
t, trustedProxies("10.0.0.0/8"), "10.0.0.1:44444",
t, trustedProxies(trustedProxyCIDR), trustedPeer,
func(i int) map[string]string {
return map[string]string{
headerXFF: fmt.Sprintf("9.9.9.%d%s", i+1, padding),
@@ -633,13 +658,13 @@ func TestRateLimitKey_LongChainAllocationIsBounded(t *testing.T) {
)
m := rateLimitMiddleware(t, &config.Config{
TrustedProxies: trustedProxies("10.0.0.0/8"),
TrustedProxies: trustedProxies(trustedProxyCIDR),
})
req := httptest.NewRequestWithContext(
context.Background(), http.MethodPost, loginPath, nil,
)
req.RemoteAddr = "10.0.0.1:44444"
req.RemoteAddr = trustedPeer
req.Header.Set(
headerXFF, "9.9.9.9"+strings.Repeat(", 10.0.0.2", hops),
)
@@ -835,3 +860,369 @@ func TestReceiverRateLimit_IgnoresForwardedFromUntrustedPeer(
"not mint a fresh receiver bucket",
)
}
// clientKeyFor returns the bucket key m computes for a request whose
// direct peer is remoteAddr and which carries no forwarded headers.
func clientKeyFor(
t *testing.T, m *middleware.Middleware, remoteAddr string,
) string {
t.Helper()
req := httptest.NewRequestWithContext(
context.Background(), http.MethodPost, loginPath, nil,
)
req.RemoteAddr = remoteAddr
return middleware.ClientKeyForTest(m, req)
}
// TestRateLimitKey_IPv6BucketsByPrefix pins the key function's
// address-family behaviour. IPv6 clients must bucket by /64 — a
// routed /64 is the normal residential and mobile allocation, so
// per-/128 keying lets one subscriber rotate source addresses and
// mint a fresh bucket per request — while IPv4 keeps keying on the
// full address and IPv4-mapped form is keyed as the IPv4 address it
// carries.
func TestRateLimitKey_IPv6BucketsByPrefix(t *testing.T) {
t.Parallel()
m := rateLimitMiddleware(t, &config.Config{})
for _, tc := range []struct {
name string
peer string
want string
about string
}{{
name: "ipv6",
peer: "[" + clientIPv6 + "]:44444",
want: clientBucketV6,
about: "an IPv6 peer must key on its /64",
}, {
name: "ipv6-other-in-same-64",
peer: "[" + clientIPv6Same + "]:1",
want: clientBucketV6,
about: "another address in the same /64 must key the same",
}, {
name: "ipv6-different-64",
peer: "[" + clientIPv6Other + "]:44444",
want: clientOtherBucketV6,
about: "a different /64 must key differently",
}, {
name: "ipv4",
peer: clientIPv4 + ":44444",
want: clientIPv4,
about: "IPv4 must keep keying on the full address",
}, {
name: "ipv4-neighbour",
peer: clientIPv4Alt + ":44444",
want: clientIPv4Alt,
about: "adjacent IPv4 addresses must not share a bucket",
}, {
name: "ipv4-mapped",
peer: "[::ffff:" + clientIPv4 + "]:44444",
want: clientIPv4,
about: "IPv4-mapped form must key as the IPv4 address, " +
"not be masked to a /64: mapped addresses all share " +
"::ffff:0:0/96, so masking would collapse every IPv4 " +
"client behind a mapping proxy into one bucket",
}} {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
assert.Equal(
t, tc.want, clientKeyFor(t, m, tc.peer), tc.about,
)
})
}
}
// TestRateLimitKey_FamiliesDoNotCollide pins the structure the
// no-collision property rests on, rather than one sample pair: every
// IPv4 key is a bare address and every IPv6 key is a /64 in CIDR
// form, so the two name spaces are disjoint by shape. Dropping the
// masking strips the suffix that guarantees it, which is why this
// asserts the form of each key and not just that two of them differ.
func TestRateLimitKey_FamiliesDoNotCollide(t *testing.T) {
t.Parallel()
// Restated here rather than imported from the package under
// test, so that changing the production bucket width fails this
// test instead of silently moving with it.
const wantBits = 64
m := rateLimitMiddleware(t, &config.Config{})
v4Keys := map[string]bool{}
for _, peer := range []string{
clientIPv4 + ":44444",
clientIPv4Alt + ":44444",
"[::ffff:" + clientIPv4 + "]:44444",
} {
key := clientKeyFor(t, m, peer)
addr, err := netip.ParseAddr(key)
require.NoError(
t, err, "%s: an IPv4 key must be a bare address", peer,
)
assert.True(
t, addr.Is4(),
"%s: an IPv4 key must be a dotted quad, got %q", peer, key,
)
v4Keys[key] = true
}
for _, peer := range []string{
"[" + clientIPv6 + "]:44444",
"[" + clientIPv6Same + "]:44444",
"[" + clientIPv6Other + "]:44444",
"[2001:db8::" + clientIPv4 + "]:44444",
} {
key := clientKeyFor(t, m, peer)
prefix, err := netip.ParsePrefix(key)
require.NoError(
t, err, "%s: an IPv6 key must be a CIDR prefix", peer,
)
assert.Equal(
t, wantBits, prefix.Bits(),
"%s: an IPv6 key must name a /64", peer,
)
assert.False(
t, v4Keys[key],
"%s: an IPv6 key must never equal an IPv4 key", peer,
)
}
}
// TestRateLimitKey_UnparseablePeerKeepsDistinctBuckets covers the
// fallback path. A RemoteAddr that is not an address must not panic,
// and must not drop unrelated clients into one shared bucket by
// accident: the raw value is the most specific identity left, so
// distinct values stay in distinct buckets.
func TestRateLimitKey_UnparseablePeerKeepsDistinctBuckets(
t *testing.T,
) {
t.Parallel()
m := rateLimitMiddleware(t, &config.Config{})
first := clientKeyFor(t, m, "not-an-address")
second := clientKeyFor(t, m, "also-not-an-address:1234")
assert.NotEmpty(t, first)
assert.NotEqual(
t, first, second,
"unparseable peers must not collapse into one bucket",
)
}
// TestLoginRateLimit_IPv6SharesBucketWithinSlash64 is the behavioural
// half, and the regression test for the bypass itself: a client that
// rotates source addresses inside its own routed /64 must stay in one
// bucket. Reverting the masking makes this test fail, because each
// rotated address would mint a fresh bucket and nothing would be
// rejected.
func TestLoginRateLimit_IPv6SharesBucketWithinSlash64(t *testing.T) {
t.Parallel()
m := rateLimitMiddleware(t, &config.Config{})
handler := m.LoginRateLimit()(okHandler())
for i := range middleware.LoginRateLimitConst {
w := postWithHeaders(
handler,
fmt.Sprintf("[2001:db8:1:2::%d]:44444", i+1),
loginPath, nil,
)
assert.Equal(
t, http.StatusOK, w.Code, "request %d should pass", i,
)
}
w := postWithHeaders(
handler, "[2001:db8:1:2::ffff]:44444", loginPath, nil,
)
assert.Equal(
t, http.StatusTooManyRequests, w.Code,
"rotating source addresses inside one routed /64 must not "+
"mint fresh buckets",
)
}
// TestLoginRateLimit_IPv6IndependentAcrossSlash64 is the other side
// of the trade: bucketing by /64 must not merge separate allocations,
// so a client in a different /64 keeps its own limit.
func TestLoginRateLimit_IPv6IndependentAcrossSlash64(t *testing.T) {
t.Parallel()
m := rateLimitMiddleware(t, &config.Config{})
handler := m.LoginRateLimit()(okHandler())
for range middleware.LoginRateLimitConst + 1 {
postWithHeaders(
handler, "[2001:db8:1:2::1]:44444", loginPath, nil,
)
}
w := postWithHeaders(
handler, "[2001:db8:1:3::1]:44444", loginPath, nil,
)
assert.Equal(
t, http.StatusOK, w.Code,
"a different /64 must have its own bucket",
)
}
// TestLoginRateLimit_IPv4IndependentPerAddress guards against the
// masking leaking into IPv4: two addresses one apart must still hold
// separate buckets.
func TestLoginRateLimit_IPv4IndependentPerAddress(t *testing.T) {
t.Parallel()
m := rateLimitMiddleware(t, &config.Config{})
handler := m.LoginRateLimit()(okHandler())
for range middleware.LoginRateLimitConst + 1 {
postWithHeaders(
handler, clientIPv4+":44444", loginPath, nil,
)
}
w := postWithHeaders(
handler, clientIPv4Alt+":44444", loginPath, nil,
)
assert.Equal(
t, http.StatusOK, w.Code,
"a second IPv4 address must have its own bucket",
)
}
// forwardedKeyFor returns the bucket key m computes for a request
// that arrives from trustedPeer — a configured trusted proxy — and
// names forwarded as its client in X-Forwarded-For. That is the
// production path: a deployment is required to run behind a reverse
// proxy with TRUSTED_PROXIES set, so the forwarded address, not the
// peer, is what the limiters bucket on there.
func forwardedKeyFor(
t *testing.T, m *middleware.Middleware, forwarded string,
) string {
t.Helper()
req := httptest.NewRequestWithContext(
context.Background(), http.MethodPost, loginPath, nil,
)
req.RemoteAddr = trustedPeer
req.Header.Set(headerXFF, forwarded)
return middleware.ClientKeyForTest(m, req)
}
// TestRateLimitKey_ForwardedIPv6BucketsByPrefix pins the /64
// bucketing on the trusted-proxy branch. The direct-peer tests above
// cannot reach it, so without this the masking could be reverted for
// forwarded clients alone — the only shape a production deployment
// runs in — and the rest of the suite would stay green.
func TestRateLimitKey_ForwardedIPv6BucketsByPrefix(t *testing.T) {
t.Parallel()
m := rateLimitMiddleware(t, &config.Config{
TrustedProxies: trustedProxies(trustedProxyCIDR),
})
for _, tc := range []struct {
name string
forwarded string
want string
about string
}{{
name: "ipv6",
forwarded: clientIPv6,
want: clientBucketV6,
about: "a forwarded IPv6 client must key on its /64",
}, {
name: "ipv6-other-in-same-64",
forwarded: clientIPv6Same,
want: clientBucketV6,
about: "another forwarded address in the same /64 must " +
"key the same",
}, {
name: "ipv6-different-64",
forwarded: clientIPv6Other,
want: clientOtherBucketV6,
about: "a forwarded address in another /64 must differ",
}, {
name: "ipv4",
forwarded: clientIPv4,
want: clientIPv4,
about: "a forwarded IPv4 client must key on the address",
}, {
name: "ipv4-mapped",
forwarded: "::ffff:" + clientIPv4,
want: clientIPv4,
about: "a proxy that forwards IPv4-mapped form must key as " +
"the IPv4 address it carries, not be masked to a /64: " +
"mapped addresses all share ::ffff:0:0/96",
}} {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
assert.Equal(
t, tc.want,
forwardedKeyFor(t, m, tc.forwarded), tc.about,
)
})
}
}
// TestLoginRateLimit_ForwardedIPv6SharesBucketWithinSlash64 is the
// behavioural half on the production path: behind a trusted proxy, a
// client rotating source addresses inside its own routed /64 must
// stay in one bucket.
func TestLoginRateLimit_ForwardedIPv6SharesBucketWithinSlash64(
t *testing.T,
) {
t.Parallel()
assertSharedBucket(
t, trustedProxies(trustedProxyCIDR), trustedPeer,
func(i int) map[string]string {
return map[string]string{
headerXFF: fmt.Sprintf("2001:db8:1:2::%d", i+1),
}
},
"rotating forwarded source addresses inside one routed /64 "+
"must not mint fresh buckets",
)
}
// TestLoginRateLimit_ForwardedIPv6IndependentAcrossSlash64 is the
// other side of that trade on the same path: bucketing by /64 must
// not merge two allocations reaching the proxy.
func TestLoginRateLimit_ForwardedIPv6IndependentAcrossSlash64(
t *testing.T,
) {
t.Parallel()
m := rateLimitMiddleware(t, &config.Config{
TrustedProxies: trustedProxies(trustedProxyCIDR),
})
handler := m.LoginRateLimit()(okHandler())
spent := map[string]string{headerXFF: clientIPv6}
for range middleware.LoginRateLimitConst + 1 {
postWithHeaders(handler, trustedPeer, loginPath, spent)
}
w := postWithHeaders(
handler, trustedPeer, loginPath,
map[string]string{headerXFF: clientIPv6Other},
)
assert.Equal(
t, http.StatusOK, w.Code,
"a forwarded client in a different /64 must have its own "+
"bucket",
)
}