Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add version = 2 #611

Merged
merged 3 commits into from
Feb 23, 2024
Merged

Add version = 2 #611

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b4bfa11
Add version = 2 to advisories and licenses
Jake-Shadle Feb 22, 2024
02790ab
Remove commented out block
Jake-Shadle Feb 22, 2024
b559d04
Remove pointless doc test
Jake-Shadle Feb 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ time = { version = "0.3", default-features = false, features = [
"macros",
] }
# Deserialization of configuration files and crate manifests
toml-span = { version = "0.1.0", features = ["reporting"] }
toml-span = { version = "0.2", features = ["reporting"] }
# Small fast hash crate
twox-hash = { version = "1.5", default-features = false }
# Url parsing/manipulation
Expand All @@ -138,7 +138,7 @@ fs_extra = "1.3"
insta = { version = "1.21", features = ["json"] }
tame-index = { version = "0.9", features = ["local-builder"] }
time = { version = "0.3", features = ["serde"] }
toml-span = { version = "0.1.0", features = ["serde"] }
toml-span = { version = "0.2", features = ["serde"] }
# We use this for creating fake crate directories for crawling license files on disk
tempfile = "3.1.0"

Expand Down
13 changes: 3 additions & 10 deletions deny.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,9 @@ targets = [
all-features = true

[advisories]
vulnerability = "deny"
unmaintained = "deny"
notice = "deny"
unsound = "deny"
version = 2
ignore = [
# rmp-serde used by askalono for the cache files, these are always utf-8 so
# the advisory is not relevant
"RUSTSEC-2022-0092",
{ id = "RUSTSEC-2022-0092", reason = "askalono always provides valid utf-8 files from a cache, this is not relevant" },
]

[bans]
Expand All @@ -44,9 +39,7 @@ unknown-registry = "deny"
unknown-git = "deny"

[licenses]
unlicensed = "deny"
allow-osi-fsf-free = "neither"
copyleft = "deny"
version = 2
# We want really high confidence when inferring licenses from text
confidence-threshold = 0.93
allow = [
Expand Down
84 changes: 43 additions & 41 deletions src/advisories/cfg.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,30 +64,34 @@ impl PartialEq for IgnoreId {

impl Eq for IgnoreId {}

pub struct Config {
/// Path to the root directory where advisory databases are stored (default: $CARGO_HOME/advisory-dbs)
pub db_path: Option<PathBuf>,
/// List of urls to git repositories of different advisory databases.
pub db_urls: Vec<Spanned<Url>>,
#[cfg_attr(test, derive(serde::Serialize))]
pub(crate) struct Deprecated {
/// How to handle crates that have a security vulnerability
pub vulnerability: LintLevel,
/// How to handle crates that have been marked as unmaintained in an advisory database
pub unmaintained: LintLevel,
/// How to handle crates that have been marked as unsound in an advisory database
pub unsound: LintLevel,
/// How to handle crates that have been yanked from eg crates.io
pub yanked: Spanned<LintLevel>,
/// How to handle crates that have been marked with a notice in the advisory database
pub notice: LintLevel,
/// Ignore advisories for the given IDs
ignore: Vec<Spanned<IgnoreId>>,
/// Ignore yanked crates
pub ignore_yanked: Vec<Spanned<PackageSpecOrExtended<Reason>>>,
/// CVSS Qualitative Severity Rating Scale threshold to alert at.
///
/// Vulnerabilities with explicit CVSS info which have a severity below
/// this threshold will be ignored.
pub severity_threshold: Option<advisory::Severity>,
}

pub struct Config {
/// Path to the root directory where advisory databases are stored (default: $CARGO_HOME/advisory-dbs)
pub db_path: Option<PathBuf>,
/// List of urls to git repositories of different advisory databases.
pub db_urls: Vec<Spanned<Url>>,
/// How to handle crates that have been yanked from eg crates.io
pub yanked: Spanned<LintLevel>,
/// Ignore advisories for the given IDs
ignore: Vec<Spanned<IgnoreId>>,
/// Ignore yanked crates
pub ignore_yanked: Vec<Spanned<PackageSpecOrExtended<Reason>>>,
/// Use the git executable to fetch advisory database rather than gitoxide
pub git_fetch_with_cli: Option<bool>,
/// If set to true, the local crates indices are not checked for yanked crates
Expand All @@ -100,7 +104,8 @@ pub struct Config {
/// use the '.' separator instead of ',' which is used by some locales and
/// supported in the RFC3339 format, but not by this implementation
pub maximum_db_staleness: Spanned<Duration>,
deprecated: Vec<Span>,
deprecated: Option<Deprecated>,
deprecated_spans: Vec<Span>,
}

impl Default for Config {
Expand All @@ -110,16 +115,12 @@ impl Default for Config {
db_urls: Vec::new(),
ignore: Vec::new(),
ignore_yanked: Vec::new(),
vulnerability: LintLevel::Deny,
unmaintained: LintLevel::Warn,
unsound: LintLevel::Warn,
yanked: Spanned::new(LintLevel::Warn),
notice: LintLevel::Warn,
severity_threshold: None,
git_fetch_with_cli: None,
disable_yank_checking: false,
maximum_db_staleness: Spanned::new(Duration::seconds_f64(NINETY_DAYS)),
deprecated: Vec::new(),
deprecated: None,
deprecated_spans: Vec::new(),
}
}
}
Expand All @@ -130,6 +131,8 @@ impl<'de> Deserialize<'de> for Config {
fn deserialize(value: &mut Value<'de>) -> Result<Self, toml_span::DeserError> {
let mut th = TableHelper::new(value)?;

let version = th.optional("version").unwrap_or(1);

let db_path = th.optional::<String>("db-path").map(PathBuf::from);
let db_urls = if let Some((_, mut urls)) = th.take("db-urls") {
let mut u = Vec::new();
Expand Down Expand Up @@ -158,15 +161,14 @@ impl<'de> Deserialize<'de> for Config {

let mut fdeps = Vec::new();

let vulnerability =
deprecated(&mut th, "vulnerability", &mut fdeps).unwrap_or(LintLevel::Deny);
let unmaintained =
deprecated(&mut th, "unmaintained", &mut fdeps).unwrap_or(LintLevel::Warn);
let unsound = deprecated(&mut th, "unsound", &mut fdeps).unwrap_or(LintLevel::Warn);
let vulnerability = deprecated(&mut th, "vulnerability", &mut fdeps);
let unmaintained = deprecated(&mut th, "unmaintained", &mut fdeps);
let unsound = deprecated(&mut th, "unsound", &mut fdeps);
let notice = deprecated(&mut th, "notice", &mut fdeps);

let yanked = th
.optional_s("yanked")
.unwrap_or(Spanned::new(LintLevel::Warn));
let notice = deprecated(&mut th, "notice", &mut fdeps).unwrap_or(LintLevel::Warn);
let (ignore, ignore_yanked) = if let Some((_, mut ignore)) = th.take("ignore") {
let mut u = Vec::new();
let mut y = Vec::new();
Expand Down Expand Up @@ -303,21 +305,29 @@ impl<'de> Deserialize<'de> for Config {
let maximum_db_staleness = maximum_db_staleness
.unwrap_or_else(|| Spanned::new(Duration::seconds_f64(NINETY_DAYS)));

let deprecated = if version <= 1 {
Some(Deprecated {
vulnerability: vulnerability.unwrap_or(LintLevel::Deny),
unmaintained: unmaintained.unwrap_or(LintLevel::Warn),
unsound: unsound.unwrap_or(LintLevel::Warn),
notice: notice.unwrap_or(LintLevel::Warn),
severity_threshold,
})
} else {
None
};
Comment on lines +308 to +318
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you're skipping the deprecated fields here if version >= 2, would it be more sensible to return an error when the fields are still set?


Ok(Self {
db_path,
db_urls,
vulnerability,
unmaintained,
unsound,
yanked,
notice,
ignore,
ignore_yanked,
severity_threshold,
git_fetch_with_cli,
disable_yank_checking,
maximum_db_staleness,
deprecated: fdeps,
deprecated,
deprecated_spans: fdeps,
})
}
}
Expand Down Expand Up @@ -349,7 +359,7 @@ impl crate::cfg::UnvalidatedConfig for Config {

// Output any deprecations, we'll remove the fields at the same time we
// remove all the logic they drive
for dep in self.deprecated {
for dep in self.deprecated_spans {
ctx.push(
Deprecated {
reason: DeprecationReason::WillBeRemoved(Some(
Expand All @@ -376,12 +386,8 @@ impl crate::cfg::UnvalidatedConfig for Config {
file_id: ctx.cfg_id,
})
.collect(),
vulnerability: self.vulnerability,
unmaintained: self.unmaintained,
unsound: self.unsound,
deprecated: self.deprecated,
yanked: self.yanked,
notice: self.notice,
severity_threshold: self.severity_threshold,
git_fetch_with_cli: self.git_fetch_with_cli.unwrap_or_default(),
disable_yank_checking: self.disable_yank_checking,
maximum_db_staleness: self.maximum_db_staleness,
Expand All @@ -396,12 +402,8 @@ pub struct ValidConfig {
pub db_urls: Vec<Spanned<Url>>,
pub(crate) ignore: Vec<IgnoreId>,
pub(crate) ignore_yanked: Vec<crate::bans::SpecAndReason>,
pub vulnerability: LintLevel,
pub unmaintained: LintLevel,
pub unsound: LintLevel,
pub(crate) deprecated: Option<Deprecated>,
pub yanked: Spanned<LintLevel>,
pub notice: LintLevel,
pub severity_threshold: Option<advisory::Severity>,
pub git_fetch_with_cli: bool,
pub disable_yank_checking: bool,
pub maximum_db_staleness: Spanned<Duration>,
Expand Down
57 changes: 28 additions & 29 deletions src/advisories/diags.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,26 +89,18 @@ impl<'a> crate::CheckCtx<'a, super::cfg::ValidConfig> {
let mut pack = Pack::with_kid(Check::Advisories, krate.id.clone());

let (severity, ty) = {
let (lint_level, ty) = match &advisory.informational {
// Everything that isn't an informational advisory is a vulnerability
None => (self.cfg.vulnerability, AdvisoryType::Vulnerability),
Some(info) => match info {
// Security notices for a crate which are published on https://rustsec.org
// but don't represent a vulnerability in a crate itself.
Informational::Notice => (self.cfg.notice, AdvisoryType::Notice),
let adv_ty = advisory.informational.as_ref().map_or(AdvisoryType::Vulnerability, |info| {
match info {
// Crate is unmaintained / abandoned
Informational::Unmaintained => {
(self.cfg.unmaintained, AdvisoryType::Unmaintained)
Informational::Unmaintained => AdvisoryType::Unmaintained,
Informational::Unsound => AdvisoryType::Unsound,
Informational::Notice => AdvisoryType::Notice,
Informational::Other(other) => {
unreachable!("rustsec only returns Informational::Other({other}) advisories if we ask, and there are none at the moment to ask for");
}
Informational::Unsound => (self.cfg.unsound, AdvisoryType::Unsound),
// Other types of informational advisories: left open-ended to add
// more of them in the future.
Informational::Other(_) => {
unreachable!("rustsec only returns these if we ask, and there are none at the moment to ask for");
}
_ => unreachable!("unknown advisory type encountered"),
},
};
_ => unreachable!("non_exhaustive enums are the worst"),
}
});

// Ok, we found a crate whose version lies within the range of an
// advisory, but the user might have decided to ignore it
Expand All @@ -132,22 +124,29 @@ impl<'a> crate::CheckCtx<'a, super::cfg::ValidConfig> {
);

LintLevel::Allow
} else if let Some(severity_threshold) = self.cfg.severity_threshold {
if let Some(advisory_severity) = advisory.cvss.as_ref().map(|cvss| cvss.severity())
{
if advisory_severity < severity_threshold {
LintLevel::Allow
} else {
lint_level
} else if let Some(deprecated) = &self.cfg.deprecated {
'll: {
if let (Some(st), Some(sev)) = (
deprecated.severity_threshold,
advisory.cvss.as_ref().map(|c| c.severity()),
) {
if sev < st {
break 'll LintLevel::Allow;
}
}

match adv_ty {
AdvisoryType::Vulnerability => deprecated.vulnerability,
AdvisoryType::Unmaintained => deprecated.unmaintained,
AdvisoryType::Unsound => deprecated.unsound,
AdvisoryType::Notice => deprecated.notice,
}
} else {
lint_level
}
} else {
lint_level
LintLevel::Deny
};

(lint_level.into(), ty)
(lint_level.into(), adv_ty)
};

let mut notes = get_notes_from_advisory(advisory);
Expand Down
12 changes: 7 additions & 5 deletions ...dvisories/snapshots/cargo_deny__advisories__cfg__test__deserializes_advisories_cfg-2.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,14 @@ expression: validated
"use-instead": null
}
],
"vulnerability": "deny",
"unmaintained": "warn",
"unsound": "warn",
"deprecated": {
"vulnerability": "deny",
"unmaintained": "warn",
"unsound": "warn",
"notice": "warn",
"severity_threshold": "medium"
},
"yanked": "warn",
"notice": "warn",
"severity_threshold": "medium",
"git_fetch_with_cli": false,
"disable_yank_checking": false,
"maximum_db_staleness": [
Expand Down
Loading