From 4007c5db684fc6d2900cf8260150122bb916335a Mon Sep 17 00:00:00 2001
From: Emp1500 <Emp1500@users.noreply.github.com>
Date: Sat, 18 Jul 2026 11:01:00 +0000
Subject: [PATCH] fix: make rate limiter fail closed on Redis outage

Upstash Ratelimit's default 5s timeout resolves { success: true,
reason: 'timeout' } when Redis is unreachable, silently bypassing rate
limits. Add an explicit 1s timeout and have checkRateLimit translate the
timeout response (and any thrown error) into a deny, so limits can't be
bypassed by inducing Redis latency. Also drop the unused globalLimit.
---
 __tests__/rate-limit.test.ts | 50 ++++++++++++++++++++++++++++++++++++
 lib/rate-limit.ts            | 23 +++++++++++++----
 2 files changed, 68 insertions(+), 5 deletions(-)
 create mode 100644 __tests__/rate-limit.test.ts

diff --git a/__tests__/rate-limit.test.ts b/__tests__/rate-limit.test.ts
new file mode 100644
index 0000000..081a2ce
--- /dev/null
+++ b/__tests__/rate-limit.test.ts
@@ -0,0 +1,50 @@
+// Keep the module import side-effect-free: the real Upstash clients need env
+// vars at construction time, which aren't set under test.
+jest.mock('@upstash/redis', () => ({ Redis: class {} }))
+jest.mock('@upstash/ratelimit', () => {
+  class Ratelimit {}
+  ;(Ratelimit as unknown as { slidingWindow: () => object }).slidingWindow = () => ({})
+  return { Ratelimit }
+})
+
+import { checkRateLimit } from '@/lib/rate-limit'
+import type { Ratelimit } from '@upstash/ratelimit'
+
+function limiterReturning(value: unknown): Ratelimit {
+  return { limit: jest.fn(async () => value) } as unknown as Ratelimit
+}
+
+describe('checkRateLimit', () => {
+  it('denies when Redis times out (Upstash fail-open override)', async () => {
+    const limiter = limiterReturning({ success: true, reset: 0, reason: 'timeout' })
+
+    const result = await checkRateLimit(limiter, 'user-1')
+
+    expect(result.success).toBe(false)
+  })
+
+  it('denies when the limiter throws (Redis unreachable)', async () => {
+    const limiter = { limit: jest.fn(async () => { throw new Error('ECONNREFUSED') }) } as unknown as Ratelimit
+
+    const result = await checkRateLimit(limiter, 'user-1')
+
+    expect(result.success).toBe(false)
+  })
+
+  it('allows a normal successful check', async () => {
+    const limiter = limiterReturning({ success: true, reset: Date.now() + 60_000 })
+
+    const result = await checkRateLimit(limiter, 'user-1')
+
+    expect(result.success).toBe(true)
+  })
+
+  it('passes through a normal deny with a positive retryAfter', async () => {
+    const limiter = limiterReturning({ success: false, reset: Date.now() + 30_000 })
+
+    const result = await checkRateLimit(limiter, 'user-1')
+
+    expect(result.success).toBe(false)
+    expect(result.retryAfter).toBeGreaterThan(0)
+  })
+})
diff --git a/lib/rate-limit.ts b/lib/rate-limit.ts
index e03cb0c..07b91e3 100644
--- a/lib/rate-limit.ts
+++ b/lib/rate-limit.ts
@@ -6,11 +6,24 @@ const redis = new Redis({
   token: process.env.UPSTASH_REDIS_REST_TOKEN!,
 })
 
-export const saveLimit   = new Ratelimit({ redis, limiter: Ratelimit.slidingWindow(30, '1m') })
-export const shareLimit  = new Ratelimit({ redis, limiter: Ratelimit.slidingWindow(10, '1m') })
-export const globalLimit = new Ratelimit({ redis, limiter: Ratelimit.slidingWindow(100, '1m') })
+// Bound how long a request waits on Redis. On timeout Upstash resolves
+// { success: true, reason: 'timeout' } — a fail-OPEN that checkRateLimit
+// overrides to a deny below.
+const timeout = 1000
+
+export const saveLimit  = new Ratelimit({ redis, limiter: Ratelimit.slidingWindow(30, '1m'), timeout })
+export const shareLimit = new Ratelimit({ redis, limiter: Ratelimit.slidingWindow(10, '1m'), timeout })
 
 export async function checkRateLimit(limiter: Ratelimit, key: string) {
-  const { success, reset } = await limiter.limit(key)
-  return { success, retryAfter: Math.ceil((reset - Date.now()) / 1000) }
+  try {
+    const { success, reset, reason } = await limiter.limit(key)
+    // Upstash fails open when Redis is unreachable (resolves success:true,
+    // reason:'timeout'). Deny instead so the limit can't be bypassed by
+    // inducing Redis latency.
+    if (reason === 'timeout') return { success: false, retryAfter: 5 }
+    return { success, retryAfter: Math.ceil((reset - Date.now()) / 1000) }
+  } catch {
+    // Redis errored — fail closed rather than letting the request through.
+    return { success: false, retryAfter: 5 }
+  }
 }