Skip to content

fix: collect --password once up-front instead of per-connection; honor -S consistently across subcommands #200

@inureyes

Description

@inureyes

Problem / Background

Running the following on macOS against an orin cluster defined in ~/.config/bssh/config.yaml:

bssh -C orin -l lablup --password -S ping

The user expected the password prompt to appear once, up-front, before execution starts. Instead, execution begins and the prompt either never appears cleanly, gets buried in the parallel progress UI, or would be issued per-node. Two underlying defects are responsible.

Defect 1 — --password is collected lazily per-connection, not up-front

  • src/ssh/auth.rs:412-435 (password_auth()) calls rpassword::prompt_password() inside each node's authentication task.
  • src/commands/ping.rs:25-64 builds the executor and immediately calls executor.execute(\"true\") — no pre-collection step.
  • Because the executor fans out N parallel connection attempts, multiple tasks race for stdin, so the prompt is either missed (drowned by progress output), repeated per node, or interleaved with the progress UI rendering.

Compare with -S (sudo-password), which is done correctly: src/app/dispatcher.rs:469-474 calls get_sudo_password() before building the executor, then wraps it in Arc<SudoPassword> and injects it into every connection task. The --password path should follow the same pattern.

Defect 2 — -S is silently dropped by ping, upload, download

  • The only dispatcher branches that read cli.sudo_password are exec (src/app/dispatcher.rs:470) and the Interactive / SSH-mode interactive paths.
  • The Ping branch (src/app/dispatcher.rs:115-148) does not read cli.sudo_password at all, and ping_nodes() (src/commands/ping.rs:25-36) has no sudo password parameter. Upload (lines ~149-180) and Download (lines ~181-) likewise ignore the flag.
  • This is silent failure: the user passes -S, no error is shown, but the flag has no effect for those subcommands.

For ping specifically, sudo isn't needed (it runs true), so -S being ignored doesn't break behavior — but accepting a flag and ignoring it is misleading. For upload/download, sudo also isn't applicable to the SFTP path, but the same UX concern applies.

Proposed Solution

The user agreed: collecting once is the consistent choice ("한 번 입력하는 게 일관되겠습니다").

1. Move --password collection into the dispatcher, mirroring the -S pattern

  • In src/app/dispatcher.rs, when cli.password is true, prompt once via rpassword::prompt_password() before building the executor.
  • Store as Arc<Zeroizing<String>> (or a new Password wrapper analogous to SudoPassword in src/security/sudo.rs, using secrecy::SecretString + Zeroizing) and thread it through the executor / connection manager so every per-node auth task uses the same value instead of prompting itself.
  • Remove the per-connection rpassword::prompt_password() call in src/ssh/auth.rs:password_auth() — it should consume the pre-collected password.
  • Apply to all subcommands that take --password: exec, ping, upload, download, interactive, and SSH mode.

2. Decide -S semantics for ping/upload/download

Since sudo isn't applicable for these subcommands, either:

  • (a) Print a clear warning when -S is passed ("-S has no effect for this subcommand"), or
  • (b) Reject it at CLI parse time.

Pick whichever fits the project's UX norms.

3. Optional: improve the prompt text

The prompt format in src/ssh/auth.rs:422 is \"Enter password for {username}@{host}: \". Once the prompt is hoisted to the dispatcher, change it to a single non-host-specific prompt, e.g., \"Enter SSH password (used for all hosts): \".

Acceptance Criteria

  • Running bssh -C <cluster> --password <subcommand> prompts exactly once, before any connection attempt or progress UI output.
  • The same password is used for every node in the cluster.
  • Password memory is zeroized after use (use Zeroizing / SecretString).
  • -S either applies to applicable subcommands or warns/errors when passed to subcommands where it has no effect.
  • Existing tests pass.
  • A unit test covers the dispatcher-level password collection (similar to existing tests in src/security/sudo.rs).

Technical Considerations

  • Follow the existing SudoPassword pattern in src/security/sudo.rs for credential handling and zeroization.
  • Threading the password through the executor will likely require adding an optional Arc<Password> to the executor's connection-task spawn API, mirroring how sudo_password is propagated today.
  • Auditing where password_auth() is invoked is required to ensure no caller relies on the embedded prompt.
  • Watch for terminal-state pitfalls: prompting after the progress UI has been initialized (indicatif) can mangle terminal output; the up-front prompt must run before any progress bar is rendered.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions