diff --git a/packages/snaps-controllers/src/snaps/SnapController.test.tsx b/packages/snaps-controllers/src/snaps/SnapController.test.tsx index 4365b17110..a23085242b 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.test.tsx +++ b/packages/snaps-controllers/src/snaps/SnapController.test.tsx @@ -6074,6 +6074,19 @@ describe('SnapController', () => { () => ({}), ); + rootMessenger.registerActionHandler( + 'PermissionController:grantPermissions', + () => { + // After install the snap should have permissions + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => MOCK_SNAP_PERMISSIONS, + ); + + return {}; + }, + ); + const preinstalledSnaps = [ { snapId: MOCK_SNAP_ID, @@ -6120,12 +6133,6 @@ describe('SnapController', () => { true, ); - // After install the snap should have permissions - rootMessenger.registerActionHandler( - 'PermissionController:getPermissions', - () => MOCK_SNAP_PERMISSIONS, - ); - const result = await snapController.handleRequest({ snapId: MOCK_SNAP_ID, origin: MOCK_ORIGIN, @@ -6236,6 +6243,19 @@ describe('SnapController', () => { () => ({}), ); + rootMessenger.registerActionHandler( + 'PermissionController:grantPermissions', + () => { + // After install the snap should have permissions + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => MOCK_SNAP_PERMISSIONS, + ); + + return {}; + }, + ); + const initialConnections = { 'npm:filsnap': {}, 'https://snaps.metamask.io': {}, @@ -6279,12 +6299,6 @@ describe('SnapController', () => { }, }; - // After install the snap should have permissions - rootMessenger.registerActionHandler( - 'PermissionController:getPermissions', - () => MOCK_SNAP_PERMISSIONS, - ); - expect(snapControllerOptions.messenger.call).toHaveBeenCalledWith( 'PermissionController:grantPermissions', { approvedPermissions, subject: { origin: 'npm:filsnap' } }, @@ -6311,6 +6325,19 @@ describe('SnapController', () => { () => ({}), ); + rootMessenger.registerActionHandler( + 'PermissionController:grantPermissions', + () => { + // After install the snap should have permissions + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => MOCK_SNAP_PERMISSIONS, + ); + + return {}; + }, + ); + const preinstalledSnaps = [ { snapId: MOCK_SNAP_ID, @@ -6350,12 +6377,6 @@ describe('SnapController', () => { }, ); - // After install the snap should have permissions - rootMessenger.registerActionHandler( - 'PermissionController:getPermissions', - () => MOCK_SNAP_PERMISSIONS, - ); - expect( await snapController.installSnaps(MOCK_ORIGIN, { [MOCK_SNAP_ID]: {}, @@ -6403,6 +6424,29 @@ describe('SnapController', () => { }, ]; + rootMessenger.registerActionHandler( + 'PermissionController:grantPermissions', + () => { + // After install the snap should have permissions + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => ({ + [SnapEndowments.Rpc]: MOCK_SNAP_PERMISSIONS[SnapEndowments.Rpc], + // eslint-disable-next-line @typescript-eslint/naming-convention + snap_getEntropy: { + caveats: null, + date: 1664187844588, + id: 'izn0WGUO8cvq_jqvLQuQP', + invoker: MOCK_SNAP_ID, + parentCapability: 'snap_getEntropy', + }, + }), + ); + + return {}; + }, + ); + const snapControllerOptions = getSnapControllerWithEESOptions({ preinstalledSnaps, rootMessenger, @@ -6447,22 +6491,6 @@ describe('SnapController', () => { true, ); - // After install the snap should have permissions - rootMessenger.registerActionHandler( - 'PermissionController:getPermissions', - () => ({ - [SnapEndowments.Rpc]: MOCK_SNAP_PERMISSIONS[SnapEndowments.Rpc], - // eslint-disable-next-line @typescript-eslint/naming-convention - snap_getEntropy: { - caveats: null, - date: 1664187844588, - id: 'izn0WGUO8cvq_jqvLQuQP', - invoker: MOCK_SNAP_ID, - parentCapability: 'snap_getEntropy', - }, - }), - ); - const result = await snapController.handleRequest({ snapId: MOCK_SNAP_ID, origin: MOCK_ORIGIN, @@ -6516,6 +6544,19 @@ describe('SnapController', () => { () => ({}), ); + rootMessenger.registerActionHandler( + 'PermissionController:grantPermissions', + () => { + // After install the snap should have permissions + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => MOCK_SNAP_PERMISSIONS, + ); + + return {}; + }, + ); + const { manifest } = await getMockSnapFilesWithUpdatedChecksum({ manifest: getSnapManifest({ proposedName: '{{ proposedName }}', @@ -6567,12 +6608,6 @@ describe('SnapController', () => { }, ); - // After install the snap should have permissions - rootMessenger.registerActionHandler( - 'PermissionController:getPermissions', - () => MOCK_SNAP_PERMISSIONS, - ); - const result = await snapController.handleRequest({ snapId: MOCK_SNAP_ID, origin: MOCK_ORIGIN, @@ -6674,6 +6709,19 @@ describe('SnapController', () => { () => ({}), ); + rootMessenger.registerActionHandler( + 'PermissionController:grantPermissions', + () => { + // After install the snap should have permissions + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => MOCK_SNAP_PERMISSIONS, + ); + + return {}; + }, + ); + const preinstalledSnaps = [ { snapId: MOCK_SNAP_ID, @@ -6713,6 +6761,19 @@ describe('SnapController', () => { () => ({}), ); + rootMessenger.registerActionHandler( + 'PermissionController:grantPermissions', + () => { + // After install the snap should have permissions + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => MOCK_SNAP_PERMISSIONS, + ); + + return {}; + }, + ); + const preinstalledSnaps = [ { snapId: MOCK_SNAP_ID, @@ -6742,6 +6803,183 @@ describe('SnapController', () => { snapController.destroy(); }); + it('recovers if preinstalled permissions are out of sync', async () => { + const rootMessenger = getControllerMessenger(); + jest.spyOn(rootMessenger, 'call'); + const log = jest.spyOn(console, 'warn').mockImplementation(); + + // Never persist any granted permissions + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => ({}), + ); + + const preinstalledSnaps = [ + { + snapId: MOCK_SNAP_ID, + manifest: getSnapManifest(), + hidden: true, + files: [ + { + path: DEFAULT_SOURCE_PATH, + value: stringToBytes(DEFAULT_SNAP_BUNDLE), + }, + { + path: DEFAULT_ICON_PATH, + value: stringToBytes(DEFAULT_SNAP_ICON), + }, + ], + }, + ]; + + const snapControllerOptions = getSnapControllerWithEESOptions({ + preinstalledSnaps, + rootMessenger, + }); + const [snapController] = getSnapControllerWithEES(snapControllerOptions); + + expect(log).toHaveBeenCalledWith( + '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.', + ); + + // We expect two calls as we mock the PermissionController to always return an empty set. + expect(snapControllerOptions.messenger.call).toHaveBeenNthCalledWith( + 3, + 'PermissionController:grantPermissions', + { + approvedPermissions: { + // eslint-disable-next-line @typescript-eslint/naming-convention + snap_dialog: {}, + [SnapEndowments.Rpc]: { + caveats: [ + { type: 'rpcOrigin', value: { dapps: false, snaps: true } }, + ], + }, + }, + subject: { origin: MOCK_SNAP_ID }, + }, + ); + + expect(snapControllerOptions.messenger.call).toHaveBeenNthCalledWith( + 6, + 'SubjectMetadataController:addSubjectMetadata', + { + subjectType: SubjectType.Snap, + name: MOCK_SNAP_NAME, + origin: MOCK_SNAP_ID, + version: '1.0.0', + svgIcon: DEFAULT_SNAP_ICON, + }, + ); + + expect(snapControllerOptions.messenger.call).toHaveBeenNthCalledWith( + 7, + 'PermissionController:grantPermissions', + { + approvedPermissions: { + // eslint-disable-next-line @typescript-eslint/naming-convention + snap_dialog: {}, + [SnapEndowments.Rpc]: { + caveats: [ + { type: 'rpcOrigin', value: { dapps: false, snaps: true } }, + ], + }, + }, + subject: { origin: MOCK_SNAP_ID }, + }, + ); + + snapController.destroy(); + }); + + it('recovers if preinstalled permissions are out of sync when Snap has limited information', async () => { + const rootMessenger = getControllerMessenger(); + jest.spyOn(rootMessenger, 'call'); + const log = jest.spyOn(console, 'warn').mockImplementation(); + + // Never persist any granted permissions + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => ({}), + ); + + const manifest = getSnapManifest(); + delete manifest.source.location.npm.iconPath; + + const preinstalledSnaps = [ + { + snapId: MOCK_SNAP_ID, + manifest, + hidden: true, + files: [ + { + path: DEFAULT_SOURCE_PATH, + value: stringToBytes(DEFAULT_SNAP_BUNDLE), + }, + ], + }, + ]; + + const snapControllerOptions = getSnapControllerWithEESOptions({ + preinstalledSnaps, + rootMessenger, + }); + const [snapController] = getSnapControllerWithEES(snapControllerOptions); + + expect(log).toHaveBeenCalledWith( + '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.', + ); + + // We expect two calls as we mock the PermissionController to always return an empty set. + expect(snapControllerOptions.messenger.call).toHaveBeenNthCalledWith( + 3, + 'PermissionController:grantPermissions', + { + approvedPermissions: { + // eslint-disable-next-line @typescript-eslint/naming-convention + snap_dialog: {}, + [SnapEndowments.Rpc]: { + caveats: [ + { type: 'rpcOrigin', value: { dapps: false, snaps: true } }, + ], + }, + }, + subject: { origin: MOCK_SNAP_ID }, + }, + ); + + expect(snapControllerOptions.messenger.call).toHaveBeenNthCalledWith( + 6, + 'SubjectMetadataController:addSubjectMetadata', + { + subjectType: SubjectType.Snap, + name: MOCK_SNAP_NAME, + origin: MOCK_SNAP_ID, + version: '1.0.0', + svgIcon: null, + }, + ); + + expect(snapControllerOptions.messenger.call).toHaveBeenNthCalledWith( + 7, + 'PermissionController:grantPermissions', + { + approvedPermissions: { + // eslint-disable-next-line @typescript-eslint/naming-convention + snap_dialog: {}, + [SnapEndowments.Rpc]: { + caveats: [ + { type: 'rpcOrigin', value: { dapps: false, snaps: true } }, + ], + }, + }, + subject: { origin: MOCK_SNAP_ID }, + }, + ); + + snapController.destroy(); + }); + it('supports onInstall for preinstalled Snaps', async () => { const rootMessenger = getControllerMessenger(); jest.spyOn(rootMessenger, 'call'); @@ -6782,7 +7020,7 @@ describe('SnapController', () => { await new Promise((resolve) => setTimeout(resolve, 10)); expect(messenger.call).toHaveBeenNthCalledWith( - 6, + 7, 'ExecutionService:handleRpcRequest', MOCK_SNAP_ID, { @@ -6848,7 +7086,7 @@ describe('SnapController', () => { await new Promise((resolve) => setTimeout(resolve, 10)); expect(messenger.call).toHaveBeenNthCalledWith( - 6, + 7, 'ExecutionService:handleRpcRequest', MOCK_SNAP_ID, { @@ -10438,6 +10676,33 @@ describe('SnapController', () => { ]; const rootMessenger = getControllerMessenger(); + + rootMessenger.registerActionHandler( + 'PermissionController:revokeAllPermissions', + () => { + // After revoking the snap should have no permissions + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => ({}), + ); + + return {}; + }, + ); + + rootMessenger.registerActionHandler( + 'PermissionController:grantPermissions', + () => { + // After install the snap should have permissions + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => MOCK_SNAP_PERMISSIONS, + ); + + return {}; + }, + ); + const messenger = getSnapControllerMessenger(rootMessenger); const snapController = getSnapController( getSnapControllerOptions({ diff --git a/packages/snaps-controllers/src/snaps/SnapController.ts b/packages/snaps-controllers/src/snaps/SnapController.ts index 54f15eaaa4..7b32f7a754 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.ts +++ b/packages/snaps-controllers/src/snaps/SnapController.ts @@ -1457,6 +1457,65 @@ export class SnapController extends BaseController< ); } } + + // Ensure all preinstalled Snaps have their expected permissions. + for (const snap of Object.values(this.state.snaps).filter( + ({ preinstalled }) => preinstalled, + )) { + const processedPermissions = processSnapPermissions( + snap.manifest.initialPermissions, + ); + + this.#validateSnapPermissions(processedPermissions); + + const { newPermissions, unusedPermissions } = + this.#calculatePermissionsChange(snap.id, processedPermissions); + + if ( + isNonEmptyArray(Object.keys(newPermissions)) || + isNonEmptyArray(Object.keys(unusedPermissions)) + ) { + const { proposedName } = getLocalizedSnapManifest( + snap.manifest, + 'en', + snap.localizationFiles ?? [], + ); + + // Recover the SVG icon from the constructor argument. + // Theoretically this may be out of date, but this is the best we can do. + const preinstalledSnap = preinstalledSnaps.find( + (potentialSnap) => potentialSnap.snapId === snap.id, + ); + const { iconPath } = snap.manifest.source.location.npm; + const svgIcon = + iconPath && preinstalledSnap + ? preinstalledSnap.files.find((file) => file.path === iconPath) + : undefined; + + // If the permissions are out of sync, it is possible that the SubjectMetadataController also is. + this.messenger.call('SubjectMetadataController:addSubjectMetadata', { + subjectType: SubjectType.Snap, + name: proposedName, + origin: snap.id, + version: snap.version, + svgIcon: svgIcon ? new VirtualFile(svgIcon).toString() : null, + }); + + this.#updatePermissions({ + snapId: snap.id, + newPermissions, + unusedPermissions, + }); + logWarning( + `The permissions for "${snap.id}" were out of sync and have been automatically restored. If you see this message, please file a bug report.`, + ); + this.messenger.captureException?.( + new Error( + `The permissions for "${snap.id}" were out of sync and have been automatically restored. This could indicate persistence issues.`, + ), + ); + } + } } #pollForLastRequestStatus() {