Skip to content

fix: warn when sudo password is ignored outside exec#202

Merged
inureyes merged 2 commits into
mainfrom
fix/issue-200-sudo-interactive-warning
May 15, 2026
Merged

fix: warn when sudo password is ignored outside exec#202
inureyes merged 2 commits into
mainfrom
fix/issue-200-sudo-interactive-warning

Conversation

@inureyes
Copy link
Copy Markdown
Member

Summary

Review Notes

  • Correctness: fix: collect --password once up-front and honor -S consistently across subcommands #201 correctly hoists SSH password collection and threads Arc<Password> through exec, ping, SFTP, interactive, jump-host, and legacy command paths. This PR closes the remaining applicability gap for ignored flags.
  • Security: no new secret logging or command execution surface is introduced. The change reduces unnecessary secret collection on local-only paths.
  • Performance: no hot-path overhead beyond small boolean helper checks before dispatch.

Test Plan

  • cargo test --bin bssh dispatcher::tests
  • cargo test --lib security::password
  • cargo test --lib ssh::auth
  • cargo test --lib jump::chain
  • cargo test --lib
  • cargo test --bin bssh
  • cargo fmt --all -- --check
  • cargo check --lib --bins --tests
  • cargo clippy --lib --bins --tests -- -D warnings

inureyes added 2 commits May 15, 2026 17:13
Tighten the issue #200 follow-up so -S is treated as applicable only for exec paths that can actually inject sudo responses. Interactive shells now warn instead of silently ignoring the flag, and local-only dispatcher paths avoid collecting an unused SSH password.

Tests cover the sudo and SSH password applicability helpers for exec, interactive, ping, list, and cache-stat paths.
Remove an unnecessary into_iter call in the internal SFTP crate so the repository-level CI command cargo clippy -- -D warnings passes on the newer clippy version used by GitHub Actions.
@inureyes
Copy link
Copy Markdown
Member Author

Review / Finalization Summary

Scope Reviewed

Findings

  • Correctness gap fixed here: -S was still silently ignored for interactive shells / SSH-mode no-command sessions, because sudo response injection only exists in exec streaming paths.
  • Defensive cleanup fixed here: local-only dispatcher paths now avoid collecting an unused --password if dispatch_command() is invoked directly for list or cache-stats.
  • CI-only issue fixed here: GitHub Actions uses a newer clippy that rejects an existing .chain(files.into_iter()) in the internal SFTP crate; the no-op conversion was removed.

Security Review

  • No new credential logging or command execution surface introduced.
  • The follow-up reduces unnecessary secret collection on local-only paths.
  • Existing BSSH_PASSWORD env-var behavior remains documented as automation-only and discouraged for production.

Performance Review

  • Dispatcher changes are constant-time flag checks before command routing.
  • SFTP clippy fix is behavior-preserving and allocation-neutral.

Validation

  • cargo test --bin bssh dispatcher::tests: passed
  • cargo test --lib security::password: passed
  • cargo test --lib ssh::auth: passed
  • cargo test --lib jump::chain: passed
  • cargo test --lib: passed (1233 passed, 9 ignored)
  • cargo test --bin bssh: passed (51 passed)
  • cargo fmt --all -- --check: passed
  • cargo check --lib --bins --tests: passed
  • cargo clippy --lib --bins --tests -- -D warnings: passed
  • cargo clippy -- -D warnings: passed
  • cargo test -p bssh-russh-sftp --lib: passed (5 passed)
  • cargo test --tests --verbose -- --skip integration_test: passed
  • GitHub Actions CI: passed
  • GitHub license/cla: passed

Remaining Risk

@inureyes inureyes merged commit ddef3aa into main May 15, 2026
2 checks passed
@inureyes inureyes deleted the fix/issue-200-sudo-interactive-warning branch May 15, 2026 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant