From feacffd87e5eafdba6eed4718d58c291798a3bfc Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Wed, 10 Dec 2025 19:02:34 +0900 Subject: [PATCH 01/13] feat: Add --sudo-password flag for automated sudo authentication Add secure sudo password handling for commands requiring elevated privileges: - Add -S/--sudo-password CLI flag to prompt for sudo password - Create security/sudo module with SudoPassword struct using zeroize - Implement sudo prompt detection patterns for various Linux distributions - Add execute_with_sudo method to SSH client with PTY support - Support both streaming and non-streaming execution paths - Support BSSH_SUDO_PASSWORD environment variable (with security warnings) Security features: - Password stored using zeroize crate for automatic memory clearing - Debug output redacts password content - PTY allocation ensures proper sudo interaction - Password never logged or printed in any output Closes #74 --- ARCHITECTURE.md | 116 +++++++- README.md | 53 ++++ src/app/dispatcher.rs | 10 + src/cli.rs | 7 + src/commands/exec.rs | 6 +- src/executor/connection_manager.rs | 38 ++- src/executor/parallel.rs | 67 ++++- src/security/mod.rs | 31 +++ src/security/sudo.rs | 286 ++++++++++++++++++++ src/{security.rs => security/validation.rs} | 0 src/ssh/client/command.rs | 108 ++++++++ src/ssh/tokio_client/channel_manager.rs | 149 ++++++++++ 12 files changed, 849 insertions(+), 22 deletions(-) create mode 100644 src/security/mod.rs create mode 100644 src/security/sudo.rs rename src/{security.rs => security/validation.rs} (100%) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 9a7932be..8678f12e 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -952,9 +952,119 @@ Comprehensive test coverage including: - Single source of truth for authentication logic - Easier to add new authentication methods - Consistent behavior across all bssh commands -- Reduced bug surface area -- Improved code maintainability -- Better test coverage + +### 4.2 Sudo Password Support (`security/sudo.rs`) + +**Status:** Implemented (2025-12-10) as Issue #74 + +**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. + +**Architecture Components:** + +1. **SudoPassword Struct (`security/sudo.rs`)** + - Wraps password string with automatic memory clearing via `zeroize` crate + - Uses `Arc` for safe sharing across async tasks + - Debug output redacts password content + + ```rust + #[derive(Clone, ZeroizeOnDrop)] + pub struct SudoPassword { + inner: Arc, + } + ``` + +2. **Prompt Detection Patterns** + - Case-insensitive matching against common sudo prompts + - Supports various Linux distributions: + - `[sudo] password for :` + - `Password:` + - `'s password:` + - Also detects failure patterns like "Sorry, try again" + +3. **Password Injection Flow** + ``` + Command Execution + | + +--> PTY Channel Opened (required for sudo interaction) + | | + | Output Monitoring + | | + | [Sudo Prompt Detected?] -- No --> Continue + | |Yes + | Send Password + Newline + | | + +--- Continue Monitoring + ``` + +**Implementation Details:** + +```rust +// Prompt detection patterns +pub const SUDO_PROMPT_PATTERNS: &[&str] = &[ + "[sudo] password for ", + "password for ", + "password:", + "'s password:", + "sudo password", + "enter password", + "[sudo]", +]; + +// Failure detection patterns +pub const SUDO_FAILURE_PATTERNS: &[&str] = &[ + "sorry, try again", + "incorrect password", + "authentication failure", + "permission denied", +]; +``` + +**SSH Channel Integration (`tokio_client/channel_manager.rs`):** +- Executes command with PTY allocation (required for sudo to send prompts) +- Monitors both stdout and stderr for sudo prompts +- Uses `channel.data()` to write password to stdin when prompt detected +- Password sent only once per execution to prevent retry loops + +```rust +pub async fn execute_with_sudo( + &self, + command: &str, + sender: Sender, + sudo_password: &SudoPassword, +) -> Result +``` + +**Security Considerations:** +- Password stored using `zeroize` crate for automatic memory clearing +- Password never logged or printed in any output +- PTY required for proper sudo interaction (prevents stdin echo issues) +- Environment variable option (`BSSH_SUDO_PASSWORD`) with security warnings + +**Execution Path Integration:** +1. CLI flag `-S/--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 +5. Per-node execution uses `execute_with_sudo()` when password present + +**Usage Patterns:** +```bash +# Basic usage - prompts for password before execution +bssh -S -C production "sudo apt update" + +# Combined with SSH agent authentication +bssh -A -S -C production "sudo systemctl restart nginx" + +# Environment variable (not recommended) +export BSSH_SUDO_PASSWORD="password" +bssh -S -C production "sudo apt update" +``` + +**Limitations:** +- Single password for all nodes (cannot handle different passwords per node) +- Assumes all nodes use the same sudo configuration +- Password cached for session duration (cleared on command completion) **Future Enhancements:** - Support for additional authentication methods (hardware tokens, certificates) diff --git a/README.md b/README.md index 39bc26bf..a5bd925a 100644 --- a/README.md +++ b/README.md @@ -183,6 +183,12 @@ bssh -A -C 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" + +# Combine sudo password with SSH agent authentication +bssh -A -S -C production "sudo systemctl restart nginx" + # Use encrypted SSH key (will prompt for passphrase) bssh -i ~/.ssh/encrypted_key -C production "df -h" @@ -350,6 +356,44 @@ bssh -A -J bastion.example.com user@internal-server "uptime" bssh -i ~/.ssh/prod_key -J "jump1,jump2" -C production "df -h" ``` +### Sudo Password Support + +bssh supports automatic sudo password injection for commands that require elevated privileges. When enabled, bssh will: +1. Securely prompt for the sudo password before command execution +2. Detect sudo password prompts in command output +3. Automatically inject the password when prompted +4. Clear the password from memory after use + +```bash +# Basic sudo command (will prompt for sudo password) +bssh -S -C production "sudo apt update" + +# Combine with SSH agent authentication +bssh -A -S -C production "sudo systemctl restart nginx" + +# Multiple sudo commands in a single session +bssh -S -C production "sudo apt update && sudo apt upgrade -y" + +# Sudo with specific SSH key +bssh -i ~/.ssh/admin_key -S -C production "sudo reboot" +``` + +**Environment Variable Alternative:** + +For automation scenarios, you can use the `BSSH_SUDO_PASSWORD` environment variable: + +```bash +# NOT RECOMMENDED for security reasons +export BSSH_SUDO_PASSWORD="your-password" +bssh -S -C 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 +- Password is automatically cleared from memory after use using `zeroize` + ## Environment Variables bssh supports configuration via environment variables: @@ -372,6 +416,14 @@ bssh supports configuration via environment variables: - **`SSH_AUTH_SOCK`**: SSH agent socket path (Unix-like systems) +### 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"` + ## Configuration ### Configuration Priority Order @@ -822,6 +874,7 @@ Options: -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 -J, --jump-host Comma-separated list of jump hosts (ProxyJump) -L, --local-forward Local port forwarding [bind_address:]port:host:hostport -R, --remote-forward Remote port forwarding [bind_address:]port:host:hostport diff --git a/src/app/dispatcher.rs b/src/app/dispatcher.rs index 06417b56..9c2b6d4b 100644 --- a/src/app/dispatcher.rs +++ b/src/app/dispatcher.rs @@ -27,8 +27,10 @@ use bssh::{ }, config::InteractiveMode, pty::PtyConfig, + security::get_sudo_password, }; use std::path::{Path, PathBuf}; +use std::sync::Arc; #[cfg(target_os = "macos")] use super::initialization::determine_use_keychain; @@ -368,6 +370,13 @@ async fn handle_exec_command(cli: &Cli, ctx: &AppContext, command: &str) -> Resu #[cfg(target_os = "macos")] let use_keychain = determine_use_keychain(&ctx.ssh_config, hostname.as_deref()); + // Get sudo password if flag is set + let sudo_password = if cli.sudo_password { + Some(Arc::new(get_sudo_password(true)?)) + } else { + None + }; + let params = ExecuteCommandParams { nodes: ctx.nodes.clone(), command, @@ -390,6 +399,7 @@ async fn handle_exec_command(cli: &Cli, ctx: &AppContext, command: &str) -> Resu }, require_all_success: cli.require_all_success, check_all_nodes: cli.check_all_nodes, + sudo_password, }; execute_command(params).await } diff --git a/src/cli.rs b/src/cli.rs index e6ddb973..8f696aca 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -90,6 +90,13 @@ 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 = 'J', long = "jump-host", diff --git a/src/commands/exec.rs b/src/commands/exec.rs index 2496415c..69976179 100644 --- a/src/commands/exec.rs +++ b/src/commands/exec.rs @@ -14,10 +14,12 @@ use anyhow::Result; use std::path::Path; +use std::sync::Arc; use crate::executor::{ExitCodeStrategy, OutputMode, ParallelExecutor, RankDetector}; use crate::forwarding::ForwardingType; use crate::node::Node; +use crate::security::SudoPassword; use crate::ssh::known_hosts::StrictHostKeyChecking; use crate::ui::OutputFormatter; use crate::utils::output::save_outputs_to_files; @@ -40,6 +42,7 @@ pub struct ExecuteCommandParams<'a> { pub port_forwards: Option>, pub require_all_success: bool, pub check_all_nodes: bool, + pub sudo_password: Option>, } pub async fn execute_command(params: ExecuteCommandParams<'_>) -> Result<()> { @@ -202,7 +205,8 @@ async fn execute_command_without_forwarding(params: ExecuteCommandParams<'_>) -> params.use_password, ) .with_timeout(params.timeout) - .with_jump_hosts(params.jump_hosts.map(|s| s.to_string())); + .with_jump_hosts(params.jump_hosts.map(|s| s.to_string())) + .with_sudo_password(params.sudo_password); // Set keychain usage if on macOS #[cfg(target_os = "macos")] diff --git a/src/executor/connection_manager.rs b/src/executor/connection_manager.rs index 2e9c364f..f7ee1468 100644 --- a/src/executor/connection_manager.rs +++ b/src/executor/connection_manager.rs @@ -16,8 +16,10 @@ use anyhow::Result; use std::path::{Path, PathBuf}; +use std::sync::Arc; use crate::node::Node; +use crate::security::SudoPassword; use crate::ssh::{ client::{CommandResult, ConnectionConfig}, known_hosts::StrictHostKeyChecking, @@ -35,6 +37,7 @@ pub(crate) struct ExecutionConfig<'a> { pub use_keychain: bool, pub timeout: Option, pub jump_hosts: Option<&'a str>, + pub sudo_password: Option>, } /// Execute a command on a node with jump host support. @@ -58,9 +61,38 @@ pub(crate) async fn execute_on_node_with_jump_hosts( jump_hosts_spec: config.jump_hosts, }; - client - .connect_and_execute_with_jump_hosts(command, &connection_config) - .await + // If sudo password is provided, use streaming execution to handle prompts + if let Some(ref sudo_password) = config.sudo_password { + use crate::ssh::tokio_client::CommandOutput; + use tokio::sync::mpsc; + + let (tx, mut rx) = mpsc::channel(1000); + let exit_status = client + .connect_and_execute_with_sudo(command, &connection_config, tx, sudo_password) + .await?; + + // Collect output from channel + let mut stdout = Vec::new(); + let mut stderr = Vec::new(); + + while let Some(output) = rx.recv().await { + match output { + CommandOutput::StdOut(data) => stdout.extend_from_slice(&data), + CommandOutput::StdErr(data) => stderr.extend_from_slice(&data), + } + } + + Ok(CommandResult { + host: node.host.clone(), + output: stdout, + stderr, + exit_status, + }) + } else { + client + .connect_and_execute_with_jump_hosts(command, &connection_config) + .await + } } /// Upload a file or directory to a node with jump host support. diff --git a/src/executor/parallel.rs b/src/executor/parallel.rs index fc0f95b9..cf743056 100644 --- a/src/executor/parallel.rs +++ b/src/executor/parallel.rs @@ -22,6 +22,7 @@ use std::sync::Arc; use tokio::sync::Semaphore; use crate::node::Node; +use crate::security::SudoPassword; use crate::ssh::known_hosts::StrictHostKeyChecking; use super::connection_manager::{download_from_node, ExecutionConfig}; @@ -44,6 +45,7 @@ pub struct ParallelExecutor { pub(crate) use_keychain: bool, pub(crate) timeout: Option, pub(crate) jump_hosts: Option, + pub(crate) sudo_password: Option>, } impl ParallelExecutor { @@ -75,6 +77,7 @@ impl ParallelExecutor { use_keychain: false, timeout: None, jump_hosts: None, + sudo_password: None, } } @@ -97,6 +100,7 @@ impl ParallelExecutor { use_keychain: false, timeout: None, jump_hosts: None, + sudo_password: None, } } @@ -120,6 +124,7 @@ impl ParallelExecutor { use_keychain: false, timeout: None, jump_hosts: None, + sudo_password: None, } } @@ -142,6 +147,15 @@ impl ParallelExecutor { self } + /// Set sudo password for automatic sudo authentication. + /// + /// When set, the executor will automatically detect sudo password prompts + /// and inject the password when commands require sudo privileges. + pub fn with_sudo_password(mut self, sudo_password: Option>) -> Self { + self.sudo_password = sudo_password; + self + } + /// Execute a command on all nodes in parallel. pub async fn execute(&self, command: &str) -> Result> { let semaphore = Arc::new(Semaphore::new(self.max_parallel)); @@ -162,6 +176,7 @@ impl ParallelExecutor { let use_keychain = self.use_keychain; let timeout = self.timeout; let jump_hosts = self.jump_hosts.clone(); + let sudo_password = self.sudo_password.clone(); let semaphore = Arc::clone(&semaphore); let pb = setup_progress_bar(&multi_progress, &node, style.clone(), "Connecting..."); @@ -175,6 +190,7 @@ impl ParallelExecutor { use_keychain, timeout, jump_hosts: jump_hosts.as_deref(), + sudo_password: sudo_password.clone(), }; execute_command_task(node, command, config, semaphore, pb).await @@ -504,6 +520,7 @@ impl ParallelExecutor { let use_keychain = self.use_keychain; let timeout = self.timeout; let jump_hosts = self.jump_hosts.clone(); + let sudo_password = self.sudo_password.clone(); let semaphore = Arc::clone(&semaphore); let handle = tokio::spawn(async move { @@ -551,22 +568,42 @@ impl ParallelExecutor { jump_hosts_spec: jump_hosts.as_deref(), }; - // Ensure channel is closed on all paths - let result = match client - .connect_and_execute_with_output_streaming(&command, &config, tx.clone()) - .await - { - Ok(exit_status) => { - tracing::debug!( - "Command completed for {}: exit code {}", - node_clone.host, - exit_status - ); - (node_clone, Ok(exit_status)) + // Execute with or without sudo password support + let result = if let Some(ref sudo_pwd) = sudo_password { + match client + .connect_and_execute_with_sudo(&command, &config, tx.clone(), sudo_pwd) + .await + { + Ok(exit_status) => { + tracing::debug!( + "Sudo command completed for {}: exit code {}", + node_clone.host, + exit_status + ); + (node_clone, Ok(exit_status)) + } + Err(e) => { + tracing::error!("Sudo command failed for {}: {}", node_clone.host, e); + (node_clone, Err(e)) + } } - Err(e) => { - tracing::error!("Command failed for {}: {}", node_clone.host, e); - (node_clone, Err(e)) + } else { + match client + .connect_and_execute_with_output_streaming(&command, &config, tx.clone()) + .await + { + Ok(exit_status) => { + tracing::debug!( + "Command completed for {}: exit code {}", + node_clone.host, + exit_status + ); + (node_clone, Ok(exit_status)) + } + Err(e) => { + tracing::error!("Command failed for {}: {}", node_clone.host, e); + (node_clone, Err(e)) + } } }; diff --git a/src/security/mod.rs b/src/security/mod.rs new file mode 100644 index 00000000..6fd8a8d3 --- /dev/null +++ b/src/security/mod.rs @@ -0,0 +1,31 @@ +// 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. + +//! Security utilities for validating and sanitizing user input and handling +//! sensitive data securely. + +mod sudo; +mod validation; + +// Re-export validation functions +pub use validation::{ + sanitize_error_message, validate_hostname, validate_local_path, validate_remote_path, + validate_username, +}; + +// Re-export sudo password handling +pub use sudo::{ + contains_sudo_failure, contains_sudo_prompt, get_sudo_password, prompt_sudo_password, + SudoPassword, SUDO_FAILURE_PATTERNS, SUDO_PROMPT_PATTERNS, +}; diff --git a/src/security/sudo.rs b/src/security/sudo.rs new file mode 100644 index 00000000..c0137a9d --- /dev/null +++ b/src/security/sudo.rs @@ -0,0 +1,286 @@ +// 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. + +//! Secure sudo password handling with automatic memory clearing. +//! +//! This module provides: +//! - `SudoPassword`: A secure wrapper for sudo passwords with automatic zeroization +//! - Sudo prompt detection patterns for various Linux distributions +//! - Secure password input functions +//! +//! # Security Considerations +//! - Passwords are automatically cleared from memory when dropped +//! - Never log or print sudo passwords +//! - Environment variable usage is discouraged due to security risks + +use anyhow::Result; +use std::fmt; +use std::sync::Arc; +use zeroize::ZeroizeOnDrop; + +/// Common sudo password prompt patterns across different distributions +/// +/// These patterns are case-insensitive and designed to match: +/// - Standard sudo prompts: `[sudo] password for username:` +/// - Generic password prompts: `Password:` +/// - User-specific prompts: `username's password:` +/// - Custom sudo prompts that may vary by distribution +pub const SUDO_PROMPT_PATTERNS: &[&str] = &[ + "[sudo] password for ", + "password for ", + "password:", + "'s password:", + "sudo password", + "enter password", + "[sudo]", +]; + +/// Patterns indicating sudo authentication failure +pub const SUDO_FAILURE_PATTERNS: &[&str] = &[ + "sorry, try again", + "incorrect password", + "authentication failure", + "permission denied", + "sudo: 3 incorrect password attempts", + "sudo: no password was provided", +]; + +/// A secure wrapper for sudo passwords that automatically clears memory on drop. +/// +/// This struct uses the `zeroize` crate to ensure the password is securely +/// cleared from memory when the struct is dropped, preventing sensitive data +/// from remaining in memory. +/// +/// # Security +/// - Password is automatically zeroized when dropped +/// - Debug output does not reveal the password +/// - Clone creates a new copy that is also zeroized independently +#[derive(Clone, ZeroizeOnDrop)] +pub struct SudoPassword { + /// The actual password bytes + #[zeroize(skip)] // We handle the inner zeroization manually via Arc + inner: Arc, +} + +/// Inner container for the password with zeroize support +#[derive(ZeroizeOnDrop)] +struct SudoPasswordInner { + password: String, +} + +impl SudoPassword { + /// Create a new SudoPassword from a string. + /// + /// The password will be automatically cleared from memory when all + /// references to this SudoPassword are dropped. + pub fn new(password: String) -> Self { + Self { + inner: Arc::new(SudoPasswordInner { password }), + } + } + + /// Get the password as bytes for sending over SSH. + /// + /// # Security Note + /// The returned bytes should be used immediately and not stored. + pub fn as_bytes(&self) -> &[u8] { + self.inner.password.as_bytes() + } + + /// Get the password with a newline appended for sudo input. + /// + /// Sudo requires a newline after the password to submit it. + pub fn with_newline(&self) -> Vec { + let mut bytes = self.inner.password.as_bytes().to_vec(); + bytes.push(b'\n'); + bytes + } + + /// Check if the password is empty. + pub fn is_empty(&self) -> bool { + self.inner.password.is_empty() + } +} + +impl fmt::Debug for SudoPassword { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("SudoPassword") + .field("password", &"[REDACTED]") + .finish() + } +} + +/// Check if the given output contains a sudo password prompt. +/// +/// This function performs case-insensitive matching against known +/// sudo prompt patterns. +/// +/// # Arguments +/// * `output` - The command output to check (stdout or stderr) +/// +/// # Returns +/// `true` if a sudo prompt is detected, `false` otherwise +pub fn contains_sudo_prompt(output: &str) -> bool { + let lower = output.to_lowercase(); + SUDO_PROMPT_PATTERNS + .iter() + .any(|pattern| lower.contains(*pattern)) +} + +/// Check if the given output contains a sudo authentication failure message. +/// +/// # Arguments +/// * `output` - The command output to check +/// +/// # Returns +/// `true` if a failure message is detected, `false` otherwise +pub fn contains_sudo_failure(output: &str) -> bool { + let lower = output.to_lowercase(); + SUDO_FAILURE_PATTERNS + .iter() + .any(|pattern| lower.contains(*pattern)) +} + +/// Prompt the user for a sudo password securely. +/// +/// This function uses `rpassword` to securely read the password without +/// echoing it to the terminal. +/// +/// # Returns +/// A `SudoPassword` containing the entered password, or an error if +/// reading fails. +/// +/// # Security Note +/// - The password is never printed to stdout/stderr +/// - The password is stored in a zeroizing container +pub fn prompt_sudo_password() -> Result { + eprintln!("Enter sudo password: "); + let password = rpassword::read_password().map_err(|e| { + anyhow::anyhow!("Failed to read sudo password: {}", e) + })?; + Ok(SudoPassword::new(password)) +} + +/// Get sudo password from environment variable (if set). +/// +/// # Security Warning +/// Using environment variables for passwords is NOT recommended in production +/// as they may be visible in process listings and shell history. +/// This is provided for automation scenarios where the security trade-off +/// is acceptable. +/// +/// # Returns +/// `Some(SudoPassword)` if `BSSH_SUDO_PASSWORD` is set, `None` otherwise. +pub fn get_sudo_password_from_env() -> Option { + std::env::var("BSSH_SUDO_PASSWORD") + .ok() + .filter(|s| !s.is_empty()) + .map(SudoPassword::new) +} + +/// Get sudo password from either environment or interactive prompt. +/// +/// This function first checks the `BSSH_SUDO_PASSWORD` environment variable. +/// If not set, it prompts the user interactively. +/// +/// # Arguments +/// * `warn_env` - If `true`, print a warning when using environment variable +/// +/// # Returns +/// A `SudoPassword` containing the password. +pub fn get_sudo_password(warn_env: bool) -> Result { + if let Some(password) = get_sudo_password_from_env() { + if warn_env { + eprintln!( + "Warning: Using sudo password from BSSH_SUDO_PASSWORD environment variable. \ + This is not recommended for security reasons." + ); + } + Ok(password) + } else { + prompt_sudo_password() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_sudo_password_creation() { + let password = SudoPassword::new("test123".to_string()); + assert_eq!(password.as_bytes(), b"test123"); + assert!(!password.is_empty()); + } + + #[test] + fn test_sudo_password_with_newline() { + let password = SudoPassword::new("test123".to_string()); + let with_newline = password.with_newline(); + assert_eq!(with_newline, b"test123\n"); + } + + #[test] + fn test_sudo_password_debug_redaction() { + let password = SudoPassword::new("secret".to_string()); + let debug_output = format!("{:?}", password); + assert!(!debug_output.contains("secret")); + assert!(debug_output.contains("[REDACTED]")); + } + + #[test] + fn test_empty_password() { + let password = SudoPassword::new(String::new()); + assert!(password.is_empty()); + } + + #[test] + fn test_contains_sudo_prompt() { + // Standard sudo prompts + assert!(contains_sudo_prompt("[sudo] password for user:")); + assert!(contains_sudo_prompt("Password:")); + assert!(contains_sudo_prompt("user's password:")); + assert!(contains_sudo_prompt("[sudo] password for admin:")); + + // Case insensitive + assert!(contains_sudo_prompt("[SUDO] PASSWORD FOR USER:")); + assert!(contains_sudo_prompt("PASSWORD:")); + + // Should not match + assert!(!contains_sudo_prompt("Command executed successfully")); + assert!(!contains_sudo_prompt("root@server:~#")); + } + + #[test] + fn test_contains_sudo_failure() { + assert!(contains_sudo_failure("Sorry, try again.")); + assert!(contains_sudo_failure("sudo: 3 incorrect password attempts")); + assert!(contains_sudo_failure("Authentication failure")); + assert!(contains_sudo_failure("Permission denied")); + + // Should not match + assert!(!contains_sudo_failure("Command executed successfully")); + assert!(!contains_sudo_failure("password accepted")); + } + + #[test] + fn test_clone_independence() { + let password1 = SudoPassword::new("original".to_string()); + let password2 = password1.clone(); + + // Both should work independently + assert_eq!(password1.as_bytes(), b"original"); + assert_eq!(password2.as_bytes(), b"original"); + } +} diff --git a/src/security.rs b/src/security/validation.rs similarity index 100% rename from src/security.rs rename to src/security/validation.rs diff --git a/src/ssh/client/command.rs b/src/ssh/client/command.rs index 1b5b0815..9e7390db 100644 --- a/src/ssh/client/command.rs +++ b/src/ssh/client/command.rs @@ -15,6 +15,7 @@ use super::config::ConnectionConfig; use super::core::SshClient; use super::result::CommandResult; +use crate::security::SudoPassword; use crate::ssh::known_hosts::StrictHostKeyChecking; use crate::ssh::tokio_client::CommandOutput; use anyhow::{Context, Result}; @@ -266,4 +267,111 @@ impl SshClient { .with_context(|| format!("Failed to execute command '{}' on {}:{}. The SSH connection was successful but the command could not be executed.", command, self.host, self.port)) } } + + /// Execute a command with sudo password support and streaming output. + /// + /// This method handles automatic sudo password injection when sudo prompts are detected + /// in the command output. + /// + /// # Arguments + /// * `command` - The command to execute (typically uses sudo) + /// * `config` - Connection configuration + /// * `output_sender` - Channel sender for streaming output + /// * `sudo_password` - The sudo password to inject when prompted + /// + /// # Returns + /// The exit status of the command + pub async fn connect_and_execute_with_sudo( + &mut self, + command: &str, + config: &ConnectionConfig<'_>, + output_sender: Sender, + sudo_password: &SudoPassword, + ) -> Result { + tracing::debug!("Connecting to {}:{} for sudo execution", self.host, self.port); + + // Determine authentication method based on parameters + let auth_method = self + .determine_auth_method( + config.key_path, + config.use_agent, + config.use_password, + #[cfg(target_os = "macos")] + config.use_keychain, + ) + .await?; + + let strict_mode = config + .strict_mode + .unwrap_or(StrictHostKeyChecking::AcceptNew); + + // Create client connection - either direct or through jump hosts + let client = self + .establish_connection( + &auth_method, + strict_mode, + config.jump_hosts_spec, + config.key_path, + config.use_agent, + config.use_password, + ) + .await?; + + tracing::debug!("Connected and authenticated successfully"); + tracing::debug!("Executing command with sudo support: {}", command); + + // Execute command with sudo support and timeout + let exit_status = self + .execute_sudo_with_timeout(&client, command, config.timeout_seconds, output_sender, sudo_password) + .await?; + + tracing::debug!("Command execution completed with status: {}", exit_status); + + Ok(exit_status) + } + + /// Execute a command with sudo support and the specified timeout + async fn execute_sudo_with_timeout( + &self, + client: &crate::ssh::tokio_client::Client, + command: &str, + timeout_seconds: Option, + output_sender: Sender, + sudo_password: &SudoPassword, + ) -> Result { + if let Some(timeout_secs) = timeout_seconds { + if timeout_secs == 0 { + // No timeout (unlimited) + tracing::debug!("Executing sudo command with no timeout (unlimited)"); + client.execute_with_sudo(command, output_sender, sudo_password) + .await + .with_context(|| format!("Failed to execute sudo command '{}' on {}:{}", command, self.host, self.port)) + } else { + // With timeout + let command_timeout = Duration::from_secs(timeout_secs); + tracing::debug!( + "Executing sudo command with timeout of {} seconds", + timeout_secs + ); + tokio::time::timeout( + command_timeout, + client.execute_with_sudo(command, output_sender, sudo_password) + ) + .await + .with_context(|| format!("Command execution timeout: The sudo command '{}' did not complete within {} seconds on {}:{}", command, timeout_secs, self.host, self.port))? + .with_context(|| format!("Failed to execute sudo command '{}' on {}:{}", command, self.host, self.port)) + } + } else { + // Default timeout if not specified + let command_timeout = Duration::from_secs(DEFAULT_COMMAND_TIMEOUT_SECS); + tracing::debug!("Executing sudo command with default timeout of 300 seconds"); + tokio::time::timeout( + command_timeout, + client.execute_with_sudo(command, output_sender, sudo_password) + ) + .await + .with_context(|| format!("Command execution timeout: The sudo command '{}' did not complete within 5 minutes on {}:{}", command, self.host, self.port))? + .with_context(|| format!("Failed to execute sudo command '{}' on {}:{}", command, self.host, self.port)) + } + } } diff --git a/src/ssh/tokio_client/channel_manager.rs b/src/ssh/tokio_client/channel_manager.rs index ef274435..575f1bed 100644 --- a/src/ssh/tokio_client/channel_manager.rs +++ b/src/ssh/tokio_client/channel_manager.rs @@ -28,6 +28,7 @@ use std::net::SocketAddr; use tokio::sync::mpsc::{channel, Receiver, Sender}; use tokio::task::JoinHandle; +use crate::security::{contains_sudo_failure, contains_sudo_prompt, SudoPassword}; use super::connection::Client; use super::ToSocketAddrsWithHostname; @@ -297,6 +298,154 @@ impl Client { } } + /// Execute a remote command with sudo password support. + /// + /// This method handles automatic sudo password injection when sudo prompts are detected + /// in the command output. It monitors both stdout and stderr for sudo password prompts + /// and automatically sends the password when detected. + /// + /// # Arguments + /// * `command` - The command to execute (typically starts with `sudo`) + /// * `sender` - Channel sender for streaming output + /// * `sudo_password` - The sudo password to inject when prompted + /// + /// # Returns + /// The exit status of the command + /// + /// # Security + /// - Password is only sent when a sudo prompt is detected + /// - Password is never logged or included in error messages + /// - Detects sudo authentication failures and reports them appropriately + pub async fn execute_with_sudo( + &self, + command: &str, + sender: Sender, + sudo_password: &SudoPassword, + ) -> Result { + // Sanitize command to prevent injection attacks + let sanitized_command = crate::utils::sanitize_command(command) + .map_err(|e| super::Error::CommandValidationFailed(e.to_string()))?; + + // Request a PTY for sudo to properly interact with + // Sudo requires a PTY to prompt for password + let mut channel = self.connection_handle.channel_open_session().await?; + + // Request PTY with reasonable defaults for sudo + channel + .request_pty( + true, // want reply + "xterm", // term type + 80, // columns + 24, // rows + 0, // pixel width + 0, // pixel height + &[], // terminal modes (empty for defaults) + ) + .await?; + + channel.exec(true, sanitized_command.as_str()).await?; + + let mut result: Option = None; + let mut password_sent = false; + let mut accumulated_output = String::new(); + + // While the channel has messages... + while let Some(msg) = channel.wait().await { + match msg { + russh::ChannelMsg::Data { ref data } => { + // Check for sudo prompt before sending to output + let text = String::from_utf8_lossy(data); + accumulated_output.push_str(&text); + + // Send output to streaming channel + match sender.try_send(CommandOutput::StdOut(data.clone())) { + Ok(_) => {} + Err(tokio::sync::mpsc::error::TrySendError::Full(output)) => { + tracing::trace!("Channel full, applying backpressure for stdout"); + if sender.send(output).await.is_err() { + tracing::debug!("Receiver dropped, stopping stdout processing"); + break; + } + } + Err(tokio::sync::mpsc::error::TrySendError::Closed(_)) => { + tracing::debug!("Channel closed, stopping stdout processing"); + break; + } + } + + // Check if we need to send the password + if !password_sent && contains_sudo_prompt(&accumulated_output) { + tracing::debug!("Sudo prompt detected, sending password"); + // Send the password with newline + let password_data = sudo_password.with_newline(); + if let Err(e) = channel.data(&password_data[..]).await { + tracing::error!("Failed to send sudo password: {}", e); + return Err(super::Error::SshError(e)); + } + password_sent = true; + // Clear accumulated output after sending password + accumulated_output.clear(); + } + + // Check for sudo failure after password was sent + if password_sent && contains_sudo_failure(&accumulated_output) { + tracing::warn!("Sudo authentication failed"); + // Continue to collect output - the exit code will indicate failure + } + } + russh::ChannelMsg::ExtendedData { ref data, ext } => { + if ext == 1 { + // Stderr - also check for sudo prompts + let text = String::from_utf8_lossy(data); + accumulated_output.push_str(&text); + + match sender.try_send(CommandOutput::StdErr(data.clone())) { + Ok(_) => {} + Err(tokio::sync::mpsc::error::TrySendError::Full(output)) => { + tracing::trace!("Channel full, applying backpressure for stderr"); + if sender.send(output).await.is_err() { + tracing::debug!("Receiver dropped, stopping stderr processing"); + break; + } + } + Err(tokio::sync::mpsc::error::TrySendError::Closed(_)) => { + tracing::debug!("Channel closed, stopping stderr processing"); + break; + } + } + + // Check if we need to send the password (sudo can prompt on stderr) + if !password_sent && contains_sudo_prompt(&accumulated_output) { + tracing::debug!("Sudo prompt detected on stderr, sending password"); + let password_data = sudo_password.with_newline(); + if let Err(e) = channel.data(&password_data[..]).await { + tracing::error!("Failed to send sudo password: {}", e); + return Err(super::Error::SshError(e)); + } + password_sent = true; + accumulated_output.clear(); + } + + // Check for sudo failure + if password_sent && contains_sudo_failure(&accumulated_output) { + tracing::warn!("Sudo authentication failed (stderr)"); + } + } + } + russh::ChannelMsg::ExitStatus { exit_status } => result = Some(exit_status), + _ => {} + } + } + + drop(sender); + + if let Some(result) = result { + Ok(result) + } else { + Err(super::Error::CommandDidntExit) + } + } + /// Execute a remote command via the ssh connection. /// /// Returns stdout, stderr and the exit code of the command, From ab6d19e3bdb1098da5dab7f700e240f76827ac9f Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Wed, 10 Dec 2025 19:12:04 +0900 Subject: [PATCH 02/13] security: Fix critical security issues in sudo password handling This commit addresses all security issues found in the PR review: 1. CRITICAL: Replace Arc with SecretString - Replaced Arc with secrecy::SecretString crate - SecretString is specifically designed for handling secrets in memory - Works correctly with cloning in parallel execution contexts - Each clone is independent and properly zeroized 2. HIGH: Empty password validation - Added validation in SudoPassword::new() to reject empty passwords - Added validation in prompt_sudo_password() to reject empty input - Added validation in get_sudo_password_from_env() to reject empty env vars - Returns clear error messages for empty passwords 3. MEDIUM: Zeroizing copy in with_newline() - Changed return type from Vec to Zeroizing> - Ensures the copy with newline is also cleared from memory - Uses zeroize crate's Zeroizing wrapper for automatic cleanup 4. MEDIUM: Unbounded buffer growth protection - Added MAX_SUDO_PROMPT_BUFFER_SIZE constant (64KB limit) - Enforced buffer size limit in execute_with_sudo() for both stdout and stderr - Truncates buffer to last 64KB when limit exceeded - Prevents memory exhaustion on commands with large output All tests pass successfully. --- Cargo.lock | 11 ++ Cargo.toml | 3 +- src/security/sudo.rs | 179 +++++++++++++++++------- src/ssh/tokio_client/channel_manager.rs | 30 ++++ 4 files changed, 169 insertions(+), 54 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 10cafe2f..20433a84 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -351,6 +351,7 @@ dependencies = [ "russh", "russh-sftp", "rustyline", + "secrecy", "security-framework", "serde", "serde_yaml", @@ -2728,6 +2729,16 @@ dependencies = [ "zeroize", ] +[[package]] +name = "secrecy" +version = "0.10.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e891af845473308773346dc847b2c23ee78fe442e0472ac50e22a18a93d3ae5a" +dependencies = [ + "serde", + "zeroize", +] + [[package]] name = "security-framework" version = "3.5.1" diff --git a/Cargo.toml b/Cargo.toml index d0549691..3831c0a3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,7 +34,8 @@ owo-colors = "4.2.2" unicode-width = "0.2.0" terminal_size = "0.4.3" once_cell = "1.20" -zeroize = "1.8" +zeroize = { version = "1.8", features = ["derive"] } +secrecy = { version = "0.10.3", features = ["serde"] } rustyline = "17.0.1" crossterm = "0.29" ratatui = "0.29" diff --git a/src/security/sudo.rs b/src/security/sudo.rs index c0137a9d..f5b5a750 100644 --- a/src/security/sudo.rs +++ b/src/security/sudo.rs @@ -25,9 +25,9 @@ //! - Environment variable usage is discouraged due to security risks use anyhow::Result; +use secrecy::{ExposeSecret, SecretString}; use std::fmt; -use std::sync::Arc; -use zeroize::ZeroizeOnDrop; +use zeroize::Zeroizing; /// Common sudo password prompt patterns across different distributions /// @@ -58,36 +58,39 @@ pub const SUDO_FAILURE_PATTERNS: &[&str] = &[ /// A secure wrapper for sudo passwords that automatically clears memory on drop. /// -/// This struct uses the `zeroize` crate to ensure the password is securely -/// cleared from memory when the struct is dropped, preventing sensitive data -/// from remaining in memory. +/// This struct uses the `secrecy` crate which is specifically designed for handling +/// secret values in memory. It ensures the password is securely cleared when dropped, +/// and works correctly with cloning (each clone is independent and properly zeroized). /// /// # Security /// - Password is automatically zeroized when dropped /// - Debug output does not reveal the password /// - Clone creates a new copy that is also zeroized independently -#[derive(Clone, ZeroizeOnDrop)] +/// - Works correctly in multi-threaded contexts (safe to share across tasks) +#[derive(Clone)] pub struct SudoPassword { - /// The actual password bytes - #[zeroize(skip)] // We handle the inner zeroization manually via Arc - inner: Arc, -} - -/// Inner container for the password with zeroize support -#[derive(ZeroizeOnDrop)] -struct SudoPasswordInner { - password: String, + /// The actual password stored securely + inner: SecretString, } impl SudoPassword { /// Create a new SudoPassword from a string. /// - /// The password will be automatically cleared from memory when all - /// references to this SudoPassword are dropped. - pub fn new(password: String) -> Self { - Self { - inner: Arc::new(SudoPasswordInner { password }), + /// The password will be automatically cleared from memory when dropped. + /// + /// # Arguments + /// * `password` - The password string (will be zeroized after conversion) + /// + /// # Returns + /// * `Ok(SudoPassword)` if the password is non-empty + /// * `Err` if the password is empty + pub fn new(password: String) -> Result { + if password.is_empty() { + anyhow::bail!("Password cannot be empty"); } + Ok(Self { + inner: SecretString::new(password.into_boxed_str()), + }) } /// Get the password as bytes for sending over SSH. @@ -95,21 +98,27 @@ impl SudoPassword { /// # Security Note /// The returned bytes should be used immediately and not stored. pub fn as_bytes(&self) -> &[u8] { - self.inner.password.as_bytes() + self.inner.expose_secret().as_bytes() } /// Get the password with a newline appended for sudo input. /// /// Sudo requires a newline after the password to submit it. - pub fn with_newline(&self) -> Vec { - let mut bytes = self.inner.password.as_bytes().to_vec(); + /// + /// # Security Note + /// Returns a `Zeroizing>` to ensure the copy is also cleared from memory. + pub fn with_newline(&self) -> Zeroizing> { + let mut bytes = self.inner.expose_secret().as_bytes().to_vec(); bytes.push(b'\n'); - bytes + Zeroizing::new(bytes) } /// Check if the password is empty. + /// + /// Note: This should always return false since empty passwords are rejected + /// during construction, but kept for API compatibility. pub fn is_empty(&self) -> bool { - self.inner.password.is_empty() + self.inner.expose_secret().is_empty() } } @@ -159,17 +168,23 @@ pub fn contains_sudo_failure(output: &str) -> bool { /// /// # Returns /// A `SudoPassword` containing the entered password, or an error if -/// reading fails. +/// reading fails or the password is empty. /// /// # Security Note /// - The password is never printed to stdout/stderr /// - The password is stored in a zeroizing container +/// - Empty passwords are rejected with a clear error message pub fn prompt_sudo_password() -> Result { eprintln!("Enter sudo password: "); let password = rpassword::read_password().map_err(|e| { anyhow::anyhow!("Failed to read sudo password: {}", e) })?; - Ok(SudoPassword::new(password)) + + if password.is_empty() { + anyhow::bail!("Empty password not allowed. Please enter a valid sudo password."); + } + + SudoPassword::new(password) } /// Get sudo password from environment variable (if set). @@ -181,12 +196,20 @@ pub fn prompt_sudo_password() -> Result { /// is acceptable. /// /// # Returns -/// `Some(SudoPassword)` if `BSSH_SUDO_PASSWORD` is set, `None` otherwise. -pub fn get_sudo_password_from_env() -> Option { - std::env::var("BSSH_SUDO_PASSWORD") - .ok() - .filter(|s| !s.is_empty()) - .map(SudoPassword::new) +/// * `Some(SudoPassword)` if `BSSH_SUDO_PASSWORD` is set and non-empty +/// * `None` if the environment variable is not set or empty +/// * `Err` if the password fails validation +pub fn get_sudo_password_from_env() -> Result> { + match std::env::var("BSSH_SUDO_PASSWORD") { + Ok(password) if !password.is_empty() => { + Ok(Some(SudoPassword::new(password)?)) + } + Ok(_) => { + // Empty password from environment + anyhow::bail!("BSSH_SUDO_PASSWORD is set but empty. Empty passwords are not allowed."); + } + Err(_) => Ok(None), + } } /// Get sudo password from either environment or interactive prompt. @@ -200,16 +223,17 @@ pub fn get_sudo_password_from_env() -> Option { /// # Returns /// A `SudoPassword` containing the password. pub fn get_sudo_password(warn_env: bool) -> Result { - if let Some(password) = get_sudo_password_from_env() { - if warn_env { - eprintln!( - "Warning: Using sudo password from BSSH_SUDO_PASSWORD environment variable. \ - This is not recommended for security reasons." - ); + match get_sudo_password_from_env()? { + Some(password) => { + if warn_env { + eprintln!( + "Warning: Using sudo password from BSSH_SUDO_PASSWORD environment variable. \ + This is not recommended for security reasons." + ); + } + Ok(password) } - Ok(password) - } else { - prompt_sudo_password() + None => prompt_sudo_password(), } } @@ -219,32 +243,45 @@ mod tests { #[test] fn test_sudo_password_creation() { - let password = SudoPassword::new("test123".to_string()); + let password = SudoPassword::new("test123".to_string()).unwrap(); assert_eq!(password.as_bytes(), b"test123"); assert!(!password.is_empty()); } + #[test] + fn test_sudo_password_empty_rejection() { + let result = SudoPassword::new(String::new()); + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("cannot be empty")); + } + #[test] fn test_sudo_password_with_newline() { - let password = SudoPassword::new("test123".to_string()); + let password = SudoPassword::new("test123".to_string()).unwrap(); + let with_newline = password.with_newline(); + assert_eq!(&*with_newline, b"test123\n"); + } + + #[test] + fn test_sudo_password_with_newline_is_zeroizing() { + let password = SudoPassword::new("test123".to_string()).unwrap(); let with_newline = password.with_newline(); - assert_eq!(with_newline, b"test123\n"); + // The type should be Zeroizing> + // When dropped, it will automatically clear memory + assert_eq!(&*with_newline, b"test123\n"); + drop(with_newline); + // After drop, memory should be cleared (we can't verify this in safe Rust, + // but the type system ensures it) } #[test] fn test_sudo_password_debug_redaction() { - let password = SudoPassword::new("secret".to_string()); + let password = SudoPassword::new("secret".to_string()).unwrap(); let debug_output = format!("{:?}", password); assert!(!debug_output.contains("secret")); assert!(debug_output.contains("[REDACTED]")); } - #[test] - fn test_empty_password() { - let password = SudoPassword::new(String::new()); - assert!(password.is_empty()); - } - #[test] fn test_contains_sudo_prompt() { // Standard sudo prompts @@ -276,11 +313,47 @@ mod tests { #[test] fn test_clone_independence() { - let password1 = SudoPassword::new("original".to_string()); + let password1 = SudoPassword::new("original".to_string()).unwrap(); let password2 = password1.clone(); // Both should work independently assert_eq!(password1.as_bytes(), b"original"); assert_eq!(password2.as_bytes(), b"original"); + + // When dropped, each will be zeroized independently + // (secrecy::SecretString handles this correctly) + } + + #[test] + fn test_get_sudo_password_from_env_empty() { + // Set environment variable to empty string + std::env::set_var("BSSH_SUDO_PASSWORD", ""); + let result = get_sudo_password_from_env(); + std::env::remove_var("BSSH_SUDO_PASSWORD"); + + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("empty")); + } + + #[test] + fn test_get_sudo_password_from_env_valid() { + // Set environment variable to valid password + std::env::set_var("BSSH_SUDO_PASSWORD", "test_password"); + let result = get_sudo_password_from_env(); + std::env::remove_var("BSSH_SUDO_PASSWORD"); + + assert!(result.is_ok()); + let password = result.unwrap(); + assert!(password.is_some()); + assert_eq!(password.unwrap().as_bytes(), b"test_password"); + } + + #[test] + fn test_get_sudo_password_from_env_not_set() { + std::env::remove_var("BSSH_SUDO_PASSWORD"); + let result = get_sudo_password_from_env(); + + assert!(result.is_ok()); + assert!(result.unwrap().is_none()); } } diff --git a/src/ssh/tokio_client/channel_manager.rs b/src/ssh/tokio_client/channel_manager.rs index 575f1bed..267a6188 100644 --- a/src/ssh/tokio_client/channel_manager.rs +++ b/src/ssh/tokio_client/channel_manager.rs @@ -49,6 +49,13 @@ use super::ToSocketAddrsWithHostname; /// - Commands producing more than 3.2MB/sec may experience throttling const OUTPUT_EVENTS_CHANNEL_SIZE: usize = 100; +/// Maximum buffer size for sudo prompt detection (64KB) +/// +/// This prevents unbounded memory growth when detecting sudo prompts in command output. +/// Sudo prompts are typically very short (<1KB), so 64KB is more than sufficient. +/// If output exceeds this size without a sudo prompt, we truncate to prevent memory issues. +const MAX_SUDO_PROMPT_BUFFER_SIZE: usize = 64 * 1024; + /// Command output variants for streaming #[derive(Debug, Clone)] pub enum CommandOutput { @@ -357,6 +364,18 @@ impl Client { let text = String::from_utf8_lossy(data); accumulated_output.push_str(&text); + // Enforce buffer size limit to prevent unbounded memory growth + if accumulated_output.len() > MAX_SUDO_PROMPT_BUFFER_SIZE { + // Keep only the last MAX_SUDO_PROMPT_BUFFER_SIZE bytes + // This ensures we can still detect sudo prompts at the end + let truncate_at = accumulated_output.len() - MAX_SUDO_PROMPT_BUFFER_SIZE; + accumulated_output = accumulated_output[truncate_at..].to_string(); + tracing::debug!( + "Sudo prompt buffer exceeded limit, truncated to {} bytes", + MAX_SUDO_PROMPT_BUFFER_SIZE + ); + } + // Send output to streaming channel match sender.try_send(CommandOutput::StdOut(data.clone())) { Ok(_) => {} @@ -399,6 +418,17 @@ impl Client { let text = String::from_utf8_lossy(data); accumulated_output.push_str(&text); + // Enforce buffer size limit to prevent unbounded memory growth + if accumulated_output.len() > MAX_SUDO_PROMPT_BUFFER_SIZE { + // Keep only the last MAX_SUDO_PROMPT_BUFFER_SIZE bytes + let truncate_at = accumulated_output.len() - MAX_SUDO_PROMPT_BUFFER_SIZE; + accumulated_output = accumulated_output[truncate_at..].to_string(); + tracing::debug!( + "Sudo prompt buffer exceeded limit (stderr), truncated to {} bytes", + MAX_SUDO_PROMPT_BUFFER_SIZE + ); + } + match sender.try_send(CommandOutput::StdErr(data.clone())) { Ok(_) => {} Err(tokio::sync::mpsc::error::TrySendError::Full(output)) => { From f0ac30a4a5f6af4e79368a66465f96518b1633fb Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Wed, 10 Dec 2025 19:18:30 +0900 Subject: [PATCH 03/13] test: Fix race condition in sudo password env tests Add #[serial] attribute to environment-related tests to prevent race conditions when tests modify the BSSH_SUDO_PASSWORD env var. Also ensure environment is clean before each test. --- src/security/sudo.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/security/sudo.rs b/src/security/sudo.rs index f5b5a750..c00d85d2 100644 --- a/src/security/sudo.rs +++ b/src/security/sudo.rs @@ -240,6 +240,7 @@ pub fn get_sudo_password(warn_env: bool) -> Result { #[cfg(test)] mod tests { use super::*; + use serial_test::serial; #[test] fn test_sudo_password_creation() { @@ -325,7 +326,10 @@ mod tests { } #[test] + #[serial] fn test_get_sudo_password_from_env_empty() { + // Ensure variable is not set from other tests + std::env::remove_var("BSSH_SUDO_PASSWORD"); // Set environment variable to empty string std::env::set_var("BSSH_SUDO_PASSWORD", ""); let result = get_sudo_password_from_env(); @@ -336,7 +340,10 @@ mod tests { } #[test] + #[serial] fn test_get_sudo_password_from_env_valid() { + // Ensure variable is not set from other tests + std::env::remove_var("BSSH_SUDO_PASSWORD"); // Set environment variable to valid password std::env::set_var("BSSH_SUDO_PASSWORD", "test_password"); let result = get_sudo_password_from_env(); @@ -349,6 +356,7 @@ mod tests { } #[test] + #[serial] fn test_get_sudo_password_from_env_not_set() { std::env::remove_var("BSSH_SUDO_PASSWORD"); let result = get_sudo_password_from_env(); From aaed45f85809f01cc5b82ca4ceaf2e27a143ea51 Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Wed, 10 Dec 2025 19:33:02 +0900 Subject: [PATCH 04/13] style: Apply rustfmt formatting to sudo password implementation --- src/security/sudo.rs | 9 +++------ src/ssh/client/command.rs | 24 ++++++++++++++++++++---- src/ssh/tokio_client/channel_manager.rs | 19 ++++++++++--------- 3 files changed, 33 insertions(+), 19 deletions(-) diff --git a/src/security/sudo.rs b/src/security/sudo.rs index c00d85d2..162b0268 100644 --- a/src/security/sudo.rs +++ b/src/security/sudo.rs @@ -176,9 +176,8 @@ pub fn contains_sudo_failure(output: &str) -> bool { /// - Empty passwords are rejected with a clear error message pub fn prompt_sudo_password() -> Result { eprintln!("Enter sudo password: "); - let password = rpassword::read_password().map_err(|e| { - anyhow::anyhow!("Failed to read sudo password: {}", e) - })?; + let password = rpassword::read_password() + .map_err(|e| anyhow::anyhow!("Failed to read sudo password: {}", e))?; if password.is_empty() { anyhow::bail!("Empty password not allowed. Please enter a valid sudo password."); @@ -201,9 +200,7 @@ pub fn prompt_sudo_password() -> Result { /// * `Err` if the password fails validation pub fn get_sudo_password_from_env() -> Result> { match std::env::var("BSSH_SUDO_PASSWORD") { - Ok(password) if !password.is_empty() => { - Ok(Some(SudoPassword::new(password)?)) - } + Ok(password) if !password.is_empty() => Ok(Some(SudoPassword::new(password)?)), Ok(_) => { // Empty password from environment anyhow::bail!("BSSH_SUDO_PASSWORD is set but empty. Empty passwords are not allowed."); diff --git a/src/ssh/client/command.rs b/src/ssh/client/command.rs index 9e7390db..7d46a530 100644 --- a/src/ssh/client/command.rs +++ b/src/ssh/client/command.rs @@ -288,7 +288,11 @@ impl SshClient { output_sender: Sender, sudo_password: &SudoPassword, ) -> Result { - tracing::debug!("Connecting to {}:{} for sudo execution", self.host, self.port); + tracing::debug!( + "Connecting to {}:{} for sudo execution", + self.host, + self.port + ); // Determine authentication method based on parameters let auth_method = self @@ -322,7 +326,13 @@ impl SshClient { // Execute command with sudo support and timeout let exit_status = self - .execute_sudo_with_timeout(&client, command, config.timeout_seconds, output_sender, sudo_password) + .execute_sudo_with_timeout( + &client, + command, + config.timeout_seconds, + output_sender, + sudo_password, + ) .await?; tracing::debug!("Command execution completed with status: {}", exit_status); @@ -343,9 +353,15 @@ impl SshClient { if timeout_secs == 0 { // No timeout (unlimited) tracing::debug!("Executing sudo command with no timeout (unlimited)"); - client.execute_with_sudo(command, output_sender, sudo_password) + client + .execute_with_sudo(command, output_sender, sudo_password) .await - .with_context(|| format!("Failed to execute sudo command '{}' on {}:{}", command, self.host, self.port)) + .with_context(|| { + format!( + "Failed to execute sudo command '{}' on {}:{}", + command, self.host, self.port + ) + }) } else { // With timeout let command_timeout = Duration::from_secs(timeout_secs); diff --git a/src/ssh/tokio_client/channel_manager.rs b/src/ssh/tokio_client/channel_manager.rs index 267a6188..38902d73 100644 --- a/src/ssh/tokio_client/channel_manager.rs +++ b/src/ssh/tokio_client/channel_manager.rs @@ -28,9 +28,9 @@ use std::net::SocketAddr; use tokio::sync::mpsc::{channel, Receiver, Sender}; use tokio::task::JoinHandle; -use crate::security::{contains_sudo_failure, contains_sudo_prompt, SudoPassword}; use super::connection::Client; use super::ToSocketAddrsWithHostname; +use crate::security::{contains_sudo_failure, contains_sudo_prompt, SudoPassword}; // Buffer size constants for SSH operations /// SSH I/O buffer size constants - optimized for different operation types @@ -340,13 +340,13 @@ impl Client { // Request PTY with reasonable defaults for sudo channel .request_pty( - true, // want reply - "xterm", // term type - 80, // columns - 24, // rows - 0, // pixel width - 0, // pixel height - &[], // terminal modes (empty for defaults) + true, // want reply + "xterm", // term type + 80, // columns + 24, // rows + 0, // pixel width + 0, // pixel height + &[], // terminal modes (empty for defaults) ) .await?; @@ -421,7 +421,8 @@ impl Client { // Enforce buffer size limit to prevent unbounded memory growth if accumulated_output.len() > MAX_SUDO_PROMPT_BUFFER_SIZE { // Keep only the last MAX_SUDO_PROMPT_BUFFER_SIZE bytes - let truncate_at = accumulated_output.len() - MAX_SUDO_PROMPT_BUFFER_SIZE; + let truncate_at = + accumulated_output.len() - MAX_SUDO_PROMPT_BUFFER_SIZE; accumulated_output = accumulated_output[truncate_at..].to_string(); tracing::debug!( "Sudo prompt buffer exceeded limit (stderr), truncated to {} bytes", From dc3ef48faa52992f621450b2d09193f705c112ca Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Wed, 10 Dec 2025 19:37:11 +0900 Subject: [PATCH 05/13] docs: Add sudo-password feature to CHANGELOG and manpage - Add [Unreleased] section to CHANGELOG with sudo password feature details - Add -S/--sudo-password option documentation to manpage - Add BSSH_SUDO_PASSWORD environment variable documentation - Add usage examples for sudo commands in manpage --- CHANGELOG.md | 12 +++++++++++ docs/man/bssh.1 | 54 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c8c8a679..1993c102 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,18 @@ All notable changes to bssh will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Added +- **Sudo Password Support** (Issue #74, PR #78) + - `-S/--sudo-password` flag for automated sudo authentication + - Securely prompts for sudo password before command execution + - Automatically detects and responds to sudo password prompts + - Works with both streaming and non-streaming execution modes + - `BSSH_SUDO_PASSWORD` environment variable support (with security warnings) + - Uses `secrecy` crate for secure memory handling + - Password cleared from memory immediately after use + ## [1.3.0] - 2025-12-10 ### Added diff --git a/docs/man/bssh.1 b/docs/man/bssh.1 index 134873ca..ce88e293 100644 --- a/docs/man/bssh.1 +++ b/docs/man/bssh.1 @@ -137,6 +137,33 @@ Use password authentication. When this option is specified, bssh will prompt for the password securely without echoing it to the terminal. This is useful for systems that don't have SSH keys configured. +.TP +.BR \-S ", " \-\-sudo\-password +Prompt for sudo password to automatically respond to sudo prompts. +When this option is specified, bssh will: +.RS +.IP \[bu] 2 +Securely prompt for sudo password before execution (no terminal echo) +.IP \[bu] 2 +Detect sudo password prompts in command output +.IP \[bu] 2 +Automatically inject the password when prompted +.RE +.IP +Alternatively, set the +.B BSSH_SUDO_PASSWORD +environment variable (not recommended for security reasons). +.IP +Security notes: +.RS +.IP \[bu] 2 +Password is stored using secure memory handling (secrecy crate) +.IP \[bu] 2 +Password is cleared from memory immediately after use +.IP \[bu] 2 +Password is never logged or printed in any output +.RE + .TP .BR \-f ", " \-\-filter " " \fIPATTERN\fR Filter hosts by pattern (supports wildcards like 'web*'). @@ -1066,6 +1093,20 @@ Use password authentication: Prompts for password interactively .RE +.TP +Execute sudo commands with automatic password injection: +.B bssh -S -C 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" +.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" @@ -1387,6 +1428,19 @@ attacks while allowing flexible jump host configurations. .br Example: BSSH_MAX_JUMP_HOSTS=20 bssh -J host1,host2,...,host20 target +.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. +.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" + .TP .B USER Used as default username when not specified From 6ffe71b60daf7fee721423f04f6a311fce259f49 Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Wed, 10 Dec 2025 19:53:33 +0900 Subject: [PATCH 06/13] feat: Support multiple sudo password prompts in single session - Change from single boolean flag to counter-based tracking - Allow up to 10 sudo password sends per session (MAX_SUDO_PASSWORD_SENDS) - Support commands like 'sudo cmd1 && sudo cmd2' when sudo cache is disabled - Stop sending passwords after authentication failure is detected - Add detailed debug logging with attempt count This handles scenarios where: - sudo -k explicitly invalidates the cache - Server has Defaults timestamp_timeout=0 - Long-running commands where sudo cache expires --- src/ssh/tokio_client/channel_manager.rs | 57 +++++++++++++++++++------ 1 file changed, 43 insertions(+), 14 deletions(-) diff --git a/src/ssh/tokio_client/channel_manager.rs b/src/ssh/tokio_client/channel_manager.rs index 38902d73..5a575791 100644 --- a/src/ssh/tokio_client/channel_manager.rs +++ b/src/ssh/tokio_client/channel_manager.rs @@ -56,6 +56,12 @@ const OUTPUT_EVENTS_CHANNEL_SIZE: usize = 100; /// If output exceeds this size without a sudo prompt, we truncate to prevent memory issues. const MAX_SUDO_PROMPT_BUFFER_SIZE: usize = 64 * 1024; +/// Maximum number of times to send sudo password in a single session. +/// This allows handling multiple sudo commands (e.g., `sudo cmd1 && sudo cmd2`) +/// while preventing infinite loops if authentication fails. +/// Set to 10 to support reasonable multi-sudo command chains. +const MAX_SUDO_PASSWORD_SENDS: u32 = 10; + /// Command output variants for streaming #[derive(Debug, Clone)] pub enum CommandOutput { @@ -353,7 +359,8 @@ impl Client { channel.exec(true, sanitized_command.as_str()).await?; let mut result: Option = None; - let mut password_sent = false; + let mut password_send_count: u32 = 0; + let mut sudo_auth_failed = false; let mut accumulated_output = String::new(); // While the channel has messages... @@ -392,24 +399,35 @@ impl Client { } } - // Check if we need to send the password - if !password_sent && contains_sudo_prompt(&accumulated_output) { - tracing::debug!("Sudo prompt detected, sending password"); + // Check if we need to send the password (supports multiple sudo prompts) + if !sudo_auth_failed + && password_send_count < MAX_SUDO_PASSWORD_SENDS + && contains_sudo_prompt(&accumulated_output) + { + password_send_count += 1; + tracing::debug!( + "Sudo prompt detected, sending password (attempt {}/{})", + password_send_count, + MAX_SUDO_PASSWORD_SENDS + ); // Send the password with newline let password_data = sudo_password.with_newline(); if let Err(e) = channel.data(&password_data[..]).await { tracing::error!("Failed to send sudo password: {}", e); return Err(super::Error::SshError(e)); } - password_sent = true; - // Clear accumulated output after sending password + // Clear accumulated output after sending password to detect next prompt accumulated_output.clear(); } // Check for sudo failure after password was sent - if password_sent && contains_sudo_failure(&accumulated_output) { - tracing::warn!("Sudo authentication failed"); - // Continue to collect output - the exit code will indicate failure + if password_send_count > 0 && contains_sudo_failure(&accumulated_output) { + tracing::warn!( + "Sudo authentication failed after {} attempt(s)", + password_send_count + ); + // Stop trying to send more passwords + sudo_auth_failed = true; } } russh::ChannelMsg::ExtendedData { ref data, ext } => { @@ -446,20 +464,31 @@ impl Client { } // Check if we need to send the password (sudo can prompt on stderr) - if !password_sent && contains_sudo_prompt(&accumulated_output) { - tracing::debug!("Sudo prompt detected on stderr, sending password"); + if !sudo_auth_failed + && password_send_count < MAX_SUDO_PASSWORD_SENDS + && contains_sudo_prompt(&accumulated_output) + { + password_send_count += 1; + tracing::debug!( + "Sudo prompt detected on stderr, sending password (attempt {}/{})", + password_send_count, + MAX_SUDO_PASSWORD_SENDS + ); let password_data = sudo_password.with_newline(); if let Err(e) = channel.data(&password_data[..]).await { tracing::error!("Failed to send sudo password: {}", e); return Err(super::Error::SshError(e)); } - password_sent = true; accumulated_output.clear(); } // Check for sudo failure - if password_sent && contains_sudo_failure(&accumulated_output) { - tracing::warn!("Sudo authentication failed (stderr)"); + if password_send_count > 0 && contains_sudo_failure(&accumulated_output) { + tracing::warn!( + "Sudo authentication failed on stderr after {} attempt(s)", + password_send_count + ); + sudo_auth_failed = true; } } } From e5c7f8ccaf5e873b217cd0ad3f00d412b97e8542 Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Wed, 10 Dec 2025 19:57:44 +0900 Subject: [PATCH 07/13] fix: Change sudo auth failure log level from warn to debug Prevents TUI layout corruption when sudo authentication fails. The warn-level logs were being output directly to terminal, interfering with the TUI display. Debug level ensures these messages only appear with -vv verbosity flag. --- src/ssh/tokio_client/channel_manager.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ssh/tokio_client/channel_manager.rs b/src/ssh/tokio_client/channel_manager.rs index 5a575791..9435cf51 100644 --- a/src/ssh/tokio_client/channel_manager.rs +++ b/src/ssh/tokio_client/channel_manager.rs @@ -422,7 +422,7 @@ impl Client { // Check for sudo failure after password was sent if password_send_count > 0 && contains_sudo_failure(&accumulated_output) { - tracing::warn!( + tracing::debug!( "Sudo authentication failed after {} attempt(s)", password_send_count ); @@ -484,7 +484,7 @@ impl Client { // Check for sudo failure if password_send_count > 0 && contains_sudo_failure(&accumulated_output) { - tracing::warn!( + tracing::debug!( "Sudo authentication failed on stderr after {} attempt(s)", password_send_count ); From 07c1ad31262a3e5977ac276286fd431c4b2c7bac Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Wed, 10 Dec 2025 20:01:40 +0900 Subject: [PATCH 08/13] fix: Properly handle sudo authentication failure - Close SSH channel immediately when sudo authentication fails - Return exit code 1 to indicate authentication failure - Display clear error message: '[bssh] Sudo authentication failed' - Change log level from warn to debug to prevent TUI corruption - TUI now returns TuiExitReason to indicate user quit vs completion - Abort pending handles when user explicitly quits (q key) This fixes: 1. TUI layout corruption from warn-level log messages 2. Unable to quit TUI when sudo fails (q key not working) 3. Node appearing stuck in 'in progress' state after sudo failure --- src/executor/parallel.rs | 16 ++++++--- src/ssh/tokio_client/channel_manager.rs | 43 +++++++++++++++++++------ src/ui/tui/mod.rs | 30 +++++++++++++---- 3 files changed, 68 insertions(+), 21 deletions(-) diff --git a/src/executor/parallel.rs b/src/executor/parallel.rs index cf743056..4d027718 100644 --- a/src/executor/parallel.rs +++ b/src/executor/parallel.rs @@ -751,13 +751,21 @@ impl ParallelExecutor { // Run TUI event loop - this will block until user quits or all complete // The TUI itself will handle polling the manager - if let Err(e) = tui::run_tui(manager, cluster_name, command).await { - tracing::error!("TUI error: {}", e); - } + let user_quit = match tui::run_tui(manager, cluster_name, command).await { + Ok(tui::TuiExitReason::UserQuit) => true, + Ok(tui::TuiExitReason::AllTasksCompleted) => false, + Err(e) => { + tracing::error!("TUI error: {}", e); + false + } + }; // Clean up any remaining handles + // If user explicitly quit, abort the handles instead of waiting for handle in pending_handles.drain(..) { - if let Err(e) = handle.await { + if user_quit { + handle.abort(); + } else if let Err(e) = handle.await { tracing::error!("Task error: {}", e); } } diff --git a/src/ssh/tokio_client/channel_manager.rs b/src/ssh/tokio_client/channel_manager.rs index 9435cf51..d8dbea26 100644 --- a/src/ssh/tokio_client/channel_manager.rs +++ b/src/ssh/tokio_client/channel_manager.rs @@ -360,7 +360,6 @@ impl Client { let mut result: Option = None; let mut password_send_count: u32 = 0; - let mut sudo_auth_failed = false; let mut accumulated_output = String::new(); // While the channel has messages... @@ -400,8 +399,7 @@ impl Client { } // Check if we need to send the password (supports multiple sudo prompts) - if !sudo_auth_failed - && password_send_count < MAX_SUDO_PASSWORD_SENDS + if password_send_count < MAX_SUDO_PASSWORD_SENDS && contains_sudo_prompt(&accumulated_output) { password_send_count += 1; @@ -423,11 +421,24 @@ impl Client { // Check for sudo failure after password was sent if password_send_count > 0 && contains_sudo_failure(&accumulated_output) { tracing::debug!( - "Sudo authentication failed after {} attempt(s)", + "Sudo authentication failed after {} attempt(s), closing channel", password_send_count ); - // Stop trying to send more passwords - sudo_auth_failed = true; + // Send error message to stderr so user can see why it failed + let error_msg = format!( + "\n[bssh] Sudo authentication failed after {} attempt(s). \ + Please verify your sudo password is correct.\n", + password_send_count + ); + let _ = sender + .send(CommandOutput::StdErr(CryptoVec::from(error_msg.as_bytes()))) + .await; + // Close the channel and return failure exit code + let _ = channel.eof().await; + let _ = channel.close().await; + drop(sender); + // Return exit code 1 to indicate sudo authentication failure + return Ok(1); } } russh::ChannelMsg::ExtendedData { ref data, ext } => { @@ -464,8 +475,7 @@ impl Client { } // Check if we need to send the password (sudo can prompt on stderr) - if !sudo_auth_failed - && password_send_count < MAX_SUDO_PASSWORD_SENDS + if password_send_count < MAX_SUDO_PASSWORD_SENDS && contains_sudo_prompt(&accumulated_output) { password_send_count += 1; @@ -485,10 +495,23 @@ impl Client { // Check for sudo failure if password_send_count > 0 && contains_sudo_failure(&accumulated_output) { tracing::debug!( - "Sudo authentication failed on stderr after {} attempt(s)", + "Sudo authentication failed on stderr after {} attempt(s), closing channel", + password_send_count + ); + // Send error message to stderr so user can see why it failed + let error_msg = format!( + "\n[bssh] Sudo authentication failed after {} attempt(s). \ + Please verify your sudo password is correct.\n", password_send_count ); - sudo_auth_failed = true; + let _ = sender + .send(CommandOutput::StdErr(CryptoVec::from(error_msg.as_bytes()))) + .await; + // Close the channel and return failure exit code + let _ = channel.eof().await; + let _ = channel.close().await; + drop(sender); + return Ok(1); } } } diff --git a/src/ui/tui/mod.rs b/src/ui/tui/mod.rs index 565b504f..c5e1f1dd 100644 --- a/src/ui/tui/mod.rs +++ b/src/ui/tui/mod.rs @@ -32,16 +32,27 @@ use std::io; use std::time::Duration; use terminal_guard::TerminalGuard; +/// Result of TUI execution +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum TuiExitReason { + /// User explicitly quit (pressed 'q' or Ctrl+C) + UserQuit, + /// All tasks completed naturally + AllTasksCompleted, +} + /// Run the TUI event loop /// /// This function sets up the terminal, runs the event loop, and cleans up /// on exit. It handles keyboard input and updates the display based on the /// current view mode. Terminal cleanup is guaranteed via RAII guards. +/// +/// Returns `TuiExitReason` to indicate whether the user quit or all tasks completed. pub async fn run_tui( manager: &mut MultiNodeStreamManager, cluster_name: &str, command: &str, -) -> Result<()> { +) -> Result { // Setup terminal with automatic cleanup guard let _terminal_guard = TerminalGuard::new()?; let backend = CrosstermBackend::new(io::stdout()); @@ -53,14 +64,15 @@ pub async fn run_tui( let mut app = TuiApp::new(); // Main event loop - let result = run_event_loop(&mut terminal, &mut app, manager, cluster_name, command).await; + let exit_reason = + run_event_loop(&mut terminal, &mut app, manager, cluster_name, command).await?; // Show cursor before exit (guard will handle the rest) terminal.show_cursor()?; // The terminal guard will automatically clean up when dropped - result + Ok(exit_reason) } /// Minimum terminal dimensions for TUI @@ -74,7 +86,7 @@ async fn run_event_loop( manager: &mut MultiNodeStreamManager, cluster_name: &str, command: &str, -) -> Result<()> { +) -> Result { // Force initial render app.mark_needs_redraw(); @@ -115,15 +127,19 @@ async fn run_event_loop( // Check exit condition (only quit when user explicitly requests) if app.should_quit { - break; + // Determine exit reason: user quit vs all tasks completed + let exit_reason = if app.all_tasks_completed { + TuiExitReason::AllTasksCompleted + } else { + TuiExitReason::UserQuit + }; + return Ok(exit_reason); } // Small delay to prevent CPU spinning // This is our main loop interval tokio::time::sleep(Duration::from_millis(50)).await; } - - Ok(()) } /// Render the UI based on current view mode From 9752330868c916322a61769c2100965a6b4c2643 Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Wed, 10 Dec 2025 20:07:51 +0900 Subject: [PATCH 09/13] fix: Mark sudo auth failure as Failed status in TUI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add ExitCode variant to CommandOutput enum for streaming exit codes - Send ExitCode(1) through channel when sudo authentication fails - Stream manager now processes exit code and sets Failed status when != 0 - TUI correctly shows ✗ (failed) instead of ✓ (completed) for sudo failures This ensures the TUI summary shows accurate success/failure counts. --- src/executor/connection_manager.rs | 3 +++ src/executor/stream_manager.rs | 24 +++++++++++++++++++++--- src/ssh/tokio_client/channel_manager.rs | 9 +++++++++ 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/src/executor/connection_manager.rs b/src/executor/connection_manager.rs index f7ee1468..3e573c16 100644 --- a/src/executor/connection_manager.rs +++ b/src/executor/connection_manager.rs @@ -79,6 +79,9 @@ pub(crate) async fn execute_on_node_with_jump_hosts( match output { CommandOutput::StdOut(data) => stdout.extend_from_slice(&data), CommandOutput::StdErr(data) => stderr.extend_from_slice(&data), + CommandOutput::ExitCode(_) => { + // Exit code is already captured from the function return value + } } } diff --git a/src/executor/stream_manager.rs b/src/executor/stream_manager.rs index 3344f473..01d527cd 100644 --- a/src/executor/stream_manager.rs +++ b/src/executor/stream_manager.rs @@ -170,6 +170,14 @@ impl NodeStream { ); } } + CommandOutput::ExitCode(code) => { + self.exit_code = Some(code); + tracing::debug!( + "Node {} received exit code: {}", + self.node.host, + code + ); + } } } Err(mpsc::error::TryRecvError::Empty) => { @@ -177,10 +185,20 @@ impl NodeStream { break; } Err(mpsc::error::TryRecvError::Disconnected) => { - // Channel closed - mark as completed if not already failed + // Channel closed - determine completion status based on exit code self.closed = true; - if self.status != ExecutionStatus::Failed(String::new()) { - self.status = ExecutionStatus::Completed; + // Only mark as completed/failed if not already set + if !matches!(self.status, ExecutionStatus::Failed(_)) { + if let Some(code) = self.exit_code { + if code != 0 { + self.status = + ExecutionStatus::Failed(format!("Exit code: {}", code)); + } else { + self.status = ExecutionStatus::Completed; + } + } else { + self.status = ExecutionStatus::Completed; + } } tracing::debug!("Channel disconnected for node {}", self.node.host); break; diff --git a/src/ssh/tokio_client/channel_manager.rs b/src/ssh/tokio_client/channel_manager.rs index d8dbea26..be17a4e7 100644 --- a/src/ssh/tokio_client/channel_manager.rs +++ b/src/ssh/tokio_client/channel_manager.rs @@ -69,6 +69,8 @@ pub enum CommandOutput { StdOut(CryptoVec), /// Standard error data StdErr(CryptoVec), + /// Exit code (sent when command completes) + ExitCode(u32), } /// Buffer for collecting streaming command output @@ -116,6 +118,9 @@ impl CommandOutputBuffer { } stderr.extend_from_slice(&buffer); } + CommandOutput::ExitCode(_) => { + // Exit code is handled by the stream manager, not collected here + } } } @@ -433,6 +438,8 @@ impl Client { let _ = sender .send(CommandOutput::StdErr(CryptoVec::from(error_msg.as_bytes()))) .await; + // Send exit code 1 to indicate failure to the stream + let _ = sender.send(CommandOutput::ExitCode(1)).await; // Close the channel and return failure exit code let _ = channel.eof().await; let _ = channel.close().await; @@ -507,6 +514,8 @@ impl Client { let _ = sender .send(CommandOutput::StdErr(CryptoVec::from(error_msg.as_bytes()))) .await; + // Send exit code 1 to indicate failure to the stream + let _ = sender.send(CommandOutput::ExitCode(1)).await; // Close the channel and return failure exit code let _ = channel.eof().await; let _ = channel.close().await; From 9effcdaf025a63121aa3ae72a2d4fdaca4e2a977 Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Wed, 10 Dec 2025 20:10:50 +0900 Subject: [PATCH 10/13] fix: Correct completed_count to exclude failed streams MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit completed_count() was using is_complete() which returns true for both Completed AND Failed status. This caused failed nodes to be counted in both success (✓) and failure (✗) columns. Now completed_count() only counts streams with ExecutionStatus::Completed, ensuring accurate TUI statistics: - ✓ = successfully completed (exit code 0) - ✗ = failed (exit code != 0 or error) - in progress = total - completed - failed --- src/executor/stream_manager.rs | 7 +++++-- src/ui/tui/views/summary.rs | 5 ++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/executor/stream_manager.rs b/src/executor/stream_manager.rs index 01d527cd..4f67c6b8 100644 --- a/src/executor/stream_manager.rs +++ b/src/executor/stream_manager.rs @@ -316,9 +316,12 @@ impl MultiNodeStreamManager { !self.streams.is_empty() && self.streams.iter().all(|s| s.is_complete()) } - /// Get count of completed streams + /// Get count of successfully completed streams (not failed) pub fn completed_count(&self) -> usize { - self.streams.iter().filter(|s| s.is_complete()).count() + self.streams + .iter() + .filter(|s| matches!(s.status(), ExecutionStatus::Completed) && s.is_closed()) + .count() } /// Get count of failed streams diff --git a/src/ui/tui/views/summary.rs b/src/ui/tui/views/summary.rs index 49cd5e77..992b82c8 100644 --- a/src/ui/tui/views/summary.rs +++ b/src/ui/tui/views/summary.rs @@ -60,11 +60,10 @@ fn render_header( let title = format!(" Cluster: {cluster_name} ({total} nodes) - {command} "); + let in_progress = total - completed - failed; let status = format!( " ✓ {} • ✗ {} • {} in progress ", - completed, - failed, - total - completed + completed, failed, in_progress ); let header_text = vec![Line::from(vec![ From b7a340015587446b79595eb43272c2629d80a519 Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Wed, 10 Dec 2025 20:12:25 +0900 Subject: [PATCH 11/13] ui: Show total count in TUI status bar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change status bar from '✓ N • ✗ N • N in progress' to 'Total: N • ✓ N • ✗ N • N in progress' for clarity. Also remove redundant '(N nodes)' from title since total is now shown. --- src/ui/tui/views/summary.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ui/tui/views/summary.rs b/src/ui/tui/views/summary.rs index 992b82c8..d5ce2369 100644 --- a/src/ui/tui/views/summary.rs +++ b/src/ui/tui/views/summary.rs @@ -58,12 +58,12 @@ fn render_header( let completed = manager.completed_count(); let failed = manager.failed_count(); - let title = format!(" Cluster: {cluster_name} ({total} nodes) - {command} "); + let title = format!(" Cluster: {cluster_name} - {command} "); let in_progress = total - completed - failed; let status = format!( - " ✓ {} • ✗ {} • {} in progress ", - completed, failed, in_progress + " Total: {} • ✓ {} • ✗ {} • {} in progress ", + total, completed, failed, in_progress ); let header_text = vec![Line::from(vec![ From 28364c541788fb8fcd673a35858133d9ae3ea186 Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Wed, 10 Dec 2025 20:16:47 +0900 Subject: [PATCH 12/13] fix: Handle ExitCode variant in streaming test --- tests/streaming_test.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/streaming_test.rs b/tests/streaming_test.rs index 818f4a3b..14b2a229 100644 --- a/tests/streaming_test.rs +++ b/tests/streaming_test.rs @@ -33,6 +33,9 @@ fn build_test_output_buffer() -> OutputBuffer { match output { CommandOutput::StdOut(buffer) => stdout.extend_from_slice(&buffer), CommandOutput::StdErr(buffer) => stderr.extend_from_slice(&buffer), + CommandOutput::ExitCode(_) => { + // Exit code is handled separately, not collected here + } } } From 002dcab8954924bd3622a7cb5f4c06163e21eeaf Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Wed, 10 Dec 2025 20:20:07 +0900 Subject: [PATCH 13/13] style: Apply rustfmt formatting to stream_manager --- src/executor/stream_manager.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/executor/stream_manager.rs b/src/executor/stream_manager.rs index 4f67c6b8..cc48e0b6 100644 --- a/src/executor/stream_manager.rs +++ b/src/executor/stream_manager.rs @@ -172,11 +172,7 @@ impl NodeStream { } CommandOutput::ExitCode(code) => { self.exit_code = Some(code); - tracing::debug!( - "Node {} received exit code: {}", - self.node.host, - code - ); + tracing::debug!("Node {} received exit code: {}", self.node.host, code); } } }