From d740fb1af990d48535cd6d7ecd25d985150d419e Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Wed, 9 Sep 2026 04:42:37 -0400 Subject: [PATCH 01/10] fix(cli): bound serialized error diagnostics --- packages/cli/src/output.ts | 195 +++++++++++++++++++++++-- tests/error-json-serialization.test.ts | 57 ++++++++ 2 files changed, 242 insertions(+), 10 deletions(-) create mode 100644 tests/error-json-serialization.test.ts diff --git a/packages/cli/src/output.ts b/packages/cli/src/output.ts index 2646845c..1bf71ade 100644 --- a/packages/cli/src/output.ts +++ b/packages/cli/src/output.ts @@ -1,5 +1,6 @@ import type { ArtifactBundle, ExecutionResult, RuntimeInfo } from "@automattic/wp-codebox-core" import type { ArtifactBundleVerificationResult } from "@automattic/wp-codebox-core/artifacts" +import { isSensitiveKey, redactString } from "@automattic/wp-codebox-core" import { listCliRecipeCommandDefinitions } from "./runtime-backends.js" interface CliError { @@ -94,22 +95,196 @@ export async function captureStdout(callback: () => Promise): Promise<{ re } } +const MAX_ERROR_DEPTH = 8 +const MAX_ERROR_ENTRIES = 50 +const MAX_ERROR_NODES = 500 +const MAX_ERROR_OUTPUT_BYTES = 192 * 1024 +const MAX_ERROR_STRING_BYTES = 8 * 1024 + export function serializeError(error: unknown): CliError { - if (error instanceof Error) { - const extras = Object.fromEntries( - Object.entries(error).filter(([key]) => !["name", "message", "stack"].includes(key)), - ) - const cause = "cause" in error && error.cause !== undefined ? serializeError(error.cause) : undefined + const budget = new ErrorSerializationBudget() + const serialized = serializeErrorValue(error, 0, new WeakSet(), budget) + return isCliError(serialized) ? serialized : { name: "Error", message: errorMessage(error) } +} + +class ErrorSerializationBudget { + bytes = 0 + nodes = 0 + + canAdd(value: unknown): boolean { + const bytes = Buffer.byteLength(JSON.stringify(value)) + if (this.nodes >= MAX_ERROR_NODES || this.bytes + bytes > MAX_ERROR_OUTPUT_BYTES) { + return false + } + this.nodes += 1 + this.bytes += bytes + return true + } +} + +function serializeErrorValue(value: unknown, depth: number, seen: WeakSet, budget: ErrorSerializationBudget): unknown { + if (!budget.canAdd(typeof value)) { + return truncation("output-budget") + } + if (isBinary(value)) { + return { type: binaryType(value), byteLength: binaryByteLength(value), omitted: true } + } + if (typeof value === "string") { + return budgetedString(value, budget) + } + if (value === null || typeof value === "boolean" || typeof value === "number") { + return value + } + if (typeof value === "bigint") { + return `${value}n` + } + if (typeof value === "undefined") { + return undefined + } + if (typeof value === "function" || typeof value === "symbol") { + return { type: typeof value, omitted: true } + } + if (depth >= MAX_ERROR_DEPTH) { + return truncation("max-depth") + } + + if (seen.has(value)) { + return truncation("circular-reference") + } + seen.add(value) + + if (value instanceof Error) { + const name = safeProperty(value, "name") + const message = safeProperty(value, "message") + const code = safeProperty(value, "code") + const causeValue = safeProperty(value, "cause") + const extras = serializeEntries(value, depth, seen, budget, new Set(["name", "message", "stack", "cause", "code"])) + const cause = causeValue === undefined ? undefined : serializeErrorValue(causeValue, depth + 1, seen, budget) return { - name: error.name, - message: error.message, - ...("code" in error && typeof error.code === "string" ? { code: error.code } : {}), + name: boundedText(typeof name === "string" ? name : "Error"), + message: boundedText(typeof message === "string" ? message : "Unknown error"), + ...(typeof code === "string" ? { code: boundedText(code) } : {}), ...extras, - ...(cause ? { cause } : {}), + ...(cause === undefined ? {} : { cause }), + } + } + + if (Array.isArray(value)) { + const entries: unknown[] = [] + for (let index = 0; index < Math.min(value.length, MAX_ERROR_ENTRIES); index += 1) { + entries.push(serializeErrorValue(safeProperty(value, String(index)), depth + 1, seen, budget)) + } + return value.length > MAX_ERROR_ENTRIES ? [...entries, truncation("max-entries", value.length - MAX_ERROR_ENTRIES)] : entries + } + + return serializeEntries(value, depth, seen, budget) +} + +function serializeEntries(value: object, depth: number, seen: WeakSet, budget: ErrorSerializationBudget, excluded = new Set()): Record { + const output: Record = Object.create(null) as Record + const keys = safeEnumerableKeys(value).filter((key) => !excluded.has(key)) + for (const key of keys.slice(0, MAX_ERROR_ENTRIES)) { + const descriptor = safeDescriptor(value, key) + if (!descriptor) continue + const outputKey = boundedKey(key) + if (!budget.canAdd(outputKey)) { + output.serialization = truncation("output-budget") + return output } + output[outputKey] = isSensitiveKey(key) ? "[redacted]" : "value" in descriptor ? serializeErrorValue(descriptor.value, depth + 1, seen, budget) : truncation("accessor-property") + } + if (keys.length > MAX_ERROR_ENTRIES) { + output.serialization = truncation("max-entries", keys.length - MAX_ERROR_ENTRIES) + } + return output +} + +function safeEnumerableKeys(value: object): string[] { + try { + return Object.keys(value) + } catch { + return [] } +} + +function safeDescriptor(value: object, key: string): PropertyDescriptor | undefined { + try { + return Object.getOwnPropertyDescriptor(value, key) + } catch { + return undefined + } +} + +function safeProperty(value: object, key: string): unknown { + let current: object | null = value + while (current) { + try { + const descriptor = Object.getOwnPropertyDescriptor(current, key) + if (descriptor) return "value" in descriptor ? descriptor.value : undefined + current = Object.getPrototypeOf(current) + } catch { + return undefined + } + } + return undefined +} + +function boundedKey(key: string): string { + if (isSensitiveKey(key)) return "[redacted]" + const redacted = redactString(key) + return Buffer.byteLength(redacted) <= MAX_ERROR_STRING_BYTES ? redacted : `${Buffer.from(redacted).subarray(0, MAX_ERROR_STRING_BYTES).toString("utf8")}...[truncated]` +} + +function truncation(reason: string, omittedEntries?: number): Record { + return { omitted: true, reason, ...(omittedEntries === undefined ? {} : { omittedEntries }) } +} + +function isCliError(value: unknown): value is CliError { + return Boolean(value && typeof value === "object" && typeof (value as CliError).name === "string" && typeof (value as CliError).message === "string") +} + +function errorMessage(value: unknown): string { + if (typeof value === "string") return boundedText(value) + if (value instanceof Error) { + const message = safeProperty(value, "message") + return boundedText(typeof message === "string" ? message : "Unknown error") + } + try { + return boundedText(String(value)) + } catch { + return "Unknown error" + } +} + +function budgetedString(value: string, budget: ErrorSerializationBudget): string | Record { + const bounded = boundedString(value) + return budget.canAdd(bounded) ? bounded : truncation("output-budget") +} + +function boundedString(value: string): string | { value: string; truncated: true; originalByteLength: number } { + const redacted = redactString(value) + const bytes = Buffer.byteLength(redacted) + if (bytes <= MAX_ERROR_STRING_BYTES) { + return redacted + } + return { value: Buffer.from(redacted).subarray(0, MAX_ERROR_STRING_BYTES).toString("utf8"), truncated: true, originalByteLength: bytes } +} + +function boundedText(value: string): string { + const bounded = boundedString(value) + return typeof bounded === "string" ? bounded : `${bounded.value}\n[truncated; originalByteLength=${bounded.originalByteLength}]` +} + +function isBinary(value: unknown): value is ArrayBuffer | ArrayBufferView { + return value instanceof ArrayBuffer || ArrayBuffer.isView(value) +} + +function binaryType(value: ArrayBuffer | ArrayBufferView): string { + return value instanceof ArrayBuffer ? "ArrayBuffer" : value.constructor.name +} - return { name: "Error", message: String(error) } +function binaryByteLength(value: ArrayBuffer | ArrayBufferView): number { + return value.byteLength } export function cliFailureEnvelope(command: string | undefined, message: string, details: Record = {}): Record { diff --git a/tests/error-json-serialization.test.ts b/tests/error-json-serialization.test.ts new file mode 100644 index 00000000..537e6c6e --- /dev/null +++ b/tests/error-json-serialization.test.ts @@ -0,0 +1,57 @@ +import assert from "node:assert/strict" +import { runCliEntrypoint } from "../packages/cli/src/cli-main.js" +import { captureStdout } from "../packages/cli/src/output.js" + +const secret = "ghp_error_serialization_canary_1234567890" +const responseBytes = Buffer.from(secret.repeat(65536)) +const root = Object.assign(new Error("PHPUnit failed after retaining files/test-results.json"), { + name: "PlaygroundCommandError", + code: "wp-codebox-playground-command-failed", + failureClassification: "runtime-command-failure", + response: { + bytes: responseBytes, + uint8: new Uint8Array(responseBytes), + arrayBuffer: responseBytes.buffer.slice(responseBytes.byteOffset, responseBytes.byteOffset + responseBytes.byteLength), + text: "PHPUnit summary: 883 passed, 161 failed, 9 skipped", + artifactRefs: [{ path: "runtime-1/files/test-results.json", kind: "test-results" }], + }, +}) +const nested = Object.assign(new Error("Nested runtime failure"), { response: { bytes: responseBytes } }) +root.cause = nested +nested.cause = root +Object.defineProperty(root, "throwingGetter", { enumerable: true, get: () => { throw new Error("must not run") } }) +Object.defineProperty(root, "token", { enumerable: true, get: () => { throw new Error("sensitive accessors must not run") } }) +const broadPayload = Object.fromEntries(Array.from({ length: 100 }, (_, index) => [`entry-${index}`, "x".repeat(32 * 1024)])) +Object.assign(root, { broadPayload }) + +let exitCode: number | undefined +const { logs } = await captureStdout(async () => await new Promise((resolve) => { + runCliEntrypoint(["recipe-run", "--json"], async () => { throw root }, ((code) => { + exitCode = code + resolve() + return undefined as never + })) +})) + +assert.equal(exitCode, 1) +const stdout = logs.join("\n") +assert.ok(Buffer.byteLength(stdout) < 512 * 1024) +assert.doesNotMatch(stdout, new RegExp(secret)) + +const output = JSON.parse(stdout) as { + error: { message: string, code: string, failureClassification: string, response: { bytes: { type: string, byteLength: number, omitted: boolean }, uint8: { type: string }, arrayBuffer: { type: string }, text: string, artifactRefs: Array<{ path: string }> }, cause: { cause: { reason: string } }, broadPayload: { serialization: { reason: string } }, throwingGetter: { reason: string }, "[redacted]": string } +} +assert.equal(output.error.message, "PHPUnit failed after retaining files/test-results.json") +assert.equal(output.error.code, "wp-codebox-playground-command-failed") +assert.equal(output.error.failureClassification, "runtime-command-failure") +assert.deepEqual(output.error.response.bytes, { type: "Buffer", byteLength: Buffer.byteLength(secret) * 65536, omitted: true }) +assert.equal(output.error.response.uint8.type, "Uint8Array") +assert.equal(output.error.response.arrayBuffer.type, "ArrayBuffer") +assert.equal(output.error.response.text, "PHPUnit summary: 883 passed, 161 failed, 9 skipped") +assert.equal(output.error.response.artifactRefs[0]?.path, "runtime-1/files/test-results.json") +assert.equal(output.error.cause.cause.reason, "circular-reference") +assert.equal(output.error.broadPayload.serialization.reason, "output-budget") +assert.equal(output.error.throwingGetter.reason, "accessor-property") +assert.equal(output.error["[redacted]"], "[redacted]") + +console.log("bounded error JSON serialization ok") From c72473739ced1d4f4f6f4c2d80fd0abde5d3d7e2 Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Wed, 9 Sep 2026 05:08:45 -0400 Subject: [PATCH 02/10] fix(cli): preserve bounded error identity --- packages/cli/src/output.ts | 50 ++++++++++++++++++++------ tests/error-json-serialization.test.ts | 17 ++++++--- 2 files changed, 53 insertions(+), 14 deletions(-) diff --git a/packages/cli/src/output.ts b/packages/cli/src/output.ts index 1bf71ade..f69aee82 100644 --- a/packages/cli/src/output.ts +++ b/packages/cli/src/output.ts @@ -98,7 +98,7 @@ export async function captureStdout(callback: () => Promise): Promise<{ re const MAX_ERROR_DEPTH = 8 const MAX_ERROR_ENTRIES = 50 const MAX_ERROR_NODES = 500 -const MAX_ERROR_OUTPUT_BYTES = 192 * 1024 +export const MAX_ERROR_OUTPUT_BYTES = 192 * 1024 const MAX_ERROR_STRING_BYTES = 8 * 1024 export function serializeError(error: unknown): CliError { @@ -127,7 +127,8 @@ function serializeErrorValue(value: unknown, depth: number, seen: WeakSet = {}): Record { + const { error, ...diagnosticDetails } = details return { schema: "wp-codebox/cli-failure/v1", success: false, status: "error", ...(command ? { command } : {}), - error: { + error: isCliError(error) ? error : { name: "Error", message, }, @@ -301,7 +331,7 @@ export function cliFailureEnvelope(command: string | undefined, message: string, { code: "cli-error", message, - ...details, + ...diagnosticDetails, }, ], } diff --git a/tests/error-json-serialization.test.ts b/tests/error-json-serialization.test.ts index 537e6c6e..3e9a61fc 100644 --- a/tests/error-json-serialization.test.ts +++ b/tests/error-json-serialization.test.ts @@ -1,6 +1,6 @@ import assert from "node:assert/strict" import { runCliEntrypoint } from "../packages/cli/src/cli-main.js" -import { captureStdout } from "../packages/cli/src/output.js" +import { captureStdout, MAX_ERROR_OUTPUT_BYTES } from "../packages/cli/src/output.js" const secret = "ghp_error_serialization_canary_1234567890" const responseBytes = Buffer.from(secret.repeat(65536)) @@ -22,7 +22,12 @@ nested.cause = root Object.defineProperty(root, "throwingGetter", { enumerable: true, get: () => { throw new Error("must not run") } }) Object.defineProperty(root, "token", { enumerable: true, get: () => { throw new Error("sensitive accessors must not run") } }) const broadPayload = Object.fromEntries(Array.from({ length: 100 }, (_, index) => [`entry-${index}`, "x".repeat(32 * 1024)])) -Object.assign(root, { broadPayload }) +const hostileUint8 = new Uint8Array(responseBytes) +Object.defineProperties(hostileUint8, { + constructor: { get: () => { throw new Error("constructor must not run") } }, + byteLength: { get: () => { throw new Error("byteLength must not run") } }, +}) +Object.assign(root, { hostileUint8, hugeInteger: BigInt("9".repeat(65536)), broadPayload }) let exitCode: number | undefined const { logs } = await captureStdout(async () => await new Promise((resolve) => { @@ -35,11 +40,11 @@ const { logs } = await captureStdout(async () => await new Promise((resolv assert.equal(exitCode, 1) const stdout = logs.join("\n") -assert.ok(Buffer.byteLength(stdout) < 512 * 1024) +assert.ok(Buffer.byteLength(stdout) <= MAX_ERROR_OUTPUT_BYTES) assert.doesNotMatch(stdout, new RegExp(secret)) const output = JSON.parse(stdout) as { - error: { message: string, code: string, failureClassification: string, response: { bytes: { type: string, byteLength: number, omitted: boolean }, uint8: { type: string }, arrayBuffer: { type: string }, text: string, artifactRefs: Array<{ path: string }> }, cause: { cause: { reason: string } }, broadPayload: { serialization: { reason: string } }, throwingGetter: { reason: string }, "[redacted]": string } + error: { message: string, code: string, failureClassification: string, response: { bytes: { type: string, byteLength: number, omitted: boolean }, uint8: { type: string }, arrayBuffer: { type: string }, text: string, artifactRefs: Array<{ path: string }> }, cause: { cause: { reason: string } }, broadPayload: { serialization: { reason: string } }, hugeInteger: { value: string, truncated: boolean, originalByteLength: number }, hostileUint8: { type: string, byteLength: number }, throwingGetter: { reason: string }, "[redacted]": string } } assert.equal(output.error.message, "PHPUnit failed after retaining files/test-results.json") assert.equal(output.error.code, "wp-codebox-playground-command-failed") @@ -49,6 +54,10 @@ assert.equal(output.error.response.uint8.type, "Uint8Array") assert.equal(output.error.response.arrayBuffer.type, "ArrayBuffer") assert.equal(output.error.response.text, "PHPUnit summary: 883 passed, 161 failed, 9 skipped") assert.equal(output.error.response.artifactRefs[0]?.path, "runtime-1/files/test-results.json") +assert.equal(output.error.hostileUint8.type, "Uint8Array") +assert.equal(output.error.hostileUint8.byteLength, responseBytes.byteLength) +assert.equal(output.error.hugeInteger.truncated, true) +assert.equal(output.error.hugeInteger.originalByteLength, 65537) assert.equal(output.error.cause.cause.reason, "circular-reference") assert.equal(output.error.broadPayload.serialization.reason, "output-budget") assert.equal(output.error.throwingGetter.reason, "accessor-property") From 1a67814c17d85a9c889626bda6d8e2f55ddd5bb2 Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Wed, 9 Sep 2026 05:09:59 -0400 Subject: [PATCH 03/10] test(cli): retain error JSON evidence --- tests/error-json-serialization.test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/error-json-serialization.test.ts b/tests/error-json-serialization.test.ts index 3e9a61fc..66930d5b 100644 --- a/tests/error-json-serialization.test.ts +++ b/tests/error-json-serialization.test.ts @@ -1,4 +1,6 @@ import assert from "node:assert/strict" +import { mkdir, writeFile } from "node:fs/promises" +import { resolve } from "node:path" import { runCliEntrypoint } from "../packages/cli/src/cli-main.js" import { captureStdout, MAX_ERROR_OUTPUT_BYTES } from "../packages/cli/src/output.js" @@ -46,6 +48,12 @@ assert.doesNotMatch(stdout, new RegExp(secret)) const output = JSON.parse(stdout) as { error: { message: string, code: string, failureClassification: string, response: { bytes: { type: string, byteLength: number, omitted: boolean }, uint8: { type: string }, arrayBuffer: { type: string }, text: string, artifactRefs: Array<{ path: string }> }, cause: { cause: { reason: string } }, broadPayload: { serialization: { reason: string } }, hugeInteger: { value: string, truncated: boolean, originalByteLength: number }, hostileUint8: { type: string, byteLength: number }, throwingGetter: { reason: string }, "[redacted]": string } } +if (process.env.ERROR_JSON_EVIDENCE_DIR) { + const evidenceDirectory = resolve(process.env.ERROR_JSON_EVIDENCE_DIR) + await mkdir(evidenceDirectory, { recursive: true }) + await writeFile(resolve(evidenceDirectory, "cli-failure.json"), `${stdout}\n`) + await writeFile(resolve(evidenceDirectory, "cli-failure-summary.json"), `${JSON.stringify({ byteLength: Buffer.byteLength(stdout), output }, null, 2)}\n`) +} assert.equal(output.error.message, "PHPUnit failed after retaining files/test-results.json") assert.equal(output.error.code, "wp-codebox-playground-command-failed") assert.equal(output.error.failureClassification, "runtime-command-failure") From db3d90b60ecf36d607064386154c5af9de83f25d Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Wed, 9 Sep 2026 05:24:28 -0400 Subject: [PATCH 04/10] fix(cli): retain actual serialization bound --- packages/cli/src/output.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/src/output.ts b/packages/cli/src/output.ts index f69aee82..0e955820 100644 --- a/packages/cli/src/output.ts +++ b/packages/cli/src/output.ts @@ -198,7 +198,7 @@ function serializeEntries(value: object, depth: number, seen: WeakSet, b } output[outputKey] = isSensitiveKey(key) ? "[redacted]" : "value" in descriptor ? serializeErrorValue(descriptor.value, depth + 1, seen, budget) : truncation("accessor-property") } - if (keys.length > MAX_ERROR_ENTRIES) { + if (keys.length > MAX_ERROR_ENTRIES && !output.serialization) { output.serialization = truncation("max-entries", keys.length - MAX_ERROR_ENTRIES) } return output From 45f45d394fc736a4664665ec97034d7a1cbd9a5a Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Wed, 9 Sep 2026 05:27:18 -0400 Subject: [PATCH 05/10] fix(cli): stop at error output bound --- packages/cli/src/output.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/cli/src/output.ts b/packages/cli/src/output.ts index 0e955820..7b30e189 100644 --- a/packages/cli/src/output.ts +++ b/packages/cli/src/output.ts @@ -110,10 +110,12 @@ export function serializeError(error: unknown): CliError { class ErrorSerializationBudget { bytes = 0 nodes = 0 + exhausted = false canAdd(value: unknown): boolean { const bytes = Buffer.byteLength(JSON.stringify(value)) if (this.nodes >= MAX_ERROR_NODES || this.bytes + bytes > MAX_ERROR_OUTPUT_BYTES) { + this.exhausted = true return false } this.nodes += 1 @@ -197,6 +199,10 @@ function serializeEntries(value: object, depth: number, seen: WeakSet, b return output } output[outputKey] = isSensitiveKey(key) ? "[redacted]" : "value" in descriptor ? serializeErrorValue(descriptor.value, depth + 1, seen, budget) : truncation("accessor-property") + if (budget.exhausted) { + output.serialization = truncation("output-budget") + return output + } } if (keys.length > MAX_ERROR_ENTRIES && !output.serialization) { output.serialization = truncation("max-entries", keys.length - MAX_ERROR_ENTRIES) From 6aba0ab094cb961f6f34e2ce8db29abb148eb649 Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Wed, 9 Sep 2026 05:33:55 -0400 Subject: [PATCH 06/10] fix(cli): type intrinsic binary accessors --- packages/cli/src/output.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/output.ts b/packages/cli/src/output.ts index 7b30e189..b3b145e2 100644 --- a/packages/cli/src/output.ts +++ b/packages/cli/src/output.ts @@ -310,13 +310,16 @@ function binaryType(value: ArrayBuffer | ArrayBufferView): string { function binaryByteLength(value: ArrayBuffer | ArrayBufferView): number { try { if (value instanceof ArrayBuffer) { - return ArrayBuffer.prototype.byteLength.call(value) + const byteLengthGetter = Object.getOwnPropertyDescriptor(ArrayBuffer.prototype, "byteLength")?.get + return typeof byteLengthGetter === "function" ? byteLengthGetter.call(value) : 0 } if (value instanceof DataView) { - return DataView.prototype.byteLength.call(value) + const byteLengthGetter = Object.getOwnPropertyDescriptor(DataView.prototype, "byteLength")?.get + return typeof byteLengthGetter === "function" ? byteLengthGetter.call(value) : 0 } const typedArrayPrototype = Object.getPrototypeOf(Uint8Array.prototype) as { byteLength: number } - return Object.getOwnPropertyDescriptor(typedArrayPrototype, "byteLength")?.get?.call(value) ?? 0 + const byteLengthGetter = Object.getOwnPropertyDescriptor(typedArrayPrototype, "byteLength")?.get + return typeof byteLengthGetter === "function" ? byteLengthGetter.call(value) : 0 } catch { return 0 } From b9f8c4cf953bb5f50672afcfef415dd7ee2a9ba6 Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Wed, 9 Sep 2026 07:16:48 -0400 Subject: [PATCH 07/10] fix(cli): preserve error object contracts --- packages/cli/src/output.ts | 10 ++++++++-- tests/error-json-serialization.test.ts | 2 ++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/output.ts b/packages/cli/src/output.ts index b3b145e2..7e3e4fc7 100644 --- a/packages/cli/src/output.ts +++ b/packages/cli/src/output.ts @@ -188,7 +188,7 @@ function serializeErrorValue(value: unknown, depth: number, seen: WeakSet, budget: ErrorSerializationBudget, excluded = new Set()): Record { - const output: Record = Object.create(null) as Record + const output: Record = {} const keys = safeEnumerableKeys(value).filter((key) => !excluded.has(key)) for (const key of keys.slice(0, MAX_ERROR_ENTRIES)) { const descriptor = safeDescriptor(value, key) @@ -198,7 +198,13 @@ function serializeEntries(value: object, depth: number, seen: WeakSet, b output.serialization = truncation("output-budget") return output } - output[outputKey] = isSensitiveKey(key) ? "[redacted]" : "value" in descriptor ? serializeErrorValue(descriptor.value, depth + 1, seen, budget) : truncation("accessor-property") + // Define untrusted keys as data properties so __proto__ cannot change the output prototype. + Object.defineProperty(output, outputKey, { + value: isSensitiveKey(key) ? "[redacted]" : "value" in descriptor ? serializeErrorValue(descriptor.value, depth + 1, seen, budget) : truncation("accessor-property"), + enumerable: true, + configurable: true, + writable: true, + }) if (budget.exhausted) { output.serialization = truncation("output-budget") return output diff --git a/tests/error-json-serialization.test.ts b/tests/error-json-serialization.test.ts index 66930d5b..3f4156cc 100644 --- a/tests/error-json-serialization.test.ts +++ b/tests/error-json-serialization.test.ts @@ -41,6 +41,8 @@ const { logs } = await captureStdout(async () => await new Promise((resolv })) assert.equal(exitCode, 1) +// The test simulates a failing CLI process but must not make this test process fail. +process.exitCode = undefined const stdout = logs.join("\n") assert.ok(Buffer.byteLength(stdout) <= MAX_ERROR_OUTPUT_BYTES) assert.doesNotMatch(stdout, new RegExp(secret)) From a117462f5d3599ef90612a139677c51fd76042df Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Wed, 9 Sep 2026 07:27:43 -0400 Subject: [PATCH 08/10] fix(cli): bound final JSON failures --- packages/cli/src/output.ts | 21 +++++++++++++++------ tests/error-json-serialization.test.ts | 18 ++++++++++++++++++ 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/packages/cli/src/output.ts b/packages/cli/src/output.ts index 7e3e4fc7..5c9bad7c 100644 --- a/packages/cli/src/output.ts +++ b/packages/cli/src/output.ts @@ -98,11 +98,14 @@ export async function captureStdout(callback: () => Promise): Promise<{ re const MAX_ERROR_DEPTH = 8 const MAX_ERROR_ENTRIES = 50 const MAX_ERROR_NODES = 500 +// This is the complete UTF-8, pretty-printed failure response limit. export const MAX_ERROR_OUTPUT_BYTES = 192 * 1024 +const MAX_ERROR_SERIALIZATION_BYTES = 160 * 1024 +const MAX_DIAGNOSTIC_DETAILS_BYTES = 8 * 1024 const MAX_ERROR_STRING_BYTES = 8 * 1024 export function serializeError(error: unknown): CliError { - const budget = new ErrorSerializationBudget() + const budget = new ErrorSerializationBudget(MAX_ERROR_SERIALIZATION_BYTES) const serialized = serializeErrorValue(error, 0, new WeakSet(), budget) return isCliError(serialized) ? serialized : { name: "Error", message: errorMessage(error) } } @@ -112,9 +115,11 @@ class ErrorSerializationBudget { nodes = 0 exhausted = false + constructor(private readonly maximumBytes = MAX_ERROR_SERIALIZATION_BYTES) {} + canAdd(value: unknown): boolean { const bytes = Buffer.byteLength(JSON.stringify(value)) - if (this.nodes >= MAX_ERROR_NODES || this.bytes + bytes > MAX_ERROR_OUTPUT_BYTES) { + if (this.nodes >= MAX_ERROR_NODES || this.bytes + bytes > this.maximumBytes) { this.exhausted = true return false } @@ -337,21 +342,25 @@ export function cliFailureEnvelope(command: string | undefined, message: string, schema: "wp-codebox/cli-failure/v1", success: false, status: "error", - ...(command ? { command } : {}), + ...(command ? { command: boundedText(command) } : {}), error: isCliError(error) ? error : { name: "Error", - message, + message: boundedText(message), }, diagnostics: [ { code: "cli-error", - message, - ...diagnosticDetails, + message: boundedText(message), + ...serializeDiagnosticDetails(diagnosticDetails), }, ], } } +function serializeDiagnosticDetails(details: Record): Record { + return serializeEntries(details, 0, new WeakSet(), new ErrorSerializationBudget(MAX_DIAGNOSTIC_DETAILS_BYTES)) +} + export function wantsJsonOutput(args: readonly string[]): boolean { return args.includes("--json") || args.includes("--format=json") || args.some((arg, index) => arg === "--format" && args[index + 1] === "json") } diff --git a/tests/error-json-serialization.test.ts b/tests/error-json-serialization.test.ts index 3f4156cc..fabffdcd 100644 --- a/tests/error-json-serialization.test.ts +++ b/tests/error-json-serialization.test.ts @@ -73,4 +73,22 @@ assert.equal(output.error.broadPayload.serialization.reason, "output-budget") assert.equal(output.error.throwingGetter.reason, "accessor-property") assert.equal(output.error["[redacted]"], "[redacted]") +const longRoot = Object.assign(new Error("x".repeat(8192)), Object.fromEntries(Array.from({ length: 50 }, (_, index) => [`field${index}`, "y".repeat(8192)]))) +let longExitCode: number | undefined +const { logs: longLogs } = await captureStdout(async () => await new Promise((resolve) => { + runCliEntrypoint(["recipe-run", "--json"], async () => { throw longRoot }, ((code) => { + longExitCode = code + resolve() + return undefined as never + })) +})) + +assert.equal(longExitCode, 1) +process.exitCode = undefined +const longStdout = longLogs.join("\n") +assert.ok(Buffer.byteLength(longStdout) <= MAX_ERROR_OUTPUT_BYTES) +const longOutput = JSON.parse(longStdout) as { error: { message: string, serialization: { omitted: boolean, reason: string } } } +assert.equal(longOutput.error.message, longRoot.message) +assert.deepEqual(longOutput.error.serialization, { omitted: true, reason: "output-budget" }) + console.log("bounded error JSON serialization ok") From ed761bdcb90e76df320df97ec2c1bf67c4a5cb99 Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Wed, 9 Sep 2026 07:29:39 -0400 Subject: [PATCH 09/10] test(cli): retain final envelope evidence --- tests/error-json-serialization.test.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/error-json-serialization.test.ts b/tests/error-json-serialization.test.ts index fabffdcd..0a75fe2a 100644 --- a/tests/error-json-serialization.test.ts +++ b/tests/error-json-serialization.test.ts @@ -88,6 +88,11 @@ process.exitCode = undefined const longStdout = longLogs.join("\n") assert.ok(Buffer.byteLength(longStdout) <= MAX_ERROR_OUTPUT_BYTES) const longOutput = JSON.parse(longStdout) as { error: { message: string, serialization: { omitted: boolean, reason: string } } } +if (process.env.ERROR_JSON_EVIDENCE_DIR) { + const evidenceDirectory = resolve(process.env.ERROR_JSON_EVIDENCE_DIR) + await writeFile(resolve(evidenceDirectory, "cli-long-failure.json"), `${longStdout}\n`) + await writeFile(resolve(evidenceDirectory, "cli-long-failure-summary.json"), `${JSON.stringify({ byteLength: Buffer.byteLength(longStdout), output: longOutput }, null, 2)}\n`) +} assert.equal(longOutput.error.message, longRoot.message) assert.deepEqual(longOutput.error.serialization, { omitted: true, reason: "output-budget" }) From 90fd6de14438c7d1df1862d7931835a1c1aa77da Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Wed, 9 Sep 2026 07:32:51 -0400 Subject: [PATCH 10/10] fix(cli): retain diagnostic error codes --- packages/cli/src/output.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/output.ts b/packages/cli/src/output.ts index 5c9bad7c..374988e8 100644 --- a/packages/cli/src/output.ts +++ b/packages/cli/src/output.ts @@ -337,7 +337,7 @@ function binaryByteLength(value: ArrayBuffer | ArrayBufferView): number { } export function cliFailureEnvelope(command: string | undefined, message: string, details: Record = {}): Record { - const { error, ...diagnosticDetails } = details + const { error, code, ...diagnosticDetails } = details return { schema: "wp-codebox/cli-failure/v1", success: false, @@ -349,7 +349,7 @@ export function cliFailureEnvelope(command: string | undefined, message: string, }, diagnostics: [ { - code: "cli-error", + code: typeof code === "string" ? boundedText(code) : "cli-error", message: boundedText(message), ...serializeDiagnosticDetails(diagnosticDetails), },