Merge dc/DC-073-caddy-upstreams-host-validation: DC-073 phantom-mute hardening [glm-grade=A]

Bug class: silent state corruption via path-style endpoint inconsistency.

Pre-fix, only POST /caddy/upstreams/mute (bare body-style) rejected unknown
hosts with a 400. The path-style POST /caddy/upstreams/:host/mute and
POST /caddy/upstreams/:host/unmute endpoints skipped that check entirely.
An authenticated operator could POST /caddy/upstreams/phantom.test:12345/mute
and caddyUpstreamWatcher.setMuted() would silently add the phantom host
to its muted Set and _saveState() would persist it to disk. The phantom
entry survives container restarts and pollutes the snapshot view.

Fix: consolidate validation in a single validateAndMuteHost() helper used
by all three mute endpoints. The helper enforces (1) host format charset,
(2) length cap, (3) membership in caddyUpstreamWatcher.upstreams (the
live registry populated by scanSites()). No phantom host can reach setMuted.

Tests: 15 new regression tests in
__tests__/routes/caddy-upstreams-dc073.routes.test.js — exercises the
helper directly (unit) and via each endpoint (integration), asserts
rejection happens BEFORE setMuted is called (no state corruption), and
the existing 3 caddy-upstreams.routes.test.js cases still pass. Router
introspection test asserts no duplicate route registrations.

Full suite: 2342/2342 tests / 101 suites.

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 });
+      }
+    });
+  });
+});

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)) {
+    const result = validateAndMuteHost(caddyUpstreamWatcher, host, wantMuted);
-      throw new ValidationError(`host ${host} is not a known upstream (run scan first)`);
-    }
-    const result = caddyUpstreamWatcher.setMuted(host, wantMuted);
     success(res, result);
   }, 'caddy-upstreams-mute-bare'));
 
-  // /:host/mute and /:host/unmute for path-style toggles
+  // Path-style /:host/mute — body { muted: true|false } OR query ?muted=true|false.
-  router.post('/caddy/upstreams/:host/mute', handleMute);
+  // 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;
+    const result = validateAndMuteHost(caddyUpstreamWatcher, req.params.host, false);
-    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);
     success(res, result);
   }, 'caddy-upstreams-unmute'));
 
   return router;
 };
+
+// Export the helper for unit tests so the validation surface can be
+// exercised without spinning up a full Express app.
+module.exports.__test = { validateAndMuteHost };