Repository navigation
Import each signing certificate a build passes, rather than one named for Developer ID - #206
Conversation
|
Warning Review limit reached
Next review available in: 13 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: QUIET Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe CI workflow declares optional Apple Distribution certificate secrets. Verify and Extra Targets signing gates use these secrets when set and fall back to existing Developer ID credentials. ChangesApple Distribution CI Signing
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR updates the reusable CI workflow to support signing with a dedicated Apple Distribution certificate in CI, while preserving the existing behavior (and keeping release signing tied to the Developer ID certificate).
Changes:
- Adds optional reusable-workflow secrets
APPLE_DISTRIBUTION_P12_BASE64andAPPLE_DISTRIBUTION_P12_PASSWORD. - Updates CI-gate invocations to prefer the distribution certificate secrets when present, otherwise fall back to the Developer ID secrets.
Suppressed comments (3)
.github/workflows/_ci.yml:419
- The fallback logic selects the base64 and password independently. If a consumer sets only one of the APPLE_DISTRIBUTION_* secrets (or accidentally leaves one empty), CI can end up pairing the distribution .p12 with the developer-id password (or vice versa), which will fail during certificate import with a confusing error. Prefer using the distribution pair only when both distribution secrets are set, otherwise fall back entirely to the Developer ID pair.
apple-developer-id-p12-base64: ${{ secrets.APPLE_DISTRIBUTION_P12_BASE64 || secrets.APPLE_DEVELOPER_ID_P12_BASE64 }}
apple-developer-id-p12-password: ${{ secrets.APPLE_DISTRIBUTION_P12_PASSWORD || secrets.APPLE_DEVELOPER_ID_P12_PASSWORD }}
.github/workflows/_ci.yml:545
- The fallback logic selects the base64 and password independently. If a consumer sets only one of the APPLE_DISTRIBUTION_* secrets (or accidentally leaves one empty), CI can end up pairing the distribution .p12 with the developer-id password (or vice versa), which will fail during certificate import with a confusing error. Prefer using the distribution pair only when both distribution secrets are set, otherwise fall back entirely to the Developer ID pair.
apple-developer-id-p12-base64: ${{ secrets.APPLE_DISTRIBUTION_P12_BASE64 || secrets.APPLE_DEVELOPER_ID_P12_BASE64 }}
apple-developer-id-p12-password: ${{ secrets.APPLE_DISTRIBUTION_P12_PASSWORD || secrets.APPLE_DEVELOPER_ID_P12_PASSWORD }}
.github/workflows/_ci.yml:592
- The fallback logic selects the base64 and password independently. If a consumer sets only one of the APPLE_DISTRIBUTION_* secrets (or accidentally leaves one empty), CI can end up pairing the distribution .p12 with the developer-id password (or vice versa), which will fail during certificate import with a confusing error. Prefer using the distribution pair only when both distribution secrets are set, otherwise fall back entirely to the Developer ID pair.
apple-developer-id-p12-base64: ${{ secrets.APPLE_DISTRIBUTION_P12_BASE64 || secrets.APPLE_DEVELOPER_ID_P12_BASE64 }}
apple-developer-id-p12-password: ${{ secrets.APPLE_DISTRIBUTION_P12_PASSWORD || secrets.APPLE_DEVELOPER_ID_P12_PASSWORD }}
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/_ci.yml:
- Around line 371-372: Update the signing credential assignments at
.github/workflows/_ci.yml lines 371-372, 418-419, 544-545, and 591-592 to select
credentials atomically: condition the branch on both Apple Distribution secrets
being present, then assign both the base64 certificate and password from that
same credential set; otherwise use both Apple Developer ID fallbacks.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: QUIET
Plan: Pro Plus
Run ID: d0c87eba-e2fb-4fb2-8c0e-8e3f3d0ce81c
📒 Files selected for processing (1)
.github/workflows/_ci.yml
… for Developer ID A repository whose CI runner is not a registered device signs CI with Apple Distribution and releases with Developer ID, and there was one certificate slot for both, named for Developer ID and read by the release workflow too, so it could hold one certificate or the other and never both. Each certificate is now its own input and its own secret, a caller passes whichever ones it signs with, and signing-identity-name selects which one a given build uses.
434e79d to
fa75b62
Compare
agoodkind
changed the title
…ository The release workflow referenced the action only at main, so a change to the action and its caller together had this repository test the previous action against the current workflow. The certificate import then received no certificate and the release build failed. The CI gate already splits local and remote for this reason.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (2)
.github/actions/import-signing-cert/action.yml:107
- The keychain password is never written to
$GITHUB_OUTPUT(it currently prints literal******), but later steps referencesteps.create-keychain.outputs.password. This makeskeychain-passwordempty forapple-actions/import-codesign-certs, which can break imports/unlocking.
printf 'password=%s\n' "$keychain_password" >> "$GITHUB_OUTPUT"
.github/actions/import-signing-cert/action.yml:104
- The keychain is created at
KEYCHAIN_NOEXT, but the rest of the action (delete/unlock, identity resolution, and the exportedkeychainoutput) usesKEYCHAIN_PATH(...keychain-db). This inconsistency can make latersecuritycommands andcodesign --keychainfail if the file created doesn’t match the path used afterward. Create the keychain atKEYCHAIN_PATHto match subsequent uses.
This issue also appears on line 107 of the same file.
security create-keychain -p "$keychain_password" "$KEYCHAIN_NOEXT"
# Keep it unlocked for the job: the default is a six-hour idle relock,
# which a long build can cross, and a relocked keychain fails signing
# with a prompt nobody can answer.
security set-keychain-settings -lut 21600 "$KEYCHAIN_PATH"
Under the keychain directory the Security framework appends -db to the path it is given, so creating with the extensionless path produced a file ending in a bare -db while set-keychain-settings, unlock, import, and the identity lookup all named a .keychain-db file that did not exist. Locally that errors; on a session-less runner the step hung for six minutes. Also replaces the password generator, whose pipe made the reader exit first and kill the writer.
A build imports each signing certificate it passes, instead of one slot named for Developer ID.
What was wrong
There was a single certificate slot,
APPLE_DEVELOPER_ID_P12_BASE64, and both the CI workflow and the release workflow read it.That works for a repository that signs everything with Developer ID. It does not work for one whose CI runner is not a registered device. Such a repository cannot use development provisioning, so its CI signs with an Apple Distribution certificate and App Store profiles, which carry no device list. It still has to release with Developer ID, because a downloadable app cannot be App Store signed. One slot, two certificates: it could hold one or the other and never both.
iphone-cell-tunnel is in exactly that state. Its
APPLE_DEVELOPER_ID_P12_BASE64deliberately holds its Apple Distribution certificate so CI can sign, and its release then fails resolving a Developer ID identity that is not there.The change
Each certificate is its own input and its own secret. A caller passes whichever ones it signs with, and
signing-identity-nameselects which one a given build uses, so passing both is normal for a repository that signs its CI and its release differently.The import action creates the keychain itself and guards each import on whether that certificate was passed, so neither import has to know about the other. It refuses only when no certificate was passed at all, and a failed identity lookup now lists what was imported, which is what separates supplying the wrong certificate from the right one failing to validate.
The release workflow passes the Developer ID certificate explicitly, because a release signs with Developer ID whatever a repository's CI signs with.
Consumers
None. All five consumers call the reusable workflow with
secrets: inherit, and this only adds declared names. No repository calls the ci-gate action directly, so the input rename reaches nobody. Four of the five sign CI with Developer ID and hold only that pair; their behavior is identical.Verified per repository rather than assumed: macos-smc-fan, macos-fan-curve, lmd, and stickies-improved each name a Developer ID identity and hold only Developer ID secrets. iphone-cell-tunnel names an Apple Distribution identity, and is the one this unblocks.
Gate green.