🛡️ Sentinel: Dual Rate Limiting on Auth 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-08-13 - Target-Based Rate Limiting Account Lockout DoS
+**Vulnerability:** The application originally rate-limited sign-in and sign-up server actions strictly based on target email identifier prefixes. While this controls credential stuffing per account, a malicious IP-blocked attacker can continually target a specific target email, polluting target-based lockout buckets and causing a localized Account Lockout Denial of Service (DoS) for legitimate users.
+**Learning:** Target-based rate limits alone are vulnerable to targeted lockout abuse. In contrast, checking client IP rate limits first allows blocking the attacker entirely at the network layer without affecting or polluting the target-based bucket of the user.
+**Prevention:** Always use a dual rate-limiting approach. Always execute the client IP check and rate limiting before checking target-based identifiers (such as emails) to stop malicious actors before they pollute target buckets.

package.json

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

src/__tests__/setup.ts

@@ -8,6 +8,9 @@ vi.mock('next/headers', () => ({
     set: vi.fn(),
     delete: vi.fn(),
   }),
+  headers: () => Promise.resolve({
+    get: () => null,
+  }),
 }));
 
 vi.mock('next/navigation', () => ({

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

@@ -29,18 +29,23 @@ vi.mock('next/headers', () => ({
     set: vi.fn(),
     delete: vi.fn(),
   }),
+  headers: () => Promise.resolve({
+    get: () => null,
+  }),
 }));
 
 vi.mock('next/navigation', () => ({
   redirect: vi.fn(),
 }));
 
 import { db } from '@/lib/db';
+import { resetRateLimits } from '@/lib/rate-limit';
 
 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(`signup:${data.email.toLowerCase()}`, 5, 60_000);
   if (!rl.allowed) {
     throw new Error(`Too many attempts. Try again in ${rl.retryAfterSeconds}s.`);
   }
@@ -125,7 +125,7 @@ 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()}`, 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,83 @@
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { rateLimit, rateLimitDual, resetRateLimits } from '../rate-limit';
+
+vi.mock('next/headers', () => ({
+  headers: vi.fn(() => Promise.resolve({ get: () => null })),
+}));
+
+import { headers } from 'next/headers';
+
+describe('rate-limit.ts', () => {
+  beforeEach(() => {
+    resetRateLimits();
+    vi.clearAllMocks();
+  });
+
+  describe('rateLimit', () => {
+    it('allows requests within limit and then blocks', () => {
+      const key = 'test-key';
+      expect(rateLimit(key, 2).allowed).toBe(true);
+      expect(rateLimit(key, 2).allowed).toBe(true);
+      expect(rateLimit(key, 2).allowed).toBe(false);
+    });
+
+    it('clears limits with resetRateLimits', () => {
+      const key = 'test-key';
+      expect(rateLimit(key, 1).allowed).toBe(true);
+      expect(rateLimit(key, 1).allowed).toBe(false);
+
+      resetRateLimits();
+
+      expect(rateLimit(key, 1).allowed).toBe(true);
+    });
+  });
+
+  describe('rateLimitDual', () => {
+    it('applies IP and target limits sequentially', async () => {
+      (headers as any).mockResolvedValue({
+        get: (name: string) => {
+          if (name === 'x-forwarded-for') return '1.2.3.4';
+          return null;
+        },
+      });
+
+      const key = 'target-key';
+
+      const res1 = await rateLimitDual(key, 2, 60_000, 3, 60_000);
+      expect(res1.allowed).toBe(true);
+
+      const res2 = await rateLimitDual(key, 2, 60_000, 3, 60_000);
+      expect(res2.allowed).toBe(true);
+
+      const res3 = await rateLimitDual(key, 2, 60_000, 3, 60_000);
+      expect(res3.allowed).toBe(false);
+    });
+
+    it('blocks IP before target-based bucket is polluted', async () => {
+      (headers as any).mockResolvedValue({
+        get: (name: string) => {
+          if (name === 'x-forwarded-for') return '5.5.5.5';
+          return null;
+        },
+      });
+
+      const res1 = await rateLimitDual('target-1', 5, 60_000, 1, 60_000);
+      expect(res1.allowed).toBe(true);
+
+      const res2 = await rateLimitDual('target-2', 5, 60_000, 1, 60_000);
+      expect(res2.allowed).toBe(false);
+    });
+
+    it('extracts IP from x-real-ip if x-forwarded-for is missing', async () => {
+      (headers as any).mockResolvedValue({
+        get: (name: string) => {
+          if (name === 'x-real-ip') return '9.9.9.9';
+          return null;
+        },
+      });
+
+      const res = await rateLimitDual('t', 5, 60_000, 1, 60_000);
+      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,40 @@ export function rateLimit(key: string, limit = 5, windowMs = 60_000): RateLimitR
 
   return { allowed: true, retryAfterSeconds: 0 };
 }
+
+/**
+ * Perform asynchronous rate limiting using client IP and target-based identifiers.
+ * Always run client IP rate check BEFORE target check to prevent account-lockout DoS.
+ */
+export async function rateLimitDual(
+  targetKey: string,
+  targetLimit = 5,
+  targetWindowMs = 60_000,
+  ipLimit = 10,
+  ipWindowMs = 60_000,
+): Promise<RateLimitResult> {
+  let ip: string | null = null;
+  try {
+    const headersList = await headers();
+    const xff = headersList.get('x-forwarded-for');
+    ip = xff ? (xff.split(',')[0]?.trim() ?? null) : (headersList.get('x-real-ip') ?? null);
+  } catch {
+    // Graceful fallback if headers() cannot be called or fails
+  }
+
+  // IP rate limiting check first
+  const ipResult = rateLimit(`ip:${ip ?? 'unknown'}`, ipLimit, ipWindowMs);
+  if (!ipResult.allowed) {
+    return ipResult;
+  }
+
+  // Target-based rate limiting check second
+  return rateLimit(targetKey, targetLimit, targetWindowMs);
+}
+
+/**
+ * Clear in-memory rate-limiting maps to prevent inter-test interference.
+ */
+export function resetRateLimits(): void {
+  buckets.clear();
+}