From 3a6d2ce93d5891e7bd68589b7d6bb5e5eebdbdf3 Mon Sep 17 00:00:00 2001 From: Sami Date: Fri, 6 Mar 2026 20:21:21 -0800 Subject: [PATCH] Fix input validation and error handling across API endpoints - Deploy endpoint: validate appId, config, and subdomain before use (prevents 500 crash on empty body) - Container ops: return 404 instead of 500 for non-existent containers - Update-subdomain: require oldSubdomain/newSubdomain fields (prevents false 200 with undefined values) - Global error handler: catch-all that never leaks stack traces or internal paths - API 404 catch-all: return JSON instead of HTML for unmatched /api/* routes Co-Authored-By: Claude Opus 4.6 --- dashcaddy-api/routes/apps/deploy.js | 9 +++++++++ dashcaddy-api/routes/apps/templates.js | 10 ++++++++++ dashcaddy-api/routes/containers.js | 25 ++++++++++++++++++++----- dashcaddy-api/server.js | 10 +++++++++- 4 files changed, 48 insertions(+), 6 deletions(-) diff --git a/dashcaddy-api/routes/apps/deploy.js b/dashcaddy-api/routes/apps/deploy.js index 01d6deb..b674dca 100644 --- a/dashcaddy-api/routes/apps/deploy.js +++ b/dashcaddy-api/routes/apps/deploy.js @@ -184,6 +184,15 @@ module.exports = function(ctx, helpers) { // Deploy new app router.post('/apps/deploy', ctx.asyncHandler(async (req, res) => { const { appId, config } = req.body; + if (!appId || typeof appId !== 'string') { + return ctx.errorResponse(res, 400, 'appId is required'); + } + if (!config || typeof config !== 'object') { + return ctx.errorResponse(res, 400, 'config object is required'); + } + if (!config.subdomain || typeof config.subdomain !== 'string') { + return ctx.errorResponse(res, 400, 'config.subdomain is required'); + } try { ctx.log.info('deploy', 'Deploying app', { appId, subdomain: config.subdomain }); const template = ctx.APP_TEMPLATES[appId]; diff --git a/dashcaddy-api/routes/apps/templates.js b/dashcaddy-api/routes/apps/templates.js index 03559e4..d1cef07 100644 --- a/dashcaddy-api/routes/apps/templates.js +++ b/dashcaddy-api/routes/apps/templates.js @@ -1,5 +1,6 @@ const express = require('express'); const { exists } = require('../../fs-helpers'); +const { REGEX } = require('../../constants'); module.exports = function(ctx, helpers) { const router = express.Router(); @@ -54,6 +55,15 @@ module.exports = function(ctx, helpers) { // Update subdomain for deployed app router.post('/apps/update-subdomain', ctx.asyncHandler(async (req, res) => { const { serviceId, oldSubdomain, newSubdomain, containerId, ip } = req.body; + if (!oldSubdomain || typeof oldSubdomain !== 'string') { + return ctx.errorResponse(res, 400, 'oldSubdomain is required'); + } + if (!newSubdomain || typeof newSubdomain !== 'string') { + return ctx.errorResponse(res, 400, 'newSubdomain is required'); + } + if (!REGEX.SUBDOMAIN.test(newSubdomain)) { + return ctx.errorResponse(res, 400, '[DC-301] Invalid subdomain format for newSubdomain'); + } ctx.log.info('deploy', 'Updating subdomain', { oldSubdomain, newSubdomain }); const results = { oldDns: null, newDns: null, caddy: null, service: null }; diff --git a/dashcaddy-api/routes/containers.js b/dashcaddy-api/routes/containers.js index a277e10..458adbe 100644 --- a/dashcaddy-api/routes/containers.js +++ b/dashcaddy-api/routes/containers.js @@ -1,27 +1,42 @@ const express = require('express'); const { DOCKER } = require('../constants'); const { paginate, parsePaginationParams } = require('../pagination'); +const { NotFoundError } = require('../errors'); module.exports = function(ctx) { const router = express.Router(); + // Helper: verify container exists before operating on it + async function getVerifiedContainer(id) { + const container = ctx.docker.client.getContainer(id); + try { + await container.inspect(); + } catch (err) { + if (err.statusCode === 404 || (err.message && err.message.includes('no such container'))) { + throw new NotFoundError(`Container ${id}`); + } + throw err; + } + return container; + } + // Start container router.post('/:id/start', ctx.asyncHandler(async (req, res) => { - const container = ctx.docker.client.getContainer(req.params.id); + const container = await getVerifiedContainer(req.params.id); await container.start(); res.json({ success: true, message: 'Container started' }); }, 'container-start')); // Stop container router.post('/:id/stop', ctx.asyncHandler(async (req, res) => { - const container = ctx.docker.client.getContainer(req.params.id); + const container = await getVerifiedContainer(req.params.id); await container.stop(); res.json({ success: true, message: 'Container stopped' }); }, 'container-stop')); // Restart container router.post('/:id/restart', ctx.asyncHandler(async (req, res) => { - const container = ctx.docker.client.getContainer(req.params.id); + const container = await getVerifiedContainer(req.params.id); await container.restart(); res.json({ success: true, message: 'Container restarted' }); }, 'container-restart')); @@ -147,7 +162,7 @@ module.exports = function(ctx) { // Get container logs router.get('/:id/logs', ctx.asyncHandler(async (req, res) => { - const container = ctx.docker.client.getContainer(req.params.id); + const container = await getVerifiedContainer(req.params.id); const logs = await container.logs({ stdout: true, stderr: true, @@ -159,7 +174,7 @@ module.exports = function(ctx) { // Delete container router.delete('/:id', ctx.asyncHandler(async (req, res) => { - const container = ctx.docker.client.getContainer(req.params.id); + const container = await getVerifiedContainer(req.params.id); await container.remove({ force: true }); res.json({ success: true, message: 'Container removed' }); }, 'container-delete')); diff --git a/dashcaddy-api/server.js b/dashcaddy-api/server.js index e6f1448..cf0baf0 100644 --- a/dashcaddy-api/server.js +++ b/dashcaddy-api/server.js @@ -1772,6 +1772,11 @@ app.get('/api/docs/spec', asyncHandler(async (req, res) => { } }, 'api-docs-spec')); +// JSON 404 catch-all for unmatched API routes +app.use('/api', (req, res) => { + res.status(404).json({ success: false, error: `Not found: ${req.method} ${req.path}` }); +}); + // Global error handler for typed errors app.use((err, req, res, next) => { if (err instanceof AppError) { @@ -1789,7 +1794,10 @@ app.use((err, req, res, next) => { errors: err.errors || undefined }); } - next(err); + // Catch-all: never leak stack traces or internal paths + const status = err.status || err.statusCode || 500; + log.error('server', 'Unhandled error', { error: err.message, path: req.path, method: req.method }); + res.status(status).json({ success: false, error: status === 413 ? 'Request payload too large' : 'An internal error occurred' }); }); // Export app for testing