Skip to content

fix(npmRegistryUtils): point users at the real env vars when integrity check fails#854

Draft
spokodev wants to merge 1 commit into
nodejs:mainfrom
spokodev:fix/integrity-check-error-message-env-var-names
Draft

fix(npmRegistryUtils): point users at the real env vars when integrity check fails#854
spokodev wants to merge 1 commit into
nodejs:mainfrom
spokodev:fix/integrity-check-error-message-env-var-names

Conversation

@spokodev
Copy link
Copy Markdown

The signature-failure path in `fetchLatestStableVersion` tells users they can disable the integrity check with `COREPACK_INTEGRITY_CHECK=0` and fall back to the bundled latest release with `COREPACK_USE_LATEST=0`. Neither variable exists anywhere else in the codebase. The runtime reads `COREPACK_INTEGRITY_KEYS` (via `shouldSkipIntegrityCheck` in `sources/corepackUtils.ts`) and `COREPACK_DEFAULT_TO_LATEST` (in `sources/corepackUtils.ts` and `sources/Engine.ts`), and those are the names the README advertises.

A user who hits the error and follows the in-message suggestion sets two variables that do nothing and keeps hitting the same wall.

```shell
$ rg COREPACK_INTEGRITY_CHECK sources/ README.md
sources/npmRegistryUtils.ts:83:...COREPACK_INTEGRITY_CHECK to 0 in your env...

$ rg COREPACK_USE_LATEST sources/ README.md
sources/npmRegistryUtils.ts:83:...COREPACK_USE_LATEST to 0

$ rg COREPACK_INTEGRITY_KEYS sources/ README.md
sources/corepackUtils.ts:451: return process.env.COREPACK_INTEGRITY_KEYS === ``
sources/corepackUtils.ts:452: || process.env.COREPACK_INTEGRITY_KEYS === `0`;
sources/npmRegistryUtils.ts:43: const {npm: trustedKeys} = process.env.COREPACK_INTEGRITY_KEYS ?
sources/npmRegistryUtils.ts:44: JSON.parse(process.env.COREPACK_INTEGRITY_KEYS) as ...
README.md:...

$ rg COREPACK_DEFAULT_TO_LATEST sources/ README.md
sources/Engine.ts:182: if (process.env.COREPACK_DEFAULT_TO_LATEST === `0`) {
sources/corepackUtils.ts:336: if (... process.env.COREPACK_DEFAULT_TO_LATEST !== `0`) {
README.md:...
```

Change

`sources/npmRegistryUtils.ts:83` - reworded the thrown error to point at `COREPACK_INTEGRITY_KEYS` and `COREPACK_DEFAULT_TO_LATEST`. The behaviour of `fetchLatestStableVersion` is otherwise unchanged.

Tests

Two cases in `tests/npmRegistryUtils.test.ts` under a new `fetchLatestStableVersion` describe block:

  • positive + negative axis in one test: when verification fails the thrown error contains the real names (`COREPACK_INTEGRITY_KEYS to 0`, `COREPACK_DEFAULT_TO_LATEST to 0`) and does not contain the phantom names. Fails on `main` (the assertion for the new strings does not match the current message); passes on the branch.
  • regression guard: when `COREPACK_INTEGRITY_KEYS=0` is set, `fetchLatestStableVersion` skips verification and returns the resolved version, so the env var name we now advertise is the one the runtime actually obeys.

Verification

`corepack yarn vitest run tests/npmRegistryUtils.test.ts` -> 9 passed (7 existing + 2 new).
`corepack yarn lint` clean on the changed files.
`corepack yarn typecheck` clean.

Fixes #849

…y check fails

When signature verification fails in fetchLatestStableVersion, the
error tells users they can disable the check by setting
COREPACK_INTEGRITY_CHECK=0 or fall back to the bundled latest release
by setting COREPACK_USE_LATEST=0. Neither of those variables exists
anywhere else in the codebase. The runtime actually reads
COREPACK_INTEGRITY_KEYS (in shouldSkipIntegrityCheck) and
COREPACK_DEFAULT_TO_LATEST (in corepackUtils.ts and Engine.ts), and
both are the names documented in the README.

So a user who hits the error and follows the suggestion sets two
variables that have no effect, then keeps hitting the same wall.

This patch rewords the error to name the variables the runtime
actually checks. Behaviour of fetchLatestStableVersion is otherwise
unchanged.

Tests cover both axes: the failing path now mentions the real names
and no longer mentions the phantom ones; and the happy path that
sets COREPACK_INTEGRITY_KEYS=0 still skips verification and returns
the resolved version (regression guard for the env-var name we now
advertise).

Fixes nodejs#849

Signed-off-by: Yarchik <spoko.dev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Non-existent environment variables COREPACK_INTEGRITY_CHECK and COREPACK_USE_LATEST referenced in error message

1 participant