fix(auth): prune expired entries from login brute-force guard map (unbounded growth) (#4111)

Integrated into release/v3.8.28 (r8)

src/server/auth/loginGuard.ts

@@ -27,6 +27,21 @@ interface AttemptState {
 
 const attempts: Map<string, AttemptState> = new Map();
 
+// Above this many tracked IPs, opportunistically drop entries whose window has elapsed and
+// that are not currently locked. Without this the map only ever grew (entries were deleted
+// only on a *successful* login), so every distinct IP that ever failed a login leaked a
+// permanent entry — unbounded under distributed brute-force. Expired/unlocked entries are
+// already treated as "allowed", so removing them never changes a guard decision.
+const PRUNE_THRESHOLD = 256;
+
+function pruneExpiredAttempts(now: number): void {
+  for (const [key, state] of attempts) {
+    const windowElapsed = now - state.firstAttemptAt > WINDOW_MS;
+    const notLocked = !state.lockedUntil || state.lockedUntil <= now;
+    if (windowElapsed && notLocked) attempts.delete(key);
+  }
+}
+
 export interface GuardDecision {
   allowed: boolean;
   retryAfterSeconds?: number;
@@ -65,6 +80,10 @@ export function recordLoginFailure(
   if (!options.enabled) return { allowed: true };
   const key = clientKey(rawIp);
   const now = nowMs();
+
+  // Keep the map from growing without bound as distinct IPs fail logins over time.
+  if (attempts.size > PRUNE_THRESHOLD) pruneExpiredAttempts(now);
+
   const existing = attempts.get(key);
 
   if (!existing || now - existing.firstAttemptAt > WINDOW_MS) {
@@ -99,6 +118,11 @@ export function resetLoginGuardForTests(): void {
   attempts.clear();
 }
 
+/** Test-only: current number of tracked IP entries. */
+export function getLoginGuardSizeForTests(): number {
+  return attempts.size;
+}
+
 export const LOGIN_GUARD_TUNABLES = Object.freeze({
   WINDOW_MS,
   LOCKOUT_MS,

tests/unit/auth/loginGuard.test.ts

@@ -1,10 +1,12 @@
 import { describe, it, beforeEach } from "node:test";
 import assert from "node:assert/strict";
+import { mock } from "node:test";
 import {
   checkLoginGuard,
   clearLoginAttempts,
   recordLoginFailure,
   resetLoginGuardForTests,
+  getLoginGuardSizeForTests,
   LOGIN_GUARD_TUNABLES,
 } from "../../../src/server/auth/loginGuard";
 
@@ -62,4 +64,49 @@ describe("loginGuard", () => {
     }
     assert.equal(checkLoginGuard(undefined, { enabled: true }).allowed, false);
   });
+
+  it("prunes expired, unlocked entries so the attempts map does not grow without bound", () => {
+    mock.timers.enable({ apis: ["Date"] });
+    try {
+      // Many distinct IPs each fail once (single, unlocked attempts).
+      for (let i = 0; i < 300; i++) {
+        recordLoginFailure(`9.${Math.floor(i / 256)}.${i % 256}.1`, { enabled: true });
+      }
+      assert.ok(getLoginGuardSizeForTests() > 256, "entries should accumulate before pruning");
+
+      // Advance past the sliding window so all those entries are expired + unlocked.
+      mock.timers.tick(LOGIN_GUARD_TUNABLES.WINDOW_MS + 1000);
+
+      // The next failure (map size > threshold) triggers the opportunistic prune.
+      recordLoginFailure("1.1.1.1", { enabled: true });
+
+      // Only the fresh entry should remain; the stale ones were reaped.
+      assert.equal(getLoginGuardSizeForTests(), 1);
+    } finally {
+      mock.timers.reset();
+    }
+  });
+
+  it("never prunes a still-locked entry", () => {
+    mock.timers.enable({ apis: ["Date"] });
+    try {
+      const lockedIp = "5.5.5.5";
+      for (let i = 0; i < LOGIN_GUARD_TUNABLES.FAILURE_THRESHOLD; i++) {
+        recordLoginFailure(lockedIp, { enabled: true });
+      }
+      assert.equal(checkLoginGuard(lockedIp, { enabled: true }).allowed, false);
+
+      // Fill past the prune threshold with unlocked entries, then trigger a prune
+      // while still inside the lockout window.
+      for (let i = 0; i < 300; i++) {
+        recordLoginFailure(`8.${Math.floor(i / 256)}.${i % 256}.1`, { enabled: true });
+      }
+      recordLoginFailure("2.2.2.2", { enabled: true });
+
+      // The locked IP must still be locked (not reaped).
+      assert.equal(checkLoginGuard(lockedIp, { enabled: true }).allowed, false);
+    } finally {
+      mock.timers.reset();
+    }
+  });
 });