Check if binary exists before passing it to node-pty

web/packages/teleterm/package.json

@@ -24,12 +24,14 @@
   "homepage": "https://goteleport.com",
   "dependencies": {
     "@grpc/grpc-js": "1.10.10",
+    "@types/which": "^3.0.4",
     "node-forge": "^1.3.1",
     "node-pty": "1.1.0-beta14",
     "ring-buffer-ts": "^1.2.0",
     "split2": "4.2.0",
     "strip-ansi": "^7.1.0",
     "tar-fs": "^3.0.6",
+    "which": "^4.0.0",
     "winston": "^3.13.0"
   },
   "devDependencies": {

web/packages/teleterm/src/sharedProcess/ptyHost/ptyEventsStreamHandler.ts

@@ -112,8 +112,13 @@ export class PtyEventsStreamHandler {
         })
       );
     });
-    this.ptyProcess.start(event.columns, event.rows);
-    this.logger.info(`stream has started`);
+    // PtyProcess.prototype.start always returns a fulfilled promise. If an error is caught during
+    // start, it's reported through PtyProcess.prototype.onStartError. Similarly, the information
+    // about a successful start is also conveyed through an emitted event rather than the method
+    // returning with no error. Hence why we can ignore what this promise returns.
+    void this.ptyProcess.start(event.columns, event.rows).then(() => {
+      this.logger.info(`stream has started`);
+    });
   }
 
   private handleDataEvent(event: PtyEventData): void {

web/packages/teleterm/src/sharedProcess/ptyHost/ptyProcess.test.ts

@@ -0,0 +1,54 @@
+/**
+ * @jest-environment node
+ */
+/**
+ * Teleport
+ * Copyright (C) 2024 Gravitational, Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+import crypto from 'node:crypto';
+
+import Logger, { NullService } from 'teleterm/logger';
+
+import { PtyProcess } from './ptyProcess';
+
+beforeAll(() => {
+  Logger.init(new NullService());
+});
+
+describe('PtyProcess', () => {
+  describe('start', () => {
+    it('emits a start error when attempting to execute a nonexistent command', async () => {
+      const path = `nonexistent-executable-${crypto.randomUUID()}`;
+      const pty = new PtyProcess({
+        path,
+        args: [],
+        env: { PATH: '/foo/bar' },
+        ptyId: '1234',
+      });
+
+      const startErrorCb = jest.fn();
+
+      pty.onStartError(startErrorCb);
+
+      await pty.start(80, 20);
+
+      expect(startErrorCb).toHaveBeenCalledWith(
+        expect.stringContaining(`not found: ${path}`)
+      );
+    });
+  });
+});

web/packages/teleterm/src/sharedProcess/ptyHost/ptyProcess.ts

@@ -16,22 +16,22 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-import { readlink } from 'fs';
-
-import { exec } from 'child_process';
-
-import { promisify } from 'util';
-
-import { EventEmitter } from 'events';
+import { readlink } from 'node:fs';
+import { exec } from 'node:child_process';
+import { promisify } from 'node:util';
+import { EventEmitter } from 'node:events';
 
 import * as nodePTY from 'node-pty';
+import which from 'which';
 
 import Logger from 'teleterm/logger';
 
 import { PtyProcessOptions, IPtyProcess } from './types';
 
 type Status = 'open' | 'not_initialized' | 'terminated';
 
+const pathEnvVar = process.platform === 'win32' ? 'Path' : 'PATH';
+
 export class PtyProcess extends EventEmitter implements IPtyProcess {
   private _buffered = true;
   private _attachedBufferTimer;
@@ -54,8 +54,18 @@ export class PtyProcess extends EventEmitter implements IPtyProcess {
     return this.options.ptyId;
   }
 
-  start(cols: number, rows: number) {
+  /**
+   * start spawns a new PTY with the arguments given through the constructor.
+   * It emits TermEventEnum.StartError on error. start itself always returns a fulfilled promise.
+   */
+  async start(cols: number, rows: number) {
     try {
+      // which throws an error if the argument is not found in path.
+      // TODO(ravicious): Remove the manual check for the existence of the executable after node-pty
+      // makes its behavior consistent across platforms.
+      // https://github.com/microsoft/node-pty/issues/689
+      await which(this.options.path, { path: this.options.env[pathEnvVar] });
+
       // TODO(ravicious): Set argv0 when node-pty adds support for it.
       // https://github.com/microsoft/node-pty/issues/472
       this._process = nodePTY.spawn(this.options.path, this.options.args, {