Install/upgrade network extension on system wake - #810
Merged
Conversation
github-actions
Bot
added
comp/santad
Contributor
📝 WalkthroughWalkthroughAdds a forced/conditional network-extension install API and an upgrade-check for the network extension, exposes additional build dependencies, and wires a PowerMonitor callback in the daemon to trigger installation on system wake and first launch after boot. Changes
Sequence Diagram(s)sequenceDiagram
participant System as System
participant PowerMon as PowerMonitor
participant Santad as Santad Daemon
participant NEQueue as SNTNetworkExtensionQueue
participant NetExt as Network Extension
System->>PowerMon: System powers on / wakes (kHasPoweredOn)
PowerMon->>Santad: invoke wake callback
Santad->>NEQueue: installNetworkExtension(reason="System woke up")
NEQueue->>NEQueue: shouldInstallNetworkExtension?
alt Installation allowed
NEQueue->>NetExt: install/upgrade extension
NetExt-->>NEQueue: success/failure
NEQueue-->>Santad: reply(success/false)
else Installation not allowed
NEQueue-->>Santad: reply(false)
end
sequenceDiagram
participant Client as External Controller
participant DCC as SNTDaemonControlController
participant Santad as Santad Daemon
participant NEQueue as SNTNetworkExtensionQueue
Client->>DCC: RPC installNetworkExtensionForce(force=BOOL)
DCC->>Santad: request install (forward)
Santad->>NEQueue: shouldInstallNetworkExtension / networkExtensionNeedsUpgrade (if not forced)
alt Installation allowed or forced
NEQueue->>NEQueue: perform install
NEQueue-->>Santad: result YES
else Denied / not needed
NEQueue-->>Santad: result NO
end
Santad-->>DCC: return result
DCC-->>Client: reply(result)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Source/santad/SNTDaemonControlController.mm (1)
735-748: Minor:shouldInstallNetworkExtensionis checked twice in the call chain.
installNetworkExtensionForcechecksshouldInstallNetworkExtension(line 737) and then delegates toinstallNetworkExtension:reply:, which redundantly rechecks it (line 707). If the state flips between the two checks, the "not authorized"LOGIininstallNetworkExtension:reply:fires despite having already passed the check here.♻️ Proposed fix – pass `reply` directly to the internal helper
Either extract the body of
installNetworkExtension:reply:(after the authorization check) into a shared helper, or skip the authorization re-check insideinstallNetworkExtension:reply:when called via theForcepath. The simplest approach is to introduce a private_installNetworkExtensionWithReply:that skips the authorization guard, letting both public entry points call it after performing their own checks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Source/santad/SNTDaemonControlController.mm` around lines 735 - 748, installNetworkExtensionForce redundantly checks shouldInstallNetworkExtension before calling installNetworkExtension:reply: which repeats the same guard; create a private helper _installNetworkExtensionWithReply: that contains the installation logic without the authorization check, update installNetworkExtension:reply: to keep its existing shouldInstallNetworkExtension guard and call _installNetworkExtensionWithReply: when authorized, and change installNetworkExtensionForce:reply: to perform its authorization/force checks and then call _installNetworkExtensionWithReply: directly so the "not authorized" LOGI inside installNetworkExtension:reply: no longer fires erroneously.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Source/santad/SNTDaemonControlController.h`:
- Around line 44-47: Update the doc comment for
-installNetworkExtensionForce:reply: to accurately state that when force is YES
it bypasses the on-disk vs loaded version comparison but does not
unconditionally call -installNetworkExtension:; the method still respects the
shouldInstallNetworkExtension check and will short-circuit with reply(NO) if
shouldInstallNetworkExtension returns NO. Mention both behaviors (bypasses
version comparison; still consults shouldInstallNetworkExtension and may return
NO instead of invoking -installNetworkExtension:).
In `@Source/santad/SNTNetworkExtensionQueue.h`:
- Around line 56-60: Update the documentation for -[SNTNetworkExtensionQueue
networkExtensionNeedsUpgrade] to mention that the call performs a blocking XPC
check with a 5-second timeout and that if the XPC call times out it will return
YES; specifically, add a sentence to the existing comment clarifying the
5-second timeout behavior and that a timeout is treated the same as "extension
not connected" (i.e., returns YES) so readers understand the blocking/fallback
semantics for networkExtensionNeedsUpgrade.
---
Nitpick comments:
In `@Source/santad/SNTDaemonControlController.mm`:
- Around line 735-748: installNetworkExtensionForce redundantly checks
shouldInstallNetworkExtension before calling installNetworkExtension:reply:
which repeats the same guard; create a private helper
_installNetworkExtensionWithReply: that contains the installation logic without
the authorization check, update installNetworkExtension:reply: to keep its
existing shouldInstallNetworkExtension guard and call
_installNetworkExtensionWithReply: when authorized, and change
installNetworkExtensionForce:reply: to perform its authorization/force checks
and then call _installNetworkExtensionWithReply: directly so the "not
authorized" LOGI inside installNetworkExtension:reply: no longer fires
erroneously.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
Source/santad/BUILDSource/santad/SNTDaemonControlController.hSource/santad/SNTDaemonControlController.mmSource/santad/SNTNetworkExtensionQueue.hSource/santad/SNTNetworkExtensionQueue.mmSource/santad/Santad.mm
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Source/santad/SNTDaemonControlController.h (1)
44-47:⚠️ Potential issue | 🟡 MinorDoc comment still exposes an internal method name and omits the version-check bypass.
The addition of "as long as installation is authorized" correctly addresses the past concern about the comment implying an unconditional call. Two small gaps remain:
delegates to installNetworkExtension:is an implementation detail and shouldn't appear in a public API doc.- The comment doesn't state that
force=YESbypasses the version-comparison check, which is the defining behavioral difference fromforce=NO.📝 Suggested correction
-/// Install the network extension, optionally checking whether an upgrade is needed first. -/// When force is YES, delegates to installNetworkExtension: as long as installation is authorized. -/// When force is NO, skips install if the loaded version already matches the on-disk version. +/// Installs or upgrades the network extension if authorized. +/// Always skips installation when the network extension is not authorized +/// (i.e., sync v2 is disabled or enable is not set in sync settings). +/// When force is YES, bypasses the version check and triggers installation directly. +/// When force is NO, skips install if the loaded version already matches the on-disk version.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Source/santad/SNTDaemonControlController.h` around lines 44 - 47, Update the doc comment for -installNetworkExtensionForce:reply: to avoid exposing internal implementation details and to explicitly state the behavioral difference when force is YES; remove the phrase "delegates to installNetworkExtension:" and replace it with a neutral description of the behavior, and add a sentence that when force is YES the method bypasses the on-disk vs loaded version comparison (i.e., installation proceeds if authorized), whereas when force is NO the method skips installation if the loaded version already matches the on-disk version; ensure the method name -installNetworkExtensionForce:reply: is referenced only as the API symbol being documented.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@Source/santad/SNTDaemonControlController.h`:
- Around line 44-47: Update the doc comment for
-installNetworkExtensionForce:reply: to avoid exposing internal implementation
details and to explicitly state the behavioral difference when force is YES;
remove the phrase "delegates to installNetworkExtension:" and replace it with a
neutral description of the behavior, and add a sentence that when force is YES
the method bypasses the on-disk vs loaded version comparison (i.e., installation
proceeds if authorized), whereas when force is NO the method skips installation
if the loaded version already matches the on-disk version; ensure the method
name -installNetworkExtensionForce:reply: is referenced only as the API symbol
being documented.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
Source/santad/SNTDaemonControlController.h
russellhancox
approved these changes
Feb 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This adds support for automatic install/upgrade of the network extension when the system wakes up.
Part of SNT-261