Skip to content

chore: use slim binary over dylib #210

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

Open
wants to merge 1 commit into
base: ethan/xpc-validation
Choose a base branch
from

Conversation

ethanndickson
Copy link
Member

@ethanndickson ethanndickson commented Jul 30, 2025

Closes #201.

This PR:

  • Replaces the dylib download with a slim binary download.
  • Removes signature validation checks that are inappropriate for the slim binary.
  • Replaces TunnelHandle with TunnelDaemon, an abstraction which runs posix_spawn on the slim binary, and manages the spawned process.
  • Adds tests for TunnelDaemon.

In a future PR:

  • Bump the minimum server version for Coder Desktop for macOS (to v2.25 once it's released)

Copy link
Member Author

ethanndickson commented Jul 30, 2025

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@@ -69,7 +69,7 @@ actor Receiver<RecvMsg: Message> {
},
onCancel: {
self.logger.debug("async stream canceled")
self.dispatch.close()
self.dispatch.close(flags: [.stop])
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
}
}
Copy link
Member Author

@ethanndickson ethanndickson Jul 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

@ethanndickson ethanndickson Jul 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add another PR to check the version by exec'ing.

@ethanndickson ethanndickson marked this pull request as ready for review July 31, 2025 04:44
@ethanndickson ethanndickson self-assigned this Jul 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant