Real-time global intelligence dashboard. AI-powered news aggregation, geopolitical monitoring, and infrastructure tracking in a unified situational awareness interface
fix(rate-limit): audible fail-closed posture + drop spoofable XFF identity (#3564)
* fix(rate-limit): audible fail-closed posture + drop spoofable XFF identity (#3531)
Two related rate-limit hygiene gaps from manual review:
M9 — Silent fail-open on Upstash error
The catch in api/_rate-limit.js + server/_shared/rate-limit.ts swallowed every
Redis transport / auth error and returned null = "allow," so any Upstash blip
silently lifted every per-IP and per-endpoint budget across the site.
- Always log a structured `[rate-limit] redis-error stage=...` line so the
bypass window is visible in Vercel logs / Sentry instead of disappearing.
- api/ mirror also captures to Sentry (fingerprint
`['rate-limit','redis-error',stage]`) so per-stage incidents stay one
issue rather than fragmenting.
- Add `RateLimitOptions.failClosed`. When true (and Redis is down), return
503 with `X-RateLimit-Mode: degraded` + `Retry-After: 5` so reverse
proxies / observability can correlate the unavailability window.
- `checkEndpointRateLimit` defaults `failClosed=true` — those policies
exist precisely because the limit IS the abuse defence (3/hr lead
capture, LLM endpoints, sanctions lookup); a Redis blip must not
silently lift them.
- `checkRateLimit` (global per-IP) and `checkScopedRateLimit` keep the
fail-open default to preserve availability-first posture; scoped
callers can now read `degraded: true` to escalate locally.
M16 — Spoofable x-forwarded-for as IP fallback
`getClientIp` listed XFF as a third fallback in BOTH rate-limit.ts and
turnstile.ts, contradicting their own comments. A direct request bypassing
Cloudflare could rotate identities by toggling the header and beat the
per-IP window.
- Drop the XFF fallback in all three getClientIp implementations.
- Return `UNKNOWN_CLIENT_IP = 'unknown'` sentinel when no trusted
cf-connecting-ip / x-real-ip header is present, so Upstash treats the
whole untrusted-identity population as one shared bucket (naturally
restrictive instead of silently bypassed).
- Trim each header value before falling through so a whitespace-only
cf-connecting-ip can't short-circuit past x-real-ip.
Tests
- tests/rate-limit.test.mts (new): IP-header preference, XFF refusal,
sentinel fallback, fail-open log shape, fail-closed degraded marker,
per-endpoint default-fail-closed, scoped degraded flag.
- api/_rate-limit.test.mjs (new): same coverage on the Vercel edge mirror
+ cross-surface degraded-marker parity assertion.
- tests/turnstile.test.mjs: reorder existing test to the new
cf-connecting-ip-first preference; add coverage for the XFF refusal
sentinel.
Verified: typecheck, biome lint, lint:rate-limit-policies, gateway/scenario
adjacent suites all green.
* fix(chat-analyst): pass failClosed:true so LLM endpoint doesn't fail-open on Redis blip (#3531)
api/chat-analyst.ts is a Pro-only streaming LLM analyst that doesn't go
through gateway.ts checkEndpointRateLimit, so the M9 fix-default of
fail-CLOSED on the per-endpoint helper didn't reach it. The bare
`checkRateLimit(req, corsHeaders)` call inherited the global helper's
fail-OPEN default — meaning a Redis outage silently lifted the only
rate-limit gate this expensive endpoint has.
Pass `{ failClosed: true }` explicitly. Add a static-analysis guard in
tests/rate-limit.test.mts that asserts the call site keeps the flag, so
a future revert is caught in CI rather than during a Redis incident.
The guard is extensible — add more high-cost direct callers to
FAIL_CLOSED_REQUIRED as they're identified.
* fix(rate-limit): report server degraded limiter to Sentry
* fix(rate-limit): audit scoped degraded callers
* test(rate-limit): use git grep, not rg, for CI portability
`rg` is not installed on the default `ubuntu-latest` GitHub Actions
runner, so the scoped-caller audit test fails with `spawnSync rg ENOENT`
in CI while passing locally where dev machines have ripgrep installed.
`git grep -lF` is always available wherever the repo is checked out,
has the same -l file-list semantics, and -F removes the need to escape
the `(` in the pattern. Same behaviour on dev and CI. E
Elie Habib committed
f492ccc2b850109100074b0bf9d0fb3fb9c39c31
Parent: 042a514
Committed by GitHub <noreply@github.com>
on 5/26/2026, 7:18:55 PM