mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-09 16:53:55 +00:00
Doctor: expose write-config blocker findings (#100093)
* doctor: add write-config lint findings * doctor: align write-config lint with config writes * doctor: check config write directory permissions * doctor: detect blocked config directory paths * doctor: detect symlink config directory blockers * doctor: require searchable config write parents
This commit is contained in:
@@ -658,6 +658,7 @@ describe("doctor health contributions", () => {
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllEnvs();
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
@@ -2703,6 +2704,260 @@ describe("doctor health contributions", () => {
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
describe("write-config lint findings", () => {
|
||||
const writeConfigContribution = requireDoctorContribution("doctor:write-config");
|
||||
const check = writeConfigContribution.healthChecks[0] as HealthCheck & {
|
||||
defaultEnabled?: boolean;
|
||||
};
|
||||
|
||||
it("keeps write-config lint opt-in for structured findings", async () => {
|
||||
expect(writeConfigContribution.healthCheckIds).toEqual(["core/doctor/write-config"]);
|
||||
expect(check.defaultEnabled).toBe(false);
|
||||
|
||||
const ctx = {
|
||||
cfg: {},
|
||||
mode: "lint" as const,
|
||||
runtime: { log: vi.fn(), error: vi.fn(), exit: vi.fn() },
|
||||
configPath: "/tmp/fake-openclaw.json",
|
||||
};
|
||||
|
||||
await expect(runDoctorLintChecks(ctx, { checks: [check] })).resolves.toMatchObject({
|
||||
checksRun: 0,
|
||||
checksSkipped: 1,
|
||||
findings: [],
|
||||
});
|
||||
});
|
||||
|
||||
it("reports Nix immutable config mode when selected", async () => {
|
||||
vi.stubEnv("OPENCLAW_NIX_MODE", "1");
|
||||
|
||||
await expect(
|
||||
runDoctorLintChecks(
|
||||
{
|
||||
cfg: {},
|
||||
mode: "lint" as const,
|
||||
runtime: { log: vi.fn(), error: vi.fn(), exit: vi.fn() },
|
||||
configPath: "/tmp/fake-openclaw.json",
|
||||
},
|
||||
{ checks: [check], onlyIds: ["core/doctor/write-config"] },
|
||||
),
|
||||
).resolves.toMatchObject({
|
||||
checksRun: 1,
|
||||
checksSkipped: 0,
|
||||
findings: [
|
||||
expect.objectContaining({
|
||||
checkId: "core/doctor/write-config",
|
||||
path: "/tmp/fake-openclaw.json",
|
||||
requirement: "mutable-config-write-path",
|
||||
}),
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
it("skips a read-only existing config when its directory is writable", async () => {
|
||||
const configPath = "/tmp/openclaw-home/openclaw.json";
|
||||
vi.spyOn(fs, "existsSync").mockImplementation((path) => path === configPath);
|
||||
vi.spyOn(fs, "statSync").mockReturnValue({
|
||||
isDirectory: () => true,
|
||||
} as fs.Stats);
|
||||
const accessSpy = vi.spyOn(fs, "accessSync").mockImplementation(() => undefined);
|
||||
|
||||
await expect(
|
||||
runDoctorLintChecks(
|
||||
{
|
||||
cfg: {},
|
||||
mode: "lint" as const,
|
||||
runtime: { log: vi.fn(), error: vi.fn(), exit: vi.fn() },
|
||||
configPath,
|
||||
},
|
||||
{ checks: [check], onlyIds: ["core/doctor/write-config"] },
|
||||
),
|
||||
).resolves.toMatchObject({
|
||||
findings: [],
|
||||
});
|
||||
expect(accessSpy).toHaveBeenCalledWith(
|
||||
"/tmp/openclaw-home",
|
||||
fs.constants.W_OK | fs.constants.X_OK,
|
||||
);
|
||||
});
|
||||
|
||||
it("reports an unwritable config directory for an existing config", async () => {
|
||||
const configPath = "/tmp/openclaw-home/openclaw.json";
|
||||
vi.spyOn(fs, "existsSync").mockImplementation((path) => path === configPath);
|
||||
vi.spyOn(fs, "statSync").mockReturnValue({
|
||||
isDirectory: () => true,
|
||||
} as fs.Stats);
|
||||
vi.spyOn(fs, "accessSync").mockImplementation(() => {
|
||||
throw new Error("EACCES");
|
||||
});
|
||||
|
||||
await expect(
|
||||
runDoctorLintChecks(
|
||||
{
|
||||
cfg: {},
|
||||
mode: "lint" as const,
|
||||
runtime: { log: vi.fn(), error: vi.fn(), exit: vi.fn() },
|
||||
configPath,
|
||||
},
|
||||
{ checks: [check], onlyIds: ["core/doctor/write-config"] },
|
||||
),
|
||||
).resolves.toMatchObject({
|
||||
findings: [
|
||||
expect.objectContaining({
|
||||
checkId: "core/doctor/write-config",
|
||||
path: "/tmp/openclaw-home",
|
||||
target: configPath,
|
||||
requirement: "writable-config-directory",
|
||||
}),
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
it("skips a missing config directory when an existing ancestor is writable", async () => {
|
||||
vi.spyOn(fs, "existsSync").mockImplementation((path) => path === "/tmp");
|
||||
const accessSpy = vi.spyOn(fs, "accessSync").mockImplementation(() => undefined);
|
||||
|
||||
await expect(
|
||||
runDoctorLintChecks(
|
||||
{
|
||||
cfg: {},
|
||||
mode: "lint" as const,
|
||||
runtime: { log: vi.fn(), error: vi.fn(), exit: vi.fn() },
|
||||
configPath: "/tmp/openclaw-home/openclaw.json",
|
||||
},
|
||||
{ checks: [check], onlyIds: ["core/doctor/write-config"] },
|
||||
),
|
||||
).resolves.toMatchObject({
|
||||
findings: [],
|
||||
});
|
||||
expect(accessSpy).toHaveBeenCalledWith("/tmp", fs.constants.W_OK | fs.constants.X_OK);
|
||||
});
|
||||
|
||||
it("reports an unwritable existing parent when the config file is missing", async () => {
|
||||
vi.spyOn(fs, "existsSync").mockImplementation((path) => path === "/tmp");
|
||||
vi.spyOn(fs, "accessSync").mockImplementation(() => {
|
||||
throw new Error("EACCES");
|
||||
});
|
||||
|
||||
await expect(
|
||||
runDoctorLintChecks(
|
||||
{
|
||||
cfg: {},
|
||||
mode: "lint" as const,
|
||||
runtime: { log: vi.fn(), error: vi.fn(), exit: vi.fn() },
|
||||
configPath: "/tmp/openclaw-home/openclaw.json",
|
||||
},
|
||||
{ checks: [check], onlyIds: ["core/doctor/write-config"] },
|
||||
),
|
||||
).resolves.toMatchObject({
|
||||
findings: [
|
||||
expect.objectContaining({
|
||||
checkId: "core/doctor/write-config",
|
||||
path: "/tmp",
|
||||
target: "/tmp/openclaw-home",
|
||||
requirement: "writable-config-directory",
|
||||
}),
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
it("reports an existing parent without search permission", async () => {
|
||||
vi.spyOn(fs, "existsSync").mockImplementation((path) => path === "/tmp");
|
||||
vi.spyOn(fs, "accessSync").mockImplementation((_path, mode) => {
|
||||
if (mode === (fs.constants.W_OK | fs.constants.X_OK)) {
|
||||
throw new Error("EACCES");
|
||||
}
|
||||
});
|
||||
|
||||
await expect(
|
||||
runDoctorLintChecks(
|
||||
{
|
||||
cfg: {},
|
||||
mode: "lint" as const,
|
||||
runtime: { log: vi.fn(), error: vi.fn(), exit: vi.fn() },
|
||||
configPath: "/tmp/openclaw-home/openclaw.json",
|
||||
},
|
||||
{ checks: [check], onlyIds: ["core/doctor/write-config"] },
|
||||
),
|
||||
).resolves.toMatchObject({
|
||||
findings: [
|
||||
expect.objectContaining({
|
||||
checkId: "core/doctor/write-config",
|
||||
path: "/tmp",
|
||||
target: "/tmp/openclaw-home",
|
||||
requirement: "writable-config-directory",
|
||||
}),
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
it("reports an existing file that blocks the config directory path", async () => {
|
||||
vi.spyOn(fs, "existsSync").mockImplementation((path) => path === "/tmp/openclaw-home");
|
||||
vi.spyOn(fs, "statSync").mockReturnValue({
|
||||
isDirectory: () => false,
|
||||
} as fs.Stats);
|
||||
const accessSpy = vi.spyOn(fs, "accessSync").mockImplementation(() => undefined);
|
||||
|
||||
await expect(
|
||||
runDoctorLintChecks(
|
||||
{
|
||||
cfg: {},
|
||||
mode: "lint" as const,
|
||||
runtime: { log: vi.fn(), error: vi.fn(), exit: vi.fn() },
|
||||
configPath: "/tmp/openclaw-home/openclaw.json",
|
||||
},
|
||||
{ checks: [check], onlyIds: ["core/doctor/write-config"] },
|
||||
),
|
||||
).resolves.toMatchObject({
|
||||
findings: [
|
||||
expect.objectContaining({
|
||||
checkId: "core/doctor/write-config",
|
||||
path: "/tmp/openclaw-home",
|
||||
target: "/tmp/openclaw-home",
|
||||
requirement: "config-directory-path",
|
||||
}),
|
||||
],
|
||||
});
|
||||
expect(accessSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("reports a dangling symlink that blocks the config directory path", async () => {
|
||||
vi.spyOn(fs, "existsSync").mockImplementation((path) => path === "/tmp");
|
||||
vi.spyOn(fs, "lstatSync").mockImplementation((path) => {
|
||||
if (path === "/tmp/openclaw-home") {
|
||||
return { isDirectory: () => false } as fs.Stats;
|
||||
}
|
||||
throw Object.assign(new Error("ENOENT"), { code: "ENOENT" });
|
||||
});
|
||||
vi.spyOn(fs, "statSync").mockImplementation(() => {
|
||||
throw Object.assign(new Error("ENOENT"), { code: "ENOENT" });
|
||||
});
|
||||
const accessSpy = vi.spyOn(fs, "accessSync").mockImplementation(() => undefined);
|
||||
|
||||
await expect(
|
||||
runDoctorLintChecks(
|
||||
{
|
||||
cfg: {},
|
||||
mode: "lint" as const,
|
||||
runtime: { log: vi.fn(), error: vi.fn(), exit: vi.fn() },
|
||||
configPath: "/tmp/openclaw-home/openclaw.json",
|
||||
},
|
||||
{ checks: [check], onlyIds: ["core/doctor/write-config"] },
|
||||
),
|
||||
).resolves.toMatchObject({
|
||||
findings: [
|
||||
expect.objectContaining({
|
||||
checkId: "core/doctor/write-config",
|
||||
path: "/tmp/openclaw-home",
|
||||
target: "/tmp/openclaw-home",
|
||||
requirement: "config-directory-path",
|
||||
}),
|
||||
],
|
||||
});
|
||||
expect(accessSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("config size drops during update", () => {
|
||||
beforeEach(() => {
|
||||
mocks.replaceConfigFile.mockReset();
|
||||
|
||||
@@ -1,11 +1,13 @@
|
||||
// Doctor health contribution helpers collect health checks from plugin manifests.
|
||||
import fs from "node:fs";
|
||||
import nodePath from "node:path";
|
||||
import type { probeGatewayMemoryStatus } from "../commands/doctor-gateway-health.js";
|
||||
import type { DoctorOptions, DoctorPrompter } from "../commands/doctor-prompter.js";
|
||||
import {
|
||||
isLegacyParentWritableUpdateDoctorPass,
|
||||
UPDATE_PARENT_SUPPORTS_DOCTOR_CONFIG_WRITE_ENV,
|
||||
} from "../commands/doctor/shared/update-phase.js";
|
||||
import { resolveIsNixMode } from "../config/paths.js";
|
||||
import type { OpenClawConfig } from "../config/types.openclaw.js";
|
||||
import type { buildGatewayConnectionDetails } from "../gateway/call.js";
|
||||
import type { UpdatePostInstallDoctorResult } from "../infra/update-doctor-result.js";
|
||||
@@ -1308,6 +1310,93 @@ async function runWriteConfigHealth(ctx: DoctorHealthFlowContext): Promise<void>
|
||||
}
|
||||
}
|
||||
|
||||
async function collectWriteConfigHealthFindings(
|
||||
ctx: Parameters<HealthCheck["detect"]>[0],
|
||||
): Promise<readonly HealthFinding[]> {
|
||||
const findings: HealthFinding[] = [];
|
||||
const configPath = ctx.configPath;
|
||||
if (resolveIsNixMode(process.env)) {
|
||||
findings.push({
|
||||
checkId: "core/doctor/write-config",
|
||||
severity: "warning",
|
||||
message: "Doctor config writes are disabled because OpenClaw is running in Nix mode.",
|
||||
...(configPath ? { path: configPath } : {}),
|
||||
requirement: "mutable-config-write-path",
|
||||
fixHint:
|
||||
"Edit the Nix source for this install and rebuild; do not run doctor --fix against this config file.",
|
||||
});
|
||||
}
|
||||
if (!configPath) {
|
||||
return findings;
|
||||
}
|
||||
const configDirectory = nodePath.dirname(configPath);
|
||||
const configPathExists = fs.existsSync(configPath);
|
||||
const existingParent = configPathExists
|
||||
? configDirectory
|
||||
: findNearestExistingParent(configDirectory);
|
||||
if (!isDirectoryPath(existingParent)) {
|
||||
findings.push({
|
||||
checkId: "core/doctor/write-config",
|
||||
severity: "warning",
|
||||
message: "Doctor cannot create the config directory because a path component is a file.",
|
||||
path: existingParent,
|
||||
target: configDirectory,
|
||||
requirement: "config-directory-path",
|
||||
fixHint: "Move the file blocking the config directory path before running doctor --fix.",
|
||||
});
|
||||
return findings;
|
||||
}
|
||||
try {
|
||||
fs.accessSync(existingParent, fs.constants.W_OK | fs.constants.X_OK);
|
||||
} catch {
|
||||
findings.push({
|
||||
checkId: "core/doctor/write-config",
|
||||
severity: "warning",
|
||||
message: configPathExists
|
||||
? "Doctor cannot write config because the config directory is not writable."
|
||||
: "Doctor cannot create the config directory because the nearest existing parent is not writable.",
|
||||
path: existingParent,
|
||||
target: configPathExists ? configPath : configDirectory,
|
||||
requirement: "writable-config-directory",
|
||||
fixHint:
|
||||
"Make the existing config directory or parent directory writable before running doctor --fix.",
|
||||
});
|
||||
}
|
||||
return findings;
|
||||
}
|
||||
|
||||
function findNearestExistingParent(path: string): string {
|
||||
let candidate = path;
|
||||
while (!pathEntryExists(candidate)) {
|
||||
const parent = nodePath.dirname(candidate);
|
||||
if (parent === candidate) {
|
||||
return candidate;
|
||||
}
|
||||
candidate = parent;
|
||||
}
|
||||
return candidate;
|
||||
}
|
||||
|
||||
function pathEntryExists(path: string): boolean {
|
||||
if (fs.existsSync(path)) {
|
||||
return true;
|
||||
}
|
||||
try {
|
||||
fs.lstatSync(path);
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
function isDirectoryPath(path: string): boolean {
|
||||
try {
|
||||
return fs.statSync(path).isDirectory();
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
async function runWorkspaceSuggestionsHealth(ctx: DoctorHealthFlowContext): Promise<void> {
|
||||
if (ctx.options.workspaceSuggestions === false) {
|
||||
return;
|
||||
@@ -2042,6 +2131,12 @@ export function resolveDoctorHealthContributions(): DoctorHealthContribution[] {
|
||||
createDoctorHealthContribution({
|
||||
id: "doctor:write-config",
|
||||
label: "Write config",
|
||||
healthChecks: {
|
||||
id: "core/doctor/write-config",
|
||||
description: "Config write blockers are findings before doctor repair writes.",
|
||||
defaultEnabled: false,
|
||||
detect: collectWriteConfigHealthFindings,
|
||||
},
|
||||
run: runWriteConfigHealth,
|
||||
}),
|
||||
createDoctorHealthContribution({
|
||||
|
||||
Reference in New Issue
Block a user