diff --git a/CHANGELOG.md b/CHANGELOG.md index 4bacb286811..76d23062bfd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ Docs: https://docs.openclaw.ai +## Unreleased + +### Fixes + +- Gateway/config: report failed backup restores as failed in logs and config observe audit records instead of marking them valid. (#70515) Thanks @davidangularme. + ## 2026.4.30 ### Changes diff --git a/src/config/io.audit.ts b/src/config/io.audit.ts index 4066640acfd..fca367eaf51 100644 --- a/src/config/io.audit.ts +++ b/src/config/io.audit.ts @@ -227,6 +227,8 @@ export type ConfigObserveAuditRecord = { clobberedPath: string | null; restoredFromBackup: boolean; restoredBackupPath: string | null; + restoreErrorCode: string | null; + restoreErrorMessage: string | null; }; export type ConfigAuditRecord = ConfigWriteAuditRecord | ConfigObserveAuditRecord; diff --git a/src/config/io.observe-recovery.test.ts b/src/config/io.observe-recovery.test.ts index 21c5890b78a..4352788e387 100644 --- a/src/config/io.observe-recovery.test.ts +++ b/src/config/io.observe-recovery.test.ts @@ -278,6 +278,82 @@ describe("config observe recovery", () => { }); }); + it("records copyFile failure instead of falsely claiming restore succeeded", async () => { + await withSuiteHome(async (home) => { + const { deps, configPath, auditPath, warn } = makeDeps(home); + await seedConfigBackup(configPath, recoverableTelegramConfig); + const clobbered = await writeClobberedUpdateChannel(configPath); + + const copyError = Object.assign(new Error("EACCES: permission denied"), { code: "EACCES" }); + const failingFs: ObserveRecoveryDeps["fs"] = { + ...deps.fs, + promises: { + ...deps.fs.promises, + copyFile: () => Promise.reject(copyError), + }, + }; + const recovered = await maybeRecoverSuspiciousConfigRead({ + deps: { ...deps, fs: failingFs }, + configPath, + raw: clobbered.raw, + parsed: clobbered.parsed, + }); + + expect((recovered.parsed as { gateway?: { mode?: string } }).gateway?.mode).toBe("local"); + await expect(fsp.readFile(configPath, "utf-8")).resolves.toBe(clobbered.raw); + expect(warn).toHaveBeenCalledWith( + expect.stringContaining("Config auto-restore from backup failed:"), + ); + expect(warn).not.toHaveBeenCalledWith( + expect.stringContaining("Config auto-restored from backup:"), + ); + + const observe = await readLastObserveEvent(auditPath); + expect(observe?.restoredFromBackup).toBe(false); + expect(observe?.valid).toBe(false); + expect(observe?.restoreErrorCode).toBe("EACCES"); + expect(observe?.restoreErrorMessage).toBe("EACCES: permission denied"); + }); + }); + + it("sync recovery records copyFileSync failure instead of falsely claiming restore succeeded", async () => { + await withSuiteHome(async (home) => { + const { deps, configPath, auditPath, warn } = makeDeps(home); + await seedConfigBackup(configPath, recoverableTelegramConfig); + const clobbered = await writeClobberedUpdateChannel(configPath); + + const copyError = Object.assign(new Error("EACCES: permission denied"), { code: "EACCES" }); + const failingFs: ObserveRecoveryDeps["fs"] = { + ...deps.fs, + copyFileSync: () => { + throw copyError; + }, + }; + const recovered = maybeRecoverSuspiciousConfigReadSync({ + deps: { ...deps, fs: failingFs }, + configPath, + raw: clobbered.raw, + parsed: clobbered.parsed, + }); + + expect((recovered.parsed as { gateway?: { mode?: string } }).gateway?.mode).toBe("local"); + await expect(fsp.readFile(configPath, "utf-8")).resolves.toBe(clobbered.raw); + expect(warn).toHaveBeenCalledWith( + expect.stringContaining("Config auto-restore from backup failed:"), + ); + expect(warn).toHaveBeenCalledWith(expect.stringContaining("EACCES: permission denied")); + expect(warn).not.toHaveBeenCalledWith( + expect.stringContaining("Config auto-restored from backup:"), + ); + + const observe = await readLastObserveEvent(auditPath); + expect(observe?.restoredFromBackup).toBe(false); + expect(observe?.valid).toBe(false); + expect(observe?.restoreErrorCode).toBe("EACCES"); + expect(observe?.restoreErrorMessage).toBe("EACCES: permission denied"); + }); + }); + it("dedupes repeated suspicious hashes", 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 391dbbcf4ab..ee865c67a52 100644 --- a/src/config/io.observe-recovery.ts +++ b/src/config/io.observe-recovery.ts @@ -127,6 +127,8 @@ function createConfigObserveAuditRecord(params: { clobberedPath: string | null; restoredFromBackup: boolean; restoredBackupPath: string | null; + restoreErrorCode?: string | null; + restoreErrorMessage?: string | null; }): ConfigObserveAuditRecord { return { ts: params.ts, @@ -175,6 +177,8 @@ function createConfigObserveAuditRecord(params: { clobberedPath: params.clobberedPath, restoredFromBackup: params.restoredFromBackup, restoredBackupPath: params.restoredBackupPath, + restoreErrorCode: params.restoreErrorCode ?? null, + restoreErrorMessage: params.restoreErrorMessage ?? null, }; } @@ -192,6 +196,24 @@ function createConfigObserveAuditAppendParams( }; } +function extractRestoreErrorDetails(error: unknown): { + code: string | null; + message: string | null; +} { + if (!error || typeof error !== "object") { + return { code: null, message: typeof error === "string" ? error : null }; + } + const code = + "code" in error && typeof (error as { code?: unknown }).code === "string" + ? (error as { code: string }).code + : null; + const message = + "message" in error && typeof (error as { message?: unknown }).message === "string" + ? (error as { message: string }).message + : null; + return { code, message }; +} + function hashConfigRaw(raw: string | null): string { return crypto .createHash("sha256") @@ -637,19 +659,34 @@ export async function maybeRecoverSuspiciousConfigRead(params: { }); let restoredFromBackup = false; + let restoreError: unknown; try { await params.deps.fs.promises.copyFile(backupPath, params.configPath); restoredFromBackup = true; - } catch {} + } catch (error) { + restoreError = error; + } - params.deps.logger.warn( - `Config auto-restored from backup: ${params.configPath} (${suspicious.join(", ")})`, - ); + const restoreErrorDetails = restoredFromBackup + ? { code: null, message: null } + : extractRestoreErrorDetails(restoreError); + + if (restoredFromBackup) { + params.deps.logger.warn( + `Config auto-restored from backup: ${params.configPath} (${suspicious.join(", ")})`, + ); + } else { + params.deps.logger.warn( + `Config auto-restore from backup failed: ${params.configPath} (${suspicious.join(", ")}${ + restoreErrorDetails.message ? `; ${restoreErrorDetails.message}` : "" + })`, + ); + } await appendConfigAuditRecord( createConfigObserveAuditAppendParams(params.deps, { ts: now, configPath: params.configPath, - valid: true, + valid: restoredFromBackup, current, suspicious, lastKnownGood: entry.lastKnownGood, @@ -657,6 +694,8 @@ export async function maybeRecoverSuspiciousConfigRead(params: { clobberedPath, restoredFromBackup, restoredBackupPath: backupPath, + restoreErrorCode: restoreErrorDetails.code, + restoreErrorMessage: restoreErrorDetails.message, }), ); @@ -727,19 +766,34 @@ export function maybeRecoverSuspiciousConfigReadSync(params: { }); let restoredFromBackup = false; + let restoreError: unknown; try { params.deps.fs.copyFileSync(backupPath, params.configPath); restoredFromBackup = true; - } catch {} + } catch (error) { + restoreError = error; + } - params.deps.logger.warn( - `Config auto-restored from backup: ${params.configPath} (${suspicious.join(", ")})`, - ); + const restoreErrorDetails = restoredFromBackup + ? { code: null, message: null } + : extractRestoreErrorDetails(restoreError); + + if (restoredFromBackup) { + params.deps.logger.warn( + `Config auto-restored from backup: ${params.configPath} (${suspicious.join(", ")})`, + ); + } else { + params.deps.logger.warn( + `Config auto-restore from backup failed: ${params.configPath} (${suspicious.join(", ")}${ + restoreErrorDetails.message ? `; ${restoreErrorDetails.message}` : "" + })`, + ); + } appendConfigAuditRecordSync( createConfigObserveAuditAppendParams(params.deps, { ts: now, configPath: params.configPath, - valid: true, + valid: restoredFromBackup, current, suspicious, lastKnownGood: entry.lastKnownGood, @@ -747,6 +801,8 @@ export function maybeRecoverSuspiciousConfigReadSync(params: { clobberedPath, restoredFromBackup, restoredBackupPath: backupPath, + restoreErrorCode: restoreErrorDetails.code, + restoreErrorMessage: restoreErrorDetails.message, }), ); diff --git a/src/config/io.ts b/src/config/io.ts index be29c72dd74..f6bd3c9160e 100644 --- a/src/config/io.ts +++ b/src/config/io.ts @@ -765,6 +765,8 @@ async function observeConfigSnapshot( clobberedPath, restoredFromBackup: false, restoredBackupPath: null, + restoreErrorCode: null, + restoreErrorMessage: null, }, }); @@ -895,6 +897,8 @@ function observeConfigSnapshotSync( clobberedPath, restoredFromBackup: false, restoredBackupPath: null, + restoreErrorCode: null, + restoreErrorMessage: null, }, });