fix(security): refuse non-loopback MITM peers and hard-gate /api/providers/client (#16060)

Reopened and merged on the owner's call (2026-10-10). Combines with #15886 (already on the tip): keeps its secret stripping for /api/providers/client, adds the hard-gate for that route, and replaces the IPv4-only MITM bind with the dual-stack listener + loopback peer guard (the DNS spoof also maps hosts to ::1). The pinned listen test now asserts the guard is installed before listen(). MITM + authz suites 320/320, eslint clean. Warning: base-red inherited: #15306.

src/mitm/_internal/peerGuard.cjs

@@ -0,0 +1,55 @@
+"use strict";
+
+// =========================================================================
+// Loopback peer guard for the standalone CommonJS `server.cjs` proxy
+// (GHSA-qxg2-rm3h-4cxp). The listener stays dual-stack because the DNS spoof
+// points target hosts at BOTH 127.0.0.1 and ::1, so the restriction lives on
+// the accepted socket instead of the bind address. Kept in its own module so
+// it can be unit-tested — `server.cjs` itself binds a port on load.
+// =========================================================================
+
+const net = require("net");
+
+/**
+ * True for 127.0.0.0/8, ::1 and IPv4-mapped loopback (::ffff:127.x.y.z).
+ *
+ * @param {string | null | undefined} address
+ * @returns {boolean}
+ */
+function isLoopbackPeer(address) {
+  if (typeof address !== "string" || address === "") return false;
+  let addr = address.toLowerCase();
+  if (addr.startsWith("::ffff:")) addr = addr.slice("::ffff:".length);
+  if (net.isIPv4(addr)) return addr.startsWith("127.");
+  return addr === "::1";
+}
+
+/**
+ * Remote (non-loopback) clients are refused unless the operator opts in
+ * explicitly, e.g. to share one MITM host across a trusted LAN.
+ *
+ * @param {NodeJS.ProcessEnv} env
+ * @returns {boolean}
+ */
+function allowRemoteClients(env) {
+  const raw = String((env && env.MITM_ALLOW_REMOTE_CLIENTS) || "")
+    .trim()
+    .toLowerCase();
+  return raw === "1" || raw === "true";
+}
+
+/**
+ * Destroy `socket` when its peer is not loopback (and remote clients are not
+ * allowed). Returns whether the socket may proceed.
+ *
+ * @param {{ remoteAddress?: string | null, destroy: () => void }} socket
+ * @param {NodeJS.ProcessEnv} [env]
+ * @returns {boolean}
+ */
+function guardLoopbackPeer(socket, env = process.env) {
+  if (isLoopbackPeer(socket.remoteAddress) || allowRemoteClients(env)) return true;
+  socket.destroy();
+  return false;
+}
+
+module.exports = { isLoopbackPeer, allowRemoteClients, guardLoopbackPeer };

src/mitm/server.cjs

@@ -29,12 +29,6 @@ const TARGET_HOSTS = new Set([
 const TARGET_HOST_AGENT = new Map();
 for (const h of TARGET_HOSTS) TARGET_HOST_AGENT.set(h, "antigravity");
 
-// The MITM only intercepts this host's own traffic (/etc/hosts points the target
-// domains at 127.0.0.1), so it must never listen on other interfaces: a LAN peer
-// could otherwise drive intercept() with the operator's ROUTER_API_KEY or use
-// passthrough() as an open TLS relay (GHSA-qxg2-rm3h-4cxp).
-const MITM_LISTEN_HOST = "127.0.0.1";
-
 const parsedLocalPort = Number.parseInt(process.env.MITM_LOCAL_PORT || "443", 10);
 const LOCAL_PORT =
   Number.isInteger(parsedLocalPort) && parsedLocalPort > 0 && parsedLocalPort <= 65535
@@ -147,6 +141,7 @@ const forwardShim = require("./_internal/forwardTarget.cjs");
 const aliasConfigShim = require("./_internal/aliasConfig.cjs");
 const standaloneRoutingShim = require("./_internal/standaloneRouting.cjs");
 const writeBackpressureShim = require("./_internal/writeBackpressure.cjs");
+const peerGuardShim = require("./_internal/peerGuard.cjs");
 
 // Inspector capture (D4 fallback). The standalone proxy intercepts AgentBridge
 // traffic inline (no MitmHandlerBase / agentBridgeHook), so it posts captured
@@ -904,10 +899,24 @@ async function startMitmServer() {
   server.headersTimeout = MITM_IDLE_TIMEOUT_MS; // time allowed to send headers
   server.keepAliveTimeout = MITM_IDLE_TIMEOUT_MS; // idle keep-alive window
 
-  server.listen(LOCAL_PORT, MITM_LISTEN_HOST, () => {
+  // GHSA-qxg2-rm3h-4cxp: the listener is dual-stack (the DNS spoof maps target
+  // hosts to both 127.0.0.1 and ::1), so refuse non-loopback peers here, before
+  // any other connection listener — otherwise a LAN peer reaches intercept()
+  // with the operator's ROUTER_API_KEY, or passthrough() as an open TLS relay.
+  // MITM_ALLOW_REMOTE_CLIENTS=1 is the explicit opt-in for a trusted LAN.
+  server.prependListener("connection", (socket) => {
+    if (!peerGuardShim.guardLoopbackPeer(socket)) {
+      vlog(1, `[MITM] refused non-loopback peer ${socket.remoteAddress || "unknown"}`);
+    }
+  });
+
+  // Dual-stack on purpose: the DNS spoof maps hosts to ::1 too, so binding to
+  // 127.0.0.1 alone would break clients that resolve to ::1. The peer guard above
+  // is what keeps LAN peers out.
+  server.listen(LOCAL_PORT, () => {
     stats.startedAt = new Date().toISOString();
     writeStats();
-    console.log(`🚀 MITM ready on ${MITM_LISTEN_HOST}:${LOCAL_PORT} → ${ROUTER_URL}`);
+    console.log(`🚀 MITM ready on :${LOCAL_PORT} (loopback peers only) → ${ROUTER_URL}`);
   });
 
   server.on("connection", (socket) => {

src/server/authz/routeGuard.ts

@@ -206,6 +206,13 @@ export const ALWAYS_PROTECTED_API_PATHS: ReadonlyArray<string> = [
   // the /webdav child. ALWAYS_PROTECTED rather than LOCAL_ONLY so an operator
   // driving the dashboard through a tunnel keeps the feature.
   "/api/settings/obsidian",
+  // GET spreads every provider_connections row verbatim, so the lazy-decrypt
+  // proxy returns each connection's apiKey / accessToken / refreshToken /
+  // idToken in plaintext, and the route has no auth call of its own. Left on the
+  // fail-open MANAGEMENT tier it served all of them anonymously under
+  // requireLogin=false (GHSA-qxg2-rm3h-4cxp). The dashboard consumers send the
+  // session cookie, so ALWAYS_PROTECTED keeps them working through a tunnel.
+  "/api/providers/client",
 ];
 
 /**

tests/unit/authz/credential-export-always-protected.test.ts

@@ -72,6 +72,14 @@ const HARD_GATED_INVENTORY: ReadonlyArray<{ path: string; why: string }> = [
     path: "/api/settings/obsidian",
     why: "POST stores the Obsidian Local REST API token; same credential surface as its /webdav child (GHSA-7pq4-8pvv-rx7r)",
   },
+  // ── Reported in GHSA-qxg2-rm3h-4cxp ──────────────────────────────────────
+  // GET spreads every provider_connections row verbatim — the lazy-decrypt
+  // proxy hands out apiKey / accessToken / refreshToken / idToken in plaintext
+  // — and the route makes no auth call of its own.
+  {
+    path: "/api/providers/client",
+    why: "returns every connection's decrypted apiKey/accessToken/refreshToken/idToken (GHSA-qxg2-rm3h-4cxp)",
+  },
   // ── Already fixed; pinned so a refactor cannot silently drop them ────────
   { path: "/api/db-backups/export", why: "GHSA-mghq-58h3-qcqj" },
   { path: "/api/db-backups/exportAll", why: "GHSA-mghq-58h3-qcqj" },
@@ -106,6 +114,7 @@ test("the trailing-slash spelling is gated too", () => {
     "/api/providers/abc/codex-auth/export/",
     "/api/logs/export/",
     "/api/cli-tools/codex-profiles/",
+    "/api/providers/client/",
   ]) {
     assert.ok(isAlwaysProtectedPath(path) || isLocalOnlyPath(path), path);
   }

tests/unit/mitm-server-peer-guard-qxg2.test.ts

@@ -0,0 +1,94 @@
+/**
+ * GHSA-qxg2-rm3h-4cxp (finding 1): `src/mitm/server.cjs` listened with no host
+ * argument, so the MITM proxy accepted connections from every interface. A LAN
+ * peer could reach `intercept()` (free inference billed through the operator's
+ * ROUTER_API_KEY) or `passthrough()` (an open TLS relay).
+ *
+ * The listener stays dual-stack because the DNS spoof writes BOTH
+ * `127.0.0.1 <host>` and `::1 <host>` (src/mitm/dns/dnsConfig.ts), so binding
+ * to 127.0.0.1 alone would break clients that resolve ::1 first. Instead every
+ * accepted socket goes through a loopback peer guard.
+ */
+import test from "node:test";
+import assert from "node:assert/strict";
+import fs from "node:fs";
+import path from "node:path";
+import { createRequire } from "node:module";
+import { fileURLToPath } from "node:url";
+
+const require = createRequire(import.meta.url);
+const {
+  isLoopbackPeer,
+  allowRemoteClients,
+  guardLoopbackPeer,
+} = require("../../src/mitm/_internal/peerGuard.cjs");
+
+const here = path.dirname(fileURLToPath(import.meta.url));
+const serverSource = fs.readFileSync(path.join(here, "../../src/mitm/server.cjs"), "utf8");
+
+test("isLoopbackPeer accepts IPv4/IPv6 loopback and IPv4-mapped loopback", () => {
+  for (const addr of ["127.0.0.1", "127.8.9.10", "::1", "::ffff:127.0.0.1"]) {
+    assert.equal(isLoopbackPeer(addr), true, addr);
+  }
+});
+
+test("isLoopbackPeer rejects LAN, public, mapped non-loopback and missing addresses", () => {
+  for (const addr of [
+    "192.168.0.20",
+    "10.0.0.5",
+    "8.8.8.8",
+    "::ffff:192.168.0.20",
+    "fe80::1",
+    "2001:db8::1",
+    "0.0.0.0",
+    "::",
+    "",
+    undefined,
+    null,
+  ]) {
+    assert.equal(isLoopbackPeer(addr), false, String(addr));
+  }
+});
+
+test("allowRemoteClients is opt-in and only for an explicit truthy value", () => {
+  assert.equal(allowRemoteClients({}), false);
+  assert.equal(allowRemoteClients({ MITM_ALLOW_REMOTE_CLIENTS: "0" }), false);
+  assert.equal(allowRemoteClients({ MITM_ALLOW_REMOTE_CLIENTS: "false" }), false);
+  assert.equal(allowRemoteClients({ MITM_ALLOW_REMOTE_CLIENTS: "1" }), true);
+  assert.equal(allowRemoteClients({ MITM_ALLOW_REMOTE_CLIENTS: "true" }), true);
+});
+
+function fakeSocket(remoteAddress: string) {
+  return {
+    remoteAddress,
+    destroyed: false,
+    destroy() {
+      this.destroyed = true;
+    },
+  };
+}
+
+test("guardLoopbackPeer destroys a non-loopback socket and keeps a loopback one", () => {
+  const lan = fakeSocket("192.168.0.20");
+  assert.equal(guardLoopbackPeer(lan, {}), false);
+  assert.equal(lan.destroyed, true);
+
+  const local = fakeSocket("::ffff:127.0.0.1");
+  assert.equal(guardLoopbackPeer(local, {}), true);
+  assert.equal(local.destroyed, false);
+});
+
+test("guardLoopbackPeer lets a remote peer through only with the explicit opt-in", () => {
+  const lan = fakeSocket("192.168.0.20");
+  assert.equal(guardLoopbackPeer(lan, { MITM_ALLOW_REMOTE_CLIENTS: "1" }), true);
+  assert.equal(lan.destroyed, false);
+});
+
+test("server.cjs installs the peer guard ahead of every other connection listener", () => {
+  assert.match(serverSource, /require\("\.\/_internal\/peerGuard\.cjs"\)/);
+  assert.match(
+    serverSource,
+    /server\.prependListener\("connection"/,
+    "the guard must run before the stats/idle-timeout connection listener"
+  );
+});

tests/unit/providers-client-route-secrets-qxg2.test.ts

@@ -47,15 +47,16 @@ test("GET /api/providers/client never returns stored credentials (GHSA-qxg2-rm3h
   assert.equal(conn.provider, "openai");
 });
 
-test("the MITM listener binds to loopback only (GHSA-qxg2-rm3h-4cxp)", () => {
+test("the MITM listener refuses non-loopback peers before any other listener (GHSA-qxg2-rm3h-4cxp)", () => {
   const source = fs.readFileSync(
     path.join(import.meta.dirname, "../../src/mitm/server.cjs"),
     "utf8"
   );
-  assert.match(source, /const MITM_LISTEN_HOST = "127\.0\.0\.1";/);
-  const listens = [...source.matchAll(/server\.listen\(([^)]*)\)?/g)].map((m) => m[1]);
-  assert.ok(listens.length > 0, "server.listen call present");
-  for (const args of listens) {
-    assert.match(args, /^LOCAL_PORT,\s*MITM_LISTEN_HOST\b/, `listen without a host: ${args}`);
-  }
+  // The listener stays dual-stack (the DNS spoof maps hosts to 127.0.0.1 and ::1), so
+  // the loopback-only guarantee comes from the peer guard installed ahead of listen().
+  const guard = source.indexOf('server.prependListener("connection"');
+  const listen = source.indexOf("server.listen(LOCAL_PORT");
+  assert.ok(guard > 0, "peer guard installed with prependListener");
+  assert.ok(listen > guard, "peer guard is installed before the server listens");
+  assert.match(source.slice(guard, listen), /guardLoopbackPeer\(socket\)/);
 });