From c22ca6218ec6574ecb1c3803e3875161f37b1eb9 Mon Sep 17 00:00:00 2001
From: clawbot <35+clawbot@noreply.example.org>
Date: Fri, 2 Oct 2026 17:09:23 +0200
Subject: [PATCH] Pin the rate-limit key for an empty RemoteAddr (closes #168)

A request whose RemoteAddr is empty has no peer identity, so the rate limiters' key falls back to the raw empty string and every such request shares one bucket: it fails closed rather than giving each its own. net/http always fills RemoteAddr for a TCP listener, so normal serving never reaches this. The behaviour is unchanged and now deliberate: a test pins the shared key, and a one-sentence comment at the fallback tells the empty case apart from a Unix-socket listener, where every peer legitimately carries the same address.

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