fix: execute ProxyCommand transport (#293)

## Summary

- Execute resolved `ProxyCommand` values as managed shell child transports instead of silently opening a direct target socket.
- Match OpenSSH proxy selection semantics across `ProxyCommand`, `ProxyJump`, command-line `-J`, explicit `none`, and effective `%h`/`%k`/`%n` token identities.
- Report bounded proxy stderr and exit status as actionable connection errors, and reject unsupported `ProxyUseFdpass` before spawning a process.

## Implementation

- Add a duplex async transport backed by child stdin/stdout with bounded stderr draining, exit monitoring, deterministic shutdown, and kill-on-drop lifecycle handling.
- Expand `%%`, `%h`, `%k`, `%n`, `%p`, and `%r`, reject malformed or unsupported tokens, and preserve shell pipes and quoting from trusted SSH configuration.
- Preserve the first-obtained SSH configuration semantics introduced by the related proxy and host-key changes, with command-line `-J` taking precedence.
- Silence only the official but unimplemented `SecurityKeyProvider` option while retaining diagnostics for genuinely unknown keywords.
- Preserve raw command exit codes after stdout and stderr and keep draining the SSH channel when the local output receiver closes, fixing integration failures exposed by the real proxy transport.
- Update user and manual documentation to describe the supported behavior and the explicit `ProxyUseFdpass` limitation.

## Security and quality review

- Reviewed shell-boundary validation, token expansion, direct-connection avoidance, child cleanup, stderr memory bounds, failure mapping, host-key alias identity, and early receiver-drop behavior; no unresolved CRITICAL or HIGH findings remain.
- Proxy commands remain an explicitly trusted SSH configuration shell boundary; target-derived tokens retain existing host and username validation, while NUL and newline injection and unsupported expansion tokens are rejected.

## Validation

- `cargo fmt --all -- --check`
- `cargo test --lib proxy` (29 passed)
- `cargo test --lib resolve_effective_jump_hosts` (11 passed)
- `cargo test --lib stream_exit` (2 passed)
- `cargo test --lib nonzero_stream_completion_preserves_remote_status` (1 passed)
- `cargo test --lib closed_output_receiver_is_not_a_command_error` (1 passed)
- `cargo test --test ssh_compat_output_test deprecated_alias_is_silent_and_unknown_keyword_uses_log_file -- --exact` (1 passed)
- `cargo check --lib --tests`
- `cargo clippy --lib --tests -- -D warnings`
- `cargo build --bin bssh`
- `cargo test --bin bssh` (57 passed)
- `TEST_SSH_UNSAFE_PERMISSIONS=1 python3 tests/openssh-regress/run.py --bssh target/debug/bssh --test proxy-connect --jobs 1 --timeout 120` (PASS, 1/1)

Local CI-equivalent note: `cargo test --tests -- --skip integration_test` passed the library tests (1420 passed, 9 ignored, 13 filtered), the bssh binary tests (57 passed), and the subsequent keygen/server and independent test targets until `tests/integration_test.rs`; three unchanged localhost SSH-dependent cases failed in the local harness (`test_localhost_multiple_file_upload`, `test_parallel_execution_with_multiple_nodes`, and `test_download_with_unique_filenames`). The PR's exact-head GitHub checks are the authoritative CI verdict for those environment-dependent cases.

Closes #280

Author: inureyes

README.md

@@ -874,11 +874,12 @@ These options control SSH proxy connection behavior:
 
 | Option | Description | Example |
 |--------|-------------|---------|
-| **ProxyUseFdpass** | Pass connected file descriptor from ProxyCommand to ssh(1) instead of continuing execution (yes/no, default: no, OpenSSH 6.5+) | `ProxyUseFdpass yes` |
+| **ProxyCommand** | Run a shell command whose stdin/stdout carry the SSH transport; supports `%%`, `%h`, `%k`, `%n`, `%p`, and `%r` | `ProxyCommand nc %h %p` |
+| **ProxyUseFdpass** | Request descriptor passing from ProxyCommand (recognized but not supported by bssh) | `ProxyUseFdpass no` |
 
-**ProxyUseFdpass** optimizes ProxyCommand usage by eliminating an unnecessary lingering process and reducing I/O overhead. When enabled, the proxy command passes the established connection file descriptor directly to ssh and exits, rather than remaining active to relay data throughout the session. This is particularly useful with proxy commands like netcat that support file descriptor passing (nc -F).
+**ProxyCommand** uses the user's shell so OpenSSH-style quoting, redirection, and pipelines work. `ProxyCommand none` explicitly selects a direct connection. `ProxyCommand` and `ProxyJump` follow OpenSSH's first-obtained ssh_config rule, while command-line `-J` takes precedence over both.
 
-*Note: This option is currently parsed from SSH configuration files for compatibility but is not yet utilized in bssh's SSH client implementation, as proxy connections are not yet supported.*
+*Note: `ProxyUseFdpass yes` is rejected with an actionable error before the command starts. Use a streaming ProxyCommand without `-F`, or set `ProxyUseFdpass no`.*
 
 ### Command Execution and Automation Options
 
@@ -1007,20 +1008,13 @@ Host *.secure.prod.example.com
 #### Proxy Connection Optimization
 
 ```ssh-config
-# Optimized proxy connection with file descriptor passing
+# HTTP CONNECT proxy
 Host internal-server
-    ProxyCommand nc -X connect -x proxy.example.com:1080 -F %h %p
-    ProxyUseFdpass yes
+    ProxyCommand nc -X connect -x proxy.example.com:1080 %h %p
 
-# SOCKS proxy with netcat (reduces overhead)
+# SOCKS proxy with netcat
 Host *.internal.example.com
-    ProxyCommand nc -x socks.example.com:1080 -F %h %p
-    ProxyUseFdpass yes
-
-# Jump host with ProxyCommand and fd passing
-Host bastion-optimized
-    ProxyCommand ssh -W %h:%p jump.example.com
-    ProxyUseFdpass yes
+    ProxyCommand nc -x socks.example.com:1080 %h %p
 ```
 
 #### Command Execution and Automation

docs/man/bssh.1

@@ -639,27 +639,30 @@ Example:
 
 .SS Proxy Options
 .TP
-.B ProxyUseFdpass
-Specifies that ProxyCommand will pass a connected file descriptor back to ssh(1)
-instead of continuing to execute and relay data.
-This reduces overhead by avoiding an unnecessary lingering process and extra I/O operations.
-When enabled, the proxy command establishes a connection, passes the file descriptor
-to ssh via the fdpass mechanism, and exits. The ssh process then communicates directly
-with the connection, eliminating the intermediate proxy process.
-.br
-Default is no.
+.B ProxyCommand
+Executes a local command whose standard input and output carry the SSH transport.
+The command is run by the user's shell, preserving OpenSSH-style quoting,
+redirection, and pipelines.
 .br
-Introduced in OpenSSH 6.5 (January 2014).
+Supported tokens are %%, %h, %k, %n, %p, and %r.
 .br
-This is particularly useful with proxy commands like netcat that support file descriptor
-passing via the -F option (nc.openbsd).
+.I ProxyCommand none
+explicitly selects a direct connection. When ProxyCommand and ProxyJump both
+appear in ssh_config, the first obtained value is used; command-line -J takes
+precedence over both.
 .br
 Example:
-.I ProxyUseFdpass yes
+.I ProxyCommand nc %h %p
+
+.TP
+.B ProxyUseFdpass
+Requests that ProxyCommand pass a connected file descriptor instead of relaying
+the SSH transport over standard input and output.
 .br
-Note: This option is currently parsed from SSH configuration files for compatibility
-but is not yet utilized in bssh's SSH client implementation, as proxy connections
-are not yet supported.
+Default is no. bssh does not support descriptor passing and rejects
+.I ProxyUseFdpass yes
+with an actionable error before starting the proxy command. Use a streaming
+ProxyCommand without netcat's -F option instead.
 
 .SS Command Execution and Automation Options
 .TP
@@ -1048,15 +1051,13 @@ Host *.secure.prod.example.com
     PermitRemoteOpen localhost:8080
     PermitRemoteOpen db.internal:5432
 
-# Optimized proxy connection with file descriptor passing
+# HTTP CONNECT proxy
 Host internal-server
-    ProxyCommand nc -X connect -x proxy.example.com:1080 -F %h %p
-    ProxyUseFdpass yes
+    ProxyCommand nc -X connect -x proxy.example.com:1080 %h %p
 
-# SOCKS proxy with netcat (reduces overhead)
+# SOCKS proxy with netcat
 Host *.internal.example.com
-    ProxyCommand nc -x socks.example.com:1080 -F %h %p
-    ProxyUseFdpass yes
+    ProxyCommand nc -x socks.example.com:1080 %h %p
 
 # Development server with automatic file sync
 Host dev-server

src/app/dispatcher.rs

@@ -62,6 +62,8 @@ fn build_ssh_connection_config_resolver(
         .with_yaml_keepalive_max(ctx.config.get_server_alive_count_max(cluster_name))
         .with_cli_address_family(AddressFamily::from_flags(cli.ipv4, cli.ipv6))
         .with_cli_host_key_alias(cli.get_ssh_option("HostKeyAlias"))
+        .with_cli_proxy_jump(cli.jump_hosts.clone())
+        .with_yaml_proxy_jump(ctx.config.get_cluster_jump_host(cluster_name))
 }
 
 /// Decide whether `-S` (sudo-password) is meaningful for the given dispatch path.

src/app/initialization.rs

@@ -424,15 +424,15 @@ Host example.com
     #[test]
     fn test_determine_effective_jump_hosts_wildcard_pattern() {
         let ssh_config_content = r#"
-Host *.internal
-    ProxyJump gateway.company.com
-
 Host db.internal
     ProxyJump db-gateway.company.com
+
+Host *.internal
+    ProxyJump gateway.company.com
 "#;
         let ssh_config = SshConfig::parse(ssh_config_content).unwrap();
 
-        // Should match the most specific pattern
+        // OpenSSH uses the first value obtained, so put the exact host first.
         let result = determine_effective_jump_hosts(None, &ssh_config, "db.internal");
         assert_eq!(result, Some("db-gateway.company.com".to_string()));
 

src/app/nodes.rs

@@ -75,11 +75,10 @@ pub fn parse_node_with_ssh_config(node_str: &str, ssh_config: &SshConfig) -> Res
     };
     let effective_port = ssh_config.get_effective_port(raw_host, cli_port);
 
-    Ok(Node::new(
-        effective_hostname,
-        effective_port,
-        effective_user,
-    ))
+    Ok(
+        Node::new(effective_hostname, effective_port, effective_user)
+            .with_original_host(validated_host),
+    )
 }
 
 #[cfg(test)]
@@ -212,7 +211,8 @@ pub async fn resolve_nodes(
         let effective_port =
             ssh_config.get_effective_port(host, port.or_else(|| cli.get_effective_port()));
 
-        let node = Node::new(effective_hostname, effective_port, effective_user);
+        let node = Node::new(effective_hostname, effective_port, effective_user)
+            .with_original_host(host.to_string());
         nodes.push(node);
     } else if let Some(hosts) = &cli.hosts {
         // Parse hosts from CLI with hostlist expression expansion

src/commands/download.rs

@@ -104,7 +104,7 @@ pub async fn download_file(
             let node_dir = validated_destination.join(node.to_string());
             let ssh_connection_config = params
                 .ssh_connection_config_resolver
-                .resolve_for_host(&node.host);
+                .resolve_for_host(node.config_host());
 
             println!(
                 "\n{} {} {} {} {:?}",

src/commands/exec.rs

@@ -106,7 +106,7 @@ async fn execute_command_with_forwarding(params: ExecuteCommandParams<'_>) -> Re
     // listener side was already constrained when the spec was parsed.
     let ssh_connection_config = params
         .ssh_connection_config_resolver
-        .resolve_for_host(&node.host);
+        .resolve_for_host(node.config_host());
     let forwarding_config = ForwardingConfig {
         address_family: ssh_connection_config.address_family,
         ..ForwardingConfig::default()

src/executor/connection_manager.rs

@@ -415,15 +415,15 @@ Host internal.example.com
     #[test]
     fn test_resolve_effective_jump_hosts_wildcard() {
         let ssh_config_content = r#"
-Host *.internal.example.com
-    ProxyJump gateway.example.com
-
 Host db.internal.example.com
     ProxyJump db-gateway.example.com
+
+Host *.internal.example.com
+    ProxyJump gateway.example.com
 "#;
         let ssh_config = SshConfig::parse(ssh_config_content).unwrap();
 
-        // Should match db.internal.example.com specifically
+        // OpenSSH uses the first value obtained, so put the exact host first.
         let result =
             resolve_effective_jump_hosts(None, Some(&ssh_config), "db.internal.example.com");
         assert_eq!(result, Some("db-gateway.example.com".to_string()));
@@ -452,15 +452,15 @@ Host *.internal.example.com
     #[test]
     fn test_resolve_effective_jump_hosts_none_value() {
         let ssh_config_content = r#"
-Host *.example.com
-    ProxyJump gateway.example.com
-
 Host direct.example.com
     ProxyJump none
+
+Host *.example.com
+    ProxyJump gateway.example.com
 "#;
         let ssh_config = SshConfig::parse(ssh_config_content).unwrap();
 
-        // direct.example.com should have ProxyJump explicitly set to "none"
+        // The explicit disable must be obtained before the wildcard fallback.
         // Note: The actual handling of "none" as special value would be
         // done by the connection layer, but the config should return it
         let result = resolve_effective_jump_hosts(None, Some(&ssh_config), "direct.example.com");

src/executor/parallel.rs

@@ -26,7 +26,9 @@ use crate::node::Node;
 use crate::security::{Password, SudoPassword};
 use crate::ssh::SshConfig;
 use crate::ssh::known_hosts::StrictHostKeyChecking;
-use crate::ssh::tokio_client::{AddressFamily, SshConnectionConfig, SshConnectionConfigResolver};
+use crate::ssh::tokio_client::{
+    AddressFamily, CommandOutput, SshConnectionConfig, SshConnectionConfigResolver,
+};
 
 use super::connection_manager::{ExecutionConfig, download_from_node};
 use super::execution_strategy::{
@@ -62,6 +64,39 @@ pub struct ParallelExecutor {
     pub(crate) ssh_connection_config_resolver: SshConnectionConfigResolver,
 }
 
+async fn report_stream_exit_status(
+    sender: &tokio::sync::mpsc::Sender<CommandOutput>,
+    result: &Result<u32>,
+) {
+    if let Ok(exit_status) = result {
+        let _ = sender.send(CommandOutput::ExitCode(*exit_status)).await;
+    }
+}
+
+fn completed_stream_result(
+    stream: &super::stream_manager::NodeStream,
+) -> Result<crate::ssh::client::CommandResult> {
+    if let Some(exit_status) = stream.exit_code() {
+        return Ok(crate::ssh::client::CommandResult {
+            host: stream.node.host.clone(),
+            output: Vec::new(),
+            stderr: Vec::new(),
+            exit_status,
+        });
+    }
+
+    if let super::stream_manager::ExecutionStatus::Failed(error) = stream.status() {
+        return Err(anyhow::anyhow!("{error}"));
+    }
+
+    Ok(crate::ssh::client::CommandResult {
+        host: stream.node.host.clone(),
+        output: Vec::new(),
+        stderr: Vec::new(),
+        exit_status: 1,
+    })
+}
+
 impl ParallelExecutor {
     /// Create a new parallel executor with default strict mode.
     pub fn new(nodes: Vec<Node>, max_parallel: usize, key_path: Option<String>) -> Self {
@@ -283,7 +318,7 @@ impl ParallelExecutor {
 
     pub(crate) fn connection_config_for_node(&self, node: &Node) -> SshConnectionConfig {
         self.ssh_connection_config_resolver
-            .resolve_for_host(&node.host)
+            .resolve_for_host(node.config_host())
     }
 
     /// Execute a command on all nodes in parallel.
@@ -1300,6 +1335,7 @@ impl ParallelExecutor {
                     }
                 };
 
+                report_stream_exit_status(&tx, &result.1).await;
                 // Explicitly drop the channel to signal completion
                 drop(tx);
                 result
@@ -1504,19 +1540,7 @@ impl ParallelExecutor {
         }
         // Collect final results from all streams
         for stream in manager.streams() {
-            use crate::ssh::client::CommandResult;
-
-            let result =
-                if let super::stream_manager::ExecutionStatus::Failed(err) = stream.status() {
-                    Err(anyhow::anyhow!("{err}"))
-                } else {
-                    Ok(CommandResult {
-                        host: stream.node.host.clone(),
-                        output: Vec::new(), // stdout already printed
-                        stderr: Vec::new(), // stderr already printed
-                        exit_status: stream.exit_code().unwrap_or(1),
-                    })
-                };
+            let result = completed_stream_result(stream);
 
             results.push(ExecutionResult {
                 node: stream.node.clone(),
@@ -1852,6 +1876,63 @@ mod tests {
     use super::*;
     use crate::ssh::SshConfig;
 
+    async fn assert_stream_exit_follows_output(exit_status: u32) {
+        let (sender, mut receiver) = tokio::sync::mpsc::channel(3);
+        sender
+            .send(CommandOutput::StdOut(bytes::Bytes::from_static(b"stdout")))
+            .await
+            .expect("stdout receiver open");
+        sender
+            .send(CommandOutput::StdErr(bytes::Bytes::from_static(b"stderr")))
+            .await
+            .expect("stderr receiver open");
+
+        let result = Ok(exit_status);
+        report_stream_exit_status(&sender, &result).await;
+        drop(sender);
+
+        match receiver.recv().await {
+            Some(CommandOutput::StdOut(data)) => assert_eq!(data, b"stdout"[..]),
+            other => panic!("stdout must be first, got {other:?}"),
+        }
+        match receiver.recv().await {
+            Some(CommandOutput::StdErr(data)) => assert_eq!(data, b"stderr"[..]),
+            other => panic!("stderr must be second, got {other:?}"),
+        }
+        match receiver.recv().await {
+            Some(CommandOutput::ExitCode(actual)) => assert_eq!(actual, exit_status),
+            other => panic!("exit status must follow output, got {other:?}"),
+        }
+        assert!(receiver.recv().await.is_none());
+    }
+
+    #[tokio::test]
+    async fn successful_stream_exit_follows_stdout_and_stderr() {
+        assert_stream_exit_follows_output(0).await;
+    }
+
+    #[tokio::test]
+    async fn nonzero_stream_exit_follows_stdout_and_stderr() {
+        assert_stream_exit_follows_output(23).await;
+    }
+
+    #[tokio::test]
+    async fn nonzero_stream_completion_preserves_remote_status() {
+        let node = Node::new("target".to_string(), 22, "user".to_string());
+        let (sender, receiver) = tokio::sync::mpsc::channel(1);
+        let mut stream = super::super::stream_manager::NodeStream::new(node, receiver);
+        sender
+            .send(CommandOutput::ExitCode(23))
+            .await
+            .expect("exit receiver open");
+        drop(sender);
+        stream.poll();
+
+        let result = completed_stream_result(&stream).expect("remote command result");
+
+        assert_eq!(result.exit_status, 23);
+    }
+
     #[test]
     fn connection_config_for_node_resolves_each_node_host_block() {
         let ssh_config = SshConfig::parse(