fix: await signature verification before installing package managers

sources/corepackUtils.ts

@@ -319,10 +319,15 @@ export async function installVersion(installTarget: string, locator: Locator, {s
   }
 
   if (!build[1] && canVerifySignature && registry.type === `npm`) {
-    if (signatures! == null || integrity! == null)
-      ({signatures, integrity} = (await npmRegistryUtils.fetchTarballURLAndSignature(registry.package, version)));
+    try {
+      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});
+    } 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`);
   }

tests/_registryServer.mjs

@@ -92,8 +92,8 @@ function generateSignature(packageName, version) {
     sig: sign.sign(privateKey, `base64`),
   }]};
 }
-function generateVersionMetadata(packageName, version) {
-  return {
+function generateVersionMetadata(packageName, version, isPackageRoot = false) {
+  const metadata = {
     name: packageName,
     version,
     bin: {
@@ -106,6 +106,9 @@ function generateVersionMetadata(packageName, version) {
       ...generateSignature(packageName, version),
     },
   };
+  if (process.env.TEST_SIGNATURES === `none` || (process.env.TEST_SIGNATURES && !isPackageRoot))
+    delete metadata.dist.signatures;
+  return metadata;
 }
 
 const TOKEN_MOCK = `SOME_DUMMY_VALUE`;
@@ -131,11 +134,20 @@ const server = createServer((req, res) => {
   const packageName = req.url.slice(1, slashPosition === -1 ? undefined : slashPosition);
   if (packageName in registry) {
     if (req.url === `/${packageName}`) {
-      res.end(JSON.stringify({"dist-tags": {
+      if (process.env.TEST_SIGNATURES === `root-error`) {
+        res.writeHead(503).end(`Service Unavailable`);
+        return;
+      }
+      const metadata = JSON.stringify({"dist-tags": {
         latest: registry[packageName].at(-1),
       }, versions: Object.fromEntries(registry[packageName].map(version =>
-        [version, generateVersionMetadata(packageName, version)],
-      ))}));
+        [version, generateVersionMetadata(packageName, version, true)],
+      ))});
+      // Simulate a slow proxy so verification must finish before the CLI runs.
+      if (process.env.TEST_SIGNATURES === `package-root`)
+        setTimeout(() => res.end(metadata), 100);
+      else
+        res.end(metadata);
       return;
     }
     const isDownloadingRequest = req.url.slice(packageName.length + 1, packageName.length + 4) === `/-/`;

tests/main.test.ts

@@ -1934,6 +1934,60 @@ describe(`handle integrity checks`, () => {
     process.env.AUTH_TYPE = `COREPACK_NPM_TOKEN`; // See `_registryServer.mjs`
   });
 
+  describe(`verify signatures before installing exact versions`, () => {
+    for (const authType of [`COREPACK_NPM_TOKEN`, `COREPACK_NPM_PASSWORD`]) {
+      for (const packageManager of [`pnpm`, `yarn`]) {
+        it(`should await package-root signatures for ${packageManager} with ${authType}`, async () => {
+          process.env.AUTH_TYPE = authType;
+          process.env.TEST_INTEGRITY = `valid`;
+          process.env.TEST_SIGNATURES = `package-root`;
+
+          await xfs.mktempPromise(async cwd => {
+            await expect(runCli(cwd, [`${packageManager}@1.9998.9999`, `--version`], true)).resolves.toMatchObject({
+              exitCode: 0,
+              stdout: `${packageManager}: Hello from custom registry\n`,
+              stderr: ``,
+            });
+
+            const installFolder = npath.toPortablePath(folderUtils.getInstallFolder());
+            const cached = await xfs.readJsonPromise(ppath.join(installFolder, packageManager as Filename, `1.9998.9999`, `.corepack`));
+            expect(cached.hash).toMatch(/^sha512\.[a-f0-9]{128}$/);
+          });
+        });
+      }
+    }
+
+    for (const {integrity, signatures, error} of [
+      {integrity: `invalid_signature`, signatures: undefined, error: `Signature does not match`},
+      {integrity: `invalid_signature`, signatures: `package-root`, error: `Signature does not match`},
+      {integrity: `valid`, signatures: `none`, error: `No compatible signature found in package metadata`},
+      {integrity: `valid`, signatures: `root-error`, error: `Server answered with HTTP 503`},
+    ]) {
+      for (const packageManager of [`pnpm`, `yarn`]) {
+        it(`should reject ${integrity} with ${signatures ?? `version-endpoint`} signatures without running or caching ${packageManager}`, async () => {
+          process.env.TEST_INTEGRITY = integrity;
+          if (signatures)
+            process.env.TEST_SIGNATURES = signatures;
+
+          await xfs.mktempPromise(async cwd => {
+            // Retry the command to detect a rejected download being reused from cache.
+            for (let attempt = 0; attempt < 2; attempt++) {
+              await expect(runCli(cwd, [`${packageManager}@1.9998.9999`, `--version`], true)).resolves.toMatchObject({
+                exitCode: 1,
+                stdout: ``,
+                stderr: expect.stringContaining(error),
+              });
+
+              const installFolder = npath.toPortablePath(folderUtils.getInstallFolder());
+              expect(await xfs.readdirPromise(installFolder)).toEqual([]);
+              expect(await xfs.existsPromise(ppath.join(npath.toPortablePath(folderUtils.getCorepackHomeFolder()), `lastKnownGood.json`))).toBe(false);
+            }
+          });
+        });
+      }
+    }
+  });
+
   describe(`when signature matches`, () => {
     beforeEach(() => {
       process.env.TEST_INTEGRITY = `valid`; // See `_registryServer.mjs`
@@ -2064,8 +2118,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(`Signature does not match`),
       });
     });
   });
@@ -2080,8 +2134,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(`Signature does not match`),
       });
     });
   });