From 8269d22434288919c51573f6078179fbacbfecb4 Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Mon, 22 Sep 2025 09:32:53 +0200 Subject: [PATCH 1/9] feat: adapt revokeSession for partial session removal (wip) --- .../src/handlers/wallet-revokeSession.ts | 63 +++++++++++++++++-- 1 file changed, 59 insertions(+), 4 deletions(-) diff --git a/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.ts b/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.ts index 59fced841d3..880d0afe50c 100644 --- a/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.ts +++ b/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.ts @@ -1,9 +1,15 @@ -import { Caip25EndowmentPermissionName } from '@metamask/chain-agnostic-permission'; +import { + Caip25CaveatMutators, + Caip25CaveatType, + type Caip25CaveatValue, + Caip25EndowmentPermissionName, +} from '@metamask/chain-agnostic-permission'; import type { JsonRpcEngineNextCallback, JsonRpcEngineEndCallback, } from '@metamask/json-rpc-engine'; import { + type Caveat, PermissionDoesNotExistError, UnrecognizedSubjectError, } from '@metamask/permission-controller'; @@ -17,25 +23,70 @@ import type { JsonRpcSuccess, JsonRpcRequest } from '@metamask/utils'; * the handler also does not return an error if there is currently no active session and instead * returns true which is the same result returned if an active session was actually revoked. * - * @param _request - The JSON-RPC request object. Unused. + * @param request - The JSON-RPC request object. Unused. * @param response - The JSON-RPC response object. * @param _next - The next middleware function. Unused. * @param end - The end callback function. * @param hooks - The hooks object. * @param hooks.revokePermissionForOrigin - The hook for revoking a permission for an origin function. + * @param hooks.updateCaveat - + * @param hooks.getCaveatForOrigin - * @returns Nothing. */ async function walletRevokeSessionHandler( - _request: JsonRpcRequest & { origin: string }, + request: JsonRpcRequest & { origin: string }, response: JsonRpcSuccess, _next: JsonRpcEngineNextCallback, end: JsonRpcEngineEndCallback, hooks: { revokePermissionForOrigin: (permissionName: string) => void; + updateCaveat: ( + target: string, + caveatType: string, + caveatValue: Caip25CaveatValue, + ) => void; + getCaveatForOrigin: ( + endowmentPermissionName: string, + caveatType: string, + ) => Caveat; }, ) { + const { + // @ts-expect-error TODO: [ffmcgee] type error + params: { sessionScopes }, + } = request; + + console.log({ sessionScopes }); try { - hooks.revokePermissionForOrigin(Caip25EndowmentPermissionName); + if (sessionScopes) { + // get the permission + const caveat = hooks.getCaveatForOrigin( + Caip25EndowmentPermissionName, + Caip25CaveatType, + ); + console.log({ caveat }); + + // remove the scopes from permission + let updatedCaveatValue; + for (const scopeString of sessionScopes) { + updatedCaveatValue = Caip25CaveatMutators[Caip25CaveatType].removeScope( + caveat.value, + scopeString, + ).value; + + console.log({ scopeString, updatedCaveatValue }); + } + + // updateCaveat with new permissions + hooks.updateCaveat( + Caip25EndowmentPermissionName, + Caip25CaveatType, + // @ts-expect-error TODO: [ffmcgee] type error + updatedCaveatValue, + ); + } else { + hooks.revokePermissionForOrigin(Caip25EndowmentPermissionName); + } } catch (err) { if ( !(err instanceof UnrecognizedSubjectError) && @@ -44,6 +95,8 @@ async function walletRevokeSessionHandler( console.error(err); return end(rpcErrors.internal()); } + + console.log({ err }); } response.result = true; @@ -54,5 +107,7 @@ export const walletRevokeSession = { implementation: walletRevokeSessionHandler, hookNames: { revokePermissionForOrigin: true, + updateCaveat: true, + getCaveatForOrigin: true, }, }; From 4f141de90bca3a50521535a2a2d683153a6d4897 Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Mon, 22 Sep 2025 11:40:35 +0200 Subject: [PATCH 2/9] code clean up and extra test cases for wallet_revokeSession --- .../src/handlers/wallet-revokeSession.test.ts | 69 +++++++++++++++++-- .../src/handlers/wallet-revokeSession.ts | 41 +++++------ 2 files changed, 83 insertions(+), 27 deletions(-) diff --git a/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.test.ts b/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.test.ts index c800383d6a0..a740c662772 100644 --- a/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.test.ts +++ b/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.test.ts @@ -1,4 +1,7 @@ -import { Caip25EndowmentPermissionName } from '@metamask/chain-agnostic-permission'; +import { + Caip25CaveatType, + Caip25EndowmentPermissionName, +} from '@metamask/chain-agnostic-permission'; import { PermissionDoesNotExistError, UnrecognizedSubjectError, @@ -8,7 +11,10 @@ import type { JsonRpcRequest } from '@metamask/utils'; import { walletRevokeSession } from './wallet-revokeSession'; -const baseRequest: JsonRpcRequest & { origin: string } = { +const baseRequest: JsonRpcRequest & { + origin: string; + params: { sessionScopes?: string[] }; +} = { origin: 'http://test.com', params: {}, jsonrpc: '2.0' as const, @@ -20,14 +26,23 @@ const createMockedHandler = () => { const next = jest.fn(); const end = jest.fn(); const revokePermissionForOrigin = jest.fn(); + const updateCaveat = jest.fn(); + const getCaveatForOrigin = jest.fn(); const response = { result: true, id: 1, jsonrpc: '2.0' as const, }; - const handler = (request: JsonRpcRequest & { origin: string }) => + const handler = ( + request: JsonRpcRequest & { + origin: string; + params: { sessionScopes?: string[] }; + }, + ) => walletRevokeSession.implementation(request, response, next, end, { revokePermissionForOrigin, + updateCaveat, + getCaveatForOrigin, }); return { @@ -35,12 +50,14 @@ const createMockedHandler = () => { response, end, revokePermissionForOrigin, + updateCaveat, + getCaveatForOrigin, handler, }; }; describe('wallet_revokeSession', () => { - it('revokes the the CAIP-25 endowment permission', async () => { + it('revokes the CAIP-25 endowment permission', async () => { const { handler, revokePermissionForOrigin } = createMockedHandler(); await handler(baseRequest); @@ -49,6 +66,50 @@ describe('wallet_revokeSession', () => { ); }); + it('partially revokes the CAIP-25 endowment permission if `sessionScopes` param is passed in', async () => { + const { handler, getCaveatForOrigin, updateCaveat } = createMockedHandler(); + getCaveatForOrigin.mockImplementation(() => ({ + value: { + optionalScopes: { + 'eip155:1': { + accounts: [], + }, + 'eip155:5': { + accounts: [], + }, + }, + requiredScopes: {}, + }, + })); + + await handler({ ...baseRequest, params: { sessionScopes: ['eip155:1'] } }); + expect(updateCaveat).toHaveBeenCalledWith( + Caip25EndowmentPermissionName, + Caip25CaveatType, + { + optionalScopes: { 'eip155:5': { accounts: [] } }, + requiredScopes: {}, + }, + ); + }); + + it('not call `updateCaveat` if `sessionScopes` param is passed in with non existing permitted scope', async () => { + const { handler, getCaveatForOrigin, updateCaveat } = createMockedHandler(); + getCaveatForOrigin.mockImplementation(() => ({ + value: { + optionalScopes: { + 'eip155:1': { + accounts: [], + }, + }, + requiredScopes: {}, + }, + })); + + await handler({ ...baseRequest, params: { sessionScopes: ['eip155:5'] } }); + expect(updateCaveat).not.toHaveBeenCalled(); + }); + it('returns true if the CAIP-25 endowment permission does not exist', async () => { const { handler, response, revokePermissionForOrigin } = createMockedHandler(); diff --git a/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.ts b/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.ts index 880d0afe50c..a02b571f0a7 100644 --- a/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.ts +++ b/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.ts @@ -34,7 +34,10 @@ import type { JsonRpcSuccess, JsonRpcRequest } from '@metamask/utils'; * @returns Nothing. */ async function walletRevokeSessionHandler( - request: JsonRpcRequest & { origin: string }, + request: JsonRpcRequest & { + origin: string; + params: { sessionScopes?: string[] }; + }, response: JsonRpcSuccess, _next: JsonRpcEngineNextCallback, end: JsonRpcEngineEndCallback, @@ -52,38 +55,32 @@ async function walletRevokeSessionHandler( }, ) { const { - // @ts-expect-error TODO: [ffmcgee] type error params: { sessionScopes }, } = request; - console.log({ sessionScopes }); try { - if (sessionScopes) { - // get the permission - const caveat = hooks.getCaveatForOrigin( + if (sessionScopes?.length) { + const existingCaveat = hooks.getCaveatForOrigin( Caip25EndowmentPermissionName, Caip25CaveatType, ); - console.log({ caveat }); - // remove the scopes from permission let updatedCaveatValue; for (const scopeString of sessionScopes) { - updatedCaveatValue = Caip25CaveatMutators[Caip25CaveatType].removeScope( - caveat.value, - scopeString, - ).value; - - console.log({ scopeString, updatedCaveatValue }); + updatedCaveatValue = + Caip25CaveatMutators[Caip25CaveatType].removeScope( + existingCaveat.value, + scopeString, + )?.value ?? updatedCaveatValue; } - // updateCaveat with new permissions - hooks.updateCaveat( - Caip25EndowmentPermissionName, - Caip25CaveatType, - // @ts-expect-error TODO: [ffmcgee] type error - updatedCaveatValue, - ); + if (updatedCaveatValue) { + hooks.updateCaveat( + Caip25EndowmentPermissionName, + Caip25CaveatType, + updatedCaveatValue, + ); + } } else { hooks.revokePermissionForOrigin(Caip25EndowmentPermissionName); } @@ -95,8 +92,6 @@ async function walletRevokeSessionHandler( console.error(err); return end(rpcErrors.internal()); } - - console.log({ err }); } response.result = true; From 53b569f8c704ae4cd55bc273f90ec299ace8f6a6 Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Mon, 22 Sep 2025 11:45:18 +0200 Subject: [PATCH 3/9] chore: update CHANGELOG.md --- packages/multichain-api-middleware/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/multichain-api-middleware/CHANGELOG.md b/packages/multichain-api-middleware/CHANGELOG.md index f9cdf00d13e..d905f341b51 100644 --- a/packages/multichain-api-middleware/CHANGELOG.md +++ b/packages/multichain-api-middleware/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Add partial permission revoke into `wallet_revokeSession` ([#6668](https://github.com/MetaMask/core/pull/6668)) - Bump `@metamask/chain-agnostic-permission` from `1.0.0` to `1.1.1` ([#6241](https://github.com/MetaMask/core/pull/6241), [#6345](https://github.com/MetaMask/core/pull/6241)) - Bump `@metamask/controller-utils` from `^11.10.0` to `^11.14.0` ([#6069](https://github.com/MetaMask/core/pull/6069), [#6303](https://github.com/MetaMask/core/pull/6303), [#6620](https://github.com/MetaMask/core/pull/6620), [#6629](https://github.com/MetaMask/core/pull/6629)) - Bump `@metamask/network-controller` from `^24.0.0` to `^24.1.0` ([#6148](https://github.com/MetaMask/core/pull/6148), [#6303](https://github.com/MetaMask/core/pull/6303)) From c89ddcd264e58531233dc71897482d5bb28c6e27 Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Mon, 22 Sep 2025 14:08:01 +0200 Subject: [PATCH 4/9] fix: fix stale caveat usage when iterating scopes --- .../src/handlers/wallet-revokeSession.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.ts b/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.ts index a02b571f0a7..e3363a31bcb 100644 --- a/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.ts +++ b/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.ts @@ -69,7 +69,7 @@ async function walletRevokeSessionHandler( for (const scopeString of sessionScopes) { updatedCaveatValue = Caip25CaveatMutators[Caip25CaveatType].removeScope( - existingCaveat.value, + updatedCaveatValue ?? existingCaveat.value, scopeString, )?.value ?? updatedCaveatValue; } From 9a871734164e9aa51ea258f18af12c6e1c66f2c9 Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Tue, 23 Sep 2025 10:07:24 +0200 Subject: [PATCH 5/9] address code review --- .../src/handlers/wallet-revokeSession.test.ts | 24 ++++-- .../src/handlers/wallet-revokeSession.ts | 85 +++++++++++++------ 2 files changed, 74 insertions(+), 35 deletions(-) diff --git a/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.test.ts b/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.test.ts index a740c662772..0efe7668e96 100644 --- a/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.test.ts +++ b/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.test.ts @@ -13,7 +13,7 @@ import { walletRevokeSession } from './wallet-revokeSession'; const baseRequest: JsonRpcRequest & { origin: string; - params: { sessionScopes?: string[] }; + params: { scopes?: string[] }; } = { origin: 'http://test.com', params: {}, @@ -36,7 +36,7 @@ const createMockedHandler = () => { const handler = ( request: JsonRpcRequest & { origin: string; - params: { sessionScopes?: string[] }; + params: { scopes?: string[] }; }, ) => walletRevokeSession.implementation(request, response, next, end, { @@ -66,34 +66,40 @@ describe('wallet_revokeSession', () => { ); }); - it('partially revokes the CAIP-25 endowment permission if `sessionScopes` param is passed in', async () => { + it('partially revokes the CAIP-25 endowment permission if `scopes` param is passed in', async () => { const { handler, getCaveatForOrigin, updateCaveat } = createMockedHandler(); getCaveatForOrigin.mockImplementation(() => ({ value: { optionalScopes: { 'eip155:1': { - accounts: [], + accounts: ['eip155:1:0xdeadbeef'], }, 'eip155:5': { - accounts: [], + accounts: ['eip155:5:0xdeadbeef'], + }, + 'eip155:10': { + accounts: ['eip155:10:0xdeadbeef'], }, }, requiredScopes: {}, }, })); - await handler({ ...baseRequest, params: { sessionScopes: ['eip155:1'] } }); + await handler({ ...baseRequest, params: { scopes: ['eip155:1'] } }); expect(updateCaveat).toHaveBeenCalledWith( Caip25EndowmentPermissionName, Caip25CaveatType, { - optionalScopes: { 'eip155:5': { accounts: [] } }, + optionalScopes: { + 'eip155:5': { accounts: ['eip155:5:0xdeadbeef'] }, + 'eip155:10': { accounts: ['eip155:10:0xdeadbeef'] }, + }, requiredScopes: {}, }, ); }); - it('not call `updateCaveat` if `sessionScopes` param is passed in with non existing permitted scope', async () => { + it('not call `updateCaveat` if `scopes` param is passed in with non existing permitted scope', async () => { const { handler, getCaveatForOrigin, updateCaveat } = createMockedHandler(); getCaveatForOrigin.mockImplementation(() => ({ value: { @@ -106,7 +112,7 @@ describe('wallet_revokeSession', () => { }, })); - await handler({ ...baseRequest, params: { sessionScopes: ['eip155:5'] } }); + await handler({ ...baseRequest, params: { scopes: ['eip155:5'] } }); expect(updateCaveat).not.toHaveBeenCalled(); }); diff --git a/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.ts b/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.ts index e3363a31bcb..3a8618efe7f 100644 --- a/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.ts +++ b/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.ts @@ -3,6 +3,7 @@ import { Caip25CaveatType, type Caip25CaveatValue, Caip25EndowmentPermissionName, + getCaipAccountIdsFromCaip25CaveatValue, } from '@metamask/chain-agnostic-permission'; import type { JsonRpcEngineNextCallback, @@ -16,6 +17,58 @@ import { import { rpcErrors } from '@metamask/rpc-errors'; import type { JsonRpcSuccess, JsonRpcRequest } from '@metamask/utils'; +/** + * Revokes specific session scopes from an existing caveat. + * Fully revokes permission if no accounts remain permitted after iterating through scopes. + * + * @param scopes - Array of scope strings to remove from the caveat. + * @param hooks - The hooks object. + * @param hooks.revokePermissionForOrigin - The hook for revoking a permission for an origin function. + * @param hooks.updateCaveat - The hook used to conditionally update the caveat rather than fully revoke the permission. + * @param hooks.getCaveatForOrigin - The hook to fetch an existing caveat for the origin of the request. + */ +function partialRevokePermissions( + scopes: string[], + hooks: { + revokePermissionForOrigin: (permissionName: string) => void; + updateCaveat: ( + target: string, + caveatType: string, + caveatValue: Caip25CaveatValue, + ) => void; + getCaveatForOrigin: ( + endowmentPermissionName: string, + caveatType: string, + ) => Caveat; + }, +) { + let updatedCaveatValue = hooks.getCaveatForOrigin( + Caip25EndowmentPermissionName, + Caip25CaveatType, + ).value; + for (const scopeString of scopes) { + updatedCaveatValue = + Caip25CaveatMutators[Caip25CaveatType].removeScope( + updatedCaveatValue, + scopeString, + )?.value ?? updatedCaveatValue; + } + + const caipAccountIds = + getCaipAccountIdsFromCaip25CaveatValue(updatedCaveatValue); + + // We fully revoke permission if no accounts are left after scope removal loop. + if (!caipAccountIds.length) { + hooks.revokePermissionForOrigin(Caip25EndowmentPermissionName); + } else { + hooks.updateCaveat( + Caip25EndowmentPermissionName, + Caip25CaveatType, + updatedCaveatValue, + ); + } +} + /** * Handler for the `wallet_revokeSession` RPC method as specified by [CAIP-285](https://chainagnostic.org/CAIPs/caip-285). * The implementation below deviates from the linked spec in that it ignores the `sessionId` param @@ -29,14 +82,14 @@ import type { JsonRpcSuccess, JsonRpcRequest } from '@metamask/utils'; * @param end - The end callback function. * @param hooks - The hooks object. * @param hooks.revokePermissionForOrigin - The hook for revoking a permission for an origin function. - * @param hooks.updateCaveat - - * @param hooks.getCaveatForOrigin - + * @param hooks.updateCaveat - The hook used to conditionally update the caveat rather than fully revoke the permission. + * @param hooks.getCaveatForOrigin - The hook to fetch an existing caveat for the origin of the request. * @returns Nothing. */ async function walletRevokeSessionHandler( request: JsonRpcRequest & { origin: string; - params: { sessionScopes?: string[] }; + params: { scopes?: string[] }; }, response: JsonRpcSuccess, _next: JsonRpcEngineNextCallback, @@ -55,32 +108,12 @@ async function walletRevokeSessionHandler( }, ) { const { - params: { sessionScopes }, + params: { scopes }, } = request; try { - if (sessionScopes?.length) { - const existingCaveat = hooks.getCaveatForOrigin( - Caip25EndowmentPermissionName, - Caip25CaveatType, - ); - - let updatedCaveatValue; - for (const scopeString of sessionScopes) { - updatedCaveatValue = - Caip25CaveatMutators[Caip25CaveatType].removeScope( - updatedCaveatValue ?? existingCaveat.value, - scopeString, - )?.value ?? updatedCaveatValue; - } - - if (updatedCaveatValue) { - hooks.updateCaveat( - Caip25EndowmentPermissionName, - Caip25CaveatType, - updatedCaveatValue, - ); - } + if (scopes?.length) { + partialRevokePermissions(scopes, hooks); } else { hooks.revokePermissionForOrigin(Caip25EndowmentPermissionName); } From 9c518af811682b9de5df818324ad2ef130b26ba5 Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Tue, 23 Sep 2025 17:18:27 +0200 Subject: [PATCH 6/9] test: add 1 more test for full permission revoke if no accounts remain --- .../src/handlers/wallet-revokeSession.test.ts | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.test.ts b/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.test.ts index 0efe7668e96..b287d407851 100644 --- a/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.test.ts +++ b/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.test.ts @@ -116,6 +116,27 @@ describe('wallet_revokeSession', () => { expect(updateCaveat).not.toHaveBeenCalled(); }); + it('fully revokes permission when all accounts are removed after scope removal', async () => { + const { handler, getCaveatForOrigin, updateCaveat, revokePermissionForOrigin } = createMockedHandler(); + getCaveatForOrigin.mockImplementation(() => ({ + value: { + optionalScopes: { + 'eip155:1': { + accounts: ['eip155:1:0xdeadbeef'], + }, + 'eip155:5': { + accounts: ['eip155:5:0xdeadbeef'], + }, + }, + requiredScopes: {}, + }, + })); + + await handler({ ...baseRequest, params: { scopes: ['eip155:1', 'eip155:5'] } }); + expect(updateCaveat).not.toHaveBeenCalled(); + expect(revokePermissionForOrigin).toHaveBeenCalledWith(Caip25EndowmentPermissionName); + }); + it('returns true if the CAIP-25 endowment permission does not exist', async () => { const { handler, response, revokePermissionForOrigin } = createMockedHandler(); From 9a55a2d03c07911453aaf0ee9e1ee33a81c2d8b5 Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Tue, 23 Sep 2025 17:33:01 +0200 Subject: [PATCH 7/9] fix: address bug with residual permission --- .../src/handlers/wallet-revokeSession.test.ts | 16 +++++++++++++--- .../src/handlers/wallet-revokeSession.ts | 15 ++++++++++----- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.test.ts b/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.test.ts index b287d407851..5824fc61c91 100644 --- a/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.test.ts +++ b/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.test.ts @@ -117,7 +117,12 @@ describe('wallet_revokeSession', () => { }); it('fully revokes permission when all accounts are removed after scope removal', async () => { - const { handler, getCaveatForOrigin, updateCaveat, revokePermissionForOrigin } = createMockedHandler(); + const { + handler, + getCaveatForOrigin, + updateCaveat, + revokePermissionForOrigin, + } = createMockedHandler(); getCaveatForOrigin.mockImplementation(() => ({ value: { optionalScopes: { @@ -132,9 +137,14 @@ describe('wallet_revokeSession', () => { }, })); - await handler({ ...baseRequest, params: { scopes: ['eip155:1', 'eip155:5'] } }); + await handler({ + ...baseRequest, + params: { scopes: ['eip155:1', 'eip155:5'] }, + }); expect(updateCaveat).not.toHaveBeenCalled(); - expect(revokePermissionForOrigin).toHaveBeenCalledWith(Caip25EndowmentPermissionName); + expect(revokePermissionForOrigin).toHaveBeenCalledWith( + Caip25EndowmentPermissionName, + ); }); it('returns true if the CAIP-25 endowment permission does not exist', async () => { diff --git a/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.ts b/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.ts index 3a8618efe7f..75eb4cdd829 100644 --- a/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.ts +++ b/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.ts @@ -46,12 +46,17 @@ function partialRevokePermissions( Caip25EndowmentPermissionName, Caip25CaveatType, ).value; + for (const scopeString of scopes) { - updatedCaveatValue = - Caip25CaveatMutators[Caip25CaveatType].removeScope( - updatedCaveatValue, - scopeString, - )?.value ?? updatedCaveatValue; + updatedCaveatValue = Caip25CaveatMutators[Caip25CaveatType].removeScope( + updatedCaveatValue, + scopeString, + )?.value ?? { + requiredScopes: {}, + optionalScopes: {}, + sessionProperties: {}, + isMultichainOrigin: true, + }; } const caipAccountIds = From dbdc6e61ea638334c998a7f33abbad507852fae7 Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Tue, 23 Sep 2025 17:36:43 +0200 Subject: [PATCH 8/9] refactor: extract WalletRevokeSessionHooks type --- .../src/handlers/types.ts | 18 +++++++++++ .../src/handlers/wallet-revokeSession.ts | 30 +++---------------- 2 files changed, 22 insertions(+), 26 deletions(-) diff --git a/packages/multichain-api-middleware/src/handlers/types.ts b/packages/multichain-api-middleware/src/handlers/types.ts index 5c0f3f336a6..b5a4bec7c83 100644 --- a/packages/multichain-api-middleware/src/handlers/types.ts +++ b/packages/multichain-api-middleware/src/handlers/types.ts @@ -1,4 +1,9 @@ import type { + Caip25CaveatType, + Caip25CaveatValue, +} from '@metamask/chain-agnostic-permission'; +import type { + Caveat, CaveatSpecificationConstraint, PermissionController, PermissionSpecificationConstraint, @@ -19,3 +24,16 @@ type AbstractPermissionController = PermissionController< export type GrantedPermissions = Awaited< ReturnType >[0]; + +export type WalletRevokeSessionHooks = { + revokePermissionForOrigin: (permissionName: string) => void; + updateCaveat: ( + target: string, + caveatType: string, + caveatValue: Caip25CaveatValue, + ) => void; + getCaveatForOrigin: ( + endowmentPermissionName: string, + caveatType: string, + ) => Caveat; +}; diff --git a/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.ts b/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.ts index 75eb4cdd829..7510497f99f 100644 --- a/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.ts +++ b/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.ts @@ -1,7 +1,6 @@ import { Caip25CaveatMutators, Caip25CaveatType, - type Caip25CaveatValue, Caip25EndowmentPermissionName, getCaipAccountIdsFromCaip25CaveatValue, } from '@metamask/chain-agnostic-permission'; @@ -10,13 +9,14 @@ import type { JsonRpcEngineEndCallback, } from '@metamask/json-rpc-engine'; import { - type Caveat, PermissionDoesNotExistError, UnrecognizedSubjectError, } from '@metamask/permission-controller'; import { rpcErrors } from '@metamask/rpc-errors'; import type { JsonRpcSuccess, JsonRpcRequest } from '@metamask/utils'; +import type { WalletRevokeSessionHooks } from './types'; + /** * Revokes specific session scopes from an existing caveat. * Fully revokes permission if no accounts remain permitted after iterating through scopes. @@ -29,18 +29,7 @@ import type { JsonRpcSuccess, JsonRpcRequest } from '@metamask/utils'; */ function partialRevokePermissions( scopes: string[], - hooks: { - revokePermissionForOrigin: (permissionName: string) => void; - updateCaveat: ( - target: string, - caveatType: string, - caveatValue: Caip25CaveatValue, - ) => void; - getCaveatForOrigin: ( - endowmentPermissionName: string, - caveatType: string, - ) => Caveat; - }, + hooks: WalletRevokeSessionHooks, ) { let updatedCaveatValue = hooks.getCaveatForOrigin( Caip25EndowmentPermissionName, @@ -99,18 +88,7 @@ async function walletRevokeSessionHandler( response: JsonRpcSuccess, _next: JsonRpcEngineNextCallback, end: JsonRpcEngineEndCallback, - hooks: { - revokePermissionForOrigin: (permissionName: string) => void; - updateCaveat: ( - target: string, - caveatType: string, - caveatValue: Caip25CaveatValue, - ) => void; - getCaveatForOrigin: ( - endowmentPermissionName: string, - caveatType: string, - ) => Caveat; - }, + hooks: WalletRevokeSessionHooks, ) { const { params: { scopes }, From 739a33969bba98dd2a7709b69a07ec1a974e5b9b Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Tue, 23 Sep 2025 17:54:45 +0200 Subject: [PATCH 9/9] fix: fix bug where invalid scope would result in full permission revoke --- .../src/handlers/wallet-revokeSession.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.ts b/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.ts index 7510497f99f..2a07207ea21 100644 --- a/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.ts +++ b/packages/multichain-api-middleware/src/handlers/wallet-revokeSession.ts @@ -9,6 +9,7 @@ import type { JsonRpcEngineEndCallback, } from '@metamask/json-rpc-engine'; import { + CaveatMutatorOperation, PermissionDoesNotExistError, UnrecognizedSubjectError, } from '@metamask/permission-controller'; @@ -37,10 +38,17 @@ function partialRevokePermissions( ).value; for (const scopeString of scopes) { - updatedCaveatValue = Caip25CaveatMutators[Caip25CaveatType].removeScope( + const result = Caip25CaveatMutators[Caip25CaveatType].removeScope( updatedCaveatValue, scopeString, - )?.value ?? { + ); + + // If operation is a Noop, it means a scope was passed that was not present in the permission, so we proceed with the loop + if (result.operation === CaveatMutatorOperation.Noop) { + continue; + } + + updatedCaveatValue = result?.value ?? { requiredScopes: {}, optionalScopes: {}, sessionProperties: {},