From bff7e03197cc74416dc3478877bbf222e6a39ed4 Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Sat, 5 Sep 2026 23:45:49 -0700 Subject: [PATCH] fix: remove ShipIt's launchd job once installation completes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A job submitted with SMJobSubmit is a registration, not a one-shot: after ShipIt exits, the job stays in the launchd domain as "not running" until the login session ends. macOS 27 shows a Dock tile for any app with a registered background job, so an updated-and-quit app looks like it is still running (Squirrel/Squirrel.Mac#21). ShipIt now removes its own job on the terminal success paths (install completed, or cancelled because the app relaunched), right before exit(0). Failure exits keep the registration so KeepAlive can respawn ShipIt to retry the install. The removal passes a NULL authorization: by that point the installer has replaced the bundle containing the running ShipIt binary, so authd can no longer validate its code signature and AuthorizationCreate fails with errAuthorizationDenied even for root. SMJobRemove with NULL authorization authorizes on the caller instead — root may modify the system domain and any caller its own user domain — which never prompts and works with the binary already gone. SIGTERM is ignored during removal because launchd terminates a job it is removing, and wait=false avoids deadlocking on our own exit. --- Squirrel/ShipIt-main.m | 50 ++++++++++++++++++++++++++ SquirrelTests/QuickSpec+SQRLFixtures.m | 20 +++++++---- SquirrelTests/SQRLInstallerSpec.m | 18 ++++++++++ 3 files changed, 82 insertions(+), 6 deletions(-) diff --git a/Squirrel/ShipIt-main.m b/Squirrel/ShipIt-main.m index e26c0f1..ebb8fe2 100644 --- a/Squirrel/ShipIt-main.m +++ b/Squirrel/ShipIt-main.m @@ -17,6 +17,10 @@ #include #include #include +#include +#include + +#import #import "NSError+SQRLVerbosityExtensions.h" #import "RACSignal+SQRLTransactionExtensions.h" @@ -87,6 +91,50 @@ static void drainMachServicePort(const char *serviceName) { } } +// Remove ShipIt's own launchd job on the way out. A job submitted with +// SMJobSubmit is a registration, not a one-shot: after ShipIt exits, the +// job would otherwise stay in the domain as "not running" until the login +// session ends (or indefinitely, for the system domain), and macOS 27 shows +// a Dock tile for any app with a registered background job — making an +// updated-and-quit app look like it is still running. Removal is limited to +// the terminal success paths; failure exits must keep the registration so +// the KeepAlive/SuccessfulExit policy can respawn ShipIt to retry. +// +// SMJobRemove is deprecated but is the counterpart of the SMJobSubmit that +// created the job (SQRLShipItLauncher); there is no other API that can +// remove a submitted job. +static void removeOwnLaunchdJob(NSString *jobLabel) { + // launchd terminates a running job as part of removing it. Ignore + // SIGTERM so we still exit through our own exit() call with the + // intended status rather than dying by signal mid-cleanup. + signal(SIGTERM, SIG_IGN); + + // Privileged installs run ShipIt as root in the system domain; + // unprivileged ones run as the user in their domain. + // + // The authorization is deliberately NULL. By this point the installer + // has replaced (and typically deleted) the bundle containing this + // running ShipIt binary, so authd can no longer validate our code + // signature on disk and AuthorizationCreate fails with + // errAuthorizationDenied, even for root (verified empirically). + // SMJobRemove with a NULL authorization instead authorizes on the + // caller itself — root may modify the system domain and any caller its + // own user domain — which neither consults authd nor can present a + // prompt, and works with the binary already gone. + CFStringRef domain = (geteuid() == 0 ? kSMDomainSystemLaunchd : kSMDomainUserLaunchd); + +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" + // `wait` must be false: waiting blocks until the job (i.e. this + // process) exits, which would deadlock against our own exit(). + CFErrorRef cfError = NULL; + if (!SMJobRemove(domain, (__bridge CFStringRef)jobLabel, NULL, false, &cfError)) { + NSError *error = CFBridgingRelease(cfError); + NSLog(@"Could not remove ShipIt launchd job %@: %@", jobLabel, error); + } +#pragma clang diagnostic pop +} + // Waits for all instances of the target application (as described in the // `request`) to exit, then sends completed. static RACSignal *waitForTerminationIfNecessary(SQRLShipItRequest *request) { @@ -231,6 +279,7 @@ static void installRequest(RACSignal *readRequestSignal, NSString *applicationId NSLog(@"Installation cancelled: %@", error); clearInstallationAttempts(applicationIdentifier); drainMachServicePort(applicationIdentifier.UTF8String); + removeOwnLaunchdJob(applicationIdentifier); exit(EXIT_SUCCESS); } else { NSLog(@"Installation error: %@", error); @@ -238,6 +287,7 @@ static void installRequest(RACSignal *readRequestSignal, NSString *applicationId } } completed:^{ drainMachServicePort(applicationIdentifier.UTF8String); + removeOwnLaunchdJob(applicationIdentifier); exit(EXIT_SUCCESS); }]; } diff --git a/SquirrelTests/QuickSpec+SQRLFixtures.m b/SquirrelTests/QuickSpec+SQRLFixtures.m index 2c83203..e77e317 100644 --- a/SquirrelTests/QuickSpec+SQRLFixtures.m +++ b/SquirrelTests/QuickSpec+SQRLFixtures.m @@ -236,13 +236,17 @@ - (NSRunningApplication *)launchTestApplicationWithEnvironment:(NSDictionary *)e // Remove ShipIt's launchd job so it doesn't relaunch itself. // SMJobRemove is deprecated but has no test-suitable replacement - // (SMAppService requires registration via the same API). + // (SMAppService requires registration via the same API). The job + // is usually already gone — ShipIt removes it on success — so a + // job-not-found error is expected and not worth logging. CFErrorRef error = NULL; #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wdeprecated-declarations" if (!SMJobRemove(kSMDomainUserLaunchd, CFSTR("com.github.Squirrel.TestApplication.ShipIt"), NULL, true, &error)) { - NSLog(@"Could not remove ShipIt job after tests: %@", error); - if (error != NULL) CFRelease(error); + NSError *removeError = CFBridgingRelease(error); + if (![removeError.domain isEqual:(__bridge id)kSMErrorDomainLaunchd] || removeError.code != kSMErrorJobNotFound) { + NSLog(@"Could not remove ShipIt job after tests: %@", removeError); + } } #pragma clang diagnostic pop }]; @@ -338,7 +342,10 @@ - (void)submitShipItRequest:(SQRLShipItRequest *)request { #pragma clang diagnostic ignored "-Wdeprecated-declarations" static NSNumber *SQRLShipItLastExitStatus(NSString *jobLabel) { NSDictionary *job = CFBridgingRelease(SMJobCopyDictionary(kSMDomainUserLaunchd, (__bridge CFStringRef)jobLabel)); - if (job == nil || job[@"PID"] != nil) return nil; + // ShipIt removes its own launchd job when it finishes successfully, so + // once the job has been submitted, its disappearance means a clean exit. + if (job == nil) return @0; + if (job[@"PID"] != nil) return nil; return job[@"LastExitStatus"]; } #pragma clang diagnostic pop @@ -348,8 +355,9 @@ - (void)waitForShipItJobToExitWithLabel:(NSString *)jobLabel { // spawn — it does not wait for ShipIt to actually run. Block until launchd // reports the job has exited so callers can assert on the install result // synchronously instead of racing Nimble's default 1s poll timeout. - // LastExitStatus only appears once the process has run and exited, which - // avoids the brief no-PID window before launchd spawns it. + // LastExitStatus only appears once the process has run and exited (and the + // job disappears entirely on a successful run), which avoids the brief + // no-PID window before launchd spawns it. expect(SQRLShipItLastExitStatus(jobLabel)).withTimeout(SQRLLongTimeout).toEventuallyNot(beNil()); } diff --git a/SquirrelTests/SQRLInstallerSpec.m b/SquirrelTests/SQRLInstallerSpec.m index 928bc10..8bf8c6b 100644 --- a/SquirrelTests/SQRLInstallerSpec.m +++ b/SquirrelTests/SQRLInstallerSpec.m @@ -19,6 +19,7 @@ #import "QuickSpec+SQRLFixtures.h" +#import #import @interface SQRLInstaller (SQRLTestingHooks) @@ -53,6 +54,23 @@ - (RACSignal *)deleteOwnedBundleAtURL:(NSURL *)bundleURL; expect(self.testApplicationBundleVersion).to(equal(SQRLTestApplicationUpdatedShortVersionString)); }); +it(@"should remove its launchd job once the install completes", ^{ + SQRLShipItRequest *request = [[SQRLShipItRequest alloc] initWithUpdateBundleURL:updateURL targetBundleURL:self.testApplicationURL bundleIdentifier:nil launchAfterInstallation:NO useUpdateBundleName:NO]; + + [self installWithRequest:request remote:YES]; + + expect(self.testApplicationBundleVersion).to(equal(SQRLTestApplicationUpdatedShortVersionString)); + + // ShipIt removes its own job on success (SMJobRemove with wait=false), + // so the registration may lag its exit by a moment — poll for it to + // disappear rather than asserting immediately. A lingering registration + // causes macOS 27 to show a Dock tile for the updated app after quit. +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" + expect(CFBridgingRelease(SMJobCopyDictionary(kSMDomainUserLaunchd, (__bridge CFStringRef)self.shipItDirectoryManager.applicationIdentifier))).withTimeout(SQRLLongTimeout).toEventually(beNil()); +#pragma clang diagnostic pop +}); + it(@"should round-trip the owned bundle through CFPreferences", ^{ SQRLInstaller *installer = [[SQRLInstaller alloc] initWithApplicationIdentifier:self.shipItDirectoryManager.applicationIdentifier]; SQRLInstallerOwnedBundle *original = [[SQRLInstallerOwnedBundle alloc] initWithOriginalURL:self.testApplicationURL temporaryURL:updateURL codeSignature:self.testApplicationSignature];