fix: resolve VariaQ CLI validation and security documentation findings

CHANGELOG.md

@@ -15,9 +15,12 @@ Unreleased compatibility update.
 - Validate canonical nested campaign solver parameters, including numeric candidate matrices, using non-recursive tool schemas.
 - Keep GPU integration explicitly opt-in with VARIAQ_TEST_GPU=1, independently of CUDA-Q CPU tests.
 - Run hosted legacy and candidate integration against immutable VariaQ commit pins.
-- Document the six development-toolchain audit findings separately from runtime dependencies.
+- Document the development-toolchain audit findings separately from runtime dependencies.
 
 - Fix duplicated plot-format requests in `variaq_report_campaign` when both `formats: ["plots"]` and `plots: true` are supplied; `formats` is now canonical and `plots: true` only ensures "plots" appears once.
+- Convert invalid `--compare` values on `bb variaq report-campaign`, `analyze-runs`, and `analyze-campaign` from an uncaught `ZodError` into structured `PluginError` CLI errors with valid-value hints, preserving valid-comparison behavior and output contracts.
+- Correct the `SECURITY.md` dependency review to the current audit state: the `source-map-js` row is remediated by the 1.2.2 override, five development-only packages remain (three high, two moderate), and advisory lists per package are now complete with production dependencies still clean.
+- Extend regression coverage: operation-level schema violations (e.g. exceeding the group-by limit) surface as structured CLI errors, and the fake VariaQ harness echoes the received `--compare` value into report envelopes/artifacts so tests prove the comparison selector is actually forwarded to the subprocess.
 - Update GitHub Actions to Node 24-native action versions (`actions/checkout@v6`, `actions/setup-node@v7`, `actions/setup-python@v7`) to resolve Node 20 deprecation warnings while preserving existing workflow behavior.
 - Apply a compatible `overrides` remediation for npm audit findings in the `bb-app` development dependency chain; runtime dependencies remain clean.
 

SECURITY.md

@@ -39,26 +39,31 @@ code is introduced by candidate compatibility.
 
 ## Candidate dependency review
 
-The candidate lockfile audit reports six affected development packages (four
+The candidate lockfile audit reports five affected development packages (three
 high, two moderate); `npm audit --omit=dev` reports zero affected production
-packages. These are real upstream advisories, not a claim that development use
-is harmless. The plugin runtime imports Zod and Node built-ins, invokes local
-Python with argument arrays, and does not call the affected libraries directly.
+packages. The previously reported `source-map-js` 1.2.1 exposure
+(GHSA-68fv-2mgg-jv7q) is remediated by the lockfile override to 1.2.2 and no
+longer appears in the audit. These are real upstream advisories, not a claim
+that development use is harmless. The plugin runtime imports Zod and Node
+built-ins, invokes local Python with argument arrays, and does not call the
+affected libraries directly. All five packages are development-only transitive
+dependencies of the npm tooling bundled inside the `bb-app` development
+dependency; none is reachable from the shipped plugin runtime.
 
 | Locked package | Development dependency path | Advisory exposure |
 | --- | --- | --- |
-| brace-expansion 5.0.9 | bb-app → npm → minimatch | [Stack/CPU exhaustion from hostile brace patterns](https://github.com/advisories/GHSA-qhr7-859c-m2p7) |
-| http-cache-semantics 4.2.0 | bb-app → npm → make-fetch-happen | [Cross-user cached response disclosure with max-stale](https://github.com/advisories/GHSA-ch52-4w7c-c8xp) |
-| ip-address 10.5.0 | bb-app → npm → proxy/SOCKS tooling | [Incorrect address classifiers/subnet checks](https://github.com/advisories/GHSA-j6r3-76f7-8jcv), plus unbounded diagnostic input |
-| postcss-selector-parser 7.1.4 | bb-app → npm → dependency query tooling | [CPU exhaustion from hostile selectors](https://github.com/advisories/GHSA-rj75-hqrm-r3gf) |
-| source-map-js 1.2.1 | vitest → vite → postcss | [Event-loop exhaustion from hostile indexed source maps](https://github.com/advisories/GHSA-68fv-2mgg-jv7q) |
-| undici 6.28.0 | bb-app → npm → download/build tooling | [WebSocket denial of service](https://github.com/advisories/GHSA-rfgv-xxqx-mfg5), plus retry-interceptor response splitting |
+| brace-expansion 5.0.9 | bb-app → npm → minimatch | [Quadratic-time `{a},b}` expansion CPU exhaustion](https://github.com/advisories/GHSA-q2hr-2g5m-vwhr) (moderate); [nested brace group recursion stack exhaustion](https://github.com/advisories/GHSA-qhr7-859c-m2p7) and [parseCommaParts recursion stack exhaustion](https://github.com/advisories/GHSA-6j4f-fj2g-mc7p) (high) |
+| http-cache-semantics 4.2.0 | bb-app → npm → make-fetch-happen | [Cross-user cached response disclosure with max-stale](https://github.com/advisories/GHSA-ch52-4w7c-c8xp) (high) |
+| ip-address 10.5.0 | bb-app → npm → proxy/SOCKS tooling | [isLinkLocal recognizes fe80::/64 rather than fe80::/10, allowing SSRF](https://github.com/advisories/GHSA-rpw4-54j3-4h4q); [no NAT64 64:ff9b:1::/48 classifier, allowing SSRF](https://github.com/advisories/GHSA-2vr4-cq9g-pvrc); [cross-family isInSubnet comparison can admit out-of-range addresses](https://github.com/advisories/GHSA-j6r3-76f7-8jcv); [unbounded parse diagnostic can stall the process](https://github.com/advisories/GHSA-h3mg-xc3c-68pw) (all moderate) |
+| postcss-selector-parser 7.1.4 | bb-app → npm → dependency query tooling | [Quadratic flat selector parsing CPU exhaustion](https://github.com/advisories/GHSA-rj75-hqrm-r3gf) (moderate) |
+| undici 6.28.0 | bb-app → npm → node-gyp build tooling | [DoS via unrequested WebSocket subprotocol](https://github.com/advisories/GHSA-rfgv-xxqx-mfg5) (high); [DoS via unhandled error in WebSocket permessage-deflate decompression](https://github.com/advisories/GHSA-3wwx-pv8p-q78v) (moderate); [downstream response splitting via retry interceptor](https://github.com/advisories/GHSA-r53p-7pc4-xj5r) (low) |
 
-The reviewed plugin surfaces expose no user-controlled glob, HTTP cache/proxy,
-CSS/source-map parsing or WebSocket path into these packages. Exploitability in
-npm/build/test tooling depends on its inputs and configuration; hostile package
-metadata, network peers and development assets remain relevant risks. This review
-does not audit BB host internals or establish that these advisories are unreachable
-in every development invocation. Use isolated CI/cache environments and review
-narrow compatible dependency fixes before a broad public release. No forced audit
-fix, broad upgrade or suppression was applied.
+The reviewed plugin surfaces expose no user-controlled glob expansion,
+HTTP cache/proxy, CSS selector, or WebSocket path into these packages.
+Exploitability in npm/build/test tooling depends on its inputs and
+configuration; hostile package metadata, network peers and development assets
+remain relevant risks. This review does not audit BB host internals or
+establish that these advisories are unreachable in every development
+invocation. Use isolated CI/cache environments and review narrow compatible
+dependency fixes before a broad public release. No forced audit fix, broad
+upgrade or suppression was applied.

server.ts

@@ -549,14 +549,20 @@ export default async function plugin(bb: BbPluginApi) {
 
   async function opAnalyzeRuns(query: import("./lib/schema.js").AnalyzeQuery) {
     const c = await cfg();
-    const validated = analyzeQuerySchema.parse(query);
-    if (validated.compare !== undefined) {
+    const validated = analyzeQuerySchema.safeParse(query);
+    if (!validated.success) {
+      throw new PluginError(
+        `Invalid analyze-runs input: ${minimizeZodIssue(validated.error)}`,
+        "Run `bb variaq analyze-runs --help` for usage.",
+      );
+    }
+    if (validated.data.compare !== undefined) {
       throw new PluginError(
         "VariaQ analyze runs does not accept comparison selectors.",
         "Use variaq_analyze_campaign or bb variaq analyze-campaign for comparisons, including paired_solvers on VariaQ 0.8.x.",
       );
     }
-    const argv = buildAnalyzeArgv("runs", validated);
+    const argv = buildAnalyzeArgv("runs", validated.data);
     const result = await runVariaqJson(c, argv, { timeoutMs: 60_000 });
     const envelope = requireSuccess(result, ["analyze", "runs"]);
     const { data, warnings } = successData<AnalyzeEnvelopeData>(envelope, "analyze runs");
@@ -565,7 +571,14 @@ export default async function plugin(bb: BbPluginApi) {
 
   async function opAnalyzeCampaign(campaignId: string, query: Omit<import("./lib/schema.js").AnalyzeQuery, "campaign_id">) {
     const c = await cfg();
-    const fullQuery = analyzeQuerySchema.parse({ ...query, campaign_id: campaignId });
+    const validated = analyzeQuerySchema.safeParse({ ...query, campaign_id: campaignId });
+    if (!validated.success) {
+      throw new PluginError(
+        `Invalid analyze-campaign input: ${minimizeZodIssue(validated.error)}`,
+        "Run `bb variaq analyze-campaign --help` for usage.",
+      );
+    }
+    const fullQuery = validated.data;
     await requireComparisonSupport(c, fullQuery.compare);
     const argv = buildAnalyzeArgv("campaign", fullQuery);
     const result = await runVariaqJson(c, argv, { timeoutMs: 60_000 });
@@ -605,8 +618,12 @@ export default async function plugin(bb: BbPluginApi) {
     },
   ) {
     const c = await cfg();
-    if (options.compare !== undefined) comparisonTypeSchema.parse(options.compare);
-    await requireComparisonSupport(c, options.compare);
+    // Validate the comparison selector before any VariaQ invocation: an
+    // invalid value must surface as a structured PluginError because the CLI
+    // wrapper only converts PluginError instances into command results (a raw
+    // ZodError would escape as an unhandled exception).
+    const compare = parseComparisonOption(options.compare);
+    await requireComparisonSupport(c, compare);
     const root = resolveReportOutputDir(c);
     const containedDir = await resolveContainedReportDir(root, options.outputDir);
 
@@ -617,7 +634,7 @@ export default async function plugin(bb: BbPluginApi) {
       for (const key of options.groupBy) argv.push("--group-by", key);
     }
     if (options.scalingX !== undefined) argv.push("--scaling-x", options.scalingX);
-    if (options.compare !== undefined) argv.push("--compare", options.compare);
+    if (compare !== undefined) argv.push("--compare", compare);
     // `--plots` is conveyed only through the canonical formats list.
     if (options.overwrite) argv.push("--overwrite");
 
@@ -846,7 +863,7 @@ export default async function plugin(bb: BbPluginApi) {
               scaling_x: opts.get("scaling-x"),
               include_failed: args.includes("--include-failed"),
               include_unavailable: args.includes("--include-unavailable"),
-              compare: opts.get("compare") as import("./lib/schema.js").AnalyzeQuery["compare"],
+              compare: parseComparisonOption(opts.get("compare")),
             });
           }
           case "analyze-campaign": {
@@ -858,7 +875,7 @@ export default async function plugin(bb: BbPluginApi) {
               scaling_x: opts.get("scaling-x"),
               include_failed: args.includes("--include-failed"),
               include_unavailable: args.includes("--include-unavailable"),
-              compare: opts.get("compare") as import("./lib/schema.js").AnalyzeQuery["compare"],
+              compare: parseComparisonOption(opts.get("compare")),
             });
           }
           case "report-campaign": {
@@ -1313,6 +1330,37 @@ export default async function plugin(bb: BbPluginApi) {
 
 // ------------------------------------------------------------ CLI helpers --
 
+/**
+ * Validate a user-supplied comparison selector from the CLI boundary.
+ *
+ * Invalid values must produce a structured, actionable PluginError: the CLI
+ * wrapper only converts PluginError instances into command results (exit code
+ * + "message / Hint:" stderr), and rethrows everything else — a raw ZodError
+ * from schema.parse() would escape that handler as an unhandled exception.
+ */
+function parseComparisonOption(value: string | undefined): ComparisonType | undefined {
+  if (value === undefined) return undefined;
+  const parsed = comparisonTypeSchema.safeParse(value);
+  if (!parsed.success) {
+    throw new PluginError(
+      `Invalid comparison '${value}'.`,
+      `Valid comparisons: ${comparisonTypeSchema.options.join(", ")}.`,
+    );
+  }
+  return parsed.data;
+}
+
+/**
+ * Reduce a ZodError to its first issue as a short "path: message" string for
+ * embedding in a PluginError message.
+ */
+function minimizeZodIssue(error: z.ZodError): string {
+  const issue = error.issues[0];
+  if (issue === undefined) return error.message;
+  const path = issue.path.length > 0 ? `${issue.path.join(".")}: ` : "";
+  return `${path}${issue.message}`;
+}
+
 function readOptions(args: string[]): Map<string, string> {
   const map = new Map<string, string>();
   for (let i = 0; i < args.length; i++) {

tests/fake.test.ts

@@ -1,6 +1,6 @@
 import { afterEach, describe, expect, it, vi } from "vitest";
 import { createFakePluginHost } from "@get-bb/plugin-sdk/testing";
-import { mkdirSync, mkdtempSync, rmSync, symlinkSync } from "node:fs";
+import { mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { dirname, join } from "node:path";
 import { fileURLToPath } from "node:url";
@@ -669,6 +669,96 @@ describe("report campaign", () => {
     expect(cliResult.stdout).toContain("Report: report-");
     expect(cliResult.stdout).toContain("generated files: 4");
   });
+
+  it("keeps valid, invalid, and omitted --compare values on the structured CLI error path", async () => {
+    const h = await setup();
+    const run = JSON.parse(String(await tool(h, "variaq_campaign_run", {
+      campaign_format_version: "1",
+      name: "compare-guard",
+      family: "maxcut",
+      problem_sizes: [4],
+      problem_seeds: [1],
+      solvers: ["exact"],
+      repeats: 1,
+      base_seed: 42,
+      generator_parameters: {},
+      solver_config: {},
+      maxRuns: 10,
+    })));
+
+    // Omitted: still valid, exit 0 with the human-readable summary.
+    const omitted = await cli(h, [
+      "report-campaign", run.campaignId, "--output-dir", "compare-omitted", "--formats", "json",
+    ]);
+    expect(omitted.exitCode, omitted.stderr).toBe(0);
+    expect(String(omitted.stdout)).toContain("Report: report-");
+
+    // Valid comparison: accepted and actually forwarded to the VariaQ
+    // subprocess. The fake records the received value in the generated
+    // markdown artifact ("comparison: <value>"); the plugin's typed result
+    // intentionally drops extra envelope fields, so read the artifact.
+    const valid = JSON.parse(String(await tool(h, "variaq_report_campaign", {
+      campaignId: run.campaignId,
+      outputDir: "compare-valid-tool",
+      formats: ["json", "markdown"],
+      compare: "classical_vs_quantum",
+    })));
+    expect(valid.reportId).toMatch(/^report-/);
+    expect(readFileSync(valid.paths.markdown, "utf8")).toContain("comparison: classical_vs_quantum");
+
+    // Invalid comparison: structured CLI error, never a leaked ZodError.
+    const invalid = await cli(h, [
+      "report-campaign", run.campaignId, "--output-dir", "compare-invalid",
+      "--formats", "json", "--compare", "totally-bogus", "--overwrite", "--json",
+    ]);
+    expect(invalid.exitCode, invalid.stderr).toBe(1);
+    expect(String(invalid.stderr)).toContain("Invalid comparison 'totally-bogus'");
+    expect(String(invalid.stderr)).toContain("Valid comparisons: classical_vs_quantum, qiskit_vs_cudaq, gpu_vs_cpu, paired_solvers");
+    expect(String(invalid.stderr)).not.toMatch(/ZodError/i);
+  });
+
+  it("rejects an invalid --compare on analyze commands as a structured CLI error", async () => {
+    const h = await setup();
+    // Validation precedes any VariaQ invocation, so a non-existent campaign id
+    // must fail with the plugin's structured comparison error, not VariaQ's
+    // exit-2 lookup failure and not a leaked ZodError.
+    const badRuns = await cli(h, ["analyze-runs", "--run-id", "run-1", "--compare", "nope", "--json"]);
+    expect(badRuns.exitCode, badRuns.stderr).toBe(1);
+    expect(String(badRuns.stderr)).toContain("Invalid comparison 'nope'");
+    expect(String(badRuns.stderr)).toContain("Valid comparisons:");
+    expect(String(badRuns.stderr)).not.toMatch(/ZodError/i);
+
+    const badCampaign = await cli(h, ["analyze-campaign", "campaign-missing", "--compare", "nope", "--json"]);
+    expect(badCampaign.exitCode, badCampaign.stderr).toBe(1);
+    expect(String(badCampaign.stderr)).toContain("Invalid comparison 'nope'");
+  });
+
+  it("surfaces operation-level schema errors over the CLI as structured errors", async () => {
+    const h = await setup();
+    // Nine --group-by values pass the comparison parser but violate
+    // analyzeQuerySchema's group_by max(8) inside the operation, exercising
+    // the safeParse -> structured PluginError conversion there.
+    const groupBy = ["a", "b", "c", "d", "e", "f", "g", "h", "i"];
+    const badRuns = await cli(h, [
+      "analyze-runs", "--run-id", "run-1",
+      ...groupBy.flatMap((key) => ["--group-by", key]),
+      "--json",
+    ]);
+    expect(badRuns.exitCode, badRuns.stderr).toBe(1);
+    expect(String(badRuns.stderr)).toContain("Invalid analyze-runs input: group_by: ");
+    expect(String(badRuns.stderr)).toContain("Hint:");
+    expect(String(badRuns.stderr)).not.toMatch(/ZodError/i);
+
+    const badCampaign = await cli(h, [
+      "analyze-campaign", "campaign-schema-guard",
+      ...groupBy.flatMap((key) => ["--group-by", key]),
+      "--json",
+    ]);
+    expect(badCampaign.exitCode, badCampaign.stderr).toBe(1);
+    expect(String(badCampaign.stderr)).toContain("Invalid analyze-campaign input: group_by: ");
+    expect(String(badCampaign.stderr)).toContain("Hint:");
+    expect(String(badCampaign.stderr)).not.toMatch(/ZodError/i);
+  });
 });
 
 describe("benchmark", () => {

tests/fake_variaq.py

@@ -350,6 +350,7 @@ def main(argv):
         formats = ["json"]
         group_by = []
         scaling_x = None
+        compare = None
         plots = False
         overwrite = False
         for i, a in enumerate(args):
@@ -361,6 +362,8 @@ def main(argv):
                 group_by.append(args[i + 1])
             if a == "--scaling-x" and i + 1 < len(args):
                 scaling_x = args[i + 1]
+            if a == "--compare" and i + 1 < len(args):
+                compare = args[i + 1]
             if a == "--plots":
                 plots = True
             if a == "--overwrite":
@@ -410,14 +413,21 @@ def main(argv):
         if "markdown" in formats:
             md_path = os.path.join(output_dir, f"{report_id}.md")
             with open(md_path, "w") as f:
-                f.write(f"# Report {report_id}\n")
+                # "comparison:" echoes the received --compare argv value
+                # (None when the flag was absent) so tests can assert that the
+                # plugin actually forwarded the comparison selector.
+                f.write(f"# Report {report_id}\ncomparison: {compare}\n")
             paths["markdown"] = md_path
         if "plots" in formats:
             plot_path = os.path.join(output_dir, f"{report_id}_plot.png")
             with open(plot_path, "wb") as f:
                 f.write(b"PNG")
             paths["plots"] = {"summary": plot_path}
-        print(json.dumps(envelope("report campaign", "success", {"report_id": report_id, "paths": paths})))
+        print(json.dumps(envelope("report campaign", "success", {
+            "report_id": report_id,
+            "paths": paths,
+            "compare": compare,
+        })))
         return 0
 
     if cmd == "capabilities":