Skip to content

Commit

Permalink
fix: block tgz (#763)
Browse files Browse the repository at this point in the history
> 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 下载时进行拦截操作

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## 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.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
elrrrrrrr authored Feb 27, 2025
1 parent ae88145 commit 3054577
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 1 deletion.
18 changes: 17 additions & 1 deletion app/core/service/PackageVersionService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -16,6 +17,12 @@ export class PackageVersionService {
@Inject()
private packageVersionRepository: PackageVersionRepository;

@Inject()
private packageRepository: PackageRepository;

@Inject()
private packageVersionBlockRepository: PackageVersionBlockRepository;

@Inject()
private readonly bugVersionService: BugVersionService;

Expand Down Expand Up @@ -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);
}
}
13 changes: 13 additions & 0 deletions app/port/controller/package/DownloadPackageVersionTar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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()', () => {
Expand All @@ -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}`;
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down

0 comments on commit 3054577

Please sign in to comment.