add ssh agent forwarding support (ForwardAgent / ssh:forwardagent)#3421
add ssh agent forwarding support (ForwardAgent / ssh:forwardagent)#3421cghamburg wants to merge 2 commits into
Conversation
Wave already uses the local ssh agent for authentication but never forwarded it to the remote, so SSH_AUTH_SOCK stayed empty in remote shells. This adds the ForwardAgent keyword (parsed from ~/.ssh/config) and the ssh:forwardagent internal config key. When enabled, the agent connection is registered on the ssh client after connect and forwarding is requested for each interactive shell session. Fixes wavetermdev#2718 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review follow-ups: forwarding no longer reuses the auth-time agent connection (agent.ForwardToAgent). Instead each forwarded channel dials the agent socket freshly, matching openssh behavior. This makes forwarding survive agent restarts (e.g. 1password relock) and decouples it from IdentitiesOnly, which only restricts auth keys in openssh and must not disable forwarding. Also mark ForwardAgent as (partial) in the docs since socket paths / env var names are not accepted as values. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
WalkthroughThis PR adds support for SSH agent forwarding. It introduces a new Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.1)frontend/types/gotypes.d.tsFile contains syntax errors that prevent linting: Line 1880: Expected a property, or a signature but instead found '#'.; Line 1880: Expected an expression, or an assignment but instead found ':'.; Line 1880: Expected an expression but instead found ']'.; Line 2193: Expected a statement but instead found '}'. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/remote/sshclient.go (1)
842-901: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftConsider adding test coverage for the agent forwarding implementation.
The new
setupAgentForwardingandforwardAgentChannelfunctions implement critical SSH agent forwarding logic with multiple code paths (agent dial failure, channel accept failure, bidirectional copy, half-close semantics, Windows fallback). No tests are included in this PR.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/remote/sshclient.go` around lines 842 - 901, Add test coverage for the new SSH agent forwarding flow in setupAgentForwarding and forwardAgentChannel. Cover the key paths: successful setup, dialIdentityAgent failure, already-enabled forwarding, channel acceptance, and bidirectional copy behavior including half-close handling and the CloseWrite fallback. Use tests that exercise the auth-agent@openssh.com channel handling so the forwarding behavior stays verified across restarts and platform differences.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@pkg/remote/sshclient.go`:
- Around line 842-901: Add test coverage for the new SSH agent forwarding flow
in setupAgentForwarding and forwardAgentChannel. Cover the key paths: successful
setup, dialIdentityAgent failure, already-enabled forwarding, channel
acceptance, and bidirectional copy behavior including half-close handling and
the CloseWrite fallback. Use tests that exercise the auth-agent@openssh.com
channel handling so the forwarding behavior stays verified across restarts and
platform differences.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 28a427e6-8bc2-442a-b7de-6bd642d67d8f
📒 Files selected for processing (7)
docs/docs/connections.mdxfrontend/types/gotypes.d.tspkg/remote/conncontroller/conncontroller.gopkg/remote/sshclient.gopkg/shellexec/shellexec.gopkg/wconfig/settingsconfig.goschema/connections.json
Fixes #2718
What
Adds SSH agent forwarding to Wave's built-in connections. Wave already uses the local ssh agent for authentication but never forwarded it, so
SSH_AUTH_SOCKstayed empty in remote shells (see #2718).ForwardAgentis now parsed from~/.ssh/config(acceptsyes/no)ssh:forwardagent(overrides ssh config, works withconn:ignoresshconfig)[conndebug])How
Instead of
x/crypto'sagent.ForwardToAgent(which serves all forwarded channels over the single agent connection opened at auth time), the client registers its ownauth-agent@openssh.comchannel handler and dials the agent socket freshly per channel — matching openssh behavior. This way:IdentitiesOnly, which in openssh only restricts auth keys and does not disable forwarding\\.\pipe\openssh-ssh-agent) via the existingdialIdentityAgentLike openssh, the agent is only forwarded to the final destination, never to
ProxyJumphops.Partial implementation notes
ForwardAgentonly acceptsyes/no; an explicit socket path or env var name (accepted by openssh >= 7.3) behaves asno— documented as "(partial)" in the keyword table, consistent withAddKeysToAgentterm:durable) run their shells under the remote connserver rather than an sshd session, so sshd never setsSSH_AUTH_SOCKthere. Supporting that would need Wave to serve a stable agent socket on the remote itself (listen via the ssh connection at a fixed path + re-listen on reconnect) — left as a follow-upTesting
Verified manually (macOS host, Linux remote, RSA key in agent): with
ForwardAgent yesin ssh config, a Wave terminal block on the connection getsSSH_AUTH_SOCKset,ssh-add -llists the local key, and an onwardsshhop from the remote authenticates via the forwarded agent. Without the setting, behavior is unchanged.🤖 Generated with Claude Code