From 316bcbd67965ddec74801308a722b506bb8da265 Mon Sep 17 00:00:00 2001 From: Paul Nothaft <53005142+the-luap@users.noreply.github.com> Date: Fri, 11 Sep 2026 10:40:54 +0200 Subject: [PATCH] fix(backend): contain and sanitize the SQLite restore source path (#1384) 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 ''` command, letting an attacker-chosen source file replace the live database. Co-authored-by: Paul Nothaft --- .../routes/restorePathAllowlist.test.js | 135 ++++++++++++-- ...oreService.sqliteRestorePathSafety.test.js | 173 ++++++++++++++++++ backend/src/routes/adminRestore.js | 60 +++++- backend/src/services/restoreService.js | 129 ++++++++++++- 4 files changed, 461 insertions(+), 36 deletions(-) create mode 100644 backend/__tests__/services/restoreService.sqliteRestorePathSafety.test.js diff --git a/backend/__tests__/routes/restorePathAllowlist.test.js b/backend/__tests__/routes/restorePathAllowlist.test.js index fbfff4eb..fb6e6d28 100644 --- a/backend/__tests__/routes/restorePathAllowlist.test.js +++ b/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 ''` — 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'); + } + }); +}); diff --git a/backend/__tests__/services/restoreService.sqliteRestorePathSafety.test.js b/backend/__tests__/services/restoreService.sqliteRestorePathSafety.test.js new file mode 100644 index 00000000..d073159c --- /dev/null +++ b/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 ''` 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); + }); + }); +}); diff --git a/backend/src/routes/adminRestore.js b/backend/src/routes/adminRestore.js index c0188d03..0e9bc35c 100644 --- a/backend/src/routes/adminRestore.js +++ b/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; } diff --git a/backend/src/services/restoreService.js b/backend/src/services/restoreService.js index 9e8743be..726a0f57 100644 --- a/backend/src/services/restoreService.js +++ b/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 ''` argument, since the whole `.restore ''` +// 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 ''` 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; @@ -1839,5 +1939,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, + }, }; \ No newline at end of file