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.

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

Copyleft "warn" will allow a license #354

Closed
lfrancke opened this issue Jul 16, 2021 · 1 comment · Fixed by #611
Closed

Copyleft "warn" will allow a license #354

lfrancke opened this issue Jul 16, 2021 · 1 comment · Fixed by #611
Labels
bug Something isn't working

Comments

@lfrancke
Copy link

lfrancke commented Jul 16, 2021

Describe the bug

When copyleft is set to warn (which is the default) all the precedence checks after point 3 in this list https://embarkstudios.github.io/cargo-deny/checks/licenses/index.html#evaluation-precedence will not be considered anymore because warn is treated as allow:

https://github.com/EmbarkStudios/cargo-deny/blob/main/src/licenses/mod.rs#L127-L138

                    match cfg.copyleft {
                        LintLevel::Allow => {
                            allow!(IsCopyleft);
                        }
                        LintLevel::Warn => {
                            warnings += 1;
                            allow!(IsCopyleft);
                        }
                        LintLevel::Deny => {
                            deny!(IsCopyleft);
                        }
                    }

This basically resembles this config:

[licenses]

copyleft = "warn"
default = "deny"

Expected behavior
I would expect warn to behave like neither from allow-osi-fsf-free with an additional warning message: Fall through to the next check.

In theory I'd be happy to provide a PR but I haven't looked at the code yet on how to emit a warning and continue.

Device:

  • Version 0.9.1
@lfrancke lfrancke added the bug Something isn't working label Jul 16, 2021
@reivilibre
Copy link

I find it odd how, with the default config from cargo deny init, I get failures for permissive licences like MIT but only warning[accepted] for copyleft licences like MPL-2.0.

I would expect the default opinion for the tool to not take a stance on copyleft and let you specify individual licences to approve like normal.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants