From e785683a989ab9c8bc7f9fbbbff5f1a64d1d2a18 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Fri, 24 Apr 2026 13:32:55 -0700 Subject: [PATCH 1/4] refactor(permission-controller)!: Remove unrestricted rpc methods --- packages/permission-controller/.eslintrc.js | 5 +- packages/permission-controller/src/index.ts | 8 +- .../src/rpc-methods/getPermissions.test.ts | 74 ------- .../src/rpc-methods/getPermissions.ts | 46 ---- .../src/rpc-methods/index.ts | 16 -- .../rpc-methods/requestPermissions.test.ts | 143 ------------- .../src/rpc-methods/requestPermissions.ts | 63 ------ .../src/rpc-methods/revokePermissions.test.ts | 196 ------------------ .../src/rpc-methods/revokePermissions.ts | 79 ------- packages/permission-controller/src/utils.ts | 49 ----- 10 files changed, 2 insertions(+), 677 deletions(-) delete mode 100644 packages/permission-controller/src/rpc-methods/getPermissions.test.ts delete mode 100644 packages/permission-controller/src/rpc-methods/getPermissions.ts delete mode 100644 packages/permission-controller/src/rpc-methods/index.ts delete mode 100644 packages/permission-controller/src/rpc-methods/requestPermissions.test.ts delete mode 100644 packages/permission-controller/src/rpc-methods/requestPermissions.ts delete mode 100644 packages/permission-controller/src/rpc-methods/revokePermissions.test.ts delete mode 100644 packages/permission-controller/src/rpc-methods/revokePermissions.ts diff --git a/packages/permission-controller/.eslintrc.js b/packages/permission-controller/.eslintrc.js index 29de5a0688d..1e60b9c9bd4 100644 --- a/packages/permission-controller/.eslintrc.js +++ b/packages/permission-controller/.eslintrc.js @@ -3,10 +3,7 @@ module.exports = { overrides: [ { - files: [ - 'src/PermissionController.test.ts', - 'src/rpc-methods/revokePermissions.test.ts', - ], + files: ['src/PermissionController.test.ts'], rules: { // This is taken directly from @metamask/eslint-config-typescript@12.1.0 '@typescript-eslint/naming-convention': [ diff --git a/packages/permission-controller/src/index.ts b/packages/permission-controller/src/index.ts index 36292db1874..ae79f7cacdb 100644 --- a/packages/permission-controller/src/index.ts +++ b/packages/permission-controller/src/index.ts @@ -27,14 +27,8 @@ export { createPermissionMiddlewareV2, type PermissionMiddlewareActions, } from './permission-middleware'; -export type { - ExtractSpecifications, - HandlerMiddlewareFunction, - HookNames, - PermittedHandlerExport, -} from './utils'; +export type { ExtractSpecifications } from './utils'; export { MethodNames } from './utils'; -export * as permissionRpcMethods from './rpc-methods'; export * from './SubjectMetadataController'; export type { SubjectMetadataControllerClearStateAction, diff --git a/packages/permission-controller/src/rpc-methods/getPermissions.test.ts b/packages/permission-controller/src/rpc-methods/getPermissions.test.ts deleted file mode 100644 index f7724f4617b..00000000000 --- a/packages/permission-controller/src/rpc-methods/getPermissions.test.ts +++ /dev/null @@ -1,74 +0,0 @@ -import { JsonRpcEngine } from '@metamask/json-rpc-engine'; -import type { JsonRpcRequest, PendingJsonRpcResponse } from '@metamask/utils'; -import { assertIsJsonRpcSuccess } from '@metamask/utils'; - -import type { PermissionConstraint } from '../Permission'; -import { getPermissionsHandler } from './getPermissions'; - -describe('getPermissions RPC method', () => { - it('returns the values of the object returned by getPermissionsForOrigin', async () => { - const { implementation } = getPermissionsHandler; - const mockGetPermissionsForOrigin = jest.fn().mockImplementationOnce(() => { - return { a: 'a', b: 'b', c: 'c' }; - }); - - const engine = new JsonRpcEngine(); - engine.push( - ( - req: JsonRpcRequest<[]>, - res: PendingJsonRpcResponse, - next, - end, - ) => { - // We intentionally do not await this promise; JsonRpcEngine won't await - // middleware anyway. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - implementation(req, res, next, end, { - getPermissionsForOrigin: mockGetPermissionsForOrigin, - }); - }, - ); - - const response = await engine.handle({ - jsonrpc: '2.0', - id: 1, - method: 'arbitraryName', - }); - assertIsJsonRpcSuccess(response); - expect(response.result).toStrictEqual(['a', 'b', 'c']); - expect(mockGetPermissionsForOrigin).toHaveBeenCalledTimes(1); - }); - - it('returns an empty array if getPermissionsForOrigin returns a falsy value', async () => { - const { implementation } = getPermissionsHandler; - const mockGetPermissionsForOrigin = jest - .fn() - .mockImplementationOnce(() => null); - - const engine = new JsonRpcEngine(); - engine.push( - ( - req: JsonRpcRequest<[]>, - res: PendingJsonRpcResponse, - next, - end, - ) => { - // We intentionally do not await this promise; JsonRpcEngine won't await - // middleware anyway. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - implementation(req, res, next, end, { - getPermissionsForOrigin: mockGetPermissionsForOrigin, - }); - }, - ); - - const response = await engine.handle({ - jsonrpc: '2.0', - id: 1, - method: 'arbitraryName', - }); - assertIsJsonRpcSuccess(response); - expect(response.result).toStrictEqual([]); - expect(mockGetPermissionsForOrigin).toHaveBeenCalledTimes(1); - }); -}); diff --git a/packages/permission-controller/src/rpc-methods/getPermissions.ts b/packages/permission-controller/src/rpc-methods/getPermissions.ts deleted file mode 100644 index b7119973227..00000000000 --- a/packages/permission-controller/src/rpc-methods/getPermissions.ts +++ /dev/null @@ -1,46 +0,0 @@ -import type { JsonRpcEngineEndCallback } from '@metamask/json-rpc-engine'; -import type { PendingJsonRpcResponse } from '@metamask/utils'; - -import type { PermissionConstraint } from '../Permission'; -import type { SubjectPermissions } from '../PermissionController'; -import type { PermittedHandlerExport } from '../utils'; -import { MethodNames } from '../utils'; - -export const getPermissionsHandler: PermittedHandlerExport< - GetPermissionsHooks, - [], - PermissionConstraint[] -> = { - methodNames: [MethodNames.GetPermissions], - implementation: getPermissionsImplementation, - hookNames: { - getPermissionsForOrigin: true, - }, -}; - -export type GetPermissionsHooks = { - // This must be bound to the requesting origin. - getPermissionsForOrigin: () => SubjectPermissions; -}; - -/** - * Get Permissions implementation to be used in JsonRpcEngine middleware. - * - * @param _req - The JsonRpcEngine request - unused - * @param res - The JsonRpcEngine result object - * @param _next - JsonRpcEngine next() callback - unused - * @param end - JsonRpcEngine end() callback - * @param options - Method hooks passed to the method implementation - * @param options.getPermissionsForOrigin - The specific method hook needed for this method implementation - * @returns A promise that resolves to nothing - */ -async function getPermissionsImplementation( - _req: unknown, - res: PendingJsonRpcResponse, - _next: unknown, - end: JsonRpcEngineEndCallback, - { getPermissionsForOrigin }: GetPermissionsHooks, -): Promise { - res.result = Object.values(getPermissionsForOrigin() || {}); - return end(); -} diff --git a/packages/permission-controller/src/rpc-methods/index.ts b/packages/permission-controller/src/rpc-methods/index.ts deleted file mode 100644 index 7f13a06cd31..00000000000 --- a/packages/permission-controller/src/rpc-methods/index.ts +++ /dev/null @@ -1,16 +0,0 @@ -import type { GetPermissionsHooks } from './getPermissions'; -import { getPermissionsHandler } from './getPermissions'; -import type { RequestPermissionsHooks } from './requestPermissions'; -import { requestPermissionsHandler } from './requestPermissions'; -import type { RevokePermissionsHooks } from './revokePermissions'; -import { revokePermissionsHandler } from './revokePermissions'; - -export type PermittedRpcMethodHooks = RequestPermissionsHooks & - GetPermissionsHooks & - RevokePermissionsHooks; - -export const handlers = [ - requestPermissionsHandler, - getPermissionsHandler, - revokePermissionsHandler, -] as const; diff --git a/packages/permission-controller/src/rpc-methods/requestPermissions.test.ts b/packages/permission-controller/src/rpc-methods/requestPermissions.test.ts deleted file mode 100644 index 4422ed67a7e..00000000000 --- a/packages/permission-controller/src/rpc-methods/requestPermissions.test.ts +++ /dev/null @@ -1,143 +0,0 @@ -import { - JsonRpcEngine, - createAsyncMiddleware, -} from '@metamask/json-rpc-engine'; -import { rpcErrors, serializeError } from '@metamask/rpc-errors'; -import { - assertIsJsonRpcFailure, - assertIsJsonRpcSuccess, - hasProperty, -} from '@metamask/utils'; -import type { - PermissionConstraint, - RequestedPermissions, -} from 'src/Permission'; - -import { requestPermissionsHandler } from './requestPermissions'; - -describe('requestPermissions RPC method', () => { - it('returns the values of the object returned by requestPermissionsForOrigin', async () => { - const { implementation } = requestPermissionsHandler; - const mockRequestPermissionsForOrigin = jest - .fn() - .mockImplementationOnce(() => { - // Resolve this promise after a timeout to ensure that the function - // is awaited properly. - return new Promise((resolve) => { - setTimeout(() => { - resolve([{ a: 'a', b: 'b', c: 'c' }]); - }, 10); - }); - }); - - const engine = new JsonRpcEngine(); - engine.push<[RequestedPermissions], PermissionConstraint[]>( - (req, res, next, end) => { - // We intentionally do not await this promise; JsonRpcEngine won't await - // middleware anyway. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - implementation(req, res, next, end, { - requestPermissionsForOrigin: mockRequestPermissionsForOrigin, - }); - }, - ); - - const response = await engine.handle({ - jsonrpc: '2.0', - id: 1, - method: 'arbitraryName', - params: [{}], - }); - assertIsJsonRpcSuccess(response); - - expect(response.result).toStrictEqual(['a', 'b', 'c']); - expect(mockRequestPermissionsForOrigin).toHaveBeenCalledTimes(1); - expect(mockRequestPermissionsForOrigin).toHaveBeenCalledWith({}); - }); - - it('returns an error if requestPermissionsForOrigin rejects', async () => { - const { implementation } = requestPermissionsHandler; - const mockRequestPermissionsForOrigin = jest - .fn() - .mockImplementationOnce(async () => { - throw new Error('foo'); - }); - - const engine = new JsonRpcEngine(); - const end = (): undefined => undefined; // this won't be called - - // Pass the middleware function to createAsyncMiddleware so the error - // is catched. - engine.push<[RequestedPermissions], PermissionConstraint[]>( - createAsyncMiddleware(async (req, res, next) => - // This promise will be awaited by the createAsyncMiddleware wrapper. - // eslint-disable-next-line @typescript-eslint/no-misused-promises - implementation(req, res, next, end, { - requestPermissionsForOrigin: mockRequestPermissionsForOrigin, - }), - ), - ); - - const response = await engine.handle({ - jsonrpc: '2.0', - id: 1, - method: 'arbitraryName', - params: [{}], - }); - assertIsJsonRpcFailure(response); - - expect(hasProperty(response, 'result')).toBe(false); - delete response.error.stack; - // @ts-expect-error We do expect this property to exist. - delete response.error.data.cause.stack; - const expectedError = new Error('foo'); - delete expectedError.stack; - expect(response.error).toStrictEqual( - serializeError(expectedError, { shouldIncludeStack: false }), - ); - expect(mockRequestPermissionsForOrigin).toHaveBeenCalledTimes(1); - expect(mockRequestPermissionsForOrigin).toHaveBeenCalledWith({}); - }); - - it('returns an error if the request params are invalid', async () => { - const { implementation } = requestPermissionsHandler; - const mockRequestPermissionsForOrigin = jest.fn(); - - const engine = new JsonRpcEngine(); - engine.push<[RequestedPermissions], PermissionConstraint[]>( - (req, res, next, end) => { - // We intentionally do not await this promise; JsonRpcEngine won't await - // middleware anyway. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - implementation(req, res, next, end, { - requestPermissionsForOrigin: mockRequestPermissionsForOrigin, - }); - }, - ); - - for (const invalidParams of ['foo', ['bar']]) { - const request = { - jsonrpc: '2.0', - id: 1, - method: 'arbitraryName', - params: invalidParams, - }; - - const expectedError = rpcErrors - .invalidParams({ - data: { request: { ...request } }, - }) - .serialize(); - delete expectedError.stack; - - // @ts-expect-error Intentional destructive testing - // ESLint is confused; this signature is async. - // eslint-disable-next-line @typescript-eslint/await-thenable - const response = await engine.handle(request); - assertIsJsonRpcFailure(response); - delete response.error.stack; - expect(response.error).toStrictEqual(expectedError); - expect(mockRequestPermissionsForOrigin).not.toHaveBeenCalled(); - } - }); -}); diff --git a/packages/permission-controller/src/rpc-methods/requestPermissions.ts b/packages/permission-controller/src/rpc-methods/requestPermissions.ts deleted file mode 100644 index ae77a2dfae5..00000000000 --- a/packages/permission-controller/src/rpc-methods/requestPermissions.ts +++ /dev/null @@ -1,63 +0,0 @@ -import { isPlainObject } from '@metamask/controller-utils'; -import type { JsonRpcEngineEndCallback } from '@metamask/json-rpc-engine'; -import type { JsonRpcRequest, PendingJsonRpcResponse } from '@metamask/utils'; - -import { invalidParams } from '../errors'; -import type { PermissionConstraint, RequestedPermissions } from '../Permission'; -import type { PermittedHandlerExport } from '../utils'; -import { MethodNames } from '../utils'; - -export const requestPermissionsHandler: PermittedHandlerExport< - RequestPermissionsHooks, - [RequestedPermissions], - PermissionConstraint[] -> = { - methodNames: [MethodNames.RequestPermissions], - implementation: requestPermissionsImplementation, - hookNames: { - requestPermissionsForOrigin: true, - }, -}; - -type RequestPermissions = ( - requestedPermissions: RequestedPermissions, -) => Promise< - [Record, { id: string; origin: string }] ->; - -export type RequestPermissionsHooks = { - requestPermissionsForOrigin: RequestPermissions; -}; - -/** - * Request Permissions implementation to be used in JsonRpcEngine middleware. - * - * @param req - The JsonRpcEngine request - * @param res - The JsonRpcEngine result object - * @param _next - JsonRpcEngine next() callback - unused - * @param end - JsonRpcEngine end() callback - * @param options - Method hooks passed to the method implementation - * @param options.requestPermissionsForOrigin - The specific method hook needed for this method implementation - * @returns A promise that resolves to nothing - */ -async function requestPermissionsImplementation( - req: JsonRpcRequest<[RequestedPermissions]>, - res: PendingJsonRpcResponse, - _next: unknown, - end: JsonRpcEngineEndCallback, - { requestPermissionsForOrigin }: RequestPermissionsHooks, -): Promise { - const { params } = req; - - if (!Array.isArray(params) || !isPlainObject(params[0])) { - return end(invalidParams({ data: { request: req } })); - } - - const [requestedPermissions] = params; - const [grantedPermissions] = - await requestPermissionsForOrigin(requestedPermissions); - - // `wallet_requestPermission` is specified to return an array. - res.result = Object.values(grantedPermissions); - return end(); -} diff --git a/packages/permission-controller/src/rpc-methods/revokePermissions.test.ts b/packages/permission-controller/src/rpc-methods/revokePermissions.test.ts deleted file mode 100644 index 60465a1b475..00000000000 --- a/packages/permission-controller/src/rpc-methods/revokePermissions.test.ts +++ /dev/null @@ -1,196 +0,0 @@ -import { JsonRpcEngine } from '@metamask/json-rpc-engine'; -import { rpcErrors } from '@metamask/rpc-errors'; -import type { Json, JsonRpcRequest } from '@metamask/utils'; -import { - assertIsJsonRpcFailure, - assertIsJsonRpcSuccess, -} from '@metamask/utils'; - -import type { RevokePermissionArgs } from './revokePermissions'; -import { revokePermissionsHandler } from './revokePermissions'; - -describe('revokePermissionsHandler', () => { - it('has the expected shape', () => { - expect(revokePermissionsHandler).toStrictEqual({ - methodNames: ['wallet_revokePermissions'], - implementation: expect.any(Function), - hookNames: { - revokePermissionsForOrigin: true, - }, - }); - }); -}); - -describe('revokePermissions RPC method', () => { - it('revokes permissions using revokePermissionsForOrigin', async () => { - const { implementation } = revokePermissionsHandler; - const mockRevokePermissionsForOrigin = jest.fn(); - - const engine = new JsonRpcEngine(); - engine.push((req, res, next, end) => { - // We intentionally do not await this promise; JsonRpcEngine won't await - // middleware anyway. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - implementation(req, res, next, end, { - revokePermissionsForOrigin: mockRevokePermissionsForOrigin, - }); - }); - - const response = await engine.handle({ - jsonrpc: '2.0', - id: 1, - method: 'wallet_revokePermissions', - params: [ - { - snap_dialog: {}, - }, - ], - }); - assertIsJsonRpcSuccess(response); - - expect(response.result).toBeNull(); - expect(mockRevokePermissionsForOrigin).toHaveBeenCalledTimes(1); - expect(mockRevokePermissionsForOrigin).toHaveBeenCalledWith([ - 'snap_dialog', - ]); - }); - - it('returns an error if the request params is a plain object', async () => { - const { implementation } = revokePermissionsHandler; - const mockRevokePermissionsForOrigin = jest.fn(); - - const engine = new JsonRpcEngine(); - engine.push((req, res, next, end) => { - // We intentionally do not await this promise; JsonRpcEngine won't await - // middleware anyway. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - implementation(req, res, next, end, { - revokePermissionsForOrigin: mockRevokePermissionsForOrigin, - }); - }); - - const req: JsonRpcRequest> = { - jsonrpc: '2.0', - id: 1, - method: 'wallet_revokePermissions', - params: {}, - }; - - const expectedError = rpcErrors - .invalidParams({ - data: { request: { ...req } }, - }) - .serialize(); - delete expectedError.stack; - - const response = await engine.handle(req); - assertIsJsonRpcFailure(response); - delete response.error.stack; - expect(response.error).toStrictEqual(expectedError); - expect(mockRevokePermissionsForOrigin).not.toHaveBeenCalled(); - }); - - it('returns an error if the permissionKeys is a plain object', async () => { - const { implementation } = revokePermissionsHandler; - const mockRevokePermissionsForOrigin = jest.fn(); - - const engine = new JsonRpcEngine(); - engine.push((req, res, next, end) => { - // We intentionally do not await this promise; JsonRpcEngine won't await - // middleware anyway. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - implementation(req, res, next, end, { - revokePermissionsForOrigin: mockRevokePermissionsForOrigin, - }); - }); - - const req: JsonRpcRequest<[Record]> = { - jsonrpc: '2.0', - id: 1, - method: 'wallet_revokePermissions', - params: [{}], - }; - - const expectedError = rpcErrors - .invalidParams({ - data: { request: { ...req } }, - }) - .serialize(); - delete expectedError.stack; - - const response = await engine.handle(req); - assertIsJsonRpcFailure(response); - delete response.error.stack; - expect(response.error).toStrictEqual(expectedError); - expect(mockRevokePermissionsForOrigin).not.toHaveBeenCalled(); - }); - - it('returns an error if the params are not set', async () => { - const { implementation } = revokePermissionsHandler; - const mockRevokePermissionsForOrigin = jest.fn(); - - const engine = new JsonRpcEngine(); - engine.push((req, res, next, end) => { - // We intentionally do not await this promise; JsonRpcEngine won't await - // middleware anyway. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - implementation(req, res, next, end, { - revokePermissionsForOrigin: mockRevokePermissionsForOrigin, - }); - }); - - const req: JsonRpcRequest<[]> = { - jsonrpc: '2.0', - id: 1, - method: 'wallet_revokePermissions', - }; - - const expectedError = rpcErrors - .invalidParams({ - data: { request: { ...req } }, - }) - .serialize(); - delete expectedError.stack; - - const response = await engine.handle(req); - assertIsJsonRpcFailure(response); - delete response.error.stack; - expect(response.error).toStrictEqual(expectedError); - expect(mockRevokePermissionsForOrigin).not.toHaveBeenCalled(); - }); - - it('returns an error if the request params is an empty array', async () => { - const { implementation } = revokePermissionsHandler; - const mockRevokePermissionsForOrigin = jest.fn(); - - const engine = new JsonRpcEngine(); - engine.push((req, res, next, end) => { - // We intentionally do not await this promise; JsonRpcEngine won't await - // middleware anyway. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - implementation(req, res, next, end, { - revokePermissionsForOrigin: mockRevokePermissionsForOrigin, - }); - }); - - const req: JsonRpcRequest = { - jsonrpc: '2.0', - id: 1, - method: 'wallet_revokePermissions', - params: [], - }; - - const expectedError = rpcErrors - .invalidParams({ - data: { request: { ...req } }, - }) - .serialize(); - delete expectedError.stack; - - const response = await engine.handle(req); - assertIsJsonRpcFailure(response); - delete response.error.stack; - expect(response.error).toStrictEqual(expectedError); - expect(mockRevokePermissionsForOrigin).not.toHaveBeenCalled(); - }); -}); diff --git a/packages/permission-controller/src/rpc-methods/revokePermissions.ts b/packages/permission-controller/src/rpc-methods/revokePermissions.ts deleted file mode 100644 index 34b9b6352ef..00000000000 --- a/packages/permission-controller/src/rpc-methods/revokePermissions.ts +++ /dev/null @@ -1,79 +0,0 @@ -import type { JsonRpcEngineEndCallback } from '@metamask/json-rpc-engine'; -import { isNonEmptyArray } from '@metamask/utils'; -import type { - Json, - JsonRpcRequest, - NonEmptyArray, - PendingJsonRpcResponse, -} from '@metamask/utils'; - -import { invalidParams } from '../errors'; -import type { PermissionConstraint } from '../Permission'; -import type { PermittedHandlerExport } from '../utils'; -import { MethodNames } from '../utils'; - -export const revokePermissionsHandler: PermittedHandlerExport< - RevokePermissionsHooks, - RevokePermissionArgs, - null -> = { - methodNames: [MethodNames.RevokePermissions], - implementation: revokePermissionsImplementation, - hookNames: { - revokePermissionsForOrigin: true, - }, -}; - -export type RevokePermissionArgs = Record< - PermissionConstraint['parentCapability'], - Json ->; - -type RevokePermissions = ( - permissions: NonEmptyArray, -) => void; - -export type RevokePermissionsHooks = { - revokePermissionsForOrigin: RevokePermissions; -}; - -/** - * Revoke Permissions implementation to be used in JsonRpcEngine middleware. - * - * @param req - The JsonRpcEngine request - * @param res - The JsonRpcEngine result object - * @param _next - JsonRpcEngine next() callback - unused - * @param end - JsonRpcEngine end() callback - * @param options - Method hooks passed to the method implementation - * @param options.revokePermissionsForOrigin - A hook that revokes given permission keys for an origin - * @returns A promise that resolves to nothing - */ -async function revokePermissionsImplementation( - req: JsonRpcRequest, - res: PendingJsonRpcResponse, - _next: unknown, - end: JsonRpcEngineEndCallback, - { revokePermissionsForOrigin }: RevokePermissionsHooks, -): Promise { - const { params } = req; - - const param = params?.[0]; - - if (!param) { - return end(invalidParams({ data: { request: req } })); - } - - // For now, this API revokes the entire permission key - // even if caveats are specified. - const permissionKeys = Object.keys(param); - - if (!isNonEmptyArray(permissionKeys)) { - return end(invalidParams({ data: { request: req } })); - } - - revokePermissionsForOrigin(permissionKeys); - - res.result = null; - - return end(); -} diff --git a/packages/permission-controller/src/utils.ts b/packages/permission-controller/src/utils.ts index e26ac47efe1..d576924ad6d 100644 --- a/packages/permission-controller/src/utils.ts +++ b/packages/permission-controller/src/utils.ts @@ -1,14 +1,3 @@ -import type { - JsonRpcEngineEndCallback, - JsonRpcEngineNextCallback, -} from '@metamask/json-rpc-engine'; -import type { - Json, - JsonRpcParams, - JsonRpcRequest, - PendingJsonRpcResponse, -} from '@metamask/utils'; - import type { CaveatConstraint, CaveatSpecificationConstraint, @@ -40,44 +29,6 @@ export type ExtractSpecifications< | PermissionSpecificationMap, > = SpecificationsMap[keyof SpecificationsMap]; -/** - * A middleware function for handling a permitted method. - */ -export type HandlerMiddlewareFunction< - Hooks, - Params extends JsonRpcParams, - Result extends Json, -> = ( - req: JsonRpcRequest, - res: PendingJsonRpcResponse, - next: JsonRpcEngineNextCallback, - end: JsonRpcEngineEndCallback, - hooks: Hooks, -) => void | Promise; - -/** - * We use a mapped object type in order to create a type that requires the - * presence of the names of all hooks for the given handler. - * This can then be used to select only the necessary hooks whenever a method - * is called for purposes of POLA. - */ -export type HookNames = { - [Property in keyof HookMap]: true; -}; - -/** - * A handler for a permitted method. - */ -export type PermittedHandlerExport< - Hooks, - Params extends JsonRpcParams, - Result extends Json, -> = { - implementation: HandlerMiddlewareFunction; - hookNames: HookNames; - methodNames: string[]; -}; - /** * Given two permission objects, computes 3 sets: * - The set of caveat pairs that are common to both permissions. From 3b9f35a9c6c8326f3973d85126abbd1072d54194 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Fri, 24 Apr 2026 14:31:50 -0700 Subject: [PATCH 2/4] feat(json-rpc-engine): consolidate legacy `createMethodMiddleware` Add `createMethodMiddlewareFactory` to the legacy (v1) surface, consolidating the near-identical `makeMethodMiddlewareMaker` implementations from metamask-extension and metamask-mobile. The new export is deprecated in favor of the v2 `createMethodMiddleware`. Extract `selectHooks` and `assertExpectedHooks` into a shared `hookUtils.ts` module used by both v1 and v2. v2 now performs the same strict missing/extraneous hook validation as v1 at middleware construction time. Co-Authored-By: Claude Opus 4.7 --- packages/json-rpc-engine/CHANGELOG.md | 2 + .../src/createMethodMiddleware.test.ts | 209 ++++++++++++++++++ .../src/createMethodMiddleware.ts | 142 ++++++++++++ packages/json-rpc-engine/src/hookUtils.ts | 63 ++++++ packages/json-rpc-engine/src/index.test.ts | 1 + packages/json-rpc-engine/src/index.ts | 7 + .../src/v2/createMethodMiddleware.test.ts | 3 +- .../src/v2/createMethodMiddleware.ts | 40 +--- packages/json-rpc-engine/src/v2/index.test.ts | 1 - packages/json-rpc-engine/src/v2/index.ts | 2 +- 10 files changed, 435 insertions(+), 35 deletions(-) create mode 100644 packages/json-rpc-engine/src/createMethodMiddleware.test.ts create mode 100644 packages/json-rpc-engine/src/createMethodMiddleware.ts create mode 100644 packages/json-rpc-engine/src/hookUtils.ts diff --git a/packages/json-rpc-engine/CHANGELOG.md b/packages/json-rpc-engine/CHANGELOG.md index e4b3e9433c4..6854484f9f6 100644 --- a/packages/json-rpc-engine/CHANGELOG.md +++ b/packages/json-rpc-engine/CHANGELOG.md @@ -12,6 +12,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add `createOriginMiddleware` utility to `v2` ([#8522](https://github.com/MetaMask/core/pull/8522)) - Add `createMethodMiddleware` utility to `v2` ([#8506](https://github.com/MetaMask/core/pull/8506)) - This utility allows JSON-RPC method implementations to use both the hooks pattern and the messenger. +- Add `createMethodMiddlewareFactory` consolidating the bespoke `makeMethodMiddlewareMaker` implementations from `metamask-extension` and `metamask-mobile`. + - Deprecated in favor of the v2 `createMethodMiddleware`. ## [10.2.4] diff --git a/packages/json-rpc-engine/src/createMethodMiddleware.test.ts b/packages/json-rpc-engine/src/createMethodMiddleware.test.ts new file mode 100644 index 00000000000..1b7519bb229 --- /dev/null +++ b/packages/json-rpc-engine/src/createMethodMiddleware.test.ts @@ -0,0 +1,209 @@ +import { + assertIsJsonRpcFailure, + assertIsJsonRpcSuccess, +} from '@metamask/utils'; +import type { Json, JsonRpcParams } from '@metamask/utils'; + +import { JsonRpcEngine, createMethodMiddlewareFactory } from '.'; +import type { MethodHandler } from './createMethodMiddleware'; + +type Hooks = { + hook1: () => number; + hook2: () => number; +}; + +const getHandler = (): MethodHandler => ({ + implementation: (req, res, _next, end, hooks): void => { + if (Array.isArray(req.params)) { + switch (req.params[0]) { + case 1: + res.result = hooks.hook1(); + break; + case 2: + res.result = hooks.hook2(); + break; + case 3: + return end(new Error('test error')); + case 4: + throw new Error('test error'); + case 5: + // eslint-disable-next-line @typescript-eslint/only-throw-error + throw 'foo'; + default: + throw new Error(`unexpected param "${String(req.params[0])}"`); + } + } + return end(); + }, + hookNames: { hook1: true, hook2: true }, + methodNames: ['method1', 'method2'], +}); + +const getDefaultHooks = (): Hooks => ({ + hook1: () => 42, + hook2: () => 99, +}); + +const method1 = 'method1'; + +describe('createMethodMiddlewareFactory', () => { + it('throws an error if a required hook is missing', () => { + const createMiddleware = createMethodMiddlewareFactory([getHandler()]); + const hooks = { hook1: () => 42 } as unknown as Hooks; + + expect(() => createMiddleware(hooks)).toThrow('Missing expected hooks'); + }); + + it('throws an error if an extraneous hook is provided', () => { + const createMiddleware = createMethodMiddlewareFactory([getHandler()]); + const hooks = { + ...getDefaultHooks(), + extraneousHook: () => 100, + } as unknown as Hooks; + + expect(() => createMiddleware(hooks)).toThrow('Received unexpected hooks'); + }); + + it('calls the handler for the matching method (uses hook1)', async () => { + const middleware = createMethodMiddlewareFactory([getHandler()])( + getDefaultHooks(), + ); + const engine = new JsonRpcEngine(); + engine.push(middleware); + + const response = await engine.handle({ + jsonrpc: '2.0', + id: 1, + method: method1, + params: [1], + }); + assertIsJsonRpcSuccess(response); + + expect(response.result).toBe(42); + }); + + it('calls the handler for the matching method (uses hook2)', async () => { + const middleware = createMethodMiddlewareFactory([getHandler()])( + getDefaultHooks(), + ); + const engine = new JsonRpcEngine(); + engine.push(middleware); + + const response = await engine.handle({ + jsonrpc: '2.0', + id: 1, + method: method1, + params: [2], + }); + assertIsJsonRpcSuccess(response); + + expect(response.result).toBe(99); + }); + + it('does not call the handler for a non-matching method', async () => { + const middleware = createMethodMiddlewareFactory([getHandler()])( + getDefaultHooks(), + ); + const engine = new JsonRpcEngine(); + engine.push(middleware); + + const response = await engine.handle({ + jsonrpc: '2.0', + id: 1, + method: 'nonMatchingMethod', + }); + assertIsJsonRpcFailure(response); + + expect(response.error).toMatchObject({ + message: expect.stringMatching( + /Response has no error or result for request/u, + ), + }); + }); + + it('handles errors returned by the implementation', async () => { + const middleware = createMethodMiddlewareFactory([getHandler()])( + getDefaultHooks(), + ); + const engine = new JsonRpcEngine(); + engine.push(middleware); + + const response = await engine.handle({ + jsonrpc: '2.0', + id: 1, + method: method1, + params: [3], + }); + assertIsJsonRpcFailure(response); + + expect(response.error.message).toBe('test error'); + expect( + (response.error.data as { cause: { message: string } }).cause.message, + ).toBe('test error'); + }); + + it('handles errors thrown by the implementation', async () => { + const middleware = createMethodMiddlewareFactory([getHandler()])( + getDefaultHooks(), + ); + const engine = new JsonRpcEngine(); + engine.push(middleware); + + const response = await engine.handle({ + jsonrpc: '2.0', + id: 1, + method: method1, + params: [4], + }); + assertIsJsonRpcFailure(response); + + expect(response.error.message).toBe('test error'); + expect( + (response.error.data as { cause: { message: string } }).cause.message, + ).toBe('test error'); + }); + + it('handles non-errors thrown by the implementation', async () => { + const middleware = createMethodMiddlewareFactory([getHandler()])( + getDefaultHooks(), + ); + const engine = new JsonRpcEngine(); + engine.push(middleware); + + const response = await engine.handle({ + jsonrpc: '2.0', + id: 1, + method: method1, + params: [5], + }); + assertIsJsonRpcFailure(response); + + expect(response.error).toMatchObject({ + message: 'Internal JSON-RPC error.', + data: 'foo', + }); + }); + + it('invokes onError when a handler throws', async () => { + const onError = jest.fn(); + const middleware = createMethodMiddlewareFactory([getHandler()], { + onError, + })(getDefaultHooks()); + const engine = new JsonRpcEngine(); + engine.push(middleware); + + const request = { + jsonrpc: '2.0' as const, + id: 1, + method: method1, + params: [4], + }; + await engine.handle(request); + + expect(onError).toHaveBeenCalledTimes(1); + const [error, receivedRequest] = onError.mock.calls[0]; + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toBe('test error'); + expect(receivedRequest).toMatchObject(request); + }); +}); diff --git a/packages/json-rpc-engine/src/createMethodMiddleware.ts b/packages/json-rpc-engine/src/createMethodMiddleware.ts new file mode 100644 index 00000000000..cee5a1f92ad --- /dev/null +++ b/packages/json-rpc-engine/src/createMethodMiddleware.ts @@ -0,0 +1,142 @@ +import { rpcErrors } from '@metamask/rpc-errors'; +import type { + Json, + JsonRpcParams, + JsonRpcRequest, + PendingJsonRpcResponse, +} from '@metamask/utils'; + +import { assertExpectedHooks, selectHooks } from './hookUtils'; +import type { + JsonRpcEngineEndCallback, + JsonRpcEngineNextCallback, + JsonRpcMiddleware, +} from './JsonRpcEngine'; + +/** + * A middleware function for handling a permitted method. + * + * @deprecated Use the v2 handler type from `./v2/createMethodMiddleware` instead. + */ +export type HandlerMiddlewareFunction< + Hooks, + Params extends JsonRpcParams, + Result extends Json, +> = ( + req: JsonRpcRequest, + res: PendingJsonRpcResponse, + next: JsonRpcEngineNextCallback, + end: JsonRpcEngineEndCallback, + hooks: Hooks, +) => void | Promise; + +/** + * We use a mapped object type in order to create a type that requires the + * presence of the names of all hooks for the given handler. + * This can then be used to select only the necessary hooks whenever a method + * is called for purposes of POLA. + * + * @deprecated Use the v2 handler type from `./v2/createMethodMiddleware` instead. + */ +export type HookNames = { + [Property in keyof HookMap]: true; +}; + +/** + * A handler for a permitted method. + * + * @deprecated Use the v2 `MethodHandler` from `./v2/createMethodMiddleware` instead. + */ +export type MethodHandler< + Hooks, + Params extends JsonRpcParams, + Result extends Json, +> = { + implementation: HandlerMiddlewareFunction; + hookNames: HookNames; + methodNames: string[]; +}; + +/** + * Options for {@link createMethodMiddlewareFactory}. + */ +export type CreateMethodMiddlewareFactoryOptions = { + /** + * Called when a handler throws, before the error is forwarded to `end`. + * Intended for logging; must not throw. + */ + onError?: (error: unknown, request: JsonRpcRequest) => void; +}; + +/** + * Creates a factory that produces a JSON-RPC method middleware from a set of + * handlers. The returned factory validates that the hooks it receives match + * exactly the union of hook names declared by the handlers (no missing, no + * extraneous), then returns a middleware that dispatches requests by method + * name. + * + * Consolidates the bespoke `makeMethodMiddlewareMaker` implementations from + * `metamask-extension` and `metamask-mobile`. + * + * @deprecated Use `createMethodMiddleware` from `./v2/createMethodMiddleware` instead. + * @param handlers - The method handlers the middleware should dispatch to. + * @param options - Optional configuration. + * @returns A function that takes the required hooks and returns a + * `JsonRpcMiddleware`. + */ +export function createMethodMiddlewareFactory( + handlers: MethodHandler[], + options: CreateMethodMiddlewareFactoryOptions = {}, +): (hooks: Hooks) => JsonRpcMiddleware { + const { onError } = options; + + const handlerMap = handlers.reduce< + Record> + >((map, handler) => { + for (const methodName of handler.methodNames) { + map[methodName] = handler; + } + return map; + }, {}); + + const expectedHookNames = new Set( + handlers.flatMap(({ hookNames }) => Object.getOwnPropertyNames(hookNames)), + ); + + return (hooks: Hooks) => { + assertExpectedHooks(hooks as Record, expectedHookNames); + + // eslint-disable-next-line @typescript-eslint/no-misused-promises + const middleware: JsonRpcMiddleware = async ( + req, + res, + next, + end, + ) => { + const handler = handlerMap[req.method]; + if (handler) { + const { implementation, hookNames } = handler; + try { + return await implementation( + req, + res, + next, + end, + selectHooks(hooks, hookNames) as Hooks, + ); + } catch (error) { + onError?.(error, req); + return end( + error instanceof Error + ? error + : rpcErrors.internal({ data: error as Json }), + ); + } + } + + return next(); + }; + + return middleware; + }; +} diff --git a/packages/json-rpc-engine/src/hookUtils.ts b/packages/json-rpc-engine/src/hookUtils.ts new file mode 100644 index 00000000000..8a63e84697c --- /dev/null +++ b/packages/json-rpc-engine/src/hookUtils.ts @@ -0,0 +1,63 @@ +import { hasProperty } from '@metamask/utils'; + +/** + * Returns the subset of the specified `hooks` that are included in the + * `hookNames` object. This is a Principle of Least Authority (POLA) measure + * to ensure that each RPC method implementation only has access to the + * API "hooks" it needs to do its job. + * + * @param hooks - The hooks to select from. + * @param hookNames - The names of the hooks to select. + * @returns The selected hooks, or `undefined` if `hookNames` is not provided. + * @template Hooks - The hooks to select from. + * @template HookName - The names of the hooks to select. + */ +export function selectHooks( + hooks: Hooks, + hookNames?: Record, +): Pick | undefined { + if (hookNames) { + return Object.keys(hookNames).reduce>>( + (subset, name) => { + const hookName = name as HookName; + subset[hookName] = hooks[hookName]; + return subset; + }, + {}, + ) as Pick; + } + return undefined; +} + +/** + * Asserts that `hooks` contains exactly the hook names in `expectedHookNames`. + * Throws on any missing hooks, then on any extraneous hooks. + * + * @param hooks - The hooks object to validate. + * @param expectedHookNames - The expected hook names. + */ +export function assertExpectedHooks( + hooks: Record, + expectedHookNames: Set, +): void { + const missingHookNames: string[] = []; + expectedHookNames.forEach((hookName) => { + if (!hasProperty(hooks, hookName)) { + missingHookNames.push(hookName); + } + }); + if (missingHookNames.length > 0) { + throw new Error( + `Missing expected hooks:\n\n${missingHookNames.join('\n')}\n`, + ); + } + + const extraneousHookNames = Object.getOwnPropertyNames(hooks).filter( + (hookName) => !expectedHookNames.has(hookName), + ); + if (extraneousHookNames.length > 0) { + throw new Error( + `Received unexpected hooks:\n\n${extraneousHookNames.join('\n')}\n`, + ); + } +} diff --git a/packages/json-rpc-engine/src/index.test.ts b/packages/json-rpc-engine/src/index.test.ts index ac4e8008419..e14c4c6e7d8 100644 --- a/packages/json-rpc-engine/src/index.test.ts +++ b/packages/json-rpc-engine/src/index.test.ts @@ -6,6 +6,7 @@ describe('@metamask/json-rpc-engine', () => { [ "asV2Middleware", "createAsyncMiddleware", + "createMethodMiddlewareFactory", "createScaffoldMiddleware", "getUniqueId", "createIdRemapMiddleware", diff --git a/packages/json-rpc-engine/src/index.ts b/packages/json-rpc-engine/src/index.ts index 57e69b8d09b..6a9b0df057d 100644 --- a/packages/json-rpc-engine/src/index.ts +++ b/packages/json-rpc-engine/src/index.ts @@ -4,6 +4,13 @@ export type { AsyncJsonrpcMiddleware, } from './createAsyncMiddleware'; export { createAsyncMiddleware } from './createAsyncMiddleware'; +export type { + CreateMethodMiddlewareFactoryOptions, + HandlerMiddlewareFunction, + HookNames, + MethodHandler, +} from './createMethodMiddleware'; +export { createMethodMiddlewareFactory } from './createMethodMiddleware'; export { createScaffoldMiddleware } from './createScaffoldMiddleware'; export { getUniqueId } from './getUniqueId'; export { createIdRemapMiddleware } from './idRemapMiddleware'; diff --git a/packages/json-rpc-engine/src/v2/createMethodMiddleware.test.ts b/packages/json-rpc-engine/src/v2/createMethodMiddleware.test.ts index 168744a3fec..ea280b27c96 100644 --- a/packages/json-rpc-engine/src/v2/createMethodMiddleware.test.ts +++ b/packages/json-rpc-engine/src/v2/createMethodMiddleware.test.ts @@ -6,13 +6,14 @@ import { MethodHandler, } from './createMethodMiddleware'; import { JsonRpcEngineV2 } from './JsonRpcEngineV2'; +import { JsonRpcRequest } from './utils'; type TestAction = { type: 'Example:TestAction'; handler: () => Promise; }; -function setup(): { engine: JsonRpcEngineV2 } { +function setup(): { engine: JsonRpcEngineV2 } { const getValueA = { hookNames: { testHook: true }, implementation: ({ hooks }): Promise => hooks.testHook(), diff --git a/packages/json-rpc-engine/src/v2/createMethodMiddleware.ts b/packages/json-rpc-engine/src/v2/createMethodMiddleware.ts index dfd87035a1d..c107ae92fb9 100644 --- a/packages/json-rpc-engine/src/v2/createMethodMiddleware.ts +++ b/packages/json-rpc-engine/src/v2/createMethodMiddleware.ts @@ -1,5 +1,6 @@ import { ActionConstraint, Messenger } from '@metamask/messenger'; +import { assertExpectedHooks, selectHooks } from '../hookUtils'; import { JsonRpcMiddleware, Next } from './JsonRpcEngineV2'; import { ContextConstraint } from './MiddlewareContext'; import { @@ -94,6 +95,13 @@ export function createMethodMiddleware< const { messenger: rootMessenger } = options; const allHooks = options.hooks as Record; + const expectedHookNames = new Set( + Object.values(options.handlers).flatMap((handler) => + handler.hookNames ? Object.getOwnPropertyNames(handler.hookNames) : [], + ), + ); + assertExpectedHooks(allHooks, expectedHookNames); + const handlers = Object.entries(options.handlers).reduce< Record >((accumulator, [handlerName, handler]) => { @@ -134,35 +142,3 @@ export function createMethodMiddleware< return implementation({ request, context, next, hooks, messenger }); }; } - -/** - * Returns the subset of the specified `hooks` that are included in the - * `hookNames` object. This is a Principle of Least Authority (POLA) measure - * to ensure that each RPC method implementation only has access to the - * API "hooks" it needs to do its job. - * - * @param hooks - The hooks to select from. - * @param hookNames - The names of the hooks to select. - * @returns The selected hooks. - * @template Hooks - The hooks to select from. - * @template HookName - The names of the hooks to select. - */ -export function selectHooks< - Hooks extends Record, - HookName extends keyof Hooks, ->( - hooks: Hooks, - hookNames?: Record, -): Pick | undefined { - if (hookNames) { - return Object.keys(hookNames).reduce>>( - (hookSubset, _hookName) => { - const hookName = _hookName as HookName; - hookSubset[hookName] = hooks[hookName]; - return hookSubset; - }, - {}, - ) as Pick; - } - return undefined; -} diff --git a/packages/json-rpc-engine/src/v2/index.test.ts b/packages/json-rpc-engine/src/v2/index.test.ts index a1e16b5ba66..6a05a07af6e 100644 --- a/packages/json-rpc-engine/src/v2/index.test.ts +++ b/packages/json-rpc-engine/src/v2/index.test.ts @@ -15,7 +15,6 @@ describe('@metamask/json-rpc-engine/v2', () => { "getUniqueId", "isNotification", "isRequest", - "selectHooks", ] `); }); diff --git a/packages/json-rpc-engine/src/v2/index.ts b/packages/json-rpc-engine/src/v2/index.ts index 9f0906a4139..9a937a60603 100644 --- a/packages/json-rpc-engine/src/v2/index.ts +++ b/packages/json-rpc-engine/src/v2/index.ts @@ -1,6 +1,6 @@ export { asLegacyMiddleware } from './asLegacyMiddleware'; export { getUniqueId } from '../getUniqueId'; -export { selectHooks, createMethodMiddleware } from './createMethodMiddleware'; +export { createMethodMiddleware } from './createMethodMiddleware'; export type { MethodHandler } from './createMethodMiddleware'; export { createOriginMiddleware } from './createOriginMiddleware'; export { createScaffoldMiddleware } from './createScaffoldMiddleware'; From b7faed3bf840ffaaadde8a64e73b0fd6a3905255 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Fri, 24 Apr 2026 15:10:58 -0700 Subject: [PATCH 3/4] refactor(json-rpc-engine)!: Support messenger handlers in legacy method middleware --- packages/json-rpc-engine/CHANGELOG.md | 1 + .../src/createMethodMiddleware.test.ts | 118 ++++++++++++--- .../src/createMethodMiddleware.ts | 135 +++++++++++++----- .../src/{hookUtils.ts => middlewareUtils.ts} | 40 ++++++ .../src/v2/createMethodMiddleware.ts | 25 ++-- 5 files changed, 249 insertions(+), 70 deletions(-) rename packages/json-rpc-engine/src/{hookUtils.ts => middlewareUtils.ts} (62%) diff --git a/packages/json-rpc-engine/CHANGELOG.md b/packages/json-rpc-engine/CHANGELOG.md index 6854484f9f6..e2598e7dca8 100644 --- a/packages/json-rpc-engine/CHANGELOG.md +++ b/packages/json-rpc-engine/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add `createMethodMiddleware` utility to `v2` ([#8506](https://github.com/MetaMask/core/pull/8506)) - This utility allows JSON-RPC method implementations to use both the hooks pattern and the messenger. - Add `createMethodMiddlewareFactory` consolidating the bespoke `makeMethodMiddlewareMaker` implementations from `metamask-extension` and `metamask-mobile`. + - Handlers may now declare `actionNames` and receive a delegated messenger as the sixth argument to `implementation`, mirroring the v2 `createMethodMiddleware`. - Deprecated in favor of the v2 `createMethodMiddleware`. ## [10.2.4] diff --git a/packages/json-rpc-engine/src/createMethodMiddleware.test.ts b/packages/json-rpc-engine/src/createMethodMiddleware.test.ts index 1b7519bb229..0e5516ecba5 100644 --- a/packages/json-rpc-engine/src/createMethodMiddleware.test.ts +++ b/packages/json-rpc-engine/src/createMethodMiddleware.test.ts @@ -1,8 +1,8 @@ +import { Messenger, MOCK_ANY_NAMESPACE } from '@metamask/messenger'; import { assertIsJsonRpcFailure, assertIsJsonRpcSuccess, } from '@metamask/utils'; -import type { Json, JsonRpcParams } from '@metamask/utils'; import { JsonRpcEngine, createMethodMiddlewareFactory } from '.'; import type { MethodHandler } from './createMethodMiddleware'; @@ -12,7 +12,7 @@ type Hooks = { hook2: () => number; }; -const getHandler = (): MethodHandler => ({ +const getHandler = (): MethodHandler => ({ implementation: (req, res, _next, end, hooks): void => { if (Array.isArray(req.params)) { switch (req.params[0]) { @@ -30,7 +30,9 @@ const getHandler = (): MethodHandler => ({ // eslint-disable-next-line @typescript-eslint/only-throw-error throw 'foo'; default: - throw new Error(`unexpected param "${String(req.params[0])}"`); + throw new Error( + `unexpected param "${JSON.stringify(req.params[0])}"`, + ); } } return end(); @@ -44,18 +46,25 @@ const getDefaultHooks = (): Hooks => ({ hook2: () => 99, }); +const getRootMessenger = (): Messenger => + new Messenger({ namespace: MOCK_ANY_NAMESPACE }); + const method1 = 'method1'; describe('createMethodMiddlewareFactory', () => { it('throws an error if a required hook is missing', () => { - const createMiddleware = createMethodMiddlewareFactory([getHandler()]); + const createMiddleware = createMethodMiddlewareFactory([getHandler()], { + messenger: getRootMessenger(), + }); const hooks = { hook1: () => 42 } as unknown as Hooks; expect(() => createMiddleware(hooks)).toThrow('Missing expected hooks'); }); it('throws an error if an extraneous hook is provided', () => { - const createMiddleware = createMethodMiddlewareFactory([getHandler()]); + const createMiddleware = createMethodMiddlewareFactory([getHandler()], { + messenger: getRootMessenger(), + }); const hooks = { ...getDefaultHooks(), extraneousHook: () => 100, @@ -65,9 +74,9 @@ describe('createMethodMiddlewareFactory', () => { }); it('calls the handler for the matching method (uses hook1)', async () => { - const middleware = createMethodMiddlewareFactory([getHandler()])( - getDefaultHooks(), - ); + const middleware = createMethodMiddlewareFactory([getHandler()], { + messenger: getRootMessenger(), + })(getDefaultHooks()); const engine = new JsonRpcEngine(); engine.push(middleware); @@ -83,9 +92,9 @@ describe('createMethodMiddlewareFactory', () => { }); it('calls the handler for the matching method (uses hook2)', async () => { - const middleware = createMethodMiddlewareFactory([getHandler()])( - getDefaultHooks(), - ); + const middleware = createMethodMiddlewareFactory([getHandler()], { + messenger: getRootMessenger(), + })(getDefaultHooks()); const engine = new JsonRpcEngine(); engine.push(middleware); @@ -101,9 +110,9 @@ describe('createMethodMiddlewareFactory', () => { }); it('does not call the handler for a non-matching method', async () => { - const middleware = createMethodMiddlewareFactory([getHandler()])( - getDefaultHooks(), - ); + const middleware = createMethodMiddlewareFactory([getHandler()], { + messenger: getRootMessenger(), + })(getDefaultHooks()); const engine = new JsonRpcEngine(); engine.push(middleware); @@ -122,9 +131,9 @@ describe('createMethodMiddlewareFactory', () => { }); it('handles errors returned by the implementation', async () => { - const middleware = createMethodMiddlewareFactory([getHandler()])( - getDefaultHooks(), - ); + const middleware = createMethodMiddlewareFactory([getHandler()], { + messenger: getRootMessenger(), + })(getDefaultHooks()); const engine = new JsonRpcEngine(); engine.push(middleware); @@ -143,9 +152,9 @@ describe('createMethodMiddlewareFactory', () => { }); it('handles errors thrown by the implementation', async () => { - const middleware = createMethodMiddlewareFactory([getHandler()])( - getDefaultHooks(), - ); + const middleware = createMethodMiddlewareFactory([getHandler()], { + messenger: getRootMessenger(), + })(getDefaultHooks()); const engine = new JsonRpcEngine(); engine.push(middleware); @@ -164,9 +173,9 @@ describe('createMethodMiddlewareFactory', () => { }); it('handles non-errors thrown by the implementation', async () => { - const middleware = createMethodMiddlewareFactory([getHandler()])( - getDefaultHooks(), - ); + const middleware = createMethodMiddlewareFactory([getHandler()], { + messenger: getRootMessenger(), + })(getDefaultHooks()); const engine = new JsonRpcEngine(); engine.push(middleware); @@ -187,6 +196,7 @@ describe('createMethodMiddlewareFactory', () => { it('invokes onError when a handler throws', async () => { const onError = jest.fn(); const middleware = createMethodMiddlewareFactory([getHandler()], { + messenger: getRootMessenger(), onError, })(getDefaultHooks()); const engine = new JsonRpcEngine(); @@ -206,4 +216,66 @@ describe('createMethodMiddlewareFactory', () => { expect((error as Error).message).toBe('test error'); expect(receivedRequest).toMatchObject(request); }); + + it('works when no hooks and no messenger are configured', async () => { + const handler: MethodHandler = { + implementation: (_req, res, _next, end) => { + res.result = 'no-deps'; + return end(); + }, + methodNames: ['noDeps'], + }; + + const middleware = createMethodMiddlewareFactory([handler])(); + const engine = new JsonRpcEngine(); + engine.push(middleware); + + const response = await engine.handle({ + jsonrpc: '2.0', + id: 1, + method: 'noDeps', + }); + assertIsJsonRpcSuccess(response); + + expect(response.result).toBe('no-deps'); + }); + + it('passes a delegated messenger to the handler', async () => { + type TestAction = { + type: 'Example:TestAction'; + handler: () => Promise; + }; + + const handler: MethodHandler = { + implementation: async (_req, res, _next, end, _hooks, messenger) => { + res.result = await messenger.call('Example:TestAction'); + return end(); + }, + methodNames: ['callAction'], + actionNames: ['Example:TestAction'], + }; + + const rootMessenger = new Messenger({ + namespace: MOCK_ANY_NAMESPACE, + }); + rootMessenger.registerActionHandler( + 'Example:TestAction', + async () => 'action-result', + ); + + const middleware = createMethodMiddlewareFactory([handler], { + messenger: rootMessenger, + })(); + const engine = new JsonRpcEngine(); + engine.push(middleware); + + const response = await engine.handle({ + jsonrpc: '2.0', + id: 1, + method: 'callAction', + }); + assertIsJsonRpcSuccess(response); + + expect(response.result).toBe('action-result'); + }); }); diff --git a/packages/json-rpc-engine/src/createMethodMiddleware.ts b/packages/json-rpc-engine/src/createMethodMiddleware.ts index cee5a1f92ad..1b7acdacbac 100644 --- a/packages/json-rpc-engine/src/createMethodMiddleware.ts +++ b/packages/json-rpc-engine/src/createMethodMiddleware.ts @@ -1,3 +1,5 @@ +import type { ActionConstraint } from '@metamask/messenger'; +import { Messenger } from '@metamask/messenger'; import { rpcErrors } from '@metamask/rpc-errors'; import type { Json, @@ -6,12 +8,16 @@ import type { PendingJsonRpcResponse, } from '@metamask/utils'; -import { assertExpectedHooks, selectHooks } from './hookUtils'; import type { JsonRpcEngineEndCallback, JsonRpcEngineNextCallback, JsonRpcMiddleware, } from './JsonRpcEngine'; +import { + assertExpectedHooks, + createHandlerMessenger, + selectHooks, +} from './middlewareUtils'; /** * A middleware function for handling a permitted method. @@ -19,15 +25,17 @@ import type { * @deprecated Use the v2 handler type from `./v2/createMethodMiddleware` instead. */ export type HandlerMiddlewareFunction< - Hooks, - Params extends JsonRpcParams, - Result extends Json, + Hooks extends Record = never, + MessengerActions extends ActionConstraint = never, + Params extends JsonRpcParams = JsonRpcParams, + Result extends Json = Json, > = ( req: JsonRpcRequest, res: PendingJsonRpcResponse, next: JsonRpcEngineNextCallback, end: JsonRpcEngineEndCallback, hooks: Hooks, + messenger: Messenger, ) => void | Promise; /** @@ -48,25 +56,54 @@ export type HookNames = { * @deprecated Use the v2 `MethodHandler` from `./v2/createMethodMiddleware` instead. */ export type MethodHandler< - Hooks, - Params extends JsonRpcParams, - Result extends Json, + Hooks extends Record = never, + MessengerActions extends ActionConstraint = never, + Params extends JsonRpcParams = JsonRpcParams, + Result extends Json = Json, > = { - implementation: HandlerMiddlewareFunction; - hookNames: HookNames; + implementation: HandlerMiddlewareFunction< + Hooks, + MessengerActions, + Params, + Result + >; methodNames: string[]; -}; +} & ([Hooks] extends [never] + ? { hookNames?: undefined } + : { hookNames: HookNames }) & + ([MessengerActions] extends [never] + ? { actionNames?: undefined } + : { actionNames: readonly MessengerActions['type'][] }); /** * Options for {@link createMethodMiddlewareFactory}. */ -export type CreateMethodMiddlewareFactoryOptions = { +export type CreateMethodMiddlewareFactoryOptions< + MessengerActions extends ActionConstraint = never, +> = { /** * Called when a handler throws, before the error is forwarded to `end`. * Intended for logging; must not throw. */ onError?: (error: unknown, request: JsonRpcRequest) => void; -}; +} & ([MessengerActions] extends [never] + ? { + /** + * The root messenger. A per-handler messenger, namespaced to each handler + * and delegated the actions the handler declared, is passed to the + * handler's `implementation` at call time. Optional when no handler + * declares any actions. + */ + messenger?: undefined; + } + : { + /** + * The root messenger. A per-handler messenger, namespaced to each handler + * and delegated the actions the handler declared, is passed to the + * handler's `implementation` at call time. + */ + messenger: Messenger; + }); /** * Creates a factory that produces a JSON-RPC method middleware from a set of @@ -80,31 +117,57 @@ export type CreateMethodMiddlewareFactoryOptions = { * * @deprecated Use `createMethodMiddleware` from `./v2/createMethodMiddleware` instead. * @param handlers - The method handlers the middleware should dispatch to. - * @param options - Optional configuration. + * @param options - Options including the root messenger. * @returns A function that takes the required hooks and returns a * `JsonRpcMiddleware`. */ -export function createMethodMiddlewareFactory( - handlers: MethodHandler[], - options: CreateMethodMiddlewareFactoryOptions = {}, -): (hooks: Hooks) => JsonRpcMiddleware { - const { onError } = options; +export function createMethodMiddlewareFactory< + Hooks extends Record = never, + MessengerActions extends ActionConstraint = never, +>( + handlers: MethodHandler[], + options?: CreateMethodMiddlewareFactoryOptions, +): [Hooks] extends [never] + ? () => JsonRpcMiddleware + : (hooks: Hooks) => JsonRpcMiddleware { + const { onError } = options ?? {}; + const rootMessenger = + options?.messenger ?? + new Messenger({ + namespace: 'json-rpc-engine', + }); - const handlerMap = handlers.reduce< - Record> - >((map, handler) => { - for (const methodName of handler.methodNames) { - map[methodName] = handler; - } - return map; - }, {}); + type HandlerEntry = { + handler: MethodHandler; + messenger: Messenger; + }; + + const handlerMap = handlers.reduce>( + (map, handler) => { + const handlerMessenger = createHandlerMessenger({ + namespace: handler.methodNames.join(':'), + actionNames: handler.actionNames, + rootMessenger, + }); + for (const methodName of handler.methodNames) { + map[methodName] = { handler, messenger: handlerMessenger }; + } + return map; + }, + {}, + ); const expectedHookNames = new Set( - handlers.flatMap(({ hookNames }) => Object.getOwnPropertyNames(hookNames)), + handlers.flatMap((handler) => + handler.hookNames ? Object.getOwnPropertyNames(handler.hookNames) : [], + ), ); - return (hooks: Hooks) => { - assertExpectedHooks(hooks as Record, expectedHookNames); + return ((hooks?: Hooks) => { + assertExpectedHooks( + (hooks ?? {}) as Record, + expectedHookNames, + ); // eslint-disable-next-line @typescript-eslint/no-misused-promises const middleware: JsonRpcMiddleware = async ( @@ -113,9 +176,12 @@ export function createMethodMiddlewareFactory( next, end, ) => { - const handler = handlerMap[req.method]; - if (handler) { - const { implementation, hookNames } = handler; + const entry = handlerMap[req.method]; + if (entry) { + const { + handler: { implementation, hookNames }, + messenger, + } = entry; try { return await implementation( req, @@ -123,6 +189,7 @@ export function createMethodMiddlewareFactory( next, end, selectHooks(hooks, hookNames) as Hooks, + messenger, ); } catch (error) { onError?.(error, req); @@ -138,5 +205,7 @@ export function createMethodMiddlewareFactory( }; return middleware; - }; + }) as [Hooks] extends [never] + ? () => JsonRpcMiddleware + : (hooks: Hooks) => JsonRpcMiddleware; } diff --git a/packages/json-rpc-engine/src/hookUtils.ts b/packages/json-rpc-engine/src/middlewareUtils.ts similarity index 62% rename from packages/json-rpc-engine/src/hookUtils.ts rename to packages/json-rpc-engine/src/middlewareUtils.ts index 8a63e84697c..7f9230e80e9 100644 --- a/packages/json-rpc-engine/src/hookUtils.ts +++ b/packages/json-rpc-engine/src/middlewareUtils.ts @@ -1,3 +1,5 @@ +import type { ActionConstraint } from '@metamask/messenger'; +import { Messenger } from '@metamask/messenger'; import { hasProperty } from '@metamask/utils'; /** @@ -61,3 +63,41 @@ export function assertExpectedHooks( ); } } + +/** + * Creates a per-handler messenger namespaced to `namespace`, and delegates the + * specified `actionNames` from `rootMessenger` to it. This lets each handler + * call only the actions it declared, per POLA. + * + * @param options - The options. + * @param options.namespace - The namespace for the handler messenger. + * @param options.actionNames - Actions to delegate from the root messenger. + * @param options.rootMessenger - The root messenger to delegate from. + * @returns The per-handler messenger. + */ +export function createHandlerMessenger({ + namespace, + actionNames, + rootMessenger, +}: { + namespace: string; + actionNames: readonly Actions['type'][] | undefined; + rootMessenger: Messenger; +}): Messenger { + const handlerMessenger = new Messenger< + string, + Actions, + never, + typeof rootMessenger + >({ + namespace, + parent: rootMessenger, + }); + + rootMessenger.delegate({ + actions: (actionNames ?? []) as Actions['type'][], + messenger: handlerMessenger, + }); + + return handlerMessenger; +} diff --git a/packages/json-rpc-engine/src/v2/createMethodMiddleware.ts b/packages/json-rpc-engine/src/v2/createMethodMiddleware.ts index c107ae92fb9..47bdebdae81 100644 --- a/packages/json-rpc-engine/src/v2/createMethodMiddleware.ts +++ b/packages/json-rpc-engine/src/v2/createMethodMiddleware.ts @@ -1,6 +1,10 @@ import { ActionConstraint, Messenger } from '@metamask/messenger'; -import { assertExpectedHooks, selectHooks } from '../hookUtils'; +import { + assertExpectedHooks, + createHandlerMessenger, + selectHooks, +} from '../middlewareUtils'; import { JsonRpcMiddleware, Next } from './JsonRpcEngineV2'; import { ContextConstraint } from './MiddlewareContext'; import { @@ -106,21 +110,14 @@ export function createMethodMiddleware< Record >((accumulator, [handlerName, handler]) => { const handlerHooks = selectHooks(allHooks, handler.hookNames) ?? {}; - const handlerMessenger = new Messenger< - string, - HandlerActions, - never, - typeof rootMessenger + const handlerMessenger = createHandlerMessenger< + HandlerActions >({ namespace: handlerName, - parent: rootMessenger, - }); - - rootMessenger.delegate({ - actions: (handler.actionNames ?? []) as HandlerActions< - Handlers[keyof Handlers] - >['type'][], - messenger: handlerMessenger, + actionNames: handler.actionNames as + | readonly HandlerActions['type'][] + | undefined, + rootMessenger, }); accumulator[handlerName] = { From be9aaca0938b0519170e6f8221f17ee3e32b4fc8 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Fri, 24 Apr 2026 15:17:27 -0700 Subject: [PATCH 4/4] chore: Update changelogs --- packages/json-rpc-engine/CHANGELOG.md | 3 ++- packages/permission-controller/CHANGELOG.md | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/json-rpc-engine/CHANGELOG.md b/packages/json-rpc-engine/CHANGELOG.md index e2598e7dca8..96265465a8d 100644 --- a/packages/json-rpc-engine/CHANGELOG.md +++ b/packages/json-rpc-engine/CHANGELOG.md @@ -12,7 +12,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add `createOriginMiddleware` utility to `v2` ([#8522](https://github.com/MetaMask/core/pull/8522)) - Add `createMethodMiddleware` utility to `v2` ([#8506](https://github.com/MetaMask/core/pull/8506)) - This utility allows JSON-RPC method implementations to use both the hooks pattern and the messenger. -- Add `createMethodMiddlewareFactory` consolidating the bespoke `makeMethodMiddlewareMaker` implementations from `metamask-extension` and `metamask-mobile`. +- Add legacy `createMethodMiddlewareFactory` ([#8583](https://github.com/MetaMask/core/pull/8583)) + - Consolidates bespoke `makeMethodMiddlewareMaker` implementations from the MetaMask extension and mobile clients. - Handlers may now declare `actionNames` and receive a delegated messenger as the sixth argument to `implementation`, mirroring the v2 `createMethodMiddleware`. - Deprecated in favor of the v2 `createMethodMiddleware`. diff --git a/packages/permission-controller/CHANGELOG.md b/packages/permission-controller/CHANGELOG.md index c9e6f6ab06a..a91b108a3a1 100644 --- a/packages/permission-controller/CHANGELOG.md +++ b/packages/permission-controller/CHANGELOG.md @@ -30,6 +30,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Removed - **BREAKING:** Remove `factoryHooks`, `validatorHooks`, and related fields from permission specification builders ([#8551](https://github.com/MetaMask/core/pull/8551)) +- **BREAKING:** Remove permitted method handlers and types ([#8583](https://github.com/MetaMask/core/pull/8583)) + - The permitted method handlers were unused in practice. Replacement types are available in `@metamask/json-rpc-engine@10.3.0`. ## [12.3.0]