chore: use slim binary over dylib #210
Merged
+532
−253
Conversation
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
This was referenced Jul 30, 2025
This was referenced Jul 30, 2025
7ef8131 to
8f00c6a
Compare
547fd97 to
6687411
Compare
8f00c6a to
479576a
Compare
ethanndickson
commented
Jul 31, 2025
| @@ -69,7 +69,7 @@ actor Receiver<RecvMsg: Message> { | |||
| }, | |||
| onCancel: { | |||
| self.logger.debug("async stream canceled") | |||
| self.dispatch.close() | |||
| self.dispatch.close(flags: [.stop]) | |||
There was a problem hiding this comment.
This causes any currently running dispatch.read calls to supply their callback with an error which lets us actually end the read loop and cause the manager and all of it's children to be deallocated, including pipe file descriptors.
Previously we would leak pipes, but it didn't matter because the process would die each time.
Comment on lines
-104
to
-127
| private static func validateInfo(infoPlist: [String: AnyObject], expectedVersion: String) throws(ValidationError) { | ||
| guard let plistIdent = infoPlist[infoIdentifierKey] as? String, plistIdent == expectedIdentifier else { | ||
| throw .invalidIdentifier(identifier: infoPlist[infoIdentifierKey] as? String) | ||
| } | ||
|
|
||
| guard let plistName = infoPlist[infoNameKey] as? String, plistName == expectedName else { | ||
| throw .invalidIdentifier(identifier: infoPlist[infoNameKey] as? String) | ||
| } | ||
|
|
||
| // Downloaded dylib must match the version of the server | ||
| guard let dylibVersion = infoPlist[infoShortVersionKey] as? String, | ||
| expectedVersion == dylibVersion | ||
| else { | ||
| throw .invalidVersion(version: infoPlist[infoShortVersionKey] as? String) | ||
| } | ||
|
|
||
| // Downloaded dylib must be at least the minimum Coder server version | ||
| guard let dylibVersion = infoPlist[infoShortVersionKey] as? String, | ||
| // x.compare(y) is .orderedDescending if x > y | ||
| minimumCoderVersion.compare(dylibVersion, options: .numeric) != .orderedDescending | ||
| else { | ||
| throw .belowMinimumCoderVersion | ||
| } | ||
| } |
There was a problem hiding this comment.
Unfortunate, but because we don't build the slim binary with an external linker (previously xcode via cgo for the dylib) we can no longer create an info.plist section in the binary with -sectcreate. If we ever need to build the slim binary with cgo then we can add this back.
There was a problem hiding this comment.
I've added another PR to check the version by exec'ing.
6687411 to
ef370db
Compare
479576a to
e9e15db
Compare
e9e15db to
c7602c2
Compare
ef370db to
55319f4
Compare
c7602c2 to
f008801
Compare
8670f11 to
be347a8
Compare
f008801 to
847006f
Compare
be347a8 to
e6a3578
Compare
847006f to
d9255bc
Compare
e6a3578 to
a1864f6
Compare
d9255bc to
4f29ff3
Compare
a1864f6 to
8b4c8cd
Compare
4f29ff3 to
5840bce
Compare
8b4c8cd to
78fd6c0
Compare
5840bce to
3f7f6b6
Compare
This was referenced Aug 4, 2025
deansheather
approved these changes
Aug 4, 2025
3f7f6b6 to
42b8f0b
Compare
a5d5337 to
c450bd4
Compare
42b8f0b to
b716554
Compare
b716554 to
02b3369
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR replaces the dylib-based tunnel implementation with a slim binary approach for the VPN functionality. It removes dylib-specific code and signature validation, introducing TunnelDaemon as a new abstraction that spawns and manages a binary process.
- Replaces
TunnelHandlewithTunnelDaemonthat usesposix_spawnto run a slim binary - Updates download and validation logic to work with binaries instead of dylibs
- Removes dylib-specific signature validation and quarantine removal processes
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/update-cask.sh | Updates cleanup paths to include new binary locations |
| Coder-Desktop/project.yml | Removes bridging header reference for dylib integration |
| Coder-Desktop/VPNLibTests/TunnelDaemonTests.swift | Adds comprehensive tests for the new TunnelDaemon functionality |
| Coder-Desktop/VPNLib/XPC.swift | Updates progress descriptions from "library" to "binary" |
| Coder-Desktop/VPNLib/Validate.swift | Simplifies validation by removing dylib-specific checks |
| Coder-Desktop/VPNLib/TunnelDaemon.swift | New implementation for managing tunnel binary process |
| Coder-Desktop/VPNLib/System.swift | Adds system utilities for process spawning and monitoring |
| Coder-Desktop/Coder-DesktopHelper/TunnelHandle.swift | Removes old dylib-based tunnel implementation |
| Coder-Desktop/Coder-DesktopHelper/Manager.swift | Major refactor to use TunnelDaemon instead of TunnelHandle |
02b3369 to
a4bbf6b
Compare
c450bd4 to
557e4fe
Compare
Merge activity
|
a4bbf6b to
3267a06
Compare
ethanndickson
added a commit
that referenced
this pull request
Aug 6, 2025
This requirement is introduced by #210, as the `vpn-daemon` command on the macOS CLI isn't present on prior versions. (2.24.3 isn't out yet, but when it does come out, it'll be compatible. 2.25 is out, and it's compatible)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes #201.
This PR:
TunnelHandlewithTunnelDaemon, an abstraction which runsposix_spawnon the slim binary, and manages the spawned process.TunnelDaemon.In a future PR: