* fix(security): stop caller-chosen database backup destination (GHSA-jw8m)
POST /api/admin/database-backup/backup forwarded req.body straight into
databaseBackupService.backup(), which merges options over config:
const { destinationPath = '/backup/database', ... } = { ...config, ...options }
destinationPath is not a persistable setting — the /config allowlist only
accepts database_backup_* keys — so the request body was its only source.
The built-in `admin` role holds backup.create but neither settings.edit nor
backup.restore, so it could aim a full DB dump (admin bcrypt hashes, gallery
password hashes, encrypted SMTP creds) at the PUBLIC /uploads static mount
(server.js mounts it with no auth middleware) and then fetch it
unauthenticated. Filed low; it is a privilege escalation to unauthenticated
disclosure.
Forward only the real knobs, and only when present so absent keys can't
override config defaults via spread.
* fix(security): backup/restore hardening — restore path allowlist, gunzip bound, manifest checksum keying (GHSA-fw4c, h652, hgp8)
- adminRestore /validate + /start: constrain caller-supplied source and
manifestPath to the operator-configured backup roots — the SAME set the
restore wizard discovers from — so disaster recovery from a rescued mount
still works, with RESTORE_ALLOWED_ROOTS as an escape hatch (GHSA-fw4c).
- restoreService.decompressFile: bound the EXPANDED size and abort the
pipeline when exceeded; default 50 GB, RESTORE_MAX_DECOMPRESSED_BYTES
overrides (GHSA-h652).
- backupManifest: BACKUP_MANIFEST_KEY upgrades new manifests to a keyed
HMAC (GHSA-hgp8). Deliberately opt-in and verify-if-present — the key
cannot live in the database because the database is inside the backup, so
a mandatory HMAC would lock operators out of the exact disaster-recovery
case this exists for.
Also fixes a pre-existing bug found while testing hgp8: the checksum passed
Object.keys().sort() as JSON.stringify's second argument, which is an array
REPLACER (a property allowlist applied at every depth), not a key sorter. All
nested keys — path, size, per-file checksum — were dropped before hashing, so
the file list sat outside the integrity check entirely and a manifest path
could be rewritten to ../../etc/passwd without disturbing the digest. Now
hashes a recursively-canonicalized copy, with the legacy serialization
accepted on validation so existing backups stay restorable.
* fix(security): codex round 2 — unbreak the restore wizard, share checksum verification, guard downgrades
- adminRestore: `source` is usually a SOURCE TYPE ('local'|'s3'|'upload'),
not a path — restoreService branches on those literals. The containment
check treated it as a path, so path.resolve('local') fell outside the
backup roots and BOTH /validate and /start returned 400, blocking every
normal restore. Type tokens are now excluded from the path check.
- backupManifest: extracted verifyManifestChecksum() as the single source of
truth for the legacy/keyed fallbacks. restoreService.performPreRestoreValidation
recomputed the digest itself with the default canonical+keyed settings,
which rejected EVERY backup written before this batch. It now delegates.
- backupManifest: guard the algorithm downgrade — with a key configured, an
attacker able to rewrite the backup store could strip checksum_algorithm,
edit the manifest and recompute a plain SHA-256 that verified. Opt-in via
BACKUP_MANIFEST_REQUIRE_KEYED so pre-key backups keep restoring by default.
* fix(security): codex round 3 — close two manifest-verification fail-opens (GHSA-hgp8)
verifyManifestChecksum returned valid for a manifest with no
verification.total_checksum at all, and restoreService only called it when
that field was present. Deleting the field was therefore a complete bypass of
the keying work: no digest check, no downgrade guard, no
BACKUP_MANIFEST_REQUIRE_KEYED. Every manifest this codebase writes stamps the
field, so an absent one now fails validation, and the call site invokes the
verifier unconditionally.
Second fail-open: the strict-mode rejection of an unkeyed manifest was gated on
`&& key`, so with BACKUP_MANIFEST_REQUIRE_KEYED=true and no BACKUP_MANIFEST_KEY
configured a plain SHA-256 manifest sailed through. Strict mode is a statement
about the operator's manifests, not about the host — it is exactly the fresh
disaster-recovery box that lacks the secret. The rejection no longer depends on
a key being present.
Claude-Session: https://claude.ai/code/session_01F211U4dDbEj4zXiyKbi9me
(cherry picked from commit 292a5b784acd7f47099aa234c1c2ea00050fca97)
---------
Co-authored-by: Paul Nothaft <[email protected]>
285 lines
8.6 KiB
JavaScript
285 lines
8.6 KiB
JavaScript
const express = require('express');
|
|
const router = express.Router();
|
|
const { adminAuth } = require('../middleware/auth');
|
|
const { requirePermission } = require('../middleware/permissions');
|
|
const { databaseBackupService } = require('../services/databaseBackup');
|
|
const { db } = require('../database/db');
|
|
const logger = require('../utils/logger');
|
|
const { getPagination } = require('../utils/routeHelpers');
|
|
|
|
// All routes require admin authentication
|
|
router.use(adminAuth);
|
|
|
|
/**
|
|
* Get database backup status and configuration
|
|
*/
|
|
router.get('/status', requirePermission('backup.view'), async (req, res) => {
|
|
try {
|
|
// Get configuration
|
|
const config = await databaseBackupService.getBackupConfig();
|
|
|
|
// Get recent backup history
|
|
const history = await databaseBackupService.getBackupHistory(10);
|
|
|
|
// Get current progress if running
|
|
const progress = databaseBackupService.getProgress();
|
|
|
|
// Calculate health status
|
|
const lastBackup = history[0];
|
|
const isHealthy = lastBackup && lastBackup.status === 'completed' &&
|
|
new Date(lastBackup.completed_at) > new Date(Date.now() - 48 * 60 * 60 * 1000); // Within 48 hours
|
|
|
|
res.json({
|
|
config,
|
|
isRunning: databaseBackupService.isRunning,
|
|
isHealthy,
|
|
currentProgress: progress,
|
|
lastBackup,
|
|
recentBackups: history,
|
|
dbType: databaseBackupService.dbType
|
|
});
|
|
} catch (error) {
|
|
logger.error('Failed to get database backup status:', error);
|
|
res.status(500).json({ error: 'Failed to get backup status' });
|
|
}
|
|
});
|
|
|
|
/**
|
|
* Update database backup configuration
|
|
*/
|
|
router.put('/config', requirePermission('backup.create'), async (req, res) => {
|
|
try {
|
|
const allowedSettings = [
|
|
'database_backup_enabled',
|
|
'database_backup_schedule',
|
|
'database_backup_destination_path',
|
|
'database_backup_compress',
|
|
'database_backup_validate_integrity',
|
|
'database_backup_include_checksums',
|
|
'database_backup_retention_days',
|
|
'database_backup_email_on_failure',
|
|
'database_backup_email_on_success'
|
|
];
|
|
|
|
const updates = [];
|
|
|
|
for (const [key, value] of Object.entries(req.body)) {
|
|
if (allowedSettings.includes(key)) {
|
|
// Check if setting exists
|
|
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),
|
|
updated_at: new Date()
|
|
});
|
|
} else {
|
|
await db('app_settings').insert({
|
|
setting_key: key,
|
|
setting_value: JSON.stringify(value),
|
|
setting_type: 'database_backup'
|
|
});
|
|
}
|
|
|
|
updates.push(key);
|
|
}
|
|
}
|
|
|
|
// Restart scheduled backups if enabled state changed
|
|
if (updates.includes('database_backup_enabled') || updates.includes('database_backup_schedule')) {
|
|
const { startScheduledBackups, stopScheduledBackups } = require('../services/databaseBackup');
|
|
stopScheduledBackups();
|
|
await startScheduledBackups();
|
|
}
|
|
|
|
res.json({
|
|
success: true,
|
|
updatedSettings: updates,
|
|
message: 'Database backup configuration updated successfully'
|
|
});
|
|
} catch (error) {
|
|
logger.error('Failed to update database backup config:', error);
|
|
res.status(500).json({ error: 'Failed to update configuration' });
|
|
}
|
|
});
|
|
|
|
/**
|
|
* Trigger manual database backup
|
|
*/
|
|
router.post('/backup', requirePermission('backup.create'), async (req, res) => {
|
|
try {
|
|
if (databaseBackupService.isRunning) {
|
|
return res.status(409).json({ error: 'Backup already in progress' });
|
|
}
|
|
|
|
// Start backup asynchronously
|
|
res.json({
|
|
success: true,
|
|
message: 'Database backup started',
|
|
trackingUrl: '/api/admin/database-backup/progress'
|
|
});
|
|
|
|
// Forward ONLY the real backup knobs (GHSA-jw8m). Passing req.body
|
|
// straight through let the caller set `destinationPath`, which the
|
|
// service merges over its config — so a backup.create holder (the
|
|
// `admin` role, which has neither settings.edit nor backup.restore)
|
|
// could dump the whole database into the PUBLIC /uploads static mount
|
|
// and fetch it unauthenticated, hashes and encrypted SMTP creds included.
|
|
// destinationPath is not a persistable setting; the request body was its
|
|
// only source, so dropping it here costs no legitimate behaviour.
|
|
const body = req.body || {};
|
|
const options = {};
|
|
for (const key of ['compress', 'validateIntegrity', 'includeChecksums']) {
|
|
if (body[key] !== undefined) options[key] = body[key];
|
|
}
|
|
databaseBackupService.backup(options).catch(error => {
|
|
logger.error('Manual database backup failed:', error);
|
|
});
|
|
} catch (error) {
|
|
logger.error('Failed to start database backup:', error);
|
|
res.status(500).json({ error: 'Failed to start backup' });
|
|
}
|
|
});
|
|
|
|
/**
|
|
* Get current backup progress
|
|
*/
|
|
router.get('/progress', requirePermission('backup.view'), async (req, res) => {
|
|
try {
|
|
const progress = databaseBackupService.getProgress();
|
|
|
|
res.json({
|
|
isRunning: databaseBackupService.isRunning,
|
|
progress
|
|
});
|
|
} catch (error) {
|
|
logger.error('Failed to get backup progress:', error);
|
|
res.status(500).json({ error: 'Failed to get progress' });
|
|
}
|
|
});
|
|
|
|
/**
|
|
* Get backup history with pagination
|
|
*/
|
|
router.get('/history', requirePermission('backup.view'), async (req, res) => {
|
|
try {
|
|
const { page, limit, offset } = getPagination(req);
|
|
|
|
const [backups, totalCount] = await Promise.all([
|
|
db('database_backup_runs')
|
|
.orderBy('started_at', 'desc')
|
|
.limit(limit)
|
|
.offset(offset),
|
|
db('database_backup_runs').count('* as count').first()
|
|
]);
|
|
|
|
res.json({
|
|
backups,
|
|
pagination: {
|
|
page,
|
|
limit,
|
|
total: totalCount.count,
|
|
pages: Math.ceil(totalCount.count / limit)
|
|
}
|
|
});
|
|
} catch (error) {
|
|
logger.error('Failed to get backup history:', error);
|
|
res.status(500).json({ error: 'Failed to get history' });
|
|
}
|
|
});
|
|
|
|
/**
|
|
* Delete old backup files
|
|
*/
|
|
router.delete('/cleanup', requirePermission('backup.delete'), async (req, res) => {
|
|
try {
|
|
const { retentionDays = 30 } = req.body;
|
|
|
|
await databaseBackupService.cleanupOldBackups(retentionDays);
|
|
|
|
res.json({
|
|
success: true,
|
|
message: `Cleaned up backups older than ${retentionDays} days`
|
|
});
|
|
} catch (error) {
|
|
logger.error('Failed to cleanup old backups:', error);
|
|
res.status(500).json({ error: 'Failed to cleanup backups' });
|
|
}
|
|
});
|
|
|
|
/**
|
|
* Test database backup configuration
|
|
*/
|
|
router.post('/test', requirePermission('backup.create'), async (req, res) => {
|
|
try {
|
|
const config = await databaseBackupService.getBackupConfig();
|
|
|
|
// Test database connection
|
|
const testResults = {
|
|
databaseConnection: false,
|
|
destinationWritable: false,
|
|
compressionAvailable: true,
|
|
estimatedSize: null
|
|
};
|
|
|
|
// Test database connection
|
|
try {
|
|
await db.raw('SELECT 1');
|
|
testResults.databaseConnection = true;
|
|
} catch (error) {
|
|
testResults.databaseConnectionError = error.message;
|
|
}
|
|
|
|
// Test destination path
|
|
if (config.destinationPath) {
|
|
try {
|
|
const fs = require('fs').promises;
|
|
const testFile = `${config.destinationPath}/.test-${Date.now()}`;
|
|
await fs.writeFile(testFile, 'test');
|
|
await fs.unlink(testFile);
|
|
testResults.destinationWritable = true;
|
|
} catch (error) {
|
|
testResults.destinationError = error.message;
|
|
}
|
|
}
|
|
|
|
// Estimate database size
|
|
try {
|
|
testResults.estimatedSize = await databaseBackupService.getDatabaseSize();
|
|
} catch (error) {
|
|
testResults.sizeError = error.message;
|
|
}
|
|
|
|
res.json({
|
|
success: testResults.databaseConnection && testResults.destinationWritable,
|
|
results: testResults
|
|
});
|
|
} catch (error) {
|
|
logger.error('Failed to test backup configuration:', error);
|
|
res.status(500).json({ error: 'Failed to test configuration' });
|
|
}
|
|
});
|
|
|
|
/**
|
|
* Get table checksums
|
|
*/
|
|
router.get('/checksums', requirePermission('backup.view'), async (req, res) => {
|
|
try {
|
|
const checksums = await databaseBackupService.getTableChecksums();
|
|
|
|
res.json({
|
|
checksums,
|
|
tableCount: Object.keys(checksums).length,
|
|
totalRows: Object.values(checksums).reduce((sum, table) => sum + table.rowCount, 0)
|
|
});
|
|
} catch (error) {
|
|
logger.error('Failed to get table checksums:', error);
|
|
res.status(500).json({ error: 'Failed to get checksums' });
|
|
}
|
|
});
|
|
|
|
module.exports = router; |