From 67daa50577423881ab26329997ddf4bb4ac3904d Mon Sep 17 00:00:00 2001 From: kang-heewon Date: Sat, 8 Aug 2026 16:38:08 +0900 Subject: [PATCH] fix: reject unsafe GraphQL body limits --- .../safe-graphql-body-limit-configuration.md | 6 + docs/problem-code-registry.json | 44 +++++-- .../en/reference/problem-recovery-cookbook.md | 33 ++++-- .../src/generated/problem-code-registry.ts | 46 ++++++-- packages/transports-graphql/README.md | 6 + packages/transports-graphql/src/index.ts | 1 + .../src/libs/GraphQLServer.ts | 13 ++- .../libs/problems/GraphQLTransportProblems.ts | 16 +++ .../src/tests/GraphQLServer.spec.ts | 107 +++++++++++++++++- public-api-surface.snapshot.json | 6 + scripts/problem-registry.mts | 9 ++ .../static-misuse-empty-catch-allowlist.json | 2 +- 12 files changed, 265 insertions(+), 24 deletions(-) create mode 100644 .changeset/safe-graphql-body-limit-configuration.md diff --git a/.changeset/safe-graphql-body-limit-configuration.md b/.changeset/safe-graphql-body-limit-configuration.md new file mode 100644 index 000000000..d019c108f --- /dev/null +++ b/.changeset/safe-graphql-body-limit-configuration.md @@ -0,0 +1,6 @@ +--- +"@croco/problems-core": patch +"@croco/transports-graphql": patch +--- + +Reject unsafe GraphQL request body limits during server initialization with the stable `transports-graphql/body-limit-invalid-configuration` Problem, while preserving an inclusive byte boundary for buffered and streamed requests. diff --git a/docs/problem-code-registry.json b/docs/problem-code-registry.json index 273899584..b80e24296 100644 --- a/docs/problem-code-registry.json +++ b/docs/problem-code-registry.json @@ -1,6 +1,6 @@ { "version": "croco.problem-code-registry.v1", - "problemCount": 599, + "problemCount": 600, "problems": [ { "code": "ACCESS_DENIED", @@ -15662,6 +15662,36 @@ } ] }, + { + "code": "transports-graphql/body-limit-invalid-configuration", + "category": "InternalServerError", + "status": 500, + "title": "Internal Server Error", + "cookbookPath": "/reference/problem-recovery-cookbook/#transports-graphql-body-limit-invalid-configuration", + "recovery": { + "cause": "The GraphQL server was configured with an invalid request body byte boundary.", + "userAction": "Ask the operator to correct the service configuration before retrying.", + "operatorAction": "Set maxBodySizeBytes to a finite positive safe integer and restart the service.", + "retryability": "not-retryable", + "redactionPolicy": "operator-only", + "telemetry": { + "eventName": "croco.problem.error", + "severity": "error", + "attributes": ["problem.code", "problem.category", "problem.status"] + } + }, + "lifecycle": { + "status": "active" + }, + "sources": [ + { + "file": "packages/transports-graphql/src/libs/problems/GraphQLTransportProblems.ts", + "line": 8, + "column": 5, + "kind": "problem-constructor" + } + ] + }, { "code": "transports-graphql/request-body-aborted", "category": "BadRequest", @@ -15686,7 +15716,7 @@ "sources": [ { "file": "packages/transports-graphql/src/libs/problems/GraphQLTransportProblems.ts", - "line": 47, + "line": 63, "column": 3, "kind": "problem-class" } @@ -15716,7 +15746,7 @@ "sources": [ { "file": "packages/transports-graphql/src/libs/problems/GraphQLTransportProblems.ts", - "line": 33, + "line": 49, "column": 5, "kind": "problem-constructor" } @@ -15746,7 +15776,7 @@ "sources": [ { "file": "packages/transports-graphql/src/libs/problems/GraphQLTransportProblems.ts", - "line": 55, + "line": 71, "column": 3, "kind": "problem-class" } @@ -15776,7 +15806,7 @@ "sources": [ { "file": "packages/transports-graphql/src/libs/problems/GraphQLTransportProblems.ts", - "line": 4, + "line": 20, "column": 3, "kind": "problem-class" } @@ -15806,7 +15836,7 @@ "sources": [ { "file": "packages/transports-graphql/src/libs/problems/GraphQLTransportProblems.ts", - "line": 16, + "line": 32, "column": 3, "kind": "problem-class" } @@ -15836,7 +15866,7 @@ "sources": [ { "file": "packages/transports-graphql/src/libs/problems/GraphQLTransportProblems.ts", - "line": 24, + "line": 40, "column": 3, "kind": "problem-class" } diff --git a/packages/docs/src/content/docs/en/reference/problem-recovery-cookbook.md b/packages/docs/src/content/docs/en/reference/problem-recovery-cookbook.md index 094bab309..af1d1a8d4 100644 --- a/packages/docs/src/content/docs/en/reference/problem-recovery-cookbook.md +++ b/packages/docs/src/content/docs/en/reference/problem-recovery-cookbook.md @@ -7,7 +7,7 @@ description: Generated Croco Problem code registry with recovery and telemetry m > Generated by `pnpm problem-registry:write`. Do not edit this file by hand. -This cookbook documents 599 public Croco Problem codes. The deterministic JSON registry is generated at `docs/problem-code-registry.json`, and generated client union types are emitted at `packages/problems-core/src/generated/problem-code-registry.ts`. +This cookbook documents 600 public Croco Problem codes. The deterministic JSON registry is generated at `docs/problem-code-registry.json`, and generated client union types are emitted at `packages/problems-core/src/generated/problem-code-registry.ts`. ## Index @@ -535,6 +535,7 @@ This cookbook documents 599 public Croco Problem codes. The deterministic JSON r | [`testing/transaction-context-not-active`](#testing-transaction-context-not-active) | InternalServerError | 500 | conditional | operator-only | active | 1 | | [`TOKEN_LIMIT_EXCEEDED`](#token-limit-exceeded) | BadRequest | 400 | not-retryable | public | active | 1 | | [`TOOL_EXECUTION_ERROR`](#tool-execution-error) | InternalServerError | 500 | conditional | operator-only | active | 1 | +| [`transports-graphql/body-limit-invalid-configuration`](#transports-graphql-body-limit-invalid-configuration) | InternalServerError | 500 | not-retryable | operator-only | active | 1 | | [`transports-graphql/request-body-aborted`](#transports-graphql-request-body-aborted) | BadRequest | 400 | not-retryable | public | active | 1 | | [`transports-graphql/request-body-too-large`](#transports-graphql-request-body-too-large) | PayloadTooLarge | 413 | not-retryable | public | active | 1 | | [`transports-graphql/request-handling-failed`](#transports-graphql-request-handling-failed) | InternalServerError | 500 | conditional | operator-only | active | 1 | @@ -10011,6 +10012,24 @@ Sources: - `packages/llm-core/src/libs/problems/LlmServiceProblem.ts:61:5` (problem-constructor) + + +## `transports-graphql/body-limit-invalid-configuration` + +- Category: `InternalServerError` +- HTTP status: `500` Internal Server Error +- Retryability: `not-retryable` +- Redaction policy: `operator-only` +- Lifecycle: `active` +- Cause: The GraphQL server was configured with an invalid request body byte boundary. +- User action: Ask the operator to correct the service configuration before retrying. +- Operator action: Set maxBodySizeBytes to a finite positive safe integer and restart the service. +- Telemetry: `croco.problem.error` (error) with `problem.code`, `problem.category`, `problem.status` + +Sources: + +- `packages/transports-graphql/src/libs/problems/GraphQLTransportProblems.ts:8:5` (problem-constructor) + ## `transports-graphql/request-body-aborted` @@ -10027,7 +10046,7 @@ Sources: Sources: -- `packages/transports-graphql/src/libs/problems/GraphQLTransportProblems.ts:47:3` (problem-class) +- `packages/transports-graphql/src/libs/problems/GraphQLTransportProblems.ts:63:3` (problem-class) @@ -10045,7 +10064,7 @@ Sources: Sources: -- `packages/transports-graphql/src/libs/problems/GraphQLTransportProblems.ts:33:5` (problem-constructor) +- `packages/transports-graphql/src/libs/problems/GraphQLTransportProblems.ts:49:5` (problem-constructor) @@ -10063,7 +10082,7 @@ Sources: Sources: -- `packages/transports-graphql/src/libs/problems/GraphQLTransportProblems.ts:55:3` (problem-class) +- `packages/transports-graphql/src/libs/problems/GraphQLTransportProblems.ts:71:3` (problem-class) @@ -10081,7 +10100,7 @@ Sources: Sources: -- `packages/transports-graphql/src/libs/problems/GraphQLTransportProblems.ts:4:3` (problem-class) +- `packages/transports-graphql/src/libs/problems/GraphQLTransportProblems.ts:20:3` (problem-class) @@ -10099,7 +10118,7 @@ Sources: Sources: -- `packages/transports-graphql/src/libs/problems/GraphQLTransportProblems.ts:16:3` (problem-class) +- `packages/transports-graphql/src/libs/problems/GraphQLTransportProblems.ts:32:3` (problem-class) @@ -10117,7 +10136,7 @@ Sources: Sources: -- `packages/transports-graphql/src/libs/problems/GraphQLTransportProblems.ts:24:3` (problem-class) +- `packages/transports-graphql/src/libs/problems/GraphQLTransportProblems.ts:40:3` (problem-class) diff --git a/packages/problems-core/src/generated/problem-code-registry.ts b/packages/problems-core/src/generated/problem-code-registry.ts index dafe4043c..31f379b53 100644 --- a/packages/problems-core/src/generated/problem-code-registry.ts +++ b/packages/problems-core/src/generated/problem-code-registry.ts @@ -3,7 +3,7 @@ import type { ProblemCodeRegistry } from "../libs/ProblemRegistry"; export const CROCO_PROBLEM_CODE_REGISTRY = { version: "croco.problem-code-registry.v1", - problemCount: 599, + problemCount: 600, problems: [ { code: "ACCESS_DENIED", @@ -16341,6 +16341,38 @@ export const CROCO_PROBLEM_CODE_REGISTRY = { }, ], }, + { + code: "transports-graphql/body-limit-invalid-configuration", + category: "InternalServerError", + status: 500, + title: "Internal Server Error", + cookbookPath: + "/reference/problem-recovery-cookbook/#transports-graphql-body-limit-invalid-configuration", + recovery: { + cause: "The GraphQL server was configured with an invalid request body byte boundary.", + userAction: "Ask the operator to correct the service configuration before retrying.", + operatorAction: + "Set maxBodySizeBytes to a finite positive safe integer and restart the service.", + retryability: "not-retryable", + redactionPolicy: "operator-only", + telemetry: { + eventName: "croco.problem.error", + severity: "error", + attributes: ["problem.code", "problem.category", "problem.status"], + }, + }, + lifecycle: { + status: "active", + }, + sources: [ + { + file: "packages/transports-graphql/src/libs/problems/GraphQLTransportProblems.ts", + line: 8, + column: 5, + kind: "problem-constructor", + }, + ], + }, { code: "transports-graphql/request-body-aborted", category: "BadRequest", @@ -16366,7 +16398,7 @@ export const CROCO_PROBLEM_CODE_REGISTRY = { sources: [ { file: "packages/transports-graphql/src/libs/problems/GraphQLTransportProblems.ts", - line: 47, + line: 63, column: 3, kind: "problem-class", }, @@ -16398,7 +16430,7 @@ export const CROCO_PROBLEM_CODE_REGISTRY = { sources: [ { file: "packages/transports-graphql/src/libs/problems/GraphQLTransportProblems.ts", - line: 33, + line: 49, column: 5, kind: "problem-constructor", }, @@ -16431,7 +16463,7 @@ export const CROCO_PROBLEM_CODE_REGISTRY = { sources: [ { file: "packages/transports-graphql/src/libs/problems/GraphQLTransportProblems.ts", - line: 55, + line: 71, column: 3, kind: "problem-class", }, @@ -16464,7 +16496,7 @@ export const CROCO_PROBLEM_CODE_REGISTRY = { sources: [ { file: "packages/transports-graphql/src/libs/problems/GraphQLTransportProblems.ts", - line: 4, + line: 20, column: 3, kind: "problem-class", }, @@ -16497,7 +16529,7 @@ export const CROCO_PROBLEM_CODE_REGISTRY = { sources: [ { file: "packages/transports-graphql/src/libs/problems/GraphQLTransportProblems.ts", - line: 16, + line: 32, column: 3, kind: "problem-class", }, @@ -16530,7 +16562,7 @@ export const CROCO_PROBLEM_CODE_REGISTRY = { sources: [ { file: "packages/transports-graphql/src/libs/problems/GraphQLTransportProblems.ts", - line: 24, + line: 40, column: 3, kind: "problem-class", }, diff --git a/packages/transports-graphql/README.md b/packages/transports-graphql/README.md index 4725d4ca2..ecad57506 100644 --- a/packages/transports-graphql/README.md +++ b/packages/transports-graphql/README.md @@ -23,6 +23,12 @@ const schema = await new SchemaCompiler().compile({ resolvers: [HealthResolver] const server = new GraphQLServer({ schema }); ``` +`maxBodySizeBytes` defaults to 1 MiB and must be a finite positive safe integer. The +server validates this option during initialization, before opening its listener. Both +declared `Content-Length` values and actual streamed bytes use an inclusive boundary: +a request with exactly the configured number of bytes is accepted, while the next byte +is rejected with `transports-graphql/request-body-too-large`. + Schemas compiled through `SchemaCompiler` execute the `UseGuards`, `Roles`, and `UseInterceptors` declarations recorded by `@croco/protocols-graphql`. Request headers are available as `context.headers`, custom server context is preserved, and diff --git a/packages/transports-graphql/src/index.ts b/packages/transports-graphql/src/index.ts index 8bc7309ae..bc943bc27 100644 --- a/packages/transports-graphql/src/index.ts +++ b/packages/transports-graphql/src/index.ts @@ -6,6 +6,7 @@ /** GraphQL Yoga server runtime. */ export { GraphQLServer } from "./libs/GraphQLServer"; export { + GraphQLBodyLimitConfigurationProblem, GraphQLRequestBodyAbortedProblem, GraphQLRequestBodyTooLargeProblem, GraphQLRequestHandlingFailedProblem, diff --git a/packages/transports-graphql/src/libs/GraphQLServer.ts b/packages/transports-graphql/src/libs/GraphQLServer.ts index a0eea02ac..e6ac1aaa9 100644 --- a/packages/transports-graphql/src/libs/GraphQLServer.ts +++ b/packages/transports-graphql/src/libs/GraphQLServer.ts @@ -11,6 +11,7 @@ import { import { isProblem, problemToGraphQLError } from "@croco/protocols-graphql"; import { createYoga, maskError } from "graphql-yoga"; import { + GraphQLBodyLimitConfigurationProblem, GraphQLRequestBodyAbortedProblem, GraphQLRequestBodyTooLargeProblem, GraphQLRequestHandlingFailedProblem, @@ -53,12 +54,22 @@ export class GraphQLServer { private yogaHandler: YogaHandler | null = null; private server: Server | null = null; private initialized = false; + private maxBodySizeBytes = DEFAULT_MAX_BODY_SIZE_BYTES; constructor(private options: GraphQLServerOptions = {}) {} async initialize(): Promise { if (this.initialized) return; + const maxBodySizeBytes = + this.options.maxBodySizeBytes === undefined + ? DEFAULT_MAX_BODY_SIZE_BYTES + : this.options.maxBodySizeBytes; + if (!Number.isSafeInteger(maxBodySizeBytes) || maxBodySizeBytes <= 0) { + throw new GraphQLBodyLimitConfigurationProblem(); + } + this.maxBodySizeBytes = maxBodySizeBytes; + const { schema, schemaOptions, @@ -205,7 +216,7 @@ export class GraphQLServer { private getBody(req: IncomingMessage): Promise { return new Promise((resolve, reject) => { - const maxBodySizeBytes = this.options.maxBodySizeBytes ?? DEFAULT_MAX_BODY_SIZE_BYTES; + const maxBodySizeBytes = this.maxBodySizeBytes; const contentLength = req.headers["content-length"]; if (typeof contentLength === "string") { diff --git a/packages/transports-graphql/src/libs/problems/GraphQLTransportProblems.ts b/packages/transports-graphql/src/libs/problems/GraphQLTransportProblems.ts index ea1c78979..1ecb4a29c 100644 --- a/packages/transports-graphql/src/libs/problems/GraphQLTransportProblems.ts +++ b/packages/transports-graphql/src/libs/problems/GraphQLTransportProblems.ts @@ -1,5 +1,21 @@ import { Problem, ProblemCategory } from "@croco/problems-core"; +const PROBLEM_TYPE_BASE = "https://croco.dev/problems/transports-graphql"; + +/** The configured request body boundary cannot be enforced safely. */ +export class GraphQLBodyLimitConfigurationProblem extends Problem { + constructor() { + super( + "transports-graphql/body-limit-invalid-configuration", + ProblemCategory.InternalServerError, + "maxBodySizeBytes must be a finite positive safe integer", + { + type: `${PROBLEM_TYPE_BASE}/body-limit-invalid-configuration`, + }, + ); + } +} + export class GraphQLResolversNotConfiguredProblem extends Problem { readonly code = "transports-graphql/resolvers-not-configured"; readonly category = ProblemCategory.InternalServerError; diff --git a/packages/transports-graphql/src/tests/GraphQLServer.spec.ts b/packages/transports-graphql/src/tests/GraphQLServer.spec.ts index b8c239cae..a0f800e66 100644 --- a/packages/transports-graphql/src/tests/GraphQLServer.spec.ts +++ b/packages/transports-graphql/src/tests/GraphQLServer.spec.ts @@ -1,5 +1,5 @@ import "reflect-metadata"; -import { ServerResponse } from "node:http"; +import { request as httpRequest, ServerResponse } from "node:http"; import { connect } from "node:net"; import { Container } from "@croco/framework-context"; import { Logger } from "@croco/framework-logger"; @@ -25,6 +25,7 @@ import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } import { GraphQLServer } from "../libs/GraphQLServer"; import { SchemaCompiler } from "../libs/SchemaCompiler"; import { + GraphQLBodyLimitConfigurationProblem, GraphQLResolversNotConfiguredProblem, GraphQLSchemaNotConfiguredProblem, GraphQLServerNotInitializedProblem, @@ -466,6 +467,77 @@ describe("GraphQLServer integration", () => { ); }); + it.each([ + Number.NaN, + Number.POSITIVE_INFINITY, + Number.NEGATIVE_INFINITY, + -1, + 0, + 0.5, + 2 ** 53, + null as unknown as number, + ])( + "should reject unsafe maxBodySizeBytes configuration %s during initialization", + async (maxBodySizeBytes) => { + const testServer = new GraphQLServer({ + schemaOptions: { + resolvers: [UserResolver], + autoDiscover: false, + }, + maxBodySizeBytes, + }); + + await expect(testServer.initialize()).rejects.toMatchObject({ + code: "transports-graphql/body-limit-invalid-configuration", + category: "InternalServerError", + detail: "maxBodySizeBytes must be a finite positive safe integer", + }); + await expect(testServer.initialize()).rejects.toBeInstanceOf( + GraphQLBodyLimitConfigurationProblem, + ); + }, + ); + + it("should enforce the exact byte boundary for buffered and streamed bodies", async () => { + const body = JSON.stringify({ query: "{ hello } # 😀" }); + const limit = Buffer.byteLength(body); + const testServer = new GraphQLServer({ + schemaOptions: { + resolvers: [UserResolver], + autoDiscover: false, + }, + maxBodySizeBytes: limit, + }); + + await testServer.start(4003); + + try { + const bufferedAtLimit = await fetch("http://localhost:4003/graphql", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body, + }); + const bufferedOverLimit = await fetch("http://localhost:4003/graphql", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: `${body} `, + }); + const multibyteOffset = body.indexOf("😀"); + const streamedAtLimit = await sendChunkedRequest(4003, [ + body.slice(0, multibyteOffset), + body.slice(multibyteOffset), + ]); + const streamedOverLimit = await sendChunkedRequest(4003, [body, " "]); + + expect(bufferedAtLimit.status).toBe(200); + expect(bufferedOverLimit.status).toBe(413); + expect(streamedAtLimit.status).toBe(200); + expect(streamedOverLimit.status).toBe(413); + } finally { + await testServer.stop(); + } + }); + it("should reject oversized request bodies with 413", async () => { const testServer = new GraphQLServer({ schemaOptions: { @@ -1117,3 +1189,36 @@ function sendRawHttpRequest(port: number, request: string): Promise { socket.on("error", reject); }); } + +function sendChunkedRequest( + port: number, + chunks: readonly string[], +): Promise<{ readonly status: number; readonly body: string }> { + return new Promise((resolve, reject) => { + const request = httpRequest( + { + host: "localhost", + port, + path: "/graphql", + method: "POST", + headers: { "content-type": "application/json" }, + }, + (response) => { + const responseChunks: Buffer[] = []; + response.on("data", (chunk: Buffer) => responseChunks.push(chunk)); + response.on("end", () => { + resolve({ + status: response.statusCode ?? 0, + body: Buffer.concat(responseChunks).toString(), + }); + }); + }, + ); + + request.on("error", reject); + for (const chunk of chunks) { + request.write(chunk); + } + request.end(); + }); +} diff --git a/public-api-surface.snapshot.json b/public-api-surface.snapshot.json index 3d8abc266..3958eda9a 100644 --- a/public-api-surface.snapshot.json +++ b/public-api-surface.snapshot.json @@ -27626,6 +27626,12 @@ ], "sourceEntrypoint": "packages/transports-graphql/src/index.ts", "runtimeExports": [ + { + "name": "GraphQLBodyLimitConfigurationProblem", + "exportKind": "named", + "source": "./libs/problems/GraphQLTransportProblems", + "declarationKind": "class" + }, { "name": "GraphQLRequestBodyAbortedProblem", "exportKind": "named", diff --git a/scripts/problem-registry.mts b/scripts/problem-registry.mts index 6643bd848..8492cedd9 100644 --- a/scripts/problem-registry.mts +++ b/scripts/problem-registry.mts @@ -2018,6 +2018,15 @@ const recoveryMetadataByCode = { redactionPolicy: "operator-only", severity: "error", }), + "transports-graphql/body-limit-invalid-configuration": recovery({ + cause: "The GraphQL server was configured with an invalid request body byte boundary.", + userAction: "Ask the operator to correct the service configuration before retrying.", + operatorAction: + "Set maxBodySizeBytes to a finite positive safe integer and restart the service.", + retryability: "not-retryable", + redactionPolicy: "operator-only", + severity: "error", + }), CROCO_HTTP_SECURITY_001: recovery({ cause: "HTTP bootstrap validation found a generated or application app without the required security middleware set.", diff --git a/scripts/static-misuse-empty-catch-allowlist.json b/scripts/static-misuse-empty-catch-allowlist.json index 513145a28..87b366a5e 100644 --- a/scripts/static-misuse-empty-catch-allowlist.json +++ b/scripts/static-misuse-empty-catch-allowlist.json @@ -51,7 +51,7 @@ { "package": "@croco/transports-graphql", "file": "packages/transports-graphql/src/libs/GraphQLServer.ts", - "line": 197, + "line": 208, "excerpt": "} catch {", "reason": "Startup logging is optional and must not prevent the GraphQL server from resolving a successful listen callback.", "owner": "framework-error-handling",