fix(backend): rate limiter admits the Nth request itself (off-by-one)

backend/src/services/__tests__/rateLimitService.test.ts

@@ -56,20 +56,33 @@ describe('rateLimitService', () => {
     expect(script).toContain('TTL');
   });
 
-  it('blocks requests once the atomic counter reaches or exceeds the limit', async () => {
+  it('still admits the maxRequests-th request itself (off-by-one guard)', async () => {
     mockEval.mockResolvedValueOnce([5, 60]);
 
     const result = await rateLimitService.checkRateLimit('user123', {
       maxRequests: 5,
       windowSeconds: 60,
     });
 
-    expect(result.allowed).toBe(false);
+    expect(result.allowed).toBe(true);
     expect(result.remaining).toBe(0);
     expect(result.currentCount).toBe(5);
   });
 
-  it('admits requests strictly below maxRequests under concurrent requests', async () => {
+  it('blocks the request past maxRequests', async () => {
+    mockEval.mockResolvedValueOnce([6, 60]);
+
+    const result = await rateLimitService.checkRateLimit('user123', {
+      maxRequests: 5,
+      windowSeconds: 60,
+    });
+
+    expect(result.allowed).toBe(false);
+    expect(result.remaining).toBe(0);
+    expect(result.currentCount).toBe(6);
+  });
+
+  it('admits exactly maxRequests out of maxRequests+1 concurrent requests', async () => {
     let counter = 0;
     mockEval.mockImplementation(async () => {
       counter += 1;
@@ -85,8 +98,8 @@ describe('rateLimitService', () => {
       ),
     );
 
-    expect(results.filter((result) => result.allowed)).toHaveLength(4);
-    expect(results.filter((result) => !result.allowed)).toHaveLength(6);
+    expect(results.filter((result) => result.allowed)).toHaveLength(5);
+    expect(results.filter((result) => !result.allowed)).toHaveLength(5);
     expect(mockEval).toHaveBeenCalledTimes(10);
   });
 

backend/src/services/rateLimitService.ts

@@ -85,7 +85,9 @@ return {currentCount, ttl}
       const resetTime = new Date(
         Date.now() + (ttlSeconds > 0 ? ttlSeconds : config.windowSeconds) * 1000,
       );
-      const allowed = currentCount < config.maxRequests;
+      // The script increments before returning, so `currentCount === maxRequests` is still the
+      // limit'th admitted request. `<=` is what makes "maxRequests per window" honest.
+      const allowed = currentCount <= config.maxRequests;
       const remaining = Math.max(0, config.maxRequests - currentCount);
 
       return {
@@ -166,7 +168,9 @@ return {currentCount, ttl}
         Date.now() + (ttlSeconds > 0 ? ttlSeconds : config.windowSeconds) * 1000,
       );
       const remaining = Math.max(0, config.maxRequests - currentCount);
-      const allowed = currentCount < config.maxRequests;
+      // The script increments before returning, so `currentCount === maxRequests` is still the
+      // limit'th admitted request. `<=` is what makes "maxRequests per window" honest.
+      const allowed = currentCount <= config.maxRequests;
 
       return {
         allowed,