From 3fad390fa22e781286e63a22927e86f8489a0bde Mon Sep 17 00:00:00 2001 From: Guillaume Roux Date: Fri, 22 Nov 2024 16:30:26 +0100 Subject: [PATCH 1/3] Migration to delete `snap_manageAccounts` from the snap permissions --- app/scripts/migrations/132.test.ts | 182 +++++++++++++++++++++++++++++ app/scripts/migrations/132.ts | 96 +++++++++++++++ 2 files changed, 278 insertions(+) create mode 100644 app/scripts/migrations/132.test.ts create mode 100644 app/scripts/migrations/132.ts diff --git a/app/scripts/migrations/132.test.ts b/app/scripts/migrations/132.test.ts new file mode 100644 index 000000000000..a0683590a602 --- /dev/null +++ b/app/scripts/migrations/132.test.ts @@ -0,0 +1,182 @@ +import { AccountsControllerState } from '@metamask/accounts-controller'; +import { cloneDeep } from 'lodash'; +import { createMockInternalAccount } from '../../../test/jest/mocks'; +import { migrate, version } from './132'; + +const oldVersion = 131; + +const mockSnapControllerState = { + snaps: { + foobar: { + id: 'foobar', + }, + }, +}; + +const mockPermissionControllerState = { + subjects: { + foobar: { + permissions: { + snap_manageAccounts: {}, + }, + }, + }, +}; + +describe(`migration #${version}`, () => { + afterEach(() => jest.resetAllMocks()); + + it('updates the version metadata', async () => { + const oldStorage = { + meta: { version: oldVersion }, + data: { + SnapController: mockSnapControllerState, + PermissionController: mockPermissionControllerState, + }, + }; + + const newStorage = await migrate(cloneDeep(oldStorage)); + + expect(newStorage.meta).toStrictEqual({ version }); + }); + + it('does nothing if SnapController is not present', async () => { + const oldStorage = { + meta: { version: oldVersion }, + data: { + PermissionController: mockPermissionControllerState, + }, + }; + + const newStorage = await migrate(cloneDeep(oldStorage)); + + expect(newStorage.data).toStrictEqual(oldStorage.data); + }); + + it('does nothing if PermissionController is not present', async () => { + const oldStorage = { + meta: { version: oldVersion }, + data: { + SnapController: mockSnapControllerState, + }, + }; + + const newStorage = await migrate(cloneDeep(oldStorage)); + + expect(newStorage.data).toStrictEqual(oldStorage.data); + }); + + it('does nothing if there is no snaps', async () => { + const oldStorage = { + meta: { version: oldVersion }, + data: { + SnapController: {}, + PermissionController: mockPermissionControllerState, + }, + }; + + const newStorage = await migrate(cloneDeep(oldStorage)); + + expect(newStorage.data).toStrictEqual(oldStorage.data); + }); + + it('does nothing if there are no permission subjects', async () => { + const oldStorage = { + meta: { version: oldVersion }, + data: { + SnapController: mockSnapControllerState, + PermissionController: {}, + }, + }; + + const newStorage = await migrate(cloneDeep(oldStorage)); + + expect(newStorage.data).toStrictEqual(oldStorage.data); + }); + + it('does nothing if the snap does not have a corresponding permission subject', async () => { + const oldStorage = { + meta: { version: oldVersion }, + data: { + SnapController: { + snaps: { + foobar: { + id: 'foobar', + }, + }, + }, + PermissionController: { + subjects: {}, + }, + }, + }; + + const newStorage = await migrate(cloneDeep(oldStorage)); + + expect(newStorage.data).toStrictEqual(oldStorage.data); + }); + + it('does nothing if the snap does not have a snap_manageAccounts permission', async () => { + const oldStorage = { + meta: { version: oldVersion }, + data: { + SnapController: { + snaps: { + foobar: { + id: 'foobar', + }, + }, + }, + PermissionController: { + subjects: { + foobar: { + permissions: { + snap_notify: {}, + }, + }, + }, + }, + }, + }; + + const newStorage = await migrate(cloneDeep(oldStorage)); + + expect(newStorage.data).toStrictEqual(oldStorage.data); + }); + + it('removes the snap_manageAccounts permission from the snap', async () => { + const oldStorage = { + meta: { version: oldVersion }, + data: { + SnapController: mockSnapControllerState, + PermissionController: { + subjects: { + foobar: { + permissions: { + snap_notify: {}, + snap_manageAccounts: {}, + }, + }, + }, + }, + }, + }; + + const expectedData = { + SnapController: mockSnapControllerState, + PermissionController: { + subjects: { + foobar: { + permissions: { + snap_notify: {}, + }, + }, + }, + }, + }; + + const newStorage = await migrate(cloneDeep(oldStorage)); + + expect(newStorage.data).toStrictEqual(expectedData); + }); +}); diff --git a/app/scripts/migrations/132.ts b/app/scripts/migrations/132.ts new file mode 100644 index 000000000000..ce4e690eb13f --- /dev/null +++ b/app/scripts/migrations/132.ts @@ -0,0 +1,96 @@ +import { + CaveatSpecificationConstraint, + ExtractPermission, + PermissionControllerState, + PermissionSpecificationConstraint, +} from '@metamask/permission-controller'; +import { SnapControllerState } from '@metamask/snaps-controllers'; +import { filterRemovedPermissions } from '@metamask/snaps-rpc-methods'; +import { hasProperty, isObject } from '@metamask/utils'; +import { cloneDeep } from 'lodash'; +import log from 'loglevel'; + +type VersionedData = { + meta: { version: number }; + data: Record; +}; + +type ActualPermissionControllerState = PermissionControllerState< + ExtractPermission< + PermissionSpecificationConstraint, + CaveatSpecificationConstraint + > +>; + +export const version = 132; + +/** + * Handle the `snap_manageAccounts` removal from the restricted methods of snap. + * + * @param originalVersionedData - Versioned MetaMask extension state, exactly + * what we persist to dist. + * @param originalVersionedData.meta - State metadata. + * @param originalVersionedData.meta.version - The current state version. + * @param originalVersionedData.data - The persisted MetaMask state, keyed by + * controller. + * @returns Updated versioned MetaMask extension state. + */ +export async function migrate( + originalVersionedData: VersionedData, +): Promise { + const versionedData = cloneDeep(originalVersionedData); + versionedData.meta.version = version; + transformState(versionedData.data); + return versionedData; +} + +function transformState(state: Record): void { + if ( + !hasProperty(state, 'SnapController') || + !isObject(state.SnapController) + ) { + return; + } + + if ( + !hasProperty(state, 'PermissionController') || + !isObject(state.PermissionController) + ) { + return; + } + + const snapControllerState = state.SnapController as SnapControllerState; + const permissionControllerState = + state.PermissionController as ActualPermissionControllerState; + + if ( + !isObject(snapControllerState) || + !hasProperty(snapControllerState, 'snaps') + ) { + return; + } + + if ( + !isObject(permissionControllerState) || + !hasProperty(permissionControllerState, 'subjects') + ) { + return; + } + + const snaps = snapControllerState.snaps; + const subjects = permissionControllerState.subjects; + + Object.keys(snaps).forEach((snapId) => { + if (subjects[snapId]) { + const newSnapPermissions = Object.fromEntries( + Object.entries(subjects[snapId].permissions).filter( + ([permissionName]) => permissionName !== 'snap_manageAccounts', + ), + ); + + (state.PermissionController as ActualPermissionControllerState).subjects[ + snapId + ].permissions = newSnapPermissions; + } + }); +} From 4e76b9565a479d613de9d05d6c67f4bbd853531c Mon Sep 17 00:00:00 2001 From: Guillaume Roux Date: Fri, 22 Nov 2024 17:33:11 +0100 Subject: [PATCH 2/3] Apply migration --- app/scripts/migrations/132.ts | 4 ++-- app/scripts/migrations/index.js | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/scripts/migrations/132.ts b/app/scripts/migrations/132.ts index ce4e690eb13f..823b5d803b9a 100644 --- a/app/scripts/migrations/132.ts +++ b/app/scripts/migrations/132.ts @@ -5,10 +5,8 @@ import { PermissionSpecificationConstraint, } from '@metamask/permission-controller'; import { SnapControllerState } from '@metamask/snaps-controllers'; -import { filterRemovedPermissions } from '@metamask/snaps-rpc-methods'; import { hasProperty, isObject } from '@metamask/utils'; import { cloneDeep } from 'lodash'; -import log from 'loglevel'; type VersionedData = { meta: { version: number }; @@ -93,4 +91,6 @@ function transformState(state: Record): void { ].permissions = newSnapPermissions; } }); + + console.log(state); } diff --git a/app/scripts/migrations/index.js b/app/scripts/migrations/index.js index d2c63eb2e35c..6cde292ba55d 100644 --- a/app/scripts/migrations/index.js +++ b/app/scripts/migrations/index.js @@ -152,6 +152,7 @@ const migrations = [ require('./129'), require('./130'), require('./131'), + require('./132'), ]; export default migrations; From b588170304a77563164354b055f63c59b3bc54ae Mon Sep 17 00:00:00 2001 From: Guillaume Roux Date: Mon, 25 Nov 2024 14:17:45 +0100 Subject: [PATCH 3/3] fix lint --- app/scripts/migrations/132.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/scripts/migrations/132.ts b/app/scripts/migrations/132.ts index 823b5d803b9a..29a1074a23cf 100644 --- a/app/scripts/migrations/132.ts +++ b/app/scripts/migrations/132.ts @@ -75,8 +75,8 @@ function transformState(state: Record): void { return; } - const snaps = snapControllerState.snaps; - const subjects = permissionControllerState.subjects; + const { snaps } = snapControllerState; + const { subjects } = permissionControllerState; Object.keys(snaps).forEach((snapId) => { if (subjects[snapId]) {