From 43be187b6727a47030dbbba4256eaad89937bded Mon Sep 17 00:00:00 2001 From: wahaha1223 <0668001153@xydigit.com> Date: Thu, 27 Aug 2026 09:15:45 +0800 Subject: [PATCH] fix(qa-lab): bound retained child output without changing verdicts (#108981) Reuse the canonical settlement owner for boundary helpers, reject truncated verification JSON, and preserve complete live output streams. Co-authored-by: Peter Steinberger --- extensions/qa-lab/src/child-output.test.ts | 7 +++ extensions/qa-lab/src/child-output.ts | 8 ++- .../src/gateway-process-boundary.test.ts | 14 ++++++ .../qa-lab/src/gateway-process-boundary.ts | 49 +++++++------------ .../test-file-scenario-command-lifecycle.ts | 41 +++++++++++++--- .../src/test-file-scenario-docker-batch.ts | 3 +- .../test-file-scenario-runner.process.test.ts | 46 ++++++++++++++++- .../qa-lab/src/test-file-scenario-runner.ts | 8 +-- 8 files changed, 129 insertions(+), 47 deletions(-) diff --git a/extensions/qa-lab/src/child-output.test.ts b/extensions/qa-lab/src/child-output.test.ts index 95f7f7a7a0d8..a8fad7851321 100644 --- a/extensions/qa-lab/src/child-output.test.ts +++ b/extensions/qa-lab/src/child-output.test.ts @@ -10,6 +10,13 @@ import { } from "./child-output.js"; describe("qa child output", () => { + it("does not mark an exactly full first stderr chunk as truncated", () => { + const tail = createQaChildOutputTail(4); + appendQaChildOutputTail(tail, Buffer.from("tail")); + expect(tail.truncated).toBe(false); + expect(formatQaChildOutputTail(tail, "stderr")).toBe("tail"); + }); + it("keeps capped stdout UTF-8 safe when the byte cap splits a code point", () => { const text = "ok \u{1f600} done"; const capture = createQaChildOutputCapture(Buffer.byteLength("ok \u{1f600}", "utf8") - 1); diff --git a/extensions/qa-lab/src/child-output.ts b/extensions/qa-lab/src/child-output.ts index c1075da9b664..e9a64b5b5a09 100644 --- a/extensions/qa-lab/src/child-output.ts +++ b/extensions/qa-lab/src/child-output.ts @@ -77,8 +77,8 @@ export function createQaChildOutputTail(maxBytes = QA_CHILD_STDERR_TAIL_BYTES) { export function appendQaChildOutputTail(tail: QaChildOutputTail, chunk: unknown) { const buffer = toBuffer(chunk); if (buffer.byteLength >= tail.maxBytes) { + tail.truncated ||= tail.buffer.byteLength > 0 || buffer.byteLength > tail.maxBytes; tail.buffer = Buffer.from(buffer.subarray(buffer.byteLength - tail.maxBytes)); - tail.truncated = true; return; } const next = Buffer.concat([tail.buffer, buffer], tail.buffer.byteLength + buffer.byteLength); @@ -90,8 +90,12 @@ export function appendQaChildOutputTail(tail: QaChildOutputTail, chunk: unknown) tail.truncated = true; } +export function readQaChildOutputTail(tail: QaChildOutputTail) { + return decodeUtf8Tail(tail.buffer, tail.truncated); +} + export function formatQaChildOutputTail(tail: QaChildOutputTail, label: string) { - const text = decodeUtf8Tail(tail.buffer, tail.truncated).trim(); + const text = readQaChildOutputTail(tail).trim(); if (!text) { return ""; } diff --git a/extensions/qa-lab/src/gateway-process-boundary.test.ts b/extensions/qa-lab/src/gateway-process-boundary.test.ts index 58fab78a9b05..f13b2b49c793 100644 --- a/extensions/qa-lab/src/gateway-process-boundary.test.ts +++ b/extensions/qa-lab/src/gateway-process-boundary.test.ts @@ -3,6 +3,7 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { afterEach, describe, expect, it } from "vitest"; +import { QA_CHILD_STDOUT_MAX_BYTES } from "./child-output.js"; import { assertQaGatewayCredentialLeaseQuarantine, createQaGatewayProcessBoundaryController, @@ -121,6 +122,19 @@ describe("gateway process boundary", () => { expect.objectContaining({ generation: prepared.generation }), ]); + // Whitespace leaves JSON valid, but oversized verification output must not + // authenticate a truncated proof as a complete response. + await fs.appendFile( + `${prepared.identityFilePath}.runtime`, + " ".repeat(QA_CHILD_STDOUT_MAX_BYTES), + ); + await expect(controller.markReady(identity)).rejects.toThrow("proxy stdout exceeded"); + + await fs.writeFile(launcherPath, '#!/bin/sh\ncat "$3.runtime" >&2\nexit 7\n'); + await expect(controller.markReady(identity)).rejects.toThrow( + "proxy exited 7 (stderr truncated)", + ); + const malformed = await controller.prepare({ args: ["gateway", "run"], cwd: tempRoot, diff --git a/extensions/qa-lab/src/gateway-process-boundary.ts b/extensions/qa-lab/src/gateway-process-boundary.ts index cacc0b3a80ee..2108f613d11e 100644 --- a/extensions/qa-lab/src/gateway-process-boundary.ts +++ b/extensions/qa-lab/src/gateway-process-boundary.ts @@ -1,4 +1,4 @@ -import { spawn, type ChildProcess } from "node:child_process"; +import type { ChildProcess } from "node:child_process"; import { createHash, randomUUID } from "node:crypto"; import { constants as fsConstants } from "node:fs"; import fs from "node:fs/promises"; @@ -7,6 +7,8 @@ import { setTimeout as sleep } from "node:timers/promises"; import { isRecord } from "openclaw/plugin-sdk/channel-secret-basic-runtime"; import { isPathInside } from "openclaw/plugin-sdk/file-access-runtime"; import { replaceFileAtomic } from "openclaw/plugin-sdk/security-runtime"; +import { QA_CHILD_STDOUT_MAX_BYTES } from "./child-output.js"; +import { runQaScenarioCommandLifecycle } from "./test-file-scenario-command-lifecycle.js"; const PROCESS_BOUNDARY_VERSION = 1; const PROCESS_BOUNDARY_START_TIMEOUT_MS = 30_000; @@ -300,45 +302,30 @@ async function runBoundaryLauncherCommand(params: { launcherPath: string; timeoutMs: number; }) { - const child = spawn(params.launcherPath, params.args, { + // The proxy gets its own process group; the verified SUT identity and its + // UID-quiescence checks remain owned by the launcher, never this cleanup. + const result = await runQaScenarioCommandLifecycle({ + command: params.launcherPath, + args: [...params.args], + cwd: process.cwd(), + timeoutMs: params.timeoutMs, env: { HOME: process.env.HOME, LANG: process.env.LANG ?? "C.UTF-8", PATH: process.env.PATH, }, - stdio: ["ignore", "pipe", "pipe"], }); - const stdout: Buffer[] = []; - const stderr: Buffer[] = []; - child.stdout.on("data", (chunk) => stdout.push(Buffer.from(chunk))); - child.stderr.on("data", (chunk) => stderr.push(Buffer.from(chunk))); - let timeout: ReturnType | undefined; - const timeoutPromise = new Promise((_resolve, reject) => { - timeout = setTimeout(() => { - child.kill("SIGKILL"); - reject(new Error(`process-boundary ${params.label} proxy timed out`)); - }, params.timeoutMs); - }); - let exitCode: number; - try { - exitCode = await Promise.race([ - new Promise((resolve, reject) => { - child.once("error", reject); - child.once("close", (code) => resolve(code ?? 1)); - }), - timeoutPromise, - ]); - } finally { - if (timeout) { - clearTimeout(timeout); - } - } - if (exitCode !== 0) { + if (result.exitCode !== 0) { throw new Error( - `process-boundary ${params.label} proxy exited ${exitCode}: ${Buffer.concat(stderr).toString("utf8").trim()}`, + `process-boundary ${params.label} proxy exited ${result.exitCode}${result.stderrTruncated ? " (stderr truncated)" : ""}: ${result.failureMessage ?? result.stderr.trim()}`, ); } - return Buffer.concat(stdout).toString("utf8"); + if (result.stdoutTruncated) { + throw new Error( + `process-boundary ${params.label} proxy stdout exceeded ${QA_CHILD_STDOUT_MAX_BYTES} bytes`, + ); + } + return result.stdout; } async function runBoundaryVerification(params: { diff --git a/extensions/qa-lab/src/test-file-scenario-command-lifecycle.ts b/extensions/qa-lab/src/test-file-scenario-command-lifecycle.ts index f7219ba10305..f2c8aa9bc973 100644 --- a/extensions/qa-lab/src/test-file-scenario-command-lifecycle.ts +++ b/extensions/qa-lab/src/test-file-scenario-command-lifecycle.ts @@ -1,5 +1,15 @@ import { spawn } from "node:child_process"; import path from "node:path"; +import { + appendQaChildOutput, + appendQaChildOutputTail, + createQaChildOutputCapture, + createQaChildOutputTail, + QA_CHILD_STDERR_TAIL_BYTES, + QA_CHILD_STDOUT_MAX_BYTES, + readQaChildOutput, + readQaChildOutputTail, +} from "./child-output.js"; import { createQaPosixCommandSettlement } from "./posix-command-settlement.js"; import { runQaWindowsTaskkill } from "./windows-system-tools.js"; @@ -18,6 +28,8 @@ export type QaScenarioCommandResult = { signal?: NodeJS.Signals | null; stdout: string; stderr: string; + stdoutTruncated?: true; + stderrTruncated?: true; }; type QaScenarioCommandTerminalResult = Pick< @@ -41,8 +53,10 @@ export function runQaScenarioCommandLifecycle( env: execution.env, stdio: ["ignore", "pipe", "pipe"], }); - const stdout: Buffer[] = []; - const stderr: Buffer[] = []; + // Logs are diagnostics, not native test verdicts: bound retention without + // failing noisy commands or truncating their live onOutput stream. + const stdout = createQaChildOutputCapture(); + const stderr = createQaChildOutputTail(); const commandLabel = path.basename(execution.command); createQaPosixCommandSettlement({ child, @@ -104,8 +118,10 @@ export function runQaScenarioCommandLifecycle( resolve({ ...result, ...(settlementFailure && result.exitCode === 0 ? { exitCode: 1 } : {}), - stdout: Buffer.concat(stdout).toString("utf8"), - stderr: Buffer.concat(stderr).toString("utf8"), + stdout: readQaChildOutput(stdout), + stderr: readQaChildOutputTail(stderr), + ...(stdout.exceeded ? { stdoutTruncated: true } : {}), + ...(stderr.truncated ? { stderrTruncated: true } : {}), ...(settlementFailure ? result.failureMessage ? { failureMessage: `${result.failureMessage}; settlement: ${settlementFailure}` } @@ -115,12 +131,12 @@ export function runQaScenarioCommandLifecycle( }, onStderrData: (chunk) => { const buffered = Buffer.from(chunk); - stderr.push(buffered); + appendQaChildOutputTail(stderr, buffered); execution.onOutput?.("stderr", buffered); }, onStdoutData: (chunk) => { const buffered = Buffer.from(chunk); - stdout.push(buffered); + appendQaChildOutput(stdout, buffered); execution.onOutput?.("stdout", buffered); }, processGroupId: isWindows ? undefined : child.pid, @@ -129,6 +145,19 @@ export function runQaScenarioCommandLifecycle( }); } +export function formatQaScenarioCommandOutput(result: QaScenarioCommandResult): string { + return [ + result.stdoutTruncated + ? `[stdout truncated to first ${QA_CHILD_STDOUT_MAX_BYTES} bytes]\n` + : "", + result.stdout, + result.stderrTruncated + ? `\n[stderr truncated to last ${QA_CHILD_STDERR_TAIL_BYTES} bytes]\n` + : "", + result.stderr, + ].join(""); +} + export function resetQaScenarioCommandCleanupTimings() { timeoutKillGraceMs = QA_SCENARIO_COMMAND_TIMEOUT_KILL_GRACE_MS; timeoutForceSettleMs = QA_SCENARIO_COMMAND_TIMEOUT_FORCE_SETTLE_MS; diff --git a/extensions/qa-lab/src/test-file-scenario-docker-batch.ts b/extensions/qa-lab/src/test-file-scenario-docker-batch.ts index b6888b604424..dc1f69b674ea 100644 --- a/extensions/qa-lab/src/test-file-scenario-docker-batch.ts +++ b/extensions/qa-lab/src/test-file-scenario-docker-batch.ts @@ -5,6 +5,7 @@ import { z } from "zod"; import type { QaSeedScenarioWithSource } from "./scenario-catalog.js"; import { shellQuote } from "./shell-quote.js"; import { + formatQaScenarioCommandOutput, runQaScenarioCommandLifecycle, type QaScenarioCommandExecution, } from "./test-file-scenario-command-lifecycle.js"; @@ -201,7 +202,7 @@ export async function runDockerE2eBatch(params: { } await fs.writeFile( logPath, - `$ ${shellQuote(process.execPath)} scripts/test-docker-all.mjs\n${commandResult.stdout}${commandResult.stderr}`, + `$ ${shellQuote(process.execPath)} scripts/test-docker-all.mjs\n${formatQaScenarioCommandOutput(commandResult)}`, "utf8", ); diff --git a/extensions/qa-lab/src/test-file-scenario-runner.process.test.ts b/extensions/qa-lab/src/test-file-scenario-runner.process.test.ts index 7f2182eb4aad..fcae0b89319e 100644 --- a/extensions/qa-lab/src/test-file-scenario-runner.process.test.ts +++ b/extensions/qa-lab/src/test-file-scenario-runner.process.test.ts @@ -1,7 +1,11 @@ import fs from "node:fs/promises"; import path from "node:path"; import { afterEach, describe, expect, it } from "vitest"; -import { runQaScenarioCommandLifecycle } from "./test-file-scenario-command-lifecycle.js"; +import { QA_CHILD_STDERR_TAIL_BYTES, QA_CHILD_STDOUT_MAX_BYTES } from "./child-output.js"; +import { + formatQaScenarioCommandOutput, + runQaScenarioCommandLifecycle, +} from "./test-file-scenario-command-lifecycle.js"; import { runQaTestFileScenarios, type QaScenarioCommandExecution, @@ -22,6 +26,46 @@ afterEach(async () => { }); describe("qa test file scenario runner", () => { + it.each([0, 7])( + "bounds retained child logs without changing exit %i or live output", + async (exitCode) => { + const streamed = { stdout: 0, stderr: 0 }; + const result = await runQaScenarioCommandLifecycle({ + command: process.execPath, + args: [ + "-e", + [ + `process.stdout.write('x'.repeat(${QA_CHILD_STDOUT_MAX_BYTES * 2}));`, + `process.stderr.write('🦞'.repeat(${QA_CHILD_STDERR_TAIL_BYTES / 2 + 1}) + '\\nfinal diagnostic\\n');`, + `process.exitCode = ${exitCode};`, + ].join("\n"), + ], + cwd: process.cwd(), + env: process.env, + onOutput: (stream, chunk) => { + streamed[stream] += chunk.byteLength; + }, + timeoutMs: 5_000, + }); + + expect(Buffer.byteLength(result.stdout)).toBe(QA_CHILD_STDOUT_MAX_BYTES); + expect(Buffer.byteLength(result.stderr)).toBeLessThanOrEqual(QA_CHILD_STDERR_TAIL_BYTES); + expect(result.exitCode).toBe(exitCode); + expect(result.failureMessage).toBeUndefined(); + expect(result.stdoutTruncated).toBe(true); + expect(result.stderrTruncated).toBe(true); + const log = formatQaScenarioCommandOutput(result); + expect(log.startsWith("[stdout truncated to first")).toBe(true); + expect(log.includes("[stderr truncated to last")).toBe(true); + expect(result.stderr).toContain("final diagnostic"); + expect(result.stderr).not.toContain("�"); + expect(streamed).toEqual({ + stdout: QA_CHILD_STDOUT_MAX_BYTES * 2, + stderr: QA_CHILD_STDERR_TAIL_BYTES * 2 + 4 + Buffer.byteLength("\nfinal diagnostic\n"), + }); + }, + ); + it("streams real native subprocess output before command settlement", async () => { const observed: Array<{ stream: "stderr" | "stdout"; value: string }> = []; let settled = false; diff --git a/extensions/qa-lab/src/test-file-scenario-runner.ts b/extensions/qa-lab/src/test-file-scenario-runner.ts index 5f864ddce70a..5cb43f4096b7 100644 --- a/extensions/qa-lab/src/test-file-scenario-runner.ts +++ b/extensions/qa-lab/src/test-file-scenario-runner.ts @@ -22,6 +22,7 @@ import type { QaSeedScenarioWithSource } from "./scenario-catalog.js"; import type { QaScorecardEvidenceMode } from "./scorecard-taxonomy.js"; import { shellQuote } from "./shell-quote.js"; import { + formatQaScenarioCommandOutput, runQaScenarioCommandLifecycle, type QaScenarioCommandExecution, type QaScenarioCommandResult, @@ -293,12 +294,7 @@ async function runScenarioCommandSteps(params: { : {}), timeoutMs, }); - if (result.stdout) { - logChunks.push(result.stdout); - } - if (result.stderr) { - logChunks.push(result.stderr); - } + logChunks.push(formatQaScenarioCommandOutput(result)); if (result.failureMessage || result.exitCode !== 0 || result.signal) { failureMessage = result.failureMessage ??