diff --git a/src/cli/dns-cli.test.ts b/src/cli/dns-cli.test.ts index 8e57f9ff70ac..4beeb2b2496f 100644 --- a/src/cli/dns-cli.test.ts +++ b/src/cli/dns-cli.test.ts @@ -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 { 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(); } }); }); diff --git a/src/cli/dns-cli.ts b/src/cli/dns-cli.ts index bd900d48ce00..d71b35b5a4c2 100644 --- a/src/cli/dns-cli.ts +++ b/src/cli/dns-cli.ts @@ -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");