🛡️ Sentinel: Implement dual-key rate limiting to prevent credential stuffing and account lockout DoS

.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-20 - Dual-Key Rate Limiting Prevents Target Lockout and Credential Stuffing DoS
+**Vulnerability:** The application used single-key in-memory rate limiting based solely on target emails for sign-in and sign-up. This allowed an attacker to lock out any arbitrary user's account from sign-in by triggering 5 failed attempts from any IP. Furthermore, it allowed an attacker to perform high-volume credential stuffing attacks across thousands of different emails from a single IP without hitting the single-email rate limits.
+**Learning:** Single-key rate limiters targeting specific credentials create a Denial of Service / account lockout vector for legitimate users. To defend against distributed credential stuffing and account locking, dual-key rate limiting (combining IP-based and target-based keys) must be used on sensitive endpoints.
+**Prevention:** Always implement dual-key rate limiting on authentication and sensitive server actions, limiting both on the target email (to prevent single-user brute forcing) and the client IP (to block high-frequency multi-target credential stuffing).

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,12 @@ import {
 // ---------------------------------------------------------------------------
 
 export const signUpAction = createSafeAction(signUpSchema, async (data) => {
-  const rl = rateLimit(`signup:${data.email.toLowerCase()}`, 5, 60_000);
+  const rl = await rateLimitDual('signup', data.email.toLowerCase(), {
+    limitIdentifier: 5,
+    windowIdentifierMs: 60_000,
+    limitIp: 20,
+    windowIpMs: 60_000,
+  });
   if (!rl.allowed) {
     throw new Error(`Too many attempts. Try again in ${rl.retryAfterSeconds}s.`);
   }
@@ -125,7 +130,12 @@ 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);
+  const rl = await rateLimitDual('signin', data.email.toLowerCase(), {
+    limitIdentifier: 5,
+    windowIdentifierMs: 60_000,
+    limitIp: 20,
+    windowIpMs: 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,132 @@
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+import { rateLimit, rateLimitDual } from '../rate-limit';
+import { headers } from 'next/headers';
+
+vi.mock('next/headers', () => ({
+  headers: vi.fn(),
+}));
+
+describe('rate-limit.ts', () => {
+  beforeEach(() => {
+    vi.useFakeTimers();
+    vi.clearAllMocks();
+  });
+
+  afterEach(() => {
+    vi.useRealTimers();
+  });
+
+  describe('rateLimit', () => {
+    it('allows requests within limit and then denies them', () => {
+      const key = 'test-key-1';
+      // Limit = 3, window = 60s
+      expect(rateLimit(key, 3, 60_000).allowed).toBe(true);
+      expect(rateLimit(key, 3, 60_000).allowed).toBe(true);
+      expect(rateLimit(key, 3, 60_000).allowed).toBe(true);
+
+      const denied = rateLimit(key, 3, 60_000);
+      expect(denied.allowed).toBe(false);
+      expect(denied.retryAfterSeconds).toBe(60);
+    });
+
+    it('recovers after window expires', () => {
+      const key = 'test-key-2';
+      expect(rateLimit(key, 1, 60_000).allowed).toBe(true);
+      expect(rateLimit(key, 1, 60_000).allowed).toBe(false);
+
+      // Advance time by 61 seconds
+      vi.advanceTimersByTime(61_000);
+
+      expect(rateLimit(key, 1, 60_000).allowed).toBe(true);
+    });
+  });
+
+  describe('rateLimitDual', () => {
+    it('limits by identifier', async () => {
+      (headers as unknown as ReturnType<typeof vi.fn>).mockResolvedValue({
+        get: (h: string) => {
+          if (h === 'x-forwarded-for') return '1.2.3.4';
+          return null;
+        },
+      });
+
+      const action = 'test-action-1';
+      const email = 'user1@example.com';
+
+      // Identifier limit = 2
+      const res1 = await rateLimitDual(action, email, {
+        limitIdentifier: 2,
+        windowIdentifierMs: 60_000,
+        limitIp: 10,
+        windowIpMs: 60_000,
+      });
+      expect(res1.allowed).toBe(true);
+
+      const res2 = await rateLimitDual(action, email, {
+        limitIdentifier: 2,
+        windowIdentifierMs: 60_000,
+        limitIp: 10,
+        windowIpMs: 60_000,
+      });
+      expect(res2.allowed).toBe(true);
+
+      // Third attempt for same email should be blocked
+      const res3 = await rateLimitDual(action, email, {
+        limitIdentifier: 2,
+        windowIdentifierMs: 60_000,
+        limitIp: 10,
+        windowIpMs: 60_000,
+      });
+      expect(res3.allowed).toBe(false);
+    });
+
+    it('limits by IP even with different identifiers', async () => {
+      (headers as unknown as ReturnType<typeof vi.fn>).mockResolvedValue({
+        get: (h: string) => {
+          if (h === 'x-forwarded-for') return '9.9.9.9';
+          return null;
+        },
+      });
+
+      const action = 'test-action-2';
+
+      // IP limit = 2, Identifier limit = 2
+      // Use different identifiers, but same IP
+      const res1 = await rateLimitDual(action, 'id1@test.com', {
+        limitIdentifier: 2,
+        limitIp: 2,
+      });
+      expect(res1.allowed).toBe(true);
+
+      const res2 = await rateLimitDual(action, 'id2@test.com', {
+        limitIdentifier: 2,
+        limitIp: 2,
+      });
+      expect(res2.allowed).toBe(true);
+
+      // Third attempt with same IP but different identifier should be blocked because of IP limit
+      const res3 = await rateLimitDual(action, 'id3@test.com', {
+        limitIdentifier: 2,
+        limitIp: 2,
+      });
+      expect(res3.allowed).toBe(false);
+    });
+
+    it('falls back to x-real-ip or unknown when x-forwarded-for is missing', async () => {
+      (headers as unknown as ReturnType<typeof vi.fn>).mockResolvedValue({
+        get: (h: string) => {
+          if (h === 'x-real-ip') return '8.8.8.8';
+          return null;
+        },
+      });
+
+      const action = 'test-action-3';
+
+      const res = await rateLimitDual(action, 'user@test.com', {
+        limitIdentifier: 1,
+        limitIp: 1,
+      });
+      expect(res.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,46 @@ export function rateLimit(key: string, limit = 5, windowMs = 60_000): RateLimitR
 
   return { allowed: true, retryAfterSeconds: 0 };
 }
+
+/**
+ * Dual rate limiter: limits by both target identifier (e.g. email) and IP address.
+ * Standard defense-in-depth against credential stuffing and brute force attacks.
+ */
+export async function rateLimitDual(
+  action: string,
+  identifier: string,
+  options?: {
+    limitIdentifier?: number;
+    windowIdentifierMs?: number;
+    limitIp?: number;
+    windowIpMs?: number;
+  },
+): Promise<RateLimitResult> {
+  const limitId = options?.limitIdentifier ?? 5;
+  const windowId = options?.windowIdentifierMs ?? 60_000;
+  const limitIp = options?.limitIp ?? 20;
+  const windowIp = options?.windowIpMs ?? 60_000;
+
+  // 1. Check identifier limit first (e.g., signup:email or signin:email)
+  const idRl = rateLimit(`${action}:${identifier}`, limitId, windowId);
+  if (!idRl.allowed) {
+    return idRl;
+  }
+
+  // 2. Check IP limit
+  let ip = 'unknown';
+  try {
+    const headersList = await headers();
+    const xff = headersList.get('x-forwarded-for');
+    ip = (xff ? xff.split(',')[0]?.trim() : null) || headersList.get('x-real-ip') || 'unknown';
+  } catch {
+    // Fail safe if headers() fails (e.g. outside request context in tests)
+  }
+
+  const ipRl = rateLimit(`ip:${ip}:${action}`, limitIp, windowIp);
+  if (!ipRl.allowed) {
+    return ipRl;
+  }
+
+  return { allowed: true, retryAfterSeconds: 0 };
+}