[v14] Check if binary exists before passing it to node-pty

web/packages/teleterm/package.json

@@ -28,10 +28,12 @@
   "homepage": "https://goteleport.com",
   "dependencies": {
     "@types/tar-fs": "^2.0.1",
+    "@types/which": "^3.0.4",
     "emittery": "^1.0.1",
     "node-pty": "1.1.0-beta14",
     "strip-ansi": "^7.1.0",
-    "tar-fs": "^3.0.3"
+    "tar-fs": "^3.0.3",
+    "which": "^4.0.0"
   },
   "devDependencies": {
     "@gravitational/build": "^1.0.0",

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

@@ -83,8 +83,13 @@ export class PtyEventsStreamHandler {
         )
       );
     });
-    this.ptyProcess.start(event.getColumns(), event.getRows());
-    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.getColumns(), event.getRows()).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

@@ -14,22 +14,22 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-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;
@@ -52,8 +52,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, {

yarn.lock

@@ -4572,6 +4572,11 @@
   dependencies:
     "@types/webidl-conversions" "*"
 
+"@types/which@^3.0.4":
+  version "3.0.4"
+  resolved "https://registry.yarnpkg.com/@types/which/-/which-3.0.4.tgz#2c3a89be70c56a84a6957a7264639f39ae4340a1"
+  integrity sha512-liyfuo/106JdlgSchJzXEQCVArk0CvevqPote8F8HgWgJ3dRCcTHgJIsLDuee0kxk/mhbInzIZk3QWSZJ8R+2w==
+
 "@types/wicg-file-system-access@^2020.9.5":
   version "2020.9.5"
   resolved "https://registry.yarnpkg.com/@types/wicg-file-system-access/-/wicg-file-system-access-2020.9.5.tgz#4a0c8f3d1ed101525f329e86c978f7735404474f"
@@ -10500,6 +10505,11 @@ isexe@^2.0.0:
   resolved "https://registry.yarnpkg.com/isexe/-/isexe-2.0.0.tgz#e8fbf374dc556ff8947a10dcb0572d633f2cfa10"
   integrity sha1-6PvzdNxVb/iUehDcsFctYz8s+hA=
 
+isexe@^3.1.1:
+  version "3.1.1"
+  resolved "https://registry.yarnpkg.com/isexe/-/isexe-3.1.1.tgz#4a407e2bd78ddfb14bea0c27c6f7072dde775f0d"
+  integrity sha512-LpB/54B+/2J5hqQ7imZHfdU31OlgQqx7ZicVlkm9kzg9/w8GKLEcFfJl/t7DCEDueOyBAD6zCCwTO6Fzs0NoEQ==
+
 isobject@^2.0.0:
   version "2.1.0"
   resolved "https://registry.yarnpkg.com/isobject/-/isobject-2.1.0.tgz#f065561096a3f1da2ef46272f815c840d87e0c89"
@@ -16968,6 +16978,13 @@ which@^2.0.1, which@^2.0.2:
   dependencies:
     isexe "^2.0.0"
 
+which@^4.0.0:
+  version "4.0.0"
+  resolved "https://registry.yarnpkg.com/which/-/which-4.0.0.tgz#cd60b5e74503a3fbcfbf6cd6b4138a8bae644c1a"
+  integrity sha512-GlaYyEb07DPxYCKhKzplCWBJtvxZcZMrL+4UkrTSJHHPyZU4mYYTv3qaOe77H7EODLSSopAUFAc6W8U4yqvscg==
+  dependencies:
+    isexe "^3.1.1"
+
 wide-align@^1.1.2, wide-align@^1.1.5:
   version "1.1.5"
   resolved "https://registry.yarnpkg.com/wide-align/-/wide-align-1.1.5.tgz#df1d4c206854369ecf3c9a4898f1b23fbd9d15d3"