diff --git a/backend/__tests__/integration/backupService.configurableWalker.test.js b/backend/__tests__/integration/backupService.configurableWalker.test.js new file mode 100644 index 00000000..65683046 --- /dev/null +++ b/backend/__tests__/integration/backupService.configurableWalker.test.js @@ -0,0 +1,180 @@ +/** + * Pins the Stage-B refactor that lifted the file-backup walker's + * subdirectory list out of hard-coded JS into the `backup_paths` + * table seeded by migration 108. + * + * Scenarios: + * 1. Walker reads canonical seed → all 7 default subdirs walked + * 2. include_in_default=false on one row → that subdir is skipped + * 3. New row inserted at runtime → walker picks it up without restart + * 4. feature_flag gating → row only walked when the named app_settings + * boolean is truthy (mirrors historical `includeArchived` behavior) + * 5. Empty table → walker falls back to LEGACY_BACKUP_PATHS (defense + * in depth — never silently scans nothing) + * + * Why not stub `db('backup_paths')`: the whole point of Stage B is + * that the walker is now data-driven, so the test has to actually + * mutate the table and observe the walker's output change. Stubs + * would re-introduce the hard-coding the refactor is meant to remove. + */ + +const fs = require('fs'); +const path = require('path'); + +const { bootCrmDb } = require('./helpers/crmDb'); + +jest.setTimeout(30000); + +describe('backupService — configurable walker (backup_paths)', () => { + let db; + let cleanup; + let storagePath; + let backupService; + + beforeAll(async () => { + ({ db, cleanup } = await bootCrmDb()); + storagePath = process.env.STORAGE_PATH; + backupService = require('../../src/services/backupService'); + }, 120000); + + afterAll(async () => { + if (cleanup) await cleanup(); + }); + + function seedFile(relPath, content = 'dummy bytes') { + const abs = path.join(storagePath, relPath); + fs.mkdirSync(path.dirname(abs), { recursive: true }); + fs.writeFileSync(abs, content); + } + + beforeEach(async () => { + // Restore canonical seed before every test. Tests mutate this table + // freely; the next test starts from a known state. + await db('backup_paths').del(); + const { + DEFAULT_PATHS, + } = require('../../migrations/core/108_add_backup_paths'); + await db('backup_paths').insert(DEFAULT_PATHS.map((row) => ({ + ...row, + created_at: new Date(), + updated_at: new Date(), + }))); + }); + + it('migration 108 seeds the canonical 7 paths', async () => { + const rows = await db('backup_paths').orderBy('display_order', 'asc').select(); + expect(rows.map((r) => r.path)).toEqual([ + 'events/active', + 'events/archived', + 'thumbnails', + 'previews', + 'heroes', + 'uploads', + 'business-docs', + ]); + // Only events/archived is gated by a feature flag. + expect(rows.filter((r) => r.feature_flag).map((r) => r.path)).toEqual([ + 'events/archived', + ]); + }); + + it('walks every default subdir when files are present', async () => { + seedFile('events/active/E1/a.jpg'); + seedFile('thumbnails/E1/a.jpg'); + seedFile('previews/E1/a.jpg'); + seedFile('heroes/E1/hero.jpg'); + seedFile('uploads/intake/x.bin'); + seedFile('business-docs/quote/2026/Q-001.pdf'); + // events/archived is gated — left out of this test; covered below. + + const files = await backupService.getFilesToBackup({ backup_include_archived: true }); + const rels = files.map((f) => f.relativePath); + + expect(rels).toEqual(expect.arrayContaining([ + 'events/active/E1/a.jpg', + 'thumbnails/E1/a.jpg', + 'previews/E1/a.jpg', + 'heroes/E1/hero.jpg', + 'uploads/intake/x.bin', + 'business-docs/quote/2026/Q-001.pdf', + ])); + }); + + it('skips a path when include_in_default is toggled off', async () => { + seedFile('thumbnails/E1/thumb.jpg'); + seedFile('events/active/E1/photo.jpg'); + + await db('backup_paths').where('path', 'thumbnails').update({ + include_in_default: false, + }); + + const files = await backupService.getFilesToBackup({ backup_include_archived: true }); + const rels = files.map((f) => f.relativePath); + + expect(rels).toContain('events/active/E1/photo.jpg'); + expect(rels).not.toContain('thumbnails/E1/thumb.jpg'); + }); + + it('picks up a new path inserted at runtime — no restart needed', async () => { + // Simulates a future feature shipping its own subdirectory and + // self-healing a `backup_paths` row at boot. + await db('backup_paths').insert({ + path: 'plugin-store', + include_in_default: true, + feature_flag: null, + display_order: 200, + description: 'Hypothetical future feature payload', + created_at: new Date(), + updated_at: new Date(), + }); + seedFile('plugin-store/cache/payload.bin'); + + const files = await backupService.getFilesToBackup({ backup_include_archived: true }); + const rels = files.map((f) => f.relativePath); + + expect(rels).toContain('plugin-store/cache/payload.bin'); + }); + + it('respects feature_flag gating (events/archived ⇄ backup_include_archived)', async () => { + seedFile('events/active/E1/active.jpg'); + seedFile('events/archived/E2/archived.jpg'); + + // backup_include_archived=false → archived/ is skipped. + const filesOff = await backupService.getFilesToBackup({ backup_include_archived: false }); + const relsOff = filesOff.map((f) => f.relativePath); + expect(relsOff).toContain('events/active/E1/active.jpg'); + expect(relsOff).not.toContain('events/archived/E2/archived.jpg'); + + // backup_include_archived=true → archived/ is included. + const filesOn = await backupService.getFilesToBackup({ backup_include_archived: true }); + const relsOn = filesOn.map((f) => f.relativePath); + expect(relsOn).toContain('events/archived/E2/archived.jpg'); + }); + + it('falls back to LEGACY_BACKUP_PATHS when the table is empty', async () => { + // Defense in depth: even if seed-and-self-heal both failed, the + // walker must still cover the historical set so "Run Backup Now" + // cannot silently degrade to no-op. + await db('backup_paths').del(); + seedFile('events/active/E1/photo.jpg'); + seedFile('business-docs/quote/2026/Q-002.pdf'); + + const files = await backupService.getFilesToBackup({ backup_include_archived: true }); + const rels = files.map((f) => f.relativePath); + + expect(rels).toContain('events/active/E1/photo.jpg'); + expect(rels).toContain('business-docs/quote/2026/Q-002.pdf'); + }); + + it('legacy boolean call signature still works (backward compat)', async () => { + // Existing call sites (and the businessDocs regression test) pass + // a boolean for `includeArchived`. Refactor must not break them. + seedFile('events/archived/E3/legacy.jpg'); + + const filesOff = await backupService.getFilesToBackup(false); + expect(filesOff.map((f) => f.relativePath)).not.toContain('events/archived/E3/legacy.jpg'); + + const filesOn = await backupService.getFilesToBackup(true); + expect(filesOn.map((f) => f.relativePath)).toContain('events/archived/E3/legacy.jpg'); + }); +}); diff --git a/backend/migrations/core/108_add_backup_paths.js b/backend/migrations/core/108_add_backup_paths.js new file mode 100644 index 00000000..5fddccf6 --- /dev/null +++ b/backend/migrations/core/108_add_backup_paths.js @@ -0,0 +1,131 @@ +/** + * Migration 108 — config-driven backup walker. + * + * Stage B of the three-stage backup-hardening plan. The file-backup + * walker (`getFilesToBackupInternal` in backupService.js) historically + * hard-coded its list of subdirectories: events/active, events/archived, + * thumbnails, previews, heroes, uploads, business-docs. + * + * That list is a footgun every time a new feature lands that drops + * artefacts under STORAGE_PATH// — the maintainer has to + * remember to edit the walker, and there's no schema-level record of + * what *should* be backed up. The CRM rollout missed `business-docs` + * for ~6 months (#XXX) for exactly this reason. + * + * This migration introduces a `backup_paths` table that the walker + * reads at runtime. New features add a row; the walker picks them up + * automatically. The `feature_flag` column gates scans behind an + * existing app_settings boolean (e.g. `backup_include_archived`), + * mirroring how the previous `includeArchived` parameter worked. + * + * Columns: + * - path : relative to STORAGE_PATH, unique + * - include_in_default : on/off without deleting the row (so + * audit trail of "we used to back this up" + * is preserved) + * - feature_flag : nullable; when set, walker checks the + * same-named app_settings boolean before + * scanning. Matches the existing pattern + * used by `backup_include_archived`. + * - display_order : controls admin-UI listing order + * - description : human-readable purpose, shown in admin UI + * + * Defense-in-depth: the walker also keeps a hard-coded LEGACY_DEFAULTS + * fallback so that if this table is somehow empty (failed migration on + * an existing install, manual truncation), backups still cover the + * historical set instead of silently shipping nothing. The boot-time + * self-heal in `_backupPathsBoot.js` re-seeds missing default rows on + * every startup so newly-added defaults reach already-deployed + * installs without a follow-up migration. + * + * Idempotent: skips the createTable if it already exists, and the + * seed uses `onConflict('path').ignore()` so re-runs don't duplicate. + */ + +const DEFAULT_PATHS = [ + { + path: 'events/active', + include_in_default: true, + feature_flag: null, + display_order: 10, + description: 'Active gallery photo originals', + }, + { + path: 'events/archived', + include_in_default: true, + feature_flag: 'backup_include_archived', + display_order: 20, + description: 'Archived gallery photo originals (gated by backup_include_archived)', + }, + { + path: 'thumbnails', + include_in_default: true, + feature_flag: null, + display_order: 30, + description: 'Generated gallery thumbnails', + }, + { + path: 'previews', + include_in_default: true, + feature_flag: null, + display_order: 40, + description: 'Lightbox preview tier (#492)', + }, + { + path: 'heroes', + include_in_default: true, + feature_flag: null, + display_order: 50, + description: 'Gallery hero header images', + }, + { + path: 'uploads', + include_in_default: true, + feature_flag: null, + display_order: 60, + description: 'Direct uploads root (wet-signature contracts, imported invoices, etc.)', + }, + { + path: 'business-docs', + include_in_default: true, + feature_flag: null, + display_order: 70, + description: 'CRM PDFs, signature artefacts, admin-imported historical invoices', + }, +]; + +exports.up = async function(knex) { + const exists = await knex.schema.hasTable('backup_paths'); + if (!exists) { + await knex.schema.createTable('backup_paths', (t) => { + t.increments('id').primary(); + t.string('path', 256).notNullable().unique(); + t.boolean('include_in_default').notNullable().defaultTo(true); + t.string('feature_flag', 64).nullable(); + t.integer('display_order').notNullable().defaultTo(100); + t.string('description', 256).nullable(); + t.timestamp('created_at').notNullable().defaultTo(knex.fn.now()); + t.timestamp('updated_at').notNullable().defaultTo(knex.fn.now()); + }); + } + + // Seed defaults — `onConflict('path').ignore()` so already-seeded rows + // (manual edits by admins, prior partial runs) survive untouched. + await knex('backup_paths') + .insert(DEFAULT_PATHS.map((row) => ({ + ...row, + created_at: new Date(), + updated_at: new Date(), + }))) + .onConflict('path') + .ignore(); +}; + +exports.down = async function(knex) { + await knex.schema.dropTableIfExists('backup_paths'); +}; + +// Exported so the self-heal boot helper can reuse the same authoritative +// list without re-declaring it. Tests also import this to assert the +// walker is reading from this source. +exports.DEFAULT_PATHS = DEFAULT_PATHS; diff --git a/backend/server.js b/backend/server.js index f7c5d7c0..527efc4e 100644 --- a/backend/server.js +++ b/backend/server.js @@ -800,6 +800,18 @@ async function startServer() { const { startS3AutoImporter } = require('./src/services/s3AutoImporter'); startS3AutoImporter(); + // Self-heal the `backup_paths` table before the backup service + // starts — the file-backup walker reads from it, so missing + // canonical rows (a new subdirectory shipped by a future feature) + // get re-seeded here on every boot. See _backupPathsBoot.js for + // the full rationale; pattern mirrors _emailTemplateBoot.js. + try { + const { seedBackupPathsAtBoot } = require('./src/services/_backupPathsBoot'); + await seedBackupPathsAtBoot(db, logger); + } catch (err) { + logger.warn('backup_paths self-heal failed at boot:', err.message); + } + // Start backup service await startBackupService(); diff --git a/backend/src/services/_backupPathsBoot.js b/backend/src/services/_backupPathsBoot.js new file mode 100644 index 00000000..d1c8ece1 --- /dev/null +++ b/backend/src/services/_backupPathsBoot.js @@ -0,0 +1,90 @@ +/** + * Boot-time self-heal for the `backup_paths` table. + * + * **Why this exists** + * + * Knex won't re-run an applied migration, so once migration + * 108_add_backup_paths.js has run, any later default we want to add + * (a new subdirectory shipped by a future feature) would never reach + * already-deployed installs. The historical fix for this kind of + * "schema is fine, seed drifted" problem is the boot-time self-heal + * pattern documented in [[feedback_self_heal_pattern]] — we just + * re-apply the canonical seed on every boot with `onConflict.ignore()` + * so admin edits stay intact and new rows trickle in. + * + * **Authoritative list** + * + * The list of defaults lives on migration 108 itself + * (`DEFAULT_PATHS` export) — one source of truth that both the + * migration and this seeder read. Tests assert these two stay in + * lockstep. + * + * **Failure semantics** + * + * If the table doesn't exist yet (migrations haven't run, fresh + * install before migration 108 lands, etc.) we no-op and log. The + * walker has a hard-coded `LEGACY_DEFAULTS` fallback for the same + * reason — defense in depth so "Run Backup Now" can never silently + * ship a files-only manifest because of a seed issue. See + * `backupService.js` getFilesToBackupInternal. + */ + +const { DEFAULT_PATHS } = require('../../migrations/core/108_add_backup_paths'); + +let booted = false; + +/** + * Idempotently re-seed `backup_paths` with the canonical defaults. + * + * @param {object} db knex instance + * @param {object} logger app logger (must expose .info / .warn) + * @returns {Promise<{ seeded: string[] }>} paths newly inserted on this boot. + */ +async function seedBackupPathsAtBoot(db, logger) { + const log = logger || { info: () => {}, warn: () => {} }; + if (booted) return { seeded: [] }; + + if (!(await db.schema.hasTable('backup_paths'))) { + log.warn('backup_paths table missing at boot — self-heal skipped (migration 108 may not have run yet)'); + return { seeded: [] }; + } + + // Diff: which canonical paths are missing from the table right now? + // We can't easily get "what got inserted by onConflict.ignore" out of + // knex on both backends, so we just compute the diff ourselves and log + // it — admins benefit from seeing exactly what got auto-added when a + // new feature ships. + const existing = await db('backup_paths').select('path'); + const existingSet = new Set(existing.map((r) => r.path)); + const missing = DEFAULT_PATHS.filter((p) => !existingSet.has(p.path)); + + if (missing.length === 0) { + booted = true; + return { seeded: [] }; + } + + try { + await db('backup_paths') + .insert(missing.map((row) => ({ + ...row, + created_at: new Date(), + updated_at: new Date(), + }))) + .onConflict('path') + .ignore(); + log.info(`backup_paths self-heal added ${missing.length} row(s): ${missing.map((m) => m.path).join(', ')}`); + } catch (err) { + log.warn(`backup_paths self-heal failed: ${err.message}`); + } + + booted = true; + return { seeded: missing.map((m) => m.path) }; +} + +// Test-only: reset the module-level boot flag so jest can re-exercise +// the seeder against a fresh test DB inside a single worker. +function _resetBootForTests() { + booted = false; +} + +module.exports = { seedBackupPathsAtBoot, _resetBootForTests }; diff --git a/backend/src/services/backupService.js b/backend/src/services/backupService.js index 118e636c..eb9f4b71 100644 --- a/backend/src/services/backupService.js +++ b/backend/src/services/backupService.js @@ -417,43 +417,111 @@ async function scanDirectory(dirPath, fileList, basePath, excludePatterns = []) } } -async function getFilesToBackupInternal(includeArchived = true) { +/** + * Hard-coded fallback when `backup_paths` is missing/empty. Mirrors + * the canonical seed in migration 108 — kept here as defense in depth + * so the walker can never silently degrade to "no directories scanned" + * because of a seed problem. + * + * Order matches the legacy behavior of the inlined sequence this + * function used to contain. + */ +const LEGACY_BACKUP_PATHS = [ + { path: 'events/active', feature_flag: null }, + { path: 'events/archived', feature_flag: 'backup_include_archived' }, + { path: 'thumbnails', feature_flag: null }, + { path: 'previews', feature_flag: null }, + { path: 'heroes', feature_flag: null }, + { path: 'uploads', feature_flag: null }, + { path: 'business-docs', feature_flag: null }, +]; + +/** + * Resolve the walker's target subdirectories from `backup_paths`. + * + * Layered fallback (defense in depth — no scenario where the walker + * silently scans nothing): + * 1. Read `backup_paths` rows where include_in_default = true, + * ordered by display_order. + * 2. If the table is missing OR returns zero rows, fall back to + * LEGACY_BACKUP_PATHS. Logged loudly so the admin sees it. + * + * Per-row gating: when `feature_flag` is set, the corresponding + * config key in `app_settings` must resolve truthy for that path to + * be included. Mirrors the historical `includeArchived` parameter, + * but now driven by data instead of a hard-coded boolean. + * + * @param {object} config resolved backup config (parseSettingValue'd). + * Used to evaluate feature_flag gates. + * @returns {Promise>} + */ +async function resolveBackupPaths(config) { + let rows; + try { + if (!(await db.schema.hasTable('backup_paths'))) { + logger.warn('backup_paths table missing — falling back to LEGACY_BACKUP_PATHS'); + rows = LEGACY_BACKUP_PATHS; + } else { + rows = await db('backup_paths') + .where('include_in_default', formatBoolean(true)) + .orderBy('display_order', 'asc') + .select('path', 'feature_flag'); + if (!rows.length) { + logger.warn('backup_paths has no rows with include_in_default=true — falling back to LEGACY_BACKUP_PATHS'); + rows = LEGACY_BACKUP_PATHS; + } + } + } catch (err) { + logger.warn(`Failed to query backup_paths (${err.message}) — falling back to LEGACY_BACKUP_PATHS`); + rows = LEGACY_BACKUP_PATHS; + } + + // Apply feature_flag gating. A row with feature_flag='backup_include_archived' + // requires config.backup_include_archived to be truthy (same semantics as + // the historical `includeArchived` parameter). + return rows.filter((row) => { + if (!row.feature_flag) return true; + const flagValue = config ? config[row.feature_flag] : undefined; + return normalizeBoolean(flagValue); + }); +} + +async function getFilesToBackupInternal(configOrIncludeArchived = true) { const files = []; const storagePath = getStoragePath(); - await scanDirectory(path.join(storagePath, 'events/active'), files, storagePath); - - if (normalizeBoolean(includeArchived)) { - await scanDirectory(path.join(storagePath, 'events/archived'), files, storagePath); + // Backward-compatible call signature: + // - Boolean `true|false` → legacy `includeArchived` argument. We + // forge a config-shaped object so the feature-flag gating + // resolves the same way the old code path did. + // - Object → full resolved backup config (preferred). + // - Anything else → treated as "include archived" (truthy). + let config; + if (typeof configOrIncludeArchived === 'object' && configOrIncludeArchived !== null) { + config = configOrIncludeArchived; + } else { + config = { backup_include_archived: normalizeBoolean(configOrIncludeArchived) }; } - await scanDirectory(path.join(storagePath, 'thumbnails'), files, storagePath); - // Lightbox preview tier (#492). Cheap to back up — typically a few - // hundred KB per photo — and saves admins the regenerate cycle on - // a restore. Tolerated when missing (admins who never enabled the - // feature won't have the folder; scanDirectory short-circuits on - // ENOENT cleanly). - await scanDirectory(path.join(storagePath, 'previews'), files, storagePath); - // Heroes too — same logic; admins who picked a hero photo for the - // gallery header had its 1920x1080 file generated and was missed - // by the original backup walk before this addition. - await scanDirectory(path.join(storagePath, 'heroes'), files, storagePath); - await scanDirectory(path.join(storagePath, 'uploads'), files, storagePath); - // CRM document estate — every PDF and signature artefact the - // service persists for legal-evidence purposes: - // - business-docs/quote//*.pdf - // - business-docs/contract//*.pdf (system-rendered + wet uploads) - // - business-docs/contract/signatures//*.{png,jpg} - // (drawn signatures, forensic-preserved per Date.now() filename) - // - business-docs/invoice//*.pdf (issued invoices + Storno) - // - business-docs/invoice-imports//*.pdf (admin-imported - // historical invoices — irrecoverable if not backed up) - // Without this scan, the audit trail (signed_pdf_sha256, signed_* - // _ip, accepted_at, etc.) survives the restore but the documents - // those values refer to do not, leaving every CRM *_path column a - // broken FK. scanDirectory short-circuits on ENOENT so installs - // that never used CRM features won't error. - await scanDirectory(path.join(storagePath, 'business-docs'), files, storagePath); + const targets = await resolveBackupPaths(config); + + for (const target of targets) { + // CRM document estate is special-cased in the comment block below + // because it's the most expensive omission to recover from: + // - business-docs/quote//*.pdf + // - business-docs/contract//*.pdf (system-rendered + wet uploads) + // - business-docs/contract/signatures//*.{png,jpg} + // (drawn signatures, forensic-preserved per Date.now() filename) + // - business-docs/invoice//*.pdf (issued invoices + Storno) + // - business-docs/invoice-imports//*.pdf (admin-imported + // historical invoices — irrecoverable if not backed up) + // Without this scan, the audit trail (signed_pdf_sha256, signed_* + // _ip, accepted_at, etc.) survives the restore but the documents + // those values refer to do not, leaving every CRM *_path column a + // broken FK. scanDirectory short-circuits on ENOENT so installs + // that never used CRM features won't error. + await scanDirectory(path.join(storagePath, target.path), files, storagePath); + } return files; } @@ -887,7 +955,11 @@ async function runBackupInternal(isManual = false) { // for the full rationale. const verifiedDatabaseInfo = await ensureDatabaseDumpForBackup(config); - const files = await service.getFilesToBackup(config.backup_include_archived); + // Pass the full config so the walker can evaluate any feature_flag + // gates declared in the backup_paths table (e.g. `events/archived` + // gated by `backup_include_archived`). Boolean signature is still + // supported for legacy callers and tests — see getFilesToBackupInternal. + const files = await service.getFilesToBackup(config); logger.info(`Found ${files.length} files to check for backup`); let result;