cli icon indicating copy to clipboard operation
cli copied to clipboard

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

Open lukekarrys opened this issue 3 years ago • 23 comments

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 [email protected]. 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.

lukekarrys avatar Nov 12 '22 22:11 lukekarrys

I can confirm that I have the same issue on the latest [email protected].

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.

AlonNavon avatar May 08 '23 16:05 AlonNavon

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

jakubmazanec avatar May 24 '23 12:05 jakubmazanec

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"
  }
}

bnbdr avatar May 24 '23 14:05 bnbdr

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 [email protected] 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 [email protected] --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.

ecl1ps avatar May 25 '23 12:05 ecl1ps

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.

AlonNavon avatar May 28 '23 10:05 AlonNavon

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?

aakashsarnobat avatar Jul 06 '23 23:07 aakashsarnobat

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.

ecl1ps avatar Jul 07 '23 05:07 ecl1ps

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?

aakashsarnobat avatar Jul 07 '23 17:07 aakashsarnobat

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.

hybridherbst avatar Jul 25 '23 09:07 hybridherbst

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.

ForbiddenEra avatar Jul 31 '23 04:07 ForbiddenEra

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

SukkaW avatar Aug 10 '23 04:08 SukkaW

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 avatar Aug 30 '23 14:08 jakubmazanec

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.

SukkaW avatar Aug 30 '23 15:08 SukkaW

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

AlonNavon avatar Nov 27 '23 12:11 AlonNavon

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?

akinnee avatar Jan 09 '24 19:01 akinnee

@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

rgant avatar Jan 09 '24 20:01 rgant

@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. 😆

akinnee avatar Jan 09 '24 21:01 akinnee

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 # [...]

OZZlE avatar Jul 31 '24 15:07 OZZlE

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

fregante avatar Aug 19 '24 17:08 fregante

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

mapmarkus avatar Oct 08 '24 14:10 mapmarkus

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.

jdalton avatar Oct 09 '24 14:10 jdalton

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.

SukkaW avatar Oct 10 '24 09:10 SukkaW

@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.

jdalton avatar Oct 11 '24 05:10 jdalton

This was fixed by #8089 and released in 11.2.0.

lukekarrys avatar Mar 12 '25 16:03 lukekarrys