fix(cli): scope dns setup timeout to brew prefix probe with SIGKILL

Make the subprocess timeout opt-in so long-running setup steps (brew
install, service restart, sudo writes) keep their unbounded wait, and
apply a 15s SIGKILL-backed deadline only to the fast brew --prefix probe
so a signal-resistant hung shim cannot block dns setup.

Also fix the regression test on Linux CI: stub process.platform via
withMockedPlatform (the action does not consult os.platform()) and point
the mocked brew prefix at a real temp dir so un-mocked fs writes succeed
without sudo. Add focused coverage that non-probe subprocesses stay
unbounded.
This commit is contained in:
thomas.szbay
2026-07-19 06:42:12 +08:00
parent 23b4577c27
commit 469ae1db40
2 changed files with 73 additions and 45 deletions
+64 -41
View File
@@ -1,8 +1,13 @@
// Regression: dns-cli subprocess probes (DNS lookup + sudo tee) must be bounded
// by a timeout so a hung binary cannot block `openclaw dns setup`.
import { afterEach, describe, expect, it, vi } from "vitest";
// Regression: the dns setup brew-prefix probe must be bounded by a SIGKILL-backed
// timeout so a hung binary cannot block `openclaw dns setup`, while long-running
// setup steps (install/restart/sudo writes) stay unbounded.
import fs from "node:fs";
import os from "node:os";
import path from "node:path";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
const spawnSyncMock = vi.hoisted(() => vi.fn());
const testState = vi.hoisted(() => ({ zonePath: "" }));
vi.mock("node:child_process", async () => {
const { mockNodeChildProcessSpawnSync } = await import("openclaw/plugin-sdk/test-node-mocks");
@@ -13,7 +18,7 @@ vi.mock("node:child_process", async () => {
vi.mock("../infra/widearea-dns.js", async () => {
return {
getWideAreaZonePath: () => "/tmp/openclaw-dns-test.zone",
getWideAreaZonePath: () => testState.zonePath,
normalizeWideAreaDomain: (d: string) => d,
resolveWideAreaDiscoveryDomain: () => "openclaw.internal.",
};
@@ -30,56 +35,74 @@ vi.mock("../config/config.js", async () => {
return { getRuntimeConfig: () => ({}) };
});
import os from "node:os";
import { Command } from "commander";
import { withMockedPlatform } from "../test-utils/vitest-spies.js";
import { registerDnsCli } from "./dns-cli.js";
function spawnOk(stdout = "") {
return { stdout, stderr: "", pid: 1, output: [], status: 0, signal: null };
}
function isBrewPrefixProbe(call: unknown[]): boolean {
const args = call[1] as string[] | undefined;
return call[0] === "brew" && args?.[0] === "--prefix";
}
describe("dns-cli probe bounds", () => {
let brewPrefix: string;
beforeEach(() => {
// A real writable temp prefix lets the un-mocked fs writes (Corefile, conf.d,
// zone bootstrap) succeed on Linux CI without touching sudo paths.
brewPrefix = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-dns-cli-test-"));
testState.zonePath = path.join(brewPrefix, "test.zone");
spawnSyncMock.mockReset();
spawnSyncMock.mockImplementation((cmd: string, args: string[]) => {
if (cmd === "brew" && args?.[0] === "--prefix") {
return spawnOk(`${brewPrefix}\n`);
}
return spawnOk();
});
});
afterEach(() => {
fs.rmSync(brewPrefix, { recursive: true, force: true });
vi.restoreAllMocks();
});
it("passes a timeout to spawned dns setup subprocesses", async () => {
vi.spyOn(os, "platform").mockReturnValue("darwin");
spawnSyncMock.mockImplementation((cmd: string, args: string[]) => {
if (cmd === "brew" && args?.[0] === "--prefix") {
return {
stdout: "/opt/homebrew",
stderr: "",
pid: 1,
output: [],
status: 0,
signal: null,
};
}
return {
stdout: "",
stderr: "",
pid: 1,
output: [],
status: 0,
signal: null,
};
});
async function runDnsSetupApply(): Promise<void> {
const program = new Command();
program.exitOverride();
registerDnsCli(program);
// The action gates on process.platform; os.platform() is not consulted.
await withMockedPlatform("darwin", () =>
program.parseAsync([
"node",
"openclaw",
"dns",
"setup",
"--domain",
"openclaw.internal",
"--apply",
]),
);
}
await program.parseAsync([
"node",
"openclaw",
"dns",
"setup",
"--domain",
"openclaw.internal",
"--apply",
]);
it("bounds the brew prefix probe with a SIGKILL-backed timeout", async () => {
await runDnsSetupApply();
const spawned = spawnSyncMock.mock.calls;
expect(spawned.length).toBeGreaterThan(0);
for (const call of spawned) {
expect(call[2]?.timeout).toBeGreaterThan(0);
const probeCall = spawnSyncMock.mock.calls.find(isBrewPrefixProbe);
expect(probeCall).toBeDefined();
expect(probeCall?.[2]).toMatchObject({ timeout: 15_000, killSignal: "SIGKILL" });
});
it("leaves long-running setup subprocesses unbounded", async () => {
await runDnsSetupApply();
const nonProbeCalls = spawnSyncMock.mock.calls.filter((call) => !isBrewPrefixProbe(call));
expect(nonProbeCalls.length).toBeGreaterThan(0);
for (const call of nonProbeCalls) {
expect(call[2]?.timeout).toBeUndefined();
}
});
});
+9 -4
View File
@@ -15,13 +15,18 @@ import {
} from "../infra/widearea-dns.js";
import { defaultRuntime } from "../runtime.js";
type RunOpts = { allowFailure?: boolean; inherit?: boolean };
type RunOpts = { allowFailure?: boolean; inherit?: boolean; timeoutMs?: number };
function run(cmd: string, args: string[], opts?: RunOpts): string {
const res = spawnSync(cmd, args, {
encoding: "utf-8",
stdio: opts?.inherit ? "inherit" : "pipe",
timeout: 15_000,
// Timeout stays opt-in: install/restart/sudo-write steps may legitimately run
// long, so only fast probes pass a deadline. SIGKILL guarantees a
// signal-resistant hung probe still dies at the deadline.
...(opts?.timeoutMs === undefined
? {}
: { timeout: opts.timeoutMs, killSignal: "SIGKILL" as const }),
});
if (res.error) {
throw res.error;
@@ -52,7 +57,6 @@ function writeFileSudoIfNeeded(filePath: string, content: string): void {
input: content,
encoding: "utf-8",
stdio: ["pipe", "ignore", "inherit"],
timeout: 15_000,
});
if (res.error) {
throw res.error;
@@ -89,7 +93,8 @@ function zoneFileNeedsBootstrap(zonePath: string): boolean {
}
function detectBrewPrefix(): string {
const out = run("brew", ["--prefix"]);
// A hung brew shim can block setup indefinitely; bound only this fast probe.
const out = run("brew", ["--prefix"], { timeoutMs: 15_000 });
const prefix = out.trim();
if (!prefix) {
throw new Error("failed to resolve Homebrew prefix");