Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support strictValidateTarballPkg #546

Merged
merged 3 commits into from
Jul 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions app/common/PackageUtil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import { pipeline } from 'node:stream/promises';
import * as ssri from 'ssri';
import tar from 'tar';
import { PackageJSONType } from '../repository/PackageRepository';

// /@cnpm%2ffoo
// /@cnpm%2Ffoo
Expand Down Expand Up @@ -102,3 +103,24 @@
throw Object.assign(new Error('[hasShrinkWrapInTgz] Fail to parse input file'), { cause: e });
}
}

export async function extractPackageJSON(tarballBytes: Buffer): Promise<PackageJSONType> {
return new Promise((resolve, reject) => {
Readable.from(tarballBytes)
.pipe(tar.t({
filter: name => name === 'package/package.json',
onentry: async entry => {
const chunks: Buffer[] = [];
for await (const 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'));
}

Check warning on line 122 in app/common/PackageUtil.ts

View check run for this annotation

Codecov / codecov/patch

app/common/PackageUtil.ts#L121-L122

Added lines #L121 - L122 were not covered by tests
},
}));
});
}
elrrrrrrr marked this conversation as resolved.
Show resolved Hide resolved
elrrrrrrr marked this conversation as resolved.
Show resolved Hide resolved
5 changes: 5 additions & 0 deletions app/port/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
elrrrrrrr marked this conversation as resolved.
Show resolved Hide resolved
fengmk2 marked this conversation as resolved.
Show resolved Hide resolved
18 changes: 17 additions & 1 deletion app/port/controller/package/SavePackageVersionController.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { PackageJson, Simplify } from 'type-fest';
import { isEqual } from 'lodash';
import {
UnprocessableEntityError,
ForbiddenError,
Expand All @@ -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,
Expand All @@ -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' ];
elrrrrrrr marked this conversation as resolved.
Show resolved Hide resolved

type PackageVersion = Simplify<PackageJson.PackageJsonStandard & {
name: 'string';
version: 'string';
Expand Down Expand Up @@ -171,6 +174,19 @@ export class SavePackageVersionController extends AbstractController {
}
}

// https://github.com/cnpm/cnpmcore/issues/542
// check tgz & manifests
if (this.config.cnpmcore.strictValidateTarballPkg) {
const tarballPkg = await extractPackageJSON(tarballBytes);
const versionManifest = pkg.versions[tarballPkg.version];
const diffKeys = STRICT_CHECK_TARBALL_FIELDS.filter(key => {
return !isEqual(tarballPkg[key], versionManifest[key]);
});
if (diffKeys.length > 0) {
throw new UnprocessableEntityError(`${diffKeys} mismatch between tarball and manifest`);
}
}

const [ scope, name ] = getScopeAndName(fullname);

// make sure readme is string
elrrrrrrr marked this conversation as resolved.
Show resolved Hide resolved
elrrrrrrr marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
1 change: 1 addition & 0 deletions config/config.default.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export const cnpmcoreConfig: CnpmcoreConfig = {
redirectNotFound: true,
enableUnpkg: true,
strictSyncSpecivicVersion: false,
strictValidateTarballPkg: false,
};

export default (appInfo: EggAppConfig) => {
elrrrrrrr marked this conversation as resolved.
Show resolved Hide resolved
elrrrrrrr marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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' });
Expand Down Expand Up @@ -86,6 +86,39 @@ describe('test/port/controller/package/SavePackageVersionController.test.ts', ()
assert(pkgEntity);
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: '0.0.1' });

pkg2.versions['0.0.1'].dependencies = { lodash: 'latest' };

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,dependencies mismatch between tarball and manifest');
});

it('should add new version success on scoped package', async () => {
const name = '@cnpm/publish-package-test';
elrrrrrrr marked this conversation as resolved.
Show resolved Hide resolved
elrrrrrrr marked this conversation as resolved.
Show resolved Hide resolved
Expand Down