fix(key): Answer HTTP 400 for JSON nested over 64 levels

README.md

@@ -430,6 +430,7 @@ characters. A run without the token must use the same patterns as the run
 that recorded, or its keys do not match. Each key has a time limit: 1 s,
 and 0.5 s for each MiB of the body. The built-in patterns take about 60
 to 110 ms for each MiB. A request whose key takes longer fails with HTTP 400.
+So does a JSON body with arrays or objects nested over 64 levels deep.
 
 All runs share one process. Anyone can start a run without the token, so
 the service makes the keys of these runs in a worker process, one key at

packages/service/src/request-key.ts

@@ -14,6 +14,7 @@
  */
 import { createHash } from "node:crypto";
 import type { IncomingHttpHeaders } from "node:http";
+import { httpError } from "./errors.ts";
 import { parseJson } from "./json-strings.ts";
 import { THINKING_BLOCK_TYPES } from "./streams.ts";
 import type { RecordingRule } from "@sentry/roach/types";
@@ -152,15 +153,41 @@ function normalizeJson(
   };
 }
 
+/**
+ * A JSON body nested deeper than this gets HTTP 400. The key walks a body
+ * with recursion, and a few thousand levels fill the call stack. Model
+ * request bodies nest far less than this.
+ */
+const MAX_JSON_DEPTH = 64;
+
+/** Whether `json` has arrays or objects nested deeper than `limit`. */
+function nestedDeeperThan(json: unknown, limit: number): boolean {
+  // A loop, not recursion, so a deep body cannot fill the call stack here.
+  const values = [json];
+  const depths = [0];
+  while (values.length > 0) {
+    const value = values.pop();
+    const depth = depths.pop()!;
+    if (value === null || typeof value !== "object") continue;
+    if (depth >= limit) return true;
+    for (const item of Object.values(value)) {
+      values.push(item);
+      depths.push(depth + 1);
+    }
+  }
+  return false;
+}
+
 const sha256 = (text: string) =>
   createHash("sha256").update(text).digest("hex");
 /**
  * The key and the changing values of a request. `known` holds the values
  * of earlier requests in the same session (`@sentry/roach/values`). It
  * throws an error with HTTP status 400 when it takes longer than
- * `timeLimitMs`, by default `keyTimeLimit()` of the body, or when the
- * request has more than `maxValues` changing values. The limits never
- * change a key: a request gets its key, or it fails.
+ * `timeLimitMs`, by default `keyTimeLimit()` of the body, when the
+ * request has more than `maxValues` changing values, or when its JSON body
+ * is nested deeper than `MAX_JSON_DEPTH`. The limits never change a key:
+ * a request gets its key, or it fails.
  */
 export function keyRequest(
   rule: RecordingRule,
@@ -187,6 +214,13 @@ function keyRequestNow(
     ]),
   );
   const json = parseJson(request.body);
+  if (json !== undefined && nestedDeeperThan(json, MAX_JSON_DEPTH)) {
+    // It is the fault of the client, so it does not go to Sentry.
+    throw httpError(
+      400,
+      `the JSON body is nested over ${MAX_JSON_DEPTH} levels deep`,
+    );
+  }
   const { text, values } =
     json === undefined
       ? extractValues(request.body, rule.values, known, maxValues)

packages/service/tests/service.test.ts

@@ -91,6 +91,21 @@ function stalledBody(): Readable {
   return body;
 }
 
+/** A JSON body of `levels` nested arrays. */
+const nested = (levels: number) => `${"[".repeat(levels)}${"]".repeat(levels)}`;
+
+/** Send `body` as it is to the upstream through the proxy at `proxyUrl`. */
+async function sendText(proxyUrl: string, body: string) {
+  const agent = new ProxyAgent({ uri: proxyUrl, proxyTunnel: true });
+  agents.push(agent);
+  const response = await request(`${origin}/v1/messages`, {
+    body,
+    dispatcher: agent,
+    method: "POST",
+  });
+  return { status: response.statusCode, body: await response.body.text() };
+}
+
 /** Wait `ms` of real time, also when `setTimeout` is fake. */
 async function realWait(ms: number) {
   const end = performance.now() + ms;
@@ -832,6 +847,34 @@ describe("limits of the service", () => {
     expect((await send(writer.url, { prompt })).status).toBe(200);
   });
 
+  it("fails a request whose JSON body is nested too deep for its key", async () => {
+    // The key walks a JSON body with recursion. Before, a body of a few
+    // thousand levels filled the stack: the client got HTTP 502, and Sentry
+    // got an error.
+    const log = vi.spyOn(process.stderr, "write");
+    onTestFinished(() => log.mockRestore());
+    // A run without the token makes the key in the key worker, and a run
+    // with the token in this process.
+    const reader = await startRun(undefined, "replay");
+    const writer = await startRun("write-token", "auto");
+    for (const run of [reader, writer]) {
+      for (const levels of [65, 100_000]) {
+        const deep = await sendText(run.url, nested(levels));
+        expect(deep.status).toBe(400);
+        expect(deep.body).toMatch(/nested over 64 levels deep/);
+      }
+    }
+    expect(liveRequests).toBe(0);
+    expect(log.mock.calls.map(([text]) => String(text)).join("")).not.toMatch(
+      /Request failed/,
+    );
+    // 64 levels still get a key, and the runs go on.
+    expect((await sendText(reader.url, nested(64))).status).toBe(412);
+    expect((await sendText(writer.url, nested(64))).status).toBe(200);
+    expect((await send(reader.url, { prompt: "hi" })).status).toBe(412);
+    expect((await send(writer.url, { prompt: "hi" })).status).toBe(200);
+  });
+
   it("answers 503 when the bodies of runs without a token fill the budget", async () => {
     // Each request holds its body until its key is done. Before the budget,
     // 30 parallel bodies of 16 MiB took about 1.9 GB.