fix: handling of plugin entrypoint string

hipcheck/src/plugin/manager.rs

@@ -3,7 +3,7 @@
 use crate::{
 	hc_error,
 	hipcheck::plugin_service_client::PluginServiceClient,
-	plugin::{HcPluginClient, Plugin, PluginContext},
+	plugin::{try_get_bin_for_entrypoint, HcPluginClient, Plugin, PluginContext},
 	Result,
 };
 use futures::future::join_all;
@@ -75,22 +75,28 @@ impl PluginExecutor {
 		// on the cmdline is not already in use, but it is still possible for that
 		// 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 mut spawn_attempts: usize = 0;
 		while spawn_attempts < self.max_spawn_attempts {
 			// 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();
 
-			if let Ok(false) = std::fs::exists(&plugin.entrypoint) {
-				log::warn!(
-					"entrypoint '{}' for {} does not exist",
-					plugin.entrypoint,
-					plugin.name
-				)
-			}
-
 			// Spawn plugin process
 			log::debug!("Spawning '{}' on port {}", &plugin.entrypoint, port_str);
 			let Ok(mut proc) = Command::new(&plugin.entrypoint)

hipcheck/src/plugin/mod.rs

@@ -12,7 +12,9 @@ use crate::error::Result;
 pub use crate::plugin::{get_plugin_key, manager::*, plugin_id::PluginId, types::*};
 pub use arch::{get_current_arch, try_set_arch, Arch};
 pub use download_manifest::{ArchiveFormat, DownloadManifest, HashAlgorithm, HashWithDigest};
-pub use plugin_manifest::{PluginManifest, PluginName, PluginPublisher, PluginVersion};
+pub use plugin_manifest::{
+	try_get_bin_for_entrypoint, PluginManifest, PluginName, PluginPublisher, PluginVersion,
+};
 pub use retrieval::retrieve_plugins;
 use serde_json::Value;
 use std::collections::HashMap;

hipcheck/src/plugin/plugin_manifest.rs

@@ -14,6 +14,7 @@ use crate::{
 use kdl::{KdlDocument, KdlNode};
 use std::{
 	collections::HashMap,
+	ops::Not,
 	path::{Path, PathBuf},
 	str::FromStr,
 };
@@ -247,20 +248,32 @@ impl PluginManifest {
 		Self::from_str(read_string(path)?.as_str())
 	}
 
+	pub fn get_entrypoint_for(&self, arch: &Arch) -> Result<String, Error> {
+		self.entrypoints
+			.0
+			.get(arch)
+			.map(String::from)
+			.ok_or(hc_error!("No entrypoint for current arch ({})", arch))
+	}
+
 	/// Update the directory that an entrypoint is stored in
 	///
 	/// Returns previous entrypoint
 	pub fn update_entrypoint<P>(&mut self, arch: &Arch, new_directory: P) -> Result<PathBuf, Error>
 	where
 		P: AsRef<Path>,
 	{
-		let current_entrypoint = self
+		let curr_entrypoint_str = self
 			.entrypoints
 			.0
 			.remove(arch)
-			.map(PathBuf::from)
 			.ok_or(hc_error!("No entrypoint for current arch ({})", arch))?;
 
+		let (opt_bin_path, args) = try_get_bin_for_entrypoint(&curr_entrypoint_str);
+		let bin_path = opt_bin_path
+			.map(PathBuf::from)
+			.ok_or(hc_error!("Malformed entrypoint string"))?;
+
 		fn new_path(new_directory: &Path, current_entrypoint: &Path) -> Result<PathBuf, Error> {
 			let entrypoint_filename =
 				new_directory.join(current_entrypoint.file_name().ok_or(hc_error!(
@@ -270,9 +283,15 @@ impl PluginManifest {
 			Ok(entrypoint_filename)
 		}
 
-		let new_entrypoint = new_path(new_directory.as_ref(), &current_entrypoint)?;
-		self.set_entrypoint(arch.clone(), new_entrypoint.to_string_lossy().to_string());
-		Ok(current_entrypoint)
+		let new_bin_path = new_path(new_directory.as_ref(), &bin_path)?;
+		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);
+		}
+
+		self.set_entrypoint(arch.clone(), new_entrypoint);
+		Ok(bin_path)
 	}
 
 	/// convert a `PluginManifest` to a `KdlDocument`
@@ -327,6 +346,11 @@ impl FromStr for PluginManifest {
 	}
 }
 
+pub fn try_get_bin_for_entrypoint(entrypoint: &str) -> (Option<&str>, String) {
+	let mut split = entrypoint.split_whitespace();
+	(split.next(), split.collect())
+}
+
 #[cfg(test)]
 mod test {
 

hipcheck/src/plugin/retrieval.rs

@@ -5,8 +5,8 @@ use crate::{
 	error::Error,
 	hc_error,
 	plugin::{
-		download_manifest::DownloadManifestEntry, ArchiveFormat, DownloadManifest, HashAlgorithm,
-		HashWithDigest, PluginId, PluginManifest,
+		download_manifest::DownloadManifestEntry, try_get_bin_for_entrypoint, ArchiveFormat,
+		DownloadManifest, HashAlgorithm, HashWithDigest, PluginId, PluginManifest,
 	},
 	policy::policy_file::{ManifestLocation, PolicyPlugin},
 	util::http::agent::agent,
@@ -121,18 +121,27 @@ fn retrieve_local_plugin(
 	let mut plugin_manifest = PluginManifest::from_file(plugin_manifest_path)?;
 	let current_arch = get_current_arch();
 
-	let original_entrypoint = plugin_manifest
-		.update_entrypoint(&current_arch, plugin_cache.plugin_download_dir(&plugin_id))?;
+	let curr_entrypoint = plugin_manifest.get_entrypoint_for(&current_arch)?;
 
-	// @Note - sneaky potential for unexpected behavior if we write local plugin manifest
-	// to a cache dir that already included a remote download
-	std::fs::copy(
-		&original_entrypoint,
-		plugin_cache
-			.plugin_download_dir(&plugin_id)
+	if let Some(curr_bin) = try_get_bin_for_entrypoint(&curr_entrypoint).0 {
+		// Only do copy if using a path to a binary instead of a PATH-based resolution (i.e.
+		// `docker _`. We wouldn't want to copy the `docker` binary in this case
+		if std::fs::exists(curr_bin)? {
+			let original_entrypoint = plugin_manifest
+				.update_entrypoint(&current_arch, plugin_cache.plugin_download_dir(&plugin_id))?;
+
+			let new_entrypoint = plugin_manifest.get_entrypoint_for(&current_arch)?;
 			// unwrap is safe here, we just updated the entrypoint for current arch
-			.join(plugin_manifest.get_entrypoint(&current_arch).unwrap()),
-	)?;
+			let new_bin = try_get_bin_for_entrypoint(&new_entrypoint).0.unwrap();
+
+			// @Note - sneaky potential for unexpected behavior if we write local plugin manifest
+			// to a cache dir that already included a remote download
+			std::fs::copy(
+				&original_entrypoint,
+				plugin_cache.plugin_download_dir(&plugin_id).join(new_bin),
+			)?;
+		}
+	}
 
 	let plugin_kdl_path = plugin_cache.plugin_kdl(&plugin_id);
 	write_all(&plugin_kdl_path, &plugin_manifest.to_kdl_formatted_string()).map_err(|e| {