-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
fix(homebrew): codesign binary only on Intel macOS #3348
Conversation
FYI, in the future you don't need to close your PR and open a new one to make fixes. You can just push your changes to your PR branch and it will be picked up. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3348 +/- ##
========================================
Coverage 11.14% 11.14%
========================================
Files 99 99
Lines 17184 17184
Branches 8008 8008
========================================
Hits 1916 1916
+ Misses 12722 12581 -141
- Partials 2546 2687 +141
Flags with carried forward coverage won't be shown. Click here to find out more. |
What is this intended to fix? The issue of having to re-grant permissions for each build of sunshine will not be fixed by this, if that's the intent. That issue occurs because the ad hoc code signature changes each time the underlying binary changes; it's not because the filename is changing. To avoid having to remove and re-grant the permission each build, it's necessary to sign each build with the same Apple Developer ID certificate as the previous build, not an ad hoc certificate. Homebrew doesn't provide a way to do this, so the issue of having to remove and re-grant permissions cannot be fixed very easily. |
Also, Homebrew already signs all code it builds, at least on arm64. This pull request is not the correct solution to whatever it is trying to fix. |
The actual solution to this general problem is that Sunshine should be distributed as a proper app bundle, and signed by some trusted certificate (perhaps owned by a trusted maintainer) as part of the GitHub build process. Then the homebrew formula should be replaced by a cask that just installs the trusted binary. This will solve the following problems:
|
To summarize my comments, everything already works as well as it can be expected to work on arm64 because Homebrew already signs code on arm64. It apparently does not sign code on Intel. As a stopgap for Intel users, I suppose this pull request is fine, but I recommend that it be made specific to Intel to avoid conflicting with Homebrew's built in ad hoc codesigning. You can check for the architecture in the Homebrew formula. Long term, Sunshine should be distributed as an app bundle, but I think it's valid to have a short term fix for Intel users. |
Thank you so much for clearing that up @cathyjf that makes sense! I am running Intel, so that's probably why I needed to sign it myself. I wasn't aware about brew signing it only for arm64. I'll look into how to make it conditional to Intel and update the pull request accordingly. I'll also remove the line copying the binary. |
…out the build. added codesigning to make sure macOS asks for permissions
d48d53f
to
17ca7ab
Compare
Quality Gate passedIssues Measures |
Description
Moving binary after build and adding codesign for macOS as per issue #3340
Screenshot
Issues Fixed or Closed
Issue #3340
Type of Change
.github/...
)Checklist