Improve error reporting on sudo execution error

Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>

Author: didier-wenzek

crates/common/tedge_config/src/lib.rs

@@ -1,5 +1,6 @@
 mod sudo;
 pub use sudo::SudoCommandBuilder;
+pub use sudo::SudoError;
 pub mod cli;
 mod system_toml;
 pub use system_toml::*;

crates/common/tedge_config/src/sudo.rs

@@ -1,5 +1,6 @@
 use std::ffi::OsStr;
 use std::process::Command;
+use std::process::Stdio;
 use std::sync::Arc;
 use tracing::warn;
 
@@ -65,4 +66,69 @@ impl SudoCommandBuilder {
             }
         }
     }
+
+    /// Check that the command can be executed using sudo
+    pub fn command_is_executable<S: AsRef<OsStr>>(
+        &self,
+        program: &impl AsRef<OsStr>,
+        args: &Vec<S>,
+    ) -> Result<(), SudoError> {
+        let status = self
+            .command(program)
+            .args(args)
+            .stdout(Stdio::null())
+            .stderr(Stdio::null())
+            .status();
+        match status {
+            Ok(status) if status.success() => Ok(()),
+            Ok(status) => match status.code() {
+                Some(exit_code) => {
+                    if self.command_is_sudo_enabled(program, args) {
+                        Err(SudoError::CannotSudo)
+                    } else {
+                        Err(SudoError::ExecutionFailed(exit_code))
+                    }
+                }
+                None => Err(SudoError::ExecutionInterrupted),
+            },
+            Err(err) => Err(SudoError::CannotExecute(err)),
+        }
+    }
+
+    fn command_is_sudo_enabled<S: AsRef<OsStr>>(
+        &self,
+        program: &impl AsRef<OsStr>,
+        args: &Vec<S>,
+    ) -> bool {
+        if !self.enabled {
+            return false;
+        }
+        let Ok(sudo) = which::which(self.sudo_program.as_ref()) else {
+            return false;
+        };
+        let status = Command::new(sudo)
+            .arg("-n")
+            .arg("--list")
+            .arg(program)
+            .args(args)
+            .stdout(Stdio::null())
+            .stderr(Stdio::null())
+            .status();
+        matches!(status, Ok(status) if status.success())
+    }
+}
+
+#[derive(thiserror::Error, Debug)]
+pub enum SudoError {
+    #[error("The user has not been authorized to run the command with sudo")]
+    CannotSudo,
+
+    #[error(transparent)]
+    CannotExecute(#[from] std::io::Error),
+
+    #[error("The command returned a non-zero exit code: {0}")]
+    ExecutionFailed(i32),
+
+    #[error("The command has been interrupted by a signal")]
+    ExecutionInterrupted,
 }

crates/core/plugin_sm/src/plugin.rs

@@ -138,7 +138,7 @@ pub trait Plugin {
         if failed_updates.is_empty() {
             let outcome = self.update_list(&updates, command_log.as_deref_mut()).await;
             if let Err(err @ SoftwareError::UpdateListNotSupported(_)) = outcome {
-                info!("{err}");
+                info!(target: "SM plugins", "{err}");
                 for update in updates.iter() {
                     if let Err(error) = self
                         .apply(update, command_log.as_deref_mut(), download_path)
@@ -228,7 +228,7 @@ pub trait Plugin {
                     url: url.url().to_string(),
                 })
         {
-            error!("Download error: {err:#?}");
+            error!(target: "SM plugins", "Download error: {err:#?}");
             if let Some(ref mut logger) = command_log {
                 logger
                     .write(format!("error: {}\n", &err).as_bytes())

crates/core/plugin_sm/src/plugin_manager.rs

@@ -5,11 +5,9 @@ use camino::Utf8PathBuf;
 use std::borrow::Cow;
 use std::collections::BTreeMap;
 use std::fs;
-use std::io::ErrorKind;
 use std::io::{self};
 use std::path::Path;
 use std::path::PathBuf;
-use std::process::Stdio;
 use tedge_api::commands::CommandStatus;
 use tedge_api::commands::SoftwareListCommand;
 use tedge_api::commands::SoftwareUpdateCommand;
@@ -18,6 +16,7 @@ use tedge_api::SoftwareError;
 use tedge_api::SoftwareType;
 use tedge_api::DEFAULT;
 use tedge_config::SudoCommandBuilder;
+use tedge_config::SudoError;
 use tracing::error;
 use tracing::info;
 use tracing::warn;
@@ -106,7 +105,7 @@ impl ExternalPlugins {
             config_dir,
         };
         if let Err(e) = plugins.load().await {
-            warn!(
+            warn!(target: "SM plugins",
                 "Reading the plugins directory ({:?}): failed with: {e:?}",
                 &plugins.plugin_dir
             );
@@ -119,15 +118,15 @@ impl ExternalPlugins {
                     .by_software_type(default_plugin_type.as_str())
                     .is_none()
                 {
-                    warn!(
+                    warn!(target: "SM plugins",
                         "The configured default plugin: {} not found",
                         default_plugin_type
                     );
                 }
-                info!("Default plugin type: {}", default_plugin_type)
+                info!(target: "SM plugins", "Default plugin type: {}", default_plugin_type)
             }
             None => {
-                info!("Default plugin type: Not configured")
+                info!(target: "SM plugins", "Default plugin type: Not configured")
             }
         }
 
@@ -145,42 +144,28 @@ impl ExternalPlugins {
             let entry = maybe_entry?;
             let path = entry.path();
             if path.is_file() {
-                let mut command = self.sudo.command(&path);
-
-                match command
-                    .arg(LIST)
-                    .stdout(Stdio::null())
-                    .stderr(Stdio::null())
-                    .status()
-                {
-                    Ok(code) if code.success() => {
-                        info!("Plugin activated: {}", path.display());
+                match self.sudo.command_is_executable(&path, &vec![LIST]) {
+                    Ok(()) => {
+                        info!(target: "SM plugins", "Plugin activated: {}", path.display());
                     }
-
-                    // If the file is not executable or returned non 0 status code we assume it is not a valid and skip further processing.
-                    Ok(_) => {
-                        error!(
-                            "File {} in plugin directory does not support list operation and may not be a valid plugin, skipping.",
+                    Err(SudoError::CannotSudo) => {
+                        error!(target: "SM plugins",
+                            "Skipping {}: not properly configured to run with sudo",
                             path.display()
                         );
                         continue;
                     }
-
-                    Err(err) if err.kind() == ErrorKind::PermissionDenied => {
-                        error!(
-                            "File {} Permission Denied, is the file an executable?\n
-                            The file will not be registered as a plugin.",
+                    Err(SudoError::ExecutionFailed(_)) => {
+                        error!(target: "SM plugins",
+                            "Skipping {}: does not support list operation and may not be a valid plugin",
                             path.display()
                         );
                         continue;
                     }
-
                     Err(err) => {
-                        error!(
-                            "An error occurred while trying to run: {}: {}\n
-                            The file will not be registered as a plugin.",
-                            path.display(),
-                            err
+                        error!(target: "SM plugins",
+                            "Skipping {}: can not be launched as a plugin: {err}",
+                            path.display()
                         );
                         continue;
                     }