Keep macOS system extension in sync across upgrades, downgrades#8614
Keep macOS system extension in sync across upgrades, downgrades#8614
Conversation
# Conflicts: # lib/lantern/lantern_platform_service.dart
There was a problem hiding this comment.
Pull request overview
This PR improves macOS system extension lifecycle handling so the app can detect and reconcile upgrades, downgrades, and same-version content changes, and report richer status details back to Flutter. It also adds macOS unit test execution in CI and enforces system extension build-number monotonicity for nightly builds.
Changes:
- Add bundle-content hashing + reconciliation logic in the macOS Runner to decide when to activate vs deactivate-then-activate.
- Switch system extension status events to a structured payload (
status+ optionaldetails) and update Dart parsing/UI to consume it. - Add macOS Runner unit tests, a Makefile target to run them, and a CI script to validate sysext build monotonicity.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
macos/Runner/VPN/SystemExtensionManager.swift |
Adds reconciliation model, content hashing, and structured ExtensionStatus with details. |
macos/Runner/Handlers/SystemExtensionStatusEventHandler.swift |
Emits structured event payload (status, details) instead of a raw string. |
lib/core/models/macos_extension_state.dart |
Adds structured event parsing + new statuses (requiresReboot, updatePending) and isReady. |
lib/lantern/lantern_platform_service.dart |
Maps event stream through MacOSExtensionState.fromEvent. |
lib/features/** (vpn_switch.dart, vpn_status.dart, server_selection.dart, system_tray_notifier.dart, macos_extension_dialog.dart) |
Uses isReady and shows richer messaging/reboot handling in the dialog flow. |
macos/RunnerTests/RunnerTests.swift |
Adds unit tests for hashing and reconciliation outcomes. |
test/macos_extension_state_test.dart |
Adds Dart unit tests for structured + legacy status parsing. |
Makefile |
Adds macos-unit-tests target running xcodebuild test. |
.github/workflows/build-macos.yml |
Runs macOS unit tests and (nightly) sysext build monotonicity check. |
.github/scripts/check_macos_sysext_build.sh |
Compares current vs previous DMG sysext CFBundleVersion. |
macos/Runner.xcodeproj/project.pbxproj |
Fixes TEST_HOST path casing/name for RunnerTests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| factory MacOSExtensionState.fromEvent(Object? event) { | ||
| if (event case final Map<Object?, Object?> payload) { | ||
| return _fromStatusFields( | ||
| _stringField(payload, 'status'), | ||
| _stringField(payload, 'details'), | ||
| ); | ||
| } |
There was a problem hiding this comment.
fromEvent attempts to pattern-match event as Map<Object?, Object?>, but EventChannel(..., JSONMethodCodec()) decodes JSON objects as Map<String, dynamic> (and the unit test passes a Map<String, String>). Because Map is invariant in its type arguments, this check will fail and the code will fall back to fromString(event.toString()), producing unknown states for structured payloads. Update the type test/pattern to accept any Map (or Map<String, dynamic>) and adjust _stringField accordingly so structured events are actually parsed.
| private func submitPropertiesRequest(context: RequestContext) { | ||
| let request = OSSystemExtensionRequest.propertiesRequest( | ||
| forExtensionWithIdentifier: tunnelBundleID, | ||
| queue: .main | ||
| ) | ||
| request.delegate = self | ||
| OSSystemExtensionManager.shared.submitRequest(request) | ||
| submit(request, context: context) |
There was a problem hiding this comment.
request(_:foundProperties:) computes contentHash for the bundled and installed extensions (via SystemExtensionBundleHasher.hashBundle), and all requests are currently configured with queue: .main. Hashing walks the full bundle and reads file contents, so doing this work on the main thread risks UI stalls. Consider using a background OperationQueue for the OSSystemExtensionRequest callbacks, or offloading the hashing/reconciliation work to a background queue and only publishing status back on the main thread.
| var fileURLs: [URL] = [] | ||
| for case let fileURL as URL in enumerator { | ||
| fileURLs.append(fileURL) | ||
| } | ||
|
|
||
| /// Opens the System Settings/Preferences pane for Privacy & Security. | ||
| /// This is where the user will approve the extension. | ||
| public func openPrivacyAndSecuritySettings() { | ||
| appLogger.log("Opening Privacy & Security settings for user approval.") | ||
| // This URL scheme attempts to open the System Extensions section directly if available. | ||
| // Fallback to the general Security & Privacy pane. | ||
| let generalSecurityPaneURL = URL( | ||
| string: "x-apple.systempreferences:com.apple.preference.security" | ||
| ) | ||
| fileURLs.sort { | ||
| relativePath(for: $0, under: url) < relativePath(for: $1, under: url) | ||
| } | ||
|
|
There was a problem hiding this comment.
hashBundle loads each regular file using Data(contentsOf:options:) and accumulates all URLs before hashing. For large system extension bundles (notably the executable), this can cause significant memory and time overhead. Prefer streaming reads (e.g., FileHandle chunks) and hashing incrementally without buffering the full file list in memory (you can still keep deterministic ordering by sorting relative paths only).
| guard values?.isRegularFile == true else { | ||
| continue | ||
| } | ||
| } else { | ||
| // For macOS versions prior to 13.0 (e.g., Monterey, Big Sur) | ||
| if let url = URL( | ||
| string: "x-apple.systempreferences:com.apple.preference.security?Privacy_SystemExtensions" | ||
| ) { | ||
| NSWorkspace.shared.open(url) | ||
| } else if let fallbackUrl = generalSecurityPaneURL { | ||
| NSWorkspace.shared.open(fallbackUrl) | ||
|
|
||
| guard let data = try? Data(contentsOf: fileURL, options: [.mappedIfSafe]) else { | ||
| return nil | ||
| } | ||
|
|
||
| hasher.update(data: Data(relative.utf8)) | ||
| hasher.update(data: Data([0])) | ||
| hasher.update(data: data) | ||
| hasher.update(data: Data([0])) |
There was a problem hiding this comment.
hashBundle aborts and returns nil if any single file read fails (Data(contentsOf:)). Because matchesContent treats a nil hash as a mismatch, a transient read failure can cause the reconciler to treat same-version installs as mismatched and trigger a deactivate/activate cycle unnecessarily. Consider making hashing failure non-fatal (e.g., skip unreadable files or fall back to version-only matching) so install/reconcile decisions remain stable when hashing cannot be completed.
Closes https://github.com/getlantern/engineering/issues/3097