diff --git a/Squirrel/SQRLInstaller.m b/Squirrel/SQRLInstaller.m index 43d7d67..d0f2741 100644 --- a/Squirrel/SQRLInstaller.m +++ b/Squirrel/SQRLInstaller.m @@ -631,17 +631,33 @@ - (RACSignal *)installItemToURL:(NSURL *)targetURL fromURL:(NSURL *)sourceURL { catch:^(NSError *error) { if (![error.domain isEqual:NSPOSIXErrorDomain] || error.code != EXDEV) return [RACSignal error:error]; - // If the locations lie on two different volumes, remove the - // destination by hand, then perform a move. - [NSFileManager.defaultManager removeItemAtURL:targetContentsURL error:NULL]; + // The source and target are on different volumes. Stage the source in a + // UUID-named temporary path on the same volume as the target, then + // rename() atomically. This ensures the target slot is never left empty: + // the existing item (if any) stays in place until the rename succeeds. + NSURL *stagingContentsURL = [targetContentsURL.URLByDeletingLastPathComponent + URLByAppendingPathComponent:[[NSUUID UUID] UUIDString]]; + + NSError *copyError = nil; + if (![NSFileManager.defaultManager copyItemAtURL:sourceContentsURL toURL:stagingContentsURL error:©Error]) { + NSString *description = [NSString stringWithFormat:NSLocalizedString(@"Couldn't stage bundle %@ on target volume at %@", nil), sourceContentsURL, stagingContentsURL]; + return [RACSignal error:[self errorByAddingDescription:description code:SQRLInstallerErrorMovingAcrossVolumes toError:copyError]]; + } - if ([NSFileManager.defaultManager moveItemAtURL:sourceContentsURL toURL:targetContentsURL error:&error]) { - NSLog(@"Moved bundle contents across volumes from %@ to %@", sourceContentsURL, targetContentsURL); + if (rename(stagingContentsURL.path.fileSystemRepresentation, targetContentsURL.path.fileSystemRepresentation) == 0) { + [NSFileManager.defaultManager removeItemAtURL:sourceContentsURL error:NULL]; + NSLog(@"Installed bundle cross-volume from %@ to %@", sourceContentsURL, targetContentsURL); return [RACSignal empty]; - } else { - NSString *description = [NSString stringWithFormat:NSLocalizedString(@"Couldn't move bundle contents %@ across volumes to %@", nil), sourceContentsURL, targetContentsURL]; - return [RACSignal error:[self errorByAddingDescription:description code:SQRLInstallerErrorMovingAcrossVolumes toError:error]]; } + + int renameErrno = errno; + [NSFileManager.defaultManager removeItemAtURL:stagingContentsURL error:NULL]; + NSMutableDictionary *renameInfo = [NSMutableDictionary dictionary]; + const char *renameDesc = strerror(renameErrno); + if (renameDesc != NULL) renameInfo[NSLocalizedDescriptionKey] = @(renameDesc); + NSError *renameError = [NSError errorWithDomain:NSPOSIXErrorDomain code:renameErrno userInfo:renameInfo]; + NSString *description = [NSString stringWithFormat:NSLocalizedString(@"Couldn't rename staged bundle contents %@ to %@", nil), stagingContentsURL, targetContentsURL]; + return [RACSignal error:[self errorByAddingDescription:description code:SQRLInstallerErrorMovingAcrossVolumes toError:renameError]]; }] setNameWithFormat:@"%@ -installItemAtURL: %@ fromURL: %@", self, targetContentsURL, sourceContentsURL]; } diff --git a/SquirrelTests/SQRLInstallerSpec.m b/SquirrelTests/SQRLInstallerSpec.m index a177a2a..d4d7a7d 100644 --- a/SquirrelTests/SQRLInstallerSpec.m +++ b/SquirrelTests/SQRLInstallerSpec.m @@ -171,6 +171,26 @@ expect([NSDictionary dictionaryWithContentsOfURL:plistURL][SQRLBundleShortVersionStringKey]).withTimeout(SQRLLongTimeout).toEventually(equal(SQRLTestApplicationUpdatedShortVersionString)); }); +it(@"should install an update from one external volume to another", ^{ + // Both the update bundle and the target app live on separate disk images, + // so every rename() call in the install path hits EXDEV. This exercises + // the cross-volume staging path twice: once when the target app is backed + // up onto the system volume and again when the update is placed onto the + // target volume. + NSURL *updateDiskImageURL = [self createAndMountDiskImageNamed:@"TestUpdate" fromDirectory:updateURL.URLByDeletingLastPathComponent]; + NSURL *crossVolumeUpdateURL = [updateDiskImageURL URLByAppendingPathComponent:updateURL.lastPathComponent]; + + NSURL *targetDiskImageURL = [self createAndMountDiskImageNamed:@"TestApplicationExternal" fromDirectory:self.testApplicationURL.URLByDeletingLastPathComponent]; + NSURL *crossVolumeTargetURL = [targetDiskImageURL URLByAppendingPathComponent:self.testApplicationURL.lastPathComponent]; + + SQRLShipItRequest *request = [[SQRLShipItRequest alloc] initWithUpdateBundleURL:crossVolumeUpdateURL targetBundleURL:crossVolumeTargetURL bundleIdentifier:nil launchAfterInstallation:NO useUpdateBundleName:NO]; + + [self installWithRequest:request remote:YES]; + + NSURL *plistURL = [crossVolumeTargetURL URLByAppendingPathComponent:@"Contents/Info.plist"]; + expect([NSDictionary dictionaryWithContentsOfURL:plistURL][SQRLBundleShortVersionStringKey]).withTimeout(SQRLLongTimeout).toEventually(equal(SQRLTestApplicationUpdatedShortVersionString)); +}); + describe(@"with backup restoration", ^{ __block NSURL *targetURL; @@ -222,6 +242,50 @@ }); }); +describe(@"with backup restoration when target is on another volume", ^{ + // Models the VS Code scenario: the app lives on a non-system volume. ShipIt + // backed it up to system temp (ownedBundle.temporaryURL) before it was + // killed. On relaunch with too many attempts it must restore the backed-up + // app back across volumes to its original location. + __block NSURL *externalTargetURL; + + beforeEach(^{ + NSURL *diskImageURL = [self createAndMountDiskImageNamed:@"TestApplicationRollback" fromDirectory:self.testApplicationURL.URLByDeletingLastPathComponent]; + externalTargetURL = [diskImageURL URLByAppendingPathComponent:self.testApplicationURL.lastPathComponent]; + + // Simulate acquireTargetBundleURLForRequest: having moved the app off the + // external volume into system temp before ShipIt was killed. + NSURL *backupURL = [self.temporaryDirectoryURL URLByAppendingPathComponent:@"TestApplication Backup.app"]; + expect(@([NSFileManager.defaultManager moveItemAtURL:externalTargetURL toURL:backupURL error:NULL])).to(beTruthy()); + + SQRLCodeSignature *codeSignature = self.testApplicationSignature; + SQRLInstallerOwnedBundle *ownedBundle = [[SQRLInstallerOwnedBundle alloc] initWithOriginalURL:externalTargetURL temporaryURL:backupURL codeSignature:codeSignature]; + NSData *ownedBundleArchive = [NSKeyedArchiver archivedDataWithRootObject:ownedBundle]; + expect(ownedBundleArchive).notTo(beNil()); + + NSString *applicationIdentifier = self.shipItDirectoryManager.applicationIdentifier; + CFPreferencesSetValue((__bridge CFStringRef)SQRLShipItInstallationAttemptsKey, (__bridge CFPropertyListRef)@(4), (__bridge CFStringRef)applicationIdentifier, kCFPreferencesCurrentUser, kCFPreferencesCurrentHost); + CFPreferencesSetValue((__bridge CFStringRef)SQRLInstallerOwnedBundleKey, (__bridge CFDataRef)ownedBundleArchive, (__bridge CFStringRef)applicationIdentifier, kCFPreferencesCurrentUser, kCFPreferencesCurrentHost); + + BOOL synchronized = CFPreferencesSynchronize((__bridge CFStringRef)applicationIdentifier, kCFPreferencesCurrentUser, kCFPreferencesCurrentHost); + expect(@(synchronized)).to(beTruthy()); + }); + + it(@"should restore the app to the external volume after too many install attempts", ^{ + SQRLShipItRequest *request = [[SQRLShipItRequest alloc] initWithUpdateBundleURL:updateURL targetBundleURL:externalTargetURL bundleIdentifier:nil launchAfterInstallation:NO useUpdateBundleName:NO]; + [self installWithRequest:request remote:YES]; + + // App must be back at its original location on the external volume with + // the original version — not at the system-temp backup path. + NSURL *plistURL = [externalTargetURL URLByAppendingPathComponent:@"Contents/Info.plist"]; + expect([NSDictionary dictionaryWithContentsOfURL:plistURL][SQRLBundleShortVersionStringKey]).withTimeout(SQRLLongTimeout).toEventually(equal(SQRLTestApplicationOriginalShortVersionString)); + + __block NSError *verifyError; + expect(@([[self.testApplicationSignature verifyBundleAtURL:externalTargetURL] waitUntilCompleted:&verifyError])).toEventually(beTruthy()); + expect(verifyError).to(beNil()); + }); +}); + it(@"should disallow writing the updated application except by the owner", ^{ NSString *command = [NSString stringWithFormat:@"chmod -R 0777 '%@'", updateURL.path]; expect(@(system(command.UTF8String))).to(equal(@0));