Pass excludedPermissions to SnapController (#17321)

Co-authored-by: Frederik Bolding <frederik.bolding@gmail.com>
MetaMask · Feb 15, 2023 · ccde549 · ccde549
1 parent 3564f73
commit ccde549
Show file tree

Hide file tree

Showing 8 changed files with 114 additions and 13 deletions.
diff --git a/app/scripts/controllers/permissions/flask/snap-permissions.js b/app/scripts/controllers/permissions/flask/snap-permissions.js
@@ -15,7 +15,7 @@ import {
 export const buildSnapEndowmentSpecifications = () =>
   Object.values(endowmentPermissionBuilders).reduce(
     (allSpecifications, { targetKey, specificationBuilder }) => {
-      if (!ExcludedSnapEndowments.has(targetKey)) {
+      if (!Object.keys(ExcludedSnapEndowments).includes(targetKey)) {
         allSpecifications[targetKey] = specificationBuilder();
       }
       return allSpecifications;
@@ -27,10 +27,10 @@ export const buildSnapEndowmentSpecifications = () =>
  * @param {Record<string, Function>} hooks - The hooks for the Snap
  * restricted method implementations.
  */
-export function buildSnapRestrictedMethodSpecifications(hooks) {
-  return Object.values(restrictedMethodPermissionBuilders).reduce(
+export const buildSnapRestrictedMethodSpecifications = (hooks) =>
+  Object.values(restrictedMethodPermissionBuilders).reduce(
     (specifications, { targetKey, specificationBuilder, methodHooks }) => {
-      if (!ExcludedSnapPermissions.has(targetKey)) {
+      if (!Object.keys(ExcludedSnapPermissions).includes(targetKey)) {
         specifications[targetKey] = specificationBuilder({
           methodHooks: selectHooks(hooks, methodHooks),
         });
@@ -39,4 +39,3 @@ export function buildSnapRestrictedMethodSpecifications(hooks) {
     },
     {},
   );
-}
diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js
@@ -85,6 +85,8 @@ import {
   RestrictedMethods,
   ///: BEGIN:ONLY_INCLUDE_IN(flask)
   EndowmentPermissions,
+  ExcludedSnapPermissions,
+  ExcludedSnapEndowments,
   ///: END:ONLY_INCLUDE_IN
 } from '../../shared/constants/permissions';
 import { UI_NOTIFICATIONS } from '../../shared/notifications';
@@ -110,6 +112,9 @@ import { STATIC_MAINNET_TOKEN_LIST } from '../../shared/constants/tokens';
 import { getTokenValueParam } from '../../shared/lib/metamask-controller-utils';
 import { isManifestV3 } from '../../shared/modules/mv3.utils';
 import { hexToDecimal } from '../../shared/modules/conversion.utils';
+///: BEGIN:ONLY_INCLUDE_IN(flask)
+import { isMain, isFlask } from '../../shared/constants/environment';
+///: END:ONLY_INCLUDE_IN
 import {
   onMessageReceived,
   checkForMultipleVersionsRunning,
@@ -775,11 +780,12 @@ export default class MetamaskController extends EventEmitter {
       ],
     });
 
-    const isMain = process.env.METAMASK_BUILD_TYPE === 'main';
-    const isFlask = process.env.METAMASK_BUILD_TYPE === 'flask';
-
     this.snapController = new SnapController({
       environmentEndowmentPermissions: Object.values(EndowmentPermissions),
+      excludedPermissions: {
+        ...ExcludedSnapPermissions,
+        ...ExcludedSnapEndowments,
+      },
       closeAllConnections: this.removeAllConnections.bind(this),
       state: initState.SnapController,
       messenger: snapControllerMessenger,

diff --git a/lavamoat/browserify/beta/policy.json b/lavamoat/browserify/beta/policy.json
@@ -1311,6 +1311,32 @@
         "browserify>buffer": true
       }
     },
+    "@metamask/rpc-methods>@metamask/key-tree": {
+      "packages": {
+        "@metamask/rpc-methods>@metamask/key-tree>@noble/ed25519": true,
+        "@metamask/rpc-methods>@metamask/key-tree>@noble/secp256k1": true,
+        "@metamask/scure-bip39": true,
+        "@metamask/snaps-utils>@noble/hashes": true,
+        "@metamask/snaps-utils>@scure/base": true,
+        "@metamask/utils": true
+      }
+    },
+    "@metamask/rpc-methods>@metamask/key-tree>@noble/ed25519": {
+      "globals": {
+        "crypto": true
+      },
+      "packages": {
+        "browserify>browser-resolve": true
+      }
+    },
+    "@metamask/rpc-methods>@metamask/key-tree>@noble/secp256k1": {
+      "globals": {
+        "crypto": true
+      },
+      "packages": {
+        "browserify>browser-resolve": true
+      }
+    },
     "@metamask/rpc-methods>nanoid": {
       "globals": {
         "crypto.getRandomValues": true

diff --git a/lavamoat/browserify/flask/policy.json b/lavamoat/browserify/flask/policy.json
@@ -1403,6 +1403,32 @@
         "browserify>buffer": true
       }
     },
+    "@metamask/rpc-methods>@metamask/key-tree": {
+      "packages": {
+        "@metamask/rpc-methods>@metamask/key-tree>@noble/ed25519": true,
+        "@metamask/rpc-methods>@metamask/key-tree>@noble/secp256k1": true,
+        "@metamask/scure-bip39": true,
+        "@metamask/snaps-utils>@noble/hashes": true,
+        "@metamask/snaps-utils>@scure/base": true,
+        "@metamask/utils": true
+      }
+    },
+    "@metamask/rpc-methods>@metamask/key-tree>@noble/ed25519": {
+      "globals": {
+        "crypto": true
+      },
+      "packages": {
+        "browserify>browser-resolve": true
+      }
+    },
+    "@metamask/rpc-methods>@metamask/key-tree>@noble/secp256k1": {
+      "globals": {
+        "crypto": true
+      },
+      "packages": {
+        "browserify>browser-resolve": true
+      }
+    },
     "@metamask/rpc-methods>nanoid": {
       "globals": {
         "crypto.getRandomValues": true

diff --git a/lavamoat/browserify/main/policy.json b/lavamoat/browserify/main/policy.json
@@ -1311,6 +1311,32 @@
         "browserify>buffer": true
       }
     },
+    "@metamask/rpc-methods>@metamask/key-tree": {
+      "packages": {
+        "@metamask/rpc-methods>@metamask/key-tree>@noble/ed25519": true,
+        "@metamask/rpc-methods>@metamask/key-tree>@noble/secp256k1": true,
+        "@metamask/scure-bip39": true,
+        "@metamask/snaps-utils>@noble/hashes": true,
+        "@metamask/snaps-utils>@scure/base": true,
+        "@metamask/utils": true
+      }
+    },
+    "@metamask/rpc-methods>@metamask/key-tree>@noble/ed25519": {
+      "globals": {
+        "crypto": true
+      },
+      "packages": {
+        "browserify>browser-resolve": true
+      }
+    },
+    "@metamask/rpc-methods>@metamask/key-tree>@noble/secp256k1": {
+      "globals": {
+        "crypto": true
+      },
+      "packages": {
+        "browserify>browser-resolve": true
+      }
+    },
     "@metamask/rpc-methods>nanoid": {
       "globals": {
         "crypto.getRandomValues": true

diff --git a/shared/constants/environment.js b/shared/constants/environment.js
@@ -0,0 +1,2 @@
+export const isMain = process.env.METAMASK_BUILD_TYPE === 'main';
+export const isFlask = process.env.METAMASK_BUILD_TYPE === 'flask';
diff --git a/shared/constants/permissions.test.js b/shared/constants/permissions.test.js
@@ -11,7 +11,10 @@ describe('EndowmentPermissions', () => {
   it('has the expected permission keys', () => {
     expect(Object.keys(EndowmentPermissions).sort()).toStrictEqual(
       Object.keys(endowmentPermissionBuilders)
-        .filter((targetKey) => !ExcludedSnapEndowments.has(targetKey))
+        .filter(
+          (targetKey) =>
+            !Object.keys(ExcludedSnapEndowments).includes(targetKey),
+        )
         .sort(),
     );
   });
@@ -23,7 +26,8 @@ describe('RestrictedMethods', () => {
       [
         'eth_accounts',
         ...Object.keys(restrictedMethodPermissionBuilders).filter(
-          (targetKey) => !ExcludedSnapPermissions.has(targetKey),
+          (targetKey) =>
+            !Object.keys(ExcludedSnapPermissions).includes(targetKey),
         ),
       ].sort(),
     );

diff --git a/shared/constants/permissions.ts b/shared/constants/permissions.ts
@@ -18,20 +18,32 @@ export const RestrictedMethods = Object.freeze({
 } as const);
 
 ///: BEGIN:ONLY_INCLUDE_IN(flask)
+/**
+ * Exclude permissions by code fencing them to avoid any potential usage of excluded permissions at runtime. See: https://github.com/MetaMask/metamask-extension/pull/17321#pullrequestreview-1287014285.
+ * This is a fix for https://github.com/MetaMask/snaps-monorepo/issues/1103 and https://github.com/MetaMask/snaps-monorepo/issues/990.
+ * TODO: Disable endowment:long-running and eth_account in stable.
+ */
 export const PermissionNamespaces = Object.freeze({
   wallet_snap_: 'wallet_snap_*',
 } as const);
 
 export const EndowmentPermissions = Object.freeze({
   'endowment:network-access': 'endowment:network-access',
-  'endowment:long-running': 'endowment:long-running',
   'endowment:transaction-insight': 'endowment:transaction-insight',
   'endowment:cronjob': 'endowment:cronjob',
   'endowment:ethereum-provider': 'endowment:ethereum-provider',
   'endowment:rpc': 'endowment:rpc',
+  'endowment:long-running': 'endowment:long-running',
 } as const);
 
 // Methods / permissions in external packages that we are temporarily excluding.
-export const ExcludedSnapPermissions = new Set([]);
-export const ExcludedSnapEndowments = new Set(['endowment:keyring']);
+export const ExcludedSnapPermissions = Object.freeze({
+  eth_accounts:
+    'eth_accounts is disabled. For more information please see https://github.com/MetaMask/snaps-monorepo/issues/990.',
+});
+
+export const ExcludedSnapEndowments = Object.freeze({
+  'endowment:keyring':
+    'This endowment is still in development therefore not available.',
+});
 ///: END:ONLY_INCLUDE_IN
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		export const isMain = process.env.METAMASK_BUILD_TYPE === 'main';
		export const isFlask = process.env.METAMASK_BUILD_TYPE === 'flask';