From e3144c50bcd94b519b6d975887c472eb78e2017a Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Wed, 14 Aug 2024 19:18:40 -0230 Subject: [PATCH] fix: Delete invalid `SelectedNetworkController` state (#26428) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** The `SelectedNetworkController` state is cleared if any invalid `networkConfigurationId`s are found in state. We are seeing reports of this happening in production in v12.0.1. The suspected cause is `NetworkController` state corruption. We resolved a few cases of this in v12.0.1, but for users that were affected by this, the invalid IDs may have propogated to the `SelectedNetworkController` state already. That is what this migration intends to fix. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/26428?quickstart=1) ## **Related issues** Fixes #26309 ## **Manual testing steps** We don't have clear reproduction steps for the bug itself. To artificially reproduce the scenario by changing extension state, this should work: * Create a dev build from v12.0.2 * Install the dev build from the `dist/chrome` directory and proceed through onboarding * Visit the test dapp, refresh, and connect to it. * Run this command in the background console: ``` chrome.storage.local.get( null, (state) => { state.data.SelectedNetworkController.domains['https://metamask.github.io'] = '123'; chrome.storage.local.set(state, () => chrome.runtime.reload()); } ); ``` * The linked error should now appear in the console of the popup * Disable the extension * Switch to this branch and create a dev build * Enable and reload the extension * The error should no longer appear. ## **Screenshots/Recordings** N/A ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- app/scripts/migrations/120.5.test.ts | 255 +++++++++++++++++++++++++++ app/scripts/migrations/120.5.ts | 126 +++++++++++++ 2 files changed, 381 insertions(+) create mode 100644 app/scripts/migrations/120.5.test.ts create mode 100644 app/scripts/migrations/120.5.ts diff --git a/app/scripts/migrations/120.5.test.ts b/app/scripts/migrations/120.5.test.ts new file mode 100644 index 000000000000..1b19e5e66e95 --- /dev/null +++ b/app/scripts/migrations/120.5.test.ts @@ -0,0 +1,255 @@ +import { cloneDeep } from 'lodash'; +import { migrate, version } from './120.5'; + +const oldVersion = 120.4; + +describe('migration #120.5', () => { + afterEach(() => { + jest.resetAllMocks(); + }); + + it('updates the version metadata', async () => { + const oldStorage = { + meta: { version: oldVersion }, + data: {}, + }; + + const newStorage = await migrate(cloneDeep(oldStorage)); + + expect(newStorage.meta).toStrictEqual({ version }); + }); + + it('does nothing if SelectedNetworkController state is not set', async () => { + const oldState = { + NetworkController: { + networkConfigurations: { + 123: {}, + }, + }, + }; + + const transformedState = await migrate({ + meta: { version: oldVersion }, + data: cloneDeep(oldState), + }); + + expect(transformedState.data).toEqual(oldState); + }); + + it('deletes the SelectedNetworkController state if it is corrupted', async () => { + const oldState = { + NetworkController: { + networkConfigurations: { + 123: {}, + }, + }, + SelectedNetworkController: 'invalid', + }; + + const transformedState = await migrate({ + meta: { version: oldVersion }, + data: cloneDeep(oldState), + }); + + expect(transformedState.data).toEqual({ + NetworkController: { + networkConfigurations: { + 123: {}, + }, + }, + }); + }); + + it('deletes the SelectedNetworkController state if it is missing the domains state', async () => { + const oldState = { + NetworkController: { + networkConfigurations: { + 123: {}, + }, + }, + SelectedNetworkController: { + somethingElse: {}, + }, + }; + + const transformedState = await migrate({ + meta: { version: oldVersion }, + data: cloneDeep(oldState), + }); + + expect(transformedState.data).toEqual({ + NetworkController: { + networkConfigurations: { + 123: {}, + }, + }, + }); + }); + + it('deletes the SelectedNetworkController state if the domains state is corrupted', async () => { + const oldState = { + NetworkController: { + networkConfigurations: { + 123: {}, + }, + }, + SelectedNetworkController: { + domains: 'invalid', + }, + }; + + const transformedState = await migrate({ + meta: { version: oldVersion }, + data: cloneDeep(oldState), + }); + + expect(transformedState.data).toEqual({ + NetworkController: { + networkConfigurations: { + 123: {}, + }, + }, + }); + }); + + it('deletes the SelectedNetworkController state if NetworkController state is missing', async () => { + const oldState = { + SelectedNetworkController: { + domains: {}, + }, + }; + + const transformedState = await migrate({ + meta: { version: oldVersion }, + data: cloneDeep(oldState), + }); + + expect(transformedState.data).toEqual({}); + }); + + it('deletes the SelectedNetworkController state if NetworkController state is corrupted', async () => { + const oldState = { + NetworkController: 'invalid', + SelectedNetworkController: { + domains: {}, + }, + }; + + const transformedState = await migrate({ + meta: { version: oldVersion }, + data: cloneDeep(oldState), + }); + + expect(transformedState.data).toEqual({ + NetworkController: 'invalid', + }); + }); + + it('deletes the SelectedNetworkController state if NetworkController has no networkConfigurations', async () => { + const oldState = { + NetworkController: {}, + SelectedNetworkController: { + domains: {}, + }, + }; + + const transformedState = await migrate({ + meta: { version: oldVersion }, + data: cloneDeep(oldState), + }); + + expect(transformedState.data).toEqual({ + NetworkController: {}, + }); + }); + + it('deletes the SelectedNetworkController state if NetworkController networkConfigurations state is corrupted', async () => { + const oldState = { + NetworkController: { networkConfigurations: 'invalid' }, + SelectedNetworkController: { + domains: {}, + }, + }; + + const transformedState = await migrate({ + meta: { version: oldVersion }, + data: cloneDeep(oldState), + }); + + expect(transformedState.data).toEqual({ + NetworkController: { networkConfigurations: 'invalid' }, + }); + }); + + it('does nothing if SelectedNetworkController domains state is empty', async () => { + const oldState = { + NetworkController: { networkConfigurations: {} }, + SelectedNetworkController: { + domains: {}, + }, + }; + + const transformedState = await migrate({ + meta: { version: oldVersion }, + data: cloneDeep(oldState), + }); + + expect(transformedState.data).toEqual(oldState); + }); + + it('does nothing if SelectedNetworkController domains state is valid', async () => { + const oldState = { + NetworkController: { + networkConfigurations: { + 123: {}, + }, + }, + SelectedNetworkController: { + domains: { + 'example1.test': '123', + 'example2.test': 'mainnet', + 'example3.test': 'goerli', + 'example4.test': 'sepolia', + 'example5.test': 'linea-goerli', + 'example6.test': 'linea-sepolia', + 'example7.test': 'linea-mainnet', + }, + }, + }; + + const transformedState = await migrate({ + meta: { version: oldVersion }, + data: cloneDeep(oldState), + }); + + expect(transformedState.data).toEqual(oldState); + }); + + it('deletes the SelectedNetworkController state if an invalid networkConfigurationId is found', async () => { + const oldState = { + NetworkController: { + networkConfigurations: { + 123: {}, + }, + }, + SelectedNetworkController: { + domains: { + 'domain.test': '456', + }, + }, + }; + + const transformedState = await migrate({ + meta: { version: oldVersion }, + data: cloneDeep(oldState), + }); + + expect(transformedState.data).toEqual({ + NetworkController: { + networkConfigurations: { + 123: {}, + }, + }, + }); + }); +}); diff --git a/app/scripts/migrations/120.5.ts b/app/scripts/migrations/120.5.ts new file mode 100644 index 000000000000..11d1fba86664 --- /dev/null +++ b/app/scripts/migrations/120.5.ts @@ -0,0 +1,126 @@ +import { hasProperty, isObject } from '@metamask/utils'; +import { cloneDeep } from 'lodash'; + +type VersionedData = { + meta: { version: number }; + data: Record; +}; + +export const version = 120.5; + +/** + * This migration removes invalid network configuration IDs from the SelectedNetworkController. + * + * @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; +} + +/** + * A list of InfuraNetworkType values from extension v12.0.1 + * This version of the extension uses `@metamask/network-controller@18.1.2`, which in turn uses + * the types from `@metamask/controller-utils@9.1.0` + * + * See https://github.com/MetaMask/core/blob/34542cf6e808f294fd83c7c5f70d1bc7418f8a9e/packages/controller-utils/src/types.ts#L4 + * + * Hard-coded here rather than imported so that this migration continues to work correctly as these + * constants get updated in the future. + */ +const infuraNetworkTypes = [ + 'mainnet', + 'goerli', + 'sepolia', + 'linea-goerli', + 'linea-sepolia', + 'linea-mainnet', +]; + +/** + * Remove invalid network configuration IDs from the SelectedNetworkController. + * + * @param state - The persisted MetaMask state, keyed by controller. + */ +function transformState(state: Record): void { + if (!hasProperty(state, 'SelectedNetworkController')) { + return; + } + if (!isObject(state.SelectedNetworkController)) { + console.error( + `Migration ${version}: Invalid SelectedNetworkController state of type '${typeof state.SelectedNetworkController}'`, + ); + delete state.SelectedNetworkController; + return; + } else if (!hasProperty(state.SelectedNetworkController, 'domains')) { + console.error( + `Migration ${version}: Missing SelectedNetworkController domains state`, + ); + delete state.SelectedNetworkController; + return; + } else if (!isObject(state.SelectedNetworkController.domains)) { + console.error( + `Migration ${version}: Invalid SelectedNetworkController domains state of type '${typeof state + .SelectedNetworkController.domains}'`, + ); + delete state.SelectedNetworkController; + return; + } + + if (!hasProperty(state, 'NetworkController')) { + delete state.SelectedNetworkController; + return; + } else if (!isObject(state.NetworkController)) { + console.error( + new Error( + `Migration ${version}: Invalid NetworkController state of type '${typeof state.NetworkController}'`, + ), + ); + delete state.SelectedNetworkController; + return; + } else if (!hasProperty(state.NetworkController, 'networkConfigurations')) { + delete state.SelectedNetworkController; + return; + } else if (!isObject(state.NetworkController.networkConfigurations)) { + console.error( + new Error( + `Migration ${version}: Invalid NetworkController networkConfigurations state of type '${typeof state.NetworkController}'`, + ), + ); + delete state.SelectedNetworkController; + return; + } + + const validNetworkConfigurationIds = [ + ...infuraNetworkTypes, + ...Object.keys(state.NetworkController.networkConfigurations), + ]; + const domainMappedNetworkConfigurationIds = Object.values( + state.SelectedNetworkController.domains, + ); + + for (const configurationId of domainMappedNetworkConfigurationIds) { + if ( + typeof configurationId !== 'string' || + !validNetworkConfigurationIds.includes(configurationId) + ) { + console.error( + new Error( + `Migration ${version}: Invalid networkConfigurationId found in SelectedNetworkController state: '${configurationId}'`, + ), + ); + delete state.SelectedNetworkController; + return; + } + } +}