fix(cli): validate gateway-rpc --timeout and cron --timeout-seconds

ruanrrn · vincentkoc · ruanrrn · commit 4dadf7e3b890 · 2026-06-01T21:42:12.000+08:00
Reject malformed explicit gateway RPC timeout values while preserving the 30000 ms omitted-option default. Keep cron agent timeout validation from silently dropping invalid inputs. - Add resolveGatewayRpcTimeoutMs() to gateway-rpc.ts for strict validation - Reject non-positive-integer --timeout values instead of silently falling back - Validate --timeout-seconds in cron add CLI (checks option source) - Add gateway-rpc.timeout-option.test.ts with edge case coverage - Carry forward #54646 with adaptations for current main Co-Authored-By: ProjectClownfish <vincentkoc@users.noreply.github.com>
diff --git a/src/cli/cron-cli/register.cron-add.ts b/src/cli/cron-cli/register.cron-add.ts
@@ -161,6 +161,12 @@ export function registerCronAddCommand(cron: Command) {
                 ? (name: string) => cmd.getOptionValueSource(name)
                 : () => undefined;
 
+            const timeoutSecondsRaw = opts.timeoutSeconds;
+            const timeoutSecondsCli = parsePositiveIntOrUndefined(timeoutSecondsRaw);
+            if (optionSource("timeoutSeconds") === "cli" && timeoutSecondsCli === undefined) {
+              throw new Error("Invalid --timeout-seconds (must be a positive integer).");
+            }
+
             const hasAnnounce = Boolean(opts.announce) || opts.deliver === true;
             const hasNoDeliver = opts.deliver === false;
             const webhookUrl = normalizeOptionalString(opts.webhook);
diff --git a/src/cli/gateway-rpc.timeout-option.test.ts b/src/cli/gateway-rpc.timeout-option.test.ts
@@ -0,0 +1,53 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+const mocks = vi.hoisted(() => {
+  return {
+    callGateway: vi.fn(async () => ({ ok: true })),
+  };
+});
+
+vi.mock("../gateway/call.js", () => ({
+  callGateway: mocks.callGateway,
+}));
+
+vi.mock("./progress.js", () => ({
+  withProgress: async (_opts: unknown, action: () => Promise<unknown>) => await action(),
+}));
+
+const { callGatewayFromCli } = await import("./gateway-rpc.js");
+
+describe("gateway-rpc timeout option", () => {
+  beforeEach(() => {
+    mocks.callGateway.mockClear();
+  });
+
+  it("passes a parsed timeoutMs to callGateway", async () => {
+    await callGatewayFromCli("health", { timeout: "1234", json: true }, {});
+
+    expect(mocks.callGateway).toHaveBeenCalledWith(
+      expect.objectContaining({
+        timeoutMs: 1234,
+      }),
+    );
+  });
+
+  it("defaults to 30s when timeout is missing", async () => {
+    await callGatewayFromCli("health", { json: true }, {});
+
+    expect(mocks.callGateway).toHaveBeenCalledWith(
+      expect.objectContaining({
+        timeoutMs: 30_000,
+      }),
+    );
+  });
+
+  it("rejects invalid timeout values instead of silently falling back", async () => {
+    for (const timeout of ["", "   ", "nope", "10ms", "1000abc", "1e3", "1.5"]) {
+      mocks.callGateway.mockClear();
+      await expect(callGatewayFromCli("health", { timeout, json: true }, {})).rejects.toThrow(
+        "--timeout must be a positive integer",
+      );
+      expect(mocks.callGateway).not.toHaveBeenCalled();
+    }
+  });
+});
diff --git a/src/cli/gateway-rpc.ts b/src/cli/gateway-rpc.ts
@@ -7,6 +7,7 @@ import type { OperatorScope } from "../gateway/operator-scopes.js";
 import type { DeviceIdentity } from "../infra/device-identity.js";
 import { createLazyImportLoader } from "../shared/lazy-promise.js";
 import type { GatewayRpcOpts } from "./gateway-rpc.types.js";
+import { parsePositiveIntOrUndefined } from "./program/helpers.js";
 export type { GatewayRpcOpts } from "./gateway-rpc.types.js";
 
 type GatewayRpcRuntimeModule = typeof import("./gateway-rpc.runtime.js");
@@ -27,6 +28,22 @@ export function addGatewayClientOptions(cmd: Command) {
     .option("--expect-final", "Wait for final response (agent)", false);
 }
 
+const DEFAULT_GATEWAY_RPC_TIMEOUT_MS = 30_000;
+
+function resolveGatewayRpcTimeoutMs(timeout: unknown): number {
+  if (timeout === undefined || timeout === null) {
+    return DEFAULT_GATEWAY_RPC_TIMEOUT_MS;
+  }
+  if (typeof timeout === "string" && timeout.trim() === "") {
+    throw new Error("--timeout must be a positive integer (milliseconds)");
+  }
+  const parsed = parsePositiveIntOrUndefined(timeout);
+  if (parsed === undefined) {
+    throw new Error("--timeout must be a positive integer (milliseconds)");
+  }
+  return parsed;
+}
+
 export async function callGatewayFromCli(
   method: string,
   opts: GatewayRpcOpts,
@@ -40,6 +57,8 @@ export async function callGatewayFromCli(
     scopes?: OperatorScope[];
   },
 ) {
+  const timeoutMs = resolveGatewayRpcTimeoutMs(opts.timeout);
+  opts.timeout = String(timeoutMs);
   const runtime = await loadGatewayRpcRuntime();
   return await runtime.callGatewayFromCliRuntime(method, opts, params, extra);
 }
