fix(GUI): prevent flashing the drive where the source image is located

Currently Etcher will allow you to flash an image to the same drive that
contains the image. As a way to protect against that case we introduce
the concept of a "source drive", which means a drive that contains the
source image.

This commit adds the following logic around this new concept:

- Don't auto-select a source drive
- De-select an already selected drive if an image inside it is selected
- Disable the drive in the drive selector modal
- Add a "Source Drive" badge to the drive in the drive selector modal

Fixes: https://github.com/resin-io/etcher/issues/830
Change-Type: minor
Changelog-Entry: Prevent flashing the drive where the source image is located.
Signed-off-by: Juan Cruz Viotti <jviotti@openmailbox.org>
This commit is contained in:
Juan Cruz Viotti 2017-01-30 16:47:23 -04:00
parent 16340b412a
commit 95d4f2f608
8 changed files with 417 additions and 36 deletions

View File

@ -40,6 +40,11 @@
<i class="glyphicon glyphicon-hdd"></i>
System Drive</span>
<span class="label label-danger"
ng-show="modal.constraints.isSourceDrive(drive, modal.state.getImage())">
<i class="glyphicon glyphicon-circle-arrow-down"></i>
Source Drive</span>
</footer>
</div>
<span class="tick tick--success"

View File

@ -95,8 +95,8 @@ const storeReducer = (state, action) => {
const drive = _.first(action.data);
// Even if there's no image selectected, we need to call `isDriveValid`
// to check if the drive is locked, and `{}` works fine with it
// Even if there's no image selected, we need to call several
// drive/image related checks, and `{}` works fine with them
const image = state.getIn([ 'selection', 'image' ], Immutable.fromJS({})).toJS();
if (_.every([
@ -268,7 +268,7 @@ const storeReducer = (state, action) => {
return _.attempt(() => {
if (selectedDrive && !_.every([
constraints.isDriveLargeEnough(selectedDrive.toJS(), action.data),
constraints.isDriveValid(selectedDrive.toJS(), action.data),
constraints.isDriveSizeRecommended(selectedDrive.toJS(), action.data)
])) {
return storeReducer(state, {

View File

@ -17,6 +17,7 @@
'use strict';
const _ = require('lodash');
const pathIsInside = require('path-is-inside');
/**
* @summary Check if a drive is locked
@ -65,6 +66,52 @@ exports.isSystemDrive = (drive) => {
return Boolean(_.get(drive, 'system', false));
};
/**
* @summary Check if a drive is source drive
* @function
* @public
*
* @description
* In the context of Etcher, a source drive is a drive
* containing the image.
*
* @param {Object} drive - drive
* @param {Object} image - image
* @returns {Boolean} whether the drive is a source drive
*
*
* @example
* if (constraints.isSourceDrive({
* device: '/dev/disk2',
* name: 'My Drive',
* size: 123456789,
* protected: true,
* system: true,
* mountpoints: [
* {
* path: '/Volumes/Untitled'
* }
* ]
* }, {
* path: '/Volumes/Untitled/image.img',
* size: 1000000000
* })) {
* console.log('This drive is a source drive!');
* }
*/
exports.isSourceDrive = (drive, image) => {
const mountpoints = _.get(drive, 'mountpoints', []);
const imagePath = _.get(image, 'path');
if (!imagePath || _.isEmpty(mountpoints)) {
return false;
}
return _.some(_.map(mountpoints, (mountpoint) => {
return pathIsInside(imagePath, mountpoint.path);
}));
};
/**
* @summary Check if a drive is large enough for an image
* @function
@ -114,7 +161,11 @@ exports.isDriveLargeEnough = (drive, image) => {
* }
*/
exports.isDriveValid = (drive, image) => {
return !this.isDriveLocked(drive) && this.isDriveLargeEnough(drive, image);
return _.every([
!this.isDriveLocked(drive),
this.isDriveLargeEnough(drive, image),
!this.isSourceDrive(drive, image)
]);
};
/**

5
npm-shrinkwrap.json generated
View File

@ -1298,6 +1298,11 @@
"from": "path-exists@>=2.0.0 <3.0.0",
"resolved": "https://registry.npmjs.org/path-exists/-/path-exists-2.1.0.tgz"
},
"path-is-inside": {
"version": "1.0.2",
"from": "path-is-inside@>=1.0.2 <2.0.0",
"resolved": "https://registry.npmjs.org/path-is-inside/-/path-is-inside-1.0.2.tgz"
},
"path-key": {
"version": "1.0.0",
"from": "path-key@>=1.0.0 <2.0.0",

View File

@ -85,6 +85,7 @@
"lzma-native": "^1.5.2",
"node-ipc": "^8.9.2",
"node-stream-zip": "^1.3.4",
"path-is-inside": "^1.0.2",
"read-chunk": "^2.0.0",
"redux": "^3.5.2",
"redux-localstorage": "^0.4.1",

View File

@ -1,6 +1,7 @@
'use strict';
const m = require('mochainon');
const path = require('path');
const angular = require('angular');
require('angular-mocks');
@ -108,9 +109,15 @@ describe('Browser: DrivesModel', function() {
describe('given a selected image and no selected drive', function() {
beforeEach(function() {
if (process.platform === 'win32') {
this.imagePath = 'E:\\bar\\foo.img';
} else {
this.imagePath = '/mnt/bar/foo.img';
}
SelectionStateModel.removeDrive();
SelectionStateModel.setImage({
path: 'foo.img',
path: this.imagePath,
size: 999999999,
recommendedDriveSize: 2000000000
});
@ -220,6 +227,27 @@ describe('Browser: DrivesModel', function() {
m.chai.expect(SelectionStateModel.hasDrive()).to.be.false;
});
it('should not auto-select a source drive', function() {
m.chai.expect(SelectionStateModel.hasDrive()).to.be.false;
DrivesModel.setDrives([
{
device: '/dev/sdb',
name: 'Foo',
size: 2000000000,
mountpoints: [
{
path: path.dirname(this.imagePath)
}
],
system: false,
protected: false
}
]);
m.chai.expect(SelectionStateModel.hasDrive()).to.be.false;
});
});
});

View File

@ -1,6 +1,8 @@
'use strict';
const m = require('mochainon');
const _ = require('lodash');
const path = require('path');
const angular = require('angular');
require('angular-mocks');
@ -485,6 +487,41 @@ describe('Browser: SelectionState', function() {
SelectionStateModel.removeImage();
});
it('should de-select a previously selected source drive', function() {
const imagePath = _.attempt(() => {
if (process.platform === 'win32') {
return 'E:\\bar\\foo.img';
}
return '/mnt/bar/foo.img';
});
DrivesModel.setDrives([
{
device: '/dev/disk1',
name: 'USB Drive',
size: 1200000000,
mountpoints: [
{
path: path.dirname(imagePath)
}
],
protected: false
}
]);
SelectionStateModel.setDrive('/dev/disk1');
m.chai.expect(SelectionStateModel.hasDrive()).to.be.true;
SelectionStateModel.setImage({
path: imagePath,
size: 999999999
});
m.chai.expect(SelectionStateModel.hasDrive()).to.be.false;
SelectionStateModel.removeImage();
});
});
});

View File

@ -1,6 +1,7 @@
'use strict';
const m = require('mochainon');
const path = require('path');
const constraints = require('../../lib/shared/drive-constraints');
describe('Shared: DriveConstraints', function() {
@ -92,6 +93,214 @@ describe('Shared: DriveConstraints', function() {
});
describe('.isSourceDrive()', function() {
it('should return false if no image', function() {
const result = constraints.isSourceDrive({
device: '/dev/disk2',
name: 'USB Drive',
size: 999999999,
protected: true,
system: false
}, undefined);
m.chai.expect(result).to.be.false;
});
it('should return false if no drive', function() {
const result = constraints.isSourceDrive(undefined, {
path: '/Volumes/Untitled/image.img'
});
m.chai.expect(result).to.be.false;
});
it('should return false if there are no mount points', function() {
const result = constraints.isSourceDrive({
device: '/dev/disk2',
name: 'USB Drive',
size: 999999999,
protected: true,
system: false
}, {
path: '/Volumes/Untitled/image.img'
});
m.chai.expect(result).to.be.false;
});
describe('given Windows paths', function() {
beforeEach(function() {
this.separator = path.sep;
path.sep = '\\';
});
afterEach(function() {
path.sep = this.separator;
});
it('should return true if the image lives directly inside a mount point of the drive', function() {
const result = constraints.isSourceDrive({
mountpoints: [
{
path: 'E:'
},
{
path: 'F:'
}
]
}, {
path: 'E:\\image.img'
});
m.chai.expect(result).to.be.true;
});
it('should return true if the image lives inside a mount point of the drive', function() {
const result = constraints.isSourceDrive({
mountpoints: [
{
path: 'E:'
},
{
path: 'F:'
}
]
}, {
path: 'E:\\foo\\bar\\image.img'
});
m.chai.expect(result).to.be.true;
});
it('should return false if the image does not live inside a mount point of the drive', function() {
const result = constraints.isSourceDrive({
mountpoints: [
{
path: 'E:'
},
{
path: 'F:'
}
]
}, {
path: 'G:\\image.img'
});
m.chai.expect(result).to.be.false;
});
it('should return false if the image is in a mount point that is a substring of the image mount point', function() {
const result = constraints.isSourceDrive({
mountpoints: [
{
path: 'E:\\fo'
}
]
}, {
path: 'E:\\foo/image.img'
});
m.chai.expect(result).to.be.false;
});
});
describe('given UNIX paths', function() {
beforeEach(function() {
this.separator = path.sep;
path.sep = '/';
});
afterEach(function() {
path.sep = this.separator;
});
it('should return true if the mount point is / and the image lives directly inside it', function() {
const result = constraints.isSourceDrive({
mountpoints: [
{
path: '/'
}
]
}, {
path: '/image.img'
});
m.chai.expect(result).to.be.true;
});
it('should return true if the image lives directly inside a mount point of the drive', function() {
const result = constraints.isSourceDrive({
mountpoints: [
{
path: '/Volumes/A'
},
{
path: '/Volumes/B'
}
]
}, {
path: '/Volumes/A/image.img'
});
m.chai.expect(result).to.be.true;
});
it('should return true if the image lives inside a mount point of the drive', function() {
const result = constraints.isSourceDrive({
mountpoints: [
{
path: '/Volumes/A'
},
{
path: '/Volumes/B'
}
]
}, {
path: '/Volumes/A/foo/bar/image.img'
});
m.chai.expect(result).to.be.true;
});
it('should return false if the image does not live inside a mount point of the drive', function() {
const result = constraints.isSourceDrive({
mountpoints: [
{
path: '/Volumes/A'
},
{
path: '/Volumes/B'
}
]
}, {
path: '/Volumes/C/image.img'
});
m.chai.expect(result).to.be.false;
});
it('should return false if the image is in a mount point that is a substring of the image mount point', function() {
const result = constraints.isSourceDrive({
mountpoints: [
{
path: '/Volumes/fo'
}
]
}, {
path: '/Volumes/foo/image.img'
});
m.chai.expect(result).to.be.false;
});
});
});
describe('.isDriveLargeEnough()', function() {
it('should return true if the drive size is greater than the image size', function() {
@ -254,48 +463,93 @@ describe('Shared: DriveConstraints', function() {
describe('.isDriveValid()', function() {
describe('given drive is large enough', function() {
beforeEach(function() {
if (process.platform === 'win32') {
this.mountpoint = 'E:\\foo';
} else {
this.mountpoint = '/mnt/foo';
}
this.drive = {
device: '/dev/disk2',
name: 'My Drive',
mountpoints: [
{
path: this.mountpoint
}
],
size: 4000000000
};
});
describe('given the drive is locked', function() {
beforeEach(function() {
this.drive = {
device: '/dev/disk2',
name: 'My Drive',
size: 4000000000
};
this.image = {
path: 'rpi.img',
size: 2000000000
};
});
it('should return true if drive is not locked', function() {
this.drive.protected = false;
m.chai.expect(constraints.isDriveValid(this.drive, this.image)).to.be.true;
});
it('should return false if drive is locked', function() {
this.drive.protected = true;
m.chai.expect(constraints.isDriveValid(this.drive, this.image)).to.be.false;
});
it('should return false if the drive is not large enough and is a source drive', function() {
m.chai.expect(constraints.isDriveValid(this.drive, {
path: path.join(this.mountpoint, 'rpi.img'),
size: 5000000000
})).to.be.false;
});
it('should return false if the drive is not large enough and is not a source drive', function() {
m.chai.expect(constraints.isDriveValid(this.drive, {
path: path.resolve(this.mountpoint, '../bar/rpi.img'),
size: 5000000000
})).to.be.false;
});
it('should return false if the drive is large enough and is a source drive', function() {
m.chai.expect(constraints.isDriveValid(this.drive, {
path: path.join(this.mountpoint, 'rpi.img'),
size: 2000000000
})).to.be.false;
});
it('should return false if the drive is large enough and is not a source drive', function() {
m.chai.expect(constraints.isDriveValid(this.drive, {
path: path.resolve(this.mountpoint, '../bar/rpi.img'),
size: 2000000000
})).to.be.false;
});
});
describe('given drive is not large enough', function() {
describe('given the drive is not locked', function() {
beforeEach(function() {
this.drive = {
device: '/dev/disk2',
name: 'My Drive',
size: 1000000000
};
this.image = {
path: 'rpi.img',
size: 2000000000
};
this.drive.protected = false;
});
it('should return false', function() {
m.chai.expect(constraints.isDriveValid(this.drive, this.image)).to.be.false;
it('should return false if the drive is not large enough and is a source drive', function() {
m.chai.expect(constraints.isDriveValid(this.drive, {
path: path.join(this.mountpoint, 'rpi.img'),
size: 5000000000
})).to.be.false;
});
it('should return false if the drive is not large enough and is not a source drive', function() {
m.chai.expect(constraints.isDriveValid(this.drive, {
path: path.resolve(this.mountpoint, '../bar/rpi.img'),
size: 5000000000
})).to.be.false;
});
it('should return false if the drive is large enough and is a source drive', function() {
m.chai.expect(constraints.isDriveValid(this.drive, {
path: path.join(this.mountpoint, 'rpi.img'),
size: 2000000000
})).to.be.false;
});
it('should return true if the drive is large enough and is not a source drive', function() {
m.chai.expect(constraints.isDriveValid(this.drive, {
path: path.resolve(this.mountpoint, '../bar/rpi.img'),
size: 2000000000
})).to.be.true;
});
});