From 7da9a7ce0dbf0ee459f1e8c1a26717e1f499bc34 Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Thu, 31 Jul 2025 15:22:56 +0200 Subject: [PATCH 01/14] refactor: migrate common functions from extension/mobile (wip) --- .../src/caip25Permission.ts | 192 ++++++++++++++++-- .../src/constants.ts | 13 ++ 2 files changed, 193 insertions(+), 12 deletions(-) create mode 100644 packages/chain-agnostic-permission/src/constants.ts diff --git a/packages/chain-agnostic-permission/src/caip25Permission.ts b/packages/chain-agnostic-permission/src/caip25Permission.ts index cc4b923a9b0..c8ae400b531 100644 --- a/packages/chain-agnostic-permission/src/caip25Permission.ts +++ b/packages/chain-agnostic-permission/src/caip25Permission.ts @@ -6,6 +6,7 @@ import type { PermissionValidatorConstraint, PermissionConstraint, EndowmentCaveatSpecificationConstraint, + PermissionController, } from '@metamask/permission-controller'; import { CaveatMutatorOperation, @@ -20,10 +21,17 @@ import { type Hex, type NonEmptyArray, } from '@metamask/utils'; -import { cloneDeep, isEqual } from 'lodash'; +import { cloneDeep, isEqual, pick } from 'lodash'; -import { setNonSCACaipAccountIdsInCaip25CaveatValue } from './operators/caip-permission-operator-accounts'; -import { setChainIdsInCaip25CaveatValue } from './operators/caip-permission-operator-permittedChains'; +import { CaveatTypes, PermissionKeys } from './constants'; +import { + setEthAccounts, + setNonSCACaipAccountIdsInCaip25CaveatValue, +} from './operators/caip-permission-operator-accounts'; +import { + setChainIdsInCaip25CaveatValue, + setPermittedEthChainIds, +} from './operators/caip-permission-operator-permittedChains'; import { assertIsInternalScopesObject } from './scope/assert'; import { isSupportedAccount, @@ -541,21 +549,181 @@ export const generateCaip25Caveat = ( export function getCaip25CaveatFromPermission(caip25Permission?: { caveats: ( | { - type: string; - value: unknown; - } + type: string; + value: unknown; + } | { - type: typeof Caip25CaveatType; - value: Caip25CaveatValue; - } + type: typeof Caip25CaveatType; + value: Caip25CaveatValue; + } )[]; }) { return caip25Permission?.caveats.find( (caveat) => caveat.type === (Caip25CaveatType as string), ) as | { - type: typeof Caip25CaveatType; - value: Caip25CaveatValue; - } + type: typeof Caip25CaveatType; + value: Caip25CaveatValue; + } | undefined; } + +/** + * Requests user approval for the CAIP-25 permission for the specified origin + * and returns a granted permissions object. + * + * @param requestedPermissions - The legacy permissions to request approval for. + * @param requestedPermissions.caveats - //FIXME: update this + * @returns the approved permissions object. + */ +export const getCaip25PermissionFromLegacyPermissions = + (requestedPermissions?: { + [PermissionKeys.eth_accounts]?: { + caveats?: { + type: keyof typeof CaveatTypes; + value: Hex[]; + }[]; + }; + [PermissionKeys.permittedChains]?: { + caveats?: { + type: keyof typeof CaveatTypes; + value: Hex[]; + }[]; + }; + }) => { + const permissions = pick(requestedPermissions, [ + PermissionKeys.eth_accounts, + PermissionKeys.permittedChains, + ]); + + if (!permissions[PermissionKeys.eth_accounts]) { + permissions[PermissionKeys.eth_accounts] = {}; + } + + if (!permissions[PermissionKeys.permittedChains]) { + permissions[PermissionKeys.permittedChains] = {}; + } + + const requestedAccounts = + permissions[PermissionKeys.eth_accounts]?.caveats?.find( + (caveat) => caveat.type === CaveatTypes.restrictReturnedAccounts, + )?.value ?? []; + + const requestedChains = + permissions[PermissionKeys.permittedChains]?.caveats?.find( + (caveat) => caveat.type === CaveatTypes.restrictNetworkSwitching, + )?.value ?? []; + + const newCaveatValue = { + requiredScopes: {}, + optionalScopes: { + 'wallet:eip155': { + accounts: [], + }, + }, + sessionProperties: {}, + isMultichainOrigin: false, + }; + + const caveatValueWithChains = setPermittedEthChainIds( + newCaveatValue, + requestedChains, + ); + + const caveatValueWithAccountsAndChains = setEthAccounts( + caveatValueWithChains, + requestedAccounts, + ); + + return { + [Caip25EndowmentPermissionName]: { + caveats: [ + { + type: Caip25CaveatType, + value: caveatValueWithAccountsAndChains, + }, + ], + }, + }; + }; + +/** + * Requests incremental permittedChains permission for the specified origin. + * and updates the existing CAIP-25 permission. + * Allows for granting without prompting for user approval which + * would be used as part of flows like `wallet_addEthereumChain` + * requests where the addition of the network and the permitting + * of the chain are combined into one approval. + * + * @param options - The options object + * @param options.origin - The origin to request approval for. + * @param options.chainId - The chainId to add to the existing permittedChains. + * @param options.autoApprove - If the chain should be granted without prompting for user approval. + * @param options.metadata - Request data for the approval. + * @param options.metadata.options - // FIXME: document + * @param options.hooks - // FIXME: document + * @param options.hooks.requestPermissionsIncremental - // FIXME: document + * @param options.hooks.grantPermissionsIncremental - // FIXME: document + */ +export const requestPermittedChainsPermissionIncremental = async ({ + origin, + chainId, + autoApprove, + hooks, + metadata, +}: { + origin: string; + chainId: Hex; + autoApprove: boolean; + hooks: { + requestPermissionsIncremental: () => Promise; // FIXME: type properly + grantPermissionsIncremental: () => void; // FIXME: type properly + }; + metadata?: { options: Record }; +}) => { + const caveatValueWithChains = setPermittedEthChainIds( + { + requiredScopes: {}, + optionalScopes: {}, + sessionProperties: {}, + isMultichainOrigin: false, + }, + [chainId], + ); + + if (!autoApprove) { + let options; + if (metadata) { + options = { metadata }; + } + await hooks.requestPermissionsIncremental( + { origin }, + { + [Caip25EndowmentPermissionName]: { + caveats: [ + { + type: Caip25CaveatType, + value: caveatValueWithChains, + }, + ], + }, + }, + options, + ); + return; + } + + await hooks.grantPermissionsIncremental({ + subject: { origin }, + approvedPermissions: { + [Caip25EndowmentPermissionName]: { + caveats: [ + { + type: Caip25CaveatType, + value: caveatValueWithChains, + }, + ], + }, + }, + }); +}; diff --git a/packages/chain-agnostic-permission/src/constants.ts b/packages/chain-agnostic-permission/src/constants.ts new file mode 100644 index 00000000000..382db59186f --- /dev/null +++ b/packages/chain-agnostic-permission/src/constants.ts @@ -0,0 +1,13 @@ +export const CaveatTypes = Object.freeze({ + restrictReturnedAccounts: 'restrictReturnedAccounts', + restrictNetworkSwitching: 'restrictNetworkSwitching', +}); + +/** + * The "keys" of permissions recognized by the PermissionController. + * Permission keys and names have distinct meanings in the permission system. + */ +export const PermissionKeys = Object.freeze({ + eth_accounts: 'eth_accounts', + permittedChains: 'endowment:permitted-chains', +}); From e5100e54500bd2f93e678b7d51775c1f9b588fd7 Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Fri, 1 Aug 2025 09:25:13 +0200 Subject: [PATCH 02/14] docs: update documentation and hook signature --- .../src/caip25Permission.ts | 60 +++++++++++++------ 1 file changed, 42 insertions(+), 18 deletions(-) diff --git a/packages/chain-agnostic-permission/src/caip25Permission.ts b/packages/chain-agnostic-permission/src/caip25Permission.ts index c8ae400b531..07becc7dd9c 100644 --- a/packages/chain-agnostic-permission/src/caip25Permission.ts +++ b/packages/chain-agnostic-permission/src/caip25Permission.ts @@ -549,22 +549,22 @@ export const generateCaip25Caveat = ( export function getCaip25CaveatFromPermission(caip25Permission?: { caveats: ( | { - type: string; - value: unknown; - } + type: string; + value: unknown; + } | { - type: typeof Caip25CaveatType; - value: Caip25CaveatValue; - } + type: typeof Caip25CaveatType; + value: Caip25CaveatValue; + } )[]; }) { return caip25Permission?.caveats.find( (caveat) => caveat.type === (Caip25CaveatType as string), ) as | { - type: typeof Caip25CaveatType; - value: Caip25CaveatValue; - } + type: typeof Caip25CaveatType; + value: Caip25CaveatValue; + } | undefined; } @@ -573,8 +573,10 @@ export function getCaip25CaveatFromPermission(caip25Permission?: { * and returns a granted permissions object. * * @param requestedPermissions - The legacy permissions to request approval for. - * @param requestedPermissions.caveats - //FIXME: update this - * @returns the approved permissions object. + * @param requestedPermissions.caveats - The legacy caveats processed by the function. + * - `restrictReturnedAccounts`: Restricts which Ethereum accounts can be accessed + * - `restrictNetworkSwitching`: Restricts which blockchain networks can be used + * @returns CAIP-25 permission object with unified caveat structure containing both account and chain restrictions */ export const getCaip25PermissionFromLegacyPermissions = (requestedPermissions?: { @@ -660,10 +662,12 @@ export const getCaip25PermissionFromLegacyPermissions = * @param options.chainId - The chainId to add to the existing permittedChains. * @param options.autoApprove - If the chain should be granted without prompting for user approval. * @param options.metadata - Request data for the approval. - * @param options.metadata.options - // FIXME: document - * @param options.hooks - // FIXME: document - * @param options.hooks.requestPermissionsIncremental - // FIXME: document - * @param options.hooks.grantPermissionsIncremental - // FIXME: document + * @param options.metadata.options - Additional metadata about the permission request. + * @param options.hooks - Permission controller hooks for incremental operations. + * @param options.hooks.requestPermissionsIncremental - Initiates an incremental permission request that prompts for user approval. + * Incremental permission requests allow the caller to replace existing and/or add brand new permissions and caveats for the specified subject. + * @param options.hooks.grantPermissionsIncremental - Incrementally grants approved permissions to the specified subject without prompting for user approval. + * Every permission and caveat is stringently validated and an error is thrown if validation fails. */ export const requestPermittedChainsPermissionIncremental = async ({ origin, @@ -676,8 +680,28 @@ export const requestPermittedChainsPermissionIncremental = async ({ chainId: Hex; autoApprove: boolean; hooks: { - requestPermissionsIncremental: () => Promise; // FIXME: type properly - grantPermissionsIncremental: () => void; // FIXME: type properly + requestPermissionsIncremental: ( + subject: { origin: string }, + requestedPermissions: Record< + string, + { caveats: { type: string; value: unknown }[] } + >, + options?: { metadata?: Record }, + ) => Promise< + | [ + Partial>, + { data?: Record; id: string; origin: string }, + ] + | [] + >; + grantPermissionsIncremental: (params: { + subject: { origin: string }; + approvedPermissions: Record< + string, + { caveats: { type: string; value: unknown }[] } + >; + requestData?: Record; + }) => Partial>; }; metadata?: { options: Record }; }) => { @@ -713,7 +737,7 @@ export const requestPermittedChainsPermissionIncremental = async ({ return; } - await hooks.grantPermissionsIncremental({ + hooks.grantPermissionsIncremental({ subject: { origin }, approvedPermissions: { [Caip25EndowmentPermissionName]: { From 305486219037c4581dd57ca5d48d616c4ea8a926 Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Fri, 1 Aug 2025 09:55:38 +0200 Subject: [PATCH 03/14] test: migrate tests (wip) --- .../src/caip25Permission.test.ts | 517 ++++++++++++++++++ .../src/caip25Permission.ts | 118 ++-- 2 files changed, 584 insertions(+), 51 deletions(-) diff --git a/packages/chain-agnostic-permission/src/caip25Permission.test.ts b/packages/chain-agnostic-permission/src/caip25Permission.test.ts index 9823811eb15..761d6c26b75 100644 --- a/packages/chain-agnostic-permission/src/caip25Permission.test.ts +++ b/packages/chain-agnostic-permission/src/caip25Permission.test.ts @@ -14,9 +14,12 @@ import { diffScopesForCaip25CaveatValue, generateCaip25Caveat, getCaip25CaveatFromPermission, + getCaip25PermissionFromLegacyPermissions, + requestPermittedChainsPermissionIncremental, } from './caip25Permission'; import { KnownSessionProperties } from './scope/constants'; import * as ScopeSupported from './scope/supported'; +import { CaveatTypes, PermissionKeys } from './constants'; jest.mock('./scope/supported', () => ({ ...jest.requireActual('./scope/supported'), @@ -1759,3 +1762,517 @@ describe('generateCaip25Caveat', () => { }); }); }); + +describe('requestPermittedChainsPermissionIncremental', () => { + it('requests permittedChains approval if autoApprove: false', async () => { + const subjectPermissions: Partial< + SubjectPermissions< + ExtractPermission< + PermissionSpecificationConstraint, + CaveatSpecificationConstraint + > + > + > = { + [Caip25EndowmentPermissionName]: { + id: 'id', + date: 1, + invoker: 'origin', + parentCapability: PermissionKeys.permittedChains, + caveats: [ + { + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { 'eip155:1': { accounts: [] } }, + isMultichainOrigin: false, + sessionProperties: {}, + }, + }, + ], + }, + }; + + const expectedCaip25Permission = { + [Caip25EndowmentPermissionName]: pick( + subjectPermissions[Caip25EndowmentPermissionName], + 'caveats', + ), + }; + + mockRequestPermissionsIncremental.mockResolvedValue([ + subjectPermissions, + { id: 'id', origin: 'origin' }, + ]); + + await requestPermittedChainsPermissionIncremental({ + origin: 'test.com', + chainId: '0x1', + autoApprove: false, + }); + + expect(mockRequestPermissionsIncremental).toHaveBeenCalledWith( + { origin: 'test.com' }, + expectedCaip25Permission, + ); + }); + + it('throws if permittedChains approval is rejected', async () => { + mockRequestPermissionsIncremental.mockRejectedValue( + new Error('approval rejected'), + ); + + await expect(() => + requestPermittedChainsPermissionIncremental({ + origin: 'test.com', + chainId: '0x1', + autoApprove: false, + }), + ).rejects.toThrow(new Error('approval rejected')); + }); + + it('grants permittedChains approval if autoApprove: true', async () => { + const subjectPermissions: Partial< + SubjectPermissions< + ExtractPermission< + PermissionSpecificationConstraint, + CaveatSpecificationConstraint + > + > + > = { + [Caip25EndowmentPermissionName]: { + id: 'id', + date: 1, + invoker: 'origin', + parentCapability: PermissionKeys.permittedChains, + caveats: [ + { + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { 'eip155:1': { accounts: [] } }, + isMultichainOrigin: false, + sessionProperties: {}, + }, + }, + ], + }, + }; + + const expectedCaip25Permission = { + [Caip25EndowmentPermissionName]: pick( + subjectPermissions[Caip25EndowmentPermissionName], + 'caveats', + ), + }; + + mockGrantPermissionsIncremental.mockReturnValue(subjectPermissions); + + await requestPermittedChainsPermissionIncremental({ + origin: 'test.com', + chainId: '0x1', + autoApprove: true, + }); + + expect(mockGrantPermissionsIncremental).toHaveBeenCalledWith({ + subject: { origin: 'test.com' }, + approvedPermissions: expectedCaip25Permission, + }); + }); + + it('throws if autoApprove: true and granting permittedChains throws', async () => { + mockGrantPermissionsIncremental.mockImplementation(() => { + throw new Error('Invalid merged permissions for subject "test.com"'); + }); + + await expect(() => + requestPermittedChainsPermissionIncremental({ + origin: 'test.com', + chainId: '0x1', + autoApprove: true, + }), + ).rejects.toThrow( + new Error('Invalid merged permissions for subject "test.com"'), + ); + }); +}); + +describe('getCaip25PermissionFromLegacyPermissions', () => { + it('returns valid CAIP-25 permissions', async () => { + const permissions = getCaip25PermissionFromLegacyPermissions( + 'test.com', + {}, + ); + + expect(permissions).toStrictEqual( + expect.objectContaining({ + [Caip25EndowmentPermissionName]: { + caveats: [ + { + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { + 'wallet:eip155': { + accounts: [], + }, + }, + isMultichainOrigin: false, + sessionProperties: {}, + }, + }, + ], + }, + }), + ); + }); + + it('returns approval from the PermissionsController for eth_accounts and permittedChains when only eth_accounts is specified in params and origin is not snapId', async () => { + const permissions = getCaip25PermissionFromLegacyPermissions('test.com', { + [PermissionKeys.eth_accounts]: { + caveats: [ + { + type: CaveatTypes.restrictReturnedAccounts, + value: ['0x0000000000000000000000000000000000000001'], + }, + ], + }, + }); + + expect(permissions).toStrictEqual( + expect.objectContaining({ + [Caip25EndowmentPermissionName]: { + caveats: [ + { + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { + 'wallet:eip155': { + accounts: [ + 'wallet:eip155:0x0000000000000000000000000000000000000001', + ], + }, + }, + isMultichainOrigin: false, + sessionProperties: {}, + }, + }, + ], + }, + }), + ); + }); + + it('returns approval from the PermissionsControllerr for eth_accounts and permittedChains when only permittedChains is specified in params and origin is not snapId', async () => { + const permissions = getCaip25PermissionFromLegacyPermissions('test.com', { + [PermissionKeys.permittedChains]: { + caveats: [ + { + type: CaveatTypes.restrictNetworkSwitching, + value: ['0x64'], + }, + ], + }, + }); + + expect(permissions).toStrictEqual( + expect.objectContaining({ + [Caip25EndowmentPermissionName]: { + caveats: [ + { + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { + 'wallet:eip155': { + accounts: [], + }, + 'eip155:100': { + accounts: [], + }, + }, + isMultichainOrigin: false, + sessionProperties: {}, + }, + }, + ], + }, + }), + ); + }); + + it('returns approval from the PermissionsController for eth_accounts and permittedChains when both are specified in params and origin is not snapId', async () => { + const permissions = getCaip25PermissionFromLegacyPermissions('test.com', { + [PermissionKeys.eth_accounts]: { + caveats: [ + { + type: CaveatTypes.restrictReturnedAccounts, + value: ['0x0000000000000000000000000000000000000001'], + }, + ], + }, + [PermissionKeys.permittedChains]: { + caveats: [ + { + type: CaveatTypes.restrictNetworkSwitching, + value: ['0x64'], + }, + ], + }, + }); + + expect(permissions).toStrictEqual( + expect.objectContaining({ + [Caip25EndowmentPermissionName]: { + caveats: [ + { + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { + 'wallet:eip155': { + accounts: [ + 'wallet:eip155:0x0000000000000000000000000000000000000001', + ], + }, + 'eip155:100': { + accounts: [ + 'eip155:100:0x0000000000000000000000000000000000000001', + ], + }, + }, + isMultichainOrigin: false, + sessionProperties: {}, + }, + }, + ], + }, + }), + ); + }); + + it('returns approval from the PermissionsController for only eth_accounts when only eth_accounts is specified in params and origin is snapId', async () => { + const permissions = getCaip25PermissionFromLegacyPermissions('npm:snap', { + [PermissionKeys.eth_accounts]: { + caveats: [ + { + type: CaveatTypes.restrictReturnedAccounts, + value: ['0x0000000000000000000000000000000000000001'], + }, + ], + }, + }); + + expect(permissions).toStrictEqual( + expect.objectContaining({ + [Caip25EndowmentPermissionName]: { + caveats: [ + { + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { + 'wallet:eip155': { + accounts: [ + 'wallet:eip155:0x0000000000000000000000000000000000000001', + ], + }, + }, + isMultichainOrigin: false, + sessionProperties: {}, + }, + }, + ], + }, + }), + ); + }); + + it('returns approval from the PermissionsController for only eth_accounts when only permittedChains is specified in params and origin is snapId', async () => { + const permissions = getCaip25PermissionFromLegacyPermissions('npm:snap', { + [PermissionKeys.permittedChains]: { + caveats: [ + { + type: CaveatTypes.restrictNetworkSwitching, + value: ['0x64'], + }, + ], + }, + }); + + expect(permissions).toStrictEqual( + expect.objectContaining({ + [Caip25EndowmentPermissionName]: { + caveats: [ + { + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { + 'eip155:100': { + accounts: [], + }, + 'wallet:eip155': { + accounts: [], + }, + }, + isMultichainOrigin: false, + sessionProperties: {}, + }, + }, + ], + }, + }), + ); + }); + + it('returns approval from the PermissionsController for eth_accounts and permittedChains when both eth_accounts and permittedChains are specified in params and origin is snapId', async () => { + const permissions = getCaip25PermissionFromLegacyPermissions('npm:snap', { + [PermissionKeys.eth_accounts]: { + caveats: [ + { + type: CaveatTypes.restrictReturnedAccounts, + value: ['0x0000000000000000000000000000000000000001'], + }, + ], + }, + [PermissionKeys.permittedChains]: { + caveats: [ + { + type: CaveatTypes.restrictNetworkSwitching, + value: ['0x64'], + }, + ], + }, + }); + + expect(permissions).toStrictEqual( + expect.objectContaining({ + [Caip25EndowmentPermissionName]: { + caveats: [ + { + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { + 'eip155:100': { + accounts: [ + 'eip155:100:0x0000000000000000000000000000000000000001', + ], + }, + 'wallet:eip155': { + accounts: [ + 'wallet:eip155:0x0000000000000000000000000000000000000001', + ], + }, + }, + isMultichainOrigin: false, + sessionProperties: {}, + }, + }, + ], + }, + }), + ); + }); + + it('returns CAIP-25 approval with accounts and chainIds specified from `eth_accounts` and `endowment:permittedChains` permissions caveats, and isMultichainOrigin: false if origin is not snapId', async () => { + const permissions = getCaip25PermissionFromLegacyPermissions('test.com', { + [PermissionKeys.eth_accounts]: { + caveats: [ + { + type: 'restrictReturnedAccounts', + value: ['0xdeadbeef'], + }, + ], + }, + [PermissionKeys.permittedChains]: { + caveats: [ + { + type: 'restrictNetworkSwitching', + value: ['0x1', '0x5'], + }, + ], + }, + }); + + expect(permissions).toStrictEqual( + expect.objectContaining({ + [Caip25EndowmentPermissionName]: { + caveats: [ + { + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { + 'wallet:eip155': { + accounts: ['wallet:eip155:0xdeadbeef'], + }, + 'eip155:1': { + accounts: ['eip155:1:0xdeadbeef'], + }, + 'eip155:5': { + accounts: ['eip155:5:0xdeadbeef'], + }, + }, + isMultichainOrigin: false, + sessionProperties: {}, + }, + }, + ], + }, + }), + ); + }); + + it('returns CAIP-25 approval with approved accounts for the `wallet:eip155` scope with isMultichainOrigin: false if origin is snapId', async () => { + const origin = 'npm:snap'; + + const permissions = getCaip25PermissionFromLegacyPermissions(origin, { + [PermissionKeys.eth_accounts]: { + caveats: [ + { + type: 'restrictReturnedAccounts', + value: ['0xdeadbeef'], + }, + ], + }, + [PermissionKeys.permittedChains]: { + caveats: [ + { + type: 'restrictNetworkSwitching', + value: ['0x1', '0x5'], + }, + ], + }, + }); + + expect(permissions).toStrictEqual( + expect.objectContaining({ + [Caip25EndowmentPermissionName]: { + caveats: [ + { + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { + 'eip155:1': { + accounts: ['eip155:1:0xdeadbeef'], + }, + 'eip155:5': { + accounts: ['eip155:5:0xdeadbeef'], + }, + 'wallet:eip155': { + accounts: ['wallet:eip155:0xdeadbeef'], + }, + }, + isMultichainOrigin: false, + sessionProperties: {}, + }, + }, + ], + }, + }), + ); + }); +}); diff --git a/packages/chain-agnostic-permission/src/caip25Permission.ts b/packages/chain-agnostic-permission/src/caip25Permission.ts index 07becc7dd9c..e34d690dd18 100644 --- a/packages/chain-agnostic-permission/src/caip25Permission.ts +++ b/packages/chain-agnostic-permission/src/caip25Permission.ts @@ -572,14 +572,16 @@ export function getCaip25CaveatFromPermission(caip25Permission?: { * Requests user approval for the CAIP-25 permission for the specified origin * and returns a granted permissions object. * + * @param origin - The origin to request approval for. * @param requestedPermissions - The legacy permissions to request approval for. * @param requestedPermissions.caveats - The legacy caveats processed by the function. * - `restrictReturnedAccounts`: Restricts which Ethereum accounts can be accessed * - `restrictNetworkSwitching`: Restricts which blockchain networks can be used * @returns CAIP-25 permission object with unified caveat structure containing both account and chain restrictions */ -export const getCaip25PermissionFromLegacyPermissions = - (requestedPermissions?: { +export const getCaip25PermissionFromLegacyPermissions = ( + origin: string, + requestedPermissions?: { [PermissionKeys.eth_accounts]?: { caveats?: { type: keyof typeof CaveatTypes; @@ -592,62 +594,70 @@ export const getCaip25PermissionFromLegacyPermissions = value: Hex[]; }[]; }; - }) => { - const permissions = pick(requestedPermissions, [ - PermissionKeys.eth_accounts, - PermissionKeys.permittedChains, - ]); - - if (!permissions[PermissionKeys.eth_accounts]) { - permissions[PermissionKeys.eth_accounts] = {}; - } - - if (!permissions[PermissionKeys.permittedChains]) { - permissions[PermissionKeys.permittedChains] = {}; - } - - const requestedAccounts = - permissions[PermissionKeys.eth_accounts]?.caveats?.find( - (caveat) => caveat.type === CaveatTypes.restrictReturnedAccounts, - )?.value ?? []; + }, +) => { + const permissions = pick(requestedPermissions, [ + PermissionKeys.eth_accounts, + PermissionKeys.permittedChains, + ]); + + if (!permissions[PermissionKeys.eth_accounts]) { + permissions[PermissionKeys.eth_accounts] = {}; + } - const requestedChains = - permissions[PermissionKeys.permittedChains]?.caveats?.find( - (caveat) => caveat.type === CaveatTypes.restrictNetworkSwitching, - )?.value ?? []; + if (!permissions[PermissionKeys.permittedChains]) { + permissions[PermissionKeys.permittedChains] = {}; + } - const newCaveatValue = { - requiredScopes: {}, - optionalScopes: { - 'wallet:eip155': { - accounts: [], - }, + // TODO: [ffmcgee] get isSnapId from snap utils + // if (isSnapId(origin)) { + // delete permissions[PermissionNames.permittedChains]; + // } + + const requestedAccounts = + permissions[PermissionKeys.eth_accounts]?.caveats?.find( + (caveat) => caveat.type === CaveatTypes.restrictReturnedAccounts, + )?.value ?? []; + + const requestedChains = + permissions[PermissionKeys.permittedChains]?.caveats?.find( + (caveat) => caveat.type === CaveatTypes.restrictNetworkSwitching, + )?.value ?? []; + + const newCaveatValue = { + requiredScopes: {}, + optionalScopes: { + 'wallet:eip155': { + accounts: [], }, - sessionProperties: {}, - isMultichainOrigin: false, - }; + }, + sessionProperties: {}, + isMultichainOrigin: false, + }; - const caveatValueWithChains = setPermittedEthChainIds( - newCaveatValue, - requestedChains, - ); + const caveatValueWithChains = setPermittedEthChainIds( + newCaveatValue, + // TODO: [ffmcgee] get isSnapId from snap utils + // isSnapId(origin) ? [] : requestedChains, + requestedChains, + ); - const caveatValueWithAccountsAndChains = setEthAccounts( - caveatValueWithChains, - requestedAccounts, - ); + const caveatValueWithAccountsAndChains = setEthAccounts( + caveatValueWithChains, + requestedAccounts, + ); - return { - [Caip25EndowmentPermissionName]: { - caveats: [ - { - type: Caip25CaveatType, - value: caveatValueWithAccountsAndChains, - }, - ], - }, - }; + return { + [Caip25EndowmentPermissionName]: { + caveats: [ + { + type: Caip25CaveatType, + value: caveatValueWithAccountsAndChains, + }, + ], + }, }; +}; /** * Requests incremental permittedChains permission for the specified origin. @@ -705,6 +715,12 @@ export const requestPermittedChainsPermissionIncremental = async ({ }; metadata?: { options: Record }; }) => { + // TODO: [ffmcgee] get isSnapId from snap utils + // if (isSnapId(origin)) { + // throw new Error( + // `Cannot request permittedChains permission for Snaps with origin "${origin}"`, + // ); + // } const caveatValueWithChains = setPermittedEthChainIds( { requiredScopes: {}, From 53f22780fe6cd22eceb46daeef35a429c086a6c9 Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Fri, 1 Aug 2025 10:40:31 +0200 Subject: [PATCH 04/14] test: finish test migration --- .../src/caip25Permission.test.ts | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/packages/chain-agnostic-permission/src/caip25Permission.test.ts b/packages/chain-agnostic-permission/src/caip25Permission.test.ts index 761d6c26b75..fb8df4001ed 100644 --- a/packages/chain-agnostic-permission/src/caip25Permission.test.ts +++ b/packages/chain-agnostic-permission/src/caip25Permission.test.ts @@ -1,7 +1,12 @@ import { CaveatMutatorOperation, PermissionType, + type SubjectPermissions, + type ExtractPermission, + type PermissionSpecificationConstraint, + type CaveatSpecificationConstraint, } from '@metamask/permission-controller'; +import { pick } from 'lodash'; import type { Caip25CaveatValue } from './caip25Permission'; import { @@ -17,9 +22,9 @@ import { getCaip25PermissionFromLegacyPermissions, requestPermittedChainsPermissionIncremental, } from './caip25Permission'; +import { CaveatTypes, PermissionKeys } from './constants'; import { KnownSessionProperties } from './scope/constants'; import * as ScopeSupported from './scope/supported'; -import { CaveatTypes, PermissionKeys } from './constants'; jest.mock('./scope/supported', () => ({ ...jest.requireActual('./scope/supported'), @@ -30,6 +35,9 @@ const MockScopeSupported = jest.mocked(ScopeSupported); const { removeAccount, removeScope } = Caip25CaveatMutators[Caip25CaveatType]; +const mockRequestPermissionsIncremental = jest.fn(); +const mockGrantPermissionsIncremental = jest.fn(); + describe('caip25EndowmentBuilder', () => { describe('specificationBuilder', () => { it('builds the expected permission specification', () => { @@ -1808,11 +1816,16 @@ describe('requestPermittedChainsPermissionIncremental', () => { origin: 'test.com', chainId: '0x1', autoApprove: false, + hooks: { + requestPermissionsIncremental: mockRequestPermissionsIncremental, + grantPermissionsIncremental: mockGrantPermissionsIncremental, + }, }); expect(mockRequestPermissionsIncremental).toHaveBeenCalledWith( { origin: 'test.com' }, expectedCaip25Permission, + undefined, // undefined metadata ); }); @@ -1826,6 +1839,10 @@ describe('requestPermittedChainsPermissionIncremental', () => { origin: 'test.com', chainId: '0x1', autoApprove: false, + hooks: { + requestPermissionsIncremental: mockRequestPermissionsIncremental, + grantPermissionsIncremental: mockGrantPermissionsIncremental, + }, }), ).rejects.toThrow(new Error('approval rejected')); }); @@ -1871,6 +1888,10 @@ describe('requestPermittedChainsPermissionIncremental', () => { origin: 'test.com', chainId: '0x1', autoApprove: true, + hooks: { + requestPermissionsIncremental: mockRequestPermissionsIncremental, + grantPermissionsIncremental: mockGrantPermissionsIncremental, + }, }); expect(mockGrantPermissionsIncremental).toHaveBeenCalledWith({ @@ -1889,6 +1910,10 @@ describe('requestPermittedChainsPermissionIncremental', () => { origin: 'test.com', chainId: '0x1', autoApprove: true, + hooks: { + requestPermissionsIncremental: mockRequestPermissionsIncremental, + grantPermissionsIncremental: mockGrantPermissionsIncremental, + }, }), ).rejects.toThrow( new Error('Invalid merged permissions for subject "test.com"'), From 7f8807a450567b2223bee962faac511832ecc306 Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Fri, 1 Aug 2025 10:41:23 +0200 Subject: [PATCH 05/14] typo --- packages/chain-agnostic-permission/src/caip25Permission.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/chain-agnostic-permission/src/caip25Permission.test.ts b/packages/chain-agnostic-permission/src/caip25Permission.test.ts index fb8df4001ed..9b0272f8805 100644 --- a/packages/chain-agnostic-permission/src/caip25Permission.test.ts +++ b/packages/chain-agnostic-permission/src/caip25Permission.test.ts @@ -1988,7 +1988,7 @@ describe('getCaip25PermissionFromLegacyPermissions', () => { ); }); - it('returns approval from the PermissionsControllerr for eth_accounts and permittedChains when only permittedChains is specified in params and origin is not snapId', async () => { + it('returns approval from the PermissionsController for eth_accounts and permittedChains when only permittedChains is specified in params and origin is not snapId', async () => { const permissions = getCaip25PermissionFromLegacyPermissions('test.com', { [PermissionKeys.permittedChains]: { caveats: [ From c300fdd7f7eb84e5f8242794d2a4807353384fef Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Fri, 1 Aug 2025 10:51:14 +0200 Subject: [PATCH 06/14] test: update existing index test, and add more cases for 100% cov --- .../src/caip25Permission.test.ts | 61 +++++++++++++++++++ .../src/caip25Permission.ts | 5 +- .../src/index.test.ts | 2 + .../chain-agnostic-permission/src/index.ts | 2 + 4 files changed, 68 insertions(+), 2 deletions(-) diff --git a/packages/chain-agnostic-permission/src/caip25Permission.test.ts b/packages/chain-agnostic-permission/src/caip25Permission.test.ts index 9b0272f8805..91fd053b50d 100644 --- a/packages/chain-agnostic-permission/src/caip25Permission.test.ts +++ b/packages/chain-agnostic-permission/src/caip25Permission.test.ts @@ -1919,6 +1919,67 @@ describe('requestPermittedChainsPermissionIncremental', () => { new Error('Invalid merged permissions for subject "test.com"'), ); }); + + it('passes metadata to requestPermissionsIncremental when metadata is provided', async () => { + const subjectPermissions: Partial< + SubjectPermissions< + ExtractPermission< + PermissionSpecificationConstraint, + CaveatSpecificationConstraint + > + > + > = { + [Caip25EndowmentPermissionName]: { + id: 'id', + date: 1, + invoker: 'origin', + parentCapability: PermissionKeys.permittedChains, + caveats: [ + { + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { 'eip155:1': { accounts: [] } }, + isMultichainOrigin: false, + sessionProperties: {}, + }, + }, + ], + }, + }; + + const expectedCaip25Permission = { + [Caip25EndowmentPermissionName]: pick( + subjectPermissions[Caip25EndowmentPermissionName], + 'caveats', + ), + }; + + const options = { someOption: 'testValue' }; + const metadata = { options }; + + mockRequestPermissionsIncremental.mockResolvedValue([ + subjectPermissions, + { id: 'id', origin: 'origin' }, + ]); + + await requestPermittedChainsPermissionIncremental({ + origin: 'test.com', + chainId: '0x1', + autoApprove: false, + metadata, + hooks: { + requestPermissionsIncremental: mockRequestPermissionsIncremental, + grantPermissionsIncremental: mockGrantPermissionsIncremental, + }, + }); + + expect(mockRequestPermissionsIncremental).toHaveBeenCalledWith( + { origin: 'test.com' }, + expectedCaip25Permission, + options, + ); + }); }); describe('getCaip25PermissionFromLegacyPermissions', () => { diff --git a/packages/chain-agnostic-permission/src/caip25Permission.ts b/packages/chain-agnostic-permission/src/caip25Permission.ts index e34d690dd18..ece19535b9a 100644 --- a/packages/chain-agnostic-permission/src/caip25Permission.ts +++ b/packages/chain-agnostic-permission/src/caip25Permission.ts @@ -572,7 +572,7 @@ export function getCaip25CaveatFromPermission(caip25Permission?: { * Requests user approval for the CAIP-25 permission for the specified origin * and returns a granted permissions object. * - * @param origin - The origin to request approval for. + * @param _origin - The origin to request approval for. * @param requestedPermissions - The legacy permissions to request approval for. * @param requestedPermissions.caveats - The legacy caveats processed by the function. * - `restrictReturnedAccounts`: Restricts which Ethereum accounts can be accessed @@ -580,7 +580,8 @@ export function getCaip25CaveatFromPermission(caip25Permission?: { * @returns CAIP-25 permission object with unified caveat structure containing both account and chain restrictions */ export const getCaip25PermissionFromLegacyPermissions = ( - origin: string, + // TODO: [ffmcgee] make sure snaps logic only triggered for extension ? + _origin: string, requestedPermissions?: { [PermissionKeys.eth_accounts]?: { caveats?: { diff --git a/packages/chain-agnostic-permission/src/index.test.ts b/packages/chain-agnostic-permission/src/index.test.ts index e6569b97d1d..5e4cf167c7d 100644 --- a/packages/chain-agnostic-permission/src/index.test.ts +++ b/packages/chain-agnostic-permission/src/index.test.ts @@ -49,6 +49,8 @@ describe('@metamask/chain-agnostic-permission', () => { "Caip25CaveatMutators", "generateCaip25Caveat", "getCaip25CaveatFromPermission", + "getCaip25PermissionFromLegacyPermissions", + "requestPermittedChainsPermissionIncremental", "KnownSessionProperties", "Caip25Errors", ] diff --git a/packages/chain-agnostic-permission/src/index.ts b/packages/chain-agnostic-permission/src/index.ts index 59abb97496e..1a9d7f2714a 100644 --- a/packages/chain-agnostic-permission/src/index.ts +++ b/packages/chain-agnostic-permission/src/index.ts @@ -71,6 +71,8 @@ export { Caip25CaveatMutators, generateCaip25Caveat, getCaip25CaveatFromPermission, + getCaip25PermissionFromLegacyPermissions, + requestPermittedChainsPermissionIncremental, } from './caip25Permission'; export { KnownSessionProperties } from './scope/constants'; export { Caip25Errors } from './scope/errors'; From f0d28930a135d1c08fc80272bdffcbf75888d2bd Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Fri, 1 Aug 2025 10:57:03 +0200 Subject: [PATCH 07/14] chore: update CHANGELOG.md --- packages/chain-agnostic-permission/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/chain-agnostic-permission/CHANGELOG.md b/packages/chain-agnostic-permission/CHANGELOG.md index 3003c146fc2..5a6eff2b675 100644 --- a/packages/chain-agnostic-permission/CHANGELOG.md +++ b/packages/chain-agnostic-permission/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Added `getCaip25PermissionFromLegacyPermissions` and `requestPermittedChainsPermissionIncremental` misc functions. ([#6225](https://github.com/MetaMask/core/pull/6225)) + ### Changed - Bump `@metamask/controller-utils` from `^11.10.0` to `^11.11.0` ([#6069](https://github.com/MetaMask/core/pull/6069)) From 1c1acbedc75fc56c1b92594062ade17f9799ba9b Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Fri, 1 Aug 2025 11:07:46 +0200 Subject: [PATCH 08/14] test: minor fix to failing metadata test --- .../chain-agnostic-permission/src/caip25Permission.test.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/chain-agnostic-permission/src/caip25Permission.test.ts b/packages/chain-agnostic-permission/src/caip25Permission.test.ts index 91fd053b50d..e9e8a417f7c 100644 --- a/packages/chain-agnostic-permission/src/caip25Permission.test.ts +++ b/packages/chain-agnostic-permission/src/caip25Permission.test.ts @@ -1955,8 +1955,7 @@ describe('requestPermittedChainsPermissionIncremental', () => { ), }; - const options = { someOption: 'testValue' }; - const metadata = { options }; + const metadata = { options: { someOption: 'testValue' } }; mockRequestPermissionsIncremental.mockResolvedValue([ subjectPermissions, @@ -1977,7 +1976,7 @@ describe('requestPermittedChainsPermissionIncremental', () => { expect(mockRequestPermissionsIncremental).toHaveBeenCalledWith( { origin: 'test.com' }, expectedCaip25Permission, - options, + { metadata }, ); }); }); From dd971429d1979fea663153be5061b194c28eef5a Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Fri, 1 Aug 2025 12:52:30 +0200 Subject: [PATCH 09/14] docs: isSnapId solution documentation --- packages/chain-agnostic-permission/src/caip25Permission.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/chain-agnostic-permission/src/caip25Permission.ts b/packages/chain-agnostic-permission/src/caip25Permission.ts index ece19535b9a..c51e50ad02b 100644 --- a/packages/chain-agnostic-permission/src/caip25Permission.ts +++ b/packages/chain-agnostic-permission/src/caip25Permission.ts @@ -580,7 +580,8 @@ export function getCaip25CaveatFromPermission(caip25Permission?: { * @returns CAIP-25 permission object with unified caveat structure containing both account and chain restrictions */ export const getCaip25PermissionFromLegacyPermissions = ( - // TODO: [ffmcgee] make sure snaps logic only triggered for extension ? + // TODO: [ffmcgee] make sure snaps logic only triggered for extension ? --> Maybe pass in the isSnapId as an optional hook, and if it's undefined, the logic won't trigger + // otherwise, it will trigger and actually validate if isSnapId(origin) if the logic should run or not _origin: string, requestedPermissions?: { [PermissionKeys.eth_accounts]?: { From 4680c10b320981b4531c6f2af81b2acfb5318aa2 Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Fri, 1 Aug 2025 14:13:13 +0200 Subject: [PATCH 10/14] fix: add conditional isSnapId logic for util functions + unit tests to match --- .../src/caip25Permission.test.ts | 96 +++++++++++++++++++ .../src/caip25Permission.ts | 40 ++++---- 2 files changed, 118 insertions(+), 18 deletions(-) diff --git a/packages/chain-agnostic-permission/src/caip25Permission.test.ts b/packages/chain-agnostic-permission/src/caip25Permission.test.ts index e9e8a417f7c..85c2eabcc60 100644 --- a/packages/chain-agnostic-permission/src/caip25Permission.test.ts +++ b/packages/chain-agnostic-permission/src/caip25Permission.test.ts @@ -1979,6 +1979,49 @@ describe('requestPermittedChainsPermissionIncremental', () => { { metadata }, ); }); + + it('throws error when isSnapId is defined and returns true', async () => { + const mockIsSnapId = jest.fn().mockReturnValue(true); + + await expect(() => + requestPermittedChainsPermissionIncremental({ + origin: 'npm:snap', + chainId: '0x1', + autoApprove: false, + hooks: { + requestPermissionsIncremental: mockRequestPermissionsIncremental, + grantPermissionsIncremental: mockGrantPermissionsIncremental, + isSnapId: mockIsSnapId, + }, + }), + ).rejects.toThrow( + 'Cannot request permittedChains permission for Snaps with origin "npm:snap"', + ); + + expect(mockIsSnapId).toHaveBeenCalledWith('npm:snap'); + + expect(mockRequestPermissionsIncremental).not.toHaveBeenCalled(); + }); + + it('does not throw error when isSnapId is undefined', async () => { + mockRequestPermissionsIncremental.mockResolvedValue([ + {}, + { id: 'id', origin: 'origin' }, + ]); + + await requestPermittedChainsPermissionIncremental({ + origin: 'npm:snap', + chainId: '0x1', + autoApprove: false, + hooks: { + requestPermissionsIncremental: mockRequestPermissionsIncremental, + grantPermissionsIncremental: mockGrantPermissionsIncremental, + // isSnapId is undefined + }, + }); + + expect(mockRequestPermissionsIncremental).toHaveBeenCalled(); + }); }); describe('getCaip25PermissionFromLegacyPermissions', () => { @@ -2360,4 +2403,57 @@ describe('getCaip25PermissionFromLegacyPermissions', () => { }), ); }); + + it('deletes permittedChains permission when isSnapId is defined and returns true', () => { + const mockIsSnapId = jest.fn().mockReturnValue(true); + + const permissions = getCaip25PermissionFromLegacyPermissions( + 'npm:snap', + { + [PermissionKeys.eth_accounts]: { + caveats: [ + { + type: CaveatTypes.restrictReturnedAccounts, + value: ['0x0000000000000000000000000000000000000001'], + }, + ], + }, + [PermissionKeys.permittedChains]: { + caveats: [ + { + type: CaveatTypes.restrictNetworkSwitching, + value: ['0x64'], + }, + ], + }, + }, + mockIsSnapId, + ); + + expect(mockIsSnapId).toHaveBeenCalledWith('npm:snap'); + + expect(permissions).toStrictEqual( + expect.objectContaining({ + [Caip25EndowmentPermissionName]: { + caveats: [ + { + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { + 'wallet:eip155': { + accounts: [ + 'wallet:eip155:0x0000000000000000000000000000000000000001', + ], + }, + }, + isMultichainOrigin: false, + sessionProperties: {}, + }, + }, + ], + }, + }), + ); + }); }); diff --git a/packages/chain-agnostic-permission/src/caip25Permission.ts b/packages/chain-agnostic-permission/src/caip25Permission.ts index c51e50ad02b..53c7f83f74c 100644 --- a/packages/chain-agnostic-permission/src/caip25Permission.ts +++ b/packages/chain-agnostic-permission/src/caip25Permission.ts @@ -572,17 +572,19 @@ export function getCaip25CaveatFromPermission(caip25Permission?: { * Requests user approval for the CAIP-25 permission for the specified origin * and returns a granted permissions object. * - * @param _origin - The origin to request approval for. + * @param origin - The origin to request approval for. * @param requestedPermissions - The legacy permissions to request approval for. * @param requestedPermissions.caveats - The legacy caveats processed by the function. * - `restrictReturnedAccounts`: Restricts which Ethereum accounts can be accessed * - `restrictNetworkSwitching`: Restricts which blockchain networks can be used - * @returns CAIP-25 permission object with unified caveat structure containing both account and chain restrictions + * @param isSnapId - Optional function to check if the origin is a valid snap ID. + * This function is a type guard that narrows the type to `SnapId` if it returns `true`. + * If provided, it will be used to conditionally apply snap-specific logic. + * If undefined, snap-specific logic will not be applied. + * @returns The converted CAIP-25 permission object. */ export const getCaip25PermissionFromLegacyPermissions = ( - // TODO: [ffmcgee] make sure snaps logic only triggered for extension ? --> Maybe pass in the isSnapId as an optional hook, and if it's undefined, the logic won't trigger - // otherwise, it will trigger and actually validate if isSnapId(origin) if the logic should run or not - _origin: string, + origin: string, requestedPermissions?: { [PermissionKeys.eth_accounts]?: { caveats?: { @@ -597,6 +599,7 @@ export const getCaip25PermissionFromLegacyPermissions = ( }[]; }; }, + isSnapId?: (value: string) => boolean, ) => { const permissions = pick(requestedPermissions, [ PermissionKeys.eth_accounts, @@ -611,10 +614,9 @@ export const getCaip25PermissionFromLegacyPermissions = ( permissions[PermissionKeys.permittedChains] = {}; } - // TODO: [ffmcgee] get isSnapId from snap utils - // if (isSnapId(origin)) { - // delete permissions[PermissionNames.permittedChains]; - // } + if (isSnapId?.(origin)) { + delete permissions[PermissionKeys.permittedChains]; + } const requestedAccounts = permissions[PermissionKeys.eth_accounts]?.caveats?.find( @@ -639,9 +641,7 @@ export const getCaip25PermissionFromLegacyPermissions = ( const caveatValueWithChains = setPermittedEthChainIds( newCaveatValue, - // TODO: [ffmcgee] get isSnapId from snap utils - // isSnapId(origin) ? [] : requestedChains, - requestedChains, + isSnapId?.(origin) ? [] : requestedChains, ); const caveatValueWithAccountsAndChains = setEthAccounts( @@ -680,6 +680,10 @@ export const getCaip25PermissionFromLegacyPermissions = ( * Incremental permission requests allow the caller to replace existing and/or add brand new permissions and caveats for the specified subject. * @param options.hooks.grantPermissionsIncremental - Incrementally grants approved permissions to the specified subject without prompting for user approval. * Every permission and caveat is stringently validated and an error is thrown if validation fails. + * @param options.hooks.isSnapId - Optional function to check if the origin is a valid snap ID. + * This function is a type guard that narrows the type to `SnapId` if it returns `true`. + * If provided, it will be used to conditionally apply snap-specific logic. + * If undefined, snap-specific logic will not be applied. */ export const requestPermittedChainsPermissionIncremental = async ({ origin, @@ -714,15 +718,15 @@ export const requestPermittedChainsPermissionIncremental = async ({ >; requestData?: Record; }) => Partial>; + isSnapId?: (value: string) => boolean; }; metadata?: { options: Record }; }) => { - // TODO: [ffmcgee] get isSnapId from snap utils - // if (isSnapId(origin)) { - // throw new Error( - // `Cannot request permittedChains permission for Snaps with origin "${origin}"`, - // ); - // } + if (hooks.isSnapId?.(origin)) { + throw new Error( + `Cannot request permittedChains permission for Snaps with origin "${origin}"`, + ); + } const caveatValueWithChains = setPermittedEthChainIds( { requiredScopes: {}, From 12ebe04134da2f61990186b27da338085a739f5c Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Fri, 1 Aug 2025 14:20:24 +0200 Subject: [PATCH 11/14] lint --- packages/chain-agnostic-permission/src/caip25Permission.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/chain-agnostic-permission/src/caip25Permission.ts b/packages/chain-agnostic-permission/src/caip25Permission.ts index 53c7f83f74c..231e34e2146 100644 --- a/packages/chain-agnostic-permission/src/caip25Permission.ts +++ b/packages/chain-agnostic-permission/src/caip25Permission.ts @@ -6,7 +6,6 @@ import type { PermissionValidatorConstraint, PermissionConstraint, EndowmentCaveatSpecificationConstraint, - PermissionController, } from '@metamask/permission-controller'; import { CaveatMutatorOperation, From 1a67553e8a80551efbddb54086b23bdd2b6bc9a6 Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Fri, 1 Aug 2025 14:38:16 +0200 Subject: [PATCH 12/14] fix: remove outdated isSnapId logic --- .../src/caip25Permission.test.ts | 76 ------------------- .../src/caip25Permission.ts | 21 +---- 2 files changed, 1 insertion(+), 96 deletions(-) diff --git a/packages/chain-agnostic-permission/src/caip25Permission.test.ts b/packages/chain-agnostic-permission/src/caip25Permission.test.ts index 85c2eabcc60..3d5c7cee1aa 100644 --- a/packages/chain-agnostic-permission/src/caip25Permission.test.ts +++ b/packages/chain-agnostic-permission/src/caip25Permission.test.ts @@ -1980,29 +1980,6 @@ describe('requestPermittedChainsPermissionIncremental', () => { ); }); - it('throws error when isSnapId is defined and returns true', async () => { - const mockIsSnapId = jest.fn().mockReturnValue(true); - - await expect(() => - requestPermittedChainsPermissionIncremental({ - origin: 'npm:snap', - chainId: '0x1', - autoApprove: false, - hooks: { - requestPermissionsIncremental: mockRequestPermissionsIncremental, - grantPermissionsIncremental: mockGrantPermissionsIncremental, - isSnapId: mockIsSnapId, - }, - }), - ).rejects.toThrow( - 'Cannot request permittedChains permission for Snaps with origin "npm:snap"', - ); - - expect(mockIsSnapId).toHaveBeenCalledWith('npm:snap'); - - expect(mockRequestPermissionsIncremental).not.toHaveBeenCalled(); - }); - it('does not throw error when isSnapId is undefined', async () => { mockRequestPermissionsIncremental.mockResolvedValue([ {}, @@ -2403,57 +2380,4 @@ describe('getCaip25PermissionFromLegacyPermissions', () => { }), ); }); - - it('deletes permittedChains permission when isSnapId is defined and returns true', () => { - const mockIsSnapId = jest.fn().mockReturnValue(true); - - const permissions = getCaip25PermissionFromLegacyPermissions( - 'npm:snap', - { - [PermissionKeys.eth_accounts]: { - caveats: [ - { - type: CaveatTypes.restrictReturnedAccounts, - value: ['0x0000000000000000000000000000000000000001'], - }, - ], - }, - [PermissionKeys.permittedChains]: { - caveats: [ - { - type: CaveatTypes.restrictNetworkSwitching, - value: ['0x64'], - }, - ], - }, - }, - mockIsSnapId, - ); - - expect(mockIsSnapId).toHaveBeenCalledWith('npm:snap'); - - expect(permissions).toStrictEqual( - expect.objectContaining({ - [Caip25EndowmentPermissionName]: { - caveats: [ - { - type: Caip25CaveatType, - value: { - requiredScopes: {}, - optionalScopes: { - 'wallet:eip155': { - accounts: [ - 'wallet:eip155:0x0000000000000000000000000000000000000001', - ], - }, - }, - isMultichainOrigin: false, - sessionProperties: {}, - }, - }, - ], - }, - }), - ); - }); }); diff --git a/packages/chain-agnostic-permission/src/caip25Permission.ts b/packages/chain-agnostic-permission/src/caip25Permission.ts index 231e34e2146..dd0c3bb834f 100644 --- a/packages/chain-agnostic-permission/src/caip25Permission.ts +++ b/packages/chain-agnostic-permission/src/caip25Permission.ts @@ -576,10 +576,6 @@ export function getCaip25CaveatFromPermission(caip25Permission?: { * @param requestedPermissions.caveats - The legacy caveats processed by the function. * - `restrictReturnedAccounts`: Restricts which Ethereum accounts can be accessed * - `restrictNetworkSwitching`: Restricts which blockchain networks can be used - * @param isSnapId - Optional function to check if the origin is a valid snap ID. - * This function is a type guard that narrows the type to `SnapId` if it returns `true`. - * If provided, it will be used to conditionally apply snap-specific logic. - * If undefined, snap-specific logic will not be applied. * @returns The converted CAIP-25 permission object. */ export const getCaip25PermissionFromLegacyPermissions = ( @@ -598,7 +594,6 @@ export const getCaip25PermissionFromLegacyPermissions = ( }[]; }; }, - isSnapId?: (value: string) => boolean, ) => { const permissions = pick(requestedPermissions, [ PermissionKeys.eth_accounts, @@ -613,10 +608,6 @@ export const getCaip25PermissionFromLegacyPermissions = ( permissions[PermissionKeys.permittedChains] = {}; } - if (isSnapId?.(origin)) { - delete permissions[PermissionKeys.permittedChains]; - } - const requestedAccounts = permissions[PermissionKeys.eth_accounts]?.caveats?.find( (caveat) => caveat.type === CaveatTypes.restrictReturnedAccounts, @@ -640,7 +631,7 @@ export const getCaip25PermissionFromLegacyPermissions = ( const caveatValueWithChains = setPermittedEthChainIds( newCaveatValue, - isSnapId?.(origin) ? [] : requestedChains, + requestedChains, ); const caveatValueWithAccountsAndChains = setEthAccounts( @@ -679,10 +670,6 @@ export const getCaip25PermissionFromLegacyPermissions = ( * Incremental permission requests allow the caller to replace existing and/or add brand new permissions and caveats for the specified subject. * @param options.hooks.grantPermissionsIncremental - Incrementally grants approved permissions to the specified subject without prompting for user approval. * Every permission and caveat is stringently validated and an error is thrown if validation fails. - * @param options.hooks.isSnapId - Optional function to check if the origin is a valid snap ID. - * This function is a type guard that narrows the type to `SnapId` if it returns `true`. - * If provided, it will be used to conditionally apply snap-specific logic. - * If undefined, snap-specific logic will not be applied. */ export const requestPermittedChainsPermissionIncremental = async ({ origin, @@ -717,15 +704,9 @@ export const requestPermittedChainsPermissionIncremental = async ({ >; requestData?: Record; }) => Partial>; - isSnapId?: (value: string) => boolean; }; metadata?: { options: Record }; }) => { - if (hooks.isSnapId?.(origin)) { - throw new Error( - `Cannot request permittedChains permission for Snaps with origin "${origin}"`, - ); - } const caveatValueWithChains = setPermittedEthChainIds( { requiredScopes: {}, From d81ff29c6f2aef5199952e1019252409caa75e4f Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Fri, 1 Aug 2025 16:51:36 +0200 Subject: [PATCH 13/14] fix: remove unecessary origin (wip) --- .../src/caip25Permission.test.ts | 39 +++---- .../src/caip25Permission.ts | 107 +++++++++--------- 2 files changed, 69 insertions(+), 77 deletions(-) diff --git a/packages/chain-agnostic-permission/src/caip25Permission.test.ts b/packages/chain-agnostic-permission/src/caip25Permission.test.ts index 3d5c7cee1aa..5f85319883a 100644 --- a/packages/chain-agnostic-permission/src/caip25Permission.test.ts +++ b/packages/chain-agnostic-permission/src/caip25Permission.test.ts @@ -2003,10 +2003,7 @@ describe('requestPermittedChainsPermissionIncremental', () => { describe('getCaip25PermissionFromLegacyPermissions', () => { it('returns valid CAIP-25 permissions', async () => { - const permissions = getCaip25PermissionFromLegacyPermissions( - 'test.com', - {}, - ); + const permissions = getCaip25PermissionFromLegacyPermissions({}); expect(permissions).toStrictEqual( expect.objectContaining({ @@ -2031,8 +2028,8 @@ describe('getCaip25PermissionFromLegacyPermissions', () => { ); }); - it('returns approval from the PermissionsController for eth_accounts and permittedChains when only eth_accounts is specified in params and origin is not snapId', async () => { - const permissions = getCaip25PermissionFromLegacyPermissions('test.com', { + it('returns approval from the PermissionsController for eth_accounts and permittedChains when only eth_accounts is specified in params', async () => { + const permissions = getCaip25PermissionFromLegacyPermissions({ [PermissionKeys.eth_accounts]: { caveats: [ { @@ -2068,8 +2065,8 @@ describe('getCaip25PermissionFromLegacyPermissions', () => { ); }); - it('returns approval from the PermissionsController for eth_accounts and permittedChains when only permittedChains is specified in params and origin is not snapId', async () => { - const permissions = getCaip25PermissionFromLegacyPermissions('test.com', { + it('returns approval from the PermissionsController for eth_accounts and permittedChains when only permittedChains is specified in params', async () => { + const permissions = getCaip25PermissionFromLegacyPermissions({ [PermissionKeys.permittedChains]: { caveats: [ { @@ -2106,8 +2103,8 @@ describe('getCaip25PermissionFromLegacyPermissions', () => { ); }); - it('returns approval from the PermissionsController for eth_accounts and permittedChains when both are specified in params and origin is not snapId', async () => { - const permissions = getCaip25PermissionFromLegacyPermissions('test.com', { + it('returns approval from the PermissionsController for eth_accounts and permittedChains when both are specified in params', async () => { + const permissions = getCaip25PermissionFromLegacyPermissions({ [PermissionKeys.eth_accounts]: { caveats: [ { @@ -2156,8 +2153,8 @@ describe('getCaip25PermissionFromLegacyPermissions', () => { ); }); - it('returns approval from the PermissionsController for only eth_accounts when only eth_accounts is specified in params and origin is snapId', async () => { - const permissions = getCaip25PermissionFromLegacyPermissions('npm:snap', { + it('returns approval from the PermissionsController for only eth_accounts when only eth_accounts is specified in params', async () => { + const permissions = getCaip25PermissionFromLegacyPermissions({ [PermissionKeys.eth_accounts]: { caveats: [ { @@ -2193,8 +2190,8 @@ describe('getCaip25PermissionFromLegacyPermissions', () => { ); }); - it('returns approval from the PermissionsController for only eth_accounts when only permittedChains is specified in params and origin is snapId', async () => { - const permissions = getCaip25PermissionFromLegacyPermissions('npm:snap', { + it('returns approval from the PermissionsController for only eth_accounts when only permittedChains is specified in params', async () => { + const permissions = getCaip25PermissionFromLegacyPermissions({ [PermissionKeys.permittedChains]: { caveats: [ { @@ -2231,8 +2228,8 @@ describe('getCaip25PermissionFromLegacyPermissions', () => { ); }); - it('returns approval from the PermissionsController for eth_accounts and permittedChains when both eth_accounts and permittedChains are specified in params and origin is snapId', async () => { - const permissions = getCaip25PermissionFromLegacyPermissions('npm:snap', { + it('returns approval from the PermissionsController for eth_accounts and permittedChains when both eth_accounts and permittedChains are specified in params', async () => { + const permissions = getCaip25PermissionFromLegacyPermissions({ [PermissionKeys.eth_accounts]: { caveats: [ { @@ -2281,8 +2278,8 @@ describe('getCaip25PermissionFromLegacyPermissions', () => { ); }); - it('returns CAIP-25 approval with accounts and chainIds specified from `eth_accounts` and `endowment:permittedChains` permissions caveats, and isMultichainOrigin: false if origin is not snapId', async () => { - const permissions = getCaip25PermissionFromLegacyPermissions('test.com', { + it('returns CAIP-25 approval with accounts and chainIds specified from `eth_accounts` and `endowment:permittedChains` permissions caveats', async () => { + const permissions = getCaip25PermissionFromLegacyPermissions({ [PermissionKeys.eth_accounts]: { caveats: [ { @@ -2330,10 +2327,8 @@ describe('getCaip25PermissionFromLegacyPermissions', () => { ); }); - it('returns CAIP-25 approval with approved accounts for the `wallet:eip155` scope with isMultichainOrigin: false if origin is snapId', async () => { - const origin = 'npm:snap'; - - const permissions = getCaip25PermissionFromLegacyPermissions(origin, { + it('returns CAIP-25 approval with approved accounts for the `wallet:eip155` scope', async () => { + const permissions = getCaip25PermissionFromLegacyPermissions({ [PermissionKeys.eth_accounts]: { caveats: [ { diff --git a/packages/chain-agnostic-permission/src/caip25Permission.ts b/packages/chain-agnostic-permission/src/caip25Permission.ts index dd0c3bb834f..ad20fcc6c5b 100644 --- a/packages/chain-agnostic-permission/src/caip25Permission.ts +++ b/packages/chain-agnostic-permission/src/caip25Permission.ts @@ -568,19 +568,17 @@ export function getCaip25CaveatFromPermission(caip25Permission?: { } /** - * Requests user approval for the CAIP-25 permission for the specified origin + * Requests user approval for the CAIP-25 permission * and returns a granted permissions object. * - * @param origin - The origin to request approval for. * @param requestedPermissions - The legacy permissions to request approval for. * @param requestedPermissions.caveats - The legacy caveats processed by the function. * - `restrictReturnedAccounts`: Restricts which Ethereum accounts can be accessed * - `restrictNetworkSwitching`: Restricts which blockchain networks can be used * @returns The converted CAIP-25 permission object. */ -export const getCaip25PermissionFromLegacyPermissions = ( - origin: string, - requestedPermissions?: { +export const getCaip25PermissionFromLegacyPermissions = + (requestedPermissions?: { [PermissionKeys.eth_accounts]?: { caveats?: { type: keyof typeof CaveatTypes; @@ -593,63 +591,62 @@ export const getCaip25PermissionFromLegacyPermissions = ( value: Hex[]; }[]; }; - }, -) => { - const permissions = pick(requestedPermissions, [ - PermissionKeys.eth_accounts, - PermissionKeys.permittedChains, - ]); - - if (!permissions[PermissionKeys.eth_accounts]) { - permissions[PermissionKeys.eth_accounts] = {}; - } + }) => { + const permissions = pick(requestedPermissions, [ + PermissionKeys.eth_accounts, + PermissionKeys.permittedChains, + ]); + + if (!permissions[PermissionKeys.eth_accounts]) { + permissions[PermissionKeys.eth_accounts] = {}; + } - if (!permissions[PermissionKeys.permittedChains]) { - permissions[PermissionKeys.permittedChains] = {}; - } + if (!permissions[PermissionKeys.permittedChains]) { + permissions[PermissionKeys.permittedChains] = {}; + } - const requestedAccounts = - permissions[PermissionKeys.eth_accounts]?.caveats?.find( - (caveat) => caveat.type === CaveatTypes.restrictReturnedAccounts, - )?.value ?? []; - - const requestedChains = - permissions[PermissionKeys.permittedChains]?.caveats?.find( - (caveat) => caveat.type === CaveatTypes.restrictNetworkSwitching, - )?.value ?? []; - - const newCaveatValue = { - requiredScopes: {}, - optionalScopes: { - 'wallet:eip155': { - accounts: [], + const requestedAccounts = + permissions[PermissionKeys.eth_accounts]?.caveats?.find( + (caveat) => caveat.type === CaveatTypes.restrictReturnedAccounts, + )?.value ?? []; + + const requestedChains = + permissions[PermissionKeys.permittedChains]?.caveats?.find( + (caveat) => caveat.type === CaveatTypes.restrictNetworkSwitching, + )?.value ?? []; + + const newCaveatValue = { + requiredScopes: {}, + optionalScopes: { + 'wallet:eip155': { + accounts: [], + }, }, - }, - sessionProperties: {}, - isMultichainOrigin: false, - }; + sessionProperties: {}, + isMultichainOrigin: false, + }; - const caveatValueWithChains = setPermittedEthChainIds( - newCaveatValue, - requestedChains, - ); + const caveatValueWithChains = setPermittedEthChainIds( + newCaveatValue, + requestedChains, + ); - const caveatValueWithAccountsAndChains = setEthAccounts( - caveatValueWithChains, - requestedAccounts, - ); + const caveatValueWithAccountsAndChains = setEthAccounts( + caveatValueWithChains, + requestedAccounts, + ); - return { - [Caip25EndowmentPermissionName]: { - caveats: [ - { - type: Caip25CaveatType, - value: caveatValueWithAccountsAndChains, - }, - ], - }, + return { + [Caip25EndowmentPermissionName]: { + caveats: [ + { + type: Caip25CaveatType, + value: caveatValueWithAccountsAndChains, + }, + ], + }, + }; }; -}; /** * Requests incremental permittedChains permission for the specified origin. From a811c5b72f477cbbf8180f83682f1c8672579dc9 Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Mon, 4 Aug 2025 09:11:58 +0200 Subject: [PATCH 14/14] test: remove reduntant test case --- .../src/caip25Permission.test.ts | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/packages/chain-agnostic-permission/src/caip25Permission.test.ts b/packages/chain-agnostic-permission/src/caip25Permission.test.ts index 5f85319883a..67bb525064e 100644 --- a/packages/chain-agnostic-permission/src/caip25Permission.test.ts +++ b/packages/chain-agnostic-permission/src/caip25Permission.test.ts @@ -1979,26 +1979,6 @@ describe('requestPermittedChainsPermissionIncremental', () => { { metadata }, ); }); - - it('does not throw error when isSnapId is undefined', async () => { - mockRequestPermissionsIncremental.mockResolvedValue([ - {}, - { id: 'id', origin: 'origin' }, - ]); - - await requestPermittedChainsPermissionIncremental({ - origin: 'npm:snap', - chainId: '0x1', - autoApprove: false, - hooks: { - requestPermissionsIncremental: mockRequestPermissionsIncremental, - grantPermissionsIncremental: mockGrantPermissionsIncremental, - // isSnapId is undefined - }, - }); - - expect(mockRequestPermissionsIncremental).toHaveBeenCalled(); - }); }); describe('getCaip25PermissionFromLegacyPermissions', () => {