NPM v7 lockFileVersion = 2 is not supported in Trivy filesystem scan

[hideme4u]

Description

we have NPM7 generated package-lock.json with lockFileVersion = 2. Now when we scan Node.js project using Trivy filesystem scan, Trivy does not find out packages from package-lock.json. It is working with lockFileVersion = 1

What did you expect to happen?

It should find out packages in package-lock.json

What happened instead?

It did not find out packages from package-lock.json

Output of run with -debug:

trivy -d fs 1/ 2021-11-01T11:46:05.190+0530 DEBUG Severities: UNKNOWN,LOW,MEDIUM,HIGH,CRITICAL 2021-11-01T11:46:05.216+0530 DEBUG cache dir: /root/.cache/trivy 2021-11-01T11:46:05.216+0530 INFO Need to update DB 2021-11-01T11:46:05.216+0530 INFO Downloading DB... 2021-11-01T11:46:06.602+0530 DEBUG asset name: trivy.db.gz

2021-11-01T11:46:09.851+0530 DEBUG Vulnerability type: [os library] 2021-11-01T11:46:09.863+0530 DEBUG OS is not detected and vulnerabilities in OS packages are not detected. 2021-11-01T11:46:09.863+0530 DEBUG Detected OS: unknown 2021-11-01T11:46:09.863+0530 INFO Number of language-specific files: 0

ls -ltr 1/

-rwxr-xr-x 1 root root 575977 Oct 29 17:39 package-lock.json

cat 1/package-lock.json |head { "name": "jhipster-public-wesite", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "jhipster-public-wesite", "devDependencies": { "browser-sync": "2.27.5", "gulp": "^3.9.1",

(paste your output here)

Output of trivy -v:

Version: 0.20.2 Vulnerability DB: Type: Full Version: 1

(paste your output here)

## Additional details (base image name, container registry info...):

[hideme4u]

@afdesk / @masahiro331/ @knqyf263 Is this parsing for a new file format not yet supported?

[JafarAkhondali]

@hideme4u I think this is no correct. Looks like Npm lock file version 2 is backward compatible. The structure in v1 have a dependencies key, and the nested object may have dependencies key as well. This is also valid in v2. The only extra thing in v2 is packages key.

In our case the problem was that Trivy didn't scan devDependencies at all, here is the source https://github.com/aquasecurity/go-dep-parser/blob/eb58e8565220b1d25ff559b75e76a20a4d92cc2d/pkg/nodejs/npm/parse.go#L35.

I'm not sure but maybe it would be better that devDependencies get scanned by default, and excluded only if a flag is given in CLI. By the way npm audit doesn't skip scanning devDependencies.

[knqyf263]

@rahul2393 Could you take a look at this issue?

[rahul2393]

Thanks for assigning me this

[RolphH]

Any update on this issue?

[knqyf263]

We need to update this file so that it could parse v2 packge-lock.json. We welcome any contributions! https://github.com/aquasecurity/go-dep-parser/blob/8257534ffed3e34636437c45348644123ab02b74/pkg/nodejs/npm/parse.go

[JafarAkhondali]

@knqyf263 Did you confirm the issue? Do we want to scan Dev Dependencies packages? If someone provide a sample v2 package-lock.json that doesn't work with Trivy I can fix it.

[trevor-vaughan]

@JafarAkhondali This appears to highlight the issue.

Running npm install results in a run with no findings. However, running yarn install results in findings as expected.

package.json

{
  "name": "test",
  "version": "0.1.0",
  "private": true,
  "dependencies": {
    "axios": "0.21.2",
    "react": "^17.0.2"
  },
  "devDependencies": {
    "react-scripts": "4.0.3"
  }
}

package-lock.txt

[JafarAkhondali]

@trevor-vaughan I think the problem you mentioned here is that in yarn parser, dev packages are not recognized. I'm not sure about the policy here. Should Trivy scan dev dependencies or not?

[trevor-vaughan]

Ah, that certainly could be the issue. I was just surprised that the following were true:

• npm audit => warnings generated ✅
• yarn audit => warnings generated ✅
• trivy npm => nothing generated ❌
• trivy yarn => warnings generated ✅

It seems like these should be the same across the board.

[danielwagstaff]

I spent a few hours trying to figure out why Trivy wasn't picking up my package-lock.json, before I realised it ignored it because it only contained devDependencies.

May I please suggest the debug output prints a message making it clear that Trivy did find the package-lock.json file, but that it only contained devDependencies, so is ignoring (unless you are going to scan devDependecies)?

[bmaupin]

Trivy seems to find vulnerabilities just fine for me with package-lock.json v2.

My suspicion is that the author only had vulnerabilities in devDependencies, which npm audit will list but Trivy will not list as per https://aquasecurity.github.io/trivy/v0.30.1/docs/vulnerability/detection/language/

Given that this is documented, this seems more like a feature request than a bug. At any rate, the issue is invalid as it's currently worded ("NPM v7 lockFileVersion = 2 is not supported in Trivy filesystem scan").

[trevor-vaughan]

@bmaupin It does seem to be working as documented but I think that a few of these bugs might be avoided by making that table a bit more prominent (like front page) in the docs.

Also, it boggles my mind a bit that you can't toggle whether or not you want to scan dev dependencies.

[ldouchy-bl]

Just had the same issue. A scan with trivy repo gave me a list of vulnerabilities while trivy fs didn't.

It would be nice to have the same behaviour across the commands. In the spirit of --ignore-unfixed, a --ignore-dev-dependencies (or anything with a better name) would be great.

[ShubhamPalriwala]

Hey, I would like to solve this issue and get more involved in the ecosystem. Could somebody tell me what we're exactly looking for? What I could understand until now is that

• we need an argument called --ignore-dev-dependencies which should ignore the dev deps for the package,json file.
• we also need to make things uniform for both yarn.lock and package-lock.json i.e. include/exclude devDeps by default same both

Would like to know everyone involved here's thought for the same.

[trevor-vaughan]

@ShubhamPalriwala My personal expectation is that this table wouldn't be different for the different commands https://aquasecurity.github.io/trivy/v0.30.4/docs/vulnerability/detection/language/.

Instead, users would be able to specify --ignore-dev-dependencies and literally everything would be scanned consistently across the board.

I suppose, as a stretch goal, you would be able to disable/enable specific scanning engines easily. The experimental rego capability might do this but I don't know that I'd call that "easy".

[ShubhamPalriwala]

Yep, I also had similar ideas about the --ignore-dev-dependencies flag to make it consistent across different languages. That would make the ecosystem easy to use and pretty uniform.

I do not know much about the rego capability/ other scanning engines; hence, I think that can be implemented once the initial part is implemented and perfectly working.

What do you say?

[trevor-vaughan]

I say that progress is progress and community contributions are the best :-D

[ldouchy-bl]

@ShubhamPalriwala Thanks for looking into that.

As @trevor-vaughan said, anything we can do to have a stable and predictable scanning would be great.

[ShubhamPalriwala]

Update: I have an approach in mind about how to proceed, will be pinging y'all here for feedback on the same in the coming days.

[ShubhamPalriwala]

Hey, sorry for the delay on my end, had my mid-term examinations and a few other stuff :(( But I'm back with an approach looking for some valuable feedback from y'all on the same before starting to implement it:

So, the below is a rough idea of what all I've understood and what all I intend to do:

End Goal

• We scan dev dependencies by default for all languages/projects
• Users can pass a --ignore-dev-dependencies to ignore these in a scan.

Approach

1. Add the --ignore-dev-dependencies here: https://github.com/aquasecurity/trivy/blob/main/pkg/types/scanoptions.go#L8-L14
2. I found that our app libraries are of the following struct: https://github.com/aquasecurity/trivy/blob/main/pkg/fanal/types/artifact.go#L25-L49 We can add a field that can help us in differentiating between deps. Hence we add the following field to the same:"devDependency": boolean
3. Now that we have the dependency type for each app library package, we, here in the scanLangPkgs, https://github.com/aquasecurity/trivy/blob/main/pkg/scanner/local/scan.go#L272, run a loop and remove devDeps from our app.Libraries before passing it on to the detect() fn.
4. This should now scan only the packages and be a robust solution.

Need to figure out

1. Where the parsing of libraries happen to fill the depDependency field accordingly.

@knqyf263 It would be really helpful if you could also provide your thoughts on this as this turned out to be quite a big issue than what I thought this to be.

[knqyf263]

@ShubhamPalriwala What if adding --include-dev-dependencies? I think most users don't need vulnerabilities in dev dependencies.

Where the parsing of libraries happen to fill the depDependency field accordingly.

There is a library for parsing dependencies here. https://github.com/aquasecurity/go-dep-parser/

[knqyf263]

@ldouchy-bl Could you give us more detail? repo and fs should be the same. If it is not the case actually, it could be a bug.

Just had the same issue. A scan with trivy repo gave me a list of vulnerabilities while trivy fs didn't.

[knqyf263]

v1, v2 and v3 are supported now