diff --git a/dashcaddy-api/__tests__/routes/caddy-upstreams-dc073.routes.test.js b/dashcaddy-api/__tests__/routes/caddy-upstreams-dc073.routes.test.js new file mode 100644 index 0000000..41145af --- /dev/null +++ b/dashcaddy-api/__tests__/routes/caddy-upstreams-dc073.routes.test.js @@ -0,0 +1,272 @@ +/** + * DC-073: regression tests for the caddy-upstreams mute endpoints. + * + * Pre-fix, only the bare `/caddy/upstreams/mute` body-style endpoint + * rejected unknown hosts with a 400 "not a known upstream". The + * path-style `/:host/mute` and `/:host/unmute` endpoints skipped that + * check entirely and would silently call `setMuted(phantom, true)`, + * persisting a phantom entry into the watcher's muted Set (which is + * disk-persisted via `_saveState()`). + * + * These tests prove: + * (1) every endpoint now rejects an unknown host with 400 + * (2) the rejection happens BEFORE setMuted is invoked (no state + * corruption — `fakeWatcher.setMuted` is asserted to be + * untouched on the rejection path) + * (3) the rejection message is the canonical "not a known upstream" + * so callers can branch on it + * (4) known hosts still mute / unmute correctly (no regression) + * (5) the bare handler still accepts the body { host, muted: 'false' } + * string-coercion quirk it had before (so the original + * caddy-upstreams.routes.test.js suite keeps passing) + * + * @module __tests__/routes/caddy-upstreams-dc073 + */ + +const express = require('express'); +const { validateAndMuteHost } = require('../../routes/caddy-upstreams').__test; + +function buildRouter(deps) { + const mod = require('../../routes/caddy-upstreams'); + return mod(deps); +} + +function buildApp(mod_deps) { + const app = express(); + app.use(express.json()); + app.use((req, res, next) => { + res.success = (data) => res.json({ success: true, ...data }); + res.errorResponse = (msg, code) => res.status(code || 500).json({ success: false, error: msg }); + next(); + }); + app.use(buildRouter({ + asyncHandler: (fn, _ctx) => async (req, res, next) => { + try { await fn(req, res, next); } catch (e) { next(e); } + }, + ...mod_deps, + })); + // Error middleware MUST be registered AFTER routes so it actually catches. + app.use((err, req, res, next) => { + if (err && err.statusCode === 400) { + return res.status(400).json({ success: false, error: err.message }); + } + return res.status(err?.statusCode || 500).json({ success: false, error: err?.message || 'unknown' }); + }); + return app; +} + +function makeKnownWatcher(known = ['known.svc.example:80', '1.1.1.1:80']) { + const upstreams = new Map(known.map(h => [h, { host: h }])); + return { + upstreams, + setMuted: jest.fn((host, muted) => ({ host, muted: !!muted })), + snapshot: jest.fn(() => ({ upstreams: [], config: {} })), + }; +} + +describe('routes/caddy-upstreams — DC-073 phantom-mute regression', () => { + describe('validateAndMuteHost helper (unit)', () => { + test('rejects empty / non-string host', () => { + const w = makeKnownWatcher(); + expect(() => validateAndMuteHost(w, '', true)).toThrow(/non-empty string/); + expect(() => validateAndMuteHost(w, null, true)).toThrow(/non-empty string/); + expect(() => validateAndMuteHost(w, undefined, true)).toThrow(/non-empty string/); + expect(() => validateAndMuteHost(w, 12345, true)).toThrow(/non-empty string/); + expect(w.setMuted).not.toHaveBeenCalled(); + }); + + test('rejects host longer than 253 chars', () => { + const w = makeKnownWatcher(); + const long = 'a'.repeat(254); + expect(() => validateAndMuteHost(w, long, true)).toThrow(/non-empty string/); + expect(w.setMuted).not.toHaveBeenCalled(); + }); + + test('rejects host with charset-violating chars', () => { + const w = makeKnownWatcher(); + for (const bad of ['host name', 'host?', 'host/abc', 'host;rm', 'host${x}', 'host<>']) { + expect(() => validateAndMuteHost(w, bad, true)).toThrow(/valid host/); + } + expect(w.setMuted).not.toHaveBeenCalled(); + }); + + test('rejects host not in watcher.upstreams (phantom-mute vector)', () => { + const w = makeKnownWatcher(['known:80']); + // This is the regression: pre-fix, this call would have + // silently added 'phantom.test:12345' to watcher.muted. + expect(() => validateAndMuteHost(w, 'phantom.test:12345', true)) + .toThrow(/not a known upstream/); + expect(w.setMuted).not.toHaveBeenCalled(); + }); + + test('accepts a known host and forwards setMuted(host, wantMuted)', () => { + const w = makeKnownWatcher(['known:80']); + const result = validateAndMuteHost(w, 'known:80', true); + expect(w.setMuted).toHaveBeenCalledWith('known:80', true); + expect(result).toEqual({ host: 'known:80', muted: true }); + + w.setMuted.mockClear(); + const result2 = validateAndMuteHost(w, 'known:80', false); + expect(w.setMuted).toHaveBeenCalledWith('known:80', false); + expect(result2).toEqual({ host: 'known:80', muted: false }); + }); + + test('handles missing watcher / upstreams map (defensive)', () => { + expect(() => validateAndMuteHost(null, 'x:80', true)).toThrow(/not a known upstream/); + expect(() => validateAndMuteHost({}, 'x:80', true)).toThrow(/not a known upstream/); + expect(() => validateAndMuteHost({ upstreams: null }, 'x:80', true)).toThrow(/not a known upstream/); + }); + }); + + describe('POST /caddy/upstreams/mute (bare body-style)', () => { + test('rejects unknown host with 400 (was already correct, regression-proof)', async () => { + const w = makeKnownWatcher(['known:80']); + const app = buildApp({ caddyUpstreamWatcher: w, healthChecker: { incidents: [] } }); + const server = app.listen(0); + const { port } = server.address(); + const res = await fetch(`http://127.0.0.1:${port}/caddy/upstreams/mute`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ host: 'phantom:12345' }), + }); + const body = await res.json(); + server.close(); + expect(res.status).toBe(400); + expect(body.error).toMatch(/not a known upstream/); + expect(w.setMuted).not.toHaveBeenCalled(); + }); + + test('muted: "false" string still coerces to unmute (regression from caddy-upstreams.routes.test.js)', async () => { + const w = makeKnownWatcher(['known:80']); + const app = buildApp({ caddyUpstreamWatcher: w, healthChecker: { incidents: [] } }); + const server = app.listen(0); + const { port } = server.address(); + const res = await fetch(`http://127.0.0.1:${port}/caddy/upstreams/mute`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ host: 'known:80', muted: 'false' }), + }); + const body = await res.json(); + server.close(); + expect(body.success).toBe(true); + expect(w.setMuted).toHaveBeenCalledWith('known:80', false); + }); + }); + + describe('POST /caddy/upstreams/:host/mute (path-style) — DC-073 main fix', () => { + test('rejects unknown host with 400 instead of silent phantom-mute', async () => { + const w = makeKnownWatcher(['known:80']); + const app = buildApp({ caddyUpstreamWatcher: w, healthChecker: { incidents: [] } }); + const server = app.listen(0); + const { port } = server.address(); + // Pre-fix this would have silently added 'phantom.test:12345' to + // the watcher's muted Set and called _saveState(). Post-fix it + // returns 400 and never touches the watcher. + const res = await fetch(`http://127.0.0.1:${port}/caddy/upstreams/phantom.test:12345/mute`, { + method: 'POST', + }); + const body = await res.json(); + server.close(); + expect(res.status).toBe(400); + expect(body.error).toMatch(/not a known upstream/); + expect(w.setMuted).not.toHaveBeenCalled(); + }); + + test('mutes a known host via bare POST (no body)', async () => { + const w = makeKnownWatcher(['known.svc.example:80']); + const app = buildApp({ caddyUpstreamWatcher: w, healthChecker: { incidents: [] } }); + const server = app.listen(0); + const { port } = server.address(); + const res = await fetch(`http://127.0.0.1:${port}/caddy/upstreams/known.svc.example:80/mute`, { + method: 'POST', + }); + const body = await res.json(); + server.close(); + expect(res.status).toBe(200); + expect(body.success).toBe(true); + expect(w.setMuted).toHaveBeenCalledWith('known.svc.example:80', true); + }); + + test('mutes via ?muted=true query', async () => { + const w = makeKnownWatcher(['known.svc.example:80']); + const app = buildApp({ caddyUpstreamWatcher: w, healthChecker: { incidents: [] } }); + const server = app.listen(0); + const { port } = server.address(); + const res = await fetch(`http://127.0.0.1:${port}/caddy/upstreams/known.svc.example:80/mute?muted=true`, { + method: 'POST', + }); + const body = await res.json(); + server.close(); + expect(w.setMuted).toHaveBeenCalledWith('known.svc.example:80', true); + expect(body.success).toBe(true); + }); + + test('unmutes via body { muted: false }', async () => { + const w = makeKnownWatcher(['known.svc.example:80']); + const app = buildApp({ caddyUpstreamWatcher: w, healthChecker: { incidents: [] } }); + const server = app.listen(0); + const { port } = server.address(); + const res = await fetch(`http://127.0.0.1:${port}/caddy/upstreams/known.svc.example:80/mute`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ muted: false }), + }); + const body = await res.json(); + server.close(); + expect(w.setMuted).toHaveBeenCalledWith('known.svc.example:80', false); + expect(body.success).toBe(true); + }); + }); + + describe('POST /caddy/upstreams/:host/unmute (path-style) — DC-073 main fix', () => { + test('rejects unknown host with 400 instead of silent phantom-unmute', async () => { + const w = makeKnownWatcher(['known:80']); + const app = buildApp({ caddyUpstreamWatcher: w, healthChecker: { incidents: [] } }); + const server = app.listen(0); + const { port } = server.address(); + const res = await fetch(`http://127.0.0.1:${port}/caddy/upstreams/phantom.test:12345/unmute`, { + method: 'POST', + }); + const body = await res.json(); + server.close(); + expect(res.status).toBe(400); + expect(body.error).toMatch(/not a known upstream/); + expect(w.setMuted).not.toHaveBeenCalled(); + }); + + test('unmutes a known host', async () => { + const w = makeKnownWatcher(['known.svc.example:80']); + const app = buildApp({ caddyUpstreamWatcher: w, healthChecker: { incidents: [] } }); + const server = app.listen(0); + const { port } = server.address(); + const res = await fetch(`http://127.0.0.1:${port}/caddy/upstreams/known.svc.example:80/unmute`, { + method: 'POST', + }); + const body = await res.json(); + server.close(); + expect(res.status).toBe(200); + expect(w.setMuted).toHaveBeenCalledWith('known.svc.example:80', false); + expect(body.success).toBe(true); + }); + }); + + describe('router introspection (DC-057-style mount-count assertion)', () => { + test('exactly one POST handler per (method,path) — no duplicate registration', () => { + const w = makeKnownWatcher(); + const router = buildRouter({ + asyncHandler: (fn) => fn, + caddyUpstreamWatcher: w, + healthChecker: { incidents: [] }, + }); + const sigs = router.stack + .filter((l) => l.route) + .map((l) => Object.keys(l.route.methods).map((m) => `${m.toUpperCase()} ${l.route.path}`)) + .flat(); + // Each (method,path) should appear exactly once + const counts = sigs.reduce((m, s) => (m[s] = (m[s] || 0) + 1, m), {}); + for (const [sig, n] of Object.entries(counts)) { + expect({ sig, n }).toEqual({ sig, n: 1 }); + } + }); + }); +}); diff --git a/dashcaddy-api/routes/caddy-upstreams.js b/dashcaddy-api/routes/caddy-upstreams.js index 79f28d0..a3690c5 100644 --- a/dashcaddy-api/routes/caddy-upstreams.js +++ b/dashcaddy-api/routes/caddy-upstreams.js @@ -4,7 +4,9 @@ * Exposes: * GET /api/v1/caddy/upstreams — full snapshot * GET /api/v1/caddy/upstreams/incidents — open dead-upstream incidents (via healthChecker) - * POST /api/v1/caddy/upstreams/:host/mute — body { muted: true|false } (also via query ?muted=true) + * POST /api/v1/caddy/upstreams/mute — body { host, muted: true|false } + * POST /api/v1/caddy/upstreams/:host/mute — body { muted: true|false } OR query ?muted=true + * POST /api/v1/caddy/upstreams/:host/unmute — clears the mute * * Auth: same as the rest of /api/v1 — handled by the global middleware * (the router is mounted under the auth-gated apiRouter in app.js). @@ -16,6 +18,48 @@ const express = require('express'); const { success, errorResponse } = require('../src/utils/responses'); const { ValidationError } = require('../src/utilities/errors'); +/** + * DC-073: shared mute helper — used by all three mute endpoints so the + * host-validation logic can't drift. + * + * Pre-fix, only the bare `/caddy/upstreams/mute` body-style endpoint + * rejected unknown hosts (with a "not a known upstream" 400). The + * path-style `/:host/mute` and `/:host/unmute` endpoints skipped that + * check entirely, so an authenticated operator could POST + * `/caddy/upstreams/phantom.test:12345/mute` and the watcher would + * silently add `phantom.test:12345` to its muted Set and `_saveState()` + * would persist it to disk. The phantom entry then survives container + * restarts, pollutes the snapshot view (the muted Set is iterated in + * places like the dashboard's "muted upstreams" badge), and would + * silently disable any future probe that happened to resolve to the + * same string. + * + * Post-fix, every mute path runs through this helper so: + * (1) host format is well-formed (rejects injection / `:` / `?` / etc.) + * (2) host is in `caddyUpstreamWatcher.upstreams` (the live registry + * populated by `scanSites()` reading every `reverse_proxy` from + * /etc/caddy/sites/*. A phantom host cannot reach setMuted.) + * (3) the muted Set never holds entries the scanner doesn't know. + * + * @param {Object} watcher caddyUpstreamWatcher instance + * @param {string} host raw host string from the request + * @param {boolean} wantMuted true to mute, false to unmute + * @returns {{host: string, muted: boolean}} the result of setMuted + * @throws {ValidationError} on invalid format or unknown host + */ +function validateAndMuteHost(watcher, host, wantMuted) { + if (typeof host !== 'string' || host.length === 0 || host.length > 253) { + throw new ValidationError('host must be a non-empty string up to 253 chars'); + } + if (!/^[a-z0-9._:-]+$/i.test(host)) { + throw new ValidationError('host must be a valid host[:port] string'); + } + if (!watcher || !watcher.upstreams || !watcher.upstreams.has(host)) { + throw new ValidationError(`host ${host} is not a known upstream (run scan first)`); + } + return watcher.setMuted(host, wantMuted); +} + module.exports = function({ asyncHandler, caddyUpstreamWatcher, healthChecker }) { const router = express.Router(); @@ -55,62 +99,48 @@ module.exports = function({ asyncHandler, caddyUpstreamWatcher, healthChecker }) success(res, { incidents: open }); }, 'caddy-upstreams-incidents')); - // POST /caddy/upstreams/mute body { host, muted } - // POST /caddy/upstreams/:host/mute body { muted: true } OR query ?muted=true - // Both shapes supported because the dashboard code is small and either is - // ergonomic depending on caller. - const handleMute = asyncHandler(async (req, res) => { - if (!caddyUpstreamWatcher) { - 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)) { - throw new ValidationError('host must be a valid host[:port] string'); - } - // Accept muted as boolean body field OR ?muted=true|false query OR - // a { muted: true|false } JSON body. Default to toggling on bare POST - // without a muted value (this is the "mute it" path). - let muted; - if (typeof req.body?.muted === 'boolean') muted = req.body.muted; - else if (typeof req.query.muted === 'string') muted = req.query.muted === 'true'; - else muted = true; // POST with no body = mute - - const result = caddyUpstreamWatcher.setMuted(host, muted); - success(res, result); - }, 'caddy-upstreams-mute'); - // Bare /mute with JSON body {host, muted}. Default mutes when muted is // absent or unparseable; require muted === false explicitly to unmute. + // DC-073: now routes through validateAndMuteHost so the unknown-host + // check applies (was already correct here pre-fix, but path-style + // was missing it — see validateAndMuteHost docblock). router.post('/caddy/upstreams/mute', asyncHandler(async (req, res) => { if (!caddyUpstreamWatcher) { 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)) { - throw new ValidationError('host must be a valid host[:port] string'); - } // Explicit boolean coercion — string 'false' should NOT mute. const wantMuted = muted === undefined ? true : muted === true; - if (caddyUpstreamWatcher.upstreams && !caddyUpstreamWatcher.upstreams.has(host)) { - throw new ValidationError(`host ${host} is not a known upstream (run scan first)`); - } - const result = caddyUpstreamWatcher.setMuted(host, wantMuted); + const result = validateAndMuteHost(caddyUpstreamWatcher, host, wantMuted); success(res, result); }, 'caddy-upstreams-mute-bare')); - // /:host/mute and /:host/unmute for path-style toggles - router.post('/caddy/upstreams/:host/mute', handleMute); + // Path-style /:host/mute — body { muted: true|false } OR query ?muted=true|false. + // DC-073: now also rejects unknown hosts (was the bug — see docblock). + router.post('/caddy/upstreams/:host/mute', asyncHandler(async (req, res) => { + if (!caddyUpstreamWatcher) { + return errorResponse(res, 503, 'Caddy upstream watcher not initialized'); + } + let wantMuted; + if (typeof req.body?.muted === 'boolean') wantMuted = req.body.muted; + else if (typeof req.query.muted === 'string') wantMuted = req.query.muted === 'true'; + else wantMuted = true; // bare POST = mute + const result = validateAndMuteHost(caddyUpstreamWatcher, req.params.host, wantMuted); + success(res, result); + }, 'caddy-upstreams-mute')); + + // DC-073: path-style /:host/unmute now also rejects unknown hosts. router.post('/caddy/upstreams/:host/unmute', asyncHandler(async (req, res) => { if (!caddyUpstreamWatcher) { return errorResponse(res, 503, 'Caddy upstream watcher not initialized'); } - const host = req.params.host; - if (!host || !/^[a-z0-9._:-]+$/i.test(host)) { - throw new ValidationError('host must be a valid host[:port] string'); - } - const result = caddyUpstreamWatcher.setMuted(host, false); + const result = validateAndMuteHost(caddyUpstreamWatcher, req.params.host, false); success(res, result); }, 'caddy-upstreams-unmute')); return router; -}; \ No newline at end of file +}; + +// Export the helper for unit tests so the validation surface can be +// exercised without spinning up a full Express app. +module.exports.__test = { validateAndMuteHost }; \ No newline at end of file