feat: Add macOS code signing and notarization#14
Conversation
- Add certificate import and keychain setup in release workflow - Configure electron-builder with hardenedRuntime and entitlements - Enable notarization with Apple Developer ID - Add macOS entitlements file for Electron security requirements - Remove "not code-signed" warning from release notes The macOS build will now be signed with Apple Developer ID certificate and automatically notarized, eliminating Gatekeeper warnings for users.
Summary of ChangesHello @tphakala, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the macOS build process by integrating Apple's code signing and notarization requirements. The changes ensure that the application is properly signed with a Developer ID certificate and automatically notarized, which will prevent Gatekeeper warnings and provide a smoother installation experience for macOS users. Highlights
Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughAdds macOS code signing and notarization to CI and build config: CI workflow steps to create/import a temporary keychain, codesign and clean up; a new macOS entitlements plist; and package.json updates to enable hardened runtime, entitlements, and notarization metadata. Changes
Sequence Diagram(s)sequenceDiagram
participant CI as CI Workflow
participant Keychain as Temporary Keychain
participant Codesign as codesign / electron-builder
participant Apple as Apple Notary/ID
participant Repo as Release artifact store
CI->>Keychain: create temporary keychain & import cert
CI->>Codesign: invoke package step with APPLE_* creds
Codesign->>Keychain: access private key for signing
Codesign->>Apple: notarize uploaded artifact (teamId, app password)
Apple-->>Codesign: notarization result
Codesign->>Keychain: remove imported cert
CI->>Repo: publish signed/notarized release
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces macOS code signing and notarization, which is a great improvement for macOS users. However, the build/entitlements.mac.plist file includes highly permissive flags such as allow-dyld-environment-variables and disable-library-validation that weaken macOS Hardened Runtime protections, increasing the risk of library injection attacks. It is recommended to review these entitlements and remove any that are not strictly necessary for the application's core functionality. Additionally, there is a minor suggestion to simplify the configuration in package.json by removing a redundant property.
| <key>com.apple.security.cs.allow-dyld-environment-variables</key> | ||
| <true/> | ||
| <key>com.apple.security.cs.disable-library-validation</key> | ||
| <true/> |
There was a problem hiding this comment.
The entitlements com.apple.security.cs.allow-dyld-environment-variables and com.apple.security.cs.disable-library-validation are enabled.
com.apple.security.cs.allow-dyld-environment-variables(lines 9-10) allows the application to be influenced byDYLD_*environment variables, which can be used to inject malicious libraries into the application's process. This is rarely required for production applications and significantly weakens the Hardened Runtime protections.com.apple.security.cs.disable-library-validation(lines 11-12) allows the application to load any signed library, even if not signed by the same developer or Apple. This should only be enabled if the application must load third-party native modules that cannot be signed with your own certificate.
Unless these are strictly required for the application to function in production, they should be removed to maintain a stronger security posture and adhere to the Principle of Least Privilege.
There was a problem hiding this comment.
Partially correct.
allow-dyld-environment-variables: Agree - removing this (not needed, security risk).
disable-library-validation: Required for this app. We use better-sqlite3, a native Node.js addon that requires this entitlement to load properly. Without it, macOS will reject the native module during hardened runtime. This is standard practice for Electron apps with native dependencies (electron-builder #4616, LM Studio #1494).
Keeping allow-jit, allow-unsigned-executable-memory, and disable-library-validation. Removing only allow-dyld-environment-variables.
| "hardenedRuntime": true, | ||
| "gatekeeperAssess": false, | ||
| "entitlements": "build/entitlements.mac.plist", | ||
| "entitlementsInherit": "build/entitlements.mac.plist", |
There was a problem hiding this comment.
The entitlementsInherit property is redundant here. According to the electron-builder documentation, entitlementsInherit defaults to the value of entitlements if not specified. Since you are setting it to the same value, you can remove this line to simplify the configuration.
There was a problem hiding this comment.
Not redundant - these serve different purposes:
entitlements: Applied to the main app binaryentitlementsInherit: Applied to child processes, frameworks, and bundles
Using the same file for both is intentional (we want consistent permissions across all processes). The default is build/entitlements.mac.inherit.plist, not the value of entitlements (electron-builder 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: 2
🤖 Fix all issues with AI agents
In @.github/workflows/release.yml:
- Around line 142-144: The workflow sets the wrong environment variable name for
electron-builder notarization: replace the APPLE_APP_PASSWORD key with
APPLE_APP_SPECIFIC_PASSWORD so the notarize integration sees the expected
variable; update the environment mapping where matrix.platform is checked (the
entries using APPLE_TEAM_ID and APPLE_ID) to export APPLE_APP_SPECIFIC_PASSWORD
instead of APPLE_APP_PASSWORD so the notarization step
(electron-builder/@electron/notarize) receives the correct secret.
In `@package.json`:
- Around line 77-79: Replace the deprecated object form of the notarize config
in package.json (the "notarize": { "teamId": "${APPLE_TEAM_ID}" } entry) with
the boolean form expected by newer electron-builder versions by setting
"notarize": true and relying on the APPLE_TEAM_ID environment variable at build
time; update any build docs or CI to ensure APPLE_TEAM_ID is exported in the
environment before running electron-builder so the notarization step can read
it.
🧹 Nitpick comments (1)
.github/workflows/release.yml (1)
109-133: Keychain setup follows a solid pattern overall.The certificate import flow is well-structured: temporary keychain with scoped lifetime, proper partition-list configuration, and prompt cleanup of the
.p12file.One minor note on Line 130:
security list-keychain -d user -s "$KEYCHAIN_PATH"replaces the entire user keychain search list, dropping the default login keychain. On ephemeral CI runners this is usually fine, but if any downstream tool needs the login keychain (e.g., for Apple root CAs), it can cause subtle failures. The safer pattern prepends:Safer keychain list management
- security list-keychain -d user -s "$KEYCHAIN_PATH" + security list-keychain -d user -s "$KEYCHAIN_PATH" $(security list-keychain -d user | tr -d '"')
Removed com.apple.security.cs.allow-dyld-environment-variables as it's not required for production and presents a security risk (allows DYLD_* environment variable injection). Kept the following entitlements required for Electron + native modules: - allow-jit: Required for V8 JavaScript engine - allow-unsigned-executable-memory: Required for V8 JIT compilation - disable-library-validation: Required for better-sqlite3 native module
Review Response SummaryImplemented:
Not implemented (with reasoning):
Final entitlements:
See inline comment threads for technical details and sources. |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| APPLE_TEAM_ID: ${{ matrix.platform == 'macos' && secrets.APPLE_TEAM_ID || '' }} | ||
| APPLE_ID: ${{ matrix.platform == 'macos' && secrets.APPLE_ID || '' }} | ||
| APPLE_APP_PASSWORD: ${{ matrix.platform == 'macos' && secrets.APPLE_APP_SPECIFIC_PASSWORD || '' }} |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
- Changed APPLE_APP_PASSWORD to APPLE_APP_SPECIFIC_PASSWORD (correct env var name expected by electron-builder) - Changed notarize config from deprecated object form to boolean (electron-builder v26+ requirement) The object form of notarize config was deprecated in v25 and removed in v26. Modern electron-builder expects 'notarize: true' with APPLE_TEAM_ID provided via environment variable.
1 participant
Summary
Changes
Workflow Updates (
.github/workflows/release.yml)APPLE_TEAM_ID,APPLE_ID, andAPPLE_APP_PASSWORDfor notarizationelectron-builder Configuration (
package.json)hardenedRuntime: true- Required for modern macOS securitygatekeeperAssess: false- Disables local assessment (CI builds are already signed)entitlementsconfiguration - Points to entitlements filenotarizeconfiguration - Enables automatic notarization with Team IDEntitlements File (
build/entitlements.mac.plist)New file with required permissions for Electron apps:
Required GitHub Secrets
The following secrets must be configured in repository settings:
APPLE_CERTIFICATE_BASE64- Developer ID certificate (base64 encoded)APPLE_CERTIFICATE_PASSWORD- Certificate passwordAPPLE_TEAM_ID- Apple Team IDAPPLE_ID- Apple ID emailAPPLE_APP_SPECIFIC_PASSWORD- App-specific password for notarizationTesting
This can only be fully tested on the next release. The workflow will:
User Impact
Users will no longer see Gatekeeper warnings when opening the macOS version of Birda GUI. The app will install and run without requiring "Open anyway" or right-click workarounds.
Summary by CodeRabbit