You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
fix: collect --password once up-front and honor -S consistently across subcommands (#201)
* fix: collect --password once up-front and honor -S across subcommands
Fixes two defects in credential collection that surfaced together in `bssh -C orin -l lablup --password -S ping` (issue #200).
Defect 1 — `--password` is collected lazily per-connection. The previous implementation prompted via `rpassword::prompt_password()` inside each per-node authentication task (`ssh/auth.rs::password_auth()`). The executor fans out N parallel connection attempts, so multiple tasks raced for stdin: the prompt could be missed, repeated per node, or interleaved with the indicatif progress UI rendering. The `-S` (sudo-password) path already did this correctly — collect once in the dispatcher, wrap in `Arc<SudoPassword>`, share across tasks — and now `--password` follows the same pattern.
Defect 2 — `-S` was silently dropped by `ping`, `upload`, and `download`. The dispatcher only read `cli.sudo_password` in the `exec` and interactive branches. Users could pass `-S` to `ping`/`upload`/`download` without any error, but the flag had no effect.
Changes:
- Add `src/security/password.rs` mirroring the `SudoPassword` design: a `Password` wrapper backed by `secrecy::SecretString` (auto-zeroized on drop), `prompt_password()` with a non-host-specific prompt ("Enter SSH password (used for all hosts): "), `get_password_from_env()` reading `BSSH_PASSWORD`, and `get_password(warn_env)` combining both. Wrapped in `Arc<Password>` for sharing across per-node tasks without duplicating secret material.
- Hoist `--password` collection to `dispatch_command` in `src/app/dispatcher.rs`. Prompt runs once, before any `ParallelExecutor` is built and before any indicatif `MultiProgress` is initialized, so the terminal is in a clean state when reading the password. The resulting `Arc<Password>` is threaded through `ping_nodes`, `FileTransferParams` (upload/download), `ExecuteCommandParams`, `InteractiveCommand`, and the SSH-mode interactive path.
- Extend `AuthContext` in `src/ssh/auth.rs` with a `password: Option<Arc<Password>>` field and a `with_pre_collected_password()` builder. `password_auth()` now consumes the pre-collected value when available; the in-task `rpassword::prompt_password()` call remains only as the fallback for the OpenSSH-style "all key methods failed, prompt for password" path (which has no dispatcher-side collection because it is opportunistic).
- Thread `ssh_password: Option<Arc<Password>>` through the executor (`ParallelExecutor::with_ssh_password`), `ExecutionConfig`, `ConnectionConfig`, and the SFTP `*_with_jump_hosts` helpers. Every per-node task clones the `Arc` so all tasks observe the same secret without copying it.
- For `-S`: emit `tracing::warn!` in the dispatcher when the flag is passed to a subcommand where it has no effect (`ping`, `upload`, `download`, `list`, `cache-stats`). Chose a warning over a hard reject to match typical CLI UX — existing scripts that happen to pass `-S` to non-applicable subcommands keep working but the user gets visible feedback. `exec` and interactive paths continue to honor `-S` as before.
Tests:
- 9 unit tests in `src/security/password.rs` mirror the structure of `src/security/sudo.rs`: creation, empty rejection, debug redaction, clone independence, `Arc` sharing, env var handling (empty/valid/unset), and a dispatcher-collection-pattern test that verifies a single `get_password()` call yields an `Arc<Password>` observably shared across three simulated per-node tasks.
- 2 new tests in `src/ssh/auth.rs`: `test_auth_context_with_pre_collected_password` checks the builder and Arc sharing semantics; `test_password_auth_uses_pre_collected_password` verifies `password_auth()` returns immediately without prompting when the pre-collected value is set (critical: it never blocks on stdin in non-interactive test environments).
- All existing tests (1233 lib tests + integration suites) continue to pass. `cargo build --release`, `cargo clippy --all-targets --all-features -- -D warnings`, and `cargo fmt --check` all pass.
Closes#200
* fix: thread pre-collected password through jump-host and legacy command paths
Addresses pr-reviewer findings on PR #201:
- jump::chain::auth::determine_auth_method now consumes Arc<Password>
instead of prompting per-call (issue #200 C1)
- ssh::client::SshClient::connect_and_execute_with_host_check accepts
Arc<Password> so the download glob-resolution step reuses the
dispatcher's single prompt (issue #200 C2)
- commands::exec::execute_command_with_forwarding now builds
AuthMethod::with_password from the pre-collected secret instead of
erroring after the user has already entered a password (issue #200 M1)
* fix: route -S ignored warning to stderr
`tracing::warn!` lands on stdout under the default tracing subscriber,
so the `-S has no effect` warning was being mixed into stdout —
breaking `bssh ... ping | grep ...` style pipelines and contradicting
the PR description's contract.
Switch to `eprintln!` to match the existing pattern used for the
`BSSH_PASSWORD` env warning in `src/security/password.rs:168-171`.
* docs: document BSSH_PASSWORD env var and single-prompt behavior
Add CHANGELOG entry for #201 (collect --password once up-front, -S
warning to stderr). Document the new BSSH_PASSWORD environment variable
in the README environment-variables section and in the bssh.1 man page.
Update the --password flag description in the man page to describe the
single-prompt / Arc-shared behavior introduced by #201.
Copy file name to clipboardExpand all lines: CHANGELOG.md
+6Lines changed: 6 additions & 0 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
7
7
8
8
## [Unreleased]
9
9
10
+
## [2.1.5] - Unreleased
11
+
12
+
### Fixed
13
+
-**`--password` prompt is now collected once up-front and shared across all parallel connection tasks** (#201, closes #200). Previously each per-node SSH task prompted for the password independently, racing each other for stdin and interleaving with the progress UI. The dispatcher now collects the password before any executor or `indicatif` UI is initialized and threads an `Arc<Password>` to every per-node auth task — matching the existing `-S` (`SudoPassword`) pattern. The `BSSH_PASSWORD` environment variable is supported for automation scenarios (discouraged; see security notes below).
14
+
-**`-S` / `--sudo-password` warning now routes to stderr** when passed to subcommands where it has no effect (`ping`, `upload`, `download`, `list`), keeping stdout clean for scripts that consume bssh output (#201).
-**Explicit**: Use `-A` flag to force SSH agent authentication
575
575
576
576
### Password Authentication
577
-
- Use `-P` flag to enable password authentication
577
+
- Use `-P` / `--password` flag to enable password authentication
578
+
- 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
578
579
- Password is prompted securely without echo
580
+
- For automation, set `BSSH_PASSWORD` in the environment (not recommended; see security notes in the Sudo Password section)
579
581
580
582
### Examples
581
583
```bash
@@ -656,6 +658,15 @@ bssh supports configuration via environment variables:
0 commit comments