Skip to content

Commit

Permalink
Remove deprecated config fields (#686)
Browse files Browse the repository at this point in the history
This finishes up the deprecation's of advisories and licenses config
fields done in #611, turning them into errors.
  • Loading branch information
Jake-Shadle authored Aug 2, 2024
1 parent 777b3df commit 580f9af
Show file tree
Hide file tree
Showing 29 changed files with 142 additions and 645 deletions.
1 change: 0 additions & 1 deletion deny.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ unknown-registry = "deny"
unknown-git = "deny"

[licenses]
version = 2
# We want really high confidence when inferring licenses from text
confidence-threshold = 0.93
allow = [
Expand Down
58 changes: 1 addition & 57 deletions docs/src/checks/advisories/cfg.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,7 @@ Default: `$CARGO_HOME/advisory-dbs`
version = 2
```

The advisories section has an upcoming breaking change, with deprecation warnings for several fields that will be removed. Setting `version = 2` will opt-in to the future default behavior.

The breaking change is as follows:
The version field is (at the time of this writing) no longer used, the following fields have been removed and will now emit errors.

- `vulnerability` - Removed, all vulnerability advisories now emit errors.
- `unmaintained` - Removed, all unmaintained advisories now emit errors.
Expand All @@ -48,36 +46,6 @@ The breaking change is as follows:

As before, if you want to ignore a specific advisory, add it to the `ignore` field.

### The `vulnerability` field (optional)

[**DEPRECATED**](#the-version-field-optional)

Determines what happens when a crate with a security vulnerability is encountered.

- `deny` (default) - Will emit an error with details about each vulnerability, and fail the check.
- `warn` - Prints a warning for each vulnerability, but does not fail the check.
- `allow` - Prints a note about the security vulnerability, but does not fail the check.

### The `unmaintained` field (optional)

[**DEPRECATED**](#the-version-field-optional)

Determines what happens when a crate with an `unmaintained` advisory is encountered.

- `deny` - Will emit an error with details about the unmaintained advisory, and fail the check.
- `warn` (default) - Prints a warning for each unmaintained advisory, but does not fail the check.
- `allow` - Prints a note about the unmaintained advisory, but does not fail the check.

### The `unsound` field (optional)

[**DEPRECATED**](#the-version-field-optional)

Determines what happens when a crate with an `unsound` advisory is encountered.

- `deny` - Will emit an error with details about the unsound advisory, and fail the check.
- `warn` (default) - Prints a warning for each unsound advisory, but does not fail the check.
- `allow` - Prints a note about the unsound advisory, but does not fail the check.

### The `yanked` field (optional)

Determines what happens when a crate with a version that has been yanked from its source registry is encountered.
Expand All @@ -86,18 +54,6 @@ Determines what happens when a crate with a version that has been yanked from it
- `warn` (default) - Prints a warning with the crate name and version that was yanked, but does not fail the check.
- `allow` - Prints a note about the yanked crate, but does not fail the check.

### The `notice` field (optional)

[**DEPRECATED**](#the-version-field-optional)

Determines what happens when a crate with a `notice` advisory is encountered.

**NOTE**: As of 2019-12-17 there are no `notice` advisories in the [RustSec Advisory DB](https://github.com/RustSec/advisory-db)

- `deny` - Will emit an error with details about the notice advisory, and fail the check.
- `warn` (default) - Prints a warning for each notice advisory, but does not fail the check.
- `allow` - Prints a note about the notice advisory, but does not fail the check.

### The `ignore` field (optional)

```ini
Expand All @@ -113,18 +69,6 @@ Every advisory in the advisory database contains a unique identifier, eg. `RUSTS

In addition, yanked crate versions can be ignored by specifying a [PackageSpec](../cfg.md#package-spec) with an optional `reason`.

### The `severity-threshold` field (optional)

[**DEPRECATED**](#the-version-field-optional)

The threshold for security vulnerabilities to be turned into notes instead of warnings or errors, depending upon its [CVSS](https://en.wikipedia.org/wiki/Common_Vulnerability_Scoring_System) score. So having a high threshold means some vulnerabilities might not fail the check, but having a log level `>= info` will mean that a note will be printed instead of a warning or error, depending on `[advisories.vulnerability]`.

- `None` (default) - CVSS Score 0.0
- `Low` - CVSS Score 0.1 - 3.9
- `Medium` - CVSS Score 4.0 - 6.9
- `High` - CVSS Score 7.0 - 8.9
- `Critical` - CVSS Score 9.0 - 10.0

### The `git-fetch-with-cli` field (optional)

Similar to cargo's [net.git-fetch-with-cli](https://doc.rust-lang.org/cargo/reference/config.html#netgit-fetch-with-cli), this field allows you to opt-in to fetching advisory databases with the git CLI rather than using `gix`.
Expand Down
58 changes: 3 additions & 55 deletions docs/src/checks/licenses/cfg.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,30 +51,17 @@ If `true`, licenses are checked even for `dev-dependencies`. By default this is
version = 2
```

The licenses section has an upcoming breaking change, with deprecation warnings for several fields that will be removed. Setting `version = 2` will opt-in to the future default behavior.

The breaking change is as follows:
The version field is (at the time of this writing) no longer used, the following fields have been removed and will now emit errors.

- `unlicensed` - Removed, if a crate is unlicensed you should open an issue/PR to fix it, and in the meantime, you may add a [clarification](#the-clarify-field-optional).
- `deny` - Removed, all licenses are denied unless explicitly allowed
- `copyleft` - Removed, all licenses are denied unless explicitly allowed
- `allow-osi-fsf-free` - Removed, all licenses are denied unless explicitly allowed
- `default` - Removed, all licenses are denied unless explicitly allowed

### The `unlicensed` field (optional)

Determines what happens when a crate has not explicitly specified its license terms, and no license information could be confidently detected via `LICENSE*` files in the crate's source.

- `deny` (default) - All unlicensed crates will emit an error and fail the license check
- `allow` - All unlicensed crates will show a note, but will not fail the license check
- `warn` - All unlicensed crates will show a warning, but will not fail the license check

### The `allow` and `deny` fields (optional)

The licenses that should be allowed or denied, note that the same license cannot
appear in both the `allow` and `deny` lists.
### The `allow` field (optional)

[`deny` is **DEPRECATED**](#the-version-field-optional)
The licenses that are explicitly allowed.

#### Note on GNU licenses

Expand All @@ -90,7 +77,6 @@ So, for example, if you wanted to disallow `GPL-2.0` licenses, but allow `GPL-3.
```ini
[licenses]
allow = [ "GPL-3.0" ]
deny = [ "GPL-2.0" ]
```

This gets worse with the GFDL licenses, which also have an `invariants` modifier. Before licenses are checked they are normalized to make them consistent for all licenses.
Expand Down Expand Up @@ -150,44 +136,6 @@ exceptions = [
]
```

### The `copyleft` field (optional)

[**DEPRECATED**](#the-version-field-optional)

Determines what happens when a license that is considered [copyleft](https://www.gnu.org/licenses/license-list.html) is encountered.

- `warn` (default) - Will emit a warning that a copyleft license was detected, but will not fail the license check
- `deny` - The license is not accepted if it is copyleft, but the license check might not fail if the expression still evaluates to true
- `allow` - The license is accepted if it is copyleft

### The `allow-osi-fsf-free` field (optional)

[**DEPRECATED**](#the-version-field-optional)

Determines what happens when licenses aren't explicitly allowed or denied, but **are** marked as [OSI Approved](https://opensource.org/licenses) or [FSF Free/Libre](https://www.gnu.org/licenses/license-list.en.html) in version 3.23 of the [SPDX License List](https://spdx.org/licenses/).

- `both` - The license is accepted if it is both OSI approved and FSF Free
- `either` - The license is accepted if it is either OSI approved or FSF Free
- `osi` - The license is accepted if it is OSI approved
- `fsf` - The license is accepted if it is FSF Free
- `osi-only` - The license is accepted if it is OSI approved and not FSF Free
- `fsf-only` - The license is accepted if it is FSF Free and not OSI approved
- `neither` (default) - No special consideration is given the license

### The `default` field (optional)

[**DEPRECATED**](#the-version-field-optional)

Determines what happens when a license is encountered that:

1. Isn't in the `allow` or `deny` lists
1. Isn't `copyleft`
1. Isn't OSI Approved nor FSF Free/Libre, or `allow-osi-fsf-free = "neither"`

- `warn` - Will emit a warning that the license was detected, but will not fail the license check
- `deny` (default) - The license is not accepted, but the license check might not fail if the expression still evaluates to true
- `allow` - The license is accepted

### The `confidence-threshold` field (optional)

`cargo-deny` uses [askalono](https://github.com/amzn/askalono) to determine the license of a LICENSE file. Due to variability in license texts because of things like authors, copyright year, and so forth, askalano assigns a confidence score to its determination, from `0.0` (no confidence) to `1.0` (perfect match). The confidence threshold value is used to reject the license determination if the score does not match or exceed the threshold.
Expand Down
52 changes: 9 additions & 43 deletions src/advisories/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,23 +65,6 @@ impl PartialEq for IgnoreId {

impl Eq for IgnoreId {}

#[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 marked with a notice in the advisory database
pub notice: LintLevel,
/// 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<Spanned<PathBuf>>,
Expand All @@ -105,7 +88,6 @@ 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: Option<Deprecated>,
deprecated_spans: Vec<Span>,
}

Expand All @@ -120,7 +102,6 @@ impl Default for Config {
git_fetch_with_cli: None,
disable_yank_checking: false,
maximum_db_staleness: Spanned::new(Duration::seconds_f64(NINETY_DAYS)),
deprecated: None,
deprecated_spans: Vec::new(),
}
}
Expand All @@ -132,7 +113,7 @@ 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 _version = th.optional("version").unwrap_or(1);

let db_path = th.optional_s::<String>("db-path").map(|s| s.map());
let db_urls = if let Some((_, mut urls)) = th.take("db-urls") {
Expand Down Expand Up @@ -162,10 +143,10 @@ impl<'de> Deserialize<'de> for Config {

let mut fdeps = Vec::new();

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 _vulnerability = deprecated::<LintLevel>(&mut th, "vulnerability", &mut fdeps);
let _unmaintained = deprecated::<LintLevel>(&mut th, "unmaintained", &mut fdeps);
let _unsound = deprecated::<LintLevel>(&mut th, "unsound", &mut fdeps);
let _notice = deprecated::<LintLevel>(&mut th, "notice", &mut fdeps);

let yanked = th
.optional_s("yanked")
Expand Down Expand Up @@ -256,7 +237,7 @@ impl<'de> Deserialize<'de> for Config {
}
};

match s.parse() {
match s.parse::<advisory::Severity>() {
Ok(st) => Some(st),
Err(err) => {
th.errors.push(
Expand All @@ -273,7 +254,7 @@ impl<'de> Deserialize<'de> for Config {
}
};

let severity_threshold = st(&mut th, &mut fdeps);
let _severity_threshold = st(&mut th, &mut fdeps);
let git_fetch_with_cli = th.optional("git-fetch-with-cli");
let disable_yank_checking = th.optional("disable-yank-checking").unwrap_or_default();
let maximum_db_staleness = if let Some((_, mut val)) = th.take("maximum-db-staleness") {
Expand Down Expand Up @@ -306,18 +287,6 @@ 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
};

Ok(Self {
db_path,
db_urls,
Expand All @@ -327,7 +296,6 @@ impl<'de> Deserialize<'de> for Config {
git_fetch_with_cli,
disable_yank_checking,
maximum_db_staleness,
deprecated,
deprecated_spans: fdeps,
})
}
Expand Down Expand Up @@ -408,9 +376,9 @@ impl crate::cfg::UnvalidatedConfig for Config {
for dep in self.deprecated_spans {
ctx.push(
Deprecated {
reason: DeprecationReason::WillBeRemoved(Some(
reason: DeprecationReason::Removed(
"https://github.com/EmbarkStudios/cargo-deny/pull/611",
)),
),
key: dep,
file_id: ctx.cfg_id,
}
Expand All @@ -432,7 +400,6 @@ impl crate::cfg::UnvalidatedConfig for Config {
file_id: ctx.cfg_id,
})
.collect(),
deprecated: self.deprecated,
yanked: self.yanked,
git_fetch_with_cli: self.git_fetch_with_cli.unwrap_or_default(),
disable_yank_checking: self.disable_yank_checking,
Expand All @@ -448,7 +415,6 @@ pub struct ValidConfig {
pub db_urls: Vec<Spanned<Url>>,
pub(crate) ignore: Vec<IgnoreId>,
pub(crate) ignore_yanked: Vec<crate::bans::SpecAndReason>,
pub(crate) deprecated: Option<Deprecated>,
pub yanked: Spanned<LintLevel>,
pub git_fetch_with_cli: bool,
pub disable_yank_checking: bool,
Expand Down
18 changes: 0 additions & 18 deletions src/advisories/diags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,24 +123,6 @@ impl<'a> crate::CheckCtx<'a, super::cfg::ValidConfig> {
);

LintLevel::Allow
} 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 {
LintLevel::Deny
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,6 @@ expression: validated
"use-instead": null
}
],
"deprecated": {
"vulnerability": "deny",
"unmaintained": "warn",
"unsound": "warn",
"notice": "warn",
"severity_threshold": "medium"
},
"yanked": "warn",
"git_fetch_with_cli": false,
"disable_yank_checking": false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,32 +2,4 @@
source: src/advisories/cfg.rs
expression: diags
---
warning[deprecated]: this key will be removed in a future update, see https://github.com/EmbarkStudios/cargo-deny/pull/611 for details
β”Œβ”€ tests/cfg/advisories.toml:4:1
β”‚
4 β”‚ vulnerability = "deny"
β”‚ ━━━━━━━━━━━━━

warning[deprecated]: this key will be removed in a future update, see https://github.com/EmbarkStudios/cargo-deny/pull/611 for details
β”Œβ”€ tests/cfg/advisories.toml:5:1
β”‚
5 β”‚ unmaintained = "warn"
β”‚ ━━━━━━━━━━━━

warning[deprecated]: this key will be removed in a future update, see https://github.com/EmbarkStudios/cargo-deny/pull/611 for details
β”Œβ”€ tests/cfg/advisories.toml:6:1
β”‚
6 β”‚ unsound = "warn"
β”‚ ━━━━━━━

warning[deprecated]: this key will be removed in a future update, see https://github.com/EmbarkStudios/cargo-deny/pull/611 for details
β”Œβ”€ tests/cfg/advisories.toml:8:1
β”‚
8 β”‚ notice = "warn"
β”‚ ━━━━━━

warning[deprecated]: this key will be removed in a future update, see https://github.com/EmbarkStudios/cargo-deny/pull/611 for details
β”Œβ”€ tests/cfg/advisories.toml:14:1
β”‚
14 β”‚ severity-threshold = "medium"
β”‚ ━━━━━━━━━━━━━━━━━━
Loading

0 comments on commit 580f9af

Please sign in to comment.