Allow for metamask: schemed URLs (#2719)

Validation logic has been updated to allow for an `Icon` to also be included as a `Link` child. A new URL scheme is also introduced `metamask:` (see [SIP-22](MetaMask/SIPs#134)), this scheme will allow a snap to navigate to the extension and other snap pages through a `metamask:` schemed URL.
MetaMask · Sep 24, 2024 · 8b194d6 · 8b194d6
1 parent 3a7cc20
commit 8b194d6
Show file tree

Hide file tree

Showing 15 changed files with 360 additions and 65 deletions.
diff --git a/packages/examples/packages/browserify-plugin/snap.manifest.json b/packages/examples/packages/browserify-plugin/snap.manifest.json
@@ -7,7 +7,7 @@
     "url": "https://github.com/MetaMask/snaps.git"
   },
   "source": {
-    "shasum": "jwgw2+/MUcLjedTMSpFqL/dhwR8/j3gHWmNJVevvfWE=",
+    "shasum": "oeprRlfJxxGH+tvOyO7i4hy3l0SjzR3rA73tZS7vlZk=",
     "location": {
       "npm": {
         "filePath": "dist/bundle.js",

diff --git a/packages/examples/packages/browserify/snap.manifest.json b/packages/examples/packages/browserify/snap.manifest.json
@@ -7,7 +7,7 @@
     "url": "https://github.com/MetaMask/snaps.git"
   },
   "source": {
-    "shasum": "u+bnPdpw6us0nDDr3k28cfDoIA+5CT1Y7A88nYJLNGk=",
+    "shasum": "PL+Bg6eUiAl4uBSuGbX4gdcrgxFZIedy/N7FjVMzKIY=",
     "location": {
       "npm": {
         "filePath": "dist/bundle.js",

diff --git a/packages/snaps-controllers/coverage.json b/packages/snaps-controllers/coverage.json
@@ -1,6 +1,6 @@
 {
   "branches": 92.6,
-  "functions": 96.95,
-  "lines": 98.02,
-  "statements": 97.72
+  "functions": 96.65,
+  "lines": 97.97,
+  "statements": 97.67
 }
diff --git a/packages/snaps-controllers/src/interface/SnapInterfaceController.ts b/packages/snaps-controllers/src/interface/SnapInterfaceController.ts
@@ -25,6 +25,7 @@ import { assert } from '@metamask/utils';
 import { castDraft } from 'immer';
 import { nanoid } from 'nanoid';
 
+import type { GetSnap } from '../snaps';
 import {
   constructState,
   getJsxInterface,
@@ -74,7 +75,8 @@ export type SnapInterfaceControllerAllowedActions =
   | TestOrigin
   | MaybeUpdateState
   | HasApprovalRequest
-  | AcceptRequest;
+  | AcceptRequest
+  | GetSnap;
 
 export type SnapInterfaceControllerActions =
   | CreateInterface
@@ -379,6 +381,10 @@ export class SnapInterfaceController extends BaseController<
     );
 
     await this.#triggerPhishingListUpdate();
-    validateJsxLinks(element, this.#checkPhishingList.bind(this));
+    validateJsxLinks(
+      element,
+      this.#checkPhishingList.bind(this),
+      (id: string) => this.messagingSystem.call('SnapController:get', id),
+    );
   }
 }
diff --git a/packages/snaps-rpc-methods/src/restricted/notify.test.ts b/packages/snaps-rpc-methods/src/restricted/notify.test.ts
@@ -20,6 +20,7 @@ describe('snap_notify', () => {
         showInAppNotification: jest.fn(),
         isOnPhishingList: jest.fn(),
         maybeUpdatePhishingList: jest.fn(),
+        getSnap: jest.fn(),
       };
 
       expect(
@@ -41,13 +42,15 @@ describe('snap_notify', () => {
       const showNativeNotification = jest.fn().mockResolvedValueOnce(true);
       const showInAppNotification = jest.fn().mockResolvedValueOnce(true);
       const isOnPhishingList = jest.fn().mockResolvedValueOnce(false);
+      const getSnap = jest.fn();
       const maybeUpdatePhishingList = jest.fn();
 
       const notificationImplementation = getImplementation({
         showNativeNotification,
         showInAppNotification,
         isOnPhishingList,
         maybeUpdatePhishingList,
+        getSnap,
       });
 
       await notificationImplementation({
@@ -72,12 +75,14 @@ describe('snap_notify', () => {
       const showInAppNotification = jest.fn().mockResolvedValueOnce(true);
       const isOnPhishingList = jest.fn().mockResolvedValueOnce(false);
       const maybeUpdatePhishingList = jest.fn();
+      const getSnap = jest.fn();
 
       const notificationImplementation = getImplementation({
         showNativeNotification,
         showInAppNotification,
         isOnPhishingList,
         maybeUpdatePhishingList,
+        getSnap,
       });
 
       await notificationImplementation({
@@ -102,12 +107,14 @@ describe('snap_notify', () => {
       const showInAppNotification = jest.fn().mockResolvedValueOnce(true);
       const isOnPhishingList = jest.fn().mockResolvedValueOnce(false);
       const maybeUpdatePhishingList = jest.fn();
+      const getSnap = jest.fn();
 
       const notificationImplementation = getImplementation({
         showNativeNotification,
         showInAppNotification,
         isOnPhishingList,
         maybeUpdatePhishingList,
+        getSnap,
       });
 
       await notificationImplementation({
@@ -132,12 +139,14 @@ describe('snap_notify', () => {
       const showInAppNotification = jest.fn().mockResolvedValueOnce(true);
       const isOnPhishingList = jest.fn().mockResolvedValueOnce(false);
       const maybeUpdatePhishingList = jest.fn();
+      const getSnap = jest.fn();
 
       const notificationImplementation = getImplementation({
         showNativeNotification,
         showInAppNotification,
         isOnPhishingList,
         maybeUpdatePhishingList,
+        getSnap,
       });
 
       await expect(
@@ -160,12 +169,14 @@ describe('snap_notify', () => {
       const showInAppNotification = jest.fn().mockResolvedValueOnce(true);
       const isOnPhishingList = jest.fn().mockResolvedValueOnce(true);
       const maybeUpdatePhishingList = jest.fn();
+      const getSnap = jest.fn();
 
       const notificationImplementation = getImplementation({
         showNativeNotification,
         showInAppNotification,
         isOnPhishingList,
         maybeUpdatePhishingList,
+        getSnap,
       });
 
       await expect(
@@ -187,12 +198,14 @@ describe('snap_notify', () => {
       const showInAppNotification = jest.fn().mockResolvedValueOnce(true);
       const isOnPhishingList = jest.fn().mockResolvedValueOnce(true);
       const maybeUpdatePhishingList = jest.fn();
+      const getSnap = jest.fn();
 
       const notificationImplementation = getImplementation({
         showNativeNotification,
         showInAppNotification,
         isOnPhishingList,
         maybeUpdatePhishingList,
+        getSnap,
       });
 
       await expect(
@@ -207,7 +220,7 @@ describe('snap_notify', () => {
           },
         }),
       ).rejects.toThrow(
-        'Invalid URL: Protocol must be one of: https:, mailto:.',
+        'Invalid URL: Protocol must be one of: https:, mailto:, metamask:.',
       );
     });
   });

diff --git a/packages/snaps-rpc-methods/src/restricted/notify.ts b/packages/snaps-rpc-methods/src/restricted/notify.ts
@@ -11,7 +11,7 @@ import type {
   NotifyResult,
   EnumToUnion,
 } from '@metamask/snaps-sdk';
-import { validateTextLinks } from '@metamask/snaps-utils';
+import { type Snap, validateTextLinks } from '@metamask/snaps-utils';
 import type { NonEmptyArray } from '@metamask/utils';
 import { isObject } from '@metamask/utils';
 
@@ -53,6 +53,8 @@ export type NotifyMethodHooks = {
   isOnPhishingList: (url: string) => boolean;
 
   maybeUpdatePhishingList: () => Promise<void>;
+
+  getSnap: (snapId: string) => Snap | undefined;
 };
 
 type SpecificationBuilderOptions = {
@@ -95,6 +97,7 @@ const methodHooks: MethodHooksObject<NotifyMethodHooks> = {
   showInAppNotification: true,
   isOnPhishingList: true,
   maybeUpdatePhishingList: true,
+  getSnap: true,
 };
 
 export const notifyBuilder = Object.freeze({
@@ -111,6 +114,7 @@ export const notifyBuilder = Object.freeze({
  * @param hooks.showInAppNotification - A function that shows a notification in the MetaMask UI.
  * @param hooks.isOnPhishingList - A function that checks for links against the phishing list.
  * @param hooks.maybeUpdatePhishingList - A function that updates the phishing list if needed.
+ * @param hooks.getSnap - A function that checks if a snap is installed.
  * @returns The method implementation which returns `null` on success.
  * @throws If the params are invalid.
  */
@@ -119,6 +123,7 @@ export function getImplementation({
   showInAppNotification,
   isOnPhishingList,
   maybeUpdatePhishingList,
+  getSnap,
 }: NotifyMethodHooks) {
   return async function implementation(
     args: RestrictedMethodOptions<NotifyParams>,
@@ -132,7 +137,7 @@ export function getImplementation({
 
     await maybeUpdatePhishingList();
 
-    validateTextLinks(validatedParams.message, isOnPhishingList);
+    validateTextLinks(validatedParams.message, isOnPhishingList, getSnap);
 
     switch (validatedParams.type) {
       case NotificationType.Native:

diff --git a/packages/snaps-sdk/src/jsx/components/Link.test.tsx b/packages/snaps-sdk/src/jsx/components/Link.test.tsx
@@ -1,3 +1,4 @@
+import { Icon } from './Icon';
 import { Link } from './Link';
 
 describe('Link', () => {
@@ -27,6 +28,27 @@ describe('Link', () => {
     });
   });
 
+  it('renders a link with an icon', () => {
+    const result = (
+      <Link href="metamask://client/">
+        <Icon name="arrow-left" size="md" />
+      </Link>
+    );
+
+    expect(result).toStrictEqual({
+      type: 'Link',
+      key: null,
+      props: {
+        href: 'metamask://client/',
+        children: {
+          type: 'Icon',
+          key: null,
+          props: { name: 'arrow-left', size: 'md' },
+        },
+      },
+    });
+  });
+
   it('renders a link with a conditional value', () => {
     const result = (
       <Link href="https://example.com">

diff --git a/packages/snaps-sdk/src/jsx/components/Link.ts b/packages/snaps-sdk/src/jsx/components/Link.ts
@@ -1,11 +1,15 @@
 import type { SnapsChildren } from '../component';
 import { createSnapComponent } from '../component';
 import type { StandardFormattingElement } from './formatting';
+import { type IconElement } from './Icon';
+import { type ImageElement } from './Image';
 
 /**
  * The children of the {@link Link} component.
  */
-export type LinkChildren = SnapsChildren<string | StandardFormattingElement>;
+export type LinkChildren = SnapsChildren<
+  string | StandardFormattingElement | IconElement | ImageElement
+>;
 
 /**
  * The props of the {@link Link} component.

diff --git a/packages/snaps-sdk/src/jsx/validation.test.tsx b/packages/snaps-sdk/src/jsx/validation.test.tsx
@@ -991,12 +991,14 @@ describe('ImageStruct', () => {
 });
 
 describe('LinkStruct', () => {
-  it.each([<Link href="https://example.com">foo</Link>])(
-    'validates a link element',
-    (value) => {
-      expect(is(value, LinkStruct)).toBe(true);
-    },
-  );
+  it.each([
+    <Link href="https://example.com">foo</Link>,
+    <Link href="metamask://client/">
+      <Icon name="arrow-left" size="md" />
+    </Link>,
+  ])('validates a link element', (value) => {
+    expect(is(value, LinkStruct)).toBe(true);
+  });
 
   it.each([
     'foo',

diff --git a/packages/snaps-sdk/src/jsx/validation.ts b/packages/snaps-sdk/src/jsx/validation.ts
@@ -590,7 +590,7 @@ export const HeadingStruct: Describe<HeadingElement> = element('Heading', {
  */
 export const LinkStruct: Describe<LinkElement> = element('Link', {
   href: string(),
-  children: children([FormattingStruct, string()]),
+  children: children([FormattingStruct, string(), IconStruct, ImageStruct]),
 });
 
 /**

diff --git a/packages/snaps-utils/coverage.json b/packages/snaps-utils/coverage.json
@@ -1,6 +1,6 @@
 {
-  "branches": 99.73,
+  "branches": 99.74,
   "functions": 98.9,
-  "lines": 99.44,
-  "statements": 96.34
+  "lines": 99.45,
+  "statements": 96.29
 }