Skip to content

fix: Squirrel.Mac crash when zip extraction fails #47271

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

nikwen
Copy link
Member

@nikwen nikwen commented May 27, 2025

Description of Change

Fixes #47270

Squirrel.Mac launches /usr/bin/ditto to extract the update zip file. When that process fails to launch, the (deprecated) launch function will raise an NSInvalidArgumentException. This exception wasn't caught. Thus, the app crashed.

This PR migrates the code to the (non-deprecated) launchAndReturnError function and adds proper error handling.

I've also included a test to prevent this from regressing.

This is the first Objective-C code I've ever written, so please be extra careful during your review.

I've tested the change as follows:

All autoUpdater tests pass (incl. the newly added test).

nikwen@MacBook spec % e test --no-remote -g "autoUpdater"
Running "/usr/local/bin/node ./script/spec-runner.js -g autoUpdater" in /Volumes/Electron/electron/src/electron
Triggering runners: main

Running: Main process specs
ok 1 autoUpdater module checkForUpdates emits an error on Windows if the feed URL is not set # SKIP -
ok 2 autoUpdater module getFeedURL returns an empty string by default
ok 3 autoUpdater module getFeedURL correctly fetches the previously set FeedURL # SKIP -
ok 4 autoUpdater module setFeedURL on Mac or Windows sets url successfully using old (url, headers) syntax
ok 5 autoUpdater module setFeedURL on Mac or Windows throws if no url is provided when using the old style
ok 6 autoUpdater module setFeedURL on Mac or Windows sets url successfully using new ({ url }) syntax
ok 7 autoUpdater module setFeedURL on Mac or Windows throws if no url is provided when using the new style
ok 8 autoUpdater module setFeedURL on Mac emits an error when the application is unsigned # SKIP -
ok 9 autoUpdater module setFeedURL on Mac does not throw if default is the serverType # SKIP -
ok 10 autoUpdater module setFeedURL on Mac does not throw if json is the serverType # SKIP -
ok 11 autoUpdater module setFeedURL on Mac does throw if an unknown string is the serverType # SKIP -
ok 12 autoUpdater module quitAndInstall emits an error on Windows when no update is available # SKIP -
ok 13 autoUpdater behavior should have a valid code signing identity
ok 14 autoUpdater behavior should fail to set the feed URL when the app is not signed # SKIP -
ok 15 autoUpdater behavior should cleanly set the feed URL when the app is signed
ok 16 autoUpdater behavior with update server should hit the update endpoint when checkForUpdates is called
ok 17 autoUpdater behavior with update server should hit the update endpoint with customer headers when checkForUpdates is called
ok 18 autoUpdater behavior with update server should hit the download endpoint when an update is available and error if the file is bad
ok 19 autoUpdater behavior with update server should hit the download endpoint when an update is available and update successfully when the zip is provided
ok 20 autoUpdater behavior with update server should hit the download endpoint when an update is available and update successfully when the zip is provided even after a different update was staged
ok 21 autoUpdater behavior with update server should update to lower version numbers
ok 22 autoUpdater behavior with update server should abort the update if the application is still running when ShipIt kicks off
ok 23 autoUpdater behavior with update server should hit the download endpoint when an update is available and fail when the zip signature is invalid
ok 24 autoUpdater behavior with update server should hit the download endpoint when an update is available and fail when the ShipIt binary is a symlink
ok 25 autoUpdater behavior with update server should hit the download endpoint when an update is available and fail when the Electron Framework is modified
ok 26 autoUpdater behavior with update server should hit the download endpoint when an update is available and fail when the zip extraction process fails to launch
ok 27 autoUpdater behavior with update server should hit the download endpoint when an update is available and update successfully when the zip is provided with JSON update mode
ok 28 autoUpdater behavior with update server should hit the download endpoint when an update is available and not update in JSON update mode when the currentRelease is older than the current version
ok 29 autoUpdater behavior with update server with ElectronSquirrelPreventDowngrades enabled should not update to lower version numbers
ok 30 autoUpdater behavior with update server with ElectronSquirrelPreventDowngrades enabled should not update to version strings that are not simple Major.Minor.Patch
ok 31 autoUpdater behavior with update server with ElectronSquirrelPreventDowngrades enabled should still update to higher version numbers
ok 32 autoUpdater behavior with update server with ElectronSquirrelPreventDowngrades enabled should compare version numbers correctly
ok 33 autoUpdater behavior with update server with SquirrelMacEnableDirectContentsWrite enabled should hit the download endpoint when an update is available and update successfully when the zip is provided leaving the parent directory untouched
# tests 25
# pass 25
# fail 0
1..33
[1755:0527/181855.901631:ERROR:base/process/kill_posix.cc:33] waitpid(1757): No child processes (10)
✓ Electron main process tests passed.

The newly added test crashes with SIGTRAP on main.

nikwen@MacBook spec % e test --no-remote -g "zip extraction process" --electronVersion 38.0.0-nightly.20250526 
Running "/usr/local/bin/node ./script/spec-runner.js -g zip extraction process --electronVersion=38.0.0-nightly.20250526" in /Volumes/Electron/electron/src/electron
Only running: [ 'main' ]
Running against Electron undefined

Running: Main process specs
Unhandled exception in main spec runner: AssertionError: expected 'SIGTRAP' to equal null
    at ChildProcess.<anonymous> (/Volumes/Electron/electron/src/electron/spec/lib/codesign-helpers.ts:72:25)
    at ChildProcess.emit (node:events:518:28)
    at Process.ChildProcess._handle.onexit (node:internal/child_process:293:12) {
  showDiff: true,
  actual: 'SIGTRAP',
  expected: null,
  operator: 'strictEqual'
}
not ok 1 autoUpdater behavior with update server should hit the download endpoint when an update is available and fail when the zip extraction process fails to launch
  expected 'SIGTRAP' to equal null
  AssertionError: expected 'SIGTRAP' to equal null
      at ChildProcess.<anonymous> (electron/spec/lib/codesign-helpers.ts:72:25)
      at ChildProcess.emit (node:events:518:28)
      at Process.ChildProcess._handle.onexit (node:internal/child_process:293:12)
✗ Electron tests failed with code 1.
ERROR Error: Command failed: /usr/local/bin/node ./script/spec-runner.js -g zip extraction process --electronVersion=38.0.0-nightly.20250526
    at genericNodeError (node:internal/errors:984:15)
    at wrappedFn (node:internal/errors:538:14)
    at checkExecSyncError (node:child_process:891:11)
    at Object.execFileSync (node:child_process:927:15)
    at runSpecRunner (/Users/nikwen/.electron_build_tools/src/e-test.js:33:16)
    at Command.<anonymous> (/Users/nikwen/.electron_build_tools/src/e-test.js:76:7)
    at Command.listener [as _actionHandler] (/Users/nikwen/.electron_build_tools/node_modules/commander/lib/command.js:480:17)
    at /Users/nikwen/.electron_build_tools/node_modules/commander/lib/command.js:1234:65
    at Command._chainOrCall (/Users/nikwen/.electron_build_tools/node_modules/commander/lib/command.js:1151:12)
    at Command._parseCommand (/Users/nikwen/.electron_build_tools/node_modules/commander/lib/command.js:1234:27)

CC @MarshallOfSound

Checklist

Release Notes

Notes: Fixed crash in autoUpdater on macOS when zip extraction failed.

@electron-cation electron-cation bot added the new-pr 🌱 PR opened recently label May 27, 2025
@nikwen nikwen force-pushed the nikwen/fix-squirrel-mac-crash branch from bfbe132 to 9ae9762 Compare May 27, 2025 01:40
@nikwen nikwen force-pushed the nikwen/fix-squirrel-mac-crash branch from 9ae9762 to f06dbd9 Compare May 27, 2025 13:31
@nikwen nikwen changed the title fix: Squirrel.Mac crash when zip extraction process fails to launch fix: Squirrel.Mac crash when zip extraction fails May 27, 2025
@nikwen nikwen requested a review from MarshallOfSound May 27, 2025 16:29
@nikwen nikwen marked this pull request as ready for review May 27, 2025 16:30
@nikwen nikwen requested a review from a team as a code owner May 27, 2025 16:30
@nikwen nikwen added target/34-x-y PR should also be added to the "34-x-y" branch. target/35-x-y PR should also be added to the "35-x-y" branch. target/36-x-y PR should also be added to the "36-x-y" branch. target/37-x-y PR should also be added to the "37-x-y" branch. semver/patch backwards-compatible bug fixes labels May 27, 2025
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened recently label May 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver/patch backwards-compatible bug fixes target/34-x-y PR should also be added to the "34-x-y" branch. target/35-x-y PR should also be added to the "35-x-y" branch. target/36-x-y PR should also be added to the "36-x-y" branch. target/37-x-y PR should also be added to the "37-x-y" branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash while Squirrel.Mac extracts update zip file
1 participant