feat: add manual install fallback for unsigned macOS auto-updates#29
feat: add manual install fallback for unsigned macOS auto-updates#29
Conversation
📝 WalkthroughWalkthroughThis PR introduces macOS code signing validation and manual update installation capabilities for unsigned builds. New modules add utility functions for validating Developer ID Application signatures, resolving downloaded update files, discovering app bundles, and generating shell scripts for manual updates. Integration into main.ts tracks downloaded update artifacts, detects unsigned builds, and routes unsigned macOS updates through a new manual installation path instead of the standard auto-update flow, with fallback to standard installation on failure. 🚥 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. 📝 Coding Plan
Comment Tip CodeRabbit can approve the review once all CodeRabbit's comments are resolved.Enable the |
| } | ||
|
|
||
| install_update() { | ||
| /bin/rm -rf "$TARGET_APP" |
There was a problem hiding this comment.
[🟡 Medium]
The generated installer removes the currently installed bundle before verifying the new bundle has been copied successfully, so a copy failure (for example disk full or I/O error during ditto) can leave users with no runnable app at the target path. This creates a real failure-mode regression in the unsigned-update path. Use a two-phase replace: copy to a temporary sibling path, verify success, then atomically swap (or move) into place and only then remove the old bundle. ```sh
apps/desktop/src/macUpdateInstaller.ts
install_update() {
/bin/rm -rf "$TARGET_APP"
/usr/bin/ditto "$SOURCE_APP" "$TARGET_APP"
}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/macUpdateInstaller.ts`:
- Around line 71-84: The install_update routine currently removes TARGET_APP
before copying, risking loss if ditto fails; change install_update (and the
osascript fallback) to copy SOURCE_APP into a sibling temporary bundle (e.g.,
TARGET_APP+".tmp" or use mktemp) first, verify the copy completed successfully,
then atomically rename/move the temp bundle over TARGET_APP (or use /bin/mv) to
replace it; keep TARGET_APP until the move succeeds so the old app is available
for rollback, and ensure wait_for_app_exit is still invoked before the final
swap; update references to SOURCE_APP and TARGET_APP in both the shell and
osascript paths accordingly.
In `@apps/desktop/src/main.ts`:
- Around line 901-906: The current branch calls
installDownloadedUnsignedMacUpdate() after backend shutdown and update-polling
was cleared, which can throw and leave the app half-shutdown; preflight the
unsigned installer instead (i.e., run the installer lookup/extract/bundle
discovery/script write/spawn steps that installDownloadedUnsignedMacUpdate()
performs) before shutting down backend and clearing polling, or wrap the call in
a try/catch that on error restarts the backend and re-enables polling before
rethrowing/logging; update the code paths around
installDownloadedUnsignedMacUpdate(), autoUpdater.quitAndInstall(), and
app.quit() so the installer preflight occurs safely or the system recovers on
failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a595743f-0d01-49a1-b5d7-48f7fa990172
📒 Files selected for processing (5)
apps/desktop/src/macCodeSigning.test.tsapps/desktop/src/macCodeSigning.tsapps/desktop/src/macUpdateInstaller.test.tsapps/desktop/src/macUpdateInstaller.tsapps/desktop/src/main.ts
| install_update() { | ||
| /bin/rm -rf "$TARGET_APP" | ||
| /usr/bin/ditto "$SOURCE_APP" "$TARGET_APP" | ||
| } | ||
|
|
||
| wait_for_app_exit | ||
|
|
||
| if ! install_update >/dev/null 2>&1; then | ||
| export SOURCE_APP TARGET_APP | ||
| /usr/bin/osascript <<'APPLESCRIPT' | ||
| set sourceApp to system attribute "SOURCE_APP" | ||
| set targetApp to system attribute "TARGET_APP" | ||
| do shell script "/bin/rm -rf " & quoted form of targetApp & " && /usr/bin/ditto " & quoted form of sourceApp & " " & quoted form of targetApp with administrator privileges | ||
| APPLESCRIPT |
There was a problem hiding this comment.
Don't remove the installed app before the replacement copy has succeeded.
Both install paths delete "$TARGET_APP" before running ditto. If the copy then fails (corrupt archive, disk full, interrupted copy), the current bundle is already gone and there is nothing left to relaunch. Copy into a sibling temp bundle first, then swap it into place only after the new bundle is complete, keeping the old bundle as rollback until the move succeeds.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/macUpdateInstaller.ts` around lines 71 - 84, The
install_update routine currently removes TARGET_APP before copying, risking loss
if ditto fails; change install_update (and the osascript fallback) to copy
SOURCE_APP into a sibling temporary bundle (e.g., TARGET_APP+".tmp" or use
mktemp) first, verify the copy completed successfully, then atomically
rename/move the temp bundle over TARGET_APP (or use /bin/mv) to replace it; keep
TARGET_APP until the move succeeds so the old app is available for rollback, and
ensure wait_for_app_exit is still invoked before the final swap; update
references to SOURCE_APP and TARGET_APP in both the shell and osascript paths
accordingly.
| if (process.platform === "darwin" && !isMacDeveloperIdSignedBuild()) { | ||
| installDownloadedUnsignedMacUpdate(); | ||
| app.quit(); | ||
| } else { | ||
| autoUpdater.quitAndInstall(); | ||
| } |
There was a problem hiding this comment.
Preflight the unsigned-mac installer before tearing the app down.
installDownloadedUnsignedMacUpdate() still does zip lookup, extraction, bundle discovery, script write, and process spawn, and any of those can throw. By this point Line 900 has already stopped the backend and Line 898 has already cleared update polling, so the catch path leaves the current session half-shutdown. Either do the manual-install preflight before Line 900, or explicitly restart the backend and polling when this branch fails.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/main.ts` around lines 901 - 906, The current branch calls
installDownloadedUnsignedMacUpdate() after backend shutdown and update-polling
was cleared, which can throw and leave the app half-shutdown; preflight the
unsigned installer instead (i.e., run the installer lookup/extract/bundle
discovery/script write/spawn steps that installDownloadedUnsignedMacUpdate()
performs) before shutting down backend and clearing polling, or wrap the call in
a try/catch that on error restarts the backend and re-enables polling before
rethrowing/logging; update the code paths around
installDownloadedUnsignedMacUpdate(), autoUpdater.quitAndInstall(), and
app.quit() so the installer preflight occurs safely or the system recovers on
failure.
What Changed
isMacDeveloperIdSignedBuild()inapps/desktop/src/main.tsto detect Developer ID signing at runtime usingcodesignand cache the result.apps/desktop/src/macCodeSigning.tswithhasDeveloperIdApplicationAuthority()to parse code-sign output and verify Developer ID authority.apps/desktop/src/macUpdateInstaller.tswith:buildMacManualUpdateInstallScript()to generate a detached installer shell script that waits for app exit, replaces the bundle viaditto, prompts for admin if needed, and relaunches.findFirstAppBundlePath()to recursively find an.appbundle in the extracted update directory.resolveDownloadedMacUpdateZipPath()to locate the downloaded update.zipfrom electron-updater's file list.installDownloadedUpdate()inapps/desktop/src/main.tsto branch at install time: signed builds continue usingautoUpdater.quitAndInstall(), while unsigned builds use the new manual fallback path.setDownloadedUpdateFiles()andclearDownloadedUpdateFiles()to retain downloaded file paths for manual install.getAutoUpdateDisabledReason()inapps/desktop/src/updateState.tsso updates remain enabled for unsigned builds.apps/desktop/src/macCodeSigning.test.tsandapps/desktop/src/macUpdateInstaller.test.ts.Why
Electron's
autoUpdater.quitAndInstall()on macOS uses Squirrel.Mac, which requires a signed Developer ID application to install without blocking. Unsigned DMG installs (e.g., local test builds opened via Gatekeeper's "Open Anyway") would download updates but fail silently when clicked to install, providing no restart or installation feedback. This PR provides a working update flow for unsigned builds by extracting the downloaded.zipand replacing the app bundle viadittofrom a detached installer script, preserving the signed-build behavior using the native Squirrel path.UI Changes
None (backend updater behavior only).
Summary by CodeRabbit
Release Notes
New Features
Tests