Validate local snap URL protocols

Author: rekmarks

packages/snaps-controllers/jest.config.js

@@ -5,7 +5,7 @@ const baseConfig = require('../../jest.config.base');
 module.exports = deepmerge(baseConfig, {
   coverageThreshold: {
     global: {
-      branches: 88.85,
+      branches: 89.25,
       functions: 96.58,
       lines: 96.74,
       statements: 96.74,

packages/snaps-controllers/src/snaps/SnapController.test.ts

@@ -2282,6 +2282,49 @@ describe('SnapController', () => {
       );
     });
 
+    it.each([
+      'local:foo',
+      'local:foo://localhost:8080',
+      'local:http://foo:8080',
+      'local:https://foo:8080',
+    ])(
+      'returns an error on invalid local snap URL in id',
+      async (invalidLocalId) => {
+        const messenger = getSnapControllerMessenger();
+        const controller = getSnapController(
+          getSnapControllerOptions({ messenger }),
+        );
+
+        const callActionMock = jest
+          .spyOn(messenger, 'call')
+          .mockImplementation((method, ..._args: unknown[]) => {
+            if (method === 'PermissionController:hasPermission') {
+              return true;
+            }
+
+            return false;
+          });
+
+        const result = await controller.installSnaps(MOCK_ORIGIN, {
+          [invalidLocalId]: {},
+        });
+
+        expect(result).toStrictEqual({
+          [invalidLocalId]: {
+            error: expect.objectContaining({
+              message: expect.stringMatching(/^Invalid URL:/iu),
+            }),
+          },
+        });
+        expect(callActionMock).toHaveBeenCalledTimes(1);
+        expect(callActionMock).toHaveBeenCalledWith(
+          'PermissionController:hasPermission',
+          MOCK_ORIGIN,
+          expect.anything(),
+        );
+      },
+    );
+
     it('updates a snap', async () => {
       const newVersion = '1.0.2';
       const newVersionRange = '>=1.0.1';
@@ -2507,11 +2550,19 @@ describe('SnapController', () => {
   });
 
   describe('updateSnap', () => {
-    it('throws an error on invalid snap id', async () => {
-      await expect(async () =>
-        getSnapController().updateSnap(MOCK_ORIGIN, 'local:foo'),
-      ).rejects.toThrow('Snap "local:foo" not found');
-    });
+    it.each([
+      'local:foo',
+      'local:foo://localhost:8080',
+      'local:http://foo:8080',
+      'local:https://foo:8080',
+    ])(
+      'throws an error on invalid local snap URL in id',
+      async (invalidLocalId) => {
+        await expect(async () =>
+          getSnapController().updateSnap(MOCK_ORIGIN, invalidLocalId as any),
+        ).rejects.toThrow(`Snap "${invalidLocalId}" not found`);
+      },
+    );
 
     it('throws an error if the specified SemVer range is invalid', async () => {
       const controller = getSnapController(

packages/snaps-controllers/src/snaps/SnapController.ts

@@ -30,6 +30,7 @@ import {
   InstallSnapsResult,
   isValidSemVerRange,
   LOCALHOST_HOSTNAMES,
+  LOCALHOST_PROTOCOLS,
   NpmSnapFileNames,
   PersistedSnap,
   ProcessSnapResult,
@@ -2069,21 +2070,28 @@ export class SnapController extends BaseController<
   /**
    * Fetches the manifest and source code of a local snap.
    *
-   * @param localhostUrl - The localhost URL to download from.
+   * @param localhostUrlString - The localhost URL to download from.
    * @returns The validated manifest and the source code.
    */
-  async #fetchLocalSnap(localhostUrl: string): Promise<FetchSnapResult> {
+  async #fetchLocalSnap(localhostUrlString: string): Promise<FetchSnapResult> {
+    const localhostUrl = new URL(localhostUrlString);
+    if (
+      !LOCALHOST_PROTOCOLS.has(localhostUrl.protocol) ||
+      !LOCALHOST_HOSTNAMES.has(localhostUrl.hostname)
+    ) {
+      throw new Error(
+        `Invalid URL: Locally hosted snaps must be hosted on localhost via one of the following protocols [ ${Array.from(
+          LOCALHOST_PROTOCOLS,
+        ).join(', ')} ]. Received URL: "${localhostUrl.toString()}"`,
+      );
+    }
+
     // Local snaps are mostly used for development purposes. Fetches were cached in the browser and were not requested
     // afterwards which lead to confusing development where old versions of snaps were installed.
     // Thus we disable caching
     const fetchOptions: RequestInit = { cache: 'no-cache' };
-    const manifestUrl = new URL(NpmSnapFileNames.Manifest, localhostUrl);
-    if (!LOCALHOST_HOSTNAMES.has(manifestUrl.hostname)) {
-      throw new Error(
-        `Invalid URL: Locally hosted Snaps must be hosted on localhost. Received URL: "${manifestUrl.toString()}"`,
-      );
-    }
 
+    const manifestUrl = new URL(NpmSnapFileNames.Manifest, localhostUrl);
     const manifest = await (
       await this.#fetchFunction(manifestUrl.toString(), fetchOptions)
     ).json();

packages/snaps-utils/src/snaps.ts

@@ -7,7 +7,19 @@ import { SnapManifest, SnapPermissions } from './manifest/validation';
 import { SnapId, SnapIdPrefixes, SnapValidationFailureReason } from './types';
 import { SemVerVersion } from './versions';
 
+/**
+ * Supported URL protocols for locally hosted snaps.
+ */
+export const LOCALHOST_PROTOCOLS = new Set(['http:', 'https:']);
+
+/**
+ * Supported URL hostnames for locally hosted snaps.
+ */
 export const LOCALHOST_HOSTNAMES = new Set(['localhost', '127.0.0.1', '::1']);
+
+/**
+ * The prefix of the Snap restricted RPC method / permission.
+ */
 export const SNAP_PREFIX = 'wallet_snap_';
 
 export const SNAP_PREFIX_REGEX = new RegExp(`^${SNAP_PREFIX}`, 'u');