Repository navigation
Conversation
The comment said the Developer-ID-named secret held the Apple Distribution certificate, which was true only because there was one certificate slot for both. Each certificate now has the secret named for it.
|
Warning Review limit reached
Next review available in: 30 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. 📝 WalkthroughWalkthroughThe release workflow now runs for eligible pull requests, creates ephemeral prereleases, and cancels superseded runs. Release builds install Go, generate configuration, run the gate-proof probe, and use distinct CI and release signing certificates. ChangesPull-request release workflow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant PullRequest
participant release.yml
participant ReusableReleaseWorkflow
PullRequest->>release.yml: trigger release workflow
release.yml->>ReusableReleaseWorkflow: pass ephemeral prerelease settings
ReusableReleaseWorkflow->>ReusableReleaseWorkflow: install Go and build release artifacts
Possibly related PRs
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
The shared pipeline builds, signs, notarizes and packages on a pull request and stops before publishing, but this repo triggered releases only on a push to main. The first thing to exercise the release path was therefore the merge, so a broken release was discovered only once it could not be undone.
agoodkind
changed the title
The engine's release-build target carries no prerequisites, so nothing rendered Config.generated.swift and the dev tool the release command runs could not compile on a fresh CI checkout. Reproduced locally: with that gitignored directory absent, make generate recreates it. The dry-run gate job was copied from the engine and is redundant, because every job of the reusable workflow already reports as its own check.
…ge as CI The vendored WireGuardKitGo target prepares a Go goroot through its own Makefile and fails unless go env GOROOT resolves. CI installs Go for that reason; the release build links the same target and did not, so it failed at wireguard-go-bridge/goroot/.prepared.
The release build on the runner takes the decoupled hard gate even though swift-mk build marks GateProof, while the same recipe takes the prologue path locally under the CI environment. The probe prints each authorization factor (freshness, ancestor, anchor, start time) in the job log, so the failing factor is named on the runner instead of guessed at.
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/release.yml:
- Around line 29-35: Secure the reusable release workflow invocation by
replacing the mutable `@main` reference on _release.yml with its full commit SHA,
and replace secrets: inherit with explicit mappings for only the secrets
required by the release workflow. Preserve the existing pull-request condition
and inputs while limiting secret exposure.
🪄 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: a7dedc23-a3c7-48db-bf89-68d7e3c0c770
📒 Files selected for processing (2)
.github/workflows/release.ymlMakefile
| # A fork pull request cannot read this repository's signing secrets, so the | ||
| # signed run would fail for a reason the contributor cannot fix. | ||
| if: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository }} | ||
| uses: agoodkind/swift-makefile/.github/workflows/_release.yml@main | ||
| with: | ||
| ephemeral: ${{ github.event_name == 'pull_request' }} | ||
| release-track: ${{ github.event_name == 'pull_request' && 'prerelease' || '' }} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Restrict secrets passed to the reusable release workflow.
The called workflow at Line [32] uses mutable @main, and Line [58] passes all available secrets with secrets: inherit. The new pull-request trigger makes this path run for same-repository pull requests. The fork check does not protect against a changed or compromised reusable workflow.
Pin agoodkind/swift-makefile/.github/workflows/_release.yml to a full commit SHA. Pass only the secrets required by the release workflow.
🧰 Tools
🪛 zizmor (1.29.0)
[warning] 32-32: secrets unconditionally inherited by called workflow (secrets-inherit): this reusable workflow
(secrets-inherit)
🤖 Prompt for 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.
In @.github/workflows/release.yml around lines 29 - 35, Secure the reusable
release workflow invocation by replacing the mutable `@main` reference on
_release.yml with its full commit SHA, and replace secrets: inherit with
explicit mappings for only the secrets required by the release workflow.
Preserve the existing pull-request condition and inputs while limiting secret
exposure.
Source: Linters/SAST tools
The probe named .make/swift-mk literally, which does not exist on a runner: setup-build-env builds the engine into the toolchain cache and exports SWIFT_MK_BIN, so the release build stopped at exit 127 before reaching the dev tool. The command now expands SWIFT_MK_BIN at shell time, which resolves on a runner and locally.
Proves both signing certificates work, and makes the release path testable before it publishes.
What was wrong
There was one certificate slot,
APPLE_DEVELOPER_ID_P12_BASE64, shared by CI and the release. This repository put its Apple Distribution certificate in it so CI could sign at all, because the CI runner is not a registered device and only App Store profiles work there.The cost was that a release could never be signed. The release build resolves a Developer ID identity, which was not in that secret, so it failed with
Developer ID Application identity ... not foundright after a successful import.The two certificates are genuinely different, which is why one slot could not serve both. The Apple Distribution one is
85:D1:3A:F9:…and the Developer ID one isD5:C4:37:9A:….The change
The shared workflow now takes each certificate as its own input and its own secret, and
signing-identity-nameselects which one signs (agoodkind/swift-makefile#206). This repository now holds both, so the CI comment that said the Developer-ID-named secret carries the distribution certificate is corrected here.The release also now runs on a pull request. The shared pipeline executes every signed stage and stops before publishing, and this repository triggered releases only on a push to main, so the first thing to exercise the release path was the merge itself. A broken release was therefore discovered only once it could not be undone.
What this pull request verifies
Both certificates, on the paths that use them, without publishing anything:
APPLE_DISTRIBUTION_P12_*and signs every product withApple Distribution: Alex Goodkind (H3BMXM4W7H).APPLE_DEVELOPER_ID_P12_*and signs, notarizes and packages withDeveloper ID Application: Alex Goodkind (H3BMXM4W7H), then stops before publish.A green run here is the first time this repository has signed with the correct certificate on both paths. Merging then publishes a pre-release, which is the downloadable build ICT-1 asks for.