diff --git a/src/api/routes/backup.js b/src/api/routes/backup.js index cd2e2391..ad39cf4d 100644 --- a/src/api/routes/backup.js +++ b/src/api/routes/backup.js @@ -184,8 +184,8 @@ router.post( router.get( '/', (req, res, next) => requireGlobalAdmin('Backup access', req, res, next), - (_req, res) => { - const backups = listBackups(); + async (_req, res) => { + const backups = await listBackups(); res.json(backups); }, ); @@ -229,9 +229,9 @@ router.get( router.post( '/', (req, res, next) => requireGlobalAdmin('Backup access', req, res, next), - (_req, res) => { + async (_req, res) => { try { - const meta = createBackup(); + const meta = await createBackup(); return res.status(201).json({ id: meta.id, size: meta.size, createdAt: meta.createdAt }); } catch (err) { return res.status(500).json({ error: 'Failed to create backup', details: err.message }); @@ -278,11 +278,11 @@ router.post( router.get( '/:id/download', (req, res, next) => requireGlobalAdmin('Backup access', req, res, next), - (req, res) => { + async (req, res) => { const { id } = req.params; try { - const payload = readBackup(id); + const payload = await readBackup(id); const filename = `${id}.json`; res.setHeader('Content-Disposition', `attachment; filename="${filename}"`); res.setHeader('Content-Type', 'application/json'); @@ -420,7 +420,7 @@ router.post( router.post( '/prune', (req, res, next) => requireGlobalAdmin('Backup access', req, res, next), - (req, res) => { + async (req, res) => { const retention = req.body ?? {}; const errors = []; @@ -439,7 +439,7 @@ router.post( return res.status(400).json({ error: 'Invalid prune options', details: errors }); } - const deleted = pruneBackups(retention); + const deleted = await pruneBackups(retention); return res.json({ deleted, count: deleted.length }); }, ); diff --git a/src/modules/backup.js b/src/modules/backup.js index 981aae20..41208430 100644 --- a/src/modules/backup.js +++ b/src/modules/backup.js @@ -5,18 +5,7 @@ * @see https://github.com/VolvoxLLC/volvox-bot/issues/129 */ -import { - existsSync, - mkdirSync, - readdirSync, - readFileSync, - statSync, - unlinkSync, - writeFileSync, -} from 'node:fs'; - -// TODO: Consider switching to fs.promises for async operations to improve performance -// and avoid blocking the event loop with synchronous file system operations. +import { access, mkdir, readdir, readFile, stat, unlink, writeFile } from 'node:fs/promises'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; import { SAFE_CONFIG_KEYS, SENSITIVE_FIELDS } from '../api/utils/configAllowlist.js'; @@ -45,12 +34,14 @@ let scheduledBackupInterval = null; * Get or create the backup directory. * * @param {string} [dir] - Override backup directory path - * @returns {string} The backup directory path + * @returns {Promise} The backup directory path */ -export function getBackupDir(dir) { +export async function getBackupDir(dir) { const backupDir = dir ?? DEFAULT_BACKUP_DIR; - if (!existsSync(backupDir)) { - mkdirSync(backupDir, { recursive: true }); + try { + await access(backupDir); + } catch { + await mkdir(backupDir, { recursive: true }); } return backupDir; } @@ -192,10 +183,10 @@ function makeBackupFilename(date = new Date()) { * Create a timestamped backup of the current config in the backup directory. * * @param {string} [backupDir] - Override backup directory - * @returns {{id: string, path: string, size: number, createdAt: string}} Backup metadata + * @returns {Promise<{id: string, path: string, size: number, createdAt: string}>} Backup metadata */ -export function createBackup(backupDir) { - const dir = getBackupDir(backupDir); +export async function createBackup(backupDir) { + const dir = await getBackupDir(backupDir); const now = new Date(); const filename = makeBackupFilename(now); const filePath = path.join(dir, filename); @@ -203,9 +194,9 @@ export function createBackup(backupDir) { const payload = exportConfig(); const json = JSON.stringify(payload, null, 2); - writeFileSync(filePath, json, 'utf8'); + await writeFile(filePath, json, 'utf8'); - const { size } = statSync(filePath); + const { size } = await stat(filePath); const id = filename.replace('.json', ''); info('Config backup created', { id, path: filePath, size }); @@ -223,16 +214,17 @@ export function createBackup(backupDir) { * * @param {string} filename - Backup filename * @param {string} dir - Directory containing the backup file - * @returns {{id: string, filename: string, createdAt: string, size: number} | null} + * @returns {Promise<{id: string, filename: string, createdAt: string, size: number} | null>} */ -function parseBackupMeta(filename, dir) { +async function parseBackupMeta(filename, dir) { const match = BACKUP_FILENAME_PATTERN.exec(filename); if (!match) return null; const filePath = path.join(dir, filename); let size = 0; try { - size = statSync(filePath).size; + const st = await stat(filePath); + size = st.size; } catch { return null; } @@ -252,22 +244,20 @@ function parseBackupMeta(filename, dir) { * List all available backups, sorted newest first. * * @param {string} [backupDir] - Override backup directory - * @returns {Array<{id: string, filename: string, createdAt: string, size: number}>} + * @returns {Promise>} */ -export function listBackups(backupDir) { - const dir = getBackupDir(backupDir); +export async function listBackups(backupDir) { + const dir = await getBackupDir(backupDir); let files; try { - files = readdirSync(dir); + files = await readdir(dir); } catch { return []; } - const backups = files - .map((filename) => parseBackupMeta(filename, dir)) - .filter(Boolean) - .sort((a, b) => new Date(b.createdAt) - new Date(a.createdAt)); + const results = await Promise.all(files.map((filename) => parseBackupMeta(filename, dir))); + const backups = results.filter(Boolean).sort((a, b) => new Date(b.createdAt) - new Date(a.createdAt)); return backups; } @@ -277,10 +267,10 @@ export function listBackups(backupDir) { * * @param {string} id - Backup ID (filename without .json) * @param {string} [backupDir] - Override backup directory - * @returns {Object} Parsed backup payload + * @returns {Promise} Parsed backup payload * @throws {Error} If backup file not found or invalid */ -export function readBackup(id, backupDir) { +export async function readBackup(id, backupDir) { // Validate ID against strict pattern: backup-YYYY-MM-DDTHH-mm-ss-SSS-NNNN const BACKUP_ID_PATTERN = /^backup-[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}-[0-9]{2}-[0-9]{2}-[0-9]{3}-[0-9]{4}$/; @@ -288,15 +278,17 @@ export function readBackup(id, backupDir) { throw new Error('Invalid backup ID'); } - const dir = getBackupDir(backupDir); + const dir = await getBackupDir(backupDir); const filename = `${id}.json`; const filePath = path.join(dir, filename); - if (!existsSync(filePath)) { + try { + await access(filePath); + } catch { throw new Error(`Backup not found: ${id}`); } - const raw = readFileSync(filePath, 'utf8'); + const raw = await readFile(filePath, 'utf8'); try { return JSON.parse(raw); } catch { @@ -313,7 +305,7 @@ export function readBackup(id, backupDir) { * @throws {Error} If backup not found or invalid */ export async function restoreBackup(id, backupDir) { - const payload = readBackup(id, backupDir); + const payload = await readBackup(id, backupDir); const validationErrors = validateImportPayload(payload); if (validationErrors.length > 0) { @@ -338,12 +330,12 @@ export async function restoreBackup(id, backupDir) { * * @param {{daily?: number, weekly?: number}} [retention] - Retention counts * @param {string} [backupDir] - Override backup directory - * @returns {string[]} IDs of deleted backups + * @returns {Promise} IDs of deleted backups */ -export function pruneBackups(retention, backupDir) { +export async function pruneBackups(retention, backupDir) { const { daily = DEFAULT_RETENTION.daily, weekly = DEFAULT_RETENTION.weekly } = retention ?? {}; - const dir = getBackupDir(backupDir); - const all = listBackups(dir); + const dir = await getBackupDir(backupDir); + const all = await listBackups(dir); if (all.length === 0) return []; @@ -372,7 +364,7 @@ export function pruneBackups(retention, backupDir) { for (const backup of all) { if (!toKeep.has(backup.id)) { try { - unlinkSync(path.join(dir, backup.filename)); + await unlink(path.join(dir, backup.filename)); deleted.push(backup.id); info('Pruned old backup', { id: backup.id }); } catch (err) { @@ -407,12 +399,14 @@ export function startScheduledBackups(opts = {}) { info('Starting scheduled config backups', { intervalMs }); scheduledBackupInterval = setInterval(() => { - try { - createBackup(backupDir); - pruneBackups(retention, backupDir); - } catch (err) { - logError('Scheduled backup failed', { error: err.message }); - } + void (async () => { + try { + await createBackup(backupDir); + await pruneBackups(retention, backupDir); + } catch (err) { + logError('Scheduled backup failed', { error: err.message }); + } + })(); }, intervalMs); // Prevent the interval from keeping the process alive unnecessarily (e.g. in tests) diff --git a/tests/modules/backup.test.js b/tests/modules/backup.test.js index 7cf3149e..1b816c94 100644 --- a/tests/modules/backup.test.js +++ b/tests/modules/backup.test.js @@ -191,26 +191,26 @@ describe('importConfig', () => { // --- createBackup / listBackups --- describe('createBackup and listBackups', () => { - it('creates a backup file', () => { - const meta = createBackup(tmpDir); + it('creates a backup file', async () => { + const meta = await createBackup(tmpDir); expect(meta.id).toMatch(/^backup-/); expect(meta.size).toBeGreaterThan(0); expect(meta.createdAt).toMatch(/^\d{4}-/); }); it('lists created backups sorted newest first', async () => { - createBackup(tmpDir); + await createBackup(tmpDir); await new Promise((r) => setTimeout(r, 10)); // ensure different timestamps (at least ms apart) - createBackup(tmpDir); - const backups = listBackups(tmpDir); + await createBackup(tmpDir); + const backups = await listBackups(tmpDir); expect(backups.length).toBe(2); expect(new Date(backups[0].createdAt) >= new Date(backups[1].createdAt)).toBe(true); }); - it('returns empty array when no backups', () => { + it('returns empty array when no backups', async () => { const emptyDir = mkdtempSync(join(tmpdir(), 'empty-backup-')); try { - expect(listBackups(emptyDir)).toEqual([]); + expect(await listBackups(emptyDir)).toEqual([]); } finally { rmSync(emptyDir, { recursive: true }); } @@ -220,28 +220,28 @@ describe('createBackup and listBackups', () => { // --- readBackup --- describe('readBackup', () => { - it('reads a valid backup by id', () => { - const meta = createBackup(tmpDir); - const payload = readBackup(meta.id, tmpDir); + it('reads a valid backup by id', async () => { + const meta = await createBackup(tmpDir); + const payload = await readBackup(meta.id, tmpDir); expect(payload).toHaveProperty('config'); expect(payload).toHaveProperty('exportedAt'); }); - it('throws for unknown id', () => { - expect(() => readBackup('backup-9999-01-01T00-00-00-000-0000', tmpDir)).toThrow( + it('throws for unknown id', async () => { + await expect(readBackup('backup-9999-01-01T00-00-00-000-0000', tmpDir)).rejects.toThrow( 'Backup not found', ); }); - it('throws for path-traversal attempts', () => { - expect(() => readBackup('../etc/passwd', tmpDir)).toThrow('Invalid backup ID'); - expect(() => readBackup('..\\windows\\system32', tmpDir)).toThrow('Invalid backup ID'); + it('throws for path-traversal attempts', async () => { + await expect(readBackup('../etc/passwd', tmpDir)).rejects.toThrow('Invalid backup ID'); + await expect(readBackup('..\\windows\\system32', tmpDir)).rejects.toThrow('Invalid backup ID'); }); - it('throws for corrupted backup', () => { + it('throws for corrupted backup', async () => { const badFile = join(tmpDir, 'backup-2020-01-01T00-00-00-000-0000.json'); writeFileSync(badFile, 'not json', 'utf8'); - expect(() => readBackup('backup-2020-01-01T00-00-00-000-0000', tmpDir)).toThrow( + await expect(readBackup('backup-2020-01-01T00-00-00-000-0000', tmpDir)).rejects.toThrow( 'Backup file is corrupted', ); }); @@ -251,7 +251,7 @@ describe('readBackup', () => { describe('restoreBackup', () => { it('restores config from a valid backup', async () => { - const meta = createBackup(tmpDir); + const meta = await createBackup(tmpDir); const result = await restoreBackup(meta.id, tmpDir); expect(result).toHaveProperty('applied'); expect(result).toHaveProperty('skipped'); @@ -270,25 +270,25 @@ describe('restoreBackup', () => { // --- pruneBackups --- describe('pruneBackups', () => { - it('keeps the N most recent backups', () => { + it('keeps the N most recent backups', async () => { for (let i = 0; i < 5; i++) { - createBackup(tmpDir); + await createBackup(tmpDir); } - const deleted = pruneBackups({ daily: 3, weekly: 0 }, tmpDir); + const deleted = await pruneBackups({ daily: 3, weekly: 0 }, tmpDir); expect(deleted.length).toBe(2); - expect(listBackups(tmpDir).length).toBe(3); + expect((await listBackups(tmpDir)).length).toBe(3); }); - it('keeps zero backups when daily=0 and weekly=0', () => { - createBackup(tmpDir); - createBackup(tmpDir); - const deleted = pruneBackups({ daily: 0, weekly: 0 }, tmpDir); + it('keeps zero backups when daily=0 and weekly=0', async () => { + await createBackup(tmpDir); + await createBackup(tmpDir); + const deleted = await pruneBackups({ daily: 0, weekly: 0 }, tmpDir); expect(deleted.length).toBe(2); - expect(listBackups(tmpDir).length).toBe(0); + expect((await listBackups(tmpDir)).length).toBe(0); }); - it('returns empty array when no backups exist', () => { - expect(pruneBackups({}, tmpDir)).toEqual([]); + it('returns empty array when no backups exist', async () => { + expect(await pruneBackups({}, tmpDir)).toEqual([]); }); });