From 7d794ca5162edb0cdcf022126ed73f878b6fb57f Mon Sep 17 00:00:00 2001 From: Aschenbath Date: Tue, 20 Oct 2026 16:56:29 +0800 Subject: [PATCH] fix: await signature verification before installing package managers Signed-off-by: Aschenbath --- sources/corepackUtils.ts | 11 +++++-- tests/_registryServer.mjs | 22 ++++++++++---- tests/main.test.ts | 62 ++++++++++++++++++++++++++++++++++++--- 3 files changed, 83 insertions(+), 12 deletions(-) diff --git a/sources/corepackUtils.ts b/sources/corepackUtils.ts index d38df1c7b..c15787736 100644 --- a/sources/corepackUtils.ts +++ b/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`); } diff --git a/tests/_registryServer.mjs b/tests/_registryServer.mjs index cb4553d04..49a6e259b 100644 --- a/tests/_registryServer.mjs +++ b/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) === `/-/`; diff --git a/tests/main.test.ts b/tests/main.test.ts index 4bb7f40f4..ece0b4938 100644 --- a/tests/main.test.ts +++ b/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 pnpm@1.9998.9999 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`, `yarn@1.9998.9999`], true)).resolves.toMatchObject({ exitCode: 1, - stderr: expect.stringContaining(`Signature does not match`), - stdout: `Installing yarn@1.9998.9999 in the project...\n`, + stderr: ``, + stdout: expect.stringContaining(`Signature does not match`), }); }); });