diff --git a/packages/snaps-controllers/src/snaps/SnapController.test.tsx b/packages/snaps-controllers/src/snaps/SnapController.test.tsx index e7bd5f888e..69b1eafe91 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.test.tsx +++ b/packages/snaps-controllers/src/snaps/SnapController.test.tsx @@ -70,10 +70,13 @@ import { import { hmac } from '@noble/hashes/hmac'; import { sha512 } from '@noble/hashes/sha512'; import { File } from 'buffer'; +import { createReadStream } from 'fs'; import fetchMock from 'jest-fetch-mock'; +import path from 'path'; import { pipeline } from 'readable-stream'; import type { Duplex } from 'readable-stream'; import { inc } from 'semver'; +import { Readable } from 'stream'; import { LEGACY_ENCRYPTION_KEY_DERIVATION_OPTIONS, @@ -9953,7 +9956,7 @@ describe('SnapController', () => { }); }); - describe('updateBlockedSnaps', () => { + describe('updateRegistry', () => { it('updates the registry database', async () => { const registry = new MockSnapsRegistry(); const rootMessenger = getControllerMessenger(registry); @@ -9967,7 +9970,7 @@ describe('SnapController', () => { }, }), ); - await snapController.updateBlockedSnaps(); + await snapController.updateRegistry(); expect(registry.update).toHaveBeenCalled(); @@ -10011,7 +10014,7 @@ describe('SnapController', () => { reason: { explanation, infoUrl }, }, }); - await snapController.updateBlockedSnaps(); + await snapController.updateRegistry(); // Ensure that CheckSnapBlockListArg is correct expect(registry.get).toHaveBeenCalledWith({ @@ -10070,7 +10073,7 @@ describe('SnapController', () => { registry.get.mockResolvedValueOnce({ [mockSnap.id]: { status: SnapsRegistryStatus.Blocked }, }); - await snapController.updateBlockedSnaps(); + await snapController.updateRegistry(); // The snap is blocked, disabled, and stopped expect(snapController.get(mockSnap.id)?.blocked).toBe(true); @@ -10124,7 +10127,7 @@ describe('SnapController', () => { [mockSnapA.id]: { status: SnapsRegistryStatus.Unverified }, [mockSnapB.id]: { status: SnapsRegistryStatus.Unverified }, }); - await snapController.updateBlockedSnaps(); + await snapController.updateRegistry(); // A is unblocked, but still disabled expect(snapController.get(mockSnapA.id)?.blocked).toBe(false); @@ -10168,7 +10171,7 @@ describe('SnapController', () => { new Promise((resolve) => (resolveBlockListPromise = resolve)), ); - const updateBlockList = snapController.updateBlockedSnaps(); + const updateBlockList = snapController.updateRegistry(); // Remove the snap while waiting for the blocklist await snapController.removeSnap(mockSnap.id); @@ -10216,7 +10219,7 @@ describe('SnapController', () => { registry.get.mockResolvedValueOnce({ [mockSnap.id]: { status: SnapsRegistryStatus.Blocked }, }); - await snapController.updateBlockedSnaps(); + await snapController.updateRegistry(); // A is blocked and disabled expect(snapController.get(mockSnap.id)?.blocked).toBe(true); @@ -10230,6 +10233,131 @@ describe('SnapController', () => { snapController.destroy(); }); + + it('updates preinstalled Snaps', async () => { + const registry = new MockSnapsRegistry(); + const rootMessenger = getControllerMessenger(registry); + const messenger = getSnapControllerMessenger(rootMessenger); + + // Simulate previous permissions, some of which will be removed + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => { + return { + [SnapEndowments.Rpc]: MOCK_RPC_ORIGINS_PERMISSION, + [SnapEndowments.LifecycleHooks]: MOCK_LIFECYCLE_HOOKS_PERMISSION, + }; + }, + ); + + const snapId = 'npm:@metamask/jsx-example-snap' as SnapId; + + const mockSnap = getPersistedSnapObject({ + id: snapId, + preinstalled: true, + }); + + const updateVersion = '1.2.1'; + + registry.resolveVersion.mockResolvedValue(updateVersion); + const fetchFunction = jest.fn().mockResolvedValueOnce({ + // eslint-disable-next-line no-restricted-globals + headers: new Headers({ 'content-length': '5477' }), + ok: true, + body: Readable.toWeb( + createReadStream( + path.resolve( + __dirname, + `../../test/fixtures/metamask-jsx-example-snap-${updateVersion}.tgz`, + ), + ), + ), + }); + + const snapController = getSnapController( + getSnapControllerOptions({ + messenger, + state: { + snaps: getPersistedSnapsState(mockSnap), + }, + fetchFunction, + featureFlags: { + autoUpdatePreinstalledSnaps: true, + }, + }), + ); + + await snapController.updateRegistry(); + + const updatedSnap = snapController.get(snapId); + assert(updatedSnap); + + expect(updatedSnap.version).toStrictEqual(updateVersion); + expect(updatedSnap.preinstalled).toBe(true); + + expect(rootMessenger.call).toHaveBeenNthCalledWith( + 7, + 'PermissionController:revokePermissions', + { [snapId]: [SnapEndowments.Rpc, SnapEndowments.LifecycleHooks] }, + ); + expect(rootMessenger.call).toHaveBeenNthCalledWith( + 8, + 'PermissionController:grantPermissions', + { + approvedPermissions: { + 'endowment:rpc': { + caveats: [{ type: 'rpcOrigin', value: { dapps: true } }], + }, + // eslint-disable-next-line @typescript-eslint/naming-convention + snap_dialog: {}, + // eslint-disable-next-line @typescript-eslint/naming-convention + snap_manageState: {}, + }, + subject: { origin: snapId }, + }, + ); + + snapController.destroy(); + }); + + it('does not update preinstalled Snaps when the feature flag is off', async () => { + const registry = new MockSnapsRegistry(); + const rootMessenger = getControllerMessenger(registry); + const messenger = getSnapControllerMessenger(rootMessenger); + + const snapId = 'npm:@metamask/jsx-example-snap' as SnapId; + + const mockSnap = getPersistedSnapObject({ + id: snapId, + preinstalled: true, + }); + + const updateVersion = '1.2.1'; + + registry.resolveVersion.mockResolvedValue(updateVersion); + + const snapController = getSnapController( + getSnapControllerOptions({ + messenger, + state: { + snaps: getPersistedSnapsState(mockSnap), + }, + featureFlags: { + autoUpdatePreinstalledSnaps: false, + }, + }), + ); + + await snapController.updateRegistry(); + + const snap = snapController.get(snapId); + assert(snap); + + expect(snap.version).toStrictEqual(mockSnap.version); + expect(registry.resolveVersion).not.toHaveBeenCalled(); + + snapController.destroy(); + }); }); describe('clearState', () => { @@ -11521,8 +11649,8 @@ describe('SnapController', () => { }); }); - describe('SnapController:updateBlockedSnaps', () => { - it('calls SnapController.updateBlockedSnaps()', async () => { + describe('SnapController:updateRegistry', () => { + it('calls SnapController.updateRegistry()', async () => { const messenger = getSnapControllerMessenger(); const snapController = getSnapController( getSnapControllerOptions({ @@ -11530,12 +11658,12 @@ describe('SnapController', () => { }), ); - const updateBlockedSnapsSpy = jest - .spyOn(snapController, 'updateBlockedSnaps') + const updateRegistrySpy = jest + .spyOn(snapController, 'updateRegistry') .mockImplementation(); - await messenger.call('SnapController:updateBlockedSnaps'); - expect(updateBlockedSnapsSpy).toHaveBeenCalledTimes(1); + await messenger.call('SnapController:updateRegistry'); + expect(updateRegistrySpy).toHaveBeenCalledTimes(1); snapController.destroy(); }); diff --git a/packages/snaps-controllers/src/snaps/SnapController.ts b/packages/snaps-controllers/src/snaps/SnapController.ts index 3f2595dfaf..69228d3f8a 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.ts +++ b/packages/snaps-controllers/src/snaps/SnapController.ts @@ -1,6 +1,7 @@ -import type { - AddApprovalRequest, - UpdateRequestState, +import { + ORIGIN_METAMASK, + type AddApprovalRequest, + type UpdateRequestState, } from '@metamask/approval-controller'; import type { RestrictedMessenger, @@ -404,9 +405,9 @@ export type ClearSnapState = { /** * Checks all installed snaps against the blocklist. */ -export type UpdateBlockedSnaps = { - type: `${typeof controllerName}:updateBlockedSnaps`; - handler: SnapController['updateBlockedSnaps']; +export type UpdateRegistry = { + type: `${typeof controllerName}:updateRegistry`; + handler: SnapController['updateRegistry']; }; export type EnableSnap = { @@ -496,7 +497,7 @@ export type SnapControllerActions = | GetSnapState | HandleSnapRequest | HasSnap - | UpdateBlockedSnaps + | UpdateRegistry | UpdateSnapState | EnableSnap | DisableSnap @@ -715,6 +716,12 @@ type FeatureFlags = { * any production builds (including beta and Flask). */ forcePreinstalledSnaps?: boolean; + + /** + * Automatically update preinstalled Snaps "over the air", + * when a new version of the Snap is added to the registry. + */ + autoUpdatePreinstalledSnaps?: boolean; }; type DynamicFeatureFlags = { @@ -1220,8 +1227,8 @@ export class SnapController extends BaseController< ); this.messagingSystem.registerActionHandler( - `${controllerName}:updateBlockedSnaps`, - async () => this.updateBlockedSnaps(), + `${controllerName}:updateRegistry`, + async () => this.updateRegistry(), ); this.messagingSystem.registerActionHandler( @@ -1459,8 +1466,10 @@ export class SnapController extends BaseController< * Checks all installed snaps against the block list and * blocks/unblocks snaps as appropriate. See {@link SnapController.blockSnap} * for more information. + * + * Also updates any preinstalled Snaps to the latest allowlisted version. */ - async updateBlockedSnaps(): Promise { + async updateRegistry(): Promise { this.#assertCanUsePlatform(); await this.messagingSystem.call('SnapsRegistry:update'); @@ -1487,6 +1496,42 @@ export class SnapController extends BaseController< return this.#unblockSnap(snapId as SnapId); }), ); + + if (!this.#featureFlags.autoUpdatePreinstalledSnaps) { + return; + } + + const preinstalledVersionRange = '*' as SemVerRange; + + await Promise.allSettled( + Object.values(this.state.snaps) + .filter((snap) => snap.preinstalled) + .map(async (snap) => { + const resolvedVersion = await this.#resolveAllowlistVersion( + snap.id, + preinstalledVersionRange, + ); + + if ( + resolvedVersion !== preinstalledVersionRange && + gtVersion(resolvedVersion as unknown as SemVerVersion, snap.version) + ) { + const location = this.#detectSnapLocation(snap.id, { + versionRange: resolvedVersion, + fetch: this.#fetchFunction, + allowLocal: false, + }); + + await this.#updateSnap({ + origin: ORIGIN_METAMASK, + snapId: snap.id, + location, + versionRange: resolvedVersion, + automaticUpdate: true, + }); + } + }), + ); } /** @@ -2864,6 +2909,8 @@ export class SnapController extends BaseController< * @param options.snapId - The id of the Snap to be updated. * @param options.location - The location implementation of the snap. * @param options.versionRange - A semver version range in which the maximum version will be chosen. + * @param options.automaticUpdate - An optional boolean flag to indicate whether this update should be done + * automatically. * @returns The snap metadata if updated, `null` otherwise. */ async #updateSnap({ @@ -2871,26 +2918,32 @@ export class SnapController extends BaseController< snapId, location, versionRange, + automaticUpdate = false, }: { origin: string; snapId: SnapId; location: SnapLocation; versionRange: SemVerRange; + automaticUpdate?: boolean; }): Promise { this.#assertCanInstallSnaps(); this.#assertCanUsePlatform(); const snap = this.getExpect(snapId); - if (snap.preinstalled) { + const { preinstalled, removable, hidden, hideSnapBranding } = snap; + + if (preinstalled && !automaticUpdate) { throw new Error('Preinstalled Snaps cannot be manually updated.'); } - let pendingApproval = this.#createApproval({ - origin, - snapId, - type: SNAP_APPROVAL_UPDATE, - }); + let pendingApproval = automaticUpdate + ? null + : this.#createApproval({ + origin, + snapId, + type: SNAP_APPROVAL_UPDATE, + }); try { this.messagingSystem.publish( @@ -2943,26 +2996,37 @@ export class SnapController extends BaseController< manifest.initialConnections ?? {}, ); - this.#updateApproval(pendingApproval.id, { - permissions: newPermissions, - newVersion: manifest.version, - newPermissions, - approvedPermissions, - unusedPermissions, - newConnections, - unusedConnections, - approvedConnections, - loading: false, - }); + let approvedNewPermissions; + let requestData; + + if (pendingApproval) { + this.#updateApproval(pendingApproval.id, { + permissions: newPermissions, + newVersion: manifest.version, + newPermissions, + approvedPermissions, + unusedPermissions, + newConnections, + unusedConnections, + approvedConnections, + loading: false, + }); - const { permissions: approvedNewPermissions, ...requestData } = - (await pendingApproval.promise) as PermissionsRequest; + const { permissions, ...rest } = + (await pendingApproval.promise) as PermissionsRequest; - pendingApproval = this.#createApproval({ - origin, - snapId, - type: SNAP_APPROVAL_RESULT, - }); + approvedNewPermissions = permissions; + requestData = rest; + + pendingApproval = this.#createApproval({ + origin, + snapId, + type: SNAP_APPROVAL_RESULT, + }); + } else { + assert(automaticUpdate); + approvedNewPermissions = newPermissions; + } if (this.isRunning(snapId)) { await this.stopSnap(snapId, SnapStatusEvents.Stop); @@ -2974,6 +3038,10 @@ export class SnapController extends BaseController< origin, id: snapId, files: newSnap, + removable, + preinstalled, + hidden, + hideSnapBranding, isUpdate: true, }); @@ -3017,10 +3085,12 @@ export class SnapController extends BaseController< const truncatedSnap = this.getTruncatedExpect(snapId); - this.#updateApproval(pendingApproval.id, { - loading: false, - type: SNAP_APPROVAL_UPDATE, - }); + if (pendingApproval) { + this.#updateApproval(pendingApproval.id, { + loading: false, + type: SNAP_APPROVAL_UPDATE, + }); + } return truncatedSnap; } catch (error) { @@ -3029,11 +3099,13 @@ export class SnapController extends BaseController< const errorString = error instanceof Error ? error.message : error.toString(); - this.#updateApproval(pendingApproval.id, { - loading: false, - error: errorString, - type: SNAP_APPROVAL_UPDATE, - }); + if (pendingApproval) { + this.#updateApproval(pendingApproval.id, { + loading: false, + error: errorString, + type: SNAP_APPROVAL_UPDATE, + }); + } this.messagingSystem.publish( 'SnapController:snapInstallFailed',