Skip to content

Commit fa99d39

Browse files
committed
feat: properly handle sudo with a command builder
1 parent 05f6787 commit fa99d39

File tree

9 files changed

+289
-99
lines changed

9 files changed

+289
-99
lines changed

Cargo.lock

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ addr2line = "0.25"
6262
gimli = "0.32"
6363
open = "5.3.2"
6464
tabled = "0.17"
65+
shell-words = "1.1.0"
6566

6667
[target.'cfg(target_os = "linux")'.dependencies]
6768
procfs = "0.17.0"

src/run/runner/helpers/apt.rs

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -96,10 +96,10 @@ pub fn install(system_info: &SystemInfo, packages: &[&str]) -> Result<()> {
9696

9797
info!("Installing packages: {}", packages.join(", "));
9898

99-
run_with_sudo(&["apt-get", "update"])?;
100-
let mut install_cmd = vec!["apt-get", "install", "-y", "--allow-downgrades"];
101-
install_cmd.extend_from_slice(packages);
102-
run_with_sudo(&install_cmd)?;
99+
run_with_sudo("apt-get", ["update"])?;
100+
let mut install_argv = vec!["install", "-y", "--allow-downgrades"];
101+
install_argv.extend_from_slice(packages);
102+
run_with_sudo("apt-get", &install_argv)?;
103103

104104
debug!("Packages installed successfully");
105105
Ok(())
@@ -155,9 +155,7 @@ fn restore_from_cache(system_info: &SystemInfo, cache_dir: &Path) -> Result<()>
155155
.to_str()
156156
.ok_or_else(|| anyhow!("Invalid cache directory path"))?;
157157

158-
let copy_cmd = format!("cp -r {cache_dir_str}/* /");
159-
160-
run_with_sudo(&["bash", "-c", &copy_cmd])?;
158+
run_with_sudo("cp", ["-r", &format!("{cache_dir_str}/*"), "/"])?;
161159

162160
debug!("Cache restored successfully");
163161
Ok(())

src/run/runner/helpers/command.rs

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
use std::{
2+
collections::BTreeMap,
3+
ffi::{OsStr, OsString},
4+
process::Command,
5+
};
6+
7+
#[derive(Debug)]
8+
pub struct CommandBuilder {
9+
program: OsString,
10+
argv: Vec<OsString>,
11+
envs: BTreeMap<OsString, OsString>,
12+
cwd: Option<OsString>,
13+
}
14+
15+
impl CommandBuilder {
16+
pub fn new<S: AsRef<OsStr>>(program: S) -> Self {
17+
Self {
18+
program: program.as_ref().to_owned(),
19+
argv: Vec::new(),
20+
envs: BTreeMap::new(),
21+
cwd: None,
22+
}
23+
}
24+
25+
pub fn build(self) -> Command {
26+
let mut command = Command::new(&self.program);
27+
command.args(&self.argv);
28+
command.envs(&self.envs);
29+
if let Some(cwd) = self.cwd {
30+
command.current_dir(cwd);
31+
}
32+
command
33+
}
34+
35+
pub fn arg<S: AsRef<OsStr>>(&mut self, arg: S) -> &mut Self {
36+
self.argv.push(arg.as_ref().to_owned());
37+
self
38+
}
39+
40+
pub fn args<I, S>(&mut self, args: I) -> &mut Self
41+
where
42+
I: IntoIterator<Item = S>,
43+
S: AsRef<OsStr>,
44+
{
45+
for arg in args {
46+
self.arg(arg.as_ref());
47+
}
48+
self
49+
}
50+
51+
pub fn current_dir<D>(&mut self, dir: D)
52+
where
53+
D: AsRef<OsStr>,
54+
{
55+
self.cwd = Some(dir.as_ref().to_owned());
56+
}
57+
58+
pub fn wrap<S, I, T>(&mut self, wrapper: S, wrapper_args: I) -> &mut Self
59+
where
60+
S: AsRef<OsStr>,
61+
I: IntoIterator<Item = T>,
62+
T: AsRef<OsStr>,
63+
{
64+
let mut new_argv = Vec::new();
65+
66+
// Add wrapper arguments first
67+
for arg in wrapper_args {
68+
new_argv.push(arg.as_ref().to_owned());
69+
}
70+
71+
// Add the current program
72+
new_argv.push(self.program.clone());
73+
74+
// Add the current arguments
75+
new_argv.extend(self.argv.iter().cloned());
76+
77+
// Update program to wrapper and argv to the new argument list
78+
self.program = wrapper.as_ref().to_owned();
79+
self.argv = new_argv;
80+
self
81+
}
82+
83+
pub fn wrap_with(&mut self, wrapper_cmd: CommandBuilder) -> &mut Self {
84+
let mut new_argv = Vec::new();
85+
86+
// Add wrapper command arguments first
87+
new_argv.extend(wrapper_cmd.argv.iter().cloned());
88+
89+
// Add the current program
90+
new_argv.push(self.program.clone());
91+
92+
// Add the current arguments
93+
new_argv.extend(self.argv.iter().cloned());
94+
95+
// Update program to wrapper and argv to the new argument list
96+
self.program = wrapper_cmd.program;
97+
self.argv = new_argv;
98+
// Update cwd if wrapper has it set
99+
self.cwd = wrapper_cmd.cwd.or(self.cwd.take());
100+
// Merge environment variables, with wrapper's envs taking precedence
101+
self.envs.extend(wrapper_cmd.envs);
102+
self
103+
}
104+
105+
/// Returns the command line as a string for debugging/testing purposes
106+
pub fn as_command_line(&self) -> String {
107+
let mut parts: Vec<String> = vec![self.program.to_string_lossy().into_owned()];
108+
parts.extend(
109+
self.argv
110+
.iter()
111+
.map(|arg| arg.to_string_lossy().into_owned()),
112+
);
113+
shell_words::join(parts)
114+
}
115+
}
116+
117+
#[cfg(test)]
118+
mod tests {
119+
use super::*;
120+
121+
#[test]
122+
fn test_wrap_with_args() {
123+
let mut builder = CommandBuilder::new("ls");
124+
builder.arg("-la").wrap("sudo", ["-n"]);
125+
assert_eq!(builder.as_command_line(), "sudo -n ls -la");
126+
}
127+
128+
#[test]
129+
fn test_wrap_without_args() {
130+
let mut builder = CommandBuilder::new("ls");
131+
builder.arg("-la").wrap("time", [] as [&str; 0]);
132+
assert_eq!(builder.as_command_line(), "time ls -la");
133+
}
134+
135+
#[test]
136+
fn test_multiple_wraps() {
137+
let mut builder = CommandBuilder::new("valgrind");
138+
builder
139+
.arg("my-program")
140+
.wrap("setarch", ["x86_64", "-R"])
141+
.wrap("sudo", ["-n"]);
142+
assert_eq!(
143+
builder.as_command_line(),
144+
"sudo -n setarch x86_64 -R valgrind my-program"
145+
);
146+
}
147+
148+
#[test]
149+
fn test_wrap_with_spaces() {
150+
let mut builder = CommandBuilder::new("echo");
151+
builder.arg("hello world").wrap("bash", ["-c"]);
152+
assert_eq!(builder.as_command_line(), "bash -c echo 'hello world'");
153+
}
154+
155+
#[test]
156+
fn test_wrap_and_build() {
157+
let mut builder = CommandBuilder::new("ls");
158+
builder.arg("-la").wrap("sudo", ["-n"]);
159+
160+
let cmd = builder.build();
161+
assert_eq!(cmd.get_program(), "sudo");
162+
163+
let args: Vec<_> = cmd.get_args().collect();
164+
assert_eq!(args, vec!["-n", "ls", "-la"]);
165+
}
166+
167+
#[test]
168+
fn test_wrap_with_builder() {
169+
let mut wrapper = CommandBuilder::new("sudo");
170+
wrapper.arg("-n");
171+
172+
let mut builder = CommandBuilder::new("ls");
173+
builder.arg("-la").wrap_with(wrapper);
174+
175+
assert_eq!(builder.as_command_line(), "sudo -n ls -la");
176+
}
177+
178+
#[test]
179+
fn test_wrap_with_preserves_env() {
180+
let mut wrapper = CommandBuilder::new("env");
181+
wrapper.arg("FOO=bar");
182+
183+
let mut builder = CommandBuilder::new("ls");
184+
builder.arg("-la").wrap_with(wrapper);
185+
186+
assert_eq!(builder.as_command_line(), "env 'FOO=bar' ls -la");
187+
}
188+
}

src/run/runner/helpers/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
pub mod apt;
2+
pub mod command;
23
pub mod env;
34
pub mod get_bench_command;
45
pub mod introspected_golang;

src/run/runner/helpers/run_with_sudo.rs

Lines changed: 32 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
1-
use crate::{local_logger::suspend_progress_bar, prelude::*};
1+
use crate::{
2+
local_logger::suspend_progress_bar, prelude::*, run::runner::helpers::command::CommandBuilder,
3+
};
24
use std::{
5+
ffi::OsStr,
36
io::IsTerminal,
47
process::{Command, Stdio},
58
};
@@ -35,9 +38,7 @@ fn validate_sudo_access() -> Result<()> {
3538

3639
if needs_password {
3740
suspend_progress_bar(|| {
38-
info!(
39-
"Sudo privileges are required to continue. Please enter your password if prompted."
40-
);
41+
info!("Sudo privileges are required to continue. Please enter your password.");
4142

4243
// Validate and cache sudo credentials
4344
let auth_status = Command::new("sudo")
@@ -57,47 +58,47 @@ fn validate_sudo_access() -> Result<()> {
5758
Ok(())
5859
}
5960

60-
/// Creates the base sudo command after validating sudo access
61-
pub fn validated_sudo_command() -> Result<Command> {
62-
validate_sudo_access()?;
63-
let mut cmd = Command::new("sudo");
64-
// Password prompt should not appear here since it has already been validated
65-
cmd.arg("--non-interactive");
66-
Ok(cmd)
67-
}
68-
69-
/// Build a command wrapped with sudo if possible
70-
pub fn build_command_with_sudo(command_args: &[&str]) -> Result<Command> {
71-
let command_str = command_args.join(" ");
61+
/// Wrap with sudo if not running as root
62+
pub fn wrap_with_sudo(mut cmd_builder: CommandBuilder) -> Result<CommandBuilder> {
7263
if is_root_user() {
73-
debug!("Running command without sudo: {command_str}");
74-
let mut c = Command::new(command_args[0]);
75-
c.args(&command_args[1..]);
76-
Ok(c)
64+
Ok(cmd_builder)
7765
} else if is_sudo_available() {
78-
debug!("Sudo is required for command: {command_str}");
79-
let mut c = validated_sudo_command()?;
80-
c.args(command_args);
81-
Ok(c)
66+
debug!("Wrapping with sudo: {}", cmd_builder.as_command_line());
67+
validate_sudo_access()?;
68+
cmd_builder.wrap(
69+
"sudo",
70+
// Password prompt should not appear here since it has already been validated
71+
["--non-interactive"],
72+
);
73+
Ok(cmd_builder)
8274
} else {
83-
bail!("Sudo is not available to run the command: {command_str}");
75+
bail!(
76+
"Sudo is not available to run the command: {}",
77+
cmd_builder.as_command_line()
78+
);
8479
}
8580
}
8681

8782
/// Run a command with sudo after validating sudo access
88-
pub fn run_with_sudo(command_args: &[&str]) -> Result<()> {
89-
let command_str = command_args.join(" ");
90-
debug!("Running command with sudo: {command_str}");
91-
let mut cmd = build_command_with_sudo(command_args)?;
83+
pub fn run_with_sudo<S, I, T>(program: S, argv: I) -> Result<()>
84+
where
85+
S: AsRef<OsStr>,
86+
I: IntoIterator<Item = T>,
87+
T: AsRef<OsStr>,
88+
{
89+
let mut builder = CommandBuilder::new(program);
90+
builder.args(argv);
91+
debug!("Running command with sudo: {}", builder.as_command_line());
92+
let mut cmd = wrap_with_sudo(builder)?.build();
9293
let output = cmd
9394
.stdout(Stdio::piped())
9495
.output()
95-
.map_err(|_| anyhow!("Failed to execute command with sudo: {command_str}"))?;
96+
.map_err(|_| anyhow!("Failed to execute command with sudo: {cmd:?}"))?;
9697

9798
if !output.status.success() {
9899
info!("stdout: {}", String::from_utf8_lossy(&output.stdout));
99100
error!("stderr: {}", String::from_utf8_lossy(&output.stderr));
100-
bail!("Failed to execute command with sudo: {command_str}");
101+
bail!("Failed to execute command with sudo: {cmd:?}");
101102
}
102103

103104
Ok(())

0 commit comments

Comments
 (0)