From 5f28f57f0f327e7ff20de6d20153b06529ab44e5 Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Tue, 1 Sep 2026 08:09:08 +0900 Subject: [PATCH 1/3] fix(cli)!: restore OpenSSH short flag semantics Reassign the seven colliding short options in single-host mode while preserving bssh extensions as long options and pdsh meanings in pdsh mode. Complete the multiplexing background lifecycle, agent forwarding, compatibility measurements, and migration documentation needed to close the final epic units. --- .gitignore | 1 + README.md | 208 +++---- docs/README.md | 3 +- docs/architecture/cli-interface.md | 2 +- docs/architecture/executor.md | 6 +- docs/architecture/ssh-client.md | 14 +- docs/architecture/ssh-jump-hosts.md | 4 +- docs/man/bssh.1 | 167 ++++-- docs/openssh-regress.md | 8 +- docs/openssh-short-flags-migration.md | 94 ++++ docs/pdsh-examples.md | 45 +- docs/pdsh-migration.md | 12 +- docs/pdsh-options.md | 60 +- docs/shell-config/README.md | 14 +- docs/shell-config/bash.sh | 10 +- docs/shell-config/fish.fish | 20 +- docs/shell-config/zsh.sh | 16 +- examples/health_check.sh | 10 +- examples/interactive_demo.rs | 1 + examples/mpi_exit_code.sh | 6 +- src/app/background.rs | 333 ++++++++++++ src/app/dispatcher.rs | 283 +++++++++- src/app/mod.rs | 1 + src/app/query.rs | 19 +- src/app/utils.rs | 6 +- src/cli/bssh.rs | 406 +++++++++++++- src/cli/mode_detection_tests.rs | 87 +-- src/cli/pdsh.rs | 38 +- src/cli/ssh_args.rs | 68 ++- src/commands/interactive/connection.rs | 45 +- src/commands/interactive/execution.rs | 5 +- src/commands/interactive/types.rs | 14 +- src/commands/interactive/utils.rs | 1 + src/jump/chain/tunnel.rs | 4 + src/main.rs | 47 +- src/ssh/control/mod.rs | 3 +- src/ssh/control/protocol.rs | 1 + src/ssh/control/runtime.rs | 143 +++-- src/ssh/session_policy.rs | 8 + src/ssh/session_policy_tests.rs | 26 + src/ssh/ssh_config/dump.rs | 10 +- .../parser/options/authentication.rs | 6 + .../ssh_config/parser/options/forwarding.rs | 22 +- src/ssh/ssh_config/parser/options/mod.rs | 11 +- src/ssh/ssh_config/parser/options/support.rs | 28 +- src/ssh/ssh_config/parser/tests.rs | 45 ++ src/ssh/ssh_config/resolver.rs | 4 + src/ssh/ssh_config/types.rs | 8 + src/ssh/tokio_client/connection.rs | 291 +++++++++- src/ssh/tokio_client/connection_tests.rs | 34 ++ src/ssh/tokio_client/mod.rs | 1 + src/ssh/tokio_client/session.rs | 14 +- src/utils/diagnostics.rs | 25 + tests/agent_forwarding_live_test.rs | 309 +++++++++++ tests/connect_timeout_test.rs | 2 +- tests/control_multiplexing_live_test.rs | 313 ++++++++++- tests/download_test.rs | 2 +- tests/fail_fast_test.rs | 28 +- tests/no_prefix_test.rs | 27 +- tests/openssh-regress/baseline.json | 4 +- tests/openssh-regress/results.json | 512 +++++++++--------- tests/pdsh_compat_test.rs | 7 + tests/upload_test.rs | 2 +- 63 files changed, 3191 insertions(+), 753 deletions(-) create mode 100644 docs/openssh-short-flags-migration.md create mode 100644 src/app/background.rs create mode 100644 tests/agent_forwarding_live_test.rs diff --git a/.gitignore b/.gitignore index 436e1c30..fd619574 100644 --- a/.gitignore +++ b/.gitignore @@ -23,6 +23,7 @@ Thumbs.db /output/ .bssh/ *.md +!docs/openssh-short-flags-migration.md .claude/ .gemini/ references/ diff --git a/README.md b/README.md index 021df597..89b3a62d 100644 --- a/README.md +++ b/README.md @@ -29,12 +29,12 @@ _See [CHANGELOG.md](./CHANGELOG.md) for the complete version history._ ## Features -- **SSH Compatibility**: Drop-in replacement for SSH with compatible command-line syntax +- **Measured SSH Compatibility**: Single-host behavior is continuously checked against a pinned OpenSSH regress suite; see the [measured score and documented skips](docs/openssh-regress.md) - **Port Forwarding**: Full support for local (-L), remote (-R), and dynamic (-D) SSH port forwarding - **Jump Host Support**: Connect through bastion hosts using OpenSSH ProxyJump syntax (`-J`) - **Parallel Execution**: Execute commands across multiple nodes simultaneously - **Hostlist Expressions**: pdsh-style range expansion (`node[1-5]`, `rack[1-2]-node[1-3]`) for compact host specification -- **Fail-Fast Mode**: Stop immediately on first failure with `-k` flag (pdsh compatible) +- **Fail-Fast Mode**: Stop immediately on first failure with `--fail-fast` (`-k` remains available only in pdsh compatibility mode) - **Interactive Terminal UI (TUI)**: Real-time monitoring with 4 view modes (Summary/Detail/Split/Diff) for multi-node operations - **Cluster Management**: Define and manage node clusters via configuration files - **Progress Tracking**: Real-time progress indicators with smart detection (percentages, fractions, apt/dpkg) @@ -49,7 +49,7 @@ _See [CHANGELOG.md](./CHANGELOG.md) for the complete version history._ ## Platform Support -- **Linux and macOS**: Fully supported, including SSH agent authentication (`-A`), PTY-based interactive sessions, and every CLI feature documented here. +- **Linux and macOS**: Fully supported, including SSH agent authentication (`--use-agent`), PTY-based interactive sessions, and every CLI feature documented here. - **Windows (native)**: Not currently supported as a client. The `bssh` crate does not build for a Windows target today because of unconditional Unix-only dependencies (`nix`, `signal-hook`, `libc`) and un-gated PTY/agent code; there is no Windows CI job or release artifact. Use **WSL2** to run `bssh` on Windows in the meantime. See [#213](https://github.com/lablup/bssh/issues/213) for the current status and the known blockers to native Windows client support. ## Installation @@ -110,6 +110,19 @@ sudo cp target/release/bssh /usr/local/bin/ ## Quick Start ### SSH-Compatible Mode (Single Host) + +The 3.0 command-line contract gives these seven letters their OpenSSH meanings. Existing bssh scripts must use the corresponding long option for the displaced bssh feature; see the [3.0 short-flag migration guide](docs/openssh-short-flags-migration.md). + +| Short flag | OpenSSH-compatible meaning | Long option for the former bssh meaning | +|------------|----------------------------|------------------------------------------| +| `-N` | Do not execute a remote command | `--no-prefix` | +| `-f` | Go to the background after authentication | `--filter` | +| `-C` | Enable SSH compression | `--cluster` | +| `-A` | Enable authentication-agent forwarding | `--use-agent` | +| `-S path` | Select the multiplexing `ControlPath` | `--sudo-password` | +| `-k` | Disable GSSAPI credential delegation | `--fail-fast` | +| `-b address` | Bind the client to a source address | `--batch` | + ```bash # Connect to a host (just like SSH!) bssh user@hostname @@ -139,32 +152,32 @@ Like OpenSSH, bssh supports escape sequences in PTY sessions. These must be type ```bash # Local port forwarding (-L) # Forward local port 8080 to example.com:80 via SSH -bssh -L 8080:example.com:80 user@host +bssh -N -L 8080:example.com:80 user@host # Remote port forwarding (-R) # Forward remote port 8080 to localhost:80 -bssh -R 8080:localhost:80 user@host +bssh -N -R 8080:localhost:80 user@host # Dynamic port forwarding / SOCKS proxy (-D) # Create SOCKS5 proxy on local port 1080 -bssh -D 1080 user@host +bssh -N -D 1080 user@host # Multiple port forwards -bssh -L 3306:db:3306 -R 80:web:80 -D 1080 user@host +bssh -N -L 3306:db:3306 -R 80:web:80 -D 1080 user@host # Bind to specific address -bssh -L 127.0.0.1:8080:web:80 user@host # Local only -bssh -L *:8080:web:80 user@host # All interfaces +bssh -N -L 127.0.0.1:8080:web:80 user@host # Local only +bssh -N -L *:8080:web:80 user@host # All interfaces # SOCKS4 proxy (specify version) -bssh -D 1080/4 user@host # SOCKS4 -bssh -D *:1080/5 user@host # SOCKS5 on all interfaces +bssh -N -D 1080/4 user@host # SOCKS4 +bssh -N -D *:1080/5 user@host # SOCKS5 on all interfaces # Port forwarding with command execution bssh -L 5432:postgres:5432 user@host "psql -h localhost" -# Port forwarding with cluster operations -bssh -C production -L 8080:internal:80 "curl http://localhost:8080" +# Port forwarding without opening a remote shell +bssh -N -L 8080:internal:80 user@gateway ``` ### Jump Host Support (ProxyJump) @@ -182,15 +195,15 @@ bssh -J admin@bastion:2222 user@internal-host bssh -J "[2001:db8::1]:22" user@destination # Combine with cluster operations -bssh -J bastion.example.com -C production "uptime" +bssh -J bastion.example.com --cluster production "uptime" # File transfer through jump host bssh -J bastion.example.com -H internal-server upload app.tar.gz /opt/ -bssh -J admin@bastion:2222 -C production download /etc/config ./backups/ +bssh -J admin@bastion:2222 --cluster production download /etc/config ./backups/ # Interactive mode through jump hosts bssh -J bastion.example.com user@internal-server -bssh -J "jump1,jump2" -C production interactive +bssh -J "jump1,jump2" --cluster production interactive # Multi-hop with file transfer bssh -J "bastion1,bastion2,bastion3" -H target upload -r ./app/ /opt/app/ @@ -202,7 +215,7 @@ bssh -J "bastion1,bastion2,bastion3" -H target upload -r ./app/ /opt/app/ bssh -H "user1@host1.com,user2@host2.com:2222" "uptime" # Using cluster from config -bssh -C production "df -h" +bssh --cluster production "df -h" # Hostlist expressions (pdsh-style range expansion) bssh -H "node[1-5]" "uptime" # node1, node2, node3, node4, node5 @@ -214,63 +227,63 @@ bssh -H "admin@db[01-03]:5432" "psql --version" # With user and port bssh -H "^/etc/hosts.cluster" "uptime" # Read hosts from file # Filter specific hosts with pattern matching -bssh -H "web1,web2,db1,db2" -f "web*" "systemctl status nginx" -bssh -C production -f "db*" "pg_dump --version" -bssh -H "node[1-10]" -f "node[1-5]" "uptime" # Filter with hostlist expression +bssh -H "web1,web2,db1,db2" --filter "web*" "systemctl status nginx" +bssh --cluster production --filter "db*" "pg_dump --version" +bssh -H "node[1-10]" --filter "node[1-5]" "uptime" # Filter with hostlist expression # Exclude specific hosts from execution bssh -H "node1,node2,node3" --exclude "node2" "uptime" -bssh -C production --exclude "db*" "systemctl restart nginx" +bssh --cluster production --exclude "db*" "systemctl restart nginx" bssh -H "node[1-10]" --exclude "node[3-5]" "uptime" # Exclude with hostlist expression # With custom SSH key -bssh -C staging -i ~/.ssh/custom_key "systemctl status nginx" +bssh --cluster staging -i ~/.ssh/custom_key "systemctl status nginx" # Use SSH agent for authentication -bssh -A -C production "systemctl status nginx" +bssh --use-agent --cluster production "systemctl status nginx" # Use password authentication (will prompt for password) bssh --password -H "user@host.com" "uptime" # Use sudo password for privileged commands (prompts securely) -bssh -S -C production "sudo apt update && sudo apt upgrade -y" +bssh --sudo-password --cluster production "sudo apt update && sudo apt upgrade -y" # Combine sudo password with SSH agent authentication -bssh -A -S -C production "sudo systemctl restart nginx" +bssh --use-agent --sudo-password --cluster production "sudo systemctl restart nginx" # Use encrypted SSH key (will prompt for passphrase) -bssh -i ~/.ssh/encrypted_key -C production "df -h" +bssh -i ~/.ssh/encrypted_key --cluster production "df -h" # Limit parallel connections -bssh -C production --parallel 5 "apt update" +bssh --cluster production --parallel 5 "apt update" # Set command timeout (10 seconds) -bssh -C production --timeout 10 "quick-check" +bssh --cluster production --timeout 10 "quick-check" # No timeout (unlimited execution time) -bssh -C staging --timeout 0 "long-running-backup" +bssh --cluster staging --timeout 0 "long-running-backup" # Set connection timeout (default: 30 seconds) -bssh -C production --connect-timeout 10 "uptime" +bssh --cluster production --connect-timeout 10 "uptime" # Different timeouts for connection and command -bssh -C production --connect-timeout 5 --timeout 600 "long-running-job" +bssh --cluster production --connect-timeout 5 --timeout 600 "long-running-job" # Configure SSH keepalive (prevent idle connection timeouts) -bssh -C production --server-alive-interval 30 "long-running-job" +bssh --cluster production --server-alive-interval 30 "long-running-job" # Disable keepalive (set interval to 0) -bssh -C production --server-alive-interval 0 "quick-job" +bssh --cluster production --server-alive-interval 0 "quick-job" # Keepalive with custom max retries (default: 3) -bssh -C production --server-alive-interval 30 --server-alive-count-max 5 "long-running-job" +bssh --cluster production --server-alive-interval 30 --server-alive-count-max 5 "long-running-job" -# Fail-fast mode: stop immediately on any failure (pdsh -k compatible) -bssh -k -H "web1,web2,web3" "deploy.sh" -bssh --fail-fast -C production "critical-script.sh" +# Fail-fast mode: stop immediately on any failure +bssh --fail-fast -H "web1,web2,web3" "deploy.sh" +bssh --fail-fast --cluster production "critical-script.sh" # Combine fail-fast with require-all-success for critical operations -bssh -k --require-all-success -C production "service-restart.sh" +bssh --fail-fast --require-all-success --cluster production "service-restart.sh" ``` ### Output Modes @@ -282,7 +295,7 @@ Interactive Terminal UI with real-time monitoring - automatically enabled when r ```bash # TUI mode automatically activates for multi-node commands -bssh -C production "apt-get update" +bssh --cluster production "apt-get update" # Features: # - Summary view: All nodes at a glance with progress bars @@ -363,7 +376,7 @@ The TUI includes an in-app log panel that captures error and warning messages wi #### Stream Mode (Real-time with Node Prefixes) ```bash # Enable stream mode explicitly -bssh -C production --stream "tail -f /var/log/syslog" +bssh --cluster production --stream "tail -f /var/log/syslog" # Output: # [node1] Oct 30 10:15:23 systemd[1]: Started nginx.service @@ -371,7 +384,7 @@ bssh -C production --stream "tail -f /var/log/syslog" # [node1] Oct 30 10:15:25 nginx: Configuration test successful # Stream mode without hostname prefix (pdsh -N compatibility) -bssh -C production --stream --no-prefix "uname -a" +bssh --cluster production --stream --no-prefix "uname -a" # Output (no [node] prefixes): # Linux node1 5.15.0-generic # Linux node2 5.15.0-generic @@ -380,7 +393,7 @@ bssh -C production --stream --no-prefix "uname -a" #### File Mode (Save to Per-Node Files) ```bash # Save each node's output to timestamped files -bssh -C production --output-dir ./logs "ps aux" +bssh --cluster production --output-dir ./logs "ps aux" # Creates: # ./logs/node1_20251030_101523.stdout @@ -391,11 +404,11 @@ bssh -C production --output-dir ./logs "ps aux" #### Normal Mode (Traditional Output) ```bash # Automatically used when output is piped or redirected -bssh -C production "uptime" | tee results.txt -bssh -C production "df -h" > disk-usage.log +bssh --cluster production "uptime" | tee results.txt +bssh --cluster production "df -h" > disk-usage.log # Manually disable TUI in terminals -CI=true bssh -C production "command" +CI=true bssh --cluster production "command" ``` ### Batch Mode (Ctrl+C Handling) @@ -406,18 +419,18 @@ bssh provides two modes for handling Ctrl+C during parallel execution: - First Ctrl+C: Shows status (running/completed counts) - Second Ctrl+C (within 1 second): Terminates all jobs -**Batch Mode (`-b` / `--batch`)**: +**Batch Mode (`--batch`)**: - Single Ctrl+C: Immediately terminates all jobs - Useful for non-interactive scripts and CI/CD pipelines ```bash # Default behavior (two-stage Ctrl+C) -bssh -C production "long-running-command" +bssh --cluster production "long-running-command" # Ctrl+C once: shows status # Ctrl+C again (within 1s): terminates # Batch mode (immediate termination) -bssh -C production -b "long-running-command" +bssh --cluster production --batch "long-running-command" # Ctrl+C once: immediately terminates all jobs # Useful for automation @@ -522,18 +535,18 @@ pdsh -w node1,node2,backup1,backup2 -x "*backup*" -q ### Built-in Commands ```bash # Test connectivity to hosts -bssh -C production ping +bssh --cluster production ping bssh -H "host1,host2" ping # List configured clusters bssh list # Interactive mode (single or multiplexed) -bssh -C production interactive +bssh --cluster production interactive bssh -H "host1,host2" interactive # File transfer operations -bssh -C production upload local.txt /tmp/ +bssh --cluster production upload local.txt /tmp/ bssh -H "host1,host2" download /etc/hosts ./backups/ ``` @@ -584,10 +597,10 @@ bssh supports multiple authentication methods: ### SSH Agent - **Auto-detection**: Automatically uses SSH agent if `SSH_AUTH_SOCK` is set -- **Explicit**: Use `-A` flag to force SSH agent authentication +- **Explicit**: Use `--use-agent` to force SSH agent authentication ### Password Authentication -- Use `-P` / `--password` flag to enable password authentication +- Use `--password` to enable password authentication - The password is prompted **once up-front**, before any parallel connection tasks start, and is shared securely across all nodes — the prompt appears exactly once regardless of how many hosts are targeted - Password is prompted securely without echo - For automation, set `BSSH_PASSWORD` in the environment (not recommended; see security notes in the Sudo Password section) @@ -598,17 +611,17 @@ bssh supports multiple authentication methods: bssh -H "user@host" "uptime" # Use specific SSH key (prompts for passphrase if encrypted) -bssh -i ~/.ssh/custom_key -c production "df -h" +bssh -i ~/.ssh/custom_key --cluster production "df -h" # Use SSH agent -bssh -A -c production "systemctl status" +bssh --use-agent --cluster production "systemctl status" # Use password authentication -bssh -P -H "user@host" "ls -la" +bssh --password -H "user@host" "ls -la" # Authentication through jump hosts -bssh -A -J bastion.example.com user@internal-server "uptime" -bssh -i ~/.ssh/prod_key -J "jump1,jump2" -C production "df -h" +bssh --use-agent -J bastion.example.com user@internal-server "uptime" +bssh -i ~/.ssh/prod_key -J "jump1,jump2" --cluster production "df -h" ``` ### Sudo Password Support @@ -621,16 +634,16 @@ bssh supports automatic sudo password injection for commands that require elevat ```bash # Basic sudo command (will prompt for sudo password) -bssh -S -C production "sudo apt update" +bssh --sudo-password --cluster production "sudo apt update" # Combine with SSH agent authentication -bssh -A -S -C production "sudo systemctl restart nginx" +bssh --use-agent --sudo-password --cluster production "sudo systemctl restart nginx" # Multiple sudo commands in a single session -bssh -S -C production "sudo apt update && sudo apt upgrade -y" +bssh --sudo-password --cluster production "sudo apt update && sudo apt upgrade -y" # Sudo with specific SSH key -bssh -i ~/.ssh/admin_key -S -C production "sudo reboot" +bssh -i ~/.ssh/admin_key --sudo-password --cluster production "sudo reboot" ``` **Environment Variable Alternative:** @@ -640,13 +653,13 @@ For automation scenarios, you can use the `BSSH_SUDO_PASSWORD` environment varia ```bash # NOT RECOMMENDED for security reasons export BSSH_SUDO_PASSWORD="your-password" -bssh -S -C production "sudo apt update" +bssh --sudo-password --cluster production "sudo apt update" ``` **Security Warnings:** - Environment variables may be visible in process listings - Avoid storing passwords in shell history -- The `-S` flag with secure prompt is the recommended approach +- The `--sudo-password` option with secure prompt is the recommended approach - Password is automatically cleared from memory after use using `zeroize` ## Environment Variables @@ -674,19 +687,19 @@ bssh supports configuration via environment variables: ### SSH Password Variable - **`BSSH_PASSWORD`**: SSH password for automated password authentication - - Used when `--password` / `-P` is set and `BSSH_PASSWORD` is non-empty; skips the interactive prompt + - Used when `--password` is set and `BSSH_PASSWORD` is non-empty; skips the interactive prompt - **WARNING**: Not recommended for security reasons - Environment variables may be visible in process listings and shell history - - Use the interactive `-P` prompt instead for security-sensitive operations - - Example: `BSSH_PASSWORD=secret bssh -P -H "user@host" "uptime"` + - Use the interactive `--password` prompt instead for security-sensitive operations + - Example: `BSSH_PASSWORD=secret bssh --password -H "user@host" "uptime"` ### Sudo Password Variable - **`BSSH_SUDO_PASSWORD`**: Sudo password for automated sudo authentication - **WARNING**: Not recommended for security reasons - Environment variables may be visible in process listings - - Use the `-S` flag with secure prompt instead - - Example: `BSSH_SUDO_PASSWORD=password bssh -S -C prod "sudo apt update"` + - Use the `--sudo-password` option with secure prompt instead + - Example: `BSSH_SUDO_PASSWORD=password bssh --sudo-password --cluster prod "sudo apt update"` ## Configuration @@ -725,7 +738,7 @@ bssh "nvidia-smi" # Check GPU status on all nodes bssh interactive # Opens interactive session with all Backend.AI nodes # You can still override with explicit options if needed: -bssh -C other-cluster "command" # Use a different cluster +bssh --cluster other-cluster "command" # Use a different cluster bssh -H specific-host "command" # Use specific host ``` @@ -1173,10 +1186,10 @@ bssh user@host.prod.example.com bssh -F ~/custom-ssh-config user@host.prod.example.com # SSH config works with cluster operations -bssh -C production "uptime" +bssh --cluster production "uptime" # Config options apply to all cluster nodes -bssh -F ~/.ssh/prod-config -C production upload app.tar.gz /opt/ +bssh -F ~/.ssh/prod-config --cluster production upload app.tar.gz /opt/ ``` ## Command-Line Options @@ -1184,27 +1197,36 @@ bssh -F ~/.ssh/prod-config -C production upload app.tar.gz /opt/ ``` Options: -H, --hosts Comma-separated list of hosts (user@host:port format) - -C, --cluster Cluster name from configuration file - -f, --filter Filter hosts by pattern (supports wildcards like 'web*') + -C Enable SSH compression + -f Go to background after authentication + --cluster Cluster name from configuration file + --filter Filter hosts by pattern (supports wildcards like 'web*') --exclude Exclude hosts from target list (comma-separated, supports wildcards) --config Configuration file path [default: ~/.config/bssh/config.yaml] -u, --user Default username for SSH connections -i, --identity SSH private key file path (prompts for passphrase if encrypted) - -A, --use-agent Use SSH agent for authentication (Unix/Linux/macOS only) - -P, --password Use password authentication (will prompt for password) - -S, --sudo-password Prompt for sudo password to auto-respond to sudo prompts + -A Enable authentication-agent forwarding + --use-agent Use SSH agent for authentication (Unix/Linux/macOS only) + --password Use password authentication (will prompt for password) + -S Multiplexing control socket path + --sudo-password Prompt for sudo password to auto-respond to sudo prompts + -N Do not execute a remote command + -k Disable GSSAPI credential delegation + -b Bind to a local source address -J, --jump-host Jump hosts: [user@]host[:port],... (uses local user if not specified) -L, --local-forward Local port forwarding [bind_address:]port:host:hostport -R, --remote-forward Remote port forwarding [bind_address:]port:host:hostport -D, --dynamic-forward Dynamic port forwarding (SOCKS) [bind_address:]port[/version] --strict-host-key-checking Host key checking mode (yes/no/accept-new) [default: accept-new] - -p, --parallel Maximum parallel connections [default: 10] + --parallel Maximum parallel connections [default: 10] --timeout Command timeout in seconds (0 for unlimited) [default: 300] --connect-timeout SSH connection timeout in seconds (minimum: 1) [default: 30] --server-alive-interval SSH keepalive interval in seconds (0 to disable) [default: 30] --server-alive-count-max Max keepalive messages without response [default: 3] --output-dir Output directory for command results - -N, --no-prefix Disable hostname prefix in output (pdsh -N compatibility) + --no-prefix Disable hostname prefix in output + --fail-fast Stop on the first failed node + --batch Terminate all jobs on the first Ctrl+C -v, --verbose Increase verbosity (-v, -vv, -vvv) -h, --help Print help -V, --version Print version @@ -1222,7 +1244,7 @@ bssh "python train.py --distributed" # Run distributed training ### Run system updates ```bash -bssh -C production "sudo apt update && sudo apt upgrade -y" +bssh --cluster production "sudo apt update && sudo apt upgrade -y" ``` ### Check disk usage @@ -1232,24 +1254,24 @@ bssh -H "server1,server2,server3" "df -h | grep -E '^/dev/'" ### Restart services ```bash -bssh -C webservers "sudo systemctl restart nginx" +bssh --cluster webservers "sudo systemctl restart nginx" ``` ### Collect logs ```bash -bssh -C production --output-dir ./logs "tail -n 100 /var/log/syslog" +bssh --cluster production --output-dir ./logs "tail -n 100 /var/log/syslog" ``` ### Long-running commands with timeout ```bash # Set 30 minute timeout for backup operations -bssh -C production --timeout 1800 "backup-database.sh" +bssh --cluster production --timeout 1800 "backup-database.sh" # No timeout for data migration (may take hours) -bssh -C production --timeout 0 "migrate-data.sh" +bssh --cluster production --timeout 0 "migrate-data.sh" # Quick health check with 5 second timeout -bssh -C monitoring --timeout 5 "health-check.sh" +bssh --cluster monitoring --timeout 5 "health-check.sh" ``` ### Interactive Mode @@ -1258,19 +1280,19 @@ Start an interactive shell session on cluster nodes: ```bash # Interactive session on all nodes (multiplex mode - default) -bssh -C production interactive +bssh --cluster production interactive # Interactive session on a single node -bssh -C production interactive --single-node +bssh --cluster production interactive --single-node # Custom prompt format bssh -H server1,server2 interactive --prompt-format "{user}@{host}> " # Set initial working directory -bssh -C staging interactive --work-dir /var/www +bssh --cluster staging interactive --work-dir /var/www # Interactive mode with keepalive for long-running sessions (e.g., tmux) -bssh -C production --server-alive-interval 30 --server-alive-count-max 5 interactive +bssh --cluster production --server-alive-interval 30 --server-alive-count-max 5 interactive ``` #### Interactive Mode Configuration @@ -1356,7 +1378,7 @@ For large clusters (>10 nodes), the prompt uses a compact format: #### Example Interactive Session ```bash -$ bssh -C production interactive +$ bssh --cluster production interactive Connected to 3 nodes [● ● ●] bssh> !status @@ -1426,13 +1448,13 @@ Each output file includes metadata headers: ### Example Usage ```bash # Save outputs to timestamped directory -bssh -C production --output-dir ./results/$(date +%Y%m%d) "ps aux | head -10" +bssh --cluster production --output-dir ./results/$(date +%Y%m%d) "ps aux | head -10" # Collect system information -bssh -C all-servers --output-dir ./system-info "uname -a; df -h; free -m" +bssh --cluster all-servers --output-dir ./system-info "uname -a; df -h; free -m" # Debug failed services -bssh -C webservers --output-dir ./debug "systemctl status nginx" +bssh --cluster webservers --output-dir ./debug "systemctl status nginx" ``` ## Development diff --git a/docs/README.md b/docs/README.md index a0159a2b..ebb244a5 100644 --- a/docs/README.md +++ b/docs/README.md @@ -39,6 +39,7 @@ Welcome to the bssh documentation. This documentation covers both the bssh clien | Document | Description | |----------|-------------| +| [OpenSSH Short Flags for 3.0](./openssh-short-flags-migration.md) | Rewriting bssh scripts for the seven reassigned short options | | [pdsh Migration](./pdsh-migration.md) | Migrating from pdsh to bssh | | [pdsh Examples](./pdsh-examples.md) | pdsh-style command examples | | [pdsh Options](./pdsh-options.md) | pdsh option compatibility | @@ -115,7 +116,7 @@ bssh user@host bssh -H host1,host2,host3 'uptime' # Using clusters -bssh -C mycluster 'hostname' +bssh --cluster mycluster 'hostname' ``` ### bssh-server diff --git a/docs/architecture/cli-interface.md b/docs/architecture/cli-interface.md index 6d26a0c6..f2cf499c 100644 --- a/docs/architecture/cli-interface.md +++ b/docs/architecture/cli-interface.md @@ -90,7 +90,7 @@ The `looks_like_host_specification` function uses the following detection patter - Detection happens BEFORE mode determination (`is_ssh_mode`) - Auto-sets `cli.cluster` to `"bai_auto"` when Backend.AI environment variables are present -- Only activates when no explicit cluster (`-C`) or hosts (`-H`) specified +- Only activates when no explicit cluster (`--cluster`) or hosts (`-H`) specified - Skips auto-detection if destination contains host indicators - Prevents commands from being misinterpreted as hostnames in SSH mode - Respects explicit user configuration over auto-detection diff --git a/docs/architecture/executor.md b/docs/architecture/executor.md index bc0dc273..d510e944 100644 --- a/docs/architecture/executor.md +++ b/docs/architecture/executor.md @@ -68,7 +68,7 @@ The executor supports two modes for handling Ctrl+C (SIGINT) signals during para 3. **Time window reset**: If >1 second passes, next Ctrl+C restarts the sequence and shows status again 4. Provides users visibility into execution progress before termination -### Batch Mode (`--batch` / `-b`) +### Batch Mode (`--batch`) - **Single Ctrl+C**: Immediately terminates all jobs with exit code 130 - Optimized for non-interactive environments (CI/CD, scripts) @@ -156,7 +156,9 @@ The batch flag is passed through the executor chain: ## Fail-Fast Mode -The `--fail-fast` / `-k` option enables immediate termination when any node fails. This is compatible with pdsh's `-k` flag and useful for: +The `--fail-fast` option enables immediate termination when any node fails. In +pdsh compatibility mode, the equivalent spelling is `-k`. This behavior is +useful for: - Critical operations where partial execution is unacceptable - Deployment scripts where all nodes must succeed - Validation checks across clusters diff --git a/docs/architecture/ssh-client.md b/docs/architecture/ssh-client.md index d534d100..8c86c593 100644 --- a/docs/architecture/ssh-client.md +++ b/docs/architecture/ssh-client.md @@ -279,7 +279,7 @@ src/executor/ **Stream Mode:** ```bash # Real-time streaming output -bssh -C production --stream "tail -f /var/log/app.log" +bssh --cluster production --stream "tail -f /var/log/app.log" # With filtering bssh -H "web*" --stream "systemctl status nginx" @@ -288,7 +288,7 @@ bssh -H "web*" --stream "systemctl status nginx" **File Mode:** ```bash # Save outputs to directory -bssh -C cluster --output-dir ./results "ps aux" +bssh --cluster cluster --output-dir ./results "ps aux" # Each node gets separate files with timestamps ls ./results/ @@ -666,7 +666,7 @@ Comprehensive test coverage including: **Status:** Implemented **Overview:** -The sudo password module provides secure handling of sudo authentication for commands that require elevated privileges. When enabled with the `-S` flag, bssh automatically detects sudo password prompts in command output and injects the password without user intervention. +The sudo password module provides secure handling of sudo authentication for commands that require elevated privileges. When enabled with `--sudo-password`, bssh automatically detects sudo password prompts in command output and injects the password without user intervention. **Architecture Components:** @@ -749,7 +749,7 @@ pub async fn execute_with_sudo( - Environment variable option (`BSSH_SUDO_PASSWORD`) with security warnings **Execution Path Integration:** -1. CLI flag `-S/--sudo-password` triggers password prompt +1. CLI flag `--sudo-password` triggers password prompt 2. Password wrapped in `Arc` for sharing across nodes 3. `ExecutionConfig` carries optional `sudo_password` field 4. Both streaming and non-streaming execution paths support sudo @@ -758,14 +758,14 @@ pub async fn execute_with_sudo( **Usage Patterns:** ```bash # Basic usage - prompts for password before execution -bssh -S -C production "sudo apt update" +bssh --sudo-password --cluster production "sudo apt update" # Combined with SSH agent authentication -bssh -A -S -C production "sudo systemctl restart nginx" +bssh --use-agent --sudo-password --cluster production "sudo systemctl restart nginx" # Environment variable (not recommended) export BSSH_SUDO_PASSWORD="password" -bssh -S -C production "sudo apt update" +bssh --sudo-password --cluster production "sudo apt update" ``` **Limitations:** diff --git a/docs/architecture/ssh-jump-hosts.md b/docs/architecture/ssh-jump-hosts.md index 1ccaedb8..4772c4b4 100644 --- a/docs/architecture/ssh-jump-hosts.md +++ b/docs/architecture/ssh-jump-hosts.md @@ -83,11 +83,11 @@ bssh -J "user@[::1]:2222" -H target "command" # File transfer through jump hosts bssh -J bastion.example.com -H internal upload app.tar.gz /opt/ -bssh -J "jump1,jump2" -C production download /etc/config ./backups/ +bssh -J "jump1,jump2" --cluster production download /etc/config ./backups/ # Interactive mode through jump hosts bssh -J bastion.example.com user@internal-server -bssh -J "jump1,jump2" -C production interactive +bssh -J "jump1,jump2" --cluster production interactive ``` ### Completed Features diff --git a/docs/man/bssh.1 b/docs/man/bssh.1 index b5953a32..ba2c53e4 100644 --- a/docs/man/bssh.1 +++ b/docs/man/bssh.1 @@ -14,16 +14,16 @@ bssh \- Broadcast SSH - SSH-compatible client with parallel execution capabiliti .SH DESCRIPTION .B bssh -is a high-performance SSH client that can be used as a drop-in replacement for standard SSH while also providing -powerful parallel execution capabilities for cluster management. Built with Rust, it supports both single-host -SSH connections (SSH compatibility mode) and multi-server operations (cluster mode). +is a high-performance SSH client that provides measured compatibility with common OpenSSH client behavior while +also offering powerful parallel execution capabilities for cluster management. Built with Rust, it supports both +single-host SSH connections (SSH compatibility mode) and multi-server operations (cluster mode). .B SSH Compatibility Mode: When used with a single destination (e.g., bssh user@host), bssh behaves like standard SSH, supporting common SSH options and automatically starting an interactive shell when no command is provided. .B Multi-Server Mode: -When used with clusters (-C) or multiple hosts (-H), bssh executes commands across multiple nodes +When used with clusters (--cluster) or multiple hosts (-H), bssh executes commands across multiple nodes simultaneously with real-time output monitoring. In interactive terminals, bssh automatically launches a Terminal User Interface (TUI) with multiple view modes (Summary, Detail, Split, Diff) for real-time monitoring. The TUI can be disabled with --stream (real-time text output) or @@ -36,6 +36,52 @@ multi-node session environments. .SH OPTIONS .SS SSH-Compatible Options +.PP +The following seven short options use their OpenSSH meanings in the 3.0 +command-line contract. Scripts that used the former bssh meanings must migrate +to the long options documented under Multi-Server Options. See +.BR docs/openssh-short-flags-migration.md . + +.TP +.BR \-N +Do not execute a remote command. This is normally used with port forwarding. + +.TP +.BR \-f +Go to the background after authentication. Authentication and requested +forward setup complete before the foreground process exits. + +.TP +.BR \-C +Enable SSH transport compression. Equivalent to +.IR "Compression yes" . + +.TP +.BR \-A +Enable forwarding of the authentication agent connection. This is distinct +from +.BR \-\-use\-agent , +which selects an agent for authenticating the local bssh client. + +.TP +.BR \-S " " \fIcontrol_path\fR +Use the specified connection-multiplexing control socket path. Equivalent to +the +.I ControlPath +SSH configuration directive. + +.TP +.BR \-k +Disable forwarding of GSSAPI credentials. Equivalent to +.IR "GSSAPIDelegateCredentials no" . + +.TP +.BR \-b " " \fIbind_address\fR +Bind the client side of the SSH connection to the specified source address. +Equivalent to the +.I BindAddress +SSH configuration directive. + .TP .BR \-i " " \fIidentity_file\fR SSH private key file path (same as ssh -i) @@ -163,8 +209,8 @@ IPv6 address literals must be enclosed in brackets. A bracket group is interpret .RE .TP -.BR \-C ", " \-\-cluster " " \fICLUSTER\fR -Cluster name from configuration file (uppercase C for multi-server mode) +.BR \-\-cluster " " \fICLUSTER\fR +Cluster name from the bssh configuration file. .TP .BR \-\-config " " \fICONFIG\fR @@ -172,7 +218,7 @@ Configuration file path (default: ~/.config/bssh/config.yaml) .TP -.BR \-A ", " \-\-use\-agent +.BR \-\-use\-agent Use SSH agent for authentication (Unix/Linux/macOS only). When this option is specified, bssh will attempt to use the SSH agent for authentication. Falls back to key file authentication if the agent @@ -191,7 +237,7 @@ prompting interactively (not recommended; see This is useful for systems that don't have SSH keys configured. .TP -.BR \-S ", " \-\-sudo\-password +.BR \-\-sudo\-password Prompt for sudo password to automatically respond to sudo prompts. When this option is specified, bssh will: .RS @@ -218,18 +264,18 @@ Password is never logged or printed in any output .RE .TP -.BR \-f ", " \-\-filter " " \fIPATTERN\fR +.BR \-\-filter " " \fIPATTERN\fR Filter hosts by pattern. Supports both wildcards and hostlist expressions. -Use with -H or -C to execute on a subset of hosts. +Use with -H or --cluster to execute on a subset of hosts. .RS .PP Examples: .IP \[bu] 2 --f "web*" \[->] matches web01, web02, etc. (glob pattern) +--filter "web*" \[->] matches web01, web02, etc. (glob pattern) .IP \[bu] 2 --f "node[1-5]" \[->] matches node1 through node5 (hostlist expression) +--filter "node[1-5]" \[->] matches node1 through node5 (hostlist expression) .IP \[bu] 2 --f "node[1,3,5]" \[->] matches node1, node3, node5 (specific values) +--filter "node[1,3,5]" \[->] matches node1, node3, node5 (specific values) .RE .TP @@ -311,16 +357,22 @@ disabled when output is piped or in CI environments. This disables the interactive TUI mode. .TP -.BR \-N ", " \-\-no\-prefix -Disable hostname prefix in output lines (pdsh -N compatibility). When +.BR \-\-no\-prefix +Disable hostname prefix in output lines. When specified, output lines are displayed without the [hostname] prefix, which is useful for programmatic parsing or cleaner display. Works with both stream mode (--stream) and file mode (--output-dir). -Example: bssh -H host1,host2 --stream -N "uname -a" +Example: bssh -H host1,host2 --stream --no-prefix "uname -a" + +.TP +.BR \-\-batch +Use single-stage interrupt handling: the first Ctrl+C terminates all jobs. +The default multi-server behavior reports status on the first Ctrl+C and +terminates on a second Ctrl+C. .TP -.BR \-k ", " \-\-fail\-fast -Stop execution immediately on first failure (pdsh -k compatible). +.BR \-\-fail\-fast +Stop execution immediately on first failure. When enabled, bssh cancels pending commands when any node fails due to connection error or non-zero exit code. This is useful for: .RS @@ -372,7 +424,7 @@ This hybrid strategy provides detailed error codes from the main rank while maintaining awareness of failures on other nodes. .SH COMMANDS -In multi-server mode (-H or -C), commands can be executed directly without the 'exec' subcommand. +In multi-server mode (-H or --cluster), commands can be executed directly without the 'exec' subcommand. For example: bssh -H "host1,host2" "uptime" (automatic command execution) .TP @@ -1385,11 +1437,11 @@ Specify target hosts: .TP .B --filter Include only matching hosts: -.B bssh -C cluster --filter "web[1-5]" "systemctl status nginx" +.B bssh --cluster cluster --filter "web[1-5]" "systemctl status nginx" .TP .B --exclude Exclude matching hosts: -.B bssh -C cluster --exclude "node[1,3,5]" "df -h" +.B bssh --cluster cluster --exclude "node[1,3,5]" "df -h" .SS Examples .nf @@ -1497,17 +1549,17 @@ Commands are executed directly without needing 'exec' subcommand .TP Use cluster from configuration: -.B bssh -C production "df -h" +.B bssh --cluster production "df -h" .TP Filter hosts with pattern matching: -.B bssh -H "web1,web2,db1,db2" -f "web*" "systemctl status nginx" +.B bssh -H "web1,web2,db1,db2" --filter "web*" "systemctl status nginx" .RS Executes command only on hosts matching the pattern 'web*' .RE .TP -.B bssh -C production -f "db*" "pg_dump --version" +.B bssh --cluster production --filter "db*" "pg_dump --version" .RS Executes command only on database nodes in the production cluster .RE @@ -1520,31 +1572,31 @@ Executes command on node1 and node3, excluding node2 .RE .TP -.B bssh -C production --exclude "db*" "systemctl restart nginx" +.B bssh --cluster production --exclude "db*" "systemctl restart nginx" .RS Executes command on all production hosts except database servers .RE .TP -.B bssh -C production --exclude "web1,web2" "apt update" +.B bssh --cluster production --exclude "web1,web2" "apt update" .RS Excludes specific hosts web1 and web2 from the cluster operation .RE .TP Test connectivity: -.B bssh -C production ping +.B bssh --cluster production ping .RS Note: 'ping' is a built-in subcommand, not an automatic execution .RE .TP Upload file to remote hosts (SFTP): -.B bssh -C production upload local_file.txt /tmp/remote_file.txt +.B bssh --cluster production upload local_file.txt /tmp/remote_file.txt .TP Download file from remote hosts (SFTP): -.B bssh -C production download /etc/passwd ./downloads/ +.B bssh --cluster production download /etc/passwd ./downloads/ .RS Downloads /etc/passwd from each host to ./downloads/ directory. Files are saved as hostname_passwd (e.g., web1_passwd, web2_passwd) @@ -1560,11 +1612,11 @@ Increase verbosity for debugging: .TP Use custom SSH key: -.B bssh -i ~/.ssh/custom_key -C staging "systemctl status" +.B bssh -i ~/.ssh/custom_key --cluster staging "systemctl status" .TP Use SSH agent for authentication: -.B bssh -A -C production "systemctl status" +.B bssh --use-agent --cluster production "systemctl status" .TP Use password authentication: @@ -1575,28 +1627,28 @@ Prompts for password interactively .TP Execute sudo commands with automatic password injection: -.B bssh -S -C production "sudo apt update && sudo apt upgrade -y" +.B bssh --sudo-password --cluster production "sudo apt update && sudo apt upgrade -y" .RS Prompts for sudo password once, then automatically responds to sudo prompts on all nodes .RE .TP Combine sudo with SSH agent authentication: -.B bssh -A -S -C production "sudo systemctl restart nginx" +.B bssh --use-agent --sudo-password --cluster production "sudo systemctl restart nginx" .RS Uses SSH agent for connection and sudo password for privilege escalation .RE .TP Use encrypted SSH key: -.B bssh -i ~/.ssh/encrypted_key -C production "df -h" +.B bssh -i ~/.ssh/encrypted_key --cluster production "df -h" .RS Automatically detects encrypted key and prompts for passphrase .RE .TP Save output to files: -.B bssh --output-dir ./results -C production "ps aux" +.B bssh --output-dir ./results --cluster production "ps aux" .RS Creates timestamped files per node: .br @@ -1611,7 +1663,7 @@ Creates timestamped files per node: .TP Interactive TUI mode (default in terminals): -.B bssh -C production "apt-get update" +.B bssh --cluster production "apt-get update" .RS Automatically launches Terminal UI with real-time monitoring. .br @@ -1644,7 +1696,7 @@ Log panel keys (when visible): .TP Stream mode with real-time prefixes: -.B bssh -C production --stream "tail -f /var/log/syslog" +.B bssh --cluster production --stream "tail -f /var/log/syslog" .RS Disables TUI and streams output with [node] prefixes in real-time. .br @@ -1660,21 +1712,21 @@ Useful for monitoring long-running commands or when piping output. .SS Fail-Fast Mode Examples .TP Stop on first failure during critical deployment: -.B bssh -k -C production "deploy.sh" +.B bssh --fail-fast --cluster production "deploy.sh" .RS Execution stops immediately if any node fails the deployment script .RE .TP Combine fail-fast with require-all-success: -.B bssh --fail-fast --require-all-success -C production "service-restart.sh" +.B bssh --fail-fast --require-all-success --cluster production "service-restart.sh" .RS Stops early on failure AND ensures final exit code reflects any failures .RE .TP Sequential fail-fast with limited parallelism: -.B bssh -k --parallel 1 -H "node1,node2,node3" "critical-operation" +.B bssh --fail-fast --parallel 1 -H "node1,node2,node3" "critical-operation" .RS Runs commands one at a time, stopping on first failure .RE @@ -1686,39 +1738,39 @@ Upload configuration file to all nodes: .TP Download logs from all web servers: -.B bssh -C webservers download /var/log/nginx/access.log ./logs/ +.B bssh --cluster webservers download /var/log/nginx/access.log ./logs/ .RS Each file is saved as hostname_access.log in the ./logs/ directory .RE .TP Upload with custom SSH key and increased parallelism: -.B bssh -i ~/.ssh/deploy_key --parallel 20 -C production upload deploy.tar.gz /tmp/ +.B bssh -i ~/.ssh/deploy_key --parallel 20 --cluster production upload deploy.tar.gz /tmp/ .TP Upload multiple files with glob pattern: -.B bssh -C production upload "*.log" /var/backups/logs/ +.B bssh --cluster production upload "*.log" /var/backups/logs/ .RS Uploads all .log files from current directory to /var/backups/logs/ on all nodes .RE .TP Download logs with wildcard pattern: -.B bssh -C production download "/var/log/app*.log" ./collected_logs/ +.B bssh --cluster production download "/var/log/app*.log" ./collected_logs/ .RS Downloads all files matching app*.log from /var/log/ on each node .RE .TP Start interactive mode with all nodes: -.B bssh -C production interactive +.B bssh --cluster production interactive .RS Opens an interactive shell session with all nodes in multiplex mode .RE .TP Start interactive mode with single node: -.B bssh -C production interactive --single-node +.B bssh --cluster production interactive --single-node .RS Prompts to select one node for interactive session .RE @@ -1729,14 +1781,14 @@ Interactive mode with custom prompt: .TP Interactive mode with initial working directory: -.B bssh -C staging interactive --work-dir /var/www +.B bssh --cluster staging interactive --work-dir /var/www .RS Sets initial working directory to /var/www on all nodes .RE .TP Interactive mode with keepalive for long-running sessions: -.B bssh -C production --server-alive-interval 30 --server-alive-count-max 5 interactive +.B bssh --cluster production --server-alive-interval 30 --server-alive-count-max 5 interactive .RS Configure SSH keepalive settings to prevent idle disconnection in long-running sessions (e.g., tmux). The keepalive settings apply to both the destination host and any jump hosts in the connection chain. @@ -1747,7 +1799,7 @@ The keepalive settings apply to both the destination host and any jump hosts in .TP MPI job with intelligent error handling: .nf -.B bssh -C cluster "mpirun -n 16 ./simulation" +.B bssh --cluster cluster "mpirun -n 16 ./simulation" .B EXIT_CODE=$? .B .B case $EXIT_CODE in @@ -1762,7 +1814,7 @@ MPI job with intelligent error handling: .TP Health check requiring all nodes: .nf -.B bssh --require-all-success -C production "disk-check" +.B bssh --require-all-success --cluster production "disk-check" .B if [ $? -ne 0 ]; then .B alert_ops "One or more nodes unhealthy" .B fi @@ -1771,7 +1823,7 @@ Health check requiring all nodes: .TP Hybrid mode - preserve main exit code but detect failures: .nf -.B bssh --check-all-nodes -C cluster "mpirun ./program" +.B bssh --check-all-nodes --cluster cluster "mpirun ./program" .B EXIT_CODE=$? .B .B if [ $EXIT_CODE -eq 1 ]; then @@ -1785,7 +1837,7 @@ Hybrid mode - preserve main exit code but detect failures: CI/CD pipeline integration (no changes needed): .nf .B # Works exactly like mpirun -.B if bssh -C cluster "mpirun ./tests"; then +.B if bssh --cluster cluster "mpirun ./tests"; then .B echo "Tests passed" .B deploy_to_production .B else @@ -1997,15 +2049,15 @@ Example: BSSH_PASSWORD=mysecret bssh --password -H "user@host" "uptime" .TP .B BSSH_SUDO_PASSWORD Sudo password for automated sudo authentication. When set along with the -.B -S -flag, bssh will use this password instead of prompting interactively. +.B \-\-sudo\-password +option, bssh will use this password instead of prompting interactively. .br .B WARNING: Using environment variables for passwords is not recommended for production use as they may be visible in process listings, shell history, or logs. Prefer the interactive prompt for security-sensitive operations. .br -Example: BSSH_SUDO_PASSWORD=mypassword bssh -S -C cluster "sudo apt update" +Example: BSSH_SUDO_PASSWORD=mypassword bssh --sudo-password --cluster cluster "sudo apt update" .TP .B BSSH_TUI_LOG_MAX_ENTRIES @@ -2015,7 +2067,7 @@ Default: 1000 .br Maximum: 10000 (prevents memory exhaustion) .br -Example: BSSH_TUI_LOG_MAX_ENTRIES=5000 bssh -C cluster "command" +Example: BSSH_TUI_LOG_MAX_ENTRIES=5000 bssh --cluster cluster "command" .TP .B USER @@ -2040,7 +2092,8 @@ Backend.AI node role (main/sub) .TP .B SSH_AUTH_SOCK SSH agent socket path. When set, bssh can automatically detect and use -the SSH agent for authentication without specifying the -A flag +the SSH agent for authentication without specifying +.BR \-\-use\-agent . .SH NOTES .SS Main Rank Detection diff --git a/docs/openssh-regress.md b/docs/openssh-regress.md index 0c4a6a8e..3986323e 100644 --- a/docs/openssh-regress.md +++ b/docs/openssh-regress.md @@ -30,12 +30,12 @@ An upstream `SKIPPED:` result is reported as `skip` and excluded from the eligib The score is `pass / (pass + fail)`. CI compares both the pass count and the environment-valid result count with platform-specific floors in `baseline.json`, uploads the generated JSON table even on failure, and fails when either count drops below its floor. Durations are milliseconds, and `first_failure_line` is the first diagnostic line selected from the captured combined harness output; per-test logs are streamed under `target/openssh-regress/logs/` and capped at 16 MiB each. -At PR head `13c35ce`, after #277 made the suite runnable, Linux measured 25 pass, 42 fail, five environmental, and six upstream skips for 67 eligible results. macOS measured 26 pass, 39 fail, seven environmental, and six upstream skips for 65 eligible results. Both platforms keep a conservative 23-pass floor, preserving the epic's measured pass floor while leaving observed slack. +The complete #275 implementation run recorded in `tests/openssh-regress/results.json` measured 60 pass, nine fail, four environmental, and six upstream skips on Linux: 60/69 eligible tests. The `agent`, `banner`, `connect-uri`, and `portnum` suites account for the final four passes over the preceding 56/69 measurement. Both Linux and macOS now enforce the epic's 60-pass floor; CI supplies the platform-specific verification on each pull request. -The eligible floors are 67 on Linux and 65 on macOS. This recalibrates the denominator from the 78 runnable manifest rows: six upstream skips are now reached and excluded, followed by the existing platform environmental allowances of five on Linux and seven on macOS (`78 - 6 - 5 = 67`; `78 - 6 - 7 = 65`). A nonzero eligible-result floor still rejects a collapsed all-environmental run. Focused `--test` runs report verdicts without enforcing full-suite floors. +The eligible floors remain 67 on Linux and 65 on macOS. The current Linux run produced 69 eligible results from 79 runnable rows after six upstream skips and four environmental results. The lower eligible floors preserve bounded tolerance for platform-specific environmental failures while still rejecting a collapsed all-environmental run. Focused `--test` runs report verdicts without enforcing full-suite floors. ## Candidate inventory -The pinned manifest contains 78 runnable client tests and 11 permanent candidate skips, preserving the epic's 89-test inventory. `forwarding` remains runnable because it covers local, remote, and standard-input forwarding; the out-of-scope X11 and tun-device features do not justify skipping it. `allow-deny-users` is excluded as a pure sshd configuration test, while `sftp-chroot` and `reconfigure` remain reasoned permanent sshd-side skips. +The pinned manifest contains 79 runnable client tests and 11 permanent candidate skips, preserving a 90-test candidate inventory. Another 25 server-only or otherwise out-of-scope rows remain explicitly excluded, accounting for all 115 shell tests in the pinned tree. `forwarding` remains runnable because it covers local, remote, and standard-input forwarding; the out-of-scope X11 and tun-device features do not justify skipping it. `allow-deny-users` is excluded as a pure sshd configuration test, while `sftp-chroot` and `reconfigure` remain reasoned permanent sshd-side skips. -The historical measurement in issue #275 reported 116 shell tests and listed `pubkey-priority` as one of eight environmental failures. The exact `V_10_3_P1` tree contains 115 shell tests, its `LTESTS` inventory has no `pubkey-priority`, and the committed historical `results.json` retains that row so the measured 23 pass, 10 skip, and 56 fail baseline is not silently rewritten. Current runs validate `selection.tsv` against the pinned tree and will reject invented or stale test names. +The historical measurement in issue #275 reported 116 shell tests and listed `pubkey-priority` as an environmental failure. The exact `V_10_3_P1` tree contains 115 shell tests and its `LTESTS` inventory has no `pubkey-priority`. The committed `results.json` now records the complete current 79-row run, while every run validates `selection.tsv` against the pinned tree and rejects invented or stale test names. diff --git a/docs/openssh-short-flags-migration.md b/docs/openssh-short-flags-migration.md new file mode 100644 index 00000000..bdc57da2 --- /dev/null +++ b/docs/openssh-short-flags-migration.md @@ -0,0 +1,94 @@ +# Migrating bssh Short Flags for 3.0 + +bssh 3.0 assigns seven single-letter options their OpenSSH meanings. Existing +bssh automation must use long options for the displaced cluster and pdsh-style +features. + +## Release-status note + +This is the target contract for 3.0. The source tree still reports version +2.4.3 while the compatibility epic is being implemented. The requirement to +warn in a final 2.x release before reassignment is a release-history +requirement: this documentation and the 3.0 implementation do not, by +themselves, prove that such a warning was shipped. Release notes must not mark +that requirement complete without evidence from a published 2.x release. + +## Script rewrites + +| Former bssh use | Rewrite for 3.0 | What the short flag means in 3.0 | +|-----------------|-----------------|----------------------------------| +| `-N` to remove node prefixes | `--no-prefix` | Do not execute a remote command | +| `-f PATTERN` to filter hosts | `--filter PATTERN` | Go to the background after authentication | +| `-C CLUSTER` to select a cluster | `--cluster CLUSTER` | Enable SSH compression | +| `-A` to authenticate with the local agent | `--use-agent` | Forward the authentication agent to the remote host | +| `-S` to prompt for a sudo password | `--sudo-password` | Set the multiplexing `ControlPath`; it now requires a path | +| `-k` to stop on the first failed node | `--fail-fast` | Disable GSSAPI credential delegation | +| `-b` to use single-stage Ctrl+C handling | `--batch` | Bind the client to a source address; it now requires an address | + +For example: + +```bash +# Before 3.0 +bssh -C production -f 'web*' -A -S -k -b 'sudo systemctl restart nginx' + +# 3.0 +bssh --cluster production --filter 'web*' --use-agent \ + --sudo-password --fail-fast --batch 'sudo systemctl restart nginx' +``` + +The authentication meanings of `-A` and `--use-agent` are deliberately +different. `--use-agent` lets bssh use a local agent to authenticate the +connection. OpenSSH-compatible `-A` makes that agent available to processes on +the remote host, which expands the trust boundary and should be enabled only +when needed. + +## OpenSSH-compatible examples + +Use `-N` for a forwarding-only connection: + +```bash +bssh -N -L 8080:internal.example.com:80 user@gateway.example.com +``` + +Use `-S` to select a connection-sharing socket: + +```bash +bssh -M -S ~/.ssh/bssh-%C user@host +bssh -S ~/.ssh/bssh-%C user@host uptime +``` + +Use the long bssh options when selecting clusters or changing multi-node +output behavior: + +```bash +bssh --cluster production --filter 'web*' --no-prefix --stream uptime +bssh --cluster production --fail-fast --batch deploy.sh +``` + +## pdsh compatibility mode + +The pdsh parser remains separate from the native bssh parser. Its established +short options keep their pdsh meanings when any of these activation methods is +used: + +```bash +bssh --pdsh-compat -w host[1-5] -f 2 -N -b -k -S uptime +pdsh -w host[1-5] -f 2 -N -b -k -S uptime +BSSH_PDSH_COMPAT=1 bssh -w host[1-5] -f 2 -N -b -k -S uptime +``` + +In pdsh mode, `-f` is fanout, `-N` removes the hostname prefix, `-b` selects +batch interrupt handling, `-k` is fail-fast, and `-S` returns the largest +remote exit status. The pdsh-compatible parser does not expose bssh's cluster, +agent-authentication, or sudo-password extensions. Use native bssh mode and +the long options `--cluster`, `--use-agent`, and `--sudo-password` for those +features. + +## Migration checklist + +1. Replace all displaced bssh short options with the long forms in the table. +2. Keep pdsh short options only in scripts that explicitly activate pdsh mode. +3. Review every former `-A`: authentication-agent forwarding is more + security-sensitive than local agent authentication. +4. Add `-N` to forwarding-only SSH invocations that should not open a shell. +5. Test wrappers and aliases because `-S` and `-b` now require values. diff --git a/docs/pdsh-examples.md b/docs/pdsh-examples.md index b0eb37f1..ee01c01a 100644 --- a/docs/pdsh-examples.md +++ b/docs/pdsh-examples.md @@ -2,6 +2,11 @@ Real-world examples of common pdsh usage patterns with bssh. +Commands written as `pdsh` use the pdsh-compatible parser. Examples that need +bssh's sudo-password helper are explicitly written as native `bssh` commands, +because pdsh mode does not expose that helper and reserves `-S` for exit-code +selection. + ## Table of Contents - [Basic Operations](#basic-operations) @@ -93,58 +98,58 @@ pdsh -w node[1-10] -x "node[3-5]" -q ### Package Management ```bash -# Update package lists (Ubuntu/Debian) -pdsh -w servers -l root -S "sudo apt update" +# Update package lists with bssh's native sudo helper +bssh -H servers -l root --sudo-password "sudo apt update" # Upgrade packages -pdsh -w servers -l admin -S "sudo apt upgrade -y" +bssh -H servers -l admin --sudo-password "sudo apt upgrade -y" # Install specific package on all hosts -pdsh -w webservers -S "sudo apt install -y nginx" +bssh -H webservers --sudo-password "sudo apt install -y nginx" # Check package version pdsh -w servers "dpkg -l | grep nginx" # Clean package cache -pdsh -w servers -S "sudo apt clean" +bssh -H servers --sudo-password "sudo apt clean" ``` ### Service Management ```bash # Restart service on all web servers -pdsh -w web[1-10] -S "sudo systemctl restart nginx" +bssh -H web[1-10] --sudo-password "sudo systemctl restart nginx" # Check service status pdsh -w app-servers "systemctl status myapp" # Enable service on boot -pdsh -w servers -S "sudo systemctl enable docker" +bssh -H servers --sudo-password "sudo systemctl enable docker" # Stop service on specific hosts -pdsh -w cache[1-3] -S "sudo systemctl stop redis" +bssh -H cache[1-3] --sudo-password "sudo systemctl stop redis" # Reload configuration -pdsh -w webservers -S "sudo systemctl reload nginx" +bssh -H webservers --sudo-password "sudo systemctl reload nginx" ``` ### User Management ```bash # Create user on all hosts -pdsh -w servers -S "sudo useradd -m -s /bin/bash deploy" +bssh -H servers --sudo-password "sudo useradd -m -s /bin/bash deploy" # Set password -pdsh -w servers -S "echo 'deploy:newpassword' | sudo chpasswd" +bssh -H servers --sudo-password "echo 'deploy:newpassword' | sudo chpasswd" # Add user to group -pdsh -w servers -S "sudo usermod -aG docker deploy" +bssh -H servers --sudo-password "sudo usermod -aG docker deploy" # Check user existence pdsh -w servers "id deploy" # Remove user -pdsh -w servers -S "sudo userdel -r olduser" +bssh -H servers --sudo-password "sudo userdel -r olduser" ``` ### File System Operations @@ -160,7 +165,7 @@ pdsh -w servers "du -sh /var/log" pdsh -w servers "find /var/log -type f -size +100M" # Clean up old logs -pdsh -w servers -S "sudo find /var/log -name '*.log' -mtime +30 -delete" +bssh -H servers --sudo-password "sudo find /var/log -name '*.log' -mtime +30 -delete" # Check mount points pdsh -w servers "mount | grep -E '^/dev/'" @@ -197,7 +202,7 @@ pdsh -w app-servers -l deploy "cd /app && git pull origin main" pdsh -w app-servers -l deploy "cd /app && npm install && npm run build" # Restart application -pdsh -w app-servers -S "sudo systemctl restart myapp" +bssh -H app-servers --sudo-password "sudo systemctl restart myapp" # Verify deployment pdsh -w app-servers "curl -s http://localhost:3000/health | jq .version" @@ -216,7 +221,7 @@ for host in $(pdsh -w web[1-5] -q); do done # Update configuration value -pdsh -w app-servers -S "sudo sed -i 's/^PORT=.*/PORT=8080/' /etc/myapp/config" +bssh -H app-servers --sudo-password "sudo sed -i 's/^PORT=.*/PORT=8080/' /etc/myapp/config" # Validate configuration pdsh -w webservers "nginx -t" @@ -238,7 +243,7 @@ for host in $(pdsh -w webservers -q); do done # Update certificate paths in config -pdsh -w webservers -S "sudo systemctl restart nginx" +bssh -H webservers --sudo-password "sudo systemctl restart nginx" # Verify certificates pdsh -w webservers "sudo openssl x509 -in /etc/ssl/cert.pem -noout -dates" @@ -398,14 +403,14 @@ pdsh -w db-servers " ```bash # Rolling restart with fanout=1 (one at a time) -pdsh -w web[1-10] -f 1 -S " +bssh -H web[1-10] --parallel 1 --sudo-password " sudo systemctl restart nginx && sleep 5 && curl -f http://localhost/health " # Update SSL certificates -pdsh -w web[1-5] -S " +bssh -H web[1-5] --sudo-password " sudo certbot renew && sudo systemctl reload nginx " @@ -487,7 +492,7 @@ wait ```bash # Check primary and failover to secondary if down pdsh -w db-primary "pg_isready" || -pdsh -w db-secondary -S "sudo -u postgres pg_ctl promote -D /var/lib/postgresql/data" +bssh -H db-secondary --sudo-password "sudo -u postgres pg_ctl promote -D /var/lib/postgresql/data" # Health check with timeout pdsh -w webservers -u 5 "curl -f -m 3 http://localhost/health" || diff --git a/docs/pdsh-migration.md b/docs/pdsh-migration.md index 38a34b42..0334856f 100644 --- a/docs/pdsh-migration.md +++ b/docs/pdsh-migration.md @@ -340,11 +340,11 @@ clusters: ssh_key: ~/.ssh/prod_key ``` -Then use with `-C` flag: +Then use bssh's native `--cluster` option: ```bash # Using config file -bssh -C production "uptime" +bssh --cluster production "uptime" # Still works with -w pdsh -w web[1-3].example.com "uptime" @@ -371,7 +371,7 @@ pdsh -w web[1-3].example.com "uptime" - [ ] Convert GENDERS files to bssh YAML format - [ ] Migrate cluster definitions to `~/.config/bssh/config.yaml` -- [ ] Test cluster access: `bssh -C "uptime"` +- [ ] Test cluster access: `bssh --cluster "uptime"` - [ ] Configure SSH keys and authentication methods - [ ] Set up any required environment variables @@ -485,8 +485,8 @@ pdsh -w hosts -S "cmd" **Solution**: ```bash -# Enable SSH agent -pdsh -A -w hosts "cmd" +# Explicitly use the SSH agent in native bssh mode +bssh --use-agent -H hosts "cmd" # Use specific SSH key pdsh -i ~/.ssh/key -w hosts "cmd" @@ -522,7 +522,7 @@ HOSTS=$(cat /path/to/hosts | tr '\n' ',' | sed 's/,$//') pdsh -w "$HOSTS" "cmd" # Or use bssh config file -bssh -C cluster-name "cmd" +bssh --cluster cluster-name "cmd" ``` ## Getting Help diff --git a/docs/pdsh-options.md b/docs/pdsh-options.md index 004a513f..c14ba33b 100644 --- a/docs/pdsh-options.md +++ b/docs/pdsh-options.md @@ -124,7 +124,7 @@ pdsh -w compute[001-100] -x "compute[080-100]" "check-gpu.sh" pdsh -g webservers "command" # bssh equivalent -bssh -C webservers "command" +bssh --cluster webservers "command" ``` **Config file** (`~/.config/bssh/config.yaml`): @@ -463,70 +463,70 @@ pdsh -w prod-servers -i ~/.ssh/prod_rsa "deploy.sh" pdsh -w servers -i ~/.ssh/encrypted_key "command" ``` -### `-A` (--use-agent) [bssh Extension] +### `--use-agent` [bssh Native Extension] **pdsh**: N/A (auto-detects agent) -**bssh Native**: `-A` or `--use-agent` +**bssh Native**: `--use-agent` -**pdsh Compat**: `-A` +**pdsh Compat**: Not exposed by the pdsh-compatible parser; use native bssh mode **Syntax**: ```bash -# Use SSH agent -pdsh -A -w hosts "command" +# Use SSH agent in native bssh mode +bssh --use-agent -H hosts "command" ``` **Examples**: ```bash # Force agent authentication -pdsh -A -w secure-hosts "sensitive-operation.sh" +bssh --use-agent -H secure-hosts "sensitive-operation.sh" # Combined with sudo -pdsh -A -S -w servers "sudo apt update" +bssh --use-agent --sudo-password -H servers "sudo apt update" ``` -### `-P` (--password) [bssh Extension] +### `--password` [bssh Native Extension] **pdsh**: N/A -**bssh Native**: `-P` or `--password` +**bssh Native**: `--password` -**pdsh Compat**: `-P` +**pdsh Compat**: Not exposed by the pdsh-compatible parser; use native bssh mode **Behavior**: Prompts for SSH password (not recommended for scripts) **Syntax**: ```bash # Password authentication -pdsh -P -w hosts "command" +bssh --password -H hosts "command" # Prompts: "Enter SSH password:" ``` -### `-S` (--sudo-password) [bssh Extension] +### `--sudo-password` [bssh Native Extension] **pdsh**: N/A -**bssh Native**: `-S` or `--sudo-password` +**bssh Native**: `--sudo-password` -**pdsh Compat**: `-S` +**pdsh Compat**: `-S` means `--any-failure`; the sudo helper is not exposed by the pdsh-compatible parser **Behavior**: Prompts for sudo password and auto-injects it **Syntax**: ```bash # Sudo password injection -pdsh -S -w hosts "sudo apt update" +bssh --sudo-password -H hosts "sudo apt update" # Prompts: "Enter sudo password:" ``` **Examples**: ```bash # System updates with sudo -pdsh -S -w servers "sudo systemctl restart nginx" +bssh --sudo-password -H servers "sudo systemctl restart nginx" # Combined with SSH agent -pdsh -A -S -w hosts "sudo reboot" +bssh --use-agent --sudo-password -H hosts "sudo reboot" ``` ## Query and Information Options @@ -707,7 +707,7 @@ The following pdsh options are **not supported** in bssh: | Option | Description | Alternative | |--------|-------------|-------------| -| `-g ` | Target host group | Use `-C ` with YAML config | +| `-g ` | Target host group | Use native bssh `--cluster ` with YAML config | | `-a` | Target all hosts | Define "all" cluster in config | | `-X ` | Exclude host group | Use `--exclude` with hostlist | @@ -731,19 +731,19 @@ The following pdsh options are **not supported** in bssh: ## bssh-Specific Extensions -These options are available in bssh but not in pdsh: +These options are available in native bssh mode, not through the pdsh-compatible parser: ### Advanced Features | Option | Description | Example | |--------|-------------|---------| -| `-J ` | Jump host (bastion) | `pdsh -J bastion -w internal-hosts "cmd"` | -| `-L ` | Local port forwarding | `pdsh -L 8080:web:80 -w hosts "cmd"` | -| `-R ` | Remote port forwarding | `pdsh -R 80:localhost:8080 -w hosts "cmd"` | -| `-D ` | Dynamic forwarding (SOCKS) | `pdsh -D 1080 -w hosts "cmd"` | -| `-F ` | SSH config file | `pdsh -F ~/.ssh/custom_config -w hosts "cmd"` | -| `-C ` | Use cluster from config | `pdsh -C production "cmd"` | -| `--filter ` | Filter hosts by pattern | `pdsh -w hosts --filter "web*" "cmd"` | +| `-J ` | Jump host (bastion) | `bssh -J bastion -H internal-hosts "cmd"` | +| `-L ` | Local port forwarding | `bssh -L 8080:web:80 -H hosts "cmd"` | +| `-R ` | Remote port forwarding | `bssh -R 80:localhost:8080 -H hosts "cmd"` | +| `-D ` | Dynamic forwarding (SOCKS) | `bssh -D 1080 -H hosts "cmd"` | +| `-F ` | SSH config file | `bssh -F ~/.ssh/custom_config -H hosts "cmd"` | +| `--cluster ` | Use cluster from config in native bssh mode | `bssh --cluster production "cmd"` | +| `--filter ` | Filter hosts in native bssh mode | `bssh -H hosts --filter "web*" "cmd"` | ### Verbosity Levels @@ -765,7 +765,7 @@ pdsh -vv -w problematic-host "command" |--------|-------------| | `--stream` | Stream mode with real-time output | | `--output-dir ` | Save per-host output to files | -| `--no-prefix` | Disable hostname prefix (same as `-N`) | +| `--no-prefix` | Disable hostname prefix (`-N` has this meaning only in pdsh mode) | ### Exit Code Strategies @@ -773,7 +773,7 @@ pdsh -vv -w problematic-host "command" |--------|-------------| | `--require-all-success` | Return 0 only if all hosts succeed | | `--check-all-nodes` | Return main rank code, or 1 if others fail | -| `--any-failure` | Return largest exit code (same as `-S`) | +| `--any-failure` | Return largest exit code (`-S` has this meaning only in pdsh mode) | ## Summary @@ -785,7 +785,7 @@ pdsh -vv -w problematic-host "command" | **Execution** | `-f`, `-b`, `-k` | ✅ Full | Direct mapping | | **Timeouts** | `-t`, `-u` | ✅ Full | Direct mapping | | **Output** | `-N`, `-o` | ✅ Partial | `-N` supported; use `--stream` instead of `-o` | -| **Authentication** | `-l` | ✅ Full | Plus additional `-i`, `-A`, `-P`, `-S` | +| **Authentication** | `-l` | ✅ Full | Native bssh additionally provides `-i`, `--use-agent`, `--password`, and `--sudo-password` | | **Query** | `-q` | ✅ Full | Direct mapping | | **Exit Codes** | `-S` | ✅ Full | Plus additional strategies | | **RCMD Modules** | `-R`, `-M` | ❌ None | SSH-only by design | diff --git a/docs/shell-config/README.md b/docs/shell-config/README.md index 0b0ddb3b..9016297a 100644 --- a/docs/shell-config/README.md +++ b/docs/shell-config/README.md @@ -173,14 +173,14 @@ bssh-info production ```bash # Bash/Zsh -alias bssh-web='bssh -C webservers' -alias bssh-db='bssh -C databases' +alias bssh-web='bssh --cluster webservers' +alias bssh-db='bssh --cluster databases' # Fish -alias bssh-web='bssh -C webservers' -alias bssh-db='bssh -C databases' +alias bssh-web='bssh --cluster webservers' +alias bssh-db='bssh --cluster databases' # Or use abbreviations -abbr --add bsw 'bssh -C webservers' +abbr --add bsw 'bssh --cluster webservers' ``` ### Custom Cluster Groups @@ -198,7 +198,7 @@ BSSH_CLUSTER_GROUPS[monitoring]="monitoring-prod monitoring-staging" ```bash # Bash/Zsh bssh-full-health() { - bssh -C "$1" " + bssh --cluster "$1" " echo '=== System Info ===' && uname -a && echo '=== Uptime ===' && @@ -212,7 +212,7 @@ bssh-full-health() { # Fish function bssh-full-health - bssh -C $argv[1] " + bssh --cluster $argv[1] " echo '=== System Info ===' && uname -a && echo '=== Uptime ===' && diff --git a/docs/shell-config/bash.sh b/docs/shell-config/bash.sh index 22400424..447b0142 100644 --- a/docs/shell-config/bash.sh +++ b/docs/shell-config/bash.sh @@ -17,13 +17,13 @@ alias pdsh='bssh --pdsh-compat' # Create shortcuts for frequently used clusters # Production cluster shortcut -alias bssh-prod='bssh -C production' +alias bssh-prod='bssh --cluster production' # Staging cluster shortcut -alias bssh-staging='bssh -C staging' +alias bssh-staging='bssh --cluster staging' # Development cluster shortcut -alias bssh-dev='bssh -C development' +alias bssh-dev='bssh --cluster development' # ============================================ # Helper Functions @@ -38,7 +38,7 @@ bssh-all() { local cluster="$1" shift - bssh -C "$cluster" "$@" + bssh --cluster "$cluster" "$@" } # Execute command with hostlist expansion @@ -72,7 +72,7 @@ bssh-health() { return 1 fi - bssh -C "$1" "uptime; free -h | grep 'Mem:'; df -h /" + bssh --cluster "$1" "uptime; free -h | grep 'Mem:'; df -h /" } # ============================================ diff --git a/docs/shell-config/fish.fish b/docs/shell-config/fish.fish index 3c076cfd..ade40075 100644 --- a/docs/shell-config/fish.fish +++ b/docs/shell-config/fish.fish @@ -17,13 +17,13 @@ alias pdsh='bssh --pdsh-compat' # Create shortcuts for frequently used clusters # Production cluster shortcut -alias bssh-prod='bssh -C production' +alias bssh-prod='bssh --cluster production' # Staging cluster shortcut -alias bssh-staging='bssh -C staging' +alias bssh-staging='bssh --cluster staging' # Development cluster shortcut -alias bssh-dev='bssh -C development' +alias bssh-dev='bssh --cluster development' # ============================================ # Helper Functions @@ -38,7 +38,7 @@ function bssh-all set cluster $argv[1] set -e argv[1] - bssh -C $cluster $argv + bssh --cluster $cluster $argv end # Execute command with hostlist expansion @@ -72,7 +72,7 @@ function bssh-health return 1 end - bssh -C $argv[1] "uptime; free -h | grep 'Mem:'; df -h /" + bssh --cluster $argv[1] "uptime; free -h | grep 'Mem:'; df -h /" end # Parallel execution with progress tracking @@ -85,7 +85,7 @@ function bssh-parallel set cluster $argv[1] set parallel $argv[2] set -e argv[1..2] - bssh -C $cluster --parallel $parallel $argv + bssh --cluster $cluster --parallel $parallel $argv end # ============================================ @@ -149,7 +149,7 @@ function bssh-ctx return 1 end - bssh -C $BSSH_CURRENT_CLUSTER $argv + bssh --cluster $BSSH_CURRENT_CLUSTER $argv end # ============================================ @@ -182,7 +182,7 @@ function bssh-group for cluster in $clusters echo "===> Running on cluster: $cluster" - bssh -C $cluster $argv + bssh --cluster $cluster $argv end end @@ -218,7 +218,7 @@ function bssh-select echo "Selected: $selected_cluster" if test (count $argv) -gt 0 - bssh -C $selected_cluster $argv + bssh --cluster $selected_cluster $argv else bssh-context $selected_cluster end @@ -237,7 +237,7 @@ function bssh-info echo "Cluster: $argv[1]" echo "Nodes:" - bssh -C $argv[1] -q 2>/dev/null | while read node + bssh --cluster $argv[1] -q 2>/dev/null | while read node echo " - $node" end end diff --git a/docs/shell-config/zsh.sh b/docs/shell-config/zsh.sh index 01691746..4eae0132 100644 --- a/docs/shell-config/zsh.sh +++ b/docs/shell-config/zsh.sh @@ -17,13 +17,13 @@ alias pdsh='bssh --pdsh-compat' # Create shortcuts for frequently used clusters # Production cluster shortcut -alias bssh-prod='bssh -C production' +alias bssh-prod='bssh --cluster production' # Staging cluster shortcut -alias bssh-staging='bssh -C staging' +alias bssh-staging='bssh --cluster staging' # Development cluster shortcut -alias bssh-dev='bssh -C development' +alias bssh-dev='bssh --cluster development' # ============================================ # Helper Functions @@ -38,7 +38,7 @@ bssh-all() { local cluster="$1" shift - bssh -C "$cluster" "$@" + bssh --cluster "$cluster" "$@" } # Execute command with hostlist expansion @@ -72,7 +72,7 @@ bssh-health() { return 1 fi - bssh -C "$1" "uptime; free -h | grep 'Mem:'; df -h /" + bssh --cluster "$1" "uptime; free -h | grep 'Mem:'; df -h /" } # Parallel execution with progress tracking @@ -85,7 +85,7 @@ bssh-parallel() { local cluster="$1" local parallel="$2" shift 2 - bssh -C "$cluster" --parallel "$parallel" "$@" + bssh --cluster "$cluster" --parallel "$parallel" "$@" } # ============================================ @@ -148,7 +148,7 @@ bssh-ctx() { return 1 fi - bssh -C "$BSSH_CURRENT_CLUSTER" "$@" + bssh --cluster "$BSSH_CURRENT_CLUSTER" "$@" } # ============================================ @@ -182,7 +182,7 @@ bssh-group() { for cluster in ${=BSSH_CLUSTER_GROUPS[$group]}; do echo "===> Running on cluster: $cluster" - bssh -C "$cluster" "$@" + bssh --cluster "$cluster" "$@" done } diff --git a/examples/health_check.sh b/examples/health_check.sh index 127ed8af..d93b3295 100755 --- a/examples/health_check.sh +++ b/examples/health_check.sh @@ -27,7 +27,7 @@ echo # Check disk space on all nodes (require all to pass) echo "1. Checking disk space..." -if bssh --require-all-success -C production exec "df -h / | awk 'NR==2 {if (\$5+0 > 90) exit 1}'"; then +if bssh --require-all-success --cluster production exec "df -h / | awk 'NR==2 {if (\$5+0 > 90) exit 1}'"; then echo " ✅ Disk space OK on all nodes" else echo " ❌ CRITICAL: Disk space exceeded on one or more nodes!" @@ -38,7 +38,7 @@ fi # Check memory usage echo echo "2. Checking memory usage..." -if bssh --require-all-success -C production exec "free | awk '/Mem:/ {if (\$3/\$2 > 0.95) exit 1}'"; then +if bssh --require-all-success --cluster production exec "free | awk '/Mem:/ {if (\$3/\$2 > 0.95) exit 1}'"; then echo " ✅ Memory usage OK on all nodes" else echo " ❌ WARNING: High memory usage detected!" @@ -49,7 +49,7 @@ fi # Check critical services echo echo "3. Checking critical services..." -if bssh --require-all-success -C production exec "systemctl is-active docker nginx"; then +if bssh --require-all-success --cluster production exec "systemctl is-active docker nginx"; then echo " ✅ All services running on all nodes" else echo " ❌ CRITICAL: Service failure detected!" @@ -60,7 +60,7 @@ fi # Check network connectivity echo echo "4. Checking network connectivity..." -if bssh --require-all-success -C production exec "ping -c 1 -W 1 8.8.8.8 > /dev/null"; then +if bssh --require-all-success --cluster production exec "ping -c 1 -W 1 8.8.8.8 > /dev/null"; then echo " ✅ Network connectivity OK on all nodes" else echo " ❌ CRITICAL: Network connectivity issue!" @@ -71,7 +71,7 @@ fi # Check GPU status (if applicable) echo echo "5. Checking GPU status..." -if bssh --require-all-success -C production exec "nvidia-smi > /dev/null 2>&1"; then +if bssh --require-all-success --cluster production exec "nvidia-smi > /dev/null 2>&1"; then echo " ✅ GPUs operational on all nodes" else echo " ⚠️ WARNING: GPU check failed (may not have GPUs)" diff --git a/examples/interactive_demo.rs b/examples/interactive_demo.rs index 8b6aad48..c19f1cc6 100644 --- a/examples/interactive_demo.rs +++ b/examples/interactive_demo.rs @@ -63,6 +63,7 @@ async fn main() -> anyhow::Result<()> { use_pty: None, session_policy: None, ssh_connection_config: SshConnectionConfig::default(), + ssh_connection_config_resolver: None, }; println!("Starting interactive session..."); diff --git a/examples/mpi_exit_code.sh b/examples/mpi_exit_code.sh index b179a188..ee991933 100755 --- a/examples/mpi_exit_code.sh +++ b/examples/mpi_exit_code.sh @@ -29,7 +29,7 @@ set -euo pipefail # Run MPI simulation across cluster echo "Running MPI simulation..." -bssh -C production exec "mpirun -n 16 ./simulation --config production.yaml" +bssh --cluster production exec "mpirun -n 16 ./simulation --config production.yaml" EXIT_CODE=$? # Handle different exit codes appropriately @@ -53,14 +53,14 @@ case $EXIT_CODE in echo " Current memory: 64GB" echo " Retrying with increased memory allocation..." # Retry with more memory - bssh -C production exec "mpirun -n 16 --bind-to none ./simulation --config production.yaml --memory 128g" + bssh --cluster production exec "mpirun -n 16 --bind-to none ./simulation --config production.yaml --memory 128g" ;; 124) echo "⚠️ Timeout detected!" echo " Extending time limit and retrying..." # Retry with extended timeout - bssh --timeout 1200 -C production exec "mpirun -n 16 ./simulation --config production.yaml" + bssh --timeout 1200 --cluster production exec "mpirun -n 16 ./simulation --config production.yaml" ;; *) diff --git a/src/app/background.rs b/src/app/background.rs new file mode 100644 index 00000000..b099b83f --- /dev/null +++ b/src/app/background.rs @@ -0,0 +1,333 @@ +// Copyright 2025 Lablup Inc. and Jeongkyu Shin +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. + +//! Safe background-process supervision for SSH-compatible invocations. +//! +//! Tokio and russh may own worker threads by the time authentication finishes, +//! so calling `fork(2)` at that point would leave the child with an invalid +//! snapshot of their synchronization state. The foreground process instead +//! resolves enough configuration to decide whether detachment is needed, then +//! re-executes bssh before creating the SSH transport. The worker authenticates; +//! the supervisor either waits for its exit or becomes the foreground passenger +//! of a persistent control master. + +use std::path::PathBuf; + +#[cfg(unix)] +use std::io::Write as _; +#[cfg(unix)] +use std::path::Path; +#[cfg(unix)] +use std::process::{ExitStatus, Stdio}; + +use anyhow::{Context, Result}; +use bssh::ssh::{SessionPolicy, control::SessionOpenRequest}; +use serde::{Deserialize, Serialize}; + +#[cfg(unix)] +use bssh::ssh::control::{AttachOutcome, attach_session, verify_same_user}; +#[cfg(unix)] +use tokio::io::{AsyncReadExt as _, AsyncWriteExt as _}; +#[cfg(unix)] +use tokio::net::{UnixListener, UnixStream}; + +const WORKER_SOCKET_ENV: &str = "BSSH_INTERNAL_BACKGROUND_SOCKET"; +const WORKER_PARENT_ENV: &str = "BSSH_INTERNAL_BACKGROUND_PARENT"; +#[cfg(unix)] +const MAX_EVENT_BYTES: usize = 1024 * 1024; + +/// A lifecycle transition sent by the authenticated worker to its supervisor. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[non_exhaustive] +pub enum BackgroundEvent { + /// `-f`: authentication and forwarding setup completed successfully. + Detached { exit_code: i32 }, + /// A ControlPersist master is ready; the supervisor owns the initial + /// foreground session while the worker keeps the authenticated transport. + PersistentMaster { + control_path: PathBuf, + session_request: Box, + invoking_policy: SessionPolicy, + }, +} + +/// Worker-side notification channel, present only in the re-executed process. +#[derive(Debug, Clone)] +pub struct BackgroundWorker { + socket_path: PathBuf, +} + +impl BackgroundWorker { + /// Discover a worker launched by this process's direct parent. + /// + /// Checking the recorded parent PID prevents LocalCommand, ProxyCommand, + /// and other descendants from accidentally inheriting the internal role. + pub fn from_environment() -> Result> { + let Some(socket_path) = std::env::var_os(WORKER_SOCKET_ENV) else { + return Ok(None); + }; + let recorded_parent = std::env::var(WORKER_PARENT_ENV) + .context("Background worker is missing its parent PID")? + .parse::() + .context("Background worker parent PID is invalid")?; + if recorded_parent != parent_process_id() { + return Ok(None); + } + Ok(Some(Self { + socket_path: PathBuf::from(socket_path), + })) + } + + /// Detach the authenticated worker and report readiness to the supervisor. + #[cfg(unix)] + pub async fn detach(&self, event: &BackgroundEvent) -> Result<()> { + // Connect before detaching so setup errors remain visible on the + // foreground terminal and an absent supervisor is never mistaken for + // successful backgrounding. + let mut stream = UnixStream::connect(&self.socket_path) + .await + .with_context(|| { + format!( + "Could not connect to background supervisor '{}'", + self.socket_path.display() + ) + })?; + detach_process()?; + write_event(&mut stream, event).await?; + stream.shutdown().await?; + Ok(()) + } + + #[cfg(not(unix))] + pub async fn detach(&self, _event: &BackgroundEvent) -> Result<()> { + let _ = &self.socket_path; + anyhow::bail!("background-after-authentication currently requires Unix") + } +} + +/// Re-execute a single-destination SSH invocation and proxy its lifecycle. +#[cfg(unix)] +pub async fn supervise(args: &[String]) -> Result { + let socket_dir = create_socket_directory()?; + let socket_path = socket_dir.join("ready.sock"); + let listener = UnixListener::bind(&socket_path).with_context(|| { + format!( + "Could not bind background supervisor socket '{}'", + socket_path.display() + ) + })?; + set_owner_only(&socket_path, 0o600)?; + let _cleanup = SocketDirectoryGuard { + socket_path: socket_path.clone(), + directory: socket_dir, + }; + + let executable = std::env::current_exe().context("Could not locate the bssh executable")?; + let mut command = tokio::process::Command::new(executable); + command + .args(args.iter().skip(1)) + .env(WORKER_SOCKET_ENV, &socket_path) + .env(WORKER_PARENT_ENV, std::process::id().to_string()) + // The worker remains the foreground SSH client for ordinary calls, so + // piped input must survive the re-exec. `-f` replaces this descriptor + // with /dev/null only after authentication has completed. + .stdin(Stdio::inherit()) + .stdout(Stdio::inherit()) + .stderr(Stdio::inherit()) + .kill_on_drop(false); + let mut child = command + .spawn() + .context("Could not start background worker")?; + + tokio::select! { + status = child.wait() => exit_status_code(status.context("Could not wait for bssh worker")?), + accepted = listener.accept() => { + let (mut stream, _) = accepted.context("Could not accept background worker readiness")?; + verify_same_user(&stream).context("Rejected background worker owned by another user")?; + let event = read_event(&mut stream).await?; + match event { + BackgroundEvent::Detached { exit_code } => Ok(exit_code), + BackgroundEvent::PersistentMaster { + control_path, + session_request, + invoking_policy, + } => match attach_session(&control_path, *session_request, &invoking_policy).await? { + AttachOutcome::ExitStatus(status) => Ok(i32::try_from(status).unwrap_or(255)), + AttachOutcome::NoMaster => anyhow::bail!( + "Persistent control master disappeared before the initial session attached" + ), + }, + } + } + } +} + +#[cfg(not(unix))] +pub async fn supervise(_args: &[String]) -> Result { + anyhow::bail!("background-after-authentication currently requires Unix") +} + +#[cfg(unix)] +async fn write_event(stream: &mut UnixStream, event: &BackgroundEvent) -> Result<()> { + let payload = serde_json::to_vec(event).context("Could not encode background event")?; + anyhow::ensure!( + payload.len() <= MAX_EVENT_BYTES, + "Background event exceeds {MAX_EVENT_BYTES} bytes" + ); + let length = u32::try_from(payload.len()).context("Background event length overflow")?; + stream.write_u32(length).await?; + stream.write_all(&payload).await?; + Ok(()) +} + +#[cfg(unix)] +async fn read_event(stream: &mut UnixStream) -> Result { + let length = usize::try_from(stream.read_u32().await?) + .context("Background event length is not representable")?; + anyhow::ensure!( + length <= MAX_EVENT_BYTES, + "Background event exceeds {MAX_EVENT_BYTES} bytes" + ); + let mut payload = vec![0; length]; + stream.read_exact(&mut payload).await?; + serde_json::from_slice(&payload).context("Could not decode background event") +} + +#[cfg(unix)] +fn create_socket_directory() -> Result { + use std::os::unix::fs::DirBuilderExt as _; + + let directory = std::env::temp_dir().join(format!( + "bssh-background-{}-{}", + std::process::id(), + uuid::Uuid::new_v4() + )); + let mut builder = std::fs::DirBuilder::new(); + builder.mode(0o700); + builder + .create(&directory) + .with_context(|| format!("Could not create '{}'", directory.display()))?; + Ok(directory) +} + +#[cfg(unix)] +fn set_owner_only(path: &Path, mode: u32) -> Result<()> { + use std::os::unix::fs::PermissionsExt as _; + + std::fs::set_permissions(path, std::fs::Permissions::from_mode(mode)) + .with_context(|| format!("Could not restrict permissions on '{}'", path.display())) +} + +#[cfg(unix)] +fn detach_process() -> Result<()> { + use std::os::fd::AsRawFd as _; + + nix::unistd::setsid().context("Could not create background process session")?; + std::io::stdout().flush().ok(); + std::io::stderr().flush().ok(); + let null = std::fs::OpenOptions::new() + .read(true) + .write(true) + .open("/dev/null") + .context("Could not open /dev/null for background process")?; + for descriptor in [libc::STDIN_FILENO, libc::STDOUT_FILENO, libc::STDERR_FILENO] { + // SAFETY: `null` remains open for the entire loop, and each target is a + // conventional process-owned standard descriptor. `dup2` atomically + // replaces only that descriptor and does not copy Tokio/russh memory. + if unsafe { libc::dup2(null.as_raw_fd(), descriptor) } == -1 { + return Err(std::io::Error::last_os_error()) + .context("Could not redirect background process standard I/O"); + } + } + Ok(()) +} + +#[cfg(unix)] +fn exit_status_code(status: ExitStatus) -> Result { + if let Some(code) = status.code() { + return Ok(code); + } + #[cfg(unix)] + { + use std::os::unix::process::ExitStatusExt as _; + + if let Some(signal) = status.signal() { + return Ok(128 + signal); + } + } + anyhow::bail!("bssh worker terminated without an exit status") +} + +#[cfg(unix)] +fn parent_process_id() -> u32 { + u32::try_from(nix::unistd::getppid().as_raw()).unwrap_or_default() +} + +#[cfg(not(unix))] +fn parent_process_id() -> u32 { + 0 +} + +#[cfg(unix)] +struct SocketDirectoryGuard { + socket_path: PathBuf, + directory: PathBuf, +} + +#[cfg(unix)] +impl Drop for SocketDirectoryGuard { + fn drop(&mut self) { + if let Err(error) = std::fs::remove_file(&self.socket_path) + && error.kind() != std::io::ErrorKind::NotFound + { + tracing::debug!(path = %self.socket_path.display(), "Could not remove supervisor socket: {error}"); + } + if let Err(error) = std::fs::remove_dir(&self.directory) + && error.kind() != std::io::ErrorKind::NotFound + { + tracing::debug!(path = %self.directory.display(), "Could not remove supervisor directory: {error}"); + } + } +} + +#[cfg(all(test, unix))] +mod tests { + use super::*; + use bssh::ssh::SessionRequest; + + #[tokio::test] + async fn event_frame_round_trips_and_preserves_session_policy() { + let (mut writer, mut reader) = UnixStream::pair().expect("unix stream pair"); + let event = BackgroundEvent::PersistentMaster { + control_path: PathBuf::from("/tmp/control-test"), + session_request: Box::new( + SessionOpenRequest::new( + SessionPolicy { + environment: vec![("LANG".into(), "C".into())], + local_command: None, + forward_agent: false, + request_pty: false, + stdin_null: true, + request: SessionRequest::Exec("true".into()), + }, + None, + ) + .expect("valid session request"), + ), + invoking_policy: SessionPolicy { + environment: Vec::new(), + local_command: Some("true".into()), + forward_agent: false, + request_pty: false, + stdin_null: false, + request: SessionRequest::Exec("true".into()), + }, + }; + let expected = event.clone(); + let write = tokio::spawn(async move { write_event(&mut writer, &event).await }); + assert_eq!(read_event(&mut reader).await.unwrap(), expected); + write.await.unwrap().unwrap(); + } +} diff --git a/src/app/dispatcher.rs b/src/app/dispatcher.rs index a964f8a4..6412bf4f 100644 --- a/src/app/dispatcher.rs +++ b/src/app/dispatcher.rs @@ -35,7 +35,8 @@ use bssh::{ control::{ AttachOutcome, ControlCommand, ControlPathContext, ControlPolicy, ControlResponseKind, SessionOpenRequest, attach_session, connect_control_socket, expand_control_path, - remove_stale_control_socket, send_control_command, start_control_master, + prepare_attached_session, remove_stale_control_socket, send_control_command, + start_control_master_with_bootstrap_session, }, tokio_client::{AddressFamily, ProxyMode, SshConnectionConfigResolver}, }, @@ -43,7 +44,9 @@ use bssh::{ use std::io::IsTerminal; use std::path::{Path, PathBuf}; use std::sync::Arc; +use std::time::Duration; +use super::background::{BackgroundEvent, BackgroundWorker}; #[cfg(target_os = "macos")] use super::initialization::determine_use_keychain; use super::initialization::{AppContext, determine_ssh_key_path}; @@ -72,6 +75,7 @@ fn build_ssh_connection_config_resolver( .with_yaml_keepalive_interval(ctx.config.get_server_alive_interval(cluster_name)) .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_quiet(cli.quiet) .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)) @@ -88,6 +92,7 @@ fn build_ssh_connection_config_resolver( struct ResolvedControlInvocation { policy: ControlPolicy, path: PathBuf, + fork_after_authentication: bool, session_policy: SessionPolicy, session_request: SessionOpenRequest, forwarding_directives: Vec, @@ -95,6 +100,57 @@ struct ResolvedControlInvocation { jump_spec: Option, } +#[derive(Debug, Clone)] +struct ResolvedDirectSession { + policy: SessionPolicy, + fork_after_authentication: bool, + jump_spec: Option, +} + +fn resolve_direct_session( + cli: &Cli, + ctx: &AppContext, + command: &str, +) -> Result { + let node = ctx + .nodes + .first() + .context("SSH session requires a destination node")?; + let effective = ctx.ssh_config.find_host_config(node.config_host()); + let resolver = build_ssh_connection_config_resolver( + cli, + ctx, + ctx.cluster_name.as_deref().or(cli.cluster.as_deref()), + ); + let resolved_connection = resolver.resolve_for_host(node.config_host()); + let jump_spec = + session_policy_jump_spec(resolved_connection.proxy_mode.as_ref()).map(str::to_string); + let mut policy = SessionPolicy::resolve_with_jump_spec( + &effective, + node, + (!command.is_empty()).then_some(command), + cli_tty_mode(cli), + std::io::stdin().is_terminal(), + jump_spec.as_deref(), + )?; + let fork_after_authentication = effective.fork_after_authentication.unwrap_or(false); + if fork_after_authentication { + anyhow::ensure!( + !matches!(policy.request, SessionRequest::Shell), + "Cannot fork into background without a command to execute" + ); + // ForkAfterAuthentication has the same remote-input semantics as -n, + // including when it came from ssh_config rather than the CLI. + policy.stdin_null = true; + policy.request_pty = false; + } + Ok(ResolvedDirectSession { + policy, + fork_after_authentication, + jump_spec, + }) +} + fn resolve_control_invocation( cli: &Cli, ctx: &AppContext, @@ -149,7 +205,7 @@ fn resolve_control_invocation( path_context = path_context.with_jump_host(jump); } let path = expand_control_path(template, &path_context)?; - let session_policy = SessionPolicy::resolve_with_jump_spec( + let mut session_policy = SessionPolicy::resolve_with_jump_spec( &effective, node, (!command.is_empty()).then_some(command), @@ -157,6 +213,15 @@ fn resolve_control_invocation( std::io::stdin().is_terminal(), jump_spec.as_deref(), )?; + let fork_after_authentication = effective.fork_after_authentication.unwrap_or(false); + if fork_after_authentication { + anyhow::ensure!( + !matches!(session_policy.request, SessionRequest::Shell), + "Cannot fork into background without a command to execute" + ); + session_policy.stdin_null = true; + session_policy.request_pty = false; + } let mut remote_policy = session_policy.clone(); remote_policy.local_command = None; let terminal = remote_policy @@ -166,6 +231,7 @@ fn resolve_control_invocation( Ok(Some(ResolvedControlInvocation { policy, path, + fork_after_authentication, session_policy, session_request, forwarding_directives: resolved_connection.forwarding_plan.directives.clone(), @@ -177,6 +243,7 @@ fn resolve_control_invocation( async fn try_existing_control_master( cli: &Cli, control: &ResolvedControlInvocation, + background_worker: Option<&BackgroundWorker>, ) -> Result> { if let Some(command) = cli.control_command.as_deref() { let command = command.parse::()?; @@ -199,6 +266,25 @@ async fn try_existing_control_master( if !control.policy.master.tries_existing() { return Ok(None); } + if control.fork_after_authentication { + let Some(attached) = prepare_attached_session( + &control.path, + control.session_request.clone(), + &control.session_policy, + ) + .await? + else { + return Ok(None); + }; + background_worker + .context("-f requires the supervised SSH worker")? + .detach(&BackgroundEvent::Detached { exit_code: 0 }) + .await?; + return attached + .finish() + .await + .map(|status| Some(i32::try_from(status).unwrap_or(255))); + } match attach_session( &control.path, control.session_request.clone(), @@ -287,6 +373,7 @@ async fn handle_control_master( ctx: &AppContext, control: &ResolvedControlInvocation, ssh_password: Option>, + background_worker: Option<&BackgroundWorker>, ) -> Result { anyhow::ensure!( control.policy.master.creates_master(), @@ -326,10 +413,14 @@ async fn handle_control_master( }; let mut ssh_client = SshClient::new(node.host.clone(), node.port, node.username.clone()); let client = ssh_client.connect_authenticated(&connection).await?; - let master = match start_control_master( + let bootstrap_session = !control.fork_after_authentication + && control.policy.persist.is_enabled() + && background_worker.is_some(); + let master = match start_control_master_with_bootstrap_session( &control.path, client.clone(), control.policy.master.requires_confirmation(), + bootstrap_session, ) { Ok(master) => master, Err(error) => { @@ -338,7 +429,32 @@ async fn handle_control_master( } }; let initial_request_was_none = matches!(control.session_policy.request, SessionRequest::None); - let status = match execute_initial_control_session(&client, &control.session_policy).await { + if control.fork_after_authentication { + control.session_policy.run_local_command().await?; + background_worker + .context("-f requires the supervised SSH worker")? + .detach(&BackgroundEvent::Detached { exit_code: 0 }) + .await?; + } else if control.policy.persist.is_enabled() + && let Some(background_worker) = background_worker + { + background_worker + .detach(&BackgroundEvent::PersistentMaster { + control_path: control.path.clone(), + session_request: Box::new(control.session_request.clone()), + invoking_policy: control.session_policy.clone(), + }) + .await?; + master + .finish_after_initial(control.policy.persist, true) + .await?; + return Ok(EXIT_SUCCESS); + } + let mut execution_policy = control.session_policy.clone(); + if control.fork_after_authentication { + execution_policy.local_command = None; + } + let status = match execute_initial_control_session(&client, &execution_policy).await { Ok(status) => status, Err(error) => { if let Err(shutdown_error) = master.shutdown_immediately().await { @@ -355,6 +471,92 @@ async fn handle_control_master( Ok(i32::try_from(status).unwrap_or(255)) } +async fn handle_direct_single_session( + cli: &Cli, + ctx: &AppContext, + resolved_session: &ResolvedDirectSession, + ssh_password: Option>, + background_worker: Option<&BackgroundWorker>, +) -> Result { + let node = ctx + .nodes + .first() + .context("SSH session requires a destination node")?; + let effective_cluster_name = ctx.cluster_name.as_deref().or(cli.cluster.as_deref()); + let resolver = build_ssh_connection_config_resolver(cli, ctx, effective_cluster_name); + let resolved_connection = resolver.resolve_for_host(node.config_host()); + let key_path = determine_ssh_key_path( + cli, + &ctx.config, + &ctx.ssh_config, + Some(node.config_host()), + effective_cluster_name, + ); + #[cfg(target_os = "macos")] + let use_keychain = determine_use_keychain(&ctx.ssh_config, Some(node.config_host())); + let connection = ConnectionConfig { + key_path: key_path.as_deref(), + strict_mode: Some(ctx.strict_mode), + use_agent: cli.use_agent, + use_password: cli.password, + #[cfg(target_os = "macos")] + use_keychain, + timeout_seconds: cli.timeout, + connect_timeout_seconds: Some(cli.connect_timeout), + jump_hosts_spec: resolved_session.jump_spec.as_deref(), + ssh_connection_config: Some(&resolved_connection), + ssh_connection_config_resolver: Some(&resolver), + session_policy: Some(&resolved_session.policy), + ssh_password, + }; + let mut ssh_client = SshClient::new(node.host.clone(), node.port, node.username.clone()); + let client = ssh_client.connect_authenticated(&connection).await?; + + let operation = async { + resolved_session.policy.run_local_command().await?; + if resolved_session.fork_after_authentication { + background_worker + .context("-f requires the supervised SSH worker")? + .detach(&BackgroundEvent::Detached { exit_code: 0 }) + .await?; + } + if matches!(resolved_session.policy.request, SessionRequest::None) { + return wait_for_no_session_transport(&client).await; + } + let mut remote_policy = resolved_session.policy.clone(); + remote_policy.local_command = None; + execute_initial_control_session(&client, &remote_policy).await + } + .await; + let disconnect = client.disconnect().await; + match (operation, disconnect) { + (Ok(status), Ok(())) => Ok(i32::try_from(status).unwrap_or(255)), + (Err(error), Ok(())) => Err(error), + (Ok(_), Err(error)) => Err(error).context("Could not disconnect SSH transport"), + (Err(error), Err(disconnect_error)) => { + tracing::warn!("SSH operation failed and disconnect also failed: {disconnect_error}"); + Err(error) + } + } +} + +async fn wait_for_no_session_transport(client: &bssh::ssh::tokio_client::Client) -> Result { + loop { + tokio::select! { + signal = tokio::signal::ctrl_c() => { + signal.context("Could not listen for Ctrl-C while keeping the SSH transport open")?; + return Ok(0); + } + () = tokio::time::sleep(Duration::from_secs(1)) => { + anyhow::ensure!( + !client.is_closed(), + "SSH transport closed while no remote session was requested" + ); + } + } + } +} + /// Decide whether `-S` (sudo-password) is meaningful for the given dispatch path. /// /// Only command execution can consume `SudoPassword`, because it monitors @@ -441,14 +643,41 @@ fn subcommand_name(command: &Option) -> &'static str { } } -/// Dispatch commands to their appropriate handlers. +/// Dispatch commands without a background supervisor. /// /// Returns the exit code the process should report. Most subcommands either /// succeed (0) or return `Err`, but `ping` completes normally while still /// having per-host failures to report, so the count has to survive the return. -/// The caller (`main`) is the single place that turns a nonzero value into the -/// process exit status. +#[allow(dead_code)] pub async fn dispatch_command(cli: &Cli, ctx: &AppContext) -> Result { + dispatch_command_with_background(cli, ctx, None).await +} + +/// Whether this initialized single-destination invocation may detach itself. +/// +/// This check deliberately runs after ssh_config and CLI overrides have been +/// resolved. Ordinary SSH sessions stay in the original process; only `-f` or +/// a master that must outlive its initial ControlPersist passenger pays the +/// self-reexec supervision cost. +pub fn requires_background_supervision(cli: &Cli, ctx: &AppContext) -> Result { + if !cli.is_ssh_mode() || cli.control_command.is_some() { + return Ok(false); + } + let command = cli.get_command(); + if let Some(control) = resolve_control_invocation(cli, ctx, &command)? { + return Ok(control.fork_after_authentication + || (control.policy.master.creates_master() && control.policy.persist.is_enabled())); + } + resolve_direct_session(cli, ctx, &command).map(|session| session.fork_after_authentication) +} + +/// Dispatch commands with the optional supervisor channel used by `-f` and +/// ControlPersist. +pub async fn dispatch_command_with_background( + cli: &Cli, + ctx: &AppContext, + background_worker: Option<&BackgroundWorker>, +) -> Result { // Get command to execute let command = cli.get_command(); @@ -487,10 +716,31 @@ pub async fn dispatch_command(cli: &Cli, ctx: &AppContext) -> Result { // invocations reuse the master's one authenticated transport. let control = resolve_control_invocation(cli, ctx, &command)?; if let Some(control) = control.as_ref() - && let Some(exit_code) = try_existing_control_master(cli, control).await? + && let Some(exit_code) = + try_existing_control_master(cli, control, background_worker).await? { return Ok(exit_code); } + let direct_session = if cli.is_ssh_mode() + && cli.stdio_forward.is_none() + && control + .as_ref() + .is_none_or(|control| !control.policy.master.creates_master()) + { + Some(resolve_direct_session(cli, ctx, &command)?) + } else { + None + }; + #[cfg(not(unix))] + if control + .as_ref() + .is_some_and(|control| control.fork_after_authentication) + || direct_session + .as_ref() + .is_some_and(|session| session.fork_after_authentication) + { + anyhow::bail!("ForkAfterAuthentication currently requires Unix"); + } // Calculate hostname for SSH config integration before deciding whether an // up-front password prompt is permitted by every actual target policy. @@ -530,6 +780,20 @@ pub async fn dispatch_command(cli: &Cli, ctx: &AppContext) -> Result { .map_err(|error| anyhow::anyhow!("Failed to collect SSH password: {error}")) })?; + if let Some(direct_session) = direct_session.as_ref() + && (direct_session.fork_after_authentication + || matches!(direct_session.policy.request, SessionRequest::None)) + { + return handle_direct_single_session( + cli, + ctx, + direct_session, + ssh_password, + background_worker, + ) + .await; + } + match &cli.command { Some(Commands::List) => { list_clusters(&ctx.config); @@ -678,7 +942,8 @@ pub async fn dispatch_command(cli: &Cli, ctx: &AppContext) -> Result { if let Some(control) = control.as_ref() && control.policy.master.creates_master() { - return handle_control_master(cli, ctx, control, ssh_password).await; + return handle_control_master(cli, ctx, control, ssh_password, background_worker) + .await; } // Execute command (auto-exec or interactive shell). This path owns // its own exit code strategy (`ExitCodeStrategy`, selected by diff --git a/src/app/mod.rs b/src/app/mod.rs index c71bfd33..476a160a 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -17,6 +17,7 @@ //! This module provides the core application logic, command dispatching, //! initialization, and utility functions for the bssh CLI. +pub mod background; pub mod cache; pub mod config_dump; pub mod dispatcher; diff --git a/src/app/query.rs b/src/app/query.rs index 9a8e85c0..8a03e7cf 100644 --- a/src/app/query.rs +++ b/src/app/query.rs @@ -18,11 +18,12 @@ use bssh::diagnosticln as eprintln; pub fn is_supported_query(query: &str) -> bool { matches!( - query, + query.to_ascii_lowercase().as_str(), "cipher" | "cipher-auth" | "mac" | "kex" + | "kexalgorithms" | "key" | "key-plain" | "key-cert" @@ -34,7 +35,7 @@ pub fn is_supported_query(query: &str) -> bool { /// Handle SSH query options (-Q) pub fn handle_query(query: &str) { - match query { + match query.to_ascii_lowercase().as_str() { "cipher" => { println!( "{}", @@ -51,7 +52,7 @@ pub fn handle_query(query: &str) { bssh::ssh::tokio_client::supported_mac_names().join("\n") ); } - "kex" => { + "kex" | "kexalgorithms" => { println!("curve25519-sha256\ncurve25519-sha256@libssh.org"); println!("ecdh-sha2-nistp256\necdh-sha2-nistp384\necdh-sha2-nistp521"); } @@ -79,3 +80,15 @@ pub fn handle_query(query: &str) { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn openssh_kex_algorithms_keyword_is_a_case_insensitive_query_alias() { + for query in ["kex", "KexAlgorithms", "kexalgorithms"] { + assert!(is_supported_query(query)); + } + } +} diff --git a/src/app/utils.rs b/src/app/utils.rs index a0d0df9f..d58f3935 100644 --- a/src/app/utils.rs +++ b/src/app/utils.rs @@ -18,10 +18,12 @@ use std::time::Duration; /// Show concise usage message (like SSH) pub fn show_usage() { - println!("usage: bssh [-46AqtTvx] [-C cluster] [-F ssh_configfile] [-H hosts]"); + println!("usage: bssh [-46ACNfkMqtTvx] [-b bind_address] [-S control_path]"); + println!(" [-F ssh_configfile] [-H hosts] [--cluster cluster]"); println!(" [-i identity_file] [-J destination] [-l login_name]"); println!(" [-o option] [-p port] [--config config] [--parallel N]"); - println!(" [--output-dir dir] [--timeout seconds] [--use-agent]"); + println!(" [--filter pattern] [--output-dir dir] [--timeout seconds]"); + println!(" [--use-agent] [--sudo-password] [--fail-fast] [--batch]"); println!(" destination [command [argument ...]]"); println!(" bssh [-Q query_option]"); println!(" bssh [list|ping|upload|download|interactive] ..."); diff --git a/src/cli/bssh.rs b/src/cli/bssh.rs index d96e9b32..b0915567 100644 --- a/src/cli/bssh.rs +++ b/src/cli/bssh.rs @@ -25,8 +25,8 @@ use super::ssh_args::StdioForwardTarget; disable_version_flag = true, before_help = "\n\nBroadcast SSH - Parallel command execution across cluster nodes", about = "Broadcast SSH - SSH-compatible parallel command execution tool", - long_about = "bssh is a high-performance SSH client with parallel execution capabilities.\nIt can be used as a drop-in replacement for SSH (single host) or as a powerful cluster management tool (multiple hosts).\n\nThe tool provides secure file transfer using SFTP and supports SSH keys, SSH agent, and password authentication.\nIt automatically detects Backend.AI multi-node session environments.\n\nOutput Modes:\n- TUI Mode (default): Interactive terminal UI with real-time monitoring (auto-enabled in terminals)\n- Stream Mode (--stream): Real-time output with [node] prefixes\n- File Mode (--output-dir): Save per-node output to timestamped files\n- Normal Mode: Traditional output after all nodes complete\n\nSSH Configuration Support:\n- Reads standard SSH config files (defaulting to ~/.ssh/config)\n- Supports Host patterns, HostName, User, Port, IdentityFile, StrictHostKeyChecking\n- ProxyJump, and many other SSH configuration directives\n- CLI arguments override SSH config values following SSH precedence rules", - after_help = "EXAMPLES:\n SSH Mode:\n bssh user@host # Interactive shell\n bssh admin@server.com \"uptime\" # Execute command\n bssh -p 2222 -i ~/.ssh/key user@host # Custom port and key\n bssh -F ~/.ssh/myconfig webserver # Use custom SSH config\n\n Port Forwarding:\n bssh -L 8080:example.com:80 user@host # Local forward: localhost:8080 -> example.com:80\n bssh -R 8080:localhost:80 user@host # Remote forward: remote:8080 -> localhost:80\n bssh -D 1080 user@host # SOCKS5 proxy on localhost:1080\n bssh -L 3306:db:3306 -R 80:web:80 user@host # Multiple forwards\n bssh -D *:1080/4 user@host # SOCKS4 proxy on all interfaces\n\n Multi-Server Mode:\n bssh -C production \"systemctl status\" # Execute on cluster (TUI mode auto-enabled)\n bssh -H \"web1,web2,web3\" \"df -h\" # Execute on multiple hosts\n bssh -H \"web1,web2,web3\" -f \"web1\" \"df -h\" # Filter to web1 only\n bssh -C production -f \"web*\" \"uptime\" # Filter cluster nodes\n bssh --parallel 20 -H web* \"apt update\" # Increase parallelism\n\n Hostlist Expression (pdsh-style range expansion):\n bssh -H \"node[1-5]\" \"uptime\" # Expands to node1, node2, node3, node4, node5\n bssh -H \"node[01-03]\" \"df -h\" # Zero-padded: node01, node02, node03\n bssh -H \"node[1,3,5]\" \"ps aux\" # Specific values: node1, node3, node5\n bssh -H \"node[1-3,7,9-10]\" \"uptime\" # Mixed: node1-3, node7, node9-10\n bssh -H \"rack[1-2]-node[1-3]\" \"uptime\" # Cartesian product: 6 hosts\n bssh -H \"web[1-3].example.com\" \"uptime\" # With domain suffix\n bssh -H \"admin@db[01-03]:5432\" \"psql\" # With user and port\n bssh -H \"^/etc/hosts.cluster\" \"uptime\" # Read hosts from file\n\n Host Exclusion (--exclude):\n bssh -H \"node1,node2,node3\" --exclude \"node2\" \"uptime\" # Exclude single host\n bssh -C production --exclude \"web1,web2\" \"apt update\" # Exclude multiple hosts\n bssh -C production --exclude \"db*\" \"systemctl restart\" # Exclude with wildcard pattern\n bssh -H \"node[1-10]\" --exclude \"node[3-5]\" \"uptime\" # Exclude with hostlist expression\n\n Fail-Fast Mode (pdsh -k compatible):\n bssh -k -H \"web[1-3]\" \"deploy.sh\" # Stop on first failure\n bssh --fail-fast -C prod \"apt upgrade\" # Critical deployment - stop if any node fails\n bssh -k --require-all-success -C prod cmd # Fail-fast + require all success\n\n Output Modes:\n bssh -C prod \"apt-get update\" # TUI mode (default, interactive monitoring)\n bssh -C prod --stream \"tail -f log\" # Stream mode (real-time with [node] prefixes)\n bssh -C prod --output-dir ./logs \"ps\" # File mode (save to timestamped files)\n bssh -C prod \"uptime\" | tee log.txt # Normal mode (auto-detected when piped)\n\n Batch Mode (Ctrl+C Handling):\n bssh -C prod \"long-running-command\" # Default: first Ctrl+C shows status, second terminates\n bssh -C prod -b \"long-command\" # Batch mode: single Ctrl+C terminates immediately\n bssh -H nodes --batch --stream \"cmd\" # Useful for CI/CD and non-interactive scripts\n\n TUI Mode Controls (when in TUI):\n 1-9 Jump to node detail view\n s Enter split view (2-4 nodes)\n d Enter diff view (compare nodes)\n f Toggle auto-scroll\n Up/Down Scroll output\n Left/Right Switch nodes\n Esc Return to summary\n ? Show help\n q Quit\n\n File Operations:\n bssh -C staging upload file.txt /tmp/ # Upload to cluster\n bssh -H host1,host2 download /etc/hosts ./backups/\n\n Other Commands:\n bssh list # List configured clusters\n bssh -C production ping # Test connectivity\n bssh -H hosts interactive # Interactive mode\n\n SSH Config Example (~/.ssh/config):\n Host web*\n HostName web.example.com\n User webuser\n Port 2222\n IdentityFile ~/.ssh/web_key\n StrictHostKeyChecking yes\n\nDeveloped and maintained as part of the Backend.AI project.\nFor more information: https://github.com/lablup/bssh" + long_about = "bssh is a high-performance SSH client with parallel execution capabilities.\nIt provides measured OpenSSH compatibility in single-host mode and powerful cluster management features in multi-host mode.\n\nThe tool provides secure file transfer using SFTP and supports SSH keys, SSH agent, and password authentication.\nIt automatically detects Backend.AI multi-node session environments.\n\nOutput Modes:\n- TUI Mode (default): Interactive terminal UI with real-time monitoring (auto-enabled in terminals)\n- Stream Mode (--stream): Real-time output with [node] prefixes\n- File Mode (--output-dir): Save per-node output to timestamped files\n- Normal Mode: Traditional output after all nodes complete\n\nSSH Configuration Support:\n- Reads standard SSH config files (defaulting to ~/.ssh/config)\n- Supports Host patterns, HostName, User, Port, IdentityFile, StrictHostKeyChecking\n- ProxyJump, and many other SSH configuration directives\n- CLI arguments override SSH config values following SSH precedence rules", + after_help = "EXAMPLES:\n SSH Mode:\n bssh user@host # Interactive shell\n bssh admin@server.com \"uptime\" # Execute command\n bssh -p 2222 -i ~/.ssh/key user@host # Custom port and key\n bssh -F ~/.ssh/myconfig webserver # Use custom SSH config\n\n Port Forwarding:\n bssh -L 8080:example.com:80 user@host # Local forward: localhost:8080 -> example.com:80\n bssh -R 8080:localhost:80 user@host # Remote forward: remote:8080 -> localhost:80\n bssh -D 1080 user@host # SOCKS5 proxy on localhost:1080\n bssh -L 3306:db:3306 -R 80:web:80 user@host # Multiple forwards\n bssh -D *:1080/4 user@host # SOCKS4 proxy on all interfaces\n\n Multi-Server Mode:\n bssh --cluster production \"systemctl status\" # Execute on cluster (TUI mode auto-enabled)\n bssh -H \"web1,web2,web3\" \"df -h\" # Execute on multiple hosts\n bssh -H \"web1,web2,web3\" --filter \"web1\" \"df -h\" # Filter to web1 only\n bssh --cluster production --filter \"web*\" \"uptime\" # Filter cluster nodes\n bssh --parallel 20 -H web* \"apt update\" # Increase parallelism\n\n Hostlist Expression (pdsh-style range expansion):\n bssh -H \"node[1-5]\" \"uptime\" # Expands to node1, node2, node3, node4, node5\n bssh -H \"node[01-03]\" \"df -h\" # Zero-padded: node01, node02, node03\n bssh -H \"node[1,3,5]\" \"ps aux\" # Specific values: node1, node3, node5\n bssh -H \"node[1-3,7,9-10]\" \"uptime\" # Mixed: node1-3, node7, node9-10\n bssh -H \"rack[1-2]-node[1-3]\" \"uptime\" # Cartesian product: 6 hosts\n bssh -H \"web[1-3].example.com\" \"uptime\" # With domain suffix\n bssh -H \"admin@db[01-03]:5432\" \"psql\" # With user and port\n bssh -H \"^/etc/hosts.cluster\" \"uptime\" # Read hosts from file\n\n Host Exclusion (--exclude):\n bssh -H \"node1,node2,node3\" --exclude \"node2\" \"uptime\" # Exclude single host\n bssh --cluster production --exclude \"web1,web2\" \"apt update\" # Exclude multiple hosts\n bssh --cluster production --exclude \"db*\" \"systemctl restart\" # Exclude with wildcard pattern\n bssh -H \"node[1-10]\" --exclude \"node[3-5]\" \"uptime\" # Exclude with hostlist expression\n\n Fail-Fast Mode:\n bssh --fail-fast -H \"web[1-3]\" \"deploy.sh\" # Stop on first failure\n bssh --fail-fast --cluster prod \"apt upgrade\" # Critical deployment - stop if any node fails\n bssh --fail-fast --require-all-success --cluster prod cmd # Fail-fast + require all success\n\n Output Modes:\n bssh --cluster prod \"apt-get update\" # TUI mode (default, interactive monitoring)\n bssh --cluster prod --stream \"tail -f log\" # Stream mode (real-time with [node] prefixes)\n bssh --cluster prod --output-dir ./logs \"ps\" # File mode (save to timestamped files)\n bssh --cluster prod \"uptime\" | tee log.txt # Normal mode (auto-detected when piped)\n\n Batch Mode (Ctrl+C Handling):\n bssh --cluster prod \"long-running-command\" # Default: first Ctrl+C shows status, second terminates\n bssh --cluster prod --batch \"long-command\" # Batch mode: single Ctrl+C terminates immediately\n bssh -H nodes --batch --stream \"cmd\" # Useful for CI/CD and non-interactive scripts\n\n TUI Mode Controls (when in TUI):\n 1-9 Jump to node detail view\n s Enter split view (2-4 nodes)\n d Enter diff view (compare nodes)\n f Toggle auto-scroll\n Up/Down Scroll output\n Left/Right Switch nodes\n Esc Return to summary\n ? Show help\n q Quit\n\n File Operations:\n bssh --cluster staging upload file.txt /tmp/ # Upload to cluster\n bssh -H host1,host2 download /etc/hosts ./backups/\n\n Other Commands:\n bssh list # List configured clusters\n bssh --cluster production ping # Test connectivity\n bssh -H hosts interactive # Interactive mode\n\n SSH Config Example (~/.ssh/config):\n Host web*\n HostName web.example.com\n User webuser\n Port 2222\n IdentityFile ~/.ssh/web_key\n StrictHostKeyChecking yes\n\nDeveloped and maintained as part of the Backend.AI project.\nFor more information: https://github.com/lablup/bssh" )] pub struct Cli { /// SSH destination in format: [user@]hostname[:port] or ssh://[user@]hostname[:port] @@ -46,9 +46,8 @@ pub struct Cli { pub hosts: Option>, #[arg( - short = 'f', long = "filter", - help = "Filter hosts by pattern (supports wildcards and hostlist expressions)\nUse with -H or -C to execute on a subset of hosts\nExamples:\n 'web*' -> matches web01, web02, etc. (glob)\n 'node[1-3]' -> matches node1, node2, node3 (hostlist)" + help = "Filter hosts by pattern (supports wildcards and hostlist expressions)\nUse with -H or --cluster to execute on a subset of hosts\nExamples:\n 'web*' -> matches web01, web02, etc. (glob)\n 'node[1-3]' -> matches node1, node2, node3 (hostlist)" )] pub filter: Option, @@ -60,7 +59,6 @@ pub struct Cli { pub exclude: Option>, #[arg( - short = 'C', long = "cluster", help = "Cluster name from configuration file (multi-server mode)" )] @@ -89,8 +87,7 @@ pub struct Cli { pub identity: Vec, #[arg( - short = 'A', - long, + long = "use-agent", help = "Use SSH agent for authentication (Unix/Linux/macOS only)\nAuto-detected when SSH_AUTH_SOCK is set. Falls back to key file if agent auth fails" )] pub use_agent: bool, @@ -102,14 +99,12 @@ pub struct Cli { pub password: bool, #[arg( - short = 'S', long = "sudo-password", help = "Prompt for sudo password to automatically respond to sudo prompts\nWhen enabled, bssh will:\n 1. Securely prompt for sudo password before execution\n 2. Detect sudo password prompts in command output\n 3. Automatically inject the password when prompted\n\nAlternatively, set BSSH_SUDO_PASSWORD environment variable (not recommended)\nSecurity: Password is cleared from memory after use" )] pub sudo_password: bool, #[arg( - short = 'b', long = "batch", help = "Batch mode: single Ctrl+C immediately terminates all jobs\nDisables two-stage Ctrl+C handling (status display on first press)\nUseful for non-interactive scripts and CI/CD pipelines\nNote: TUI mode has its own quit handling (q or Ctrl+C) and ignores this flag" )] @@ -140,10 +135,17 @@ pub struct Cli { short = 'p', long = "port", value_name = "port", + value_parser = clap::value_parser!(u16).range(1..=u16::MAX as i64), help = "Port to connect to on the remote host (SSH-compatible)" )] pub port: Option, + #[arg( + short = '2', + help = "Force SSH protocol version 2 (accepted for OpenSSH compatibility)" + )] + pub protocol_2: bool, + #[arg( long, help = "Stream output in real-time with [node] prefixes\nEach line of output is prefixed with the node hostname and displayed as it arrives.\nUseful for monitoring long-running commands across multiple nodes.\nAutomatically disabled when output is piped or in CI environments." @@ -162,7 +164,6 @@ pub struct Cli { pub version: bool, #[arg( - short = 'N', long = "no-prefix", help = "Disable hostname prefix in output lines (pdsh -N compatibility)\nUseful for programmatic parsing or cleaner display" )] @@ -239,7 +240,6 @@ pub struct Cli { pub check_all_nodes: bool, #[arg( - short = 'k', long = "fail-fast", help = "Stop execution immediately on first failure (pdsh -k compatible)\nCancels pending commands when any node fails (connection error or non-zero exit)\nUseful for critical operations where partial execution is unacceptable" )] @@ -269,6 +269,33 @@ pub struct Cli { help = "SSH options (e.g., -o StrictHostKeyChecking=no)")] pub ssh_options: Vec, + #[arg( + short = 'N', + overrides_with = "subsystem", + help = "Do not execute a remote command" + )] + pub no_remote_command: bool, + + #[arg(short = 'f', help = "Go to the background after authentication")] + pub fork_after_authentication: bool, + + #[arg(short = 'C', help = "Enable SSH transport compression")] + pub compression: bool, + + #[arg(short = 'A', help = "Enable SSH authentication agent forwarding")] + pub forward_agent: bool, + + #[arg(short = 'k', help = "Disable forwarding of GSSAPI credentials")] + pub disable_gssapi_credential_forwarding: bool, + + #[arg( + short = 'b', + value_name = "bind_address", + overrides_with = "bind_address", + help = "Use the specified local address as the connection source" + )] + pub bind_address: Option, + #[arg( short = 'M', long = "control-master", @@ -288,8 +315,10 @@ pub struct Cli { pub control_command: Option, #[arg( + short = 'S', long = "control-path", value_name = "path", + overrides_with = "control_path", help = "Path template for the connection-sharing control socket" )] pub control_path: Option, @@ -317,6 +346,7 @@ pub struct Cli { #[arg( short = 's', long = "subsystem", + overrides_with = "no_remote_command", help = "Invoke the remote command as an SSH subsystem" )] pub subsystem: bool, @@ -485,7 +515,7 @@ pub enum Commands { #[command( about = "Start interactive shell session", long_about = "Opens an interactive shell session with one or more remote hosts.\nSupports both single-node and multiplex modes for efficient cluster management.\nIn multiplex mode, commands are sent to all active nodes simultaneously.\n\nSpecial commands (default prefix '!'):\n !all - Activate all connected nodes\n !broadcast - Execute on all nodes temporarily\n !node - Switch to specific node (e.g., !node1)\n !list - List all nodes and connection status\n !status - Show currently active nodes\n !help - Show special commands help\n exit - Exit interactive mode\n\nSettings can be configured globally or per-cluster in config file.\nCLI arguments override configuration file settings.", - after_help = "Examples:\n bssh interactive # Auto-detect or use defaults\n bssh -C prod interactive # Use production cluster\n bssh interactive --single-node # Connect to one node only\n bssh interactive --prompt-format '{user}>' # Custom prompt\n bssh interactive --work-dir /var/www # Set initial directory" + after_help = "Examples:\n bssh interactive # Auto-detect or use defaults\n bssh --cluster prod interactive # Use production cluster\n bssh interactive --single-node # Connect to one node only\n bssh interactive --prompt-format '{user}>' # Custom prompt\n bssh interactive --work-dir /var/www # Set initial directory" )] Interactive { #[arg( @@ -540,6 +570,65 @@ pub enum Commands { } impl Cli { + /// Return migration notices for reassigned OpenSSH short options. + /// + /// bssh 2.x used these letters for extensions. The 3.0-compatible parser + /// gives the letters back to OpenSSH, so each notice names the long option + /// that preserves the former bssh behavior. Only the option prefix of a + /// single-destination invocation is inspected; remote-command arguments + /// are never treated as bssh flags. + pub fn short_flag_migration_warnings(&self, args: &[String]) -> Vec<&'static str> { + if !self.is_ssh_mode() { + return Vec::new(); + } + let search_end = args.len().saturating_sub(self.command_args.len()); + let Some(destination_index) = self.destination.as_ref().and_then(|destination| { + args[..search_end] + .iter() + .rposition(|argument| argument == destination) + }) else { + return Vec::new(); + }; + + let mut warnings = Vec::new(); + let mut index = 1usize; + while index < destination_index { + let argument = &args[index]; + if argument == "--" { + break; + } + if let Some(long) = argument.strip_prefix("--") { + let (name, attached) = long + .split_once('=') + .map_or((long, false), |(name, _)| (name, true)); + if !attached && migration_long_takes_value(name) { + index += 1; + } + index += 1; + continue; + } + let Some(shorts) = argument.strip_prefix('-').filter(|value| !value.is_empty()) else { + index += 1; + continue; + }; + for (position, short) in shorts.char_indices() { + if let Some(warning) = migration_warning(short) + && !warnings.contains(&warning) + { + warnings.push(warning); + } + if migration_short_takes_value(short) { + if position + short.len_utf8() == shorts.len() { + index += 1; + } + break; + } + } + index += 1; + } + warnings + } + pub fn get_command(&self) -> String { // In multi-server mode with destination, treat destination as first command arg if self.is_multi_server_mode() @@ -623,7 +712,15 @@ impl Cli { let Some(destination) = self.destination.as_ref() else { return Ok(None); }; - let destination = destination.strip_prefix("ssh://").unwrap_or(destination); + let destination = if let Some(uri) = destination.strip_prefix("ssh://") { + match uri.split_once('/') { + Some((authority, "")) => authority, + Some((_, path)) => anyhow::bail!("SSH URI paths are not supported: /{path}"), + None => uri, + } + } else { + destination + }; let spec = crate::node::parse_node_spec(destination)?; Ok(Some(spec)) @@ -702,7 +799,13 @@ impl Cli { + usize::from(self.cipher.is_some()) + usize::from(self.macs.is_some()) + usize::from(self.subsystem) - + usize::from(self.stdin_null) + + usize::from(self.no_remote_command) + + usize::from(self.stdin_null || self.fork_after_authentication) + + usize::from(self.fork_after_authentication) + + usize::from(self.compression) + + usize::from(self.forward_agent) + + usize::from(self.disable_gssapi_credential_forwarding) + + usize::from(self.bind_address.is_some()) + usize::from(self.control_master > 0) + usize::from(self.control_path.is_some()), ); @@ -717,6 +820,21 @@ impl Cli { if let Some(path) = &self.control_path { options.push(format!("ControlPath={}", path.display())); } + if self.compression { + options.push("Compression=yes".to_string()); + } + if self.forward_agent { + options.push("ForwardAgent=yes".to_string()); + } + if self.disable_gssapi_credential_forwarding { + options.push("GSSAPIDelegateCredentials=no".to_string()); + } + if let Some(address) = &self.bind_address { + options.push(format!("BindAddress={address}")); + } + if self.fork_after_authentication { + options.push("ForkAfterAuthentication=yes".to_string()); + } if let Some(cipher) = &self.cipher { options.push(format!("Ciphers={cipher}")); } @@ -726,7 +844,10 @@ impl Cli { if self.subsystem { options.push("SessionType=subsystem".to_string()); } - if self.stdin_null { + if self.no_remote_command { + options.push("SessionType=none".to_string()); + } + if self.stdin_null || self.fork_after_authentication { options.push("StdinNull=yes".to_string()); } options.extend(self.ssh_options.iter().cloned()); @@ -907,6 +1028,89 @@ impl Cli { } } +fn migration_warning(short: char) -> Option<&'static str> { + match short { + 'N' => Some( + "Warning: -N now means 'no remote command' for OpenSSH compatibility; bssh 2.x scripts that used it for output prefixes must use --no-prefix", + ), + 'f' => Some( + "Warning: -f now means 'background after authentication' for OpenSSH compatibility; bssh 2.x scripts that used it for host filtering must use --filter", + ), + 'C' => Some( + "Warning: -C now enables SSH compression for OpenSSH compatibility; bssh 2.x scripts that selected a cluster must use --cluster", + ), + 'A' => Some( + "Warning: -A now enables SSH agent forwarding for OpenSSH compatibility; bssh 2.x scripts that used an agent for authentication must use --use-agent", + ), + 'S' => Some( + "Warning: -S now specifies ControlPath for OpenSSH compatibility; bssh 2.x scripts that prompted for sudo credentials must use --sudo-password", + ), + 'k' => Some( + "Warning: -k now disables GSSAPI credential forwarding for OpenSSH compatibility; bssh 2.x scripts that stopped on the first failure must use --fail-fast", + ), + 'b' => Some( + "Warning: -b now specifies the source bind address for OpenSSH compatibility; bssh 2.x scripts that changed Ctrl+C handling must use --batch", + ), + _ => None, + } +} + +fn migration_short_takes_value(short: char) -> bool { + matches!( + short, + 'H' | 'i' + | 'J' + | 'p' + | 'E' + | 'Q' + | 'L' + | 'R' + | 'D' + | 'o' + | 'O' + | 'S' + | 'b' + | 'c' + | 'm' + | 'W' + | 'F' + ) +} + +fn migration_long_takes_value(name: &str) -> bool { + matches!( + name, + "hosts" + | "filter" + | "exclude" + | "cluster" + | "config" + | "login" + | "identity" + | "jump-host" + | "parallel" + | "port" + | "color" + | "output-dir" + | "timeout" + | "connect-timeout" + | "server-alive-interval" + | "server-alive-count-max" + | "option" + | "control-command" + | "control-path" + | "bind-address" + | "cipher" + | "macs" + | "stdio-forward" + | "ssh-config" + | "query" + | "local-forward" + | "remote-forward" + | "dynamic-forward" + ) +} + #[cfg(test)] mod tests { use super::*; @@ -922,6 +1126,42 @@ mod tests { ); } + #[test] + fn port_flag_accepts_only_nonzero_u16_values() { + for port in ["1", "22", "65535"] { + let cli = Cli::try_parse_from(["bssh", "-p", port, "target"]) + .unwrap_or_else(|error| panic!("valid port {port} was rejected: {error}")); + assert_eq!(cli.port, Some(port.parse().unwrap())); + } + + for port in ["0", "65536", "131073", "2000blah", "blah2000"] { + assert!( + Cli::try_parse_from(["bssh", "-p", port, "target"]).is_err(), + "invalid port {port} was accepted" + ); + } + } + + #[test] + fn ssh_uri_allows_only_an_empty_path() { + for destination in [ + "ssh://user@example.com:2222", + "ssh://user@example.com:2222/", + ] { + let cli = Cli::try_parse_from(["bssh", destination]).unwrap(); + let parsed = cli.parse_destination_result().unwrap().unwrap(); + assert_eq!(parsed.user, Some("user")); + assert_eq!(parsed.host, "example.com"); + assert_eq!(parsed.port, Some(2222)); + } + + let cli = Cli::try_parse_from(["bssh", "ssh://user@example.com:2222/command"]).unwrap(); + let error = cli + .parse_destination_result() + .expect_err("non-empty SSH URI path must be rejected"); + assert!(error.to_string().contains("paths are not supported")); + } + #[test] fn openssh_tty_flags_accept_repetition_and_last_flag_wins() { for (args, expected) in [ @@ -1018,6 +1258,142 @@ mod tests { assert_eq!(repeated_more.ssh_config_overrides()[0], "ControlMaster=ask"); } + #[test] + fn openssh_short_flags_are_distinct_from_displaced_bssh_extensions() { + let cli = Cli::try_parse_from([ + "bssh", + "-2NfCAk", + "-S", + "/tmp/control-%C", + "-b127.0.0.2", + "target", + ]) + .unwrap(); + + assert!(cli.protocol_2); + assert!(cli.no_remote_command); + assert!(cli.fork_after_authentication); + assert!(cli.compression); + assert!(cli.forward_agent); + assert!(cli.disable_gssapi_credential_forwarding); + assert_eq!( + cli.control_path.as_deref(), + Some(std::path::Path::new("/tmp/control-%C")) + ); + assert_eq!(cli.bind_address.as_deref(), Some("127.0.0.2")); + assert!(cli.filter.is_none()); + assert!(cli.cluster.is_none()); + assert!(!cli.use_agent); + assert!(!cli.sudo_password); + assert!(!cli.batch); + assert!(!cli.no_prefix); + assert!(!cli.fail_fast); + assert_eq!( + cli.ssh_config_overrides(), + [ + "ControlPath=/tmp/control-%C", + "Compression=yes", + "ForwardAgent=yes", + "GSSAPIDelegateCredentials=no", + "BindAddress=127.0.0.2", + "ForkAfterAuthentication=yes", + "SessionType=none", + "StdinNull=yes", + ] + ); + } + + #[test] + fn displaced_bssh_features_remain_available_by_long_option() { + let cli = Cli::try_parse_from([ + "bssh", + "--filter", + "web*", + "--cluster", + "production", + "--use-agent", + "--sudo-password", + "--batch", + "--no-prefix", + "--fail-fast", + "uptime", + ]) + .unwrap(); + + assert_eq!(cli.filter.as_deref(), Some("web*")); + assert_eq!(cli.cluster.as_deref(), Some("production")); + assert!(cli.use_agent); + assert!(cli.sudo_password); + assert!(cli.batch); + assert!(cli.no_prefix); + assert!(cli.fail_fast); + assert!(!cli.no_remote_command); + assert!(!cli.fork_after_authentication); + assert!(!cli.compression); + assert!(!cli.forward_agent); + assert!(!cli.disable_gssapi_credential_forwarding); + assert!(cli.control_path.is_none()); + assert!(cli.bind_address.is_none()); + } + + #[test] + fn reassigned_short_flags_name_every_long_migration_replacement() { + let args = [ + "bssh", + "-NfCAk", + "-S", + "/tmp/control", + "-b127.0.0.2", + "target", + ] + .map(str::to_string); + let cli = Cli::try_parse_from(&args).unwrap(); + let warnings = cli.short_flag_migration_warnings(&args); + + assert_eq!(warnings.len(), 7); + for replacement in [ + "--no-prefix", + "--filter", + "--cluster", + "--use-agent", + "--sudo-password", + "--fail-fast", + "--batch", + ] { + assert!( + warnings.iter().any(|warning| warning.contains(replacement)), + "missing migration replacement {replacement}: {warnings:?}" + ); + } + } + + #[test] + fn migration_scan_ignores_remote_command_short_options() { + let args = ["bssh", "target", "printf", "-NfCAkSb"].map(str::to_string); + let cli = Cli::try_parse_from(&args).unwrap(); + assert!(cli.short_flag_migration_warnings(&args).is_empty()); + } + + #[test] + fn session_type_short_flags_use_the_last_explicit_choice() { + let none = Cli::try_parse_from(["bssh", "-sN", "target", "sftp"]).unwrap(); + assert!(none.no_remote_command); + assert!(!none.subsystem); + assert!( + none.ssh_config_overrides() + .contains(&"SessionType=none".to_string()) + ); + + let subsystem = Cli::try_parse_from(["bssh", "-Ns", "target", "sftp"]).unwrap(); + assert!(!subsystem.no_remote_command); + assert!(subsystem.subsystem); + assert!( + subsystem + .ssh_config_overrides() + .contains(&"SessionType=subsystem".to_string()) + ); + } + #[test] fn compatibility_flags_parse_repetition_modifiers_and_session_overrides() { let cli = Cli::try_parse_from([ diff --git a/src/cli/mode_detection_tests.rs b/src/cli/mode_detection_tests.rs index d30e43e6..8292bd0c 100644 --- a/src/cli/mode_detection_tests.rs +++ b/src/cli/mode_detection_tests.rs @@ -19,7 +19,7 @@ #[cfg(test)] mod tests { - use crate::cli::pdsh::PDSH_COMPAT_ENV_VAR; + use crate::cli::pdsh::{PDSH_COMPAT_ENV_VAR, is_pdsh_binary_name}; use crate::test_helpers::EnvGuard; use serial_test::serial; use std::env; @@ -107,118 +107,49 @@ mod tests { /// Test binary name detection logic for "pdsh" #[test] fn test_binary_name_pdsh() { - use std::path::Path; - - let arg0 = "/usr/bin/pdsh"; - let binary_name = Path::new(arg0) - .file_name() - .and_then(|n| n.to_str()) - .unwrap_or(""); - - assert_eq!(binary_name, "pdsh"); - assert!(binary_name == "pdsh" || binary_name.starts_with("pdsh.")); + assert!(is_pdsh_binary_name("/usr/bin/pdsh")); } /// Test binary name detection for relative path #[test] fn test_binary_name_relative_path() { - use std::path::Path; - - let arg0 = "./pdsh"; - let binary_name = Path::new(arg0) - .file_name() - .and_then(|n| n.to_str()) - .unwrap_or(""); - - assert_eq!(binary_name, "pdsh"); + assert!(is_pdsh_binary_name("./pdsh")); } /// Test binary name detection for "pdsh.exe" (Windows) #[test] #[cfg(windows)] fn test_binary_name_windows() { - use std::path::Path; - - let arg0 = "C:\\Program Files\\bssh\\pdsh.exe"; - let binary_name = Path::new(arg0) - .file_name() - .and_then(|n| n.to_str()) - .unwrap_or(""); - - assert!(binary_name.starts_with("pdsh.")); + assert!(is_pdsh_binary_name("C:\\Program Files\\bssh\\pdsh.exe")); } /// Test binary name detection for "pdsh.exe" pattern #[test] fn test_binary_name_exe_extension() { - use std::path::Path; - - // Test just the filename (works cross-platform) - let arg0 = "pdsh.exe"; - let binary_name = Path::new(arg0) - .file_name() - .and_then(|n| n.to_str()) - .unwrap_or(""); - - assert!(binary_name.starts_with("pdsh.")); + assert!(is_pdsh_binary_name("pdsh.exe")); } /// Test that bssh binary name is not detected as pdsh #[test] fn test_binary_name_bssh() { - use std::path::Path; - - let arg0 = "/usr/bin/bssh"; - let binary_name = Path::new(arg0) - .file_name() - .and_then(|n| n.to_str()) - .unwrap_or(""); - - assert_eq!(binary_name, "bssh"); - assert!(!(binary_name == "pdsh" || binary_name.starts_with("pdsh."))); + assert!(!is_pdsh_binary_name("/usr/bin/bssh")); } /// Test that symlinked pdsh is detected #[test] fn test_binary_name_symlink() { - use std::path::Path; - - // When bssh is symlinked as pdsh, arg0 would be the symlink name - let arg0 = "/usr/local/bin/pdsh"; - let binary_name = Path::new(arg0) - .file_name() - .and_then(|n| n.to_str()) - .unwrap_or(""); - - assert_eq!(binary_name, "pdsh"); + assert!(is_pdsh_binary_name("/usr/local/bin/pdsh")); } /// Test edge case: empty arg0 #[test] fn test_binary_name_empty() { - use std::path::Path; - - let arg0 = ""; - let binary_name = Path::new(arg0) - .file_name() - .and_then(|n| n.to_str()) - .unwrap_or(""); - - assert!(binary_name.is_empty()); - assert!(!(binary_name == "pdsh" || binary_name.starts_with("pdsh."))); + assert!(!is_pdsh_binary_name("")); } /// Test edge case: just filename without path #[test] fn test_binary_name_no_path() { - use std::path::Path; - - let arg0 = "pdsh"; - let binary_name = Path::new(arg0) - .file_name() - .and_then(|n| n.to_str()) - .unwrap_or(""); - - assert_eq!(binary_name, "pdsh"); + assert!(is_pdsh_binary_name("pdsh")); } } diff --git a/src/cli/pdsh.rs b/src/cli/pdsh.rs index 53c5f83b..ec6c62a3 100644 --- a/src/cli/pdsh.rs +++ b/src/cli/pdsh.rs @@ -157,21 +157,24 @@ pub fn is_pdsh_compat_mode() -> bool { } // Check argv[0] for "pdsh" binary name - if let Some(arg0) = std::env::args().next() { - let binary_name = Path::new(&arg0) - .file_name() - .and_then(|n| n.to_str()) - .unwrap_or(""); - - // Match exact "pdsh" or "pdsh.exe" (Windows) or "pdsh.*" patterns - if binary_name == "pdsh" || binary_name.starts_with("pdsh.") { - return true; - } + if let Some(arg0) = std::env::args().next() + && is_pdsh_binary_name(&arg0) + { + return true; } false } +/// Return whether an argv[0] path selects the dedicated pdsh parser. +pub(crate) fn is_pdsh_binary_name(arg0: &str) -> bool { + let binary_name = Path::new(arg0) + .file_name() + .and_then(|name| name.to_str()) + .unwrap_or(""); + binary_name == "pdsh" || binary_name.starts_with("pdsh.") +} + /// Checks if pdsh compatibility mode should be enabled based on arguments. /// /// This function checks for the `--pdsh-compat` flag in the provided arguments. @@ -307,6 +310,7 @@ impl PdshCli { sudo_password: false, jump_hosts: None, port: None, + protocol_2: false, stream: false, color: crate::ui::ColorMode::Auto, version: false, @@ -317,6 +321,12 @@ impl PdshCli { require_all_success: false, check_all_nodes: false, ssh_options: Vec::new(), + no_remote_command: false, + fork_after_authentication: false, + compression: false, + forward_agent: false, + disable_gssapi_credential_forwarding: false, + bind_address: None, control_master: 0, control_command: None, control_path: None, @@ -548,6 +558,14 @@ mod tests { assert!(bssh_cli.fail_fast); // Any failure flag assert!(bssh_cli.any_failure); + // The pdsh parser owns these short flags; no SSH-mode field leaks. + assert!(!bssh_cli.no_remote_command); + assert!(!bssh_cli.fork_after_authentication); + assert!(!bssh_cli.compression); + assert!(!bssh_cli.forward_agent); + assert!(!bssh_cli.disable_gssapi_credential_forwarding); + assert!(bssh_cli.control_path.is_none()); + assert!(bssh_cli.bind_address.is_none()); // Command assert_eq!(bssh_cli.command_args, vec!["df", "-h"]); } diff --git a/src/cli/ssh_args.rs b/src/cli/ssh_args.rs index d2a26ec2..34543b03 100644 --- a/src/cli/ssh_args.rs +++ b/src/cli/ssh_args.rs @@ -112,8 +112,8 @@ fn scoped_second_pass_width(argument: &str, next: Option<&String>) -> Option {} - 'c' | 'm' | 'W' | 'F' | 'o' | 'O' => { + '2' | 's' | 'n' | 'M' | 'N' | 'f' | 'C' | 'A' | 'k' => {} + 'c' | 'm' | 'W' | 'F' | 'o' | 'O' | 'S' | 'b' => { let attached = position + short.len_utf8() < shorts.len(); return Some(usize::from(!attached && next.is_some()) + 1); } @@ -320,10 +320,18 @@ impl SshDumpInvocation { set_priority(&mut priority_overrides, "sessiontype", "SessionType=none") } 'n' => set_priority(&mut priority_overrides, "stdinnull", "StdinNull=yes"), - 'f' => set_priority( + 'f' => { + set_priority( + &mut priority_overrides, + "forkafterauthentication", + "ForkAfterAuthentication=yes", + ); + set_priority(&mut priority_overrides, "stdinnull", "StdinNull=yes"); + } + 'k' => set_priority( &mut priority_overrides, - "forkafterauthentication", - "ForkAfterAuthentication=yes", + "gssapidelegatecredentials", + "GSSAPIDelegateCredentials=no", ), 'g' => set_priority( &mut priority_overrides, @@ -352,7 +360,7 @@ impl SshDumpInvocation { version = true; break 'arguments; } - 'q' | 'v' | 'y' => {} + '2' | 'q' | 'v' | 'y' => {} _ => anyhow::bail!("Unknown option '-{short}'"), } } @@ -813,6 +821,54 @@ mod tests { normalize_ssh_option_pass(&missing_value, "host", 1), args(&["bssh", "-c", "host"]) ); + + let reassigned = args(&[ + "bssh", + "host", + "-2NfCAk", + "-S", + "/tmp/control", + "-b127.0.0.2", + "remote-command", + ]); + assert_eq!( + normalize_ssh_option_pass(&reassigned, "host", 5), + args(&[ + "bssh", + "-2NfCAk", + "-S", + "/tmp/control", + "-b127.0.0.2", + "host", + "remote-command", + ]) + ); + } + + #[test] + fn config_dump_maps_all_reassigned_openssh_short_flags() { + let parsed = SshDumpInvocation::from_argv(&args(&[ + "bssh", + "-G2NfCAk", + "-S/tmp/control", + "-b127.0.0.2", + "host", + ])) + .unwrap(); + + assert_eq!( + parsed.overrides, + [ + "SessionType=none", + "ForkAfterAuthentication=yes", + "StdinNull=yes", + "Compression=yes", + "ForwardAgent=yes", + "GSSAPIDelegateCredentials=no", + "ControlPath=/tmp/control", + "BindAddress=127.0.0.2", + ] + ); } #[test] diff --git a/src/commands/interactive/connection.rs b/src/commands/interactive/connection.rs index 9e89737f..048a5c77 100644 --- a/src/commands/interactive/connection.rs +++ b/src/commands/interactive/connection.rs @@ -28,8 +28,8 @@ use crate::ssh::{ SessionPolicy, SessionPurpose, SessionRequest, known_hosts::get_check_method_for_target, tokio_client::{ - AuthMethod, Client, Error as SshError, ServerCheckMethod, SshConnectionConfig, - SshConnectionConfigResolver, select_proxy_jump, + AgentForwardingLease, AuthMethod, Client, Error as SshError, ServerCheckMethod, + SshConnectionConfig, SshConnectionConfigResolver, select_proxy_jump, }, }; @@ -312,7 +312,7 @@ impl InteractiveCommand { term_type: &str, width: u32, height: u32, - ) -> Result> { + ) -> Result<(Channel, Option)> { if let Some(policy) = self.session_policy.as_ref() { if !matches!(policy.request, SessionRequest::Shell) { anyhow::bail!("Interactive mode requires a shell session policy"); @@ -320,10 +320,25 @@ impl InteractiveCommand { policy.run_local_command().await?; } - client + let channel = client .request_interactive_shell(term_type, width, height) .await - .context("Failed to open interactive session channel") + .context("Failed to open interactive session channel")?; + let agent_forwarding_lease = if self + .session_policy + .as_ref() + .is_some_and(|policy| policy.forward_agent) + { + Some( + client + .request_agent_forwarding(&channel) + .await + .context("Failed to request SSH agent forwarding")?, + ) + } else { + None + }; + Ok((channel, agent_forwarding_lease)) } /// Connect to a single node and establish an interactive shell @@ -461,7 +476,7 @@ impl InteractiveCommand { // Get terminal dimensions let (width, height) = terminal::size().unwrap_or((80, 24)); - let channel = self + let (channel, agent_forwarding_lease) = self .open_interactive_channel( &client, "xterm-256color", @@ -490,11 +505,20 @@ impl InteractiveCommand { String::from("~") }; - Ok(NodeSession::new(node, client, channel, working_dir)) + Ok(NodeSession::new( + node, + client, + channel, + working_dir, + agent_forwarding_lease, + )) } /// Connect to a single node and establish a PTY-enabled SSH channel - pub(super) async fn connect_to_node_pty(&self, node: Node) -> Result<(Client, Channel)> { + pub(super) async fn connect_to_node_pty( + &self, + node: Node, + ) -> Result<(Client, Channel, Option)> { let target_config = interactive_target_connection_config( &node, &self.ssh_connection_config, @@ -629,12 +653,12 @@ impl InteractiveCommand { // The PTY manager retains channel ownership for raw stdin, resize, and // byte-transparent output. It requests PTY and shell after policy env. - let channel = self + let (channel, agent_forwarding_lease) = self .open_interactive_channel(&client, &self.pty_config.term_type, width, height) .await .context("Failed to request interactive shell with PTY")?; - Ok((client, channel)) + Ok((client, channel, agent_forwarding_lease)) } } @@ -856,6 +880,7 @@ Host beta let policy = SessionPolicy { environment: Vec::new(), local_command: None, + forward_agent: false, request_pty: false, stdin_null: false, request: SessionRequest::Shell, diff --git a/src/commands/interactive/execution.rs b/src/commands/interactive/execution.rs index a436b0d8..6f102159 100644 --- a/src/commands/interactive/execution.rs +++ b/src/commands/interactive/execution.rs @@ -57,16 +57,18 @@ impl InteractiveCommand { // Connect to all selected nodes and get SSH channels let mut channels = Vec::new(); let mut clients = Vec::new(); + let mut agent_forwarding_leases = Vec::new(); let mut connected_nodes = Vec::new(); for node in nodes_to_connect { match self.connect_to_node_pty(node.clone()).await { - Ok((client, channel)) => { + Ok((client, channel, agent_forwarding_lease)) => { if !ssh_compatible { println!("✓ Connected to {}", node.to_string().green()); } channels.push(channel); clients.push(client); + agent_forwarding_leases.extend(agent_forwarding_lease); connected_nodes.push(node); } Err(e) => { @@ -117,6 +119,7 @@ impl InteractiveCommand { Ok(()) } .await; + drop(agent_forwarding_leases); let disconnect_result = Self::disconnect_clients(&clients).await; if requested_remote_pty { diff --git a/src/commands/interactive/types.rs b/src/commands/interactive/types.rs index 3b5e48b1..4b9bc242 100644 --- a/src/commands/interactive/types.rs +++ b/src/commands/interactive/types.rs @@ -27,7 +27,9 @@ use crate::pty::PtyConfig; use crate::security::Password; use crate::ssh::SessionPolicy; use crate::ssh::known_hosts::StrictHostKeyChecking; -use crate::ssh::tokio_client::{Client, SshConnectionConfig, SshConnectionConfigResolver}; +use crate::ssh::tokio_client::{ + AgentForwardingLease, Client, SshConnectionConfig, SshConnectionConfigResolver, +}; /// SSH output polling interval for responsive display /// - 10ms provides very responsive output display @@ -90,6 +92,7 @@ pub(super) struct NodeSession { #[allow(dead_code)] pub client: Client, pub channel: Channel, + _agent_forwarding_lease: Option, pub working_dir: String, pub is_connected: bool, pub is_active: bool, // Whether this node is currently active for commands @@ -97,11 +100,18 @@ pub(super) struct NodeSession { impl NodeSession { /// Create a new NodeSession - pub fn new(node: Node, client: Client, channel: Channel, working_dir: String) -> Self { + pub fn new( + node: Node, + client: Client, + channel: Channel, + working_dir: String, + agent_forwarding_lease: Option, + ) -> Self { Self { node, client, channel, + _agent_forwarding_lease: agent_forwarding_lease, working_dir, is_connected: true, is_active: true, diff --git a/src/commands/interactive/utils.rs b/src/commands/interactive/utils.rs index d9fbff11..713ce511 100644 --- a/src/commands/interactive/utils.rs +++ b/src/commands/interactive/utils.rs @@ -106,6 +106,7 @@ mod tests { session_policy: Some(crate::ssh::SessionPolicy { environment: vec![("POLICY".into(), "value".into())], local_command: None, + forward_agent: false, request_pty: false, stdin_null: false, request: crate::ssh::SessionRequest::Shell, diff --git a/src/jump/chain/tunnel.rs b/src/jump/chain/tunnel.rs index 2aad3807..22fac499 100644 --- a/src/jump/chain/tunnel.rs +++ b/src/jump/chain/tunnel.rs @@ -184,6 +184,7 @@ pub(super) async fn connect_through_tunnel( let fatal_transport = handler.fatal_transport_state(); let hostkey_rotation = handler.hostkey_rotation_tasks(); let remote_forward_registry = handler.remote_forward_registry(); + let agent_forwarding = handler.agent_forwarding_state(); // Connect through the stream let handle = tokio::time::timeout( @@ -241,6 +242,7 @@ pub(super) async fn connect_through_tunnel( fatal_transport, hostkey_rotation, remote_forward_registry, + agent_forwarding, ) .await; @@ -342,6 +344,7 @@ pub(super) async fn connect_to_destination( let fatal_transport = handler.fatal_transport_state(); let hostkey_rotation = handler.hostkey_rotation_tasks(); let remote_forward_registry = handler.remote_forward_registry(); + let agent_forwarding = handler.agent_forwarding_state(); // Connect through the stream let handle = tokio::time::timeout( @@ -387,6 +390,7 @@ pub(super) async fn connect_to_destination( fatal_transport, hostkey_rotation, remote_forward_registry, + agent_forwarding, ) .await; client diff --git a/src/main.rs b/src/main.rs index 07cbd920..e07186c3 100644 --- a/src/main.rs +++ b/src/main.rs @@ -25,10 +25,15 @@ use glob::Pattern; mod app; +#[cfg(unix)] +use app::background; +#[cfg(unix)] +use app::dispatcher::requires_background_supervision; use app::{ + background::BackgroundWorker, cache::handle_cache_stats, config_dump::handle_config_dump, - dispatcher::dispatch_command, + dispatcher::dispatch_command_with_background, initialization::{AppContext, initialize_app}, query::{handle_query, is_supported_query}, utils::show_usage, @@ -108,8 +113,12 @@ async fn run() -> Result<()> { /// exit code. `dispatch_command` returns the code instead of exiting itself, so /// the mapping stays in one place instead of being scattered across command /// implementations. -async fn dispatch_and_exit(cli: &Cli, ctx: &AppContext) -> Result<()> { - match dispatch_command(cli, ctx).await { +async fn dispatch_and_exit( + cli: &Cli, + ctx: &AppContext, + background_worker: Option<&BackgroundWorker>, +) -> Result<()> { + match dispatch_command_with_background(cli, ctx, background_worker).await { Ok(0) => Ok(()), Ok(exit_code) => std::process::exit(exit_code), Err(e) => Err(map_hard_failure(&cli.command, cli.is_ssh_mode(), e)), @@ -194,7 +203,7 @@ async fn run_pdsh_mode(args: &[String]) -> Result<()> { // Initialize and run let ctx = initialize_app(&mut cli, args).await?; - dispatch_and_exit(&cli, &ctx).await + dispatch_and_exit(&cli, &ctx, None).await } /// Handle pdsh query mode (-q) @@ -321,6 +330,13 @@ async fn run_bssh_mode(args: &[String]) -> Result<()> { if effective_args != args { cli = Cli::parse_from(&effective_args); } + bssh::utils::diagnostics::set_quiet_warnings(cli.quiet); + let background_worker = BackgroundWorker::from_environment()?; + if background_worker.is_none() { + for warning in cli.short_flag_migration_warnings(&effective_args) { + bssh::warningln!("{warning}"); + } + } bssh::ui::configure_color(cli.color); if cli.version { @@ -359,6 +375,11 @@ async fn run_bssh_mode(args: &[String]) -> Result<()> { return Ok(()); } + #[cfg(not(unix))] + if cli.is_ssh_mode() && cli.fork_after_authentication { + anyhow::bail!("-f background-after-authentication currently requires Unix"); + } + // Initialize the application and load all configurations. A failure here is // a pre-connection failure, which `ping` reports as 255. let init_result = initialize_app(&mut cli, &effective_args).await; @@ -367,6 +388,22 @@ async fn run_bssh_mode(args: &[String]) -> Result<()> { Err(e) => return Err(map_hard_failure(&cli.command, cli.is_ssh_mode(), e)), }; + // Re-execute only invocations that may actually detach. This decision is + // made after effective ssh_config resolution so config-only + // ForkAfterAuthentication and ControlPersist remain supported, while + // ordinary commands and subsystem transports retain their original + // single-process stdio and latency characteristics. + #[cfg(unix)] + if background_worker.is_none() && requires_background_supervision(&cli, &ctx)? { + let exit_code = background::supervise(&effective_args) + .await + .map_err(|error| map_hard_failure(&cli.command, true, error))?; + if exit_code == 0 { + return Ok(()); + } + std::process::exit(exit_code); + } + // Dispatch to the appropriate command handler - dispatch_and_exit(&cli, &ctx).await + dispatch_and_exit(&cli, &ctx, background_worker.as_ref()).await } diff --git a/src/ssh/control/mod.rs b/src/ssh/control/mod.rs index 8feb9b79..0c8249cb 100644 --- a/src/ssh/control/mod.rs +++ b/src/ssh/control/mod.rs @@ -31,7 +31,8 @@ pub use protocol::{ write_control_message, }; pub use runtime::{ - AttachOutcome, RunningControlMaster, attach_session, send_control_command, start_control_master, + AttachOutcome, AttachedSession, RunningControlMaster, attach_session, prepare_attached_session, + send_control_command, start_control_master, start_control_master_with_bootstrap_session, }; #[cfg(unix)] pub use socket::verify_same_user; diff --git a/src/ssh/control/protocol.rs b/src/ssh/control/protocol.rs index 34542798..3926b281 100644 --- a/src/ssh/control/protocol.rs +++ b/src/ssh/control/protocol.rs @@ -277,6 +277,7 @@ mod tests { SessionPolicy { environment: vec![("LANG".into(), "C.UTF-8".into())], local_command: None, + forward_agent: true, request_pty: false, stdin_null: false, request: SessionRequest::Exec("printf test".into()), diff --git a/src/ssh/control/runtime.rs b/src/ssh/control/runtime.rs index 16a05b4e..8c91d59a 100644 --- a/src/ssh/control/runtime.rs +++ b/src/ssh/control/runtime.rs @@ -11,7 +11,7 @@ mod unix { use std::os::fd::AsRawFd as _; use std::path::Path; use std::sync::Arc; - use std::sync::atomic::{AtomicU64, AtomicUsize, Ordering}; + use std::sync::atomic::{AtomicBool, AtomicU64, AtomicUsize, Ordering}; use std::time::Duration; use anyhow::{Context, Result}; @@ -116,6 +116,18 @@ mod unix { path: &Path, client: Client, require_confirmation: bool, + ) -> Result { + start_control_master_with_bootstrap_session(path, client, require_confirmation, false) + } + + /// Start a master and optionally let its own initial foreground passenger + /// attach without an `ask` prompt. Subsequent shared sessions still honor + /// `require_confirmation`. + pub fn start_control_master_with_bootstrap_session( + path: &Path, + client: Client, + require_confirmation: bool, + allow_bootstrap_session: bool, ) -> Result { let (listener, guard) = bind_control_socket(path)?; let (signal_tx, signal_rx) = mpsc::channel(8); @@ -127,6 +139,7 @@ mod unix { guard, client, require_confirmation, + allow_bootstrap_session, signal_rx, task_signal, active_tx, @@ -145,6 +158,20 @@ mod unix { session: SessionOpenRequest, invoking_policy: &crate::ssh::SessionPolicy, ) -> Result { + let Some(attached) = prepare_attached_session(path, session, invoking_policy).await? else { + return Ok(AttachOutcome::NoMaster); + }; + attached.finish().await.map(AttachOutcome::ExitStatus) + } + + /// Complete the hello and OpenSession exchange without starting local I/O. + /// Callers implementing `-f` may safely detach after this returns because + /// the master has authenticated the control peer and accepted the session. + pub async fn prepare_attached_session( + path: &Path, + session: SessionOpenRequest, + invoking_policy: &crate::ssh::SessionPolicy, + ) -> Result> { let mut stream = match tokio::time::timeout( CONTROL_HANDSHAKE_TIMEOUT, connect_control_socket(path), @@ -154,7 +181,7 @@ mod unix { Ok(Ok(stream)) => stream, Err(_) => { tracing::debug!(path = %path.display(), "Control master connect timed out; falling back"); - return Ok(AttachOutcome::NoMaster); + return Ok(None); } Ok(Err(error)) if matches!( @@ -162,7 +189,7 @@ mod unix { io::ErrorKind::NotFound | io::ErrorKind::ConnectionRefused ) => { - return Ok(AttachOutcome::NoMaster); + return Ok(None); } Ok(Err(error)) => { return Err(error).with_context(|| { @@ -174,17 +201,53 @@ mod unix { Ok(Ok(())) => {} Ok(Err(error)) => { tracing::debug!(path = %path.display(), "Control master handshake failed; falling back: {error:#}"); - return Ok(AttachOutcome::NoMaster); + return Ok(None); } Err(_) => { tracing::debug!(path = %path.display(), "Control master handshake timed out; falling back"); - return Ok(AttachOutcome::NoMaster); + return Ok(None); } } invoking_policy.run_local_command().await?; - run_attached_session(stream, session) - .await - .map(AttachOutcome::ExitStatus) + AttachedSession::open(stream, session).await.map(Some) + } + + pub struct AttachedSession { + stream: UnixStream, + session_id: u64, + stdin_null: bool, + } + + impl AttachedSession { + async fn open(mut stream: UnixStream, session: SessionOpenRequest) -> Result { + write_control_message( + &mut stream, + &ControlMessage::Request(ControlRequest { + request_id: OPERATION_REQUEST_ID, + operation: ControlOperation::OpenSession(session.clone()), + }), + ) + .await?; + let opened = read_response(&mut stream, OPERATION_REQUEST_ID).await?; + let session_id = match opened { + ControlResponseKind::SessionOpened { session_id } => session_id, + ControlResponseKind::Error { code, message } => { + anyhow::bail!("control master rejected session ({code}): {message}") + } + response => { + anyhow::bail!("unexpected control response while opening session: {response:?}") + } + }; + Ok(Self { + stream, + session_id, + stdin_null: session.policy.stdin_null, + }) + } + + pub async fn finish(self) -> Result { + run_attached_session(self.stream, self.session_id, self.stdin_null).await + } } pub async fn send_control_command( @@ -223,29 +286,11 @@ mod unix { } async fn run_attached_session( - mut stream: UnixStream, - session: SessionOpenRequest, + stream: UnixStream, + session_id: u64, + stdin_null: bool, ) -> Result { - write_control_message( - &mut stream, - &ControlMessage::Request(ControlRequest { - request_id: OPERATION_REQUEST_ID, - operation: ControlOperation::OpenSession(session.clone()), - }), - ) - .await?; - let opened = read_response(&mut stream, OPERATION_REQUEST_ID).await?; - let session_id = match opened { - ControlResponseKind::SessionOpened { session_id } => session_id, - ControlResponseKind::Error { code, message } => { - anyhow::bail!("control master rejected session ({code}): {message}") - } - response => { - anyhow::bail!("unexpected control response while opening session: {response:?}") - } - }; let (mut reader, mut writer) = stream.into_split(); - let stdin_null = session.policy.stdin_null; let input = tokio::spawn(async move { let mut sequence = 0u64; if !stdin_null { @@ -366,6 +411,7 @@ mod unix { guard: super::super::ControlSocketGuard, client: Client, require_confirmation: bool, + allow_bootstrap_session: bool, mut signal_rx: mpsc::Receiver, signal_tx: mpsc::Sender, active_tx: watch::Sender, @@ -373,6 +419,7 @@ mod unix { let next_session = Arc::new(AtomicU64::new(1)); let active = Arc::new(AtomicUsize::new(0)); let confirmation = Arc::new(Mutex::new(())); + let bootstrap_session = Arc::new(AtomicBool::new(allow_bootstrap_session)); let handler_slots = Arc::new(Semaphore::new(MAX_CONTROL_CLIENTS)); let mut handlers = JoinSet::new(); let immediate = loop { @@ -397,12 +444,14 @@ mod unix { let handler_active_tx = active_tx.clone(); let handler_next_session = Arc::clone(&next_session); let handler_confirmation = Arc::clone(&confirmation); + let handler_bootstrap_session = Arc::clone(&bootstrap_session); handlers.spawn(async move { let _handler_slot = handler_slot; if let Err(error) = handle_control_connection( stream, handler_client, require_confirmation, + handler_bootstrap_session, handler_confirmation, handler_signal, handler_active, @@ -448,6 +497,7 @@ mod unix { mut stream: UnixStream, client: Client, require_confirmation: bool, + bootstrap_session: Arc, confirmation: Arc>, signal: mpsc::Sender, active: Arc, @@ -478,7 +528,8 @@ mod unix { .await } ControlOperation::OpenSession(session) => { - if require_confirmation { + let is_bootstrap = bootstrap_session.swap(false, Ordering::AcqRel); + if require_confirmation && !is_bootstrap { let approved = confirm_attach(Arc::clone(&confirmation)).await?; if !approved { return send_error( @@ -785,6 +836,7 @@ mod unix { SessionPolicy { environment: Vec::new(), local_command: None, + forward_agent: false, request_pty: false, stdin_null: true, request: SessionRequest::None, @@ -835,7 +887,8 @@ mod unix { #[cfg(unix)] pub use unix::{ - AttachOutcome, RunningControlMaster, attach_session, send_control_command, start_control_master, + AttachOutcome, AttachedSession, RunningControlMaster, attach_session, prepare_attached_session, + send_control_command, start_control_master, start_control_master_with_bootstrap_session, }; #[cfg(not(unix))] @@ -857,6 +910,14 @@ mod unsupported { pub struct RunningControlMaster; + pub struct AttachedSession; + + impl AttachedSession { + pub async fn finish(self) -> Result { + anyhow::bail!("connection multiplexing requires Unix-domain sockets") + } + } + impl RunningControlMaster { pub async fn shutdown_immediately(self) -> Result<()> { anyhow::bail!("connection multiplexing requires Unix-domain sockets") @@ -879,6 +940,15 @@ mod unsupported { anyhow::bail!("connection multiplexing requires Unix-domain sockets") } + pub fn start_control_master_with_bootstrap_session( + _path: &Path, + _client: Client, + _require_confirmation: bool, + _allow_bootstrap_session: bool, + ) -> Result { + anyhow::bail!("connection multiplexing requires Unix-domain sockets") + } + pub async fn attach_session( _path: &Path, _session: SessionOpenRequest, @@ -887,6 +957,14 @@ mod unsupported { Ok(AttachOutcome::NoMaster) } + pub async fn prepare_attached_session( + _path: &Path, + _session: SessionOpenRequest, + _invoking_policy: &crate::ssh::SessionPolicy, + ) -> Result> { + Ok(None) + } + pub async fn send_control_command( _path: &Path, _command: ControlCommand, @@ -899,5 +977,6 @@ mod unsupported { #[cfg(not(unix))] pub use unsupported::{ - AttachOutcome, RunningControlMaster, attach_session, send_control_command, start_control_master, + AttachOutcome, AttachedSession, RunningControlMaster, attach_session, prepare_attached_session, + send_control_command, start_control_master, start_control_master_with_bootstrap_session, }; diff --git a/src/ssh/session_policy.rs b/src/ssh/session_policy.rs index dc577b7c..2bd738fd 100644 --- a/src/ssh/session_policy.rs +++ b/src/ssh/session_policy.rs @@ -61,6 +61,13 @@ pub enum SessionRequest { pub struct SessionPolicy { pub environment: Vec<(String, String)>, pub local_command: Option, + /// Request forwarding of the local authentication agent on session channels. + /// + /// This is deliberately independent from using an agent to authenticate the + /// SSH connection: OpenSSH `-A` controls forwarding, while bssh + /// `--use-agent` controls client authentication. + #[serde(default)] + pub forward_agent: bool, pub request_pty: bool, pub stdin_null: bool, pub request: SessionRequest, @@ -239,6 +246,7 @@ impl SessionPolicy { Ok(Self { environment, local_command, + forward_agent: config.forward_agent.unwrap_or(false), request_pty, stdin_null, request, diff --git a/src/ssh/session_policy_tests.rs b/src/ssh/session_policy_tests.rs index 5b850640..5d1d9db7 100644 --- a/src/ssh/session_policy_tests.rs +++ b/src/ssh/session_policy_tests.rs @@ -143,6 +143,32 @@ fn stdin_null_disables_automatic_pty_but_not_forced_pty() { assert!(forced.request_pty); } +#[test] +fn forward_agent_is_an_explicit_session_policy_independent_of_authentication() { + let enabled = SessionPolicy::resolve( + &SshHostConfig { + forward_agent: Some(true), + ..Default::default() + }, + &node(), + Some("true"), + CliTtyMode::Default, + false, + ) + .unwrap(); + assert!(enabled.forward_agent); + + let disabled = SessionPolicy::resolve( + &SshHostConfig::default(), + &node(), + Some("true"), + CliTtyMode::Default, + false, + ) + .unwrap(); + assert!(!disabled.forward_agent); +} + #[test] fn session_purpose_tracks_the_resolved_pty_policy() { let interactive = SessionPolicy::resolve( diff --git a/src/ssh/ssh_config/dump.rs b/src/ssh/ssh_config/dump.rs index f4c918a3..df20ea68 100644 --- a/src/ssh/ssh_config/dump.rs +++ b/src/ssh/ssh_config/dump.rs @@ -53,8 +53,10 @@ pub fn render_resolved_config(original_host: &str, config: &SshHostConfig) -> Re "forwardx11trusted", config.forward_x11_trusted.unwrap_or(false), )?; - let forward_agent = raw_option(config, "forwardagent") - .map(|value| tokens.expand_path_for_dump(&value)) + let forward_agent = config + .forward_agent_socket_path + .as_deref() + .map(|value| tokens.expand_path_for_dump(value)) .transpose()? .unwrap_or_else(|| yes_no(config.forward_agent.unwrap_or(false)).to_string()); output.line("forwardagent", forward_agent)?; @@ -66,6 +68,10 @@ pub fn render_resolved_config(original_host: &str, config: &SshHostConfig) -> Re "gssapiauthentication", config.gssapi_authentication.unwrap_or(false), )?; + output.bool( + "gssapidelegatecredentials", + config.gssapi_delegate_credentials.unwrap_or(false), + )?; output.bool("hashknownhosts", config.hash_known_hosts.unwrap_or(false))?; output.bool( "hostbasedauthentication", diff --git a/src/ssh/ssh_config/parser/options/authentication.rs b/src/ssh/ssh_config/parser/options/authentication.rs index 76a593fb..54692e7c 100644 --- a/src/ssh/ssh_config/parser/options/authentication.rs +++ b/src/ssh/ssh_config/parser/options/authentication.rs @@ -224,6 +224,12 @@ pub(super) fn parse_authentication_option( } host.gssapi_authentication = Some(parse_yes_no(&args[0], line_number)?); } + "gssapidelegatecredentials" => { + if args.is_empty() { + anyhow::bail!("GSSAPIDelegateCredentials requires a value at line {line_number}"); + } + host.gssapi_delegate_credentials = Some(parse_yes_no(&args[0], line_number)?); + } "preferredauthentications" => { if args.is_empty() { anyhow::bail!("PreferredAuthentications requires a value at line {line_number}"); diff --git a/src/ssh/ssh_config/parser/options/forwarding.rs b/src/ssh/ssh_config/parser/options/forwarding.rs index b8a07daa..6cf51716 100644 --- a/src/ssh/ssh_config/parser/options/forwarding.rs +++ b/src/ssh/ssh_config/parser/options/forwarding.rs @@ -33,11 +33,27 @@ pub(super) fn parse_forwarding_option( if args.is_empty() { anyhow::bail!("ForwardAgent requires a value at line {line_number}"); } - // OpenSSH also accepts an agent socket path. Runtime forwarding is - // declared unimplemented, so retain that raw value for `-G` while - // preserving the existing typed yes/no representation when possible. if matches!(args[0].to_ascii_lowercase().as_str(), "yes" | "no") { host.forward_agent = Some(parse_yes_no(&args[0], line_number)?); + } else { + let path = &args[0]; + if path.contains('\0') { + anyhow::bail!("ForwardAgent socket path contains NUL at line {line_number}"); + } + if let Some(name) = path.strip_prefix('$') + && (name.is_empty() + || !name.bytes().enumerate().all(|(index, byte)| { + byte == b'_' + || byte.is_ascii_alphabetic() + || (index > 0 && byte.is_ascii_digit()) + })) + { + anyhow::bail!( + "Invalid ForwardAgent environment variable name '{path}' at line {line_number}" + ); + } + host.forward_agent = Some(true); + host.forward_agent_socket_path = Some(path.clone()); } } "forwardx11" => { diff --git a/src/ssh/ssh_config/parser/options/mod.rs b/src/ssh/ssh_config/parser/options/mod.rs index 14c9761b..407a635b 100644 --- a/src/ssh/ssh_config/parser/options/mod.rs +++ b/src/ssh/ssh_config/parser/options/mod.rs @@ -54,13 +54,17 @@ pub fn parse_option( if reported_diagnostics.insert(format!("unknown:{accepted_keyword}")) { let keyword = escape_field(accepted_keyword); let location = source.location(); - crate::diagnosticln!("Unknown SSH config option '{keyword}' at {location}"); + crate::warningln!("Unknown SSH config option '{keyword}' at {location}"); } return Ok(()); }; let keyword = spec.canonical; - if spec.support == support::KeywordSupport::Unimplemented { + let enforced_gssapi_disable = keyword == "gssapidelegatecredentials" + && args + .first() + .is_some_and(|value| value.eq_ignore_ascii_case("no")); + if spec.support == support::KeywordSupport::Unimplemented && !enforced_gssapi_disable { validate_retained_option(keyword, args, line_number)?; if !args.is_empty() { host.unimplemented_options @@ -69,7 +73,7 @@ pub fn parse_option( } if reported_diagnostics.insert(format!("unsupported:{keyword}")) { let location = source.location(); - crate::diagnosticln!( + crate::warningln!( "Unsupported SSH config option '{keyword}' at {location}; bssh parses this value for inspection but does not implement its runtime behavior" ); } @@ -92,6 +96,7 @@ pub fn parse_option( | "kbdinteractiveauthentication" | "challengeresponseauthentication" | "gssapiauthentication" + | "gssapidelegatecredentials" | "preferredauthentications" | "hostbasedauthentication" | "hostbasedacceptedalgorithms" diff --git a/src/ssh/ssh_config/parser/options/support.rs b/src/ssh/ssh_config/parser/options/support.rs index 69b568f6..0a7f7153 100644 --- a/src/ssh/ssh_config/parser/options/support.rs +++ b/src/ssh/ssh_config/parser/options/support.rs @@ -78,6 +78,11 @@ pub(super) const ACCEPTED_KEYWORDS: &[(&str, &str, KeywordSupport)] = &[ "gssapiauthentication", Unimplemented, ), + ( + "gssapidelegatecredentials", + "gssapidelegatecredentials", + Unimplemented, + ), ( "preferredauthentications", "preferredauthentications", @@ -154,7 +159,7 @@ pub(super) const ACCEPTED_KEYWORDS: &[(&str, &str, KeywordSupport)] = &[ ), ("requiredrsasize", "requiredrsasize", Unimplemented), ("fingerprinthash", "fingerprinthash", Unimplemented), - ("forwardagent", "forwardagent", Unimplemented), + ("forwardagent", "forwardagent", Runtime(Session)), ("forwardx11", "forwardx11", Unimplemented), ("localforward", "localforward", Runtime(Forwarding)), ("remoteforward", "remoteforward", Runtime(Forwarding)), @@ -207,7 +212,7 @@ pub(super) const ACCEPTED_KEYWORDS: &[(&str, &str, KeywordSupport)] = &[ ("setenv", "setenv", Runtime(Session)), ("requesttty", "requesttty", Runtime(Session)), ("escapechar", "escapechar", Unimplemented), - ("loglevel", "loglevel", Unimplemented), + ("loglevel", "loglevel", Runtime(Session)), ("syslogfacility", "syslogfacility", Unimplemented), ("protocol", "protocol", Unimplemented), ("permitlocalcommand", "permitlocalcommand", Runtime(Session)), @@ -221,10 +226,10 @@ pub(super) const ACCEPTED_KEYWORDS: &[(&str, &str, KeywordSupport)] = &[ ( "forkafterauthentication", "forkafterauthentication", - Unimplemented, + Runtime(Session), ), ("sessiontype", "sessiontype", Runtime(Session)), - ("stdinnull", "stdinnull", Unimplemented), + ("stdinnull", "stdinnull", Runtime(Session)), ("cipher", "cipher", Unimplemented), ("fallbacktorsh", "fallbacktorsh", Unimplemented), ( @@ -300,9 +305,9 @@ mod tests { use super::*; use std::collections::HashSet; - const ACCEPTED_SPELLING_COUNT: usize = 107; - const RUNTIME_SPELLING_COUNT: usize = 54; - const UNIMPLEMENTED_SPELLING_COUNT: usize = 53; + const ACCEPTED_SPELLING_COUNT: usize = 108; + const RUNTIME_SPELLING_COUNT: usize = 58; + const UNIMPLEMENTED_SPELLING_COUNT: usize = 50; #[test] fn accepted_keywords_and_aliases_have_one_consistent_classification() { @@ -367,6 +372,7 @@ mod tests { ("hostkeyalias", HostVerification), ("verifyhostkeydns", HostVerification), ("updatehostkeys", HostVerification), + ("forwardagent", Session), ("localforward", Forwarding), ("remoteforward", Forwarding), ("dynamicforward", Forwarding), @@ -392,11 +398,14 @@ mod tests { ("sendenv", Session), ("setenv", Session), ("requesttty", Session), + ("loglevel", Session), ("permitlocalcommand", Session), ("localcommand", Session), ("remotecommand", Session), ("knownhostscommand", HostVerification), + ("forkafterauthentication", Session), ("sessiontype", Session), + ("stdinnull", Session), ]; let runtime = ACCEPTED_KEYWORDS .iter() @@ -421,6 +430,7 @@ mod tests { "identityagent", "kbdinteractiveauthentication", "gssapiauthentication", + "gssapidelegatecredentials", "hostbasedauthentication", "hostbasedacceptedalgorithms", "enablesshkeysign", @@ -430,7 +440,6 @@ mod tests { "visualhostkey", "requiredrsasize", "fingerprinthash", - "forwardagent", "forwardx11", "gatewayports", "permitremoteopen", @@ -438,11 +447,8 @@ mod tests { "forwardx11trusted", "connecttimeout", "escapechar", - "loglevel", "syslogfacility", "protocol", - "forkafterauthentication", - "stdinnull", "cipher", "fallbacktorsh", "globalknownhostsfile2", diff --git a/src/ssh/ssh_config/parser/tests.rs b/src/ssh/ssh_config/parser/tests.rs index 17394928..c018542c 100644 --- a/src/ssh/ssh_config/parser/tests.rs +++ b/src/ssh/ssh_config/parser/tests.rs @@ -206,6 +206,51 @@ Match all assert_eq!(hosts[0].server_alive_count_max, Some(3)); } +#[test] +fn gssapi_credential_delegation_disable_is_enforced_but_enable_is_retained_as_unsupported() { + let disabled = parse( + "Host disabled\n GSSAPIDelegateCredentials no\n\ + Host enabled\n GSSAPIDelegateCredentials yes\n", + ) + .unwrap(); + + assert_eq!(disabled[0].gssapi_delegate_credentials, Some(false)); + assert!( + !disabled[0] + .unimplemented_options + .contains_key("gssapidelegatecredentials") + ); + assert_eq!(disabled[1].gssapi_delegate_credentials, Some(true)); + assert_eq!( + disabled[1] + .unimplemented_options + .get("gssapidelegatecredentials") + .map(Vec::as_slice), + Some(["yes".to_string()].as_slice()) + ); +} + +#[test] +fn forward_agent_accepts_explicit_and_environment_selected_sockets() { + let hosts = parse( + "Host explicit\n ForwardAgent /tmp/agent.sock\n\ + Host environment\n ForwardAgent $FORWARD_AGENT_SOCK\n", + ) + .unwrap(); + + assert_eq!(hosts[0].forward_agent, Some(true)); + assert_eq!( + hosts[0].forward_agent_socket_path.as_deref(), + Some("/tmp/agent.sock") + ); + assert_eq!(hosts[1].forward_agent, Some(true)); + assert_eq!( + hosts[1].forward_agent_socket_path.as_deref(), + Some("$FORWARD_AGENT_SOCK") + ); + assert!(parse("Host invalid\n ForwardAgent $9INVALID\n").is_err()); +} + #[test] fn test_parse_match_with_exec() { use crate::ssh::ssh_config::match_directive::MatchCondition; diff --git a/src/ssh/ssh_config/resolver.rs b/src/ssh/ssh_config/resolver.rs index be41f9de..8e5f212f 100644 --- a/src/ssh/ssh_config/resolver.rs +++ b/src/ssh/ssh_config/resolver.rs @@ -289,6 +289,7 @@ pub(super) fn merge_host_config(base: &mut SshHostConfig, overlay: &SshHostConfi } if base.forward_agent.is_none() && overlay.forward_agent.is_some() { base.forward_agent = overlay.forward_agent; + base.forward_agent_socket_path = overlay.forward_agent_socket_path.clone(); } if base.forward_x11.is_none() && overlay.forward_x11.is_some() { base.forward_x11 = overlay.forward_x11; @@ -331,6 +332,9 @@ pub(super) fn merge_host_config(base: &mut SshHostConfig, overlay: &SshHostConfi if base.gssapi_authentication.is_none() && overlay.gssapi_authentication.is_some() { base.gssapi_authentication = overlay.gssapi_authentication; } + if base.gssapi_delegate_credentials.is_none() && overlay.gssapi_delegate_credentials.is_some() { + base.gssapi_delegate_credentials = overlay.gssapi_delegate_credentials; + } if base.host_key_algorithms.is_empty() && !overlay.host_key_algorithms.is_empty() { base.host_key_algorithms = overlay.host_key_algorithms.clone(); base.resolved_host_key_algorithms = overlay.resolved_host_key_algorithms.clone(); diff --git a/src/ssh/ssh_config/types.rs b/src/ssh/ssh_config/types.rs index 29ad2634..50f3c741 100644 --- a/src/ssh/ssh_config/types.rs +++ b/src/ssh/ssh_config/types.rs @@ -86,6 +86,9 @@ pub struct SshHostConfig { /// Raw `GlobalKnownHostsFile` values in OpenSSH lookup order. pub global_known_hosts_file: Option>, pub forward_agent: Option, + /// Optional explicit agent socket path or `$ENVIRONMENT_VARIABLE` selected + /// by `ForwardAgent`. `None` uses `SSH_AUTH_SOCK` when forwarding is enabled. + pub forward_agent_socket_path: Option, pub forward_x11: Option, pub server_alive_interval: Option, pub server_alive_count_max: Option, @@ -99,6 +102,11 @@ pub struct SshHostConfig { pub password_authentication: Option, pub keyboard_interactive_authentication: Option, pub gssapi_authentication: Option, + /// Whether GSSAPI credentials may be delegated. + /// + /// bssh does not implement GSSAPI authentication, so `no` is an enforced + /// safe state while `yes` remains explicitly unsupported. + pub gssapi_delegate_credentials: Option, pub host_key_algorithms: Vec, pub kex_algorithms: Vec, pub ciphers: Vec, diff --git a/src/ssh/tokio_client/connection.rs b/src/ssh/tokio_client/connection.rs index 1f31230b..a425f8b5 100644 --- a/src/ssh/tokio_client/connection.rs +++ b/src/ssh/tokio_client/connection.rs @@ -23,7 +23,7 @@ use std::net::{IpAddr, SocketAddr}; use std::path::PathBuf; use std::sync::{ Arc, Mutex, - atomic::{AtomicBool, Ordering}, + atomic::{AtomicBool, AtomicUsize, Ordering}, }; use std::time::Duration; use std::{fmt::Debug, io}; @@ -119,6 +119,14 @@ pub struct SshConnectionConfig { /// Optional local interface selected by ssh_config `BindInterface`. pub bind_interface: Option, + /// Optional explicit socket path or `$ENVIRONMENT_VARIABLE` selected by + /// `ForwardAgent`. `None` forwards the current `SSH_AUTH_SOCK`. + pub forward_agent_socket_path: Option, + + /// Suppress authentication banners, as requested by `-q` or + /// `LogLevel QUIET`. + pub suppress_auth_banner: bool, + /// Interactive and bulk socket traffic classes selected by `IPQoS`. pub ip_qos: IpQosPolicy, @@ -175,6 +183,8 @@ impl Default for SshConnectionConfig { tcp_keep_alive: true, bind_address: None, bind_interface: None, + forward_agent_socket_path: None, + suppress_auth_banner: false, ip_qos: IpQosPolicy::default(), session_purpose: SessionPurpose::Bulk, user_known_hosts_files: None, @@ -215,6 +225,7 @@ pub struct SshConnectionConfigResolver { yaml_keepalive_interval: Option, yaml_keepalive_max: Option, cli_address_family: Option, + cli_quiet: bool, cli_host_key_alias: Option, cli_proxy_jump: Option, yaml_proxy_jump: Option, @@ -284,6 +295,16 @@ impl SshConnectionConfigResolver { self } + /// Apply the command-line quiet policy to every resolved destination. + #[must_use] + pub fn with_cli_quiet(mut self, quiet: bool) -> Self { + if let Some(config) = self.fixed_config.as_mut() { + config.suppress_auth_banner = quiet; + } + self.cli_quiet = quiet; + self + } + #[must_use] pub fn with_cli_host_key_alias(mut self, alias: Option) -> Self { if let (Some(config), Some(alias)) = (self.fixed_config.as_mut(), alias.as_ref()) { @@ -397,6 +418,14 @@ impl SshConnectionConfigResolver { let bind_interface = host_config .as_ref() .and_then(|config| config.bind_interface.clone()); + let forward_agent_socket_path = host_config + .as_ref() + .and_then(|config| config.forward_agent_socket_path.clone()); + let suppress_auth_banner = self.cli_quiet + || host_config + .as_ref() + .and_then(|config| config.log_level.as_deref()) + .is_some_and(|level| level.eq_ignore_ascii_case("quiet")); let ip_qos = host_config .as_ref() .and_then(|config| config.ipqos) @@ -582,6 +611,8 @@ impl SshConnectionConfigResolver { .with_connection_attempts(connection_attempts) .with_tcp_keep_alive(tcp_keep_alive) .with_source_binding(bind_address, bind_interface) + .with_forward_agent_socket_path(forward_agent_socket_path) + .with_suppress_auth_banner(suppress_auth_banner) .with_ip_qos(ip_qos) .with_known_hosts_files(user_known_hosts_files, global_known_hosts_files) .with_host_key_alias(host_key_alias) @@ -721,6 +752,13 @@ impl SshConnectionConfig { self } + /// Set whether server authentication banners should be hidden. + #[must_use] + pub fn with_suppress_auth_banner(mut self, suppress: bool) -> Self { + self.suppress_auth_banner = suppress; + self + } + /// Set the maximum number of keepalive attempts. #[must_use] pub fn with_keepalive_max(mut self, max: usize) -> Self { @@ -771,6 +809,13 @@ impl SshConnectionConfig { self } + /// Select a non-default local agent socket for `ForwardAgent`. + #[must_use] + pub fn with_forward_agent_socket_path(mut self, socket_path: Option) -> Self { + self.forward_agent_socket_path = socket_path; + self + } + /// Set the interactive/bulk traffic-class policy. #[must_use] pub fn with_ip_qos(mut self, ip_qos: IpQosPolicy) -> Self { @@ -1295,6 +1340,71 @@ pub struct Client { hostkey_rotation: HostkeyRotationTasks, forwarding_runtime: Arc, remote_forward_registry: RemoteForwardRegistry, + agent_forwarding: AgentForwardingState, +} + +/// Session-scoped permission for server-initiated agent channels. +/// +/// The handler starts disabled. Each session that successfully requests +/// `auth-agent-req@openssh.com` retains a lease, and the handler accepts new +/// agent channels only while at least one lease is alive. This prevents one +/// `-A` session on a persistent connection from granting agent access to later +/// sessions that did not request forwarding. +#[derive(Debug, Clone, Default)] +pub(crate) struct AgentForwardingState { + active_leases: Arc, + socket_path: Arc>>, +} + +impl AgentForwardingState { + fn acquire(&self) -> AgentForwardingLease { + self.active_leases.fetch_add(1, Ordering::AcqRel); + AgentForwardingLease { + state: self.clone(), + } + } + + fn is_enabled(&self) -> bool { + self.active_leases.load(Ordering::Acquire) > 0 + } + + fn set_socket_path(&self, socket_path: Option) { + *self + .socket_path + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) = socket_path; + } + + #[cfg(unix)] + fn socket_path(&self) -> Option { + let configured = self + .socket_path + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .clone(); + match configured.as_deref() { + None => std::env::var_os("SSH_AUTH_SOCK"), + Some(value) if value.starts_with('$') => std::env::var_os(&value[1..]), + Some("~") => dirs::home_dir().map(std::path::PathBuf::into_os_string), + Some(value) if value.starts_with("~/") => { + dirs::home_dir().map(|home| home.join(&value[2..]).into_os_string()) + } + Some(value) => Some(std::ffi::OsString::from(value)), + } + } +} + +/// Keeps agent forwarding enabled for the lifetime of one requested session. +#[derive(Debug)] +pub(crate) struct AgentForwardingLease { + state: AgentForwardingState, +} + +impl Drop for AgentForwardingLease { + fn drop(&mut self) { + let previous = self.state.active_leases.fetch_sub(1, Ordering::AcqRel); + debug_assert!(previous > 0, "agent forwarding lease count underflow"); + } } #[derive(Debug, Clone)] @@ -1414,6 +1524,7 @@ struct DirectCarrierOptions<'a> { bind_address: Option<&'a str>, bind_interface: Option<&'a str>, ip_qos: IpQosValue, + suppress_auth_banner: bool, } impl Client { @@ -1511,10 +1622,14 @@ impl Client { Arc::clone(&config), policy.clone(), proxy, + ssh_config.suppress_auth_banner, ) .await; match result { Ok(client) => { + client + .agent_forwarding + .set_socket_path(ssh_config.forward_agent_socket_path.clone()); client .initialize_forwarding(&ssh_config.forwarding_plan) .await?; @@ -1568,9 +1683,13 @@ impl Client { bind_address: ssh_config.bind_address.as_deref(), bind_interface: ssh_config.bind_interface.as_deref(), ip_qos: ssh_config.selected_ip_qos(), + suppress_auth_banner: ssh_config.suppress_auth_banner, }, ) .await?; + client + .agent_forwarding + .set_socket_path(ssh_config.forward_agent_socket_path.clone()); client .initialize_forwarding(&ssh_config.forwarding_plan) .await?; @@ -1587,6 +1706,7 @@ impl Client { config: Arc, policy: KnownHostRuntimePolicy, proxy: &ProxyCommandConfig, + suppress_auth_banner: bool, ) -> Result { let proxy_session = spawn_proxy_command(proxy, host, port, username)?; let process = Arc::clone(&proxy_session.process); @@ -1608,10 +1728,12 @@ impl Client { verification_address, server_check, policy, - ); + ) + .with_suppress_auth_banner(suppress_auth_banner); let fatal_transport = handler.fatal_transport_state(); let hostkey_rotation = handler.hostkey_rotation_tasks(); let remote_forward_registry = handler.remote_forward_registry(); + let agent_forwarding = handler.agent_forwarding_state(); let mut handle = match russh::client::connect_stream(config, proxy_session.stream, handler) .await { @@ -1648,6 +1770,7 @@ impl Client { hostkey_rotation, forwarding_runtime: Arc::new(ForwardingRuntime::default()), remote_forward_registry, + agent_forwarding, }; client.flush_hostkey_updates().await; Ok(client) @@ -1681,6 +1804,7 @@ impl Client { bind_address: None, bind_interface: None, ip_qos: IpQosValue::None, + suppress_auth_banner: false, }, ) .await @@ -1715,6 +1839,7 @@ impl Client { bind_address, bind_interface, ip_qos, + suppress_auth_banner, } = carrier; let connection_attempts = connection_attempts.max(1); let target_host = addr.hostname(); @@ -1813,10 +1938,12 @@ impl Client { } let handler = - ClientHandler::new_with_policy(target_host.clone(), address, server_check, policy); + ClientHandler::new_with_policy(target_host.clone(), address, server_check, policy) + .with_suppress_auth_banner(suppress_auth_banner); let fatal_transport = handler.fatal_transport_state(); let hostkey_rotation = handler.hostkey_rotation_tasks(); let remote_forward_registry = handler.remote_forward_registry(); + let agent_forwarding = handler.agent_forwarding_state(); let mut handle = russh::client::connect_stream(Arc::new(config), stream, handler) .await .map_err(|error| { @@ -1840,6 +1967,7 @@ impl Client { hostkey_rotation, forwarding_runtime: Arc::new(ForwardingRuntime::default()), remote_forward_registry, + agent_forwarding, }; client.flush_hostkey_updates().await; Ok(client) @@ -1857,6 +1985,25 @@ impl Client { self.fatal_transport.take_error().unwrap_or(fallback) } + /// Request OpenSSH agent forwarding on a session channel. + /// + /// Permission becomes visible to the connection handler + /// before the request is sent, so a server response cannot race the opt-in. + /// The returned lease revokes that session's permission when dropped. + pub(crate) async fn request_agent_forwarding( + &self, + channel: &russh::Channel, + ) -> Result { + let lease = self.agent_forwarding.acquire(); + if let Err(source) = channel.agent_forward(true).await { + return Err(self.session_error_or(super::Error::CommandExecution { + action: "agent forwarding request", + source, + })); + } + Ok(lease) + } + /// Create a Client from an existing russh handle and address. /// /// This is used internally for jump host connections where we already have @@ -1872,6 +2019,7 @@ impl Client { address, FatalTransportState::default(), RemoteForwardRegistry::default(), + AgentForwardingState::default(), ) } @@ -1881,6 +2029,7 @@ impl Client { address: SocketAddr, fatal_transport: FatalTransportState, remote_forward_registry: RemoteForwardRegistry, + agent_forwarding: AgentForwardingState, ) -> Self { Self { connection_handle: handle.clone(), @@ -1892,6 +2041,7 @@ impl Client { hostkey_rotation: HostkeyRotationTasks::new(false), forwarding_runtime: Arc::new(ForwardingRuntime::default()), remote_forward_registry, + agent_forwarding, } } @@ -1902,6 +2052,7 @@ impl Client { fatal_transport: FatalTransportState, hostkey_rotation: HostkeyRotationTasks, remote_forward_registry: RemoteForwardRegistry, + agent_forwarding: AgentForwardingState, ) -> Self { let client = Self { connection_handle: handle.clone(), @@ -1913,6 +2064,7 @@ impl Client { hostkey_rotation, forwarding_runtime: Arc::new(ForwardingRuntime::default()), remote_forward_registry, + agent_forwarding, }; client.flush_hostkey_updates().await; client @@ -2214,6 +2366,8 @@ pub struct ClientHandler { hostkey_rotation: HostkeyRotationTasks, fatal_transport: FatalTransportState, remote_forward_registry: RemoteForwardRegistry, + agent_forwarding: AgentForwardingState, + suppress_auth_banner: bool, } impl ClientHandler { @@ -2247,9 +2401,16 @@ impl ClientHandler { hostkey_rotation, fatal_transport: FatalTransportState::default(), remote_forward_registry: RemoteForwardRegistry::default(), + agent_forwarding: AgentForwardingState::default(), + suppress_auth_banner: false, } } + fn with_suppress_auth_banner(mut self, suppress: bool) -> Self { + self.suppress_auth_banner = suppress; + self + } + pub(crate) fn hostkey_rotation_tasks(&self) -> HostkeyRotationTasks { self.hostkey_rotation.clone() } @@ -2262,6 +2423,10 @@ impl ClientHandler { self.remote_forward_registry.clone() } + pub(crate) fn agent_forwarding_state(&self) -> AgentForwardingState { + self.agent_forwarding.clone() + } + async fn run_known_hosts_lookup( &self, hostname: &str, @@ -2777,6 +2942,25 @@ where impl Handler for ClientHandler { type Error = super::Error; + fn auth_banner( + &mut self, + banner: &str, + _session: &mut russh::client::Session, + ) -> impl std::future::Future> + Send { + let suppress = self.suppress_auth_banner; + let banner = banner.to_string(); + async move { + if !suppress { + use std::io::Write as _; + + let mut stderr = std::io::stderr().lock(); + stderr.write_all(banner.as_bytes())?; + stderr.flush()?; + } + Ok(()) + } + } + async fn disconnected( &mut self, reason: DisconnectReason, @@ -2818,6 +3002,66 @@ impl Handler for ClientHandler { } } + fn server_channel_open_agent_forward( + &mut self, + channel: russh::Channel, + reply: russh::client::ChannelOpenHandle, + _session: &mut russh::client::Session, + ) -> impl std::future::Future> + Send { + let forwarding = self.agent_forwarding.clone(); + async move { + if !forwarding.is_enabled() { + reply + .reject(russh::ChannelOpenFailure::AdministrativelyProhibited) + .await; + return Ok(()); + } + + #[cfg(unix)] + { + let Some(socket_path) = forwarding.socket_path() else { + tracing::warn!( + "The server requested agent forwarding, but the selected agent socket is unavailable" + ); + reply.reject(russh::ChannelOpenFailure::ConnectFailed).await; + return Ok(()); + }; + let mut agent = match tokio::net::UnixStream::connect(&socket_path).await { + Ok(agent) => agent, + Err(error) => { + tracing::warn!( + path = %std::path::Path::new(&socket_path).display(), + %error, + "Could not connect to the local SSH agent for forwarding" + ); + reply.reject(russh::ChannelOpenFailure::ConnectFailed).await; + return Ok(()); + } + }; + + reply.accept().await; + tokio::spawn(async move { + let mut stream = channel.into_stream(); + if let Err(error) = tokio::io::copy_bidirectional(&mut stream, &mut agent).await + { + tracing::debug!(%error, "SSH agent forwarding channel closed with an I/O error"); + } + }); + } + + #[cfg(not(unix))] + { + let _ = channel; + tracing::warn!("SSH agent forwarding requires a Unix-domain agent socket"); + reply + .reject(russh::ChannelOpenFailure::AdministrativelyProhibited) + .await; + } + + Ok(()) + } + } + async fn check_server_key( &mut self, server_key: &russh::keys::PublicKeyOrCertificate, @@ -3143,6 +3387,8 @@ impl Handler for ClientHandler { mod fatal_transport_tests { use super::*; use crate::ssh::tokio_client::{Error, TransportIntegrityCause}; + #[cfg(unix)] + use crate::test_helpers::EnvGuard; #[test] fn first_typed_integrity_cause_is_preserved_and_consumed_once() { @@ -3232,6 +3478,45 @@ mod fatal_transport_tests { first.fatal_transport_state().take_error().is_none(), "a consumed cause must not be reused by another operation" ); + + let first_agent = first.agent_forwarding_state(); + assert!(!first_agent.is_enabled()); + let first_lease = first_agent.acquire(); + assert!(first_clone.agent_forwarding_state().is_enabled()); + assert!( + !second.agent_forwarding_state().is_enabled(), + "agent forwarding permission must not leak into another connection" + ); + let second_lease = first_agent.acquire(); + drop(first_lease); + assert!( + first_clone.agent_forwarding_state().is_enabled(), + "another active session must retain permission" + ); + drop(second_lease); + assert!( + !first_clone.agent_forwarding_state().is_enabled(), + "the final session lease must revoke permission" + ); + } + + #[cfg(unix)] + #[test] + #[serial_test::serial] + fn agent_socket_selection_supports_explicit_paths_and_environment_names() { + let state = AgentForwardingState::default(); + state.set_socket_path(Some("/tmp/explicit-agent.sock".to_string())); + assert_eq!( + state.socket_path().as_deref(), + Some(std::ffi::OsStr::new("/tmp/explicit-agent.sock")) + ); + + let _socket = EnvGuard::set("BSSH_TEST_FORWARD_AGENT_SOCK", "/tmp/env-agent.sock"); + state.set_socket_path(Some("$BSSH_TEST_FORWARD_AGENT_SOCK".to_string())); + assert_eq!( + state.socket_path().as_deref(), + Some(std::ffi::OsStr::new("/tmp/env-agent.sock")) + ); } } diff --git a/src/ssh/tokio_client/connection_tests.rs b/src/ssh/tokio_client/connection_tests.rs index 470a20dc..7726eff3 100644 --- a/src/ssh/tokio_client/connection_tests.rs +++ b/src/ssh/tokio_client/connection_tests.rs @@ -204,6 +204,26 @@ Host target assert_eq!(config.keepalive_max, 9); } +#[test] +fn auth_banner_suppression_resolves_from_quiet_cli_or_log_level() { + let ssh_config = + SshConfig::parse("Host quiet-config\n LogLevel QUIET\nHost normal\n LogLevel INFO\n") + .unwrap(); + let resolver = SshConnectionConfigResolver::new().with_ssh_config(Some(ssh_config.clone())); + + assert!( + resolver + .resolve_for_host("quiet-config") + .suppress_auth_banner + ); + assert!(!resolver.resolve_for_host("normal").suppress_auth_banner); + + let quiet_cli = SshConnectionConfigResolver::new() + .with_ssh_config(Some(ssh_config)) + .with_cli_quiet(true); + assert!(quiet_cli.resolve_for_host("normal").suppress_auth_banner); +} + #[test] fn stdio_forward_defaults_clear_forwards_but_preserves_explicit_no() { let implicit = @@ -804,6 +824,20 @@ fn source_binding_and_ipqos_reach_the_connection_config() { ); } +#[test] +fn forward_agent_socket_path_reaches_the_connection_config() { + let ssh_config = + SshConfig::parse("Host target\n ForwardAgent $FORWARD_AGENT_SOCK\n").unwrap(); + let config = SshConnectionConfigResolver::new() + .with_ssh_config(Some(ssh_config)) + .resolve_for_host("target"); + + assert_eq!( + config.forward_agent_socket_path.as_deref(), + Some("$FORWARD_AGENT_SOCK") + ); +} + #[tokio::test] async fn direct_socket_binds_the_requested_loopback_source() { let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); diff --git a/src/ssh/tokio_client/mod.rs b/src/ssh/tokio_client/mod.rs index 33d7c19d..1fc75289 100644 --- a/src/ssh/tokio_client/mod.rs +++ b/src/ssh/tokio_client/mod.rs @@ -40,6 +40,7 @@ pub use algorithms::{supported_cipher_names, supported_mac_names}; pub use auth_policy::SshAuthenticationPolicy; pub use authentication::{AuthKeyboardInteractive, AuthMethod, ServerCheckMethod}; pub use channel_manager::{CommandExecutedResult, CommandOutput}; +pub(crate) use connection::AgentForwardingLease; pub use connection::{ Client, ClientHandler, DEFAULT_KEEPALIVE_INTERVAL, DEFAULT_KEEPALIVE_MAX, SshConnectionConfig, SshConnectionConfigResolver, diff --git a/src/ssh/tokio_client/session.rs b/src/ssh/tokio_client/session.rs index 320a7085..97b110d7 100644 --- a/src/ssh/tokio_client/session.rs +++ b/src/ssh/tokio_client/session.rs @@ -22,7 +22,7 @@ use tokio::sync::mpsc::Sender; use crate::ssh::{SessionPolicy, SessionRequest}; use super::channel_manager::{CommandExecutedResult, CommandOutput, CommandOutputBuffer}; -use super::connection::Client; +use super::connection::{AgentForwardingLease, Client}; impl Client { pub async fn execute_session_streaming( @@ -70,7 +70,7 @@ impl Client { if matches!(policy.request, SessionRequest::None) { return Ok(0); } - let channel = self.open_policy_channel(policy, terminal).await?; + let (channel, _agent_forwarding_lease) = self.open_policy_channel(policy, terminal).await?; self.drain_policy_channel_with_input(channel, sender, input) .await } @@ -98,13 +98,19 @@ impl Client { &self, policy: &SessionPolicy, terminal: Option<&str>, - ) -> Result, super::Error> { + ) -> Result<(Channel, Option), super::Error> { let channel = self .connection_handle .channel_open_session() .await .map_err(|source| self.session_error_or(super::Error::ChannelOpen { source }))?; + let agent_forwarding_lease = if policy.forward_agent { + Some(self.request_agent_forwarding(&channel).await?) + } else { + None + }; + if policy.request_pty { let inherited_terminal; let terminal = match terminal { @@ -157,7 +163,7 @@ impl Client { source, }) })?; - Ok(channel) + Ok((channel, agent_forwarding_lease)) } async fn drain_policy_channel_with_input( diff --git a/src/utils/diagnostics.rs b/src/utils/diagnostics.rs index 69b86dde..73364019 100644 --- a/src/utils/diagnostics.rs +++ b/src/utils/diagnostics.rs @@ -25,11 +25,22 @@ use std::io::{self, Write}; use std::os::unix::fs::OpenOptionsExt; use std::path::Path; use std::sync::Mutex; +use std::sync::atomic::{AtomicBool, Ordering}; use anyhow::{Context, Result}; use tracing_subscriber::fmt::MakeWriter; static LOG_FILE: Mutex> = Mutex::new(None); +static QUIET_WARNINGS: AtomicBool = AtomicBool::new(false); + +/// Select whether non-error diagnostics should be suppressed. +/// +/// Errors continue to use [`write_line`] and are always emitted. This switch +/// is intentionally process-wide because the CLI initializes it once before +/// any configuration warnings are produced. +pub fn set_quiet_warnings(quiet: bool) { + QUIET_WARNINGS.store(quiet, Ordering::Relaxed); +} /// Open `path` as the destination for subsequent bssh diagnostics. /// @@ -84,6 +95,13 @@ pub fn write_line(arguments: fmt::Arguments<'_>) { } } +/// Write one warning unless OpenSSH-compatible quiet mode is active. +pub fn write_warning_line(arguments: fmt::Arguments<'_>) { + if !QUIET_WARNINGS.load(Ordering::Relaxed) { + write_line(arguments); + } +} + /// A tracing writer that emits each formatted event as one diagnostic write. #[derive(Debug, Default)] pub struct DiagnosticWriter { @@ -132,3 +150,10 @@ macro_rules! diagnosticln { $crate::utils::diagnostics::write_line(format_args!($($argument)*)) }; } + +#[macro_export] +macro_rules! warningln { + ($($argument:tt)*) => { + $crate::utils::diagnostics::write_warning_line(format_args!($($argument)*)) + }; +} diff --git a/tests/agent_forwarding_live_test.rs b/tests/agent_forwarding_live_test.rs new file mode 100644 index 00000000..6a968e7a --- /dev/null +++ b/tests/agent_forwarding_live_test.rs @@ -0,0 +1,309 @@ +// Copyright 2025 Lablup Inc. and Jeongkyu Shin +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#![cfg(unix)] + +mod common; + +use std::net::{Ipv4Addr, SocketAddr}; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::sync::{Arc, Mutex}; +use std::time::Duration; + +use anyhow::Result; +use bssh::ssh::tokio_client::{AuthMethod, Client, ServerCheckMethod, SshConnectionConfig}; +use bssh::ssh::{SessionPolicy, SessionRequest}; +use common::EnvGuard; +use russh::keys::{Algorithm, PrivateKey}; +use russh::server::{self, Msg, Server, Session}; +use russh::{Channel, ChannelId, ChannelOpenFailure}; +use serial_test::serial; +use tempfile::TempDir; +use tokio::io::{AsyncReadExt, AsyncWriteExt}; +use tokio::net::{TcpListener, UnixListener}; +use tokio::sync::Notify; +use tokio::task::JoinHandle; +use tokio::time::timeout; + +const TEST_TIMEOUT: Duration = Duration::from_secs(5); +const AGENT_REQUEST: &[u8] = b"agent-request"; +const AGENT_RESPONSE: &[u8] = b"agent-response"; + +#[derive(Default)] +struct ServerState { + agent_requests: AtomicUsize, + handle: Mutex>, + held_session_started: Notify, + release_held_session: Notify, +} + +impl ServerState { + fn handle(&self) -> server::Handle { + self.handle + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .clone() + .expect("session handle must be captured before opening an agent channel") + } +} + +#[derive(Clone)] +struct AgentForwardingServer { + state: Arc, +} + +impl Server for AgentForwardingServer { + type Handler = Self; + + fn new_client(&mut self, _peer_addr: Option) -> Self::Handler { + self.clone() + } +} + +impl server::Handler for AgentForwardingServer { + type Error = anyhow::Error; + + async fn auth_password( + &mut self, + _user: &str, + password: &str, + ) -> Result { + if password == "test" { + Ok(server::Auth::Accept) + } else { + Ok(server::Auth::Reject { + proceed_with_methods: None, + partial_success: false, + }) + } + } + + async fn channel_open_session( + &mut self, + _channel: Channel, + reply: server::ChannelOpenHandle, + session: &mut Session, + ) -> Result<(), Self::Error> { + *self + .state + .handle + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) = Some(session.handle()); + reply.accept().await; + Ok(()) + } + + async fn agent_request( + &mut self, + channel: ChannelId, + session: &mut Session, + ) -> Result { + self.state.agent_requests.fetch_add(1, Ordering::SeqCst); + session.channel_success(channel)?; + Ok(true) + } + + async fn exec_request( + &mut self, + channel: ChannelId, + data: &[u8], + session: &mut Session, + ) -> Result<(), Self::Error> { + session.channel_success(channel)?; + if data == b"hold" { + let state = Arc::clone(&self.state); + let handle = session.handle(); + state.held_session_started.notify_one(); + tokio::spawn(async move { + state.release_held_session.notified().await; + let _ = handle.exit_status_request(channel, 0).await; + let _ = handle.eof(channel).await; + let _ = handle.close(channel).await; + }); + } else { + session.exit_status_request(channel, 0)?; + session.eof(channel)?; + session.close(channel)?; + } + Ok(()) + } +} + +struct RunningSshServer { + address: SocketAddr, + state: Arc, + task: JoinHandle>, +} + +impl RunningSshServer { + async fn start() -> Self { + let listener = TcpListener::bind((Ipv4Addr::LOCALHOST, 0)) + .await + .expect("bind SSH test server"); + let address = listener.local_addr().expect("SSH server address"); + let state = Arc::new(ServerState::default()); + let key = PrivateKey::random(&mut rand::rng(), Algorithm::Ed25519) + .expect("generate SSH test host key"); + let config = Arc::new(server::Config { + keys: vec![key], + auth_rejection_time: Duration::ZERO, + auth_rejection_time_initial: Some(Duration::ZERO), + ..Default::default() + }); + let mut server = AgentForwardingServer { + state: Arc::clone(&state), + }; + let task = tokio::spawn(async move { server.run_on_socket(config, &listener).await }); + Self { + address, + state, + task, + } + } + + async fn connect(&self, config: &SshConnectionConfig) -> Client { + Client::connect_with_ssh_config( + self.address, + "test", + AuthMethod::with_password("test"), + ServerCheckMethod::NoCheck, + config, + ) + .await + .expect("connect to SSH test server") + } + + async fn shutdown(self) { + self.task.abort(); + let _ = self.task.await; + } +} + +fn policy(forward_agent: bool) -> SessionPolicy { + SessionPolicy { + environment: Vec::new(), + local_command: None, + forward_agent, + request_pty: false, + stdin_null: true, + request: SessionRequest::Exec(if forward_agent { "hold" } else { "true" }.to_string()), + } +} + +#[tokio::test] +#[serial] +async fn agent_channel_bridges_only_after_forwarding_is_requested() { + let directory = TempDir::new().expect("agent socket tempdir"); + + let enabled_socket = directory.path().join("enabled-agent.sock"); + let enabled_listener = UnixListener::bind(&enabled_socket).expect("bind fake SSH agent"); + let enabled_agent = tokio::spawn(async move { + let (mut stream, _) = timeout(TEST_TIMEOUT, enabled_listener.accept()) + .await + .expect("client did not connect to fake SSH agent") + .expect("accept fake SSH agent connection"); + let mut request = vec![0; AGENT_REQUEST.len()]; + timeout(TEST_TIMEOUT, stream.read_exact(&mut request)) + .await + .expect("agent request timed out") + .expect("read forwarded agent request"); + assert_eq!(request, AGENT_REQUEST); + stream + .write_all(AGENT_RESPONSE) + .await + .expect("write fake agent response"); + }); + + { + let _socket = EnvGuard::remove("SSH_AUTH_SOCK"); + let ssh = RunningSshServer::start().await; + let config = SshConnectionConfig::default() + .with_forward_agent_socket_path(Some(enabled_socket.to_string_lossy().into_owned())); + let client = ssh.connect(&config).await; + let session_client = client.clone(); + let session_task = + tokio::spawn(async move { session_client.execute_session(&policy(true)).await }); + timeout(TEST_TIMEOUT, ssh.state.held_session_started.notified()) + .await + .expect("forwarded session did not start"); + assert_eq!(ssh.state.agent_requests.load(Ordering::SeqCst), 1); + + let channel = timeout(TEST_TIMEOUT, ssh.state.handle().channel_open_agent()) + .await + .expect("opening forwarded agent channel timed out") + .expect("client rejected enabled agent channel"); + let mut stream = channel.into_stream(); + stream + .write_all(AGENT_REQUEST) + .await + .expect("write request through agent channel"); + let mut response = vec![0; AGENT_RESPONSE.len()]; + timeout(TEST_TIMEOUT, stream.read_exact(&mut response)) + .await + .expect("forwarded agent response timed out") + .expect("read response through agent channel"); + assert_eq!(response, AGENT_RESPONSE); + + enabled_agent.await.expect("fake SSH agent task failed"); + ssh.state.release_held_session.notify_one(); + timeout(TEST_TIMEOUT, session_task) + .await + .expect("forwarded session did not finish") + .expect("forwarded session task failed") + .expect("execute session with agent forwarding"); + + let error = timeout(TEST_TIMEOUT, ssh.state.handle().channel_open_agent()) + .await + .expect("post-session agent channel rejection timed out") + .expect_err("client retained agent permission after the forwarded session ended"); + assert!(matches!( + error, + russh::Error::ChannelOpenFailure(ChannelOpenFailure::AdministrativelyProhibited) + )); + + client.disconnect().await.expect("disconnect SSH client"); + ssh.shutdown().await; + } + + let disabled_socket = directory.path().join("disabled-agent.sock"); + let disabled_listener = UnixListener::bind(&disabled_socket).expect("bind unused SSH agent"); + { + let _socket = EnvGuard::set("SSH_AUTH_SOCK", &disabled_socket); + let ssh = RunningSshServer::start().await; + let client = ssh.connect(&SshConnectionConfig::default()).await; + client + .execute_session(&policy(false)) + .await + .expect("execute session without agent forwarding"); + assert_eq!(ssh.state.agent_requests.load(Ordering::SeqCst), 0); + + let error = timeout(TEST_TIMEOUT, ssh.state.handle().channel_open_agent()) + .await + .expect("agent channel rejection timed out") + .expect_err("client accepted agent channel without ForwardAgent opt-in"); + assert!(matches!( + error, + russh::Error::ChannelOpenFailure(ChannelOpenFailure::AdministrativelyProhibited) + )); + assert!( + timeout(Duration::from_millis(100), disabled_listener.accept()) + .await + .is_err(), + "disabled forwarding must not connect to SSH_AUTH_SOCK" + ); + + client.disconnect().await.expect("disconnect SSH client"); + ssh.shutdown().await; + } +} diff --git a/tests/connect_timeout_test.rs b/tests/connect_timeout_test.rs index cad73d8b..98cafb6c 100644 --- a/tests/connect_timeout_test.rs +++ b/tests/connect_timeout_test.rs @@ -148,7 +148,7 @@ fn test_connect_timeout_with_cluster() { .args([ "--connect-timeout", "5", - "-C", + "--cluster", "nonexistent_cluster", "echo", "test", diff --git a/tests/control_multiplexing_live_test.rs b/tests/control_multiplexing_live_test.rs index 98fa9f4c..bd79d264 100644 --- a/tests/control_multiplexing_live_test.rs +++ b/tests/control_multiplexing_live_test.rs @@ -1,4 +1,6 @@ use std::net::{Ipv4Addr, SocketAddr}; +use std::path::Path; +use std::process::Stdio; use std::sync::Arc; use std::sync::atomic::{AtomicUsize, Ordering}; use std::time::Duration; @@ -18,6 +20,7 @@ use russh::server::{self, Msg, Server, Session}; use russh::{Channel, ChannelId}; use tempfile::TempDir; use tokio::net::TcpListener; +use tokio::sync::Notify; use tokio::task::JoinHandle; use tokio::time::timeout; @@ -27,6 +30,8 @@ const TEST_TIMEOUT: Duration = Duration::from_secs(5); struct ServerState { authentications: AtomicUsize, sessions: AtomicUsize, + commands_completed: AtomicUsize, + release_blocked_command: Notify, } #[derive(Clone)] @@ -48,10 +53,17 @@ impl server::Handler for MultiplexTestServer { async fn auth_password( &mut self, _user: &str, - _password: &str, + password: &str, ) -> Result { self.state.authentications.fetch_add(1, Ordering::SeqCst); - Ok(server::Auth::Accept) + if password == "test" { + Ok(server::Auth::Accept) + } else { + Ok(server::Auth::Reject { + proceed_with_methods: None, + partial_success: false, + }) + } } async fn channel_open_session( @@ -73,9 +85,12 @@ impl server::Handler for MultiplexTestServer { ) -> Result<(), Self::Error> { session.channel_success(channel)?; session.data(channel, data.to_vec())?; - if data == b"slow" { + if data == b"blocked" { + self.state.release_blocked_command.notified().await; + } else if data == b"slow" { tokio::time::sleep(Duration::from_millis(250)).await; } + self.state.commands_completed.fetch_add(1, Ordering::SeqCst); session.exit_status_request(channel, 0)?; session.eof(channel)?; session.close(channel)?; @@ -125,6 +140,7 @@ fn session(command: &str) -> (SessionPolicy, SessionOpenRequest) { let policy = SessionPolicy { environment: Vec::new(), local_command: None, + forward_agent: false, request_pty: false, stdin_null: true, request: SessionRequest::Exec(command.to_string()), @@ -326,3 +342,294 @@ async fn control_persist_timeout_resets_after_a_new_passenger() { assert_eq!(ssh.state.authentications.load(Ordering::SeqCst), 1); ssh.shutdown().await; } + +async fn run_bssh_background( + ssh: &RunningSshServer, + control_path: Option<&Path>, + password: &str, + extra_args: &[&str], +) -> std::process::Output { + let isolated_home = TempDir::new().expect("isolated bssh home"); + let mut command = tokio::process::Command::new(env!("CARGO_BIN_EXE_bssh")); + command + .arg("--password") + .arg("-o") + .arg("StrictHostKeyChecking=no") + .arg("-p") + .arg(ssh.address.port().to_string()) + .args(extra_args); + if let Some(path) = control_path { + command.arg("-M").arg("-S").arg(path); + } + command + .arg("test@127.0.0.1") + .env("BSSH_PASSWORD", password) + .env("HOME", isolated_home.path()) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + timeout(Duration::from_secs(10), command.output()) + .await + .expect("bssh subprocess timed out") + .expect("run bssh subprocess") +} + +async fn run_bssh_existing_master_passenger( + ssh: &RunningSshServer, + control_path: &Path, +) -> std::process::Output { + let isolated_home = TempDir::new().expect("isolated bssh passenger home"); + let mut command = tokio::process::Command::new(env!("CARGO_BIN_EXE_bssh")); + command + .arg("--password") + .arg("-o") + .arg("StrictHostKeyChecking=no") + .arg("-p") + .arg(ssh.address.port().to_string()) + .arg("-f") + .arg("-S") + .arg(control_path) + .arg("test@127.0.0.1") + .arg("blocked") + // A fallback direct connection would fail, which also proves that the + // passenger reused the master's authenticated transport. + .env("BSSH_PASSWORD", "wrong") + .env("HOME", isolated_home.path()) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + timeout(Duration::from_secs(10), command.output()) + .await + .expect("bssh passenger subprocess timed out") + .expect("run bssh passenger subprocess") +} + +async fn run_bssh_direct_background_command(ssh: &RunningSshServer) -> std::process::Output { + let isolated_home = TempDir::new().expect("isolated direct bssh home"); + let mut command = tokio::process::Command::new(env!("CARGO_BIN_EXE_bssh")); + command + .arg("--password") + .arg("-o") + .arg("StrictHostKeyChecking=no") + .arg("-p") + .arg(ssh.address.port().to_string()) + .arg("-f") + .arg("test@127.0.0.1") + .arg("blocked") + .env("BSSH_PASSWORD", "test") + .env("HOME", isolated_home.path()) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + timeout(Duration::from_secs(10), command.output()) + .await + .expect("direct bssh subprocess timed out") + .expect("run direct bssh subprocess") +} + +async fn stop_background_master(path: &Path) { + send_control_command(path, ControlCommand::Exit, Vec::new(), AddressFamily::Any) + .await + .expect("stop subprocess control master"); + timeout(TEST_TIMEOUT, async { + while path.exists() { + tokio::task::yield_now().await; + } + }) + .await + .expect("background control socket was not removed"); +} + +#[tokio::test] +async fn fork_after_authentication_returns_only_after_one_ready_authentication() { + let ssh = RunningSshServer::start().await; + let directory = TempDir::new().expect("control tempdir"); + let path = directory.path().join("fork-after-auth"); + let output = run_bssh_background( + &ssh, + Some(&path), + "test", + &["-f", "-N", "-o", "ControlPersist=no"], + ) + .await; + assert!( + output.status.success(), + "-f failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + assert!(path.exists(), "-f returned before ControlPath was ready"); + assert_eq!(ssh.state.authentications.load(Ordering::SeqCst), 1); + assert_eq!(ssh.state.sessions.load(Ordering::SeqCst), 0); + + stop_background_master(&path).await; + ssh.shutdown().await; +} + +#[tokio::test] +async fn fork_after_authentication_detaches_direct_session_after_authentication() { + let ssh = RunningSshServer::start().await; + let output = run_bssh_direct_background_command(&ssh).await; + assert!( + output.status.success(), + "direct -f failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + assert_eq!(ssh.state.authentications.load(Ordering::SeqCst), 1); + timeout(TEST_TIMEOUT, async { + while ssh.state.sessions.load(Ordering::SeqCst) != 1 { + tokio::task::yield_now().await; + } + }) + .await + .expect("detached direct command never opened its remote session"); + assert_eq!(ssh.state.commands_completed.load(Ordering::SeqCst), 0); + + ssh.state.release_blocked_command.notify_one(); + timeout(TEST_TIMEOUT, async { + while ssh.state.commands_completed.load(Ordering::SeqCst) != 1 { + tokio::task::yield_now().await; + } + }) + .await + .expect("detached direct command did not finish"); + ssh.shutdown().await; +} + +#[tokio::test] +async fn fork_after_authentication_prepares_existing_master_session_before_detaching() { + let ssh = RunningSshServer::start().await; + let directory = TempDir::new().expect("control tempdir"); + let path = directory.path().join("fork-existing-master"); + let master = run_bssh_background( + &ssh, + Some(&path), + "test", + &["-f", "-N", "-o", "ControlPersist=no"], + ) + .await; + assert!( + master.status.success(), + "master setup failed: {}", + String::from_utf8_lossy(&master.stderr) + ); + + let passenger = run_bssh_existing_master_passenger(&ssh, &path).await; + assert!( + passenger.status.success(), + "existing-master -f failed: {}", + String::from_utf8_lossy(&passenger.stderr) + ); + assert_eq!(ssh.state.authentications.load(Ordering::SeqCst), 1); + timeout(TEST_TIMEOUT, async { + while ssh.state.sessions.load(Ordering::SeqCst) != 1 { + tokio::task::yield_now().await; + } + }) + .await + .expect("prepared passenger never opened its remote session"); + assert_eq!(ssh.state.commands_completed.load(Ordering::SeqCst), 0); + + ssh.state.release_blocked_command.notify_one(); + timeout(TEST_TIMEOUT, async { + while ssh.state.commands_completed.load(Ordering::SeqCst) != 1 { + tokio::task::yield_now().await; + } + }) + .await + .expect("detached passenger did not finish its remote command"); + stop_background_master(&path).await; + ssh.shutdown().await; +} + +#[tokio::test] +async fn control_persist_detaches_master_but_keeps_initial_exit_status_and_one_authentication() { + let ssh = RunningSshServer::start().await; + let directory = TempDir::new().expect("control tempdir"); + let path = directory.path().join("implicit-persist"); + let output = run_bssh_background( + &ssh, + Some(&path), + "test", + &["-M", "-N", "-o", "ControlPersist=yes"], + ) + .await; + assert!( + output.status.success(), + "ControlPersist failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + assert!( + path.exists(), + "persistent master did not remain in background" + ); + assert_eq!(ssh.state.authentications.load(Ordering::SeqCst), 1); + assert_eq!(ssh.state.sessions.load(Ordering::SeqCst), 0); + + stop_background_master(&path).await; + ssh.shutdown().await; +} + +#[tokio::test] +async fn fork_after_authentication_reports_bad_auth_in_foreground() { + let ssh = RunningSshServer::start().await; + let output = run_bssh_background(&ssh, None, "wrong", &["-f", "-N"]).await; + assert_eq!(output.status.code(), Some(255)); + assert!( + !output.stderr.is_empty(), + "authentication failure should remain visible before detach" + ); + ssh.shutdown().await; +} + +#[cfg(unix)] +#[tokio::test] +async fn no_remote_command_keeps_transport_until_ctrl_c_without_opening_a_session() { + use nix::sys::signal::{Signal, killpg}; + use nix::unistd::Pid; + use std::os::unix::process::CommandExt as _; + + let ssh = RunningSshServer::start().await; + let isolated_home = TempDir::new().expect("isolated bssh home"); + let mut command = tokio::process::Command::new(env!("CARGO_BIN_EXE_bssh")); + command + .arg("--password") + .arg("-o") + .arg("StrictHostKeyChecking=no") + .arg("-p") + .arg(ssh.address.port().to_string()) + .arg("-N") + .arg("test@127.0.0.1") + .env("BSSH_PASSWORD", "test") + .env("HOME", isolated_home.path()) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()); + command.as_std_mut().process_group(0); + let mut child = command.spawn().expect("spawn foreground -N"); + let child_pid = child.id().expect("foreground -N pid"); + + timeout(TEST_TIMEOUT, async { + while ssh.state.authentications.load(Ordering::SeqCst) != 1 { + tokio::task::yield_now().await; + } + }) + .await + .expect("foreground -N did not authenticate"); + tokio::time::sleep(Duration::from_millis(100)).await; + assert!( + child.try_wait().expect("inspect foreground -N").is_none(), + "-N must keep the authenticated transport alive" + ); + assert_eq!(ssh.state.sessions.load(Ordering::SeqCst), 0); + + killpg( + Pid::from_raw(i32::try_from(child_pid).expect("pid fits i32")), + Signal::SIGINT, + ) + .expect("interrupt foreground -N process group"); + timeout(TEST_TIMEOUT, child.wait()) + .await + .expect("foreground -N did not exit after Ctrl-C") + .expect("wait for foreground -N"); + ssh.shutdown().await; +} diff --git a/tests/download_test.rs b/tests/download_test.rs index b5a4cc23..827ffbb7 100644 --- a/tests/download_test.rs +++ b/tests/download_test.rs @@ -53,7 +53,7 @@ fn test_download_command_parsing() { fn test_download_command_with_cluster() { let args = vec![ "bssh", - "-C", + "--cluster", "staging", "download", "/var/log/app.log", diff --git a/tests/fail_fast_test.rs b/tests/fail_fast_test.rs index 71c38299..f06ae1b7 100644 --- a/tests/fail_fast_test.rs +++ b/tests/fail_fast_test.rs @@ -220,12 +220,7 @@ fn test_cli_fail_fast_flag_parsing() { use bssh::cli::Cli; use clap::Parser; - // Test short form -k - let args = ["bssh", "-H", "host1,host2", "-k", "echo test"]; - let cli = Cli::try_parse_from(args).expect("Should parse with -k flag"); - assert!(cli.fail_fast, "Short flag -k should set fail_fast=true"); - - // Test long form --fail-fast + // --fail-fast is intentionally long-only in SSH mode. let args = ["bssh", "-H", "host1,host2", "--fail-fast", "echo test"]; let cli = Cli::try_parse_from(args).expect("Should parse with --fail-fast flag"); assert!( @@ -297,7 +292,14 @@ fn test_fail_fast_flag_combinations() { assert!(cli.check_all_nodes); // fail-fast + verbose - let args = ["bssh", "-H", "host1,host2", "-k", "-v", "echo test"]; + let args = [ + "bssh", + "-H", + "host1,host2", + "--fail-fast", + "-v", + "echo test", + ]; let cli = Cli::try_parse_from(args).expect("Should parse with fail-fast and verbose"); assert!(cli.fail_fast); assert_eq!(cli.verbose, 1); @@ -307,7 +309,7 @@ fn test_fail_fast_flag_combinations() { "bssh", "-H", "host1,host2", - "-k", + "--fail-fast", "--timeout", "60", "echo test", @@ -317,20 +319,18 @@ fn test_fail_fast_flag_combinations() { assert_eq!(cli.timeout, Some(60)); } -/// Test that -k doesn't conflict with existing short options +/// OpenSSH-compatible -k disables GSSAPI credential delegation. #[test] #[serial] -fn test_k_flag_no_conflict() { +fn test_k_flag_uses_openssh_semantics() { use bssh::cli::Cli; use clap::Parser; - // Verify -k is distinct from other flags - // The -k flag is now assigned to fail-fast (pdsh compatibility) - let args = ["bssh", "-H", "host1", "-k", "uptime"]; let result = Cli::try_parse_from(args); assert!(result.is_ok(), "-k should be a valid flag"); let cli = result.unwrap(); - assert!(cli.fail_fast, "-k should set fail_fast=true"); + assert!(cli.disable_gssapi_credential_forwarding); + assert!(!cli.fail_fast, "-k must not enable --fail-fast"); } diff --git a/tests/no_prefix_test.rs b/tests/no_prefix_test.rs index 44c091c2..b452feba 100644 --- a/tests/no_prefix_test.rs +++ b/tests/no_prefix_test.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Tests for --no-prefix / -N option functionality +//! Tests for the long-only --no-prefix option functionality. use bssh::cli::Cli; use bssh::executor::OutputMode; @@ -34,14 +34,15 @@ fn test_no_prefix_long_option() { ); } -/// Test CLI parsing with -N short option +/// OpenSSH-compatible -N must not enable the legacy no-prefix behavior. #[test] -fn test_no_prefix_short_option() { +fn test_openssh_no_remote_command_does_not_enable_no_prefix() { let args = vec!["bssh", "-H", "host1,host2", "-N", "uptime"]; let cli = Cli::parse_from(args); - assert!(cli.no_prefix, "-N should set no_prefix to true"); + assert!(cli.no_remote_command, "-N should disable remote sessions"); + assert!(!cli.no_prefix, "-N must not enable --no-prefix"); } /// Test CLI parsing without no_prefix option (default should be false) @@ -86,13 +87,13 @@ fn test_no_prefix_with_output_dir() { "host1,host2", "--output-dir", "/tmp/output", - "-N", + "--no-prefix", "uptime", ]; let cli = Cli::parse_from(args); - assert!(cli.no_prefix, "-N should be set"); + assert!(cli.no_prefix, "--no-prefix should be set"); assert_eq!(cli.output_dir, Some(PathBuf::from("/tmp/output"))); // OutputMode should respect both flags @@ -105,7 +106,7 @@ fn test_no_prefix_with_output_dir() { /// Test --no-prefix with cluster option #[test] fn test_no_prefix_with_cluster() { - let args = vec!["bssh", "-C", "production", "--no-prefix", "df -h"]; + let args = vec!["bssh", "--cluster", "production", "--no-prefix", "df -h"]; let cli = Cli::parse_from(args); @@ -113,19 +114,23 @@ fn test_no_prefix_with_cluster() { assert_eq!(cli.cluster, Some("production".to_string())); } -/// Test -N does not conflict with other short options +/// Test --no-prefix with the long-only authentication-agent option. #[test] fn test_no_prefix_with_other_options() { let args = vec![ - "bssh", "-H", "host1", "-N", "-A", // use-agent + "bssh", + "-H", + "host1", + "--no-prefix", + "--use-agent", "-v", // verbose "uptime", ]; let cli = Cli::parse_from(args); - assert!(cli.no_prefix, "-N should be set"); - assert!(cli.use_agent, "-A should be set"); + assert!(cli.no_prefix, "--no-prefix should be set"); + assert!(cli.use_agent, "--use-agent should be set"); assert_eq!(cli.verbose, 1, "-v should increase verbosity"); } diff --git a/tests/openssh-regress/baseline.json b/tests/openssh-regress/baseline.json index a575e0cc..7f3eca08 100644 --- a/tests/openssh-regress/baseline.json +++ b/tests/openssh-regress/baseline.json @@ -4,8 +4,8 @@ "macos": 65 }, "minimum_pass": { - "linux": 23, - "macos": 23 + "linux": 60, + "macos": 60 }, "openssh_tag": "V_10_3_P1", "schema_version": 1 diff --git a/tests/openssh-regress/results.json b/tests/openssh-regress/results.json index c8a26a64..dc4a998b 100644 --- a/tests/openssh-regress/results.json +++ b/tests/openssh-regress/results.json @@ -1,572 +1,571 @@ { - "generated_at": "2026-08-26T00:00:00+00:00", - "note": "Initial unmodified bssh measurement recorded by epic #275; durations and environmental reference diagnostics were not retained. The historical pubkey-priority row is preserved even though the exact V_10_3_P1 tree does not contain that script; see docs/openssh-regress.md.", + "generated_at": "2026-08-31T22:47:33.974717+00:00", "openssh_tag": "V_10_3_P1", - "platform": "macos", + "platform": "linux", "results": [ { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 365, + "first_failure_line": null, "reference_first_failure_line": null, "test": "addrmatch", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 14729, + "first_failure_line": null, "reference_first_failure_line": null, "test": "agent", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 215, + "first_failure_line": "SKIPPED: need SUDO to switch to uid nobody", "reference_first_failure_line": null, "test": "agent-getpeereid", - "verdict": "fail" + "verdict": "skip" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 214, + "first_failure_line": null, "reference_first_failure_line": null, "test": "agent-ptrace", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", - "reference_first_failure_line": "not retained in the historical measurement", + "duration_ms": 35753, + "first_failure_line": "WARNING: Unsafe (group or world writable) directory permissions found:", + "reference_first_failure_line": null, "test": "agent-restrict", - "verdict": "environmental" + "verdict": "fail" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 10233, + "first_failure_line": null, "reference_first_failure_line": null, "test": "agent-subprocess", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 20279, + "first_failure_line": null, "reference_first_failure_line": null, "test": "agent-timeout", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 6569, + "first_failure_line": null, "reference_first_failure_line": null, "test": "banner", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 316, + "first_failure_line": null, "reference_first_failure_line": null, "test": "broken-pipe", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 1076, + "first_failure_line": null, "reference_first_failure_line": null, "test": "brokenkeys", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 7893, + "first_failure_line": null, "reference_first_failure_line": null, "test": "cert-file", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 866, + "first_failure_line": null, "reference_first_failure_line": null, "test": "cfginclude", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 12537, + "first_failure_line": null, "reference_first_failure_line": null, "test": "cfgmatch", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 316, + "first_failure_line": null, "reference_first_failure_line": null, "test": "cfgparse", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", - "reference_first_failure_line": "not retained in the historical measurement", + "duration_ms": 85315, + "first_failure_line": "ssh returned unexpected error code 1", + "reference_first_failure_line": "FATAL: open mux failed", "test": "channel-timeout", "verdict": "environmental" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 1830, + "first_failure_line": null, "reference_first_failure_line": null, "test": "connect", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 1124, + "first_failure_line": null, "reference_first_failure_line": null, "test": "connect-bigconf", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 1829, + "first_failure_line": null, "reference_first_failure_line": null, "test": "connect-uri", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", - "reference_first_failure_line": "not retained in the historical measurement", + "duration_ms": 24058, + "first_failure_line": "Error: Failed to load SSH config from \"none\"", + "reference_first_failure_line": "FATAL: failed to start ssh 255", "test": "connection-timeout", "verdict": "environmental" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 265, + "first_failure_line": null, "reference_first_failure_line": null, "test": "dhgex", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 20130, + "first_failure_line": "FATAL: failed to start dynamic forwarding 255", "reference_first_failure_line": null, "test": "dynamic-forward", "verdict": "fail" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 7236, + "first_failure_line": null, "reference_first_failure_line": null, "test": "envpass", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 32897, + "first_failure_line": null, "reference_first_failure_line": null, "test": "exit-status", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 1227, + "first_failure_line": null, "reference_first_failure_line": null, "test": "exit-status-signal", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", - "reference_first_failure_line": "not retained in the historical measurement", + "duration_ms": 101306, + "first_failure_line": null, + "reference_first_failure_line": null, "test": "forward-control", - "verdict": "environmental" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", - "reference_first_failure_line": "not retained in the historical measurement", + "duration_ms": 23114, + "first_failure_line": "WARNING: Unsafe (group or world writable) directory permissions found:", + "reference_first_failure_line": "WARNING: Unsafe (group or world writable) directory permissions found:", "test": "forwarding", "verdict": "environmental" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 971, + "first_failure_line": null, "reference_first_failure_line": null, "test": "host-expand", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 215, + "first_failure_line": "SKIPPED: TEST_SSH_HOSTBASED_AUTH not set.", "reference_first_failure_line": null, "test": "hostbased", - "verdict": "fail" + "verdict": "skip" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 6160, + "first_failure_line": "WARNING: Unsafe (group or world writable) directory permissions found:", "reference_first_failure_line": null, "test": "hostkey-agent", "verdict": "fail" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 9984, + "first_failure_line": null, "reference_first_failure_line": null, "test": "hostkey-rotate", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 70446, + "first_failure_line": null, "reference_first_failure_line": null, "test": "integrity", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 216, + "first_failure_line": "SKIPPED: Password auth requires SUDO and kbdintpw file.", "reference_first_failure_line": null, "test": "kbdint", - "verdict": "fail" + "verdict": "skip" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 15863, + "first_failure_line": null, "reference_first_failure_line": null, "test": "kextype", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 13398, + "first_failure_line": null, "reference_first_failure_line": null, "test": "key-options", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 3334, + "first_failure_line": null, "reference_first_failure_line": null, "test": "keygen-change", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 1927, + "first_failure_line": null, "reference_first_failure_line": null, "test": "keygen-comment", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 2178, + "first_failure_line": null, "reference_first_failure_line": null, "test": "keygen-convert", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 415, + "first_failure_line": null, "reference_first_failure_line": null, "test": "keygen-knownhosts", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 266, + "first_failure_line": null, "reference_first_failure_line": null, "test": "keygen-moduli", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 265, + "first_failure_line": null, "reference_first_failure_line": null, "test": "keygen-sshfp", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 466, + "first_failure_line": null, "reference_first_failure_line": null, "test": "keyscan", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 14384, + "first_failure_line": null, "reference_first_failure_line": null, "test": "keytype", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 5673, + "first_failure_line": null, "reference_first_failure_line": null, "test": "knownhosts", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 6526, + "first_failure_line": null, "reference_first_failure_line": null, "test": "knownhosts-command", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 15417, + "first_failure_line": null, "reference_first_failure_line": null, "test": "limit-keytype", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 1021, + "first_failure_line": null, "reference_first_failure_line": null, "test": "localcommand", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 16067, + "first_failure_line": null, "reference_first_failure_line": null, "test": "login-timeout", - "verdict": "fail" + "verdict": "pass" + }, + { + "duration_ms": 6514, + "first_failure_line": null, + "reference_first_failure_line": null, + "test": "match-subsystem", + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", - "reference_first_failure_line": "not retained in the historical measurement", + "duration_ms": 45370, + "first_failure_line": "error: the argument '-E ' cannot be used multiple times", + "reference_first_failure_line": null, "test": "multiplex", - "verdict": "environmental" + "verdict": "fail" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 6432, + "first_failure_line": null, "reference_first_failure_line": null, "test": "multipubkey", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 266, + "first_failure_line": "SKIPPED: Password auth requires SUDO and password file.", "reference_first_failure_line": null, "test": "password", - "verdict": "fail" + "verdict": "skip" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 21677, + "first_failure_line": "WARNING: Unsafe (group or world writable) directory permissions found:", "reference_first_failure_line": null, "test": "percent", "verdict": "fail" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 4103, + "first_failure_line": null, "reference_first_failure_line": null, "test": "portnum", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 265, + "first_failure_line": null, "reference_first_failure_line": null, "test": "proto-mismatch", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 265, + "first_failure_line": null, "reference_first_failure_line": null, "test": "proto-version", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 2540, + "first_failure_line": null, "reference_first_failure_line": null, "test": "proxy-connect", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 817, + "first_failure_line": "WARNING: Unsafe (group or world writable) directory permissions found:", "reference_first_failure_line": null, "test": "proxyjump", "verdict": "fail" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", - "reference_first_failure_line": "not retained in the historical measurement", - "test": "pubkey-priority", - "verdict": "environmental" - }, - { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 120116, + "first_failure_line": "timed out", "reference_first_failure_line": null, "test": "rekey", "verdict": "fail" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 20715, + "first_failure_line": null, "reference_first_failure_line": null, "test": "scp", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 3631, + "first_failure_line": null, "reference_first_failure_line": null, "test": "scp-uri", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", - "reference_first_failure_line": "not retained in the historical measurement", + "duration_ms": 6744, + "first_failure_line": "WARNING: Unsafe (group or world writable) directory permissions found:", + "reference_first_failure_line": null, "test": "scp3", - "verdict": "environmental" + "verdict": "fail" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 314, + "first_failure_line": null, "reference_first_failure_line": null, "test": "servcfginclude", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 120017, + "first_failure_line": "timed out", "reference_first_failure_line": null, "test": "sftp", "verdict": "fail" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", - "reference_first_failure_line": null, + "duration_ms": 416, + "first_failure_line": "WARNING: Unsafe (group or world writable) directory permissions found:", + "reference_first_failure_line": "WARNING: Unsafe (group or world writable) directory permissions found:", "test": "sftp-badcmds", - "verdict": "fail" + "verdict": "environmental" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 717, + "first_failure_line": null, "reference_first_failure_line": null, "test": "sftp-batch", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 3832, + "first_failure_line": null, "reference_first_failure_line": null, "test": "sftp-cmds", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 315, + "first_failure_line": null, "reference_first_failure_line": null, "test": "sftp-glob", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 2927, + "first_failure_line": null, "reference_first_failure_line": null, "test": "sftp-perm", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 3237, + "first_failure_line": null, "reference_first_failure_line": null, "test": "sftp-resume", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 19766, + "first_failure_line": null, "reference_first_failure_line": null, "test": "sftp-uri", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 215, + "first_failure_line": "SKIPPED: $PATH or $HOME has whitespace, not supported in this test", "reference_first_failure_line": null, "test": "ssh-tty", - "verdict": "fail" + "verdict": "skip" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 466, + "first_failure_line": null, "reference_first_failure_line": null, "test": "sshcfgparse", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 266, + "first_failure_line": "SKIPPED: TEST_SSH_SSHFP_DOMAIN not set.", "reference_first_failure_line": null, "test": "sshfp-connect", - "verdict": "fail" + "verdict": "skip" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 95735, + "first_failure_line": null, "reference_first_failure_line": null, "test": "sshsig", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 3048, + "first_failure_line": null, "reference_first_failure_line": null, "test": "stderr-after-eof", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 28809, + "first_failure_line": null, "reference_first_failure_line": null, "test": "stderr-data", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 35794, + "first_failure_line": null, "reference_first_failure_line": null, "test": "transfer", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 30944, + "first_failure_line": null, "reference_first_failure_line": null, "test": "try-ciphers", - "verdict": "fail" + "verdict": "pass" }, { - "duration_ms": null, - "first_failure_line": "error: unexpected argument '-E' found", + "duration_ms": 3253, + "first_failure_line": null, "reference_first_failure_line": null, "test": "yes-head", - "verdict": "fail" + "verdict": "pass" } ], "schema_version": 1, "score": { - "eligible": 71, - "passed": 0, + "eligible": 69, + "passed": 60, "verdicts": { - "environmental": 8, - "fail": 71, - "pass": 0, - "skip": 0 + "environmental": 4, + "fail": 9, + "pass": 60, + "skip": 6 } }, "selection": { @@ -621,11 +620,6 @@ "reason": "sshd-side AuthorizedKeysCommand test; outside the client candidate set.", "test": "keys-command" }, - { - "disposition": "exclude", - "reason": "sshd-side subsystem matching test; outside the client candidate set.", - "test": "match-subsystem" - }, { "disposition": "exclude", "reason": "sshd-side penalty configuration test; outside the client candidate set.", @@ -666,11 +660,6 @@ "reason": "Upstream helper script; it is not a standalone regression test.", "test": "scp-ssh-wrapper" }, - { - "disposition": "exclude", - "reason": "sshd ChrootDirectory and in-process SFTP behavior is server-side.", - "test": "sftp-chroot" - }, { "disposition": "exclude", "reason": "Permanent exclusion: ssh.com interoperability is outside the compatibility target.", @@ -753,6 +742,11 @@ "reason": "Permanent candidate skip: sshd process reconfiguration is server-side lifecycle behavior.", "test": "reconfigure" }, + { + "disposition": "skip", + "reason": "Permanent candidate skip: sshd ChrootDirectory and in-process SFTP behavior are server-side.", + "test": "sftp-chroot" + }, { "disposition": "skip", "reason": "Permanent candidate skip: PKCS#11 and FIDO/security-key middleware are outside the compatibility target.", diff --git a/tests/pdsh_compat_test.rs b/tests/pdsh_compat_test.rs index 6ed807d9..3f45ef6d 100644 --- a/tests/pdsh_compat_test.rs +++ b/tests/pdsh_compat_test.rs @@ -272,6 +272,13 @@ fn test_pdsh_to_bssh_flags_conversion() { assert!(bssh_cli.fail_fast); assert!(bssh_cli.any_failure); assert!(bssh_cli.pdsh_compat); // pdsh_compat should be set + assert!(!bssh_cli.no_remote_command); + assert!(!bssh_cli.fork_after_authentication); + assert!(!bssh_cli.compression); + assert!(!bssh_cli.forward_agent); + assert!(!bssh_cli.disable_gssapi_credential_forwarding); + assert!(bssh_cli.control_path.is_none()); + assert!(bssh_cli.bind_address.is_none()); } #[test] diff --git a/tests/upload_test.rs b/tests/upload_test.rs index 7551debe..5c0cb484 100644 --- a/tests/upload_test.rs +++ b/tests/upload_test.rs @@ -53,7 +53,7 @@ fn test_upload_command_parsing() { fn test_upload_command_with_cluster() { let args = vec![ "bssh", - "-C", + "--cluster", "production", "upload", "./local.conf", From 17a834fc9016664bdb1f8c03ef11edaff3442e62 Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Tue, 1 Sep 2026 08:57:44 +0900 Subject: [PATCH 2/3] fix(test): stabilize OpenSSH compatibility scoring Keep migration notices on interactive stderr so redirected protocol streams remain byte-transparent. Add audited per-test minimum timeouts for the two upstream matrices whose valid runtimes exceed the base limit under local or shared CI load. --- docs/openssh-regress.md | 2 +- docs/openssh-short-flags-migration.md | 5 +++ src/main.rs | 5 ++- tests/openssh-regress/run.py | 64 +++++++++++++++++++++++---- tests/openssh-regress/selection.tsv | 6 +-- tests/openssh-regress/test_run.py | 23 ++++++++-- 6 files changed, 89 insertions(+), 16 deletions(-) diff --git a/docs/openssh-regress.md b/docs/openssh-regress.md index 3986323e..d3b40330 100644 --- a/docs/openssh-regress.md +++ b/docs/openssh-regress.md @@ -14,7 +14,7 @@ make openssh-regress-list make openssh-regress ``` -The first full invocation builds `target/debug/bssh`, clones `V_10_3_P1` under `target/openssh-regress/`, configures and builds the reference binaries, and runs every selected test. Later invocations reuse both build trees. Use `--bssh` or `--openssh-tree` with `tests/openssh-regress/run.py` to supply prebuilt trees, `--timeout` to change the per-test limit, and repeated `--test NAME` arguments for focused diagnosis. +The first full invocation builds `target/debug/bssh`, clones `V_10_3_P1` under `target/openssh-regress/`, configures and builds the reference binaries, and runs every selected test. Later invocations reuse both build trees. Use `--bssh` or `--openssh-tree` with `tests/openssh-regress/run.py` to supply prebuilt trees, `--timeout` to change the base per-test limit, and repeated `--test NAME` arguments for focused diagnosis. The manifest may declare a higher minimum for an upstream test whose normal runtime exceeds the base limit. `forward-control` uses 180 seconds after a measured run completed in about 158 seconds, and `sshsig` uses the same minimum after completing in 96 seconds locally but exceeding 120 seconds on a shared Linux CI runner. `make openssh-regress-update` writes the full per-test table to `tests/openssh-regress/results.json` and updates the current platform's minimum passing and eligible-result floors in `baseline.json`. Review both diffs before committing them. Never update the baseline merely to make an unexplained regression green. diff --git a/docs/openssh-short-flags-migration.md b/docs/openssh-short-flags-migration.md index bdc57da2..b6b10afc 100644 --- a/docs/openssh-short-flags-migration.md +++ b/docs/openssh-short-flags-migration.md @@ -13,6 +13,11 @@ requirement: this documentation and the 3.0 implementation do not, by themselves, prove that such a warning was shipped. Release notes must not mark that requirement complete without evidence from a published 2.x release. +The transition notices in this source tree are written only when standard +error is an interactive terminal. Redirected standard error and `-E` logs stay +byte-transparent so automation and SSH protocol tests are not corrupted by a +human-facing migration message. + ## Script rewrites | Former bssh use | Rewrite for 3.0 | What the short flag means in 3.0 | diff --git a/src/main.rs b/src/main.rs index e07186c3..5e68cb00 100644 --- a/src/main.rs +++ b/src/main.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::io::IsTerminal; use std::process::ExitCode; use anyhow::Result; @@ -332,7 +333,9 @@ async fn run_bssh_mode(args: &[String]) -> Result<()> { } bssh::utils::diagnostics::set_quiet_warnings(cli.quiet); let background_worker = BackgroundWorker::from_environment()?; - if background_worker.is_none() { + // Migration notices are human-facing. Keep redirected stderr and `-E` + // logs byte-transparent for OpenSSH-compatible scripts and protocols. + if background_worker.is_none() && cli.log_file.is_none() && std::io::stderr().is_terminal() { for warning in cli.short_flag_migration_warnings(&effective_args) { bssh::warningln!("{warning}"); } diff --git a/tests/openssh-regress/run.py b/tests/openssh-regress/run.py index 7d759774..602581be 100644 --- a/tests/openssh-regress/run.py +++ b/tests/openssh-regress/run.py @@ -46,6 +46,7 @@ class Selection: test: str disposition: str reason: str + timeout_seconds: int | None = None @dataclass(frozen=True) @@ -83,13 +84,28 @@ def read_selection(path: Path) -> list[Selection]: reader = csv.DictReader( (line for line in stream if not line.startswith("#")), delimiter="\t" ) - if reader.fieldnames != ["test", "disposition", "reason"]: - raise ValueError("selection.tsv must have test, disposition, reason columns") + if reader.fieldnames != [ + "test", + "disposition", + "reason", + "timeout_seconds", + ]: + raise ValueError( + "selection.tsv must have test, disposition, reason, timeout_seconds columns" + ) for raw in reader: + timeout_text = raw["timeout_seconds"] or "" + try: + timeout_seconds = int(timeout_text) if timeout_text else None + except ValueError as error: + raise ValueError( + f"invalid timeout for {raw['test']}: {timeout_text!r}" + ) from error row = Selection( test=raw["test"], disposition=raw["disposition"], reason=raw["reason"] or "", + timeout_seconds=timeout_seconds, ) if row.disposition not in {"run", "skip", "exclude"}: raise ValueError(f"invalid disposition for {row.test}: {row.disposition}") @@ -97,6 +113,11 @@ def read_selection(path: Path) -> list[Selection]: raise ValueError(f"invalid test name: {row.test}") if row.disposition != "run" and not row.reason: raise ValueError(f"{row.test} needs an exclusion reason") + if row.timeout_seconds is not None: + if row.timeout_seconds < 1: + raise ValueError(f"{row.test} timeout must be positive") + if row.disposition != "run": + raise ValueError(f"{row.test} timeout is valid only for runnable tests") rows.append(row) names = [row.test for row in rows] if len(names) != len(set(names)): @@ -516,7 +537,12 @@ def parse_args() -> argparse.Namespace: parser.add_argument("--selection", type=Path, default=DEFAULT_SELECTION) parser.add_argument("--baseline", type=Path, default=DEFAULT_BASELINE) parser.add_argument("--results", type=Path, default=DEFAULT_RESULTS) - parser.add_argument("--timeout", type=int, default=120, help="seconds per client run") + parser.add_argument( + "--timeout", + type=int, + default=120, + help="base seconds per client run; manifest entries may set a higher minimum", + ) parser.add_argument("--jobs", type=int, default=max(1, min(os.cpu_count() or 1, 4))) parser.add_argument("--test", action="append", help="run only the named selected test") parser.add_argument("--list", action="store_true", help="validate and list the manifest") @@ -544,7 +570,11 @@ def main() -> int: f"{len(declared_skips)} permanent skips, {len(excluded)} excluded" ) for row in selection: - print(f"{row.disposition:7} {row.test} {row.reason}") + timeout_note = ( + f"minimum timeout {row.timeout_seconds}s" if row.timeout_seconds else "" + ) + details = "; ".join(part for part in [row.reason, timeout_note] if part) + print(f"{row.disposition:7} {row.test} {details}") return 0 if args.timeout < 1: raise ValueError("--timeout must be positive") @@ -561,8 +591,12 @@ def main() -> int: validate_tree_inventory(tree, selection) results: list[TestResult] = [] for index, row in enumerate(runnable, start=1): - print(f"[{index}/{len(runnable)}] {row.test}", flush=True) - result = classify(tree, bssh, tree / "ssh", row.test, args.timeout, log_dir) + test_timeout = max(args.timeout, row.timeout_seconds or 0) + print( + f"[{index}/{len(runnable)}] {row.test} (timeout {test_timeout}s)", + flush=True, + ) + result = classify(tree, bssh, tree / "ssh", row.test, test_timeout, log_dir) results.append(result) print(f" {result.verdict} ({result.duration_ms} ms)", flush=True) verdicts = ["pass", "skip", "fail", "environmental"] @@ -577,8 +611,22 @@ def main() -> int: "platform": platform_key(), "selection": { "runnable": len(runnable), - "permanent_skips": [asdict(row) for row in declared_skips], - "excluded": [asdict(row) for row in excluded], + "permanent_skips": [ + { + "test": row.test, + "disposition": row.disposition, + "reason": row.reason, + } + for row in declared_skips + ], + "excluded": [ + { + "test": row.test, + "disposition": row.disposition, + "reason": row.reason, + } + for row in excluded + ], }, "score": {"passed": counts["pass"], "eligible": denominator, "verdicts": counts}, "results": [asdict(result) for result in sorted(results, key=lambda item: item.test)], diff --git a/tests/openssh-regress/selection.tsv b/tests/openssh-regress/selection.tsv index 0a6c710c..0743eca0 100644 --- a/tests/openssh-regress/selection.tsv +++ b/tests/openssh-regress/selection.tsv @@ -1,4 +1,4 @@ -test disposition reason +test disposition reason timeout_seconds addrmatch run agent-getpeereid run agent-pkcs11-cert skip Permanent candidate skip: PKCS#11 and FIDO/security-key middleware are outside the compatibility target. @@ -38,7 +38,7 @@ envpass run exit-status-signal run exit-status run forcecommand exclude sshd-side ForceCommand test; outside the client candidate set. -forward-control run +forward-control run 180 forwarding run gss-auth skip Permanent candidate skip: GSSAPI authentication is outside the compatibility target. host-expand run @@ -106,7 +106,7 @@ ssh-tty run ssh2putty exclude Upstream conversion helper; it is not a standalone regression test. sshcfgparse run sshfp-connect run -sshsig run +sshsig run 180 stderr-after-eof run stderr-data run test-exec exclude Upstream test driver; it is not a standalone regression test. diff --git a/tests/openssh-regress/test_run.py b/tests/openssh-regress/test_run.py index 274db07d..54795347 100755 --- a/tests/openssh-regress/test_run.py +++ b/tests/openssh-regress/test_run.py @@ -30,6 +30,12 @@ def test_committed_manifest_is_valid(self) -> None: self.assertEqual( next(row.disposition for row in selection if row.test == "forwarding"), "run" ) + timeout_overrides = { + row.test: row.timeout_seconds + for row in selection + if row.timeout_seconds is not None + } + self.assertEqual(timeout_overrides, {"forward-control": 180, "sshsig": 180}) self.assertNotIn("pubkey-priority", {row.test for row in selection}) def test_pin_includes_an_immutable_commit(self) -> None: @@ -49,14 +55,25 @@ def test_pin_includes_an_immutable_commit(self) -> None: def test_manifest_rejects_duplicate_names(self) -> None: with tempfile.TemporaryDirectory() as directory: path = Path(directory) / "selection.tsv" - rows = ["test\tdisposition\treason"] - rows.extend(f"test-{index}\trun\t" for index in range(88)) - rows.extend(["duplicate\trun\t", "duplicate\texclude\treason"]) + rows = ["test\tdisposition\treason\ttimeout_seconds"] + rows.extend(f"test-{index}\trun\t\t" for index in range(88)) + rows.extend(["duplicate\trun\t\t", "duplicate\texclude\treason\t"]) path.write_text("\n".join(rows) + "\n", encoding="utf-8") with self.assertRaisesRegex(ValueError, "duplicate"): openssh_regress.read_selection(path) + def test_manifest_rejects_invalid_timeout(self) -> None: + with tempfile.TemporaryDirectory() as directory: + path = Path(directory) / "selection.tsv" + rows = ["test\tdisposition\treason\ttimeout_seconds"] + rows.extend(f"test-{index}\trun\t\t" for index in range(89)) + rows.append("invalid\trun\t\tzero") + path.write_text("\n".join(rows) + "\n", encoding="utf-8") + + with self.assertRaisesRegex(ValueError, "invalid timeout"): + openssh_regress.read_selection(path) + def test_tree_inventory_reports_drift(self) -> None: selection = [ openssh_regress.Selection("present", "run", ""), From 33d8b78b4ae51196a5bb31c06d181638b829c895 Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Tue, 1 Sep 2026 10:04:12 +0900 Subject: [PATCH 3/3] fix(ssh): stabilize OpenSSH regression edge cases Accept repeated diagnostic files and proxy control sessions, preserve the OpenSSH /dev/null config convention, and decouple transfer fixtures from debug binary size. --- docs/openssh-regress.md | 2 ++ docs/openssh-short-flags-migration.md | 6 ++++++ src/app/dispatcher.rs | 12 +++++++++-- src/cli/bssh.rs | 7 ++++++- src/ssh/ssh_config/include/validation.rs | 26 +++++++++++++++++++++++- tests/log_file_test.rs | 14 +++++++++++++ tests/openssh-regress/run.py | 24 +++++++++++++++++++++- tests/openssh-regress/test_run.py | 18 +++++++++++++--- 8 files changed, 101 insertions(+), 8 deletions(-) diff --git a/docs/openssh-regress.md b/docs/openssh-regress.md index d3b40330..3467b9c4 100644 --- a/docs/openssh-regress.md +++ b/docs/openssh-regress.md @@ -16,6 +16,8 @@ make openssh-regress The first full invocation builds `target/debug/bssh`, clones `V_10_3_P1` under `target/openssh-regress/`, configures and builds the reference binaries, and runs every selected test. Later invocations reuse both build trees. Use `--bssh` or `--openssh-tree` with `tests/openssh-regress/run.py` to supply prebuilt trees, `--timeout` to change the base per-test limit, and repeated `--test NAME` arguments for focused diagnosis. The manifest may declare a higher minimum for an upstream test whose normal runtime exceeds the base limit. `forward-control` uses 180 seconds after a measured run completed in about 158 seconds, and `sshsig` uses the same minimum after completing in 96 seconds locally but exceeding 120 seconds on a shared Linux CI runner. +Each client runs through a padded 1 MiB shell wrapper. OpenSSH's upstream `test-exec.sh` copies its SSH executable into the generic transfer fixture; using the Rust debug binary directly would make that fixture hundreds of MiB on some platforms and turn compatibility checks into debug-symbol transfer benchmarks. The wrapper executes the exact candidate or reference client while keeping fixture size stable across platforms. + `make openssh-regress-update` writes the full per-test table to `tests/openssh-regress/results.json` and updates the current platform's minimum passing and eligible-result floors in `baseline.json`. Review both diffs before committing them. Never update the baseline merely to make an unexplained regression green. ## Interpret results diff --git a/docs/openssh-short-flags-migration.md b/docs/openssh-short-flags-migration.md index b6b10afc..47304476 100644 --- a/docs/openssh-short-flags-migration.md +++ b/docs/openssh-short-flags-migration.md @@ -60,8 +60,14 @@ Use `-S` to select a connection-sharing socket: ```bash bssh -M -S ~/.ssh/bssh-%C user@host bssh -S ~/.ssh/bssh-%C user@host uptime +bssh -O proxy -S ~/.ssh/bssh-%C user@host uptime ``` +`-O proxy` requires an existing bssh control master and opens the requested +session on that master's authenticated transport. bssh control sockets are not +wire-compatible with OpenSSH control sockets; each client must use a master +created by the same program. + Use the long bssh options when selecting clusters or changing multi-node output behavior: diff --git a/src/app/dispatcher.rs b/src/app/dispatcher.rs index 6412bf4f..3df9ce10 100644 --- a/src/app/dispatcher.rs +++ b/src/app/dispatcher.rs @@ -245,7 +245,11 @@ async fn try_existing_control_master( control: &ResolvedControlInvocation, background_worker: Option<&BackgroundWorker>, ) -> Result> { - if let Some(command) = cli.control_command.as_deref() { + // OpenSSH's `-O proxy` exposes a raw SSH connection through its mux + // socket. bssh's private control protocol provides the equivalent user + // behavior by opening the requested session on the existing transport. + let proxy_session = cli.control_command.as_deref() == Some("proxy"); + if let Some(command) = cli.control_command.as_deref().filter(|_| !proxy_session) { let command = command.parse::()?; let forwards = if matches!(command, ControlCommand::Forward | ControlCommand::Cancel) { control.forwarding_directives.clone() @@ -263,7 +267,7 @@ async fn try_existing_control_master( } return Ok(Some(EXIT_SUCCESS)); } - if !control.policy.master.tries_existing() { + if !proxy_session && !control.policy.master.tries_existing() { return Ok(None); } if control.fork_after_authentication { @@ -292,6 +296,10 @@ async fn try_existing_control_master( ) .await? { + AttachOutcome::NoMaster if proxy_session => anyhow::bail!( + "-O proxy requires a running control master at '{}'", + control.path.display() + ), AttachOutcome::NoMaster => Ok(None), AttachOutcome::ExitStatus(status) => Ok(Some(i32::try_from(status).unwrap_or(255))), } diff --git a/src/cli/bssh.rs b/src/cli/bssh.rs index b0915567..1c8d1bab 100644 --- a/src/cli/bssh.rs +++ b/src/cli/bssh.rs @@ -178,6 +178,7 @@ pub struct Cli { #[arg( short = 'E', value_name = "log_file", + overrides_with = "log_file", help = "Append bssh diagnostics and debug logs to the specified file (SSH-compatible)" )] pub log_file: Option, @@ -308,7 +309,7 @@ pub struct Cli { short = 'O', long = "control-command", value_name = "command", - value_parser = ["check", "forward", "cancel", "exit", "stop"], + value_parser = ["check", "forward", "cancel", "exit", "stop", "proxy"], conflicts_with = "stdio_forward", help = "Send a control command to an existing connection master" )] @@ -1253,6 +1254,10 @@ mod tests { assert_eq!(repeated.control_command.as_deref(), Some("check")); assert_eq!(repeated.ssh_config_overrides()[0], "ControlMaster=ask"); + let proxy = + Cli::try_parse_from(["bssh", "-Oproxy", "--control-path=/tmp/c", "target"]).unwrap(); + assert_eq!(proxy.control_command.as_deref(), Some("proxy")); + let repeated_more = Cli::try_parse_from(["bssh", "-MMM", "target"]).unwrap(); assert_eq!(repeated_more.control_master, 3); assert_eq!(repeated_more.ssh_config_overrides()[0], "ControlMaster=ask"); diff --git a/src/ssh/ssh_config/include/validation.rs b/src/ssh/ssh_config/include/validation.rs index a46c58eb..2246049e 100644 --- a/src/ssh/ssh_config/include/validation.rs +++ b/src/ssh/ssh_config/include/validation.rs @@ -73,6 +73,18 @@ fn validate_opened_metadata(path: &Path, metadata: &std::fs::Metadata) -> Result Ok(()) } +#[cfg(unix)] +fn is_openssh_null_config(path: &Path, metadata: &std::fs::Metadata) -> bool { + use std::os::unix::fs::FileTypeExt as _; + + path == Path::new("/dev/null") && metadata.file_type().is_char_device() +} + +#[cfg(not(unix))] +fn is_openssh_null_config(_path: &Path, _metadata: &std::fs::Metadata) -> bool { + false +} + /// Open, validate with `fstat`, and read from the same handle. pub(crate) async fn read_config_file( path: &Path, @@ -110,7 +122,7 @@ where })?; if check_permissions { validate_opened_metadata(path, &metadata)?; - } else if !metadata.is_file() { + } else if !metadata.is_file() && !is_openssh_null_config(path, &metadata) { anyhow::bail!( "SSH config path is not a regular file: {}", escape_path(path) @@ -187,4 +199,16 @@ mod tests { .unwrap(); assert_eq!(content, "User safe\n"); } + + #[cfg(unix)] + #[tokio::test] + async fn exact_null_device_is_an_empty_root_config_only() { + let path = Path::new("/dev/null"); + + assert_eq!( + read_config_file(path, false, false).await.unwrap(), + Some(String::new()) + ); + assert!(read_config_file(path, true, false).await.is_err()); + } } diff --git a/tests/log_file_test.rs b/tests/log_file_test.rs index 3c86074a..8dd3163f 100644 --- a/tests/log_file_test.rs +++ b/tests/log_file_test.rs @@ -44,6 +44,20 @@ fn parses_separated_and_attached_log_file_options() { assert_eq!(attached.log_file, expected); } +#[test] +fn repeated_log_file_option_uses_the_last_value() { + let parsed = Cli::try_parse_from([ + "bssh", + "-E", + "/tmp/first.log", + "-E/tmp/last.log", + "example.com", + ]) + .expect("repeated -E should parse like OpenSSH"); + + assert_eq!(parsed.log_file, Some(PathBuf::from("/tmp/last.log"))); +} + #[test] fn human_diagnostics_are_routed_and_repeated_runs_append() { let directory = tempdir().expect("failed to create temporary directory"); diff --git a/tests/openssh-regress/run.py b/tests/openssh-regress/run.py index 602581be..9a3e175c 100644 --- a/tests/openssh-regress/run.py +++ b/tests/openssh-regress/run.py @@ -10,6 +10,7 @@ import platform import re import signal +import shlex import shutil import subprocess import sys @@ -29,6 +30,7 @@ PIN_FILE = HERE / "openssh-version" MAX_CAPTURE_BYTES = 1024 * 1024 MAX_LOG_BYTES = 16 * 1024 * 1024 +CLIENT_WRAPPER_BYTES = 1024 * 1024 TRUNCATION_NOTICE = b"\n...[output truncated by harness]...\n" FAILURE_PATTERN = re.compile( r"(?:^|\b)(?:FAIL(?:ED)?|FATAL|ERROR|Error|error|timed out|unexpected argument)(?:\b|:)", @@ -233,12 +235,32 @@ def make_value(makefile: Path, name: str, default: str = "") -> str: return match.group(1).strip().strip(chr(34)) if match else default +def prepare_client_wrapper(tree: Path, client: Path) -> Path: + """Create a stable-size executable wrapper for the SSH client under test. + + OpenSSH's test-exec.sh copies the SSH executable into its generic data + fixture. A Rust debug binary can contain hundreds of MiB of symbols, which + turns ordinary transfer tests into accidental stress tests. The padded + wrapper keeps that fixture deterministic while still execing the exact + requested client. + """ + wrapper = tree / "regress" / ".bssh-openssh-client" + wrapper.parent.mkdir(parents=True, exist_ok=True) + contents = f'#!/bin/sh\nexec {shlex.quote(str(client))} "$@"\n#'.encode() + if len(contents) < CLIENT_WRAPPER_BYTES: + contents += b"x" * (CLIENT_WRAPPER_BYTES - len(contents) - 1) + b"\n" + wrapper.write_bytes(contents) + wrapper.chmod(0o700) + return wrapper + + def reference_environment(tree: Path, client: Path) -> dict[str, str]: makefile = tree / "Makefile" env = os.environ.copy() + client_wrapper = prepare_client_wrapper(tree, client) helpers = { "TEST_SSH_SCP": "scp", - "TEST_SSH_SSH": str(client), + "TEST_SSH_SSH": str(client_wrapper), "TEST_SSH_SSHD": "sshd", "TEST_SSH_SSHD_SESSION": "sshd-session", "TEST_SSH_SSHD_AUTH": "sshd-auth", diff --git a/tests/openssh-regress/test_run.py b/tests/openssh-regress/test_run.py index 54795347..230f458d 100755 --- a/tests/openssh-regress/test_run.py +++ b/tests/openssh-regress/test_run.py @@ -4,6 +4,7 @@ from __future__ import annotations import importlib.util +import os import sys import tempfile import unittest @@ -35,7 +36,10 @@ def test_committed_manifest_is_valid(self) -> None: for row in selection if row.timeout_seconds is not None } - self.assertEqual(timeout_overrides, {"forward-control": 180, "sshsig": 180}) + self.assertEqual( + timeout_overrides, + {"forward-control": 180, "sshsig": 180}, + ) self.assertNotIn("pubkey-priority", {row.test for row in selection}) def test_pin_includes_an_immutable_commit(self) -> None: @@ -96,10 +100,19 @@ def test_reference_environment_names_all_t_exec_helpers(self) -> None: (tree / "Makefile").write_text( "TEST_SHELL = /bin/sh\nTEST_MALLOC_OPTIONS = CFGJRSUX\n", encoding="utf-8" ) - client = tree / "candidate-bssh" + client = tree / "candidate bssh" env = openssh_regress.reference_environment(tree, client) + wrapper = Path(env["TEST_SSH_SSH"]) + self.assertEqual(wrapper.stat().st_size, openssh_regress.CLIENT_WRAPPER_BYTES) + self.assertTrue(os.access(wrapper, os.X_OK)) + self.assertTrue( + wrapper.read_text(encoding="utf-8").startswith( + f'#!/bin/sh\nexec \'{client}\' "$@"\n#' + ) + ) + expected = { "TEST_SSH_SCP", "TEST_SSH_SSH", @@ -117,7 +130,6 @@ def test_reference_environment_names_all_t_exec_helpers(self) -> None: "TEST_SSH_SFTPSERVER", } self.assertTrue(expected.issubset(env)) - self.assertEqual(env["TEST_SSH_SSH"], str(client)) self.assertEqual(env["MALLOC_OPTIONS"], "CFGJRSUX") def test_process_timeout_terminates_the_process_group(self) -> None: