fix: improve integrity verification errors

INTEGRITY.md

@@ -0,0 +1,106 @@
+# Integrity checks
+
+**An integrity failure can indicate that a package manager download or its
+metadata has been tampered with, including by a man-in-the-middle attack.
+Running a malicious package manager can compromise your project, credentials,
+and machine. Investigate the failure before bypassing verification.**
+
+Registry configuration problems, missing metadata, and signing key rotations
+can also cause these errors. A failure alone does not prove an attack.
+
+Corepack checks downloads of package managers such as npm, pnpm, and Yarn. These
+checks do not verify your project's dependencies; those are downloaded and
+managed by the package manager itself.
+
+## What Corepack verifies
+
+When a package manager version is pinned with a hash (for example in the
+`packageManager` field of `package.json`), Corepack compares the downloaded file
+against that hash.
+
+For supported npm registry downloads without a pinned hash, Corepack verifies
+the registry's signature against its trusted public keys, then compares the
+download against the signed integrity hash. Corepack also verifies signatures
+when resolving the latest stable version from an npm registry. The signed
+message is `${packageName}@${version}:${dist.integrity}`.
+
+Not every download supports registry signatures. See
+[`COREPACK_ON_UNVERIFIED_DOWNLOAD`](./README.md#environment-variables) for options
+to reject downloads that cannot be verified or that do not have a pinned hash.
+
+## Troubleshooting
+
+- **Missing or invalid `dist.signatures`:** Check that your registry or registry
+  proxy preserves the upstream signing metadata. Corepack also looks for
+  signatures in the package-root metadata when the per-version endpoint omits
+  them. If your registry signs packages itself, configure its trusted public
+  keys as described below. Adding keys cannot fix missing signatures.
+- **Missing or invalid `dist.integrity`:** The registry must provide the
+  SHA-512 integrity hash used to sign the package metadata. Ask the registry
+  administrator to restore it.
+- **Not signed by any trusted keys:** Verify which registry you are using
+  (`COREPACK_NPM_REGISTRY`) and whether its signing keys have changed. For the
+  public npm registry, update Corepack to a release with the current trusted
+  keys. Check whether `COREPACK_INTEGRITY_KEYS` is overriding the bundled keys.
+  For a custom registry, obtain the correct public keys from its administrator
+  through a trusted channel.
+- **Invalid `COREPACK_INTEGRITY_KEYS` or public key:** Check the JSON structure
+  and public key encoding against the example below.
+- **Cannot verify signature using trusted key:** Check that the public key type
+  and signature format support SHA-256 signature verification.
+- **Signature does not match:** The metadata does not match the signature made
+  with the trusted key. Investigate the registry, proxy, and signing process;
+  do not assume that downloading a new key or disabling verification is safe.
+- **Mismatch hashes:** The downloaded file does not match the pinned or signed
+  hash. Investigate possible corruption or tampering. Confirm the expected
+  artifact with its publisher before changing a pinned hash.
+
+If resolving the latest stable version fails, setting
+`COREPACK_DEFAULT_TO_LATEST=0` uses the release already known to your Corepack
+installation. It does not disable verification of subsequent downloads.
+
+## Configuring trusted keys
+
+By default, Corepack uses the keys in [`config.json`](./config.json). To override
+them, set `COREPACK_INTEGRITY_KEYS` to a JSON object whose `npm` property is an
+array of trusted public keys. This array **replaces** the bundled npm keys; it
+does not add to them.
+
+For example, in a POSIX shell, replace the placeholders with values obtained
+through a trusted channel from your registry administrator:
+
+```sh
+export COREPACK_INTEGRITY_KEYS='{"npm":[{"keyid":"SHA256:<trusted-key-id>","key":"<base64-encoded-SPKI-public-key>"}]}'
+```
+
+- `keyid` must match the `keyid` in the registry's `dist.signatures` entries.
+- `key` is the base64 encoding of the DER public key in SubjectPublicKeyInfo
+  (SPKI) format, without PEM headers, footers, or line breaks. Use a public key,
+  never a private key.
+- Multiple keys can be included in the array, for example during a key rotation.
+
+The npm registry's `/-/npm/v1/keys` endpoint returns an object with a `keys`
+array. Corepack expects that array under `npm`, rather than the endpoint's entire
+response. Authenticate any replacement keys independently; blindly trusting
+keys returned alongside a failing download defeats signature verification.
+
+Unset `COREPACK_INTEGRITY_KEYS` to restore the bundled trusted keys.
+
+## Disabling signature verification
+
+**Disabling verification can allow an attacker to substitute a malicious
+package manager. Only bypass it if you have established the authenticity of
+the download by another trusted means.**
+
+To explicitly disable signature verification for one command in a POSIX shell,
+set `COREPACK_INTEGRITY_KEYS` to an empty string:
+
+```sh
+COREPACK_INTEGRITY_KEYS='' corepack install --global pnpm
+```
+
+This skips registry signature verification and the protection provided by its
+signed integrity hash. Explicitly pinned hashes are still checked. Avoid
+persisting this setting in a shell profile or CI configuration; unset it to
+restore verification. A hash obtained from the same untrusted source as the
+download does not establish authenticity.

README.md

@@ -375,14 +375,17 @@ same major line. Should you need to upgrade to a new major, use an explicit
 - `HTTP_PROXY`, `HTTPS_PROXY`, and `NO_PROXY` are supported through
   [`NODE_USE_ENV_PROXY=1`](https://nodejs.org/api/cli.html#node_use_env_proxy1).
 
-- `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.
+- `COREPACK_INTEGRITY_KEYS` can be set to a JSON string containing custom trusted
+  keys. Setting it to an empty string disables signature verification and can
+  allow malicious package managers to run. See [Integrity checks](./INTEGRITY.md)
+  for the key format, troubleshooting, and the risks of disabling verification.
 
 ## Troubleshooting
 
 The environment variable `DEBUG` can be set to `corepack` to enable additional debug logging.
 
+For signature verification or hash mismatch errors, see [Integrity checks](./INTEGRITY.md).
+
 ### Networking
 
 There are a wide variety of networking issues that can occur while running

sources/corepackUtils.ts

@@ -322,13 +322,18 @@ 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});
+    try {
+      await npmRegistryUtils.verifySignature({signatures, integrity, packageName: registry.package, version});
+    } catch (error) {
+      await fs.promises.rm(tmpFolder, {recursive: true, force: true});
+      throw error;
+    }
     // @ts-expect-error ignore readonly
     build[1] = Buffer.from(integrity.slice(`sha512-`.length), `base64`).toString(`hex`);
   }
   if (build[1] && actualHash !== build[1]) {
     await fs.promises.rm(tmpFolder, {recursive: true, force: true});
-    throw new Error(`Mismatch hashes. Expected ${build[1]}, got ${actualHash}`);
+    throw new Error(`Mismatch hashes. Expected ${build[1]}, got ${actualHash}. See ${npmRegistryUtils.INTEGRITY_KEYS_URL} for troubleshooting.`);
   }
 
   const serializedHash = `${algo}.${actualHash}`;

sources/npmRegistryUtils.ts

@@ -1,17 +1,18 @@
-import {UsageError}               from 'clipanion';
-import {createVerify}             from 'crypto';
+import {UsageError}                    from 'clipanion';
+import {createPublicKey, createVerify} from 'crypto';
 
-import defaultConfig              from '../config.json' with {type: 'json'};
+import defaultConfig                   from '../config.json' with {type: 'json'};
 
-import {shouldSkipIntegrityCheck} from './corepackUtils.ts';
-import * as httpUtils             from './httpUtils.ts';
+import {shouldSkipIntegrityCheck}      from './corepackUtils.ts';
+import * as httpUtils                  from './httpUtils.ts';
 
 // load abbreviated metadata as that's all we need for these calls
 // see: https://github.com/npm/registry/blob/cfe04736f34db9274a780184d1cdb2fb3e4ead2a/docs/responses/package-metadata.md
 export const DEFAULT_HEADERS: Record<string, string> = {
   [`Accept`]: `application/vnd.npm.install-v1+json; q=1.0, application/json; q=0.8`,
 };
 export const DEFAULT_NPM_REGISTRY_URL = `https://registry.npmjs.org`;
+export const INTEGRITY_KEYS_URL = `https://github.com/nodejs/corepack/blob/main/INTEGRITY.md`;
 
 export async function fetchAsJson(packageName: string, version?: string) {
   // Strip any trailing slashes so a `COREPACK_NPM_REGISTRY` with a trailing
@@ -36,22 +37,37 @@ export async function fetchAsJson(packageName: string, version?: string) {
 }
 
 export async function verifySignature({signatures, integrity, packageName, version}: {
-  signatures: Array<{keyid: string, sig: string}>;
-  integrity: string;
+  signatures: unknown;
+  integrity: unknown;
   packageName: string;
   version: string;
 }) {
+  const integrityError = (message: string) => new UsageError(`${message} (${packageName}@${version}). See ${INTEGRITY_KEYS_URL} for troubleshooting.`);
+
   if (!Array.isArray(signatures) || !signatures.length) {
     // Some registry proxies omit `dist.signatures` on the per-version endpoint but keep it on the package root.
     const rootMetadata = await fetchAsJson(packageName);
-    signatures = rootMetadata.versions?.[version]?.dist?.signatures;
+    signatures = rootMetadata?.versions?.[version]?.dist?.signatures;
   }
 
-  if (!Array.isArray(signatures) || !signatures.length) throw new Error(`No compatible signature found in package metadata`);
+  if (!Array.isArray(signatures) || !signatures.length)
+    throw integrityError(`No compatible signature found in package metadata; the registry must provide dist.signatures`);
+
+  if (signatures.some(s => typeof s?.keyid !== `string` || !s.keyid || typeof s?.sig !== `string` || !s.sig))
+    throw integrityError(`Invalid dist.signatures in package metadata; each signature must contain non-empty keyid and sig strings`);
+
+  if (typeof integrity !== `string` || !integrity.startsWith(`sha512-`) || integrity.length === `sha512-`.length)
+    throw integrityError(`Missing or invalid dist.integrity in package metadata; the registry must provide a SHA-512 integrity hash`);
 
-  const {npm: trustedKeys} = process.env.COREPACK_INTEGRITY_KEYS ?
-    JSON.parse(process.env.COREPACK_INTEGRITY_KEYS) as typeof defaultConfig.keys :
-    defaultConfig.keys;
+  let trustedKeys: typeof defaultConfig.keys.npm;
+  try {
+    const keys = process.env.COREPACK_INTEGRITY_KEYS ? JSON.parse(process.env.COREPACK_INTEGRITY_KEYS) : defaultConfig.keys;
+    if (!Array.isArray(keys?.npm) || keys.npm.some((k: any) => typeof k?.keyid !== `string` || !k.keyid || typeof k?.key !== `string` || !k.key))
+      throw new Error(`Invalid trusted keys`);
+    trustedKeys = keys.npm;
+  } catch {
+    throw integrityError(`Invalid COREPACK_INTEGRITY_KEYS; expected a JSON object with an npm array of trusted keys containing non-empty keyid and key strings`);
+  }
 
   let signature: typeof signatures[0] | undefined;
   let key!: string;
@@ -62,17 +78,26 @@ export async function verifySignature({signatures, integrity, packageName, versi
       break;
     }
   }
-  if (signature?.sig == null) throw new UsageError(`The package was not signed by any trusted keys: ${JSON.stringify({signatures, trustedKeys}, undefined, 2)}`);
+  if (signature == null)
+    throw integrityError(`The package was not signed by any trusted keys; check the registry's signing keys and COREPACK_INTEGRITY_KEYS`);
+
+  let publicKey;
+  try {
+    publicKey = createPublicKey(`-----BEGIN PUBLIC KEY-----\n${key}\n-----END PUBLIC KEY-----`);
+  } catch {
+    throw integrityError(`Invalid public key for ${signature.keyid}; COREPACK_INTEGRITY_KEYS must contain base64-encoded DER public keys in SubjectPublicKeyInfo format`);
+  }
 
   const verifier = createVerify(`SHA256`);
   verifier.end(`${packageName}@${version}:${integrity}`);
-  const valid = verifier.verify(
-    `-----BEGIN PUBLIC KEY-----\n${key}\n-----END PUBLIC KEY-----`,
-    signature.sig,
-    `base64`,
-  );
+  let valid;
+  try {
+    valid = verifier.verify(publicKey, signature.sig, `base64`);
+  } catch {
+    throw integrityError(`Cannot verify signature using trusted key ${signature.keyid}; check the public key and signature format`);
+  }
   if (!valid) {
-    throw new Error(`Signature does not match`);
+    throw integrityError(`Signature does not match; the package metadata may have been tampered with`);
   }
 }
 
@@ -88,8 +113,7 @@ export async function fetchLatestStableVersion(packageName: string) {
         integrity, signatures,
       });
     } catch (cause) {
-      // TODO: consider switching to `UsageError` when https://github.com/arcanis/clipanion/issues/157 is fixed
-      throw new Error(`Corepack cannot download the latest stable version of ${packageName}; you can disable signature verification by setting COREPACK_INTEGRITY_KEYS to 0 in your env, or instruct Corepack to use the latest stable release known by this version of Corepack by setting COREPACK_DEFAULT_TO_LATEST to 0`, {cause});
+      throw new UsageError(`Corepack cannot download the latest stable version of ${packageName}: ${cause instanceof Error ? cause.message : String(cause)}\nYou can use the latest stable release known by this version of Corepack by setting COREPACK_DEFAULT_TO_LATEST to 0. For integrity key configuration, see ${INTEGRITY_KEYS_URL}.`);
     }
   }
 

tests/_registryServer.mjs

@@ -13,6 +13,11 @@ switch (process.env.TEST_INTEGRITY) {
     }));
   }
   // eslint-disable-next-line no-fallthrough
+  case `missing_version_signatures`:
+  case `missing_integrity`:
+  case `malformed_signatures`:
+  case `invalid_keys`:
+  case `untrusted_key`:
   case `invalid_integrity`:
   case `valid`: {
     const {privateKey: p, publicKey} = generateKeyPairSync(`ec`, {
@@ -31,6 +36,10 @@ switch (process.env.TEST_INTEGRITY) {
       scheme: `ecdsa-sha2-sect239k1`,
       key: publicKey.split(`\n`).slice(1, -2).join(``),
     }]});
+    if (process.env.TEST_INTEGRITY === `invalid_keys`)
+      process.env.COREPACK_INTEGRITY_KEYS = `{"npm":{}}`;
+    if (process.env.TEST_INTEGRITY === `untrusted_key`)
+      keyid = `SHA256:untrusted-key`;
     break;
   }
 }
@@ -87,10 +96,13 @@ const registry = {
 function generateSignature(packageName, version) {
   if (privateKey == null) return undefined;
   const sign = createSign(`SHA256`).end(`${packageName}@${version}:${integrity}`);
-  return {integrity, signatures: [{
-    keyid,
-    sig: sign.sign(privateKey, `base64`),
-  }]};
+  return {
+    integrity: process.env.TEST_INTEGRITY === `missing_integrity` ? undefined : integrity,
+    signatures: process.env.TEST_INTEGRITY === `malformed_signatures` ? [null] : [{
+      keyid,
+      sig: sign.sign(privateKey, `base64`),
+    }],
+  };
 }
 function generateVersionMetadata(packageName, version) {
   return {
@@ -150,10 +162,13 @@ const server = createServer((req, res) => {
     }
     if (version === `latest`) version = registry[packageName].at(-1);
     if (registry[packageName].includes(version)) {
+      const metadata = generateVersionMetadata(packageName, version);
+      if (process.env.TEST_INTEGRITY === `missing_version_signatures`)
+        delete metadata.dist.signatures;
       res.end(
         isDownloadingRequest ?
           mockPackageTarGz :
-          JSON.stringify(generateVersionMetadata(packageName, version)),
+          JSON.stringify(metadata),
       );
     } else {
       res.writeHead(404).end(`Not Found`);

tests/main.test.ts

@@ -1934,6 +1934,55 @@ describe(`handle integrity checks`, () => {
     process.env.AUTH_TYPE = `COREPACK_NPM_TOKEN`; // See `_registryServer.mjs`
   });
 
+  it.each([
+    [`unsigned`, `No compatible signature found`],
+    [`missing_integrity`, `Missing or invalid dist.integrity`],
+    [`malformed_signatures`, `Invalid dist.signatures`],
+    [`invalid_keys`, `Invalid COREPACK_INTEGRITY_KEYS`],
+    [`untrusted_key`, `not signed by any trusted keys`],
+    [`invalid_signature`, `Signature does not match`],
+  ])(`rejects %s metadata without caching or running the package manager`, async (mode, message) => {
+    process.env.TEST_INTEGRITY = mode; // See `_registryServer.mjs`
+
+    await xfs.mktempPromise(async cwd => {
+      // A second attempt must also fail, rather than execute a cached download.
+      for (let attempt = 0; attempt < 2; attempt++) {
+        const result = await runCli(cwd, [`[email protected]`, `--version`], true);
+        expect(result).toMatchObject({exitCode: 1, stdout: ``});
+        expect(result.stderr).toContain(message);
+        expect(result.stderr).toContain(`[email protected]`);
+        expect(result.stderr).toContain(`https://github.com/nodejs/corepack/blob/main/INTEGRITY.md`);
+        expect(result.stderr).not.toMatch(/TypeError|UnhandledPromiseRejection|\bat .+:\d+:\d+/);
+      }
+
+      expect(await xfs.readdirPromise(npath.toPortablePath(folderUtils.getInstallFolder()))).toEqual([]);
+    });
+  });
+
+  it(`explains missing signatures when resolving the latest version`, async () => {
+    process.env.COREPACK_DEFAULT_TO_LATEST = `1`;
+
+    await xfs.mktempPromise(async cwd => {
+      const result = await runCli(cwd, [`pnpm`, `--version`], true);
+      expect(result).toMatchObject({exitCode: 1, stdout: ``});
+      expect(result.stderr).toContain(`No compatible signature found`);
+      expect(result.stderr).toContain(`https://github.com/nodejs/corepack/blob/main/INTEGRITY.md`);
+      expect(result.stderr).not.toMatch(/TypeError|UnhandledPromiseRejection|\bat .+:\d+:\d+/);
+    });
+  });
+
+  it(`waits for signatures from the package-root endpoint before installing`, async () => {
+    process.env.TEST_INTEGRITY = `missing_version_signatures`; // See `_registryServer.mjs`
+
+    await xfs.mktempPromise(async cwd => {
+      await expect(runCli(cwd, [`[email protected]`, `--version`], true)).resolves.toMatchObject({
+        exitCode: 0,
+        stdout: `pnpm: Hello from custom registry\n`,
+        stderr: ``,
+      });
+    });
+  });
+
   describe(`when signature matches`, () => {
     beforeEach(() => {
       process.env.TEST_INTEGRITY = `valid`; // See `_registryServer.mjs`
@@ -2064,8 +2113,8 @@ describe(`handle integrity checks`, () => {
       });
       await expect(runCli(cwd, [`use`, `pnpm`], true)).resolves.toMatchObject({
         exitCode: 1,
-        stderr: expect.stringContaining(`Signature does not match`),
-        stdout: `Installing [email protected] in the project...\n`,
+        stderr: ``,
+        stdout: expect.stringContaining(`Usage Error: Signature does not match`),
       });
     });
   });
@@ -2080,8 +2129,8 @@ describe(`handle integrity checks`, () => {
       });
       await expect(runCli(cwd, [`use`, `[email protected]`], true)).resolves.toMatchObject({
         exitCode: 1,
-        stderr: expect.stringContaining(`Signature does not match`),
-        stdout: `Installing [email protected] in the project...\n`,
+        stderr: ``,
+        stdout: expect.stringContaining(`Usage Error: Signature does not match`),
       });
     });
   });