Persist the dns resolver settings in the app's own local state

app/network/NetworkApp.swift

@@ -169,11 +169,11 @@ struct NetworkApp: App {
     private func setupDeviceStores(_ device: SdkDeviceRemote) {
         let localState = deviceManager.asyncLocalState?.getLocalState()
         throughputStore.setup(device)
-        // the app local state mirrors the split rule and transport settings
-        // edits so they survive an app relaunch while the tunnel is down
-        // (see the stores)
+        // the app local state mirrors the split rule, dns and transport
+        // settings edits so they survive an app relaunch while the tunnel is
+        // down (see the stores)
         blockActionsStore.setup(device, localState: localState)
-        dnsSettingsStore.setup(device)
+        dnsSettingsStore.setup(device, localState: localState)
         transportSettingsStore.setup(device, localState: localState)
         networkPeersStore.setup(device)
         reliabilityStore.setup(device)

app/network/Shared/ViewModels/DeviceManager.swift

@@ -1238,6 +1238,19 @@ extension DeviceManager {
             device.setBlockActionOverrides(blockActionOverrides)
         }
 
+        // the dns resolver settings cross the same way, mirrored by
+        // DnsSettingsStore. The editor opens on whatever the device reports, so
+        // without the seed a tunnel-down launch shows the blank
+        // never-configured form over the resolver the extension has on disk,
+        // and applying an edit from that blank base replaces it on the next
+        // connect. Nothing stored means never mirrored -- leave the
+        // extension's resolver alone; settings with everything turned off are
+        // a real value and are seeded as one. Above setDevice for the same
+        // reason as the rules.
+        if let dnsResolverSettings = localState.getDnsResolverSettings() {
+            device.setDnsResolverSettings(dnsResolverSettings)
+        }
+
         self.setDevice(device: device)
         return true
     }

app/network/Shared/ViewModels/DnsSettingsStore.swift

@@ -113,53 +113,140 @@ private class DnsResolverSettingsListener: NSObject, SdkDnsResolverSettingsChang
     }
 }
 
+private class DnsSettingsRemoteListener: NSObject, SdkRemoteChangeListenerProtocol {
+    private let callback: (Bool) -> Void
+    init(callback: @escaping (Bool) -> Void) {
+        self.callback = callback
+    }
+    func remoteChanged(_ remoteConnected: Bool) {
+        callback(remoteConnected)
+    }
+}
+
 /**
- * Publishes the device dns resolver settings and applies edits
+ * Publishes the device dns resolver settings and applies edits.
+ *
+ * The device persists the settings in its own local state, but in the network
+ * extension's container, which the app process cannot read. The store mirrors
+ * every reading it can vouch for into the app's own local state, and
+ * `DeviceManager.initDevice` seeds the next device from that mirror. Without
+ * it the editor opens on the blank never-configured form whenever the tunnel
+ * is down -- and an edit applied from that blank base is not merely lost with
+ * the app process, it replaces the resolver the extension still holds on the
+ * next connect.
  */
 @MainActor
 class DnsSettingsStore: ObservableObject {
 
     @Published private(set) var settings: DnsSettings? = nil
 
     private var device: SdkDeviceRemote?
+    // the app's own store, which the resolver settings are mirrored into; the
+    // extension keeps its copy in a container this process cannot read
+    private var localState: SdkLocalState?
     private var settingsSub: SdkSubProtocol?
+    private var remoteSub: SdkSubProtocol?
 
-    func setup(_ device: SdkDeviceRemote) {
+    func setup(_ device: SdkDeviceRemote, localState: SdkLocalState?) {
         reset()
 
         self.device = device
+        self.localState = localState
         self.settingsSub = device.add(DnsResolverSettingsListener { [weak self] in
             DispatchQueue.main.async {
                 self?.update()
             }
         })
+        // the settings the extension replays on connect arrive over the
+        // reverse sync, which runs before the remote publishes its service, so
+        // that read still comes out of the remote's own memory. Re-read once
+        // the rpc is up so the mirror is written from the extension's resolver
+        // rather than from this process's guess at it
+        self.remoteSub = device.add(DnsSettingsRemoteListener { [weak self] remoteConnected in
+            DispatchQueue.main.async {
+                if remoteConnected {
+                    self?.update()
+                }
+            }
+        })
 
         update()
     }
 
     func reset() {
         settingsSub?.close()
         settingsSub = nil
+        remoteSub?.close()
+        remoteSub = nil
         device = nil
+        // nothing is persisted from here: reset runs on every backgrounding
+        // and the mirror is already current by then
+        localState = nil
         settings = nil
     }
 
-    private func update() {
+    /**
+     * Re-reads the settings from the device and, when the read is one this
+     * process can vouch for, mirrors it.
+     *
+     * Only two reads qualify: one taken off a connected device, which is the
+     * extension's own resolver, and one taken right after an edit made here.
+     * With the rpc down and nothing seeded the device answers out of its own
+     * empty memory instead, and mirroring that would replace the app's only
+     * durable copy with settings it never read.
+     *
+     * The mirror is written from the read-back and never from the value just
+     * pushed: `DeviceLocal.SetDnsResolverSettings` returns without persisting
+     * when the settings are nil or the mux is disabled, so the app cannot take
+     * its own write as accepted. Connected, the read-back is the extension's
+     * answer to the edit -- a dropped edit mirrors as the settings still in
+     * force; disconnected, it is the edit the remote queued, which is what the
+     * next connect applies.
+     *
+     * `getConnected()` is GetRemoteConnected; swift's importer strips the
+     * redundant "Remote".
+     */
+    private func update(afterEdit: Bool = false) {
         guard let device = self.device else {
             return
         }
         if let sdkSettings = device.getDnsResolverSettings() {
             settings = DnsSettings(sdkSettings)
+            if afterEdit || device.getConnected() {
+                persist(sdkSettings)
+            }
         } else {
             settings = nil
         }
     }
 
+    /**
+     * Writes the settings to the app's own local state.
+     *
+     * Always the sdk object read back from the device rather than the
+     * `DnsSettings` projection, so the fields this app does not model (the
+     * upgrade mask address) survive the mirror.
+     *
+     * nil is never written: it takes the store's delete branch, and a deleted
+     * store reads back as never configured, which is what
+     * `DeviceManager.initDevice` reads as "leave the extension's resolver
+     * alone". A device that reports no settings therefore mirrors nothing,
+     * while settings with everything turned off are a real value -- the user
+     * emptied the form -- and are mirrored as one.
+     */
+    private func persist(_ sdkSettings: SdkDnsResolverSettings) {
+        do {
+            try localState?.setDnsResolverSettings(sdkSettings)
+        } catch {
+            print("[DnsSettingsStore]failed to persist dns resolver settings: \(error.localizedDescription)")
+        }
+    }
+
     func apply(_ newSettings: DnsSettings) {
         guard let device = self.device else {
             return
         }
         device.setDnsResolverSettings(newSettings.toSdk())
-        update()
+        update(afterEdit: true)
     }
 }

app/networkTests/DnsResolverSettingsMirrorTests.swift

@@ -0,0 +1,125 @@
+//
+//  DnsResolverSettingsMirrorTests.swift
+//  networkTests
+//
+//  Covers the store the dns resolver settings are mirrored into.
+//
+//  The settings themselves live in the network extension's container, which
+//  the app process cannot read, so this mirror is the app's only durable copy
+//  -- it is what the editor opens on with the tunnel down and what
+//  DeviceManager.initDevice seeds the next device from.
+//
+//  The distinction the fix would be silently wrong without: writing nil
+//  DELETES the store, and a deleted store reads back exactly like one that was
+//  never written, so "the user turned everything off" and "this install has
+//  never mirrored" are the same value unless an emptied form is written as a
+//  settings object. Read as never-mirrored it would restore the resolver the
+//  user just cleared; read as emptied it would push a blank resolver over the
+//  one the extension still holds. DnsSettingsStore.persist and the seed's
+//  `if let` both rest on that line.
+//
+
+import Testing
+import Foundation
+import URnetworkSdk
+@testable import URnetwork
+
+struct DnsResolverSettingsMirrorTests {
+
+    /// Each test gets its own storage home; the SDK roots the store at
+    /// `<home>/.by`, so a fresh temporary directory is a fresh store.
+    private func withLocalState(_ body: (SdkLocalState) throws -> Void) rethrows {
+        let home = FileManager.default.temporaryDirectory
+            .appendingPathComponent("DnsResolverSettingsMirrorTests-\(UUID().uuidString)")
+        try? FileManager.default.createDirectory(at: home, withIntermediateDirectories: true)
+        defer { try? FileManager.default.removeItem(at: home) }
+
+        guard let asyncLocalState = SdkAsyncLocalState(home.path()),
+              let localState = asyncLocalState.getLocalState() else {
+            Issue.record("could not open a local state under \(home.path())")
+            return
+        }
+        defer { asyncLocalState.close() }
+        try body(localState)
+    }
+
+    /// a configuration with both a toggle set and servers in more than one
+    /// list, so a round trip that drops either shows up
+    private var configured: DnsSettings {
+        var settings = DnsSettings()
+        settings.enableRemoteDoh = true
+        settings.enableLocalDns = true
+        settings.enableFallback = true
+        settings.remoteDohUrlsIpv4 = ["https://1.1.1.1/dns-query", "https://8.8.8.8/dns-query"]
+        settings.remoteDohUrlsIpv6 = ["https://[2606:4700:4700::1111]/dns-query"]
+        settings.localDnsIpv4 = ["192.168.1.1"]
+        settings.localDnsIpv6 = ["fe80::1"]
+        return settings
+    }
+
+    /// Every field the editor writes and the connect drawer summarizes --
+    /// settings that round-trip without them render as a resolver the user
+    /// never configured.
+    @Test func settingsRoundTripWithTheirTogglesAndServers() throws {
+        try withLocalState { localState in
+            let configured = self.configured
+
+            try localState.setDnsResolverSettings(configured.toSdk())
+
+            let read = try #require(localState.getDnsResolverSettings())
+            #expect(DnsSettings(read) == configured)
+        }
+    }
+
+    /// The mirror copies the device's own settings object rather than the
+    /// DnsSettings projection the editor renders, so fields the app never
+    /// models have to survive it too.
+    @Test func fieldsTheAppDoesNotModelSurviveTheRoundTrip() throws {
+        try withLocalState { localState in
+            let sdkSettings = configured.toSdk()
+            sdkSettings.dnsUpgradeMaskAddress = "10.64.0.1"
+
+            try localState.setDnsResolverSettings(sdkSettings)
+
+            let read = try #require(localState.getDnsResolverSettings())
+            #expect(read.dnsUpgradeMaskAddress == "10.64.0.1")
+        }
+    }
+
+    /// "I turned everything off" -- an emptied form is a real value and has to
+    /// come back as one, or the seed skips it and the extension's resolver
+    /// stands.
+    @Test func settingsWithNothingEnabledRoundTripAsAValue() throws {
+        try withLocalState { localState in
+            try localState.setDnsResolverSettings(configured.toSdk())
+            try localState.setDnsResolverSettings(DnsSettings().toSdk())
+
+            let read = try #require(localState.getDnsResolverSettings())
+            #expect(DnsSettings(read) == DnsSettings())
+        }
+    }
+
+    /// Writing nil is the delete branch. DnsSettingsStore.persist must never
+    /// reach it: the result is indistinguishable from an install that never
+    /// mirrored.
+    @Test func writingNilDeletesTheStore() throws {
+        try withLocalState { localState in
+            try localState.setDnsResolverSettings(configured.toSdk())
+            #expect(localState.getDnsResolverSettings() != nil)
+
+            try localState.setDnsResolverSettings(nil)
+
+            #expect(localState.getDnsResolverSettings() == nil)
+        }
+    }
+
+    /// The upgrade guard. A build that has never mirrored reads nil, so
+    /// DeviceManager.initDevice queues nothing and the resolver a pre-fix build
+    /// left in the extension is still there on the first launch after this
+    /// ships -- rather than being replaced by a blank one.
+    @Test func aStoreThatWasNeverWrittenReadsNilRatherThanEmptySettings() throws {
+        try withLocalState { localState in
+            #expect(localState.getDnsResolverSettings() == nil)
+        }
+    }
+}