From c5be86b7af3c3fb55b7b61c5057d0518b8dfd43e Mon Sep 17 00:00:00 2001 From: Leonardo Rocha Date: Fri, 26 Apr 2024 17:12:04 +0200 Subject: [PATCH 1/4] feat: COREPACK_INTEGRITY_KEYS should support `0` and `false` values to disable integrity checks (#468) --- README.md | 5 +++-- sources/corepackUtils.ts | 8 +++++++- sources/npmRegistryUtils.ts | 3 ++- tests/corepackUtils.test.ts | 40 +++++++++++++++++++++++++++++++++++++ 4 files changed, 52 insertions(+), 4 deletions(-) create mode 100644 tests/corepackUtils.test.ts diff --git a/README.md b/README.md index 42dc8c190..610ed04b1 100644 --- a/README.md +++ b/README.md @@ -296,8 +296,9 @@ same major line. Should you need to upgrade to a new major, use an explicit - `HTTP_PROXY`, `HTTPS_PROXY`, and `NO_PROXY` are supported through [`node-proxy-agent`](https://github.com/TooTallNate/node-proxy-agent). -- `COREPACK_INTEGRITY_KEYS` can be set to an empty string to instruct Corepack - to skip integrity checks, or a JSON string containing custom keys. +- `COREPACK_INTEGRITY_KEYS` can be set to an empty string, `0`, or `false` to + instruct Corepack to skip integrity checks, or to a JSON string containing + custom keys. ## Troubleshooting diff --git a/sources/corepackUtils.ts b/sources/corepackUtils.ts index 7d5e1f518..83c310e8e 100644 --- a/sources/corepackUtils.ts +++ b/sources/corepackUtils.ts @@ -283,7 +283,7 @@ export async function installVersion(installTarget: string, locator: Locator, {s if (!build[1]) { const registry = getRegistryFromPackageManagerSpec(spec); - if (registry.type === `npm` && !registry.bin && process.env.COREPACK_INTEGRITY_KEYS !== ``) { + if (registry.type === `npm` && !registry.bin && !shouldSkipIntegrityCheck()) { if (signatures! == null || integrity! == null) ({signatures, integrity} = (await npmRegistryUtils.fetchTarballURLAndSignature(registry.package, version))); @@ -432,3 +432,9 @@ export async function runVersion(locator: Locator, installSpec: InstallSpec & {s // the stack trace of the package manager. process.nextTick(Module.runMain, binPath); } + +export function shouldSkipIntegrityCheck() { + return [``, `0`, `false`].includes( + process.env.COREPACK_INTEGRITY_KEYS?.toLowerCase().trim() + ); +} diff --git a/sources/npmRegistryUtils.ts b/sources/npmRegistryUtils.ts index 40199472c..c9bd698ab 100644 --- a/sources/npmRegistryUtils.ts +++ b/sources/npmRegistryUtils.ts @@ -4,6 +4,7 @@ import {createVerify} from 'crypto'; import defaultConfig from '../config.json'; import * as httpUtils from './httpUtils'; +import { shouldSkipIntegrityCheck } from './corepackUtils'; // load abbreviated metadata as that's all we need for these calls // see: https://github.com/npm/registry/blob/cfe04736f34db9274a780184d1cdb2fb3e4ead2a/docs/responses/package-metadata.md @@ -63,7 +64,7 @@ export async function fetchLatestStableVersion(packageName: string) { const {version, dist: {integrity, signatures}} = metadata; - if (process.env.COREPACK_INTEGRITY_KEYS !== ``) { + if (!shouldSkipIntegrityCheck()) { verifySignature({ packageName, version, integrity, signatures, diff --git a/tests/corepackUtils.test.ts b/tests/corepackUtils.test.ts new file mode 100644 index 000000000..8d08864f0 --- /dev/null +++ b/tests/corepackUtils.test.ts @@ -0,0 +1,40 @@ +import { describe, it, expect } from "@jest/globals"; + +import { shouldSkipIntegrityCheck } from "../sources/corepackUtils"; + +describe(`corepack utils shouldSkipIntegrityCheck`, () => { + it(`should return false if COREPACK_INTEGRITY_KEYS env is not set`, () => { + delete process.env.COREPACK_INTEGRITY_KEYS; + expect(shouldSkipIntegrityCheck()).toBe(false); + }); + + it(`should return true if COREPACK_INTEGRITY_KEYS env is set to 0`, () => { + process.env.COREPACK_INTEGRITY_KEYS = `0`; + expect(shouldSkipIntegrityCheck()).toBe(true); + }); + + it(`should return true if COREPACK_INTEGRITY_KEYS env is set to false`, () => { + process.env.COREPACK_INTEGRITY_KEYS = `false`; + expect(shouldSkipIntegrityCheck()).toBe(true); + }); + + it(`should return true if COREPACK_INTEGRITY_KEYS env is set to FALSE`, () => { + process.env.COREPACK_INTEGRITY_KEYS = `FALSE`; + expect(shouldSkipIntegrityCheck()).toBe(true); + }); + + it(`should return true if COREPACK_INTEGRITY_KEYS env is set to an empty string`, () => { + process.env.COREPACK_INTEGRITY_KEYS = ``; + expect(shouldSkipIntegrityCheck()).toBe(true); + }); + + it(`should return true if COREPACK_INTEGRITY_KEYS env is set to a string with leading spaces`, () => { + process.env.COREPACK_INTEGRITY_KEYS = ` false `; + expect(shouldSkipIntegrityCheck()).toBe(true); + }); + + it(`should return false if COREPACK_INTEGRITY_KEYS env is set to any other value`, () => { + process.env.COREPACK_INTEGRITY_KEYS = JSON.stringify({ foo: `bar` }); + expect(shouldSkipIntegrityCheck()).toBe(false); + }); +}); From a30eba2f13e0cf0974e8e4e95c837d758216c36c Mon Sep 17 00:00:00 2001 From: Leonardo Rocha Date: Fri, 3 May 2024 16:25:41 +0200 Subject: [PATCH 2/4] feat: remove 'false' as possible value to COREPACK_INTEGRITY_KEYS --- README.md | 2 +- sources/corepackUtils.ts | 4 ++-- tests/corepackUtils.test.ts | 18 ++++-------------- 3 files changed, 7 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 610ed04b1..f72f7f71e 100644 --- a/README.md +++ b/README.md @@ -296,7 +296,7 @@ same major line. Should you need to upgrade to a new major, use an explicit - `HTTP_PROXY`, `HTTPS_PROXY`, and `NO_PROXY` are supported through [`node-proxy-agent`](https://github.com/TooTallNate/node-proxy-agent). -- `COREPACK_INTEGRITY_KEYS` can be set to an empty string, `0`, or `false` to +- `COREPACK_INTEGRITY_KEYS` can be set to an empty string or `0` to instruct Corepack to skip integrity checks, or to a JSON string containing custom keys. diff --git a/sources/corepackUtils.ts b/sources/corepackUtils.ts index 83c310e8e..39595b23c 100644 --- a/sources/corepackUtils.ts +++ b/sources/corepackUtils.ts @@ -434,7 +434,7 @@ export async function runVersion(locator: Locator, installSpec: InstallSpec & {s } export function shouldSkipIntegrityCheck() { - return [``, `0`, `false`].includes( - process.env.COREPACK_INTEGRITY_KEYS?.toLowerCase().trim() + return [``, `0`].includes( + process.env.COREPACK_INTEGRITY_KEYS?.toLowerCase().trim(), ); } diff --git a/tests/corepackUtils.test.ts b/tests/corepackUtils.test.ts index 8d08864f0..f1f858074 100644 --- a/tests/corepackUtils.test.ts +++ b/tests/corepackUtils.test.ts @@ -1,6 +1,6 @@ -import { describe, it, expect } from "@jest/globals"; +import {describe, it, expect} from '@jest/globals'; -import { shouldSkipIntegrityCheck } from "../sources/corepackUtils"; +import {shouldSkipIntegrityCheck} from '../sources/corepackUtils'; describe(`corepack utils shouldSkipIntegrityCheck`, () => { it(`should return false if COREPACK_INTEGRITY_KEYS env is not set`, () => { @@ -13,28 +13,18 @@ describe(`corepack utils shouldSkipIntegrityCheck`, () => { expect(shouldSkipIntegrityCheck()).toBe(true); }); - it(`should return true if COREPACK_INTEGRITY_KEYS env is set to false`, () => { - process.env.COREPACK_INTEGRITY_KEYS = `false`; - expect(shouldSkipIntegrityCheck()).toBe(true); - }); - - it(`should return true if COREPACK_INTEGRITY_KEYS env is set to FALSE`, () => { - process.env.COREPACK_INTEGRITY_KEYS = `FALSE`; - expect(shouldSkipIntegrityCheck()).toBe(true); - }); - it(`should return true if COREPACK_INTEGRITY_KEYS env is set to an empty string`, () => { process.env.COREPACK_INTEGRITY_KEYS = ``; expect(shouldSkipIntegrityCheck()).toBe(true); }); it(`should return true if COREPACK_INTEGRITY_KEYS env is set to a string with leading spaces`, () => { - process.env.COREPACK_INTEGRITY_KEYS = ` false `; + process.env.COREPACK_INTEGRITY_KEYS = ` `; expect(shouldSkipIntegrityCheck()).toBe(true); }); it(`should return false if COREPACK_INTEGRITY_KEYS env is set to any other value`, () => { - process.env.COREPACK_INTEGRITY_KEYS = JSON.stringify({ foo: `bar` }); + process.env.COREPACK_INTEGRITY_KEYS = JSON.stringify({foo: `bar`}); expect(shouldSkipIntegrityCheck()).toBe(false); }); }); From f68401741737356d36fd43213d0713459e7d418b Mon Sep 17 00:00:00 2001 From: Leonardo Rocha Date: Wed, 8 May 2024 13:29:13 +0200 Subject: [PATCH 3/4] feat: COREPACK_INTEGRITY_KEYS case sensitive validation --- sources/corepackUtils.ts | 5 ++--- tests/corepackUtils.test.ts | 5 ----- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/sources/corepackUtils.ts b/sources/corepackUtils.ts index 39595b23c..3af96c044 100644 --- a/sources/corepackUtils.ts +++ b/sources/corepackUtils.ts @@ -434,7 +434,6 @@ export async function runVersion(locator: Locator, installSpec: InstallSpec & {s } export function shouldSkipIntegrityCheck() { - return [``, `0`].includes( - process.env.COREPACK_INTEGRITY_KEYS?.toLowerCase().trim(), - ); + return process.env.COREPACK_INTEGRITY_KEYS === `` + || process.env.COREPACK_INTEGRITY_KEYS === `0`; } diff --git a/tests/corepackUtils.test.ts b/tests/corepackUtils.test.ts index f1f858074..8e3cc30b1 100644 --- a/tests/corepackUtils.test.ts +++ b/tests/corepackUtils.test.ts @@ -18,11 +18,6 @@ describe(`corepack utils shouldSkipIntegrityCheck`, () => { expect(shouldSkipIntegrityCheck()).toBe(true); }); - it(`should return true if COREPACK_INTEGRITY_KEYS env is set to a string with leading spaces`, () => { - process.env.COREPACK_INTEGRITY_KEYS = ` `; - expect(shouldSkipIntegrityCheck()).toBe(true); - }); - it(`should return false if COREPACK_INTEGRITY_KEYS env is set to any other value`, () => { process.env.COREPACK_INTEGRITY_KEYS = JSON.stringify({foo: `bar`}); expect(shouldSkipIntegrityCheck()).toBe(false); From 4498f7b775e9bd24f26f67b9281705e266446809 Mon Sep 17 00:00:00 2001 From: Leonardo Rocha Date: Wed, 8 May 2024 14:43:45 +0200 Subject: [PATCH 4/4] chore: fixing lint errors --- sources/npmRegistryUtils.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/sources/npmRegistryUtils.ts b/sources/npmRegistryUtils.ts index c9bd698ab..c1df215bb 100644 --- a/sources/npmRegistryUtils.ts +++ b/sources/npmRegistryUtils.ts @@ -1,10 +1,10 @@ -import {UsageError} from 'clipanion'; -import {createVerify} from 'crypto'; +import {UsageError} from 'clipanion'; +import {createVerify} from 'crypto'; -import defaultConfig from '../config.json'; +import defaultConfig from '../config.json'; -import * as httpUtils from './httpUtils'; -import { shouldSkipIntegrityCheck } from './corepackUtils'; +import {shouldSkipIntegrityCheck} from './corepackUtils'; +import * as httpUtils from './httpUtils'; // load abbreviated metadata as that's all we need for these calls // see: https://github.com/npm/registry/blob/cfe04736f34db9274a780184d1cdb2fb3e4ead2a/docs/responses/package-metadata.md