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];