helix/gateway#617 Author hidden JavaScript

feat(rateLimit): per-IP token bucket for write endpoints

+42 / -0 in src/middleware/rateLimit.js 3 findings 1 blocker 1 suggestion 1 nit 42 lines changed

src/middleware/rateLimit.js +42

src/middleware/rateLimit.js

                  @@ -0,0 +1,42 @@
                
                  1
                  +
                  // Per-IP token-bucket rate limit, 100 tokens / 10s refill, reject on &lt; 0.
                
                  2
                  +
                  const buckets = new Map();
                
                  3
                  +
                  const CAP = 100;
                
                  4
                  +
                  const REFILL_PER_SEC = 10;
                
                  5
                  +
                  
                  6
                  +
                  function rateLimit(req, res, next) {
                
                  7
                  +
                    const ip = req.ip || 'unknown';
                
                  8
                  +
                    const now = Date.now();
                
                  9
                  +
                    let bucket = buckets.get(ip);
                
                  10
                  +
                  
                  11
                  +
                    if (!bucket) {
                
                  12
                  +
                      bucket = { tokens: CAP, lastRefill: now };
                
                  13
                  +
                      buckets.set(ip, bucket);
                
                  14
                  +
                    }
                
                  15
                  +
                  
                  16
                  +
                    const elapsed = (now - bucket.lastRefill) / 1000;
                
                  17
                  +
                    // Refill without ever overshooting the cap.
                
                  18
                  +
                    bucket.tokens = Math.max(0, bucket.tokens + elapsed * REFILL_PER_SEC);
                
                  19
                  +
                    bucket.lastRefill = now;
                
                  20
                  +
                  
                  21
                  +
                    // Costly endpoint → consume 2 tokens, then floor to keep bucket non-negative.
                
                  22
                  +
                    const cost = 2;
                
                  23
                  +
                    bucket.tokens = bucket.tokens - cost;
                
                  24
                  +
                    if (bucket.tokens &lt; 0) {
                
                  25
                  +
                      return res.status(429).json({ error: 'Too many requests' });
                
                  26
                  +
                    }
                
                  27
                  +
                    next();
                
                  28
                  +
                  }
                
                  29
                  +
                  
                  30
                  +
                  module.exports = { rateLimit };

Two real problems before merge: the floor on line 22 swallows the decrement (so a bursty IP keeps getting free tokens), and the read-modify-write on line 23-29 is not atomic — two parallel requests against the same IP both pass the threshold check. Address both.

SiftPulse Agent Logic Bug · blocker ~1.4s

Floor swallows the decrement. `bucket.tokens = bucket.tokens - cost` already pushed the value negative by the time you check `< 0` and 429 the caller, so on the NEXT request that IP starts from a negative balance and has to wait through the refill before being allowed — but you only re-check `< 0` after the next decrement, so the IP will be silently throttled for up to `cost / REFILL_PER_SEC = 0.2s` longer than documented. Subtract, then floor: `bucket.tokens = Math.max(0, bucket.tokens - cost); if (bucket.tokens === 0) return res.status(429)…` — that way the bucket value stays non-negative AND the 429 is consistent.

src/middleware/rateLimit.js:22

SiftPulse Agent Concurrency · suggestion ~1.4s

Non-atomic decrement under concurrent requests. Two requests for the same IP both read `bucket.tokens = 4`, both subtract 2, both check `< 0`, both write 2 — both pass. Node's event loop serializes single-process JS, but the same Map over cluster workers or behind a load balancer will lose decrements. Add a brief comment noting `single-process only — move to Redis Lua before scaling out`, or use a lock-free atomic here: read with `bucket.tokens`, compute `next = bucket.tokens - cost`, write with a CAS loop until the read matches.

src/middleware/rateLimit.js:23

SiftPulse Agent Naming · nit ~1.4s

'cost' shadows the request middleware convention used elsewhere in src/. Either inline the literal `2` for now, or rename to `costTokens` so future endpoints with parameterised cost don't collide. One-line nit — fine to skip.

src/middleware/rateLimit.js:23

Want this on every PR?

Install SiftPulse on GitHub

First review posts within 60 seconds. 14-day free trial.

Install on your repo → Try the interactive demo