diff --git a/backend/src/__tests__/rate-limiting.test.ts b/backend/src/__tests__/rate-limiting.test.ts index 9fb00016..38086b63 100644 --- a/backend/src/__tests__/rate-limiting.test.ts +++ b/backend/src/__tests__/rate-limiting.test.ts @@ -120,33 +120,86 @@ describe('Tier 2: Standard endpoints', () => { }); // ── Node proxy bypass ──────────────────────────────────────────────────────── +// Valid node_proxy JWTs skip both limiters. Polling paths (/api/stats) exercise +// pollingLimiter.skip; standard paths (/api/stacks) exercise globalApiLimiter.skip +// (global skip returns early for polling paths before consulting isNodeProxyRequest). describe('Node proxy bypass', () => { - it('requests with valid node_proxy token bypass all rate limiters', async () => { - const nodeToken = jwt.sign({ scope: 'node_proxy' }, TEST_JWT_SECRET, { expiresIn: '1h' }); + function scopedToken( + scope: string, + secret = TEST_JWT_SECRET, + opts: jwt.SignOptions = { expiresIn: '1h' }, + ): string { + return jwt.sign({ scope }, secret, opts); + } + it('valid node_proxy token bypasses the polling limiter on /api/stats', async () => { const res = await request(app) .get('/api/stats') - .set('Authorization', `Bearer ${nodeToken}`); + .set('Authorization', `Bearer ${scopedToken('node_proxy')}`); - // Node proxy requests should NOT receive rate limit headers from either limiter - // (both skip functions return true for node_proxy tokens) // When skipped, express-rate-limit does not set headers expect(res.headers['ratelimit-limit']).toBeUndefined(); }); + it('valid node_proxy token bypasses the global limiter on /api/stacks', async () => { + const res = await request(app) + .get('/api/stacks') + .set('Authorization', `Bearer ${scopedToken('node_proxy')}`); + + expect(res.headers['ratelimit-limit']).toBeUndefined(); + }); + + it('forged node_proxy claim (wrong secret) still hits the polling limiter', async () => { + const res = await request(app) + .get('/api/stats') + .set('Authorization', `Bearer ${scopedToken('node_proxy', 'wrong-secret')}`); + + expect(parseInt(res.headers['ratelimit-limit'], 10)).toBe(3000); + expect(res.status).toBe(401); + }); + + it('forged node_proxy claim (wrong secret) still hits the global limiter', async () => { + const res = await request(app) + .get('/api/stacks') + .set('Authorization', `Bearer ${scopedToken('node_proxy', 'wrong-secret')}`); + + expect(parseInt(res.headers['ratelimit-limit'], 10)).toBe(1000); + expect(res.status).toBe(401); + }); + + it('expired node_proxy token still gets rate limited (fail closed)', async () => { + const expired = scopedToken('node_proxy', TEST_JWT_SECRET, { expiresIn: '-1s' }); + + const polling = await request(app) + .get('/api/stats') + .set('Authorization', `Bearer ${expired}`); + expect(parseInt(polling.headers['ratelimit-limit'], 10)).toBe(3000); + + const standard = await request(app) + .get('/api/stacks') + .set('Authorization', `Bearer ${expired}`); + expect(parseInt(standard.headers['ratelimit-limit'], 10)).toBe(1000); + }); + + it('valid pilot_tunnel token remains rate-limited (not exempt)', async () => { + const res = await request(app) + .get('/api/stacks') + .set('Authorization', `Bearer ${scopedToken('pilot_tunnel')}`); + + expect(parseInt(res.headers['ratelimit-limit'], 10)).toBe(1000); + }); + it('requests with invalid Bearer token still get rate limited', async () => { const res = await request(app) .get('/api/stats') .set('Authorization', 'Bearer invalid-token-here'); // Should still get rate limit headers (polling tier for /stats) - const limit = parseInt(res.headers['ratelimit-limit'], 10); - expect(limit).toBe(3000); + expect(parseInt(res.headers['ratelimit-limit'], 10)).toBe(3000); }); it('requests with non-node_proxy Bearer token still get rate limited', async () => { - // A regular user JWT (not node_proxy scope) should still be rate limited const userToken = jwt.sign( { username: 'testuser', role: 'admin' }, TEST_JWT_SECRET, @@ -157,8 +210,7 @@ describe('Node proxy bypass', () => { .get('/api/stacks') .set('Authorization', `Bearer ${userToken}`); - const limit = parseInt(res.headers['ratelimit-limit'], 10); - expect(limit).toBe(1000); + expect(parseInt(res.headers['ratelimit-limit'], 10)).toBe(1000); }); }); @@ -179,9 +231,12 @@ describe('Hybrid key generator', () => { const remaining1 = parseInt(res1.headers['ratelimit-remaining'], 10); const remaining2 = parseInt(res2.headers['ratelimit-remaining'], 10); - // Each should have its own budget (999 for this test since each made 1 request) - expect(remaining1).toBeGreaterThanOrEqual(990); - expect(remaining2).toBeGreaterThanOrEqual(990); + // Each should have its own budget. Floor is suite-aware: earlier cases in + // this file (including forged/expired node_proxy requests keyed by IP) + // already spent some of the anonymous bucket; independence still holds as + // long as both remain near the 1000 ceiling. + expect(remaining1).toBeGreaterThanOrEqual(980); + expect(remaining2).toBeGreaterThanOrEqual(980); }); it('two different users get independent rate limit budgets', async () => { diff --git a/backend/src/middleware/rateLimiters.ts b/backend/src/middleware/rateLimiters.ts index ba107875..6fc2302c 100644 --- a/backend/src/middleware/rateLimiters.ts +++ b/backend/src/middleware/rateLimiters.ts @@ -30,15 +30,20 @@ const POLLING_EXEMPT_PATHS = new Set([ type CachedProxyFlagReq = Request & { _isNodeProxy?: boolean }; +/** Claims read from a verified session/bearer JWT on the rate-limiter path. */ +type VerifiedJwtClaims = { username?: string; sub?: string; scope?: string }; + /** - * True when the request bears a node_proxy Bearer token. Uses `jwt.decode()` - * (no signature verification) to keep the hot path cheap; `authMiddleware` - * verifies signatures downstream. Worst case for a forged token: it bypasses - * the rate limiter but is still rejected at auth. Result is memoized on the - * request object so sequential limiters don't repeat the work. + * True when the request bears a verified node_proxy Bearer JWT. Signature and + * expiry are checked via `verifiedJwtPayload` against this instance's cached + * signing secret (the same secret authMiddleware uses downstream). Forged, + * expired, or otherwise unverifiable credentials fail closed: no skip, so the + * request stays in the normal per-IP bucket. Only exact `scope === 'node_proxy'` + * skips; `pilot_tunnel` and other machine scopes remain rate-limited. Result is + * memoized on the request object so sequential limiters don't repeat the work. */ function isNodeProxyRequest(req: Request): boolean { - const cached = (req as CachedProxyFlagReq); + const cached = req as CachedProxyFlagReq; if (cached._isNodeProxy !== undefined) return cached._isNodeProxy; const auth = req.headers.authorization; if (!auth?.startsWith('Bearer ')) { @@ -47,20 +52,13 @@ function isNodeProxyRequest(req: Request): boolean { } const bearer = auth.slice(7); // Opaque API tokens are never node_proxy credentials, and they are not - // JWTs — short-circuit so `jwt.decode` is never invoked on them. + // JWTs; short-circuit so verification is never invoked on them. if (looksLikeApiToken(bearer)) { cached._isNodeProxy = false; return false; } - try { - const decoded = jwt.decode(bearer) as { scope?: string } | null; - const result = decoded?.scope === 'node_proxy'; - cached._isNodeProxy = result; - return result; - } catch { - cached._isNodeProxy = false; - return false; - } + cached._isNodeProxy = verifiedJwtPayload(bearer)?.scope === 'node_proxy'; + return cached._isNodeProxy; } /** @@ -70,15 +68,16 @@ function isNodeProxyRequest(req: Request): boolean { * cache (no per-call DB hit) and `jwt.verify` costs microseconds, so this is safe * on the rate-limiter hot path. Verifying here is what stops one source from * fragmenting the limiter by rotating a forged JWT with a varying username: a - * credential that does not verify yields no per-user key. Fails closed: any error - * returns null, so the caller degrades to per-IP keying rather than throwing out - * of the key generator. + * credential that does not verify yields no per-user key. The same helper gates + * the node_proxy limiter skip so a forged scope claim cannot bypass limiting. + * Fails closed: any error returns null, so callers degrade to per-IP keying / + * no skip rather than throwing out of the hot path. */ -function verifiedJwtPayload(token: string): { username?: string; sub?: string } | null { +function verifiedJwtPayload(token: string): VerifiedJwtClaims | null { try { const secret = DatabaseService.getInstance().getGlobalSettings().auth_jwt_secret; if (!secret) return null; - return jwt.verify(token, secret) as { username?: string; sub?: string }; + return jwt.verify(token, secret) as VerifiedJwtClaims; } catch { return null; }