🤖 feat: add SSH2 runtime using ssh2 library #1782
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4125fbd5b3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
ethanndickson
force-pushed
the
ethan/ssh2-runtime
branch
from
0f2fc6d to
16004da
Compare
|
@codex review |
ethanndickson
force-pushed
the
ethan/ssh2-runtime
branch
from
43ba4d8 to
ded4137
Compare
|
@codex review The previous comment about identity fallback is already addressed in the code:
This matches the behavior described in the concern - users without an agent but with default key files will have those keys tried automatically. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ded4137ab0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Replace OpenSSH ControlMaster-based SSH runtime with pure JavaScript ssh2 implementation to support Windows and Coder's ProxyCommand. Key changes: - SSH2Runtime: extends SSHRuntime, overrides spawnRemoteProcess() to use ssh2 library's client.exec() instead of spawning system ssh command - SSH2ConnectionPool: manages persistent connections with health tracking, exponential backoff, and singleflighting concurrent connection attempts - sshConfigParser: parses ~/.ssh/config using ssh-config library, handles Match host ... !exec blocks for Coder's ProxyCommand configuration - CoderSSH2Runtime: mixin pattern produces Coder-aware SSH2 runtime - PTY support: SSH2Pty class wraps ClientChannel for terminal sessions - Path quoting: shellQuotePath() prevents shell injection in cd commands SSH2 is now the default on all platforms. The old OpenSSH CLI-based SSHRuntime remains available but unused by default.
Connections are now closed after 60 seconds of inactivity to match ControlMaster's ControlPersist=60 behavior. This prevents accumulating stale connections to many Coder workspaces. Each acquireConnection() call resets the idle timer, so actively used connections stay open indefinitely.
TODO(ethan): Revert this after SSH2 runtime testing
…raction - Replace SSH2Runtime with transport abstraction (OpenSSHTransport, SSH2Transport) - SSHRuntime now delegates to transport for exec and PTY operations - Unified PTY handling with PtyHandle interface and spawnPtyProcess utility - PTYService no longer needs to know transport-specific details - Narrowed transport config to SSHConnectionConfig for cleaner boundaries SSH2 auth improvements: - Agent-first fallback: try SSH agent first, then default keys on auth failure - Two-pass connection flow in SSH2ConnectionPool.connect() Bug fixes: - Fix bundle corruption: remove streamProcessToLogger stdout drain that consumed bundle data before pipe could read it - Fix PTY double-quoting: expandTildeForSSH already quotes, don't wrap again - PTY sessions fail fast with maxWaitMs: 0 (no 2-minute backoff waits) - Local abort-timeout for resolvePath/bundle upload (no remote timeout binary needed) --- _Generated with `mux` • Model: `anthropic:claude-opus-4-5` • Thinking: `high` • Cost: `$82.21`_
ethanndickson
force-pushed
the
ethan/ssh2-runtime
branch
from
9af944e to
40f2913
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40f2913cea
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 73a6bea899
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review Regarding the P2 badge about TL;DR: Our implementation correctly matches OpenSSH semantics. No code changes needed. OpenSSH defines:
So with: Running
Our code passes the resolved Sources: man7.org ssh_config(5), man.openbsd.org ssh_config(5) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 73a6bea899
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…d failure - sshConfigParser: Default %r expansion to local username when no User is specified, matching SSH connection behavior - SSH2Transport: Exit PTY session on cd failure (|| exit 1) to match OpenSSH transport behavior (cd ... && exec $SHELL -i) - Add test for %r expansion with no explicit User
|
@codex review Addressed both P2 items:
Added test coverage for the |
|
Codex Review: Didn't find any major issues. 👍 ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
Tested SSH2 on Windows & Linux, tested OpenSSH on Linux, merging.. |
…Docker - Revert temporary Windows build CI overrides (only run on merge-queue/main) - Revert Windows code signing to only run on main pushes - Replace --alias:jsonc-parser with --external:jsonc-parser in Dockerfile esbuild (matches Makefile ESBUILD_CLI_FLAGS approach, avoids UMD/ESM interop issues) - Copy jsonc-parser to runtime image alongside @lydell/node-pty
The ssh2 package (new SSH2 runtime) includes optional native .node addons (cpu-features, sshcrypto) that esbuild cannot bundle. Externalize ssh2 and copy its runtime dependencies into the Docker image: - ssh2, asn1, safer-buffer, bcrypt-pbkdf, tweetnacl Keep jsonc-parser as --alias (ESM version) since it bundles fine — only ssh2 needs externalization due to native addons. Also includes the Windows CI revert from the previous commit.
ethanndickson
force-pushed
the
ethan/ssh2-runtime
branch
from
39a613d to
6420fe5
Compare
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
SSH2 transport may have flaky behavior in CI. Use OpenSSH transport (ControlMaster) for CoderSSHRuntime tests unless explicitly testing SSH2.
ethanndickson
force-pushed
the
ethan/ssh2-runtime
branch
from
165cf06 to
cf58059
Compare
1 participant
Summary
Unify SSH transports under a single
SSHRuntimewith pluggable transport abstraction. Adds hidden config option to switch between OpenSSH (default on non-Windows) and SSH2 (always on Windows).Motivation
Match host ... !execblocks in SSH config for ProxyCommand; the ssh2 approach parses and honors these rules directlyKey Changes
Architecture Refactor
SSH2Runtime.ts: No longer a separate classSSHTransportinterface withOpenSSHTransportandSSH2TransportimplementationsSSHRuntime: Delegates to transport for exec and PTY operationsPtyHandleinterface: Common interface for PTY sessions across transports;PTYServiceno longer branches by transport typeNew Files
src/node/runtime/transports/- Transport abstraction layersrc/node/runtime/ptyHandle.ts- Unified PTY interfacesrc/node/runtime/ptySpawn.ts- Shared PTY spawn utilitysrc/node/runtime/shellQuote.ts- Shell quoting for Docker pathsSSH2 Auth Improvements
~/.ssh/id_rsa,id_ed25519, etc.) on auth failureSSH2ConnectionPool.connect()Bug Fixes
streamProcessToLoggerstdout drain that consumed git bundle data before pipe could read itexpandTildeForSSHalready quotes paths; don't wrap again withshellQuotePathmaxWaitMs: 0to avoid 2-minute backoff waitsresolvePathand bundle upload use local abort-timeout (no remotetimeoutbinary needed)Config Option
useSSH2Transportboolean to~/.mux/config.json"useSSH2Transport": trueto use SSH2Testing
make static-checkpassesGenerated with
mux• Model:anthropic:claude-opus-4-5• Thinking:high• Cost:$52.40