From fcc22435a478a9cde01f2b3d3f00ca187b234048 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Sat, 1 Aug 2026 14:07:53 +0000 Subject: [PATCH] fix(dns): actually use the catch-all routing that #195 built MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #195 added the catch-all config generators, upstream parsing and the forwarding path, and wired none of them in. `dns enable` still called resolvedConf(tlds) and `dns start` never passed upstreams, so v0.16.0 shipped the capability and none of the behaviour: a box that upgraded got the same 4586-ending list, the same silent truncation at the resolver's cap, and the same `curl: (6) Could not resolve host`. The release notes said routing had stopped being a list. It had not. Wiring it is the easy half. The hard half is that catch-all routing is only safe when the bridge can forward what is not ours — point every lookup at a bridge with no upstreams and the machine loses DNS entirely, which is far worse than a Moshpit name that does not resolve. So it is conditional by construction rather than by flag: - `discoverUpstreams` reads /etc/resolv.conf BEFORE routing is switched, because afterwards resolv.conf may point at us and the real servers are no longer discoverable from it - loopback entries are dropped, so the bridge cannot forward to itself - upstreams found → `Domains=~.` and the bridge forwards - none found → the per-ending list, exactly as before, which can only ever break Moshpit names - `dns start` passes the same upstreams and the claimed-ending set to the server, and says which upstreams it will use The dnsmasq backend follows the same rule, with no-resolv so it does not inherit upstreams that point back here. Co-Authored-By: Claude Opus 5 (1M context) --- src/dns-system.mjs | 53 ++++++++++++++++++++++++++++++-------- src/dns.mjs | 27 ++++++++++++++++++- test/dns-catchall.test.mjs | 34 ++++++++++++++++++++++++ 3 files changed, 102 insertions(+), 12 deletions(-) diff --git a/src/dns-system.mjs b/src/dns-system.mjs index 7172632..6ba060a 100644 --- a/src/dns-system.mjs +++ b/src/dns-system.mjs @@ -50,7 +50,14 @@ export function enablePlan({ host = "127.0.0.1", port = 5354, linuxBackend = "systemd-resolved", + // Catch-all routing is opt-in and conditional, never assumed. Sending every + // lookup on the machine to the bridge is only safe if the bridge can forward + // the ones that are not ours — so the caller passes the upstreams it found, + // and an empty list keeps the per-ending routing that cannot break anything + // beyond Moshpit names. Getting this backwards takes the whole box offline. + upstreams = [], }) { + const catchAll = Array.isArray(upstreams) && upstreams.length > 0; const clean = [...new Set((tlds || []).map((t) => String(t).replace(/^\.+/, "").toLowerCase()).filter(Boolean))]; if (!clean.length) throw new Error("no TLDs to route"); @@ -80,8 +87,16 @@ export function enablePlan({ steps: [ write( "/etc/dnsmasq.d/moshpit.conf", - ["# Written by `moshcode dns enable`.", ...clean.map((t) => `server=/${t}/${host}#${port}`), ""].join("\n"), - "route the Moshpit TLDs", + catchAll + ? [ + "# Written by `moshcode dns enable`.", + "# no-resolv so dnsmasq does not also inherit upstreams that point back here.", + "no-resolv", + `server=${host}#${port}`, + "", + ].join("\n") + : ["# Written by `moshcode dns enable`.", ...clean.map((t) => `server=/${t}/${host}#${port}`), ""].join("\n"), + catchAll ? "send every lookup to the bridge, which forwards what is not ours" : "route the Moshpit TLDs", ), run("systemctl", ["restart", "dnsmasq"], "dnsmasq reads its config at start"), ], @@ -99,15 +114,31 @@ export function enablePlan({ steps: [ write( "/etc/systemd/resolved.conf.d/moshpit.conf", - [ - "# Written by `moshcode dns enable`. Routes Moshpit TLDs to the local", - "# bridge; every other name keeps using your normal resolver.", - "[Resolve]", - `DNS=${host}:${port}`, - `Domains=${clean.map((t) => `~${t}`).join(" ")}`, - "", - ].join("\n"), - "route the Moshpit TLDs, and nothing else", + catchAll + ? [ + "# Written by `moshcode dns enable`. Sends every lookup to the local", + "# bridge, which answers claimed Moshpit endings and forwards the rest", + "# upstream untouched.", + "#", + "# Naming each ending instead does not survive the registry growing:", + "# systemd-resolved caps how many search domains it accepts and drops", + "# the remainder with no error a caller can see.", + "[Resolve]", + `DNS=${host}:${port}`, + "Domains=~.", + "", + ].join("\n") + : [ + "# Written by `moshcode dns enable`. Routes Moshpit TLDs to the local", + "# bridge; every other name keeps using your normal resolver.", + "[Resolve]", + `DNS=${host}:${port}`, + `Domains=${clean.map((t) => `~${t}`).join(" ")}`, + "", + ].join("\n"), + catchAll + ? "send every lookup to the bridge, which forwards what is not ours" + : "route the Moshpit TLDs, and nothing else", ), run("systemctl", ["restart", "systemd-resolved"], "drop-ins are read at start"), ], diff --git a/src/dns.mjs b/src/dns.mjs index 46404ab..60454b0 100644 --- a/src/dns.mjs +++ b/src/dns.mjs @@ -500,6 +500,22 @@ export function createServer(options = {}) { /* ------------------------------------------------------- system integration */ +/** + * The upstreams this machine was using before we touched anything. + * + * Read once, before routing is switched, because afterwards resolv.conf may + * point at us and the real servers are no longer discoverable from it. An + * empty result is the signal to leave routing per-ending: catch-all with + * nowhere to forward is every lookup on the box failing, not just Moshpit ones. + */ +export async function discoverUpstreams(readImpl) { + const read = readImpl || (async () => { + const { readFile } = await import("node:fs/promises"); + return readFile("/etc/resolv.conf", "utf8"); + }); + return parseUpstreams(await read().catch(() => "")); +} + /** * The routing suffixes the resolver actually accepted. * @@ -792,10 +808,19 @@ export async function dnsCommand(args = [], out = console.log) { // parking host, which is all there ever was. const park = parking ? parking.address : await parkingAddress(); if (!park) out("! parking host did not resolve — unpointed names will return NXDOMAIN"); + // Without these the bridge answers only for endings it is authoritative + // for, which is correct for per-ending routing and fatal for catch-all. + const upstreams = await discoverUpstreams(); + const tldSet = new Set(await fetchTlds({ registryBase }).catch(() => [])); + if (upstreams.length) out(`forwarding non-Moshpit lookups to ${upstreams.join(", ")}`); + else out("! no upstreams found in /etc/resolv.conf — this bridge can only answer Moshpit names"); + const server = await createServer({ port, registryBase, parkingAddress: park, + upstreams, + tldSet, onQuery: ({ name, address }) => out(` ${name} → ${address || "NXDOMAIN"}`), }); if (parking) out(`parked names → http://${parking.address}:${parking.port} → ${registryBase}/n/`); @@ -857,7 +882,7 @@ export async function dnsCommand(args = [], out = console.log) { let plan; try { plan = sub === "enable" - ? enablePlan({ platform, tlds, port: wanted, linuxBackend }) + ? enablePlan({ platform, tlds, port: wanted, linuxBackend, upstreams: await discoverUpstreams() }) : disablePlan({ platform, tlds, linuxBackend }); } catch (err) { out(err.message); diff --git a/test/dns-catchall.test.mjs b/test/dns-catchall.test.mjs index 4eccde5..f16566d 100644 --- a/test/dns-catchall.test.mjs +++ b/test/dns-catchall.test.mjs @@ -243,3 +243,37 @@ test("the shortfall reproduces the failure that started this", async () => { assert.equal(shortfall.missing.length, 3496); assert.equal(shortfall.missing[0], "t1090"); }); + +/* -------------------------------------- catch-all only when it is safe */ + +test("catch-all routing is written only when there is somewhere to forward", async () => { + const { enablePlan } = await import("../src/dns-system.mjs"); + const conf = (plan) => plan.steps.find((s) => s.path?.includes("moshpit.conf"))?.content ?? ""; + + // With upstreams: one line that never grows. + const withUp = enablePlan({ platform: "linux", tlds: ["eggs", "hacker"], upstreams: ["67.207.67.3"] }); + assert.match(conf(withUp), /^Domains=~\.$/m); + + // Without: the per-ending list, which cannot take the machine's DNS with it. + // Getting this backwards sends every lookup to a bridge with nowhere to + // forward, and the whole box loses DNS rather than just Moshpit names. + const withoutUp = enablePlan({ platform: "linux", tlds: ["eggs", "hacker"], upstreams: [] }); + assert.match(conf(withoutUp), /^Domains=~eggs ~hacker$/m); + assert.doesNotMatch(conf(withoutUp), /~\./); + + // Same rule for dnsmasq. + const dnsmasqOn = enablePlan({ platform: "linux", linuxBackend: "dnsmasq", tlds: ["eggs"], upstreams: ["1.1.1.1"] }); + assert.match(conf(dnsmasqOn), /^no-resolv$/m); + const dnsmasqOff = enablePlan({ platform: "linux", linuxBackend: "dnsmasq", tlds: ["eggs"], upstreams: [] }); + assert.match(conf(dnsmasqOff), /^server=\/eggs\//m); + assert.doesNotMatch(dnsmasqOff.steps.map((s) => s.content).join(""), /no-resolv/); +}); + +test("upstreams are read before routing is switched, and loopback is dropped", async () => { + const { discoverUpstreams } = await import("../src/dns.mjs"); + const resolv = "nameserver 127.0.0.53\nnameserver 67.207.67.3\nnameserver 67.207.67.2\n"; + assert.deepEqual(await discoverUpstreams(async () => resolv), ["67.207.67.3", "67.207.67.2"]); + // An unreadable resolv.conf must read as "no upstreams", which keeps routing + // per-ending rather than pointing everything at a bridge that cannot forward. + assert.deepEqual(await discoverUpstreams(async () => { throw new Error("nope"); }), []); +});