diff --git a/src/backuptask.js b/src/backuptask.js index c3ead0d4c..70dd8fc71 100644 --- a/src/backuptask.js +++ b/src/backuptask.js @@ -60,7 +60,7 @@ async function checkPreconditions(backupConfig, dataLayout) { let used = 0; for (const localPath of dataLayout.localPaths()) { debug(`checkPreconditions: getting disk usage of ${localPath}`); - const result = await shell.promises.exec('checkPreconditions', `du -Dsb --exclude='*.lock' --exclude='dovecot.list.index.log.*' "${localPath}"`, {}); + const result = await shell.promises.execArgs('checkPreconditions', 'du', [ '-Dsb', '--exclude=*.lock', '--exclude=dovecot.list.index.log.*', localPath], {}); used += parseInt(result, 10); } diff --git a/src/database.js b/src/database.js index 3de506174..f1d77d093 100644 --- a/src/database.js +++ b/src/database.js @@ -78,7 +78,7 @@ async function clear() { await fs.promises.writeFile('/tmp/extra.cnf', `[client]\nhost=${gDatabase.hostname}\nuser=${gDatabase.username}\npassword=${gDatabase.password}\ndatabase=${gDatabase.name}`, 'utf8'); const cmd = 'mysql --defaults-extra-file=/tmp/extra.cnf -Nse "SHOW TABLES" | grep -v "^migrations$" | while read table; do mysql --defaults-extra-file=/tmp/extra.cnf -e "SET FOREIGN_KEY_CHECKS = 0; TRUNCATE TABLE $table"; done'; - await shell.promises.exec('clear_database', cmd, {}); + await shell.promises.exec('clear_database', cmd, { shell: '/bin/bash' }); } async function query() { @@ -136,7 +136,7 @@ async function importFromFile(file) { const cmd = `/usr/bin/mysql -h "${gDatabase.hostname}" -u ${gDatabase.username} -p${gDatabase.password} ${gDatabase.name} < ${file}`; await query('CREATE DATABASE IF NOT EXISTS box'); - const [error] = await safe(shell.promises.exec('importFromFile', cmd, {})); + const [error] = await safe(shell.promises.exec('importFromFile', cmd, { shell: '/bin/bash' })); if (error) throw new BoxError(BoxError.DATABASE_ERROR, error); } @@ -150,6 +150,6 @@ async function exportToFile(file) { const cmd = `/usr/bin/mysqldump -h "${gDatabase.hostname}" -u root -p${gDatabase.password} ${colStats} --single-transaction --routines --triggers ${gDatabase.name} > "${file}"`; - const [error] = await safe(shell.promises.exec('exportToFile', cmd, {})); + const [error] = await safe(shell.promises.exec('exportToFile', cmd, { shell: '/bin/bash' })); if (error) throw new BoxError(BoxError.DATABASE_ERROR, error); } diff --git a/src/reverseproxy.js b/src/reverseproxy.js index 69f289007..81a133888 100644 --- a/src/reverseproxy.js +++ b/src/reverseproxy.js @@ -153,8 +153,7 @@ async function validateCertificate(subdomain, domain, certificate) { // -checkhost checks for SAN or CN exclusively. SAN takes precedence and if present, ignores the CN. const fqdn = dns.fqdn(subdomain, domain); - const [checkHostError, checkHostOutput] = await safe(shell.promises.exec('validateCertificate', `openssl x509 -noout -checkhost "${fqdn}"`, { input: cert })); - console.log(checkHostError, checkHostOutput); + const [checkHostError, checkHostOutput] = await safe(shell.promises.exec('validateCertificate', `openssl x509 -noout -checkhost ${fqdn}`, { input: cert })); if (checkHostError) throw new BoxError(BoxError.BAD_FIELD, 'Could not validate certificate'); if (checkHostOutput.indexOf('does match certificate') === -1) throw new BoxError(BoxError.BAD_FIELD, `Certificate is not valid for this domain. Expecting ${fqdn}`); diff --git a/src/shell.js b/src/shell.js index 552b7f2ba..78e30de52 100644 --- a/src/shell.js +++ b/src/shell.js @@ -12,46 +12,15 @@ exports = module.exports = { promises: { exec: util.promisify(exec), - execFile: util.promisify(execFile), + execArgs: util.promisify(execArgs), sudo: util.promisify(sudo) } }; const SUDO = '/usr/bin/sudo'; -// default encoding utf8, shell, handles input, full command -function exec(tag, cmd, options, callback) { - assert.strictEqual(typeof tag, 'string'); - assert.strictEqual(typeof cmd, 'string'); - assert.strictEqual(typeof options, 'object'); - assert.strictEqual(typeof callback, 'function'); - - debug(`${tag} exec: ${cmd}`); - - const execOptions = Object.assign({ encoding: 'utf8', shell: false }, options); - - // https://github.com/nodejs/node/issues/25231 - const cp = child_process.exec(cmd, execOptions, function (error, stdout, stderr) { - let e = null; - if (error) { - e = new BoxError(BoxError.SHELL_ERROR, `${tag} errored with code ${error.code} message ${error.message}`); - e.stdout = stdout; // when promisified, this is the way to get stdout - e.stderr = stderr; // when promisified, this is the way to get stderr - - debug(`${tag}: ${cmd} errored`, error); - } - - callback(e, stdout); - }); - - if (options.input) { - cp.stdin.write(options.input); - cp.stdin.end(); - } -} - // default encoding utf8, no shell, separate args -function execFile(tag, file, args, options, callback) { +function execArgs(tag, file, args, options, callback) { assert.strictEqual(typeof tag, 'string'); assert.strictEqual(typeof file, 'string'); assert(Array.isArray(args)); @@ -82,6 +51,17 @@ function execFile(tag, file, args, options, callback) { } } +// default encoding utf8, shell, handles input, full command +function exec(tag, cmd, options, callback) { + assert.strictEqual(typeof tag, 'string'); + assert.strictEqual(typeof cmd, 'string'); + assert.strictEqual(typeof options, 'object'); + assert.strictEqual(typeof callback, 'function'); + + const [file, ...args] = cmd.split(' '); + execArgs(tag, file, args, options, callback); +} + // use this when you are afraid of how arguments will split up function spawn(tag, file, args, options, callback) { assert.strictEqual(typeof tag, 'string'); diff --git a/src/storage/filesystem.js b/src/storage/filesystem.js index 78515ea0f..c94cdaac9 100644 --- a/src/storage/filesystem.js +++ b/src/storage/filesystem.js @@ -187,7 +187,7 @@ async function copy(apiConfig, oldFilePath, newFilePath, progressCallback) { let cpOptions = ((apiConfig.provider !== PROVIDER_MOUNTPOINT && apiConfig.provider !== PROVIDER_CIFS) || apiConfig.preserveAttributes) ? '-a' : '-dR'; cpOptions += apiConfig.noHardlinks ? '' : 'l'; // this will hardlink backups saving space - const [copyError] = await safe(shell.promises.execFile('copy', 'cp', [ cpOptions, oldFilePath, newFilePath ], {})); + const [copyError] = await safe(shell.promises.execArgs('copy', 'cp', [ cpOptions, oldFilePath, newFilePath ], {})); if (copyError) throw new BoxError(BoxError.EXTERNAL_ERROR, copyError.message); } @@ -212,7 +212,7 @@ async function removeDir(apiConfig, pathPrefix, progressCallback) { progressCallback({ message: `Removing directory ${pathPrefix}` }); - const [error] = await safe(shell.promises.execFile('removeDir', 'rm', [ '-rf', pathPrefix ], {})); + const [error] = await safe(shell.promises.execArgs('removeDir', 'rm', [ '-rf', pathPrefix ], {})); if (error) throw new BoxError(BoxError.EXTERNAL_ERROR, error.message); } diff --git a/src/test/shell-test.js b/src/test/shell-test.js index 297a49a4c..f648cbc17 100644 --- a/src/test/shell-test.js +++ b/src/test/shell-test.js @@ -11,18 +11,18 @@ const BoxError = require('../boxerror.js'), shell = require('../shell.js'); describe('shell', function () { - describe('execFile', function () { + describe('execArgs', function () { it('can run valid program', async function () { - await shell.promises.execFile('test', 'ls', [ '-l' ], {}); + await shell.promises.execArgs('test', 'ls', [ '-l' ], {}); }); it('fails on invalid program', async function () { - const [error] = await safe(shell.promises.execFile('test', 'randomprogram', [ ], {})); + const [error] = await safe(shell.promises.execArgs('test', 'randomprogram', [ ], {})); expect(error.reason).to.be(BoxError.SHELL_ERROR); }); it('fails on failing program', async function () { - const [error] = await safe(shell.promises.execFile('test', '/usr/bin/false', [ ], {})); + const [error] = await safe(shell.promises.execArgs('test', '/usr/bin/false', [ ], {})); expect(error.reason).to.be(BoxError.SHELL_ERROR); }); }); @@ -50,10 +50,6 @@ describe('shell', function () { }); describe('exec', function () { - it('exec a valid shell program', async function () { - await shell.promises.exec('test', 'ls -l | wc -c', {}); - }); - it('exec throws for invalid program', async function () { const [error] = await safe(shell.promises.exec('test', 'cannotexist', {})); expect(error.reason).to.be(BoxError.SHELL_ERROR); @@ -68,5 +64,14 @@ describe('shell', function () { const [error] = await safe(shell.promises.exec('sleeping', 'sleep 20', { timeout: 1000 })); expect(error.reason).to.be(BoxError.SHELL_ERROR); }); + + it('cannot exec a shell program by default', async function () { + const [error] = await safe(shell.promises.exec('test', 'ls -l | wc -c', {})); + expect(error.reason).to.be(BoxError.SHELL_ERROR); + }); + + it('cannot exec a shell program b', async function () { + await shell.promises.exec('test', 'ls -l | wc -c', { shell: '/bin/bash' }); + }); }); });