From eb5837fbdbd15adfd8a91dad7e15fc6c226d033c Mon Sep 17 00:00:00 2001
From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com>
Date: Thu, 24 Sep 2026 23:14:10 -0700
Subject: [PATCH 1/6] fix(rate-limit): key every limiter on the Cloudflare
 address (TASK-126)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

21 sites in 18 files fell back to `req.ip`: 20 via `ipKeyGenerator(req.ip)`,
and routes/agentHooks.ts:29 on a raw `` `ip:${req.ip || 'unknown'}` `` that no
spelling-based census could see. Behind nginx `req.ip` is a cloudflared pod
address for every external caller, so each of these was one limiter bucket
serving the whole internet. All 21 now call cloudflareIpRateLimitKeyGenerator,
which prefers `cf-connecting-ip` and falls back to `req.ip` only when that
header is absent — unchanged behaviour for a caller that sends none.

The guard for this is a behavioural probe over the loaded app's router stack
(ruled 2026-09-25): two requests differing only in `cf-connecting-ip` must not
share a bucket. A source scan is deliberately NOT the guard here — the census
that missed routes/agentHooks.ts:29 enumerated a spelling and called it the
class, and the same mistake is available to the next scan.

Verified: backend suite 429 suites / 4019 tests; lint:ts 0 errors. The site list
was derived twice, independently (kai from nearest-declaration + brace depth,
vera from nearest-preceding `const <name> =`), and the two agreed exactly.

(cherry picked from commit 661d3b073f3ef6b6078ea03a117a0d55a4eed4a3)
---
 backend/middleware/agentRateLimit.ts    |  9 +++++----
 backend/routes/agentBinding.ts          |  5 +++--
 backend/routes/agentHooks.ts            |  3 ++-
 backend/routes/approvals.ts             |  5 +++--
 backend/routes/credentials.ts           |  5 +++--
 backend/routes/grants.ts                |  5 +++--
 backend/routes/hosted.ts                |  5 +++--
 backend/routes/machines.ts              |  5 +++--
 backend/routes/messages.ts              | 11 ++++++-----
 backend/routes/pg-messages.ts           |  5 +++--
 backend/routes/podInvites.ts            |  5 +++--
 backend/routes/pods.ts                  |  4 ++--
 backend/routes/registry/agent-tokens.ts |  5 +++--
 backend/routes/registry/files.ts        | 10 +++++-----
 backend/routes/spawnCredentials.ts      |  5 +++--
 backend/routes/summaries.ts             |  6 ++++--
 backend/routes/tasksApi.ts              |  5 +++--
 backend/routes/users.ts                 |  6 +++---
 18 files changed, 60 insertions(+), 44 deletions(-)

diff --git a/backend/middleware/agentRateLimit.ts b/backend/middleware/agentRateLimit.ts
index 0af3dbd040..399e62c521 100644
--- a/backend/middleware/agentRateLimit.ts
+++ b/backend/middleware/agentRateLimit.ts
@@ -15,7 +15,7 @@
 
 import type { Request } from 'express';
 import { createHash } from 'crypto';
-import { ipKeyGenerator } from 'express-rate-limit';
+import { cloudflareIpRateLimitKeyGenerator } from './ipRateLimit';
 
 export const agentRateLimitKeyGenerator = (req: Request): string => {
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -25,9 +25,10 @@ export const agentRateLimitKeyGenerator = (req: Request): string => {
   if (typeof auth === 'string' && auth.length > 0) {
     return `hdr:${createHash('sha256').update(auth).digest('hex')}`;
   }
-  // ipKeyGenerator collapses IPv6 to its prefix so a client can't rotate
-  // through its /64 to bypass the limit (ERR_ERL_KEY_GEN_IPV6, #652).
-  return `ip:${req.ip ? ipKeyGenerator(req.ip) : 'unknown'}`;
+  // The Cloudflare-aware helper keys on the client address the edge recorded,
+  // and collapses IPv6 to its prefix so a client can't rotate through its /64
+  // to bypass the limit (ERR_ERL_KEY_GEN_IPV6, #652).
+  return `ip:${cloudflareIpRateLimitKeyGenerator(req)}`;
 };
 
 // CJS compat for the require()-style imports used elsewhere in backend/.
diff --git a/backend/routes/agentBinding.ts b/backend/routes/agentBinding.ts
index 0ac15ef79b..4a1bf80e6f 100644
--- a/backend/routes/agentBinding.ts
+++ b/backend/routes/agentBinding.ts
@@ -5,7 +5,8 @@
 // predicate is enforced HERE from the daemon credential's server-side
 // machineId — never from a caller-supplied value.
 import express from 'express';
-import rateLimit, { ipKeyGenerator } from 'express-rate-limit';
+import rateLimit from 'express-rate-limit';
+import { cloudflareIpRateLimitKeyGenerator } from '../middleware/ipRateLimit';
 import { createHash } from 'crypto';
 import daemonAuth, { DaemonAuthedRequest } from '../middleware/daemonAuth';
 import { GRANT_BROKER_ID, GRANT_BROKER_URL } from '../services/installable/toolInstallables';
@@ -41,7 +42,7 @@ const bindingRateLimit = rateLimit({
     if (authHeader) {
       return `tok:${createHash('sha256').update(authHeader).digest('hex').slice(0, 16)}`;
     }
-    return req.ip ? ipKeyGenerator(req.ip) : 'anon';
+    return cloudflareIpRateLimitKeyGenerator(req as never);
   },
   handler: (_req: unknown, res: { status: (n: number) => { json: (b: unknown) => void } }) => {
     res.status(429).json({ msg: 'rate limit exceeded: 120 binding ops per 60s' });
diff --git a/backend/routes/agentHooks.ts b/backend/routes/agentHooks.ts
index 59c853b8cd..d61b9a69cc 100644
--- a/backend/routes/agentHooks.ts
+++ b/backend/routes/agentHooks.ts
@@ -1,6 +1,7 @@
 export {};
 
 import rateLimit from 'express-rate-limit';
+import { cloudflareIpRateLimitKeyGenerator } from '../middleware/ipRateLimit';
 import { createHash } from 'crypto';
 
 const express = require('express');
@@ -26,7 +27,7 @@ const hookRateLimit = rateLimit({
     const token = String(req.header('x-commonly-agent-token') || req.header('authorization') || '').trim();
     return token
       ? `token:${createHash('sha256').update(token).digest('hex')}`
-      : `ip:${req.ip || 'unknown'}`;
+      : cloudflareIpRateLimitKeyGenerator(req as never);
   },
   handler: (_req: any, res: any) => res.status(429).json({
     code: 'rate_limited',
diff --git a/backend/routes/approvals.ts b/backend/routes/approvals.ts
index 25708a37b2..25135fb045 100644
--- a/backend/routes/approvals.ts
+++ b/backend/routes/approvals.ts
@@ -4,7 +4,8 @@
 // the service as defense in depth.
 // ESM import (not require) so CodeQL's js/missing-rate-limiting query
 // recognises the limiter on the POST route — same pattern as messages.ts.
-import rateLimit, { ipKeyGenerator } from 'express-rate-limit';
+import rateLimit from 'express-rate-limit';
+import { cloudflareIpRateLimitKeyGenerator } from '../middleware/ipRateLimit';
 import { createHash } from 'crypto';
 import type { Request } from 'express';
 
@@ -30,7 +31,7 @@ const approvalResolveLimit = rateLimit({
     if (authHeader) {
       return `apr:${createHash('sha256').update(authHeader).digest('hex').slice(0, 16)}`;
     }
-    return req.ip ? ipKeyGenerator(req.ip) : 'anon';
+    return cloudflareIpRateLimitKeyGenerator(req as never);
   },
   handler: (_req, res) => res.status(429).json({ error: 'rate limit exceeded: 30 approval decisions per 60s' }),
 });
diff --git a/backend/routes/credentials.ts b/backend/routes/credentials.ts
index 2021963e99..61483e2287 100644
--- a/backend/routes/credentials.ts
+++ b/backend/routes/credentials.ts
@@ -5,7 +5,8 @@ import express from 'express';
 import { Types } from 'mongoose';
 // ESM import (not require) so CodeQL's js/missing-rate-limiting query
 // recognizes the limiter (same pattern as routes/messages.ts).
-import rateLimit, { ipKeyGenerator } from 'express-rate-limit';
+import rateLimit from 'express-rate-limit';
+import { cloudflareIpRateLimitKeyGenerator } from '../middleware/ipRateLimit';
 import { createHash } from 'crypto';
 import AgentCredential from '../models/AgentCredential';
 
@@ -28,7 +29,7 @@ const credentialRateLimit = rateLimit({
     if (authHeader) {
       return `tok:${createHash('sha256').update(authHeader).digest('hex').slice(0, 16)}`;
     }
-    return req.ip ? ipKeyGenerator(req.ip) : 'anon';
+    return cloudflareIpRateLimitKeyGenerator(req as never);
   },
   handler: (_req: unknown, res: { status: (n: number) => { json: (b: unknown) => void } }) => {
     res.status(429).json({ msg: 'rate limit exceeded: 60 credential ops per 60s' });
diff --git a/backend/routes/grants.ts b/backend/routes/grants.ts
index 0fa8b5b1ba..122958ddbb 100644
--- a/backend/routes/grants.ts
+++ b/backend/routes/grants.ts
@@ -1,5 +1,6 @@
 import express from 'express';
-import rateLimit, { ipKeyGenerator } from 'express-rate-limit';
+import rateLimit from 'express-rate-limit';
+import { cloudflareIpRateLimitKeyGenerator } from '../middleware/ipRateLimit';
 import { createHash } from 'crypto';
 import { Types } from 'mongoose';
 import Integration from '../models/Integration';
@@ -52,7 +53,7 @@ const grantRateLimit = rateLimit({
   keyGenerator: (req: express.Request): string => {
     const authHeader = req.get('Authorization') || req.get('x-auth-token');
     if (authHeader) return `grant:${createHash('sha256').update(authHeader).digest('hex').slice(0, 16)}`;
-    return req.ip ? ipKeyGenerator(req.ip) : 'anon';
+    return cloudflareIpRateLimitKeyGenerator(req as never);
   },
   handler: (_req, res) => res.status(429).json({ message: 'rate limit exceeded: 60 grant operations per 60s' }),
 });
diff --git a/backend/routes/hosted.ts b/backend/routes/hosted.ts
index 8ea9012614..742ad5648b 100644
--- a/backend/routes/hosted.ts
+++ b/backend/routes/hosted.ts
@@ -13,7 +13,8 @@
  * bypass here; provisioning someone else's agent is not an ops action.
  */
 import express from 'express';
-import rateLimit, { ipKeyGenerator } from 'express-rate-limit';
+import rateLimit from 'express-rate-limit';
+import { cloudflareIpRateLimitKeyGenerator } from '../middleware/ipRateLimit';
 import { createHash } from 'crypto';
 
 const auth = require('../middleware/auth');
@@ -38,7 +39,7 @@ const hostedRateLimit = rateLimit({
     if (authHeader) {
       return `tok:${createHash('sha256').update(authHeader).digest('hex').slice(0, 16)}`;
     }
-    return req.ip ? ipKeyGenerator(req.ip) : 'anon';
+    return cloudflareIpRateLimitKeyGenerator(req as never);
   },
   handler: (_req: unknown, res: express.Response) => {
     res.status(429).json({ message: 'rate limit exceeded: 30 hosted-runtime requests per 60s' });
diff --git a/backend/routes/machines.ts b/backend/routes/machines.ts
index 8de069fff6..584b5f17d2 100644
--- a/backend/routes/machines.ts
+++ b/backend/routes/machines.ts
@@ -1,6 +1,7 @@
 import express from 'express';
 import { createHash } from 'crypto';
-import rateLimit, { ipKeyGenerator } from 'express-rate-limit';
+import rateLimit from 'express-rate-limit';
+import { cloudflareIpRateLimitKeyGenerator } from '../middleware/ipRateLimit';
 import { Types } from 'mongoose';
 import {
   listMachinesForOwner,
@@ -32,7 +33,7 @@ const machineRateLimit = rateLimit({
     if (authHeader) {
       return `tok:${createHash('sha256').update(authHeader).digest('hex').slice(0, 16)}`;
     }
-    return req.ip ? ipKeyGenerator(req.ip) : 'anon';
+    return cloudflareIpRateLimitKeyGenerator(req as never);
   },
   handler: (_req: unknown, res: express.Response) => {
     res.status(429).json({ message: 'rate limit exceeded: 60 machine requests per 60s' });
diff --git a/backend/routes/messages.ts b/backend/routes/messages.ts
index 10b8c4d4ca..fb766f9a44 100644
--- a/backend/routes/messages.ts
+++ b/backend/routes/messages.ts
@@ -1,6 +1,7 @@
 // ESM import (not require) so CodeQL's js/missing-rate-limiting query
 // recognises the limiter on the POST route — same pattern as uploads.ts.
-import rateLimit, { ipKeyGenerator } from 'express-rate-limit';
+import rateLimit from 'express-rate-limit';
+import { cloudflareIpRateLimitKeyGenerator } from '../middleware/ipRateLimit';
 import { createHash } from 'crypto';
 // eslint-disable-next-line global-require
 const express = require('express');
@@ -40,7 +41,7 @@ interface RateLimitRes {
 // for a minute straight) and tight enough to blunt a runaway client looping
 // on a flaky send. Keyed on the Authorization header (hashed) so each user's
 // bearer token gets its own bucket — NAT'd users sharing one office IP don't
-// collide. Falls back to the IPv6-safe ipKeyGenerator for the rare unauth
+// collide. Falls back to the Cloudflare-aware key helper for the rare unauth
 // path. Applied as the FIRST middleware on POST so CodeQL's
 // js/missing-rate-limiting query sees it.
 const sendMessageRateLimit = rateLimit({
@@ -53,7 +54,7 @@ const sendMessageRateLimit = rateLimit({
     if (authHeader) {
       return `tok:${createHash('sha256').update(authHeader).digest('hex').slice(0, 16)}`;
     }
-    return req.ip ? ipKeyGenerator(req.ip) : 'anon';
+    return cloudflareIpRateLimitKeyGenerator(req as never);
   },
   handler: (_req: unknown, res: RateLimitRes) => {
     res.status(429).json({ msg: 'rate limit exceeded: 60 messages per 60s' });
@@ -73,7 +74,7 @@ const readMessageRateLimit = rateLimit({
     if (authHeader) {
       return `tok:${createHash('sha256').update(authHeader).digest('hex').slice(0, 16)}`;
     }
-    return req.ip ? ipKeyGenerator(req.ip) : 'anon';
+    return cloudflareIpRateLimitKeyGenerator(req as never);
   },
   handler: (_req: unknown, res: RateLimitRes) => {
     res.status(429).json({ msg: 'rate limit exceeded: 240 message reads per 60s' });
@@ -100,7 +101,7 @@ const reactionRateLimit = rateLimit({
     if (authHeader) {
       return `rxn:${createHash('sha256').update(authHeader).digest('hex').slice(0, 16)}`;
     }
-    return req.ip ? ipKeyGenerator(req.ip) : 'anon';
+    return cloudflareIpRateLimitKeyGenerator(req as never);
   },
   handler: (_req: unknown, res: RateLimitRes) => {
     res.status(429).json({ msg: 'rate limit exceeded: 120 reactions per 60s' });
diff --git a/backend/routes/pg-messages.ts b/backend/routes/pg-messages.ts
index 4ce9dcedf9..d92578c548 100644
--- a/backend/routes/pg-messages.ts
+++ b/backend/routes/pg-messages.ts
@@ -1,7 +1,8 @@
 // ESM import (not require) so CodeQL's js/missing-rate-limiting query can
 // trace the middleware — it does not follow a rate-limit factory through a
 // require() return. Same shape as routes/messages.ts.
-import rateLimit, { ipKeyGenerator } from 'express-rate-limit';
+import rateLimit from 'express-rate-limit';
+import { cloudflareIpRateLimitKeyGenerator } from '../middleware/ipRateLimit';
 import { createHash } from 'crypto';
 
 // eslint-disable-next-line global-require
@@ -27,7 +28,7 @@ const keyByToken = (req: RateLimitReq) => {
   if (authHeader) {
     return `tok:${createHash('sha256').update(authHeader).digest('hex').slice(0, 16)}`;
   }
-  return req.ip ? ipKeyGenerator(req.ip) : 'anon';
+  return cloudflareIpRateLimitKeyGenerator(req as never);
 };
 
 const readLimit = rateLimit({
diff --git a/backend/routes/podInvites.ts b/backend/routes/podInvites.ts
index a5e011cf0c..9d4c567033 100644
--- a/backend/routes/podInvites.ts
+++ b/backend/routes/podInvites.ts
@@ -7,7 +7,8 @@
 import express from 'express';
 import crypto, { createHash } from 'crypto';
 import mongoose from 'mongoose';
-import rateLimit, { ipKeyGenerator } from 'express-rate-limit';
+import rateLimit from 'express-rate-limit';
+import { cloudflareIpRateLimitKeyGenerator } from '../middleware/ipRateLimit';
 const router = express.Router();
 const auth = require('../middleware/auth');
 const Pod = require('../models/Pod');
@@ -23,7 +24,7 @@ const inviteRateLimitKey = (req: any) => {
   if (authHeader) {
     return `tok:${createHash('sha256').update(authHeader).digest('hex').slice(0, 16)}`;
   }
-  return req.ip ? ipKeyGenerator(req.ip) : 'anon';
+  return cloudflareIpRateLimitKeyGenerator(req as never);
 };
 
 const inviteReadRateLimit = rateLimit({
diff --git a/backend/routes/pods.ts b/backend/routes/pods.ts
index 04607c1032..50764e244a 100644
--- a/backend/routes/pods.ts
+++ b/backend/routes/pods.ts
@@ -1,7 +1,7 @@
 import path from 'path';
 import fs from 'fs';
 import { createHash } from 'crypto';
-import rateLimit, { ipKeyGenerator } from 'express-rate-limit';
+import rateLimit from 'express-rate-limit';
 // eslint-disable-next-line global-require
 const express = require('express');
 // eslint-disable-next-line global-require
@@ -60,7 +60,7 @@ const podJoinRateLimitKey = (req: any) => {
   if (authHeader) {
     return `tok:${createHash('sha256').update(authHeader).digest('hex').slice(0, 16)}`;
   }
-  return req.ip ? ipKeyGenerator(req.ip) : 'anon';
+  return cloudflareIpRateLimitKeyGenerator(req as never);
 };
 
 const podFocusRateLimit = rateLimit({
diff --git a/backend/routes/registry/agent-tokens.ts b/backend/routes/registry/agent-tokens.ts
index 22f5844bb1..583764b17f 100644
--- a/backend/routes/registry/agent-tokens.ts
+++ b/backend/routes/registry/agent-tokens.ts
@@ -4,7 +4,8 @@
 // ESM import (not require) so CodeQL's js/missing-rate-limiting query can
 // trace the middleware; it cannot follow a limiter through a require() return
 // or a router.use wrapper. Same shape as routes/messages.ts.
-import rateLimit, { ipKeyGenerator } from 'express-rate-limit';
+import rateLimit from 'express-rate-limit';
+import { cloudflareIpRateLimitKeyGenerator } from '../../middleware/ipRateLimit';
 import { createHash } from 'crypto';
 
 interface TokenRateReq { get?: (name: string) => string | undefined; ip?: string }
@@ -23,7 +24,7 @@ const tokenRouteLimit = rateLimit({
     if (authHeader) {
       return `tok:${createHash('sha256').update(authHeader).digest('hex').slice(0, 16)}`;
     }
-    return req.ip ? ipKeyGenerator(req.ip) : 'anon';
+    return cloudflareIpRateLimitKeyGenerator(req as never);
   },
   handler: (_req: unknown, res: TokenRateRes) => {
     res.status(429).json({ error: 'rate limit exceeded: 60 token requests per 60s' });
diff --git a/backend/routes/registry/files.ts b/backend/routes/registry/files.ts
index f9eb80ed44..3314770726 100644
--- a/backend/routes/registry/files.ts
+++ b/backend/routes/registry/files.ts
@@ -21,20 +21,20 @@
 // `req.userId` is not set yet, which is why the key generators below hash the
 // Authorization header instead — the idiom `routes/messages.ts` already uses
 // for its pre-auth limiters, and the reason those are clean.
-import rateLimit, { ipKeyGenerator } from 'express-rate-limit';
+import rateLimit from 'express-rate-limit';
+import { cloudflareIpRateLimitKeyGenerator } from '../../middleware/ipRateLimit';
 const { createHash } = require('crypto');
 
 // Per-caller key that works BEFORE `auth` has run. Hashing the raw header
 // keeps one token's budget isolated from another's without leaking the token
-// into rate-limiter state; unauthenticated callers fall back to
-// ipKeyGenerator so IPv6 clients can't rotate within their /64
-// (ERR_ERL_KEY_GEN_IPV6, #652).
+// into rate-limiter state; unauthenticated callers fall back to the
+// Cloudflare-aware helper (IPv6-safe — see middleware/ipRateLimit.ts).
 const preAuthCallerKey = (req: any) => {
   const authHeader = req.get?.('authorization');
   if (authHeader) {
     return `tok:${createHash('sha256').update(authHeader).digest('hex').slice(0, 16)}`;
   }
-  return req.ip ? ipKeyGenerator(req.ip) : 'anon';
+  return cloudflareIpRateLimitKeyGenerator(req as never);
 };
 
 const express = require('express');
diff --git a/backend/routes/spawnCredentials.ts b/backend/routes/spawnCredentials.ts
index e62e3fe6de..3af4cf0394 100644
--- a/backend/routes/spawnCredentials.ts
+++ b/backend/routes/spawnCredentials.ts
@@ -24,7 +24,8 @@
 import express from 'express';
 // ESM import (not require) so CodeQL's js/missing-rate-limiting query recognizes
 // the limiter (same pattern as routes/credentials.ts and routes/messages.ts).
-import rateLimit, { ipKeyGenerator } from 'express-rate-limit';
+import rateLimit from 'express-rate-limit';
+import { cloudflareIpRateLimitKeyGenerator } from '../middleware/ipRateLimit';
 import { createHash } from 'crypto';
 import { Types } from 'mongoose';
 import {
@@ -61,7 +62,7 @@ const spawnCredentialRateLimit = rateLimit({
     if (authHeader) {
       return `tok:${createHash('sha256').update(authHeader).digest('hex').slice(0, 16)}`;
     }
-    return req.ip ? ipKeyGenerator(req.ip) : 'anon';
+    return cloudflareIpRateLimitKeyGenerator(req as never);
   },
   handler: (_req: unknown, res: { status: (n: number) => { json: (b: unknown) => void } }) => {
     res.status(429).json({ message: 'rate limit exceeded: 240 spawn credential ops per 60s' });
diff --git a/backend/routes/summaries.ts b/backend/routes/summaries.ts
index efe00ece3e..02af044d04 100644
--- a/backend/routes/summaries.ts
+++ b/backend/routes/summaries.ts
@@ -17,7 +17,9 @@ const { AgentInstallation } = require('../models/AgentRegistry');
 // eslint-disable-next-line global-require
 const dailyDigestService = require('../services/dailyDigestService');
 // eslint-disable-next-line global-require
-const { rateLimit, ipKeyGenerator } = require('express-rate-limit');
+const { rateLimit } = require('express-rate-limit');
+// eslint-disable-next-line global-require
+const { cloudflareIpRateLimitKeyGenerator } = require('../middleware/ipRateLimit');
 // eslint-disable-next-line global-require
 const { createHash } = require('crypto');
 // eslint-disable-next-line global-require
@@ -49,7 +51,7 @@ const summariesRateLimitKey = (req: { get?: (h: string) => string | undefined; i
   if (authHeader) {
     return `tok:${createHash('sha256').update(authHeader).digest('hex').slice(0, 16)}`;
   }
-  return req.ip ? ipKeyGenerator(req.ip) : 'anon';
+  return cloudflareIpRateLimitKeyGenerator(req as never);
 };
 
 // Reads hit Mongo (and canViewPod adds per-pod lookups) — throttle so a
diff --git a/backend/routes/tasksApi.ts b/backend/routes/tasksApi.ts
index bd1aa826c8..2e828710cf 100644
--- a/backend/routes/tasksApi.ts
+++ b/backend/routes/tasksApi.ts
@@ -1,6 +1,7 @@
 import { createHash } from 'crypto';
 import type { Request, Response } from 'express';
-import rateLimit, { ipKeyGenerator } from 'express-rate-limit';
+import rateLimit from 'express-rate-limit';
+import { cloudflareIpRateLimitKeyGenerator } from '../middleware/ipRateLimit';
 // eslint-disable-next-line global-require
 const express = require('express');
 // eslint-disable-next-line global-require
@@ -100,7 +101,7 @@ const taskWriteRateLimitKey = (req: Request): string => {
   if (authHeader) {
     return `tok:${createHash('sha256').update(authHeader).digest('hex').slice(0, 16)}`;
   }
-  return req.ip ? ipKeyGenerator(req.ip) : 'anon';
+  return cloudflareIpRateLimitKeyGenerator(req as never);
 };
 
 // Create and claim were limited; complete, updates and patch were not, though
diff --git a/backend/routes/users.ts b/backend/routes/users.ts
index 85a798730d..3f6ddaaaa3 100644
--- a/backend/routes/users.ts
+++ b/backend/routes/users.ts
@@ -1,6 +1,6 @@
 // ESM imports keep CodeQL's missing-rate-limiting dataflow connected to the
 // DB-backed polling route below (same established pattern as messages.ts).
-import rateLimit, { ipKeyGenerator } from 'express-rate-limit';
+import rateLimit from 'express-rate-limit';
 import { createHash } from 'crypto';
 import { cloudflareIpRateLimitKeyGenerator } from '../middleware/ipRateLimit';
 // eslint-disable-next-line global-require
@@ -35,7 +35,7 @@ const agentConnectionReadLimit = rateLimit({
     if (authHeader) {
       return `agent-connection:${createHash('sha256').update(authHeader).digest('hex').slice(0, 16)}`;
     }
-    return req.ip ? ipKeyGenerator(req.ip) : 'anon';
+    return cloudflareIpRateLimitKeyGenerator(req as never);
   },
   handler: (_req: unknown, res: any) => res.status(429).json({
     msg: 'rate limit exceeded: 60 agent connection reads per 60s',
@@ -76,7 +76,7 @@ const profileWriteUserLimit = rateLimit({
   }) => {
     const userId = req.userId || req.user?.id || req.user?._id;
     if (userId) return `profile-write:${String(userId)}`;
-    return req.ip ? ipKeyGenerator(req.ip) : 'anon';
+    return cloudflareIpRateLimitKeyGenerator(req as never);
   },
   handler: (_req: unknown, res: any) => res.status(429).json({
     msg: 'rate limit exceeded: 30 profile writes per 15 minutes',

From 6fc166d8a1ca37538a09ab497268662abba4d93b Mon Sep 17 00:00:00 2001
From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com>
Date: Thu, 24 Sep 2026 23:54:21 -0700
Subject: [PATCH 2/6] test(ratelimit): probe that no mounted limiter collapses
 two cf-connecting-ip callers
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

TASK-126. Every limiter instance found in the loaded app is re-mounted alone on a
stub with a 204 terminal and exercised with A, A, B requests differing only in
`cf-connecting-ip`. Fails on main; passes on this branch.

Why a stub instead of requests through the app — each reason measured:
- limiters are identified by identity (`resetKey`/`getKey` own properties),
  because EVERY router-stack entry has `handle.name === ''`;
- on a route carrying two limiters both write `RateLimit-Remaining` and the last
  one wins, so earlier limiters are invisible: on
  `POST /api/webhooks/groupme/:integrationId` the 3000-max IP limiter reported the
  600-max limiter's values, and the walk entry for the 600-max limiter then read
  596, because the first entry's three requests had decremented its bucket;
- a limiter mounted behind `auth` never runs for a header-free request;
- three instances never answer through the app at all (their handlers need Mongo).
On the stub no param and no auth header reaches a key function, so every limiter
takes its IP branch and every one of them answers.

A2 is the instrument's own control: two requests cannot separate "different
buckets" from "not counting at all". The fixtures prove the instrument
discriminates on the stub itself — a raw-`req.ip` limiter must COLLAPSE
(r3 === r2 - 1, a third hit in the same bucket) and a CF-aware one must SEPARATE
(r3 === r1). Without them a stub that separated everything would pass all
instances and establish nothing.

The app only mounts pg-messages/pg-status when PG_HOST is set AND the connect
resolves truthy (server.ts:67,380-400), so the probe presents that shape; an
earlier walk without it saw 82 instances and could not find pg-messages.
PG_HOST and NODE_ENV are restored per file/per request, since process.env is
per worker and a leak would change every later suite's DB path and skips.

Measured: 84 instances mounted, 84 separated, 0 collapsed, 0 unobserved. The 84
is 75 direct instances + 9 factory invocations (platformIpRateLimit x6,
taskWriteRateLimit x3) — 77 `rateLimit({` source sites, two of which are factory
bodies. On main the same file fails over 38 collapsing instances.

(cherry picked from commit 516d04f58ffdca1ca294398eb2b021587d203d88)
---
 .../rateLimitIpKeySeparation.test.js          | 236 ++++++++++++++++++
 1 file changed, 236 insertions(+)
 create mode 100644 backend/__tests__/unit/middleware/rateLimitIpKeySeparation.test.js

diff --git a/backend/__tests__/unit/middleware/rateLimitIpKeySeparation.test.js b/backend/__tests__/unit/middleware/rateLimitIpKeySeparation.test.js
new file mode 100644
index 0000000000..66898a7960
--- /dev/null
+++ b/backend/__tests__/unit/middleware/rateLimitIpKeySeparation.test.js
@@ -0,0 +1,236 @@
+/**
+ * TASK-126 — behavioural probe: no rate limiter mounted in the app may put two
+ * callers that differ only in `cf-connecting-ip` into one bucket.
+ *
+ * Why each walked instance is re-mounted on a stub instead of being exercised
+ * through the loaded app — all four measured, not assumed:
+ *   - Every entry in the loaded router stack is a Layer whose `handle.name` is
+ *     `''` and whose `entry.name` is `<anonymous>`, for middleware generally and
+ *     for limiters specifically. Identity (`resetKey`/`getKey` own properties)
+ *     is the only census instrument; anything name-keyed mis-attributes.
+ *   - On a route carrying two limiters both write `RateLimit-Remaining` and the
+ *     last one that runs wins. Proved on `POST /api/webhooks/groupme/:integrationId`:
+ *     the 3000-max IP limiter reported `remaining` 599/598/597 (the 600-max
+ *     limiter's values) and the walk entry for the 600-max limiter then read 596,
+ *     because the first entry's three requests had decremented its bucket. So a
+ *     hidden limiter's count is invisible in that response.
+ *   - A limiter mounted behind `auth` (`routes/users.ts:171`
+ *     `profileWriteUserLimit`) never runs for a header-free request.
+ *   - Three instances never answer at all through the app: their handlers need
+ *     Mongo and unit mode runs without it.
+ * On the stub there are no params and no auth header, so every key function
+ * takes its IP branch (each one falls back to the Cloudflare-aware helper), and
+ * the 204 terminal answers for all of them — no exemption list, no database.
+ *
+ * The A,A,B sequence is deliberate: two requests cannot separate "different
+ * buckets" from "not counting at all", so A2 is the instrument's own control.
+ * The two fixtures prove the instrument discriminates on the stub itself: a
+ * deliberately raw-`req.ip` limiter must COLLAPSE and a
+ * `cloudflareIpRateLimitKeyGenerator` one must SEPARATE. A stub that separates
+ * everything handed to it would pass all 82 and establish nothing.
+ */
+/* eslint-disable import/no-unresolved, import/extensions */
+const fs = require('fs');
+const express = require('express');
+const request = require('supertest');
+const rateLimit = require('express-rate-limit');
+const { cloudflareIpRateLimitKeyGenerator } = require('../../../middleware/ipRateLimit');
+
+// The app only mounts pg-messages/pg-status when PG_HOST is set AND the connect
+// resolves truthy (server.ts:67,380-400), so the probe has to present that shape.
+// process.env is per WORKER, not per file: leaving PG_HOST set would hand every
+// later suite in this worker a PG-mounted app.
+const ENTRY_PG_HOST = process.env.PG_HOST;
+process.env.PG_HOST = ENTRY_PG_HOST || 'probe';
+jest.mock('../../../config/db', () => jest.fn());
+jest.mock('../../../config/db-pg', () => {
+  const actual = jest.requireActual('../../../config/db-pg');
+  const pool = { query: jest.fn().mockResolvedValue({ rows: [] }), on: jest.fn(), end: jest.fn() };
+  return { ...actual, connectPG: jest.fn().mockResolvedValue(pool) };
+});
+jest.mock('../../../config/init-pg-db', () => jest.fn().mockResolvedValue(true));
+jest.setTimeout(300000);
+
+const { app } = require('../../../server');
+
+const isLimiter = (fn) => fn && typeof fn === 'function'
+  && Object.prototype.hasOwnProperty.call(fn, 'resetKey')
+  && Object.prototype.hasOwnProperty.call(fn, 'getKey');
+
+const prefixOf = (layer) => {
+  const src = (layer.regexp && layer.regexp.source) || '';
+  const m = src.match(/^\^\\?\/?([A-Za-z0-9_/\\-]*?)\\?\/\?\(\?=\\\/\|\$\)/);
+  return m ? `/${m[1].replace(/\\\//g, '/').replace(/\/$/, '')}` : '';
+};
+const normPath = (p) => p.replace(/:[A-Za-z0-9_]+/g, 'test-id').replace(/\/{2,}/g, '/');
+
+const collect = (router, prefix, found, seen) => {
+  ((router && router.stack) || []).forEach((layer) => {
+    if (layer.route) {
+      const path = normPath(prefix + layer.route.path);
+      const method = Object.keys(layer.route.methods || {})[0] || 'get';
+      (layer.route.stack || []).forEach((entry) => {
+        if (isLimiter(entry.handle) && !seen.has(entry.handle)) {
+          seen.add(entry.handle);
+          found.push({ instance: entry.handle, path, method });
+        }
+      });
+      return;
+    }
+    if (isLimiter(layer.handle) && !seen.has(layer.handle)) {
+      seen.add(layer.handle);
+      found.push({ instance: layer.handle, path: normPath(prefix) || '/', method: 'get' });
+    }
+    if (layer.handle && layer.handle.stack) {
+      collect(layer.handle, prefix + prefixOf(layer), found, seen);
+    }
+  });
+};
+
+const walked = [];
+const seen = new Set();
+
+const remainingOf = (res) => {
+  if (res.headers['ratelimit-remaining'] !== undefined) return Number(res.headers['ratelimit-remaining']);
+  const combined = res.headers.ratelimit;
+  const m = combined && String(combined).match(/remaining=(\d+)/);
+  return m ? Number(m[1]) : null;
+};
+
+const single = (instance) => {
+  const stub = express();
+  stub.all('/', instance, (_req, res) => res.status(204).end());
+  return stub;
+};
+
+const hit = async (stub, ip) => {
+  const entry = process.env.NODE_ENV;
+  let outcome;
+  try {
+    process.env.NODE_ENV = 'development';
+    const res = await request(stub).get('/').set('cf-connecting-ip', ip)
+      .timeout({ response: 5000, deadline: 8000 });
+    outcome = { remaining: remainingOf(res), status: res.status };
+  } catch (err) {
+    outcome = { error: String((err && err.message) || err).slice(0, 80) };
+  } finally {
+    // Restore per REQUEST, not per suite: a throw mid-walk would otherwise leave
+    // NODE_ENV flipped for the rest of this worker and silently un-skip every
+    // limiter in later suites.
+    process.env.NODE_ENV = entry;
+  }
+  if (process.env.NODE_ENV !== entry) throw new Error('NODE_ENV leaked');
+  return outcome;
+};
+
+const observe = async (instance, tag) => {
+  const a = `198.51.100.${tag}`;
+  const b = `203.0.113.${tag}`;
+  const stub = single(instance);
+  const first = await hit(stub, a);
+  const second = await hit(stub, a);
+  const other = await hit(stub, b);
+  return {
+    r1: first.remaining, r2: second.remaining, r3: other.remaining, status: first.status, error: first.error,
+  };
+};
+
+const fmt = (label, rec) => {
+  const head = `${label} [r1=${rec.r1} r2=${rec.r2} r3=${rec.r3} status=${rec.status}`;
+  return `${head}${rec.error ? ` err=${rec.error}` : ''}]`;
+};
+
+describe('TASK-126 ip key separation', () => {
+  afterAll(() => {
+    if (ENTRY_PG_HOST === undefined) delete process.env.PG_HOST;
+    else process.env.PG_HOST = ENTRY_PG_HOST;
+  });
+
+  beforeAll(async () => {
+    // pg-messages and pg-status mount from promise callbacks (server.ts:380-400),
+    // so the walk has to wait for the PG connect to resolve or those limiters are
+    // invisible to it.
+    await new Promise((resolve) => { setTimeout(resolve, 2000); });
+    collect(app._router || app.router, '', walked, seen);
+  });
+
+  it('walks the loaded app and finds limiter instances by identity', () => {
+    expect(walked.some((w) => w.path.startsWith('/api/pg/messages'))).toBe(true);
+    expect(walked.length).toBeGreaterThanOrEqual(80);
+    const distinct = new Set(walked.map((w) => w.instance));
+    expect(distinct.size).toBe(walked.length);
+  });
+
+  it('negative control: the instrument catches a raw req.ip limiter on a stub', async () => {
+    const raw = rateLimit({
+      windowMs: 60000,
+      max: 5,
+      standardHeaders: true,
+      legacyHeaders: false,
+      keyGenerator: (req) => req.ip,
+    });
+    const rec = await observe(raw, 250);
+    expect(rec.r2).toBe(rec.r1 - 1);
+    expect(rec.r3).toBe(rec.r2 - 1);
+    expect(rec.r3).not.toBe(rec.r1);
+  });
+
+  it('positive control: the CF-aware key separates on the same stub', async () => {
+    const cf = rateLimit({
+      windowMs: 60000,
+      max: 5,
+      standardHeaders: true,
+      legacyHeaders: false,
+      keyGenerator: (req) => cloudflareIpRateLimitKeyGenerator(req),
+    });
+    const rec = await observe(cf, 251);
+    expect(rec.r2).toBe(rec.r1 - 1);
+    expect(rec.r3).toBe(rec.r1);
+  });
+
+  it('every walked instance counts and separates; none collapses', async () => {
+    const results = [];
+    const runs = walked.map(async (w, i) => {
+      const rec = await observe(w.instance, i + 1);
+      results[i] = { path: w.path, method: w.method, ...rec };
+    });
+    await Promise.all(runs);
+
+    const unseen = results.filter((r) => r.r1 === null || r.r1 === undefined);
+    const notCounting = results.filter((r) => r.r1 !== null && r.r1 !== undefined && r.r2 !== r.r1 - 1);
+    // Collapse signature is r3 === r2 - 1, NOT r3 === r2: a third request into the
+    // same bucket keeps counting down. Measured on the raw-req.ip fixture below.
+    const collapsed = results.filter((r) => r.r1 !== null && r.r3 === r.r2 - 1);
+    const separated = results.filter((r) => r.r1 !== null && r.r3 === r.r1);
+
+    expect(unseen.map((r) => fmt(`${r.method.toUpperCase()} ${r.path}`, r))).toEqual([]);
+    expect(notCounting.map((r) => fmt(`${r.method.toUpperCase()} ${r.path}`, r))).toEqual([]);
+    expect(collapsed.map((r) => fmt(`${r.method.toUpperCase()} ${r.path}`, r))).toEqual([]);
+    expect(separated.length).toBe(results.length);
+    // The floor is on observations, not violations: an assertion that every
+    // member of an EMPTY set is fine would pass on a walk that found nothing.
+    expect(separated.length).toBeGreaterThanOrEqual(80);
+    fs.writeFileSync(process.env.PROBE_CENSUS || '/tmp/kai126-probe-census.json', JSON.stringify({
+      instances: results.length,
+      separated: separated.length,
+      notCounting: notCounting.length,
+      collapsed: collapsed.length,
+      unseen: unseen.length,
+      paths: results.map((r) => `${r.method.toUpperCase()} ${r.path}`),
+    }, null, 1));
+  });
+
+  it('finds the limiters that mount late or behind a router', () => {
+    const modules = ['../../../routes/agentHooks', '../../../routes/podInvites', '../../../routes/pg-messages'];
+    const report = modules.map((mod) => {
+      // eslint-disable-next-line global-require, import/no-dynamic-require
+      const router = require(mod);
+      const found = [];
+      collect(router, '', found, new Set());
+      const missing = found.filter((f) => !seen.has(f.instance));
+      return { mod, found: found.length, missing: missing.length };
+    });
+    expect(report.filter((r) => r.found === 0).map((r) => r.mod)).toEqual([]);
+    expect(report.filter((r) => r.missing > 0).map((r) => r.mod)).toEqual([]);
+  });
+});

From 4b28c8652c8d3dc878bce4dc74419b9107f48e92 Mon Sep 17 00:00:00 2001
From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com>
Date: Fri, 25 Sep 2026 00:06:48 -0700
Subject: [PATCH 3/6] test(ratelimit): state what the stub measures, and the
 one stub-only arm
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two ruled disclosures, both about what the probe does NOT prove. The probe's own
behaviour is unchanged.

wren 73671 (ruling (b)): the assertion now says what the stub measures — a
request with no credential, no param and no body, carrying only
`cf-connecting-ip`, is bucketed per `cf-connecting-ip` on every instance. Stricter
than a per-arm check, because a limiter with no IP arm at all keys that request on
a CONSTANT: one bucket for every anonymous caller, the same defect as raw `req.ip`
behind cloudflared, and it collapses A and B here identically.

vera 73673, trap 2: `users.ts:65` `profileWriteUserLimit` keys on
`req.userId || req.user?.id || req.user?._id` first, and `auth` runs before it
(`users.ts:171`), so an unauthenticated request never reaches it. Its IP arm is
reachable on this stub only. The arm must still be CF-derived, but the walk proves
the stub's shape, not liveness — in production that limiter keys per account.

Trap 1 needed no code change: the probe builds each request from nothing and sets
only `cf-connecting-ip` (vera 73674), so no credential header — `authorization`,
`x-auth-token`, `x-commonly-agent-token` — can select a token arm, and
`req.agentTokenHash` is unset.

(cherry picked from commit d7c27bede25b29549ed5af2e4481f6c53d204559)
---
 .../middleware/rateLimitIpKeySeparation.test.js | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/backend/__tests__/unit/middleware/rateLimitIpKeySeparation.test.js b/backend/__tests__/unit/middleware/rateLimitIpKeySeparation.test.js
index 66898a7960..b590d47296 100644
--- a/backend/__tests__/unit/middleware/rateLimitIpKeySeparation.test.js
+++ b/backend/__tests__/unit/middleware/rateLimitIpKeySeparation.test.js
@@ -188,6 +188,13 @@ describe('TASK-126 ip key separation', () => {
     expect(rec.r3).toBe(rec.r1);
   });
 
+  // What this measures (wren 73671, ruling (b)), not what it might be read as:
+  // a request with no credential, no param and no body — built from nothing and
+  // carrying only `cf-connecting-ip` — is bucketed per `cf-connecting-ip` on
+  // every instance. That is stricter than a per-arm check: a limiter with no IP
+  // arm at all keys such a request on a CONSTANT, one bucket for every anonymous
+  // caller, which is the same defect as raw `req.ip` behind cloudflared — and it
+  // collapses A and B here exactly the same way.
   it('every walked instance counts and separates; none collapses', async () => {
     const results = [];
     const runs = walked.map(async (w, i) => {
@@ -196,6 +203,14 @@ describe('TASK-126 ip key separation', () => {
     });
     await Promise.all(runs);
 
+    // Stub-only arm, disclosed rather than left implied (vera 73673): on the real
+    // app `users.ts:65` `profileWriteUserLimit` keys on
+    // `req.userId || req.user?.id || req.user?._id` FIRST, and `auth` runs before
+    // it (`users.ts:171`, `router.put('/profile', profileWriteIngressLimit, auth,
+    // profileWriteUserLimit, ...)`) — an unauthenticated request stops at auth, so
+    // its IP arm is unreachable in production. The arm is real and must stay
+    // CF-derived, but what this walk proves about it is the stub's behaviour, not
+    // liveness: in production that limiter keys per account, not per address.
     const unseen = results.filter((r) => r.r1 === null || r.r1 === undefined);
     const notCounting = results.filter((r) => r.r1 !== null && r.r1 !== undefined && r.r2 !== r.r1 - 1);
     // Collapse signature is r3 === r2 - 1, NOT r3 === r2: a third request into the
@@ -210,6 +225,8 @@ describe('TASK-126 ip key separation', () => {
     // The floor is on observations, not violations: an assertion that every
     // member of an EMPTY set is fine would pass on a walk that found nothing.
     expect(separated.length).toBeGreaterThanOrEqual(80);
+    // Census dump for reconciliation, not an assertion: it is what let 84 be
+    // accounted as 75 direct + 9 factory invocations instead of assumed.
     fs.writeFileSync(process.env.PROBE_CENSUS || '/tmp/kai126-probe-census.json', JSON.stringify({
       instances: results.length,
       separated: separated.length,

From cbc8e2f3d6145c5ba6f5ede5e4605ab1f690f983 Mon Sep 17 00:00:00 2001
From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com>
Date: Fri, 25 Sep 2026 00:21:42 -0700
Subject: [PATCH 4/6] test(ratelimit): serialise the probe walk and make its
 NODE_ENV check a witness
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The 84/84 result is unchanged; what changes is that the instrument can now fail.

wren 73692, accepted: with 84 chains started in one tick, only the first reads the
suite's NODE_ENV and the other 83 read the 'development' it just set, each
restoring what it read — so the walk ended flipped and no per-request check could
see it. The walk is now serial, and the assertion that NODE_ENV is where it
started runs once after it. Measured: ends at 'test', same 84/84, 323ms against
177ms. The `for...of` and its `await` are eslint-disabled on purpose — the
serialisation is the property under test.

wren 73694, accepted, second half: the PG_HOST save/restore is gone. setup.js sets
PG_HOST to the literal string 'undefined' in in-memory mode, which is truthy, so
`ENTRY_PG_HOST || 'probe'` wrote back the value it had just read and changed
nothing — the pg-messages mount comes from the connect mocks. Measured after
removal: 84 instances, pg-messages still found.

Two comments of mine were wrong and are corrected: process.env is per TEST FILE
here, not per worker (measured — same pid, writer file first, reader file sees
undefined), so the "later suite in this worker" rationale was false. The census
dump is now written only when PROBE_CENSUS is set, so CI writes no /tmp file.

(cherry picked from commit 005ff9cd0f92d2741801a5da56301cace32d0558)
---
 .../rateLimitIpKeySeparation.test.js          | 73 ++++++++++++-------
 1 file changed, 46 insertions(+), 27 deletions(-)

diff --git a/backend/__tests__/unit/middleware/rateLimitIpKeySeparation.test.js b/backend/__tests__/unit/middleware/rateLimitIpKeySeparation.test.js
index b590d47296..885bca6922 100644
--- a/backend/__tests__/unit/middleware/rateLimitIpKeySeparation.test.js
+++ b/backend/__tests__/unit/middleware/rateLimitIpKeySeparation.test.js
@@ -27,7 +27,7 @@
  * The two fixtures prove the instrument discriminates on the stub itself: a
  * deliberately raw-`req.ip` limiter must COLLAPSE and a
  * `cloudflareIpRateLimitKeyGenerator` one must SEPARATE. A stub that separates
- * everything handed to it would pass all 82 and establish nothing.
+ * everything handed to it would pass all 84 and establish nothing.
  */
 /* eslint-disable import/no-unresolved, import/extensions */
 const fs = require('fs');
@@ -36,12 +36,18 @@ const request = require('supertest');
 const rateLimit = require('express-rate-limit');
 const { cloudflareIpRateLimitKeyGenerator } = require('../../../middleware/ipRateLimit');
 
-// The app only mounts pg-messages/pg-status when PG_HOST is set AND the connect
-// resolves truthy (server.ts:67,380-400), so the probe has to present that shape.
-// process.env is per WORKER, not per file: leaving PG_HOST set would hand every
-// later suite in this worker a PG-mounted app.
-const ENTRY_PG_HOST = process.env.PG_HOST;
-process.env.PG_HOST = ENTRY_PG_HOST || 'probe';
+// Captured at MODULE LOAD, before any test can flip it. A baseline re-read inside
+// a test moves with a leak instead of catching it: measured, with the per-request
+// restore removed an earlier test left 'development' set, the walk test's own
+// `entry` then read 'development' too, and the comparison passed.
+const ENTRY_NODE_ENV = process.env.NODE_ENV;
+
+// The app only mounts pg-messages/pg-status when PG_HOST is truthy AND the
+// connect resolves truthy (server.ts:67,378-400). That condition is already met
+// here: setup.js sets PG_HOST to the literal string 'undefined' in in-memory
+// mode, which is TRUTHY, so the mount in this file comes from the connect mocks
+// below and needs no PG_HOST handling. Measured: PG_HOST is "undefined" at
+// module scope, in a file that never touches it.
 jest.mock('../../../config/db', () => jest.fn());
 jest.mock('../../../config/db-pg', () => {
   const actual = jest.requireActual('../../../config/db-pg');
@@ -114,12 +120,10 @@ const hit = async (stub, ip) => {
   } catch (err) {
     outcome = { error: String((err && err.message) || err).slice(0, 80) };
   } finally {
-    // Restore per REQUEST, not per suite: a throw mid-walk would otherwise leave
-    // NODE_ENV flipped for the rest of this worker and silently un-skip every
-    // limiter in later suites.
+    // Restore per REQUEST so each iteration ends where it started; the assertion
+    // after the walk is what enforces that it did.
     process.env.NODE_ENV = entry;
   }
-  if (process.env.NODE_ENV !== entry) throw new Error('NODE_ENV leaked');
   return outcome;
 };
 
@@ -141,9 +145,11 @@ const fmt = (label, rec) => {
 };
 
 describe('TASK-126 ip key separation', () => {
+  // The walk's own assertion covers the probe path; this covers every test in the
+  // file, the two controls included — the leak mutation was introduced by the
+  // negative control, before the walk ever ran.
   afterAll(() => {
-    if (ENTRY_PG_HOST === undefined) delete process.env.PG_HOST;
-    else process.env.PG_HOST = ENTRY_PG_HOST;
+    expect(process.env.NODE_ENV).toBe(ENTRY_NODE_ENV);
   });
 
   beforeAll(async () => {
@@ -197,11 +203,21 @@ describe('TASK-126 ip key separation', () => {
   // collapses A and B here exactly the same way.
   it('every walked instance counts and separates; none collapses', async () => {
     const results = [];
-    const runs = walked.map(async (w, i) => {
-      const rec = await observe(w.instance, i + 1);
-      results[i] = { path: w.path, method: w.method, ...rec };
-    });
-    await Promise.all(runs);
+    // Serial, not `Promise.all` (wren 73692): with 84 chains started in one tick,
+    // only the first reads the suite's NODE_ENV; the other 83 read the
+    // 'development' it just set and each restores what it read — so the walk ends
+    // flipped, and no per-request check can see it. Serialised, every iteration
+    // restores before the next begins, which is what makes the assertion below a
+    // witness rather than decoration: it fails if the walk does not end where it
+    // started.
+    /* eslint-disable no-await-in-loop, no-restricted-syntax -- serialised on
+       purpose: the NODE_ENV restore of one iteration must complete before the
+       next reads it, which is the property under test. */
+    for (const [i, w] of walked.entries()) {
+      results[i] = { path: w.path, method: w.method, ...(await observe(w.instance, i + 1)) };
+    }
+    /* eslint-enable no-await-in-loop, no-restricted-syntax */
+    expect(process.env.NODE_ENV).toBe(ENTRY_NODE_ENV);
 
     // Stub-only arm, disclosed rather than left implied (vera 73673): on the real
     // app `users.ts:65` `profileWriteUserLimit` keys on
@@ -226,15 +242,18 @@ describe('TASK-126 ip key separation', () => {
     // member of an EMPTY set is fine would pass on a walk that found nothing.
     expect(separated.length).toBeGreaterThanOrEqual(80);
     // Census dump for reconciliation, not an assertion: it is what let 84 be
-    // accounted as 75 direct + 9 factory invocations instead of assumed.
-    fs.writeFileSync(process.env.PROBE_CENSUS || '/tmp/kai126-probe-census.json', JSON.stringify({
-      instances: results.length,
-      separated: separated.length,
-      notCounting: notCounting.length,
-      collapsed: collapsed.length,
-      unseen: unseen.length,
-      paths: results.map((r) => `${r.method.toUpperCase()} ${r.path}`),
-    }, null, 1));
+    // accounted as 75 direct + 9 factory invocations instead of assumed. Opt-in,
+    // so CI does not write a /tmp file on every run.
+    if (process.env.PROBE_CENSUS) {
+      fs.writeFileSync(process.env.PROBE_CENSUS, JSON.stringify({
+        instances: results.length,
+        separated: separated.length,
+        notCounting: notCounting.length,
+        collapsed: collapsed.length,
+        unseen: unseen.length,
+        paths: results.map((r) => `${r.method.toUpperCase()} ${r.path}`),
+      }, null, 1));
+    }
   });
 
   it('finds the limiters that mount late or behind a router', () => {

From 217237ea2964a8d7a81d5980f84a41bff743be63 Mon Sep 17 00:00:00 2001
From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com>
Date: Fri, 25 Sep 2026 00:52:59 -0700
Subject: [PATCH 5/6] test(ratelimit): set PG_HOST in the probe instead of
 inheriting a harness accident
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The probe mounts pg-messages/pg-status only when PG_HOST is truthy (server.ts:67,
378-400), and until now it inherited that truthiness from setup.js storing the
STRING 'undefined'. TASK-128 deletes the key, which would have dropped this probe
to 82 instances and quietly falsified its own must-find assertion. Set it here.

No restore machinery, and that is a measurement rather than a hope: process.env
is per test FILE here, so the assignment cannot reach another suite. Verified in
both worlds — 84 instances, 84 separated, pg-messages present with the key set
here, including with TASK-128's `delete` applied to setup.js.

(cherry picked from commit 6250ebc22b36ffd5b60aed3e20538ab908e25f1d)
---
 .../unit/middleware/rateLimitIpKeySeparation.test.js | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/backend/__tests__/unit/middleware/rateLimitIpKeySeparation.test.js b/backend/__tests__/unit/middleware/rateLimitIpKeySeparation.test.js
index 885bca6922..07aa8814fa 100644
--- a/backend/__tests__/unit/middleware/rateLimitIpKeySeparation.test.js
+++ b/backend/__tests__/unit/middleware/rateLimitIpKeySeparation.test.js
@@ -43,11 +43,13 @@ const { cloudflareIpRateLimitKeyGenerator } = require('../../../middleware/ipRat
 const ENTRY_NODE_ENV = process.env.NODE_ENV;
 
 // The app only mounts pg-messages/pg-status when PG_HOST is truthy AND the
-// connect resolves truthy (server.ts:67,378-400). That condition is already met
-// here: setup.js sets PG_HOST to the literal string 'undefined' in in-memory
-// mode, which is TRUTHY, so the mount in this file comes from the connect mocks
-// below and needs no PG_HOST handling. Measured: PG_HOST is "undefined" at
-// module scope, in a file that never touches it.
+// connect resolves truthy (server.ts:67,378-400), so this file sets it rather
+// than inheriting it: setup.js used to leave the truthy STRING 'undefined' in
+// PG_HOST (TASK-128 deletes the key instead), and a probe that silently needs
+// that accident drops to 82 instances the day it is fixed. No restore machinery
+// is needed — process.env is per test FILE (measured), so this assignment cannot
+// reach any other suite.
+process.env.PG_HOST = 'probe';
 jest.mock('../../../config/db', () => jest.fn());
 jest.mock('../../../config/db-pg', () => {
   const actual = jest.requireActual('../../../config/db-pg');

From c621dd92d20ade1d16d700d2041f632864461b7a Mon Sep 17 00:00:00 2001
From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com>
Date: Fri, 25 Sep 2026 01:17:50 -0700
Subject: [PATCH 6/6] test(ratelimit): key the negative control the way the
 swapped sites actually were
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Vera 73704, taken rather than refused: the control keyed `(req) => req.ip`, a
paraphrase of the defect. The 21 sites were `req.ip ? ipKeyGenerator(req.ip) :
'anon'` (the spelling still at `middleware/ipRateLimit.ts:54`). Both collapse on
the stub — no `trust proxy`, so every request carries the loopback socket
address — but the bare form also hands express-rate-limit's IPv6 key validation
a raw value, so it measured a second thing alongside the collapse. The fixture
now reproduces what was there.

Vera 73712: the comment cited `server.ts:67`; the gate is on **`:68`** on
`origin/main` (`:67` is blank). Inherited from the comment this replaced, and it
is in the file under her gate.

Probe unchanged: 5/5, 84 instances / 84 separated / 0 collapsed / 0
not-counting / 0 unobserved, both pg-messages routes still found.

(cherry picked from commit 1e6beaef4cc2b2e04853211ab06856e550ded785)
---
 .../rateLimitIpKeySeparation.test.js          | 21 +++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/backend/__tests__/unit/middleware/rateLimitIpKeySeparation.test.js b/backend/__tests__/unit/middleware/rateLimitIpKeySeparation.test.js
index 07aa8814fa..8d97613b4f 100644
--- a/backend/__tests__/unit/middleware/rateLimitIpKeySeparation.test.js
+++ b/backend/__tests__/unit/middleware/rateLimitIpKeySeparation.test.js
@@ -24,9 +24,10 @@
  *
  * The A,A,B sequence is deliberate: two requests cannot separate "different
  * buckets" from "not counting at all", so A2 is the instrument's own control.
- * The two fixtures prove the instrument discriminates on the stub itself: a
- * deliberately raw-`req.ip` limiter must COLLAPSE and a
- * `cloudflareIpRateLimitKeyGenerator` one must SEPARATE. A stub that separates
+ * The two fixtures prove the instrument discriminates on the stub itself: one
+ * keyed the way the 21 swapped sites were — `req.ip ? ipKeyGenerator(req.ip) :
+ * 'anon'`, the spelling at `middleware/ipRateLimit.ts:54` — must COLLAPSE, and
+ * one keyed on `cloudflareIpRateLimitKeyGenerator` must SEPARATE. A stub that separates
  * everything handed to it would pass all 84 and establish nothing.
  */
 /* eslint-disable import/no-unresolved, import/extensions */
@@ -34,6 +35,7 @@ const fs = require('fs');
 const express = require('express');
 const request = require('supertest');
 const rateLimit = require('express-rate-limit');
+const { ipKeyGenerator } = require('express-rate-limit');
 const { cloudflareIpRateLimitKeyGenerator } = require('../../../middleware/ipRateLimit');
 
 // Captured at MODULE LOAD, before any test can flip it. A baseline re-read inside
@@ -43,7 +45,7 @@ const { cloudflareIpRateLimitKeyGenerator } = require('../../../middleware/ipRat
 const ENTRY_NODE_ENV = process.env.NODE_ENV;
 
 // The app only mounts pg-messages/pg-status when PG_HOST is truthy AND the
-// connect resolves truthy (server.ts:67,378-400), so this file sets it rather
+// connect resolves truthy (server.ts:68,378-400), so this file sets it rather
 // than inheriting it: setup.js used to leave the truthy STRING 'undefined' in
 // PG_HOST (TASK-128 deletes the key instead), and a probe that silently needs
 // that accident drops to 82 instances the day it is fixed. No restore machinery
@@ -169,13 +171,20 @@ describe('TASK-126 ip key separation', () => {
     expect(distinct.size).toBe(walked.length);
   });
 
-  it('negative control: the instrument catches a raw req.ip limiter on a stub', async () => {
+  it('negative control: the instrument catches a limiter keyed the way the swapped sites were', async () => {
+    // The defect's own spelling, not a cousin of it (vera 73704): the 21 sites
+    // keyed `req.ip ? ipKeyGenerator(req.ip) : 'anon'`. A bare `req.ip` collapses
+    // on this stub for the same reason — no `trust proxy`, so both requests carry
+    // the loopback socket address — but it also hands express-rate-limit's IPv6
+    // key validation a raw value, which would make this control measure a second
+    // thing. The form below is what was actually there, so the fixture proves the
+    // defect rather than a paraphrase of it.
     const raw = rateLimit({
       windowMs: 60000,
       max: 5,
       standardHeaders: true,
       legacyHeaders: false,
-      keyGenerator: (req) => req.ip,
+      keyGenerator: (req) => (req.ip ? ipKeyGenerator(req.ip) : 'anon'),
     });
     const rec = await observe(raw, 250);
     expect(rec.r2).toBe(rec.r1 - 1);