From d0e56c8186b5ad3ee5d6360bfde9d9ab47b1577e Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 5 Nov 2026 18:06:20 +0000 Subject: [PATCH 1/3] fix(key): Answer HTTP 400 for a JSON body nested too deep for its key The key walks a parsed JSON body with recursion (`stableStringify` and the thinking-block walk of `request-key.ts`). A body nested some thousands of levels deep, such as 100,000 `[` and `]`, filled the call stack. The `RangeError` had no status, so the client got HTTP 502 and Sentry got an error event, for a run with or without the token. `keyRequest()` now turns that `RangeError` into an error with HTTP status 400. The key worker passes the status on, so both paths answer 400 and send nothing to Sentry. The run goes on. No key changes. The walk is the same code, so a body that got a key on main gets the same key. A body that filled the stack on main got no key, and it gets none now. The depth where V8 fills the stack depends on JIT tiers, on main too (4,500 to 8,000 levels of `[` in one process). The pinned key tests pass without a change, and a randomized compare of 30,000 bodies against main gives the same keys. Co-Authored-By: David Cramer <23610+dcramer@users.noreply.github.com> --- README.md | 2 ++ packages/service/src/request-key.ts | 28 ++++++++++++++++----- packages/service/tests/service.test.ts | 34 ++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 0e04af5..b9df4fc 100644 --- a/README.md +++ b/README.md @@ -430,6 +430,8 @@ 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 that is nested too deep for the key to read, such as +thousands of levels of `[`. Such a body never got a key. 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 diff --git a/packages/service/src/request-key.ts b/packages/service/src/request-key.ts index 7478ed7..b7b1a48 100644 --- a/packages/service/src/request-key.ts +++ b/packages/service/src/request-key.ts @@ -158,9 +158,10 @@ const sha256 = (text: string) => * 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 too deep for the key. The limits never change a key: a request + * gets its key, or it fails. */ export function keyRequest( rule: RecordingRule, @@ -169,9 +170,24 @@ export function keyRequest( timeLimitMs = keyTimeLimit(Buffer.byteLength(request.body)), maxValues = Infinity, ): KeyedRequest { - return withTimeLimit(timeLimitMs, () => - keyRequestNow(rule, request, known, maxValues), - ); + try { + return withTimeLimit(timeLimitMs, () => + keyRequestNow(rule, request, known, maxValues), + ); + } catch (error) { + // The key walks a JSON body with recursion, so a body that is nested + // very deep, such as 5,000 levels of `[`, fills the call stack. Such a + // body never got a key. It is the fault of the client, so it gets HTTP + // 400 and does not go to Sentry. + const overflow = + error instanceof RangeError && + error.message === "Maximum call stack size exceeded"; + if (!overflow) throw error; + throw Object.assign( + new Error("the JSON body is nested too deep to make its key"), + { status: 400 }, + ); + } } function keyRequestNow( diff --git a/packages/service/tests/service.test.ts b/packages/service/tests/service.test.ts index ca4aa1b..5871d88 100644 --- a/packages/service/tests/service.test.ts +++ b/packages/service/tests/service.test.ts @@ -832,6 +832,40 @@ 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, such a body filled + // the stack: the client got HTTP 502, and Sentry got an error. + const log = vi.spyOn(process.stderr, "write"); + onTestFinished(() => log.mockRestore()); + const body = `${"[".repeat(100_000)}${"]".repeat(100_000)}`; + const sendText = async (proxyUrl: 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() }; + }; + // 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]) { + const deep = await sendText(run.url); + expect(deep.status).toBe(400); + expect(deep.body).toMatch(/nested too deep/); + } + expect(liveRequests).toBe(0); + expect(log.mock.calls.map(([text]) => String(text)).join("")).not.toMatch( + /Request failed/, + ); + // The request fails, but the runs go on. + 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. From a3d62e05f54746102465b841bdb6fe158fd78dd2 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 5 Nov 2026 19:09:48 +0000 Subject: [PATCH 2/3] fix(key): Limit JSON depth instead of catching a stack overflow Catching RangeError also caught a regex stack overflow from a client value pattern, with a wrong message, and could hide a real recursion bug. Check the depth of the parsed body in a loop before the key walks it, and fail over 1,000 levels with HTTP 400. On Node 24 the key overflowed from about 2,500 to 3,000 levels, so bodies between 1,001 and that edge, which got a key before, now fail. Keys of all other bodies do not change. Co-Authored-By: David Cramer <23610+dcramer@users.noreply.github.com> --- README.md | 5 ++- packages/service/src/request-key.ts | 57 +++++++++++++++++--------- packages/service/tests/service.test.ts | 43 +++++++++++-------- 3 files changed, 66 insertions(+), 39 deletions(-) diff --git a/README.md b/README.md index b9df4fc..da496f6 100644 --- a/README.md +++ b/README.md @@ -430,8 +430,9 @@ 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 that is nested too deep for the key to read, such as -thousands of levels of `[`. Such a body never got a key. +So does a JSON body with arrays or objects nested over 1,000 levels deep. +The key reads a body with recursion, and a few thousand levels fill its +call stack. 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 diff --git a/packages/service/src/request-key.ts b/packages/service/src/request-key.ts index b7b1a48..05faa14 100644 --- a/packages/service/src/request-key.ts +++ b/packages/service/src/request-key.ts @@ -152,6 +152,31 @@ 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. Where + * that happens changes from run to run, so the limit is well below it. + */ +const MAX_JSON_DEPTH = 1000; + +/** 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"); /** @@ -160,8 +185,8 @@ const sha256 = (text: string) => * throws an error with HTTP status 400 when it takes longer than * `timeLimitMs`, by default `keyTimeLimit()` of the body, when the * request has more than `maxValues` changing values, or when its JSON body - * is nested too deep for the key. The limits never change a key: a request - * gets its key, or it fails. + * 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, @@ -170,24 +195,9 @@ export function keyRequest( timeLimitMs = keyTimeLimit(Buffer.byteLength(request.body)), maxValues = Infinity, ): KeyedRequest { - try { - return withTimeLimit(timeLimitMs, () => - keyRequestNow(rule, request, known, maxValues), - ); - } catch (error) { - // The key walks a JSON body with recursion, so a body that is nested - // very deep, such as 5,000 levels of `[`, fills the call stack. Such a - // body never got a key. It is the fault of the client, so it gets HTTP - // 400 and does not go to Sentry. - const overflow = - error instanceof RangeError && - error.message === "Maximum call stack size exceeded"; - if (!overflow) throw error; - throw Object.assign( - new Error("the JSON body is nested too deep to make its key"), - { status: 400 }, - ); - } + return withTimeLimit(timeLimitMs, () => + keyRequestNow(rule, request, known, maxValues), + ); } function keyRequestNow( @@ -203,6 +213,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 Object.assign( + new Error(`the JSON body is nested over ${MAX_JSON_DEPTH} levels deep`), + { status: 400 }, + ); + } const { text, values } = json === undefined ? extractValues(request.body, rule.values, known, maxValues) diff --git a/packages/service/tests/service.test.ts b/packages/service/tests/service.test.ts index 5871d88..3cef5f3 100644 --- a/packages/service/tests/service.test.ts +++ b/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; @@ -833,35 +848,29 @@ describe("limits of the service", () => { }); it("fails a request whose JSON body is nested too deep for its key", async () => { - // The key walks a JSON body with recursion. Before, such a body filled - // the stack: the client got HTTP 502, and Sentry got an error. + // 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()); - const body = `${"[".repeat(100_000)}${"]".repeat(100_000)}`; - const sendText = async (proxyUrl: 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() }; - }; // 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]) { - const deep = await sendText(run.url); - expect(deep.status).toBe(400); - expect(deep.body).toMatch(/nested too deep/); + for (const levels of [1001, 100_000]) { + const deep = await sendText(run.url, nested(levels)); + expect(deep.status).toBe(400); + expect(deep.body).toMatch(/nested over 1000 levels deep/); + } } expect(liveRequests).toBe(0); expect(log.mock.calls.map(([text]) => String(text)).join("")).not.toMatch( /Request failed/, ); - // The request fails, but the runs go on. + // 1000 levels still get a key, and the runs go on. + expect((await sendText(reader.url, nested(1000))).status).toBe(412); + expect((await sendText(writer.url, nested(1000))).status).toBe(200); expect((await send(reader.url, { prompt: "hi" })).status).toBe(412); expect((await send(writer.url, { prompt: "hi" })).status).toBe(200); }); From ce669c836d38d46919e2ed8722418fb383f2f16c Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 5 Nov 2026 19:38:11 +0000 Subject: [PATCH 3/3] fix(key): Lower the JSON depth limit to 64 levels Model request bodies nest far less than this. Use the shared httpError. --- README.md | 4 +--- packages/service/src/request-key.ts | 13 +++++++------ packages/service/tests/service.test.ts | 10 +++++----- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index da496f6..a58e1e0 100644 --- a/README.md +++ b/README.md @@ -430,9 +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 1,000 levels deep. -The key reads a body with recursion, and a few thousand levels fill its -call stack. +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 diff --git a/packages/service/src/request-key.ts b/packages/service/src/request-key.ts index 05faa14..62018d1 100644 --- a/packages/service/src/request-key.ts +++ b/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"; @@ -154,10 +155,10 @@ 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. Where - * that happens changes from run to run, so the limit is well below it. + * with recursion, and a few thousand levels fill the call stack. Model + * request bodies nest far less than this. */ -const MAX_JSON_DEPTH = 1000; +const MAX_JSON_DEPTH = 64; /** Whether `json` has arrays or objects nested deeper than `limit`. */ function nestedDeeperThan(json: unknown, limit: number): boolean { @@ -215,9 +216,9 @@ 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 Object.assign( - new Error(`the JSON body is nested over ${MAX_JSON_DEPTH} levels deep`), - { status: 400 }, + throw httpError( + 400, + `the JSON body is nested over ${MAX_JSON_DEPTH} levels deep`, ); } const { text, values } = diff --git a/packages/service/tests/service.test.ts b/packages/service/tests/service.test.ts index 3cef5f3..160d4bd 100644 --- a/packages/service/tests/service.test.ts +++ b/packages/service/tests/service.test.ts @@ -858,19 +858,19 @@ describe("limits of the service", () => { const reader = await startRun(undefined, "replay"); const writer = await startRun("write-token", "auto"); for (const run of [reader, writer]) { - for (const levels of [1001, 100_000]) { + 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 1000 levels deep/); + 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/, ); - // 1000 levels still get a key, and the runs go on. - expect((await sendText(reader.url, nested(1000))).status).toBe(412); - expect((await sendText(writer.url, nested(1000))).status).toBe(200); + // 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); });