fix(cli): reject malformed timeout values

This commit is contained in:
Peter Steinberger
2026-05-27 13:04:39 -04:00
parent de5971eedc
commit 5f7e21e26a
12 changed files with 195 additions and 42 deletions
+47 -1
View File
@@ -1926,10 +1926,56 @@ describe("capability cli", () => {
argv: ["capability", "image", "generate", "--prompt", "portrait", "--timeout-ms", "1000ms"],
}),
).rejects.toThrow("exit 1");
expectRuntimeErrorContains("--timeout-ms must be a finite number");
expectRuntimeErrorContains("Invalid --timeout. Use a positive millisecond value");
expect(mocks.generateImage).not.toHaveBeenCalled();
});
it.each([
[
"image generate",
["capability", "image", "generate", "--prompt", "portrait", "--timeout-ms", "1000ms"],
],
[
"image edit",
[
"capability",
"image",
"edit",
"--file",
"photo.png",
"--prompt",
"crop it",
"--timeout-ms",
"1000ms",
],
],
[
"image describe",
["capability", "image", "describe", "--file", "photo.png", "--timeout-ms", "1000ms"],
],
[
"image describe-many",
["capability", "image", "describe-many", "--file", "photo.png", "--timeout-ms", "1000ms"],
],
[
"video generate",
["capability", "video", "generate", "--prompt", "clip", "--timeout-ms", "1000ms"],
],
])("rejects malformed %s timeout before provider dispatch", async (_name, argv) => {
await expect(
runRegisteredCli({
register: registerCapabilityCli as (program: Command) => void,
argv,
}),
).rejects.toThrow("exit 1");
expectRuntimeErrorContains("Invalid --timeout. Use a positive millisecond value");
expect(mocks.generateImage).not.toHaveBeenCalled();
expect(mocks.generateVideo).not.toHaveBeenCalled();
expect(mocks.describeImageFile).not.toHaveBeenCalled();
expect(mocks.describeImageFileWithModel).not.toHaveBeenCalled();
});
it("routes audio transcribe through transcription, not realtime", async () => {
await runRegisteredCli({
register: registerCapabilityCli as (program: Command) => void,
+13 -5
View File
@@ -106,6 +106,7 @@ import {
getModelsCommandSecretTargetIds,
getTtsCommandSecretTargetIds,
} from "./command-secret-targets.js";
import { parseTimeoutMsWithFallback } from "./parse-timeout.js";
import { removeCommandByName } from "./program/command-tree.js";
import { collectOption } from "./program/helpers.js";
@@ -1179,6 +1180,13 @@ function parseOptionalPositiveInteger(raw: unknown, label: string): number | und
return value;
}
function parseOptionalTimeoutMs(raw: string | number | undefined): number | undefined {
if (raw === undefined || (typeof raw === "string" && raw.trim() === "")) {
return undefined;
}
return parseTimeoutMsWithFallback(raw, 0, { invalidType: "error" });
}
function normalizeImageOutputFormat(
raw: string | undefined,
): ImageGenerationOutputFormat | undefined {
@@ -1961,7 +1969,7 @@ export function registerCapabilityCli(program: Command) {
opts.openaiBackground as string | undefined,
"--openai-background",
),
timeoutMs: parseOptionalFiniteNumber(opts.timeoutMs, "--timeout-ms"),
timeoutMs: parseOptionalTimeoutMs(opts.timeoutMs),
output: opts.output as string | undefined,
});
emitJsonOrText(defaultRuntime, Boolean(opts.json), result, formatEnvelopeForText);
@@ -2000,7 +2008,7 @@ export function registerCapabilityCli(program: Command) {
opts.openaiBackground as string | undefined,
"--openai-background",
),
timeoutMs: parseOptionalFiniteNumber(opts.timeoutMs, "--timeout-ms"),
timeoutMs: parseOptionalTimeoutMs(opts.timeoutMs),
output: opts.output as string | undefined,
});
emitJsonOrText(defaultRuntime, Boolean(opts.json), result, formatEnvelopeForText);
@@ -2022,7 +2030,7 @@ export function registerCapabilityCli(program: Command) {
files: [String(opts.file)],
model: opts.model as string | undefined,
prompt: opts.prompt as string | undefined,
timeoutMs: parseOptionalFiniteNumber(opts.timeoutMs, "--timeout-ms"),
timeoutMs: parseOptionalTimeoutMs(opts.timeoutMs),
});
emitJsonOrText(defaultRuntime, Boolean(opts.json), result, formatEnvelopeForText);
});
@@ -2043,7 +2051,7 @@ export function registerCapabilityCli(program: Command) {
files: opts.file as string[],
model: opts.model as string | undefined,
prompt: opts.prompt as string | undefined,
timeoutMs: parseOptionalFiniteNumber(opts.timeoutMs, "--timeout-ms"),
timeoutMs: parseOptionalTimeoutMs(opts.timeoutMs),
});
emitJsonOrText(defaultRuntime, Boolean(opts.json), result, formatEnvelopeForText);
});
@@ -2352,7 +2360,7 @@ export function registerCapabilityCli(program: Command) {
durationSeconds: parseOptionalFiniteNumber(opts.duration, "--duration"),
audio: opts.audio === true ? true : undefined,
watermark: opts.watermark === true ? true : undefined,
timeoutMs: parseOptionalFiniteNumber(opts.timeoutMs, "--timeout-ms"),
timeoutMs: parseOptionalTimeoutMs(opts.timeoutMs),
});
emitJsonOrText(defaultRuntime, Boolean(opts.json), result, formatEnvelopeForText);
});
+58
View File
@@ -79,6 +79,64 @@ describe("native hook relay CLI", () => {
expect(stderr.text()).toBe("err");
});
it("rejects malformed timeouts before reading relay input", async () => {
const invokeBridge = vi.fn();
const callGateway = vi.fn();
const stdout = createWritableTextBuffer();
const stderr = createWritableTextBuffer();
const exitCode = await runNativeHookRelayCli(
{
provider: "codex",
relayId: "relay-1",
generation: "generation-1",
event: "pre_tool_use",
timeout: "5000ms",
},
{
stdin: createReadableTextStream("{}"),
stdout,
stderr,
invokeBridge: invokeBridge as never,
callGateway: callGateway as never,
},
);
expect(exitCode).toBe(1);
expect(stdout.text()).toBe("");
expect(stderr.text()).toContain("invalid native hook timeout");
expect(stderr.text()).toContain('Received: "5000ms"');
expect(invokeBridge).not.toHaveBeenCalled();
expect(callGateway).not.toHaveBeenCalled();
});
it("rejects fractional timeouts before gateway fallback", async () => {
const invokeBridge = vi.fn();
const callGateway = vi.fn();
const stderr = createWritableTextBuffer();
const exitCode = await runNativeHookRelayCli(
{
provider: "codex",
relayId: "relay-1",
generation: "generation-1",
event: "pre_tool_use",
timeout: "1.5",
},
{
stdin: createReadableTextStream("{}"),
stderr,
invokeBridge: invokeBridge as never,
callGateway: callGateway as never,
},
);
expect(exitCode).toBe(1);
expect(stderr.text()).toContain('Received: "1.5"');
expect(invokeBridge).not.toHaveBeenCalled();
expect(callGateway).not.toHaveBeenCalled();
});
it("renders unavailable output for legacy relay commands without a generation", async () => {
const invokeBridge = vi.fn(async () => {
throw new Error("generation must be non-empty string");
+10 -7
View File
@@ -7,6 +7,7 @@ import {
} from "../agents/harness/native-hook-relay.js";
import { callGateway } from "../gateway/call.js";
import { ADMIN_SCOPE } from "../gateway/method-scopes.js";
import { parseTimeoutMsWithFallback } from "./parse-timeout.js";
const MAX_NATIVE_HOOK_STDIN_BYTES = 1024 * 1024;
@@ -39,6 +40,13 @@ export async function runNativeHookRelayCli(
const relayId = readRequiredOption(opts.relayId, "relay-id");
const generation = opts.generation?.trim() || undefined;
const event = readRequiredOption(opts.event, "event");
let timeoutMs: number;
try {
timeoutMs = parseTimeoutMsWithFallback(opts.timeout, 5_000);
} catch (error) {
writeText(stderr, formatRelayCliError("invalid native hook timeout", error));
return 1;
}
let rawPayload: unknown;
try {
@@ -57,7 +65,7 @@ export async function runNativeHookRelayCli(
event,
rawPayload,
registrationTimeoutMs: 100,
timeoutMs: normalizeTimeoutMs(opts.timeout),
timeoutMs,
});
writeText(stdout, response.stdout);
writeText(stderr, response.stderr);
@@ -82,7 +90,7 @@ export async function runNativeHookRelayCli(
const response = await callGatewayFn<NativeHookRelayProcessResponse>({
method: "nativeHook.invoke",
params: { provider, relayId, generation, event, rawPayload },
timeoutMs: normalizeTimeoutMs(opts.timeout),
timeoutMs,
scopes: [ADMIN_SCOPE],
});
writeText(stdout, response.stdout);
@@ -122,11 +130,6 @@ async function readStreamText(stream: NodeJS.ReadableStream, maxBytes: number):
return Buffer.concat(chunks, total).toString("utf8");
}
function normalizeTimeoutMs(value: string | undefined): number {
const parsed = Number(value);
return Number.isFinite(parsed) && parsed > 0 ? Math.floor(parsed) : 5_000;
}
function writeText(stream: NodeJS.WritableStream, value: string | undefined): void {
if (value) {
stream.write(value);
+3 -2
View File
@@ -20,12 +20,13 @@ describe("program helpers", () => {
{ value: null, expected: undefined },
{ value: "", expected: undefined },
{ value: 5, expected: 5 },
{ value: 5.9, expected: 5 },
{ value: 5.9, expected: undefined },
{ value: 0, expected: undefined },
{ value: -1, expected: undefined },
{ value: Number.NaN, expected: undefined },
{ value: "10", expected: 10 },
{ value: "10ms", expected: 10 },
{ value: "10ms", expected: undefined },
{ value: "1.5", expected: undefined },
{ value: "0", expected: undefined },
{ value: "nope", expected: undefined },
{ value: true, expected: undefined },
+1 -15
View File
@@ -9,21 +9,7 @@ export function parsePositiveIntOrUndefined(value: unknown): number | undefined
if (value === undefined || value === null || value === "") {
return undefined;
}
if (typeof value === "number") {
if (!Number.isFinite(value)) {
return undefined;
}
const parsed = Math.trunc(value);
return parsed > 0 ? parsed : undefined;
}
if (typeof value === "string") {
const parsed = Number.parseInt(value, 10);
if (Number.isNaN(parsed) || parsed <= 0) {
return undefined;
}
return parsed;
}
return undefined;
return parseStrictPositiveInteger(value);
}
export function parseStrictPositiveIntOrUndefined(value: unknown): number | undefined {
@@ -227,6 +227,16 @@ describe("channelsStatusCommand SecretRef fallback flow", () => {
});
});
it("rejects malformed timeouts before gateway status requests", async () => {
const { runtime } = createCapturingTestRuntime();
await expect(
channelsStatusCommand({ channel: "imsg", timeout: "10s", probe: true }, runtime as never),
).rejects.toThrow('Received: "10s"');
expect(mocks.callGateway).not.toHaveBeenCalled();
});
it("keeps read-only fallback output when SecretRefs are unresolved", async () => {
mocks.callGateway.mockRejectedValue(new Error("gateway closed"));
mocks.requireValidConfigSnapshot.mockResolvedValue({ secretResolved: false, channels: {} });
@@ -178,6 +178,32 @@ describe("channelsCapabilitiesCommand", () => {
]);
});
it("rejects malformed timeouts before capability probes", async () => {
const probeAccount = vi.fn(async () => ({ ok: true }));
const plugin = buildPlugin({
id: "slack",
account: {
accountId: "default",
botToken: "xoxb-bot",
},
probe: { ok: true },
});
plugin.status = { ...plugin.status, probeAccount };
vi.mocked(listChannelPlugins).mockReturnValue([plugin]);
vi.mocked(getChannelPlugin).mockReturnValue(plugin);
mocks.resolveInstallableChannelPlugin.mockResolvedValue({
cfg: { channels: {} },
channelId: "slack",
plugin,
configChanged: false,
});
await expect(
channelsCapabilitiesCommand({ channel: "slack", timeout: "10s" }, runtime),
).rejects.toThrow('Received: "10s"');
expect(probeAccount).not.toHaveBeenCalled();
});
it("prints Teams Graph permission hints when present", async () => {
const plugin = buildPlugin({
id: "msteams",
+2 -9
View File
@@ -12,6 +12,7 @@ import type {
} from "../../channels/plugins/types.public.js";
import { formatCliCommand } from "../../cli/command-format.js";
import { formatUnknownChannelMessage } from "../../cli/error-format.js";
import { parseTimeoutMsWithFallback } from "../../cli/parse-timeout.js";
import { commitConfigWithPendingPluginInstalls } from "../../cli/plugins-install-record-commit.js";
import { refreshPluginRegistryAfterConfigMutation } from "../../cli/plugins-registry-refresh.js";
import {
@@ -51,14 +52,6 @@ type ChannelCapabilitiesReport = {
diagnostics?: ChannelCapabilitiesDiagnostics;
};
function normalizeTimeout(raw: unknown, fallback = 10_000) {
const value = typeof raw === "string" ? Number(raw) : Number(raw);
if (!Number.isFinite(value) || value <= 0) {
return fallback;
}
return value;
}
function formatSupport(capabilities?: ChannelCapabilities) {
if (!capabilities) {
return "unknown";
@@ -226,7 +219,7 @@ export async function channelsCapabilitiesCommand(
return;
}
let cfg = loadedCfg;
const timeoutMs = normalizeTimeout(opts.timeout, 10_000);
const timeoutMs = parseTimeoutMsWithFallback(opts.timeout, 10_000);
const rawChannel = normalizeLowercaseStringOrEmpty(opts.channel);
const rawTarget = normalizeOptionalString(opts.target) ?? "";
+2 -1
View File
@@ -2,6 +2,7 @@ import { normalizeChannelId } from "../../channels/plugins/index.js";
import { resolveCommandConfigWithSecrets } from "../../cli/command-config-resolution.js";
import { formatCliCommand } from "../../cli/command-format.js";
import { getConfiguredChannelsCommandSecretTargetIds } from "../../cli/command-secret-targets.js";
import { parseTimeoutMsWithFallback } from "../../cli/parse-timeout.js";
import { withProgress } from "../../cli/progress.js";
import { readConfigFileSnapshot } from "../../config/config.js";
import { callGateway } from "../../gateway/call.js";
@@ -214,7 +215,7 @@ export async function channelsStatusCommand(
opts: ChannelsStatusOptions,
runtime: RuntimeEnv = defaultRuntime,
) {
const timeoutMs = Number(opts.timeout ?? (opts.probe ? 30_000 : 10_000));
const timeoutMs = parseTimeoutMsWithFallback(opts.timeout, opts.probe ? 30_000 : 10_000);
const requestedChannel = opts.channel ? normalizeChannelId(opts.channel) : null;
const statusLabel = opts.probe ? "Checking channel status (probe)…" : "Checking channel status…";
const shouldLogStatus = opts.json !== true && !process.stderr.isTTY;
+20
View File
@@ -353,6 +353,26 @@ describe("sessionsCommand", () => {
fs.rmSync(store);
});
it("rejects partial --active values", async () => {
const store = writeStore(
{
demo: {
sessionId: "demo",
updatedAt: Date.now() - 5 * 60_000,
},
},
"sessions-active-partial",
);
const { runtime, errors } = makeRuntime();
await expect(sessionsCommand({ store, active: "10m" }, runtime)).rejects.toThrow("exit 1");
expect(errors).toStrictEqual([
"--active must be a positive number of minutes, for example --active 30.",
]);
fs.rmSync(store);
});
it("rejects invalid --limit values", async () => {
const store = writeStore(
{
+3 -2
View File
@@ -7,6 +7,7 @@ import { loadSessionStore, resolveSessionTotalTokens } from "../config/sessions.
import type { SessionEntry } from "../config/sessions/types.js";
import type { OpenClawConfig } from "../config/types.openclaw.js";
import { info } from "../globals.js";
import { parseStrictPositiveInteger } from "../infra/parse-finite-number.js";
import { parseAgentSessionKey } from "../routing/session-key.js";
import { type RuntimeEnv, writeRuntimeJson } from "../runtime.js";
import { classifySessionKind, type SessionKind } from "../sessions/classify-session-kind.js";
@@ -329,8 +330,8 @@ export async function sessionsCommand(
let activeMinutes: number | undefined;
if (opts.active !== undefined) {
const parsed = Number.parseInt(opts.active, 10);
if (Number.isNaN(parsed) || parsed <= 0) {
const parsed = parseStrictPositiveInteger(opts.active);
if (parsed === undefined) {
runtime.error("--active must be a positive number of minutes, for example --active 30.");
runtime.exit(1);
return;