feat: introduce type checking in hc check

A previous commit introduced a typing system for policy expressions
so that we can detect malformed expressions before execution. However,
policy expressions from policy files and from plugin defaults were
still being stored as String objects until after analysis. This commit
causes expressions to be stored as Expr types and introduces a standard
pipeline of pre-processing passes to run on parsed Expr types.

Signed-off-by: jlanson <[email protected]>

hipcheck/Cargo.toml

@@ -141,6 +141,7 @@ zip = "2.2.1"
 zip-extensions = "0.8.1"
 zstd = "0.13.2"
 hipcheck-common = { version = "0.1.0", path = "../hipcheck-common" }
+serde_with = "3.11.0"
 
 [build-dependencies]
 

hipcheck/src/config.rs

@@ -10,6 +10,7 @@ use crate::{
 		policy_file::{PolicyAnalysis, PolicyCategory, PolicyCategoryChild},
 		PolicyFile,
 	},
+	policy_exprs::{std_parse, Expr},
 	score::*,
 	util::fs as file,
 	BINARY_CONFIG_FILE, F64, LANGS_FILE, ORGS_FILE, TYPO_FILE,
@@ -468,7 +469,7 @@ pub trait ConfigSource: salsa::Database {
 #[salsa::query_group(RiskConfigQueryStorage)]
 pub trait RiskConfigQuery: ConfigSource {
 	/// Returns the risk policy expr
-	fn risk_policy(&self) -> Rc<String>;
+	fn risk_policy(&self) -> Result<Rc<Expr>>;
 }
 
 /// Query for accessing the languages analysis config
@@ -522,7 +523,7 @@ pub struct Analysis {
 }
 
 #[derive(Debug, Clone, PartialEq, Eq)]
-pub struct PoliciedAnalysis(pub Analysis, pub String);
+pub struct PoliciedAnalysis(pub Analysis, pub Option<Expr>);
 
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum AnalysisTreeNode {
@@ -566,9 +567,9 @@ impl AnalysisTreeNode {
 			weight,
 		}
 	}
-	pub fn analysis(analysis: Analysis, raw_policy: String, weight: F64) -> Self {
+	pub fn analysis(analysis: Analysis, opt_policy: Option<Expr>, weight: F64) -> Self {
 		AnalysisTreeNode::Analysis {
-			analysis: PoliciedAnalysis(analysis, raw_policy),
+			analysis: PoliciedAnalysis(analysis, opt_policy),
 			weight,
 		}
 	}
@@ -653,13 +654,13 @@ impl AnalysisTree {
 		&mut self,
 		under: NodeId,
 		analysis: Analysis,
-		raw_policy: String,
+		opt_policy: Option<Expr>,
 		weight: F64,
 	) -> Result<NodeId> {
 		if self.node_is_category(under)? {
 			let child = self
 				.tree
-				.new_node(AnalysisTreeNode::analysis(analysis, raw_policy, weight));
+				.new_node(AnalysisTreeNode::analysis(analysis, opt_policy, weight));
 			under.append(child, &mut self.tree);
 			Ok(child)
 		} else {
@@ -748,16 +749,16 @@ fn add_analysis(
 		Some(u) => F64::new(u as f64)?,
 		None => F64::new(1.0)?,
 	};
-	let raw_policy = match analysis.policy_expression {
-		Some(x) => x,
-		None => "".to_owned(),
-	};
+	let opt_policy = analysis
+		.policy_expression
+		.map(|s| s.parse::<Expr>())
+		.transpose()?;
 	let analysis = Analysis {
 		publisher: publisher.0,
 		plugin: plugin.0,
 		query: DEFAULT_QUERY.to_owned(),
 	};
-	tree.add_analysis(under, analysis, raw_policy, weight)
+	tree.add_analysis(under, analysis, opt_policy, weight)
 }
 
 fn add_category(
@@ -815,8 +816,8 @@ pub fn analysis_tree(db: &dyn WeightTreeProvider) -> Result<Rc<AnalysisTree>> {
 	let update_policy = |node: &mut AnalysisTreeNode| -> Result<()> {
 		if let AnalysisTreeNode::Analysis { analysis, .. } = node {
 			let a: &Analysis = &analysis.0;
-			if analysis.1.is_empty() {
-				analysis.1 = db.default_policy_expr(a.publisher.clone(), a.plugin.clone())?.ok_or(hc_error!("plugin {}::{} does not have a default policy, please define a policy in your policy file", a.publisher.clone(), a.plugin.clone()))?;
+			if analysis.1.is_none() {
+				analysis.1 = Some(db.default_policy_expr(a.publisher.clone(), a.plugin.clone())?.ok_or(hc_error!("plugin {}::{} does not have a default policy, please define a policy in your policy file", a.publisher.clone(), a.plugin.clone()))?);
 			}
 		}
 		Ok(())
@@ -866,9 +867,13 @@ pub fn normalized_unresolved_analysis_tree(db: &dyn ConfigSource) -> Result<Rc<A
 // field is `String`, it is returned wrapped in an `Rc`.  This is
 // done to keep Salsa's cloning cheap.
 
-fn risk_policy(db: &dyn RiskConfigQuery) -> Rc<String> {
+fn risk_policy(db: &dyn RiskConfigQuery) -> Result<Rc<Expr>> {
 	let policy = db.policy();
-	Rc::new(policy.analyze.investigate_policy.0.clone())
+	let expr_str = policy.analyze.investigate_policy.0.as_str();
+	let expr = std_parse(expr_str)
+		.map_err(|e| hc_error!("Malformed risk policy expression '{}': {}", expr_str, e))?;
+
+	Ok(Rc::new(expr))
 }
 
 fn langs_file_rel(_db: &dyn LanguagesConfigQuery) -> Rc<String> {

hipcheck/src/engine.rs

@@ -9,6 +9,7 @@ use crate::{
 		QueryResult,
 	},
 	policy::PolicyFile,
+	policy_exprs::Expr,
 	Result,
 };
 use futures::future::{BoxFuture, FutureExt};
@@ -27,7 +28,7 @@ pub trait HcEngine: salsa::Database {
 	#[salsa::input]
 	fn core(&self) -> Arc<HcPluginCore>;
 
-	fn default_policy_expr(&self, publisher: String, plugin: String) -> Result<Option<String>>;
+	fn default_policy_expr(&self, publisher: String, plugin: String) -> Result<Option<Expr>>;
 
 	fn default_query_explanation(
 		&self,
@@ -48,7 +49,7 @@ fn default_policy_expr(
 	db: &dyn HcEngine,
 	publisher: String,
 	plugin: String,
-) -> Result<Option<String>> {
+) -> Result<Option<Expr>> {
 	let core = db.core();
 	let key = get_plugin_key(publisher.as_str(), plugin.as_str());
 	let Some(p_handle) = core.plugins.get(&key) else {

hipcheck/src/plugin/mod.rs

@@ -10,6 +10,7 @@ mod types;
 
 use crate::error::Result;
 pub use crate::plugin::{get_plugin_key, manager::*, plugin_id::PluginId, types::*};
+use crate::policy_exprs::Expr;
 pub use arch::{get_current_arch, try_set_arch, Arch};
 pub use download_manifest::{ArchiveFormat, DownloadManifest, HashAlgorithm, HashWithDigest};
 use hipcheck_common::types::{Query, QueryDirection};
@@ -55,7 +56,7 @@ impl ActivePlugin {
 		}
 	}
 
-	pub fn get_default_policy_expr(&self) -> Option<&String> {
+	pub fn get_default_policy_expr(&self) -> Option<&Expr> {
 		self.channel.opt_default_policy_expr.as_ref()
 	}
 

hipcheck/src/plugin/types.rs

@@ -1,6 +1,10 @@
 // SPDX-License-Identifier: Apache-2.0
 
-use crate::{hc_error, Result};
+use crate::{
+	hc_error,
+	policy_exprs::{std_parse, Expr},
+	Result,
+};
 use futures::{Stream, StreamExt};
 use hipcheck_common::proto::{
 	plugin_service_client::PluginServiceClient, ConfigurationStatus, Empty,
@@ -284,7 +288,11 @@ impl PluginContext {
 
 		self.set_configuration(&config).await?.as_result()?;
 
-		let opt_default_policy_expr = self.get_default_policy_expression().await?;
+		let opt_default_policy_expr = self
+			.get_default_policy_expression()
+			.await?
+			.map(|s| std_parse(s.as_str()))
+			.transpose()?;
 
 		let opt_explain_default_query = self.explain_default_query().await?;
 
@@ -387,7 +395,7 @@ impl MultiplexedQueryReceiver {
 #[derive(Debug)]
 pub struct PluginTransport {
 	pub schemas: HashMap<String, Schema>,
-	pub opt_default_policy_expr: Option<String>,
+	pub opt_default_policy_expr: Option<Expr>,
 	pub opt_explain_default_query: Option<String>,
 	ctx: PluginContext,
 	tx: mpsc::Sender<PluginQuery>,

hipcheck/src/policy_exprs/expr.rs

@@ -15,6 +15,7 @@ use nom::{
 	Finish as _, IResult,
 };
 use ordered_float::NotNan;
+use serde_with::{DeserializeFromStr, SerializeDisplay};
 use std::{
 	cmp::Ordering,
 	fmt::Display,
@@ -27,7 +28,7 @@ use std::{
 use jiff::civil::Date;
 
 /// A `deke` expression to evaluate.
-#[derive(Debug, PartialEq, Eq, Clone)]
+#[derive(Debug, PartialEq, Eq, Clone, SerializeDisplay, DeserializeFromStr)]
 pub enum Expr {
 	/// Primitive data (ints, floats, bool).
 	Primitive(Primitive),
@@ -453,7 +454,7 @@ impl Display for Expr {
 				)
 			}
 			Expr::Function(func) => func.fmt(f),
-			Expr::Lambda(l) => write!(f, "(lambda ({}) {})", l.arg, l.body),
+			Expr::Lambda(l) => l.fmt(f),
 			Expr::JsonPointer(pointer) => write!(f, "${}", pointer.pointer),
 		}
 	}
@@ -472,6 +473,22 @@ impl Display for Primitive {
 	}
 }
 
+impl Display for Lambda {
+	fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+		let mut out: Vec<String> = vec![self.body.ident.to_string()];
+		// Filter out references to placeholder var
+		let arg: Expr = Primitive::Identifier(self.arg.clone()).into();
+		out.extend(self.body.args.iter().filter_map(|x| {
+			if *x != arg {
+				Some(ToString::to_string(x))
+			} else {
+				None
+			}
+		}));
+		write!(f, "({})", out.join(" "))
+	}
+}
+
 impl Primitive {
 	pub fn resolve(&self, env: &Env) -> Result<Primitive> {
 		match self {
@@ -491,8 +508,9 @@ impl Primitive {
 
 impl Display for Function {
 	fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-		let args = self.args.iter().map(ToString::to_string).join(" ");
-		write!(f, "({} {})", self.ident, args)
+		let mut out: Vec<String> = vec![self.ident.to_string()];
+		out.extend(self.args.iter().map(ToString::to_string));
+		write!(f, "({})", out.join(" "))
 	}
 }