Merge pull request #209 from Kong/macos

search command in PATH instead of relying on execvpe()

src/horust/supervisor/process_spawner.rs

@@ -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;
@@ -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))
@@ -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>,
@@ -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),
@@ -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, .. }) => {
@@ -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,
@@ -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(())
 }

tests/horust.rs

@@ -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"]);
@@ -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));
 }
@@ -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));
 }

tests/section_environment.rs

@@ -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"#;
@@ -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]
@@ -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"));
 }
 
@@ -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()
@@ -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()

tests/section_failure.rs

@@ -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()),
@@ -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));
 }

tests/section_general.rs

@@ -25,7 +25,7 @@ printf "{}" {}"#,
         pattern, redir
     );
     let service = format!(r#"{}="{}""#, stream, to);
-    store_service(
+    store_service_script(
         temp_dir.path(),
         script.as_str(),
         Some(service.as_str()),
@@ -56,6 +56,22 @@ fn test_output_redirection() {
         .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();
@@ -64,7 +80,7 @@ fn test_cwd() {
     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()));
 }
 
@@ -84,14 +100,14 @@ fn test_start_after() {
     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"));
 
     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),
@@ -102,7 +118,7 @@ echo "b""#;
     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"));
 }
@@ -116,8 +132,8 @@ fn test_user() {
     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"));
 }
 
@@ -133,7 +149,7 @@ done
     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");

tests/section_healthiness.rs

@@ -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);
 

tests/section_restart.rs

@@ -38,7 +38,7 @@ attempts = {}
             .display(),
         attempts
     );
-    store_service(
+    store_service_script(
         temp_dir.path(),
         failing_once_script.as_str(),
         Some(service.as_str()),
@@ -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()),
@@ -103,7 +103,7 @@ kill -{} $$
 [restart]
 strategy = "always"
 "#;
-    store_service(
+    store_service_script(
         temp_dir.path(),
         suicide_script.as_str(),
         Some(service),
@@ -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()

tests/section_termination.rs

@@ -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");
@@ -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()),
@@ -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));
 }