fix(backup): reject a database-backup destination inside a public static mount

Making database_backup_destination_path actually take effect
reopens a GHSA-jw8m-43r2-jqrm-class exfiltration path: that
setting is writable via PUT /api/admin/database-backup/config
under backup.create alone (the built-in admin role has it
without settings.edit or backup.restore), with no path
validation. Before this fix the setting was silently ignored
(the destructuring bug), so pointing it at the public
uploads/logos or fonts mount was harmless; now that it is
honored, it needed the same defense GHSA-jw8m already applies
to the per-request override.

Rejects the setting at both the config write (immediate 400)
and, defensively, at backup() time before mkdir.

Found by codex review.

backend/__tests__/routes/databaseBackupConfigDestination.test.js

@@ -0,0 +1,98 @@
+/**
+ * PUT /api/admin/database-backup/config must reject a
+ * database_backup_destination_path that resolves inside a publicly served
+ * directory (GHSA-jw8m-43r2-jqrm class, #1365).
+ *
+ * Before #1365, database_backup_destination_path was silently ignored by
+ * databaseBackupService.backup() (a destructuring bug always fell back to
+ * the hardcoded /backup/database), so this setting being freely writable by
+ * any backup.create holder — the built-in `admin` role has it without
+ * settings.edit or backup.restore — was harmless. Making the setting
+ * actually take effect reopens the exact exfiltration path GHSA-jw8m fixed
+ * for the per-request override, through the persisted setting instead.
+ */
+
+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-config-')), 'db.sqlite',
+);
+process.env.JWT_SECRET = process.env.JWT_SECRET || 'dbbackup-config-test-secret';
+process.env.STORAGE_PATH = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-dbbackup-storage-'));
+
+const request = require('supertest');
+const express = require('express');
+const bcrypt = require('bcrypt');
+const jwt = require('jsonwebtoken');
+
+const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb');
+
+describe('database backup destination-path config guard (GHSA-jw8m class, #1365)', () => {
+  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: '[email protected]',
+      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(); });
+
+  it('rejects a destination inside the public uploads/logos mount', async () => {
+    const res = await request(app)
+      .put('/api/admin/database-backup/config')
+      .set('Authorization', `Bearer ${adminToken}`)
+      .send({ database_backup_destination_path: path.join(process.env.STORAGE_PATH, 'uploads', 'logos') });
+
+    expect(res.status).toBe(400);
+
+    // The seeded default must survive untouched — the rejected value never lands.
+    const row = await db('app_settings').where({ setting_key: 'database_backup_destination_path' }).first();
+    expect(JSON.parse(row.setting_value)).toBe('/backup/database');
+  });
+
+  it('rejects a destination inside the public fonts mount', async () => {
+    const res = await request(app)
+      .put('/api/admin/database-backup/config')
+      .set('Authorization', `Bearer ${adminToken}`)
+      .send({ database_backup_destination_path: path.join(process.env.STORAGE_PATH, 'fonts') });
+
+    expect(res.status).toBe(400);
+  });
+
+  it('accepts a destination outside any public mount', async () => {
+    const safePath = path.join(process.env.STORAGE_PATH, 'db-backups');
+    const res = await request(app)
+      .put('/api/admin/database-backup/config')
+      .set('Authorization', `Bearer ${adminToken}`)
+      .send({ database_backup_destination_path: safePath });
+
+    expect(res.status).toBe(200);
+
+    const row = await db('app_settings').where({ setting_key: 'database_backup_destination_path' }).first();
+    expect(JSON.parse(row.setting_value)).toBe(safePath);
+  });
+});

backend/src/routes/adminDatabaseBackup.js

@@ -2,7 +2,7 @@ const express = require('express');
 const router = express.Router();
 const { adminAuth } = require('../middleware/auth');
 const { requirePermission } = require('../middleware/permissions');
-const { databaseBackupService } = require('../services/databaseBackup');
+const { databaseBackupService, isUnderPubliclyServableRoot } = require('../services/databaseBackup');
 const { db } = require('../database/db');
 const logger = require('../utils/logger');
 const { getPagination } = require('../utils/routeHelpers');
@@ -60,7 +60,18 @@ router.put('/config', requirePermission('backup.create'), async (req, res) => {
       'database_backup_email_on_failure',
       'database_backup_email_on_success'
     ];
-    
+
+    // A backup.create holder (the built-in `admin` role has it without
+    // settings.edit or backup.restore) could otherwise point backups at a
+    // public static mount and fetch the dump unauthenticated — see
+    // isUnderPubliclyServableRoot's comment (GHSA-jw8m-43r2-jqrm class).
+    if (
+      typeof req.body.database_backup_destination_path === 'string'
+      && isUnderPubliclyServableRoot(req.body.database_backup_destination_path)
+    ) {
+      return res.status(400).json({ error: 'Destination path must not be inside a publicly served directory' });
+    }
+
     const updates = [];
     
     for (const [key, value] of Object.entries(req.body)) {

backend/src/services/__tests__/databaseBackup.test.js

@@ -1,5 +1,6 @@
 const { db } = require('../../database/db');
 const fs = require('fs').promises;
+const path = require('path');
 const crypto = require('crypto');
 
 // Mock dependencies
@@ -9,7 +10,7 @@ jest.mock('../emailProcessor');
 jest.mock('child_process');
 jest.mock('node-cron', () => ({ schedule: jest.fn(() => ({ stop: jest.fn() })) }));
 
-const { DatabaseBackupService, startScheduledBackups } = require('../databaseBackup');
+const { DatabaseBackupService, startScheduledBackups, isUnderPubliclyServableRoot } = require('../databaseBackup');
 const cron = require('node-cron');
 
 describe('DatabaseBackupService', () => {
@@ -231,6 +232,58 @@ describe('DatabaseBackupService', () => {
     });
   });
 
+  describe('isUnderPubliclyServableRoot (GHSA-jw8m class, #1365)', () => {
+    const originalStoragePath = process.env.STORAGE_PATH;
+    const storage = '/tmp/picpeak-test-storage';
+
+    beforeEach(() => {
+      process.env.STORAGE_PATH = storage;
+    });
+
+    afterAll(() => {
+      if (originalStoragePath === undefined) {
+        delete process.env.STORAGE_PATH;
+      } else {
+        process.env.STORAGE_PATH = originalStoragePath;
+      }
+    });
+
+    it.each([
+      path.join(storage, 'uploads', 'logos'),
+      path.join(storage, 'uploads', 'logos', 'sub'),
+      path.join(storage, 'uploads', 'favicons'),
+      path.join(storage, 'fonts'),
+      path.join(storage, 'fonts', 'inter')
+    ])('flags %s as publicly servable', (candidate) => {
+      expect(isUnderPubliclyServableRoot(candidate)).toBe(true);
+    });
+
+    it.each([
+      path.join(storage, 'backups'),
+      path.join(storage, 'uploads', 'contracts', 'signed'),
+      path.join(storage, 'uploads', 'transfers', '123'),
+      '/data/db-backups'
+    ])('does not flag %s', (candidate) => {
+      expect(isUnderPubliclyServableRoot(candidate)).toBe(false);
+    });
+
+    it('backup() refuses a destination inside a publicly servable root without ever calling mkdir', async () => {
+      const publicPath = path.join(storage, 'uploads', 'logos');
+      db.mockReturnValue({
+        where: jest.fn().mockReturnThis(),
+        select: jest.fn().mockResolvedValue([
+          { setting_key: 'database_backup_destination_path', setting_value: JSON.stringify(publicPath) }
+        ])
+      });
+
+      const mkdirSpy = jest.spyOn(fs, 'mkdir');
+
+      await expect(service.backup({})).rejects.toThrow('publicly served directory');
+
+      expect(mkdirSpy).not.toHaveBeenCalled();
+    });
+  });
+
   describe('startScheduledBackups (#1365)', () => {
     // Same key-mismatch bug as backup(): getBackupConfig() returns
     // database_backup_*-prefixed keys, but this read `config.enabled` /

backend/src/services/databaseBackup.js

@@ -28,6 +28,36 @@ const packageJson = require('../../package.json');
 // createSQLiteBackup below.
 const FACE_TABLES = ['photo_faces', 'event_people', 'event_people_merge_dismissals'];
 
+function getStoragePath() {
+  return process.env.STORAGE_PATH || path.join(__dirname, '../../../storage');
+}
+
+// Public, unauthenticated static mounts (server.js) that must never become a
+// backup destination — a dump landing there is downloadable by anyone who
+// learns or guesses the filename, GHSA-jw8m-43r2-jqrm's exact class. Before
+// #1365, `database_backup_destination_path` was silently ignored (a
+// destructuring bug always fell back to the hardcoded /backup/database), so
+// this setting being freely writable by any backup.create holder — the
+// built-in `admin` role has it without settings.edit or backup.restore — was
+// harmless. Making the setting actually take effect reopens that exact
+// exfiltration path unless it's rejected here too.
+function getPubliclyServableRoots() {
+  const storage = getStoragePath();
+  return [
+    path.join(storage, 'uploads', 'logos'),
+    path.join(storage, 'uploads', 'favicons'),
+    path.join(storage, 'fonts')
+  ];
+}
+
+function isUnderPubliclyServableRoot(candidatePath) {
+  const resolved = path.resolve(candidatePath);
+  return getPubliclyServableRoots().some((root) => {
+    const resolvedRoot = path.resolve(root);
+    return resolved === resolvedRoot || resolved.startsWith(resolvedRoot + path.sep);
+  });
+}
+
 /**
  * Database Backup Service
  * Supports both SQLite and PostgreSQL with proper escaping,
@@ -396,6 +426,12 @@ class DatabaseBackupService {
         ...options
       };
       
+      if (isUnderPubliclyServableRoot(destinationPath)) {
+        throw new Error(
+          `Refusing to write a database backup to a publicly served directory: ${destinationPath}`
+        );
+      }
+
       // Create backup directory
       await fs.mkdir(destinationPath, { recursive: true });
       
@@ -822,5 +858,6 @@ module.exports = {
   databaseBackupService,
   startScheduledBackups,
   stopScheduledBackups,
+  isUnderPubliclyServableRoot,
   DatabaseBackupService // Export class for testing
 };