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

Yarn dependabot PR for advisory with multiple versions only updated one version #2271

Open
h-lame opened this issue Jul 1, 2020 · 5 comments
Labels
F: security-updates 🔐 Issues specific to security updates L: javascript:yarn npm packages via yarn T: bug 🐞 Something isn't working versioning

Comments

@h-lame
Copy link

h-lame commented Jul 1, 2020

Package manage/ecosystem

yarn

Manifest contents prior to update

acorn@^6.0.1:
  version "6.4.0"
  resolved "https://registry.yarnpkg.com/acorn/-/acorn-6.4.0.tgz#b659d2ffbafa24baf5db1cdbb2c94a983ecd2784"
  integrity sha512-gac8OEcQ2Li1dxIEWGZzsp2BitJxwkwcOm0zHAJLcPJaVvm58FRnk6RkuLRpU1EujipU2ZFODv2P9DLMfnV8mw==
 
acorn@^7.1.0:
  version "7.1.0"
  resolved "https://registry.yarnpkg.com/acorn/-/acorn-7.1.0.tgz#949d36f2c292535da602283586c2477c57eb2d6c"
  integrity sha512-kL5CuoXA/dgxlBbVrflsflzQ3PAas7RYZB52NOm/6839iVYJgKMJ3cQJD+t2i5+qFa8h3MDpEOJiS64E8JLnSQ==

Updated dependency

acorn, 6.x -> 6.4.1 & 7.x -> 7.3.1

To resolve: GHSA-6chw-6frg-f759

What you expected to see, versus what you actually saw

I expected to get a PR that bumped both versions of acorn defined in our yarn.lock, but instead we got a PR that bumped the 6.x version, but not the 7.x version. After merging this PR the advisory was still flagged on our repo, but we couldn't get dependabot to re-raise a PR to fix the 7.x version, so we did it manually.

Images of the diff or a link to the PR, issue or logs

What we got was:

 acorn@^6.0.1:
-  version "6.4.0"
-  resolved "https://registry.yarnpkg.com/acorn/-/acorn-6.4.0.tgz#b659d2ffbafa24baf5db1cdbb2c94a983ecd2784"
-  integrity sha512-gac8OEcQ2Li1dxIEWGZzsp2BitJxwkwcOm0zHAJLcPJaVvm58FRnk6RkuLRpU1EujipU2ZFODv2P9DLMfnV8mw==
+  version "6.4.1"
+  resolved "https://registry.yarnpkg.com/acorn/-/acorn-6.4.1.tgz#531e58ba3f51b9dacb9a6646ca4debf5b14ca474"
+  integrity sha512-ZVA9k326Nwrj3Cj9jlh3wGFutC2ZornPNARZwsNYqQYgN0EsV2d53w5RN/co65Ohn4sUAUtb1rSUAOD6XN9idA==
 
 acorn@^7.1.0:
   version "7.1.0"
   resolved "https://registry.yarnpkg.com/acorn/-/acorn-7.1.0.tgz#949d36f2c292535da602283586c2477c57eb2d6c"
   integrity sha512- acorn@^7.1.0:

What I expected / wanted would be:

 acorn@^6.0.1:
-  version "6.4.0"
-  resolved "https://registry.yarnpkg.com/acorn/-/acorn-6.4.0.tgz#b659d2ffbafa24baf5db1cdbb2c94a983ecd2784"
-  integrity sha512-gac8OEcQ2Li1dxIEWGZzsp2BitJxwkwcOm0zHAJLcPJaVvm58FRnk6RkuLRpU1EujipU2ZFODv2P9DLMfnV8mw==
+  version "6.4.1"
+  resolved "https://registry.yarnpkg.com/acorn/-/acorn-6.4.1.tgz#531e58ba3f51b9dacb9a6646ca4debf5b14ca474"
+  integrity sha512-ZVA9k326Nwrj3Cj9jlh3wGFutC2ZornPNARZwsNYqQYgN0EsV2d53w5RN/co65Ohn4sUAUtb1rSUAOD6XN9idA==
 
 acorn@^7.1.0:
-  version "7.1.0"
-  resolved "https://registry.yarnpkg.com/acorn/-/acorn-7.1.0.tgz#949d36f2c292535da602283586c2477c57eb2d6c"
-  integrity sha512-kL5CuoXA/dgxlBbVrflsflzQ3PAas7RYZB52NOm/6839iVYJgKMJ3cQJD+t2i5+qFa8h3MDpEOJiS64E8JLnSQ==
+  version "7.3.1"
+  resolved "https://registry.yarnpkg.com/acorn/-/acorn-7.3.1.tgz#85010754db53c3fbaf3b9ea3e083aa5c5d147ffd"
+  integrity sha512-tLc0wSnatxAQHVHUapaHdz72pi9KUyHjq5KyHjGg9Y8Ifdc79pTh2XvI6I1/chZbnM7QtNKzh66ooDogPZSleA==

Note, we're wholly using dependabot via the GitHub security advisories, not via dependabot.com. Unfortunately, this example is from a private repo. Should it be relevant, acorn in this repo is not a direct dependency from package.json it's a transitive one, although, only one level deep.

@feelepxyz
Copy link
Contributor

@h-lame thanks for the feedback! We're aware if the issue and have some plans for improving things. The underlying issue is that there hasn't been a good way in npm to make targetted sub-dependency updates. Will be a while before we get to this though as we're a tiny team and currently focusing on integrating dependabot.com functionality natively in GitHub.

@h-lame
Copy link
Author

h-lame commented Jul 1, 2020

@feelepxyz - no worries - I couldn't see the issue in a quick scour through the already raised issues, so I thought it'd post it in case you weren't already aware. Obvs, no rush on a fix as there are workarounds for us to bump the dependency manually, and arguably it's still working in the most important way which is that the advisory remains to tell us we had a risk even once the dependabot PR that fixed the 6.x version was merged.

@lseppala lseppala added F: security-updates 🔐 Issues specific to security updates L: javascript:yarn npm packages via yarn T: bug 🐞 Something isn't working labels Dec 7, 2021
@jeffwidman
Copy link
Member

jeffwidman commented Nov 24, 2022

👋 We shipped a lot of improvements to yarn / npm over the past year... Given this issue is over two years old, is it still reproducible or should we close it?

See also: https://github.blog/changelog/2022-09-07-dependabot-unlocks-transitive-dependencies-for-npm-projects/

@kreintjes
Copy link

@jeffwidman I'm still running into this issue today. I have a repo which uses two versions of json5, both 1.0.1 and 2.2.1. Dependabot creates two security alerts for this (GHSA-9c47-m6qq-7p4h) in GitHub. However it only creates one PR, which only updates the 1.0.1 version (both security alerts link to the same PR). After merging this PR, one of the security alerts disappears, but the other one (for the 2.2.1 version) stays open. When Dependabot tries to create a PR for that as well, it fails:

image

What I would expect is one PR which updates both versions at once and after merging it closes both security alerts.

Might be related to:

@h-lame
Copy link
Author

h-lame commented Jan 10, 2023

Yup, came here to report the same issue with json5 - we are using 1.0.1 and 2.2.1 and got an alert to upgrade to 1.0.2 and 2.2.2 but only a PR for 1.0.2. The alert for 2.2.2 can't create a PR as it fails with errors about 1.0.2 being the only possible version because of transitive deps. I do wonder if this is a yarn / npm issue more than dependabot - but seems like it should be possible to mitigate as I assume we'll be able to manually resolve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F: security-updates 🔐 Issues specific to security updates L: javascript:yarn npm packages via yarn T: bug 🐞 Something isn't working versioning
Projects
None yet
Development

No branches or pull requests

6 participants