diff --git a/backend/src/services/__tests__/rateLimitService.test.ts b/backend/src/services/__tests__/rateLimitService.test.ts index 424be723..69dcb82c 100644 --- a/backend/src/services/__tests__/rateLimitService.test.ts +++ b/backend/src/services/__tests__/rateLimitService.test.ts @@ -56,7 +56,7 @@ 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', { @@ -64,12 +64,25 @@ describe('rateLimitService', () => { 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); }); diff --git a/backend/src/services/rateLimitService.ts b/backend/src/services/rateLimitService.ts index b275bd51..62c1fb4e 100644 --- a/backend/src/services/rateLimitService.ts +++ b/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,