From f32418e72a517511b24284c39136295ec51b4dac Mon Sep 17 00:00:00 2001 From: Dotta Date: Mon, 6 Jul 2026 21:31:11 +0000 Subject: [PATCH] fix(server): make backup health checks async and flag future backup mtimes Address review findings: directory scan and marker reads no longer block the event loop, and a backup mtime ahead of the clock now warns as clock skew instead of passing the staleness check. Co-Authored-By: Paperclip --- server/src/__tests__/health.test.ts | 38 ++++++++ server/src/routes/health.ts | 2 +- server/src/routes/openapi.ts | 1 + server/src/services/database-backup-health.ts | 94 +++++++++++++------ 4 files changed, 105 insertions(+), 30 deletions(-) diff --git a/server/src/__tests__/health.test.ts b/server/src/__tests__/health.test.ts index 42b4ab5a170..5135b3909b8 100644 --- a/server/src/__tests__/health.test.ts +++ b/server/src/__tests__/health.test.ts @@ -166,11 +166,49 @@ describe("GET /health", () => { expect(res.body.warnings).toEqual(res.body.databaseBackup.warnings); }); + it("warns instead of reporting fresh when the latest backup mtime is in the future", async () => { + const backupDir = fs.mkdtempSync(path.join(os.tmpdir(), "paperclip-health-backups-")); + const backupFile = path.join(backupDir, "paperclip-20260707-120000.sql.gz"); + fs.writeFileSync(backupFile, "backup"); + fs.utimesSync( + backupFile, + new Date("2026-07-07T12:00:00.000Z"), + new Date("2026-07-07T12:00:00.000Z"), + ); + const app = createApp(createHealthyDb(), testServerInfo, { + enabled: true, + backupDir, + maxAgeHours: 26, + now: new Date("2026-07-06T12:00:00.000Z"), + }); + + const res = await request(app).get("/health"); + + expect(res.status).toBe(200); + expect(res.body.databaseBackup).toMatchObject({ + status: "warning", + latestBackup: { + name: "paperclip-20260707-120000.sql.gz", + ageHours: -24, + }, + warnings: [ + { + code: "database_backup_clock_skew", + }, + ], + }); + }); + it("surfaces database backup failure markers in full health details", async () => { const backupDir = fs.mkdtempSync(path.join(os.tmpdir(), "paperclip-health-backups-")); const backupFile = path.join(backupDir, "paperclip-20260706-031702.sql.gz"); const alertFile = path.join(backupDir, "db-backup-to-s3.failure"); fs.writeFileSync(backupFile, "backup"); + fs.utimesSync( + backupFile, + new Date("2026-07-06T03:17:02.000Z"), + new Date("2026-07-06T03:17:02.000Z"), + ); fs.writeFileSync(alertFile, "db-backup-to-s3 failed at 2026-07-06T03:17:00.000Z exit=1\n"); const app = createApp(createHealthyDb(), testServerInfo, { enabled: true, diff --git a/server/src/routes/health.ts b/server/src/routes/health.ts index 86c2ac52c1e..3bbc70ce97b 100644 --- a/server/src/routes/health.ts +++ b/server/src/routes/health.ts @@ -168,7 +168,7 @@ export function healthRoutes( } const databaseBackup = exposeFullDetails && opts.databaseBackupHealth - ? inspectDatabaseBackupHealth(opts.databaseBackupHealth) + ? await inspectDatabaseBackupHealth(opts.databaseBackupHealth) : undefined; const warnings = databaseBackup?.warnings.length ? databaseBackup.warnings : undefined; diff --git a/server/src/routes/openapi.ts b/server/src/routes/openapi.ts index 95971a8d786..16778f1add5 100644 --- a/server/src/routes/openapi.ts +++ b/server/src/routes/openapi.ts @@ -900,6 +900,7 @@ registry.registerPath({ warnings: z.array(z.object({ code: z.enum([ "database_backup_check_failed", + "database_backup_clock_skew", "database_backup_last_failure", "database_backup_missing", "database_backup_stale", diff --git a/server/src/services/database-backup-health.ts b/server/src/services/database-backup-health.ts index 2393ed84939..625410cca50 100644 --- a/server/src/services/database-backup-health.ts +++ b/server/src/services/database-backup-health.ts @@ -1,8 +1,9 @@ -import { existsSync, readdirSync, readFileSync, statSync } from "node:fs"; +import { readdir, readFile, stat } from "node:fs/promises"; import { basename, join } from "node:path"; export type DatabaseBackupHealthWarningCode = | "database_backup_check_failed" + | "database_backup_clock_skew" | "database_backup_last_failure" | "database_backup_missing" | "database_backup_stale"; @@ -44,45 +45,73 @@ function roundHours(value: number): number { return Math.round(value * 10) / 10; } -function readLastFailure(alertFile: string | undefined) { - if (!alertFile || !existsSync(alertFile)) return null; - - const stat = statSync(alertFile); - const message = readFileSync(alertFile, "utf8").trim().split(/\r?\n/)[0] || "Database backup failure marker is present."; - return { - path: alertFile, - mtime: new Date(stat.mtimeMs).toISOString(), - message, - }; +function isMissingFileError(error: unknown): boolean { + return ( + typeof error === "object" && + error !== null && + (error as NodeJS.ErrnoException).code === "ENOENT" + ); } -function findLatestBackup(backupDir: string, nowMs: number) { - if (!existsSync(backupDir)) return null; +async function readLastFailure(alertFile: string | undefined) { + if (!alertFile) return null; - const candidates = readdirSync(backupDir) - .filter((name) => name.endsWith(".sql.gz")) - .map((name) => { - const fullPath = join(backupDir, name); - const stat = statSync(fullPath); - return { fullPath, name, stat }; - }) - .sort((a, b) => b.stat.mtimeMs - a.stat.mtimeMs); + try { + const [alertStat, contents] = await Promise.all([ + stat(alertFile), + readFile(alertFile, "utf8"), + ]); + const message = contents.trim().split(/\r?\n/)[0] || "Database backup failure marker is present."; + return { + path: alertFile, + mtime: new Date(alertStat.mtimeMs).toISOString(), + message, + }; + } catch (error) { + if (isMissingFileError(error)) return null; + throw error; + } +} + +async function findLatestBackup(backupDir: string, nowMs: number) { + let names: string[]; + try { + names = await readdir(backupDir); + } catch (error) { + if (isMissingFileError(error)) return null; + throw error; + } + + let latest: { fullPath: string; mtimeMs: number; size: number } | null = null; + for (const name of names) { + if (!name.endsWith(".sql.gz")) continue; + const fullPath = join(backupDir, name); + let fileStat; + try { + fileStat = await stat(fullPath); + } catch (error) { + if (isMissingFileError(error)) continue; + throw error; + } + if (!latest || fileStat.mtimeMs > latest.mtimeMs) { + latest = { fullPath, mtimeMs: fileStat.mtimeMs, size: fileStat.size }; + } + } - const latest = candidates[0]; if (!latest) return null; return { name: basename(latest.fullPath), path: latest.fullPath, - mtime: new Date(latest.stat.mtimeMs).toISOString(), - ageHours: roundHours((nowMs - latest.stat.mtimeMs) / 3_600_000), - sizeBytes: latest.stat.size, + mtime: new Date(latest.mtimeMs).toISOString(), + ageHours: roundHours((nowMs - latest.mtimeMs) / 3_600_000), + sizeBytes: latest.size, }; } -export function inspectDatabaseBackupHealth( +export async function inspectDatabaseBackupHealth( opts: InspectDatabaseBackupHealthOptions, -): DatabaseBackupHealthStatus { +): Promise { const warnings: DatabaseBackupHealthWarning[] = []; const now = opts.now ?? new Date(); const maxAgeHours = Math.max(1, opts.maxAgeHours); @@ -91,14 +120,21 @@ export function inspectDatabaseBackupHealth( let lastFailure: DatabaseBackupHealthStatus["lastFailure"] = null; try { - latestBackup = findLatestBackup(opts.backupDir, now.getTime()); - lastFailure = readLastFailure(opts.alertFile); + [latestBackup, lastFailure] = await Promise.all([ + findLatestBackup(opts.backupDir, now.getTime()), + readLastFailure(opts.alertFile), + ]); if (!latestBackup) { warnings.push({ code: "database_backup_missing", message: `No .sql.gz database backups found in ${opts.backupDir}.`, }); + } else if (latestBackup.ageHours < 0) { + warnings.push({ + code: "database_backup_clock_skew", + message: `Latest database backup timestamp ${latestBackup.mtime} is in the future; backup freshness cannot be verified (check system clock).`, + }); } else if (latestBackup.ageHours > maxAgeHours) { warnings.push({ code: "database_backup_stale",