Skip to content
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.

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

search command in PATH instead of relying on execvpe() #209

Merged
merged 3 commits into from
Sep 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
52 changes: 39 additions & 13 deletions src/horust/supervisor/process_spawner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::os::unix::io::AsRawFd;
use std::path::PathBuf;
use std::time::Duration;

use anyhow::{Context, Result};
use anyhow::{anyhow, Context, Result};
use crossbeam::channel::{after, tick};
use nix::errno::Errno;
use nix::fcntl;
Expand Down Expand Up @@ -61,7 +61,7 @@ pub(crate) fn spawn_fork_exec_handler(
fn exec_args(service: &Service) -> Result<(CString, Vec<CString>, Vec<CString>)> {
let chunks: Vec<String> =
shlex::split(&service.command).context(format!("Invalid command: {}", service.command,))?;
let program_name = CString::new(chunks.get(0).unwrap().as_str())?;
let program_name = String::from(chunks.get(0).unwrap());
let to_cstring = |s: Vec<String>| {
s.into_iter()
.map(|arg| CString::new(arg).map_err(Into::into))
Expand All @@ -70,14 +70,18 @@ fn exec_args(service: &Service) -> Result<(CString, Vec<CString>, Vec<CString>)>
let arg_cstrings = to_cstring(chunks)?;
let environment = service.get_environment()?;
let env_cstrings = to_cstring(environment)?;

Ok((program_name, arg_cstrings, env_cstrings))
let path = if program_name.contains("/") {
program_name.to_string()
} else {
find_program(&program_name)?
};
Ok((CString::new(path)?, arg_cstrings, env_cstrings))
}

#[inline]
fn child_process_main(
service: &Service,
program_name: CString,
path: CString,
cwd: PathBuf,
uid: Uid,
arg_cptr: Vec<&CStr>,
Expand All @@ -99,7 +103,7 @@ fn child_process_main(
102,
);
}
if let Err(errno) = exec(program_name, arg_cptr, env_cptr, uid, cwd) {
if let Err(errno) = exec(path, arg_cptr, env_cptr, uid, cwd) {
panic_ssafe(
"child_process_main: Failed to exec the new process.",
Some(&service.name),
Expand All @@ -112,14 +116,14 @@ fn child_process_main(
/// Fork the process
fn spawn_process(service: &Service) -> Result<Pid> {
debug!("Spawning process for service: {}", service.name);
let (program_name, arg_cstrings, env_cstrings) = exec_args(service)?;
let (path, arg_cstrings, env_cstrings) = exec_args(service)?;
let uid = service.user.get_uid()?;
let cwd = service.working_directory.clone();
let arg_cptr: Vec<&CStr> = arg_cstrings.iter().map(|c| c.as_c_str()).collect();
let env_cptr: Vec<&CStr> = env_cstrings.iter().map(|c| c.as_c_str()).collect();
match unsafe { fork() } {
Ok(ForkResult::Child) => {
child_process_main(service, program_name, cwd, uid, arg_cptr, env_cptr);
child_process_main(service, path, cwd, uid, arg_cptr, env_cptr);
unreachable!();
}
Ok(ForkResult::Parent { child, .. }) => {
Expand Down Expand Up @@ -169,14 +173,39 @@ fn redirect_output(
Ok(())
}

/// Find program on PATH.
///
fn find_program(program_name: &String) -> Result<String> {
let path_var = match std::env::var_os("PATH") {
Some(val) => val,
None => return Err(anyhow!("PATH environment variable is not set")),
};

let paths: Vec<PathBuf> = std::env::split_paths(&path_var).collect();

for path in paths {
let program_path = path.join(program_name);

// Check if the program file exists at this path
if program_path.is_file() {
return Ok(program_path.into_os_string().into_string().unwrap());
}
}

Err(anyhow!(
"Program {:?} not found in any of the PATH directories",
program_name
))
}

/// Exec wrapper.
///
/// # Safety
///
/// Use only async-signal-safe, otherwise it might lock.
#[inline]
fn exec(
program_name: CString,
path: CString,
arg_cptr: Vec<&CStr>,
env_cptr: Vec<&CStr>,
uid: Uid,
Expand All @@ -188,9 +217,6 @@ fn exec(
unistd::setsid()?;
// Set the user ID
unistd::setuid(uid)?;
#[cfg(target_os = "linux")]
unistd::execvpe(program_name.as_ref(), arg_cptr.as_ref(), env_cptr.as_ref())?;
#[cfg(not(target_os = "linux"))]
unistd::execve(program_name.as_ref(), arg_cptr.as_ref(), env_cptr.as_ref())?;
unistd::execve(path.as_ref(), arg_cptr.as_ref(), env_cptr.as_ref())?;
Ok(())
}
10 changes: 5 additions & 5 deletions tests/horust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ fn test_config_unsuccessful_exit_finished_failed() {
let failing_script = r#"#!/usr/bin/env bash
exit 1
"#;
store_service(temp_dir.path(), failing_script, None, None);
store_service_script(temp_dir.path(), failing_script, None, None);
let recv = run_async(&mut cmd, true);
recv.recv_or_kill(Duration::from_secs(15));
let cmd = cmd.args(vec!["--unsuccessful-exit-finished-failed"]);
Expand Down Expand Up @@ -82,14 +82,14 @@ fn test_multiple() {
} else {
temp_dir_2.path()
};
store_service(
store_service_script(
t_dir,
script,
Some(service.as_str()),
Some(i.to_string().as_str()),
);
}
store_service(temp_dir.path(), script, None, Some("0"));
store_service_script(temp_dir.path(), script, None, Some("0"));
let recv = run_async(&mut cmd, true);
recv.recv_or_kill(Duration::from_secs(max * 2));
}
Expand All @@ -104,14 +104,14 @@ fn test_stress_test_chained_services() {

for i in 1..max {
let service = format!(r#"start-after = ["{}.toml"]"#, i - 1);
store_service(
store_service_script(
temp_dir.path(),
script,
Some(service.as_str()),
Some(i.to_string().as_str()),
);
}
store_service(temp_dir.path(), script, None, Some("0"));
store_service_script(temp_dir.path(), script, None, Some("0"));
let recv = run_async(&mut cmd, true);
recv.recv_or_kill(Duration::from_secs(max * 2));
}
10 changes: 5 additions & 5 deletions tests/section_environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use predicates::str::contains;

#[allow(dead_code)]
mod utils;
use utils::{get_cli, store_service};
use utils::{get_cli, store_service_script};

static ENVIRONMENT_SCRIPT: &str = r#"#!/usr/bin/env bash
printenv"#;
Expand All @@ -14,7 +14,7 @@ printenv"#;
fn test_environment_additional() {
let (mut cmd, temp_dir) = get_cli();

store_service(temp_dir.path(), ENVIRONMENT_SCRIPT, None, None);
store_service_script(temp_dir.path(), ENVIRONMENT_SCRIPT, None, None);
cmd.assert().success().stdout(contains("bar").not());

let service = r#"[environment]
Expand All @@ -23,7 +23,7 @@ re-export = [ "TERM" ]
additional = { TERM = "bar" }
"#;
// Additional should overwrite TERM
store_service(temp_dir.path(), ENVIRONMENT_SCRIPT, Some(service), None);
store_service_script(temp_dir.path(), ENVIRONMENT_SCRIPT, Some(service), None);
cmd.assert().success().stdout(contains("bar"));
}

Expand All @@ -34,7 +34,7 @@ fn test_environment_keep_env() {
let service = r#"[environment]
keep-env = true
"#;
store_service(temp_dir.path(), ENVIRONMENT_SCRIPT, Some(service), None);
store_service_script(temp_dir.path(), ENVIRONMENT_SCRIPT, Some(service), None);
cmd.env("DB_PASS", "MyPassword")
.assert()
.success()
Expand All @@ -49,7 +49,7 @@ fn test_environment_re_export() {
keep-env = false
re-export = [ "DB_PASS" ]
"#;
store_service(temp_dir.path(), ENVIRONMENT_SCRIPT, Some(service), None);
store_service_script(temp_dir.path(), ENVIRONMENT_SCRIPT, Some(service), None);
cmd.env("DB_PASS", "MyPassword")
.assert()
.success()
Expand Down
4 changes: 2 additions & 2 deletions tests/section_failure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ strategy = "{}"
# Let's give horust some time to spinup the other service as well.
sleep 1
exit 1"#;
store_service(
store_service_script(
temp_dir.path(),
failing_script,
Some(failing_service.as_str()),
Expand All @@ -32,7 +32,7 @@ wait = "500millis"
sleep 30"#;

//store_service(temp_dir.path(), sleep_script, None, None);
store_service(temp_dir.path(), sleep_script, Some(sleep_service), None);
store_service_script(temp_dir.path(), sleep_script, Some(sleep_service), None);
let recv = run_async(&mut cmd, true);
recv.recv_or_kill(Duration::from_secs(15));
}
Expand Down
32 changes: 24 additions & 8 deletions tests/section_general.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
pattern, redir
);
let service = format!(r#"{}="{}""#, stream, to);
store_service(
store_service_script(
temp_dir.path(),
script.as_str(),
Some(service.as_str()),
Expand Down Expand Up @@ -56,6 +56,22 @@
.for_each(|(stream, to)| test_single_output_redirection(stream, to));
}

#[test]
fn test_search_path_not_found() {
let (mut cmd, temp_dir) = get_cli();
store_service(temp_dir.path(), "command = \"non-existent-command\"", None);
cmd.assert().success().stderr(contains(
"Program \"non-existent-command\" not found in any of the PATH directories",
));
}

#[test]
fn test_search_path_found() {
let (mut cmd, temp_dir) = get_cli();
store_service(temp_dir.path(), "command = \"echo kilroy was here\"", None);
cmd.assert().success().stdout(contains("kilroy was here"));
}

#[test]
fn test_cwd() {
let (mut cmd, temp_dir) = get_cli();
Expand All @@ -64,7 +80,7 @@
let service = format!(r#"working-directory = "{}""#, displ);
let script = r#"#!/usr/bin/env bash
pwd"#;
store_service(temp_dir.path(), script, Some(service.as_str()), None);
store_service_script(temp_dir.path(), script, Some(service.as_str()), None);
cmd.assert().success().stdout(contains(displ.as_str()));
}

Expand All @@ -73,14 +89,14 @@
let (mut cmd, temp_dir) = get_cli();
let script_first = r#"#!/usr/bin/env bash
echo "a""#;
store_service(temp_dir.path(), script_first, None, Some("a"));
store_service_script(temp_dir.path(), script_first, None, Some("a"));

Check failure on line 92 in tests/section_general.rs

View workflow job for this annotation

GitHub Actions / Test (beta)

this function takes 3 arguments but 4 arguments were supplied

let service_second = r#"start-delay = "500millis"
start-after = ["a.toml"]
"#;
let script_second = r#"#!/usr/bin/env bash
echo "b""#;
store_service(
store_service_script(
temp_dir.path(),
script_second,
Some(service_second),
Expand All @@ -91,7 +107,7 @@
start-after = ["b.toml"]"#;
let script_c = r#"#!/usr/bin/env bash
echo "c""#;
store_service(temp_dir.path(), script_c, Some(service_c), None);
store_service_script(temp_dir.path(), script_c, Some(service_c), None);

cmd.assert().success().stdout(contains("a\nb\nc"));
}
Expand All @@ -105,8 +121,8 @@
let service = r#"user = "games""#;
let script = r#"#!/usr/bin/env bash
whoami"#;
store_service(temp_dir.path(), script, Some(service), None);
store_service(temp_dir.path(), script, None, None);
store_service_script(temp_dir.path(), script, Some(service), None);
store_service_script(temp_dir.path(), script, None, None);
cmd.assert().success().stdout(contains("games"));
}

Expand All @@ -122,7 +138,7 @@
let service = r#"
start-delay = "10s"
"#;
store_service(temp_dir.path(), script, Some(service), None);
store_service_script(temp_dir.path(), script, Some(service), None);
let recv = run_async(&mut cmd, true);
sleep(Duration::from_secs(1));
kill(recv.pid, Signal::SIGINT).expect("kill");
Expand Down
2 changes: 1 addition & 1 deletion tests/section_healthiness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ http-endpoint = "{}""#,
let script = r#"#!/usr/bin/env bash
sleep 2
"#;
store_service(tempdir.path(), script, Some(service.as_str()), None);
store_service_script(tempdir.path(), script, Some(service.as_str()), None);
let (sender, receiver) = mpsc::sync_channel(0);
let (stop_listener, sl_receiver) = mpsc::sync_channel(1);

Expand Down
8 changes: 4 additions & 4 deletions tests/section_restart.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ attempts = {}
.display(),
attempts
);
store_service(
store_service_script(
temp_dir.path(),
failing_once_script.as_str(),
Some(service.as_str()),
Expand Down Expand Up @@ -75,7 +75,7 @@ attempts = 0
strategy = "on-failure"
"#
.to_string();
store_service(
store_service_script(
temp_dir.path(),
failing_once_script.as_str(),
Some(service.as_str()),
Expand Down Expand Up @@ -103,7 +103,7 @@ kill -{} $$
[restart]
strategy = "always"
"#;
store_service(
store_service_script(
temp_dir.path(),
suicide_script.as_str(),
Some(service),
Expand Down Expand Up @@ -150,7 +150,7 @@ sleep 0.5
[restart]
strategy = "always"
"#;
store_service(temp_dir.path(), suicide_script, Some(service), None);
store_service_script(temp_dir.path(), suicide_script, Some(service), None);
cmd.timeout(Duration::from_millis(2000))
.assert()
.failure()
Expand Down
8 changes: 4 additions & 4 deletions tests/section_termination.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ done
"#;
let service = r#"[termination]
wait = "1s""#;
store_service(temp_dir.path(), script, Some(service), None);
store_service_script(temp_dir.path(), script, Some(service), None);

let recv = run_async(&mut cmd, true);
kill(recv.pid, Signal::SIGINT).expect("kill");
Expand Down Expand Up @@ -66,7 +66,7 @@ wait = "10s""#,
friendly_name
); // wait is higher than the test duration.

store_service(
store_service_script(
temp_dir.path(),
script.as_str(),
Some(service.as_str()),
Expand Down Expand Up @@ -110,12 +110,12 @@ done
wait = "0s"
die-if-failed = ["a.toml"]"#;

store_service(temp_dir.path(), script, Some(service), None);
store_service_script(temp_dir.path(), script, Some(service), None);
let script = r#"#!/usr/bin/env bash
sleep 1
exit 1
"#;
store_service(temp_dir.path(), script, None, Some("a"));
store_service_script(temp_dir.path(), script, None, Some("a"));
let recv = run_async(&mut cmd, true);
recv.recv_or_kill(Duration::from_secs(10));
}
Loading
Loading