From 27900279a314827a47877004f1396b4feaca957e Mon Sep 17 00:00:00 2001 From: Air1 Date: Fri, 14 Feb 2020 14:35:07 +0400 Subject: [PATCH 1/6] refactor: allow multiple ports in bridgechain schema --- .../src/transactions/utils/bridgechain-schemas.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/core-magistrate-crypto/src/transactions/utils/bridgechain-schemas.ts b/packages/core-magistrate-crypto/src/transactions/utils/bridgechain-schemas.ts index a2a2f66db3..ce096c40d7 100644 --- a/packages/core-magistrate-crypto/src/transactions/utils/bridgechain-schemas.ts +++ b/packages/core-magistrate-crypto/src/transactions/utils/bridgechain-schemas.ts @@ -11,7 +11,7 @@ export const seedNodesSchema = { export const portsSchema = { type: "object", - maxProperties: 1, + maxProperties: 10, minProperties: 1, required: ["@arkecosystem/core-api"], additionalProperties: false, @@ -22,4 +22,11 @@ export const portsSchema = { maximum: 65535, }, }, + patternProperties: { + "^.{1,100}$": { + type: "integer", + minimum: 0, + maximum: 65535, + }, + }, }; From 71689cd19ad55abb4b420f12dbe1b38155a2cdca Mon Sep 17 00:00:00 2001 From: Air1 Date: Fri, 14 Feb 2020 14:36:09 +0400 Subject: [PATCH 2/6] test: multiple ports for bridgechain --- .../tests/scenarios/scenario1/business-bridgechain/utils.js | 4 ++-- __tests__/unit/core-magistrate/helper.ts | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/__tests__/e2e/tests/scenarios/scenario1/business-bridgechain/utils.js b/__tests__/e2e/tests/scenarios/scenario1/business-bridgechain/utils.js index 99f49cc779..aa75076270 100644 --- a/__tests__/e2e/tests/scenarios/scenario1/business-bridgechain/utils.js +++ b/__tests__/e2e/tests/scenarios/scenario1/business-bridgechain/utils.js @@ -34,7 +34,7 @@ const bridgechainRegistrationAsset = { genesisHash: "127e6fbfe24a750e72930c220a8e138275656b8e5d8f48a98c3c92df2caba935", bridgechainRepository: "http://www.repository.com/myorg/myrepo", bridgechainAssetRepository: "http://www.repository.com/myorg/myassetrepo", - ports: { "@arkecosystem/core-api": 12345 }, + ports: { "@arkecosystem/core-api": 12345, "custom/api": 3456 }, }; const bridgechainUpdateAsset = { @@ -42,7 +42,7 @@ const bridgechainUpdateAsset = { seedNodes: [ "75.125.224.71", ], - ports: { "@arkecosystem/core-api": 54321 }, + ports: { "@arkecosystem/core-api": 54321, "custom/other-api": 9876 }, bridgechainRepository: "http://www.newrepository.com/neworg/newrepo", bridgechainAssetRepository: "http://www.newrepository.com/neworg/newassetrepo", }; diff --git a/__tests__/unit/core-magistrate/helper.ts b/__tests__/unit/core-magistrate/helper.ts index 204b3346ba..f606c7663e 100644 --- a/__tests__/unit/core-magistrate/helper.ts +++ b/__tests__/unit/core-magistrate/helper.ts @@ -67,7 +67,10 @@ export const bridgechainRegistrationAsset1: IBridgechainRegistrationAsset = { genesisHash: "127e6fbfe24a750e72930c220a8e138275656b8e5d8f48a98c3c92df2caba935", bridgechainRepository: "http://www.repository.com/myorg/myrepo", bridgechainAssetRepository: "http://www.repository.com/myorg/myassetrepo", - ports: { "@arkecosystem/core-api": 12345 }, + ports: { + "@arkecosystem/core-api": 12345, + "@custom/api": 3333, + }, }; export const bridgechainRegistrationAsset2: IBridgechainRegistrationAsset = { From e2958dc56430f9677ba2a93ec425493d5be3d5b4 Mon Sep 17 00:00:00 2001 From: Air1 Date: Fri, 14 Feb 2020 14:37:06 +0400 Subject: [PATCH 3/6] chore: fix e2e workflow --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9b4db61b22..e2f9852d00 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -105,7 +105,7 @@ jobs: with: node-version: ${{ matrix.node-version }} - name: Install and build packages - run: yarn setup && cd __tests__/e2e && ../../node_modules/.bin/tsc && yarn cache clean && yarn install --frozen-lockfile + run: yarn setup && cd __tests__ && ../node_modules/.bin/tsc && cd e2e && yarn cache clean && yarn install --frozen-lockfile - name: Docker compose up run: cd __tests__/e2e/lib/config && docker-compose up -d - name: Wait 20 sec for docker containers to be up and nodes be running From 33e640f09417c6f2250d39a1cc8acc443d853a03 Mon Sep 17 00:00:00 2001 From: Air1 Date: Fri, 14 Feb 2020 15:13:48 +0400 Subject: [PATCH 4/6] chore: fix e2e workflow --- .github/workflows/test.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index e2f9852d00..df3c74a205 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -105,7 +105,11 @@ jobs: with: node-version: ${{ matrix.node-version }} - name: Install and build packages - run: yarn setup && cd __tests__ && ../node_modules/.bin/tsc && cd e2e && yarn cache clean && yarn install --frozen-lockfile + run: yarn setup && cd __tests__/e2e && yarn cache clean && yarn install --frozen-lockfile + - name: Build test helpers + run: cd __tests__/helpers && echo "{}" > tsconfig.json && ../../node_modules/.bin/tsc || true + - name: Build test utils + run: cd __tests__/utils && echo "{}" > tsconfig.json && ../../node_modules/.bin/tsc || true - name: Docker compose up run: cd __tests__/e2e/lib/config && docker-compose up -d - name: Wait 20 sec for docker containers to be up and nodes be running From a481fd388460dc5d31c9b76a2c23955bd95f8ff6 Mon Sep 17 00:00:00 2001 From: Air1 Date: Fri, 21 Feb 2020 16:03:02 +0400 Subject: [PATCH 5/6] refactor: validate package name regex in handler because AJV has unwanted behavior with patternProperties --- .../src/transactions/utils/bridgechain-schemas.ts | 11 +++-------- packages/core-magistrate-transactions/src/errors.ts | 6 ++++++ .../src/handlers/bridgechain-registration.ts | 10 +++++++++- .../src/handlers/bridgechain-update.ts | 8 ++++++++ .../src/handlers/utils.ts | 1 + 5 files changed, 27 insertions(+), 9 deletions(-) create mode 100644 packages/core-magistrate-transactions/src/handlers/utils.ts diff --git a/packages/core-magistrate-crypto/src/transactions/utils/bridgechain-schemas.ts b/packages/core-magistrate-crypto/src/transactions/utils/bridgechain-schemas.ts index ce096c40d7..98fdc84954 100644 --- a/packages/core-magistrate-crypto/src/transactions/utils/bridgechain-schemas.ts +++ b/packages/core-magistrate-crypto/src/transactions/utils/bridgechain-schemas.ts @@ -15,15 +15,10 @@ export const portsSchema = { minProperties: 1, required: ["@arkecosystem/core-api"], additionalProperties: false, - properties: { - "@arkecosystem/core-api": { - type: "integer", - minimum: 0, - maximum: 65535, - }, - }, patternProperties: { - "^.{1,100}$": { + // just allow anything within length limitation of npm package name, more + // precise validation will be done in transaction handler + "^.{1,214}$": { type: "integer", minimum: 0, maximum: 65535, diff --git a/packages/core-magistrate-transactions/src/errors.ts b/packages/core-magistrate-transactions/src/errors.ts index 189243e1c6..eee1833ce5 100644 --- a/packages/core-magistrate-transactions/src/errors.ts +++ b/packages/core-magistrate-transactions/src/errors.ts @@ -48,3 +48,9 @@ export class GenesisHashAlreadyRegisteredError extends Errors.TransactionError { super("Failed to apply transaction, because genesis hash is already registered by another bridgechain."); } } + +export class PortKeyMustBeValidPackageNameError extends Errors.TransactionError { + constructor() { + super("Failed to apply transaction, because the package name(s) defined in ports is not valid."); + } +} diff --git a/packages/core-magistrate-transactions/src/handlers/bridgechain-registration.ts b/packages/core-magistrate-transactions/src/handlers/bridgechain-registration.ts index d76bf6f9f3..ad572a6cc3 100644 --- a/packages/core-magistrate-transactions/src/handlers/bridgechain-registration.ts +++ b/packages/core-magistrate-transactions/src/handlers/bridgechain-registration.ts @@ -6,12 +6,14 @@ import { BridgechainAlreadyRegisteredError, BusinessIsResignedError, GenesisHashAlreadyRegisteredError, + PortKeyMustBeValidPackageNameError, WalletIsNotBusinessError, } from "../errors"; import { MagistrateApplicationEvents } from "../events"; import { IBridgechainWalletAttributes, IBusinessWalletAttributes } from "../interfaces"; import { BusinessRegistrationTransactionHandler } from "./business-registration"; import { MagistrateTransactionHandler } from "./magistrate-handler"; +import { packageNameRegex } from "./utils"; export class BridgechainRegistrationTransactionHandler extends MagistrateTransactionHandler { public getConstructor(): Transactions.TransactionConstructor { @@ -100,6 +102,12 @@ export class BridgechainRegistrationTransactionHandler extends MagistrateTransac } } + for (const portKey of Object.keys(data.asset.bridgechainRegistration.ports)) { + if (!packageNameRegex.test(portKey)) { + throw new PortKeyMustBeValidPackageNameError(); + } + } + return super.throwIfCannotBeApplied(transaction, wallet, walletManager); } @@ -111,7 +119,7 @@ export class BridgechainRegistrationTransactionHandler extends MagistrateTransac data: Interfaces.ITransactionData, pool: TransactionPool.IConnection, processor: TransactionPool.IProcessor, - ): Promise<{ type: string, message: string } | null> { + ): Promise<{ type: string; message: string } | null> { return null; } diff --git a/packages/core-magistrate-transactions/src/handlers/bridgechain-update.ts b/packages/core-magistrate-transactions/src/handlers/bridgechain-update.ts index 820c1474a3..cbf2d223e8 100644 --- a/packages/core-magistrate-transactions/src/handlers/bridgechain-update.ts +++ b/packages/core-magistrate-transactions/src/handlers/bridgechain-update.ts @@ -12,11 +12,13 @@ import { BridgechainIsResignedError, BusinessIsNotRegisteredError, BusinessIsResignedError, + PortKeyMustBeValidPackageNameError, } from "../errors"; import { MagistrateApplicationEvents } from "../events"; import { IBridgechainWalletAttributes, IBusinessWalletAttributes } from "../interfaces"; import { BridgechainRegistrationTransactionHandler } from "./bridgechain-registration"; import { MagistrateTransactionHandler } from "./magistrate-handler"; +import { packageNameRegex } from "./utils"; export class BridgechainUpdateTransactionHandler extends MagistrateTransactionHandler { public getConstructor(): Transactions.TransactionConstructor { @@ -89,6 +91,12 @@ export class BridgechainUpdateTransactionHandler extends MagistrateTransactionHa throw new BridgechainIsResignedError(); } + for (const portKey of Object.keys(bridgechainUpdate.ports || {})) { + if (!packageNameRegex.test(portKey)) { + throw new PortKeyMustBeValidPackageNameError(); + } + } + return super.throwIfCannotBeApplied(transaction, wallet, walletManager); } diff --git a/packages/core-magistrate-transactions/src/handlers/utils.ts b/packages/core-magistrate-transactions/src/handlers/utils.ts new file mode 100644 index 0000000000..4b11509612 --- /dev/null +++ b/packages/core-magistrate-transactions/src/handlers/utils.ts @@ -0,0 +1 @@ +export const packageNameRegex = new RegExp("^[a-z0-9@/.~_-]{1,214}$"); From 941f7764f7b5a74cfcb655847a1dbd2b89b6dd5d Mon Sep 17 00:00:00 2001 From: Air1 Date: Fri, 21 Feb 2020 16:04:05 +0400 Subject: [PATCH 6/6] test: ports with regex for package name validation --- .../handlers/bridgechain-registration.test.ts | 25 +++++++++++ .../handlers/bridgechain-update.test.ts | 21 +++++++++ .../bridgechain-registration.test.ts | 45 +++++++++++++++++++ 3 files changed, 91 insertions(+) diff --git a/__tests__/unit/core-magistrate/handlers/bridgechain-registration.test.ts b/__tests__/unit/core-magistrate/handlers/bridgechain-registration.test.ts index d24f8cdc8d..74eb530e80 100644 --- a/__tests__/unit/core-magistrate/handlers/bridgechain-registration.test.ts +++ b/__tests__/unit/core-magistrate/handlers/bridgechain-registration.test.ts @@ -7,6 +7,7 @@ import { Handlers } from "@arkecosystem/core-transactions"; import { Managers, Utils } from "@arkecosystem/crypto"; import { BridgechainAlreadyRegisteredError, + PortKeyMustBeValidPackageNameError, WalletIsNotBusinessError, } from "../../../../packages/core-magistrate-transactions/src/errors"; import { @@ -121,6 +122,30 @@ describe("should test marketplace transaction handlers", () => { bridgechainRegistrationHandler.throwIfCannotBeApplied(actual.build(), senderWallet, walletManager), ).rejects.toThrowError(BridgechainAlreadyRegisteredError); }); + + it.each([["@invalid/UPPERCASE"], ["@invalid/char)"]])( + "should throw because ports contains invalid package name", + async invalidName => { + const actual = bridgechainRegistrationBuilder + .bridgechainRegistrationAsset({ + ...bridgechainRegistrationAsset1, + ports: { + ...bridgechainRegistrationAsset1.ports, + [invalidName]: 4444, + }, + }) + .nonce("2") + .sign("clay harbor enemy utility margin pretty hub comic piece aerobic umbrella acquire"); + + await expect( + bridgechainRegistrationHandler.throwIfCannotBeApplied( + actual.build(), + senderWallet, + walletManager, + ), + ).rejects.toThrowError(PortKeyMustBeValidPackageNameError); + }, + ); }); describe("applyToSender tests", () => { diff --git a/__tests__/unit/core-magistrate/handlers/bridgechain-update.test.ts b/__tests__/unit/core-magistrate/handlers/bridgechain-update.test.ts index e2e75f66e8..c584cb2bf9 100644 --- a/__tests__/unit/core-magistrate/handlers/bridgechain-update.test.ts +++ b/__tests__/unit/core-magistrate/handlers/bridgechain-update.test.ts @@ -9,6 +9,7 @@ import { Managers, Utils } from "@arkecosystem/crypto"; import { BridgechainIsNotRegisteredByWalletError, BusinessIsNotRegisteredError, + PortKeyMustBeValidPackageNameError, } from "../../../../packages/core-magistrate-transactions/src/errors"; import { BridgechainRegistrationTransactionHandler, @@ -128,6 +129,26 @@ describe("Bridgechain update handler", () => { await bridgechainRegistrationHandler.applyToSender(bridgechainRegistration.build(), walletManager); }); + it.each([["@invalid/UPPERCASE"], ["@invalid/char)"]])( + "should throw because ports contains invalid package name", + async invalidName => { + const actual = bridgechainUpdateBuilder + .bridgechainUpdateAsset({ + ...bridgechainUpdateAsset1, + ports: { + ...bridgechainUpdateAsset1.ports, + [invalidName]: 4444, + }, + }) + .nonce("3") + .sign("clay harbor enemy utility margin pretty hub comic piece aerobic umbrella acquire"); + + await expect( + bridgechainUpdateHandler.throwIfCannotBeApplied(actual.build(), senderWallet, walletManager), + ).rejects.toThrowError(PortKeyMustBeValidPackageNameError); + }, + ); + it("should not throw because bridgechain is registered", async () => { const actual = bridgechainUpdateBuilder .bridgechainUpdateAsset(bridgechainUpdateAsset1) diff --git a/__tests__/unit/core-magistrate/transactions/bridgechain-registration.test.ts b/__tests__/unit/core-magistrate/transactions/bridgechain-registration.test.ts index 38111f31fe..ae37fdf25e 100644 --- a/__tests__/unit/core-magistrate/transactions/bridgechain-registration.test.ts +++ b/__tests__/unit/core-magistrate/transactions/bridgechain-registration.test.ts @@ -250,5 +250,50 @@ describe("Bridgechain registration transaction", () => { expect(error).not.toBeUndefined(); }); }); + + describe("should test edge cases for ports", () => { + it.each([["empty is invalid", {}], ["needs to include core-api", { "@arkecosystem/not-core-api": 123 }]])( + "should fail to validate for '%s' case", + (_, ports) => { + const bridgechainRegistration = builder + .bridgechainRegistrationAsset({ + name: "google", + genesisHash: "000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f", + bridgechainRepository: "http://www.repository.com/google/syzkaller", + bridgechainAssetRepository: "http://www.repository.com/google/asset", + seedNodes: ["66.102.0.0"], + ports, + }) + .sign("passphrase"); + + const struct = bridgechainRegistration.getStruct(); + const { error } = Validation.validator.validate(transactionSchema, struct); + console.log(JSON.stringify(struct)); + expect(error).not.toBeUndefined(); + }, + ); + + it.each([ + [{ "@arkecosystem/core-api": 123 }], + [{ "@arkecosystem/core-api": 123, "@valid/pa_ckage-name": 124 }], + [{ "@arkecosystem/core-api": 123, "valid-package_name": 124 }], + [{ "@arkecosystem/core-api": 123, "@invalid/UPPERCASE": 124 }], + [{ "@arkecosystem/core-api": 123, "@invalid/char)": 124 }], + ])("should validate", ports => { + const bridgechainRegistration = builder + .bridgechainRegistrationAsset({ + name: "google", + genesisHash: "000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f", + bridgechainRepository: "http://www.repository.com/google/syzkaller", + bridgechainAssetRepository: "http://www.repository.com/google/asset", + seedNodes: ["66.102.0.0"], + ports, + }) + .sign("passphrase"); + + const { error } = Validation.validator.validate(transactionSchema, bridgechainRegistration.getStruct()); + expect(error).toBeUndefined(); + }); + }); }); });