From 589ea7fc3fcd0f543a035c10a46abd7dcd5a8f68 Mon Sep 17 00:00:00 2001 From: Marc Bernard <59966492+mbtools@users.noreply.github.com> Date: Fri, 20 Dec 2024 08:21:09 -0500 Subject: [PATCH] chore: move tarball utils to core (#4972) * chore: move tarball utils to core * update middleware, storage * better regex * Fix redos vulnerability --- .changeset/breezy-geckos-search.md | 9 +++ packages/core/core/src/index.ts | 2 + packages/core/core/src/pkg-utils.ts | 13 --- packages/core/core/src/tarball-utils.ts | 43 ++++++++++ packages/core/core/test/pkg-utils.spec.ts | 16 ---- packages/core/core/test/tarball-utils.spec.ts | 81 +++++++++++++++++++ .../tarball/src/getLocalRegistryTarballUri.ts | 10 +-- packages/core/tarball/src/index.ts | 2 +- .../tests/getLocalRegistryTarballUri.spec.ts | 80 +++++++++--------- packages/middleware/src/middlewares/allow.ts | 5 +- packages/store/src/storage.ts | 7 +- packages/utils/src/index.ts | 1 - packages/utils/src/middleware-utils.ts | 10 --- packages/utils/test/middleware-utils.spec.ts | 20 ----- 14 files changed, 187 insertions(+), 112 deletions(-) create mode 100644 .changeset/breezy-geckos-search.md create mode 100644 packages/core/core/src/tarball-utils.ts create mode 100644 packages/core/core/test/tarball-utils.spec.ts delete mode 100644 packages/utils/src/middleware-utils.ts delete mode 100644 packages/utils/test/middleware-utils.spec.ts diff --git a/.changeset/breezy-geckos-search.md b/.changeset/breezy-geckos-search.md new file mode 100644 index 000000000..9b77c61cd --- /dev/null +++ b/.changeset/breezy-geckos-search.md @@ -0,0 +1,9 @@ +--- +'@verdaccio/tarball': patch +'@verdaccio/core': patch +'@verdaccio/utils': patch +'@verdaccio/middleware': patch +'@verdaccio/store': patch +--- + +chore: move tarball utils to core diff --git a/packages/core/core/src/index.ts b/packages/core/core/src/index.ts index ad00e98cd..feba0513a 100644 --- a/packages/core/core/src/index.ts +++ b/packages/core/core/src/index.ts @@ -6,6 +6,7 @@ import * as pluginUtils from './plugin-utils'; import * as searchUtils from './search-utils'; import * as streamUtils from './stream-utils'; import * as stringUtils from './string-utils'; +import * as tarballUtils from './tarball-utils'; import * as validatioUtils from './validation-utils'; import * as warningUtils from './warning-utils'; @@ -41,4 +42,5 @@ export { constants, pluginUtils, warningUtils, + tarballUtils, }; diff --git a/packages/core/core/src/pkg-utils.ts b/packages/core/core/src/pkg-utils.ts index 1c4d1570f..b441ef65e 100644 --- a/packages/core/core/src/pkg-utils.ts +++ b/packages/core/core/src/pkg-utils.ts @@ -1,22 +1,9 @@ import semver from 'semver'; -import { URL } from 'url'; import { Manifest } from '@verdaccio/types'; import { DIST_TAGS } from './constants'; -/** - * Extract the tarball name from a registry dist url - * 'https://registry.npmjs.org/test/-/test-0.0.2.tgz' - * @param tarball tarball url - * @returns tarball filename - */ -export function extractTarballName(tarball: string) { - const urlObject: any = new URL(tarball); - const filename = urlObject.pathname.replace(/^.*\//, ''); - return filename; -} - /** * Function filters out bad semver versions and sorts the array. * @return {Array} sorted Array diff --git a/packages/core/core/src/tarball-utils.ts b/packages/core/core/src/tarball-utils.ts new file mode 100644 index 000000000..0921ae842 --- /dev/null +++ b/packages/core/core/src/tarball-utils.ts @@ -0,0 +1,43 @@ +import { URL } from 'url'; + +/** + * Return package version from tarball name + * + * test-1.2.4.tgz -> 1.2.4 + * @param {String} fileName + * @returns {String} + */ +export function getVersionFromTarball(fileName: string): string | void { + const groups = fileName.replace(/\.tgz$/, '').match(/^[^/]+-(\d+\.\d+\.\d+.*)/); + + return groups !== null ? groups[1] : undefined; +} + +/** + * Extract the tarball name from a registry dist url + * + * https://registry.npmjs.org/test/-/test-0.0.2.tgz -> test-0.0.2.tgz + * @param tarball tarball url + * @returns tarball filename + */ +export function extractTarballFromUrl(url: string): string { + const urlObject = new URL(url); + return urlObject.pathname.replace(/^.*\//, ''); +} + +/** + * Build the tarball filename from paackage name and version + * + * test, 1.2.4 -> test-1.2.4.tgz + * @scope/name, 1.2.4 -> name-1.2.4.tgz + * @param name package name + * @param version package version + * @returns tarball filename + */ +export function composeTarballFromPackage(name: string, version: string): string { + if (name.includes('/')) { + return `${name.split('/')[1]}-${version}.tgz`; + } else { + return `${name}-${version}.tgz`; + } +} diff --git a/packages/core/core/test/pkg-utils.spec.ts b/packages/core/core/test/pkg-utils.spec.ts index 34420650e..7f1ce949d 100644 --- a/packages/core/core/test/pkg-utils.spec.ts +++ b/packages/core/core/test/pkg-utils.spec.ts @@ -3,22 +3,6 @@ import { describe, expect, test } from 'vitest'; import { DIST_TAGS, pkgUtils } from '../src'; describe('pkg-utils', () => { - test('extractTarballName', () => { - expect(pkgUtils.extractTarballName('https://registry.npmjs.org/test/-/test-0.0.2.tgz')).toBe( - 'test-0.0.2.tgz' - ); - }); - - test('extractTarballName with no tarball should not fails', () => { - expect(pkgUtils.extractTarballName('https://registry.npmjs.org/')).toBe(''); - }); - - test('extractTarballName fails', () => { - expect(() => - pkgUtils.extractTarballName('xxxxregistry.npmjs.org/test/-/test-0.0.2.tgz') - ).toThrow(); - }); - test('getLatest fails if no versions', () => { expect(() => // @ts-expect-error diff --git a/packages/core/core/test/tarball-utils.spec.ts b/packages/core/core/test/tarball-utils.spec.ts new file mode 100644 index 000000000..e752a7f06 --- /dev/null +++ b/packages/core/core/test/tarball-utils.spec.ts @@ -0,0 +1,81 @@ +import { describe, expect, test } from 'vitest'; + +import { + composeTarballFromPackage, + extractTarballFromUrl, + getVersionFromTarball, +} from '../src/tarball-utils'; + +describe('Utilities', () => { + describe('getVersionFromTarball', () => { + test('should get the right version', () => { + const simpleName = 'test-name-4.2.12.tgz'; + const complexName = 'test-5.6.4-beta.2.tgz'; + const otherComplexName = 'test-3.5.0-6.tgz'; + expect(getVersionFromTarball(simpleName)).toEqual('4.2.12'); + expect(getVersionFromTarball(complexName)).toEqual('5.6.4-beta.2'); + expect(getVersionFromTarball(otherComplexName)).toEqual('3.5.0-6'); + }); + + test('should fail at incorrect tarball name', () => { + expect(getVersionFromTarball('incorrectName')).toBeUndefined(); + expect(getVersionFromTarball('test-1.2.tgz')).toBeUndefined(); + }); + }); +}); + +describe('extractTarballFromUrl', () => { + const metadata: any = { + name: 'npm_test', + versions: { + '1.0.0': { + dist: { + tarball: 'http://registry.org/npm_test/-/npm_test-1.0.0.tgz', + }, + }, + '1.0.1': { + dist: { + tarball: 'https://localhost:4873/npm_test/-/npm_test-1.0.1.tgz', + }, + }, + '1.0.2': { + dist: { + tarball: 'https://localhost/npm_test-1.0.2.tgz', + }, + }, + '1.0.3': { + dist: { + tarball: 'http://registry.org/@org/npm_test/-/npm_test-1.0.3.tgz', + }, + }, + }, + }; + + test('should return only name of tarball', () => { + expect(extractTarballFromUrl(metadata.versions['1.0.0'].dist.tarball)).toEqual( + 'npm_test-1.0.0.tgz' + ); + expect(extractTarballFromUrl(metadata.versions['1.0.1'].dist.tarball)).toEqual( + 'npm_test-1.0.1.tgz' + ); + expect(extractTarballFromUrl(metadata.versions['1.0.2'].dist.tarball)).toEqual( + 'npm_test-1.0.2.tgz' + ); + expect(extractTarballFromUrl(metadata.versions['1.0.3'].dist.tarball)).toEqual( + 'npm_test-1.0.3.tgz' + ); + }); + + test('without tarball should not fails', () => { + expect(extractTarballFromUrl('https://registry.npmjs.org/')).toBe(''); + }); + + test('fails with incomplete URL', () => { + expect(() => extractTarballFromUrl('xxxxregistry.npmjs.org/test/-/test-0.0.2.tgz')).toThrow(); + }); +}); + +test('composeTarballFromPackage - should return filename of tarball', () => { + expect(composeTarballFromPackage('npm_test', '1.0.0')).toEqual('npm_test-1.0.0.tgz'); + expect(composeTarballFromPackage('@mbtools/npm_test', '1.0.1')).toEqual('npm_test-1.0.1.tgz'); +}); diff --git a/packages/core/tarball/src/getLocalRegistryTarballUri.ts b/packages/core/tarball/src/getLocalRegistryTarballUri.ts index f70fb0026..4eda0934c 100644 --- a/packages/core/tarball/src/getLocalRegistryTarballUri.ts +++ b/packages/core/tarball/src/getLocalRegistryTarballUri.ts @@ -1,15 +1,11 @@ import buildDebug from 'debug'; -import URL from 'url'; +import { tarballUtils } from '@verdaccio/core'; import { RequestOptions } from '@verdaccio/url'; import { getPublicUrl } from '@verdaccio/url'; -const debug = buildDebug('verdaccio:core:url'); +const debug = buildDebug('verdaccio:core:tarball'); -export function extractTarballFromUrl(url: string): string { - // @ts-ignore - return URL.parse(url).pathname.replace(/^.*\//, ''); -} /** * Filter a tarball url. * @param {*} uri @@ -26,7 +22,7 @@ export function getLocalRegistryTarballUri( if (!currentHost) { return uri; } - const tarballName = extractTarballFromUrl(uri); + const tarballName = tarballUtils.extractTarballFromUrl(uri); debug('tarball name %o', tarballName); // header only set with proxy that setup with HTTPS const domainRegistry = getPublicUrl(urlPrefix || '', requestOptions); diff --git a/packages/core/tarball/src/index.ts b/packages/core/tarball/src/index.ts index a72248298..4039b4c86 100644 --- a/packages/core/tarball/src/index.ts +++ b/packages/core/tarball/src/index.ts @@ -4,7 +4,7 @@ export { convertDistRemoteToLocalTarballUrls, convertDistVersionToLocalTarballsUrl, } from './convertDistRemoteToLocalTarballUrls'; -export { extractTarballFromUrl, getLocalRegistryTarballUri } from './getLocalRegistryTarballUri'; +export { getLocalRegistryTarballUri } from './getLocalRegistryTarballUri'; export { getTarballDetails, TarballDetails } from './getTarballDetails'; export { RequestOptions }; diff --git a/packages/core/tarball/tests/getLocalRegistryTarballUri.spec.ts b/packages/core/tarball/tests/getLocalRegistryTarballUri.spec.ts index dda315178..4f08792d2 100644 --- a/packages/core/tarball/tests/getLocalRegistryTarballUri.spec.ts +++ b/packages/core/tarball/tests/getLocalRegistryTarballUri.spec.ts @@ -1,46 +1,52 @@ import { describe, expect, test } from 'vitest'; -import { extractTarballFromUrl } from '../src'; +import { getLocalRegistryTarballUri } from '../src/getLocalRegistryTarballUri'; -describe('extractTarballFromUrl', () => { - const metadata: any = { - name: 'npm_test', - versions: { - '1.0.0': { - dist: { - tarball: 'http://registry.org/npm_test/-/npm_test-1.0.0.tgz', - }, +describe('getLocalRegistryTarballUri', () => { + test('should return the right tarball uri', () => { + const uri = 'http://registry.org/npm_test/-/npm_test-1.0.0.tgz'; + const pkgName = 'npm_test'; + const requestOptions = { + host: 'localhost:4873', + protocol: 'http', + headers: { + host: 'localhost:4873', }, - '1.0.1': { - dist: { - tarball: 'npm_test-1.0.1.tgz', - }, - }, - '1.0.2': { - dist: { - tarball: 'https://localhost/npm_test-1.0.2.tgz', - }, - }, - '1.0.3': { - dist: { - tarball: 'http://registry.org/@org/npm_test/-/npm_test-1.0.3.tgz', - }, - }, - }, - }; + }; + const urlPrefix = '/'; + expect(getLocalRegistryTarballUri(uri, pkgName, requestOptions, urlPrefix)).toEqual( + 'http://localhost:4873/npm_test/-/npm_test-1.0.0.tgz' + ); + }); - test('should return only name of tarball', () => { - expect(extractTarballFromUrl(metadata.versions['1.0.0'].dist.tarball)).toEqual( - 'npm_test-1.0.0.tgz' + test('should return the right tarball uri with prefix', () => { + const uri = 'http://registry.org/npm_test/-/npm_test-1.0.0.tgz'; + const pkgName = 'npm_test'; + const requestOptions = { + host: 'localhost:4873', + protocol: 'http', + headers: { + host: 'localhost:4873', + }, + }; + const urlPrefix = '/local/'; + expect(getLocalRegistryTarballUri(uri, pkgName, requestOptions, urlPrefix)).toEqual( + 'http://localhost:4873/local/npm_test/-/npm_test-1.0.0.tgz' ); - expect(extractTarballFromUrl(metadata.versions['1.0.1'].dist.tarball)).toEqual( - 'npm_test-1.0.1.tgz' - ); - expect(extractTarballFromUrl(metadata.versions['1.0.2'].dist.tarball)).toEqual( - 'npm_test-1.0.2.tgz' - ); - expect(extractTarballFromUrl(metadata.versions['1.0.3'].dist.tarball)).toEqual( - 'npm_test-1.0.3.tgz' + }); + + test('should return the right tarball uri without prefix', () => { + const uri = 'http://registry.org/npm_test/-/npm_test-1.0.0.tgz'; + const pkgName = 'npm_test'; + const requestOptions = { + host: 'localhost:4873', + protocol: 'http', + headers: { + host: 'localhost:4873', + }, + }; + expect(getLocalRegistryTarballUri(uri, pkgName, requestOptions, undefined)).toEqual( + 'http://localhost:4873/npm_test/-/npm_test-1.0.0.tgz' ); }); }); diff --git a/packages/middleware/src/middlewares/allow.ts b/packages/middleware/src/middlewares/allow.ts index 94720afc9..23a4c553f 100644 --- a/packages/middleware/src/middlewares/allow.ts +++ b/packages/middleware/src/middlewares/allow.ts @@ -1,7 +1,6 @@ import buildDebug from 'debug'; -import { API_ERROR, errorUtils } from '@verdaccio/core'; -import { getVersionFromTarball } from '@verdaccio/utils'; +import { API_ERROR, errorUtils, tarballUtils } from '@verdaccio/core'; import { $NextFunctionVer, $RequestExtend, $ResponseExtend } from '../types'; @@ -24,7 +23,7 @@ export function allow( ? `@${req.params.scope}/${req.params.package}` : req.params.package; const packageVersion = req.params.filename - ? getVersionFromTarball(req.params.filename) + ? tarballUtils.getVersionFromTarball(req.params.filename) : req.params.version ? req.params.version : undefined; diff --git a/packages/store/src/storage.ts b/packages/store/src/storage.ts index 94aca68c5..e98614f99 100644 --- a/packages/store/src/storage.ts +++ b/packages/store/src/storage.ts @@ -19,9 +19,9 @@ import { SUPPORT_ERRORS, USERS, errorUtils, - pkgUtils, pluginUtils, searchUtils, + tarballUtils, validatioUtils, } from '@verdaccio/core'; import { asyncLoadPlugin } from '@verdaccio/loaders'; @@ -39,7 +39,6 @@ import { TarballDetails, convertDistRemoteToLocalTarballUrls, convertDistVersionToLocalTarballsUrl, - extractTarballFromUrl, getTarballDetails, } from '@verdaccio/tarball'; import { @@ -1424,7 +1423,7 @@ class Storage { // if uploaded tarball has a different shasum, it's very likely that we // have some kind of error if (validatioUtils.isObject(metadata.dist) && _.isString(metadata.dist.tarball)) { - const tarball = extractTarballFromUrl(metadata.dist.tarball); + const tarball = tarballUtils.extractTarballFromUrl(metadata.dist.tarball); if (validatioUtils.isObject(data._attachments[tarball])) { if ( _.isNil(data._attachments[tarball].shasum) === false && @@ -1979,7 +1978,7 @@ class Storage { cacheManifest.versions[versionId] = version; if (version?.dist?.tarball) { - const filename = pkgUtils.extractTarballName(version.dist.tarball); + const filename = tarballUtils.extractTarballFromUrl(version.dist.tarball); // store a fast access to the dist file by tarball name // it does NOT overwrite any existing records if (_.isNil(cacheManifest?._distfiles[filename])) { diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index 0a982c4a4..deb2eb338 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -3,4 +3,3 @@ export * from './utils'; export * from './crypto-utils'; export * from './replace-lodash'; export * from './matcher'; -export * from './middleware-utils'; diff --git a/packages/utils/src/middleware-utils.ts b/packages/utils/src/middleware-utils.ts deleted file mode 100644 index 2d17d2197..000000000 --- a/packages/utils/src/middleware-utils.ts +++ /dev/null @@ -1,10 +0,0 @@ -/** - * return package version from tarball name - * @param {String} name - * @returns {String} - */ -export function getVersionFromTarball(name: string): string | void { - const groups = name.match(/.+-(\d.+)\.tgz/); - - return groups !== null ? groups[1] : undefined; -} diff --git a/packages/utils/test/middleware-utils.spec.ts b/packages/utils/test/middleware-utils.spec.ts deleted file mode 100644 index 44fd03e26..000000000 --- a/packages/utils/test/middleware-utils.spec.ts +++ /dev/null @@ -1,20 +0,0 @@ -import { describe, expect, test } from 'vitest'; - -import { getVersionFromTarball } from '../src/middleware-utils'; - -describe('Utilities', () => { - describe('getVersionFromTarball', () => { - test('should get the right version', () => { - const simpleName = 'test-name-4.2.12.tgz'; - const complexName = 'test-5.6.4-beta.2.tgz'; - const otherComplexName = 'test-3.5.0-6.tgz'; - expect(getVersionFromTarball(simpleName)).toEqual('4.2.12'); - expect(getVersionFromTarball(complexName)).toEqual('5.6.4-beta.2'); - expect(getVersionFromTarball(otherComplexName)).toEqual('3.5.0-6'); - }); - - test("should don'n fall at incorrect tarball name", () => { - expect(getVersionFromTarball('incorrectName')).toBeUndefined(); - }); - }); -});