Fix bounded CLI error JSON serialization

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,44 +95,272 @@ export async function captureStdout<T>(callback: () => Promise<T>): 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 {
-  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(MAX_ERROR_SERIALIZATION_BYTES)
+  const serialized = serializeErrorValue(error, 0, new WeakSet(), budget)
+  return isCliError(serialized) ? serialized : { name: "Error", message: errorMessage(error) }
+}
+
+class ErrorSerializationBudget {
+  bytes = 0
+  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 > this.maximumBytes) {
+      this.exhausted = true
+      return false
+    }
+    this.nodes += 1
+    this.bytes += bytes
+    return true
+  }
+}
+
+function serializeErrorValue(value: unknown, depth: number, seen: WeakSet<object>, budget: ErrorSerializationBudget): unknown {
+  if (!budget.canAdd(typeof value)) {
+    return truncation("output-budget")
+  }
+  if (isBinary(value)) {
+    const binary = { type: binaryType(value), byteLength: binaryByteLength(value), omitted: true }
+    return budget.canAdd(binary) ? binary : truncation("output-budget")
+  }
+  if (typeof value === "string") {
+    return budgetedString(value, budget)
+  }
+  if (value === null || typeof value === "boolean" || typeof value === "number") {
+    return value
+  }
+  if (typeof value === "bigint") {
+    return budgetedString(`${value}n`, budget)
+  }
+  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")
+    // Reserve the identity needed to diagnose this error before traversing extras.
+    const serializedName = budgetedText(typeof name === "string" ? name : "Error", "Error", budget)
+    const serializedMessage = budgetedText(typeof message === "string" ? message : "Unknown error", "Unknown error", budget)
+    const serializedCode = typeof code === "string" ? budgetedText(code, undefined, budget) : undefined
+    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: serializedName,
+      message: serializedMessage,
+      ...(serializedCode === undefined ? {} : { code: serializedCode }),
       ...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<object>, budget: ErrorSerializationBudget, excluded = new Set<string>()): Record<string, unknown> {
+  const output: Record<string, unknown> = {}
+  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
+    }
+    // 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
+    }
+  }
+  if (keys.length > MAX_ERROR_ENTRIES && !output.serialization) {
+    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<string, unknown> {
+  return { omitted: true, reason, ...(omittedEntries === undefined ? {} : { omittedEntries }) }
+}
 
-  return { name: "Error", message: String(error) }
+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<string, unknown> {
+  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 budgetedText(value: string, fallback: string | undefined, budget: ErrorSerializationBudget): string | undefined {
+  const bounded = boundedText(value)
+  if (budget.canAdd(bounded)) return bounded
+  if (fallback && budget.canAdd(fallback)) return fallback
+  return undefined
+}
+
+function isBinary(value: unknown): value is ArrayBuffer | ArrayBufferView {
+  return value instanceof ArrayBuffer || ArrayBuffer.isView(value)
+}
+
+function binaryType(value: ArrayBuffer | ArrayBufferView): string {
+  if (value instanceof ArrayBuffer) return "ArrayBuffer"
+  if (Buffer.isBuffer(value)) return "Buffer"
+  if (value instanceof DataView) return "DataView"
+  for (const constructor of [Int8Array, Uint8Array, Uint8ClampedArray, Int16Array, Uint16Array, Int32Array, Uint32Array, Float32Array, Float64Array, BigInt64Array, BigUint64Array]) {
+    if (value instanceof constructor) return constructor.name
+  }
+  return "ArrayBufferView"
+}
+
+function binaryByteLength(value: ArrayBuffer | ArrayBufferView): number {
+  try {
+    if (value instanceof ArrayBuffer) {
+      const byteLengthGetter = Object.getOwnPropertyDescriptor(ArrayBuffer.prototype, "byteLength")?.get
+      return typeof byteLengthGetter === "function" ? byteLengthGetter.call(value) : 0
+    }
+    if (value instanceof DataView) {
+      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 }
+    const byteLengthGetter = Object.getOwnPropertyDescriptor(typedArrayPrototype, "byteLength")?.get
+    return typeof byteLengthGetter === "function" ? byteLengthGetter.call(value) : 0
+  } catch {
+    return 0
+  }
 }
 
 export function cliFailureEnvelope(command: string | undefined, message: string, details: Record<string, unknown> = {}): Record<string, unknown> {
+  const { error, code, ...diagnosticDetails } = details
   return {
     schema: "wp-codebox/cli-failure/v1",
     success: false,
     status: "error",
-    ...(command ? { command } : {}),
-    error: {
+    ...(command ? { command: boundedText(command) } : {}),
+    error: isCliError(error) ? error : {
       name: "Error",
-      message,
+      message: boundedText(message),
     },
     diagnostics: [
       {
-        code: "cli-error",
-        message,
-        ...details,
+        code: typeof code === "string" ? boundedText(code) : "cli-error",
+        message: boundedText(message),
+        ...serializeDiagnosticDetails(diagnosticDetails),
       },
     ],
   }
 }
 
+function serializeDiagnosticDetails(details: Record<string, unknown>): Record<string, unknown> {
+  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")
 }

tests/error-json-serialization.test.ts

@@ -0,0 +1,99 @@
+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"
+
+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)]))
+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<void>((resolve) => {
+  runCliEntrypoint(["recipe-run", "--json"], async () => { throw root }, ((code) => {
+    exitCode = code
+    resolve()
+    return undefined as never
+  }))
+}))
+
+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))
+
+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")
+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.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")
+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<void>((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 } } }
+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" })
+
+console.log("bounded error JSON serialization ok")