From 5d174a5becee989c11d0c0e1679da880e5300a46 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Sun, 24 May 2026 16:57:21 -0700 Subject: [PATCH] fix(config): do not suppress recovery retry after failed backup restore (#85787) maybeRecoverSuspiciousConfigRead unconditionally recorded lastObservedSuspiciousSignature in health state even when restoredFromBackup was false (copyFile failed). The guard at resolveConfigReadRecoveryContext then prevented the same signature from ever being retried, permanently accepting the suspicious config on every subsequent launch. Only record the dedup signature when the backup restore actually succeeded. --- src/config/io.observe-recovery.test.ts | 81 ++++++++++++++++++++++++++ src/config/io.observe-recovery.ts | 28 +++++---- 2 files changed, 97 insertions(+), 12 deletions(-) diff --git a/src/config/io.observe-recovery.test.ts b/src/config/io.observe-recovery.test.ts index 00b7bbb5568..95f572ec44b 100644 --- a/src/config/io.observe-recovery.test.ts +++ b/src/config/io.observe-recovery.test.ts @@ -454,6 +454,87 @@ describe("config observe recovery", () => { }); }); + it("retries recovery on next launch after a failed copyFile restore", 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), + }, + }; + await maybeRecoverSuspiciousConfigRead({ + deps: { ...deps, fs: failingFs }, + configPath, + raw: clobbered.raw, + parsed: clobbered.parsed, + }); + + expectWarnContaining(warn, "Config auto-restore from backup failed:"); + const firstEvents = await readObserveEvents(auditPath); + expect(firstEvents).toHaveLength(1); + expect(firstEvents[0]?.restoredFromBackup).toBe(false); + + const retryResult = await maybeRecoverSuspiciousConfigRead({ + deps, + configPath, + raw: clobbered.raw, + parsed: clobbered.parsed, + }); + + expect((retryResult.parsed as { gateway?: { mode?: string } }).gateway?.mode).toBe("local"); + await expect(fsp.readFile(configPath, "utf-8")).resolves.not.toBe(clobbered.raw); + const retryEvents = await readObserveEvents(auditPath); + expect(retryEvents).toHaveLength(2); + expect(retryEvents[1]?.restoredFromBackup).toBe(true); + }); + }); + + it("sync recovery retries on next launch after a failed copyFileSync restore", 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; + }, + }; + maybeRecoverSuspiciousConfigReadSync({ + deps: { ...deps, fs: failingFs }, + configPath, + raw: clobbered.raw, + parsed: clobbered.parsed, + }); + + expectWarnContaining(warn, "Config auto-restore from backup failed:"); + const firstEvents = await readObserveEvents(auditPath); + expect(firstEvents).toHaveLength(1); + expect(firstEvents[0]?.restoredFromBackup).toBe(false); + + const retryResult = maybeRecoverSuspiciousConfigReadSync({ + deps, + configPath, + raw: clobbered.raw, + parsed: clobbered.parsed, + }); + + expect((retryResult.parsed as { gateway?: { mode?: string } }).gateway?.mode).toBe("local"); + await expect(fsp.readFile(configPath, "utf-8")).resolves.not.toBe(clobbered.raw); + const retryEvents = await readObserveEvents(auditPath); + expect(retryEvents).toHaveLength(2); + expect(retryEvents[1]?.restoredFromBackup).toBe(true); + }); + }); + 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 3cba90235d8..d26884df2e3 100644 --- a/src/config/io.observe-recovery.ts +++ b/src/config/io.observe-recovery.ts @@ -680,12 +680,14 @@ export async function maybeRecoverSuspiciousConfigRead(params: { }), ); - healthState = setConfigHealthEntry( - healthState, - params.configPath, - createLastObservedSuspiciousEntry(entry, suspiciousSignature), - ); - await writeConfigHealthState(params.deps, healthState); + if (restoredFromBackup) { + healthState = setConfigHealthEntry( + healthState, + params.configPath, + createLastObservedSuspiciousEntry(entry, suspiciousSignature), + ); + await writeConfigHealthState(params.deps, healthState); + } return { raw: backupRaw, parsed: backupParsed }; } @@ -790,12 +792,14 @@ export function maybeRecoverSuspiciousConfigReadSync(params: { }), ); - healthState = setConfigHealthEntry( - healthState, - params.configPath, - createLastObservedSuspiciousEntry(entry, suspiciousSignature), - ); - writeConfigHealthStateSync(params.deps, healthState); + if (restoredFromBackup) { + healthState = setConfigHealthEntry( + healthState, + params.configPath, + createLastObservedSuspiciousEntry(entry, suspiciousSignature), + ); + writeConfigHealthStateSync(params.deps, healthState); + } return { raw: backupRaw, parsed: backupParsed }; }