From e2c8e7c8ae65ecd4c7621a041e80da54fbdccf34 Mon Sep 17 00:00:00 2001 From: hcl Date: Tue, 19 May 2026 19:36:12 +0800 Subject: [PATCH] fix(cli): reject out-of-range port numbers in parsePort (#83900) (#84008) Summary: - The PR adds a 65,535 upper-bound check to the shared CLI `parsePort` helper, a colocated regression test, and a changelog entry for the linked port-range bug. - Reproducibility: yes. Source inspection on current main shows `parsePort('99999')` delegates to `parseStrict ... sitive safe integer, so the return would be `99999`; I did not execute it because this review is read-only. Automerge notes: - PR branch already contained follow-up commit before automerge: fix(cli): reject out-of-range port numbers in parsePort (#83900) Validation: - ClawSweeper review passed for head 9ad0705c44334dd167ac1f7ec407e61d870d86f2. - Required merge gates passed before the squash merge. Prepared head SHA: 9ad0705c44334dd167ac1f7ec407e61d870d86f2 Review: https://github.com/openclaw/openclaw/pull/84008#issuecomment-4484883200 Co-authored-by: HCL Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Co-authored-by: clawsweeper[bot] <274271284+clawsweeper[bot]@users.noreply.github.com> Approved-by: takhoffman Co-authored-by: takhoffman <781889+takhoffman@users.noreply.github.com> --- CHANGELOG.md | 1 + src/cli/shared/parse-port.test.ts | 37 +++++++++++++++++++++++++++++++ src/cli/shared/parse-port.ts | 12 +++++++++- 3 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 src/cli/shared/parse-port.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 68df6dfcba7..f827fb6721b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -70,6 +70,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- CLI: reject explicit port numbers above 65535 before they reach Gateway or Node bind paths. Fixes #83900. (#84008) Thanks @hclsys. - Codex app-server: preserve plugin tool auth profiles when Codex owns model transport so OpenClaw dynamic tools can resolve their provider credentials. (#83603) Thanks @rubencu. - Memory/search: scan the JS-side fallback vector path (used when the sqlite-vec index is unavailable or has a mismatched dimension) in bounded rowid batches and yield to the event loop between batches so large chunk tables can no longer pin the Node.js main thread for multi-second windows. Also keeps the SQL prepared statement rooted in a local so node:sqlite cannot finalize it mid-scan under heap pressure. Fixes #81172. Thanks @dev23xyz-oss. - Memory Wiki: preserve fs-safe diagnostics when bridge source page writes fail for non-symlink filesystem safety reasons, so directory collisions are reported with the underlying error code. (#83776) Thanks @TurboTheTurtle. diff --git a/src/cli/shared/parse-port.test.ts b/src/cli/shared/parse-port.test.ts new file mode 100644 index 00000000000..c33cba8bc40 --- /dev/null +++ b/src/cli/shared/parse-port.test.ts @@ -0,0 +1,37 @@ +import { describe, expect, it } from "vitest"; +import { parsePort } from "./parse-port.js"; + +describe("parsePort (#83900)", () => { + it("returns null for nullish inputs", () => { + expect(parsePort(undefined)).toBeNull(); + expect(parsePort(null)).toBeNull(); + }); + + it("returns null for zero and negative values (already enforced)", () => { + expect(parsePort(0)).toBeNull(); + expect(parsePort(-1)).toBeNull(); + expect(parsePort("0")).toBeNull(); + }); + + it("accepts valid TCP port values", () => { + expect(parsePort(1)).toBe(1); + expect(parsePort(8080)).toBe(8080); + expect(parsePort("3000")).toBe(3000); + expect(parsePort(65535)).toBe(65535); + }); + + it("rejects port numbers above 65535 (regression for #83900)", () => { + expect(parsePort(65536)).toBeNull(); + expect(parsePort(99999)).toBeNull(); + expect(parsePort("100000")).toBeNull(); + // Largest 16-bit value is the inclusive boundary. + expect(parsePort(65535)).toBe(65535); + }); + + it("rejects non-integer and non-finite inputs", () => { + expect(parsePort(1.5)).toBeNull(); + expect(parsePort(Number.NaN)).toBeNull(); + expect(parsePort(Number.POSITIVE_INFINITY)).toBeNull(); + expect(parsePort("abc")).toBeNull(); + }); +}); diff --git a/src/cli/shared/parse-port.ts b/src/cli/shared/parse-port.ts index 9b8c7a7c225..d4e332be58b 100644 --- a/src/cli/shared/parse-port.ts +++ b/src/cli/shared/parse-port.ts @@ -1,8 +1,18 @@ import { parseStrictPositiveInteger } from "../../infra/parse-finite-number.js"; +// TCP/UDP ports are 16-bit, so 65535 is the max. `parseStrictPositiveInteger` +// only enforces positivity, so values like 99999 were returned as-is and +// reached gateway-cli / node-cli bind paths; the OS then surfaced the error +// instead of the CLI rejecting it cleanly at parse time. See #83900. +const MAX_TCP_PORT = 65_535; + export function parsePort(raw: unknown): number | null { if (raw === undefined || raw === null) { return null; } - return parseStrictPositiveInteger(raw) ?? null; + const parsed = parseStrictPositiveInteger(raw); + if (parsed === undefined || parsed > MAX_TCP_PORT) { + return null; + } + return parsed; }