ci(macos): add fail-closed release candidate gate - #801
Conversation
📝 WalkthroughWalkthroughThe PR adds macOS distribution policies, a release artifact verifier, and a manually dispatched workflow that builds, signs, notarizes, tests, and evaluates x64 and arm64 release candidates. ChangesmacOS distribution validation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Dispatcher
participant SourceAuthorization
participant BuildJobs
participant MacOSDistributionVerifier
participant Verdict
Dispatcher->>SourceAuthorization: dispatch with source SHA
SourceAuthorization->>SourceAuthorization: validate repository, branch, SHA, and current main commit
SourceAuthorization->>BuildJobs: provide authorized SHA
BuildJobs->>BuildJobs: build, sign, and notarize x64 and arm64 candidates
BuildJobs->>MacOSDistributionVerifier: verify DMG and ZIP artifacts
MacOSDistributionVerifier-->>BuildJobs: return verification report
BuildJobs->>Verdict: report architecture results
Verdict->>Verdict: evaluate authorization and build results
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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.
Actionable comments posted: 4
🧹 Nitpick comments (1)
scripts/verify-macos-distribution.mjs (1)
283-289: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winBatch the
fileinvocations to reduce subprocess count.The loop spawns one
fileprocess per regular file. An Electron bundle contains thousands of files, so this step dominates the verification runtime.fileaccepts multiple paths in a single call and prints one line per path when-bis omitted, so you can classify files in batches.♻️ Sketch of a batched classification
const machOBinaries = []; - for (const filePath of walkRegularFiles(appPath)) { - const fileType = runProcess("file", ["-b", filePath]).stdout; - if (fileType.includes("Mach-O")) { - machOBinaries.push(filePath); - } - } + const allFiles = walkRegularFiles(appPath); + const batchSize = 200; + for (let index = 0; index < allFiles.length; index += batchSize) { + const batch = allFiles.slice(index, index + batchSize); + const lines = runProcess("file", ["-h", ...batch]).stdout.split(/\r?\n/); + for (const line of lines) { + if (!line.includes("Mach-O")) { + continue; + } + const separator = line.indexOf(": "); + if (separator > 0) { + machOBinaries.push(line.slice(0, separator)); + } + } + }🤖 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 `@scripts/verify-macos-distribution.mjs` around lines 283 - 289, Update the Mach-O discovery loop around walkRegularFiles and runProcess to classify regular files in batches rather than spawning one file process per path. Pass multiple file paths to each invocation, omit the -b option so outputs remain associated one line per input path, and add only paths whose corresponding output identifies Mach-O to machOBinaries.
🤖 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/macos-release-candidate.yml:
- Around line 161-166: Update the PKCS#12 extraction command in the macOS
release workflow to retry with openssl pkcs12 -legacy when the initial
extraction fails due to legacy RC2/3DES encryption. Preserve the existing
non-legacy attempt first and reuse the same certificate input, password, and
output paths for the fallback.
In `@scripts/macos-distribution-policy.mjs`:
- Around line 76-84: Update the entitlement validation branch for
com.apple.security.get-task-allow so both true and false values are recognized
as expected keys: reject only when the value is true, and continue without
adding an unexpected-entitlement error when it is false.
In `@scripts/verify-macos-distribution.mjs`:
- Around line 455-457: Update the catch block around writeReport so
report-writing failures cannot replace the original verification error: wrap the
writeReport call in its own try/catch, preserve the existing report paths, and
rethrow the original error after handling any reporting failure.
- Around line 411-422: Update the “DMG mounts read-only” check to attach the
image without forcing the -readonly option, then inspect the resulting mount
flags and fail unless the mounted volume is actually read-only; alternatively,
rename the check to accurately describe forced read-only attachment if that
behavior is intended.
---
Nitpick comments:
In `@scripts/verify-macos-distribution.mjs`:
- Around line 283-289: Update the Mach-O discovery loop around walkRegularFiles
and runProcess to classify regular files in batches rather than spawning one
file process per path. Pass multiple file paths to each invocation, omit the -b
option so outputs remain associated one line per input path, and add only paths
whose corresponding output identifies Mach-O to machOBinaries.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 56c55772-f393-4131-b6e2-e68bebe0bc66
📒 Files selected for processing (5)
.github/workflows/macos-release-candidate.ymlelectron/macosDistributionPolicy.test.mjspackage.jsonscripts/macos-distribution-policy.mjsscripts/verify-macos-distribution.mjs
Description
Adds a manual, candidate-only macOS distribution gate for x64 and arm64. It accepts only the canonical repository's exact current
mainSHA, builds before exposing Apple credentials, forces Developer ID signing and notarization, and always packages with--publish never.The shared verifier fails closed on publisher identity, Team ID, secure timestamp, hardened runtime, an exact root-entitlement allowlist, nested Mach-O signatures/architecture, stapled tickets, Gatekeeper, syspolicy, DMG integrity/read-only mounting, and apps extracted from both DMG and ZIP. Candidate artifacts expire after three days.
Motivation
The existing release job relies mainly on packaging success and historical credential evidence. That is not enough to prove that current artifacts are signed, notarized, stapled, and accepted by macOS without risking a public release. This PR creates a reversible validation layer; it does not change the release workflow, create tags/releases, publish updater metadata, or claim physical macOS runtime readiness.
Type of Change
Related Issue(s)
No issue closed. Physical ScreenCaptureKit, TCC, microphone, system-audio, and multi-display acceptance remains a separate follow-up on real Macs.
Screenshots / Video
Not applicable; no product UI changes.
Testing Guide
npm test— 107 files passed; 1005 tests passed, 1 skipped.npx tsc --noEmitandnpm run i18n:checkpassed.After review and merge, dispatch the new workflow with the exact merged
mainSHA. Treat its output as temporary distribution evidence only; a public macOS beta still requires physical Mac runtime acceptance and a separate release decision.Checklist
Summary by CodeRabbit
New Features
Tests