fix(backend): contain and sanitize the SQLite restore source path

The restore flow accepted an unvalidated database.backup_file from
the manifest (absolute paths and traversal both worked, and no
containment check enforced the configured backup root), then
interpolated it unescaped into a `sqlite3 .restore '<path>'` command,
letting an attacker-chosen source file replace the live database.

.release-please-manifest-beta.json

@@ -1,3 +1,3 @@
 {
-  ".": "3.131.1-beta.0"
+  ".": "3.131.3-beta.0"
 }

CHANGELOG.md

@@ -5,6 +5,20 @@ All notable changes to PicPeak will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [3.131.3-beta.0](https://github.com/PicPeak/picpeak/compare/v3.131.2-beta.0...v3.131.3-beta.0) (2026-09-10)
+
+
+### Bug Fixes
+
+* **video:** try metadata extraction and thumbnail generation independently ([#1371](https://github.com/PicPeak/picpeak/issues/1371)) ([a2bf1f6](https://github.com/PicPeak/picpeak/commit/a2bf1f644c78734fc9a86a24d441b7a70bdb38fb))
+
+## [3.131.2-beta.0](https://github.com/PicPeak/picpeak/compare/v3.131.1-beta.0...v3.131.2-beta.0) (2026-09-09)
+
+
+### Bug Fixes
+
+* **backup:** honor the configured database-backup destination path ([#1366](https://github.com/PicPeak/picpeak/issues/1366)) ([15cd5ed](https://github.com/PicPeak/picpeak/commit/15cd5ede82171f5869342de869903f55c73f3871))
+
 ## [3.131.1-beta.0](https://github.com/PicPeak/picpeak/compare/v3.131.0-beta.0...v3.131.1-beta.0) (2026-09-08)
 
 

backend/__tests__/routes/databaseBackupConfigDestination.test.js

@@ -0,0 +1,122 @@
+/**
+ * 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: 'limited-admin-config@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(); });
+
+  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);
+  });
+
+  // A retention of 0 or less pushes cleanupOldBackups' cutoff to today or
+  // the future, deleting every completed backup on the next scheduled run
+  // — a backup.create holder achieving what backup.delete gates on /cleanup.
+  it.each([-1, 0])('rejects database_backup_retention_days=%s', async (bad) => {
+    const res = await request(app)
+      .put('/api/admin/database-backup/config')
+      .set('Authorization', `Bearer ${adminToken}`)
+      .send({ database_backup_retention_days: bad });
+
+    expect(res.status).toBe(400);
+  });
+
+  it('accepts a positive database_backup_retention_days', async () => {
+    const res = await request(app)
+      .put('/api/admin/database-backup/config')
+      .set('Authorization', `Bearer ${adminToken}`)
+      .send({ database_backup_retention_days: 90 });
+
+    expect(res.status).toBe(200);
+
+    const row = await db('app_settings').where({ setting_key: 'database_backup_retention_days' }).first();
+    expect(JSON.parse(row.setting_value)).toBe(90);
+  });
+});

backend/__tests__/routes/restorePathAllowlist.test.js

@@ -22,29 +22,38 @@ process.env.JWT_SECRET = process.env.JWT_SECRET || 'restorepath-test-secret';
 
 const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb');
 
+// `bootCrmDb()` hands back the process-wide `db` singleton (module cache —
+// see its own comment), so it must only be called ONCE per test file: a
+// second call re-runs migrations against the same connection, and the first
+// call's `cleanup()` (db.destroy()) would tear down the connection both
+// describe blocks below share. Boot once at file scope; each describe below
+// only touches app_settings / env vars, never the connection lifecycle.
+let db; let cleanup; let checkRestorePathsAllowed;
+
+beforeAll(async () => {
+  ({ db, cleanup } = await bootCrmDb());
+  await seedMinimal(db);
+  ({ checkRestorePathsAllowed } = require('../../src/routes/adminRestore')._internal);
+}, 120000);
+
+afterAll(async () => { if (cleanup) await cleanup(); });
+
+async function setBackupSetting(key, value) {
+  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',
+    });
+  }
+}
+
 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(); });
+    await setBackupSetting('backup_destination_path', '/backup');
+  });
 
   it('allows the wizard\'s source TYPE tokens', async () => {
     for (const source of ['local', 's3', 'upload']) {
@@ -84,3 +93,89 @@ describe('restore path allowlist (GHSA-fw4c)', () => {
     expect(err).toBeNull();
   });
 });
+
+/**
+ * GHSA-xfvx-j447-732c: `checkRestorePathsAllowed` constrained the top-level
+ * `source`/`manifestPath` request fields (GHSA-fw4c above), but never looked
+ * INSIDE the manifest itself. `manifest.database.backup_file` — handed
+ * straight to restoreService's candidate resolution and eventually
+ * interpolated into `sqlite3 .restore '<path>'` — was unchecked, so an
+ * absolute path there could point the restore at an arbitrary file even
+ * though `source`/`manifestPath` both passed containment.
+ */
+describe('restore path allowlist — manifest database.backup_file containment (GHSA-xfvx)', () => {
+  let tmpRoot;
+
+  beforeAll(async () => {
+    await setBackupSetting('backup_destination_path', '/backup');
+
+    tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-xfvx-manifest-'));
+    // Additional allowed root via the documented escape hatch — keeps this
+    // describe block's fixtures out of the shared '/backup' root above.
+    process.env.RESTORE_ALLOWED_ROOTS = tmpRoot;
+  });
+
+  afterAll(() => {
+    delete process.env.RESTORE_ALLOWED_ROOTS;
+    fs.rmSync(tmpRoot, { recursive: true, force: true });
+  });
+
+  const writeManifest = (name, databaseSection) => {
+    const manifestPath = path.join(tmpRoot, name);
+    fs.writeFileSync(manifestPath, JSON.stringify({
+      manifest: { version: '1.0', id: 'test' },
+      backup: { type: 'full' },
+      system: { platform: 'linux' },
+      application: { version: '1.0.0' },
+      files: { count: 0, manifest: [] },
+      database: databaseSection,
+      verification: { total_checksum: null, checksum_algorithm: null },
+    }));
+    return manifestPath;
+  };
+
+  it('rejects a manifest whose database.backup_file is an absolute path outside every configured root', async () => {
+    const manifestPath = writeManifest('evil-1.json', { backup_file: '/etc/passwd' });
+    const err = await checkRestorePathsAllowed({ source: 'local', manifestPath });
+    expect(err).toMatch(/database\.backup_file must be inside a configured backup location/i);
+  });
+
+  it('accepts a manifest whose database.backup_file is an absolute path inside a configured root', async () => {
+    const dbFile = path.join(tmpRoot, 'database', 'picpeak-db-sqlite-1.sql.gz');
+    fs.mkdirSync(path.dirname(dbFile), { recursive: true });
+    fs.writeFileSync(dbFile, 'not a real sqlite dump, just a fixture');
+    const manifestPath = writeManifest('legit-1.json', { backup_file: dbFile });
+    const err = await checkRestorePathsAllowed({ source: 'local', manifestPath });
+    expect(err).toBeNull();
+  });
+
+  it('does not choke on a manifest whose database.backup_file is a legitimate relative path', async () => {
+    // Relative candidates are resolved against restoreService's own
+    // `backupPath` (which this route-level pre-check doesn't have — it only
+    // sees `source`/`manifestPath`), so this layer intentionally defers
+    // relative-path containment to restoreService.performDatabaseRestore
+    // and must not false-positive here.
+    const manifestPath = writeManifest('legit-2.json', { backup_file: 'database/picpeak-db-sqlite-1.sql.gz' });
+    const err = await checkRestorePathsAllowed({ source: 'local', manifestPath });
+    expect(err).toBeNull();
+  });
+
+  it('rejects everything when no backup location is configured at all (fail closed, not fail open)', async () => {
+    // Simulate an install that never had backup_destination_path /
+    // backup_manifest_path seeded/configured, and isn't using the
+    // RESTORE_ALLOWED_ROOTS escape hatch either.
+    const savedRoots = process.env.RESTORE_ALLOWED_ROOTS;
+    delete process.env.RESTORE_ALLOWED_ROOTS;
+    await db('app_settings').whereIn('setting_key', ['backup_destination_path', 'backup_manifest_path']).del();
+
+    try {
+      const err = await checkRestorePathsAllowed({
+        source: '/backup/run-1', manifestPath: '/backup/run-1/manifest.json',
+      });
+      expect(err).toMatch(/no backup location is configured/i);
+    } finally {
+      process.env.RESTORE_ALLOWED_ROOTS = savedRoots;
+      await setBackupSetting('backup_destination_path', '/backup');
+    }
+  });
+});

backend/__tests__/services/restoreService.sqliteRestorePathSafety.test.js

@@ -0,0 +1,173 @@
+/**
+ * GHSA-xfvx-j447-732c: the SQLite restore path let an attacker-influenced
+ * `manifest.database.backup_file` replace the live database.
+ *
+ * Two independent bugs, both fixed here:
+ *
+ *   1. Candidate resolution (restoreService.js's performDatabaseRestore,
+ *      ~L1000) tried an absolute `dbBackupFile` and a
+ *      `path.join(backupPath, dbBackupFile)` candidate with NO check that
+ *      the resolved path actually stayed inside the configured backup
+ *      root — a manifest could point `.restore` at any file on disk.
+ *
+ *   2. The resolved path was interpolated unescaped into a
+ *      `sqlite3 .restore '<path>'` dot-command string. sqlite3's CLI
+ *      parses that string itself (not the shell), so a single quote in
+ *      the path breaks out of the quoted argument regardless of
+ *      spawn()'s `shell: false` argv separation.
+ *
+ * These tests pin the fix directly against the exported helpers
+ * (`resolveContainedDbBackupCandidates`, `assertSafeSqlitePath`,
+ * `isContainedInRoots`, `getConfiguredBackupRoots`) — the exact functions
+ * `performDatabaseRestore` calls before ever running `sqlite3 .restore` —
+ * rather than driving the full restore (which does a real `db.destroy()` +
+ * live-file swap against the shared app db and isn't worth the added
+ * fragility for what's fundamentally a path-validation contract).
+ */
+
+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-restoresvc-')), 'db.sqlite',
+);
+process.env.JWT_SECRET = process.env.JWT_SECRET || 'restoresvc-test-secret';
+
+const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb');
+
+describe('restoreService — sqlite restore path safety (GHSA-xfvx)', () => {
+  let db; let cleanup; let _internal;
+  let backupPath;
+
+  beforeAll(async () => {
+    ({ db, cleanup } = await bootCrmDb());
+    await seedMinimal(db);
+
+    // The restore run's resolved local backup root — analogous to
+    // `localBackupPath` in restoreService.restore(). Real directory with a
+    // real database/ subfolder, matching what a genuine backup run leaves
+    // on disk.
+    backupPath = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-xfvx-backuproot-'));
+    fs.mkdirSync(path.join(backupPath, 'database'), { recursive: true });
+
+    ({ _internal } = require('../../src/services/restoreService'));
+  }, 120000);
+
+  afterAll(async () => { if (cleanup) await cleanup(); });
+
+  describe('assertSafeSqlitePath — the sqlite3 dot-command injection gate', () => {
+    it.each([
+      ['/backup/database/picpeak-db-sqlite-1.sql'],
+      [`${backupPath || '/backup'}/database/picpeak-db-sqlite-2024-01-01.sql.gz`],
+    ])('accepts a normal backup path: %s', (p) => {
+      expect(() => _internal.assertSafeSqlitePath(p)).not.toThrow();
+    });
+
+    it.each([
+      ['/backup/database/x\'; DROP TABLE admin_users; --.sql'],
+      ['/backup/database/x\' .restore \'/etc/passwd'],
+      ['/backup/database/x\n.shell rm -rf /'],
+      ['/backup/database/has space.sql'],
+      ['/backup/database/semi;colon.sql'],
+      [null],
+      [undefined],
+      [42],
+    ])('rejects an unsafe/non-string path: %j', (p) => {
+      expect(() => _internal.assertSafeSqlitePath(p)).toThrow(/unsafe path/i);
+    });
+  });
+
+  describe('isContainedInRoots', () => {
+    it('accepts a path inside a root', () => {
+      expect(_internal.isContainedInRoots('/backup/database/x.sql', ['/backup'])).toBe(true);
+    });
+
+    it('accepts a root path equal to the root itself', () => {
+      expect(_internal.isContainedInRoots('/backup', ['/backup'])).toBe(true);
+    });
+
+    it('rejects a path outside every root', () => {
+      expect(_internal.isContainedInRoots('/etc/passwd', ['/backup'])).toBe(false);
+    });
+
+    it('rejects a sibling directory that merely shares a prefix', () => {
+      // '/backup-evil' starts with the string '/backup' but is NOT inside it.
+      expect(_internal.isContainedInRoots('/backup-evil/x.sql', ['/backup'])).toBe(false);
+    });
+
+    it('rejects a `..`-traversal path that resolves outside the root', () => {
+      expect(_internal.isContainedInRoots('/backup/../etc/passwd', ['/backup'])).toBe(false);
+    });
+  });
+
+  describe('getConfiguredBackupRoots', () => {
+    afterEach(async () => {
+      delete process.env.RESTORE_ALLOWED_ROOTS;
+      await db('app_settings').whereIn('setting_key', ['backup_destination_path', 'backup_manifest_path']).del();
+    });
+
+    it('always includes the trusted root even with nothing else configured', async () => {
+      const roots = await _internal.getConfiguredBackupRoots('/some/trusted/backup-path');
+      expect(roots).toContain(path.resolve('/some/trusted/backup-path'));
+    });
+
+    it('adds configured backup_destination_path / backup_manifest_path and RESTORE_ALLOWED_ROOTS', async () => {
+      await db('app_settings').insert([
+        { setting_key: 'backup_destination_path', setting_value: JSON.stringify('/backup/dest'), setting_type: 'backup' },
+        { setting_key: 'backup_manifest_path', setting_value: JSON.stringify('/backup/manifests'), setting_type: 'backup' },
+      ]);
+      process.env.RESTORE_ALLOWED_ROOTS = '/extra/root';
+
+      const roots = await _internal.getConfiguredBackupRoots('/trusted');
+      expect(roots).toEqual(expect.arrayContaining([
+        path.resolve('/trusted'),
+        path.resolve('/backup/dest'),
+        path.resolve('/backup/manifests'),
+        path.resolve('/extra/root'),
+      ]));
+    });
+  });
+
+  describe('resolveContainedDbBackupCandidates — the manifest.database.backup_file gate', () => {
+    it('rejects an absolute backup_file outside every configured root, but still offers the safe legacy basename candidate', async () => {
+      const candidates = await _internal.resolveContainedDbBackupCandidates(
+        backupPath, '/etc/passwd', () => {}
+      );
+      // The raw absolute escape must NOT be present.
+      expect(candidates).not.toContain('/etc/passwd');
+      // Candidate (3), the basename-only legacy reconstruct, is inherently
+      // safe (can't escape backupPath) and stays available as a fallback.
+      expect(candidates).toContain(path.join(backupPath, 'database', 'passwd'));
+    });
+
+    it('rejects a `..`-traversal relative backup_file, keeping only the contained legacy candidate', async () => {
+      const candidates = await _internal.resolveContainedDbBackupCandidates(
+        backupPath, '../../../../etc/passwd', () => {}
+      );
+      const escaped = candidates.some((c) => !_internal.isContainedInRoots(c, [path.resolve(backupPath)]));
+      expect(escaped).toBe(false);
+      expect(candidates).toContain(path.join(backupPath, 'database', 'passwd'));
+    });
+
+    it('accepts a legitimate relative backup_file recorded by a real backup run', async () => {
+      const candidates = await _internal.resolveContainedDbBackupCandidates(
+        backupPath, 'database/picpeak-db-sqlite-2024-01-01.sql.gz', () => {}
+      );
+      expect(candidates).toContain(path.join(backupPath, 'database', 'picpeak-db-sqlite-2024-01-01.sql.gz'));
+      // Every returned candidate must actually be safe to use.
+      for (const c of candidates) {
+        expect(_internal.isContainedInRoots(c, [path.resolve(backupPath)])).toBe(true);
+      }
+    });
+
+    it('accepts a legitimate absolute backup_file that IS inside backupPath (the real dumper shape)', async () => {
+      const absFile = path.join(backupPath, 'database', 'picpeak-db-sqlite-2024-02-02.sql.gz');
+      const candidates = await _internal.resolveContainedDbBackupCandidates(
+        backupPath, absFile, () => {}
+      );
+      expect(candidates).toContain(absFile);
+    });
+  });
+});

backend/__tests__/services/videoPlaceholderNoDbLookup.test.js

@@ -0,0 +1,62 @@
+/**
+ * generateVideoPlaceholder() must not touch the database when the caller
+ * already supplies width/height (videoProcessor.js's thumbnail-generation
+ * fallback does exactly this).
+ *
+ * Why it matters: processUploadedPhotos() (chunked video upload) holds a
+ * per-file SQLite transaction open across thumbnail generation. SQLite's
+ * knex pool defaults to a single connection, so any second, un-transacted
+ * db() query made while that transaction is open blocks until
+ * acquireConnectionTimeout (60s in production) — verified directly against
+ * an isolated SQLite db (codex review of #1371/#1372). Passing explicit
+ * dimensions must skip getThumbnailSettings()'s db() call entirely, not
+ * just tolerate its failure.
+ */
+
+const path = require('path');
+const fs = require('fs').promises;
+const os = require('os');
+
+const mockDbSpy = jest.fn(() => {
+  throw new Error('db() must not be called when width/height are supplied');
+});
+jest.mock('../../src/database/db', () => ({ db: (...args) => mockDbSpy(...args) }));
+
+const storageModule = require('../../src/services/storage');
+const LocalFsStorage = require('../../src/services/storage/LocalFsStorage');
+
+describe('generateVideoPlaceholder skips the settings DB lookup given explicit dimensions', () => {
+  let storage;
+  let root;
+  let imageProcessor;
+
+  beforeAll(async () => {
+    root = await fs.mkdtemp(path.join(os.tmpdir(), 'picpeak-vidplaceholder-'));
+    storage = new LocalFsStorage({ root });
+    await storage.init();
+    storageModule.setStorageForTesting(storage);
+    imageProcessor = require('../../src/services/imageProcessor');
+  }, 30000);
+
+  afterAll(async () => {
+    storageModule.resetStorage();
+    await fs.rm(root, { recursive: true, force: true }).catch(() => {});
+  });
+
+  afterEach(() => mockDbSpy.mockClear());
+
+  it('never calls db() when width/height are provided', async () => {
+    const key = await imageProcessor.generateVideoPlaceholder('demo.mp4', { width: 300, height: 300 });
+
+    expect(key).toBe('thumbnails/thumb_demo.jpg');
+    expect(await storage.exists(key)).toBe(true);
+    expect(mockDbSpy).not.toHaveBeenCalled();
+  });
+
+  it('falls through to defaults (not a throw) when db() fails and no dimensions were given', async () => {
+    const key = await imageProcessor.generateVideoPlaceholder('demo2.mp4');
+
+    expect(key).toBe('thumbnails/thumb_demo2.jpg');
+    expect(mockDbSpy).toHaveBeenCalled();
+  });
+});

backend/package.json

@@ -1,6 +1,6 @@
 {
   "name": "picpeak-backend",
-  "version": "3.131.1-beta.0",
+  "version": "3.131.3-beta.0",
   "description": "Backend for PicPeak event photo sharing platform",
   "main": "server.js",
   "engines": {

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,28 @@ 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' });
+    }
+
+    // A retention of 0 or less pushes cleanupOldBackups' cutoff to today or
+    // the future, deleting every completed backup on the next scheduled run
+    // — a backup.create holder achieving what backup.delete gates on /cleanup.
+    if (
+      req.body.database_backup_retention_days !== undefined
+      && (!Number.isFinite(req.body.database_backup_retention_days) || req.body.database_backup_retention_days < 1)
+    ) {
+      return res.status(400).json({ error: 'database_backup_retention_days must be a positive number' });
+    }
+
     const updates = [];
     
     for (const [key, value] of Object.entries(req.body)) {

backend/src/routes/adminRestore.js

@@ -810,24 +810,72 @@ async function checkRestorePathsAllowed({ source, manifestPath }) {
     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;
+    // GHSA-xfvx: nothing configured to compare against used to mean "a
+    // restore can't be scoped, so don't pretend to enforce" — returning
+    // null (allow). That's fail-OPEN: on a fresh install (or one where an
+    // operator never set backup_destination_path/backup_manifest_path) any
+    // authenticated `backup.restore` caller could point source/manifestPath
+    // — and, via the manifest, database.backup_file — at literally any path
+    // on disk. Require configuration instead of silently allowing
+    // everything; the normal restore wizard already needs one of these
+    // settings populated to discover backups in the first place.
+    logger.warn('Refusing restore: no backup location configured to scope it to', { candidates });
+    return 'No backup location is configured (backup_destination_path / backup_manifest_path). ' +
+      'Configure one before restoring.';
   }
 
   const resolvedRoots = roots.map((r) => path.resolve(r));
-  for (const candidate of candidates) {
+  const isInsideRoots = (candidate) => {
     const resolved = path.resolve(candidate);
-    const inside = resolvedRoots.some(
+    return resolvedRoots.some(
       (root) => resolved === root || resolved.startsWith(root + path.sep)
     );
-    if (!inside) {
+  };
+
+  for (const candidate of candidates) {
+    if (!isInsideRoots(candidate)) {
       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';
     }
   }
+
+  // GHSA-xfvx: source/manifestPath containment alone isn't enough — the
+  // manifest FILE (which just passed containment above) can itself carry a
+  // `database.backup_file` field that restoreService's candidate resolution
+  // used to hand straight to `sqlite3 .restore` with no containment check at
+  // all. Peek at the manifest here (it's already proven to live inside an
+  // allowed root) and reject an ABSOLUTE backup_file that escapes the same
+  // roots — the case that's unambiguous to check without re-deriving
+  // restoreService's own `backupPath` resolution for the relative-path
+  // candidates. This is deliberately defense in depth, not the only gate:
+  // restoreService.performDatabaseRestore independently re-derives and
+  // enforces containment (including relative/`..` candidates) against
+  // `backupPath` right before ever using the resolved path, and remains the
+  // authoritative check for S3-sourced manifests (downloaded after this
+  // pre-check runs).
+  if (manifestPath && !isS3(manifestPath) && !isTypeToken(manifestPath)) {
+    try {
+      const raw = await fs.readFile(manifestPath, 'utf8');
+      const trimmed = raw.trimStart();
+      const parsed = (trimmed.startsWith('{') || trimmed.startsWith('['))
+        ? JSON.parse(raw)
+        : null; // non-JSON (e.g. YAML) manifests are re-checked inside restoreService
+      const dbBackupFile = parsed?.database?.backup_file;
+      if (typeof dbBackupFile === 'string' && path.isAbsolute(dbBackupFile) && !isInsideRoots(dbBackupFile)) {
+        logger.warn('Refusing restore: manifest database.backup_file escapes configured backup roots', {
+          manifestPath, backupFile: dbBackupFile,
+        });
+        return 'Manifest database.backup_file must be inside a configured backup location';
+      }
+    } catch (_) {
+      // Unreadable/corrupt/non-JSON manifest: let the normal restore flow
+      // surface the real error (loadAndValidateManifest) instead of failing
+      // this pre-check for an unrelated reason.
+    }
+  }
+
   return null;
 }
 

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

@@ -1,6 +1,6 @@
-const { DatabaseBackupService } = require('../databaseBackup');
 const { db } = require('../../database/db');
 const fs = require('fs').promises;
+const path = require('path');
 const crypto = require('crypto');
 
 // Mock dependencies
@@ -8,6 +8,10 @@ jest.mock('../../database/db');
 jest.mock('../../utils/logger');
 jest.mock('../emailProcessor');
 jest.mock('child_process');
+jest.mock('node-cron', () => ({ schedule: jest.fn(() => ({ stop: jest.fn() })) }));
+
+const { DatabaseBackupService, startScheduledBackups, databaseBackupService, isUnderPubliclyServableRoot } = require('../databaseBackup');
+const cron = require('node-cron');
 
 describe('DatabaseBackupService', () => {
   let service;
@@ -191,6 +195,213 @@ describe('DatabaseBackupService', () => {
     });
   });
 
+  describe('backup() destination path resolution (#1365)', () => {
+    // getBackupConfig() returns database_backup_*-prefixed keys.
+    // Regression: backup() used to destructure the unprefixed names
+    // (`destinationPath`, ...) straight off that object, which never
+    // matched, so the configured path was silently ignored and every
+    // run tried to create the hardcoded /backup/database default.
+    it('creates the directory from database_backup_destination_path when configured', async () => {
+      db.mockReturnValue({
+        where: jest.fn().mockReturnThis(),
+        select: jest.fn().mockResolvedValue([
+          { setting_key: 'database_backup_destination_path', setting_value: JSON.stringify('/data/db-backups') }
+        ])
+      });
+
+      const stop = new Error('stop after mkdir — nothing past it matters for this test');
+      const mkdirSpy = jest.spyOn(fs, 'mkdir').mockRejectedValue(stop);
+
+      await expect(service.backup({})).rejects.toThrow(stop.message);
+
+      expect(mkdirSpy).toHaveBeenCalledWith('/data/db-backups', { recursive: true });
+      mkdirSpy.mockRestore();
+    });
+
+    it('falls back to /backup/database only when nothing is configured', async () => {
+      db.mockReturnValue({
+        where: jest.fn().mockReturnThis(),
+        select: jest.fn().mockResolvedValue([])
+      });
+
+      const stop = new Error('stop after mkdir');
+      const mkdirSpy = jest.spyOn(fs, 'mkdir').mockRejectedValue(stop);
+
+      await expect(service.backup({})).rejects.toThrow(stop.message);
+
+      expect(mkdirSpy).toHaveBeenCalledWith('/backup/database', { recursive: true });
+      mkdirSpy.mockRestore();
+    });
+  });
+
+  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'),
+      // Bundled fallback fonts — nodejs-owned per the Dockerfile's
+      // COPY --chown, and served at the same public /fonts route.
+      path.resolve(__dirname, '../../../assets/fonts'),
+      // Case-insensitive-but-preserving filesystems (APFS, NTFS, Docker
+      // Desktop bind mounts of either) resolve this to the same directory
+      // as uploads/logos even though path.resolve() never folds case.
+      path.join(storage, 'UPLOADS', 'Logos')
+    ])('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();
+      mkdirSpy.mockRestore();
+    });
+
+    it('flags FRONTEND_DIR — the all-in-one image serves its built SPA unauthenticated', () => {
+      const originalFrontendDir = process.env.FRONTEND_DIR;
+      process.env.FRONTEND_DIR = '/app/frontend/dist';
+      try {
+        expect(isUnderPubliclyServableRoot('/app/frontend/dist')).toBe(true);
+        expect(isUnderPubliclyServableRoot(path.join('/app/frontend/dist', 'assets'))).toBe(true);
+      } finally {
+        if (originalFrontendDir === undefined) delete process.env.FRONTEND_DIR;
+        else process.env.FRONTEND_DIR = originalFrontendDir;
+      }
+    });
+
+    it('resolves a symlinked alias of a public root to the same real directory (all-in-one /app/storage -> /data/storage)', async () => {
+      const os = require('os');
+      const realRoot = await fs.mkdtemp(path.join(os.tmpdir(), 'picpeak-real-'));
+      const linkRoot = path.join(os.tmpdir(), `picpeak-link-${process.pid}-${Date.now()}`);
+      await fs.mkdir(path.join(realRoot, 'uploads', 'logos'), { recursive: true });
+      await fs.symlink(realRoot, linkRoot, 'dir');
+
+      try {
+        // STORAGE_PATH (what the guard's roots are built from) is the real
+        // path; the attacker-supplied destination goes through the symlink
+        // — exactly the all-in-one image's /app/storage -> /data/storage.
+        process.env.STORAGE_PATH = realRoot;
+        const aliased = path.join(linkRoot, 'uploads', 'logos');
+
+        expect(isUnderPubliclyServableRoot(aliased)).toBe(true);
+      } finally {
+        await fs.unlink(linkRoot);
+        await fs.rm(realRoot, { recursive: true, force: true });
+      }
+    });
+  });
+
+  describe('startScheduledBackups (#1365)', () => {
+    // Same key-mismatch bug as backup(): getBackupConfig() returns
+    // database_backup_*-prefixed keys, but this read `config.enabled` /
+    // `config.schedule` / `config.retentionDays` — always undefined, so
+    // the scheduler silently treated every install as disabled.
+    it('does not start the schedule while database_backup_enabled is false', async () => {
+      db.mockReturnValue({
+        where: jest.fn().mockReturnThis(),
+        select: jest.fn().mockResolvedValue([
+          { setting_key: 'database_backup_enabled', setting_value: 'false' }
+        ])
+      });
+
+      await startScheduledBackups();
+
+      expect(cron.schedule).not.toHaveBeenCalled();
+    });
+
+    it('starts the schedule with the configured cron when database_backup_enabled is true', async () => {
+      db.mockReturnValue({
+        where: jest.fn().mockReturnThis(),
+        select: jest.fn().mockResolvedValue([
+          { setting_key: 'database_backup_enabled', setting_value: 'true' },
+          { setting_key: 'database_backup_schedule', setting_value: JSON.stringify('0 4 * * *') }
+        ])
+      });
+
+      await startScheduledBackups();
+
+      expect(cron.schedule).toHaveBeenCalledWith('0 4 * * *', expect.any(Function));
+    });
+
+    it('re-reads retention on every tick instead of the value captured at schedule start (#1365)', async () => {
+      db.mockReturnValue({
+        where: jest.fn().mockReturnThis(),
+        select: jest.fn().mockResolvedValue([
+          { setting_key: 'database_backup_enabled', setting_value: 'true' },
+          { setting_key: 'database_backup_retention_days', setting_value: JSON.stringify(30) }
+        ])
+      });
+
+      await startScheduledBackups();
+      const tick = cron.schedule.mock.calls[0][1];
+
+      // A /config update between schedule-start and this tick raised
+      // retention to 365 — the closed-over 30 must not be what runs.
+      db.mockReturnValue({
+        where: jest.fn().mockReturnThis(),
+        select: jest.fn().mockResolvedValue([
+          { setting_key: 'database_backup_enabled', setting_value: 'true' },
+          { setting_key: 'database_backup_retention_days', setting_value: JSON.stringify(365) }
+        ])
+      });
+      jest.spyOn(databaseBackupService, 'backup').mockResolvedValue({ success: true });
+      const cleanupSpy = jest.spyOn(databaseBackupService, 'cleanupOldBackups').mockResolvedValue(undefined);
+
+      await tick();
+
+      expect(cleanupSpy).toHaveBeenCalledWith(365);
+
+      jest.restoreAllMocks();
+    });
+  });
+
+  describe('cleanupOldBackups destructive-retention guard (#1365)', () => {
+    it.each([-1, 0, NaN, Infinity])('refuses retentionDays=%s without touching the database', async (bad) => {
+      const dbSpy = jest.fn();
+      db.mockImplementation(dbSpy);
+
+      await service.cleanupOldBackups(bad);
+
+      expect(dbSpy).not.toHaveBeenCalled();
+    });
+  });
+
   describe('cleanupOldBackups', () => {
     it('should delete old backup files and records', async () => {
       const oldBackups = [

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

@@ -0,0 +1,125 @@
+jest.mock('../../utils/logger');
+jest.mock('fluent-ffmpeg');
+jest.mock('../storage', () => ({
+  getStorage: jest.fn()
+}));
+jest.mock('../imageProcessor', () => ({
+  generateVideoPlaceholder: jest.fn(),
+  DEFAULT_THUMBNAIL_WIDTH: 300,
+  DEFAULT_THUMBNAIL_HEIGHT: 300
+}));
+
+const ffmpeg = require('fluent-ffmpeg');
+const { getStorage } = require('../storage');
+const { generateVideoPlaceholder } = require('../imageProcessor');
+const {
+  extractVideoMetadata,
+  processUploadedVideo
+} = require('../videoProcessor');
+
+describe('extractVideoMetadata (#1370)', () => {
+  afterEach(() => jest.clearAllMocks());
+
+  it('returns null duration rather than 0 when ffprobe has none, so "unknown" and "a real 0s clip" stay distinguishable', async () => {
+    ffmpeg.ffprobe = jest.fn((videoPath, cb) => {
+      cb(null, {
+        streams: [{ codec_type: 'video', width: 1920, height: 1080, codec_name: 'hevc' }],
+        format: {} // no duration field at all
+      });
+    });
+
+    const metadata = await extractVideoMetadata('/tmp/video.mp4');
+
+    expect(metadata.duration).toBeNull();
+    expect(metadata.width).toBe(1920);
+    expect(metadata.videoCodec).toBe('hevc');
+  });
+
+  it('floors a real duration', async () => {
+    ffmpeg.ffprobe = jest.fn((videoPath, cb) => {
+      cb(null, { streams: [], format: { duration: 12.9 } });
+    });
+
+    const metadata = await extractVideoMetadata('/tmp/video.mp4');
+
+    expect(metadata.duration).toBe(12);
+  });
+});
+
+describe('processUploadedVideo degrades gracefully instead of rejecting the whole video (#1370)', () => {
+  let storage;
+
+  beforeEach(() => {
+    storage = { putFromFile: jest.fn().mockResolvedValue(undefined), exists: jest.fn().mockResolvedValue(true) };
+    getStorage.mockReturnValue(storage);
+    generateVideoPlaceholder.mockResolvedValue('thumbnails/thumb_placeholder.jpg');
+  });
+
+  afterEach(() => jest.clearAllMocks());
+
+  it('keeps the thumbnail when only metadata extraction fails', async () => {
+    ffmpeg.ffprobe = jest.fn((videoPath, cb) => cb(new Error('moov atom not found')));
+    ffmpeg.mockImplementation(() => ({
+      screenshots: jest.fn(function screenshots({ filename, folder }) {
+        require('fs').writeFileSync(require('path').join(folder, filename), 'jpeg-bytes');
+        return this;
+      }),
+      on(event, handler) {
+        if (event === 'end') setImmediate(handler);
+        return this;
+      }
+    }));
+
+    const result = await processUploadedVideo('/tmp/video.mp4', 'thumbnails/thumb_video.jpg');
+
+    expect(result.success).toBe(true);
+    expect(result.metadata).toBeNull();
+    expect(result.thumbnailKey).toBe('thumbnails/thumb_video.jpg');
+    // A real thumbnail already succeeded — never touch the placeholder path.
+    expect(generateVideoPlaceholder).not.toHaveBeenCalled();
+  });
+
+  it('falls back to the SVG placeholder when thumbnail generation fails, so the gallery never falls back to rendering the raw video as an <img> (codex review)', async () => {
+    ffmpeg.ffprobe = jest.fn((videoPath, cb) => {
+      cb(null, {
+        streams: [{ codec_type: 'video', width: 1080, height: 1920, codec_name: 'h264' }],
+        format: { duration: 5.4 }
+      });
+    });
+    ffmpeg.mockImplementation(() => ({
+      screenshots() { return this; },
+      on(event, handler) {
+        if (event === 'error') setImmediate(() => handler(new Error('ffmpeg seek failed')));
+        return this;
+      }
+    }));
+
+    const result = await processUploadedVideo('/tmp/video.mp4', 'thumbnails/thumb_wedding_001.jpg');
+
+    expect(result.success).toBe(true);
+    expect(result.metadata).toEqual(expect.objectContaining({ duration: 5, videoCodec: 'h264' }));
+    // thumbnailKey is always thumbnails/thumb_<name>.jpg — strip the prefix
+    // back to a filename so generateVideoPlaceholder recomputes the same key.
+    // Explicit width/height so generateVideoPlaceholder skips its DB-backed
+    // settings lookup — this can run inside an open per-file SQLite
+    // transaction (chunked video upload), where that lookup deadlocks.
+    expect(generateVideoPlaceholder).toHaveBeenCalledWith('wedding_001.jpg', { width: 300, height: 300 });
+    expect(result.thumbnailKey).toBe('thumbnails/thumb_placeholder.jpg');
+    expect(storage.putFromFile).not.toHaveBeenCalled();
+  });
+
+  it('throws when metadata, thumbnail generation, AND the placeholder all fail, so the caller surfaces a retryable failure instead of completing with nothing to show (codex review)', async () => {
+    ffmpeg.ffprobe = jest.fn((videoPath, cb) => cb(new Error('Invalid data found when processing input')));
+    ffmpeg.mockImplementation(() => ({
+      screenshots() { return this; },
+      on(event, handler) {
+        if (event === 'error') setImmediate(() => handler(new Error('ffmpeg seek failed')));
+        return this;
+      }
+    }));
+    generateVideoPlaceholder.mockRejectedValue(new Error('sharp render failed'));
+
+    await expect(processUploadedVideo('/tmp/corrupt.mp4', 'thumbnails/thumb_corrupt.jpg'))
+      .rejects.toThrow('Unable to generate any thumbnail');
+  });
+});

backend/src/services/backupService.js

@@ -1041,7 +1041,7 @@ function buildManifestFiles(backedUpFiles, allFiles) {
 
 async function saveManifestToLocal(manifest, manifestFileName, config) {
   const manifestDir = config.backup_manifest_path
-    || path.join(config.backup_destination_path || '/backup', 'manifests');
+    || path.join(config.backup_destination_path || path.join(getStoragePath(), 'backups'), 'manifests');
   await fs.mkdir(manifestDir, { recursive: true });
   const manifestPath = path.join(manifestDir, manifestFileName);
   await backupManifest.saveManifest(manifest, manifestPath, config.backup_manifest_format || 'json');

backend/src/services/databaseBackup.js

@@ -4,7 +4,7 @@ const crypto = require('crypto');
 const { spawnAsync, spawnToFile } = require('../utils/safeExec');
 const zlib = require('zlib');
 const { pipeline } = require('stream/promises');
-const { createReadStream, createWriteStream } = require('fs');
+const { createReadStream, createWriteStream, realpathSync } = require('fs');
 const { db } = require('../database/db');
 const knexConfig = require('../../knexfile');
 const logger = require('../utils/logger');
@@ -28,6 +28,76 @@ 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'),
+    // Bundled fallback fonts (server.js mounts both at /fonts, storage wins
+    // on overlap but express.static falls through to this one on a miss).
+    // COPY --chown=nodejs:nodejs in the Dockerfile makes this nodejs-owned
+    // and therefore writable at runtime, not just a read-only image layer.
+    path.resolve(__dirname, '../../assets/fonts'),
+    // The all-in-one image's built frontend bundle (Dockerfile.aio ships it
+    // nodejs-owned) — server.js serves it unauthenticated as the SPA itself.
+    process.env.FRONTEND_DIR || path.resolve(__dirname, '../../../frontend/dist')
+  ];
+}
+
+// Resolves symlinks in whatever prefix of candidatePath currently exists,
+// then re-appends any not-yet-created remainder literally. A plain
+// fs.realpathSync would throw ENOENT for the common case where the backup
+// destination doesn't exist yet; a plain path.resolve() would miss the
+// all-in-one image's `/app/storage -> /data/storage` symlink (Dockerfile.aio),
+// which lets `/app/storage/uploads/logos` alias the real public logos
+// directory under a name that never lexically matches it.
+function resolveRealish(candidatePath) {
+  let current = path.resolve(candidatePath);
+  const remainder = [];
+  for (;;) {
+    try {
+      const real = realpathSync(current);
+      return remainder.length ? path.join(real, ...remainder) : real;
+    } catch (error) {
+      if (error.code !== 'ENOENT') {
+        return path.resolve(candidatePath);
+      }
+      const parent = path.dirname(current);
+      if (parent === current) {
+        return path.resolve(candidatePath);
+      }
+      remainder.unshift(path.basename(current));
+      current = parent;
+    }
+  }
+}
+
+function isUnderPubliclyServableRoot(candidatePath) {
+  // Lowercased comparison: on a case-insensitive-but-preserving filesystem
+  // (default macOS APFS, NTFS, and Docker Desktop's bind-mount passthrough
+  // of either) `STORAGE_PATH/UPLOADS/logos` and `.../uploads/logos` name the
+  // same directory on disk even though path.resolve() never folds case.
+  const resolved = resolveRealish(candidatePath).toLowerCase();
+  return getPubliclyServableRoots().some((root) => {
+    const resolvedRoot = resolveRealish(root).toLowerCase();
+    return resolved === resolvedRoot || resolved.startsWith(resolvedRoot + path.sep);
+  });
+}
+
 /**
  * Database Backup Service
  * Supports both SQLite and PostgreSQL with proper escaping,
@@ -375,15 +445,33 @@ class DatabaseBackupService {
     let backupRun = null;
     
     try {
-      // Get configuration
+      // Get configuration. getBackupConfig() returns the raw
+      // database_backup_*-prefixed setting keys, not the unprefixed
+      // names used internally below — map them explicitly rather than
+      // spreading `config` straight into the destructure, which silently
+      // matched nothing and always fell through to the hardcoded
+      // defaults (notably `/backup/database`, regardless of what was
+      // configured).
       const config = await this.getBackupConfig();
       const {
         destinationPath = '/backup/database',
         compress = true,
         validateIntegrity = true,
         includeChecksums = true
-      } = { ...config, ...options };
+      } = {
+        destinationPath: config.database_backup_destination_path,
+        compress: config.database_backup_compress,
+        validateIntegrity: config.database_backup_validate_integrity,
+        includeChecksums: config.database_backup_include_checksums,
+        ...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 });
       
@@ -502,7 +590,7 @@ class DatabaseBackupService {
       logger.info(`Database backup completed: ${finalFile} (${(finalStats.size / 1024 / 1024).toFixed(2)} MB) in ${durationSeconds}s`);
       
       // Send success notification if configured
-      if (config.emailOnSuccess) {
+      if (config.database_backup_email_on_success) {
         await this.sendBackupNotification('success', {
           duration: durationSeconds,
           size: finalStats.size,
@@ -536,7 +624,7 @@ class DatabaseBackupService {
       
       // Send failure notification
       const config = await this.getBackupConfig();
-      if (config.emailOnFailure) {
+      if (config.database_backup_email_on_failure) {
         await this.sendBackupNotification('failure', {
           error: error.message
         });
@@ -617,10 +705,19 @@ class DatabaseBackupService {
    * Clean up old backups
    */
   async cleanupOldBackups(retentionDays = 30) {
+    // A zero/negative/non-finite value pushes the cutoff to today or the
+    // future, matching (and deleting) every completed backup — including
+    // the one a scheduled run just created. Defense in depth: PUT /config
+    // already rejects such values, but this is also reachable with
+    // whatever database_backup_retention_days happens to be persisted.
+    if (!Number.isFinite(retentionDays) || retentionDays < 1) {
+      logger.error(`Refusing to clean up backups with invalid retentionDays: ${retentionDays}`);
+      return;
+    }
     try {
       const cutoffDate = new Date();
       cutoffDate.setDate(cutoffDate.getDate() - retentionDays);
-      
+
       // Get old backup records
       const oldBackups = await db('database_backup_runs')
         .where('completed_at', '<', cutoffDate)
@@ -765,25 +862,30 @@ async function startScheduledBackups() {
   
   try {
     const config = await databaseBackupService.getBackupConfig();
-    
-    if (!config.enabled) {
+
+    if (!config.database_backup_enabled) {
       logger.info('Database backup service is disabled');
       return;
     }
-    
+
     // Stop existing schedule
     if (backupSchedule) {
       backupSchedule.stop();
     }
-    
+
     // Default schedule: 3 AM daily (offset from file backups at 2 AM)
-    const schedule = config.schedule || '0 3 * * *';
-    
+    const schedule = config.database_backup_schedule || '0 3 * * *';
+
     backupSchedule = cron.schedule(schedule, async () => {
       logger.info('Starting scheduled database backup');
       try {
         await databaseBackupService.backup();
-        await databaseBackupService.cleanupOldBackups(config.retentionDays || 30);
+        // Re-read retention on every tick rather than closing over the value
+        // from schedule start — a retention-only /config update doesn't
+        // restart the schedule (only enabled/schedule changes do), so the
+        // closed-over value would otherwise run stale until next restart.
+        const latestConfig = await databaseBackupService.getBackupConfig();
+        await databaseBackupService.cleanupOldBackups(latestConfig.database_backup_retention_days || 30);
       } catch (error) {
         logger.error('Scheduled database backup failed:', error);
       }
@@ -810,5 +912,6 @@ module.exports = {
   databaseBackupService,
   startScheduledBackups,
   stopScheduledBackups,
+  isUnderPubliclyServableRoot,
   DatabaseBackupService // Export class for testing
 };

backend/src/services/imageProcessor.js

@@ -610,9 +610,15 @@ async function generateVideoPlaceholder(originalFilename, options = {}) {
   const thumbnailRelKey = path.posix.join('thumbnails', thumbnailFilename);
   const storage = getStorage();
 
-  const settings = await getThumbnailSettings();
-  const width = settings.width || DEFAULT_THUMBNAIL_WIDTH;
-  const height = settings.height || DEFAULT_THUMBNAIL_HEIGHT;
+  // Skip the settings lookup when the caller already supplies dimensions.
+  // This can run from inside an open per-file SQLite transaction (chunked
+  // video upload's fallback path in videoProcessor.js) — a second,
+  // un-transacted db() query for settings there deadlocks against SQLite's
+  // single-connection pool until acquireConnectionTimeout (60s), reproduced
+  // directly against an isolated SQLite db (codex review of #1371/#1372).
+  const settings = (options.width && options.height) ? {} : await getThumbnailSettings();
+  const width = options.width || settings.width || DEFAULT_THUMBNAIL_WIDTH;
+  const height = options.height || settings.height || DEFAULT_THUMBNAIL_HEIGHT;
 
   if (options.regenerate) {
     await storage.delete(thumbnailRelKey).catch(() => {});
@@ -1495,4 +1501,6 @@ module.exports = {
   extractRawPreview,
   withProcessableImage,
   RAW_EXTENSIONS,
+  DEFAULT_THUMBNAIL_WIDTH,
+  DEFAULT_THUMBNAIL_HEIGHT,
 };

backend/src/services/restoreService.js

@@ -22,6 +22,85 @@ function pathEscapes(baseDir, candidate) {
   const rel = path.relative(path.resolve(baseDir), path.resolve(candidate));
   return !rel || rel === '..' || rel.startsWith('..' + path.sep) || path.isAbsolute(rel);
 }
+
+// GHSA-xfvx: `manifest.database.backup_file` is just as attacker-influenceable
+// as the file-manifest entries `pathEscapes` guards above (hand-crafted or
+// tampered backup manifest) — an absolute path or a `..`-laden relative one
+// must not be allowed to point the SQLite/PG restore at an arbitrary file on
+// disk. Resolve the SAME operator-configured backup roots that
+// `adminRestore.js`'s `checkRestorePathsAllowed` (GHSA-fw4c) enforces for the
+// top-level `source`/`manifestPath` request fields, plus the already-trusted
+// `backupPath` this restore run resolved to (always included, so this never
+// fails open even when no backup_destination_path/backup_manifest_path is
+// configured yet).
+async function getConfiguredBackupRoots(trustedRoot) {
+  const roots = [];
+  if (trustedRoot) roots.push(trustedRoot);
+  try {
+    const rows = await db('app_settings')
+      .whereIn('setting_key', ['backup_destination_path', 'backup_manifest_path'])
+      .select('setting_value');
+    for (const row of rows) {
+      let value;
+      try { value = JSON.parse(row.setting_value); } catch (_) { value = row.setting_value; }
+      if (value) roots.push(value);
+    }
+  } catch (_) {
+    // best effort — fall through to whatever roots we already have
+  }
+  for (const extra of (process.env.RESTORE_ALLOWED_ROOTS || '').split(':')) {
+    if (extra.trim()) roots.push(extra.trim());
+  }
+  return roots.map((r) => path.resolve(r));
+}
+
+function isContainedInRoots(candidate, resolvedRoots) {
+  const resolved = path.resolve(candidate);
+  return resolvedRoots.some(
+    (root) => resolved === root || resolved.startsWith(root + path.sep)
+  );
+}
+
+// sqlite3's `.restore`/`.backup` are dot-commands parsed by sqlite3's OWN
+// tokenizer, not the shell — spawn()'s argv separation (shell: false) does
+// NOT protect against a single quote embedded in the path breaking out of
+// the `.restore '<path>'` argument, since the whole `.restore '<path>'`
+// string is one argv element that sqlite3 re-parses itself. sqlite3 offers
+// no parameterized dot-command form, so constrain the path to a
+// conservative safe charset before it is ever interpolated (GHSA-xfvx).
+const SAFE_SQLITE_PATH_RE = /^[A-Za-z0-9._/-]+$/;
+function assertSafeSqlitePath(p) {
+  if (typeof p !== 'string' || !SAFE_SQLITE_PATH_RE.test(p)) {
+    throw new Error(`Refusing to run sqlite3 against an unsafe path: ${p}`);
+  }
+}
+
+// GHSA-xfvx: the layered candidate resolution for `manifest.database.backup_file`
+// (see performDatabaseRestore), factored out so the containment rule can be
+// pinned directly in tests without exercising the surrounding DB-swap/spawn
+// side effects. `warn` is an optional `(msg, meta) => void` logger hook.
+async function resolveContainedDbBackupCandidates(backupPath, dbBackupFile, warn) {
+  const allowedRoots = await getConfiguredBackupRoots(backupPath);
+  const rawCandidates = [
+    // (1) Honour absolute paths recorded by the dumper.
+    path.isAbsolute(dbBackupFile) ? dbBackupFile : null,
+    // (2) Relative-to-backupPath as-stored (no basename munging).
+    path.join(backupPath, dbBackupFile),
+    // (3) Legacy reconstruct. Inherently safe: path.basename() strips any
+    // directory component, so this candidate can never escape backupPath.
+    path.join(backupPath, 'database', path.basename(dbBackupFile)),
+  ].filter(Boolean);
+
+  return rawCandidates.filter((candidate) => {
+    const contained = isContainedInRoots(candidate, allowedRoots);
+    if (!contained && warn) {
+      warn('Refusing database backup candidate outside configured backup roots', {
+        candidate, dbBackupFile,
+      });
+    }
+    return contained;
+  });
+}
 const { formatBytes } = require('../utils/formatBytes');
 const os = require('os');
 
@@ -959,14 +1038,26 @@ class RestoreService {
     // `Database backup file not found: local/database/...sql.gz`
     // even though the file existed at exactly the path the manifest
     // recorded.
-    const candidates = [
-      // (1) Honour absolute paths recorded by the dumper.
-      path.isAbsolute(dbBackupFile) ? dbBackupFile : null,
-      // (2) Relative-to-backupPath as-stored (no basename munging).
-      path.join(backupPath, dbBackupFile),
-      // (3) Legacy reconstruct.
-      path.join(backupPath, 'database', path.basename(dbBackupFile)),
-    ].filter(Boolean);
+    // GHSA-xfvx: `dbBackupFile` comes straight out of the manifest, which is
+    // attacker-influenceable (hand-crafted or tampered backup). Neither
+    // candidate (1) nor (2) below used to be checked for containment, so a
+    // manifest could point `.restore` at an arbitrary file anywhere on disk
+    // (absolute path, or `../../` traversal through the path.join). Resolve
+    // each candidate and drop any that escape the configured backup roots
+    // BEFORE it's ever fs.access'd/candidate-listed. Candidate (3) is
+    // inherently safe (path.basename() strips any directory component) and
+    // is always inside `backupPath`, which is itself always one of the
+    // allowed roots below.
+    const candidates = await resolveContainedDbBackupCandidates(
+      backupPath, dbBackupFile, (msg, meta) => this.log('warn', msg, meta)
+    );
+
+    if (candidates.length === 0) {
+      throw new Error(
+        'Database backup file path is not inside a configured backup location. ' +
+        `Manifest recorded path: ${dbBackupFile}.`
+      );
+    }
 
     let dbBackupPath = null;
     for (const candidate of candidates) {
@@ -1036,7 +1127,12 @@ class RestoreService {
         await fs.copyFile(dbPath, currentBackup);
         
         try {
-          // Restore from backup
+          // Restore from backup. `restoreFile` is contained-checked above,
+          // but the FILENAME component still comes from the manifest — a
+          // quote in it would break out of the `.restore '<path>'` dot-
+          // command sqlite3 parses (GHSA-xfvx). Charset-validate right
+          // before use as the final gate.
+          assertSafeSqlitePath(restoreFile);
           await spawnAsync('sqlite3', [dbPath, `.restore '${restoreFile}'`]);
 
           // Verify integrity
@@ -1535,6 +1631,10 @@ END $$;`
 
         if (this.dbType === 'sqlite') {
           const dbPath = knexConfig.connection.filename;
+          // Defense in depth: same dot-command injection surface as the
+          // main restore path (GHSA-xfvx), even though this path is
+          // internally generated rather than manifest-controlled.
+          assertSafeSqlitePath(decompressedPath);
           await spawnAsync('sqlite3', [dbPath, `.restore '${decompressedPath}'`]);
         } else {
           const { host, port, user, password, database } = knexConfig.connection;
@@ -1805,5 +1905,14 @@ const restoreService = new RestoreService();
 
 module.exports = {
   restoreService,
-  RestoreService // Export class for testing
+  RestoreService, // Export class for testing
+  // Exposed for tests: the manifest `database.backup_file` containment +
+  // sqlite dot-command charset rules (GHSA-xfvx) are worth pinning directly.
+  _internal: {
+    getConfiguredBackupRoots,
+    isContainedInRoots,
+    assertSafeSqlitePath,
+    pathEscapes,
+    resolveContainedDbBackupCandidates,
+  },
 };

backend/src/services/videoProcessor.js

@@ -32,7 +32,10 @@ async function extractVideoMetadata(videoPath) {
         const audioStream = metadata.streams.find(s => s.codec_type === 'audio');
 
         const result = {
-          duration: Math.floor(metadata.format.duration || 0),
+          // null (not 0) when ffprobe genuinely has no duration — a real
+          // 0-second clip and "unknown" must stay distinguishable, since
+          // downstream code treats `duration != null` as "trust this value".
+          duration: metadata.format.duration != null ? Math.floor(metadata.format.duration) : null,
           width: videoStream?.width || null,
           height: videoStream?.height || null,
           videoCodec: videoStream?.codec_name || null,
@@ -130,35 +133,108 @@ async function getVideoDuration(videoPath) {
  * Process an uploaded video: extract metadata and produce a thumbnail through
  * the storage backend.
  *
+ * Metadata extraction and thumbnail generation are independent, best-effort
+ * steps — mirroring how the image pipeline treats thumbnail/dimension/EXIF
+ * failures (log a warning, keep the upload). This used to gate everything
+ * behind isValidVideo(), which rejects the whole video if ffprobe can't read
+ * even one of duration/width/height — common on some iPhone/Lightroom-
+ * exported MP4s (#1370). Callers (photoProcessor.js's processPhoto and
+ * processUploadedPhotos) already catch that throw and fall back to a static
+ * placeholder thumbnail plus a metadata-only retry (codex review of #845),
+ * but that fallback never got a REAL thumbnail even when
+ * generateVideoThumbnail() would have succeeded on its own — thumbnailing
+ * doesn't need valid duration/width/height, it just seeks and grabs a frame.
+ * Trying both steps independently means a real thumbnail (and whatever
+ * metadata ffprobe *can* read) survives far more often. metadata is still
+ * allowed to come back null (ffprobe failed) — a video with no thumbnail
+ * would fall back to rendering the raw video as an <img> in the gallery
+ * grid (`photo.thumbnail_url || photo.url`), so this only resolves when a
+ * real thumbnail or the SVG placeholder produced *something*; if both fail
+ * (storage backend down, disk full — not a quirk of one file) it throws
+ * instead, so the caller surfaces a retryable failure rather than silently
+ * completing with nothing to show.
+ *
  * @param {string} videoPath - Local path to the source video (ffmpeg requires fs).
  * @param {string} thumbnailKey - Relative storage key for the thumbnail.
- * @returns {Promise<{success: boolean, metadata: Object, thumbnailKey: string}>}
+ * @returns {Promise<{success: boolean, metadata: Object|null, thumbnailKey: string}>}
  */
 async function processUploadedVideo(videoPath, thumbnailKey, options = {}) {
+  let metadata = null;
   try {
-    const isValid = await isValidVideo(videoPath);
-    if (!isValid) {
-      throw new Error('Invalid video file');
-    }
-
-    const metadata = await extractVideoMetadata(videoPath);
-    await generateVideoThumbnail(videoPath, thumbnailKey, options);
-
-    const storage = getStorage();
-    const exists = await storage.exists(thumbnailKey);
-    if (!exists) {
-      throw new Error('Thumbnail generation failed (not in storage)');
-    }
-
-    return {
-      success: true,
-      metadata,
-      thumbnailKey
-    };
+    metadata = await extractVideoMetadata(videoPath);
   } catch (error) {
-    logger.error('Error processing video', { error: error.message, videoPath });
-    throw error;
+    logger.error('Video metadata extraction failed — continuing without duration/codec/dimensions', {
+      error: error.message,
+      videoPath
+    });
   }
+
+  let generatedThumbnailKey = null;
+  try {
+    await generateVideoThumbnail(videoPath, thumbnailKey, options);
+    const storage = getStorage();
+    if (await storage.exists(thumbnailKey)) {
+      generatedThumbnailKey = thumbnailKey;
+    }
+  } catch (error) {
+    logger.error('Video thumbnail generation failed — continuing without a thumbnail', {
+      error: error.message,
+      videoPath
+    });
+  }
+
+  // Never return "success" with no thumbnail at all: the gallery grid
+  // (GridGalleryLayout/JustifiedGalleryLayout) falls back to
+  // `photo.thumbnail_url || photo.url` when there's no thumbnail, which
+  // makes AuthenticatedImage download the full ORIGINAL VIDEO and try to
+  // render it as an <img> — a broken tile and a multi-GB fetch just from
+  // opening the gallery (codex review, #1371/#1372). Fall back to the same
+  // ffmpeg-free SVG placeholder the callers already generate for a total
+  // processing failure, so a bare thumbnail-generation failure degrades to
+  // that placeholder too, not to "no thumbnail". thumbnailKey is always
+  // `thumbnails/thumb_<name>.jpg` (see callers) — strip the prefix back to
+  // a filename so generateVideoPlaceholder recomputes this exact same key.
+  if (!generatedThumbnailKey) {
+    try {
+      const {
+        generateVideoPlaceholder,
+        DEFAULT_THUMBNAIL_WIDTH,
+        DEFAULT_THUMBNAIL_HEIGHT
+      } = require('./imageProcessor');
+      const placeholderFilename = path.basename(thumbnailKey).replace(/^thumb_/, '');
+      // Explicit width/height make generateVideoPlaceholder skip its
+      // configured-thumbnail-size DB lookup (see its own comment) — this
+      // call can run from inside processUploadedPhotos' open per-file
+      // SQLite transaction, where that lookup would otherwise deadlock.
+      const placeholderKey = await generateVideoPlaceholder(placeholderFilename, {
+        width: DEFAULT_THUMBNAIL_WIDTH,
+        height: DEFAULT_THUMBNAIL_HEIGHT
+      });
+      if (placeholderKey) {
+        generatedThumbnailKey = placeholderKey;
+      }
+    } catch (error) {
+      logger.error('Video placeholder generation also failed', { error: error.message, videoPath });
+    }
+  }
+
+  // A real thumbnail AND the ffmpeg-free SVG placeholder both failing points
+  // at something systemic (storage backend down, disk full) rather than a
+  // quirk of this one file — that's worth surfacing as a retryable failure
+  // rather than silently completing with no thumbnail at all, which would
+  // make the gallery fall back to rendering the raw video as an <img>
+  // (codex review, #1371/#1372). Metadata (if any was extracted) is lost
+  // here, same trade-off the callers' own pre-existing total-failure
+  // handling already makes.
+  if (!generatedThumbnailKey) {
+    throw new Error('Unable to generate any thumbnail (real or placeholder) for this video');
+  }
+
+  return {
+    success: true,
+    metadata,
+    thumbnailKey: generatedThumbnailKey
+  };
 }
 
 /**

frontend/package.json

@@ -1,7 +1,7 @@
 {
   "name": "picpeak-frontend",
   "private": true,
-  "version": "3.131.1-beta.0",
+  "version": "3.131.3-beta.0",
   "type": "module",
   "scripts": {
     "dev": "vite",