Skip to content

Commit 84c2d21

Browse files
refactor(snaps-rpc-methods)!: Use messenger for permitted methods (#3987)
Migrate the permitted method handlers to use the `messenger` where applicable. https://consensyssoftware.atlassian.net/browse/WPC-995 <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Refactors many permitted RPC handlers to route through controller `messenger.call` actions instead of direct hook functions, which can subtly change permission checks, origin handling, and side effects across core Snap RPC methods. > > **Overview** > **Permitted Snap RPC methods are migrated from hook-based implementations to `messenger`-driven controller actions.** Handlers now declare `actionNames`, accept `JsonRpcRequestWithOrigin`, and call into actions like `PermissionController:hasPermission`, `SnapController:*`, `CronjobController:*`, `WebSocketService:*`, `KeyringController:getState`, and `SnapInterfaceController:*`. > > This removes the exported permitted `handlers` list and updates schema generation to read permitted methods from `src/permitted/middleware.ts` instead of the deleted `handlers.ts`, with associated test updates to use `MockControllerMessenger` and origin middleware. > > Also tightens the `get-file` example by asserting `snap_getFile` returns content before `JSON.parse`, updates its manifest shasum, and adjusts Jest coverage thresholds slightly. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 399e36b. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 92ace80 commit 84c2d21

73 files changed

Lines changed: 3750 additions & 3339 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

packages/examples/packages/get-file/snap.manifest.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
"url": "https://github.com/MetaMask/snaps.git"
88
},
99
"source": {
10-
"shasum": "YEIejIfr+8rWTP0dSPc4HcLO1JwM/QQCwA/BItw5goE=",
10+
"shasum": "HwhCRDjZxqIRT9rbHCNT11CoQ/YS8tO7ZFXrjxllpZ0=",
1111
"location": {
1212
"npm": {
1313
"filePath": "dist/bundle.js",

packages/examples/packages/get-file/src/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import {
22
MethodNotFoundError,
3+
assert,
34
type OnRpcRequestHandler,
45
} from '@metamask/snaps-sdk';
56

@@ -28,6 +29,7 @@ export const onRpcRequest: OnRpcRequestHandler = async ({ request }) => {
2829
method: 'snap_getFile',
2930
params: { path: './files/foo.json', encoding: 'utf8' },
3031
});
32+
assert(fileInPlaintext);
3133
return JSON.parse(fileInPlaintext);
3234
}
3335

packages/snaps-rpc-methods/jest.config.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,10 @@ module.exports = deepmerge(baseConfig, {
1010
],
1111
coverageThreshold: {
1212
global: {
13-
branches: 97.29,
14-
functions: 98.85,
15-
lines: 99.14,
16-
statements: 98.81,
13+
branches: 97.28,
14+
functions: 98.88,
15+
lines: 99.19,
16+
statements: 98.91,
1717
},
1818
},
1919
});

packages/snaps-rpc-methods/scripts/generate-schema.mts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1294,7 +1294,7 @@ async function processPermittedHandler(
12941294
}
12951295

12961296
/**
1297-
* Process the permitted handlers defined in `src/permitted/handlers.ts`,
1297+
* Process the permitted handlers defined in `src/permitted/middleware.ts`,
12981298
* extracting the method names, descriptions, parameters, return types, and
12991299
* subject types for each handler.
13001300
*
@@ -1303,7 +1303,7 @@ async function processPermittedHandler(
13031303
* @returns An array of method schemas extracted from the permitted handlers.
13041304
*/
13051305
async function processPermittedHandlers(project: Project) {
1306-
const handlersFile = project.getSourceFile('src/permitted/handlers.ts');
1306+
const handlersFile = project.getSourceFile('src/permitted/middleware.ts');
13071307
assert(handlersFile, 'Handlers file not found.');
13081308

13091309
const permittedHandlers =

packages/snaps-rpc-methods/src/index.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,6 @@
1-
export {
2-
handlers as permittedMethods,
3-
createSnapsMethodMiddleware,
4-
} from './permitted';
1+
export { createSnapsMethodMiddleware } from './permitted';
52
export type { PermittedRpcMethodHooks } from './permitted';
63
export { SnapCaveatType } from '@metamask/snaps-utils';
7-
export { selectHooks } from './utils';
84
export * from './endowments';
95
export * from './middleware';
106
export * from './permissions';

packages/snaps-rpc-methods/src/permitted/cancelBackgroundEvent.test.ts

Lines changed: 69 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,55 +1,74 @@
1-
import { JsonRpcEngine } from '@metamask/json-rpc-engine';
1+
import {
2+
JsonRpcEngine,
3+
createOriginMiddleware,
4+
} from '@metamask/json-rpc-engine';
25
import type {
36
CancelBackgroundEventParams,
47
CancelBackgroundEventResult,
8+
SnapId,
59
} from '@metamask/snaps-sdk';
6-
import { MOCK_SNAP_ID } from '@metamask/snaps-utils/test-utils';
10+
import {
11+
MOCK_SNAP_ID,
12+
MockControllerMessenger,
13+
} from '@metamask/snaps-utils/test-utils';
714
import type { JsonRpcRequest, PendingJsonRpcResponse } from '@metamask/utils';
815

16+
import type { CancelBackgroundEventMethodActions } from './cancelBackgroundEvent';
917
import { cancelBackgroundEventHandler } from './cancelBackgroundEvent';
1018

1119
describe('snap_cancelBackgroundEvent', () => {
1220
describe('cancelBackgroundEventHandler', () => {
1321
it('has the expected shape', () => {
1422
expect(cancelBackgroundEventHandler).toMatchObject({
15-
methodNames: ['snap_cancelBackgroundEvent'],
1623
implementation: expect.any(Function),
17-
hookNames: {
18-
cancelBackgroundEvent: true,
19-
},
24+
actionNames: [
25+
'PermissionController:hasPermission',
26+
'CronjobController:cancel',
27+
],
2028
});
2129
});
2230
});
2331

2432
describe('implementation', () => {
25-
const createOriginMiddleware =
26-
(origin: string) =>
27-
(request: any, _response: unknown, next: () => void, _end: unknown) => {
28-
request.origin = origin;
29-
next();
30-
};
31-
32-
it('returns null after calling the `cancelBackgroundEvent` hook', async () => {
33-
const { implementation } = cancelBackgroundEventHandler;
33+
const getMessenger = () => {
34+
const messenger = new MockControllerMessenger<
35+
CancelBackgroundEventMethodActions,
36+
never
37+
>();
38+
39+
messenger.registerActionHandler(
40+
'PermissionController:hasPermission',
41+
() => true,
42+
);
3443

35-
const cancelBackgroundEvent = jest.fn();
36-
const hasPermission = jest.fn().mockImplementation(() => true);
44+
messenger.registerActionHandler(
45+
'CronjobController:cancel',
46+
() => undefined,
47+
);
3748

38-
const hooks = {
39-
cancelBackgroundEvent,
40-
hasPermission,
41-
};
49+
jest.spyOn(messenger, 'call');
50+
51+
return messenger;
52+
};
53+
54+
it('returns null after calling the `CronjobController:cancel` action', async () => {
55+
const { implementation } = cancelBackgroundEventHandler;
56+
57+
const messenger = getMessenger();
4258

4359
const engine = new JsonRpcEngine();
4460

4561
engine.push(createOriginMiddleware(MOCK_SNAP_ID));
4662
engine.push((request, response, next, end) => {
4763
const result = implementation(
48-
request as JsonRpcRequest<CancelBackgroundEventParams>,
64+
request as JsonRpcRequest<CancelBackgroundEventParams> & {
65+
origin: SnapId;
66+
},
4967
response as PendingJsonRpcResponse<CancelBackgroundEventResult>,
5068
next,
5169
end,
52-
hooks,
70+
{} as never,
71+
messenger,
5372
);
5473

5574
result?.catch(end);
@@ -70,24 +89,21 @@ describe('snap_cancelBackgroundEvent', () => {
7089
it('cancels a background event', async () => {
7190
const { implementation } = cancelBackgroundEventHandler;
7291

73-
const cancelBackgroundEvent = jest.fn();
74-
const hasPermission = jest.fn().mockImplementation(() => true);
75-
76-
const hooks = {
77-
cancelBackgroundEvent,
78-
hasPermission,
79-
};
92+
const messenger = getMessenger();
8093

8194
const engine = new JsonRpcEngine();
8295

8396
engine.push(createOriginMiddleware(MOCK_SNAP_ID));
8497
engine.push((request, response, next, end) => {
8598
const result = implementation(
86-
request as JsonRpcRequest<CancelBackgroundEventParams>,
99+
request as JsonRpcRequest<CancelBackgroundEventParams> & {
100+
origin: SnapId;
101+
},
87102
response as PendingJsonRpcResponse<CancelBackgroundEventResult>,
88103
next,
89104
end,
90-
hooks,
105+
{} as never,
106+
messenger,
91107
);
92108

93109
result?.catch(end);
@@ -102,30 +118,36 @@ describe('snap_cancelBackgroundEvent', () => {
102118
},
103119
});
104120

105-
expect(cancelBackgroundEvent).toHaveBeenCalledWith('foo');
121+
expect(messenger.call).toHaveBeenCalledWith(
122+
'CronjobController:cancel',
123+
MOCK_SNAP_ID,
124+
'foo',
125+
);
106126
});
107127

108128
it('throws if a snap does not have the "endowment:cronjob" permission', async () => {
109129
const { implementation } = cancelBackgroundEventHandler;
110130

111-
const cancelBackgroundEvent = jest.fn();
112-
const hasPermission = jest.fn().mockImplementation(() => false);
131+
const messenger = getMessenger();
113132

114-
const hooks = {
115-
cancelBackgroundEvent,
116-
hasPermission,
117-
};
133+
messenger.registerActionHandler(
134+
'PermissionController:hasPermission',
135+
() => false,
136+
);
118137

119138
const engine = new JsonRpcEngine();
120139

121140
engine.push(createOriginMiddleware(MOCK_SNAP_ID));
122141
engine.push((request, response, next, end) => {
123142
const result = implementation(
124-
request as JsonRpcRequest<CancelBackgroundEventParams>,
143+
request as JsonRpcRequest<CancelBackgroundEventParams> & {
144+
origin: SnapId;
145+
},
125146
response as PendingJsonRpcResponse<CancelBackgroundEventResult>,
126147
next,
127148
end,
128-
hooks,
149+
{} as never,
150+
messenger,
129151
);
130152

131153
result?.catch(end);
@@ -155,24 +177,21 @@ describe('snap_cancelBackgroundEvent', () => {
155177
it('throws on invalid params', async () => {
156178
const { implementation } = cancelBackgroundEventHandler;
157179

158-
const cancelBackgroundEvent = jest.fn();
159-
const hasPermission = jest.fn().mockImplementation(() => true);
160-
161-
const hooks = {
162-
cancelBackgroundEvent,
163-
hasPermission,
164-
};
180+
const messenger = getMessenger();
165181

166182
const engine = new JsonRpcEngine();
167183

168184
engine.push(createOriginMiddleware(MOCK_SNAP_ID));
169185
engine.push((request, response, next, end) => {
170186
const result = implementation(
171-
request as JsonRpcRequest<CancelBackgroundEventParams>,
187+
request as JsonRpcRequest<CancelBackgroundEventParams> & {
188+
origin: SnapId;
189+
},
172190
response as PendingJsonRpcResponse<CancelBackgroundEventResult>,
173191
next,
174192
end,
175-
hooks,
193+
{} as never,
194+
messenger,
176195
);
177196

178197
result?.catch(end);

packages/snaps-rpc-methods/src/permitted/cancelBackgroundEvent.ts

Lines changed: 37 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,28 @@
1-
import type { JsonRpcEngineEndCallback } from '@metamask/json-rpc-engine';
1+
import type {
2+
JsonRpcEngineEndCallback,
3+
MethodHandler,
4+
} from '@metamask/json-rpc-engine';
5+
import type { Messenger } from '@metamask/messenger';
6+
import type { PermissionControllerHasPermissionAction } from '@metamask/permission-controller';
27
import { providerErrors, rpcErrors } from '@metamask/rpc-errors';
38
import type {
4-
JsonRpcRequest,
59
CancelBackgroundEventParams,
610
CancelBackgroundEventResult,
11+
SnapId,
712
} from '@metamask/snaps-sdk';
813
import { type InferMatching } from '@metamask/snaps-utils';
914
import { StructError, create, object, string } from '@metamask/superstruct';
1015
import { type PendingJsonRpcResponse } from '@metamask/utils';
1116

1217
import { SnapEndowments } from '../endowments';
13-
import type { PermittedHandlerExport } from '../types';
14-
import type { MethodHooksObject } from '../utils';
15-
16-
const methodName = 'snap_cancelBackgroundEvent';
17-
18-
const hookNames: MethodHooksObject<CancelBackgroundEventMethodHooks> = {
19-
cancelBackgroundEvent: true,
20-
hasPermission: true,
21-
};
18+
import type {
19+
CronjobControllerCancelAction,
20+
JsonRpcRequestWithOrigin,
21+
} from '../types';
2222

23-
export type CancelBackgroundEventMethodHooks = {
24-
cancelBackgroundEvent: (id: string) => void;
25-
hasPermission: (permissionName: string) => boolean;
26-
};
23+
export type CancelBackgroundEventMethodActions =
24+
| PermissionControllerHasPermissionAction
25+
| CronjobControllerCancelAction;
2726

2827
/**
2928
* Cancel a background event created by
@@ -46,13 +45,17 @@ export type CancelBackgroundEventMethodHooks = {
4645
* ```
4746
*/
4847
export const cancelBackgroundEventHandler = {
49-
methodNames: [methodName] as const,
5048
implementation: getCancelBackgroundEventImplementation,
51-
hookNames,
52-
} satisfies PermittedHandlerExport<
53-
CancelBackgroundEventMethodHooks,
49+
actionNames: [
50+
'PermissionController:hasPermission',
51+
'CronjobController:cancel',
52+
],
53+
} satisfies MethodHandler<
54+
never,
55+
CancelBackgroundEventMethodActions,
5456
CancelBackgroundEventParameters,
55-
CancelBackgroundEventResult
57+
CancelBackgroundEventResult,
58+
{ origin: SnapId }
5659
>;
5760

5861
const CancelBackgroundEventsParametersStruct = object({
@@ -72,21 +75,27 @@ export type CancelBackgroundEventParameters = InferMatching<
7275
* @param _next - The `json-rpc-engine` "next" callback. Not used by this
7376
* function.
7477
* @param end - The `json-rpc-engine` "end" callback.
75-
* @param hooks - The RPC method hooks.
76-
* @param hooks.cancelBackgroundEvent - The function to cancel a background event.
77-
* @param hooks.hasPermission - The function to check if a snap has the `endowment:cronjob` permission.
78+
* @param _hooks - The RPC method hooks. Not used by this function.
79+
* @param messenger - The messenger used to call controller actions.
7880
* @returns Nothing.
7981
*/
8082
async function getCancelBackgroundEventImplementation(
81-
req: JsonRpcRequest<CancelBackgroundEventParameters>,
83+
req: JsonRpcRequestWithOrigin<CancelBackgroundEventParameters>,
8284
res: PendingJsonRpcResponse<CancelBackgroundEventResult>,
8385
_next: unknown,
8486
end: JsonRpcEngineEndCallback,
85-
{ cancelBackgroundEvent, hasPermission }: CancelBackgroundEventMethodHooks,
87+
_hooks: never,
88+
messenger: Messenger<string, CancelBackgroundEventMethodActions>,
8689
): Promise<void> {
87-
const { params } = req;
90+
const { params, origin } = req;
8891

89-
if (!hasPermission(SnapEndowments.Cronjob)) {
92+
if (
93+
!messenger.call(
94+
'PermissionController:hasPermission',
95+
origin,
96+
SnapEndowments.Cronjob,
97+
)
98+
) {
9099
return end(providerErrors.unauthorized());
91100
}
92101

@@ -95,7 +104,7 @@ async function getCancelBackgroundEventImplementation(
95104

96105
const { id } = validatedParams;
97106

98-
cancelBackgroundEvent(id);
107+
messenger.call('CronjobController:cancel', origin, id);
99108
res.result = null;
100109
} catch (error) {
101110
return end(error);

0 commit comments

Comments
 (0)