diff --git a/src/errors.rs b/src/errors.rs index 431c0f95..b6e91a6d 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -29,10 +29,6 @@ pub enum AuditAsError { #[error(transparent)] #[diagnostic(transparent)] ShouldntBeAuditAs(ShouldntBeAuditAsErrors), - // FIXME: we should probably just make the caller pass this in? - #[error(transparent)] - #[diagnostic(transparent)] - CacheAcquire(CacheAcquireError), } #[derive(Debug, Error, Diagnostic)] diff --git a/src/main.rs b/src/main.rs index 28aa5341..24a4ea52 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1342,7 +1342,8 @@ fn fix_audit_as(cfg: &Config, store: &mut Store) -> Result<(), CacheAcquireError // NOTE: In the future this might require Network, but for now `cargo metadata` is a precondition // and guarantees a fully populated and up to date index, so we can just rely on that and know // this is Networkless. - let issues = check_audit_as_crates_io(cfg, store); + let mut cache = Cache::acquire(cfg)?; + let issues = check_audit_as_crates_io(cfg, store, &mut cache); if let Err(AuditAsErrors { errors }) = issues { for error in errors { match error { @@ -1366,7 +1367,6 @@ fn fix_audit_as(cfg: &Config, store: &mut Store) -> Result<(), CacheAcquireError .audit_as_crates_io = Some(false); } } - AuditAsError::CacheAcquire(err) => return Err(err), } } } @@ -1487,7 +1487,8 @@ fn cmd_check(out: &Arc, cfg: &Config, sub_args: &CheckArgs) -> Result<( if !cfg.cli.locked { // Check if any of our first-parties are in the crates.io registry - check_audit_as_crates_io(cfg, &store)?; + let mut cache = Cache::acquire(cfg).into_diagnostic()?; + check_audit_as_crates_io(cfg, &store, &mut cache)?; } // DO THE THING!!!! @@ -2026,14 +2027,20 @@ fn first_party_packages_strict<'a>( .filter(move |package| !package.is_third_party(&empty_policy)) } -fn check_audit_as_crates_io(cfg: &Config, store: &Store) -> Result<(), AuditAsErrors> { - let cache = Cache::acquire(cfg).map_err(|e| AuditAsErrors { - errors: vec![AuditAsError::CacheAcquire(e)], - })?; +fn check_audit_as_crates_io( + cfg: &Config, + store: &Store, + cache: &mut Cache, +) -> Result<(), AuditAsErrors> { + // If we don't have a registry, we can't check audit-as-crates-io. + if !cache.has_registry() { + return Ok(()); + } + let mut needs_audit_as_entry = vec![]; let mut shouldnt_be_audit_as = vec![]; - 'packages: for package in first_party_packages_strict(&cfg.metadata, &store.config) { + for package in first_party_packages_strict(&cfg.metadata, &store.config) { let audit_policy = store .config .policy @@ -2055,13 +2062,21 @@ fn check_audit_as_crates_io(cfg: &Config, store: &Store) -> Result<(), AuditAsEr }); } // Now that we've found a version match, we're done with this package - continue 'packages; + continue; } } // If we reach this point, then we couldn't find a matching package in the registry, // So any `audit-as-crates-io = true` is an error that should be corrected if audit_policy == Some(true) { + // We found a crate which someone thought was on crates-io, but + // doesn't appear to be according to our local index. + // Before reporting an error, ensure the index is up to date, + // and restart if it was not. + if cache.ensure_index_up_to_date() { + return check_audit_as_crates_io(cfg, store, cache); + } + shouldnt_be_audit_as.push(ShouldntBeAuditAsError { package: package.name.clone(), version: package.version.clone(), diff --git a/src/storage.rs b/src/storage.rs index 20fe8c1a..767ad59f 100644 --- a/src/storage.rs +++ b/src/storage.rs @@ -9,7 +9,6 @@ use std::{ }; use cargo_metadata::Version; -use crates_index::Index; use flate2::read::GzDecoder; use futures_util::future::try_join_all; use miette::{NamedSource, SourceOffset}; @@ -33,12 +32,20 @@ use crate::{ SortedMap, SAFE_TO_DEPLOY, SAFE_TO_RUN, }, network::Network, - out::{progress_bar, IncProgressOnDrop}, + out::{indeterminate_spinner, progress_bar, IncProgressOnDrop}, resolver::{self, Conclusion}, serialization::{spanned::Spanned, to_formatted_toml}, Config, PartialConfig, }; +/// The type to use for accessing information from crates.io. +#[cfg(not(test))] +type CratesIndex = crates_index::Index; + +/// When running tests, a mock index is used instead of the real one. +#[cfg(test)] +type CratesIndex = crate::tests::MockIndex; + // tmp cache for various shenanigans const CACHE_DIFF_CACHE: &str = "diff-cache.toml"; const CACHE_COMMAND_HISTORY: &str = "command-history.json"; @@ -818,11 +825,13 @@ async fn fetch_imported_audit( /// A Registry in CARGO_HOME (usually the crates.io one) pub struct CargoRegistry { /// The queryable index - index: Index, + index: CratesIndex, /// The base path all registries share (`$CARGO_HOME/registry`) base_dir: PathBuf, /// The name of the registry (`github.com-1ecc6299db9ec823`) registry: OsString, + /// Whether or not the index is known to be up-to-date + index_up_to_date: bool, } impl CargoRegistry { @@ -905,12 +914,19 @@ impl Drop for Cache { impl Cache { /// Acquire the cache pub fn acquire(cfg: &PartialConfig) -> Result { + // Try to get the cargo registry + let cargo_registry = find_cargo_registry(); + if let Err(e) = &cargo_registry { + // ERRORS: this warning really rides the line, I'm not sure if the user can/should care + warn!("Couldn't find cargo registry: {e}"); + } + if cfg.mock_cache { // We're in unit tests, everything should be mocked and not touch real caches return Ok(Cache { _lock: None, root: None, - cargo_registry: None, + cargo_registry: cargo_registry.ok(), diff_cache_path: None, command_history_path: None, diff_semaphore: tokio::sync::Semaphore::new(MAX_CONCURRENT_DIFFS), @@ -968,13 +984,6 @@ impl Cache { .and_then(|f| load_json(f).ok()) .unwrap_or_default(); - // Try to get the cargo registry - let cargo_registry = find_cargo_registry(); - if let Err(e) = &cargo_registry { - // ERRORS: this warning really rides the line, I'm not sure if the user can/should care - warn!("Couldn't find cargo registry: {e}"); - } - Ok(Self { _lock: Some(lock), root: Some(root), @@ -991,27 +1000,48 @@ impl Cache { }) } + /// Check if the Cache has access to the registry or information about the + /// crates.io index. + pub fn has_registry(&self) -> bool { + self.cargo_registry.is_some() + } + + /// Ensures that the local copy of the crates.io index has the most + /// up-to-date information about what crates are available. + /// + /// Returns `true` if the state of the index may have been changed by this + /// call, and `false` if the index is already up-to-date. + pub fn ensure_index_up_to_date(&mut self) -> bool { + let reg = match &mut self.cargo_registry { + Some(reg) => reg, + None => return false, + }; + if reg.index_up_to_date { + return false; + } + let _spinner = indeterminate_spinner("Updating", "registry index"); + reg.index_up_to_date = true; + match reg.index.update() { + Ok(()) => true, + Err(e) => { + warn!("Couldn't update cargo index: {e}"); + false + } + } + } + /// Gets any information the crates.io index has on this package, locally - /// with no downloads. The fact that we invoke `cargo metadata` on startup - /// means the index should be as populated as we're able to get it. + /// with no downloads. The index may be out of date, however a caller can + /// use `ensure_index_up_to_date` to make sure it is up to date before + /// calling this method. /// /// However this may do some expensive disk i/o, so ideally we should do /// some bulk processing of this later. For now let's get it working... - #[cfg(not(test))] pub fn query_package_from_index(&self, name: PackageStr) -> Option { let reg = self.cargo_registry.as_ref()?; reg.index.crate_(name) } - #[cfg(test)] - pub fn query_package_from_index(&self, name: PackageStr) -> Option { - if let Some(reg) = self.cargo_registry.as_ref() { - reg.index.crate_(name) - } else { - crate::tests::MockRegistry::testing_cinematic_universe().package(name) - } - } - #[tracing::instrument(skip(self, network), err)] pub async fn fetch_package( &self, @@ -1545,7 +1575,7 @@ fn find_cargo_registry() -> Result { // ERRORS: all of this is genuinely fallible internal workings // but if these path adjustments don't work then something is very fundamentally wrong - let index = Index::new_cargo_default()?; + let index = CratesIndex::new_cargo_default()?; let base_dir = index.path().parent().unwrap().parent().unwrap().to_owned(); let registry = index.path().file_name().unwrap().to_owned(); @@ -1554,6 +1584,7 @@ fn find_cargo_registry() -> Result { index, base_dir, registry, + index_up_to_date: false, }) } diff --git a/src/tests/audit_as_crates_io.rs b/src/tests/audit_as_crates_io.rs index 827d5b6f..16d2abb3 100644 --- a/src/tests/audit_as_crates_io.rs +++ b/src/tests/audit_as_crates_io.rs @@ -1,7 +1,8 @@ use super::*; fn get_audit_as_crates_io(cfg: &Config, store: &Store) -> String { - let res = crate::check_audit_as_crates_io(cfg, store); + let mut cache = crate::storage::Cache::acquire(cfg).unwrap(); + let res = crate::check_audit_as_crates_io(cfg, store, &mut cache); match res { Ok(()) => String::new(), Err(e) => format!("{:?}", miette::Report::new(e)), @@ -9,7 +10,8 @@ fn get_audit_as_crates_io(cfg: &Config, store: &Store) -> String { } fn get_audit_as_crates_io_json(cfg: &Config, store: &Store) -> String { - let res = crate::check_audit_as_crates_io(cfg, store); + let mut cache = crate::storage::Cache::acquire(cfg).unwrap(); + let res = crate::check_audit_as_crates_io(cfg, store, &mut cache); match res { Ok(()) => String::new(), Err(e) => { @@ -232,3 +234,25 @@ fn cycle_audit_as_crates_io() { let output = get_audit_as_crates_io(&cfg, &store); insta::assert_snapshot!("cycle-audit-as-crates-io", output); } + +#[test] +fn audit_as_crates_io_need_update() { + // The "root" and "first" packages don't have their versions on crates.io + // until the index is updated. If an `audit_as_crates_io` entry requires + // `first` to be on crates.io, and it's not, we'll do an update, which + // should reveal that `root` also needs to be audited. + + let _enter = TEST_RUNTIME.enter(); + + let mock = MockMetadata::haunted_tree(); + let metadata = mock.metadata(); + let (mut config, audits, imports) = builtin_files_full_audited(&metadata); + config + .policy + .insert("first".to_owned(), audit_as_policy(Some(true))); + let store = Store::mock(config, audits, imports); + let cfg = mock_cfg(&metadata); + + let output = get_audit_as_crates_io(&cfg, &store); + insta::assert_snapshot!("audit-as-crates-io-need-update", output); +} diff --git a/src/tests/mod.rs b/src/tests/mod.rs index 4370233d..5baa817e 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -2,7 +2,7 @@ use std::{ collections::BTreeMap, ffi::OsString, fmt, fs, io, - path::PathBuf, + path::{Path, PathBuf}, sync::{Arc, Mutex}, }; @@ -110,8 +110,9 @@ struct MockDependency { version: Version, } -pub struct MockRegistry { +pub struct MockIndex { packages: FastMap, Vec>, + path: PathBuf, } struct MockRegistryVersion { @@ -127,9 +128,12 @@ fn reg_ver(pub_ver: u64) -> MockRegistryVersion { } } -impl MockRegistry { - pub fn testing_cinematic_universe() -> Self { - Self { +// The `MockIndex` type should provide the same interface as the +// `crates_index::Index` type, as it is used in place of that type in test +// builds. +impl MockIndex { + pub fn new_cargo_default() -> Result { + Ok(Self { packages: [ ("root-package", vec![reg_ver(DEFAULT_VER)]), ("third-party1", vec![reg_ver(DEFAULT_VER)]), @@ -157,12 +161,26 @@ impl MockRegistry { ("dev-cycle-indirect", vec![reg_ver(DEFAULT_VER)]), ("third-normal", vec![reg_ver(DEFAULT_VER)]), ("third-dev", vec![reg_ver(DEFAULT_VER)]), + ("first", vec![reg_ver(1)]), ] .into_iter() .collect(), + path: Path::new(env!("CARGO_MANIFEST_DIR")) + .join("/tests/mock-registry/index/testing-cinematic-universe"), + }) + } + pub fn update(&mut self) -> Result<(), crates_index::Error> { + if self.packages.contains_key("new-package") { + return Ok(()); // already updated } + self.packages.insert("root", vec![reg_ver(DEFAULT_VER)]); + self.packages + .get_mut("first") + .unwrap() + .push(reg_ver(DEFAULT_VER)); + Ok(()) } - pub fn package(&self, name: PackageStr) -> Option { + pub fn crate_(&self, name: PackageStr) -> Option { use std::io::Write; let package_entry = self.packages.get(name)?; @@ -222,6 +240,9 @@ impl MockRegistry { .expect("failed to parse mock crates index file"); Some(result) } + pub fn path(&self) -> &Path { + &self.path + } } impl Default for MockPackage { diff --git a/src/tests/snapshots/cargo_vet__tests__audit_as_crates_io__audit-as-crates-io-need-update.snap b/src/tests/snapshots/cargo_vet__tests__audit_as_crates_io__audit-as-crates-io-need-update.snap new file mode 100644 index 00000000..c3327bb0 --- /dev/null +++ b/src/tests/snapshots/cargo_vet__tests__audit_as_crates_io__audit-as-crates-io-need-update.snap @@ -0,0 +1,12 @@ +--- +source: src/tests/audit_as_crates_io.rs +expression: output +--- + + × There are some issues with your policy.audit-as-crates-io entries + +Error: + × Some non-crates.io-fetched packages match published crates.io versions + │ root:10.0.0 + help: Add a `policy.*.audit-as-crates-io` entry for them +