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

Backport non-breaking security-related fixes to v10 #243

Closed
1 task done
legobeat opened this issue May 17, 2023 · 6 comments
Closed
1 task done

Backport non-breaking security-related fixes to v10 #243

legobeat opened this issue May 17, 2023 · 6 comments
Labels
Needs Triage needs an initial review

Comments

@legobeat
Copy link

legobeat commented May 17, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

make-fetch v10 is exposed to several issues through its dependencies. (#210, among others).

make-fetch v11 fixes these, but breaks compatibility with Node.js v12 through bumps of cacache (#184) minipass-fetch (#186), and ssri (#187).

Expected Behavior

While Node.js v12 is EoL as of 2022-04-30, several ecosystem packages still indicate support for it in their mainline versions.

Releasing a non-breaking backport 10.x release addressing security issues and subdependency deprecation (to reasonable extent) would be very helpful and provide protection for users who are yet to update (perhaps because they are using node-gyp...).

It would also make it more straightforward for lagging dependent packages to make the move off Node 12 themselves.

Steps To Reproduce

n/a

Environment

n/a

@legobeat legobeat added the Needs Triage needs an initial review label May 17, 2023
@wraithgar
Copy link
Member

$ npm i make-fetch-happen@10
npm WARN deprecated @npmcli/move-file@2.0.1: This functionality has been moved to @npmcli/fs

added 68 packages, and audited 69 packages in 2s

found 0 vulnerabilities
~/D/n/s/mfh $ npm audit
found 0 vulnerabilities

What vulnerabilities? Package-lock entries are not really vulnerabilities as they do not affect what the user will get when they install a module in their own project.

@legobeat
Copy link
Author

legobeat commented May 17, 2023

The issue does not mention anything about vulnerabilities (there are other bugs fixed), but since you brought it up:

In the case of http-cache-semantics (a runtime dependency) here, you are on a fresh environment (I assume) and get the latest semver-compatible dependencies with the relevant fixes.

A user utilizing lockfiles (yarn,npm,pnpm,shrinkwrap all apply here AFAIK) who are on an existing installation of make-fetch-happen@10 who runs npm i (or equivalent) in their repo will not get the necessary security updates due to their lockfile. Upgrade commands can disregard transitive dependencies, but do bump them as the ranges get bumped when their parents do.

The latest public release of make-fetch-happen@10 says "any version of http-cache-semantics at least 4.1.0 but below 4.1.1 is all good".

This is a potential risk for users of packages using make-fetch-happen as a dependency.

@wraithgar
Copy link
Member

Typically we don't bump semver ranges in a package.json to clear security vulnerabilities. The semver range allows for the fix, and folks can run their own audits. The v11 bump of http-cache-semantics was not normal.

We don't currently plan on backporting fixes to v10 here, given the imminent EOL of node12 and the fact that the semver ranges in v10 allow for an install with no vulnerabilities.

I don't know of any other "non-breaking security related fixes" this module got. The file permissions change was a breaking change and can't be backported. Looking at the changelog it's all dependency updates and a change to allow configurable cached headers.

@legobeat
Copy link
Author

legobeat commented May 17, 2023

Maybe you consider the way others use your software misguided.

This is, however, a release-blocker for the maintainers of node-gyp (as they're still on node 12 and can't release the update to 11 on current major but need to resolve the security advisories)

The use there is minimal and either way I'm sure that situation will be resoled somehow, whether through replacing the http library or something else.

There are many others who are in similar situations with supported versions still on 12 and 14 with their own reasons why some issue needs fixed and why a resolutions entry isn't a fix.

nodejs/node-gyp#2816 (comment)

@legobeat
Copy link
Author

legobeat commented May 17, 2023

Looks like we replied on the same time.

Thank you for taking the time to explain the stance of the project and considering the issue @wraithgar.

@wraithgar
Copy link
Member

I'm sorry but maybe I'm not seeing where the issue is, there are a lot of comments sprinkled now through half a dozen repos. If there are package declarations in v10 that have engines declarations that are out of band with the engines declaration in this module that's actionable. I don't see that as the case currently and that is one of the things we check during our CI via template-oss "check engines" step.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Triage needs an initial review
Projects
None yet
Development

No branches or pull requests

2 participants