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

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)) {
-      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;
-};
+};
+
+// Export the helper for unit tests so the validation surface can be
+// exercised without spinning up a full Express app.
+module.exports.__test = { validateAndMuteHost };