ssh: Attempt cloud username if 'root' authentication fails

Also refactor several files to avoid code duplication.

Change-type: minor

lib/commands/ssh.ts

@@ -230,19 +230,15 @@ export default class SshCmd extends Command {
 		} else {
 			accessCommand = `host ${deviceUuid}`;
 		}
-
+		const { runRemoteCommand } = await import('../utils/ssh');
-		const command = this.generateVpnSshCommand({
+		await runRemoteCommand({
-			uuid: deviceUuid,
+			cmd: accessCommand,
-			command: accessCommand,
+			hostname: `ssh.${proxyUrl}`,
-			verbose: options.verbose,
 			port: options.port,
 			proxyCommand,
-			proxyUrl: proxyUrl || '',
+			username,
-			username: username!,
+			verbose: options.verbose,
 		});
-
-		const { spawnSshAndThrowOnError } = await import('../utils/ssh');
-		return spawnSshAndThrowOnError(command);
 	}
 
 	async getContainerId(
@@ -295,53 +291,28 @@ export default class SshCmd extends Command {
 			containerId = body.services[serviceName];
 		} else {
 			console.error(stripIndent`
-			Using legacy method to detect container ID. This will be slow.
+				Using slow legacy method to determine container IDs. To speed up
-			To speed up this process, please update your device to an OS
+				this process, update the device supervisor to v8.6.0 or later.
-			which has a supervisor version of at least v8.6.0.
+			`);
-		`);
 			// We need to execute a balena ps command on the device,
 			// and parse the output, looking for a specific
 			// container
-			const childProcess = await import('child_process');
 			const { escapeRegExp } = await import('lodash');
-			const { which } = await import('../utils/which');
 			const { deviceContainerEngineBinary } = await import(
 				'../utils/device/ssh'
 			);
+			const { getRemoteCommandOutput } = await import('../utils/ssh');
 
-			const sshBinary = await which('ssh');
+			const containers: string = (
-			const sshArgs = this.generateVpnSshCommand({
+				await getRemoteCommandOutput({
-				uuid,
+					cmd: `host ${uuid} "${deviceContainerEngineBinary}" ps --format "{{.ID}} {{.Names}}"`,
-				verbose: false,
+					hostname: `ssh.${sshOpts.proxyUrl}`,
-				port: sshOpts.port,
+					port: sshOpts.port,
-				command: `host ${uuid} "${deviceContainerEngineBinary}" ps --format "{{.ID}} {{.Names}}"`,
+					proxyCommand: sshOpts.proxyCommand,
-				proxyCommand: sshOpts.proxyCommand,
+					stderr: 'inherit',
-				proxyUrl: sshOpts.proxyUrl,
+					username: sshOpts.username,
-				username: sshOpts.username,
+				})
-			});
+			).stdout.toString();
-
-			if (process.env.DEBUG) {
-				console.error(`[debug] [${sshBinary}, ${sshArgs.join(', ')}]`);
-			}
-			const subProcess = childProcess.spawn(sshBinary, sshArgs, {
-				stdio: [null, 'pipe', null],
-			});
-			const containers = await new Promise<string>((resolve, reject) => {
-				const output: string[] = [];
-				subProcess.stdout.on('data', (chunk) => output.push(chunk.toString()));
-				subProcess.on('close', (code: number) => {
-					if (code !== 0) {
-						reject(
-							new Error(
-								`Non-zero error code when looking for service container: ${code}`,
-							),
-						);
-					} else {
-						resolve(output.join(''));
-					}
-				});
-			});
-
 			const lines = containers.split('\n');
 			const regex = new RegExp(`\\/?${escapeRegExp(serviceName)}_\\d+_\\d+`);
 			for (const container of lines) {
@@ -360,28 +331,4 @@ export default class SshCmd extends Command {
 		}
 		return containerId;
 	}
-
-	generateVpnSshCommand(opts: {
-		uuid: string;
-		command: string;
-		verbose: boolean;
-		port?: number;
-		username: string;
-		proxyUrl: string;
-		proxyCommand?: string[];
-	}) {
-		return [
-			...(opts.verbose ? ['-vvv'] : []),
-			'-t',
-			...['-o', 'LogLevel=ERROR'],
-			...['-o', 'StrictHostKeyChecking=no'],
-			...['-o', 'UserKnownHostsFile=/dev/null'],
-			...(opts.proxyCommand && opts.proxyCommand.length
-				? ['-o', `ProxyCommand=${opts.proxyCommand.join(' ')}`]
-				: []),
-			...(opts.port ? ['-p', opts.port.toString()] : []),
-			`${opts.username}@ssh.${opts.proxyUrl}`,
-			opts.command,
-		];
-	}
 }

lib/utils/device/ssh.ts

@@ -14,6 +14,8 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 import type { ContainerInfo } from 'dockerode';
+
+import { ExpectedError } from '../../errors';
 import { stripIndent } from '../lazy';
 
 export interface DeviceSSHOpts {
@@ -26,76 +28,80 @@ export interface DeviceSSHOpts {
 
 export const deviceContainerEngineBinary = `$(if [ -f /usr/bin/balena ]; then echo "balena"; else echo "docker"; fi)`;
 
+/**
+ * List the running containers on the device with dockerode, and return the
+ * container ID that matches the given service name.
+ */
+async function getContainerIdForService(
+	service: string,
+	deviceAddress: string,
+): Promise<string> {
+	const { escapeRegExp, reduce } = await import('lodash');
+	const Docker = await import('dockerode');
+	const docker = new Docker({
+		host: deviceAddress,
+		port: 2375,
+	});
+	const regex = new RegExp(`(^|\\/)${escapeRegExp(service)}_\\d+_\\d+`);
+	const nameRegex = /\/?([a-zA-Z0-9_-]+)_\d+_\d+/;
+	let allContainers: ContainerInfo[];
+	try {
+		allContainers = await docker.listContainers();
+	} catch (_e) {
+		throw new ExpectedError(stripIndent`
+			Could not access docker daemon on device ${deviceAddress}.
+			Please ensure the device is in local mode.`);
+	}
+
+	const serviceNames: string[] = [];
+	const containers: Array<{ id: string; name: string }> = [];
+	for (const container of allContainers) {
+		for (const name of container.Names) {
+			if (regex.test(name)) {
+				containers.push({ id: container.Id, name });
+				break;
+			}
+			const match = name.match(nameRegex);
+			if (match) {
+				serviceNames.push(match[1]);
+			}
+		}
+	}
+	if (containers.length > 1) {
+		throw new ExpectedError(stripIndent`
+			Found more than one container matching service name "${service}":
+			${containers.map((container) => container.name).join(', ')}
+			Use different service names to avoid ambiguity.
+		`);
+	}
+	const containerId = containers.length ? containers[0].id : '';
+	if (!containerId) {
+		throw new ExpectedError(
+			`Could not find a service on device with name ${service}. ${
+				serviceNames.length > 0
+					? `Available services:\n${reduce(
+							serviceNames,
+							(str, name) => `${str}\t${name}\n`,
+							'',
+					  )}`
+					: ''
+			}`,
+		);
+	}
+	return containerId;
+}
+
 export async function performLocalDeviceSSH(
 	opts: DeviceSSHOpts,
 ): Promise<void> {
-	const { escapeRegExp, reduce } = await import('lodash');
+	let cmd = '';
-	const { spawnSshAndThrowOnError } = await import('../ssh');
-	const { ExpectedError } = await import('../../errors');
 
-	let command = '';
+	if (opts.service) {
+		const containerId = await getContainerIdForService(
+			opts.service,
+			opts.address,
+		);
 
-	if (opts.service != null) {
-		// Get the containers which are on-device. Currently we
-		// are single application, which means we can assume any
-		// container which fulfills the form of
-		// $serviceName_$appId_$releaseId is what we want. Once
-		// we have multi-app, we should show a dialog which
-		// allows the user to choose the correct container
-
-		const Docker = await import('dockerode');
-		const docker = new Docker({
-			host: opts.address,
-			port: 2375,
-		});
-
-		const regex = new RegExp(`(^|\\/)${escapeRegExp(opts.service)}_\\d+_\\d+`);
-		const nameRegex = /\/?([a-zA-Z0-9_-]+)_\d+_\d+/;
-		let allContainers: ContainerInfo[];
-		try {
-			allContainers = await docker.listContainers();
-		} catch (_e) {
-			throw new ExpectedError(stripIndent`
-				Could not access docker daemon on device ${opts.address}.
-				Please ensure the device is in local mode.`);
-		}
-
-		const serviceNames: string[] = [];
-		const containers: Array<{ id: string; name: string }> = [];
-		for (const container of allContainers) {
-			for (const name of container.Names) {
-				if (regex.test(name)) {
-					containers.push({ id: container.Id, name });
-					break;
-				}
-				const match = name.match(nameRegex);
-				if (match) {
-					serviceNames.push(match[1]);
-				}
-			}
-		}
-		if (containers.length === 0) {
-			throw new ExpectedError(
-				`Could not find a service on device with name ${opts.service}. ${
-					serviceNames.length > 0
-						? `Available services:\n${reduce(
-								serviceNames,
-								(str, name) => `${str}\t${name}\n`,
-								'',
-						  )}`
-						: ''
-				}`,
-			);
-		}
-		if (containers.length > 1) {
-			throw new ExpectedError(stripIndent`
-				Found more than one container matching service name "${opts.service}":
-				${containers.map((container) => container.name).join(', ')}
-				Use different service names to avoid ambiguity.
-			`);
-		}
-
-		const containerId = containers[0].id;
 		const shellCmd = `/bin/sh -c "if [ -e /bin/bash ]; then exec /bin/bash; else exec /bin/sh; fi"`;
 		// stdin (fd=0) is not a tty when data is piped in, for example
 		// echo 'ls -la; exit;' | balena ssh 192.168.0.20 service1
@@ -103,17 +109,32 @@ export async function performLocalDeviceSSH(
 		//     https://assets.balena.io/newsletter/2020-01/pipe.png
 		const isTTY = !!opts.forceTTY || (await import('tty')).isatty(0);
 		const ttyFlag = isTTY ? '-t' : '';
-		command = `${deviceContainerEngineBinary} exec -i ${ttyFlag} ${containerId} ${shellCmd}`;
+		cmd = `${deviceContainerEngineBinary} exec -i ${ttyFlag} ${containerId} ${shellCmd}`;
 	}
 
-	return spawnSshAndThrowOnError([
+	const { findBestUsernameForDevice, runRemoteCommand } = await import(
-		...(opts.verbose ? ['-vvv'] : []),
+		'../ssh'
-		'-t',
+	);
-		...['-p', opts.port ? opts.port.toString() : '22222'],
+
-		...['-o', 'LogLevel=ERROR'],
+	// Before we started using `findBestUsernameForDevice`, we tried the approach
-		...['-o', 'StrictHostKeyChecking=no'],
+	// of attempting ssh with the 'root' username first and, if that failed, then
-		...['-o', 'UserKnownHostsFile=/dev/null'],
+	// attempting ssh with a regular user (balenaCloud username). The problem with
-		`root@${opts.address}`,
+	// that approach was that it would print the following message to the console:
-		...(command ? [command] : []),
+	//     "root@192.168.1.36: Permission denied (publickey)"
-	]);
+	// ... right before having success as a regular user, which looked broken or
+	// confusing from users' point of view.  Capturing stderr to prevent that
+	// message from being printed is tricky because the messages printed to stderr
+	// may include the stderr output of remote commands that are of interest to
+	// the user.  Workarounds based on delays (timing) are tricky too because a
+	// ssh session length may vary from a fraction of a second (non interactive)
+	// to hours or days.
+	const username = await findBestUsernameForDevice(opts.address);
+
+	await runRemoteCommand({
+		cmd,
+		hostname: opts.address,
+		port: Number(opts.port) || 'local',
+		username,
+		verbose: opts.verbose,
+	});
 }

lib/utils/promote.ts

@@ -20,7 +20,7 @@ import { ExpectedError, printErrorMessage } from '../errors';
 import { getVisuals, stripIndent, getCliForm } from './lazy';
 import Logger = require('./logger');
 import { confirm } from './patterns';
-import { exec, execBuffered, getDeviceOsRelease } from './ssh';
+import { getLocalDeviceCmdStdout, getDeviceOsRelease } from './ssh';
 
 const MIN_BALENAOS_VERSION = 'v2.14.0';
 
@@ -102,8 +102,11 @@ async function execCommand(
 	});
 
 	spinner.start();
-	await exec(deviceIp, cmd, stream);
+	try {
-	spinner.stop();
+		await getLocalDeviceCmdStdout(deviceIp, cmd, stream);
+	} finally {
+		spinner.stop();
+	}
 }
 
 async function configure(deviceIp: string, config: any): Promise<void> {
@@ -123,7 +126,7 @@ async function deconfigure(deviceIp: string): Promise<void> {
 async function assertDeviceIsCompatible(deviceIp: string): Promise<void> {
 	const cmd = 'os-config --version';
 	try {
-		await execBuffered(deviceIp, cmd);
+		await getLocalDeviceCmdStdout(deviceIp, cmd);
 	} catch (err) {
 		if (err instanceof ExpectedError) {
 			throw err;

lib/utils/ssh.ts

@@ -16,147 +16,309 @@
  */
 import { spawn, StdioOptions } from 'child_process';
 import * as _ from 'lodash';
-import { TypedError } from 'typed-error';
 
 import { ExpectedError } from '../errors';
 
-export class ExecError extends TypedError {
+export class SshPermissionDeniedError extends ExpectedError {}
-	public cmd: string;
-	public exitCode: number;
 
-	constructor(cmd: string, exitCode: number) {
+export class RemoteCommandError extends ExpectedError {
-		super(`Command '${cmd}' failed with error: ${exitCode}`);
+	cmd: string;
+	exitCode?: number;
+	exitSignal?: NodeJS.Signals;
+
+	constructor(cmd: string, exitCode?: number, exitSignal?: NodeJS.Signals) {
+		super(sshErrorMessage(cmd, exitSignal, exitCode));
 		this.cmd = cmd;
 		this.exitCode = exitCode;
+		this.exitSignal = exitSignal;
 	}
 }
 
-export async function exec(
+export interface SshRemoteCommandOpts {
-	deviceIp: string,
+	cmd?: string;
-	cmd: string,
+	hostname: string;
-	stdout?: NodeJS.WritableStream,
+	ignoreStdin?: boolean;
-): Promise<void> {
+	port?: number | 'cloud' | 'local';
+	proxyCommand?: string[];
+	username?: string;
+	verbose?: boolean;
+}
+
+export const stdioIgnore: {
+	stdin: 'ignore';
+	stdout: 'ignore';
+	stderr: 'ignore';
+} = {
+	stdin: 'ignore',
+	stdout: 'ignore',
+	stderr: 'ignore',
+};
+
+export function sshArgsForRemoteCommand({
+	cmd = '',
+	hostname,
+	ignoreStdin = false,
+	port,
+	proxyCommand,
+	username = 'root',
+	verbose = false,
+}: SshRemoteCommandOpts): string[] {
+	port = port === 'local' ? 22222 : port === 'cloud' ? 22 : port;
+	return [
+		...(verbose ? ['-vvv'] : []),
+		...(ignoreStdin ? ['-n'] : []),
+		'-t',
+		...(port ? ['-p', port.toString()] : []),
+		...['-o', 'LogLevel=ERROR'],
+		...['-o', 'StrictHostKeyChecking=no'],
+		...['-o', 'UserKnownHostsFile=/dev/null'],
+		...(proxyCommand && proxyCommand.length
+			? ['-o', `ProxyCommand=${proxyCommand.join(' ')}`]
+			: []),
+		`${username}@${hostname}`,
+		...(cmd ? [cmd] : []),
+	];
+}
+
+/**
+ * Execute the given command on a local balenaOS device over ssh.
+ * @param cmd Shell command to execute on the device
+ * @param hostname Device's hostname or IP address
+ * @param port SSH server TCP port number or 'local' (22222) or 'cloud' (22)
+ * @param stdin Readable stream to pipe to the remote command stdin,
+ * or 'ignore' or 'inherit' as documented in the child_process.spawn function.
+ * @param stdout Writeable stream to pipe from the remote command stdout,
+ * or 'ignore' or 'inherit' as documented in the child_process.spawn function.
+ * @param stderr Writeable stream to pipe from the remote command stdout,
+ * or 'ignore' or 'inherit' as documented in the child_process.spawn function.
+ * @param username SSH username for authorization. With balenaOS 2.44.0 or
+ * later, it can be a balenaCloud username.
+ * @param verbose Produce debugging output
+ */
+export async function runRemoteCommand({
+	cmd = '',
+	hostname,
+	port,
+	proxyCommand,
+	stdin = 'inherit',
+	stdout = 'inherit',
+	stderr = 'inherit',
+	username = 'root',
+	verbose = false,
+}: SshRemoteCommandOpts & {
+	stdin?: 'ignore' | 'inherit' | NodeJS.ReadableStream;
+	stdout?: 'ignore' | 'inherit' | NodeJS.WritableStream;
+	stderr?: 'ignore' | 'inherit' | NodeJS.WritableStream;
+}): Promise<void> {
+	let ignoreStdin: boolean;
+	if (stdin === 'ignore') {
+		// Set ignoreStdin=true in order for the "ssh -n" option to be used to
+		// prevent the ssh client from using the CLI process stdin. In addition,
+		// stdin must be forced to 'inherit' (if it is not a readable stream) in
+		// order to work around 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 actually fixed the bug in newer versions of the ssh client:
+		// https://github.com/PowerShell/Win32-OpenSSH/issues/856 but users
+		// have to manually download and install a new client.
+		ignoreStdin = true;
+		stdin = 'inherit';
+	} else {
+		ignoreStdin = false;
+	}
 	const { which } = await import('./which');
 	const program = await which('ssh');
-	const args = [
+	const args = sshArgsForRemoteCommand({
-		'-n',
-		'-t',
-		'-p',
-		'22222',
-		'-o',
-		'LogLevel=ERROR',
-		'-o',
-		'StrictHostKeyChecking=no',
-		'-o',
-		'UserKnownHostsFile=/dev/null',
-		`root@${deviceIp}`,
 		cmd,
-	];
+		hostname,
+		ignoreStdin,
+		port,
+		proxyCommand,
+		username,
+		verbose,
+	});
+
 	if (process.env.DEBUG) {
 		const logger = (await import('./logger')).getLogger();
 		logger.logDebug(`Executing [${program},${args}]`);
 	}
 
-	// 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',
+		typeof stdin === 'string' ? stdin : 'pipe',
-		stdout ? 'pipe' : 'inherit',
+		typeof stdout === 'string' ? stdout : 'pipe',
-		'inherit',
+		typeof stderr === 'string' ? stderr : 'pipe',
 	];
+	let exitCode: number | undefined;
+	let exitSignal: NodeJS.Signals | undefined;
+	try {
+		[exitCode, exitSignal] = await new Promise<[number, NodeJS.Signals]>(
+			(resolve, reject) => {
+				const ps = spawn(program, args, { stdio })
+					.on('error', reject)
+					.on('close', (code, signal) => resolve([code, signal]));
 
-	const exitCode = await new Promise<number>((resolve, reject) => {
+				if (ps.stdin && stdin && typeof stdin !== 'string') {
-		const ps = spawn(program, args, { stdio })
+					stdin.pipe(ps.stdin);
-			.on('error', reject)
+				}
-			.on('close', resolve);
+				if (ps.stdout && stdout && typeof stdout !== 'string') {
-
+					ps.stdout.pipe(stdout);
-		if (stdout && ps.stdout) {
+				}
-			ps.stdout.pipe(stdout);
+				if (ps.stderr && stderr && typeof stderr !== 'string') {
-		}
+					ps.stderr.pipe(stderr);
-	});
+				}
-	if (exitCode !== 0) {
+			},
-		throw new ExecError(cmd, exitCode);
+		);
+	} catch (error) {
+		const msg = [
+			`ssh failed with exit code=${exitCode} signal=${exitSignal}:`,
+			`[${program}, ${args.join(', ')}]`,
+			...(error ? [`${error}`] : []),
+		];
+		throw new ExpectedError(msg.join('\n'));
+	}
+	if (exitCode || exitSignal) {
+		throw new RemoteCommandError(cmd, exitCode, exitSignal);
 	}
 }
 
-export async function execBuffered(
+/**
-	deviceIp: string,
+ * Execute the given command on a local balenaOS device over ssh.
-	cmd: string,
+ * Capture stdout and/or stderr to Buffers and return them.
-	enc?: string,
+ *
-): Promise<string> {
+ * @param deviceIp IP address of the local device
-	const through = await import('through2');
+ * @param cmd Shell command to execute on the device
-	const buffer: string[] = [];
+ * @param opts Options
-	await exec(
+ * @param opts.username SSH username for authorization. With balenaOS 2.44.0 or
-		deviceIp,
+ * later, it may be a balenaCloud username. Otherwise, 'root'.
+ * @param opts.stdin Passed through to the runRemoteCommand function
+ * @param opts.stdout If 'capture', capture stdout to a Buffer.
+ * @param opts.stderr If 'capture', capture stdout to a Buffer.
+ */
+export async function getRemoteCommandOutput({
+	cmd,
+	hostname,
+	port,
+	proxyCommand,
+	stdin = 'ignore',
+	stdout = 'capture',
+	stderr = 'capture',
+	username = 'root',
+	verbose = false,
+}: SshRemoteCommandOpts & {
+	stdin?: 'ignore' | 'inherit' | NodeJS.ReadableStream;
+	stdout?: 'capture' | 'ignore' | 'inherit' | NodeJS.WritableStream;
+	stderr?: 'capture' | 'ignore' | 'inherit' | NodeJS.WritableStream;
+}): Promise<{ stdout: Buffer; stderr: Buffer }> {
+	const { Writable } = await import('stream');
+	const stdoutChunks: Buffer[] = [];
+	const stderrChunks: Buffer[] = [];
+	const stdoutStream = new Writable({
+		write(chunk: Buffer, _enc, callback) {
+			stdoutChunks.push(chunk);
+			callback();
+		},
+	});
+	const stderrStream = new Writable({
+		write(chunk: Buffer, _enc, callback) {
+			stderrChunks.push(chunk);
+			callback();
+		},
+	});
+	await runRemoteCommand({
 		cmd,
-		through(function (data, _enc, cb) {
+		hostname,
-			buffer.push(data.toString(enc));
+		port,
-			cb();
+		proxyCommand,
-		}),
+		stdin,
-	);
+		stdout: stdout === 'capture' ? stdoutStream : stdout,
-	return buffer.join('');
+		stderr: stderr === 'capture' ? stderrStream : stderr,
+		username,
+		verbose,
+	});
+	return {
+		stdout: Buffer.concat(stdoutChunks),
+		stderr: Buffer.concat(stderrChunks),
+	};
 }
 
+/** Convenience wrapper for getRemoteCommandOutput */
+export async function getLocalDeviceCmdStdout(
+	hostname: string,
+	cmd: string,
+	stdout: 'capture' | 'ignore' | 'inherit' | NodeJS.WritableStream = 'capture',
+): Promise<Buffer> {
+	return (
+		await getRemoteCommandOutput({
+			cmd,
+			hostname,
+			port: 'local',
+			stdout,
+			stderr: 'inherit',
+			username: await findBestUsernameForDevice(hostname),
+		})
+	).stdout;
+}
+
+/**
+ * Run a trivial 'exit 0' command over ssh on the target hostname (typically the
+ * IP address of a local device) with the 'root' username, in order to determine
+ * whether root authentication suceeds. It should succeed with development
+ * variants of balenaOS and fail with production variants, unless a ssh key was
+ * added to the device's 'config.json' file.
+ * @return True if succesful, false on any errors.
+ */
+export const isRootUserGood = _.memoize(
+	async (hostname: string, port = 'local') => {
+		try {
+			await runRemoteCommand({ cmd: 'exit 0', hostname, port, ...stdioIgnore });
+		} catch (e) {
+			return false;
+		}
+		return true;
+	},
+);
+
+/**
+ * Determine whether the given local device (hostname or IP address) should be
+ * accessed as the 'root' user or as a regular cloud user (balenaCloud or
+ * openBalena). Where possible, the root user is preferable because:
+ * - It allows ssh to be used in air-gapped scenarios (no internet access).
+ *   Logging in as a regular user requires the device to fetch public keys from
+ *   the cloud backend.
+ * - Root authentication is significantly faster for local devices (a fraction
+ *   of a second versus 5+ seconds).
+ * - Non-root authentication requires balenaOS v2.44.0 or later, so not (yet)
+ *   universally possible.
+ */
+export const findBestUsernameForDevice = _.memoize(
+	async (hostname: string, port = 'local'): Promise<string> => {
+		let username: string | undefined;
+		if (await isRootUserGood(hostname, port)) {
+			username = 'root';
+		} else {
+			const { getCachedUsername } = await import('./bootstrap');
+			username = (await getCachedUsername())?.username;
+		}
+		return username || 'root';
+	},
+);
+
 /**
  * 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) =>
+export const getDeviceOsRelease = _.memoize(async (hostname: string) =>
-	execBuffered(deviceIp, 'cat /etc/os-release'),
+	(await getLocalDeviceCmdStdout(hostname, 'cat /etc/os-release')).toString(),
 );
 
-// TODO: consolidate the various forms of executing ssh child processes
+function sshErrorMessage(cmd: string, exitSignal?: string, exitCode?: number) {
-// in the CLI, like exec and spawn, starting with the files:
-//   lib/actions/ssh.ts
-//   lib/utils/ssh.ts
-//   lib/utils/device/ssh.ts
-
-/**
- * Obtain the full path for ssh using which, then spawn a child process.
- * - If the child process returns error code 0, return the function normally
- *   (do not throw an error).
- * - If the child process returns a non-zero error code, set process.exitCode
- *   to that error code, and throw ExpectedError with a warning message.
- * - If the child process is terminated by a process signal, set
- *   process.exitCode = 1, and throw ExpectedError with a warning message.
- */
-export async function spawnSshAndThrowOnError(
-	args: string[],
-	options?: import('child_process').SpawnOptions,
-) {
-	const { whichSpawn } = await import('./which');
-	const [exitCode, exitSignal] = await whichSpawn(
-		'ssh',
-		args,
-		options,
-		true, // returnExitCodeOrSignal
-	);
-	if (exitCode || exitSignal) {
-		// ssh returns a wide range of exit codes, including return codes of
-		// interactive shells. For example, if the user types CTRL-C on an
-		// interactive shell and then `exit`, ssh returns error code 130.
-		// Another example, typing "exit 1" on an interactive shell causes ssh
-		// to return exit code 1. In these cases, print a short one-line warning
-		// message, and exits the CLI process with the same error code.
-		process.exitCode = exitCode;
-		throw new ExpectedError(sshErrorMessage(exitSignal, exitCode));
-	}
-}
-
-function sshErrorMessage(exitSignal?: string, exitCode?: number) {
 	const msg: string[] = [];
+	cmd = cmd ? `Remote command "${cmd}"` : 'Process';
 	if (exitSignal) {
-		msg.push(`Warning: ssh process was terminated with signal "${exitSignal}"`);
+		msg.push(`SSH: ${cmd} terminated with signal "${exitSignal}"`);
 	} else {
-		msg.push(`Warning: ssh process exited with non-zero code "${exitCode}"`);
+		msg.push(`SSH: ${cmd} exited with non-zero status code "${exitCode}"`);
 		switch (exitCode) {
 			case 255:
 				msg.push(`

lib/utils/which.ts

@@ -95,52 +95,3 @@ export async function which(
 	}
 	return programPath;
 }
-
-/**
- * Call which(programName) and spawn() with the given arguments.
- *
- * If returnExitCodeOrSignal is true, the returned promise will resolve to
- * an array [code, signal] with the child process exit code number or exit
- * signal string respectively (as provided by the spawn close event).
- *
- * If returnExitCodeOrSignal is false, the returned promise will reject with
- * a custom error if the child process returns a non-zero exit code or a
- * non-empty signal string (as reported by the spawn close event).
- *
- * In either case and if spawn itself emits an error event or fails synchronously,
- * the returned promise will reject with a custom error that includes the error
- * message of spawn's error.
- */
-export async function whichSpawn(
-	programName: string,
-	args: string[],
-	options: import('child_process').SpawnOptions = { stdio: 'inherit' },
-	returnExitCodeOrSignal = false,
-): Promise<[number | undefined, string | undefined]> {
-	const { spawn } = await import('child_process');
-	const program = await which(programName);
-	if (process.env.DEBUG) {
-		console.error(`[debug] [${program}, ${args.join(', ')}]`);
-	}
-	let error: Error | undefined;
-	let exitCode: number | undefined;
-	let exitSignal: string | undefined;
-	try {
-		[exitCode, exitSignal] = await new Promise((resolve, reject) => {
-			spawn(program, args, options)
-				.on('error', reject)
-				.on('close', (code, signal) => resolve([code, signal]));
-		});
-	} catch (err) {
-		error = err;
-	}
-	if (error || (!returnExitCodeOrSignal && (exitCode || exitSignal))) {
-		const msg = [
-			`${programName} failed with exit code=${exitCode} signal=${exitSignal}:`,
-			`[${program}, ${args.join(', ')}]`,
-			...(error ? [`${error}`] : []),
-		];
-		throw new Error(msg.join('\n'));
-	}
-	return [exitCode, exitSignal];
-}

tests/commands/ssh.spec.ts

@@ -32,33 +32,51 @@ describe('balena ssh', function () {
 	let hasSshExecutable = false;
 	let mockedExitCode = 0;
 
+	async function mockSpawn({ revert = false } = {}) {
+		if (revert) {
+			mock.stopAll();
+			mock.reRequire('../../build/utils/ssh');
+			return;
+		}
+		const { EventEmitter } = await import('stream');
+		const childProcessMod = await import('child_process');
+		const originalSpawn = childProcessMod.spawn;
+		mock('child_process', {
+			...childProcessMod,
+			spawn: (program: string, ...args: any[]) => {
+				if (program.includes('ssh')) {
+					const emitter = new EventEmitter();
+					setTimeout(() => emitter.emit('close', mockedExitCode), 1);
+					return emitter;
+				}
+				return originalSpawn(program, ...args);
+			},
+		});
+	}
+
 	this.beforeAll(async function () {
 		hasSshExecutable = await checkSsh();
-		if (hasSshExecutable) {
+		if (!hasSshExecutable) {
-			[sshServer, sshServerPort] = await startMockSshServer();
+			this.skip();
 		}
-		const modPath = '../../build/utils/which';
+		[sshServer, sshServerPort] = await startMockSshServer();
-		const mod = await import(modPath);
+		await mockSpawn();
-		mock(modPath, {
-			...mod,
-			whichSpawn: async () => [mockedExitCode, undefined],
-		});
 	});
 
-	this.afterAll(function () {
+	this.afterAll(async function () {
 		if (sshServer) {
 			sshServer.close();
 			sshServer = undefined;
 		}
-		mock.stopAll();
+		await mockSpawn({ revert: true });
 	});
 
-	this.beforeEach(() => {
+	this.beforeEach(function () {
 		api = new BalenaAPIMock();
 		api.expectGetMixpanel({ optional: true });
 	});
 
-	this.afterEach(() => {
+	this.afterEach(function () {
 		// Check all expected api calls have been made and clean up.
 		api.done();
 	});
@@ -87,7 +105,7 @@ describe('balena ssh', function () {
 		async () => {
 			const deviceUUID = 'abc1234';
 			const expectedErrLines = [
-				'Warning: ssh process exited with non-zero code "255"',
+				'SSH: Remote command "host abc1234" exited with non-zero status code "255"',
 			];
 			api.expectGetWhoAmI({ optional: true, persist: true });
 			api.expectGetDevice({ fullUUID: deviceUUID, isOnline: true });
@@ -99,21 +117,6 @@ describe('balena ssh', function () {
 		},
 	);
 
-	it('should produce the expected error message (real ssh, device IP address)', async function () {
-		if (!hasSshExecutable) {
-			this.skip();
-		}
-		mock.stop('../../build/utils/helpers');
-		const expectedErrLines = [
-			'Warning: ssh process exited with non-zero code "255"',
-		];
-		const { err, out } = await runCommand(
-			`ssh 127.0.0.1 -p ${sshServerPort} --noproxy`,
-		);
-		expect(cleanOutput(err, true)).to.include.members(expectedErrLines);
-		expect(out).to.be.empty;
-	});
-
 	it('should fail if device not online (mocked, device UUID)', async () => {
 		const deviceUUID = 'abc1234';
 		const expectedErrLines = ['Device with UUID abc1234 is offline'];
@@ -126,6 +129,18 @@ describe('balena ssh', function () {
 		expect(cleanOutput(err, true)).to.include.members(expectedErrLines);
 		expect(out).to.be.empty;
 	});
+
+	it('should produce the expected error message (real ssh, device IP address)', async function () {
+		await mockSpawn({ revert: true });
+		const expectedErrLines = [
+			'SSH: Process exited with non-zero status code "255"',
+		];
+		const { err, out } = await runCommand(
+			`ssh 127.0.0.1 -p ${sshServerPort} --noproxy`,
+		);
+		expect(cleanOutput(err, true)).to.include.members(expectedErrLines);
+		expect(out).to.be.empty;
+	});
 });
 
 /** Check whether the 'ssh' tool (executable) exists in the PATH */