-
Notifications
You must be signed in to change notification settings - Fork 3
Sign release build with distribution certificate #150
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
Conversation
|
If I run The signature used seems to be an Though I remember that it's not the first time I encounter this error, in fact I had a And checking the signature confirms it uses the expected one: So |
The name of that target was not super clear, and looking at the implementation we can see it has some `xcode-143` hardcoded value not up-to-date and confusing (as it will not match what's in the VM at the end of this command nowadays…). Given this feels like a target that is a bit too-specific (i.e. if we go that route why wouldn't we create Make targets for every single `hostmgr` command to test too?) and in practice it's easier to just run `make build-debug` then run the command we want to test manually afterwards so we can adjust it to what we want to test, it made more sense to me to remove it to avoid adding confusion in the Makefile.
To not require the ASC API Key when in `readonly` mode
Those only call the corresponding fastlane lanes and nothing esle; but it helps discoverability Note that I deliberately didn't add those recipes as dependency to the `build:`/`build-debug:`/`build-helper-debug:` recipes, despite those recipes indeed needing the code signing identities to have been fetched and be present in the keychain for `codesign` to work as expected. This is because those fastlane lanes require the `MATCH_S3_ACCESS_KEY` and `MATCH_S3_SECRET_ACCESS_KEY` env vars to be declared locally for them to run, but we don't usually have those set in all our shell sessions unless we `export` them explicitly first. Conversely, we usually already have the necessary certificates in our keychain already when we're working on `hostmgr` so there's no need to re-fetch them systematically from S3 every single time we want to `build` or `build-debug`.
|
@mokagio I took the liberty to push a couple of commits to fix some nitpicks:
PS: When calling This is a known issue in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After applying my changes and pushing a couple of commits (see previous comment), I tested the following:
- Ran
make fetch-codesigning(after exporting bothMATCH_S3_ACCESS_KEYandMATCH_S3_SECRET_ACCESS_KEYin my env first) and validated it fetched the latest certs - Ran
make clean buildand validated viacodesign -d -vv …that bothhostmgrandhostmgr-helperwere signed correctly with release cert - Ran
make clean build-debug build-helper-debug- At first I ran into the same issue as you with
ambiguouserror duringcodesign. Indeed, after I checked in my Keychain, I had 2 of those certificates there (one expiring in… April 2026 I think? And the other expiring in May). - So I deleted the older one to only keep the one expiring in May 2026, then retried, and this time it passed.
- Then I validated via
codesign -d -vv …that bothhostmgrandhostmgr-helperwere signed correctly with dev cert
- At first I ran into the same issue as you with
Makefile
Outdated
| @echo "--- Building and Signing hostmgr for Local Development" | ||
| swift build | ||
| codesign --entitlements Sources/hostmgr/hostmgr.entitlements -s "${CERTIFICATE_NAME_DEBUG}" .build/arm64-apple-macosx/debug/hostmgr -v | ||
| codesign --entitlements Sources/hostmgr/hostmgr.entitlements -s "${CERTIFICATE_NAME_DEBUG}" .build/arm64-apple-macosx/debug/hostmgr --force --verbose |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also +1 on using the verbose flag name instead of the short, much clearer when reading the code.
|
Thanks for the enhancements and fixes @AliSoftware |
Automattic/wordpress-rs#966 reminded us that
hostmgrhas always been signed only with the development certificate.Evidently, this is not an issue for using the tool, but I've always felt it would have been more appropriate to use the distribution certificate for a release-level build. This PR does that.
CI runs
make verify-signing, which validates the change.I also verified
make build, which now uses the distribution certificate, locally:I also verified
make build-debuglocally, to make sure the interpolation of the old certificate is valid. This failed for me because I have two certificates in my machine and somehow I can't delete either from the keychain:The fact we have two development certificates is not okay, even though it doesn't seem to be a problem for any of our apps. I logged https://linear.app/a8c/issue/AINFRA-1500/only-have-one-development-certificate-in-the-automattic-developer for this.