Skip to content

Commit

Permalink
Add support for imports.exclude
Browse files Browse the repository at this point in the history
This is a fairly straightforward implementation of imports.exclude. The
configuration option is already documented in the book. After changing
this configuration value, you'll need to update imports.lock by running
a successful `cargo vet` without `--locked` (or by running `cargo vet
regenerate imports`).

I also added some validation to help catch outdated `imports.lock` files
when running with `--locked`. These will never fire when unlocked, as
`imports.lock` will be automatically updated in those cases.

Fixes mozilla#80
  • Loading branch information
mystor committed Aug 25, 2022
1 parent abb612b commit 856bc0a
Show file tree
Hide file tree
Showing 11 changed files with 318 additions and 25 deletions.
3 changes: 3 additions & 0 deletions src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,9 @@ pub enum StoreValidateError {
#[diagnostic(transparent)]
#[error(transparent)]
InvalidCriteria(InvalidCriteriaError),
#[error("imports.lock is out-of-date with respect to configuration")]
#[diagnostic(help("run `cargo vet` without --locked to update imports"))]
ImportsLockOutdated,
}

#[derive(Debug, Error, Diagnostic)]
Expand Down
6 changes: 5 additions & 1 deletion src/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -381,10 +381,14 @@ pub static DEFAULT_POLICY_CRITERIA: CriteriaStr = SAFE_TO_DEPLOY;
pub static DEFAULT_POLICY_DEV_CRITERIA: CriteriaStr = SAFE_TO_RUN;

/// A remote audits.toml that we trust the contents of (by virtue of trusting the maintainer).
#[derive(serde::Serialize, serde::Deserialize, Clone)]
#[derive(serde::Serialize, serde::Deserialize, Clone, Default)]
pub struct RemoteImport {
/// URL of the foreign audits.toml
pub url: String,
/// A list of crates for which no audits or violations should be imported.
#[serde(skip_serializing_if = "Vec::is_empty")]
#[serde(default)]
pub exclude: Vec<PackageName>,
/// A list of criteria that are implied by foreign criteria
#[serde(rename = "criteria-map")]
pub criteria_map: Vec<CriteriaMapping>,
Expand Down
52 changes: 49 additions & 3 deletions src/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,10 +197,10 @@ impl Store {
// If this command isn't locked, and the network is available, fetch the
// live state of imported audits.
let live_imports = if let (false, Some(network)) = (cfg.cli.locked, network) {
let imported_audits = tokio::runtime::Handle::current()
let fetched_audits = tokio::runtime::Handle::current()
.block_on(fetch_imported_audits(network, &config))?;
let live_imports =
process_imported_audits(imported_audits, &imports, allow_criteria_changes)?;
process_imported_audits(fetched_audits, &config, &imports, allow_criteria_changes)?;
Some(live_imports)
} else {
None
Expand Down Expand Up @@ -249,7 +249,7 @@ impl Store {
allow_criteria_changes: bool,
) -> Result<Self, CriteriaChangeErrors> {
let live_imports =
process_imported_audits(fetched_audits, &imports, allow_criteria_changes)?;
process_imported_audits(fetched_audits, &config, &imports, allow_criteria_changes)?;
Ok(Self {
lock: None,
config,
Expand Down Expand Up @@ -504,9 +504,16 @@ impl Store {
}
}

// If we're locked, and therefore not fetching new live imports,
// validate that our imports.lock is in sync with config.toml.
let imports_lock_outdated = self
.imports_lock_outdated()
.then_some(StoreValidateError::ImportsLockOutdated);

let errors = invalid_criteria_errors
.into_iter()
.map(StoreValidateError::InvalidCriteria)
.chain(imports_lock_outdated)
.collect::<Vec<_>>();
if !errors.is_empty() {
return Err(StoreValidateErrors { errors });
Expand All @@ -515,6 +522,33 @@ impl Store {
Ok(())
}

fn imports_lock_outdated(&self) -> bool {
// If we have live imports, we're going to be updating imports.lock, so
// it's OK if it's out-of-date with regard to the config.
if self.live_imports.is_some() {
return false;
}

// We must have the exact same set of imports, otherwise an import has
// been added or removed and we're out of date.
if self.config.imports.keys().ne(self.imports.audits.keys()) {
return true;
}

for (import_name, config) in &self.config.imports {
let audits_file = self.imports.audits.get(import_name).unwrap();
// If we have any excluded crates in the imports.lock, it is out of
// date and needs to be regenerated.
for crate_name in &config.exclude {
if audits_file.audits.contains_key(crate_name) {
return true;
}
}
}

false
}

/// Return an updated version of the `imports.lock` file taking into account
/// the result of running the resolver, to pick which audits need to be
/// vendored.
Expand Down Expand Up @@ -622,6 +656,7 @@ impl Store {
/// description of the live state of imported audits.
fn process_imported_audits(
fetched_audits: Vec<(ImportName, AuditsFile)>,
config_file: &ConfigFile,
imports_lock: &ImportsFile,
allow_criteria_changes: bool,
) -> Result<ImportsFile, CriteriaChangeErrors> {
Expand All @@ -630,6 +665,17 @@ fn process_imported_audits(
};
let mut changed_criteria = Vec::new();
for (import_name, mut audits_file) in fetched_audits {
let config = config_file
.imports
.get(&import_name)
.expect("fetched audit without config?");

// Remove any excluded audits from the live copy. We'll effectively
// pretend they don't exist upstream.
for excluded in &config.exclude {
audits_file.audits.remove(excluded);
}

// By default all audits read from the network are fresh.
for audit_entry in audits_file.audits.values_mut().flat_map(|v| v.iter_mut()) {
audit_entry.is_fresh_import = true;
Expand Down
106 changes: 93 additions & 13 deletions src/tests/import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ fn new_peer_import() {
FOREIGN.to_owned(),
crate::format::RemoteImport {
url: FOREIGN_URL.to_owned(),
criteria_map: Vec::new(),
..Default::default()
},
);

Expand All @@ -81,7 +81,7 @@ fn new_peer_import() {
OTHER_FOREIGN.to_owned(),
crate::format::RemoteImport {
url: OTHER_FOREIGN_URL.to_owned(),
criteria_map: Vec::new(),
..Default::default()
},
);

Expand Down Expand Up @@ -141,7 +141,7 @@ fn existing_peer_skip_import() {
FOREIGN.to_owned(),
crate::format::RemoteImport {
url: FOREIGN_URL.to_owned(),
criteria_map: Vec::new(),
..Default::default()
},
);

Expand Down Expand Up @@ -195,7 +195,7 @@ fn existing_peer_remove_unused() {
FOREIGN.to_owned(),
crate::format::RemoteImport {
url: FOREIGN_URL.to_owned(),
criteria_map: Vec::new(),
..Default::default()
},
);

Expand Down Expand Up @@ -283,7 +283,7 @@ fn existing_peer_import_delta_audit() {
FOREIGN.to_owned(),
crate::format::RemoteImport {
url: FOREIGN_URL.to_owned(),
criteria_map: Vec::new(),
..Default::default()
},
);

Expand All @@ -295,7 +295,7 @@ fn existing_peer_import_delta_audit() {
OTHER_FOREIGN.to_owned(),
crate::format::RemoteImport {
url: OTHER_FOREIGN_URL.to_owned(),
criteria_map: Vec::new(),
..Default::default()
},
);

Expand Down Expand Up @@ -366,7 +366,7 @@ fn existing_peer_import_custom_criteria() {
FOREIGN.to_owned(),
crate::format::RemoteImport {
url: FOREIGN_URL.to_owned(),
criteria_map: Vec::new(),
..Default::default()
},
);

Expand Down Expand Up @@ -428,7 +428,7 @@ fn new_audit_for_unused_criteria_basic() {
FOREIGN.to_owned(),
crate::format::RemoteImport {
url: FOREIGN_URL.to_owned(),
criteria_map: Vec::new(),
..Default::default()
},
);

Expand Down Expand Up @@ -494,7 +494,7 @@ fn new_audit_for_unused_criteria_transitive() {
FOREIGN.to_owned(),
crate::format::RemoteImport {
url: FOREIGN_URL.to_owned(),
criteria_map: Vec::new(),
..Default::default()
},
);

Expand Down Expand Up @@ -546,7 +546,7 @@ fn existing_peer_revoked_audit() {
FOREIGN.to_owned(),
crate::format::RemoteImport {
url: FOREIGN_URL.to_owned(),
criteria_map: Vec::new(),
..Default::default()
},
);

Expand Down Expand Up @@ -607,7 +607,7 @@ fn existing_peer_add_violation() {
FOREIGN.to_owned(),
crate::format::RemoteImport {
url: FOREIGN_URL.to_owned(),
criteria_map: Vec::new(),
..Default::default()
},
);

Expand Down Expand Up @@ -658,7 +658,7 @@ fn peer_audits_exemption_no_minimize() {
FOREIGN.to_owned(),
crate::format::RemoteImport {
url: FOREIGN_URL.to_owned(),
criteria_map: Vec::new(),
..Default::default()
},
);

Expand Down Expand Up @@ -709,7 +709,7 @@ fn peer_audits_exemption_minimize() {
FOREIGN.to_owned(),
crate::format::RemoteImport {
url: FOREIGN_URL.to_owned(),
criteria_map: Vec::new(),
..Default::default()
},
);

Expand All @@ -734,6 +734,86 @@ fn peer_audits_exemption_minimize() {
insta::assert_snapshot!(output);
}

#[test]
fn peer_audits_import_exclusion() {
// (Pass) Exclusions in the config should make a crate's audits and
// violations appear to be revoked upstream, but audits for other crates
// shouldn't be impacted.

let _enter = TEST_RUNTIME.enter();
let mock = MockMetadata::simple();

let metadata = mock.metadata();
let (mut config, mut audits, mut imports) = builtin_files_full_audited(&metadata);

audits.audits.remove("transitive-third-party1");

let old_foreign_audits = AuditsFile {
criteria: SortedMap::new(),
audits: [
(
"third-party2".to_owned(),
vec![full_audit(ver(DEFAULT_VER), SAFE_TO_DEPLOY)],
),
(
"third-party1".to_owned(),
vec![violation("*".parse().unwrap(), SAFE_TO_DEPLOY)],
),
(
"transitive-third-party1".to_owned(),
vec![full_audit(ver(DEFAULT_VER), SAFE_TO_DEPLOY)],
),
]
.into_iter()
.collect(),
};

let new_foreign_audits = old_foreign_audits.clone();

imports
.audits
.insert(FOREIGN.to_owned(), old_foreign_audits);

config.imports.insert(
FOREIGN.to_owned(),
crate::format::RemoteImport {
url: FOREIGN_URL.to_owned(),
exclude: vec!["third-party1".to_owned(), "third-party2".to_owned()],
..Default::default()
},
);

let store = Store::mock_online(
config,
audits,
imports,
vec![(FOREIGN.to_owned(), new_foreign_audits)],
true,
)
.unwrap();

let imported = store
.imported_audits()
.get(FOREIGN)
.expect("The remote should be present in `imported_audits`");

assert!(
!imported.audits.contains_key("third-party1"),
"The `third-party1` crate should be completely missing from `imported_audits`"
);
assert!(
!imported.audits.contains_key("third-party2"),
"The `third-party2` crate should be completely missing from `imported_audits`"
);
assert!(
imported.audits.contains_key("transitive-third-party1"),
"The `transitive-third-party1` crate should still be present in `imported_audits`"
);

let output = get_imports_file_changes(&metadata, &store, false);
insta::assert_snapshot!(output);
}

// Other tests worth adding:
//
// - used edges with dependency-criteria should cause criteria to be imported
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
source: src/tests/import.rs
expression: output
---

-[[audits.peer-company.audits.third-party1]]
-criteria = "safe-to-deploy"
-violation = "*"
-
-[[audits.peer-company.audits.third-party2]]
-criteria = "safe-to-deploy"
-version = "10.0.0"
-
[[audits.peer-company.audits.transitive-third-party1]]
criteria = "safe-to-deploy"
version = "10.0.0"

Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
source: src/tests/store_parsing.rs
expression: acquire_errors
---

× Your cargo-vet store (supply-chain) has consistency errors

Error:
× imports.lock is out-of-date with respect to configuration
help: run `cargo vet` without --locked to update imports

Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
source: src/tests/store_parsing.rs
expression: acquire_errors
---

× Your cargo-vet store (supply-chain) has consistency errors

Error:
× imports.lock is out-of-date with respect to configuration
help: run `cargo vet` without --locked to update imports

Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
source: src/tests/store_parsing.rs
expression: acquire_errors
---

× Your cargo-vet store (supply-chain) has consistency errors

Error:
× imports.lock is out-of-date with respect to configuration
help: run `cargo vet` without --locked to update imports

Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
source: src/tests/store_parsing.rs
expression: acquire_errors
---

Loading

0 comments on commit 856bc0a

Please sign in to comment.