Notarize serve-sim camera dylibs in macOS releases#7077
Conversation
Co-authored-by: Orca <help@stably.ai>
📝 WalkthroughWalkthroughChangesThis change adds macOS-specific notarization of serve-sim camera dylibs to the Electron Builder 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
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: 1
🧹 Nitpick comments (2)
config/scripts/electron-builder-config.test.mjs (2)
324-358: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider splitting into separate
itblocks for clearer failure isolation.This single test iterates four distinct scenarios (missing Apple ID password, missing API key trio, invalid notary status, invalid JSON output) via loops. If one sub-case fails, the reported failure won't clearly indicate which credential/output scenario broke.
360-387: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winRedaction test only checks one secret value.
The assertion
expect(error.message).not.toContain('app-password')verifies only the Apple app-specific password is redacted. GivenappleIdMacReleaseEnvalso includesAPPLE_ID(an email, potentially PII) andAPPLE_TEAM_ID, broadening the assertion to cover these values would give stronger confidence thatredactCredentialValuesscrubs all credential-derived args, not just the password.✏️ Proposed strengthening of the assertion
expect(error.message).toContain('Standalone binary notarization subprocess failed') expect(error.message).not.toContain('app-password') + expect(error.message).not.toContain('release@example.com') expect(error.message).toContain('[REDACTED]')
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 12372744-d05f-4d30-8e07-8450497be5e6
📒 Files selected for processing (2)
config/electron-builder.config.cjsconfig/scripts/electron-builder-config.test.mjs
Co-authored-by: Orca <help@stably.ai>
Co-authored-by: Orca <help@stably.ai>
Summary
Fixes #6877.
This adds a macOS release-only
afterSignstep that submits the packaged standaloneserve-simlibSimCameraInjector.dylibcopies to Apple notarization via zippedxcrun notarytool submit --waitarchives. The outer app notarization path is unchanged; this closes the gap where nested camera injector dylibs could be signed but not directly accepted by Gatekeeper assessment.Important boundary: local validation cannot prove the final Apple Gatekeeper result without release credentials and a produced signed macOS artifact. Before release, run the macOS release pipeline and assess the shipped app plus nested dylib copies with
spctl/codesign.Design Approach
The smallest scoped change is in
config/electron-builder.config.cjs: keep existing macOS app signing/notarization behavior, then add a release-gated hook for the nested standalone dylibs thatstaplercannot ticket directly. The hook requires the two packaged resource copies currently declared by the build and notarizes theapp.asar.unpackedcopy only when present.Design Review Outcome
Design review completed clean after tightening the test plan around behavior rather than source-string checks, and after explicitly treating release artifact
spctlvalidation as a credentialed release gate.Implementation
afterSignnotarization for packagedserve-simcamera injector dylibs whenORCA_MAC_RELEASE=1andelectronPlatformName === 'darwin'.dittoandnotarytool --waitsubprocesses so release jobs fail instead of hanging indefinitely.No UI was touched, so
docs/STYLEGUIDE.mdis not applicable.Completeness
Completeness verification passed against the requested macOS release-packaging scope. The headline behavior is implemented locally, with the live signed-artifact Gatekeeper assessment remaining as the release-validation gap.
Consistency
Source-of-truth behavior is the macOS release packaging/notarization path in electron-builder. Neighboring surfaces are Windows/Linux packaging and local development packaging; those remain unchanged because the new hook exits outside macOS release builds.
Risks And Non-Goals
serve-simpackaging layout beyond requiring the declared release dylib copies to be present.spctlchecks.Performance
Performance audit found no runtime hot path. The new work is bounded to macOS release packaging and runs two required, plus one optional,
ditto/notarytoolsubmissions.Code Review
The code-review loop completed with a fresh final round of two reviewers. Both returned clean. An earlier actionable finding was fixed by making the
app.asar.unpackeddylib optional instead of hard-required.Merge-Confidence Validation
Result: land after release-artifact validation, or land now only if the live Apple artifact gap is explicitly accepted.
Passed locally:
pnpm exec vitest run --config config/vitest.config.ts config/scripts/electron-builder-config.test.mjs— 26 passedpnpm exec oxfmt --check config/electron-builder.config.cjs config/scripts/electron-builder-config.test.mjs— passedpnpm exec oxlint config/electron-builder.config.cjs config/scripts/electron-builder-config.test.mjs— passedgit diff --check— passedpnpm run typecheck— passedpnpm run lint— passed with existing unrelated warningspnpm build:unpack— passed locally; runner output was truncated because the build is noisy, but the command exited 0 and passed electron-builder packaging/config validation__testseamNot applicable:
Release gate still needed:
spctl -a -vvvandcodesign --verify.Post-PR Stabilization
65a81fd.verify, Ubuntu native smoke, Windows native smoke); other platform jobs are skipped by workflow rules.Made with Orca 🐋