🛡️ Sentinel: Dual Rate Limiting on Auth Server Actions

.jules/sentinel.md

@@ -4,3 +4,8 @@
 **Vulnerability:** The application used `scryptSync` (synchronous CPU-intensive password hashing) inside Next.js server action handlers for registration and login. Because Node.js runs on a single main event loop, a small number of concurrent authentication requests (or a distributed credential stuffing attack) completely blocks the event loop, starving all other concurrent requests and causing a full Denial of Service (DoS).
 **Learning:** Next.js Server Actions and Route Handlers run on Node's main thread by default. Using synchronous cryptography operations (such as `scryptSync` or `pbkdf2Sync`) prevents the server from processing other concurrent connections.
 **Prevention:** Always use asynchronous password-hashing implementations (such as async `scrypt` wrapped in a Promise or bcrypt/argon2 async variants) inside Next.js/Node.js web entry points to delegate heavy hashing computations to the Node.js libuv thread pool, keeping the main event loop responsive.
+
+## 2026-07-16 - Rate-Limit Bypass and Account Lockout via Target Key Pollution
+**Vulnerability:** Sensitive authentication endpoints used standard, single-key rate-limiting bound only to user emails. A malicious actor could block legitimate users from signing in or signing up by flooding the rate limiter with target emails from a single IP, causing a widespread Account Lockout Denial of Service (DoS).
+**Learning:** Checking or incrementing target-based rate limits before checking client IP rate limits allows IP-blocked attackers to pollute target-based rate limiting buckets and lockout innocent users.
+**Prevention:** Always use a dual rate-limiting approach that checks and blocks malicious client IPs first before incrementing or checking target-based keys (such as email). Ensure the IP extraction safely fallback and handle array indexes, and that all calling utilities are asynchronous to support Next.js 15 async headers.

package.json

@@ -72,5 +72,5 @@
       "eslint --fix"
     ]
   },
-  "packageManager": "pnpm@11.13.0+sha512.88d94724d8f2e6c186744a5584c6e59ecac869ec7ba15e9cb4cd628e8dc7066820b2481d8ee3b51ea8da323a7378068aa58c556a3720d32b7c20a051d088363a"
+  "packageManager": "pnpm@11.15.0+sha512.266f8957a30d2be6e9468e5e66bcdedd35a794175f71b067ba8504d686cce1d0c0f429b33c323c3c569ad4891e667574a49ff71d1b89a22cc66f13c65818c578"
 }

src/app/actions/__tests__/auth-actions.test.ts

@@ -1,6 +1,7 @@
 import { describe, it, expect, vi, beforeEach } from 'vitest';
 import { signInAction, signUpAction, signOutAction } from '@/app/actions/auth';
 import { PLACEHOLDER_PASSWORD_PREFIX } from '@/lib/claim-token';
+import { resetRateLimits } from '@/lib/rate-limit';
 
 const mockSignToken = vi.fn();
 const mockSetAuthCookie = vi.fn();
@@ -29,6 +30,7 @@ vi.mock('next/headers', () => ({
     set: vi.fn(),
     delete: vi.fn(),
   }),
+  headers: () => Promise.resolve({ get: () => null }),
 }));
 
 vi.mock('next/navigation', () => ({
@@ -41,6 +43,7 @@ describe('auth actions', () => {
   beforeEach(() => {
     vi.resetAllMocks();
     mockSignToken.mockResolvedValue('token');
+    resetRateLimits();
   });
 
   it('signInAction rejects unknown email', async () => {

src/app/actions/auth.ts

@@ -15,7 +15,7 @@ import {
   getSession,
 } from '@/lib/auth';
 import { logger } from '@/lib/logger';
-import { rateLimit } from '@/lib/rate-limit';
+import { rateLimitDual } from '@/lib/rate-limit';
 import {
   hashClaimToken,
   PLACEHOLDER_PASSWORD_PREFIX,
@@ -32,7 +32,7 @@ import {
 // ---------------------------------------------------------------------------
 
 export const signUpAction = createSafeAction(signUpSchema, async (data) => {
-  const rl = rateLimit(`signup:${data.email.toLowerCase()}`, 5, 60_000);
+  const rl = await rateLimitDual(data.email, 5, 60_000);
   if (!rl.allowed) {
     throw new Error(`Too many attempts. Try again in ${rl.retryAfterSeconds}s.`);
   }
@@ -123,9 +123,9 @@ export const signUpAction = createSafeAction(signUpSchema, async (data) => {
 // ---------------------------------------------------------------------------
 
 export const signInAction = createSafeAction(signInSchema, async (data) => {
-  // Rate-limit BEFORE any DB or scrypt work — the sync scrypt verify is
-  // exactly what an attacker would use to burn the event loop.
-  const rl = rateLimit(`signin:${data.email.toLowerCase()}`, 5, 60_000);
+  // Rate-limit BEFORE any DB or scrypt work to prevent event-loop abuse.
+  // Using dual rate limiting checks the client IP before target-based email.
+  const rl = await rateLimitDual(data.email, 5, 60_000);
   if (!rl.allowed) {
     throw new Error(`Too many attempts. Try again in ${rl.retryAfterSeconds}s.`);
   }

src/lib/__tests__/rate-limit.test.ts

@@ -0,0 +1,152 @@
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+import { rateLimit, rateLimitDual, resetRateLimits } from '../rate-limit';
+
+const { mockHeaders } = vi.hoisted(() => ({
+  mockHeaders: {
+    get: vi.fn(),
+  },
+}));
+
+vi.mock('next/headers', () => ({
+  headers: vi.fn(() => Promise.resolve(mockHeaders)),
+}));
+
+describe('rate-limit.ts', () => {
+  beforeEach(() => {
+    vi.useFakeTimers();
+    vi.clearAllMocks();
+    resetRateLimits();
+  });
+
+  afterEach(() => {
+    vi.useRealTimers();
+  });
+
+  describe('rateLimit', () => {
+    it('allows requests within limit and blocks when exceeded', () => {
+      // Limit = 3, window = 60s
+      expect(rateLimit('user-1', 3, 60_000).allowed).toBe(true);
+      expect(rateLimit('user-1', 3, 60_000).allowed).toBe(true);
+      expect(rateLimit('user-1', 3, 60_000).allowed).toBe(true);
+
+      const block = rateLimit('user-1', 3, 60_000);
+      expect(block.allowed).toBe(false);
+      expect(block.retryAfterSeconds).toBe(60);
+    });
+
+    it('re-allows requests after the sliding window expires', () => {
+      expect(rateLimit('user-2', 2, 60_000).allowed).toBe(true);
+
+      // Advance time by 30 seconds
+      vi.advanceTimersByTime(30_000);
+      expect(rateLimit('user-2', 2, 60_000).allowed).toBe(true);
+      expect(rateLimit('user-2', 2, 60_000).allowed).toBe(false);
+
+      // Advance time by another 31 seconds (total 61s from first hit)
+      vi.advanceTimersByTime(31_000);
+      // The first hit has expired, so we should be allowed again
+      expect(rateLimit('user-2', 2, 60_000).allowed).toBe(true);
+    });
+
+    it('performs opportunistic cleanup of old buckets', () => {
+      // Populate 10,001 keys
+      for (let i = 0; i < 10005; i++) {
+        rateLimit(`key-${i}`, 5, 60_000);
+      }
+      // Advance past window to expire all hits
+      vi.advanceTimersByTime(61_000);
+      // The next hit should trigger cleanup
+      rateLimit('new-trigger', 5, 60_000);
+    });
+  });
+
+  describe('rateLimitDual', () => {
+    it('allows requests when both IP and target are under limits', async () => {
+      mockHeaders.get.mockImplementation((name: string) => {
+        if (name === 'x-forwarded-for') return '1.2.3.4';
+        return null;
+      });
+
+      const res = await rateLimitDual('test@example.com', 2, 60_000, 3);
+      expect(res.allowed).toBe(true);
+    });
+
+    it('correctly handles case-insensitivity of targets', async () => {
+      mockHeaders.get.mockImplementation((name: string) => {
+        if (name === 'x-forwarded-for') return '1.2.3.4';
+        return null;
+      });
+
+      expect((await rateLimitDual('TEST@example.com', 2, 60_000, 5)).allowed).toBe(true);
+      expect((await rateLimitDual('test@EXAMPLE.com', 2, 60_000, 5)).allowed).toBe(true);
+      expect((await rateLimitDual('Test@Example.Com', 2, 60_000, 5)).allowed).toBe(false);
+    });
+
+    it('extracts client IP correctly from comma-separated x-forwarded-for', async () => {
+      mockHeaders.get.mockImplementation((name: string) => {
+        if (name === 'x-forwarded-for') return '2.3.4.5, 8.8.8.8';
+        return null;
+      });
+
+      // We should hit IP limit for 2.3.4.5 after 2 hits
+      expect((await rateLimitDual('a@b.com', 5, 60_000, 2)).allowed).toBe(true);
+      expect((await rateLimitDual('b@b.com', 5, 60_000, 2)).allowed).toBe(true);
+
+      const blocked = await rateLimitDual('c@b.com', 5, 60_000, 2);
+      expect(blocked.allowed).toBe(false);
+    });
+
+    it('falls back to x-real-ip if x-forwarded-for is missing', async () => {
+      mockHeaders.get.mockImplementation((name: string) => {
+        if (name === 'x-real-ip') return '3.4.5.6';
+        return null;
+      });
+
+      expect((await rateLimitDual('a@b.com', 5, 60_000, 1)).allowed).toBe(true);
+      expect((await rateLimitDual('b@b.com', 5, 60_000, 1)).allowed).toBe(false);
+    });
+
+    it('falls back to 127.0.0.1 if both headers are missing', async () => {
+      mockHeaders.get.mockReturnValue(null);
+
+      expect((await rateLimitDual('a@b.com', 5, 60_000, 1)).allowed).toBe(true);
+      expect((await rateLimitDual('b@b.com', 5, 60_000, 1)).allowed).toBe(false);
+    });
+
+    it('blocks IP first and prevents target bucket pollution (Account Lockout DoS protection)', async () => {
+      // Scenario: Attacker at IP 9.9.9.9 tries to brute force/block 'victim@example.com'
+      // The IP limit is 2, the target limit is 5.
+      mockHeaders.get.mockImplementation((name: string) => {
+        if (name === 'x-forwarded-for') return '9.9.9.9';
+        return null;
+      });
+
+      // 1. IP is within limit (1st hit, allowed)
+      expect((await rateLimitDual('victim@example.com', 5, 60_000, 2)).allowed).toBe(true);
+
+      // 2. IP is within limit (2nd hit, allowed)
+      expect((await rateLimitDual('victim@example.com', 5, 60_000, 2)).allowed).toBe(true);
+
+      // 3. IP exceeds limit (3rd hit from 9.9.9.9, blocked on IP)
+      const res3 = await rateLimitDual('victim@example.com', 5, 60_000, 2);
+      expect(res3.allowed).toBe(false);
+
+      // 4. Attacker tries a different target from same blocked IP 9.9.9.9
+      const res4 = await rateLimitDual('other@example.com', 5, 60_000, 2);
+      expect(res4.allowed).toBe(false); // blocked because IP is blocked!
+
+      // 5. The key security claim: has 'other@example.com' or 'victim@example.com' been polluted?
+      // Change IP to legitimate user's IP (e.g., 7.7.7.7)
+      mockHeaders.get.mockImplementation((name: string) => {
+        if (name === 'x-forwarded-for') return '7.7.7.7';
+        return null;
+      });
+
+      // Legitimate user from 7.7.7.7 should still be able to request 'victim@example.com'
+      // because target 'victim@example.com' has only 2 actual hits (not blocked, limit is 5)
+      // and 'other@example.com' should have 0 hits (not polluted by attacker's attempt on step 4).
+      expect((await rateLimitDual('victim@example.com', 5, 60_000, 2)).allowed).toBe(true);
+      expect((await rateLimitDual('other@example.com', 5, 60_000, 2)).allowed).toBe(true);
+    });
+  });
+});

src/lib/rate-limit.ts

@@ -8,6 +8,7 @@
  */
 
 import 'server-only';
+import { headers } from 'next/headers';
 
 const buckets = new Map<string, number[]>();
 
@@ -47,3 +48,34 @@ export function rateLimit(key: string, limit = 5, windowMs = 60_000): RateLimitR
 
   return { allowed: true, retryAfterSeconds: 0 };
 }
+
+/**
+ * Dual rate limiting that checks the client IP first before checking the target-based identifier.
+ * This prevents Account Lockout DoS attacks where an IP-blocked attacker pollutes the target bucket.
+ */
+export async function rateLimitDual(
+  targetKey: string,
+  limit = 5,
+  windowMs = 60_000,
+  ipLimit = 20,
+): Promise<RateLimitResult> {
+  const headersList = await headers();
+  const xff = headersList.get('x-forwarded-for');
+  const ip = (xff ? xff.split(',')[0]?.trim() : null) || headersList.get('x-real-ip') || '127.0.0.1';
+
+  // 1. IP check first to prevent Account Lockout DoS
+  const ipResult = rateLimit(`ip:${ip}`, ipLimit, windowMs);
+  if (!ipResult.allowed) {
+    return ipResult;
+  }
+
+  // 2. Target check second
+  return rateLimit(`target:${targetKey.toLowerCase()}`, limit, windowMs);
+}
+
+/**
+ * Helper to reset in-memory rate limits between tests.
+ */
+export function resetRateLimits(): void {
+  buckets.clear();
+}