feat: download the latest version instead of a pinned one #134

aduh95 · 2022-06-26T13:18:24Z

This PR ensures that fresh install of Corepack would always use the latest available version for a given package manager.

We probably want to add a command to clear the cache for lastKnownGood version (although I think that's more or less what corepack prepare is doing), and/or have a expiracy date for the lastKnownGood version to make Corepack update the default version from time to time.

Fixes: #93

sources/Engine.ts

aduh95 · 2022-07-07T17:31:20Z

README.md

@@ -106,6 +113,8 @@ This command will retrieve the given package manager from the specified archive

 - `COREPACK_ENABLE_NETWORK` can be set to `0` to prevent Corepack from accessing the network (in which case you'll be responsible for hydrating the package manager versions that will be required for the projects you'll run, using `corepack hydrate`).

+- `COREPACK_NO_LOOKUP` can be set in order to instruct Corepack not to lookup on the remote registry for the latest version of the selected package manager.


We should probably find a better name for this env variable, I don't think one could infer what it's doing from the name only. Does anyone has suggestions?

I'm trying to understand the difference between COREPACK_ENABLE_NETWORK=0 and COREPACK_NO_LOOKUP=1? In theory I would expect no network to behave the same since corepack ships with Known Good Versions but I must be overlooking a use case here.

When COREPACK_NO_LOOKUP is set, Corepack will use the version defined in config.json instead of asking the npm registry for the most up-to-date version. COREPACK_ENABLE_NETWORK can disable all access to the network, both for finding what's the most up-to-date version number and to download the tarball.

Is config.json available to configure by the user of corepack or is it just internals?

It's only internals, bundled in the corepack executable.

How about COREPACK_IGNORE_LATEST?

I went with COREPACK_DEFAULT_TO_LATEST which is true by default, and is set to 0 to use the value in config.json instead.

Not sure about using “default” as a verb here.

How about COREPACK_ENABLE_FETCH_LATEST=0

I'd like the naming to make clear that it's only relevant if there's no local clue on what version to use. ENABLE_FETCH_LATEST could mean that the latest is always downloaded, or at least that's how I read it.

Not sure about using “default” as a verb here.

I'm not a native english speaker, so genuine question: is it incorrect or something to use "default" as a verb?

It’s not incorrect, just that “default” can be a verb or a noun. It’s probably fine 👍

styfle · 2022-07-08T22:54:59Z

README.md


-The Known Good Releases can be updated system-wide using the `--activate` flag from the `corepack prepare` and `corepack hydrate` commands.
+If there is no Known Good Release for the requested package manager, Corepack


In what case would there be no Known Good Release?

Upon the first time Corepack is run with a given package manager. The idea behind this PR would be not to rely on what version is set on config.json but instead get the most up-to-date version from npm registry.

How often does it check the registry? Does it check every time you invoke yarn/pnpm/npm because that seems slow. Especially if I just want to do npm run <script> which should work offline.

No, only the first time, at which point it saves it as Known Good Release. Users can then update (or downgrade) that version by using corepack prepare or corepack hydrate (e.g. corepack prepare pnpm@latest --activate to get the latest pnpm by default). We could make an automated process for the Known Good Version to expire after some time, but that's not part of this initial implementation.

Thanks, I understand now 👍

I think expiring would could be confusing. I also think this any auto update could be confusing because it no longer ties the package manager version to the corepack version (or node version for that matter).

I think it’s best to let the user decide when to upgrade and perhaps provide better docs how to upgrade (this isn’t super clear today). Without corepack, this is handled with a notification that says something like npm i -g npm@latest.

Has any of this behavior been reviewed by the npm team? I think they had some ideas about Know Good Release.

I also think this any auto update could be confusing because it no longer ties the package manager version to the corepack version (or node version for that matter).

Well that's the reason Corepack exists, because the Node.js TSC doesn't want to have specific version of a package manager tied in with a specific Node.js version – specifically, several Node.js TSC members have expressed that the reason they are supportive of Corepack is to get rid of having to do security releases every time a vulnerability is reported in npm rather than node itself.
So the auto-lookup during the first use has to be the default behavior.

I think it’s best to let the user decide when to upgrade and perhaps provide better docs how to upgrade (this isn’t super clear today). Without corepack, this is handled with a notification that says something like npm i -g npm@latest.

Yes that makes sense, letting the end-user in control is very much the goal here. So instead of npm i -g npm@latest, users would type corepack prepare npm@latest --activate, which is arguably quite similar. Improving docs is of course always a goal.

Has any of this behavior been reviewed by the npm team? I think they had some ideas about Know Good Release.

The npm team hasn't shown much interest in the development of Corepack, so I don't think they have.

styfle · 2022-07-24T18:41:53Z

sources/corepackUtils.ts

+export async function fetchLatestStableVersion(spec: RegistrySpec) {
+  switch (spec.type) {
+    case `npm`: {
+      const {[`dist-tags`]: {latest}, versions: {[latest]: {dist: {shasum}}}} =


Should this consider the major version? What about when npm 7 bumped to npm 8 to drop support for node 12. Imagine if corepack existed then and suddenly npm stopped working because of the automatic fetch latest.

That’s worth discussing, but imo it’s ok to ignore that for the initial implementation.

arcanis

I think that's acceptable to me (Yarn isn't impacted that much since we don't make very frequent / risky releases to 1.22 anyway). Maybe we should wait a bit to see if @zkochan has an opinion?

zkochan · 2022-08-20T13:06:38Z

Sorry for not answering. I think it is OK to ship the latest pnpm. Thanks

andersk · 2022-08-23T20:41:21Z

I have some concerns about this change.

This negates the hash protection of chore: add sha1 to default versions when available #137.
This could break the reproducibility of builds or CI jobs that start from a specific Node.js version (unless specifically overridden).
Consider a user who gets semi-regular automatic Node.js updates from their distribution, which means they get semi-regular automatic corepack updates. Under the previous scheme, they would therefore get semi-regular automatic updates to the default versions of Yarn and pnpm. But now those versions will be fixed from the first time they’re run (until a manual update).

aduh95 · 2022-08-24T00:34:57Z

This negates the hash protection of chore: add sha1 to default versions when available #137.

This could break the reproducibility of builds or CI jobs that start from a specific Node.js version (unless specifically overridden).

The workaround is to set a specific version in the project package.json and/or set COREPACK_DEFAULT_TO_LATEST to 0 in the CI env.

3. Consider a user who gets semi-regular automatic Node.js updates from their distribution, which means they get semi-regular automatic corepack updates. Under the previous scheme, they would therefore get semi-regular automatic updates to the default versions of Yarn and pnpm. But now those versions will be fixed from the first time they’re run (until a manual update).

Contributions are welcome to improve this workflow.

andersk · 2022-08-24T02:08:22Z

Of course there are workarounds. But good defaults matter, and I’m proposing that the default should remain the secure known-good version.

A user who discovers they need a later version than the known-good version is in a position to go read the corepack documentation and configure it appropriately. (Most likely what they really want is to set a version in package.json so that everyone working on that project gets the right version. But if they want to set it system-wide, sure, they can have that option.)

Meanwhile, a user who unexpectedly gets a later (or earlier!) version when they didn’t make a change—they just re-ran the same script on a different system—is in a much more confusing position. They could very well be trying to get dozens of things working at the same time; they might not be an expert on the particular part of the script that failed, and might not even realize in the moment that their Yarn or pnpm is coming from corepack. Could they (or someone else who wrote the script) have had the foresight to reconfigure corepack more deterministically? Of course, but a trap that could be avoided is still a trap; good defaults matter.

Users don’t expect that installing a deterministic version of A should give them a nondeterministic version of B. We shouldn’t force them to remember or disable an exception to this rule.

ljharb · 2022-08-24T02:33:33Z

That’s exactly what users in this ecosystem expect - determinism, for better or worse, is simply not an expectation (beyond a lockfile), and anything else is surprising and very inconvenient, and in this case, causes many more people to have a buggy or insecure version of their package manager CLI.

aduh95 · 2022-08-24T08:12:19Z

But good defaults matter, and I’m proposing that the default should remain the secure known-good version.

That's the catch, the known-good version is more likely to be less secure than the latest one.

I agree with Jordan, I don't think most users care about that kind of determinism (otherwise, they would set a specific version either system-wise, or at the project level). Also, as I wrote in #134, the reason Corepack is integrated in the Node.js project is that it make decouple what version of npm comes bundled with the Node.js installation, users can get a secure version of their package manager without Node.js needing to issue a security update.

styfle · 2022-08-24T13:46:19Z

I don't think most users care about that kind of determinism (otherwise, they would set a specific version either system-wise

What makes you think that users are not setting a specific version? I see many projects using npm i -g pnpm@7.9.5. In fact, package managers print a warning and tell the user the upgrade command when running an outdated version.

aduh95 · 2022-08-24T14:43:37Z

I don't think most users care about that kind of determinism (otherwise, they would set a specific version either system-wise

What makes you think that users are not setting a specific version? I see many projects using npm i -g pnpm@7.9.5. In fact, package managers print a warning and tell the user the upgrade command when running an outdated version.

Having a fixed default version would not change anything for those users, since they're setting a specific one anyway, right? What I'm saying is that in my experience, folks expect to get the latest version by default, and they are used to set a specific version instead if/when they want determinism (and they should, that's why we added the packageManager field in the package.json).

andersk · 2022-08-24T21:49:28Z

This change isn’t giving them the latest version. It’s giving them the latest version as of the time they ran corepack prepare. That could be ages earlier than the time they upgraded Node.js and corepack, since it’s a different tool they could easily forget to run regularly when upgrading everything else.

But the security concern isn’t just that they might get an older version—it’s that skipping the hash protection provided by known-good versions may make it easier for an attacker to maliciously alter the code in transit, since HTTPS can much easier to defeat in environments with intercepting proxy middleboxes and dubious certificate authorities.

aduh95 · 2022-08-24T22:17:51Z

This change isn’t giving them the latest version. It’s giving them the latest version as of the time they ran corepack prepare. That could be ages earlier than the time they upgraded Node.js and corepack, since it’s a different tool they could easily forget to run regularly when upgrading everything else.

I reckon the current setup is not ideal, it's a first implementation and I'd be delighted to see it improve in a way that's more convenient for the users. Contributions would be much welcome, as we don't have a clear plan on how to give a way for users to update without relying on them remembering to run a command manually on a regular basis.

But the security concern isn’t just that they might get an older version—it’s that skipping the hash protection provided by known-good versions may make it easier for an attacker to maliciously alter the code in transit, since HTTPS can much easier to defeat in environments with intercepting proxy middleboxes and dubious certificate authorities.

I think #10 would be a better response to this, as it should give us the best of both worlds.

feat: download the latest version instead of a pinned one

88f6807

Fixes: #93

aduh95 requested a review from arcanis June 26, 2022 13:18

add COREPACK_NO_LOOKUP env flag

6c9e041

arcanis reviewed Jun 27, 2022

View reviewed changes

sources/Engine.ts Outdated Show resolved Hide resolved

Use KGR to cache the version

0668fb0

merceyz reviewed Jun 27, 2022

View reviewed changes

sources/Engine.ts Outdated Show resolved Hide resolved

don't hardcode the registry

c33fa1d

aduh95 force-pushed the dynamic-default-version branch from 0a588c3 to c33fa1d Compare June 29, 2022 11:36

aduh95 marked this pull request as ready for review June 29, 2022 18:24

aduh95 requested a review from arcanis June 30, 2022 11:45

aduh95 commented Jul 7, 2022

View reviewed changes

styfle reviewed Jul 8, 2022

View reviewed changes

aduh95 added 2 commits July 23, 2022 14:54

Merge branch 'main'

12a91f9

COREPACK_NO_LOOKUP -> COREPACK_DEFAULT_TO_LATEST

4ce5e2c

styfle reviewed Jul 24, 2022

View reviewed changes

arcanis approved these changes Aug 4, 2022

View reviewed changes

Merge branch 'main' into dynamic-default-version

9563bcd

aduh95 merged commit 055b928 into main Aug 19, 2022

aduh95 deleted the dynamic-default-version branch August 19, 2022 21:53

github-actions bot mentioned this pull request Aug 19, 2022

chore(main): release 0.13.0 #162

Merged

andersk mentioned this pull request Oct 11, 2022

Project Status (experimental to stable?) #104

Open

andersk mentioned this pull request Sep 18, 2023

[Feature] Allow opting-out of vendoring berry's files yarnpkg/berry#969

Closed

2 tasks

aduh95 mentioned this pull request Dec 19, 2023

enable corepack by default nodejs/node#50963

Open

aduh95 mentioned this pull request Jan 24, 2024

Update Corepack instructions in installation.md pnpm/pnpm.io#504

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: download the latest version instead of a pinned one #134

feat: download the latest version instead of a pinned one #134

aduh95 commented Jun 26, 2022 •

edited

Loading

aduh95 Jul 7, 2022

styfle Jul 8, 2022

aduh95 Jul 8, 2022

styfle Jul 9, 2022

aduh95 Jul 9, 2022

styfle Jul 9, 2022

aduh95 Jul 23, 2022 •

edited

Loading

styfle Jul 24, 2022 •

edited

Loading

aduh95 Jul 24, 2022 •

edited

Loading

styfle Jul 24, 2022

styfle Jul 8, 2022

aduh95 Jul 8, 2022

styfle Jul 9, 2022 •

edited

Loading

aduh95 Jul 9, 2022

styfle Jul 9, 2022

aduh95 Jul 9, 2022 •

edited

Loading

styfle Jul 24, 2022 •

edited

Loading

aduh95 Jul 24, 2022

arcanis left a comment

zkochan commented Aug 20, 2022

andersk commented Aug 23, 2022

aduh95 commented Aug 24, 2022

andersk commented Aug 24, 2022

ljharb commented Aug 24, 2022

aduh95 commented Aug 24, 2022

styfle commented Aug 24, 2022

aduh95 commented Aug 24, 2022

andersk commented Aug 24, 2022 •

edited

Loading

aduh95 commented Aug 24, 2022 •

edited

Loading

		@@ -106,6 +113,8 @@ This command will retrieve the given package manager from the specified archive

		- `COREPACK_ENABLE_NETWORK` can be set to `0` to prevent Corepack from accessing the network (in which case you'll be responsible for hydrating the package manager versions that will be required for the projects you'll run, using `corepack hydrate`).

		- `COREPACK_NO_LOOKUP` can be set in order to instruct Corepack not to lookup on the remote registry for the latest version of the selected package manager.


		The Known Good Releases can be updated system-wide using the `--activate` flag from the `corepack prepare` and `corepack hydrate` commands.
		If there is no Known Good Release for the requested package manager, Corepack

feat: download the latest version instead of a pinned one #134

feat: download the latest version instead of a pinned one #134

Conversation

aduh95 commented Jun 26, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aduh95 Jul 23, 2022 • edited Loading

Choose a reason for hiding this comment

styfle Jul 24, 2022 • edited Loading

Choose a reason for hiding this comment

aduh95 Jul 24, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

styfle Jul 9, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aduh95 Jul 9, 2022 • edited Loading

Choose a reason for hiding this comment

styfle Jul 24, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arcanis left a comment

Choose a reason for hiding this comment

zkochan commented Aug 20, 2022

andersk commented Aug 23, 2022

aduh95 commented Aug 24, 2022

andersk commented Aug 24, 2022

ljharb commented Aug 24, 2022

aduh95 commented Aug 24, 2022

styfle commented Aug 24, 2022

aduh95 commented Aug 24, 2022

andersk commented Aug 24, 2022 • edited Loading

aduh95 commented Aug 24, 2022 • edited Loading

aduh95 commented Jun 26, 2022 •

edited

Loading

aduh95 Jul 23, 2022 •

edited

Loading

styfle Jul 24, 2022 •

edited

Loading

aduh95 Jul 24, 2022 •

edited

Loading

styfle Jul 9, 2022 •

edited

Loading

aduh95 Jul 9, 2022 •

edited

Loading

styfle Jul 24, 2022 •

edited

Loading

andersk commented Aug 24, 2022 •

edited

Loading

aduh95 commented Aug 24, 2022 •

edited

Loading