From 10d00d22e906de19707da382cbc6ce3508816bbf Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Thu, 8 Dec 2022 11:16:54 +0100 Subject: [PATCH] Coerce snap manifests to remove prefixes --- packages/snaps-controllers/jest.config.js | 2 +- .../src/snaps/location/http.test.ts | 4 +-- .../src/snaps/location/http.ts | 7 ++-- .../src/snaps/location/local.test.ts | 4 +-- .../src/snaps/location/npm.ts | 7 ++-- .../src/test-utils/location.ts | 21 ++++++++--- .../src/manifest/validation.test.ts | 36 +++++++++++++++++++ .../snaps-utils/src/manifest/validation.ts | 14 ++++++++ 8 files changed, 77 insertions(+), 18 deletions(-) diff --git a/packages/snaps-controllers/jest.config.js b/packages/snaps-controllers/jest.config.js index 8d1cf377b5..2a2382fe1c 100644 --- a/packages/snaps-controllers/jest.config.js +++ b/packages/snaps-controllers/jest.config.js @@ -5,7 +5,7 @@ const baseConfig = require('../../jest.config.base'); module.exports = deepmerge(baseConfig, { coverageThreshold: { global: { - branches: 90.81, + branches: 91.08, functions: 97.45, lines: 97.45, statements: 97.45, diff --git a/packages/snaps-controllers/src/snaps/location/http.test.ts b/packages/snaps-controllers/src/snaps/location/http.test.ts index 7617af519a..b7a0001d11 100644 --- a/packages/snaps-controllers/src/snaps/location/http.test.ts +++ b/packages/snaps-controllers/src/snaps/location/http.test.ts @@ -62,8 +62,8 @@ describe('HttpLocation', () => { const manifest = await location.manifest(); const bundle = await location.fetch('./foo.js'); - expect(manifest.path).toBe('./snap.manifest.json'); - expect(bundle.path).toBe('./foo.js'); + expect(manifest.path).toBe('snap.manifest.json'); + expect(bundle.path).toBe('foo.js'); expect(manifest.data.canonicalPath).toBe(canonical.manifest); expect(bundle.data.canonicalPath).toBe(canonical.bundle); }); diff --git a/packages/snaps-controllers/src/snaps/location/http.ts b/packages/snaps-controllers/src/snaps/location/http.ts index 012a97a903..c7a6bfc664 100644 --- a/packages/snaps-controllers/src/snaps/location/http.ts +++ b/packages/snaps-controllers/src/snaps/location/http.ts @@ -1,9 +1,9 @@ import { SnapManifest, - assertIsSnapManifest, VirtualFile, HttpSnapIdStruct, NpmSnapFileNames, + createSnapManifest, } from '@metamask/snaps-utils'; import { assert, assertStruct } from '@metamask/utils'; @@ -60,11 +60,10 @@ export class HttpLocation implements SnapLocation { await this.fetchFn(canonicalPath, this.fetchOptions) ).text(); const manifest = JSON.parse(contents); - assertIsSnapManifest(manifest); const vfile = new VirtualFile({ value: contents, - result: manifest, - path: `./${NpmSnapFileNames.Manifest}`, + result: createSnapManifest(manifest), + path: NpmSnapFileNames.Manifest, data: { canonicalPath }, }); this.validatedManifest = vfile; diff --git a/packages/snaps-controllers/src/snaps/location/local.test.ts b/packages/snaps-controllers/src/snaps/location/local.test.ts index 65afe338f5..04626ca681 100644 --- a/packages/snaps-controllers/src/snaps/location/local.test.ts +++ b/packages/snaps-controllers/src/snaps/location/local.test.ts @@ -65,8 +65,8 @@ describe('LocalLocation', () => { const manifest = await location.manifest(); const bundle = await location.fetch('./foo.js'); - expect(manifest.path).toBe('./snap.manifest.json'); - expect(bundle.path).toBe('./foo.js'); + expect(manifest.path).toBe('snap.manifest.json'); + expect(bundle.path).toBe('foo.js'); expect(manifest.data.canonicalPath).toBe(canonical.manifest); expect(bundle.data.canonicalPath).toBe(canonical.bundle); }); diff --git a/packages/snaps-controllers/src/snaps/location/npm.ts b/packages/snaps-controllers/src/snaps/location/npm.ts index 76a9daa07c..42029206f9 100644 --- a/packages/snaps-controllers/src/snaps/location/npm.ts +++ b/packages/snaps-controllers/src/snaps/location/npm.ts @@ -1,6 +1,6 @@ import { assertIsSemVerVersion, - assertIsSnapManifest, + createSnapManifest, DEFAULT_REQUESTED_SNAP_VERSION, getTargetVersion, isValidUrl, @@ -112,10 +112,9 @@ export class NpmLocation implements SnapLocation { return this.validatedManifest.clone(); } - const vfile = await this.fetch('./snap.manifest.json'); + const vfile = await this.fetch('snap.manifest.json'); const result = JSON.parse(vfile.toString()); - assertIsSnapManifest(result); - vfile.result = result; + vfile.result = createSnapManifest(result); this.validatedManifest = vfile as VirtualFile; return this.manifest(); diff --git a/packages/snaps-controllers/src/test-utils/location.ts b/packages/snaps-controllers/src/test-utils/location.ts index 1bb06bc11d..c53c35858f 100644 --- a/packages/snaps-controllers/src/test-utils/location.ts +++ b/packages/snaps-controllers/src/test-utils/location.ts @@ -1,4 +1,8 @@ -import { VirtualFile, SnapManifest } from '@metamask/snaps-utils'; +import { + VirtualFile, + SnapManifest, + createSnapManifest, +} from '@metamask/snaps-utils'; import { DEFAULT_SNAP_BUNDLE, DEFAULT_SNAP_ICON, @@ -7,7 +11,7 @@ import { import { assert } from '@metamask/utils'; import { SnapLocation } from 'src/snaps/location'; -const MANIFEST_PATH = './snap.manifest.json'; +const MANIFEST_PATH = 'snap.manifest.json'; type LoopbackOptions = { /** @@ -24,6 +28,12 @@ type LoopbackOptions = { shouldAlwaysReload?: boolean; }; +// Mirror NPM and HTTP implementation +const coerceManifest = (manifest: VirtualFile) => { + manifest.result = createSnapManifest(manifest.result); + return manifest; +}; + export class LoopbackLocation implements SnapLocation { #manifest: VirtualFile; @@ -33,14 +43,15 @@ export class LoopbackLocation implements SnapLocation { constructor(opts: LoopbackOptions = {}) { const shouldAlwaysReload = opts.shouldAlwaysReload ?? false; - const manifest = + const manifest = coerceManifest( opts.manifest instanceof VirtualFile ? opts.manifest : new VirtualFile({ value: '', result: opts.manifest ?? getSnapManifest(), - path: './snap.manifest.json', - }); + path: 'snap.manifest.json', + }), + ); let files; if (opts.files === undefined) { files = [ diff --git a/packages/snaps-utils/src/manifest/validation.test.ts b/packages/snaps-utils/src/manifest/validation.test.ts index e75ed136ec..41ccb2e0e9 100644 --- a/packages/snaps-utils/src/manifest/validation.test.ts +++ b/packages/snaps-utils/src/manifest/validation.test.ts @@ -7,6 +7,7 @@ import { Base64Opts, Bip32EntropyStruct, Bip32PathStruct, + createSnapManifest, isSnapManifest, } from './validation'; @@ -228,3 +229,38 @@ describe('assertIsSnapManifest', () => { ); }); }); + +describe('createSnapManifest', () => { + it('does not throw for a valid snap manifest', () => { + expect(() => createSnapManifest(getSnapManifest())).not.toThrow(); + }); + + it('coerces source paths', () => { + expect( + createSnapManifest( + getSnapManifest({ filePath: './bundle.js', iconPath: './icon.svg' }), + ), + ).toStrictEqual( + getSnapManifest({ filePath: 'bundle.js', iconPath: 'icon.svg' }), + ); + }); + + it.each([ + true, + false, + null, + undefined, + 0, + 1, + '', + 'foo', + [], + {}, + { name: 'foo' }, + { version: '1.0.0' }, + getSnapManifest({ version: 'foo bar' }), + ])('throws for an invalid snap manifest', (value) => { + // eslint-disable-next-line jest/require-to-throw-message + expect(() => createSnapManifest(value)).toThrow(); + }); +}); diff --git a/packages/snaps-utils/src/manifest/validation.ts b/packages/snaps-utils/src/manifest/validation.ts index 148c27af41..ac0e8b62a0 100644 --- a/packages/snaps-utils/src/manifest/validation.ts +++ b/packages/snaps-utils/src/manifest/validation.ts @@ -3,6 +3,7 @@ import { array, boolean, coerce, + create, enums, Infer, integer, @@ -248,3 +249,16 @@ export function assertIsSnapManifest( `"${NpmSnapFileNames.Manifest}" is invalid`, ); } + +/** + * Creates a {@link SnapManifest} object from JSON. + * + * + * @param value - The value to check. + * @throws If the value cannot be coerced to a {@link SnapManifest} object. + * @returns The created {@link SnapManifest} object. + */ +export function createSnapManifest(value: unknown): SnapManifest { + // TODO: Add a utility to prefix these errors similar to assertStruct + return create(value, SnapManifestStruct); +}