fix: Apply SSH config ProxyJump directive when -J option not specified (issue #117) #119
+445
−14
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…d (issue #117) The get_proxy_jump() function existed but was never called during connection. Now bssh automatically uses ProxyJump from ~/.ssh/config when connecting to hosts that have it configured. Priority order for jump host resolution: 1. CLI -J option (highest priority) 2. SSH config ProxyJump directive 3. None (direct connection) Changes: - Add ssh_config field to ExecutionConfig and ParallelExecutor - Update execute_on_node_with_jump_hosts to resolve effective jump hosts - Pass ssh_config through dispatcher to execution commands - Add determine_effective_jump_hosts helper function - Add comprehensive unit tests for jump host resolution - Update documentation in docs/architecture/ssh-jump-hosts.md The implementation resolves jump hosts per-node, allowing different nodes to use different jump hosts based on their SSH config patterns.
inureyes
added
type:bug
This commit extends the SSH config ProxyJump integration to file transfer operations (upload/download), addressing the consistency issue where command execution respected SSH config ProxyJump but file transfers did not. Changes: - Add ssh_config parameter to upload_to_node, download_from_node, and download_dir_from_node functions in connection_manager.rs - Apply the same jump host resolution logic (CLI precedence over SSH config) for all file transfer operations - Update execution_strategy.rs task functions to accept ssh_config - Update parallel.rs to pass ssh_config to file transfer tasks This ensures consistent behavior where users configuring ProxyJump in their ~/.ssh/config will have it applied to both command execution and file transfers automatically.
Completes the SSH config ProxyJump integration for the upload and download CLI commands by adding ssh_config to FileTransferParams. Changes: - Add ssh_config field to FileTransferParams struct in upload.rs - Update dispatcher.rs to pass ctx.ssh_config in FileTransferParams for both upload and download commands - Update upload_file() to call executor.with_ssh_config() - Update download_file() to call executor.with_ssh_config() - Update download_dir_from_node() call to pass params.ssh_config This ensures that `bssh upload` and `bssh download` commands now properly respect ProxyJump directives from ~/.ssh/config.
6 tasks
Adds 11 new unit tests for the SSH config ProxyJump resolution logic in connection_manager.rs: Test Coverage: - CLI jump hosts precedence over SSH config - SSH config ProxyJump fallback when CLI not specified - No jump hosts when neither CLI nor SSH config specifies one - No jump hosts when SSH config is not provided - Multi-hop ProxyJump chains (comma-separated) - ProxyJump with port specification (host:port) - ProxyJump with user and port (user@host:port) - Wildcard pattern matching (*.internal.example.com) - Unmatched hosts returning no ProxyJump - ProxyJump "none" value for disabling jump - Complex multi-hop chains with mixed formats Also adds a `resolve_effective_jump_hosts` helper function that encapsulates the priority logic for easier testing.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #117
The
get_proxy_jump()function existed in the SSH config parser but was never called during connection establishment. This PR implements automatic ProxyJump directive resolution from~/.ssh/configwhen the CLI-Joption is not specified.Changes
Core Implementation
src/executor/connection_manager.rs: Updatedexecute_on_node_with_jump_hosts()to resolve effective jump hosts by checking CLI option first, then falling back to SSH config ProxyJumpsrc/executor/parallel.rs: Addedssh_configfield toParallelExecutorand updated all constructorssrc/commands/exec.rs: Addedssh_configfield toExecuteCommandParamsand passed it to the executorsrc/app/dispatcher.rs: Updated to pass SSH config through to execution commandssrc/app/initialization.rs: Addeddetermine_effective_jump_hosts()helper function with comprehensive unit testsDocumentation
docs/architecture/ssh-jump-hosts.md: Added new section documenting SSH config ProxyJump integration with examples and usage patternsPriority Order
Jump host resolution now follows this priority:
-Joption (highest priority) - Explicitly specified jump hostsProxyJumpdirective - Per-host configuration from~/.ssh/configExample Usage
Given this SSH config:
Now works automatically:
Testing
src/app/initialization.rs::tests:test_determine_effective_jump_hosts_cli_takes_precedencetest_determine_effective_jump_hosts_falls_back_to_ssh_configtest_determine_effective_jump_hosts_no_jump_hosttest_determine_effective_jump_hosts_wildcard_patternBenefits
-Jfor frequently accessed internal hosts