diff --git a/app/scripts/migrations/137.test.ts b/app/scripts/migrations/137.test.ts index 778f85265c2f..69735752f4db 100644 --- a/app/scripts/migrations/137.test.ts +++ b/app/scripts/migrations/137.test.ts @@ -104,25 +104,6 @@ describe('migration #137', () => { expect(newStorage.data).toStrictEqual(oldStorage.data); }); - it('does nothing if SelectedNetworkController state is missing', async () => { - const oldStorage = { - meta: { version: oldVersion }, - data: { - PermissionController: {}, - NetworkController: {}, - }, - }; - - const newStorage = await migrate(oldStorage); - - expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( - new Error( - `Migration ${version}: typeof state.SelectedNetworkController is undefined`, - ), - ); - expect(newStorage.data).toStrictEqual(oldStorage.data); - }); - it('does nothing if SelectedNetworkController state is not an object', async () => { const oldStorage = { meta: { version: oldVersion }, @@ -516,8 +497,194 @@ describe('migration #137', () => { domains: {}, }, }); + const baseEthAccountsPermissionMetadata = { + id: '1', + date: 2, + invoker: 'test.com', + parentCapability: PermissionNames.eth_accounts, + }; const currentScope = `eip155:${chainId}`; + it('skips eth_accounts and permittedChains permissions when they are missing metadata', async () => { + const oldStorage = { + meta: { version: oldVersion }, + data: { + ...baseData(), + PermissionController: { + subjects: { + 'test.com': { + permissions: { + unrelated: { + foo: 'bar', + }, + [PermissionNames.eth_accounts]: { + invoker: 'test.com', + parentCapability: PermissionNames.eth_accounts, + date: 2, + caveats: [ + { + type: 'restrictReturnedAccounts', + value: ['0xdeadbeef', '0x999'], + }, + ], + }, + [PermissionNames.permittedChains]: { + invoker: 'test.com', + parentCapability: PermissionNames.permittedChains, + date: 2, + caveats: [ + { + type: 'restrictNetworkSwitching', + value: ['0xa', '0x64'], + }, + ], + }, + }, + }, + }, + }, + }, + }; + + const newStorage = await migrate(oldStorage); + expect(newStorage.data).toStrictEqual({ + ...baseData(), + PermissionController: { + subjects: { + 'test.com': { + permissions: { + unrelated: { + foo: 'bar', + }, + }, + }, + }, + }, + }); + }); + + it('resolves a chainId for the origin even if there are other malformed network configurations', async () => { + const oldStorage = { + meta: { version: oldVersion }, + data: { + ...baseData(), + NetworkController: { + selectedNetworkClientId: 'mainnet', + networkConfigurationsByChainId: { + '0xInvalid': 'invalid-network-configuration', + '0x1': { + rpcEndpoints: [ + { + networkClientId: 'mainnet', + }, + ], + }, + '0xa': { + rpcEndpoints: [ + { + networkClientId: 'bar', + }, + ], + }, + }, + }, + SelectedNetworkController: { + domains: { + 'test.com': 'bar', + }, + }, + PermissionController: { + subjects: { + 'test.com': { + permissions: { + unrelated: { + foo: 'bar', + }, + [PermissionNames.eth_accounts]: { + ...baseEthAccountsPermissionMetadata, + caveats: [ + { + type: 'restrictReturnedAccounts', + value: ['0xdeadbeef', '0x999'], + }, + ], + }, + }, + }, + }, + }, + }, + }; + + const newStorage = await migrate(oldStorage); + expect(newStorage.data).toStrictEqual({ + ...baseData(), + NetworkController: { + selectedNetworkClientId: 'mainnet', + networkConfigurationsByChainId: { + '0xInvalid': 'invalid-network-configuration', + '0x1': { + rpcEndpoints: [ + { + networkClientId: 'mainnet', + }, + ], + }, + '0xa': { + rpcEndpoints: [ + { + networkClientId: 'bar', + }, + ], + }, + }, + }, + PermissionController: { + subjects: { + 'test.com': { + permissions: { + unrelated: { + foo: 'bar', + }, + 'endowment:caip25': { + ...baseEthAccountsPermissionMetadata, + parentCapability: 'endowment:caip25', + caveats: [ + { + type: 'authorizedScopes', + value: { + isMultichainOrigin: false, + requiredScopes: {}, + optionalScopes: { + 'eip155:10': { + accounts: [ + 'eip155:10:0xdeadbeef', + 'eip155:10:0x999', + ], + }, + 'wallet:eip155': { + accounts: [ + 'wallet:eip155:0xdeadbeef', + 'wallet:eip155:0x999', + ], + }, + }, + }, + }, + ], + }, + }, + }, + }, + }, + SelectedNetworkController: { + domains: { + 'test.com': 'bar', + }, + }, + }); + }); + it('replaces the eth_accounts permission with a CAIP-25 permission using the eth_accounts value for the currently selected chain id when the origin does not have its own network client', async () => { const oldStorage = { meta: { version: oldVersion }, @@ -531,6 +698,7 @@ describe('migration #137', () => { foo: 'bar', }, [PermissionNames.eth_accounts]: { + ...baseEthAccountsPermissionMetadata, caveats: [ { type: 'restrictReturnedAccounts', @@ -556,6 +724,7 @@ describe('migration #137', () => { foo: 'bar', }, 'endowment:caip25': { + ...baseEthAccountsPermissionMetadata, parentCapability: 'endowment:caip25', caveats: [ { @@ -588,7 +757,7 @@ describe('migration #137', () => { }); }); - it('replaces the eth_accounts permission with a CAIP-25 permission using the eth_accounts value for the currently selected chain id when the origin does have its own network client that cannot be resolved', async () => { + it('replaces the eth_accounts permission with a CAIP-25 permission using the globally selected chain id value for the currently selected chain id when the origin does have its own network client that cannot be resolved', async () => { const oldStorage = { meta: { version: oldVersion }, data: { @@ -606,6 +775,7 @@ describe('migration #137', () => { foo: 'bar', }, [PermissionNames.eth_accounts]: { + ...baseEthAccountsPermissionMetadata, caveats: [ { type: 'restrictReturnedAccounts', @@ -643,6 +813,7 @@ describe('migration #137', () => { foo: 'bar', }, 'endowment:caip25': { + ...baseEthAccountsPermissionMetadata, parentCapability: 'endowment:caip25', caveats: [ { @@ -693,6 +864,7 @@ describe('migration #137', () => { foo: 'bar', }, [PermissionNames.eth_accounts]: { + ...baseEthAccountsPermissionMetadata, caveats: [ { type: 'restrictReturnedAccounts', @@ -723,6 +895,7 @@ describe('migration #137', () => { foo: 'bar', }, 'endowment:caip25': { + ...baseEthAccountsPermissionMetadata, parentCapability: 'endowment:caip25', caveats: [ { @@ -768,6 +941,7 @@ describe('migration #137', () => { foo: 'bar', }, [PermissionNames.eth_accounts]: { + ...baseEthAccountsPermissionMetadata, caveats: [ { type: 'restrictReturnedAccounts', @@ -793,6 +967,7 @@ describe('migration #137', () => { foo: 'bar', }, 'endowment:caip25': { + ...baseEthAccountsPermissionMetadata, parentCapability: 'endowment:caip25', caveats: [ { @@ -850,6 +1025,7 @@ describe('migration #137', () => { foo: 'bar', }, [PermissionNames.eth_accounts]: { + ...baseEthAccountsPermissionMetadata, caveats: [ { type: 'restrictReturnedAccounts', @@ -893,6 +1069,7 @@ describe('migration #137', () => { foo: 'bar', }, 'endowment:caip25': { + ...baseEthAccountsPermissionMetadata, parentCapability: 'endowment:caip25', caveats: [ { @@ -938,6 +1115,7 @@ describe('migration #137', () => { foo: 'bar', }, [PermissionNames.permittedChains]: { + ...baseEthAccountsPermissionMetadata, caveats: [ { type: 'restrictNetworkSwitching', @@ -982,6 +1160,7 @@ describe('migration #137', () => { foo: 'bar', }, [PermissionNames.eth_accounts]: { + ...baseEthAccountsPermissionMetadata, caveats: [ { type: 'restrictReturnedAccounts', @@ -990,6 +1169,7 @@ describe('migration #137', () => { ], }, [PermissionNames.permittedChains]: { + ...baseEthAccountsPermissionMetadata, caveats: [ { type: 'restrictNetworkSwitching', @@ -1015,6 +1195,7 @@ describe('migration #137', () => { foo: 'bar', }, 'endowment:caip25': { + ...baseEthAccountsPermissionMetadata, parentCapability: 'endowment:caip25', caveats: [ { @@ -1063,6 +1244,7 @@ describe('migration #137', () => { 'test.com': { permissions: { [PermissionNames.eth_accounts]: { + ...baseEthAccountsPermissionMetadata, caveats: [ { type: 'restrictReturnedAccounts', @@ -1075,6 +1257,7 @@ describe('migration #137', () => { 'test2.com': { permissions: { [PermissionNames.eth_accounts]: { + ...baseEthAccountsPermissionMetadata, caveats: [ { type: 'restrictReturnedAccounts', @@ -1097,6 +1280,7 @@ describe('migration #137', () => { 'test.com': { permissions: { 'endowment:caip25': { + ...baseEthAccountsPermissionMetadata, parentCapability: 'endowment:caip25', caveats: [ { @@ -1121,6 +1305,7 @@ describe('migration #137', () => { 'test2.com': { permissions: { 'endowment:caip25': { + ...baseEthAccountsPermissionMetadata, parentCapability: 'endowment:caip25', caveats: [ { @@ -1148,117 +1333,4 @@ describe('migration #137', () => { }); }, ); - - it('skips malformed subjects while migrating valid ones', async () => { - const baseData = () => ({ - PermissionController: { - subjects: {}, - }, - SelectedNetworkController: { - domains: {}, - }, - NetworkController: { - selectedNetworkClientId: 'mainnet', - networkConfigurationsByChainId: {}, - }, - }); - const oldStorage = { - meta: { version: oldVersion }, - data: { - ...baseData(), - PermissionController: { - subjects: { - 'test.com': { - permissions: { - eth_accounts: { - caveats: [ - { - type: 'restrictReturnedAccounts', - value: ['0xdeadbeef'], - }, - ], - parentCapability: 'eth_accounts', - }, - }, - }, - 'malformed.com': 'invalid-subject', // This should be skipped - 'valid.com': { - permissions: { - eth_accounts: { - caveats: [ - { - type: 'restrictReturnedAccounts', - value: ['0x123'], - }, - ], - parentCapability: 'eth_accounts', - }, - }, - }, - }, - }, - }, - }; - - const newStorage = await migrate(oldStorage); - - expect(sentryCaptureExceptionMock).toHaveBeenCalledWith(expect.any(Error)); - - expect(newStorage.data).toStrictEqual({ - ...baseData(), - PermissionController: { - subjects: { - 'test.com': { - permissions: { - 'endowment:caip25': { - parentCapability: 'endowment:caip25', - caveats: [ - { - type: 'authorizedScopes', - value: { - requiredScopes: {}, - optionalScopes: { - 'wallet:eip155': { - accounts: ['wallet:eip155:0xdeadbeef'], - }, - 'eip155:1': { - accounts: ['eip155:1:0xdeadbeef'], - }, - }, - isMultichainOrigin: false, - }, - }, - ], - }, - }, - }, - 'malformed.com': 'invalid-subject', - 'valid.com': { - permissions: { - 'endowment:caip25': { - parentCapability: 'endowment:caip25', - caveats: [ - { - type: 'authorizedScopes', - value: { - requiredScopes: {}, - optionalScopes: { - 'wallet:eip155': { - accounts: ['wallet:eip155:0x123'], - }, - 'eip155:1': { - accounts: ['eip155:1:0x123'], - }, - }, - isMultichainOrigin: false, - }, - }, - ], - }, - }, - }, - }, - }, - }); - }); }); diff --git a/app/scripts/migrations/137.ts b/app/scripts/migrations/137.ts index 3b7f806e9b2c..e25ce81d03ed 100644 --- a/app/scripts/migrations/137.ts +++ b/app/scripts/migrations/137.ts @@ -52,6 +52,24 @@ const BUILT_IN_NETWORKS: ReadonlyMap = new Map([ const snapsPrefixes = ['npm:', 'local:'] as const; +function isPermissionConstraint(obj: unknown): obj is PermissionConstraint { + return ( + isObject(obj) && + obj !== null && + hasProperty(obj, 'caveats') && + Array.isArray(obj.caveats) && + obj.caveats.length > 0 && + hasProperty(obj, 'date') && + typeof obj.date === 'number' && + hasProperty(obj, 'id') && + typeof obj.id === 'string' && + hasProperty(obj, 'invoker') && + typeof obj.invoker === 'string' && + hasProperty(obj, 'parentCapability') && + typeof obj.parentCapability === 'string' + ); +} + /** * This migration transforms `eth_accounts` and `permittedChains` permissions into * an equivalent CAIP-25 permission. @@ -69,89 +87,116 @@ export async function migrate( ): Promise { const versionedData = cloneDeep(originalVersionedData); versionedData.meta.version = version; - transformState(versionedData.data); + + const newState = transformState(versionedData.data); + versionedData.data = newState as Record; return versionedData; } -function transformState(state: Record) { +function transformState(oldState: Record) { + const newState = cloneDeep(oldState); if ( - !hasProperty(state, 'PermissionController') || - !isObject(state.PermissionController) + !hasProperty(newState, 'PermissionController') || + !isObject(newState.PermissionController) ) { global.sentry?.captureException?.( new Error( - `Migration ${version}: typeof state.PermissionController is ${typeof state.PermissionController}`, + `Migration ${version}: typeof state.PermissionController is ${typeof newState.PermissionController}`, ), ); - return state; + return oldState; } if ( - !hasProperty(state, 'NetworkController') || - !isObject(state.NetworkController) + !hasProperty(newState, 'NetworkController') || + !isObject(newState.NetworkController) ) { global.sentry?.captureException?.( new Error( - `Migration ${version}: typeof state.NetworkController is ${typeof state.NetworkController}`, + `Migration ${version}: typeof state.NetworkController is ${typeof newState.NetworkController}`, ), ); - return state; + return newState; } - if ( - !hasProperty(state, 'SelectedNetworkController') || - !isObject(state.SelectedNetworkController) - ) { + if (!hasProperty(newState, 'SelectedNetworkController')) { + console.warn( + `Migration ${version}: typeof state.SelectedNetworkController is ${typeof newState.SelectedNetworkController}`, + ); + newState.SelectedNetworkController = { + domains: {}, + }; + } + + if (!isObject(newState.SelectedNetworkController)) { global.sentry?.captureException?.( new Error( - `Migration ${version}: typeof state.SelectedNetworkController is ${typeof state.SelectedNetworkController}`, + `Migration ${version}: typeof state.SelectedNetworkController is ${typeof newState.SelectedNetworkController}`, ), ); - return state; + return oldState; } const { - PermissionController: { subjects }, NetworkController: { selectedNetworkClientId, networkConfigurationsByChainId, }, - SelectedNetworkController: { domains }, - } = state; + PermissionController: { subjects }, + } = newState; if (!isObject(subjects)) { - global.sentry?.captureException( + global.sentry?.captureException?.( new Error( `Migration ${version}: typeof state.PermissionController.subjects is ${typeof subjects}`, ), ); - return state; + return oldState; } + if (!selectedNetworkClientId || typeof selectedNetworkClientId !== 'string') { - global.sentry?.captureException( + global.sentry?.captureException?.( new Error( `Migration ${version}: typeof state.NetworkController.selectedNetworkClientId is ${typeof selectedNetworkClientId}`, ), ); - return state; + return oldState; } + if (!isObject(networkConfigurationsByChainId)) { - global.sentry?.captureException( + global.sentry?.captureException?.( new Error( - `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId is ${typeof networkConfigurationsByChainId}`, + `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId is ${typeof newState + .NetworkController.networkConfigurationsByChainId}`, ), ); - return state; + return oldState; } - if (!isObject(domains)) { - global.sentry?.captureException( + + if ( + !hasProperty(newState.SelectedNetworkController, 'domains') || + !isObject(newState.SelectedNetworkController.domains) + ) { + const { domains } = newState.SelectedNetworkController; + global.sentry?.captureException?.( new Error( `Migration ${version}: typeof state.SelectedNetworkController.domains is ${typeof domains}`, ), ); - return state; + return oldState; } + if (!isObject(newState.PermissionController)) { + global.sentry?.captureException?.( + new Error( + `Migration ${version}: typeof state.PermissionController is ${typeof newState.PermissionController}`, + ), + ); + return oldState; + } + + const { domains } = newState.SelectedNetworkController; + const getChainIdForNetworkClientId = ( networkClientId: string, propertyName: string, @@ -165,7 +210,7 @@ function transformState(state: Record) { `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId["${chainId}"] is ${typeof networkConfiguration}`, ), ); - return undefined; + continue; } if (!Array.isArray(networkConfiguration.rpcEndpoints)) { global.sentry?.captureException( @@ -173,7 +218,7 @@ function transformState(state: Record) { `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId["${chainId}"].rpcEndpoints is ${typeof networkConfiguration.rpcEndpoints}`, ), ); - return undefined; + continue; } for (const rpcEndpoint of networkConfiguration.rpcEndpoints) { if (!isObject(rpcEndpoint)) { @@ -182,7 +227,7 @@ function transformState(state: Record) { `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId["${chainId}"].rpcEndpoints[] is ${typeof rpcEndpoint}`, ), ); - return undefined; + continue; } if (rpcEndpoint.networkClientId === networkClientId) { return chainId; @@ -206,54 +251,57 @@ function transformState(state: Record) { 'selectedNetworkClientId', ); if (!currentChainId) { - return state; + return oldState; } + // perform mutations on the cloned state for (const [origin, subject] of Object.entries(subjects)) { if (!isObject(subject)) { - global.sentry?.captureException( + global.sentry?.captureException?.( new Error( `Migration ${version}: Invalid subject for origin "${origin}" of type ${typeof subject}`, ), ); - continue; + return oldState; } - const { permissions } = subject as { - permissions: Record; - }; - if (!isObject(permissions)) { - global.sentry?.captureException( + if ( + !hasProperty(subject, 'permissions') || + !isObject(subject.permissions) + ) { + global.sentry?.captureException?.( new Error( - `Migration ${version}: Invalid permissions for origin "${origin}" of type ${typeof permissions}`, + `Migration ${version}: Invalid permissions for origin "${origin}" of type ${typeof subject.permissions}`, ), ); - continue; + return oldState; } + const { permissions } = subject; + let basePermission: PermissionConstraint | undefined; let ethAccounts: string[] = []; - if ( - isObject(permissions[PermissionNames.eth_accounts]) && - Array.isArray(permissions[PermissionNames.eth_accounts].caveats) - ) { + const ethAccountsPermission = permissions[PermissionNames.eth_accounts]; + if (isPermissionConstraint(ethAccountsPermission)) { ethAccounts = - (permissions[PermissionNames.eth_accounts].caveats?.[0] - ?.value as string[]) ?? []; - basePermission = permissions[PermissionNames.eth_accounts]; + (ethAccountsPermission.caveats?.[0]?.value as string[] | undefined) ?? + []; + basePermission = ethAccountsPermission; } + delete permissions[PermissionNames.eth_accounts]; let chainIds: string[] = []; - if ( - isObject(permissions[PermissionNames.permittedChains]) && - Array.isArray(permissions[PermissionNames.permittedChains].caveats) - ) { + const permittedChainsPermission = + permissions[PermissionNames.permittedChains]; + if (isPermissionConstraint(permittedChainsPermission)) { chainIds = - (permissions[PermissionNames.permittedChains].caveats?.[0] - ?.value as string[]) ?? []; - basePermission ??= permissions[PermissionNames.permittedChains]; + (permittedChainsPermission.caveats?.[0]?.value as + | string[] + | undefined) ?? []; + + basePermission ??= permittedChainsPermission; } delete permissions[PermissionNames.permittedChains]; @@ -311,8 +359,9 @@ function transformState(state: Record) { }, ], }; + permissions[Caip25EndowmentPermissionName] = caip25Permission; } - return state; + return newState; }