diff --git a/eslint-warning-thresholds.json b/eslint-warning-thresholds.json index 1ccfeb8c6ef..06a1f144cf3 100644 --- a/eslint-warning-thresholds.json +++ b/eslint-warning-thresholds.json @@ -6,7 +6,6 @@ "jsdoc/check-tag-names": 13 }, "packages/approval-controller/src/ApprovalController.test.ts": { - "import-x/order": 1, "jest/no-conditional-in-test": 16 }, "packages/approval-controller/src/ApprovalController.ts": { diff --git a/packages/approval-controller/CHANGELOG.md b/packages/approval-controller/CHANGELOG.md index bfd726e315e..9ef93f47031 100644 --- a/packages/approval-controller/CHANGELOG.md +++ b/packages/approval-controller/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Bump `@metamask/base-controller` from ^8.0.0 to ^8.0.1 ([#5722](https://github.com/MetaMask/core/pull/5722)) +### Fixed + +- Add logic to properly rate limit snap install flow requests ([#5824](https://github.com/MetaMask/core/pull/5824)) + ## [7.1.3] ### Changed diff --git a/packages/approval-controller/src/ApprovalController.test.ts b/packages/approval-controller/src/ApprovalController.test.ts index 18cc824b451..9530343c6c3 100644 --- a/packages/approval-controller/src/ApprovalController.test.ts +++ b/packages/approval-controller/src/ApprovalController.test.ts @@ -4,7 +4,6 @@ import { Messenger } from '@metamask/base-controller'; import { errorCodes, JsonRpcError } from '@metamask/rpc-errors'; import { nanoid } from 'nanoid'; -import { flushPromises } from '../../../tests/helpers'; import type { AddApprovalOptions, ApprovalControllerActions, @@ -25,6 +24,7 @@ import { MissingApprovalFlowError, NoApprovalFlowsError, } from './errors'; +import { flushPromises } from '../../../tests/helpers'; jest.mock('nanoid'); @@ -310,6 +310,100 @@ describe('approval controller', () => { ).toThrow(getInvalidRequestStateError()); }); + it.each([ + [ + { + id: 'foo', + origin: 'bar.baz', + type: 'wallet_requestPermissions', + requestData: { + permissions: { + wallet_snap: { 'npm:@metamask/example-snap': {} }, + }, + }, + }, + { + id: 'bar', + origin: 'bar.baz', + type: 'wallet_requestPermissions', + requestData: { + permissions: { + wallet_snap: { 'npm:@metamask/foo-snap': {} }, + }, + }, + }, + ], + [ + { + id: 'foo', + origin: 'bar.baz', + type: 'wallet_installSnap', + requestData: {}, + }, + { + id: 'bar', + origin: 'bar.baz', + type: 'wallet_requestPermissions', + requestData: { + permissions: { + wallet_snap: { 'npm:@metamask/foo-snap': {} }, + }, + }, + }, + ], + [ + { + id: 'foo', + origin: 'bar.baz', + type: 'wallet_requestPermissions', + requestData: { + permissions: { + wallet_snap: { 'npm:@metamask/example-snap': {} }, + }, + }, + }, + { + id: 'bar', + origin: 'bar.baz', + type: 'wallet_updateSnap', + requestData: {}, + }, + ], + [ + { + id: 'foo', + origin: 'bar.baz', + type: 'wallet_installSnapResult', + requestData: {}, + }, + { + id: 'bar', + origin: 'bar.baz', + type: 'wallet_requestPermissions', + requestData: { + permissions: { + wallet_snap: { 'npm:@metamask/example-snap': {} }, + }, + }, + }, + ], + ])( + 'throws on snap install flow collisions', + (firstRequest, secondRequest) => { + // eslint-disable-next-line @typescript-eslint/no-floating-promises + approvalController.add(firstRequest); + + // Second request should throw + const expectedError = + 'Snap installation flow already pending for origin bar.baz. Please wait.'; + + expect(() => { + // eslint-disable-next-line @typescript-eslint/no-floating-promises + approvalController.add(secondRequest); + }).toThrow(expectedError); + }, + ); + it('adds correctly specified entry', () => { expect(() => approvalController.add({ diff --git a/packages/approval-controller/src/ApprovalController.ts b/packages/approval-controller/src/ApprovalController.ts index 5b7398a83f8..157ae4e3d04 100644 --- a/packages/approval-controller/src/ApprovalController.ts +++ b/packages/approval-controller/src/ApprovalController.ts @@ -32,6 +32,15 @@ const stateMetadata = { approvalFlows: { persist: false, anonymous: false }, }; +const SnapApprovalTypes = new Set([ + 'wallet_installSnap', + 'wallet_updateSnap', + 'wallet_installSnapResult', +]); + +const getAlreadyPendingSnapInstallFlowMessage = (origin: string) => + `Snap installation flow already pending for origin ${origin}. Please wait.`; + const getAlreadyPendingMessage = (origin: string, type: string) => `Request of type '${type}' already pending for origin ${origin}. Please wait.`; @@ -916,6 +925,8 @@ export class ApprovalController extends BaseController< ): Promise { this.#validateAddParams(id, origin, type, requestData, requestState); + this.#checkForSnapInstallFlow(type, origin, requestData || {}); + if ( !this.#typesExcludedFromRateLimiting.includes(type) && this.has({ origin, type }) @@ -1108,6 +1119,42 @@ export class ApprovalController extends BaseController< } } } + + #checkForSnapInstallFlow( + type: string, + origin: string, + requestData: Record, + ) { + const pendingApprovals = Object.values(this.state.pendingApprovals); + const isSnapApproval = SnapApprovalTypes.has(type); + const isSnapConnectApproval = + type === 'wallet_requestPermissions' && + Boolean( + (requestData?.permissions as Record)?.wallet_snap, + ); + + if (!(isSnapApproval || isSnapConnectApproval)) { + return; + } + + const hasSomeSnapInstallFlows = pendingApprovals.some((approval) => { + const isSnapApprovalType = SnapApprovalTypes.has(approval.type); + const isSnapConnectApprovalType = Boolean( + (approval.requestData?.permissions as Record) + ?.wallet_snap, + ); + return ( + approval.origin === origin && + (isSnapApprovalType || isSnapConnectApprovalType) + ); + }); + + if (hasSomeSnapInstallFlows) { + throw rpcErrors.resourceUnavailable( + getAlreadyPendingSnapInstallFlowMessage(origin), + ); + } + } } export default ApprovalController; diff --git a/packages/controller-utils/CHANGELOG.md b/packages/controller-utils/CHANGELOG.md index 095b86aa337..f15c47d234e 100644 --- a/packages/controller-utils/CHANGELOG.md +++ b/packages/controller-utils/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add snap approval types for use in the client ([#5824](https://github.com/MetaMask/core/pull/5824)) + ## [11.9.0] ### Added diff --git a/packages/controller-utils/src/constants.ts b/packages/controller-utils/src/constants.ts index 977c15d7b0f..3499ae7fc35 100644 --- a/packages/controller-utils/src/constants.ts +++ b/packages/controller-utils/src/constants.ts @@ -169,6 +169,9 @@ export enum ApprovalType { WalletConnect = 'wallet_connect', WalletRequestPermissions = 'wallet_requestPermissions', WatchAsset = 'wallet_watchAsset', + WalletInstallSnap = 'wallet_installSnap', + WalletUpdateSnap = 'wallet_updateSnap', + WalletInstallSnapResult = 'wallet_installSnapResult', } /**