diff --git a/README.md b/README.md index 0e04af5..a58e1e0 100644 --- a/README.md +++ b/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 diff --git a/packages/service/src/request-key.ts b/packages/service/src/request-key.ts index 7478ed7..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"; @@ -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) diff --git a/packages/service/tests/service.test.ts b/packages/service/tests/service.test.ts index ca4aa1b..160d4bd 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; @@ -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.