diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index c795476942f..94b3ae59b64 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -154,9 +154,11 @@ /packages/wallet/src/initialization/instances/gas-fee-controller/ @MetaMask/confirmations /packages/wallet/src/initialization/instances/keyring-controller/ @MetaMask/accounts-engineers @MetaMask/core-platform /packages/wallet/src/initialization/instances/passkey-controller/ @MetaMask/web3auth +/packages/wallet/src/initialization/instances/permission-controller/ @MetaMask/core-platform /packages/wallet/src/initialization/instances/remote-feature-flag-controller/ @MetaMask/extension-platform @MetaMask/mobile-platform @MetaMask/core-platform /packages/wallet/src/initialization/instances/seedless-onboarding-controller/ @MetaMask/web3auth /packages/wallet/src/initialization/instances/storage-service/ @MetaMask/extension-platform @MetaMask/mobile-platform @MetaMask/core-platform +/packages/wallet/src/initialization/instances/subject-metadata-controller/ @MetaMask/core-platform /packages/wallet/src/initialization/instances/transaction-controller/ @MetaMask/confirmations ## Package Release related diff --git a/README.md b/README.md index 733f6c8b295..ee1db6e74ce 100644 --- a/README.md +++ b/README.md @@ -662,6 +662,7 @@ linkStyle default opacity:0.5 wallet --> messenger; wallet --> network_controller; wallet --> passkey_controller; + wallet --> permission_controller; wallet --> remote_feature_flag_controller; wallet --> seedless_onboarding_controller; wallet --> storage_service; diff --git a/codeowners.ts b/codeowners.ts index db72782dc89..0beca537d6e 100644 --- a/codeowners.ts +++ b/codeowners.ts @@ -14,12 +14,13 @@ type PackageInfo = { teams: string[]; /** - * The package's directory name under - * `/packages/wallet/src/initialization/instances`, used to generate its rule - * in the "Initialization" section. Omit this if the package has not been - * added to the Wallet Library yet. + * The package's directory name(s) under + * `/packages/wallet/src/initialization/instances`, used to generate its rule(s) + * in the "Initialization" section. Pass an array for packages that export more + * than one wired controller. Omit this if the package has not been added to the + * Wallet Library yet. */ - initializationPath?: string; + initializationPath?: string | string[]; }; /** @@ -256,6 +257,10 @@ const PACKAGES: Record = { }, 'permission-controller': { teams: ['@MetaMask/core-platform'], + initializationPath: [ + 'permission-controller', + 'subject-metadata-controller', + ], }, 'permission-log-controller': { teams: ['@MetaMask/core-platform'], @@ -607,16 +612,14 @@ function buildJointTeamOwnershipSection(): CodeownersSection { function buildInitializationSection(): CodeownersSection { return { title: 'Initialization', - rules: Object.keys(PACKAGES) - .filter((name) => PACKAGES[name].initializationPath !== undefined) - .sort() - .map((name) => { - const { teams, initializationPath } = PACKAGES[name]; - return { - pattern: `/packages/wallet/src/initialization/instances/${initializationPath}/`, + rules: Object.values(PACKAGES) + .flatMap(({ teams, initializationPath = [] }) => + [initializationPath].flat().map((instancePath) => ({ + pattern: `/packages/wallet/src/initialization/instances/${instancePath}/`, owners: teams, - }; - }), + })), + ) + .sort((ruleA, ruleB) => ruleA.pattern.localeCompare(ruleB.pattern)), }; } diff --git a/packages/wallet/CHANGELOG.md b/packages/wallet/CHANGELOG.md index de719b4991a..67bcb72e0dd 100644 --- a/packages/wallet/CHANGELOG.md +++ b/packages/wallet/CHANGELOG.md @@ -7,8 +7,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- **BREAKING:** Wire `PermissionController` and `SubjectMetadataController` into the default wallet initialization ([#9300](https://github.com/MetaMask/core/pull/9300)) + - Consumers that pass their own `messenger` and already wire either controller must remove their own before upgrading, or the duplicate action registration will collide. + - Adds optional `instanceOptions.permissionController` (`caveatSpecifications`, `permissionSpecifications`, `unrestrictedMethods`) and `instanceOptions.subjectMetadataController.subjectCacheLimit`. Specifications default to empty, so no permission can be granted until a consumer injects them; `subjectCacheLimit` defaults to `100`. + - Consumers restoring persisted state must restore the `PermissionController` key alongside the `SubjectMetadataController` key, since subject metadata is retained only for origins holding permissions. + - Consumers whose `permissionSpecifications` invoke actions that `PermissionControllerMessenger` does not declare (for example the Snaps `wallet_snap` specifications) must replace the `PermissionController` configuration via `initializationConfigurations`. +- Export the `InitializationConfiguration` type, which a consumer needs in order to supply `initializationConfigurations` ([#9300](https://github.com/MetaMask/core/pull/9300)) + ### Changed +- **BREAKING:** A configuration passed to `initializationConfigurations` that overrides a default is now initialized in that default's position rather than ahead of all defaults, preserving construction-order dependencies between default controllers (e.g. `PermissionController` before `SubjectMetadataController`) ([#9300](https://github.com/MetaMask/core/pull/9300)) + - An override that relied on being constructed before every other default must now account for its default's position; configurations that do not override a default are still initialized first. +- `initialize` now throws when two entries in `initializationConfigurations` share a `name` ([#9300](https://github.com/MetaMask/core/pull/9300)) - Bump `@metamask/network-controller` from `^35.0.0` to `^35.0.1` ([#9758](https://github.com/MetaMask/core/pull/9758)) ## [9.0.0] diff --git a/packages/wallet/package.json b/packages/wallet/package.json index b5e3f192d1f..aa9183e70d4 100644 --- a/packages/wallet/package.json +++ b/packages/wallet/package.json @@ -67,6 +67,7 @@ "@metamask/messenger": "^2.0.0", "@metamask/network-controller": "^35.0.1", "@metamask/passkey-controller": "^3.0.0", + "@metamask/permission-controller": "^13.1.1", "@metamask/remote-feature-flag-controller": "^5.0.0", "@metamask/scure-bip39": "^2.1.1", "@metamask/seedless-onboarding-controller": "^10.1.0", diff --git a/packages/wallet/src/Wallet.test.ts b/packages/wallet/src/Wallet.test.ts index e370c699aa0..1f854225df1 100644 --- a/packages/wallet/src/Wallet.test.ts +++ b/packages/wallet/src/Wallet.test.ts @@ -8,6 +8,9 @@ import { webcrypto } from 'crypto'; import MockEncryptor from '../../keyring-controller/tests/mocks/mockEncryptor.js'; import * as initializationModule from './initialization/initialization.js'; import { AlwaysOnlineAdapter } from './initialization/instances/connectivity-controller/always-online-adapter.js'; +import { subjectMetadataController } from './initialization/instances/subject-metadata-controller/subject-metadata-controller.js'; +import type { InitializationConfiguration } from './initialization/types.js'; +import type { WalletOptions } from './types.js'; import { importSecretRecoveryPhrase } from './utilities.js'; import { Wallet } from './Wallet.js'; @@ -23,8 +26,35 @@ const REMOTE_FEATURE_FLAG_OPTIONS = { }, }; -async function setupWallet(): Promise { - const wallet = new Wallet({ +const PERSISTED_SUBJECT_ORIGIN = 'https://metamask.io'; + +const PERSISTED_PERMISSIONED_SUBJECT_STATE = { + PermissionController: { + subjects: { + [PERSISTED_SUBJECT_ORIGIN]: { + origin: PERSISTED_SUBJECT_ORIGIN, + permissions: { somePermission: {} }, + }, + }, + }, + SubjectMetadataController: { + subjectMetadata: { + [PERSISTED_SUBJECT_ORIGIN]: { + origin: PERSISTED_SUBJECT_ORIGIN, + name: 'MetaMask', + subjectType: null, + extensionId: null, + iconUrl: null, + }, + }, + }, +}; + +function createWallet( + options: Omit = {}, +): Wallet { + return new Wallet({ + ...options, instanceOptions: { connectivityController: { connectivityAdapter: new AlwaysOnlineAdapter(), @@ -41,6 +71,10 @@ async function setupWallet(): Promise { remoteFeatureFlagController: REMOTE_FEATURE_FLAG_OPTIONS, }, }); +} + +async function setupWallet(): Promise { + const wallet = createWallet(); await importSecretRecoveryPhrase(wallet, TEST_PASSWORD, TEST_SRP); @@ -508,4 +542,55 @@ describe('Wallet', () => { ).toStrictEqual({ testFlag: true }); }); }); + + describe('PermissionController', () => { + it('is wired and exposes its state on the wallet messenger', async () => { + const wallet = await setupWallet(); + + expect( + wallet.messenger.call('PermissionController:getState'), + ).toStrictEqual({ subjects: {} }); + }); + }); + + describe('SubjectMetadataController', () => { + it('is wired and exposes its state on the wallet messenger', async () => { + const wallet = await setupWallet(); + + expect( + wallet.messenger.call('SubjectMetadataController:getState'), + ).toStrictEqual({ subjectMetadata: {} }); + }); + + // Hydrating persisted metadata calls `PermissionController:hasPermissions`, + // so construction throws unless `PermissionController` came first. + it.each([ + { + description: 'from the defaults', + initializationConfigurations: undefined, + }, + { + description: 'when overridden', + initializationConfigurations: [ + subjectMetadataController as InitializationConfiguration< + unknown, + unknown + >, + ], + }, + ])( + 'hydrates persisted subject metadata $description, consulting the wired PermissionController for retention', + ({ initializationConfigurations }) => { + const wallet = createWallet({ + initializationConfigurations, + state: PERSISTED_PERMISSIONED_SUBJECT_STATE, + }); + + // The subject holds permissions, so its metadata survives hydration. + expect( + Object.keys(wallet.state.SubjectMetadataController.subjectMetadata), + ).toContain(PERSISTED_SUBJECT_ORIGIN); + }, + ); + }); }); diff --git a/packages/wallet/src/index.ts b/packages/wallet/src/index.ts index 50e9b004160..1c85a0fcfd5 100644 --- a/packages/wallet/src/index.ts +++ b/packages/wallet/src/index.ts @@ -2,6 +2,7 @@ export { Wallet } from './Wallet.js'; export { AlwaysOnlineAdapter } from './initialization/instances/connectivity-controller/always-online-adapter.js'; export { importSecretRecoveryPhrase } from './utilities.js'; export type { WalletOptions } from './types.js'; +export type { InitializationConfiguration } from './initialization/types.js'; export type { DefaultActions, DefaultEvents, diff --git a/packages/wallet/src/initialization/initialization.test.ts b/packages/wallet/src/initialization/initialization.test.ts new file mode 100644 index 00000000000..a6f07b1499f --- /dev/null +++ b/packages/wallet/src/initialization/initialization.test.ts @@ -0,0 +1,213 @@ +import { Messenger } from '@metamask/messenger'; +import { InMemoryStorageAdapter } from '@metamask/storage-service'; + +import type { InstanceSpecificOptions } from '../types.js'; +import type { + DefaultActions, + DefaultEvents, + RootMessenger, +} from './defaults.js'; +import { defaultConfigurations } from './defaults.js'; +import { initialize } from './initialization.js'; +import { AlwaysOnlineAdapter } from './instances/connectivity-controller/always-online-adapter.js'; +import type { InitializationConfiguration } from './types.js'; + +type StubInstance = { state: Record }; + +/** + * The names of every default configuration, in the order `initialize` builds + * them. + * + * @returns The default configuration names. + */ +function getDefaultNames(): string[] { + return Object.values(defaultConfigurations).map((config) => config.name); +} + +/** + * Creates a stand-in configuration that records when it is constructed, so a + * test can assert construction order without building real controllers. + * + * @param name - The instance name. + * @param constructionOrder - An array each `init` call appends its name to. + * @returns A configuration usable as an override or an addition. + */ +function createStubConfiguration( + name: string, + constructionOrder: string[], +): InitializationConfiguration { + return { + name, + getMessenger: (): Messenger => new Messenger({ namespace: name }), + init: (): StubInstance => { + constructionOrder.push(name); + return { state: {} }; + }, + }; +} + +/** + * Builds the required instance options. Every test here overrides all defaults + * with stubs, so these values are never read; they exist to satisfy the type. + * + * @returns The instance options. + */ +function getInstanceOptions(): InstanceSpecificOptions { + return { + connectivityController: { + connectivityAdapter: new AlwaysOnlineAdapter(), + }, + gasFeeController: { clientId: 'test' }, + networkController: { infuraProjectId: 'fake-infura-project-id' }, + storageService: { storage: new InMemoryStorageAdapter() }, + remoteFeatureFlagController: { + clientConfigApiService: { + fetchRemoteFeatureFlags: async () => ({ + remoteFeatureFlags: {}, + cacheTimestamp: 0, + }), + }, + }, + }; +} + +/** + * Creates a root messenger for use in tests. + * + * @returns A root messenger. + */ +function getRootMessenger(): RootMessenger { + return new Messenger({ namespace: 'Root' }); +} + +describe('initialize', () => { + describe('default construction order', () => { + it('exports `PermissionController` before `SubjectMetadataController`', () => { + // Pins the export order of `instances/index.ts`, which the hydration + // dependency relies on and which no lint rule enforces. + const names = getDefaultNames(); + + expect(names.indexOf('PermissionController')).toBeLessThan( + names.indexOf('SubjectMetadataController'), + ); + }); + }); + + describe('overriding configurations', () => { + it('initializes an override in its default position, not in the caller-supplied order', () => { + const constructionOrder: string[] = []; + const defaultNames = getDefaultNames(); + // Reversed, so caller order and default order cannot coincide. + const overrides = [...defaultNames] + .reverse() + .map((name) => createStubConfiguration(name, constructionOrder)); + + initialize({ + messenger: getRootMessenger(), + initializationConfigurations: overrides, + instanceOptions: getInstanceOptions(), + }); + + expect(constructionOrder).toStrictEqual(defaultNames); + }); + + it('keeps `PermissionController` before `SubjectMetadataController` when both are overridden in reverse', () => { + const constructionOrder: string[] = []; + const overrides = [ + createStubConfiguration('SubjectMetadataController', constructionOrder), + createStubConfiguration('PermissionController', constructionOrder), + ]; + + initialize({ + messenger: getRootMessenger(), + initializationConfigurations: [ + ...getDefaultNames() + .filter( + (name) => + name !== 'PermissionController' && + name !== 'SubjectMetadataController', + ) + .map((name) => createStubConfiguration(name, constructionOrder)), + ...overrides, + ], + instanceOptions: getInstanceOptions(), + }); + + expect(constructionOrder.indexOf('PermissionController')).toBeLessThan( + constructionOrder.indexOf('SubjectMetadataController'), + ); + }); + + it('replaces the default instance with the override', () => { + const overridden = { state: { overridden: true } }; + + const instances = initialize({ + messenger: getRootMessenger(), + initializationConfigurations: getDefaultNames().map((name) => ({ + name, + getMessenger: (): Messenger => + new Messenger({ namespace: name }), + init: (): StubInstance => overridden, + })), + instanceOptions: getInstanceOptions(), + }); + + expect(instances.PermissionController).toBe(overridden); + }); + }); + + describe('additional configurations', () => { + it('initializes configurations that do not override a default before the defaults', () => { + const constructionOrder: string[] = []; + + initialize({ + messenger: getRootMessenger(), + initializationConfigurations: [ + ...getDefaultNames().map((name) => + createStubConfiguration(name, constructionOrder), + ), + createStubConfiguration('TestService', constructionOrder), + ], + instanceOptions: getInstanceOptions(), + }); + + expect(constructionOrder[0]).toBe('TestService'); + }); + }); + + describe('duplicate names', () => { + it('throws when two configurations share a name', () => { + const constructionOrder: string[] = []; + + expect(() => + initialize({ + messenger: getRootMessenger(), + initializationConfigurations: [ + createStubConfiguration('PermissionController', constructionOrder), + createStubConfiguration('PermissionController', constructionOrder), + ], + instanceOptions: getInstanceOptions(), + }), + ).toThrow( + 'Duplicate initialization configuration name: PermissionController', + ); + }); + + it('throws before constructing anything', () => { + const constructionOrder: string[] = []; + + expect(() => + initialize({ + messenger: getRootMessenger(), + initializationConfigurations: [ + createStubConfiguration('TestService', constructionOrder), + createStubConfiguration('TestService', constructionOrder), + ], + instanceOptions: getInstanceOptions(), + }), + ).toThrow('Duplicate initialization configuration name: TestService'); + + expect(constructionOrder).toStrictEqual([]); + }); + }); +}); diff --git a/packages/wallet/src/initialization/initialization.ts b/packages/wallet/src/initialization/initialization.ts index 9ed426722de..4d969b2d0b4 100644 --- a/packages/wallet/src/initialization/initialization.ts +++ b/packages/wallet/src/initialization/initialization.ts @@ -25,15 +25,38 @@ export function initialize(options: InitializeOptions): DefaultInstances { instanceOptions, } = options; - const overriddenConfiguration = initializationConfigurations.map( - (config) => config.name, - ); + const defaultConfigurationEntries = Object.values( + defaultConfigurations, + ) as InitializationConfiguration[]; + + // Resolving two configurations with the same name would mean silently + // discarding one, so reject it instead of picking a winner. + const seenNames = new Set(); + for (const { name } of initializationConfigurations) { + if (seenNames.has(name)) { + throw new Error(`Duplicate initialization configuration name: ${name}`); + } + seenNames.add(name); + } - const configurationEntries = initializationConfigurations.concat( - Object.values(defaultConfigurations).filter( - (config) => !overriddenConfiguration.includes(config.name), - ) as InitializationConfiguration[], + const defaultNames = new Set( + defaultConfigurationEntries.map((config) => config.name), ); + const overridesByName = new Map( + initializationConfigurations.map((config) => [config.name, config]), + ); + + const configurationEntries = [ + // A configuration that does not override a default is additive, and runs + // before the defaults — a default may depend on an action it registers. + ...initializationConfigurations.filter( + (config) => !defaultNames.has(config.name), + ), + // An override takes its default's slot; see `instances/index.ts`. + ...defaultConfigurationEntries.map( + (config) => overridesByName.get(config.name) ?? config, + ), + ]; const instances: Record = {}; diff --git a/packages/wallet/src/initialization/instances/index.ts b/packages/wallet/src/initialization/instances/index.ts index d45bb0917b2..bf31342e052 100644 --- a/packages/wallet/src/initialization/instances/index.ts +++ b/packages/wallet/src/initialization/instances/index.ts @@ -1,3 +1,7 @@ +// `initialize` constructs defaults in this order, and some depend on an earlier +// one (`PermissionController` before `SubjectMetadataController`). Keep it +// alphabetical: the ESM build sorts namespace keys per spec, but the CommonJS +// build preserves the order below, so only declaration order holds for both. export { accountsController } from './accounts-controller/accounts-controller.js'; export { addressBookController } from './address-book-controller/address-book-controller.js'; export { approvalController } from './approval-controller/approval-controller.js'; @@ -6,7 +10,9 @@ export { gasFeeController } from './gas-fee-controller/gas-fee-controller.js'; export { keyringController } from './keyring-controller/keyring-controller.js'; export { networkController } from './network-controller/network-controller.js'; export { passkeyController } from './passkey-controller/passkey-controller.js'; +export { permissionController } from './permission-controller/permission-controller.js'; export { remoteFeatureFlagController } from './remote-feature-flag-controller/remote-feature-flag-controller.js'; export { seedlessOnboardingController } from './seedless-onboarding-controller/seedless-onboarding-controller.js'; export { storageService } from './storage-service/storage-service.js'; +export { subjectMetadataController } from './subject-metadata-controller/subject-metadata-controller.js'; export { transactionController } from './transaction-controller/transaction-controller.js'; diff --git a/packages/wallet/src/initialization/instances/permission-controller/permission-controller.test.ts b/packages/wallet/src/initialization/instances/permission-controller/permission-controller.test.ts new file mode 100644 index 00000000000..d73f57a074e --- /dev/null +++ b/packages/wallet/src/initialization/instances/permission-controller/permission-controller.test.ts @@ -0,0 +1,241 @@ +import { Messenger } from '@metamask/messenger'; +import { + PermissionController, + PermissionType, +} from '@metamask/permission-controller'; + +import { defaultConfigurations } from '../../defaults.js'; +import type { + DefaultActions, + DefaultEvents, + RootMessenger, +} from '../../defaults.js'; +import { permissionController } from './permission-controller.js'; + +/** + * Creates a root messenger for use in tests. + * + * @returns A root messenger. + */ +function getRootMessenger(): RootMessenger { + return new Messenger({ namespace: 'Root' }); +} + +describe('permissionController', () => { + it('is registered as a default initialization configuration', () => { + expect(Object.values(defaultConfigurations)).toContain( + permissionController, + ); + }); + + it('initializes a PermissionController with default state and no permissions registered', () => { + const messenger = permissionController.getMessenger(getRootMessenger()); + + const instance = permissionController.init({ + state: undefined, + messenger, + options: {}, + }); + + expect(instance).toBeInstanceOf(PermissionController); + expect(instance.state).toStrictEqual({ subjects: {} }); + expect(instance.unrestrictedMethods).toStrictEqual(new Set()); + }); + + it('forwards the provided state to the controller', () => { + const messenger = permissionController.getMessenger(getRootMessenger()); + + const subjects = { + 'https://metamask.io': { + origin: 'https://metamask.io', + permissions: {}, + }, + }; + + const instance = permissionController.init({ + state: { subjects }, + messenger, + options: {}, + }); + + expect(instance.state.subjects).toStrictEqual(subjects); + }); + + it('forwards injected unrestrictedMethods to the controller', () => { + const messenger = permissionController.getMessenger(getRootMessenger()); + + const instance = permissionController.init({ + state: undefined, + messenger, + options: { unrestrictedMethods: ['eth_chainId', 'eth_blockNumber'] }, + }); + + expect(instance.hasUnrestrictedMethod('eth_chainId')).toBe(true); + expect(instance.hasUnrestrictedMethod('eth_sendTransaction')).toBe(false); + }); + + it('forwards injected permission specifications to the controller', () => { + const messenger = permissionController.getMessenger(getRootMessenger()); + const origin = 'https://metamask.io'; + + const instance = permissionController.init({ + state: undefined, + messenger, + options: { + permissionSpecifications: { + wallet_noop: { + permissionType: PermissionType.RestrictedMethod, + targetName: 'wallet_noop', + allowedCaveats: null, + methodImplementation: () => null, + }, + }, + }, + }); + + // Granting the injected permission only succeeds if its specification was + // forwarded to the controller; an unknown target would throw. + instance.grantPermissions({ + subject: { origin }, + approvedPermissions: { wallet_noop: {} }, + }); + + expect(instance.getPermissions(origin)).toHaveProperty('wallet_noop'); + }); + + it('forwards injected caveat specifications to the controller', () => { + const messenger = permissionController.getMessenger(getRootMessenger()); + const origin = 'https://metamask.io'; + + const instance = permissionController.init({ + state: undefined, + messenger, + options: { + caveatSpecifications: { + noopCaveat: { + type: 'noopCaveat', + decorator: (methodImplementation) => methodImplementation, + validator: ({ value }) => { + if (value !== 'allowed') { + throw new Error('Invalid noopCaveat value'); + } + }, + }, + }, + permissionSpecifications: { + wallet_noop: { + permissionType: PermissionType.RestrictedMethod, + targetName: 'wallet_noop', + allowedCaveats: ['noopCaveat'], + methodImplementation: () => null, + }, + }, + }, + }); + + // The injected caveat's validator only runs if its specification was + // forwarded to the controller. + expect(() => + instance.grantPermissions({ + subject: { origin }, + approvedPermissions: { + wallet_noop: { + caveats: [{ type: 'noopCaveat', value: 'rejected' }], + }, + }, + }), + ).toThrow('Invalid noopCaveat value'); + + instance.grantPermissions({ + subject: { origin }, + approvedPermissions: { + wallet_noop: { caveats: [{ type: 'noopCaveat', value: 'allowed' }] }, + }, + }); + + expect( + instance.getCaveat(origin, 'wallet_noop', 'noopCaveat'), + ).toStrictEqual({ type: 'noopCaveat', value: 'allowed' }); + }); + + it('exposes its actions through the root messenger', () => { + const rootMessenger = getRootMessenger(); + const messenger = permissionController.getMessenger(rootMessenger); + + permissionController.init({ state: undefined, messenger, options: {} }); + + expect(rootMessenger.call('PermissionController:getState')).toStrictEqual({ + subjects: {}, + }); + }); + + it('has every external action it declares delegated to its messenger', () => { + const messenger = permissionController.getMessenger(getRootMessenger()); + + // A delegated action with no handler reports "has not been registered"; + // one missing from the allowlist reports "has not been delegated to". A + // dropped entry still type-checks, so only this distinction catches it. + expect(() => + messenger.call( + 'ApprovalController:addRequest', + { origin: 'https://metamask.io', type: 'test' }, + false, + ), + ).toThrow('has not been registered'); + expect(() => + messenger.call('ApprovalController:hasRequest', { id: 'test-id' }), + ).toThrow('has not been registered'); + expect(() => + messenger.call('ApprovalController:acceptRequest', 'test-id'), + ).toThrow('has not been registered'); + expect(() => + messenger.call( + 'ApprovalController:rejectRequest', + 'test-id', + new Error('rejected'), + ), + ).toThrow('has not been registered'); + expect(() => + messenger.call( + 'SubjectMetadataController:getSubjectMetadata', + 'https://metamask.io', + ), + ).toThrow('has not been registered'); + }); + + it('can reach the actions delegated to its messenger', () => { + const rootMessenger = getRootMessenger(); + + // Register stub handlers as the real ApprovalController and + // SubjectMetadataController would, then confirm the PermissionController's + // messenger can call them — proving the delegation allowlist is wired. + const approvalControllerMessenger = new Messenger({ + namespace: 'ApprovalController', + parent: rootMessenger, + }); + approvalControllerMessenger.registerActionHandler( + 'ApprovalController:hasRequest', + () => true, + ); + const subjectMetadataControllerMessenger = new Messenger({ + namespace: 'SubjectMetadataController', + parent: rootMessenger, + }); + subjectMetadataControllerMessenger.registerActionHandler( + 'SubjectMetadataController:getSubjectMetadata', + () => undefined, + ); + + const messenger = permissionController.getMessenger(rootMessenger); + + expect(messenger.call('ApprovalController:hasRequest', { id: 'x' })).toBe( + true, + ); + expect( + messenger.call( + 'SubjectMetadataController:getSubjectMetadata', + 'https://metamask.io', + ), + ).toBeUndefined(); + }); +}); diff --git a/packages/wallet/src/initialization/instances/permission-controller/permission-controller.ts b/packages/wallet/src/initialization/instances/permission-controller/permission-controller.ts new file mode 100644 index 00000000000..07c2eba0138 --- /dev/null +++ b/packages/wallet/src/initialization/instances/permission-controller/permission-controller.ts @@ -0,0 +1,56 @@ +import { Messenger } from '@metamask/messenger'; +import { + CaveatSpecificationConstraint, + GenericPermissionController, + PermissionController, + PermissionControllerMessenger, + PermissionSpecificationConstraint, +} from '@metamask/permission-controller'; + +import type { + DefaultActions, + DefaultEvents, + RootMessenger, +} from '../../defaults.js'; +import type { InitializationConfiguration } from '../../types.js'; + +export const permissionController: InitializationConfiguration< + GenericPermissionController, + PermissionControllerMessenger +> = { + name: 'PermissionController', + init: ({ state, messenger, options }) => + new PermissionController< + PermissionSpecificationConstraint, + CaveatSpecificationConstraint + >({ + state, + messenger, + caveatSpecifications: options.caveatSpecifications ?? {}, + permissionSpecifications: options.permissionSpecifications ?? {}, + unrestrictedMethods: options.unrestrictedMethods ?? [], + }), + getMessenger: (parent: RootMessenger) => { + const permissionControllerMessenger: PermissionControllerMessenger = + new Messenger({ + namespace: 'PermissionController', + parent, + }); + + // Snaps specifications reach `SnapController:*` through side effects, which + // `DefaultActions` does not declare — so widening this allowlist means + // replacing the whole configuration, not appending to it. + parent.delegate({ + messenger: permissionControllerMessenger, + actions: [ + 'ApprovalController:addRequest', + 'ApprovalController:hasRequest', + 'ApprovalController:acceptRequest', + 'ApprovalController:rejectRequest', + 'SubjectMetadataController:getSubjectMetadata', + ], + }); + + return permissionControllerMessenger; + }, +}; diff --git a/packages/wallet/src/initialization/instances/permission-controller/types.ts b/packages/wallet/src/initialization/instances/permission-controller/types.ts new file mode 100644 index 00000000000..f615cc6343e --- /dev/null +++ b/packages/wallet/src/initialization/instances/permission-controller/types.ts @@ -0,0 +1,35 @@ +import type { + CaveatSpecificationConstraint, + PermissionControllerOptions, + PermissionSpecificationConstraint, +} from '@metamask/permission-controller'; + +type GenericPermissionControllerOptions = PermissionControllerOptions< + PermissionSpecificationConstraint, + CaveatSpecificationConstraint +>; + +/** + * Per-instance options for the wallet's `PermissionController`. + * + * The permission and caveat specifications define which permissions exist and + * how their caveats behave; they vary substantially between clients (CAIP-25 + * account permissions, Snaps endowments and restricted methods, etc.), as does + * the set of unrestricted JSON-RPC methods. They are therefore injected rather + * than hardcoded. Each field is optional and defaults to an empty set, so the + * controller initializes with no permissions registered. + */ +export type PermissionControllerInstanceOptions = { + /** + * Specifications of all caveats available to the controller. + */ + caveatSpecifications?: GenericPermissionControllerOptions['caveatSpecifications']; + /** + * Specifications of all permissions available to the controller. + */ + permissionSpecifications?: GenericPermissionControllerOptions['permissionSpecifications']; + /** + * Names of all JSON-RPC methods that bypass permission gating. + */ + unrestrictedMethods?: GenericPermissionControllerOptions['unrestrictedMethods']; +}; diff --git a/packages/wallet/src/initialization/instances/subject-metadata-controller/subject-metadata-controller.test.ts b/packages/wallet/src/initialization/instances/subject-metadata-controller/subject-metadata-controller.test.ts new file mode 100644 index 00000000000..11ad83032d0 --- /dev/null +++ b/packages/wallet/src/initialization/instances/subject-metadata-controller/subject-metadata-controller.test.ts @@ -0,0 +1,169 @@ +import { Messenger } from '@metamask/messenger'; +import { SubjectMetadataController } from '@metamask/permission-controller'; + +import { defaultConfigurations } from '../../defaults.js'; +import type { + DefaultActions, + DefaultEvents, + RootMessenger, +} from '../../defaults.js'; +import { subjectMetadataController } from './subject-metadata-controller.js'; + +/** + * Creates a root messenger for use in tests. + * + * @returns A root messenger. + */ +function getRootMessenger(): RootMessenger { + return new Messenger({ namespace: 'Root' }); +} + +/** + * Registers a stub `PermissionController:hasPermissions` handler on the bus, as + * the real `PermissionController` would. `SubjectMetadataController` calls this + * action when hydrating from state and when trimming its cache. + * + * @param rootMessenger - The root messenger to register the handler on. + * @param hasPermissions - The value the stub returns for any origin. + */ +function registerHasPermissionsStub( + rootMessenger: RootMessenger, + hasPermissions: boolean, +): void { + const permissionControllerMessenger = new Messenger({ + namespace: 'PermissionController', + parent: rootMessenger, + }); + + permissionControllerMessenger.registerActionHandler( + 'PermissionController:hasPermissions', + () => hasPermissions, + ); +} + +describe('subjectMetadataController', () => { + it('is registered as a default initialization configuration', () => { + expect(Object.values(defaultConfigurations)).toContain( + subjectMetadataController, + ); + }); + + it('initializes a SubjectMetadataController with default state', () => { + const messenger = + subjectMetadataController.getMessenger(getRootMessenger()); + + const instance = subjectMetadataController.init({ + state: undefined, + messenger, + options: {}, + }); + + expect(instance).toBeInstanceOf(SubjectMetadataController); + expect(instance.state).toStrictEqual({ subjectMetadata: {} }); + }); + + it('forwards the provided state to the controller', () => { + const rootMessenger = getRootMessenger(); + // Subjects retained on hydration only when they still hold permissions. + registerHasPermissionsStub(rootMessenger, true); + const messenger = subjectMetadataController.getMessenger(rootMessenger); + + const subjectMetadata = { + 'https://metamask.io': { + origin: 'https://metamask.io', + name: 'MetaMask', + subjectType: null, + extensionId: null, + iconUrl: null, + }, + }; + + const instance = subjectMetadataController.init({ + state: { subjectMetadata }, + messenger, + options: {}, + }); + + expect(instance.state.subjectMetadata).toStrictEqual(subjectMetadata); + }); + + it('exposes its actions through the root messenger', () => { + const rootMessenger = getRootMessenger(); + const messenger = subjectMetadataController.getMessenger(rootMessenger); + + subjectMetadataController.init({ + state: undefined, + messenger, + options: {}, + }); + + expect( + rootMessenger.call('SubjectMetadataController:getState'), + ).toStrictEqual({ subjectMetadata: {} }); + }); + + it('forwards a custom subjectCacheLimit, evicting the oldest permissionless subject', () => { + const rootMessenger = getRootMessenger(); + registerHasPermissionsStub(rootMessenger, false); + const messenger = subjectMetadataController.getMessenger(rootMessenger); + + const instance = subjectMetadataController.init({ + state: undefined, + messenger, + options: { subjectCacheLimit: 1 }, + }); + + instance.addSubjectMetadata({ origin: 'https://a.example' }); + instance.addSubjectMetadata({ origin: 'https://b.example' }); + + expect(Object.keys(instance.state.subjectMetadata)).toStrictEqual([ + 'https://b.example', + ]); + }); + + it('retains a subject with permissions even when the cache limit is exceeded', () => { + const rootMessenger = getRootMessenger(); + // Every subject reports as holding permissions. + registerHasPermissionsStub(rootMessenger, true); + const messenger = subjectMetadataController.getMessenger(rootMessenger); + + const instance = subjectMetadataController.init({ + state: undefined, + messenger, + options: { subjectCacheLimit: 1 }, + }); + + instance.addSubjectMetadata({ origin: 'https://a.example' }); + instance.addSubjectMetadata({ origin: 'https://b.example' }); + + // Metadata for subjects with permissions is never evicted. + expect(Object.keys(instance.state.subjectMetadata)).toStrictEqual([ + 'https://a.example', + 'https://b.example', + ]); + }); + + it('does not evict until the default cache limit of 100 is exceeded', () => { + const rootMessenger = getRootMessenger(); + registerHasPermissionsStub(rootMessenger, false); + const messenger = subjectMetadataController.getMessenger(rootMessenger); + + const instance = subjectMetadataController.init({ + state: undefined, + messenger, + options: {}, + }); + + for (let index = 0; index < 100; index++) { + instance.addSubjectMetadata({ origin: `https://${index}.example` }); + } + expect(Object.keys(instance.state.subjectMetadata)).toHaveLength(100); + + // The 101st permissionless subject evicts the oldest (FIFO). + instance.addSubjectMetadata({ origin: 'https://overflow.example' }); + const origins = Object.keys(instance.state.subjectMetadata); + expect(origins).toHaveLength(100); + expect(origins).not.toContain('https://0.example'); + expect(origins).toContain('https://overflow.example'); + }); +}); diff --git a/packages/wallet/src/initialization/instances/subject-metadata-controller/subject-metadata-controller.ts b/packages/wallet/src/initialization/instances/subject-metadata-controller/subject-metadata-controller.ts new file mode 100644 index 00000000000..49c26eab602 --- /dev/null +++ b/packages/wallet/src/initialization/instances/subject-metadata-controller/subject-metadata-controller.ts @@ -0,0 +1,45 @@ +import { Messenger } from '@metamask/messenger'; +import { + SubjectMetadataController, + SubjectMetadataControllerMessenger, +} from '@metamask/permission-controller'; + +import type { + DefaultActions, + DefaultEvents, + RootMessenger, +} from '../../defaults.js'; +import type { InitializationConfiguration } from '../../types.js'; + +// `100` matches the value both the extension and mobile use. +const DEFAULT_SUBJECT_CACHE_LIMIT = 100; + +export const subjectMetadataController: InitializationConfiguration< + SubjectMetadataController, + SubjectMetadataControllerMessenger +> = { + name: 'SubjectMetadataController', + // Hydrating persisted metadata calls `PermissionController:hasPermissions`; + // see the ordering note in `instances/index.ts`. + init: ({ state, messenger, options }) => + new SubjectMetadataController({ + state, + messenger, + subjectCacheLimit: + options.subjectCacheLimit ?? DEFAULT_SUBJECT_CACHE_LIMIT, + }), + getMessenger: (parent: RootMessenger) => { + const subjectMetadataControllerMessenger: SubjectMetadataControllerMessenger = + new Messenger({ + namespace: 'SubjectMetadataController', + parent, + }); + + parent.delegate({ + messenger: subjectMetadataControllerMessenger, + actions: ['PermissionController:hasPermissions'], + }); + + return subjectMetadataControllerMessenger; + }, +}; diff --git a/packages/wallet/src/initialization/instances/subject-metadata-controller/types.ts b/packages/wallet/src/initialization/instances/subject-metadata-controller/types.ts new file mode 100644 index 00000000000..6032f4aba9d --- /dev/null +++ b/packages/wallet/src/initialization/instances/subject-metadata-controller/types.ts @@ -0,0 +1,12 @@ +/** + * Per-instance options for the wallet's `SubjectMetadataController`. + */ +export type SubjectMetadataControllerInstanceOptions = { + /** + * Maximum number of distinct permissionless subjects (origins) to retain + * metadata for, evicted oldest-first once exceeded. Subjects that hold + * permissions are never evicted. Defaults to `100`. Must be a positive + * integer or the controller throws, failing wallet construction. + */ + subjectCacheLimit?: number; +}; diff --git a/packages/wallet/src/types.ts b/packages/wallet/src/types.ts index 978bf1b1afd..25321bf4dfd 100644 --- a/packages/wallet/src/types.ts +++ b/packages/wallet/src/types.ts @@ -11,9 +11,11 @@ import type { GasFeeControllerInstanceOptions } from './initialization/instances import type { KeyringControllerInstanceOptions } from './initialization/instances/keyring-controller/types.js'; import type { NetworkControllerInstanceOptions } from './initialization/instances/network-controller/types.js'; import type { PasskeyControllerInstanceOptions } from './initialization/instances/passkey-controller/types.js'; +import type { PermissionControllerInstanceOptions } from './initialization/instances/permission-controller/types.js'; import type { RemoteFeatureFlagControllerInstanceOptions } from './initialization/instances/remote-feature-flag-controller/types.js'; import type { SeedlessOnboardingControllerInstanceOptions } from './initialization/instances/seedless-onboarding-controller/types.js'; import type { StorageServiceInstanceOptions } from './initialization/instances/storage-service/types.js'; +import type { SubjectMetadataControllerInstanceOptions } from './initialization/instances/subject-metadata-controller/types.js'; import type { TransactionControllerInstanceOptions } from './initialization/instances/transaction-controller/types.js'; import type { InitializationConfiguration } from './initialization/types.js'; @@ -33,8 +35,10 @@ export type InstanceSpecificOptions = { gasFeeController: GasFeeControllerInstanceOptions; keyringController?: KeyringControllerInstanceOptions; networkController: NetworkControllerInstanceOptions; + permissionController?: PermissionControllerInstanceOptions; remoteFeatureFlagController: RemoteFeatureFlagControllerInstanceOptions; storageService: StorageServiceInstanceOptions; + subjectMetadataController?: SubjectMetadataControllerInstanceOptions; transactionController?: TransactionControllerInstanceOptions; passkeyController?: PasskeyControllerInstanceOptions; seedlessOnboardingController?: SeedlessOnboardingControllerInstanceOptions; diff --git a/packages/wallet/tsconfig.build.json b/packages/wallet/tsconfig.build.json index dae6d88285a..dadece731c6 100644 --- a/packages/wallet/tsconfig.build.json +++ b/packages/wallet/tsconfig.build.json @@ -17,6 +17,7 @@ { "path": "../messenger/tsconfig.build.json" }, { "path": "../network-controller/tsconfig.build.json" }, { "path": "../passkey-controller/tsconfig.build.json" }, + { "path": "../permission-controller/tsconfig.build.json" }, { "path": "../remote-feature-flag-controller/tsconfig.build.json" }, { "path": "../seedless-onboarding-controller/tsconfig.build.json" }, { "path": "../storage-service/tsconfig.build.json" }, diff --git a/packages/wallet/tsconfig.json b/packages/wallet/tsconfig.json index 10221177468..7e3cb6d8ba6 100644 --- a/packages/wallet/tsconfig.json +++ b/packages/wallet/tsconfig.json @@ -37,6 +37,9 @@ { "path": "../passkey-controller" }, + { + "path": "../permission-controller" + }, { "path": "../remote-feature-flag-controller" }, diff --git a/yarn.lock b/yarn.lock index 011f96c52aa..44efb4eeae7 100644 --- a/yarn.lock +++ b/yarn.lock @@ -9543,6 +9543,7 @@ __metadata: "@metamask/messenger": "npm:^2.0.0" "@metamask/network-controller": "npm:^35.0.1" "@metamask/passkey-controller": "npm:^3.0.0" + "@metamask/permission-controller": "npm:^13.1.1" "@metamask/remote-feature-flag-controller": "npm:^5.0.0" "@metamask/scure-bip39": "npm:^2.1.1" "@metamask/seedless-onboarding-controller": "npm:^10.1.0"