From 2754f7e7ed3cdf2159f7eb422591094d396c2d1d Mon Sep 17 00:00:00 2001 From: David Drazic Date: Fri, 23 Sep 2022 18:56:46 +0200 Subject: [PATCH] Add changes to support blocking Snaps by source shasum (#15830) Refactor code and add unit tests for blocklist Add small fix for undefined Update property names Structural refactoring Refactor and improve unit tests Add comment that explains part of snaps blocking logic Refactor blocklist utility --- app/scripts/flask/snaps-blocklist.js | 24 ++++ app/scripts/flask/snaps-utilities.js | 35 ++++++ app/scripts/flask/snaps-utilities.test.js | 127 ++++++++++++++++++++++ app/scripts/metamask-controller.js | 34 +----- jest.config.js | 1 + 5 files changed, 192 insertions(+), 29 deletions(-) create mode 100644 app/scripts/flask/snaps-blocklist.js create mode 100644 app/scripts/flask/snaps-utilities.js create mode 100644 app/scripts/flask/snaps-utilities.test.js diff --git a/app/scripts/flask/snaps-blocklist.js b/app/scripts/flask/snaps-blocklist.js new file mode 100644 index 000000000..470eb828b --- /dev/null +++ b/app/scripts/flask/snaps-blocklist.js @@ -0,0 +1,24 @@ +/** + * Represents a list of Snaps that are not allowed to be used. + * Can be blocked by [ID, VERSION] or SHASUM of a source code (or both). + * + * Example: + * { + * id: 'npm:@consensys/snap-id', + * versionRange: '<0.1.11', + * shasum: 'TEIbWsAyQe/8rBNXOHx3bOP9YF61PIPP/YHeokLchJE=', + * }, + * { + * shasum: 'eCYGZiYvZ3/uxkKI3npfl79kTQXS/5iD9ojsBS4A3rI=', + * }, + */ +export const SNAP_BLOCKLIST = [ + { + id: 'npm:@consensys/starknet-snap', + versionRange: '<0.1.11', + }, + { + // @consensys/starknet-snap v:0.1.10 + shasum: 'A83r5/ZIcKuKwuAnQHHByVFCuofj7jGK5hOStmHY6A0=', + }, +]; diff --git a/app/scripts/flask/snaps-utilities.js b/app/scripts/flask/snaps-utilities.js new file mode 100644 index 000000000..98bd2d340 --- /dev/null +++ b/app/scripts/flask/snaps-utilities.js @@ -0,0 +1,35 @@ +import { satisfies as satisfiesSemver } from 'semver'; + +/** + * Checks if provided snaps are on the block list. + * + * @param snapsToCheck - An object containing snap ids and other information. + * @param blocklist - An object containing snap ids, version or shasum of the blocked snaps. + * @returns An object structure containing snaps block information. + */ +async function checkSnapsBlockList(snapsToCheck, blocklist) { + return Object.entries(snapsToCheck).reduce((acc, [snapId, snapInfo]) => { + const blockInfo = blocklist.find( + (blocked) => + (blocked.id === snapId && + satisfiesSemver(snapInfo.version, blocked.versionRange, { + includePrerelease: true, + })) || + // Check for null/undefined for a case in which SnapController did not return + // a valid message. This will avoid blocking all snaps in the given case. + // Avoid having (undefined === undefined). + (blocked.shasum ? blocked.shasum === snapInfo.shasum : false), + ); + + acc[snapId] = blockInfo + ? { + blocked: true, + reason: blockInfo.reason, + infoUrl: blockInfo.infoUrl, + } + : { blocked: false }; + return acc; + }, {}); +} + +export { checkSnapsBlockList }; diff --git a/app/scripts/flask/snaps-utilities.test.js b/app/scripts/flask/snaps-utilities.test.js new file mode 100644 index 000000000..abb8afa3a --- /dev/null +++ b/app/scripts/flask/snaps-utilities.test.js @@ -0,0 +1,127 @@ +import { strict as assert } from 'assert'; +import { checkSnapsBlockList } from './snaps-utilities'; + +describe('Snaps Controller utilities', function () { + describe('checkSnapsBlockList', function () { + it('returns one of the given snaps as blocked by its version', async function () { + const mockBlocklist = [ + { + id: 'npm:@consensys/starknet-snap', + versionRange: '<0.1.11', + }, + ]; + const mockSnapsToBeChecked = { + 'npm:exampleA': { + version: '1.0.0', + shasum: 'F5IapP6v1Bp7bl16NkCszfOhtVSZAm362X5zl7wgMhI=', + }, + 'npm:exampleB': { + version: '1.0.0', + shasum: 'eCYGZiYvZ3/uxkKI3npfl79kTQXS/5iD9ojsBS4A3rI=', + }, + 'npm:@consensys/starknet-snap': { + version: '0.1.10', + shasum: 'A83r5/ZIcKuKwuAnQHHByVFCuofj7jGK5hOStmHY6A0=', + }, + }; + + const blockedSnaps = await checkSnapsBlockList( + mockSnapsToBeChecked, + mockBlocklist, + ); + assert.deepEqual(blockedSnaps, { + 'npm:exampleA': { blocked: false }, + 'npm:exampleB': { blocked: false }, + 'npm:@consensys/starknet-snap': { + blocked: true, + reason: undefined, + infoUrl: undefined, + }, + }); + }); + + it('returns given snap as blocked by its shasum', async function () { + const mockBlocklist = [ + { + shasum: 'A83r5/ZIcKuKwuAnQHHByVFCuofj7jGK5hOStmHY6A0=', + }, + ]; + const mockSnapsToBeChecked = { + 'npm:@consensys/starknet-snap': { + version: '0.3.15', // try to fake version with the same source sha + shasum: 'A83r5/ZIcKuKwuAnQHHByVFCuofj7jGK5hOStmHY6A0=', + }, + }; + + const blockedSnaps = await checkSnapsBlockList( + mockSnapsToBeChecked, + mockBlocklist, + ); + assert.deepEqual(blockedSnaps, { + 'npm:@consensys/starknet-snap': { + blocked: true, + reason: undefined, + infoUrl: undefined, + }, + }); + }); + + it('returns false for blocked for the same blocklisted snap but different version', async function () { + const mockBlocklist = [ + { + id: 'npm:@consensys/starknet-snap', + versionRange: '<0.1.11', + }, + ]; + const mockSnapsToBeChecked = { + 'npm:@consensys/starknet-snap': { + version: '0.2.1', + shasum: 'Z4jo37WG1E2rxqF05WaXOSUDxR5upUmOdaTvmgVY/L0=', + }, + }; + + const blockedSnaps = await checkSnapsBlockList( + mockSnapsToBeChecked, + mockBlocklist, + ); + assert.deepEqual(blockedSnaps, { + 'npm:@consensys/starknet-snap': { + blocked: false, + }, + }); + }); + + it('returns false for blocked for multiple snaps that are not on the blocklist', async function () { + const mockBlocklist = [ + { + id: 'npm:@consensys/starknet-snap', + versionRange: '<0.1.11', + }, + ]; + const mockSnapsToBeChecked = { + 'npm:exampleA': { + version: '1.0.0', + shasum: 'F5IapP6v1Bp7bl16NkCszfOhtVSZAm362X5zl7wgMhI=', + }, + 'npm:exampleB': { + version: '2.1.3', + shasum: 'eCYGZiYvZ3/uxkKI3npfl79kTQXS/5iD9ojsBS4A3rI=', + }, + 'npm:exampleC': { + version: '3.7.9', + shasum: '2QqUxo5joo4kKKr7yiCjdYsZOZcIFBnIBEdwU9Yx7+M=', + }, + }; + + const blockedSnaps = await checkSnapsBlockList( + mockSnapsToBeChecked, + mockBlocklist, + ); + assert.deepEqual(blockedSnaps, { + 'npm:exampleA': { blocked: false }, + 'npm:exampleB': { blocked: false }, + 'npm:exampleC': { blocked: false }, + }); + }); + }); +}); diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 5f646eb38..72efbeb35 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -50,7 +50,6 @@ import { SnapController, IframeExecutionService, } from '@metamask/snap-controllers'; -import { satisfies as satisfiesSemver } from 'semver'; ///: END:ONLY_INCLUDE_IN import { @@ -156,6 +155,10 @@ import { ///: END:ONLY_INCLUDE_IN } from './controllers/permissions'; import createRPCMethodTrackingMiddleware from './lib/createRPCMethodTrackingMiddleware'; +///: BEGIN:ONLY_INCLUDE_IN(flask) +import { checkSnapsBlockList } from './flask/snaps-utilities'; +import { SNAP_BLOCKLIST } from './flask/snaps-blocklist'; +///: END:ONLY_INCLUDE_IN export const METAMASK_CONTROLLER_EVENTS = { // Fired after state changes that impact the extension badge (unapproved msg count) @@ -685,13 +688,6 @@ export default class MetamaskController extends EventEmitter { ], }); - const SNAP_BLOCKLIST = [ - { - id: 'npm:@consensys/starknet-snap', - versionRange: '<0.1.11', - }, - ]; - this.snapController = new SnapController({ environmentEndowmentPermissions: Object.values(EndowmentPermissions), closeAllConnections: this.removeAllConnections.bind(this), @@ -701,27 +697,7 @@ export default class MetamaskController extends EventEmitter { return this.getAppKeyForSubject(`${appKeyType}:${subject}`); }, checkBlockList: async (snapsToCheck) => { - return Object.entries(snapsToCheck).reduce( - (acc, [snapId, snapVersion]) => { - const blockInfo = SNAP_BLOCKLIST.find( - (blocked) => - blocked.id === snapId && - satisfiesSemver(snapVersion, blocked.versionRange, { - includePrerelease: true, - }), - ); - - const cur = blockInfo - ? { - blocked: true, - reason: blockInfo.reason, - infoUrl: blockInfo.infoUrl, - } - : { blocked: false }; - return { ...acc, [snapId]: cur }; - }, - {}, - ); + return checkSnapsBlockList(snapsToCheck, SNAP_BLOCKLIST); }, state: initState.SnapController, messenger: snapControllerMessenger, diff --git a/jest.config.js b/jest.config.js index e17379e8f..04b26dfe6 100644 --- a/jest.config.js +++ b/jest.config.js @@ -51,6 +51,7 @@ module.exports = { '/app/scripts/platforms/*.test.js', 'app/scripts/controllers/network/**/*.test.js', '/app/scripts/controllers/permissions/**/*.test.js', + '/app/scripts/flask/**/*.test.js', '/app/scripts/lib/createRPCMethodTrackingMiddleware.test.js', '/app/scripts/constants/error-utils.test.js', ],