Skip to content

refactor: harden how ShipIt is launched and re-launched#306

Closed
MarshallOfSound wants to merge 1 commit into
sam/patch-prevent-downgradesfrom
sam/patch-shipit-launch
Closed

refactor: harden how ShipIt is launched and re-launched#306
MarshallOfSound wants to merge 1 commit into
sam/patch-prevent-downgradesfrom
sam/patch-shipit-launch

Conversation

@MarshallOfSound

@MarshallOfSound MarshallOfSound commented May 2, 2026

Copy link
Copy Markdown
Collaborator

Stacked on #305.

Upstreams refactor_use_posix_spawn_instead_of_nstask_so_we_can_disclaim_the.patch + fix_trigger_shipit_mach_service_after_smjobsubmit_to_unblock.patch + chore_turn_off_launchapplicationaturl_deprecation_errors_in_squirrel.patch. No new specs — launchd/TCC/mach not unit-reachable; existing remote-ShipIt SQRLInstallerSpec exercises the new posix_spawn path.


Part of upstreaming electron/patches/squirrel.mac/ into this repo.

Fixes #196.

@MarshallOfSound MarshallOfSound force-pushed the sam/patch-prevent-downgrades branch from c81b054 to 7a9582a Compare May 2, 2026 23:15
@MarshallOfSound MarshallOfSound force-pushed the sam/patch-shipit-launch branch from 11f3e97 to 13c624e Compare May 2, 2026 23:15
@MarshallOfSound MarshallOfSound force-pushed the sam/patch-shipit-launch branch from 13c624e to 175faf8 Compare May 3, 2026 00:08
@MarshallOfSound MarshallOfSound force-pushed the sam/patch-prevent-downgrades branch from 7a9582a to 0d46ed1 Compare May 3, 2026 00:08
@MarshallOfSound MarshallOfSound force-pushed the sam/patch-shipit-launch branch from 175faf8 to c6e71c6 Compare May 3, 2026 00:09
MarshallOfSound added a commit that referenced this pull request May 3, 2026
…310)

Fixes the intermittent `SQRLTerminationListenerSpec` failures the matrix
CI surfaced (#305, #306).

`launchApplicationAtURL:` returns immediately while the spawned process
is still doing its `NSApplication` startup. `-[SQRLTerminationListener
waitForTermination]` then calls
`runningApplicationsWithBundleIdentifier:` — if the app hasn't checked
in with Launch Services yet, that returns an empty array and the signal
completes immediately.

Both observed failure modes are this same window:
- `observedApp == nil` + `completed == YES` from the start (one-instance
test)
- `completed` going true after only one of two terminates
(multi-instance — listener only saw one app at subscribe time)

Block in the fixture until `app.finishedLaunching` so every caller
(these two specs plus the seven `SKIP_IF_RUNNING_ON_TRAVIS`-tagged
`SQRLUpdaterSpec` cases) sees a stable starting state.
[upstream] electron/patches/squirrel.mac/refactor_use_posix_spawn_instead_of_nstask_so_we_can_disclaim_the.patch
[upstream] electron/patches/squirrel.mac/fix_trigger_shipit_mach_service_after_smjobsubmit_to_unblock.patch
[upstream] electron/patches/squirrel.mac/chore_turn_off_launchapplicationaturl_deprecation_errors_in_squirrel.patch

- ShipIt-main.m: use posix_spawn with the responsibility-disclaim flag
  when re-spawning the post-install relauncher so a hot-swapped binary
  can't inherit the parent's TCC permissions
- SQRLShipItLauncher.m: after SMJobSubmit, send a one-shot Mach message
  to the registered service so launchd starts the job even when the
  user domain is in on-demand-only mode (pending OS update)
- ShipIt-main.m: pragma-suppress the launchApplicationAtURL deprecation
  (replacement is 10.15+; tracked at electron/electron#43168)

No new specs: launchd / TCC / mach-port behaviour isn't reachable in a
unit test. Existing remote-ShipIt SQRLInstallerSpec specs continue to
exercise the new spawn path.
@MarshallOfSound

Copy link
Copy Markdown
Collaborator Author

Folded into #312.

@MarshallOfSound MarshallOfSound deleted the branch sam/patch-prevent-downgrades May 3, 2026 09:25
@MarshallOfSound MarshallOfSound deleted the sam/patch-shipit-launch branch May 3, 2026 09:25
MarshallOfSound added a commit that referenced this pull request May 3, 2026
Upstreams the remaining `electron/patches/squirrel.mac/` patches into
this repo so Electron can eventually drop them. Six commits, **8 files,
+537/−51**, **63 → 74 tests**.

| Commit | Upstreams | Tests added |
|---|---|---|
| `feat: SquirrelMacEnableDirectContentsWrite` |
`feat_add_new_squirrel_mac_bundle_installation_method_behind_flag` |
parent-dir-untouched e2e |
| `fix: abort install if app running; resolve target path once` |
`fix_abort_installation_attempt_at_the_final_mile_if_the_app_is` +
`fix_resolve_target_bundle_path_once_at_start_of_install` |
symlink-target rejected, abort-if-running |
| `feat: ElectronSquirrelPreventDowngrades` |
`feat_add_ability_to_prevent_version_downgrades` |
`+isVersionAllowedForUpdate:from:` units |
| `refactor: harden ShipIt launch` |
`refactor_use_posix_spawn_instead_of_nstask…` +
`fix_trigger_shipit_mach_service_after_smjobsubmit…` +
`chore_turn_off_launchapplicationaturl_deprecation…` | (existing
remote-ShipIt spec exercises new path) |
| `refactor: non-deprecated NSKeyedArchiver` |
`refactor_use_non-deprecated_nskeyedarchiver_apis` |
`SQRLInstallerOwnedBundle` round-trip |
| `fix: prune orphaned staged updates` |
`fix_clean_up_orphaned_staged_updates_before_downloading_new_update` |
unit prune + e2e bounded-count; replaces `xit` |

Fixes #124. Fixes #196. Fixes #264.

> [!NOTE]
> Two adaptations should be backported to Electron's patches:
> - `SQRLUpdater.m`: `BOOL launchPrivileged = !targetWritable` in the
original `direct-contents-write` patch shadowed the outer var — it was a
dead store
> - `SQRLInstaller.m`: `runningApplicationsWithBundleIdentifier:` throws
on `nil`; guarded it

Replaces the stacked #303 / #304 / #305 / #306 / #307 / #309. #302
(strict codesign validation) stays separate — it touches only
`SQRLCodeSignature.m`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant