From d84270be8dd17bbc8b6feaffad7f9a8bd544bcbc Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Sat, 28 Mar 2026 16:51:33 -0700 Subject: [PATCH] Adapt per review --- .../pipInterceptor.customRegistries.spec.js | 60 ++++++++++++------- .../interceptors/pip/pipInterceptor.js | 24 +++++--- .../pipInterceptor.packageDownload.spec.js | 19 +++++- .../src/scanning/packageNameVariants.js | 9 ++- 4 files changed, 76 insertions(+), 36 deletions(-) diff --git a/packages/safe-chain/src/registryProxy/interceptors/pip/pipInterceptor.customRegistries.spec.js b/packages/safe-chain/src/registryProxy/interceptors/pip/pipInterceptor.customRegistries.spec.js index 9a5cd91..c7ad597 100644 --- a/packages/safe-chain/src/registryProxy/interceptors/pip/pipInterceptor.customRegistries.spec.js +++ b/packages/safe-chain/src/registryProxy/interceptors/pip/pipInterceptor.customRegistries.spec.js @@ -2,7 +2,7 @@ import { describe, it, mock } from "node:test"; import assert from "node:assert"; describe("pipInterceptor custom registries", async () => { - let lastPackage; + let scannedPackages; let malwareResponse = false; let customRegistries = []; @@ -27,7 +27,7 @@ describe("pipInterceptor custom registries", async () => { mock.module("../../../scanning/audit/index.js", { namedExports: { isMalwarePackage: async (packageName, version) => { - lastPackage = { packageName, version }; + scannedPackages.push({ packageName, version }); return malwareResponse; }, }, @@ -46,6 +46,7 @@ describe("pipInterceptor custom registries", async () => { }); it("should parse package from custom registry URL", async () => { + scannedPackages = []; customRegistries = ["my-custom-registry.example.com"]; const url = "https://my-custom-registry.example.com/packages/xx/yy/foobar-1.2.3.tar.gz"; @@ -55,13 +56,16 @@ describe("pipInterceptor custom registries", async () => { await interceptor.handleRequest(url); - assert.deepEqual(lastPackage, { - packageName: "foobar", - version: "1.2.3", - }); + assert.ok( + scannedPackages.some( + ({ packageName, version }) => + packageName === "foobar" && version === "1.2.3" + ) + ); }); it("should parse wheel package from custom registry URL", async () => { + scannedPackages = []; customRegistries = ["private-pypi.internal.com"]; const url = "https://private-pypi.internal.com/packages/foo_bar-2.0.0-py3-none-any.whl"; @@ -71,10 +75,12 @@ describe("pipInterceptor custom registries", async () => { await interceptor.handleRequest(url); - assert.deepEqual(lastPackage, { - packageName: "foo-bar", - version: "2.0.0", - }); + assert.ok( + scannedPackages.some( + ({ packageName, version }) => + packageName === "foo-bar" && version === "2.0.0" + ) + ); }); it("should handle multiple custom registries", async () => { @@ -96,6 +102,7 @@ describe("pipInterceptor custom registries", async () => { }); it("should block malicious package from custom registry", async () => { + scannedPackages = []; customRegistries = ["my-custom-registry.example.com"]; malwareResponse = true; @@ -115,6 +122,7 @@ describe("pipInterceptor custom registries", async () => { }); it("should still work with known registries when custom registries are set", async () => { + scannedPackages = []; customRegistries = ["my-custom-registry.example.com"]; const url = @@ -126,10 +134,12 @@ describe("pipInterceptor custom registries", async () => { await interceptor.handleRequest(url); - assert.deepEqual(lastPackage, { - packageName: "foobar", - version: "1.2.3", - }); + assert.ok( + scannedPackages.some( + ({ packageName, version }) => + packageName === "foobar" && version === "1.2.3" + ) + ); }); it("should not create interceptor for unknown registry when custom registries are set", () => { @@ -152,6 +162,7 @@ describe("pipInterceptor custom registries", async () => { }); it("should parse .whl.metadata from custom registry", async () => { + scannedPackages = []; customRegistries = ["private-pypi.internal.com"]; const url = "https://private-pypi.internal.com/packages/foo_bar-2.0.0-py3-none-any.whl.metadata"; @@ -161,13 +172,16 @@ describe("pipInterceptor custom registries", async () => { await interceptor.handleRequest(url); - assert.deepEqual(lastPackage, { - packageName: "foo-bar", - version: "2.0.0", - }); + assert.ok( + scannedPackages.some( + ({ packageName, version }) => + packageName === "foo-bar" && version === "2.0.0" + ) + ); }); it("should parse .tar.gz.metadata from custom registry", async () => { + scannedPackages = []; customRegistries = ["private-pypi.internal.com"]; const url = "https://private-pypi.internal.com/packages/foo_bar-2.0.0.tar.gz.metadata"; @@ -177,9 +191,11 @@ describe("pipInterceptor custom registries", async () => { await interceptor.handleRequest(url); - assert.deepEqual(lastPackage, { - packageName: "foo-bar", - version: "2.0.0", - }); + assert.ok( + scannedPackages.some( + ({ packageName, version }) => + packageName === "foo-bar" && version === "2.0.0" + ) + ); }); }); diff --git a/packages/safe-chain/src/registryProxy/interceptors/pip/pipInterceptor.js b/packages/safe-chain/src/registryProxy/interceptors/pip/pipInterceptor.js index 5194bec..abdda17 100644 --- a/packages/safe-chain/src/registryProxy/interceptors/pip/pipInterceptor.js +++ b/packages/safe-chain/src/registryProxy/interceptors/pip/pipInterceptor.js @@ -1,8 +1,10 @@ import { + ECOSYSTEM_PY, getPipCustomRegistries, skipMinimumPackageAge, } from "../../../config/settings.js"; import { isMalwarePackage } from "../../../scanning/audit/index.js"; +import { getEquivalentPackageNames } from "../../../scanning/packageNameVariants.js"; import { openNewPackagesDatabase } from "../../../scanning/newPackagesListCache.js"; import { interceptRequests } from "../interceptorBuilder.js"; import { isExcludedFromMinimumPackageAge } from "../minimumPackageAgeExclusions.js"; @@ -50,14 +52,21 @@ function createPipRequestHandler(registry) { registry ); - // PyPI treats hyphens and underscores as equivalent distribution names. - const hyphenName = packageName?.includes("_") - ? packageName.replace(/_/g, "-") - : packageName; + if (!packageName) { + return; + } - const isMalicious = - await isMalwarePackage(packageName, version) || - await isMalwarePackage(hyphenName, version); + const equivalentPackageNames = getEquivalentPackageNames( + packageName, + ECOSYSTEM_PY + ); + let isMalicious = false; + for (const equivalentPackageName of equivalentPackageNames) { + if (await isMalwarePackage(equivalentPackageName, version)) { + isMalicious = true; + break; + } + } if (isMalicious) { reqContext.blockMalware(packageName, version); @@ -65,7 +74,6 @@ function createPipRequestHandler(registry) { } if ( - packageName && version && !skipMinimumPackageAge() && !isExcludedFromMinimumPackageAge(packageName) diff --git a/packages/safe-chain/src/registryProxy/interceptors/pip/pipInterceptor.packageDownload.spec.js b/packages/safe-chain/src/registryProxy/interceptors/pip/pipInterceptor.packageDownload.spec.js index 61f279e..d6fdec6 100644 --- a/packages/safe-chain/src/registryProxy/interceptors/pip/pipInterceptor.packageDownload.spec.js +++ b/packages/safe-chain/src/registryProxy/interceptors/pip/pipInterceptor.packageDownload.spec.js @@ -2,13 +2,13 @@ import { describe, it, mock } from "node:test"; import assert from "node:assert"; describe("pipInterceptor", async () => { - let lastPackage; + let scannedPackages; let malwareResponse = false; mock.module("../../../scanning/audit/index.js", { namedExports: { isMalwarePackage: async (packageName, version) => { - lastPackage = { packageName, version }; + scannedPackages.push({ packageName, version }); return malwareResponse; }, }, @@ -111,12 +111,24 @@ describe("pipInterceptor", async () => { parserCases.forEach(({ url, expected }, index) => { it(`should parse URL ${index + 1}: ${url}`, async () => { + scannedPackages = []; const interceptor = pipInterceptorForUrl(url); assert.ok(interceptor, "Interceptor should be created for known pip registry"); await interceptor.handleRequest(url); - assert.deepEqual(lastPackage, expected); + if (expected.packageName === undefined) { + assert.deepEqual(scannedPackages, []); + return; + } + + assert.ok( + scannedPackages.some( + ({ packageName, version }) => + packageName === expected.packageName && + version === expected.version + ) + ); }); }); @@ -127,6 +139,7 @@ describe("pipInterceptor", async () => { }); it("should block malicious package", async () => { + scannedPackages = []; const url = "https://files.pythonhosted.org/packages/xx/yy/malicious_package-1.0.0.tar.gz"; malwareResponse = true; diff --git a/packages/safe-chain/src/scanning/packageNameVariants.js b/packages/safe-chain/src/scanning/packageNameVariants.js index 19c0c32..97db91b 100644 --- a/packages/safe-chain/src/scanning/packageNameVariants.js +++ b/packages/safe-chain/src/scanning/packageNameVariants.js @@ -10,7 +10,10 @@ export function getEquivalentPackageNames(packageName, ecosystem) { return [packageName]; } - return [...new Set([packageName, ...["-", "_", "."].map((separator) => - packageName.replaceAll(/[._-]/g, separator) - )])]; + const pythonSeparatorPattern = /[._-]/g; + const hyphenName = packageName.replaceAll(pythonSeparatorPattern, "-"); + const underscoreName = packageName.replaceAll(pythonSeparatorPattern, "_"); + const dotName = packageName.replaceAll(pythonSeparatorPattern, "."); + + return [...new Set([packageName, hyphenName, underscoreName, dotName])]; }