Skip to content

Commit 08afa19

Browse files
ralyodioclaude
andauthored
fix(dns): make dns disable an undo, not a guess at one (#261)
`dns disable` removed /etc/systemd/resolved.conf.d/moshpit.conf, restarted systemd-resolved, and printed "Moshpit TLDs are back to your normal resolver". On a machine where anything else routes to the bridge it changed nothing anyone could observe and reported success anyway. That is not a cosmetic bug: a silently successful no-op is worse than a failure, because nobody re-reads the output of a command that said it worked. It is also not hypothetical — the preflight added in the previous commit found it live on the dev box, where the moshpit-proxy installer writes 00-moshpit.conf with the same routing to a lower-sorting filename. The reason the old implementation could not do better is that it had nothing to work from. It deduced an undo from a template. So `enable` now records the machine instead. The restore point. Before anything moves, `enable` captures every drop-in under resolved.conf.d that steers a query, whole, plus every path its plan is about to write. That breadth is the entire point: the file that keeps a machine routed after a disable is by definition one this code did not write, so a snapshot limited to our own filename could never have caught it. `Domains=` counts as steering alongside `DNS=` — a drop-in setting only `Domains=~.` sends every lookup to the global scope, and restoring half of a routing decision is not restoring it. A path that does not exist is recorded as null, and that null is what later tells `disable` to remove the file rather than leave an empty one. It is versioned, and it carries its own restart command rather than trusting whatever build eventually reads it: a machine enabled by one build gets disabled by whatever is installed months later, and re-deriving the restart there means undoing a run with a command that build invented. It lives at /var/lib/moshcode/dns-restore.json, overridable by MOSHCODE_DNS_MANIFEST. The brief asked for it "next to the backup" and also "not under resolved.conf.d/" — those are the same directory, since the backup is <path>.moshcode-backup. Outside won: the `*.conf` glob is the smaller reason, and the larger one is that a manifest is state about the machine while resolved.conf.d is the one directory guaranteed to be templated, synced and wiped wholesale by other tooling. A manifest that outlives the run it describes is a loaded gun, so a rolled back `enable` deletes its own, and a completed `disable` deletes the one it consumed. `disable`, with a manifest, replays it. Restoring is not the same as removing and the manifest is the only thing that knows which each file needs — the foreign drop-in goes back exactly as it was before `enable` ran, which is the correct undo of this run even though it leaves Moshpit names resolving. That last fact is now stated at enable time rather than discovered later. `disable`, without one — an older build, or a machine set up by hand — falls back to detection: every drop-in pointing at the bridge, named, with our own distinguished by the header we stamp rather than by filename, and a guess at the author for the ones we recognise. Files this repo did not write are never removed silently. Deleting another tool's config may be the only reason a machine resolves anything, so it takes --remove-foreign, and without that flag the command names each file, says who likely wrote it, and exits non-zero — because routing is still in place, and reporting success there is the original bug. `Moshpit TLDs are back to your normal resolver` is now printed only when it is true. Detection finding nothing and no manifest is a clean no-op that returns 0 and skips the restart. Restarting systemd-resolved is a real, if brief, outage; paying it to undo something that was never done is a cost for nothing. Disable now runs through applyWithRollback, like enable. Undoing a switch is a switch: it rewrites resolver config, it restarts the resolver, and restoring a prior state that turns out not to resolve is the same outage in the other direction. That required teaching applyWithRollback to snapshot `remove` targets as well as `write` ones, which is a one-word change and the reason disable can be reversed at all. Verification asks for clearnet only — after a disable Moshpit names are supposed to stop resolving, so requiring one would roll back every successful undo. --dry-run prints the whole thing for both verbs now, including which files would be restored versus removed, and which foreign ones would be left alone. Confirmed against the live box, unprivileged, nothing written: restore point none — falling back to detection found /etc/systemd/resolved.conf.d/00-moshpit.conf → 127.0.0.1:5354 (not written by moshcode — likely the moshpit-proxy installer) ... NOT removed — these route Moshpit names and moshcode did not write them: /etc/systemd/resolved.conf.d/00-moshpit.conf Seventeen more tests, same shape as before: filesystem, exec and manifest injected, with the round-trip one running the real apply loop against a temp directory and asserting the directory ends byte-identical to how it started — including the file moshcode never wrote. 779 tests, 0 fail. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent a20b0ea commit 08afa19

3 files changed

Lines changed: 700 additions & 29 deletions

File tree

0 commit comments

Comments
 (0)