Get NPM signing keys from @sigstore/tuf

[geigerzaehler]

Instead of hardcoding npm signing keys for verification we get them from sigstore’s TUF repository. This is in line with how npm implements signature verification.

Fixes #616, fixes #612

---

The implementation is ready to review, @aduh95. The only thing missing is a test for failed signature verification that does use COREPACK_INTEGRITY_KEYS. If you can think of more tests, let me know. I’m also not sure how the test setup works with regards to recording http requests.

• [x] Test that signature verification fails if providing an invalid signature for TUF keys.

[MikeMcC399]

@geigerzaehler

Many thanks for addressing the issue https://github.com/nodejs/corepack/issues/616 I logged. This PR should be a quantum leap forward for Corepack!

I don't have the skills to comment on the code unfortunately. Just a couple of other minor points:

1. If you write Fixes #616, fixes #612 it will automatically link to both issues
2. npm is supposed to be written lowercase. See https://github.com/npm/cli#faq-on-branding

[geigerzaehler]

This change also does not check for key expiry. This requires some more investigation and I’ll tackle this in a follow-up change.

[aduh95]

This makes the bundle size go from 936.5kb to 2.4mb, do we actually need all that code?

[aduh95]

Maybe we could try patching tuf-js to use undici instead of make-fetch-happen (https://github.com/theupdateframework/tuf-js/blob/0303dab1e09cd2c58b9707c4c2b57566c7df349e/packages/client/src/fetcher.ts#L3)

EDIT: I've tried doing that, if we're fine getting rid of the retry: 2 default option, with the following patch we're down to a 1.2MiB bundle:

diff --git a/dist/fetcher.js b/dist/fetcher.js
index f966ce1bb0cdc6c785ce1263f1faea15d3fe764c..cb361aafe39d48ffb1ce8fb0a909bcb958bec46f 100644
--- a/dist/fetcher.js
+++ b/dist/fetcher.js
@@ -6,7 +6,7 @@ Object.defineProperty(exports, "__esModule", { value: true });
 exports.DefaultFetcher = exports.BaseFetcher = void 0;
 const debug_1 = __importDefault(require("debug"));
 const fs_1 = __importDefault(require("fs"));
-const make_fetch_happen_1 = __importDefault(require("make-fetch-happen"));
+const stream_1 = __importDefault(require("stream"));
 const util_1 = __importDefault(require("util"));
 const error_1 = require("./error");
 const tmpfile_1 = require("./utils/tmpfile");
@@ -61,14 +61,13 @@ class DefaultFetcher extends BaseFetcher {
     }
     async fetch(url) {
         log('GET %s', url);
-        const response = await (0, make_fetch_happen_1.default)(url, {
-            timeout: this.timeout,
-            retry: this.retry,
+        const response = await globalThis.fetch(url, {
+            signal: this.timeout && AbortSignal.timeout(this.timeout),
         });
         if (!response.ok || !response?.body) {
             throw new error_1.DownloadHTTPError('Failed to download', response.status);
         }
-        return response.body;
+        return stream.Readable.fromWeb(response.body);
     }
 }
 exports.DefaultFetcher = DefaultFetcher;

[geigerzaehler]

EDIT: I've tried doing that, if we're fine getting rid of the retry: 2 default option, with the following patch we're down to a 1.2MiB bundle:

That’s great. Would you like me to add this as a yarn patch? Or do you want to get this into tuf-js?

[aduh95]

EDIT: I've tried doing that, if we're fine getting rid of the retry: 2 default option, with the following patch we're down to a 1.2MiB bundle:

That’s great. Would you like me to add this as a yarn patch? Or do you want to get this into tuf-js?

I think we should add it as a Yarn patch. I can certainly send a PR their way (https://github.com/theupdateframework/tuf-js/pull/849), but given it would be a major change for them, I don't think it would make sense for us to wait on them.

[geigerzaehler]

I’ve implemented the test and addressed all the feedback. This is ready for review now

[aduh95]

As pointed out in https://github.com/theupdateframework/tuf-js/pull/849#pullrequestreview-2613597859, we should think about proxy support in our patch (ideally using the same one as Corepack does)

[geigerzaehler]

As pointed out in theupdateframework/tuf-js#849 (review), we should think about proxy support in our patch (ideally using the same one as Corepack does)

Addressed in https://github.com/nodejs/corepack/pull/647/commits/3a2115e62501e7e7f8829e5e823fc4063c5fd53a

[geigerzaehler]

It looks like we can’t just use replayed responses for TUF request because @sigstore/tuf checks timestamps and those in the recorded responses become stale and the client throws an error. I’m leaning towards not recording TUF requests and rely on network requests in the tests. Alternatively, we could mock the current date. This would be a bit more cumbersome to deal with when requests are re-recorded and would depend on https://github.com/nodejs/corepack/pull/654. WDYT @aduh95?

[OR13]

Replay with mocked current time seems most aligned with the intention of the tests.

[cupofjoakim]

Is this work ongoing or has it stalled?

[geigerzaehler]

I’m working on rebasing this and fixing the last issues. I hope to get this done next week.

[geigerzaehler]

I’ve updated this PR and everything should work now. PTAL @aduh95

[MikeMcC399]

@geigerzaehler / @aduh95

If there any intention to continue with this PR? It seems to have stalled with no recent progress.

[geigerzaehler]

From my side, the PR is ready to review. Happy to address any feedback and rebase it when necessary.

[MikeMcC399]

@geigerzaehler

• It's good to hear you are ready and waiting! I guess we need action from the maintainers. The last comment from @aduh95 was however in February 2025 and since then nobody from the maintainer team has been active with respect to this PR.

• I would like to see my issue https://github.com/nodejs/corepack/issues/616 able to be closed. Without a resolution, there is still exposure to Corepack failing on new pnpm releases, although we have no idea whether npm will rotate keys again, or if it won't happen again for a number of years!

[MikeMcC399]

@geigerzaehler

• Since there has been no further maintainer feedback since February 2025, and this would be essential to make any progress, I have closed the issue https://github.com/nodejs/corepack/issues/616