refactor(snaps-rpc-methods)!: Use messenger for permitted methods by FrederikBolding · Pull Request #3987 · MetaMask/snaps

FrederikBolding · 2026-05-06T13:52:05Z

Migrate the permitted method handlers to use the messenger where applicable.

https://consensyssoftware.atlassian.net/browse/WPC-995

Note

Medium Risk
Refactors many permitted RPC method handlers to route permission checks and side effects through controller messenger.call actions, which could subtly change authorization, origin handling, or return types if any action wiring is incorrect.

Overview
Migrates permitted RPC method handlers in snaps-rpc-methods from hook-based implementations to messenger-driven controller actions, adding explicit actionNames and using messenger.call(...) for permission checks and operations like state access, cronjob/websocket management, UI interface operations, and Snap invocation.

Updates the permitted middleware/schema plumbing to source handlers from src/permitted/middleware.ts, adjusts public exports (dropping permittedMethods/selectHooks), and rewrites unit tests to inject origin via createOriginMiddleware plus MockControllerMessenger. Also includes small follow-ups: tighten snap_getFile example snap parsing with an assert, refresh the example manifest shasum, and bump Jest coverage thresholds.

^{Reviewed by Cursor Bugbot for commit 7bb3579. Bugbot is set up for automated code reviews on this repo. Configure here.}

FrederikBolding · 2026-05-06T13:53:00Z

-  /**
-   * @returns All installed Snaps.
-   */
-  getAllSnaps: () => Promise<GetSnapsResult>;


Turns out the types here have always been wrong

FrederikBolding · 2026-05-06T13:53:19Z

+ * file does not exist.
 */
-export type GetFileResult = string;
+export type GetFileResult = string | null;


This has always been possible, but the type was wrong.

FrederikBolding · 2026-05-07T11:05:46Z

+import { updateInterfaceHandler } from './updateInterface';
+
+/* eslint-disable @typescript-eslint/naming-convention */
+const methodHandlers = {


TypeScript failed to generate declarations for this, but we don't actually need to export it, so I moved it here and stopped exporting.

codecov · 2026-05-08T07:53:20Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.58%. Comparing base (0044932) to head (7bb3579).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3987   +/-   ##
=======================================
  Coverage   98.58%   98.58%           
=======================================
  Files         427      425    -2     
  Lines       12400    12360   -40     
  Branches     1948     1948           
=======================================
- Hits        12224    12185   -39     
+ Misses        176      175    -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 644d708. Configure here.}

FrederikBolding · 2026-05-08T12:15:42Z

+      const response = await engine.handle({
+        jsonrpc: '2.0',
+        id: 1,
+        method: 'snap_updateInterface',


Some of the tests were not correctly nested under the describe() which results in some whitespace changes. Likely easier to review with whitespace off.

Mrtenz · 2026-05-08T12:02:43Z

+      origin,
      params.path,
-      params.encoding ?? AuxiliaryFileEncoding.Base64,
+      (params.encoding as AuxiliaryFileEncoding) ??


How come this isn't typed properly anymore? Or was the hook not as strict?

The hook used InferredGetFileParams, which makes the enum a union: https://github.com/MetaMask/snaps-skunkworks/blob/fb/refactor-permitted-methods/packages/snaps-sdk/src/types/methods/get-file.ts#L25

The controller expects the union type.

Mrtenz · 2026-05-08T12:24:05Z

+  hooks: PermittedRpcMethodHooks,
+  messenger: Messenger<string, PermittedRpcMethodActions>,
 ): JsonRpcMiddleware<JsonRpcParams, Json> {
-  // This is not actually a misused promise, the type is just wrong


This comment still seems accurate, no?

Maybe? I don't think I understand why the linter thinks this is a misused promise.

Mrtenz · 2026-05-08T12:39:06Z

+ * @param origin - The origin to set on the request.
+ * @returns The middleware.
+ */
+export function createOriginMiddleware(origin: string) {


Can we use the one from json-rpc-engine?

Yes, I added this before going back to add it to core. I have yet to release core though.

FrederikBolding commented May 6, 2026

View reviewed changes

FrederikBolding changed the base branch from main to fb/restricted-methods-messenger May 6, 2026 14:51

FrederikBolding force-pushed the fb/refactor-permitted-methods branch from 41732f0 to 84d1229 Compare May 6, 2026 14:53

FrederikBolding changed the title ~~refactor(snaps-rpc-methods): Use messenger for permitted methods~~ refactor(snaps-rpc-methods)!: Use messenger for permitted methods May 7, 2026

FrederikBolding commented May 7, 2026

View reviewed changes

FrederikBolding marked this pull request as ready for review May 8, 2026 07:59

FrederikBolding requested a review from a team as a code owner May 8, 2026 07:59

cursor Bot reviewed May 8, 2026

View reviewed changes

Comment thread packages/snaps-rpc-methods/src/permitted/listEntropySources.ts

Base automatically changed from fb/restricted-methods-messenger to main May 8, 2026 11:21

FrederikBolding force-pushed the fb/refactor-permitted-methods branch 2 times, most recently from 8d8aceb to 0e1747b Compare May 8, 2026 11:46

Mrtenz requested changes May 8, 2026

View reviewed changes

Comment thread packages/examples/packages/get-file/src/index.ts Outdated

Comment thread packages/snaps-rpc-methods/src/permitted/cancelBackgroundEvent.ts Outdated

Comment thread packages/snaps-rpc-methods/src/permitted/createInterface.test.tsx Outdated

FrederikBolding commented May 8, 2026

View reviewed changes

Mrtenz requested changes May 8, 2026

View reviewed changes

FrederikBolding added 14 commits May 8, 2026 15:44

refactor(snaps-rpc-methods): Use messenger for permitted methods

77b2f55

Migrate the last few

d7ceb32

Fix types after rebase

d86697d

Fix a couple type errors

d197870

Fix additional type issues and rewrite middleware

36b0cf4

Wire up snaps-simulation

520055e

Update permitted hooks used by snaps-simulation

043c6c9

Add origin middleware and keyring state mock

1abc8ff

Add missing wiring for getSnapFile

418f026

Add coverage for snaps-simulation

d85fc22

Port most of the tests

bcaa6f1

Fix remaining tests

bd48f5a

Fix coverage

338afdd

Address PR comments

a6edf0d

Add assertions instead of having to disable lint

3d46203

FrederikBolding force-pushed the fb/refactor-permitted-methods branch from 7bdac91 to 3d46203 Compare May 8, 2026 13:44

FrederikBolding added 2 commits May 8, 2026 15:44

Use createOriginMiddleware

f3a9431

Fix import

7bb3579

FrederikBolding requested a review from Mrtenz May 8, 2026 13:46

Conversation

FrederikBolding commented May 6, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

FrederikBolding commented May 6, 2026 •

edited by cursor Bot

Loading

codecov Bot commented May 8, 2026 •

edited

Loading