fix: stop VPN Bypass's own route churn destabilising GlobalProtect (#65)

.gitignore

@@ -44,3 +44,6 @@ dist/
 # AI tooling state
 .omc/
 .lynxprompt/
+
+# goal-loop durable state (loop-owned, not part of the product)
+.goal-loop/

Helper/HelperTool.swift

@@ -2,6 +2,14 @@
 // Privileged helper tool implementation that runs as root.
 
 import Foundation
+import os.log
+
+/// Structured logging for the privileged helper. Until now the helper logged nothing at all, so a
+/// root daemon that owns the routing table left no record of which routes it wrote or which callers
+/// it rejected. Read with:
+///   log stream --predicate 'subsystem == "com.geiserx.vpnbypass.helper"' --info
+/// Destinations/gateways are route data the caller already supplied; no credentials pass through here.
+let helperLog = Logger(subsystem: "com.geiserx.vpnbypass.helper", category: "routes")
 
 // MARK: - XPC Listener Delegate
 
@@ -104,101 +112,155 @@ class HelperToolDelegate: NSObject, NSXPCListenerDelegate {
 // MARK: - Helper Tool Implementation
 
 class HelperTool: NSObject, HelperProtocol {
-
+
+    /// Serialises every route/hosts mutation across ALL XPC connections. `HelperToolDelegate`
+    /// creates a fresh `HelperTool` per connection, and the app drops and recreates its connection
+    /// on an XPC deadline, so without this two handlers could mutate the routing table at the same
+    /// time — the condition behind the concurrent `/sbin/route` processes seen in #65.
+    /// Deliberately NOT used by `getVersion`: that is the app's liveness probe, and queueing it
+    /// behind a long batch would time it out and trigger a spurious helper reinstall.
+    private static let routeQueue = DispatchQueue(label: "com.geiserx.vpnbypass.helper.routes")
+
     // MARK: - Route Management
-    
+
     func addRoute(destination: String, gateway: String, isNetwork: Bool, withReply reply: @escaping (Bool, String?) -> Void) {
         // Validate inputs
         guard isValidDestination(destination), isValidGateway(gateway) else {
             reply(false, "Invalid destination or gateway format")
             return
         }
 
-        // First try to delete existing route (ignore result)
-        _ = executeRoute(args: ["-n", "delete", destination])
-
-        // Add the new route
-        let result = executeRoute(args: buildRouteAddArgs(destination: destination, gateway: gateway, isNetwork: isNetwork))
-        reply(result.success, result.error)
+        Self.routeQueue.async {
+            let result = self.installRoute(destination: destination, gateway: gateway, isNetwork: isNetwork)
+            reply(result.success, result.error)
+        }
     }
-    
+
     func removeRoute(destination: String, withReply reply: @escaping (Bool, String?) -> Void) {
         guard isValidDestination(destination) else {
             reply(false, "Invalid destination format")
             return
         }
-
-        let result = executeRoute(args: ["-n", "delete", destination])
-        reply(result.success, result.error)
+
+        Self.routeQueue.async {
+            let result = self.executeRoute(args: ["-n", "delete", destination])
+            if !result.success {
+                helperLog.info("delete \(destination, privacy: .public) failed: \(result.error ?? "unknown", privacy: .public)")
+            }
+            reply(result.success, result.error)
+        }
     }
 
     // MARK: - Batch Route Management (for startup/stop performance)
 
     func addRoutesBatch(routes: [[String: Any]], withReply reply: @escaping (Int, Int, [String], String?) -> Void) {
-        var successCount = 0
-        var failureCount = 0
-        var failedDestinations: [String] = []
-        var lastError: String?
-
-        for route in routes {
-            guard let destination = route["destination"] as? String,
-                  let gateway = route["gateway"] as? String else {
-                failureCount += 1
-                continue
-            }
-
-            let isNetwork = route["isNetwork"] as? Bool ?? false
+        Self.routeQueue.async {
+            var successCount = 0
+            var failureCount = 0
+            var failedDestinations: [String] = []
+            var lastError: String?
+
+            for route in routes {
+                guard let destination = route["destination"] as? String,
+                      let gateway = route["gateway"] as? String else {
+                    failureCount += 1
+                    continue
+                }
 
-            // Validate inputs
-            guard isValidDestination(destination), isValidGateway(gateway) else {
-                failureCount += 1
-                failedDestinations.append(destination)
-                continue
-            }
+                let isNetwork = route["isNetwork"] as? Bool ?? false
 
-            // First try to delete existing route (ignore result)
-            _ = executeRoute(args: ["-n", "delete", destination])
+                // Validate inputs
+                guard self.isValidDestination(destination), self.isValidGateway(gateway) else {
+                    failureCount += 1
+                    failedDestinations.append(destination)
+                    continue
+                }
 
-            // Add the new route
-            let result = executeRoute(args: buildRouteAddArgs(destination: destination, gateway: gateway, isNetwork: isNetwork))
-            if result.success {
-                successCount += 1
-            } else {
-                failureCount += 1
-                failedDestinations.append(destination)
-                lastError = result.error
+                let result = self.installRoute(destination: destination, gateway: gateway, isNetwork: isNetwork)
+                if result.success {
+                    successCount += 1
+                } else {
+                    failureCount += 1
+                    failedDestinations.append(destination)
+                    lastError = result.error
+                }
             }
-        }
 
-        reply(successCount, failureCount, failedDestinations, lastError)
+            helperLog.info("addRoutesBatch: \(routes.count, privacy: .public) requested, \(successCount, privacy: .public) ok, \(failureCount, privacy: .public) failed")
+            reply(successCount, failureCount, failedDestinations, lastError)
+        }
     }
 
     func removeRoutesBatch(destinations: [String], withReply reply: @escaping (Int, Int, [String], String?) -> Void) {
-        var successCount = 0
-        var failureCount = 0
-        var failedDestinations: [String] = []
-        var lastError: String?
-
-        for destination in destinations {
-            guard isValidDestination(destination) else {
-                failureCount += 1
-                failedDestinations.append(destination)
-                continue
+        Self.routeQueue.async {
+            var successCount = 0
+            var failureCount = 0
+            var failedDestinations: [String] = []
+            var lastError: String?
+
+            for destination in destinations {
+                guard self.isValidDestination(destination) else {
+                    failureCount += 1
+                    failedDestinations.append(destination)
+                    continue
+                }
+
+                let result = self.executeRoute(args: ["-n", "delete", destination])
+                if result.success {
+                    successCount += 1
+                } else {
+                    failureCount += 1
+                    failedDestinations.append(destination)
+                    lastError = result.error
+                }
             }
 
-            let result = executeRoute(args: ["-n", "delete", destination])
-            if result.success {
-                successCount += 1
-            } else {
-                failureCount += 1
-                failedDestinations.append(destination)
-                lastError = result.error
+            helperLog.info("removeRoutesBatch: \(destinations.count, privacy: .public) requested, \(successCount, privacy: .public) removed, \(failureCount, privacy: .public) failed")
+            reply(successCount, failureCount, failedDestinations, lastError)
+        }
+    }
+
+    /// Install one route WITHOUT the old blind `route delete` that preceded every add.
+    ///
+    /// #65: the previous `delete` + `add` pair cost two kernel mutations per route even when the
+    /// route was already correct, and — worse — it briefly REMOVED the route. Each mutation raises a
+    /// kernel route-change event, and GlobalProtect re-validates its own gateway route on every one;
+    /// its teardowns were preceded by `Failed to find route for <gateway>`, exactly what a transient
+    /// removal produces. The blind delete could also remove a route this app never owned.
+    ///
+    /// The ladder below never opens a window where the destination has no route:
+    ///   1. `change` — rewrites an existing route in place (one mutation, no gap). Succeeds in the
+    ///      re-apply case, which is the common one.
+    ///   2. `add` — only when nothing was there to change (one mutation).
+    ///   3. `delete` + `add` — last resort, only if `add` reports the route already exists (a race
+    ///      with another writer between steps 1 and 2). This is the sole path that can still open a
+    ///      gap, and it is now rare rather than universal.
+    private func installRoute(destination: String, gateway: String, isNetwork: Bool) -> (success: Bool, error: String?) {
+        let changed = executeRoute(args: buildRouteArgs(verb: "change", destination: destination, gateway: gateway, isNetwork: isNetwork))
+        if changed.success { return (true, nil) }
+
+        let added = executeRoute(args: buildRouteArgs(verb: "add", destination: destination, gateway: gateway, isNetwork: isNetwork))
+        if added.success { return (true, nil) }
+
+        // `add` refused because a route for this destination exists after all — fall back to the
+        // old replace, which is the only remaining way to converge.
+        if (added.error ?? "").localizedCaseInsensitiveContains("exists") {
+            helperLog.info("install \(destination, privacy: .public): change+add both refused, replacing")
+            let removed = executeRoute(args: ["-n", "delete", destination])
+            guard removed.success else {
+                // Report why the removal failed rather than letting the follow-up add fail with a
+                // secondary "exists" that hides the real cause.
+                helperLog.error("install \(destination, privacy: .public): delete before replace failed: \(removed.error ?? "unknown", privacy: .public)")
+                return (false, removed.error)
             }
+            let replaced = executeRoute(args: buildRouteArgs(verb: "add", destination: destination, gateway: gateway, isNetwork: isNetwork))
+            return (replaced.success, replaced.error)
         }
 
-        reply(successCount, failureCount, failedDestinations, lastError)
+        helperLog.error("install \(destination, privacy: .public) failed: \(added.error ?? "unknown", privacy: .public)")
+        return (false, added.error)
     }
-    
+
     private func executeRoute(args: [String]) -> (success: Bool, error: String?) {
         let process = Process()
         process.executableURL = URL(fileURLWithPath: "/sbin/route")
@@ -227,12 +289,22 @@ class HelperTool: NSObject, HelperProtocol {
     // MARK: - Hosts File Management
 
     func updateHostsFile(entries: [[String: String]], withReply reply: @escaping (Bool, String?) -> Void) {
+        // Serialised alongside route mutations on the same queue. This is a read-modify-write of
+        // /etc/hosts, so without it two concurrent XPC connections could read the same content and
+        // the later write would silently discard the earlier update.
+        Self.routeQueue.async {
+            let result = self.performHostsUpdate(entries: entries)
+            reply(result.success, result.error)
+        }
+    }
+
+    /// The /etc/hosts read-modify-write itself. Always call it on `routeQueue`.
+    private func performHostsUpdate(entries: [[String: String]]) -> (success: Bool, error: String?) {
         let hostsPath = "/etc/hosts"
-        
+
         // Read current hosts file
         guard let currentContent = try? String(contentsOfFile: hostsPath, encoding: .utf8) else {
-            reply(false, "Could not read /etc/hosts")
-            return
+            return (false, "Could not read /etc/hosts")
         }
 
         // Remove existing VPN-BYPASS section
@@ -294,9 +366,9 @@ class HelperTool: NSObject, HelperProtocol {
 
         do {
             try newContent.write(toFile: hostsPath, atomically: true, encoding: .utf8)
-            reply(true, nil)
+            return (true, nil)
         } catch {
-            reply(false, "Failed to write hosts file: \(error.localizedDescription)")
+            return (false, "Failed to write hosts file: \(error.localizedDescription)")
         }
     }
 
@@ -347,8 +419,10 @@ class HelperTool: NSObject, HelperProtocol {
         return name.allSatisfy { $0.isLetter || $0.isNumber } && name.count <= 16
     }
 
-    private func buildRouteAddArgs(destination: String, gateway: String, isNetwork: Bool) -> [String] {
-        var args = ["-n", "add"]
+    /// Build `route(8)` arguments for `verb` ("add" or "change"). Both take an identical argument
+    /// shape, so `installRoute` can try an in-place change before falling back to an add.
+    private func buildRouteArgs(verb: String, destination: String, gateway: String, isNetwork: Bool) -> [String] {
+        var args = ["-n", verb]
         args.append(isNetwork ? "-net" : "-host")
         args.append(destination)
         if gateway.hasPrefix("iface:") {

Helper/Info.plist

@@ -2,22 +2,19 @@
 <!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
 <plist version="1.0">
 <dict>
-    <key>CFBundleIdentifier</key>
-    <string>com.geiserx.vpnbypass.helper</string>
-    <key>CFBundleInfoDictionaryVersion</key>
-    <string>6.0</string>
-    <key>CFBundleName</key>
-    <string>VPNBypassHelper</string>
-    <key>CFBundleShortVersionString</key>
-    <string>1.8.0</string>
-    <key>CFBundleVersion</key>
-    <string>8</string>
-    <key>SMAuthorizedClients</key>
-    <array>
-        <!-- The main app that can communicate with this helper -->
-        <!-- For development, we use a relaxed requirement -->
-        <!-- In production, this should be: identifier "com.geiserx.vpn-bypass" and anchor apple generic and certificate leaf[subject.CN] = "Developer ID Application: YOUR NAME" -->
-        <string>identifier "com.geiserx.vpn-bypass"</string>
-    </array>
+	<key>CFBundleIdentifier</key>
+	<string>com.geiserx.vpnbypass.helper</string>
+	<key>CFBundleInfoDictionaryVersion</key>
+	<string>6.0</string>
+	<key>CFBundleName</key>
+	<string>VPNBypassHelper</string>
+	<key>CFBundleShortVersionString</key>
+	<string>1.9.0</string>
+	<key>CFBundleVersion</key>
+	<string>9</string>
+	<key>SMAuthorizedClients</key>
+	<array>
+		<string>identifier "com.geiserx.vpn-bypass"</string>
+	</array>
 </dict>
 </plist>

ROADMAP.md

@@ -1,13 +1,14 @@
 # VPN Bypass - Product Roadmap
 
-## Current State (v3.1.5)
+## Current State (v3.1.7)
 
 Three routing modes ship today: **Bypass** (listed domains/services skip the VPN), **VPN Only** (everything
 tunnels except listed items), and **Custom** (the multi-route epic, shipped in 3.0 — a per-rule engine mapping
 each domain/service/CIDR to a named route). Custom-mode egresses include the local gateway, a specific VPN
 interface (multi-VPN), an HTTP/SOCKS5 proxy via a local `127.0.0.1` listener, and a Tailscale-peer exit
 (proxy-over-tailnet). A bundled **`vpnb` CLI** scripts it all over a user-only socket (on `PATH` via the tap
-cask since 3.1.2), and the privileged helper is cdhash-pinned (fail-closed) and audit-token-verified (1.8.0). The whole
+cask since 3.1.2), and the privileged helper (1.9.0) is cdhash-pinned (fail-closed, since 1.8.0) and audit-token-verified
+(since 1.7.0). The whole
 engine stays **entitlement-free** — kernel routes + `/etc/hosts` + local proxy listeners + a PF/local-CA
 path, **no Network Extension** — so the app remains ad-hoc-signable.
 

Sources/VPNBypassCore/HelperProtocol.swift

@@ -87,7 +87,15 @@ struct HelperConstants {
     // self-heals a missing/stale pin by reinstalling. Bumped from 1.7.0 so existing
     // (possibly pin-less, identifier-only) helpers detect the mismatch and reinstall to
     // the fail-closed + guaranteed-pin build.
-    static let helperVersion = "1.8.0"
+    // 1.9.0: the helper no longer issues a blind `route delete` before every `route add` (#65).
+    // That cost two kernel mutations per route even when the route was already correct, and it
+    // briefly REMOVED the route — every mutation raises a kernel route-change event, and
+    // GlobalProtect re-validates its gateway route on each one, tearing down its tunnel when the
+    // lookup transiently fails. It now changes-in-place, adds only when nothing was there, and
+    // replaces only as a last resort. Route/hosts mutations are also serialised across XPC
+    // connections (concurrent handlers were producing simultaneous /sbin/route processes), and the
+    // helper finally emits os_log records. Bumped from 1.8.0 so installed helpers pick this up.
+    static let helperVersion = "1.9.0"
     static let bundleID = "com.geiserx.vpnbypass.helper"
     static let hostMarkerStart = "# VPN-BYPASS-MANAGED - START"
     static let hostMarkerEnd = "# VPN-BYPASS-MANAGED - END"