From d84e42c6709c43416855a89cb5c305eca95ff81f Mon Sep 17 00:00:00 2001 From: sneak Date: Fri, 2 Oct 2026 14:52:04 +0000 Subject: [PATCH] Pin the rate-limit key for an empty RemoteAddr (closes #168) A request with an empty RemoteAddr falls through to the raw-value fallback and keys on the empty string, so every such request shares one bucket. That is the fail-closed direction and stays as it is. A test now pins it, and the comment at the fallback tells it apart from the Unix-socket case. Model: opus-5-5 --- internal/middleware/ratelimit.go | 6 +++++- internal/middleware/ratelimit_test.go | 17 +++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/internal/middleware/ratelimit.go b/internal/middleware/ratelimit.go index a3ebe85..93020fe 100644 --- a/internal/middleware/ratelimit.go +++ b/internal/middleware/ratelimit.go @@ -219,7 +219,11 @@ func (m *Middleware) clientKey(r *http.Request) string { // 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. + // which is the fail-closed direction. An empty RemoteAddr + // is a different case, which net/http never produces for + // a TCP listener and only a hand-built request carries, + // but it fails closed the same way: every such request + // shares the one bucket keyed on the empty string. return r.RemoteAddr } diff --git a/internal/middleware/ratelimit_test.go b/internal/middleware/ratelimit_test.go index aa3a129..373ca32 100644 --- a/internal/middleware/ratelimit_test.go +++ b/internal/middleware/ratelimit_test.go @@ -1012,6 +1012,23 @@ func TestRateLimitKey_UnparseablePeerKeepsDistinctBuckets( ) } +// TestRateLimitKey_EmptyPeerSharesOneBucket pins what the fallback +// does with an empty RemoteAddr: it keys on the empty string, so every +// such request shares one bucket. That is the fail-closed direction +// and is kept on purpose; only a hand-built request carries an empty +// RemoteAddr. +func TestRateLimitKey_EmptyPeerSharesOneBucket(t *testing.T) { + t.Parallel() + + m := rateLimitMiddleware(t, &config.Config{}) + + assert.Empty( + t, clientKeyFor(t, m, ""), + "every peer with an empty RemoteAddr must key on the "+ + "empty string and so share one bucket", + ) +} + // TestPostRateLimit_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