fix: get proper version from download manifest and spawn with canonical path

config/local.Hipcheck.kdl

@@ -0,0 +1,54 @@
+plugins {
+    plugin "mitre/activity" version="0.1.0" manifest="plugins/activity/local-plugin.kdl"
+    plugin "mitre/affiliation" version="0.1.0" manifest="plugins/affiliation/local-plugin.kdl"
+    plugin "mitre/binary" version="0.1.0" manifest="plugins/binary/local-plugin.kdl"
+    plugin "mitre/churn" version="0.1.0" manifest="plugins/churn/local-plugin.kdl"
+    plugin "mitre/entropy" version="0.1.0" manifest="plugins/entropy/local-plugin.kdl"
+    plugin "mitre/fuzz" version="0.1.1" manifest="plugins/fuzz/local-plugin.kdl"
+    plugin "mitre/review" version="0.1.0" manifest="plugins/review/local-plugin.kdl"
+    plugin "mitre/typo" version="0.1.0" manifest="plugins/typo/local-plugin.kdl"
+}
+
+patch {
+	plugin "mitre/github" {
+		api-token-var "HC_GITHUB_TOKEN"
+	}
+}
+
+analyze {
+    investigate policy="(gt 0.5 $)"
+    investigate-if-fail "mitre/typo" "mitre/binary"
+
+    category "practices" {
+        analysis "mitre/activity" policy="(lte $ P52w)" weight=3
+        analysis "mitre/binary" {
+			binary-file "./config/Binary.toml"
+			binary-file-threshold 0
+		}
+        analysis "mitre/fuzz" policy="(eq #t $)"
+        analysis "mitre/review" policy="(lte (divz (count (filter (eq #f) $)) (count $)) 0.05)"
+    }
+
+    category "attacks" {
+        analysis "mitre/typo" {
+            typo-file "./config/Typos.toml"
+            count-threshold 0
+        }
+
+        category "commit" {
+            analysis "mitre/affiliation" {
+                orgs-file "./plugins/affiliation/test/example_orgs.kdl"
+                count-threshold 0
+            }
+
+            analysis "mitre/entropy" policy="(eq 0 (count (filter (gt 8.0) $)))" {
+				langs-file "./config/Langs.toml"
+				entropy-threshold 10.0
+				commit-percentage 0.0
+	 		}
+            analysis "mitre/churn" policy="(lte (divz (count (filter (gt 3) $)) (count $)) 0.02)" {
+				langs-file "./config/Langs.toml"
+			}
+        }
+    }
+}

hipcheck/src/engine.rs

@@ -239,7 +239,12 @@ pub fn start_plugins(
 
 	let mut plugins = vec![];
 	for plugin_id in required_plugin_names.iter() {
-		let plugin_manifest = PluginManifest::from_file(plugin_cache.plugin_kdl(plugin_id))?;
+		let plugin_kdl = plugin_cache.plugin_kdl(plugin_id);
+		let working_dir = plugin_kdl
+			.parent()
+			.expect("The plugin.kdl is always in the plugin cache")
+			.to_owned();
+		let plugin_manifest = PluginManifest::from_file(plugin_kdl)?;
 		let entrypoint = plugin_manifest
 			.get_entrypoint(&current_arch)
 			.ok_or_else(|| {
@@ -252,6 +257,7 @@ pub fn start_plugins(
 
 		let plugin = Plugin {
 			name: plugin_id.to_policy_file_plugin_identifier(),
+			working_dir,
 			entrypoint,
 		};
 

hipcheck/src/main.rs

@@ -489,16 +489,18 @@ fn cmd_plugin(args: PluginArgs) {
 	use std::sync::Arc;
 	use tokio::task::JoinSet;
 
-	let tgt_dir = "./target/debug";
+	let working_dir = PathBuf::from("./target/debug");
 
-	let entrypoint1 = pathbuf![tgt_dir, "dummy_rand_data"];
-	let entrypoint2 = pathbuf![tgt_dir, "dummy_sha256"];
+	let entrypoint1 = pathbuf!["dummy_rand_data"];
+	let entrypoint2 = pathbuf!["dummy_sha256"];
 	let plugin1 = Plugin {
 		name: "dummy/rand_data".to_owned(),
+		working_dir: working_dir.clone(),
 		entrypoint: entrypoint1.display().to_string(),
 	};
 	let plugin2 = Plugin {
 		name: "dummy/sha256".to_owned(),
+		working_dir: working_dir.clone(),
 		entrypoint: entrypoint2.display().to_string(),
 	};
 	let plugin_executor = PluginExecutor::new(

hipcheck/src/plugin/manager.rs

@@ -8,7 +8,7 @@ use crate::{
 use futures::future::join_all;
 use hipcheck_common::proto::plugin_service_client::PluginServiceClient;
 use rand::Rng;
-use std::{ops::Range, process::Command};
+use std::{ffi::OsString, ops::Range, path::Path, process::Command};
 use tokio::time::{sleep_until, Duration, Instant};
 
 #[derive(Clone, Debug)]
@@ -76,31 +76,69 @@ impl PluginExecutor {
 		// port to become unavailable between our check and the plugin's bind attempt.
 		// Hence the need for subsequent attempts if we get unlucky
 
-		log::debug!("Starting plugin '{}'", plugin.name);
-
 		// `entrypoint` is a string that represents a CLI invocation which may contain
 		// arguments, so we have to split off only the first token
-		if let Some(bin_path) = try_get_bin_for_entrypoint(&plugin.entrypoint).0 {
-			if which::which(bin_path).is_err() {
-				log::warn!(
-					"Binary '{}' used to spawn {} does not exist, spawn is unlikely to succeed",
-					bin_path,
-					plugin.name
-				);
-			}
-		}
+		let (opt_bin_path_str, args) = try_get_bin_for_entrypoint(&plugin.entrypoint);
+		let Some(bin_path_str) = opt_bin_path_str else {
+			return Err(hc_error!(
+				"Unable to get bin path for plugin entrypoint '{}'",
+				&plugin.entrypoint
+			));
+		};
+
+		// Entrypoints are often "<BIN_NAME>" which can overlap with existing binaries on the
+		// system like "git", "npm", so we must search for <BIN_NAME> from within the plugin
+		// cache subfolder. First, grab the existing path.
+		let Some(mut os_paths) =
+			std::env::var_os("PATH").map(|s| std::env::split_paths(&s).collect::<Vec<_>>())
+		else {
+			return Err(hc_error!("Unable to get system PATH var"));
+		};
+
+		let Ok(canon_working_dir) = plugin.working_dir.canonicalize() else {
+			return Err(hc_error!(
+				"Failed to canonicalize plugin working dir: {:?}",
+				&plugin.working_dir
+			));
+		};
+
+		// Add canonicalized plugin cache dir to temp PATH
+		os_paths.insert(0, canon_working_dir.clone());
+		let search_path = std::env::join_paths(os_paths).unwrap();
+
+		// Find the binary_str using temp PATH
+		let Ok(canon_bin_path) = which::which_in::<&str, &OsString, &Path>(
+			bin_path_str,
+			Some(&search_path),
+			canon_working_dir.as_ref(),
+		) else {
+			return Err(hc_error!(
+				"Failed to find binary '{}' for plugin",
+				bin_path_str
+			));
+		};
+
+		log::debug!(
+			"Starting plugin '{}' at '{:?}'",
+			plugin.name,
+			&canon_bin_path
+		);
 
 		let mut spawn_attempts: usize = 0;
 		while spawn_attempts < self.max_spawn_attempts {
+			let mut spawn_args = args.clone();
+
 			// Find free port for process. Don't retry if we fail since this means all
 			// ports in the desired range are already bound
 			let port = self.get_available_port()?;
 			let port_str = port.to_string();
+			spawn_args.push("--port");
+			spawn_args.push(port_str.as_str());
 
 			// Spawn plugin process
 			log::debug!("Spawning '{}' on port {}", &plugin.entrypoint, port_str);
-			let Ok(mut proc) = Command::new(&plugin.entrypoint)
-				.args(["--port", port_str.as_str()])
+			let Ok(mut proc) = Command::new(&canon_bin_path)
+				.args(spawn_args)
 				// @Temporary - directly forward stdout/stderr from plugin to shell
 				.stdout(std::io::stdout())
 				.stderr(std::io::stderr())

hipcheck/src/plugin/plugin_manifest.rs

@@ -288,7 +288,7 @@ impl PluginManifest {
 		let mut new_entrypoint = new_bin_path.to_string_lossy().to_string();
 		if args.is_empty().not() {
 			new_entrypoint.push(' ');
-			new_entrypoint.push_str(&args);
+			new_entrypoint.push_str(&args.join(" "));
 		}
 
 		self.set_entrypoint(arch.clone(), new_entrypoint);
@@ -347,7 +347,7 @@ impl FromStr for PluginManifest {
 	}
 }
 
-pub fn try_get_bin_for_entrypoint(entrypoint: &str) -> (Option<&str>, String) {
+pub fn try_get_bin_for_entrypoint(entrypoint: &str) -> (Option<&str>, Vec<&str>) {
 	let mut split = entrypoint.split_whitespace();
 	(split.next(), split.collect())
 }

hipcheck/src/plugin/retrieval.rs

@@ -59,7 +59,11 @@ fn retrieve_plugin(
 	// TODO: if the plugin.kdl file for the plugin already exists, then should we skip the retrieval process?
 	// if plugin_cache.plugin_kdl(&plugin_id).exists()
 
-	log::debug!("Retrieving Plugin ID: {:?}", plugin_id);
+	log::debug!(
+		"Retrieving Plugin ID {:?} from {:?}",
+		plugin_id,
+		manifest_location
+	);
 
 	let plugin_manifest = match manifest_location {
 		Some(ManifestLocation::Url(plugin_url)) => {
@@ -94,15 +98,17 @@ fn retrieve_plugin_from_network(
 	plugin_cache: &HcPluginCache,
 ) -> Result<PluginManifest, Error> {
 	let current_arch = get_current_arch();
+	let version = plugin_id.version();
 	let download_manifest = retrieve_download_manifest(plugin_url)?;
 	for entry in &download_manifest.entries {
-		if entry.arch == current_arch {
+		if entry.arch == current_arch && version == &entry.version {
 			return download_and_unpack_plugin(entry, plugin_id, plugin_cache);
 		}
 	}
 	Err(hc_error!(
-		"Could not find download manifest entry for arch '{}'",
-		current_arch
+		"Could not find download manifest entry for arch '{}' with version '{}'",
+		current_arch,
+		version.0
 	))
 }
 

hipcheck/src/plugin/types.rs

@@ -19,6 +19,7 @@ use std::{
 	convert::TryFrom,
 	future::poll_fn,
 	ops::Not as _,
+	path::PathBuf,
 	pin::Pin,
 	process::Child,
 	result::Result as StdResult,
@@ -32,6 +33,7 @@ pub type HcPluginClient = PluginServiceClient<Channel>;
 #[derive(Clone, Debug)]
 pub struct Plugin {
 	pub name: String,
+	pub working_dir: PathBuf,
 	pub entrypoint: String,
 }