Fix 'balena ssh' on MSYS Windows shell ("unexpected end of file")

Resolves: #1681
Change-type: patch

doc/cli.markdown

@@ -1205,7 +1205,8 @@ support) please check:
 
 #### --port, -p <port>
 
-SSH gateway port
+SSH server port number (default 22222) if the target is an IP address or .local
+hostname. Otherwise, port number for the balenaCloud gateway (default 22).
 
 #### --verbose, -v
 
@@ -1213,8 +1214,7 @@ Increase verbosity
 
 #### --noproxy
 
-Don't use the proxy configuration for this connection. This flag
+Bypass global proxy configuration for the ssh connection
-only make sense if you've configured a proxy globally.
 
 ## tunnel <deviceOrApplication>
 

lib/actions/local/common.coffee

@@ -58,5 +58,3 @@ exports.pipeContainerStream = Promise.method ({ deviceIp, name, outStream, follo
 		if err is '404'
 			return console.log(getChalk().red.bold("Container '#{name}' not found."))
 		throw err
-
-exports.getSubShellCommand = require('../../utils/helpers').getSubShellCommand

lib/actions/ssh.ts

@@ -1,5 +1,5 @@
 /*
-Copyright 2016-2019 Balena
+Copyright 2016-2020 Balena
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -17,7 +17,6 @@ import * as BalenaSdk from 'balena-sdk';
 import { CommandDefinition } from 'capitano';
 import { stripIndent } from 'common-tags';
 
-import { BalenaDeviceNotFound } from 'balena-errors';
 import { getBalenaSdk } from '../utils/lazy';
 import { validateDotLocalUrl, validateIPAddress } from '../utils/validation';
 
@@ -27,7 +26,7 @@ async function getContainerId(
 	serviceName: string,
 	sshOpts: {
 		port?: number;
-		proxyCommand?: string;
+		proxyCommand?: string[];
 		proxyUrl: string;
 		username: string;
 	},
@@ -70,7 +69,7 @@ async function getContainerId(
 		}
 		containerId = body.services[serviceName];
 	} else {
-		console.log(stripIndent`
+		console.error(stripIndent`
 			Using legacy method to detect container ID. This will be slow.
 			To speed up this process, please update your device to an OS
 			which has a supervisor version of at least v8.6.0.
@@ -80,30 +79,29 @@ async function getContainerId(
 		// container
 		const { child_process } = await import('mz');
 		const escapeRegex = await import('lodash/escapeRegExp');
-		const { getSubShellCommand } = await import('../utils/helpers');
+		const { which } = await import('../utils/helpers');
 		const { deviceContainerEngineBinary } = await import('../utils/device/ssh');
 
-		const command = generateVpnSshCommand({
+		const sshBinary = await which('ssh');
+		const sshArgs = generateVpnSshCommand({
 			uuid,
 			verbose: false,
 			port: sshOpts.port,
-			command: `host ${uuid} '"${deviceContainerEngineBinary}" ps --format "{{.ID}} {{.Names}}"'`,
+			command: `host ${uuid} "${deviceContainerEngineBinary}" ps --format "{{.ID}} {{.Names}}"`,
 			proxyCommand: sshOpts.proxyCommand,
 			proxyUrl: sshOpts.proxyUrl,
 			username: sshOpts.username,
 		});
 
-		const subShellCommand = getSubShellCommand(command);
+		if (process.env.DEBUG) {
-		const subprocess = child_process.spawn(
+			console.error(`[debug] [${sshBinary}, ${sshArgs.join(', ')}]`);
-			subShellCommand.program,
+		}
-			subShellCommand.args,
+		const subprocess = child_process.spawn(sshBinary, sshArgs, {
-			{
+			stdio: [null, 'pipe', null],
-				stdio: [null, 'pipe', null],
+		});
-			},
-		);
 		const containers = await new Promise<string>((resolve, reject) => {
-			let output = '';
+			const output: string[] = [];
-			subprocess.stdout.on('data', chunk => (output += chunk.toString()));
+			subprocess.stdout.on('data', chunk => output.push(chunk.toString()));
 			subprocess.on('close', (code: number) => {
 				if (code !== 0) {
 					reject(
@@ -112,7 +110,7 @@ async function getContainerId(
 						),
 					);
 				} else {
-					resolve(output);
+					resolve(output.join(''));
 				}
 			});
 		});
@@ -143,17 +141,21 @@ function generateVpnSshCommand(opts: {
 	port?: number;
 	username: string;
 	proxyUrl: string;
-	proxyCommand?: string;
+	proxyCommand?: string[];
 }) {
-	return (
+	return [
-		`ssh ${
+		...(opts.verbose ? ['-vvv'] : []),
-			opts.verbose ? '-vvv' : ''
+		'-t',
-		} -t -o LogLevel=ERROR -o StrictHostKeyChecking=no ` +
+		...['-o', 'LogLevel=ERROR'],
-		`-o UserKnownHostsFile=/dev/null ` +
+		...['-o', 'StrictHostKeyChecking=no'],
-		`${opts.proxyCommand != null ? opts.proxyCommand : ''} ` +
+		...['-o', 'UserKnownHostsFile=/dev/null'],
-		`${opts.port != null ? `-p ${opts.port}` : ''} ` +
+		...(opts.proxyCommand && opts.proxyCommand.length
-		`${opts.username}@ssh.${opts.proxyUrl} ${opts.command}`
+			? ['-o', `ProxyCommand=${opts.proxyCommand.join(' ')}`]
-	);
+			: []),
+		...(opts.port ? ['-p', opts.port.toString()] : []),
+		`${opts.username}@ssh.${opts.proxyUrl}`,
+		opts.command,
+	];
 }
 
 export const ssh: CommandDefinition<
@@ -207,7 +209,9 @@ export const ssh: CommandDefinition<
 		{
 			signature: 'port',
 			parameter: 'port',
-			description: 'SSH gateway port',
+			description: stripIndent`
+				SSH server port number (default 22222) if the target is an IP address or .local
+				hostname. Otherwise, port number for the balenaCloud gateway (default 22).`,
 			alias: 'p',
 		},
 		{
@@ -219,24 +223,19 @@ export const ssh: CommandDefinition<
 		{
 			signature: 'noproxy',
 			boolean: true,
-			description: stripIndent`
+			description: 'Bypass global proxy configuration for the ssh connection',
-				Don't use the proxy configuration for this connection. This flag
-				only make sense if you've configured a proxy globally.`,
 		},
 	],
 	action: async (params, options) => {
 		const applicationOrDevice =
 			params.applicationOrDevice_raw || params.applicationOrDevice;
-		const bash = await import('bash');
+		const { ExpectedError } = await import('../errors');
-		const { getProxyConfig, getSubShellCommand, which } = await import(
+		const { getProxyConfig, which, whichSpawn } = await import(
 			'../utils/helpers'
 		);
-		const { child_process } = await import('mz');
+		const { checkLoggedIn, getOnlineTargetUuid } = await import(
-		const {
+			'../utils/patterns'
-			exitIfNotLoggedIn,
+		);
-			exitWithExpectedError,
-			getOnlineTargetUuid,
-		} = await import('../utils/patterns');
 		const sdk = getBalenaSdk();
 
 		const verbose = options.verbose === true;
@@ -259,34 +258,30 @@ export const ssh: CommandDefinition<
 		}
 
 		// this will be a tunnelled SSH connection...
-		await exitIfNotLoggedIn();
+		await checkLoggedIn();
 		const uuid = await getOnlineTargetUuid(sdk, applicationOrDevice);
 		let version: string | undefined;
 		let id: number | undefined;
 
-		try {
+		const device = await sdk.models.device.get(uuid, {
-			const device = await sdk.models.device.get(uuid, {
+			$select: ['id', 'supervisor_version', 'is_online'],
-				$select: ['id', 'supervisor_version', 'is_online'],
+		});
-			});
+		id = device.id;
-			id = device.id;
+		version = device.supervisor_version;
-			version = device.supervisor_version;
-		} catch (e) {
-			if (e instanceof BalenaDeviceNotFound) {
-				exitWithExpectedError(`Could not find device: ${uuid}`);
-			}
-		}
 
 		const [whichProxytunnel, username, proxyUrl] = await Promise.all([
 			useProxy ? which('proxytunnel', false) : undefined,
 			sdk.auth.whoami(),
+			// note that `proxyUrl` refers to the balenaCloud "resin-proxy"
+			// service, currently "balena-devices.com", rather than some
+			// local proxy server URL
 			sdk.settings.get('proxyUrl'),
 		]);
 
 		const getSshProxyCommand = () => {
-			if (!useProxy) {
+			if (!proxyConfig) {
-				return '';
+				return;
 			}
-
 			if (!whichProxytunnel) {
 				console.warn(stripIndent`
 					Proxy is enabled but the \`proxytunnel\` binary cannot be found.
@@ -295,27 +290,32 @@ export const ssh: CommandDefinition<
 					for the \`ssh\` requests.
 
 					Attempting the unproxied request for now.`);
-				return '';
+				return;
 			}
 
-			const p = proxyConfig!;
+			const p = proxyConfig;
-			const tunnelOptions: Dictionary<string> = {
-				proxy: `${p.host}:${p.port}`,
-				dest: '%h:%p',
-			};
 			if (p.username && p.password) {
-				tunnelOptions.user = p.username;
+				// proxytunnel understands these variables for proxy authentication.
-				tunnelOptions.pass = p.password;
+				// Setting the variables instead of command-line options avoids the
+				// need for shell-specific escaping of special characters like '$'.
+				process.env.PROXYUSER = p.username;
+				process.env.PROXYPASS = p.password;
 			}
 
-			const ProxyCommand = `proxytunnel ${bash.args(tunnelOptions, '--', '=')}`;
+			return [
-			return `-o ${bash.args({ ProxyCommand }, '', '=')}`;
+				'proxytunnel',
+				`--proxy=${p.host}:${p.port}`,
+				// ssh replaces these %h:%p variables in the ProxyCommand option
+				// https://linux.die.net/man/5/ssh_config
+				'--dest=%h:%p',
+				...(verbose ? ['--verbose'] : []),
+			];
 		};
 
-		const proxyCommand = getSshProxyCommand();
+		const proxyCommand = useProxy ? getSshProxyCommand() : undefined;
 
 		if (username == null) {
-			exitWithExpectedError(
+			throw new ExpectedError(
 				`Opening an SSH connection to a remote device requires you to be logged in.`,
 			);
 		}
@@ -356,9 +356,6 @@ export const ssh: CommandDefinition<
 			username: username!,
 		});
 
-		const subShellCommand = getSubShellCommand(command);
+		await whichSpawn('ssh', command);
-		await child_process.spawn(subShellCommand.program, subShellCommand.args, {
-			stdio: 'inherit',
-		});
 	},
 };

lib/utils/device/ssh.ts

@@ -1,5 +1,5 @@
 /*
-Copyright 2016-2019 Balena
+Copyright 2016-2020 Balena
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -27,12 +27,10 @@ export const deviceContainerEngineBinary = `$(if [ -f /usr/bin/balena ]; then ec
 export async function performLocalDeviceSSH(
 	opts: DeviceSSHOpts,
 ): Promise<void> {
-	const childProcess = await import('child_process');
 	const reduce = await import('lodash/reduce');
-	const { getSubShellCommand } = await import('../helpers');
+	const { whichSpawn } = await import('../helpers');
-	const { exitWithExpectedError } = await import('../patterns');
+	const { ExpectedError } = await import('../../errors');
 	const { stripIndent } = await import('common-tags');
-	const os = await import('os');
 
 	let command = '';
 
@@ -57,10 +55,9 @@ export async function performLocalDeviceSSH(
 		try {
 			allContainers = await docker.listContainers();
 		} catch (_e) {
-			exitWithExpectedError(stripIndent`
+			throw new ExpectedError(stripIndent`
 				Could not access docker daemon on device ${opts.address}.
 				Please ensure the device is in local mode.`);
-			return;
 		}
 
 		const serviceNames: string[] = [];
@@ -80,7 +77,7 @@ export async function performLocalDeviceSSH(
 			.filter(c => c != null);
 
 		if (containers.length === 0) {
-			exitWithExpectedError(
+			throw new ExpectedError(
 				`Could not find a service on device with name ${opts.service}. ${
 					serviceNames.length > 0
 						? `Available services:\n${reduce(
@@ -93,36 +90,26 @@ export async function performLocalDeviceSSH(
 			);
 		}
 		if (containers.length > 1) {
-			exitWithExpectedError(stripIndent`
+			throw new ExpectedError(stripIndent`
 				Found more than one container with a service name ${opts.service}.
 				This state is not supported, please contact support.
 			`);
 		}
 
-		// Getting a command to work on all platforms is a pain,
+		const shellCmd = `/bin/sh -c "if [ -e /bin/bash ]; then exec /bin/bash; else exec /bin/sh; fi"`;
-		// so we just define slightly different ones for windows
+		command = `${deviceContainerEngineBinary} exec -ti ${
-		if (os.platform() !== 'win32') {
+			containers[0]!.id
-			const shellCmd = `/bin/sh -c "if [ -e /bin/bash ]; then exec /bin/bash; else exec /bin/sh; fi"`;
+		} ${shellCmd}`;
-			command = `'${deviceContainerEngineBinary}' exec -ti ${
-				containers[0]!.id
-			} '${shellCmd}'`;
-		} else {
-			const shellCmd = `/bin/sh -c "if [ -e /bin/bash ]; then exec /bin/bash; else exec /bin/sh; fi"`;
-			command = `${deviceContainerEngineBinary} exec -ti ${
-				containers[0]!.id
-			} ${shellCmd}`;
-		}
 	}
-	// Generate the SSH command
-	const sshCommand = `ssh \
-	${opts.verbose ? '-vvv' : ''} \
-	-t \
-	-p ${opts.port ? opts.port : 22222} \
-	-o LogLevel=ERROR \
-	-o StrictHostKeyChecking=no \
-	-o UserKnownHostsFile=/dev/null \
-	root@${opts.address} ${command}`;
 
-	const subShell = getSubShellCommand(sshCommand);
+	return whichSpawn('ssh', [
-	childProcess.spawn(subShell.program, subShell.args, { stdio: 'inherit' });
+		...(opts.verbose ? ['-vvv'] : []),
+		'-t',
+		...['-p', opts.port ? opts.port.toString() : '22222'],
+		...['-o', 'LogLevel=ERROR'],
+		...['-o', 'StrictHostKeyChecking=no'],
+		...['-o', 'UserKnownHostsFile=/dev/null'],
+		`root@${opts.address}`,
+		...(command ? [command] : []),
+	]);
 }

lib/utils/helpers.ts

@@ -16,9 +16,10 @@ limitations under the License.
 
 import { InitializeEmitter, OperationState } from 'balena-device-init';
 import * as BalenaSdk from 'balena-sdk';
-import Bluebird = require('bluebird');
+import * as Bluebird from 'bluebird';
-import _ = require('lodash');
+import { spawn, SpawnOptions } from 'child_process';
-import os = require('os');
+import * as _ from 'lodash';
+import * as os from 'os';
 import * as ShellEscape from 'shell-escape';
 
 import { ExpectedError } from '../errors';
@@ -187,35 +188,6 @@ export function getApplication(applicationName: string) {
 	return balena.models.application.get(applicationName, extraOptions);
 }
 
-/**
- * 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 {
-			program: 'cmd.exe',
-			args: ['/s', '/c', command],
-		};
-	} else {
-		return {
-			program: '/bin/sh',
-			args: ['-c', command],
-		};
-	}
-}
-
 /**
  * Call `func`, and if func() throws an error or returns a promise that
  * eventually rejects, retry it `times` many times, each time printing a
@@ -406,6 +378,40 @@ export async function which(
 	return programPath;
 }
 
+/**
+ * Call which(programName) and spawn() with the given arguments.
+ * Reject the promise if the process exit code is not zero.
+ */
+export async function whichSpawn(
+	programName: string,
+	args: string[],
+	options: SpawnOptions = { stdio: 'inherit' },
+): Promise<void> {
+	const program = await which(programName);
+	if (process.env.DEBUG) {
+		console.error(`[debug] [${program}, ${args.join(', ')}]`);
+	}
+	let error: Error | undefined;
+	let exitCode: number | undefined;
+	try {
+		exitCode = await new Promise<number>((resolve, reject) => {
+			spawn(program, args, options)
+				.on('error', reject)
+				.on('close', resolve);
+		});
+	} catch (err) {
+		error = err;
+	}
+	if (error || exitCode) {
+		const msg = [
+			`${programName} failed with exit code ${exitCode}:`,
+			`[${program}, ${args.join(', ')}]`,
+			...(error ? [`${error}`] : []),
+		];
+		throw new Error(msg.join('\n'));
+	}
+}
+
 export interface ProxyConfig {
 	host: string;
 	port: string;

npm-shrinkwrap.json

@@ -2431,11 +2431,6 @@
       "resolved": "https://registry.npmjs.org/base64-js/-/base64-js-1.3.0.tgz",
       "integrity": "sha512-ccav/yGvoa80BQDljCxsmmQ3Xvx60/UpBIij5QN21W3wBi/hhIC9OoO+KLpu9IJTS9j4DRVJ3aDDF9cMSoa2lw=="
     },
-    "bash": {
-      "version": "0.0.1",
-      "resolved": "https://registry.npmjs.org/bash/-/bash-0.0.1.tgz",
-      "integrity": "sha1-nuDnp1K8Xu8Wi8SHGVVqdFSKrgw="
-    },
     "bcrypt-pbkdf": {
       "version": "1.0.2",
       "resolved": "https://registry.npmjs.org/bcrypt-pbkdf/-/bcrypt-pbkdf-1.0.2.tgz",

package.json

@@ -177,7 +177,6 @@
     "balena-semver": "^2.2.0",
     "balena-settings-client": "^4.0.4",
     "balena-sync": "^10.2.0",
-    "bash": "0.0.1",
     "bluebird": "^3.7.2",
     "body-parser": "^1.19.0",
     "capitano": "^1.9.2",

typings/bash/index.d.ts

@@ -1 +0,0 @@
-declare module 'bash';