Skip to content

Rate limiter: replace check-then-act with an atomic Redis Lua script #7

Description

@stxkxs

Deferred from the quality-audit fix pass (#6). Tracking the standing systems-correctness gap.

Location: src/ratelimit/redis-limiter.ts:61-101

Problem (TOCTOU): the sliding-window limiter reads the count and writes the new entry in two separate, non-atomic Redis pipelines — zremrangebyscore+zcard (read, lines 61-69), then a decision, then zadd+expire (write, lines 95-101). Between the read and the write, N concurrent requests for the same key can all observe count < limit before any zadd lands, so they all pass and burst past the configured cap.

Impact: bounded — the limiter is fail-open by design and a throttle (not auth), so overshoot is bounded by concurrency, not unbounded. But it is below production-correct.

Proposed fix: collapse to a single server-side EVAL Lua script per key that does ZREMRANGEBYSCORE (prune) → ZCARD (count) → compare → and only if under-limit ZADD+EXPIRE, returning {allowed, remaining, resetAt}. Add eval() to RateLimiterRedisPort; rewrite redis-limiter.test.ts to drive the fake eval.

Why deferred: a Lua rewrite of a tested, working limiter can't be validated against a real Redis in the dev/CI environment here; shipping an unvalidated rate-limiter script risks either blocking everyone or limiting nothing in prod. Needs a live Redis to verify.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething in a shipped artifact is brokenenhancementNew capability or improvement

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions