mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 06:20:43 +00:00
fix(config): surface backup restore copy failures in audit and logs (#70515)
Merged via squash.
Prepared head SHA: 7c779748bf
Co-authored-by: davidangularme <18486579+davidangularme@users.noreply.github.com>
Co-authored-by: jalehman <550978+jalehman@users.noreply.github.com>
Reviewed-by: @jalehman
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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,
|
||||
}),
|
||||
);
|
||||
|
||||
|
||||
@@ -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,
|
||||
},
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user