From ccde54937f7144e8e24c5317b5634b57524787a0 Mon Sep 17 00:00:00 2001 From: Guillaume Roux Date: Wed, 15 Feb 2023 11:09:47 +0100 Subject: [PATCH] Pass `excludedPermissions` to `SnapController` (#17321) Co-authored-by: Frederik Bolding --- .../permissions/flask/snap-permissions.js | 9 +++---- app/scripts/metamask-controller.js | 12 ++++++--- lavamoat/browserify/beta/policy.json | 26 +++++++++++++++++++ lavamoat/browserify/flask/policy.json | 26 +++++++++++++++++++ lavamoat/browserify/main/policy.json | 26 +++++++++++++++++++ shared/constants/environment.js | 2 ++ shared/constants/permissions.test.js | 8 ++++-- shared/constants/permissions.ts | 18 ++++++++++--- 8 files changed, 114 insertions(+), 13 deletions(-) create mode 100644 shared/constants/environment.js diff --git a/app/scripts/controllers/permissions/flask/snap-permissions.js b/app/scripts/controllers/permissions/flask/snap-permissions.js index f8465ee1b..92d4bc52a 100644 --- 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} 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 index b72c68741..570b7d60d 100644 --- 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 index 93a93aa1d..0f71689d4 100644 --- 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 index 48c20441c..29dab89ea 100644 --- 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 index 93a93aa1d..0f71689d4 100644 --- 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 new file mode 100644 index 000000000..ffc551ccd --- /dev/null +++ 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 index e12b6ab93..aa4f9f489 100644 --- 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 index 0b98b3b57..fd57e4f5e 100644 --- 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