Skip to content

Commit c6379c6

Browse files
release(runway): cherry-pick fix: cp-13.2.0 Make SnapsNameProvider respect the chainIds caveat (#35509)
- fix: cp-13.2.0 Make SnapsNameProvider respect the chainIds caveat (#35477) <!-- 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** Previously the SnapsNameProvider would not respect the `chainIds` caveat when specified by the Snap, this would result in calls to the Snap that it is not expecting and could crash it. In production this crashes the Solana Snap. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/35477?quickstart=1) ## **Related issues** Fixes: #35473 Fixes: #35364 [62e1a87](62e1a87) Co-authored-by: Frederik Bolding <[email protected]>
1 parent 11e75ce commit c6379c6

File tree

4 files changed

+72
-22
lines changed

4 files changed

+72
-22
lines changed

app/scripts/lib/SnapsNameProvider.test.ts

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { NameType } from '@metamask/name-controller';
2-
import { HandlerType } from '@metamask/snaps-utils';
2+
import { HandlerType, SnapCaveatType } from '@metamask/snaps-utils';
33
import {
44
GetAllSnaps,
55
GetSnap,
@@ -39,6 +39,13 @@ const SNAP_MOCK_3 = {
3939
},
4040
};
4141

42+
const SNAP_MOCK_4 = {
43+
id: 'testSnap4',
44+
manifest: {
45+
proposedName: 'Test Snap 4',
46+
},
47+
};
48+
4249
function createMockMessenger({
4350
getAllSnaps,
4451
getSnap,
@@ -56,7 +63,9 @@ function createMockMessenger({
5663
// TODO: Fix in https://github.com/MetaMask/metamask-extension/issues/31880
5764
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
5865
getAllSnaps ||
59-
jest.fn().mockReturnValue([SNAP_MOCK, SNAP_MOCK_2, SNAP_MOCK_3]);
66+
jest
67+
.fn()
68+
.mockReturnValue([SNAP_MOCK, SNAP_MOCK_2, SNAP_MOCK_3, SNAP_MOCK_4]);
6069

6170
const getSnapMock =
6271
// TODO: Fix in https://github.com/MetaMask/metamask-extension/issues/31880
@@ -65,7 +74,9 @@ function createMockMessenger({
6574
jest
6675
.fn()
6776
.mockImplementation((snapId) =>
68-
[SNAP_MOCK, SNAP_MOCK_2, SNAP_MOCK_3].find(({ id }) => id === snapId),
77+
[SNAP_MOCK, SNAP_MOCK_2, SNAP_MOCK_3, SNAP_MOCK_4].find(
78+
({ id }) => id === snapId,
79+
),
6980
);
7081

7182
const handleSnapRequestMock =
@@ -79,9 +90,21 @@ function createMockMessenger({
7990
getPermissionControllerState ||
8091
jest.fn().mockReturnValue({
8192
subjects: {
82-
[SNAP_MOCK.id]: { permissions: { 'endowment:name-lookup': true } },
83-
[SNAP_MOCK_2.id]: { permissions: { 'endowment:name-lookup': true } },
93+
[SNAP_MOCK.id]: { permissions: { 'endowment:name-lookup': {} } },
94+
[SNAP_MOCK_2.id]: { permissions: { 'endowment:name-lookup': {} } },
8495
[SNAP_MOCK_3.id]: { permissions: {} },
96+
[SNAP_MOCK_4.id]: {
97+
permissions: {
98+
'endowment:name-lookup': {
99+
caveats: [
100+
{
101+
type: SnapCaveatType.ChainIds,
102+
value: ['solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp'],
103+
},
104+
],
105+
},
106+
},
107+
},
85108
},
86109
});
87110

@@ -122,12 +145,17 @@ describe('SnapsNameProvider', () => {
122145
const { sourceIds, sourceLabels } = metadata;
123146

124147
expect(sourceIds).toStrictEqual({
125-
[NameType.ETHEREUM_ADDRESS]: [SNAP_MOCK.id, SNAP_MOCK_2.id],
148+
[NameType.ETHEREUM_ADDRESS]: [
149+
SNAP_MOCK.id,
150+
SNAP_MOCK_2.id,
151+
SNAP_MOCK_4.id,
152+
],
126153
});
127154

128155
expect(sourceLabels).toStrictEqual({
129156
[SNAP_MOCK.id]: SNAP_MOCK.manifest.proposedName,
130157
[SNAP_MOCK_2.id]: SNAP_MOCK_2.manifest.proposedName,
158+
[SNAP_MOCK_4.id]: SNAP_MOCK_4.manifest.proposedName,
131159
});
132160
});
133161
});

app/scripts/lib/SnapsNameProvider.ts

Lines changed: 35 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import { GetPermissionControllerState } from '@metamask/permission-controller';
1010
import {
1111
AddressLookupArgs,
1212
AddressLookupResult,
13+
CaipChainId,
14+
SnapId,
1315
Snap as TruncatedSnap,
1416
} from '@metamask/snaps-sdk';
1517
import { HandlerType } from '@metamask/snaps-utils';
@@ -20,6 +22,7 @@ import {
2022
HandleSnapRequest,
2123
} from '@metamask/snaps-controllers';
2224
import { RestrictedMessenger } from '@metamask/base-controller';
25+
import { getChainIdsCaveat } from '@metamask/snaps-rpc-methods';
2326

2427
type AllowedActions =
2528
| GetAllSnaps
@@ -73,10 +76,15 @@ export class SnapsNameProvider implements NameProvider {
7376
async getProposedNames(
7477
request: NameProviderRequest,
7578
): Promise<NameProviderResult> {
76-
const nameSnaps = this.#getNameLookupSnaps();
79+
const { variation: chainIdHex, value } = request;
80+
const caipChainId = `eip155:${parseInt(chainIdHex, 16)}` as CaipChainId;
81+
82+
const nameSnaps = this.#getNameLookupSnaps(caipChainId);
7783

7884
const snapResults = await Promise.all(
79-
nameSnaps.map((snap) => this.#getSnapProposedName(snap, request)),
85+
nameSnaps.map((snap) =>
86+
this.#getSnapProposedName(snap.id, caipChainId, value),
87+
),
8088
);
8189

8290
const results = snapResults.reduce(
@@ -93,29 +101,41 @@ export class SnapsNameProvider implements NameProvider {
93101
return { results };
94102
}
95103

96-
#getNameLookupSnaps(): TruncatedSnap[] {
104+
#getNameLookupSnaps(chainId?: string): TruncatedSnap[] {
97105
const permissionSubjects = this.#messenger.call(
98106
'PermissionController:getState',
99107
).subjects;
100108

101109
const snaps = this.#messenger.call('SnapController:getAll');
102110

103-
return snaps.filter(
104-
({ id }) => permissionSubjects[id]?.permissions['endowment:name-lookup'],
105-
);
111+
return snaps.filter(({ id }) => {
112+
const permission =
113+
permissionSubjects[id]?.permissions['endowment:name-lookup'];
114+
115+
if (!permission) {
116+
return false;
117+
}
118+
119+
const chainIdCaveat = getChainIdsCaveat(permission);
120+
121+
if (chainId && chainIdCaveat && !chainIdCaveat.includes(chainId)) {
122+
return false;
123+
}
124+
125+
return true;
126+
});
106127
}
107128

108129
async #getSnapProposedName(
109-
snap: TruncatedSnap,
110-
request: NameProviderRequest,
130+
snapId: SnapId,
131+
caipChainId: CaipChainId,
132+
address: string,
111133
): Promise<{ sourceId: string; result: NameProviderSourceResult }> {
112-
const { variation: chainIdHex, value } = request;
113-
const sourceId = snap.id;
114-
const chainIdDecimal = parseInt(chainIdHex, 16);
134+
const sourceId = snapId;
115135

116136
const nameLookupRequest: AddressLookupArgs = {
117-
chainId: `eip155:${chainIdDecimal}`,
118-
address: value,
137+
chainId: caipChainId,
138+
address,
119139
};
120140

121141
let proposedNames;
@@ -125,7 +145,7 @@ export class SnapsNameProvider implements NameProvider {
125145
const result = (await this.#messenger.call(
126146
'SnapController:handleRequest',
127147
{
128-
snapId: snap.id,
148+
snapId,
129149
origin: 'metamask',
130150
handler: HandlerType.OnNameLookup,
131151
request: {
@@ -144,7 +164,7 @@ export class SnapsNameProvider implements NameProvider {
144164
: [];
145165
} catch (error) {
146166
log.error('Snap name provider request failed', {
147-
snapId: snap.id,
167+
snapId,
148168
request: nameLookupRequest,
149169
error,
150170
});

privacy-snapshot.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
"accounts.api.cx.metamask.io",
33
"acl.execution.metamask.io",
44
"api.devnet.solana.com",
5+
"api.etherscan.io",
56
"api.lens.dev",
67
"api.segment.io",
78
"api.simplehash.com",

test/e2e/tests/petnames/petnames-signatures.spec.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ describe('Petnames - Signatures', function (this: Suite) {
9494
fixtures: new FixtureBuilder()
9595
.withPermissionControllerConnectedToTestDapp()
9696
.withNoNames()
97+
.withNetworkControllerOnMainnet()
9798
.build(),
9899
testSpecificMock: mockLookupSnap,
99100
title: this.test?.fullTitle(),
@@ -112,7 +113,7 @@ describe('Petnames - Signatures', function (this: Suite) {
112113
await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog);
113114
await confirmation.checkProposedNames('0xCD2a3...DD826', [
114115
['test.lens', 'Lens Protocol'],
115-
['cd2.1337.test.domain', 'Name Lookup Example Snap'],
116+
['cd2.1.test.domain', 'Name Lookup Example Snap'],
116117
]);
117118
},
118119
);

0 commit comments

Comments
 (0)