Merge pull request #168 from RustSec/refactor-package-scopes

Refactor package scopes (fixes #153)

src/database.rs

@@ -4,12 +4,11 @@ mod entries;
 mod index;
 mod query;
 
-/// Contains the definition of a [PackageScope]
-pub mod package_scope;
+pub mod scope;
 
 pub use self::query::Query;
 
-use self::{entries::Entries, index::Index, package_scope::PackageScope};
+use self::{entries::Entries, index::Index};
 use crate::{
     advisory::{self, Advisory},
     collection::Collection,
@@ -106,16 +105,18 @@ impl Database {
     }
 
     /// Find vulnerabilities in the provided `Lockfile` which match a given query.
+    // TODO(tarcieri): move `package_scope` into `Query`?
     pub fn query_vulnerabilities(
         &self,
         lockfile: &Lockfile,
         query: &Query,
-        scope: &PackageScope,
+        package_scope: impl Into<scope::Package>,
     ) -> Vec<Vulnerability> {
+        let package_scope = package_scope.into();
         let mut vulns = vec![];
 
         for package in &lockfile.packages {
-            if scope.is_no_local() && package.source.is_none() {
+            if package_scope.is_remote() && package.source.is_none() {
                 continue;
             }
 
@@ -137,7 +138,7 @@ impl Database {
 
     /// Scan for vulnerabilities in the provided `Lockfile`.
     pub fn vulnerabilities(&self, lockfile: &Lockfile) -> Vec<Vulnerability> {
-        self.query_vulnerabilities(lockfile, &Query::crate_scope(), &PackageScope::default())
+        self.query_vulnerabilities(lockfile, &Query::crate_scope(), scope::Package::default())
     }
 
     /// Iterate over all of the advisories in the database

src/database/query.rs

@@ -1,9 +1,9 @@
 //! Queries against the RustSec database
-
-use crate::database::package_scope::PackageScope;
+//!
 use crate::{
     advisory::{Advisory, Severity},
     collection::Collection,
+    database::scope,
     package,
     platforms::target::{Arch, OS},
     version::Version,
@@ -40,7 +40,7 @@ pub struct Query {
     informational: Option<bool>,
 
     /// Scope of packages which should be considered for audit
-    package_scope: Option<PackageScope>,
+    package_scope: Option<scope::Package>,
 }
 
 impl Query {

src/database/scope.rs

@@ -0,0 +1,73 @@
+//! Database scopes
+
+use serde::{Deserialize, Serialize};
+
+/// Registries where packages are located
+#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
+pub enum Registry {
+    /// Public package published to <https://crates.io>
+    #[serde(rename = "public")]
+    Public,
+
+    /// Package is local
+    #[serde(rename = "local")]
+    Local,
+
+    /// Package is located in a private registry
+    #[serde(rename = "private")]
+    Private {
+        /// URI of the private registry
+        uri: String,
+    },
+
+    /// All sources should be considered
+    #[serde(rename = "all")]
+    All,
+}
+
+impl Default for Registry {
+    fn default() -> Self {
+        Registry::Public
+    }
+}
+
+/// Scopes for packages to be queried (i.e. their sources)
+#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
+pub struct Package {
+    /// Source of a package
+    pub source: Vec<Registry>,
+}
+
+impl Package {
+    /// Is the scope only for remote crates?
+    pub fn is_remote(&self) -> bool {
+        self.source.iter().any(|source| match source {
+            Registry::Public | Registry::Private { .. } => true,
+            _ => false,
+        })
+    }
+}
+
+impl Default for Package {
+    fn default() -> Self {
+        Registry::default().into()
+    }
+}
+
+impl Package {
+    /// Creates a new [[Scope]] from a specific registry uri `source`
+    pub fn from_registry(source: &str) -> Self {
+        Registry::Private {
+            uri: source.to_string(),
+        }
+        .into()
+    }
+}
+
+impl From<Registry> for Package {
+    fn from(registry: Registry) -> Self {
+        Self {
+            source: vec![registry],
+        }
+    }
+}

src/report.rs

@@ -5,7 +5,7 @@
 
 use crate::{
     advisory,
-    database::{package_scope::PackageScope, Database, Query},
+    database::{scope, Database, Query},
     lockfile::Lockfile,
     map,
     platforms::target::{Arch, OS},
@@ -38,10 +38,7 @@ pub struct Report {
 impl Report {
     /// Generate a report for the given advisory database and lockfile
     pub fn generate(db: &Database, lockfile: &Lockfile, settings: &Settings) -> Self {
-        let mut package_scope = &PackageScope::default();
-        if let Some(scope) = &settings.package_scope {
-            package_scope = scope;
-        }
+        let package_scope = settings.package_scope.as_ref().cloned().unwrap_or_default();
 
         let vulnerabilities = db
             .query_vulnerabilities(lockfile, &settings.query(), package_scope)
@@ -80,7 +77,7 @@ pub struct Settings {
     pub informational_warnings: Vec<advisory::Informational>,
 
     /// Scope of packages which should be considered for audit
-    pub package_scope: Option<PackageScope>,
+    pub package_scope: Option<scope::Package>,
 }
 
 impl Settings {
@@ -196,12 +193,9 @@ impl WarningInfo {
 /// Find warnings from the given advisory [`Database`] and [`Lockfile`]
 pub fn find_warnings(db: &Database, lockfile: &Lockfile, settings: &Settings) -> WarningInfo {
     let query = settings.query().informational(true);
-    let mut warnings = WarningInfo::default();
+    let package_scope = settings.package_scope.as_ref().cloned().unwrap_or_default();
 
-    let mut package_scope = &PackageScope::default();
-    if let Some(scope) = &settings.package_scope {
-        package_scope = scope;
-    }
+    let mut warnings = WarningInfo::default();
 
     // TODO(tarcieri): abstract `Cargo.lock` query logic between vulnerabilities/warnings
     for advisory_vuln in db.query_vulnerabilities(lockfile, &query, package_scope) {

tests/database.rs

@@ -1,6 +1,6 @@
 use cargo_lock::Lockfile;
 use once_cell::sync::Lazy;
-use rustsec::database::package_scope::{PackageScope, PackageSource};
+use rustsec::database::scope;
 use rustsec::database::Query;
 use rustsec::Database;
 use std::path::Path;
@@ -28,30 +28,23 @@ fn query_vulnerabilities_default() {
         Lockfile::load(lockfile_path).expect("Should find the lock file in support folder.");
     let db = DEFAULT_DATABASE.lock().unwrap();
     let vuln_all =
-        db.query_vulnerabilities(&lockfile, &Query::crate_scope(), &PackageScope::default());
+        db.query_vulnerabilities(&lockfile, &Query::crate_scope(), scope::Package::default());
     let vuln = db.vulnerabilities(&lockfile);
     assert_eq!(vuln_all, vuln);
 }
 
-/// packages without source should not be queried in `PackageScope::LocalCrates` but in `PackageScope::PublicCrates`
+/// packages without source should not be queried in `package::Scope::LocalCrates` but in `PackageScope::PublicCrates`
 #[test]
 fn query_vulnerabilities_scope_public() {
     let lockfile_path = Path::new("./tests/support/local_cargo.lock");
     let lockfile =
         Lockfile::load(lockfile_path).expect("Should find the lock file in support folder.");
     let db = DEFAULT_DATABASE.lock().unwrap();
 
-    let vuln_public = db.query_vulnerabilities(
-        &lockfile,
-        &Query::crate_scope(),
-        &PackageScope::from_source(PackageSource::Public),
-    );
+    let vuln_public =
+        db.query_vulnerabilities(&lockfile, &Query::crate_scope(), scope::Registry::Public);
     assert_eq!(vuln_public.len(), 0);
 
-    let vuln_all = db.query_vulnerabilities(
-        &lockfile,
-        &Query::crate_scope(),
-        &PackageScope::from_source(PackageSource::All),
-    );
+    let vuln_all = db.query_vulnerabilities(&lockfile, &Query::crate_scope(), scope::Registry::All);
     assert_eq!(vuln_all.len(), 1);
 }