From 456e684f0ffc195c487a78bb38b1d50223788c19 Mon Sep 17 00:00:00 2001 From: elrrrrrrr Date: Sun, 9 Jul 2023 00:29:00 +0800 Subject: [PATCH 1/3] feat: support strictlyValidateTarballPkg --- app/common/PackageUtil.ts | 23 +++++++++++++++++++ app/port/config.ts | 5 ++++ .../package/SavePackageVersionController.ts | 18 ++++++++++++++- config/config.default.ts | 1 + .../SavePackageVersionController.test.ts | 17 +++++++++++++- 5 files changed, 62 insertions(+), 2 deletions(-) diff --git a/app/common/PackageUtil.ts b/app/common/PackageUtil.ts index c3e146e4..5e689dc5 100644 --- a/app/common/PackageUtil.ts +++ b/app/common/PackageUtil.ts @@ -3,6 +3,7 @@ import { Readable } from 'node:stream'; import { pipeline } from 'node:stream/promises'; import * as ssri from 'ssri'; import tar from 'tar'; +import { PackageJSONType } from '../repository/PackageRepository'; // /@cnpm%2ffoo // /@cnpm%2Ffoo @@ -102,3 +103,25 @@ export async function hasShrinkWrapInTgz(contentOrFile: Uint8Array | string): Pr throw Object.assign(new Error('[hasShrinkWrapInTgz] Fail to parse input file'), { cause: e }); } } + +export async function extractPackageJSON(tarballBytes: Buffer): Promise { + return new Promise((resolve, reject) => { + Readable.from(tarballBytes) + .pipe(tar.t({ + filter: name => name === 'package/package.json', + onentry: entry => { + let json = ''; + entry.on('data', data => { + json += data.toString(); + }); + entry.on('end', () => { + try { + resolve(JSON.parse(json)); + } catch (err) { + reject(new Error('Error parsing package.json')); + } + }); + }, + })); + }); +} diff --git a/app/port/config.ts b/app/port/config.ts index f5f7fa1c..1ce26ec6 100644 --- a/app/port/config.ts +++ b/app/port/config.ts @@ -150,4 +150,9 @@ export type CnpmcoreConfig = { * in most cases, you should set to false to keep the same behavior as source registry. */ strictSyncSpecivicVersion: boolean, + + /** + * strictly enforces/validates manifest and tgz when publish, https://github.com/cnpm/cnpmcore/issues/542 + */ + strictValidateTarballPkg?: boolean, }; diff --git a/app/port/controller/package/SavePackageVersionController.ts b/app/port/controller/package/SavePackageVersionController.ts index 59e3dfa2..79b07698 100644 --- a/app/port/controller/package/SavePackageVersionController.ts +++ b/app/port/controller/package/SavePackageVersionController.ts @@ -1,4 +1,5 @@ import { PackageJson, Simplify } from 'type-fest'; +import { isEqual } from 'lodash'; import { UnprocessableEntityError, ForbiddenError, @@ -17,7 +18,7 @@ import * as ssri from 'ssri'; import validateNpmPackageName from 'validate-npm-package-name'; import { Static, Type } from '@sinclair/typebox'; import { AbstractController } from '../AbstractController'; -import { getScopeAndName, FULLNAME_REG_STRING } from '../../../common/PackageUtil'; +import { getScopeAndName, FULLNAME_REG_STRING, extractPackageJSON } from '../../../common/PackageUtil'; import { PackageManagerService } from '../../../core/service/PackageManagerService'; import { VersionRule, @@ -28,6 +29,8 @@ import { import { RegistryManagerService } from '../../../core/service/RegistryManagerService'; import { PackageJSONType } from '../../../repository/PackageRepository'; +const STRICT_CHECK_TARBALL_FIELDS: (keyof PackageJson)[] = [ 'name', 'version', 'scripts', 'dependencies', 'devDependencies', 'peerDependencies', 'optionalDependencies', 'license', 'licenses', 'bin' ]; + type PackageVersion = Simplify { + return !isEqual(tarballPkg[key], versionManifest[key]); + }); + if (diffKey) { + throw new UnprocessableEntityError(`${diffKey} mismatch between tarball and manifest`); + } + } + const [ scope, name ] = getScopeAndName(fullname); // make sure readme is string diff --git a/config/config.default.ts b/config/config.default.ts index 4be24121..04df8fae 100644 --- a/config/config.default.ts +++ b/config/config.default.ts @@ -54,6 +54,7 @@ export const cnpmcoreConfig: CnpmcoreConfig = { redirectNotFound: true, enableUnpkg: true, strictSyncSpecivicVersion: false, + strictValidateTarballPkg: false, }; export default (appInfo: EggAppConfig) => { diff --git a/test/port/controller/package/SavePackageVersionController.test.ts b/test/port/controller/package/SavePackageVersionController.test.ts index 2a794503..9daefd14 100644 --- a/test/port/controller/package/SavePackageVersionController.test.ts +++ b/test/port/controller/package/SavePackageVersionController.test.ts @@ -21,7 +21,7 @@ describe('test/port/controller/package/SavePackageVersionController.test.ts', () }); describe('[PUT /:fullname] save()', () => { - it('should set registry filed after publish', async () => { + it('should set registry field after publish', async () => { mock(app.config.cnpmcore, 'allowPublishNonScopePackage', true); const { pkg, user } = await TestUtil.createPackage({ name: 'non_scope_pkg', version: '1.0.0' }); const pkg2 = await TestUtil.getFullPackage({ name: pkg.name, version: '2.0.0' }); @@ -86,6 +86,21 @@ describe('test/port/controller/package/SavePackageVersionController.test.ts', () assert(pkgEntity); assert.equal(pkgEntity.registryId, selfRegistry.registryId); }); + it('should verify tgz and manifest', async () => { + mock(app.config.cnpmcore, 'allowPublishNonScopePackage', true); + const { pkg, user } = await TestUtil.createPackage({ name: 'non_scope_pkg', version: '1.0.0' }); + const pkg2 = await TestUtil.getFullPackage({ name: pkg.name, version: '2.0.0' }); + + mock(app.config.cnpmcore, 'strictValidateTarballPkg', true); + const res = await app.httpRequest() + .put(`/${pkg2.name}`) + .set('authorization', user.authorization) + .set('user-agent', user.ua) + .send(pkg2) + .expect(422); + + assert.equal(res.body.error, '[UNPROCESSABLE_ENTITY] name mismatch between tarball and manifest'); + }); it('should add new version success on scoped package', async () => { const name = '@cnpm/publish-package-test'; From 0728b942c62d38ff7748496c72aade053cdfa820 Mon Sep 17 00:00:00 2001 From: elrrrrrrr Date: Sun, 9 Jul 2023 22:23:27 +0800 Subject: [PATCH 2/3] feat: use Buffer.concat --- app/common/PackageUtil.ts | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/app/common/PackageUtil.ts b/app/common/PackageUtil.ts index 5e689dc5..2596e0cb 100644 --- a/app/common/PackageUtil.ts +++ b/app/common/PackageUtil.ts @@ -109,18 +109,17 @@ export async function extractPackageJSON(tarballBytes: Buffer): Promise name === 'package/package.json', - onentry: entry => { - let json = ''; - entry.on('data', data => { - json += data.toString(); - }); - entry.on('end', () => { - try { - resolve(JSON.parse(json)); - } catch (err) { - reject(new Error('Error parsing package.json')); - } - }); + onentry: async entry => { + let chunks: Buffer[] = []; + for await (let chunk of entry) { + chunks.push(chunk); + } + try { + const data = Buffer.concat(chunks); + return resolve(JSON.parse(data.toString())); + } catch (err) { + reject(new Error('Error parsing package.json')); + } }, })); }); From f56ec9de59c194c9ffb0269c5eb4b1a574655cf3 Mon Sep 17 00:00:00 2001 From: elrrrrrrr Date: Sun, 9 Jul 2023 22:37:40 +0800 Subject: [PATCH 3/3] feat: show all mismatch keys --- app/common/PackageUtil.ts | 4 ++-- .../package/SavePackageVersionController.ts | 6 ++--- .../SavePackageVersionController.test.ts | 22 +++++++++++++++++-- 3 files changed, 25 insertions(+), 7 deletions(-) diff --git a/app/common/PackageUtil.ts b/app/common/PackageUtil.ts index 2596e0cb..2ffbbffa 100644 --- a/app/common/PackageUtil.ts +++ b/app/common/PackageUtil.ts @@ -110,8 +110,8 @@ export async function extractPackageJSON(tarballBytes: Buffer): Promise name === 'package/package.json', onentry: async entry => { - let chunks: Buffer[] = []; - for await (let chunk of entry) { + const chunks: Buffer[] = []; + for await (const chunk of entry) { chunks.push(chunk); } try { diff --git a/app/port/controller/package/SavePackageVersionController.ts b/app/port/controller/package/SavePackageVersionController.ts index 79b07698..3715e92d 100644 --- a/app/port/controller/package/SavePackageVersionController.ts +++ b/app/port/controller/package/SavePackageVersionController.ts @@ -179,11 +179,11 @@ export class SavePackageVersionController extends AbstractController { if (this.config.cnpmcore.strictValidateTarballPkg) { const tarballPkg = await extractPackageJSON(tarballBytes); const versionManifest = pkg.versions[tarballPkg.version]; - const diffKey = STRICT_CHECK_TARBALL_FIELDS.find(key => { + const diffKeys = STRICT_CHECK_TARBALL_FIELDS.filter(key => { return !isEqual(tarballPkg[key], versionManifest[key]); }); - if (diffKey) { - throw new UnprocessableEntityError(`${diffKey} mismatch between tarball and manifest`); + if (diffKeys.length > 0) { + throw new UnprocessableEntityError(`${diffKeys} mismatch between tarball and manifest`); } } diff --git a/test/port/controller/package/SavePackageVersionController.test.ts b/test/port/controller/package/SavePackageVersionController.test.ts index 9daefd14..9e660601 100644 --- a/test/port/controller/package/SavePackageVersionController.test.ts +++ b/test/port/controller/package/SavePackageVersionController.test.ts @@ -87,9 +87,27 @@ describe('test/port/controller/package/SavePackageVersionController.test.ts', () assert.equal(pkgEntity.registryId, selfRegistry.registryId); }); it('should verify tgz and manifest', async () => { + const { pkg, user } = await TestUtil.createPackage({ name: '@cnpm/banana', version: '1.0.0' }); + const pkg2 = await TestUtil.getFullPackage({ name: pkg.name, version: '0.0.1' }); + + pkg2.versions['0.0.1'].name = '@cnpm/orange'; + + mock(app.config.cnpmcore, 'strictValidateTarballPkg', true); + const res = await app.httpRequest() + .put(`/${pkg2.name}`) + .set('authorization', user.authorization) + .set('user-agent', user.ua) + .send(pkg2) + .expect(422); + + assert.equal(res.body.error, '[UNPROCESSABLE_ENTITY] name mismatch between tarball and manifest'); + }); + it('should verify tgz and manifest with multiple fields', async () => { mock(app.config.cnpmcore, 'allowPublishNonScopePackage', true); const { pkg, user } = await TestUtil.createPackage({ name: 'non_scope_pkg', version: '1.0.0' }); - const pkg2 = await TestUtil.getFullPackage({ name: pkg.name, version: '2.0.0' }); + const pkg2 = await TestUtil.getFullPackage({ name: pkg.name, version: '0.0.1' }); + + pkg2.versions['0.0.1'].dependencies = { lodash: 'latest' }; mock(app.config.cnpmcore, 'strictValidateTarballPkg', true); const res = await app.httpRequest() @@ -99,7 +117,7 @@ describe('test/port/controller/package/SavePackageVersionController.test.ts', () .send(pkg2) .expect(422); - assert.equal(res.body.error, '[UNPROCESSABLE_ENTITY] name mismatch between tarball and manifest'); + assert.equal(res.body.error, '[UNPROCESSABLE_ENTITY] name,dependencies mismatch between tarball and manifest'); }); it('should add new version success on scoped package', async () => {