From c2d0283d9b9d309926e741ed3b2235ab0d1797c5 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 10 Aug 2026 13:08:45 +0000 Subject: [PATCH 1/3] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20impleme?= =?UTF-8?q?nt=20dual=20rate-limiting=20on=20authentication=20server=20acti?= =?UTF-8?q?ons?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: projectamazonph <286085559+projectamazonph@users.noreply.github.com> --- .jules/sentinel.md | 5 + src/app/actions/auth.ts | 22 ++- src/lib/__tests__/rate-limit.test.ts | 216 +++++++++++++++++++++++++++ src/lib/rate-limit.ts | 55 +++++++ 4 files changed, 293 insertions(+), 5 deletions(-) create mode 100644 src/lib/__tests__/rate-limit.test.ts diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 29abdfb..7f1138c 100644 --- a/.jules/sentinel.md +++ b/.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 Rate-Limiting Ordering Prevents Account Lockout DoS +**Vulnerability:** Legacy rate-limiting checked only target-based identifiers (e.g. email) on authentication routes. This allowed attackers to perform distributed credential stuffing attacks by varying target emails. Furthermore, checking target buckets first meant malicious IP-blocked actors could still spam and fill target-based buckets, locking out or blocking legitimate users. +**Learning:** Target-only rate-limiting leaves endpoints exposed to distributed attacks. When adding IP-based rate-limiting alongside target-based rate-limiting, the client IP check must be executed and enforced *before* the target-based check. +**Prevention:** Implement `rateLimitDual` which performs the IP bucket verification first. If the client IP is blocked, return immediately without recording a hit in the target-based bucket, ensuring legitimate users' targets are not polluted or blocked by malicious IPs. diff --git a/src/app/actions/auth.ts b/src/app/actions/auth.ts index 2f10075..1756859 100644 --- a/src/app/actions/auth.ts +++ b/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,13 @@ import { // --------------------------------------------------------------------------- export const signUpAction = createSafeAction(signUpSchema, async (data) => { - const rl = rateLimit(`signup:${data.email.toLowerCase()}`, 5, 60_000); + // Use dual rate-limiting combining client IP check and target-based checks (email) + const rl = await rateLimitDual('signup-ip', `signup:${data.email.toLowerCase()}`, { + ipLimit: 10, + ipWindowMs: 60_000, + targetLimit: 5, + targetWindowMs: 60_000, + }); if (!rl.allowed) { throw new Error(`Too many attempts. Try again in ${rl.retryAfterSeconds}s.`); } @@ -123,9 +129,15 @@ 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); + // Use dual rate-limiting combining client IP check and target-based checks (email). + // Rate-limit BEFORE any DB or scrypt work — the async scrypt verify can still + // be targeted to cause severe load. + const rl = await rateLimitDual('signin-ip', `signin:${data.email.toLowerCase()}`, { + ipLimit: 10, + ipWindowMs: 60_000, + targetLimit: 5, + targetWindowMs: 60_000, + }); if (!rl.allowed) { throw new Error(`Too many attempts. Try again in ${rl.retryAfterSeconds}s.`); } diff --git a/src/lib/__tests__/rate-limit.test.ts b/src/lib/__tests__/rate-limit.test.ts new file mode 100644 index 0000000..504245e --- /dev/null +++ b/src/lib/__tests__/rate-limit.test.ts @@ -0,0 +1,216 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { headers } from 'next/headers'; +import { rateLimit, rateLimitDual, resetRateLimits } from '../rate-limit'; + +vi.mock('next/headers', () => ({ + headers: vi.fn(), +})); + +describe('Rate Limiter', () => { + beforeEach(() => { + vi.useFakeTimers(); + resetRateLimits(); + vi.clearAllMocks(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + describe('rateLimit', () => { + it('allows hits within limits and blocks when exceeded', () => { + // Limit 3 hits in 1 minute (60,000 ms) + expect(rateLimit('user1', 3, 60_000).allowed).toBe(true); + expect(rateLimit('user1', 3, 60_000).allowed).toBe(true); + expect(rateLimit('user1', 3, 60_000).allowed).toBe(true); + + // 4th hit should be blocked + const blocked = rateLimit('user1', 3, 60_000); + expect(blocked.allowed).toBe(false); + expect(blocked.retryAfterSeconds).toBe(60); // 1 minute (60s) + }); + + it('sliding window lets hits expire', () => { + expect(rateLimit('user1', 2, 10_000).allowed).toBe(true); + + // Advance time by 5s + vi.advanceTimersByTime(5000); + expect(rateLimit('user1', 2, 10_000).allowed).toBe(true); + + // Third hit within 10s should block + expect(rateLimit('user1', 2, 10_000).allowed).toBe(false); + + // Advance time by 6s (total 11s), first hit has expired, so we should be allowed again + vi.advanceTimersByTime(6000); + expect(rateLimit('user1', 2, 10_000).allowed).toBe(true); + }); + + it('cleans up obsolete buckets', () => { + // We need to insert > 10,000 buckets to trigger cleanup. + for (let i = 0; i < 10001; i++) { + rateLimit(`user_${i}`, 1, 10_000); + } + // Buckets are now full. Let's advance timers so they are obsolete + vi.advanceTimersByTime(11000); + // Trigger another rate limit to run cleanup + rateLimit('trigger', 1, 10_000); + + // Since all other 10001 buckets were obsolete, they should be cleaned up. + // Let's verify that hitting an existing one starts a fresh limit instead of old one + const res = rateLimit('user_0', 1, 10_000); + expect(res.allowed).toBe(true); + }); + }); + + describe('rateLimitDual', () => { + it('applies IP rate limit before target-based check', async () => { + // Mock headers with IP 1.2.3.4 + (headers as any).mockResolvedValue({ + get: (name: string) => { + if (name === 'x-forwarded-for') return '1.2.3.4, 5.6.7.8'; + return null; + }, + }); + + // Target limit is 2, IP limit is 3. + // 1st request + const r1 = await rateLimitDual('ip-test', 'target-test', { + ipLimit: 3, + targetLimit: 2, + }); + expect(r1.allowed).toBe(true); + + // 2nd request + const r2 = await rateLimitDual('ip-test', 'target-test', { + ipLimit: 3, + targetLimit: 2, + }); + expect(r2.allowed).toBe(true); + + // 3rd request should hit target limit (since targetLimit is 2) + const r3 = await rateLimitDual('ip-test', 'target-test', { + ipLimit: 3, + targetLimit: 2, + }); + expect(r3.allowed).toBe(false); + }); + + it('blocks by IP limit before target limit, and does not record target-based hit on IP failure', async () => { + // Mock headers with IP 1.2.3.4 + let currentIp = '1.2.3.4'; + (headers as any).mockResolvedValue({ + get: (name: string) => { + if (name === 'x-forwarded-for') return currentIp; + return null; + }, + }); + + // Target limit is 5, IP limit is 2 + const r1 = await rateLimitDual('ip-test', 'target-test', { + ipLimit: 2, + targetLimit: 5, + }); + expect(r1.allowed).toBe(true); + + const r2 = await rateLimitDual('ip-test', 'target-test', { + ipLimit: 2, + targetLimit: 5, + }); + expect(r2.allowed).toBe(true); + + // 3rd request hits IP limit first + const r3 = await rateLimitDual('ip-test', 'target-test', { + ipLimit: 2, + targetLimit: 5, + }); + expect(r3.allowed).toBe(false); + + // Crucial: since IP blocked the 3rd request, target-based bucket should NOT have been polluted + // i.e. target-based hits should still be 2. Let's change IP to 2.2.2.2 and try same target + currentIp = '2.2.2.2'; + + // Target hit count is still 2. We can hit it 3 more times with new IPs each time to avoid IP limit. + const r4 = await rateLimitDual('ip-test', 'target-test', { + ipLimit: 2, + targetLimit: 5, + }); + expect(r4.allowed).toBe(true); // target hit 3 + + currentIp = '3.3.3.3'; + const r5 = await rateLimitDual('ip-test', 'target-test', { + ipLimit: 2, + targetLimit: 5, + }); + expect(r5.allowed).toBe(true); // target hit 4 + + currentIp = '4.4.4.4'; + const r6 = await rateLimitDual('ip-test', 'target-test', { + ipLimit: 2, + targetLimit: 5, + }); + expect(r6.allowed).toBe(true); // target hit 5 + + currentIp = '5.5.5.5'; + const r7 = await rateLimitDual('ip-test', 'target-test', { + ipLimit: 2, + targetLimit: 5, + }); + expect(r7.allowed).toBe(false); // target limit exceeded + }); + + it('falls back to 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 r1 = await rateLimitDual('ip-real', 'target-real', { ipLimit: 1 }); + expect(r1.allowed).toBe(true); + + // 2nd hit should block on IP + const r2 = await rateLimitDual('ip-real', 'target-real2', { ipLimit: 1 }); + expect(r2.allowed).toBe(false); + }); + + it('skips IP check and uses target check if no IP headers exist', async () => { + (headers as any).mockResolvedValue({ + get: () => null, + }); + + const r1 = await rateLimitDual('ip-none', 'target-only', { ipLimit: 1, targetLimit: 2 }); + expect(r1.allowed).toBe(true); + + const r2 = await rateLimitDual('ip-none', 'target-only', { ipLimit: 1, targetLimit: 2 }); + expect(r2.allowed).toBe(true); + + const r3 = await rateLimitDual('ip-none', 'target-only', { ipLimit: 1, targetLimit: 2 }); + expect(r3.allowed).toBe(false); + }); + + it('falls back gracefully to target-only rate limiting if headers() throws', async () => { + (headers as any).mockRejectedValue(new Error('Outside of request context')); + + const r1 = await rateLimitDual('ip-error', 'target-fallback', { ipLimit: 1, targetLimit: 2 }); + expect(r1.allowed).toBe(true); + + const r2 = await rateLimitDual('ip-error', 'target-fallback', { ipLimit: 1, targetLimit: 2 }); + expect(r2.allowed).toBe(true); + + const r3 = await rateLimitDual('ip-error', 'target-fallback', { ipLimit: 1, targetLimit: 2 }); + expect(r3.allowed).toBe(false); + }); + }); + + describe('resetRateLimits', () => { + it('clears rate limiting maps', () => { + expect(rateLimit('u', 1).allowed).toBe(true); + expect(rateLimit('u', 1).allowed).toBe(false); + + resetRateLimits(); + + expect(rateLimit('u', 1).allowed).toBe(true); + }); + }); +}); diff --git a/src/lib/rate-limit.ts b/src/lib/rate-limit.ts index 5d0ef23..dbb32ac 100644 --- a/src/lib/rate-limit.ts +++ b/src/lib/rate-limit.ts @@ -8,6 +8,7 @@ */ import 'server-only'; +import { headers } from 'next/headers'; const buckets = new Map(); @@ -47,3 +48,57 @@ export function rateLimit(key: string, limit = 5, windowMs = 60_000): RateLimitR return { allowed: true, retryAfterSeconds: 0 }; } + +/** + * Dual rate limiter: combines client IP check and target-based checks (like email). + * Always executes the client IP check and rate limiting before checking the target-based + * identifier to prevent malicious IP-blocked actors from polluting target-based buckets + * and causing an Account Lockout Denial of Service (DoS) for legitimate users. + */ +export async function rateLimitDual( + ipKeyPrefix: string, + targetKey: string, + options: { + ipLimit?: number; + ipWindowMs?: number; + targetLimit?: number; + targetWindowMs?: number; + } = {} +): Promise { + const { + ipLimit = 10, + ipWindowMs = 60_000, + targetLimit = 5, + targetWindowMs = 60_000, + } = options; + + let heads; + try { + heads = await headers(); + } catch { + // Fallback gracefully if called outside of request context (e.g. testing) + } + + if (heads) { + const xff = heads.get('x-forwarded-for'); + // Safe access for split under noUncheckedIndexedAccess rule + const ip = xff ? xff.split(',')[0]?.trim() : (heads.get('x-real-ip') ?? null); + + if (ip) { + const ipKey = `${ipKeyPrefix}:${ip}`; + const ipResult = rateLimit(ipKey, ipLimit, ipWindowMs); + if (!ipResult.allowed) { + return ipResult; + } + } + } + + return rateLimit(targetKey, targetLimit, targetWindowMs); +} + +/** + * Resets the in-memory rate-limiting buckets (primarily for testing purposes). + */ +export function resetRateLimits(): void { + buckets.clear(); +} From d26e2972f207563c0a2bd0ff202eed417484e5d7 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 10 Aug 2026 13:15:11 +0000 Subject: [PATCH 2/3] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20impleme?= =?UTF-8?q?nt=20dual=20rate-limiting=20on=20authentication=20server=20acti?= =?UTF-8?q?ons=20and=20update=20pnpm=20to=20a=20healthy=20version?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: projectamazonph <286085559+projectamazonph@users.noreply.github.com> --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index e60c440..bc7cc2f 100644 --- a/package.json +++ b/package.json @@ -72,5 +72,5 @@ "eslint --fix" ] }, - "packageManager": "pnpm@11.13.0+sha512.88d94724d8f2e6c186744a5584c6e59ecac869ec7ba15e9cb4cd628e8dc7066820b2481d8ee3b51ea8da323a7378068aa58c556a3720d32b7c20a051d088363a" + "packageManager": "pnpm@11.14.0+sha512.66c1ac4c7d4762d6d7dde44c7f3e5a73591ed0a0806e751d4ed32d4f004f25b2285a906b1fd8a9e3e621df3b4e2858bf88e50e0cf626bedbe977fe434a5caf85" } From 1fc72ad8e4704f8b37cce0c6595b196c06cf034b Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 10 Aug 2026 13:19:07 +0000 Subject: [PATCH 3/3] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20impleme?= =?UTF-8?q?nt=20dual=20rate-limiting=20on=20authentication=20server=20acti?= =?UTF-8?q?ons=20and=20update=20pnpm=20to=20a=20healthy=20version?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: projectamazonph <286085559+projectamazonph@users.noreply.github.com>