Skip to content
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: MacOS Frameworks not signed, closes #7690 and #7710 #7691

Closed
wants to merge 8 commits into from

Conversation

tr3ysmith
Copy link
Contributor

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Docs
  • New Binding issue #___
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change?

  • Yes, and the changes were approved in issue #___
  • No (however it does fix an issue I found with notarytool, which now seems to require a team-id. I added a new env variable called APPLE_TEAM_ID for notarizing

Checklist

  • When resolving issues, they are referenced in the PR's title (e.g fix: remove a typo, closes #___, #___)
  • A change file is added if any packages will require a version bump due to this PR per the instructions in the readme.
  • I have added a convincing reason for adding this feature, if necessary

Other information

@tr3ysmith
Copy link
Contributor Author

#7690

@tr3ysmith
Copy link
Contributor Author

So this morning, I came in to build my app, and it failed signing... So I'm looking into this further, I've also submitted a support ticket with Apple to get more clarity

@tr3ysmith
Copy link
Contributor Author

Linking in this article, looks like Apple codesign can fail if extra attributes exist on files inside the bundle....
https://developer.apple.com/library/archive/qa/qa1940/_index.html

Wondering if we should add the command to remove all extra attributes on the app bundle before signing?

@tr3ysmith
Copy link
Contributor Author

Added a function to strip extra attributes and it seems to have fixed an error I saw this morning

@tr3ysmith
Copy link
Contributor Author

FROM APPLE

When embedding frameworks there are two important things to note:

  • You must copy the framework in a way that preserves symlinks. I often see problems where folks copy a framework using cp -r, which expands the symlinks, which effectively breaks the code. I generally recommend copying code with ditto.

  • The notary service requires that all code be signed with a valid Developer ID, so it’s possible to build an app from components signed by different developers. My general advice, however, is that you re-sign any nested code with your Developer ID. Sign each code item separately, from the inside out.

@tr3ysmith tr3ysmith changed the title fix: MacOS Frameworks not signed, closes #7690 fix: MacOS Frameworks not signed, closes #7690 and #7710 Aug 29, 2023
@tr3ysmith
Copy link
Contributor Author

#7710

@tr3ysmith
Copy link
Contributor Author

So in testing, I think this is actually all good to go for now. I think we need to ultimately address assigning specific entitlement.plist files to each included binary in the app, both sidecar and frameworks. Apple recommends not using the same plist file for everything as it could break stuff. We should probably also look at removing the --deep argument on the codesign tool in the future and manually signing stuff

@tr3ysmith
Copy link
Contributor Author

@FabianLars if this pull request gets approved, we'll need to add APPLE_TEAM_ID as an argument to the GitHub action once its released, since it seems like notarytool requires it now.

@tr3ysmith
Copy link
Contributor Author

@FabianLars I've added signing for sidecar binaries, but I haven't been able to fully test it as I don't have a sidecar project setup. I've also removed --deep in this latest commit

@lucasfernog
Copy link
Member

@tr3ysmith I see you added a fix for 3 separate issues in this PR (and you also added a breaking change with the APPLE_TEAM_ID environment variable). Can you break it into 3 different PRs? it'll also help our auditing team.

Thank you ❤️

@lucasfernog
Copy link
Member

@FabianLars if this pull request gets approved, we'll need to add APPLE_TEAM_ID as an argument to the GitHub action once its released, since it seems like notarytool requires it now.

From my tests the team id isn't required. We also already merged notarytool usage into 1.x.

@tr3ysmith
Copy link
Contributor Author

@lucasfernog ah sounds good, so I can adjust this to allow team id but not require it.

I'll close this PR and open 3 separate ones.

@tr3ysmith tr3ysmith closed this Sep 7, 2023
@tr3ysmith
Copy link
Contributor Author

#7773 - add rpath
#7774 - fix codesigning
#7775 - Apple Team ID

@tr3ysmith tr3ysmith deleted the fix-framework-codesign branch September 7, 2023 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 📬Proposal
Development

Successfully merging this pull request may close these issues.

2 participants