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

Dependabot should gracefully handle the overrides field in package.json #5590

Open
2 tasks
Nishnha opened this issue Aug 25, 2022 · 0 comments
Open
2 tasks
Labels

Comments

@Nishnha
Copy link
Member

Nishnha commented Aug 25, 2022

🖼️ Context

npm 8.3.0 added support for the overrides field in package.json, which allows authors to force sub-dependencies to resolve to specific versions.

Example: top-level dependency foo@^1.3.0 depends on bar@^6.4.1, which is a vulnerable version. Fortunately there is a patch in bar@6.4.2. If, however, the author decided to force bar to resolve to the vulnerable version in the overrides field, it would be impossible to resolve the vuln. (This is precisely what happens with npm audit fix, i.e., it fails to fix the vuln.)

"overrides": {
  "foo@^1.3.0": {
    "bar": "6.4.1"
  }
}

When such an example is given to the dry-run script, Dependabot believes a fix is possible by updating from bar@6.4.1 to bar@6.4.1, and creates a PR stating as much, which is obviously nonsense.

Updates `pg` from 6.4.1 to 6.4.1
<details>
<summary>Commits</summary>
<ul>
<li>See full diff in <a href="https://github.com/brianc/node-postgres/commits/v6.4.1/packages/pg">compare view</a></li>
</ul>
</details>
<br />

While this is unlikely to come up often and is arguably user error, Dependabot should still gracefully handle this field.

There 2 situations where we would definitely want to raise an update not possible error:

  1. a security update where a vulnerable dependency (or its vulnerable sub-dependency) is overridden to a vulnerable version
  2. a version update where an override locks a dependency to an older version

Those situations make sense to error with update not possible, but what if a an override is used to lock a non-vulnerable dependency to a non-vulnerable version?

Example: non-vulnerable sub-dependency B of some vulnerable dependency A is overridden to a non-vulnerable version.

--> A (vulnerable and receives security update, requires non-vulnerable B@v2.5.1)
  |--> B (overrides to non-vulnerable `v2.5.0`)

Opening a PR in this situation could help maintainers fix vulnerabilities quicker, even if it doesn't work.

But it might still be appropriate to raise update_not_possible because an overrides entry is still preventing a security update, the only difference being the blocked dependency is the parent rather than the sub-dependency itself.

Seeing a failure indicating that use of the overrides field prevented a security update of the vulnerable parent is explicit and doesn't require having to figure out why Dependabot opened a PR with a potentially empty diff to update the parent from 2.5.1 to 2.5.1.

Anyhow, here's a scenario where Dependabot might still want to open a PR with an override present:

Example: some dependency A receives a version update and has a sub-dependency B that is overridden to an older version. Would we still want to raise an update not possible error on the version update for A?

--> A (receives a version update, requires B@v2.5.1)
  |--> B (overrides to `v2.5.0`)

✅ Tasks

  • Check for and handle overrides field in package.json
  • Raise update_not_possible with descriptive error message
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Planned
Development

No branches or pull requests

3 participants
@abdulapopoola @Nishnha and others