From 293089462430fe807f9cacfd207d4f85a51e1fc2 Mon Sep 17 00:00:00 2001 From: Sander Declerck Date: Tue, 21 Apr 2026 09:26:07 +0200 Subject: [PATCH 1/2] Fix concurrency bug leading to multiple fetches of the malware database --- .../src/scanning/malwareDatabase.js | 72 +++++++++---------- .../src/scanning/newPackagesListCache.js | 34 ++++----- 2 files changed, 51 insertions(+), 55 deletions(-) diff --git a/packages/safe-chain/src/scanning/malwareDatabase.js b/packages/safe-chain/src/scanning/malwareDatabase.js index 4aba43c..afc8b98 100644 --- a/packages/safe-chain/src/scanning/malwareDatabase.js +++ b/packages/safe-chain/src/scanning/malwareDatabase.js @@ -15,8 +15,12 @@ import { getEcoSystem, ECOSYSTEM_PY } from "../config/settings.js"; * @property {function(string, string): boolean} isMalware */ -/** @type {MalwareDatabase | null} */ -let cachedMalwareDatabase = null; +// Caching the Promise (rather than the resolved database) prevents duplicate fetches. If we cached the resolved +// value, multiple callers could pass the null-check before the first fetch completes (because each `await` yields +// control back to the event loop, allowing other callers to run). Since the Promise assignment is synchronous, all +// concurrent callers see it immediately and share a single fetch. +/** @type {Promise | null} */ +let cachedMalwareDatabasePromise = null; /** * Normalize package name for comparison. @@ -34,45 +38,41 @@ function normalizePackageName(name) { return name; } -export async function openMalwareDatabase() { - if (cachedMalwareDatabase) { - return cachedMalwareDatabase; - } +export function openMalwareDatabase() { + if (!cachedMalwareDatabasePromise) { + cachedMalwareDatabasePromise = getMalwareDatabase().then((malwareDatabase) => { + /** + * @param {string} name + * @param {string} version + * @returns {string} + */ + function getPackageStatus(name, version) { + const normalizedName = normalizePackageName(name); + const packageData = malwareDatabase.find( + (pkg) => { + const normalizedPkgName = normalizePackageName(pkg.package_name); + return normalizedPkgName === normalizedName && + (pkg.version === version || pkg.version === "*"); + } + ); - const malwareDatabase = await getMalwareDatabase(); + if (!packageData) { + return MALWARE_STATUS_OK; + } - /** - * @param {string} name - * @param {string} version - * @returns {string} - */ - function getPackageStatus(name, version) { - const normalizedName = normalizePackageName(name); - const packageData = malwareDatabase.find( - (pkg) => { - const normalizedPkgName = normalizePackageName(pkg.package_name); - return normalizedPkgName === normalizedName && - (pkg.version === version || pkg.version === "*"); + return packageData.reason; } - ); - if (!packageData) { - return MALWARE_STATUS_OK; - } - - return packageData.reason; + return { + getPackageStatus, + isMalware: (name, version) => { + const status = getPackageStatus(name, version); + return isMalwareStatus(status); + }, + }; + }); } - - // This implicitly caches the malware database - // that's closed over by the getPackageStatus function - cachedMalwareDatabase = { - getPackageStatus, - isMalware: (name, version) => { - const status = getPackageStatus(name, version); - return isMalwareStatus(status); - }, - }; - return cachedMalwareDatabase; + return cachedMalwareDatabasePromise; } /** diff --git a/packages/safe-chain/src/scanning/newPackagesListCache.js b/packages/safe-chain/src/scanning/newPackagesListCache.js index dfac247..b6c990e 100644 --- a/packages/safe-chain/src/scanning/newPackagesListCache.js +++ b/packages/safe-chain/src/scanning/newPackagesListCache.js @@ -16,30 +16,26 @@ import { warnOnceAboutUnavailableDatabase } from "./newPackagesDatabaseWarnings. */ // Shared per-process cache to avoid rebuilding the same feed-backed database on each request. -/** @type {NewPackagesDatabase | null} */ -let cachedNewPackagesDatabase = null; +// Caching the Promise (rather than the resolved database) prevents duplicate fetches. If we cached the resolved +// value, multiple callers could pass the null-check before the first fetch completes (because each `await` yields +// control back to the event loop, allowing other callers to run). Since the Promise assignment is synchronous, all +// concurrent callers see it immediately and share a single fetch. +/** @type {Promise | null} */ +let cachedNewPackagesDatabasePromise = null; /** * @returns {Promise} */ -export async function openNewPackagesDatabase() { - if (cachedNewPackagesDatabase) { - return cachedNewPackagesDatabase; +export function openNewPackagesDatabase() { + if (!cachedNewPackagesDatabasePromise) { + cachedNewPackagesDatabasePromise = getNewPackagesList() + .then((newPackagesList) => buildNewPackagesDatabase(newPackagesList)) + .catch((/** @type {any} */ error) => { + warnOnceAboutUnavailableDatabase(error); + return { isNewlyReleasedPackage: () => false }; + }); } - - /** @type {import("../api/aikido.js").NewPackageEntry[]} */ - let newPackagesList; - - try { - newPackagesList = await getNewPackagesList(); - } catch (/** @type {any} */ error) { - warnOnceAboutUnavailableDatabase(error); - cachedNewPackagesDatabase = { isNewlyReleasedPackage: () => false }; - return cachedNewPackagesDatabase; - } - - cachedNewPackagesDatabase = buildNewPackagesDatabase(newPackagesList); - return cachedNewPackagesDatabase; + return cachedNewPackagesDatabasePromise; } /** From 9fae225277b769824d74125f6973c4b871b894fa Mon Sep 17 00:00:00 2001 From: Sander Declerck Date: Tue, 21 Apr 2026 09:31:26 +0200 Subject: [PATCH 2/2] Make sure rejected promise is not cached in malware list / new packages cache --- packages/safe-chain/src/scanning/malwareDatabase.js | 5 ++++- packages/safe-chain/src/scanning/newPackagesListCache.js | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/safe-chain/src/scanning/malwareDatabase.js b/packages/safe-chain/src/scanning/malwareDatabase.js index afc8b98..0eccc88 100644 --- a/packages/safe-chain/src/scanning/malwareDatabase.js +++ b/packages/safe-chain/src/scanning/malwareDatabase.js @@ -65,11 +65,14 @@ export function openMalwareDatabase() { return { getPackageStatus, - isMalware: (name, version) => { + isMalware: (/** @type {string} */ name, /** @type {string} */ version) => { const status = getPackageStatus(name, version); return isMalwareStatus(status); }, }; + }).catch((error) => { + cachedMalwareDatabasePromise = null; + throw error; }); } return cachedMalwareDatabasePromise; diff --git a/packages/safe-chain/src/scanning/newPackagesListCache.js b/packages/safe-chain/src/scanning/newPackagesListCache.js index b6c990e..418dbdd 100644 --- a/packages/safe-chain/src/scanning/newPackagesListCache.js +++ b/packages/safe-chain/src/scanning/newPackagesListCache.js @@ -32,6 +32,7 @@ export function openNewPackagesDatabase() { .then((newPackagesList) => buildNewPackagesDatabase(newPackagesList)) .catch((/** @type {any} */ error) => { warnOnceAboutUnavailableDatabase(error); + cachedNewPackagesDatabasePromise = null; return { isNewlyReleasedPackage: () => false }; }); }