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

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

Open
lukekarrys opened this issue Nov 12, 2022 · 23 comments
Open
Labels
Bug thing that needs fixing config:overrides Issues dealing with the overrides feature Priority 0 will get attention right away Release 9.x work is associated with a specific npm 9 release

Comments

@lukekarrys
Copy link
Contributor

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

From: #4232 (comment)


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
Copy link

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
Copy link

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

@bnbdr
Copy link

bnbdr commented May 24, 2023

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
Copy link

ecl1ps commented May 25, 2023

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
Copy link

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.

@lukekarrys lukekarrys added Priority 0 will get attention right away Release 9.x work is associated with a specific npm 9 release and removed Release 8.x work is associated with a specific npm 8 release labels Jun 28, 2023
@lukekarrys lukekarrys self-assigned this Jun 28, 2023
@aakashsarnobat
Copy link

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
Copy link

ecl1ps commented Jul 7, 2023

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
Copy link

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
Copy link

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
Copy link

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
Copy link

SukkaW commented Aug 10, 2023

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

@jakubmazanec
Copy link

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
Copy link

SukkaW commented Aug 30, 2023

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
Copy link

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

@akinnee
Copy link

akinnee commented Jan 9, 2024

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
Copy link

rgant commented Jan 9, 2024

@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
Copy link

akinnee commented Jan 9, 2024

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

@lukekarrys lukekarrys removed their assignment May 13, 2024
dlguswo333 added a commit to dlguswo333/react-playground that referenced this issue May 15, 2024
@OZZlE
Copy link

OZZlE commented Jul 31, 2024

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
Copy link
Contributor

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

@mapmarkus
Copy link

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
Copy link
Contributor

jdalton commented Oct 9, 2024

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
Copy link

SukkaW commented Oct 10, 2024

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 #4958).

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

@jdalton
Copy link
Contributor

jdalton commented Oct 11, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing config:overrides Issues dealing with the overrides feature Priority 0 will get attention right away Release 9.x work is associated with a specific npm 9 release
Projects
None yet
Development

No branches or pull requests