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

Fail retrieving vulnerable dependencies unless overriden #4333

Open
alestiago opened this issue Jul 23, 2024 · 5 comments
Open

Fail retrieving vulnerable dependencies unless overriden #4333

alestiago opened this issue Jul 23, 2024 · 5 comments
Labels
type-enhancement A request for a change that isn't a bug

Comments

@alestiago
Copy link

alestiago commented Jul 23, 2024

Description

As a developer, I would like the dart pub get to fail when retrieving vulnerable versions unless I explicitly provide a flag to override it.

Context

When a Dart package has a dependency on a vulnerable package (those with an an entry on the OSV database) the dart pub get command already reports the affected dependencies.

For example:

dart pub get
Resolving dependencies... 
Downloading packages... 
  _fe_analyzer_shared 68.0.0 (73.0.0 available)
  analyzer 6.5.0 (6.8.0 available)
  http 0.13.2 (affected by advisory: [^0], 1.2.2 available)
  macros 0.1.0-main.0 (0.1.2-main.4 available)
Got dependencies!
Dependencies are affected by security advisories:
  [^0]: https://github.com/advisories/GHSA-4rgh-jx4f-qfcq
4 packages have newer versions incompatible with dependency constraints.
Try `dart pub outdated` for more information.

The above succeeds, instead I would like it to fail and only succeed when providing a flag to override it if needed (for example, --accept-vulnerabilities).

Use-cases

  1. Continuous Integration.

Most Dart developers are already running dart pub get on their CIs, if the CI fails on the dart pub get stage due to a vulnerability it will block them until it gets resolved, hence avoiding vulnerabilities to creep in. Most developers only check if their CI is passing or not, hence they might completely miss the message.

In addition, this also accommodates for the limitations the OSV Scanner has, these are:

  • There is only a reusable action for GitHub (users of other Git related services can't benefit)
  • The scanner requires an existing pubspec.lock to exist, however the Dart team recommends to ignore comiting the pubspec.lock file for packages (What not to commit, Dart Documentation).
  1. Executables

Dart can also install executables to be run locally as a CLI (for example, with: dart pub global activate). When activating the executable the dependencies are fetched. If getting dependencies fails it would protect the user from having vulnerabilities. Only if the user explicitly wants to by-pass and accept the consequences of having a vulnerability the local CLI should be installed (a similar experience to what the browser does when trying to access a non-secure website).

  1. Raises awareness and improves the ecosystem

We should never depend on vulnerable dependencies. Making the get fail by default would raise awareness that Dart dependencies can also be vulnerable and the community will soon know which are those and how to prevent new vulnerabilities.


P.S.: If the feature is not possible for whatever reason, having a flag (for example, --no-vulnerabilities) that could be provided to fail when vulnerable dependencies are detected could be considered instead.

@sigurdm
Copy link
Contributor

sigurdm commented Jul 29, 2024

We should never depend on vulnerable dependencies.

This is not clear to me. A vulnerability might not affect a given consumer of a package.

The scanner requires an existing pubspec.lock to exist, however the Dart team recommends to ignore comiting the pubspec.lock file for packages (What not to commit, Dart Documentation).

If you are making a package for others to consume, you cannot easily specify which version of your dependencies they will end up consuming - only what range of versions. It is up to them to avoid the vulnerabilities in their pubspec.lock. Committing a pubspec.lock file with the package would not help much.

A workaround if you want to not run CI with a dependency with a vulnerability, would be to have your github action detect "Dependencies are affected by security advisories" in the output from dart pub get and fail if present.

@szakarias and @jonasfj might have more to say.

@sigurdm sigurdm added the type-enhancement A request for a change that isn't a bug label Jul 29, 2024
@alestiago
Copy link
Author

Hi @sigurdm thanks for the reply.

This is not clear to me. A vulnerability might not affect a given consumer of a package.

I was referring to the fact that I think that depending on a vulnerable version should be avoided at all costs. It is true that you can depend on a dependency that is tagged as vulnerable but not be vulnerable (for example, depending on vulnerable Archive version but not using symlinks), however I believe such practice is like walking on a tight rope because there is no automated mechanism in place that ensures you won't be using such vulnerable functionalities in the future.

If you are making a package for others to consume, you cannot easily specify which version of your dependencies they will end up consuming - only what range of versions. It is up to them to avoid the vulnerabilities in their pubspec.lock. Committing a pubspec.lock file with the package would not help much.

You're completely right. However, if I'm maintaining a package and I get dependencies I will not like to get a vulnerable version installed in my system. Instead, I would like to be made aware that I'm about to install a vulnerable dependency, and then decide if I want to abort or continue.

A workaround if you want to not run CI with a dependency with a vulnerability, would be to have your github action detect "Dependencies are affected by security advisories" in the output from dart pub get and fail if present.

That would be nice and it would move the tooling closer to the ideal solution (i.e. give me the option before installing a vulnerable version locally). However, I would rather not have to string match the output of the command but instead pass a flag to avoid it being fragile to changes in the output formatting for example.

@mx1up
Copy link

mx1up commented Dec 18, 2024

i agree an option to generate a non zero exit code would be pratical. See my comment on an older issue: #2106 (comment)

@jonasfj
Copy link
Member

jonasfj commented Dec 18, 2024

I was referring to the fact that I think that depending on a vulnerable version should be avoided at all costs.

I think all costs is too high 🤣

My thoughts are that instead of adding all sorts of flags, we should add a policies section in pubspec.yaml which is then used to configure polices for a project.

For example, you might want to require that all dependencies have a verified publisher, there are no unignored advisories, and things like that.

Basically, a set of lints/requirements that are checked against you dependencies after they have been resolved.

This could allow a lot of flexible policies. One of these could be: all advisories affecting a package version in the resolution must be in the list of ignored_advisories.


If someone wants to prototype such a feature this could be done in a separate package. With policies written to separate dependency_policies.yaml.

@jonasfj
Copy link
Member

jonasfj commented Dec 18, 2024

You could also imagine a policy to forbid retracted package versions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

4 participants