diff --git a/src/bans/cfg.rs b/src/bans/cfg.rs index 94c49392..15fc24c4 100644 --- a/src/bans/cfg.rs +++ b/src/bans/cfg.rs @@ -81,6 +81,9 @@ pub struct Config { /// down to a certain depth #[serde(default)] pub skip_tree: Vec>, + /// How to handle wildcard dependencies + #[serde(default = "crate::lint_warn")] + pub wildcards: LintLevel, } impl Default for Config { @@ -92,6 +95,7 @@ impl Default for Config { allow: Vec::new(), skip: Vec::new(), skip_tree: Vec::new(), + wildcards: LintLevel::Warn, } } } @@ -162,6 +166,7 @@ impl Config { denied, allowed, skipped, + wildcards: self.wildcards, tree_skipped: self .skip_tree .into_iter() @@ -182,6 +187,7 @@ pub struct ValidConfig { pub(crate) allowed: Vec, pub(crate) skipped: Vec, pub(crate) tree_skipped: Vec>, + pub wildcards: LintLevel, } #[cfg(test)] diff --git a/src/bans/mod.rs b/src/bans/mod.rs index 12c7eaac..2266240e 100644 --- a/src/bans/mod.rs +++ b/src/bans/mod.rs @@ -194,6 +194,8 @@ pub fn check( output_graph: Option>, sender: crossbeam::channel::Sender, ) { + let wildcard = VersionReq::parse("*").expect("Parsing wildcard mustnt fail"); + let ValidConfig { file_id, denied, @@ -202,6 +204,7 @@ pub fn check( multiple_versions, highlight, tree_skipped, + wildcards, .. } = ctx.cfg; @@ -361,6 +364,56 @@ pub fn check( multi_detector.dupes.clear(); multi_detector.dupes.push(i); } + + if wildcards != LintLevel::Allow { + let severity = match wildcards { + LintLevel::Warn => Severity::Warning, + LintLevel::Deny => Severity::Error, + LintLevel::Allow => unreachable!(), + }; + + let wildcards = krate + .deps + .iter() + .filter(|dep| dep.req == wildcard) + .collect::>(); + + if !wildcards.is_empty() { + let labels = if let Some(ref cargo_spans) = ctx.cargo_spans { + let (file_id, map) = &cargo_spans[&krate.id]; + + wildcards + .into_iter() + .map(|dep| { + Label::primary(*file_id, map[&dep.name].clone()) + .with_message("lock entries") + }) + .collect::>() + } else { + vec![] + }; + + let msg = if labels.len() == 1 { + format!("found 1 wildcard dependency for crate '{}'", krate.name) + } else { + format!( + "found {} wildcard dependencies for crate '{}'", + labels.len(), + krate.name + ) + }; + let diag = Diag::new( + Diagnostic::new(severity) + .with_message(msg) + .with_labels(labels), + ); + + let mut pack = Pack::with_kid(Check::Bans, krate.id.clone()); + pack.push(diag); + + sender.send(pack).unwrap(); + } + } } if !pack.is_empty() { diff --git a/src/cargo-deny/check.rs b/src/cargo-deny/check.rs index 7ed74147..9d544e1b 100644 --- a/src/cargo-deny/check.rs +++ b/src/cargo-deny/check.rs @@ -2,7 +2,7 @@ use crate::stats::{AllStats, Stats}; use anyhow::{Context, Error}; use cargo_deny::{ advisories, bans, - diag::{Diagnostic, Files, Severity}, + diag::{CargoSpans, Diagnostic, Files, Severity}, licenses, sources, CheckCtx, }; use clap::arg_enum; @@ -260,10 +260,16 @@ pub(crate) fn cmd( None }; - let (krate_spans, spans_id) = krate_spans - .map(|(spans, contents)| { + let (krate_spans, spans_id, cargo_spans) = krate_spans + .map(|(spans, contents, raw_cargo_spans)| { let id = files.add(krates.lock_path(), contents); - (spans, id) + + let mut cargo_spans = CargoSpans::new(); + for (key, val) in raw_cargo_spans { + let cargo_id = files.add(val.0, val.1); + cargo_spans.insert(key, (cargo_id, val.2)); + } + (spans, id, cargo_spans) }) .unwrap(); @@ -334,6 +340,7 @@ pub(crate) fn cmd( krate_spans: &krate_spans, spans_id, serialize_extra, + cargo_spans: None, }; s.spawn(move |_| { @@ -386,6 +393,7 @@ pub(crate) fn cmd( krate_spans: &krate_spans, spans_id, serialize_extra, + cargo_spans: Some(cargo_spans), }; s.spawn(|_| { @@ -408,6 +416,7 @@ pub(crate) fn cmd( krate_spans: &krate_spans, spans_id, serialize_extra, + cargo_spans: None, }; s.spawn(|_| { @@ -429,6 +438,7 @@ pub(crate) fn cmd( krate_spans: &krate_spans, spans_id, serialize_extra, + cargo_spans: None, }; s.spawn(move |_| { diff --git a/src/cargo-deny/common.rs b/src/cargo-deny/common.rs index 4e4b23ed..0af0bd00 100644 --- a/src/cargo-deny/common.rs +++ b/src/cargo-deny/common.rs @@ -135,7 +135,6 @@ impl KrateContext { }), ); } - let graph = gb.build(mdc, |filtered: krates::cm::Package| match filtered.source { Some(src) => { if src.is_crates_io() { @@ -146,7 +145,6 @@ impl KrateContext { } None => log::debug!("filtered {} {}", filtered.name, filtered.version), }); - if let Ok(ref krates) = graph { let end = std::time::Instant::now(); log::info!( diff --git a/src/diag.rs b/src/diag.rs index b97bf468..5e0d67ef 100644 --- a/src/diag.rs +++ b/src/diag.rs @@ -1,3 +1,7 @@ +use std::collections::HashMap; +use std::ops::Range; +use std::path::PathBuf; + use crate::{DepKind, Kid, Krate, Krates}; use anyhow::{Context, Error}; pub use codespan_reporting::diagnostic::Severity; @@ -8,6 +12,10 @@ pub use codespan::FileId; pub type Diagnostic = codespan_reporting::diagnostic::Diagnostic; pub type Label = codespan_reporting::diagnostic::Label; pub type Files = codespan::Files; +// Map of crate id => (path to cargo.toml, synthetic cargo.toml content, map(cratename => crate span)) +pub type RawCargoSpans = HashMap>)>; +// Same as RawCargoSpans but path to cargo.toml and cargo.toml content replaced with FileId +pub type CargoSpans = HashMap>)>; pub struct Diag { pub diag: Diagnostic, @@ -120,11 +128,13 @@ impl std::ops::Index for KrateSpans { } impl KrateSpans { - pub fn new(krates: &Krates) -> (Self, String) { + pub fn new(krates: &Krates) -> (Self, String, RawCargoSpans) { use std::fmt::Write; let mut sl = String::with_capacity(4 * 1024); let mut spans = Vec::with_capacity(krates.len()); + let mut cargo_spans = RawCargoSpans::new(); + for krate in krates.krates().map(|kn| &kn.krate) { let span_start = sl.len(); match &krate.source { @@ -145,9 +155,25 @@ impl KrateSpans { spans.push(KrateSpan { span: span_start..span_end, }); + + let mut sl2 = String::with_capacity(4 * 1024); + let mut deps_map = HashMap::new(); + + for dep in &krate.deps { + let span_start = sl2.len(); + writeln!(sl2, "{} = \"{}\"", dep.name, dep.req) + .expect("unable to synthesize Cargo.toml"); + let span_end = sl2.len() - 1; + deps_map.insert(dep.name.clone(), span_start..span_end); + } + + cargo_spans.insert( + krate.id.clone(), + (krate.manifest_path.clone(), sl2, deps_map), + ); } - (Self { spans }, sl) + (Self { spans }, sl, cargo_spans) } } diff --git a/src/lib.rs b/src/lib.rs index ac5b6a1f..b3edd1f7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -297,6 +297,7 @@ pub struct CheckCtx<'ctx, T> { /// Requests for additional information the check can provide to be /// serialized to the diagnostic pub serialize_extra: bool, + pub cargo_spans: Option, } impl<'ctx, T> CheckCtx<'ctx, T> { diff --git a/tests/advisories.rs b/tests/advisories.rs index 818c365b..2616a9cd 100644 --- a/tests/advisories.rs +++ b/tests/advisories.rs @@ -79,6 +79,7 @@ fn detects_vulnerabilities() { krate_spans: &ctx.spans.0, spans_id: ctx.spans.1, serialize_extra: true, + cargo_spans: None, }; advisories::check(ctx2, &ctx.db, ctx.lock, tx); @@ -138,6 +139,7 @@ fn detects_unmaintained() { krate_spans: &ctx.spans.0, spans_id: ctx.spans.1, serialize_extra: true, + cargo_spans: None, }; advisories::check(ctx2, &ctx.db, ctx.lock, tx); @@ -195,6 +197,7 @@ fn downgrades() { krate_spans: &ctx.spans.0, spans_id: ctx.spans.1, serialize_extra: true, + cargo_spans: None, }; advisories::check(ctx2, &ctx.db, ctx.lock, tx); @@ -274,6 +277,7 @@ fn detects_yanked() { krate_spans: &ctx.spans.0, spans_id: ctx.spans.1, serialize_extra: true, + cargo_spans: None, }; advisories::check(ctx2, &ctx.db, ctx.lock, tx); diff --git a/tests/bans.rs b/tests/bans.rs index 81a952fc..378e8de2 100644 --- a/tests/bans.rs +++ b/tests/bans.rs @@ -18,20 +18,27 @@ fn with_test_data(project_name: &str, accept_ctx: impl FnOnce(CheckCtx<'_, Valid .build(metadata_cmd, krates::NoneFilter) .unwrap(); - let spans = KrateSpans::new(krates); + let (spans, content, hashmap) = KrateSpans::new(krates); let mut files = Files::new(); - let spans_id = files.add(project_dir.join("Cargo.lock"), spans.1); + let spans_id = files.add(project_dir.join("Cargo.lock"), content); let config: Config = fs::read_to_string(project_dir.join("deny.toml")) .map(|it| toml::from_str(&it).unwrap()) .unwrap_or_default(); + let mut newmap = std::collections::HashMap::new(); + for (key, val) in hashmap { + let cargo_id = files.add(val.0, val.1); + newmap.insert(key, (cargo_id, val.2)); + } + accept_ctx(cargo_deny::CheckCtx { krates, - krate_spans: &spans.0, + krate_spans: &spans, spans_id, cfg: config.validate(spans_id).unwrap(), serialize_extra: false, + cargo_spans: Some(newmap), }); } @@ -65,3 +72,56 @@ fn cyclic_dependencies_do_not_cause_infinite_loop() { handle.join().unwrap(); } + +#[test] +fn wildcards_deny_test() { + let (tx, rx) = crossbeam::unbounded(); + + let handle = std::thread::spawn(|| { + with_test_data("wildcards/maincrate", |check_ctx| { + let graph_output = Box::new(|_| Ok(())); + bans::check(check_ctx, Some(graph_output), tx); + }); + }); + + let timeout_duration = Duration::from_millis(10000); + let timeout = crossbeam::after(timeout_duration); + let mut packs: Vec = vec![]; + loop { + crossbeam::select! { + recv(rx) -> msg => { + match msg { + Ok(msg) => packs.push(msg), + Err(_e) => { + // Yay, the sender was dopped (i.e. check was finished) + break; + } + } + } + recv(timeout) -> _ => { + panic!("Bans check timed out after {:?}", timeout_duration); + } + } + } + + handle.join().unwrap(); + + assert_eq!(packs.len(), 2); + + let mut msgs = packs.into_iter().map(|pack| { + let mut diags = pack.into_iter().collect::>(); + assert_eq!(diags.len(), 1); + let diag = diags.pop().unwrap(); + + assert_eq!( + diag.diag.severity, + codespan_reporting::diagnostic::Severity::Error + ); + assert!(diag.diag.message.starts_with("found 1 wildcard dependency")); + diag.diag.message + }); + + // both crates have wildcard deps so check that both are reported + assert!(msgs.any(|s| s.contains("wildcards-test-crate"))); + assert!(msgs.any(|s| s.contains("wildcards-test-dep"))); +} diff --git a/tests/test_data/wildcards/dependency/Cargo.toml b/tests/test_data/wildcards/dependency/Cargo.toml new file mode 100644 index 00000000..cad8fd59 --- /dev/null +++ b/tests/test_data/wildcards/dependency/Cargo.toml @@ -0,0 +1,11 @@ +[package] +name = "wildcards-test-dep" +version = "0.1.0" +authors = [] +edition = "2018" +license = "MIT" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] +itoa = "*" diff --git a/tests/test_data/wildcards/dependency/src/lib.rs b/tests/test_data/wildcards/dependency/src/lib.rs new file mode 100644 index 00000000..f328e4d9 --- /dev/null +++ b/tests/test_data/wildcards/dependency/src/lib.rs @@ -0,0 +1 @@ +fn main() {} diff --git a/tests/test_data/wildcards/maincrate/Cargo.lock b/tests/test_data/wildcards/maincrate/Cargo.lock new file mode 100644 index 00000000..e4714cd1 --- /dev/null +++ b/tests/test_data/wildcards/maincrate/Cargo.lock @@ -0,0 +1,53 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +[[package]] +name = "ansi_term" +version = "0.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d52a9bb7ec0cf484c551830a7ce27bd20d67eac647e1befb56b0be4ee39a55d2" +dependencies = [ + "winapi", +] + +[[package]] +name = "itoa" +version = "0.4.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc6f3ad7b9d11a0c00842ff8de1b60ee58661048eb8049ed33c73594f359d7e6" + +[[package]] +name = "wildcards-test-crate" +version = "0.1.0" +dependencies = [ + "ansi_term", + "wildcards-test-dep", +] + +[[package]] +name = "wildcards-test-dep" +version = "0.1.0" +dependencies = [ + "itoa", +] + +[[package]] +name = "winapi" +version = "0.3.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5c839a674fcd7a98952e593242ea400abe93992746761e38641405d28b00f419" +dependencies = [ + "winapi-i686-pc-windows-gnu", + "winapi-x86_64-pc-windows-gnu", +] + +[[package]] +name = "winapi-i686-pc-windows-gnu" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6" + +[[package]] +name = "winapi-x86_64-pc-windows-gnu" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" diff --git a/tests/test_data/wildcards/maincrate/Cargo.toml b/tests/test_data/wildcards/maincrate/Cargo.toml new file mode 100644 index 00000000..2a128664 --- /dev/null +++ b/tests/test_data/wildcards/maincrate/Cargo.toml @@ -0,0 +1,12 @@ +[package] +name = "wildcards-test-crate" +version = "0.1.0" +authors = [] +edition = "2018" +license = "MIT" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] +ansi_term = "*" +wildcards-test-dep = { path = "../dependency", version = "0.1.0" } diff --git a/tests/test_data/wildcards/maincrate/deny.toml b/tests/test_data/wildcards/maincrate/deny.toml new file mode 100644 index 00000000..b2d67312 --- /dev/null +++ b/tests/test_data/wildcards/maincrate/deny.toml @@ -0,0 +1 @@ +wildcards = "deny" diff --git a/tests/test_data/wildcards/maincrate/src/main.rs b/tests/test_data/wildcards/maincrate/src/main.rs new file mode 100644 index 00000000..f328e4d9 --- /dev/null +++ b/tests/test_data/wildcards/maincrate/src/main.rs @@ -0,0 +1 @@ +fn main() {}