diff --git a/lib/gui/app/os/windows-network-drives.js b/lib/gui/app/os/windows-network-drives.js old mode 100644 new mode 100755 index c4a7e025..a2abdfb3 --- a/lib/gui/app/os/windows-network-drives.js +++ b/lib/gui/app/os/windows-network-drives.js @@ -23,59 +23,9 @@ const _ = require('lodash') const os = require('os') const Path = require('path') const process = require('process') -const tmp = require('tmp') const { promisify } = require('util') -/** - * @summary returns { path: String, cleanup: Function } - * @function - * - * @returns {Promise<{ path: String, cleanup: Function }>} - * - * @example - * tmpFileAsync() - * .then({ path, cleanup } => { - * console.log(path) - * cleanup() - * }); - */ -const tmpFileAsync = () => { - return new Promise((resolve, reject) => { - const options = { - - // Close the file once it's created - discardDescriptor: true, - - // Wmic fails with "Invalid global switch" when the "/output:" switch filename contains a dash ("-") - prefix: 'tmp' - } - tmp.file(options, (error, path, _fd, cleanup) => { - if (error) { - reject(error) - } else { - resolve({ path, cleanup }) - } - }) - }) -} - -/** - * @summary Disposer for tmpFileAsync, calls cleanup() - * @function - * - * @returns {Disposer<{ path: String, cleanup: Function }>} - * - * @example - * await Bluebird.using(tmpFileDisposer(), ({ path }) => { - * console.log(path); - * }) - */ -const tmpFileDisposer = () => { - return Bluebird.resolve(tmpFileAsync()) - .disposer(({ cleanup }) => { - cleanup() - }) -} +const { tmpFileDisposer } = require('../../../shared/utils') const readFileAsync = promisify(fs.readFile) @@ -99,7 +49,15 @@ exports.getWmicNetworkDrivesOutput = async () => { // We could also use wmic's "/output:" switch but it doesn't work when the filename // contains a space and the os temp dir may contain spaces ("D:\Windows Temp Files" for example). // So we just redirect to a file and read it afterwards as we know it will be ucs2 encoded. - return Bluebird.using(tmpFileDisposer(), async ({ path }) => { + const options = { + + // Close the file once it's created + discardDescriptor: true, + + // Wmic fails with "Invalid global switch" when the "/output:" switch filename contains a dash ("-") + prefix: 'tmp' + } + return Bluebird.using(tmpFileDisposer(options), async ({ path }) => { const command = [ Path.join(process.env.SystemRoot, 'System32', 'Wbem', 'wmic'), 'path', diff --git a/lib/shared/permissions.js b/lib/shared/permissions.js old mode 100644 new mode 100755 index c31533aa..2e89084f --- a/lib/shared/permissions.js +++ b/lib/shared/permissions.js @@ -14,17 +14,24 @@ * limitations under the License. */ +/* eslint-disable lodash/prefer-lodash-method,quotes,no-magic-numbers,require-jsdoc */ + 'use strict' -const os = require('os') -const nativeModule = require('./native-module') const Bluebird = require('bluebird') const childProcess = Bluebird.promisifyAll(require('child_process')) -const sudoPrompt = Bluebird.promisifyAll(require('sudo-prompt')) -const commandJoin = require('command-join') +const fs = require('fs') const _ = require('lodash') +const os = require('os') +const sudoPrompt = Bluebird.promisifyAll(require('sudo-prompt')) +const { promisify } = require('util') + const errors = require('./errors') +const { tmpFileDisposer } = require('./utils') + +const writeFileAsync = promisify(fs.writeFile) + /** * @summary The user id of the UNIX "superuser" * @constant @@ -90,68 +97,61 @@ exports.isElevatedUnixSync = () => { return (process.geteuid() === UNIX_SUPERUSER_USER_ID) } -/** - * @summary Get environment command prefix - * @function - * @private - * - * @param {Object} environment - environment map - * @returns {String[]} command arguments - * - * @example - * const commandPrefix = permissions.getEnvironmentCommandPrefix({ - * FOO: 'bar', - * BAR: 'baz' - * }); - * - * childProcess.execSync(_.join(_.concat(commandPrefix, [ 'mycommand' ]), ' ')); - */ -exports.getEnvironmentCommandPrefix = (environment) => { - const isWindows = os.platform() === 'win32' - - if (_.isEmpty(environment)) { - return [] - } - - const argv = _.flatMap(environment, (value, key) => { - if (_.isNil(value)) { - return [] - } - - if (isWindows) { - // Doing something like `set foo=bar &&` (notice - // the space before the first ampersand) would - // cause the environment variable's value to - // contain a trailing space. - // See https://superuser.com/a/57726 - return [ 'set', `${key}=${value}&&` ] - } - - return [ `${key}=${value}` ] - }) - - if (isWindows) { - // This is a trick to make the binary afterwards catch - // the environment variables set just previously. - return _.concat(argv, [ 'call' ]) - } - - return _.concat([ 'env' ], argv) +const escapeSh = (value) => { + // Make sure it's a string + // Replace ' -> '\'' (closing quote, escaped quote, opening quote) + // Surround with quotes + return `'${String(value).replace(/'/g, "'\\''")}'` } -/** - * @summary Quote a string - * @function - * @private - * - * @param {String} string - input string - * @returns {String} quoted string - * - * @example - * const result = quote('foo'); - */ -const quoteString = (string) => { - return `"${string}"` +const escapeParamCmd = (value) => { + // Make sure it's a string + // Escape " -> \" + // Surround with double quotes + return `"${String(value).replace(/"/g, '\\"')}"` +} + +const setEnvVarSh = (value, name) => { + return `export ${name}=${escapeSh(value)}` +} + +const setEnvVarCmd = (value, name) => { + return `set "${name}=${String(value)}"` +} + +// Exported for tests +exports.createLaunchScript = (command, argv, environment) => { + const isWindows = os.platform() === 'win32' + const lines = [] + if (isWindows) { + // Switch to utf8 + lines.push('chcp 65001') + } + const [ setEnvVarFn, escapeFn ] = isWindows ? [ setEnvVarCmd, escapeParamCmd ] : [ setEnvVarSh, escapeSh ] + lines.push(..._.map(environment, setEnvVarFn)) + lines.push([ command, ...argv ].map(escapeFn).join(' ')) + return lines.join(os.EOL) +} + +const elevateScriptWindows = async (path) => { + // './nativeModule' imported here as it only exists on windows + // TODO: replace this with sudo-prompt once https://github.com/jorangreef/sudo-prompt/issues/96 is fixed + const nativeModule = require('./native-module') + const elevateAsync = promisify(nativeModule.load('elevator').elevate) + + // '&' needs to be escaped here (but not when written to a .cmd file) + const cmd = [ 'cmd', '/c', escapeParamCmd(path).replace(/&/g, '^&') ] + const { cancelled } = await elevateAsync(cmd) + return { cancelled } +} + +const elevateScriptUnix = async (path, name) => { + const cmd = [ 'sh', escapeSh(path) ].join(' ') + const [ , stderr ] = await sudoPrompt.execAsync(cmd, { name }) + if (!_.isEmpty(stderr)) { + throw errors.createError({ title: stderr }) + } + return { cancelled: false } } /** @@ -178,73 +178,41 @@ const quoteString = (string) => { * } * }); */ -exports.elevateCommand = (command, options) => { +exports.elevateCommand = async (command, options) => { const isWindows = os.platform() === 'win32' - - const prefixedCommand = _.concat( - exports.getEnvironmentCommandPrefix(options.environment), - _.map(command, (string) => { - return isWindows ? quoteString(string) : string - }) - ) - - if (isWindows) { - const elevator = Bluebird.promisifyAll(nativeModule.load('elevator')) - return elevator.elevateAsync([ - 'cmd.exe', - '/c', - quoteString(_.join(prefixedCommand, ' ')) - ]).then((results) => { - return { - cancelled: results.cancelled + const launchScript = exports.createLaunchScript(command[0], command.slice(1), options.environment) + return Bluebird.using(tmpFileDisposer({ postfix: '.cmd' }), async ({ path }) => { + await writeFileAsync(path, launchScript) + if (isWindows) { + return elevateScriptWindows(path) + } + try { + return await elevateScriptUnix(path, options.applicationName) + } catch (error) { + // We're hardcoding internal error messages declared by `sudo-prompt`. + // There doesn't seem to be a better way to handle these errors, so + // for now, we should make sure we double check if the error messages + // have changed every time we upgrade `sudo-prompt`. + console.log('error', error) + if (_.includes(error.message, 'is not in the sudoers file')) { + throw errors.createUserError({ + title: "Your user doesn't have enough privileges to proceed", + description: 'This application requires sudo privileges to be able to write to drives' + }) + } else if (_.startsWith(error.message, 'Command failed:')) { + throw errors.createUserError({ + title: 'The elevated process died unexpectedly', + description: `The process error code was ${error.code}` + }) + } else if (error.message === 'User did not grant permission.') { + return { cancelled: true } + } else if (error.message === 'No polkit authentication agent found.') { + throw errors.createUserError({ + title: 'No polkit authentication agent found', + description: 'Please install a polkit authentication agent for your desktop environment of choice to continue' + }) } - }) - } - - return sudoPrompt.execAsync(commandJoin(prefixedCommand), { - name: options.applicationName - }).then((stdout, stderr) => { - if (!_.isEmpty(stderr)) { - throw errors.createError({ - title: stderr - }) + throw error } - - return { - cancelled: false - } - - // We're hardcoding internal error messages declared by `sudo-prompt`. - // There doesn't seem to be a better way to handle these errors, so - // for now, we should make sure we double check if the error messages - // have changed every time we upgrade `sudo-prompt`. - }).catch((error) => { - console.log('error', error.cause) - return _.includes(error.message, 'is not in the sudoers file') - }, () => { - throw errors.createUserError({ - title: 'Your user doesn\'t have enough privileges to proceed', - description: 'This application requires sudo privileges to be able to write to drives' - }) - }).catch((error) => { - return _.startsWith(error.message, 'Command failed:') - }, (error) => { - throw errors.createUserError({ - title: 'The elevated process died unexpectedly', - description: `The process error code was ${error.code}` - }) - }).catch({ - message: 'User did not grant permission.' - }, () => { - return { - cancelled: true - } - }).catch({ - message: 'No polkit authentication agent found.' - }, () => { - throw errors.createUserError({ - title: 'No polkit authentication agent found', - description: 'Please install a polkit authentication agent for your desktop environment of choice to continue' - }) }) } diff --git a/lib/shared/utils.js b/lib/shared/utils.js old mode 100644 new mode 100755 index c356da5f..88189a79 --- a/lib/shared/utils.js +++ b/lib/shared/utils.js @@ -19,6 +19,8 @@ const _ = require('lodash') const Bluebird = require('bluebird') const request = Bluebird.promisifyAll(require('request')) +const tmp = require('tmp') + const errors = require('./errors') /** @@ -168,3 +170,50 @@ exports.getConfig = (configUrl) => { return request.getAsync(configUrl, { json: true }) .get('body') } + +/** + * @summary returns { path: String, cleanup: Function } + * @function + * + * @param {Object} options - options + * + * @returns {Promise<{ path: String, cleanup: Function }>} + * + * @example + * tmpFileAsync() + * .then({ path, cleanup } => { + * console.log(path) + * cleanup() + * }); + */ +const tmpFileAsync = (options) => { + return new Promise((resolve, reject) => { + tmp.file(options, (error, path, _fd, cleanup) => { + if (error) { + reject(error) + } else { + resolve({ path, cleanup }) + } + }) + }) +} + +/** + * @summary Disposer for tmpFileAsync, calls cleanup() + * @function + * + * @param {Object} options - options + * + * @returns {Disposer<{ path: String, cleanup: Function }>} + * + * @example + * await Bluebird.using(tmpFileDisposer(), ({ path }) => { + * console.log(path); + * }) + */ +exports.tmpFileDisposer = (options) => { + return Bluebird.resolve(tmpFileAsync(options)) + .disposer(({ cleanup }) => { + cleanup() + }) +} diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index 306c9739..889853f5 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -3131,11 +3131,6 @@ "delayed-stream": "~1.0.0" } }, - "command-join": { - "version": "2.0.0", - "resolved": "https://registry.npmjs.org/command-join/-/command-join-2.0.0.tgz", - "integrity": "sha1-Uui5hPSHLZUv8b3IuYOX0nxxRM8=" - }, "commander": { "version": "2.8.1", "resolved": "https://registry.npmjs.org/commander/-/commander-2.8.1.tgz", @@ -13961,4 +13956,4 @@ } } } -} +} \ No newline at end of file diff --git a/package.json b/package.json index 42b2eff3..288c5607 100644 --- a/package.json +++ b/package.json @@ -49,7 +49,6 @@ "bluebird": "^3.5.3", "bootstrap-sass": "^3.3.6", "color": "^2.0.1", - "command-join": "^2.0.0", "d3": "^4.13.0", "debug": "^3.1.0", "electron-is-running-in-asar": "^1.0.0", diff --git a/tests/shared/permissions.spec.js b/tests/shared/permissions.spec.js index 31cf64ff..9a48c44a 100644 --- a/tests/shared/permissions.spec.js +++ b/tests/shared/permissions.spec.js @@ -14,6 +14,8 @@ * limitations under the License. */ +/* eslint-disable quotes */ + 'use strict' const m = require('mochainon') @@ -21,7 +23,7 @@ const os = require('os') const permissions = require('../../lib/shared/permissions') describe('Shared: permissions', function () { - describe('.getEnvironmentCommandPrefix()', function () { + describe('.createLaunchScript()', function () { describe('given windows', function () { beforeEach(function () { this.osPlatformStub = m.sinon.stub(os, 'platform') @@ -32,197 +34,62 @@ describe('Shared: permissions', function () { this.osPlatformStub.restore() }) - it('should return an empty array if no environment', function () { - m.chai.expect(permissions.getEnvironmentCommandPrefix()).to.deep.equal([]) - }) - - it('should return an empty array if the environment is an empty object', function () { - m.chai.expect(permissions.getEnvironmentCommandPrefix({})).to.deep.equal([]) - }) - - it('should create an environment command prefix out of one variable', function () { - m.chai.expect(permissions.getEnvironmentCommandPrefix({ - FOO: 'bar' - })).to.deep.equal([ - 'set', - 'FOO=bar&&', - 'call' - ]) - }) - - it('should create an environment command prefix out of many variables', function () { - m.chai.expect(permissions.getEnvironmentCommandPrefix({ - FOO: 'bar', - BAR: 'baz', - BAZ: 'qux' - })).to.deep.equal([ - 'set', - 'FOO=bar&&', - 'set', - 'BAR=baz&&', - 'set', - 'BAZ=qux&&', - 'call' - ]) - }) - - it('should ignore undefined and null variable values', function () { - m.chai.expect(permissions.getEnvironmentCommandPrefix({ - FOO: null, - BAR: 'qux', - BAZ: undefined - })).to.deep.equal([ - 'set', - 'BAR=qux&&', - 'call' - ]) - }) - - it('should stringify number values', function () { - m.chai.expect(permissions.getEnvironmentCommandPrefix({ - FOO: 1, - BAR: 0, - BAZ: -1 - })).to.deep.equal([ - 'set', - 'FOO=1&&', - 'set', - 'BAR=0&&', - 'set', - 'BAZ=-1&&', - 'call' - ]) + it('should escape environment variables and arguments', function () { + m.chai.expect( + permissions.createLaunchScript( + "C:\\Users\\Alice & Bob's Laptop\\\"what\"\\balenaEtcher", + [ + '"a Laser"', + 'arg1', + "'&/ ^ \\", + '" $ % *' + ], + { + key: 'value', + key2: ' " \' ^ & = + $ % / \\', + key3: 8 + } + ) + ).to.equal( + `chcp 65001${os.EOL}` + + `set "key=value"${os.EOL}` + + `set "key2= " ' ^ & = + $ % / \\"${os.EOL}` + + `set "key3=8"${os.EOL}` + + `"C:\\Users\\Alice & Bob's Laptop\\\\"what\\"\\balenaEtcher" "\\"a Laser\\"" "arg1" "'&/ ^ \\" "\\" $ % *"` + ) }) }) - describe('given linux', function () { - beforeEach(function () { - this.osPlatformStub = m.sinon.stub(os, 'platform') - this.osPlatformStub.returns('linux') - }) + for (const platform of [ 'linux', 'darwin' ]) { + describe(`given ${platform}`, function () { + beforeEach(function () { + this.osPlatformStub = m.sinon.stub(os, 'platform') + this.osPlatformStub.returns(platform) + }) - afterEach(function () { - this.osPlatformStub.restore() - }) + afterEach(function () { + this.osPlatformStub.restore() + }) - it('should return an empty array if no environment', function () { - m.chai.expect(permissions.getEnvironmentCommandPrefix()).to.deep.equal([]) + it('should escape environment variables and arguments', function () { + m.chai.expect( + permissions.createLaunchScript( + "/home/Alice & Bob's Laptop/\"what\"/balenaEtcher", + [ 'arg1', "'&/ ^ \\", '" $ % *' ], + { + key: 'value', + key2: ' " \' ^ & = + $ % / \\', + key3: 8 + } + ) + ).to.equal( + `export key='value'${os.EOL}` + + `export key2=' " '\\'' ^ & = + $ % / \\'${os.EOL}` + + `export key3='8'${os.EOL}` + + `'/home/Alice & Bob'\\''s Laptop/"what"/balenaEtcher' 'arg1' ''\\''&/ ^ \\' '" $ % *'` + ) + }) }) - - it('should return an empty array if the environment is an empty object', function () { - m.chai.expect(permissions.getEnvironmentCommandPrefix({})).to.deep.equal([]) - }) - - it('should create an environment command prefix out of one variable', function () { - m.chai.expect(permissions.getEnvironmentCommandPrefix({ - FOO: 'bar' - })).to.deep.equal([ - 'env', - 'FOO=bar' - ]) - }) - - it('should create an environment command prefix out of many variables', function () { - m.chai.expect(permissions.getEnvironmentCommandPrefix({ - FOO: 'bar', - BAR: 'baz', - BAZ: 'qux' - })).to.deep.equal([ - 'env', - 'FOO=bar', - 'BAR=baz', - 'BAZ=qux' - ]) - }) - - it('should ignore undefined and null variable values', function () { - m.chai.expect(permissions.getEnvironmentCommandPrefix({ - FOO: null, - BAR: 'qux', - BAZ: undefined - })).to.deep.equal([ - 'env', - 'BAR=qux' - ]) - }) - - it('should stringify number values', function () { - m.chai.expect(permissions.getEnvironmentCommandPrefix({ - FOO: 1, - BAR: 0, - BAZ: -1 - })).to.deep.equal([ - 'env', - 'FOO=1', - 'BAR=0', - 'BAZ=-1' - ]) - }) - }) - - describe('given darwin', function () { - beforeEach(function () { - this.osPlatformStub = m.sinon.stub(os, 'platform') - this.osPlatformStub.returns('darwin') - }) - - afterEach(function () { - this.osPlatformStub.restore() - }) - - it('should return an empty array if no environment', function () { - m.chai.expect(permissions.getEnvironmentCommandPrefix()).to.deep.equal([]) - }) - - it('should return an empty array if the environment is an empty object', function () { - m.chai.expect(permissions.getEnvironmentCommandPrefix({})).to.deep.equal([]) - }) - - it('should create an environment command prefix out of one variable', function () { - m.chai.expect(permissions.getEnvironmentCommandPrefix({ - FOO: 'bar' - })).to.deep.equal([ - 'env', - 'FOO=bar' - ]) - }) - - it('should create an environment command prefix out of many variables', function () { - m.chai.expect(permissions.getEnvironmentCommandPrefix({ - FOO: 'bar', - BAR: 'baz', - BAZ: 'qux' - })).to.deep.equal([ - 'env', - 'FOO=bar', - 'BAR=baz', - 'BAZ=qux' - ]) - }) - - it('should ignore undefined and null variable values', function () { - m.chai.expect(permissions.getEnvironmentCommandPrefix({ - FOO: null, - BAR: 'qux', - BAZ: undefined - })).to.deep.equal([ - 'env', - 'BAR=qux' - ]) - }) - - it('should stringify number values', function () { - m.chai.expect(permissions.getEnvironmentCommandPrefix({ - FOO: 1, - BAR: 0, - BAZ: -1 - })).to.deep.equal([ - 'env', - 'FOO=1', - 'BAR=0', - 'BAZ=-1' - ]) - }) - }) + } }) })