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

chore: update analysis config to allow non-string values #540

Merged
merged 1 commit into from
Oct 29, 2024
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
8 changes: 6 additions & 2 deletions hipcheck/src/analysis/score.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,12 @@ pub struct PluginAnalysisResults {

impl PluginAnalysisResults {
pub fn get_legacy(&self, analysis: &str) -> Option<&PluginAnalysisResult> {
let key = Analysis::legacy(analysis);
self.table.get(&key)
if MITRE_LEGACY_PLUGINS.contains(&analysis) {
let key = Analysis::legacy(analysis);
self.table.get(&key)
} else {
None
}
}
/// Get all results from non-legacy analyses.
pub fn plugin_results(&self) -> impl Iterator<Item = (&Analysis, &PluginAnalysisResult)> {
Expand Down
39 changes: 31 additions & 8 deletions hipcheck/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use indextree::{Arena, NodeEdge, NodeId};
use num_traits::identities::Zero;
use pathbuf::pathbuf;
use serde::{Deserialize, Serialize};
use serde_json::Value;
use smart_default::SmartDefault;
use std::{
collections::HashMap,
Expand Down Expand Up @@ -835,7 +836,10 @@ fn langs_file(db: &dyn LanguagesConfigQuery) -> Result<Rc<PathBuf>> {
let policy_file = db.policy();
for opt in options {
if let Some(langs_config) = policy_file.get_config(opt) {
if let Some(filepath) = langs_config.get("langs-file") {
if let Some(v_filepath) = langs_config.get("langs-file") {
let Value::String(filepath) = v_filepath else {
return Err(hc_error!("'langs-file' was not a string"));
};
return Ok(Rc::new(Path::new(&filepath).to_path_buf()));
}
};
Expand All @@ -858,7 +862,10 @@ fn binary_formats_file(db: &dyn PracticesConfigQuery) -> Result<Rc<PathBuf>> {

let policy_file = db.policy();
if let Some(binary_config) = policy_file.get_config("mitre/binary") {
if let Some(filepath) = binary_config.get("binary-file") {
if let Some(v_filepath) = binary_config.get("binary-file") {
let Value::String(filepath) = v_filepath else {
return Err(hc_error!("'binary-file' was not a string"));
};
return Ok(Rc::new(Path::new(&filepath).to_path_buf()));
}
};
Expand All @@ -880,7 +887,10 @@ fn typo_file(db: &dyn AttacksConfigQuery) -> Result<Rc<PathBuf>> {

let policy_file = db.policy();
if let Some(typo_config) = policy_file.get_config("mitre/typo") {
if let Some(filepath) = typo_config.get("typo-file") {
if let Some(v_filepath) = typo_config.get("typo-file") {
let Value::String(filepath) = v_filepath else {
return Err(hc_error!("'typo-file' was not a string"));
};
return Ok(Rc::new(Path::new(&filepath).to_path_buf()));
}
};
Expand All @@ -902,7 +912,10 @@ fn orgs_file(db: &dyn CommitConfigQuery) -> Result<Rc<PathBuf>> {

let policy_file = db.policy();
if let Some(affiliation_config) = policy_file.get_config("mitre/affiliation") {
if let Some(filepath) = affiliation_config.get("orgs-file") {
if let Some(v_filepath) = affiliation_config.get("orgs-file") {
let Value::String(filepath) = v_filepath else {
return Err(hc_error!("'orgs-file' was not a string"));
};
return Ok(Rc::new(Path::new(&filepath).to_path_buf()));
}
};
Expand All @@ -913,8 +926,13 @@ fn orgs_file(db: &dyn CommitConfigQuery) -> Result<Rc<PathBuf>> {
fn contributor_trust_value_threshold(db: &dyn CommitConfigQuery) -> Result<u64> {
let policy_file = db.policy();
if let Some(trust_config) = policy_file.get_config("mitre/contributor-trust") {
if let Some(str_threshold) = trust_config.get("value-threshold") {
return str_threshold.parse::<u64>().map_err(|e| hc_error!("{}", e));
if let Some(v_threshold) = trust_config.get("value-threshold") {
let Value::Number(threshold) = v_threshold else {
return Err(hc_error!("'value-threshold' was not a number"));
};
return threshold
.as_u64()
.ok_or_else(|| hc_error!("'value-threshold' was too large to be a u64"));
}
};

Expand All @@ -924,8 +942,13 @@ fn contributor_trust_value_threshold(db: &dyn CommitConfigQuery) -> Result<u64>
fn contributor_trust_month_count_threshold(db: &dyn CommitConfigQuery) -> Result<u64> {
let policy_file = db.policy();
if let Some(trust_config) = policy_file.get_config("mitre/contributor-trust") {
if let Some(str_threshold) = trust_config.get("month-count-threshold") {
return str_threshold.parse::<u64>().map_err(|e| hc_error!("{}", e));
if let Some(v_threshold) = trust_config.get("month-count-threshold") {
let Value::Number(threshold) = v_threshold else {
return Err(hc_error!("'month-count-threshold' was not a number"));
};
return threshold
.as_u64()
.ok_or_else(|| hc_error!("'month-count-threshold' was too large to be a u64"));
}
};

Expand Down
3 changes: 1 addition & 2 deletions hipcheck/src/plugin/retrieval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,12 @@ use xz2::read::XzDecoder;
use super::get_current_arch;

/// The plugins currently are not delegated via the `plugin` system and are still part of `hipcheck` core
pub const MITRE_LEGACY_PLUGINS: [&str; 8] = [
pub const MITRE_LEGACY_PLUGINS: [&str; 7] = [
"activity",
"entropy",
"affiliation",
"binary",
"churn",
"identity",
"review",
"typo",
];
Expand Down
13 changes: 10 additions & 3 deletions hipcheck/src/policy/config_to_policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use crate::{
plugin::PluginVersion,
};

use serde_json::Value;
use std::collections::HashMap;
use url::Url;

Expand All @@ -42,7 +43,7 @@ pub fn config_to_policy(config: Config) -> Result<PolicyFile> {
PolicyPluginName::new("mitre/github_api")?,
PolicyConfig(HashMap::from_iter(vec![(
"api-token-var".to_owned(),
"HC_GITHUB_TOKEN".to_owned(),
Value::String("HC_GITHUB_TOKEN".to_owned()),
)])),
)]);

Expand Down Expand Up @@ -315,7 +316,10 @@ fn parse_typo(plugins: &mut PolicyPluginList, attacks: &mut PolicyCategory, typo
let file = typo.typo_file.clone();
let mut config = PolicyConfig::new();
config
.insert("typo-file".to_string(), format!("./config/{}", file))
.insert(
"typo-file".to_string(),
Value::String(format!("./config/{}", file)),
)
.unwrap();

// Add the plugin
Expand Down Expand Up @@ -357,7 +361,10 @@ fn parse_affiliation(
let file = affiliation.orgs_file.clone();
let mut config = PolicyConfig::new();
config
.insert("orgs-file".to_string(), format!("./config/{}", file))
.insert(
"orgs-file".to_string(),
Value::String(format!("./config/{}", file)),
)
.unwrap();

// Add the plugin
Expand Down
3 changes: 2 additions & 1 deletion hipcheck/src/policy/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use crate::{
util::kdl::extract_data,
};
use kdl::KdlDocument;
use serde_json::Value;
use std::{collections::HashMap, path::Path, str::FromStr};

#[derive(Clone, Debug, PartialEq, Eq)]
Expand Down Expand Up @@ -74,7 +75,7 @@ impl PolicyFile {
// that returns plugin configs as a "view" of the combined analysis/patch
// sections
#[allow(unused)]
pub fn get_config(&self, analysis_name: &str) -> Option<HashMap<String, String>> {
pub fn get_config(&self, analysis_name: &str) -> Option<HashMap<String, Value>> {
let opt_conf = self
.analyze
.find_analysis_by_name(analysis_name)
Expand Down
34 changes: 30 additions & 4 deletions hipcheck/src/policy/policy_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use crate::{
};

use kdl::KdlNode;
use serde_json::Value;
use std::{collections::HashMap, fmt, fmt::Display, path::PathBuf};
use url::Url;

Expand Down Expand Up @@ -149,15 +150,34 @@ impl ParseKdlNode for PolicyPluginList {
}
}

fn try_to_serde_json(value: &kdl::KdlValue) -> Result<Value> {
use kdl::KdlValue::*;
let value = value.clone();
Ok(match value {
RawString(s) => Value::String(s),
String(s) => Value::String(s),
Base2(i) | Base8(i) | Base10(i) | Base16(i) => Value::Number(
serde_json::Number::from_i128(i.into())
.ok_or_else(|| hc_error!("kdl value contained an extremely large int"))?,
),
Base10Float(f) => Value::Number(
serde_json::Number::from_f64(f)
.ok_or_else(|| hc_error!("kdl value contained infinite or NaN float"))?,
),
Bool(b) => Value::Bool(b),
Null => Value::Null,
})
}

#[derive(Clone, Debug, PartialEq, Eq, Default)]
pub struct PolicyConfig(pub HashMap<String, String>);
pub struct PolicyConfig(pub HashMap<String, Value>);

impl PolicyConfig {
pub fn new() -> Self {
Self(HashMap::new())
}

pub fn insert(&mut self, description: String, info: String) -> Result<()> {
pub fn insert(&mut self, description: String, info: Value) -> Result<()> {
match self.0.insert(description.clone(), info) {
Some(_duplicate_key) => Err(hc_error!(
"Duplicate configuration information specified for {}",
Expand All @@ -173,7 +193,7 @@ impl PolicyConfig {
}

#[allow(dead_code)]
pub fn iter(&self) -> impl Iterator<Item = (&String, &String)> {
pub fn iter(&self) -> impl Iterator<Item = (&String, &Value)> {
self.0.iter()
}
}
Expand All @@ -189,7 +209,13 @@ impl ParseKdlNode for PolicyConfig {
let description = node.name().to_string();
if let Some(info) = node.entries().first() {
if config
.insert(description.clone(), info.value().as_string()?.to_string())
.insert(
description.clone(),
try_to_serde_json(info.value()).unwrap_or_else(|e| {
log::error!("error converting KDL node to serde_json::Value: {e:?}");
Value::Null
}),
)
.is_err()
{
log::error!(
Expand Down
31 changes: 25 additions & 6 deletions hipcheck/src/policy/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ mod test {

use kdl::KdlNode;
use pathbuf::pathbuf;
use serde_json::Value;
use std::{env, str::FromStr};
use url::Url;

Expand Down Expand Up @@ -96,7 +97,10 @@ mod test {

let mut config = PolicyConfig::new();
config
.insert("typo-file".to_string(), "./config/typo.kdl".to_string())
.insert(
"typo-file".to_string(),
Value::String("./config/typo.kdl".to_string()),
)
.unwrap();

let expected = PolicyAnalysis::new(
Expand All @@ -118,7 +122,10 @@ mod test {

let mut config = PolicyConfig::new();
config
.insert("typo-file".to_string(), "./config/typo.kdl".to_string())
.insert(
"typo-file".to_string(),
Value::String("./config/typo.kdl".to_string()),
)
.unwrap();

let expected = PolicyAnalysis::new(
Expand All @@ -141,10 +148,16 @@ mod test {

let mut config = PolicyConfig::new();
config
.insert("typo-file".to_string(), "./config/typo.kdl".to_string())
.insert(
"typo-file".to_string(),
Value::String("./config/typo.kdl".to_string()),
)
.unwrap();
config
.insert("typo-file-2".to_string(), "./config/typo2.kdl".to_string())
.insert(
"typo-file-2".to_string(),
Value::String("./config/typo2.kdl".to_string()),
)
.unwrap();

let expected = PolicyAnalysis::new(
Expand Down Expand Up @@ -207,12 +220,18 @@ mod test {

let mut affiliation_config = PolicyConfig::new();
affiliation_config
.insert("orgs-file".to_string(), "./config/orgs.kdl".to_string())
.insert(
"orgs-file".to_string(),
Value::String("./config/orgs.kdl".to_string()),
)
.unwrap();

let mut typo_config = PolicyConfig::new();
typo_config
.insert("typo-file".to_string(), "./config/typo.kdl".to_string())
.insert(
"typo-file".to_string(),
Value::String("./config/typo.kdl".to_string()),
)
.unwrap();

let mut commit = PolicyCategory::new("commit".to_string(), None);
Expand Down
4 changes: 2 additions & 2 deletions plugins/identity/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use std::{
#[derive(Deserialize)]
struct Config {
#[serde(rename = "percent-threshold")]
percent_threshold: Option<u64>,
percent_threshold: Option<f64>,
}

#[derive(Debug, Clone, PartialEq, Eq, Hash, Deserialize, Serialize, JsonSchema)]
Expand Down Expand Up @@ -100,7 +100,7 @@ async fn identity(engine: &mut PluginEngine, key: Target) -> Result<Vec<bool>> {

#[derive(Clone, Debug, Default)]
struct IdentityPlugin {
policy_conf: OnceLock<Option<u64>>,
policy_conf: OnceLock<Option<f64>>,
}

impl Plugin for IdentityPlugin {
Expand Down