moshcoder
diff --git a/‎src/dns-system.mjs‎
Lines changed: 18 additions & 2 deletions b/‎src/dns-system.mjs‎
Lines changed: 18 additions & 2 deletions
diff --git a/‎src/dns.mjs‎
Lines changed: 119 additions & 11 deletions b/‎src/dns.mjs‎
Lines changed: 119 additions & 11 deletions
@@ -332,8 +332,24 @@ function defaultRunner(command, args) {
  * root to listen on 5354, and requiring it to write a pidfile somewhere
  * privileged would make the whole daemon need privileges it otherwise does not.
  */
-export function pidfilePath() {
-  const base = process.env.XDG_RUNTIME_DIR || join(homedir(), ".moshcode") || tmpdir();
+export function pidfilePath(env = process.env, exists = existsSync) {
+  // `dns enable` escalates, so the run that *starts* the bridge is root and the
+  // runs that later ask about it are not. Under sudo both XDG_RUNTIME_DIR and
+  // HOME belong to root, so the pidfile went to /root/.moshcode — a path the
+  // unprivileged `dns status` and `dns disable` never look at and could not
+  // read if they did. The bridge was reported "not running" for the rest of its
+  // life, and the fix status advised started a second one on top of it.
+  //
+  // So an escalated run records against the invoking user's runtime dir, and
+  // only when that directory is really there: deriving /run/user/<uid> on a
+  // machine without one trades an unreadable path for a nonexistent one. macOS
+  // has no /run/user and falls through unchanged — the escalated paths this
+  // matters for are the systemd-resolved ones.
+  const invoker = env.SUDO_UID ? `/run/user/${env.SUDO_UID}` : null;
+  const base = (invoker && exists(invoker) ? invoker : null)
+    || env.XDG_RUNTIME_DIR
+    || join(homedir(), ".moshcode")
+    || tmpdir();
   return join(base, "moshpit-dns.pid");
 }
 
 
@@ -1742,7 +1742,7 @@ export function parseUdpListeners(text) {
   return out;
 }
 
-const defaultUdpListeners = async () => {
+export const defaultUdpListeners = async () => {
   const { execFile } = await import("node:child_process");
   const text = await new Promise((resolve) => {
     execFile("ss", ["-lnup"], { timeout: 5000 }, (err, stdout) => resolve(err ? "" : String(stdout)));
@@ -1783,6 +1783,89 @@ export function portHolder(listeners, { host = DEFAULT_HOST, port = DEFAULT_PORT
   return null;
 }
 
+/**
+ * What is actually on the bridge's port, rather than what our pidfile claims.
+ *
+ * `status` used to answer this from the pidfile alone, and that file only ever
+ * describes a bridge *this tool* started, in *this* privilege context. Every
+ * other way a bridge reaches 5354 read as "not running": a systemd unit, a
+ * hand-started `dns start`, or — the common one — an `enable` that escalated to
+ * root and therefore wrote its pidfile under root's HOME instead of the
+ * invoking user's runtime dir.
+ *
+ * That is not a cosmetic lie. Status followed it with "routing is in place but
+ * the bridge is not running", and the fix it advised starts a second bridge on
+ * 127.0.0.1 while the working one holds 0.0.0.0. The kernel delivers to the
+ * more specific socket, so the advice shadows the bridge it was meant to
+ * rescue and the machine stops resolving — the outage `portHolder` above
+ * already describes, arrived at this time by following our own instructions.
+ *
+ * So the port gets asked. A bridge nothing here started is still a bridge.
+ * Both questions are put because they fail differently: a resolver that has
+ * stopped answering Moshpit names loses a namespace, and one that has stopped
+ * forwarding takes the machine off the internet.
+ */
+export async function bridgePresence({
+  host = DEFAULT_HOST,
+  port = DEFAULT_PORT,
+  recorded = { running: false, pid: null, stale: false },
+  listeners = defaultUdpListeners,
+  answers = probeResolver,
+  forwards = probeForwarding,
+} = {}) {
+  const [moshpit, clearnet] = await Promise.all([
+    answers({ host, port }).catch(() => false),
+    forwards({ host, port, name: CLEARNET_PROBE }).catch(() => false),
+  ]);
+  const answering = Boolean(moshpit || clearnet);
+
+  // Ours and alive is the ordinary case, and the probe still runs first: a
+  // recorded pid that no longer answers is worth saying out loud rather than
+  // reporting as a healthy bridge on the strength of the file alone.
+  if (recorded.running) {
+    return { kind: "ours", pid: recorded.pid, answering, forwards: clearnet, moshpit };
+  }
+
+  if (!answering) {
+    return recorded.stale
+      ? { kind: "stale", pid: recorded.pid, answering: false, forwards: false, moshpit: false }
+      : { kind: "none", pid: null, answering: false, forwards: false, moshpit: false };
+  }
+
+  // Only asked once something is known to be there, because `ss` is the
+  // expensive half and an unattributable owner is not a reason to call a
+  // demonstrably answering bridge absent.
+  const holder = portHolder(await listeners().catch(() => []), { host, port });
+  return {
+    kind: "foreign",
+    pid: holder?.pid ?? null,
+    process: holder?.process ?? null,
+    answering: true,
+    forwards: clearnet,
+    moshpit,
+  };
+}
+
+/** One line for `status`, kept next to the states it names. */
+export function describeBridge(presence, { host = DEFAULT_HOST, port = DEFAULT_PORT } = {}) {
+  switch (presence.kind) {
+    case "ours":
+      return presence.answering
+        ? `running (pid ${presence.pid})`
+        : `running (pid ${presence.pid}) — but not answering on ${host}:${port}`;
+    case "foreign": {
+      const who = presence.pid
+        ? `pid ${presence.pid}${presence.process ? `, ${presence.process}` : ""}`
+        : "owner not visible";
+      return `answering on ${host}:${port} (${who}) — started by something other than \`dns enable\``;
+    }
+    case "stale":
+      return `NOT running — stale pidfile for ${presence.pid}`;
+    default:
+      return "not running";
+  }
+}
+
 /**
  * Everything that has to be true of the machine before the routing is written.
  *
@@ -2151,7 +2234,7 @@ import { existsSync } from "node:fs";
 import { fileURLToPath } from "node:url";
 import {
   applyPlan, daemonStatus, describePlan, detectPlatform, disablePlan, enablePlan,
-  requiredPort, startDaemon, stopDaemon,
+  probeResolver, requiredPort, startDaemon, stopDaemon,
 } from "./dns-system.mjs";
 import { escalateSelf } from "./escalate.mjs";
 
@@ -2230,6 +2313,8 @@ export async function dnsCommand(args = [], out = console.log, deps = {}) {
     applyWith = applyWithRollback,
     verify = verifyResolution,
     bridgeStatus = daemonStatus,
+    presenceImpl = bridgePresence,
+    exists = existsSync,
     startBridge = startDaemon,
     proxyReachableImpl = proxyReachable,
     findLocalProxyImpl = findLocalProxy,
@@ -3009,25 +3094,48 @@ export async function dnsCommand(args = [], out = console.log, deps = {}) {
 
   if (sub === "status") {
     const platform = detectPlatform();
-    const daemon = await daemonStatus();
+    const daemon = await bridgeStatus();
+    const statusPort = requiredPort(platform, port);
+    const presence = await presenceImpl({ port: statusPort, recorded: daemon });
     out(`platform   ${platform || process.platform}`);
-    out(`bridge     ${daemon.running ? `running (pid ${daemon.pid})` : daemon.stale ? `NOT running — stale pidfile for ${daemon.pid}` : "not running"}`);
+    out(`bridge     ${describeBridge(presence, { port: statusPort })}`);
 
     // Routing is read off the filesystem rather than remembered, so a config
     // someone edited or removed by hand is reported as it actually is.
     const marker = platform === "macos" ? "/etc/resolver" : MOSHPIT_DROPIN;
-    const routed = platform === "linux" ? existsSync(marker) : platform === "macos" ? existsSync(marker) : null;
+    // Injected like every other system call this command makes. Read straight
+    // off the filesystem, "is this machine routed" made the status tests depend
+    // on whether the machine running them happened to have Moshpit enabled —
+    // green on a developer's box, red on a clean runner.
+    const routed = platform === "linux" || platform === "macos" ? exists(marker) : null;
     out(`routing    ${routed === null ? "(check NRPT: Get-DnsClientNrptRule)" : routed ? `configured (${marker})` : "not configured"}`);
 
-    // The state worth shouting about: names are pointed at a bridge that is not
-    // there, so every Moshpit name fails instead of falling through.
-    if (routed && !daemon.running) {
+    // The state worth shouting about, and the condition is "nothing answers"
+    // rather than "our pidfile is empty". Those are not the same machine, and
+    // shouting on the second one sent people to start a bridge that shadowed
+    // the working one they already had.
+    //
+    // The advice drops its `sudo` too: the CLI escalates the one step that
+    // needs root, and teaching `sudo moshcode` is how `sudo moshcode update`
+    // ends up reinstalling the whole tool into /root.
+    if (routed && !presence.answering) {
+      out("");
+      out(`! routing is in place but nothing answers on ${DEFAULT_HOST}:${statusPort} — Moshpit names will fail.`);
+      out("  fix with: moshcode dns enable     undo with: moshcode dns disable");
+    }
+
+    // Answering but not forwarding is the dangerous half, and it is invisible
+    // from the Moshpit side: names resolve, and everything else on the machine
+    // stops. Catch-all routing is what makes it total.
+    if (routed && presence.answering && !presence.forwards) {
       out("");
-      out("! routing is in place but the bridge is not running — Moshpit names will fail.");
-      out("  fix with: sudo moshcode dns enable     undo with: sudo moshcode dns disable");
+      out(`! the bridge on ${DEFAULT_HOST}:${statusPort} answers Moshpit names but is not forwarding`);
+      out("  clearnet lookups routed through it will fail. Restart it, or `moshcode dns disable`.");
     }
 
-    const known = await fetchTlds({ registryBase }).catch(() => null);
+    // The injected one, like every other caller. Reaching past it here made
+    // `status` the one subcommand that could not be tested without a network.
+    const known = await fetchTldsImpl({ registryBase }).catch(() => null);
     const probe = known
       ? await resolveName(`probe.${known[0] || "moshpit"}`, { registryBase }).catch(() => null)
       : null;