Skip to content

Commit 1c2b700

Browse files
Loosen allowlist requirements
1 parent 2bebe60 commit 1c2b700

File tree

4 files changed

+49
-8
lines changed

4 files changed

+49
-8
lines changed
+2-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"branches": 91.17,
3-
"functions": 96.92,
3+
"functions": 96.93,
44
"lines": 98,
5-
"statements": 97.72
5+
"statements": 97.73
66
}

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

+19-4
Original file line numberDiff line numberDiff line change
@@ -584,7 +584,11 @@ describe('SnapController', () => {
584584
);
585585

586586
expect(messenger.call).toHaveBeenNthCalledWith(2, 'SnapsRegistry:get', {
587-
[MOCK_SNAP_ID]: { version: '1.0.0', checksum: DEFAULT_SNAP_SHASUM },
587+
[MOCK_SNAP_ID]: {
588+
version: '1.0.0',
589+
checksum: DEFAULT_SNAP_SHASUM,
590+
permissions: getSnapManifest().initialPermissions,
591+
},
588592
});
589593

590594
expect(messenger.call).toHaveBeenNthCalledWith(
@@ -768,11 +772,22 @@ describe('SnapController', () => {
768772
});
769773

770774
it('throws an error if snap is not on allowlist and allowlisting is required', async () => {
775+
const { manifest, sourceCode, svgIcon } = await getMockSnapFilesWithUpdatedChecksum({
776+
manifest: getSnapManifest({
777+
initialPermissions: {
778+
// eslint-disable-next-line @typescript-eslint/naming-convention
779+
snap_getBip44Entropy: [{ coinType: 1 }],
780+
},
781+
}),
782+
});
783+
771784
const controller = getSnapController(
772785
getSnapControllerOptions({
773786
featureFlags: { requireAllowlist: true },
774-
detectSnapLocation: (_location, options) =>
775-
new LoopbackLocation(options),
787+
detectSnapLocation: loopbackDetect({
788+
manifest: manifest.result,
789+
files: [sourceCode, svgIcon as VirtualFile],
790+
}),
776791
}),
777792
);
778793

@@ -781,7 +796,7 @@ describe('SnapController', () => {
781796
[MOCK_SNAP_ID]: { version: DEFAULT_REQUESTED_SNAP_VERSION },
782797
}),
783798
).rejects.toThrow(
784-
'Failed to fetch snap "npm:@metamask/example-snap": The snap is not on the allowlist.',
799+
'Cannot install version "1.0.0" of snap "npm:@metamask/example-snap": The snap is not on the allowlist.',
785800
);
786801

787802
controller.destroy();

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

+16-2
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ import type {
4545
PersistedSnap,
4646
Snap,
4747
SnapManifest,
48+
SnapPermissions,
4849
SnapRpcHookArgs,
4950
StatusContext,
5051
StatusEvents,
@@ -107,6 +108,7 @@ import type {
107108
TerminateSnapAction,
108109
} from '../services';
109110
import { fetchSnap, hasTimedOut, setDiff, withTimeout } from '../utils';
111+
import { ALLOWED_PERMISSIONS } from './constants';
110112
import {
111113
getMaxRequestTimeCaveat,
112114
handlerEndowments,
@@ -1197,7 +1199,10 @@ export class SnapController extends BaseController<
11971199
this.messagingSystem.publish(`${controllerName}:snapUnblocked`, snapId);
11981200
}
11991201

1200-
async #assertIsInstallAllowed(snapId: SnapId, snapInfo: SnapsRegistryInfo) {
1202+
async #assertIsInstallAllowed(
1203+
snapId: SnapId,
1204+
snapInfo: SnapsRegistryInfo & { permissions: SnapPermissions },
1205+
) {
12011206
const results = await this.messagingSystem.call('SnapsRegistry:get', {
12021207
[snapId]: snapInfo,
12031208
});
@@ -1210,8 +1215,15 @@ export class SnapController extends BaseController<
12101215
result.reason?.explanation ?? ''
12111216
}`,
12121217
);
1213-
} else if (
1218+
}
1219+
1220+
const isAllowlistingRequired = !Object.keys(snapInfo.permissions).every(
1221+
(permission) => ALLOWED_PERMISSIONS.includes(permission),
1222+
);
1223+
1224+
if (
12141225
this.#featureFlags.requireAllowlist &&
1226+
isAllowlistingRequired &&
12151227
result.status !== SnapsRegistryStatus.Verified
12161228
) {
12171229
throw new Error(
@@ -2194,6 +2206,7 @@ export class SnapController extends BaseController<
21942206
await this.#assertIsInstallAllowed(snapId, {
21952207
version: newVersion,
21962208
checksum: manifest.source.shasum,
2209+
permissions: manifest.initialPermissions,
21972210
});
21982211

21992212
const processedPermissions = processSnapPermissions(
@@ -2367,6 +2380,7 @@ export class SnapController extends BaseController<
23672380
await this.#assertIsInstallAllowed(snapId, {
23682381
version: manifest.version,
23692382
checksum: manifest.source.shasum,
2383+
permissions: manifest.initialPermissions,
23702384
});
23712385

23722386
return this.#set({
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import { SnapEndowments } from './endowments';
2+
3+
// These permissions are allowed without being on the allowlist.
4+
export const ALLOWED_PERMISSIONS = Object.freeze([
5+
'snap_dialog',
6+
'snap_manageState',
7+
'snap_notify',
8+
'snap_getLocale',
9+
SnapEndowments.Cronjob,
10+
SnapEndowments.HomePage,
11+
SnapEndowments.LifecycleHooks,
12+
]);

0 commit comments

Comments
 (0)