From f8e673cdbc716a799b33e1aa1a44d02795a38caf Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 17 Jan 2026 10:25:24 +0000 Subject: [PATCH] fix: block invalid config startup Co-authored-by: Muhammed Mukhthar CM --- CHANGELOG.md | 1 + src/cli/program/preaction.ts | 9 +++ src/commands/doctor-config-flow.test.ts | 37 +++++++++++ src/commands/doctor-config-flow.ts | 4 +- src/config/config.backup-rotation.test.ts | 36 +++++++++++ src/config/io.ts | 79 +++++++++++++++++++---- src/gateway/server.impl.ts | 15 ++++- 7 files changed, 164 insertions(+), 17 deletions(-) create mode 100644 src/commands/doctor-config-flow.test.ts create mode 100644 src/config/config.backup-rotation.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 23ddbc1d3..ca0beb27a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,7 @@ Docs: https://docs.clawd.bot - Verbose: wrap tool summaries/output in markdown only for markdown-capable channels. - Telegram: accept tg/group/telegram prefixes + topic targets for inline button validation. (#1072) — thanks @danielz1z. - Telegram: split long captions into follow-up messages. +- Config: block startup on invalid config, preserve best-effort doctor config, and keep rolling config backups. (#1083) — thanks @mukhtharcm. - Sub-agents: normalize announce delivery origin + queue bucketing by accountId to keep multi-account routing stable. (#1061, #1058) — thanks @adam91holt. - Sessions: include deliveryContext in sessions.list and reuse normalized delivery routing for announce/restart fallbacks. (#1058) - Sessions: propagate deliveryContext into last-route updates to keep account/channel routing stable. (#1058) diff --git a/src/cli/program/preaction.ts b/src/cli/program/preaction.ts index 0b5ce458f..1d6c8850f 100644 --- a/src/cli/program/preaction.ts +++ b/src/cli/program/preaction.ts @@ -61,6 +61,15 @@ export function registerPreActionHooks(program: Command, programVersion: string) program.hook("preAction", async (_thisCommand, actionCommand) => { if (actionCommand.name() === "doctor") return; + const snapshot = await readConfigFileSnapshot(); + if (snapshot.exists && !snapshot.valid) { + defaultRuntime.error(`Config invalid at ${snapshot.path}.`); + for (const issue of snapshot.issues) { + defaultRuntime.error(`- ${issue.path || ""}: ${issue.message}`); + } + defaultRuntime.error("Run `clawdbot doctor` to repair, then retry."); + process.exit(1); + } const cfg = loadConfig(); await autoMigrateLegacyState({ cfg }); }); diff --git a/src/commands/doctor-config-flow.test.ts b/src/commands/doctor-config-flow.test.ts new file mode 100644 index 000000000..df7a5a9ee --- /dev/null +++ b/src/commands/doctor-config-flow.test.ts @@ -0,0 +1,37 @@ +import fs from "node:fs/promises"; +import path from "node:path"; + +import { describe, expect, it } from "vitest"; + +import { withTempHome } from "../../test/helpers/temp-home.js"; +import { loadAndMaybeMigrateDoctorConfig } from "./doctor-config-flow.js"; + +describe("doctor config flow", () => { + it("preserves invalid config for doctor repairs", async () => { + await withTempHome(async (home) => { + const configDir = path.join(home, ".clawdbot"); + await fs.mkdir(configDir, { recursive: true }); + await fs.writeFile( + path.join(configDir, "clawdbot.json"), + JSON.stringify( + { + gateway: { auth: { mode: "token", token: 123 } }, + agents: { list: [{ id: "pi" }] }, + }, + null, + 2, + ), + "utf-8", + ); + + const result = await loadAndMaybeMigrateDoctorConfig({ + options: { nonInteractive: true }, + confirm: async () => false, + }); + + expect((result.cfg as Record).gateway).toEqual({ + auth: { mode: "token", token: 123 }, + }); + }); + }); +}); diff --git a/src/commands/doctor-config-flow.ts b/src/commands/doctor-config-flow.ts index 6c658b896..e8b665ec1 100644 --- a/src/commands/doctor-config-flow.ts +++ b/src/commands/doctor-config-flow.ts @@ -46,9 +46,9 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { confirm: (p: { message: string; initialValue: boolean }) => Promise; }) { const snapshot = await readConfigFileSnapshot(); - let cfg: ClawdbotConfig = snapshot.valid ? snapshot.config : {}; + let cfg: ClawdbotConfig = snapshot.config ?? {}; if (snapshot.exists && !snapshot.valid && snapshot.legacyIssues.length === 0) { - note("Config invalid; doctor will run with defaults.", "Config"); + note("Config invalid; doctor will run with best-effort config.", "Config"); } if (snapshot.legacyIssues.length > 0) { diff --git a/src/config/config.backup-rotation.test.ts b/src/config/config.backup-rotation.test.ts new file mode 100644 index 000000000..b98688578 --- /dev/null +++ b/src/config/config.backup-rotation.test.ts @@ -0,0 +1,36 @@ +import fs from "node:fs/promises"; + +import { describe, expect, it } from "vitest"; + +import { withTempHome } from "./test-helpers.js"; +import type { ClawdbotConfig } from "./types.js"; + +describe("config backup rotation", () => { + it("keeps a 5-deep backup ring for config writes", async () => { + await withTempHome(async () => { + const { resolveConfigPath, writeConfigFile } = await import("./config.js"); + const configPath = resolveConfigPath(); + const buildConfig = (version: number): ClawdbotConfig => + ({ + identity: { name: `v${version}` }, + }) as ClawdbotConfig; + + for (let version = 0; version <= 6; version += 1) { + await writeConfigFile(buildConfig(version)); + } + + const readName = async (suffix = "") => { + const raw = await fs.readFile(`${configPath}${suffix}`, "utf-8"); + return (JSON.parse(raw) as { identity?: { name?: string } }).identity?.name ?? null; + }; + + await expect(readName()).resolves.toBe("v6"); + await expect(readName(".bak")).resolves.toBe("v5"); + await expect(readName(".bak.1")).resolves.toBe("v4"); + await expect(readName(".bak.2")).resolves.toBe("v3"); + await expect(readName(".bak.3")).resolves.toBe("v2"); + await expect(readName(".bak.4")).resolves.toBe("v1"); + await expect(fs.stat(`${configPath}.bak.5`)).rejects.toThrow(); + }); + }); +}); diff --git a/src/config/io.ts b/src/config/io.ts index 20cd5c774..1e6e303cf 100644 --- a/src/config/io.ts +++ b/src/config/io.ts @@ -52,6 +52,8 @@ const SHELL_ENV_EXPECTED_KEYS = [ "CLAWDBOT_GATEWAY_PASSWORD", ]; +const CONFIG_BACKUP_COUNT = 5; + export type ParseConfigJson5Result = { ok: true; parsed: unknown } | { ok: false; error: string }; function hashConfigRaw(raw: string | null): string { @@ -73,6 +75,53 @@ export function resolveConfigSnapshotHash(snapshot: { return hashConfigRaw(snapshot.raw); } +function coerceConfig(value: unknown): ClawdbotConfig { + if (!value || typeof value !== "object" || Array.isArray(value)) { + return {}; + } + return value as ClawdbotConfig; +} + +function rotateConfigBackupsSync(configPath: string, ioFs: typeof fs): void { + if (CONFIG_BACKUP_COUNT <= 1) return; + const backupBase = `${configPath}.bak`; + const maxIndex = CONFIG_BACKUP_COUNT - 1; + try { + ioFs.unlinkSync(`${backupBase}.${maxIndex}`); + } catch { + // best-effort + } + for (let index = maxIndex - 1; index >= 1; index -= 1) { + try { + ioFs.renameSync(`${backupBase}.${index}`, `${backupBase}.${index + 1}`); + } catch { + // best-effort + } + } + try { + ioFs.renameSync(backupBase, `${backupBase}.1`); + } catch { + // best-effort + } +} + +async function rotateConfigBackups(configPath: string, ioFs: typeof fs.promises): Promise { + if (CONFIG_BACKUP_COUNT <= 1) return; + const backupBase = `${configPath}.bak`; + const maxIndex = CONFIG_BACKUP_COUNT - 1; + await ioFs.unlink(`${backupBase}.${maxIndex}`).catch(() => { + // best-effort + }); + for (let index = maxIndex - 1; index >= 1; index -= 1) { + await ioFs.rename(`${backupBase}.${index}`, `${backupBase}.${index + 1}`).catch(() => { + // best-effort + }); + } + await ioFs.rename(backupBase, `${backupBase}.1`).catch(() => { + // best-effort + }); +} + export type ConfigIoDeps = { fs?: typeof fs; json5?: typeof JSON5; @@ -165,10 +214,13 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) { deps.fs.writeFileSync(tmp, json, { encoding: "utf-8", mode: 0o600 }); - try { - deps.fs.copyFileSync(configPath, `${configPath}.bak`); - } catch { - // best-effort + if (deps.fs.existsSync(configPath)) { + rotateConfigBackupsSync(configPath, deps.fs); + try { + deps.fs.copyFileSync(configPath, `${configPath}.bak`); + } catch { + // best-effort + } } try { @@ -344,7 +396,7 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) { raw, parsed: parsedRes.parsed, valid: false, - config: {}, + config: coerceConfig(parsedRes.parsed), hash, issues: [{ path: "", message }], legacyIssues: [], @@ -366,7 +418,7 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) { raw, parsed: parsedRes.parsed, valid: false, - config: {}, + config: coerceConfig(resolved), hash, issues: [{ path: "", message }], legacyIssues: [], @@ -379,17 +431,13 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) { const validated = validateConfigObject(resolvedConfigRaw); if (!validated.ok) { - const resolvedConfig = - typeof resolvedConfigRaw === "object" && resolvedConfigRaw !== null - ? (resolvedConfigRaw as ClawdbotConfig) - : {}; return { path: configPath, exists: true, raw, parsed: parsedRes.parsed, valid: false, - config: resolvedConfig, + config: coerceConfig(resolvedConfigRaw), hash, issues: validated.issues, legacyIssues, @@ -450,9 +498,12 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) { mode: 0o600, }); - await deps.fs.promises.copyFile(configPath, `${configPath}.bak`).catch(() => { - // best-effort - }); + if (deps.fs.existsSync(configPath)) { + await rotateConfigBackups(configPath, deps.fs.promises); + await deps.fs.promises.copyFile(configPath, `${configPath}.bak`).catch(() => { + // best-effort + }); + } try { await deps.fs.promises.rename(tmp, configPath); diff --git a/src/gateway/server.impl.ts b/src/gateway/server.impl.ts index 2e8be05a5..89dad77b3 100644 --- a/src/gateway/server.impl.ts +++ b/src/gateway/server.impl.ts @@ -134,7 +134,7 @@ export async function startGatewayServer( // Ensure all default port derivations (browser/bridge/canvas) see the actual runtime port. process.env.CLAWDBOT_GATEWAY_PORT = String(port); - const configSnapshot = await readConfigFileSnapshot(); + let configSnapshot = await readConfigFileSnapshot(); if (configSnapshot.legacyIssues.length > 0) { if (isNixMode) { throw new Error( @@ -157,6 +157,19 @@ export async function startGatewayServer( } } + configSnapshot = await readConfigFileSnapshot(); + if (configSnapshot.exists && !configSnapshot.valid) { + const issues = + configSnapshot.issues.length > 0 + ? configSnapshot.issues + .map((issue) => `${issue.path || ""}: ${issue.message}`) + .join("\n") + : "Unknown validation issue."; + throw new Error( + `Invalid config at ${configSnapshot.path}.\n${issues}\nRun "clawdbot doctor" to repair, then retry.`, + ); + } + const cfgAtStart = loadConfig(); initSubagentRegistry(); await autoMigrateLegacyState({ cfg: cfgAtStart, log });