🛡️ Sentinel: add dual rate limiting to authentication 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-17 - Dual-Layer Rate Limiting for Signup and Signin Server Actions
+**Vulnerability:** The signup and signin Server Actions used only target-based (email) rate limiting. This left the application vulnerable to distributed credential stuffing attacks, where attackers query different target accounts from a single client IP (or a small set of IPs) without hitting target-based lockout thresholds.
+**Learning:** Target-only rate limiting can be bypassed by distributing requests across a wide variety of target keys (e.g., trying a common password against many different usernames/emails).
+**Prevention:** Always combine target-based rate limiting with client IP-based rate limiting (dual-layer rate limiting) using the `x-forwarded-for` and `x-real-ip` headers on sensitive authentication actions, allowing graceful degradation if headers are not present.

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/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);
   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);
   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,180 @@
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+
+vi.mock('next/headers', () => ({
+  headers: vi.fn(),
+}));
+
+import { rateLimit, rateLimitDual } from '../rate-limit';
+import { headers } from 'next/headers';
+
+describe('rate-limit.ts', () => {
+  beforeEach(() => {
+    vi.useFakeTimers();
+    vi.clearAllMocks();
+  });
+
+  afterEach(() => {
+    vi.useRealTimers();
+  });
+
+  describe('rateLimit (sliding window)', () => {
+    it('allows hits within limits', () => {
+      const key = 'user1';
+      for (let i = 0; i < 5; i++) {
+        const res = rateLimit(key, 5, 60_000);
+        expect(res.allowed).toBe(true);
+        expect(res.retryAfterSeconds).toBe(0);
+      }
+    });
+
+    it('denies hits when limit is exceeded', () => {
+      const key = 'user2';
+      for (let i = 0; i < 5; i++) {
+        rateLimit(key, 5, 60_000);
+      }
+
+      const res = rateLimit(key, 5, 60_000);
+      expect(res.allowed).toBe(false);
+      expect(res.retryAfterSeconds).toBe(60); // 60 seconds remaining
+    });
+
+    it('denies hits and calculates remaining lockout seconds correctly', () => {
+      const key = 'user3';
+      // T = 0ms: hit 1
+      rateLimit(key, 2, 60_000);
+
+      // Advance by 15.5 seconds (T = 15500ms)
+      vi.advanceTimersByTime(15_500);
+
+      // T = 15.5s: hit 2 (limit reached)
+      rateLimit(key, 2, 60_000);
+
+      // Hit 3 at T = 15.5s should be denied
+      const res = rateLimit(key, 2, 60_000);
+      expect(res.allowed).toBe(false);
+      // Hit 1 was at T=0. It falls out at T=60000ms.
+      // So retryAfterSeconds should be ceil((0 + 60000 - 15500) / 1000) = ceil(44.5) = 45 seconds.
+      expect(res.retryAfterSeconds).toBe(45);
+    });
+
+    it('allows hits again after window has elapsed', () => {
+      const key = 'user4';
+      rateLimit(key, 1, 60_000);
+
+      // Denied
+      expect(rateLimit(key, 1, 60_000).allowed).toBe(false);
+
+      // Advance by 60.1 seconds
+      vi.advanceTimersByTime(60_100);
+
+      // Allowed again
+      expect(rateLimit(key, 1, 60_000).allowed).toBe(true);
+    });
+
+    it('performs opportunistic cleanup of buckets Map when size exceeds threshold', () => {
+      // Create over 10,000 keys with a past timestamp so they are stale and cleaned up.
+      // Let's set time to T=100000
+      vi.setSystemTime(new Date(100_000));
+
+      for (let i = 0; i < 10005; i++) {
+        rateLimit(`key-${i}`, 5, 10); // small window of 10ms
+      }
+
+      // Advance time so all of them are now past cutoff (10ms)
+      vi.advanceTimersByTime(50);
+
+      // Calling rateLimit again with a new key will trigger size > 10_000 check and cleanup
+      const res = rateLimit('trigger-cleanup-key', 5, 10);
+      expect(res.allowed).toBe(true);
+    });
+  });
+
+  describe('rateLimitDual', () => {
+    it('allows when under limits and headers are available', async () => {
+      (headers as unknown as ReturnType<typeof vi.fn>).mockResolvedValue({
+        get: (h: string) => {
+          if (h === 'x-forwarded-for') return '192.168.1.1';
+          return null;
+        },
+      });
+
+      const res = await rateLimitDual('login', 'test@example.com', { limit: 3 });
+      expect(res.allowed).toBe(true);
+      expect(res.retryAfterSeconds).toBe(0);
+    });
+
+    it('applies lowercase to targetKey', async () => {
+      (headers as unknown as ReturnType<typeof vi.fn>).mockResolvedValue({
+        get: () => null,
+      });
+
+      // Email with uppercase chars
+      await rateLimitDual('test-case', 'User@Example.Com', { limit: 1 });
+
+      // Hit with same email, lowercase
+      const res = await rateLimitDual('test-case', 'user@example.com', { limit: 1 });
+      expect(res.allowed).toBe(false);
+    });
+
+    it('extracts first IP from x-forwarded-for list correctly', async () => {
+      (headers as unknown as ReturnType<typeof vi.fn>).mockResolvedValue({
+        get: (h: string) => {
+          if (h === 'x-forwarded-for') return '1.2.3.4, 5.6.7.8, 9.10.11.12';
+          return null;
+        },
+      });
+
+      // IP limit is limit * 2 = 2.
+      // 1st hit from IP 1.2.3.4 (target different)
+      await rateLimitDual('ip-test', 'email1@example.com', { limit: 1 });
+      // 2nd hit from same IP (target different)
+      await rateLimitDual('ip-test', 'email2@example.com', { limit: 1 });
+      // 3rd hit from same IP (exceeds IP limit of 2)
+      const res = await rateLimitDual('ip-test', 'email3@example.com', { limit: 1 });
+      expect(res.allowed).toBe(false);
+    });
+
+    it('falls back to x-real-ip if x-forwarded-for is missing', async () => {
+      (headers as unknown as ReturnType<typeof vi.fn>).mockResolvedValue({
+        get: (h: string) => {
+          if (h === 'x-real-ip') return '9.9.9.9';
+          return null;
+        },
+      });
+
+      // limit * 2 = 2 hits allowed for IP 9.9.9.9.
+      await rateLimitDual('real-ip-test', 'email1@example.com', { limit: 1 });
+      await rateLimitDual('real-ip-test', 'email2@example.com', { limit: 1 });
+
+      const res = await rateLimitDual('real-ip-test', 'email3@example.com', { limit: 1 });
+      expect(res.allowed).toBe(false);
+    });
+
+    it('falls back to unknown if no IP headers are present', async () => {
+      (headers as unknown as ReturnType<typeof vi.fn>).mockResolvedValue({
+        get: () => null,
+      });
+
+      await rateLimitDual('unknown-ip-test', 'email1@example.com', { limit: 1 });
+      await rateLimitDual('unknown-ip-test', 'email2@example.com', { limit: 1 });
+
+      const res = await rateLimitDual('unknown-ip-test', 'email3@example.com', { limit: 1 });
+      expect(res.allowed).toBe(false);
+    });
+
+    it('degrades gracefully if headers() throws an error', async () => {
+      (headers as unknown as ReturnType<typeof vi.fn>).mockRejectedValue(new Error('Headers unavailable'));
+
+      // IP-based limit won't apply because headers throws, but target limit still applies.
+      // Limit = 2.
+      const res1 = await rateLimitDual('throw-test', 'email1@example.com', { limit: 2 });
+      expect(res1.allowed).toBe(true);
+
+      const res2 = await rateLimitDual('throw-test', 'email1@example.com', { limit: 2 });
+      expect(res2.allowed).toBe(true);
+
+      const res3 = await rateLimitDual('throw-test', 'email1@example.com', { limit: 2 });
+      expect(res3.allowed).toBe(false);
+    });
+  });
+});

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 };
 }
+
+/**
+ * Dual rate limiter: combines target-based rate limiting with client IP-based rate limiting.
+ * Protects against credential stuffing and distributed brute-force attacks.
+ */
+export async function rateLimitDual(
+  actionType: string,
+  targetKey: string,
+  options: { limit?: number; windowMs?: number } = {}
+): Promise<RateLimitResult> {
+  const limit = options.limit ?? 5;
+  const windowMs = options.windowMs ?? 60_000;
+
+  // 1. Target-based rate limiting (e.g., lowercase email)
+  const targetKeyLower = targetKey.toLowerCase();
+  const targetKeyPrefixed = `${actionType}:target:${targetKeyLower}`;
+  const targetResult = rateLimit(targetKeyPrefixed, limit, windowMs);
+  if (!targetResult.allowed) {
+    return targetResult;
+  }
+
+  // 2. IP-based rate limiting (double the limit of target-based to allow NAT/shared IPs)
+  try {
+    const heads = await headers();
+    const xff = heads.get('x-forwarded-for');
+    const ip = (xff ? xff.split(',')[0]?.trim() : null) ?? heads.get('x-real-ip') ?? 'unknown';
+    const ipPrefixed = `${actionType}:ip:${ip}`;
+    const ipResult = rateLimit(ipPrefixed, limit * 2, windowMs);
+    if (!ipResult.allowed) {
+      return ipResult;
+    }
+  } catch {
+    // Graceful degradation if headers() throws or is unavailable
+  }
+
+  return { allowed: true, retryAfterSeconds: 0 };
+}