From eea925b12b0f4f0a8dfa9a3680daf251bbca0c7c Mon Sep 17 00:00:00 2001 From: merlin Date: Thu, 5 Mar 2026 18:17:58 +0800 Subject: [PATCH] fix(gateway): validate config before restart to prevent crash + macOS permission loss (#35862) When 'openclaw gateway restart' is run with an invalid config, the new process crashes on startup due to config validation failure. On macOS, this causes Full Disk Access (TCC) permissions to be lost because the respawned process has a different PID. Add getConfigValidationError() helper and pre-flight config validation in both runServiceRestart() and runServiceStart(). If config is invalid, abort with a clear error message instead of crashing. The config watcher's hot-reload path already had this guard (handleInvalidSnapshot), but the CLI restart/start commands did not. AI-assisted (OpenClaw agent, fully tested) --- .../lifecycle-core.config-guard.test.ts | 145 ++++++++++++++++++ src/cli/daemon-cli/lifecycle-core.ts | 44 +++++- 2 files changed, 188 insertions(+), 1 deletion(-) create mode 100644 src/cli/daemon-cli/lifecycle-core.config-guard.test.ts diff --git a/src/cli/daemon-cli/lifecycle-core.config-guard.test.ts b/src/cli/daemon-cli/lifecycle-core.config-guard.test.ts new file mode 100644 index 00000000000..4c1f1a53537 --- /dev/null +++ b/src/cli/daemon-cli/lifecycle-core.config-guard.test.ts @@ -0,0 +1,145 @@ +import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; + +const readConfigFileSnapshotMock = vi.fn(); +const loadConfig = vi.fn(() => ({})); + +const runtimeLogs: string[] = []; +const defaultRuntime = { + log: (message: string) => runtimeLogs.push(message), + error: vi.fn(), + exit: (code: number) => { + throw new Error(`__exit__:${code}`); + }, +}; + +const service = { + label: "TestService", + loadedText: "loaded", + notLoadedText: "not loaded", + install: vi.fn(), + uninstall: vi.fn(), + stop: vi.fn(), + isLoaded: vi.fn(), + readCommand: vi.fn(), + readRuntime: vi.fn(), + restart: vi.fn(), +}; + +vi.mock("../../config/config.js", () => ({ + loadConfig: () => loadConfig(), + readConfigFileSnapshot: () => readConfigFileSnapshotMock(), +})); + +vi.mock("../../config/issue-format.js", () => ({ + formatConfigIssueLines: ( + issues: Array<{ path: string; message: string }>, + _prefix: string, + _opts?: unknown, + ) => issues.map((i) => `${i.path}: ${i.message}`), +})); + +vi.mock("../../runtime.js", () => ({ + defaultRuntime, +})); + +describe("runServiceRestart config pre-flight (#35862)", () => { + let runServiceRestart: typeof import("./lifecycle-core.js").runServiceRestart; + + beforeAll(async () => { + ({ runServiceRestart } = await import("./lifecycle-core.js")); + }); + + beforeEach(() => { + runtimeLogs.length = 0; + readConfigFileSnapshotMock.mockReset(); + readConfigFileSnapshotMock.mockResolvedValue({ + exists: true, + valid: true, + config: {}, + issues: [], + }); + loadConfig.mockReset(); + loadConfig.mockReturnValue({}); + service.isLoaded.mockClear(); + service.readCommand.mockClear(); + service.restart.mockClear(); + service.isLoaded.mockResolvedValue(true); + service.readCommand.mockResolvedValue({ environment: {} }); + service.restart.mockResolvedValue(undefined); + vi.unstubAllEnvs(); + vi.stubEnv("OPENCLAW_GATEWAY_TOKEN", ""); + vi.stubEnv("CLAWDBOT_GATEWAY_TOKEN", ""); + }); + + it("aborts restart when config is invalid", async () => { + readConfigFileSnapshotMock.mockResolvedValue({ + exists: true, + valid: false, + config: {}, + issues: [{ path: "agents.defaults.pdfModel", message: "Unrecognized key" }], + }); + + await expect( + runServiceRestart({ + serviceNoun: "Gateway", + service, + renderStartHints: () => [], + opts: { json: true }, + }), + ).rejects.toThrow("__exit__:1"); + + expect(service.restart).not.toHaveBeenCalled(); + }); + + it("proceeds with restart when config is valid", async () => { + readConfigFileSnapshotMock.mockResolvedValue({ + exists: true, + valid: true, + config: {}, + issues: [], + }); + + const result = await runServiceRestart({ + serviceNoun: "Gateway", + service, + renderStartHints: () => [], + opts: { json: true }, + }); + + expect(result).toBe(true); + expect(service.restart).toHaveBeenCalledTimes(1); + }); + + it("proceeds with restart when config file does not exist", async () => { + readConfigFileSnapshotMock.mockResolvedValue({ + exists: false, + valid: true, + config: {}, + issues: [], + }); + + const result = await runServiceRestart({ + serviceNoun: "Gateway", + service, + renderStartHints: () => [], + opts: { json: true }, + }); + + expect(result).toBe(true); + expect(service.restart).toHaveBeenCalledTimes(1); + }); + + it("proceeds with restart when snapshot read throws", async () => { + readConfigFileSnapshotMock.mockRejectedValue(new Error("read failed")); + + const result = await runServiceRestart({ + serviceNoun: "Gateway", + service, + renderStartHints: () => [], + opts: { json: true }, + }); + + expect(result).toBe(true); + expect(service.restart).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/cli/daemon-cli/lifecycle-core.ts b/src/cli/daemon-cli/lifecycle-core.ts index 00d70f24a8a..012bcbac0c3 100644 --- a/src/cli/daemon-cli/lifecycle-core.ts +++ b/src/cli/daemon-cli/lifecycle-core.ts @@ -1,5 +1,6 @@ import type { Writable } from "node:stream"; -import { readBestEffortConfig } from "../../config/config.js"; +import { readBestEffortConfig, readConfigFileSnapshot } from "../../config/config.js"; +import { formatConfigIssueLines } from "../../config/issue-format.js"; import { resolveIsNixMode } from "../../config/paths.js"; import { checkTokenDrift } from "../../daemon/service-audit.js"; import type { GatewayService } from "../../daemon/service.js"; @@ -107,6 +108,25 @@ async function resolveServiceLoadedOrFail(params: { } } +/** + * Best-effort config validation. Returns a string describing the issues if + * config exists and is invalid, or null if config is valid/missing/unreadable. + * (#35862) + */ +async function getConfigValidationError(): Promise { + try { + const snapshot = await readConfigFileSnapshot(); + if (!snapshot.exists || snapshot.valid) { + return null; + } + return snapshot.issues.length > 0 + ? formatConfigIssueLines(snapshot.issues, "", { normalizeRoot: true }).join("\n") + : "Unknown validation issue."; + } catch { + return null; + } +} + export async function runServiceUninstall(params: { serviceNoun: string; service: GatewayService; @@ -187,6 +207,17 @@ export async function runServiceStart(params: { }); return; } + // Pre-flight config validation (#35862) + { + const configError = await getConfigValidationError(); + if (configError) { + fail( + `${params.serviceNoun} aborted: config is invalid.\n${configError}\nFix the config and retry, or run "openclaw doctor" to repair.`, + ); + return false; + } + } + try { await params.service.restart({ env: process.env, stdout }); } catch (err) { @@ -353,6 +384,17 @@ export async function runServiceRestart(params: { } } + // Pre-flight config validation (#35862) + { + const configError = await getConfigValidationError(); + if (configError) { + fail( + `${params.serviceNoun} aborted: config is invalid.\n${configError}\nFix the config and retry, or run "openclaw doctor" to repair.`, + ); + return false; + } + } + try { if (loaded) { await params.service.restart({ env: process.env, stdout });