Skip to content

Commit

Permalink
chore: Remove pattern in favour of definePattern (#3142)
Browse files Browse the repository at this point in the history
`@metamask/utils` introduced a new `definePattern` superstruct helper to
have better error messages for `pattern`. This updates the repo to use
it.

Fixes: #3066

---------

Co-authored-by: Maarten Zuidhoorn <[email protected]>
  • Loading branch information
GuillaumeRx and Mrtenz authored Feb 21, 2025
1 parent f5e359f commit 70a83bd
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions packages/snaps-utils/coverage.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"branches": 99.74,
"functions": 98.92,
"functions": 98.93,
"lines": 99.61,
"statements": 96.91
"statements": 96.94
}
46 changes: 41 additions & 5 deletions packages/snaps-utils/src/snaps.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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".`,
);
});

Expand All @@ -75,22 +76,27 @@ 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,
);
});

it.each(['local:😎', 'local:␡'])(
'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}"\`.`,
);
},
);
Expand Down Expand Up @@ -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<PermissionConstraint> = {
Expand Down
45 changes: 39 additions & 6 deletions packages/snaps-utils/src/snaps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,24 @@ 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,
empty,
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';
Expand Down Expand Up @@ -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:']),
Expand Down Expand Up @@ -284,7 +289,35 @@ export const HttpSnapIdStruct = intersection([
}),
]) as unknown as Struct<string, null>;

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.
Expand Down
7 changes: 3 additions & 4 deletions packages/snaps-utils/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import {
instance,
is,
optional,
pattern,
refine,
size,
string,
Expand All @@ -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';
Expand All @@ -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,
Expand Down

0 comments on commit 70a83bd

Please sign in to comment.