diff --git a/lib/actions/join.ts b/lib/actions/join.ts index 9cbb2302..e9539018 100644 --- a/lib/actions/join.ts +++ b/lib/actions/join.ts @@ -60,6 +60,7 @@ export const join: CommandDefinition = { }, ], + permission: 'user', primary: true, async action(params, options, done) { diff --git a/lib/utils/helpers.ts b/lib/utils/helpers.ts index 4ccdd672..f207acd8 100644 --- a/lib/utils/helpers.ts +++ b/lib/utils/helpers.ts @@ -14,6 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ +import { InitializeEmitter, OperationState } from 'balena-device-init'; import BalenaSdk = require('balena-sdk'); import Bluebird = require('bluebird'); import chalk from 'chalk'; @@ -22,7 +23,7 @@ import os = require('os'); import visuals = require('resin-cli-visuals'); import * as ShellEscape from 'shell-escape'; -import { InitializeEmitter, OperationState } from 'balena-device-init'; +import { ExpectedError } from '../errors'; const balena = BalenaSdk.fromSharedOptions(); @@ -190,10 +191,21 @@ export function getApplication(applicationName: string) { return balena.models.application.get(applicationName, extraOptions); } -// A function to reliably execute a command -// in all supported operating systems, including -// different Windows environments like `cmd.exe` -// and `Cygwin`. +/** + * Choose between 'cmd.exe' and '/bin/sh' for running the given command string, + * depending on the value of `os.platform()`. + * When writing new code, consider whether it would be possible to avoid using a + * shell at all, using the which() function in this module to obtain a program's + * full path, executing the program directly and passing the arguments as an + * array instead of a long string. Avoiding a shell has several benefits: + * - Avoids the need to shell-escape arguments, especially nested commands. + * - Bypasses the incompatibilities between cmd.exe and /bin/sh. + * - Reduces the security risks of lax input validation. + * Code example avoiding a shell: + * const program = await which('ssh'); + * const args = ['root@192.168.1.1', 'cat /etc/os-release']; + * const child = spawn(program, args); + */ export function getSubShellCommand(command: string) { if (os.platform() === 'win32') { return { @@ -292,6 +304,21 @@ export function getManualSortCompareFunction( }; } +/** + * Decide whether the current shell (that executed the CLI process) is a Windows + * 'cmd.exe' shell, including PowerShell, by checking a few environment + * variables. + */ +export function isWindowsComExeShell() { + return ( + // neither bash nor sh (e.g. not MSYS, MSYS2, Cygwin, WSL) + process.env.SHELL == null && + // Windows cmd.exe or PowerShell + process.env.ComSpec != null && + process.env.ComSpec.endsWith('cmd.exe') + ); +} + /** * Shell argument escaping compatible with sh, bash and Windows cmd.exe. * @param arg Arguments to be escaped @@ -303,18 +330,10 @@ export function getManualSortCompareFunction( * env.ComSpec (cmd.exe) on Windows, even when running on MSYS / MSYS2. */ export function shellEscape(args: string[], detectShell = false): string[] { - let isWindowsCmdExeShell: boolean; - if (detectShell) { - isWindowsCmdExeShell = - // neither bash nor sh (e.g. not MSYS, MSYS2, WSL) - process.env.SHELL == null && - // Windows cmd.exe or PowerShell - process.env.ComSpec != null && - process.env.ComSpec.endsWith('cmd.exe'); - } else { - isWindowsCmdExeShell = process.platform === 'win32'; - } - if (isWindowsCmdExeShell) { + const isCmdExe = detectShell + ? isWindowsComExeShell() + : process.platform === 'win32'; + if (isCmdExe) { return args.map(v => windowsCmdExeEscapeArg(v)); } else { const shellEscapeFunc: typeof ShellEscape = require('shell-escape'); @@ -356,3 +375,28 @@ export async function workaroundWindowsDnsIssue(ipOrHostname: string) { await new Promise(r => setTimeout(r, delay)); } } + +/** + * Error handling wrapper around the npm `which` package: + * "Like the unix which utility. Finds the first instance of a specified + * executable in the PATH environment variable. Does not cache the results, + * so hash -r is not needed when the PATH changes." + * + * @param program Basename of a program, for example 'ssh' + * @returns The program's full path, e.g. 'C:\WINDOWS\System32\OpenSSH\ssh.EXE' + */ +export async function which(program: string): Promise { + const whichMod = await import('which'); + let programPath: string; + try { + programPath = await whichMod(program); + } catch (err) { + if (err.code === 'ENOENT') { + throw new ExpectedError( + `'${program}' program not found. Is it installed?`, + ); + } + throw err; + } + return programPath; +} diff --git a/lib/utils/promote.ts b/lib/utils/promote.ts index b26f5fea..5104ca0b 100644 --- a/lib/utils/promote.ts +++ b/lib/utils/promote.ts @@ -17,9 +17,9 @@ import * as BalenaSdk from 'balena-sdk'; import { stripIndent } from 'common-tags'; -import { runCommand } from './helpers'; +import { ExpectedError } from '../errors'; import Logger = require('./logger'); -import { exec, execBuffered } from './ssh'; +import { exec, execBuffered, getDeviceOsRelease } from './ssh'; const MIN_BALENAOS_VERSION = 'v2.14.0'; @@ -29,14 +29,6 @@ export async function join( deviceHostnameOrIp?: string, appName?: string, ): Promise { - logger.logDebug('Checking login...'); - const isLoggedIn = await sdk.auth.isLoggedIn(); - if (!isLoggedIn) { - logger.logInfo("Looks like you're not logged in yet!"); - logger.logInfo("Let's go through a quick wizard to get you started.\n"); - await runCommand('login'); - } - logger.logDebug('Determining device...'); const deviceIp = await getOrSelectLocalDevice(deviceHostnameOrIp); await assertDeviceIsCompatible(deviceIp); @@ -117,7 +109,7 @@ async function configure(deviceIp: string, config: any): Promise { const json = JSON.stringify(config); const b64 = Buffer.from(json).toString('base64'); const str = `"$(base64 -d <<< ${b64})"`; - await execCommand(deviceIp, `os-config join '${str}'`, 'Configuring...'); + await execCommand(deviceIp, `os-config join ${str}`, 'Configuring...'); } async function deconfigure(deviceIp: string): Promise { @@ -125,18 +117,24 @@ async function deconfigure(deviceIp: string): Promise { } async function assertDeviceIsCompatible(deviceIp: string): Promise { - const { exitWithExpectedError } = await import('../utils/patterns'); + const cmd = 'os-config --version'; try { - await execBuffered(deviceIp, 'os-config --version'); + await execBuffered(deviceIp, cmd); } catch (err) { - exitWithExpectedError(stripIndent` - Device "${deviceIp}" is incompatible and cannot join or leave an application. - Please select or provision device with balenaOS newer than ${MIN_BALENAOS_VERSION}.`); + if (err instanceof ExpectedError) { + throw err; + } + console.error(`${err}\n`); + throw new ExpectedError(stripIndent` + Failed to execute "${cmd}" on device "${deviceIp}". + Depending on more specific error messages above, this may mean that the device + is incompatible. Please ensure that the device is running a balenaOS release + newer than ${MIN_BALENAOS_VERSION}.`); } } async function getDeviceType(deviceIp: string): Promise { - const output = await execBuffered(deviceIp, 'cat /etc/os-release'); + const output = await getDeviceOsRelease(deviceIp); const match = /^SLUG="([^"]+)"$/m.exec(output); if (!match) { throw new Error('Failed to determine device type'); @@ -145,7 +143,7 @@ async function getDeviceType(deviceIp: string): Promise { } async function getOsVersion(deviceIp: string): Promise { - const output = await execBuffered(deviceIp, 'cat /etc/os-release'); + const output = await getDeviceOsRelease(deviceIp); const match = /^VERSION_ID="([^"]+)"$/m.exec(output); if (!match) { throw new Error('Failed to determine OS version ID'); diff --git a/lib/utils/ssh.ts b/lib/utils/ssh.ts index 36e86863..aef31f9b 100644 --- a/lib/utils/ssh.ts +++ b/lib/utils/ssh.ts @@ -16,10 +16,9 @@ */ import * as Bluebird from 'bluebird'; import { spawn, StdioOptions } from 'child_process'; +import * as _ from 'lodash'; import { TypedError } from 'typed-error'; -import { getSubShellCommand } from './helpers'; - export class ExecError extends TypedError { public cmd: string; public exitCode: number; @@ -36,17 +35,41 @@ export async function exec( cmd: string, stdout?: NodeJS.WritableStream, ): Promise { - const command = `ssh \ - -t \ - -p 22222 \ - -o LogLevel=ERROR \ - -o StrictHostKeyChecking=no \ - -o UserKnownHostsFile=/dev/null \ - root@${deviceIp} \ - ${cmd}`; + const { which } = await import('./helpers'); + const program = await which('ssh'); + const args = [ + '-n', + '-t', + '-p', + '22222', + '-o', + 'LogLevel=ERROR', + '-o', + 'StrictHostKeyChecking=no', + '-o', + 'UserKnownHostsFile=/dev/null', + `root@${deviceIp}`, + cmd, + ]; + if (process.env.DEBUG) { + const logger = (await import('./logger')).getLogger(); + logger.logDebug(`Executing [${program},${args}]`); + } - const stdio: StdioOptions = ['ignore', stdout ? 'pipe' : 'inherit', 'ignore']; - const { program, args } = getSubShellCommand(command); + // Note: stdin must be 'inherit' to workaround a bug in older versions of + // the built-in Windows 10 ssh client that otherwise prints the following + // to stderr and hangs: "GetConsoleMode on STD_INPUT_HANDLE failed with 6" + // They fixed the bug in newer versions of the ssh client: + // https://github.com/PowerShell/Win32-OpenSSH/issues/856 + // but users whould have to manually download and install a new client. + // Note that "ssh -n" does not solve the problem, but should in theory + // prevent the ssh client from using the CLI process stdin, even if it + // is connected with 'inherit'. + const stdio: StdioOptions = [ + 'inherit', + stdout ? 'pipe' : 'inherit', + 'inherit', + ]; const exitCode = await new Bluebird((resolve, reject) => { const ps = spawn(program, args, { stdio }) @@ -79,3 +102,12 @@ export async function execBuffered( ); return buffer.join(''); } + +/** + * Return a device's balenaOS release by executing 'cat /etc/os-release' + * over ssh to the given deviceIp address. The result is cached with + * lodash's memoize. + */ +export const getDeviceOsRelease = _.memoize(async (deviceIp: string) => + execBuffered(deviceIp, 'cat /etc/os-release'), +); diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index a8bdaba0..39fb22f3 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -825,6 +825,12 @@ "@types/node": "*" } }, + "@types/which": { + "version": "1.3.2", + "resolved": "https://registry.npmjs.org/@types/which/-/which-1.3.2.tgz", + "integrity": "sha512-8oDqyLC7eD4HM307boe2QWKyuzdzWBj56xI/imSl2cpL+U3tCMaTAkMJ4ee5JBZ/FsOJlvRGeIShiZDAl1qERA==", + "dev": true + }, "@yarnpkg/lockfile": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/@yarnpkg/lockfile/-/lockfile-1.1.0.tgz", @@ -3462,6 +3468,16 @@ "lru-cache": "^4.0.1", "shebang-command": "^1.2.0", "which": "^1.2.9" + }, + "dependencies": { + "which": { + "version": "1.3.1", + "resolved": "https://registry.npmjs.org/which/-/which-1.3.1.tgz", + "integrity": "sha512-HxJdYWq1MTIQbJ3nw0cqssHoTNU267KlrDuGZ1WYlxDStUtKUhOaJmh112/TZmHxxUfuJqPXSOm7tDyas0OSIQ==", + "requires": { + "isexe": "^2.0.0" + } + } } } } @@ -3476,6 +3492,16 @@ "semver": "^5.5.0", "shebang-command": "^1.2.0", "which": "^1.2.9" + }, + "dependencies": { + "which": { + "version": "1.3.1", + "resolved": "https://registry.npmjs.org/which/-/which-1.3.1.tgz", + "integrity": "sha512-HxJdYWq1MTIQbJ3nw0cqssHoTNU267KlrDuGZ1WYlxDStUtKUhOaJmh112/TZmHxxUfuJqPXSOm7tDyas0OSIQ==", + "requires": { + "isexe": "^2.0.0" + } + } } }, "crypt": { @@ -3734,6 +3760,17 @@ "lru-cache": "^4.0.1", "shebang-command": "^1.2.0", "which": "^1.2.9" + }, + "dependencies": { + "which": { + "version": "1.3.1", + "resolved": "https://registry.npmjs.org/which/-/which-1.3.1.tgz", + "integrity": "sha512-HxJdYWq1MTIQbJ3nw0cqssHoTNU267KlrDuGZ1WYlxDStUtKUhOaJmh112/TZmHxxUfuJqPXSOm7tDyas0OSIQ==", + "dev": true, + "requires": { + "isexe": "^2.0.0" + } + } } }, "execa": { @@ -6391,6 +6428,17 @@ "ini": "^1.3.4", "is-windows": "^1.0.1", "which": "^1.2.14" + }, + "dependencies": { + "which": { + "version": "1.3.1", + "resolved": "https://registry.npmjs.org/which/-/which-1.3.1.tgz", + "integrity": "sha512-HxJdYWq1MTIQbJ3nw0cqssHoTNU267KlrDuGZ1WYlxDStUtKUhOaJmh112/TZmHxxUfuJqPXSOm7tDyas0OSIQ==", + "dev": true, + "requires": { + "isexe": "^2.0.0" + } + } } }, "global-tunnel-ng": { @@ -8942,6 +8990,15 @@ "has-flag": "^3.0.0" } }, + "which": { + "version": "1.3.1", + "resolved": "https://registry.npmjs.org/which/-/which-1.3.1.tgz", + "integrity": "sha512-HxJdYWq1MTIQbJ3nw0cqssHoTNU267KlrDuGZ1WYlxDStUtKUhOaJmh112/TZmHxxUfuJqPXSOm7tDyas0OSIQ==", + "dev": true, + "requires": { + "isexe": "^2.0.0" + } + }, "which-module": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/which-module/-/which-module-2.0.0.tgz", @@ -16829,6 +16886,16 @@ "lru-cache": "^4.0.1", "shebang-command": "^1.2.0", "which": "^1.2.9" + }, + "dependencies": { + "which": { + "version": "1.3.1", + "resolved": "https://registry.npmjs.org/which/-/which-1.3.1.tgz", + "integrity": "sha512-HxJdYWq1MTIQbJ3nw0cqssHoTNU267KlrDuGZ1WYlxDStUtKUhOaJmh112/TZmHxxUfuJqPXSOm7tDyas0OSIQ==", + "requires": { + "isexe": "^2.0.0" + } + } } }, "execa": { @@ -17860,9 +17927,9 @@ "dev": true }, "which": { - "version": "1.3.1", - "resolved": "https://registry.npmjs.org/which/-/which-1.3.1.tgz", - "integrity": "sha512-HxJdYWq1MTIQbJ3nw0cqssHoTNU267KlrDuGZ1WYlxDStUtKUhOaJmh112/TZmHxxUfuJqPXSOm7tDyas0OSIQ==", + "version": "2.0.2", + "resolved": "https://registry.npmjs.org/which/-/which-2.0.2.tgz", + "integrity": "sha512-BLI3Tl1TW3Pvl70l3yq3Y64i+awpwXqsGBYWkkqMtnbXgrMD+yj7rhW0kuEDxzJaYXGjEW5ogapKNMEKNMjibA==", "requires": { "isexe": "^2.0.0" } diff --git a/package.json b/package.json index 703b7ba0..ded157c6 100644 --- a/package.json +++ b/package.json @@ -123,6 +123,7 @@ "@types/stream-to-promise": "2.2.0", "@types/tar-stream": "1.6.0", "@types/through2": "2.0.33", + "@types/which": "^1.3.2", "catch-uncommitted": "^1.3.0", "chai": "^4.2.0", "chai-as-promised": "^7.1.1", @@ -236,6 +237,7 @@ "umount": "^1.1.6", "unzip2": "balena-io-library/node-unzip-2#v0.2.8", "update-notifier": "^2.2.0", + "which": "^2.0.2", "window-size": "^1.1.0" }, "optionalDependencies": {