Merge pull request #1689 from balena-io/add-ssh-tty-option

balena ssh: handle exit codes and add `-t` option (force TTY allocation)

doc/cli.markdown

@@ -1210,6 +1210,10 @@ support) please check:
 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).
 
+#### --tty, -t
+
+Force pseudo-terminal allocation (bypass TTY autodetection for stdin)
+
 #### --verbose, -v
 
 Increase verbosity

lib/actions/ssh.ts

@@ -169,6 +169,7 @@ export const ssh: CommandDefinition<
 	{
 		port: string;
 		service: string;
+		tty: boolean;
 		verbose: true | undefined;
 		noProxy: boolean;
 	}
@@ -214,6 +215,13 @@ export const ssh: CommandDefinition<
 				hostname. Otherwise, port number for the balenaCloud gateway (default 22).`,
 			alias: 'p',
 		},
+		{
+			signature: 'tty',
+			boolean: true,
+			description:
+				'Force pseudo-terminal allocation (bypass TTY autodetection for stdin)',
+			alias: 't',
+		},
 		{
 			signature: 'verbose',
 			boolean: true,
@@ -230,12 +238,11 @@ export const ssh: CommandDefinition<
 		const applicationOrDevice =
 			params.applicationOrDevice_raw || params.applicationOrDevice;
 		const { ExpectedError } = await import('../errors');
-		const { getProxyConfig, which, whichSpawn } = await import(
+		const { getProxyConfig, which } = await import('../utils/helpers');
-			'../utils/helpers'
-		);
 		const { checkLoggedIn, getOnlineTargetUuid } = await import(
 			'../utils/patterns'
 		);
+		const { spawnSshAndExitOnError } = await import('../utils/ssh');
 		const sdk = getBalenaSdk();
 
 		const verbose = options.verbose === true;
@@ -252,6 +259,7 @@ export const ssh: CommandDefinition<
 			return await performLocalDeviceSSH({
 				address: applicationOrDevice,
 				port,
+				forceTTY: options.tty === true,
 				verbose,
 				service: params.serviceName,
 			});
@@ -356,6 +364,6 @@ export const ssh: CommandDefinition<
 			username: username!,
 		});
 
-		await whichSpawn('ssh', command);
+		return spawnSshAndExitOnError(command);
 	},
 };

lib/utils/device/ssh.ts

@@ -18,6 +18,7 @@ import { ContainerInfo } from 'dockerode';
 export interface DeviceSSHOpts {
 	address: string;
 	port?: number;
+	forceTTY?: boolean;
 	verbose: boolean;
 	service?: string;
 }
@@ -28,10 +29,9 @@ export async function performLocalDeviceSSH(
 	opts: DeviceSSHOpts,
 ): Promise<void> {
 	const reduce = await import('lodash/reduce');
-	const { whichSpawn } = await import('../helpers');
+	const { spawnSshAndExitOnError } = await import('../ssh');
 	const { ExpectedError } = await import('../../errors');
 	const { stripIndent } = await import('common-tags');
-	const { isatty } = await import('tty');
 
 	let command = '';
 
@@ -103,11 +103,12 @@ export async function performLocalDeviceSSH(
 		// echo 'ls -la; exit;' | balena ssh 192.168.0.20 service1
 		// See https://www.balena.io/blog/balena-monthly-roundup-january-2020/#charliestipsntricks
 		//     https://assets.balena.io/newsletter/2020-01/pipe.png
-		const ttyFlag = isatty(0) ? '-t' : '';
+		const isTTY = !!opts.forceTTY || (await import('tty')).isatty(0);
+		const ttyFlag = isTTY ? '-t' : '';
 		command = `${deviceContainerEngineBinary} exec -i ${ttyFlag} ${containerId} ${shellCmd}`;
 	}
 
-	return whichSpawn('ssh', [
+	return spawnSshAndExitOnError([
 		...(opts.verbose ? ['-vvv'] : []),
 		'-t',
 		...['-p', opts.port ? opts.port.toString() : '22222'],

lib/utils/helpers.ts

@@ -380,36 +380,50 @@ export async function which(
 
 /**
  * Call which(programName) and spawn() with the given arguments.
- * Reject the promise if the process exit code is not zero.
+ *
+ * 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: SpawnOptions = { stdio: 'inherit' },
-): Promise<void> {
+	returnExitCodeOrSignal = false,
+): Promise<[number | undefined, string | undefined]> {
 	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 = await new Promise<number>((resolve, reject) => {
+		[exitCode, exitSignal] = await new Promise((resolve, reject) => {
 			spawn(program, args, options)
 				.on('error', reject)
-				.on('close', resolve);
+				.on('close', (code, signal) => resolve([code, signal]));
 		});
 	} catch (err) {
 		error = err;
 	}
-	if (error || exitCode) {
+	if (error || (!returnExitCodeOrSignal && (exitCode || exitSignal))) {
 		const msg = [
-			`${programName} failed with exit code ${exitCode}:`,
+			`${programName} failed with exit code=${exitCode} signal=${exitSignal}:`,
 			`[${program}, ${args.join(', ')}]`,
 			...(error ? [`${error}`] : []),
 		];
 		throw new Error(msg.join('\n'));
 	}
+	return [exitCode, exitSignal];
 }
 
 export interface ProxyConfig {

lib/utils/ssh.ts

@@ -111,3 +111,49 @@ export async function execBuffered(
 export const getDeviceOsRelease = _.memoize(async (deviceIp: string) =>
 	execBuffered(deviceIp, 'cat /etc/os-release'),
 );
+
+// TODO: consolidate the various forms of executing ssh child processes
+// 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 call process.exit()).
+ * - If the child process returns a non-zero error code, print a single-line
+ *   warning message and call process.exit(code) with the same non-zero error
+ *   code.
+ * - If the child process is terminated by a process signal, print a
+ *   single-line warning message and call process.exit(1).
+ */
+export async function spawnSshAndExitOnError(
+	args: string[],
+	options?: import('child_process').SpawnOptions,
+) {
+	const { whichSpawn } = await import('./helpers');
+	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.
+		const codeMsg = exitSignal
+			? `was terminated with signal "${exitSignal}"`
+			: `exited with non-zero code "${exitCode}"`;
+		console.error(`Warning: ssh process ${codeMsg}`);
+		// TODO: avoid process.exit by refactoring CLI error handling to allow
+		// exiting with an error code and single-line warning "without a fuss"
+		// about contacting support and filing Github issues. (ExpectedError
+		// does not currently devlivers that.)
+		process.exit(exitCode || 1);
+	}
+}