index dependency kinds correctly

Signed-off-by: Markus Theil <theil.markus@gmail.com>

Author: thillux

cargo-cyclonedx/src/generator.rs

@@ -1,4 +1,5 @@
 use crate::config::Describe;
+use std::cmp::min;
 /*
  * This file is part of CycloneDX Rust Cargo.
  *
@@ -54,8 +55,8 @@ use once_cell::sync::Lazy;
 use regex::Regex;
 
 use log::Level;
-use std::collections::BTreeMap;
 use std::collections::HashMap;
+use std::collections::{BTreeMap, LinkedList};
 use std::convert::TryFrom;
 use std::fs::File;
 use std::io::BufWriter;
@@ -68,7 +69,42 @@ use validator::validate_email;
 // Maps from PackageId to Package for efficiency - faster lookups than in a Vec
 type PackageMap = BTreeMap<PackageId, Package>;
 type ResolveMap = BTreeMap<PackageId, Node>;
-type DependencyKindMap = BTreeMap<PackageId, Vec<DependencyKind>>;
+type DependencyKindMap = BTreeMap<PackageId, DependencyKind>;
+
+/// The values are ordered from weakest to strongest so that casting to integer would make sense
+#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Copy, Clone)]
+enum PrivateDepKind {
+    Development,
+    Build,
+    Runtime,
+}
+impl From<PrivateDepKind> for DependencyKind {
+    fn from(priv_kind: PrivateDepKind) -> Self {
+        match priv_kind {
+            PrivateDepKind::Development => DependencyKind::Development,
+            PrivateDepKind::Build => DependencyKind::Build,
+            PrivateDepKind::Runtime => DependencyKind::Normal,
+        }
+    }
+}
+
+impl From<&DependencyKind> for PrivateDepKind {
+    fn from(kind: &DependencyKind) -> Self {
+        match kind {
+            DependencyKind::Normal => PrivateDepKind::Runtime,
+            DependencyKind::Development => PrivateDepKind::Development,
+            DependencyKind::Build => PrivateDepKind::Build,
+            _ => panic!("Unknown dependency kind"),
+        }
+    }
+}
+
+fn strongest_dep_kind(deps: &[cargo_metadata::DepKindInfo]) -> PrivateDepKind {
+    deps.iter()
+        .map(|d| PrivateDepKind::from(&d.kind))
+        .max()
+        .unwrap_or(PrivateDepKind::Runtime) // for compatibility with Rust earlier than 1.41
+}
 
 pub struct SbomGenerator {
     config: SbomConfig,
@@ -99,13 +135,13 @@ impl SbomGenerator {
         for member in members.iter() {
             log::trace!("Processing the package {}", member);
 
-            let mut dep_kinds = DependencyKindMap::new();
+            let dep_kinds = index_dep_kinds(member, &resolve);
 
             let (dependencies, pruned_resolve) =
                 if config.included_dependencies() == IncludedDependencies::AllDependencies {
-                    all_dependencies(member, &packages, &resolve, &mut dep_kinds, config)
+                    all_dependencies(member, &packages, &resolve, config)
                 } else {
-                    top_level_dependencies(member, &packages, &resolve, &mut dep_kinds, config)
+                    top_level_dependencies(member, &packages, &resolve, config)
                 };
 
             let manifest_path = packages[member].manifest_path.clone().into_std_path_buf();
@@ -199,14 +235,12 @@ impl SbomGenerator {
         );
 
         component.purl = purl;
-        component.scope = if let Some(deps) = dep_kinds.get(&package.id) {
-            if deps.iter().any(|d| *d == DependencyKind::Normal) {
-                Some(Scope::Required)
-            } else {
-                Some(Scope::Excluded)
-            }
-        } else {
-            Some(Scope::Required)
+        component.scope = match dep_kinds
+            .get(&package.id)
+            .unwrap_or(&DependencyKind::Normal)
+        {
+            DependencyKind::Normal => Some(Scope::Required),
+            _ => Some(Scope::Excluded),
         };
         component.external_references = Self::get_external_references(package);
         component.licenses = self.get_licenses(package);
@@ -559,6 +593,49 @@ fn index_resolve(packages: Vec<Node>) -> ResolveMap {
         .collect()
 }
 
+fn index_dep_kinds(root: &PackageId, resolve: &ResolveMap) -> DependencyKindMap {
+    // cache strongest found dependency kind for every node
+    let mut id_to_dep_kind: HashMap<&PackageId, PrivateDepKind> = HashMap::new();
+    id_to_dep_kind.insert(root, PrivateDepKind::Runtime);
+
+    let root_node = &resolve[root];
+    let mut nodes_to_visit = LinkedList::<&Node>::new();
+    nodes_to_visit.push_front(root_node);
+
+    // perform a simple iterative DFS over the dependencies,
+    // mark child deps with the minimum of parent kind and their own strongest value
+    // therefore e.g. mark decendants of build dependencies as build dependencies,
+    // as long as they never occur as normal dependency.
+    while !nodes_to_visit.is_empty() {
+        let node = nodes_to_visit.pop_front().unwrap();
+        let parent_node_kind = id_to_dep_kind[&node.id];
+
+        for child_dep in &node.deps {
+            let child_node = &resolve[&child_dep.pkg];
+            let dep_kind = strongest_dep_kind(child_dep.dep_kinds.as_slice());
+            let child_node_kind = min(parent_node_kind, dep_kind);
+
+            let dep_kind_on_previous_visit = id_to_dep_kind.get(&child_node.id);
+            // insert/update a nodes dependency kind, when its new or stronger than the previous value
+            if dep_kind_on_previous_visit.is_none()
+                || child_node_kind > *dep_kind_on_previous_visit.unwrap()
+            {
+                let _ = id_to_dep_kind.insert(&child_node.id, child_node_kind);
+            }
+
+            // cargo metadata already return a DAG, so assume this will terminate
+            // and don't mark already visited notes. It's ok and necessary to visit
+            // deps as often as they occur.
+            nodes_to_visit.push_front(child_node);
+        }
+    }
+
+    id_to_dep_kind
+        .iter()
+        .map(|(x, y)| ((*x).clone(), DependencyKind::from(*y)))
+        .collect()
+}
+
 #[derive(Error, Debug)]
 pub enum GeneratorError {
     #[error("Expected a root package in the cargo config: {config_filepath}")]
@@ -601,13 +678,12 @@ fn top_level_dependencies(
     root: &PackageId,
     packages: &PackageMap,
     resolve: &ResolveMap,
-    dep_kinds: &mut DependencyKindMap,
     config: &SbomConfig,
 ) -> (PackageMap, ResolveMap) {
     log::trace!("Adding top-level dependencies to SBOM");
 
     // Only include packages that have dependency kinds other than "Development"
-    let root_node = add_filtered_dependencies(&resolve[root], config, dep_kinds);
+    let root_node = add_filtered_dependencies(&resolve[root], config);
 
     let mut pkg_result = PackageMap::new();
 
@@ -635,7 +711,6 @@ fn all_dependencies(
     root: &PackageId,
     packages: &PackageMap,
     resolve: &ResolveMap,
-    dep_kinds: &mut DependencyKindMap,
     config: &SbomConfig,
 ) -> (PackageMap, ResolveMap) {
     log::trace!("Adding all dependencies to SBOM");
@@ -657,14 +732,10 @@ fn all_dependencies(
             // If we haven't processed this node yet...
             if !out_resolve.contains_key(&node.id) {
                 // Add the node to the output
-                out_resolve.insert(
-                    node.id.to_owned(),
-                    add_filtered_dependencies(node, config, dep_kinds),
-                );
+                out_resolve.insert(node.id.to_owned(), add_filtered_dependencies(node, config));
                 // Queue its dependencies for the next BFS loop iteration
                 next_queue.extend(
-                    filtered_dependencies(&node.deps, config, dep_kinds)
-                        .map(|dep| &resolve[&dep.pkg]),
+                    filtered_dependencies(&node.deps, config).map(|dep| &resolve[&dep.pkg]),
                 );
             }
         }
@@ -681,15 +752,9 @@ fn all_dependencies(
     (out_packages, out_resolve)
 }
 
-fn add_filtered_dependencies(
-    node: &Node,
-    config: &SbomConfig,
-    dep_kinds: &mut DependencyKindMap,
-) -> Node {
+fn add_filtered_dependencies(node: &Node, config: &SbomConfig) -> Node {
     let mut node = node.clone();
-    node.deps = filtered_dependencies(&node.deps, config, dep_kinds)
-        .cloned()
-        .collect();
+    node.deps = filtered_dependencies(&node.deps, config).cloned().collect();
     node.dependencies = node.deps.iter().map(|d| d.pkg.to_owned()).collect();
     node
 }
@@ -699,18 +764,9 @@ fn add_filtered_dependencies(
 fn filtered_dependencies<'a>(
     input: &'a [NodeDep],
     config: &'a SbomConfig,
-    dep_kinds: &'a mut DependencyKindMap,
 ) -> impl Iterator<Item = &'a NodeDep> {
     input.iter().filter(|p| {
         p.dep_kinds.iter().any(|dep| {
-            if let Some(deps) = dep_kinds.get_mut(&p.pkg) {
-                if !deps.iter().any(|d| *d == dep.kind) {
-                    deps.push(dep.kind);
-                }
-            } else {
-                dep_kinds.insert(p.pkg.clone(), vec![dep.kind]);
-            }
-
             if let Some(true) = config.only_normal_deps {
                 dep.kind == DependencyKind::Normal
             } else {