From 0cec599d7bb5c18281ff8ffeedbc5e616938f302 Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 12:40:30 +0100 Subject: [PATCH 01/11] Port Merlin DNS lifecycle fixes to v1.x --- internal/router/dnsmasq/dnsmasq.go | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/internal/router/dnsmasq/dnsmasq.go b/internal/router/dnsmasq/dnsmasq.go index 64041802..00cd0a00 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 ( @@ -47,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 7132ddd2c86e10621aae14ee1e6d24afea0d9e53 Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 12:40:33 +0100 Subject: [PATCH 02/11] Port Merlin DNS lifecycle fixes to v1.x --- internal/router/merlin/merlin.go | 1276 ++++++++++++++++++++++++++++-- 1 file changed, 1197 insertions(+), 79 deletions(-) diff --git a/internal/router/merlin/merlin.go b/internal/router/merlin/merlin.go index 855cfe4a..050d9b61 100644 --- a/internal/router/merlin/merlin.go +++ b/internal/router/merlin/merlin.go @@ -2,6 +2,8 @@ package merlin import ( "bytes" + "crypto/sha256" + "encoding/hex" "fmt" "io" "os" @@ -21,16 +23,19 @@ import ( const Name = "merlin" +const ( + 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. var nvramKvMap = map[string]string{ "dnspriv_enable": "0", // Ensure Merlin native DoT disabled. } -// 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 { @@ -59,8 +64,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,114 +100,1126 @@ 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.SetKVWithVolatileRetryMarker(nvramKvMap, nvram.CtrldSetupKey); err != nil { + return err + } + + if err := m.writeDnsmasqPostconf(); err != nil { + return err } - // Restart dnsmasq service. + // 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 } +const ( + legacyCleanupPhase = "cleanup-v2" + legacyFinalizePhase = "finalize-v2" + + legacyEntryPending = "pending" + legacyEntryDelete = "delete" + legacyEntryRestore = "restore" +) + +type legacySnapshotEntry struct { + state string + 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 + } + } + + if legacy { + if err := cleanupLegacySnapshots(&journal); err != nil { + return err + } + } else { + if err := cleanupOwnedMainSnapshot(); 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) + // Restore NVRAM only after ctrld-owned artifacts are cleaned successfully. + if setupVal == "1" { + if err := nvram.RestoreWithVolatileRetryMarker(nvramKvMap, nvram.CtrldSetupKey); err != nil { + return err } } - // Restart dnsmasq service. + 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) { + // 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 { + return legacyCleanupJournal{}, err + } + paths = append(paths, matches...) + + journal := legacyCleanupJournal{phase: legacyCleanupPhase} + 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{ + state: legacyEntryPending, + 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') + 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) + b.WriteByte('\n') + } + return []byte(b.String()), nil +} + +func readLegacyCleanupJournal() (legacyCleanupJournal, error) { + buf, err := os.ReadFile(merlinCleanupPendingPath) if os.IsNotExist(err) { - return nil // nothing to do if conf file does not exist. + return legacyCleanupJournal{}, nil } if err != nil { - return fmt.Errorf("failed to open dnsmasq config: %w", err) + return legacyCleanupJournal{}, err } - defer src.Close() + return parseLegacyCleanupJournal(buf) +} - // 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) +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 == legacyFinalizePhase { + if len(lines) != 1 { + return legacyCleanupJournal{}, fmt.Errorf("finalize journal contains snapshot entries") + } + return journal, nil + } + 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", 3) + if len(parts) != 3 || !isLegacySnapshotPath(parts[1]) { + return legacyCleanupJournal{}, fmt.Errorf("invalid legacy cleanup journal entry %q", line) + } + 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[2]); err != nil { + return legacyCleanupJournal{}, fmt.Errorf("invalid legacy snapshot hash for %s: %w", parts[1], err) + } + 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 fmt.Errorf("failed to create %s: %w", cfg.jffsConfPath, err) + 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) } - defer dst.Close() - if _, err := io.Copy(dst, src); err != nil { - return fmt.Errorf("failed to copy current dnsmasq config: %w", err) + f, err := os.Open(entry.path) + if os.IsNotExist(err) { + return removeLegacyJournalEntry(journal, index) } - if err := dst.Close(); err != nil { - return fmt.Errorf("failed to save %s: %w", cfg.jffsConfPath, err) + 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) } - // Run postconf script on cfg.jffsConfPath directly. - cmd := exec.Command("/bin/sh", dnsmasq.MerlinPostConfPath, cfg.jffsConfPath) - if out, err := cmd.CombinedOutput(); err != nil { - return fmt.Errorf("failed to run post conf: %s: %w", string(out), err) + 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 + } + 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 + } + 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) { + buf, err := os.ReadFile(path) + if os.IsNotExist(err) { + return false, nil + } + if err != nil { + return false, err + } + + listener := m.cfg.FirstListener() + if listener == nil { + return false, fmt.Errorf("missing ctrld listener") + } + 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) + + 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 represented by a private hard-link anchor to the exact inode +// ctrld published, plus a durable phase/hash journal. +func (m *Merlin) setupMainDnsmasqFallback() error { + // 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 { + return fmt.Errorf("refusing to overwrite unowned Merlin custom config: %s", dnsmasq.MerlinJffsConfPath) + } + + 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 + } + if err := publishMainSnapshot(built, 0644); err != nil { + return fmt.Errorf("publish ctrld dnsmasq fallback: %w", err) } return nil } -// 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) { +// 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 err } + tmpPath := tmp.Name() + defer os.Remove(tmpPath) + + if err := tmp.Chmod(mode); err != nil { + _ = tmp.Close() + return err + } + if _, err := tmp.Write(data); err != nil { + _ = tmp.Close() + return err + } + if err := tmp.Sync(); err != nil { + _ = tmp.Close() + return err + } + if err := tmp.Close(); err != nil { + return 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) + } + 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 + } + if err := syncParentDir(dir); err != nil { + return fmt.Errorf("sync published fallback: %w", err) + } + + 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) { + 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 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 +} + +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 encodeMainSnapshotState(state mainSnapshotState) ([]byte, error) { + if state.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 nil, fmt.Errorf("invalid Merlin snapshot hash length") + } + if _, err := hex.DecodeString(state.hash); err != nil { + return nil, fmt.Errorf("invalid Merlin snapshot hash: %w", err) + } + return []byte("snapshot-v2\nphase=" + state.phase + "\nsha256=" + state.hash + "\n"), nil +} + +func parseMainSnapshotState(buf []byte) (mainSnapshotState, error) { + lines := strings.Split(strings.TrimSpace(string(buf)), "\n") + if len(lines) == 1 && strings.HasPrefix(lines[0], "sha256=") { + 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{}, fmt.Errorf("invalid Merlin snapshot state") + } + state := mainSnapshotState{ + phase: strings.TrimPrefix(lines[1], "phase="), + hash: strings.TrimPrefix(lines[2], "sha256="), + } + if _, err := encodeMainSnapshotState(state); err != nil { + return mainSnapshotState{}, err + } + return state, nil +} + +func writeMainSnapshotState(state mainSnapshotState) error { + buf, err := encodeMainSnapshotState(state) + if err != nil { + return 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 +} + +func sameFilePaths(a, b string) (bool, error) { + ai, err := os.Stat(a) + if err != nil { + return false, err + } + bi, err := os.Stat(b) + if err != nil { + return false, err + } + 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 { + 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(state) + 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() + } + + same, err := sameFilePaths(dnsmasq.MerlinJffsConfPath, merlinSnapshotAnchorPath) + if err != nil { + return err + } + 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 cleanupOwnedMainSnapshot() +} + +func cleanupQuarantinedMainSnapshot(state mainSnapshotState) error { + quarantineExists, err := pathExists(merlinSnapshotQuarantinePath) + if err != nil { + return err + } + 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) + } + } + // 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) + if err != nil { + return err + } + if !anchorExists { + state.phase = snapshotPhaseRestore + if err := writeMainSnapshotState(state); err != nil { + return err + } + return cleanupOwnedMainSnapshot() + } + + owned, err := snapshotFileStillOwned( + merlinSnapshotQuarantinePath, + merlinSnapshotAnchorPath, + state.hash, + ) + if err != nil { + return err + } + if owned { + 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 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) + } + 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 !same { + return fmt.Errorf( + "cannot restore captured Merlin fallback because %s was recreated; preserved capture at %s", + dnsmasq.MerlinJffsConfPath, merlinSnapshotQuarantinePath, + ) + } + } 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) + } + } + // 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 + if err := writeMainSnapshotState(state); err != nil { + return fmt.Errorf("journal restored Merlin fallback: %w", 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 cleanupSnapshotPrivateState() +} + +// 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 +1229,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,15 +1244,91 @@ func (m *Merlin) writeDnsmasqPostconf() error { dnsmasq.MerlinPostConfEndMarker, }, "\n")) - update, err := prepareMerlinHookUpdate(dnsmasq.MerlinPostConfPath, block) + return writeMerlinHookUpdates( + []string{dnsmasq.MerlinPostConfPath, dnsmasq.MerlinSdnPostConfPath}, + block, + ) +} + +func writeMerlinHookUpdates(paths []string, block []byte) error { + return writeMerlinHookUpdatesWith(paths, block, atomicWriteFile) +} + +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) + } + + 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) + } + return nil +} + +func rollbackMerlinHookUpdates(updates []merlinHookUpdate) error { + for i := len(updates) - 1; i >= 0; i-- { + update := updates[i] + 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 { + return fmt.Errorf("restore %s: %w", update.path, err) + } + continue + } + 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 err := revalidateMerlinHookUpdate(update); err != nil { - return fmt.Errorf("revalidate Merlin hook %s: %w", update.path, err) + if update.existed && info.Mode().Type() != update.pathType { + return fmt.Errorf("shared hook type changed after ctrld write") } - if err := atomicWriteFile(update.path, update.data, 0750); err != nil { - return fmt.Errorf("write Merlin hook %s: %w", update.path, err) + 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 } @@ -231,15 +1340,13 @@ func prepareMerlinHookUpdate(path string, block []byte) (merlinHookUpdate, error } func prepareMerlinHookCleanup(path string) (*merlinHookUpdate, error) { - info, err := os.Lstat(path) + _, err := os.Lstat(path) if os.IsNotExist(err) { return nil, nil } if err != nil { return nil, err } - _ = info - update, err := prepareMerlinHookReplacement(path, merlinParsePostConf) if err != nil { return nil, err @@ -345,19 +1452,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. @@ -665,8 +1759,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 +1767,32 @@ 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) { + // 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 + } + return syncParentDir(filepath.Dir(path)) +} + // waitDirExists waits until the specified directory exists, polling its existence every second. func waitDirExists(dir string) { for { From 780f4f76a7f9943ddf37be7034fdeb68d98f1953 Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 12:40:36 +0100 Subject: [PATCH 03/11] Port Merlin DNS lifecycle fixes to v1.x --- internal/router/merlin/merlin_test.go | 374 ++++++++++++++++++++++++++ 1 file changed, 374 insertions(+) diff --git a/internal/router/merlin/merlin_test.go b/internal/router/merlin/merlin_test.go index ddcabf1c..2ea56215 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" ) @@ -96,6 +97,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 +289,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 +424,170 @@ 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{ + {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: legacyCleanupPhase, 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(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(legacyFinalizePhase + "\n")); err != nil { + t.Fatalf("valid finalize journal rejected: %v", err) + } + hash := merlinSnapshotHash([]byte("x")) + 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 + 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 +608,152 @@ func Test_cleanupPreparationRevalidatesBeforeWrite(t *testing.T) { t.Fatal("expected cleanup revalidation failure after external modification") } } + + + +func Test_mainSnapshotStateRoundTrip(t *testing.T) { + want := mainSnapshotState{ + phase: snapshotPhaseQuarantine, + hash: merlinSnapshotHash([]byte("owned snapshot")), + } + buf, err := encodeMainSnapshotState(want) + if err != nil { + t.Fatal(err) + } + got, err := parseMainSnapshotState(buf) + if err != nil { + t.Fatal(err) + } + if got != want { + t.Fatalf("snapshot state mismatch: want %#v, got %#v", 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_sameFilePathsDistinguishesHardLinkFromIdenticalCopy(t *testing.T) { + dir := t.TempDir() + original := filepath.Join(dir, "original") + hardLink := filepath.Join(dir, "hard-link") + copyPath := filepath.Join(dir, "copy") + content := []byte("same bytes\n") + + if err := os.WriteFile(original, content, 0644); err != nil { + t.Fatal(err) + } + if err := os.Link(original, hardLink); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(copyPath, content, 0644); err != nil { + t.Fatal(err) + } + + same, err := sameFilePaths(original, hardLink) + if err != nil { + t.Fatal(err) + } + if !same { + t.Fatal("hard link to same inode was not recognized") + } + same, err = sameFilePaths(original, copyPath) + if err != nil { + t.Fatal(err) + } + if same { + t.Fatal("byte-identical copy must not be treated as the owned inode") + } +} + +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) + } + } +} + + +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) + } +} + + +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 0fabf14f63443d7c80307be2c64cd1affe60c2f3 Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 12:40:39 +0100 Subject: [PATCH 04/11] Port Merlin DNS lifecycle fixes to v1.x --- internal/router/nvram/nvram.go | 131 ++++++++++++++++++++++++++++++--- 1 file changed, 121 insertions(+), 10 deletions(-) diff --git a/internal/router/nvram/nvram.go b/internal/router/nvram/nvram.go index e76c0171..0dd1eb87 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,9 +37,11 @@ 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. +// +// 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 { - // Backup current value, store ctrld's configs. for key, value := range m { old, err := Run("get", key) if err != nil { @@ -51,28 +54,121 @@ func SetKV(m map[string]string, setupKey string) error { return fmt.Errorf("%s: %w", out, err) } } - if out, err := Run("set", setupKey+"=1"); err != nil { return fmt.Errorf("%s: %w", out, err) } - // Commit. if out, err := Run("commit"); err != nil { return 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. +// 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-- { + 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 { + return rearmVolatile(errors.Join( + cause, + fmt.Errorf("nvram rollback incomplete: %w", restoreErr), + )) + } + + if out, err := Run("unset", setupKey); err != nil { + 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: %s: %w", out, err), + )) + } + 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 fail(fmt.Errorf("%s: %w", old, err)) + } + if out, err := Run("set", CtrldKeyPrefix+key+"="+old); err != nil { + return fail(fmt.Errorf("%s: %w", out, err)) + } + modified = append(modified, key) + if out, err := Run("set", key+"="+value); err != nil { + return rollback(fmt.Errorf("%s: %w", out, err)) + } + } + + if out, err := Run("set", setupKey+"=1"); err != nil { + return rollback(fmt.Errorf("%s: %w", out, err)) + } + if out, err := Run("commit"); err != nil { + return rollback(fmt.Errorf("%s: %w", out, err)) + } + return nil +} + +// 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. + 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) 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 +177,24 @@ 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 { - return fmt.Errorf("%s: %w", out, err) + 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, + fmt.Errorf("failed to re-arm volatile %s after restore commit failure: %s: %w", setupKey, markerOut, markerErr), + ) + } + return commitErr } return nil } From 2d38ccec6a5b9ec40ddc52d33be43f3068296977 Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 12:44:34 +0100 Subject: [PATCH 05/11] Make mode preservation test umask-safe --- internal/router/merlin/merlin_test.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/internal/router/merlin/merlin_test.go b/internal/router/merlin/merlin_test.go index 2ea56215..103bb035 100644 --- a/internal/router/merlin/merlin_test.go +++ b/internal/router/merlin/merlin_test.go @@ -410,6 +410,11 @@ func Test_atomicWriteFilePreservesExistingMode(t *testing.T) { if err := os.WriteFile(path, []byte("old"), mode); err != nil { t.Fatal(err) } + before, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + wantMode := before.Mode().Perm() if err := atomicWriteFile(path, []byte("new"), 0750); err != nil { t.Fatal(err) } @@ -417,8 +422,8 @@ func Test_atomicWriteFilePreservesExistingMode(t *testing.T) { if err != nil { t.Fatal(err) } - if got := info.Mode().Perm(); got != mode.Perm() { - t.Fatalf("mode = %v, want %v", got, mode.Perm()) + if got := info.Mode().Perm(); got != wantMode { + t.Fatalf("mode = %v, want preserved %v", got, wantMode) } }) } From 23ace09e62c39fbca43423ea2053fe5ba6517be1 Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 15:02:30 +0100 Subject: [PATCH 06/11] Sync latest Merlin DNS lifecycle hardening --- internal/router/merlin/merlin.go | 226 +++++++++++++++++++++++++------ 1 file changed, 184 insertions(+), 42 deletions(-) diff --git a/internal/router/merlin/merlin.go b/internal/router/merlin/merlin.go index 050d9b61..2f46f525 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) } } @@ -279,7 +282,30 @@ 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 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. orphans, err := filepath.Glob(filepath.Join( @@ -296,6 +322,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 +342,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 @@ -463,23 +567,47 @@ 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) qExists, err := pathExists(qpath) if err != nil { return err } + 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 + } + 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) @@ -487,35 +615,39 @@ func cleanupPendingLegacySnapshot(journal *legacyCleanupJournal, index int) erro return restoreLegacySnapshot(journal, index) } - f, err := os.Open(entry.path) - if os.IsNotExist(err) { + 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 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) } @@ -523,15 +655,14 @@ func cleanupPendingLegacySnapshot(journal *legacyCleanupJournal, index int) erro 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) + owned, err = snapshotFileStillOwned(qpath, anchor, entry.hash) 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) @@ -549,16 +680,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) @@ -569,20 +702,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) } @@ -612,6 +751,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 263ece1fb2a43e4908d49e60bdca32e3b5b9ebfb Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 15:02:34 +0100 Subject: [PATCH 07/11] Sync latest Merlin DNS regression tests --- 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 103bb035..d3437948 100644 --- a/internal/router/merlin/merlin_test.go +++ b/internal/router/merlin/merlin_test.go @@ -762,3 +762,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 741048d2ff894cbf049892c11e05d6b6fb9ec58a Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 15:16:01 +0100 Subject: [PATCH 08/11] Sync POSIX Merlin cleanup path semantics --- 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 d029e689cb81b4612c488e167a4188b07c62aeaf Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 15:16:08 +0100 Subject: [PATCH 09/11] Sync POSIX Merlin journal regression 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 d3437948..ef74e5d1 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" @@ -525,8 +526,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) @@ -580,10 +581,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 { @@ -822,7 +823,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 9f405054be7894536ade47381ddf6cfb13d27307 Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 21:10:50 +0100 Subject: [PATCH 10/11] Port final Merlin DNS/SDN safety fixes to v1.x --- internal/router/merlin/merlin.go | 50 +++++++++++++++++++++++++++++--- 1 file changed, 46 insertions(+), 4 deletions(-) diff --git a/internal/router/merlin/merlin.go b/internal/router/merlin/merlin.go index 5168c65d..7169e6ed 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,40 @@ func (m *Merlin) PreRun() error { return nil } +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) + } + // 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) + } + + // 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 +} + // 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 +242,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 +324,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 +365,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 +862,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 2d79f58ca4396eebcd5717cfe03f310c67cca7f6 Mon Sep 17 00:00:00 2001 From: Maghuro <1546139+maghuro@users.noreply.github.com> Date: Sun, 27 Sep 2026 21:10:53 +0100 Subject: [PATCH 11/11] Port final Merlin DNS regression coverage to v1.x --- internal/router/merlin/merlin_test.go | 37 ++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/internal/router/merlin/merlin_test.go b/internal/router/merlin/merlin_test.go index ef74e5d1..4fed479c 100644 --- a/internal/router/merlin/merlin_test.go +++ b/internal/router/merlin/merlin_test.go @@ -408,14 +408,15 @@ 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) } - before, err := os.Stat(path) - if err != nil { + // 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) } - wantMode := before.Mode().Perm() if err := atomicWriteFile(path, []byte("new"), 0750); err != nil { t.Fatal(err) } @@ -423,8 +424,8 @@ func Test_atomicWriteFilePreservesExistingMode(t *testing.T) { if err != nil { t.Fatal(err) } - if got := info.Mode().Perm(); got != wantMode { - t.Fatalf("mode = %v, want preserved %v", got, wantMode) + if got := info.Mode().Perm(); got != mode.Perm() { + t.Fatalf("mode = %v, want %v", got, mode.Perm()) } }) } @@ -494,6 +495,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",