From 6a9b3e6c7ab6cbcb8a99d23049b3bf65d0f0f802 Mon Sep 17 00:00:00 2001 From: jlanson Date: Thu, 5 Sep 2024 09:32:22 -0400 Subject: [PATCH] feat: implement PolicyFile --> AnalysisTree conversion, remove use of WeightTree in scoring --- hipcheck/src/analysis/score.rs | 36 +++++----- hipcheck/src/config.rs | 112 +++++++++++++++++++++++++---- hipcheck/src/engine.rs | 5 +- hipcheck/src/plugin/mod.rs | 3 +- hipcheck/src/plugin/types.rs | 17 ++--- hipcheck/src/policy/mod.rs | 2 +- hipcheck/src/policy/policy_file.rs | 26 ++++--- hipcheck/src/session/mod.rs | 20 +++--- 8 files changed, 155 insertions(+), 66 deletions(-) diff --git a/hipcheck/src/analysis/score.rs b/hipcheck/src/analysis/score.rs index e1833d0c..2945907a 100644 --- a/hipcheck/src/analysis/score.rs +++ b/hipcheck/src/analysis/score.rs @@ -4,14 +4,15 @@ use crate::analysis::result::*; use crate::analysis::AnalysisOutcome; use crate::analysis::AnalysisProvider; use crate::config::{ - visit_leaves, Analysis, AnalysisTree, WeightTree, WeightTreeProvider, LEGACY_PLUGIN, - MITRE_PUBLISHER, + visit_leaves, Analysis, AnalysisTree, WeightTreeProvider, LEGACY_PLUGIN, MITRE_PUBLISHER, }; use crate::engine::HcEngine; use crate::error::Result; use crate::hc_error; +use crate::policy::PolicyFile; use crate::report::Concern; use crate::shell::spinner_phase::SpinnerPhase; +use crate::F64; use num_traits::identities::Zero; use serde_json::Value; use std::cmp::Ordering; @@ -209,7 +210,7 @@ impl ScoreTree { .get(analysis_root) .ok_or(hc_error!("AnalysisTree root not in tree, invalid state"))? .get() - .augment(&scores.table), + .augment_plugin(&scores.table), ); let mut scope: Vec = vec![score_root]; @@ -220,9 +221,9 @@ impl ScoreTree { analysis_tree .tree .get(n) - .ok_or(hc_error!("WeightTree root not in tree, invalid state"))? + .ok_or(hc_error!("AnalaysisTree node not in tree, invalid state"))? .get() - .augment(&scores.table), + .augment_plugin(&scores.table), ); scope .last() @@ -242,33 +243,33 @@ impl ScoreTree { }) } - // Given a weight tree and set of analysis results, produce an AltScoreTree by creating + // Given an analysis tree and set of analysis results, produce an AltScoreTree by creating // ScoreTreeNode objects for each analysis that was not skipped, and composing them into // a tree structure matching that of the WeightTree - pub fn synthesize(weight_tree: &WeightTree, scores: &AnalysisResults) -> Result { + pub fn synthesize(analysis_tree: &AnalysisTree, scores: &AnalysisResults) -> Result { use indextree::NodeEdge::*; let mut tree = Arena::::new(); - let weight_root = weight_tree.root; + let analysis_root = analysis_tree.root; let score_root = tree.new_node( - weight_tree + analysis_tree .tree - .get(weight_root) - .ok_or(hc_error!("WeightTree root not in tree, invalid state"))? + .get(analysis_root) + .ok_or(hc_error!("AnalysisTree root not in tree, invalid state"))? .get() - .augment(scores), + .augment(scores)?, ); let mut scope: Vec = vec![score_root]; - for edge in weight_root.traverse(&weight_tree.tree) { + for edge in analysis_root.traverse(&analysis_tree.tree) { match edge { Start(n) => { let curr_node = tree.new_node( - weight_tree + analysis_tree .tree .get(n) - .ok_or(hc_error!("WeightTree root not in tree, invalid state"))? + .ok_or(hc_error!("AnalysisTree node not in tree, invalid state"))? .get() - .augment(scores), + .augment(scores)?, ); scope .last() @@ -432,7 +433,6 @@ pub fn score_results(phase: &SpinnerPhase, db: &dyn ScoringProvider) -> Result Result Rc; + #[salsa::input] + fn policy(&self) -> Option>; /// Returns the token set in HC_GITHUB_TOKEN env var #[salsa::input] fn github_api_token(&self) -> Option>; @@ -605,7 +609,7 @@ pub struct Analysis { } #[derive(Debug, Clone, PartialEq, Eq)] -pub struct PoliciedAnalysis(pub Analysis, pub Expr); +pub struct PoliciedAnalysis(pub Analysis, pub String); #[derive(Debug, Clone, PartialEq, Eq)] pub enum AnalysisTreeNode { @@ -641,13 +645,35 @@ impl AnalysisTreeNode { weight, } } - pub fn analysis(analysis: Analysis, expr: Expr, weight: F64) -> Self { + pub fn analysis(analysis: Analysis, raw_policy: String, weight: F64) -> Self { AnalysisTreeNode::Analysis { - analysis: PoliciedAnalysis(analysis, expr), + analysis: PoliciedAnalysis(analysis, raw_policy), weight, } } - pub fn augment(&self, metrics: &HashMap>) -> ScoreTreeNode { + pub fn augment(&self, scores: &AnalysisResults) -> Result { + match self { + AnalysisTreeNode::Category { label, weight } => Ok(ScoreTreeNode { + label: label.clone(), + score: 0f64, + weight: (*weight).into(), + }), + AnalysisTreeNode::Analysis { analysis, weight } => { + let label = analysis.0.query.clone(); + let stored_res = scores.table.get(&label).ok_or(hc_error!( + "missing expected analysis results {}", + analysis.0.query + ))?; + let score = stored_res.score().0; + Ok(ScoreTreeNode { + label, + score: score as f64, + weight: (*weight).into(), + }) + } + } + } + pub fn augment_plugin(&self, metrics: &HashMap>) -> ScoreTreeNode { match self { AnalysisTreeNode::Category { label, weight } => ScoreTreeNode { label: label.clone(), @@ -713,13 +739,13 @@ impl AnalysisTree { &mut self, under: NodeId, analysis: Analysis, - policy: Expr, + raw_policy: String, weight: F64, ) -> Result { if self.node_is_category(under)? { let child = self .tree - .new_node(AnalysisTreeNode::analysis(analysis, policy, weight)); + .new_node(AnalysisTreeNode::analysis(analysis, raw_policy, weight)); under.append(child, &mut self.tree); Ok(child) } else { @@ -738,7 +764,7 @@ impl AnalysisTree { .get(weight_root) .ok_or(hc_error!("WeightTree root not in tree, invalid state"))? .get() - .with_hardcoded_expr(), + .as_category_node(), ); let mut scope: Vec = vec![analysis_root]; @@ -786,6 +812,7 @@ impl WeightTreeNode { weight, } } + #[allow(unused)] pub fn augment(&self, scores: &AnalysisResults) -> ScoreTreeNode { let score = match scores.table.get(&self.label) { Some(res) => res.score().0, @@ -806,7 +833,7 @@ impl WeightTreeNode { // @Temporary - until policy file impl'd and integrated, we hard-code // the policy for our analyses pub fn with_hardcoded_expr(&self) -> AnalysisTreeNode { - let expr = Expr::Primitive(Primitive::Bool(false)); + let expr = "true".to_owned(); let analysis = Analysis { publisher: MITRE_PUBLISHER.to_owned(), plugin: LEGACY_PLUGIN.to_owned(), @@ -882,7 +909,7 @@ where #[salsa::query_group(WeightTreeQueryStorage)] pub trait WeightTreeProvider: - FuzzConfigQuery + PracticesConfigQuery + AttacksConfigQuery + CommitConfigQuery + FuzzConfigQuery + PracticesConfigQuery + AttacksConfigQuery + CommitConfigQuery + HcEngine { /// Returns the tree of raw analysis weights from the config fn weight_tree(&self) -> Result>; @@ -895,9 +922,70 @@ pub trait WeightTreeProvider: fn normalized_analysis_tree(&self) -> Result>; } +fn add_analysis( + core: &dyn WeightTreeProvider, + tree: &mut AnalysisTree, + under: NodeId, + analysis: PolicyAnalysis, +) -> Result { + let publisher = analysis.name.publisher; + let plugin = analysis.name.name; + let weight = match analysis.weight { + Some(u) => F64::new(u as f64)?, + None => F64::new(1.0)?, + }; + let raw_policy = match analysis.policy_expression { + Some(x) => x, + None => core.default_policy_expr(publisher.clone(), plugin.clone())?.ok_or(hc_error!("plugin {}::{} does not have a default policy, please define a policy in your policy file"))? + }; + let analysis = Analysis { + publisher, + plugin, + query: "default".to_owned(), + }; + tree.add_analysis(under, analysis, raw_policy, weight) +} + +fn add_category( + core: &dyn WeightTreeProvider, + tree: &mut AnalysisTree, + under: NodeId, + category: &PolicyCategory, +) -> Result { + let weight = F64::new(match category.weight { + Some(w) => w as f64, + None => 1.0, + }) + .unwrap(); + let id = tree.add_category(under, category.name.as_str(), weight)?; + for c in category.children.iter() { + match c { + PolicyCategoryChild::Analysis(analysis) => { + add_analysis(core, tree, id, analysis.clone())?; + } + PolicyCategoryChild::Category(category) => { + add_category(core, tree, id, category)?; + } + } + } + Ok(id) +} + pub fn analysis_tree(db: &dyn WeightTreeProvider) -> Result> { - let weight_tree = db.weight_tree()?; - AnalysisTree::from_weight_tree(&weight_tree).map(Rc::new) + // @Todo - once ConfigFile-->PolicyFile implemented, deprecate else block + if let Some(policy) = db.policy() { + let mut tree = AnalysisTree::new("risk"); + let root = tree.root; + + for c in policy.analyze.categories.iter() { + add_category(db, &mut tree, root, c)?; + } + + Ok(Rc::new(tree)) + } else { + let weight_tree = db.weight_tree()?; + AnalysisTree::from_weight_tree(&weight_tree).map(Rc::new) + } } pub fn weight_tree(db: &dyn WeightTreeProvider) -> Result> { diff --git a/hipcheck/src/engine.rs b/hipcheck/src/engine.rs index 86340b4f..0a1b1b90 100644 --- a/hipcheck/src/engine.rs +++ b/hipcheck/src/engine.rs @@ -7,7 +7,6 @@ use crate::analysis::{ }, AnalysisProvider, }; -use crate::config::{visit_leaves, WeightTree, WeightTreeProvider}; use crate::metric::{review::PullReview, MetricProvider}; use crate::plugin::{ActivePlugin, PluginResponse}; pub use crate::plugin::{HcPluginCore, PluginExecutor, PluginWithConfig}; @@ -29,7 +28,7 @@ pub trait HcEngine: salsa::Database { #[salsa::input] fn core(&self) -> Arc; - fn default_policy_expr(&self, publisher: String, plugin: String) -> Result>; + fn default_policy_expr(&self, publisher: String, plugin: String) -> Result>; fn query(&self, publisher: String, plugin: String, query: String, key: Value) -> Result; } @@ -38,7 +37,7 @@ fn default_policy_expr( db: &dyn HcEngine, publisher: String, plugin: String, -) -> Result> { +) -> Result> { let core = db.core(); // @Todo - plugins map should be keyed on publisher too let Some(p_handle) = core.plugins.get(&plugin) else { diff --git a/hipcheck/src/plugin/mod.rs b/hipcheck/src/plugin/mod.rs index 7d4058ef..15125007 100644 --- a/hipcheck/src/plugin/mod.rs +++ b/hipcheck/src/plugin/mod.rs @@ -9,7 +9,6 @@ use crate::context::Context; use crate::kdl_helper::{extract_data, ParseKdlNode}; pub use crate::plugin::manager::*; pub use crate::plugin::types::*; -use crate::policy_exprs::Expr; use crate::Result; pub use download_manifest::{ ArchiveFormat, DownloadManifest, DownloadManifestEntry, HashAlgorithm, HashWithDigest, @@ -64,7 +63,7 @@ impl ActivePlugin { channel, } } - pub fn get_default_policy_expr(&self) -> Option<&Expr> { + pub fn get_default_policy_expr(&self) -> Option<&String> { self.channel.opt_default_policy_expr.as_ref() } async fn get_unique_id(&self) -> usize { diff --git a/hipcheck/src/plugin/types.rs b/hipcheck/src/plugin/types.rs index a906b860..a3e485b5 100644 --- a/hipcheck/src/plugin/types.rs +++ b/hipcheck/src/plugin/types.rs @@ -1,4 +1,3 @@ -use crate::policy_exprs::{parse, Expr}; use crate::{ hc_error, hipcheck::{ @@ -184,19 +183,17 @@ impl PluginContext { res.into_inner().try_into() } - pub async fn get_default_policy_expression(&mut self) -> Result> { + pub async fn get_default_policy_expression(&mut self) -> Result> { let req = GetDefaultPolicyExpressionRequest { empty: Some(Empty {}), }; let mut res = self.grpc.get_default_policy_expression(req).await?; - let expr_str = res.get_ref().policy_expression.as_str(); - if expr_str.is_empty() { - Ok(None) + let raw_expr = res.get_ref().policy_expression.clone(); + Ok(if raw_expr.is_empty() { + None } else { - parse(expr_str) - .map_err(|e| hc_error!("{}", e.to_string())) - .map(Some) - } + Some(raw_expr) + }) } pub async fn initiate_query_protocol( @@ -410,7 +407,7 @@ impl MultiplexedQueryReceiver { #[derive(Debug)] pub struct PluginTransport { pub schemas: HashMap, - pub opt_default_policy_expr: Option, + pub opt_default_policy_expr: Option, ctx: PluginContext, tx: mpsc::Sender, rx: Mutex, diff --git a/hipcheck/src/policy/mod.rs b/hipcheck/src/policy/mod.rs index 4f393489..54d3b8d7 100644 --- a/hipcheck/src/policy/mod.rs +++ b/hipcheck/src/policy/mod.rs @@ -2,7 +2,7 @@ //! Data types and functions for parsing policy KDL files -mod policy_file; +pub mod policy_file; mod tests; use crate::kdl_helper::extract_data; diff --git a/hipcheck/src/policy/policy_file.rs b/hipcheck/src/policy/policy_file.rs index 141b1e23..b7ab2fa9 100644 --- a/hipcheck/src/policy/policy_file.rs +++ b/hipcheck/src/policy/policy_file.rs @@ -174,10 +174,10 @@ impl ParseKdlNode for PolicyConfig { #[derive(Clone, Debug, PartialEq, Eq)] pub struct PolicyAnalysis { - name: PolicyPluginName, - policy_expression: Option, - weight: Option, - config: Option, + pub name: PolicyPluginName, + pub policy_expression: Option, + pub weight: Option, + pub config: Option, } impl PolicyAnalysis { @@ -240,9 +240,9 @@ impl ParseKdlNode for PolicyAnalysis { #[derive(Clone, Debug, PartialEq, Eq)] pub struct PolicyCategory { - name: String, - weight: Option, - children: Vec, + pub name: String, + pub weight: Option, + pub children: Vec, } impl PolicyCategory { @@ -306,7 +306,11 @@ impl ParseKdlNode for PolicyCategory { } } - Some(Self { name, weight, children }) + Some(Self { + name, + weight, + children, + }) } } @@ -379,7 +383,7 @@ impl ParseKdlNode for InvestigateIfFail { pub struct PolicyAnalyze { investigate_policy: InvestigatePolicy, if_fail: Option, - categories: Vec, + pub categories: Vec, } impl PolicyAnalyze { @@ -451,8 +455,8 @@ impl ParseKdlNode for PolicyAnalyze { #[derive(Clone, Debug, PartialEq, Eq)] pub struct PolicyPluginName { - publisher: String, - name: String, + pub publisher: String, + pub name: String, } impl PolicyPluginName { diff --git a/hipcheck/src/session/mod.rs b/hipcheck/src/session/mod.rs index b8ff3069..391b32a1 100644 --- a/hipcheck/src/session/mod.rs +++ b/hipcheck/src/session/mod.rs @@ -163,24 +163,26 @@ impl Session { // Check if a policy file was provided, otherwise use a config folder the old way // Currently this does nothing, as the PolicyFile is not actively parsed if policy_path.is_some() { - let (_policy, _policy_dir, _data_dir, _hc_github_token) = + let (policy, _policy_dir, _data_dir, _hc_github_token) = match load_policy_and_data(policy_path.as_deref(), data_path.as_deref()) { Ok(results) => results, Err(err) => return Err(err), }; // Needed if we use Salsa for this - // session.set_policy(Rc::new(policy)); - // session.set_policy_dir(Rc::new(policy_dir)); + session.set_policy(Some(Rc::new(policy))); + // session.set_policy_dir(Rc::new(policy_dir)); - // Set data folder location for module analysis - // session.set_data_dir(Arc::new(data_dir)); + // Set data folder location for module analysis + // session.set_data_dir(Arc::new(data_dir)); - // Set github token in salsa - // session.set_github_api_token(Some(Rc::new(hc_github_token))); + // Set github token in salsa + // session.set_github_api_token(Some(Rc::new(hc_github_token))); + } else { + session.set_policy(None); } - // Once we no longer need config information, put this in an else block until config has been deprecated - // Until then we need this for Hipcheck to still run + // Once we no longer need config information, put this in the above else block until config has + // been deprecated. Until then we need this for Hipcheck to still run 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,