From 2a8ebe88deedc6cbbdd0fabef10e173fcfd99c91 Mon Sep 17 00:00:00 2001 From: Zhihe Li Date: Wed, 20 Sep 2023 11:48:30 -0400 Subject: [PATCH] =?UTF-8?q?feat:=20=F0=9F=8E=B8=20Prevent=20automatically?= =?UTF-8?q?=20sending=20SSH=20command=20on=20windows=20(#1906)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ✅ Closes: ICU-11109 --- .../app/components/session/tabs/index.js | 19 ++++++++++++++----- ui/desktop/electron-app/src/ipc/handlers.js | 16 ++++++++++++++-- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/ui/desktop/app/components/session/tabs/index.js b/ui/desktop/app/components/session/tabs/index.js index 439b2c2c5..2c464f095 100644 --- a/ui/desktop/app/components/session/tabs/index.js +++ b/ui/desktop/app/components/session/tabs/index.js @@ -1,5 +1,6 @@ import Component from '@glimmer/component'; import { action } from '@ember/object'; +import { inject as service } from '@ember/service'; import { Terminal } from 'xterm'; import { FitAddon } from 'xterm-addon-fit'; import { v4 as uuidv4 } from 'uuid'; @@ -10,6 +11,10 @@ const terminalOptions = { }; export default class SessionTerminalTabsComponent extends Component { + // =services + + @service ipc; + // =attributes id; @@ -19,7 +24,7 @@ export default class SessionTerminalTabsComponent extends Component { // =actions @action - openTerminal() { + async openTerminal() { // Don't re-initialize the terminal if it's already been opened if (this.terminal) { return; @@ -39,8 +44,11 @@ export default class SessionTerminalTabsComponent extends Component { this.id = uuidv4(); this.#setupTerminal(fitAddon, xterm); + const { isWindows } = await this.ipc.invoke('checkOS'); const { model } = this.args; - if (model.target?.isSSH) { + + // Don't send the command on windows as most users won't have an openSSH client installed + if (model.target?.isSSH && !isWindows) { const { proxy_address, proxy_port } = model; // Send an SSH command immediately @@ -69,13 +77,14 @@ export default class SessionTerminalTabsComponent extends Component { // Terminal is exposed by contextBridge within the preload script window.terminal.create({ id: this.id, cols: xterm.cols, rows: xterm.rows }); xterm.onData((data) => window.terminal.send(data, this.id)); + + // Save the handler to cleanup the listener on the renderer process later this.removeListener = window.terminal.receive((event, value) => { xterm.write(value); }, this.id); - // Handle resizing terminal windows. We debounce the resizing as we - // don't want to resize xterm and the pty process before the previous - // one has finished + // Handle resizing terminal windows. We debounce the resizing as we don't want + // to resize xterm and the pty process before the previous one has finished const debouncedFit = debounce(() => { fitAddon.fit(); }, 150); diff --git a/ui/desktop/electron-app/src/ipc/handlers.js b/ui/desktop/electron-app/src/ipc/handlers.js index e2660fe65..68a729be2 100644 --- a/ui/desktop/electron-app/src/ipc/handlers.js +++ b/ui/desktop/electron-app/src/ipc/handlers.js @@ -10,7 +10,7 @@ const boundaryCli = require('../cli/index.js'); const sessionManager = require('../services/session-manager.js'); const runtimeSettings = require('../services/runtime-settings.js'); const sanitizer = require('../utils/sanitizer.js'); -const { isMac } = require('../helpers/platform.js'); +const { isLinux, isMac, isWindows } = require('../helpers/platform.js'); const os = require('node:os'); const pty = require('node-pty'); @@ -111,6 +111,15 @@ handle('toggleFullscreenWindow', () => { */ handle('closeWindow', () => app.quit()); +/** + * Return an object containing helper fields for determining what OS we're running on + */ +handle('checkOS', () => ({ + isLinux: isLinux(), + isMac: isMac(), + isWindows: isWindows(), +})); + /** * Handler to help create terminal windows. We don't use the helper `handle` method * as we need access to the event and don't need to be using `ipcMain.handle`. @@ -119,7 +128,9 @@ ipcMain.on('createTerminal', (event, payload) => { const { id, cols, rows } = payload; const { sender } = event; const terminalShell = - os.platform() === 'win32' ? 'powershell.exe' : process.env.SHELL || '/bin/bash'; + os.platform() === 'win32' + ? 'powershell.exe' + : process.env.SHELL || '/bin/bash'; const ptyProcess = pty.spawn(terminalShell, [], { name: 'xterm-color', cols, @@ -152,6 +163,7 @@ ipcMain.on('createTerminal', (event, payload) => { ipcMain.once(removeChannel, () => { // Just let the error bubble up since we can't do anything try { + // TODO: Should we be killing entire process tree in windows with its pid? ptyProcess.kill(); } finally { // Remove listeners