mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-12 07:20:45 +00:00
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)
This commit is contained in:
committed by
Peter Steinberger
parent
88aee9161e
commit
eea925b12b
145
src/cli/daemon-cli/lifecycle-core.config-guard.test.ts
Normal file
145
src/cli/daemon-cli/lifecycle-core.config-guard.test.ts
Normal file
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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<string | null> {
|
||||
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 });
|
||||
|
||||
Reference in New Issue
Block a user