From f402d736aa927f705b7069ec335e81bad702235f Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 08:00:02 +0100 Subject: [PATCH 01/44] Add Merlin 3006 dnsmasq and SDN integration --- internal/router/dnsmasq/dnsmasq.go | 1 + internal/router/merlin/merlin.go | 725 ++++++++++++++++++++++---- internal/router/merlin/merlin_test.go | 217 ++++++++ internal/router/nvram/nvram.go | 98 +++- 4 files changed, 941 insertions(+), 100 deletions(-) diff --git a/internal/router/dnsmasq/dnsmasq.go b/internal/router/dnsmasq/dnsmasq.go index 438a7cb7..f9341e1f 100644 --- a/internal/router/dnsmasq/dnsmasq.go +++ b/internal/router/dnsmasq/dnsmasq.go @@ -32,6 +32,7 @@ const ( MerlinJffsConfDir = "/jffs/configs" MerlinJffsConfPath = "/jffs/configs/dnsmasq.conf" MerlinPostConfPath = "/jffs/scripts/dnsmasq.postconf" + MerlinSdnPostConfPath = "/jffs/scripts/dnsmasq-sdn.postconf" ) const ( diff --git a/internal/router/merlin/merlin.go b/internal/router/merlin/merlin.go index 855cfe4a..fdbe4a76 100644 --- a/internal/router/merlin/merlin.go +++ b/internal/router/merlin/merlin.go @@ -2,8 +2,9 @@ package merlin import ( "bytes" + "crypto/sha256" + "encoding/hex" "fmt" - "io" "os" "os/exec" "path/filepath" @@ -21,6 +22,12 @@ import ( const Name = "merlin" +const ( + merlinManagedStatePath = "/jffs/controld/.merlin-dnsmasq-hooks-v2" + merlinSnapshotStatePath = "/jffs/controld/.merlin-dnsmasq-snapshot" + merlinCleanupPendingPath = "/jffs/controld/.merlin-dnsmasq-cleanup-pending" +) + // nvramKvMap is a map of NVRAM key-value pairs used to configure and manage Merlin-specific settings. var nvramKvMap = map[string]string{ "dnspriv_enable": "0", // Ensure Merlin native DoT disabled. @@ -59,8 +66,23 @@ func (m *Merlin) Uninstall(_ *service.Config) error { // PreRun prepares the Merlin instance for operation by waiting for essential services and directories to become available. func (m *Merlin) PreRun() error { + // Reconcile any previous Merlin integration before starting again. Router + // startup is a one-shot launch, so retry transient JFFS/NVRAM/dnsmasq errors + // here instead of abandoning DNS in a partially reconciled state. + var cleanupErr error + for attempt := 1; attempt <= 3; attempt++ { + cleanupErr = m.Cleanup() + if cleanupErr == nil { + break + } + if attempt < 3 { + time.Sleep(time.Second) + } + } + if cleanupErr != nil { + return fmt.Errorf("failed to cleanup previous Merlin integration after retries: %w", cleanupErr) + } // Wait NTP ready. - _ = m.Cleanup() if err := ntp.WaitNvram(); err != nil { return err } @@ -80,101 +102,577 @@ func (m *Merlin) PreRun() error { } // Setup initializes and configures the Merlin instance for use, including setting up dnsmasq and necessary nvram settings. -func (m *Merlin) Setup() error { +func (m *Merlin) Setup() (retErr error) { if m.cfg.FirstListener().IsDirectDnsListener() { return nil } // Already setup. - if val, _ := nvram.Run("get", nvram.CtrldSetupKey); val == "1" { + setupVal, err := nvram.Run("get", nvram.CtrldSetupKey) + if err != nil { + return fmt.Errorf("read Merlin setup state: %w", err) + } + if setupVal == "1" { return nil } - if err := m.writeDnsmasqPostconf(); err != nil { - return err + // Mark this as the hook-based integration before changing NVRAM so any + // failure after this point is rollback-visible. + if err := atomicWriteFile(merlinManagedStatePath, []byte("hooks-v2\n"), 0600); err != nil { + return fmt.Errorf("failed to write Merlin integration state: %w", err) } - for _, cfg := range getDnsmasqConfigs() { - if err := m.setupDnsmasq(cfg); err != nil { - return fmt.Errorf("failed to setup dnsmasq: config: %s, error: %w", cfg.confPath, err) + defer func() { + if retErr == nil { + return + } + if cleanupErr := m.Cleanup(); cleanupErr != nil { + retErr = fmt.Errorf("%v; rollback failed: %w", retErr, cleanupErr) } + }() + + // Apply NVRAM changes before regenerating dnsmasq so the firmware-generated + // config reflects ctrld's desired DNS Privacy state. nvram.SetKV rolls back + // its own partial mutations on failure. + if err := nvram.SetKV(nvramKvMap, nvram.CtrldSetupKey); err != nil { + return err } - // Restart dnsmasq service. + if err := m.writeDnsmasqPostconf(); err != nil { + return err + } + + // On Merlin 3006 this restarts the main and all SDN dnsmasq instances, + // causing dnsmasq.postconf and dnsmasq-sdn.postconf to run. if err := restartDNSMasq(); err != nil { return err } - if err := nvram.SetKV(nvramKvMap, nvram.CtrldSetupKey); err != nil { + mainOK, err := m.dnsmasqConfigUsesCtrld(dnsmasq.MerlinConfPath) + if err != nil { return err } + if !mainOK { + // Compatibility fallback for older Merlin devices where postconf is + // known not to execute. Never overwrite an unowned user config. + if err := m.setupMainDnsmasqFallback(); err != nil { + return err + } + if err := restartDNSMasq(); err != nil { + return err + } + mainOK, err = m.dnsmasqConfigUsesCtrld(dnsmasq.MerlinConfPath) + if err != nil { + return err + } + if !mainOK { + return fmt.Errorf("Merlin dnsmasq integration was not applied to %s", dnsmasq.MerlinConfPath) + } + } + + // Additional dnsmasq instances are Guest Network Pro / SDN instances on + // Merlin 3006. Their supported extension point is dnsmasq-sdn.postconf; + // full /jffs/configs/dnsmasq-N.conf files are not consumed by Merlin. + for _, path := range dnsmasq.AdditionalConfigFiles() { + ok, err := m.dnsmasqConfigUsesCtrld(path) + if err != nil { + return err + } + if !ok { + return fmt.Errorf("Merlin SDN dnsmasq integration was not applied to %s", path) + } + } return nil } +type legacySnapshotEntry struct { + path string + hash string +} + +type legacyCleanupJournal struct { + phase string + entries []legacySnapshotEntry +} + // Cleanup restores the original dnsmasq and nvram configurations and restarts dnsmasq if necessary. func (m *Merlin) Cleanup() error { + // Preserve the existing direct-listener lifecycle. A direct listener needs + // port 53 itself; restarting dnsmasq here would reclaim that port before + // ctrld binds. Transitioning between forwarding and direct-listener modes + // requires a separate port-ownership design and is outside this change. if m.cfg.FirstListener().IsDirectDnsListener() { return nil } - if val, _ := nvram.Run("get", nvram.CtrldSetupKey); val != "1" { - return nil // was restored, nothing to do. + + setupVal, err := nvram.Run("get", nvram.CtrldSetupKey) + if err != nil { + return fmt.Errorf("read Merlin setup state during cleanup: %w", err) + } + managed, err := pathExists(merlinManagedStatePath) + if err != nil { + return fmt.Errorf("stat Merlin managed state: %w", err) } - // Restore old configs. - if err := nvram.Restore(nvramKvMap, nvram.CtrldSetupKey); err != nil { - return err + journal, err := readLegacyCleanupJournal() + if err != nil { + return fmt.Errorf("read Merlin legacy cleanup state: %w", err) + } + if setupVal != "1" && !managed && journal.phase == "" { + return nil } - if err := cleanupDnsmasqPostconf(dnsmasq.MerlinPostConfPath); err != nil { - return err + legacy := journal.phase != "" || (setupVal == "1" && !managed) + if legacy && journal.phase == "" { + journal, err = buildLegacyCleanupJournal() + if err != nil { + return err + } + if err := writeLegacyCleanupJournal(journal); err != nil { + return fmt.Errorf("journal legacy dnsmasq snapshots: %w", err) + } + } + + for _, path := range []string{dnsmasq.MerlinPostConfPath, dnsmasq.MerlinSdnPostConfPath} { + if err := cleanupDnsmasqPostconf(path); err != nil { + return err + } } - for _, cfg := range getDnsmasqConfigs() { - if err := m.cleanupDnsmasqJffs(cfg); err != nil { - return fmt.Errorf("failed to cleanup jffs dnsmasq: config: %s, error: %w", cfg.confPath, err) + if legacy { + switch journal.phase { + case "cleanup-v1": + // The exact snapshot paths and hashes were durably recorded before + // the first destructive operation. Retries consult only this journal, + // never a fresh directory enumeration, so a user-created replacement + // cannot be mistaken for ctrld's old snapshot. + for _, entry := range journal.entries { + buf, err := os.ReadFile(entry.path) + switch { + case err == nil: + if merlinSnapshotHash(buf) != entry.hash { + continue + } + if err := os.Remove(entry.path); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("remove journaled legacy snapshot %s: %w", entry.path, err) + } + case os.IsNotExist(err): + // Already removed on a previous attempt. + default: + return fmt.Errorf("read journaled legacy snapshot %s: %w", entry.path, err) + } + } + if err := syncParentDir(dnsmasq.MerlinJffsConfDir); err != nil { + return fmt.Errorf("sync legacy snapshot directory: %w", err) + } + journal.phase = "finalize" + journal.entries = nil + if err := writeLegacyCleanupJournal(journal); err != nil { + return fmt.Errorf("advance Merlin legacy cleanup state: %w", err) + } + case "finalize": + // Destructive legacy cleanup already completed. A retry must never + // reinterpret newly created user files as legacy ctrld snapshots. + default: + return fmt.Errorf("unknown Merlin legacy cleanup phase %q", journal.phase) + } + } else { + if err := cleanupOwnedMainSnapshot(); err != nil { + return err } } - // Restart dnsmasq service. + + // Restore NVRAM only after ctrld-owned artifacts are cleaned successfully. + if setupVal == "1" { + if err := nvram.Restore(nvramKvMap, nvram.CtrldSetupKey); err != nil { + return err + } + } + if err := restartDNSMasq(); err != nil { return err } + + if err := removeFileDurable(merlinManagedStatePath); err != nil { + return fmt.Errorf("remove Merlin managed state: %w", err) + } + if journal.phase != "" { + if err := removeFileDurable(merlinCleanupPendingPath); err != nil { + return fmt.Errorf("remove Merlin legacy cleanup state: %w", err) + } + } return nil } -// setupDnsmasq sets up dnsmasq configuration by writing postconf, copying configuration, and running a postconf script. -func (m *Merlin) setupDnsmasq(cfg *dnsmasqConfig) error { - src, err := os.Open(cfg.confPath) +func buildLegacyCleanupJournal() (legacyCleanupJournal, error) { + paths := []string{dnsmasq.MerlinJffsConfPath} + matches, err := filepath.Glob(filepath.Join(dnsmasq.MerlinJffsConfDir, "dnsmasq-*.conf")) + if err != nil { + return legacyCleanupJournal{}, err + } + paths = append(paths, matches...) + + journal := legacyCleanupJournal{phase: "cleanup-v1"} + for _, path := range paths { + if !isLegacySnapshotPath(path) { + continue + } + buf, err := os.ReadFile(path) + if os.IsNotExist(err) { + continue + } + if err != nil { + return legacyCleanupJournal{}, fmt.Errorf("read legacy dnsmasq snapshot %s: %w", path, err) + } + journal.entries = append(journal.entries, legacySnapshotEntry{ + path: path, + hash: merlinSnapshotHash(buf), + }) + } + return journal, nil +} + +func writeLegacyCleanupJournal(journal legacyCleanupJournal) error { + buf, err := encodeLegacyCleanupJournal(journal) + if err != nil { + return err + } + return atomicWriteFile(merlinCleanupPendingPath, buf, 0600) +} + +func encodeLegacyCleanupJournal(journal legacyCleanupJournal) ([]byte, error) { + var b strings.Builder + b.WriteString(journal.phase) + b.WriteByte('\n') + for _, entry := range journal.entries { + if !isLegacySnapshotPath(entry.path) { + return nil, fmt.Errorf("invalid legacy snapshot path %q", entry.path) + } + if _, err := hex.DecodeString(entry.hash); err != nil || len(entry.hash) != sha256.Size*2 { + return nil, fmt.Errorf("invalid legacy snapshot hash for %s", entry.path) + } + b.WriteString(entry.path) + b.WriteByte('\t') + b.WriteString(entry.hash) + b.WriteByte('\n') + } + return []byte(b.String()), nil +} + +func readLegacyCleanupJournal() (legacyCleanupJournal, error) { + buf, err := os.ReadFile(merlinCleanupPendingPath) + if os.IsNotExist(err) { + return legacyCleanupJournal{}, nil + } + if err != nil { + return legacyCleanupJournal{}, err + } + return parseLegacyCleanupJournal(buf) +} + +func parseLegacyCleanupJournal(buf []byte) (legacyCleanupJournal, error) { + lines := strings.Split(strings.TrimSpace(string(buf)), "\n") + if len(lines) == 0 || lines[0] == "" { + return legacyCleanupJournal{}, fmt.Errorf("empty legacy cleanup journal") + } + journal := legacyCleanupJournal{phase: lines[0]} + if journal.phase == "finalize" { + if len(lines) != 1 { + return legacyCleanupJournal{}, fmt.Errorf("finalize journal contains snapshot entries") + } + return journal, nil + } + if journal.phase != "cleanup-v1" { + return legacyCleanupJournal{}, fmt.Errorf("unknown legacy cleanup phase %q", journal.phase) + } + for _, line := range lines[1:] { + if line == "" { + continue + } + parts := strings.SplitN(line, "\t", 2) + if len(parts) != 2 || !isLegacySnapshotPath(parts[0]) { + return legacyCleanupJournal{}, fmt.Errorf("invalid legacy cleanup journal entry %q", line) + } + if len(parts[1]) != sha256.Size*2 { + return legacyCleanupJournal{}, fmt.Errorf("invalid legacy snapshot hash for %s", parts[0]) + } + if _, err := hex.DecodeString(parts[1]); err != nil { + return legacyCleanupJournal{}, fmt.Errorf("invalid legacy snapshot hash for %s: %w", parts[0], err) + } + journal.entries = append(journal.entries, legacySnapshotEntry{path: parts[0], hash: parts[1]}) + } + return journal, nil +} + +func isLegacySnapshotPath(path string) bool { + if path == dnsmasq.MerlinJffsConfPath { + return true + } + if filepath.Dir(path) != dnsmasq.MerlinJffsConfDir { + return false + } + base := filepath.Base(path) + if !strings.HasPrefix(base, "dnsmasq-") || !strings.HasSuffix(base, ".conf") { + return false + } + middle := strings.TrimSuffix(strings.TrimPrefix(base, "dnsmasq-"), ".conf") + if middle == "" { + return false + } + for _, r := range middle { + if r < '0' || r > '9' { + return false + } + } + return true +} + +func readMerlinState(path string) (string, error) { + buf, err := os.ReadFile(path) if os.IsNotExist(err) { - return nil // nothing to do if conf file does not exist. + return "", nil } if err != nil { - return fmt.Errorf("failed to open dnsmasq config: %w", err) + return "", err } - defer src.Close() + return strings.TrimSpace(string(buf)), nil +} - // Copy current dnsmasq config to cfg.jffsConfPath, - // Then we will run postconf script on this file. - // - // Normally, adding postconf script is enough. However, we see - // reports on some Merlin devices that postconf scripts does not - // work, but manipulating the config directly via /jffs/configs does. - dst, err := os.Create(cfg.jffsConfPath) +// dnsmasqConfigUsesCtrld reports whether a generated dnsmasq config has the +// complete ctrld forwarding shape, not merely one matching server line. +func (m *Merlin) dnsmasqConfigUsesCtrld(path string) (bool, error) { + buf, err := os.ReadFile(path) + if os.IsNotExist(err) { + return false, nil + } if err != nil { - return fmt.Errorf("failed to create %s: %w", cfg.jffsConfPath, err) + return false, err } - defer dst.Close() - if _, err := io.Copy(dst, src); err != nil { - return fmt.Errorf("failed to copy current dnsmasq config: %w", err) + listener := m.cfg.FirstListener() + if listener == nil { + return false, fmt.Errorf("missing ctrld listener") } - if err := dst.Close(); err != nil { - return fmt.Errorf("failed to save %s: %w", cfg.jffsConfPath, err) + ip := listener.IP + if ip == "" || ip == "0.0.0.0" || ip == "::" { + ip = "127.0.0.1" } + expectedServer := fmt.Sprintf("server=%s#%d", ip, listener.Port) - // Run postconf script on cfg.jffsConfPath directly. - cmd := exec.Command("/bin/sh", dnsmasq.MerlinPostConfPath, cfg.jffsConfPath) + var ( + serverCount int + expectedSeen bool + noResolv bool + addMAC bool + addSubnet bool + cacheOff bool + ) + for _, raw := range strings.Split(string(buf), "\n") { + line := strings.TrimSpace(raw) + if line == "" || strings.HasPrefix(line, "#") { + continue + } + switch { + case strings.HasPrefix(line, "server="): + serverCount++ + if line == expectedServer { + expectedSeen = true + } + case strings.HasPrefix(line, "servers-file="), + strings.HasPrefix(line, "resolv-file="), + line == "dnssec", + strings.HasPrefix(line, "trust-anchor="): + return false, nil + case line == "no-resolv": + noResolv = true + case line == "add-mac": + addMAC = true + case line == "add-subnet=32,128": + addSubnet = true + case line == "cache-size=0": + cacheOff = true + } + } + + return serverCount == 1 && expectedSeen && noResolv && addMAC && addSubnet && cacheOff, nil +} + +// setupMainDnsmasqFallback retains the old full-config mechanism only for the +// main dnsmasq when the supported postconf hook demonstrably did not apply. +// Ownership is published before the final snapshot, and content hashes prevent +// ctrld from later deleting or overwriting a user-modified file. +func (m *Merlin) setupMainDnsmasqFallback() error { + owned, err := mainSnapshotOwnership() + if err != nil { + return err + } + snapshotExists, err := pathExists(dnsmasq.MerlinJffsConfPath) + if err != nil { + return fmt.Errorf("stat Merlin fallback config: %w", err) + } + if snapshotExists { + if !owned { + return fmt.Errorf("refusing to overwrite unowned or modified Merlin custom config: %s", dnsmasq.MerlinJffsConfPath) + } + if err := cleanupOwnedMainSnapshot(); err != nil { + return err + } + } else { + stateExists, err := pathExists(merlinSnapshotStatePath) + if err != nil { + return fmt.Errorf("stat Merlin fallback ownership: %w", err) + } + if stateExists { + // A previous crash may have published ownership before the snapshot. + if err := removeFileDurable(merlinSnapshotStatePath); err != nil { + return fmt.Errorf("remove orphaned Merlin snapshot state: %w", err) + } + } + } + + buf, err := os.ReadFile(dnsmasq.MerlinConfPath) + if err != nil { + return fmt.Errorf("failed to read dnsmasq config for fallback: %w", err) + } + built, err := m.buildMainDnsmasqFallback(buf) + if err != nil { + return err + } + hash := merlinSnapshotHash(built) + + // Publish durable ownership first. If the router stops before the snapshot + // rename, cleanup sees a harmless orphaned marker and removes it. + if err := atomicWriteFile(merlinSnapshotStatePath, []byte("sha256="+hash+"\n"), 0600); err != nil { + return fmt.Errorf("mark ctrld dnsmasq fallback ownership: %w", err) + } + if err := atomicWriteFile(dnsmasq.MerlinJffsConfPath, built, 0644); err != nil { + current, readErr := os.ReadFile(dnsmasq.MerlinJffsConfPath) + switch { + case readErr == nil && merlinSnapshotHash(current) == hash: + // Rename reached the target. Keep the already-durable ownership marker + // so deferred/retry cleanup can safely remove the visible fallback. + return fmt.Errorf("publish ctrld dnsmasq fallback after rename: %w", err) + case os.IsNotExist(readErr): + if removeErr := removeFileDurable(merlinSnapshotStatePath); removeErr != nil { + return fmt.Errorf("publish ctrld dnsmasq fallback: %w; remove ownership state: %v", err, removeErr) + } + return fmt.Errorf("publish ctrld dnsmasq fallback: %w", err) + case readErr == nil: + // A different readable file is present, so ctrld cannot claim it. + if removeErr := removeFileDurable(merlinSnapshotStatePath); removeErr != nil { + return fmt.Errorf("publish ctrld dnsmasq fallback: %w; unexpected target and ownership cleanup failed: %v", err, removeErr) + } + return fmt.Errorf("publish ctrld dnsmasq fallback: %w; target content does not match ctrld snapshot", err) + default: + // Publication state is uncertain. Retain the marker rather than turn + // a possibly successful rename into a permanently unowned snapshot. + return fmt.Errorf("publish ctrld dnsmasq fallback: %w; verify target: %v", err, readErr) + } + } + return nil +} + +func (m *Merlin) buildMainDnsmasqFallback(buf []byte) ([]byte, error) { + tmp, err := os.CreateTemp(dnsmasq.MerlinJffsConfDir, ".dnsmasq.conf.ctrld-build-*") + if err != nil { + return nil, fmt.Errorf("create ctrld dnsmasq fallback build file: %w", err) + } + path := tmp.Name() + defer os.Remove(path) + + if err := tmp.Chmod(0644); err != nil { + _ = tmp.Close() + return nil, err + } + if _, err := tmp.Write(buf); err != nil { + _ = tmp.Close() + return nil, err + } + if err := tmp.Close(); err != nil { + return nil, err + } + + script, err := dnsmasq.ConfTmpl(dnsmasq.MerlinPostConfTmpl, m.cfg) + if err != nil { + return nil, fmt.Errorf("render ctrld fallback postconf: %w", err) + } + // Apply only ctrld's managed logic. Executing the shared Merlin hook here + // would also execute unrelated addon/user blocks a second time against a + // temporary file. + cmd := exec.Command("/bin/sh", "-c", script, "ctrld-fallback", path) if out, err := cmd.CombinedOutput(); err != nil { - return fmt.Errorf("failed to run post conf: %s: %w", string(out), err) + return nil, fmt.Errorf("failed to apply ctrld fallback postconf: %s: %w", string(out), err) + } + built, err := os.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("read ctrld dnsmasq fallback build: %w", err) + } + return built, nil +} + +func merlinSnapshotHash(buf []byte) string { + sum := sha256.Sum256(buf) + return hex.EncodeToString(sum[:]) +} + +func readMainSnapshotHash() (string, bool, error) { + state, err := readMerlinState(merlinSnapshotStatePath) + if err != nil { + return "", false, err + } + if state == "" { + return "", false, nil + } + const prefix = "sha256=" + if !strings.HasPrefix(state, prefix) || len(state) != len(prefix)+sha256.Size*2 { + return "", false, fmt.Errorf("invalid Merlin dnsmasq snapshot ownership state") + } + hash := strings.TrimPrefix(state, prefix) + if _, err := hex.DecodeString(hash); err != nil { + return "", false, fmt.Errorf("invalid Merlin dnsmasq snapshot hash: %w", err) + } + return hash, true, nil +} + +func mainSnapshotOwnership() (bool, error) { + expected, marked, err := readMainSnapshotHash() + if err != nil || !marked { + return false, err + } + buf, err := os.ReadFile(dnsmasq.MerlinJffsConfPath) + if os.IsNotExist(err) { + return false, nil + } + if err != nil { + return false, err + } + return merlinSnapshotHash(buf) == expected, nil +} + +func cleanupOwnedMainSnapshot() error { + expected, marked, err := readMainSnapshotHash() + if err != nil { + return err + } + if !marked { + return nil + } + + buf, err := os.ReadFile(dnsmasq.MerlinJffsConfPath) + if os.IsNotExist(err) { + return removeFileDurable(merlinSnapshotStatePath) + } + if err != nil { + return err + } + if merlinSnapshotHash(buf) != expected { + return fmt.Errorf("refusing to remove modified Merlin dnsmasq fallback: %s", dnsmasq.MerlinJffsConfPath) + } + if err := removeFileDurable(dnsmasq.MerlinJffsConfPath); err != nil { + return err + } + if err := removeFileDurable(merlinSnapshotStatePath); err != nil { + return err } return nil } @@ -188,6 +686,8 @@ func (m *Merlin) cleanupDnsmasqJffs(cfg *dnsmasqConfig) error { return nil } +// merlinHookUpdate is prepared entirely before any shared hook is modified. +// This lets us validate/read both Merlin hook paths before the first write. type merlinHookUpdate struct { path string data []byte @@ -197,9 +697,10 @@ type merlinHookUpdate struct { symlinkTarget string } -// writeDnsmasqPostconf manages only ctrld's marked block in the shared main -// dnsmasq.postconf hook. Merlin 3006 SDN support is intentionally handled in a -// separate change so this patch does not alter router lifecycle semantics. +// writeDnsmasqPostconf installs ctrld-owned blocks in Merlin's main and SDN +// hooks while preserving unrelated content. Both paths are preflighted before +// either is modified, avoiding a half-installed integration when the second +// shared hook is unreadable or otherwise invalid. func (m *Merlin) writeDnsmasqPostconf() error { data, err := dnsmasq.ConfTmpl(dnsmasq.MerlinPostConfTmpl, m.cfg) if err != nil { @@ -211,43 +712,73 @@ func (m *Merlin) writeDnsmasqPostconf() error { dnsmasq.MerlinPostConfEndMarker, }, "\n")) - update, err := prepareMerlinHookUpdate(dnsmasq.MerlinPostConfPath, block) - if err != nil { - return err - } - if err := revalidateMerlinHookUpdate(update); err != nil { - return fmt.Errorf("revalidate Merlin hook %s: %w", update.path, err) - } - if err := atomicWriteFile(update.path, update.data, 0750); err != nil { - return fmt.Errorf("write Merlin hook %s: %w", update.path, err) - } - return nil + return writeMerlinHookUpdates( + []string{dnsmasq.MerlinPostConfPath, dnsmasq.MerlinSdnPostConfPath}, + block, + ) } -func prepareMerlinHookUpdate(path string, block []byte) (merlinHookUpdate, error) { - return prepareMerlinHookReplacement(path, func(buf []byte) []byte { - return merlinUpsertPostConf(buf, block) - }) +func writeMerlinHookUpdates(paths []string, block []byte) error { + return writeMerlinHookUpdatesWith(paths, block, atomicWriteFile) } -func prepareMerlinHookCleanup(path string) (*merlinHookUpdate, error) { - info, err := os.Lstat(path) - if os.IsNotExist(err) { - return nil, nil +func writeMerlinHookUpdatesWith( + paths []string, + block []byte, + writeFile func(string, []byte, os.FileMode) error, +) error { + updates := make([]merlinHookUpdate, 0, len(paths)) + for _, path := range paths { + update, err := prepareMerlinHookUpdate(path, block) + if err != nil { + return err + } + updates = append(updates, update) } - if err != nil { - return nil, err + + written := make([]merlinHookUpdate, 0, len(updates)) + for _, update := range updates { + if err := revalidateMerlinHookUpdate(update); err != nil { + if rollbackErr := rollbackMerlinHookUpdates(written); rollbackErr != nil { + return fmt.Errorf("revalidate Merlin hook %s: %w; rollback failed: %v", update.path, err, rollbackErr) + } + return fmt.Errorf("revalidate Merlin hook %s: %w", update.path, err) + } + if err := writeFile(update.path, update.data, 0750); err != nil { + if rollbackErr := rollbackMerlinHookUpdates(written); rollbackErr != nil { + return fmt.Errorf("write Merlin hook %s: %w; rollback failed: %v", update.path, err, rollbackErr) + } + return fmt.Errorf("write Merlin hook %s: %w", update.path, err) + } + written = append(written, update) } - _ = info + return nil +} - update, err := prepareMerlinHookReplacement(path, merlinParsePostConf) - if err != nil { - return nil, err +func rollbackMerlinHookUpdates(updates []merlinHookUpdate) error { + for i := len(updates) - 1; i >= 0; i-- { + update := updates[i] + current, err := os.ReadFile(update.path) + if err != nil { + return fmt.Errorf("read %s during rollback: %w", update.path, err) + } + if !bytes.Equal(current, update.data) { + return fmt.Errorf("refusing to roll back %s after external modification", update.path) + } + if update.existed { + if err := atomicWriteFile(update.path, update.original, 0750); err != nil { + return fmt.Errorf("restore %s: %w", update.path, err) + } + continue + } + if err := os.Remove(update.path); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("remove newly created %s: %w", update.path, err) + } } - return &update, nil + return nil } -func prepareMerlinHookReplacement(path string, transform func([]byte) []byte) (merlinHookUpdate, error) { +func prepareMerlinHookUpdate(path string, block []byte) (merlinHookUpdate, error) { info, statErr := os.Lstat(path) pathMissing := os.IsNotExist(statErr) if statErr != nil && !pathMissing { @@ -264,7 +795,7 @@ func prepareMerlinHookReplacement(path string, transform func([]byte) []byte) (m update := merlinHookUpdate{ path: path, - data: transform(buf), + data: merlinUpsertPostConf(buf, block), original: append([]byte(nil), buf...), existed: !pathMissing, } @@ -321,19 +852,19 @@ func revalidateMerlinHookUpdate(update merlinHookUpdate) error { } func cleanupDnsmasqPostconf(path string) error { - update, err := prepareMerlinHookCleanup(path) - if err != nil || update == nil { - return err - } - if bytes.Equal(update.original, update.data) { + buf, err := os.ReadFile(path) + if os.IsNotExist(err) { return nil } - if err := revalidateMerlinHookUpdate(*update); err != nil { - return fmt.Errorf("revalidate Merlin hook cleanup %s: %w", path, err) + if err != nil { + return err } - // Never delete a shared Merlin hook outright. Cleanup removes only ctrld's - // owned bytes and preserves the path, content and mode belonging to others. - return atomicWriteFile(path, update.data, 0750) + + clean := merlinParsePostConf(buf) + // Never delete a shared Merlin hook outright. We cannot safely prove + // persistent ownership across addon rewrites/reboots, so cleanup removes + // only ctrld-owned bytes and leaves any resulting stub in place. + return atomicWriteFile(path, clean, 0750) } // restartDNSMasq restarts the dnsmasq service by executing the appropriate system command using "service". @@ -665,8 +1196,6 @@ func syncParentDir(dir string) error { } defer f.Close() if err := f.Sync(); err != nil { - // Directory fsync is not supported by the Windows CI filesystem. Merlin - // itself is Unix-only, where this is required for rename durability. if runtime.GOOS == "windows" { return nil } @@ -675,6 +1204,28 @@ func syncParentDir(dir string) error { return nil } +func pathExists(path string) (bool, error) { + _, err := os.Stat(path) + if err == nil { + return true, nil + } + if os.IsNotExist(err) { + return false, nil + } + return false, err +} + +func removeFileDurable(path string) error { + err := os.Remove(path) + if os.IsNotExist(err) { + return nil + } + if err != nil { + return err + } + return syncParentDir(filepath.Dir(path)) +} + // waitDirExists waits until the specified directory exists, polling its existence every second. func waitDirExists(dir string) { for { diff --git a/internal/router/merlin/merlin_test.go b/internal/router/merlin/merlin_test.go index ddcabf1c..0c7cf6d5 100644 --- a/internal/router/merlin/merlin_test.go +++ b/internal/router/merlin/merlin_test.go @@ -96,6 +96,7 @@ func Test_merlinPostConfDoesNotExitHostHook(t *testing.T) { } } + func Test_merlinLegacyMigrationPreservesPrependedContent(t *testing.T) { legacy := "echo addon-before\n" + legacyMerlinPostConf("echo addon-after") block := []byte("# BEGIN ctrld\necho ctrld-new\n# END ctrld") @@ -287,6 +288,65 @@ func Test_merlinLegacyEmptyHookCanBeReinstalled(t *testing.T) { } } +func Test_writeMerlinHookUpdatesPreflightsAllPaths(t *testing.T) { + dir := t.TempDir() + first := filepath.Join(dir, "dnsmasq.postconf") + second := filepath.Join(dir, "dnsmasq-sdn.postconf") + orig := []byte("#!/bin/sh\necho untouched\n") + if err := os.WriteFile(first, orig, 0750); err != nil { + t.Fatal(err) + } + if err := os.Mkdir(second, 0755); err != nil { + t.Fatal(err) + } + + block := []byte("# BEGIN ctrld\necho managed\n# END ctrld") + if err := writeMerlinHookUpdates([]string{first, second}, block); err == nil { + t.Fatal("expected second-path preflight error") + } + got, err := os.ReadFile(first) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(got, orig) { + t.Fatalf("first hook changed before second path passed preflight:\nwant: %q\ngot: %q", orig, got) + } +} + +func Test_writeMerlinHookUpdatesRollsBackPartialWrites(t *testing.T) { + dir := t.TempDir() + first := filepath.Join(dir, "dnsmasq.postconf") + second := filepath.Join(dir, "dnsmasq-sdn.postconf") + orig := []byte("#!/bin/sh\necho original\n") + if err := os.WriteFile(first, orig, 0750); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(second, []byte("#!/bin/sh\necho second\n"), 0750); err != nil { + t.Fatal(err) + } + + writes := 0 + writeFile := func(path string, data []byte, mode os.FileMode) error { + writes++ + if writes == 2 { + return os.ErrPermission + } + return atomicWriteFile(path, data, mode) + } + + block := []byte("# BEGIN ctrld\necho managed\n# END ctrld") + if err := writeMerlinHookUpdatesWith([]string{first, second}, block, writeFile); err == nil { + t.Fatal("expected second write failure") + } + got, err := os.ReadFile(first) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(got, orig) { + t.Fatalf("first hook was not rolled back:\nwant: %q\ngot: %q", orig, got) + } +} + func Test_revalidateMerlinHookUpdateDetectsContentChange(t *testing.T) { path := filepath.Join(t.TempDir(), "dnsmasq.postconf") orig := []byte("#!/bin/sh\necho original\n") @@ -363,6 +423,162 @@ func Test_atomicWriteFilePreservesExistingMode(t *testing.T) { } } +func Test_dnsmasqConfigUsesCtrld(t *testing.T) { + good := func(server string) string { + return strings.Join([]string{ + "no-resolv", + server, + "add-mac", + "add-subnet=32,128", + "cache-size=0", + "", + }, "\n") + } + + tests := []struct { + name string + ip string + port int + content string + want bool + }{ + { + name: "wildcard listener maps to loopback", + ip: "0.0.0.0", + port: 5354, + content: good("server=127.0.0.1#5354"), + want: true, + }, + { + name: "explicit listener", + ip: "127.0.0.2", + port: 5355, + content: good("server=127.0.0.2#5355"), + want: true, + }, + { + name: "wrong upstream", + ip: "0.0.0.0", + port: 5354, + content: good("server=1.1.1.1#53"), + want: false, + }, + { + name: "additional server bypass", + ip: "0.0.0.0", + port: 5354, + content: good("server=127.0.0.1#5354") + + "server=1.1.1.1#53\n", + want: false, + }, + { + name: "servers file bypass", + ip: "0.0.0.0", + port: 5354, + content: good("server=127.0.0.1#5354") + + "servers-file=/tmp/resolv.dnsmasq\n", + want: false, + }, + { + name: "resolv file bypass", + ip: "0.0.0.0", + port: 5354, + content: good("server=127.0.0.1#5354") + + "resolv-file=/tmp/resolv.conf\n", + want: false, + }, + { + name: "missing metadata directives", + ip: "0.0.0.0", + port: 5354, + content: "no-resolv\nserver=127.0.0.1#5354\ncache-size=0\n", + want: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "dnsmasq.conf") + if err := os.WriteFile(path, []byte(tc.content), 0600); err != nil { + t.Fatal(err) + } + m := New(&ctrld.Config{Listener: map[string]*ctrld.ListenerConfig{ + "0": {IP: tc.ip, Port: tc.port}, + }}) + got, err := m.dnsmasqConfigUsesCtrld(path) + if err != nil { + t.Fatal(err) + } + if got != tc.want { + t.Fatalf("dnsmasqConfigUsesCtrld() = %v, want %v", got, tc.want) + } + }) + } +} + +func Test_legacyCleanupJournalRoundTrip(t *testing.T) { + entries := []legacySnapshotEntry{ + {path: dnsmasq.MerlinJffsConfPath, hash: merlinSnapshotHash([]byte("main"))}, + {path: filepath.Join(dnsmasq.MerlinJffsConfDir, "dnsmasq-1.conf"), hash: merlinSnapshotHash([]byte("sdn1"))}, + {path: filepath.Join(dnsmasq.MerlinJffsConfDir, "dnsmasq-3.conf"), hash: merlinSnapshotHash([]byte("sdn3"))}, + } + want := legacyCleanupJournal{phase: "cleanup-v1", entries: entries} + buf, err := encodeLegacyCleanupJournal(want) + if err != nil { + t.Fatal(err) + } + got, err := parseLegacyCleanupJournal(buf) + if err != nil { + t.Fatal(err) + } + if got.phase != want.phase || len(got.entries) != len(want.entries) { + t.Fatalf("journal mismatch: want %#v, got %#v", want, got) + } + for i := range want.entries { + if got.entries[i] != want.entries[i] { + t.Fatalf("entry %d mismatch: want %#v, got %#v", i, want.entries[i], got.entries[i]) + } + } +} + +func Test_parseLegacyCleanupJournalRejectsUnsafePath(t *testing.T) { + hash := merlinSnapshotHash([]byte("x")) + buf := []byte("cleanup-v1\n/jffs/configs/profile.add\t" + hash + "\n") + if _, err := parseLegacyCleanupJournal(buf); err == nil { + t.Fatal("expected unsafe legacy snapshot path to be rejected") + } +} + +func Test_parseLegacyCleanupJournalFinalizeHasNoEntries(t *testing.T) { + if _, err := parseLegacyCleanupJournal([]byte("finalize\n")); err != nil { + t.Fatalf("valid finalize journal rejected: %v", err) + } + hash := merlinSnapshotHash([]byte("x")) + buf := []byte("finalize\n" + dnsmasq.MerlinJffsConfPath + "\t" + hash + "\n") + if _, err := parseLegacyCleanupJournal(buf); err == nil { + t.Fatal("expected finalize journal with entries to be rejected") + } +} + +func Test_isLegacySnapshotPath(t *testing.T) { + tests := []struct { + path string + want bool + }{ + {dnsmasq.MerlinJffsConfPath, true}, + {filepath.Join(dnsmasq.MerlinJffsConfDir, "dnsmasq-1.conf"), true}, + {filepath.Join(dnsmasq.MerlinJffsConfDir, "dnsmasq-123.conf"), true}, + {filepath.Join(dnsmasq.MerlinJffsConfDir, "dnsmasq.conf.add"), false}, + {filepath.Join(dnsmasq.MerlinJffsConfDir, "dnsmasq-sdn.conf"), false}, + {"/tmp/dnsmasq-1.conf", false}, + } + for _, tc := range tests { + if got := isLegacySnapshotPath(tc.path); got != tc.want { + t.Fatalf("isLegacySnapshotPath(%q) = %v, want %v", tc.path, got, tc.want) + } + } +} + func Test_cleanupPreparationRevalidatesBeforeWrite(t *testing.T) { path := filepath.Join(t.TempDir(), "dnsmasq.postconf") managed := []byte("#!/bin/sh\n# BEGIN ctrld\necho managed\n# END ctrld\necho addon\n") @@ -383,3 +599,4 @@ func Test_cleanupPreparationRevalidatesBeforeWrite(t *testing.T) { t.Fatal("expected cleanup revalidation failure after external modification") } } + diff --git a/internal/router/nvram/nvram.go b/internal/router/nvram/nvram.go index e76c0171..59e2717d 100644 --- a/internal/router/nvram/nvram.go +++ b/internal/router/nvram/nvram.go @@ -2,6 +2,7 @@ package nvram import ( "bytes" + "errors" "fmt" "os/exec" "strings" @@ -36,43 +37,111 @@ NOTE: */ // SetKV writes the given key/value from map to nvram. -// The given setupKey is set to 1 to indicates key/value set. +// The given setupKey is set to 1 to indicate key/value set. +// +// NVRAM mutations are not transactional. Keep enough rollback information in +// volatile NVRAM and restore it on any failure so callers never have to infer +// whether a partially failed sequence changed router state. func SetKV(m map[string]string, setupKey string) error { - // Backup current value, store ctrld's configs. + modified := make([]string, 0, len(m)) + + rollback := func(cause error) error { + var restoreErr error + for i := len(modified) - 1; i >= 0; i-- { + key := modified[i] + ctrldKey := CtrldKeyPrefix + key + old, err := Run("get", ctrldKey) + if err != nil { + restoreErr = errors.Join(restoreErr, fmt.Errorf("read rollback %s: %w", ctrldKey, err)) + continue + } + if out, err := Run("set", key+"="+old); err != nil { + restoreErr = errors.Join(restoreErr, fmt.Errorf("%s: %w", out, err)) + } + } + + if restoreErr != nil { + // At least one ctrld-modified value may still be active. Keep an + // explicit setup marker so Merlin Cleanup/Restore will retry from the + // retained ctrld_* backup values instead of treating the state as clean. + var markerErr error + if out, err := Run("set", setupKey+"=1"); err != nil { + markerErr = errors.Join(markerErr, fmt.Errorf("%s: %w", out, err)) + } + if out, err := Run("commit"); err != nil { + markerErr = errors.Join(markerErr, fmt.Errorf("%s: %w", out, err)) + } + return errors.Join( + cause, + fmt.Errorf("nvram rollback incomplete: %w", restoreErr), + markerErr, + ) + } + + if out, err := Run("unset", setupKey); err != nil { + return errors.Join(cause, fmt.Errorf("%s: %w", out, err)) + } + if _, err := Run("commit"); err != nil { + // The restored values may only be volatile. Re-arm the marker before + // returning so the caller's deferred Cleanup has a retryable state. + var markerErr error + if out, setErr := Run("set", setupKey+"=1"); setErr != nil { + markerErr = errors.Join(markerErr, fmt.Errorf("%s: %w", out, setErr)) + } + if out, commitErr := Run("commit"); commitErr != nil { + markerErr = errors.Join(markerErr, fmt.Errorf("%s: %w", out, commitErr)) + } + return errors.Join( + cause, + fmt.Errorf("nvram rollback commit failed: %w", err), + markerErr, + ) + } + return cause + } + + fail := func(err error) error { + if len(modified) == 0 { + return err + } + return rollback(err) + } + for key, value := range m { old, err := Run("get", key) if err != nil { - return fmt.Errorf("%s: %w", old, err) + return fail(fmt.Errorf("%s: %w", old, err)) } if out, err := Run("set", CtrldKeyPrefix+key+"="+old); err != nil { - return fmt.Errorf("%s: %w", out, err) + return fail(fmt.Errorf("%s: %w", out, err)) } + modified = append(modified, key) if out, err := Run("set", key+"="+value); err != nil { - return fmt.Errorf("%s: %w", out, err) + return rollback(fmt.Errorf("%s: %w", out, err)) } } if out, err := Run("set", setupKey+"=1"); err != nil { - return fmt.Errorf("%s: %w", out, err) + return rollback(fmt.Errorf("%s: %w", out, err)) } - // Commit. if out, err := Run("commit"); err != nil { - return fmt.Errorf("%s: %w", out, err) + return rollback(fmt.Errorf("%s: %w", out, err)) } return nil } -// Restore restores the old value of given key from map m. -// The given setupKey is set to 0 to indicates key/value restored. +// Restore restores the old value of each key from ctrld's backup NVRAM. +// Backup keys are deliberately retained after the restore commit. They are tiny, +// harmless, and keeping them avoids a second non-transactional "cleanup commit" +// that could destroy the only rollback copy after a transient nvram failure. +// A future SetKV overwrites each backup with the then-current value. func Restore(m map[string]string, setupKey string) error { - // Restore old configs. for key := range m { ctrldKey := CtrldKeyPrefix + key old, err := Run("get", ctrldKey) if err != nil { return fmt.Errorf("%s: %w", old, err) } - _, _ = Run("unset", ctrldKey) if out, err := Run("set", key+"="+old); err != nil { return fmt.Errorf("%s: %w", out, err) } @@ -81,9 +150,12 @@ func Restore(m map[string]string, setupKey string) error { if out, err := Run("unset", setupKey); err != nil { return fmt.Errorf("%s: %w", out, err) } - // Commit. if out, err := Run("commit"); err != nil { + // Do not persist setupKey=1 after the original values have been restored: + // that marker means "ctrld is configured" to other router backends. If + // this commit fails, return the error and let the caller retry/reconcile. return fmt.Errorf("%s: %w", out, err) + } return nil } From d4058d75116fbfaef3bdbabd359d8a606d3bb5d0 Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 08:03:54 +0100 Subject: [PATCH 02/44] Revalidate shared Merlin hooks during cleanup --- internal/router/merlin/merlin.go | 43 ++++++++++++++++++++++++-------- 1 file changed, 32 insertions(+), 11 deletions(-) diff --git a/internal/router/merlin/merlin.go b/internal/router/merlin/merlin.go index fdbe4a76..1ce3fe29 100644 --- a/internal/router/merlin/merlin.go +++ b/internal/router/merlin/merlin.go @@ -779,6 +779,27 @@ func rollbackMerlinHookUpdates(updates []merlinHookUpdate) error { } func prepareMerlinHookUpdate(path string, block []byte) (merlinHookUpdate, error) { + return prepareMerlinHookReplacement(path, func(buf []byte) []byte { + return merlinUpsertPostConf(buf, block) + }) +} + +func prepareMerlinHookCleanup(path string) (*merlinHookUpdate, error) { + _, err := os.Lstat(path) + if os.IsNotExist(err) { + return nil, nil + } + if err != nil { + return nil, err + } + update, err := prepareMerlinHookReplacement(path, merlinParsePostConf) + if err != nil { + return nil, err + } + return &update, nil +} + +func prepareMerlinHookReplacement(path string, transform func([]byte) []byte) (merlinHookUpdate, error) { info, statErr := os.Lstat(path) pathMissing := os.IsNotExist(statErr) if statErr != nil && !pathMissing { @@ -795,7 +816,7 @@ func prepareMerlinHookUpdate(path string, block []byte) (merlinHookUpdate, error update := merlinHookUpdate{ path: path, - data: merlinUpsertPostConf(buf, block), + data: transform(buf), original: append([]byte(nil), buf...), existed: !pathMissing, } @@ -852,19 +873,19 @@ func revalidateMerlinHookUpdate(update merlinHookUpdate) error { } func cleanupDnsmasqPostconf(path string) error { - buf, err := os.ReadFile(path) - if os.IsNotExist(err) { + update, err := prepareMerlinHookCleanup(path) + if err != nil || update == nil { + return err + } + if bytes.Equal(update.original, update.data) { return nil } - if err != nil { - return err + if err := revalidateMerlinHookUpdate(*update); err != nil { + return fmt.Errorf("revalidate Merlin hook cleanup %s: %w", path, err) } - - clean := merlinParsePostConf(buf) - // Never delete a shared Merlin hook outright. We cannot safely prove - // persistent ownership across addon rewrites/reboots, so cleanup removes - // only ctrld-owned bytes and leaves any resulting stub in place. - return atomicWriteFile(path, clean, 0750) + // Never delete a shared Merlin hook outright. Cleanup removes only ctrld's + // owned bytes and preserves the path, content and mode belonging to others. + return atomicWriteFile(path, update.data, 0750) } // restartDNSMasq restarts the dnsmasq service by executing the appropriate system command using "service". From 264088b5bbff0920d98d7a27cdfd99bc29e30db1 Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 08:05:37 +0100 Subject: [PATCH 03/44] Revalidate shared hooks before rollback --- internal/router/merlin/merlin.go | 37 ++++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/internal/router/merlin/merlin.go b/internal/router/merlin/merlin.go index 1ce3fe29..7bf249ae 100644 --- a/internal/router/merlin/merlin.go +++ b/internal/router/merlin/merlin.go @@ -758,12 +758,8 @@ func writeMerlinHookUpdatesWith( func rollbackMerlinHookUpdates(updates []merlinHookUpdate) error { for i := len(updates) - 1; i >= 0; i-- { update := updates[i] - current, err := os.ReadFile(update.path) - if err != nil { - return fmt.Errorf("read %s during rollback: %w", update.path, err) - } - if !bytes.Equal(current, update.data) { - return fmt.Errorf("refusing to roll back %s after external modification", update.path) + if err := revalidateMerlinHookRollback(update); err != nil { + return fmt.Errorf("refusing to roll back %s: %w", update.path, err) } if update.existed { if err := atomicWriteFile(update.path, update.original, 0750); err != nil { @@ -771,13 +767,40 @@ func rollbackMerlinHookUpdates(updates []merlinHookUpdate) error { } continue } - if err := os.Remove(update.path); err != nil && !os.IsNotExist(err) { + if err := removeFileDurable(update.path); err != nil { return fmt.Errorf("remove newly created %s: %w", update.path, err) } } return nil } +func revalidateMerlinHookRollback(update merlinHookUpdate) error { + info, err := os.Lstat(update.path) + if err != nil { + return err + } + if update.existed && info.Mode().Type() != update.pathType { + return fmt.Errorf("shared hook type changed after ctrld write") + } + if update.existed && update.pathType&os.ModeSymlink != 0 { + target, err := os.Readlink(update.path) + if err != nil { + return err + } + if target != update.symlinkTarget { + return fmt.Errorf("shared hook symlink target changed after ctrld write") + } + } + current, err := os.ReadFile(update.path) + if err != nil { + return err + } + if !bytes.Equal(current, update.data) { + return fmt.Errorf("shared hook content changed after ctrld write") + } + return nil +} + func prepareMerlinHookUpdate(path string, block []byte) (merlinHookUpdate, error) { return prepareMerlinHookReplacement(path, func(buf []byte) []byte { return merlinUpsertPostConf(buf, block) From 3f97e7769b5108708577a14862121ce071963203 Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 10:25:17 +0100 Subject: [PATCH 04/44] Re-arm volatile NVRAM marker after restore failure --- internal/router/nvram/nvram.go | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/internal/router/nvram/nvram.go b/internal/router/nvram/nvram.go index 59e2717d..abff803c 100644 --- a/internal/router/nvram/nvram.go +++ b/internal/router/nvram/nvram.go @@ -151,11 +151,20 @@ func Restore(m map[string]string, setupKey string) error { return fmt.Errorf("%s: %w", out, err) } if out, err := Run("commit"); err != nil { - // Do not persist setupKey=1 after the original values have been restored: - // that marker means "ctrld is configured" to other router backends. If - // this commit fails, return the error and let the caller retry/reconcile. - return fmt.Errorf("%s: %w", out, err) - + // The persistent commit may have failed while the volatile nvram view + // already has setupKey unset. Re-arm setupKey only in volatile NVRAM so + // Merlin Cleanup will retry Restore in this boot, but deliberately do not + // commit the marker: if the restored values actually reached persistent + // storage despite the reported error, a reboot must not resurrect a + // completed ctrld setup transaction. + commitErr := fmt.Errorf("%s: %w", out, err) + if markerOut, markerErr := Run("set", setupKey+"=1"); markerErr != nil { + return errors.Join( + commitErr, + fmt.Errorf("failed to re-arm volatile %s after restore commit failure: %s: %w", setupKey, markerOut, markerErr), + ) + } + return commitErr } return nil } From 0eb072fa124d0afaaeda13116b9c56815534cb77 Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 10:30:57 +0100 Subject: [PATCH 05/44] Scope NVRAM retry marker to Merlin --- internal/router/nvram/nvram.go | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/internal/router/nvram/nvram.go b/internal/router/nvram/nvram.go index abff803c..4b0ca708 100644 --- a/internal/router/nvram/nvram.go +++ b/internal/router/nvram/nvram.go @@ -136,6 +136,19 @@ func SetKV(m map[string]string, setupKey string) error { // that could destroy the only rollback copy after a transient nvram failure. // A future SetKV overwrites each backup with the then-current value. func Restore(m map[string]string, setupKey string) error { + return restore(m, setupKey, false) +} + +// RestoreWithVolatileRetryMarker is Merlin's retry-aware Restore variant. +// If the persistent restore commit fails after setupKey was unset in the +// volatile NVRAM view, setupKey is re-armed only in RAM so Merlin PreRun can +// retry Cleanup in the same boot. Other router backends intentionally use +// Restore and keep their existing one-shot semantics. +func RestoreWithVolatileRetryMarker(m map[string]string, setupKey string) error { + return restore(m, setupKey, true) +} + +func restore(m map[string]string, setupKey string, rearmVolatile bool) error { for key := range m { ctrldKey := CtrldKeyPrefix + key old, err := Run("get", ctrldKey) @@ -151,13 +164,16 @@ func Restore(m map[string]string, setupKey string) error { return fmt.Errorf("%s: %w", out, err) } if out, err := Run("commit"); err != nil { - // The persistent commit may have failed while the volatile nvram view - // already has setupKey unset. Re-arm setupKey only in volatile NVRAM so - // Merlin Cleanup will retry Restore in this boot, but deliberately do not - // commit the marker: if the restored values actually reached persistent - // storage despite the reported error, a reboot must not resurrect a - // completed ctrld setup transaction. commitErr := fmt.Errorf("%s: %w", out, err) + if !rearmVolatile { + return commitErr + } + + // Merlin retries Cleanup during PreRun. The persistent commit may have + // failed while the volatile NVRAM view already has setupKey unset. + // Re-arm setupKey only in RAM so that same-boot retry remains possible, + // but do not commit it: if the restored values actually reached flash + // despite the reported error, reboot must not resurrect a completed setup. if markerOut, markerErr := Run("set", setupKey+"=1"); markerErr != nil { return errors.Join( commitErr, From f058e01f460fa24ca298c5dac7fef5ef7e902edd Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 10:31:06 +0100 Subject: [PATCH 06/44] Use Merlin-specific NVRAM restore retry marker --- internal/router/merlin/merlin.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/router/merlin/merlin.go b/internal/router/merlin/merlin.go index 7bf249ae..7624758c 100644 --- a/internal/router/merlin/merlin.go +++ b/internal/router/merlin/merlin.go @@ -284,7 +284,7 @@ func (m *Merlin) Cleanup() error { // Restore NVRAM only after ctrld-owned artifacts are cleaned successfully. if setupVal == "1" { - if err := nvram.Restore(nvramKvMap, nvram.CtrldSetupKey); err != nil { + if err := nvram.RestoreWithVolatileRetryMarker(nvramKvMap, nvram.CtrldSetupKey); err != nil { return err } } From 0c963939faf4bfa45cc4db3bbf2ea3e4b5eb4521 Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 10:40:52 +0100 Subject: [PATCH 07/44] Scope transactional NVRAM setup to Merlin --- internal/router/nvram/nvram.go | 80 ++++++++++++++++++++-------------- 1 file changed, 47 insertions(+), 33 deletions(-) diff --git a/internal/router/nvram/nvram.go b/internal/router/nvram/nvram.go index 4b0ca708..0dd1eb87 100644 --- a/internal/router/nvram/nvram.go +++ b/internal/router/nvram/nvram.go @@ -39,12 +39,48 @@ NOTE: // SetKV writes the given key/value from map to nvram. // The given setupKey is set to 1 to indicate key/value set. // -// NVRAM mutations are not transactional. Keep enough rollback information in -// volatile NVRAM and restore it on any failure so callers never have to infer -// whether a partially failed sequence changed router state. +// Keep the long-standing generic semantics for router backends which do not +// have Merlin's retrying PreRun reconciliation. func SetKV(m map[string]string, setupKey string) error { + for key, value := range m { + old, err := Run("get", key) + if err != nil { + return fmt.Errorf("%s: %w", old, err) + } + if out, err := Run("set", CtrldKeyPrefix+key+"="+old); err != nil { + return fmt.Errorf("%s: %w", out, err) + } + if out, err := Run("set", key+"="+value); err != nil { + return fmt.Errorf("%s: %w", out, err) + } + } + if out, err := Run("set", setupKey+"=1"); err != nil { + return fmt.Errorf("%s: %w", out, err) + } + if out, err := Run("commit"); err != nil { + return fmt.Errorf("%s: %w", out, err) + } + return nil +} + +// SetKVWithVolatileRetryMarker is Merlin's transactional SetKV variant. +// It rolls back partially-applied values on failure. If that rollback cannot +// be made durable, setupKey is re-armed only in volatile NVRAM so Merlin's +// deferred/retrying Cleanup can reconcile the same boot without persisting a +// false "setup completed" marker for the next boot. +func SetKVWithVolatileRetryMarker(m map[string]string, setupKey string) error { modified := make([]string, 0, len(m)) + rearmVolatile := func(cause error) error { + if out, err := Run("set", setupKey+"=1"); err != nil { + return errors.Join( + cause, + fmt.Errorf("failed to re-arm volatile %s: %s: %w", setupKey, out, err), + ) + } + return cause + } + rollback := func(cause error) error { var restoreErr error for i := len(modified) - 1; i >= 0; i-- { @@ -59,43 +95,21 @@ func SetKV(m map[string]string, setupKey string) error { restoreErr = errors.Join(restoreErr, fmt.Errorf("%s: %w", out, err)) } } - if restoreErr != nil { - // At least one ctrld-modified value may still be active. Keep an - // explicit setup marker so Merlin Cleanup/Restore will retry from the - // retained ctrld_* backup values instead of treating the state as clean. - var markerErr error - if out, err := Run("set", setupKey+"=1"); err != nil { - markerErr = errors.Join(markerErr, fmt.Errorf("%s: %w", out, err)) - } - if out, err := Run("commit"); err != nil { - markerErr = errors.Join(markerErr, fmt.Errorf("%s: %w", out, err)) - } - return errors.Join( + return rearmVolatile(errors.Join( cause, fmt.Errorf("nvram rollback incomplete: %w", restoreErr), - markerErr, - ) + )) } if out, err := Run("unset", setupKey); err != nil { - return errors.Join(cause, fmt.Errorf("%s: %w", out, err)) - } - if _, err := Run("commit"); err != nil { - // The restored values may only be volatile. Re-arm the marker before - // returning so the caller's deferred Cleanup has a retryable state. - var markerErr error - if out, setErr := Run("set", setupKey+"=1"); setErr != nil { - markerErr = errors.Join(markerErr, fmt.Errorf("%s: %w", out, setErr)) - } - if out, commitErr := Run("commit"); commitErr != nil { - markerErr = errors.Join(markerErr, fmt.Errorf("%s: %w", out, commitErr)) - } - return errors.Join( + return rearmVolatile(errors.Join(cause, fmt.Errorf("%s: %w", out, err))) + } + if out, err := Run("commit"); err != nil { + return rearmVolatile(errors.Join( cause, - fmt.Errorf("nvram rollback commit failed: %w", err), - markerErr, - ) + fmt.Errorf("nvram rollback commit failed: %s: %w", out, err), + )) } return cause } From ad6acf06c240538ed39463cfcc3de0995b3292f2 Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 10:41:04 +0100 Subject: [PATCH 08/44] Use Merlin-specific transactional NVRAM setup --- internal/router/merlin/merlin.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/router/merlin/merlin.go b/internal/router/merlin/merlin.go index 7624758c..9481bc2a 100644 --- a/internal/router/merlin/merlin.go +++ b/internal/router/merlin/merlin.go @@ -133,7 +133,7 @@ func (m *Merlin) Setup() (retErr error) { // Apply NVRAM changes before regenerating dnsmasq so the firmware-generated // config reflects ctrld's desired DNS Privacy state. nvram.SetKV rolls back // its own partial mutations on failure. - if err := nvram.SetKV(nvramKvMap, nvram.CtrldSetupKey); err != nil { + if err := nvram.SetKVWithVolatileRetryMarker(nvramKvMap, nvram.CtrldSetupKey); err != nil { return err } From bacef5c4b3a53f641fd4a281e196b733ed8ecf37 Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 10:41:07 +0100 Subject: [PATCH 09/44] Fix Merlin verifier test import --- internal/router/merlin/merlin_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/router/merlin/merlin_test.go b/internal/router/merlin/merlin_test.go index 0c7cf6d5..e74a16ca 100644 --- a/internal/router/merlin/merlin_test.go +++ b/internal/router/merlin/merlin_test.go @@ -8,6 +8,7 @@ import ( "strings" "testing" + "github.com/Control-D-Inc/ctrld" "github.com/Control-D-Inc/ctrld/internal/router/dnsmasq" ) From 4659a75c500afdac47c51dc4ecf1b3eccae0b196 Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 10:46:35 +0100 Subject: [PATCH 10/44] Publish Merlin fallback without clobbering user config --- internal/router/merlin/merlin.go | 49 +++++++++++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/internal/router/merlin/merlin.go b/internal/router/merlin/merlin.go index 9481bc2a..ea9937b3 100644 --- a/internal/router/merlin/merlin.go +++ b/internal/router/merlin/merlin.go @@ -545,7 +545,10 @@ func (m *Merlin) setupMainDnsmasqFallback() error { if err := atomicWriteFile(merlinSnapshotStatePath, []byte("sha256="+hash+"\n"), 0600); err != nil { return fmt.Errorf("mark ctrld dnsmasq fallback ownership: %w", err) } - if err := atomicWriteFile(dnsmasq.MerlinJffsConfPath, built, 0644); err != nil { + // Publish without replacement semantics. Between the earlier ownership + // check and this point an administrator/addon may legitimately create a new + // dnsmasq.conf; ctrld must never clobber that concurrently-created file. + if err := writeFileNoReplace(dnsmasq.MerlinJffsConfPath, built, 0644); err != nil { current, readErr := os.ReadFile(dnsmasq.MerlinJffsConfPath) switch { case readErr == nil && merlinSnapshotHash(current) == hash: @@ -572,6 +575,50 @@ func (m *Merlin) setupMainDnsmasqFallback() error { return nil } +func writeFileNoReplace(target string, data []byte, mode os.FileMode) error { + dir := filepath.Dir(target) + base := filepath.Base(target) + tmp, err := os.CreateTemp(dir, "."+base+".ctrld-noreplace-*") + if err != nil { + return err + } + tmpPath := tmp.Name() + cleanup := true + defer func() { + _ = tmp.Close() + if cleanup { + _ = os.Remove(tmpPath) + } + }() + + if err := tmp.Chmod(mode); err != nil { + return err + } + if _, err := tmp.Write(data); err != nil { + return err + } + if err := tmp.Sync(); err != nil { + return err + } + if err := tmp.Close(); err != nil { + return err + } + + // link(2) is atomic with respect to target existence and never replaces an + // existing pathname. The temporary file lives in the same directory/filesystem. + if err := os.Link(tmpPath, target); err != nil { + return err + } + if err := os.Remove(tmpPath); err != nil { + // The target is already published and valid; retain normal publication + // semantics but report cleanup failure to the caller for reconciliation. + cleanup = false + return err + } + cleanup = false + return syncParentDir(dir) +} + func (m *Merlin) buildMainDnsmasqFallback(buf []byte) ([]byte, error) { tmp, err := os.CreateTemp(dnsmasq.MerlinJffsConfDir, ".dnsmasq.conf.ctrld-build-*") if err != nil { From 9472a636cb9be8ceac5a54650a329df30b711a1c Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 10:47:11 +0100 Subject: [PATCH 11/44] Test no-clobber Merlin fallback publication --- internal/router/merlin/merlin_test.go | 38 +++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/internal/router/merlin/merlin_test.go b/internal/router/merlin/merlin_test.go index e74a16ca..0e90b2a8 100644 --- a/internal/router/merlin/merlin_test.go +++ b/internal/router/merlin/merlin_test.go @@ -601,3 +601,41 @@ func Test_cleanupPreparationRevalidatesBeforeWrite(t *testing.T) { } } + + +func Test_writeFileNoReplaceRefusesExistingTarget(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "dnsmasq.conf") + original := []byte("user-owned\n") + if err := os.WriteFile(path, original, 0644); err != nil { + t.Fatal(err) + } + + if err := writeFileNoReplace(path, []byte("ctrld-owned\n"), 0644); err == nil { + t.Fatal("expected no-replace publication to fail when target already exists") + } + got, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(got, original) { + t.Fatalf("existing target was modified:\nwant: %q\ngot: %q", original, got) + } +} + +func Test_writeFileNoReplacePublishesAbsentTarget(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "dnsmasq.conf") + want := []byte("ctrld-owned\n") + + if err := writeFileNoReplace(path, want, 0644); err != nil { + t.Fatal(err) + } + got, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(got, want) { + t.Fatalf("published target mismatch:\nwant: %q\ngot: %q", want, got) + } +} From 459549138ecf4ed2472756d589b15e55b661420b Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 10:56:52 +0100 Subject: [PATCH 12/44] Prepare Merlin snapshot ownership transactions --- internal/router/merlin/merlin.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/router/merlin/merlin.go b/internal/router/merlin/merlin.go index ea9937b3..f4e84850 100644 --- a/internal/router/merlin/merlin.go +++ b/internal/router/merlin/merlin.go @@ -2,6 +2,7 @@ package merlin import ( "bytes" + "crypto/rand" "crypto/sha256" "encoding/hex" "fmt" From 21a15311fdfbed248a9736bd05291ddc6d2fdc1c Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 10:58:34 +0100 Subject: [PATCH 13/44] Bind Merlin fallback ownership to publication and removal identity --- internal/router/merlin/merlin.go | 145 ++++++++++++++++++++++--------- 1 file changed, 104 insertions(+), 41 deletions(-) diff --git a/internal/router/merlin/merlin.go b/internal/router/merlin/merlin.go index f4e84850..1416037a 100644 --- a/internal/router/merlin/merlin.go +++ b/internal/router/merlin/merlin.go @@ -2,7 +2,6 @@ package merlin import ( "bytes" - "crypto/rand" "crypto/sha256" "encoding/hex" "fmt" @@ -549,39 +548,33 @@ func (m *Merlin) setupMainDnsmasqFallback() error { // Publish without replacement semantics. Between the earlier ownership // check and this point an administrator/addon may legitimately create a new // dnsmasq.conf; ctrld must never clobber that concurrently-created file. - if err := writeFileNoReplace(dnsmasq.MerlinJffsConfPath, built, 0644); err != nil { - current, readErr := os.ReadFile(dnsmasq.MerlinJffsConfPath) - switch { - case readErr == nil && merlinSnapshotHash(current) == hash: - // Rename reached the target. Keep the already-durable ownership marker - // so deferred/retry cleanup can safely remove the visible fallback. - return fmt.Errorf("publish ctrld dnsmasq fallback after rename: %w", err) - case os.IsNotExist(readErr): + published, err := writeFileNoReplace(dnsmasq.MerlinJffsConfPath, built, 0644) + if err != nil { + if !published { + // No link to the target was created by ctrld. Even if another actor + // concurrently created byte-identical content, ctrld must not infer + // ownership from bytes alone. if removeErr := removeFileDurable(merlinSnapshotStatePath); removeErr != nil { return fmt.Errorf("publish ctrld dnsmasq fallback: %w; remove ownership state: %v", err, removeErr) } - return fmt.Errorf("publish ctrld dnsmasq fallback: %w", err) - case readErr == nil: - // A different readable file is present, so ctrld cannot claim it. - if removeErr := removeFileDurable(merlinSnapshotStatePath); removeErr != nil { - return fmt.Errorf("publish ctrld dnsmasq fallback: %w; unexpected target and ownership cleanup failed: %v", err, removeErr) - } - return fmt.Errorf("publish ctrld dnsmasq fallback: %w; target content does not match ctrld snapshot", err) - default: - // Publication state is uncertain. Retain the marker rather than turn - // a possibly successful rename into a permanently unowned snapshot. - return fmt.Errorf("publish ctrld dnsmasq fallback: %w; verify target: %v", err, readErr) } + // If published is true, the no-replace link succeeded and only a + // post-publication cleanup/sync step failed. Retain ownership so Cleanup + // can reconcile the exact published snapshot safely. + return fmt.Errorf("publish ctrld dnsmasq fallback: %w", err) } return nil } -func writeFileNoReplace(target string, data []byte, mode os.FileMode) error { +// writeFileNoReplace publishes target only if it does not already exist. +// published reports whether ctrld's inode was successfully linked at target; +// callers must not infer ownership from target contents when published is false. +func writeFileNoReplace(target string, data []byte, mode os.FileMode) (published bool, err error) { dir := filepath.Dir(target) base := filepath.Base(target) tmp, err := os.CreateTemp(dir, "."+base+".ctrld-noreplace-*") if err != nil { - return err + return false, err } tmpPath := tmp.Name() cleanup := true @@ -593,31 +586,35 @@ func writeFileNoReplace(target string, data []byte, mode os.FileMode) error { }() if err := tmp.Chmod(mode); err != nil { - return err + return false, err } if _, err := tmp.Write(data); err != nil { - return err + return false, err } if err := tmp.Sync(); err != nil { - return err + return false, err } if err := tmp.Close(); err != nil { - return err + return false, err } // link(2) is atomic with respect to target existence and never replaces an // existing pathname. The temporary file lives in the same directory/filesystem. if err := os.Link(tmpPath, target); err != nil { - return err + return false, err } + published = true if err := os.Remove(tmpPath); err != nil { - // The target is already published and valid; retain normal publication - // semantics but report cleanup failure to the caller for reconciliation. + // The target is already published and valid; report the failure with + // published=true so the caller retains ownership state. cleanup = false - return err + return true, err } cleanup = false - return syncParentDir(dir) + if err := syncParentDir(dir); err != nil { + return true, err + } + return true, nil } func (m *Merlin) buildMainDnsmasqFallback(buf []byte) ([]byte, error) { @@ -706,23 +703,89 @@ func cleanupOwnedMainSnapshot() error { return nil } - buf, err := os.ReadFile(dnsmasq.MerlinJffsConfPath) - if os.IsNotExist(err) { + removed, err := quarantineRemoveOwnedFile(dnsmasq.MerlinJffsConfPath, expected) + if err != nil { + return err + } + if !removed { + // No target exists. The marker is orphaned and can be removed. return removeFileDurable(merlinSnapshotStatePath) } + return removeFileDurable(merlinSnapshotStatePath) +} + +// quarantineRemoveOwnedFile atomically moves path out of the public pathname, +// then verifies the captured file before deleting it. This binds deletion to +// the exact inode that was renamed, rather than to a pathname that another +// addon/user could replace between a hash check and unlink. +// +// If the captured file is not ctrld-owned, it is restored with no-clobber +// semantics. If another actor recreates path before restoration, both files are +// preserved and an error identifies the quarantine location for recovery. +func quarantineRemoveOwnedFile(path, expectedHash string) (bool, error) { + dir := filepath.Dir(path) + base := filepath.Base(path) + quarantineDir, err := os.MkdirTemp(dir, "."+base+".ctrld-quarantine-*") + if err != nil { + return false, err + } + quarantine := filepath.Join(quarantineDir, "snapshot") + cleanupDir := true + defer func() { + if cleanupDir { + _ = os.Remove(quarantineDir) + } + }() + + if err := os.Rename(path, quarantine); err != nil { + if os.IsNotExist(err) { + return false, nil + } + return false, err + } + + buf, err := os.ReadFile(quarantine) if err != nil { - return err + cleanupDir = false + return true, fmt.Errorf("read quarantined Merlin fallback %s: %w", quarantine, err) + } + if merlinSnapshotHash(buf) != expectedHash { + // Restore only if nobody recreated the public pathname in the meantime. + if err := os.Link(quarantine, path); err != nil { + cleanupDir = false + return true, fmt.Errorf( + "refusing to remove modified Merlin dnsmasq fallback; preserved captured file at %s and could not restore %s: %w", + quarantine, path, err, + ) + } + if err := os.Remove(quarantine); err != nil { + cleanupDir = false + return true, fmt.Errorf("restored modified Merlin fallback but could not remove quarantine %s: %w", quarantine, err) + } + if err := os.Remove(quarantineDir); err != nil { + cleanupDir = false + return true, err + } + cleanupDir = false + if err := syncParentDir(dir); err != nil { + return true, err + } + return true, fmt.Errorf("refusing to remove modified Merlin dnsmasq fallback: %s", path) } - if merlinSnapshotHash(buf) != expected { - return fmt.Errorf("refusing to remove modified Merlin dnsmasq fallback: %s", dnsmasq.MerlinJffsConfPath) + + if err := os.Remove(quarantine); err != nil { + cleanupDir = false + return true, fmt.Errorf("remove quarantined ctrld fallback %s: %w", quarantine, err) } - if err := removeFileDurable(dnsmasq.MerlinJffsConfPath); err != nil { - return err + if err := os.Remove(quarantineDir); err != nil { + cleanupDir = false + return true, err } - if err := removeFileDurable(merlinSnapshotStatePath); err != nil { - return err + cleanupDir = false + if err := syncParentDir(dir); err != nil { + return true, err } - return nil + return true, nil } // cleanupDnsmasqJffs removes the JFFS configuration file specified in the given dnsmasqConfig, if it exists. From 2d38d651378d6d18a66a578bd4c4a5b758e19909 Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 10:59:06 +0100 Subject: [PATCH 14/44] Test Merlin fallback publication identity and quarantine cleanup --- internal/router/merlin/merlin_test.go | 58 ++++++++++++++++++++++++++- 1 file changed, 56 insertions(+), 2 deletions(-) diff --git a/internal/router/merlin/merlin_test.go b/internal/router/merlin/merlin_test.go index 0e90b2a8..a623d39e 100644 --- a/internal/router/merlin/merlin_test.go +++ b/internal/router/merlin/merlin_test.go @@ -611,9 +611,13 @@ func Test_writeFileNoReplaceRefusesExistingTarget(t *testing.T) { t.Fatal(err) } - if err := writeFileNoReplace(path, []byte("ctrld-owned\n"), 0644); err == nil { + published, err := writeFileNoReplace(path, []byte("ctrld-owned\n"), 0644) + if err == nil { t.Fatal("expected no-replace publication to fail when target already exists") } + if published { + t.Fatal("existing-target conflict must report published=false") + } got, err := os.ReadFile(path) if err != nil { t.Fatal(err) @@ -628,9 +632,13 @@ func Test_writeFileNoReplacePublishesAbsentTarget(t *testing.T) { path := filepath.Join(dir, "dnsmasq.conf") want := []byte("ctrld-owned\n") - if err := writeFileNoReplace(path, want, 0644); err != nil { + published, err := writeFileNoReplace(path, want, 0644) + if err != nil { t.Fatal(err) } + if !published { + t.Fatal("successful no-replace publication must report published=true") + } got, err := os.ReadFile(path) if err != nil { t.Fatal(err) @@ -639,3 +647,49 @@ func Test_writeFileNoReplacePublishesAbsentTarget(t *testing.T) { t.Fatalf("published target mismatch:\nwant: %q\ngot: %q", want, got) } } + + +func Test_quarantineRemoveOwnedFileDeletesCapturedOwnedSnapshot(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "dnsmasq.conf") + content := []byte("ctrld-owned\n") + if err := os.WriteFile(path, content, 0644); err != nil { + t.Fatal(err) + } + + removed, err := quarantineRemoveOwnedFile(path, merlinSnapshotHash(content)) + if err != nil { + t.Fatal(err) + } + if !removed { + t.Fatal("expected an existing owned snapshot to be captured") + } + if _, err := os.Stat(path); !os.IsNotExist(err) { + t.Fatalf("owned snapshot still exists after cleanup: %v", err) + } +} + +func Test_quarantineRemoveOwnedFileRestoresModifiedSnapshot(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "dnsmasq.conf") + owned := []byte("ctrld-owned\n") + modified := []byte("user-modified\n") + if err := os.WriteFile(path, modified, 0644); err != nil { + t.Fatal(err) + } + + removed, err := quarantineRemoveOwnedFile(path, merlinSnapshotHash(owned)) + if err == nil { + t.Fatal("expected modified snapshot cleanup refusal") + } + if !removed { + t.Fatal("expected modified snapshot to be captured and restored") + } + got, readErr := os.ReadFile(path) + if readErr != nil { + t.Fatal(readErr) + } + if !bytes.Equal(got, modified) { + t.Fatalf("modified snapshot was not restored byte-for-byte:\nwant: %q\ngot: %q", modified, got) + } +} From 3c7deba2a57b3cdaf29d08b34e0ff48efe1160b7 Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 11:18:25 +0100 Subject: [PATCH 15/44] Add durable Merlin snapshot identity paths --- internal/router/merlin/merlin.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/internal/router/merlin/merlin.go b/internal/router/merlin/merlin.go index 1416037a..74cb1860 100644 --- a/internal/router/merlin/merlin.go +++ b/internal/router/merlin/merlin.go @@ -23,9 +23,11 @@ import ( const Name = "merlin" const ( - merlinManagedStatePath = "/jffs/controld/.merlin-dnsmasq-hooks-v2" - merlinSnapshotStatePath = "/jffs/controld/.merlin-dnsmasq-snapshot" - merlinCleanupPendingPath = "/jffs/controld/.merlin-dnsmasq-cleanup-pending" + merlinManagedStatePath = "/jffs/controld/.merlin-dnsmasq-hooks-v2" + merlinSnapshotStatePath = "/jffs/controld/.merlin-dnsmasq-snapshot" + merlinSnapshotAnchorPath = "/jffs/configs/.dnsmasq.conf.ctrld-anchor" + merlinSnapshotQuarantinePath = "/jffs/configs/.dnsmasq.conf.ctrld-quarantine" + merlinCleanupPendingPath = "/jffs/controld/.merlin-dnsmasq-cleanup-pending" ) // nvramKvMap is a map of NVRAM key-value pairs used to configure and manage Merlin-specific settings. From bde9735028d8572f4a3a491eaaab081919380e5a Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 11:19:40 +0100 Subject: [PATCH 16/44] Journal exact Merlin fallback inode ownership --- internal/router/merlin/merlin.go | 129 +++++++++++++------------------ 1 file changed, 53 insertions(+), 76 deletions(-) diff --git a/internal/router/merlin/merlin.go b/internal/router/merlin/merlin.go index 74cb1860..4aa873ae 100644 --- a/internal/router/merlin/merlin.go +++ b/internal/router/merlin/merlin.go @@ -501,35 +501,21 @@ func (m *Merlin) dnsmasqConfigUsesCtrld(path string) (bool, error) { // setupMainDnsmasqFallback retains the old full-config mechanism only for the // main dnsmasq when the supported postconf hook demonstrably did not apply. -// Ownership is published before the final snapshot, and content hashes prevent -// ctrld from later deleting or overwriting a user-modified file. +// Ownership is represented by a private hard-link anchor to the exact inode +// ctrld published, plus a durable phase/hash journal. func (m *Merlin) setupMainDnsmasqFallback() error { - owned, err := mainSnapshotOwnership() - if err != nil { + // Reconcile any interrupted ctrld fallback transaction first. Private ctrld + // artifacts are handled without inferring ownership from public-path bytes. + if err := cleanupOwnedMainSnapshot(); err != nil { return err } + snapshotExists, err := pathExists(dnsmasq.MerlinJffsConfPath) if err != nil { return fmt.Errorf("stat Merlin fallback config: %w", err) } if snapshotExists { - if !owned { - return fmt.Errorf("refusing to overwrite unowned or modified Merlin custom config: %s", dnsmasq.MerlinJffsConfPath) - } - if err := cleanupOwnedMainSnapshot(); err != nil { - return err - } - } else { - stateExists, err := pathExists(merlinSnapshotStatePath) - if err != nil { - return fmt.Errorf("stat Merlin fallback ownership: %w", err) - } - if stateExists { - // A previous crash may have published ownership before the snapshot. - if err := removeFileDurable(merlinSnapshotStatePath); err != nil { - return fmt.Errorf("remove orphaned Merlin snapshot state: %w", err) - } - } + return fmt.Errorf("refusing to overwrite unowned Merlin custom config: %s", dnsmasq.MerlinJffsConfPath) } buf, err := os.ReadFile(dnsmasq.MerlinConfPath) @@ -540,83 +526,74 @@ func (m *Merlin) setupMainDnsmasqFallback() error { if err != nil { return err } - hash := merlinSnapshotHash(built) - - // Publish durable ownership first. If the router stops before the snapshot - // rename, cleanup sees a harmless orphaned marker and removes it. - if err := atomicWriteFile(merlinSnapshotStatePath, []byte("sha256="+hash+"\n"), 0600); err != nil { - return fmt.Errorf("mark ctrld dnsmasq fallback ownership: %w", err) - } - // Publish without replacement semantics. Between the earlier ownership - // check and this point an administrator/addon may legitimately create a new - // dnsmasq.conf; ctrld must never clobber that concurrently-created file. - published, err := writeFileNoReplace(dnsmasq.MerlinJffsConfPath, built, 0644) - if err != nil { - if !published { - // No link to the target was created by ctrld. Even if another actor - // concurrently created byte-identical content, ctrld must not infer - // ownership from bytes alone. - if removeErr := removeFileDurable(merlinSnapshotStatePath); removeErr != nil { - return fmt.Errorf("publish ctrld dnsmasq fallback: %w; remove ownership state: %v", err, removeErr) - } - } - // If published is true, the no-replace link succeeded and only a - // post-publication cleanup/sync step failed. Retain ownership so Cleanup - // can reconcile the exact published snapshot safely. + if err := publishMainSnapshot(built, 0644); err != nil { return fmt.Errorf("publish ctrld dnsmasq fallback: %w", err) } return nil } -// writeFileNoReplace publishes target only if it does not already exist. -// published reports whether ctrld's inode was successfully linked at target; -// callers must not infer ownership from target contents when published is false. -func writeFileNoReplace(target string, data []byte, mode os.FileMode) (published bool, err error) { - dir := filepath.Dir(target) - base := filepath.Base(target) - tmp, err := os.CreateTemp(dir, "."+base+".ctrld-noreplace-*") +// publishMainSnapshot publishes ctrld's fallback with identity-based ownership. +// +// The private anchor is a hard link to the exact inode prepared by ctrld. A +// pending journal is made durable before the public hard link is attempted, so +// cleanup can distinguish a failed no-clobber publication from a ctrld-owned +// target even after power loss. +func publishMainSnapshot(data []byte, mode os.FileMode) error { + dir := dnsmasq.MerlinJffsConfDir + base := filepath.Base(dnsmasq.MerlinJffsConfPath) + tmp, err := os.CreateTemp(dir, "."+base+".ctrld-publish-*") if err != nil { - return false, err + return err } tmpPath := tmp.Name() - cleanup := true - defer func() { - _ = tmp.Close() - if cleanup { - _ = os.Remove(tmpPath) - } - }() + defer os.Remove(tmpPath) if err := tmp.Chmod(mode); err != nil { - return false, err + _ = tmp.Close() + return err } if _, err := tmp.Write(data); err != nil { - return false, err + _ = tmp.Close() + return err } if err := tmp.Sync(); err != nil { - return false, err + _ = tmp.Close() + return err } if err := tmp.Close(); err != nil { - return false, err + return err } - // link(2) is atomic with respect to target existence and never replaces an - // existing pathname. The temporary file lives in the same directory/filesystem. - if err := os.Link(tmpPath, target); err != nil { - return false, err + // The anchor must be created before the journal/public link. It gives + // cleanup an inode identity that hashes alone cannot provide. + if err := os.Link(tmpPath, merlinSnapshotAnchorPath); err != nil { + return fmt.Errorf("create fallback ownership anchor: %w", err) } - published = true - if err := os.Remove(tmpPath); err != nil { - // The target is already published and valid; report the failure with - // published=true so the caller retains ownership state. - cleanup = false - return true, err + if err := syncParentDir(dir); err != nil { + return fmt.Errorf("sync fallback ownership anchor: %w", err) + } + + state := mainSnapshotState{phase: snapshotPhasePending, hash: merlinSnapshotHash(data)} + if err := writeMainSnapshotState(state); err != nil { + return fmt.Errorf("journal pending fallback publication: %w", err) + } + + // Hard-link publication is atomic and no-clobber: a concurrent user/addon + // file at dnsmasq.conf causes EEXIST rather than replacement. + if err := os.Link(tmpPath, dnsmasq.MerlinJffsConfPath); err != nil { + return err } - cleanup = false if err := syncParentDir(dir); err != nil { - return true, err + return fmt.Errorf("sync published fallback: %w", err) } - return true, nil + + state.phase = snapshotPhasePublished + if err := writeMainSnapshotState(state); err != nil { + // Pending state plus the anchor is sufficient for cleanup to prove that + // the public inode is ours, so this remains safely retryable. + return fmt.Errorf("journal published fallback: %w", err) + } + return nil } func (m *Merlin) buildMainDnsmasqFallback(buf []byte) ([]byte, error) { From cf13a5e7d9731917e09dbefd514e630bd6ceca4c Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 11:22:04 +0100 Subject: [PATCH 17/44] Make Merlin fallback cleanup crash-resumable --- internal/router/merlin/merlin.go | 341 +++++++++++++++++++++++-------- 1 file changed, 259 insertions(+), 82 deletions(-) diff --git a/internal/router/merlin/merlin.go b/internal/router/merlin/merlin.go index 4aa873ae..558b0990 100644 --- a/internal/router/merlin/merlin.go +++ b/internal/router/merlin/merlin.go @@ -634,137 +634,314 @@ func (m *Merlin) buildMainDnsmasqFallback(buf []byte) ([]byte, error) { return built, nil } +const ( + snapshotPhasePending = "pending-v2" + snapshotPhasePublished = "published-v2" + snapshotPhaseQuarantine = "quarantine-v2" + snapshotPhaseDelete = "delete-v2" + snapshotPhaseRestore = "restore-v2" + snapshotPhaseRestored = "restored-v2" +) + +type mainSnapshotState struct { + phase string + hash string +} + func merlinSnapshotHash(buf []byte) string { sum := sha256.Sum256(buf) return hex.EncodeToString(sum[:]) } -func readMainSnapshotHash() (string, bool, error) { - state, err := readMerlinState(merlinSnapshotStatePath) +func writeMainSnapshotState(state mainSnapshotState) error { + if state.phase == "" { + return fmt.Errorf("empty Merlin snapshot phase") + } + if len(state.hash) != sha256.Size*2 { + return fmt.Errorf("invalid Merlin snapshot hash length") + } + if _, err := hex.DecodeString(state.hash); err != nil { + return fmt.Errorf("invalid Merlin snapshot hash: %w", err) + } + buf := []byte("snapshot-v2\nphase=" + state.phase + "\nsha256=" + state.hash + "\n") + return atomicWriteFile(merlinSnapshotStatePath, buf, 0600) +} + +func readMainSnapshotState() (mainSnapshotState, bool, error) { + buf, err := os.ReadFile(merlinSnapshotStatePath) + if os.IsNotExist(err) { + return mainSnapshotState{}, false, nil + } if err != nil { - return "", false, err + return mainSnapshotState{}, false, err } - if state == "" { - return "", false, nil + lines := strings.Split(strings.TrimSpace(string(buf)), "\n") + + // No released ctrld version wrote this hash-only format. Reject it rather + // than guessing inode ownership if an intermediate development build left it. + if len(lines) == 1 && strings.HasPrefix(lines[0], "sha256=") { + return mainSnapshotState{}, true, fmt.Errorf( + "unsafe hash-only Merlin snapshot state requires manual reconciliation: %s", + merlinSnapshotStatePath, + ) } - const prefix = "sha256=" - if !strings.HasPrefix(state, prefix) || len(state) != len(prefix)+sha256.Size*2 { - return "", false, fmt.Errorf("invalid Merlin dnsmasq snapshot ownership state") + if len(lines) != 3 || lines[0] != "snapshot-v2" || + !strings.HasPrefix(lines[1], "phase=") || + !strings.HasPrefix(lines[2], "sha256=") { + return mainSnapshotState{}, true, fmt.Errorf("invalid Merlin snapshot state") } - hash := strings.TrimPrefix(state, prefix) - if _, err := hex.DecodeString(hash); err != nil { - return "", false, fmt.Errorf("invalid Merlin dnsmasq snapshot hash: %w", err) + + state := mainSnapshotState{ + phase: strings.TrimPrefix(lines[1], "phase="), + hash: strings.TrimPrefix(lines[2], "sha256="), + } + switch state.phase { + case snapshotPhasePending, snapshotPhasePublished, snapshotPhaseQuarantine, + snapshotPhaseDelete, snapshotPhaseRestore, snapshotPhaseRestored: + default: + return mainSnapshotState{}, true, fmt.Errorf("unknown Merlin snapshot phase %q", state.phase) + } + if len(state.hash) != sha256.Size*2 { + return mainSnapshotState{}, true, fmt.Errorf("invalid Merlin snapshot hash length") + } + if _, err := hex.DecodeString(state.hash); err != nil { + return mainSnapshotState{}, true, fmt.Errorf("invalid Merlin snapshot hash: %w", err) } - return hash, true, nil + return state, true, nil } -func mainSnapshotOwnership() (bool, error) { - expected, marked, err := readMainSnapshotHash() - if err != nil || !marked { +func sameFilePaths(a, b string) (bool, error) { + ai, err := os.Stat(a) + if err != nil { return false, err } - buf, err := os.ReadFile(dnsmasq.MerlinJffsConfPath) - if os.IsNotExist(err) { - return false, nil - } + bi, err := os.Stat(b) if err != nil { return false, err } - return merlinSnapshotHash(buf) == expected, nil + return os.SameFile(ai, bi), nil +} + +func cleanupSnapshotPrivateState() error { + if err := removeFileDurable(merlinSnapshotAnchorPath); err != nil { + return fmt.Errorf("remove Merlin snapshot anchor: %w", err) + } + if err := removeFileDurable(merlinSnapshotStatePath); err != nil { + return fmt.Errorf("remove Merlin snapshot state: %w", err) + } + return nil } +// cleanupOwnedMainSnapshot reconciles ctrld's fallback transaction without ever +// deleting a pathname based on content alone. The anchor proves inode identity; +// the quarantine phases make every move resumable after power loss. func cleanupOwnedMainSnapshot() error { - expected, marked, err := readMainSnapshotHash() + state, marked, err := readMainSnapshotState() if err != nil { return err } if !marked { + quarantineExists, err := pathExists(merlinSnapshotQuarantinePath) + if err != nil { + return err + } + if quarantineExists { + return fmt.Errorf( + "unjournaled Merlin fallback quarantine requires manual reconciliation: %s", + merlinSnapshotQuarantinePath, + ) + } + // A private anchor without a journal can only precede public publication: + // publishMainSnapshot makes the journal durable before linking dnsmasq.conf. + if err := removeFileDurable(merlinSnapshotAnchorPath); err != nil { + return fmt.Errorf("remove orphaned Merlin snapshot anchor: %w", err) + } + return nil + } + + switch state.phase { + case snapshotPhasePending: + return cleanupPendingMainSnapshot(state) + case snapshotPhasePublished: + state.phase = snapshotPhaseQuarantine + if err := writeMainSnapshotState(state); err != nil { + return fmt.Errorf("journal Merlin fallback quarantine: %w", err) + } + return cleanupOwnedMainSnapshot() + case snapshotPhaseQuarantine: + return cleanupQuarantinedMainSnapshot(state) + case snapshotPhaseDelete: + return finalizeOwnedMainSnapshotDelete() + case snapshotPhaseRestore: + return restoreQuarantinedMainSnapshot(state) + case snapshotPhaseRestored: + return finalizeRestoredMainSnapshot() + default: + return fmt.Errorf("unknown Merlin snapshot phase %q", state.phase) + } +} + +func cleanupPendingMainSnapshot(state mainSnapshotState) error { + anchorExists, err := pathExists(merlinSnapshotAnchorPath) + if err != nil { + return err + } + targetExists, err := pathExists(dnsmasq.MerlinJffsConfPath) + if err != nil { + return err + } + + if !anchorExists { + // Without the private inode proof, never touch a public target. + if err := removeFileDurable(merlinSnapshotStatePath); err != nil { + return err + } + if targetExists { + return fmt.Errorf( + "Merlin fallback publication lost its ownership anchor; left %s untouched", + dnsmasq.MerlinJffsConfPath, + ) + } return nil } + if !targetExists { + return cleanupSnapshotPrivateState() + } - removed, err := quarantineRemoveOwnedFile(dnsmasq.MerlinJffsConfPath, expected) + same, err := sameFilePaths(dnsmasq.MerlinJffsConfPath, merlinSnapshotAnchorPath) if err != nil { return err } - if !removed { - // No target exists. The marker is orphaned and can be removed. - return removeFileDurable(merlinSnapshotStatePath) + if !same { + // Publication lost a no-clobber race. The public file belongs to someone + // else regardless of whether its bytes happen to match ctrld's snapshot. + return cleanupSnapshotPrivateState() + } + + state.phase = snapshotPhasePublished + if err := writeMainSnapshotState(state); err != nil { + return fmt.Errorf("promote pending Merlin fallback ownership: %w", err) } - return removeFileDurable(merlinSnapshotStatePath) + return cleanupOwnedMainSnapshot() } -// quarantineRemoveOwnedFile atomically moves path out of the public pathname, -// then verifies the captured file before deleting it. This binds deletion to -// the exact inode that was renamed, rather than to a pathname that another -// addon/user could replace between a hash check and unlink. -// -// If the captured file is not ctrld-owned, it is restored with no-clobber -// semantics. If another actor recreates path before restoration, both files are -// preserved and an error identifies the quarantine location for recovery. -func quarantineRemoveOwnedFile(path, expectedHash string) (bool, error) { - dir := filepath.Dir(path) - base := filepath.Base(path) - quarantineDir, err := os.MkdirTemp(dir, "."+base+".ctrld-quarantine-*") +func cleanupQuarantinedMainSnapshot(state mainSnapshotState) error { + quarantineExists, err := pathExists(merlinSnapshotQuarantinePath) if err != nil { - return false, err + return err } - quarantine := filepath.Join(quarantineDir, "snapshot") - cleanupDir := true - defer func() { - if cleanupDir { - _ = os.Remove(quarantineDir) + if !quarantineExists { + targetExists, err := pathExists(dnsmasq.MerlinJffsConfPath) + if err != nil { + return err } - }() + if !targetExists { + // Nothing remains at the public pathname. Drop only ctrld-private proof. + return cleanupSnapshotPrivateState() + } + if err := os.Rename(dnsmasq.MerlinJffsConfPath, merlinSnapshotQuarantinePath); err != nil { + if os.IsNotExist(err) { + return cleanupOwnedMainSnapshot() + } + return fmt.Errorf("quarantine Merlin fallback: %w", err) + } + if err := syncParentDir(dnsmasq.MerlinJffsConfDir); err != nil { + return fmt.Errorf("sync Merlin fallback quarantine: %w", err) + } + } - if err := os.Rename(path, quarantine); err != nil { - if os.IsNotExist(err) { - return false, nil + anchorExists, err := pathExists(merlinSnapshotAnchorPath) + if err != nil { + return err + } + if !anchorExists { + state.phase = snapshotPhaseRestore + if err := writeMainSnapshotState(state); err != nil { + return err } - return false, err + return cleanupOwnedMainSnapshot() } - buf, err := os.ReadFile(quarantine) + same, err := sameFilePaths(merlinSnapshotQuarantinePath, merlinSnapshotAnchorPath) if err != nil { - cleanupDir = false - return true, fmt.Errorf("read quarantined Merlin fallback %s: %w", quarantine, err) - } - if merlinSnapshotHash(buf) != expectedHash { - // Restore only if nobody recreated the public pathname in the meantime. - if err := os.Link(quarantine, path); err != nil { - cleanupDir = false - return true, fmt.Errorf( - "refusing to remove modified Merlin dnsmasq fallback; preserved captured file at %s and could not restore %s: %w", - quarantine, path, err, - ) + return err + } + buf, err := os.ReadFile(merlinSnapshotQuarantinePath) + if err != nil { + return fmt.Errorf("read quarantined Merlin fallback: %w", err) + } + if same && merlinSnapshotHash(buf) == state.hash { + state.phase = snapshotPhaseDelete + } else { + // Different inode or changed bytes mean user/addon ownership may have + // superseded ctrld. Preserve and restore the captured file. + state.phase = snapshotPhaseRestore + } + if err := writeMainSnapshotState(state); err != nil { + return fmt.Errorf("advance Merlin fallback quarantine state: %w", err) + } + return cleanupOwnedMainSnapshot() +} + +func finalizeOwnedMainSnapshotDelete() error { + if err := removeFileDurable(merlinSnapshotQuarantinePath); err != nil { + return fmt.Errorf("remove quarantined ctrld fallback: %w", err) + } + return cleanupSnapshotPrivateState() +} + +func restoreQuarantinedMainSnapshot(state mainSnapshotState) error { + quarantineExists, err := pathExists(merlinSnapshotQuarantinePath) + if err != nil { + return err + } + if !quarantineExists { + return fmt.Errorf( + "Merlin fallback marked for restoration but quarantine is missing: %s", + merlinSnapshotQuarantinePath, + ) + } + + targetExists, err := pathExists(dnsmasq.MerlinJffsConfPath) + if err != nil { + return err + } + if targetExists { + same, err := sameFilePaths(dnsmasq.MerlinJffsConfPath, merlinSnapshotQuarantinePath) + if err != nil { + return err } - if err := os.Remove(quarantine); err != nil { - cleanupDir = false - return true, fmt.Errorf("restored modified Merlin fallback but could not remove quarantine %s: %w", quarantine, err) + if !same { + return fmt.Errorf( + "cannot restore captured Merlin fallback because %s was recreated; preserved capture at %s", + dnsmasq.MerlinJffsConfPath, merlinSnapshotQuarantinePath, + ) } - if err := os.Remove(quarantineDir); err != nil { - cleanupDir = false - return true, err + } else { + // Restore with no-clobber hard-link semantics. If another actor wins the + // pathname race, the quarantine remains durably journaled and untouched. + if err := os.Link(merlinSnapshotQuarantinePath, dnsmasq.MerlinJffsConfPath); err != nil { + return fmt.Errorf("restore quarantined Merlin fallback: %w", err) } - cleanupDir = false - if err := syncParentDir(dir); err != nil { - return true, err + if err := syncParentDir(dnsmasq.MerlinJffsConfDir); err != nil { + return fmt.Errorf("sync restored Merlin fallback: %w", err) } - return true, fmt.Errorf("refusing to remove modified Merlin dnsmasq fallback: %s", path) } - if err := os.Remove(quarantine); err != nil { - cleanupDir = false - return true, fmt.Errorf("remove quarantined ctrld fallback %s: %w", quarantine, err) - } - if err := os.Remove(quarantineDir); err != nil { - cleanupDir = false - return true, err + state.phase = snapshotPhaseRestored + if err := writeMainSnapshotState(state); err != nil { + return fmt.Errorf("journal restored Merlin fallback: %w", err) } - cleanupDir = false - if err := syncParentDir(dir); err != nil { - return true, err + return cleanupOwnedMainSnapshot() +} + +func finalizeRestoredMainSnapshot() error { + // Restoration was durably recorded only after a public hard link existed. + // Therefore a retry never needs to infer ownership from the current target. + if err := removeFileDurable(merlinSnapshotQuarantinePath); err != nil { + return fmt.Errorf("remove restored Merlin fallback quarantine: %w", err) } - return true, nil + return cleanupSnapshotPrivateState() } // cleanupDnsmasqJffs removes the JFFS configuration file specified in the given dnsmasqConfig, if it exists. From 4226108176fc1b8cc26cac947f8f13d31c7a4f1b Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 11:23:01 +0100 Subject: [PATCH 18/44] Make Merlin snapshot journal parsing testable --- internal/router/merlin/merlin.go | 69 +++++++++++++++++--------------- 1 file changed, 37 insertions(+), 32 deletions(-) diff --git a/internal/router/merlin/merlin.go b/internal/router/merlin/merlin.go index 558b0990..358ec395 100644 --- a/internal/router/merlin/merlin.go +++ b/internal/router/merlin/merlin.go @@ -653,59 +653,64 @@ func merlinSnapshotHash(buf []byte) string { return hex.EncodeToString(sum[:]) } -func writeMainSnapshotState(state mainSnapshotState) error { +func encodeMainSnapshotState(state mainSnapshotState) ([]byte, error) { if state.phase == "" { - return fmt.Errorf("empty Merlin snapshot phase") + return nil, fmt.Errorf("empty Merlin snapshot phase") + } + switch state.phase { + case snapshotPhasePending, snapshotPhasePublished, snapshotPhaseQuarantine, + snapshotPhaseDelete, snapshotPhaseRestore, snapshotPhaseRestored: + default: + return nil, fmt.Errorf("unknown Merlin snapshot phase %q", state.phase) } if len(state.hash) != sha256.Size*2 { - return fmt.Errorf("invalid Merlin snapshot hash length") + return nil, fmt.Errorf("invalid Merlin snapshot hash length") } if _, err := hex.DecodeString(state.hash); err != nil { - return fmt.Errorf("invalid Merlin snapshot hash: %w", err) + return nil, fmt.Errorf("invalid Merlin snapshot hash: %w", err) } - buf := []byte("snapshot-v2\nphase=" + state.phase + "\nsha256=" + state.hash + "\n") - return atomicWriteFile(merlinSnapshotStatePath, buf, 0600) + return []byte("snapshot-v2\nphase=" + state.phase + "\nsha256=" + state.hash + "\n"), nil } -func readMainSnapshotState() (mainSnapshotState, bool, error) { - buf, err := os.ReadFile(merlinSnapshotStatePath) - if os.IsNotExist(err) { - return mainSnapshotState{}, false, nil - } - if err != nil { - return mainSnapshotState{}, false, err - } +func parseMainSnapshotState(buf []byte) (mainSnapshotState, error) { lines := strings.Split(strings.TrimSpace(string(buf)), "\n") - - // No released ctrld version wrote this hash-only format. Reject it rather - // than guessing inode ownership if an intermediate development build left it. if len(lines) == 1 && strings.HasPrefix(lines[0], "sha256=") { - return mainSnapshotState{}, true, fmt.Errorf( - "unsafe hash-only Merlin snapshot state requires manual reconciliation: %s", - merlinSnapshotStatePath, - ) + return mainSnapshotState{}, fmt.Errorf("unsafe hash-only Merlin snapshot state") } if len(lines) != 3 || lines[0] != "snapshot-v2" || !strings.HasPrefix(lines[1], "phase=") || !strings.HasPrefix(lines[2], "sha256=") { - return mainSnapshotState{}, true, fmt.Errorf("invalid Merlin snapshot state") + return mainSnapshotState{}, fmt.Errorf("invalid Merlin snapshot state") } - state := mainSnapshotState{ phase: strings.TrimPrefix(lines[1], "phase="), hash: strings.TrimPrefix(lines[2], "sha256="), } - switch state.phase { - case snapshotPhasePending, snapshotPhasePublished, snapshotPhaseQuarantine, - snapshotPhaseDelete, snapshotPhaseRestore, snapshotPhaseRestored: - default: - return mainSnapshotState{}, true, fmt.Errorf("unknown Merlin snapshot phase %q", state.phase) + if _, err := encodeMainSnapshotState(state); err != nil { + return mainSnapshotState{}, err } - if len(state.hash) != sha256.Size*2 { - return mainSnapshotState{}, true, fmt.Errorf("invalid Merlin snapshot hash length") + return state, nil +} + +func writeMainSnapshotState(state mainSnapshotState) error { + buf, err := encodeMainSnapshotState(state) + if err != nil { + return err } - if _, err := hex.DecodeString(state.hash); err != nil { - return mainSnapshotState{}, true, fmt.Errorf("invalid Merlin snapshot hash: %w", err) + return atomicWriteFile(merlinSnapshotStatePath, buf, 0600) +} + +func readMainSnapshotState() (mainSnapshotState, bool, error) { + buf, err := os.ReadFile(merlinSnapshotStatePath) + if os.IsNotExist(err) { + return mainSnapshotState{}, false, nil + } + if err != nil { + return mainSnapshotState{}, false, err + } + state, err := parseMainSnapshotState(buf) + if err != nil { + return mainSnapshotState{}, true, fmt.Errorf("%w: %s", err, merlinSnapshotStatePath) } return state, true, nil } From 7d76bcf9fc4d4af32bfed45275c9c57cc143fc4e Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 11:23:28 +0100 Subject: [PATCH 19/44] Test Merlin fallback journal and inode identity --- internal/router/merlin/merlin_test.go | 121 ++++++++++++-------------- 1 file changed, 57 insertions(+), 64 deletions(-) diff --git a/internal/router/merlin/merlin_test.go b/internal/router/merlin/merlin_test.go index a623d39e..975b1acb 100644 --- a/internal/router/merlin/merlin_test.go +++ b/internal/router/merlin/merlin_test.go @@ -603,93 +603,86 @@ func Test_cleanupPreparationRevalidatesBeforeWrite(t *testing.T) { -func Test_writeFileNoReplaceRefusesExistingTarget(t *testing.T) { - dir := t.TempDir() - path := filepath.Join(dir, "dnsmasq.conf") - original := []byte("user-owned\n") - if err := os.WriteFile(path, original, 0644); err != nil { - t.Fatal(err) - } - - published, err := writeFileNoReplace(path, []byte("ctrld-owned\n"), 0644) - if err == nil { - t.Fatal("expected no-replace publication to fail when target already exists") - } - if published { - t.Fatal("existing-target conflict must report published=false") +func Test_mainSnapshotStateRoundTrip(t *testing.T) { + want := mainSnapshotState{ + phase: snapshotPhaseQuarantine, + hash: merlinSnapshotHash([]byte("owned snapshot")), } - got, err := os.ReadFile(path) + buf, err := encodeMainSnapshotState(want) if err != nil { t.Fatal(err) } - if !bytes.Equal(got, original) { - t.Fatalf("existing target was modified:\nwant: %q\ngot: %q", original, got) - } -} - -func Test_writeFileNoReplacePublishesAbsentTarget(t *testing.T) { - dir := t.TempDir() - path := filepath.Join(dir, "dnsmasq.conf") - want := []byte("ctrld-owned\n") - - published, err := writeFileNoReplace(path, want, 0644) + got, err := parseMainSnapshotState(buf) if err != nil { t.Fatal(err) } - if !published { - t.Fatal("successful no-replace publication must report published=true") - } - got, err := os.ReadFile(path) - if err != nil { - t.Fatal(err) + if got != want { + t.Fatalf("snapshot state mismatch: want %#v, got %#v", want, got) } - if !bytes.Equal(got, want) { - t.Fatalf("published target mismatch:\nwant: %q\ngot: %q", want, got) +} + +func Test_parseMainSnapshotStateRejectsHashOnlyOwnership(t *testing.T) { + buf := []byte("sha256=" + merlinSnapshotHash([]byte("legacy")) + "\n") + if _, err := parseMainSnapshotState(buf); err == nil { + t.Fatal("expected hash-only ownership state to be rejected") } } +func Test_parseMainSnapshotStateRejectsUnknownPhase(t *testing.T) { + buf := []byte( + "snapshot-v2\nphase=surprise\nsha256=" + + merlinSnapshotHash([]byte("owned")) + "\n", + ) + if _, err := parseMainSnapshotState(buf); err == nil { + t.Fatal("expected unknown snapshot phase to be rejected") + } +} -func Test_quarantineRemoveOwnedFileDeletesCapturedOwnedSnapshot(t *testing.T) { +func Test_sameFilePathsDistinguishesHardLinkFromIdenticalCopy(t *testing.T) { dir := t.TempDir() - path := filepath.Join(dir, "dnsmasq.conf") - content := []byte("ctrld-owned\n") - if err := os.WriteFile(path, content, 0644); err != nil { - t.Fatal(err) - } + original := filepath.Join(dir, "original") + hardLink := filepath.Join(dir, "hard-link") + copyPath := filepath.Join(dir, "copy") + content := []byte("same bytes\n") - removed, err := quarantineRemoveOwnedFile(path, merlinSnapshotHash(content)) - if err != nil { + if err := os.WriteFile(original, content, 0644); err != nil { t.Fatal(err) } - if !removed { - t.Fatal("expected an existing owned snapshot to be captured") + if err := os.Link(original, hardLink); err != nil { + t.Fatal(err) } - if _, err := os.Stat(path); !os.IsNotExist(err) { - t.Fatalf("owned snapshot still exists after cleanup: %v", err) + if err := os.WriteFile(copyPath, content, 0644); err != nil { + t.Fatal(err) } -} -func Test_quarantineRemoveOwnedFileRestoresModifiedSnapshot(t *testing.T) { - dir := t.TempDir() - path := filepath.Join(dir, "dnsmasq.conf") - owned := []byte("ctrld-owned\n") - modified := []byte("user-modified\n") - if err := os.WriteFile(path, modified, 0644); err != nil { + same, err := sameFilePaths(original, hardLink) + if err != nil { t.Fatal(err) } - - removed, err := quarantineRemoveOwnedFile(path, merlinSnapshotHash(owned)) - if err == nil { - t.Fatal("expected modified snapshot cleanup refusal") + if !same { + t.Fatal("hard link to same inode was not recognized") } - if !removed { - t.Fatal("expected modified snapshot to be captured and restored") + same, err = sameFilePaths(original, copyPath) + if err != nil { + t.Fatal(err) } - got, readErr := os.ReadFile(path) - if readErr != nil { - t.Fatal(readErr) + if same { + t.Fatal("byte-identical copy must not be treated as the owned inode") } - if !bytes.Equal(got, modified) { - t.Fatalf("modified snapshot was not restored byte-for-byte:\nwant: %q\ngot: %q", modified, got) +} + +func Test_encodeMainSnapshotStateAcceptsEveryTransactionPhase(t *testing.T) { + hash := merlinSnapshotHash([]byte("owned")) + for _, phase := range []string{ + snapshotPhasePending, + snapshotPhasePublished, + snapshotPhaseQuarantine, + snapshotPhaseDelete, + snapshotPhaseRestore, + snapshotPhaseRestored, + } { + if _, err := encodeMainSnapshotState(mainSnapshotState{phase: phase, hash: hash}); err != nil { + t.Fatalf("phase %q rejected: %v", phase, err) + } } } From 3a78348fa7676055973488dbaa011ecf85a915ee Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 11:30:02 +0100 Subject: [PATCH 20/44] Revalidate and durably finalize Merlin fallback deletion --- internal/router/merlin/merlin.go | 75 ++++++++++++++++++++++++++++---- 1 file changed, 66 insertions(+), 9 deletions(-) diff --git a/internal/router/merlin/merlin.go b/internal/router/merlin/merlin.go index 358ec395..1b15310a 100644 --- a/internal/router/merlin/merlin.go +++ b/internal/router/merlin/merlin.go @@ -776,7 +776,7 @@ func cleanupOwnedMainSnapshot() error { case snapshotPhaseQuarantine: return cleanupQuarantinedMainSnapshot(state) case snapshotPhaseDelete: - return finalizeOwnedMainSnapshotDelete() + return finalizeOwnedMainSnapshotDelete(state) case snapshotPhaseRestore: return restoreQuarantinedMainSnapshot(state) case snapshotPhaseRestored: @@ -867,15 +867,15 @@ func cleanupQuarantinedMainSnapshot(state mainSnapshotState) error { return cleanupOwnedMainSnapshot() } - same, err := sameFilePaths(merlinSnapshotQuarantinePath, merlinSnapshotAnchorPath) + owned, err := snapshotFileStillOwned( + merlinSnapshotQuarantinePath, + merlinSnapshotAnchorPath, + state.hash, + ) if err != nil { return err } - buf, err := os.ReadFile(merlinSnapshotQuarantinePath) - if err != nil { - return fmt.Errorf("read quarantined Merlin fallback: %w", err) - } - if same && merlinSnapshotHash(buf) == state.hash { + if owned { state.phase = snapshotPhaseDelete } else { // Different inode or changed bytes mean user/addon ownership may have @@ -888,7 +888,60 @@ func cleanupQuarantinedMainSnapshot(state mainSnapshotState) error { return cleanupOwnedMainSnapshot() } -func finalizeOwnedMainSnapshotDelete() error { +func snapshotFileStillOwned(path, anchor, expectedHash string) (bool, error) { + pathExistsNow, err := pathExists(path) + if err != nil { + return false, err + } + if !pathExistsNow { + return false, nil + } + anchorExists, err := pathExists(anchor) + if err != nil { + return false, err + } + if !anchorExists { + return false, nil + } + same, err := sameFilePaths(path, anchor) + if err != nil { + return false, err + } + if !same { + return false, nil + } + buf, err := os.ReadFile(path) + if err != nil { + return false, err + } + return merlinSnapshotHash(buf) == expectedHash, nil +} + +func finalizeOwnedMainSnapshotDelete(state mainSnapshotState) error { + quarantineExists, err := pathExists(merlinSnapshotQuarantinePath) + if err != nil { + return err + } + if quarantineExists { + // Revalidate immediately before finalization. A process may have kept a + // writable descriptor open across the public->quarantine rename and + // changed the inode after the earlier classification. + owned, err := snapshotFileStillOwned( + merlinSnapshotQuarantinePath, + merlinSnapshotAnchorPath, + state.hash, + ) + if err != nil { + return err + } + if !owned { + state.phase = snapshotPhaseRestore + if err := writeMainSnapshotState(state); err != nil { + return fmt.Errorf("journal late-modified Merlin fallback restoration: %w", err) + } + return cleanupOwnedMainSnapshot() + } + } if err := removeFileDurable(merlinSnapshotQuarantinePath); err != nil { return fmt.Errorf("remove quarantined ctrld fallback: %w", err) } @@ -1534,7 +1587,11 @@ func pathExists(path string) (bool, error) { func removeFileDurable(path string) error { err := os.Remove(path) if os.IsNotExist(err) { - return nil + // The previous attempt may have unlinked the file successfully but + // failed while syncing the directory. Re-sync even on ENOENT so a retry + // can make that already-visible deletion durable before its journal is + // advanced or removed. + return syncParentDir(filepath.Dir(path)) } if err != nil { return err From cdcf3b321679761764047cd3f8d844a0ed0fbe76 Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 11:32:17 +0100 Subject: [PATCH 21/44] Test final Merlin fallback deletion revalidation --- internal/router/merlin/merlin_test.go | 44 +++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/internal/router/merlin/merlin_test.go b/internal/router/merlin/merlin_test.go index 975b1acb..6efb2bf4 100644 --- a/internal/router/merlin/merlin_test.go +++ b/internal/router/merlin/merlin_test.go @@ -686,3 +686,47 @@ func Test_encodeMainSnapshotStateAcceptsEveryTransactionPhase(t *testing.T) { } } } + + +func Test_snapshotFileStillOwnedDetectsLateInodeModification(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "quarantine") + anchor := filepath.Join(dir, "anchor") + original := []byte("ctrld-owned\n") + if err := os.WriteFile(path, original, 0644); err != nil { + t.Fatal(err) + } + if err := os.Link(path, anchor); err != nil { + t.Fatal(err) + } + expected := merlinSnapshotHash(original) + + owned, err := snapshotFileStillOwned(path, anchor, expected) + if err != nil { + t.Fatal(err) + } + if !owned { + t.Fatal("unchanged hard-linked snapshot should be recognized as owned") + } + + // Simulate a process which kept the inode open across quarantine and wrote + // new contents before final deletion. + if err := os.WriteFile(path, []byte("modified-through-same-inode\n"), 0644); err != nil { + t.Fatal(err) + } + owned, err = snapshotFileStillOwned(path, anchor, expected) + if err != nil { + t.Fatal(err) + } + if owned { + t.Fatal("late modification of the same inode must invalidate ownership for deletion") + } +} + +func Test_removeFileDurableAllowsRetryAfterFileAlreadyGone(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "already-gone") + if err := removeFileDurable(path); err != nil { + t.Fatalf("durability retry for absent file failed: %v", err) + } +} From 59db76003c04fc1dcd8d141c32fcc5d33969cb82 Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 11:33:41 +0100 Subject: [PATCH 22/44] Validate ctrld PID ownership in Merlin DNS hook --- internal/router/dnsmasq/dnsmasq.go | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/internal/router/dnsmasq/dnsmasq.go b/internal/router/dnsmasq/dnsmasq.go index f9341e1f..14b2cd9b 100644 --- a/internal/router/dnsmasq/dnsmasq.go +++ b/internal/router/dnsmasq/dnsmasq.go @@ -48,7 +48,24 @@ config_file="$1" . /usr/sbin/helper.sh pid=$(cat /tmp/ctrld.pid 2>/dev/null) -if [ -n "$pid" ] && [ -f "/proc/${pid}/cmdline" ]; then +ctrld_cmd="" +case "$pid" in + ''|*[!0-9]*) ;; + *) + if [ -r "/proc/${pid}/cmdline" ]; then + ctrld_cmd=$(tr '\\000' '\\n' < "/proc/${pid}/cmdline" 2>/dev/null | sed -n '1p') + fi + ;; +esac +case "$ctrld_cmd" in + ctrld|*/ctrld) + ctrld_running=1 + ;; + *) + ctrld_running=0 + ;; +esac +if [ "$ctrld_running" -eq 1 ]; then pc_delete "servers-file" "$config_file" # no WAN DNS settings pc_append "no-resolv" "$config_file" # do not read /etc/resolv.conf # use ctrld as upstream From 661349e8ee781ac8bafb3779d3d97f2b91691aec Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 11:34:01 +0100 Subject: [PATCH 23/44] Test Merlin DNS hook PID ownership validation --- internal/router/merlin/merlin_test.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/internal/router/merlin/merlin_test.go b/internal/router/merlin/merlin_test.go index 6efb2bf4..3d6c6c54 100644 --- a/internal/router/merlin/merlin_test.go +++ b/internal/router/merlin/merlin_test.go @@ -730,3 +730,22 @@ func Test_removeFileDurableAllowsRetryAfterFileAlreadyGone(t *testing.T) { t.Fatalf("durability retry for absent file failed: %v", err) } } + + +func Test_merlinPostConfValidatesCtrldPidOwnership(t *testing.T) { + for _, want := range []string{ + `case "$pid" in`, + `[ -r "/proc/${pid}/cmdline" ]`, + `tr '\\000' '\\n'`, + `ctrld|*/ctrld)`, + `ctrld_running=1`, + `if [ "$ctrld_running" -eq 1 ]; then`, + } { + if !strings.Contains(dnsmasq.MerlinPostConfTmpl, want) { + t.Fatalf("Merlin postconf is missing ctrld PID ownership check %q", want) + } + } + if strings.Contains(dnsmasq.MerlinPostConfTmpl, `[ -f "/proc/${pid}/cmdline" ]; then`) { + t.Fatal("Merlin postconf must not treat PID existence alone as ctrld ownership") + } +} From 519a5d50ccc83fffc2d112801d0df691091a5da4 Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 11:36:56 +0100 Subject: [PATCH 24/44] Fix BusyBox tr escapes in Merlin DNS hook --- internal/router/dnsmasq/dnsmasq.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/router/dnsmasq/dnsmasq.go b/internal/router/dnsmasq/dnsmasq.go index 14b2cd9b..f153bc27 100644 --- a/internal/router/dnsmasq/dnsmasq.go +++ b/internal/router/dnsmasq/dnsmasq.go @@ -53,7 +53,7 @@ case "$pid" in ''|*[!0-9]*) ;; *) if [ -r "/proc/${pid}/cmdline" ]; then - ctrld_cmd=$(tr '\\000' '\\n' < "/proc/${pid}/cmdline" 2>/dev/null | sed -n '1p') + ctrld_cmd=$(tr '\000' '\n' < "/proc/${pid}/cmdline" 2>/dev/null | sed -n '1p') fi ;; esac From 7f347007ff0e7e7c88aa4830ec00a105de60fd6c Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 11:36:59 +0100 Subject: [PATCH 25/44] Match Merlin DNS hook tr escape test From bd63d89352fc40069a14cd0f147a3edd169769d3 Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 11:37:22 +0100 Subject: [PATCH 26/44] Fix Merlin DNS hook tr expectation --- internal/router/merlin/merlin_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/router/merlin/merlin_test.go b/internal/router/merlin/merlin_test.go index 3d6c6c54..170a187d 100644 --- a/internal/router/merlin/merlin_test.go +++ b/internal/router/merlin/merlin_test.go @@ -736,7 +736,7 @@ func Test_merlinPostConfValidatesCtrldPidOwnership(t *testing.T) { for _, want := range []string{ `case "$pid" in`, `[ -r "/proc/${pid}/cmdline" ]`, - `tr '\\000' '\\n'`, + `tr '\000' '\n'`, `ctrld|*/ctrld)`, `ctrld_running=1`, `if [ "$ctrld_running" -eq 1 ]; then`, From 9348c7a6a15a2020a3b5eea1602b538850186ff6 Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 12:13:01 +0100 Subject: [PATCH 27/44] Remove obsolete Merlin snapshot helpers --- internal/router/merlin/merlin.go | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/internal/router/merlin/merlin.go b/internal/router/merlin/merlin.go index 1b15310a..e4541f10 100644 --- a/internal/router/merlin/merlin.go +++ b/internal/router/merlin/merlin.go @@ -36,10 +36,6 @@ var nvramKvMap = map[string]string{ } // dnsmasqConfig represents configuration paths for dnsmasq operations in Merlin firmware. -type dnsmasqConfig struct { - confPath string - jffsConfPath string -} // Merlin represents a configuration handler for setting up and managing ctrld on Merlin routers. type Merlin struct { @@ -1002,15 +998,6 @@ func finalizeRestoredMainSnapshot() error { return cleanupSnapshotPrivateState() } -// cleanupDnsmasqJffs removes the JFFS configuration file specified in the given dnsmasqConfig, if it exists. -func (m *Merlin) cleanupDnsmasqJffs(cfg *dnsmasqConfig) error { - // Remove cfg.jffsConfPath file. - if err := os.Remove(cfg.jffsConfPath); err != nil && !os.IsNotExist(err) { - return err - } - return nil -} - // merlinHookUpdate is prepared entirely before any shared hook is modified. // This lets us validate/read both Merlin hook paths before the first write. type merlinHookUpdate struct { @@ -1245,19 +1232,6 @@ func restartDNSMasq() error { return nil } -// getDnsmasqConfigs retrieves a list of dnsmasqConfig containing configuration and JFFS paths for dnsmasq operations. -func getDnsmasqConfigs() []*dnsmasqConfig { - cfgs := []*dnsmasqConfig{ - {dnsmasq.MerlinConfPath, dnsmasq.MerlinJffsConfPath}, - } - for _, path := range dnsmasq.AdditionalConfigFiles() { - jffsConfPath := filepath.Join(dnsmasq.MerlinJffsConfDir, filepath.Base(path)) - cfgs = append(cfgs, &dnsmasqConfig{path, jffsConfPath}) - } - - return cfgs -} - // merlinExactLineBounds finds marker only when it occupies a complete line. // The returned end excludes the line ending so callers can decide whether to // preserve or consume that separator. From ce1a6d7f7033ca84b48f78d3f9d4642d40e2eec8 Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 12:13:17 +0100 Subject: [PATCH 28/44] Remove stale Merlin snapshot comment --- internal/router/merlin/merlin.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/router/merlin/merlin.go b/internal/router/merlin/merlin.go index e4541f10..5b9aef3d 100644 --- a/internal/router/merlin/merlin.go +++ b/internal/router/merlin/merlin.go @@ -35,7 +35,6 @@ var nvramKvMap = map[string]string{ "dnspriv_enable": "0", // Ensure Merlin native DoT disabled. } -// dnsmasqConfig represents configuration paths for dnsmasq operations in Merlin firmware. // Merlin represents a configuration handler for setting up and managing ctrld on Merlin routers. type Merlin struct { From d4ece38855a529e0fa48e4ec539941072c9c7058 Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 12:18:48 +0100 Subject: [PATCH 29/44] Re-sync retried Merlin snapshot moves before journaling --- internal/router/merlin/merlin.go | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/internal/router/merlin/merlin.go b/internal/router/merlin/merlin.go index 5b9aef3d..f7eefa20 100644 --- a/internal/router/merlin/merlin.go +++ b/internal/router/merlin/merlin.go @@ -845,9 +845,13 @@ func cleanupQuarantinedMainSnapshot(state mainSnapshotState) error { } return fmt.Errorf("quarantine Merlin fallback: %w", err) } - if err := syncParentDir(dnsmasq.MerlinJffsConfDir); err != nil { - return fmt.Errorf("sync Merlin fallback quarantine: %w", err) - } + } + // Always sync before advancing the journal. On a retry, quarantine may + // already exist because rename succeeded previously while its directory + // fsync failed; observing the pathname is not proof that the rename is + // durable across power loss. + if err := syncParentDir(dnsmasq.MerlinJffsConfDir); err != nil { + return fmt.Errorf("sync Merlin fallback quarantine: %w", err) } anchorExists, err := pathExists(merlinSnapshotAnchorPath) @@ -976,9 +980,12 @@ func restoreQuarantinedMainSnapshot(state mainSnapshotState) error { if err := os.Link(merlinSnapshotQuarantinePath, dnsmasq.MerlinJffsConfPath); err != nil { return fmt.Errorf("restore quarantined Merlin fallback: %w", err) } - if err := syncParentDir(dnsmasq.MerlinJffsConfDir); err != nil { - return fmt.Errorf("sync restored Merlin fallback: %w", err) - } + } + // Re-sync even when the public hard link already exists. It may be the + // result of a previous attempt where link(2) succeeded but directory fsync + // failed, so the pathname alone is not enough to publish restored-v2. + if err := syncParentDir(dnsmasq.MerlinJffsConfDir); err != nil { + return fmt.Errorf("sync restored Merlin fallback: %w", err) } state.phase = snapshotPhaseRestored From 1de6bbe048e19fa89789f740a7ad13de3b79c7aa Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 12:30:34 +0100 Subject: [PATCH 30/44] Make legacy Merlin snapshot cleanup identity-safe --- internal/router/merlin/merlin.go | 295 ++++++++++++++++++++++++++----- 1 file changed, 246 insertions(+), 49 deletions(-) diff --git a/internal/router/merlin/merlin.go b/internal/router/merlin/merlin.go index f7eefa20..1f38cc60 100644 --- a/internal/router/merlin/merlin.go +++ b/internal/router/merlin/merlin.go @@ -182,9 +182,19 @@ func (m *Merlin) Setup() (retErr error) { return nil } +const ( + legacyCleanupPhase = "cleanup-v2" + legacyFinalizePhase = "finalize-v2" + + legacyEntryPending = "pending" + legacyEntryDelete = "delete" + legacyEntryRestore = "restore" +) + type legacySnapshotEntry struct { - path string - hash string + state string + path string + hash string } type legacyCleanupJournal struct { @@ -237,41 +247,8 @@ func (m *Merlin) Cleanup() error { } if legacy { - switch journal.phase { - case "cleanup-v1": - // The exact snapshot paths and hashes were durably recorded before - // the first destructive operation. Retries consult only this journal, - // never a fresh directory enumeration, so a user-created replacement - // cannot be mistaken for ctrld's old snapshot. - for _, entry := range journal.entries { - buf, err := os.ReadFile(entry.path) - switch { - case err == nil: - if merlinSnapshotHash(buf) != entry.hash { - continue - } - if err := os.Remove(entry.path); err != nil && !os.IsNotExist(err) { - return fmt.Errorf("remove journaled legacy snapshot %s: %w", entry.path, err) - } - case os.IsNotExist(err): - // Already removed on a previous attempt. - default: - return fmt.Errorf("read journaled legacy snapshot %s: %w", entry.path, err) - } - } - if err := syncParentDir(dnsmasq.MerlinJffsConfDir); err != nil { - return fmt.Errorf("sync legacy snapshot directory: %w", err) - } - journal.phase = "finalize" - journal.entries = nil - if err := writeLegacyCleanupJournal(journal); err != nil { - return fmt.Errorf("advance Merlin legacy cleanup state: %w", err) - } - case "finalize": - // Destructive legacy cleanup already completed. A retry must never - // reinterpret newly created user files as legacy ctrld snapshots. - default: - return fmt.Errorf("unknown Merlin legacy cleanup phase %q", journal.phase) + if err := cleanupLegacySnapshots(&journal); err != nil { + return err } } else { if err := cleanupOwnedMainSnapshot(); err != nil { @@ -309,7 +286,7 @@ func buildLegacyCleanupJournal() (legacyCleanupJournal, error) { } paths = append(paths, matches...) - journal := legacyCleanupJournal{phase: "cleanup-v1"} + journal := legacyCleanupJournal{phase: legacyCleanupPhase} for _, path := range paths { if !isLegacySnapshotPath(path) { continue @@ -322,8 +299,9 @@ func buildLegacyCleanupJournal() (legacyCleanupJournal, error) { return legacyCleanupJournal{}, fmt.Errorf("read legacy dnsmasq snapshot %s: %w", path, err) } journal.entries = append(journal.entries, legacySnapshotEntry{ - path: path, - hash: merlinSnapshotHash(buf), + state: legacyEntryPending, + path: path, + hash: merlinSnapshotHash(buf), }) } return journal, nil @@ -341,13 +319,29 @@ func encodeLegacyCleanupJournal(journal legacyCleanupJournal) ([]byte, error) { var b strings.Builder b.WriteString(journal.phase) b.WriteByte('\n') + if journal.phase == legacyFinalizePhase { + if len(journal.entries) != 0 { + return nil, fmt.Errorf("finalize journal contains snapshot entries") + } + return []byte(b.String()), nil + } + if journal.phase != legacyCleanupPhase { + return nil, fmt.Errorf("unknown legacy cleanup phase %q", journal.phase) + } for _, entry := range journal.entries { if !isLegacySnapshotPath(entry.path) { return nil, fmt.Errorf("invalid legacy snapshot path %q", entry.path) } + switch entry.state { + case legacyEntryPending, legacyEntryDelete, legacyEntryRestore: + default: + return nil, fmt.Errorf("invalid legacy snapshot state %q for %s", entry.state, entry.path) + } if _, err := hex.DecodeString(entry.hash); err != nil || len(entry.hash) != sha256.Size*2 { return nil, fmt.Errorf("invalid legacy snapshot hash for %s", entry.path) } + b.WriteString(entry.state) + b.WriteByte('\t') b.WriteString(entry.path) b.WriteByte('\t') b.WriteString(entry.hash) @@ -373,34 +367,237 @@ func parseLegacyCleanupJournal(buf []byte) (legacyCleanupJournal, error) { return legacyCleanupJournal{}, fmt.Errorf("empty legacy cleanup journal") } journal := legacyCleanupJournal{phase: lines[0]} - if journal.phase == "finalize" { + if journal.phase == legacyFinalizePhase { if len(lines) != 1 { return legacyCleanupJournal{}, fmt.Errorf("finalize journal contains snapshot entries") } return journal, nil } - if journal.phase != "cleanup-v1" { + if journal.phase != legacyCleanupPhase { return legacyCleanupJournal{}, fmt.Errorf("unknown legacy cleanup phase %q", journal.phase) } for _, line := range lines[1:] { if line == "" { continue } - parts := strings.SplitN(line, "\t", 2) - if len(parts) != 2 || !isLegacySnapshotPath(parts[0]) { + parts := strings.SplitN(line, "\t", 3) + if len(parts) != 3 || !isLegacySnapshotPath(parts[1]) { return legacyCleanupJournal{}, fmt.Errorf("invalid legacy cleanup journal entry %q", line) } - if len(parts[1]) != sha256.Size*2 { - return legacyCleanupJournal{}, fmt.Errorf("invalid legacy snapshot hash for %s", parts[0]) + switch parts[0] { + case legacyEntryPending, legacyEntryDelete, legacyEntryRestore: + default: + return legacyCleanupJournal{}, fmt.Errorf("invalid legacy snapshot state %q", parts[0]) + } + if len(parts[2]) != sha256.Size*2 { + return legacyCleanupJournal{}, fmt.Errorf("invalid legacy snapshot hash for %s", parts[1]) } - if _, err := hex.DecodeString(parts[1]); err != nil { - return legacyCleanupJournal{}, fmt.Errorf("invalid legacy snapshot hash for %s: %w", parts[0], err) + if _, err := hex.DecodeString(parts[2]); err != nil { + return legacyCleanupJournal{}, fmt.Errorf("invalid legacy snapshot hash for %s: %w", parts[1], err) } - journal.entries = append(journal.entries, legacySnapshotEntry{path: parts[0], hash: parts[1]}) + journal.entries = append(journal.entries, legacySnapshotEntry{state: parts[0], path: parts[1], hash: parts[2]}) } return journal, nil } +func legacySnapshotQuarantinePath(path string) string { + return filepath.Join( + dnsmasq.MerlinJffsConfDir, + "."+filepath.Base(path)+".ctrld-legacy-quarantine", + ) +} + +func removeLegacyJournalEntry(journal *legacyCleanupJournal, index int) error { + journal.entries = append(journal.entries[:index], journal.entries[index+1:]...) + return writeLegacyCleanupJournal(*journal) +} + +func cleanupLegacySnapshots(journal *legacyCleanupJournal) error { + switch journal.phase { + case legacyFinalizePhase: + return nil + case legacyCleanupPhase: + default: + return fmt.Errorf("unknown Merlin legacy cleanup phase %q", journal.phase) + } + + for len(journal.entries) > 0 { + entry := journal.entries[0] + var err error + switch entry.state { + case legacyEntryPending: + err = cleanupPendingLegacySnapshot(journal, 0) + case legacyEntryDelete: + err = finalizeLegacySnapshotDelete(journal, 0) + case legacyEntryRestore: + err = restoreLegacySnapshot(journal, 0) + default: + err = fmt.Errorf("unknown legacy snapshot state %q", entry.state) + } + if err != nil { + return err + } + } + + journal.phase = legacyFinalizePhase + if err := writeLegacyCleanupJournal(*journal); err != nil { + return fmt.Errorf("advance Merlin legacy cleanup state: %w", err) + } + return nil +} + +// cleanupPendingLegacySnapshot captures the exact pathname into a private +// quarantine before deletion. The pre-rename FileInfo is compared with the +// quarantined inode, so an atomic replacement between validation and rename is +// detected even when the replacement has byte-identical contents. +// +// If a retry finds a quarantine while the durable entry is still "pending", the +// previous attempt may have crashed after rename but before proving identity. +// In that ambiguous state we restore rather than delete. +func cleanupPendingLegacySnapshot(journal *legacyCleanupJournal, index int) error { + entry := journal.entries[index] + qpath := legacySnapshotQuarantinePath(entry.path) + + qExists, err := pathExists(qpath) + if err != nil { + return err + } + if qExists { + journal.entries[index].state = legacyEntryRestore + if err := writeLegacyCleanupJournal(*journal); err != nil { + return fmt.Errorf("journal ambiguous legacy snapshot restoration: %w", err) + } + return restoreLegacySnapshot(journal, index) + } + + f, err := os.Open(entry.path) + if os.IsNotExist(err) { + return removeLegacyJournalEntry(journal, index) + } + if err != nil { + return fmt.Errorf("open journaled legacy snapshot %s: %w", entry.path, err) + } + info, err := f.Stat() + if err != nil { + _ = f.Close() + return fmt.Errorf("stat journaled legacy snapshot %s: %w", entry.path, err) + } + buf, err := io.ReadAll(f) + closeErr := f.Close() + if err != nil { + return fmt.Errorf("read journaled legacy snapshot %s: %w", entry.path, err) + } + if closeErr != nil { + return fmt.Errorf("close journaled legacy snapshot %s: %w", entry.path, closeErr) + } + if merlinSnapshotHash(buf) != entry.hash { + // The path changed before ctrld touched it. It is no longer attributable + // to the old ctrld snapshot, so leave it untouched forever. + return removeLegacyJournalEntry(journal, index) + } + + if err := os.Rename(entry.path, qpath); err != nil { + if os.IsNotExist(err) { + return removeLegacyJournalEntry(journal, index) + } + return fmt.Errorf("quarantine legacy snapshot %s: %w", entry.path, err) + } + if err := syncParentDir(dnsmasq.MerlinJffsConfDir); err != nil { + return fmt.Errorf("sync legacy snapshot quarantine: %w", err) + } + + qInfo, err := os.Stat(qpath) + if err != nil { + return fmt.Errorf("stat quarantined legacy snapshot %s: %w", qpath, err) + } + qBuf, err := os.ReadFile(qpath) + if err != nil { + return fmt.Errorf("read quarantined legacy snapshot %s: %w", qpath, err) + } + if !os.SameFile(info, qInfo) || merlinSnapshotHash(qBuf) != entry.hash { + journal.entries[index].state = legacyEntryRestore + if err := writeLegacyCleanupJournal(*journal); err != nil { + return fmt.Errorf("journal replaced legacy snapshot restoration: %w", err) + } + return restoreLegacySnapshot(journal, index) + } + + journal.entries[index].state = legacyEntryDelete + if err := writeLegacyCleanupJournal(*journal); err != nil { + return fmt.Errorf("journal legacy snapshot deletion: %w", err) + } + return finalizeLegacySnapshotDelete(journal, index) +} + +func finalizeLegacySnapshotDelete(journal *legacyCleanupJournal, index int) error { + entry := journal.entries[index] + qpath := legacySnapshotQuarantinePath(entry.path) + qExists, err := pathExists(qpath) + if err != nil { + return err + } + if qExists { + buf, err := os.ReadFile(qpath) + if err != nil { + return fmt.Errorf("read quarantined legacy snapshot %s: %w", qpath, err) + } + if merlinSnapshotHash(buf) != entry.hash { + journal.entries[index].state = legacyEntryRestore + if err := writeLegacyCleanupJournal(*journal); err != nil { + return fmt.Errorf("journal modified legacy snapshot restoration: %w", err) + } + return restoreLegacySnapshot(journal, index) + } + if err := removeFileDurable(qpath); err != nil { + return fmt.Errorf("remove quarantined legacy snapshot %s: %w", qpath, err) + } + } + return removeLegacyJournalEntry(journal, index) +} + +func restoreLegacySnapshot(journal *legacyCleanupJournal, index int) error { + entry := journal.entries[index] + qpath := legacySnapshotQuarantinePath(entry.path) + qExists, err := pathExists(qpath) + if err != nil { + return err + } + if !qExists { + // Either restoration/removal completed before a journal update failed or + // no destructive operation ever occurred. Never infer ownership from the + // public path on retry. + return removeLegacyJournalEntry(journal, index) + } + + targetExists, err := pathExists(entry.path) + if err != nil { + return err + } + if targetExists { + same, err := sameFilePaths(entry.path, qpath) + if err != nil { + return err + } + if !same { + return fmt.Errorf( + "cannot restore captured legacy snapshot because %s was recreated; preserved capture at %s", + entry.path, qpath, + ) + } + } else { + if err := os.Link(qpath, entry.path); err != nil { + return fmt.Errorf("restore captured legacy snapshot %s: %w", entry.path, err) + } + } + if err := syncParentDir(dnsmasq.MerlinJffsConfDir); err != nil { + return fmt.Errorf("sync restored legacy snapshot: %w", err) + } + if err := removeFileDurable(qpath); err != nil { + return fmt.Errorf("remove restored legacy snapshot quarantine: %w", err) + } + return removeLegacyJournalEntry(journal, index) +} + func isLegacySnapshotPath(path string) bool { if path == dnsmasq.MerlinJffsConfPath { return true From 68c3b806afbe651de32042ff748f68c5ff40ea42 Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 12:30:44 +0100 Subject: [PATCH 31/44] Import io for legacy snapshot capture --- internal/router/merlin/merlin.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/router/merlin/merlin.go b/internal/router/merlin/merlin.go index 1f38cc60..37eb6e1a 100644 --- a/internal/router/merlin/merlin.go +++ b/internal/router/merlin/merlin.go @@ -5,6 +5,7 @@ import ( "crypto/sha256" "encoding/hex" "fmt" + "io" "os" "os/exec" "path/filepath" From e104eb4e4c20ec0391c95556b40dcf3e3e07106a Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 12:31:05 +0100 Subject: [PATCH 32/44] Update legacy cleanup journal tests --- internal/router/merlin/merlin_test.go | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/internal/router/merlin/merlin_test.go b/internal/router/merlin/merlin_test.go index 170a187d..2ea56215 100644 --- a/internal/router/merlin/merlin_test.go +++ b/internal/router/merlin/merlin_test.go @@ -519,11 +519,11 @@ func Test_dnsmasqConfigUsesCtrld(t *testing.T) { func Test_legacyCleanupJournalRoundTrip(t *testing.T) { entries := []legacySnapshotEntry{ - {path: dnsmasq.MerlinJffsConfPath, hash: merlinSnapshotHash([]byte("main"))}, - {path: filepath.Join(dnsmasq.MerlinJffsConfDir, "dnsmasq-1.conf"), hash: merlinSnapshotHash([]byte("sdn1"))}, - {path: filepath.Join(dnsmasq.MerlinJffsConfDir, "dnsmasq-3.conf"), hash: merlinSnapshotHash([]byte("sdn3"))}, + {state: legacyEntryPending, path: dnsmasq.MerlinJffsConfPath, hash: merlinSnapshotHash([]byte("main"))}, + {state: legacyEntryDelete, path: filepath.Join(dnsmasq.MerlinJffsConfDir, "dnsmasq-1.conf"), hash: merlinSnapshotHash([]byte("sdn1"))}, + {state: legacyEntryRestore, path: filepath.Join(dnsmasq.MerlinJffsConfDir, "dnsmasq-3.conf"), hash: merlinSnapshotHash([]byte("sdn3"))}, } - want := legacyCleanupJournal{phase: "cleanup-v1", entries: entries} + want := legacyCleanupJournal{phase: legacyCleanupPhase, entries: entries} buf, err := encodeLegacyCleanupJournal(want) if err != nil { t.Fatal(err) @@ -544,23 +544,31 @@ func Test_legacyCleanupJournalRoundTrip(t *testing.T) { func Test_parseLegacyCleanupJournalRejectsUnsafePath(t *testing.T) { hash := merlinSnapshotHash([]byte("x")) - buf := []byte("cleanup-v1\n/jffs/configs/profile.add\t" + hash + "\n") + buf := []byte(legacyCleanupPhase + "\n" + legacyEntryPending + "\t/jffs/configs/profile.add\t" + hash + "\n") if _, err := parseLegacyCleanupJournal(buf); err == nil { t.Fatal("expected unsafe legacy snapshot path to be rejected") } } func Test_parseLegacyCleanupJournalFinalizeHasNoEntries(t *testing.T) { - if _, err := parseLegacyCleanupJournal([]byte("finalize\n")); err != nil { + if _, err := parseLegacyCleanupJournal([]byte(legacyFinalizePhase + "\n")); err != nil { t.Fatalf("valid finalize journal rejected: %v", err) } hash := merlinSnapshotHash([]byte("x")) - buf := []byte("finalize\n" + dnsmasq.MerlinJffsConfPath + "\t" + hash + "\n") + buf := []byte(legacyFinalizePhase + "\n" + legacyEntryPending + "\t" + dnsmasq.MerlinJffsConfPath + "\t" + hash + "\n") if _, err := parseLegacyCleanupJournal(buf); err == nil { t.Fatal("expected finalize journal with entries to be rejected") } } +func Test_parseLegacyCleanupJournalRejectsUnknownEntryState(t *testing.T) { + hash := merlinSnapshotHash([]byte("x")) + buf := []byte(legacyCleanupPhase + "\nsurprise\t" + dnsmasq.MerlinJffsConfPath + "\t" + hash + "\n") + if _, err := parseLegacyCleanupJournal(buf); err == nil { + t.Fatal("expected unknown legacy entry state to be rejected") + } +} + func Test_isLegacySnapshotPath(t *testing.T) { tests := []struct { path string From 9dc944c79e66137f4569ff060c1f764fb2fdaa3c Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 12:31:50 +0100 Subject: [PATCH 33/44] Reject unjournaled legacy snapshot quarantines --- internal/router/merlin/merlin.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/internal/router/merlin/merlin.go b/internal/router/merlin/merlin.go index 37eb6e1a..050d9b61 100644 --- a/internal/router/merlin/merlin.go +++ b/internal/router/merlin/merlin.go @@ -280,6 +280,22 @@ func (m *Merlin) Cleanup() error { } func buildLegacyCleanupJournal() (legacyCleanupJournal, error) { + // A quarantine is meaningful only together with a durable journal. Never + // adopt an unjournaled private-looking pathname as ctrld-owned data. + orphans, err := filepath.Glob(filepath.Join( + dnsmasq.MerlinJffsConfDir, + ".dnsmasq*.ctrld-legacy-quarantine", + )) + if err != nil { + return legacyCleanupJournal{}, err + } + if len(orphans) != 0 { + return legacyCleanupJournal{}, fmt.Errorf( + "unjournaled Merlin legacy quarantine requires manual reconciliation: %s", + strings.Join(orphans, ", "), + ) + } + paths := []string{dnsmasq.MerlinJffsConfPath} matches, err := filepath.Glob(filepath.Join(dnsmasq.MerlinJffsConfDir, "dnsmasq-*.conf")) if err != nil { From dc7000af7bf9e01b353c5835f128d3c1bbe27e16 Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 14:52:20 +0100 Subject: [PATCH 34/44] Capture legacy snapshot inode identity before journaling --- internal/router/merlin/merlin.go | 100 +++++++++++++++++++++++++++++-- 1 file changed, 96 insertions(+), 4 deletions(-) diff --git a/internal/router/merlin/merlin.go b/internal/router/merlin/merlin.go index 050d9b61..2c3d7a5f 100644 --- a/internal/router/merlin/merlin.go +++ b/internal/router/merlin/merlin.go @@ -279,7 +279,21 @@ func (m *Merlin) Cleanup() error { return nil } -func buildLegacyCleanupJournal() (legacyCleanupJournal, error) { +func legacySnapshotAnchorPath(path string) string { + return filepath.Join( + "/jffs/controld", + "."+filepath.Base(path)+".ctrld-legacy-anchor", + ) +} + +func removeLegacySnapshotAnchor(path string) error { + if err := removeFileDurable(legacySnapshotAnchorPath(path)); err != nil { + return fmt.Errorf("remove legacy snapshot anchor for %s: %w", path, err) + } + return nil +} + +func buildLegacyCleanupJournal() (journal legacyCleanupJournal, retErr error) { // A quarantine is meaningful only together with a durable journal. Never // adopt an unjournaled private-looking pathname as ctrld-owned data. orphans, err := filepath.Glob(filepath.Join( @@ -296,6 +310,19 @@ func buildLegacyCleanupJournal() (legacyCleanupJournal, error) { ) } + // With no durable journal present, any ctrld-private legacy anchors can only + // be leftovers from a crash before journal publication. Removing an anchor + // drops only ctrld's extra hard link; it never touches the public snapshot. + anchorOrphans, err := filepath.Glob("/jffs/controld/.dnsmasq*.ctrld-legacy-anchor") + if err != nil { + return legacyCleanupJournal{}, err + } + for _, anchor := range anchorOrphans { + if err := removeFileDurable(anchor); err != nil { + return legacyCleanupJournal{}, fmt.Errorf("remove orphaned legacy snapshot anchor %s: %w", anchor, err) + } + } + paths := []string{dnsmasq.MerlinJffsConfPath} matches, err := filepath.Glob(filepath.Join(dnsmasq.MerlinJffsConfDir, "dnsmasq-*.conf")) if err != nil { @@ -303,22 +330,87 @@ func buildLegacyCleanupJournal() (legacyCleanupJournal, error) { } paths = append(paths, matches...) - journal := legacyCleanupJournal{phase: legacyCleanupPhase} + journal = legacyCleanupJournal{phase: legacyCleanupPhase} + captured := make([]string, 0, len(paths)) + defer func() { + if retErr == nil { + return + } + for _, path := range captured { + _ = removeLegacySnapshotAnchor(path) + } + }() + for _, path := range paths { if !isLegacySnapshotPath(path) { continue } - buf, err := os.ReadFile(path) + + f, err := os.Open(path) if os.IsNotExist(err) { continue } + if err != nil { + return legacyCleanupJournal{}, fmt.Errorf("open legacy dnsmasq snapshot %s: %w", path, err) + } + info, err := f.Stat() + if err != nil { + _ = f.Close() + return legacyCleanupJournal{}, fmt.Errorf("stat legacy dnsmasq snapshot %s: %w", path, err) + } + buf, err := io.ReadAll(f) + closeErr := f.Close() if err != nil { return legacyCleanupJournal{}, fmt.Errorf("read legacy dnsmasq snapshot %s: %w", path, err) } + if closeErr != nil { + return legacyCleanupJournal{}, fmt.Errorf("close legacy dnsmasq snapshot %s: %w", path, closeErr) + } + hash := merlinSnapshotHash(buf) + + anchor := legacySnapshotAnchorPath(path) + if err := os.Link(path, anchor); err != nil { + if os.IsNotExist(err) { + continue + } + return legacyCleanupJournal{}, fmt.Errorf("capture legacy snapshot identity %s: %w", path, err) + } + captured = append(captured, path) + if err := syncParentDir(filepath.Dir(anchor)); err != nil { + return legacyCleanupJournal{}, fmt.Errorf("sync legacy snapshot anchor %s: %w", anchor, err) + } + + anchorInfo, err := os.Stat(anchor) + if err != nil { + return legacyCleanupJournal{}, fmt.Errorf("stat legacy snapshot anchor %s: %w", anchor, err) + } + if !os.SameFile(info, anchorInfo) { + // The public pathname was replaced between Open and Link. Drop only + // our private link and never claim the replacement. + if err := removeLegacySnapshotAnchor(path); err != nil { + return legacyCleanupJournal{}, err + } + captured = captured[:len(captured)-1] + continue + } + anchorBuf, err := os.ReadFile(anchor) + if err != nil { + return legacyCleanupJournal{}, fmt.Errorf("read legacy snapshot anchor %s: %w", anchor, err) + } + if merlinSnapshotHash(anchorBuf) != hash { + // Same inode was modified while identity was being captured. Treat it + // as no longer safely attributable to ctrld. + if err := removeLegacySnapshotAnchor(path); err != nil { + return legacyCleanupJournal{}, err + } + captured = captured[:len(captured)-1] + continue + } + journal.entries = append(journal.entries, legacySnapshotEntry{ state: legacyEntryPending, path: path, - hash: merlinSnapshotHash(buf), + hash: hash, }) } return journal, nil From b26485f164fa28641adca8696f2a55e0491bc86a Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 14:53:28 +0100 Subject: [PATCH 35/44] Bind legacy cleanup to captured inode anchors --- internal/router/merlin/merlin.go | 106 +++++++++++++++++++++---------- 1 file changed, 74 insertions(+), 32 deletions(-) diff --git a/internal/router/merlin/merlin.go b/internal/router/merlin/merlin.go index 2c3d7a5f..9db63c32 100644 --- a/internal/router/merlin/merlin.go +++ b/internal/router/merlin/merlin.go @@ -237,6 +237,9 @@ func (m *Merlin) Cleanup() error { return err } if err := writeLegacyCleanupJournal(journal); err != nil { + if cleanupErr := cleanupLegacyCaptureAnchors(journal); cleanupErr != nil { + return fmt.Errorf("journal legacy dnsmasq snapshots: %w; cleanup captured anchors: %v", err, cleanupErr) + } return fmt.Errorf("journal legacy dnsmasq snapshots: %w", err) } } @@ -293,6 +296,15 @@ func removeLegacySnapshotAnchor(path string) error { return nil } +func cleanupLegacyCaptureAnchors(journal legacyCleanupJournal) error { + for _, entry := range journal.entries { + if err := removeLegacySnapshotAnchor(entry.path); err != nil { + return err + } + } + return nil +} + func buildLegacyCleanupJournal() (journal legacyCleanupJournal, retErr error) { // A quarantine is meaningful only together with a durable journal. Never // adopt an unjournaled private-looking pathname as ctrld-owned data. @@ -566,12 +578,34 @@ func cleanupLegacySnapshots(journal *legacyCleanupJournal) error { func cleanupPendingLegacySnapshot(journal *legacyCleanupJournal, index int) error { entry := journal.entries[index] qpath := legacySnapshotQuarantinePath(entry.path) + anchor := legacySnapshotAnchorPath(entry.path) + + anchorExists, err := pathExists(anchor) + if err != nil { + return err + } + if !anchorExists { + // The durable journal no longer has its original inode proof. Never infer + // ownership from a public pathname or hash alone. + return removeLegacyJournalEntry(journal, index) + } qExists, err := pathExists(qpath) if err != nil { return err } if qExists { + owned, err := snapshotFileStillOwned(qpath, anchor, entry.hash) + if err != nil { + return err + } + if owned { + journal.entries[index].state = legacyEntryDelete + if err := writeLegacyCleanupJournal(*journal); err != nil { + return fmt.Errorf("journal quarantined legacy snapshot deletion: %w", err) + } + return finalizeLegacySnapshotDelete(journal, index) + } journal.entries[index].state = legacyEntryRestore if err := writeLegacyCleanupJournal(*journal); err != nil { return fmt.Errorf("journal ambiguous legacy snapshot restoration: %w", err) @@ -579,35 +613,33 @@ func cleanupPendingLegacySnapshot(journal *legacyCleanupJournal, index int) erro return restoreLegacySnapshot(journal, index) } - f, err := os.Open(entry.path) - if os.IsNotExist(err) { - return removeLegacyJournalEntry(journal, index) - } + targetExists, err := pathExists(entry.path) if err != nil { - return fmt.Errorf("open journaled legacy snapshot %s: %w", entry.path, err) + return err } - info, err := f.Stat() - if err != nil { - _ = f.Close() - return fmt.Errorf("stat journaled legacy snapshot %s: %w", entry.path, err) + if !targetExists { + if err := removeLegacySnapshotAnchor(entry.path); err != nil { + return err + } + return removeLegacyJournalEntry(journal, index) } - buf, err := io.ReadAll(f) - closeErr := f.Close() + + owned, err := snapshotFileStillOwned(entry.path, anchor, entry.hash) if err != nil { - return fmt.Errorf("read journaled legacy snapshot %s: %w", entry.path, err) - } - if closeErr != nil { - return fmt.Errorf("close journaled legacy snapshot %s: %w", entry.path, closeErr) + return err } - if merlinSnapshotHash(buf) != entry.hash { - // The path changed before ctrld touched it. It is no longer attributable - // to the old ctrld snapshot, so leave it untouched forever. + if !owned { + // The public pathname was replaced or modified after journal creation. + // Drop only ctrld's private anchor and leave the public file untouched. + if err := removeLegacySnapshotAnchor(entry.path); err != nil { + return err + } return removeLegacyJournalEntry(journal, index) } if err := os.Rename(entry.path, qpath); err != nil { if os.IsNotExist(err) { - return removeLegacyJournalEntry(journal, index) + return cleanupPendingLegacySnapshot(journal, index) } return fmt.Errorf("quarantine legacy snapshot %s: %w", entry.path, err) } @@ -615,15 +647,14 @@ func cleanupPendingLegacySnapshot(journal *legacyCleanupJournal, index int) erro return fmt.Errorf("sync legacy snapshot quarantine: %w", err) } - qInfo, err := os.Stat(qpath) + owned, err = snapshotFileStillOwned(qpath, anchor, entry.hash) if err != nil { - return fmt.Errorf("stat quarantined legacy snapshot %s: %w", qpath, err) - } - qBuf, err := os.ReadFile(qpath) - if err != nil { - return fmt.Errorf("read quarantined legacy snapshot %s: %w", qpath, err) + return err } - if !os.SameFile(info, qInfo) || merlinSnapshotHash(qBuf) != entry.hash { + if !owned { + // A pathname replacement won the race between the pre-rename identity + // check and rename(2). Restore the captured replacement; the original + // ctrld inode remains proven by the private anchor only. journal.entries[index].state = legacyEntryRestore if err := writeLegacyCleanupJournal(*journal); err != nil { return fmt.Errorf("journal replaced legacy snapshot restoration: %w", err) @@ -641,16 +672,18 @@ func cleanupPendingLegacySnapshot(journal *legacyCleanupJournal, index int) erro func finalizeLegacySnapshotDelete(journal *legacyCleanupJournal, index int) error { entry := journal.entries[index] qpath := legacySnapshotQuarantinePath(entry.path) + anchor := legacySnapshotAnchorPath(entry.path) + qExists, err := pathExists(qpath) if err != nil { return err } if qExists { - buf, err := os.ReadFile(qpath) + owned, err := snapshotFileStillOwned(qpath, anchor, entry.hash) if err != nil { - return fmt.Errorf("read quarantined legacy snapshot %s: %w", qpath, err) + return err } - if merlinSnapshotHash(buf) != entry.hash { + if !owned { journal.entries[index].state = legacyEntryRestore if err := writeLegacyCleanupJournal(*journal); err != nil { return fmt.Errorf("journal modified legacy snapshot restoration: %w", err) @@ -661,20 +694,26 @@ func finalizeLegacySnapshotDelete(journal *legacyCleanupJournal, index int) erro return fmt.Errorf("remove quarantined legacy snapshot %s: %w", qpath, err) } } + if err := removeLegacySnapshotAnchor(entry.path); err != nil { + return err + } return removeLegacyJournalEntry(journal, index) } func restoreLegacySnapshot(journal *legacyCleanupJournal, index int) error { entry := journal.entries[index] qpath := legacySnapshotQuarantinePath(entry.path) + qExists, err := pathExists(qpath) if err != nil { return err } if !qExists { - // Either restoration/removal completed before a journal update failed or - // no destructive operation ever occurred. Never infer ownership from the - // public path on retry. + // Restoration/removal completed before the journal update, or no + // destructive operation occurred. Never infer ownership from public state. + if err := removeLegacySnapshotAnchor(entry.path); err != nil { + return err + } return removeLegacyJournalEntry(journal, index) } @@ -704,6 +743,9 @@ func restoreLegacySnapshot(journal *legacyCleanupJournal, index int) error { if err := removeFileDurable(qpath); err != nil { return fmt.Errorf("remove restored legacy snapshot quarantine: %w", err) } + if err := removeLegacySnapshotAnchor(entry.path); err != nil { + return err + } return removeLegacyJournalEntry(journal, index) } From c56bf7b9f9d31406131bfcfefeba95e608732c43 Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 14:54:29 +0100 Subject: [PATCH 36/44] Restore anchorless legacy quarantines safely --- internal/router/merlin/merlin.go | 40 +++++++++++++++++++------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/internal/router/merlin/merlin.go b/internal/router/merlin/merlin.go index 9db63c32..2f46f525 100644 --- a/internal/router/merlin/merlin.go +++ b/internal/router/merlin/merlin.go @@ -567,34 +567,36 @@ func cleanupLegacySnapshots(journal *legacyCleanupJournal) error { return nil } -// cleanupPendingLegacySnapshot captures the exact pathname into a private -// quarantine before deletion. The pre-rename FileInfo is compared with the -// quarantined inode, so an atomic replacement between validation and rename is -// detected even when the replacement has byte-identical contents. -// -// If a retry finds a quarantine while the durable entry is still "pending", the -// previous attempt may have crashed after rename but before proving identity. -// In that ambiguous state we restore rather than delete. +// cleanupPendingLegacySnapshot uses the private hard-link captured before +// journal publication as the durable identity proof for the original legacy +// snapshot. Public-path hashes alone are never treated as ownership. A rename +// race is detected by comparing the quarantined inode with that anchor; any +// ambiguous or unproven quarantined data is restored rather than deleted. func cleanupPendingLegacySnapshot(journal *legacyCleanupJournal, index int) error { entry := journal.entries[index] qpath := legacySnapshotQuarantinePath(entry.path) anchor := legacySnapshotAnchorPath(entry.path) - anchorExists, err := pathExists(anchor) + qExists, err := pathExists(qpath) if err != nil { return err } - if !anchorExists { - // The durable journal no longer has its original inode proof. Never infer - // ownership from a public pathname or hash alone. - return removeLegacyJournalEntry(journal, index) - } - - qExists, err := pathExists(qpath) + anchorExists, err := pathExists(anchor) if err != nil { return err } + if qExists { + if !anchorExists { + // A prior attempt already moved data out of the public pathname but + // the private identity proof is gone. Preservation wins: restore the + // quarantined data rather than orphaning it. + journal.entries[index].state = legacyEntryRestore + if err := writeLegacyCleanupJournal(*journal); err != nil { + return fmt.Errorf("journal anchorless legacy snapshot restoration: %w", err) + } + return restoreLegacySnapshot(journal, index) + } owned, err := snapshotFileStillOwned(qpath, anchor, entry.hash) if err != nil { return err @@ -613,6 +615,12 @@ func cleanupPendingLegacySnapshot(journal *legacyCleanupJournal, index int) erro return restoreLegacySnapshot(journal, index) } + if !anchorExists { + // No destructive operation has occurred and the durable identity proof is + // gone. Never infer ownership from public bytes alone. + return removeLegacyJournalEntry(journal, index) + } + targetExists, err := pathExists(entry.path) if err != nil { return err From 380e948666e8c42555ba6af87638b37fe525f1e0 Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 14:55:22 +0100 Subject: [PATCH 37/44] Test inode-bound legacy snapshot ownership --- internal/router/merlin/merlin_test.go | 66 +++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/internal/router/merlin/merlin_test.go b/internal/router/merlin/merlin_test.go index 2ea56215..c3d39f2d 100644 --- a/internal/router/merlin/merlin_test.go +++ b/internal/router/merlin/merlin_test.go @@ -757,3 +757,69 @@ func Test_merlinPostConfValidatesCtrldPidOwnership(t *testing.T) { t.Fatal("Merlin postconf must not treat PID existence alone as ctrld ownership") } } + + +func Test_snapshotFileStillOwnedRejectsByteIdenticalReplacement(t *testing.T) { + dir := t.TempDir() + public := filepath.Join(dir, "dnsmasq.conf") + anchor := filepath.Join(dir, "anchor") + original := []byte("same bytes\n") + if err := os.WriteFile(public, original, 0644); err != nil { + t.Fatal(err) + } + if err := os.Link(public, anchor); err != nil { + t.Fatal(err) + } + hash := merlinSnapshotHash(original) + + // Replace the public pathname with a different inode containing identical + // bytes. Hash-only ownership would misclassify this as ctrld-owned. + replacement := filepath.Join(dir, "replacement") + if err := os.WriteFile(replacement, original, 0644); err != nil { + t.Fatal(err) + } + if err := os.Rename(replacement, public); err != nil { + t.Fatal(err) + } + + owned, err := snapshotFileStillOwned(public, anchor, hash) + if err != nil { + t.Fatal(err) + } + if owned { + t.Fatal("byte-identical replacement must not be treated as the captured inode") + } +} + +func Test_snapshotFileStillOwnedAcceptsCapturedHardLink(t *testing.T) { + dir := t.TempDir() + public := filepath.Join(dir, "dnsmasq.conf") + anchor := filepath.Join(dir, "anchor") + quarantine := filepath.Join(dir, "quarantine") + content := []byte("legacy snapshot\n") + if err := os.WriteFile(public, content, 0644); err != nil { + t.Fatal(err) + } + if err := os.Link(public, anchor); err != nil { + t.Fatal(err) + } + if err := os.Rename(public, quarantine); err != nil { + t.Fatal(err) + } + + owned, err := snapshotFileStillOwned(quarantine, anchor, merlinSnapshotHash(content)) + if err != nil { + t.Fatal(err) + } + if !owned { + t.Fatal("quarantined hard link to captured inode should remain ctrld-owned") + } +} + +func Test_legacySnapshotAnchorPathUsesCtrldPrivateDir(t *testing.T) { + got := legacySnapshotAnchorPath(filepath.Join(dnsmasq.MerlinJffsConfDir, "dnsmasq-2.conf")) + want := "/jffs/controld/.dnsmasq-2.conf.ctrld-legacy-anchor" + if got != want { + t.Fatalf("anchor path = %q, want %q", got, want) + } +} From c052d0860678c10a78e32768764d83f9f5e4073a Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 15:15:23 +0100 Subject: [PATCH 38/44] Use POSIX paths for Merlin cleanup journals --- internal/router/merlin/merlin.go | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/internal/router/merlin/merlin.go b/internal/router/merlin/merlin.go index 2f46f525..5168c65d 100644 --- a/internal/router/merlin/merlin.go +++ b/internal/router/merlin/merlin.go @@ -8,6 +8,7 @@ import ( "io" "os" "os/exec" + pathpkg "path" "path/filepath" "runtime" "strings" @@ -283,9 +284,9 @@ func (m *Merlin) Cleanup() error { } func legacySnapshotAnchorPath(path string) string { - return filepath.Join( + return pathpkg.Join( "/jffs/controld", - "."+filepath.Base(path)+".ctrld-legacy-anchor", + "."+pathpkg.Base(path)+".ctrld-legacy-anchor", ) } @@ -522,9 +523,9 @@ func parseLegacyCleanupJournal(buf []byte) (legacyCleanupJournal, error) { } func legacySnapshotQuarantinePath(path string) string { - return filepath.Join( + return pathpkg.Join( dnsmasq.MerlinJffsConfDir, - "."+filepath.Base(path)+".ctrld-legacy-quarantine", + "."+pathpkg.Base(path)+".ctrld-legacy-quarantine", ) } @@ -761,10 +762,10 @@ func isLegacySnapshotPath(path string) bool { if path == dnsmasq.MerlinJffsConfPath { return true } - if filepath.Dir(path) != dnsmasq.MerlinJffsConfDir { + if pathpkg.Dir(path) != dnsmasq.MerlinJffsConfDir { return false } - base := filepath.Base(path) + base := pathpkg.Base(path) if !strings.HasPrefix(base, "dnsmasq-") || !strings.HasSuffix(base, ".conf") { return false } @@ -780,17 +781,6 @@ func isLegacySnapshotPath(path string) bool { return true } -func readMerlinState(path string) (string, error) { - buf, err := os.ReadFile(path) - if os.IsNotExist(err) { - return "", nil - } - if err != nil { - return "", err - } - return strings.TrimSpace(string(buf)), nil -} - // dnsmasqConfigUsesCtrld reports whether a generated dnsmasq config has the // complete ctrld forwarding shape, not merely one matching server line. func (m *Merlin) dnsmasqConfigUsesCtrld(path string) (bool, error) { From 5171b79ede2f975fba85e537239173f09ef0e2e2 Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 15:15:44 +0100 Subject: [PATCH 39/44] Use POSIX paths in Merlin journal tests --- internal/router/merlin/merlin_test.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/internal/router/merlin/merlin_test.go b/internal/router/merlin/merlin_test.go index c3d39f2d..a72976e5 100644 --- a/internal/router/merlin/merlin_test.go +++ b/internal/router/merlin/merlin_test.go @@ -3,6 +3,7 @@ package merlin import ( "bytes" "os" + pathpkg "path" "path/filepath" "runtime" "strings" @@ -520,8 +521,8 @@ func Test_dnsmasqConfigUsesCtrld(t *testing.T) { func Test_legacyCleanupJournalRoundTrip(t *testing.T) { entries := []legacySnapshotEntry{ {state: legacyEntryPending, path: dnsmasq.MerlinJffsConfPath, hash: merlinSnapshotHash([]byte("main"))}, - {state: legacyEntryDelete, path: filepath.Join(dnsmasq.MerlinJffsConfDir, "dnsmasq-1.conf"), hash: merlinSnapshotHash([]byte("sdn1"))}, - {state: legacyEntryRestore, path: filepath.Join(dnsmasq.MerlinJffsConfDir, "dnsmasq-3.conf"), hash: merlinSnapshotHash([]byte("sdn3"))}, + {state: legacyEntryDelete, path: pathpkg.Join(dnsmasq.MerlinJffsConfDir, "dnsmasq-1.conf"), hash: merlinSnapshotHash([]byte("sdn1"))}, + {state: legacyEntryRestore, path: pathpkg.Join(dnsmasq.MerlinJffsConfDir, "dnsmasq-3.conf"), hash: merlinSnapshotHash([]byte("sdn3"))}, } want := legacyCleanupJournal{phase: legacyCleanupPhase, entries: entries} buf, err := encodeLegacyCleanupJournal(want) @@ -575,10 +576,10 @@ func Test_isLegacySnapshotPath(t *testing.T) { want bool }{ {dnsmasq.MerlinJffsConfPath, true}, - {filepath.Join(dnsmasq.MerlinJffsConfDir, "dnsmasq-1.conf"), true}, - {filepath.Join(dnsmasq.MerlinJffsConfDir, "dnsmasq-123.conf"), true}, - {filepath.Join(dnsmasq.MerlinJffsConfDir, "dnsmasq.conf.add"), false}, - {filepath.Join(dnsmasq.MerlinJffsConfDir, "dnsmasq-sdn.conf"), false}, + {pathpkg.Join(dnsmasq.MerlinJffsConfDir, "dnsmasq-1.conf"), true}, + {pathpkg.Join(dnsmasq.MerlinJffsConfDir, "dnsmasq-123.conf"), true}, + {pathpkg.Join(dnsmasq.MerlinJffsConfDir, "dnsmasq.conf.add"), false}, + {pathpkg.Join(dnsmasq.MerlinJffsConfDir, "dnsmasq-sdn.conf"), false}, {"/tmp/dnsmasq-1.conf", false}, } for _, tc := range tests { @@ -817,7 +818,7 @@ func Test_snapshotFileStillOwnedAcceptsCapturedHardLink(t *testing.T) { } func Test_legacySnapshotAnchorPathUsesCtrldPrivateDir(t *testing.T) { - got := legacySnapshotAnchorPath(filepath.Join(dnsmasq.MerlinJffsConfDir, "dnsmasq-2.conf")) + got := legacySnapshotAnchorPath(pathpkg.Join(dnsmasq.MerlinJffsConfDir, "dnsmasq-2.conf")) want := "/jffs/controld/.dnsmasq-2.conf.ctrld-legacy-anchor" if got != want { t.Fatalf("anchor path = %q, want %q", got, want) From fbe9d0e7deb96c9d9b55838e8ad2628f84aa1569 Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 16:34:22 +0100 Subject: [PATCH 40/44] Harden Merlin state paths and dnsmasq verification --- internal/router/merlin/merlin.go | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/internal/router/merlin/merlin.go b/internal/router/merlin/merlin.go index 5168c65d..8c4fd527 100644 --- a/internal/router/merlin/merlin.go +++ b/internal/router/merlin/merlin.go @@ -25,11 +25,12 @@ import ( const Name = "merlin" const ( - merlinManagedStatePath = "/jffs/controld/.merlin-dnsmasq-hooks-v2" + merlinStateDir = "/jffs/controld" + merlinManagedStatePath = merlinStateDir + "/.merlin-dnsmasq-hooks-v2" merlinSnapshotStatePath = "/jffs/controld/.merlin-dnsmasq-snapshot" merlinSnapshotAnchorPath = "/jffs/configs/.dnsmasq.conf.ctrld-anchor" merlinSnapshotQuarantinePath = "/jffs/configs/.dnsmasq.conf.ctrld-quarantine" - merlinCleanupPendingPath = "/jffs/controld/.merlin-dnsmasq-cleanup-pending" + merlinCleanupPendingPath = merlinStateDir + "/.merlin-dnsmasq-cleanup-pending" ) // nvramKvMap is a map of NVRAM key-value pairs used to configure and manage Merlin-specific settings. @@ -65,6 +66,9 @@ func (m *Merlin) Uninstall(_ *service.Config) error { // PreRun prepares the Merlin instance for operation by waiting for essential services and directories to become available. func (m *Merlin) PreRun() error { + if err := ensureMerlinStateDir(); err != nil { + return err + } // Reconcile any previous Merlin integration before starting again. Router // startup is a one-shot launch, so retry transient JFFS/NVRAM/dnsmasq errors // here instead of abandoning DNS in a partially reconciled state. @@ -100,8 +104,18 @@ func (m *Merlin) PreRun() error { return nil } +func ensureMerlinStateDir() error { + if err := os.MkdirAll(merlinStateDir, 0755); err != nil { + return fmt.Errorf("create Merlin state directory %s: %w", merlinStateDir, err) + } + return nil +} + // Setup initializes and configures the Merlin instance for use, including setting up dnsmasq and necessary nvram settings. func (m *Merlin) Setup() (retErr error) { + if err := ensureMerlinStateDir(); err != nil { + return err + } if m.cfg.FirstListener().IsDirectDnsListener() { return nil } @@ -206,6 +220,9 @@ type legacyCleanupJournal struct { // Cleanup restores the original dnsmasq and nvram configurations and restarts dnsmasq if necessary. func (m *Merlin) Cleanup() error { + if err := ensureMerlinStateDir(); err != nil { + return err + } // Preserve the existing direct-listener lifecycle. A direct listener needs // port 53 itself; restarting dnsmasq here would reclaim that port before // ctrld binds. Transitioning between forwarding and direct-listener modes @@ -285,7 +302,7 @@ func (m *Merlin) Cleanup() error { func legacySnapshotAnchorPath(path string) string { return pathpkg.Join( - "/jffs/controld", + merlinStateDir, "."+pathpkg.Base(path)+".ctrld-legacy-anchor", ) } @@ -326,7 +343,7 @@ func buildLegacyCleanupJournal() (journal legacyCleanupJournal, retErr error) { // With no durable journal present, any ctrld-private legacy anchors can only // be leftovers from a crash before journal publication. Removing an anchor // drops only ctrld's extra hard link; it never touches the public snapshot. - anchorOrphans, err := filepath.Glob("/jffs/controld/.dnsmasq*.ctrld-legacy-anchor") + anchorOrphans, err := filepath.Glob(filepath.Join(merlinStateDir, ".dnsmasq*.ctrld-legacy-anchor")) if err != nil { return legacyCleanupJournal{}, err } @@ -823,6 +840,9 @@ func (m *Merlin) dnsmasqConfigUsesCtrld(path string) (bool, error) { } case strings.HasPrefix(line, "servers-file="), strings.HasPrefix(line, "resolv-file="), + strings.HasPrefix(line, "conf-file="), + strings.HasPrefix(line, "conf-dir="), + strings.HasPrefix(line, "conf-script="), line == "dnssec", strings.HasPrefix(line, "trust-anchor="): return false, nil From 59f51c00a2b0956806223025d975ea5f5259dc08 Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 16:34:41 +0100 Subject: [PATCH 41/44] Test dnsmasq include directives fail closed --- internal/router/merlin/merlin_test.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/internal/router/merlin/merlin_test.go b/internal/router/merlin/merlin_test.go index a72976e5..52ccd5f6 100644 --- a/internal/router/merlin/merlin_test.go +++ b/internal/router/merlin/merlin_test.go @@ -489,6 +489,30 @@ func Test_dnsmasqConfigUsesCtrld(t *testing.T) { "resolv-file=/tmp/resolv.conf\n", want: false, }, + { + name: "conf file include is unresolved", + ip: "0.0.0.0", + port: 5354, + content: good("server=127.0.0.1#5354") + + "conf-file=/jffs/configs/extra-dnsmasq.conf\n", + want: false, + }, + { + name: "conf dir include is unresolved", + ip: "0.0.0.0", + port: 5354, + content: good("server=127.0.0.1#5354") + + "conf-dir=/jffs/configs/dnsmasq.d\n", + want: false, + }, + { + name: "conf script include is unresolved", + ip: "0.0.0.0", + port: 5354, + content: good("server=127.0.0.1#5354") + + "conf-script=/jffs/scripts/dnsmasq-extra.sh\n", + want: false, + }, { name: "missing metadata directives", ip: "0.0.0.0", From a83b4eca5589c17370c910ff0714d417754baf61 Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 16:41:48 +0100 Subject: [PATCH 42/44] Make Merlin mode-preservation test umask independent --- internal/router/merlin/merlin_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/internal/router/merlin/merlin_test.go b/internal/router/merlin/merlin_test.go index 52ccd5f6..4fed479c 100644 --- a/internal/router/merlin/merlin_test.go +++ b/internal/router/merlin/merlin_test.go @@ -408,7 +408,13 @@ func Test_atomicWriteFilePreservesExistingMode(t *testing.T) { for _, mode := range []os.FileMode{0700, 0770} { t.Run(mode.String(), func(t *testing.T) { path := filepath.Join(t.TempDir(), "dnsmasq.postconf") - if err := os.WriteFile(path, []byte("old"), mode); err != nil { + if err := os.WriteFile(path, []byte("old"), 0600); err != nil { + t.Fatal(err) + } + // os.WriteFile is subject to the process umask; set the fixture mode + // explicitly so the assertion tests atomicWriteFile rather than the + // runner's umask. + if err := os.Chmod(path, mode); err != nil { t.Fatal(err) } if err := atomicWriteFile(path, []byte("new"), 0750); err != nil { From fcd016c5fa36afa08ba6b866cb267e003a49357b Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 16:44:20 +0100 Subject: [PATCH 43/44] Make Merlin state directory creation durable --- internal/router/merlin/merlin.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/internal/router/merlin/merlin.go b/internal/router/merlin/merlin.go index 8c4fd527..29349bbe 100644 --- a/internal/router/merlin/merlin.go +++ b/internal/router/merlin/merlin.go @@ -105,9 +105,25 @@ func (m *Merlin) PreRun() error { } func ensureMerlinStateDir() error { + if info, err := os.Stat(merlinStateDir); err == nil { + if !info.IsDir() { + return fmt.Errorf("Merlin state path is not a directory: %s", merlinStateDir) + } + return nil + } else if !os.IsNotExist(err) { + return fmt.Errorf("stat Merlin state directory %s: %w", merlinStateDir, err) + } + + // A newly created JFFS directory entry is not crash-durable until its + // parent has been synced. State files below this directory are used to + // distinguish current hook-based installs from legacy snapshot ownership, + // so losing only the directory across power loss would be unsafe. if err := os.MkdirAll(merlinStateDir, 0755); err != nil { return fmt.Errorf("create Merlin state directory %s: %w", merlinStateDir, err) } + if err := syncParentDir(filepath.Dir(merlinStateDir)); err != nil { + return fmt.Errorf("sync parent of Merlin state directory %s: %w", merlinStateDir, err) + } return nil } From cf269e253db39e65703fba25a538d82c9975a60b Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 16:50:51 +0100 Subject: [PATCH 44/44] Reconfirm Merlin state directory durability on retry --- internal/router/merlin/merlin.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internal/router/merlin/merlin.go b/internal/router/merlin/merlin.go index 29349bbe..7169e6ed 100644 --- a/internal/router/merlin/merlin.go +++ b/internal/router/merlin/merlin.go @@ -109,6 +109,12 @@ func ensureMerlinStateDir() error { if !info.IsDir() { return fmt.Errorf("Merlin state path is not a directory: %s", merlinStateDir) } + // Always re-sync the parent before relying on state below this + // directory. This also closes the retry window where mkdir succeeded + // previously but the parent fsync reported a transient failure. + if err := syncParentDir(filepath.Dir(merlinStateDir)); err != nil { + return fmt.Errorf("sync parent of existing Merlin state directory %s: %w", merlinStateDir, err) + } return nil } else if !os.IsNotExist(err) { return fmt.Errorf("stat Merlin state directory %s: %w", merlinStateDir, err)