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

Scan groups are inconsistent when there are duplicate packages in npm #924

Open
michaelkedar opened this issue Apr 17, 2024 · 4 comments · May be fixed by #939
Open

Scan groups are inconsistent when there are duplicate packages in npm #924

michaelkedar opened this issue Apr 17, 2024 · 4 comments · May be fixed by #939
Assignees
Labels
bug Something isn't working

Comments

@michaelkedar
Copy link
Member

If the same package version is installed multiple times under different groups in a package-lock.json file (i.e. in both dev and prod), osv-scanner scan behaves inconsistently in showing which groups the package belongs to.

e.g. with this package-lock.json (from package.json), the output is randomly:

╭─────────────────────────────────────┬──────┬───────────┬─────────┬─────────┬───────────────────╮
│ OSV URL                             │ CVSS │ ECOSYSTEM │ PACKAGE │ VERSION │ SOURCE            │
├─────────────────────────────────────┼──────┼───────────┼─────────┼─────────┼───────────────────┤
│ https://osv.dev/GHSA-v88g-cgmw-v5xw │ 5.6  │ npm       │ ajv     │ 5.5.2   │ package-lock.json │
╰─────────────────────────────────────┴──────┴───────────┴─────────┴─────────┴───────────────────╯

or

╭─────────────────────────────────────┬──────┬───────────┬───────────┬─────────┬───────────────────╮
│ OSV URL                             │ CVSS │ ECOSYSTEM │ PACKAGE   │ VERSION │ SOURCE            │
├─────────────────────────────────────┼──────┼───────────┼───────────┼─────────┼───────────────────┤
│ https://osv.dev/GHSA-v88g-cgmw-v5xw │ 5.6  │ npm       │ ajv (dev) │ 5.5.2   │ package-lock.json │
╰─────────────────────────────────────┴──────┴───────────┴───────────┴─────────┴───────────────────╯

Because ajv@5.5.2 is installed twice, once as a prod dependency, and once as a dev dependency:

    "node_modules/eslint/node_modules/ajv": {
      "version": "5.5.2",
      "resolved": "https://registry.npmjs.org/ajv/-/ajv-5.5.2.tgz",
      "integrity": "sha512-Ajr4IcMXq/2QmMkEmSvxqfLN5zGmJ92gHXAeOXq1OekoH2rfDNsgdDoL2f7QaRCy7G/E6TpxBVdRuNraMztGHw==",
      "dev": true,
      "dependencies": {
        "co": "^4.6.0",
        "fast-deep-equal": "^1.0.0",
        "fast-json-stable-stringify": "^2.0.0",
        "json-schema-traverse": "^0.3.0"
      }
    },
    "node_modules/table/node_modules/ajv": {
      "version": "5.5.2",
      "resolved": "https://registry.npmjs.org/ajv/-/ajv-5.5.2.tgz",
      "integrity": "sha512-Ajr4IcMXq/2QmMkEmSvxqfLN5zGmJ92gHXAeOXq1OekoH2rfDNsgdDoL2f7QaRCy7G/E6TpxBVdRuNraMztGHw==",
      "dependencies": {
        "co": "^4.6.0",
        "fast-deep-equal": "^1.0.0",
        "fast-json-stable-stringify": "^2.0.0",
        "json-schema-traverse": "^0.3.0"
      }
    },

Presumably, this would also affect groups in other lockfiles that can support the same package being installed multiple times.

@oliverchang
Copy link
Collaborator

@cuixq can you please take a look?

@G-Rath
Copy link
Collaborator

G-Rath commented Apr 17, 2024

@oliverchang I was actually thinking this one could be something I pick up if you want @cuixq to focus on other things, but up to you 🙂

@cuixq
Copy link
Contributor

cuixq commented Apr 17, 2024

@G-Rath feel free to take it! I am working on another issue at the moment. :)

@G-Rath
Copy link
Collaborator

G-Rath commented Apr 18, 2024

Ok this has been an interesting one - I think I've boiled it down to the question of "should packages be merged based on their group?"

i.e. if I have a@1 that is "optional" and a@1 that is "dev", should that be a@1, groups: optional and a@1, groups: dev or should be it a@1, groups: optional,dev?

I've got PRs for both and they're probably both alright but for the first one its a bit weird then that we have "groups" when they'll only ever have one item, and in the second it means an empty group denotes "production" and that we want to only include groups that all packages of the same name+version are in.

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.

4 participants