From 16a42338d657f77bdd9c3913ea3d9f29c5220c58 Mon Sep 17 00:00:00 2001 From: Ranjan Singh Date: Wed, 5 Aug 2026 21:35:23 +0530 Subject: [PATCH] feat: trust signing keys published by a custom npm registry When `COREPACK_NPM_REGISTRY` points at a registry that re-signs the packages it serves, the signature is made with that registry's own key, which is not part of the npm keys Corepack bundles. Verification then fails with "The package was not signed by any trusted keys", and there is no way to work around it in environments where `COREPACK_INTEGRITY_KEYS` cannot be set. Fall back to the keys the configured registry publishes at `/-/npm/v1/keys` when the bundled keys don't already cover the signature, and trust them in addition to the bundled ones so that packages which keep their original npm signature continue to verify. An explicit `COREPACK_INTEGRITY_KEYS` still takes precedence, and the extra request only happens when verification would otherwise fail, so setups that work today are unaffected. When the keys cannot be fetched or none of them are usable, Corepack keeps verifying against the bundled keys rather than skipping the check. Refs: https://github.com/nodejs/corepack/issues/884 --- README.md | 6 +- sources/corepackUtils.ts | 2 +- sources/npmRegistryUtils.ts | 125 +++++++++++++++++++---- tests/npmRegistryUtils.test.ts | 179 ++++++++++++++++++++++++++++++++- 4 files changed, 287 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index dd32b4ad0..47d7f90dc 100644 --- a/README.md +++ b/README.md @@ -354,7 +354,11 @@ same major line. Should you need to upgrade to a new major, use an explicit - `COREPACK_INTEGRITY_KEYS` can be set to an empty string or `0` to instruct Corepack to skip integrity checks, or to a JSON string containing - custom keys. + custom keys. When it is not set and `COREPACK_NPM_REGISTRY` points to a + registry other than the default one, Corepack additionally trusts the signing + keys that registry publishes at `/-/npm/v1/keys`, so registries which re-sign + the packages they serve can still be verified. Those keys are only requested + when the bundled npm keys don't already cover the signature. ## Troubleshooting diff --git a/sources/corepackUtils.ts b/sources/corepackUtils.ts index c02571ab6..10fef4a9c 100644 --- a/sources/corepackUtils.ts +++ b/sources/corepackUtils.ts @@ -300,7 +300,7 @@ export async function installVersion(installTarget: string, locator: Locator, {s if (signatures! == null || integrity! == null) ({signatures, integrity} = (await npmRegistryUtils.fetchTarballURLAndSignature(registry.package, version))); - npmRegistryUtils.verifySignature({signatures, integrity, packageName: registry.package, version}); + await npmRegistryUtils.verifySignature({signatures, integrity, packageName: registry.package, version}); // @ts-expect-error ignore readonly build[1] = Buffer.from(integrity.slice(`sha512-`.length), `base64`).toString(`hex`); } diff --git a/sources/npmRegistryUtils.ts b/sources/npmRegistryUtils.ts index a7d110e95..8a2f57cd1 100644 --- a/sources/npmRegistryUtils.ts +++ b/sources/npmRegistryUtils.ts @@ -4,6 +4,7 @@ import {createVerify} from 'crypto'; import defaultConfig from '../config.json'; import {shouldSkipIntegrityCheck} from './corepackUtils'; +import * as debugUtils from './debugUtils'; import * as httpUtils from './httpUtils'; // load abbreviated metadata as that's all we need for these calls @@ -13,12 +14,23 @@ export const DEFAULT_HEADERS: Record = { }; export const DEFAULT_NPM_REGISTRY_URL = `https://registry.npmjs.org`; -export async function fetchAsJson(packageName: string, version?: string) { - const npmRegistryUrl = process.env.COREPACK_NPM_REGISTRY || DEFAULT_NPM_REGISTRY_URL; +// Standard endpoint at which an npm registry publishes its signing keys. +// see: https://docs.npmjs.com/about-registry-signatures +const KEYS_ENDPOINT_PATH = `/-/npm/v1/keys`; - if (process.env.COREPACK_ENABLE_NETWORK === `0`) - throw new UsageError(`Network access disabled by the environment; can't reach npm repository ${npmRegistryUrl}`); +interface TrustedKey { + keyid: string; + key: string; +} + +// A configured registry may carry a trailing slash, which would turn the keys +// path into `//-/npm/v1/keys`; not every registry resolves that to the same +// route, so it gets trimmed before the path is appended. +function trimTrailingSlashes(url: string) { + return url.replace(/\/+$/, ``); +} +function getRegistryHeaders() { const headers = {...DEFAULT_HEADERS}; if (`COREPACK_NPM_TOKEN` in process.env) { @@ -29,10 +41,70 @@ export async function fetchAsJson(packageName: string, version?: string) { headers.authorization = `Basic ${encodedCreds}`; } - return httpUtils.fetchAsJson(`${npmRegistryUrl}/${packageName}${version ? `/${version}` : ``}`, {headers}); + return headers; } -export function verifySignature({signatures, integrity, packageName, version}: { +export async function fetchAsJson(packageName: string, version?: string) { + const npmRegistryUrl = process.env.COREPACK_NPM_REGISTRY || DEFAULT_NPM_REGISTRY_URL; + + if (process.env.COREPACK_ENABLE_NETWORK === `0`) + throw new UsageError(`Network access disabled by the environment; can't reach npm repository ${npmRegistryUrl}`); + + return httpUtils.fetchAsJson(`${npmRegistryUrl}/${packageName}${version ? `/${version}` : ``}`, {headers: getRegistryHeaders()}); +} + +const registryKeysCache = new Map | null>>(); + +async function fetchRegistryKeys(registryUrl: string): Promise | null> { + const url = `${registryUrl}${KEYS_ENDPOINT_PATH}`; + + try { + const data = await httpUtils.fetchAsJson(url, {headers: getRegistryHeaders()}); + + // Only keep well-formed entries; anything else is unusable for verification. + const keys = Array.isArray(data?.keys) ? + (data.keys as Array).filter((key): key is TrustedKey => + typeof (key as TrustedKey)?.keyid === `string` && typeof (key as TrustedKey)?.key === `string`) : + []; + + if (!keys.length) { + debugUtils.log(`No usable signing keys returned by ${url}`); + return null; + } + + return keys; + } catch (error) { + debugUtils.log(`Failed to fetch signing keys from ${url}: ${error}`); + return null; + } +} + +function getRegistryKeys(registryUrl: string) { + let keys = registryKeysCache.get(registryUrl); + if (keys == null) { + keys = fetchRegistryKeys(registryUrl); + registryKeysCache.set(registryUrl, keys); + } + + return keys; +} + +function findTrustedSignature(signatures: Array<{keyid: string, sig: string}>, trustedKeys: Array) { + let signature: typeof signatures[0] | undefined; + let key!: string; + for (const k of trustedKeys) { + signature = signatures.find(({keyid}) => keyid === k.keyid); + if (signature != null) { + key = k.key; + break; + } + } + if (signature?.sig == null) return null; + + return {signature, key}; +} + +export async function verifySignature({signatures, integrity, packageName, version}: { signatures: Array<{keyid: string, sig: string}>; integrity: string; packageName: string; @@ -41,25 +113,42 @@ export function verifySignature({signatures, integrity, packageName, version}: { if (!Array.isArray(signatures) || !signatures.length) throw new Error(`No compatible signature found in package metadata`); const {npm: trustedKeys} = process.env.COREPACK_INTEGRITY_KEYS ? - JSON.parse(process.env.COREPACK_INTEGRITY_KEYS) as typeof defaultConfig.keys : + JSON.parse(process.env.COREPACK_INTEGRITY_KEYS) as {npm: Array} : defaultConfig.keys; - let signature: typeof signatures[0] | undefined; - let key!: string; - for (const k of trustedKeys) { - signature = signatures.find(({keyid}) => keyid === k.keyid); - if (signature != null) { - key = k.key; - break; + let match = findTrustedSignature(signatures, trustedKeys); + + // The bundled keys only describe the public npm registry. When a custom + // registry is configured, the package it serves may legitimately be signed by + // that registry's own key (some registries re-sign what they serve), so fall + // back to the keys it publishes rather than rejecting the package outright. + // Those keys come from the very registry that serves the tarball we are about + // to execute, so consulting them doesn't widen the trust boundary. An explicit + // COREPACK_INTEGRITY_KEYS always wins, and the keys are only fetched when the + // bundled ones don't already cover the signature. + let registryKeys: Array | null = null; + const customRegistryUrl = process.env.COREPACK_NPM_REGISTRY ? + trimTrailingSlashes(process.env.COREPACK_NPM_REGISTRY) : + undefined; + if (match == null && !process.env.COREPACK_INTEGRITY_KEYS && customRegistryUrl && customRegistryUrl !== DEFAULT_NPM_REGISTRY_URL) { + registryKeys = await getRegistryKeys(customRegistryUrl); + if (registryKeys != null) { + match = findTrustedSignature(signatures, registryKeys); } } - if (signature?.sig == null) throw new UsageError(`The package was not signed by any trusted keys: ${JSON.stringify({signatures, trustedKeys}, undefined, 2)}`); + + if (match == null) { + throw new UsageError(`The package was not signed by any trusted keys: ${JSON.stringify({ + signatures, + trustedKeys: registryKeys == null ? trustedKeys : [...trustedKeys, ...registryKeys], + }, undefined, 2)}`); + } const verifier = createVerify(`SHA256`); verifier.end(`${packageName}@${version}:${integrity}`); const valid = verifier.verify( - `-----BEGIN PUBLIC KEY-----\n${key}\n-----END PUBLIC KEY-----`, - signature.sig, + `-----BEGIN PUBLIC KEY-----\n${match.key}\n-----END PUBLIC KEY-----`, + match.signature.sig, `base64`, ); if (!valid) { @@ -74,7 +163,7 @@ export async function fetchLatestStableVersion(packageName: string) { if (!shouldSkipIntegrityCheck()) { try { - verifySignature({ + await verifySignature({ packageName, version, integrity, signatures, }); diff --git a/tests/npmRegistryUtils.test.ts b/tests/npmRegistryUtils.test.ts index 728d6f977..4aac1bccc 100644 --- a/tests/npmRegistryUtils.test.ts +++ b/tests/npmRegistryUtils.test.ts @@ -1,9 +1,10 @@ -import {Buffer} from 'node:buffer'; -import process from 'node:process'; -import {describe, beforeEach, it, expect, vi} from 'vitest'; +import {Buffer} from 'node:buffer'; +import {createHash, createSign, generateKeyPairSync} from 'node:crypto'; +import process from 'node:process'; +import {describe, beforeEach, it, expect, vi} from 'vitest'; -import {fetchAsJson as httpFetchAsJson} from '../sources/httpUtils'; -import {DEFAULT_HEADERS, DEFAULT_NPM_REGISTRY_URL, fetchAsJson} from '../sources/npmRegistryUtils'; +import {fetchAsJson as httpFetchAsJson} from '../sources/httpUtils'; +import {DEFAULT_HEADERS, DEFAULT_NPM_REGISTRY_URL, fetchAsJson, verifySignature} from '../sources/npmRegistryUtils'; vi.mock(`../sources/httpUtils`); @@ -90,3 +91,171 @@ describe(`npm registry utils fetchAsJson`, () => { expect(httpFetchAsJson).lastCalledWith(`${DEFAULT_NPM_REGISTRY_URL}/package-name`, {headers: DEFAULT_HEADERS}); }); }); + +describe(`npm registry utils verifySignature`, () => { + const packageName = `package-name`; + const version = `1.0.0`; + const KEYS_PATH = `/-/npm/v1/keys`; + + // Fetched keys are cached per registry for the lifetime of the process, so + // every test needs its own registry URL to stay independent. + let registryCount = 0; + function uniqueRegistryUrl() { + return `https://registry-${++registryCount}.example.org`; + } + + function signPackage() { + const integrity = `sha512-${Buffer.from(`${packageName}@${version}`).toString(`base64`)}`; + const {privateKey, publicKey} = generateKeyPairSync(`ec`, { + namedCurve: `prime256v1`, + publicKeyEncoding: {type: `spki`, format: `pem`}, + privateKeyEncoding: {type: `pkcs8`, format: `pem`}, + }); + const keyid = `SHA256:${createHash(`SHA256`).end(publicKey).digest(`base64`)}`; + const sig = createSign(`SHA256`).end(`${packageName}@${version}:${integrity}`).sign(privateKey, `base64`); + + return { + integrity, + signatures: [{keyid, sig}], + publishedKey: { + expires: null, + keyid, + keytype: `ecdsa-sha2-nistp256`, + scheme: `ecdsa-sha2-nistp256`, + key: publicKey.split(`\n`).slice(1, -2).join(``), + }, + }; + } + + function mockKeysEndpoint(payload: any) { + vi.mocked(httpFetchAsJson).mockImplementation(async (input: string | URL) => { + if (`${input}`.endsWith(KEYS_PATH)) return payload; + throw new Error(`Unexpected request to ${input}`); + }); + } + + beforeEach(() => { + vi.resetAllMocks(); + }); + + it(`verifies a signature made with a key published by the custom registry`, async () => { + const {integrity, signatures, publishedKey} = signPackage(); + const registryUrl = uniqueRegistryUrl(); + process.env.COREPACK_NPM_REGISTRY = registryUrl; + mockKeysEndpoint({keys: [publishedKey]}); + + await expect(verifySignature({signatures, integrity, packageName, version})).resolves.toBeUndefined(); + + expect(httpFetchAsJson).lastCalledWith(`${registryUrl}${KEYS_PATH}`, {headers: DEFAULT_HEADERS}); + }); + + it(`does not double the slash when the configured registry ends with one`, async () => { + const {integrity, signatures, publishedKey} = signPackage(); + const registryUrl = uniqueRegistryUrl(); + process.env.COREPACK_NPM_REGISTRY = `${registryUrl}/`; + mockKeysEndpoint({keys: [publishedKey]}); + + await expect(verifySignature({signatures, integrity, packageName, version})).resolves.toBeUndefined(); + + expect(httpFetchAsJson).lastCalledWith(`${registryUrl}${KEYS_PATH}`, {headers: DEFAULT_HEADERS}); + }); + + it(`does not fetch the published keys when the default registry is set explicitly with a trailing slash`, async () => { + const {integrity, signatures} = signPackage(); + process.env.COREPACK_NPM_REGISTRY = `${DEFAULT_NPM_REGISTRY_URL}/`; + + await expect(verifySignature({signatures, integrity, packageName, version})).rejects.toThrowError(/not signed by any trusted keys/); + + expect(httpFetchAsJson).not.toBeCalled(); + }); + + it(`sends the registry credentials when fetching the keys`, async () => { + const {integrity, signatures, publishedKey} = signPackage(); + const registryUrl = uniqueRegistryUrl(); + process.env.COREPACK_NPM_REGISTRY = registryUrl; + process.env.COREPACK_NPM_TOKEN = `foo`; + mockKeysEndpoint({keys: [publishedKey]}); + + await verifySignature({signatures, integrity, packageName, version}); + + expect(httpFetchAsJson).lastCalledWith(`${registryUrl}${KEYS_PATH}`, {headers: { + ...DEFAULT_HEADERS, + authorization: `Bearer foo`, + }}); + }); + + it(`ignores malformed entries in the published keys`, async () => { + const {integrity, signatures, publishedKey} = signPackage(); + process.env.COREPACK_NPM_REGISTRY = uniqueRegistryUrl(); + mockKeysEndpoint({keys: [`invalid`, {keyid: `SHA256:no-key`}, publishedKey]}); + + await expect(verifySignature({signatures, integrity, packageName, version})).resolves.toBeUndefined(); + }); + + it(`only fetches the published keys once per registry`, async () => { + const {integrity, signatures, publishedKey} = signPackage(); + process.env.COREPACK_NPM_REGISTRY = uniqueRegistryUrl(); + mockKeysEndpoint({keys: [publishedKey]}); + + await verifySignature({signatures, integrity, packageName, version}); + await verifySignature({signatures, integrity, packageName, version}); + + expect(httpFetchAsJson).toHaveBeenCalledTimes(1); + }); + + it(`does not fetch the published keys when no custom registry is configured`, async () => { + const {integrity, signatures} = signPackage(); + + await expect(verifySignature({signatures, integrity, packageName, version})).rejects.toThrowError(/not signed by any trusted keys/); + + expect(httpFetchAsJson).not.toBeCalled(); + }); + + it(`does not fetch the published keys when COREPACK_INTEGRITY_KEYS is set`, async () => { + const {integrity, signatures, publishedKey} = signPackage(); + process.env.COREPACK_NPM_REGISTRY = uniqueRegistryUrl(); + process.env.COREPACK_INTEGRITY_KEYS = JSON.stringify({npm: [{keyid: `SHA256:other`, key: `other`}]}); + mockKeysEndpoint({keys: [publishedKey]}); + + await expect(verifySignature({signatures, integrity, packageName, version})).rejects.toThrowError(/not signed by any trusted keys/); + + expect(httpFetchAsJson).not.toBeCalled(); + }); + + it(`does not fetch the published keys when the bundled keys already cover the signature`, async () => { + const {integrity, signatures, publishedKey} = signPackage(); + process.env.COREPACK_NPM_REGISTRY = uniqueRegistryUrl(); + process.env.COREPACK_INTEGRITY_KEYS = JSON.stringify({npm: [publishedKey]}); + mockKeysEndpoint({keys: []}); + + await expect(verifySignature({signatures, integrity, packageName, version})).resolves.toBeUndefined(); + + expect(httpFetchAsJson).not.toBeCalled(); + }); + + it(`throws the usual error when the keys endpoint is unreachable`, async () => { + const {integrity, signatures} = signPackage(); + process.env.COREPACK_NPM_REGISTRY = uniqueRegistryUrl(); + vi.mocked(httpFetchAsJson).mockRejectedValue(new Error(`HTTP 404`)); + + await expect(verifySignature({signatures, integrity, packageName, version})).rejects.toThrowError(/not signed by any trusted keys/); + }); + + it(`throws the usual error when the registry publishes no usable keys`, async () => { + const {integrity, signatures} = signPackage(); + process.env.COREPACK_NPM_REGISTRY = uniqueRegistryUrl(); + mockKeysEndpoint({keys: []}); + + await expect(verifySignature({signatures, integrity, packageName, version})).rejects.toThrowError(/not signed by any trusted keys/); + }); + + it(`throws when the signature does not match the published key`, async () => { + const {integrity, signatures} = signPackage(); + const {publishedKey: otherKey} = signPackage(); + process.env.COREPACK_NPM_REGISTRY = uniqueRegistryUrl(); + // Same keyid as the signature, but a key the signature was not made with. + mockKeysEndpoint({keys: [{...otherKey, keyid: signatures[0].keyid}]}); + + await expect(verifySignature({signatures, integrity, packageName, version})).rejects.toThrowError(/Signature does not match/); + }); +});