Skip to content

Commit c1ce02e

Browse files
FrederikBoldingFrederikBolding
committed
fix: Recover missing permissions for preinstalled Snaps
1 parent 08cf90f commit c1ce02e

File tree

2 files changed

+200
-42
lines changed

2 files changed

+200
-42
lines changed

‎packages/snaps-controllers/src/snaps/SnapController.test.tsx‎

Lines changed: 172 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -6074,6 +6074,19 @@ describe('SnapController', () => {
60746074
() => ({}),
60756075
);
60766076

6077+
rootMessenger.registerActionHandler(
6078+
'PermissionController:grantPermissions',
6079+
() => {
6080+
// After install the snap should have permissions
6081+
rootMessenger.registerActionHandler(
6082+
'PermissionController:getPermissions',
6083+
() => MOCK_SNAP_PERMISSIONS,
6084+
);
6085+
6086+
return {};
6087+
},
6088+
);
6089+
60776090
const preinstalledSnaps = [
60786091
{
60796092
snapId: MOCK_SNAP_ID,
@@ -6120,12 +6133,6 @@ describe('SnapController', () => {
61206133
true,
61216134
);
61226135

6123-
// After install the snap should have permissions
6124-
rootMessenger.registerActionHandler(
6125-
'PermissionController:getPermissions',
6126-
() => MOCK_SNAP_PERMISSIONS,
6127-
);
6128-
61296136
const result = await snapController.handleRequest({
61306137
snapId: MOCK_SNAP_ID,
61316138
origin: MOCK_ORIGIN,
@@ -6236,6 +6243,19 @@ describe('SnapController', () => {
62366243
() => ({}),
62376244
);
62386245

6246+
rootMessenger.registerActionHandler(
6247+
'PermissionController:grantPermissions',
6248+
() => {
6249+
// After install the snap should have permissions
6250+
rootMessenger.registerActionHandler(
6251+
'PermissionController:getPermissions',
6252+
() => MOCK_SNAP_PERMISSIONS,
6253+
);
6254+
6255+
return {};
6256+
},
6257+
);
6258+
62396259
const initialConnections = {
62406260
'npm:filsnap': {},
62416261
'https://snaps.metamask.io': {},
@@ -6279,12 +6299,6 @@ describe('SnapController', () => {
62796299
},
62806300
};
62816301

6282-
// After install the snap should have permissions
6283-
rootMessenger.registerActionHandler(
6284-
'PermissionController:getPermissions',
6285-
() => MOCK_SNAP_PERMISSIONS,
6286-
);
6287-
62886302
expect(snapControllerOptions.messenger.call).toHaveBeenCalledWith(
62896303
'PermissionController:grantPermissions',
62906304
{ approvedPermissions, subject: { origin: 'npm:filsnap' } },
@@ -6311,6 +6325,19 @@ describe('SnapController', () => {
63116325
() => ({}),
63126326
);
63136327

6328+
rootMessenger.registerActionHandler(
6329+
'PermissionController:grantPermissions',
6330+
() => {
6331+
// After install the snap should have permissions
6332+
rootMessenger.registerActionHandler(
6333+
'PermissionController:getPermissions',
6334+
() => MOCK_SNAP_PERMISSIONS,
6335+
);
6336+
6337+
return {};
6338+
},
6339+
);
6340+
63146341
const preinstalledSnaps = [
63156342
{
63166343
snapId: MOCK_SNAP_ID,
@@ -6350,12 +6377,6 @@ describe('SnapController', () => {
63506377
},
63516378
);
63526379

6353-
// After install the snap should have permissions
6354-
rootMessenger.registerActionHandler(
6355-
'PermissionController:getPermissions',
6356-
() => MOCK_SNAP_PERMISSIONS,
6357-
);
6358-
63596380
expect(
63606381
await snapController.installSnaps(MOCK_ORIGIN, {
63616382
[MOCK_SNAP_ID]: {},
@@ -6403,6 +6424,29 @@ describe('SnapController', () => {
64036424
},
64046425
];
64056426

6427+
rootMessenger.registerActionHandler(
6428+
'PermissionController:grantPermissions',
6429+
() => {
6430+
// After install the snap should have permissions
6431+
rootMessenger.registerActionHandler(
6432+
'PermissionController:getPermissions',
6433+
() => ({
6434+
[SnapEndowments.Rpc]: MOCK_SNAP_PERMISSIONS[SnapEndowments.Rpc],
6435+
// eslint-disable-next-line @typescript-eslint/naming-convention
6436+
snap_getEntropy: {
6437+
caveats: null,
6438+
date: 1664187844588,
6439+
id: 'izn0WGUO8cvq_jqvLQuQP',
6440+
invoker: MOCK_SNAP_ID,
6441+
parentCapability: 'snap_getEntropy',
6442+
},
6443+
}),
6444+
);
6445+
6446+
return {};
6447+
},
6448+
);
6449+
64066450
const snapControllerOptions = getSnapControllerWithEESOptions({
64076451
preinstalledSnaps,
64086452
rootMessenger,
@@ -6447,22 +6491,6 @@ describe('SnapController', () => {
64476491
true,
64486492
);
64496493

6450-
// After install the snap should have permissions
6451-
rootMessenger.registerActionHandler(
6452-
'PermissionController:getPermissions',
6453-
() => ({
6454-
[SnapEndowments.Rpc]: MOCK_SNAP_PERMISSIONS[SnapEndowments.Rpc],
6455-
// eslint-disable-next-line @typescript-eslint/naming-convention
6456-
snap_getEntropy: {
6457-
caveats: null,
6458-
date: 1664187844588,
6459-
id: 'izn0WGUO8cvq_jqvLQuQP',
6460-
invoker: MOCK_SNAP_ID,
6461-
parentCapability: 'snap_getEntropy',
6462-
},
6463-
}),
6464-
);
6465-
64666494
const result = await snapController.handleRequest({
64676495
snapId: MOCK_SNAP_ID,
64686496
origin: MOCK_ORIGIN,
@@ -6516,6 +6544,19 @@ describe('SnapController', () => {
65166544
() => ({}),
65176545
);
65186546

6547+
rootMessenger.registerActionHandler(
6548+
'PermissionController:grantPermissions',
6549+
() => {
6550+
// After install the snap should have permissions
6551+
rootMessenger.registerActionHandler(
6552+
'PermissionController:getPermissions',
6553+
() => MOCK_SNAP_PERMISSIONS,
6554+
);
6555+
6556+
return {};
6557+
},
6558+
);
6559+
65196560
const { manifest } = await getMockSnapFilesWithUpdatedChecksum({
65206561
manifest: getSnapManifest({
65216562
proposedName: '{{ proposedName }}',
@@ -6567,12 +6608,6 @@ describe('SnapController', () => {
65676608
},
65686609
);
65696610

6570-
// After install the snap should have permissions
6571-
rootMessenger.registerActionHandler(
6572-
'PermissionController:getPermissions',
6573-
() => MOCK_SNAP_PERMISSIONS,
6574-
);
6575-
65766611
const result = await snapController.handleRequest({
65776612
snapId: MOCK_SNAP_ID,
65786613
origin: MOCK_ORIGIN,
@@ -6674,6 +6709,19 @@ describe('SnapController', () => {
66746709
() => ({}),
66756710
);
66766711

6712+
rootMessenger.registerActionHandler(
6713+
'PermissionController:grantPermissions',
6714+
() => {
6715+
// After install the snap should have permissions
6716+
rootMessenger.registerActionHandler(
6717+
'PermissionController:getPermissions',
6718+
() => MOCK_SNAP_PERMISSIONS,
6719+
);
6720+
6721+
return {};
6722+
},
6723+
);
6724+
66776725
const preinstalledSnaps = [
66786726
{
66796727
snapId: MOCK_SNAP_ID,
@@ -6713,6 +6761,19 @@ describe('SnapController', () => {
67136761
() => ({}),
67146762
);
67156763

6764+
rootMessenger.registerActionHandler(
6765+
'PermissionController:grantPermissions',
6766+
() => {
6767+
// After install the snap should have permissions
6768+
rootMessenger.registerActionHandler(
6769+
'PermissionController:getPermissions',
6770+
() => MOCK_SNAP_PERMISSIONS,
6771+
);
6772+
6773+
return {};
6774+
},
6775+
);
6776+
67166777
const preinstalledSnaps = [
67176778
{
67186779
snapId: MOCK_SNAP_ID,
@@ -6742,6 +6803,48 @@ describe('SnapController', () => {
67426803
snapController.destroy();
67436804
});
67446805

6806+
it('recovers if preinstalled permissions are out of sync', async () => {
6807+
const rootMessenger = getControllerMessenger();
6808+
jest.spyOn(rootMessenger, 'call');
6809+
const log = jest.spyOn(console, 'warn').mockImplementation();
6810+
6811+
// Never persist any granted permissions
6812+
rootMessenger.registerActionHandler(
6813+
'PermissionController:getPermissions',
6814+
() => ({}),
6815+
);
6816+
6817+
const preinstalledSnaps = [
6818+
{
6819+
snapId: MOCK_SNAP_ID,
6820+
manifest: getSnapManifest(),
6821+
hidden: true,
6822+
files: [
6823+
{
6824+
path: DEFAULT_SOURCE_PATH,
6825+
value: stringToBytes(DEFAULT_SNAP_BUNDLE),
6826+
},
6827+
{
6828+
path: DEFAULT_ICON_PATH,
6829+
value: stringToBytes(DEFAULT_SNAP_ICON),
6830+
},
6831+
],
6832+
},
6833+
];
6834+
6835+
const snapControllerOptions = getSnapControllerWithEESOptions({
6836+
preinstalledSnaps,
6837+
rootMessenger,
6838+
});
6839+
const [snapController] = getSnapControllerWithEES(snapControllerOptions);
6840+
6841+
expect(log).toHaveBeenCalledWith(
6842+
'The permissions for "npm:@metamask/example-snap" were out of sync and have been automatically restored. If you see this message, please file a bug report.',
6843+
);
6844+
6845+
snapController.destroy();
6846+
});
6847+
67456848
it('supports onInstall for preinstalled Snaps', async () => {
67466849
const rootMessenger = getControllerMessenger();
67476850
jest.spyOn(rootMessenger, 'call');
@@ -6782,7 +6885,7 @@ describe('SnapController', () => {
67826885
await new Promise((resolve) => setTimeout(resolve, 10));
67836886

67846887
expect(messenger.call).toHaveBeenNthCalledWith(
6785-
6,
6888+
7,
67866889
'ExecutionService:handleRpcRequest',
67876890
MOCK_SNAP_ID,
67886891
{
@@ -6848,7 +6951,7 @@ describe('SnapController', () => {
68486951
await new Promise((resolve) => setTimeout(resolve, 10));
68496952

68506953
expect(messenger.call).toHaveBeenNthCalledWith(
6851-
6,
6954+
7,
68526955
'ExecutionService:handleRpcRequest',
68536956
MOCK_SNAP_ID,
68546957
{
@@ -10438,6 +10541,33 @@ describe('SnapController', () => {
1043810541
];
1043910542

1044010543
const rootMessenger = getControllerMessenger();
10544+
10545+
rootMessenger.registerActionHandler(
10546+
'PermissionController:revokeAllPermissions',
10547+
() => {
10548+
// After revoking the snap should have no permissions
10549+
rootMessenger.registerActionHandler(
10550+
'PermissionController:getPermissions',
10551+
() => ({}),
10552+
);
10553+
10554+
return {};
10555+
},
10556+
);
10557+
10558+
rootMessenger.registerActionHandler(
10559+
'PermissionController:grantPermissions',
10560+
() => {
10561+
// After install the snap should have permissions
10562+
rootMessenger.registerActionHandler(
10563+
'PermissionController:getPermissions',
10564+
() => MOCK_SNAP_PERMISSIONS,
10565+
);
10566+
10567+
return {};
10568+
},
10569+
);
10570+
1044110571
const messenger = getSnapControllerMessenger(rootMessenger);
1044210572
const snapController = getSnapController(
1044310573
getSnapControllerOptions({

‎packages/snaps-controllers/src/snaps/SnapController.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1457,6 +1457,34 @@ export class SnapController extends BaseController<
14571457
);
14581458
}
14591459
}
1460+
1461+
// Ensure all preinstalled Snaps have their expected permissions.
1462+
for (const snap of Object.values(this.state.snaps).filter(
1463+
({ preinstalled }) => preinstalled,
1464+
)) {
1465+
const processedPermissions = processSnapPermissions(
1466+
snap.manifest.initialPermissions,
1467+
);
1468+
1469+
this.#validateSnapPermissions(processedPermissions);
1470+
1471+
const { newPermissions, unusedPermissions } =
1472+
this.#calculatePermissionsChange(snap.id, processedPermissions);
1473+
1474+
if (
1475+
isNonEmptyArray(Object.keys(newPermissions)) ||
1476+
isNonEmptyArray(Object.keys(unusedPermissions))
1477+
) {
1478+
this.#updatePermissions({
1479+
snapId: snap.id,
1480+
newPermissions,
1481+
unusedPermissions,
1482+
});
1483+
logWarning(
1484+
`The permissions for "${snap.id}" were out of sync and have been automatically restored. If you see this message, please file a bug report.`,
1485+
);
1486+
}
1487+
}
14601488
}
14611489

14621490
#pollForLastRequestStatus() {

0 commit comments

Comments
 (0)