npm default install command always runs if binding.gyp exists, and package install script is ignored

[benwalder]

Is there an existing issue for this?

• I have searched the existing issues

This issue exists in the latest npm version

• I am using the latest npm

Current Behavior

1. use npm 7 or 8
2. package.json contains install script
3. project root directory contains binding.gyp
4. npm install results in default npm install command node-gyp rebuild running instead of the install script in package.json

It works correctly with npm 6.

Expected Behavior

Expect it to behave as documented:

If there is a binding.gyp file in the root of your package and you haven't defined your own install or preinstall scripts, npm will default the install command to compile using node-gyp via node-gyp rebuild

I assume this means:

If there is a binding.gyp file in the root of your package and you have defined your own install or preinstall scripts, npm will run your install and preinstall scripts and not default the install command to compile using node-gyp via node-gyp rebuild

Steps To Reproduce

1. Create new directory with new package.json that has no dependencies
2. See canvas has binding.gyp and an install script in package.json.
3. npm i canvas@latest -S --verbose
4. "install": "node-pre-gyp install --fallback-to-build --update-binary", in canvas package.json is NOt run but should be
5. npm info run canvas@2.9.3 install node_modules/canvas node-gyp rebuild is run but should NOT be
6. Behaves the same if canvas is added as a dependency in package.json and run npm install

npm timing build:link Completed in 14ms
npm info run bufferutil@4.0.6 install node_modules/bufferutil node-gyp rebuild
npm info run canvas@2.9.3 install node_modules/canvas node-gyp rebuild
npm info run utf-8-validate@5.0.9 install node_modules/utf-8-validate node-gyp rebuild
npm timing auditReport:getReport Completed in 2206ms
npm timing metavuln:packument:simple-get Completed in 0ms
npm timing metavuln:cache:get:security-advisory:simple-get:dbHiY3JnNP4Jr4742q+4dVLSv3XH7FpEftzGzOaOhPntyoPMZBTutjtG1UEIt3y0lLR8CJaI5/gMVJm6WwgYGg== Completed in 7ms
npm timing metavuln:load:security-advisory:simple-get:196538 Completed in 1ms
npm timing metavuln:calculate:security-advisory:simple-get:196538 Completed in 8ms
npm timing metavuln:packument:canvas Completed in 0ms
npm timing metavuln:cache:get:security-advisory:canvas:iM1APVjmhbDx0X3xpRuxOwUQvflbdVAPg0uVWMVNMm0wj7U/KvLbCf7/QXuuKtAJTVE+pHimM8kwJEs3s13Iog== Completed in 10ms
npm timing metavuln:load:security-advisory:canvas:dbHiY3JnNP4Jr4742q+4dVLSv3XH7FpEftzGzOaOhPntyoPMZBTutjtG1UEIt3y0lLR8CJaI5/gMVJm6WwgYGg== Completed in 1ms
npm timing metavuln:calculate:security-advisory:canvas:dbHiY3JnNP4Jr4742q+4dVLSv3XH7FpEftzGzOaOhPntyoPMZBTutjtG1UEIt3y0lLR8CJaI5/gMVJm6WwgYGg== Completed in 11ms
npm timing auditReport:init Completed in 80ms
npm timing reify:audit Completed in 2287ms
npm info run canvas@2.9.3 install { code: 1, signal: null }
npm timing reify:rollback:createSparse Completed in 421ms
npm timing reify:rollback:retireShallow Completed in 0ms
npm timing command:i Completed in 22622ms
npm verb stack Error: command failed
npm verb stack     at ChildProcess.<anonymous> (/usr/local/lib/node_modules/npm/node_modules/@npmcli/promise-spawn/lib/index.js:63:27)
npm verb stack     at ChildProcess.emit (node:events:527:28)
npm verb stack     at maybeClose (node:internal/child_process:1092:16)
npm verb stack     at Process.ChildProcess._handle.onexit (node:internal/child_process:302:5)
npm verb pkgid canvas@2.9.3
npm verb cwd /Users/dev/Documents/dev/test
npm verb Darwin 21.5.0
npm verb node v16.16.0
npm verb npm  v8.15.1
npm ERR! code 1
< non applicable lines removed >
npm ERR! gyp ERR! command "/usr/local/bin/node" "/usr/local/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js" "rebuild"
npm ERR! gyp ERR! cwd /Users/dev/Documents/dev/test/node_modules/canvas
npm ERR! gyp ERR! node -v v16.16.0
npm ERR! gyp ERR! node-gyp -v v9.0.0
npm ERR! gyp ERR! not ok
npm verb exit 1
npm timing npm Completed in 22717ms
npm verb unfinished npm timer reify 1659048451229
npm verb unfinished npm timer reify:build 1659048471025
npm verb unfinished npm timer build 1659048471026
npm verb unfinished npm timer build:deps 1659048471026
npm verb unfinished npm timer build:run:install 1659048471044
npm verb unfinished npm timer build:run:install:node_modules/bufferutil 1659048471044
npm verb unfinished npm timer build:run:install:node_modules/canvas 1659048471063
npm verb unfinished npm timer build:run:install:node_modules/utf-8-validate 1659048471082
npm verb code 1

It seems the problem is identified here #3341. arborist is making the decision of what install to run when it only has the manifest of the package which does not include scripts.

This error prevents the install from being successful because all the binary dependencies needed to build canvas locally are not installed. Installing the dependencies every place the install is run is not feasible for us.

The work around mentioned in #3341 of adding "hasInstallScript": true to package-lock.json is not feasible for us.

Environment

• npm -v 8.15.1
• node -v v16.16.0
• macOS 12.4
• System Darwin 21.5.0
• npm config ls:

; "user" config from /Users/dev/.npmrc

canvas_binary_host_mirror = "https://dev:<token removed>@statefarm.jfrog.io/artifactory/github-com-remote/Automattic/node-canvas/releases/download/" 
disturl = "https://dev:<token removed>@statefarm.jfrog.io/artifactory/nodejs-org-download-release-remote/%" 
fse_binary_host_mirror = "https://dev:<token removed>@statefarm.jfrog.io/artifactory/fsevents-binaries-aws-remote/" 
proxy = "http://in00pxy1.opr.statefarm.org:8000/" 
registry = "https://statefarm.jfrog.io/artifactory/api/npm/npm-virtual/" 

; "project" config from /Users/dev/Documents/dev/test/.npmrc

; node bin location = /usr/local/bin/node
; node version = v16.16.0
; npm local prefix = /Users/dev/Documents/dev/test
; npm version = 8.15.1
; cwd = /Users/dev/Documents/dev/test
; HOME = /Users/dev

[jussisaurio]

We have a very similar issue currently. Our builds started failing after this change to dd-native-metrics: DataDog/dd-native-metrics-js#21

Here's a list of things that are the same or similar as in your case:

• node-gyp rebuild gets run when installing this package and we don't understand why
• Works with npm 6
• Does not work with npm 8
• We are using a private npm registry w/ Artifactory. The issue does not occur when installing from the public npm registry.

Here's a list of things that are dissimilar:

• In the case of @datadog/native-metrics the binding.gyp file was removed, but node-gyp rebuild still runs

[rochdev]

We're getting a lot of reports of this since removing the binding.gyp file from our project:

DataDog/dd-trace-js#2239
DataDog/dd-native-metrics-js#28

The file doesn't exist anymore, so it's unclear why npm keeps trying to build it anyway.

[jussisaurio]

We're getting a lot of reports of this since removing the binding.gyp file from our project:

DataDog/dd-trace-js#2239 DataDog/dd-native-metrics-js#22 DataDog/dd-native-metrics-js#28

The file doesn't exist anymore, so it's unclear why npm keeps trying to build it anyway.

Seems that all of these reports share at least one thing in common: a private registry.

[rochdev]

Seems that all of these reports share at least one thing in common: a private registry.

This was also my assumption at first, but according to DataDog/dd-native-metrics-js#27 (comment) it seems to be isolated to newer versions of npm. Maybe it's a combination of both that somehow triggers the issue.

[jussisaurio]

Seems that all of these reports share at least one thing in common: a private registry.

This was also my assumption at first, but according to DataDog/dd-native-metrics-js#27 (comment) it seems to be isolated to newer versions of npm. Maybe it's a combination of both that somehow triggers the issue.

Oh yeah, true, it's definitely also newer versions of npm.

[rochdev]

I was able to reproduce with the following steps:

nvm install 18.7.0
docker run -d -p 127.0.0.1:4873:4873/tcp verdaccio/verdaccio
export NPM_CONFIG_REGISTRY=http://localhost:4873
npm init -y
npm install --save @datadog/native-metrics@1.4.0
npm install --save @datadog/native-metrics@1.4.1

It looks like updating a package from a version that had a binding.gyp to a version that doesn't have one fails when using a registry proxy. I'm trying to isolate the specific npm version where the issue started

[rochdev]

The issue occurs with npm >=7.0.0 but not with 6.14.17

[rochdev]

Using npm install --ignore-scripts --save @datadog/native-metrics@1.4.1 works, even though there are no scripts in the latest version of the package (but there used to be an install script before).

[mkaufmaner]

@rochdev If you were to publish a new major version of @datadog/native-metrics, testing locally with verdaccio, could that potentially circumvent this issue?

[rochdev]

@mkaufmaner I don't think so because in my local tests, once the issue started happening, I was unable to install any version of dd-trace anymore, even older versions that do have the binding.gyp available so I think the issue is not related to the version itself and once the cache is corrupted it's game over. The above workaround does seem to permanently fix the cache.

[mkaufmaner]

@rochdev

Thank you, just thinking out loud here.

The above workaround does seem to permanently fix the cache.

The problem is I can't implement this workaround for the thousands of CICD pipelines we have. It is simply impractical.

once the cache is corrupted it's game over.

Well, I don't think this is a local cache problem because even when I run npm cache clear --force && npm i I still get the same build error as before. Are you talking about the remote cache in the proxy registry?

[rochdev]

The problem is I can't implement this workaround for the thousands of CICD pipelines we have. It is simply impractical.

@mkaufmaner How do you generally handle updates in these pipelines? Wouldn't the same approach work in this case?

Well, I don't think this is a local cache problem

I thought the same thing, but after removing the Verdaccio container completely and running a new one I was getting the same issue. I think it's possible that npm cache clean doesn't actually clean everything.

[rochdev]

@mkaufmaner I tried to switch version of Node with nvm to test your theory since that uses a different npm instance with its own installation folder including its cache. Even with this change I am still getting the same issue, so I'm not sure what's going on. I basically changed the instance of npm completely, and recreated the container entirely with a fresh proxy with no cached dependencies, and the issue is still there. Re-using the above workaround clears the issue again for all versions, but if I uninstall dd-trace and start over I get the issue again. I wonder if npm caches anything outside of its own folder.

[mkaufmaner]

@rochdev

I thought the same thing, but after removing the Verdaccio container completely and running a new one I was getting the same issue. I think it's possible that npm cache clean doesn't actually clean everything.

You have to pass the --force flag for the cache clean to actually work.
npm cache ls && npm cache clean --force && npm cache ls

How do you generally handle updates in these pipelines? Wouldn't the same approach work in this case?

Without divulging too much information, I can say that updating CICD pipelines ares not assisted with automation. Thus, not a feasible solution.

I tried to switch version of Node with nvm to test your theory since that uses a different npm instance with its own installation folder including its cache. Even with this change I am still getting the same issue, so I'm not sure what's going on. I basically changed the instance of npm completely, and recreated the container entirely with a fresh proxy with no cached dependencies, and the issue is still there. Re-using the above workaround clears the issue again for all versions, but if I uninstall dd-trace and start over I get the issue again. I wonder if npm caches anything outside of its own folder.

I tried similar strategies with the same results.

We have also opened up tickets with jFrog support to see if they have anything to add.

Regardless, we can't deploy our applications and libraries that consume the @datadog/native-metrics library, and subsequent dd-trace library, until this is problem is fixed. Furthermore, out of urgency and transparency, I have to escalate this with our account rep. So please, at the very least, add the binding.gyp file back to the published package and release a new patch version. I would be more than happy to help contribute to a fix, I am just trying to unblock our developers.

[rochdev]

@mkaufmaner Would environment variables work for your use case instead? The same workaround can be applied with NPM_CONFIG_IGNORE_SCRIPTS=true. I'm trying to not have to revert this change since it will come back in the future. If all else fails I'll try to add an empty install script to see if that would fix the issue since I think this is related more to scripts than it is to the binding.gyp file.

[mkaufmaner]

@mkaufmaner Would environment variables work for your use case instead? The same workaround can be applied with NPM_CONFIG_IGNORE_SCRIPTS=true. I'm trying to not have to revert this change since it will come back in the future. If all else fails I'll try to add an empty install script to see if that would fix the issue since I think this is related more to scripts than it is to the binding.gyp file.

Setting NPM_CONFIG_IGNORE_SCRIPTS=true would not work since we have other dependencies that rely on install scripts. I understand that this will come back in the future, and I do believe this is a problem with npm itself, but in the meantime we need a way forward. I hate reverting changes as much as the next person but I can't find a quick solution that can be implemented across the entire organization that wouldn't require a significant amount of developer resources.

[rochdev]

@mkaufmaner It looks like adding an empty install script resolves the issue. I'll add that for now until the issue is fixed in npm.

[mkaufmaner]

@mkaufmaner It looks like adding an empty install script resolves the issue. I'll add that for now until the issue is fixed in npm.

Thank you, I hope this fixes the problem.

[joshuakb2]

I am seeing this issue, too, using a private registry and trying to install the sqlite3 package. The hasInstallScript workaround mentioned above is working for me.

[wraithgar]

I'm a little confused by the original issue here. This very well may be a documentation issue.

Some of the additional comments don't appear to be the same as what was originally posted either. I would ask that if you have an issue that isn't the one in the main issue body to open a separate issue, it makes things very confusing very fast if folks try to glob "similar" issues into one. They may seem similar at a glance but it's likely not, under the hood.

To address the "how it works/how it should work" section of this issue, this is how npm works:

During install (and rebuild), npm will run any install scripts defined for a package. This step is skipped if ignoreScripts is set (--ignore-scripts flag/config).

When the script running code is asked to run install, if one is not found it checks if there is a node-gyp binding file in the package and runs a default gyp install command (node-gyp rebuild). If the install script is found it runs that and does not attempt the gyp install.

If I run npm i canvas@latest --loglevel verbose --foreground-scripts I do see canvas's install script run, and not the default

~/D/n/s/node-gyp $ npm view canvas scripts.install
node-pre-gyp install --fallback-to-build --update-binary
$ npm i canvas@latest --loglevel verbose --foreground-scripts
[...]
npm info run canvas@2.10.1 install node_modules/canvas node-pre-gyp install --fallback-to-build --update-binary

So as far as I can tell things are working as expected. canvas defines an install script, and that is what npm is running when I install it.

ETA: The presence of a binding.gyp file in my project root did not affect the outcome in any way.

[joshuakb2]

Apologies if this issue is not actually the same as mine. This message made me think that this bug specifically pertains to install behavior being different when using a custom registry:
#5234 (comment)

[wraithgar]

npm does not do anything differently with scripts when using a custom registry

[joshuakb2]

npm does not do anything differently with scripts when using a custom registry

All I know is that when I use my custom registry, the bug is seen, and when I use https://registry.npmjs.org the bug is not seen, and others have reported the same thing. I'm using Verdaccio, maybe others are too and it's a bug with that project instead.

Edit: Others in this thread have mentioned that they are using Artifactory, not Verdaccio, so if the bug were not in npm it would have to be the same bug in 2 different projects.

[rochdev]

The bug also appeared only in more recent versions of npm and was not present in older versions.

[rochdev]

@wraithgar Here's a docker-compose.yml file that makes it easy to reproduce:

version: "2"
services:
  node:
    image: node:18.7.0
    depends_on:
      - registry
    working_dir: /app
    environment:
      - NPM_CONFIG_REGISTRY=http://registry:4873
    command:
      - /bin/sh
      - -c
      - |
        npm init -y
        npm install --save @datadog/native-metrics@1.4.0
        npm install --save @datadog/native-metrics@1.4.1
  registry:
    image: verdaccio/verdaccio:5.14.0

The problem exists with both Verdaccio and Artifactory, and didn't exist with npm <7.

[wraithgar]

@rochdev what is "the problem"?

[joshuakb2]

If you run that docker-compose.yml file (docker-compose run node) you'll see that npm fails to install one of those packages.

[rochdev]

It actually fails to install the exact same package but with a new version that has the binding.gyp removed, and for some reason npm remembers that there was a binding.gyp before and is looking for it.

[joshuakb2]

I don't know about you, but when I remove the line that installs 1.4.0 and just directly install 1.4.1, it still fails.

However, your reproduction doesn't have any trouble installing the sqlite3 package, which I'm having trouble with. So maybe my issue must be a little bit different.

[wraithgar]

Mystery solved. This is a bug in the published package @datadog/native-metrics. The packument itself has a scripts entry that includes an install, but the package.json inside the attached tarball does not. So when you run something like:

$ npm view @datadog/native-metrics@1.4.1 scripts
{
  rebuild: 'node-gyp rebuild',
  lint: 'node scripts/check_licenses.js && eslint .',
  test: "mocha 'test/**/*.spec.js'",
  install: 'node-gyp rebuild'
}

Everything seems ok. But if you actually download the package itself you find this

~/D/n/s/n/package $ curl (npm view @datadog/native-metrics@1.4.1 dist.tarball) -sO
~/D/n/s/n/package $ tar zxvf native-metrics-1.4.1.tgz package/package.json
x package/package.json
~/D/n/s/n/package $ cat package/package.json|json scripts
{
  "rebuild": "node-gyp rebuild",
  "lint": "node scripts/check_licenses.js && eslint .",
  "test": "mocha 'test/**/*.spec.js'"
}

The place npm ends up looking for scripts doesn't have an install script, so of course can not run one.

[wraithgar]

1.4.0 does not have this bug

~/D/n/s/node-gyp $ curl (npm view @datadog/native-metrics@1.4.0 dist.tarball) -sO
~/D/n/s/node-gyp $ tar zxvf native-metrics-1.4.0.tgz package/package.json
x package/package.json
~/D/n/s/node-gyp $ cat package/package.json|json scripts
{
  "install": "(node scripts/should_rebuild && node-gyp-build) || exit 0",
  "rebuild": "node-gyp rebuild",
  "lint": "node scripts/check_licenses.js && eslint .",
  "test": "mocha 'test/**/*.spec.js'"
}

[wraithgar]

npm only looks inside the packument for dependencies, not for scripts. It uses the package.json in the attached tarball for the remaining metadata.

[wraithgar]

Closing this issue as it looks like the original issue was a misunderstanding of the docs. and the "I also have this issue" issue appears to be a bug w/ the published package and not npm. If folks have other issues I'd humbly ask that you make new issues for them so that they can be properly described in our issue template, and triaged without trying to look at multiple issues at once.

[rochdev]

@wraithgar We removed the binding.gyp on purpose in 1.4.1, which triggered this bug in npm but only when using a private registry. How can we remove binding.gyp without causing issues if a build is no longer needed?

[rochdev]

We were forced to add back an empty install script to work around the bug, but we'd like to be able to remove that too since some security tools complain about the presence of an install script.

[wraithgar]

@rochdev Let's start a new issue for this. Please include the actual error that you get when installing, and the log files if possible.

[wraithgar]

@rochdev actually no you don't need to open a new issue. The inverse of my statement is still true.

~/D/n/s/node-gyp $ npm view @datadog/native-metrics@1.4.1 scripts
{
  rebuild: 'node-gyp rebuild',
  lint: 'node scripts/check_licenses.js && eslint .',
  test: "mocha 'test/**/*.spec.js'",
  install: 'node-gyp rebuild'
}

The published version of @datadog/native-metrics@1.4.1 has an install script. The one in the tarball doesn't have one, but that's not the one npm is looking at. It only looks at the manifest inside the packument at [registry]/[package-name] under the versions object for that version.

1.4.1 somehow had an install script still in the packument, but not in the one in the tarball. I don't think this is a registry bug, we have recently removed scripts from npm itself and the registry didn't "keep" them. Also the script isn't identical to the last version. This is either a verdaccio bug (where it merged objects before uploading? I don't know verdaccio just guessing assuming you published through verdaccio too) or some weird state w/ the package locally where that script was in the package.json before npm pack but not during.

Try publishing a new version to the dev tag with no install script whatsoever, and verify that npm view shows no install script.

[wraithgar]

The error as-is is not an npm bug, it's doing exactly what was asked. How you published that package w/ the disconnect between the package.json in the tarball and the manifest itself is another mystery. If you can reproduce that behavior you have yourself a new bug.

[wraithgar]

The reason this is happening in verdaccio and not against the npm registry appears to be because verdaccio does not support what we call "corgi" docs, which are streamlined packuments with some fields removed. I am debugging this right now to see if this is a problem w/ npm and how it finds package scripts.

[wraithgar]

The reason there is an install script in the published manifest is because of this https://github.com/npm/read-package-json/blob/main/lib/read-json.js#L237

There was a binding.gyp file in the cwd when this package was published. We're looking into that behavior to see if this can be removed. npm already does this behavior during install without the need of having that install script set.

[rochdev]

@wraithgar So should we just make sure to remove the binding.gyp completely from the folder before publishing? I think we left it there but simply added it to .npmignore.

[wraithgar]

@rochdev yes that would prevent npm from auto-adding that install script during publish. You could then safely remove the exit 0 you have there to keep it from happening.

[feross]

From my perspective, this is a security issue. Spurred by Darcy's recent research on the matter, Socket Security has published a detection for this issue to help developers protect their repos from any malicious use of this potential attack vector.

[panki27]

I've written a script to test for mismatches between the manifests:
https://github.com/panki27/npm-manifest-check

[grayashh]

Is there a way to solve the current issue by building a proxy?