Automatic check for dependencies' vulnerabilities in Node.js CI

[facutuesca]

Hi all, I'm opening this issue to propose a way of automatically checking Node's dependencies for new vulnerabilities, as part of CI.

The idea is to query both the NVD and the GH Advisory Database with Node's direct dependencies (the ones here) and have a GH action fail in case any new vulnerabilities are found.

I have a working POC in the form of a Python script that:

1. Has a list of the current dependencies, and the names to use when querying the NVD database (their Common Platform Enumeration)
2. Goes through the deps/ folder, parsing the current version of each of the dependencies
3. Queries the NVD using the CPE with the parsed version, to see if there are any active vulnerabilities
4. Queries the GH Advisory Database for the deps that are NPM packages
5. Filters the found vulnerabilities using an "ignore" list, that contains known false-positives
6. Exits successfully if no vulnerabilities are found. Otherwise, it fails and prints a message like:

WARNING: New vulnerabilities found
- npm (version 1.2.1) :
	- GHSA-v3jv-wrf4-5845: https://github.com/advisories/GHSA-v3jv-wrf4-5845
	- GHSA-93f3-23rq-pjfp: https://github.com/advisories/GHSA-93f3-23rq-pjfp
	- GHSA-m6cx-g6qm-p2cx: https://github.com/advisories/GHSA-m6cx-g6qm-p2cx
	- GHSA-4328-8hgf-7wjr: https://github.com/advisories/GHSA-4328-8hgf-7wjr
	- GHSA-x8qc-rrcw-4r46: https://github.com/advisories/GHSA-x8qc-rrcw-4r46
	- GHSA-m5h6-hr3q-22h5: https://github.com/advisories/GHSA-m5h6-hr3q-22h5
- acorn (version 6.0.0) :
	- GHSA-6chw-6frg-f759: https://github.com/advisories/GHSA-6chw-6frg-f759

For each dependency and vulnerability, check the following:
- Check the vulnerability's description to see if it applies to the dependency as
used by Node. If not, the vulnerability ID (either a CVE or a GHSA) can be added to the ignore list in
dependencies.py. IMPORTANT: Only do this if certain that the vulnerability found is a false positive.
- Otherwise, the vulnerability found must be remediated by updating the dependency in the Node repo to a
non-affected version.

If anyone has any comments or suggestions about the approach, please feel free to comment. Also, let me know if I should open a PR with the script in the public Node repo.

[richardlau]

Also, let me know if I should open a PR with the script in the public Node repo.

I think that would be a good thing -- it's easier to comment on an implementation.

[lirantal]

How would you know what are false positives to put in the ignore list? (referring to step 5 in your outlined strategy).

[lirantal]

Work on increasing security awareness, education and tooling for developers is welcome and blessed but I wonder if you're just building more tools that are duplicate of existing, and would potentially worsen the experience for developers?

Trying to say: how is this different than npm audit ?

[facutuesca]

@lirantal

How would you know what are false positives to put in the ignore list? (referring to step 5 in your outlined strategy).

It really is a judgement call that needs to be done case-by-case. For example: the script currently reports that Node's zlib version is vulnerable due to this. At first glance, it's valid because Node includes zlib 1.12.11 and the problem was fixed in 1.12.12. However, Node actually uses chromium's fork of zlib (which is not vulnerable). So it ends up being a false positive.

Work on increasing security awareness, education and tooling for developers is welcome and blessed but I wonder if you're just building more tools that are duplicate of existing, and would potentially worsen the experience for developers?

Trying to say: how is this different than npm audit ?

That's a good point. The main difference is that npm audit only works for NPM packages, where the idea here is to query the vulnerabilities for all dependencies (both NPM packages and C/C++ libraries).

We could even add the npm audit check to the script, so that it runs for each NPM package (in which case the results might even be better than the GH Advisory Database).

The main idea is to have a single command which the user (or CI) can run to check for vulnerabilities. As things stand right now, running npm audit manually will only give you the vulns for a single dependency, and only for NPM packages.

[lirantal]

Appreciate the elaborate answer and reference. With regards to the Node.js runtime version. Checking that in CI means what exactly though...? it seems out of context because as a team I may have 5 microservices I'm building, all of which are running different versions of the Node.js runtime. Moreover, in some scenarios you also don't just install Node.js from source, but rather use the distro's own package manager, and they sort out patches with their own security advisories, so you'd need to work with some tool like Snyk container scanning to find those.

Maybe I have everything I need with Snyk (dependency scanning, container scanning for OS libraries as well as runtime) so I'm not entirely sure I see benefits with this effort. I also still maintain the perspective that I think you might worsen the overall security by not correctly triaging vulnerabilities and allowing for false positives, or true negatives (NVD and npm audit miss quite a bunch of vulnerabilities). Not to mention that it sounds like you'd need to maintain this "true state" vulnerability database yourself if you don't trust upstream NVD and alike.

Disclaimer: I'm a developer advocate at Snyk.

[DanielRuf]

And shouldn't the security tab + GHA already alert maintainers (most reports are not public)? I think most findings would have to and will be manually checked ("are we affected or is it mitigated by design?").

[UlisesGascon]

Hi @facutuesca!

Thanks a lot for reaching us and sharing your concerns and proposing a solution. 🙂

As the discussion is getting more active, I want to add some personal conclusions.

In general, it is very easy for any developer to check how the project dependencies are in terms of vulnerabilities by running npm audit. Currently today, in my experience trying to extend its use of it across teams is hard because we can face a lot of false positives due to the specific project context. This is a great article about this frustration

So, in general, adding more checks to this command might impact the developers in the wrong direction. As well is important to notice that not all the Node.js users are using Npm in all projects (zero dependencies approach/scripting...).

I agree with @liran that is easy to use Snyk as an all-in-one tool to understand the dependencies and runtime potential vulnerabilities.

As well as repo owner/writer you can get the information in Github as @DanielRuf just said.

But for me, the key issue here is that for many developers is very complex and chaotic to evaluate/understand how these vulnerabilities can affect them. Does this open an opportunity to offer a guide on "how to understand CVEs and the impact in your project context" kind of thing? or maybe there are enough resources out there that we can promote?

[facutuesca]

@lirantal @UlisesGascon

Checking that in CI means what exactly though...? it seems out of context because as a team I may have 5 microservices I'm building, all of which are running different versions of the Node.js runtime.

In general, it is very easy for any developer to check how the project dependencies are in terms of vulnerabilities by running npm audit

As well is important to notice that not all the Node.js users are using Npm in all projects

Sorry, I realize now that my original description of the script was not totally clear. The script would be run on the CI of the Node.js repo, in order for the Node team to quickly see if a new vulnerability was reported for Node's dependencies. It is not intended (or really useful) for Node.js users, since they can simply use npm-audit for their package's dependencies.

@DanielRuf

And shouldn't the security tab + GHA already alert maintainers (most reports are not public)? I think most findings would have to and will be manually checked ("are we affected or is it mitigated by design?").

For Node.js users, definitely (see my clarification above). As for the Node.js repo itself, I don't think GHA will alert for any new vulnerability of Node's dependencies. At least for the C/C++ libraries like brotli which have their source code copy-pasted into Node's repo, there is no way for GH to know that brotli is a dependency of Node

[facutuesca]

In short, the use case of the script would be:

• A new vulnerability is found in a dependency of Node.js, such as brotli
• During a scheduled CI run of the script (on the Node.js repo), it finds the new vulnerability and a failure is reported with a link to the vuln's description.
• The Node.js maintainers are notified, and can quickly react to the issue (either by updating the dependency, or deciding that Node is not affected by it)

So the main point is to have an automated process to detect and report those vulnerabilities, to minimise the time until the Node.js maintainers become aware of the issue.

[RafaelGSS]

Overall +1. I think it would be a great addition to Node.js itself.

[DanielRuf]

Makes more sense now. At least they will still have to evaluate (if Node is affected at all).

[facutuesca]

PR with the script is open for review here

[lirantal]

@facutuesca thanks for explaining a second time, it's much clearer for me and I agree it is useful. I wonder if it's worth bringing up with the Node.js core team who manages security releases to validate first if this workflow is something useful for them or how to make it such. They're very busy and often over-burdened.

[mhdawson]

@lirantal the reality is that we get questions from the community about vulnerabilities in our dependencies already. Today we have to adhoc follow those up.

I'm hoping that once we have the generation automated, that were possible we can pull in experts from some of the depenencies (for example npm) to review and comment proactively. This would let us have clear and visible answers versus having to backchannel to ask questions/figure it out when the project is asked about a publicly reported vulnerability.

[lirantal]

Yep, it's all good. I was confused at first thinking this was something to do with the npm ecosystem, but it's been made clear that this is with regards to the Node.js core project and the dependencies that it itself bundles or relies upon.

[mhdawson]

Some updates:

1. @facutuesca script landed in -> nodejs/node@07411da
2. Requested new repo to run GitHub action in - New repo called nodejs-dependency-vuln-assessments admin#704
3. Working on action in - https://github.com/mhdawson/nodejs-dependency-vuln-assessments/
   • action is working, creates issue in repo when the scan reports failures. Ready to go once we have the ok for the repo under
     Nodejs.org
   • Need backports for the script to 18, 16, and 14 before we can scan branches other than main.

[RafaelGSS]

Amazing!

[github-actions]

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.

[mhdawson]

It's now been enabled for 14 and 16 as well so I think we can close this issue.