fix(config): surface backup restore copy failures in audit and logs

davidangularme · jalehman · commit f8f939cb6e17 · 2026-05-01T11:10:15.000-07:00
Previously a failed copyFile during suspicious-read recovery was swallowed by
a bare catch, still logged as 'Config auto-restored from backup', and the
audit record was written with valid: true — making a silent failure look
like success. Capture the error, log a distinct failure warning with the
message, set valid to restoredFromBackup, and persist restoreErrorCode /
restoreErrorMessage on the observe audit record for both async and sync
paths.
diff --git 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
@@ -278,6 +278,44 @@ 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("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
@@ -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,35 @@ 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 createConfigObserveAnomalyAuditAppendParams(
+  deps: ObserveRecoveryDeps,
+  params: Omit<ConfigObserveAuditRecordParams, "restoredFromBackup" | "restoredBackupPath">,
+) {
+  return createConfigObserveAuditAppendParams(deps, {
+    ...params,
+    restoredFromBackup: false,
+    restoredBackupPath: null,
+  });
+}
+
 function hashConfigRaw(raw: string | null): string {
   return crypto
     .createHash("sha256")
@@ -637,26 +670,43 @@ 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,
       backup,
       clobberedPath,
       restoredFromBackup,
       restoredBackupPath: backupPath,
+      restoreErrorCode: restoreErrorDetails.code,
+      restoreErrorMessage: restoreErrorDetails.message,
     }),
   );
 
@@ -727,26 +777,43 @@ 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,
       backup,
       clobberedPath,
       restoredFromBackup,
       restoredBackupPath: backupPath,
+      restoreErrorCode: restoreErrorDetails.code,
+      restoreErrorMessage: restoreErrorDetails.message,
     }),
   );
 
diff --git 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,
     },
   });
 
