From 305457777e96b81570b2750cce92a8e36ddbf0b8 Mon Sep 17 00:00:00 2001 From: elrrrrrrr Date: Wed, 26 Feb 2025 22:43:28 -0800 Subject: [PATCH] fix: block tgz (#763) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit > The tgz download interface does not check if the package is blocked, which may pose additional risks for parsing package-lock.json or other lock files. [exp](https://registry.npmmirror.com/joker-su/-/joker-su-1.0.0.tgz) 1. 🛡️ Add validation logic for DownloadPackageVersionTarController#download to check if the package is allowed to be downloaded. 2. 🧶 Add PackageVersionService#findBlockInfo to check if the corresponding package is blocked. 3. ♻️ When a single version is blocked, skip check as per the current manifest logic. --------- > tgz 下载接口没有判断包是否被 block，对于 package-lock.json 或者其他依赖锁文件解析可能会有额外风险，[exp](https://registry.npmmirror.com/joker-su/-/joker-su-1.0.0.tgz) 1. 🛡️ `DownloadPackageVersionTarController#download` 接口新增校验逻辑，判断是否允许下载 2. 🧶 新增 PackageVersionService#findBlockInfo 判断对应包是否被全局拦截 3. ♻️ 单版本被 block 时，考虑到误封场景，按目前 manifest 逻辑，不在 tgz 下载时进行拦截操作 ## Summary by CodeRabbit - **New Features** - Enhanced the package download process with an additional block check. Now, if a package is flagged, the download will be halted and a clear error response is returned to inform users of the block. - Introduced a method to retrieve block information related to package versions, improving the service's capabilities. - **Tests** - Added new test cases to verify the blocking functionality for package downloads, ensuring the application correctly handles requests for blocked packages. --- app/core/service/PackageVersionService.ts | 18 ++++++++++- .../package/DownloadPackageVersionTar.ts | 13 ++++++++ ...ownloadPackageVersionTarController.test.ts | 30 +++++++++++++++++++ 3 files changed, 60 insertions(+), 1 deletion(-) diff --git a/app/core/service/PackageVersionService.ts b/app/core/service/PackageVersionService.ts index 8852ca48..85fc22d1 100644 --- a/app/core/service/PackageVersionService.ts +++ b/app/core/service/PackageVersionService.ts @@ -5,9 +5,10 @@ import { PackageVersionRepository } from '../../repository/PackageVersionReposit import { getScopeAndName } from '../../common/PackageUtil'; import { SqlRange } from '../entity/SqlRange'; import { BugVersionService } from './BugVersionService'; -import type { PackageJSONType } from '../../repository/PackageRepository'; +import type { PackageJSONType, PackageRepository } from '../../repository/PackageRepository'; import { DistRepository } from '../../repository/DistRepository'; import { BugVersionAdvice } from '../entity/BugVersion'; +import { PackageVersionBlockRepository } from '../../repository/PackageVersionBlockRepository'; @SingletonProto({ accessLevel: AccessLevel.PUBLIC, @@ -16,6 +17,12 @@ export class PackageVersionService { @Inject() private packageVersionRepository: PackageVersionRepository; + @Inject() + private packageRepository: PackageRepository; + + @Inject() + private packageVersionBlockRepository: PackageVersionBlockRepository; + @Inject() private readonly bugVersionService: BugVersionService; @@ -115,4 +122,13 @@ export class PackageVersionService { } return version; } + + async findBlockInfo(fullname: string) { + const [ scope, name ] = getScopeAndName(fullname); + const pkg = await this.packageRepository.findPackage(scope, name); + if (!pkg) { + return null; + } + return await this.packageVersionBlockRepository.findPackageBlock(pkg.packageId); + } } diff --git a/app/port/controller/package/DownloadPackageVersionTar.ts b/app/port/controller/package/DownloadPackageVersionTar.ts index 03ddb15c..a2ca67e9 100644 --- a/app/port/controller/package/DownloadPackageVersionTar.ts +++ b/app/port/controller/package/DownloadPackageVersionTar.ts @@ -19,6 +19,7 @@ import { PackageManagerService } from '../../../core/service/PackageManagerServi import { ProxyCacheService } from '../../../core/service/ProxyCacheService'; import { PackageSyncerService } from '../../../core/service/PackageSyncerService'; import { RegistryManagerService } from '../../../core/service/RegistryManagerService'; +import { PackageVersionService } from '../../../core/service/PackageVersionService'; @HTTPController() export class DownloadPackageVersionTarController extends AbstractController { @@ -31,6 +32,8 @@ export class DownloadPackageVersionTarController extends AbstractController { @Inject() private packageSyncerService: PackageSyncerService; @Inject() + private packageVersionService: PackageVersionService; + @Inject() private nfsAdapter: NFSAdapter; // Support OPTIONS Request on tgz download @@ -55,6 +58,16 @@ export class DownloadPackageVersionTarController extends AbstractController { const version = this.getAndCheckVersionFromFilename(ctx, fullname, filenameWithVersion); const storeKey = `/packages/${fullname}/${version}/${filenameWithVersion}.tgz`; const downloadUrl = await this.nfsAdapter.getDownloadUrl(storeKey); + + // block tgz only all versions have been blocked + const blockInfo = await this.packageVersionService.findBlockInfo(fullname); + if (blockInfo?.reason) { + this.setCDNHeaders(ctx); + ctx.logger.info('[PackageController:downloadVersionTar] %s@%s, block for %s', + fullname, version, blockInfo.reason); + throw this.createPackageBlockError(blockInfo.reason, fullname, version); + } + if (this.config.cnpmcore.syncMode === SyncMode.all && downloadUrl) { // try nfs url first, avoid db query this.packageManagerService.plusPackageVersionCounter(fullname, version); diff --git a/test/port/controller/package/DownloadPackageVersionTarController.test.ts b/test/port/controller/package/DownloadPackageVersionTarController.test.ts index 1fd7abb7..a8142659 100644 --- a/test/port/controller/package/DownloadPackageVersionTarController.test.ts +++ b/test/port/controller/package/DownloadPackageVersionTarController.test.ts @@ -4,13 +4,16 @@ import { app, mock } from 'egg-mock/bootstrap'; import { TestUtil } from '../../../../test/TestUtil'; import { NFSClientAdapter } from '../../../../app/infra/NFSClientAdapter'; import { SyncMode } from '../../../../app/common/constants'; +import { PackageManagerService } from '../../../../app/core/service/PackageManagerService'; describe('test/port/controller/package/DownloadPackageVersionTarController.test.ts', () => { let publisher: any; let nfsClientAdapter: NFSClientAdapter; + let packageManagerService: PackageManagerService; beforeEach(async () => { publisher = await TestUtil.createUser(); nfsClientAdapter = await app.getEggObject(NFSClientAdapter); + packageManagerService = await app.getEggObject(PackageManagerService); }); const scope = '@cnpm'; @@ -38,6 +41,8 @@ describe('test/port/controller/package/DownloadPackageVersionTarController.test. assert(res.status === 201); assert(res.body.ok === true); assert.match(res.body.rev, /^\d+\-\w{24}$/); + + await packageManagerService.unblockPackageByFullname(scopedName); }); describe('[GET /:fullname/-/:name-:version.tgz] download()', () => { @@ -56,6 +61,14 @@ describe('test/port/controller/package/DownloadPackageVersionTarController.test. assert(res.headers.location === `https://cdn.mock.com/packages/${scopedName}/1.0.0/${name}-1.0.0.tgz`); }); + it('should block tgz', async () => { + await packageManagerService.blockPackageByFullname(scopedName, 'test'); + const res = await app.httpRequest() + .get(`/${scopedName}/-/testmodule-download-version-tar-1.0.0.tgz`) + .expect(451); + assert.equal(res.body.error, '[UNAVAILABLE_FOR_LEGAL_REASONS] @cnpm/testmodule-download-version-tar@1.0.0 was blocked, reason: test'); + }); + it('should support cors OPTIONS Request', async () => { mock(nfsClientAdapter, 'url', async (storeKey: string) => { return `https://cdn.mock.com${storeKey}`; @@ -337,6 +350,14 @@ describe('test/port/controller/package/DownloadPackageVersionTarController.test. assert(res.headers.location === `https://cdn.mock.com/packages/${scopedName}/1.0.0/${name}-1.0.0.tgz`); }); + it('should block tgz', async () => { + await packageManagerService.blockPackageByFullname(scopedName, 'test'); + const res = await app.httpRequest() + .get(`/${scopedName}/download/${scopedName}-1.0.0.tgz`) + .expect(451); + assert.equal(res.body.error, '[UNAVAILABLE_FOR_LEGAL_REASONS] @cnpm/testmodule-download-version-tar@1.0.0 was blocked, reason: test'); + }); + it('should download a version tar with streaming success', async () => { mock(nfsClientAdapter, 'url', 'not-function'); const res = await app.httpRequest() @@ -385,6 +406,15 @@ describe('test/port/controller/package/DownloadPackageVersionTarController.test. assert(res.headers.location === `https://cdn.mock.com/packages/${scopedName}/1.0.0/${name}-1.0.0.tgz`); }); + it('should block tgz', async () => { + await packageManagerService.blockPackageByFullname(scopedName, 'test'); + const res = await app.httpRequest() + .get(`/${scopedName}/-/${scope}/${name}-1.0.0.tgz`) + .expect(451); + assert.equal(res.body.error, '[UNAVAILABLE_FOR_LEGAL_REASONS] @cnpm/testmodule-download-version-tar@1.0.0 was blocked, reason: test'); + }); + + it('should download a version tar with streaming success', async () => { mock(nfsClientAdapter, 'url', 'not-function'); const res = await app.httpRequest()