Patching corepack to work with private registries

[mikeduminy]

trafficstars

TL;DR suggestion: read engine.config from environment vars.

Hi there 👋

Where I work we want to use corepack to manage package manager versions, however we find that we need to patch it in a few ways. I'm hoping we can find a solution that doesn't rely on patching as it leads to more maintenance over time and I believe that many other companies would benefit from solving this.

Default headers

 var DEFAULT_HEADERS = {
-  [`Accept`]: `application/vnd.npm.install-v1+json`
+  [`Accept`]: `application/vnd.npm.install-v1+json;q=1.0, application/json;1=0.8`
 };

Our private registry expects specific headers.

A simple solution here would be to add an environment variable to be spread onto these headers. WDYT?

Yarn config

         ">=2.0.0": {
           name: "yarn",
-          url: "https://repo.yarnpkg.com/{}/packages/yarnpkg-cli/bin/yarn.js",
-          bin: [
-            "yarn",
-            "yarnpkg"
-          ],
+          url: "https://<our-private-repo>/@yarnpkg/cli-dist/-/cli-dist-{}.tgz",
+          bin: {
+            yarn: "./bin/yarn.js",
+            yarnpkg: "./bin/yarn.js"
+          },

Yarn is unique in that it is the only package manager that is not fetched from npm. If it was then we could easily use the COREPACK_NPM_REGISTRY environment variable to change this to our private repo. Today we don't have the ability to modify this via env vars.

Due to the way our private repo mirrors external repositories we also need to change the location of the bins for yarn 2+. There is no way to address this other than changing our private repo or patching corepack.

Suggestion 1: Allow custom configs

Since the Engine already supports a config being provided, what if we implemented a new environment variable that can point to the location of a custom config: COREPACK_CONFIG. This could even be spread onto the default config, thereby allowing users to only change what is needed.

Pros:

• Super simple to do
• Already supported in Engine
• Only adds one more env var
• Addresses bin file patching

Cons:

• Restricts the schema of the config in the future
  • Although it could be versioned, or simply fail

Suggestion 2: Add more env vars

For the registry we could add a yarn specific setting, or allow a more dynamic resolution such as (replacing this part of corepackUtils.installVersion):

  const packageManagerRegistry = `COREPACK_${locator.name.toUpperCase()}_REGISTRY`;
  const registryUrl = process.env[packageManagerRegistry] || (process.env.COREPACK_NPM_REGISTRY ?
    spec.url.replace(
      npmRegistryUtils.DEFAULT_NPM_REGISTRY_URL,
      () => process.env.COREPACK_NPM_REGISTRY!,
    ) :
    spec.url);
  const url = registryUrl.replace(`{}`, version);

This would allow us to specify COREPACK_YARN_REGISTRY and open the door to other ones too, such as COREPACK_PNPM_REGISTRY. Granted, it is not super clean.

Pros:

• Targeted changes to config values
• Config schema is unaffected

Cons:

• A bit messy, adds several more env variables
• Doesn't address bin file patching

Conclusion

I'm happy to implement any / all of these, but I'd like to first get some feedback on the design.

[mikeduminy]

An alternative would be to standardise the registries so that even yarn is loaded from NPM via the @yarnpkg/cli-dist package. This would make the COREPACK_NPM_REGISTRY work for yarn too.

[arcanis]

Couple of notes:

• Of course the preferred solution here should be to allowlist the repo.yarnpkg.com domain. The rest of the discussion is around "how to workaround that the ideal solution isn't achievable due to non-Corepack-related reasons" - so that weights a bit in the level of complexity it's reasonable to add into Corepack, imo.

  • I'd prefer to keep the configuration file an implementation detail, as I could imagine it changing from time to time (perhaps its content, or perhaps we'd add a new range, etc). Allowing unconstrained modification sounds risky.

  • I considered using @yarnpkg/cli-dist, but it'd make it complicated to compute the same checksum between people using old Corepack (which relies on the url) and possible new Corepack (which would rely on the package).

• Some options I could be more favorable to:

  • A COREPACK_YARN_PATTERN variable to override the default one (it could be awkward if we add another range later on with a different url pattern, but I don't see that happening)
  • A corepack.config.js file where you could export a list of regexp to apply to any requested urls (but it requires a configuration file, which we currently avoid to allow easier configuration in cloud environments).

• It's probably fine to add application/json;q=0.8 to the headers by default, it seems a small change and makes sense from a purely semantical point of view.

• Regarding your bin patching, it's not clear to me why this would be needed, can you detail more?

[mikeduminy]

Thanks for taking the time to consider this and reply. I'll get back to you on the bin patching question, but the rest of your suggestions seem fine to me.

I originally suggested the config approach because it would avoid these hyper specific env vars which could make it tougher to maintain in the future. These things are always a trade-off and I value your perspective here.

One quick question:

I considered using @yarnpkg/cli-dist, but it'd make it complicated to compute the same checksum between people using old Corepack (which relies on the url) and possible new Corepack (which would rely on the package).

What would the effect of this checksum mismatch be? Would it basically cause corepack to re-download a previously downloaded yarn, or would it hard fail for users?

[adityasrini]

Co-sign and wanted to say that I'm running into this issue at my workplace.

It's probably fine to add application/json;q=0.8 to the headers by default, it seems a small change and makes sense from a purely semantical point of view.

@arcanis Happy to make a relatively small PR contribution if it would incrementally solve this!

[rotu]

An alternative would be to standardise the registries so that even yarn is loaded from NPM via the @yarnpkg/cli-dist package. This would make the COREPACK_NPM_REGISTRY work for yarn too.

I think a better way to specify this is to override yarn with a package specifier like yarn@https://our-private-repo/@yarnpkg/cli-dist/-/cli-dist-{}.tgz. It's much more explicit than an environment variable. Unfortunately this seems currently broken (#313).

[simonhaenisch]

I believe the first problem with the headers was solved by #314?

What would the effect of this checksum mismatch be? Would it basically cause corepack to re-download a previously downloaded yarn, or would it hard fail for users?

I actually just ran into that cause I ran corepack use yarn@4 in a project to update the version which was saved with one checksum (from repo.yarnpkg.com), then our CI used Corepack with the above patch to use our internal registry where @yarnpkg/[email protected] apparently has a different checksum, so it failed to run yarn install -- immutable with

Internal Error: Mismatch hashes. Expected 6d855253732ba8d231b6cd917961654f6c8439164c962a4e355c9c58360ebe44, got 061d21cf42190f6a03a8b8c2242a7ee893c30845f91933ce4a4e2a65c88c02f6
      at installVersion (/usr/lib/node_modules/corepack/dist/lib/corepack.cjs:39179:11)
      at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
      at async Engine.ensurePackageManager (/usr/lib/node_modules/corepack/dist/lib/corepack.cjs:39370:32)
      at async executePackageManagerRequest (/usr/lib/node_modules/corepack/dist/lib/corepack.cjs:40198:23)
      at async BinaryCommand.validateAndExecute (/usr/lib/node_modules/corepack/dist/lib/corepack.cjs:37297:22)
      at async Cli.run (/usr/lib/node_modules/corepack/dist/lib/corepack.cjs:38272:18)
      at async Object.runMain (/usr/lib/node_modules/corepack/dist/lib/corepack.cjs:40240:12)

We definitely need a way to configure those registries without having to patch corepack which is not feasible to do on every dev's machine.

[christianblos]

I would love to see this feature as well. The lack of specifying a custom repository for yarn (including username and password for connecting to the repository) prevents me from using corepack at the moment.

[lsrocha]

@mikeduminy The issues reported here were fixed in versions 0.22 and 0.26.

Setting a private registry using COREPACK_NPM_REGISTRY now leads to Yarn Berry being fetched correctly.

[lsrocha]

I actually just ran into that cause I ran corepack use yarn@4 in a project to update the version which was saved with one checksum (from repo.yarnpkg.com), then our CI used Corepack with the above patch to use our internal registry where @yarnpkg/[email protected] apparently has a different checksum, so it failed to run yarn install -- immutable

@simonhaenisch The checksum error was fixed in version 0.28