Skip to content

Commit 625b793

Browse files
steipetesteipete
committed
fix(daemon): centralize tcp port bounds
1 parent aa53823 commit 625b793

8 files changed

Lines changed: 142 additions & 49 deletions

File tree

‎src/cli/shared/parse-port.ts‎

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,7 @@
1-
import { parseStrictPositiveInteger } from "../../infra/parse-finite-number.js";
1+
import { parseTcpPort } from "../../infra/tcp-port.js";
22

3-
// TCP/UDP ports are 16-bit, so 65535 is the max. `parseStrictPositiveInteger`
4-
// only enforces positivity, so values like 99999 were returned as-is and
5-
// reached gateway-cli / node-cli bind paths; the OS then surfaced the error
6-
// instead of the CLI rejecting it cleanly at parse time. See #83900.
7-
const MAX_TCP_PORT = 65_535;
3+
export { MAX_TCP_PORT, parseTcpPort } from "../../infra/tcp-port.js";
84

95
export function parsePort(raw: unknown): number | null {
10-
if (raw === undefined || raw === null) {
11-
return null;
12-
}
13-
const parsed = parseStrictPositiveInteger(raw);
14-
if (parsed === undefined || parsed > MAX_TCP_PORT) {
15-
return null;
16-
}
17-
return parsed;
6+
return parseTcpPort(raw);
187
}

‎src/daemon/launchd.test.ts‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1319,6 +1319,22 @@ describe("launchd install", () => {
13191319
expect(cleanStaleGatewayProcessesSync).toHaveBeenCalledWith(19001);
13201320
});
13211321

1322+
it("ignores invalid configured gateway ports for stale cleanup", async () => {
1323+
const env = {
1324+
...createDefaultLaunchdEnv(),
1325+
OPENCLAW_GATEWAY_PORT: "65536",
1326+
};
1327+
state.files.clear();
1328+
1329+
await restartLaunchAgent({
1330+
env,
1331+
stdout: new PassThrough(),
1332+
});
1333+
1334+
expect(cleanStaleGatewayProcessesSync).not.toHaveBeenCalled();
1335+
expect(inspectPortUsage).not.toHaveBeenCalled();
1336+
});
1337+
13221338
it("uses the stored LaunchAgent environment port for restart stale cleanup", async () => {
13231339
const env = createDefaultLaunchdEnv();
13241340
await installLaunchAgent({
@@ -1338,6 +1354,25 @@ describe("launchd install", () => {
13381354
expect(inspectPortUsage).toHaveBeenCalledWith(19007);
13391355
});
13401356

1357+
it("ignores invalid stored LaunchAgent environment ports for stale cleanup", async () => {
1358+
const env = createDefaultLaunchdEnv();
1359+
await installLaunchAgent({
1360+
env,
1361+
stdout: new PassThrough(),
1362+
programArguments: defaultProgramArguments,
1363+
environment: { OPENCLAW_GATEWAY_PORT: "65536" },
1364+
});
1365+
state.launchctlCalls.length = 0;
1366+
1367+
await restartLaunchAgent({
1368+
env,
1369+
stdout: new PassThrough(),
1370+
});
1371+
1372+
expect(cleanStaleGatewayProcessesSync).not.toHaveBeenCalled();
1373+
expect(inspectPortUsage).not.toHaveBeenCalled();
1374+
});
1375+
13411376
it("fails restart before kickstart when the configured gateway port remains busy", async () => {
13421377
const env = {
13431378
...createDefaultLaunchdEnv(),

‎src/daemon/launchd.ts‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { normalizeEnvVarKey } from "../infra/host-env-security.js";
44
import { parseStrictInteger, parseStrictPositiveInteger } from "../infra/parse-finite-number.js";
55
import { formatPortDiagnostics, inspectPortUsage } from "../infra/ports.js";
66
import { cleanStaleGatewayProcessesSync } from "../infra/restart-stale-pids.js";
7+
import { parseTcpPort } from "../infra/tcp-port.js";
78
import { normalizeLowercaseStringOrEmpty } from "../shared/string-coerce.js";
89
import { sanitizeForLog } from "../terminal/ansi.js";
910
import {
@@ -368,15 +369,15 @@ function parseGatewayPortFromProgramArguments(
368369
continue;
369370
}
370371
if (current === "--port") {
371-
const next = parseStrictPositiveInteger(programArguments[index + 1] ?? "");
372-
if (next !== undefined) {
372+
const next = parseTcpPort(programArguments[index + 1] ?? "");
373+
if (next !== null) {
373374
return next;
374375
}
375376
continue;
376377
}
377378
if (current.startsWith("--port=")) {
378-
const value = parseStrictPositiveInteger(current.slice("--port=".length));
379-
if (value !== undefined) {
379+
const value = parseTcpPort(current.slice("--port=".length));
380+
if (value !== null) {
380381
return value;
381382
}
382383
}
@@ -390,14 +391,11 @@ async function resolveLaunchAgentGatewayPort(env: GatewayServiceEnv): Promise<nu
390391
if (fromArgs !== null) {
391392
return fromArgs;
392393
}
393-
const fromServiceEnv = parseStrictPositiveInteger(
394-
command?.environment?.OPENCLAW_GATEWAY_PORT ?? "",
395-
);
396-
if (fromServiceEnv !== undefined) {
394+
const fromServiceEnv = parseTcpPort(command?.environment?.OPENCLAW_GATEWAY_PORT ?? "");
395+
if (fromServiceEnv !== null) {
397396
return fromServiceEnv;
398397
}
399-
const fromEnv = parseStrictPositiveInteger(env.OPENCLAW_GATEWAY_PORT ?? "");
400-
return fromEnv ?? null;
398+
return parseTcpPort(env.OPENCLAW_GATEWAY_PORT ?? "");
401399
}
402400

403401
function resolveGuiDomain(): string {

‎src/daemon/schtasks.ts‎

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ import os from "node:os";
44
import path from "node:path";
55
import { isGatewayArgv } from "../infra/gateway-process-argv.js";
66
import { findVerifiedGatewayListenerPidsOnPortSync } from "../infra/gateway-processes.js";
7-
import { parseStrictPositiveInteger } from "../infra/parse-finite-number.js";
87
import { inspectPortUsage } from "../infra/ports.js";
8+
import { parseTcpPort } from "../infra/tcp-port.js";
99
import { getWindowsInstallRoots } from "../infra/windows-install-roots.js";
1010
import { killProcessTree } from "../process/kill-tree.js";
1111
import { normalizeLowercaseStringOrEmpty } from "../shared/string-coerce.js";
@@ -500,23 +500,11 @@ async function launchFallbackTaskScript(env: GatewayServiceEnv): Promise<void> {
500500
}
501501

502502
function resolveConfiguredGatewayPort(env: GatewayServiceEnv): number | null {
503-
const raw = env.OPENCLAW_GATEWAY_PORT?.trim();
504-
if (!raw) {
505-
return null;
506-
}
507-
return parseStrictPositiveInteger(raw) ?? null;
503+
return parseTcpPort(env.OPENCLAW_GATEWAY_PORT);
508504
}
509505

510506
function parsePositivePort(raw: string | undefined): number | null {
511-
const value = raw?.trim();
512-
if (!value) {
513-
return null;
514-
}
515-
if (!/^\d+$/.test(value)) {
516-
return null;
517-
}
518-
const parsed = parseStrictPositiveInteger(value);
519-
return parsed !== undefined && parsed <= 65535 ? parsed : null;
507+
return parseTcpPort(raw);
520508
}
521509

522510
function parsePortFromProgramArguments(programArguments?: string[]): number | null {

‎src/daemon/service-audit.test.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,9 @@ describe("auditGatewayServiceConfig", () => {
353353
expect(
354354
readGatewayServiceCommandPort(["/usr/bin/node", "entry.js", "gateway", "--port=0"]),
355355
).toBe(undefined);
356+
expect(
357+
readGatewayServiceCommandPort(["/usr/bin/node", "entry.js", "gateway", "--port=65536"]),
358+
).toBe(undefined);
356359
});
357360

358361
it("flags gateway service port drift from the expected config port", async () => {
@@ -377,6 +380,28 @@ describe("auditGatewayServiceConfig", () => {
377380
});
378381
});
379382

383+
it("flags explicit invalid gateway service ports", async () => {
384+
const audit = await auditGatewayServiceConfig({
385+
env: { HOME: "/tmp" },
386+
platform: "win32",
387+
expectedPort: 18888,
388+
command: {
389+
programArguments: ["/usr/bin/node", "entry.js", "gateway", "--port=65536"],
390+
environment: {},
391+
},
392+
});
393+
394+
const issue = audit.issues.find(
395+
(entry) => entry.code === SERVICE_AUDIT_CODES.gatewayPortMismatch,
396+
);
397+
expect(issue).toStrictEqual({
398+
code: SERVICE_AUDIT_CODES.gatewayPortMismatch,
399+
message: "Gateway service port does not match current gateway config.",
400+
detail: "65536 -> 18888",
401+
level: "recommended",
402+
});
403+
});
404+
380405
it("accepts gateway service ports that match the expected config port", async () => {
381406
const audit = await auditGatewayServiceConfig({
382407
env: { HOME: "/tmp" },

‎src/daemon/service-audit.ts‎

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import fs from "node:fs/promises";
22
import path from "node:path";
33
import { normalizeEnvVarKey } from "../infra/host-env-security.js";
4-
import { parseStrictPositiveInteger } from "../infra/parse-finite-number.js";
4+
import { parseTcpPort } from "../infra/tcp-port.js";
55
import {
66
normalizeLowercaseStringOrEmpty,
77
normalizeOptionalString,
@@ -249,14 +249,25 @@ function auditGatewayCommand(programArguments: string[] | undefined, issues: Ser
249249
}
250250
}
251251

252-
function parseGatewayPortArg(value: string | undefined): number | undefined {
253-
const parsed = parseStrictPositiveInteger(value ?? "");
254-
return parsed !== undefined && parsed <= 65535 ? parsed : undefined;
252+
type GatewayServiceCommandPort =
253+
| { kind: "missing" }
254+
| { kind: "valid"; port: number }
255+
| { kind: "invalid"; raw: string };
256+
257+
function parseGatewayPortArg(value: string | undefined): GatewayServiceCommandPort {
258+
const raw = value?.trim() ?? "";
259+
const port = parseTcpPort(raw);
260+
if (port !== null) {
261+
return { kind: "valid", port };
262+
}
263+
return raw ? { kind: "invalid", raw } : { kind: "missing" };
255264
}
256265

257-
export function readGatewayServiceCommandPort(programArguments?: string[]): number | undefined {
266+
function readGatewayServiceCommandPortState(
267+
programArguments?: string[],
268+
): GatewayServiceCommandPort {
258269
if (!programArguments || programArguments.length === 0) {
259-
return undefined;
270+
return { kind: "missing" };
260271
}
261272
for (let index = 0; index < programArguments.length; index += 1) {
262273
const arg = programArguments[index];
@@ -267,7 +278,12 @@ export function readGatewayServiceCommandPort(programArguments?: string[]): numb
267278
return parseGatewayPortArg(arg.slice("--port=".length));
268279
}
269280
}
270-
return undefined;
281+
return { kind: "missing" };
282+
}
283+
284+
export function readGatewayServiceCommandPort(programArguments?: string[]): number | undefined {
285+
const servicePort = readGatewayServiceCommandPortState(programArguments);
286+
return servicePort.kind === "valid" ? servicePort.port : undefined;
271287
}
272288

273289
function auditGatewayServicePort(params: {
@@ -283,14 +299,21 @@ function auditGatewayServicePort(params: {
283299
) {
284300
return;
285301
}
286-
const servicePort = readGatewayServiceCommandPort(params.programArguments);
287-
if (servicePort === undefined || servicePort === params.expectedPort) {
302+
const servicePort = readGatewayServiceCommandPortState(params.programArguments);
303+
if (servicePort.kind === "missing") {
304+
return;
305+
}
306+
if (servicePort.kind === "valid" && servicePort.port === params.expectedPort) {
288307
return;
289308
}
309+
const detail =
310+
servicePort.kind === "valid"
311+
? `${servicePort.port} -> ${params.expectedPort}`
312+
: `${servicePort.raw} -> ${params.expectedPort}`;
290313
params.issues.push({
291314
code: SERVICE_AUDIT_CODES.gatewayPortMismatch,
292315
message: "Gateway service port does not match current gateway config.",
293-
detail: `${servicePort} -> ${params.expectedPort}`,
316+
detail,
294317
level: "recommended",
295318
});
296319
}

‎src/infra/tcp-port.test.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
import { describe, expect, it } from "vitest";
2+
import { parseTcpPort } from "./tcp-port.js";
3+
4+
describe("parseTcpPort", () => {
5+
it("accepts valid TCP port values", () => {
6+
expect(parseTcpPort(1)).toBe(1);
7+
expect(parseTcpPort("8080")).toBe(8080);
8+
expect(parseTcpPort(" 65535 ")).toBe(65_535);
9+
});
10+
11+
it("rejects invalid TCP port values", () => {
12+
expect(parseTcpPort(undefined)).toBeNull();
13+
expect(parseTcpPort(null)).toBeNull();
14+
expect(parseTcpPort(0)).toBeNull();
15+
expect(parseTcpPort(-1)).toBeNull();
16+
expect(parseTcpPort(65_536)).toBeNull();
17+
expect(parseTcpPort("100000")).toBeNull();
18+
expect(parseTcpPort("8080ms")).toBeNull();
19+
expect(parseTcpPort("1.5")).toBeNull();
20+
});
21+
});

‎src/infra/tcp-port.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import { parseStrictPositiveInteger } from "./parse-finite-number.js";
2+
3+
export const MAX_TCP_PORT = 65_535;
4+
5+
export function parseTcpPort(raw: unknown): number | null {
6+
if (raw === undefined || raw === null) {
7+
return null;
8+
}
9+
const parsed = parseStrictPositiveInteger(raw);
10+
if (parsed === undefined || parsed > MAX_TCP_PORT) {
11+
return null;
12+
}
13+
return parsed;
14+
}

0 commit comments

Comments
 (0)