diff --git a/extensions/canvas/src/cli.test.ts b/extensions/canvas/src/cli.test.ts index 86360f56b46..25af3c88ec6 100644 --- a/extensions/canvas/src/cli.test.ts +++ b/extensions/canvas/src/cli.test.ts @@ -130,4 +130,44 @@ describe("canvas CLI", () => { expect(deps.callGatewayCli).not.toHaveBeenCalled(); expect(writtenFiles).toHaveLength(0); }); + + it.each([ + ["--max-width", "640px", "--max-width must be a positive integer."], + ["--quality", "0.8x", "--quality must be a number."], + ])("rejects partial numeric snapshot %s values", async (flag, value, message) => { + const program = new Command(); + program.exitOverride(); + const nodes = program.command("nodes"); + const { deps } = createCanvasCliDeps(); + + registerNodesCanvasCommands(nodes, deps); + + await expect( + program.parseAsync(["nodes", "canvas", "snapshot", "--node", "ios-node", flag, value], { + from: "user", + }), + ).rejects.toThrow(message); + expect(deps.callGatewayCli).not.toHaveBeenCalled(); + }); + + it.each([ + ["--x", "1x"], + ["--y", "2px"], + ["--width", "800wide"], + ["--height", "600tall"], + ])("rejects partial numeric present %s values", async (flag, value) => { + const program = new Command(); + program.exitOverride(); + const nodes = program.command("nodes"); + const { deps } = createCanvasCliDeps(); + + registerNodesCanvasCommands(nodes, deps); + + await expect( + program.parseAsync(["nodes", "canvas", "present", "--node", "ios-node", flag, value], { + from: "user", + }), + ).rejects.toThrow(`${flag} must be a number.`); + expect(deps.callGatewayCli).not.toHaveBeenCalled(); + }); }); diff --git a/extensions/canvas/src/cli.ts b/extensions/canvas/src/cli.ts index 90467e52374..30404a8fea6 100644 --- a/extensions/canvas/src/cli.ts +++ b/extensions/canvas/src/cli.ts @@ -8,6 +8,10 @@ import { resolveNodeFromNodeList, type NodeMatchCandidate, } from "openclaw/plugin-sdk/gateway-runtime"; +import { + parseStrictFiniteNumber, + parseStrictPositiveInteger, +} from "openclaw/plugin-sdk/infra-runtime"; import { defaultRuntime } from "openclaw/plugin-sdk/runtime"; import { normalizeLowercaseStringOrEmpty, @@ -87,13 +91,29 @@ function parseTimeoutMs(raw: unknown): number | undefined { if (raw === undefined || raw === null) { return undefined; } - const value = - typeof raw === "number" || typeof raw === "bigint" - ? Number(raw) - : typeof raw === "string" && raw.trim() - ? Number.parseInt(raw.trim(), 10) - : Number.NaN; - return Number.isFinite(value) ? value : undefined; + return parseStrictPositiveInteger(raw); +} + +function parseCanvasPositiveIntOption(raw: string | undefined, flag: string): number | undefined { + if (!raw) { + return undefined; + } + const parsed = parseStrictPositiveInteger(raw); + if (parsed === undefined) { + throw new Error(`${flag} must be a positive integer.`); + } + return parsed; +} + +function parseCanvasFiniteNumberOption(raw: string | undefined, flag: string): number | undefined { + if (!raw) { + return undefined; + } + const parsed = parseStrictFiniteNumber(raw); + if (parsed === undefined) { + throw new Error(`${flag} must be a number.`); + } + return parsed; } function parseNodeCandidates(raw: unknown): CanvasNodeCandidate[] { @@ -249,8 +269,8 @@ export function registerNodesCanvasCommands(nodes: Command, deps: CanvasCliDepen .action(async (opts: CanvasNodesRpcOpts) => { await deps.runNodesCommand("canvas snapshot", async () => { const format = parseCanvasSnapshotRequestFormat(opts.format); - const maxWidth = opts.maxWidth ? Number.parseInt(opts.maxWidth, 10) : undefined; - const quality = opts.quality ? Number.parseFloat(opts.quality) : undefined; + const maxWidth = parseCanvasPositiveIntOption(opts.maxWidth, "--max-width"); + const quality = parseCanvasFiniteNumberOption(opts.quality, "--quality"); const raw = await invokeCanvas(deps, opts, "canvas.snapshot", { format, maxWidth: Number.isFinite(maxWidth) ? maxWidth : undefined, @@ -287,10 +307,10 @@ export function registerNodesCanvasCommands(nodes: Command, deps: CanvasCliDepen .action(async (opts: CanvasNodesRpcOpts) => { await deps.runNodesCommand("canvas present", async () => { const placement = { - x: opts.x ? Number.parseFloat(opts.x) : undefined, - y: opts.y ? Number.parseFloat(opts.y) : undefined, - width: opts.width ? Number.parseFloat(opts.width) : undefined, - height: opts.height ? Number.parseFloat(opts.height) : undefined, + x: parseCanvasFiniteNumberOption(opts.x, "--x"), + y: parseCanvasFiniteNumberOption(opts.y, "--y"), + width: parseCanvasFiniteNumberOption(opts.width, "--width"), + height: parseCanvasFiniteNumberOption(opts.height, "--height"), }; const params: Record = {}; if (opts.target) { diff --git a/src/cli/logs-cli.test.ts b/src/cli/logs-cli.test.ts index f26f92a9dc9..b93c4b310ca 100644 --- a/src/cli/logs-cli.test.ts +++ b/src/cli/logs-cli.test.ts @@ -193,6 +193,17 @@ describe("logs cli", () => { ); }); + it.each([ + ["--limit", "10x"], + ["--max-bytes", "250kb"], + ["--interval", "1s"], + ])("rejects partial numeric %s values", async (flag, value) => { + await expect(runLogsCli(["logs", flag, value])).rejects.toThrow( + `${flag} must be a positive integer.`, + ); + expect(callGatewayFromCli).not.toHaveBeenCalled(); + }); + it("keeps explicit Gateway URLs on the normal CLI client identity", async () => { callGatewayFromCli.mockResolvedValueOnce({ file: "/tmp/openclaw.log", @@ -418,10 +429,9 @@ describe("logs cli", () => { readSystemdServiceRuntime.mockResolvedValue({ status: "running", pid: 2557 }); execFileUtf8Tail .mockResolvedValueOnce({ - stdout: [ - "Authorization: Bearer sk-abcdefghijklmnopqrstuvwxyz", - "-- cursor: s=abc", - ].join("\n"), + stdout: ["Authorization: Bearer sk-abcdefghijklmnopqrstuvwxyz", "-- cursor: s=abc"].join( + "\n", + ), stderr: "", code: 0, truncated: false, diff --git a/src/cli/logs-cli.ts b/src/cli/logs-cli.ts index ba3a2ed9cde..0ed3f0dc244 100644 --- a/src/cli/logs-cli.ts +++ b/src/cli/logs-cli.ts @@ -10,6 +10,7 @@ import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } from "../gateway/protocol/ import { readConnectPairingRequiredMessage } from "../gateway/protocol/connect-error-details.js"; import { computeBackoff } from "../infra/backoff.js"; import { formatErrorMessage } from "../infra/errors.js"; +import { parseStrictPositiveInteger } from "../infra/parse-finite-number.js"; import { readConfiguredLogTail } from "../logging/log-tail.js"; import { parseLogLine } from "../logging/parse-log-line.js"; import { redactSensitiveLines, resolveRedactOptions } from "../logging/redact.js"; @@ -81,21 +82,24 @@ const JOURNAL_CURSOR_PREFIX = "-- cursor: "; const JOURNAL_MAX_LIMIT = 5000; const JOURNAL_MAX_BYTES = 1_000_000; -function parsePositiveInt(value: string | undefined, fallback: number): number { +function parsePositiveInt(value: string | undefined, fallback: number, flag: string): number { if (!value) { return fallback; } - const parsed = Number.parseInt(value, 10); - return Number.isFinite(parsed) && parsed > 0 ? parsed : fallback; + const parsed = parseStrictPositiveInteger(value); + if (parsed === undefined) { + throw new Error(`${flag} must be a positive integer.`); + } + return parsed; } async function fetchLogs( opts: LogsCliOptions, cursors: LogCursorState, showProgress: boolean, + params: { limit: number; maxBytes: number }, ): Promise { - const limit = parsePositiveInt(opts.limit, 200); - const maxBytes = parsePositiveInt(opts.maxBytes, 250_000); + const { limit, maxBytes } = params; if (cursors.forceJournal) { const journalPayload = await readSystemdJournalFallback({ cursor: cursors.journal, @@ -493,7 +497,9 @@ export function registerLogsCli(program: Command) { logs.action(async (opts: LogsCliOptions) => { const { logLine, errorLine, emitJsonLine } = createLogWriters(); - const interval = parsePositiveInt(opts.interval, 1000); + const interval = parsePositiveInt(opts.interval, 1000, "--interval"); + const limit = parsePositiveInt(opts.limit, 200, "--limit"); + const maxBytes = parsePositiveInt(opts.maxBytes, 250_000, "--max-bytes"); let gatewayCursor: number | undefined; let journalCursor: string | undefined; let journalSince: string | undefined; @@ -515,6 +521,7 @@ export function registerLogsCli(program: Command) { opts, { gateway: gatewayCursor, journal: journalCursor, journalSince, forceJournal }, showProgress, + { limit, maxBytes }, ); } catch (err) { if (err instanceof JournalFallbackUnavailableError) { diff --git a/src/cli/program/register.onboard.test.ts b/src/cli/program/register.onboard.test.ts index b52519e0b06..a5387193f4d 100644 --- a/src/cli/program/register.onboard.test.ts +++ b/src/cli/program/register.onboard.test.ts @@ -102,6 +102,12 @@ describe("registerOnboardCommand", () => { await runCli(["onboard", "--gateway-port", "nope"]); expect(setupWizardOptions(1).gatewayPort).toBeUndefined(); + + await runCli(["onboard", "--gateway-port", "18789x"]); + expect(setupWizardOptions(2).gatewayPort).toBeUndefined(); + + await runCli(["onboard", "--gateway-port", "99999"]); + expect(setupWizardOptions(3).gatewayPort).toBeUndefined(); }); it("forwards --reset-scope to setup wizard options", async () => { diff --git a/src/cli/program/register.onboard.ts b/src/cli/program/register.onboard.ts index 346b8fdec14..191170de31a 100644 --- a/src/cli/program/register.onboard.ts +++ b/src/cli/program/register.onboard.ts @@ -15,6 +15,7 @@ import { resolveManifestProviderOnboardAuthFlags } from "../../plugins/provider- import { formatDocsLink } from "../../terminal/links.js"; import { theme } from "../../terminal/theme.js"; import { runCommandWithRuntime } from "../cli-utils.js"; +import { parsePort } from "../shared/parse-port.js"; function resolveInstallDaemonFlag( command: unknown, @@ -191,8 +192,7 @@ export function registerOnboardCommand(program: Command): void { const installDaemon = resolveInstallDaemonFlag(commandRuntime, { installDaemon: Boolean(opts.installDaemon), }); - const gatewayPort = - typeof opts.gatewayPort === "string" ? Number.parseInt(opts.gatewayPort, 10) : undefined; + const gatewayPort = parsePort(opts.gatewayPort); const providerAuthOptionValues = pickOnboardProviderAuthOptionValues( opts as Record, ); @@ -224,10 +224,7 @@ export function registerOnboardCommand(program: Command): void { : opts.customImageInput === true ? true : undefined, - gatewayPort: - typeof gatewayPort === "number" && Number.isFinite(gatewayPort) - ? gatewayPort - : undefined, + gatewayPort: gatewayPort ?? undefined, gatewayBind: opts.gatewayBind as GatewayBind | undefined, gatewayAuth: opts.gatewayAuth as GatewayAuthChoice | undefined, gatewayToken: opts.gatewayToken as string | undefined, diff --git a/src/commands/channels.add.test.ts b/src/commands/channels.add.test.ts index 3568abe7934..89643d1e420 100644 --- a/src/commands/channels.add.test.ts +++ b/src/commands/channels.add.test.ts @@ -880,6 +880,41 @@ describe("channelsAddCommand", () => { expect(runtime.exit).not.toHaveBeenCalled(); }); + it("drops malformed numeric channel setup options before plugin setup", async () => { + const applyAccountConfig = vi.fn(({ cfg, input }: ApplyAccountConfigParams) => ({ + ...cfg, + channels: { + ...cfg.channels, + matrix: { + enabled: true, + initialSyncLimit: input.initialSyncLimit, + }, + }, + })); + const plugin = { + ...createChannelTestPluginBase({ id: "matrix", label: "Matrix" }), + setup: { applyAccountConfig }, + }; + configMocks.readConfigFileSnapshot.mockResolvedValue({ ...baseConfigSnapshot }); + setActivePluginRegistry(createTestRegistry([{ pluginId: "matrix", plugin, source: "test" }])); + + await channelsAddCommand( + { + channel: "matrix", + initialSyncLimit: "10x", + }, + runtime, + { hasFlags: true }, + ); + + expect(applyAccountConfig).toHaveBeenCalledTimes(1); + expect( + (applyAccountConfig.mock.calls[0]?.[0] as ApplyAccountConfigParams | undefined)?.input + .initialSyncLimit, + ).toBeUndefined(); + expect(writtenChannel("matrix").initialSyncLimit).toBeUndefined(); + }); + it("falls back from untrusted workspace catalog shadows when adding by alias", async () => { configMocks.readConfigFileSnapshot.mockResolvedValue({ ...baseConfigSnapshot }); setActivePluginRegistry(createTestRegistry()); diff --git a/src/commands/channels/add.ts b/src/commands/channels/add.ts index b533056320a..eb2a6743e71 100644 --- a/src/commands/channels/add.ts +++ b/src/commands/channels/add.ts @@ -14,6 +14,7 @@ import { import { commitConfigWithPendingPluginInstalls } from "../../cli/plugins-install-record-commit.js"; import { refreshPluginRegistryAfterConfigMutation } from "../../cli/plugins-registry-refresh.js"; import type { OpenClawConfig } from "../../config/config.js"; +import { parseStrictNonNegativeInteger } from "../../infra/parse-finite-number.js"; import { DEFAULT_ACCOUNT_ID, normalizeAccountId } from "../../routing/session-key.js"; import { defaultRuntime, type RuntimeEnv } from "../../runtime.js"; import { createLazyImportLoader } from "../../shared/lazy-promise.js"; @@ -80,13 +81,7 @@ async function resolveCatalogChannelEntry(raw: string, cfg: OpenClawConfig | nul } function parseOptionalInt(value: unknown): number | undefined { - if (typeof value === "number") { - return value; - } - if (typeof value === "string" && value.trim()) { - return Number.parseInt(value, 10); - } - return undefined; + return parseStrictNonNegativeInteger(value); } function parseOptionalDelimitedInput(value: unknown): string[] | undefined {