Move snap_manageAccounts to a gated permitted method

packages/snaps-controllers/src/snaps/SnapController.test.tsx

@@ -18,7 +18,7 @@
   handlerEndowments,
   SnapEndowments,
 } from '@metamask/snaps-rpc-methods';
-import type { SnapId } from '@metamask/snaps-sdk';
+import type { Snap, SnapId } from '@metamask/snaps-sdk';
 import { AuxiliaryFileEncoding, text } from '@metamask/snaps-sdk';
 import { Text } from '@metamask/snaps-sdk/jsx';
 import type { SnapPermissions, RpcOrigins } from '@metamask/snaps-utils';
@@ -1010,6 +1010,42 @@
     snapController.destroy();
   });
 
+  it('filters out removed permissions', async () => {
+    const messenger = getSnapControllerMessenger();
+    const initialPermissions: SnapPermissions = {
+      [handlerEndowments.onRpcRequest as string]: { snaps: false, dapps: true },
+      // eslint-disable-next-line @typescript-eslint/naming-convention
+      snap_manageAccounts: {},
+    };
+
+    const { manifest } = await getMockSnapFilesWithUpdatedChecksum({
+      manifest: getSnapManifest({
+        initialPermissions,
+      }),
+    });
+
+    const snapController = getSnapController(
+      getSnapControllerOptions({
+        messenger,
+        detectSnapLocation: loopbackDetect({
+          manifest: manifest.result,
+        }),
+      }),
+    );
+
+    const snap = await snapController.installSnaps(MOCK_ORIGIN, {
+      [MOCK_SNAP_ID]: {},
+    });
+
+    const permissions = (snap[MOCK_SNAP_ID] as Snap).initialPermissions;
+
+    expect(permissions).toStrictEqual({
+      [handlerEndowments.onRpcRequest as string]: { snaps: false, dapps: true },
+    });
+
+    snapController.destroy();
+  });
+
   it('throws an error if the installation is disabled during installSnaps', async () => {
     const controller = getSnapController(
       getSnapControllerOptions({
@@ -1850,7 +1886,7 @@
   });
 
   // This isn't stable in CI unfortunately
   it.skip('throws if the Snap is terminated while executing', async () => {
     const { manifest, sourceCode, svgIcon } =
       await getMockSnapFilesWithUpdatedChecksum({
         sourceCode: `

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

@@ -10,10 +10,10 @@ module.exports = deepmerge(baseConfig, {
   ],
   coverageThreshold: {
     global: {
-      branches: 92.85,
-      functions: 97.23,
-      lines: 97.8,
-      statements: 97.31,
+      branches: 92.88,
+      functions: 97.22,
+      lines: 97.81,
+      statements: 97.32,
     },
   },
 });

packages/snaps-rpc-methods/src/permissions.test.ts

@@ -211,15 +211,6 @@ describe('buildSnapRestrictedMethodSpecifications', () => {
           ],
           "targetName": "snap_getPreferences",
         },
-        "snap_manageAccounts": {
-          "allowedCaveats": null,
-          "methodImplementation": [Function],
-          "permissionType": "RestrictedMethod",
-          "subjectTypes": [
-            "snap",
-          ],
-          "targetName": "snap_manageAccounts",
-        },
         "snap_manageState": {
           "allowedCaveats": null,
           "methodImplementation": [Function],

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

@@ -15,6 +15,21 @@ import {
 } from './restricted';
 import { selectHooks } from './utils';
 
+const REMOVED_PERMISSIONS = Object.freeze(['snap_manageAccounts']);
+
+/**
+ * Filters out permissions that have been removed from the Snap API.
+ *
+ * @param initialPermission - The initial permission to filter.
+ * @returns Whether the permission has been removed.
+ */
+export const filterRemovedPermissions = (
+  initialPermission: [string, unknown],
+) => {
+  const [value] = initialPermission;
+  return !REMOVED_PERMISSIONS.some((permission) => permission === value);
+};
+
 /**
  * Map initial permissions as defined in a Snap manifest to something that can
  * be processed by the PermissionsController. Each caveat mapping function
@@ -30,22 +45,32 @@ export function processSnapPermissions(
   initialPermissions: SnapPermissions,
 ): Record<string, Pick<PermissionConstraint, 'caveats'>> {
   return Object.fromEntries(
-    Object.entries(initialPermissions).map(([initialPermission, value]) => {
-      if (hasProperty(caveatMappers, initialPermission)) {
-        return [initialPermission, caveatMappers[initialPermission](value)];
-      } else if (hasProperty(endowmentCaveatMappers, initialPermission)) {
+    Object.entries(initialPermissions)
+      .filter(filterRemovedPermissions)
+      .map(([initialPermission, value]) => {
+        if (
+          REMOVED_PERMISSIONS.some(
+            (permission) => permission === initialPermission,
+          )
+        ) {
+          return [];
+        }
+
+        if (hasProperty(caveatMappers, initialPermission)) {
+          return [initialPermission, caveatMappers[initialPermission](value)];
+        } else if (hasProperty(endowmentCaveatMappers, initialPermission)) {
+          return [
+            initialPermission,
+            endowmentCaveatMappers[initialPermission](value),
+          ];
+        }
+
+        // If we have no mapping, this may be a non-snap permission, return as-is
         return [
           initialPermission,
-          endowmentCaveatMappers[initialPermission](value),
+          value as Pick<PermissionConstraint, 'caveats'>,
         ];
-      }
-
-      // If we have no mapping, this may be a non-snap permission, return as-is
-      return [
-        initialPermission,
-        value as Pick<PermissionConstraint, 'caveats'>,
-      ];
-    }),
+      }),
   );
 }
 

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

@@ -8,6 +8,7 @@ import { getInterfaceStateHandler } from './getInterfaceState';
 import { getSnapsHandler } from './getSnaps';
 import { invokeKeyringHandler } from './invokeKeyring';
 import { invokeSnapSugarHandler } from './invokeSnapSugar';
+import { manageAccountsHandler } from './manageAccounts';
 import { requestSnapsHandler } from './requestSnaps';
 import { resolveInterfaceHandler } from './resolveInterface';
 import { updateInterfaceHandler } from './updateInterface';
@@ -27,6 +28,7 @@ export const methodHandlers = {
   snap_resolveInterface: resolveInterfaceHandler,
   snap_getCurrencyRate: getCurrencyRateHandler,
   snap_experimentalProviderRequest: providerRequestHandler,
+  snap_manageAccounts: manageAccountsHandler,
 };
 /* eslint-enable @typescript-eslint/naming-convention */
 

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

@@ -5,6 +5,7 @@ import type { GetClientStatusHooks } from './getClientStatus';
 import type { GetCurrencyRateMethodHooks } from './getCurrencyRate';
 import type { GetInterfaceStateMethodHooks } from './getInterfaceState';
 import type { GetSnapsHooks } from './getSnaps';
+import type { ManageAccountsMethodHooks } from './manageAccounts';
 import type { RequestSnapsHooks } from './requestSnaps';
 import type { ResolveInterfaceMethodHooks } from './resolveInterface';
 import type { UpdateInterfaceMethodHooks } from './updateInterface';
@@ -18,7 +19,8 @@ export type PermittedRpcMethodHooks = GetAllSnapsHooks &
   GetInterfaceStateMethodHooks &
   ResolveInterfaceMethodHooks &
   GetCurrencyRateMethodHooks &
-  ProviderRequestMethodHooks;
+  ProviderRequestMethodHooks &
+  ManageAccountsMethodHooks;
 
 export * from './handlers';
 export * from './middleware';

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

@@ -0,0 +1,208 @@
+import { JsonRpcEngine } from '@metamask/json-rpc-engine';
+import { rpcErrors } from '@metamask/rpc-errors';
+import type { ManageAccountsResult } from '@metamask/snaps-sdk';
+import type {
+  JsonRpcFailure,
+  JsonRpcRequest,
+  PendingJsonRpcResponse,
+} from '@metamask/utils';
+
+import type { ManageAccountsParameters } from './manageAccounts';
+import { manageAccountsHandler } from './manageAccounts';
+
+describe('snap_manageAccounts', () => {
+  describe('manageAccountsHandler', () => {
+    it('has the expected shape', () => {
+      expect(manageAccountsHandler).toMatchObject({
+        methodNames: ['snap_manageAccounts'],
+        implementation: expect.any(Function),
+        hookNames: {
+          hasPermission: true,
+          handleKeyringSnapMessage: true,
+        },
+      });
+    });
+  });
+
+  describe('implementation', () => {
+    it('returns the result from the `handleKeyringSnapMessage` hook', async () => {
+      const { implementation } = manageAccountsHandler;
+
+      const hasPermission = jest.fn().mockReturnValue(true);
+      const handleKeyringSnapMessage = jest.fn().mockReturnValue('foo');
+
+      const hooks = {
+        hasPermission,
+        handleKeyringSnapMessage,
+      };
+
+      const engine = new JsonRpcEngine();
+
+      engine.push((request, response, next, end) => {
+        const result = implementation(
+          request as JsonRpcRequest<ManageAccountsParameters>,
+          response as PendingJsonRpcResponse<ManageAccountsResult>,
+          next,
+          end,
+          hooks,
+        );
+
+        result?.catch(end);
+      });
+
+      const response = await engine.handle({
+        jsonrpc: '2.0',
+        id: 1,
+        method: 'snap_manageAccounts',
+        params: {
+          method: 'foo',
+          params: { bar: 'baz' },
+        },
+      });
+
+      expect(handleKeyringSnapMessage).toHaveBeenCalledWith({
+        method: 'foo',
+        params: { bar: 'baz' },
+      });
+
+      expect(response).toStrictEqual({ jsonrpc: '2.0', id: 1, result: 'foo' });
+    });
+
+    it('throws an error if the snap does not have permission', async () => {
+      const { implementation } = manageAccountsHandler;
+
+      const hasPermission = jest.fn().mockReturnValue(false);
+      const handleKeyringSnapMessage = jest.fn().mockReturnValue('foo');
+
+      const hooks = {
+        hasPermission,
+        handleKeyringSnapMessage,
+      };
+
+      const engine = new JsonRpcEngine();
+
+      engine.push((request, response, next, end) => {
+        const result = implementation(
+          request as JsonRpcRequest<ManageAccountsParameters>,
+          response as PendingJsonRpcResponse<ManageAccountsResult>,
+          next,
+          end,
+          hooks,
+        );
+
+        result?.catch(end);
+      });
+
+      const response = (await engine.handle({
+        jsonrpc: '2.0',
+        id: 1,
+        method: 'snap_manageAccounts',
+        params: {
+          method: 'foo',
+          params: { bar: 'baz' },
+        },
+      })) as JsonRpcFailure;
+
+      expect(response.error).toStrictEqual({
+        ...rpcErrors.methodNotFound().serialize(),
+        stack: expect.any(String),
+      });
+    });
+
+    it('throws an error if the `handleKeyringSnapMessage` hook throws', async () => {
+      const { implementation } = manageAccountsHandler;
+
+      const hasPermission = jest.fn().mockReturnValue(true);
+      const handleKeyringSnapMessage = jest
+        .fn()
+        .mockRejectedValue(new Error('foo'));
+
+      const hooks = {
+        hasPermission,
+        handleKeyringSnapMessage,
+      };
+
+      const engine = new JsonRpcEngine();
+
+      engine.push((request, response, next, end) => {
+        const result = implementation(
+          request as JsonRpcRequest<ManageAccountsParameters>,
+          response as PendingJsonRpcResponse<ManageAccountsResult>,
+          next,
+          end,
+          hooks,
+        );
+
+        result?.catch(end);
+      });
+
+      const response = (await engine.handle({
+        jsonrpc: '2.0',
+        id: 1,
+        method: 'snap_manageAccounts',
+        params: {
+          method: 'foo',
+          params: { bar: 'baz' },
+        },
+      })) as JsonRpcFailure;
+
+      expect(response.error).toStrictEqual({
+        code: -32603,
+        message: 'foo',
+        data: {
+          cause: {
+            message: 'foo',
+            stack: expect.any(String),
+          },
+        },
+      });
+    });
+
+    it('throws on invalid params', async () => {
+      const { implementation } = manageAccountsHandler;
+
+      const hasPermission = jest.fn().mockReturnValue(true);
+      const handleKeyringSnapMessage = jest.fn().mockReturnValue('foo');
+
+      const hooks = {
+        hasPermission,
+        handleKeyringSnapMessage,
+      };
+
+      const engine = new JsonRpcEngine();
+
+      engine.push((request, response, next, end) => {
+        const result = implementation(
+          request as JsonRpcRequest<ManageAccountsParameters>,
+          response as PendingJsonRpcResponse<ManageAccountsResult>,
+          next,
+          end,
+          hooks,
+        );
+
+        result?.catch(end);
+      });
+
+      const response = await engine.handle({
+        jsonrpc: '2.0',
+        id: 1,
+        method: 'snap_manageAccounts',
+        params: {
+          method: 'foo',
+          params: 42,
+        },
+      });
+
+      expect(response).toStrictEqual({
+        jsonrpc: '2.0',
+        id: 1,
+        error: {
+          code: -32602,
+          message:
+            'Invalid params: At path: params -- Expected the value to satisfy a union of `array | record`, but received: 42.',
+          stack: expect.any(String),
+        },
+      });
+    });
+  });
+});