From 828d7c04d4a264c3c067ee0ee2f0b4839c7f17c7 Mon Sep 17 00:00:00 2001
From: Ethan Dickson <ethan@coder.com>
Date: Tue, 13 May 2025 12:41:08 +1000
Subject: [PATCH 1/3] fix: manually upgrade network extension

---
 .../Coder-Desktop/VPN/NetworkExtension.swift  |   3 +-
 .../Coder-Desktop/VPN/VPNService.swift        |   3 +-
 .../VPN/VPNSystemExtension.swift              | 124 ++++++++++++------
 .../Coder-Desktop/Views/VPN/VPNMenu.swift     |  19 ++-
 .../Coder-Desktop/Views/VPN/VPNState.swift    |   6 +-
 5 files changed, 114 insertions(+), 41 deletions(-)

diff --git a/Coder-Desktop/Coder-Desktop/VPN/NetworkExtension.swift b/Coder-Desktop/Coder-Desktop/VPN/NetworkExtension.swift
index 660ef37d..7c90bd5d 100644
--- a/Coder-Desktop/Coder-Desktop/VPN/NetworkExtension.swift
+++ b/Coder-Desktop/Coder-Desktop/VPN/NetworkExtension.swift
@@ -58,8 +58,9 @@ extension CoderVPNService {
             try await tm.saveToPreferences()
             neState = .disabled
         } catch {
+            // This typically fails when the user declines the permission dialog
             logger.error("save tunnel failed: \(error)")
-            neState = .failed(error.localizedDescription)
+            neState = .failed("Failed to save tunnel: \(error.localizedDescription). Try logging in and out again.")
         }
     }
 
diff --git a/Coder-Desktop/Coder-Desktop/VPN/VPNService.swift b/Coder-Desktop/Coder-Desktop/VPN/VPNService.swift
index c3c17738..baa16804 100644
--- a/Coder-Desktop/Coder-Desktop/VPN/VPNService.swift
+++ b/Coder-Desktop/Coder-Desktop/VPN/VPNService.swift
@@ -81,12 +81,13 @@ final class CoderVPNService: NSObject, VPNService {
     // systemExtnDelegate holds a reference to the SystemExtensionDelegate so that it doesn't get
     // garbage collected while the OSSystemExtensionRequest is in flight, since the OS framework
     // only stores a weak reference to the delegate.
-    var systemExtnDelegate: SystemExtensionDelegate<CoderVPNService>?
+    var systemExtnDelegate: SystemExtensionDelegate<CoderVPNService>!
 
     var serverAddress: String?
 
     override init() {
         super.init()
+        systemExtnDelegate = SystemExtensionDelegate(asyncDelegate: self)
     }
 
     func start() async {
diff --git a/Coder-Desktop/Coder-Desktop/VPN/VPNSystemExtension.swift b/Coder-Desktop/Coder-Desktop/VPN/VPNSystemExtension.swift
index aade55d9..a6ed431f 100644
--- a/Coder-Desktop/Coder-Desktop/VPN/VPNSystemExtension.swift
+++ b/Coder-Desktop/Coder-Desktop/VPN/VPNSystemExtension.swift
@@ -22,6 +22,35 @@ enum SystemExtensionState: Equatable, Sendable {
     }
 }
 
+let extensionBundle: Bundle = {
+    let extensionsDirectoryURL = URL(
+        fileURLWithPath: "Contents/Library/SystemExtensions",
+        relativeTo: Bundle.main.bundleURL
+    )
+    let extensionURLs: [URL]
+    do {
+        extensionURLs = try FileManager.default.contentsOfDirectory(at: extensionsDirectoryURL,
+                                                                    includingPropertiesForKeys: nil,
+                                                                    options: .skipsHiddenFiles)
+    } catch {
+        fatalError("Failed to get the contents of " +
+            "\(extensionsDirectoryURL.absoluteString): \(error.localizedDescription)")
+    }
+
+    // here we're just going to assume that there is only ever going to be one SystemExtension
+    // packaged up in the application bundle. If we ever need to ship multiple versions or have
+    // multiple extensions, we'll need to revisit this assumption.
+    guard let extensionURL = extensionURLs.first else {
+        fatalError("Failed to find any system extensions")
+    }
+
+    guard let extensionBundle = Bundle(url: extensionURL) else {
+        fatalError("Failed to create a bundle with URL \(extensionURL.absoluteString)")
+    }
+
+    return extensionBundle
+}()
+
 protocol SystemExtensionAsyncRecorder: Sendable {
     func recordSystemExtensionState(_ state: SystemExtensionState) async
 }
@@ -36,35 +65,6 @@ extension CoderVPNService: SystemExtensionAsyncRecorder {
         }
     }
 
-    var extensionBundle: Bundle {
-        let extensionsDirectoryURL = URL(
-            fileURLWithPath: "Contents/Library/SystemExtensions",
-            relativeTo: Bundle.main.bundleURL
-        )
-        let extensionURLs: [URL]
-        do {
-            extensionURLs = try FileManager.default.contentsOfDirectory(at: extensionsDirectoryURL,
-                                                                        includingPropertiesForKeys: nil,
-                                                                        options: .skipsHiddenFiles)
-        } catch {
-            fatalError("Failed to get the contents of " +
-                "\(extensionsDirectoryURL.absoluteString): \(error.localizedDescription)")
-        }
-
-        // here we're just going to assume that there is only ever going to be one SystemExtension
-        // packaged up in the application bundle. If we ever need to ship multiple versions or have
-        // multiple extensions, we'll need to revisit this assumption.
-        guard let extensionURL = extensionURLs.first else {
-            fatalError("Failed to find any system extensions")
-        }
-
-        guard let extensionBundle = Bundle(url: extensionURL) else {
-            fatalError("Failed to create a bundle with URL \(extensionURL.absoluteString)")
-        }
-
-        return extensionBundle
-    }
-
     func installSystemExtension() {
         logger.info("activating SystemExtension")
         guard let bundleID = extensionBundle.bundleIdentifier else {
@@ -75,9 +75,7 @@ extension CoderVPNService: SystemExtensionAsyncRecorder {
             forExtensionWithIdentifier: bundleID,
             queue: .main
         )
-        let delegate = SystemExtensionDelegate(asyncDelegate: self)
-        systemExtnDelegate = delegate
-        request.delegate = delegate
+        request.delegate = systemExtnDelegate
         OSSystemExtensionManager.shared.submitRequest(request)
         logger.info("submitted SystemExtension request with bundleID: \(bundleID)")
     }
@@ -90,6 +88,10 @@ class SystemExtensionDelegate<AsyncDelegate: SystemExtensionAsyncRecorder>:
 {
     private var logger = Logger(subsystem: Bundle.main.bundleIdentifier!, category: "vpn-installer")
     private var asyncDelegate: AsyncDelegate
+    // The `didFinishWithResult` function is called for both activation,
+    // deactivation, and replacement requests. The API provides no way to
+    // differentiate them. https://developer.apple.com/forums/thread/684021
+    private var state: SystemExtensionDelegateState = .installing
 
     init(asyncDelegate: AsyncDelegate) {
         self.asyncDelegate = asyncDelegate
@@ -109,9 +111,35 @@ class SystemExtensionDelegate<AsyncDelegate: SystemExtensionAsyncRecorder>:
             }
             return
         }
-        logger.info("SystemExtension activated")
-        Task { [asyncDelegate] in
-            await asyncDelegate.recordSystemExtensionState(SystemExtensionState.installed)
+        switch state {
+        case .installing:
+            logger.info("SystemExtension installed")
+            Task { [asyncDelegate] in
+                await asyncDelegate.recordSystemExtensionState(SystemExtensionState.installed)
+            }
+        case .deleting:
+            logger.info("SystemExtension deleted")
+            Task { [asyncDelegate] in
+                await asyncDelegate.recordSystemExtensionState(SystemExtensionState.uninstalled)
+            }
+            let request = OSSystemExtensionRequest.activationRequest(
+                forExtensionWithIdentifier: extensionBundle.bundleIdentifier!,
+                queue: .main
+            )
+            request.delegate = self
+            state = .installing
+            OSSystemExtensionManager.shared.submitRequest(request)
+        case .replacing:
+            logger.info("SystemExtension replaced")
+            // The installed extension now has the same version strings as this
+            // bundle, so sending the deactivationRequest will work.
+            let request = OSSystemExtensionRequest.deactivationRequest(
+                forExtensionWithIdentifier: extensionBundle.bundleIdentifier!,
+                queue: .main
+            )
+            request.delegate = self
+            state = .deleting
+            OSSystemExtensionManager.shared.submitRequest(request)
         }
     }
 
@@ -135,8 +163,30 @@ class SystemExtensionDelegate<AsyncDelegate: SystemExtensionAsyncRecorder>:
         actionForReplacingExtension existing: OSSystemExtensionProperties,
         withExtension extension: OSSystemExtensionProperties
     ) -> OSSystemExtensionRequest.ReplacementAction {
-        // swiftlint:disable:next line_length
-        logger.info("Replacing \(request.identifier) v\(existing.bundleShortVersion) with v\(`extension`.bundleShortVersion)")
+        logger.info("Replacing \(request.identifier) v\(existing.bundleVersion) with v\(`extension`.bundleVersion)")
+        // This is counterintuitive, but this function is only called if the
+        // versions are the same in a dev environment.
+        // In a release build, this only gets called when the version string is
+        // different. We don't want to manually reinstall the extension in a dev
+        // environment, because the bug doesn't happen.
+        if existing.bundleVersion == `extension`.bundleVersion {
+            return .replace
+        }
+        // To work around the bug described in
+        // https://github.com/coder/coder-desktop-macos/issues/121,
+        // we're going to manually reinstall after the replacement is done.
+        // If we returned `.cancel` here the deactivation request will fail as
+        // it looks for an extension with the *current* version string.
+        // There's no way to modify the deactivate request to use a different
+        // version string (i.e. `existing.bundleVersion`).
+        logger.info("App upgrade detected, replacing and then reinstalling")
+        state = .replacing
         return .replace
     }
 }
+
+enum SystemExtensionDelegateState {
+    case installing
+    case replacing
+    case deleting
+}
diff --git a/Coder-Desktop/Coder-Desktop/Views/VPN/VPNMenu.swift b/Coder-Desktop/Coder-Desktop/Views/VPN/VPNMenu.swift
index 83757efd..89365fd3 100644
--- a/Coder-Desktop/Coder-Desktop/Views/VPN/VPNMenu.swift
+++ b/Coder-Desktop/Coder-Desktop/Views/VPN/VPNMenu.swift
@@ -81,6 +81,21 @@ struct VPNMenu<VPN: VPNService, FS: FileSyncDaemon>: View {
                     }.buttonStyle(.plain)
                     TrayDivider()
                 }
+                // This shows when
+                // 1. The user is logged in
+                // 2. The network extension is installed
+                // 3. The VPN is unconfigured
+                // It's accompanied by a message in the VPNState view
+                // that the user needs to reconfigure.
+                if state.hasSession, vpn.state == .failed(.networkExtensionError(.unconfigured)) {
+                    Button {
+                        state.reconfigure()
+                    } label: {
+                        ButtonRowView {
+                            Text("Reconfigure VPN")
+                        }
+                    }.buttonStyle(.plain)
+                }
                 if vpn.state == .failed(.systemExtensionError(.needsUserApproval)) {
                     Button {
                         openSystemExtensionSettings()
@@ -128,7 +143,9 @@ struct VPNMenu<VPN: VPNService, FS: FileSyncDaemon>: View {
         vpn.state == .connecting ||
             vpn.state == .disconnecting ||
             // Prevent starting the VPN before the user has approved the system extension.
-            vpn.state == .failed(.systemExtensionError(.needsUserApproval))
+            vpn.state == .failed(.systemExtensionError(.needsUserApproval)) ||
+            // Prevent starting the VPN without a VPN configuration.
+            vpn.state == .failed(.networkExtensionError(.unconfigured))
     }
 }
 
diff --git a/Coder-Desktop/Coder-Desktop/Views/VPN/VPNState.swift b/Coder-Desktop/Coder-Desktop/Views/VPN/VPNState.swift
index 64c08568..23319020 100644
--- a/Coder-Desktop/Coder-Desktop/Views/VPN/VPNState.swift
+++ b/Coder-Desktop/Coder-Desktop/Views/VPN/VPNState.swift
@@ -17,6 +17,10 @@ struct VPNState<VPN: VPNService>: View {
                 Text("Sign in to use Coder Desktop")
                     .font(.body)
                     .foregroundColor(.secondary)
+            case (.failed(.networkExtensionError(.unconfigured)), _):
+                Text("The system VPN requires reconfiguration.")
+                    .font(.body)
+                    .foregroundStyle(.secondary)
             case (.disabled, _):
                 Text("Enable Coder Connect to see workspaces")
                     .font(.body)
@@ -38,7 +42,7 @@ struct VPNState<VPN: VPNService>: View {
                     .padding(.horizontal, Theme.Size.trayInset)
                     .padding(.vertical, Theme.Size.trayPadding)
                     .frame(maxWidth: .infinity)
-            default:
+            case (.connected, true):
                 EmptyView()
             }
         }

From 7e61eb926128a4b41952672d14b7ed4bc8109030 Mon Sep 17 00:00:00 2001
From: Ethan Dickson <ethan@coder.com>
Date: Wed, 14 May 2025 13:38:45 +1000
Subject: [PATCH 2/3] review

---
 .../Coder-Desktop/VPN/VPNService.swift        |  3 +-
 .../VPN/VPNSystemExtension.swift              | 53 +++++++++++--------
 2 files changed, 32 insertions(+), 24 deletions(-)

diff --git a/Coder-Desktop/Coder-Desktop/VPN/VPNService.swift b/Coder-Desktop/Coder-Desktop/VPN/VPNService.swift
index baa16804..c3c17738 100644
--- a/Coder-Desktop/Coder-Desktop/VPN/VPNService.swift
+++ b/Coder-Desktop/Coder-Desktop/VPN/VPNService.swift
@@ -81,13 +81,12 @@ final class CoderVPNService: NSObject, VPNService {
     // systemExtnDelegate holds a reference to the SystemExtensionDelegate so that it doesn't get
     // garbage collected while the OSSystemExtensionRequest is in flight, since the OS framework
     // only stores a weak reference to the delegate.
-    var systemExtnDelegate: SystemExtensionDelegate<CoderVPNService>!
+    var systemExtnDelegate: SystemExtensionDelegate<CoderVPNService>?
 
     var serverAddress: String?
 
     override init() {
         super.init()
-        systemExtnDelegate = SystemExtensionDelegate(asyncDelegate: self)
     }
 
     func start() async {
diff --git a/Coder-Desktop/Coder-Desktop/VPN/VPNSystemExtension.swift b/Coder-Desktop/Coder-Desktop/VPN/VPNSystemExtension.swift
index a6ed431f..7aae356b 100644
--- a/Coder-Desktop/Coder-Desktop/VPN/VPNSystemExtension.swift
+++ b/Coder-Desktop/Coder-Desktop/VPN/VPNSystemExtension.swift
@@ -66,18 +66,8 @@ extension CoderVPNService: SystemExtensionAsyncRecorder {
     }
 
     func installSystemExtension() {
-        logger.info("activating SystemExtension")
-        guard let bundleID = extensionBundle.bundleIdentifier else {
-            logger.error("Bundle has no identifier")
-            return
-        }
-        let request = OSSystemExtensionRequest.activationRequest(
-            forExtensionWithIdentifier: bundleID,
-            queue: .main
-        )
-        request.delegate = systemExtnDelegate
-        OSSystemExtensionManager.shared.submitRequest(request)
-        logger.info("submitted SystemExtension request with bundleID: \(bundleID)")
+        systemExtnDelegate = SystemExtensionDelegate(asyncDelegate: self)
+        systemExtnDelegate!.installSystemExtension()
     }
 }
 
@@ -91,7 +81,8 @@ class SystemExtensionDelegate<AsyncDelegate: SystemExtensionAsyncRecorder>:
     // The `didFinishWithResult` function is called for both activation,
     // deactivation, and replacement requests. The API provides no way to
     // differentiate them. https://developer.apple.com/forums/thread/684021
-    private var state: SystemExtensionDelegateState = .installing
+    // This tracks the last request type made, to handle them accordingly.
+    private var action: SystemExtensionDelegateAction = .none
 
     init(asyncDelegate: AsyncDelegate) {
         self.asyncDelegate = asyncDelegate
@@ -99,6 +90,19 @@ class SystemExtensionDelegate<AsyncDelegate: SystemExtensionAsyncRecorder>:
         logger.info("SystemExtensionDelegate initialized")
     }
 
+    func installSystemExtension() {
+        logger.info("activating SystemExtension")
+        let bundleID = extensionBundle.bundleIdentifier!
+        let request = OSSystemExtensionRequest.activationRequest(
+            forExtensionWithIdentifier: bundleID,
+            queue: .main
+        )
+        request.delegate = self
+        action = .installing
+        OSSystemExtensionManager.shared.submitRequest(request)
+        logger.info("submitted SystemExtension request with bundleID: \(bundleID)")
+    }
+
     func request(
         _: OSSystemExtensionRequest,
         didFinishWithResult result: OSSystemExtensionRequest.Result
@@ -111,23 +115,24 @@ class SystemExtensionDelegate<AsyncDelegate: SystemExtensionAsyncRecorder>:
             }
             return
         }
-        switch state {
+        switch action {
         case .installing:
             logger.info("SystemExtension installed")
             Task { [asyncDelegate] in
-                await asyncDelegate.recordSystemExtensionState(SystemExtensionState.installed)
+                await asyncDelegate.recordSystemExtensionState(.installed)
             }
+            action = .none
         case .deleting:
             logger.info("SystemExtension deleted")
             Task { [asyncDelegate] in
-                await asyncDelegate.recordSystemExtensionState(SystemExtensionState.uninstalled)
+                await asyncDelegate.recordSystemExtensionState(.uninstalled)
             }
             let request = OSSystemExtensionRequest.activationRequest(
                 forExtensionWithIdentifier: extensionBundle.bundleIdentifier!,
                 queue: .main
             )
             request.delegate = self
-            state = .installing
+            action = .installing
             OSSystemExtensionManager.shared.submitRequest(request)
         case .replacing:
             logger.info("SystemExtension replaced")
@@ -138,8 +143,11 @@ class SystemExtensionDelegate<AsyncDelegate: SystemExtensionAsyncRecorder>:
                 queue: .main
             )
             request.delegate = self
-            state = .deleting
+            action = .deleting
             OSSystemExtensionManager.shared.submitRequest(request)
+        case .none:
+            logger.warning("Received an unexpected request result")
+            break
         }
     }
 
@@ -147,14 +155,14 @@ class SystemExtensionDelegate<AsyncDelegate: SystemExtensionAsyncRecorder>:
         logger.error("System extension request failed: \(error.localizedDescription)")
         Task { [asyncDelegate] in
             await asyncDelegate.recordSystemExtensionState(
-                SystemExtensionState.failed(error.localizedDescription))
+                .failed(error.localizedDescription))
         }
     }
 
     func requestNeedsUserApproval(_ request: OSSystemExtensionRequest) {
         logger.error("Extension \(request.identifier) requires user approval")
         Task { [asyncDelegate] in
-            await asyncDelegate.recordSystemExtensionState(SystemExtensionState.needsUserApproval)
+            await asyncDelegate.recordSystemExtensionState(.needsUserApproval)
         }
     }
 
@@ -180,12 +188,13 @@ class SystemExtensionDelegate<AsyncDelegate: SystemExtensionAsyncRecorder>:
         // There's no way to modify the deactivate request to use a different
         // version string (i.e. `existing.bundleVersion`).
         logger.info("App upgrade detected, replacing and then reinstalling")
-        state = .replacing
+        action = .replacing
         return .replace
     }
 }
 
-enum SystemExtensionDelegateState {
+enum SystemExtensionDelegateAction {
+    case none
     case installing
     case replacing
     case deleting

From daa06e9521d087eb4a8f53092ba9be9f7bd231de Mon Sep 17 00:00:00 2001
From: Ethan Dickson <ethan@coder.com>
Date: Wed, 14 May 2025 15:11:15 +1000
Subject: [PATCH 3/3] fmt

---
 Coder-Desktop/Coder-Desktop/VPN/VPNSystemExtension.swift | 1 -
 1 file changed, 1 deletion(-)

diff --git a/Coder-Desktop/Coder-Desktop/VPN/VPNSystemExtension.swift b/Coder-Desktop/Coder-Desktop/VPN/VPNSystemExtension.swift
index 7aae356b..39d625ca 100644
--- a/Coder-Desktop/Coder-Desktop/VPN/VPNSystemExtension.swift
+++ b/Coder-Desktop/Coder-Desktop/VPN/VPNSystemExtension.swift
@@ -147,7 +147,6 @@ class SystemExtensionDelegate<AsyncDelegate: SystemExtensionAsyncRecorder>:
             OSSystemExtensionManager.shared.submitRequest(request)
         case .none:
             logger.warning("Received an unexpected request result")
-            break
         }
     }