Skip to content

Commit 99d0bdc

Browse files
openclaw-clownfish[bot]ruanrrncomeran
authored
fix(cli): validate gateway RPC timeout inputs
Reject malformed or explicit empty Gateway RPC timeout values before opening Gateway calls, align the shared Gateway RPC omitted-timeout fallback with the 30000 ms CLI default, and validate explicit `cron add --timeout-seconds` values at the CLI boundary. Carries forward the useful source work from #54646 and the earlier timeout-validation context from #40953. #60661 remains separate accepted-run timeout semantics work and is intentionally not folded into this change. Validation: - `npm run review-results -- /tmp/clownfish-check-27341769444` - `git diff --check` - OpenClaw PR checks on `ce7bd8b9388a5689b14ddc2b3a984f7b4647e5ca`: 132 pass, 0 pending, 0 failing - ClawSweeper re-review: https://github.com/openclaw/clawsweeper/actions/runs/27344244608 Co-authored-by: RayRuan <[email protected]> Co-authored-by: Homeran <[email protected]>
1 parent 6fb0c94 commit 99d0bdc

10 files changed

Lines changed: 96 additions & 8 deletions

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -631,6 +631,27 @@ describe("cron cli", () => {
631631
expect(params?.delivery?.mode).toBe("none");
632632
});
633633

634+
it.each(["", "0", "-1", "1.5", "1000ms"])(
635+
"rejects invalid cron add --timeout-seconds value %j",
636+
async (timeoutSeconds) => {
637+
await expectCronCommandExit([
638+
"cron",
639+
"add",
640+
"--name",
641+
"Invalid timeout",
642+
"--cron",
643+
"* * * * *",
644+
"--message",
645+
"hello",
646+
"--timeout-seconds",
647+
timeoutSeconds,
648+
]);
649+
650+
expectRuntimeErrorContaining("Invalid --timeout-seconds (must be a positive integer).");
651+
expect(callGatewayFromCli.mock.calls.some((call) => call[0] === "cron.add")).toBe(false);
652+
},
653+
);
654+
634655
it("rejects cron add with both message and command payloads", async () => {
635656
await expectCronCommandExit([
636657
"cron",

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@ import { sanitizeAgentId } from "../../routing/session-key.js";
1010
import { defaultRuntime } from "../../runtime.js";
1111
import type { GatewayRpcOpts } from "../gateway-rpc.js";
1212
import { addGatewayClientOptions, callGatewayFromCli } from "../gateway-rpc.js";
13-
import { parsePositiveIntOrUndefined } from "../program/helpers.js";
13+
import {
14+
parsePositiveIntOrUndefined,
15+
parseStrictPositiveIntOrUndefined,
16+
} from "../program/helpers.js";
1417
import { resolveCronCreateScheduleFromArgs } from "./schedule-options.js";
1518
import {
1619
getCronChannelOptions,
@@ -217,7 +220,10 @@ export function registerCronAddCommand(cron: Command) {
217220
if (systemEvent) {
218221
return { kind: "systemEvent" as const, text: systemEvent };
219222
}
220-
const timeoutSeconds = parsePositiveIntOrUndefined(opts.timeoutSeconds);
223+
const timeoutSeconds = parseStrictPositiveIntOrUndefined(opts.timeoutSeconds);
224+
if (opts.timeoutSeconds !== undefined && timeoutSeconds === undefined) {
225+
throw new Error("Invalid --timeout-seconds (must be a positive integer).");
226+
}
221227
if (commandShell || commandArgv) {
222228
const rawNoOutputTimeoutSeconds =
223229
opts.noOutputTimeoutSeconds ??

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,15 @@ describe("gateway-cli coverage", () => {
159159
expect(runtimeLogs.join("\n")).toContain('"ok": true');
160160
});
161161

162+
it("rejects invalid gateway call timeout before calling Gateway", async () => {
163+
callGateway.mockClear();
164+
165+
await expectGatewayExit(["gateway", "call", "health", "--timeout", "1000ms", "--json"]);
166+
167+
expect(callGateway).not.toHaveBeenCalled();
168+
expect(runtimeErrors.join("\n")).toContain("Gateway call failed: Error: Invalid --timeout");
169+
});
170+
162171
it("registers gateway probe and routes to gatewayStatusCommand", async () => {
163172
gatewayStatusCommand.mockClear();
164173

‎src/cli/gateway-cli/call.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,11 @@ export const gatewayCallOpts = (cmd: Command) =>
3030
.option("--expect-final", "Wait for final response (agent)", false)
3131
.option("--json", "Output JSON", false);
3232

33-
export const callGatewayCli = async (method: string, opts: GatewayRpcOpts, params?: unknown) =>
34-
withProgress(
33+
export const callGatewayCli = async (method: string, opts: GatewayRpcOpts, params?: unknown) => {
34+
const timeoutMs = parseTimeoutMsWithFallback(opts.timeout, DEFAULT_GATEWAY_RPC_TIMEOUT_MS, {
35+
invalidType: "error",
36+
});
37+
return await withProgress(
3538
{
3639
label: `Gateway ${method}`,
3740
indeterminate: true,
@@ -46,8 +49,9 @@ export const callGatewayCli = async (method: string, opts: GatewayRpcOpts, param
4649
method,
4750
params,
4851
expectFinal: Boolean(opts.expectFinal),
49-
timeoutMs: parseTimeoutMsWithFallback(opts.timeout, DEFAULT_GATEWAY_RPC_TIMEOUT_MS),
52+
timeoutMs,
5053
clientName: GATEWAY_CLIENT_NAMES.CLI,
5154
mode: GATEWAY_CLIENT_MODES.CLI,
5255
}),
5356
);
57+
};

‎src/cli/gateway-rpc.runtime.test.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,17 @@ describe("callGatewayFromCliRuntime", () => {
1717
callGatewayMock.mockClear().mockResolvedValue({ ok: true });
1818
});
1919

20+
it("uses the 30s Gateway RPC default timeout when --timeout is omitted", async () => {
21+
await callGatewayFromCliRuntime("cron.status", {});
22+
23+
expect(callGatewayMock).toHaveBeenCalledWith(
24+
expect.objectContaining({
25+
method: "cron.status",
26+
timeoutMs: 30_000,
27+
}),
28+
);
29+
});
30+
2031
it.each([
2132
["cron status", "cron.status"],
2233
["cron list", "cron.list"],
@@ -36,6 +47,14 @@ describe("callGatewayFromCliRuntime", () => {
3647
expect(callGatewayMock).not.toHaveBeenCalled();
3748
});
3849

50+
it.each(["", " "])("rejects explicit empty shared --timeout value %j", async (timeout) => {
51+
await expect(callGatewayFromCliRuntime("cron.status", { timeout })).rejects.toThrow(
52+
"Invalid --timeout",
53+
);
54+
55+
expect(callGatewayMock).not.toHaveBeenCalled();
56+
});
57+
3958
it.each(["0", "-1", "1.5"])("rejects invalid shared --timeout value %j", async (timeout) => {
4059
await expect(callGatewayFromCliRuntime("cron.status", { timeout })).rejects.toThrow(
4160
`Received: "${timeout}"`,

‎src/cli/gateway-rpc.runtime.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ type CallGatewayFromCliRuntimeExtra = {
1717
scopes?: Parameters<typeof callGateway>[0]["scopes"];
1818
};
1919

20-
const DEFAULT_GATEWAY_RPC_TIMEOUT_MS = 10_000;
20+
const DEFAULT_GATEWAY_RPC_TIMEOUT_MS = 30_000;
2121

2222
export async function callGatewayFromCliRuntime(
2323
method: string,
@@ -27,6 +27,9 @@ export async function callGatewayFromCliRuntime(
2727
) {
2828
// Progress is disabled for JSON output so stdout stays parseable.
2929
const showProgress = extra?.progress ?? opts.json !== true;
30+
const timeoutMs = parseTimeoutMsWithFallback(opts.timeout, DEFAULT_GATEWAY_RPC_TIMEOUT_MS, {
31+
invalidType: "error",
32+
});
3033
return await withProgress(
3134
{
3235
label: `Gateway ${method}`,
@@ -42,7 +45,7 @@ export async function callGatewayFromCliRuntime(
4245
deviceIdentity: extra?.deviceIdentity,
4346
expectFinal: extra?.expectFinal ?? Boolean(opts.expectFinal),
4447
scopes: extra?.scopes,
45-
timeoutMs: parseTimeoutMsWithFallback(opts.timeout, DEFAULT_GATEWAY_RPC_TIMEOUT_MS),
48+
timeoutMs,
4649
clientName: extra?.clientName ?? GATEWAY_CLIENT_NAMES.CLI,
4750
mode: extra?.mode ?? GATEWAY_CLIENT_MODES.CLI,
4851
}),

‎src/cli/parse-timeout.test.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,12 @@ describe("parseTimeoutMsWithFallback", () => {
4343
);
4444
});
4545

46+
it("throws on empty values when requested", () => {
47+
expect(() => parseTimeoutMsWithFallback(" ", 3000, { invalidType: "error" })).toThrow(
48+
"Invalid --timeout",
49+
);
50+
});
51+
4652
it("throws on non-positive parsed values", () => {
4753
expect(() => parseTimeoutMsWithFallback("0", 3000)).toThrow('Received: "0"');
4854
expect(() => parseTimeoutMsWithFallback("-1", 3000)).toThrow('Received: "-1"');
@@ -56,4 +62,8 @@ describe("parseTimeoutMsWithFallback", () => {
5662
"Received",
5763
);
5864
});
65+
66+
it("throws on partial-numeric values", () => {
67+
expect(() => parseTimeoutMsWithFallback("1000ms", 3000)).toThrow('Received: "1000ms"');
68+
});
5969
});

‎src/cli/parse-timeout.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,9 @@ export function parseTimeoutMsWithFallback(
5555
}
5656

5757
if (!value) {
58+
if (options.invalidType === "error") {
59+
throw invalidTimeout();
60+
}
5861
return fallbackMs;
5962
}
6063

‎src/commands/channels.status.command-flow.test.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,4 +352,15 @@ describe("channelsStatusCommand SecretRef fallback flow", () => {
352352
expect(payload.configOnly).toBe(true);
353353
expect(payload.configuredChannels).toStrictEqual([]);
354354
});
355+
356+
it("rejects invalid timeout before falling back to config-only status", async () => {
357+
const { runtime } = createCapturingTestRuntime();
358+
359+
await expect(channelsStatusCommand({ timeout: "1000ms" }, runtime as never)).rejects.toThrow(
360+
'Received: "1000ms"',
361+
);
362+
363+
expect(mocks.callGateway).not.toHaveBeenCalled();
364+
expect(mocks.requireValidConfigSnapshot).not.toHaveBeenCalled();
365+
});
355366
});

‎src/commands/channels/status.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,9 @@ export async function channelsStatusCommand(
218218
opts: ChannelsStatusOptions,
219219
runtime: RuntimeEnv = defaultRuntime,
220220
) {
221-
const timeoutMs = parseTimeoutMsWithFallback(opts.timeout, opts.probe ? 30_000 : 10_000);
221+
const timeoutMs = parseTimeoutMsWithFallback(opts.timeout, opts.probe ? 30_000 : 10_000, {
222+
invalidType: "error",
223+
});
222224
const requestedChannel = opts.channel ? normalizeChannelId(opts.channel) : null;
223225
const statusLabel = opts.probe ? "Checking channel status (probe)…" : "Checking channel status…";
224226
const shouldLogStatus = opts.json !== true && !process.stderr.isTTY;

0 commit comments

Comments
 (0)