Skip to content

Commit 324fd45

Browse files
FrederikBoldingmetamaskbotMrtenz
authored
fix: Use withKeyring for accessing keyring entropy for Snaps (#40296)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** Use `withKeyring` instead of `getKeyringsByType` for accessing the mnemonic for Snaps. `withKeyring` has more guarantees that should prevent race conditions and `getKeyringsByType` is deprecated for that same reason. Additionally, move `getMnemonic` and `getMnemonicSeed` to utility functions that can be re-used. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/40296?quickstart=1) ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: null <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches keyring entropy retrieval paths used by Snaps; incorrect assumptions about keyring shape/types could break Snap initialization or restricted methods despite being a scoped refactor. > > **Overview** > Refactors Snap entropy access to use `KeyringController:withKeyring` instead of the deprecated `getKeyringsByType`, reducing the chance of races when retrieving the primary HD keyring’s mnemonic/seed. > > The mnemonic/seed lookup logic is centralized into new reusable utilities (`controllers/permissions/snaps/utils.ts`) and wired into both Snap permission specifications and `SnapController` initialization; legacy `metamask-controller` helpers/tests for primary keyring mnemonic access are removed. Dependency/policy updates add a direct `@metamask/eth-hd-keyring` dependency and adjust LavaMoat policies accordingly. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit b8717b6. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: MetaMask Bot <metamaskbot@users.noreply.github.com> Co-authored-by: Maarten Zuidhoorn <maarten@zuidhoorn.com>
1 parent cb1f3f9 commit 324fd45

File tree

20 files changed

+348
-360
lines changed

20 files changed

+348
-360
lines changed

app/scripts/controller-init/messengers/snaps/snap-controller-messenger.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,9 @@ import {
3636
UpdateRequestState,
3737
} from '@metamask/approval-controller';
3838
import {
39-
KeyringControllerGetKeyringsByTypeAction,
4039
KeyringControllerLockEvent,
4140
KeyringControllerUnlockEvent,
41+
KeyringControllerWithKeyringAction,
4242
} from '@metamask/keyring-controller';
4343
import { SelectedNetworkControllerGetNetworkClientIdForDomainAction } from '@metamask/selected-network-controller';
4444
import { NetworkControllerGetNetworkClientByIdAction } from '@metamask/network-controller';
@@ -166,7 +166,7 @@ export function getSnapControllerMessenger(
166166
}
167167

168168
type InitActions =
169-
| KeyringControllerGetKeyringsByTypeAction
169+
| KeyringControllerWithKeyringAction
170170
| PreferencesControllerGetStateAction
171171
| MetaMetricsControllerTrackEventAction
172172
| SetClientActive
@@ -203,7 +203,7 @@ export function getSnapControllerInitMessenger(
203203
messenger.delegate({
204204
messenger: controllerInitMessenger,
205205
actions: [
206-
'KeyringController:getKeyringsByType',
206+
'KeyringController:withKeyring',
207207
'PreferencesController:getState',
208208
'MetaMetricsController:trackEvent',
209209
'SnapController:setClientActive',

app/scripts/controller-init/snaps/snap-controller-init.ts

Lines changed: 3 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,19 @@
11
import { SnapController } from '@metamask/snaps-controllers';
2-
import { createDeferredPromise, hasProperty, Json } from '@metamask/utils';
2+
import { createDeferredPromise, Json } from '@metamask/utils';
33
import { ControllerInitFunction } from '../types';
44
import {
55
EndowmentPermissions,
66
ExcludedSnapEndowments,
77
ExcludedSnapPermissions,
88
} from '../../../../shared/constants/snaps/permissions';
99
import { encryptorFactory } from '../../lib/encryptor-factory';
10-
import { KeyringType } from '../../../../shared/constants/keyring';
1110
import {
1211
SnapControllerInitMessenger,
1312
SnapControllerMessenger,
1413
} from '../messengers/snaps';
1514
import { getBooleanFlag } from '../../lib/util';
1615
import { OnboardingControllerState } from '../../controllers/onboarding';
16+
import { getMnemonicSeed } from '../../controllers/permissions/snaps/utils';
1717

1818
// Copied from `@metamask/snaps-controllers`, since it is not exported.
1919
type TrackingEventPayload = {
@@ -61,23 +61,6 @@ export const SnapControllerInit: ControllerInitFunction<
6161
);
6262
///: END:ONLY_INCLUDE_IF
6363

64-
async function getMnemonicSeed() {
65-
const keyrings = initMessenger.call(
66-
'KeyringController:getKeyringsByType',
67-
KeyringType.hdKeyTree,
68-
);
69-
70-
if (
71-
!keyrings[0] ||
72-
!hasProperty(keyrings[0], 'seed') ||
73-
!(keyrings[0].seed instanceof Uint8Array)
74-
) {
75-
throw new Error('Primary keyring mnemonic unavailable.');
76-
}
77-
78-
return keyrings[0].seed;
79-
}
80-
8164
/**
8265
* Get the feature flags for the `SnapController.
8366
*
@@ -150,7 +133,7 @@ export const SnapControllerInit: ControllerInitFunction<
150133
// TODO: Look into the type mismatch.
151134
encryptor: encryptorFactory(600_000),
152135

153-
getMnemonicSeed,
136+
getMnemonicSeed: getMnemonicSeed.bind(null, initMessenger, undefined),
154137

155138
preinstalledSnaps,
156139
getFeatureFlags,

app/scripts/controllers/permissions/snaps/specifications.ts

Lines changed: 3 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import {
1919
KeyringTypes,
2020
KeyringMetadata,
2121
} from '@metamask/keyring-controller';
22-
import { hasProperty } from '@metamask/utils';
2322
import {
2423
RateLimitControllerCallApiAction,
2524
RateLimitedApiMap,
@@ -33,6 +32,7 @@ import { PreferencesControllerGetStateAction } from '../../preferences-controlle
3332
import { KeyringType } from '../../../../../shared/constants/keyring';
3433
import { AppStateControllerGetUnlockPromiseAction } from '../../app-state-controller';
3534
import { RootMessenger } from '../../../lib/messenger';
35+
import { getMnemonic, getMnemonicSeed } from './utils';
3636

3737
export type SnapPermissionSpecificationsActions =
3838
| AppStateControllerGetUnlockPromiseAction
@@ -122,99 +122,8 @@ export function getSnapPermissionSpecifications(
122122
'SnapController:clearSnapState',
123123
),
124124

125-
/**
126-
* Get the mnemonic for a given entropy source. If no source is
127-
* provided, the primary HD keyring's mnemonic will be returned.
128-
*
129-
* @param source - The ID of the entropy source keyring.
130-
* @returns The mnemonic.
131-
*/
132-
getMnemonic: async (source: string) => {
133-
if (!source) {
134-
const [keyring] = messenger.call(
135-
'KeyringController:getKeyringsByType',
136-
KeyringType.hdKeyTree,
137-
) as { mnemonic?: string }[];
138-
139-
if (!keyring.mnemonic) {
140-
throw new Error('Primary keyring mnemonic unavailable.');
141-
}
142-
143-
return keyring.mnemonic;
144-
}
145-
146-
try {
147-
const { type, mnemonic } = (await messenger.call(
148-
'KeyringController:withKeyring',
149-
{
150-
id: source,
151-
},
152-
async ({ keyring }) => ({
153-
type: keyring.type,
154-
mnemonic: hasProperty(keyring, 'mnemonic')
155-
? keyring.mnemonic
156-
: undefined,
157-
}),
158-
)) as { type: string; mnemonic?: string };
159-
160-
if (type !== KeyringTypes.hd || !mnemonic) {
161-
// The keyring isn't guaranteed to have a mnemonic (e.g.,
162-
// hardware wallets, which can't be used as entropy sources),
163-
// so we throw an error if it doesn't.
164-
throw new Error(`Entropy source with ID "${source}" not found.`);
165-
}
166-
167-
return mnemonic;
168-
} catch {
169-
throw new Error(`Entropy source with ID "${source}" not found.`);
170-
}
171-
},
172-
173-
/**
174-
* Get the mnemonic seed for a given entropy source. If no source is
175-
* provided, the primary HD keyring's mnemonic seed will be returned.
176-
*
177-
* @param source - The ID of the entropy source keyring.
178-
* @returns The mnemonic seed.
179-
*/
180-
getMnemonicSeed: async (source: string) => {
181-
if (!source) {
182-
const [keyring] = messenger.call(
183-
'KeyringController:getKeyringsByType',
184-
KeyringType.hdKeyTree,
185-
) as { seed?: Uint8Array }[];
186-
187-
if (!keyring.seed) {
188-
throw new Error('Primary keyring mnemonic unavailable.');
189-
}
190-
191-
return keyring.seed;
192-
}
193-
194-
try {
195-
const { type, seed } = (await messenger.call(
196-
'KeyringController:withKeyring',
197-
{
198-
id: source,
199-
},
200-
async ({ keyring }) => ({
201-
type: keyring.type,
202-
seed: hasProperty(keyring, 'seed') ? keyring.seed : undefined,
203-
}),
204-
)) as { type: string; seed?: Uint8Array };
205-
206-
if (type !== KeyringTypes.hd || !seed) {
207-
// The keyring isn't guaranteed to have a mnemonic (e.g.,
208-
// hardware wallets, which can't be used as entropy sources),
209-
// so we throw an error if it doesn't.
210-
throw new Error(`Entropy source with ID "${source}" not found.`);
211-
}
212-
213-
return seed;
214-
} catch {
215-
throw new Error(`Entropy source with ID "${source}" not found.`);
216-
}
217-
},
125+
getMnemonic: getMnemonic.bind(null, messenger),
126+
getMnemonicSeed: getMnemonicSeed.bind(null, messenger),
218127

219128
getUnlockPromise: messenger.call.bind(
220129
messenger,
Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
import {
2+
KeyringControllerWithKeyringAction,
3+
KeyringTypes,
4+
} from '@metamask/keyring-controller';
5+
import type { HdKeyring } from '@metamask/eth-hd-keyring';
6+
import { RootMessenger } from '../../../lib/messenger';
7+
8+
/**
9+
* Get the mnemonic for a given entropy source. If no source is
10+
* provided, the primary HD keyring's mnemonic will be returned.
11+
*
12+
* @param messenger - The messenger.
13+
* @param source - The ID of the entropy source keyring.
14+
* @returns The mnemonic.
15+
*/
16+
export async function getMnemonic(
17+
messenger: RootMessenger<KeyringControllerWithKeyringAction, never>,
18+
source?: string | undefined,
19+
): Promise<Uint8Array> {
20+
if (!source) {
21+
const mnemonic = (await messenger.call(
22+
'KeyringController:withKeyring',
23+
{
24+
type: KeyringTypes.hd,
25+
index: 0,
26+
},
27+
async ({ keyring }) => (keyring as HdKeyring).mnemonic,
28+
)) as Uint8Array | null;
29+
30+
if (!mnemonic) {
31+
throw new Error('Primary keyring mnemonic unavailable.');
32+
}
33+
34+
return mnemonic;
35+
}
36+
37+
try {
38+
const keyringData = await messenger.call(
39+
'KeyringController:withKeyring',
40+
{
41+
id: source,
42+
},
43+
async ({ keyring }) => ({
44+
type: keyring.type,
45+
mnemonic: (keyring as HdKeyring).mnemonic,
46+
}),
47+
);
48+
49+
const { type, mnemonic } = keyringData as {
50+
type: string;
51+
mnemonic?: Uint8Array;
52+
};
53+
54+
if (type !== KeyringTypes.hd || !mnemonic) {
55+
// The keyring isn't guaranteed to have a mnemonic (e.g.,
56+
// hardware wallets, which can't be used as entropy sources),
57+
// so we throw an error if it doesn't.
58+
throw new Error(`Entropy source with ID "${source}" not found.`);
59+
}
60+
61+
return mnemonic;
62+
} catch {
63+
throw new Error(`Entropy source with ID "${source}" not found.`);
64+
}
65+
}
66+
67+
/**
68+
* Get the mnemonic seed for a given entropy source. If no source is
69+
* provided, the primary HD keyring's mnemonic seed will be returned.
70+
*
71+
* @param messenger - The messenger.
72+
* @param source - The ID of the entropy source keyring.
73+
* @returns The mnemonic seed.
74+
*/
75+
export async function getMnemonicSeed(
76+
messenger: RootMessenger<KeyringControllerWithKeyringAction, never>,
77+
source?: string | undefined,
78+
): Promise<Uint8Array> {
79+
if (!source) {
80+
const seed = (await messenger.call(
81+
'KeyringController:withKeyring',
82+
{
83+
type: KeyringTypes.hd,
84+
index: 0,
85+
},
86+
async ({ keyring }) => (keyring as HdKeyring).seed,
87+
)) as Uint8Array | null;
88+
89+
if (!seed) {
90+
throw new Error('Primary keyring mnemonic unavailable.');
91+
}
92+
93+
return seed;
94+
}
95+
96+
try {
97+
const keyringData = await messenger.call(
98+
'KeyringController:withKeyring',
99+
{
100+
id: source,
101+
},
102+
async ({ keyring }) => ({
103+
type: keyring.type,
104+
seed: (keyring as HdKeyring).seed,
105+
}),
106+
);
107+
108+
const { type, seed } = keyringData as { type: string; seed?: Uint8Array };
109+
110+
if (type !== KeyringTypes.hd || !seed) {
111+
// The keyring isn't guaranteed to have a mnemonic (e.g.,
112+
// hardware wallets, which can't be used as entropy sources),
113+
// so we throw an error if it doesn't.
114+
throw new Error(`Entropy source with ID "${source}" not found.`);
115+
}
116+
117+
return seed;
118+
} catch {
119+
throw new Error(`Entropy source with ID "${source}" not found.`);
120+
}
121+
}

app/scripts/metamask-controller.js

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -5494,41 +5494,6 @@ export default class MetamaskController extends EventEmitter {
54945494
await this.keyringController.verifyPassword(password);
54955495
}
54965496

5497-
/**
5498-
* @type Identity
5499-
* @property {string} name - The account nickname.
5500-
* @property {string} address - The account's ethereum address, in lower case.
5501-
* receiving funds from our automatic Ropsten faucet.
5502-
*/
5503-
5504-
/**
5505-
* Gets the mnemonic of the user's primary keyring.
5506-
*/
5507-
getPrimaryKeyringMnemonic() {
5508-
const [keyring] = this.keyringController.getKeyringsByType(
5509-
KeyringType.hdKeyTree,
5510-
);
5511-
if (!keyring.mnemonic) {
5512-
throw new Error('Primary keyring mnemonic unavailable.');
5513-
}
5514-
5515-
return keyring.mnemonic;
5516-
}
5517-
5518-
/**
5519-
* Gets the mnemonic seed of the user's primary keyring.
5520-
*/
5521-
getPrimaryKeyringMnemonicSeed() {
5522-
const [keyring] = this.keyringController.getKeyringsByType(
5523-
KeyringType.hdKeyTree,
5524-
);
5525-
if (!keyring.seed) {
5526-
throw new Error('Primary keyring mnemonic unavailable.');
5527-
}
5528-
5529-
return keyring.seed;
5530-
}
5531-
55325497
//
55335498
// Hardware
55345499
//

0 commit comments

Comments
 (0)