Skip to content

Conversation

@ethanndickson
Copy link
Member

@ethanndicksonethanndickson commented Jul 24, 2025

Continues to address #201.

I've manually tested that this change prevents binaries not signed by the Coder Apple development team from connecting to the Helper over XPC.

Most of the PR diff is me moving the validator out of Download.swift and into Validate.swift

@ethanndicksonGraphite App
Copy link
MemberAuthor

ethanndickson commented Jul 24, 2025

@ethanndicksonethanndicksonforce-pushed the ethan/networking-in-launchdaemon branch from eebf562 to 291e5a1CompareJuly 31, 2025 07:26
@ethanndicksonethanndicksonforce-pushed the ethan/networking-in-launchdaemon branch from 291e5a1 to b0c196fCompareAugust 4, 2025 02:59
@ethanndicksonethanndicksonforce-pushed the ethan/networking-in-launchdaemon branch from b0c196f to b81afc9CompareAugust 4, 2025 03:00
@ethanndicksonethanndicksonforce-pushed the ethan/networking-in-launchdaemon branch from b81afc9 to e96075eCompareAugust 4, 2025 03:03
@ethanndicksonethanndicksonforce-pushed the ethan/xpc-validation branch 2 times, most recently from be347a8 to e6a3578CompareAugust 4, 2025 07:58
@ethanndicksonethanndicksonforce-pushed the ethan/networking-in-launchdaemon branch 2 times, most recently from a4b58e5 to bd905aeCompareAugust 4, 2025 08:05
@ethanndicksonethanndicksonforce-pushed the ethan/networking-in-launchdaemon branch from bd905ae to 33931d6CompareAugust 4, 2025 08:07
Copy link

CopilotAI left a 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 enhances security by adding code signing requirements to XPC connections to prevent unauthorized binaries from connecting to the Helper service. The changes implement validation that ensures only binaries signed by the Coder Apple development team can establish XPC connections.

Key changes:

  • Refactored validation logic from Download.swift into a dedicated Validate.swift file
  • Added xpcPeerRequirement property to enforce code signing requirements on XPC connections
  • Applied code signing validation to all XPC connection points in the application

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
FileDescription
Coder-Desktop/VPNLib/Validate.swiftNew file containing extracted validation logic with added XPC peer requirement string
Coder-Desktop/VPNLib/Download.swiftRemoved validation code that was moved to Validate.swift
Coder-Desktop/VPN/NEHelperXPCClient.swiftAdded code signing requirement to XPC client connection
Coder-Desktop/Coder-DesktopHelper/HelperXPCListeners.swiftAdded code signing requirements to both XPC server listeners
Coder-Desktop/Coder-Desktop/AppHelperXPCClient.swiftAdded code signing requirement to app helper XPC client

}

guard let plistName = infoPlist[infoNameKey] as? String, plistName == expectedName else{
throw .invalidIdentifier(identifier: infoPlist[infoNameKey] as? String)
Copy link

CopilotAIAug 6, 2025

Choose a reason for hiding this comment

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

The error type should be a name-specific validation error, not invalidIdentifier. This validation is checking the bundle name, not the identifier, so it should throw a different error type or the existing invalidIdentifier case should be renamed to be more generic.

Suggested change
throw.invalidIdentifier(identifier:infoPlist[infoNameKey]as?String)
throw.invalidName(name:infoPlist[infoNameKey]as?String)

Copilot uses AI. Check for mistakes.
@ethanndicksonethanndicksonforce-pushed the ethan/networking-in-launchdaemon branch from d09250b to d286679CompareAugust 6, 2025 03:37
@ethanndicksonGraphite App
Copy link
MemberAuthor

ethanndickson commented Aug 6, 2025

Merge activity

  • Aug 6, 3:39 AM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Aug 6, 3:45 AM UTC: Graphite rebased this pull request as part of a merge.
  • Aug 6, 3:47 AM UTC: @ethanndickson merged this pull request with Graphite.

@ethanndicksonethanndickson changed the base branch from ethan/networking-in-launchdaemon to graphite-base/206August 6, 2025 03:42
@ethanndicksonethanndickson changed the base branch from graphite-base/206 to mainAugust 6, 2025 03:44
@ethanndicksonethanndickson merged commit ff169e3 into mainAug 6, 2025
4 checks passed
@ethanndicksonethanndickson deleted the ethan/xpc-validation branch August 6, 2025 03:47
Sign up for freeto 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.

2 participants

@ethanndickson@deansheather