mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
fix(nodes): keep Gateway connections healthy and consistent (#103093)
* fix(gateway): evict unresponsive node sockets Co-authored-by: hoangsaga123 <hoangsaga123@gmail.com> * fix(nodes): align invoke timeout semantics * fix(node): preserve managed connection settings * docs(changelog): note node connection fixes * fix(node): redact service credentials in status * fix(systemd): preserve escaped operator values * fix(node): carry explicit plaintext service mode * fix(systemd): avoid promoting stale shell references * fix(node): clear fingerprints for plaintext runs * docs(changelog): defer node notes to release * fix(nodes): normalize forwarded system run timeout * fix(systemd): preserve operator environment values * fix(nodes): type normalized invoke payload --------- Co-authored-by: hoangsaga123 <hoangsaga123@gmail.com>
This commit is contained in:
committed by
GitHub
parent
b8537cd81e
commit
b4428fb9df
@@ -60,6 +60,7 @@ Options:
|
||||
- `--port <port>`: Gateway WebSocket port (default: `18789`)
|
||||
- `--context-path <path>`: Gateway WebSocket context path (e.g. `/openclaw-gw`). Appended to the WebSocket URL.
|
||||
- `--tls`: Use TLS for the gateway connection
|
||||
- `--no-tls`: Force a plaintext Gateway connection even when the local Gateway config enables TLS
|
||||
- `--tls-fingerprint <sha256>`: Expected TLS certificate fingerprint (sha256)
|
||||
- `--node-id <id>`: Override node id (clears pairing token)
|
||||
- `--display-name <name>`: Override the node display name
|
||||
|
||||
@@ -1,7 +1,8 @@
|
||||
// Node daemon tests cover node daemon command runtime behavior and errors.
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { GatewayServiceRuntime } from "../../daemon/service-runtime.js";
|
||||
import { runNodeDaemonStatus } from "./daemon.js";
|
||||
import type { GatewayServiceCommandConfig } from "../../daemon/service-types.js";
|
||||
import { runNodeDaemonInstall, runNodeDaemonStatus } from "./daemon.js";
|
||||
|
||||
const mocks = vi.hoisted(() => {
|
||||
const service = {
|
||||
@@ -14,7 +15,7 @@ const mocks = vi.hoisted(() => {
|
||||
stop: vi.fn(),
|
||||
restart: vi.fn(),
|
||||
isLoaded: vi.fn(async () => true),
|
||||
readCommand: vi.fn(async () => null),
|
||||
readCommand: vi.fn<() => Promise<GatewayServiceCommandConfig | null>>(async () => null),
|
||||
readRuntime: vi.fn<() => Promise<GatewayServiceRuntime>>(async () => ({ status: "running" })),
|
||||
};
|
||||
return {
|
||||
@@ -25,6 +26,12 @@ const mocks = vi.hoisted(() => {
|
||||
exit: vi.fn(),
|
||||
},
|
||||
service,
|
||||
buildNodeInstallPlan: vi.fn(async () => ({
|
||||
programArguments: ["node", "node-host"],
|
||||
environment: {},
|
||||
environmentValueSources: {},
|
||||
})),
|
||||
loadNodeHostConfig: vi.fn(),
|
||||
};
|
||||
});
|
||||
|
||||
@@ -36,6 +43,14 @@ vi.mock("../../daemon/node-service.js", () => ({
|
||||
resolveNodeService: () => mocks.service,
|
||||
}));
|
||||
|
||||
vi.mock("../../commands/node-daemon-install-helpers.js", () => ({
|
||||
buildNodeInstallPlan: mocks.buildNodeInstallPlan,
|
||||
}));
|
||||
|
||||
vi.mock("../../node-host/config.js", () => ({
|
||||
loadNodeHostConfig: mocks.loadNodeHostConfig,
|
||||
}));
|
||||
|
||||
vi.mock("../../daemon/runtime-hints.js", () => ({
|
||||
buildPlatformRuntimeLogHints: () => [
|
||||
"Logs: node service log",
|
||||
@@ -70,9 +85,77 @@ vi.mock("../daemon-cli/shared.js", async () => {
|
||||
}),
|
||||
formatRuntimeStatus: (runtime: GatewayServiceRuntime | undefined) => runtime?.status ?? "",
|
||||
resolveRuntimeStatusColor: () => "",
|
||||
failIfNixDaemonInstallMode: () => false,
|
||||
};
|
||||
});
|
||||
|
||||
describe("runNodeDaemonInstall", () => {
|
||||
beforeEach(() => {
|
||||
mocks.runtime.log.mockClear();
|
||||
mocks.runtime.error.mockClear();
|
||||
mocks.runtime.writeJson.mockClear();
|
||||
mocks.runtime.exit.mockClear();
|
||||
mocks.service.install.mockReset().mockResolvedValue(undefined);
|
||||
mocks.service.isLoaded.mockReset().mockResolvedValue(false);
|
||||
mocks.buildNodeInstallPlan.mockReset().mockResolvedValue({
|
||||
programArguments: ["node", "node-host"],
|
||||
environment: {},
|
||||
environmentValueSources: {},
|
||||
});
|
||||
mocks.loadNodeHostConfig.mockReset().mockResolvedValue({
|
||||
gateway: {
|
||||
host: "saved-gateway.local",
|
||||
port: 18789,
|
||||
contextPath: "/saved",
|
||||
tls: true,
|
||||
tlsFingerprint: "saved-fingerprint",
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it.each([
|
||||
["host", { host: "new-gateway.local" }],
|
||||
["port", { port: 19_001 }],
|
||||
])("does not inherit saved TLS when %s explicitly retargets the gateway", async (_name, opts) => {
|
||||
await runNodeDaemonInstall({ ...opts, force: true });
|
||||
|
||||
expect(mocks.buildNodeInstallPlan).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
tls: false,
|
||||
tlsFingerprint: undefined,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("inherits saved TLS when the gateway endpoint is unchanged", async () => {
|
||||
await runNodeDaemonInstall({ force: true });
|
||||
|
||||
expect(mocks.buildNodeInstallPlan).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
host: "saved-gateway.local",
|
||||
port: 18789,
|
||||
contextPath: "/saved",
|
||||
tls: true,
|
||||
tlsFingerprint: "saved-fingerprint",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it.each([
|
||||
["host", { host: "saved-gateway.local" }],
|
||||
["port", { port: 18_789 }],
|
||||
])("keeps saved TLS when explicit %s resolves to the saved endpoint", async (_name, opts) => {
|
||||
await runNodeDaemonInstall({ ...opts, force: true });
|
||||
|
||||
expect(mocks.buildNodeInstallPlan).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
tls: true,
|
||||
tlsFingerprint: "saved-fingerprint",
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("runNodeDaemonStatus", () => {
|
||||
function stdout(): string {
|
||||
return mocks.runtime.log.mock.calls.map(([line]) => line).join("\n");
|
||||
@@ -115,4 +198,28 @@ describe("runNodeDaemonStatus", () => {
|
||||
expect(stderr()).not.toContain("Logs: node service log");
|
||||
expect(stderr()).not.toContain("Restart attempts: node restart log");
|
||||
});
|
||||
|
||||
it("redacts service credentials from JSON status output", async () => {
|
||||
mocks.service.readCommand.mockResolvedValue({
|
||||
programArguments: ["node", "node-host"],
|
||||
environment: {
|
||||
OPENCLAW_PROFILE: "work",
|
||||
OPENCLAW_GATEWAY_TOKEN: "gateway-token",
|
||||
OPENCLAW_GATEWAY_PASSWORD: "gateway-password",
|
||||
},
|
||||
});
|
||||
|
||||
await runNodeDaemonStatus({ json: true });
|
||||
|
||||
expect(mocks.runtime.writeJson).toHaveBeenCalledWith({
|
||||
service: expect.objectContaining({
|
||||
command: expect.objectContaining({
|
||||
environment: { OPENCLAW_PROFILE: "work" },
|
||||
}),
|
||||
}),
|
||||
});
|
||||
const payload = JSON.stringify(mocks.runtime.writeJson.mock.calls[0]?.[0]);
|
||||
expect(payload).not.toContain("gateway-token");
|
||||
expect(payload).not.toContain("gateway-password");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -31,6 +31,7 @@ import {
|
||||
createCliStatusTextStyles,
|
||||
createDaemonInstallActionContext,
|
||||
failIfNixDaemonInstallMode,
|
||||
filterDaemonEnv,
|
||||
formatRuntimeStatus,
|
||||
parsePort,
|
||||
resolveRuntimeStatusColor,
|
||||
@@ -81,18 +82,23 @@ function resolveNodeDefaults(
|
||||
config: Awaited<ReturnType<typeof loadNodeHostConfig>>,
|
||||
) {
|
||||
// CLI flags override node-host config; missing values fall back to loopback Gateway defaults.
|
||||
const host = normalizeOptionalString(opts.host) || config?.gateway?.host || "127.0.0.1";
|
||||
const savedHost = config?.gateway?.host || "127.0.0.1";
|
||||
const host = normalizeOptionalString(opts.host) || savedHost;
|
||||
const retargeted = opts.host !== undefined || opts.port !== undefined;
|
||||
const portOverride = parsePort(opts.port);
|
||||
if (opts.port !== undefined && portOverride === null) {
|
||||
return { host, port: null };
|
||||
return { host, port: null, retargeted, endpointChanged: false };
|
||||
}
|
||||
const port = portOverride ?? config?.gateway?.port ?? 18789;
|
||||
const retargeted = opts.host !== undefined || opts.port !== undefined;
|
||||
const savedPort = config?.gateway?.port ?? 18789;
|
||||
const port = portOverride ?? savedPort;
|
||||
const endpointChanged =
|
||||
(opts.host !== undefined && host !== savedHost) ||
|
||||
(opts.port !== undefined && port !== savedPort);
|
||||
const explicitContextPath = opts.contextPath !== undefined;
|
||||
const contextPath =
|
||||
normalizeOptionalString(opts.contextPath) ||
|
||||
(explicitContextPath || retargeted ? undefined : config?.gateway?.contextPath);
|
||||
return { host, port, contextPath };
|
||||
return { host, port, contextPath, retargeted, endpointChanged };
|
||||
}
|
||||
|
||||
export async function runNodeDaemonInstall(opts: NodeDaemonInstallOptions) {
|
||||
@@ -102,7 +108,7 @@ export async function runNodeDaemonInstall(opts: NodeDaemonInstallOptions) {
|
||||
}
|
||||
|
||||
const config = await loadNodeHostConfig();
|
||||
const { host, port, contextPath } = resolveNodeDefaults(opts, config);
|
||||
const { host, port, contextPath, endpointChanged } = resolveNodeDefaults(opts, config);
|
||||
if (!Number.isFinite(port ?? Number.NaN) || (port ?? 0) <= 0 || (port ?? 0) > 65_535) {
|
||||
fail(
|
||||
opts.port !== undefined
|
||||
@@ -142,8 +148,10 @@ export async function runNodeDaemonInstall(opts: NodeDaemonInstallOptions) {
|
||||
}
|
||||
|
||||
const tlsFingerprint =
|
||||
normalizeOptionalString(opts.tlsFingerprint) || config?.gateway?.tlsFingerprint;
|
||||
const tls = Boolean(opts.tls) || Boolean(tlsFingerprint) || Boolean(config?.gateway?.tls);
|
||||
normalizeOptionalString(opts.tlsFingerprint) ||
|
||||
(endpointChanged ? undefined : config?.gateway?.tlsFingerprint);
|
||||
const inheritedTls = endpointChanged ? undefined : config?.gateway?.tls;
|
||||
const tls = Boolean(opts.tls) || Boolean(tlsFingerprint) || Boolean(inheritedTls);
|
||||
const { programArguments, workingDirectory, environment, environmentValueSources, description } =
|
||||
await buildNodeInstallPlan({
|
||||
env: process.env,
|
||||
@@ -248,7 +256,18 @@ export async function runNodeDaemonStatus(opts: NodeDaemonStatusOptions = {}) {
|
||||
};
|
||||
|
||||
if (json) {
|
||||
defaultRuntime.writeJson(payload);
|
||||
const safeEnvironment = filterDaemonEnv(command?.environment);
|
||||
defaultRuntime.writeJson({
|
||||
service: {
|
||||
...payload.service,
|
||||
command: command
|
||||
? {
|
||||
...command,
|
||||
environment: Object.keys(safeEnvironment).length > 0 ? safeEnvironment : undefined,
|
||||
}
|
||||
: command,
|
||||
},
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
|
||||
@@ -134,4 +134,39 @@ describe("registerNodeCli", () => {
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("passes an explicit plaintext selection to the node host", async () => {
|
||||
daemonMocks.loadNodeHostConfig.mockResolvedValue({
|
||||
version: 1,
|
||||
nodeId: "node-existing",
|
||||
gateway: {
|
||||
host: "10.0.0.2",
|
||||
port: 19001,
|
||||
tls: true,
|
||||
tlsFingerprint: "saved-fingerprint",
|
||||
},
|
||||
});
|
||||
|
||||
await createProgram().parseAsync(["node", "run", "--no-tls"], { from: "user" });
|
||||
|
||||
expect(daemonMocks.runNodeHost).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
gatewayTls: false,
|
||||
gatewayTlsFingerprint: undefined,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects a TLS fingerprint with an explicit plaintext selection", async () => {
|
||||
await createProgram().parseAsync(
|
||||
["node", "run", "--no-tls", "--tls-fingerprint", "sha256:fingerprint"],
|
||||
{ from: "user" },
|
||||
);
|
||||
|
||||
expect(daemonMocks.runNodeHost).not.toHaveBeenCalled();
|
||||
expect(daemonMocks.defaultRuntime.error).toHaveBeenCalledWith(
|
||||
"--no-tls cannot be combined with --tls-fingerprint",
|
||||
);
|
||||
expect(daemonMocks.defaultRuntime.exit).toHaveBeenCalledWith(1);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -52,6 +52,7 @@ export function registerNodeCli(program: Command) {
|
||||
.option("--port <port>", "Gateway port")
|
||||
.option("--context-path <path>", "Gateway WebSocket context path (e.g. /openclaw-gw)")
|
||||
.option("--tls", "Use TLS for the gateway connection")
|
||||
.option("--no-tls", "Disable TLS for the gateway connection")
|
||||
.option("--tls-fingerprint <sha256>", "Expected TLS certificate fingerprint (sha256)")
|
||||
.option("--node-id <id>", "Override node id (clears pairing token)")
|
||||
.option("--display-name <name>", "Override node display name")
|
||||
@@ -69,8 +70,16 @@ export function registerNodeCli(program: Command) {
|
||||
}
|
||||
const retargetedGateway = opts.host !== undefined || opts.port !== undefined;
|
||||
const explicitContextPath = opts.contextPath !== undefined;
|
||||
const explicitTlsDisabled = opts.tls === false;
|
||||
if (explicitTlsDisabled && opts.tlsFingerprint !== undefined) {
|
||||
defaultRuntime.error("--no-tls cannot be combined with --tls-fingerprint");
|
||||
defaultRuntime.exit(1);
|
||||
return;
|
||||
}
|
||||
const tlsFingerprint =
|
||||
opts.tlsFingerprint ?? (retargetedGateway ? undefined : existing?.gateway?.tlsFingerprint);
|
||||
explicitTlsDisabled || retargetedGateway
|
||||
? opts.tlsFingerprint
|
||||
: (opts.tlsFingerprint ?? existing?.gateway?.tlsFingerprint);
|
||||
const inheritedTls = retargetedGateway ? undefined : existing?.gateway?.tls;
|
||||
await runNodeHost({
|
||||
gatewayHost: host,
|
||||
|
||||
@@ -58,6 +58,7 @@ describe("buildNodeInstallPlan", () => {
|
||||
});
|
||||
expect(plan.environmentValueSources).toEqual({
|
||||
OPENCLAW_GATEWAY_TOKEN: "file",
|
||||
OPENCLAW_GATEWAY_PASSWORD: "file", // pragma: allowlist secret
|
||||
});
|
||||
expect(mocks.resolvePreferredNodePath).not.toHaveBeenCalled();
|
||||
expect(mocks.buildNodeServiceEnvironment).toHaveBeenCalledWith({
|
||||
@@ -95,7 +96,7 @@ describe("buildNodeInstallPlan", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("marks node gateway tokens as file-backed service env", async () => {
|
||||
it("marks node gateway credentials as file-backed service env", async () => {
|
||||
mocks.resolveNodeProgramArguments.mockResolvedValue({
|
||||
programArguments: ["node", "node-host"],
|
||||
workingDirectory: "/Users/me",
|
||||
@@ -108,19 +109,25 @@ describe("buildNodeInstallPlan", () => {
|
||||
mocks.renderSystemNodeWarning.mockReturnValue(undefined);
|
||||
mocks.buildNodeServiceEnvironment.mockReturnValue({
|
||||
OPENCLAW_GATEWAY_TOKEN: "node-token",
|
||||
OPENCLAW_GATEWAY_PASSWORD: "node-password",
|
||||
OPENCLAW_SERVICE_VERSION: "2026.3.22",
|
||||
});
|
||||
|
||||
const plan = await buildNodeInstallPlan({
|
||||
env: { OPENCLAW_GATEWAY_TOKEN: "node-token" },
|
||||
env: {
|
||||
OPENCLAW_GATEWAY_TOKEN: "node-token",
|
||||
OPENCLAW_GATEWAY_PASSWORD: "node-password",
|
||||
},
|
||||
host: "127.0.0.1",
|
||||
port: 18789,
|
||||
runtime: "node",
|
||||
});
|
||||
|
||||
expect(plan.environment.OPENCLAW_GATEWAY_TOKEN).toBe("node-token");
|
||||
expect(plan.environment.OPENCLAW_GATEWAY_PASSWORD).toBe("node-password");
|
||||
expect(plan.environmentValueSources).toEqual({
|
||||
OPENCLAW_GATEWAY_TOKEN: "file",
|
||||
OPENCLAW_GATEWAY_PASSWORD: "file", // pragma: allowlist secret
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -25,6 +25,7 @@ function buildNodeInstallEnvironmentValueSources(): Record<
|
||||
> {
|
||||
return {
|
||||
OPENCLAW_GATEWAY_TOKEN: "file",
|
||||
OPENCLAW_GATEWAY_PASSWORD: "file", // pragma: allowlist secret
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -38,7 +38,7 @@ vi.mock("node:child_process", async () => {
|
||||
};
|
||||
});
|
||||
|
||||
import { resolveGatewayProgramArguments } from "./program-args.js";
|
||||
import { resolveGatewayProgramArguments, resolveNodeProgramArguments } from "./program-args.js";
|
||||
|
||||
const originalArgv = [...process.argv];
|
||||
|
||||
@@ -243,3 +243,31 @@ describe("resolveGatewayProgramArguments", () => {
|
||||
).rejects.toThrow("OPENCLAW_WRAPPER must point to an executable file");
|
||||
});
|
||||
});
|
||||
|
||||
describe("resolveNodeProgramArguments", () => {
|
||||
it("carries an explicit plaintext selection into the managed node command", async () => {
|
||||
const entryPath = path.resolve("/opt/openclaw/dist/entry.js");
|
||||
const indexPath = path.resolve("/opt/openclaw/dist/index.js");
|
||||
process.argv = ["node", entryPath];
|
||||
fsMocks.realpath.mockResolvedValue(entryPath);
|
||||
fsMocks.access.mockResolvedValue(undefined);
|
||||
|
||||
const result = await resolveNodeProgramArguments({
|
||||
host: "gateway.example",
|
||||
port: 18789,
|
||||
tls: false,
|
||||
});
|
||||
|
||||
expect(result.programArguments).toEqual([
|
||||
process.execPath,
|
||||
indexPath,
|
||||
"node",
|
||||
"run",
|
||||
"--host",
|
||||
"gateway.example",
|
||||
"--port",
|
||||
"18789",
|
||||
"--no-tls",
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -321,7 +321,11 @@ export async function resolveNodeProgramArguments(params: {
|
||||
nodePath?: string;
|
||||
}): Promise<GatewayProgramArgs> {
|
||||
const args = ["node", "run", "--host", params.host, "--port", String(params.port)];
|
||||
if (params.tls || params.tlsFingerprint) {
|
||||
if (params.tls === false && !params.tlsFingerprint) {
|
||||
// Managed services must carry plaintext explicitly; omission would let the
|
||||
// node runtime re-inherit TLS from the operator's global Gateway config.
|
||||
args.push("--no-tls");
|
||||
} else if (params.tls || params.tlsFingerprint) {
|
||||
args.push("--tls");
|
||||
}
|
||||
if (params.tlsFingerprint) {
|
||||
|
||||
@@ -854,6 +854,13 @@ describe("buildNodeServiceEnvironment", () => {
|
||||
expect(env.OPENCLAW_GATEWAY_TOKEN).toBe("node-token");
|
||||
});
|
||||
|
||||
it("passes through OPENCLAW_GATEWAY_PASSWORD for node services", () => {
|
||||
const env = buildNodeServiceEnvironment({
|
||||
env: { HOME: "/home/user", OPENCLAW_GATEWAY_PASSWORD: " node-password " },
|
||||
});
|
||||
expect(env.OPENCLAW_GATEWAY_PASSWORD).toBe("node-password");
|
||||
});
|
||||
|
||||
it("passes through OPENCLAW_ALLOW_INSECURE_PRIVATE_WS for node services", () => {
|
||||
const env = buildNodeServiceEnvironment({
|
||||
env: { HOME: "/home/user", OPENCLAW_ALLOW_INSECURE_PRIVATE_WS: " 1 " },
|
||||
|
||||
@@ -461,10 +461,12 @@ export function buildNodeServiceEnvironment(params: {
|
||||
params.execPath,
|
||||
);
|
||||
const gatewayToken = normalizeOptionalString(env.OPENCLAW_GATEWAY_TOKEN);
|
||||
const gatewayPassword = normalizeOptionalString(env.OPENCLAW_GATEWAY_PASSWORD);
|
||||
const allowInsecurePrivateWs = normalizeOptionalString(env.OPENCLAW_ALLOW_INSECURE_PRIVATE_WS);
|
||||
return {
|
||||
...buildCommonServiceEnvironment(env, sharedEnv),
|
||||
OPENCLAW_GATEWAY_TOKEN: gatewayToken,
|
||||
OPENCLAW_GATEWAY_PASSWORD: gatewayPassword,
|
||||
OPENCLAW_ALLOW_INSECURE_PRIVATE_WS: allowInsecurePrivateWs,
|
||||
OPENCLAW_LAUNCHD_LABEL: resolveNodeLaunchAgentLabel(),
|
||||
OPENCLAW_SYSTEMD_UNIT: resolveNodeSystemdServiceName(),
|
||||
|
||||
@@ -1198,7 +1198,9 @@ describe("readSystemdServiceExecStart", () => {
|
||||
"# comment",
|
||||
"; another comment",
|
||||
'OPENCLAW_GATEWAY_TOKEN="quoted token"', // pragma: allowlist secret
|
||||
"OPENCLAW_GATEWAY_PASSWORD=quoted-password", // pragma: allowlist secret
|
||||
'OPENCLAW_GATEWAY_PASSWORD="symbol \\" \\\\ \\$ \\`"', // pragma: allowlist secret
|
||||
'MIXED_API_KEY="55\\"55" "FIVE" cinco',
|
||||
'UNQUOTED_QUOTES_API_KEY=foo"bar"',
|
||||
].join("\n");
|
||||
}
|
||||
throw new Error(`unexpected readFile path: ${pathValue}`);
|
||||
@@ -1207,11 +1209,15 @@ describe("readSystemdServiceExecStart", () => {
|
||||
const command = await readSystemdServiceExecStart({ HOME: "/home/test" });
|
||||
expect(command?.environment).toEqual({
|
||||
OPENCLAW_GATEWAY_TOKEN: "quoted token",
|
||||
OPENCLAW_GATEWAY_PASSWORD: "quoted-password", // pragma: allowlist secret
|
||||
OPENCLAW_GATEWAY_PASSWORD: 'symbol " \\ $ `', // pragma: allowlist secret
|
||||
MIXED_API_KEY: '55"55FIVEcinco',
|
||||
UNQUOTED_QUOTES_API_KEY: 'foo"bar"',
|
||||
});
|
||||
expect(command?.environmentValueSources).toEqual({
|
||||
OPENCLAW_GATEWAY_TOKEN: "file",
|
||||
OPENCLAW_GATEWAY_PASSWORD: "file", // pragma: allowlist secret
|
||||
MIXED_API_KEY: "file",
|
||||
UNQUOTED_QUOTES_API_KEY: "file",
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -1298,6 +1304,7 @@ describe("stageSystemdService", () => {
|
||||
it("writes node file-backed managed values to the node env file instead of the unit", async () => {
|
||||
await withStageFixture(async ({ env, stateDir, unitPath, envFilePath, nodeEnvFilePath }) => {
|
||||
await fs.rm(stateDir, { recursive: true, force: true });
|
||||
const gatewayPassword = 'symbol " \\ $ `'; // pragma: allowlist secret
|
||||
|
||||
mockSystemctlStatusOk();
|
||||
|
||||
@@ -1308,12 +1315,14 @@ describe("stageSystemdService", () => {
|
||||
workingDirectory: "/tmp",
|
||||
environment: {
|
||||
OPENCLAW_GATEWAY_TOKEN: "file-backed-token",
|
||||
OPENCLAW_GATEWAY_PASSWORD: gatewayPassword,
|
||||
OPENCLAW_GATEWAY_PORT: "18789",
|
||||
OPENCLAW_SERVICE_MANAGED_ENV_KEYS: "OPENCLAW_GATEWAY_TOKEN",
|
||||
OPENCLAW_SERVICE_MANAGED_ENV_KEYS: "OPENCLAW_GATEWAY_PASSWORD,OPENCLAW_GATEWAY_TOKEN", // pragma: allowlist secret
|
||||
OPENCLAW_SERVICE_KIND: "node",
|
||||
},
|
||||
environmentValueSources: {
|
||||
OPENCLAW_GATEWAY_TOKEN: "file",
|
||||
OPENCLAW_GATEWAY_PASSWORD: "file", // pragma: allowlist secret
|
||||
OPENCLAW_SERVICE_MANAGED_ENV_KEYS: "inline",
|
||||
},
|
||||
});
|
||||
@@ -1327,8 +1336,16 @@ describe("stageSystemdService", () => {
|
||||
expect(unit).toContain(`EnvironmentFile=-${nodeEnvFilePath}`);
|
||||
expect(unit).toContain("Environment=OPENCLAW_GATEWAY_PORT=18789");
|
||||
expect(unit).not.toContain("Environment=OPENCLAW_GATEWAY_TOKEN=file-backed-token");
|
||||
expect(envFile).toBe("OPENCLAW_GATEWAY_TOKEN=file-backed-token\n");
|
||||
expect(unit).not.toContain("Environment=OPENCLAW_GATEWAY_PASSWORD=");
|
||||
expect(envFile).toBe(
|
||||
'OPENCLAW_GATEWAY_TOKEN=file-backed-token\nOPENCLAW_GATEWAY_PASSWORD="symbol \\" \\\\ \\$ \\`"\n',
|
||||
);
|
||||
expect(envFileStat.mode & 0o777).toBe(0o600);
|
||||
await expect(readSystemdServiceExecStart(env)).resolves.toMatchObject({
|
||||
environment: {
|
||||
OPENCLAW_GATEWAY_PASSWORD: gatewayPassword,
|
||||
},
|
||||
});
|
||||
await expect(fs.access(envFilePath)).rejects.toThrow();
|
||||
});
|
||||
});
|
||||
@@ -1637,6 +1654,37 @@ describe("stageSystemdService", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("preserves explicit literal shell references and mixed quoted values", async () => {
|
||||
await withStageFixture(async ({ env, envFilePath }) => {
|
||||
await fs.writeFile(
|
||||
envFilePath,
|
||||
[
|
||||
"OPENROUTER_API_KEY=\\$SECRET_FROM_SHELL",
|
||||
"SINGLE_QUOTED_LITERAL_API_KEY='$SECRET_FROM_SHELL'",
|
||||
'DOUBLE_QUOTED_LITERAL_API_KEY="$SECRET_FROM_SHELL"',
|
||||
'MIXED_API_KEY="foo"bar',
|
||||
].join("\n") + "\n",
|
||||
{ encoding: "utf8", mode: 0o600 },
|
||||
);
|
||||
|
||||
mockSystemctlStatusOk();
|
||||
|
||||
await stageSystemdService({
|
||||
env,
|
||||
stdout: { write: vi.fn() } as unknown as NodeJS.WritableStream,
|
||||
programArguments: ["/usr/bin/openclaw", "gateway", "run"],
|
||||
workingDirectory: "/tmp",
|
||||
environment: { OPENCLAW_GATEWAY_PORT: "18789" },
|
||||
});
|
||||
|
||||
const envFile = await fs.readFile(envFilePath, "utf8");
|
||||
expect(envFile).toContain('OPENROUTER_API_KEY="\\$SECRET_FROM_SHELL"');
|
||||
expect(envFile).toContain('SINGLE_QUOTED_LITERAL_API_KEY="\\$SECRET_FROM_SHELL"');
|
||||
expect(envFile).toContain('DOUBLE_QUOTED_LITERAL_API_KEY="\\$SECRET_FROM_SHELL"');
|
||||
expect(envFile).toContain("MIXED_API_KEY=foobar");
|
||||
});
|
||||
});
|
||||
|
||||
it("removes a stale literal reference on re-stage when state-dir .env now skips that key (#88274)", async () => {
|
||||
await withStageFixture(async ({ env, stateDir, envFilePath }) => {
|
||||
// A prior install generated a literal reference for LLM_API_KEY (an unexpanded
|
||||
@@ -1725,7 +1773,7 @@ describe("stageSystemdService", () => {
|
||||
const envFile = await fs.readFile(envFilePath, "utf8");
|
||||
expect(envFile).toContain("ANTHROPIC_API_KEY=sk-ant-operator-secret");
|
||||
expect(envFile).toContain("OPENROUTER_API_KEY=or-operator-key");
|
||||
expect(envFile).toContain("LOWERCASE_LITERAL_API_KEY=$ecret123");
|
||||
expect(envFile).toContain('LOWERCASE_LITERAL_API_KEY="\\$ecret123"');
|
||||
expect(envFile).not.toContain("LLM_API_KEY");
|
||||
});
|
||||
});
|
||||
@@ -1985,7 +2033,15 @@ describe("systemd service install and uninstall", () => {
|
||||
await fs.writeFile(unitPath, "[Unit]\nDescription=OpenClaw Node\n", "utf8");
|
||||
await fs.writeFile(
|
||||
nodeEnvFilePath,
|
||||
"OPENCLAW_GATEWAY_TOKEN=stale-node-token\nOPENROUTER_API_KEY=operator-key\n",
|
||||
[
|
||||
"OPENCLAW_GATEWAY_TOKEN=stale-node-token",
|
||||
"OPENCLAW_GATEWAY_PASSWORD=stale-password",
|
||||
"OPENROUTER_API_KEY=operator-key",
|
||||
"LLM_API_KEY=$SECRET_FROM_SHELL",
|
||||
"LITERAL_API_KEY=\\$SECRET_FROM_SHELL",
|
||||
"SINGLE_QUOTED_LITERAL_API_KEY='$SECRET_FROM_SHELL'",
|
||||
'DOUBLE_QUOTED_LITERAL_API_KEY="$SECRET_FROM_SHELL"',
|
||||
].join("\n") + "\n",
|
||||
{ encoding: "utf8", mode: 0o600 },
|
||||
);
|
||||
|
||||
@@ -2010,13 +2066,45 @@ describe("systemd service install and uninstall", () => {
|
||||
}
|
||||
expect(accessError?.code).toBe("ENOENT");
|
||||
await expect(fs.readFile(nodeEnvFilePath, "utf8")).resolves.toBe(
|
||||
"OPENROUTER_API_KEY=operator-key\n",
|
||||
[
|
||||
"OPENROUTER_API_KEY=operator-key",
|
||||
'LITERAL_API_KEY="\\$SECRET_FROM_SHELL"',
|
||||
'SINGLE_QUOTED_LITERAL_API_KEY="\\$SECRET_FROM_SHELL"',
|
||||
'DOUBLE_QUOTED_LITERAL_API_KEY="\\$SECRET_FROM_SHELL"',
|
||||
].join("\n") + "\n",
|
||||
);
|
||||
expect(requireFirstWrite(write)).toContain("Removed systemd service");
|
||||
expect(execFileMock).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
});
|
||||
|
||||
it("removes a password-only node environment file during uninstall", async () => {
|
||||
await withNodeSystemdFixture(async ({ env, unitPath, nodeEnvFilePath }) => {
|
||||
await fs.mkdir(path.dirname(unitPath), { recursive: true });
|
||||
await fs.writeFile(unitPath, "[Unit]\nDescription=OpenClaw Node\n", "utf8");
|
||||
await fs.writeFile(nodeEnvFilePath, "OPENCLAW_GATEWAY_PASSWORD=stale-password\n", {
|
||||
encoding: "utf8",
|
||||
mode: 0o600,
|
||||
});
|
||||
|
||||
execFileMock
|
||||
.mockImplementationOnce((_cmd, args, _opts, cb) => {
|
||||
assertUserSystemctlArgs(args, "status");
|
||||
cb(null, "", "");
|
||||
})
|
||||
.mockImplementationOnce((_cmd, args, _opts, cb) => {
|
||||
assertUserSystemctlArgs(args, "disable", "--now", NODE_SERVICE);
|
||||
cb(null, "", "");
|
||||
});
|
||||
|
||||
const { stdout } = createWritableStreamMock();
|
||||
await uninstallSystemdService({ env, stdout });
|
||||
|
||||
await expect(fs.access(nodeEnvFilePath)).rejects.toThrow();
|
||||
expect(execFileMock).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
});
|
||||
|
||||
it("preserves node env file values when unit removal fails during uninstall", async () => {
|
||||
await withNodeSystemdFixture(async ({ env, unitPath, nodeEnvFilePath }) => {
|
||||
await fs.mkdir(path.dirname(unitPath), { recursive: true });
|
||||
|
||||
+162
-30
@@ -408,32 +408,149 @@ function parseEnvironmentFileSpecs(raw: string): string[] {
|
||||
return normalizeStringEntries(splitArgsPreservingQuotes(raw, { escapeMode: "backslash" }));
|
||||
}
|
||||
|
||||
function parseEnvironmentFileLine(rawLine: string): { key: string; value: string } | null {
|
||||
const trimmed = rawLine.trim();
|
||||
if (!trimmed || trimmed.startsWith("#") || trimmed.startsWith(";")) {
|
||||
function decodeSystemdEnvironmentFileValue(rawValue: string): {
|
||||
value: string;
|
||||
literalDollar: boolean;
|
||||
} {
|
||||
type ParseState =
|
||||
| "pre"
|
||||
| "unquoted"
|
||||
| "unquoted-escape"
|
||||
| "single-quoted"
|
||||
| "double-quoted"
|
||||
| "double-quoted-escape";
|
||||
|
||||
// Mirror systemd's parse_env_file_internal state transitions. In particular,
|
||||
// a closing quoted segment returns to `pre`, so `"foo"bar` decodes to `foobar`.
|
||||
let state: ParseState = "pre";
|
||||
let decoded = "";
|
||||
let literalDollar = false;
|
||||
let trailingWhitespaceStart: number | undefined;
|
||||
for (const char of rawValue) {
|
||||
const whitespace = char === " " || char === "\t" || char === "\r";
|
||||
if (state === "pre") {
|
||||
if (whitespace) {
|
||||
continue;
|
||||
}
|
||||
if (char === "'") {
|
||||
state = "single-quoted";
|
||||
continue;
|
||||
}
|
||||
if (char === '"') {
|
||||
state = "double-quoted";
|
||||
continue;
|
||||
}
|
||||
if (char === "\\") {
|
||||
state = "unquoted-escape";
|
||||
continue;
|
||||
}
|
||||
state = "unquoted";
|
||||
decoded += char;
|
||||
continue;
|
||||
}
|
||||
if (state === "unquoted") {
|
||||
if (char === "\\") {
|
||||
state = "unquoted-escape";
|
||||
trailingWhitespaceStart = undefined;
|
||||
continue;
|
||||
}
|
||||
if (whitespace) {
|
||||
trailingWhitespaceStart ??= decoded.length;
|
||||
} else {
|
||||
trailingWhitespaceStart = undefined;
|
||||
}
|
||||
decoded += char;
|
||||
continue;
|
||||
}
|
||||
if (state === "unquoted-escape") {
|
||||
state = "unquoted";
|
||||
literalDollar ||= char === "$";
|
||||
decoded += char;
|
||||
continue;
|
||||
}
|
||||
if (state === "single-quoted") {
|
||||
if (char === "'") {
|
||||
state = "pre";
|
||||
} else {
|
||||
literalDollar ||= char === "$";
|
||||
decoded += char;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if (state === "double-quoted") {
|
||||
if (char === '"') {
|
||||
state = "pre";
|
||||
} else if (char === "\\") {
|
||||
state = "double-quoted-escape";
|
||||
} else {
|
||||
literalDollar ||= char === "$";
|
||||
decoded += char;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
state = "double-quoted";
|
||||
if (['"', "\\", "`", "$"].includes(char)) {
|
||||
literalDollar ||= char === "$";
|
||||
decoded += char;
|
||||
} else {
|
||||
decoded += `\\${char}`;
|
||||
}
|
||||
}
|
||||
if (state === "unquoted" && trailingWhitespaceStart !== undefined) {
|
||||
decoded = decoded.slice(0, trailingWhitespaceStart);
|
||||
}
|
||||
return { value: decoded, literalDollar };
|
||||
}
|
||||
|
||||
function parseEnvironmentFileLine(
|
||||
rawLine: string,
|
||||
): { key: string; value: string; literalShellReference: boolean } | null {
|
||||
const trimmedStart = rawLine.trimStart();
|
||||
if (!trimmedStart || trimmedStart.startsWith("#") || trimmedStart.startsWith(";")) {
|
||||
return null;
|
||||
}
|
||||
const eq = trimmed.indexOf("=");
|
||||
const eq = trimmedStart.indexOf("=");
|
||||
if (eq <= 0) {
|
||||
return null;
|
||||
}
|
||||
const key = trimmed.slice(0, eq).trim();
|
||||
const key = trimmedStart.slice(0, eq).trim();
|
||||
if (!key) {
|
||||
return null;
|
||||
}
|
||||
let value = trimmed.slice(eq + 1).trim();
|
||||
if (
|
||||
value.length >= 2 &&
|
||||
((value.startsWith('"') && value.endsWith('"')) ||
|
||||
(value.startsWith("'") && value.endsWith("'")))
|
||||
) {
|
||||
value = value.slice(1, -1);
|
||||
}
|
||||
return { key, value };
|
||||
const decoded = decodeSystemdEnvironmentFileValue(trimmedStart.slice(eq + 1));
|
||||
return {
|
||||
key,
|
||||
value: decoded.value,
|
||||
literalShellReference: decoded.literalDollar && isUnresolvedShellReference(decoded.value),
|
||||
};
|
||||
}
|
||||
|
||||
async function readSystemdEnvironmentFile(pathname: string): Promise<Record<string, string>> {
|
||||
function serializeSystemdEnvironmentFileValue(value: string): string {
|
||||
// EnvironmentFile double quotes only unescape \", \\, \`, and \$. Escape
|
||||
// exactly that set so credentials survive systemd parsing byte-for-byte.
|
||||
if (!/[\s\\'"`$]/u.test(value)) {
|
||||
return value;
|
||||
}
|
||||
const escaped = value
|
||||
.replaceAll("\\", "\\\\")
|
||||
.replaceAll('"', '\\"')
|
||||
.replaceAll("`", "\\`")
|
||||
.replaceAll("$", "\\$");
|
||||
return `"${escaped}"`;
|
||||
}
|
||||
|
||||
function serializeSystemdEnvironmentFile(environment: Record<string, string>): string {
|
||||
return Object.entries(environment)
|
||||
.map(([key, value]) => `${key}=${serializeSystemdEnvironmentFileValue(value)}`)
|
||||
.join("\n");
|
||||
}
|
||||
|
||||
async function readSystemdEnvironmentFile(pathname: string): Promise<{
|
||||
environment: Record<string, string>;
|
||||
literalShellReferenceKeys: Set<string>;
|
||||
}> {
|
||||
const environment: Record<string, string> = {};
|
||||
const literalShellReferenceKeys = new Set<string>();
|
||||
const content = await fs.readFile(pathname, "utf8");
|
||||
for (const rawLine of content.split(/\r?\n/)) {
|
||||
const parsed = parseEnvironmentFileLine(rawLine);
|
||||
@@ -441,8 +558,13 @@ async function readSystemdEnvironmentFile(pathname: string): Promise<Record<stri
|
||||
continue;
|
||||
}
|
||||
environment[parsed.key] = parsed.value;
|
||||
if (parsed.literalShellReference) {
|
||||
literalShellReferenceKeys.add(parsed.key);
|
||||
} else {
|
||||
literalShellReferenceKeys.delete(parsed.key);
|
||||
}
|
||||
}
|
||||
return environment;
|
||||
return { environment, literalShellReferenceKeys };
|
||||
}
|
||||
|
||||
async function resolveSystemdEnvironmentFiles(params: {
|
||||
@@ -468,7 +590,7 @@ async function resolveSystemdEnvironmentFiles(params: {
|
||||
: path.posix.resolve(unitDir, expanded);
|
||||
try {
|
||||
const fromFile = await readSystemdEnvironmentFile(pathname);
|
||||
Object.assign(resolved, fromFile);
|
||||
Object.assign(resolved, fromFile.environment);
|
||||
} catch {
|
||||
// Keep service auditing resilient even when env files are unavailable
|
||||
// in the current runtime context. Both optional and non-optional
|
||||
@@ -985,6 +1107,7 @@ async function writeSystemdGatewayEnvironmentFile(params: {
|
||||
// file copy would override the fresh inline Environment= value because systemd's
|
||||
// EnvironmentFile takes precedence over inline Environment= directives.
|
||||
const existing: Record<string, string> = {};
|
||||
const literalShellReferenceKeys = new Set<string>();
|
||||
const legacyNodeEnvFilePath = resolveLegacyNodeSystemdEnvironmentFilePath({
|
||||
stateDir: params.stateDir,
|
||||
environment: params.environment,
|
||||
@@ -994,7 +1117,15 @@ async function writeSystemdGatewayEnvironmentFile(params: {
|
||||
continue;
|
||||
}
|
||||
try {
|
||||
Object.assign(existing, await readSystemdEnvironmentFile(sourceEnvFilePath));
|
||||
const fromFile = await readSystemdEnvironmentFile(sourceEnvFilePath);
|
||||
for (const [key, value] of Object.entries(fromFile.environment)) {
|
||||
existing[key] = value;
|
||||
if (fromFile.literalShellReferenceKeys.has(key)) {
|
||||
literalShellReferenceKeys.add(key);
|
||||
} else {
|
||||
literalShellReferenceKeys.delete(key);
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
// File does not exist yet — nothing to preserve.
|
||||
}
|
||||
@@ -1013,7 +1144,9 @@ async function writeSystemdGatewayEnvironmentFile(params: {
|
||||
if (normalized && managedKeysToDrop.has(normalized)) {
|
||||
return false;
|
||||
}
|
||||
return !isUnresolvedShellReference(value);
|
||||
// Quoting or escaping `$VAR` records operator intent; bare references can
|
||||
// still be stale values copied from the state-dir dotenv file.
|
||||
return literalShellReferenceKeys.has(key) || !isUnresolvedShellReference(value);
|
||||
}),
|
||||
);
|
||||
const merged = { ...operatorOnly, ...incoming };
|
||||
@@ -1030,9 +1163,7 @@ async function writeSystemdGatewayEnvironmentFile(params: {
|
||||
return { environmentFiles: [], environmentKeys };
|
||||
}
|
||||
|
||||
const content = Object.entries(merged)
|
||||
.map(([key, value]) => `${key}=${value}`)
|
||||
.join("\n");
|
||||
const content = serializeSystemdEnvironmentFile(merged);
|
||||
await fs.mkdir(path.dirname(envFilePath), { recursive: true });
|
||||
await fs.writeFile(envFilePath, `${content}\n`, { encoding: "utf8", mode: 0o600 });
|
||||
await fs.chmod(envFilePath, 0o600);
|
||||
@@ -1048,26 +1179,27 @@ async function removeNodeSystemdManagedEnvironmentKeys(env: GatewayServiceEnv):
|
||||
stateDir,
|
||||
environment: env,
|
||||
});
|
||||
let existing: Record<string, string>;
|
||||
let existingFile: Awaited<ReturnType<typeof readSystemdEnvironmentFile>>;
|
||||
try {
|
||||
existing = await readSystemdEnvironmentFile(envFilePath);
|
||||
existingFile = await readSystemdEnvironmentFile(envFilePath);
|
||||
} catch {
|
||||
return;
|
||||
}
|
||||
const managedKeys = new Set([normalizeSystemdEnvironmentKey("OPENCLAW_GATEWAY_TOKEN")]);
|
||||
const managedKeys = new Set(["OPENCLAW_GATEWAY_TOKEN", "OPENCLAW_GATEWAY_PASSWORD"]);
|
||||
const remaining = Object.fromEntries(
|
||||
Object.entries(existing).filter(([key]) => {
|
||||
Object.entries(existingFile.environment).filter(([key, value]) => {
|
||||
const normalized = normalizeSystemdEnvironmentKey(key);
|
||||
return !normalized || !managedKeys.has(normalized);
|
||||
if (normalized && managedKeys.has(normalized)) {
|
||||
return false;
|
||||
}
|
||||
return existingFile.literalShellReferenceKeys.has(key) || !isUnresolvedShellReference(value);
|
||||
}),
|
||||
);
|
||||
if (Object.keys(remaining).length === 0) {
|
||||
await fs.rm(envFilePath, { force: true });
|
||||
return;
|
||||
}
|
||||
const content = Object.entries(remaining)
|
||||
.map(([key, value]) => `${key}=${value}`)
|
||||
.join("\n");
|
||||
const content = serializeSystemdEnvironmentFile(remaining);
|
||||
await fs.writeFile(envFilePath, `${content}\n`, { encoding: "utf8", mode: 0o600 });
|
||||
await fs.chmod(envFilePath, 0o600);
|
||||
}
|
||||
|
||||
@@ -248,13 +248,17 @@ describe("gateway/node-registry", () => {
|
||||
const registry = new NodeRegistry();
|
||||
try {
|
||||
const frames = registerNode(registry);
|
||||
const { invoke } = invokeSystemRun(
|
||||
const { invoke, request } = invokeSystemRun(
|
||||
registry,
|
||||
frames,
|
||||
{ runId: "run-timeout", sessionKey: "agent:main:main", timeoutMs: 0 },
|
||||
1,
|
||||
);
|
||||
const forwarded = JSON.parse(request.payload?.paramsJSON ?? "{}") as {
|
||||
timeoutMs?: number | null;
|
||||
};
|
||||
|
||||
expect(forwarded.timeoutMs).toBeNull();
|
||||
await vi.advanceTimersByTimeAsync(1);
|
||||
await expect(invoke).resolves.toEqual({
|
||||
ok: false,
|
||||
@@ -272,6 +276,57 @@ describe("gateway/node-registry", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("keeps zero-timeout invokes pending until the node responds", async () => {
|
||||
vi.useFakeTimers();
|
||||
const registry = new NodeRegistry();
|
||||
try {
|
||||
const frames = registerNode(registry);
|
||||
const invoke = registry.invoke({
|
||||
nodeId: "node-1",
|
||||
command: "debug.ping",
|
||||
timeoutMs: 0,
|
||||
});
|
||||
const request = JSON.parse(frames[0] ?? "{}") as {
|
||||
payload?: { id?: string; timeoutMs?: number };
|
||||
};
|
||||
|
||||
expect(request.payload?.timeoutMs).toBe(0);
|
||||
await vi.advanceTimersByTimeAsync(60_000);
|
||||
expect(
|
||||
registry.handleInvokeResult({
|
||||
id: request.payload?.id ?? "",
|
||||
nodeId: "node-1",
|
||||
connId: "conn-1",
|
||||
ok: true,
|
||||
}),
|
||||
).toBe(true);
|
||||
await expect(invoke).resolves.toEqual({
|
||||
ok: true,
|
||||
payload: undefined,
|
||||
payloadJSON: null,
|
||||
error: null,
|
||||
});
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects zero-timeout invokes when the node disconnects", async () => {
|
||||
const registry = new NodeRegistry();
|
||||
registerNode(registry);
|
||||
const invoke = registry.invoke({
|
||||
nodeId: "node-1",
|
||||
command: "debug.ping",
|
||||
timeoutMs: 0,
|
||||
});
|
||||
const disconnected = invoke.catch((error: unknown) => error);
|
||||
|
||||
expect(registry.unregister("conn-1")).toBe("node-1");
|
||||
const error = await disconnected;
|
||||
expect(error).toBeInstanceOf(Error);
|
||||
expect((error as Error).message).toBe("node disconnected (debug.ping)");
|
||||
});
|
||||
|
||||
it("caps oversized invoke and system.run authorization timers", async () => {
|
||||
vi.useFakeTimers();
|
||||
vi.setSystemTime(0);
|
||||
@@ -288,7 +343,15 @@ describe("gateway/node-registry", () => {
|
||||
},
|
||||
Number.MAX_SAFE_INTEGER,
|
||||
);
|
||||
const request = JSON.parse(frames[0] ?? "{}") as {
|
||||
payload?: { paramsJSON?: string | null; timeoutMs?: number };
|
||||
};
|
||||
const forwarded = JSON.parse(request.payload?.paramsJSON ?? "{}") as {
|
||||
timeoutMs?: number | null;
|
||||
};
|
||||
|
||||
expect(request.payload?.timeoutMs).toBe(MAX_TIMER_TIMEOUT_MS);
|
||||
expect(forwarded.timeoutMs).toBe(MAX_TIMER_TIMEOUT_MS);
|
||||
await vi.advanceTimersByTimeAsync(MAX_TIMER_TIMEOUT_MS);
|
||||
await expect(invoke).resolves.toEqual({
|
||||
ok: false,
|
||||
|
||||
@@ -46,7 +46,7 @@ type PendingInvoke = {
|
||||
systemRunEvent?: PendingSystemRunEvent;
|
||||
resolve: (value: NodeInvokeResult) => void;
|
||||
reject: (err: Error) => void;
|
||||
timer: ReturnType<typeof setTimeout>;
|
||||
timer?: ReturnType<typeof setTimeout>;
|
||||
};
|
||||
|
||||
/** system.run metadata remembered while waiting for node events. */
|
||||
@@ -163,8 +163,8 @@ function resolvePendingSystemRunEvent(params: {
|
||||
};
|
||||
}
|
||||
|
||||
/** Ensure system.run requests have a runId before they are sent to a node. */
|
||||
function withSystemRunEventRunId(params: { command: string; params?: unknown }): unknown {
|
||||
/** Keep node execution and Gateway authorization on the same canonical system.run fields. */
|
||||
function normalizeSystemRunInvokeParams(params: { command: string; params?: unknown }): unknown {
|
||||
if (
|
||||
params.command !== "system.run" ||
|
||||
!params.params ||
|
||||
@@ -174,10 +174,17 @@ function withSystemRunEventRunId(params: { command: string; params?: unknown }):
|
||||
return params.params;
|
||||
}
|
||||
const obj = params.params as Record<string, unknown>;
|
||||
if (normalizeString(obj.runId)) {
|
||||
return params.params;
|
||||
const normalized: Record<string, unknown> = {
|
||||
...obj,
|
||||
runId: normalizeString(obj.runId) || randomUUID(),
|
||||
};
|
||||
const timeoutMs = normalizeSystemRunTimeoutMs(obj.timeoutMs);
|
||||
if (timeoutMs === undefined) {
|
||||
delete normalized.timeoutMs;
|
||||
} else {
|
||||
normalized.timeoutMs = timeoutMs;
|
||||
}
|
||||
return { ...obj, runId: randomUUID() };
|
||||
return normalized;
|
||||
}
|
||||
|
||||
/** Registry of currently connected Gateway nodes. */
|
||||
@@ -298,7 +305,9 @@ export class NodeRegistry {
|
||||
if (pending.connId !== connId) {
|
||||
continue;
|
||||
}
|
||||
clearTimeout(pending.timer);
|
||||
if (pending.timer !== undefined) {
|
||||
clearTimeout(pending.timer);
|
||||
}
|
||||
pending.reject(new Error(`node disconnected (${pending.command})`));
|
||||
this.pendingInvokes.delete(id);
|
||||
}
|
||||
@@ -486,17 +495,19 @@ export class NodeRegistry {
|
||||
};
|
||||
}
|
||||
const requestId = randomUUID();
|
||||
const invokeParams = withSystemRunEventRunId({
|
||||
const invokeParams = normalizeSystemRunInvokeParams({
|
||||
command: params.command,
|
||||
params: params.params,
|
||||
});
|
||||
// Keep node and Gateway on the same timer-safe value; zero disables both deadlines.
|
||||
const timeoutMs = resolveTimerTimeoutMs(params.timeoutMs, 30_000, 0);
|
||||
const payload = {
|
||||
id: requestId,
|
||||
nodeId: params.nodeId,
|
||||
command: params.command,
|
||||
paramsJSON:
|
||||
"params" in params && invokeParams !== undefined ? JSON.stringify(invokeParams) : null,
|
||||
timeoutMs: params.timeoutMs,
|
||||
timeoutMs,
|
||||
idempotencyKey: params.idempotencyKey,
|
||||
};
|
||||
const ok = this.sendEventToSession(node, "node.invoke.request", payload);
|
||||
@@ -517,15 +528,17 @@ export class NodeRegistry {
|
||||
...systemRunEvent,
|
||||
});
|
||||
}
|
||||
const timeoutMs = resolveTimerTimeoutMs(params.timeoutMs, 30_000, 0);
|
||||
return await new Promise<NodeInvokeResult>((resolve, reject) => {
|
||||
const timer = setTimeout(() => {
|
||||
this.pendingInvokes.delete(requestId);
|
||||
resolve({
|
||||
ok: false,
|
||||
error: { code: "TIMEOUT", message: "node invoke timed out" },
|
||||
});
|
||||
}, timeoutMs);
|
||||
const timer =
|
||||
timeoutMs > 0
|
||||
? setTimeout(() => {
|
||||
this.pendingInvokes.delete(requestId);
|
||||
resolve({
|
||||
ok: false,
|
||||
error: { code: "TIMEOUT", message: "node invoke timed out" },
|
||||
});
|
||||
}, timeoutMs)
|
||||
: undefined;
|
||||
this.pendingInvokes.set(requestId, {
|
||||
nodeId: params.nodeId,
|
||||
connId: node.connId,
|
||||
@@ -533,7 +546,7 @@ export class NodeRegistry {
|
||||
systemRunEvent,
|
||||
resolve,
|
||||
reject,
|
||||
timer,
|
||||
...(timer !== undefined ? { timer } : {}),
|
||||
});
|
||||
});
|
||||
}
|
||||
@@ -703,7 +716,9 @@ export class NodeRegistry {
|
||||
if (pending.nodeId !== params.nodeId || pending.connId !== params.connId) {
|
||||
return false;
|
||||
}
|
||||
clearTimeout(pending.timer);
|
||||
if (pending.timer !== undefined) {
|
||||
clearTimeout(pending.timer);
|
||||
}
|
||||
this.pendingInvokes.delete(params.id);
|
||||
if (!params.ok && pending.systemRunEvent) {
|
||||
this.forgetAuthorizedSystemRunEvent({
|
||||
|
||||
@@ -161,9 +161,11 @@ describe("attachGatewayWsConnectionHandler", () => {
|
||||
expect(clients.size).toBe(0);
|
||||
});
|
||||
|
||||
it("sends protocol pings until the connection closes", async () => {
|
||||
it("continues protocol pings after pong and stops when the connection closes", async () => {
|
||||
vi.useFakeTimers();
|
||||
const socket = createGatewayWsTestSocket({ ping: true });
|
||||
const socket = Object.assign(createGatewayWsTestSocket({ ping: true }), {
|
||||
terminate: vi.fn(),
|
||||
});
|
||||
const { passed } = await connectTestWs({ socket });
|
||||
const handlerParams = passed as {
|
||||
setClient: (client: unknown) => boolean;
|
||||
@@ -179,10 +181,63 @@ describe("attachGatewayWsConnectionHandler", () => {
|
||||
|
||||
vi.advanceTimersByTime(25_000);
|
||||
expect(socket.ping).toHaveBeenCalledTimes(1);
|
||||
socket.emit("pong");
|
||||
|
||||
vi.advanceTimersByTime(25_000);
|
||||
expect(socket.ping).toHaveBeenCalledTimes(2);
|
||||
expect(socket.terminate).not.toHaveBeenCalled();
|
||||
|
||||
socket.emit("close", 1000, Buffer.from("done"));
|
||||
vi.advanceTimersByTime(25_000);
|
||||
expect(socket.ping).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("terminates a connection after one missed protocol pong", async () => {
|
||||
vi.useFakeTimers();
|
||||
const unregister = vi.fn();
|
||||
const clients = new Set<unknown>();
|
||||
const socket = Object.assign(createGatewayWsTestSocket({ ping: true }), {
|
||||
terminate: vi.fn(),
|
||||
});
|
||||
socket.terminate.mockImplementation(() => {
|
||||
socket.emit("close", 1006, Buffer.from("heartbeat timeout"));
|
||||
});
|
||||
const { passed } = await connectTestWs({
|
||||
clients,
|
||||
socket,
|
||||
options: {
|
||||
buildRequestContext: () =>
|
||||
createGatewayWsTestRequestContext({ nodeRegistry: { unregister } }) as never,
|
||||
},
|
||||
});
|
||||
const handlerParams = passed as {
|
||||
setClient: (client: unknown) => boolean;
|
||||
};
|
||||
expect(
|
||||
handlerParams.setClient({
|
||||
socket,
|
||||
connect: {
|
||||
role: "node",
|
||||
client: { id: "stale-node", mode: "node" },
|
||||
},
|
||||
connId: "stale-node-conn",
|
||||
usesSharedGatewayAuth: false,
|
||||
}),
|
||||
).toBe(true);
|
||||
expect(clients.size).toBe(1);
|
||||
|
||||
vi.advanceTimersByTime(25_000);
|
||||
expect(socket.ping).toHaveBeenCalledTimes(1);
|
||||
expect(socket.terminate).not.toHaveBeenCalled();
|
||||
|
||||
vi.advanceTimersByTime(25_000);
|
||||
expect(socket.terminate).toHaveBeenCalledTimes(1);
|
||||
expect(socket.ping).toHaveBeenCalledTimes(1);
|
||||
expect(unregister).toHaveBeenCalledTimes(1);
|
||||
expect(clients.size).toBe(0);
|
||||
|
||||
vi.advanceTimersByTime(25_000);
|
||||
expect(socket.terminate).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("closes slow consumers before writing direct response frames", async () => {
|
||||
|
||||
@@ -329,6 +329,7 @@ export function attachGatewayWsConnectionHandler(params: AttachGatewayWsConnecti
|
||||
};
|
||||
|
||||
let pingTimer: ReturnType<typeof setInterval> | undefined;
|
||||
let awaitingPong = false;
|
||||
const handshakeTimeoutMs = resolvePreauthHandshakeTimeoutMs({
|
||||
configuredTimeoutMs: params.preauthHandshakeTimeoutMs,
|
||||
});
|
||||
@@ -412,6 +413,10 @@ export function attachGatewayWsConnectionHandler(params: AttachGatewayWsConnecti
|
||||
close();
|
||||
});
|
||||
|
||||
socket.on("pong", () => {
|
||||
awaitingPong = false;
|
||||
});
|
||||
|
||||
const isNoisySwiftPmHelperClose = (userAgent: string | undefined, remote: string | undefined) =>
|
||||
normalizeLowercaseStringOrEmpty(userAgent).includes("swiftpm-testing-helper") &&
|
||||
isLoopbackAddress(remote);
|
||||
@@ -568,6 +573,18 @@ export function attachGatewayWsConnectionHandler(params: AttachGatewayWsConnecti
|
||||
client = next;
|
||||
clients.add(next);
|
||||
pingTimer = setInterval(() => {
|
||||
// A half-open TCP connection can remain OPEN indefinitely. Terminate
|
||||
// after one missed pong so the normal close handler releases node state.
|
||||
if (awaitingPong) {
|
||||
setCloseCause("heartbeat-timeout");
|
||||
try {
|
||||
socket.terminate();
|
||||
} catch {
|
||||
close();
|
||||
}
|
||||
return;
|
||||
}
|
||||
awaitingPong = true;
|
||||
try {
|
||||
socket.ping();
|
||||
} catch {
|
||||
|
||||
Reference in New Issue
Block a user