Use @metamask/json-rpc-engine, @metamask/rpc-errors, and `@metama…

…sk/utils` in `permission-controller`

In order to remove the dependency on `@metamask/types` inside `snaps-monorepo`, we need the `PermittedHandlerExport` type. This type depends on types from the `@metamask/json-rpc-engine` package, which were previously duplicated inside `@metamask/types`. In this commit, I have moved `PermittedHandlerExport` to this package, which required bumping `@metamask/json-rpc-engine`, `@metamask/rpc-errors`, and `@metamask/utils`.

packages/permission-controller/package.json

@@ -32,12 +32,12 @@
     "@metamask/approval-controller": "workspace:^",
     "@metamask/base-controller": "workspace:^",
     "@metamask/controller-utils": "workspace:^",
-    "@metamask/types": "^1.1.0",
+    "@metamask/json-rpc-engine": "^7.0.0",
+    "@metamask/rpc-errors": "^5.0.0",
+    "@metamask/utils": "^5.0.1",
     "@types/deep-freeze-strict": "^1.1.0",
     "deep-freeze-strict": "^1.1.1",
-    "eth-rpc-errors": "^4.0.0",
     "immer": "^9.0.6",
-    "json-rpc-engine": "^6.1.0",
     "nanoid": "^3.1.31"
   },
   "devDependencies": {

packages/permission-controller/src/Caveat.ts

@@ -1,5 +1,4 @@
-import { Json } from '@metamask/types';
-import { hasProperty } from '@metamask/controller-utils';
+import { Json, hasProperty } from '@metamask/utils';
 import {
   CaveatSpecificationMismatchError,
   UnrecognizedCaveatTypeError,

packages/permission-controller/src/Permission.ts

@@ -1,6 +1,5 @@
-import { Json } from '@metamask/types';
+import { Json, NonEmptyArray } from '@metamask/utils';
 import { nanoid } from 'nanoid';
-import { NonEmptyArray } from '@metamask/controller-utils';
 import { ActionConstraint, EventConstraint } from '@metamask/base-controller';
 import { CaveatConstraint } from './Caveat';
 
@@ -258,7 +257,10 @@ type RestrictedMethodContext = Readonly<{
   [key: string]: any;
 }>;
 
-export type RestrictedMethodParameters = Json[] | Record<string, Json> | void;
+export type RestrictedMethodParameters =
+  | Json[]
+  | Record<string, Json>
+  | undefined;
 
 /**
  * The arguments passed to a restricted method implementation.

packages/permission-controller/src/PermissionController.test.ts

@@ -1,5 +1,5 @@
 import assert from 'assert';
-import { JsonRpcEngine, PendingJsonRpcResponse } from 'json-rpc-engine';
+import { JsonRpcEngine } from '@metamask/json-rpc-engine';
 import {
   AcceptRequest as AcceptApprovalRequest,
   AddApprovalRequest,
@@ -8,6 +8,7 @@ import {
 } from '@metamask/approval-controller';
 import { ControllerMessenger } from '@metamask/base-controller';
 import { Json, hasProperty, isPlainObject } from '@metamask/controller-utils';
+import { PendingJsonRpcResponse } from '@metamask/utils';
 import * as errors from './errors';
 import { EndowmentGetterParams } from './Permission';
 import {
@@ -247,7 +248,7 @@ function getDefaultPermissionSpecifications() {
         CaveatTypes.filterArrayResponse,
         CaveatTypes.reverseArrayResponse,
       ],
-      methodImplementation: (_args: RestrictedMethodOptions<void>) => {
+      methodImplementation: (_args: RestrictedMethodOptions<undefined>) => {
         return ['a', 'b', 'c'];
       },
     },
@@ -258,7 +259,7 @@ function getDefaultPermissionSpecifications() {
         CaveatTypes.filterObjectResponse,
         CaveatTypes.noopCaveat,
       ],
-      methodImplementation: (_args: RestrictedMethodOptions<void>) => {
+      methodImplementation: (_args: RestrictedMethodOptions<undefined>) => {
         return { a: 'x', b: 'y', c: 'z' };
       },
       validator: (permission: PermissionConstraint) => {
@@ -275,7 +276,7 @@ function getDefaultPermissionSpecifications() {
       permissionType: PermissionType.RestrictedMethod,
       targetKey: PermissionKeys['wallet_getSecret_*'],
       allowedCaveats: [CaveatTypes.noopCaveat],
-      methodImplementation: (args: RestrictedMethodOptions<void>) => {
+      methodImplementation: (args: RestrictedMethodOptions<undefined>) => {
         return `Hello, secret friend "${args.method.replace(
           'wallet_getSecret_',
           '',
@@ -311,15 +312,15 @@ function getDefaultPermissionSpecifications() {
       permissionType: PermissionType.RestrictedMethod,
       targetKey: PermissionKeys.wallet_noop,
       allowedCaveats: null,
-      methodImplementation: (_args: RestrictedMethodOptions<void>) => {
+      methodImplementation: (_args: RestrictedMethodOptions<undefined>) => {
         return null;
       },
     },
     [PermissionKeys.wallet_noopWithPermittedAndFailureSideEffects]: {
       permissionType: PermissionType.RestrictedMethod,
       targetKey: PermissionKeys.wallet_noopWithPermittedAndFailureSideEffects,
       allowedCaveats: null,
-      methodImplementation: (_args: RestrictedMethodOptions<void>) => {
+      methodImplementation: (_args: RestrictedMethodOptions<undefined>) => {
         return null;
       },
       sideEffect: {
@@ -331,7 +332,7 @@ function getDefaultPermissionSpecifications() {
       permissionType: PermissionType.RestrictedMethod,
       targetKey: PermissionKeys.wallet_noopWithPermittedAndFailureSideEffects2,
       allowedCaveats: null,
-      methodImplementation: (_args: RestrictedMethodOptions<void>) => {
+      methodImplementation: (_args: RestrictedMethodOptions<undefined>) => {
         return null;
       },
       sideEffect: {
@@ -343,7 +344,7 @@ function getDefaultPermissionSpecifications() {
       permissionType: PermissionType.RestrictedMethod,
       targetKey: PermissionKeys.wallet_noopWithPermittedSideEffects,
       allowedCaveats: null,
-      methodImplementation: (_args: RestrictedMethodOptions<void>) => {
+      methodImplementation: (_args: RestrictedMethodOptions<undefined>) => {
         return null;
       },
       sideEffect: {
@@ -354,7 +355,7 @@ function getDefaultPermissionSpecifications() {
     [PermissionKeys.wallet_noopWithValidator]: {
       permissionType: PermissionType.RestrictedMethod,
       targetKey: PermissionKeys.wallet_noopWithValidator,
-      methodImplementation: (_args: RestrictedMethodOptions<void>) => {
+      methodImplementation: (_args: RestrictedMethodOptions<undefined>) => {
         return null;
       },
       allowedCaveats: [CaveatTypes.noopCaveat, CaveatTypes.filterArrayResponse],
@@ -373,7 +374,7 @@ function getDefaultPermissionSpecifications() {
     [PermissionKeys.wallet_noopWithFactory]: {
       permissionType: PermissionType.RestrictedMethod,
       targetKey: PermissionKeys.wallet_noopWithFactory,
-      methodImplementation: (_args: RestrictedMethodOptions<void>) => {
+      methodImplementation: (_args: RestrictedMethodOptions<undefined>) => {
         return null;
       },
       allowedCaveats: [CaveatTypes.filterArrayResponse],
@@ -3710,6 +3711,40 @@ describe('PermissionController', () => {
       );
     });
 
+    it('throws if requested permissions object is not JSON', async () => {
+      const options = getPermissionControllerOptions();
+      const { messenger } = options;
+      const origin = 'metamask.io';
+      const controller = getDefaultPermissionController(options);
+
+      jest
+        .spyOn(messenger, 'call')
+        .mockImplementationOnce(async (...args: any) => {
+          const [, { requestData }] = args;
+          return {
+            metadata: { ...requestData.metadata },
+            permissions: { ...requestData.permissions },
+          };
+        });
+
+      await expect(
+        async () =>
+          await controller.requestPermissions(
+            { origin },
+            {
+              [PermissionNames.wallet_getSecretArray]: {
+                // @ts-expect-error - Invalid JSON value.
+                foo: () => {},
+              },
+            },
+          ),
+      ).rejects.toThrow(
+        errors.invalidParams({
+          message: `Approved permissions request for subject "metamask.io" is not a valid JSON value.`,
+        }),
+      );
+    });
+
     it('throws if requested permissions object is not a plain object', async () => {
       const options = getPermissionControllerOptions();
       const { messenger } = options;

packages/permission-controller/src/PermissionController.ts

@@ -1,9 +1,8 @@
 /* eslint-enable @typescript-eslint/no-unused-vars */
-import { Mutable } from '@metamask/types';
 import deepFreeze from 'deep-freeze-strict';
 import { castDraft, Draft, Patch } from 'immer';
 import { nanoid } from 'nanoid';
-import { EthereumRpcError } from 'eth-rpc-errors';
+import { JsonRpcError, serializeError } from '@metamask/rpc-errors';
 import {
   AcceptRequest as AcceptApprovalRequest,
   AddApprovalRequest,
@@ -25,6 +24,7 @@ import {
   Json,
   NonEmptyArray,
 } from '@metamask/controller-utils';
+import { assert, Mutable } from '@metamask/utils';
 import {
   CaveatConstraint,
   CaveatSpecificationConstraint,
@@ -1825,6 +1825,10 @@ export class PermissionController<
     origin: OriginString,
     target: string,
   ): void {
+    if (!isValidJson(caveat)) {
+      throw new CaveatInvalidJsonError(caveat, origin, target);
+    }
+
     if (!isPlainObject(caveat)) {
       throw new InvalidCaveatError(caveat, origin, target);
     }
@@ -1846,10 +1850,6 @@ export class PermissionController<
       throw new CaveatMissingValueError(caveat, origin, target);
     }
 
-    if (!isValidJson(caveat.value)) {
-      throw new CaveatInvalidJsonError(caveat, origin, target);
-    }
-
     // Typecast: TypeScript still believes that the caveat is a PlainObject.
     specification.validator?.(caveat as CaveatConstraint, origin, target);
   }
@@ -1963,7 +1963,7 @@ export class PermissionController<
    */
   private validateRequestedPermissions(
     origin: OriginString,
-    requestedPermissions: unknown,
+    requestedPermissions: Json,
   ): void {
     if (!isPlainObject(requestedPermissions)) {
       throw invalidParams({
@@ -2103,7 +2103,9 @@ export class PermissionController<
             failureHandlersList.map((failureHandler) => failureHandler(params)),
           );
         } catch (error) {
-          throw internalError('Unexpected error in side-effects', { error });
+          throw internalError('Unexpected error in side-effects', {
+            error: serializeError(error),
+          });
         }
       }
       const reasons = rejectedHandlers.map((handler) => handler.reason);
@@ -2115,7 +2117,7 @@ export class PermissionController<
       throw reasons.length > 1
         ? internalError(
             'Multiple errors occurred during side-effects execution',
-            { errors: reasons },
+            { errors: reasons.map((error) => serializeError(error)) },
           )
         : reasons[0];
     }
@@ -2145,6 +2147,12 @@ export class PermissionController<
   ) {
     const { id, origin } = originalMetadata;
 
+    if (!isValidJson(approvedRequest)) {
+      throw internalError(
+        `Approved permissions request for subject "${origin}" is not a valid JSON value.`,
+      );
+    }
+
     if (
       !isPlainObject(approvedRequest) ||
       !isPlainObject(approvedRequest.metadata)
@@ -2177,15 +2185,19 @@ export class PermissionController<
     try {
       this.validateRequestedPermissions(origin, permissions);
     } catch (error) {
-      if (error instanceof EthereumRpcError) {
+      if (error instanceof JsonRpcError) {
         // Re-throw as an internal error; we should never receive invalid approved
         // permissions.
         throw internalError(
           `Invalid approved permissions request: ${error.message}`,
           error.data,
         );
       }
-      throw internalError('Unrecognized error type', { error });
+
+      /* istanbul ignore next: should never happen */
+      throw internalError('Unrecognized error type', {
+        error: serializeError(error),
+      });
     }
   }