Connect: Do not leak last terminal input in logs (#67049)

* Do not leak `lastInput` in logs

* Security hardening: use `execFile` instead of `exec`

gen/proto/ts/teleport/web/teleterm/ptyhost/v1/pty_host_service_pb.ts

@@ -207,9 +207,9 @@ export interface PtyEventExit {
      */
     signal?: number;
     /**
-     * @generated from protobuf field: string last_input = 3;
+     * @generated from protobuf field: bool last_input_was_ctrl_d = 4;
      */
-    lastInput: string;
+    lastInputWasCtrlD: boolean;
 }
 /**
  * PtyEventStartError is sent by the PTY process when the shared process fails to start it.
@@ -715,13 +715,13 @@ class PtyEventExit$Type extends MessageType<PtyEventExit> {
         super("teleport.web.teleterm.ptyhost.v1.PtyEventExit", [
             { no: 1, name: "exit_code", kind: "scalar", T: 13 /*ScalarType.UINT32*/ },
             { no: 2, name: "signal", kind: "scalar", opt: true, T: 13 /*ScalarType.UINT32*/ },
-            { no: 3, name: "last_input", kind: "scalar", T: 9 /*ScalarType.STRING*/ }
+            { no: 4, name: "last_input_was_ctrl_d", kind: "scalar", T: 8 /*ScalarType.BOOL*/ }
         ]);
     }
     create(value?: PartialMessage<PtyEventExit>): PtyEventExit {
         const message = globalThis.Object.create((this.messagePrototype!));
         message.exitCode = 0;
-        message.lastInput = "";
+        message.lastInputWasCtrlD = false;
         if (value !== undefined)
             reflectionMergePartial<PtyEventExit>(this, message, value);
         return message;
@@ -737,8 +737,8 @@ class PtyEventExit$Type extends MessageType<PtyEventExit> {
                 case /* optional uint32 signal */ 2:
                     message.signal = reader.uint32();
                     break;
-                case /* string last_input */ 3:
-                    message.lastInput = reader.string();
+                case /* bool last_input_was_ctrl_d */ 4:
+                    message.lastInputWasCtrlD = reader.bool();
                     break;
                 default:
                     let u = options.readUnknownField;
@@ -758,9 +758,9 @@ class PtyEventExit$Type extends MessageType<PtyEventExit> {
         /* optional uint32 signal = 2; */
         if (message.signal !== undefined)
             writer.tag(2, WireType.Varint).uint32(message.signal);
-        /* string last_input = 3; */
-        if (message.lastInput !== "")
-            writer.tag(3, WireType.LengthDelimited).string(message.lastInput);
+        /* bool last_input_was_ctrl_d = 4; */
+        if (message.lastInputWasCtrlD !== false)
+            writer.tag(4, WireType.Varint).bool(message.lastInputWasCtrlD);
         let u = options.writeUnknownFields;
         if (u !== false)
             (u == true ? UnknownFieldHandler.onWrite : u)(this.typeName, message, writer);

proto/teleport/web/teleterm/ptyhost/v1/pty_host_service.proto

@@ -93,7 +93,11 @@ message PtyEventOpen {}
 message PtyEventExit {
   uint32 exit_code = 1;
   optional uint32 signal = 2;
-  string last_input = 3;
+
+  reserved 3;
+  reserved "last_input";
+
+  bool last_input_was_ctrl_d = 4;
 }
 
 // PtyEventStartError is sent by the PTY process when the shared process fails to start it.

web/packages/teleterm/src/services/pty/ptyHost/ptyEventsStreamHandler.ts

@@ -110,14 +110,19 @@ export class PtyEventsStreamHandler {
     callback: (reason: {
       exitCode: number;
       signal?: number;
-      lastInput: string;
+      lastInputWasCtrlD: boolean;
     }) => void
   ): RemoveListenerFunction {
     return this.addDataListenerAndReturnRemovalFunction(
       (event: ManagePtyProcessResponse) => {
         if (ptyEventOneOfIsExit(event.event)) {
-          this.logger.info('On exit', event.event.exit);
-          callback(event.event.exit);
+          const exitEvent = event.event.exit;
+          this.logger.info('On exit', {
+            lastInputWasCtrlD: exitEvent.lastInputWasCtrlD,
+            exitCode: exitEvent.exitCode,
+            signal: exitEvent.signal,
+          });
+          callback(exitEvent);
         }
       }
     );

web/packages/teleterm/src/services/pty/ptyHost/ptyProcess.ts

@@ -65,7 +65,7 @@ export function createPtyProcess(
       callback: (reason: {
         exitCode: number;
         signal?: number;
-        lastInput: string;
+        lastInputWasCtrlD: boolean;
       }) => void
     ) {
       return stream.onExit(callback);

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

@@ -54,7 +54,7 @@ describe('PtyProcess', () => {
   });
 
   if (process.platform !== 'win32') {
-    test('including the last input in the exit event', async () => {
+    test('reports whether the last input was Ctrl+D in the exit event', async () => {
       const pty = new PtyProcess({
         path: 'sh',
         env: {},
@@ -73,7 +73,7 @@ describe('PtyProcess', () => {
         tick: 10,
       });
       expect(listener).toHaveBeenCalledWith(
-        expect.objectContaining({ lastInput: '\x04' })
+        expect.objectContaining({ lastInputWasCtrlD: true })
       );
     });
   }

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

@@ -16,7 +16,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-import { exec } from 'node:child_process';
+import { execFile } from 'node:child_process';
 import { EventEmitter } from 'node:events';
 import { readlink } from 'node:fs';
 import { promisify } from 'node:util';
@@ -42,7 +42,7 @@ export class PtyProcess extends EventEmitter implements IPtyProcess {
   private _logger: Logger;
   private _status: Status = 'not_initialized';
   private _disposed = false;
-  private _lastInput = '';
+  private _lastInputWasCtrlD = false;
 
   constructor(private options: PtyProcessOptions & { ptyId: string }) {
     super();
@@ -116,7 +116,7 @@ export class PtyProcess extends EventEmitter implements IPtyProcess {
       return;
     }
 
-    this._lastInput = data;
+    this._lastInputWasCtrlD = data === '\x04'; // Ctrl+D
     this._process.write(data);
   }
 
@@ -187,7 +187,11 @@ export class PtyProcess extends EventEmitter implements IPtyProcess {
   }
 
   onExit(
-    cb: (ev: { exitCode: number; signal?: number; lastInput: string }) => void
+    cb: (ev: {
+      exitCode: number;
+      signal?: number;
+      lastInputWasCtrlD: boolean;
+    }) => void
   ) {
     return this.addListenerAndReturnRemovalFunction(TermEventEnum.Exit, cb);
   }
@@ -233,7 +237,10 @@ export class PtyProcess extends EventEmitter implements IPtyProcess {
   }
 
   private _handleExit(e: { exitCode: number; signal?: number }) {
-    this.emit(TermEventEnum.Exit, { ...e, lastInput: this._lastInput });
+    this.emit(TermEventEnum.Exit, {
+      ...e,
+      lastInputWasCtrlD: this._lastInputWasCtrlD,
+    });
     this._logger.info(`pty has been terminated with exit code: ${e.exitCode}`);
     this._setStatus('terminated');
   }
@@ -276,12 +283,20 @@ export enum TermEventEnum {
 async function getWorkingDirectory(pid: number): Promise<string> {
   switch (process.platform) {
     case 'darwin':
-      const asyncExec = promisify(exec);
+      const asyncExec = promisify(execFile);
       // -a: join using AND instead of OR for the -p and -d options
       // -p: PID
       // -d: only include the file descriptor, cwd
       // -F: fields to output (the n character outputs 3 things, the last one is cwd)
-      const { stdout } = await asyncExec(`lsof -a -p ${pid} -d cwd -F n`);
+      const { stdout } = await asyncExec('lsof', [
+        '-a',
+        '-p',
+        String(pid),
+        '-d',
+        'cwd',
+        '-F',
+        'n',
+      ]);
       return stdout.split('\n').filter(Boolean).reverse()[0].substring(1);
     case 'linux':
       const asyncReadlink = promisify(readlink);

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

@@ -45,7 +45,11 @@ export type IPtyProcess = {
   onOpen(cb: () => void): RemoveListenerFunction;
   onStartError(cb: (message: string) => void): RemoveListenerFunction;
   onExit(
-    cb: (ev: { exitCode: number; signal?: number; lastInput: string }) => void
+    cb: (ev: {
+      exitCode: number;
+      signal?: number;
+      lastInputWasCtrlD: boolean;
+    }) => void
   ): RemoveListenerFunction;
 };
 

web/packages/teleterm/src/ui/DocumentTerminal/useDocumentTerminal.ts

@@ -211,7 +211,7 @@ async function setUpPtyProcess(
     // We also have to account for Ctrl+D, as executing it makes the shell exit with the last
     // reported exit code. If we depended on the exit code alone, it'd mean that the terminal tab
     // wouldn't close if Ctrl+D followed a command that failed, say cd to a nonexistent directory.
-    if (event.exitCode === 0 || event.lastInput === /* Ctrl+D */ '\x04') {
+    if (event.exitCode === 0 || event.lastInputWasCtrlD) {
       documentsService.close(doc.uri);
     }
   });