fix: make rate limiter fail closed when Redis is unreachable

__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)
+  })
+})

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 }
+  }
 }