fix(rate-limit): verify node_proxy JWT before skipping limiters (#1647)

Forged scope claims must not bypass the global or polling limiters; only
unexpired node_proxy tokens signed with this instance's secret skip.

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 () => {

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;
   }