fix(security): backup/restore hardening — public-dir DB dump, restore path allowlist, gunzip bound, manifest keying (stable) (#962)

* fix(security): stop caller-chosen database backup destination (GHSA-jw8m)

POST /api/admin/database-backup/backup forwarded req.body straight into
databaseBackupService.backup(), which merges options over config:
  const { destinationPath = '/backup/database', ... } = { ...config, ...options }

destinationPath is not a persistable setting — the /config allowlist only
accepts database_backup_* keys — so the request body was its only source.
The built-in `admin` role holds backup.create but neither settings.edit nor
backup.restore, so it could aim a full DB dump (admin bcrypt hashes, gallery
password hashes, encrypted SMTP creds) at the PUBLIC /uploads static mount
(server.js mounts it with no auth middleware) and then fetch it
unauthenticated. Filed low; it is a privilege escalation to unauthenticated
disclosure.

Forward only the real knobs, and only when present so absent keys can't
override config defaults via spread.

* fix(security): backup/restore hardening — restore path allowlist, gunzip bound, manifest checksum keying (GHSA-fw4c, h652, hgp8)

- adminRestore /validate + /start: constrain caller-supplied source and
  manifestPath to the operator-configured backup roots — the SAME set the
  restore wizard discovers from — so disaster recovery from a rescued mount
  still works, with RESTORE_ALLOWED_ROOTS as an escape hatch (GHSA-fw4c).
- restoreService.decompressFile: bound the EXPANDED size and abort the
  pipeline when exceeded; default 50 GB, RESTORE_MAX_DECOMPRESSED_BYTES
  overrides (GHSA-h652).
- backupManifest: BACKUP_MANIFEST_KEY upgrades new manifests to a keyed
  HMAC (GHSA-hgp8). Deliberately opt-in and verify-if-present — the key
  cannot live in the database because the database is inside the backup, so
  a mandatory HMAC would lock operators out of the exact disaster-recovery
  case this exists for.

Also fixes a pre-existing bug found while testing hgp8: the checksum passed
Object.keys().sort() as JSON.stringify's second argument, which is an array
REPLACER (a property allowlist applied at every depth), not a key sorter. All
nested keys — path, size, per-file checksum — were dropped before hashing, so
the file list sat outside the integrity check entirely and a manifest path
could be rewritten to ../../etc/passwd without disturbing the digest. Now
hashes a recursively-canonicalized copy, with the legacy serialization
accepted on validation so existing backups stay restorable.

* fix(security): codex round 2 — unbreak the restore wizard, share checksum verification, guard downgrades

- adminRestore: `source` is usually a SOURCE TYPE ('local'|'s3'|'upload'),
  not a path — restoreService branches on those literals. The containment
  check treated it as a path, so path.resolve('local') fell outside the
  backup roots and BOTH /validate and /start returned 400, blocking every
  normal restore. Type tokens are now excluded from the path check.
- backupManifest: extracted verifyManifestChecksum() as the single source of
  truth for the legacy/keyed fallbacks. restoreService.performPreRestoreValidation
  recomputed the digest itself with the default canonical+keyed settings,
  which rejected EVERY backup written before this batch. It now delegates.
- backupManifest: guard the algorithm downgrade — with a key configured, an
  attacker able to rewrite the backup store could strip checksum_algorithm,
  edit the manifest and recompute a plain SHA-256 that verified. Opt-in via
  BACKUP_MANIFEST_REQUIRE_KEYED so pre-key backups keep restoring by default.

* fix(security): codex round 3 — close two manifest-verification fail-opens (GHSA-hgp8)

verifyManifestChecksum returned valid for a manifest with no
verification.total_checksum at all, and restoreService only called it when
that field was present. Deleting the field was therefore a complete bypass of
the keying work: no digest check, no downgrade guard, no
BACKUP_MANIFEST_REQUIRE_KEYED. Every manifest this codebase writes stamps the
field, so an absent one now fails validation, and the call site invokes the
verifier unconditionally.

Second fail-open: the strict-mode rejection of an unkeyed manifest was gated on
`&& key`, so with BACKUP_MANIFEST_REQUIRE_KEYED=true and no BACKUP_MANIFEST_KEY
configured a plain SHA-256 manifest sailed through. Strict mode is a statement
about the operator's manifests, not about the host — it is exactly the fresh
disaster-recovery box that lacks the secret. The rejection no longer depends on
a key being present.

Claude-Session: https://claude.ai/code/session_01F211U4dDbEj4zXiyKbi9me
(cherry picked from commit 292a5b784acd7f47099aa234c1c2ea00050fca97)

---------

Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>

backend/__tests__/routes/databaseBackupDestination.test.js

@@ -0,0 +1,120 @@
+/**
+ * Manual database backup must not honour a caller-supplied destination
+ * (GHSA-jw8m-43r2-jqrm).
+ *
+ * POST /api/admin/database-backup/backup forwarded req.body straight into
+ * databaseBackupService.backup(), which merges options over its config:
+ *   const { destinationPath = '/backup/database', ... } = { ...config, ...options }
+ * `destinationPath` is not a persistable setting (the /config allowlist only
+ * accepts `database_backup_*` keys), so the request body was its ONLY source.
+ *
+ * The `admin` role holds backup.create but neither settings.edit nor
+ * backup.restore — so it could aim a full DB dump (bcrypt hashes, gallery
+ * password hashes, encrypted SMTP creds) at the PUBLIC /uploads static mount
+ * (server.js mounts it with no auth middleware) and fetch it unauthenticated.
+ *
+ * Pins that destinationPath from the body is ignored, while the legitimate
+ * knobs still pass through.
+ */
+
+const path = require('path');
+const fs = require('fs');
+const os = require('os');
+
+process.env.NODE_ENV = 'test';
+process.env.TEST_DATABASE_PATH = path.join(
+  fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-dbbackup-')), 'db.sqlite',
+);
+process.env.JWT_SECRET = process.env.JWT_SECRET || 'dbbackup-test-secret';
+
+// Capture what the route hands the service; never run a real backup.
+const mockBackup = jest.fn(async () => ({ success: true }));
+jest.mock('../../src/services/databaseBackup', () => ({
+  databaseBackupService: {
+    get isRunning() { return false; },
+    backup: (...args) => mockBackup(...args),
+  },
+}));
+
+const request = require('supertest');
+const express = require('express');
+const bcrypt = require('bcrypt');
+const jwt = require('jsonwebtoken');
+
+const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb');
+
+describe('manual database backup destination (GHSA-jw8m)', () => {
+  let db; let cleanup; let app; let adminToken;
+
+  beforeAll(async () => {
+    ({ db, cleanup } = await bootCrmDb());
+    await seedMinimal(db);
+
+    const role = await db('roles').where({ name: 'admin' }).first();
+    const r = await db('admin_users').insert({
+      username: 'limited-admin',
+      email: 'limited-admin@example.com',
+      password_hash: await bcrypt.hash('Passw0rd!', 4),
+      role_id: role.id,
+      is_active: 1,
+      created_at: new Date(),
+      updated_at: new Date(),
+    }).returning('id');
+    const id = r[0]?.id ?? r[0];
+    adminToken = jwt.sign(
+      { id, username: 'limited-admin', type: 'admin', role: 'admin', loginTime: Date.now() },
+      process.env.JWT_SECRET,
+      { expiresIn: '1h', issuer: 'picpeak-auth' },
+    );
+
+    app = express();
+    app.use(express.json());
+    app.use('/api/admin/database-backup', require('../../src/routes/adminDatabaseBackup'));
+  }, 120000);
+
+  afterAll(async () => { if (cleanup) await cleanup(); });
+
+  beforeEach(() => mockBackup.mockClear());
+
+  it('ignores a caller-supplied destinationPath', async () => {
+    const res = await request(app)
+      .post('/api/admin/database-backup/backup')
+      .set('Authorization', `Bearer ${adminToken}`)
+      .send({ destinationPath: '/app/storage/uploads' });
+
+    expect(res.status).toBe(200);
+    // Give the fire-and-forget call a tick to land.
+    await new Promise((resolve) => setImmediate(resolve));
+    expect(mockBackup).toHaveBeenCalled();
+    const opts = mockBackup.mock.calls[0][0];
+    expect(opts).not.toHaveProperty('destinationPath');
+    expect(JSON.stringify(opts)).not.toContain('uploads');
+  });
+
+  it('still forwards the legitimate backup knobs', async () => {
+    const res = await request(app)
+      .post('/api/admin/database-backup/backup')
+      .set('Authorization', `Bearer ${adminToken}`)
+      .send({ compress: false, validateIntegrity: false, destinationPath: '/tmp/evil' });
+
+    expect(res.status).toBe(200);
+    await new Promise((resolve) => setImmediate(resolve));
+    const opts = mockBackup.mock.calls[0][0];
+    expect(opts.compress).toBe(false);
+    expect(opts.validateIntegrity).toBe(false);
+    expect(opts).not.toHaveProperty('destinationPath');
+  });
+
+  it('omits absent knobs entirely so service/config defaults still apply', async () => {
+    const res = await request(app)
+      .post('/api/admin/database-backup/backup')
+      .set('Authorization', `Bearer ${adminToken}`)
+      .send({});
+
+    expect(res.status).toBe(200);
+    await new Promise((resolve) => setImmediate(resolve));
+    // An explicit `{compress: undefined}` would override config on spread —
+    // absent keys must simply not be present.
+    expect(mockBackup.mock.calls[0][0]).toEqual({});
+  });
+});

backend/__tests__/routes/restorePathAllowlist.test.js

@@ -0,0 +1,86 @@
+/**
+ * Restore path containment must not break the normal restore wizard
+ * (GHSA-fw4c, codex round 2).
+ *
+ * `source` is usually a SOURCE TYPE, not a path: RestoreWizard posts
+ * 'local' | 's3' | 'upload', and restoreService.restore() branches on those
+ * literals before deriving a directory. The first version of the containment
+ * check treated `source` as a path, so path.resolve('local') landed outside
+ * the configured backup roots and BOTH /validate and /start returned 400 —
+ * blocking every normal restore.
+ */
+
+const path = require('path');
+const fs = require('fs');
+const os = require('os');
+
+process.env.NODE_ENV = 'test';
+process.env.TEST_DATABASE_PATH = path.join(
+  fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-restorepath-')), 'db.sqlite',
+);
+process.env.JWT_SECRET = process.env.JWT_SECRET || 'restorepath-test-secret';
+
+const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb');
+
+describe('restore path allowlist (GHSA-fw4c)', () => {
+  let db; let cleanup; let checkRestorePathsAllowed;
+
+  beforeAll(async () => {
+    ({ db, cleanup } = await bootCrmDb());
+    await seedMinimal(db);
+
+    // Configure a backup root so the allowlist is actually active.
+    for (const [key, value] of [['backup_destination_path', '/backup']]) {
+      const existing = await db('app_settings').where({ setting_key: key }).first();
+      if (existing) {
+        await db('app_settings').where({ setting_key: key }).update({ setting_value: JSON.stringify(value) });
+      } else {
+        await db('app_settings').insert({
+          setting_key: key, setting_value: JSON.stringify(value), setting_type: 'backup',
+        });
+      }
+    }
+
+    ({ checkRestorePathsAllowed } = require('../../src/routes/adminRestore')._internal);
+  }, 120000);
+
+  afterAll(async () => { if (cleanup) await cleanup(); });
+
+  it('allows the wizard\'s source TYPE tokens', async () => {
+    for (const source of ['local', 's3', 'upload']) {
+      const err = await checkRestorePathsAllowed({
+        source, manifestPath: '/backup/manifests/backup-manifest-1.json',
+      });
+      expect(err).toBeNull();
+    }
+  });
+
+  it('allows an s3:// source URL', async () => {
+    const err = await checkRestorePathsAllowed({
+      source: 's3://bucket/key/backup.tar.gz',
+      manifestPath: '/backup/manifests/backup-manifest-1.json',
+    });
+    expect(err).toBeNull();
+  });
+
+  it('still rejects a manifestPath outside the configured roots', async () => {
+    const err = await checkRestorePathsAllowed({
+      source: 'local', manifestPath: '/etc/passwd',
+    });
+    expect(err).toMatch(/inside a configured backup location/i);
+  });
+
+  it('still rejects a traversal manifestPath', async () => {
+    const err = await checkRestorePathsAllowed({
+      source: 'local', manifestPath: '/backup/../etc/shadow',
+    });
+    expect(err).toBeTruthy();
+  });
+
+  it('accepts a real path source inside the roots', async () => {
+    const err = await checkRestorePathsAllowed({
+      source: '/backup/run-1', manifestPath: '/backup/run-1/manifest.json',
+    });
+    expect(err).toBeNull();
+  });
+});

backend/__tests__/services/backupRestoreHardening.test.js

@@ -0,0 +1,241 @@
+/**
+ * Backup/restore hardening — GHSA-h652 (unbounded gunzip) and GHSA-hgp8
+ * (unkeyed manifest checksum).
+ *
+ * h652: decompressFile() piped gunzip straight to disk with no expanded-size
+ * bound, so a small crafted .gz could fill the volume.
+ *
+ * hgp8: the manifest checksum is a plain SHA-256 — it proves the manifest was
+ * not corrupted, not that it is authentic. BACKUP_MANIFEST_KEY upgrades new
+ * manifests to a keyed HMAC. It is deliberately OPT-IN and verify-if-present:
+ * the key cannot live in the database (the database is inside the backup), so
+ * a mandatory HMAC would lock an operator out of the exact disaster-recovery
+ * case this system exists for.
+ */
+
+const path = require('path');
+const fs = require('fs');
+const os = require('os');
+const zlib = require('zlib');
+
+process.env.NODE_ENV = 'test';
+process.env.TEST_DATABASE_PATH = path.join(
+  fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-bkharden-')), 'db.sqlite',
+);
+process.env.JWT_SECRET = process.env.JWT_SECRET || 'bkharden-test-secret';
+
+const { restoreService } = require('../../src/services/restoreService');
+const backupManifest = require('../../src/services/backupManifest');
+
+describe('decompressFile expanded-size bound (GHSA-h652)', () => {
+  let dir;
+
+  beforeAll(() => { dir = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-gz-')); });
+  afterAll(() => { fs.rmSync(dir, { recursive: true, force: true }); });
+  afterEach(() => { delete process.env.RESTORE_MAX_DECOMPRESSED_BYTES; });
+
+  it('aborts when the decompressed stream exceeds the limit', async () => {
+    // 5 MB of zeroes compresses to a few KB — the classic shape of the attack.
+    const gzPath = path.join(dir, 'bomb.gz');
+    fs.writeFileSync(gzPath, zlib.gzipSync(Buffer.alloc(5 * 1024 * 1024, 0)));
+
+    process.env.RESTORE_MAX_DECOMPRESSED_BYTES = String(64 * 1024); // 64 KB
+    await expect(
+      restoreService.decompressFile(gzPath, path.join(dir, 'out-bomb'))
+    ).rejects.toThrow(/exceeds limit/i);
+  });
+
+  it('still decompresses a normal file within the limit', async () => {
+    const payload = Buffer.from('SELECT 1;\n'.repeat(100));
+    const gzPath = path.join(dir, 'ok.gz');
+    fs.writeFileSync(gzPath, zlib.gzipSync(payload));
+
+    const outPath = path.join(dir, 'out-ok');
+    await restoreService.decompressFile(gzPath, outPath);
+    expect(fs.readFileSync(outPath)).toEqual(payload);
+  });
+});
+
+describe('manifest checksum keying (GHSA-hgp8)', () => {
+  // validateManifest requires all of these sections to be present.
+  const baseManifest = () => ({
+    manifest: { version: '1.0', id: 'test' },
+    backup: { type: 'full' },
+    system: { platform: 'linux' },
+    application: { version: '1.0.0' },
+    files: { count: 1, manifest: [{ path: 'a.jpg', size: 1 }] },
+    database: { type: 'sqlite' },
+    verification: { total_checksum: null, checksum_algorithm: null },
+  });
+
+  afterEach(() => { delete process.env.BACKUP_MANIFEST_KEY; });
+
+  it('produces a different digest when a key is set', () => {
+    const m = baseManifest();
+    const unkeyed = backupManifest.calculateManifestChecksum(m, { keyed: false });
+    const keyed = backupManifest.calculateManifestChecksum(m, { keyed: 'secret-key' });
+    expect(keyed).not.toBe(unkeyed);
+  });
+
+  it('validates a legacy unkeyed manifest even when a key IS configured', () => {
+    // Disaster recovery: manifests written before keying must not become
+    // un-restorable the moment the operator sets a key.
+    const m = baseManifest();
+    m.verification.checksum_algorithm = 'sha256';
+    m.verification.total_checksum = backupManifest.calculateManifestChecksum(m, { keyed: false });
+
+    process.env.BACKUP_MANIFEST_KEY = 'secret-key';
+    expect(() => backupManifest.validateManifest(m)).not.toThrow();
+  });
+
+  it('accepts a keyed manifest when the matching key is configured', () => {
+    process.env.BACKUP_MANIFEST_KEY = 'secret-key';
+    const m = baseManifest();
+    m.verification.checksum_algorithm = 'hmac-sha256';
+    m.verification.total_checksum = backupManifest.calculateManifestChecksum(m, { keyed: 'secret-key' });
+
+    expect(() => backupManifest.validateManifest(m)).not.toThrow();
+  });
+
+  it('rejects a keyed manifest whose body was tampered with', () => {
+    process.env.BACKUP_MANIFEST_KEY = 'secret-key';
+    const m = baseManifest();
+    m.verification.checksum_algorithm = 'hmac-sha256';
+    m.verification.total_checksum = backupManifest.calculateManifestChecksum(m, { keyed: 'secret-key' });
+
+    m.files.manifest[0].path = '../../etc/passwd';
+    expect(() => backupManifest.validateManifest(m)).toThrow(/checksum verification failed/i);
+  });
+
+  it('does NOT brick restore when a keyed manifest meets a missing key', () => {
+    // Key lost with the host — the precise moment a restore is needed.
+    const m = baseManifest();
+    m.verification.checksum_algorithm = 'hmac-sha256';
+    m.verification.total_checksum = backupManifest.calculateManifestChecksum(m, { keyed: 'secret-key' });
+
+    delete process.env.BACKUP_MANIFEST_KEY;
+    expect(() => backupManifest.validateManifest(m)).not.toThrow();
+  });
+});
+
+describe('manifest checksum coverage (canonicalization)', () => {
+  const fullManifest = () => ({
+    manifest: { version: '1.0', id: 'test' },
+    backup: { type: 'full' },
+    system: { platform: 'linux' },
+    application: { version: '1.0.0' },
+    files: { count: 1, manifest: [{ path: 'a.jpg', size: 1 }] },
+    database: { type: 'sqlite' },
+    verification: { total_checksum: null, checksum_algorithm: 'sha256' },
+  });
+
+  afterEach(() => { delete process.env.BACKUP_MANIFEST_KEY; });
+
+  it('covers nested file entries (the old replacer dropped them)', () => {
+    const m = fullManifest();
+    m.verification.total_checksum = backupManifest.calculateManifestChecksum(m, { keyed: false });
+    // Tampering a file path must now change the digest.
+    m.files.manifest[0].path = '../../etc/passwd';
+    expect(() => backupManifest.validateManifest(m)).toThrow(/checksum verification failed/i);
+  });
+
+  it('still accepts a manifest written with the legacy serialization', () => {
+    const m = fullManifest();
+    m.verification.total_checksum = backupManifest.calculateManifestChecksum(
+      m, { keyed: false, legacy: true }
+    );
+    expect(() => backupManifest.validateManifest(m)).not.toThrow();
+  });
+});
+
+describe('checksum verification is shared and downgrade-aware (codex round 2)', () => {
+  const fullManifest = () => ({
+    manifest: { version: '1.0', id: 'test' },
+    backup: { type: 'full' },
+    system: { platform: 'linux' },
+    application: { version: '1.0.0' },
+    files: { count: 1, manifest: [{ path: 'a.jpg', size: 1 }] },
+    database: { type: 'sqlite' },
+    verification: { total_checksum: null, checksum_algorithm: 'sha256' },
+  });
+
+  afterEach(() => {
+    delete process.env.BACKUP_MANIFEST_KEY;
+    delete process.env.BACKUP_MANIFEST_REQUIRE_KEYED;
+  });
+
+  it('accepts a legacy-serialized manifest through the SHARED verifier', () => {
+    // restoreService recomputed the digest itself with the canonical
+    // serializer, which rejected every pre-existing backup.
+    const m = fullManifest();
+    m.verification.total_checksum = backupManifest.calculateManifestChecksum(
+      m, { keyed: false, legacy: true },
+    );
+    const res = backupManifest.verifyManifestChecksum(m);
+    expect(res.valid).toBe(true);
+    expect(res.warnings.join(' ')).toMatch(/legacy checksum serialization/i);
+  });
+
+  it('warns but accepts an unkeyed manifest when a key is configured', () => {
+    const m = fullManifest();
+    m.verification.total_checksum = backupManifest.calculateManifestChecksum(m, { keyed: false });
+    process.env.BACKUP_MANIFEST_KEY = 'secret-key';
+
+    const res = backupManifest.verifyManifestChecksum(m);
+    expect(res.valid).toBe(true);
+    expect(res.warnings.join(' ')).toMatch(/authenticity NOT established/i);
+  });
+
+  it('REJECTS the algorithm downgrade once REQUIRE_KEYED is on', () => {
+    // Attacker rewrites the manifest, strips checksum_algorithm and recomputes
+    // a plain SHA-256. With the strict flag set that must not verify.
+    const m = fullManifest();
+    m.files.manifest[0].path = '../../etc/passwd';
+    m.verification.total_checksum = backupManifest.calculateManifestChecksum(m, { keyed: false });
+
+    process.env.BACKUP_MANIFEST_KEY = 'secret-key';
+    process.env.BACKUP_MANIFEST_REQUIRE_KEYED = 'true';
+
+    const res = backupManifest.verifyManifestChecksum(m);
+    expect(res.valid).toBe(false);
+    expect(res.error).toMatch(/downgrade/i);
+  });
+
+  it('rejects a keyed manifest with no key when REQUIRE_KEYED is on', () => {
+    const m = fullManifest();
+    m.verification.checksum_algorithm = 'hmac-sha256';
+    m.verification.total_checksum = backupManifest.calculateManifestChecksum(m, { keyed: 'k' });
+    process.env.BACKUP_MANIFEST_REQUIRE_KEYED = 'true';
+
+    expect(backupManifest.verifyManifestChecksum(m).valid).toBe(false);
+  });
+
+  it('REJECTS a manifest whose checksum was stripped entirely', () => {
+    // The cheapest bypass of every rule above: delete the field instead of
+    // forging it. Both the helper's early return and restoreService's
+    // `if (…total_checksum)` guard used to wave that through.
+    const m = fullManifest();
+    delete m.verification.total_checksum;
+
+    const res = backupManifest.verifyManifestChecksum(m);
+    expect(res.valid).toBe(false);
+    expect(res.error).toMatch(/no checksum/i);
+
+    delete m.verification;
+    expect(backupManifest.verifyManifestChecksum(m).valid).toBe(false);
+  });
+
+  it('REJECTS an unkeyed manifest under REQUIRE_KEYED even with no key configured', () => {
+    // Strict mode is a claim about the manifests, not about this host — so a
+    // fresh disaster-recovery box that lost BACKUP_MANIFEST_KEY must not
+    // silently start accepting plain SHA-256 manifests again.
+    const m = fullManifest();
+    m.verification.total_checksum = backupManifest.calculateManifestChecksum(m, { keyed: false });
+    process.env.BACKUP_MANIFEST_REQUIRE_KEYED = 'true';
+    delete process.env.BACKUP_MANIFEST_KEY;
+
+    const res = backupManifest.verifyManifestChecksum(m);
+    expect(res.valid).toBe(false);
+    expect(res.error).toMatch(/downgrade/i);
+  });
+});

backend/src/routes/adminDatabaseBackup.js

@@ -123,8 +123,20 @@ router.post('/backup', requirePermission('backup.create'), async (req, res) => {
       trackingUrl: '/api/admin/database-backup/progress'
     });
     
-    // Run backup in background
-    databaseBackupService.backup(req.body).catch(error => {
+    // Forward ONLY the real backup knobs (GHSA-jw8m). Passing req.body
+    // straight through let the caller set `destinationPath`, which the
+    // service merges over its config — so a backup.create holder (the
+    // `admin` role, which has neither settings.edit nor backup.restore)
+    // could dump the whole database into the PUBLIC /uploads static mount
+    // and fetch it unauthenticated, hashes and encrypted SMTP creds included.
+    // destinationPath is not a persistable setting; the request body was its
+    // only source, so dropping it here costs no legitimate behaviour.
+    const body = req.body || {};
+    const options = {};
+    for (const key of ['compress', 'validateIntegrity', 'includeChecksums']) {
+      if (body[key] !== undefined) options[key] = body[key];
+    }
+    databaseBackupService.backup(options).catch(error => {
       logger.error('Manual database backup failed:', error);
     });
   } catch (error) {

backend/src/routes/adminRestore.js

@@ -91,6 +91,12 @@ router.post('/validate', requirePermission('backup.restore'), [
   }
 
   try {
+    // Constrain the caller-supplied paths to configured backup roots (GHSA-fw4c)
+    const pathError = await checkRestorePathsAllowed(req.body);
+    if (pathError) {
+      return res.status(400).json({ success: false, error: pathError });
+    }
+
     // Transform S3 config from frontend format
     const s3Config = transformS3Config(req.body);
 
@@ -165,6 +171,12 @@ router.post('/start', requirePermission('backup.restore'), [
       });
     }
 
+    // Constrain the caller-supplied paths to configured backup roots (GHSA-fw4c)
+    const pathError = await checkRestorePathsAllowed(req.body);
+    if (pathError) {
+      return res.status(400).json({ success: false, error: pathError });
+    }
+
     // Check permissions for dangerous options
     const settings = await getRestoreSettings();
     if (req.body.force && !settings.restore_allow_force) {
@@ -759,4 +771,67 @@ async function getBackupConfig() {
   return config;
 }
 
+/**
+ * GHSA-fw4c: `source` and `manifestPath` were validated only as "not empty"
+ * before being handed to the privileged restore engine, which reads them,
+ * parses the manifest and executes the referenced SQL against the live
+ * database. Constrain them to the operator-configured backup locations.
+ *
+ * The allowlist is the SAME set the restore wizard discovers from
+ * (`backup_destination_path` + `backup_manifest_path`), so the disaster-
+ * recovery flow is untouched: an operator restoring from a rescued mount
+ * already has to point those settings at it for the backup to be listed.
+ * RESTORE_ALLOWED_ROOTS (colon-separated) is an escape hatch for unusual
+ * layouts. S3 sources are URLs, not paths, and are validated elsewhere.
+ *
+ * @returns {Promise<string|null>} an error message, or null when acceptable
+ */
+// `source` is usually a SOURCE TYPE, not a path: the restore wizard posts
+// 'local' | 's3' | 'upload' and restoreService.restore() branches on those
+// literals before deriving an actual directory (see its comment at the
+// `options.source === 'local'` branch). Treating them as paths resolved
+// 'local' to <cwd>/local, failed containment, and 400'd the entire normal
+// restore workflow — so type tokens are excluded from the path check.
+const SOURCE_TYPE_TOKENS = ['local', 's3', 'upload'];
+
+async function checkRestorePathsAllowed({ source, manifestPath }) {
+  const isS3 = (v) => typeof v === 'string' && v.startsWith('s3://');
+  const isTypeToken = (v) => typeof v === 'string'
+    && SOURCE_TYPE_TOKENS.includes(v.trim().toLowerCase());
+  const candidates = [source, manifestPath]
+    .filter((v) => v && !isS3(v) && !isTypeToken(v));
+  if (candidates.length === 0) return null;
+
+  const config = await getBackupConfig();
+  const roots = [];
+  if (config.backup_destination_path) roots.push(config.backup_destination_path);
+  if (config.backup_manifest_path) roots.push(config.backup_manifest_path);
+  for (const extra of (process.env.RESTORE_ALLOWED_ROOTS || '').split(':')) {
+    if (extra.trim()) roots.push(extra.trim());
+  }
+  if (roots.length === 0) {
+    // Nothing configured to compare against — a restore can't be scoped, so
+    // don't pretend to enforce. Discovery would find nothing either.
+    return null;
+  }
+
+  const resolvedRoots = roots.map((r) => path.resolve(r));
+  for (const candidate of candidates) {
+    const resolved = path.resolve(candidate);
+    const inside = resolvedRoots.some(
+      (root) => resolved === root || resolved.startsWith(root + path.sep)
+    );
+    if (!inside) {
+      logger.warn('Refusing restore path outside the configured backup roots', {
+        candidate, roots,
+      });
+      return 'Backup source and manifest path must be inside a configured backup location';
+    }
+  }
+  return null;
+}
+
 module.exports = router;
+// Exposed for tests: the source/manifestPath containment rules (GHSA-fw4c) are
+// worth pinning directly, especially the source-TYPE-token carve-out.
+module.exports._internal = { checkRestorePathsAllowed, SOURCE_TYPE_TOKENS };

backend/src/services/backupManifest.js

@@ -134,7 +134,10 @@ class BackupManifestGenerator {
       }
     };
 
-    // Calculate total checksum of the manifest
+    // Calculate total checksum of the manifest. Records WHICH algorithm was
+    // used so validation can tell a keyed manifest from a legacy unkeyed one
+    // (GHSA-hgp8).
+    manifest.verification.checksum_algorithm = this.getManifestKey() ? 'hmac-sha256' : 'sha256';
     manifest.verification.total_checksum = this.calculateManifestChecksum(manifest);
 
     return manifest;
@@ -227,11 +230,15 @@ class BackupManifestGenerator {
       throw new Error('File count mismatch');
     }
 
-    // Validate total checksum
-    const calculatedChecksum = this.calculateManifestChecksum(manifest);
-    if (manifest.verification.total_checksum !== calculatedChecksum) {
-      throw new Error('Manifest checksum verification failed');
+    // Validate total checksum (GHSA-hgp8) — delegated so every caller shares
+    // the same fallback rules. restoreService.performPreRestoreValidation()
+    // used to recompute the digest itself with the default (canonical, keyed)
+    // settings, which silently rejected every pre-existing backup.
+    const checksumResult = this.verifyManifestChecksum(manifest);
+    if (!checksumResult.valid) {
+      throw new Error(checksumResult.error || 'Manifest checksum verification failed');
     }
+    checksumResult.warnings.forEach((w) => logger.warn(w));
 
     logger.info('Manifest validation passed');
     return true;
@@ -327,6 +334,7 @@ class BackupManifestGenerator {
     // otherwise validateManifest() rejects the loaded manifest because
     // generateManifest() stamped a checksum that did NOT include this
     // section.
+    fullManifest.verification.checksum_algorithm = this.getManifestKey() ? 'hmac-sha256' : 'sha256';
     fullManifest.verification.total_checksum = this.calculateManifestChecksum(fullManifest);
 
     return fullManifest;
@@ -439,16 +447,167 @@ class BackupManifestGenerator {
     }
   }
 
-  calculateManifestChecksum(manifest) {
+  /**
+   * GHSA-hgp8: the plain SHA-256 below proves the manifest wasn't CORRUPTED,
+   * not that it is AUTHENTIC — anyone who can rewrite the file can recompute
+   * it. Setting BACKUP_MANIFEST_KEY upgrades new manifests to a keyed HMAC,
+   * which matters when the backup store is a different trust domain from the
+   * host (S3 bucket creds != host creds).
+   *
+   * Deliberately OPT-IN and verify-if-present: the key cannot live in the
+   * database (the database is inside the backup), so a mandatory HMAC would
+   * lock an operator out of the exact disaster-recovery case this system
+   * exists for — total host loss, fresh install, only the backup survives.
+   * Unkeyed manifests therefore still validate, and a keyed manifest is only
+   * held to the keyed check when a key is configured.
+   */
+  getManifestKey() {
+    const key = process.env.BACKUP_MANIFEST_KEY;
+    return typeof key === 'string' && key.trim() ? key.trim() : null;
+  }
+
+  /**
+   * Single source of truth for "does this manifest's checksum verify?"
+   * (GHSA-hgp8). Returns a result object rather than throwing so callers can
+   * surface warnings without duplicating the fallback rules — a duplicated
+   * check in restoreService recomputed the digest with the default canonical
+   * serializer and rejected every manifest written before that change.
+   *
+   * Rules, in order:
+   *   - keyed manifest + no key configured  → cannot verify; accept with a
+   *     loud warning (refusing would brick recovery when the key was lost with
+   *     the host, which is exactly when a restore is needed), UNLESS
+   *     BACKUP_MANIFEST_REQUIRE_KEYED is set.
+   *   - unkeyed manifest + key configured   → possible downgrade. Accepted with
+   *     a warning by default for backward compatibility; rejected when
+   *     BACKUP_MANIFEST_REQUIRE_KEYED is set, which is the setting an operator
+   *     turns on once all their backups are keyed.
+   *   - digest mismatch → retry with the legacy (pre-canonicalization)
+   *     serialization so old backups stay restorable, then fail.
+   *   - no checksum at all → reject. Every manifest this codebase has ever
+   *     written stamps `verification.total_checksum` (generateManifest and
+   *     the incremental path both do), so an absent one means the manifest
+   *     was rewritten — and accepting it would let an attacker strip the
+   *     field to skip verification entirely, walking straight past both the
+   *     downgrade guard and BACKUP_MANIFEST_REQUIRE_KEYED.
+   *
+   * @returns {{valid: boolean, error?: string, warnings: string[]}}
+   */
+  verifyManifestChecksum(manifest) {
+    const warnings = [];
+    if (!manifest?.verification?.total_checksum) {
+      return {
+        valid: false,
+        error: 'Manifest carries no checksum — refusing to treat an unverifiable manifest as authentic',
+        warnings,
+      };
+    }
+
+    const declaredAlgorithm = manifest.verification.checksum_algorithm || 'sha256';
+    const key = this.getManifestKey();
+    const requireKeyed = /^(1|true|yes)$/i.test(String(process.env.BACKUP_MANIFEST_REQUIRE_KEYED || ''));
+
+    if (declaredAlgorithm === 'hmac-sha256' && !key) {
+      if (requireKeyed) {
+        return {
+          valid: false,
+          error: 'Manifest is keyed but BACKUP_MANIFEST_KEY is not set (BACKUP_MANIFEST_REQUIRE_KEYED is on)',
+          warnings,
+        };
+      }
+      warnings.push(
+        'Manifest declares a keyed checksum but BACKUP_MANIFEST_KEY is not set — '
+        + 'authenticity cannot be verified. Set the key to enable verification.'
+      );
+      return { valid: true, warnings };
+    }
+
+    // Downgrade guard: with a key configured, an attacker who can rewrite the
+    // backup store could otherwise strip checksum_algorithm, edit the manifest
+    // and recompute a plain SHA-256 that we would happily accept. Rejecting
+    // that by default would break every pre-key backup, so it is opt-in.
+    //
+    // The strict rejection must NOT be conditional on a key being configured:
+    // strict mode is a statement about the manifests ("all mine are keyed"),
+    // not about this host. Gating it on `key` made the flag fail open on
+    // exactly the fresh disaster-recovery host that is missing the secret.
+    if (declaredAlgorithm !== 'hmac-sha256') {
+      if (requireKeyed) {
+        return {
+          valid: false,
+          error: 'Manifest is not keyed but BACKUP_MANIFEST_REQUIRE_KEYED is on — refusing a possible checksum downgrade',
+          warnings,
+        };
+      }
+      if (key) {
+        warnings.push(
+          'Manifest uses an unkeyed checksum while BACKUP_MANIFEST_KEY is set — integrity verified, '
+          + 'authenticity NOT established (a rewritten manifest could have downgraded the algorithm). '
+          + 'Set BACKUP_MANIFEST_REQUIRE_KEYED=true once all backups are keyed.'
+        );
+      }
+    }
+
+    const keyedArg = declaredAlgorithm === 'hmac-sha256' ? key : false;
+    const expected = manifest.verification.total_checksum;
+
+    if (expected === this.calculateManifestChecksum(manifest, { keyed: keyedArg })) {
+      return { valid: true, warnings };
+    }
+    // Pre-canonicalization manifests hashed a serialization that omitted
+    // nested fields; accept those so existing backups stay restorable.
+    if (expected === this.calculateManifestChecksum(manifest, { keyed: keyedArg, legacy: true })) {
+      warnings.push(
+        'Manifest uses the legacy checksum serialization, which did not cover the file list — '
+        + 'integrity of file paths/sizes is unverified. Re-run a backup to upgrade it.'
+      );
+      return { valid: true, warnings };
+    }
+    return { valid: false, error: 'Manifest checksum verification failed', warnings };
+  }
+
+  /**
+   * Canonical JSON: object keys sorted recursively so the digest is stable
+   * regardless of property insertion order, and — critically — so NESTED
+   * values are actually covered.
+   *
+   * The previous implementation passed `Object.keys(manifest).sort()` as
+   * JSON.stringify's second argument. That parameter is an array *replacer*
+   * (a property allowlist applied at every depth), not a key sorter, so every
+   * nested key absent from that top-level list — `path`, `size`, per-file
+   * `checksum` — was dropped before hashing. The file list was therefore
+   * outside the "integrity" check entirely: a manifest path could be rewritten
+   * to `../../etc/passwd` without disturbing the checksum.
+   */
+  canonicalize(value) {
+    if (Array.isArray(value)) return value.map((v) => this.canonicalize(v));
+    if (value && typeof value === 'object') {
+      return Object.keys(value).sort().reduce((acc, k) => {
+        acc[k] = this.canonicalize(value[k]);
+        return acc;
+      }, {});
+    }
+    return value;
+  }
+
+  calculateManifestChecksum(manifest, { keyed = null, legacy = false } = {}) {
     // Create a copy without the checksum field
     const manifestCopy = JSON.parse(JSON.stringify(manifest));
     if (manifestCopy.verification) {
       delete manifestCopy.verification.total_checksum;
+      delete manifestCopy.verification.checksum_algorithm;
     }
 
-    // Calculate SHA256 of the sorted JSON
-    const content = JSON.stringify(manifestCopy, Object.keys(manifestCopy).sort());
-    return crypto.createHash('sha256').update(content).digest('hex');
+    // `legacy` reproduces the old (under-covering) serialization so manifests
+    // written by earlier versions still validate — see validateManifest.
+    const content = legacy
+      ? JSON.stringify(manifestCopy, Object.keys(manifestCopy).sort())
+      : JSON.stringify(this.canonicalize(manifestCopy));
+
+    const key = keyed === null ? this.getManifestKey() : keyed;
+    return key
+      ? crypto.createHmac('sha256', key).update(content).digest('hex')
+      : crypto.createHash('sha256').update(content).digest('hex');
   }
 
   /**

backend/src/services/restoreService.js

@@ -3,6 +3,7 @@ const path = require('path');
 const crypto = require('crypto');
 const zlib = require('zlib');
 const { pipeline } = require('stream/promises');
+const { Transform } = require('stream');
 const { createReadStream, createWriteStream } = require('fs');
 const { spawnAsync, spawnToFile, spawnFromFile } = require('../utils/safeExec');
 const { db } = require('../database/db');
@@ -519,13 +520,21 @@ class RestoreService {
     };
 
     try {
-      // Check backup integrity
-      if (manifest.verification && manifest.verification.total_checksum) {
-        const calculatedChecksum = backupManifest.calculateManifestChecksum(manifest);
-        if (calculatedChecksum !== manifest.verification.total_checksum) {
-          validation.errors.push('Manifest checksum verification failed');
-          validation.isValid = false;
-        }
+      // Check backup integrity. MUST delegate to verifyManifestChecksum rather
+      // than recomputing here — that helper owns the legacy-serialization and
+      // keyed/unkeyed fallbacks (GHSA-hgp8). Recomputing with the default
+      // canonical+keyed settings rejected every backup written before those
+      // changes, i.e. every existing one.
+      //
+      // Called UNCONDITIONALLY: the old `if (…total_checksum)` guard meant an
+      // attacker who could rewrite the backup store simply deleted the field
+      // to skip verification altogether. The helper owns that case now and
+      // rejects it.
+      const checksumResult = backupManifest.verifyManifestChecksum(manifest);
+      checksumResult.warnings.forEach((w) => this.log('warn', w));
+      if (!checksumResult.valid) {
+        validation.errors.push(checksumResult.error || 'Manifest checksum verification failed');
+        validation.isValid = false;
       }
 
       // Check backup age
@@ -1512,10 +1521,33 @@ END $$;`
    * Decompress gzip file
    */
   async decompressFile(inputPath, outputPath) {
+    // Bound the EXPANDED size (GHSA-h652). gunzip happily inflates a small
+    // crafted .gz into an unbounded stream, filling the disk before any later
+    // validation runs. Cap it and fail the pipeline the moment the limit is
+    // crossed. The default is deliberately generous — real database dumps are
+    // large — and overridable for installs with genuinely bigger data.
+    const configured = Number(process.env.RESTORE_MAX_DECOMPRESSED_BYTES);
+    const maxBytes = Number.isFinite(configured) && configured > 0
+      ? configured
+      : 50 * 1024 * 1024 * 1024; // 50 GB
+
+    let written = 0;
+    const limiter = new Transform({
+      transform(chunk, _enc, cb) {
+        written += chunk.length;
+        if (written > maxBytes) {
+          return cb(new Error(
+            `Decompressed size exceeds limit of ${maxBytes} bytes — refusing to continue`
+          ));
+        }
+        cb(null, chunk);
+      },
+    });
+
     const gunzip = zlib.createGunzip();
     const source = createReadStream(inputPath);
     const destination = createWriteStream(outputPath);
-    await pipeline(source, gunzip, destination);
+    await pipeline(source, gunzip, limiter, destination);
   }
 
   /**