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

Discussion: Using caret (^) with our own dependencies #279

Open
UlisesGascon opened this issue Oct 8, 2024 · 10 comments · May be fixed by #290
Open

Discussion: Using caret (^) with our own dependencies #279

UlisesGascon opened this issue Oct 8, 2024 · 10 comments · May be fixed by #290
Assignees
Labels
meeting tc agenda top priority Issues which the TC deem our current highest priorities for the project

Comments

@UlisesGascon
Copy link
Member

Historically, we have avoided the use of caret in the package.json files for our own projects. I am just wondering if we want to keep this policy or change it. Anyway, I would like to use this issue to document that decision and to have all the discussion around it in one place.

Ref: expressjs/express#6017 (comment)

@UlisesGascon UlisesGascon changed the title Using caret (^) with our own dependencies Discussion: Using caret (^) with our own dependencies Oct 8, 2024
@UlisesGascon UlisesGascon added meeting top priority Issues which the TC deem our current highest priorities for the project tc agenda labels Oct 8, 2024
@kurt-apple
Copy link

Via semver.org:

Given a version number MAJOR.MINOR.PATCH, increment the:

  1. MAJOR version when you make incompatible API changes
  2. MINOR version when you add functionality in a backward compatible manner
  3. PATCH version when you make backward compatible bug fixes
    Additional labels for pre-release and build metadata are available as extensions to the MAJOR.MINOR.PATCH format.

In my opinion, pinning to a specific patch version is as extreme as using a ">" or "latest" in package.json. In one extreme, breaking/incompatible changes by definition will be blindly accepted. In another extreme, patches that are backward compatible are rejected and you have to ask yourself, why?

There is an edge case if the dependencies of express have an unexpected regression or decide to throw semver out the window. I think it was noted in the linked PR that express has chosen dependencies that strictly follow semver. When this is the case, then accepting patches from deps is strictly abiding by semver specifications. See semver.org for a more detailed summary of the advantages of doing so.

@dpopp07
Copy link

dpopp07 commented Oct 8, 2024

Hi 👋 I'm new around here but thought I'd chime in

In another extreme, patches that are backward compatible are rejected and you have to ask yourself, why?
... accepting patches from deps is strictly abiding by semver specifications

I agree with this sentiment, though there are risks. In practice, I have run into the issue of pulling in unexpected regressions. They're especially sneaky since they can cause behavior changes for users of a library without any changes to that library, etc. but they've always been patched quickly (and then, once again, no library changes are needed to fix the issue). In worse, extreme cases, a dependency can be compromised and a patch version with malicious code can make it into a library without anyone being aware, which is harder to identify and resolve.

As someone new, one question I'd ask is: what has the process been (or perhaps, what should it be) for regularly keeping dependencies up to date while using a hard-pinning strategy? I've found it's very easy to let pinned dependencies go a good while without updating them, which can lead to missing out on important patch updates until someone reports a problem. To me, that's the "extreme" aspect of the strategy, like @kurt-apple is talking about. Realistically, regularly updating dependencies manually can be a challenge. Adhering to semver and pulling in patch updates alleviates that issue, to a degree.

With the risks in mind, I can understand wanting to pin dependencies but IMO there should be an established practice for keeping dependencies up to date (and there may be one, of which I am unaware!)

@kurt-apple
Copy link

Well said, @dpopp07
I have definitely experienced unexpected regressions or incompatibilities with my dependencies...
Perhaps the best of both worlds is to be notified of updated packages and prioritize response based on the context/nature of the updated code, reliability of test coverage, and bandwidth to manually review dependencies. Something like dependabot comes to mind, but I look forward to learning more about this myself.

@ljharb
Copy link
Contributor

ljharb commented Oct 9, 2024

Those risks should be mitigated by end users, using lockfiles - not by intervening packages. Every dependency should always use ^ (unless it doesn't follow semver, or needs to be pinned due to bugs a la =).

@klaude
Copy link

klaude commented Oct 9, 2024

Another random developer piping in! There are two takes that support using carets or tildes.

Philosophically, explicit version pinning is the downstream project's responsibility. Libraries and frameworks should use carets to express the features they need by their dependencies' major and minor versions. Express' users should pin dependencies to satisfy their security and compliance requirements, but express should keep it looser to allow for the most use cases.

Practically, it's less work for you and your users. There were four pull requests to update the dependency and release new versions. Each of those had accompanying noise from comments, reactions, and related issues. Your users had to wait for the 5.x and 4.x releases when they would have been able to take care of the update on their own as soon as a fixed version of cookie was available.

It's true that you can pull in an unexpected regression, but it's the app maintainer's responsibility to validate and test dependency changes. They should do this regardless of express' dependency pinning strategy.

@bjohansebas
Copy link
Member

Personally, I don’t like using ^ or ~ because of cases that have occurred in the ecosystem (faker.js), and I have always chosen not to use them. However, in this case, all the dependencies of Express are from the organization, so I don’t see a problem, and it would help reduce the number of PRs, as @klaude mentioned.

@ljharb
Copy link
Contributor

ljharb commented Oct 19, 2024

That has happened so rarely that it rounds to zero, and either way, application users protect against that using lockfiles - intermediate pinning doesn't solve anything.

@kjugi
Copy link
Member

kjugi commented Oct 21, 2024

Another vulnerability incident will happen in the futrure for sure. It just a matter of time. I don't want to make a pressure on maintainers to release the change just because I can see a problem in my npm audit. At the same time, I don't want to just ignore it for a day or two as I'm already aware about issue.

Using ^ for all dependencies in this project can decrease those cases at least a little bit.

I would assume that cases like faker.js are rare and extreme. Minor/patch library bump should solve real problems

@UlisesGascon
Copy link
Member Author

Added PR with a policy proposal #290

@jonchurch
Copy link
Member

jonchurch commented Oct 23, 2024

im in favor of adding carets to our deps.

Pick your evil, supply chain risks will always exist. Our goal should be to ensure that any fresh npm install of a given release is as secure as possible, while providing an easy path for users with vulnerabilities in their lockfile to get the latest safe versions when available.

Carets help achieve that by ensuring that if an issue existed in a dep of one of our releases, someone can get the latest patch for said dep when it is available via npm audit fix, or their initial npm i express without us having to release a new version.

Sometimes we'll need to cut a new version to fix the situation, but that's rare and would be an ecosystem wide problem with a lot of attention if something broke that bad. Then the motion just becomes "hey, update express"

Pinning is useful if we can guarantee that all deps are safe at time of publish. When can we ever do that? Otherwise, easy updates are safest IMO.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meeting tc agenda top priority Issues which the TC deem our current highest priorities for the project
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants