DC-059: Joi validation middleware + schemas for destructive routes
CI / Test & Lint (push) Has been cancelled
CI / Security audit (push) Has been cancelled

[grade=B]

- New src/utilities/validate.js: validateBody(schema) middleware + 9 schemas
  (backupConfigUpdate, backupScheduleCreate, backupRestore, backupRestoreFile,
   appDeploy, appRestore, appRevert, assetUpload, logoUpload)
- Uses Joi's authoritative CIDR validator (rejects malformed IPv6 like ::::/64
  that the previous hex/colon regex would have accepted)
- appDeploy.config uses .unknown(true) for forward-compat with template-specific
  fields (sslType, dnsType, plexClaimToken, etc.) — preserves fields the live
  frontend posts, prevents a behavioural regression
- appRestore uses Joi.any().custom() so the empty-body semantics hold under
  middleware stripUnknown (default) — body with extra keys now rejected
- Wired into 8 destructive routes: backups schedule/restore/config, apps
  deploy/restore/revert, assets upload/logo
- Duplicate legacy POST /backups/schedule handler (line 519) marked LEGACY
  with TODO removal note (Express only matches first registration; this
  handler is unreachable under normal routing)
- Removed redundant manual appId check in /backups/schedule (Joi schema
  enforces it)
- Removed unused 'mime' destructure in /assets/favicon (decodeImageData
  validates MIME internally)
- 41 unit tests covering every exported schema + middleware integration
- 1539/1539 Jest tests pass, zero new ESLint warnings
This commit is contained in:
Hermes
2026-08-08 15:39:48 -07:00
parent c1358df0ec
commit a667de7920
8 changed files with 291 additions and 67 deletions
+12 -10
View File
@@ -3,6 +3,7 @@ const fsp = require('fs').promises;
const fs = require('fs');
const path = require('path');
const { success } = require('../src/utils/responses');
const { validateBody, schemas } = require('../src/utilities/validate');
const DEFAULT_BACKUP_DIR = process.env.BACKUP_DIR || path.join(__dirname, '..', 'backups');
const DEFAULT_MAX_STORAGE_BYTES = process.env.BACKUP_MAX_STORAGE_BYTES
@@ -56,14 +57,10 @@ module.exports = function({ backupManager, licenseManager, asyncHandler }) {
}, 'backups-schedule-list'));
// Create or update a scheduled backup for an app
router.post('/backups/schedule', premiumGating, asyncHandler(async (req, res) => {
router.post('/backups/schedule', premiumGating, validateBody(schemas.backupScheduleCreate), asyncHandler(async (req, res) => {
// appId is guaranteed present by the Joi schema (backupScheduleCreate requires it)
const { appId, enabled, schedule, retention, runImmediately, destination, destinationPath, maxStorageBytes } = req.body;
if (!appId) {
const { ValidationError } = require('../src/utilities/errors');
throw new ValidationError('appId is required');
}
const config = backupManager.getConfig();
if (!config.backups) config.backups = {};
@@ -234,7 +231,7 @@ module.exports = function({ backupManager, licenseManager, asyncHandler }) {
}, 'backups-files-app'));
// Restore from a specific backup file on disk
router.post('/backups/restore-file/:filename', asyncHandler(async (req, res) => {
router.post('/backups/restore-file/:filename', validateBody(schemas.backupRestoreFile), asyncHandler(async (req, res) => {
const { filename } = req.params;
const { encryptionKey, restartContainers } = req.body || {};
@@ -483,7 +480,7 @@ module.exports = function({ backupManager, licenseManager, asyncHandler }) {
}, 'backups-config-get'));
// Update backup configuration
router.post('/backups/config', asyncHandler(async (req, res) => {
router.post('/backups/config', validateBody(schemas.backupConfigUpdate), asyncHandler(async (req, res) => {
// P0-3 fix: was `backupManager.updateConfig(req.body)` which allowed
// arbitrary keys from HTTP request body to be merged into persisted config.
// Now destructure only the two known top-level fields.
@@ -515,6 +512,11 @@ module.exports = function({ backupManager, licenseManager, asyncHandler }) {
}, 'backups-storage-info'));
// Schedule a backup
// LEGACY: this is a duplicate registration of POST /backups/schedule (see also line 60,
// which uses the appId-keyed schema and is the route the frontend actually calls).
// Express only matches the first registered handler per METHOD+PATH, so this handler
// is unreachable. It is preserved for now to avoid removing a route any unknown
// integration might still POST to. TODO: audit + remove in a dedicated cleanup PR.
router.post('/backups/schedule', asyncHandler(async (req, res) => {
const { name, schedule, maxStorageBytes, ...backupConfig } = req.body;
@@ -539,10 +541,10 @@ module.exports = function({ backupManager, licenseManager, asyncHandler }) {
backupManager.updateConfig(config);
success(res, { message: `Backup '${name}' scheduled`, maxStorageBytes: maxBytes });
}, 'backups-schedule'));
}, 'backups-schedule-legacy'));
// Restore from backup
router.post('/backups/restore/:backupId', asyncHandler(async (req, res) => {
router.post('/backups/restore/:backupId', validateBody(schemas.backupRestore), asyncHandler(async (req, res) => {
const result = await backupManager.restoreBackup(req.params.backupId, req.body);
success(res, { result });
}, 'backups-restore'));