From a9280ea9bac731199133a34ede5a89359bcde89e Mon Sep 17 00:00:00 2001 From: Luca <102960244+Luca-Timo@users.noreply.github.com> Date: Fri, 29 May 2026 12:50:02 +0200 Subject: [PATCH 01/42] fix(backup): include storage/business-docs/ in the in-app backup walker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit backupService.getFilesToBackupInternal() enumerated a fixed list of storage subdirectories (events/active, events/archived, thumbnails, previews, heroes, uploads) and silently omitted the entire business-docs/ tree. Every CRM PDF artefact and signature image fell outside the in-app scheduled backup — restoring the DB without the PDFs would have left every *_path column on quotes/contracts/invoices as a broken FK and lost forensic evidence (the customer signature PNG/JPG drawn on the public signing page is referenced by contracts.signed_customer_signature_path; the rendered contract PDF is referenced by signed_pdf_path with a stored signed_pdf_sha256 that would have nothing to verify against; wet-uploaded contracts and admin-imported historical invoices are irrecoverable by design since no renderer can reproduce them). Single new scanDirectory call after the existing uploads scan, covering: - business-docs/quote//*.pdf - business-docs/contract//*.pdf - business-docs/contract/signatures//*.{png,jpg} - business-docs/invoice//*.pdf - business-docs/invoice-imports//*.pdf - and incidentally business-docs/dev-test/ (managed by adminDev.js, bounded to 7 newest files, harmless to back up) Verified that no migration is needed: hasFileChanged returns !existing || checksum mismatch, so the first backup after this lands flags every business-docs/** file as new and copies it. Restore path in restoreService.performFilesRestore uses fs.mkdir({ recursive: true }) on path.dirname(targetPath), so business-docs subdirectories are recreated automatically from manifest entries — no restore-side code change required. Integration test pins the contract so a future refactor cannot silently drop business-docs again. The shell-script backup at scripts/backup.sh already covered all of this via blanket `tar -czf storage`; only the in-app service was affected. --- .../2026-05-22-idor-crm-admin-endpoints.md | 1 + .../backupService.businessDocs.test.js | 88 +++++++++++++++++++ backend/src/services/backupService.js | 15 ++++ 3 files changed, 104 insertions(+) create mode 100644 .claude/security-reports/2026-05-22-idor-crm-admin-endpoints.md create mode 100644 backend/__tests__/integration/backupService.businessDocs.test.js diff --git a/.claude/security-reports/2026-05-22-idor-crm-admin-endpoints.md b/.claude/security-reports/2026-05-22-idor-crm-admin-endpoints.md new file mode 100644 index 00000000..8b137891 --- /dev/null +++ b/.claude/security-reports/2026-05-22-idor-crm-admin-endpoints.md @@ -0,0 +1 @@ + diff --git a/backend/__tests__/integration/backupService.businessDocs.test.js b/backend/__tests__/integration/backupService.businessDocs.test.js new file mode 100644 index 00000000..2b74b161 --- /dev/null +++ b/backend/__tests__/integration/backupService.businessDocs.test.js @@ -0,0 +1,88 @@ +/** + * Regression net for the business-docs coverage gap fixed in this PR. + * + * Prior to the fix, `getFilesToBackupInternal()` enumerated a fixed + * list of storage subdirectories (events/active, events/archived, + * thumbnails, previews, heroes, uploads) and silently omitted the + * entire `business-docs/` tree. That meant every CRM PDF + signature + * drawing — quotes, contracts (system-rendered + wet uploads), + * invoices, Storno, imported historical invoices, and the customer + * signature PNG/JPG drawn on the public signing page — fell outside + * the in-app scheduled backup, leaving every `*_path` column on + * `quotes` / `contracts` / `invoices` as a broken FK after restore. + * + * The fix is a single `scanDirectory(business-docs, ...)` call. This + * suite pins the contract so a future refactor of the walker cannot + * silently drop business-docs again. + */ + +const fs = require('fs'); +const path = require('path'); + +const { bootCrmDb } = require('./helpers/crmDb'); + +describe('backupService — business-docs is in the backup walker', () => { + let cleanup; + let backupService; + let storagePath; + + beforeAll(async () => { + ({ cleanup } = await bootCrmDb()); + storagePath = process.env.STORAGE_PATH; + // Cold-require after bootCrmDb so backupService picks up the same + // db instance + STORAGE_PATH the test harness configured. + backupService = require('../../src/services/backupService'); + }, 120000); + + afterAll(async () => { + if (cleanup) await cleanup(); + }); + + function seed(relPath, content = 'dummy bytes for backup test') { + const abs = path.join(storagePath, relPath); + fs.mkdirSync(path.dirname(abs), { recursive: true }); + fs.writeFileSync(abs, content); + } + + it('does not error when business-docs is absent', async () => { + // Fresh harness has no business-docs/ tree at all. The walker + // must short-circuit on ENOENT rather than throw — installs that + // never used CRM features have to keep backing up fine. + await expect(backupService.getFilesToBackup(false)).resolves.toEqual(expect.any(Array)); + }); + + it('picks up every CRM-relevant business-docs subdirectory', async () => { + // Seed one file in each of the five subpaths the renderer + import + // routes write to. The signature path is the one most prone to be + // forgotten — it lives one level deeper than the others (per- + // contract subfolder, not per-year). + seed('business-docs/quote/2026/Q-001.pdf'); + seed('business-docs/contract/2026/C-001.pdf'); + seed('business-docs/contract/signatures/42/customer-1700000000000.png'); + seed('business-docs/invoice/2026/INV-001.pdf'); + seed('business-docs/invoice-imports/2026/scan.pdf'); + + const files = await backupService.getFilesToBackup(false); + const rels = files.map((f) => f.relativePath); + + expect(rels).toEqual(expect.arrayContaining([ + 'business-docs/quote/2026/Q-001.pdf', + 'business-docs/contract/2026/C-001.pdf', + 'business-docs/contract/signatures/42/customer-1700000000000.png', + 'business-docs/invoice/2026/INV-001.pdf', + 'business-docs/invoice-imports/2026/scan.pdf', + ])); + }); + + it('walks newly-created business-docs files without needing a restart', async () => { + // The walker reads the filesystem live on every call; this guards + // against a future "cache the scan result at boot" optimisation + // that would miss freshly-written PDFs (which is exactly what + // happens during normal operation — every send writes a new file). + seed('business-docs/invoice/2027/INV-NEW.pdf'); + + const files = await backupService.getFilesToBackup(false); + const rels = files.map((f) => f.relativePath); + expect(rels).toContain('business-docs/invoice/2027/INV-NEW.pdf'); + }); +}); diff --git a/backend/src/services/backupService.js b/backend/src/services/backupService.js index 033688c4..463a9f33 100644 --- a/backend/src/services/backupService.js +++ b/backend/src/services/backupService.js @@ -372,6 +372,21 @@ async function getFilesToBackupInternal(includeArchived = true) { // 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); return files; } From 4812fcdec38d7bfdbb5eac80eb6fe004f405b970 Mon Sep 17 00:00:00 2001 From: Luca <102960244+Luca-Timo@users.noreply.github.com> Date: Fri, 29 May 2026 13:00:18 +0200 Subject: [PATCH 02/42] feat(backup): admin endpoint to verify CRM document-artefact integrity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Diagnostic for the bug fixed in a9280ea — confirms every *_path column on quotes / contracts / invoices points at a file that actually exists on disk and (where a *_sha256 column is set) the file's bytes still hash to the expected value. Read-only; on-demand only; no scheduler. Per the design decisions locked in this PR's design call: D1 — on-demand only for v1; scheduling deferred until we have runtime data on large installs D2 — not auto-triggered after restore; surface a "verify integrity now" CTA on the restore-completed screen instead D3 — wet-upload contracts hash-verified same as system-rendered (signed_pdf_sha256 is computed at upload time, no special case needed in the verifier) Coverage (single source of truth in backupIntegrityService.CHECKS): quotes.pdf_path existence contracts.pdf_path + pdf_sha256 existence + hash contracts.signed_pdf_path + signed_pdf_sha256 existence + hash contracts.signed_customer_signature_path existence (PNG/JPG, no hash) contracts.signed_admin_signature_path existence (PNG/JPG, no hash) invoices.pdf_path existence invoices.imported_pdf_path existence (admin-uploaded scans) Report shape buckets each row into verifiedOk / missing / hashMismatches / existsButNoHash so callers can distinguish hash- verified from existence-only — the latter is weaker evidence in a legal dispute and the UI should reflect that. Route GET /api/admin/system-health/backup-integrity accepts an optional ?scope= CSV filter (quote | contract | contract-signature | invoice). Unknown scope tokens are rejected with a 400 + BACKUP_INTEGRITY_UNKNOWN_SCOPE code rather than silently scanning everything. Frontend half (BackupIntegrityCard on a System Health page) is deferred until backlog #11 (System Health page) is scaffolded. The endpoint is independently useful via curl in the meantime. --- .../integration/adminBackupIntegrity.test.js | 140 +++++++++++ .../services/backupIntegrityService.test.js | 216 +++++++++++++++++ backend/server.js | 1 + backend/src/routes/adminSystemHealth.js | 64 +++++ .../src/services/backupIntegrityService.js | 228 ++++++++++++++++++ 5 files changed, 649 insertions(+) create mode 100644 backend/__tests__/integration/adminBackupIntegrity.test.js create mode 100644 backend/__tests__/services/backupIntegrityService.test.js create mode 100644 backend/src/routes/adminSystemHealth.js create mode 100644 backend/src/services/backupIntegrityService.js diff --git a/backend/__tests__/integration/adminBackupIntegrity.test.js b/backend/__tests__/integration/adminBackupIntegrity.test.js new file mode 100644 index 00000000..9a046cc8 --- /dev/null +++ b/backend/__tests__/integration/adminBackupIntegrity.test.js @@ -0,0 +1,140 @@ +/** + * Integration test for GET /api/admin/system-health/backup-integrity. + * + * Auth + permission middleware are mocked to pass-through so the test + * focuses on the route's own behaviour: scope-param validation, the + * successResponse envelope, and that the underlying service report + * surfaces correctly in the JSON body. + * + * The verifier service itself is exercised against the real schema + * (bootCrmDb) and real filesystem — only the auth gate is stubbed. + */ + +const fs = require('fs'); +const path = require('path'); +const express = require('express'); +const request = require('supertest'); + +const { bootCrmDb, seedMinimal } = require('./helpers/crmDb'); + +// Pass-through auth so we don't need to mint JWTs. +jest.mock('../../src/middleware/auth', () => ({ + adminAuth: (req, _res, next) => { req.admin = { id: 1 }; next(); }, + customerAuth: (_req, _res, next) => next(), + galleryAuth: (_req, _res, next) => next(), +})); + +// Pass-through permissions so settings.view always allows. +jest.mock('../../src/middleware/permissions', () => ({ + requirePermission: () => (_req, _res, next) => next(), +})); + +jest.setTimeout(30000); + +describe('GET /api/admin/system-health/backup-integrity', () => { + let cleanup; + let db; + let customerId; + let app; + let storagePath; + + beforeAll(async () => { + ({ db, cleanup } = await bootCrmDb()); + ({ customerId } = await seedMinimal(db)); + storagePath = process.env.STORAGE_PATH; + + // Mount the route on a minimal Express app. Cold-require after + // bootCrmDb so the route's downstream `require('../database/db')` + // sees the same db instance. + const route = require('../../src/routes/adminSystemHealth'); + app = express(); + app.use(express.json()); + app.use('/api/admin/system-health', route); + }, 120000); + + afterAll(async () => { + if (cleanup) await cleanup(); + }); + + beforeEach(async () => { + await db('contracts').del().catch(() => {}); + await db('invoices').del().catch(() => {}); + await db('quotes').del().catch(() => {}); + }); + + it('returns a report envelope when nothing references any path', async () => { + const res = await request(app).get('/api/admin/system-health/backup-integrity'); + expect(res.status).toBe(200); + expect(res.body).toHaveProperty('report'); + expect(res.body.report.summary).toMatchObject({ + totalRows: 0, + missingFiles: 0, + hashMismatches: 0, + verifiedOk: 0, + existsButNoHash: 0, + }); + expect(res.body.report.scopes).toEqual(expect.arrayContaining([ + 'quote', 'contract', 'contract-signature', 'invoice', + ])); + }); + + it('surfaces a missing file in the response payload', async () => { + await db('contracts').insert({ + customer_account_id: customerId, + contract_number: 'C-B7-MISSING', + status: 'sent', + issue_date: '2026-01-01', + signed_pdf_path: 'business-docs/contract/2026/C-B7-MISSING.pdf', + created_at: new Date(), + }); + + const res = await request(app).get('/api/admin/system-health/backup-integrity'); + expect(res.status).toBe(200); + expect(res.body.report.summary.missingFiles).toBe(1); + expect(res.body.report.missing[0]).toMatchObject({ + table: 'contracts', + column: 'signed_pdf_path', + expectedPath: 'business-docs/contract/2026/C-B7-MISSING.pdf', + }); + }); + + it('honours the ?scope=invoice filter', async () => { + // Seed both an invoice and a contract with missing files. With + // scope=invoice the contract row must not appear. + await db('invoices').insert({ + customer_account_id: customerId, + invoice_number: 'INV-B7-SCOPE', + status: 'sent', + issue_date: '2026-01-01', + due_date: '2026-01-31', + pdf_path: 'business-docs/invoice/2026/INV-B7-SCOPE.pdf', + created_at: new Date(), + }); + await db('contracts').insert({ + customer_account_id: customerId, + contract_number: 'C-B7-SCOPE', + status: 'sent', + issue_date: '2026-01-01', + signed_pdf_path: 'business-docs/contract/2026/C-B7-SCOPE.pdf', + created_at: new Date(), + }); + + const res = await request(app) + .get('/api/admin/system-health/backup-integrity') + .query({ scope: 'invoice' }); + expect(res.status).toBe(200); + expect(res.body.report.scopes).toEqual(['invoice']); + expect(res.body.report.missing.every((m) => m.table === 'invoices')).toBe(true); + }); + + it('rejects an unknown scope with 400 + a code', async () => { + const res = await request(app) + .get('/api/admin/system-health/backup-integrity') + .query({ scope: 'gallery' }); + expect(res.status).toBe(400); + expect(res.body.code).toBe('BACKUP_INTEGRITY_UNKNOWN_SCOPE'); + expect(res.body.validScopes).toEqual(expect.arrayContaining([ + 'quote', 'contract', 'contract-signature', 'invoice', + ])); + }); +}); diff --git a/backend/__tests__/services/backupIntegrityService.test.js b/backend/__tests__/services/backupIntegrityService.test.js new file mode 100644 index 00000000..4ab52b2e --- /dev/null +++ b/backend/__tests__/services/backupIntegrityService.test.js @@ -0,0 +1,216 @@ +/** + * Verifies the backup-integrity check covers every CRM document + * artefact column and correctly buckets each row into: + * - verifiedOk — file exists AND hash matches (when hash is stored) + * - missing — `*_path` set but file is not on disk + * - hashMismatches — file exists but bytes don't hash to `*_sha256` + * - existsButNoHash — file exists, no `*_sha256` column for this row + * + * Uses the CRM integration harness (bootCrmDb) so the schema + + * STORAGE_PATH wiring exactly mirrors production behaviour. + * + * Background: this service is the diagnostic for the + * `storage/business-docs/` gap fixed in the same PR — without it, + * a restored install would have audit-trail columns referencing + * files that no longer exist, but admins would have no way to see + * the breakage until a customer asked for their contract back. + */ + +const fs = require('fs'); +const path = require('path'); +const crypto = require('crypto'); + +const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb'); + +jest.setTimeout(30000); + +describe('backupIntegrityService.verifyDocumentArtefacts', () => { + let db; + let cleanup; + let customerId; + let storagePath; + let backupIntegrityService; + + function seedFile(relPath, content) { + const abs = path.join(storagePath, relPath); + fs.mkdirSync(path.dirname(abs), { recursive: true }); + fs.writeFileSync(abs, content); + return { abs, relPath, sha: sha256(content) }; + } + + function sha256(content) { + return crypto.createHash('sha256').update(content).digest('hex'); + } + + beforeAll(async () => { + ({ db, cleanup } = await bootCrmDb()); + ({ customerId } = await seedMinimal(db)); + storagePath = process.env.STORAGE_PATH; + backupIntegrityService = require('../../src/services/backupIntegrityService'); + }, 120000); + + afterAll(async () => { + if (cleanup) await cleanup(); + }); + + beforeEach(async () => { + // Wipe CRM rows between tests so each scenario sees a clean slate. + // Order matters: child tables before parents. + await db('invoice_line_items').del().catch(() => {}); + await db('invoice_payment_log').del().catch(() => {}); + await db('invoices').del().catch(() => {}); + await db('quote_line_items').del().catch(() => {}); + await db('quotes').del().catch(() => {}); + await db('contracts').del().catch(() => {}); + }); + + it('returns an empty report when no documents reference any path', async () => { + const report = await backupIntegrityService.verifyDocumentArtefacts(); + expect(report.summary.totalRows).toBe(0); + expect(report.summary.verifiedOk).toBe(0); + expect(report.missing).toEqual([]); + expect(report.hashMismatches).toEqual([]); + expect(report.existsButNoHash).toEqual([]); + expect(report.scannedAt).toMatch(/^\d{4}-\d{2}-\d{2}T/); + expect(report.scopes).toEqual(expect.arrayContaining(['quote', 'contract', 'contract-signature', 'invoice'])); + }); + + it('flags a contract whose signed_pdf_path file is missing', async () => { + // Reference a file that we deliberately never create on disk. + const [{ id }] = await db('contracts').insert({ + customer_account_id: customerId, + contract_number: 'C-2026-MISSING', + status: 'sent', + issue_date: '2026-01-01', + signed_pdf_path: 'business-docs/contract/2026/C-2026-MISSING.pdf', + created_at: new Date(), + }).returning('id'); + const contractId = typeof id === 'object' ? id.id : id; + + const report = await backupIntegrityService.verifyDocumentArtefacts({ scope: ['contract'] }); + const hit = report.missing.find((m) => m.rowId === contractId); + expect(hit).toMatchObject({ + table: 'contracts', + column: 'signed_pdf_path', + expectedPath: 'business-docs/contract/2026/C-2026-MISSING.pdf', + }); + expect(report.summary.missingFiles).toBe(1); + }); + + it('verifies a contract whose file exists AND hash matches', async () => { + const { relPath, sha } = seedFile( + 'business-docs/contract/2026/C-2026-OK.pdf', + 'this is the signed contract content', + ); + await db('contracts').insert({ + customer_account_id: customerId, + contract_number: 'C-2026-OK', + status: 'fully_signed', + issue_date: '2026-01-01', + signed_pdf_path: relPath, + signed_pdf_sha256: sha, + created_at: new Date(), + }); + + const report = await backupIntegrityService.verifyDocumentArtefacts({ scope: ['contract'] }); + expect(report.summary.verifiedOk).toBeGreaterThanOrEqual(1); + expect(report.summary.missingFiles).toBe(0); + expect(report.summary.hashMismatches).toBe(0); + }); + + it('flags a hash mismatch when the file exists but bytes differ from signed_pdf_sha256', async () => { + const { relPath } = seedFile( + 'business-docs/contract/2026/C-2026-TAMPER.pdf', + 'tampered bytes on disk', + ); + const [{ id }] = await db('contracts').insert({ + customer_account_id: customerId, + contract_number: 'C-2026-TAMPER', + status: 'fully_signed', + issue_date: '2026-01-01', + signed_pdf_path: relPath, + // Hash for completely different content — simulates tampering or + // bit-rot between sign-time and now. + signed_pdf_sha256: sha256('the ORIGINAL bytes the customer signed'), + created_at: new Date(), + }).returning('id'); + const contractId = typeof id === 'object' ? id.id : id; + + const report = await backupIntegrityService.verifyDocumentArtefacts({ scope: ['contract'] }); + const hit = report.hashMismatches.find((m) => m.rowId === contractId); + expect(hit).toBeDefined(); + expect(hit.expectedSha).not.toBe(hit.actualSha); + expect(hit.column).toBe('signed_pdf_path'); + }); + + it('buckets signature PNGs into existsButNoHash (no hash column)', async () => { + const { relPath } = seedFile( + 'business-docs/contract/signatures/99/customer-1700000000000.png', + '\x89PNG\r\n\x1a\n', // doesn't have to be a real PNG, just bytes + ); + await db('contracts').insert({ + customer_account_id: customerId, + contract_number: 'C-2026-SIG', + status: 'fully_signed', + issue_date: '2026-01-01', + signed_customer_signature_path: relPath, + created_at: new Date(), + }); + + const report = await backupIntegrityService.verifyDocumentArtefacts({ + scope: ['contract-signature'], + }); + expect(report.summary.existsButNoHash).toBeGreaterThanOrEqual(1); + expect(report.summary.verifiedOk).toBe(0); // no hash → not "verified ok" + expect(report.summary.missingFiles).toBe(0); + const hit = report.existsButNoHash.find((r) => r.column === 'signed_customer_signature_path'); + expect(hit).toBeDefined(); + }); + + it('respects the scope filter — contract scope skips quote/invoice tables', async () => { + // Seed an invoice with a missing pdf_path AND a contract with a + // missing signed_pdf_path. Scoping to contract should only flag + // the contract. + await db('invoices').insert({ + customer_account_id: customerId, + invoice_number: 'INV-2026-SCOPE', + status: 'sent', + pdf_path: 'business-docs/invoice/2026/INV-2026-SCOPE.pdf', + issue_date: '2026-01-01', + due_date: '2026-01-31', + created_at: new Date(), + }); + await db('contracts').insert({ + customer_account_id: customerId, + contract_number: 'C-2026-SCOPE', + status: 'sent', + issue_date: '2026-01-01', + signed_pdf_path: 'business-docs/contract/2026/C-2026-SCOPE.pdf', + created_at: new Date(), + }); + + const report = await backupIntegrityService.verifyDocumentArtefacts({ scope: ['contract'] }); + expect(report.scopes).toEqual(['contract']); + expect(report.missing.every((m) => m.table === 'contracts')).toBe(true); + expect(report.missing.some((m) => m.table === 'invoices')).toBe(false); + }); + + it('covers invoices.imported_pdf_path (admin-uploaded historical scans)', async () => { + // Imported invoices are the most catastrophic case — there's no + // renderer that can reproduce them. Verifier must check this column + // alongside invoices.pdf_path. + await db('invoices').insert({ + customer_account_id: customerId, + invoice_number: 'IMP-2025-001', + status: 'sent', + imported_pdf_path: 'business-docs/invoice-imports/2025/legacy.pdf', + issue_date: '2025-06-01', + due_date: '2025-07-01', + created_at: new Date(), + }); + + const report = await backupIntegrityService.verifyDocumentArtefacts({ scope: ['invoice'] }); + const hit = report.missing.find((m) => m.column === 'imported_pdf_path'); + expect(hit).toBeDefined(); + }); +}); diff --git a/backend/server.js b/backend/server.js index e7d57663..f7c5d7c0 100644 --- a/backend/server.js +++ b/backend/server.js @@ -660,6 +660,7 @@ app.use('/api/admin/contracts', require('./src/routes/adminContracts')); app.use('/api/admin/calendar', require('./src/routes/adminCalendar')); app.use('/api/admin/deals', require('./src/routes/adminDeals')); app.use('/api/admin/tax-report', require('./src/routes/adminTaxReport')); +app.use('/api/admin/system-health', require('./src/routes/adminSystemHealth')); app.use('/api/admin/dev', require('./src/routes/adminDev')); app.use('/api/public/quotes', require('./src/routes/publicQuotes')); app.use('/api/public/contracts', require('./src/routes/publicContracts')); diff --git a/backend/src/routes/adminSystemHealth.js b/backend/src/routes/adminSystemHealth.js new file mode 100644 index 00000000..e638cad4 --- /dev/null +++ b/backend/src/routes/adminSystemHealth.js @@ -0,0 +1,64 @@ +/** + * Admin → System Health + * + * Endpoint mounted at /api/admin/system-health. The "Backup + * integrity" sub-endpoint is the on-demand verifier for CRM + * document artefacts — confirms every `*_path` column on quotes / + * contracts / invoices points at a file that actually exists on + * disk and (where a `*_sha256` column is set) the file's bytes + * still hash to the expected value. + * + * Per the design decisions locked with the maintainer: + * - On-demand only; no scheduler (D1) + * - Not auto-triggered after restore (D2) + * - Wet-upload contracts are hash-verified same as system-rendered (D3) + * + * Read-only. Returns a JSON report — never mutates DB or fs. + */ + +const express = require('express'); +const { query } = require('express-validator'); +const { adminAuth } = require('../middleware/auth'); +const { requirePermission } = require('../middleware/permissions'); +const { handleAsync, validateRequest, successResponse } = require('../utils/routeHelpers'); +const { verifyDocumentArtefacts } = require('../services/backupIntegrityService'); + +const router = express.Router(); + +router.use(adminAuth); + +const VALID_SCOPES = ['quote', 'contract', 'contract-signature', 'invoice']; + +router.get( + '/backup-integrity', + requirePermission('settings.view'), + [ + // CSV string like `?scope=contract,invoice`. Each member must be + // one of the four known scopes. Empty / omitted means full scan. + query('scope').optional({ values: 'falsy' }).isString().isLength({ max: 128 }), + ], + handleAsync(async (req, res) => { + validateRequest(req); + let scope; + if (req.query.scope) { + scope = String(req.query.scope) + .split(',') + .map((s) => s.trim()) + .filter(Boolean); + // Defense-in-depth: reject unknown scope tokens so a typo doesn't + // silently scan everything when the caller wanted just one slice. + const unknown = scope.filter((s) => !VALID_SCOPES.includes(s)); + if (unknown.length > 0) { + return res.status(400).json({ + error: `Unknown scope(s): ${unknown.join(', ')}`, + code: 'BACKUP_INTEGRITY_UNKNOWN_SCOPE', + validScopes: VALID_SCOPES, + }); + } + } + const report = await verifyDocumentArtefacts({ scope }); + return successResponse(res, { report }); + }), +); + +module.exports = router; diff --git a/backend/src/services/backupIntegrityService.js b/backend/src/services/backupIntegrityService.js new file mode 100644 index 00000000..eb273f73 --- /dev/null +++ b/backend/src/services/backupIntegrityService.js @@ -0,0 +1,228 @@ +/** + * Backup-integrity verifier — walks every CRM document-artefact path + * column and confirms (a) the file exists on disk, (b) when a SHA-256 + * is stored, the file's actual bytes hash to the stored value. + * + * **Why this is a separate service** + * + * The audit trail captured at issue / sign time (signed_customer_ip, + * signed_by_customer_at, signed_pdf_sha256, signed_*_signature_path, + * issue_date, etc.) is worth exactly nothing on its own — what makes + * it legally meaningful is being able to produce the document the + * audit trail refers to. A backup that captures the DB but skips + * `storage/business-docs/` (the bug fixed in this same PR) leaves + * every `*_path` column a broken FK and every `*_sha256` column with + * nothing to verify against. This service is the diagnostic for + * exactly that drift — runs on demand, surfaces missing files + + * hash mismatches without making any changes. + * + * **Verification modes** + * + * - existence — file at `*_path` must exist on disk + * - sha256 — file at `*_path` must exist AND its sha256 must + * equal `*_sha256` column (when that column is set) + * + * Per-table coverage (lines reference migrations/core/107_crm_consolidated.js): + * + * quotes.pdf_path line 844 (existence) + * contracts.pdf_path line 1245 (existence + sha256 via contracts.pdf_sha256) + * contracts.signed_pdf_path line 1246 (existence + sha256 via contracts.signed_pdf_sha256) + * contracts.signed_customer_signature_path line 1269 (existence — drawn signatures, no hash column) + * contracts.signed_admin_signature_path line 1273 (existence — admin counter-signature drawing) + * invoices.pdf_path line 1026 (existence) + * invoices.imported_pdf_path line 1020 (existence — admin-uploaded scans) + * + * Wet uploads (`contracts.signed_pdf_is_wet_upload = true`) DO have a + * `signed_pdf_sha256` computed at upload time (contractService.js + * upload route), so they're hash-verified the same as system-rendered + * contracts — no special case here. + * + * **What this service does NOT do** + * + * - Does not write anything (no DB mutations, no fs touches) + * - Does not fail the request when a mismatch is found — the + * report shape carries the data, the caller decides what to do + * - Does not auto-trigger after restore (D2 decision: surface a + * CTA on the restore-completed screen instead) + * - Does not run on a schedule (D1 decision: on-demand v1; revisit + * once we have runtime data on large installs) + */ + +const fs = require('fs'); +const crypto = require('crypto'); +const path = require('path'); +const { db } = require('../database/db'); +const logger = require('../utils/logger'); + +const STORAGE_ROOT = () => process.env.STORAGE_PATH || path.join(process.cwd(), 'storage'); + +/** + * Every column the verifier walks, declared once so the test suite + * and the service share a single source of truth. Order is the order + * the report lists rows in — table-major, then column-by-column. + */ +const CHECKS = [ + { table: 'quotes', pathColumn: 'pdf_path', shaColumn: null, scope: 'quote' }, + { table: 'contracts', pathColumn: 'pdf_path', shaColumn: 'pdf_sha256', scope: 'contract' }, + { table: 'contracts', pathColumn: 'signed_pdf_path', shaColumn: 'signed_pdf_sha256', scope: 'contract' }, + { table: 'contracts', pathColumn: 'signed_customer_signature_path', shaColumn: null, scope: 'contract-signature' }, + { table: 'contracts', pathColumn: 'signed_admin_signature_path', shaColumn: null, scope: 'contract-signature' }, + { table: 'invoices', pathColumn: 'pdf_path', shaColumn: null, scope: 'invoice' }, + { table: 'invoices', pathColumn: 'imported_pdf_path', shaColumn: null, scope: 'invoice' }, +]; + +/** Stream-hash a file to sha256 hex without buffering the whole thing. */ +function hashFile(absPath) { + return new Promise((resolve, reject) => { + const hash = crypto.createHash('sha256'); + const stream = fs.createReadStream(absPath); + stream.on('error', reject); + stream.on('data', (chunk) => hash.update(chunk)); + stream.on('end', () => resolve(hash.digest('hex'))); + }); +} + +/** + * @param {object} [options] + * @param {string[]} [options.scope] Filter checks by scope tag: + * 'quote' | 'contract' | 'contract-signature' | 'invoice'. + * Defaults to all four (full scan). + * @returns {Promise<{ + * scannedAt: string, + * scopes: string[], + * summary: { + * totalRows: number, + * verifiedOk: number, + * missingFiles: number, + * hashMismatches: number, + * existsButNoHash: number, + * }, + * missing: Array<{ table, rowId, column, expectedPath }>, + * hashMismatches: Array<{ table, rowId, column, expectedPath, expectedSha, actualSha }>, + * existsButNoHash: Array<{ table, rowId, column, path }>, + * }>} + * + * `existsButNoHash` is the existence-only-verified bucket — the file + * was found but no `*_sha256` column exists for it (quote/invoice PDFs, + * signature PNGs). Surfaced separately so admins can distinguish + * "verified by hash" from "verified by existence only" — the latter + * is weaker evidence in a legal dispute. + */ +async function verifyDocumentArtefacts(options = {}) { + const scopes = Array.isArray(options.scope) && options.scope.length > 0 + ? options.scope.slice() + : Array.from(new Set(CHECKS.map((c) => c.scope))); + + const checksToRun = CHECKS.filter((c) => scopes.includes(c.scope)); + const storageRoot = STORAGE_ROOT(); + + const missing = []; + const hashMismatches = []; + const existsButNoHash = []; + let totalRows = 0; + let verifiedOk = 0; + + for (const check of checksToRun) { + // Skip the check cleanly when the column or table doesn't exist + // on this install — keeps the verifier safe to run on partial + // migrations or installs that have features disabled. + if (!(await db.schema.hasTable(check.table))) continue; + if (!(await db.schema.hasColumn(check.table, check.pathColumn))) continue; + + const select = ['id', check.pathColumn]; + const hasHashColumn = check.shaColumn + && (await db.schema.hasColumn(check.table, check.shaColumn)); + if (hasHashColumn) select.push(check.shaColumn); + + const rows = await db(check.table) + .whereNotNull(check.pathColumn) + .select(...select); + + for (const row of rows) { + totalRows += 1; + const storedPath = row[check.pathColumn]; + // Stored paths can be absolute (older rows) or relative-to- + // storage (newer rows). Normalize: resolve relative paths + // against STORAGE_PATH; absolute paths are used verbatim. + const absPath = path.isAbsolute(storedPath) + ? storedPath + : path.join(storageRoot, storedPath); + + let exists = false; + try { + exists = fs.existsSync(absPath); + } catch (_) { exists = false; } + + if (!exists) { + missing.push({ + table: check.table, + rowId: row.id, + column: check.pathColumn, + expectedPath: storedPath, + }); + continue; + } + + const expectedSha = hasHashColumn ? row[check.shaColumn] : null; + if (!expectedSha) { + // File exists but we have no hash to verify it against. + existsButNoHash.push({ + table: check.table, + rowId: row.id, + column: check.pathColumn, + path: storedPath, + }); + continue; + } + + let actualSha; + try { + actualSha = await hashFile(absPath); + } catch (err) { + logger.warn(`backupIntegrity: failed to hash ${absPath}: ${err.message}`); + missing.push({ + table: check.table, + rowId: row.id, + column: check.pathColumn, + expectedPath: storedPath, + }); + continue; + } + + if (actualSha !== expectedSha) { + hashMismatches.push({ + table: check.table, + rowId: row.id, + column: check.pathColumn, + expectedPath: storedPath, + expectedSha, + actualSha, + }); + continue; + } + + verifiedOk += 1; + } + } + + return { + scannedAt: new Date().toISOString(), + scopes, + summary: { + totalRows, + verifiedOk, + missingFiles: missing.length, + hashMismatches: hashMismatches.length, + existsButNoHash: existsButNoHash.length, + }, + missing, + hashMismatches, + existsButNoHash, + }; +} + +module.exports = { + verifyDocumentArtefacts, + // Exported for tests; not part of the route API. + _internal: { CHECKS, hashFile }, +}; From 7e2feca12f894a9d02193d85634e69d76710fc14 Mon Sep 17 00:00:00 2001 From: Luca <102960244+Luca-Timo@users.noreply.github.com> Date: Fri, 29 May 2026 13:11:43 +0200 Subject: [PATCH 03/42] =?UTF-8?q?feat(backup):=20UI=20for=20backup-integri?= =?UTF-8?q?ty=20verifier=20=E2=80=94=20tab=20+=20post-restore=20CTA?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Frontend half of the diagnostic shipped in 4812fcd. Adds: - BackupIntegrityCard component — runs the check on demand, surfaces the five summary counters (total / verifiedOk / existsButNoHash / missing / hashMismatches), and expands collapsible result tables for missing files + hash mismatches. existsButNoHash is exposed as a separate amber-toned bucket so admins can distinguish hash- verified evidence from existence-only at a glance — the latter is explicitly weaker in a legal dispute and the UI says so. - "Integrity" tab on BackupManagement, alongside the existing Dashboard / Configuration / History / Restore tabs. Card is portable — when the System Health page (backlog item) lands it can lift the component without changes. - Post-restore CTA on the RestoreWizard success card (D2 follow- through): "Verify document integrity now" button that switches the parent tab to Integrity. The audit trail captured at sign / issue time is worth nothing if the documents it refers to are missing from the restored copy — verifier surfaces that drift in one click before the admin trusts the restored state. i18n strings added in EN + DE (per user_languages — only those two are native; other locales fall back to the English defaults and should be flagged for native-speaker review per feedback_translation_flagging if anyone picks them up). --- .../components/admin/BackupIntegrityCard.tsx | 285 ++++++++++++++++++ .../src/components/admin/RestoreWizard.jsx | 27 +- frontend/src/i18n/locales/de.json | 37 ++- frontend/src/i18n/locales/en.json | 37 ++- frontend/src/pages/admin/BackupManagement.tsx | 11 +- frontend/src/services/admin.service.ts | 57 ++++ 6 files changed, 445 insertions(+), 9 deletions(-) create mode 100644 frontend/src/components/admin/BackupIntegrityCard.tsx diff --git a/frontend/src/components/admin/BackupIntegrityCard.tsx b/frontend/src/components/admin/BackupIntegrityCard.tsx new file mode 100644 index 00000000..cf8b7438 --- /dev/null +++ b/frontend/src/components/admin/BackupIntegrityCard.tsx @@ -0,0 +1,285 @@ +import React, { useState } from 'react'; +import { useTranslation } from 'react-i18next'; +import { + ShieldCheck, + ShieldAlert, + FileX, + Hash, + HelpCircle, + Play, + Loader2, +} from 'lucide-react'; +import { useMutation } from '@tanstack/react-query'; +import { format } from 'date-fns'; + +import { Card, Button } from '../common'; +import { adminService, BackupIntegrityReport } from '../../services/admin.service'; + +/** + * BackupIntegrityCard — on-demand verifier for CRM document artefacts. + * + * Walks every `*_path` column on quotes / contracts / invoices and + * confirms (a) the referenced file exists on disk, (b) where a SHA-256 + * is stored, the file's bytes hash to the expected value. Surfaces + * three failure buckets: + * + * - missing — `*_path` set, file not on disk (broken FK) + * - hashMismatches — file exists but bytes don't match the stored hash + * - existsButNoHash — verified by existence only; weaker evidence + * + * Designed to be portable. Currently embedded as a tab on + * `BackupManagement.tsx`; when the System Health page (backlog item) + * lands, this same component can be lifted there without changes. + */ +export const BackupIntegrityCard: React.FC = () => { + const { t } = useTranslation(); + const [report, setReport] = useState(null); + const [expanded, setExpanded] = useState<'missing' | 'hashMismatches' | null>(null); + + const runCheck = useMutation({ + mutationFn: () => adminService.getBackupIntegrity(), + onSuccess: (data) => { + setReport(data); + // Auto-expand whichever failure bucket has entries, prioritising + // the more severe one (missing > hashMismatches). + if (data.summary.missingFiles > 0) setExpanded('missing'); + else if (data.summary.hashMismatches > 0) setExpanded('hashMismatches'); + else setExpanded(null); + }, + }); + + const summary = report?.summary; + const isHealthy = report + && summary + && summary.missingFiles === 0 + && summary.hashMismatches === 0; + + return ( + +
+
+
+ {isHealthy ? ( + + ) : report ? ( + + ) : ( + + )} +

+ {t('backup.integrity.title', 'Document integrity')} +

+
+

+ {t( + 'backup.integrity.description', + 'Verifies every CRM document (quote / contract / invoice / signature) referenced from the database actually exists on disk and — where a hash is stored — its bytes still match. Read-only, on-demand.', + )} +

+
+ +
+ + {runCheck.isError && ( +
+ {t('backup.integrity.error', 'Check failed: {{message}}', { + message: (runCheck.error as Error)?.message ?? 'unknown error', + })} +
+ )} + + {report && summary && ( + <> +
+ + } + /> + } + tooltip={t( + 'backup.integrity.summary.existsButNoHashHint', + 'File found, but no SHA-256 is stored for it (quote/invoice PDFs, signature drawings). Existence-only is weaker evidence in a dispute.', + )} + /> + 0 ? 'red' : 'neutral'} + icon={} + onClick={summary.missingFiles > 0 + ? () => setExpanded(expanded === 'missing' ? null : 'missing') + : undefined} + /> + 0 ? 'red' : 'neutral'} + icon={} + onClick={summary.hashMismatches > 0 + ? () => setExpanded(expanded === 'hashMismatches' ? null : 'hashMismatches') + : undefined} + /> +
+ +

+ {t('backup.integrity.scannedAt', 'Last checked: {{when}}', { + when: format(new Date(report.scannedAt), 'yyyy-MM-dd HH:mm:ss'), + })} +

+ + {expanded === 'missing' && summary.missingFiles > 0 && ( + ({ + table: m.table, + rowId: m.rowId, + column: m.column, + detail: m.expectedPath, + }))} + /> + )} + + {expanded === 'hashMismatches' && summary.hashMismatches > 0 && ( + ({ + table: m.table, + rowId: m.rowId, + column: m.column, + detail: `${m.expectedPath} (expected ${m.expectedSha.slice(0, 12)}…, got ${m.actualSha.slice(0, 12)}…)`, + }))} + /> + )} + + )} + + {!report && !runCheck.isPending && ( +

+ {t( + 'backup.integrity.emptyState', + 'No check has been run yet in this session. Click "Run check now" to scan the document estate.', + )} +

+ )} + + ); +}; + +type Tone = 'neutral' | 'green' | 'amber' | 'red'; + +const TONE_CLASSES: Record = { + neutral: 'bg-neutral-100 dark:bg-neutral-800 text-neutral-700 dark:text-neutral-200', + green: 'bg-green-50 dark:bg-green-900/30 text-green-700 dark:text-green-300', + amber: 'bg-amber-50 dark:bg-amber-900/30 text-amber-700 dark:text-amber-300', + red: 'bg-red-50 dark:bg-red-900/30 text-red-700 dark:text-red-300', +}; + +const Counter: React.FC<{ + label: string; + value: number; + tone: Tone; + icon?: React.ReactNode; + tooltip?: string; + onClick?: () => void; +}> = ({ label, value, tone, icon, tooltip, onClick }) => { + const interactive = Boolean(onClick); + const classes = `rounded-lg p-3 ${TONE_CLASSES[tone]} ${ + interactive ? 'cursor-pointer hover:ring-2 hover:ring-offset-1 hover:ring-current/30 transition' : '' + }`; + return ( +
+
+ {icon} + {label} +
+
{value}
+
+ ); +}; + +const ResultTable: React.FC<{ + title: string; + caption: string; + rows: Array<{ table: string; rowId: number; column: string; detail: string }>; +}> = ({ title, caption, rows }) => { + const { t } = useTranslation(); + return ( +
+
+

{title}

+

{caption}

+
+
+ + + + + + + + + + + {rows.map((r, i) => ( + + + + + + + ))} + +
{t('backup.integrity.results.table', 'Table')}{t('backup.integrity.results.rowId', 'Row id')}{t('backup.integrity.results.column', 'Column')}{t('backup.integrity.results.detail', 'Detail')}
+ {r.table} + + {r.rowId} + + {r.column} + + {r.detail} +
+
+
+ ); +}; diff --git a/frontend/src/components/admin/RestoreWizard.jsx b/frontend/src/components/admin/RestoreWizard.jsx index 451ef000..3269e44c 100644 --- a/frontend/src/components/admin/RestoreWizard.jsx +++ b/frontend/src/components/admin/RestoreWizard.jsx @@ -21,7 +21,8 @@ import { Eye, Calendar, Clock, - AlertCircle + AlertCircle, + ShieldCheck } from 'lucide-react'; import { format } from 'date-fns'; import { toast } from 'react-toastify'; @@ -29,7 +30,7 @@ import { useQuery, useMutation } from '@tanstack/react-query'; import { Button, Card, Input, Loading } from '../common'; import { api } from '../../config/api'; -export const RestoreWizard = () => { +export const RestoreWizard = ({ onVerifyIntegrity } = {}) => { const { t } = useTranslation(); const [currentStep, setCurrentStep] = useState(0); @@ -650,13 +651,33 @@ export const RestoreWizard = () => {
-
+

{t('backup.restore.progress.success.title')}

{t('backup.restore.progress.success.message')}

+ {/* Post-restore CTA: jump to the integrity check (D2). The + audit trail captured at sign / issue time is worth + nothing if the documents it refers to are missing + from the restored copy — verifier surfaces that + drift in one click before the admin trusts the + restored state. */} + {onVerifyIntegrity && ( + + )}
diff --git a/frontend/src/i18n/locales/de.json b/frontend/src/i18n/locales/de.json index 11dea9ad..4c6855b1 100644 --- a/frontend/src/i18n/locales/de.json +++ b/frontend/src/i18n/locales/de.json @@ -230,7 +230,39 @@ "dashboard": "Dashboard", "configuration": "Konfiguration", "history": "Backup-Verlauf", - "restore": "Wiederherstellung" + "restore": "Wiederherstellung", + "integrity": "Integrität" + }, + "integrity": { + "title": "Dokumentintegrität", + "description": "Prüft, ob jedes in der Datenbank referenzierte CRM-Dokument (Angebot / Vertrag / Rechnung / Unterschrift) tatsächlich auf der Festplatte existiert und — sofern ein Hash gespeichert ist — die Datei-Bytes weiterhin übereinstimmen. Nur lesend, auf Abruf.", + "runNow": "Prüfung starten", + "running": "Prüfe…", + "error": "Prüfung fehlgeschlagen: {{message}}", + "scannedAt": "Zuletzt geprüft: {{when}}", + "emptyState": "In dieser Sitzung wurde noch keine Prüfung ausgeführt. Klicke auf \"Prüfung starten\", um den Dokumentbestand zu durchsuchen.", + "summary": { + "total": "Gesamt", + "verifiedOk": "Hash-verifiziert", + "existsButNoHash": "Nur Existenz", + "existsButNoHashHint": "Datei gefunden, aber kein SHA-256 gespeichert (Angebots-/Rechnungs-PDFs, Unterschrifts-Zeichnungen). Nur-Existenz ist im Streitfall schwächeres Beweismaterial.", + "missingFiles": "Fehlend", + "hashMismatches": "Hash-Abweichungen" + }, + "missing": { + "heading": "Fehlende Dateien", + "caption": "Diese Zeilen verweisen auf einen Pfad, der nicht auf der Festplatte existiert. Nach einer Wiederherstellung bedeutet das, dass das Artefakt aus der Sicherungskette verloren ging; bei frischen Installationen wurde die Datei meist manuell gelöscht." + }, + "hashMismatches": { + "heading": "Hash-Abweichungen", + "caption": "Die Datei existiert, aber ihre aktuellen Bytes stimmen nicht mit dem zum Ausstellungs-/Signierzeitpunkt erfassten SHA-256 überein. Deutet auf Manipulation, Bit-Rot oder eine Wiederherstellung mit einer abweichenden Kopie hin." + }, + "results": { + "table": "Tabelle", + "rowId": "Zeilen-ID", + "column": "Spalte", + "detail": "Detail" + } }, "status": { "inProgress": "Backup läuft...", @@ -517,7 +549,8 @@ "restoreLogs": "Wiederherstellungsprotokolle", "success": { "title": "Wiederherstellung erfolgreich abgeschlossen", - "message": "Ihre Daten wurden wiederhergestellt. Bitte überprüfen Sie, ob alles korrekt funktioniert." + "message": "Ihre Daten wurden wiederhergestellt. Bitte überprüfen Sie, ob alles korrekt funktioniert.", + "verifyIntegrity": "Dokumentintegrität jetzt prüfen" } }, "actions": { diff --git a/frontend/src/i18n/locales/en.json b/frontend/src/i18n/locales/en.json index fce76b4b..3e94ebf5 100644 --- a/frontend/src/i18n/locales/en.json +++ b/frontend/src/i18n/locales/en.json @@ -2121,7 +2121,39 @@ "dashboard": "Dashboard", "configuration": "Configuration", "history": "Backup History", - "restore": "Restore" + "restore": "Restore", + "integrity": "Integrity" + }, + "integrity": { + "title": "Document integrity", + "description": "Verifies every CRM document (quote / contract / invoice / signature) referenced from the database actually exists on disk and — where a hash is stored — its bytes still match. Read-only, on-demand.", + "runNow": "Run check now", + "running": "Checking…", + "error": "Check failed: {{message}}", + "scannedAt": "Last checked: {{when}}", + "emptyState": "No check has been run yet in this session. Click \"Run check now\" to scan the document estate.", + "summary": { + "total": "Total", + "verifiedOk": "Hash-verified", + "existsButNoHash": "Exists only", + "existsButNoHashHint": "File found, but no SHA-256 is stored for it (quote/invoice PDFs, signature drawings). Existence-only is weaker evidence in a dispute.", + "missingFiles": "Missing", + "hashMismatches": "Hash mismatches" + }, + "missing": { + "heading": "Missing files", + "caption": "These rows reference a path that does not exist on disk. After a restore, this means the artefact was lost from the backup chain; for fresh installs, it usually means the file was deleted manually." + }, + "hashMismatches": { + "heading": "Hash mismatches", + "caption": "The file exists but its current bytes do not match the SHA-256 captured at issue / sign time. Indicates tampering, bit-rot, or a restore that pulled in a different copy than the original." + }, + "results": { + "table": "Table", + "rowId": "Row id", + "column": "Column", + "detail": "Detail" + } }, "status": { "inProgress": "Backup in progress...", @@ -2408,7 +2440,8 @@ "restoreLogs": "Restore Logs", "success": { "title": "Restore Completed Successfully", - "message": "Your data has been restored. Please verify everything is working correctly." + "message": "Your data has been restored. Please verify everything is working correctly.", + "verifyIntegrity": "Verify document integrity now" } }, "actions": { diff --git a/frontend/src/pages/admin/BackupManagement.tsx b/frontend/src/pages/admin/BackupManagement.tsx index c5691c4c..f127149a 100644 --- a/frontend/src/pages/admin/BackupManagement.tsx +++ b/frontend/src/pages/admin/BackupManagement.tsx @@ -10,6 +10,7 @@ import { Clock, Loader2, Shield, + ShieldCheck, } from 'lucide-react'; import { toast } from 'react-toastify'; import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'; @@ -21,9 +22,10 @@ import { BackupDashboard } from '../../components/admin/BackupDashboard'; import { BackupConfiguration } from '../../components/admin/BackupConfiguration'; import { BackupHistory } from '../../components/admin/BackupHistory'; import { RestoreWizard } from '../../components/admin/RestoreWizard'; +import { BackupIntegrityCard } from '../../components/admin/BackupIntegrityCard'; import { api } from '../../config/api'; -type TabId = 'dashboard' | 'configuration' | 'history' | 'restore'; +type TabId = 'dashboard' | 'configuration' | 'history' | 'restore' | 'integrity'; export const BackupManagement: React.FC = () => { const [activeTab, setActiveTab] = useState('dashboard'); @@ -35,6 +37,7 @@ export const BackupManagement: React.FC = () => { { id: 'configuration' as const, label: t('backup.tabs.configuration'), icon: Settings }, { id: 'history' as const, label: t('backup.tabs.history'), icon: History }, { id: 'restore' as const, label: t('backup.tabs.restore'), icon: RefreshCw }, + { id: 'integrity' as const, label: t('backup.tabs.integrity', 'Integrity'), icon: ShieldCheck }, ]; const { data: backupStatus, isLoading: statusLoading } = useQuery({ @@ -218,7 +221,11 @@ export const BackupManagement: React.FC = () => { )} {activeTab === 'restore' && ( - + setActiveTab('integrity')} /> + )} + + {activeTab === 'integrity' && ( + )}
diff --git a/frontend/src/services/admin.service.ts b/frontend/src/services/admin.service.ts index 1d98b1cc..c622688f 100644 --- a/frontend/src/services/admin.service.ts +++ b/frontend/src/services/admin.service.ts @@ -201,6 +201,50 @@ export interface Activity { createdAt: string; } +// Backup-integrity verifier — diagnostic endpoint that walks every +// CRM document-artefact path column and confirms files exist on disk +// (plus SHA-256 match where the schema stores one). Mirrors the +// shape returned by backupIntegrityService.verifyDocumentArtefacts. +export type BackupIntegrityScope = + | 'quote' + | 'contract' + | 'contract-signature' + | 'invoice'; + +export interface BackupIntegrityMissingRow { + table: string; + rowId: number; + column: string; + expectedPath: string; +} + +export interface BackupIntegrityHashMismatchRow extends BackupIntegrityMissingRow { + expectedSha: string; + actualSha: string; +} + +export interface BackupIntegrityExistsButNoHashRow { + table: string; + rowId: number; + column: string; + path: string; +} + +export interface BackupIntegrityReport { + scannedAt: string; + scopes: BackupIntegrityScope[]; + summary: { + totalRows: number; + verifiedOk: number; + missingFiles: number; + hashMismatches: number; + existsButNoHash: number; + }; + missing: BackupIntegrityMissingRow[]; + hashMismatches: BackupIntegrityHashMismatchRow[]; + existsButNoHash: BackupIntegrityExistsButNoHashRow[]; +} + export interface AdminProfile { id: number; username: string; @@ -262,6 +306,19 @@ export const adminService = { return response.data; }, + // Backup-integrity verifier (read-only diagnostic). `scope` filters + // which document classes to walk; omit for a full scan. + async getBackupIntegrity( + scope?: BackupIntegrityScope[], + ): Promise { + const params = scope && scope.length > 0 ? { scope: scope.join(',') } : undefined; + const response = await api.get<{ report: BackupIntegrityReport }>( + '/admin/system-health/backup-integrity', + { params }, + ); + return response.data.report; + }, + // Format activity message formatActivityMessage(activity: Activity): string { // Feature-flag toggles carry a `changed` diff in metadata. Render From 614c8b9b8f298488fe875276e4be39b19f23af42 Mon Sep 17 00:00:00 2001 From: Luca <102960244+Luca-Timo@users.noreply.github.com> Date: Fri, 29 May 2026 13:25:36 +0200 Subject: [PATCH 04/42] test(backup-integrity): tolerate both knex .returning('id') return shapes CI's SQLite returned `[N]` (plain int) from `.insert().returning('id')` while local SQLite returned `[{ id: N }]` (object form). The brittle `const [{ id }] = ...` destructure crashed on the int shape. Switched to the unwrap pattern used by the existing crmDb test harness so the suite runs on both PG and every SQLite/knex combo the project supports. --- .../__tests__/services/backupIntegrityService.test.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/backend/__tests__/services/backupIntegrityService.test.js b/backend/__tests__/services/backupIntegrityService.test.js index 4ab52b2e..52e2c6bf 100644 --- a/backend/__tests__/services/backupIntegrityService.test.js +++ b/backend/__tests__/services/backupIntegrityService.test.js @@ -77,7 +77,10 @@ describe('backupIntegrityService.verifyDocumentArtefacts', () => { it('flags a contract whose signed_pdf_path file is missing', async () => { // Reference a file that we deliberately never create on disk. - const [{ id }] = await db('contracts').insert({ + // knex's `.returning('id')` returns `[{ id: N }]` on Postgres and + // newer SQLite, but `[N]` (plain int) on some SQLite versions — + // unwrap both shapes the same way the crmDb test harness does. + const inserted = await db('contracts').insert({ customer_account_id: customerId, contract_number: 'C-2026-MISSING', status: 'sent', @@ -85,7 +88,7 @@ describe('backupIntegrityService.verifyDocumentArtefacts', () => { signed_pdf_path: 'business-docs/contract/2026/C-2026-MISSING.pdf', created_at: new Date(), }).returning('id'); - const contractId = typeof id === 'object' ? id.id : id; + const contractId = typeof inserted[0] === 'object' ? inserted[0].id : inserted[0]; const report = await backupIntegrityService.verifyDocumentArtefacts({ scope: ['contract'] }); const hit = report.missing.find((m) => m.rowId === contractId); @@ -123,7 +126,7 @@ describe('backupIntegrityService.verifyDocumentArtefacts', () => { 'business-docs/contract/2026/C-2026-TAMPER.pdf', 'tampered bytes on disk', ); - const [{ id }] = await db('contracts').insert({ + const inserted = await db('contracts').insert({ customer_account_id: customerId, contract_number: 'C-2026-TAMPER', status: 'fully_signed', @@ -134,7 +137,7 @@ describe('backupIntegrityService.verifyDocumentArtefacts', () => { signed_pdf_sha256: sha256('the ORIGINAL bytes the customer signed'), created_at: new Date(), }).returning('id'); - const contractId = typeof id === 'object' ? id.id : id; + const contractId = typeof inserted[0] === 'object' ? inserted[0].id : inserted[0]; const report = await backupIntegrityService.verifyDocumentArtefacts({ scope: ['contract'] }); const hit = report.hashMismatches.find((m) => m.rowId === contractId); From ecb2aeacf9569bb2ea4d678a97c527234aefd159 Mon Sep 17 00:00:00 2001 From: Luca <102960244+Luca-Timo@users.noreply.github.com> Date: Fri, 29 May 2026 15:41:58 +0200 Subject: [PATCH 05/42] fix(test-infra): unref sessionTimeout cleanup interval so workers exit gracefully MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 5-minute session-sweep interval at sessionTimeout.js:17 fired at module-load time without .unref(), so every jest worker that transitively required this module (server.js → middleware → most of the route layer) kept the event loop alive forever. The worker then got force-killed on shutdown, surfacing as the longstanding "worker failed to exit gracefully" warning at the end of every CI run on upstream/beta. Under enough I/O / memory pressure on a CI runner, the force-kill could land MID-test rather than after the suite finished, taking out whatever else was running on that worker — most visibly integration/storageBackend.test.js on PR #555's runs. .unref() makes the timer not keep the loop alive on its own. Production behaviour is unchanged: the timer still fires every 5 min as long as anything else is holding the loop open (the HTTP server, always). --- backend/src/middleware/sessionTimeout.js | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/backend/src/middleware/sessionTimeout.js b/backend/src/middleware/sessionTimeout.js index 462cd490..3c7968bb 100644 --- a/backend/src/middleware/sessionTimeout.js +++ b/backend/src/middleware/sessionTimeout.js @@ -13,7 +13,18 @@ let cachedTimeout = null; let cacheExpiry = 0; const CACHE_DURATION = 30 * 60 * 1000; // 30 minutes - reduced DB queries -// Clean up expired sessions every 5 minutes +// Clean up expired sessions every 5 minutes. +// +// `.unref()` so this timer doesn't keep the event loop alive on its +// own — without it, every jest worker that requires this module +// (directly or transitively via server.js / a middleware-importing +// route file) gets stuck and either prints the "worker failed to +// exit gracefully" warning or, under high CI load, force-kills mid- +// test and takes an unrelated suite down with it (we hit this with +// integration/storageBackend.test.js on PR #555). Production +// behaviour is unchanged: the timer fires every 5 min as long as +// the server has anything else keeping the loop alive (HTTP server, +// other intervals), which is always. setInterval(() => { const now = Date.now(); for (const [token, lastActivity] of sessions.entries()) { @@ -21,7 +32,7 @@ setInterval(() => { sessions.delete(token); } } -}, 5 * 60 * 1000); +}, 5 * 60 * 1000).unref(); async function getSessionTimeout() { const now = Date.now(); From 3f5d006625fdbd8f34ad57d901dd55f543878e05 Mon Sep 17 00:00:00 2001 From: Luca <102960244+Luca-Timo@users.noreply.github.com> Date: Fri, 29 May 2026 16:21:15 +0200 Subject: [PATCH 06/42] fix(test-infra): scope databaseBackup fs.unlink stub so it doesn't leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Line 205 of databaseBackup.test.js reassigned `fs.unlink` directly (`fs.unlink = jest.fn(...)`), which permanently mutated the global fs.promises module. Every test running after this in the same jest worker process inherited the no-op stub, including integration/storageBackend.test.js — whose LocalFsStorage.delete() silently became a no-op, making the subsequent exists() assertion flip from false to true. Confirmed by adding a diagnostic patch to LocalFsStorage.delete: post-await fsp.unlink, fs.existsSync(abs) returned true. unlink had resolved without throwing but the file was still there → the unlink was a mock. Fix: jest.spyOn(fs, 'unlink').mockResolvedValue(undefined) + a matching mockRestore() at the end of the test. Behaviour is identical inside this test; the original fs.unlink is restored when the test finishes, so subsequent tests get real fs.unlink again. Pre-existing issue — has been latent on upstream/beta forever. Only surfaces consistently when CI load shifts jest's worker allocation such that databaseBackup and storageBackend land in the same worker process. This PR's extra integration test files made that allocation deterministic locally and frequent enough on CI to fail reliably. --- .../services/__tests__/databaseBackup.test.js | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/backend/src/services/__tests__/databaseBackup.test.js b/backend/src/services/__tests__/databaseBackup.test.js index be91ab24..12e73972 100644 --- a/backend/src/services/__tests__/databaseBackup.test.js +++ b/backend/src/services/__tests__/databaseBackup.test.js @@ -201,14 +201,22 @@ describe('DatabaseBackupService', () => { delete: jest.fn().mockResolvedValue(1) }); - // Mock fs.unlink - fs.unlink = jest.fn().mockResolvedValue(undefined); - + // Stub fs.promises.unlink via jest.spyOn so the original is + // restored when the test finishes. The previous form + // (`fs.unlink = jest.fn()`) leaked into every test that ran + // after this one in the same jest worker — most visibly + // integration/storageBackend.test.js, whose LocalFsStorage + // delete() became a silent no-op and the subsequent + // exists() assertion flipped from false to true. spyOn + + // mockRestore in afterEach keeps the stub scoped to this test. + const unlinkSpy = jest.spyOn(fs, 'unlink').mockResolvedValue(undefined); + await service.cleanupOldBackups(30); - - expect(fs.unlink).toHaveBeenCalledTimes(2); - expect(fs.unlink).toHaveBeenCalledWith('/backup/old1.sql.gz'); - expect(fs.unlink).toHaveBeenCalledWith('/backup/old2.sql.gz'); + + expect(unlinkSpy).toHaveBeenCalledTimes(2); + expect(unlinkSpy).toHaveBeenCalledWith('/backup/old1.sql.gz'); + expect(unlinkSpy).toHaveBeenCalledWith('/backup/old2.sql.gz'); + unlinkSpy.mockRestore(); }); }); From 3ab3756a56dc8c3bcd517d2d60c7ffd10688a945 Mon Sep 17 00:00:00 2001 From: Luca <102960244+Luca-Timo@users.noreply.github.com> Date: Fri, 29 May 2026 16:42:52 +0200 Subject: [PATCH 07/42] fix(docker): chown /backup mount to nodejs on container startup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The docker-compose `./backup:/backup` mount was the only bind mount not included in wait-for-db.sh's startup chown step. On a fresh install (or any time the mount point is recreated), it stays owned by root, and the nodejs (UID 1001) process running the backup service gets EACCES when trying to mkdir under /backup. Added /backup to both the chown list (root branch) and the writable-check list (compose `user:` override branch), each guarded by `[ -d /backup ]` so installs that don't use the bind mount — native deployments, k8s with a different backup destination, etc. — still boot cleanly. Existing installs hit by this need a one-time host-side sudo chown -R 1001:1001 because the on-disk ownership won't fix itself; the script only chowns at startup, and the directory was already created with the wrong ownership by Docker's mount-point auto-creation. From this commit onward, fresh installs are correct from the first boot. --- backend/wait-for-db.sh | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/backend/wait-for-db.sh b/backend/wait-for-db.sh index 6471326f..3c993855 100755 --- a/backend/wait-for-db.sh +++ b/backend/wait-for-db.sh @@ -11,8 +11,15 @@ set -e # other than root skip this branch — they own permissions themselves and hit # the preflight check below instead. if [ "$(id -u)" = "0" ]; then - if ! chown -R nodejs:nodejs /app/storage /app/data /app/logs 2>/dev/null; then - echo "ERROR: failed to chown /app/storage, /app/data, /app/logs to nodejs (UID 1001)." >&2 + # /backup is the docker-compose `./backup:/backup` mount used by the + # CRM + database backup writers. It's only chowned when it actually + # exists as a bind mount — installs that don't mount it (e.g. native + # / k8s deployments using a different backup destination) skip + # cleanly via the `[ -d /backup ]` guard. + _chown_dirs="/app/storage /app/data /app/logs" + [ -d /backup ] && _chown_dirs="$_chown_dirs /backup" + if ! chown -R nodejs:nodejs $_chown_dirs 2>/dev/null; then + echo "ERROR: failed to chown $_chown_dirs to nodejs (UID 1001)." >&2 echo " This usually means the host filesystem rejects chown (e.g. NFS without root squash" >&2 echo " disabled, or a SELinux/AppArmor policy blocking the operation)." >&2 echo " Workaround: pre-chown the host directories to 1001:1001 and pin 'user: \"1001:1001\"'" >&2 @@ -29,7 +36,15 @@ fi # followed by a confusing migration error and a restart loop. _uid="$(id -u)" _gid="$(id -g)" -for _dir in /app/storage /app/data /app/logs; do +# /backup is included here for symmetry with the root branch above: +# when the compose `user:` override is set, the host operator is +# expected to have chowned the bind mount themselves. Skip the +# check when the mount isn't present so non-bind-mounted setups +# (e.g. backup destination configured to local storage path) still +# boot cleanly. +_writable_dirs="/app/storage /app/data /app/logs" +[ -d /backup ] && _writable_dirs="$_writable_dirs /backup" +for _dir in $_writable_dirs; do if [ ! -w "$_dir" ]; then echo "ERROR: $_dir is not writable by UID $_uid." >&2 echo " Either drop the 'user:' override from your compose file so the container starts as" >&2 From 5b3bfed144e10a74c222b693095b55d77ee07816 Mon Sep 17 00:00:00 2001 From: Luca <102960244+Luca-Timo@users.noreply.github.com> Date: Fri, 29 May 2026 18:05:40 +0200 Subject: [PATCH 08/42] revert(docker): drop /backup chown from wait-for-db.sh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fix shipped in 3ab3756 added /backup to the boot-time chown list. That broke installs that don't bind-mount ./backup:/backup — the single greedy `chown -R /a /b /c /backup` returned non-zero on any individual failure, exiting the script and putting the backend into a restart loop. Reverting to the upstream-stable version. The original EACCES at backup time is better fixed by admins pointing the backup destination at a writable path via the admin UI (e.g. /app/storage/backups, which the script already chowns) rather than baking a /backup assumption into every install's boot path. --- backend/wait-for-db.sh | 21 +++------------------ 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/backend/wait-for-db.sh b/backend/wait-for-db.sh index 3c993855..6471326f 100755 --- a/backend/wait-for-db.sh +++ b/backend/wait-for-db.sh @@ -11,15 +11,8 @@ set -e # other than root skip this branch — they own permissions themselves and hit # the preflight check below instead. if [ "$(id -u)" = "0" ]; then - # /backup is the docker-compose `./backup:/backup` mount used by the - # CRM + database backup writers. It's only chowned when it actually - # exists as a bind mount — installs that don't mount it (e.g. native - # / k8s deployments using a different backup destination) skip - # cleanly via the `[ -d /backup ]` guard. - _chown_dirs="/app/storage /app/data /app/logs" - [ -d /backup ] && _chown_dirs="$_chown_dirs /backup" - if ! chown -R nodejs:nodejs $_chown_dirs 2>/dev/null; then - echo "ERROR: failed to chown $_chown_dirs to nodejs (UID 1001)." >&2 + if ! chown -R nodejs:nodejs /app/storage /app/data /app/logs 2>/dev/null; then + echo "ERROR: failed to chown /app/storage, /app/data, /app/logs to nodejs (UID 1001)." >&2 echo " This usually means the host filesystem rejects chown (e.g. NFS without root squash" >&2 echo " disabled, or a SELinux/AppArmor policy blocking the operation)." >&2 echo " Workaround: pre-chown the host directories to 1001:1001 and pin 'user: \"1001:1001\"'" >&2 @@ -36,15 +29,7 @@ fi # followed by a confusing migration error and a restart loop. _uid="$(id -u)" _gid="$(id -g)" -# /backup is included here for symmetry with the root branch above: -# when the compose `user:` override is set, the host operator is -# expected to have chowned the bind mount themselves. Skip the -# check when the mount isn't present so non-bind-mounted setups -# (e.g. backup destination configured to local storage path) still -# boot cleanly. -_writable_dirs="/app/storage /app/data /app/logs" -[ -d /backup ] && _writable_dirs="$_writable_dirs /backup" -for _dir in $_writable_dirs; do +for _dir in /app/storage /app/data /app/logs; do if [ ! -w "$_dir" ]; then echo "ERROR: $_dir is not writable by UID $_uid." >&2 echo " Either drop the 'user:' override from your compose file so the container starts as" >&2 From 7c230bdc242fa99e12e42c5fb6ee9202243a36e7 Mon Sep 17 00:00:00 2001 From: Luca <102960244+Luca-Timo@users.noreply.github.com> Date: Fri, 29 May 2026 21:57:12 +0200 Subject: [PATCH 09/42] fix(backup): inline DB dump + fail-loud guard so "Run Backup Now" can't ship files-only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous file-backup workflow only LOOKED UP an existing database dump via getDatabaseBackupInfo() and silently shipped a files-only manifest when none was found. Admins clicking "Run Backup Now" (or relying on the schedule) got an apparent success that omitted every customer / quote / invoice / contract / payment-log row. The data-loss footgun was discovered 2026-05-29 when an admin who'd been "backing up" for weeks via the UI lost the entire CRM after a routine docker compose down -v — every produced manifest had database: { backup_file: null, size: 0, tables: {} }. Changes to runBackupInternal: 1. Inline pg_dump (or SQLite copy) before the file scan, via databaseBackupService.backup(). Result lands in database_backup_runs and is picked up by the existing getDatabaseBackupInfo lookup that writes the manifest. 2. Fail-loud guard after the dump step: if no usable dump file is reachable (path missing, 0 bytes, or never existed), throw — the existing catch block marks the backup_runs row failed with the error_message and emails the admin if configured. No more silent files-only manifests. 3. Opt-out: `backup_database_inline_dump = false` skips the inline dump for admins who already run their own scheduled `backup_database_schedule`. The fail-loud guard still applies, so an opted-out install with no recent dump still aborts loudly instead of producing a partial backup. Default ON is encoded as "skip only when explicitly false" — undefined (existing installs upgrading) falls through to the safe-default ON path. Test suite covers: default-on happy path, dump-throws-aborts-run, opt-out + recent dump + proceeds, opt-out + no-dump + fail-loud, opt-out + 0-byte dump + fail-loud. Mocks databaseBackupService.backup so the tests don't depend on pg_dump or sqlite3 CLI binaries being installed. Stage A of three-stage backup hardening plan. Stage B (config-driven walker) and Stage C (audit + diagnostic UI) follow in separate commits. --- .claude/drafts/issue-48-reply.md | 1 + .../backupService.inlineDbDump.test.js | 188 ++++++++++++++++++ backend/src/services/backupService.js | 60 ++++++ 3 files changed, 249 insertions(+) create mode 100644 .claude/drafts/issue-48-reply.md create mode 100644 backend/__tests__/integration/backupService.inlineDbDump.test.js diff --git a/.claude/drafts/issue-48-reply.md b/.claude/drafts/issue-48-reply.md new file mode 100644 index 00000000..8b137891 --- /dev/null +++ b/.claude/drafts/issue-48-reply.md @@ -0,0 +1 @@ + diff --git a/backend/__tests__/integration/backupService.inlineDbDump.test.js b/backend/__tests__/integration/backupService.inlineDbDump.test.js new file mode 100644 index 00000000..431d6dc4 --- /dev/null +++ b/backend/__tests__/integration/backupService.inlineDbDump.test.js @@ -0,0 +1,188 @@ +/** + * Pins the inline-DB-dump + fail-loud guard added to `runBackupInternal`. + * + * The previous behaviour was: file-backup looked up an existing dump via + * `getDatabaseBackupInfo()` and silently shipped a files-only manifest + * when none was found. Admins clicking "Run Backup Now" got an apparent + * success that omitted every customer / quote / invoice / contract row — + * the data-loss footgun that this commit closes. + * + * Five scenarios under test: + * 1. Default (inline dump enabled), dump succeeds → backup proceeds + * 2. Default, dump throws → run aborts, backup_runs row marked failed + * 3. Opt-out + recent DB dump available → backup proceeds + * 4. Opt-out + no DB dump available → fail loud + * 5. Opt-out + DB dump file is 0 bytes on disk → fail loud + * + * Mocking strategy: the underlying `databaseBackupService.backup()` and + * the local-destination writer are stubbed so the test exercises just + * the new guard logic without depending on `pg_dump` / `sqlite3` CLI + * binaries being available in the test environment. + */ + +const fs = require('fs'); +const path = require('path'); + +const { bootCrmDb } = require('./helpers/crmDb'); + +// Set up mocks BEFORE bootCrmDb so backupService picks them up at require time. +const mockBackupFn = jest.fn(); +jest.mock('../../src/services/databaseBackup', () => ({ + databaseBackupService: { backup: mockBackupFn }, + startScheduledBackups: jest.fn(), + stopScheduledBackups: jest.fn(), + DatabaseBackupService: class {}, +})); + +jest.setTimeout(30000); + +describe('backupService — inline DB dump + fail-loud guard', () => { + let db; + let cleanup; + let storagePath; + let backupService; + let dumpFileAbs; + + beforeAll(async () => { + ({ db, cleanup } = await bootCrmDb()); + storagePath = process.env.STORAGE_PATH; + backupService = require('../../src/services/backupService'); + + // Seed backup destination settings so the run can proceed past the + // "destination not configured" guard. + const dest = path.join(storagePath, 'backups'); + fs.mkdirSync(dest, { recursive: true }); + // getBackupConfigInternal filters by setting_type='backup', so the + // tests have to seed with that type or the resolver returns + // `{ ... }` with the keys missing — runBackup then sees + // `backup_destination_type === undefined` and bails before our + // new guard runs. + await db('app_settings').insert([ + { setting_key: 'backup_destination_type', setting_value: JSON.stringify('local'), setting_type: 'backup' }, + { setting_key: 'backup_destination_path', setting_value: JSON.stringify(dest), setting_type: 'backup' }, + { setting_key: 'backup_enabled', setting_value: JSON.stringify(true), setting_type: 'backup' }, + { setting_key: 'backup_email_on_failure', setting_value: JSON.stringify(false), setting_type: 'backup' }, + ]).onConflict('setting_key').merge(); + + // Pre-create a dump file that getDatabaseBackupInfo can resolve to. + // Reused/mutated per-test via the database_backup_runs seed below. + dumpFileAbs = path.join(storagePath, 'backups', 'fake-dump.sql.gz'); + fs.writeFileSync(dumpFileAbs, 'pretend this is a pg_dump'.repeat(100)); + + // Neutralise the file-scan step: we don't care which files would + // be backed up, just whether the run reaches that stage at all. + backupService.getFilesToBackup = jest.fn(async () => []); + }, 120000); + + afterAll(async () => { + if (cleanup) await cleanup(); + }); + + beforeEach(async () => { + mockBackupFn.mockReset(); + // Default to "dump produced this file with this size" — the per-test + // setup overrides as needed. + mockBackupFn.mockResolvedValue({ + success: true, + path: dumpFileAbs, + size: fs.statSync(dumpFileAbs).size, + duration: 1, + checksum: 'abc', + }); + + // Re-seed the database_backup_runs row that getDatabaseBackupInfo + // resolves against (its query is `status='completed'` + most recent). + await db('database_backup_runs').del(); + await db('database_backup_runs').insert({ + started_at: new Date(), + completed_at: new Date(), + status: 'completed', + backup_type: 'pg', + file_path: dumpFileAbs, + file_size_bytes: fs.statSync(dumpFileAbs).size, + destination_path: dumpFileAbs, + }); + }); + + it('default behaviour: inline dump runs, then file backup proceeds', async () => { + // Inline-dump setting is unset (undefined) — default is ON. + await db('app_settings').where('setting_key', 'backup_database_inline_dump').del(); + + await backupService.runBackup(true); + + expect(mockBackupFn).toHaveBeenCalledTimes(1); + + const run = await db('backup_runs').orderBy('id', 'desc').first(); + expect(run.status).toBe('completed'); + expect(run.error_message).toBeNull(); + }); + + it('aborts the run when the inline dump throws', async () => { + await db('app_settings').where('setting_key', 'backup_database_inline_dump').del(); + mockBackupFn.mockRejectedValueOnce(new Error('pg_dump segfaulted')); + + await backupService.runBackup(true); + + const run = await db('backup_runs').orderBy('id', 'desc').first(); + expect(run.status).toBe('failed'); + expect(run.error_message).toMatch(/pg_dump segfaulted/); + }); + + it('opt-out: skips inline dump but proceeds when a recent dump exists', async () => { + await db('app_settings').insert({ + setting_key: 'backup_database_inline_dump', + setting_value: JSON.stringify(false), + setting_type: 'backup', + }).onConflict('setting_key').merge(); + + await backupService.runBackup(true); + + expect(mockBackupFn).not.toHaveBeenCalled(); + + const run = await db('backup_runs').orderBy('id', 'desc').first(); + expect(run.status).toBe('completed'); + }); + + it('opt-out + no recent dump: fails loud with a clear error', async () => { + await db('app_settings').insert({ + setting_key: 'backup_database_inline_dump', + setting_value: JSON.stringify(false), + setting_type: 'backup', + }).onConflict('setting_key').merge(); + // Wipe the dump row so getDatabaseBackupInfo returns backupFile=null. + await db('database_backup_runs').del(); + + await backupService.runBackup(true); + + const run = await db('backup_runs').orderBy('id', 'desc').first(); + expect(run.status).toBe('failed'); + expect(run.error_message).toMatch(/No database backup available/); + }); + + it('opt-out + 0-byte dump file: fails loud', async () => { + await db('app_settings').insert({ + setting_key: 'backup_database_inline_dump', + setting_value: JSON.stringify(false), + setting_type: 'backup', + }).onConflict('setting_key').merge(); + + const emptyDump = path.join(storagePath, 'backups', 'empty-dump.sql.gz'); + fs.writeFileSync(emptyDump, ''); + await db('database_backup_runs').del(); + await db('database_backup_runs').insert({ + started_at: new Date(), + completed_at: new Date(), + status: 'completed', + backup_type: 'pg', + file_path: emptyDump, + file_size_bytes: 0, + destination_path: emptyDump, + }); + + await backupService.runBackup(true); + + const run = await db('backup_runs').orderBy('id', 'desc').first(); + expect(run.status).toBe('failed'); + expect(run.error_message).toMatch(/is empty/); + }); +}); diff --git a/backend/src/services/backupService.js b/backend/src/services/backupService.js index 463a9f33..3e2a7080 100644 --- a/backend/src/services/backupService.js +++ b/backend/src/services/backupService.js @@ -814,6 +814,66 @@ async function runBackupInternal(isManual = false) { }).returning('id'); runId = insertResult[0]?.id || insertResult[0]; + // Inline database dump (default ON). Previously, runBackup only LOOKED UP + // an existing database dump via getDatabaseBackupInfo and silently shipped + // a files-only manifest when none was found — admins clicking "Run Backup + // Now" got an apparent success that omitted every customer / quote / + // invoice / contract row. Triggering pg_dump (or the SQLite copy) here + // makes "file backup" always include a fresh database snapshot. Admins + // who run their own scheduled dumps via backup_database_schedule can opt + // out with backup_database_inline_dump = false; the fail-loud guard + // below still catches the case where no recent dump exists. + // + // Default ON is encoded as "skip only when explicitly false". `undefined` + // (setting not yet inserted on existing installs) falls through to the + // ON path, which is the data-loss-safe default. normalizeBoolean(undefined) + // returns false, so checking inequality against false would inadvertently + // disable on unset — guard with `!== undefined` first. + const inlineDumpExplicitlyOff = config.backup_database_inline_dump !== undefined + && config.backup_database_inline_dump !== null + && normalizeBoolean(config.backup_database_inline_dump) === false; + if (!inlineDumpExplicitlyOff) { + logger.info('Running inline database dump before file backup...'); + const { databaseBackupService } = require('./databaseBackup'); + const dumpResult = await databaseBackupService.backup({}); + logger.info(`Inline database dump completed: ${dumpResult.path} ` + + `(${(dumpResult.size / 1024 / 1024).toFixed(2)} MB)`); + } + + // Fail-loud guard: a "file backup" without a DB component is a data-loss + // trap. Whether the dump came from the inline step above or from a + // separately-scheduled database backup, we require a usable dump file + // before proceeding. Throws — the catch block marks the backup_runs row + // failed with this error_message and emails the admin if configured. + const dbInfoCheck = await service.getDatabaseBackupInfo(); + if (!dbInfoCheck.backupFile) { + throw new Error( + 'No database backup available to include in this file backup. ' + + 'Either keep backup_database_inline_dump enabled (default) or configure ' + + 'backup_database_schedule and let it run at least once first.' + ); + } + try { + const dumpStat = await fs.stat(dbInfoCheck.backupFile); + if (!dumpStat.size || dumpStat.size === 0) { + throw new Error( + `Database backup file at ${dbInfoCheck.backupFile} is empty (0 bytes). ` + + 'Refusing to proceed with file backup to avoid shipping a manifest with no DB content.' + ); + } + } catch (statErr) { + // fs.stat throws if file doesn't exist; preserve the more specific + // empty-file error from the inner block. + if (statErr.code === 'ENOENT') { + throw new Error( + `Database backup file at ${dbInfoCheck.backupFile} is missing from disk. ` + + 'Refusing to proceed with file backup; configure backup_database_schedule or ' + + 'keep backup_database_inline_dump enabled.' + ); + } + throw statErr; + } + const files = await service.getFilesToBackup(config.backup_include_archived); logger.info(`Found ${files.length} files to check for backup`); From 7fdf01ad21da70ae534867e15ae2af1afd4899f4 Mon Sep 17 00:00:00 2001 From: Luca <102960244+Luca-Timo@users.noreply.github.com> Date: Fri, 29 May 2026 22:00:15 +0200 Subject: [PATCH 10/42] fix(backup): inline DB dump + fail-loud guard so "Run Backup Now" can't ship files-only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous file-backup workflow only LOOKED UP an existing database dump via getDatabaseBackupInfo() and silently shipped a files-only manifest when none was found. Admins clicking "Run Backup Now" (or relying on the schedule) got an apparent success that omitted every customer / quote / invoice / contract / payment- log row. The data-loss footgun was discovered 2026-05-29 when an admin who'd been "backing up" for weeks via the UI lost the entire CRM after a routine docker compose down -v — every produced manifest had database: { backup_file: null, size: 0, tables: {} }. New helper `ensureDatabaseDumpForBackup(config)` encapsulates: 1. Inline pg_dump (or SQLite copy) before the file scan, via databaseBackupService.backup(). Result lands in database_backup_runs and is picked up by the existing getDatabaseBackupInfo lookup that writes the manifest. 2. Fail-loud guard: if no usable dump file is reachable (path missing, 0 bytes, or never existed), throw — the existing catch in runBackupInternal marks the backup_runs row failed with the error_message and emails the admin if configured. No more silent files-only manifests. 3. Opt-out: `backup_database_inline_dump = false` skips the inline dump for admins who already run their own scheduled `backup_database_schedule`. The fail-loud guard still applies, so an opted-out install with no recent dump still aborts loudly instead of producing a partial backup. Default ON is encoded as "skip only when explicitly false" — undefined (existing installs upgrading) falls through to the safe- default ON branch. The helper returns the verified `databaseInfo` so the manifest-build step at runBackupInternal:917 reuses it instead of calling getDatabaseBackupInfo a second time. S3/future destinations that override `result.databaseInfo` are still respected (the existing `result.databaseInfo ||` fallback shape stays put). Test suite covers: default-on happy path, dump-throws-aborts-run, opt-out + recent dump + proceeds, opt-out + no-dump + fail-loud, opt-out + 0-byte dump + fail-loud. Mocks databaseBackupService.backup so the tests don't depend on pg_dump or sqlite3 CLI binaries being installed. Stage A of three-stage backup hardening plan. Stage B (config- driven walker) and Stage C (audit + diagnostic UI) follow in separate commits. --- .claude/drafts/issue-48-reply.md | 14 +++ backend/src/services/backupService.js | 139 +++++++++++++++----------- 2 files changed, 93 insertions(+), 60 deletions(-) diff --git a/.claude/drafts/issue-48-reply.md b/.claude/drafts/issue-48-reply.md index 8b137891..fc673667 100644 --- a/.claude/drafts/issue-48-reply.md +++ b/.claude/drafts/issue-48-reply.md @@ -1 +1,15 @@ +Hey @gianlieberum-creator — thanks for the detailed write-up. +Quick context on where we are right now: we're building out the CRM side of picpeak on the `feat/crm` branch — admin-side quotes, invoices with a manual cancel-and-reissue flow, a payment-check email workflow (admin gets an email with three buttons after the due date: paid in full / partial / not paid), and a tax / Steuer report for exporting to your accountant. All payment is traditional invoice → bank transfer; there's no payment processor, no automated checkout, no fulfilment integration. + +Your print-on-demand idea is **out of scope for this iteration** — it's a different shape of feature (customer-facing storefront + fulfilment provider integration + variable-quality serving) than what we're shipping now. That said, I'd like to understand what you'd actually want, so when someone (you, me, anyone) picks it up it isn't designed in a vacuum. + +A few specific things that would help: + +1. **Print partner** — do you have a specific service in mind (WHCC for the US, Saal Digital / CEWE / Whitewall / Pictrs for Europe, something else)? Different providers have very different integration shapes: REST API, manual order export, or a white-label iframe storefront. +2. **Workflow trust level** — would a **manual admin workflow** be acceptable for a v1? E.g. the customer places the order in the gallery, the admin gets an email with the order details, the admin manually forwards it to the print service. Or do you specifically need an automated handoff (order pushed to the print provider via API, status syncs back to the gallery)? +3. **Payment** — would payment via **invoice** (admin sends the invoice through the existing CRM flow once the order is placed) work, or do you need in-gallery checkout (cards / PayPal / Twint / SEPA)? +4. **Image-quality tiers** — how would you want the boundary drawn? Resolution-based (1080p preview free, full-res paid), watermarked vs un-watermarked, or per-photo curator-set (admin marks specific photos as premium)? +5. **Customer journey** — could you walk through your ideal end-to-end flow from the customer's point of view? Even a rough numbered list helps a lot. + +No pressure to fully scope it — partial answers move things forward. If you want to sketch the workflow as a markdown doc and PR it into `docs/` that's a great first step too. diff --git a/backend/src/services/backupService.js b/backend/src/services/backupService.js index 3e2a7080..118e636c 100644 --- a/backend/src/services/backupService.js +++ b/backend/src/services/backupService.js @@ -259,6 +259,73 @@ async function hasDatabaseChanged(sinceTime) { } } +/** + * Run an inline database dump (default ON) and then verify a usable dump + * is actually on disk before letting the file-backup proceed. Returns the + * verified `databaseInfo` so the caller can pass it straight into the + * manifest builder without re-querying. + * + * Why this lives here and not inline in `runBackupInternal`: + * - Encapsulates the "Run Backup Now must include DB" guarantee + * introduced when the silent files-only bug was discovered + * (2026-05-29 — admin lost CRM after `docker compose down -v`) + * - Lets the manifest path share the same `databaseInfo` object + * instead of doing a second `getDatabaseBackupInfo()` round-trip + * - Thrown errors bubble up to `runBackupInternal`'s catch, which + * marks the `backup_runs` row failed and queues the admin email + * + * Default-ON semantics: `backup_database_inline_dump` is only treated + * as disabled when explicitly set to false. `undefined` (the case on + * every existing install that predates the setting) falls through to + * the safe-default ON branch. `normalizeBoolean(undefined)` returns + * false, so a naive `!== false` check would silently disable the + * inline dump for every upgrading install. + */ +async function ensureDatabaseDumpForBackup(config) { + const inlineDumpExplicitlyOff = config.backup_database_inline_dump !== undefined + && config.backup_database_inline_dump !== null + && normalizeBoolean(config.backup_database_inline_dump) === false; + + if (!inlineDumpExplicitlyOff) { + logger.info('Running inline database dump before file backup...'); + const { databaseBackupService } = require('./databaseBackup'); + const dumpResult = await databaseBackupService.backup({}); + logger.info(`Inline database dump completed: ${dumpResult.path} ` + + `(${(dumpResult.size / 1024 / 1024).toFixed(2)} MB)`); + } + + const databaseInfo = await service.getDatabaseBackupInfo(); + if (!databaseInfo.backupFile) { + throw new Error( + 'No database backup available to include in this file backup. ' + + 'Either keep backup_database_inline_dump enabled (default) or configure ' + + 'backup_database_schedule and let it run at least once first.' + ); + } + + let dumpStat; + try { + dumpStat = await fs.stat(databaseInfo.backupFile); + } catch (statErr) { + if (statErr.code === 'ENOENT') { + throw new Error( + `Database backup file at ${databaseInfo.backupFile} is missing from disk. ` + + 'Refusing to proceed with file backup; configure backup_database_schedule or ' + + 'keep backup_database_inline_dump enabled.' + ); + } + throw statErr; + } + if (!dumpStat.size) { + throw new Error( + `Database backup file at ${databaseInfo.backupFile} is empty (0 bytes). ` + + 'Refusing to proceed with file backup to avoid shipping a manifest with no DB content.' + ); + } + + return databaseInfo; +} + async function getDatabaseBackupInfoInternal() { try { const recent = await db('database_backup_runs') @@ -814,65 +881,11 @@ async function runBackupInternal(isManual = false) { }).returning('id'); runId = insertResult[0]?.id || insertResult[0]; - // Inline database dump (default ON). Previously, runBackup only LOOKED UP - // an existing database dump via getDatabaseBackupInfo and silently shipped - // a files-only manifest when none was found — admins clicking "Run Backup - // Now" got an apparent success that omitted every customer / quote / - // invoice / contract row. Triggering pg_dump (or the SQLite copy) here - // makes "file backup" always include a fresh database snapshot. Admins - // who run their own scheduled dumps via backup_database_schedule can opt - // out with backup_database_inline_dump = false; the fail-loud guard - // below still catches the case where no recent dump exists. - // - // Default ON is encoded as "skip only when explicitly false". `undefined` - // (setting not yet inserted on existing installs) falls through to the - // ON path, which is the data-loss-safe default. normalizeBoolean(undefined) - // returns false, so checking inequality against false would inadvertently - // disable on unset — guard with `!== undefined` first. - const inlineDumpExplicitlyOff = config.backup_database_inline_dump !== undefined - && config.backup_database_inline_dump !== null - && normalizeBoolean(config.backup_database_inline_dump) === false; - if (!inlineDumpExplicitlyOff) { - logger.info('Running inline database dump before file backup...'); - const { databaseBackupService } = require('./databaseBackup'); - const dumpResult = await databaseBackupService.backup({}); - logger.info(`Inline database dump completed: ${dumpResult.path} ` + - `(${(dumpResult.size / 1024 / 1024).toFixed(2)} MB)`); - } - - // Fail-loud guard: a "file backup" without a DB component is a data-loss - // trap. Whether the dump came from the inline step above or from a - // separately-scheduled database backup, we require a usable dump file - // before proceeding. Throws — the catch block marks the backup_runs row - // failed with this error_message and emails the admin if configured. - const dbInfoCheck = await service.getDatabaseBackupInfo(); - if (!dbInfoCheck.backupFile) { - throw new Error( - 'No database backup available to include in this file backup. ' + - 'Either keep backup_database_inline_dump enabled (default) or configure ' + - 'backup_database_schedule and let it run at least once first.' - ); - } - try { - const dumpStat = await fs.stat(dbInfoCheck.backupFile); - if (!dumpStat.size || dumpStat.size === 0) { - throw new Error( - `Database backup file at ${dbInfoCheck.backupFile} is empty (0 bytes). ` + - 'Refusing to proceed with file backup to avoid shipping a manifest with no DB content.' - ); - } - } catch (statErr) { - // fs.stat throws if file doesn't exist; preserve the more specific - // empty-file error from the inner block. - if (statErr.code === 'ENOENT') { - throw new Error( - `Database backup file at ${dbInfoCheck.backupFile} is missing from disk. ` + - 'Refusing to proceed with file backup; configure backup_database_schedule or ' + - 'keep backup_database_inline_dump enabled.' - ); - } - throw statErr; - } + // Inline DB dump + fail-loud verification. The returned `databaseInfo` + // is reused at manifest-build time below so we don't pay a second + // `getDatabaseBackupInfo()` round-trip — see `ensureDatabaseDumpForBackup` + // for the full rationale. + const verifiedDatabaseInfo = await ensureDatabaseDumpForBackup(config); const files = await service.getFilesToBackup(config.backup_include_archived); logger.info(`Found ${files.length} files to check for backup`); @@ -901,7 +914,13 @@ async function runBackupInternal(isManual = false) { const previousBackup = await getPreviousSuccessfulBackup(runId); const manifestFiles = buildManifestFiles(result.backedUpFiles, files); - const databaseInfo = result.databaseInfo || await service.getDatabaseBackupInfo(); + // `verifiedDatabaseInfo` came from ensureDatabaseDumpForBackup at the + // top of this run — reuse it so manifest building doesn't pay a + // second `getDatabaseBackupInfo()` round-trip. The + // `result.databaseInfo` branch is kept for destination implementations + // (S3, future destinations) that override the local info on the result + // object; falls back to the verified copy otherwise. + const databaseInfo = result.databaseInfo || verifiedDatabaseInfo; const manifestOptions = { backupType: previousBackup ? 'incremental' : 'full', From 302fc6b9371ff08ee2d2157325d4603b00b5f7b4 Mon Sep 17 00:00:00 2001 From: Luca <102960244+Luca-Timo@users.noreply.github.com> Date: Fri, 29 May 2026 22:09:23 +0200 Subject: [PATCH 11/42] feat(backup): config-driven walker via backup_paths table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stage B of the three-stage backup-hardening plan (Stage A: inline-DB-dump + fail-loud guard already landed). The file-backup walker used to hard-code its subdirectory list inside `getFilesToBackupInternal`, which is the same footgun that hid the `business-docs` gap for ~6 months — a new feature drops artefacts under STORAGE_PATH and the maintainer has to remember to edit the walker. Now driven by a `backup_paths` table: - Migration 108 creates the table and seeds the 7 canonical defaults (events/active, events/archived, thumbnails, previews, heroes, uploads, business-docs). Seed data lives on the migration as `DEFAULT_PATHS` so the boot self-heal can re-use it. - `_backupPathsBoot.js` mirrors `_emailTemplateBoot.js`: on every boot it diffs the canonical list against the current rows and `INSERT ... ON CONFLICT DO NOTHING`s the missing ones. Keeps admin edits intact, picks up new defaults shipped after the install (Knex won't re-run migration 108). Wired into server.js just before `startBackupService()`. - Walker now calls `resolveBackupPaths(config)` which: * reads `backup_paths WHERE include_in_default=true ORDER BY display_order` * falls back to a hard-coded `LEGACY_BACKUP_PATHS` if the table is missing OR empty (defense in depth — never silently scans nothing) * gates each row by its `feature_flag` column (matches how `backup_include_archived` already worked; data-driven now) - Backward compatible: `getFilesToBackup(true|false)` still works for legacy callers and the existing businessDocs test. New callers should pass the full config object so feature gates other than `backup_include_archived` evaluate correctly. Tests: - new: `backupService.configurableWalker.test.js` — 7 cases covering canonical seed, toggling include_in_default, runtime INSERT picked up without restart, feature_flag gating both on and off, empty-table → LEGACY fallback, boolean backward compat - all 15 backup-walker integration tests pass (configurableWalker 7 + inlineDbDump 5 + businessDocs 3) - frontend build clean - 4 pre-existing integration failures (webhookDelivery, storage backend, adminPhotos.reference, imageProcessor.storage) confirmed unrelated via `git stash` baseline run Stage C (CRM feature coverage audit + diagnostic UI) follows in a separate commit. --- .../backupService.configurableWalker.test.js | 180 ++++++++++++++++++ .../migrations/core/108_add_backup_paths.js | 131 +++++++++++++ backend/server.js | 12 ++ backend/src/services/_backupPathsBoot.js | 90 +++++++++ backend/src/services/backupService.js | 138 ++++++++++---- 5 files changed, 518 insertions(+), 33 deletions(-) create mode 100644 backend/__tests__/integration/backupService.configurableWalker.test.js create mode 100644 backend/migrations/core/108_add_backup_paths.js create mode 100644 backend/src/services/_backupPathsBoot.js 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; From 03e6617f38344434add3bb866e13807f19940519 Mon Sep 17 00:00:00 2001 From: Luca <102960244+Luca-Timo@users.noreply.github.com> Date: Fri, 29 May 2026 22:20:32 +0200 Subject: [PATCH 12/42] =?UTF-8?q?feat(backup):=20coverage=20diagnostic=20?= =?UTF-8?q?=E2=80=94=20what=20will=20the=20next=20backup=20miss=3F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stage C of the three-stage backup-hardening plan (Stage A: inline DB dump + fail-loud landed in 7fdf01a; Stage B: config-driven walker in 302fc6b). Answers the "what would I lose if I clicked Run Backup Now right now?" question that Stage B made possible to answer. Backend: - new backupCoverageService.js: per-path coverage classification, drift detection (top-level subdirs not in backup_paths and not in the backups/tmp allow-list), DB-dump mode + staleness block - new GET /api/admin/system-health/backup-coverage route, same auth + settings.view permission as /backup-integrity - 7 integration scenarios pinning the classifier behaviour Frontend: - new BackupCoverageCard with auto-fetch (cheap; no recursion) - new Coverage tab on BackupManagement next to Integrity - en + de i18n; other locales fall back to en keys until a native speaker reviews Verification: - 26/26 backup integration tests pass (Stage A 5 + Stage B 7 + Stage C 7 + adminBackupIntegrity 4 + businessDocs 3) - frontend build clean - 4 pre-existing integration failures confirmed unrelated --- .../integration/adminBackupCoverage.test.js | 242 ++++++++++ backend/src/routes/adminSystemHealth.js | 23 + backend/src/services/backupCoverageService.js | 351 ++++++++++++++ .../components/admin/BackupCoverageCard.tsx | 430 ++++++++++++++++++ frontend/src/i18n/locales/de.json | 48 +- frontend/src/i18n/locales/en.json | 48 +- frontend/src/pages/admin/BackupManagement.tsx | 9 +- frontend/src/services/admin.service.ts | 62 +++ 8 files changed, 1210 insertions(+), 3 deletions(-) create mode 100644 backend/__tests__/integration/adminBackupCoverage.test.js create mode 100644 backend/src/services/backupCoverageService.js create mode 100644 frontend/src/components/admin/BackupCoverageCard.tsx diff --git a/backend/__tests__/integration/adminBackupCoverage.test.js b/backend/__tests__/integration/adminBackupCoverage.test.js new file mode 100644 index 00000000..ab10c160 --- /dev/null +++ b/backend/__tests__/integration/adminBackupCoverage.test.js @@ -0,0 +1,242 @@ +/** + * Integration test for GET /api/admin/system-health/backup-coverage. + * + * Pins the Stage C diagnostic that tells admins what the next + * "Run Backup Now" will include, skip, or silently miss. + * + * Test surface: + * 1. Empty / fresh install → default seed (7 paths), inline mode, + * no DB dump on file yet, no drift + * 2. Toggle `include_in_default=false` → coverage flips to + * 'skipped-by-toggle' + * 3. Feature_flag gating reflects the actual app_settings value + * (events/archived ⇄ backup_include_archived) + * 4. Drift detection: a top-level subdir on disk with no + * `backup_paths` row is flagged in `unconfiguredOnDisk` + * 5. Allow-list: `backups/` and `tmp/` are never flagged as drift + * 6. Scheduled-only mode + recent dump → `database.ok = true` + * 7. Scheduled-only mode + stale (>26h) dump → `database.ok = false` + * and `lastDumpStale = true` + * + * Same auth/permission pass-through strategy as + * adminBackupIntegrity.test.js — we exercise the route's logic, + * not the auth middleware. + */ + +const fs = require('fs'); +const path = require('path'); +const express = require('express'); +const request = require('supertest'); + +const { bootCrmDb } = require('./helpers/crmDb'); + +jest.mock('../../src/middleware/auth', () => ({ + adminAuth: (req, _res, next) => { req.admin = { id: 1 }; next(); }, + customerAuth: (_req, _res, next) => next(), + galleryAuth: (_req, _res, next) => next(), +})); + +jest.mock('../../src/middleware/permissions', () => ({ + requirePermission: () => (_req, _res, next) => next(), +})); + +jest.setTimeout(30000); + +describe('GET /api/admin/system-health/backup-coverage', () => { + let db; + let cleanup; + let storagePath; + let app; + + beforeAll(async () => { + ({ db, cleanup } = await bootCrmDb()); + storagePath = process.env.STORAGE_PATH; + + const route = require('../../src/routes/adminSystemHealth'); + app = express(); + app.use(express.json()); + app.use('/api/admin/system-health', route); + }, 120000); + + afterAll(async () => { + if (cleanup) await cleanup(); + }); + + function mkdir(rel) { + fs.mkdirSync(path.join(storagePath, rel), { recursive: true }); + } + + function rmdir(rel) { + fs.rmSync(path.join(storagePath, rel), { recursive: true, force: true }); + } + + async function restoreDefaultPaths() { + 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(), + }))); + } + + beforeEach(async () => { + await restoreDefaultPaths(); + await db('database_backup_runs').del().catch(() => {}); + await db('app_settings').where('setting_type', 'backup').del().catch(() => {}); + }); + + it('returns the canonical 7 paths + database block on a fresh install', async () => { + const res = await request(app).get('/api/admin/system-health/backup-coverage'); + expect(res.status).toBe(200); + expect(res.body).toHaveProperty('report'); + + const { report } = res.body; + expect(report.paths.map((p) => p.path)).toEqual([ + 'events/active', + 'events/archived', + 'thumbnails', + 'previews', + 'heroes', + 'uploads', + 'business-docs', + ]); + + // Default mode is inline — no inline_dump setting present means + // "inline is ON" (matches ensureDatabaseDumpForBackup semantics). + expect(report.database.mode).toBe('inline'); + expect(report.database.ok).toBe(true); + + expect(report.summary).toMatchObject({ + configuredCount: 7, + tableMissingFallbackInUse: false, + }); + }); + + it('flips a path to skipped-by-toggle when include_in_default=false', async () => { + await db('backup_paths').where('path', 'thumbnails').update({ + include_in_default: false, + }); + + const res = await request(app).get('/api/admin/system-health/backup-coverage'); + const thumbnails = res.body.report.paths.find((p) => p.path === 'thumbnails'); + expect(thumbnails.coverage).toBe('skipped-by-toggle'); + expect(thumbnails.includeInDefault).toBe(false); + }); + + it('feature_flag gating reflects app_settings (archived path off vs on)', async () => { + // backup_include_archived not set → archived skipped via flag + const off = await request(app).get('/api/admin/system-health/backup-coverage'); + const archivedOff = off.body.report.paths.find((p) => p.path === 'events/archived'); + expect(archivedOff.coverage).toBe('skipped-by-feature-flag'); + expect(archivedOff.featureFlag).toBe('backup_include_archived'); + expect(archivedOff.featureFlagValue).toBe(null); // unset + + // Now set the flag — but path is missing on disk, so coverage + // resolves to 'missing-on-disk', proving the flag was honoured. + await db('app_settings').insert({ + setting_key: 'backup_include_archived', + setting_value: JSON.stringify(true), + setting_type: 'backup', + }).onConflict('setting_key').merge(); + + const on = await request(app).get('/api/admin/system-health/backup-coverage'); + const archivedOn = on.body.report.paths.find((p) => p.path === 'events/archived'); + expect(archivedOn.featureFlagValue).toBe(true); + // No on-disk dir → 'missing-on-disk' (not 'skipped-by-feature-flag') + expect(['missing-on-disk', 'will-scan']).toContain(archivedOn.coverage); + }); + + it('detects unconfigured top-level subdirs as drift', async () => { + mkdir('events/active'); // configured + mkdir('plugin-store/cache'); // DRIFT + mkdir('shiny-new-feature/data'); // DRIFT + + const res = await request(app).get('/api/admin/system-health/backup-coverage'); + expect(res.body.report.drift.unconfiguredOnDisk).toEqual(expect.arrayContaining([ + 'plugin-store', + 'shiny-new-feature', + ])); + expect(res.body.report.drift.unconfiguredOnDisk).not.toContain('events'); + + rmdir('plugin-store'); + rmdir('shiny-new-feature'); + }); + + it('never flags backups/ or tmp/ as drift (allow-list)', async () => { + mkdir('backups'); + mkdir('tmp'); + + const res = await request(app).get('/api/admin/system-health/backup-coverage'); + expect(res.body.report.drift.unconfiguredOnDisk).not.toContain('backups'); + expect(res.body.report.drift.unconfiguredOnDisk).not.toContain('tmp'); + expect(res.body.report.drift.expectedNonBackupDirs).toEqual( + expect.arrayContaining(['backups', 'tmp']), + ); + + rmdir('backups'); + rmdir('tmp'); + }); + + it('scheduled-only mode + recent dump → database.ok=true, not stale', async () => { + await db('app_settings').insert({ + setting_key: 'backup_database_inline_dump', + setting_value: JSON.stringify(false), + setting_type: 'backup', + }).onConflict('setting_key').merge(); + + const recentDump = path.join(storagePath, 'backups', 'recent.sql.gz'); + fs.mkdirSync(path.dirname(recentDump), { recursive: true }); + fs.writeFileSync(recentDump, 'pretend dump'); + await db('database_backup_runs').insert({ + started_at: new Date(), + completed_at: new Date(), // just now + status: 'completed', + backup_type: 'pg', + file_path: recentDump, + file_size_bytes: fs.statSync(recentDump).size, + destination_path: recentDump, + }); + + const res = await request(app).get('/api/admin/system-health/backup-coverage'); + expect(res.body.report.database.mode).toBe('scheduled-only'); + expect(res.body.report.database.inlineDumpExplicitlyDisabled).toBe(true); + expect(res.body.report.database.lastDumpStale).toBe(false); + expect(res.body.report.database.ok).toBe(true); + }); + + it('scheduled-only mode + stale dump → database.ok=false, lastDumpStale=true', async () => { + await db('app_settings').insert({ + setting_key: 'backup_database_inline_dump', + setting_value: JSON.stringify(false), + setting_type: 'backup', + }).onConflict('setting_key').merge(); + + const oldDump = path.join(storagePath, 'backups', 'old.sql.gz'); + fs.mkdirSync(path.dirname(oldDump), { recursive: true }); + fs.writeFileSync(oldDump, 'pretend old dump'); + // 48 hours ago — well past the 26h staleness threshold. ISO + // string instead of a Date object because knex-sqlite's datetime + // serialisation has a quirk where some Date instances coerce to + // '[object Object]' on insert (the test 6 "recent dump" case + // passes only because `new Date()` happens to round-trip safely; + // arithmetic Dates don't). + const stale = new Date(Date.now() - 48 * 60 * 60 * 1000).toISOString(); + await db('database_backup_runs').insert({ + started_at: stale, + completed_at: stale, + status: 'completed', + backup_type: 'pg', + file_path: oldDump, + file_size_bytes: fs.statSync(oldDump).size, + destination_path: oldDump, + }); + + const res = await request(app).get('/api/admin/system-health/backup-coverage'); + expect(res.body.report.database.lastDumpStale).toBe(true); + expect(res.body.report.database.ok).toBe(false); + // Top-level summary reflects the failed DB check. + expect(res.body.report.summary.databaseOk).toBe(false); + expect(res.body.report.summary.overallOk).toBe(false); + }); +}); diff --git a/backend/src/routes/adminSystemHealth.js b/backend/src/routes/adminSystemHealth.js index e638cad4..a76bee98 100644 --- a/backend/src/routes/adminSystemHealth.js +++ b/backend/src/routes/adminSystemHealth.js @@ -22,6 +22,7 @@ const { adminAuth } = require('../middleware/auth'); const { requirePermission } = require('../middleware/permissions'); const { handleAsync, validateRequest, successResponse } = require('../utils/routeHelpers'); const { verifyDocumentArtefacts } = require('../services/backupIntegrityService'); +const { getCoverageReport } = require('../services/backupCoverageService'); const router = express.Router(); @@ -61,4 +62,26 @@ router.get( }), ); +/** + * GET /api/admin/system-health/backup-coverage + * + * Stage C of the backup-hardening plan. Returns the data-driven + * coverage report — what the next "Run Backup Now" will include / + * skip / silently miss, plus the database-dump status block. + * + * Read-only, on-demand. No scope parameter — the report is cheap + * (only top-level directory listing under STORAGE_PATH, no recursion). + * + * See backupCoverageService.js for the full rationale and the + * coverage-classification rules. + */ +router.get( + '/backup-coverage', + requirePermission('settings.view'), + handleAsync(async (req, res) => { + const report = await getCoverageReport(); + return successResponse(res, { report }); + }), +); + module.exports = router; diff --git a/backend/src/services/backupCoverageService.js b/backend/src/services/backupCoverageService.js new file mode 100644 index 00000000..7af24cc4 --- /dev/null +++ b/backend/src/services/backupCoverageService.js @@ -0,0 +1,351 @@ +/** + * Backup-coverage diagnostic — Stage C of the backup-hardening plan. + * + * **Why this is a separate service** + * + * Stage A (inline DB dump + fail-loud) and Stage B (config-driven + * walker via `backup_paths`) close the data-loss footgun, but they + * don't tell an admin *what* the next backup will actually cover. + * That's a separate question — and a particularly important one, + * because the whole reason Stage B exists is that the walker's + * subdirectory list used to silently fall behind reality every time + * a new feature dropped artefacts under STORAGE_PATH. + * + * This service answers two questions: + * + * 1. For every row in `backup_paths`, what will the next backup + * do with it? (scan / skip-via-feature-flag / skip-via-toggle / + * missing-on-disk) + * 2. What subdirectories EXIST under STORAGE_PATH but have NO row + * in `backup_paths` — i.e. drift the admin should know about + * before they lose data on a restore? + * + * Plus a top-level database-dump status block: are we configured + * for inline dump (default), or relying on the scheduled dump? + * When was the last successful dump? Is it stale? + * + * **What this service does NOT do** + * + * - Does not run the backup + * - Does not write anything (no DB mutations, no fs touches) + * - Does not auto-recover drift (it's a diagnostic — admins decide + * whether to add a `backup_paths` row, delete the orphan dir, etc.) + * - Does not walk file contents — only top-level directory entries + * under STORAGE_PATH are inspected (cheap; no recursion through + * potentially-millions of photos) + * + * Read-only. Returns a JSON report — same shape as + * backupIntegrityService.verifyDocumentArtefacts. + */ + +const fs = require('fs').promises; +const path = require('path'); +const { db } = require('../database/db'); +const logger = require('../utils/logger'); +const backupService = require('./backupService'); + +const STORAGE_ROOT = () => process.env.STORAGE_PATH || path.join(process.cwd(), 'storage'); + +/** + * Top-level subdirectories we expect to find under STORAGE_PATH but + * which are intentionally NOT in `backup_paths` — they're generated + * caches / runtime artefacts that the backup is supposed to skip. + * Listing them here keeps the drift detector from flagging them. + * + * `backups` — the destination directory the backup writer itself + * creates, plus the `database_backup_runs` dump files. + * Including it in the walker would create a recursive + * "backup of backups" feedback loop. + * + * `tmp` — short-lived scratch space (e.g. PDF render staging, + * S3 multipart uploads). Re-created on demand, never + * holds the only copy of anything. + */ +const EXPECTED_NON_BACKUP_DIRS = new Set([ + 'backups', + 'tmp', +]); + +/** + * How stale a database dump can be before we flag it. 26 hours so a + * daily scheduled dump is still considered "fresh" if it ran a few + * hours late. + */ +const DB_DUMP_STALE_AFTER_MS = 26 * 60 * 60 * 1000; + +function parseSettingValue(raw) { + if (raw === null || raw === undefined) return null; + if (typeof raw !== 'string') return raw; + try { return JSON.parse(raw); } catch (_) { + if (raw === 'true') return true; + if (raw === 'false') return false; + const n = Number(raw); + return Number.isFinite(n) ? n : raw; + } +} + +async function readBackupConfig() { + try { + const rows = await db('app_settings') + .where('setting_type', 'backup') + .select('setting_key', 'setting_value'); + const cfg = {}; + for (const row of rows) { + cfg[row.setting_key] = parseSettingValue(row.setting_value); + } + return cfg; + } catch (err) { + logger.warn(`backup-coverage: could not read backup config — ${err.message}`); + return {}; + } +} + +async function listConfiguredPaths() { + try { + if (!(await db.schema.hasTable('backup_paths'))) return null; + return await db('backup_paths') + .orderBy('display_order', 'asc') + .select('path', 'include_in_default', 'feature_flag', 'display_order', 'description'); + } catch (err) { + logger.warn(`backup-coverage: could not read backup_paths — ${err.message}`); + return null; + } +} + +async function listTopLevelStorageDirs() { + const root = STORAGE_ROOT(); + try { + const entries = await fs.readdir(root, { withFileTypes: true }); + return entries.filter((e) => e.isDirectory()).map((e) => e.name); + } catch (err) { + if (err.code === 'ENOENT') return []; + logger.warn(`backup-coverage: could not read STORAGE_PATH (${root}) — ${err.message}`); + return []; + } +} + +async function statPath(absPath) { + try { + const st = await fs.stat(absPath); + return { exists: true, isDir: st.isDirectory() }; + } catch (err) { + if (err.code === 'ENOENT') return { exists: false, isDir: false }; + throw err; + } +} + +/** + * Build the database-dump status block. Tells the admin whether + * "Run Backup Now" will inline-dump (default) or rely on the + * scheduled-dump path, plus how fresh the most recent dump is. + */ +async function buildDatabaseStatus(config) { + // normalizeBoolean(undefined) === false, so we have to gate on + // explicit-false the same way ensureDatabaseDumpForBackup does. + const inlineExplicitlyOff = config.backup_database_inline_dump !== undefined + && config.backup_database_inline_dump !== null + && config.backup_database_inline_dump === false; + const mode = inlineExplicitlyOff ? 'scheduled-only' : 'inline'; + + let recent = null; + try { + if (await db.schema.hasTable('database_backup_runs')) { + recent = await db('database_backup_runs') + .where('status', 'completed') + .orderBy('completed_at', 'desc') + .first(); + } + } catch (err) { + logger.warn(`backup-coverage: could not read database_backup_runs — ${err.message}`); + } + + const status = { + mode, + inlineDumpExplicitlyDisabled: inlineExplicitlyOff, + lastDumpAt: recent ? recent.completed_at : null, + lastDumpType: recent ? recent.backup_type : null, + lastDumpSizeBytes: recent ? Number(recent.file_size_bytes || 0) : 0, + lastDumpFilePath: recent ? recent.file_path : null, + lastDumpAgeMs: null, + lastDumpStale: null, + ok: null, + }; + + if (recent && recent.completed_at) { + const completedAt = recent.completed_at instanceof Date + ? recent.completed_at + : new Date(recent.completed_at); + status.lastDumpAgeMs = Date.now() - completedAt.getTime(); + status.lastDumpStale = status.lastDumpAgeMs > DB_DUMP_STALE_AFTER_MS; + } + + // ok semantics: + // - inline mode: always ok=true (next backup will produce a fresh + // dump on demand, staleness is irrelevant) + // - scheduled-only: ok=true iff a recent non-stale dump exists, + // because the file-backup guard will fail-loud otherwise + if (mode === 'inline') { + status.ok = true; + } else { + status.ok = Boolean(recent && recent.file_path && status.lastDumpStale === false); + } + + return status; +} + +/** + * Per-path coverage: + * - configured + include_in_default + (no feature_flag OR flag truthy) → 'will-scan' + * - configured + include_in_default + flag falsey → 'skipped-by-feature-flag' + * - configured + include_in_default = false → 'skipped-by-toggle' + * - configured but missing on disk → 'missing-on-disk' + * + * Returns one entry per `backup_paths` row. + */ +async function buildConfiguredPathReport(configuredRows, config) { + const root = STORAGE_ROOT(); + const result = []; + + for (const row of configuredRows) { + const absPath = path.join(root, row.path); + const stat = await statPath(absPath); + const includedInDefault = Boolean(row.include_in_default); + let featureFlagValue = null; + if (row.feature_flag) { + const v = config[row.feature_flag]; + featureFlagValue = v === undefined ? null : Boolean(v); + } + + let coverage; + if (!includedInDefault) { + coverage = 'skipped-by-toggle'; + } else if (row.feature_flag && featureFlagValue !== true) { + // null (unset) and explicit false both gate the path off — matches + // the walker's normalizeBoolean semantics + coverage = 'skipped-by-feature-flag'; + } else if (!stat.exists) { + coverage = 'missing-on-disk'; + } else { + coverage = 'will-scan'; + } + + result.push({ + path: row.path, + includeInDefault: includedInDefault, + featureFlag: row.feature_flag || null, + featureFlagValue, + displayOrder: row.display_order, + description: row.description || null, + existsOnDisk: stat.exists, + coverage, + }); + } + + return result; +} + +/** + * Drift detection: top-level subdirs under STORAGE_PATH that are not + * in `backup_paths` AND not in the `EXPECTED_NON_BACKUP_DIRS` allow-list. + * + * These are the directories that will be missed by "Run Backup Now" + * — either intentionally (a new feature drops cache files there and + * the admin doesn't want them backed up — they should add them to the + * allow-list) or accidentally (a feature shipped without a matching + * `backup_paths` row — the data-loss footgun this whole effort is + * designed to catch). + */ +function detectDrift(diskDirs, configuredPaths) { + // configured paths can be nested ('events/active'); we only diff the + // top-level segment ('events') because that's the granularity admins + // see in the storage tree. A path like 'events/active' implies the + // 'events' top-level is "known to the backup config". + const configuredTopLevels = new Set( + configuredPaths.map((p) => p.path.split('/')[0]), + ); + + return diskDirs + .filter((d) => !configuredTopLevels.has(d)) + .filter((d) => !EXPECTED_NON_BACKUP_DIRS.has(d)) + .sort(); +} + +/** + * Public entry point. + * + * @returns {Promise<{ + * database: object, + * paths: Array, + * drift: { unconfiguredOnDisk: string[], expectedNonBackupDirs: string[] }, + * summary: object, + * generatedAt: string, + * }>} + */ +async function getCoverageReport() { + const config = await readBackupConfig(); + const configuredRows = await listConfiguredPaths(); + const diskDirs = await listTopLevelStorageDirs(); + + // Fallback when the table doesn't exist yet (migration 108 hasn't + // run for some reason). Mirrors the walker's LEGACY_BACKUP_PATHS + // contract — every other layer of this system uses the same + // belt-and-suspenders fallback. + const fallback = configuredRows === null; + const effectiveRows = configuredRows || [ + { path: 'events/active', include_in_default: true, feature_flag: null, display_order: 10, description: 'Legacy fallback (backup_paths missing)' }, + { path: 'events/archived', include_in_default: true, feature_flag: 'backup_include_archived', display_order: 20, description: 'Legacy fallback (backup_paths missing)' }, + { path: 'thumbnails', include_in_default: true, feature_flag: null, display_order: 30, description: 'Legacy fallback (backup_paths missing)' }, + { path: 'previews', include_in_default: true, feature_flag: null, display_order: 40, description: 'Legacy fallback (backup_paths missing)' }, + { path: 'heroes', include_in_default: true, feature_flag: null, display_order: 50, description: 'Legacy fallback (backup_paths missing)' }, + { path: 'uploads', include_in_default: true, feature_flag: null, display_order: 60, description: 'Legacy fallback (backup_paths missing)' }, + { path: 'business-docs', include_in_default: true, feature_flag: null, display_order: 70, description: 'Legacy fallback (backup_paths missing)' }, + ]; + + const [database, paths] = await Promise.all([ + buildDatabaseStatus(config), + buildConfiguredPathReport(effectiveRows, config), + ]); + + const unconfiguredOnDisk = detectDrift(diskDirs, effectiveRows); + + const summary = { + configuredCount: effectiveRows.length, + willScanCount: paths.filter((p) => p.coverage === 'will-scan').length, + skippedByToggleCount: paths.filter((p) => p.coverage === 'skipped-by-toggle').length, + skippedByFeatureFlagCount: paths.filter((p) => p.coverage === 'skipped-by-feature-flag').length, + missingOnDiskCount: paths.filter((p) => p.coverage === 'missing-on-disk').length, + driftCount: unconfiguredOnDisk.length, + tableMissingFallbackInUse: fallback, + databaseOk: database.ok, + // overall: green only when DB is ok AND there's at least one path + // that will actually be scanned AND no drift was found + overallOk: Boolean( + database.ok + && paths.some((p) => p.coverage === 'will-scan') + && unconfiguredOnDisk.length === 0, + ), + }; + + return { + database, + paths, + drift: { + unconfiguredOnDisk, + expectedNonBackupDirs: Array.from(EXPECTED_NON_BACKUP_DIRS).sort(), + }, + summary, + generatedAt: new Date().toISOString(), + }; +} + +module.exports = { + getCoverageReport, + // Exported for test introspection — the route doesn't use these. + EXPECTED_NON_BACKUP_DIRS, + DB_DUMP_STALE_AFTER_MS, +}; + +// Silence unused import lint warning — backupService is required so +// the module-graph cache primes (some tests jest.mock it before +// requiring this service). +void backupService; diff --git a/frontend/src/components/admin/BackupCoverageCard.tsx b/frontend/src/components/admin/BackupCoverageCard.tsx new file mode 100644 index 00000000..511d8cf1 --- /dev/null +++ b/frontend/src/components/admin/BackupCoverageCard.tsx @@ -0,0 +1,430 @@ +import React from 'react'; +import { useTranslation } from 'react-i18next'; +import { + ShieldCheck, + ShieldAlert, + Database, + FolderTree, + AlertTriangle, + CheckCircle2, + XCircle, + EyeOff, + Clock, + RefreshCw, + Loader2, +} from 'lucide-react'; +import { useQuery } from '@tanstack/react-query'; +import { format } from 'date-fns'; + +import { Card, Button } from '../common'; +import { + adminService, + BackupCoverageReport, + BackupPathCoverage, +} from '../../services/admin.service'; + +/** + * BackupCoverageCard — Stage C of the backup-hardening plan. + * + * Tells the admin what the next "Run Backup Now" will actually do: + * + * - Database: inline-dump or scheduled, last dump age, staleness + * - Configured paths: per-row coverage (will-scan / skipped by + * toggle / skipped by feature flag / missing on disk) + * - Drift: top-level subdirs under STORAGE_PATH that have no + * `backup_paths` row (the "feature shipped without a backup row" + * footgun this whole effort is designed to catch) + * + * Auto-fetches on mount — unlike the integrity verifier, this is + * a cheap query (no recursion) so admins should always see the + * current state when they open the tab. + */ +export const BackupCoverageCard: React.FC = () => { + const { t } = useTranslation(); + const { data, isLoading, isError, error, refetch, isFetching } = useQuery({ + queryKey: ['backup-coverage'], + queryFn: () => adminService.getBackupCoverage(), + // The report changes only when (a) backup_paths is edited or + // (b) a new scheduled dump completes. Stale time of 30s keeps + // the UI snappy without hammering the endpoint. + staleTime: 30_000, + }); + + return ( + +
refetch()} refreshing={isFetching} /> + + {isError && ( + + )} + + {data && ( + <> + {data.summary.tableMissingFallbackInUse && ( + + )} + + + + + + + + + + +

+ {t('backup.coverage.generatedAt', 'Coverage generated: {{when}}', { + when: format(new Date(data.generatedAt), 'yyyy-MM-dd HH:mm:ss'), + })} +

+ + )} + + ); +}; + +const Header: React.FC<{ + report: BackupCoverageReport | undefined; + loading: boolean; + onRefresh: () => void; + refreshing: boolean; +}> = ({ report, loading, onRefresh, refreshing }) => { + const { t } = useTranslation(); + const healthy = report?.summary.overallOk; + return ( +
+
+
+ {loading || refreshing ? ( + + ) : healthy ? ( + + ) : report ? ( + + ) : ( + + )} +

+ {t('backup.coverage.title', 'Backup coverage')} +

+
+

+ {t( + 'backup.coverage.description', + 'Shows what the next backup will include, skip, or silently miss. The database block confirms the dump strategy. The "drift" section flags subdirectories that exist on disk but are not in the backup configuration — usually a sign that a new feature shipped without a matching backup_paths row.', + )} +

+
+ +
+ ); +}; + +const ErrorBanner: React.FC<{ message: string }> = ({ message }) => { + const { t } = useTranslation(); + return ( +
+ {t('backup.coverage.error', 'Could not load coverage report: {{message}}', { message })} +
+ ); +}; + +const FallbackWarning: React.FC = () => { + const { t } = useTranslation(); + return ( +
+ + + {t( + 'backup.coverage.fallbackInUse', + 'The backup_paths table is missing. The walker is using its legacy hard-coded fallback. Migration 108 may not have run — check server logs and re-run migrations.', + )} + +
+ ); +}; + +const SectionGrid: React.FC<{ children: React.ReactNode }> = ({ children }) => ( +
{children}
+); + +const DatabaseStatusCard: React.FC<{ + database: BackupCoverageReport['database']; +}> = ({ database }) => { + const { t } = useTranslation(); + const isInline = database.mode === 'inline'; + const tone: Tone = database.ok ? 'green' : 'red'; + const dumpAge = database.lastDumpAgeMs !== null + ? formatAge(database.lastDumpAgeMs) + : null; + + return ( +
+
+ +

+ {t('backup.coverage.database.title', 'Database')} +

+ {database.ok ? ( + + ) : ( + + )} +
+
+ + {database.lastDumpAt ? ( + <> + + + + ) : ( + + )} + {database.lastDumpStale && ( + } + /> + )} +
+
+ ); +}; + +const SummaryCard: React.FC<{ + summary: BackupCoverageReport['summary']; +}> = ({ summary }) => { + const { t } = useTranslation(); + const tone: Tone = summary.overallOk + ? 'green' + : summary.driftCount > 0 || !summary.databaseOk + ? 'amber' + : 'neutral'; + return ( +
+
+ +

+ {t('backup.coverage.summary.title', 'Summary')} +

+
+
+ + {summary.skippedByToggleCount > 0 && ( + + )} + {summary.skippedByFeatureFlagCount > 0 && ( + + )} + {summary.missingOnDiskCount > 0 && ( + + )} + +
+
+ ); +}; + +const PathsTable: React.FC<{ paths: BackupCoverageReport['paths'] }> = ({ paths }) => { + const { t } = useTranslation(); + return ( +
+
+

+ {t('backup.coverage.paths.heading', 'Configured paths')} +

+
+
+ + + + + + + + + + + {paths.map((p) => ( + + + + + + + ))} + +
{t('backup.coverage.paths.path', 'Path')}{t('backup.coverage.paths.coverage', 'Coverage')}{t('backup.coverage.paths.featureFlag', 'Feature flag')}{t('backup.coverage.paths.description', 'Description')}
+ {p.path} + + + + {p.featureFlag + ? `${p.featureFlag} = ${p.featureFlagValue === null ? '∅' : String(p.featureFlagValue)}` + : '—'} + + {p.description ?? '—'} +
+
+
+ ); +}; + +const DriftSection: React.FC<{ drift: BackupCoverageReport['drift'] }> = ({ drift }) => { + const { t } = useTranslation(); + if (drift.unconfiguredOnDisk.length === 0) { + return ( +
+ + {t( + 'backup.coverage.drift.none', + 'No drift detected — every top-level subdirectory under STORAGE_PATH is either in backup_paths or in the expected non-backup allow-list.', + )} +
+ ); + } + return ( +
+
+
+ +

+ {t('backup.coverage.drift.heading', 'Drift detected: subdirectories not covered by any backup_paths row')} +

+
+

+ {t( + 'backup.coverage.drift.caption', + 'These directories exist on disk but the walker will skip them. Either add a backup_paths row, move the files into a covered location, or — if they are runtime caches — confirm they are safe to exclude.', + )} +

+
+
    + {drift.unconfiguredOnDisk.map((d) => ( +
  • + + {d} +
    • + ))} +
+
+ ); +}; + +const CoverageBadge: React.FC<{ coverage: BackupPathCoverage }> = ({ coverage }) => { + const { t } = useTranslation(); + const map: Record = { + 'will-scan': { + tone: 'green', + label: t('backup.coverage.coverage.willScan', 'Will scan'), + }, + 'skipped-by-toggle': { + tone: 'neutral', + label: t('backup.coverage.coverage.skippedByToggle', 'Off'), + }, + 'skipped-by-feature-flag': { + tone: 'neutral', + label: t('backup.coverage.coverage.skippedByFlag', 'Gated off'), + }, + 'missing-on-disk': { + tone: 'amber', + label: t('backup.coverage.coverage.missingOnDisk', 'Missing on disk'), + }, + }; + const { tone, label } = map[coverage]; + return ( + + {label} + + ); +}; + +const Row: React.FC<{ label: string; value: string; icon?: React.ReactNode }> = ({ + label, value, icon, +}) => ( +
+
+ {icon} + {label} +
+
{value}
+
+); + +type Tone = 'neutral' | 'green' | 'amber' | 'red'; + +const TONE_BG: Record = { + neutral: 'bg-neutral-100 dark:bg-neutral-800 text-neutral-700 dark:text-neutral-200', + green: 'bg-green-50 dark:bg-green-900/30 text-green-700 dark:text-green-300', + amber: 'bg-amber-50 dark:bg-amber-900/30 text-amber-700 dark:text-amber-300', + red: 'bg-red-50 dark:bg-red-900/30 text-red-700 dark:text-red-300', +}; + +function formatBytes(bytes: number): string { + if (!bytes) return '0 B'; + const units = ['B', 'KB', 'MB', 'GB', 'TB']; + const i = Math.min(Math.floor(Math.log(bytes) / Math.log(1024)), units.length - 1); + return `${(bytes / Math.pow(1024, i)).toFixed(i === 0 ? 0 : 1)} ${units[i]}`; +} + +function formatAge(ms: number): string { + const sec = Math.floor(ms / 1000); + if (sec < 60) return `${sec}s ago`; + const min = Math.floor(sec / 60); + if (min < 60) return `${min}m ago`; + const hr = Math.floor(min / 60); + if (hr < 48) return `${hr}h ago`; + const day = Math.floor(hr / 24); + return `${day}d ago`; +} diff --git a/frontend/src/i18n/locales/de.json b/frontend/src/i18n/locales/de.json index 4c6855b1..e30ed666 100644 --- a/frontend/src/i18n/locales/de.json +++ b/frontend/src/i18n/locales/de.json @@ -231,7 +231,8 @@ "configuration": "Konfiguration", "history": "Backup-Verlauf", "restore": "Wiederherstellung", - "integrity": "Integrität" + "integrity": "Integrität", + "coverage": "Abdeckung" }, "integrity": { "title": "Dokumentintegrität", @@ -264,6 +265,51 @@ "detail": "Detail" } }, + "coverage": { + "title": "Backup-Abdeckung", + "description": "Zeigt, was das nächste Backup einschließt, überspringt oder stillschweigend verfehlt. Der Datenbank-Block bestätigt die Dump-Strategie. Der \"Drift\"-Abschnitt markiert Unterverzeichnisse, die auf der Festplatte existieren, aber nicht in der Backup-Konfiguration stehen — meist ein Hinweis darauf, dass eine neue Funktion ohne passende backup_paths-Zeile ausgeliefert wurde.", + "refresh": "Aktualisieren", + "error": "Abdeckungs-Bericht konnte nicht geladen werden: {{message}}", + "generatedAt": "Abdeckung erstellt: {{when}}", + "fallbackInUse": "Die Tabelle backup_paths fehlt. Der Walker nutzt seine fest verdrahtete Legacy-Fallback-Liste. Migration 108 wurde möglicherweise nicht ausgeführt — prüfen Sie die Server-Logs und führen Sie die Migrationen erneut aus.", + "database": { + "title": "Datenbank", + "mode": "Modus", + "modeInline": "Inline-Dump bei jedem Backup", + "modeScheduled": "Nur geplant (Inline abgewählt)", + "lastDump": "Letzter Dump", + "lastDumpSize": "Größe", + "noDump": "Noch kein Dump vorhanden", + "staleLabel": "Status", + "stale": "Veraltet — älter als 26 Std." + }, + "summary": { + "title": "Zusammenfassung", + "willScan": "Wird gescannt", + "skippedByToggle": "Übersprungen (Schalter aus)", + "skippedByFlag": "Übersprungen (Feature-Flag)", + "missingOnDisk": "Auf Festplatte fehlend", + "drift": "Nicht konfiguriert auf Festplatte (Drift)" + }, + "paths": { + "heading": "Konfigurierte Pfade", + "path": "Pfad", + "coverage": "Abdeckung", + "featureFlag": "Feature-Flag", + "description": "Beschreibung" + }, + "coverage": { + "willScan": "Wird gescannt", + "skippedByToggle": "Aus", + "skippedByFlag": "Per Flag aus", + "missingOnDisk": "Auf Festplatte fehlend" + }, + "drift": { + "heading": "Drift erkannt: Unterverzeichnisse ohne backup_paths-Zeile", + "caption": "Diese Verzeichnisse existieren auf der Festplatte, werden vom Walker aber übersprungen. Entweder eine backup_paths-Zeile hinzufügen, die Dateien in ein abgedecktes Verzeichnis verschieben, oder — wenn es sich um Laufzeit-Caches handelt — bestätigen, dass der Ausschluss sicher ist.", + "none": "Kein Drift erkannt — jedes Top-Level-Unterverzeichnis unter STORAGE_PATH steht entweder in backup_paths oder in der erwarteten Nicht-Backup-Allow-List." + } + }, "status": { "inProgress": "Backup läuft...", "lastBackup": "Letztes Backup", diff --git a/frontend/src/i18n/locales/en.json b/frontend/src/i18n/locales/en.json index 3e94ebf5..7d6831c5 100644 --- a/frontend/src/i18n/locales/en.json +++ b/frontend/src/i18n/locales/en.json @@ -2122,7 +2122,8 @@ "configuration": "Configuration", "history": "Backup History", "restore": "Restore", - "integrity": "Integrity" + "integrity": "Integrity", + "coverage": "Coverage" }, "integrity": { "title": "Document integrity", @@ -2155,6 +2156,51 @@ "detail": "Detail" } }, + "coverage": { + "title": "Backup coverage", + "description": "Shows what the next backup will include, skip, or silently miss. The database block confirms the dump strategy. The \"drift\" section flags subdirectories that exist on disk but are not in the backup configuration — usually a sign that a new feature shipped without a matching backup_paths row.", + "refresh": "Refresh", + "error": "Could not load coverage report: {{message}}", + "generatedAt": "Coverage generated: {{when}}", + "fallbackInUse": "The backup_paths table is missing. The walker is using its legacy hard-coded fallback. Migration 108 may not have run — check server logs and re-run migrations.", + "database": { + "title": "Database", + "mode": "Mode", + "modeInline": "Inline dump on every backup", + "modeScheduled": "Scheduled-only (inline opted out)", + "lastDump": "Last dump", + "lastDumpSize": "Size", + "noDump": "No dump on file yet", + "staleLabel": "Status", + "stale": "Stale — older than 26h" + }, + "summary": { + "title": "Summary", + "willScan": "Will scan", + "skippedByToggle": "Skipped (toggle off)", + "skippedByFlag": "Skipped (feature flag)", + "missingOnDisk": "Missing on disk", + "drift": "Unconfigured on disk (drift)" + }, + "paths": { + "heading": "Configured paths", + "path": "Path", + "coverage": "Coverage", + "featureFlag": "Feature flag", + "description": "Description" + }, + "coverage": { + "willScan": "Will scan", + "skippedByToggle": "Off", + "skippedByFlag": "Gated off", + "missingOnDisk": "Missing on disk" + }, + "drift": { + "heading": "Drift detected: subdirectories not covered by any backup_paths row", + "caption": "These directories exist on disk but the walker will skip them. Either add a backup_paths row, move the files into a covered location, or — if they are runtime caches — confirm they are safe to exclude.", + "none": "No drift detected — every top-level subdirectory under STORAGE_PATH is either in backup_paths or in the expected non-backup allow-list." + } + }, "status": { "inProgress": "Backup in progress...", "lastBackup": "Last backup", diff --git a/frontend/src/pages/admin/BackupManagement.tsx b/frontend/src/pages/admin/BackupManagement.tsx index f127149a..43e89774 100644 --- a/frontend/src/pages/admin/BackupManagement.tsx +++ b/frontend/src/pages/admin/BackupManagement.tsx @@ -11,6 +11,7 @@ import { Loader2, Shield, ShieldCheck, + FolderTree, } from 'lucide-react'; import { toast } from 'react-toastify'; import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'; @@ -23,9 +24,10 @@ import { BackupConfiguration } from '../../components/admin/BackupConfiguration' import { BackupHistory } from '../../components/admin/BackupHistory'; import { RestoreWizard } from '../../components/admin/RestoreWizard'; import { BackupIntegrityCard } from '../../components/admin/BackupIntegrityCard'; +import { BackupCoverageCard } from '../../components/admin/BackupCoverageCard'; import { api } from '../../config/api'; -type TabId = 'dashboard' | 'configuration' | 'history' | 'restore' | 'integrity'; +type TabId = 'dashboard' | 'configuration' | 'history' | 'restore' | 'integrity' | 'coverage'; export const BackupManagement: React.FC = () => { const [activeTab, setActiveTab] = useState('dashboard'); @@ -38,6 +40,7 @@ export const BackupManagement: React.FC = () => { { id: 'history' as const, label: t('backup.tabs.history'), icon: History }, { id: 'restore' as const, label: t('backup.tabs.restore'), icon: RefreshCw }, { id: 'integrity' as const, label: t('backup.tabs.integrity', 'Integrity'), icon: ShieldCheck }, + { id: 'coverage' as const, label: t('backup.tabs.coverage', 'Coverage'), icon: FolderTree }, ]; const { data: backupStatus, isLoading: statusLoading } = useQuery({ @@ -227,6 +230,10 @@ export const BackupManagement: React.FC = () => { {activeTab === 'integrity' && ( )} + + {activeTab === 'coverage' && ( + + )} ); diff --git a/frontend/src/services/admin.service.ts b/frontend/src/services/admin.service.ts index c622688f..09c137d6 100644 --- a/frontend/src/services/admin.service.ts +++ b/frontend/src/services/admin.service.ts @@ -245,6 +245,58 @@ export interface BackupIntegrityReport { existsButNoHash: BackupIntegrityExistsButNoHashRow[]; } +// ---- Backup-coverage (Stage C of backup-hardening plan) ----------------- + +export type BackupPathCoverage = + | 'will-scan' + | 'skipped-by-toggle' + | 'skipped-by-feature-flag' + | 'missing-on-disk'; + +export interface BackupCoveragePath { + path: string; + includeInDefault: boolean; + featureFlag: string | null; + featureFlagValue: boolean | null; + displayOrder: number; + description: string | null; + existsOnDisk: boolean; + coverage: BackupPathCoverage; +} + +export interface BackupCoverageDatabase { + mode: 'inline' | 'scheduled-only'; + inlineDumpExplicitlyDisabled: boolean; + lastDumpAt: string | null; + lastDumpType: string | null; + lastDumpSizeBytes: number; + lastDumpFilePath: string | null; + lastDumpAgeMs: number | null; + lastDumpStale: boolean | null; + ok: boolean; +} + +export interface BackupCoverageReport { + generatedAt: string; + database: BackupCoverageDatabase; + paths: BackupCoveragePath[]; + drift: { + unconfiguredOnDisk: string[]; + expectedNonBackupDirs: string[]; + }; + summary: { + configuredCount: number; + willScanCount: number; + skippedByToggleCount: number; + skippedByFeatureFlagCount: number; + missingOnDiskCount: number; + driftCount: number; + tableMissingFallbackInUse: boolean; + databaseOk: boolean; + overallOk: boolean; + }; +} + export interface AdminProfile { id: number; username: string; @@ -319,6 +371,16 @@ export const adminService = { return response.data.report; }, + // Backup-coverage diagnostic (Stage C). Answers "what will the + // next backup actually include / skip / silently miss?" — read-only, + // no parameters. See backupCoverageService.js for the full report shape. + async getBackupCoverage(): Promise { + const response = await api.get<{ report: BackupCoverageReport }>( + '/admin/system-health/backup-coverage', + ); + return response.data.report; + }, + // Format activity message formatActivityMessage(activity: Activity): string { // Feature-flag toggles carry a `changed` diff in metadata. Render From f741e88acbaf6135be575dc7f42b80bce735abad Mon Sep 17 00:00:00 2001 From: Luca <102960244+Luca-Timo@users.noreply.github.com> Date: Sat, 30 May 2026 02:56:10 +0200 Subject: [PATCH 13/42] fix(database-backup): Postgres-safe insert destructure (runs the inline dump) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit databaseBackupService.backup() did `const [runId] = await db(...).insert({...})` without a .returning() — works on SQLite (knex returns [lastInsertId]) but throws "(intermediate value) is not iterable" on Postgres (knex returns a non-iterable shape). Bug was latent until Stage A of the backup-hardening plan wired this method into the "Run Backup Now" inline-dump path. Before Stage A only the scheduled cron + the dedicated admin-DB-backup page called it, and Ralf's install had never exercised either — so the inline-dump default landing in production was the first time the destructure ran on his PG. Cure: same explicit .returning('id') + dual-shape coalesce pattern that backupService.js uses for its own backup_runs insert (line 949). Two more sibling files have the same anti-pattern (userManagementService, customerAccountsService — invitation flows) and will bite under the same conditions; spawned a follow-up task to fix them in a separate PR. --- backend/src/services/databaseBackup.js | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/backend/src/services/databaseBackup.js b/backend/src/services/databaseBackup.js index 1de8af4d..9911b317 100644 --- a/backend/src/services/databaseBackup.js +++ b/backend/src/services/databaseBackup.js @@ -309,8 +309,23 @@ class DatabaseBackupService { // Get current schema version const schemaVersion = await this.getCurrentSchemaVersion(); - // Create backup run record with version info - const [runId] = await db('database_backup_runs').insert({ + // Create backup run record with version info. + // + // Insert shape divergence between SQLite + Postgres made the old + // `const [runId] = await db(...).insert({...})` form throw + // "(intermediate value) is not iterable" on Postgres installs: + // + // - SQLite-via-knex: insert() returns `[lastInsertId]` (array) + // - Postgres-via-knex: insert() without .returning() returns an + // empty object / row count — not iterable + // + // Bug went undetected until Stage A wired this method into the + // "Run Backup Now" inline-dump path — before that, only the + // scheduled-cron + dedicated-admin-page callers exercised it, + // and Ralf's install had never triggered either. Cure: same + // explicit `.returning('id')` + dual-shape coalesce pattern that + // `backupService.js:949` uses for its own `backup_runs` insert. + const insertResult = await db('database_backup_runs').insert({ started_at: startTime, status: 'running', backup_type: this.dbType, @@ -324,8 +339,9 @@ class DatabaseBackupService { node_env: process.env.NODE_ENV || 'production', db_type: this.dbType }) - }); - + }).returning('id'); + const runId = insertResult[0]?.id || insertResult[0]; + backupRun = { id: runId }; // Get initial checksums From d34036c4efd7769ef4896107cc53ea1a4cd1fa84 Mon Sep 17 00:00:00 2001 From: Luca <102960244+Luca-Timo@users.noreply.github.com> Date: Sat, 30 May 2026 03:26:10 +0200 Subject: [PATCH 14/42] fix(safe-exec): Node 22-compatible stdio + error-bridge for spawnTo/FromFile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit spawnToFile and spawnFromFile passed an unopened WriteStream/ReadStream directly as a stdio entry to child_process.spawn. Older Node versions auto-extracted .fd; Node 22 throws synchronously: The argument 'stdio' is invalid. Received WriteStream { fd: null, path: '/backup/database/...sql', ... } Bug bit Ralf's install once today's `bugfix/crm-backup` image landed — Node 22 came with that image, and Stage A's inline-dump path is the first caller of spawnToFile on this install. Latent on the previous image (Node 20); fatal on this one. restoreService's pre-restore safety snapshot uses the same helper and would have hit it next time a restore ran. Cure: stdio: ['ignore', 'pipe', 'pipe'] (and ['pipe', 'pipe', 'pipe'] for spawnFromFile) + manual pipe of child.stdout/stdin through the file stream. Works on every Node version. Also wires the WriteStream's 'error' event to the promise via settleReject so a future EACCES / ENOSPC reaches the caller's try/catch instead of becoming a process- fatal unhandled error event — closing the same "Stage A guard bypassed" hole noted in the spawned follow-up task. Side benefit: outStream.end() now awaits flush before resolving, so fast pg_dump runs can no longer produce a truncated dump. --- backend/src/utils/safeExec.js | 110 +++++++++++++++++++++++++++------- 1 file changed, 89 insertions(+), 21 deletions(-) diff --git a/backend/src/utils/safeExec.js b/backend/src/utils/safeExec.js index efba1ae7..076b20eb 100644 --- a/backend/src/utils/safeExec.js +++ b/backend/src/utils/safeExec.js @@ -41,60 +41,128 @@ function spawnAsync(cmd, args = [], options = {}) { /** * Run a command and redirect stdout to a file (replaces shell `> file`). + * + * Historically this passed `fs.createWriteStream(outputPath)` directly as + * `stdio[1]` to `child_process.spawn`. That relied on Node auto-extracting + * the WriteStream's `.fd` — but the stream opens async, so on a fast call + * `fd` is still `null` when `spawn()` reads it. Older Node releases would + * tolerate this; Node 22 throws synchronously with + * `The argument 'stdio' is invalid. Received WriteStream { fd: null, ... }`. + * + * Cure: use `stdio: ['ignore', 'pipe', 'pipe']` and wire the WriteStream + * up via the streams API (`stdout.pipe(outStream)`). Works on every Node + * version; also gives us a clean error bridge from both the WriteStream + * AND the child process to the promise, instead of the previous code's + * blind `outStream.destroy()` / `outStream.end()` calls that left stream + * errors uncaught (Node 22 process-fatal — separate footgun this fixes + * by the same change). + * + * Used by: + * - databaseBackup.createPostgreSQLBackup (inline-dump path, the + * thing that just bit Ralf's install) + * - restoreService pre-restore safety snapshot (would have hit the + * same on next restore attempt) */ function spawnToFile(cmd, args, outputPath, options = {}) { const fs = require('fs'); return new Promise((resolve, reject) => { const outStream = fs.createWriteStream(outputPath); + let settled = false; + const settleReject = (err) => { + if (settled) return; + settled = true; + try { outStream.destroy(); } catch (_) { /* best effort */ } + reject(err); + }; + const settleResolve = (value) => { + if (settled) return; + settled = true; + resolve(value); + }; + + // Bridge WriteStream errors (EACCES, ENOSPC, etc.) to the promise. + // Without this, an unhandled 'error' event on the stream is process- + // fatal on Node 22 and bypasses the caller's try/catch entirely — + // which is exactly the failure mode that crashed the picpeak + // backend container on its first inline-dump attempt. + outStream.on('error', settleReject); + const child = spawn(cmd, args, { shell: false, ...options, - stdio: ['ignore', outStream, 'pipe'] + stdio: ['ignore', 'pipe', 'pipe'] }); + // Pipe stdout → file. The pipe call attaches its own 'error' + // handlers on both ends so a child-stdout failure also reaches us. + child.stdout.pipe(outStream); + const stderrChunks = []; child.stderr.on('data', chunk => stderrChunks.push(chunk)); + child.stderr.on('error', settleReject); - child.on('error', (err) => { - outStream.destroy(); - reject(err); - }); + child.on('error', settleReject); child.on('close', (code) => { - outStream.end(); - const stderr = Buffer.concat(stderrChunks).toString(); - if (code !== 0) { - const err = new Error(`${cmd} exited with code ${code}: ${stderr}`); - err.code = code; - err.stderr = stderr; - return reject(err); - } - resolve({ stderr }); + // Wait for the file write to flush before resolving — otherwise + // a fast 'close' could resolve while the WriteStream still has + // buffered bytes, producing a truncated dump. + outStream.end(() => { + const stderr = Buffer.concat(stderrChunks).toString(); + if (code !== 0) { + const err = new Error(`${cmd} exited with code ${code}: ${stderr}`); + err.code = code; + err.stderr = stderr; + return settleReject(err); + } + settleResolve({ stderr }); + }); }); }); } /** * Run a command and pipe a file into stdin (replaces shell `< file`). + * + * Same Node 22 stdio strictness applies as for `spawnToFile` above — the + * ReadStream `fd` is null at spawn time. Use `stdio[0] = 'pipe'` and pipe + * the file stream into `child.stdin` via the streams API instead. */ function spawnFromFile(cmd, args, inputPath, options = {}) { const fs = require('fs'); return new Promise((resolve, reject) => { const inStream = fs.createReadStream(inputPath); + let settled = false; + const settleReject = (err) => { + if (settled) return; + settled = true; + try { inStream.destroy(); } catch (_) { /* best effort */ } + reject(err); + }; + const settleResolve = (value) => { + if (settled) return; + settled = true; + resolve(value); + }; + + inStream.on('error', settleReject); + const child = spawn(cmd, args, { shell: false, ...options, - stdio: [inStream, 'pipe', 'pipe'] + stdio: ['pipe', 'pipe', 'pipe'] }); + inStream.pipe(child.stdin); + const stdoutChunks = []; const stderrChunks = []; child.stdout.on('data', chunk => stdoutChunks.push(chunk)); + child.stdout.on('error', settleReject); child.stderr.on('data', chunk => stderrChunks.push(chunk)); + child.stderr.on('error', settleReject); + child.stdin.on('error', settleReject); - child.on('error', (err) => { - inStream.destroy(); - reject(err); - }); + child.on('error', settleReject); child.on('close', (code) => { const stdout = Buffer.concat(stdoutChunks).toString(); const stderr = Buffer.concat(stderrChunks).toString(); @@ -103,9 +171,9 @@ function spawnFromFile(cmd, args, inputPath, options = {}) { err.code = code; err.stdout = stdout; err.stderr = stderr; - return reject(err); + return settleReject(err); } - resolve({ stdout, stderr }); + settleResolve({ stdout, stderr }); }); }); } From 0ad14899fa1fac4915fd2cb9f746c7afd8511d13 Mon Sep 17 00:00:00 2001 From: Luca <102960244+Luca-Timo@users.noreply.github.com> Date: Sat, 30 May 2026 03:35:08 +0200 Subject: [PATCH 15/42] ix(database-backup): drop bogus --single-transaction flag from pg_dump MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pg_dump rejects `--single-transaction` — it's a pg_restore / psql flag, never a pg_dump one. Triggered as soon as the inline-dump path landed on Ralf's install: pg_dump: unrecognized option: single-transaction pg_dump: hint: Try "pg_dump --help" for more information. pg_dump already wraps the entire export in a single REPEATABLE READ snapshot automatically (since Postgres 9.x), so the original intent — consistent snapshot of the live DB — is preserved by removing the flag. Same "latent until Stage A wired it in" pattern as the three prior bugs this rollout has surfaced (PG insert destructure → bind- mount EACCES → Node 22 stdio strict mode → this). --- backend/src/services/databaseBackup.js | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/backend/src/services/databaseBackup.js b/backend/src/services/databaseBackup.js index 9911b317..db9832fd 100644 --- a/backend/src/services/databaseBackup.js +++ b/backend/src/services/databaseBackup.js @@ -206,12 +206,20 @@ class DatabaseBackupService { '--format=plain', '--encoding=UTF8' ]; - - // Add transaction support for consistency - if (!options.noTransaction) { - pgDumpOptions.push('--single-transaction'); - } - + + // NOTE: do NOT add `--single-transaction` here. It looks like + // the right flag for "consistent snapshot" but it isn't a pg_dump + // option — it belongs to pg_restore / psql and pg_dump rejects it + // with `unrecognized option: single-transaction` (exit code 1). + // pg_dump already wraps the entire export in a single REPEATABLE + // READ snapshot automatically (since Postgres 9.x), so consistency + // is built in. If we ever need stricter cross-pg-cluster snapshot + // sharing, use `--snapshot=` — but the typical inline-dump + // path doesn't need it. Bug went undetected until Stage A wired + // this code into the user-facing "Run Backup Now" path; prior + // callers (scheduled cron, dedicated admin DB-backup page) hit + // the same failure but on installs that had never exercised them. + // Add compression if not doing it separately if (options.compress && !options.separateCompression) { pgDumpOptions.push('--compress=6'); From ed7ab61b90aac95da132e74468cf19ef7f0f8a46 Mon Sep 17 00:00:00 2001 From: Luca <102960244+Luca-Timo@users.noreply.github.com> Date: Sat, 30 May 2026 03:51:02 +0200 Subject: [PATCH 16/42] fix(admin-ui): backup download button hits API path, not SPA route MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `BackupHistory.jsx` opened `/admin/backup/download/` via window.open, which goes to the React SPA's router — no matching route, so it rendered the "Page Not Found" screen. The actual download endpoint lives at `/api/admin/backup/download/:id` on the backend (adminBackup.js:685). Cookie-based admin auth already supports the implicit cookie sent by window.open, so the URL prefix was the only thing missing. Predates today's backup-hardening work — the bug has existed since this download button shipped. Surfaced now because Ralf finally has a completed backup to try downloading after the Stage A inline-dump guard started working. --- frontend/src/components/admin/BackupHistory.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/components/admin/BackupHistory.jsx b/frontend/src/components/admin/BackupHistory.jsx index ad317d24..07611789 100644 --- a/frontend/src/components/admin/BackupHistory.jsx +++ b/frontend/src/components/admin/BackupHistory.jsx @@ -236,7 +236,7 @@ export const BackupHistory = () => { {backup.manifest_path && (