fix: fail closed when no shared cache is configured in production

src/app/actions/leaderboard.test.ts

@@ -28,6 +28,8 @@ vi.mock('@/lib/cache', () => ({
   cacheGet: mocks.mockCacheGet,
   cacheSet: mocks.mockCacheSet,
   cacheRateLimitHitSlidingWindow: mocks.mockCacheRateLimitHitSlidingWindow,
+  isProductionDeploy: () => false,
+  isSharedCacheAvailable: () => true,
 }));
 
 vi.mock('@/lib/github/app', () => ({

src/lib/cache.test.ts

@@ -12,6 +12,7 @@ import {
 
 const redisRegistry = vi.hoisted(() => ({
   instance: null as any,
+  options: null as any,
   handlers: new Map<string, (err?: Error) => void>(),
 }));
 
@@ -28,8 +29,9 @@ vi.mock('ioredis', () => ({
     incr = vi.fn().mockResolvedValue(1);
     expire = vi.fn().mockResolvedValue(1);
     ttl = vi.fn().mockResolvedValue(60);
-    constructor() {
+    constructor(_url: string, options?: unknown) {
       redisRegistry.instance = this;
+      redisRegistry.options = options;
     }
   },
 }));
@@ -337,3 +339,120 @@ describe('pickDefaultBackend (REDIS_URL path) — regression for #843', () => {
     }
   });
 });
+
+describe('pickDefaultBackend fail-closed (regression for #861)', () => {
+  function withEnv(
+    env: Record<string, string | undefined>,
+    fn: () => Promise<void>,
+  ): Promise<void> {
+    const prev = new Map<string, string | undefined>();
+    return (async () => {
+      for (const [k, v] of Object.entries(env)) {
+        prev.set(k, process.env[k]);
+        if (v === undefined) delete process.env[k];
+        else process.env[k] = v;
+      }
+      try {
+        await fn();
+      } finally {
+        for (const [k, v] of prev) {
+          if (v === undefined) delete process.env[k];
+          else process.env[k] = v;
+        }
+      }
+    })();
+  }
+
+  it('blocks all rate-limit hits on a production deploy with no shared cache', async () => {
+    await withEnv(
+      {
+        KV_REST_API_URL: undefined,
+        KV_REST_API_TOKEN: undefined,
+        REDIS_URL: undefined,
+        VERCEL_ENV: 'production',
+        NODE_ENV: 'production',
+      },
+      async () => {
+        vi.resetModules();
+        const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
+        try {
+          const cache = await import('./cache');
+
+          const fixed = await cache.cacheRateLimitHit('rl:prod', 60, 1000);
+          expect(fixed.count).toBe(Number.MAX_SAFE_INTEGER);
+          expect(fixed.resetAt).toBe(1000 + 60 * 1000);
+
+          const sliding = await cache.cacheRateLimitHitSlidingWindow('rl:prod', 60, 5, 2000);
+          expect(sliding.count).toBe(Number.MAX_SAFE_INTEGER);
+          expect(sliding.resetAt).toBe(2000 + 60 * 1000);
+        } finally {
+          errorSpy.mockRestore();
+        }
+      },
+    );
+  });
+
+  it('makes non-rate-limit cache ops safe no-ops when failing closed', async () => {
+    await withEnv(
+      {
+        KV_REST_API_URL: undefined,
+        KV_REST_API_TOKEN: undefined,
+        REDIS_URL: undefined,
+        VERCEL_ENV: 'production',
+        NODE_ENV: 'production',
+      },
+      async () => {
+        vi.resetModules();
+        const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
+        try {
+          const cache = await import('./cache');
+          await cache.cacheSet('k', 'v', 60);
+          expect(await cache.cacheGet('k')).toBeNull();
+          await cache.cacheDel('k');
+          await cache.cacheDelByPrefix('k:');
+        } finally {
+          errorSpy.mockRestore();
+        }
+      },
+    );
+  });
+
+  it('keeps the memory backend on a Vercel preview deploy without a shared cache', async () => {
+    await withEnv(
+      {
+        KV_REST_API_URL: undefined,
+        KV_REST_API_TOKEN: undefined,
+        REDIS_URL: undefined,
+        VERCEL_ENV: 'preview',
+        NODE_ENV: 'production',
+      },
+      async () => {
+        vi.resetModules();
+        const cache = await import('./cache');
+        const result = await cache.cacheRateLimitHit('rl:preview', 60, 1000);
+        expect(result.count).toBe(1);
+      },
+    );
+  });
+
+  it('configures a bounded reconnect retryStrategy instead of giving up permanently', async () => {
+    await withEnv({ REDIS_URL: 'redis://localhost:6379' }, async () => {
+      vi.resetModules();
+      const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
+      try {
+        await import('./cache');
+        expect(redisRegistry.options).not.toBeNull();
+        const retryStrategy = redisRegistry.options?.retryStrategy as (
+          times: number,
+        ) => number | null;
+        expect(retryStrategy).toBeTypeOf('function');
+        expect(retryStrategy(1)).toBeGreaterThanOrEqual(250);
+        expect(retryStrategy(1)).toBeLessThanOrEqual(5000);
+        expect(retryStrategy(10)).toBeLessThanOrEqual(5000);
+        expect(retryStrategy(11)).toBeNull();
+      } finally {
+        warnSpy.mockRestore();
+      }
+    });
+  });
+});

src/lib/cache.ts

@@ -3,6 +3,11 @@
  * Tests + local dev = in-memory map (no network, deterministic).
  *
  * Swap providers later by replacing the backend below — call sites never change.
+ *
+ * Fail-closed: if a real production deploy has no distributed backend configured,
+ * rate limiting must NOT silently degrade to a per-invocation MemoryBackend (which
+ * disables webhook/action throttling on serverless). Instead every rate-limit hit
+ * reports over the limit so calls are blocked, and other cache ops become safe no-ops.
  */
 
 import { Redis as UpstashRedis } from '@upstash/redis';
@@ -107,6 +112,36 @@ class MemoryBackend implements CacheBackend {
   }
 }
 
+/**
+ * Fail-closed backend for a production deploy with no shared cache configured.
+ * Every rate-limit hit reports over the limit (blocking), and all other cache
+ * operations are safe no-ops so nothing reads stale or cross-tenant state.
+ */
+class BlockingBackend implements CacheBackend {
+  async get<T>(): Promise<T | null> {
+    return null;
+  }
+
+  async set(): Promise<void> {}
+
+  async del(): Promise<void> {}
+
+  async scanDel(): Promise<void> {}
+
+  async rateLimitHit(_key: string, windowSec: number, now: number): Promise<RateLimitBucket> {
+    return blockedRateLimitBucket(windowSec, now);
+  }
+
+  async rateLimitHitSlidingWindow(
+    _key: string,
+    windowSec: number,
+    _limit: number,
+    now: number,
+  ): Promise<RateLimitBucket> {
+    return blockedRateLimitBucket(windowSec, now);
+  }
+}
+
 export class UpstashBackend implements CacheBackend {
   constructor(private redis: UpstashRedis) {}
 
@@ -291,7 +326,13 @@ function pickDefaultBackend(): CacheBackend {
   if (redisUrl) {
     const client = new Redis(redisUrl, {
       maxRetriesPerRequest: 1,
-      retryStrategy: () => null, // Do not keep retrying connection
+      // Reconnect with bounded backoff instead of giving up permanently: a
+      // transient Redis blip must not kill the client (and with it rate
+      // limiting) for the life of a warm serverless instance.
+      retryStrategy: (times: number) => {
+        if (times > 10) return null;
+        return Math.min(times * 250, 5000);
+      },
     });
     client.on('error', (err: Error) => {
       console.warn(
@@ -301,23 +342,36 @@ function pickDefaultBackend(): CacheBackend {
     return new IoRedisBackend(client);
   }
 
-  // No distributed backend configured. Fall back to in-process MemoryBackend.
-  // In Vercel (and any other serverless runtime), each function invocation runs
-  // in an isolated process with its own memory, so counters are never shared
-  // across concurrent invocations. Rate limiting is effectively disabled.
-  // Set KV_REST_API_URL + KV_REST_API_TOKEN (Upstash) or REDIS_URL to enable
-  // shared, durable rate limiting.
-  if (process.env.NODE_ENV === 'production') {
+  if (isProductionDeploy()) {
+    // No distributed backend on a real production deploy. Do NOT silently
+    // degrade to MemoryBackend: each serverless invocation would get its own
+    // counter, effectively disabling webhook/action throttling. Fail closed so
+    // rate limiting blocks every call and nothing runs unthrottled.
     console.error(
-      '[cache] MISCONFIGURATION: No Redis or Upstash backend is configured. ' +
-        'Falling back to MemoryBackend. Rate limiting is NOT shared across ' +
-        'serverless invocations and is effectively disabled in production. ' +
-        'Set KV_REST_API_URL + KV_REST_API_TOKEN (Upstash) or REDIS_URL.',
+      '[cache] CRITICAL MISCONFIGURATION: No Redis or Upstash backend is configured on a production deploy. ' +
+        'Rate limiting is failing CLOSED (all calls blocked). ' +
+        'Set KV_REST_API_URL + KV_REST_API_TOKEN (Upstash) or REDIS_URL immediately.',
     );
+    return new BlockingBackend();
   }
+
+  // No distributed backend outside a production deploy (local dev, tests, Vercel
+  // preview). Use an in-process MemoryBackend — deterministic for tests, and fine
+  // for ephemeral dev/preview traffic.
   return new MemoryBackend();
 }
 
+/**
+ * True on a real production deploy. `next build` sets NODE_ENV=production on
+ * Vercel Preview deployments too, so gate on VERCEL_ENV first (only set to
+ * 'production' on production deploys) and fall back to NODE_ENV off-Vercel.
+ */
+export function isProductionDeploy(): boolean {
+  return process.env.VERCEL_ENV
+    ? process.env.VERCEL_ENV === 'production'
+    : process.env.NODE_ENV === 'production';
+}
+
 /** True when a distributed cache backend (Upstash or Redis) is configured. */
 export function isSharedCacheAvailable(): boolean {
   const hasUpstash = Boolean(process.env.KV_REST_API_URL) && Boolean(process.env.KV_REST_API_TOKEN);

src/lib/rate-limit.ts

@@ -1,5 +1,6 @@
 import {
   cacheRateLimitHitSlidingWindow,
+  isProductionDeploy,
   isSharedCacheAvailable,
   blockedRateLimitBucket,
 } from './cache';
@@ -25,17 +26,6 @@ export type RateLimitResult = {
   resetAt: number;
 };
 
-/**
- * True on a real production deploy. `next build` sets NODE_ENV=production on
- * Vercel Preview deployments too, so gate on VERCEL_ENV first (only set to
- * 'production' on production deploys) and fall back to NODE_ENV off-Vercel.
- */
-function isProductionDeploy(): boolean {
-  return process.env.VERCEL_ENV
-    ? process.env.VERCEL_ENV === 'production'
-    : process.env.NODE_ENV === 'production';
-}
-
 /**
  * Sliding-window counter. Every hit in the trailing `windowSec` seconds counts,
  * so a caller can't spend a full budget at the end of one window and another at