Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v16] Enable ConPTY by default in terminals on Windows #44742

Merged
merged 11 commits into from
Jul 30, 2024
1 change: 1 addition & 0 deletions docs/cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -953,6 +953,7 @@
"winadj",
"windowsaccountname",
"windowsdesktop",
"winpty",
"winscp",
"winserver",
"workgroups",
Expand Down
2 changes: 2 additions & 0 deletions docs/pages/connect-your-client/teleport-connect.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,7 @@ Below is the list of the supported config properties.
| `theme` | `system` | Color theme for the app. Available modes: `light`, `dark`, `system`. |
| `terminal.fontFamily` | `Menlo, Monaco, monospace` on macOS<br/>`Consolas, monospace` on Windows<br/>`'Droid Sans Mono', monospace` on Linux | Font family for the terminal. |
| `terminal.fontSize` | 15 | Font size for the terminal. |
| `terminal.windowsBackend` | `auto` | `auto` uses modern [ConPTY](https://devblogs.microsoft.com/commandline/windows-command-line-introducing-the-windows-pseudo-console-conpty/) system if available, which requires Windows 10 (19H1) or above. Set to `winpty` to use winpty even if ConPTY is available. |
| `usageReporting.enabled` | `false` | Enables collecting anonymous usage data (see [Telemetry](#telemetry)). |
| `keymap.tab1` - `keymap.tab9` | `Command+1` - `Command+9` on macOS <br/> `Ctrl+1` - `Ctrl+9` on Windows<br/>`Alt+1` - `Alt+9` on Linux | Shortcut to open tab 1–9. |
| `keymap.closeTab` | `Command+W` on macOS<br/>`Ctrl+Shift+W` on Windows/Linux | Shortcut to close a tab. |
Expand All @@ -458,6 +459,7 @@ Below is the list of the supported config properties.
| `keymap.openProfiles` | `Command+I` on macOS<br/>`Ctrl+Shift+I` on Windows/Linux | Shortcut to open the profile selector. |
| `keymap.openSearchBar` | `Command+K` on macOS<br/>`Ctrl+Shift+K` on Windows/Linux | Shortcut to open the search bar. |
| `headless.skipConfirm` | false | Skips the confirmation prompt for Headless WebAuthn approval and instead prompts for WebAuthn immediately. |
| `ssh.noResume` | false | Disables SSH connection resumption. |

<Admonition
type="note"
Expand Down
7 changes: 6 additions & 1 deletion web/packages/teleterm/src/mainProcess/mainProcess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import {
ChildProcessAddresses,
MainProcessIpc,
RendererIpc,
TERMINATE_MESSAGE,
} from 'teleterm/mainProcess/types';
import { getAssetPath } from 'teleterm/mainProcess/runtimeSettings';
import { RootClusterUri } from 'teleterm/ui/uri';
Expand Down Expand Up @@ -143,7 +144,11 @@ export default class MainProcess {
terminateWithTimeout(this.tshdProcess, 10_000, () => {
this.gracefullyKillTshdProcess();
}),
terminateWithTimeout(this.sharedProcess),
terminateWithTimeout(this.sharedProcess, 5_000, process =>
// process.kill doesn't allow running a cleanup code in the child process
// on Windows
process.send(TERMINATE_MESSAGE)
),
this.agentRunner.killAll(),
]);
}
Expand Down
9 changes: 9 additions & 0 deletions web/packages/teleterm/src/mainProcess/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -276,3 +276,12 @@ export enum MainProcessIpc {
export enum WindowsManagerIpc {
SignalUserInterfaceReadiness = 'windows-manager-signal-user-interface-readiness',
}

/**
* A custom message to gracefully quit a process.
* It is sent to the child process with `process.send`.
*
* We need this because `process.kill('SIGTERM')` doesn't work on Windows,
* so we couldn't run any cleanup logic.
*/
export const TERMINATE_MESSAGE = 'TERMINATE_MESSAGE';
9 changes: 8 additions & 1 deletion web/packages/teleterm/src/preload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,14 @@ async function getElectronGlobals(): Promise<ElectronGlobals> {
credentials.shared,
runtimeSettings,
{
noResume: mainProcessClient.configService.get('ssh.noResume').value,
ssh: {
noResume: mainProcessClient.configService.get('ssh.noResume').value,
},
terminal: {
windowsBackend: mainProcessClient.configService.get(
'terminal.windowsBackend'
).value,
},
}
);
const {
Expand Down
9 changes: 9 additions & 0 deletions web/packages/teleterm/src/services/config/appConfigSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ import { Platform } from 'teleterm/mainProcess/types';

import { createKeyboardShortcutSchema } from './keyboardShortcutSchema';

// When adding a new config property, add it to the docs too
// (teleport-connect.mdx#configuration).

export type AppConfigSchema = ReturnType<typeof createAppConfigSchema>;
export type AppConfig = z.infer<AppConfigSchema>;

Expand Down Expand Up @@ -54,6 +57,12 @@ export const createAppConfigSchema = (platform: Platform) => {
.max(256)
.default(15)
.describe('Font size for the terminal.'),
'terminal.windowsBackend': z
.enum(['auto', 'winpty'])
.default('auto')
.describe(
'`auto` uses modern ConPTY system if available, which requires Windows 10 (19H1) or above. Set to `winpty` to use winpty even if ConPTY is available.'
),
'usageReporting.enabled': z
.boolean()
.default(false)
Expand Down
5 changes: 4 additions & 1 deletion web/packages/teleterm/src/services/pty/fixtures/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { IPtyProcess } from 'teleterm/sharedProcess/ptyHost';
import {
PtyProcessCreationStatus,
PtyServiceClient,
WindowsPty,
} from 'teleterm/services/pty';

export class MockPtyProcess implements IPtyProcess {
Expand All @@ -29,7 +30,7 @@ export class MockPtyProcess implements IPtyProcess {

resize() {}

dispose() {}
async dispose() {}

onData() {
return () => {};
Expand Down Expand Up @@ -64,10 +65,12 @@ export class MockPtyServiceClient implements PtyServiceClient {
createPtyProcess(): Promise<{
process: IPtyProcess;
creationStatus: PtyProcessCreationStatus;
windowsPty: WindowsPty;
}> {
return Promise.resolve({
process: new MockPtyProcess(),
creationStatus: PtyProcessCreationStatus.Ok,
windowsPty: undefined,
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ describe('getPtyProcessOptions', () => {

const { env } = getPtyProcessOptions(
makeRuntimeSettings(),
{ noResume: false },
{ ssh: { noResume: false }, windowsPty: { useConpty: true } },
cmd,
processEnv
);
Expand Down Expand Up @@ -76,7 +76,7 @@ describe('getPtyProcessOptions', () => {

const { env } = getPtyProcessOptions(
makeRuntimeSettings(),
{ noResume: false },
{ ssh: { noResume: false }, windowsPty: { useConpty: true } },
cmd,
processEnv
);
Expand All @@ -103,7 +103,7 @@ describe('getPtyProcessOptions', () => {

const { args } = getPtyProcessOptions(
makeRuntimeSettings(),
{ noResume: true },
{ ssh: { noResume: true }, windowsPty: { useConpty: true } },
cmd,
processEnv
);
Expand Down
22 changes: 17 additions & 5 deletions web/packages/teleterm/src/services/pty/ptyHost/buildPtyOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,24 @@ import { assertUnreachable } from 'teleterm/ui/utils';
import {
PtyCommand,
PtyProcessCreationStatus,
SshOptions,
TshKubeLoginCommand,
SshOptions,
WindowsPty,
} from '../types';

import {
resolveShellEnvCached,
ResolveShellEnvTimeoutError,
} from './resolveShellEnv';

type PtyOptions = {
ssh: SshOptions;
windowsPty: Pick<WindowsPty, 'useConpty'>;
};

export async function buildPtyOptions(
settings: RuntimeSettings,
sshOptions: SshOptions,
options: PtyOptions,
cmd: PtyCommand
): Promise<{
processOptions: PtyProcessOptions;
Expand Down Expand Up @@ -68,7 +74,7 @@ export async function buildPtyOptions(
return {
processOptions: getPtyProcessOptions(
settings,
sshOptions,
options,
cmd,
combinedEnv
),
Expand All @@ -79,10 +85,12 @@ export async function buildPtyOptions(

export function getPtyProcessOptions(
settings: RuntimeSettings,
sshOptions: SshOptions,
options: PtyOptions,
cmd: PtyCommand,
env: typeof process.env
): PtyProcessOptions {
const useConpty = options.windowsPty?.useConpty;

switch (cmd.kind) {
case 'pty.shell': {
// Teleport Connect bundles a tsh binary, but the user might have one already on their system.
Expand All @@ -104,6 +112,7 @@ export function getPtyProcessOptions(
cwd: cmd.cwd,
env: { ...env, ...cmd.env },
initMessage: cmd.initMessage,
useConpty,
};
}

Expand All @@ -129,6 +138,7 @@ export function getPtyProcessOptions(
path: settings.defaultShell,
args: isWindows ? powershellCommandArgs : bashCommandArgs,
env: { ...env, KUBECONFIG: getKubeConfigFilePath(cmd, settings) },
useConpty,
};
}

Expand All @@ -140,7 +150,7 @@ export function getPtyProcessOptions(
const args = [
`--proxy=${cmd.rootClusterId}`,
'ssh',
...(sshOptions.noResume ? ['--no-resume'] : []),
...(options.ssh.noResume ? ['--no-resume'] : []),
'--forward-agent',
loginHost,
];
Expand All @@ -149,6 +159,7 @@ export function getPtyProcessOptions(
path: settings.tshd.binaryPath,
args,
env,
useConpty,
};
}

Expand All @@ -159,6 +170,7 @@ export function getPtyProcessOptions(
path: cmd.path,
args: cmd.args,
env: { ...env, ...cmd.env },
useConpty,
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export function createPtyHostClient(
args: ptyOptions.args,
path: ptyOptions.path,
env: Struct.fromJson(ptyOptions.env),
useConpty: ptyOptions.useConpty,
});

if (ptyOptions.cwd) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export function createPtyProcess(
exchangeEventsStream.resize(columns, rows);
},

dispose(): void {
async dispose(): Promise<void> {
exchangeEventsStream.dispose();
},

Expand Down
61 changes: 61 additions & 0 deletions web/packages/teleterm/src/services/pty/ptyHost/windowsPty.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/**
* 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 { makeRuntimeSettings } from 'teleterm/mainProcess/fixtures/mocks';

import { getWindowsPty } from './windowsPty';

test.each([
{
name: 'uses conpty on supported Windows version',
platform: 'win32' as const,
osVersion: '10.0.22621',
terminalOptions: { windowsBackend: 'auto' as const },
expected: { useConpty: true, buildNumber: 22621 },
},
{
name: 'uses winpty on unsupported Windows version',
platform: 'win32' as const,
osVersion: '10.0.18308',
terminalOptions: { windowsBackend: 'auto' as const },
expected: { useConpty: false, buildNumber: 18308 },
},
{
name: 'uses winpty when Windows version is supported, but conpty is disabled in options',
platform: 'win32' as const,
osVersion: '10.0.22621',
terminalOptions: { windowsBackend: 'winpty' as const },
expected: { useConpty: false, buildNumber: 22621 },
},
{
name: 'undefined on non-Windows OS',
platform: 'darwin' as const,
osVersion: '23.5.0',
terminalOptions: { windowsBackend: 'auto' as const },
expected: undefined,
},
])('$name', ({ platform, osVersion, terminalOptions, expected }) => {
const pty = getWindowsPty(
makeRuntimeSettings({
platform,
osVersion,
}),
terminalOptions
);
expect(pty).toEqual(expected);
});
49 changes: 49 additions & 0 deletions web/packages/teleterm/src/services/pty/ptyHost/windowsPty.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/**
* 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 { RuntimeSettings } from 'teleterm/mainProcess/types';

import { TerminalOptions, WindowsPty } from '../types';

export const WIN_BUILD_STABLE_CONPTY = 18309;

export function getWindowsPty(
runtimeSettings: RuntimeSettings,
terminalOptions: TerminalOptions
): WindowsPty {
if (runtimeSettings.platform !== 'win32') {
return undefined;
}

const buildNumber = getWindowsBuildNumber(runtimeSettings.osVersion);
const useConpty =
terminalOptions.windowsBackend === 'auto' &&
buildNumber >= WIN_BUILD_STABLE_CONPTY;
return {
useConpty,
buildNumber,
};
}

function getWindowsBuildNumber(osVersion: string): number {
const parsedOsVersion = /(\d+)\.(\d+)\.(\d+)/g.exec(osVersion);
if (parsedOsVersion?.length === 4) {
return parseInt(parsedOsVersion[3]);
}
return 0;
}
Loading
Loading