From 338e7904e427aa9a1088ebb21c92347737aca966 Mon Sep 17 00:00:00 2001
From: Maarten Zuidhoorn <maarten@zuidhoorn.com>
Date: Mon, 9 Sep 2024 11:16:13 +0200
Subject: [PATCH] Improve `validateLink` error handling (#2702)

This improves the error handling of `validateLink` in case of assertion
errors. Previously we used `instanceof`, which can cause false negatives
when importing from libraries. I've changed it to look at the error code
instead.
---
 packages/snaps-utils/coverage.json   |  2 +-
 packages/snaps-utils/src/ui.test.tsx | 56 ++++++++++++++++++++++++++++
 packages/snaps-utils/src/ui.tsx      |  5 +--
 3 files changed, 59 insertions(+), 4 deletions(-)

diff --git a/packages/snaps-utils/coverage.json b/packages/snaps-utils/coverage.json
index fab111efc3..7c789ce487 100644
--- a/packages/snaps-utils/coverage.json
+++ b/packages/snaps-utils/coverage.json
@@ -2,5 +2,5 @@
   "branches": 99.73,
   "functions": 98.9,
   "lines": 99.43,
-  "statements": 96.32
+  "statements": 96.33
 }
diff --git a/packages/snaps-utils/src/ui.test.tsx b/packages/snaps-utils/src/ui.test.tsx
index afb16a96d3..569ace6a65 100644
--- a/packages/snaps-utils/src/ui.test.tsx
+++ b/packages/snaps-utils/src/ui.test.tsx
@@ -42,6 +42,7 @@ import {
   walkJsx,
   getJsxChildren,
   serialiseJsx,
+  validateLink,
 } from './ui';
 
 describe('getTextChildren', () => {
@@ -541,6 +542,61 @@ describe('getJsxElementFromComponent', () => {
   });
 });
 
+describe('validateLink', () => {
+  it('passes for a valid link', () => {
+    const fn = jest.fn().mockReturnValue(false);
+
+    expect(() => validateLink('https://foo.bar', fn)).not.toThrow();
+    expect(() => validateLink('mailto:foo@bar.com', fn)).not.toThrow();
+
+    expect(fn).toHaveBeenCalledTimes(2);
+    expect(fn).toHaveBeenCalledWith('foo.bar');
+    expect(fn).toHaveBeenCalledWith('bar.com');
+  });
+
+  it('throws an error for an invalid protocol', () => {
+    const fn = jest.fn().mockReturnValue(false);
+
+    expect(() => validateLink('http://foo.bar', fn)).toThrow(
+      'Invalid URL: Protocol must be one of: https:, mailto:.',
+    );
+
+    expect(fn).not.toHaveBeenCalled();
+  });
+
+  it('throws an error for an invalid URL', () => {
+    const fn = jest.fn().mockReturnValue(false);
+
+    expect(() => validateLink('foo.bar', fn)).toThrow(
+      'Invalid URL: Unable to parse URL.',
+    );
+
+    expect(fn).not.toHaveBeenCalled();
+  });
+
+  it('throws an error for a phishing link', () => {
+    const fn = jest.fn().mockReturnValue(true);
+
+    expect(() => validateLink('https://test.metamask-phishing.io', fn)).toThrow(
+      'Invalid URL: The specified URL is not allowed.',
+    );
+
+    expect(fn).toHaveBeenCalledTimes(1);
+    expect(fn).toHaveBeenCalledWith('test.metamask-phishing.io');
+  });
+
+  it('throws an error for a phishing email', () => {
+    const fn = jest.fn().mockReturnValue(true);
+
+    expect(() =>
+      validateLink('mailto:foo@test.metamask-phishing.io', fn),
+    ).toThrow('Invalid URL: The specified URL is not allowed.');
+
+    expect(fn).toHaveBeenCalledTimes(1);
+    expect(fn).toHaveBeenCalledWith('test.metamask-phishing.io');
+  });
+});
+
 describe('validateTextLinks', () => {
   it('passes for valid links', () => {
     expect(() =>
diff --git a/packages/snaps-utils/src/ui.tsx b/packages/snaps-utils/src/ui.tsx
index 9da9518ed5..b53c8b8ef4 100644
--- a/packages/snaps-utils/src/ui.tsx
+++ b/packages/snaps-utils/src/ui.tsx
@@ -33,7 +33,6 @@ import {
 import {
   assert,
   assertExhaustive,
-  AssertionError,
   hasProperty,
   isPlainObject,
 } from '@metamask/utils';
@@ -332,7 +331,7 @@ function getMarkdownLinks(text: string) {
  * @param isOnPhishingList - The function that checks the link against the
  * phishing list.
  */
-function validateLink(
+export function validateLink(
   link: string,
   isOnPhishingList: (url: string) => boolean,
 ) {
@@ -350,7 +349,7 @@ function validateLink(
   } catch (error) {
     throw new Error(
       `Invalid URL: ${
-        error instanceof AssertionError ? error.message : 'Unable to parse URL.'
+        error?.code === 'ERR_ASSERTION' ? error.message : 'Unable to parse URL.'
       }`,
     );
   }