5 low vulnerabilities caused by optional dependency gc-stats

[TrevorNodeJS]

Describe the bug
get below vulnerabilities reports, caused by gc-stats, and I find that gc-stats is optional dependency.

https://github.com/tdeekens/promster/blob/main/packages/metrics/package.json#L53-L55

┌──────────────────────────────────────────────────────────────────────────────┐
│                                Manual Review                                 │
│            Some vulnerabilities require your attention to resolve            │
│                                                                              │
│         Visit https://go.npm.me/audit-guide for additional guidance          │
└──────────────────────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Low           │ Regular Expression Denial of Service                         │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ braces                                                       │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=2.3.1                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ cpx [dev]                                                    │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ cpx > chokidar > anymatch > micromatch > braces              │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://npmjs.com/advisories/786                             │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Low           │ Prototype Pollution                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ minimist                                                     │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=0.2.1 <1.0.0 || >=1.2.3                                    │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @promster/express                                            │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ @promster/express > @promster/metrics > gc-stats >           │
│               │ node-pre-gyp > mkdirp > minimist                             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://npmjs.com/advisories/1179                            │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Low           │ Prototype Pollution                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ minimist                                                     │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=0.2.1 <1.0.0 || >=1.2.3                                    │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @promster/express                                            │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ @promster/express > @promster/metrics > gc-stats >           │
│               │ node-pre-gyp > tar > mkdirp > minimist                       │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://npmjs.com/advisories/1179                            │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Low           │ Prototype Pollution                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ minimist                                                     │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=0.2.1 <1.0.0 || >=1.2.3                                    │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @promster/express                                            │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ @promster/express > @promster/metrics > gc-stats >           │
│               │ node-pre-gyp > rc > minimist                                 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://npmjs.com/advisories/1179                            │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Low           │ Prototype Pollution                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ ini                                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >1.3.6                                                       │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @promster/express                                            │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ @promster/express > @promster/metrics > gc-stats >           │
│               │ node-pre-gyp > rc > ini                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://npmjs.com/advisories/1589                            │
└───────────────┴──────────────────────────────────────────────────────────────┘

To Reproduce
Steps to reproduce the behavior:
npm audit --registry=https://registry.npmjs.org

Expected behavior
Is it possible to remove gc-stats?

Screenshots

Additional context
Add any other context about the problem here.

[tdeekens]

Thanks. gc-stats can not be removed. It collects the Garbage Collection stats. You can however decide not to add it in your project.

Maybe we fix the issue upstream at gc-stats?

[tdeekens]

Did you look into fixing things upstream?

[kungfutse]

Wouldn't hold my breath on this getting fixed in upstream as dainis hasn't been active for over six months now :(.

There is a fork of gcstats with up-to-date dependencies, not sure if it also fixes the issue that causes gc-stats build to fail on node14

https://github.com/adnanrahic/node-gcstats

[tdeekens]

Thanks for sharing. Would be a breaking change to this library. As an alternative we could also maintain a fork as a package here. Then we have more control. 🤔

[Ghazgkull]

@tdeekens When I look at the difference between the abandoned gc-stats repository and the fork, the only differences I see look like dependency updates and build improvements. Are you sure that moving to the maintained fork would be a breaking change?

For convenience, here's the comparison between the two: dainis/node-gcstats@master...adnanrahic:master

[tdeekens]

Thanks. Let me explain why I think it would be:

1. Right now gc-stats is an optional dependency
2. With the change node-gcstats would be an optional dependency

With that change anybody using promster would have to update/add a node-gcstats dependency while removing the regular gc-stats dependency.

As such it would be a breaking change. I don't strictly mind if we think it's a good decision though.

[mdsummers]

This is raised in my audit CI checks for a project I'm working on. While I can see based on this thread that it's unlikely to cause a genuine security issue, it does normalise a failing audit check and make it harder for me to spot genuine issues.

[Ghazgkull]

@tdeekens Is there nothing that can be done about this? As @mdsummers points out, this affects all NodeJS applications which want to expose Prometheus metrics using this package. We're all stuck with broken audits because of an abandoned project (gc-stats) which doesn't actually install anymore anyway.

I understand the concern about a breaking change. But can we just remove gc-stats, publish a major version bump of this module, and call it a day?

[tdeekens]

Sure. We can move away from gc-stats to nodegc-stats in the hope of it being better maintained. It seems to also have received the last commit 12 months ago. Can you open a PR to kick things off? Thanks.

[tdeekens]

@Ghazgkull any progress on this on your end? 🤔

[jdevalk2]

I am evaluating Promster, and running into this issue. It is hard to justify using Promster if the project gc-stats spits out a bunch of vulnerabilities that are unfixed for months. In the gc-stats repository people are resorting to forking it and updating its dependencies.

I like promster but it's not easy to adopt in the current state.

[tdeekens]

I can totally agree with that! Have you considered opening a PR to promster to propose a change to move away from gc-stats?

[jdevalk2]

I see two possible paths:

1. Promster project forks gc-stats into their own repo and references the forked version.
   There is a PR in gc-stats that updates to remove the vulerabilities so the work seems to be done there. Just ugly to fork it.

2. Promster project switches gc-stats implemenation as you said.
   I am not familiar with how that integrates but it is envisionable that there might be differences in integration, datastructures etc. Most likely not a simple switch. But it could be.

And maybe a 3rd, since I am not a NodeJS expert and I am using this for a commercial project whereby I have to justify my time spent. I am thinking option (2) might be more time than I can justify so it would have to be a labor of love and contend with all my other priorities.

Maybe one other option:

3. Promster could potentially choose to build a version with and without gc-stats. The clients interested in NodeJs garbage collection stats could choose to submit a PR.

Definitely not ideal either, but I also do not think its fair to discontinue using Promster just because gc-stats library (whose stats we do not use in our project at least) eliminates the usability of the whole project - - and so far it looks like the best of the NodeJs exporters.

[jdevalk2]

@tdeekens I am re-reading this thread and you say

Right now gc-stats is an optional dependency

This seems to be my mentioned point (3).. How do I get a version of Promster that does not include gc-stats in the dependencies so that "npm audit" will pass? From your comment it already seems possible.

[YoannMa]

@jdevalk2 npm install --no-optional but that would not change the outcome of npm audit

[tdeekens]

Thanks for sharing your thoughts. We already follow option 3 as you noted yourself. I'd avoid forking gc-stats into this project (option 1.). This would be my last resort. Before I'd try to use the already forked and hopefully maintained version. I am not fully sure but wouldn't expect to many integration pains.

Regarding your comment:

so it would have to be a labor of love and contend with all my other priorities.

I'd also like to point out that this project is "labor of love and contend" by myself as Open Source is in general. I don't get time from any employer nor paid by anybody to work on the project.

[tdeekens]

Please refer to #703 as a suggested change.

[Ghazgkull]

@tdeekens Thank you for addressing this. You mention a comment above that you're already providing this option:

Promster could potentially choose to build a version with and without gc-stats. The clients interested in NodeJs garbage collection stats could choose to submit a PR.

Can you please provide info on where to find the version of promster which doesn't include gc-stats? Unfortunately, even the newer fork is also lagging dependencies already and pulls in a number of High severity audit issues due to using the deprecated version of node-pre-gyp.

[Ghazgkull]

This issue talks about "5 low vulnerabilities", but the current situation is actually "10 high vulnerabilities". I've opened a new issue to take the conversation forward from here: #713