fix: respect gypfile:false when detecting install scripts

docs/lib/content/configuring-npm/package-json.md

@@ -554,6 +554,9 @@ To prevent npm from automatically building your module with node-gyp, set `gypfi
 }
 ```
 
+This setting disables only the automatic `node-gyp rebuild` script.
+It does not disable explicit lifecycle scripts in `package.json`.
+
 This is useful for packages that include native addons but want to handle the build process differently, or packages that have a binding.gyp file but should not be built as a native addon.
 
 ### config
@@ -1262,7 +1265,8 @@ npm will default some values based on package contents.
 
 * `"scripts":{"install": "node-gyp rebuild"}`
 
-  If there is a `binding.gyp` file in the root of your package and you have not defined an `install` or `preinstall` script, npm will default the `install` command to compile using node-gyp.
+  If the root of your package contains `binding.gyp`, npm uses `node-gyp rebuild` as the default `install` script.
+  An explicit `install` or `preinstall` script, or `"gypfile": false`, disables this default.
 
 * `"contributors": [...]`
 

docs/lib/content/using-npm/scripts.md

@@ -145,6 +145,8 @@ Because `preinstall` runs before reify, scripts cannot rely on packages from `no
 
 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`
 
+Setting `"gypfile": false` disables this default and leaves explicit lifecycle scripts unchanged.
+
 These are run from the scripts of `<pkg-name>`
 
 #### [`npm pack`](/commands/npm-pack)

lib/utils/check-allow-scripts.js

@@ -15,13 +15,16 @@ const { collectUnreviewedScripts } = require('@npmcli/arborist/lib/unreviewed-sc
 // `includeWhenIgnored` keeps listing unreviewed packages even when
 // ignore-scripts is set, so approve/deny can show what you'd move from a
 // blanket ignore-scripts to an allowlist. Execution callers leave it false.
-const checkAllowScripts = async ({ arb, npm, tree, includeWhenIgnored = false }) =>
+const checkAllowScripts = async ({
+  arb, npm, tree, includeWhenIgnored = false, readFromDisk = true,
+}) =>
   collectUnreviewedScripts({
     tree: tree || arb.actualTree,
     policy: arb.options?.allowScripts || null,
     ignoreScripts: !!arb.options?.ignoreScripts,
     dangerouslyAllowAllScripts: !!npm?.flatOptions?.dangerouslyAllowAllScripts,
     includeWhenIgnored,
+    readFromDisk,
   })
 
 module.exports = checkAllowScripts

lib/utils/strict-allow-scripts-preflight.js

@@ -33,7 +33,9 @@ const strictAllowScriptsPreflight = async ({ arb, npm, idealTreeOpts }) => {
     tree = arb.actualTree
   }
 
-  const unreviewed = await checkAllowScripts({ arb, npm, tree })
+  const unreviewed = await checkAllowScripts({
+    arb, npm, tree, readFromDisk: idealTreeOpts === undefined,
+  })
   if (unreviewed.length === 0) {
     return
   }

test/lib/commands/ci.js

@@ -49,6 +49,66 @@ const abbrev = {
   test: 'test file',
 }
 
+t.test('ci respects gypfile:false in installed package data', async t => {
+  require('../../fixtures/tspawk')(t)
+  const cases = [
+    { name: 'default policy' },
+    { name: 'stale presence flag', hasInstallScript: true },
+    { name: 'approved', allowScripts: { 'abbrev@1.0.0': true } },
+    { name: 'bypass', config: { 'dangerously-allow-all-scripts': true } },
+    { name: 'ignored', config: { 'ignore-scripts': true } },
+    { name: 'strict', config: { 'strict-allow-scripts': true } },
+  ]
+  for (const lockfileVersion of [2, 3]) {
+    for (const { name, config, allowScripts, hasInstallScript } of cases) {
+      await t.test(`v${lockfileVersion}: ${name}`, async t => {
+        const lock = {
+          ...packageLock,
+          lockfileVersion,
+          packages: {
+            ...packageLock.packages,
+            'node_modules/abbrev': {
+              ...packageLock.packages['node_modules/abbrev'],
+              ...(hasInstallScript ? { hasInstallScript } : {}),
+            },
+          },
+        }
+        if (lockfileVersion === 3) {
+          delete lock.dependencies
+        }
+        const { npm, registry, logs } = await loadMockNpm(t, {
+          config: { audit: false, ...config },
+          prefixDir: {
+            'package.json': JSON.stringify({ ...packageJson, allowScripts }),
+            'package-lock.json': JSON.stringify(lock),
+            node_modules: { stale: 'removed by ci' },
+            abbrev: {
+              'package.json': JSON.stringify({ name: 'abbrev', version: '1.0.0', gypfile: false }),
+              'binding.gyp': '',
+            },
+          },
+        })
+        const manifest = registry.manifest({ name: 'abbrev' })
+        await registry.tarball({
+          manifest: manifest.versions['1.0.0'],
+          tarball: path.join(npm.prefix, 'abbrev'),
+        })
+        await npm.exec('ci', [])
+
+        t.ok(fs.existsSync(path.join(npm.prefix, 'node_modules/abbrev/binding.gyp')),
+          'installs the native package')
+        t.notOk(fs.existsSync(path.join(npm.prefix, 'node_modules/stale')), 'cleans node_modules')
+        t.strictSame(logs.warn.byTitle('install-scripts'), [], 'no blocked-script warning')
+        t.strictSame(
+          JSON.parse(fs.readFileSync(path.join(npm.prefix, 'package-lock.json'), 'utf8')),
+          lock,
+          'ci does not rewrite the project lockfile'
+        )
+      })
+    }
+  }
+})
+
 t.test('reifies, but doesn\'t remove node_modules because --dry-run', async t => {
   const { npm, joinedOutput } = await loadMockNpm(t, {
     config: {

test/lib/commands/install-scripts.js

@@ -148,6 +148,52 @@ t.test('install-scripts ls with no unreviewed says so', async t => {
   t.match(joinedOutput(), /No packages with unreviewed install scripts/)
 })
 
+t.test('install-scripts respects gypfile:false through the hidden lockfile', async t => {
+  for (const hasInstallScript of [false, true]) {
+    for (const command of ['ls', 'approve', 'prune']) {
+      await t.test(`${command}, hasInstallScript=${hasInstallScript}`, async t => {
+        const fixture = setupProject({
+          withScripts: [],
+          noScripts: ['native-pkg'],
+          allowScripts: command === 'prune' ? { 'native-pkg': true } : undefined,
+        })
+        fixture.node_modules['native-pkg'] = {
+          'package.json': JSON.stringify({
+            name: 'native-pkg',
+            version: '1.0.0',
+            gypfile: false,
+          }),
+          'binding.gyp': '',
+        }
+        const lock = JSON.parse(fixture['package-lock.json'])
+        if (hasInstallScript) {
+          lock.packages['node_modules/native-pkg'].hasInstallScript = true
+        }
+        fixture['package-lock.json'] = JSON.stringify(lock)
+        fixture.node_modules['.package-lock.json'] = JSON.stringify(lock)
+        const { npm, prefix, joinedOutput } = await mockNpm(t, {
+          prefixDir: fixture,
+          config: { all: command === 'approve' },
+        })
+        const Arborist = require('@npmcli/arborist')
+        const tree = await new Arborist({ path: prefix }).loadActual()
+        t.equal(tree.meta.hiddenLockfile, true, 'uses the hidden lockfile')
+        t.equal(tree.children.get('native-pkg').package.gypfile, undefined,
+          'tree metadata omits the opt-out')
+
+        await npm.exec('install-scripts', [command])
+
+        if (command === 'ls') {
+          t.match(joinedOutput(), /No packages with unreviewed install scripts/)
+        } else {
+          const pkg = JSON.parse(fs.readFileSync(resolve(prefix, 'package.json'), 'utf8'))
+          t.notOk(pkg.allowScripts, 'does not retain an unnecessary approval')
+        }
+      })
+    }
+  }
+})
+
 t.test('install-scripts ls rejects positional args', async t => {
   const { npm } = await mockNpm(t, {
     prefixDir: setupProject({ withScripts: ['canvas'] }),

test/lib/commands/install.js

@@ -33,6 +33,173 @@ const packageJson = {
   },
 }
 
+t.test('gypfile:false survives fresh, lockfile, and repeated installs', async t => {
+  const cases = [
+    { name: 'default policy' },
+    { name: 'approved', allowScripts: { 'abbrev@1.0.0': true } },
+    { name: 'bypass', config: { 'dangerously-allow-all-scripts': true } },
+    { name: 'ignored', config: { 'ignore-scripts': true } },
+    { name: 'strict', config: { 'strict-allow-scripts': true } },
+  ]
+  for (const { name, config, allowScripts } of cases) {
+    await t.test(name, async t => {
+      const { npm, registry, logs } = await loadMockNpm(t, {
+        config: { audit: false, ...config },
+        prefixDir: {
+          'package.json': JSON.stringify({ ...packageJson, allowScripts }),
+          abbrev: {
+            'package.json': JSON.stringify({ name: 'abbrev', version: '1.0.0', gypfile: false }),
+            'binding.gyp': '',
+          },
+        },
+      })
+      const manifest = registry.manifest({ name: 'abbrev' })
+      await registry.package({ manifest })
+
+      for (const phase of ['fresh', 'lockfile', 'repeated']) {
+        if (phase === 'lockfile') {
+          fs.rmSync(path.join(npm.prefix, 'node_modules'), { recursive: true, force: true })
+          fs.rmSync(path.join(npm.config.get('cache'), '_cacache'), { recursive: true, force: true })
+        }
+        if (phase !== 'repeated') {
+          await registry.tarball({
+            manifest: manifest.versions['1.0.0'],
+            tarball: path.join(npm.prefix, 'abbrev'),
+          })
+        }
+        await npm.exec('install', [])
+
+        const lock = JSON.parse(fs.readFileSync(path.join(npm.prefix, 'package-lock.json'), 'utf8'))
+        t.notOk(lock.packages['node_modules/abbrev'].hasInstallScript,
+          `${phase}: no false script-presence flag`)
+        t.notOk('gypfile' in lock.packages['node_modules/abbrev'],
+          `${phase}: lockfile format is unchanged`)
+        t.strictSame(logs.warn.byTitle('install-scripts'), [], `${phase}: no blocked-script warning`)
+      }
+    })
+  }
+})
+
+t.test('transitive native opt-outs clear stale flags in both install strategies', async t => {
+  for (const installStrategy of ['hoisted', 'linked']) {
+    await t.test(installStrategy, async t => {
+      const { npm, registry, logs } = await loadMockNpm(t, {
+        config: { audit: false, 'install-strategy': installStrategy },
+        prefixDir: {
+          'package.json': JSON.stringify({
+            name: 'host',
+            version: '1.0.0',
+            dependencies: { 'parent-pkg': '1.0.0' },
+          }),
+          parent: {
+            'package.json': JSON.stringify({
+              name: 'parent-pkg',
+              version: '1.0.0',
+              dependencies: { 'native-pkg': '1.0.0' },
+            }),
+          },
+          native: {
+            'package.json': JSON.stringify({ name: 'native-pkg', version: '1.0.0', gypfile: false }),
+            'binding.gyp': '',
+          },
+        },
+      })
+      const parent = registry.manifest({
+        name: 'parent-pkg',
+        packuments: [{ version: '1.0.0', dependencies: { 'native-pkg': '1.0.0' } }],
+      })
+      const native = registry.manifest({
+        name: 'native-pkg',
+        packuments: [{ version: '1.0.0', hasInstallScript: true }],
+      })
+      await registry.package({ manifest: parent })
+      await registry.package({ manifest: native })
+      const lockPath = path.join(npm.prefix, 'package-lock.json')
+
+      for (const phase of ['fresh', 'clean-lockfile', 'repeat']) {
+        if (phase === 'clean-lockfile') {
+          const staleLock = JSON.parse(fs.readFileSync(lockPath, 'utf8'))
+          staleLock.packages['node_modules/native-pkg'].hasInstallScript = true
+          fs.writeFileSync(lockPath, JSON.stringify(staleLock))
+        }
+        if (phase === 'clean-lockfile') {
+          fs.rmSync(path.join(npm.prefix, 'node_modules'), { recursive: true, force: true })
+          fs.rmSync(path.join(npm.config.get('cache'), '_cacache'), { recursive: true, force: true })
+        }
+        if (phase !== 'repeat') {
+          await registry.tarball({
+            manifest: parent.versions['1.0.0'],
+            tarball: path.join(npm.prefix, 'parent'),
+          })
+          await registry.tarball({
+            manifest: native.versions['1.0.0'],
+            tarball: path.join(npm.prefix, 'native'),
+          })
+        }
+        await npm.exec('install', [])
+
+        const lock = JSON.parse(fs.readFileSync(lockPath, 'utf8'))
+        t.notOk(lock.packages['node_modules/native-pkg'].hasInstallScript,
+          `${phase}: clears the stale project-lockfile flag`)
+        t.strictSame(logs.warn.byTitle('install-scripts'), [], `${phase}: no false warning`)
+        const parentPath = fs.realpathSync(path.join(npm.prefix, 'node_modules/parent-pkg/package.json'))
+        const nativePath = require('node:module').createRequire(parentPath).resolve('native-pkg/package.json')
+        t.equal(JSON.parse(fs.readFileSync(nativePath, 'utf8')).gypfile, false,
+          `${phase}: the transitive opted-out package is installed`)
+      }
+    })
+  }
+})
+
+t.test('strict preflight rejects same-version replacements before changing installed files', async t => {
+  const { npm } = await loadMockNpm(t, {
+    config: { audit: false, 'allow-file': 'all', 'strict-allow-scripts': true },
+    prefixDir: {
+      'package.json': JSON.stringify({ name: 'host', version: '1.0.0' }),
+      old: {
+        'package.json': JSON.stringify({ name: 'native-pkg', version: '1.0.0', gypfile: false }),
+        'binding.gyp': '',
+        'contents.txt': 'old',
+      },
+      incoming: {
+        'package.json': JSON.stringify({
+          name: 'native-pkg',
+          version: '1.0.0',
+          scripts: { postinstall: 'echo incoming' },
+        }),
+        'binding.gyp': '',
+        'contents.txt': 'incoming',
+      },
+    },
+  })
+  for (const source of ['old', 'incoming']) {
+    await require('tar').c({
+      cwd: path.join(npm.prefix, source),
+      prefix: 'package',
+      gzip: true,
+      file: path.join(npm.prefix, `${source}.tgz`),
+    }, ['package.json', 'binding.gyp', 'contents.txt'])
+  }
+  await npm.exec('install', ['./old.tgz'])
+  const files = [
+    'package.json',
+    'package-lock.json',
+    'node_modules/native-pkg/package.json',
+    'node_modules/native-pkg/contents.txt',
+  ]
+  const before = files.map(file => fs.readFileSync(path.join(npm.prefix, file), 'utf8'))
+
+  await t.rejects(npm.exec('install', ['./incoming.tgz']), {
+    code: 'ESTRICTALLOWSCRIPTS',
+    message: /postinstall: echo incoming/,
+  })
+  t.strictSame(
+    files.map(file => fs.readFileSync(path.join(npm.prefix, file), 'utf8')),
+    before,
+    'strict rejection leaves installed contents and project manifests unchanged'
+  )
+})
+
 t.test('exec commands', async t => {
   await t.test('with args does not run lifecycle scripts', async t => {
     const { npm, registry } = await loadMockNpm(t, {

test/lib/utils/strict-allow-scripts-preflight.js

@@ -164,6 +164,27 @@ t.test('uses actualTree when idealTreeOpts is not provided (rebuild path)', asyn
   )
 })
 
+t.test('only rebuild preflight can recover a stale script flag from installed metadata', async t => {
+  const path = t.testdir({
+    'package.json': JSON.stringify({ name: 'pkg', version: '1.0.0', gypfile: false }),
+  })
+  const pending = tree([{
+    ...node({ scripts: {} }),
+    path,
+    hasInstallScript: true,
+  }])
+  const npm = { flatOptions: { strictAllowScripts: true } }
+  await t.rejects(preflight({
+    arb: makeArb({ ideal: pending }),
+    npm,
+    idealTreeOpts: {},
+  }), { code: 'ESTRICTALLOWSCRIPTS' }, 'incoming metadata remains unreviewed')
+  await t.resolves(preflight({
+    arb: makeArb({ actual: pending }),
+    npm,
+  }), 'rebuild can inspect the installed package')
+})
+
 t.test('error message includes script bodies', async t => {
   const arb = makeArb({
     ideal: tree([node({ name: 'canvas', version: '2.11.0', scripts: { install: 'node-gyp rebuild' } })]),