Skip to content

Commit

Permalink
feat (cli): Adds new --policy flag with default to pass policy file t…
Browse files Browse the repository at this point in the history
…o Hipcheck. Currently errors if a file is passed.
  • Loading branch information
mchernicoff authored and alilleybrinker committed Aug 22, 2024
1 parent d71c1cc commit 667d17b
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 5 deletions.
49 changes: 46 additions & 3 deletions hipcheck/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use crate::target::{
use clap::{Parser as _, ValueEnum};
use hipcheck_macros as hc;
use pathbuf::pathbuf;
use std::path::{Path, PathBuf};
use std::{env, path::{Path, PathBuf}};
use url::Url;

/// Automatated supply chain risk assessment of software packages.
Expand Down Expand Up @@ -109,6 +109,16 @@ struct PathArgs {
long_help = "Path to the cache folder. Can also be set with the `HC_CACHE` environment variable"
)]
cache: Option<PathBuf>,

/// Path to the policy file
#[arg(
short = 'p',
long = "policy",
global = true,
help_heading = "Path Flags",
long_help = "Path to the policy file."
)]
policy: Option<PathBuf>,
}

/// Soft-deprecated arguments, to be removed in a future version.
Expand Down Expand Up @@ -231,6 +241,11 @@ impl CliConfig {
}
}

/// Get the path to the policy file.
pub fn policy(&self) -> Option<&Path> {
self.path_args.policy.as_deref()
}

/// Check if the `--print-home` flag was used.
pub fn print_home(&self) -> bool {
self.deprecated_args.print_home.unwrap_or(false)
Expand Down Expand Up @@ -274,6 +289,8 @@ impl CliConfig {
config: hc_env_var("config"),
data: hc_env_var("data"),
cache: hc_env_var("cache"),
// For now, we do not get this from the environment, so pass a None to never update this field
policy: None,
},
deprecated_args: DeprecatedArgs {
home: hc_env_var("home"),
Expand All @@ -293,6 +310,8 @@ impl CliConfig {
cache: platform_cache(),
config: platform_config(),
data: platform_data(),
// There is no central per-user or per-system location for the policy file, so pass a None to never update this field
policy: None,
},
..Default::default()
}
Expand All @@ -305,6 +324,7 @@ impl CliConfig {
config: dirs::home_dir().map(|dir| pathbuf![&dir, "hipcheck", "config"]),
data: dirs::home_dir().map(|dir| pathbuf![&dir, "hipcheck", "data"]),
cache: dirs::home_dir().map(|dir| pathbuf![&dir, "hipcheck", "cache"]),
policy: env::current_dir().ok().map(|dir| pathbuf![&dir, "Hipcheck.kdl"]),
},
..Default::default()
}
Expand Down Expand Up @@ -839,7 +859,7 @@ pub struct CliCacheListArgs {
#[arg(short = 'm', long)]
pub max: Option<usize>,
/// Consider only entries matching this pattern
#[arg(short = 'p', long = "pattern")]
#[arg(short = 'P', long = "pattern")]
pub filter: Option<String>,
}
impl From<CliCacheListArgs> for CacheOp {
Expand All @@ -866,7 +886,7 @@ pub struct CliCacheDeleteArgs {
#[arg(short = 's', long, num_args=1..=2, value_delimiter = ' ')]
pub strategy: Vec<String>,
/// Consider only entries matching this pattern
#[arg(short = 'p', long = "pattern")]
#[arg(short = 'P', long = "pattern")]
pub filter: Option<String>,
/// Do not prompt user to confirm the entries to delete
#[arg(long, default_value_t = false)]
Expand Down Expand Up @@ -1181,6 +1201,29 @@ mod tests {
});
}

#[test]
fn resolve_policy_with_flag() {
let tempdir = TempDir::with_prefix(TEMPDIR_PREFIX).unwrap();

let expected = pathbuf![tempdir.path(), "HipcheckPolicy.kdl"];

let config = {
let mut temp = CliConfig::empty();
temp.update(&CliConfig::from_platform());
temp.update(&CliConfig::from_env());
temp.update(&CliConfig {
path_args: PathArgs {
policy: Some(expected.clone()),
..Default::default()
},
..Default::default()
});
temp
};

assert_eq!(config.policy().unwrap(), expected);
}

#[test]
fn hc_check_schema_no_args_gives_help() {
let check_args = vec!["hc", "check"];
Expand Down
27 changes: 26 additions & 1 deletion hipcheck/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ fn cmd_check(args: &CheckArgs, config: &CliConfig) -> ExitCode {
config.config().map(ToOwned::to_owned),
config.data().map(ToOwned::to_owned),
config.cache().map(ToOwned::to_owned),
config.policy().map(ToOwned::to_owned),
config.format(),
raw_version,
);
Expand Down Expand Up @@ -224,6 +225,7 @@ fn cmd_print_weights(config: &CliConfig) -> Result<()> {
config.config().map(ToOwned::to_owned),
config.data().map(ToOwned::to_owned),
config.cache().map(ToOwned::to_owned),
config.policy().map(ToOwned::to_owned),
config.format(),
raw_version,
)?;
Expand Down Expand Up @@ -445,6 +447,7 @@ struct ReadyChecks {
config_path_check: StdResult<PathBuf, PathCheckError>,
data_path_check: StdResult<PathBuf, PathCheckError>,
cache_path_check: StdResult<PathBuf, PathCheckError>,
policy_path_check: StdResult<PathBuf, PathCheckError>,
github_token_check: StdResult<(), EnvVarCheckError>,
}

Expand All @@ -459,6 +462,7 @@ impl ReadyChecks {
&& self.config_path_check.is_ok()
&& self.data_path_check.is_ok()
&& self.cache_path_check.is_ok()
&& self.policy_path_check.is_ok()
}
}

Expand Down Expand Up @@ -492,12 +496,14 @@ enum VersionCheckErrorKind {
#[derive(Debug)]
enum PathCheckError {
PathNotFound,
PolicyNotFound,
}

impl Display for PathCheckError {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
match self {
PathCheckError::PathNotFound => write!(f, "path not found"),
PathCheckError::PathNotFound => write!(f, "Path not found"),
PathCheckError::PolicyNotFound => write!(f, "Policy file not found. Specify the location of a policy file using the --policy flag.")
}
}
}
Expand Down Expand Up @@ -606,6 +612,16 @@ fn check_data_path(config: &CliConfig) -> StdResult<PathBuf, PathCheckError> {
Ok(path.to_owned())
}

fn check_policy_path(config: &CliConfig) -> StdResult<PathBuf, PathCheckError> {
let path = config.policy().ok_or(PathCheckError::PathNotFound)?;

if path.exists().not() {
return Err(PathCheckError::PolicyNotFound);
}

Ok(path.to_owned())
}

/// Check that a GitHub token has been provided as an environment variable
/// This does not check if the token is valid or not
/// The absence of a token does not trigger the failure state for the readiness check, because
Expand All @@ -629,6 +645,7 @@ fn cmd_ready(config: &CliConfig) {
config_path_check: check_config_path(config),
data_path_check: check_data_path(config),
cache_path_check: check_cache_path(config),
policy_path_check: check_policy_path(config),
github_token_check: check_github_token(),
};

Expand Down Expand Up @@ -662,6 +679,12 @@ fn cmd_ready(config: &CliConfig) {
Err(e) => println!("{:<17} {}", "Data Path:", e),
}

match &ready.policy_path_check {
Ok(path) => println!("{:<17} {}", "Policy Path:", path.display()),
Err(e) => println!("{:<17} {}", "Policy Path:", e),
}


match &ready.github_token_check {
Ok(_) => println!("{:<17} Found!", "GitHub Token:"),
Err(e) => println!("{:<17} {}", "GitHub Token:", e),
Expand Down Expand Up @@ -864,6 +887,7 @@ fn run(
config_path: Option<PathBuf>,
data_path: Option<PathBuf>,
home_dir: Option<PathBuf>,
policy_path: Option<PathBuf>,
format: Format,
raw_version: &str,
) -> Result<AnyReport> {
Expand All @@ -873,6 +897,7 @@ fn run(
config_path,
data_path,
home_dir,
policy_path,
format,
raw_version,
) {
Expand Down
9 changes: 8 additions & 1 deletion hipcheck/src/session/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ impl Session {
config_path: Option<PathBuf>,
data_path: Option<PathBuf>,
home_dir: Option<PathBuf>,
policy_path: Option<PathBuf>,
format: Format,
raw_version: &str,
) -> StdResult<Session, Error> {
Expand Down Expand Up @@ -158,6 +159,12 @@ impl Session {
* Loading configuration.
*-----------------------------------------------------------------*/

// Check if a currently unsuporrted policy file was provided
// TODO: Remove this error once policy files are supported
if policy_path.is_some() {
return Err(hc_error!("Policy files are not supported by Hipcheck at this time."))
}

let (config, config_dir, data_dir, hc_github_token) =
match load_config_and_data(config_path.as_deref(), data_path.as_deref()) {
Ok(results) => results,
Expand Down Expand Up @@ -241,7 +248,7 @@ fn load_config_and_data(
phase.inc();
// Set the spinner phase to tick constantly, 10 times a second.
phase.enable_steady_tick(Duration::from_millis(100));

// Resolve the path to the config file.
let valid_config_path = config_path
.ok_or_else(|| hc_error!("Failed to load configuration. Please make sure the path set by the hc_config env variable exists."))?;
Expand Down

0 comments on commit 667d17b

Please sign in to comment.