From 302a2f7d2cd68c0e7e2404418cffb4a7e406597a Mon Sep 17 00:00:00 2001 From: "C. Spencer Beggs" Date: Tue, 16 Jun 2026 09:49:00 -0400 Subject: [PATCH] fix(config/reader): don't warn when packageManager and devEngines.packageManager match (#12287) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When `package.json` sets both `packageManager` and `devEngines.packageManager` to the same pnpm version with the same integrity hash pnpm prints a spurious warning on every command. For example, a `package.json` file that looks like: ```json { "packageManager": "pnpm@11.5.1+sha512.93f7b57422ea7068257235b4c16eb60762eb68e1dc23723199cc739043ea9be2c4143274a399d8c6defa2b1176226d9ca1c4b63482d6200c1a8fbaa78c1d1485", "devEngines": { "packageManager": { "name": "pnpm", "version": "11.5.1+sha512.93f7b57422ea7068257235b4c16eb60762eb68e1dc23723199cc739043ea9be2c4143274a399d8c6defa2b1176226d9ca1c4b63482d6200c1a8fbaa78c1d1485", "onFail": "ignore" }, "runtime": [ { "name": "node", "version": "26.3.0", "onFail": "ignore" } ] } } ``` Issues a warning on every `pnpm` command: > Cannot use both "packageManager" and "devEngines.packageManager" in package.json. "packageManager" will be ignored. ## Root cause `getWantedPackageManager` compares the two fields to decide whether to warn, but the two sides were normalized differently: - `parsePackageManager` **strips the integrity hash** from the legacy `packageManager` field → `11.5.1` - the `devEngines.packageManager` version was compared **with its hash intact** → `11.5.1+sha512.93f7b57…` So, `"11.5.1" !== "11.5.1+sha512…"` was always true and the warning fired, even for identical specs. An earlier fix in #11307 only suppressed the warning when *neither* side carried a hash. ## Fix `parsePackageManager` now also returns the hash (via a shared `splitPackageManagerVersion`), and `getPackageManagerConflictWarning` compares the fields structurally. The warning is suppressed **only when the two specifiers are identical** (name + version + hash, both-absent counts as equal): | name | version | hash | result | |------|---------|------|--------| | same | same | both absent, or both present & equal | ✅ no warning | | same | same | present on **one side only** | ⚠️ generic "Cannot use both…" | | same | same | both present & **differ** | ⚠️ "…contradictory integrity hashes" | | same | **differ** | — | ⚠️ "…different versions of pnpm" | | **differ** | — | — | ⚠️ "…different package managers" | A hash on only one side is still a divergence — dropping the ignored `packageManager` field would lose that hash — so it warns with the original generic message. Two contradictory hashes for one version (a likely wrong-hash mistake) get a dedicated message. The generic single message is otherwise replaced by one tailored to each conflict, each ending with `"packageManager" will be ignored`. Closes #12028. --------- Signed-off-by: C. Spencer Beggs Signed-off-by: Zoltan Kochan Co-authored-by: Zoltan Kochan --- .../devengines-package-manager-same-hash.md | 8 ++ config/reader/src/index.ts | 100 +++++++++++++++-- config/reader/test/index.ts | 105 +++++++++++++++++- pnpm/test/packageManagerCheck.test.ts | 63 ++++++++++- 4 files changed, 261 insertions(+), 15 deletions(-) create mode 100644 .changeset/devengines-package-manager-same-hash.md diff --git a/.changeset/devengines-package-manager-same-hash.md b/.changeset/devengines-package-manager-same-hash.md new file mode 100644 index 0000000000..570efcceb6 --- /dev/null +++ b/.changeset/devengines-package-manager-same-hash.md @@ -0,0 +1,8 @@ +--- +"@pnpm/config.reader": patch +"pnpm": patch +--- + +No longer warn about using both `packageManager` and `devEngines.packageManager` when the two fields pin the same package manager at the same version with the same integrity hash (e.g. both `pnpm@11.5.1+sha512.…`). Previously the hash was stripped from the legacy `packageManager` field but not from `devEngines.packageManager`, so even identical specifications looked like a mismatch [#12028](https://github.com/pnpm/pnpm/issues/12028). + +The warning still fires on any genuine divergence, and several cases now state the specific reason instead of a single generic message: a different package manager, a different version, or contradictory integrity hashes for the same version. diff --git a/config/reader/src/index.ts b/config/reader/src/index.ts index 1e86ba72ef..571f999284 100644 --- a/config/reader/src/index.ts +++ b/config/reader/src/index.ts @@ -1,6 +1,7 @@ import fs from 'node:fs' import os from 'node:os' import path from 'node:path' +import { stripVTControlCharacters } from 'node:util' import { getCatalogsFromWorkspaceManifest } from '@pnpm/catalogs.config' import { createMatcher } from '@pnpm/config.matcher' @@ -793,8 +794,12 @@ function getWantedPackageManager (manifest: ProjectManifest): { pm?: WantedPacka } if (manifest.packageManager) { const legacyPm = parsePackageManager(manifest.packageManager) - if (legacyPm.name !== pmFromDevEngines.name || legacyPm.version !== pmFromDevEngines.version) { - warnings.push('Cannot use both "packageManager" and "devEngines.packageManager" in package.json. "packageManager" will be ignored') + const conflictWarning = getPackageManagerConflictWarning(legacyPm, { + name: pmFromDevEngines.name, + ...splitPackageManagerVersion(pmFromDevEngines.version), + }) + if (conflictWarning) { + warnings.push(conflictWarning) } } return { pm: { ...pmFromDevEngines, fromDevEngines: true }, warnings } @@ -848,17 +853,90 @@ function getIgnoredPnpmFieldKeys (manifest: ProjectManifest): string[] { return Object.keys(legacyField as Record).filter(k => MIGRATED_PNPM_FIELD_KEYS.has(k)) } -export function parsePackageManager (packageManager: string): { name: string, version: string | undefined } { - if (!packageManager.includes('@')) return { name: packageManager, version: undefined } - const [name, pmReference] = packageManager.split('@') +export interface ParsedPackageManager { + name: string + version: string | undefined + hash: string | undefined +} + +export function parsePackageManager (packageManager: string): ParsedPackageManager { + // Split on the `@` that separates the name from the reference. A leading `@` + // belongs to a scoped name (e.g. `@scope/pm@1.2.3`), so skip it; otherwise + // the first `@` is the separator. The first `@` (not the last) is used so a + // reference that is a URL containing `@` (e.g. credentials) stays intact. + const separatorIndex = packageManager.startsWith('@') + ? packageManager.indexOf('@', 1) + : packageManager.indexOf('@') + if (separatorIndex === -1) return { name: packageManager, version: undefined, hash: undefined } + const name = packageManager.slice(0, separatorIndex) + const pmReference = packageManager.slice(separatorIndex + 1) // pmReference is semantic versioning, not URL - if (pmReference.includes(':')) return { name, version: undefined } - // Remove the integrity hash. Ex: "pnpm@9.5.0+sha512.140036830124618d624a2187b50d04289d5a087f326c9edfc0ccd733d76c4f52c3a313d4fc148794a2a9d81553016004e6742e8cf850670268a7387fc220c903" - const [version] = pmReference.split('+') - return { - name, - version, + if (pmReference.includes(':')) return { name, version: undefined, hash: undefined } + return { name, ...splitPackageManagerVersion(pmReference) } +} + +/** + * Splits a package manager version reference into its semver part and the + * integrity hash carried as semver build metadata, e.g. + * "9.5.0+sha512.140036830124618d624a2187b50d04289d5a087f326c9edfc0ccd733d76c4f52c3a313d4fc148794a2a9d81553016004e6742e8cf850670268a7387fc220c903" + * becomes `{ version: "9.5.0", hash: "sha512.14003…" }`. A reference without a + * hash yields an undefined hash; an undefined reference yields both undefined. + */ +function splitPackageManagerVersion (reference: string | undefined): { version: string | undefined, hash: string | undefined } { + if (reference == null) return { version: undefined, hash: undefined } + // Split on the first `+` only. The integrity hash is semver build metadata — + // everything after that `+` — and must be preserved whole, so a reference is + // never truncated at a later `+`. + const hashIndex = reference.indexOf('+') + if (hashIndex === -1) return { version: reference, hash: undefined } + return { version: reference.slice(0, hashIndex), hash: reference.slice(hashIndex + 1) } +} + +/** + * Describes how the legacy `packageManager` field disagrees with + * `devEngines.packageManager`, or returns undefined when the two specifiers are + * identical (so keeping both fields in sync produces no warning). Any + * divergence warns — including an integrity hash (semver build metadata) on + * only one side, since dropping the ignored `packageManager` field would lose + * it. In every conflict `devEngines.packageManager` wins and `packageManager` + * is ignored. + */ +function getPackageManagerConflictWarning (legacy: ParsedPackageManager, devEngines: ParsedPackageManager): string | undefined { + const ignoredSuffix = '. "packageManager" will be ignored' + const genericWarning = `Cannot use both "packageManager" and "devEngines.packageManager" in package.json${ignoredSuffix}` + if (legacy.name !== devEngines.name) { + return `"packageManager" (${sanitizeManifestValue(legacy.name)}) and "devEngines.packageManager" (${sanitizeManifestValue(devEngines.name)}) specify different package managers in package.json${ignoredSuffix}` } + if (legacy.version !== devEngines.version) { + // "different versions" only makes sense when both sides are concrete + // versions. If one side has no semver version — e.g. the legacy field is a + // URL or a bare name — fall back to the generic notice rather than claiming + // a version mismatch. + if (legacy.version == null || devEngines.version == null) return genericWarning + return `"packageManager" and "devEngines.packageManager" specify different versions of ${sanitizeManifestValue(legacy.name)} in package.json${ignoredSuffix}` + } + if (legacy.hash !== devEngines.hash) { + // Same name and version, but the integrity hashes differ. Two distinct + // hashes for one version is a likely wrong-hash mistake, so call it out + // specifically; a hash on only one side is a softer mismatch (the version + // still agrees) and gets the generic notice. + if (legacy.hash != null && devEngines.hash != null && legacy.version != null) { + return `"packageManager" and "devEngines.packageManager" specify ${sanitizeManifestValue(legacy.name)}@${sanitizeManifestValue(legacy.version)} with different integrity hashes in package.json${ignoredSuffix}` + } + return genericWarning + } + return undefined +} + +/** + * Renders a package.json-controlled value safe to embed in a warning printed to + * the terminal. Strips ANSI escape sequences and replaces remaining control + * characters (including newlines) with spaces so a malicious manifest cannot + * forge or rewrite terminal/CI log output. + */ +function sanitizeManifestValue (value: string): string { + // eslint-disable-next-line no-control-regex + return stripVTControlCharacters(value).replace(/[\u0000-\u001f\u007f]/g, ' ') } /** diff --git a/config/reader/test/index.ts b/config/reader/test/index.ts index 75f804d656..272c2fd457 100644 --- a/config/reader/test/index.ts +++ b/config/reader/test/index.ts @@ -13,7 +13,7 @@ import { writeYamlFileSync } from 'write-yaml-file' jest.unstable_mockModule('@pnpm/network.git-utils', () => ({ getCurrentBranch: jest.fn() })) -const { getConfig } = await import('@pnpm/config.reader') +const { getConfig, parsePackageManager } = await import('@pnpm/config.reader') const { getCurrentBranch } = await import('@pnpm/network.git-utils') // To override any local settings, @@ -207,6 +207,109 @@ test('devEngines.packageManager with explicit onFail is respected (regression gu expect(context.wantedPackageManager?.onFail).toBe('error') }) +describe('"packageManager" / "devEngines.packageManager" conflict warning', () => { + const HASH_A = 'sha512.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' + const HASH_B = 'sha512.bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb' + const IGNORED = '. "packageManager" will be ignored' + + async function warningsFor (packageManager: string, devEnginesVersion: string): Promise { + prepare({ + packageManager, + devEngines: { + packageManager: { name: 'pnpm', version: devEnginesVersion, onFail: 'ignore' }, + }, + }) + const { warnings } = await getConfig({ + cliOptions: {}, + packageManager: { name: 'pnpm', version: '1.2.3' }, + }) + return warnings + } + + test.each([ + { caseName: 'same version without a hash', packageManager: 'pnpm@1.2.3', devEnginesVersion: '1.2.3' }, + { caseName: 'same version with the same hash', packageManager: `pnpm@1.2.3+${HASH_A}`, devEnginesVersion: `1.2.3+${HASH_A}` }, + ])('does not warn when the specifiers are identical: $caseName', async ({ packageManager, devEnginesVersion }) => { + const warnings = await warningsFor(packageManager, devEnginesVersion) + expect(warnings.some(w => w.includes(IGNORED))).toBe(false) + }) + + test.each([ + { caseName: 'hash on the legacy field only', packageManager: `pnpm@1.2.3+${HASH_A}`, devEnginesVersion: '1.2.3' }, + { caseName: 'hash on the devEngines field only', packageManager: 'pnpm@1.2.3', devEnginesVersion: `1.2.3+${HASH_A}` }, + ])('warns generically when the integrity hash is present on only one field: $caseName', async ({ packageManager, devEnginesVersion }) => { + const warnings = await warningsFor(packageManager, devEnginesVersion) + expect(warnings).toContain(`Cannot use both "packageManager" and "devEngines.packageManager" in package.json${IGNORED}`) + }) + + test('warns about contradictory integrity hashes for the same version', async () => { + const warnings = await warningsFor(`pnpm@1.2.3+${HASH_A}`, `1.2.3+${HASH_B}`) + expect(warnings).toContain(`"packageManager" and "devEngines.packageManager" specify pnpm@1.2.3 with different integrity hashes in package.json${IGNORED}`) + }) + + test.each([ + { caseName: 'the legacy field is a URL reference', packageManager: 'pnpm@https://github.com/pnpm/pnpm' }, + { caseName: 'the legacy field is a bare name with no version', packageManager: 'pnpm' }, + ])('warns generically rather than claiming a version mismatch when one side is not a concrete version: $caseName', async ({ packageManager }) => { + const warnings = await warningsFor(packageManager, '1.2.3') + expect(warnings).toContain(`Cannot use both "packageManager" and "devEngines.packageManager" in package.json${IGNORED}`) + expect(warnings.some(w => w.includes('different versions'))).toBe(false) + }) + + test.each([ + { caseName: 'different exact versions', devEnginesVersion: '1.2.4' }, + { caseName: 'exact version versus a range', devEnginesVersion: '>=1.0.0' }, + ])('warns about a version mismatch: $caseName', async ({ devEnginesVersion }) => { + const warnings = await warningsFor('pnpm@1.2.3', devEnginesVersion) + expect(warnings).toContain(`"packageManager" and "devEngines.packageManager" specify different versions of pnpm in package.json${IGNORED}`) + }) + + test('warns when the fields name different package managers', async () => { + prepare({ + packageManager: 'yarn@1.2.3', + devEngines: { + packageManager: { name: 'pnpm', version: '1.2.3', onFail: 'ignore' }, + }, + }) + const { warnings } = await getConfig({ + cliOptions: {}, + packageManager: { name: 'pnpm', version: '1.2.3' }, + }) + expect(warnings).toContain(`"packageManager" (yarn) and "devEngines.packageManager" (pnpm) specify different package managers in package.json${IGNORED}`) + }) + + test('strips control characters from package.json values embedded in the warning', async () => { + prepare({ + packageManager: 'ev\u001b[31mi\nl@1.2.3', + devEngines: { + packageManager: { name: 'pnpm', version: '1.2.3', onFail: 'ignore' }, + }, + }) + const { warnings } = await getConfig({ + cliOptions: {}, + packageManager: { name: 'pnpm', version: '1.2.3' }, + }) + const warning = warnings.find(w => w.includes('different package managers')) + expect(warning).toBeDefined() + // eslint-disable-next-line no-control-regex + expect(warning).not.toMatch(/[\u0000-\u001f\u007f]/) + expect(warning).toContain('"packageManager" (evi l)') + }) +}) + +describe('parsePackageManager', () => { + test.each([ + { input: 'pnpm@9.5.0', expected: { name: 'pnpm', version: '9.5.0', hash: undefined } }, + { input: 'pnpm@9.5.0+sha512.abc123', expected: { name: 'pnpm', version: '9.5.0', hash: 'sha512.abc123' } }, + { input: 'pnpm@9.5.0+a+b', expected: { name: 'pnpm', version: '9.5.0', hash: 'a+b' } }, + { input: 'pnpm', expected: { name: 'pnpm', version: undefined, hash: undefined } }, + { input: '@scope/pm@1.2.3', expected: { name: '@scope/pm', version: '1.2.3', hash: undefined } }, + { input: 'pnpm@https://github.com/pnpm/pnpm', expected: { name: 'pnpm', version: undefined, hash: undefined } }, + ])('parses $input', ({ input, expected }) => { + expect(parsePackageManager(input)).toEqual(expected) + }) +}) + test('throw error if --link-workspace-packages is used with --global', async () => { await expect(getConfig({ cliOptions: { diff --git a/pnpm/test/packageManagerCheck.test.ts b/pnpm/test/packageManagerCheck.test.ts index 7364a3c69a..e31bd2aa30 100644 --- a/pnpm/test/packageManagerCheck.test.ts +++ b/pnpm/test/packageManagerCheck.test.ts @@ -506,7 +506,62 @@ test('no warning when packageManager and devEngines.packageManager specify the s const { stderr } = execPnpmSync(['install']) - expect(stderr.toString()).not.toContain('Cannot use both') + expect(stderr.toString()).not.toContain('"packageManager" will be ignored') +}) + +test('no warning when packageManager and devEngines.packageManager specify the same version with the same integrity hash', async () => { + const hash = 'sha512.93f7b57422ea7068257235b4c16eb60762eb68e1dc23723199cc739043ea9be2c4143274a399d8c6defa2b1176226d9ca1c4b63482d6200c1a8fbaa78c1d1485' + prepare({ + packageManager: `pnpm@1.2.3+${hash}`, + devEngines: { + packageManager: { + name: 'pnpm', + version: `1.2.3+${hash}`, + onFail: 'ignore', + }, + }, + }) + + const { stderr } = execPnpmSync(['install']) + + expect(stderr.toString()).not.toContain('"packageManager" will be ignored') +}) + +test('warns when packageManager and devEngines.packageManager pin the same version but different integrity hashes', async () => { + prepare({ + packageManager: 'pnpm@1.2.3+sha512.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', + devEngines: { + packageManager: { + name: 'pnpm', + version: '1.2.3+sha512.bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb', + onFail: 'ignore', + }, + }, + }) + + const { stderr } = execPnpmSync(['install']) + + expect(stderr.toString()).toContain('different integrity hashes') + expect(stderr.toString()).toContain('"packageManager" will be ignored') +}) + +test('warns when the integrity hash is present on packageManager but not devEngines.packageManager', async () => { + const hash = 'sha512.93f7b57422ea7068257235b4c16eb60762eb68e1dc23723199cc739043ea9be2c4143274a399d8c6defa2b1176226d9ca1c4b63482d6200c1a8fbaa78c1d1485' + prepare({ + packageManager: `pnpm@1.2.3+${hash}`, + devEngines: { + packageManager: { + name: 'pnpm', + version: '1.2.3', + onFail: 'ignore', + }, + }, + }) + + const { stderr } = execPnpmSync(['install']) + + expect(stderr.toString()).toContain('Cannot use both "packageManager" and "devEngines.packageManager"') + expect(stderr.toString()).toContain('"packageManager" will be ignored') }) test('warns when packageManager specifies a different package manager from devEngines.packageManager', async () => { @@ -523,7 +578,8 @@ test('warns when packageManager specifies a different package manager from devEn const { stderr } = execPnpmSync(['install']) - expect(stderr.toString()).toContain('Cannot use both "packageManager" and "devEngines.packageManager"') + expect(stderr.toString()).toContain('specify different package managers') + expect(stderr.toString()).toContain('"packageManager" will be ignored') }) test('warns when packageManager version does not match the devEngines.packageManager version string exactly', async () => { @@ -540,7 +596,8 @@ test('warns when packageManager version does not match the devEngines.packageMan const { stderr } = execPnpmSync(['install']) - expect(stderr.toString()).toContain('Cannot use both "packageManager" and "devEngines.packageManager"') + expect(stderr.toString()).toContain('specify different versions of pnpm') + expect(stderr.toString()).toContain('"packageManager" will be ignored') }) test('pmOnFail=ignore via env var bypasses the devEngines.packageManager check', async () => {