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

ci: Notarize MacOS Binaries and Add Flat Package Installers #3571

Merged
merged 4 commits into from
Feb 27, 2022

Conversation

chipbuster
Copy link
Contributor

@chipbuster chipbuster commented Feb 7, 2022

Description

This PR adds notarization, codesigning, and flat packages (.pkg) to our deploy workflow.

It also adds internal documentation for the procedure, since it was so stupidly complicated.

Motivation and Context

Closes #1774

Screenshots (if appropriate):

image

(Unfortunately, it looks like the logos don't show up in dark mode, for any flat installers)

How Has This Been Tested?

This was tested by using a dummy job (consisting of part of the github_build job from the deploy workflow) to build simulated build artifacts, then placing the new job after that as a dependent workflow.

There is no guarantee that this will work in the real deploy workflow, but I think this is the best that I can do.

To see the GitHub workflow that I (believe) verified that this was basically-working, see https://github.com/starship/starship/blob/c3932b327a2bec9e1e919f17cf4934e96cf9f1a8/.github/workflows/apple_deploy.yml.

To see the artifacts generated by that workflow, see https://github.com/starship/starship/actions/runs/1843775397 (will expire approximately at the end of Feb).

Checklist:

  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.

@chipbuster chipbuster force-pushed the apple-notarize branch 2 times, most recently from 0bcff4f to f7a2afb Compare February 13, 2022 04:27
@chipbuster chipbuster marked this pull request as ready for review February 14, 2022 22:44
@chipbuster
Copy link
Contributor Author

I apologize for the somewhat horrific mess of commits involved here. There are interesting implications to only being able to test code by pushing.

@chipbuster chipbuster requested a review from a team February 14, 2022 22:44
@chipbuster chipbuster changed the title ci: Notarize MacOS Binaries ci: Notarize MacOS Binaries and Add Flat Package Installers Feb 14, 2022
@davidkna
Copy link
Member

I'm getting a "starship cannot run because it is from an unidentified developer" prompt when running starship via starship-aarch64-apple-darwin.tar.gz from the linked workflow artifacts. Is that supposed to happen?

I think it might be to test the current version of the workflow with a workflow_dispatch trigger.

@chipbuster
Copy link
Contributor Author

chipbuster commented Feb 15, 2022

I'm getting a "starship cannot run because it is from an unidentified developer" prompt when running starship via starship-aarch64-apple-darwin.tar.gz from the linked workflow artifacts. Is that supposed to happen?

No, I don't think that's supposed to happen. In fact, I don't know why the workflow is generating these artifacts at all, since I'm not uploading them from the workflow.

However, it would probably be best to modify this action to re-upload the binaries, since I codesign them as part of this workflow, and I don't think Gatekeeper will attempt to verify notarization for unsigned binaries. (I was originally focused on getting signed .pkg files out, but it's probably good to distribute signed binaries as well while we're doing this work).

I think it might be to test the current version of the workflow with a workflow_dispatch trigger.

From my understanding, we can just keep the trigger permanently embedded this into the deploy workflow?

@davidkna
Copy link
Member

From my understanding, we can just keep the trigger permanently embedded this into the deploy workflow?

I think so.

@chipbuster
Copy link
Contributor Author

Okay. I'll need to do some thinking on how to best do this, since I think it's dangerous to allow all steps of the deploy workflow to fire on a test (in particular, things like pushing to the docs website and publishing to cargo on a test workflow seem dicey).

My current inclination is to do something like this:

  • If a version tag is present, fire all workflow jobs
  • Else, an input string is provided to specify which jobs in the workflow to fire
  • If public external state is modified by the job (e.g. cargo deploy, gh release), then both the string to specify this job and an additional string like "DEPLOY_OVERRIDE" must be provided in order to fire the job.

@chipbuster chipbuster force-pushed the apple-notarize branch 2 times, most recently from baf68fd to 8936794 Compare February 22, 2022 22:48
@chipbuster
Copy link
Contributor Author

Rebased to get rid of some nasty shenanigans where my fork's master branch somehow picked up a bunch of unrelated PRs.

(Also to get rid of the 50+ commits which were starting to hurt when I looked at them).

Copy link
Member

@matchai matchai left a comment

Choose a reason for hiding this comment

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

Looks great (as a person with zero experience with signing and notarization)!
Thank you for taking on this huge task!!

P12_PASSWORD: ${{ secrets.APPLEDEV_SIGNKEY_PASS }}
KEYCHAIN_PASSWORD: ${{ secrets.APPLEDEV_SIGNKEY_PASS }}
APPLEID_USERNAME: ${{ secrets.APPLEDEV_ID_NAME }}
APPLEID_TEAMID: ${{ secrets.APPLEDEV_TEAM_ID }}
Copy link
Member

@matchai matchai Feb 27, 2022

Choose a reason for hiding this comment

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

Out of curiosity, will this be signed as Starship or as you?
(Also, please please don't hesitate to expense this to our OpenCollective)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is going to be signed by me. We can get signing certificates for at least one other mission control member as well, in case I get kidnapped by cephalopods or hit by a bus, but Apple has some rather high requirements for enrolling an organization: in particular, the organization needs to have legal entity status (be capable of sued) and a DUNS number.

Also, I believe the signing certificate I exported was due to expire in 2027, so if it happens in the next couple years, you should have quite a bit of time to work out a replacement :)

Copy link
Member

Choose a reason for hiding this comment

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

I'm not at my computer right now, but we may qualify as part of our Open Collective fiscal sponsorship, which qualifies us as a non-profit in the US, which may give us that legal status.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh interesting. If that's the case, I believe you would have to be the one to do the registration, since another requirement is

As the person enrolling your organization in the Apple Developer Program, you must have the legal authority to bind your organization to legal agreements. You must be the organization’s owner/founder, executive team member, senior project lead, or an employee with legal authority granted to you by a senior employee.

Once that's done though, I believe individuals should be able to acquire signing keys that work on behalf of the organization.

I don't believe that signing keys from an organization would fundamentally be any different than those from an individual (the organization ID + signing key IDs would have to change in the GH secrets, but that's about it) so switching over if we decide to go for an Apple Dev organization shouldn't be too much of an issue.

arch="$3"
pkgname="${4:-}"

if [[ ! -d "$starship_docs_dir/.vuepress/dist" ]]; then
Copy link
Member

Choose a reason for hiding this comment

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

Nice 👍

mkdir -p "$pkgdir/usr/local/bin"
cp "$starship_program_file" "$pkgdir/usr/local/bin/starship"

# Now we get to make documentation! Vuepress was not designed to build locally
Copy link
Member

Choose a reason for hiding this comment

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

Oof. If it must be done! 🙃

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I was a little surprised that npm serve didn't actually yield an HTML hierarchy that could be scraped by wget. It seems to do the thing where you have a single line of JS which can be evaluated to generate the DOM for the real deal, but wget can't evaluate the JS, so it just pulls a single script.

@@ -0,0 +1,262 @@
# MacOS Codesigning Scripts
Copy link
Member

Choose a reason for hiding this comment

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

Absolutely love this documentation! 😍
Hopefully we won't need to refer to it too often 🤞

fi

if [[ -z ${KEYCHAIN_FILENAME+x} ]]; then
error "Environmental variable KEYCHAIN_ENTRY must be set."
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops.

@chipbuster
Copy link
Contributor Author

Results from a straight copy-paste of the current subsection of the deploy workflow can be found at https://github.com/starship/starship/actions/runs/1906861805 (with irrelevant artifacts deleted).

If someone could confirm that the binaries in the .tar.gz are notarized and the .pkg files are notarized, I think we'd be ready to merge.

Command to check notarization: spctl -a -vvv -t install <file>

@williamwebb
Copy link

If someone could confirm that the binaries in the .tar.gz are notarized and the .pkg files are notarized, I think we'd be ready to merge.

Command to check notarization: spctl -a -vvv -t install <file>

Both LGTM

image

@davidkna
Copy link
Member

davidkna commented Feb 27, 2022

The binaries in the tar.gz file are reported as signed and notarized. The pkg are only reported as validly signed (but with a notarized Developer ID), but there's no warning when attempting to run it and the installed binary is reported as signed and notarized.

@davidkna
Copy link
Member

Also, Finder still doesn't seem to like the binary files from the tar.gz files.

@chipbuster
Copy link
Contributor Author

Also, Finder still doesn't seem to like the binary files from the tar.gz files.

What does it do? On my system it seems to display a terminal executable and that's about it

image

@williamwebb
Copy link

When attempting to run starship directly from finder you get,

image

Which I believe is because of this,

image

@chipbuster
Copy link
Contributor Author

Oh I see. I'm not sure why, but I'm not getting that even when using the versions downloaded from the GH Action. Possibly because I codesigned something with an identical hash locally, so Gatekeeper is letting it through?

Did previous releases of starship have this issue? Not sure if there's much we can do about that at the moment since binaries still can't be stapled.

@davidkna
Copy link
Member

Did previous releases of starship have this issue?

I don't think any past release got past gatekeeper.

Also, despite the missing stapling, Terminal.app will execute it without complaints. So it's probably fine.

@chipbuster
Copy link
Contributor Author

Alright. I'd like to do one last set of tests on a clean slate Mac (my university has a few public access terminals that should work nicely) and if those all come up clean, I'll go ahead and merged this.

@chipbuster
Copy link
Contributor Author

Unfortunately, I am unable to replicate these results on any Macs I tested, including school macs, which should (in principle) be running on the strictest policy possible. I don't know what's causing the difference, but all systems I tried reported that the new binaries and pkgs are notarized and David reports that the binaries run in the Terminal without problems, so I'm going to go ahead and declare that this is working and accept new issues to the contrary.

@chipbuster
Copy link
Contributor Author

Thank you @davidkna, @matchai, @williamwebb, and @JaySoyer for ideas, verification, debugging, and review! This was a hell of a PR and I couldn't have gotten any of this done without your help.

@chipbuster chipbuster merged commit 955a0f7 into master Feb 27, 2022
@chipbuster chipbuster deleted the apple-notarize branch February 27, 2022 21:58
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.

Notarize Starship
4 participants