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

(fix) Exclude dev dependencies from npm's package-lock.json and Fix Java DB download endpoint #1893

Merged
merged 4 commits into from
Apr 17, 2024

Conversation

shino
Copy link
Collaborator

@shino shino commented Apr 15, 2024

What did you implement:

At 0.25.1, there is regression of npm's package-lock.json scan where dev dependencies are included in library list.
This PR fixes it.
Trivy of 0.50 has dev dependencies excluding feature of three types;

  1. npm (package-lock.json), the main point of this fix PR
  2. yarn, dev deps exclusion is triggered only when package.json is there alongside of yarn.lock, not the case for vuls (as a consequence, yarn lock file scans include dev dependencies)
  3. maven (pom.xml) only for integration test cases, it has very specific path pattern "**/[src|target]/it/*/pom.xml"

Realistically, only effect is npm.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Manually.

Results will be added as comments.

Checklist:

You don't have to satisfy all of the following.

  • Write tests
  • Write documentation
  • Check that there aren't other open pull requests for the same issue/feature
  • Format your source code by make fmt
  • Pass the test by make test
  • Provide verification config / commands
  • Enable "Allow edits from maintainers" for this PR
  • Update the messages below

Reference

@shino
Copy link
Collaborator Author

shino commented Apr 16, 2024

NPM v1

Compared results among three binaries

  • vuls.0.24.9 at v0.24.9 tag
  • vuls.master at current master
  • vuls.new from this PR

Summary line:

% for k in {0.24.9,master,new}; do ./vuls.${k} scan -config integration/int-config.toml npm-v1 2>&1 | grep -A 2 'Scan Summary'; done
Scan Summary
================
npm-v1  pseudo  0 installed, 0 updatable        240 libs
Scan Summary
================
npm-v1  pseudo  0 installed, 0 updatable        242 libs
Scan Summary
================
npm-v1  pseudo  0 installed, 0 updatable        240 libs

types/lodash from dev deps https://github.com/vulsio/integration/blob/b4cb452761ce17b6c2852e2be89221c15378e3c2/data/lockfile/npm-v1/package-lock.json#L292-L297

Appeared in "master" result
(These command are executed just after the above command, so "master" result are second oldest, note that tail has "-2" option)

% find results -type f -printf '%T@ %p\n' | grep '.json' | sort -n | tail -2 | head -1 | cut -d' ' -f2 | xargs -I{} bash -c "cat "{}"" | jq -C '.' | grep 'types/lodash'
          "Name": "@types/lodash",
          "PURL": "pkg:npm/%40types/lodash@4.14.157",
%

Not in "new" result
(In this case, tail has "-1" option, the latest one)

% find results -type f -printf '%T@ %p\n' | grep '.json' | sort -n | tail -1 | head -1 | cut -d' ' -f2 | xargs -I{} bash -c "cat "{}"" | jq -C '.' | grep 'types/lodash'```
%

@shino
Copy link
Collaborator Author

shino commented Apr 16, 2024

NPM v2

% for k in {0.24.9,master,new}; do ./vuls.${k} scan -config integration/int-config.toml npm-v2 2>&1 | grep -A 2 'Scan Summary'; done
Scan Summary
================
npm-v2  pseudo  0 installed, 0 updatable        267 libs
Scan Summary
================
npm-v2  pseudo  0 installed, 0 updatable        324 libs
Scan Summary
================
npm-v2  pseudo  0 installed, 0 updatable        267 libs

"babel/core" from dev deps https://github.com/vulsio/integration/blob/b4cb452761ce17b6c2852e2be89221c15378e3c2/data/lockfile/npm-v2/package-lock.json#L100-L104

"master"

          "FilePath": "",
% find results -type f -printf '%T@ %p\n' | grep '.json' | sort -n | tail -2 | head -1 | cut -d' ' -f2 | xargs -I{} bash -c "cat "{}"" | jq -C '.' | grep 'b
abel/core'
          "Name": "@babel/core",
          "PURL": "pkg:npm/%40babel/core@7.19.3",
% find results -type f -printf '%T@ %p\n' | grep '.json' | sort -n | tail -1 | head -1 | cut -d' ' -f2 | xargs -I{} bash -c "cat "{}"" | jq -C '.' | grep 'b
abel/core'
%  

@shino
Copy link
Collaborator Author

shino commented Apr 16, 2024

NPM v3

0.24.9 does not support v3 format

abel/core'
% for k in {0.24.9,master,new}; do ./vuls.${k} scan -config integration/int-config.toml npm-v3 2>&1 | grep -A 2 'Scan Summary'; done
Scan Summary
================
npm-v3  pseudo  0 installed, 0 updatable
Scan Summary
================
npm-v3  pseudo  0 installed, 0 updatable        1327 libs
Scan Summary
================
npm-v3  pseudo  0 installed, 0 updatable        250 libs

"@typescript-eslint/eslint-plugin" from dev deps https://github.com/vulsio/integration/blob/b4cb452761ce17b6c2852e2be89221c15378e3c2/data/lockfile/npm-v3/package-lock.json#L3283-L3287

"master"

% find results -type f -printf '%T@ %p\n' | grep '.json' | sort -n | tail -2 | head -1 | cut -d' ' -f2 | xargs -I{} bash -c "cat "{}"" | jq -C '.' | grep 'typescript-eslint/eslint-plugin'
          "Name": "@typescript-eslint/eslint-plugin",
          "PURL": "pkg:npm/%40typescript-eslint/eslint-plugin@5.62.0",
"""

"new"
"""
% find results -type f -printf '%T@ %p\n' | grep '.json' | sort -n | tail -1 | head -1 | cut -d' ' -f2 | xargs -I{} bash -c "cat "{}"" | jq -C '.' | grep 't
ypescript-eslint/eslint-plugin'
%

@shino shino marked this pull request as ready for review April 16, 2024 01:07
@shino shino requested a review from MaineK00n April 16, 2024 01:07
Copy link
Collaborator

@MaineK00n MaineK00n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that you need to change npm to npm-v1, npm-v2, or npm-v3.

:100644 100644 065fe8a 0000000 M	GNUmakefile

diff --git a/GNUmakefile b/GNUmakefile
index 065fe8a..83e021a 100644
--- a/GNUmakefile
+++ b/GNUmakefile
@@ -90,7 +90,7 @@ NOW=$(shell date '+%Y-%m-%dT%H-%M-%S%z')
 NOW_JSON_DIR := '${BASE_DIR}/$(NOW)'
 ONE_SEC_AFTER=$(shell date -d '+1 second' '+%Y-%m-%dT%H-%M-%S%z')
 ONE_SEC_AFTER_JSON_DIR := '${BASE_DIR}/$(ONE_SEC_AFTER)'
-LIBS := 'bundler' 'dart' 'elixir' 'pip' 'pipenv' 'poetry' 'composer' 'npm' 'yarn' 'pnpm' 'cargo' 'gomod' 'gosum' 'gobinary' 'jar' 'jar-wrong-name-log4j-core' 'war' 'pom' 'gradle' 'nuget-lock' 'nuget-config' 'dotnet-deps' 'dotnet-package-props' 'conan' 'swift-cocoapods' 'swift-swift' 'rust-binary'
+LIBS := 'bundler' 'dart' 'elixir' 'pip' 'pipenv' 'poetry' 'composer' 'npm-v1' 'npm-v2' 'npm-v3' 'yarn' 'pnpm' 'cargo' 'gomod' 'gosum' 'gobinary' 'jar' 'jar-wrong-name-log4j-core' 'war' 'pom' 'gradle' 'nuget-lock' 'nuget-config' 'dotnet-deps' 'dotnet-package-props' 'conan' 'swift-cocoapods' 'swift-swift' 'rust-binary'
 
 diff:
 	# git clone git@github.com:vulsio/vulsctl.git

@shino
Copy link
Collaborator Author

shino commented Apr 16, 2024

@MaineK00n I encountered error in Java DB download

[Apr 16 15:28:07] ERROR [localhost] Failed to fill with Library dependency:
    github.com/future-architect/vuls/detector.Detect
        /home/shino/g/vuls/detector/detector.go:50
  - Failed to update Trivy Java DB. err:
    github.com/future-architect/vuls/detector.DetectLibsCves
        /home/shino/g/vuls/detector/library.go:64
  - Failed to download Trivy Java DB. err:
    github.com/future-architect/vuls/detector/javadb.UpdateJavaDB
        /home/shino/g/vuls/detector/javadb/javadb.go:52
  - repository name error (ghcr.io/aquasecurity/trivy-java-db:1:1):
    github.com/aquasecurity/trivy/pkg/oci.(*Artifact).populate
        /home/shino/go/pkg/mod/github.com/aquasecurity/trivy@v0.50.1/pkg/oci/artifact.go:88
  - could not parse reference: ghcr.io/aquasecurity/trivy-java-db:1:1

I, for now, included the fix commit in this PR e0a076b.
If it should be independent PR, I will do so. How do you think?

@shino shino changed the title (fix) Exclude dev dependencies from npm's package-lock.json (fix) Exclude dev dependencies from npm's package-lock.json and Fix Java DB download endpoint Apr 17, 2024
@shino shino merged commit 8f40251 into master Apr 17, 2024
7 checks passed
@shino shino deleted the shino/fix-dev-deps branch April 17, 2024 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants