Skip to content

fix(sudo): handle root user case #140

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
fargito wants to merge 1 commit into from
Closed

fix(sudo): handle root user case #140

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 33 additions & 7 deletions src/run/runner/helpers/run_with_sudo.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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(())
Expand Down
46 changes: 29 additions & 17 deletions src/run/runner/wall_time/executor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()?;
Expand Down Expand Up @@ -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(),
))
}
}

Expand All @@ -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
}
};
Expand Down
34 changes: 17 additions & 17 deletions src/run/runner/wall_time/perf/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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};

Expand Down Expand Up @@ -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()?;

Expand All @@ -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"
Expand All @@ -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",
Expand All @@ -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);
Expand Down
Loading