-
Notifications
You must be signed in to change notification settings - Fork 5
chore: run coder connect networking from launchdaemon #203
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
49d5c99 to
72071e5
Compare
72071e5 to
c7dbde8
Compare
c7dbde8 to
ef8832a
Compare
1737580 to
16c716d
Compare
ef8832a to
e32d7de
Compare
| guard let proxy = conn.remoteObjectProxyWithErrorHandler({ err in | ||
| self.logger.error("failed to connect to HelperXPC \(err.localizedDescription, privacy: .public)") | ||
| continuation.resume(throwing: err) | ||
| }) as? HelperAppXPCInterface else { | ||
| self.logger.error("failed to get proxy for HelperXPC") | ||
| continuation.resume(throwing: XPCError.wrongProxyType) | ||
| return | ||
| } | ||
| proxy.ping { | ||
| self.logger.info("Connected to Helper over XPC") | ||
| continuation.resume() | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important to note that I've refactored all the XPC connections to use this pattern. With this, you're guaranteed that either the the XPC reply will be run (proxy.ping { reply } in this case) or the [...]WithErrorHandler callback.
| // /var/root/Downloads | ||
| private let dest = FileManager.default.urls(for: .downloadsDirectory, in: .userDomainMask) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Temporary. I've put it in /var/root/Library/Application\ Support/com.coder.Coder-Desktop/ as part of the PR that downloads the slim binary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The XPC code seems a lot nicer but the type names and directions of the XPC types are hard to understand
eebf562 to
291e5a1
Compare
b0c196f to
b81afc9
Compare
7b9491c to
6a93fac
Compare
b81afc9 to
e96075e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The class is called HelperXPCClient, but you can't have multiple Swift files with the same name. So, I've prepended NE, since this is the HelperXPCClient that runs within the network extension.
6eeb8aa to
c8ecda2
Compare
a4b58e5 to
bd905ae
Compare
c8ecda2 to
21a8db1
Compare
bd905ae to
33931d6
Compare
21a8db1 to
f4ebbbf
Compare
33931d6 to
0999089
Compare
f4ebbbf to
04dd34b
Compare
0999089 to
1453e77
Compare
04dd34b to
1e9fe08
Compare
1453e77 to
d09250b
Compare
1e9fe08 to
b535a7d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR reworks the XPC architecture to move VPN networking functionality from the network extension to a privileged helper daemon. The helper now manages the VPN tunnel and communicates with both the GUI app and network extension via separate XPC interfaces, implementing a more secure and maintainable design.
Key changes:
- Moved VPN networking code from network extension to privileged helper daemon
- Established bidirectional XPC communication between helper, app, and network extension
- Updated project configuration to support the new architecture
Reviewed Changes
Copilot reviewed 18 out of 20 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| project.yml | Updated build configuration to support helper dependencies and framework loading |
| XPC.swift | Defined new XPC interfaces for helper-app and helper-network extension communication |
| Download.swift | Renamed SignatureValidator class to Validator |
| main.swift | Simplified network extension entry point, removed XPC listener setup |
| PacketTunnelProvider.swift | Refactored to delegate VPN operations to helper via XPC |
| NEHelperXPCClient.swift | New XPC client for network extension to communicate with helper |
| Manager.swift | Moved to helper, updated to work without direct PacketTunnelProvider dependency |
| HelperXPCListeners.swift | New XPC server implementations for helper to handle app and network extension connections |
| AppHelperXPCClient.swift | New XPC client for GUI app to communicate with helper |
d09250b to
d286679
Compare
b535a7d to
493701d
Compare
Merge activity
|
d286679 to
d9c0210
Compare
With the changes made in #203, it now takes a moment longer to receive the first progress update, when we either start the download (if not already downloaded), or validate the dylib. To address this, the progress indicator will immediately start making progress towards 25%. This prevents it from appearing stuck in what is an expected situation. https://github.com/user-attachments/assets/da57270d-a50b-49ab-9e53-ae02368c71dc

Continues to address #201.
This PR reworks all XPC connections, such that the networking code runs within the privileged helper, instead of the network extension.
The XPC interfaces are described in
XPC.swift, and roughly follow this sequence diagram:(One difference is that we don't posix spawn the tunnel in this PR)
sequenceDiagram note left of App: User requests to start VPN: App->>+NetExt: Start VPN NetExt->>+PrivHelper: Request start VPN with TUN FD note right of PrivHelper: Privileged helper downloads and verifies binary. PrivHelper->>Tunnel: posix_spawn child process with FDs PrivHelper->>+Tunnel: Send proto start request Tunnel-->>-PrivHelper: Send proto start response PrivHelper->>+NetExt: Request for network config change NetExt-->>-PrivHelper: Response for network config change PrivHelper-->>-NetExt: Start VPN respons NetExt-->>-App: VPN started App->>PrivHelper: Request peer state PrivHelper->>Tunnel: Request peer state Tunnel-->>PrivHelper: Peer state response PrivHelper-->>App: Peer state response note left of App: Tunnel updates (bypass NetExt): Tunnel->>PrivHelper: Tunnel update proto message PrivHelper->>App: Tunnel update proto message note left of App: User requests to stop VPN: App->>+NetExt: Stop VPN NetExt->>+PrivHelper: Request stop VPN PrivHelper->>+Tunnel: Request stop VPN Tunnel-->>-PrivHelper: Stop VPN response note right of Tunnel: Tunnel binary exits PrivHelper-->>-NetExt: Stop VPN response NetExt-->>-App: VPN stoppedOf note is that the network extension starts and stops the daemon running within the privileged helper.
This is to support starting and stopping the VPN from the toggle in System Settings, and to ensure the "Connecting" and "Disconnecting" phase of the system VPN is indicative of the time the VPN is actually setting itself up and tearing itself down.
To accomplish this, the privileged helper listens on two different service names. One is connected to by the app, the other the network extension. (Once an XPC listener is connected to, communication is bidirectional)