fix(sudo): handle root user case

Author: art049

src/run/runner/helpers/run_with_sudo.rs

@@ -4,6 +4,16 @@ use std::{
     process::{Command, Stdio},
 };
 
+fn is_sudo_available() -> bool {
+    Command::new("sudo")
+        .arg("--version")
+        .stdout(Stdio::null())
+        .stderr(Stdio::null())
+        .status()
+        .map(|status| status.success())
+        .unwrap_or(false)
+}
+
 /// Validate sudo access, prompting the user for their password if necessary
 fn validate_sudo_access() -> Result<()> {
     let needs_password = IsTerminal::is_terminal(&std::io::stdout())
@@ -37,32 +47,48 @@ fn validate_sudo_access() -> Result<()> {
             Ok(())
         })?;
     }
+    debug!("Sudo access validated");
     Ok(())
 }
 
 /// Creates the base sudo command after validating sudo access
-pub fn validated_sudo_command() -> Result<Command> {
+fn validated_sudo_command() -> Result<Command> {
     validate_sudo_access()?;
     let mut cmd = Command::new("sudo");
     // Password prompt should not appear here since it has already been validated
     cmd.arg("--non-interactive");
     Ok(cmd)
 }
 
-/// Run a command with sudo after validating sudo access
+/// Build a command wrapped with sudo if possible
+pub fn build_command_with_sudo(command_args: &[&str]) -> Result<Command> {
+    let command_str = command_args.join(" ");
+    if is_sudo_available() {
+        debug!("Sudo is required for command: {command_str}");
+        let mut c = validated_sudo_command()?;
+        c.args(command_args);
+        Ok(c)
+    } else {
+        debug!("Running command without sudo: {command_str}");
+        let mut c = Command::new(command_args[0]);
+        c.args(&command_args[1..]);
+        Ok(c)
+    }
+}
+
+/// Run a command with sudo after validating sudo access (if possible)
 pub fn run_with_sudo(command_args: &[&str]) -> Result<()> {
+    let mut cmd = build_command_with_sudo(command_args)?;
     let command_str = command_args.join(" ");
-    debug!("Running command with sudo: {command_str}");
-    let output = validated_sudo_command()?
-        .args(command_args)
+    let output = cmd
         .stdout(Stdio::piped())
         .output()
-        .map_err(|_| anyhow!("Failed to execute command with sudo: {command_str}"))?;
+        .map_err(|_| anyhow!("Failed to execute: {command_str}"))?;
 
     if !output.status.success() {
         info!("stdout: {}", String::from_utf8_lossy(&output.stdout));
         error!("stderr: {}", String::from_utf8_lossy(&output.stderr));
-        bail!("Failed to execute command with sudo: {command_str}");
+        bail!("Failed to execute command: {command_str}");
     }
 
     Ok(())

src/run/runner/wall_time/executor.rs

@@ -8,7 +8,7 @@ use crate::run::runner::helpers::get_bench_command::get_bench_command;
 use crate::run::runner::helpers::introspected_golang;
 use crate::run::runner::helpers::introspected_nodejs;
 use crate::run::runner::helpers::run_command_with_log_pipe::run_command_with_log_pipe;
-use crate::run::runner::helpers::run_with_sudo::validated_sudo_command;
+use crate::run::runner::helpers::run_with_sudo::build_command_with_sudo;
 use crate::run::runner::{ExecutorName, RunData};
 use crate::run::{check_system::SystemInfo, config::Config};
 use async_trait::async_trait;
@@ -100,7 +100,7 @@ impl WallTimeExecutor {
     fn walltime_bench_cmd(
         config: &Config,
         run_data: &RunData,
-    ) -> Result<(NamedTempFile, NamedTempFile, String)> {
+    ) -> Result<(NamedTempFile, NamedTempFile, Vec<String>)> {
         let bench_cmd = get_bench_command(config)?;
 
         let system_env = get_exported_system_env()?;
@@ -150,11 +150,24 @@ impl WallTimeExecutor {
         // - We have to pass the environment variables because `--scope` only inherits the system and not the user environment variables.
         let uid = nix::unistd::Uid::current().as_raw();
         let gid = nix::unistd::Gid::current().as_raw();
-        let cmd = format!(
-            "systemd-run {quiet_flag} --scope --slice=codspeed.slice --same-dir --uid={uid} --gid={gid} -- bash {}",
-            script_file.path().display()
-        );
-        Ok((env_file, script_file, cmd))
+        let script_path = script_file.path().to_string_lossy().to_string();
+        let cmd = [
+            "systemd-run",
+            quiet_flag,
+            "--scope",
+            "--slice=codspeed.slice",
+            "--same-dir",
+            &format!("--uid={uid}"),
+            &format!("--gid={gid}"),
+            "--",
+            "bash",
+            &script_path,
+        ];
+        Ok((
+            env_file,
+            script_file,
+            cmd.into_iter().map(|s| s.to_string()).collect(),
+        ))
     }
 }
 
@@ -179,25 +192,24 @@ impl Executor for WallTimeExecutor {
         run_data: &RunData,
         _mongo_tracer: &Option<MongoTracer>,
     ) -> Result<()> {
-        let mut cmd = validated_sudo_command()?;
-
-        if let Some(cwd) = &config.working_directory {
-            let abs_cwd = canonicalize(cwd)?;
-            cmd.current_dir(abs_cwd);
-        }
-
         let status = {
             let _guard = HookScriptsGuard::setup();
 
+            // Keep the temporary files alive for the duration of the command execution
             let (_env_file, _script_file, bench_cmd) = Self::walltime_bench_cmd(config, run_data)?;
             if let Some(perf) = &self.perf
                 && config.enable_perf
             {
-                perf.run(cmd, &bench_cmd, config).await
+                perf.run(bench_cmd, config).await
             } else {
-                cmd.args(["sh", "-c", &bench_cmd]);
+                let mut cmd = build_command_with_sudo(
+                    &bench_cmd.iter().map(|s| s.as_str()).collect::<Vec<_>>(),
+                )?;
                 debug!("cmd: {cmd:?}");
-
+                if let Some(cwd) = &config.working_directory {
+                    let abs_cwd = canonicalize(cwd)?;
+                    cmd.current_dir(abs_cwd);
+                }
                 run_command_with_log_pipe(cmd).await
             }
         };

src/run/runner/wall_time/perf/mod.rs

@@ -5,6 +5,7 @@ use crate::run::UnwindingMode;
 use crate::run::config::Config;
 use crate::run::runner::helpers::env::is_codspeed_debug_enabled;
 use crate::run::runner::helpers::run_command_with_log_pipe::run_command_with_log_pipe_and_callback;
+use crate::run::runner::helpers::run_with_sudo::build_command_with_sudo;
 use crate::run::runner::helpers::run_with_sudo::run_with_sudo;
 use crate::run::runner::valgrind::helpers::ignored_objects_path::get_objects_path_to_ignore;
 use crate::run::runner::valgrind::helpers::perf_maps::harvest_perf_maps_for_pids;
@@ -24,8 +25,8 @@ use runner_shared::fifo::MarkerType;
 use runner_shared::metadata::PerfMetadata;
 use runner_shared::unwind_data::UnwindData;
 use std::collections::HashSet;
+use std::fs::canonicalize;
 use std::path::{Path, PathBuf};
-use std::process::Command;
 use std::time::Duration;
 use std::{cell::OnceCell, collections::HashMap, process::ExitStatus};
 
@@ -84,12 +85,7 @@ impl PerfRunner {
         }
     }
 
-    pub async fn run(
-        &self,
-        mut cmd: Command,
-        bench_cmd: &str,
-        config: &Config,
-    ) -> anyhow::Result<ExitStatus> {
+    pub async fn run(&self, bench_cmd: Vec<String>, config: &Config) -> anyhow::Result<ExitStatus> {
         let perf_fifo = PerfFifo::new()?;
         let runner_fifo = RunnerFifo::new()?;
 
@@ -111,11 +107,11 @@ impl PerfRunner {
             (UnwindingMode::Dwarf, None)
         };
 
-        let cg_mode = match cg_mode {
-            UnwindingMode::FramePointer => "fp",
-            UnwindingMode::Dwarf => &format!("dwarf,{}", stack_size.unwrap_or(8192)),
+        let cg_mode_str = match cg_mode {
+            UnwindingMode::FramePointer => "fp".to_string(),
+            UnwindingMode::Dwarf => format!("dwarf,{}", stack_size.unwrap_or(8192)),
         };
-        debug!("Using call graph mode: {cg_mode:?}");
+        debug!("Using call graph mode: {cg_mode_str:?}");
 
         let quiet_flag = if !is_codspeed_debug_enabled() {
             "--quiet"
@@ -126,6 +122,7 @@ impl PerfRunner {
         let working_perf_executable =
             get_working_perf_executable().context("Failed to find a working perf executable")?;
 
+        let bench_str = bench_cmd.join(" ");
         let perf_args = [
             working_perf_executable.as_str(),
             "record",
@@ -138,20 +135,23 @@ impl PerfRunner {
             "--delay=-1",
             "-g",
             "--user-callchains",
-            &format!("--call-graph={cg_mode}"),
+            &format!("--call-graph={cg_mode_str}"),
             &format!(
                 "--control=fifo:{},{}",
                 perf_fifo.ctl_fifo_path.to_string_lossy(),
                 perf_fifo.ack_fifo_path.to_string_lossy()
             ),
             &format!("--output={PERF_DATA_PATH}"),
             "--",
-            bench_cmd,
+            &bench_str,
         ];
-
-        cmd.args(["sh", "-c", &perf_args.join(" ")]);
-        debug!("cmd: {cmd:?}");
-
+        let perf_cmd = perf_args.join(" ");
+        debug!("raw perf cmd: {perf_cmd:?}");
+        let mut cmd = build_command_with_sudo(&["sh", "-c", &perf_cmd])?;
+        if let Some(cwd) = &config.working_directory {
+            let abs_cwd = canonicalize(cwd)?;
+            cmd.current_dir(abs_cwd);
+        }
         let on_process_started = async |_| -> anyhow::Result<()> {
             let data = Self::handle_fifo(runner_fifo, perf_fifo).await?;
             let _ = self.benchmark_data.set(data);