Surprising default configuration for advisories #449
Labels
bug
Something isn't working
Comments
|
The defaults were chose as warn basically because:
These could be of course changed it would just be a breaking change that would require a new version of the action as well. |
Jake-Shadle
added a commit
that referenced
this issue
Feb 21, 2024
This PR deprecates several fields which will be removed in a future update. I'll explain in detail why below, but the TLDR is that cargo-deny surfaces several configuration options that were added because we _could_, but not necessarily because they are useful in practice. ## Licenses ### [`deny`](https://embarkstudios.github.io/cargo-deny/checks/licenses/cfg.html#the-allow-and-deny-fields-optional) This field was only added for consistency with `[bans]` but makes no sense for `[licenses]`, if a license you don't explicitly allow is used it is implicitly denied. ### [`copyleft`](https://embarkstudios.github.io/cargo-deny/checks/licenses/cfg.html#the-copyleft-field-optional) There is no reason to treat these differently from any other license, if it's not explicitly allowed it should be denied, and it just adds confusion due to the terrible default. See: #602 See: #354 ### [`allow-osi-fsf-free`](https://embarkstudios.github.io/cargo-deny/checks/licenses/cfg.html#the-allow-osi-fsf-free-field-optional) Similarly to copyleft, this field just makes no sense and was only added because the SPDX metadata allowed us to query this information. ### [`default`](https://embarkstudios.github.io/cargo-deny/checks/licenses/cfg.html#the-default-field-optional) This was added so that users could just ignore/warn all their dependencies not following the set of allowed licenses, but just isn't much value. Even in large projects with literally hundreds of external dependencies the set of licenses that need to be allowed are relatively small compared to the total set of licenses in SPDX due to the Rust ecosystem generally using only a handful of licenses, with rare exceptions. ### [`unlicensed`](https://embarkstudios.github.io/cargo-deny/checks/licenses/cfg.html#the-unlicensed-field-optional) Crates that don't specify a license via `[package.license/license-file]` or have a license file in their package source are incredibly rare, and there is already a [mechanism](https://embarkstudios.github.io/cargo-deny/checks/licenses/cfg.html#the-clarify-field-optional) to provide/override license information for those rare crates. ## Advisories ### Blanket - [`vulnerability`](https://embarkstudios.github.io/cargo-deny/checks/advisories/cfg.html#the-vulnerability-field-optional) - [`unmaintained`](https://embarkstudios.github.io/cargo-deny/checks/advisories/cfg.html#the-unmaintained-field-optional) - [`unsound`](https://embarkstudios.github.io/cargo-deny/checks/advisories/cfg.html#the-unsound-field-optional) - [`notice`](https://embarkstudios.github.io/cargo-deny/checks/advisories/cfg.html#the-notice-field-optional) There's no need to blanket handle any of these specific advisory types, there just aren't enough advisories (currently, this could change in the future) that a typical workspace will encounter that they can't be handled explicitly via `ignore`. See: #449 ### [`severity-threshold`](https://embarkstudios.github.io/cargo-deny/checks/advisories/cfg.html#the-severity-threshold-field-optional) This optional field is available in rustsec advisories, but provides no real value as it's just flavor on top of a reported vulnerability, but doesn't fundamentally change that it is a vulnerability, and can either be ignored or better yet, updated to a version without the vulnerability.
Merged
Jake-Shadle
added a commit
that referenced
this issue
Feb 23, 2024
This is a follow up to #606 that actually provides a way to remove the deprecated fields and opt in to the new behavior until the fields are removed and the new behavior becomes the only behavior. Basically, `version = 2` can be added to the `[advisories]` and `[licenses]`, which opts in to the new behavior, and means any of the deprecated keys no longer impact the results of the checks. The new behavior is as follows: ### `[advisories]` - `vulnerability` - `deny` - `unmaintained` - `deny`, old default = `warn` - `unsound` - `deny`, old default = `warn` - `notice` - `deny`, old default = `warn` - `severity-threshold` - CVSS severity no longer considered Resolves: #449 ### `[licenses]` #### `unlicensed` New default of `deny`, old default was `warn`. If a crate is unlicensed, a [clarification](https://embarkstudios.github.io/cargo-deny/checks/licenses/cfg.html#the-clarify-field-optional) can be used to assign a license based on one or more source files in the package #### `allow-osi-fsf-free` Old default was `both`, the new default is `neither`, ie, it doesn't matter if the license is osi and/or fsf free, only if it is in the allow (or exception) list. #### `copyleft` Old default was `warn`, the new default is `deny, it only matters if the license is allowed in the allow or exception list. Resolves: #602 Resolves: #354 #### `default` Provided the default for a license not otherwise listed, now all licenses are `deny` unless explicitly in the allow or exception list. #### `deny` This list served no purpose, if the license is not in the allow or exception list, it is denied.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
I was surprised to note that RUSTSEC-2022-0049 (which I happen to be aware of as a chrono maintainer) only generated a warning. After looking at the documentation I noticed that
unsounddefaults towarn, which I find quite surprising. Similarly, I think something stricter would be a better default foryanked.In particular, this feedback pertains to running EmbarkStudios/cargo-deny-action@v1 in CI, where I'm pretty unlikely to notice the warnings -- and I think both of these should be noticeable by default when run in CI.
The text was updated successfully, but these errors were encountered: