From ec429e159c352d307f5f31e8245148e0da2ee1b5 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Sun, 30 Aug 2026 12:28:23 +0000 Subject: [PATCH] fix(dns): EPERM means the bridge is alive, not stale `isAlive` asked `process.kill(pid, 0)` and caught every failure as "dead". That call fails two ways which mean opposite things: ESRCH for a pid that is gone, and EPERM for one that is there but belongs to another user. The second is the case this tool manufactures. `dns enable` escalates, so the bridge it starts is root's while every later `status` asking after it is not. A live root-owned bridge therefore reported as a stale pidfile for the rest of its life, and each caller drew a worse conclusion from it than the last: - `status` printed "bridge NOT running - stale pidfile for " and advised `dns enable` as the fix - `stopDaemon` deleted the pidfile and reported it had cleared a stale entry, while the daemon it named kept running - `startDaemon` saw nothing running and started a second bridge on 127.0.0.1 underneath the working one on 0.0.0.0. The kernel delivers to the more specific socket, so the advice shadowed the bridge it was meant to rescue and took the machine's resolver down Only ESRCH is dead now. EPERM is alive and someone else's, which the caller is told rather than having it papered over: `stop` returns "Operation not permitted" instead of silently orphaning the daemon, and `start` short-circuits on `alreadyRunning` instead of shadowing it. `isAlive` takes an injectable `kill` so both branches are testable without privileges, matching how every other system call here is reached. Observed as `dns status` reporting a stale pidfile on a Kubuntu desktop whose bridge had been up the entire time. The port probe in `bridgePresence` was already authoritative and is untouched; this fixes the half that reads the pidfile. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01ThnQwoieWt8VR6N7gtgnhp --- src/dns-system.mjs | 28 ++++++++++++++++++---- test/dns-daemon-verify.test.mjs | 42 +++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 5 deletions(-) diff --git a/src/dns-system.mjs b/src/dns-system.mjs index 390d31f2..d71e3c88 100644 --- a/src/dns-system.mjs +++ b/src/dns-system.mjs @@ -362,14 +362,32 @@ export async function readPid(path = pidfilePath()) { } } -/** Is that pid actually ours and alive? A stale pidfile must not read as running. */ -export function isAlive(pid) { +/** + * Is that pid alive? A stale pidfile must not read as running. + * + * "Alive" and "ours" are different questions, and answering the first with the + * second cost a machine its resolver. `process.kill(pid, 0)` fails two ways: + * ESRCH for a pid that is gone, and EPERM for one that is there but belongs to + * another user. Catching both as "dead" was wrong in precisely the case this + * tool manufactures — `dns enable` escalates, so the bridge is root's while + * every later `status` asking after it is not. + * + * A live root-owned bridge therefore read as a stale pidfile for the rest of + * its life: `stop` deleted the file and reported it cleared while the daemon + * kept running, and `start` saw nothing there and put a second bridge on + * 127.0.0.1 underneath the working one on 0.0.0.0 — the shadowing outage + * `bridgePresence` describes, arrived at by believing our own liveness check. + * + * So only ESRCH is dead. EPERM is alive and someone else's, which the caller + * needs told rather than papered over. + */ +export function isAlive(pid, kill = (target) => process.kill(target, 0)) { if (!pid) return false; try { - process.kill(pid, 0); + kill(pid); return true; - } catch { - return false; + } catch (error) { + return error?.code === "EPERM"; } } diff --git a/test/dns-daemon-verify.test.mjs b/test/dns-daemon-verify.test.mjs index 755a6c06..daf323a5 100644 --- a/test/dns-daemon-verify.test.mjs +++ b/test/dns-daemon-verify.test.mjs @@ -305,3 +305,45 @@ test("an already-running daemon still short-circuits without spawning", async () }); assert.deepEqual(result, { started: false, pid: process.pid, alreadyRunning: true }); }); + +/* ----------------------------------------------- alive, but not ours */ + +// `process.kill(pid, 0)` has two failure modes and they mean opposite things. +// Collapsing them is not academic: `dns enable` escalates, so the bridge it +// starts is root's and every unprivileged `status` afterwards got EPERM, +// called it dead, and advised a fix that starts a second bridge on top of the +// live one. Observed as `moshcode dns status` reporting "stale pidfile for +// 641911" on a Kubuntu desktop whose bridge was running the whole time. + +const errno = (code) => Object.assign(new Error(code), { code }); + +test("EPERM means alive and someone else's, not dead", () => { + const denied = () => { + throw errno("EPERM"); + }; + assert.equal(isAlive(4242, denied), true, "a pid we may not signal is still a pid that exists"); +}); + +test("ESRCH is the only failure that means dead", () => { + const gone = () => { + throw errno("ESRCH"); + }; + assert.equal(isAlive(4242, gone), false); +}); + +test("a real process this test cannot signal reads as alive", () => { + // pid 1 always exists and, unprivileged, cannot be signalled — the exact + // shape of a root-owned bridge. Running as root it simply succeeds, so this + // holds either way rather than depending on who runs the suite. + assert.equal(isAlive(1), true); +}); + +test("a pidfile naming another user's live process is running, not stale", async () => { + const dir = await scratch(); + const path = join(dir, "moshpit-dns.pid"); + await writeFile(path, "1\n"); + + // The whole bug in one assertion: `stale: true` here is what sent people to + // `dns enable` and took their resolver down. + assert.deepEqual(await daemonStatus(path), { running: true, pid: 1, stale: false }); +});