diff --git a/CHANGELOG.md b/CHANGELOG.md index 3099e069c91..44d6db3c2bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ Docs: https://docs.openclaw.ai ### Fixes - Agents/sessions: keep delayed `sessions_send` A2A replies alive after soft wait-window timeouts, while preserving terminal run timeouts and avoiding stale target replies in requester sessions. Fixes #76443. Thanks @ryswork1993 and @vincentkoc. +- Config/doctor: cap `.clobbered.*` forensic snapshots per config path and serialize snapshot writes so repeated `doctor --fix` recovery loops cannot flood the config directory. Fixes #76454; carries forward #65649. Thanks @JUSTICEESSIELP, @rsnow, and @vincentkoc. - Channels/secrets: resolve SecretRef-backed channel credentials through external plugin secret contracts after the plugin split, covering runtime startup, target discovery, webhook auth, disabled-account enumeration, and late-bound web_search config. Fixes #76371. (#76449) Thanks @joshavant and @neeravmakwana. - Docker/Gateway: pass Docker setup `.env` values into gateway and CLI containers and preserve exec SecretRef `passEnv` keys in managed service plans, so 1Password Connect-backed Discord tokens keep resolving after doctor or plugin repair. Thanks @vincentkoc. - Control UI/WebChat: explain compaction boundaries in chat history and link directly to session checkpoint controls so pre-compaction turns no longer look silently lost after refresh. Fixes #76415. Thanks @BunsDev. diff --git a/src/config/io.clobber-snapshot.test.ts b/src/config/io.clobber-snapshot.test.ts new file mode 100644 index 00000000000..f9a8f4d67c0 --- /dev/null +++ b/src/config/io.clobber-snapshot.test.ts @@ -0,0 +1,86 @@ +import fs from "node:fs"; +import fsp from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; +import { + CONFIG_CLOBBER_SNAPSHOT_LIMIT, + persistBoundedClobberedConfigSnapshot, + persistBoundedClobberedConfigSnapshotSync, +} from "./io.clobber-snapshot.js"; + +describe("config clobber snapshots", () => { + let fixtureRoot = ""; + let caseId = 0; + + beforeAll(async () => { + fixtureRoot = await fsp.mkdtemp(path.join(os.tmpdir(), "openclaw-config-clobber-")); + }); + + afterAll(async () => { + await fsp.rm(fixtureRoot, { recursive: true, force: true }); + }); + + async function withCase(fn: (configPath: string) => Promise): Promise { + const home = path.join(fixtureRoot, `case-${caseId++}`); + const configPath = path.join(home, ".openclaw", "openclaw.json"); + await fsp.mkdir(path.dirname(configPath), { recursive: true }); + await fsp.writeFile(configPath, "{}\n", "utf-8"); + return await fn(configPath); + } + + async function listClobberFiles(configPath: string): Promise { + const entries = await fsp.readdir(path.dirname(configPath)); + const prefix = `${path.basename(configPath)}.clobbered.`; + return entries.filter((entry) => entry.startsWith(prefix)); + } + + it("keeps concurrent async snapshots under the per-path cap", async () => { + await withCase(async (configPath) => { + const warn = vi.fn(); + const observedAt = "2026-05-03T00:00:00.000Z"; + + await Promise.all( + Array.from({ length: CONFIG_CLOBBER_SNAPSHOT_LIMIT + 24 }, async (_, index) => { + await persistBoundedClobberedConfigSnapshot({ + deps: { fs, logger: { warn } }, + configPath, + raw: `polluted-${index}\n`, + observedAt, + }); + }), + ); + + const clobberFiles = await listClobberFiles(configPath); + expect(clobberFiles).toHaveLength(CONFIG_CLOBBER_SNAPSHOT_LIMIT); + const capWarnings = warn.mock.calls.filter( + ([message]) => + typeof message === "string" && message.includes("Config clobber snapshot cap reached"), + ); + expect(capWarnings).toHaveLength(1); + }); + }); + + it("keeps sync snapshots under the per-path cap and warns once", async () => { + await withCase(async (configPath) => { + const warn = vi.fn(); + + for (let index = 0; index < CONFIG_CLOBBER_SNAPSHOT_LIMIT + 3; index++) { + persistBoundedClobberedConfigSnapshotSync({ + deps: { fs, logger: { warn } }, + configPath, + raw: `polluted-${index}\n`, + observedAt: `2026-05-03T00:00:${String(index).padStart(2, "0")}.000Z`, + }); + } + + const clobberFiles = await listClobberFiles(configPath); + expect(clobberFiles).toHaveLength(CONFIG_CLOBBER_SNAPSHOT_LIMIT); + const capWarnings = warn.mock.calls.filter( + ([message]) => + typeof message === "string" && message.includes("Config clobber snapshot cap reached"), + ); + expect(capWarnings).toHaveLength(1); + }); + }); +}); diff --git a/src/config/io.clobber-snapshot.ts b/src/config/io.clobber-snapshot.ts new file mode 100644 index 00000000000..39d42a727cd --- /dev/null +++ b/src/config/io.clobber-snapshot.ts @@ -0,0 +1,239 @@ +import path from "node:path"; + +export const CONFIG_CLOBBER_SNAPSHOT_LIMIT = 32; + +const CONFIG_CLOBBER_LOCK_STALE_MS = 30_000; +const CONFIG_CLOBBER_LOCK_RETRY_MS = 10; +const CONFIG_CLOBBER_LOCK_TIMEOUT_MS = 2_000; +const clobberCapWarnedPaths = new Set(); + +type ConfigClobberSnapshotFs = { + promises: { + mkdir(path: string, options?: { recursive?: boolean; mode?: number }): Promise; + readdir(path: string): Promise; + rmdir(path: string): Promise; + stat(path: string): Promise<{ mtimeMs?: number } | null>; + writeFile( + path: string, + data: string, + options?: { encoding?: BufferEncoding; mode?: number; flag?: string }, + ): Promise; + }; + mkdirSync(path: string, options?: { recursive?: boolean; mode?: number }): unknown; + readdirSync(path: string): string[]; + rmdirSync(path: string): unknown; + statSync(path: string, options?: { throwIfNoEntry?: boolean }): { mtimeMs?: number } | null; + writeFileSync( + path: string, + data: string, + options?: { encoding?: BufferEncoding; mode?: number; flag?: string }, + ): unknown; +}; + +export type ConfigClobberSnapshotDeps = { + fs: ConfigClobberSnapshotFs; + logger: Pick; +}; + +function formatConfigArtifactTimestamp(ts: string): string { + return ts.replaceAll(":", "-").replaceAll(".", "-"); +} + +function isFsErrorCode(error: unknown, code: string): boolean { + return ( + error instanceof Error && + "code" in error && + typeof (error as { code?: unknown }).code === "string" && + (error as { code: string }).code === code + ); +} + +function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +function resolveClobberPaths(configPath: string): { + dir: string; + prefix: string; + lockPath: string; +} { + const dir = path.dirname(configPath); + const basename = path.basename(configPath); + return { + dir, + prefix: `${basename}.clobbered.`, + lockPath: path.join(dir, `${basename}.clobber.lock`), + }; +} + +function shouldRemoveStaleLock(mtimeMs: number | undefined, nowMs: number): boolean { + return typeof mtimeMs === "number" && nowMs - mtimeMs > CONFIG_CLOBBER_LOCK_STALE_MS; +} + +async function acquireClobberLock( + deps: ConfigClobberSnapshotDeps, + lockPath: string, +): Promise { + const startedAt = Date.now(); + while (Date.now() - startedAt < CONFIG_CLOBBER_LOCK_TIMEOUT_MS) { + try { + await deps.fs.promises.mkdir(lockPath, { mode: 0o700 }); + return true; + } catch (error) { + if (!isFsErrorCode(error, "EEXIST")) { + return false; + } + const stat = await deps.fs.promises.stat(lockPath).catch(() => null); + if (shouldRemoveStaleLock(stat?.mtimeMs, Date.now())) { + await deps.fs.promises.rmdir(lockPath).catch(() => {}); + continue; + } + await sleep(CONFIG_CLOBBER_LOCK_RETRY_MS); + } + } + return false; +} + +function acquireClobberLockSync(deps: ConfigClobberSnapshotDeps, lockPath: string): boolean { + for (let attempt = 0; attempt < 2; attempt++) { + try { + deps.fs.mkdirSync(lockPath, { mode: 0o700 }); + return true; + } catch (error) { + if (!isFsErrorCode(error, "EEXIST")) { + return false; + } + const stat = deps.fs.statSync(lockPath, { throwIfNoEntry: false }); + if (!shouldRemoveStaleLock(stat?.mtimeMs, Date.now())) { + return false; + } + try { + deps.fs.rmdirSync(lockPath); + } catch { + return false; + } + } + } + return false; +} + +async function countClobberedSiblings( + deps: ConfigClobberSnapshotDeps, + dir: string, + prefix: string, +): Promise { + try { + const entries = await deps.fs.promises.readdir(dir); + return entries.filter((entry) => entry.startsWith(prefix)).length; + } catch { + return 0; + } +} + +function countClobberedSiblingsSync( + deps: ConfigClobberSnapshotDeps, + dir: string, + prefix: string, +): number { + try { + return deps.fs.readdirSync(dir).filter((entry) => entry.startsWith(prefix)).length; + } catch { + return 0; + } +} + +function warnClobberCapReached( + deps: ConfigClobberSnapshotDeps, + configPath: string, + existing: number, +): void { + if (clobberCapWarnedPaths.has(configPath)) { + return; + } + clobberCapWarnedPaths.add(configPath); + deps.logger.warn( + `Config clobber snapshot cap reached for ${configPath}: ${existing} existing .clobbered.* files; skipping additional forensic snapshots.`, + ); +} + +function buildClobberedTargetPath(configPath: string, observedAt: string, attempt: number): string { + const basePath = `${configPath}.clobbered.${formatConfigArtifactTimestamp(observedAt)}`; + return attempt === 0 ? basePath : `${basePath}-${String(attempt).padStart(2, "0")}`; +} + +export async function persistBoundedClobberedConfigSnapshot(params: { + deps: ConfigClobberSnapshotDeps; + configPath: string; + raw: string; + observedAt: string; +}): Promise { + const paths = resolveClobberPaths(params.configPath); + const locked = await acquireClobberLock(params.deps, paths.lockPath); + if (!locked) { + return null; + } + try { + const existing = await countClobberedSiblings(params.deps, paths.dir, paths.prefix); + if (existing >= CONFIG_CLOBBER_SNAPSHOT_LIMIT) { + warnClobberCapReached(params.deps, params.configPath, existing); + return null; + } + for (let attempt = 0; attempt < CONFIG_CLOBBER_SNAPSHOT_LIMIT; attempt++) { + const targetPath = buildClobberedTargetPath(params.configPath, params.observedAt, attempt); + try { + await params.deps.fs.promises.writeFile(targetPath, params.raw, { + encoding: "utf-8", + mode: 0o600, + flag: "wx", + }); + return targetPath; + } catch (error) { + if (!isFsErrorCode(error, "EEXIST")) { + return null; + } + } + } + return null; + } finally { + await params.deps.fs.promises.rmdir(paths.lockPath).catch(() => {}); + } +} + +export function persistBoundedClobberedConfigSnapshotSync(params: { + deps: ConfigClobberSnapshotDeps; + configPath: string; + raw: string; + observedAt: string; +}): string | null { + const paths = resolveClobberPaths(params.configPath); + if (!acquireClobberLockSync(params.deps, paths.lockPath)) { + return null; + } + try { + const existing = countClobberedSiblingsSync(params.deps, paths.dir, paths.prefix); + if (existing >= CONFIG_CLOBBER_SNAPSHOT_LIMIT) { + warnClobberCapReached(params.deps, params.configPath, existing); + return null; + } + for (let attempt = 0; attempt < CONFIG_CLOBBER_SNAPSHOT_LIMIT; attempt++) { + const targetPath = buildClobberedTargetPath(params.configPath, params.observedAt, attempt); + try { + params.deps.fs.writeFileSync(targetPath, params.raw, { + encoding: "utf-8", + mode: 0o600, + flag: "wx", + }); + return targetPath; + } catch (error) { + if (!isFsErrorCode(error, "EEXIST")) { + return null; + } + } + } + return null; + } finally { + try { + params.deps.fs.rmdirSync(paths.lockPath); + } catch {} + } +} diff --git a/src/config/io.observe-recovery.test.ts b/src/config/io.observe-recovery.test.ts index 89de8f02c1d..a7f188082b7 100644 --- a/src/config/io.observe-recovery.test.ts +++ b/src/config/io.observe-recovery.test.ts @@ -4,6 +4,7 @@ import os from "node:os"; import path from "node:path"; import JSON5 from "json5"; import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; +import { CONFIG_CLOBBER_SNAPSHOT_LIMIT } from "./io.clobber-snapshot.js"; import { maybeRecoverSuspiciousConfigRead, maybeRecoverSuspiciousConfigReadSync, @@ -71,6 +72,12 @@ describe("config observe recovery", () => { .filter((line) => line.event === "config.observe"); } + async function listClobberFiles(configPath: string): Promise { + const entries = await fsp.readdir(path.dirname(configPath)); + const prefix = `${path.basename(configPath)}.clobbered.`; + return entries.filter((entry) => entry.startsWith(prefix)); + } + async function readLastObserveEvent( auditPath: string, ): Promise | undefined> { @@ -409,6 +416,31 @@ describe("config observe recovery", () => { }); }); + it("caps concurrent recovery clobber snapshots while preserving audit records", async () => { + await withSuiteHome(async (home) => { + const { deps, configPath, auditPath, warn } = makeDeps(home); + await seedConfigBackup(configPath, recoverableTelegramConfig); + await writeClobberedUpdateChannel(configPath); + + await Promise.all( + Array.from({ length: CONFIG_CLOBBER_SNAPSHOT_LIMIT + 18 }, async () => { + await recoverClobberedUpdateChannel({ deps, configPath }); + }), + ); + + const clobberFiles = await listClobberFiles(configPath); + expect(clobberFiles.length).toBeLessThanOrEqual(CONFIG_CLOBBER_SNAPSHOT_LIMIT); + const observeEvents = await readObserveEvents(auditPath); + expect(observeEvents.length).toBeGreaterThan(0); + expect(observeEvents.at(-1)).toHaveProperty("clobberedPath"); + const capWarnings = warn.mock.calls.filter( + ([message]) => + typeof message === "string" && message.includes("Config clobber snapshot cap reached"), + ); + expect(capWarnings.length).toBeLessThanOrEqual(1); + }); + }); + it("sync recovery uses backup baseline when health state is absent", async () => { await withSuiteHome(async (home) => { const { deps, configPath, auditPath } = makeDeps(home); diff --git a/src/config/io.observe-recovery.ts b/src/config/io.observe-recovery.ts index 028b1f1025e..a9f676139bd 100644 --- a/src/config/io.observe-recovery.ts +++ b/src/config/io.observe-recovery.ts @@ -7,6 +7,10 @@ import { snapshotConfigAuditProcessInfo, type ConfigObserveAuditRecord, } from "./io.audit.js"; +import { + persistBoundedClobberedConfigSnapshot, + persistBoundedClobberedConfigSnapshotSync, +} from "./io.clobber-snapshot.js"; import { formatConfigIssueSummary } from "./issue-format.js"; import { resolveStateDir } from "./paths.js"; import { @@ -37,6 +41,8 @@ export type ObserveRecoveryDeps = { copyFile(src: string, dest: string): Promise; chmod?(path: string, mode: number): Promise; mkdir(path: string, options?: { recursive?: boolean; mode?: number }): Promise; + readdir(path: string): Promise; + rmdir(path: string): Promise; appendFile( path: string, data: string, @@ -65,6 +71,8 @@ export type ObserveRecoveryDeps = { copyFileSync(src: string, dest: string): unknown; chmodSync?(path: string, mode: number): unknown; mkdirSync(path: string, options?: { recursive?: boolean; mode?: number }): unknown; + readdirSync(path: string): string[]; + rmdirSync(path: string): unknown; appendFileSync( path: string, data: string, @@ -530,10 +538,6 @@ function readConfigFingerprintForPathSync( } } -function formatConfigArtifactTimestamp(ts: string): string { - return ts.replaceAll(":", "-").replaceAll(".", "-"); -} - export function resolveLastKnownGoodConfigPath(configPath: string): string { return `${configPath}.last-good`; } @@ -575,44 +579,6 @@ function collectPollutedSecretPlaceholders( return output; } -async function persistClobberedConfigSnapshot(params: { - deps: ObserveRecoveryDeps; - configPath: string; - raw: string; - observedAt: string; -}): Promise { - const targetPath = `${params.configPath}.clobbered.${formatConfigArtifactTimestamp(params.observedAt)}`; - try { - await params.deps.fs.promises.writeFile(targetPath, params.raw, { - encoding: "utf-8", - mode: 0o600, - flag: "wx", - }); - return targetPath; - } catch { - return null; - } -} - -function persistClobberedConfigSnapshotSync(params: { - deps: ObserveRecoveryDeps; - configPath: string; - raw: string; - observedAt: string; -}): string | null { - const targetPath = `${params.configPath}.clobbered.${formatConfigArtifactTimestamp(params.observedAt)}`; - try { - params.deps.fs.writeFileSync(targetPath, params.raw, { - encoding: "utf-8", - mode: 0o600, - flag: "wx", - }); - return targetPath; - } catch { - return null; - } -} - export async function maybeRecoverSuspiciousConfigRead(params: { deps: ObserveRecoveryDeps; configPath: string; @@ -663,7 +629,7 @@ export async function maybeRecoverSuspiciousConfigRead(params: { return { raw: params.raw, parsed: params.parsed }; } - const clobberedPath = await persistClobberedConfigSnapshot({ + const clobberedPath = await persistBoundedClobberedConfigSnapshot({ deps: params.deps, configPath: params.configPath, raw: params.raw, @@ -770,7 +736,7 @@ export function maybeRecoverSuspiciousConfigReadSync(params: { return { raw: params.raw, parsed: params.parsed }; } - const clobberedPath = persistClobberedConfigSnapshotSync({ + const clobberedPath = persistBoundedClobberedConfigSnapshotSync({ deps: params.deps, configPath: params.configPath, raw: params.raw, @@ -924,7 +890,7 @@ export async function recoverConfigFromLastKnownGood(params: { stat: stat as ConfigStatMetadataSource, observedAt: now, }); - const clobberedPath = await persistClobberedConfigSnapshot({ + const clobberedPath = await persistBoundedClobberedConfigSnapshot({ deps, configPath: snapshot.path, raw: snapshot.raw, diff --git a/src/config/io.ts b/src/config/io.ts index 494e806cf2f..d66ed79401c 100644 --- a/src/config/io.ts +++ b/src/config/io.ts @@ -50,6 +50,10 @@ import { snapshotConfigAuditProcessInfo, type ConfigWriteAuditResult, } from "./io.audit.js"; +import { + persistBoundedClobberedConfigSnapshot, + persistBoundedClobberedConfigSnapshotSync, +} from "./io.clobber-snapshot.js"; import { throwInvalidConfig } from "./io.invalid-config.js"; import { maybeRecoverSuspiciousConfigRead, @@ -579,44 +583,6 @@ function formatConfigArtifactTimestamp(ts: string): string { return ts.replaceAll(":", "-").replaceAll(".", "-"); } -async function persistClobberedConfigSnapshot(params: { - deps: Required; - configPath: string; - raw: string; - observedAt: string; -}): Promise { - const targetPath = `${params.configPath}.clobbered.${formatConfigArtifactTimestamp(params.observedAt)}`; - try { - await params.deps.fs.promises.writeFile(targetPath, params.raw, { - encoding: "utf-8", - mode: 0o600, - flag: "wx", - }); - return targetPath; - } catch { - return null; - } -} - -function persistClobberedConfigSnapshotSync(params: { - deps: Required; - configPath: string; - raw: string; - observedAt: string; -}): string | null { - const targetPath = `${params.configPath}.clobbered.${formatConfigArtifactTimestamp(params.observedAt)}`; - try { - params.deps.fs.writeFileSync(targetPath, params.raw, { - encoding: "utf-8", - mode: 0o600, - flag: "wx", - }); - return targetPath; - } catch { - return null; - } -} - function sameFingerprint( left: ConfigHealthFingerprint | undefined, right: ConfigHealthFingerprint, @@ -701,7 +667,7 @@ async function observeConfigSnapshot( const backup = (backupBaseline?.hash ? backupBaseline : null) ?? (await readConfigFingerprintForPath(deps, `${snapshot.path}.bak`)); - const clobberedPath = await persistClobberedConfigSnapshot({ + const clobberedPath = await persistBoundedClobberedConfigSnapshot({ deps, configPath: snapshot.path, raw: snapshot.raw, @@ -833,7 +799,7 @@ function observeConfigSnapshotSync( const backup = (backupBaseline?.hash ? backupBaseline : null) ?? readConfigFingerprintForPathSync(deps, `${snapshot.path}.bak`); - const clobberedPath = persistClobberedConfigSnapshotSync({ + const clobberedPath = persistBoundedClobberedConfigSnapshotSync({ deps, configPath: snapshot.path, raw: snapshot.raw, @@ -1026,7 +992,7 @@ async function persistPrefixedConfigRecovery(params: { recoveredRaw: string; }): Promise { const observedAt = new Date().toISOString(); - const clobberedPath = await persistClobberedConfigSnapshot({ + const clobberedPath = await persistBoundedClobberedConfigSnapshot({ deps: params.deps, configPath: params.configPath, raw: params.originalRaw, diff --git a/src/config/io.write-config.test.ts b/src/config/io.write-config.test.ts index e50c46159b4..99652897681 100644 --- a/src/config/io.write-config.test.ts +++ b/src/config/io.write-config.test.ts @@ -5,6 +5,7 @@ import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest import { readPersistedInstalledPluginIndex } from "../plugins/installed-plugin-index-store.js"; import type { PluginManifestRegistry } from "../plugins/manifest-registry.js"; import { createSuiteTempRootTracker } from "../test-helpers/temp-dir.js"; +import { CONFIG_CLOBBER_SNAPSHOT_LIMIT } from "./io.clobber-snapshot.js"; import { createConfigIO, getRuntimeConfigSourceSnapshot, @@ -618,6 +619,41 @@ describe("config io write", () => { }); }); + it("caps repeated prefix-recovery clobber snapshots for doctor-style repair loops", async () => { + await withSuiteHome(async (home) => { + const configPath = path.join(home, ".openclaw", "openclaw.json"); + const cleanConfig = { + gateway: { mode: "local" }, + agents: { list: [{ id: "main", default: true }] }, + } satisfies ConfigFileSnapshot["config"]; + const cleanRaw = `${JSON.stringify(cleanConfig, null, 2)}\n`; + const warn = vi.fn(); + const io = createConfigIO({ + env: { VITEST: "true" } as NodeJS.ProcessEnv, + homedir: () => home, + logger: { warn, error: vi.fn() }, + }); + + await fs.mkdir(path.dirname(configPath), { recursive: true }); + for (let index = 0; index < CONFIG_CLOBBER_SNAPSHOT_LIMIT + 4; index++) { + await fs.writeFile(configPath, `Found and updated: False ${index}\n${cleanRaw}`, "utf-8"); + const snapshot = await io.readConfigFileSnapshot(); + expect(snapshot.valid).toBe(false); + await expect(io.recoverConfigFromJsonRootSuffix(snapshot)).resolves.toBe(true); + } + + const entries = await fs.readdir(path.dirname(configPath)); + const clobbered = entries.filter((entry) => entry.includes(".clobbered.")); + expect(clobbered).toHaveLength(CONFIG_CLOBBER_SNAPSHOT_LIMIT); + const capWarnings = warn.mock.calls.filter( + ([message]) => + typeof message === "string" && message.includes("Config clobber snapshot cap reached"), + ); + expect(capWarnings).toHaveLength(1); + await expect(fs.readFile(configPath, "utf-8")).resolves.toBe(cleanRaw); + }); + }); + it("rejects destructive internal writes before replacing the config", async () => { await withSuiteHome(async (home) => { const configPath = path.join(home, ".openclaw", "openclaw.json");