feat: configure macOS hardened runtime, entitlements, and build envir…#300
feat: configure macOS hardened runtime, entitlements, and build envir…#300samirpatil2000 wants to merge 2 commits intosiddharthvaddem:mainfrom
Conversation
…onment variables for notarization
📝 WalkthroughWalkthroughAdds macOS build/signing assets: an example environment file and .gitignore entry, macOS entitlements and updated electron-builder config, plus a new build script that packages, signs, creates DMGs, notarizes, and staples artifacts per architecture. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Script as build_macos.sh
participant EB as Electron Builder
participant Signer as codesign
participant Notary as xcrun notarytool
participant Stapler as xcrun stapler
Dev->>Script: run build_macos.sh
Script->>Script: load .env, validate tools & identities
Script->>EB: npm ci, npx tsc, npx vite build
Script->>EB: electron-builder --mac --arm64 / --x64
EB->>Script: produce .app bundles
rect rgba(100,150,255,0.5)
Script->>Signer: codesign .app (verify)
Script->>Script: create DMG with .app + /Applications symlink
Script->>Signer: codesign DMG
end
Script->>Notary: submit DMG --wait
Notary-->>Script: notarization ticket
Script->>Stapler: stapler staple DMG
Stapler-->>Script: validate stapling
Script->>Dev: output final signed/notarized DMGs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 78901a8076
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| CSC_NAME="$CSC_NAME" npx electron-builder --mac --${ARCH} --dir | ||
|
|
||
| # Find the .app bundle | ||
| APP_BUNDLE=$(find "${RELEASE_DIR}" -maxdepth 2 -name "*.app" -type d | grep -i "${ARCH}\|mac" | head -n1) |
There was a problem hiding this comment.
The app bundle lookup is not architecture-specific: grep -i "${ARCH}\|mac" | head -n1 will match any previously generated macOS bundle, and the script does not clean release/${version} between arch iterations. When the x64 pass runs after arm64, this can pick the arm64 .app and package/notarize it as the x64 DMG, which would ship a non-runnable build to Intel Mac users. Restrict this lookup to the current arch output path (or clean per-arch outputs) before selecting the bundle.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.env.example (1)
1-10: Clarify which env vars are required at build time vs one-time setup.
BUNDLE_IDandAPPLE_APP_SPECIFIC_PASSWORDappear unused by the current build execution path, so this template can mislead users about mandatory inputs. Consider labeling optional/one-time variables inline.✏️ Suggested clarification
APP_NAME=Openscreen BUNDLE_ID=com.siddharthvaddem.openscreen APPLE_ID= TEAM_ID= SIGN_IDENTITY="Developer ID Application: Samir Patil ()" CSC_NAME="Samir Patil ()" +# Required for build+notarize execution NOTARY_PROFILE=OpenScreen-notary + +# Optional: only needed when creating/updating notary credentials profile APPLE_APP_SPECIFIC_PASSWORD=🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.env.example around lines 1 - 10, The .env.example currently lists variables like BUNDLE_ID and APPLE_APP_SPECIFIC_PASSWORD without indicating whether they are required at build time or only needed for one-time setup, which can mislead users; update the template to annotate each variable (e.g., BUNDLE_ID, APPLE_ID, TEAM_ID, SIGN_IDENTITY, CSC_NAME, NOTARY_PROFILE, APPLE_APP_SPECIFIC_PASSWORD) with a short inline comment marking it as "required at build time", "one-time setup", or "optional", and where applicable note the exact command or step that consumes it (e.g., notarization step uses NOTARY_PROFILE and APPLE_APP_SPECIFIC_PASSWORD) so users know when to populate them.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/build_macos.sh`:
- Around line 15-23: After sourcing ENV_FILE, add an explicit non-empty
validation step for required environment variables (e.g., APP_NAME,
SIGN_IDENTITY, NOTARY_PROFILE, etc.) to prevent empty values slipping through;
implement this by declaring a list/array like REQUIRED_ENV=("APP_NAME"
"SIGN_IDENTITY" "NOTARY_PROFILE") and looping over it, testing each with: if [
-z "${!var}" ]; then echo "ERROR: $var is required and must not be empty"; exit
1; fi; run this check immediately after the source "$ENV_FILE" block so
preflight code (the later checks that reference SIGN_IDENTITY and others) never
sees empty values.
- Around line 138-139: The script currently masks codesign failures by piping a
failing codesign into a warning and then unconditionally printing success;
change the logic around the codesign --verify --deep --strict "$APP_BUNDLE" call
(referenced by APP_BUNDLE and ARCH and the print_warn/print_ok helpers) so that
you capture its exit status and fail the script on non-zero (e.g., call
print_error and exit 1) instead of downgrading to a warning; only call print_ok
"[${ARCH}] .app signature verified" when the verify command succeeded, and if
you still want to surface non-fatal warnings, separate stdout/stderr parsing
from the command exit code rather than overriding a non-zero exit to a success
path.
- Around line 124-128: The current APP_BUNDLE selection is non-deterministic
because the grep "-i 'ARCH|mac'" can match unrelated results and pick the wrong
.app; change the selection to search deterministically by first finding .app
directories whose names include the exact ARCH token (case-insensitive) using
RELEASE_DIR and ARCH (e.g. find -iname "*${ARCH}*.app"), then if none found fall
back to a mac-specific name (e.g. "*mac*.app"), and only as a final fallback
pick any .app; update the APP_BUNDLE assignment logic to try these searches in
that order so APP_BUNDLE reliably picks the correct architecture bundle.
---
Nitpick comments:
In @.env.example:
- Around line 1-10: The .env.example currently lists variables like BUNDLE_ID
and APPLE_APP_SPECIFIC_PASSWORD without indicating whether they are required at
build time or only needed for one-time setup, which can mislead users; update
the template to annotate each variable (e.g., BUNDLE_ID, APPLE_ID, TEAM_ID,
SIGN_IDENTITY, CSC_NAME, NOTARY_PROFILE, APPLE_APP_SPECIFIC_PASSWORD) with a
short inline comment marking it as "required at build time", "one-time setup",
or "optional", and where applicable note the exact command or step that consumes
it (e.g., notarization step uses NOTARY_PROFILE and APPLE_APP_SPECIFIC_PASSWORD)
so users know when to populate them.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: aca11e56-f7d3-4acd-8550-494508ed5d48
📒 Files selected for processing (5)
.env.example.gitignoreelectron-builder.json5macos.entitlementsscripts/build_macos.sh
| if [ -f "$ENV_FILE" ]; then | ||
| set -a | ||
| source "$ENV_FILE" | ||
| set +a | ||
| else | ||
| echo "ERROR: .env file not found at ${ENV_FILE}" | ||
| echo "Create one with APP_NAME, SIGN_IDENTITY, NOTARY_PROFILE, etc." | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Validate required env vars are non-empty before preflight checks.
Right now unset variables fail fast, but empty values can slip through (notably Line 70 with an empty SIGN_IDENTITY) and break later stages unpredictably.
🛠️ Proposed fix
if [ -f "$ENV_FILE" ]; then
set -a
source "$ENV_FILE"
set +a
else
@@
fi
+
+REQUIRED_VARS=(APP_NAME SIGN_IDENTITY CSC_NAME NOTARY_PROFILE)
+for var in "${REQUIRED_VARS[@]}"; do
+ if [ -z "${!var:-}" ]; then
+ print_err "Required variable '$var' is missing or empty in .env"
+ exit 1
+ fi
+done
@@
-if ! security find-identity -v -p codesigning | grep -q "$SIGN_IDENTITY"; then
+if ! security find-identity -v -p codesigning | grep -Fq "$SIGN_IDENTITY"; thenAlso applies to: 69-75
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 17-17: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/build_macos.sh` around lines 15 - 23, After sourcing ENV_FILE, add an
explicit non-empty validation step for required environment variables (e.g.,
APP_NAME, SIGN_IDENTITY, NOTARY_PROFILE, etc.) to prevent empty values slipping
through; implement this by declaring a list/array like REQUIRED_ENV=("APP_NAME"
"SIGN_IDENTITY" "NOTARY_PROFILE") and looping over it, testing each with: if [
-z "${!var}" ]; then echo "ERROR: $var is required and must not be empty"; exit
1; fi; run this check immediately after the source "$ENV_FILE" block so
preflight code (the later checks that reference SIGN_IDENTITY and others) never
sees empty values.
| APP_BUNDLE=$(find "${RELEASE_DIR}" -maxdepth 2 -name "*.app" -type d | grep -i "${ARCH}\|mac" | head -n1) | ||
| if [ -z "$APP_BUNDLE" ]; then | ||
| # Fallback: find any .app in the output | ||
| APP_BUNDLE=$(find "${RELEASE_DIR}" -maxdepth 2 -name "*.app" -type d | head -n1) | ||
| fi |
There was a problem hiding this comment.
Architecture bundle selection is non-deterministic and can package the wrong .app.
Line 124 matches any path containing mac, so during the second loop iteration it can pick the previous architecture’s app bundle. This can produce mislabeled installers.
🧭 Proposed fix
- APP_BUNDLE=$(find "${RELEASE_DIR}" -maxdepth 2 -name "*.app" -type d | grep -i "${ARCH}\|mac" | head -n1)
- if [ -z "$APP_BUNDLE" ]; then
- # Fallback: find any .app in the output
- APP_BUNDLE=$(find "${RELEASE_DIR}" -maxdepth 2 -name "*.app" -type d | head -n1)
- fi
+ APP_BUNDLE=""
+ case "$ARCH" in
+ arm64)
+ [ -d "${RELEASE_DIR}/mac-arm64/${APP_NAME}.app" ] && APP_BUNDLE="${RELEASE_DIR}/mac-arm64/${APP_NAME}.app"
+ ;;
+ x64)
+ [ -d "${RELEASE_DIR}/mac/${APP_NAME}.app" ] && APP_BUNDLE="${RELEASE_DIR}/mac/${APP_NAME}.app"
+ [ -z "$APP_BUNDLE" ] && [ -d "${RELEASE_DIR}/mac-x64/${APP_NAME}.app" ] && APP_BUNDLE="${RELEASE_DIR}/mac-x64/${APP_NAME}.app"
+ ;;
+ esac🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/build_macos.sh` around lines 124 - 128, The current APP_BUNDLE
selection is non-deterministic because the grep "-i 'ARCH|mac'" can match
unrelated results and pick the wrong .app; change the selection to search
deterministically by first finding .app directories whose names include the
exact ARCH token (case-insensitive) using RELEASE_DIR and ARCH (e.g. find -iname
"*${ARCH}*.app"), then if none found fall back to a mac-specific name (e.g.
"*mac*.app"), and only as a final fallback pick any .app; update the APP_BUNDLE
assignment logic to try these searches in that order so APP_BUNDLE reliably
picks the correct architecture bundle.
| codesign --verify --deep --strict "$APP_BUNDLE" 2>&1 || print_warn "[${ARCH}] Deep verify had warnings (may be expected pre-notarization)" | ||
| print_ok "[${ARCH}] .app signature verified" |
There was a problem hiding this comment.
Do not mask code-sign verification failures.
Line 138 currently downgrades verification failure to warning and still prints success on Line 139, which can push a broken artifact further into notarization/stapling.
✅ Proposed fix
- codesign --verify --deep --strict "$APP_BUNDLE" 2>&1 || print_warn "[${ARCH}] Deep verify had warnings (may be expected pre-notarization)"
- print_ok "[${ARCH}] .app signature verified"
+ if ! codesign --verify --deep --strict "$APP_BUNDLE" 2>&1; then
+ print_err "[${ARCH}] .app signature verification failed"
+ exit 1
+ fi
+ print_ok "[${ARCH}] .app signature verified"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| codesign --verify --deep --strict "$APP_BUNDLE" 2>&1 || print_warn "[${ARCH}] Deep verify had warnings (may be expected pre-notarization)" | |
| print_ok "[${ARCH}] .app signature verified" | |
| if ! codesign --verify --deep --strict "$APP_BUNDLE" 2>&1; then | |
| print_err "[${ARCH}] .app signature verification failed" | |
| exit 1 | |
| fi | |
| print_ok "[${ARCH}] .app signature verified" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/build_macos.sh` around lines 138 - 139, The script currently masks
codesign failures by piping a failing codesign into a warning and then
unconditionally printing success; change the logic around the codesign --verify
--deep --strict "$APP_BUNDLE" call (referenced by APP_BUNDLE and ARCH and the
print_warn/print_ok helpers) so that you capture its exit status and fail the
script on non-zero (e.g., call print_error and exit 1) instead of downgrading to
a warning; only call print_ok "[${ARCH}] .app signature verified" when the
verify command succeeded, and if you still want to surface non-fatal warnings,
separate stdout/stderr parsing from the command exit code rather than overriding
a non-zero exit to a success path.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@electron-builder.json5`:
- Around line 43-49: Remove the redundant entitlement key
"com.apple.security.device.audio-input" from the extendInfo object so
entitlements are only declared in the actual entitlements file; edit the
extendInfo block to keep NSAudioCaptureUsageDescription,
NSMicrophoneUsageDescription, NSCameraUsageDescription and
NSCameraUseContinuityCameraDeviceType but delete the
com.apple.security.device.audio-input entry (entitlements belong in
macos.entitlements, not extendInfo).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| "extendInfo": { | ||
| "NSAudioCaptureUsageDescription": "OpenScreen needs audio capture permission to record system audio.", | ||
| "NSMicrophoneUsageDescription": "OpenScreen needs microphone access to record voice audio.", | ||
| "NSCameraUsageDescription": "OpenScreen needs camera access to record webcam video.", | ||
| "NSCameraUseContinuityCameraDeviceType": true, | ||
| "com.apple.security.device.audio-input": true | ||
| } |
There was a problem hiding this comment.
Entitlement key in extendInfo is ineffective.
Line 48 places com.apple.security.device.audio-input in extendInfo, which populates Info.plist. However, this is an entitlement key that must be in the entitlements file to take effect—and it's already correctly declared in macos.entitlements. Placing it here has no functional effect and may confuse future maintainers.
Suggested fix: remove the redundant entitlement from extendInfo
"extendInfo": {
"NSAudioCaptureUsageDescription": "OpenScreen needs audio capture permission to record system audio.",
"NSMicrophoneUsageDescription": "OpenScreen needs microphone access to record voice audio.",
"NSCameraUsageDescription": "OpenScreen needs camera access to record webcam video.",
- "NSCameraUseContinuityCameraDeviceType": true,
- "com.apple.security.device.audio-input": true
+ "NSCameraUseContinuityCameraDeviceType": true
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "extendInfo": { | |
| "NSAudioCaptureUsageDescription": "OpenScreen needs audio capture permission to record system audio.", | |
| "NSMicrophoneUsageDescription": "OpenScreen needs microphone access to record voice audio.", | |
| "NSCameraUsageDescription": "OpenScreen needs camera access to record webcam video.", | |
| "NSCameraUseContinuityCameraDeviceType": true, | |
| "com.apple.security.device.audio-input": true | |
| } | |
| "extendInfo": { | |
| "NSAudioCaptureUsageDescription": "OpenScreen needs audio capture permission to record system audio.", | |
| "NSMicrophoneUsageDescription": "OpenScreen needs microphone access to record voice audio.", | |
| "NSCameraUsageDescription": "OpenScreen needs camera access to record webcam video.", | |
| "NSCameraUseContinuityCameraDeviceType": true | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@electron-builder.json5` around lines 43 - 49, Remove the redundant
entitlement key "com.apple.security.device.audio-input" from the extendInfo
object so entitlements are only declared in the actual entitlements file; edit
the extendInfo block to keep NSAudioCaptureUsageDescription,
NSMicrophoneUsageDescription, NSCameraUsageDescription and
NSCameraUseContinuityCameraDeviceType but delete the
com.apple.security.device.audio-input entry (entitlements belong in
macos.entitlements, not extendInfo).
1 participant
Description
This PR implements a complete, signed, and notarized macOS build pipeline for the Openscreen application. It introduces a manual build script that handles packaging, code signing with Apple's Hardened Runtime, and notarization via Apple's Notary Service (notarytool). This ensures the produced
.dmginstallers for both arm64 and x64 architectures are trusted by Gatekeeper and can be distributed outside the Mac App Store without security warnings.Motivation
Previously, macOS builds were unsigned, leading to the "App is from an unidentified developer" warning, which creates friction for users. By implementing the Hardened Runtime and Notarization process, we satisfy Apple's security requirements for distribution.
Type of Change
Related Issue(s)
Fixes issues related to macOS distribution and Gatekeeper acceptance.
Screenshots / Video
Verification of Notarized DMG:
Testing
chmod +x scripts/build_macos.sh && ./scripts/build_macos.shon an arm64 Mac.codesign --verifyto ensure the.appbundle and the resulting.dmgare correctly signed.spctlto confirm they are accepted by the system as Notarized Developer ID packages.Checklist
.gitignoreto prevent leaking signing credentials.Summary by CodeRabbit
New Features
Chores