Skip to content

fix(cache): make DistributedCache.incr() fail closed on Redis errors#6144

Open
taniy8 wants to merge 2 commits into
JhaSourav07:mainfrom
taniy8:fix/distributed-cache-incr-fail-open
Open

fix(cache): make DistributedCache.incr() fail closed on Redis errors#6144
taniy8 wants to merge 2 commits into
JhaSourav07:mainfrom
taniy8:fix/distributed-cache-incr-fail-open

Conversation

@taniy8

@taniy8 taniy8 commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Description

Fixes #6138

Problem

DistributedCache.incr() in lib/cache.ts is the core primitive behind
every rate limiter in the app (lib/rate-limit.ts calls cache.incr() for
all per-IP rate limit checks). When the Redis call failed (network error,
timeout, Upstash outage), the catch block silently fell back to the LOCAL
in-memory TTLCache as if it were the authoritative distributed counter.

Serverless instances don't share memory, so each instance would maintain
its own independent counter starting from 0 during any Redis blip. With
N concurrent instances, an attacker effectively gets N times the intended
rate limit defeating rate limiting across every protected endpoint
(/api/achievements, /api/compare, /api/og, /api/user-details, /api/ci-analytics,
/api/notify, /api/webhook, etc.) for the duration of the Redis instability.

Fix

Changed incr() to fail closed on Redis errors instead of failing open.
On failure, it now returns Number.MAX_SAFE_INTEGER, which
RateLimiter.check() already correctly treats as "limit exceeded"
(count > this.limit), no changes needed elsewhere. During a Redis
outage, requests are now blocked rather than silently allowed through
unlimited, which is the safer default for a security control.

Added a test simulating two separate DistributedCache instances
(representing two serverless instances) both calling incr() on the
same key while Redis is down, verifying both independently fail closed
rather than each starting an unsynced local counter.

Pillar

  • 🎨 Pillar 1 — New Theme Design
  • 📐 Pillar 2 — Geometric SVG Improvement
  • 🕐 Pillar 3 — Timezone Logic Optimization
  • 🛠️ Other (Bug fix, refactoring, docs)

Visual Preview

N/A

Checklist before requesting a review:

  • I have read the CONTRIBUTING.md file.
  • I have tested these changes locally (localhost:3000/api/streak?user=YOUR_USERNAME).
  • I have run npm run format and npm run lint locally and resolved all errors (CI will fail otherwise).
  • My commits follow the Conventional Commits format (e.g., feat(themes): ..., fix(calculate): ...).
  • I have updated README.md if I added a new theme or URL parameter.
  • I have started the repo.
  • I have made sure that i have only one commit to merge in this PR.
  • The SVG output matches the CommitPulse "premium quality" aesthetic standard (no raw elements, smooth animations, correct fonts).
  • (Recommended) I joined the CommitPulse Discord community for contributor discussions, mentorship, and faster PR support.

@vercel

vercel Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

@taniy8 is attempting to deploy a commit to the jhasourav07's projects Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions

github-actions Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

📦 Next.js Bundle Size Report (Gzipped Sizes)

✨ No significant bundle size changes detected.

📊 Summary of Totals

Category PR Size Base Size Difference
Total JS 3697.00 KB 3697.00 KB 0 B
Total CSS 296.58 KB 296.58 KB 0 B

@Aamod-Dev Aamod-Dev left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent catch on the fail-open vulnerability. Returning Number.MAX_SAFE_INTEGER inside DistributedCache.incr() is a very clean way to force the rate limiter to deny requests during a Redis outage, preventing the N-instance amplification attack vector. The lib/cache.test.ts update clearly proves the fail-closed behavior across mock instances. Approved!

@Aamod-Dev Aamod-Dev added GSSoC 2026 mentor:Aamod007 level:advanced Complex contributions involving architecture, optimization, or significant feature work quality:clean PR follows clean coding practices, proper formatting, documentation, and maintainability standards. security bug Something isn't working labels Jun 21, 2026
@github-actions github-actions Bot added the type:bug Something isn't working as expected label Jun 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working GSSoC 2026 level:advanced Complex contributions involving architecture, optimization, or significant feature work mentor:Aamod007 quality:clean PR follows clean coding practices, proper formatting, documentation, and maintainability standards. security type:bug Something isn't working as expected

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: DistributedCache.incr() silently falls back to per-instance local counter on Redis failure, defeating all rate limiting

2 participants