Skip to content

Commit e917ed1

Browse files
committed
release: v6.14.1 — asyncHandler refactor + accidental info-leak fix
Post-Express-5 cleanup promised in v6.14.0 release notes. 175 async handlers across 21 route files now use a single asyncHandler() wrapper instead of the duplicated try/catch + res.status(500).json({ error: err.message }) boilerplate. Net diff: -521 LOC. Non-obvious security fix discovered in the process: The central error middleware at src/server.js:168 sanitizes 5xx responses (scrubs /home/ and /data/ paths, redacts credentials in URLs, replaces raw err.message with "Internal server error"). But the try/catch wrappers in 21 route files were BYPASSING that sanitizer by calling res.status(500).json({ error: err.message }) directly. After this release all generic 500s go through the sanitizer — no more accidental path/credential leaks in error messages. What's left alone (by design): handlers with dynamic status codes, non-generic catch shapes, 4xx-mapping logic, SSE callbacks, or business-logic in the catch block. 10 legitimate res.status(500) call sites remain — all inspected and confirmed non-generic. Tests: 740 passing / 4 skipped (unchanged). Lint: zero warnings on src/routes/.
1 parent 821d5a9 commit e917ed1

27 files changed

Lines changed: 1961 additions & 2384 deletions

CHANGELOG.md

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,75 @@
22

33
All notable changes to Docker Dash are documented here.
44

5+
## [6.14.1] - 2026-04-22 — "asyncHandler refactor (+ accidental info-leak fix)"
6+
7+
Post-v6.14.0 cleanup promised in the previous release notes: consolidate the try/catch + `res.status(500).json({ error: err.message })` boilerplate into a single `asyncHandler(fn)` wrapper. 175 handlers migrated across 21 route files. **Net diff: −521 LOC.**
8+
9+
### What this actually fixes (the non-obvious win)
10+
11+
Docker Dash's central error middleware at [src/server.js:168-190](src/server.js#L168-L190) already **sanitizes** 5xx responses — scrubs home/data paths, redacts URL credentials, and replaces the raw `err.message` with `'Internal server error'`. Until now, the try/catch wrappers in 21 route files **bypassed** that sanitization by calling `res.status(500).json({ error: err.message })` directly. So any backend error surfacing through those handlers was leaking the raw exception string to the client.
12+
13+
After this release, all generic 500 responses go through the central middleware → **no more accidental path or credential exposure in error messages.**
14+
15+
This wasn't the stated goal of the refactor (the goal was LOC reduction), but it's the more important outcome. Worth calling out for anyone reading the CHANGELOG looking for security-relevant deltas.
16+
17+
### Added — `src/utils/asyncHandler.js`
18+
19+
Four lines of utility:
20+
```js
21+
function asyncHandler(fn) {
22+
return (req, res, next) => Promise.resolve(fn(req, res, next)).catch(next);
23+
}
24+
```
25+
26+
Rejected promises now auto-forward to the Express 5 error middleware chain (where the existing sanitizer at line 168 takes over).
27+
28+
### Changed — 21 route files refactored (175 handler invocations)
29+
30+
Sample before/after from [src/routes/containers.js](src/routes/containers.js):
31+
32+
```js
33+
// Before
34+
router.get('/:id/inspect', requireAuth, async (req, res) => {
35+
try {
36+
const data = await dockerService.inspectContainer(req.params.id, req.hostId);
37+
res.json(data);
38+
} catch (err) {
39+
res.status(500).json({ error: err.message });
40+
}
41+
});
42+
43+
// After
44+
router.get('/:id/inspect', requireAuth, asyncHandler(async (req, res) => {
45+
const data = await dockerService.inspectContainer(req.params.id, req.hostId);
46+
res.json(data);
47+
}));
48+
```
49+
50+
### What was deliberately NOT unwrapped
51+
52+
Per the refactor brief, handlers with any of the following keep their try/catch blocks:
53+
- Dynamic status codes (e.g. `err.statusCode === 404 ? 404 : 500`)
54+
- Non-generic catch responses (extra fields like `{ error, steps: err.steps || [] }`)
55+
- 4xx-mapping catches (`err.message.includes('forbidden') ? 403 : 500`)
56+
- Callback-based async inside a handler (SSE streaming, `docker.loadImage`)
57+
- Catches that do additional business logic (`log.error(…)` then respond)
58+
59+
10 legitimate `res.status(500)` call sites remain — all inspected and confirmed non-generic.
60+
61+
### Verification
62+
63+
- **Tests:** 740 passing / 4 skipped (identical to v6.14.0 baseline).
64+
- **Lint:** `eslint src/routes/ --max-warnings 0` clean.
65+
- Behavior-preserving: clients keep receiving `{ error: "<sanitized message>" }` with 5xx status — the sanitization itself is the only behavior change, and that's an upgrade (not a downgrade) from the previous accidental leak.
66+
67+
### Files touched
68+
69+
- `src/utils/asyncHandler.js` (new)
70+
- 21 files in `src/routes/` — net −521 LOC
71+
72+
---
73+
574
## [6.14.0] - 2026-04-22 — "Express 4 → Express 5"
675

776
BACKLOG P2 item closed. Deep-spec ([plans/deep-spec-express5-migration.md](plans/deep-spec-express5-migration.md)) predicted 3-5h based on evidence that the codebase was already v5-idiomatic. Actual execution cost ~2h with one mid-flight snag (see below).

docker-compose.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ services:
44
context: .
55
dockerfile: Dockerfile
66
args:
7-
APP_VERSION: "${APP_VERSION:-6.14.0}"
8-
image: docker-dash:${APP_VERSION:-6.14.0}
7+
APP_VERSION: "${APP_VERSION:-6.14.1}"
8+
image: docker-dash:${APP_VERSION:-6.14.1}
99
container_name: docker-dash
1010
restart: unless-stopped
1111
env_file:
@@ -54,7 +54,7 @@ services:
5454
dd-egress-filter:
5555
build:
5656
context: ./docker/egress-filter
57-
image: docker-dash-egress-filter:${APP_VERSION:-6.14.0}
57+
image: docker-dash-egress-filter:${APP_VERSION:-6.14.1}
5858
container_name: dd-egress-filter
5959
restart: unless-stopped
6060
# Uses the default bridge so target containers on the default bridge can

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "docker-dash",
3-
"version": "6.14.0",
3+
"version": "6.14.1",
44
"description": "Full-featured Docker management dashboard",
55
"main": "src/server.js",
66
"scripts": {

public/js/pages/whatsnew.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,17 @@ const WhatsNewPage = {
99
// Add new releases at the TOP of this array.
1010
// Types: feature, fix, improvement, security, breaking
1111
_releases: [
12+
{
13+
version: '6.14.1',
14+
date: '2026-04-22',
15+
title: 'asyncHandler refactor + accidental info-leak fix',
16+
changes: [
17+
{ type: 'security', text: 'Non-obvious security fix discovered during the refactor: Docker Dash\'s central error middleware at src/server.js:168 already sanitizes 5xx responses (scrubs /home/ and /data/ paths, redacts credentials in URLs, replaces raw err.message with "Internal server error"). The 21 route files with try/catch wrappers were BYPASSING this sanitization by calling res.status(500).json({ error: err.message }) directly — leaking raw exception text to clients. All generic 500 handlers now go through the sanitizer.' },
18+
{ type: 'improvement', text: 'Post-Express-5 cleanup: new src/utils/asyncHandler.js (4 lines) wraps async handlers so rejected promises auto-forward to the error middleware. 175 handler invocations migrated across 21 route files. Net diff: -521 LOC of boilerplate gone.' },
19+
{ type: 'improvement', text: 'Handlers with dynamic status codes, non-generic catch shapes (extra fields), 4xx-mapping logic, SSE streaming, or business-logic in the catch block were LEFT ALONE intentionally — those aren\'t boilerplate, they\'re intentional error handling. 10 legitimate res.status(500) call sites remain.' },
20+
{ type: 'improvement', text: 'Tests: 740 passing / 4 skipped (unchanged). Lint: zero warnings on src/routes/.' },
21+
],
22+
},
1223
{
1324
version: '6.14.0',
1425
date: '2026-04-22',

src/routes/alerts.js

Lines changed: 25 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ const alertService = require('../services/alerts');
55
const auditService = require('../services/audit');
66
const { requireAuth, requireRole } = require('../middleware/auth');
77
const { getClientIp } = require('../utils/helpers');
8+
const asyncHandler = require('../utils/asyncHandler');
89

910
const router = Router();
1011

@@ -15,44 +16,36 @@ router.get('/rules/:id', requireAuth, (req, res) => {
1516
res.json(rule);
1617
});
1718

18-
router.post('/rules', requireAuth, requireRole('admin', 'operator'), (req, res) => {
19-
try {
20-
const result = alertService.createRule({ ...req.body, created_by: req.user.id });
21-
auditService.log({ userId: req.user.id, username: req.user.username,
22-
action: 'alert_rule_create', targetType: 'alert_rule', targetId: String(result.id), ip: getClientIp(req) });
23-
res.status(201).json(result);
24-
} catch (err) { res.status(500).json({ error: err.message }); }
25-
});
26-
27-
router.put('/rules/:id', requireAuth, requireRole('admin', 'operator'), (req, res) => {
28-
try {
29-
alertService.updateRule(parseInt(req.params.id), req.body);
30-
auditService.log({ userId: req.user.id, username: req.user.username,
31-
action: 'alert_rule_update', targetType: 'alert_rule', targetId: req.params.id, ip: getClientIp(req) });
32-
res.json({ ok: true });
33-
} catch (err) { res.status(500).json({ error: err.message }); }
34-
});
35-
36-
router.delete('/rules/:id', requireAuth, requireRole('admin'), (req, res) => {
37-
try {
38-
alertService.deleteRule(parseInt(req.params.id));
39-
auditService.log({ userId: req.user.id, username: req.user.username,
40-
action: 'alert_rule_delete', targetType: 'alert_rule', targetId: req.params.id, ip: getClientIp(req) });
41-
res.json({ ok: true });
42-
} catch (err) { res.status(500).json({ error: err.message }); }
43-
});
19+
router.post('/rules', requireAuth, requireRole('admin', 'operator'), asyncHandler((req, res) => {
20+
const result = alertService.createRule({ ...req.body, created_by: req.user.id });
21+
auditService.log({ userId: req.user.id, username: req.user.username,
22+
action: 'alert_rule_create', targetType: 'alert_rule', targetId: String(result.id), ip: getClientIp(req) });
23+
res.status(201).json(result);
24+
}));
25+
26+
router.put('/rules/:id', requireAuth, requireRole('admin', 'operator'), asyncHandler((req, res) => {
27+
alertService.updateRule(parseInt(req.params.id), req.body);
28+
auditService.log({ userId: req.user.id, username: req.user.username,
29+
action: 'alert_rule_update', targetType: 'alert_rule', targetId: req.params.id, ip: getClientIp(req) });
30+
res.json({ ok: true });
31+
}));
32+
33+
router.delete('/rules/:id', requireAuth, requireRole('admin'), asyncHandler((req, res) => {
34+
alertService.deleteRule(parseInt(req.params.id));
35+
auditService.log({ userId: req.user.id, username: req.user.username,
36+
action: 'alert_rule_delete', targetType: 'alert_rule', targetId: req.params.id, ip: getClientIp(req) });
37+
res.json({ ok: true });
38+
}));
4439

4540
router.get('/active', requireAuth, (req, res) => { res.json(alertService.getActiveAlerts()); });
4641
router.get('/history', requireAuth, (req, res) => {
4742
const { page, limit } = req.query;
4843
res.json(alertService.getAlertHistory({ page: parseInt(page) || 1, limit: parseInt(limit) || 50 }));
4944
});
5045

51-
router.post('/events/:id/acknowledge', requireAuth, (req, res) => {
52-
try {
53-
alertService.acknowledge(parseInt(req.params.id), req.user.id);
54-
res.json({ ok: true });
55-
} catch (err) { res.status(500).json({ error: err.message }); }
56-
});
46+
router.post('/events/:id/acknowledge', requireAuth, asyncHandler((req, res) => {
47+
alertService.acknowledge(parseInt(req.params.id), req.user.id);
48+
res.json({ ok: true });
49+
}));
5750

5851
module.exports = router;

src/routes/audit.js

Lines changed: 46 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -3,70 +3,59 @@
33
const { Router } = require('express');
44
const auditService = require('../services/audit');
55
const { requireAuth, requireRole } = require('../middleware/auth');
6+
const asyncHandler = require('../utils/asyncHandler');
67

78
const router = Router();
89

910
// Query audit log (admin only)
10-
router.get('/', requireAuth, requireRole('admin'), (req, res) => {
11-
try {
12-
const { action, targetType, userId, page, limit, since, until } = req.query;
13-
const result = auditService.query({
14-
action,
15-
targetType,
16-
userId: userId ? parseInt(userId) : undefined,
17-
page: parseInt(page) || 1,
18-
limit: Math.min(parseInt(limit) || 50, 500),
19-
since,
20-
until,
21-
});
22-
res.json(result);
23-
} catch (err) {
24-
res.status(500).json({ error: err.message });
25-
}
26-
});
11+
router.get('/', requireAuth, requireRole('admin'), asyncHandler((req, res) => {
12+
const { action, targetType, userId, page, limit, since, until } = req.query;
13+
const result = auditService.query({
14+
action,
15+
targetType,
16+
userId: userId ? parseInt(userId) : undefined,
17+
page: parseInt(page) || 1,
18+
limit: Math.min(parseInt(limit) || 50, 500),
19+
since,
20+
until,
21+
});
22+
res.json(result);
23+
}));
2724

2825
// Verify audit log integrity (admin only)
29-
router.get('/verify', requireAuth, requireRole('admin'), (req, res) => {
30-
try {
31-
const { from, to } = req.query;
32-
const result = auditService.verify({
33-
fromId: from ? parseInt(from) : undefined,
34-
toId: to ? parseInt(to) : undefined,
35-
});
36-
res.json(result);
37-
} catch (err) {
38-
res.status(500).json({ error: err.message });
39-
}
40-
});
26+
router.get('/verify', requireAuth, requireRole('admin'), asyncHandler((req, res) => {
27+
const { from, to } = req.query;
28+
const result = auditService.verify({
29+
fromId: from ? parseInt(from) : undefined,
30+
toId: to ? parseInt(to) : undefined,
31+
});
32+
res.json(result);
33+
}));
4134

4235
// Export audit log (admin only)
43-
router.get('/export', requireAuth, requireRole('admin'), (req, res) => {
44-
try {
45-
const { format, since, until, action, userId } = req.query;
46-
const validFormats = ['json', 'csv', 'syslog'];
47-
const fmt = validFormats.includes(format) ? format : 'json';
48-
49-
const data = auditService.export(fmt, {
50-
since,
51-
until,
52-
action,
53-
userId: userId ? parseInt(userId) : undefined,
54-
});
55-
56-
const contentTypes = {
57-
json: 'application/json',
58-
csv: 'text/csv',
59-
syslog: 'text/plain',
60-
};
61-
62-
const extensions = { json: 'json', csv: 'csv', syslog: 'log' };
63-
64-
res.setHeader('Content-Type', contentTypes[fmt]);
65-
res.setHeader('Content-Disposition', `attachment; filename="audit-export.${extensions[fmt]}"`);
66-
res.send(data);
67-
} catch (err) {
68-
res.status(500).json({ error: err.message });
69-
}
70-
});
36+
router.get('/export', requireAuth, requireRole('admin'), asyncHandler((req, res) => {
37+
const { format, since, until, action, userId } = req.query;
38+
const validFormats = ['json', 'csv', 'syslog'];
39+
const fmt = validFormats.includes(format) ? format : 'json';
40+
41+
const data = auditService.export(fmt, {
42+
since,
43+
until,
44+
action,
45+
userId: userId ? parseInt(userId) : undefined,
46+
});
47+
48+
const contentTypes = {
49+
json: 'application/json',
50+
csv: 'text/csv',
51+
syslog: 'text/plain',
52+
};
53+
54+
const extensions = { json: 'json', csv: 'csv', syslog: 'log' };
55+
56+
res.setHeader('Content-Type', contentTypes[fmt]);
57+
res.setHeader('Content-Disposition', `attachment; filename="audit-export.${extensions[fmt]}"`);
58+
res.send(data);
59+
}));
7160

7261
module.exports = router;

0 commit comments

Comments
 (0)