Mike/freno-dev
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(p8-010): move rate-limit store to a shared distributed DB store

Browse Source 
Replace the per-Vercel-instance in-memory Map rate-limit cache with an
atomic shared store backed by the existing Turso RateLimit table, so limits
hold across all instances/redeploys and cannot be bypassed by distributing
brute-force attempts across instances (audit finding p8-010, MEDIUM).

- checkRateLimit now performs a single atomic round-trip:
  INSERT ... ON CONFLICT(identifier) DO UPDATE ... RETURNING count, reset_at
  with window-reset semantics (CASE WHEN reset_at < now THEN 1 ELSE count+1).
- The DB is now the primary source of truth (no longer a fire-and-forget
  fallback). The per-instance Map is reduced to a short-TTL local cache used
  ONLY to fast-fail already-blocked identifiers (cuts DB load during brute-
  force storms); it can never let a request bypass the limit.
- ensureRateLimitSchema() creates the table + a UNIQUE identifier index so
  ON CONFLICT upserts are well-defined; added RateLimit to db/create.ts.
- resetLoginRateLimits / clearRateLimitStore invalidate the local cache.
- getClientIP now trusts proxy headers in non-development environments
  (production + test); local dev stays strict against header spoofing.
- bunfig.toml defines import.meta.env.SSR=true so the server-only env guard
  loads under 'bun test'.
- Tests: await clearRateLimitStore in beforeEach (fixes a race where an
  un-awaited clear let leftover rows corrupt the next upsert); unique test
  identifiers; realistic remote-shared-store perf bounds; new p8-010
  distributed-store tests (restart-survival, multi-instance aggregation,
  no bypass by alternating instances).
This commit is contained in:
Michael Freno
2026-07-22 18:10:28 -04:00
parent 3bb3e80b77
commit e446eb1775
4 changed files with 314 additions and 172 deletions
Download Patch File Download Diff File Expand all files Collapse all files

154
src/server/security/rate-limit.test.ts
View File

@@ -12,20 +12,36 @@ import {
rateLimitRegistration,
rateLimitEmailVerification,
clearRateLimitStore,
clearRateLimitLocalCache,
RATE_LIMITS
} from "~/server/security";
import { createMockEvent, randomIP } from "./test-utils";
import { TRPCError } from "@trpc/server";
/**
* Unique identifier helper — Date.now() alone collides when tests run within
* the same millisecond, which leaks state between tests. Appending randomness
* keeps each test's bucket isolated.
*/
let idCounter = 0;
function uniqueId(prefix = "test"): string {
idCounter += 1;
return `${prefix}-${Date.now()}-${idCounter}-${Math.random()
.toString(36)
.slice(2, 8)}`;
}
describe("Rate Limiting", () => {
// Clear rate limit store before each test to ensure isolation
beforeEach(() => {
clearRateLimitStore();
// Clear rate limit store before each test to ensure isolation. MUST be
// awaited — clearRateLimitStore is async (DB round-trip) and an un-awaited
// clear lets leftover rows race the next test's atomic upsert.
beforeEach(async () => {
await clearRateLimitStore();
});
describe("checkRateLimit", () => {
it("should allow requests within rate limit", async () => {
const identifier = `test-${Date.now()}`;
const identifier = uniqueId();
const maxAttempts = 5;
const windowMs = 60000;
@@ -40,7 +56,7 @@ describe("Rate Limiting", () => {
});
it("should block requests exceeding rate limit", async () => {
const identifier = `test-${Date.now()}`;
const identifier = uniqueId();
const maxAttempts = 3;
const windowMs = 60000;
@@ -59,7 +75,7 @@ describe("Rate Limiting", () => {
});
it("should include remaining time in error message", async () => {
const identifier = `test-${Date.now()}`;
const identifier = uniqueId();
const maxAttempts = 2;
const windowMs = 60000;
@@ -79,7 +95,7 @@ describe("Rate Limiting", () => {
});
it("should reset after time window expires", async () => {
const identifier = `test-${Date.now()}`;
const identifier = uniqueId();
const maxAttempts = 3;
const windowMs = 500; // 500ms window for testing
@@ -105,7 +121,7 @@ describe("Rate Limiting", () => {
});
it("should handle concurrent requests correctly", async () => {
const identifier = `test-${Date.now()}`;
const identifier = uniqueId();
const maxAttempts = 10;
const windowMs = 60000;
@@ -123,8 +139,8 @@ describe("Rate Limiting", () => {
const maxAttempts = 3;
const windowMs = 60000;
const id1 = `test1-${Date.now()}`;
const id2 = `test2-${Date.now()}`;
const id1 = uniqueId("test1");
const id2 = uniqueId("test2");
// Use up attempts for id1
for (let i = 0; i < maxAttempts; i++) {
@@ -488,34 +504,112 @@ describe("Rate Limiting", () => {
});
describe("Performance", () => {
it("should handle high volume of rate limit checks efficiently", async () => {
const start = performance.now();
it("should keep single-key shared-store check latency within an acceptable bound", async () => {
// p8-010: the rate-limit state now lives in the shared DB store instead of
// an in-memory Map. The latency that matters for logins is a single
// checkRateLimit round-trip, not aggregate throughput. Assert it stays
// within an acceptable bound for a remote shared store.
const id = uniqueId("perf");
const maxAttempts = 5;
const windowMs = 60000;
// Check 100 different identifiers (reduced from 1000 due to async overhead)
// Warm the bucket so we measure the ON CONFLICT UPDATE path.
await checkRateLimit(id, maxAttempts, windowMs);
const start = performance.now();
await checkRateLimit(id, maxAttempts, windowMs);
const singleLatency = performance.now() - start;
// Generous bound for a remote libSQL/Turso round-trip; catches gross
// regressions (e.g. falling back to multi-statement SELECT+UPDATE).
expect(singleLatency).toBeLessThan(2000);
}, 15000);
it("should not crash with many distinct identifiers", async () => {
// Each call performs a DB upsert; keep the volume bounded so the test
// stays well under the remote-DB latency budget.
const promises = [];
for (let i = 0; i < 100; i++) {
promises.push(checkRateLimit(`test-${i}`, 5, 60000));
for (let i = 0; i < 30; i++) {
promises.push(checkRateLimit(uniqueId("perf-many"), 5, 60000));
}
await Promise.all(promises);
const duration = performance.now() - start;
// This test mainly ensures no crashes occur under concurrent upserts.
// Memory cleanup is tested by the cleanup interval in security.ts.
expect(true).toBe(true);
}, 20000);
});
// Should complete in reasonable time (adjusted for async operations)
expect(duration).toBeLessThan(1000);
// ===========================================================================
// p8-010: distributed rate-limit store. The authoritative counter lives in the
// shared `RateLimit` DB table (atomic upsert), so limits hold across instances
// and survive restarts/redeploys. The per-instance Map is now only a short-TTL
// local cache for fast-failing already-blocked identifiers.
// ===========================================================================
describe("Distributed rate-limit store (p8-010)", () => {
it("state survives a simulated instance restart (local cache cleared, shared store blocks)", async () => {
const id = uniqueId("dist-restart");
const maxAttempts = 3;
const windowMs = 60000;
// Exhaust the limit: 3 allowed, 4th blocked.
for (let i = 0; i < maxAttempts; i++) {
await checkRateLimit(id, maxAttempts, windowMs);
}
await expect(checkRateLimit(id, maxAttempts, windowMs)).rejects.toThrow(
TRPCError
);
// Simulate an instance restart: wipe ONLY the in-memory cache. A naive
// per-instance Map would lose the block here; the shared store must keep
// blocking from the DB.
clearRateLimitLocalCache();
await expect(checkRateLimit(id, maxAttempts, windowMs)).rejects.toThrow(
TRPCError
);
});
it("should not leak memory with many identifiers", async () => {
// Create rate limit entries (reduced significantly due to database overhead)
// Each call performs database operations which are slower than in-memory checks
const promises = [];
for (let i = 0; i < 100; i++) {
promises.push(checkRateLimit(`test-${i}`, 5, 60000));
}
await Promise.all(promises);
it("two simulated instances aggregate the count for the same key", async () => {
const id = uniqueId("dist-multi");
const maxAttempts = 5;
const windowMs = 60000;
// This test mainly ensures no crashes occur
// Memory cleanup is tested by the cleanup interval in security.ts
expect(true).toBe(true);
}, 10000); // Increase timeout to 10 seconds for database operations
// Instance A: 3 attempts.
clearRateLimitLocalCache();
for (let i = 0; i < 3; i++) {
await checkRateLimit(id, maxAttempts, windowMs);
}
// Instance B (fresh local cache) makes 2 more -> combined count = 5.
clearRateLimitLocalCache();
await checkRateLimit(id, maxAttempts, windowMs); // count 4
const remaining = await checkRateLimit(id, maxAttempts, windowMs); // count 5
expect(remaining).toBe(0); // 5th allowed, no remaining
// A 6th attempt from a fresh instance must be blocked — the shared store
// aggregated the count across the two "instances".
clearRateLimitLocalCache();
await expect(checkRateLimit(id, maxAttempts, windowMs)).rejects.toThrow(
TRPCError
);
});
it("cannot bypass the limit by alternating between instances", async () => {
const id = uniqueId("dist-bypass");
const maxAttempts = 4;
const windowMs = 60000;
// Each request simulates landing on a different instance (fresh local
// cache). The shared DB counter must still aggregate every hit.
for (let i = 0; i < maxAttempts; i++) {
clearRateLimitLocalCache();
await checkRateLimit(id, maxAttempts, windowMs);
}
clearRateLimitLocalCache();
await expect(checkRateLimit(id, maxAttempts, windowMs)).rejects.toThrow(
TRPCError
);
});
});
});