From e52d59c22f3223e1c3a511f8f458384907b9994a Mon Sep 17 00:00:00 2001 From: legobt <6wbvkn0j@anonaddy.me> Date: Wed, 15 May 2024 22:12:33 +0000 Subject: [PATCH 1/8] feat(snaps-utils): Allow overriding allowed protocols in validateLink --- packages/snaps-utils/src/ui.tsx | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/packages/snaps-utils/src/ui.tsx b/packages/snaps-utils/src/ui.tsx index cad94f4d53..6d5fa6b149 100644 --- a/packages/snaps-utils/src/ui.tsx +++ b/packages/snaps-utils/src/ui.tsx @@ -40,7 +40,7 @@ import { lexer, walkTokens } from 'marked'; import type { Token, Tokens } from 'marked'; const MAX_TEXT_LENGTH = 50_000; // 50 kb -const ALLOWED_PROTOCOLS = ['https:', 'mailto:']; +const DEFAULT_ALLOWED_PROTOCOLS = ['https:', 'mailto:']; /** * Get the button variant from a legacy button component variant. @@ -320,16 +320,18 @@ function getMarkdownLinks(text: string) { * @param link - The link to validate. * @param isOnPhishingList - The function that checks the link against the * phishing list. + * @param allowedProtocols - Allowed protocols (example: ['https:']) */ function validateLink( link: string, isOnPhishingList: (url: string) => boolean, + allowedProtocols: string[], ) { try { const url = new URL(link); assert( - ALLOWED_PROTOCOLS.includes(url.protocol), - `Protocol must be one of: ${ALLOWED_PROTOCOLS.join(', ')}.`, + allowedProtocols.includes(url.protocol), + `Protocol must be one of: ${allowedProtocols.join(', ')}.`, ); const hostname = @@ -352,16 +354,18 @@ function validateLink( * @param text - The text to verify. * @param isOnPhishingList - The function that checks the link against the * phishing list. + * @param allowedProtocols - Allowed protocols (example: ['https:']) * @throws If the text contains a link that is not allowed. */ export function validateTextLinks( text: string, isOnPhishingList: (url: string) => boolean, + allowedProtocols: string[] = DEFAULT_ALLOWED_PROTOCOLS, ) { const links = getMarkdownLinks(text); for (const link of links) { - validateLink(link.href, isOnPhishingList); + validateLink(link.href, isOnPhishingList, allowedProtocols); } } @@ -372,17 +376,19 @@ export function validateTextLinks( * @param node - The JSX node to walk. * @param isOnPhishingList - The function that checks the link against the * phishing list. + * @param allowedProtocols - Allowed protocols (example: ['https:']) */ export function validateJsxLinks( node: JSXElement, isOnPhishingList: (url: string) => boolean, + allowedProtocols: string[] = DEFAULT_ALLOWED_PROTOCOLS, ) { walkJsx(node, (childNode) => { if (childNode.type !== 'Link') { return; } - validateLink(childNode.props.href, isOnPhishingList); + validateLink(childNode.props.href, isOnPhishingList, allowedProtocols); }); } From cb451def0474800f893022f29e8d3d63318af56d Mon Sep 17 00:00:00 2001 From: legobt <6wbvkn0j@anonaddy.me> Date: Wed, 15 May 2024 22:30:08 +0000 Subject: [PATCH 2/8] add test --- packages/snaps-utils/src/ui.test.tsx | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/packages/snaps-utils/src/ui.test.tsx b/packages/snaps-utils/src/ui.test.tsx index 1f61c52a0c..2bd28ad24a 100644 --- a/packages/snaps-utils/src/ui.test.tsx +++ b/packages/snaps-utils/src/ui.test.tsx @@ -531,6 +531,31 @@ describe('getJsxElementFromComponent', () => { }); describe('validateTextLinks', () => { + it('handles custom protocols', () => { + const allowedProtocols = ['http:', 'file:']; + expect(() => + validateTextLinks( + '[test](http://foo.bar)', + () => false, + allowedProtocols, + ), + ).not.toThrow(); + expect(() => + validateTextLinks( + '[test](https://foo.bar)', + () => false, + allowedProtocols, + ), + ).toThrow('Invalid URL: Protocol must be one of: http:, file:.'); + expect(() => + validateTextLinks( + '', + () => false, + allowedProtocols, + ), + ).not.toThrow(); + }); + it('passes for valid links', () => { expect(() => validateTextLinks('[test](https://foo.bar)', () => false), From eb9420c090db29d7dd8dea3d65399a582f3d7e69 Mon Sep 17 00:00:00 2001 From: legobt <6wbvkn0j@anonaddy.me> Date: Wed, 15 May 2024 22:30:32 +0000 Subject: [PATCH 3/8] lint:fix --- packages/snaps-utils/src/ui.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/snaps-utils/src/ui.tsx b/packages/snaps-utils/src/ui.tsx index 6d5fa6b149..049d69d267 100644 --- a/packages/snaps-utils/src/ui.tsx +++ b/packages/snaps-utils/src/ui.tsx @@ -320,7 +320,7 @@ function getMarkdownLinks(text: string) { * @param link - The link to validate. * @param isOnPhishingList - The function that checks the link against the * phishing list. - * @param allowedProtocols - Allowed protocols (example: ['https:']) + * @param allowedProtocols - Allowed protocols (example: ['https:']). */ function validateLink( link: string, @@ -354,7 +354,7 @@ function validateLink( * @param text - The text to verify. * @param isOnPhishingList - The function that checks the link against the * phishing list. - * @param allowedProtocols - Allowed protocols (example: ['https:']) + * @param allowedProtocols - Allowed protocols (example: ['https:']). * @throws If the text contains a link that is not allowed. */ export function validateTextLinks( @@ -376,7 +376,7 @@ export function validateTextLinks( * @param node - The JSX node to walk. * @param isOnPhishingList - The function that checks the link against the * phishing list. - * @param allowedProtocols - Allowed protocols (example: ['https:']) + * @param allowedProtocols - Allowed protocols (example: ['https:']). */ export function validateJsxLinks( node: JSXElement, From 55072df3c0d132b69bf2d070e76c5ebb90bddc93 Mon Sep 17 00:00:00 2001 From: legobt <6wbvkn0j@anonaddy.me> Date: Wed, 15 May 2024 22:31:40 +0000 Subject: [PATCH 4/8] add coverage update --- packages/snaps-utils/coverage.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/snaps-utils/coverage.json b/packages/snaps-utils/coverage.json index ce1f9b87d8..6f02f472e8 100644 --- a/packages/snaps-utils/coverage.json +++ b/packages/snaps-utils/coverage.json @@ -1,5 +1,5 @@ { - "branches": 96.7, + "branches": 96.72, "functions": 98.72, "lines": 98.81, "statements": 94.79 From 59fd2bef36fa0147e6e17d5c6fa77b560c44d4d9 Mon Sep 17 00:00:00 2001 From: legobt <6wbvkn0j@anonaddy.me> Date: Wed, 15 May 2024 22:32:57 +0000 Subject: [PATCH 5/8] chore: reorder test --- packages/snaps-utils/src/ui.test.tsx | 50 ++++++++++++++-------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/packages/snaps-utils/src/ui.test.tsx b/packages/snaps-utils/src/ui.test.tsx index 2bd28ad24a..4768a2ab3a 100644 --- a/packages/snaps-utils/src/ui.test.tsx +++ b/packages/snaps-utils/src/ui.test.tsx @@ -531,31 +531,6 @@ describe('getJsxElementFromComponent', () => { }); describe('validateTextLinks', () => { - it('handles custom protocols', () => { - const allowedProtocols = ['http:', 'file:']; - expect(() => - validateTextLinks( - '[test](http://foo.bar)', - () => false, - allowedProtocols, - ), - ).not.toThrow(); - expect(() => - validateTextLinks( - '[test](https://foo.bar)', - () => false, - allowedProtocols, - ), - ).toThrow('Invalid URL: Protocol must be one of: http:, file:.'); - expect(() => - validateTextLinks( - '', - () => false, - allowedProtocols, - ), - ).not.toThrow(); - }); - it('passes for valid links', () => { expect(() => validateTextLinks('[test](https://foo.bar)', () => false), @@ -640,6 +615,31 @@ describe('validateTextLinks', () => { 'Invalid URL: Unable to parse URL.', ); }); + + it('handles custom protocols', () => { + const allowedProtocols = ['http:', 'file:']; + expect(() => + validateTextLinks( + '[test](http://foo.bar)', + () => false, + allowedProtocols, + ), + ).not.toThrow(); + expect(() => + validateTextLinks( + '[test](https://foo.bar)', + () => false, + allowedProtocols, + ), + ).toThrow('Invalid URL: Protocol must be one of: http:, file:.'); + expect(() => + validateTextLinks( + '', + () => false, + allowedProtocols, + ), + ).not.toThrow(); + }); }); describe('validateJsxLinks', () => { From 79af980e7f48913836f53634911e73bbdf3624a9 Mon Sep 17 00:00:00 2001 From: legobt <6wbvkn0j@anonaddy.me> Date: Thu, 16 May 2024 00:27:36 +0000 Subject: [PATCH 6/8] feat(rpc-methods): expose allowedProtocols for link validation --- .../src/interface/SnapInterfaceController.ts | 1 + packages/snaps-rpc-methods/src/restricted/notify.ts | 7 +++++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/snaps-controllers/src/interface/SnapInterfaceController.ts b/packages/snaps-controllers/src/interface/SnapInterfaceController.ts index 854a36f450..fae47ba9e7 100644 --- a/packages/snaps-controllers/src/interface/SnapInterfaceController.ts +++ b/packages/snaps-controllers/src/interface/SnapInterfaceController.ts @@ -272,6 +272,7 @@ export class SnapInterfaceController extends BaseController< ); await this.#triggerPhishingListUpdate(); + // TODO: Expose configuration for allowedProtocols in validateJsxLinks validateJsxLinks(element, this.#checkPhishingList.bind(this)); } } diff --git a/packages/snaps-rpc-methods/src/restricted/notify.ts b/packages/snaps-rpc-methods/src/restricted/notify.ts index fdf3001be3..216a40db1f 100644 --- a/packages/snaps-rpc-methods/src/restricted/notify.ts +++ b/packages/snaps-rpc-methods/src/restricted/notify.ts @@ -111,6 +111,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 allowedProtocols - Allowed protocols for links (example: ['https:']). * @returns The method implementation which returns `null` on success. * @throws If the params are invalid. */ @@ -119,7 +120,9 @@ export function getImplementation({ showInAppNotification, isOnPhishingList, maybeUpdatePhishingList, -}: NotifyMethodHooks) { +}: NotifyMethodHooks, + allowedProtocols: string[], +) { return async function implementation( args: RestrictedMethodOptions, ): Promise { @@ -132,7 +135,7 @@ export function getImplementation({ await maybeUpdatePhishingList(); - validateTextLinks(validatedParams.message, isOnPhishingList); + validateTextLinks(validatedParams.message, isOnPhishingList, allowedProtocols); switch (validatedParams.type) { case NotificationType.Native: From 2e0748a982a2f8eb0d2a54df0f967dcec2658753 Mon Sep 17 00:00:00 2001 From: legobt <6wbvkn0j@anonaddy.me> Date: Thu, 16 May 2024 08:12:48 +0000 Subject: [PATCH 7/8] lint:fix --- .../src/restricted/notify.ts | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/packages/snaps-rpc-methods/src/restricted/notify.ts b/packages/snaps-rpc-methods/src/restricted/notify.ts index 216a40db1f..5ebc7cb56b 100644 --- a/packages/snaps-rpc-methods/src/restricted/notify.ts +++ b/packages/snaps-rpc-methods/src/restricted/notify.ts @@ -115,12 +115,13 @@ export const notifyBuilder = Object.freeze({ * @returns The method implementation which returns `null` on success. * @throws If the params are invalid. */ -export function getImplementation({ - showNativeNotification, - showInAppNotification, - isOnPhishingList, - maybeUpdatePhishingList, -}: NotifyMethodHooks, +export function getImplementation( + { + showNativeNotification, + showInAppNotification, + isOnPhishingList, + maybeUpdatePhishingList, + }: NotifyMethodHooks, allowedProtocols: string[], ) { return async function implementation( @@ -135,7 +136,11 @@ export function getImplementation({ await maybeUpdatePhishingList(); - validateTextLinks(validatedParams.message, isOnPhishingList, allowedProtocols); + validateTextLinks( + validatedParams.message, + isOnPhishingList, + allowedProtocols, + ); switch (validatedParams.type) { case NotificationType.Native: From d066a8c5de0dc4e6359aaee2e2776907e2abffcf Mon Sep 17 00:00:00 2001 From: legobt <6wbvkn0j@anonaddy.me> Date: Thu, 16 May 2024 08:19:45 +0000 Subject: [PATCH 8/8] fix: allowedProtocols optional in notify getImplementation --- packages/snaps-rpc-methods/src/restricted/notify.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/snaps-rpc-methods/src/restricted/notify.ts b/packages/snaps-rpc-methods/src/restricted/notify.ts index 5ebc7cb56b..1155df448d 100644 --- a/packages/snaps-rpc-methods/src/restricted/notify.ts +++ b/packages/snaps-rpc-methods/src/restricted/notify.ts @@ -122,7 +122,7 @@ export function getImplementation( isOnPhishingList, maybeUpdatePhishingList, }: NotifyMethodHooks, - allowedProtocols: string[], + allowedProtocols?: string[], ) { return async function implementation( args: RestrictedMethodOptions,