From cd80b4efca0a60d14838571aefaf00d72f7317fc Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Thu, 28 May 2026 14:36:47 -0400 Subject: [PATCH] fix: parse qa cli integers strictly --- extensions/qa-lab/src/cli.runtime.test.ts | 11 ++++ extensions/qa-lab/src/cli.runtime.ts | 4 +- extensions/qa-lab/src/cli.test.ts | 27 ++++++++++ extensions/qa-lab/src/cli.ts | 65 ++++++++++++++++++----- 4 files changed, 91 insertions(+), 16 deletions(-) diff --git a/extensions/qa-lab/src/cli.runtime.test.ts b/extensions/qa-lab/src/cli.runtime.test.ts index 9bf69827d69..1e64910a948 100644 --- a/extensions/qa-lab/src/cli.runtime.test.ts +++ b/extensions/qa-lab/src/cli.runtime.test.ts @@ -456,6 +456,17 @@ describe("qa cli runtime", () => { }); }); + it("rejects fractional suite concurrency from programmatic callers", async () => { + await expect( + runQaSuiteCommand({ + repoRoot: "/tmp/openclaw-repo", + scenarioIds: ["channel-chat-baseline"], + concurrency: 1.5, + }), + ).rejects.toThrow("--concurrency must be a positive integer"); + expect(runQaSuiteFromRuntime).not.toHaveBeenCalled(); + }); + it("sets a failing exit code when host suite scenarios fail", async () => { const priorExitCode = process.exitCode; process.exitCode = undefined; diff --git a/extensions/qa-lab/src/cli.runtime.ts b/extensions/qa-lab/src/cli.runtime.ts index 9dc9925eb42..4cd64fa2915 100644 --- a/extensions/qa-lab/src/cli.runtime.ts +++ b/extensions/qa-lab/src/cli.runtime.ts @@ -158,10 +158,10 @@ function parseQaPositiveIntegerOption(label: string, value: number | undefined) if (value === undefined) { return undefined; } - if (!Number.isFinite(value) || value < 1) { + if (!Number.isSafeInteger(value) || value < 1) { throw new Error(`${label} must be a positive integer`); } - return Math.floor(value); + return value; } function normalizeQaOptionalModelRef(input: string | undefined) { diff --git a/extensions/qa-lab/src/cli.test.ts b/extensions/qa-lab/src/cli.test.ts index 4c764cded8f..e66789fd71d 100644 --- a/extensions/qa-lab/src/cli.test.ts +++ b/extensions/qa-lab/src/cli.test.ts @@ -567,6 +567,33 @@ describe("qa cli registration", () => { }); }); + it.each([ + [["qa", "suite", "--concurrency", "1.5"], "--concurrency must be a positive integer."], + [["qa", "suite", "--cpus", "0x4"], "--cpus must be a positive integer."], + [ + ["qa", "manual", "--message", "hi", "--timeout-ms", "1e3"], + "--timeout-ms must be a positive integer.", + ], + [["qa", "credentials", "list", "--limit", "0x10"], "--limit must be a positive integer."], + [["qa", "ui", "--port", "1e4"], "--port must be a positive integer."], + [ + ["qa", "docker-scaffold", "--output-dir", "/tmp/qa", "--gateway-port", "1.5"], + "--gateway-port must be a positive integer.", + ], + [["qa", "up", "--qa-lab-port", "0x43124"], "--qa-lab-port must be a positive integer."], + [["qa", "aimock", "--port", "1e4"], "--port must be a positive integer."], + ])("rejects non-decimal QA numeric option %j", async (args, message) => { + const invalidProgram = new Command(); + invalidProgram.exitOverride(); + invalidProgram.configureOutput({ + writeErr: () => {}, + writeOut: () => {}, + }); + registerQaLabCli(invalidProgram); + + await expect(invalidProgram.parseAsync(["node", "openclaw", ...args])).rejects.toThrow(message); + }); + it("shows an enable hint when a discovered runner plugin is installed but blocked", async () => { listQaRunnerCliContributions.mockReset().mockReturnValue([createBlockedQaRunnerContribution()]); const blockedProgram = new Command(); diff --git a/extensions/qa-lab/src/cli.ts b/extensions/qa-lab/src/cli.ts index e387360e306..6ba2cd65489 100644 --- a/extensions/qa-lab/src/cli.ts +++ b/extensions/qa-lab/src/cli.ts @@ -23,6 +23,25 @@ async function loadQaLabCliRuntime(): Promise { return await qaLabCliRuntimePromise; } +const DECIMAL_INTEGER_RE = /^\d+$/; + +function invalidQaCliArgument(message: string): Error & { code: string; exitCode: number } { + const error = new Error(message) as Error & { code: string; exitCode: number }; + error.name = "InvalidArgumentError"; + error.code = "commander.invalidArgument"; + error.exitCode = 1; + return error; +} + +function parseQaCliPositiveIntegerOption(value: string, flag: string): number { + const trimmed = value.trim(); + const parsed = DECIMAL_INTEGER_RE.test(trimmed) ? Number(trimmed) : Number.NaN; + if (!Number.isSafeInteger(parsed) || parsed < 1) { + throw invalidQaCliArgument(`${flag} must be a positive integer.`); + } + return parsed; +} + async function runQaSelfCheck(opts: { repoRoot?: string; output?: string }) { const runtime = await loadQaLabCliRuntime(); await runtime.runQaLabSelfCheckCommand(opts); @@ -302,7 +321,7 @@ export function registerQaLabCli(program: Command) { [], ) .option("--concurrency ", "Scenario worker concurrency", (value: string) => - Number(value), + parseQaCliPositiveIntegerOption(value, "--concurrency"), ) .option("--preflight", "Run a single-scenario bootstrap preflight and stop", false) .option( @@ -316,7 +335,9 @@ export function registerQaLabCli(program: Command) { "Suite thinking default: off|minimal|low|medium|high|xhigh|adaptive|max", ) .option("--image ", "Multipass image alias") - .option("--cpus ", "Multipass vCPU count", (value: string) => Number(value)) + .option("--cpus ", "Multipass vCPU count", (value: string) => + parseQaCliPositiveIntegerOption(value, "--cpus"), + ) .option("--memory ", "Multipass memory size") .option("--disk ", "Multipass disk size") .option("--runtime-pair ", "Run each scenario under both runtimes, e.g. openclaw,codex") @@ -534,17 +555,17 @@ export function registerQaLabCli(program: Command) { [], ) .option("--judge-timeout-ms ", "Override judge wait timeout", (value: string) => - Number(value), + parseQaCliPositiveIntegerOption(value, "--judge-timeout-ms"), ) .option( "--blind-judge-models", "Hide candidate model refs from judge prompts; reports still map rankings back to real refs", ) .option("--concurrency ", "Candidate model run concurrency", (value: string) => - Number(value), + parseQaCliPositiveIntegerOption(value, "--concurrency"), ) .option("--judge-concurrency ", "Judge model run concurrency", (value: string) => - Number(value), + parseQaCliPositiveIntegerOption(value, "--judge-concurrency"), ) .action( async (opts: { @@ -574,7 +595,9 @@ export function registerQaLabCli(program: Command) { .option("--model ", "Primary provider/model ref (defaults by provider mode)") .option("--alt-model ", "Alternate provider/model ref") .option("--fast", "Enable provider fast mode where supported", false) - .option("--timeout-ms ", "Override agent.wait timeout", (value: string) => Number(value)) + .option("--timeout-ms ", "Override agent.wait timeout", (value: string) => + parseQaCliPositiveIntegerOption(value, "--timeout-ms"), + ) .action( async (opts: { message: string; @@ -672,7 +695,9 @@ export function registerQaLabCli(program: Command) { .description("List credential rows in the shared Convex pool") .option("--kind ", "Filter by credential kind") .option("--status ", 'Filter by row status: "active", "disabled", or "all"', "all") - .option("--limit ", "Max rows to return", (value: string) => Number(value)) + .option("--limit ", "Max rows to return", (value: string) => + parseQaCliPositiveIntegerOption(value, "--limit"), + ) .option("--show-secrets", "Include credential payload JSON in output", false) .option("--site-url ", "Override OPENCLAW_QA_CONVEX_SITE_URL") .option("--endpoint-prefix ", "Override OPENCLAW_QA_CONVEX_ENDPOINT_PREFIX") @@ -697,10 +722,12 @@ export function registerQaLabCli(program: Command) { .description("Start the private QA debugger UI and local QA bus") .option("--repo-root ", "Repository root to target when running from a neutral cwd") .option("--host ", "Bind host", "127.0.0.1") - .option("--port ", "Bind port", (value: string) => Number(value)) + .option("--port ", "Bind port", (value: string) => + parseQaCliPositiveIntegerOption(value, "--port"), + ) .option("--advertise-host ", "Optional public host to advertise in bootstrap payloads") .option("--advertise-port ", "Optional public port to advertise", (value: string) => - Number(value), + parseQaCliPositiveIntegerOption(value, "--advertise-port"), ) .option("--control-ui-url ", "Optional Control UI URL to embed beside the QA panel") .option( @@ -737,8 +764,12 @@ export function registerQaLabCli(program: Command) { .description("Write a prebaked Docker scaffold for the QA dashboard + gateway lane") .option("--repo-root ", "Repository root to target when running from a neutral cwd") .requiredOption("--output-dir ", "Output directory for docker-compose + state files") - .option("--gateway-port ", "Gateway host port", (value: string) => Number(value)) - .option("--qa-lab-port ", "QA lab host port", (value: string) => Number(value)) + .option("--gateway-port ", "Gateway host port", (value: string) => + parseQaCliPositiveIntegerOption(value, "--gateway-port"), + ) + .option("--qa-lab-port ", "QA lab host port", (value: string) => + parseQaCliPositiveIntegerOption(value, "--qa-lab-port"), + ) .option("--provider-base-url ", "Provider base URL for the QA gateway") .option("--image ", "Prebaked image name", "openclaw:qa-local-prebaked") .option("--use-prebuilt-image", "Use image: instead of build: in docker-compose", false) @@ -774,8 +805,12 @@ export function registerQaLabCli(program: Command) { .description("Build the QA site, start the Docker-backed QA stack, and print the QA Lab URL") .option("--repo-root ", "Repository root to target when running from a neutral cwd") .option("--output-dir ", "Output directory for docker-compose + state files") - .option("--gateway-port ", "Gateway host port", (value: string) => Number(value)) - .option("--qa-lab-port ", "QA lab host port", (value: string) => Number(value)) + .option("--gateway-port ", "Gateway host port", (value: string) => + parseQaCliPositiveIntegerOption(value, "--gateway-port"), + ) + .option("--qa-lab-port ", "QA lab host port", (value: string) => + parseQaCliPositiveIntegerOption(value, "--qa-lab-port"), + ) .option("--provider-base-url ", "Provider base URL for the QA gateway") .option("--image ", "Image tag", "openclaw:qa-local-prebaked") .option("--use-prebuilt-image", "Use image: instead of build: in docker-compose", false) @@ -805,7 +840,9 @@ export function registerQaLabCli(program: Command) { qa.command(providerCommand.name) .description(providerCommand.description) .option("--host ", "Bind host", "127.0.0.1") - .option("--port ", "Bind port", (value: string) => Number(value)) + .option("--port ", "Bind port", (value: string) => + parseQaCliPositiveIntegerOption(value, "--port"), + ) .action(async (opts: { host?: string; port?: number }) => { await runQaProviderServer(providerCommand.providerMode, opts); });