fix(canvas): validate CLI numeric options

rohitjavvadi · web-flow · commit e55cebf4c278 · 2026-06-14T07:39:17.000+08:00
Validate Canvas CLI numeric arguments before node invocation.

- Reject malformed `--invoke-timeout` values through the shared Canvas invoke path before resolving a node.
- Keep snapshot `--quality` bounded to the existing Canvas tool schema range of 0..1.
- Add focused CLI regressions for timeout validation, quality bounds, and accepted boundary values.

Verification:
- `node scripts/run-vitest.mjs extensions/canvas/src/cli.test.ts --maxWorkers=1`
- `.agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main ...`
- AWS Crabbox `cbx_b7838c58daba`, run `run_bbb58ca536f4`: `check:changed`
- Exact PR head `a970ce594ea6e7284ace519352fc258b1b81cb80`: GitHub checks green
diff --git a/extensions/canvas/src/cli.test.ts b/extensions/canvas/src/cli.test.ts
@@ -1,7 +1,11 @@
 // Canvas tests cover cli plugin behavior.
 import { Command } from "commander";
 import { describe, expect, it, vi } from "vitest";
-import { registerNodesCanvasCommands, type CanvasCliDependencies } from "./cli.js";
+import {
+  createDefaultCanvasCliDependencies,
+  registerNodesCanvasCommands,
+  type CanvasCliDependencies,
+} from "./cli.js";
 
 function createCanvasCliDeps() {
   const writtenFiles: Array<{ filePath: string; base64: string }> = [];
@@ -47,6 +51,26 @@ function createCanvasCliDeps() {
   return { deps, runtime, writtenFiles };
 }
 
+function createCanvasCliDepsWithDefaultParsers() {
+  const baseDeps = createDefaultCanvasCliDependencies();
+  const harness = createCanvasCliDeps();
+  return {
+    ...harness,
+    deps: {
+      ...baseDeps,
+      defaultRuntime: harness.runtime,
+      nodesCallOpts: harness.deps.nodesCallOpts,
+      runNodesCommand: harness.deps.runNodesCommand,
+      getNodesTheme: harness.deps.getNodesTheme,
+      resolveNodeId: harness.deps.resolveNodeId,
+      buildNodeInvokeParams: harness.deps.buildNodeInvokeParams,
+      callGatewayCli: harness.deps.callGatewayCli,
+      writeBase64ToFile: harness.deps.writeBase64ToFile,
+      shortenHomePath: harness.deps.shortenHomePath,
+    },
+  };
+}
+
 describe("canvas CLI", () => {
   it("registers under nodes and captures a snapshot media path", async () => {
     const program = new Command();
@@ -135,6 +159,8 @@ describe("canvas CLI", () => {
   it.each([
     ["--max-width", "640px", "--max-width must be a positive integer."],
     ["--quality", "0.8x", "--quality must be a number."],
+    ["--quality", "-0.1", "--quality must be between 0 and 1."],
+    ["--quality", "5", "--quality must be between 0 and 1."],
   ])("rejects partial numeric snapshot %s values", async (flag, value, message) => {
     const program = new Command();
     program.exitOverride();
@@ -151,6 +177,62 @@ describe("canvas CLI", () => {
     expect(deps.callGatewayCli).not.toHaveBeenCalled();
   });
 
+  it.each(["0", "1"])("accepts snapshot --quality boundary value %s", async (quality) => {
+    const program = new Command();
+    program.exitOverride();
+    const nodes = program.command("nodes");
+    const { deps } = createCanvasCliDeps();
+
+    registerNodesCanvasCommands(nodes, deps);
+
+    await program.parseAsync(
+      ["nodes", "canvas", "snapshot", "--node", "ios-node", "--quality", quality],
+      {
+        from: "user",
+      },
+    );
+    expect(deps.callGatewayCli).toHaveBeenCalledWith(
+      "node.invoke",
+      expect.any(Object),
+      expect.objectContaining({
+        params: expect.objectContaining({
+          quality: Number(quality),
+        }),
+      }),
+    );
+  });
+
+  it.each([
+    ["snapshot"],
+    ["present"],
+    ["hide"],
+    ["navigate", "https://example.com"],
+    ["eval", "1 + 1"],
+    ["a2ui", "push", "--text", "hello"],
+    ["a2ui", "reset"],
+  ])("rejects invalid %s invoke timeouts before invoking the node", async (...args) => {
+    const program = new Command();
+    program.exitOverride();
+    const nodes = program.command("nodes");
+    const { deps } = createCanvasCliDepsWithDefaultParsers();
+    deps.resolveNodeId = vi.fn(async () => {
+      throw new Error("resolveNodeId should not be called");
+    });
+
+    registerNodesCanvasCommands(nodes, deps);
+
+    await expect(
+      program.parseAsync(
+        ["nodes", "canvas", ...args, "--node", "ios-node", "--invoke-timeout", "20ms"],
+        {
+          from: "user",
+        },
+      ),
+    ).rejects.toThrow("--invoke-timeout must be a positive integer.");
+    expect(deps.resolveNodeId).not.toHaveBeenCalled();
+    expect(deps.callGatewayCli).not.toHaveBeenCalled();
+  });
+
   it.each([
     ["--x", "1x"],
     ["--y", "2px"],
diff --git a/extensions/canvas/src/cli.ts b/extensions/canvas/src/cli.ts
@@ -97,7 +97,11 @@ function parseTimeoutMs(raw: unknown): number | undefined {
   if (raw === undefined || raw === null) {
     return undefined;
   }
-  return parseStrictPositiveInteger(raw);
+  const parsed = parseStrictPositiveInteger(raw);
+  if (parsed === undefined) {
+    throw new Error("--invoke-timeout must be a positive integer.");
+  }
+  return parsed;
 }
 
 function parseCanvasPositiveIntOption(raw: string | undefined, flag: string): number | undefined {
@@ -122,6 +126,14 @@ function parseCanvasFiniteNumberOption(raw: string | undefined, flag: string): n
   return parsed;
 }
 
+function parseCanvasSnapshotQualityOption(raw: string | undefined): number | undefined {
+  const parsed = parseCanvasFiniteNumberOption(raw, "--quality");
+  if (parsed !== undefined && (parsed < 0 || parsed > 1)) {
+    throw new Error("--quality must be between 0 and 1.");
+  }
+  return parsed;
+}
+
 function parseNodeCandidates(raw: unknown): CanvasNodeCandidate[] {
   const payload =
     raw && typeof raw === "object" ? (raw as { nodes?: unknown; paired?: unknown }) : {};
@@ -245,8 +257,8 @@ async function invokeCanvas(
   command: string,
   params?: Record<string, unknown>,
 ) {
-  const nodeId = await deps.resolveNodeId(opts, normalizeOptionalString(opts.node) ?? "");
   const timeoutMs = deps.parseTimeoutMs(opts.invokeTimeout);
+  const nodeId = await deps.resolveNodeId(opts, normalizeOptionalString(opts.node) ?? "");
   return await deps.callGatewayCli(
     "node.invoke",
     opts,
@@ -278,7 +290,7 @@ export function registerNodesCanvasCommands(nodes: Command, deps: CanvasCliDepen
         await deps.runNodesCommand("canvas snapshot", async () => {
           const format = parseCanvasSnapshotRequestFormat(opts.format);
           const maxWidth = parseCanvasPositiveIntOption(opts.maxWidth, "--max-width");
-          const quality = parseCanvasFiniteNumberOption(opts.quality, "--quality");
+          const quality = parseCanvasSnapshotQualityOption(opts.quality);
           const raw = await invokeCanvas(deps, opts, "canvas.snapshot", {
             format,
             maxWidth: Number.isFinite(maxWidth) ? maxWidth : undefined,
