mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-26 12:26:38 -06:00
fix(cli): reject malformed usage-cost --days instead of silently defaulting (#127978)
* fix(cli): reject malformed usage-cost --days instead of silently defaulting openclaw gateway usage-cost --days abc (or 7d, 1e3, 1.5) silently queried 30 days of cost data, printing a valid-looking report for the wrong window. The same operator-input class was fixed for backup git log --limit in #127875 (reject, don't default), and the shared gateway --timeout flag already throws 'Invalid --timeout' for malformed values via parseTimeoutMsWithFallback in callGatewayFromCliRuntime, so --days was the last lenient outlier. parseDaysOption now throws on a present-but-unparseable value; the error surfaces through the existing runGatewayCommand failure path. Also collapse the duplicate lenient timeout parser in the health auth-diagnostic path onto the canonical parseTimeoutMsWithFallback so both --timeout sites share one policy (no reachable behavior change there; the RPC path rejects first). Regression tests fail pre-fix: --days abc/7d/1.5 called usage.cost with days:30 instead of rejecting. The pre-existing 'falls back for non-decimal --days' test pinned the lenient behavior and is updated to the strict contract. * fix(cli): reject explicit empty usage-cost --days values too An explicit empty or whitespace-only --days still reached the silent fallback because the throw only ran for non-empty strings. Treat any string value that fails strict parsing as operator error, per review on #127978. * test(cli): preserve usage-cost JSON validation --------- Co-authored-by: Altay <altay@hey.com>
This commit is contained in:
committed by
GitHub
parent
8c088944ef
commit
8d05d6c98e
@@ -341,6 +341,35 @@ describe("gateway-cli coverage", () => {
|
||||
expect(runtimeErrors).toHaveLength(0);
|
||||
});
|
||||
|
||||
it.each(["abc", "7d", "1.5", "", " "])(
|
||||
"rejects malformed usage-cost --days %j",
|
||||
async (days) => {
|
||||
callGateway.mockClear();
|
||||
|
||||
await expectGatewayExit(["gateway", "usage-cost", "--days", days, "--json"]);
|
||||
|
||||
expect(callGateway).not.toHaveBeenCalled();
|
||||
expect(defaultRuntime.writeJson).toHaveBeenCalledWith({
|
||||
ok: false,
|
||||
error: { type: "cli_error", message: expect.stringContaining("Invalid --days") },
|
||||
});
|
||||
expect(runtimeErrors).toHaveLength(0);
|
||||
},
|
||||
);
|
||||
|
||||
it("rejects a malformed health --timeout before calling Gateway", async () => {
|
||||
callGateway.mockClear();
|
||||
|
||||
await expectGatewayExit(["gateway", "health", "--timeout", "abc", "--json"]);
|
||||
|
||||
expect(callGateway).not.toHaveBeenCalled();
|
||||
expect(defaultRuntime.writeJson).toHaveBeenCalledWith({
|
||||
ok: false,
|
||||
error: { type: "cli_error", message: expect.stringContaining("Invalid --timeout") },
|
||||
});
|
||||
expect(runtimeErrors).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("writes JSON for gateway health transport failures in JSON mode", async () => {
|
||||
const error = new Error("gateway closed (1006)");
|
||||
const payload = {
|
||||
|
||||
@@ -353,21 +353,25 @@ describe("gateway register option collisions", () => {
|
||||
expectLocalGatewayCall("diagnostics.stability", 19095, { limit: 25 });
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "falls back for non-decimal usage-cost --days values",
|
||||
argv: ["gateway", "usage-cost", "--days", "1e3", "--json"],
|
||||
assert: () => {
|
||||
expect(callGatewayCli).toHaveBeenCalledTimes(1);
|
||||
const [method, _opts, params] = firstGatewayCall();
|
||||
expect(method).toBe("usage.cost");
|
||||
expect(params).toEqual({ days: 30 });
|
||||
},
|
||||
},
|
||||
])("$name", async ({ argv, assert }) => {
|
||||
await sharedProgram.parseAsync(argv, { from: "user" });
|
||||
assert();
|
||||
});
|
||||
|
||||
it("rejects non-decimal usage-cost --days values instead of silently defaulting", async () => {
|
||||
await sharedProgram.parseAsync(["gateway", "usage-cost", "--days", "1e3", "--json"], {
|
||||
from: "user",
|
||||
});
|
||||
|
||||
expect(callGatewayCli).not.toHaveBeenCalled();
|
||||
expect(defaultRuntime.writeJson).toHaveBeenCalledWith({
|
||||
ok: false,
|
||||
error: { type: "cli_error", message: expect.stringContaining("Invalid --days") },
|
||||
});
|
||||
expect(defaultRuntime.error).not.toHaveBeenCalled();
|
||||
expect(defaultRuntime.exit).toHaveBeenCalledWith(1);
|
||||
});
|
||||
|
||||
it.each([
|
||||
{
|
||||
name: "call",
|
||||
@@ -396,21 +400,22 @@ describe("gateway register option collisions", () => {
|
||||
expect(defaultRuntime.exit).toHaveBeenCalledWith(1);
|
||||
});
|
||||
|
||||
it("uses the effective local port override for gateway health auth diagnostics", async () => {
|
||||
it("uses the effective local port and timeout for gateway health auth diagnostics", async () => {
|
||||
const authError = new Error("gateway auth required");
|
||||
callGatewayCli.mockRejectedValueOnce(authError);
|
||||
emitReachableGatewayAuthDiagnostic.mockResolvedValueOnce(true);
|
||||
|
||||
await sharedProgram.parseAsync(["gateway", "health", "--port", "19081", "--json"], {
|
||||
from: "user",
|
||||
});
|
||||
await sharedProgram.parseAsync(
|
||||
["gateway", "health", "--port", "19081", "--timeout", "1234", "--json"],
|
||||
{ from: "user" },
|
||||
);
|
||||
|
||||
expect(emitReachableGatewayAuthDiagnostic).toHaveBeenCalledTimes(1);
|
||||
expect(emitReachableGatewayAuthDiagnostic).toHaveBeenCalledWith({
|
||||
error: authError,
|
||||
config: {},
|
||||
runtime: defaultRuntime,
|
||||
timeoutMs: 10000,
|
||||
timeoutMs: 1234,
|
||||
token: undefined,
|
||||
password: undefined,
|
||||
localPortOverride: 19081,
|
||||
|
||||
@@ -24,6 +24,7 @@ import { formatCliJsonFailure, rethrowExpectedCliError } from "../failure-output
|
||||
import { parseGatewayPortOption } from "../gateway-port-option.js";
|
||||
import { addGatewayClientOptions, callGatewayFromCliWithTransport } from "../gateway-rpc.js";
|
||||
import { formatHelpExamples } from "../help-format.js";
|
||||
import { parseTimeoutMsWithFallback } from "../parse-timeout.js";
|
||||
import { setCommandJsonMode } from "../program/json-mode.js";
|
||||
import type { GatewayDiscoverOpts } from "./discover.js";
|
||||
import { isGatewayMachineOutput } from "./output-mode.js";
|
||||
@@ -165,24 +166,15 @@ function parseDaysOption(raw: unknown, fallback = 30): number {
|
||||
if (typeof raw === "number" && Number.isFinite(raw)) {
|
||||
return Math.max(1, Math.floor(raw));
|
||||
}
|
||||
if (typeof raw === "string" && raw.trim() !== "") {
|
||||
const parsed = parseStrictPositiveInteger(raw);
|
||||
if (parsed !== undefined) {
|
||||
return parsed;
|
||||
}
|
||||
}
|
||||
return fallback;
|
||||
}
|
||||
|
||||
function parseGatewayRpcTimeoutOption(raw: unknown, fallback = 10_000): number {
|
||||
if (typeof raw === "number" && Number.isFinite(raw) && raw > 0) {
|
||||
return Math.floor(raw);
|
||||
}
|
||||
if (typeof raw === "string" && raw.trim() !== "") {
|
||||
if (typeof raw === "string") {
|
||||
const parsed = parseStrictPositiveInteger(raw);
|
||||
if (parsed !== undefined) {
|
||||
return parsed;
|
||||
}
|
||||
// A present-but-unparseable value (including an explicit empty one) is
|
||||
// operator error; the main RPC path rejects malformed --timeout the same
|
||||
// way instead of silently defaulting.
|
||||
throw new Error(`Invalid --days. Use a positive integer, e.g. --days 30. Received: "${raw}".`);
|
||||
}
|
||||
return fallback;
|
||||
}
|
||||
@@ -699,7 +691,9 @@ export function registerGatewayCli(program: Command, deps: GatewayCliDependencie
|
||||
error,
|
||||
config: rpcOpts.config ?? (await readNonObservingHealthConfig()),
|
||||
runtime: defaultRuntime,
|
||||
timeoutMs: parseGatewayRpcTimeoutOption(rpcOpts.timeout),
|
||||
timeoutMs: parseTimeoutMsWithFallback(rpcOpts.timeout, 10_000, {
|
||||
invalidType: "error",
|
||||
}),
|
||||
token: rpcOpts.token,
|
||||
password: rpcOpts.password,
|
||||
localPortOverride: rpcOpts.localPortOverride,
|
||||
|
||||
Reference in New Issue
Block a user