From 2646739d23346945c047f4304af7bdb0f7f29d2f Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 8 Mar 2026 02:52:44 +0000 Subject: [PATCH] refactor: centralize strict numeric parsing --- .../mattermost/src/mattermost/monitor.ts | 6 ++-- src/cli/daemon-cli/status.gather.ts | 4 +-- src/cli/shared/parse-port.ts | 17 ++------- src/config/cache-utils.test.ts | 14 ++++++++ src/config/cache-utils.ts | 5 +-- src/daemon/launchd.test.ts | 13 +++++++ src/daemon/launchd.ts | 9 ++--- src/daemon/systemd.test.ts | 15 ++++++++ src/daemon/systemd.ts | 9 ++--- src/infra/parse-finite-number.test.ts | 36 ++++++++++++++++++- src/infra/parse-finite-number.ts | 30 ++++++++++++++++ 11 files changed, 128 insertions(+), 30 deletions(-) create mode 100644 src/config/cache-utils.test.ts diff --git a/extensions/mattermost/src/mattermost/monitor.ts b/extensions/mattermost/src/mattermost/monitor.ts index f603ee2bb..a7b692c2c 100644 --- a/extensions/mattermost/src/mattermost/monitor.ts +++ b/extensions/mattermost/src/mattermost/monitor.ts @@ -30,6 +30,7 @@ import { listSkillCommandsForAgents, type HistoryEntry, } from "openclaw/plugin-sdk/mattermost"; +import { parseStrictPositiveInteger } from "../../../../src/infra/parse-finite-number.js"; import { getMattermostRuntime } from "../runtime.js"; import { resolveMattermostAccount } from "./accounts.js"; import { @@ -348,9 +349,8 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {} // The gateway sets OPENCLAW_GATEWAY_PORT when it boots, but the config file may still contain // a different port. const envPortRaw = process.env.OPENCLAW_GATEWAY_PORT?.trim(); - const envPort = envPortRaw ? Number.parseInt(envPortRaw, 10) : NaN; - const slashGatewayPort = - Number.isFinite(envPort) && envPort > 0 ? envPort : (cfg.gateway?.port ?? 18789); + const envPort = parseStrictPositiveInteger(envPortRaw); + const slashGatewayPort = envPort ?? cfg.gateway?.port ?? 18789; const slashCallbackUrl = resolveCallbackUrl({ config: slashConfig, diff --git a/src/cli/daemon-cli/status.gather.ts b/src/cli/daemon-cli/status.gather.ts index 6834f379b..4fc517c34 100644 --- a/src/cli/daemon-cli/status.gather.ts +++ b/src/cli/daemon-cli/status.gather.ts @@ -27,6 +27,7 @@ import { } from "../../gateway/credentials.js"; import { resolveGatewayBindHost } from "../../gateway/net.js"; import { resolveRequiredConfiguredSecretRefInputString } from "../../gateway/resolve-configured-secret-input-string.js"; +import { parseStrictPositiveInteger } from "../../infra/parse-finite-number.js"; import { formatPortDiagnostics, inspectPortUsage, @@ -308,8 +309,7 @@ export async function gatherDaemonStatus( { deep: Boolean(opts.deep) }, ).catch(() => []); - const timeoutMsRaw = Number.parseInt(String(opts.rpc.timeout ?? "10000"), 10); - const timeoutMs = Number.isFinite(timeoutMsRaw) && timeoutMsRaw > 0 ? timeoutMsRaw : 10_000; + const timeoutMs = parseStrictPositiveInteger(opts.rpc.timeout ?? "10000") ?? 10_000; const tlsEnabled = daemonCfg.gateway?.tls?.enabled === true; const shouldUseLocalTlsRuntime = opts.probe && !probeUrlOverride && tlsEnabled; diff --git a/src/cli/shared/parse-port.ts b/src/cli/shared/parse-port.ts index 003fb9ea3..9b8c7a7c2 100644 --- a/src/cli/shared/parse-port.ts +++ b/src/cli/shared/parse-port.ts @@ -1,19 +1,8 @@ +import { parseStrictPositiveInteger } from "../../infra/parse-finite-number.js"; + export function parsePort(raw: unknown): number | null { if (raw === undefined || raw === null) { return null; } - const value = - typeof raw === "string" - ? raw - : typeof raw === "number" || typeof raw === "bigint" - ? raw.toString() - : null; - if (value === null) { - return null; - } - const parsed = Number.parseInt(value, 10); - if (!Number.isFinite(parsed) || parsed <= 0) { - return null; - } - return parsed; + return parseStrictPositiveInteger(raw) ?? null; } diff --git a/src/config/cache-utils.test.ts b/src/config/cache-utils.test.ts new file mode 100644 index 000000000..d21d5d687 --- /dev/null +++ b/src/config/cache-utils.test.ts @@ -0,0 +1,14 @@ +import { describe, expect, it } from "vitest"; +import { resolveCacheTtlMs } from "./cache-utils.js"; + +describe("resolveCacheTtlMs", () => { + it("accepts exact non-negative integers", () => { + expect(resolveCacheTtlMs({ envValue: "0", defaultTtlMs: 60_000 })).toBe(0); + expect(resolveCacheTtlMs({ envValue: "120000", defaultTtlMs: 60_000 })).toBe(120_000); + }); + + it("rejects malformed env values and falls back to the default", () => { + expect(resolveCacheTtlMs({ envValue: "0abc", defaultTtlMs: 60_000 })).toBe(60_000); + expect(resolveCacheTtlMs({ envValue: "15ms", defaultTtlMs: 60_000 })).toBe(60_000); + }); +}); diff --git a/src/config/cache-utils.ts b/src/config/cache-utils.ts index e0024c098..f13cd7a77 100644 --- a/src/config/cache-utils.ts +++ b/src/config/cache-utils.ts @@ -1,4 +1,5 @@ import fs from "node:fs"; +import { parseStrictNonNegativeInteger } from "../infra/parse-finite-number.js"; export function resolveCacheTtlMs(params: { envValue: string | undefined; @@ -6,8 +7,8 @@ export function resolveCacheTtlMs(params: { }): number { const { envValue, defaultTtlMs } = params; if (envValue) { - const parsed = Number.parseInt(envValue, 10); - if (Number.isFinite(parsed) && parsed >= 0) { + const parsed = parseStrictNonNegativeInteger(envValue); + if (parsed !== undefined) { return parsed; } } diff --git a/src/daemon/launchd.test.ts b/src/daemon/launchd.test.ts index d1c808b2a..3030b6ffc 100644 --- a/src/daemon/launchd.test.ts +++ b/src/daemon/launchd.test.ts @@ -122,6 +122,19 @@ describe("launchd runtime parsing", () => { expect(info.pid).toBeUndefined(); expect(info.state).toBe("waiting"); }); + + it("rejects pid and exit status values with junk suffixes", () => { + const output = [ + "state = waiting", + "pid = 123abc", + "last exit status = 7ms", + "last exit reason = exited", + ].join("\n"); + expect(parseLaunchctlPrint(output)).toEqual({ + state: "waiting", + lastExitReason: "exited", + }); + }); }); describe("launchctl list detection", () => { diff --git a/src/daemon/launchd.ts b/src/daemon/launchd.ts index b859e9ad4..5b62fad98 100644 --- a/src/daemon/launchd.ts +++ b/src/daemon/launchd.ts @@ -1,5 +1,6 @@ import fs from "node:fs/promises"; import path from "node:path"; +import { parseStrictInteger, parseStrictPositiveInteger } from "../infra/parse-finite-number.js"; import { GATEWAY_LAUNCH_AGENT_LABEL, resolveGatewayServiceDescription, @@ -127,15 +128,15 @@ export function parseLaunchctlPrint(output: string): LaunchctlPrintInfo { } const pidValue = entries.pid; if (pidValue) { - const pid = Number.parseInt(pidValue, 10); - if (Number.isFinite(pid) && pid > 0) { + const pid = parseStrictPositiveInteger(pidValue); + if (pid !== undefined) { info.pid = pid; } } const exitStatusValue = entries["last exit status"]; if (exitStatusValue) { - const status = Number.parseInt(exitStatusValue, 10); - if (Number.isFinite(status)) { + const status = parseStrictInteger(exitStatusValue); + if (status !== undefined) { info.lastExitStatus = status; } } diff --git a/src/daemon/systemd.test.ts b/src/daemon/systemd.test.ts index 485b2fc53..1d72adaaf 100644 --- a/src/daemon/systemd.test.ts +++ b/src/daemon/systemd.test.ts @@ -351,6 +351,21 @@ describe("systemd runtime parsing", () => { execMainCode: "exited", }); }); + + it("rejects pid and exit status values with junk suffixes", () => { + const output = [ + "ActiveState=inactive", + "SubState=dead", + "MainPID=42abc", + "ExecMainStatus=2ms", + "ExecMainCode=exited", + ].join("\n"); + expect(parseSystemdShow(output)).toEqual({ + activeState: "inactive", + subState: "dead", + execMainCode: "exited", + }); + }); }); describe("resolveSystemdUserUnitPath", () => { diff --git a/src/daemon/systemd.ts b/src/daemon/systemd.ts index 22161ae3b..bce7593e2 100644 --- a/src/daemon/systemd.ts +++ b/src/daemon/systemd.ts @@ -1,6 +1,7 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; +import { parseStrictInteger, parseStrictPositiveInteger } from "../infra/parse-finite-number.js"; import { splitArgsPreservingQuotes } from "./arg-split.js"; import { LEGACY_GATEWAY_SYSTEMD_SERVICE_NAMES, @@ -231,15 +232,15 @@ export function parseSystemdShow(output: string): SystemdServiceInfo { } const mainPidValue = entries.mainpid; if (mainPidValue) { - const pid = Number.parseInt(mainPidValue, 10); - if (Number.isFinite(pid) && pid > 0) { + const pid = parseStrictPositiveInteger(mainPidValue); + if (pid !== undefined) { info.mainPid = pid; } } const execMainStatusValue = entries.execmainstatus; if (execMainStatusValue) { - const status = Number.parseInt(execMainStatusValue, 10); - if (Number.isFinite(status)) { + const status = parseStrictInteger(execMainStatusValue); + if (status !== undefined) { info.execMainStatus = status; } } diff --git a/src/infra/parse-finite-number.test.ts b/src/infra/parse-finite-number.test.ts index 8dd592b65..99b093dfe 100644 --- a/src/infra/parse-finite-number.test.ts +++ b/src/infra/parse-finite-number.test.ts @@ -1,5 +1,10 @@ import { describe, expect, it } from "vitest"; -import { parseFiniteNumber } from "./parse-finite-number.js"; +import { + parseFiniteNumber, + parseStrictInteger, + parseStrictNonNegativeInteger, + parseStrictPositiveInteger, +} from "./parse-finite-number.js"; describe("parseFiniteNumber", () => { it("returns finite numbers", () => { @@ -17,3 +22,32 @@ describe("parseFiniteNumber", () => { expect(parseFiniteNumber(null)).toBeUndefined(); }); }); + +describe("parseStrictInteger", () => { + it("parses exact integers", () => { + expect(parseStrictInteger("42")).toBe(42); + expect(parseStrictInteger(" -7 ")).toBe(-7); + }); + + it("rejects junk prefixes and suffixes", () => { + expect(parseStrictInteger("42ms")).toBeUndefined(); + expect(parseStrictInteger("0abc")).toBeUndefined(); + expect(parseStrictInteger("1.5")).toBeUndefined(); + }); +}); + +describe("parseStrictPositiveInteger", () => { + it("accepts only positive integers", () => { + expect(parseStrictPositiveInteger("9")).toBe(9); + expect(parseStrictPositiveInteger("0")).toBeUndefined(); + expect(parseStrictPositiveInteger("-1")).toBeUndefined(); + }); +}); + +describe("parseStrictNonNegativeInteger", () => { + it("accepts zero and positive integers only", () => { + expect(parseStrictNonNegativeInteger("0")).toBe(0); + expect(parseStrictNonNegativeInteger("9")).toBe(9); + expect(parseStrictNonNegativeInteger("-1")).toBeUndefined(); + }); +}); diff --git a/src/infra/parse-finite-number.ts b/src/infra/parse-finite-number.ts index cf0fa0a37..c469c91f6 100644 --- a/src/infra/parse-finite-number.ts +++ b/src/infra/parse-finite-number.ts @@ -1,3 +1,8 @@ +function normalizeNumericString(value: string): string | undefined { + const trimmed = value.trim(); + return trimmed ? trimmed : undefined; +} + export function parseFiniteNumber(value: unknown): number | undefined { if (typeof value === "number" && Number.isFinite(value)) { return value; @@ -10,3 +15,28 @@ export function parseFiniteNumber(value: unknown): number | undefined { } return undefined; } + +export function parseStrictInteger(value: unknown): number | undefined { + if (typeof value === "number") { + return Number.isSafeInteger(value) ? value : undefined; + } + if (typeof value !== "string") { + return undefined; + } + const normalized = normalizeNumericString(value); + if (!normalized || !/^[+-]?\d+$/.test(normalized)) { + return undefined; + } + const parsed = Number(normalized); + return Number.isSafeInteger(parsed) ? parsed : undefined; +} + +export function parseStrictPositiveInteger(value: unknown): number | undefined { + const parsed = parseStrictInteger(value); + return parsed !== undefined && parsed > 0 ? parsed : undefined; +} + +export function parseStrictNonNegativeInteger(value: unknown): number | undefined { + const parsed = parseStrictInteger(value); + return parsed !== undefined && parsed >= 0 ? parsed : undefined; +}