From f06d535d8d3aba404553ca82526d02bac9be4eb4 Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Wed, 27 May 2026 10:28:14 +0200 Subject: [PATCH] Connect: Do not leak last terminal input in logs (#67049) * Do not leak `lastInput` in logs * Security hardening: use `execFile` instead of `exec` --- .../ptyhost/v1/pty_host_service_pb.ts | 18 ++++++------ .../ptyhost/v1/pty_host_service.proto | 6 +++- .../pty/ptyHost/ptyEventsStreamHandler.ts | 11 +++++-- .../src/services/pty/ptyHost/ptyProcess.ts | 2 +- .../sharedProcess/ptyHost/ptyProcess.test.ts | 4 +-- .../src/sharedProcess/ptyHost/ptyProcess.ts | 29 ++++++++++++++----- .../src/sharedProcess/ptyHost/types.ts | 6 +++- .../DocumentTerminal/useDocumentTerminal.ts | 2 +- 8 files changed, 53 insertions(+), 25 deletions(-) diff --git a/gen/proto/ts/teleport/web/teleterm/ptyhost/v1/pty_host_service_pb.ts b/gen/proto/ts/teleport/web/teleterm/ptyhost/v1/pty_host_service_pb.ts index 2be7d35d1c9..7c0b5590daa 100644 --- a/gen/proto/ts/teleport/web/teleterm/ptyhost/v1/pty_host_service_pb.ts +++ b/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 { 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 { const message = globalThis.Object.create((this.messagePrototype!)); message.exitCode = 0; - message.lastInput = ""; + message.lastInputWasCtrlD = false; if (value !== undefined) reflectionMergePartial(this, message, value); return message; @@ -737,8 +737,8 @@ class PtyEventExit$Type extends MessageType { 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 { /* 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); diff --git a/proto/teleport/web/teleterm/ptyhost/v1/pty_host_service.proto b/proto/teleport/web/teleterm/ptyhost/v1/pty_host_service.proto index 5963539060e..b8697af9d17 100644 --- a/proto/teleport/web/teleterm/ptyhost/v1/pty_host_service.proto +++ b/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. diff --git a/web/packages/teleterm/src/services/pty/ptyHost/ptyEventsStreamHandler.ts b/web/packages/teleterm/src/services/pty/ptyHost/ptyEventsStreamHandler.ts index 4b56a71777f..ad12b35de72 100644 --- a/web/packages/teleterm/src/services/pty/ptyHost/ptyEventsStreamHandler.ts +++ b/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); } } ); diff --git a/web/packages/teleterm/src/services/pty/ptyHost/ptyProcess.ts b/web/packages/teleterm/src/services/pty/ptyHost/ptyProcess.ts index 24df852b6a0..b2bf3911b84 100644 --- a/web/packages/teleterm/src/services/pty/ptyHost/ptyProcess.ts +++ b/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); diff --git a/web/packages/teleterm/src/sharedProcess/ptyHost/ptyProcess.test.ts b/web/packages/teleterm/src/sharedProcess/ptyHost/ptyProcess.test.ts index 43c2065fc26..edec6a61b7b 100644 --- a/web/packages/teleterm/src/sharedProcess/ptyHost/ptyProcess.test.ts +++ b/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 }) ); }); } diff --git a/web/packages/teleterm/src/sharedProcess/ptyHost/ptyProcess.ts b/web/packages/teleterm/src/sharedProcess/ptyHost/ptyProcess.ts index 5f2835aa5ef..cef35cad70d 100644 --- a/web/packages/teleterm/src/sharedProcess/ptyHost/ptyProcess.ts +++ b/web/packages/teleterm/src/sharedProcess/ptyHost/ptyProcess.ts @@ -16,7 +16,7 @@ * along with this program. If not, see . */ -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 { 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); diff --git a/web/packages/teleterm/src/sharedProcess/ptyHost/types.ts b/web/packages/teleterm/src/sharedProcess/ptyHost/types.ts index 9a898f95395..b82eeee5f3b 100644 --- a/web/packages/teleterm/src/sharedProcess/ptyHost/types.ts +++ b/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; }; diff --git a/web/packages/teleterm/src/ui/DocumentTerminal/useDocumentTerminal.ts b/web/packages/teleterm/src/ui/DocumentTerminal/useDocumentTerminal.ts index 4b7aac0da8b..08173969c61 100644 --- a/web/packages/teleterm/src/ui/DocumentTerminal/useDocumentTerminal.ts +++ b/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); } });