diff --git a/backend/__tests__/integration/picpeakRoundtrip.test.js b/backend/__tests__/integration/picpeakRoundtrip.test.js index e31ee1d7..8e26de07 100644 --- a/backend/__tests__/integration/picpeakRoundtrip.test.js +++ b/backend/__tests__/integration/picpeakRoundtrip.test.js @@ -18,13 +18,14 @@ let cleanup; let tmpDir; let createPicpeak; let importFromPicpeak; +let validateManifest; let superAdminRoleId; beforeAll(async () => { ({ db, cleanup, tmpDir } = await bootCrmDb()); process.env.STORAGE_PATH = tmpDir; ({ createPicpeak } = require('../../src/services/picpeakExportService')); - ({ importFromPicpeak } = require('../../src/services/picpeakImportService')); + ({ importFromPicpeak, validateManifest } = require('../../src/services/picpeakImportService')); const role = await db('roles').where({ name: 'super_admin' }).first(); superAdminRoleId = role.id; }, 60000); @@ -116,4 +117,64 @@ describe('.picpeak roundtrip (export → import)', () => { fs.rmSync(path.dirname(filePath), { recursive: true, force: true }); } }); + + it('restores files/ and reports filesRestored', async () => { + // A business-doc that lives in storage → travels in the backup. + const docDir = path.join(tmpDir, 'business-docs'); + const marker = path.join(docDir, 'roundtrip-doc.txt'); + fs.mkdirSync(docDir, { recursive: true }); + fs.writeFileSync(marker, 'hello'); + await db('admin_users').del(); + const [id] = await db('admin_users').insert(adminRow('files@example.com', 'H')).returning('id'); + const currentAdminId = typeof id === 'object' ? id.id : id; + + const { filePath } = await createPicpeak({ includePhotos: false }); + try { + fs.rmSync(marker); // delete on disk so the restore must bring it back + const result = await importFromPicpeak({ picpeakPath: filePath, currentAdminId }); + expect(result.filesRestored).toBeGreaterThanOrEqual(1); + expect(fs.existsSync(marker)).toBe(true); + expect(fs.readFileSync(marker, 'utf8')).toBe('hello'); + } finally { + fs.rmSync(path.dirname(filePath), { recursive: true, force: true }); + fs.rmSync(docDir, { recursive: true, force: true }); + } + }); +}); + +describe('.picpeak manifest validation', () => { + it('rejects a database-engine mismatch', async () => { + // Harness runs on SQLite, so a pg manifest must be refused. + const blockers = await validateManifest({ + kind: 'picpeak-backup', format: 1, database: { engine: 'pg' }, tables: {}, + }); + expect(blockers.some((b) => /engine/i.test(b))).toBe(true); + }); + + it('rejects a backup from a newer schema (forward-only)', async () => { + // validateManifest reads knex_migrations for the target's latest migration; + // the harness has none, so create it with an older migration than the backup. + await db.schema.createTable('knex_migrations', (t) => { + t.increments('id'); + t.string('name'); + t.integer('batch'); + t.timestamp('migration_time'); + }); + try { + await db('knex_migrations').insert({ name: '100_baseline', batch: 1 }); + const blockers = await validateManifest({ + kind: 'picpeak-backup', format: 1, + database: { engine: 'sqlite', latest_migration: '999_from_the_future' }, + tables: {}, + }); + expect(blockers.some((b) => /newer/i.test(b))).toBe(true); + } finally { + await db.schema.dropTableIfExists('knex_migrations'); + } + }); + + it('rejects a file that is not a PicPeak backup', async () => { + const blockers = await validateManifest({ some: 'random-json' }); + expect(blockers.length).toBeGreaterThan(0); + }); }); diff --git a/backend/src/services/picpeakExportService.js b/backend/src/services/picpeakExportService.js index 0fb8f469..561f6f54 100644 --- a/backend/src/services/picpeakExportService.js +++ b/backend/src/services/picpeakExportService.js @@ -184,23 +184,31 @@ async function createPicpeak({ includePhotos = false, outDir } = {}) { const stamp = manifest.created_at.replace(/[:.]/g, '-'); const filePath = path.join(targetDir, `picpeak-backup-${stamp}.picpeak`); - await new Promise((resolve, reject) => { - const output = fs.createWriteStream(filePath); - const archive = archiver('zip', { zlib: { level: 9 } }); - output.on('close', resolve); - output.on('error', reject); - archive.on('error', reject); - // Surface archiver warnings (e.g. a file vanished mid-run) instead of - // silently shipping an incomplete archive. - archive.on('warning', (err) => reject(err)); - archive.pipe(output); - archive.file(path.join(staging, 'manifest.json'), { name: 'manifest.json' }); - archive.directory(dataDir, 'data'); - for (const f of files) { - archive.file(f.abs, { name: path.posix.join('files', f.rel.split(path.sep).join('/')) }); - } - archive.finalize(); - }); + try { + await new Promise((resolve, reject) => { + const output = fs.createWriteStream(filePath); + const archive = archiver('zip', { zlib: { level: 9 } }); + output.on('close', resolve); + output.on('error', reject); + archive.on('error', reject); + // Surface archiver warnings (e.g. a file vanished mid-run) instead of + // silently shipping an incomplete archive. + archive.on('warning', (err) => reject(err)); + archive.pipe(output); + archive.file(path.join(staging, 'manifest.json'), { name: 'manifest.json' }); + archive.directory(dataDir, 'data'); + for (const f of files) { + archive.file(f.abs, { name: path.posix.join('files', f.rel.split(path.sep).join('/')) }); + } + archive.finalize(); + }); + } catch (err) { + // Archiver failed → the partial .picpeak holds plaintext secrets and is + // useless; remove our own temp out dir so it isn't orphaned. A + // caller-supplied outDir is left untouched. + if (!outDir) await fsp.rm(targetDir, { recursive: true, force: true }).catch(() => {}); + throw err; + } logger.info( `[picpeak-export] wrote ${filePath} (${tables.length} tables, ${files.length} files, includePhotos=${!!includePhotos})` @@ -215,6 +223,7 @@ async function createPicpeak({ includePhotos = false, outDir } = {}) { module.exports = { PICPEAK_FORMAT_VERSION, + EXCLUDED_TABLES, createPicpeak, // exported for reuse/testing listDataTables, diff --git a/backend/src/services/picpeakImportService.js b/backend/src/services/picpeakImportService.js index ae37455d..b1a6b0f3 100644 --- a/backend/src/services/picpeakImportService.js +++ b/backend/src/services/picpeakImportService.js @@ -23,7 +23,7 @@ const knexConfig = require('../../knexfile'); const { getStoragePath } = require('../config/storage'); const { hasColumnCached } = require('../utils/schemaCache'); const logger = require('../utils/logger'); -const { PICPEAK_FORMAT_VERSION } = require('./picpeakExportService'); +const { PICPEAK_FORMAT_VERSION, EXCLUDED_TABLES, listDataTables } = require('./picpeakExportService'); const isPostgres = () => knexConfig.client === 'pg'; @@ -129,8 +129,23 @@ function serialiseJsonColumns(rows, jsonCols) { // in the data set, so the target's schema/migration state is left intact. async function replaceAllTables(tables, dataDir, currentAdmin) { await db.transaction(async (trx) => { - if (isPostgres()) await trx.raw("SET session_replication_role = 'replica'"); - else await trx.raw('PRAGMA defer_foreign_keys = ON'); + if (isPostgres()) { + try { + await trx.raw("SET session_replication_role = 'replica'"); + } catch (_) { + // session_replication_role requires a Postgres SUPERUSER. The bundled + // postgres image's role is one; managed Postgres (RDS / Cloud SQL / …) + // app users usually are not. Fail fast with a clear message BEFORE any + // rows are deleted — the transaction rolls back, so nothing is wiped. + const err = new Error( + 'Restore needs a PostgreSQL superuser to suspend foreign-key checks during the full replace, but this instance’s database user is not a superuser (common on managed Postgres such as RDS or Cloud SQL). Restore onto the bundled Postgres, or grant the role superuser for the restore.' + ); + err.statusCode = 400; + throw err; + } + } else { + await trx.raw('PRAGMA defer_foreign_keys = ON'); + } for (const table of tables) { await trx(table).del(); @@ -223,7 +238,18 @@ async function importFromPicpeak({ picpeakPath, currentAdminId }) { } const dataDir = path.join(staging, 'data'); - const tables = Object.keys(manifest.tables || {}); + // Only touch tables that (a) the uploaded manifest lists AND (b) actually + // exist as real tables in THIS database. listDataTables() already excludes + // knex_migrations/_lock (EXCLUDED_TABLES), so a crafted or corrupted + // .picpeak can never make the restore delete the migration bookkeeping — or + // any table that isn't a genuine data table here. + const dbTables = new Set(await listDataTables()); + const manifestTables = Object.keys(manifest.tables || {}); + const tables = manifestTables.filter((tbl) => dbTables.has(tbl) && !EXCLUDED_TABLES.has(tbl)); + const skipped = manifestTables.filter((tbl) => !tables.includes(tbl)); + if (skipped.length) { + logger.warn(`[picpeak-import] ignoring ${skipped.length} backup table(s) not present in this DB (or protected): ${skipped.join(', ')}`); + } await replaceAllTables(tables, dataDir, currentAdmin); const filesRestored = await restoreFiles(staging);