diff --git a/src/dns-service.mjs b/src/dns-service.mjs index a54c604..3c58f08 100644 --- a/src/dns-service.mjs +++ b/src/dns-service.mjs @@ -29,9 +29,8 @@ // it, and the entry is the script this very command was invoked from. Nothing // is guessed and nothing depends on PATH. import { spawn } from "node:child_process"; -import { mkdir, rm, writeFile } from "node:fs/promises"; -import { existsSync } from "node:fs"; -import { homedir } from "node:os"; +import { mkdir, readFile, rm, writeFile } from "node:fs/promises"; +import { existsSync, statSync } from "node:fs"; import { dirname, join } from "node:path"; import { operatorHome } from "./trust.mjs"; @@ -48,10 +47,49 @@ export const UNIT_NAME = "moshcode-dns.service"; * /run/user//moshpit-dns.pid for the daemon and for the person asking * after it. Under a system unit those are two different paths. */ -export function servicePaths({ system = false, home = homedir() } = {}) { +export function servicePaths({ system = false, home = operatorHome(), env = process.env } = {}) { return system ? { path: join("/etc/systemd/system", UNIT_NAME), systemctl: ["systemctl"], scope: "system" } - : { path: join(home, ".config/systemd/user", UNIT_NAME), systemctl: ["systemctl", "--user"], scope: "user" }; + : { path: join(home, ".config/systemd/user", UNIT_NAME), systemctl: userSystemctl(env), scope: "user" }; +} + +/** + * How to reach the operator's own systemd from wherever this is running. + * + * `systemctl --user` talks to the session of whoever is running it. `dns enable` + * escalates, so from there it is root's session — which has no bridge in it, has + * never had one, and reports every query about one as "not loaded". Meanwhile + * the operator's bridge keeps running with whatever it started with. + * + * That is why enabling proxy mode could be detected, written, and still not take + * effect: the unit that had to change belongs to a session the escalated half of + * the command cannot see. + * + * So an escalated run drops back to the invoking user, and hands them the runtime + * directory their session bus lives in — deriving it rather than inheriting it, + * because sudo does not carry XDG_RUNTIME_DIR across and the default under sudo + * points at root's. + */ +export function userSystemctl(env = process.env, { home = operatorHome({ env }), owner = ownerOf } = {}) { + const user = env.SUDO_USER || env.DOAS_USER; + // Not escalated, or escalated from root itself: the session in reach is the + // right one. + if (!user || user === "root") return ["systemctl", "--user"]; + // sudo publishes the uid; doas publishes only the name. Falling back to the + // owner of the operator's home covers that, and covers an escalator that + // publishes neither — without it, a doas machine would quietly address root's + // session, which has no bridge in it and never will. + const uid = env.SUDO_UID || env.DOAS_UID || owner(home); + if (uid === null || uid === undefined) return ["systemctl", "--user"]; + return ["sudo", "-u", user, "env", `XDG_RUNTIME_DIR=/run/user/${uid}`, "systemctl", "--user"]; +} + +function ownerOf(path) { + try { + return statSync(path).uid; + } catch { + return null; + } } /** @@ -127,6 +165,8 @@ export function serviceUnit({ return lines.join("\n"); } +const defaultRead = async (path) => readFile(path, "utf8").catch(() => ""); + function run(command, args) { return new Promise((resolve) => { const child = spawn(command, args, { stdio: ["ignore", "pipe", "pipe"] }); @@ -140,8 +180,8 @@ function run(command, args) { } /** Write the unit and start it. Returns the steps taken, in order, for printing. */ -export async function installService(unit, { system = false, home = homedir(), exec = run } = {}) { - const { path, systemctl, scope } = servicePaths({ system, home }); +export async function installService(unit, { system = false, home = operatorHome(), exec = run, env = process.env } = {}) { + const { path, systemctl, scope } = servicePaths({ system, home, env }); const steps = []; try { await mkdir(dirname(path), { recursive: true }); @@ -152,7 +192,14 @@ export async function installService(unit, { system = false, home = homedir(), e } const [cmd, ...flags] = systemctl; - for (const args of [[...flags, "daemon-reload"], [...flags, "enable", "--now", UNIT_NAME]]) { + // Enable *and* restart. `enable --now` starts a stopped unit and does nothing + // to a running one, so rewriting the unit to add `--proxy` would leave the old + // bridge running without it — the change on disk, no change in behaviour. + for (const args of [ + [...flags, "daemon-reload"], + [...flags, "enable", UNIT_NAME], + [...flags, "restart", UNIT_NAME], + ]) { const result = await exec(cmd, args); steps.push({ step: `${cmd} ${args.join(" ")}`, ok: result.ok, error: result.error }); if (!result.ok) return { ok: false, path, scope, steps }; @@ -160,9 +207,119 @@ export async function installService(unit, { system = false, home = homedir(), e return { ok: true, path, scope, steps }; } +/** + * The `--upstream` servers an installed unit already forwards to. + * + * Read back rather than recomputed. A supervised bridge needs upstreams to hand + * the clearnet to, and the machine may already be routing every lookup at that + * bridge — so asking the system resolver what its upstreams are can answer + * "this bridge", and a bridge whose upstream is itself resolves nothing at all. + * Whatever the unit was working with is the safe answer to keep. + */ +export function unitUpstreams(text) { + const line = String(text ?? "").split("\n").find((l) => l.startsWith("ExecStart=")); + if (!line) return []; + const parts = line.trim().split(/\s+/); + const found = []; + for (let i = 0; i < parts.length; i += 1) { + if (parts[i] !== "--upstream") continue; + const value = parts[i + 1]; + if (!value || value.startsWith("--")) continue; + if (!found.includes(value)) found.push(value); + } + return found; +} + +/** + * Re-describe the bridge unit to match the run happening now, and restart it so + * the description becomes the truth. + * + * This is the step that made proxy mode arrive one reboot late. A supervised + * bridge is not started by `enable`: it is already up under `Restart=always`, + * so `startDaemon` finds a live pidfile and reports "already running" — true, + * and useless, because what a resolver answers with is fixed when it spawns. A + * bridge that came up before the proxy existed goes on answering origins + * forever, and stopping it by hand does not help, since systemd brings the same + * ExecStart straight back. + * + * The only thing that changes a supervised bridge's mind is rewriting its unit + * and restarting it. That is all this is. + * + * With no unit installed it does nothing and says so. An unsupervised machine + * is `startDaemon`'s business, and writing a unit here would be `enable` + * quietly making the bridge outlive a reboot on a machine that never asked for + * that — a different decision, and one `dns service --write` exists to make. + */ +export async function refreshService({ + entry, + port, + registryBase = null, + proxy = null, + system = false, + home = operatorHome(), + env = process.env, + exec = run, + exists = existsSync, + read = defaultRead, +} = {}) { + const { path, scope } = servicePaths({ system, home, env }); + if (!exists(path)) return { refreshed: false, reason: "no unit installed", path, scope, upstreams: [], steps: [] }; + + const current = await read(path); + const upstreams = unitUpstreams(current); + const unit = serviceUnit({ system, entry, port, registryBase, upstreams, proxy }); + + // Deliberately not short-circuited on `current === unit`. Matching text says + // the unit describes the right bridge, not that the bridge is running it: a + // unit can be installed and stopped, installed and never enabled, or running + // what it was spawned with before the file last changed. Since the whole + // point here is to make what is running match what is written, the enable and + // restart happen either way, and cost a moment of no resolver during a + // command that is already rewriting the machine's routing. + + const result = await installService(unit, { system, home, env, exec }); + return { + refreshed: result.ok, + reason: result.ok ? null : "systemctl refused the unit", + path, + scope, + upstreams, + steps: result.steps, + }; +} + +/** + * Stop the supervised bridge and leave the unit where it is. + * + * `stopDaemon` cannot do this. It reads the pidfile and signals that process, + * which is right for a bridge started by hand and useless for one systemd owns: + * the unit is `Restart=always`, so the pid dies and the same ExecStart is back + * within the second. `disable` printed "bridge stopped" and left a bridge + * running — on a machine whose routing had just been put back, so the bridge + * was still up, still answering, and no longer on anybody's path. + * + * The unit file stays. Removing it is a different decision than turning + * resolution off for an afternoon, and `enable` re-enables what it finds — so + * leaving it costs nothing and deleting it would quietly take away a unit the + * operator may have written themselves. + */ +export async function stopService({ system = false, home = operatorHome(), env = process.env, exec = run, exists = existsSync } = {}) { + const { path, systemctl, scope } = servicePaths({ system, home, env }); + if (!exists(path)) return { stopped: false, reason: "no unit installed", path, scope, steps: [] }; + const [cmd, ...flags] = systemctl; + const result = await exec(cmd, [...flags, "disable", "--now", UNIT_NAME]); + return { + stopped: result.ok, + reason: result.ok ? null : (result.error || "systemctl refused"), + path, + scope, + steps: [{ step: `${cmd} ${flags.join(" ")} disable --now ${UNIT_NAME}`, ok: result.ok, error: result.error }], + }; +} + /** Stop it and take the unit away. Missing is not a failure — removal is idempotent. */ -export async function removeService({ system = false, home = homedir(), exec = run } = {}) { - const { path, systemctl, scope } = servicePaths({ system, home }); +export async function removeService({ system = false, home = operatorHome(), exec = run, env = process.env } = {}) { + const { path, systemctl, scope } = servicePaths({ system, home, env }); const [cmd, ...flags] = systemctl; const steps = []; for (const args of [[...flags, "disable", "--now", UNIT_NAME]]) { diff --git a/src/dns.mjs b/src/dns.mjs index 82bc4bf..c2098a5 100644 --- a/src/dns.mjs +++ b/src/dns.mjs @@ -2148,6 +2148,59 @@ export async function verifyResolution({ return { ok: checks.every((c) => c.ok), checks }; } +/** + * Wait for a bridge that systemd has just restarted to start answering. + * + * `startDaemon` cannot be asked this. It spawns, watches its own child, and + * decides from a pidfile — none of which describes a unit that systemd owns and + * has just cycled. Calling it here would either report the pre-restart pid as + * "already running" or, on a pidfile not yet rewritten, spawn a second bridge + * against the one systemd is bringing up. + * + * `Type=simple` reports active the moment the process forks, so systemd saying + * the restart worked is not yet a resolver that answers. Hence the probe. + * + * A timeout is reported as started-but-unverified rather than as a failure, the + * same way `startDaemon` treats a live process that has not answered yet: the + * unit is active, and refusing this machine its DNS over a slow first registry + * fetch would be the worse mistake. + */ +export async function supervisedReady({ + host = DEFAULT_HOST, + port, + probe = probeResolver, + status = daemonStatus, + timeoutMs = READY_TIMEOUT_MS, + sleep = (ms) => new Promise((r) => setTimeout(r, ms)), +} = {}) { + const deadline = Date.now() + timeoutMs; + let answered = false; + while (Date.now() < deadline) { + if (await probe({ host, port }).catch(() => false)) { + answered = true; + break; + } + await sleep(150); + } + const current = await Promise.resolve(status()).catch(() => null); + if (!answered) { + // Reported as a failure, unlike `startDaemon`'s slow-but-alive case, and + // for a reason that does not apply there: that one has watched its own + // child and knows it is running. Nothing here has. `systemctl restart` + // returns as soon as a Type=simple unit forks, so it returns 0 for a bridge + // that forked and died — and the next thing this run does is point every + // lookup on the machine at that port. Refusing is the safe direction. + return { + started: false, + alreadyRunning: false, + pid: current?.pid ?? null, + supervised: true, + error: `${UNIT_NAME} restarted but the bridge did not answer on ${host}:${port}`, + }; + } + return { started: true, pid: current?.pid ?? null, alreadyRunning: false, supervised: true, verified: true }; +} + const defaultReadMaybe = async (path) => { const { readFile: rf } = await import("node:fs/promises"); return rf(path, "utf8").catch(() => null); @@ -2407,10 +2460,10 @@ import { readFile, writeFile } from "node:fs/promises"; import { existsSync } from "node:fs"; import { fileURLToPath } from "node:url"; import { isRealTld } from "./iana-tlds.mjs"; -import { installService, removeService, serviceUnit, servicePaths, UNIT_NAME, ensureProxyService, removeProxyService, proxyServicePaths, proxyWrapperPath } from "./dns-service.mjs"; +import { installService, refreshService, removeService, serviceUnit, servicePaths, stopService, UNIT_NAME, ensureProxyService, removeProxyService, proxyServicePaths, proxyWrapperPath } from "./dns-service.mjs"; import { applyPlan, daemonStatus, describePlan, detectPlatform, disablePlan, enablePlan, - probeResolver, requiredPort, startDaemon, stopDaemon, + probeResolver, READY_TIMEOUT_MS, requiredPort, startDaemon, stopDaemon, } from "./dns-system.mjs"; import { escalateSelf } from "./escalate.mjs"; @@ -2508,10 +2561,13 @@ export async function dnsCommand(args = [], out = console.log, deps = {}) { presenceImpl = bridgePresence, exists = existsSync, startBridge = startDaemon, + refreshBridge = refreshService, + bridgeReady = supervisedReady, proxyReachableImpl = proxyReachable, findLocalProxyImpl = findLocalProxy, autoTrustImpl = createAutoTrust, stopBridge = stopDaemon, + stopSupervised = stopService, // The two proxy-service calls, injected for the same reason as every // other system call here: a test must be able to exercise the branch // without shelling out to systemctl or writing to /etc. @@ -3224,6 +3280,19 @@ export async function dnsCommand(args = [], out = console.log, deps = {}) { return 1; } + // The supervised bridge first, and by asking systemd rather than by + // signalling a pid. `stopDaemon` kills what the pidfile names, and the + // unit is `Restart=always` — so the process died, systemd replaced it + // within the second, and this command reported "bridge stopped" on a + // machine where the bridge was still up and answering, just no longer on + // anything's path. + const unsupervised = await stopSupervised(); + if (unsupervised.reason !== "no unit installed") { + out(unsupervised.stopped + ? ` ok ${UNIT_NAME} stopped and disabled` + : ` -- could not stop ${UNIT_NAME} (${unsupervised.reason}) — it will restart itself`); + } + const stopped = await stopBridge(); out(stopped.stopped ? " ok bridge stopped" : ` ok bridge was not running${stopped.reason ? ` (${stopped.reason})` : ""}`); @@ -3412,17 +3481,42 @@ export async function dnsCommand(args = [], out = console.log, deps = {}) { } } + // A bridge under systemd is re-described, not started — and this is the step + // whose absence left the last of this to be done by hand. + // + // `startBridge` below reports "already running" for a supervised bridge and + // leaves it alone. That is correct, and it is also why proxy mode arrived a + // reboot late: what a resolver answers with is fixed when it spawns, so a + // bridge that came up before the proxy existed goes on answering origins no + // matter what this run decides. Stopping it does not help either, since + // `Restart=always` brings the same ExecStart back. + // + // So the unit is rewritten to match the run happening now, and restarted. + // v4 by preference: `dns start --proxy` takes one address and probes both + // families itself, so handing it the v4 loopback lets it find ::1 too + // rather than pinning the answer to one family. + const proxyArg = proxyAddress ? (proxyAddress.v4 || proxyAddress.v6) : null; + + let refreshed = null; + if (!reusing && platform === "linux") { + refreshed = await refreshBridge({ entry: cliEntry(), port: wanted, registryBase, proxy: proxyArg }); + for (const step of refreshed.steps || []) { + out(` ${step.ok ? "ok " : "-- "} ${step.step}${step.error ? ` — ${step.error}` : ""}`); + } + if (refreshed.refreshed) { + const forwarding = refreshed.upstreams?.length ? `, forwarding the clearnet to ${refreshed.upstreams.join(", ")}` : ""; + out(` ok ${UNIT_NAME} restarted with proxy mode ${proxyArg ? "on" : "off"}${forwarding}`); + } else if (refreshed.reason !== "no unit installed") { + out(` -- could not update ${UNIT_NAME} (${refreshed.reason})`); + out(" the bridge already running keeps the mode it started with"); + } + } + const started = reusing ? { started: false, pid: reusing.pid, alreadyRunning: true, reused: true } - : await startBridge({ - port: wanted, - registryBase, - entry: cliEntry(), - // v4 by preference: `dns start --proxy` takes one address and probes - // both families itself, so handing it the v4 loopback lets it find ::1 - // too rather than pinning the answer to one family. - proxy: proxyAddress ? (proxyAddress.v4 || proxyAddress.v6) : null, - }); + : refreshed?.refreshed + ? await bridgeReady({ host: DEFAULT_HOST, port: wanted }) + : await startBridge({ port: wanted, registryBase, entry: cliEntry(), proxy: proxyArg }); // The routing this is about to install is catch-all — every lookup on the // machine, not just Moshpit ones — so a bridge that did not come up is not // a degraded feature, it is the machine's resolver pointed at nothing. @@ -3504,7 +3598,13 @@ export async function dnsCommand(args = [], out = console.log, deps = {}) { out(`Moshpit names now resolve on this machine. Try: moshcode dns resolve ${moshpitProbe || ""}`); out(`Routing covers the ${tlds.length} TLDs claimed right now. New ones do not route`); out("until you re-run this — there is no common suffix to match, so every TLD is listed."); - out("Note: the bridge does not yet survive a reboot. Re-run `moshcode dns enable` after one."); + // Only where it is still true. On a supervised machine the unit was just + // enabled and restarted, so the bridge does come back — and telling + // someone to re-run a command they do not need is how advice stops being + // read at all. + out(started.supervised + ? `Note: ${UNIT_NAME} brings the bridge back after a reboot.` + : "Note: the bridge does not yet survive a reboot. Re-run `moshcode dns enable` after one."); return 0; } @@ -3512,7 +3612,14 @@ export async function dnsCommand(args = [], out = console.log, deps = {}) { report(outcome.rolledBack.results); // Started by this run and no longer routed to, so leaving it would be a // process holding 5354 that the next enable's preflight refuses to run past. - if (started.started) { + // + // A supervised bridge is exempt: this run did not start it, only restarted + // it, so it was holding that port before the run and is meant to go on + // holding it. Signalling its pid would not stop it anyway — `Restart=always` + // replaces it within the second — so the only thing the old line achieved + // there was printing "remove bridge started by this run" about a bridge + // that was neither started by this run nor removed. + if (started.started && !started.supervised) { const stopped = await stopBridge(); if (stopped.stopped) out(" ok remove bridge started by this run"); } diff --git a/test/dns-enable-rollback.test.mjs b/test/dns-enable-rollback.test.mjs index b436547..c70770b 100644 --- a/test/dns-enable-rollback.test.mjs +++ b/test/dns-enable-rollback.test.mjs @@ -463,6 +463,20 @@ function noSystem() { // whatever holds 443 on the machine running the suite. findLocalProxyImpl: async () => ({ found: false, why: null, address: { v4: null, v6: null } }), startBridge: async () => ({ started: true, pid: 1, alreadyRunning: false }), + // No systemd unit on this machine, which is the state these tests were + // written in. Stubbed rather than left to the real one, which would read + // the suite runner's own ~/.config/systemd/user and, on a developer's box, + // restart their actual bridge. + refreshBridge: async () => ({ refreshed: false, reason: "no unit installed", upstreams: [], steps: [] }), + stopSupervised: async () => ({ stopped: false, reason: "no unit installed", steps: [] }), + // Both of these reach the real machine when left to their defaults: one + // looks for an installed proxy wrapper and starts it, the other writes + // /etc/systemd/resolved.conf.d and restarts systemd-resolved. A test that + // forgets to stub them does not fail cleanly — it either edits the system + // running the suite or, unprivileged, fails on EACCES from somewhere that + // reads as a bug in whatever it was actually testing. + proxyWrapper: () => null, + applyWith: async () => ({ saved: { ok: true }, applied: { ok: true, results: [] }, verified: { ok: true, checks: [] }, rolledBack: null, backups: [] }), stopBridge: async () => ({ stopped: true, reason: null }), dropins: async () => [], readManifest: async () => null, @@ -538,3 +552,196 @@ test("with no proxy installed, nothing is claimed and DNS still comes up", async assert.equal(startedWith?.proxy, null); assert.match(lines.join("\n"), /no pinned-TLS proxy installed/); }); + +/* ------------------- a supervised bridge is re-described, not left alone ---*/ + +// The last thing that had to be done by hand. `startBridge` reports "already +// running" for a bridge systemd owns and leaves it alone — correct, and the +// reason proxy mode arrived a reboot late: what a resolver answers with is +// fixed when it spawns, so a bridge that came up before the proxy existed goes +// on answering origins whatever this run decides. Stopping it does not help +// either, because `Restart=always` brings the same ExecStart back. + +test("an installed unit is rewritten with proxy mode and restarted", async () => { + const lines = []; + let asked = null; + let startedByHand = false; + const code = await dnsCommand(["enable"], (l) => lines.push(String(l)), { + ...noSystem(), + proxyWrapper: () => "/home/x/.local/bin/moshpit-proxy", + ensureProxy: async () => ({ ok: true, steps: [] }), + refreshBridge: async (opts) => { + asked = opts; + return { refreshed: true, reason: null, upstreams: ["1.1.1.1"], steps: [{ step: "systemctl --user restart moshcode-dns.service", ok: true }] }; + }, + bridgeReady: async () => ({ started: true, pid: 99, alreadyRunning: false, supervised: true, verified: true }), + startBridge: async () => { startedByHand = true; return { started: true, pid: 1, alreadyRunning: false }; }, + }); + + assert.equal(code, 0); + assert.equal(asked?.proxy, "127.0.0.1", "the unit has to carry --proxy, or the bridge still answers origins"); + assert.equal(startedByHand, false, "systemd owns this bridge — starting a second one is how two bridges end up on the port"); + assert.match(lines.join("\n"), /restarted with proxy mode on/); + assert.match(lines.join("\n"), /forwarding the clearnet to 1\.1\.1\.1/); +}); + +test("with no unit installed, nothing is said about one and the daemon is started", async () => { + const lines = []; + let startedByHand = false; + const code = await dnsCommand(["enable"], (l) => lines.push(String(l)), { + ...noSystem(), + startBridge: async () => { startedByHand = true; return { started: true, pid: 1, alreadyRunning: false }; }, + }); + + assert.equal(code, 0); + assert.equal(startedByHand, true); + assert.doesNotMatch(lines.join("\n"), /could not update/, "an unsupervised machine is not a failure to report"); +}); + +test("a unit that restarts but never answers refuses the machine its routing", async () => { + // `systemctl restart` returns as soon as a Type=simple unit forks, so it + // returns 0 for a bridge that forked and died. The next thing enable does is + // point every lookup on the machine at that port. + const lines = []; + let applied = false; + const code = await dnsCommand(["enable"], (l) => lines.push(String(l)), { + ...noSystem(), + refreshBridge: async () => ({ refreshed: true, reason: null, upstreams: [], steps: [] }), + bridgeReady: async () => ({ started: false, alreadyRunning: false, pid: null, supervised: true, error: "moshcode-dns.service restarted but the bridge did not answer on 127.0.0.1:5354" }), + applyWith: async () => { applied = true; return { ok: true, steps: [] }; }, + }); + + assert.equal(code, 1); + assert.equal(applied, false, "nothing may be written when the bridge is not answering"); + assert.match(lines.join("\n"), /did not answer/); + assert.match(lines.join("\n"), /Nothing has been changed/); +}); + +test("a unit that cannot be updated is reported, and the run continues", async () => { + // A refusal from systemctl is worth naming — proxy mode will not take effect + // — but it is not a reason to leave the machine without DNS. + const lines = []; + let startedByHand = false; + const code = await dnsCommand(["enable"], (l) => lines.push(String(l)), { + ...noSystem(), + refreshBridge: async () => ({ refreshed: false, reason: "systemctl refused the unit", upstreams: [], steps: [{ step: "systemctl --user restart moshcode-dns.service", ok: false, error: "Interactive authentication required" }] }), + startBridge: async () => { startedByHand = true; return { started: true, pid: 1, alreadyRunning: false }; }, + }); + + assert.equal(code, 0); + assert.equal(startedByHand, true); + assert.match(lines.join("\n"), /could not update moshcode-dns\.service/); + assert.match(lines.join("\n"), /keeps the mode it started with/); +}); + +/* ------------------ turning it off has to stop a bridge systemd is holding -*/ + +test("disable stops the unit through systemd, not by signalling a pid", async () => { + // `stopDaemon` kills what the pidfile names, and the unit is Restart=always: + // the process dies and systemd has the same ExecStart back within the second. + // So disable printed "bridge stopped" on a machine where the bridge was still + // up and answering — just no longer on anything's path. + const lines = []; + let askedSystemd = false; + const code = await dnsCommand(["disable"], (l) => lines.push(String(l)), { + ...noSystem(), + dropins: async () => [{ name: "moshpit.conf", content: "[Resolve]\nDNS=127.0.0.1:5354\nDomains=~.\n" }], + stopSupervised: async () => { askedSystemd = true; return { stopped: true, reason: null, steps: [] }; }, + }); + + assert.equal(code, 0); + assert.equal(askedSystemd, true, "killing the pid alone leaves a bridge systemd will restart"); + assert.match(lines.join("\n"), /moshcode-dns\.service stopped and disabled/); +}); + +test("disable says nothing about a unit on a machine that has none", async () => { + const lines = []; + const code = await dnsCommand(["disable"], (l) => lines.push(String(l)), { + ...noSystem(), + dropins: async () => [{ name: "moshpit.conf", content: "[Resolve]\nDNS=127.0.0.1:5354\nDomains=~.\n" }], + }); + + assert.equal(code, 0); + assert.doesNotMatch(lines.join("\n"), /moshcode-dns\.service/); +}); + +test("a unit that will not stop is named, because it is about to come back", async () => { + const lines = []; + const code = await dnsCommand(["disable"], (l) => lines.push(String(l)), { + ...noSystem(), + dropins: async () => [{ name: "moshpit.conf", content: "[Resolve]\nDNS=127.0.0.1:5354\nDomains=~.\n" }], + stopSupervised: async () => ({ stopped: false, reason: "Interactive authentication required", steps: [] }), + }); + + assert.equal(code, 0, "a bridge that will not stop is not a reason to leave the routing in place"); + assert.match(lines.join("\n"), /could not stop moshcode-dns\.service/); + assert.match(lines.join("\n"), /it will restart itself/); +}); + +test("a supervised machine is not told to re-run enable after a reboot", async () => { + // The unit was just enabled and restarted, so the bridge does come back. + // Telling someone to re-run a command they do not need is how advice stops + // being read at all. + const lines = []; + await dnsCommand(["enable"], (l) => lines.push(String(l)), { + ...noSystem(), + refreshBridge: async () => ({ refreshed: true, reason: null, upstreams: [], steps: [] }), + bridgeReady: async () => ({ started: true, pid: 99, alreadyRunning: false, supervised: true, verified: true }), + }); + + const out = lines.join("\n"); + assert.match(out, /moshcode-dns\.service brings the bridge back after a reboot/); + assert.doesNotMatch(out, /does not yet survive a reboot/); +}); + +test("an unsupervised machine is still told the bridge dies at reboot", async () => { + const lines = []; + await dnsCommand(["enable"], (l) => lines.push(String(l)), { ...noSystem() }); + assert.match(lines.join("\n"), /does not yet survive a reboot/); +}); + +test("a rollback leaves a supervised bridge where it found it", async () => { + // It was holding the port before this run and is meant to go on holding it. + // Signalling its pid would not stop it anyway — Restart=always replaces it + // within the second — so the old line printed "remove bridge started by this + // run" about a bridge that was neither started by this run nor removed. + const lines = []; + let killed = false; + const code = await dnsCommand(["enable"], (l) => lines.push(String(l)), { + ...noSystem(), + refreshBridge: async () => ({ refreshed: true, reason: null, upstreams: [], steps: [] }), + bridgeReady: async () => ({ started: true, pid: 99, alreadyRunning: false, supervised: true, verified: true }), + applyWith: async () => ({ + saved: { ok: true }, + applied: { ok: true, results: [] }, + verified: { ok: false, checks: [{ name: "pit.moshcode.sh", kind: "clearnet", ok: false, error: "ENOTFOUND" }] }, + rolledBack: { ok: true, results: [] }, + backups: [], + }), + stopBridge: async () => { killed = true; return { stopped: true, reason: null }; }, + }); + + assert.equal(code, 1); + assert.equal(killed, false, "systemd would put it straight back, so the kill is noise that reads as a fact"); + assert.doesNotMatch(lines.join("\n"), /remove bridge started by this run/); +}); + +test("a rollback still cleans up a bridge this run really did start", async () => { + const lines = []; + let killed = false; + await dnsCommand(["enable"], (l) => lines.push(String(l)), { + ...noSystem(), + startBridge: async () => ({ started: true, pid: 7, alreadyRunning: false }), + applyWith: async () => ({ + saved: { ok: true }, + applied: { ok: true, results: [] }, + verified: { ok: false, checks: [{ name: "pit.moshcode.sh", kind: "clearnet", ok: false, error: "ENOTFOUND" }] }, + rolledBack: { ok: true, results: [] }, + backups: [], + }), + stopBridge: async () => { killed = true; return { stopped: true, reason: null }; }, + }); + + assert.equal(killed, true, "an unsupervised bridge left on 5354 is what the next preflight refuses to run past"); + assert.match(lines.join("\n"), /remove bridge started by this run/); +}); diff --git a/test/dns-service.test.mjs b/test/dns-service.test.mjs index f016fea..5fded55 100644 --- a/test/dns-service.test.mjs +++ b/test/dns-service.test.mjs @@ -19,12 +19,12 @@ // that looks plausible and dies at 203/EXEC with nothing useful in the journal. import test from "node:test"; import assert from "node:assert/strict"; -import { mkdtemp, mkdir, writeFile } from "node:fs/promises"; +import { mkdtemp, mkdir, readFile, writeFile } from "node:fs/promises"; import { existsSync } from "node:fs"; import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { dirname, join } from "node:path"; -import { installService, removeService, serviceUnit, servicePaths, UNIT_NAME } from "../src/dns-service.mjs"; +import { installService, refreshService, removeService, serviceUnit, servicePaths, stopService, unitUpstreams, userSystemctl, UNIT_NAME } from "../src/dns-service.mjs"; import { proxyProbeFromArgs, captureRestorePoint, probeName } from "../src/dns.mjs"; import { proxyServiceUnit, proxyServicePaths, PROXY_UNIT_NAME } from "../src/dns-service.mjs"; import { rootIsNarrow, ensureProxyService } from "../src/dns-service.mjs"; @@ -89,11 +89,28 @@ test("install writes the unit, then reloads and enables — and stops at the fir const result = await installService(unit(), { home, exec }); assert.equal(result.ok, false); - assert.deepEqual(calls, ["systemctl --user daemon-reload", `systemctl --user enable --now ${UNIT_NAME}`]); + assert.deepEqual(calls, ["systemctl --user daemon-reload", `systemctl --user enable ${UNIT_NAME}`]); assert.equal(existsSync(join(home, ".config/systemd/user", UNIT_NAME)), true, "the file is written before systemctl is asked about it"); assert.equal(result.steps.at(-1).error, "Failed to enable", "the reason systemctl gave is carried back, not swallowed"); }); +test("install restarts, so a rewritten unit actually takes effect", async () => { + // `enable --now` starts a stopped unit and does nothing at all to a running + // one. That is why rewriting the unit to add `--proxy` used to change the + // file and nothing else: the bridge kept running with the arguments it was + // spawned with, and proxy mode arrived on the next reboot. + const home = await scratch(); + const calls = []; + const exec = async (cmd, args) => { + calls.push(args.join(" ")); + return { ok: true }; + }; + + const result = await installService(unit(), { home, exec }); + assert.equal(result.ok, true); + assert.deepEqual(calls, ["--user daemon-reload", `--user enable ${UNIT_NAME}`, `--user restart ${UNIT_NAME}`]); +}); + test("remove takes the unit away even when it was never enabled", async () => { const home = await scratch(); // `disable` on a unit systemd has never heard of is an error, and removal @@ -388,3 +405,175 @@ test("--proxy-probe wins, for a machine that needs a specific real name", () => assert.equal(probeName(["hacker"], ["--proxy-probe"]), "a.hacker"); assert.equal(probeName(["hacker"], ["--proxy-probe", "--write"]), "a.hacker"); }); + +/* --------------------------------- reaching the operator's own systemd ---- */ + +// `systemctl --user` talks to the session of whoever runs it, and `dns enable` +// escalates. From there it is root's session: no bridge in it, none ever, and +// every question about one answered "not loaded" — while the operator's bridge +// keeps running with whatever it was spawned with. That is why the unit could +// be rewritten and still not take effect. + +test("an escalated run drives the invoking user's systemd, not root's", () => { + assert.deepEqual( + userSystemctl({ SUDO_USER: "ettinger", SUDO_UID: "1000" }), + ["sudo", "-u", "ettinger", "env", "XDG_RUNTIME_DIR=/run/user/1000", "systemctl", "--user"], + ); +}); + +test("an unescalated run uses the session it is already in", () => { + assert.deepEqual(userSystemctl({}), ["systemctl", "--user"]); +}); + +test("root running this directly is not sent back through sudo to itself", () => { + // SUDO_USER=root happens with `sudo -u root`, and also survives in the + // environment of anything root spawns afterwards. + assert.deepEqual(userSystemctl({ SUDO_USER: "root", SUDO_UID: "0" }), ["systemctl", "--user"]); +}); + +test("doas publishes a name and no uid, so the uid comes off the operator's home", () => { + // sudo sets SUDO_UID; doas sets DOAS_USER alone. Without this the doas case + // addresses root's session — which has no bridge in it and never will — and + // does it silently. + assert.deepEqual( + userSystemctl({ DOAS_USER: "ettinger" }, { home: "/home/ettinger", owner: () => 1000 }), + ["sudo", "-u", "ettinger", "env", "XDG_RUNTIME_DIR=/run/user/1000", "systemctl", "--user"], + ); +}); + +test("an unresolvable uid falls back rather than building a command with 'null' in it", () => { + assert.deepEqual( + userSystemctl({ SUDO_USER: "ettinger" }, { home: "/home/gone", owner: () => null }), + ["systemctl", "--user"], + ); +}); + +/* ------------------------------------- upstreams are kept, not recomputed - */ + +test("the upstreams already in a unit are read back off its ExecStart", () => { + const text = [ + "[Service]", + "ExecStart=/usr/bin/node /home/x/bin/moshcode.mjs dns start --port 5354 --upstream 1.1.1.1 --upstream 8.8.8.8#53 --proxy 127.0.0.1", + ].join("\n"); + assert.deepEqual(unitUpstreams(text), ["1.1.1.1", "8.8.8.8#53"]); +}); + +test("a unit with no upstreams, and a malformed one, read as none rather than throwing", () => { + assert.deepEqual(unitUpstreams("[Service]\nExecStart=/usr/bin/node x dns start --port 5354"), []); + assert.deepEqual(unitUpstreams("ExecStart=/usr/bin/node x dns start --upstream --proxy 127.0.0.1"), [], "a flag is not an upstream"); + assert.deepEqual(unitUpstreams(""), []); + assert.deepEqual(unitUpstreams(null), []); +}); + +/* ------------------------------------------------- refreshing the unit ---- */ + +test("with no unit installed, refresh does nothing and says which", async () => { + const home = await scratch(); + let ran = false; + const result = await refreshService({ + entry: "/home/x/bin/moshcode.mjs", port: 5354, home, + exec: async () => { ran = true; return { ok: true }; }, + }); + assert.equal(result.refreshed, false); + assert.equal(result.reason, "no unit installed"); + assert.equal(ran, false, "an unsupervised machine is startDaemon's business"); +}); + +test("refresh keeps the unit's upstreams and adds the proxy this run found", async () => { + const home = await scratch(); + const unitPath = join(home, ".config/systemd/user", UNIT_NAME); + await mkdir(dirname(unitPath), { recursive: true }); + await writeFile(unitPath, [ + "[Service]", + "ExecStart=/usr/bin/node /home/x/bin/moshcode.mjs dns start --port 5354 --upstream 9.9.9.9", + "", + ].join("\n")); + + const calls = []; + const result = await refreshService({ + entry: "/home/x/bin/moshcode.mjs", port: 5354, registryBase: "https://pit.moshcode.sh", proxy: "127.0.0.1", + home, exec: async (cmd, args) => { calls.push(args.join(" ")); return { ok: true }; }, + }); + + assert.equal(result.refreshed, true); + assert.deepEqual(result.upstreams, ["9.9.9.9"]); + + const written = await readFile(unitPath, "utf8"); + assert.match(written, /--proxy 127\.0\.0\.1/); + assert.match(written, /--upstream 9\.9\.9\.9/, "dropping the upstreams would leave the bridge with nothing to forward the clearnet to"); + assert.deepEqual(calls, ["--user daemon-reload", `--user enable ${UNIT_NAME}`, `--user restart ${UNIT_NAME}`]); +}); + +test("an unchanged unit is still enabled and restarted", async () => { + // Matching text says the unit describes the right bridge, not that the bridge + // is running it: installed-and-stopped and running-something-older are both + // real states, and both look identical to a text comparison. + const home = await scratch(); + const unitPath = join(home, ".config/systemd/user", UNIT_NAME); + await mkdir(dirname(unitPath), { recursive: true }); + const settled = serviceUnit({ entry: "/home/x/bin/moshcode.mjs", port: 5354, proxy: "127.0.0.1" }); + await writeFile(unitPath, settled); + + const calls = []; + const result = await refreshService({ + entry: "/home/x/bin/moshcode.mjs", port: 5354, proxy: "127.0.0.1", + home, exec: async (cmd, args) => { calls.push(args.join(" ")); return { ok: true }; }, + }); + + assert.equal(result.refreshed, true); + assert.ok(calls.includes(`--user restart ${UNIT_NAME}`)); +}); + +test("systemctl refusing is carried back rather than reported as success", async () => { + const home = await scratch(); + const unitPath = join(home, ".config/systemd/user", UNIT_NAME); + await mkdir(dirname(unitPath), { recursive: true }); + await writeFile(unitPath, "[Service]\nExecStart=/usr/bin/node x dns start --port 5354\n"); + + const result = await refreshService({ + entry: "/home/x/bin/moshcode.mjs", port: 5354, home, + exec: async (cmd, args) => (args.includes("restart") + ? { ok: false, error: "Interactive authentication required" } + : { ok: true }), + }); + + assert.equal(result.refreshed, false); + assert.equal(result.reason, "systemctl refused the unit"); + assert.equal(result.steps.at(-1).error, "Interactive authentication required"); +}); + +/* -------------------------------------------------- stopping, not removing -*/ + +test("stop disables the unit now and leaves the file alone", async () => { + const home = await scratch(); + const unitPath = join(home, ".config/systemd/user", UNIT_NAME); + await mkdir(dirname(unitPath), { recursive: true }); + await writeFile(unitPath, "[Service]\nExecStart=/usr/bin/node x dns start --port 5354\n"); + + const calls = []; + const result = await stopService({ home, exec: async (cmd, args) => { calls.push(args.join(" ")); return { ok: true }; } }); + + assert.equal(result.stopped, true); + assert.deepEqual(calls, [`--user disable --now ${UNIT_NAME}`], "--now, or the unit stays running until reboot"); + assert.equal(existsSync(unitPath), true, "turning resolution off for an afternoon must not delete the unit"); +}); + +test("stop on a machine with no unit asks systemd nothing", async () => { + const home = await scratch(); + let ran = false; + const result = await stopService({ home, exec: async () => { ran = true; return { ok: true }; } }); + assert.equal(result.stopped, false); + assert.equal(result.reason, "no unit installed"); + assert.equal(ran, false); +}); + +test("a refusal from systemctl comes back with the reason it gave", async () => { + const home = await scratch(); + const unitPath = join(home, ".config/systemd/user", UNIT_NAME); + await mkdir(dirname(unitPath), { recursive: true }); + await writeFile(unitPath, "[Service]\n"); + + const result = await stopService({ home, exec: async () => ({ ok: false, error: "Failed to disable unit" }) }); + assert.equal(result.stopped, false); + assert.equal(result.reason, "Failed to disable unit"); +});