From 2df8ce463c19c50cd5996ad897590c1ae2f4ad65 Mon Sep 17 00:00:00 2001 From: Reinier Criel Date: Fri, 27 Mar 2026 13:17:58 -0700 Subject: [PATCH] Adapt per review --- packages/safe-chain/src/config/configFile.js | 84 +++++-------------- packages/safe-chain/src/main.js | 4 +- .../interceptors/npm/parseNpmPackageUrl.js | 25 ++++-- .../src/registryProxy/registryProxy.js | 17 ++-- .../src/scanning/newPackagesDatabase.js | 51 ++++++++++- .../src/scanning/newPackagesDatabase.spec.js | 53 +++++++----- 6 files changed, 127 insertions(+), 107 deletions(-) diff --git a/packages/safe-chain/src/config/configFile.js b/packages/safe-chain/src/config/configFile.js index 0246fa9..b421fde 100644 --- a/packages/safe-chain/src/config/configFile.js +++ b/packages/safe-chain/src/config/configFile.js @@ -203,70 +203,6 @@ export function readDatabaseFromLocalCache() { } } -/** - * @param {import("../api/aikido.js").NewPackageEntry[]} data - * @param {string | number} version - * - * @returns {void} - */ -export function writeNewPackagesListToLocalCache(data, version) { - try { - const listPath = getNewPackagesListPath(); - const versionPath = getNewPackagesListVersionPath(); - - fs.writeFileSync(listPath, JSON.stringify(data)); - fs.writeFileSync(versionPath, version.toString()); - } catch { - ui.writeWarning( - "Failed to write new packages list to local cache, next time the list will be fetched from the server again." - ); - } -} - -/** - * @returns {{newPackagesList: import("../api/aikido.js").NewPackageEntry[] | null, version: string | null}} - */ -export function readNewPackagesListFromLocalCache() { - try { - const listPath = getNewPackagesListPath(); - if (!fs.existsSync(listPath)) { - return { newPackagesList: null, version: null }; - } - - const data = fs.readFileSync(listPath, "utf8"); - const newPackagesList = JSON.parse(data); - const versionPath = getNewPackagesListVersionPath(); - let version = null; - if (fs.existsSync(versionPath)) { - version = fs.readFileSync(versionPath, "utf8").trim(); - } - return { newPackagesList, version }; - } catch { - ui.writeWarning( - "Failed to read new packages list from local cache. Continuing without local cache." - ); - return { newPackagesList: null, version: null }; - } -} - -/** - * @returns {string} - */ -function getNewPackagesListPath() { - const safeChainDir = getSafeChainDirectory(); - const ecosystem = getEcoSystem(); - return path.join(safeChainDir, `newPackagesList_${ecosystem}.json`); -} - -/** - * @returns {string} - */ -function getNewPackagesListVersionPath() { - const safeChainDir = getSafeChainDirectory(); - const ecosystem = getEcoSystem(); - return path.join(safeChainDir, `newPackagesList_version_${ecosystem}.txt`); -} - /** * @returns {SafeChainConfig} */ @@ -312,6 +248,24 @@ function getDatabaseVersionPath() { return path.join(aikidoDir, `version_${ecosystem}.txt`); } +/** + * @returns {string} + */ +export function getNewPackagesListPath() { + const safeChainDir = getSafeChainDirectory(); + const ecosystem = getEcoSystem(); + return path.join(safeChainDir, `newPackagesList_${ecosystem}.json`); +} + +/** + * @returns {string} + */ +export function getNewPackagesListVersionPath() { + const safeChainDir = getSafeChainDirectory(); + const ecosystem = getEcoSystem(); + return path.join(safeChainDir, `newPackagesList_version_${ecosystem}.txt`); +} + /** * @returns {string} */ @@ -332,7 +286,7 @@ function getConfigFilePath() { /** * @returns {string} */ -function getSafeChainDirectory() { +export function getSafeChainDirectory() { const homeDir = os.homedir(); const safeChainDir = path.join(homeDir, ".safe-chain"); diff --git a/packages/safe-chain/src/main.js b/packages/safe-chain/src/main.js index d9e5417..74f8a25 100644 --- a/packages/safe-chain/src/main.js +++ b/packages/safe-chain/src/main.js @@ -64,11 +64,11 @@ export async function main(args) { // Write all buffered logs ui.writeBufferedLogsAndStopBuffering(); - if (!proxy.verifyNoMaliciousPackages()) { + if (proxy.hasBlockedMaliciousPackages()) { return 1; } - if (!proxy.verifyNoMinimumAgeBlockedRequests()) { + if (proxy.hasBlockedMinimumAgeRequests()) { return 1; } diff --git a/packages/safe-chain/src/registryProxy/interceptors/npm/parseNpmPackageUrl.js b/packages/safe-chain/src/registryProxy/interceptors/npm/parseNpmPackageUrl.js index 5d12c0e..13cb99a 100644 --- a/packages/safe-chain/src/registryProxy/interceptors/npm/parseNpmPackageUrl.js +++ b/packages/safe-chain/src/registryProxy/interceptors/npm/parseNpmPackageUrl.js @@ -5,16 +5,29 @@ */ export function parseNpmPackageUrl(url, registry) { let packageName, version; - const urlWithoutParams = url.split("?")[0].split("#")[0]; + let parsedUrl; - if (!registry || !urlWithoutParams.endsWith(".tgz")) { + try { + parsedUrl = new URL(url); + } catch { return { packageName, version }; } - const registryIndex = urlWithoutParams.indexOf(registry); - const afterRegistry = decodeURIComponent(urlWithoutParams.substring( - registryIndex + registry.length + 1 - )); // +1 to skip the slash + const pathname = parsedUrl.pathname; + + if (!registry || !pathname.endsWith(".tgz")) { + return { packageName, version }; + } + + const registryPrefix = `${registry}/`; + const urlAfterProtocol = `${parsedUrl.host}${pathname}`; + if (!urlAfterProtocol.startsWith(registryPrefix)) { + return { packageName, version }; + } + + const afterRegistry = decodeURIComponent( + urlAfterProtocol.substring(registryPrefix.length) + ); const separatorIndex = afterRegistry.indexOf("/-/"); if (separatorIndex === -1) { diff --git a/packages/safe-chain/src/registryProxy/registryProxy.js b/packages/safe-chain/src/registryProxy/registryProxy.js index 4adba61..81b265d 100644 --- a/packages/safe-chain/src/registryProxy/registryProxy.js +++ b/packages/safe-chain/src/registryProxy/registryProxy.js @@ -28,8 +28,8 @@ export function createSafeChainProxy() { return { startServer: () => startServer(server), stopServer: () => stopServer(server), - verifyNoMaliciousPackages, - verifyNoMinimumAgeBlockedRequests, + hasBlockedMaliciousPackages, + hasBlockedMinimumAgeRequests, hasSuppressedVersions: getHasSuppressedVersions, }; } @@ -198,10 +198,9 @@ function onMinimumAgeRequestBlocked(packageName, version, url) { state.blockedMinimumAgeRequests.push({ packageName, version, url }); } -function verifyNoMaliciousPackages() { +function hasBlockedMaliciousPackages() { if (state.blockedRequests.length === 0) { - // No malicious packages were blocked, so nothing to block - return true; + return false; } ui.emptyLine(); @@ -220,12 +219,12 @@ function verifyNoMaliciousPackages() { ui.writeExitWithoutInstallingMaliciousPackages(); ui.emptyLine(); - return false; + return true; } -function verifyNoMinimumAgeBlockedRequests() { +function hasBlockedMinimumAgeRequests() { if (state.blockedMinimumAgeRequests.length === 0) { - return true; + return false; } ui.emptyLine(); @@ -252,5 +251,5 @@ function verifyNoMinimumAgeBlockedRequests() { ); ui.emptyLine(); - return false; + return true; } diff --git a/packages/safe-chain/src/scanning/newPackagesDatabase.js b/packages/safe-chain/src/scanning/newPackagesDatabase.js index acda1e9..6a74656 100644 --- a/packages/safe-chain/src/scanning/newPackagesDatabase.js +++ b/packages/safe-chain/src/scanning/newPackagesDatabase.js @@ -1,10 +1,11 @@ +import fs from "fs"; import { fetchNewPackagesList, fetchNewPackagesListVersion, } from "../api/aikido.js"; import { - readNewPackagesListFromLocalCache, - writeNewPackagesListToLocalCache, + getNewPackagesListPath, + getNewPackagesListVersionPath, } from "../config/configFile.js"; import { ui } from "../environment/userInteraction.js"; import { @@ -138,3 +139,49 @@ async function getNewPackagesList() { throw error; } } + +/** + * @param {import("../api/aikido.js").NewPackageEntry[]} data + * @param {string | number} version + * + * @returns {void} + */ +export function writeNewPackagesListToLocalCache(data, version) { + try { + const listPath = getNewPackagesListPath(); + const versionPath = getNewPackagesListVersionPath(); + + fs.writeFileSync(listPath, JSON.stringify(data)); + fs.writeFileSync(versionPath, version.toString()); + } catch { + ui.writeWarning( + "Failed to write new packages list to local cache, next time the list will be fetched from the server again." + ); + } +} + +/** + * @returns {{newPackagesList: import("../api/aikido.js").NewPackageEntry[] | null, version: string | null}} + */ +export function readNewPackagesListFromLocalCache() { + try { + const listPath = getNewPackagesListPath(); + if (!fs.existsSync(listPath)) { + return { newPackagesList: null, version: null }; + } + + const data = fs.readFileSync(listPath, "utf8"); + const newPackagesList = JSON.parse(data); + const versionPath = getNewPackagesListVersionPath(); + let version = null; + if (fs.existsSync(versionPath)) { + version = fs.readFileSync(versionPath, "utf8").trim(); + } + return { newPackagesList, version }; + } catch { + ui.writeWarning( + "Failed to read new packages list from local cache. Continuing without local cache." + ); + return { newPackagesList: null, version: null }; + } +} diff --git a/packages/safe-chain/src/scanning/newPackagesDatabase.spec.js b/packages/safe-chain/src/scanning/newPackagesDatabase.spec.js index 58c9a74..29f04d5 100644 --- a/packages/safe-chain/src/scanning/newPackagesDatabase.spec.js +++ b/packages/safe-chain/src/scanning/newPackagesDatabase.spec.js @@ -1,9 +1,10 @@ import { describe, it, mock, beforeEach } from "node:test"; import assert from "node:assert"; +import fs from "fs"; +import path from "path"; +import os from "os"; // --- shared mutable state for mocks --- -let cachedList = null; -let cachedVersion = null; let fetchedList = []; let fetchedVersion = "etag-1"; let fetchVersionResult = "etag-1"; @@ -13,6 +14,7 @@ let writeWarningCalls = []; let fetchListError = null; let fetchVersionError = null; let importCounter = 0; +let testHomeDir = ""; mock.module("../api/aikido.js", { namedExports: { @@ -36,16 +38,6 @@ mock.module("../api/aikido.js", { }, }); -mock.module("../config/configFile.js", { - namedExports: { - readNewPackagesListFromLocalCache: () => ({ - newPackagesList: cachedList, - version: cachedVersion, - }), - writeNewPackagesListToLocalCache: () => {}, - }, -}); - mock.module("../environment/userInteraction.js", { namedExports: { ui: { @@ -66,8 +58,6 @@ mock.module("../config/settings.js", { describe("newPackagesDatabase", async () => { beforeEach(() => { - cachedList = null; - cachedVersion = null; fetchedList = []; fetchedVersion = "etag-1"; fetchVersionResult = "etag-1"; @@ -76,6 +66,13 @@ describe("newPackagesDatabase", async () => { writeWarningCalls = []; fetchListError = null; fetchVersionError = null; + testHomeDir = path.join( + os.tmpdir(), + `safe-chain-new-packages-db-${process.pid}-${importCounter}` + ); + fs.rmSync(testHomeDir, { recursive: true, force: true }); + fs.mkdirSync(testHomeDir, { recursive: true }); + process.env.HOME = testHomeDir; }); async function openNewPackagesDatabase() { @@ -93,6 +90,19 @@ describe("newPackagesDatabase", async () => { return Math.floor((Date.now() - hours * 3600 * 1000) / 1000); } + function writeCachedList(list, version) { + const safeChainDir = path.join(testHomeDir, ".safe-chain"); + fs.mkdirSync(safeChainDir, { recursive: true }); + fs.writeFileSync( + path.join(safeChainDir, `newPackagesList_${ecosystem}.json`), + JSON.stringify(list) + ); + fs.writeFileSync( + path.join(safeChainDir, `newPackagesList_version_${ecosystem}.txt`), + version + ); + } + describe("isNewlyReleasedPackage", () => { it("returns true for a package released within the age threshold", async () => { fetchedList = [ @@ -171,10 +181,9 @@ describe("newPackagesDatabase", async () => { describe("caching behaviour", () => { it("uses local cache when etag matches", async () => { - cachedList = [ + writeCachedList([ { package_name: "cached-pkg", version: "1.0.0", released_on: hoursAgo(1), scraped_on: hoursAgo(1) }, - ]; - cachedVersion = "etag-1"; + ], "etag-1"); fetchVersionResult = "etag-1"; // fetchedList is empty — if we used the remote list, the lookup would return false fetchedList = []; @@ -184,10 +193,9 @@ describe("newPackagesDatabase", async () => { }); it("fetches fresh list when etag does not match", async () => { - cachedList = [ + writeCachedList([ { package_name: "stale-pkg", version: "1.0.0", released_on: hoursAgo(1), scraped_on: hoursAgo(1) }, - ]; - cachedVersion = "etag-old"; + ], "etag-old"); fetchVersionResult = "etag-new"; fetchedList = [ { package_name: "fresh-pkg", version: "2.0.0", released_on: hoursAgo(1), scraped_on: hoursAgo(1) }, @@ -199,15 +207,14 @@ describe("newPackagesDatabase", async () => { }); it("falls back to local cache when fetch fails", async () => { - cachedList = [ + writeCachedList([ { package_name: "cached-pkg", version: "1.0.0", released_on: hoursAgo(1), scraped_on: hoursAgo(1), }, - ]; - cachedVersion = "etag-old"; + ], "etag-old"); fetchVersionResult = "etag-new"; fetchListError = new Error("Network error");