From a4a8231243524d7a21592a54c976a478b6605cb7 Mon Sep 17 00:00:00 2001 From: ychampion Date: Wed, 8 Jul 2026 15:06:36 +0000 Subject: [PATCH 1/3] fix(plugin-npm): limit implicit node-gyp checks to build scripts --- .../plugin-npm-node-gyp-build-scripts.yml | 8 +++ .../plugin-npm/sources/NpmSemverResolver.ts | 9 ++- .../tests/NpmSemverResolver.test.ts | 66 ++++++++++++++++++- 3 files changed, 78 insertions(+), 5 deletions(-) create mode 100644 .yarn/versions/plugin-npm-node-gyp-build-scripts.yml diff --git a/.yarn/versions/plugin-npm-node-gyp-build-scripts.yml b/.yarn/versions/plugin-npm-node-gyp-build-scripts.yml new file mode 100644 index 00000000000..54e72a830b1 --- /dev/null +++ b/.yarn/versions/plugin-npm-node-gyp-build-scripts.yml @@ -0,0 +1,8 @@ +releases: + "@yarnpkg/plugin-npm": patch + +declined: + - "@yarnpkg/cli" + - "@yarnpkg/core" + - "@yarnpkg/plugin-compat" + - "@yarnpkg/plugin-npm-cli" diff --git a/packages/plugin-npm/sources/NpmSemverResolver.ts b/packages/plugin-npm/sources/NpmSemverResolver.ts index 6b68fb35d17..5d40623b400 100644 --- a/packages/plugin-npm/sources/NpmSemverResolver.ts +++ b/packages/plugin-npm/sources/NpmSemverResolver.ts @@ -11,6 +11,7 @@ import * as npmHttpUtils const NODE_GYP_IDENT = structUtils.makeIdent(null, `node-gyp`); const NODE_GYP_MATCH = /\b(node-gyp|prebuild-install)\b/; +const NODE_GYP_BUILD_SCRIPTS = [`preinstall`, `install`, `postinstall`]; /** * Returns the versions from `versions` that satisfy the given range. @@ -177,10 +178,12 @@ export class NpmSemverResolver implements Resolver { // Manually add node-gyp dependency if there is a script using it and not already set // This is because the npm registry will automatically add a `node-gyp rebuild` install script // in the metadata if there is not already an install script and a binding.gyp file exists. - // Also, node-gyp is not always set as a dependency in packages, so it will also be added if used in scripts. + // Also, node-gyp is not always set as a dependency in packages, so it will also be added if used in build scripts. if (!manifest.dependencies.has(NODE_GYP_IDENT.identHash) && !manifest.peerDependencies.has(NODE_GYP_IDENT.identHash)) { - for (const value of manifest.scripts.values()) { - if (value.match(NODE_GYP_MATCH)) { + for (const scriptName of NODE_GYP_BUILD_SCRIPTS) { + const value = manifest.scripts.get(scriptName); + + if (typeof value !== `undefined` && NODE_GYP_MATCH.test(value)) { manifest.dependencies.set(NODE_GYP_IDENT.identHash, structUtils.makeDescriptor(NODE_GYP_IDENT, `latest`)); break; } diff --git a/packages/plugin-npm/tests/NpmSemverResolver.test.ts b/packages/plugin-npm/tests/NpmSemverResolver.test.ts index 6e39038676d..35d98d6dd16 100644 --- a/packages/plugin-npm/tests/NpmSemverResolver.test.ts +++ b/packages/plugin-npm/tests/NpmSemverResolver.test.ts @@ -1,6 +1,15 @@ -import {semverUtils, structUtils} from '@yarnpkg/core'; +jest.mock(`../sources/npmHttpUtils`, () => ({ + getPackageMetadata: jest.fn(), +})); -import {NpmSemverResolver, selectMatchingVersions} from '../sources/NpmSemverResolver'; +import {semverUtils, structUtils} from '@yarnpkg/core'; + +const {NpmSemverResolver, selectMatchingVersions}: typeof import('../sources/NpmSemverResolver') = require(`../sources/NpmSemverResolver`); +const npmHttpUtils: typeof import('../sources/npmHttpUtils') = require(`../sources/npmHttpUtils`); + +afterEach(() => { + jest.clearAllMocks(); +}); const select = (rangeString: string, versions: Array) => { const range = semverUtils.validRange(rangeString); @@ -48,4 +57,57 @@ describe(`NpmSemverResolver`, () => { expect(select(`*`, [])).toEqual([]); }); }); + + describe(`resolve`, () => { + const ident = structUtils.makeIdent(null, `native-package`); + const nodeGypIdent = structUtils.makeIdent(null, `node-gyp`); + + const makeResolveOptions = () => ({ + project: { + configuration: { + normalizeDependencyMap: (dependencies: Map) => dependencies, + }, + }, + } as any); + + const mockPackageMetadata = (scripts: Record) => { + const getPackageMetadata = npmHttpUtils.getPackageMetadata as jest.MockedFunction; + + getPackageMetadata.mockResolvedValue({ + versions: { + [`1.0.0`]: { + name: structUtils.stringifyIdent(ident), + version: `1.0.0`, + scripts, + }, + }, + } as any); + }; + + it(`shouldn't inject node-gyp when only a non-build script uses it`, async () => { + mockPackageMetadata({ + test: `node-gyp rebuild`, + }); + + const resolver = new NpmSemverResolver(); + const locator = structUtils.makeLocator(ident, `npm:1.0.0`); + + const pkg = await resolver.resolve(locator, makeResolveOptions()); + + expect(pkg.dependencies.has(nodeGypIdent.identHash)).toEqual(false); + }); + + it(`should inject node-gyp when an install script uses it`, async () => { + mockPackageMetadata({ + install: `node-gyp rebuild`, + }); + + const resolver = new NpmSemverResolver(); + const locator = structUtils.makeLocator(ident, `npm:1.0.0`); + + const pkg = await resolver.resolve(locator, makeResolveOptions()); + + expect(pkg.dependencies.has(nodeGypIdent.identHash)).toEqual(true); + }); + }); }); From 0ea9948675905daaf032a5c1715c629b69fe8131 Mon Sep 17 00:00:00 2001 From: ychampion Date: Wed, 8 Jul 2026 15:16:34 +0000 Subject: [PATCH 2/3] chore: update install artifacts for node-gyp resolution --- .pnp.cjs | 3 +-- yarn.lock | 2 -- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/.pnp.cjs b/.pnp.cjs index 0516a8e5d31..b73d7550cb7 100644 --- a/.pnp.cjs +++ b/.pnp.cjs @@ -31018,8 +31018,7 @@ const RAW_RUNTIME_STATE = ["npm:4.3.0", {\ "packageLocation": "./.yarn/unplugged/node-addon-api-npm-4.3.0-a07a1232df/node_modules/node-addon-api/",\ "packageDependencies": [\ - ["node-addon-api", "npm:4.3.0"],\ - ["node-gyp", "npm:12.1.0"]\ + ["node-addon-api", "npm:4.3.0"]\ ],\ "linkType": "HARD"\ }]\ diff --git a/yarn.lock b/yarn.lock index 6f067370dcf..a0d9fba1490 100644 --- a/yarn.lock +++ b/yarn.lock @@ -15929,8 +15929,6 @@ __metadata: "node-addon-api@npm:^4.3.0": version: 4.3.0 resolution: "node-addon-api@npm:4.3.0" - dependencies: - node-gyp: "npm:latest" checksum: 10/d3b38d16cb9ad0714d965331d0e38cef1c27750c2c3343cd3464a9ed8158501a2910ccbf2fd9fdc476e806a19dbc9e0524ff9d66a7c779d42a9752a63ba30b80 languageName: node linkType: hard From b0613e564a2eac692959cd7900865d7912409da1 Mon Sep 17 00:00:00 2001 From: ychampion Date: Wed, 8 Jul 2026 16:53:27 +0000 Subject: [PATCH 3/3] Preserve delegated native build detection Constraint: Maintainer pointed out install scripts can delegate to other package scripts. Rejected: Only scan preinstall/install/postinstall bodies | Misses install scripts that call build scripts using node-gyp. Confidence: high Scope-risk: narrow Directive: Keep test-only node-gyp references ignored unless an install lifecycle script makes package scripts reachable during install. Tested: corepack yarn jest packages/plugin-npm/tests/NpmSemverResolver.test.ts --runInBand; corepack yarn jest packages/plugin-npm/tests --runInBand; corepack yarn test:lint; corepack yarn constraints; corepack yarn version check; corepack yarn dedupe --check; corepack yarn typecheck:all; YARN_ENABLE_HARDENED_MODE=1 corepack yarn install --immutable --immutable-cache; git diff --check. Not-tested: Netlify docs build, already failing on upstream master for an unrelated schema example issue. --- .../plugin-npm/sources/NpmSemverResolver.ts | 17 ++++++++++------- .../plugin-npm/tests/NpmSemverResolver.test.ts | 14 ++++++++++++++ 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/packages/plugin-npm/sources/NpmSemverResolver.ts b/packages/plugin-npm/sources/NpmSemverResolver.ts index 5d40623b400..0a0670649b8 100644 --- a/packages/plugin-npm/sources/NpmSemverResolver.ts +++ b/packages/plugin-npm/sources/NpmSemverResolver.ts @@ -11,7 +11,7 @@ import * as npmHttpUtils const NODE_GYP_IDENT = structUtils.makeIdent(null, `node-gyp`); const NODE_GYP_MATCH = /\b(node-gyp|prebuild-install)\b/; -const NODE_GYP_BUILD_SCRIPTS = [`preinstall`, `install`, `postinstall`]; +const NODE_GYP_INSTALL_SCRIPTS = [`preinstall`, `install`, `postinstall`]; /** * Returns the versions from `versions` that satisfy the given range. @@ -178,12 +178,15 @@ export class NpmSemverResolver implements Resolver { // Manually add node-gyp dependency if there is a script using it and not already set // This is because the npm registry will automatically add a `node-gyp rebuild` install script // in the metadata if there is not already an install script and a binding.gyp file exists. - // Also, node-gyp is not always set as a dependency in packages, so it will also be added if used in build scripts. - if (!manifest.dependencies.has(NODE_GYP_IDENT.identHash) && !manifest.peerDependencies.has(NODE_GYP_IDENT.identHash)) { - for (const scriptName of NODE_GYP_BUILD_SCRIPTS) { - const value = manifest.scripts.get(scriptName); - - if (typeof value !== `undefined` && NODE_GYP_MATCH.test(value)) { + // Also, node-gyp is not always set as a dependency in packages, so it will also be added + // if used in package scripts that may be reached from an install lifecycle script. + if ( + !manifest.dependencies.has(NODE_GYP_IDENT.identHash) && + !manifest.peerDependencies.has(NODE_GYP_IDENT.identHash) && + NODE_GYP_INSTALL_SCRIPTS.some(scriptName => manifest.scripts.has(scriptName)) + ) { + for (const value of manifest.scripts.values()) { + if (NODE_GYP_MATCH.test(value)) { manifest.dependencies.set(NODE_GYP_IDENT.identHash, structUtils.makeDescriptor(NODE_GYP_IDENT, `latest`)); break; } diff --git a/packages/plugin-npm/tests/NpmSemverResolver.test.ts b/packages/plugin-npm/tests/NpmSemverResolver.test.ts index 35d98d6dd16..46a2042d023 100644 --- a/packages/plugin-npm/tests/NpmSemverResolver.test.ts +++ b/packages/plugin-npm/tests/NpmSemverResolver.test.ts @@ -109,5 +109,19 @@ describe(`NpmSemverResolver`, () => { expect(pkg.dependencies.has(nodeGypIdent.identHash)).toEqual(true); }); + + it(`should inject node-gyp when an install script delegates to another script using it`, async () => { + mockPackageMetadata({ + build: `node-gyp rebuild`, + install: `yarn build`, + }); + + const resolver = new NpmSemverResolver(); + const locator = structUtils.makeLocator(ident, `npm:1.0.0`); + + const pkg = await resolver.resolve(locator, makeResolveOptions()); + + expect(pkg.dependencies.has(nodeGypIdent.identHash)).toEqual(true); + }); }); });