Skip to content

Fix cross-volume update failure that deletes the app without replacing it #294

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.

Sign up for GitHub

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
augiekim wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix cross-volume update failure that deletes the app without replacing it #294

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 24 additions & 8 deletions Squirrel/SQRLInstaller.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:&copyError]) {
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];
}
Expand Down
64 changes: 64 additions & 0 deletions SquirrelTests/SQRLInstallerSpec.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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));
Expand Down