Draft
Conversation
Adds a new ISSHRemoteAgentHostService that automates connecting to a remote machine via SSH, installing the VS Code CLI, starting 'code agent-host', and forwarding the agent host port back through the SSH tunnel. - New service interface and types in common/sshRemoteAgentHost.ts - Full implementation using ssh2 in electron-browser/ with dynamic imports to respect layering rules - Multi-step quick input flow for SSH connection details integrated into the remote agent host picker - 'Connect via SSH' command registered in contributions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an SSH-based bootstrap flow to the Sessions app’s remote agent host feature, enabling users to connect to a remote machine via SSH, automatically install/start code agent-host, and tunnel the connection back locally.
Changes:
- Introduces
ISSHRemoteAgentHostService(+ implementation) to manage SSH bootstrap, CLI install, remote agent-host startup, and local port forwarding. - Extends the Sessions remote agent host picker with a “Connect via SSH...” option and adds a dedicated
workbench.action.sessions.connectViaSSHcommand. - Adds
ssh2(+ typings) dependencies to support SSH connectivity.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vs/sessions/sessions.desktop.main.ts | Wires the SSH remote agent host service into the Sessions desktop entrypoint. |
| src/vs/sessions/contrib/remoteAgentHost/browser/remoteAgentHostPicker.ts | Adds SSH option to the picker and implements a multi-step quick input SSH connection flow. |
| src/vs/sessions/contrib/remoteAgentHost/browser/remoteAgentHost.contribution.ts | Registers a standalone “Connect via SSH” command that reuses the picker flow. |
| src/vs/platform/agentHost/electron-browser/sshRemoteAgentHostServiceImpl.ts | Implements SSH bootstrap: connect, detect platform, install CLI, start agent-host, forward port, register with remote host service. |
| src/vs/platform/agentHost/electron-browser/sshRemoteAgentHostService.ts | Registers the SSH remote agent host service singleton. |
| src/vs/platform/agentHost/common/sshRemoteAgentHost.ts | Defines the shared service contract/types for SSH remote agent host connections. |
| package.json | Adds ssh2 and @types/ssh2. |
| package-lock.json | Locks transitive dependencies for ssh2 and typings. |
Comments suppressed due to low confidence (5)
src/vs/platform/agentHost/electron-browser/sshRemoteAgentHostServiceImpl.ts:407
fs.readFileSync(config.privateKeyPath)will not expand~, but the UI flow suggests~/.ssh/id_rsaas the default value. This will fail on most platforms unless the user manually expands it. Consider expanding a leading~to the user home directory (or switch the UI to use a file picker / require an absolute path).
case SSHAuthMethod.KeyFile:
if (config.privateKeyPath) {
connectConfig.privateKey = fs.readFileSync(config.privateKeyPath);
}
src/vs/platform/agentHost/electron-browser/sshRemoteAgentHostServiceImpl.ts:403
- For
SSHAuthMethod.Agent, the code unconditionally setsconnectConfig.agent = process.env['SSH_AUTH_SOCK']. If no agent is running /SSH_AUTH_SOCKis unset (common on Windows or when the agent isn’t configured), this fails with a low-level ssh2 error. Consider detecting this case up front and throwing a clearer, user-actionable error (or falling back to prompting for a different auth method).
switch (config.authMethod) {
case SSHAuthMethod.Agent:
connectConfig.agent = process.env['SSH_AUTH_SOCK'];
break;
src/vs/platform/agentHost/electron-browser/sshRemoteAgentHostServiceImpl.ts:369
disconnect(host)matches connections viakey.includes(host), which can disconnect the wrong connection when host strings overlap (e.g.hostmatching part ofotherhost, or passing22). Prefer an exact match againstconn.config.host, the fullconnectionKey, orconn.localAddress(with explicit parsing if you want to accept multiple input formats).
async disconnect(host: string): Promise<void> {
for (const [key, conn] of this._connections) {
if (key.includes(host) || conn.localAddress === host) {
conn.dispose();
return;
src/vs/platform/agentHost/electron-browser/sshRemoteAgentHostServiceImpl.ts:427
- The CLI download quality is hardcoded to
'stable'. In insiders/exploration builds this may install a mismatched CLI on the remote, which can be confusing and may affect compatibility/debugging. Consider deriving the quality fromIProductService(or equivalent) so it matches the running build’s channel.
this._logService.info(`${LOG_PREFIX} Installing VS Code CLI on remote...`);
const quality = 'stable';
const url = buildCLIDownloadUrl(platform.os, platform.arch, quality);
src/vs/platform/agentHost/electron-browser/sshRemoteAgentHostServiceImpl.ts:350
- If
addRemoteAgentHost(...)throws (e.g. feature flag disabled, config write failure), the code has already transferred ownership toSSHAgentHostConnectionand inserted it into_connections, but the catch block only disposesstore. That will leak the SSH session/forwarder and leave a stale entry in_connections. Consider wrapping the registration step in its own try/catch that disposesconnand removes the map entry on failure, or delayingstore.dispose()/_connections.set(...)until after registration succeeds.
store.dispose(); // ownership transferred to SSHAgentHostConnection
// 7. Register with IRemoteAgentHostService
this._logService.info(`${LOG_PREFIX} Registering remote agent host at ${localAddress}`);
await this._remoteAgentHostService.addRemoteAgentHost({
Comment on lines
+30
to
+34
| /** Path to the private key file (when {@link authMethod} is KeyFile). */ | ||
| readonly privateKeyPath?: string; | ||
| /** Password string (when {@link authMethod} is Password). */ | ||
| readonly password?: string; | ||
| /** Display name for this connection. */ |
There was a problem hiding this comment.
password is part of ISSHAgentHostConfig, and connection objects expose that config. This makes plaintext credentials retrievable by any consumer of the service/connection list and keeps them in memory longer than needed. Consider keeping secrets out of the exposed config (e.g., omit password from the connection’s config, or store auth material separately and clear it after connect).
Comment on lines
+155
to
+159
| stream.stderr.on('data', (data: Buffer) => { | ||
| const text = data.toString(); | ||
| stderrBuf += text; | ||
| logService.trace(`${LOG_PREFIX} remote stderr: ${text.trimEnd()}`); | ||
|
|
There was a problem hiding this comment.
The remote agent host prints the full WebSocket URL including ?tkn=... to stderr. Logging raw stderr chunks at trace level will leak the connection token into logs. Consider redacting the token (e.g., replace it with ***) before calling logService.trace / including it in error messages.
Comment on lines
+285
to
+307
| // Step 1: SSH host | ||
| const hostInput = await quickInputService.input({ | ||
| title: localize('sshHostTitle', "Connect via SSH"), | ||
| prompt: localize('sshHostPrompt', "Enter the SSH host (e.g. user@hostname or hostname)."), | ||
| placeHolder: 'user@myserver.example.com', | ||
| ignoreFocusLost: true, | ||
| validateInput: async value => value.trim() ? undefined : localize('sshHostEmpty', "Enter an SSH host."), | ||
| }); | ||
| if (!hostInput) { | ||
| return undefined; | ||
| } | ||
|
|
||
| // Parse user@host format | ||
| const trimmed = hostInput.trim(); | ||
| let username: string; | ||
| let host: string; | ||
| const atIndex = trimmed.indexOf('@'); | ||
| if (atIndex !== -1) { | ||
| username = trimmed.substring(0, atIndex); | ||
| host = trimmed.substring(atIndex + 1); | ||
| } else { | ||
| host = trimmed; | ||
|
|
There was a problem hiding this comment.
The SSH host parsing only splits on '@' and treats the remainder as the host verbatim. Inputs like user@host:2222 (common SSH syntax) will end up with host set to host:2222 and port still defaulting to 22, which will fail DNS resolution. Consider parsing/validating an optional :port segment and also validating that both username/host are non-empty after splitting.
Suggested change
| // Step 1: SSH host | |
| const hostInput = await quickInputService.input({ | |
| title: localize('sshHostTitle', "Connect via SSH"), | |
| prompt: localize('sshHostPrompt', "Enter the SSH host (e.g. user@hostname or hostname)."), | |
| placeHolder: 'user@myserver.example.com', | |
| ignoreFocusLost: true, | |
| validateInput: async value => value.trim() ? undefined : localize('sshHostEmpty', "Enter an SSH host."), | |
| }); | |
| if (!hostInput) { | |
| return undefined; | |
| } | |
| // Parse user@host format | |
| const trimmed = hostInput.trim(); | |
| let username: string; | |
| let host: string; | |
| const atIndex = trimmed.indexOf('@'); | |
| if (atIndex !== -1) { | |
| username = trimmed.substring(0, atIndex); | |
| host = trimmed.substring(atIndex + 1); | |
| } else { | |
| host = trimmed; | |
| const validateSshHostInput = (value: string): string | undefined => { | |
| const trimmed = value.trim(); | |
| if (!trimmed) { | |
| return localize('sshHostEmpty', "Enter an SSH host."); | |
| } | |
| const atIndex = trimmed.indexOf('@'); | |
| if (atIndex === 0) { | |
| return localize('sshUsernameMissingInHost', "Enter a username before '@'."); | |
| } | |
| if (atIndex === trimmed.length - 1) { | |
| return localize('sshHostMissingInHost', "Enter a host name after '@'."); | |
| } | |
| let hostPart = atIndex !== -1 ? trimmed.substring(atIndex + 1) : trimmed; | |
| if (!hostPart) { | |
| return localize('sshHostMissingInHost', "Enter a host name after '@'."); | |
| } | |
| const colonIndex = hostPart.lastIndexOf(':'); | |
| if (colonIndex !== -1) { | |
| const hostName = hostPart.substring(0, colonIndex); | |
| const portStr = hostPart.substring(colonIndex + 1); | |
| if (!hostName) { | |
| return localize('sshHostMissingInHost', "Enter a host name after '@'."); | |
| } | |
| if (!portStr) { | |
| return localize('sshHostInvalidPort', "Enter a valid port number."); | |
| } | |
| const port = Number(portStr); | |
| if (!Number.isInteger(port) || port <= 0 || port > 65535) { | |
| return localize('sshHostInvalidPort', "Enter a valid port number."); | |
| } | |
| } | |
| return undefined; | |
| }; | |
| // Step 1: SSH host | |
| const hostInput = await quickInputService.input({ | |
| title: localize('sshHostTitle', "Connect via SSH"), | |
| prompt: localize('sshHostPrompt', "Enter the SSH host (e.g. user@hostname or hostname)."), | |
| placeHolder: 'user@myserver.example.com', | |
| ignoreFocusLost: true, | |
| validateInput: async value => validateSshHostInput(value), | |
| }); | |
| if (!hostInput) { | |
| return undefined; | |
| } | |
| // Parse user@host[:port] format | |
| const trimmed = hostInput.trim(); | |
| let username: string; | |
| let host: string; | |
| const atIndex = trimmed.indexOf('@'); | |
| let hostPart: string; | |
| if (atIndex !== -1) { | |
| username = trimmed.substring(0, atIndex); | |
| hostPart = trimmed.substring(atIndex + 1); | |
| } else { | |
| hostPart = trimmed; | |
| } | |
| const colonIndex = hostPart.lastIndexOf(':'); | |
| if (colonIndex !== -1) { | |
| host = hostPart.substring(0, colonIndex); | |
| } else { | |
| host = hostPart; | |
| } | |
| if (atIndex === -1) { |
Comment on lines
+242
to
+246
| this._register(toDisposable(() => { | ||
| remoteStream.close(); | ||
| localServer.close(); | ||
| sshClient.end(); | ||
| this._onDidClose.fire(); |
There was a problem hiding this comment.
When disposing a connection, _onDidClose.fire() is invoked here, but the SSH client's 'close'/'error' handlers below also fire the same event and call dispose(). This can result in onDidClose firing multiple times for a single disconnect. Consider guarding with a closed flag / Event.once, and ensuring the event is emitted from a single code path.
This issue also appears in the following locations of the same file:
- line 346
- line 365
- line 400
- line 404
- line 425
- Strip password/privateKeyPath from ISSHAgentHostConnection.config so secrets are not exposed to consumers after connect - Redact connection tokens (?tkn=...) in all log output and error messages to prevent credential leakage - Parse user@host:port format in SSH host input with proper validation for port range and missing components - Guard onDidClose with a closed flag to prevent double-fire when dispose and SSH close/error events overlap Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Connecting to a remote agent host currently requires the user to manually set up and start the agent host process on the remote machine, then provide the WebSocket address. This PR adds an SSH-based bootstrap flow that automates the entire process — SSH in, install the CLI, start
code agent-host, and tunnel the port back.Approach
Introduces
ISSHRemoteAgentHostServicewith an implementation that uses thessh2library to:uname -s/uname -mcode agent-host --port 0and parse thews://127.0.0.1:<port>?tkn=<token>outputIRemoteAgentHostServiceso the sessions app connects seamlesslyThe service lives in
electron-browser/to satisfy the sessions desktop entry point layering rules. Since this layer can't use static imports of Node.js builtins or npm packages, allssh2/net/fsusage goes throughawait import()with locally-defined minimal interfaces cast at the boundary.UI Integration
workbench.action.sessions.connectViaSSHcommand is registeredNotable decisions
electron-browserlayer forbids static imports ofssh2,net, andfs. The implementation defines minimal typed interfaces locally and casts at theawait import()boundary. This keeps the layer checker happy while providing type safety within the module.~/.vscode-cli/on the remote, matching the standard CLI install path.