From 4e920feab1a5d70619ddcd35b180e86008a84acf Mon Sep 17 00:00:00 2001
From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com>
Date: Sun, 18 Oct 2026 12:27:18 -0700
Subject: [PATCH 1/8] feat(backend): refuse $-prefixed keys in request query
 strings and bodies

A new middleware (middleware/refuseDollarKeys.ts) answers 400
`dollar_key_refused` when any key in req.query or a JSON body starts with `$`,
at any depth. It is mounted right after the global express.json(), before any
route, and again after multer in the two multipart routes (pods
/external-link, uploads), whose fields are parsed later.

An audit on 2026-10-18 found no client that sends such a key: the frontend,
cli, commonly-mcp, SDK examples, the agent-runtime worker, scripts and the
openclaw extension at its pin, and no stored Mongo document or Postgres
json/jsonb value holding one. Two narrow exemptions: JSON Schema keywords
inside a marketplace publish component's widgetConfigSchema or schemaFields,
and anything under a Slack event's metadata.event_payload, whose keys other
Slack apps define. Buffer bodies (the signature-verified webhooks) are not
inspected.

Tests: query, nested body and array cases; values and mid-key `$` pass; the
two exemptions and their limits; multipart fields; a 5,000-deep body; and the
mount order in server.ts. Seven mutations of the guard each fail a test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MCgCsf3UJRCPRbnNKxsECe
---
 .../unit/middleware/refuseDollarKeys.test.js  | 154 ++++++++++++++++++
 backend/middleware/refuseDollarKeys.ts        |  96 +++++++++++
 backend/routes/pods.ts                        |   3 +-
 backend/routes/uploads.ts                     |   5 +-
 backend/server.ts                             |   5 +
 5 files changed, 261 insertions(+), 2 deletions(-)
 create mode 100644 backend/__tests__/unit/middleware/refuseDollarKeys.test.js
 create mode 100644 backend/middleware/refuseDollarKeys.ts

diff --git a/backend/__tests__/unit/middleware/refuseDollarKeys.test.js b/backend/__tests__/unit/middleware/refuseDollarKeys.test.js
new file mode 100644
index 0000000000..417cc13833
--- /dev/null
+++ b/backend/__tests__/unit/middleware/refuseDollarKeys.test.js
@@ -0,0 +1,154 @@
+// @ts-nocheck
+// refuseDollarKeys: request keys starting with `$` are refused with 400, in the
+// query string and the body, at any depth, with two narrow body exemptions.
+// The app below mirrors server.ts's order: route-specific parsers first, then
+// the global express.json(), then the guard, then the routes.
+
+const fs = require('fs');
+const path = require('path');
+const express = require('express');
+const multer = require('multer');
+const request = require('supertest');
+const { refuseDollarKeys, findDollarKey, DOLLAR_KEY_REFUSED } = require('../../../middleware/refuseDollarKeys');
+
+const buildApp = () => {
+  const app = express();
+  app.use('/raw', express.raw({ type: 'application/json' }));
+  app.use(express.json());
+  app.use(refuseDollarKeys);
+  const ok = (req, res) => res.json({ ok: true });
+  app.get('/x', ok);
+  app.post('/x', ok);
+  app.post('/raw', ok);
+  app.post('/api/marketplace/publish', ok);
+  app.put('/api/marketplace/publish', ok);
+  app.post('/api/webhooks/slack/events', ok);
+  // A multipart route, as pods.ts and uploads.ts have: multer parses its
+  // fields after the global guard, so the route runs the guard again.
+  app.post('/multipart', multer().none(), refuseDollarKeys, ok);
+  app.post('/multipart-unguarded', multer().none(), (req, res) => res.json({ body: req.body }));
+  return app;
+};
+
+describe('refuseDollarKeys', () => {
+  const app = buildApp();
+
+  test.each([
+    ['a nested query operator', '/x?a[$ne]=1', 'query.a.$ne'],
+    ['a top-level query key', '/x?$where=1', 'query.$where'],
+    ['a query key inside an array', '/x?a[0][$gt]=1', 'query.a.0.$gt'],
+  ])('refuses %s', async (_label, url, field) => {
+    const res = await request(app).get(url);
+    expect(res.status).toBe(400);
+    expect(res.body).toMatchObject({ code: DOLLAR_KEY_REFUSED, field });
+  });
+
+  test('refuses a body key at any depth, arrays included', async () => {
+    const res = await request(app).post('/x').send({ a: { b: [{ c: 1 }, { $gt: 1 }] } });
+    expect(res.status).toBe(400);
+    expect(res.body).toMatchObject({ code: DOLLAR_KEY_REFUSED, field: 'body.a.b.1.$gt' });
+  });
+
+  test('passes `$` in values, and in keys that do not start with it', async () => {
+    const res = await request(app).post('/x?q=$5').send({ price: '$5', a$b: 1, list: ['$x'] });
+    expect(res.status).toBe(200);
+  });
+
+  test('does not inspect a raw Buffer body (signature-verified webhooks)', async () => {
+    const res = await request(app).post('/raw').set('Content-Type', 'application/json').send('{"$schema":"x"}');
+    expect(res.status).toBe(200);
+  });
+
+  test('a deeply nested body does not exhaust the stack', async () => {
+    let body = { leaf: 1 };
+    for (let i = 0; i < 5000; i += 1) body = { n: body };
+    const res = await request(app).post('/x').send(body);
+    expect(res.status).toBe(200);
+  });
+
+  describe('marketplace publish: JSON Schema keywords in schema fields only', () => {
+    const schema = { $schema: 'https://json-schema.org/draft/2020-12/schema', $defs: { a: { type: 'string' } }, properties: { x: { $ref: '#/$defs/a' } } };
+
+    test('allows keywords in widgetConfigSchema and schemaFields, at any depth', async () => {
+      const res = await request(app).post('/api/marketplace/publish')
+        .send({ components: [{ widgetConfigSchema: schema }, { schemaFields: { $id: 'f' } }] });
+      expect(res.status).toBe(200);
+    });
+
+    test.each([
+      ['a non-keyword in a schema field', { components: [{ widgetConfigSchema: { $ne: 1 } }] }, 'body.components.0.widgetConfigSchema.$ne'],
+      ['a keyword in another component field', { components: [{ metadata: { $schema: 'x' } }] }, 'body.components.0.metadata.$schema'],
+      ['a keyword outside components', { $schema: 'x' }, 'body.$schema'],
+      ['a non-keyword nested under a keyword', { components: [{ widgetConfigSchema: { $defs: { $where: 1 } } }] }, 'body.components.0.widgetConfigSchema.$defs.$where'],
+    ])('refuses %s', async (_label, body, field) => {
+      const res = await request(app).post('/api/marketplace/publish').send(body);
+      expect(res.status).toBe(400);
+      expect(res.body.field).toBe(field);
+    });
+
+    test('the exemption is POST /api/marketplace/publish only', async () => {
+      const body = { components: [{ widgetConfigSchema: schema }] };
+      expect((await request(app).put('/api/marketplace/publish').send(body)).status).toBe(400);
+      expect((await request(app).post('/x').send(body)).status).toBe(400);
+    });
+  });
+
+  describe('slack webhooks: event.metadata.event_payload is the sending app\'s', () => {
+    test('allows any key under event_payload', async () => {
+      const res = await request(app).post('/api/webhooks/slack/events')
+        .send({ event: { type: 'message', metadata: { event_type: 't', event_payload: { $a: { $b: 1 } } } } });
+      expect(res.status).toBe(200);
+    });
+
+    test('refuses a key elsewhere in the event', async () => {
+      const res = await request(app).post('/api/webhooks/slack/events').send({ event: { $x: 1 } });
+      expect(res.status).toBe(400);
+    });
+  });
+
+  describe('multipart fields', () => {
+    test('multer builds nested objects from bracketed field names, which is why it needs its own check', async () => {
+      const res = await request(app).post('/multipart-unguarded').field('podId[$ne]', 'x');
+      expect(res.body.body).toEqual({ podId: { $ne: 'x' } });
+    });
+
+    test('the check after multer refuses them', async () => {
+      const res = await request(app).post('/multipart').field('podId[$ne]', 'x');
+      expect(res.status).toBe(400);
+      expect(res.body.field).toBe('body.podId.$ne');
+    });
+  });
+});
+
+describe('findDollarKey', () => {
+  test('null for strings, Buffers and null', () => {
+    expect(findDollarKey('$x')).toBeNull();
+    expect(findDollarKey(Buffer.from('{"$x":1}'))).toBeNull();
+    expect(findDollarKey(null)).toBeNull();
+  });
+
+  test('counts null-prototype objects (what qs builds) as plain', () => {
+    const o = Object.create(null);
+    o.$ne = 1;
+    expect(findDollarKey({ a: o })).toBe('a.$ne');
+  });
+});
+
+describe('where the guard is mounted', () => {
+  const src = (p) => fs.readFileSync(path.join(__dirname, '../../..', p), 'utf8');
+
+  test('server.ts: after the global express.json() and before the first route', () => {
+    const server = src('server.ts');
+    const json = server.indexOf('app.use(express.json());');
+    const guard = server.indexOf('app.use(refuseDollarKeys);');
+    const firstRoute = server.indexOf("app.use('/api/auth'");
+    expect(json).toBeGreaterThan(-1);
+    expect(guard).toBeGreaterThan(json);
+    expect(firstRoute).toBeGreaterThan(guard);
+  });
+
+  test('both multipart routes run it after multer', () => {
+    expect(src('routes/pods.ts')).toMatch(/upload\.single\('qrCode'\), refuseDollarKeys,/);
+    expect(src('routes/uploads.ts')).toMatch(/return refuseDollarKeys\(req as never, res as never, next as never\);/);
+  });
+});
diff --git a/backend/middleware/refuseDollarKeys.ts b/backend/middleware/refuseDollarKeys.ts
new file mode 100644
index 0000000000..ba7bf55bad
--- /dev/null
+++ b/backend/middleware/refuseDollarKeys.ts
@@ -0,0 +1,96 @@
+import type { Request, Response, NextFunction } from 'express';
+
+// Request keys that start with `$` are refused, in the query string and in the
+// body, at any depth, before any route runs. `$` opens a MongoDB operator name,
+// and no client of this API sends such a key: the 2026-10-18 audit read every
+// client in the repo, the openclaw extension at its pin, and every stored Mongo
+// and Postgres document, and found none. One check here keeps them out of
+// every handler, instead of each handler re-checking its own inputs.
+//
+// Mounted in server.ts right after the global express.json(), so every
+// route-specific parser has already run. Multipart routes parse their fields
+// later, inside multer, so they call this again after multer.
+//
+// The two exemptions are the narrowest the audit found, each a subtree a
+// third party or a schema author legitimately fills with `$` keys:
+//   · POST /api/marketplace/publish: JSON Schema keywords inside a
+//     component's `widgetConfigSchema` or `schemaFields` (stored as free-form
+//     JSON, never used as a query);
+//   · /api/webhooks/slack: anything under `event.metadata.event_payload`,
+//     whose keys other Slack apps define. Refusing it would make Slack retry
+//     the event and then disable the subscription.
+
+export const DOLLAR_KEY_REFUSED = 'dollar_key_refused';
+
+const JSON_SCHEMA_KEYWORDS = new Set([
+  '$schema', '$id', '$ref', '$defs', '$comment', '$anchor', '$dynamicRef', '$dynamicAnchor', '$vocabulary',
+]);
+const SCHEMA_COMPONENT_FIELDS = new Set(['widgetConfigSchema', 'schemaFields']);
+
+type KeyPath = ReadonlyArray<string>;
+
+// true when `key`, found at `path` inside the body, is one of the exemptions.
+const exemptBodyKey = (req: Request, path: KeyPath, key: string): boolean => {
+  if (req.method === 'POST' && req.path === '/api/marketplace/publish') {
+    return path[0] === 'components' && /^\d+$/.test(path[1] || '')
+      && SCHEMA_COMPONENT_FIELDS.has(path[2] || '') && JSON_SCHEMA_KEYWORDS.has(key);
+  }
+  if (req.path === '/api/webhooks/slack' || req.path.startsWith('/api/webhooks/slack/')) {
+    return path[0] === 'event' && path[1] === 'metadata' && path[2] === 'event_payload';
+  }
+  return false;
+};
+
+// Plain objects and arrays only: a Buffer body (the signature-verified
+// webhooks) or a string is not inspected. qs builds null-prototype objects,
+// which this also counts as plain.
+const isContainer = (value: unknown): value is Record<string, unknown> | unknown[] => (
+  Array.isArray(value) || Object.prototype.toString.call(value) === '[object Object]'
+);
+
+// The dotted path of the first `$` key, or null. Iterative, so a deeply
+// nested body cannot exhaust the stack.
+export const findDollarKey = (
+  value: unknown,
+  isExempt: (path: KeyPath, key: string) => boolean = () => false,
+): string | null => {
+  if (!isContainer(value)) return null;
+  const stack: Array<{ node: unknown; path: string[] }> = [{ node: value, path: [] }];
+  while (stack.length) {
+    const { node, path } = stack.pop() as { node: unknown; path: string[] };
+    if (Array.isArray(node)) {
+      node.forEach((child, index) => {
+        if (isContainer(child)) stack.push({ node: child, path: [...path, String(index)] });
+      });
+    } else if (isContainer(node)) {
+      const keys = Object.keys(node);
+      for (let i = 0; i < keys.length; i += 1) {
+        const key = keys[i];
+        if (key.startsWith('$') && !isExempt(path, key)) return [...path, key].join('.');
+        // Exempt keys are walked too: an exemption allows a key at a path,
+        // never everything beneath it.
+        const child = (node as Record<string, unknown>)[key];
+        if (isContainer(child)) stack.push({ node: child, path: [...path, key] });
+      }
+    }
+  }
+  return null;
+};
+
+const refuse = (res: Response, where: string, field: string) => res.status(400).json({
+  message: `Keys starting with "$" are not accepted (${where}: ${field})`,
+  code: DOLLAR_KEY_REFUSED,
+  field: `${where}.${field}`,
+});
+
+export const refuseDollarKeys = (req: Request, res: Response, next: NextFunction) => {
+  const inQuery = findDollarKey(req.query);
+  if (inQuery) return refuse(res, 'query', inQuery);
+  const inBody = findDollarKey(req.body, (path, key) => exemptBodyKey(req, path, key));
+  if (inBody) return refuse(res, 'body', inBody);
+  return next();
+};
+
+// CJS compat for the require()-style imports used elsewhere in backend/.
+// eslint-disable-next-line @typescript-eslint/no-require-imports
+module.exports = { refuseDollarKeys, findDollarKey, DOLLAR_KEY_REFUSED };
diff --git a/backend/routes/pods.ts b/backend/routes/pods.ts
index ef2c10eeb6..c36ec21c38 100644
--- a/backend/routes/pods.ts
+++ b/backend/routes/pods.ts
@@ -9,6 +9,7 @@ const multer = require('multer');
 // eslint-disable-next-line global-require
 const auth = require('../middleware/auth');
 const { cloudflareIpRateLimitKeyGenerator } = require('../middleware/ipRateLimit');
+const { refuseDollarKeys } = require('../middleware/refuseDollarKeys');
 // eslint-disable-next-line global-require
 const { getAllPods, getPodsByType, getPodById, createPod, joinPod, leavePod, removeMember, deletePod } = require('../controllers/podController');
 // eslint-disable-next-line global-require
@@ -220,7 +221,7 @@ const deriveLinkName = (rawUrl: string): string => {
   }
 };
 
-router.post('/external-link', auth, upload.single('qrCode'), async (req: AuthReq, res: Res) => {
+router.post('/external-link', auth, upload.single('qrCode'), refuseDollarKeys, async (req: AuthReq, res: Res) => {
   try {
     const { podId, name: rawName, type: rawType, url } = (req.body || {}) as { podId?: string; name?: string; type?: string; url?: string };
     if (!podId) return res.status(400).json({ message: 'Missing podId' });
diff --git a/backend/routes/uploads.ts b/backend/routes/uploads.ts
index 58ddd8f85f..be1cf671e4 100644
--- a/backend/routes/uploads.ts
+++ b/backend/routes/uploads.ts
@@ -39,6 +39,7 @@ const multer = require('multer');
 const File = require('../models/File');
 // eslint-disable-next-line global-require
 const auth = require('../middleware/auth');
+const { refuseDollarKeys } = require('../middleware/refuseDollarKeys');
 // eslint-disable-next-line global-require
 const agentRuntimeAuth = require('../middleware/agentRuntimeAuth');
 import { getObjectStore } from '../services/objectStore';
@@ -131,7 +132,9 @@ const uploadSingle = (field: string) => (req: AuthReq, res: Res, next: (err?: un
       }
       return res.status(400).json({ msg: e?.message || 'Invalid upload' });
     }
-    next();
+    // multer parses the multipart fields only now, after the global
+    // `$`-key check has run, so the fields are checked here too.
+    return refuseDollarKeys(req as never, res as never, next as never);
   });
 };
 
diff --git a/backend/server.ts b/backend/server.ts
index 8baf424c5b..216b066b02 100644
--- a/backend/server.ts
+++ b/backend/server.ts
@@ -44,6 +44,7 @@ const moltbotProviderRoutes = require('./routes/providers/moltbot');
 const activityRoutes = require('./routes/activity');
 const marketplaceRoutes = require('./routes/marketplace');
 const marketplaceApiRoutes = require('./routes/marketplace-api');
+const { refuseDollarKeys } = require('./middleware/refuseDollarKeys');
 const gatewayRoutes = require('./routes/gateways');
 const skillsRoutes = require('./routes/skills');
 const devRoutes = require('./routes/dev');
@@ -235,6 +236,10 @@ app.use('/api/webhooks/github', githubGigWebhookRoutes.githubGigRawError);
 // JSON parsing for all other routes
 app.use(express.json());
 
+// Refuse `$`-prefixed keys in every query string and body, after every
+// parser above and before any route (middleware/refuseDollarKeys.ts).
+app.use(refuseDollarKeys);
+
 // Routes
 app.use('/api/auth', authRoutes);
 app.use('/api/posts', postRoutes);

From e7bad85745b5d4857003d98447843a5f2f3fb9c8 Mon Sep 17 00:00:00 2001
From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com>
Date: Sun, 25 Oct 2026 09:27:11 -0700
Subject: [PATCH 2/8] perf(backend): walk request keys in one linear pass, with
 depth and key caps

The key walk copied the whole path for every container it pushed, so a
chain N deep cost O(N^2) time and memory. Frames now keep a parent pointer
and the first three segments (all the exemptions read); the dotted path is
built only for a refused key.

A container deeper than MAX_KEY_DEPTH (64), or more than MAX_KEYS (100,000)
keys and array elements, stops the walk with a 400 that names the limit
(code key_limit_exceeded, limit, max, field). A body exactly at the depth
cap passes, and a $ key at the cap is still found.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VD3azQJZrdMmw8g51iscWD
---
 .../unit/middleware/refuseDollarKeys.test.js  |  53 +++++++-
 backend/middleware/refuseDollarKeys.ts        | 124 ++++++++++++++----
 2 files changed, 145 insertions(+), 32 deletions(-)

diff --git a/backend/__tests__/unit/middleware/refuseDollarKeys.test.js b/backend/__tests__/unit/middleware/refuseDollarKeys.test.js
index 417cc13833..dbe8cfbc29 100644
--- a/backend/__tests__/unit/middleware/refuseDollarKeys.test.js
+++ b/backend/__tests__/unit/middleware/refuseDollarKeys.test.js
@@ -9,7 +9,9 @@ const path = require('path');
 const express = require('express');
 const multer = require('multer');
 const request = require('supertest');
-const { refuseDollarKeys, findDollarKey, DOLLAR_KEY_REFUSED } = require('../../../middleware/refuseDollarKeys');
+const {
+  refuseDollarKeys, findDollarKey, scanKeys, DOLLAR_KEY_REFUSED, KEY_LIMIT_EXCEEDED, MAX_KEY_DEPTH, MAX_KEYS,
+} = require('../../../middleware/refuseDollarKeys');
 
 const buildApp = () => {
   const app = express();
@@ -59,11 +61,50 @@ describe('refuseDollarKeys', () => {
     expect(res.status).toBe(200);
   });
 
-  test('a deeply nested body does not exhaust the stack', async () => {
-    let body = { leaf: 1 };
-    for (let i = 0; i < 5000; i += 1) body = { n: body };
-    const res = await request(app).post('/x').send(body);
-    expect(res.status).toBe(200);
+  // `levels` containers below the root: the root is depth 0, the innermost
+  // container is depth `levels`.
+  const nest = (levels, leaf = { leaf: 1 }) => {
+    let body = leaf;
+    for (let i = 1; i < levels; i += 1) body = { n: body };
+    return { n: body };
+  };
+
+  describe('caps', () => {
+    test('a body nested exactly MAX_KEY_DEPTH levels passes', async () => {
+      const res = await request(app).post('/x').send(nest(MAX_KEY_DEPTH));
+      expect(res.status).toBe(200);
+    });
+
+    test('a $ key at the depth cap is still found', async () => {
+      const res = await request(app).post('/x').send(nest(MAX_KEY_DEPTH, { $where: '1' }));
+      expect(res.status).toBe(400);
+      expect(res.body.code).toBe(DOLLAR_KEY_REFUSED);
+    });
+
+    test('one level past the cap is refused, naming the depth limit', async () => {
+      const res = await request(app).post('/x').send(nest(MAX_KEY_DEPTH + 1));
+      expect(res.status).toBe(400);
+      expect(res.body).toMatchObject({ code: KEY_LIMIT_EXCEEDED, limit: 'MAX_KEY_DEPTH', max: MAX_KEY_DEPTH });
+      expect(res.body.message).toContain(String(MAX_KEY_DEPTH));
+      expect(res.body.field.startsWith('body.n.n')).toBe(true);
+    });
+
+    test('a 5000-deep body is refused at the cap without exhausting the stack', async () => {
+      const res = await request(app).post('/x').send(nest(5000));
+      expect(res.status).toBe(400);
+      expect(res.body.limit).toBe('MAX_KEY_DEPTH');
+    });
+
+    test('a $ key past the depth cap is not reported as passing', async () => {
+      const res = await request(app).post('/x').send(nest(MAX_KEY_DEPTH + 5, { $where: '1' }));
+      expect(res.status).toBe(400);
+    });
+
+    test('more than MAX_KEYS keys and elements is refused, naming the key limit', () => {
+      const wide = { a: new Array(MAX_KEYS).fill(0), b: 1 };
+      expect(scanKeys(wide)).toMatchObject({ kind: 'limit', limit: 'keys', max: MAX_KEYS });
+      expect(scanKeys({ a: new Array(MAX_KEYS - 1).fill(0) })).toBeNull();
+    });
   });
 
   describe('marketplace publish: JSON Schema keywords in schema fields only', () => {
diff --git a/backend/middleware/refuseDollarKeys.ts b/backend/middleware/refuseDollarKeys.ts
index ba7bf55bad..97646f19cf 100644
--- a/backend/middleware/refuseDollarKeys.ts
+++ b/backend/middleware/refuseDollarKeys.ts
@@ -48,49 +48,121 @@ const isContainer = (value: unknown): value is Record<string, unknown> | unknown
   Array.isArray(value) || Object.prototype.toString.call(value) === '[object Object]'
 );
 
-// The dotted path of the first `$` key, or null. Iterative, so a deeply
-// nested body cannot exhaust the stack.
-export const findDollarKey = (
+// The walk visits each key and array element once. A frame keeps its parent
+// and its own key instead of a copy of the whole path, so a chain N deep costs
+// O(N) rather than O(N^2); the dotted path is built only for the key that is
+// refused. Exemptions look at most three segments deep, so each frame carries
+// just those (`head`).
+//
+// Two caps bound the work on a body the parser accepted. A container deeper
+// than MAX_KEY_DEPTH, or more than MAX_KEYS keys and elements in total, stops
+// the walk and the request is refused, naming the cap. Neither is reachable by
+// a legitimate client: the global express.json() keeps a body under 100 KB
+// (about 50,000 values at most), and Mongo itself refuses nesting past 100.
+export const MAX_KEY_DEPTH = 64;
+export const MAX_KEYS = 100_000;
+export const KEY_LIMIT_EXCEEDED = 'key_limit_exceeded';
+
+const HEAD_SEGMENTS = 3;
+
+interface Frame {
+  node: Record<string, unknown> | unknown[];
+  parent: Frame | null;
+  key: string;
+  depth: number;
+  head: string[];
+}
+
+export type KeyScan =
+  | { kind: 'dollar'; field: string }
+  | { kind: 'limit'; limit: 'depth' | 'keys'; max: number; field: string }
+  | null;
+
+const pathOf = (frame: Frame | null, key?: string): string => {
+  const parts: string[] = key === undefined ? [] : [key];
+  for (let f = frame; f && f.parent; f = f.parent) parts.push(f.key);
+  return parts.reverse().join('.');
+};
+
+export const scanKeys = (
   value: unknown,
   isExempt: (path: KeyPath, key: string) => boolean = () => false,
-): string | null => {
+): KeyScan => {
   if (!isContainer(value)) return null;
-  const stack: Array<{ node: unknown; path: string[] }> = [{ node: value, path: [] }];
+  const stack: Frame[] = [{ node: value, parent: null, key: '', depth: 0, head: [] }];
+  let seen = 0;
   while (stack.length) {
-    const { node, path } = stack.pop() as { node: unknown; path: string[] };
-    if (Array.isArray(node)) {
-      node.forEach((child, index) => {
-        if (isContainer(child)) stack.push({ node: child, path: [...path, String(index)] });
-      });
-    } else if (isContainer(node)) {
-      const keys = Object.keys(node);
-      for (let i = 0; i < keys.length; i += 1) {
-        const key = keys[i];
-        if (key.startsWith('$') && !isExempt(path, key)) return [...path, key].join('.');
-        // Exempt keys are walked too: an exemption allows a key at a path,
-        // never everything beneath it.
-        const child = (node as Record<string, unknown>)[key];
-        if (isContainer(child)) stack.push({ node: child, path: [...path, key] });
+    const frame = stack.pop() as Frame;
+    const { node } = frame;
+    const keys = Array.isArray(node) ? null : Object.keys(node);
+    const count = keys ? keys.length : (node as unknown[]).length;
+    seen += count;
+    if (seen > MAX_KEYS) return { kind: 'limit', limit: 'keys', max: MAX_KEYS, field: pathOf(frame) };
+    for (let i = 0; i < count; i += 1) {
+      const key = keys ? keys[i] : String(i);
+      if (keys && key.startsWith('$') && !isExempt(frame.head, key)) {
+        return { kind: 'dollar', field: pathOf(frame, key) };
+      }
+      // Exempt keys are walked too: an exemption allows a key at a path,
+      // never everything beneath it.
+      const child = keys ? (node as Record<string, unknown>)[key] : (node as unknown[])[i];
+      if (!isContainer(child)) continue;
+      if (frame.depth + 1 > MAX_KEY_DEPTH) {
+        return { kind: 'limit', limit: 'depth', max: MAX_KEY_DEPTH, field: pathOf(frame, key) };
       }
+      stack.push({
+        node: child,
+        parent: frame,
+        key,
+        depth: frame.depth + 1,
+        head: frame.head.length < HEAD_SEGMENTS ? [...frame.head, key] : frame.head,
+      });
     }
   }
   return null;
 };
 
+// The dotted path of the first `$` key, or null (a cap that was hit counts as
+// no `$` key here; refuseDollarKeys refuses those separately).
+export const findDollarKey = (
+  value: unknown,
+  isExempt: (path: KeyPath, key: string) => boolean = () => false,
+): string | null => {
+  const scan = scanKeys(value, isExempt);
+  return scan && scan.kind === 'dollar' ? scan.field : null;
+};
+
 const refuse = (res: Response, where: string, field: string) => res.status(400).json({
   message: `Keys starting with "$" are not accepted (${where}: ${field})`,
   code: DOLLAR_KEY_REFUSED,
   field: `${where}.${field}`,
 });
 
-export const refuseDollarKeys = (req: Request, res: Response, next: NextFunction) => {
-  const inQuery = findDollarKey(req.query);
-  if (inQuery) return refuse(res, 'query', inQuery);
-  const inBody = findDollarKey(req.body, (path, key) => exemptBodyKey(req, path, key));
-  if (inBody) return refuse(res, 'body', inBody);
-  return next();
+const refuseLimit = (res: Response, where: string, scan: { limit: 'depth' | 'keys'; max: number; field: string }) => (
+  res.status(400).json({
+    message: scan.limit === 'depth'
+      ? `The ${where} is nested more than ${scan.max} levels deep`
+      : `The ${where} has more than ${scan.max} keys and array elements`,
+    code: KEY_LIMIT_EXCEEDED,
+    limit: scan.limit === 'depth' ? 'MAX_KEY_DEPTH' : 'MAX_KEYS',
+    max: scan.max,
+    field: scan.field ? `${where}.${scan.field}` : where,
+  })
+);
+
+const check = (res: Response, where: string, scan: KeyScan) => {
+  if (!scan) return null;
+  return scan.kind === 'dollar' ? refuse(res, where, scan.field) : refuseLimit(res, where, scan);
 };
 
+export const refuseDollarKeys = (req: Request, res: Response, next: NextFunction) => (
+  check(res, 'query', scanKeys(req.query))
+  || check(res, 'body', scanKeys(req.body, (path, key) => exemptBodyKey(req, path, key)))
+  || next()
+);
+
 // CJS compat for the require()-style imports used elsewhere in backend/.
 // eslint-disable-next-line @typescript-eslint/no-require-imports
-module.exports = { refuseDollarKeys, findDollarKey, DOLLAR_KEY_REFUSED };
+module.exports = {
+  refuseDollarKeys, findDollarKey, scanKeys, DOLLAR_KEY_REFUSED, KEY_LIMIT_EXCEEDED, MAX_KEY_DEPTH, MAX_KEYS,
+};

From 96098735d5f907c94ffa9a9728aebd1ed1dfcc2c Mon Sep 17 00:00:00 2001
From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com>
Date: Sun, 25 Oct 2026 09:37:51 -0700
Subject: [PATCH 3/8] test(backend): keep the deep-body check independent of
 the JSON parser

A 5000-deep body failed in CI with a 400 that was not the guard's. The
HTTP test now sends 1,000 levels and prints the whole response on failure;
a 100,000-deep object built in memory checks the walk itself.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VD3azQJZrdMmw8g51iscWD
---
 .../unit/middleware/refuseDollarKeys.test.js      | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/backend/__tests__/unit/middleware/refuseDollarKeys.test.js b/backend/__tests__/unit/middleware/refuseDollarKeys.test.js
index dbe8cfbc29..e7e180bf70 100644
--- a/backend/__tests__/unit/middleware/refuseDollarKeys.test.js
+++ b/backend/__tests__/unit/middleware/refuseDollarKeys.test.js
@@ -89,10 +89,17 @@ describe('refuseDollarKeys', () => {
       expect(res.body.field.startsWith('body.n.n')).toBe(true);
     });
 
-    test('a 5000-deep body is refused at the cap without exhausting the stack', async () => {
-      const res = await request(app).post('/x').send(nest(5000));
-      expect(res.status).toBe(400);
-      expect(res.body.limit).toBe('MAX_KEY_DEPTH');
+    test('a 1000-deep body is refused at the cap, naming it', async () => {
+      const res = await request(app).post('/x').send(nest(1000));
+      expect({ status: res.status, body: res.body }).toMatchObject({
+        status: 400, body: { code: KEY_LIMIT_EXCEEDED, limit: 'MAX_KEY_DEPTH' },
+      });
+    });
+
+    // Built in memory, not sent: a JSON parser may refuse this depth on its
+    // own, which would hide what the walk does with it.
+    test('a 100,000-deep object stops at the cap without exhausting the stack', () => {
+      expect(scanKeys(nest(100000))).toMatchObject({ kind: 'limit', limit: 'depth', max: MAX_KEY_DEPTH });
     });
 
     test('a $ key past the depth cap is not reported as passing', async () => {

From 6f09d0bf79d4db06bb02590d9e6f023698b3b649 Mon Sep 17 00:00:00 2001
From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com>
Date: Sun, 25 Oct 2026 09:38:18 -0700
Subject: [PATCH 4/8] refactor(backend): one key list for arrays and objects in
 the key walk

Array indices become string keys, so the loop reads no length from a
value whose type it would have to assert.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VD3azQJZrdMmw8g51iscWD
---
 backend/middleware/refuseDollarKeys.ts | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/backend/middleware/refuseDollarKeys.ts b/backend/middleware/refuseDollarKeys.ts
index 97646f19cf..a5464bdec5 100644
--- a/backend/middleware/refuseDollarKeys.ts
+++ b/backend/middleware/refuseDollarKeys.ts
@@ -94,18 +94,18 @@ export const scanKeys = (
   while (stack.length) {
     const frame = stack.pop() as Frame;
     const { node } = frame;
-    const keys = Array.isArray(node) ? null : Object.keys(node);
-    const count = keys ? keys.length : (node as unknown[]).length;
-    seen += count;
+    // Array indices become keys too ('0', '1', ...); none starts with `$`.
+    const keys = Array.isArray(node) ? Array.from(node.keys(), String) : Object.keys(node);
+    seen += keys.length;
     if (seen > MAX_KEYS) return { kind: 'limit', limit: 'keys', max: MAX_KEYS, field: pathOf(frame) };
-    for (let i = 0; i < count; i += 1) {
-      const key = keys ? keys[i] : String(i);
-      if (keys && key.startsWith('$') && !isExempt(frame.head, key)) {
+    for (let i = 0; i < keys.length; i += 1) {
+      const key = keys[i];
+      if (key.startsWith('$') && !isExempt(frame.head, key)) {
         return { kind: 'dollar', field: pathOf(frame, key) };
       }
       // Exempt keys are walked too: an exemption allows a key at a path,
       // never everything beneath it.
-      const child = keys ? (node as Record<string, unknown>)[key] : (node as unknown[])[i];
+      const child = (node as Record<string, unknown>)[key];
       if (!isContainer(child)) continue;
       if (frame.depth + 1 > MAX_KEY_DEPTH) {
         return { kind: 'limit', limit: 'depth', max: MAX_KEY_DEPTH, field: pathOf(frame, key) };

From f345b248739bc584209a00ea651e279b1c98aecc Mon Sep 17 00:00:00 2001
From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com>
Date: Sun, 25 Oct 2026 09:47:29 -0700
Subject: [PATCH 5/8] fix(backend): bound sparse arrays by length; rate-limit
 external-link creation

The key walk now measures an array by its length before listing its
indices, so a multipart field like a[99999999]=x is refused before a
hundred-million-entry index list is built (CodeRabbit).

POST /api/pods/external-link gets a 30/min token-keyed limiter and leaves
the rate-limit baseline. CodeQL flagged it as unlimited once this PR
touched the line.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VD3azQJZrdMmw8g51iscWD
---
 .../unit/middleware/refuseDollarKeys.test.js  | 19 +++++++++++++++++++
 .../routes/routeRateLimitGuard.baseline.json  |  1 -
 backend/middleware/refuseDollarKeys.ts        |  6 ++++++
 backend/routes/pods.ts                        | 13 ++++++++++++-
 4 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/backend/__tests__/unit/middleware/refuseDollarKeys.test.js b/backend/__tests__/unit/middleware/refuseDollarKeys.test.js
index e7e180bf70..11367b74fe 100644
--- a/backend/__tests__/unit/middleware/refuseDollarKeys.test.js
+++ b/backend/__tests__/unit/middleware/refuseDollarKeys.test.js
@@ -112,6 +112,25 @@ describe('refuseDollarKeys', () => {
       expect(scanKeys(wide)).toMatchObject({ kind: 'limit', limit: 'keys', max: MAX_KEYS });
       expect(scanKeys({ a: new Array(MAX_KEYS - 1).fill(0) })).toBeNull();
     });
+
+    test('a sparse array is refused by its length, before its indices are listed', () => {
+      const sparse = [];
+      sparse[2 ** 32 - 2] = 'x';
+      const keys = jest.spyOn(Array.prototype, 'keys');
+      try {
+        expect(scanKeys({ a: sparse })).toMatchObject({ kind: 'limit', limit: 'keys', field: 'a' });
+        expect(keys).not.toHaveBeenCalled();
+      } finally {
+        keys.mockRestore();
+      }
+    });
+
+    test('multer turns a bracketed index into that sparse array, and the route refuses it', async () => {
+      const res = await request(app).post('/multipart').field('a[99999999]', 'x');
+      expect({ status: res.status, body: res.body }).toMatchObject({
+        status: 400, body: { code: KEY_LIMIT_EXCEEDED, limit: 'MAX_KEYS' },
+      });
+    });
   });
 
   describe('marketplace publish: JSON Schema keywords in schema fields only', () => {
diff --git a/backend/__tests__/unit/routes/routeRateLimitGuard.baseline.json b/backend/__tests__/unit/routes/routeRateLimitGuard.baseline.json
index 26f90b1080..d10e0239b0 100644
--- a/backend/__tests__/unit/routes/routeRateLimitGuard.baseline.json
+++ b/backend/__tests__/unit/routes/routeRateLimitGuard.baseline.json
@@ -117,7 +117,6 @@
   "routes/pods.ts POST / [unlimited]",
   "routes/pods.ts POST /:id/leave [unlimited]",
   "routes/pods.ts POST /announcement [unlimited]",
-  "routes/pods.ts POST /external-link [unlimited]",
   "routes/registry/admin.ts DELETE /admin/installations/:installationId [unlimited]",
   "routes/registry/admin.ts DELETE /admin/installations/:installationId/runtime-tokens/:tokenId [unlimited]",
   "routes/registry/admin.ts GET /admin/installations [unlimited]",
diff --git a/backend/middleware/refuseDollarKeys.ts b/backend/middleware/refuseDollarKeys.ts
index a5464bdec5..cfa5d269d5 100644
--- a/backend/middleware/refuseDollarKeys.ts
+++ b/backend/middleware/refuseDollarKeys.ts
@@ -94,6 +94,12 @@ export const scanKeys = (
   while (stack.length) {
     const frame = stack.pop() as Frame;
     const { node } = frame;
+    // An array is measured by its length before its indices are listed:
+    // multer builds `a[999999999]=x` as a sparse array of that length.
+    const size = Array.isArray(node) ? node.length : null;
+    if (size !== null && seen + size > MAX_KEYS) {
+      return { kind: 'limit', limit: 'keys', max: MAX_KEYS, field: pathOf(frame) };
+    }
     // Array indices become keys too ('0', '1', ...); none starts with `$`.
     const keys = Array.isArray(node) ? Array.from(node.keys(), String) : Object.keys(node);
     seen += keys.length;
diff --git a/backend/routes/pods.ts b/backend/routes/pods.ts
index c36ec21c38..ff44cd0b74 100644
--- a/backend/routes/pods.ts
+++ b/backend/routes/pods.ts
@@ -221,7 +221,18 @@ const deriveLinkName = (rawUrl: string): string => {
   }
 };
 
-router.post('/external-link', auth, upload.single('qrCode'), refuseDollarKeys, async (req: AuthReq, res: Res) => {
+// Creating an external link writes a row and may store an uploaded QR image.
+// CodeQL flags it as `js/missing-rate-limiting`; it had been unlimited since
+// the route landed (routeRateLimitGuard.baseline.json, row deleted with this
+// change). Keyed on the token, like the focus writers above.
+const externalLinkCreateRateLimit = rateLimit({
+  windowMs: 60 * 1000,
+  limit: 30,
+  standardHeaders: 'draft-7',
+  legacyHeaders: false,
+  keyGenerator: podJoinRateLimitKey,
+});
+router.post('/external-link', externalLinkCreateRateLimit, auth, upload.single('qrCode'), refuseDollarKeys, async (req: AuthReq, res: Res) => {
   try {
     const { podId, name: rawName, type: rawType, url } = (req.body || {}) as { podId?: string; name?: string; type?: string; url?: string };
     if (!podId) return res.status(400).json({ message: 'Missing podId' });

From 27cb9299549463ae9ba9a9bd1cb4e9c1e6827ff6 Mon Sep 17 00:00:00 2001
From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com>
Date: Sun, 25 Oct 2026 10:01:24 -0700
Subject: [PATCH 6/8] fix(backend): remove a stored upload when the key guard
 refuses the request

multer writes a disk-stored file before the guard runs on a multipart
route, and a refused request never reaches the handler that keeps or
deletes it. The guard now unlinks req.file / req.files on refusal
(CodeRabbit). A test runs multer disk storage against a temp dir.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VD3azQJZrdMmw8g51iscWD
---
 .../unit/middleware/refuseDollarKeys.test.js  | 21 ++++++++++++++++
 backend/middleware/refuseDollarKeys.ts        | 25 +++++++++++++++----
 2 files changed, 41 insertions(+), 5 deletions(-)

diff --git a/backend/__tests__/unit/middleware/refuseDollarKeys.test.js b/backend/__tests__/unit/middleware/refuseDollarKeys.test.js
index 11367b74fe..43b08dd565 100644
--- a/backend/__tests__/unit/middleware/refuseDollarKeys.test.js
+++ b/backend/__tests__/unit/middleware/refuseDollarKeys.test.js
@@ -5,6 +5,7 @@
 // the global express.json(), then the guard, then the routes.
 
 const fs = require('fs');
+const os = require('os');
 const path = require('path');
 const express = require('express');
 const multer = require('multer');
@@ -174,6 +175,26 @@ describe('refuseDollarKeys', () => {
   });
 
   describe('multipart fields', () => {
+    test('a refused request leaves no uploaded file on disk', async () => {
+      const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'refuse-dollar-'));
+      try {
+        const diskApp = express();
+        diskApp.post('/qr', multer({ dest: dir }).single('qrCode'), refuseDollarKeys, (req, res) => res.json({ ok: true }));
+        const res = await request(diskApp).post('/qr').field('podId[$ne]', 'x').attach('qrCode', Buffer.from('png'), 'qr.png');
+        expect(res.status).toBe(400);
+        // The unlink is fire-and-forget; give it a moment to land.
+        for (let i = 0; i < 20 && fs.readdirSync(dir).length; i += 1) {
+          await new Promise((r) => { setTimeout(r, 25); });
+        }
+        expect(fs.readdirSync(dir)).toEqual([]);
+        const kept = await request(diskApp).post('/qr').field('podId', 'x').attach('qrCode', Buffer.from('png'), 'qr.png');
+        expect(kept.status).toBe(200);
+        expect(fs.readdirSync(dir)).toHaveLength(1);
+      } finally {
+        fs.rmSync(dir, { recursive: true, force: true });
+      }
+    });
+
     test('multer builds nested objects from bracketed field names, which is why it needs its own check', async () => {
       const res = await request(app).post('/multipart-unguarded').field('podId[$ne]', 'x');
       expect(res.body.body).toEqual({ podId: { $ne: 'x' } });
diff --git a/backend/middleware/refuseDollarKeys.ts b/backend/middleware/refuseDollarKeys.ts
index cfa5d269d5..cb4f2a7cfb 100644
--- a/backend/middleware/refuseDollarKeys.ts
+++ b/backend/middleware/refuseDollarKeys.ts
@@ -1,3 +1,4 @@
+import { unlink } from 'fs';
 import type { Request, Response, NextFunction } from 'express';
 
 // Request keys that start with `$` are refused, in the query string and in the
@@ -161,11 +162,25 @@ const check = (res: Response, where: string, scan: KeyScan) => {
   return scan.kind === 'dollar' ? refuse(res, where, scan.field) : refuseLimit(res, where, scan);
 };
 
-export const refuseDollarKeys = (req: Request, res: Response, next: NextFunction) => (
-  check(res, 'query', scanKeys(req.query))
-  || check(res, 'body', scanKeys(req.body, (path, key) => exemptBodyKey(req, path, key)))
-  || next()
-);
+// On a multipart route, multer has already written any disk-stored file by the
+// time this runs. A refused request never reaches the handler that would keep
+// or remove it, so the guard removes it.
+const discardUploads = (req: Request) => {
+  const { file, files } = req as Request & { file?: { path?: unknown }; files?: unknown };
+  const all: Array<{ path?: unknown }> = [];
+  if (file) all.push(file);
+  if (Array.isArray(files)) all.push(...files);
+  else if (files && typeof files === 'object') Object.values(files).forEach((list) => { if (Array.isArray(list)) all.push(...list); });
+  all.forEach(({ path }) => { if (typeof path === 'string') unlink(path, () => {}); });
+};
+
+export const refuseDollarKeys = (req: Request, res: Response, next: NextFunction) => {
+  const refused = check(res, 'query', scanKeys(req.query))
+    || check(res, 'body', scanKeys(req.body, (path, key) => exemptBodyKey(req, path, key)));
+  if (!refused) return next();
+  discardUploads(req);
+  return refused;
+};
 
 // CJS compat for the require()-style imports used elsewhere in backend/.
 // eslint-disable-next-line @typescript-eslint/no-require-imports

From ea14a10806f03fefc2d496829efe886fbdf0e57e Mon Sep 17 00:00:00 2001
From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com>
Date: Sun, 25 Oct 2026 10:11:46 -0700
Subject: [PATCH 7/8] fix(backend): remove refused uploads only inside uploads/

The cleanup unlinked whatever path the request object carried. It now
resolves the path and removes it only under the app's uploads/ directory,
where every disk-storing route writes (CodeQL js/path-injection). A test
shows a path outside uploads/ is left alone.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VD3azQJZrdMmw8g51iscWD
---
 .../unit/middleware/refuseDollarKeys.test.js  | 19 +++++++++++++++++--
 backend/middleware/refuseDollarKeys.ts        | 11 +++++++++--
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/backend/__tests__/unit/middleware/refuseDollarKeys.test.js b/backend/__tests__/unit/middleware/refuseDollarKeys.test.js
index 43b08dd565..fe6c40102f 100644
--- a/backend/__tests__/unit/middleware/refuseDollarKeys.test.js
+++ b/backend/__tests__/unit/middleware/refuseDollarKeys.test.js
@@ -5,7 +5,6 @@
 // the global express.json(), then the guard, then the routes.
 
 const fs = require('fs');
-const os = require('os');
 const path = require('path');
 const express = require('express');
 const multer = require('multer');
@@ -175,8 +174,24 @@ describe('refuseDollarKeys', () => {
   });
 
   describe('multipart fields', () => {
+    test('a refused request never removes a file outside uploads/', async () => {
+      const outside = path.join(fs.mkdtempSync(path.join(require('os').tmpdir(), 'refuse-dollar-')), 'keep.txt');
+      fs.writeFileSync(outside, 'x');
+      try {
+        const req = { query: {}, body: { $where: '1' }, file: { path: outside }, files: [{ path: '../../keep.txt' }], method: 'POST', path: '/x' };
+        const res = { status: () => res, json: () => res };
+        refuseDollarKeys(req, res, () => { throw new Error('should refuse'); });
+        await new Promise((r) => { setTimeout(r, 100); });
+        expect(fs.existsSync(outside)).toBe(true);
+      } finally {
+        fs.rmSync(path.dirname(outside), { recursive: true, force: true });
+      }
+    });
+
     test('a refused request leaves no uploaded file on disk', async () => {
-      const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'refuse-dollar-'));
+      // Under uploads/, the only place the guard removes files from.
+      fs.mkdirSync('uploads', { recursive: true });
+      const dir = fs.mkdtempSync(path.join('uploads', 'refuse-dollar-'));
       try {
         const diskApp = express();
         diskApp.post('/qr', multer({ dest: dir }).single('qrCode'), refuseDollarKeys, (req, res) => res.json({ ok: true }));
diff --git a/backend/middleware/refuseDollarKeys.ts b/backend/middleware/refuseDollarKeys.ts
index cb4f2a7cfb..49433cbfb5 100644
--- a/backend/middleware/refuseDollarKeys.ts
+++ b/backend/middleware/refuseDollarKeys.ts
@@ -1,4 +1,5 @@
 import { unlink } from 'fs';
+import { resolve, sep } from 'path';
 import type { Request, Response, NextFunction } from 'express';
 
 // Request keys that start with `$` are refused, in the query string and in the
@@ -164,14 +165,20 @@ const check = (res: Response, where: string, scan: KeyScan) => {
 
 // On a multipart route, multer has already written any disk-stored file by the
 // time this runs. A refused request never reaches the handler that would keep
-// or remove it, so the guard removes it.
+// or remove it, so the guard removes it — only inside the app's uploads/
+// directory, where every disk-storing route writes.
+const UPLOAD_ROOT = resolve('uploads');
 const discardUploads = (req: Request) => {
   const { file, files } = req as Request & { file?: { path?: unknown }; files?: unknown };
   const all: Array<{ path?: unknown }> = [];
   if (file) all.push(file);
   if (Array.isArray(files)) all.push(...files);
   else if (files && typeof files === 'object') Object.values(files).forEach((list) => { if (Array.isArray(list)) all.push(...list); });
-  all.forEach(({ path }) => { if (typeof path === 'string') unlink(path, () => {}); });
+  all.forEach(({ path }) => {
+    if (typeof path !== 'string') return;
+    const target = resolve(path);
+    if (target.startsWith(UPLOAD_ROOT + sep)) unlink(target, () => {});
+  });
 };
 
 export const refuseDollarKeys = (req: Request, res: Response, next: NextFunction) => {

From 51e90c93268a614b085f3f457d5fb51493ad7b72 Mon Sep 17 00:00:00 2001
From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com>
Date: Sun, 25 Oct 2026 10:33:39 -0700
Subject: [PATCH 8/8] fix($-key guard): cover the OAuth start form and literal
 query keys

- auth.ts runs the guard again after the OAuth start form's urlencoded
  parser, which runs after the global guard.
- The query is refused for a `$` anywhere in a key: qs stops at depth 5
  and keeps the rest literally (`[$gt]`).
- The exemption comment now states what the code does: an exemption
  covers the subtree under its prefix. The walk still descends, so the
  caps apply there (tested).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VD3azQJZrdMmw8g51iscWD
---
 .../unit/middleware/refuseDollarKeys.test.js  | 27 +++++++++++++++++++
 backend/middleware/refuseDollarKeys.ts        | 20 +++++++++++---
 backend/routes/auth.ts                        |  4 ++-
 3 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/backend/__tests__/unit/middleware/refuseDollarKeys.test.js b/backend/__tests__/unit/middleware/refuseDollarKeys.test.js
index fe6c40102f..e077e5a445 100644
--- a/backend/__tests__/unit/middleware/refuseDollarKeys.test.js
+++ b/backend/__tests__/unit/middleware/refuseDollarKeys.test.js
@@ -28,6 +28,8 @@ const buildApp = () => {
   // A multipart route, as pods.ts and uploads.ts have: multer parses its
   // fields after the global guard, so the route runs the guard again.
   app.post('/multipart', multer().none(), refuseDollarKeys, ok);
+  // A urlencoded route parser, as auth.ts has for the OAuth start form.
+  app.post('/form', express.urlencoded({ extended: false, limit: '4kb' }), refuseDollarKeys, ok);
   app.post('/multipart-unguarded', multer().none(), (req, res) => res.json({ body: req.body }));
   return app;
 };
@@ -45,6 +47,20 @@ describe('refuseDollarKeys', () => {
     expect(res.body).toMatchObject({ code: DOLLAR_KEY_REFUSED, field });
   });
 
+  test('refuses a query key past qs depth 5, kept literally with `$` inside it', async () => {
+    const res = await request(app).get('/x?a[b][c][d][e][f][$gt]=1');
+    expect(res.status).toBe(400);
+    expect(res.body).toMatchObject({ code: DOLLAR_KEY_REFUSED, field: 'query.a.b.c.d.e.f.[$gt]' });
+  });
+
+  test('refuses a `$` key in a urlencoded body parsed after the global guard', async () => {
+    const res = await request(app).post('/form').type('form').send('$gt=1');
+    expect(res.status).toBe(400);
+    expect(res.body).toMatchObject({ code: DOLLAR_KEY_REFUSED, field: 'body.$gt' });
+    const okRes = await request(app).post('/form').type('form').send('invite=abc');
+    expect(okRes.status).toBe(200);
+  });
+
   test('refuses a body key at any depth, arrays included', async () => {
     const res = await request(app).post('/x').send({ a: { b: [{ c: 1 }, { $gt: 1 }] } });
     expect(res.status).toBe(400);
@@ -167,6 +183,13 @@ describe('refuseDollarKeys', () => {
       expect(res.status).toBe(200);
     });
 
+    test('the walk still descends into event_payload, so the depth cap applies', async () => {
+      const res = await request(app).post('/api/webhooks/slack/events')
+        .send({ event: { metadata: { event_payload: nest(MAX_KEY_DEPTH) } } });
+      expect(res.status).toBe(400);
+      expect(res.body).toMatchObject({ code: KEY_LIMIT_EXCEEDED, limit: 'MAX_KEY_DEPTH' });
+    });
+
     test('refuses a key elsewhere in the event', async () => {
       const res = await request(app).post('/api/webhooks/slack/events').send({ event: { $x: 1 } });
       expect(res.status).toBe(400);
@@ -254,4 +277,8 @@ describe('where the guard is mounted', () => {
     expect(src('routes/pods.ts')).toMatch(/upload\.single\('qrCode'\), refuseDollarKeys,/);
     expect(src('routes/uploads.ts')).toMatch(/return refuseDollarKeys\(req as never, res as never, next as never\);/);
   });
+
+  test('the OAuth start form runs it after its urlencoded parser', () => {
+    expect(src('routes/auth.ts')).toMatch(/express\.urlencoded\(\{ extended: false, limit: '4kb' \}\), refuseDollarKeys, startOAuth\)/);
+  });
 });
diff --git a/backend/middleware/refuseDollarKeys.ts b/backend/middleware/refuseDollarKeys.ts
index 49433cbfb5..3a99ae8006 100644
--- a/backend/middleware/refuseDollarKeys.ts
+++ b/backend/middleware/refuseDollarKeys.ts
@@ -86,9 +86,12 @@ const pathOf = (frame: Frame | null, key?: string): string => {
   return parts.reverse().join('.');
 };
 
+const startsWithDollar = (key: string) => key.startsWith('$');
+
 export const scanKeys = (
   value: unknown,
   isExempt: (path: KeyPath, key: string) => boolean = () => false,
+  isDollarKey: (key: string) => boolean = startsWithDollar,
 ): KeyScan => {
   if (!isContainer(value)) return null;
   const stack: Frame[] = [{ node: value, parent: null, key: '', depth: 0, head: [] }];
@@ -108,11 +111,14 @@ export const scanKeys = (
     if (seen > MAX_KEYS) return { kind: 'limit', limit: 'keys', max: MAX_KEYS, field: pathOf(frame) };
     for (let i = 0; i < keys.length; i += 1) {
       const key = keys[i];
-      if (key.startsWith('$') && !isExempt(frame.head, key)) {
+      if (isDollarKey(key) && !isExempt(frame.head, key)) {
         return { kind: 'dollar', field: pathOf(frame, key) };
       }
-      // Exempt keys are walked too: an exemption allows a key at a path,
-      // never everything beneath it.
+      // Exemptions match on the first three segments only (`head`), so an
+      // exemption covers the whole subtree under that prefix: every key below
+      // event.metadata.event_payload, and each JSON Schema keyword at any depth
+      // inside a component's schema field. Other `$` keys there are still
+      // refused, and the walk still descends, so the caps still apply.
       const child = (node as Record<string, unknown>)[key];
       if (!isContainer(child)) continue;
       if (frame.depth + 1 > MAX_KEY_DEPTH) {
@@ -181,8 +187,14 @@ const discardUploads = (req: Request) => {
   });
 };
 
+// The query is refused for a `$` anywhere in a key, not just at its start:
+// qs stops nesting at depth 5 and keeps the rest of a key literally, so
+// `a[b][c][d][e][f][$gt]=1` arrives with a key `[$gt]` under `f`. No query exemption
+// exists, so this never refuses something the body rule would allow.
+const containsDollar = (key: string) => key.includes('$');
+
 export const refuseDollarKeys = (req: Request, res: Response, next: NextFunction) => {
-  const refused = check(res, 'query', scanKeys(req.query))
+  const refused = check(res, 'query', scanKeys(req.query, undefined, containsDollar))
     || check(res, 'body', scanKeys(req.body, (path, key) => exemptBodyKey(req, path, key)));
   if (!refused) return next();
   discardUploads(req);
diff --git a/backend/routes/auth.ts b/backend/routes/auth.ts
index 57c41c30dd..e82abe6a26 100644
--- a/backend/routes/auth.ts
+++ b/backend/routes/auth.ts
@@ -4,6 +4,7 @@ const express = require('express');
 const rateLimit = require('express-rate-limit');
 const { cloudflareIpRateLimitKeyGenerator } = require('../middleware/ipRateLimit');
 const { platformIpRateLimit } = require('../middleware/platformRateLimit');
+const { refuseDollarKeys } = require('../middleware/refuseDollarKeys');
 // eslint-disable-next-line global-require
 const auth = require('../middleware/auth');
 // eslint-disable-next-line global-require
@@ -228,7 +229,8 @@ router.get('/oauth/providers', getOAuthProviders);
 router.get('/oauth/:provider/start', oauthLimiter, startOAuth);
 // The same start as a form POST, so an invite code rides in the body, not the URL
 // (the register page uses it whenever it holds a code; TASK-404).
-router.post('/oauth/:provider/start', oauthLimiter, express.urlencoded({ extended: false, limit: '4kb' }), startOAuth);
+// This parser runs after the global $-key guard, so the guard runs again on its body.
+router.post('/oauth/:provider/start', oauthLimiter, express.urlencoded({ extended: false, limit: '4kb' }), refuseDollarKeys, startOAuth);
 router.get('/oauth/:provider/callback', oauthLimiter, oauthCallback);
 router.post('/oauth/exchange', oauthLimiter, exchangeOAuthCode);
 router.get('/registration-policy', registrationPolicyLimit, getRegistrationPolicy);