diff --git a/crates/osmodifier/src/users.rs b/crates/osmodifier/src/users.rs index 68fda4ab8..1b2222d00 100644 --- a/crates/osmodifier/src/users.rs +++ b/crates/osmodifier/src/users.rs @@ -3,7 +3,7 @@ //! User management — create/update users, passwords, SSH keys, groups. -use std::{fs, io::Write, os::unix::fs::PermissionsExt, path::Path, process::Command}; +use std::{fs, os::unix::fs::PermissionsExt, path::Path}; use anyhow::{bail, Context, Error}; use log::{debug, info}; @@ -164,40 +164,19 @@ fn validate_shadow_value(value: &str) -> Result<(), Error> { } fn hash_password(plaintext: &str) -> Result { - // Use Dependency::Openssl to resolve the binary path for consistent - // detection, but use std::process::Command for stdin piping which - // the Dependency Command wrapper doesn't yet support. - let openssl_path = Dependency::Openssl - .path() - .context("openssl is required for password hashing")?; - - let mut child = Command::new(openssl_path) - .args(["passwd", "-6", "-stdin"]) - .stdin(std::process::Stdio::piped()) - .stdout(std::process::Stdio::piped()) - .stderr(std::process::Stdio::piped()) - .spawn() - .context("Failed to start openssl passwd")?; - - if let Some(mut stdin) = child.stdin.take() { - stdin - .write_all(plaintext.as_bytes()) - .context("Failed to write password to openssl stdin")?; - } + let raw = Dependency::Openssl + .cmd() + .with_arg("passwd") + .with_arg("-6") + .with_arg("-stdin") + .with_stdin(plaintext.as_bytes()) + .raw_output_and_check() + .context("Failed to hash password with openssl")?; - let output = child - .wait_with_output() - .context("Failed to wait for openssl passwd")?; + let stdout = + String::from_utf8(raw.stdout).context("openssl passwd produced non-UTF-8 output")?; - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - bail!("openssl passwd failed: {stderr}"); - } - - Ok(String::from_utf8(output.stdout) - .context("openssl passwd produced non-UTF-8 output")? - .trim() - .to_string()) + Ok(stdout.trim().to_string()) } fn create_user(user: &MICUser) -> Result<(), Error> { @@ -266,38 +245,16 @@ fn update_user_password(ctx: &OsModifierContext, username: &str, hash: &str) -> /// Set password on a newly created user via chpasswd -e (stdin), avoiding /// leaking the hash through /proc/cmdline. fn set_password_via_chpasswd(username: &str, hash: &str) -> Result<(), Error> { - // Use Dependency::Chpasswd to resolve the binary path for consistent - // detection, but use std::process::Command for stdin piping which - // the Dependency Command wrapper doesn't yet support. - let chpasswd_path = Dependency::Chpasswd - .path() - .context("chpasswd is required for setting user passwords")?; - debug!("Setting password for new user '{username}' via chpasswd"); let input = format!("{username}:{hash}\n"); - let mut child = Command::new(chpasswd_path) - .arg("-e") - .stdin(std::process::Stdio::piped()) - .stdout(std::process::Stdio::piped()) - .stderr(std::process::Stdio::piped()) - .spawn() - .context("Failed to start chpasswd")?; - - if let Some(mut stdin) = child.stdin.take() { - stdin - .write_all(input.as_bytes()) - .context("Failed to write to chpasswd stdin")?; - } - - let output = child - .wait_with_output() - .context("Failed to wait for chpasswd")?; + Dependency::Chpasswd + .cmd() + .with_arg("-e") + .with_stdin(input.into_bytes()) + .run_and_check() + .with_context(|| format!("Failed to set password for '{username}' via chpasswd"))?; - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - bail!("chpasswd failed for '{username}': {stderr}"); - } Ok(()) } diff --git a/crates/osutils/src/dependencies.rs b/crates/osutils/src/dependencies.rs index e5e36c6ee..9d683890f 100644 --- a/crates/osutils/src/dependencies.rs +++ b/crates/osutils/src/dependencies.rs @@ -1,10 +1,10 @@ use std::{ borrow::Cow, ffi::{OsStr, OsString}, - io, + io::{self, Write}, os::unix::process::ExitStatusExt, path::PathBuf, - process::{Command as StdCommand, Output}, + process::{Command as StdCommand, Output, Stdio}, }; use log::trace; @@ -153,6 +153,8 @@ pub enum Dependency { #[cfg(test)] DoesNotExist, #[cfg(test)] + Cat, + #[cfg(test)] Echo, #[cfg(test)] False, @@ -207,6 +209,7 @@ impl Dependency { dependency: *self, args: vec![], envs: vec![], + stdin_data: None, } } } @@ -215,6 +218,7 @@ pub struct Command { dependency: Dependency, args: Vec, envs: Vec<(OsString, OsString)>, + stdin_data: Option>, } impl Command { @@ -249,6 +253,18 @@ impl Command { self } + /// Set data to pipe to the command's stdin. + pub fn stdin(&mut self, data: impl Into>) -> &mut Self { + self.stdin_data = Some(data.into()); + self + } + + /// Owned-builder variant of [`stdin`](Self::stdin). + pub fn with_stdin(mut self, data: impl Into>) -> Self { + self.stdin(data); + self + } + pub fn envs(&mut self, vars: I) -> &mut Command where I: IntoIterator, @@ -275,7 +291,7 @@ impl Command { } fn render_command(&self) -> String { - if self.args.is_empty() { + let base = if self.args.is_empty() { self.dependency.to_string() } else { format!( @@ -292,6 +308,11 @@ impl Command { .collect::>() .join(" "), ) + }; + if self.stdin_data.is_some() { + format!("{base} (with stdin)") + } else { + base } } @@ -301,12 +322,70 @@ impl Command { cmd.envs(self.envs.clone()); let rendered_command = self.render_command(); trace!("Executing '{rendered_command}'"); - let output = cmd - .output() - .map_err(|inner| DependencyError::CouldNotExecute { - dependency: self.dependency, - inner, - })?; + + let output = if let Some(ref stdin_data) = self.stdin_data { + cmd.stdin(Stdio::piped()); + cmd.stdout(Stdio::piped()); + cmd.stderr(Stdio::piped()); + + let mut child = cmd + .spawn() + .map_err(|inner| DependencyError::CouldNotExecute { + dependency: self.dependency, + inner, + })?; + + // Write stdin on a scoped thread to avoid deadlock if the child + // fills its stdout/stderr pipe before consuming all input. + // Using thread::scope lets us borrow stdin_data (no clone) and + // ensures panics are surfaced rather than silently swallowed. + let child_stdin = child.stdin.take(); + let dep = self.dependency; + + std::thread::scope(|s| { + let writer = s.spawn(|| -> io::Result<()> { + if let Some(mut handle) = child_stdin { + handle.write_all(stdin_data)?; + // handle is dropped here, closing stdin so the child sees EOF + } + Ok(()) + }); + + let output = + child + .wait_with_output() + .map_err(|inner| DependencyError::CouldNotExecute { + dependency: dep, + inner, + })?; + + // Collect stdin write result. A panic in the writer thread will + // be re-raised here. BrokenPipe is not fatal — the child may + // have exited early, and its exit status / stderr carries the + // real diagnostic. + match writer.join().expect("stdin writer thread panicked") { + Ok(()) => {} + Err(write_err) => { + if write_err.kind() != io::ErrorKind::BrokenPipe && output.status.success() + { + return Err(Box::new(DependencyError::CouldNotExecute { + dependency: dep, + inner: write_err, + })); + } + } + } + + Ok(output) + })? + } else { + cmd.output() + .map_err(|inner| DependencyError::CouldNotExecute { + dependency: self.dependency, + inner, + })? + }; + let output = CommandOutput { rendered_command: rendered_command.clone(), dependency: self.dependency, @@ -500,4 +579,64 @@ mod tests { )); assert_eq!(output.explain_exit(), "exited with status: 1"); } + + #[test] + fn test_stdin_piped_to_cat() { + let output = Dependency::Cat + .cmd() + .with_stdin(b"hello from stdin".to_vec()) + .output_and_check() + .unwrap(); + assert_eq!(output, "hello from stdin"); + } + + #[test] + fn test_stdin_empty() { + let output = Dependency::Cat + .cmd() + .with_stdin(Vec::new()) + .output_and_check() + .unwrap(); + assert_eq!(output, ""); + } + + #[test] + fn test_stdin_with_failing_command() { + let result = Dependency::False + .cmd() + .with_stdin(b"ignored".to_vec()) + .output() + .unwrap(); + assert!(!result.success()); + assert!(matches!( + *result.check().unwrap_err(), + DependencyError::ExecutionFailed { .. } + )); + } + + #[test] + fn test_render_command_with_stdin() { + let cmd = Dependency::Cat.cmd().with_stdin(b"secret".to_vec()); + assert_eq!(cmd.render_command(), "cat (with stdin)"); + } + + #[test] + fn test_render_command_with_args_and_stdin() { + let cmd = Dependency::Echo + .cmd() + .with_arg("-n") + .with_stdin(b"data".to_vec()); + assert_eq!(cmd.render_command(), "echo -n (with stdin)"); + } + + #[test] + fn test_stdin_replaces_previous() { + let output = Dependency::Cat + .cmd() + .with_stdin(b"first".to_vec()) + .with_stdin(b"second".to_vec()) + .output_and_check() + .unwrap(); + assert_eq!(output, "second"); + } }