🐛 Count rejected requests in Redis sliding window

Mirror the in-memory fix from #79: the Lua script now ZADDs the
current request before checking limits and uses '>' instead of '>='
for the comparison. This way a sender who keeps hammering the policy
server while over-limit extends their own blocking window instead of
getting a free reset once the oldest timestamps fall out.

Update the hourly/daily limit tests to expect count=4 on the rejected
4th attempt, and replace the old "rejected does not increment" test
with the equivalent of the in-memory test that asserts rejected
attempts keep accumulating.

Co-Authored-By: Claude <claude@anthropic.com>

Author: 0x46616c6b

ratelimit.go

@@ -20,7 +20,13 @@ const rateLimitKeyPrefix = "userli:ratelimit:sender:"
 
 // rateLimitScript implements the sliding-window check atomically.
 // ARGV: hourLimit, dayLimit, now (unix nano), hourAgo, dayAgo, member suffix, TTL seconds.
-// Returns {allowed (1/0), hourCount, dayCount}. The new entry is only added when allowed.
+// Returns {allowed (1/0), hourCount, dayCount}.
+//
+// The current request is always recorded — even when it gets rejected — so a
+// sender who keeps trying while over-limit extends their own blocking window
+// instead of getting a free reset once old timestamps expire. The reported
+// counts therefore include the just-recorded attempt; the limit comparison
+// uses '>' rather than '>='.
 const rateLimitScript = `
 local key = KEYS[1]
 local hour_limit = tonumber(ARGV[1])
@@ -33,20 +39,20 @@ local ttl = tonumber(ARGV[7])
 
 redis.call("ZREMRANGEBYSCORE", key, "-inf", "(" .. day_ago)
 
+redis.call("ZADD", key, now, now .. ":" .. suffix)
+redis.call("EXPIRE", key, ttl)
+
 local day_count = tonumber(redis.call("ZCARD", key))
 local hour_count = tonumber(redis.call("ZCOUNT", key, "(" .. hour_ago, "+inf"))
 
-if hour_limit > 0 and hour_count >= hour_limit then
+if hour_limit > 0 and hour_count > hour_limit then
     return {0, hour_count, day_count}
 end
-if day_limit > 0 and day_count >= day_limit then
+if day_limit > 0 and day_count > day_limit then
     return {0, hour_count, day_count}
 end
 
-redis.call("ZADD", key, now, now .. ":" .. suffix)
-redis.call("EXPIRE", key, ttl)
-
-return {1, hour_count + 1, day_count + 1}
+return {1, hour_count, day_count}
 `
 
 // RateLimiter enforces per-sender sliding-window quotas using Redis as backing store.

ratelimit_test.go

@@ -122,12 +122,13 @@ func TestRateLimiter_CheckAndIncrement_HourlyLimit(t *testing.T) {
 		}
 	}
 
+	// 4th message should be rejected but still counted
 	allowed, hourCount, _ := rl.CheckAndIncrement(ctx, sender, quota)
 	if allowed {
 		t.Error("4th message should be rejected due to hourly limit")
 	}
-	if hourCount != 3 {
-		t.Errorf("Expected hourCount to be 3, got %d", hourCount)
+	if hourCount != 4 {
+		t.Errorf("Expected hourCount to be 4, got %d", hourCount)
 	}
 }
 
@@ -145,12 +146,13 @@ func TestRateLimiter_CheckAndIncrement_DailyLimit(t *testing.T) {
 		}
 	}
 
+	// 4th message should be rejected but still counted
 	allowed, _, dayCount := rl.CheckAndIncrement(ctx, sender, quota)
 	if allowed {
 		t.Error("4th message should be rejected due to daily limit")
 	}
-	if dayCount != 3 {
-		t.Errorf("Expected dayCount to be 3, got %d", dayCount)
+	if dayCount != 4 {
+		t.Errorf("Expected dayCount to be 4, got %d", dayCount)
 	}
 }
 
@@ -228,29 +230,39 @@ func TestRateLimiter_KeyTTL(t *testing.T) {
 	}
 }
 
-// TestRateLimiter_RejectedDoesNotIncrement guards the conditional-ZADD invariant
-// in the Lua script: a rejected message must not push the counter above the limit.
-func TestRateLimiter_RejectedDoesNotIncrement(t *testing.T) {
+// TestRateLimiter_CheckAndIncrement_RejectedRequestsExtendWindow ensures
+// rejected attempts are still recorded so a sender who keeps trying while
+// over-limit extends their own blocking window instead of getting a free
+// reset once old timestamps expire.
+func TestRateLimiter_CheckAndIncrement_RejectedRequestsExtendWindow(t *testing.T) {
 	rl, _ := newTestRateLimiter(t)
 	ctx := context.Background()
 
 	quota := &Quota{PerHour: 2, PerDay: 100}
-	sender := "reject@example.org"
+	sender := "spammer@example.org"
 
+	// Use up the hourly quota.
 	for i := 0; i < 2; i++ {
-		rl.CheckAndIncrement(ctx, sender, quota)
+		allowed, _, _ := rl.CheckAndIncrement(ctx, sender, quota)
+		if !allowed {
+			t.Errorf("Message %d should be allowed", i+1)
+		}
 	}
 
+	// Continued spam attempts must be rejected and still counted.
 	for i := 0; i < 5; i++ {
 		allowed, _, _ := rl.CheckAndIncrement(ctx, sender, quota)
 		if allowed {
-			t.Errorf("Attempt %d above limit should be rejected", i+1)
+			t.Errorf("Spam attempt %d should be rejected", i+1)
 		}
 	}
 
-	hourCount, _ := rl.GetCounts(ctx, sender)
-	if hourCount != 2 {
-		t.Errorf("Expected hourCount to stay at 2 after rejected attempts, got %d", hourCount)
+	hourCount, dayCount := rl.GetCounts(ctx, sender)
+	if hourCount != 7 {
+		t.Errorf("Expected hourCount to include rejected attempts (7), got %d", hourCount)
+	}
+	if dayCount != 7 {
+		t.Errorf("Expected dayCount to include rejected attempts (7), got %d", dayCount)
 	}
 }