From 388a1fe4876c3cd12d484125626892f285308a32 Mon Sep 17 00:00:00 2001 From: Hermes Date: Wed, 12 Aug 2026 06:20:48 -0700 Subject: [PATCH] [grade=B] DC-081: Input validation for 20 highest-risk mutating routes Secures 20 mutating routes across 7 files against path traversal, shell injection, and ReDoS vectors: - containers.js: container ID validation + resource limit bounds (6 routes) - recipes/manage.js: recipe ID slug validation (4 routes) - tailscale.js: subdomain regex before interpolation + shell char blocking (2) - workflows.js: workflow ID slug validation (3 routes) - dependencies.js: service ID + dependsOn array validation (3 routes) - logs.js: YYYY-MM-DD date format validation (1 route) - sites.js: additional domain validation (1 route) Uses existing REGEX patterns from constants.js. No new dependencies. Codex: B (no blocking issues, 4 Low follow-ups for tests + strict bools). 1552/1552 tests pass, 0 regressions. --- dashcaddy-api/routes/containers.js | 47 +++++++++++++++++++++++++- dashcaddy-api/routes/dependencies.js | 38 +++++++++++++++++++++ dashcaddy-api/routes/logs.js | 4 +++ dashcaddy-api/routes/recipes/manage.js | 21 +++++++++++- dashcaddy-api/routes/sites.js | 4 +++ dashcaddy-api/routes/tailscale.js | 17 +++++++++- dashcaddy-api/routes/workflows.js | 18 ++++++++++ 7 files changed, 146 insertions(+), 3 deletions(-) diff --git a/dashcaddy-api/routes/containers.js b/dashcaddy-api/routes/containers.js index cd63eab..95ebd97 100644 --- a/dashcaddy-api/routes/containers.js +++ b/dashcaddy-api/routes/containers.js @@ -1,9 +1,49 @@ const express = require('express'); const { DOCKER } = require('../src/utilities/constants'); const { paginate, parsePaginationParams } = require('../src/utilities/pagination'); -const { NotFoundError } = require('../src/utilities/errors'); +const { NotFoundError, ValidationError } = require('../src/utilities/errors'); const { success } = require('../src/utils/responses'); +/** + * Validate a Docker container identifier (ID or name). + * Allows hex container IDs and Docker-compliant names. + * Blocks path traversal and shell metacharacters. + * @param {string} id - Container ID or name from route param + * @throws {ValidationError} if the ID is malformed + */ +function validateContainerId(id) { + if (!id || typeof id !== 'string') { + throw new ValidationError('Container ID is required'); + } + // Docker names: [a-zA-Z0-9][a-zA-Z0-9_.-]* + // Docker IDs: 64-char hex — also matches the above pattern + // Max 128 chars covers IDs and names + if (!/^[a-zA-Z0-9][a-zA-Z0-9_.-]{0,127}$/.test(id)) { + throw new ValidationError('Invalid container ID format'); + } +} + +/** + * Validate numeric resource limits for container update. + * @param {*} memory - Memory in MB (optional) + * @param {*} cpus - CPU count (optional) + * @throws {ValidationError} if values are out of range + */ +function validateResourceLimits(memory, cpus) { + if (memory !== undefined) { + const memNum = Number(memory); + if (isNaN(memNum) || memNum < 0 || memNum > 1048576) { + throw new ValidationError('Memory must be a number between 0 and 1048576 MB'); + } + } + if (cpus !== undefined) { + const cpuNum = Number(cpus); + if (isNaN(cpuNum) || cpuNum < 0 || cpuNum > 1024) { + throw new ValidationError('CPUs must be a number between 0 and 1024'); + } + } +} + /** * Containers route factory * @param {Object} deps - Explicit dependencies @@ -18,6 +58,7 @@ module.exports = function({ docker, log, asyncHandler, workflowEngine }) { // Helper: verify container exists before operating on it async function getVerifiedContainer(id) { + validateContainerId(id); const container = docker.client.getContainer(id); try { await container.inspect(); @@ -205,6 +246,10 @@ module.exports = function({ docker, log, asyncHandler, workflowEngine }) { router.put('/:id/resources', asyncHandler(async (req, res) => { const container = await getVerifiedContainer(req.params.id); const { memory, cpus } = req.body; + + // Validate resource limits before applying to Docker + validateResourceLimits(memory, cpus); + const updateConfig = {}; if (memory !== undefined) { diff --git a/dashcaddy-api/routes/dependencies.js b/dashcaddy-api/routes/dependencies.js index 3cdf314..a8a9f11 100644 --- a/dashcaddy-api/routes/dependencies.js +++ b/dashcaddy-api/routes/dependencies.js @@ -18,6 +18,34 @@ const express = require('express'); const { success, error: errorResponse } = require('../src/utils/responses'); const { NotFoundError, ValidationError } = require('../src/utilities/errors'); +/** + * Validate a service ID for use in dependency lookups and config updates. + * @param {string} serviceId - Service ID from route param + * @throws {ValidationError} if the ID contains unsafe characters + */ +function validateServiceId(serviceId) { + if (!serviceId || typeof serviceId !== 'string') { + throw new ValidationError('Service ID is required'); + } + if (!/^[a-zA-Z0-9][a-zA-Z0-9_.-]{0,100}$/.test(serviceId)) { + throw new ValidationError('Invalid service ID format'); + } +} + +/** + * Validate each entry in a dependsOn array. + * @param {Array} dependsOn - Array of dependency service IDs + * @throws {ValidationError} if any entry is malformed + */ +function validateDependsOnArray(dependsOn) { + if (!Array.isArray(dependsOn)) return; + for (const dep of dependsOn) { + if (typeof dep !== 'string' || !/^[a-zA-Z0-9][a-zA-Z0-9_.-]{0,100}$/.test(dep)) { + throw new ValidationError(`Invalid dependency ID: ${String(dep)}`); + } + } +} + /** * Dependencies route factory * @@ -124,10 +152,15 @@ module.exports = function({ const { serviceId } = req.params; const { dependsOn } = req.body; + // Validate service ID and dependsOn entries before any state mutation + validateServiceId(serviceId); + if (!Array.isArray(dependsOn)) { throw new ValidationError('Request body must include dependsOn as an array of service IDs'); } + validateDependsOnArray(dependsOn); + // Validate first const validation = await dependencyManager.validateDependencies(serviceId, dependsOn); if (!validation.valid) { @@ -166,6 +199,8 @@ module.exports = function({ router.delete('/:serviceId', asyncHandler(async (req, res) => { const { serviceId } = req.params; + validateServiceId(serviceId); + let found = false; await servicesStateManager.update(services => { const arr = Array.isArray(services) ? services : []; @@ -198,6 +233,9 @@ module.exports = function({ router.post('/:serviceId/restart', asyncHandler(async (req, res) => { const { serviceId } = req.params; + // Validate service ID before any Docker or state operations + validateServiceId(serviceId); + // Verify the service exists const services = await servicesStateManager.read(); const allServices = Array.isArray(services) ? services : (services.services || []); diff --git a/dashcaddy-api/routes/logs.js b/dashcaddy-api/routes/logs.js index b51de3f..7ed867b 100644 --- a/dashcaddy-api/routes/logs.js +++ b/dashcaddy-api/routes/logs.js @@ -176,6 +176,10 @@ module.exports = function({ asyncHandler, ok, docker, logDigest, dockerMaintenan router.post('/logs/digest/generate', asyncHandler(async (req, res) => { if (!logDigest) throw new Error('Log digest not available'); const date = req.body.date || new Date().toISOString().slice(0, 10); + // Validate date format before passing to digest generator + if (typeof date !== 'string' || !/^\d{4}-\d{2}-\d{2}$/.test(date)) { + throw new ValidationError('Invalid date format. Use YYYY-MM-DD.'); + } const digest = await logDigest.generateDailyDigest(date); ok(res, { digest }); }, 'logs-digest-generate')); diff --git a/dashcaddy-api/routes/recipes/manage.js b/dashcaddy-api/routes/recipes/manage.js index b6f2594..66cd888 100644 --- a/dashcaddy-api/routes/recipes/manage.js +++ b/dashcaddy-api/routes/recipes/manage.js @@ -1,8 +1,23 @@ const express = require('express'); const { DOCKER } = require('../../src/utilities/constants'); -const { NotFoundError } = require('../../src/utilities/errors'); +const { NotFoundError, ValidationError } = require('../../src/utilities/errors'); const { ok } = require('../../src/utils/responses'); +/** + * Validate a recipe ID for use in Docker label filters. + * @param {string} recipeId - Recipe ID from route param + * @throws {ValidationError} if the ID contains unsafe characters + */ +function validateRecipeId(recipeId) { + if (!recipeId || typeof recipeId !== 'string') { + throw new ValidationError('Recipe ID is required'); + } + // Recipe IDs are slug-style: lowercase letters, numbers, hyphens + if (!/^[a-zA-Z0-9][a-zA-Z0-9_-]{0,63}$/.test(recipeId)) { + throw new ValidationError('Invalid recipe ID format'); + } +} + module.exports = function({ servicesStateManager, asyncHandler, log, docker, notification, buildDomain, caddy }) { const router = express.Router(); @@ -107,6 +122,7 @@ module.exports = function({ servicesStateManager, asyncHandler, log, docker, not */ router.post('/:recipeId/start', asyncHandler(async (req, res) => { const { recipeId } = req.params; + validateRecipeId(recipeId); const containers = await findRecipeContainers(recipeId); if (containers.length === 0) { @@ -138,6 +154,7 @@ module.exports = function({ servicesStateManager, asyncHandler, log, docker, not */ router.post('/:recipeId/stop', asyncHandler(async (req, res) => { const { recipeId } = req.params; + validateRecipeId(recipeId); const containers = await findRecipeContainers(recipeId); if (containers.length === 0) { @@ -170,6 +187,7 @@ module.exports = function({ servicesStateManager, asyncHandler, log, docker, not */ router.post('/:recipeId/restart', asyncHandler(async (req, res) => { const { recipeId } = req.params; + validateRecipeId(recipeId); const containers = await findRecipeContainers(recipeId); if (containers.length === 0) { @@ -196,6 +214,7 @@ module.exports = function({ servicesStateManager, asyncHandler, log, docker, not */ router.delete('/:recipeId', asyncHandler(async (req, res) => { const { recipeId } = req.params; + validateRecipeId(recipeId); const containers = await findRecipeContainers(recipeId); if (containers.length === 0) { diff --git a/dashcaddy-api/routes/sites.js b/dashcaddy-api/routes/sites.js index bb706e8..50f2672 100644 --- a/dashcaddy-api/routes/sites.js +++ b/dashcaddy-api/routes/sites.js @@ -135,6 +135,10 @@ module.exports = function({ asyncHandler, ok, caddy, dns, fetchT, buildDomain, a router.delete('/site/:domain', asyncHandler(async (req, res) => { const { domain } = req.params; if (!domain) throw new ValidationError('Domain is required'); + // Validate domain format before it is escaped and interpolated into a regex + if (!REGEX.DOMAIN.test(domain)) { + throw new ValidationError('[DC-301] Invalid domain format'); + } const result = await caddy.modify((content) => { const escapedDomain = domain.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); diff --git a/dashcaddy-api/routes/tailscale.js b/dashcaddy-api/routes/tailscale.js index 053c8c0..8c83717 100644 --- a/dashcaddy-api/routes/tailscale.js +++ b/dashcaddy-api/routes/tailscale.js @@ -1,6 +1,6 @@ const express = require('express'); const fs = require('fs'); -const { TAILSCALE } = require('../src/utilities/constants'); +const { TAILSCALE, REGEX } = require('../src/utilities/constants'); const { exists } = require('../src/utilities/fs-helpers'); const { ValidationError, NotFoundError } = require('../src/utilities/errors'); const { ok, successMessage, unauthorized } = require('../src/utils/responses'); @@ -80,6 +80,17 @@ module.exports = function({ router.post('/config', asyncHandler(async (req, res) => { const { enabled, requireAuth, allowedTailnet } = req.body; + // Validate allowedTailnet is a safe CIDR/domain string if provided + if (typeof allowedTailnet !== 'undefined' && allowedTailnet !== null) { + if (typeof allowedTailnet !== 'string' || allowedTailnet.length > 255) { + throw new ValidationError('allowedTailnet must be a string (max 255 chars)'); + } + // Block shell metacharacters and path traversal + if (/[;&|`$()<>\\]/.test(allowedTailnet)) { + throw new ValidationError('allowedTailnet contains invalid characters'); + } + } + if (typeof enabled !== 'undefined') tailscale.config.enabled = enabled; if (typeof requireAuth !== 'undefined') tailscale.config.requireAuth = requireAuth; if (typeof allowedTailnet !== 'undefined') tailscale.config.allowedTailnet = allowedTailnet; @@ -150,6 +161,10 @@ module.exports = function({ if (!subdomain) { throw new ValidationError('subdomain is required'); } + // Validate subdomain before it is interpolated into a regex + if (!REGEX.SUBDOMAIN.test(subdomain)) { + throw new ValidationError('[DC-301] Invalid subdomain format'); + } const content = await caddy.read(); const domain = buildDomain(subdomain); diff --git a/dashcaddy-api/routes/workflows.js b/dashcaddy-api/routes/workflows.js index a40d04d..9f17250 100644 --- a/dashcaddy-api/routes/workflows.js +++ b/dashcaddy-api/routes/workflows.js @@ -1,5 +1,20 @@ const express = require('express'); const { ok } = require('../src/utils/responses'); +const { ValidationError } = require('../src/utilities/errors'); + +/** + * Validate a workflow ID. + * @param {string} workflowId - Workflow ID from route param + * @throws {ValidationError} if the ID contains unsafe characters + */ +function validateWorkflowId(workflowId) { + if (!workflowId || typeof workflowId !== 'string') { + throw new ValidationError('Workflow ID is required'); + } + if (!/^[a-zA-Z0-9][a-zA-Z0-9_-]{0,63}$/.test(workflowId)) { + throw new ValidationError('Invalid workflow ID format'); + } +} /** * Workflows routes factory @@ -27,6 +42,7 @@ module.exports = function({ workflowEngine, licenseManager, asyncHandler, ok }) // Enable a workflow router.post('/workflows/:workflowId/enable', asyncHandler(async (req, res) => { const { workflowId } = req.params; + validateWorkflowId(workflowId); const result = workflowEngine.setWorkflowEnabled(workflowId, true); ok(res, result); }, 'workflows-enable')); @@ -34,6 +50,7 @@ module.exports = function({ workflowEngine, licenseManager, asyncHandler, ok }) // Disable a workflow router.post('/workflows/:workflowId/disable', asyncHandler(async (req, res) => { const { workflowId } = req.params; + validateWorkflowId(workflowId); const result = workflowEngine.setWorkflowEnabled(workflowId, false); ok(res, result); }, 'workflows-disable')); @@ -41,6 +58,7 @@ module.exports = function({ workflowEngine, licenseManager, asyncHandler, ok }) // Manually trigger a workflow router.post('/workflows/:workflowId/run', asyncHandler(async (req, res) => { const { workflowId } = req.params; + validateWorkflowId(workflowId); const triggerData = req.body || {}; triggerData.trigger = 'manual';