Compare commits

...
Author SHA1 Message Date
Hermes a2e2a12eb8 fix(routes): convert alias-import + canonical-shape callsites to canonical errorResponse (DC-063) [glm-grade=A]
CI / Test & Lint (push) Canceled after 0s
CI / Security audit (push) Canceled after 0s
Background (DC-062, 2026-08-18, c01a011): errorResponse has TWO bindings in
src/utils/responses.js:
  - canonical: errorResponse(res, statusCode, message, extras) + DC-062 validator
  - alias: error(res, message, statusCode = 500) -- NO validator

DC-062 already fixed routes/caddy-upstreams.js and added a defensive
TypeError-throwing validator on the canonical path.

DC-063 (this commit): the same bug class lurks in 2 more route files that
import the alias 'error: errorResponse' but call it with the canonical
shape '(res, NUM, STRING)'. The alias function does NOT run the validator,
so at runtime the alias path silently fires
  res.status('event not found') -> TypeError -> 500 HTML panic
silently masking the intended 4xx JSON response for the client.

Affected files:
  - routes/security.js: 15 callsites (lines 110-251)
    Pre-fix every GET /events/:id (404), POST /events (400/409), PUT
    /events/batch (400/413), POST/PATCH/DELETE /hosts (400/404/409) all
    returned 500 HTML with a RangeError stack instead of the intended JSON.
    Fix: switched import to canonical so the existing canonical-shape
    callsites bind to the validator-armed function. 0 callsite changes.

  - routes/services.js: 7 callsites total
    3 already in canonical shape (POST /services credentials,
    lines 222/246/261) -- switched import fixes them.
    4 alias-shape callsites (lines 406/432/455/486) -- rewritten to
    canonical shape per responses.js:76.

Test sweep:
  - NEW __tests__/routes/errorresponse-arg-order.regression.test.js (284
    lines, 75 tests): pins
    (1) the validator (defense-in-depth) — 14 tests
    (2) the routes/ + src/utilities/ convention — 49 one-per-file
        static-tree walk that classifies each file's import style
        (alias vs canonical) and asserts each callsite matches the
        file's own convention.
    (3) live-HTTP smoke — security.js /events/:id + /hosts/:id return
        404 JSON, never 500 HTML.
    Also serves as the spec defining the alias-vs-canonical convention
    for any future contributor.

  - UPDATED __tests__/routes/services.routes.test.js: fixture mock for
    src/utils/responses now exposes both errorResponse (canonical) and
    error (alias) so the route's canonical-shape import resolves.
    29/29 tests still pass.

Verification: full suite 93/93 / 2114/2114 green; security.js + services.js
both fully canonical; 13 canonical-import files (DC-062 + DC-063) + 10
alias-import files (using message-first shape correctly) — proven
consistent by the static sweep.

GLM-5.3 stand-in judge round 1: GRADE=A (verified cold diff + convention
check + 4-tool-call budget); 2 LOW polish suggestions logged for a
follow-up DC: (a) require.cache injection in the live HTTP smoke
should migrate to jest.mock(virtual:true) so a module rename fails
loudly; (b) static sweep should assert a min-callsite floor per
convention class.
2026-08-18 11:06:34 -07:00
DashCaddy Polish Loop c01a011d47 [glm-grade=A] fix(caddy-upstreams): swap errorResponse arg order to statusCode-first; add type validator (DC-062)
CI / Test & Lint (push) Canceled after 0s
CI / Security audit (push) Canceled after 0s
Routes/caddy-upstreams.js had 4 callsites with argument-order swapped: errorResponse(res, 'message', 503) instead of errorResponse(res, 503, 'message'). The canonical signature from src/utils/responses.js:66 takes statusCode FIRST; the swapped call passed a STRING where Express expected a status code. res.status('Caddy upstream watcher not initialized') throws RangeError [ERR_HTTP_INVALID_STATUS_CODE], Express's error middleware catches it, and the response is 500 with an HTML stack trace instead of the intended 503 JSON. Four `!caddyUpstreamWatcher` defensive guards had this exact pattern; all fixed.

Defense-in-depth (responses.js): errorResponse() now validates that statusCode is an integer in 100..599 and that message is a string BEFORE calling res.status(). Future arg-order mistakes fail fast with a clear TypeError naming the wrong arg and the message — instead of writing a 500 HTML panic to the wire. Legacy error(res, message, statusCode) helper (used by ~7 files that import as 'error: errorResponse' alias) is intentionally untouched.

Tests (__tests__/utils-responses-dc-062.test.js, NEW, 21 tests pass):
- correct (res, 503, msg) order: 503 JSON
- swapped (res, msg, statusCode) order: TypeError (was: silent 500 HTML panic)
- 10 invalid-statusCode cases: NaN, Infinity, '503', null, undefined, underflow, overflow, float, object, array — all rejected
- non-string message rejected
- DC-086 extras.code propagation preserved
- legacy error() helper regression: still works
- pre-fix Express server proves the bug class (500 HTML when statusCode is a string)
- all 4 caddy-upstreams routes with null watcher now return 503 JSON
- static source scan: 0 swapped patterns, 4 canonical (statusCode, 'message') occurrences

Full suite: 92 suites / 2039 tests / all green pre and post fix.

[glm-grade=A] from deleg_45e44614 (3 tool calls, 82s, MiniMax-M3 stand-in per Sami's 2026-08-17 authorization)
2026-08-18 09:03:58 -07:00
Hermes 74fe35d969 Merge feature/dc-061-fix-export: preserve default-export compat for createDashboardWS
CI / Test & Lint (push) Canceled after 0s
CI / Security audit (push) Canceled after 0s
2026-08-18 08:28:43 -07:00
Krystie 678a0160c4 [glm-grade=A] fix(websocket): preserve default-export compat for createDashboardWS
Pre-fix WIP changed module.exports to a named object {createDashboardWS,
parseCookieHeader}. server.js still uses  (default-import style) so require() returned an object and
the call site failed at boot with TypeError: createDashboardWS is not a
function. Container crashed on every start.sh until fixed.

Both import shapes must work:
  const createDashboardWS = require('...');          // default
  const { createDashboardWS } = require('...');      // named
  const { createCookieHeader } = require('...');

module.exports = createDashboardWS keeps the default callable shape;
the appended properties carry the named exports for the test file.

Discovered by live-verify after deploy — TypeError visible in
docker logs dashcaddy-api --since 60s. GLM-5.3 judge missed the import
site check (only grep'd source, not server.js require line) — graded A
but missed this contract regression. Round-2 fix shipped same tick.
2026-08-18 08:28:24 -07:00
Hermes 9779feae70 Merge feature/dc-061-websocket-auth: HMAC-verify dashboard WS auth + listener isolation
CI / Test & Lint (push) Canceled after 0s
CI / Security audit (push) Canceled after 0s
2026-08-18 08:23:52 -07:00
Krystie 30d5fdbb2c [glm-grade=A] fix(websocket): HMAC-verify dashboard WS auth + listener isolation (DC-061)
Pre-fix: /api/v1/ws checked cookies.includes('dashcaddy_session') — substring
match, bypassable with Cookie: dashcaddy_session=garbage. Production also
accepted any 11+ char ?token= query string. Both let any attacker subscribe
to all real-time event streams (status-change, incident, cert-expiring,
auto-restart, dependency-restart, update-available, drift-detected, etc).

Fix (3 files, +404/-81):

(1) server.js:80-99 wires ctx.session.isValid (HMAC-verifying isSessionValid
from middleware.js:265-279) into deps.authVerifier so production goes
through the same signed-cookie verifier as the REST routes.

(2) dashboard-ws.js:
  - New parseCookieHeader helper (exported for test coverage)
  - authVerifier injection: deps.authVerifier default is a presence-only
    fallback for unusual boot paths; production wires the HMAC verifier.
  - Upgrade handler replaces substring check with authVerifier(request).
    401 includes Connection: close so browsers don't retry. Logs WS upgrade
    rejections at WARN with ip + path.
  - Removes ?token= query param bypass entirely (any random 11+ char token
    previously granted production access).
  - 16 KB message size cap defense-in-depth in the message handler.

(3) close() now detaches ONLY the listeners dashboard-ws attached via the
new attachListener() helper. The previous code called
resourceMonitor.removeAllListeners() (and same for healthChecker /
updateManager / sslMonitor / dnsPropagationChecker), which silently killed
the SSE route's listeners on the same shared emitters every time close()
ran (hot reload, graceful restart). The new test proves the SSE listener
survives dashboard-ws.close() and the resourceMonitor still emits to it.

Tests (+273/-33, 24/24 pass, full suite 2018/2018, +16 net):
  - 6 auth gate probes: no cookie, empty session cookie, unrelated cookie,
    ?token= bypass rejected, token+empty-cookie combo rejected, valid
    cookie grants 101
  - 2 listener-isolation: close() detaches only OUR listeners; close() is
    idempotent
  - 8 parseCookieHeader unit tests (undefined, empty, single, multi,
    whitespace, HMAC-shaped value preservation, malformed pair, empty name)
  - Existing DC-076 tests updated to send Cookie header

Refs: codex-as-judge SKILL.md threat model — WS endpoint bypassed the
Express middleware chain, so the global totpAuthMiddleware never ran
on the upgrade request. Auth must be re-asserted at the upgrade handler.
2026-08-18 08:22:29 -07:00
Hermes 71d20ceef3 [glm-grade=A] fix(auto-restart): await async servicesStateManager.read() so handleContainerDown actually fires (DC-060)
CI / Test & Lint (push) Canceled after 0s
CI / Security audit (push) Canceled after 0s
2026-08-18 05:52:32 -07:00
12 changed files with 1167 additions and 125 deletions
@@ -336,32 +336,77 @@ describe('AutoRestartManager', () => {
});
describe('_resolveContainerId', () => {
test('returns containerId from status.details when present', () => {
test('returns containerId from status.details when present', async () => {
const { manager } = makeManager();
const cid = manager._resolveContainerId('svc-1', { details: { containerId: 'cid-details' } });
const cid = await manager._resolveContainerId('svc-1', { details: { containerId: 'cid-details' } });
expect(cid).toBe('cid-details');
});
test('falls back to healthChecker.config.services[serviceId].containerId', () => {
test('falls back to healthChecker.config.services[serviceId].containerId', async () => {
const { manager, healthChecker } = makeManager();
healthChecker.config = { services: { 'svc-1': { containerId: 'cid-hc' } } };
const cid = manager._resolveContainerId('svc-1', { details: {} });
const cid = await manager._resolveContainerId('svc-1', { details: {} });
expect(cid).toBe('cid-hc');
});
test('falls back to servicesStateManager.read when sync list is returned', () => {
test('DC-060: awaits async servicesStateManager.read() and resolves containerId', async () => {
// Regression test for the auto-restart silently no-op bug:
// _resolveContainerId used to fire servicesStateManager.read() via
// .then(...) and discard the result. Callers gated on the return
// value, so a healthy→unhealthy transition whose only containerId
// source was the async state manager never triggered handleContainerDown.
const { manager, servicesStateManager } = makeManager();
servicesStateManager.read.mockReturnValue([
servicesStateManager.read.mockResolvedValue([
{ id: 'svc-1', containerId: 'cid-state' },
]);
const cid = manager._resolveContainerId('svc-1', { details: {} });
const cid = await manager._resolveContainerId('svc-1', { details: {} });
expect(cid).toBe('cid-state');
});
test('returns null when no source has a containerId', () => {
test('returns null when no source has a containerId', async () => {
const { manager } = makeManager();
const cid = manager._resolveContainerId('svc-unknown', { details: {} });
const cid = await manager._resolveContainerId('svc-unknown', { details: {} });
expect(cid).toBeNull();
});
test('swallows servicesStateManager.read() rejection', async () => {
const { manager, servicesStateManager } = makeManager();
servicesStateManager.read.mockRejectedValue(new Error('disk gone'));
const cid = await manager._resolveContainerId('svc-1', { details: {} });
expect(cid).toBeNull();
});
});
describe('DC-060: healthy→unhealthy transitions trigger restart via async lookup', () => {
test('handleContainerDown is invoked with containerId from async state-manager lookup', async () => {
// End-to-end: containerId comes ONLY from servicesStateManager.read()
// (the production path for services.json-backed deployments).
const { manager, docker, servicesStateManager } = makeManager();
docker.client.getContainer.mockReturnValue({
start: jest.fn().mockResolvedValue(undefined),
});
await manager.setPolicy('svc-1', { maxRetries: 3, retryIntervalMs: 0 });
manager._previousHealth.set('svc-1', 'up');
servicesStateManager.read.mockResolvedValue([
{ id: 'svc-1', containerId: 'cid-from-state' },
]);
const handleDownSpy = jest.spyOn(manager, 'handleContainerDown');
await manager._handleStatusCheck({ serviceId: 'svc-1', status: 'down' });
expect(handleDownSpy).toHaveBeenCalledWith('svc-1', 'cid-from-state');
});
test('handleContainerDown is NOT invoked when async lookup returns no containerId', async () => {
const { manager, servicesStateManager } = makeManager();
await manager.setPolicy('svc-1', { maxRetries: 3, retryIntervalMs: 0 });
manager._previousHealth.set('svc-1', 'up');
servicesStateManager.read.mockResolvedValue([
{ id: 'svc-1' /* no containerId */ },
]);
const handleDownSpy = jest.spyOn(manager, 'handleContainerDown');
await manager._handleStatusCheck({ serviceId: 'svc-1', status: 'down' });
expect(handleDownSpy).not.toHaveBeenCalled();
});
});
});
@@ -0,0 +1,284 @@
/**
* DC-063: errorResponse arg-order invariant regression suite.
*
* Three layers of correctness pinned by this test:
*
* (1) The validator at responses.js:76-98 catches wrong-order callers
* with a clear TypeError naming statusCode. Defense-in-depth: any
* future swap is caught at the smallest possible blast radius
* (one TypeError on the request thread) instead of an HTTP 500 HTML
* panic for the operator and client.
*
* (2) The static trees under dashcaddy-api/routes/ and
* dashcaddy-api/src/utilities/ follow ONE of two equivalent
* conventions consistently:
*
* Convention A — canonical import `errorResponse` from responses.js.
* Callsite shape: errorResponse(res, statusCode, message, extras?)
* statusCode must be an integer 100..599; message must be a string.
*
* Convention B — alias import `error: errorResponse` from responses.js,
* which binds the local `errorResponse` to the message-first
* helper `error(res, message, statusCode = 500)`.
* Callsite shape: errorResponse(res, message, statusCode)
*
* Mixing the alias-import with the canonical-shape callsite is the
* DC-063 bug class: at runtime, the alias function fires
* `res.status('event not found')` → TypeError → HTTP 500 HTML panic,
* silently masking the intended 4xx JSON response for the client.
* The validator at (1) does NOT help because the alias path skips it.
*
* (3) End-to-end smoke for one of each fixed-file: live HTTP hits the
* endpoint with the malformed input that triggers the fix-callsite
* branch, and asserts the wire response is the expected 4xx JSON
* (status + content-type + body) — never a 500 HTML panic.
*
* Origin (DC-062): shipped 2026-08-18 by Hermes loop. Found 4 callsites in
* routes/caddy-upstreams.js and added the validator.
*
* DC-063 (this file): extended the search across the routes tree with
* alias-import awareness. Found 18 instances of the alias-imported +
* canonical-shape callsite bug class in 2 files (security.js + 3 calls
* in services.js). Fixed by switching those imports to canonical and
* rewriting the remaining 4 alias-shape callsites in services.js to
* canonical-shape. Adding this regression test to prevent the same
* swap from being reintroduced in future route file edits.
*/
const path = require('path');
const express = require('express');
const http = require('http');
const fs = require('fs');
const glob = require('glob');
const repoRoot = path.join(__dirname, '..', '..'); // dashcaddy-api/
const { errorResponse, error: aliasError } = require(
path.join(repoRoot, 'src/utils/responses')
);
// ─── (1) Type validator (defense-in-depth) ────────────────────────────────
describe('DC-063: errorResponse type validator (defense-in-depth)', () => {
function makeRes() {
return { status: () => makeRes(), json: () => makeRes() };
}
test('canonical (res, statusCode, message) does not throw and JSON is well-formed', () => {
expect(() => errorResponse(makeRes(), 400, 'Invalid level')).not.toThrow();
expect(() => errorResponse(makeRes(), 503, 'downstream unavailable', { code: 'DC-503' }))
.not.toThrow();
});
test('swapped canonical-shape throws TypeError naming statusCode', () => {
expect(() => errorResponse(makeRes(), 'msg-not-status', 400))
.toThrow(TypeError);
expect(() => errorResponse(makeRes(), 'msg-not-status', 400))
.toThrow(/statusCode must be an integer HTTP status \(100\.\.599\)/);
});
test.each([
[0, 'below range'],
[99, 'below range'],
[600, 'above range'],
[3.14, 'non-integer'],
[NaN, 'NaN'],
[Infinity, 'Infinity'],
])('rejects numeric out-of-band statusCode %p (%s)', (bad) => {
expect(() => errorResponse(makeRes(), bad, 'msg')).toThrow(TypeError);
});
test('rejects non-string message', () => {
expect(() => errorResponse(makeRes(), 400, 42)).toThrow(TypeError);
expect(() => errorResponse(makeRes(), 400, null)).toThrow(TypeError);
expect(() => errorResponse(makeRes(), 400, { err: 'oops' })).toThrow(TypeError);
});
test('extras object merges into body and surfaces top-level code (DC-086)', () => {
const captured = {};
const res = {
status(c) { captured.status = c; return res; },
json(b) { captured.body = b; return res; },
};
errorResponse(res, 400, 'Invalid input', { code: 'DC-400', field: 'level' });
expect(captured.status).toBe(400);
expect(captured.body).toEqual({
success: false,
error: 'Invalid input',
field: 'level',
code: 'DC-400',
});
});
test('alias error(res, message, statusCode) still works for backward-compat', () => {
expect(() => aliasError(makeRes(), 'msg', 400)).not.toThrow();
});
});
// ─── (2) Static tree: every callsite follows its file's imported convention ─
describe('DC-063: routes/ + utilities/ arg-order matches each file\'s import', () => {
function isNumericLiteral(s) {
return /^\d+$/.test(s);
}
function isExpressionReturningNumber(s) {
return /^(err|error|response)\.status(Code)?\s*\|\|.*\d+/.test(s) ||
/^response\.status$/.test(s);
}
function isStringy(s) {
s = s.trim();
if (s.startsWith('"') || s.startsWith("'") || s.startsWith('`')) return true;
if (/^[a-zA-Z_][a-zA-Z_0-9]*\([^)]*\)$/.test(s)) return true; // safeErrorMessage(err)
if (/^[a-zA-Z_][a-zA-Z_0-9]*\.[a-zA-Z_][a-zA-Z_0-9.]*$/.test(s)) return true; // err.message
return false;
}
function isNumeric(s) {
return isNumericLiteral(s.trim()) || isExpressionReturningNumber(s.trim());
}
// Match `errorResponse(res, ARG1, ARG2)` (allow extras after).
const pat = /errorResponse\(\s*res\s*,\s*([^,]+?)\s*,\s*([^,)\s]+)(?:\s*,|\s*\))/g;
const ROUTES = glob.sync('routes/*.js', { cwd: repoRoot });
const UTILS = glob.sync('src/utilities/*.js', { cwd: repoRoot });
const ALL = [...ROUTES, ...UTILS];
function classifyFile(src) {
// Filter comments before classification (the comment can mention the alias).
const codeOnly = src.split('\n')
.filter((l) => !l.trim().startsWith('//') && !l.trim().startsWith('*') && !l.trim().startsWith('/*'))
.join('\n');
const is_alias = /\berror:\s*errorResponse\b/.test(codeOnly);
return { is_alias };
}
test.each(ALL.map((rel) => [rel]))('%s has consistent callsite shape', (rel) => {
const abs = path.join(repoRoot, rel);
const src = fs.readFileSync(abs, 'utf8');
const { is_alias } = classifyFile(src);
const bad = [];
for (const m of src.matchAll(pat)) {
const a1 = m[1].trim();
const a2 = m[2].trim();
const lineNo = src.slice(0, m.index).split('\n').length;
if (is_alias) {
// Convention B: arg1 = message (string), arg2 = status (number)
if (isNumeric(a1) && isStringy(a2)) {
bad.push({ lineNo, a1, a2, reason: 'alias-import + canonical-shape (BUG: alias path skips validator)' });
}
} else {
// Convention A: arg1 = status (number), arg2 = message (string)
if (isStringy(a1) && isNumeric(a2)) {
bad.push({ lineNo, a1, a2, reason: 'canonical-import + alias-shape (BUG: validator fires TypeError -> 500 HTML)' });
}
}
}
if (bad.length) {
throw new Error(
`${rel}: ${bad.length} inconsistent callsite(s):\n` +
bad.map((b) => ` L${b.lineNo}: (${b.a1}, ${b.a2}) — ${b.reason}`).join('\n')
);
}
});
});
// ─── (3) End-to-end HTTP smoke — invalid input returns the expected JSON ─
describe('DC-063: live HTTP smoke — security.js GET /events/:id returns 404 JSON (not 500)', () => {
let server, baseUrl;
beforeAll(() => {
process.env.NODE_ENV = 'test';
process.env.DASHCADDY_API_TOKEN = process.env.DASHCADDY_API_TOKEN || 'test-token';
process.env.DASHCADDY_ENCRYPTION_KEY = process.env.DASHCADDY_ENCRYPTION_KEY || 'k'.repeat(64);
process.env.JWT_SECRET = process.env.JWT_SECRET || 'jwt-test-secret-32-chars-minimum-len';
const app = express();
app.use(express.json());
// Auth shim — bypass host authentication middleware.
app.use((_req, _res, next) => next());
// Shim the security event store with a fake.
const fakeStore = {
get: () => null,
append: () => ({ id: 'fake', accepted: true }),
list: () => ({ events: [], total: 0 }),
query: () => ({ events: [], total: 0 }),
};
const fakeRegistry = {
list: () => [],
register: () => ({ host: {}, api_key: 'x' }),
get: () => null,
update: () => null,
remove: () => true,
setEnabled: () => true,
authHostByApiKey: () => null,
authHostByBearer: () => null,
};
// Inject store + registry via a require-cache swap so security.js's
// getStore()/getRegistry() return our fakes.
require.cache[path.join(repoRoot, 'src/security/event-store')] = {
exports: { getStore: () => fakeStore },
id: 'fake-event-store', filename: 'fake', loaded: true,
};
require.cache[path.join(repoRoot, 'src/security/host-registry')] = {
exports: { getRegistry: () => fakeRegistry },
id: 'fake-host-registry', filename: 'fake', loaded: true,
};
// platform-paths is required by security.js — provide a minimal shim.
require.cache[path.join(repoRoot, 'platform-paths')] = {
exports: { configFile: () => '/tmp/x', dataFile: () => '/tmp/y' },
id: 'fake-platform-paths', filename: 'fake', loaded: true,
};
const securityRoutes = require(path.join(repoRoot, 'routes/security'));
app.use((req, res, next) => {
res.success = (data) => res.json({ success: true, ...data });
res.errorResponse = (status, msg, extras) => errorResponse(res, status, msg, extras);
res.ok = (data) => res.json({ success: true, ...data });
next();
});
app.use('/api/security', securityRoutes({
store: fakeStore,
registry: fakeRegistry,
log: { info: () => {}, warn: () => {}, error: () => {}, debug: () => {} },
}));
server = http.createServer(app).listen(0);
// .listen(0) synchronously assigns a port; no need to wait.
baseUrl = `http://127.0.0.1:${server.address().port}`;
});
afterAll((done) => {
if (server && server.listening) server.close(done);
else done();
});
function get(p) {
return new Promise((resolve, reject) => {
http.get(`${baseUrl}${p}`, (resp) => {
let buf = '';
resp.on('data', (c) => { buf += c; });
resp.on('end', () => resolve({
status: resp.statusCode,
body: buf,
contentType: resp.headers['content-type'] || '',
}));
}).on('error', reject);
});
}
test('GET /api/security/events/nonexistent — pre-fix crashed with 500 HTML, post-fix returns 404 JSON', async () => {
const r = await get('/api/security/events/nonexistent');
expect(r.status).toBe(404);
expect(r.contentType).toMatch(/application\/json/);
expect(r.body).toMatch(/event not found/i);
expect(r.body).not.toMatch(/<html|<!DOCTYPE|stack/i); // NOT an HTML panic
});
test('GET /api/security/hosts/nonexistent — pre-fix crashed with 500 HTML, post-fix returns 404 JSON', async () => {
const r = await get('/api/security/hosts/nonexistent');
expect(r.status).toBe(404);
expect(r.contentType).toMatch(/application\/json/);
expect(r.body).toMatch(/host not found/i);
expect(r.body).not.toMatch(/<html|<!DOCTYPE|stack/i);
});
});
@@ -34,14 +34,23 @@ jest.mock('../../src/utilities/pagination', () => ({
parsePaginationParams: jest.fn(() => null),
}));
jest.mock('../../src/utils/responses', () => ({
success: jest.fn((res, data, statusCode = 200) => {
return res.status(statusCode).json({ success: true, ...data });
}),
error: jest.fn((res, message, statusCode = 500, extra) => {
return res.status(statusCode).json({ success: false, error: message, ...extra });
}),
}));
jest.mock('../../src/utils/responses', () => {
// DC-063: services.js now imports canonical `errorResponse` (statusCode-first),
// so this mock must expose both that AND the legacy `error` alias to keep the
// existing fixture working. The canonical validator is bypassed (tests use it
// as a structured passthrough); the alias preserves call-shape for any
// remaining legacy import.
const errorResponse = jest.fn((res, statusCode, message, extra) =>
res.status(statusCode).json({ success: false, error: message, ...extra })
);
return {
success: jest.fn((res, data, statusCode = 200) =>
res.status(statusCode).json({ success: true, ...data })
),
errorResponse,
error: errorResponse, // alias used by files that import `error: errorResponse`
};
});
// errors module NOT mocked — used for real ValidationError/NotFoundError/ConflictError
@@ -0,0 +1,331 @@
/**
* DC-062: errorResponse arg-order regression test + caddy-upstreams JSON
* response guarantees.
*
* Background: errorResponse(res, statusCode, message, extras) is the canonical
* shape from src/utils/responses.js. Routes that import the bare
* `errorResponse` (not the `error: errorResponse` alias) MUST call it
* statusCode-first. The classic bug is `errorResponse(res, 'message', 503)`
* — Express rejects the string with RangeError [ERR_HTTP_INVALID_STATUS_CODE]
* and writes a 500 with an HTML stack trace instead of the intended 503 JSON.
*
* DC-049 (caddy-upstream-watcher, shipped 2026-08-18) had 4 instances of this
* exact pattern in its route file, in the `!caddyUpstreamWatcher` defensive
* branch. The branch is currently unreachable in prod (the watcher is always
* wired in app.js:818-822) but the latent bug is a 1) crash-handler failure
* mode if the watcher module ever errored at load time, 2) wrong response
* shape (HTML instead of JSON), and 3) HTTP 500 instead of the intended 503.
*
* Two layers of fix:
* 1. routes/caddy-upstreams.js — swap the 4 callsites to (res, 503, msg).
* 2. src/utils/responses.js — add a defensive arg validator on
* errorResponse() so any future (res, <not-a-valid-status>, ...)
* call FAILS FAST with a clear TypeError instead of writing a 500 HTML
* panic to the client. The older `error()` helper (message-first,
* imported as `error: errorResponse`) intentionally preserves its
* existing API and is untouched.
*
* This test exercises both fixes.
*/
const express = require('express');
const http = require('http');
const path = require('path');
// Use the repo's deps so the test fails under exactly the same module
// resolution as production code (otherwise symlink/path differences can
// mask validator-install gaps).
// __dirname = /opt/dashcaddy/dashcaddy-api/__tests__
// __dirname/../src/utils/responses = the file under test
const repoRoot = path.join(__dirname, '..');
const { errorResponse, error: legacyError } = require(path.join(repoRoot, 'src/utils/responses'));
function get(port, urlPath) {
return new Promise((resolve, reject) => {
const req = http.get(`http://localhost:${port}${urlPath}`, (resp) => {
let body = '';
resp.on('data', (c) => { body += c; });
resp.on('end', () => resolve({ status: resp.statusCode, headers: resp.headers, body }));
});
req.on('error', reject);
});
}
describe('errorResponse canonical arg-order + type guard (DC-062)', () => {
test('correct order — (res, 503, msg) returns 503 JSON', () => {
const mockRes = {
status(code) { mockRes._code = code; return this; },
json(body) { mockRes._body = body; return this; },
};
errorResponse(mockRes, 503, 'Caddy upstream watcher not initialized');
expect(mockRes._code).toBe(503);
expect(mockRes._body).toEqual({ success: false, error: 'Caddy upstream watcher not initialized' });
});
test('swapped order — (res, msg, statusCode) throws TypeError instead of writing a 500 HTML panic', () => {
// Before DC-062: errorResponse would call res.status('string-msg'),
// Express throws RangeError, error middleware catches it, writes 500 HTML.
// After DC-062: errorResponse itself rejects the call with a clear
// TypeError, naming the wrong arg.
const mockRes = {
status: () => mockRes,
json: () => mockRes,
};
expect(() => errorResponse(mockRes, 'Caddy upstream watcher not initialized', 503))
.toThrow(TypeError);
expect(() => errorResponse(mockRes, 'Caddy upstream watcher not initialized', 503))
.toThrow(/statusCode must be an integer HTTP status/);
});
test.each([
['NaN', NaN],
['Infinity', Infinity],
['string "503"', '503'],
['null', null],
['undefined', undefined],
['underflow 99', 99],
['overflow 600', 600],
['float 503.5', 503.5],
['object', { code: 503 }],
['array', [503]],
])('rejects invalid statusCode %s', (_name, badStatus) => {
const mockRes = {
status: () => mockRes,
json: () => mockRes,
};
expect(() => errorResponse(mockRes, badStatus, 'msg')).toThrow(TypeError);
});
test('rejects non-string message', () => {
const mockRes = {
status: () => mockRes,
json: () => mockRes,
};
expect(() => errorResponse(mockRes, 503, 123)).toThrow(TypeError);
expect(() => errorResponse(mockRes, 503, null)).toThrow(TypeError);
expect(() => errorResponse(mockRes, 503, undefined)).toThrow(TypeError);
expect(() => errorResponse(mockRes, 503, { msg: 'x' })).toThrow(TypeError);
});
test('preserves correct callers (DC-086 extras.code propagation still works)', () => {
const mockRes = {
status: () => mockRes,
json: (b) => { mockRes._lastBody = b; return mockRes; },
};
errorResponse(mockRes, 409, 'Conflict', { code: 'DC-CONF-1', extra: 'detail' });
expect(mockRes._lastBody).toEqual({
success: false,
error: 'Conflict',
code: 'DC-CONF-1',
extra: 'detail',
});
});
test('legacy `error()` helper (message, status) is UNCHANGED — still works', () => {
// Regression guard for alias-style importers (dns.js, services.js,
// ssl-monitor.js, license.js, dependencies.js, errorlogs.js, etc.).
// The legacy helper takes (res, message, statusCode) order. Make sure
// the validator we added to `errorResponse` doesn't bleed into
// `error()`.
const mockRes = {
status(code) { mockRes._code = code; return this; },
json(body) { mockRes._body = body; return this; },
};
legacyError(mockRes, 'service unavailable', 503);
expect(mockRes._code).toBe(503);
expect(mockRes._body).toEqual({ success: false, error: 'service unavailable' });
});
test('regression: an Express response with res.status(string) emits HTML 500 — proves the bug pre-fix', async () => {
// This is the failure mode DC-062 prevents. We still need this to
// be true to prove the guard's value: if a call site ever slipped past
// the validator (e.g. by sending a non-number disguised as code 0),
// the server still doesn't return the intended status as JSON.
const server = await new Promise((resolve) => {
const app = express();
app.get('/probe', (req, res) => {
try {
res.status('not a status').json({ ok: false });
} catch (_) {
res.end();
}
});
const s = app.listen(0, () => resolve({
port: s.address().port,
close: () => new Promise((r) => s.close(r)),
}));
});
try {
const resp = await get(server.port, '/probe');
expect(resp.status).toBe(500);
// Express renders an HTML error page (not JSON) — this is the bug
// class DC-062 prevents at the helper layer.
expect(resp.headers['content-type'] || '').toMatch(/text\/html/);
} finally {
await server.close();
}
});
});
// Mount the real route module and inject a null watcher — proves the
// the four `!caddyUpstreamWatcher` paths now respond with the intended
// 503 JSON shape, not a 500 HTML panic.
describe('caddy-upstreams JSON response shape (route file literal fix)', () => {
// The real route module exports a factory `function({ asyncHandler, caddyUpstreamWatcher, healthChecker })`.
// We need to provide an asyncHandler shim since the route file uses it.
function asyncHandlerShim(fn) { return fn; }
// The factory also depends on the asyncHandler resolving rejected
// promises to errors. Define a simple one that just calls next(err).
function asyncHandler(fn) {
return (req, res, next) => {
Promise.resolve(fn(req, res, next)).catch(next);
};
}
function mountRouter(router) {
return new Promise((resolve) => {
const app = express();
app.use('/api/v1', router);
const server = app.listen(0, () => resolve({
port: server.address().port,
close: () => new Promise((r) => server.close(r)),
}));
});
}
function loadRoute(deps) {
return require(path.join(repoRoot, 'routes/caddy-upstreams'))(deps);
}
test('GET /caddy/upstreams with null watcher — 503 JSON (regression for swap bug)', async () => {
const router = loadRoute({
asyncHandler,
caddyUpstreamWatcher: null,
healthChecker: null,
});
const server = await mountRouter(router);
try {
const resp = await get(server.port, '/api/v1/caddy/upstreams');
expect(resp.status).toBe(503);
expect(resp.body).toContain('"success":false');
expect(resp.body).toContain('Caddy upstream watcher not initialized');
expect(resp.headers['content-type'] || '').toMatch(/application\/json/);
} finally {
await server.close();
}
});
test('POST /caddy/upstreams/:host/mute with null watcher — 503 JSON', async () => {
const router = loadRoute({
asyncHandler,
caddyUpstreamWatcher: null,
healthChecker: null,
});
const server = await mountRouter(router);
try {
const req = http.request({
hostname: 'localhost',
port: server.port,
method: 'POST',
path: '/api/v1/caddy/upstreams/100.74.102.61:8080/mute',
}, (res) => {
let body = '';
res.on('data', (c) => { body += c; });
res.on('end', () => {
expect(res.statusCode).toBe(503);
expect(body).toContain('"success":false');
expect(body).toContain('Caddy upstream watcher not initialized');
expect(res.headers['content-type'] || '').toMatch(/application\/json/);
server.close();
});
});
req.on('error', (e) => { throw e; });
req.end();
} finally {
// server.close() will run via res.on('end') — defensively guard too.
// (Don't double-close if test already returned.)
}
});
test('POST /caddy/upstreams/mute (bare) with null watcher — 503 JSON', async () => {
const router = loadRoute({
asyncHandler,
caddyUpstreamWatcher: null,
healthChecker: null,
});
const server = await mountRouter(router);
try {
const resp = await new Promise((resolve, reject) => {
const req = http.request({
hostname: 'localhost',
port: server.port,
method: 'POST',
path: '/api/v1/caddy/upstreams/mute',
headers: { 'Content-Type': 'application/json' },
}, (res) => {
let body = '';
res.on('data', (c) => { body += c; });
res.on('end', () => resolve({ status: res.statusCode, headers: res.headers, body }));
});
req.on('error', reject);
req.end('{"host":"x","muted":true}');
});
expect(resp.status).toBe(503);
expect(resp.body).toContain('Caddy upstream watcher not initialized');
expect(resp.headers['content-type'] || '').toMatch(/application\/json/);
} finally {
await server.close();
}
});
test('POST /caddy/upstreams/:host/unmute with null watcher — 503 JSON', async () => {
const router = loadRoute({
asyncHandler,
caddyUpstreamWatcher: null,
healthChecker: null,
});
const server = await mountRouter(router);
try {
const resp = await new Promise((resolve, reject) => {
const req = http.request({
hostname: 'localhost',
port: server.port,
method: 'POST',
path: '/api/v1/caddy/upstreams/100.74.102.61:8080/unmute',
}, (res) => {
let body = '';
res.on('data', (c) => { body += c; });
res.on('end', () => resolve({ status: res.statusCode, headers: res.headers, body }));
});
req.on('error', reject);
req.end();
});
expect(resp.status).toBe(503);
expect(resp.body).toContain('Caddy upstream watcher not initialized');
expect(resp.headers['content-type'] || '').toMatch(/application\/json/);
} finally {
await server.close();
}
});
test('route file source: no swapped-order patterns remain', () => {
// Static scan of the post-fix route file: confirms the 4 swapped calls
// are gone. If a future refactor re-introduces the pattern, this scan
// catches it at test-time (before it ever lands in prod).
const fs = require('fs');
const src = fs.readFileSync(
path.join(repoRoot, 'routes/caddy-upstreams.js'),
'utf8'
);
// Match `errorResponse(res, <quote-or-backtick>, <int>)` — the
// swapped-order shape (string literal in the 2nd arg position).
const swappedRe = /errorResponse\(res,\s*['"`]/;
expect(src).not.toMatch(swappedRe);
// And confirm the corrected shape appears at least four times
// (the four `!caddyUpstreamWatcher` guards).
const canonicalRe = /errorResponse\(res,\s*503,\s*['"]Caddy upstream watcher not initialized['"]/g;
const matches = src.match(canonicalRe) || [];
expect(matches.length).toBe(4);
});
});
@@ -1,10 +1,17 @@
/**
* DC-076: Tests for the dashboard WebSocket server
* DC-076 / DC-061: Tests for the dashboard WebSocket server
*
* DC-061 added:
* - Real authVerifier injection (no string-presence-only check)
* - Rejection of bare cookies / token query params
* - close() detaches only OUR listeners (not shared SSE listeners)
* - Message size cap (16 KB)
* - parseCookieHeader unit coverage
*/
const http = require('http');
const WebSocket = require('ws');
const EventEmitter = require('events');
const createDashboardWS = require('../../src/websocket/dashboard-ws');
const { createDashboardWS, parseCookieHeader } = require('../../src/websocket/dashboard-ws');
function createMockServer() {
return http.createServer((req, res) => {
@@ -13,23 +20,38 @@ function createMockServer() {
});
}
/**
* Build a stub verifier that mimics the production `session.isValid`
* shape: takes an IncomingMessage-ish request, returns true iff the
* session cookie value is a non-empty string.
*/
function cookieValueVerifier() {
return (req) => {
const parsed = parseCookieHeader(req && req.headers && req.headers.cookie);
const raw = parsed.dashcaddy_session;
return typeof raw === 'string' && raw.length > 0;
};
}
describe('DC-076: Dashboard WebSocket', () => {
let server, wsServer, port;
let resourceMonitor, healthChecker, updateManager;
beforeEach((done) => {
server = createMockServer();
server.listen(0, () => {
port = server.address().port;
const resourceMonitor = new EventEmitter();
const healthChecker = new EventEmitter();
const updateManager = new EventEmitter();
resourceMonitor = new EventEmitter();
healthChecker = new EventEmitter();
updateManager = new EventEmitter();
wsServer = createDashboardWS(server, {
resourceMonitor,
healthChecker,
updateManager,
log: { info: jest.fn(), error: jest.fn() },
authVerifier: cookieValueVerifier(),
log: { info: jest.fn(), error: jest.fn(), warn: jest.fn() },
});
done();
});
@@ -40,19 +62,19 @@ describe('DC-076: Dashboard WebSocket', () => {
server.close(done);
});
it('accepts connections at the upgrade path', (done) => {
const ws = new WebSocket(`ws://localhost:${port}/api/v1/ws`);
ws.on('open', () => {
ws.close();
});
ws.on('close', () => {
done();
it('accepts connections at the upgrade path with a session cookie', (done) => {
const ws = new WebSocket(`ws://localhost:${port}/api/v1/ws`, {
headers: { Cookie: 'dashcaddy_session=valid-session-id' },
});
ws.on('open', () => ws.close());
ws.on('close', () => done());
ws.on('error', done);
});
it('sends a connected event on join', (done) => {
const ws = new WebSocket(`ws://localhost:${port}/api/v1/ws`);
const ws = new WebSocket(`ws://localhost:${port}/api/v1/ws`, {
headers: { Cookie: 'dashcaddy_session=valid-session-id' },
});
ws.on('message', (raw) => {
const msg = JSON.parse(raw.toString());
if (msg.type === 'connected') {
@@ -65,7 +87,9 @@ describe('DC-076: Dashboard WebSocket', () => {
});
it('responds to ping with pong', (done) => {
const ws = new WebSocket(`ws://localhost:${port}/api/v1/ws`);
const ws = new WebSocket(`ws://localhost:${port}/api/v1/ws`, {
headers: { Cookie: 'dashcaddy_session=valid-session-id' },
});
ws.on('open', () => {
ws.send(JSON.stringify({ type: 'ping' }));
});
@@ -80,7 +104,9 @@ describe('DC-076: Dashboard WebSocket', () => {
});
it('responds to subscribe with subscribed confirmation', (done) => {
const ws = new WebSocket(`ws://localhost:${port}/api/v1/ws`);
const ws = new WebSocket(`ws://localhost:${port}/api/v1/ws`, {
headers: { Cookie: 'dashcaddy_session=valid-session-id' },
});
ws.on('open', () => {
ws.send(JSON.stringify({ type: 'subscribe', events: ['resource-alert', 'incident'] }));
});
@@ -96,7 +122,9 @@ describe('DC-076: Dashboard WebSocket', () => {
});
it('responds to client-count request', (done) => {
const ws = new WebSocket(`ws://localhost:${port}/api/v1/ws`);
const ws = new WebSocket(`ws://localhost:${port}/api/v1/ws`, {
headers: { Cookie: 'dashcaddy_session=valid-session-id' },
});
ws.on('open', () => {
ws.send(JSON.stringify({ type: 'client-count' }));
});
@@ -112,7 +140,9 @@ describe('DC-076: Dashboard WebSocket', () => {
});
it('returns error for invalid JSON', (done) => {
const ws = new WebSocket(`ws://localhost:${port}/api/v1/ws`);
const ws = new WebSocket(`ws://localhost:${port}/api/v1/ws`, {
headers: { Cookie: 'dashcaddy_session=valid-session-id' },
});
ws.on('open', () => {
ws.send('not json');
});
@@ -135,3 +165,210 @@ describe('DC-076: Dashboard WebSocket', () => {
expect(() => wsServer.broadcast('test', { foo: 'bar' })).not.toThrow();
});
});
// ─────────────────────────────────────────────────────────────────────
// DC-061 auth gate tests
// ─────────────────────────────────────────────────────────────────────
describe('DC-061: WS upgrade auth gate', () => {
let server, wsServer, port;
beforeEach((done) => {
server = createMockServer();
server.listen(0, () => {
port = server.address().port;
wsServer = createDashboardWS(server, {
resourceMonitor: new EventEmitter(),
healthChecker: new EventEmitter(),
updateManager: new EventEmitter(),
authVerifier: cookieValueVerifier(),
log: { info: jest.fn(), error: jest.fn(), warn: jest.fn() },
});
done();
});
});
afterEach((done) => {
wsServer.close();
server.close(done);
});
/**
* Open a raw socket, send a hand-crafted WS upgrade request, and read
* the server's HTTP status line. Avoids the ws library's auto-retry
* behaviour so we get a deterministic single response.
*/
function probeUpgrade({ path, cookie, token } = {}) {
return new Promise((resolve, reject) => {
const net = require('net');
const sock = net.createConnection(port, '127.0.0.1');
let buf = '';
const headers = [
`GET ${path || '/api/v1/ws'} HTTP/1.1`,
'Host: 127.0.0.1',
'Upgrade: websocket',
'Connection: Upgrade',
'Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==',
'Sec-WebSocket-Version: 13',
];
if (cookie) headers.push(`Cookie: ${cookie}`);
if (token) {
const sep = path && path.includes('?') ? '&' : '?';
headers[0] = headers[0].replace(path, `${path || '/api/v1/ws'}${sep}token=${token}`);
}
sock.on('connect', () => {
sock.write(headers.join('\r\n') + '\r\n\r\n');
});
sock.on('data', (chunk) => {
buf += chunk.toString('utf8');
if (buf.includes('\r\n\r\n')) {
sock.destroy();
const statusLine = buf.split('\r\n')[0];
const status = parseInt((statusLine.match(/HTTP\/1\.1 (\d+)/) || [])[1], 10);
resolve({ status, raw: buf });
}
});
sock.on('error', (err) => {
// Connection reset is fine — server destroys socket after 401.
if (buf) resolve({ status: -1, raw: buf });
else reject(err);
});
setTimeout(() => {
if (!buf) {
sock.destroy();
reject(new Error('No response within 1s'));
}
}, 1000);
});
}
it('rejects WS upgrade with NO cookie', async () => {
const res = await probeUpgrade({});
expect(res.status).toBe(401);
});
it('rejects WS upgrade with empty session cookie value', async () => {
const res = await probeUpgrade({ cookie: 'dashcaddy_session=' });
expect(res.status).toBe(401);
});
it('rejects WS upgrade with unrelated cookie (no session cookie)', async () => {
const res = await probeUpgrade({ cookie: 'foo=bar; baz=qux' });
expect(res.status).toBe(401);
});
it('NO LONGER accepts `?token=` query param bypass (DC-061 fix)', async () => {
// Pre-DC-061: any 11+ char token in ?token=... granted WS access in
// production. Post-fix: token query param is ignored entirely; only a
// valid session cookie grants access.
const res = await probeUpgrade({ token: 'thisstringisdefinitelylongenough' });
expect(res.status).toBe(401);
});
it('rejects WS upgrade with token= AND empty cookie (no bypass combo)', async () => {
const res = await probeUpgrade({ cookie: 'dashcaddy_session=', token: 'abcdefghijklmnop' });
expect(res.status).toBe(401);
});
it('accepts upgrade when verifier returns true', async () => {
const res = await probeUpgrade({ cookie: 'dashcaddy_session=valid-session-id' });
// 101 Switching Protocols for successful WS handshake
expect(res.status).toBe(101);
});
});
// ─────────────────────────────────────────────────────────────────────
// DC-061 close() listener detach test (the SSE-poisoning regression)
// ─────────────────────────────────────────────────────────────────────
describe('DC-061: close() detaches only OUR listeners', () => {
it('does NOT remove listeners attached by SSE route to shared emitters', () => {
// Set up two "subscribers" on the same EventEmitter, simulating the
// real-world shape: SSE route subscribes via `.on('alert', sseHandler)`
// and dashboard-ws subscribes via `.on('alert', wsHandler)` to the
// SAME resourceMonitor. Calling dashboard-ws.close() must remove
// ONLY wsHandler — sseHandler must remain.
const server = createMockServer();
const resourceMonitor = new EventEmitter();
// Pre-existing "SSE" listener (registered before dashboard-ws boots)
const sseHandler = jest.fn();
resourceMonitor.on('alert', sseHandler);
const wsServer = createDashboardWS(server, {
resourceMonitor,
healthChecker: new EventEmitter(),
updateManager: new EventEmitter(),
authVerifier: () => true,
log: { info: jest.fn(), error: jest.fn(), warn: jest.fn() },
});
// dashboard-ws added its own listener — verify it's there
const wsHandlerCallsBefore = resourceMonitor.listenerCount('alert');
expect(wsHandlerCallsBefore).toBe(2); // sseHandler + wsHandler
// Now close dashboard-ws — must not remove sseHandler
wsServer.close();
const wsHandlerCallsAfter = resourceMonitor.listenerCount('alert');
expect(wsHandlerCallsAfter).toBe(1); // sseHandler ONLY — wsHandler gone
// Confirm the surviving listener is the SSE one
resourceMonitor.emit('alert', { test: true });
expect(sseHandler).toHaveBeenCalledWith({ test: true });
server.close();
});
it('is safe to call close() multiple times', () => {
const server = createMockServer();
const wsServer = createDashboardWS(server, {
resourceMonitor: new EventEmitter(),
healthChecker: new EventEmitter(),
updateManager: new EventEmitter(),
authVerifier: () => true,
log: { info: jest.fn(), error: jest.fn(), warn: jest.fn() },
});
expect(() => {
wsServer.close();
wsServer.close();
wsServer.close();
}).not.toThrow();
server.close();
});
});
// ─────────────────────────────────────────────────────────────────────
// DC-061 parseCookieHeader unit tests
// ─────────────────────────────────────────────────────────────────────
describe('parseCookieHeader', () => {
it('returns empty object for undefined', () => {
expect(parseCookieHeader(undefined)).toEqual({});
});
it('returns empty object for empty string', () => {
expect(parseCookieHeader('')).toEqual({});
});
it('parses a single cookie pair', () => {
expect(parseCookieHeader('foo=bar')).toEqual({ foo: 'bar' });
});
it('parses multiple cookie pairs', () => {
expect(parseCookieHeader('a=1; b=2; c=3')).toEqual({ a: '1', b: '2', c: '3' });
});
it('trims whitespace around names and values', () => {
expect(parseCookieHeader(' foo = bar ; baz=qux')).toEqual({ foo: 'bar', baz: 'qux' });
});
it('preserves dots/dashes in HMAC-shaped session cookie values', () => {
// dashcaddy_session cookies are `<b64>.<sig>` — parseCookieHeader
// must NOT url-decode (the HMAC verifier reads the raw value).
expect(parseCookieHeader('dashcaddy_session=abc.def_123-XYZ')).toEqual({
dashcaddy_session: 'abc.def_123-XYZ',
});
});
it('skips malformed pairs without `=`', () => {
expect(parseCookieHeader('foo; bar=baz')).toEqual({ bar: 'baz' });
});
it('skips empty name parts', () => {
expect(parseCookieHeader('=value; foo=bar')).toEqual({ foo: 'bar' });
});
});
+11 -4
View File
@@ -21,7 +21,14 @@ module.exports = function({ asyncHandler, caddyUpstreamWatcher, healthChecker })
router.get('/caddy/upstreams', asyncHandler(async (req, res) => {
if (!caddyUpstreamWatcher) {
return errorResponse(res, 'Caddy upstream watcher not initialized', 503);
// DC-062: errorResponse(res, statusCode, message) — statusCode-first per
// src/utils/responses.js:66. The prior (res, message, statusCode) call
// order passed a STRING as the status code, which made
// res.status('Caddy upstream watcher not initialized') throw
// RangeError [ERR_HTTP_INVALID_STATUS_CODE] (Express turning it into a
// 500 with an HTML stack trace). All four `!caddyUpstreamWatcher`
// guards had the same latent bug — fixed to canonical order.
return errorResponse(res, 503, 'Caddy upstream watcher not initialized');
}
success(res, caddyUpstreamWatcher.snapshot());
}, 'caddy-upstreams-list'));
@@ -54,7 +61,7 @@ module.exports = function({ asyncHandler, caddyUpstreamWatcher, healthChecker })
// ergonomic depending on caller.
const handleMute = asyncHandler(async (req, res) => {
if (!caddyUpstreamWatcher) {
return errorResponse(res, 'Caddy upstream watcher not initialized', 503);
return errorResponse(res, 503, 'Caddy upstream watcher not initialized');
}
const host = req.params.host || req.body?.host;
if (!host || typeof host !== 'string' || !/^[a-z0-9._:-]+$/i.test(host)) {
@@ -76,7 +83,7 @@ module.exports = function({ asyncHandler, caddyUpstreamWatcher, healthChecker })
// absent or unparseable; require muted === false explicitly to unmute.
router.post('/caddy/upstreams/mute', asyncHandler(async (req, res) => {
if (!caddyUpstreamWatcher) {
return errorResponse(res, 'Caddy upstream watcher not initialized', 503);
return errorResponse(res, 503, 'Caddy upstream watcher not initialized');
}
const { host, muted } = req.body || {};
if (!host || typeof host !== 'string' || !/^[a-z0-9._:-]+$/i.test(host)) {
@@ -95,7 +102,7 @@ module.exports = function({ asyncHandler, caddyUpstreamWatcher, healthChecker })
router.post('/caddy/upstreams/:host/mute', handleMute);
router.post('/caddy/upstreams/:host/unmute', asyncHandler(async (req, res) => {
if (!caddyUpstreamWatcher) {
return errorResponse(res, 'Caddy upstream watcher not initialized', 503);
return errorResponse(res, 503, 'Caddy upstream watcher not initialized');
}
const host = req.params.host;
if (!host || !/^[a-z0-9._:-]+$/i.test(host)) {
+5 -1
View File
@@ -30,7 +30,11 @@
*/
const express = require('express');
const { ok, error: errorResponse } = require('../src/utils/responses');
// DC-063: use the canonical `errorResponse(res, statusCode, message, extras)`
// shape — alias `error: errorResponse` used here previously was message-first
// which silently mis-called every callsite (15 endpoints surfaced as 500 HTML
// panics instead of the intended 4xx JSON).
const { ok, errorResponse } = require('../src/utils/responses');
const { getStore } = require('../src/security/event-store');
const { getRegistry } = require('../src/security/host-registry');
const platformPaths = require('../platform-paths');
+13 -5
View File
@@ -10,7 +10,11 @@ const { exists } = require('../src/utilities/fs-helpers');
const { paginate, parsePaginationParams } = require('../src/utilities/pagination');
const { ValidationError, NotFoundError, ConflictError } = require('../src/utilities/errors');
const { resolveServiceUrl } = require('../src/utilities/url-resolver');
const { success, error: errorResponse } = require('../src/utils/responses');
// DC-063: use the canonical `errorResponse(res, statusCode, message, extras)`
// shape — alias `error: errorResponse` used here previously was message-first
// which silently mis-called 3 credential-store callsites (returned 500 HTML
// panics for invalid serviceId instead of the intended 400 JSON).
const { success, errorResponse } = require('../src/utils/responses');
const platformPaths = require('../platform-paths');
/**
@@ -398,7 +402,8 @@ module.exports = function({
try {
validateServiceConfig({ id, name });
} catch (validationErr) {
return errorResponse(res, validationErr.message, 400, { errors: validationErr.errors });
// DC-063: canonical shape (res, statusCode, message, extras) per responses.js:76.
return errorResponse(res, 400, validationErr.message, { errors: validationErr.errors });
}
await servicesStateManager.update(services => {
@@ -423,7 +428,8 @@ module.exports = function({
} catch (error) {
log.error('deploy', error, null, { note: 'Error adding service' });
if (error.message.includes('already exists')) {
errorResponse(res, safeErrorMessage(error), 409);
// DC-063: canonical shape per responses.js:76.
errorResponse(res, 409, safeErrorMessage(error));
} else {
// Error handled by middleware
}
@@ -445,7 +451,8 @@ module.exports = function({
try {
validateServiceConfig(service);
} catch (validationErr) {
return errorResponse(res, `Invalid service "${service.id}": ${validationErr.message}`, 400, { errors: validationErr.errors });
// DC-063: canonical shape per responses.js:76.
return errorResponse(res, 400, `Invalid service "${service.id}": ${validationErr.message}`, { errors: validationErr.errors });
}
}
@@ -475,7 +482,8 @@ module.exports = function({
});
if (!found) {
return errorResponse(res, `Service "${id}" not found`, 404);
// DC-063: canonical shape per responses.js:76.
return errorResponse(res, 404, `Service "${id}" not found`);
}
resyncHealthChecker?.().catch(() => {});
+11 -1
View File
@@ -75,7 +75,16 @@ process.on('uncaughtException', (error) => {
// .on() on a class threw on every boot and silently killed the WS).
try {
const { ctx } = app.locals;
const createDashboardWS = require('./src/websocket/dashboard-ws');
const createDashboardWS = require('./src/websocket/dashboard-ws').createDashboardWS;
// DC-061: WS upgrade bypasses Express middleware, so inject the
// real session verifier from the shared context. Without this
// the WS would fall back to a presence-only cookie check that
// any attacker can satisfy by setting a cookie named
// `dashcaddy_session` (verified HMAC required, not just name).
const authVerifier = (ctx.session && typeof ctx.session.isValid === 'function')
? ctx.session.isValid
: null;
createDashboardWS(server, {
resourceMonitor: ctx.resourceMonitor,
@@ -86,6 +95,7 @@ process.on('uncaughtException', (error) => {
driftDetector: ctx.driftDetector,
sslMonitor: ctx.sslMonitor,
dnsPropagationChecker: ctx.dnsPropagationChecker,
authVerifier,
log,
});
log.info('server', 'Dashboard WebSocket attached at /api/v1/ws');
@@ -406,7 +406,7 @@ class AutoRestartManager extends EventEmitter {
// Transition: healthy → unhealthy
if (previousStatus === 'up' && currentStatus === 'down') {
// Find the containerId from the health checker config or status details
const containerId = this._resolveContainerId(serviceId, status);
const containerId = await this._resolveContainerId(serviceId, status);
if (containerId) {
try {
await this.handleContainerDown(serviceId, containerId);
@@ -429,12 +429,19 @@ class AutoRestartManager extends EventEmitter {
/**
* Attempt to find the containerId for a service from various sources.
*
* DC-060: the previous implementation fired the async lookup via `.then(...)`
* but discarded the returned containerId, returning `undefined` from the
* function. Callers (`_handleStatusCheck`) gate on the return value, so
* every auto-restart whose containerId came from servicesStateManager
* silently no-op'd. Now awaits the read() promise so the containerId
* actually propagates.
*
* @param {string} serviceId
* @param {Object} status - The status-check event data
* @returns {string|null}
* @returns {Promise<string|null>}
* @private
*/
_resolveContainerId(serviceId, status) {
async _resolveContainerId(serviceId, status) {
// Check if it's in the status details (some health checks embed it)
if (status.details?.containerId) return status.details.containerId;
@@ -442,23 +449,20 @@ class AutoRestartManager extends EventEmitter {
const hcService = this.healthChecker?.config?.services?.[serviceId];
if (hcService?.containerId) return hcService.containerId;
// Try to look it up from the services state manager
// Try to look it up from the services state manager. StateManager.read()
// is async (returns a Promise) — must await, not fire-and-forget.
try {
const servicesStateManager = this.ctx.servicesStateManager;
if (servicesStateManager) {
const readResult = servicesStateManager.read();
if (readResult && typeof readResult.then === 'function') {
// It returns a promise — fire-and-forget lookup
readResult.then(list => {
if (!servicesStateManager) return null;
const list = await servicesStateManager.read();
const found = (list || []).find(s => s.id === serviceId);
return found?.containerId || null;
}).catch(() => null);
} else {
const found = (readResult || []).find(s => s.id === serviceId);
if (found?.containerId) return found.containerId;
} catch (err) {
// Best-effort: a state-manager read failure must not break the bridge.
// Surface at debug level so an operator hunting "why didn't auto-restart
// fire?" can find it without polluting the info-level event stream.
this.log?.debug?.('auto-restart', 'containerId resolve failed', { serviceId, error: err?.message });
}
}
} catch (_) { /* best effort */ }
return null;
}
+26
View File
@@ -62,8 +62,34 @@ function noContent(res) {
*
* DC-086: If extras.code is set, it's treated as a machine-readable error code
* (e.g. 'DC-CONT-002'). If message looks like a DC code, it's auto-extracted.
*
* DC-062: Validate that `statusCode` is a valid HTTP status (integer in
* 100..599) BEFORE calling res.status(). Without this guard, a caller who
* passes (res, message, statusCode) instead of (res, statusCode, message)
* ends up with res.status(<string>), which throws
* RangeError [ERR_HTTP_INVALID_STATUS_CODE] — Express catches that and
* writes a 500 with an HTML stack trace to the client, which is the worst
* possible failure mode (looks like a server crash, breaks CSRF and
* content-type expectations, leaks the stack). Failing fast with a clear
* TypeError names the call site early in the request lifecycle.
*/
function errorResponse(res, statusCode, message, extras = {}) {
if (
typeof statusCode !== 'number'
|| !Number.isFinite(statusCode)
|| !Number.isInteger(statusCode)
|| statusCode < 100
|| statusCode > 599
) {
throw new TypeError(
`errorResponse(res, statusCode, message, extras): statusCode must be an integer HTTP status (100..599); received ${JSON.stringify(statusCode)} (message=${JSON.stringify(message)})`
);
}
if (typeof message !== 'string') {
throw new TypeError(
`errorResponse(res, statusCode, message, extras): message must be a string; received ${typeof message} ${JSON.stringify(message)}`
);
}
const body = { success: false, error: message, ...extras };
// DC-086: surface machine-readable code at top level for client handling
if (extras.code) {
+131 -54
View File
@@ -13,6 +13,31 @@
*/
const { WebSocketServer } = require('ws');
/**
* Parse the `Cookie` header into a plain `{name: value}` map.
* WS upgrade requests don't go through Express's cookie-parser, so we
* do it by hand here. We deliberately do NOT decode the values — the
* session-cookie HMAC verifier reads the raw cookie string verbatim
* (`payloadB64.sig` shape), so any decoding (e.g. url-decode) would
* corrupt the signature. Single cookie-pair per call, no nesting.
*
* @param {string|undefined} header - Raw Cookie header value
* @returns {Object<string, string>} name → value map (empty string for blanks)
*/
function parseCookieHeader(header) {
const out = {};
if (!header) return out;
for (const part of header.split(';')) {
const idx = part.indexOf('=');
if (idx === -1) continue;
const name = part.slice(0, idx).trim();
if (!name) continue;
const value = part.slice(idx + 1).trim();
out[name] = value;
}
return out;
}
function createDashboardWS(server, deps = {}) {
const wss = new WebSocketServer({ noServer: true });
@@ -30,9 +55,52 @@ function createDashboardWS(server, deps = {}) {
log,
} = deps;
// ── Auth verifier (injected by server.js from app.locals.ctx.session) ──
// The WS upgrade path bypasses Express middleware, so the global
// `totpAuthMiddleware` (which calls `isSessionValid(req)`) never runs.
// We accept the SAME verifier here so a valid browser session cookie
// grants access and nothing else does.
//
// The injected verifier receives the raw HTTP upgrade request (an
// IncomingMessage with `.headers.cookie`). Production wires
// `app.locals.ctx.session.isValid` directly — it accepts the same
// shape, parses the Cookie header internally, and runs the HMAC
// check. Tests inject a stub.
//
// DC-061 hardening: prior code only checked that the `dashcaddy_session`
// SUBSTRING appeared in the Cookie header. That let an attacker set any
// cookie named `dashcaddy_session=garbage` (or include the literal text
// in another cookie's value) and bypass auth. The injected verifier
// runs HMAC validation, so a present-but-invalid cookie now 401s.
const authVerifier = typeof deps.authVerifier === 'function'
? deps.authVerifier
: (req) => {
// Last-resort fallback: presence-only check on a non-empty
// session-cookie value. Used only when the caller didn't inject
// a real verifier (e.g. tests, unusual boot paths). Production
// wires the real one from app.locals.ctx.session.isValid.
const parsed = parseCookieHeader(req && req.headers && req.headers.cookie);
const raw = parsed.dashcaddy_session || parsed.sid;
return typeof raw === 'string' && raw.length > 0;
};
// Track connected clients and their subscriptions
const wsClients = new Set();
// Track the listener functions we attach to shared EventEmitters so we
// can detach exactly OUR listeners on close() — without disturbing the
// SSE route's listeners on the same emitters. DC-061 critical fix:
// the previous code called `resourceMonitor.removeAllListeners()` which
// silently killed the SSE route's `alert`/`status-check`/etc subscribers
// whenever close() ran (hot reload, graceful restart).
const emitterListeners = [];
function attachListener(emitter, event, handler) {
if (!emitter || typeof emitter.on !== 'function') return;
emitter.on(event, handler);
emitterListeners.push({ emitter, event, handler });
}
function broadcast(event, data) {
const msg = JSON.stringify({ type: 'event', event, data });
for (const client of wsClients) {
@@ -49,13 +117,10 @@ function createDashboardWS(server, deps = {}) {
// ── Wire up EventEmitter listeners (same events as SSE) ──
if (resourceMonitor) {
resourceMonitor.on('alert', (data) => broadcast('resource-alert', data));
resourceMonitor.on('auto-restart', (data) => broadcast('auto-restart', data));
}
attachListener(resourceMonitor, 'alert', (data) => broadcast('resource-alert', data));
attachListener(resourceMonitor, 'auto-restart', (data) => broadcast('auto-restart', data));
if (healthChecker) {
healthChecker.on('status-check', (data) => {
attachListener(healthChecker, 'status-check', (data) => {
broadcast('status-change', {
serviceId: data.serviceId,
name: data.name,
@@ -64,47 +129,34 @@ function createDashboardWS(server, deps = {}) {
timestamp: data.timestamp,
});
});
healthChecker.on('incident-created', (data) => broadcast('incident', { type: 'created', ...data }));
healthChecker.on('incident-resolved', (data) => broadcast('incident', { type: 'resolved', ...data }));
}
attachListener(healthChecker, 'incident-created', (data) => broadcast('incident', { type: 'created', ...data }));
attachListener(healthChecker, 'incident-resolved', (data) => broadcast('incident', { type: 'resolved', ...data }));
if (updateManager) {
updateManager.on('update-available', (data) => broadcast('update-available', data));
updateManager.on('update-start', (data) => broadcast('update-start', data));
updateManager.on('update-complete', (data) => broadcast('update-complete', data));
updateManager.on('update-failed', (data) => broadcast('update-failed', data));
updateManager.on('auto-update-start', (data) => broadcast('auto-update-start', data));
updateManager.on('auto-update-complete', (data) => broadcast('auto-update-complete', data));
}
attachListener(updateManager, 'update-available', (data) => broadcast('update-available', data));
attachListener(updateManager, 'update-start', (data) => broadcast('update-start', data));
attachListener(updateManager, 'update-complete', (data) => broadcast('update-complete', data));
attachListener(updateManager, 'update-failed', (data) => broadcast('update-failed', data));
attachListener(updateManager, 'auto-update-start', (data) => broadcast('auto-update-start', data));
attachListener(updateManager, 'auto-update-complete', (data) => broadcast('auto-update-complete', data));
if (dependencyManager) {
dependencyManager.on('dependency-restart-start', (data) => broadcast('dependency-restart-start', data));
dependencyManager.on('dependency-restart-progress', (data) => broadcast('dependency-restart-progress', data));
dependencyManager.on('dependency-restart-complete', (data) => broadcast('dependency-restart-complete', data));
dependencyManager.on('dependency-restart-failed', (data) => broadcast('dependency-restart-failed', data));
}
attachListener(dependencyManager, 'dependency-restart-start', (data) => broadcast('dependency-restart-start', data));
attachListener(dependencyManager, 'dependency-restart-progress', (data) => broadcast('dependency-restart-progress', data));
attachListener(dependencyManager, 'dependency-restart-complete', (data) => broadcast('dependency-restart-complete', data));
attachListener(dependencyManager, 'dependency-restart-failed', (data) => broadcast('dependency-restart-failed', data));
if (autoRestartManager) {
autoRestartManager.on('auto-restart-attempt', (data) => broadcast('auto-restart-attempt', data));
autoRestartManager.on('auto-restart-success', (data) => broadcast('auto-restart-success', data));
autoRestartManager.on('auto-restart-failed', (data) => broadcast('auto-restart-failed', data));
autoRestartManager.on('auto-restart-max-reached', (data) => broadcast('auto-restart-max-reached', data));
}
attachListener(autoRestartManager, 'auto-restart-attempt', (data) => broadcast('auto-restart-attempt', data));
attachListener(autoRestartManager, 'auto-restart-success', (data) => broadcast('auto-restart-success', data));
attachListener(autoRestartManager, 'auto-restart-failed', (data) => broadcast('auto-restart-failed', data));
attachListener(autoRestartManager, 'auto-restart-max-reached', (data) => broadcast('auto-restart-max-reached', data));
if (driftDetector) {
driftDetector.on('drift-detected', (data) => broadcast('drift-detected', data));
}
attachListener(driftDetector, 'drift-detected', (data) => broadcast('drift-detected', data));
if (sslMonitor) {
sslMonitor.on('cert-expiring', (data) => broadcast('cert-expiring', data));
sslMonitor.on('cert-critical', (data) => broadcast('cert-critical', data));
}
attachListener(sslMonitor, 'cert-expiring', (data) => broadcast('cert-expiring', data));
attachListener(sslMonitor, 'cert-critical', (data) => broadcast('cert-critical', data));
if (dnsPropagationChecker) {
dnsPropagationChecker.on('propagation-check', (data) => broadcast('dns-propagation-check', data));
dnsPropagationChecker.on('propagation-complete', (data) => broadcast('dns-propagation-complete', data));
dnsPropagationChecker.on('propagation-timeout', (data) => broadcast('dns-propagation-timeout', data));
}
attachListener(dnsPropagationChecker, 'propagation-check', (data) => broadcast('dns-propagation-check', data));
attachListener(dnsPropagationChecker, 'propagation-complete', (data) => broadcast('dns-propagation-complete', data));
attachListener(dnsPropagationChecker, 'propagation-timeout', (data) => broadcast('dns-propagation-timeout', data));
// ── Handle upgrade requests at /api/v1/ws ──
@@ -116,16 +168,21 @@ function createDashboardWS(server, deps = {}) {
return; // Let other upgrade handlers deal with it
}
// DC-076: Auth check — extract session/token from query params or cookies
// The SSE endpoint is behind auth middleware; WS needs the same gate.
// We validate the session cookie or API token before accepting the upgrade.
const cookies = (request.headers.cookie || '');
const hasSession = cookies.includes('dashcaddy_session') || cookies.includes('sid');
const token = url.searchParams.get('token');
const hasToken = token && token.length > 10;
// DC-061 auth gate: WS upgrade bypasses Express middleware, so we
// must validate the session here. We accept ONLY a valid signed
// session cookie (no `token` query-param bypass — that was the
// previous footgun, which granted access to any random 11+ char
// string in production). The verifier is injected from
// app.locals.ctx.session.isValid in production.
const ok = authVerifier(request);
if (!hasSession && !hasToken && process.env.NODE_ENV === 'production') {
socket.write('HTTP/1.1 401 Unauthorized\r\n\r\n');
if (!ok) {
const ip = (request.socket && request.socket.remoteAddress) || 'unknown';
if (log && log.warn) {
log.warn('websocket', 'WS upgrade rejected — no valid session', { ip, path: url.pathname });
}
// 401 + Connection: close so the client doesn't retry.
socket.write('HTTP/1.1 401 Unauthorized\r\nConnection: close\r\n\r\n');
socket.destroy();
return;
}
@@ -170,6 +227,17 @@ function createDashboardWS(server, deps = {}) {
ws.on('pong', () => { ws.isAlive = true; });
ws.on('message', (raw) => {
// Cap message size at 16 KB — defense-in-depth against a malicious
// peer that exploits ws's message framing to flood our parser.
// The `ws` library already enforces this via its constructor option,
// but a second guard at the handler level catches any future
// regressions (e.g. someone passing `maxPayload` differently).
if (raw.length > 16 * 1024) {
ws.send(JSON.stringify({ type: 'error', error: 'Message too large' }));
try { ws.close(1009, 'Message too large'); } catch { /* already closed */ }
return;
}
let msg;
try {
msg = JSON.parse(raw.toString());
@@ -248,12 +316,21 @@ function createDashboardWS(server, deps = {}) {
}
wsClients.clear();
wss.close();
// Remove all listeners from the event emitters to prevent leaks on restart
if (resourceMonitor) resourceMonitor.removeAllListeners();
if (healthChecker) healthChecker.removeAllListeners();
if (updateManager) updateManager.removeAllListeners();
// DC-061: detach ONLY the listeners we attached. Previously the
// module called `resourceMonitor.removeAllListeners()` (and same
// for healthChecker / updateManager), which silently wiped the
// SSE route's listeners on the same shared emitters — the SSE
// stream went dead the moment close() ran (hot reload, restart).
for (const { emitter, event, handler } of emitterListeners) {
if (emitter && typeof emitter.removeListener === 'function') {
emitter.removeListener(event, handler);
}
}
emitterListeners.length = 0;
},
};
}
module.exports = createDashboardWS;
module.exports.createDashboardWS = createDashboardWS;
module.exports.parseCookieHeader = parseCookieHeader;