diff --git a/packages/snaps-controllers/src/snaps/SnapController.test.tsx b/packages/snaps-controllers/src/snaps/SnapController.test.tsx index 233b0054d8..4dd68e957d 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.test.tsx +++ b/packages/snaps-controllers/src/snaps/SnapController.test.tsx @@ -6597,7 +6597,7 @@ describe('SnapController', () => { [snapId]: {}, }), ).rejects.toThrow( - `Invalid snap ID: Expected the value to satisfy a union of \`intersection | string\`, but received: "foo".`, + `Invalid snap ID: Invalid or no prefix found. Expected Snap ID to start with one of: "npm:", "local:", but received: "foo".`, ); controller.destroy(); diff --git a/packages/snaps-utils/coverage.json b/packages/snaps-utils/coverage.json index 2497636a15..b2aafe3a34 100644 --- a/packages/snaps-utils/coverage.json +++ b/packages/snaps-utils/coverage.json @@ -1,6 +1,6 @@ { "branches": 99.74, - "functions": 98.92, + "functions": 98.93, "lines": 99.61, - "statements": 96.91 + "statements": 96.94 } diff --git a/packages/snaps-utils/src/snaps.test.ts b/packages/snaps-utils/src/snaps.test.ts index dd9fbddaa6..37a5ab5518 100644 --- a/packages/snaps-utils/src/snaps.test.ts +++ b/packages/snaps-utils/src/snaps.test.ts @@ -14,6 +14,7 @@ import { verifyRequestedSnapPermissions, stripSnapPrefix, isSnapId, + SnapIdPrefixStruct, } from './snaps'; import { MOCK_SNAP_ID } from './test-utils'; import { uri, WALLET_SNAP_PERMISSION_KEY } from './types'; @@ -52,14 +53,14 @@ describe('assertIsValidSnapId', () => { // TODO: Either fix this lint violation or explain why it's necessary to // ignore. // eslint-disable-next-line @typescript-eslint/restrict-template-expressions, @typescript-eslint/no-base-to-string - `Invalid snap ID: Expected the value to satisfy a union of \`intersection | string\`, but received: ${value}.`, + `Invalid snap ID: Expected a string, but received: ${value}.`, ); }, ); it('throws for invalid snap id', () => { expect(() => assertIsValidSnapId('foo:bar')).toThrow( - `Invalid snap ID: Expected the value to satisfy a union of \`intersection | string\`, but received: "foo:bar".`, + `Invalid snap ID: Invalid or no prefix found. Expected Snap ID to start with one of: "npm:", "local:", but received: "foo:bar".`, ); }); @@ -75,14 +76,19 @@ describe('assertIsValidSnapId', () => { ).not.toThrow(); }); + it('disallows whitespace at the beginning', () => { + expect(() => assertIsValidSnapId(' local:http://localhost:8000')).toThrow( + 'Invalid snap ID: Invalid or no prefix found. Expected Snap ID to start with one of: "npm:", "local:", but received: " local:http://localhost:8000".', + ); + }); + it.each([ - ' local:http://localhost:8000', 'local:http://localhost:8000 ', 'local:http://localhost:8000\n', 'local:http://localhost:8000\r', ])('disallows whitespace #%#', (value) => { expect(() => assertIsValidSnapId(value)).toThrow( - /Invalid snap ID: Expected the value to satisfy a union of `intersection \| string`, but received: .+\./u, + /Invalid snap ID: Expected a value of type `Base Snap Id`, but received: .+\./u, ); }); @@ -90,7 +96,7 @@ describe('assertIsValidSnapId', () => { 'disallows non-ASCII symbols #%#', (value) => { expect(() => assertIsValidSnapId(value)).toThrow( - `Invalid snap ID: Expected the value to satisfy a union of \`intersection | string\`, but received: "${value}".`, + `Invalid snap ID: Expected a value of type \`Base Snap Id\`, but received: \`"${value}"\`.`, ); }, ); @@ -238,6 +244,36 @@ describe('HttpSnapIdStruct', () => { }); }); +describe('SnapIdPrefixStruct', () => { + it.each(['local:', 'npm:', 'local:foobar', 'npm:foobar'])( + 'validates "%s" as proper Snap ID prefix', + (value) => { + expect(is(value, SnapIdPrefixStruct)).toBe(true); + }, + ); + + it.each([ + 0, + 1, + false, + true, + {}, + [], + uri, + URL, + new URL('http://github.com'), + '', + 'local', + 'npm', + 'foo:npm', + 'foo:local', + 'localfoobar', + 'npmfoobar', + ])('invalidates an improper Snap ID prefix', (value) => { + expect(is(value, SnapIdPrefixStruct)).toBe(false); + }); +}); + describe('isSnapPermitted', () => { it("will check an origin's permissions object to see if it has permission to interact with a specific snap", () => { const validPermissions: SubjectPermissions = { diff --git a/packages/snaps-utils/src/snaps.ts b/packages/snaps-utils/src/snaps.ts index 9fefd8e011..10c3b4d63f 100644 --- a/packages/snaps-utils/src/snaps.ts +++ b/packages/snaps-utils/src/snaps.ts @@ -4,7 +4,11 @@ import type { PermissionConstraint, } from '@metamask/permission-controller'; import type { BlockReason } from '@metamask/snaps-registry'; -import type { SnapId, Snap as TruncatedSnap } from '@metamask/snaps-sdk'; +import { + selectiveUnion, + type SnapId, + type Snap as TruncatedSnap, +} from '@metamask/snaps-sdk'; import type { Struct } from '@metamask/superstruct'; import { is, @@ -12,14 +16,12 @@ import { enums, intersection, literal, - pattern, refine, string, - union, validate, } from '@metamask/superstruct'; import type { Json } from '@metamask/utils'; -import { assert, isObject, assertStruct } from '@metamask/utils'; +import { assert, isObject, assertStruct, definePattern } from '@metamask/utils'; import { base64 } from '@scure/base'; import stableStringify from 'fast-json-stable-stringify'; import validateNPMPackage from 'validate-npm-package-name'; @@ -228,7 +230,10 @@ export async function validateSnapShasum( export const LOCALHOST_HOSTNAMES = ['localhost', '127.0.0.1', '[::1]'] as const; // Require snap ids to only consist of printable ASCII characters -export const BaseSnapIdStruct = pattern(string(), /^[\x21-\x7E]*$/u); +export const BaseSnapIdStruct = definePattern( + 'Base Snap Id', + /^[\x21-\x7E]*$/u, +); const LocalSnapIdSubUrlStruct = uri({ protocol: enums(['http:', 'https:']), @@ -284,7 +289,35 @@ export const HttpSnapIdStruct = intersection([ }), ]) as unknown as Struct; -export const SnapIdStruct = union([NpmSnapIdStruct, LocalSnapIdStruct]); +export const SnapIdPrefixStruct = refine( + string(), + 'Snap ID prefix', + (value) => { + if ( + Object.values(SnapIdPrefixes).some((prefix) => value.startsWith(prefix)) + ) { + return true; + } + + const allowedPrefixes = Object.values(SnapIdPrefixes) + .map((prefix) => `"${prefix}"`) + .join(', '); + + return `Invalid or no prefix found. Expected Snap ID to start with one of: ${allowedPrefixes}, but received: "${value}"`; + }, +); + +export const SnapIdStruct = selectiveUnion((value) => { + if (typeof value === 'string' && value.startsWith(SnapIdPrefixes.npm)) { + return NpmSnapIdStruct; + } + + if (typeof value === 'string' && value.startsWith(SnapIdPrefixes.local)) { + return LocalSnapIdStruct; + } + + return SnapIdPrefixStruct; +}); /** * Extracts the snap prefix from a snap ID. diff --git a/packages/snaps-utils/src/types.ts b/packages/snaps-utils/src/types.ts index abbba8b2e6..0658225143 100644 --- a/packages/snaps-utils/src/types.ts +++ b/packages/snaps-utils/src/types.ts @@ -2,7 +2,6 @@ import { instance, is, optional, - pattern, refine, size, string, @@ -12,7 +11,7 @@ import { } from '@metamask/superstruct'; import type { Infer, Struct } from '@metamask/superstruct'; import type { Json } from '@metamask/utils'; -import { VersionStruct } from '@metamask/utils'; +import { definePattern, VersionStruct } from '@metamask/utils'; import type { SnapCaveatType } from './caveats'; import type { SnapFunctionExports, SnapRpcHookArgs } from './handlers'; @@ -26,8 +25,8 @@ export enum NpmSnapFileNames { } export const NameStruct = size( - pattern( - string(), + definePattern( + 'Snap Name', /^(?:@[a-z0-9-*~][a-z0-9-*._~]*\/)?[a-z0-9-~][a-z0-9-._~]*$/u, ), 1,