Node-gyp: Security issue in dependency

[florianbrinkmann]

Hi,

there is a security issue (CVE-2018-3728) in hoek, that is needed by hawk, and hawk is a dependency of request (it is the same issue like node-sass has in the current version https://github.com/sass/node-sass/issues/2355).

Could you increase the version of the request dependency accordingly?

Thanks in advance,
Florian

[bnoordhuis]

Could you increase the version of the request dependency accordingly?

In a word: no. node-gyp depends on "request": "2" in order to share a copy with npm. The common case is that node-gyp is bundled with npm.

[florianbrinkmann]

Did not notice that, thanks for clarification!

[cyrilchapon]

Hey. Landed here after going upward the chain hoek hawk request.

What do you mean "share a copy with npm" ?

Why was this closed, since there is still a potential vulnarability ?

[cyrilchapon]

Found https://github.com/sass/node-sass/issues/2355 that should be referenced here.

This issue is poorly documenting.

[bnoordhuis]

You could have done a quick search, you know. This isn't the first issue about it.

What do you mean "share a copy with npm" ?

What exactly is unclear?

[cyrilchapon]

You could have done a quick search, you know. This isn't the first issue about it.

Yes I did. I think I can argue this is the one and only issue in this repository about this.

But if you found another one, I think any information about a "duplicate" would be way more useful than a silent close. If you're talking about about a piece of information in another repository, you could still add a "related" stuff here. Or at least leave it open to wait a PR for it, and add a label like "help needed"; you know, some open-source common and good practises

The whole point of npm audit is to track down security issues on a dependency chain. When running npm audit, node-gyp pops out, and it says "it has a security issue".

Well I think leaving an open issue for a security issue sounds fair, precisely since a security issue is actually remaining in the package. Event if it's waiting for an obscure deep dependency update. That close widely sounds like "not my problem". Well as I mentionned, npm audit report a security issue with this package, so this is our problem.

What do you mean "share a copy with npm" ?

What exactly is unclear?

I can't understand what you don't find "unclear" with it.

• A copy of what ?
• Why do you need to share this ?
• What do you mean, share ?

[bnoordhuis]

Yes I did. I think I can argue this is the one and only issue in this repository about this.

If you had done a search for 'hoek' or 'request', you'll find at least five issues where I explain why we don't upgrade them.

Well as I mentionned, npm audit report a security issue with this package, so this is our problem.

Then npm audit is wrong. node-gyp accepts any request 2.x, including the latest. Your problem is likely that some other dependency restricts the request version - such as npm, and that brings us to...

I can't understand what you don't find "unclear" with it.

Yeah, and I can't understand what you don't get about "share a copy..."

99% of node-gyp installs are what people get when they install node and npm. node-gyp is a dependency of npm and shares npm's copy of request to reduce the size of the tarball. Clear now?

[cyrilchapon]

If you had done a search for 'hoek' or 'request', you'll find at least five issues where I explain why we don't upgrade them.

Well I confess I haven't. I "searched" for "security", "vulnerability" and stuff that concern that package. This doesn't cancel the fact there are no links in that thread, and it was silently closed.

Yeah, and I can't understand what you don't get about "share a copy...". 99% of node-gyp installs are what people get when they install node and npm

Well, believe it or not, that's not my case, I installed it for node-sass. Which is indeed the package restricting it to another version. And believe it or not, people don't usually go through npm self dependencies before opening an issue, to check if the module is a dependency of npm itself.

Then npm audit is wrong. node-gyp accepts any request 2.x, including the latest.

Well I'd rather think, on the first look, that npm audit is reporting a security issue because the installed release of the dependency has a security issue. The fact that ultra-permissive semver "2.x" is allowing any, including breaking changes, request release seems like a real trouble. Is there a real need for "old npm" (which come with, unmaintained anymore, Node 4) using "new node-gyp" release ? I can't see the need for a "2.x", instead of just more accurately semversionning the dependency ?

[cyrilchapon]

Furthermore, "We can't upgrade / precisely define a version for dependency because another package rely on that deep, too permissive semversionned dependency", be it npm itself, doesn't sound like an decent argument in a module ecosystem. It's up to npm to restrict its dependencies accurately isn't it ?

[cyrilchapon]

And

If you had done a search for 'hoek' or 'request', you'll find at least five issues where I explain why we don't upgrade them.

..is actually incorrect. There are no opened issue with something like that in the title. The security issue is actually remaining possible as the request version is "2.X" allowing to install an old, unsecurred request release. The issue, be it irresolvable, whatever it's title, should remain open, shouldn't it ?

[bnoordhuis]

No. Only actionable issues remain open. It's a bug tracker, not a discussion forum.

It's up to npm to restrict its dependencies accurately isn't it ?

Wrong tree to be barking up. Take that up with npm.

The fact that ultra-permissive semver "2.x" is allowing any, including breaking changes

You misunderstand how semver works.

[cyrilchapon]

You misunderstand how semver works.

I don't think so. 2.x disallow non-backward compatibility in the API, but does allow some other kind of "breaking changes". But that's another topic.

My point is not about how request implemented semver.

My point is 2.x allow to install an old, unsecured release.

EDIT :

No. Only actionable issues remain open.

Well, I'm hard cmd + fing "issue" inside contributing guidelines but it seems that nothing says that.

It's a bug tracker, not a discussion forum.

There is actually a bug. The 2.x definition of the dependency request.js allows to install an unsecured release

EDIT2:

Wrong tree to be barking up. Take that up with npm.

and the real, actual question about npm dependency on node-gyp was

Is there a real need for "old npm" (which come with, unmaintained anymore, Node 4) using "new node-gyp" release ? I can't see the need for a "2.x", instead of just more accurately semversionning the dependency ?

[matthew-white]

With node-gyp requiring "request": "2", I'd been able to use node-gyp with the latest version of request, avoiding the npm audit alerts for request. However, it seems like node-gyp release 3.6.3 from six days ago pins the request version to <2.82.0. (I think the commit for that is https://github.com/nodejs/node-gyp/commit/790012233753f2389ab0ff52aba5dfd22d14c028.)

I think this change means that I can't use both the latest version of node-gyp and the latest version of request. I'm eager to pull in the security fixes that request introduced in 2.82 and 2.86.

Is it possible for node-gyp to require "request": "2" again rather than "request": ">=2.9.0 <2.82.0"? If not, do you think a future version of node-gyp will permit the latest version of request?

Thank you!

[cyrilchapon]

Maybe "request": "2.82.x - 2.x" would be better yet ?
Enforcing 2.82 + while allowing anything under 3.x

[Rohithzr]

npm is currently pinned to "request": "^2.86.0" I think it should be the same as this

[bnoordhuis]

That will happen in node-gyp@4. For now we pin request for compatibility with old Node.js versions.

Note that you'll only get audit warnings when you depend on node-gyp directly but that's not a recommended practice, you should use the copy that is distributed with node/npm.

[Rohithzr]

@bnoordhuis i do understand your point, older node versions cannot be abandoned.
With older do you mean node < 4 ? or node >= 4.x?
Also from what I can see people are coming here from the node-sass library which directly depends on "node-gyp": "^3.3.1".

from what I am seeing at the moment, request: 2.86.0 seems to be giving the most compatibility.

[bnoordhuis]

node <= v4.x. We'll drop support for them in node-gyp@4 but not in node-gyp@3.

I'll chime in on the node-sass issue.

[Rohithzr]

@bnoordhuis are you sure request 2.86 is incompatible with node 4 ? see https://github.com/nodejs/node-gyp/pull/1471

[bnoordhuis]

Yes, const and let are supported in that version. <= v4.x also means v0.10 and v0.12, which don't support those keywords.

[Rohithzr]

Ah! thanks for clarifying, well hopefully we can wait for node-gyp@4.