Repository navigation
fix(release): check the mac terminal host against the fork's signing team - #126
Merged
Merged
Conversation
…team v1.4.230 added a signed-team check for the packaged macOS terminal host with upstream's Developer ID team hardcoded, so every fork release build fails 'signed team is <fork team>, expected 6CX3WHS9HZ' after signing. Read the team from APPLE_TEAM_ID, which the publish step already passes for notarization, and fail a release build that omits it so the check can never silently turn off.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ELI5
Fork releases can't build on macOS right now. A check that came in with upstream v1.4.230 insists the app is signed by upstream's Apple developer account. The fork signs with its own account, so the check always fails. This makes the check expect the fork's account instead.
What Changed
config/electron-builder.config.cjs(a permanent fork exception) hardcoded upstream's Developer ID team,6CX3WHS9HZ, as the team the packaged macOS terminal host must be signed by. It now reads the team fromAPPLE_TEAM_ID.release-mac-build.ymlalready passes that variable to the publish step for notarization. A release build (ORCA_MAC_RELEASE=1) without it now throws instead of skipping the check. Local and CI builds are unchanged: they never set an expected team.Before: every fork mac release fails after signing with
[macos-terminal-host] signed team is ***, expected 6CX3WHS9HZ. That happened on run https://github.com/zpyoung/orca/actions/runs/39311063583, the build for v1.4.231-rc.0.zy01. After: the helper is checked against the team the fork's certificate and notarization credentials belong to.Why
The check is worth keeping, because it catches a helper signed by the wrong identity. Only its expected value is upstream-specific. Hardcoding the fork's team ID in the file would also work, but that ID lives in a repository secret and shows as
***in logs. Reading the variable the notarization step already depends on keeps one source of truth. Failing hard when the variable is missing means the check can never turn itself off.Linked Issue
N/A (fork release pipeline fix)
Visual Proof
N/A (build configuration only, no UI change)
Testing
node -e "require('./config/electron-builder.config.cjs')"loads cleanly, and oxlint and oxfmt pass on the file. The config tests that load in release mode are unaffected, because the team is read lazily inside the packaging hook. PR CI never runs the mac release build, so the real test is the recut of v1.4.231-rc.0.zy01 after this merges.