Skip to content

Commit 35e5b21

Browse files
authored
Merge pull request #246 from wheval/fix/issues-116
fix(security): add Redis-backed tiered rate limiting (#116)
2 parents e7d0c2e + d191a09 commit 35e5b21

3 files changed

Lines changed: 261 additions & 16 deletions

File tree

PR_DESCRIPTION.md

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,45 @@
11
## Summary
2+
3+
Implements **distributed rate limiting** for the tiered API limiter using **Redis** when `REDIS_URL` is configured. This removes the bypass where limits lived only in process memory (restart or horizontal scaling cleared or split enforcement).
4+
5+
## Purpose / Motivation
6+
7+
Issue [#116](https://github.com/nathydre21/wata-board/issues/116): in-memory rate limiting could be bypassed by restarting the server or routing traffic across multiple instances. Production deployments need a shared store so limits are consistent and durable across replicas.
8+
9+
## Changes Made
10+
11+
- **`TieredRateLimiter`** (`backend/src/middleware/rateLimiter.ts`):
12+
- When Redis is enabled (`REDIS_URL`), uses Redis sorted sets for a sliding window of request timestamps and a separate counter key for queue depth.
13+
- Uses **Lua scripts** so check/consume and status reads are atomic and safe under concurrency.
14+
- Keeps the previous **in-memory** behavior when Redis is not configured (e.g. local dev/tests without Redis).
15+
- Exposes `resetTime` as ISO strings aligned with `TierRateLimitStatus`.
16+
- Middleware is **async**, forwards limiter errors via `next(error)` instead of failing silently.
17+
- **Monitoring** (`backend/src/routes/monitoring.ts`): `GET /api/monitoring/rate-limit-status` awaits async `getStatus()`.
18+
19+
## Operational notes
20+
21+
- Set **`REDIS_URL`** in environments where multiple app instances run or restarts must not reset limits (same variable as existing Redis pub/sub for WebSockets).
22+
- Keys use the prefix `rl:tier:requests:` and `rl:tier:queue:` per user id derived from `x-user-id` or IP.
23+
24+
## How to Test
25+
26+
1. **With Redis**: Point `REDIS_URL` at a Redis instance, start the API, and hit `/api/payment` (or any route behind `tieredRateLimiter`) until `429` / queued responses; repeat from another client or after restart — counts should **continue** from shared state, not reset per process.
27+
2. **Without Redis**: Unset `REDIS_URL` — behavior should match prior in-memory limiting for a single process.
28+
3. **Monitoring**: `GET /api/monitoring/rate-limit-status` with `x-user-id` should return tier + limit fields without throwing.
29+
30+
## Breaking Changes
31+
32+
None intended. Response shapes and HTTP status codes for rate limit / queue paths are unchanged; Redis is additive behind configuration.
33+
34+
## Related Issues
35+
36+
Closes nathydre21/wata-board#116
37+
38+
## Checklist
39+
40+
- [x] Implementation scoped to tiered limiter + monitoring route
41+
- [ ] CI / full backend build (repo may have pre-existing TS issues outside this change)
42+
- [ ] Redis connectivity verified in staging
243
This PR resolves three related reliability issues in one delivery:
344

445
- `#120` Missing Error Recovery Mechanism

backend/src/middleware/rateLimiter.ts

Lines changed: 218 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { UserTier, TierRateLimitStatus } from '../types/userTier';
1212
import { getRateLimitForTier } from '../config/rateLimits';
1313
import { userTierService } from '../services/userTierService';
1414
import logger from '../utils/logger';
15+
import { getPublisher, isRedisEnabled } from '../utils/redis';
1516

1617
interface WindowEntry {
1718
timestamps: number[];
@@ -21,8 +22,11 @@ interface WindowEntry {
2122
export class TieredRateLimiter {
2223
private windows: Map<string, WindowEntry> = new Map();
2324
private cleanupInterval: NodeJS.Timeout;
25+
private redisEnabled: boolean;
2426

2527
constructor() {
28+
this.redisEnabled = isRedisEnabled();
29+
2630
// Prune stale entries every 2 minutes
2731
this.cleanupInterval = setInterval(() => this.cleanup(), 2 * 60 * 1000);
2832
}
@@ -32,12 +36,16 @@ export class TieredRateLimiter {
3236
/**
3337
* Check (and consume) one request slot for a user.
3438
*/
35-
checkLimit(userId: string): TierRateLimitStatus {
39+
async checkLimit(userId: string): Promise<TierRateLimitStatus> {
3640
const tier = userTierService.getUserTier(userId);
3741
const config = getRateLimitForTier(tier);
3842
const now = Date.now();
3943
const windowStart = now - config.windowMs;
4044

45+
if (this.redisEnabled) {
46+
return this.checkLimitRedis(userId, tier, config, now);
47+
}
48+
4149
let entry = this.windows.get(userId);
4250
if (!entry) {
4351
entry = { timestamps: [], queueCount: 0 };
@@ -94,10 +102,15 @@ export class TieredRateLimiter {
94102
/**
95103
* Read-only status check (does NOT consume a request slot).
96104
*/
97-
getStatus(userId: string): TierRateLimitStatus {
105+
async getStatus(userId: string): Promise<TierRateLimitStatus> {
98106
const tier = userTierService.getUserTier(userId);
99107
const config = getRateLimitForTier(tier);
100108
const now = Date.now();
109+
110+
if (this.redisEnabled) {
111+
return this.getStatusRedis(userId, tier, config, now);
112+
}
113+
101114
const windowStart = now - config.windowMs;
102115

103116
const entry = this.windows.get(userId);
@@ -124,6 +137,12 @@ export class TieredRateLimiter {
124137
// ── Express middleware factory ─────────────────────────────
125138

126139
middleware() {
140+
return async (req: Request, res: Response, next: NextFunction) => {
141+
try {
142+
const userId =
143+
(req.headers['x-user-id'] as string) || req.ip || 'unknown';
144+
const status = await this.checkLimit(userId);
145+
const resetAtMs = Date.parse(status.resetTime);
127146
return (req: Request, res: Response, next: NextFunction) => {
128147
const userId =
129148
(req.headers['x-user-id'] as string) || req.ip || 'unknown';
@@ -150,26 +169,55 @@ export class TieredRateLimiter {
150169
});
151170
}
152171

153-
if (status.queued) {
154-
logger.info('Request queued', {
155-
userId,
156-
tier: status.tier,
157-
position: status.queuePosition,
158-
});
159-
return res.status(202).json({
160-
message: 'Request queued',
161-
queuePosition: status.queuePosition,
162-
tier: status.tier,
163-
});
164-
}
172+
// Always expose rate-limit headers
173+
res.set('X-RateLimit-Limit', String(status.limit));
174+
res.set('X-RateLimit-Remaining', String(status.remainingRequests));
175+
res.set(
176+
'X-RateLimit-Reset',
177+
String(Math.ceil(resetAtMs / 1000)),
178+
);
179+
res.set('X-RateLimit-Tier', status.tier);
180+
181+
if (!status.allowed && !status.queued) {
182+
logger.warn('Rate limit exceeded', { userId, tier: status.tier });
183+
return res.status(429).json({
184+
error: 'Rate limit exceeded',
185+
tier: status.tier,
186+
retryAfter: Math.ceil((resetAtMs - Date.now()) / 1000),
187+
limit: status.limit,
188+
});
189+
}
165190

191+
if (status.queued) {
192+
logger.info('Request queued', {
193+
userId,
194+
tier: status.tier,
195+
position: status.queuePosition,
196+
});
197+
return res.status(202).json({
198+
message: 'Request queued',
199+
queuePosition: status.queuePosition,
200+
tier: status.tier,
201+
});
202+
}
203+
204+
return next();
205+
} catch (error) {
206+
logger.error('Rate limiter middleware failure', { error });
207+
return next(error);
208+
}
166209
return next();
167210
};
168211
}
169212

170213
// ── Helpers ────────────────────────────────────────────────
171214

172215
private cleanup() {
216+
if (this.redisEnabled) {
217+
// Redis key expiry handles cleanup in distributed mode.
218+
return;
219+
}
220+
173221
const now = Date.now();
174222
for (const [userId, entry] of this.windows.entries()) {
175223
entry.timestamps = entry.timestamps.filter(
@@ -184,6 +232,162 @@ export class TieredRateLimiter {
184232
destroy() {
185233
clearInterval(this.cleanupInterval);
186234
}
235+
236+
private buildRedisKeys(userId: string): { requestsKey: string; queueKey: string } {
237+
return {
238+
requestsKey: `rl:tier:requests:${userId}`,
239+
queueKey: `rl:tier:queue:${userId}`,
240+
};
241+
}
242+
243+
private toStatus(
244+
tier: UserTier,
245+
allowed: number,
246+
remainingRequests: number,
247+
resetTimeMs: number,
248+
queued: number,
249+
queuePosition: number,
250+
limit: number,
251+
): TierRateLimitStatus {
252+
return {
253+
tier,
254+
allowed: allowed === 1,
255+
remainingRequests,
256+
resetTime: new Date(resetTimeMs).toISOString(),
257+
queued: queued === 1,
258+
queuePosition: queuePosition > 0 ? queuePosition : undefined,
259+
limit,
260+
};
261+
}
262+
263+
private async checkLimitRedis(
264+
userId: string,
265+
tier: UserTier,
266+
config: { windowMs: number; maxRequests: number; queueSize: number },
267+
now: number,
268+
): Promise<TierRateLimitStatus> {
269+
const client = getPublisher();
270+
const { requestsKey, queueKey } = this.buildRedisKeys(userId);
271+
const member = `${now}-${Math.random().toString(36).slice(2, 10)}`;
272+
273+
const script = `
274+
local requestsKey = KEYS[1]
275+
local queueKey = KEYS[2]
276+
local now = tonumber(ARGV[1])
277+
local windowMs = tonumber(ARGV[2])
278+
local maxRequests = tonumber(ARGV[3])
279+
local queueSize = tonumber(ARGV[4])
280+
local member = ARGV[5]
281+
local windowStart = now - windowMs
282+
283+
redis.call('ZREMRANGEBYSCORE', requestsKey, '-inf', windowStart)
284+
local count = redis.call('ZCARD', requestsKey)
285+
286+
if count < maxRequests then
287+
redis.call('ZADD', requestsKey, now, member)
288+
redis.call('PEXPIRE', requestsKey, windowMs)
289+
local minData = redis.call('ZRANGE', requestsKey, 0, 0, 'WITHSCORES')
290+
local reset = now + windowMs
291+
if minData[2] then
292+
reset = tonumber(minData[2]) + windowMs
293+
end
294+
local remaining = maxRequests - count - 1
295+
return {1, remaining, reset, 0, 0, maxRequests}
296+
end
297+
298+
local queueCount = redis.call('INCR', queueKey)
299+
if queueCount == 1 then
300+
redis.call('PEXPIRE', queueKey, windowMs)
301+
end
302+
303+
local minData = redis.call('ZRANGE', requestsKey, 0, 0, 'WITHSCORES')
304+
local reset = now + windowMs
305+
if minData[2] then
306+
reset = tonumber(minData[2]) + windowMs
307+
end
308+
309+
if queueCount <= queueSize then
310+
return {0, 0, reset, 1, queueCount, maxRequests}
311+
end
312+
313+
redis.call('DECR', queueKey)
314+
return {0, 0, reset, 0, 0, maxRequests}
315+
`;
316+
317+
const result = (await client.eval(
318+
script,
319+
2,
320+
requestsKey,
321+
queueKey,
322+
String(now),
323+
String(config.windowMs),
324+
String(config.maxRequests),
325+
String(config.queueSize),
326+
member,
327+
)) as [number, number, number, number, number, number];
328+
329+
return this.toStatus(
330+
tier,
331+
Number(result[0]),
332+
Number(result[1]),
333+
Number(result[2]),
334+
Number(result[3]),
335+
Number(result[4]),
336+
Number(result[5]),
337+
);
338+
}
339+
340+
private async getStatusRedis(
341+
userId: string,
342+
tier: UserTier,
343+
config: { windowMs: number; maxRequests: number; queueSize: number },
344+
now: number,
345+
): Promise<TierRateLimitStatus> {
346+
const client = getPublisher();
347+
const { requestsKey } = this.buildRedisKeys(userId);
348+
349+
const script = `
350+
local requestsKey = KEYS[1]
351+
local now = tonumber(ARGV[1])
352+
local windowMs = tonumber(ARGV[2])
353+
local maxRequests = tonumber(ARGV[3])
354+
local windowStart = now - windowMs
355+
356+
redis.call('ZREMRANGEBYSCORE', requestsKey, '-inf', windowStart)
357+
local count = redis.call('ZCARD', requestsKey)
358+
local minData = redis.call('ZRANGE', requestsKey, 0, 0, 'WITHSCORES')
359+
local reset = now + windowMs
360+
if minData[2] then
361+
reset = tonumber(minData[2]) + windowMs
362+
end
363+
364+
local remaining = maxRequests - count
365+
if remaining < 0 then remaining = 0 end
366+
local allowed = 0
367+
if remaining > 0 then allowed = 1 end
368+
369+
return {allowed, remaining, reset, maxRequests}
370+
`;
371+
372+
const result = (await client.eval(
373+
script,
374+
1,
375+
requestsKey,
376+
String(now),
377+
String(config.windowMs),
378+
String(config.maxRequests),
379+
)) as [number, number, number, number];
380+
381+
return this.toStatus(
382+
tier,
383+
Number(result[0]),
384+
Number(result[1]),
385+
Number(result[2]),
386+
0,
387+
0,
388+
Number(result[3]),
389+
);
390+
}
187391
}
188392

189393
/** Singleton instance */

backend/src/routes/monitoring.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,10 @@ router.get('/dashboard', (_req: Request, res: Response) => {
3636
});
3737

3838
/** GET /api/monitoring/rate-limit-status */
39-
router.get('/rate-limit-status', (req: Request, res: Response) => {
39+
router.get('/rate-limit-status', async (req: Request, res: Response) => {
4040
const rawId = (req.headers['x-user-id'] as string) || req.ip || 'unknown';
4141
const userId = sanitizeAlphanumeric(rawId, 100) || 'unknown';
42-
const status = tieredRateLimiter.getStatus(userId);
42+
const status = await tieredRateLimiter.getStatus(userId);
4343
const tierInfo = userTierService.getUserTierInfo(userId);
4444
res.json({ ...status, ...tierInfo });
4545
});

0 commit comments

Comments
 (0)