Skip to content

Commit c95d348

Browse files
steipetesteipete
committed
fix(cli): reject loose numeric options
1 parent 717003a commit c95d348

15 files changed

Lines changed: 194 additions & 14 deletions

‎src/cli/capability-cli.test.ts‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1896,6 +1896,28 @@ describe("capability cli", () => {
18961896
expectRuntimeErrorContains("Video asset at index 0 has neither buffer nor url");
18971897
});
18981898

1899+
it("rejects partial image generate count before provider dispatch", async () => {
1900+
await expect(
1901+
runRegisteredCli({
1902+
register: registerCapabilityCli as (program: Command) => void,
1903+
argv: ["capability", "image", "generate", "--prompt", "portrait", "--count", "2x"],
1904+
}),
1905+
).rejects.toThrow("exit 1");
1906+
expectRuntimeErrorContains("--count must be a positive integer");
1907+
expect(mocks.generateImage).not.toHaveBeenCalled();
1908+
});
1909+
1910+
it("rejects partial image generate timeout before provider dispatch", async () => {
1911+
await expect(
1912+
runRegisteredCli({
1913+
register: registerCapabilityCli as (program: Command) => void,
1914+
argv: ["capability", "image", "generate", "--prompt", "portrait", "--timeout-ms", "1000ms"],
1915+
}),
1916+
).rejects.toThrow("exit 1");
1917+
expectRuntimeErrorContains("--timeout-ms must be a finite number");
1918+
expect(mocks.generateImage).not.toHaveBeenCalled();
1919+
});
1920+
18991921
it("routes audio transcribe through transcription, not realtime", async () => {
19001922
await runRegisteredCli({
19011923
register: registerCapabilityCli as (program: Command) => void,
@@ -2475,6 +2497,19 @@ describe("capability cli", () => {
24752497
);
24762498
});
24772499

2500+
it("rejects partial web search limit before provider dispatch", async () => {
2501+
const webSearchRuntime = await import("../web-search/runtime.js");
2502+
vi.mocked(webSearchRuntime.runWebSearch).mockClear();
2503+
await expect(
2504+
runRegisteredCli({
2505+
register: registerCapabilityCli as (program: Command) => void,
2506+
argv: ["capability", "web", "search", "--query", "ping", "--limit", "3x"],
2507+
}),
2508+
).rejects.toThrow("exit 1");
2509+
expectRuntimeErrorContains("--limit must be a positive integer");
2510+
expect(webSearchRuntime.runWebSearch).not.toHaveBeenCalled();
2511+
});
2512+
24782513
it("uses the infer web search provider override when resolving SecretRefs", async () => {
24792514
const unresolvedConfig = {
24802515
tools: { web: { search: { provider: "exa", enabled: true } } },

‎src/cli/capability-cli.ts‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ import type {
3838
ImageGenerationBackground,
3939
ImageGenerationOutputFormat,
4040
} from "../image-generation/types.js";
41+
import {
42+
parseStrictFiniteNumber,
43+
parseStrictPositiveInteger,
44+
} from "../infra/parse-finite-number.js";
4145
import { buildMediaUnderstandingRegistry } from "../media-understanding/provider-registry.js";
4246
import type { RunMediaUnderstandingFileResult } from "../media-understanding/runtime-types.js";
4347
import {
@@ -1156,13 +1160,24 @@ function parseOptionalFiniteNumber(
11561160
if (raw === undefined || (typeof raw === "string" && raw.trim() === "")) {
11571161
return undefined;
11581162
}
1159-
const value = Number(raw);
1160-
if (!Number.isFinite(value)) {
1163+
const value = parseStrictFiniteNumber(raw);
1164+
if (value === undefined) {
11611165
throw new Error(`${label} must be a finite number`);
11621166
}
11631167
return value;
11641168
}
11651169

1170+
function parseOptionalPositiveInteger(raw: unknown, label: string): number | undefined {
1171+
if (raw === undefined || (typeof raw === "string" && raw.trim() === "")) {
1172+
return undefined;
1173+
}
1174+
const value = parseStrictPositiveInteger(raw);
1175+
if (value === undefined) {
1176+
throw new Error(`${label} must be a positive integer`);
1177+
}
1178+
return value;
1179+
}
1180+
11661181
function normalizeImageOutputFormat(
11671182
raw: string | undefined,
11681183
): ImageGenerationOutputFormat | undefined {
@@ -1933,7 +1948,7 @@ export function registerCapabilityCli(program: Command) {
19331948
capability: "image.generate",
19341949
prompt: String(opts.prompt),
19351950
model: opts.model as string | undefined,
1936-
count: opts.count ? Number.parseInt(String(opts.count), 10) : undefined,
1951+
count: parseOptionalPositiveInteger(opts.count, "--count"),
19371952
size: opts.size as string | undefined,
19381953
aspectRatio: opts.aspectRatio as string | undefined,
19391954
resolution: opts.resolution as "1K" | "2K" | "4K" | undefined,
@@ -2410,7 +2425,7 @@ export function registerCapabilityCli(program: Command) {
24102425
const result = await runWebSearchCommand({
24112426
query: String(opts.query),
24122427
provider: opts.provider as string | undefined,
2413-
limit: opts.limit ? Number.parseInt(String(opts.limit), 10) : undefined,
2428+
limit: parseOptionalPositiveInteger(opts.limit, "--limit"),
24142429
});
24152430
emitJsonOrText(defaultRuntime, Boolean(opts.json), result, formatEnvelopeForText);
24162431
});

‎src/cli/cron-cli.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -594,6 +594,16 @@ describe("cron cli", () => {
594594
expect(stdoutText()).toContain('"id": "job-1"');
595595
});
596596

597+
it("rejects partial cron runs limit", async () => {
598+
await expectCronCommandExit(["cron", "runs", "--id", "job-1", "--limit", "10x"]);
599+
expectRuntimeErrorContaining("Invalid --limit");
600+
expect(callGatewayFromCli).not.toHaveBeenCalledWith(
601+
"cron.runs",
602+
expect.anything(),
603+
expect.anything(),
604+
);
605+
});
606+
597607
it("paginates cron show lookups", async () => {
598608
resetGatewayMock();
599609
callGatewayFromCli.mockImplementation(
@@ -1352,6 +1362,16 @@ describe("cron cli", () => {
13521362
expect(patch?.patch?.failureAlert?.to).toBe("19098680");
13531363
});
13541364

1365+
it("rejects partial failure alert threshold on cron edit", async () => {
1366+
await expectCronCommandExit(["cron", "edit", "job-1", "--failure-alert-after", "3x"]);
1367+
expectRuntimeErrorContaining("Invalid --failure-alert-after");
1368+
expect(callGatewayFromCli).not.toHaveBeenCalledWith(
1369+
"cron.update",
1370+
expect.anything(),
1371+
expect.anything(),
1372+
);
1373+
});
1374+
13551375
it("supports --no-failure-alert on cron edit", async () => {
13561376
callGatewayFromCli.mockClear();
13571377

‎src/cli/cron-cli/register.cron-edit.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type { Command } from "commander";
22
import type { CronJob } from "../../cron/types.js";
33
import { danger } from "../../globals.js";
4+
import { parseStrictPositiveInteger } from "../../infra/parse-finite-number.js";
45
import { sanitizeAgentId } from "../../routing/session-key.js";
56
import { defaultRuntime } from "../../runtime.js";
67
import {
@@ -363,8 +364,8 @@ export function registerCronEditCommand(cron: Command) {
363364
} else if (failureAlertFlag === true || hasFailureAlertFields) {
364365
const failureAlert: Record<string, unknown> = {};
365366
if (hasFailureAlertAfter) {
366-
const after = Number.parseInt(String(opts.failureAlertAfter), 10);
367-
if (!Number.isFinite(after) || after <= 0) {
367+
const after = parseStrictPositiveInteger(opts.failureAlertAfter);
368+
if (after === undefined) {
368369
throw new Error("Invalid --failure-alert-after (must be a positive integer).");
369370
}
370371
failureAlert.after = after;

‎src/cli/cron-cli/register.cron-simple.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { Command } from "commander";
22
import type { CronDeliveryPreview, CronJob } from "../../cron/types.js";
3+
import { parseStrictPositiveInteger } from "../../infra/parse-finite-number.js";
34
import { defaultRuntime } from "../../runtime.js";
45
import { normalizeLowercaseStringOrEmpty } from "../../shared/string-coerce.js";
56
import type { GatewayRpcOpts } from "../gateway-rpc.js";
@@ -225,8 +226,10 @@ export function registerCronSimpleCommands(cron: Command) {
225226
.option("--limit <n>", "Max entries (default 50)", "50")
226227
.action(async (opts) => {
227228
try {
228-
const limitRaw = Number.parseInt(String(opts.limit ?? "50"), 10);
229-
const limit = Number.isFinite(limitRaw) && limitRaw > 0 ? limitRaw : 50;
229+
const limit = parseStrictPositiveInteger(opts.limit ?? "50");
230+
if (limit === undefined) {
231+
throw new Error("Invalid --limit (must be a positive integer).");
232+
}
230233
const id = String(opts.id);
231234
const res = await callGatewayFromCli("cron.runs", opts, {
232235
id,

‎src/cli/nodes-cli.coverage.test.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,14 @@ describe("nodes-cli coverage", () => {
212212
args: ["nodes", "camera", "snap", "--node", "mac-1", "--invoke-timeout", "20s"],
213213
flag: "--invoke-timeout",
214214
},
215+
{
216+
args: ["nodes", "camera", "snap", "--node", "mac-1", "--quality", "0.8jpg"],
217+
flag: "--quality",
218+
},
219+
{
220+
args: ["nodes", "camera", "snap", "--node", "mac-1", "--quality", "1.1"],
221+
flag: "--quality",
222+
},
215223
{
216224
args: ["nodes", "camera", "clip", "--node", "mac-1", "--invoke-timeout", "90s"],
217225
flag: "--invoke-timeout",
@@ -224,6 +232,14 @@ describe("nodes-cli coverage", () => {
224232
args: ["nodes", "screen", "record", "--node", "mac-1", "--invoke-timeout", "120s"],
225233
flag: "--invoke-timeout",
226234
},
235+
{
236+
args: ["nodes", "screen", "record", "--node", "mac-1", "--fps", "10fps"],
237+
flag: "--fps",
238+
},
239+
{
240+
args: ["nodes", "screen", "record", "--node", "mac-1", "--fps", "0"],
241+
flag: "--fps",
242+
},
227243
{
228244
args: ["nodes", "notify", "--node", "mac-1", "--title", "Ping", "--invoke-timeout", "15s"],
229245
flag: "--invoke-timeout",

‎src/cli/nodes-cli/register.camera.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
buildNodeInvokeParams,
2121
callGatewayCli,
2222
nodesCallOpts,
23+
parseOptionalNodeFiniteNumber,
2324
parseOptionalNodeNonNegativeInteger,
2425
parseOptionalNodePositiveInteger,
2526
resolveNode,
@@ -134,7 +135,10 @@ export function registerNodesCameraCommands(nodes: Command) {
134135
})();
135136

136137
const maxWidth = parseOptionalNodePositiveInteger(opts.maxWidth, "--max-width");
137-
const quality = opts.quality ? Number.parseFloat(opts.quality) : undefined;
138+
const quality = parseOptionalNodeFiniteNumber(opts.quality, "--quality", {
139+
minInclusive: 0,
140+
maxInclusive: 1,
141+
});
138142
const delayMs = parseOptionalNodeNonNegativeInteger(opts.delayMs, "--delay-ms");
139143
const deviceId = normalizeOptionalString(opts.deviceId);
140144
if (deviceId && facings.length > 1) {

‎src/cli/nodes-cli/register.screen.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
buildNodeInvokeParams,
1313
callGatewayCli,
1414
nodesCallOpts,
15+
parseOptionalNodeFiniteNumber,
1516
parseOptionalNodeNonNegativeInteger,
1617
parseOptionalNodePositiveInteger,
1718
resolveNodeId,
@@ -39,7 +40,9 @@ export function registerNodesScreenCommands(nodes: Command) {
3940
const nodeId = await resolveNodeId(opts, opts.node ?? "");
4041
const durationMs = parseDurationMs(opts.duration ?? "");
4142
const screenIndex = parseOptionalNodeNonNegativeInteger(opts.screen ?? "0", "--screen");
42-
const fps = Number.parseFloat(opts.fps ?? "10");
43+
const fps = parseOptionalNodeFiniteNumber(opts.fps ?? "10", "--fps", {
44+
minExclusive: 0,
45+
});
4346
const timeoutMs = parseOptionalNodePositiveInteger(
4447
opts.invokeTimeout,
4548
"--invoke-timeout",

‎src/cli/nodes-cli/rpc.ts‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { randomUUID } from "node:crypto";
22
import type { Command } from "commander";
33
import type { OperatorScope } from "../../gateway/method-scopes.js";
44
import {
5+
parseStrictFiniteNumber,
56
parseStrictNonNegativeInteger,
67
parseStrictPositiveInteger,
78
} from "../../infra/parse-finite-number.js";
@@ -96,6 +97,34 @@ export function parseOptionalNodeNonNegativeInteger(
9697
return parsed;
9798
}
9899

100+
export function parseOptionalNodeFiniteNumber(
101+
value: unknown,
102+
flag: string,
103+
bounds?: {
104+
minExclusive?: number;
105+
minInclusive?: number;
106+
maxInclusive?: number;
107+
},
108+
): number | undefined {
109+
if (!hasOptionalValue(value)) {
110+
return undefined;
111+
}
112+
const parsed = parseStrictFiniteNumber(value);
113+
if (parsed === undefined) {
114+
throw new Error(`${flag} must be a finite number.`);
115+
}
116+
if (bounds?.minExclusive !== undefined && parsed <= bounds.minExclusive) {
117+
throw new Error(`${flag} must be greater than ${bounds.minExclusive}.`);
118+
}
119+
if (bounds?.minInclusive !== undefined && parsed < bounds.minInclusive) {
120+
throw new Error(`${flag} must be at least ${bounds.minInclusive}.`);
121+
}
122+
if (bounds?.maxInclusive !== undefined && parsed > bounds.maxInclusive) {
123+
throw new Error(`${flag} must be at most ${bounds.maxInclusive}.`);
124+
}
125+
return parsed;
126+
}
127+
99128
export function unauthorizedHintForMessage(message: string): string | null {
100129
const haystack = normalizeLowercaseStringOrEmpty(message);
101130
if (

‎src/cli/program.smoke.test.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,14 @@ describe("cli program (smoke)", () => {
8787
expect(options?.timeoutMs).toBeUndefined();
8888
});
8989

90+
it("rejects partial tui history limits", async () => {
91+
await expect(runProgram(["tui", "--history-limit", "10x"])).rejects.toThrow("exit");
92+
expect(runtime.error).toHaveBeenCalledWith(
93+
"Error: --history-limit must be a positive integer.",
94+
);
95+
expect(runTui).not.toHaveBeenCalled();
96+
});
97+
9098
it("runs setup wizard when wizard flags are present", async () => {
9199
await runProgram(["setup", "--remote-url", "ws://example"]);
92100

0 commit comments

Comments
 (0)