From bedf0d1c4aaa3bb67f0cc83cce95da43c3875530 Mon Sep 17 00:00:00 2001 From: Dan Sully Date: Fri, 4 Aug 2023 10:42:30 -0700 Subject: [PATCH 1/2] Implement a solution for additional license exceptions. Implement a solution for additional license exceptions configuration as described in https://github.com/EmbarkStudios/cargo-deny/issues/541 --- docs/src/checks/licenses/cfg.md | 15 ++++++++++++ src/cargo-deny/check.rs | 43 ++++++++++++++++++++++++++++++--- src/cargo-deny/common.rs | 31 ++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 3 deletions(-) diff --git a/docs/src/checks/licenses/cfg.md b/docs/src/checks/licenses/cfg.md index eba7b90d2..9e02d0a6a 100644 --- a/docs/src/checks/licenses/cfg.md +++ b/docs/src/checks/licenses/cfg.md @@ -101,6 +101,21 @@ The name of the crate that you are adding an exception for An optional version constraint specifying the range of crate versions you are excepting. Defaults to any version. +### Additional exceptions configuration file + +In some cases it's useful to have global cargo-deny config and project-local exceptions. This can be accomplished with a project exceptions file in any of these locations relative to your top level `Cargo.toml` manifest file. + +`cargo-deny` will look for the following files: `/deny.exceptions.toml`, `/.deny.exceptions.toml` and `/.cargo/deny.exceptions.toml` + +Only the exceptions field should be set: + +```ini +exceptions = [ + # Each entry is the crate and version constraint, and its specific allow list. + { allow = ["CDDL-1.0"], name = "inferno", version = "*" }, +] +``` + #### The `allow` field This is the exact same as the general `allow` field. diff --git a/src/cargo-deny/check.rs b/src/cargo-deny/check.rs index 34af4bb4f..277a9be4e 100644 --- a/src/cargo-deny/check.rs +++ b/src/cargo-deny/check.rs @@ -147,6 +147,7 @@ struct ValidConfig { impl ValidConfig { fn load( cfg_path: Option, + exceptions_cfg_path: Option, files: &mut Files, log_ctx: crate::common::LogContext, ) -> Result { @@ -170,9 +171,44 @@ impl ValidConfig { } }; - let cfg: Config = toml::from_str(&cfg_contents) + let mut cfg: Config = toml::from_str(&cfg_contents) .with_context(|| format!("failed to deserialize config from '{cfg_path}'"))?; + // Allow for project-local exceptions. Relevant in corporate environments. + // https://github.com/EmbarkStudios/cargo-deny/issues/541 + // + // This isn't the cleanest, but cfg.licenses isn't mutable, so appending/extending the Vec + // isn't possible. Similarly, the various Config/ValidConfig structs don't derive from + // Copy/Clone, so cloning and updating isn't possible either. + // + // This is the most minimally invasive approach I could come up with. + if let Some(exceptions_cfg_path) = exceptions_cfg_path { + // TOML can't have unnamed arrays at the root. + #[derive(Deserialize)] + pub struct ExceptionsConfig { + pub exceptions: Vec, + } + + let content = std::fs::read_to_string(&exceptions_cfg_path) + .with_context(|| format!("failed to read config from {exceptions_cfg_path}"))?; + + let ex_cfg: ExceptionsConfig = toml::from_str(&content).with_context(|| { + format!("failed to deserialize config from '{exceptions_cfg_path}'") + })?; + + if cfg.licenses.is_some() { + let l = cfg.licenses.unwrap_or_default(); + + let exceptions = l + .exceptions + .into_iter() + .chain(ex_cfg.exceptions.into_iter()) + .collect(); + + cfg.licenses = Some(licenses::Config { exceptions, ..l }); + } + }; + log::info!("using config from {cfg_path}"); let id = files.add(&cfg_path, cfg_contents); @@ -270,6 +306,7 @@ pub(crate) fn cmd( features, } = ValidConfig::load( krate_ctx.get_config_path(args.config.clone()), + krate_ctx.get_local_exceptions_path(), &mut files, log_ctx, )?; @@ -332,7 +369,7 @@ pub(crate) fn cmd( match cl { CodeOrLevel::Code(code) => { if let Some(current) = code_overrides.get(code.as_str()) { - anyhow::bail!("unable to override code '{code}' to '{severity:?}', it has already been overriden to '{current:?}'"); + anyhow::bail!("unable to override code '{code}' to '{severity:?}', it has already been overridden to '{current:?}'"); } code_overrides.insert(code.as_str(), severity); @@ -348,7 +385,7 @@ pub(crate) fn cmd( } }) { - anyhow::bail!("unable to override level '{level:?}' to '{severity:?}', it has already been overriden to '{current:?}'"); + anyhow::bail!("unable to override level '{level:?}' to '{severity:?}', it has already been overridden to '{current:?}'"); } level_overrides.push((ls, severity)); diff --git a/src/cargo-deny/common.rs b/src/cargo-deny/common.rs index 36840ac45..36d203d2a 100644 --- a/src/cargo-deny/common.rs +++ b/src/cargo-deny/common.rs @@ -102,6 +102,37 @@ impl KrateContext { } } + // Allow for project-local exceptions. Relevant in corporate environments. + // https://github.com/EmbarkStudios/cargo-deny/issues/541 + pub fn get_local_exceptions_path(&self) -> Option { + let mut p = self.manifest_path.parent(); + + while let Some(parent) = p { + let mut config_path = parent.join("deny.exceptions.toml"); + + if config_path.exists() { + return Some(config_path); + } + + config_path.pop(); + config_path.push(".deny.exceptions.toml"); + + if config_path.exists() { + return Some(config_path); + } + + config_path.pop(); + config_path.push(".cargo/deny.exceptions.toml"); + if config_path.exists() { + return Some(config_path); + } + + p = parent.parent(); + } + + None + } + #[inline] pub fn fetch_krates(&self) -> anyhow::Result<()> { fetch(MetadataOptions { From 9867908160fffb9bd49b46abf558b628d4f50797 Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Sun, 3 Sep 2023 07:58:24 +0200 Subject: [PATCH 2/2] Cleanup --- src/cargo-deny/check.rs | 46 ++++------------- src/cargo-deny/common.rs | 2 - src/licenses.rs | 7 ++- src/licenses/cfg.rs | 108 +++++++++++++++++++++++++++++++-------- 4 files changed, 101 insertions(+), 62 deletions(-) diff --git a/src/cargo-deny/check.rs b/src/cargo-deny/check.rs index 277a9be4e..75e306e1b 100644 --- a/src/cargo-deny/check.rs +++ b/src/cargo-deny/check.rs @@ -171,44 +171,9 @@ impl ValidConfig { } }; - let mut cfg: Config = toml::from_str(&cfg_contents) + let cfg: Config = toml::from_str(&cfg_contents) .with_context(|| format!("failed to deserialize config from '{cfg_path}'"))?; - // Allow for project-local exceptions. Relevant in corporate environments. - // https://github.com/EmbarkStudios/cargo-deny/issues/541 - // - // This isn't the cleanest, but cfg.licenses isn't mutable, so appending/extending the Vec - // isn't possible. Similarly, the various Config/ValidConfig structs don't derive from - // Copy/Clone, so cloning and updating isn't possible either. - // - // This is the most minimally invasive approach I could come up with. - if let Some(exceptions_cfg_path) = exceptions_cfg_path { - // TOML can't have unnamed arrays at the root. - #[derive(Deserialize)] - pub struct ExceptionsConfig { - pub exceptions: Vec, - } - - let content = std::fs::read_to_string(&exceptions_cfg_path) - .with_context(|| format!("failed to read config from {exceptions_cfg_path}"))?; - - let ex_cfg: ExceptionsConfig = toml::from_str(&content).with_context(|| { - format!("failed to deserialize config from '{exceptions_cfg_path}'") - })?; - - if cfg.licenses.is_some() { - let l = cfg.licenses.unwrap_or_default(); - - let exceptions = l - .exceptions - .into_iter() - .chain(ex_cfg.exceptions.into_iter()) - .collect(); - - cfg.licenses = Some(licenses::Config { exceptions, ..l }); - } - }; - log::info!("using config from {cfg_path}"); let id = files.add(&cfg_path, cfg_contents); @@ -225,10 +190,17 @@ impl ValidConfig { .validate(id, files, &mut diags); let bans = cfg.bans.unwrap_or_default().validate(id, files, &mut diags); - let licenses = cfg + let mut licenses = cfg .licenses .unwrap_or_default() .validate(id, files, &mut diags); + + // Allow for project-local exceptions. Relevant in corporate environments. + // https://github.com/EmbarkStudios/cargo-deny/issues/541 + if let Some(ecp) = exceptions_cfg_path { + licenses::cfg::load_exceptions(&mut licenses, ecp, files, &mut diags); + }; + let sources = cfg .sources .unwrap_or_default() diff --git a/src/cargo-deny/common.rs b/src/cargo-deny/common.rs index 36d203d2a..3130c90c9 100644 --- a/src/cargo-deny/common.rs +++ b/src/cargo-deny/common.rs @@ -102,8 +102,6 @@ impl KrateContext { } } - // Allow for project-local exceptions. Relevant in corporate environments. - // https://github.com/EmbarkStudios/cargo-deny/issues/541 pub fn get_local_exceptions_path(&self) -> Option { let mut p = self.manifest_path.parent(); diff --git a/src/licenses.rs b/src/licenses.rs index 775c27a52..1b16c52fa 100644 --- a/src/licenses.rs +++ b/src/licenses.rs @@ -348,9 +348,14 @@ pub fn check( .zip(ctx.cfg.exceptions.into_iter()) .filter_map(|(hit, exc)| if !hit { Some(exc) } else { None }) { + // Don't print warnings for exception overrides + if exc.file_id != ctx.cfg.file_id { + continue; + } + pack.push(diags::UnmatchedLicenseException { license_exc_cfg: CfgCoord { - file: ctx.cfg.file_id, + file: exc.file_id, span: exc.name.span, }, }); diff --git a/src/licenses/cfg.rs b/src/licenses/cfg.rs index 99e4fe244..54e0bad30 100644 --- a/src/licenses/cfg.rs +++ b/src/licenses/cfg.rs @@ -189,6 +189,30 @@ impl Default for Config { } } +fn parse_license( + ls: &Spanned, + v: &mut Vec, + diags: &mut Vec, + cfg_file: FileId, +) { + match spdx::Licensee::parse(ls.as_ref()) { + Ok(licensee) => { + v.push(Licensee::new(licensee, ls.span.clone())); + } + Err(pe) => { + let offset = ls.span.start + 1; + let span = pe.span.start + offset..pe.span.end + offset; + diags.push( + Diagnostic::error() + .with_message("invalid licensee") + .with_labels(vec![ + Label::primary(cfg_file, span).with_message(format!("{}", pe.reason)) + ]), + ); + } + } +} + impl crate::cfg::UnvalidatedConfig for Config { type ValidCfg = ValidConfig; @@ -223,32 +247,14 @@ impl crate::cfg::UnvalidatedConfig for Config { } } - let mut parse_license = |ls: &Spanned, v: &mut Vec| { - match spdx::Licensee::parse(ls.as_ref()) { - Ok(licensee) => { - v.push(Licensee::new(licensee, ls.span.clone())); - } - Err(pe) => { - let offset = ls.span.start + 1; - let span = pe.span.start + offset..pe.span.end + offset; - diags.push( - Diagnostic::error() - .with_message("invalid licensee") - .with_labels(vec![Label::primary(cfg_file, span) - .with_message(format!("{}", pe.reason))]), - ); - } - } - }; - let mut denied = Vec::with_capacity(self.deny.len()); for d in &self.deny { - parse_license(d, &mut denied); + parse_license(d, &mut denied, diags, cfg_file); } let mut allowed: Vec = Vec::with_capacity(self.allow.len()); for a in &self.allow { - parse_license(a, &mut allowed); + parse_license(a, &mut allowed, diags, cfg_file); } denied.par_sort(); @@ -259,13 +265,14 @@ impl crate::cfg::UnvalidatedConfig for Config { let mut allowed = Vec::with_capacity(exc.allow.len()); for allow in &exc.allow { - parse_license(allow, &mut allowed); + parse_license(allow, &mut allowed, diags, cfg_file); } exceptions.push(ValidException { name: exc.name, version: exc.version, allowed, + file_id: cfg_file, }); } @@ -299,7 +306,7 @@ impl crate::cfg::UnvalidatedConfig for Config { Diagnostic::error() .with_message("unable to parse license expression") .with_labels(vec![Label::primary(cfg_file, expr_span) - .with_message(format!("{}", err.reason))]), + .with_message(err.reason.to_string())]), ); continue; @@ -336,6 +343,61 @@ impl crate::cfg::UnvalidatedConfig for Config { } } +pub fn load_exceptions( + cfg: &mut ValidConfig, + path: crate::PathBuf, + files: &mut crate::diag::Files, + diags: &mut Vec, +) { + // TOML can't have unnamed arrays at the root. + #[derive(Deserialize)] + pub struct ExceptionsConfig { + pub exceptions: Vec, + } + + let content = match std::fs::read_to_string(&path) { + Ok(c) => c, + Err(err) => { + diags.push( + Diagnostic::error() + .with_message("failed to read exceptions override") + .with_notes(vec![format!("path = '{path}'"), format!("error = {err:#}")]), + ); + return; + } + }; + + let exc_cfg: ExceptionsConfig = match toml::from_str(&content) { + Ok(ec) => ec, + Err(err) => { + diags.push( + Diagnostic::error() + .with_message("failed to deserialize exceptions override") + .with_notes(vec![format!("path = '{path}'"), format!("error = {err:#}")]), + ); + return; + } + }; + + let file_id = files.add(path, content); + + cfg.exceptions.reserve(exc_cfg.exceptions.len()); + for exc in exc_cfg.exceptions { + let mut allowed = Vec::with_capacity(exc.allow.len()); + + for allow in &exc.allow { + parse_license(allow, &mut allowed, diags, file_id); + } + + cfg.exceptions.push(ValidException { + name: exc.name, + version: exc.version, + allowed, + file_id, + }); + } +} + #[doc(hidden)] #[cfg_attr(test, derive(Debug, PartialEq))] pub struct ValidClarification { @@ -352,6 +414,7 @@ pub struct ValidException { pub name: crate::Spanned, pub version: Option, pub allowed: Vec, + pub file_id: FileId, } pub type Licensee = Spanned; @@ -423,6 +486,7 @@ mod test { name: "adler32".to_owned().fake(), allowed: vec![spdx::Licensee::parse("Zlib").unwrap().fake()], version: Some(semver::VersionReq::parse("0.1.1").unwrap()), + file_id: cd.id, }] ); let p: PathBuf = "LICENSE".into();