fix: reject partial numeric CLI options

This commit is contained in:
Peter Steinberger
2026-05-27 03:34:44 -04:00
parent f4e20f806e
commit d2d5010aec
13 changed files with 195 additions and 32 deletions

View File

@@ -186,4 +186,64 @@ describe("nodes-cli coverage", () => {
});
expect(invoke.params?.timeoutMs).toBe(6000);
});
it.each([
{
args: ["nodes", "location", "get", "--node", "mac-1", "--max-age", "1000ms"],
flag: "--max-age",
},
{
args: ["nodes", "location", "get", "--node", "mac-1", "--location-timeout", "5s"],
flag: "--location-timeout",
},
{
args: ["nodes", "location", "get", "--node", "mac-1", "--invoke-timeout", "6s"],
flag: "--invoke-timeout",
},
{
args: ["nodes", "camera", "snap", "--node", "mac-1", "--max-width", "1024px"],
flag: "--max-width",
},
{
args: ["nodes", "camera", "snap", "--node", "mac-1", "--delay-ms", "20ms"],
flag: "--delay-ms",
},
{
args: ["nodes", "camera", "snap", "--node", "mac-1", "--invoke-timeout", "20s"],
flag: "--invoke-timeout",
},
{
args: ["nodes", "camera", "clip", "--node", "mac-1", "--invoke-timeout", "90s"],
flag: "--invoke-timeout",
},
{
args: ["nodes", "screen", "record", "--node", "mac-1", "--screen", "1x"],
flag: "--screen",
},
{
args: ["nodes", "screen", "record", "--node", "mac-1", "--invoke-timeout", "120s"],
flag: "--invoke-timeout",
},
{
args: ["nodes", "notify", "--node", "mac-1", "--title", "Ping", "--invoke-timeout", "15s"],
flag: "--invoke-timeout",
},
{
args: [
"nodes",
"invoke",
"--node",
"mac-1",
"--command",
"canvas.eval",
"--invoke-timeout",
"15s",
],
flag: "--invoke-timeout",
},
])("rejects partial numeric option for $args", async ({ args, flag }) => {
await expect(sharedProgram.parseAsync(args, { from: "user" })).rejects.toThrow("__exit__:1");
expect(runtimeErrors.at(-1)).toContain(`${flag} must be`);
expect(lastNodeInvokeCall).toBeNull();
});
});

View File

@@ -20,6 +20,8 @@ import {
buildNodeInvokeParams,
callGatewayCli,
nodesCallOpts,
parseOptionalNodeNonNegativeInteger,
parseOptionalNodePositiveInteger,
resolveNode,
resolveNodeId,
} from "./rpc.js";
@@ -131,16 +133,17 @@ export function registerNodesCameraCommands(nodes: Command) {
);
})();
const maxWidth = opts.maxWidth ? Number.parseInt(opts.maxWidth, 10) : undefined;
const maxWidth = parseOptionalNodePositiveInteger(opts.maxWidth, "--max-width");
const quality = opts.quality ? Number.parseFloat(opts.quality) : undefined;
const delayMs = opts.delayMs ? Number.parseInt(opts.delayMs, 10) : undefined;
const delayMs = parseOptionalNodeNonNegativeInteger(opts.delayMs, "--delay-ms");
const deviceId = normalizeOptionalString(opts.deviceId);
if (deviceId && facings.length > 1) {
throw new Error("facing=both is not allowed when --device-id is set");
}
const timeoutMs = opts.invokeTimeout
? Number.parseInt(opts.invokeTimeout, 10)
: undefined;
const timeoutMs = parseOptionalNodePositiveInteger(
opts.invokeTimeout,
"--invoke-timeout",
);
const results: Array<{
facing: CameraFacing;
@@ -216,9 +219,10 @@ export function registerNodesCameraCommands(nodes: Command) {
const facing = parseFacing(opts.facing ?? "front");
const durationMs = parseDurationMs(opts.duration ?? "3000");
const includeAudio = opts.audio !== false;
const timeoutMs = opts.invokeTimeout
? Number.parseInt(opts.invokeTimeout, 10)
: undefined;
const timeoutMs = parseOptionalNodePositiveInteger(
opts.invokeTimeout,
"--invoke-timeout",
);
const deviceId = normalizeOptionalString(opts.deviceId);
const invokeParams = buildNodeInvokeParams({

View File

@@ -6,7 +6,12 @@ import {
normalizeOptionalString,
} from "../../shared/string-coerce.js";
import { getNodesTheme, runNodesCommand } from "./cli-utils.js";
import { callGatewayCli, nodesCallOpts, resolveNodeId } from "./rpc.js";
import {
callGatewayCli,
nodesCallOpts,
parseOptionalNodePositiveInteger,
resolveNodeId,
} from "./rpc.js";
import type { NodesRpcOpts } from "./types.js";
const BLOCKED_NODE_INVOKE_COMMANDS = new Set(["system.run", "system.run.prepare"]);
@@ -37,9 +42,10 @@ export function registerNodesInvokeCommands(nodes: Command) {
);
}
const params = JSON.parse(opts.params ?? "{}") as unknown;
const timeoutMs = opts.invokeTimeout
? Number.parseInt(opts.invokeTimeout, 10)
: undefined;
const timeoutMs = parseOptionalNodePositiveInteger(
opts.invokeTimeout,
"--invoke-timeout",
);
const invokeParams: Record<string, unknown> = {
nodeId,

View File

@@ -3,7 +3,13 @@ import { randomIdempotencyKey } from "../../gateway/call.js";
import { defaultRuntime } from "../../runtime.js";
import { normalizeOptionalLowercaseString } from "../../shared/string-coerce.js";
import { runNodesCommand } from "./cli-utils.js";
import { callGatewayCli, nodesCallOpts, resolveNodeId } from "./rpc.js";
import {
callGatewayCli,
nodesCallOpts,
parseOptionalNodeNonNegativeInteger,
parseOptionalNodePositiveInteger,
resolveNodeId,
} from "./rpc.js";
import type { NodesRpcOpts } from "./types.js";
export function registerNodesLocationCommands(nodes: Command) {
@@ -24,7 +30,7 @@ export function registerNodesLocationCommands(nodes: Command) {
.action(async (opts: NodesRpcOpts) => {
await runNodesCommand("location get", async () => {
const nodeId = await resolveNodeId(opts, opts.node ?? "");
const maxAgeMs = opts.maxAge ? Number.parseInt(opts.maxAge, 10) : undefined;
const maxAgeMs = parseOptionalNodeNonNegativeInteger(opts.maxAge, "--max-age");
const desiredAccuracyRaw = normalizeOptionalLowercaseString(opts.accuracy);
const desiredAccuracy =
desiredAccuracyRaw === "coarse" ||
@@ -32,12 +38,14 @@ export function registerNodesLocationCommands(nodes: Command) {
desiredAccuracyRaw === "precise"
? desiredAccuracyRaw
: undefined;
const timeoutMs = opts.locationTimeout
? Number.parseInt(opts.locationTimeout, 10)
: undefined;
const invokeTimeoutMs = opts.invokeTimeout
? Number.parseInt(opts.invokeTimeout, 10)
: undefined;
const timeoutMs = parseOptionalNodePositiveInteger(
opts.locationTimeout,
"--location-timeout",
);
const invokeTimeoutMs = parseOptionalNodePositiveInteger(
opts.invokeTimeout,
"--invoke-timeout",
);
const invokeParams: Record<string, unknown> = {
nodeId,

View File

@@ -3,7 +3,12 @@ import { randomIdempotencyKey } from "../../gateway/call.js";
import { defaultRuntime } from "../../runtime.js";
import { normalizeOptionalString } from "../../shared/string-coerce.js";
import { getNodesTheme, runNodesCommand } from "./cli-utils.js";
import { callGatewayCli, nodesCallOpts, resolveNodeId } from "./rpc.js";
import {
callGatewayCli,
nodesCallOpts,
parseOptionalNodePositiveInteger,
resolveNodeId,
} from "./rpc.js";
import type { NodesRpcOpts } from "./types.js";
export function registerNodesNotifyCommand(nodes: Command) {
@@ -26,9 +31,10 @@ export function registerNodesNotifyCommand(nodes: Command) {
if (!title && !body) {
throw new Error("missing --title or --body");
}
const invokeTimeout = opts.invokeTimeout
? Number.parseInt(opts.invokeTimeout, 10)
: undefined;
const invokeTimeout = parseOptionalNodePositiveInteger(
opts.invokeTimeout,
"--invoke-timeout",
);
const invokeParams: Record<string, unknown> = {
nodeId,
command: "system.notify",

View File

@@ -8,7 +8,14 @@ import {
} from "../nodes-screen.js";
import { parseDurationMs } from "../parse-duration.js";
import { runNodesCommand } from "./cli-utils.js";
import { buildNodeInvokeParams, callGatewayCli, nodesCallOpts, resolveNodeId } from "./rpc.js";
import {
buildNodeInvokeParams,
callGatewayCli,
nodesCallOpts,
parseOptionalNodeNonNegativeInteger,
parseOptionalNodePositiveInteger,
resolveNodeId,
} from "./rpc.js";
import type { NodesRpcOpts } from "./types.js";
export function registerNodesScreenCommands(nodes: Command) {
@@ -31,11 +38,12 @@ export function registerNodesScreenCommands(nodes: Command) {
await runNodesCommand("screen record", async () => {
const nodeId = await resolveNodeId(opts, opts.node ?? "");
const durationMs = parseDurationMs(opts.duration ?? "");
const screenIndex = Number.parseInt(opts.screen ?? "0", 10);
const screenIndex = parseOptionalNodeNonNegativeInteger(opts.screen ?? "0", "--screen");
const fps = Number.parseFloat(opts.fps ?? "10");
const timeoutMs = opts.invokeTimeout
? Number.parseInt(opts.invokeTimeout, 10)
: undefined;
const timeoutMs = parseOptionalNodePositiveInteger(
opts.invokeTimeout,
"--invoke-timeout",
);
const invokeParams = buildNodeInvokeParams({
nodeId,

View File

@@ -1,6 +1,10 @@
import { randomUUID } from "node:crypto";
import type { Command } from "commander";
import type { OperatorScope } from "../../gateway/method-scopes.js";
import {
parseStrictNonNegativeInteger,
parseStrictPositiveInteger,
} from "../../infra/parse-finite-number.js";
import { createLazyImportLoader } from "../../shared/lazy-promise.js";
import { resolveNodeFromNodeList } from "../../shared/node-resolve.js";
import { normalizeLowercaseStringOrEmpty } from "../../shared/string-coerce.js";
@@ -63,6 +67,35 @@ export function buildNodeInvokeParams(params: {
return invokeParams;
}
function hasOptionalValue(value: unknown): boolean {
return value !== undefined && value !== null && value !== "";
}
export function parseOptionalNodePositiveInteger(value: unknown, flag: string): number | undefined {
if (!hasOptionalValue(value)) {
return undefined;
}
const parsed = parseStrictPositiveInteger(value);
if (parsed === undefined) {
throw new Error(`${flag} must be a positive integer.`);
}
return parsed;
}
export function parseOptionalNodeNonNegativeInteger(
value: unknown,
flag: string,
): number | undefined {
if (!hasOptionalValue(value)) {
return undefined;
}
const parsed = parseStrictNonNegativeInteger(value);
if (parsed === undefined) {
throw new Error(`${flag} must be a non-negative integer.`);
}
return parsed;
}
export function unauthorizedHintForMessage(message: string): string | null {
const haystack = normalizeLowercaseStringOrEmpty(message);
if (

View File

@@ -3,6 +3,7 @@ import { formatDocsLink } from "../terminal/links.js";
import { theme } from "../terminal/theme.js";
import type { PluginInspectOptions } from "./plugins-inspect-command.js";
import type { PluginsListOptions } from "./plugins-list-command.js";
import { parseStrictPositiveIntOption } from "./program/helpers.js";
import { applyParentDefaultHelpAction } from "./program/parent-default-help.js";
export type PluginUpdateOptions = {
@@ -75,7 +76,7 @@ export function registerPluginsCli(program: Command) {
.command("search")
.description("Search ClawHub plugin packages")
.argument("[query...]", "Search query")
.option("--limit <n>", "Max results", (value) => Number.parseInt(value, 10))
.option("--limit <n>", "Max results", (value) => parseStrictPositiveIntOption(value, "--limit"))
.option("--json", "Print JSON", false)
.action(async (queryParts: string[], opts: PluginSearchOptions) => {
const { runPluginsSearchCommand } = await import("./plugins-search-command.js");

View File

@@ -1,3 +1,4 @@
import { Command } from "commander";
import { beforeEach, describe, expect, it, vi } from "vitest";
const mocks = vi.hoisted(() => {
@@ -35,6 +36,7 @@ vi.mock("../infra/clawhub.js", () => ({
}));
const { runPluginsSearchCommand } = await import("./plugins-search-command.js");
const { registerPluginsCli } = await import("./plugins-cli.js");
describe("plugins search command", () => {
beforeEach(() => {
@@ -107,4 +109,15 @@ describe("plugins search command", () => {
expect(mocks.runtime.writeJson).toHaveBeenCalledWith({ results: [] }, 2);
});
it("rejects partial numeric search limits", async () => {
const program = new Command();
program.exitOverride();
registerPluginsCli(program);
await expect(
program.parseAsync(["plugins", "search", "calendar", "--limit", "10ms"], { from: "user" }),
).rejects.toThrow("--limit must be a positive integer.");
expect(mocks.searchClawHubPackages).not.toHaveBeenCalled();
});
});

View File

@@ -3,6 +3,7 @@ import { describe, expect, it } from "vitest";
import {
collectOption,
parsePositiveIntOrUndefined,
parseStrictPositiveIntOption,
parseStrictPositiveIntOrUndefined,
resolveActionArgs,
resolveCommandOptionArgs,
@@ -53,6 +54,13 @@ describe("program helpers", () => {
expect(parseStrictPositiveIntOrUndefined(value)).toBe(expected);
});
it("parseStrictPositiveIntOption rejects partial numeric strings", () => {
expect(parseStrictPositiveIntOption("10", "--limit")).toBe(10);
expect(() => parseStrictPositiveIntOption("10ms", "--limit")).toThrow(
"--limit must be a positive integer.",
);
});
it("resolveActionArgs returns args when command has arg array", () => {
const command = new Command();
(command as Command & { args?: string[] }).args = ["one", "two"];

View File

@@ -1,4 +1,4 @@
import type { Command } from "commander";
import { InvalidArgumentError, type Command } from "commander";
import { parseStrictPositiveInteger } from "../../infra/parse-finite-number.js";
export function collectOption(value: string, previous: string[] = []): string[] {
@@ -30,6 +30,14 @@ export function parseStrictPositiveIntOrUndefined(value: unknown): number | unde
return parseStrictPositiveInteger(value);
}
export function parseStrictPositiveIntOption(value: string, flag: string): number {
const parsed = parseStrictPositiveInteger(value);
if (parsed === undefined) {
throw new InvalidArgumentError(`${flag} must be a positive integer.`);
}
return parsed;
}
export function resolveActionArgs(actionCommand?: Command): string[] {
if (!actionCommand) {
return [];

View File

@@ -289,6 +289,13 @@ describe("skills cli commands", () => {
).toBe(true);
});
it("rejects partial numeric search limits", async () => {
await expect(runCommand(["skills", "search", "calendar", "--limit", "10ms"])).rejects.toThrow(
"--limit must be a positive integer.",
);
expect(searchSkillsFromClawHubMock).not.toHaveBeenCalled();
});
it("installs a skill from ClawHub into the active workspace", async () => {
installSkillFromClawHubMock.mockResolvedValue({
ok: true,

View File

@@ -21,6 +21,7 @@ import { formatDocsLink } from "../terminal/links.js";
import { theme } from "../terminal/theme.js";
import { CONFIG_DIR } from "../utils.js";
import { resolveOptionFromCommand } from "./cli-utils.js";
import { parseStrictPositiveIntOption } from "./program/helpers.js";
import { formatSkillInfo, formatSkillsCheck, formatSkillsList } from "./skills-cli.format.js";
export type {
@@ -123,7 +124,7 @@ export function registerSkillsCli(program: Command) {
.command("search")
.description("Search ClawHub skills")
.argument("[query...]", "Optional search query")
.option("--limit <n>", "Max results", (value) => Number.parseInt(value, 10))
.option("--limit <n>", "Max results", (value) => parseStrictPositiveIntOption(value, "--limit"))
.option("--json", "Output as JSON", false)
.action(async (queryParts: string[], opts: { limit?: number; json?: boolean }) => {
try {