diff --git a/app/scripts/migrations/121.1.test.ts b/app/scripts/migrations/121.1.test.ts index 56d14bdcf415..1d2d6f9ab70e 100644 --- a/app/scripts/migrations/121.1.test.ts +++ b/app/scripts/migrations/121.1.test.ts @@ -1,7 +1,14 @@ import { AccountsControllerState } from '@metamask/accounts-controller'; +import { cloneDeep } from 'lodash'; import { createMockInternalAccount } from '../../../test/jest/mocks'; import { migrate, version } from './121.1'; +const sentryCaptureExceptionMock = jest.fn(); + +global.sentry = { + captureException: sentryCaptureExceptionMock, +}; + const oldVersion = 121; const mockInternalAccount = createMockInternalAccount(); @@ -25,7 +32,8 @@ describe('migration #121.1', () => { }, }; - const newStorage = await migrate(oldStorage); + const newStorage = await migrate(cloneDeep(oldStorage)); + expect(newStorage.meta).toStrictEqual({ version }); }); @@ -43,14 +51,15 @@ describe('migration #121.1', () => { }, }; - const newStorage = await migrate(oldStorage); - const { - internalAccounts: { selectedAccount }, - } = newStorage.data.AccountsController as AccountsControllerState; - expect(selectedAccount).toStrictEqual(mockInternalAccount.id); - expect(newStorage.data.AccountsController).toStrictEqual( - mockAccountsControllerState, - ); + const newStorage = await migrate(cloneDeep(oldStorage)); + + expect(newStorage.data.AccountsController).toStrictEqual({ + ...mockAccountsControllerState, + internalAccounts: { + ...mockAccountsControllerState.internalAccounts, + selectedAccount: mockInternalAccount.id, + }, + }); }); it('does nothing if the selectedAccount is found in the list of accounts', async () => { @@ -61,9 +70,169 @@ describe('migration #121.1', () => { }, }; - const newStorage = await migrate(oldStorage); - expect(newStorage.data.AccountsController).toStrictEqual( - mockAccountsControllerState, - ); + const newStorage = await migrate(cloneDeep(oldStorage)); + + expect(newStorage.data).toStrictEqual(oldStorage.data); + }); + + it('does nothing if AccountsController state is missing', async () => { + const oldStorage = { + meta: { version: oldVersion }, + data: { + OtherController: {}, + }, + }; + + const newStorage = await migrate(cloneDeep(oldStorage)); + + expect(newStorage.data).toStrictEqual(oldStorage.data); + }); + + it('does nothing if there are no accounts', async () => { + const oldStorage = { + meta: { version: oldVersion }, + data: { + AccountsController: { + ...mockAccountsControllerState, + internalAccounts: { + ...mockAccountsControllerState.internalAccounts, + accounts: {}, + }, + }, + }, + }; + + const newStorage = await migrate(cloneDeep(oldStorage)); + + expect(newStorage.data).toStrictEqual(oldStorage.data); + }); + + it('does nothing if selectedAccount is unset', async () => { + const oldStorage = { + meta: { version: oldVersion }, + data: { + AccountsController: { + ...mockAccountsControllerState, + internalAccounts: { + ...mockAccountsControllerState.internalAccounts, + selectedAccount: '', + }, + }, + }, + }; + + const newStorage = await migrate(cloneDeep(oldStorage)); + + expect(newStorage.data).toStrictEqual(oldStorage.data); }); + + const invalidState = [ + { + errorMessage: `Migration ${version}: Invalid AccountsController state of type 'string'`, + label: 'AccountsController type', + state: { AccountsController: 'invalid' }, + }, + { + errorMessage: `Migration ${version}: Invalid AccountsController state, missing internalAccounts`, + label: 'Missing internalAccounts', + state: { AccountsController: {} }, + }, + { + errorMessage: `Migration ${version}: Invalid AccountsController internalAccounts state of type 'string'`, + label: 'Invalid internalAccounts', + state: { AccountsController: { internalAccounts: 'invalid' } }, + }, + { + errorMessage: `Migration ${version}: Invalid AccountsController internalAccounts state, missing selectedAccount`, + label: 'Missing selectedAccount', + state: { AccountsController: { internalAccounts: { accounts: {} } } }, + }, + { + errorMessage: `Migration ${version}: Invalid AccountsController internalAccounts.selectedAccount state of type 'object'`, + label: 'Invalid selectedAccount', + state: { + AccountsController: { + internalAccounts: { accounts: {}, selectedAccount: {} }, + }, + }, + }, + { + errorMessage: `Migration ${version}: Invalid AccountsController internalAccounts state, missing accounts`, + label: 'Missing accounts', + state: { + AccountsController: { + internalAccounts: { selectedAccount: '' }, + }, + }, + }, + { + errorMessage: `Migration ${version}: Invalid AccountsController internalAccounts.accounts state of type 'string'`, + label: 'Missing accounts', + state: { + AccountsController: { + internalAccounts: { accounts: 'invalid', selectedAccount: '' }, + }, + }, + }, + { + errorMessage: `Migration ${version}: Invalid AccountsController internalAccounts.accounts state, entry found of type 'string'`, + label: 'Account entry type', + state: { + AccountsController: { + internalAccounts: { + accounts: { [mockInternalAccount.id]: 'invalid' }, + selectedAccount: 'unknown id', + }, + }, + }, + }, + { + errorMessage: `Migration ${version}: Invalid AccountsController internalAccounts.accounts state, entry found that is missing an id`, + label: 'Account entry missing ID', + state: { + AccountsController: { + internalAccounts: { + accounts: { [mockInternalAccount.id]: {} }, + selectedAccount: 'unknown id', + }, + }, + }, + }, + { + errorMessage: `Migration ${version}: Invalid AccountsController internalAccounts.accounts state, entry found with an id of type 'object'`, + label: 'Account entry missing ID', + state: { + AccountsController: { + internalAccounts: { + accounts: { [mockInternalAccount.id]: { id: {} } }, + selectedAccount: 'unknown id', + }, + }, + }, + }, + ]; + + // @ts-expect-error 'each' function missing from type definitions, but it does exist + it.each(invalidState)( + 'captures error when state is invalid due to: $label', + async ({ + errorMessage, + state, + }: { + errorMessage: string; + state: Record; + }) => { + const oldStorage = { + meta: { version: oldVersion }, + data: state, + }; + + const newStorage = await migrate(cloneDeep(oldStorage)); + + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error(errorMessage), + ); + expect(newStorage.data).toStrictEqual(oldStorage.data); + }, + ); }); diff --git a/app/scripts/migrations/121.1.ts b/app/scripts/migrations/121.1.ts index 0e979297b6b4..f81893bb00e8 100644 --- a/app/scripts/migrations/121.1.ts +++ b/app/scripts/migrations/121.1.ts @@ -1,5 +1,6 @@ -import { AccountsControllerState } from '@metamask/accounts-controller'; -import { cloneDeep } from 'lodash'; +import { hasProperty } from '@metamask/utils'; +import { cloneDeep, isObject } from 'lodash'; +import log from 'loglevel'; type VersionedData = { meta: { version: number }; @@ -9,7 +10,8 @@ type VersionedData = { export const version = 121.1; /** - * This migration removes depreciated `Txcontroller` key if it is present in state. + * Fix AccountsController state corruption, where the `selectedAccount` state is set to an invalid + * ID. * * @param originalVersionedData - Versioned MetaMask extension state, exactly * what we persist to dist. @@ -28,22 +30,114 @@ export async function migrate( return versionedData; } -function transformState(state: Record) { - const accountsControllerState = state?.AccountsController as - | AccountsControllerState - | undefined; +function transformState(state: Record): void { + if (!hasProperty(state, 'AccountsController')) { + return; + } + + const accountsControllerState = state.AccountsController; + + if (!isObject(accountsControllerState)) { + global.sentry?.captureException( + new Error( + `Migration ${version}: Invalid AccountsController state of type '${typeof accountsControllerState}'`, + ), + ); + return; + } else if (!hasProperty(accountsControllerState, 'internalAccounts')) { + global.sentry?.captureException( + new Error( + `Migration ${version}: Invalid AccountsController state, missing internalAccounts`, + ), + ); + return; + } else if (!isObject(accountsControllerState.internalAccounts)) { + global.sentry?.captureException( + new Error( + `Migration ${version}: Invalid AccountsController internalAccounts state of type '${typeof accountsControllerState.internalAccounts}'`, + ), + ); + return; + } else if ( + !hasProperty(accountsControllerState.internalAccounts, 'selectedAccount') + ) { + global.sentry?.captureException( + new Error( + `Migration ${version}: Invalid AccountsController internalAccounts state, missing selectedAccount`, + ), + ); + return; + } else if ( + typeof accountsControllerState.internalAccounts.selectedAccount !== 'string' + ) { + global.sentry?.captureException( + new Error( + `Migration ${version}: Invalid AccountsController internalAccounts.selectedAccount state of type '${typeof accountsControllerState + .internalAccounts.selectedAccount}'`, + ), + ); + return; + } else if ( + !hasProperty(accountsControllerState.internalAccounts, 'accounts') + ) { + global.sentry?.captureException( + new Error( + `Migration ${version}: Invalid AccountsController internalAccounts state, missing accounts`, + ), + ); + return; + } else if (!isObject(accountsControllerState.internalAccounts.accounts)) { + global.sentry?.captureException( + new Error( + `Migration ${version}: Invalid AccountsController internalAccounts.accounts state of type '${typeof accountsControllerState + .internalAccounts.accounts}'`, + ), + ); + return; + } + + if ( + Object.keys(accountsControllerState.internalAccounts.accounts).length === 0 + ) { + log.warn(`Migration ${version}: Skipping, no accounts found`); + return; + } else if (accountsControllerState.internalAccounts.selectedAccount === '') { + log.warn(`Migration ${version}: Skipping, no selected account set`); + return; + } + + const firstAccount = Object.values( + accountsControllerState.internalAccounts.accounts, + )[0]; + if (!isObject(firstAccount)) { + global.sentry?.captureException( + new Error( + `Migration ${version}: Invalid AccountsController internalAccounts.accounts state, entry found of type '${typeof firstAccount}'`, + ), + ); + return; + } else if (!hasProperty(firstAccount, 'id')) { + global.sentry?.captureException( + new Error( + `Migration ${version}: Invalid AccountsController internalAccounts.accounts state, entry found that is missing an id`, + ), + ); + return; + } else if (typeof firstAccount.id !== 'string') { + global.sentry?.captureException( + new Error( + `Migration ${version}: Invalid AccountsController internalAccounts.accounts state, entry found with an id of type '${typeof firstAccount.id}'`, + ), + ); + return; + } if ( - accountsControllerState && - Object.values(accountsControllerState?.internalAccounts.accounts).length > - 0 && - !accountsControllerState?.internalAccounts.accounts[ - accountsControllerState?.internalAccounts.selectedAccount - ] + !hasProperty( + accountsControllerState.internalAccounts.accounts, + accountsControllerState.internalAccounts.selectedAccount, + ) ) { - accountsControllerState.internalAccounts.selectedAccount = Object.values( - accountsControllerState?.internalAccounts.accounts, - )[0].id; + accountsControllerState.internalAccounts.selectedAccount = firstAccount.id; } - return state; }