npm: inconsistent results when dependency has both a dev and non-dev entry in lockfile

[nikpivkin]

Discussed in https://github.com/aquasecurity/trivy/discussions/5134

^{Originally posted by **ngraef** September 7, 2023} ### Description Trivy is giving inconsistent results between successive vulnerability scans with the same database version. A sample `package-lock.json` that induces the behavior is included (collapsed) below. I believe the issue is triggered by the same package ID (`name@version`) having multiple entries in the lockfile, where one has `"dev": true` and another doesn't. In this sample, `minimist@0.0.8` meets that condition. Related: https://github.com/aquasecurity/go-dep-parser/issues/149 ### Desired Behavior I expect the scan results to be determinate and include all affected non-dev packages. With the sample lockfile, the expected output is: ``` package-lock.json (npm) ======================= Total: 3 (UNKNOWN: 0, LOW: 0, MEDIUM: 2, HIGH: 0, CRITICAL: 1) ┌───────────┬────────────────┬──────────┬────────┬───────────────────┬───────────────┬───────────────────────────────────────────────────────────┐ │ Library │ Vulnerability │ Severity │ Status │ Installed Version │ Fixed Version │ Title │ ├───────────┼────────────────┼──────────┼────────┼───────────────────┼───────────────┼───────────────────────────────────────────────────────────┤ │ minimist │ CVE-2021-44906 │ CRITICAL │ fixed │ 0.0.8 │ 1.2.6, 0.2.4 │ prototype pollution │ │ │ │ │ │ │ │ https://avd.aquasec.com/nvd/cve-2021-44906 │ │ ├────────────────┼──────────┤ │ ├───────────────┼───────────────────────────────────────────────────────────┤ │ │ CVE-2020-7598 │ MEDIUM │ │ │ 0.2.1, 1.2.3 │ nodejs-minimist: prototype pollution allows adding or │ │ │ │ │ │ │ │ modifying properties of Object.prototype using a... │ │ │ │ │ │ │ │ https://avd.aquasec.com/nvd/cve-2020-7598 │ ├───────────┼────────────────┤ │ ├───────────────────┼───────────────┼───────────────────────────────────────────────────────────┤ │ validator │ CVE-2021-3765 │ │ │ 7.2.0 │ 13.7.0 │ Inefficient Regular Expression Complexity in Validator.js │ │ │ │ │ │ │ │ https://avd.aquasec.com/nvd/cve-2021-3765 │ └───────────┴────────────────┴──────────┴────────┴───────────────────┴───────────────┴───────────────────────────────────────────────────────────┘ ``` ### Actual Behavior With the sample lockfile, the output excludes `minimist@0.0.8` in roughly 50% of scans: ``` package-lock.json (npm) ======================= Total: 1 (UNKNOWN: 0, LOW: 0, MEDIUM: 1, HIGH: 0, CRITICAL: 0) ┌───────────┬───────────────┬──────────┬────────┬───────────────────┬───────────────┬───────────────────────────────────────────────────────────┐ │ Library │ Vulnerability │ Severity │ Status │ Installed Version │ Fixed Version │ Title │ ├───────────┼───────────────┼──────────┼────────┼───────────────────┼───────────────┼───────────────────────────────────────────────────────────┤ │ validator │ CVE-2021-3765 │ MEDIUM │ fixed │ 7.2.0 │ 13.7.0 │ Inefficient Regular Expression Complexity in Validator.js │ │ │ │ │ │ │ │ https://avd.aquasec.com/nvd/cve-2021-3765 │ └───────────┴───────────────┴──────────┴────────┴───────────────────┴───────────────┴───────────────────────────────────────────────────────────┘ ``` ### Reproduction Steps ```bash 1. npm init -y && npm install minimist@0.0.8 validator@7.2.0 && npm install -D fsevents@1.2.9 2. Run `trivy fs --scanners vuln --format table --vuln-type library ./package-lock.json` 3. Repeat step 2 several times ``` ### Target Filesystem ### Scanner Vulnerability ### Output Format Table ### Mode Standalone ### Debug Output ```bash 2023-09-06T12:04:30.419-0600 DEBUG Severities: ["UNKNOWN" "LOW" "MEDIUM" "HIGH" "CRITICAL"] 2023-09-06T12:04:30.419-0600 DEBUG Ignore statuses {"statuses": null} 2023-09-06T12:04:30.432-0600 DEBUG cache dir: /Users/me/Library/Caches/trivy 2023-09-06T12:04:30.433-0600 DEBUG DB update was skipped because the local DB is the latest 2023-09-06T12:04:30.433-0600 DEBUG DB Schema: 2, UpdatedAt: 2023-09-06 12:13:03.850148455 +0000 UTC, NextUpdate: 2023-09-06 18:13:03.850147555 +0000 UTC, DownloadedAt: 2023-09-06 16:59:07.828888 +0000 UTC 2023-09-06T12:04:30.433-0600 INFO Vulnerability scanning is enabled 2023-09-06T12:04:30.433-0600 DEBUG Vulnerability type: [library] 2023-09-06T12:04:30.434-0600 DEBUG Walk the file tree rooted at 'package-lock.json' in parallel 2023-09-06T12:04:30.434-0600 INFO To collect the license information of packages in "package-lock.json", "npm install" needs to be performed beforehand 2023-09-06T12:04:30.454-0600 DEBUG OS is not detected. 2023-09-06T12:04:30.454-0600 INFO Number of language-specific files: 1 2023-09-06T12:04:30.454-0600 INFO Detecting npm vulnerabilities... 2023-09-06T12:04:30.454-0600 DEBUG Detecting library vulnerabilities, type: npm, path: package-lock.json ``` ### Operating System macOS Ventura 13.5.1 ### Version ```bash Version: 0.45.0 Vulnerability DB: Version: 2 UpdatedAt: 2023-09-06 12:13:03.850148455 +0000 UTC NextUpdate: 2023-09-06 18:13:03.850147555 +0000 UTC DownloadedAt: 2023-09-06 16:59:07.828888 +0000 UTC ``` ### Checklist - [X] Run `trivy image --reset` - [X] Read [the troubleshooting](https://aquasecurity.github.io/trivy/latest/docs/references/troubleshooting/)

[ngraef]

@nikpivkin I notice you changed the reproduction steps to include installing the dependencies in a fresh project. That won't work out of the box because newer patch versions of fsevents remove the node-pre-gyp dependency, which is what contributed the minimist@0.0.8 entry with "dev": true in my example. You can force the older version by adding this override in package.json before installing gulp:

"overrides": {
  "gulp": {
    "fsevents": "1.2.9"
  }
}

Or, probably simpler, change the instructions to npm install -D fsevents@1.2.9 instead of gulp.

[nikpivkin]

@ngraef Good catch, I fixed it

[ngraef]

Repost from https://github.com/aquasecurity/trivy/discussions/5134#discussioncomment-6930978

This patch in aquasecurity/go-dep-parser fixes my use case:

diff --git a/pkg/nodejs/npm/parse.go b/pkg/nodejs/npm/parse.go
--- a/pkg/nodejs/npm/parse.go
+++ b/pkg/nodejs/npm/parse.go
@@ -116,6 +117,7 @@
      // There are cases when similar libraries use same dependencies
      // we need to add location for each these dependencies
      if savedLib, ok := libs[pkgID]; ok {
+         savedLib.Dev = savedLib.Dev && pkg.Dev
          savedLib.Locations = append(savedLib.Locations, location)
          sort.Sort(savedLib.Locations)
          libs[pkgID] = savedLib

I'm not sure what other implications that has or whether the same issue exists in parsers for other package managers.

Is there anything else I can do to help move this forward?

[faizanH]

Is there any update on this issue?

[SemProvoost]

Any updates here? Issue seems to be consistently reproducible.

[DmitriyLewen]

Hi all! Created #6356 for this task.