npm / cli

the package manager for JavaScript
https://docs.npmjs.com/cli/
Other
8.44k stars 3.14k forks source link

bug: overrides property only honored when running install the first time #5850

Open lukekarrys opened 1 year ago

lukekarrys commented 1 year ago

Opening a new issue since #4232 is getting crowded with other possibly unreleated bug reports. But this one I have confirmed.

From: https://github.com/npm/cli/issues/4232#issuecomment-1281801614


Note that the INITIAL install will abide by the override rules set, and the subsequent installs (e.g., run npm install twice) will ignore overrides.

I can confirm this is the behavior in the latest npm@8.19.2. This can be reproduced easily with the following package.json:

{
  "name": "test",
  "version": "1.0.0",
  "engines": {
    "npm": ">=8.3.0"
  },
  "dependencies": {
    "json-server": "^0.17.0"
  },
  "overrides": {
    "json-server": {
      "package-json": "7.0.0"
    }
  }
}
  1. npm install in the folder containing only the above package.json --> 0 vulnerabilities
  2. Subsequent npm install right after the previous (so node_modules and package-lock.json exists) --> 5 vulnerabilities
  3. npm update --> 0 vulnerabilities
  4. rm -rf node_modules/ && npm install --> 5 vulnerabilities
  5. rm package-lock.json && npm install --> 5 vulnerabilities
  6. rm -rf node_modules/ && rm package-lock.json && npm install --> 0 vulnerabilities

From the above it can be concluded that the overrides property is only honored when running npm install first time (i.e. without package-lock.json and node_modules present) and when running npm update.

AlonNavon commented 1 year ago

I can confirm that I have the same issue on the latest npm@9.6.6.

This is really disruptive for our CI. Anyone working with a committed package-lock.json isn't running the overrides for the first time. So in a sense, it's not functional for them. Moreover, since it reverts to the overridden version quietly, a lot of people probably have no idea that the vulnerable library they've overridden has come back to haunt them.

I think this bug should be prioritized.

jakubmazanec commented 1 year ago

I too think this bug should be a priority. overrides is critical functionality for our team and this bug makes lockfile practically useless.

bnbdr commented 1 year ago

This reproduced for me on 9.6.6, but works as expected when specifying the override 'globally':

{
  "name": "test",
  "version": "1.0.0",
  "dependencies": {
    "json-server": "0.17.0"
  },
  "overrides": {
    "package-json": "7.0.0"
  }
}
ecl1ps commented 1 year ago

Hi, this problem is still present in npm 9.6.7. It causes huge issues for a long time and can cause security vulnerabilities for developers unaware of this bug.

I have created a more thorough repro case.

Initial state Monorepo package package1 depends on cssstyle@2.2.0 and it depends on cssom@~0.3.6. After running the initial npm install the lockfile contains:

        "node_modules/cssom": {
            "version": "0.3.8",
        },
        "node_modules/cssstyle": {
            "version": "2.2.0",
            "dependencies": {
                "cssom": "~0.3.6"
            },
        }

After adding an override and running the npm install there are no changes to the lockfile nor the installed packages. The override is completely ignored.

After changing the version of the cssstyle to 2.3.0 and running the npm install the are the following changes:

        "node_modules/cssom": {
            "version": "0.3.8",
        },
        "packages/package1/node_modules/cssstyle": {
            "version": "2.3.0",
            "dependencies": {
                "cssom": "~0.3.6"
            },
        }

Now the override is still not honored and on top of that the cssstyle is moved under packages/package1/node_modules/ for no apparent reason. It causes a duplication of this package in the monorepo. And that is another problem with some packages that require you to have only one version of it in the repo (like react and many others). Without having the override set up this move wouldn't occur.

The only workaround we found to work is to uninstall and then install that package again when touching the override or the package's version. Which is really really cumbersome in large monorepos.

Running npm uninstall cssstyle --workspace packages/package1 && npm install cssstyle@2.3.0 --workspace packages/package1 finally produces the correct outcome.

        "node_modules/cssom": {
            "version": "0.3.6",
        },
        "node_modules/cssstyle": {
            "version": "2.3.0",
            "dependencies": {
                "cssom": "~0.3.6"
            },
        }

I hope this helps to move this issue further. Feel free to ask for additional info.

AlonNavon commented 1 year ago

Also, I'd like to pile on that as this issue can cause packages to silently revert to vulnerable versions, IMHO this is not a regular bug, but a security issue, that should also be fixed on the latest version 8. It's fair to assume that the major reason for tinkering with an internal dependency is a security issue, and these silent reverts are a security incident waiting to happen.

aakashsarnobat commented 1 year ago

The override property runs fine in my local machine but somehow doesn't read in the CI pipeline and gives the security vulnerabilities leading to a pipeline failure. Any suggestions on this?

ecl1ps commented 1 year ago

The override property runs fine in my local machine but somehow doesn't read in the CI pipeline and gives the security vulnerabilities leading to a pipeline failure. Any suggestions on this?

Did you try my repro case? I think it is still the same problem.

aakashsarnobat commented 1 year ago

cssstyle

I was looking to override the version of semver package which is on the outer layer . Do you recommend adding the npm uninstall semver and then npm install semver to the docker file which is used in the CI pipeline?

hybridherbst commented 1 year ago

Also running into this issue! Additionally it's not quite clear to me when overrides are applied and when not, and how it affects upstream packages, etc.

ForbiddenEra commented 1 year ago

Just want to pop back in and say I tried using an override again recently, including with wiping node_modules + package-lock.json etc and no luck; a package I was using had a dependency that had it's own dependency with an audit issue (tap using semver) and I wanted to override it, not sure if it's because of how the semver dependency was included in tap but regardless should be able to override IMHO but just kept getting a message saying "use npm audit fix" to fix which wouldn't work, I had to manually change tap to use newer semver in my node_modules which is far from ideal ofc.

SukkaW commented 1 year ago

I am facing the same issue on npm v9.8.1 as well. Using npm update to rebuild package-lock.json does help.

jakubmazanec commented 1 year ago

Is this being worked on? If not, can we please at least get an estimate when this will be fixed? This really is critical bug and makes npm almost unusable.

SukkaW commented 1 year ago

Is this being worked on? If not, can we please at least get an estimate when this will be fixed? This really is critical bug and makes npm almost unusable.

@jakubmazanec

You should switch to yarn or pnpm if it becomes critical. Both yarn and pnpm will update the lockfile after seeing modified overrides.

You really should not expect npm to fix this issue. The npm team, having realized the issue (this issue was opened by one of them) a year ago, has yet to take any action on fixing this. They never share their ideas and updates about this issue.

If you have no choice but to stick with npm, viable workarounds do exist. You can simply run npm update to rebuild the lockfile. Deleting package-lock.json and re-running npm i to generate a brand new lockfile is also possible.

AlonNavon commented 11 months ago

I opened a pull request that starts to fix the issue.

akinnee commented 9 months ago

I am encountering this on 10.2.5. My workaround was to go delete the entry for the offending package from package-lock.json by hand before running npm install.

Anyone know of a good tool to automate removing all references to a specific package from package-lock.json?

rgant commented 9 months ago

@akinnee

For this override:

  "overrides": {
    "ng2-charts@^5.0.4": {
      "@angular/cdk@>=16.0.0": "^16.0.0"
    }

I just write a sed like:

sed -i '' -e 's#"@angular/cdk": ">=16.0.0",#"@angular/cdk": "^16.0.0",#' package-lock.json
akinnee commented 9 months ago

@rgant Nice, that will cover a lot of cases.

I was thinking maybe a node script that reads package-lock.json in as an object, loops through finds any package definitions (resolutions?) that end with the target package name, deletes them from the object, and then writes back to package-lock.json

e.g. (not a real command for those of you who are just skimming for a solution)

npx delete-package-from-lockfile node-fetch

would edit package-lock.json to delete the object that has the key node_modules/gaxios/node_modules/node-fetch, for example.

Overkill for me to write that now that I've already solved it manually. 😆

OZZlE commented 2 months ago

I found a little tediuos but working workaround (after adding overrides):

npm ls volnerable-package 

then in the ouput note the module prefixed by +-- at the top level (no indentation)

take all those packages and uninstall them, then install with the same versions again, eg:

npm uninstall package1 packge2 && npm i package1@^same.version.as-before package2@^same.version.as-before # [...]
fregante commented 2 months ago

@OZZlE that worked wonderfully! Much better than the advice to run npm update, which has a lot of side effects.

mapmarkus commented 2 weeks ago

I had the same issue. Using @OZZlE's solution is the only thing that worked for me.

Affected versions:

> node --version
v20.12.1
> npm --version
10.9.0
jdalton commented 2 weeks ago

With packages that add overrides as a service this is not the best. I'm looking for a scripted way to avoid this. At the moment that's calling npm update.

SukkaW commented 2 weeks ago

With packages that add overrides as a service this is not the best. I'm looking for a scripted way to avoid this. At the moment that's calling npm update.

I wouldn't recommend using npm update. npm update is very slow and could potentially break an existing application when libraries don't follow semver (Considering that even libraries maintained by npm team members wouldn't follow semver and introduce breaking changes in minor version bumps. Besides, npm's semver uses its own standard and is completely not compliant with the official semver standard, see https://github.com/npm/cli/issues/4958).

In fact, I wouldn't recommend using npm at all, just use pnpm/yarn.

jdalton commented 1 week ago

@SukkaW

I wouldn't recommend using npm update

Agreed but no other options for folks using npm. It's definitely not a good option.

Update:

Hoping this can land soon https://github.com/npm/cli/pull/7025.