diff --git a/packages/snaps-controllers/coverage.json b/packages/snaps-controllers/coverage.json index d5f5c7edc3..d59ba0f1c8 100644 --- a/packages/snaps-controllers/coverage.json +++ b/packages/snaps-controllers/coverage.json @@ -1,6 +1,6 @@ { - "branches": 90.09, - "functions": 96.35, - "lines": 97.31, - "statements": 96.99 + "branches": 91.03, + "functions": 96.36, + "lines": 97.69, + "statements": 97.37 } diff --git a/packages/snaps-controllers/src/snaps/location/npm.test.ts b/packages/snaps-controllers/src/snaps/location/npm.test.ts index 2151a5fb8c..364a6056ad 100644 --- a/packages/snaps-controllers/src/snaps/location/npm.test.ts +++ b/packages/snaps-controllers/src/snaps/location/npm.test.ts @@ -1,3 +1,4 @@ +import type { SemVerRange } from '@metamask/utils'; import { assert } from '@metamask/utils'; import { createReadStream } from 'fs'; import { readFile } from 'fs/promises'; @@ -5,7 +6,11 @@ import fetchMock from 'jest-fetch-mock'; import path from 'path'; import { Readable } from 'stream'; -import { NpmLocation } from './npm'; +import { + DEFAULT_NPM_REGISTRY, + NpmLocation, + getNpmCanonicalBasePath, +} from './npm'; fetchMock.enableMocks(); @@ -68,9 +73,17 @@ describe('NpmLocation', () => { ); const manifest = await location.manifest(); - const sourceCode = ( - await location.fetch(manifest.result.source.location.npm.filePath) - ).toString(); + expect(manifest.path).toBe('snap.manifest.json'); + expect(manifest.data.canonicalPath).toBe( + 'npm://registry.npmjs.cf/@metamask/template-snap/snap.manifest.json', + ); + const sourceCode = await location.fetch( + manifest.result.source.location.npm.filePath, + ); + expect(sourceCode.path).toBe('dist/bundle.js'); + expect(sourceCode.data.canonicalPath).toBe( + 'npm://registry.npmjs.cf/@metamask/template-snap/dist/bundle.js', + ); assert(manifest.result.source.location.npm.iconPath); const svgIcon = ( await location.fetch(manifest.result.source.location.npm.iconPath) @@ -94,7 +107,7 @@ describe('NpmLocation', () => { ), ); - expect(sourceCode).toStrictEqual( + expect(sourceCode.toString()).toStrictEqual( ( await readFile( require.resolve('@metamask/template-snap/dist/bundle.js'), @@ -105,6 +118,8 @@ describe('NpmLocation', () => { expect(svgIcon?.startsWith('')).toBe( true, ); + + expect(location.version).toBe(templateSnapVersion); }); it('fetches a package tarball directly without fetching the metadata when possible', async () => { @@ -170,6 +185,8 @@ describe('NpmLocation', () => { expect(svgIcon?.startsWith('')).toBe( true, ); + + expect(location.version).toBe(templateSnapVersion); }); it('falls back to zlib if DecompressionStream is unavailable', async () => { @@ -251,6 +268,86 @@ describe('NpmLocation', () => { ); }); + it('throws if fetching the NPM tarball fails', async () => { + const { version: templateSnapVersion } = JSON.parse( + ( + await readFile(require.resolve('@metamask/template-snap/package.json')) + ).toString('utf8'), + ); + + const tarballRegistry = `https://registry.npmjs.org/@metamask/template-snap/-/template-snap-${templateSnapVersion}.tgz`; + + const customFetchMock = jest.fn(); + + customFetchMock + .mockResolvedValueOnce({ + ok: true, + json: async () => ({ + // eslint-disable-next-line @typescript-eslint/naming-convention + 'dist-tags': { + latest: templateSnapVersion, + }, + versions: { + [templateSnapVersion]: { + dist: { + // return npmjs.org registry here so that we can check overriding it with npmjs.cf works + tarball: tarballRegistry, + }, + }, + }, + }), + } as any) + .mockResolvedValue({ + ok: false, + body: null, + } as any); + + const location = new NpmLocation(new URL('npm:@metamask/template-snap'), { + versionRange: templateSnapVersion, + fetch: customFetchMock as typeof fetch, + }); + + await expect(location.manifest()).rejects.toThrow( + 'Failed to fetch tarball for package "@metamask/template-snap"', + ); + }); + + it('throws if the NPM tarball URL is invalid', async () => { + const { version: templateSnapVersion } = JSON.parse( + ( + await readFile(require.resolve('@metamask/template-snap/package.json')) + ).toString('utf8'), + ); + + const customFetchMock = jest.fn(); + + customFetchMock.mockResolvedValueOnce({ + ok: true, + json: async () => ({ + // eslint-disable-next-line @typescript-eslint/naming-convention + 'dist-tags': { + latest: templateSnapVersion, + }, + versions: { + [templateSnapVersion]: { + dist: { + tarball: 'foo', + }, + }, + }, + }), + } as any); + + const location = new NpmLocation(new URL('npm:@metamask/template-snap'), { + versionRange: '*' as SemVerRange, + fetch: customFetchMock as typeof fetch, + }); + + await expect(location.manifest()).rejects.toThrow( + `Failed to find valid tarball URL in NPM metadata for package "@metamask/template-snap".`, + ); + }); + it("can't use custom registries by default", () => { expect( () => @@ -288,3 +385,20 @@ describe('NpmLocation', () => { // TODO(ritave): Requires writing tarball packing utility out of scope for a hot-fix blocking release. it.todo('paths are normalized to remove "./" prefix'); }); + +describe('getNpmCanonicalBasePath', () => { + it('returns the default base path', () => { + expect( + getNpmCanonicalBasePath(DEFAULT_NPM_REGISTRY, '@metamask/example-snap'), + ).toBe('npm://registry.npmjs.org/@metamask/example-snap/'); + }); + + it('returns a path for a custom registry', () => { + expect( + getNpmCanonicalBasePath( + new URL('https://foo:bar@registry.com/'), + '@metamask/example-snap', + ), + ).toBe('npm://foo:bar@registry.com/@metamask/example-snap/'); + }); +}); diff --git a/packages/snaps-controllers/src/snaps/location/npm.ts b/packages/snaps-controllers/src/snaps/location/npm.ts index f21ab1c94a..f637ae09df 100644 --- a/packages/snaps-controllers/src/snaps/location/npm.ts +++ b/packages/snaps-controllers/src/snaps/location/npm.ts @@ -50,12 +50,13 @@ export interface NpmOptions { allowCustomRegistries?: boolean; } -export class NpmLocation implements SnapLocation { - private readonly meta: NpmMeta; +// Base class for NPM implementation, useful for extending with custom NPM fetching logic +export abstract class BaseNpmLocation implements SnapLocation { + protected readonly meta: NpmMeta; - private validatedManifest?: VirtualFile; + #validatedManifest?: VirtualFile; - private files?: Map; + #files?: Map; constructor(url: URL, opts: DetectSnapLocationOptions = {}) { const allowCustomRegistries = opts.allowCustomRegistries ?? false; @@ -118,25 +119,25 @@ export class NpmLocation implements SnapLocation { } async manifest(): Promise> { - if (this.validatedManifest) { - return this.validatedManifest.clone(); + if (this.#validatedManifest) { + return this.#validatedManifest.clone(); } const vfile = await this.fetch('snap.manifest.json'); const result = parseJson(vfile.toString()); vfile.result = createSnapManifest(result); - this.validatedManifest = vfile as VirtualFile; + this.#validatedManifest = vfile as VirtualFile; return this.manifest(); } async fetch(path: string): Promise { const relativePath = normalizeRelative(path); - if (!this.files) { + if (!this.#files) { await this.#lazyInit(); - assert(this.files !== undefined); + assert(this.#files !== undefined); } - const vfile = this.files.get(relativePath); + const vfile = this.#files.get(relativePath); assert( vfile !== undefined, new TypeError(`File "${path}" not found in package.`), @@ -165,36 +166,82 @@ export class NpmLocation implements SnapLocation { } async #lazyInit() { - assert(this.files === undefined); + assert(this.#files === undefined); const resolvedVersion = await this.meta.resolveVersion( this.meta.requestedRange, ); - const [tarballResponse, actualVersion] = await fetchNpmTarball( + + const { tarballURL, targetVersion } = await resolveNpmVersion( this.meta.packageName, resolvedVersion, this.meta.registry, this.meta.fetch, ); - this.meta.version = actualVersion; - let canonicalBase = 'npm://'; - if (this.meta.registry.username !== '') { - canonicalBase += this.meta.registry.username; - if (this.meta.registry.password !== '') { - canonicalBase += `:${this.meta.registry.password}`; - } - canonicalBase += '@'; + if (!isValidUrl(tarballURL) || !tarballURL.toString().endsWith('.tgz')) { + throw new Error( + `Failed to find valid tarball URL in NPM metadata for package "${this.meta.packageName}".`, + ); + } + + // Override the tarball hostname/protocol with registryUrl hostname/protocol + const newTarballUrl = new URL(tarballURL); + newTarballUrl.hostname = this.meta.registry.hostname; + newTarballUrl.protocol = this.meta.registry.protocol; + + const files = await this.fetchNpmTarball(newTarballUrl); + + this.#files = files; + this.meta.version = targetVersion; + } + + /** + * Fetches and unpacks the tarball (`.tgz` file) from the specified URL. + * + * @param tarballUrl - The tarball URL to fetch and unpack. + * @returns A the files for the package tarball. + * @throws If fetching the tarball fails. + */ + abstract fetchNpmTarball(tarballUrl: URL): Promise>; +} + +// Safety limit for tarballs, 250 MB in bytes +export const TARBALL_SIZE_SAFETY_LIMIT = 262144000; + +// Main NPM implementation, contains a browser tarball fetching implementation. +export class NpmLocation extends BaseNpmLocation { + /** + * Fetches and unpacks the tarball (`.tgz` file) from the specified URL. + * + * @param tarballUrl - The tarball URL to fetch and unpack. + * @returns A the files for the package tarball. + * @throws If fetching the tarball fails. + */ + async fetchNpmTarball( + tarballUrl: URL, + ): Promise>> { + // Perform a raw fetch because we want the Response object itself. + const tarballResponse = await this.meta.fetch(tarballUrl.toString()); + if (!tarballResponse.ok || !tarballResponse.body) { + throw new Error( + `Failed to fetch tarball for package "${this.meta.packageName}".`, + ); } - canonicalBase += this.meta.registry.host; - // TODO(ritave): Lazily extract files instead of up-front extracting all of them - // We would need to replace tar-stream package because it requires immediate consumption of streams. - await new Promise((resolve, reject) => { - this.files = new Map(); + // We assume that NPM is a good actor and provides us with a valid `content-length` header. + const tarballSizeString = tarballResponse.headers.get('content-length'); + assert(tarballSizeString, 'Snap tarball has invalid content-length'); + const tarballSize = parseInt(tarballSizeString, 10); + assert( + tarballSize <= TARBALL_SIZE_SAFETY_LIMIT, + 'Snap tarball exceeds size limit', + ); + return new Promise((resolve, reject) => { + const files = new Map(); const tarballStream = createTarballStream( - `${canonicalBase}/${this.meta.packageName}/`, - this.files, + getNpmCanonicalBasePath(this.meta.registry, this.meta.packageName), + files, ); // The "gz" in "tgz" stands for "gzip". The tarball needs to be decompressed @@ -205,33 +252,32 @@ export class NpmLocation implements SnapLocation { if ('DecompressionStream' in globalThis) { const decompressionStream = new DecompressionStream('gzip'); const decompressedStream = - tarballResponse.pipeThrough(decompressionStream); + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + tarballResponse.body!.pipeThrough(decompressionStream); pipeline( getNodeStream(decompressedStream), tarballStream, (error: unknown) => { - error ? reject(error) : resolve(); + error ? reject(error) : resolve(files); }, ); return; } pipeline( - getNodeStream(tarballResponse), + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + getNodeStream(tarballResponse.body!), createGunzip(), tarballStream, (error: unknown) => { - error ? reject(error) : resolve(); + error ? reject(error) : resolve(files); }, ); }); } } -// Safety limit for tarballs, 250 MB in bytes -const TARBALL_SIZE_SAFETY_LIMIT = 262144000; - // Incomplete type export type PartialNpmMetadata = { versions: Record; @@ -280,6 +326,25 @@ export async function fetchNpmMetadata( return packageMetadata as PartialNpmMetadata; } +/** + * Gets the canonical base path for an NPM snap. + * + * @param registryUrl - A registry URL. + * @param packageName - A package name. + * @returns The canonical base path. + */ +export function getNpmCanonicalBasePath(registryUrl: URL, packageName: string) { + let canonicalBase = 'npm://'; + if (registryUrl.username !== '') { + canonicalBase += registryUrl.username; + if (registryUrl.password !== '') { + canonicalBase += `:${registryUrl.password}`; + } + canonicalBase += '@'; + } + return `${canonicalBase}${registryUrl.host}/${packageName}/`; +} + /** * Determine if a registry URL is NPM. * @@ -343,61 +408,6 @@ async function resolveNpmVersion( return { tarballURL, targetVersion }; } -/** - * Fetches the tarball (`.tgz` file) of the specified package and version from - * the public npm registry. - * - * @param packageName - The name of the package whose tarball to fetch. - * @param versionRange - The SemVer range of the package to fetch. The highest - * version satisfying the range will be fetched. - * @param registryUrl - The URL of the npm registry to fetch the tarball from. - * @param fetchFunction - The fetch function to use. Defaults to the global - * {@link fetch}. Useful for Node.js compatibility. - * @returns A tuple of the {@link Response} for the package tarball and the - * actual version of the package. - * @throws If fetching the tarball fails. - */ -async function fetchNpmTarball( - packageName: string, - versionRange: SemVerRange, - registryUrl: URL, - fetchFunction: typeof fetch, -): Promise<[ReadableStream, SemVerVersion]> { - const { tarballURL, targetVersion } = await resolveNpmVersion( - packageName, - versionRange, - registryUrl, - fetchFunction, - ); - - if (!isValidUrl(tarballURL) || !tarballURL.toString().endsWith('.tgz')) { - throw new Error( - `Failed to find valid tarball URL in NPM metadata for package "${packageName}".`, - ); - } - - // Override the tarball hostname/protocol with registryUrl hostname/protocol - const newRegistryUrl = new URL(registryUrl); - const newTarballUrl = new URL(tarballURL); - newTarballUrl.hostname = newRegistryUrl.hostname; - newTarballUrl.protocol = newRegistryUrl.protocol; - - // Perform a raw fetch because we want the Response object itself. - const tarballResponse = await fetchFunction(newTarballUrl.toString()); - if (!tarballResponse.ok || !tarballResponse.body) { - throw new Error(`Failed to fetch tarball for package "${packageName}".`); - } - // We assume that NPM is a good actor and provides us with a valid `content-length` header. - const tarballSizeString = tarballResponse.headers.get('content-length'); - assert(tarballSizeString, 'Snap tarball has invalid content-length'); - const tarballSize = parseInt(tarballSizeString, 10); - assert( - tarballSize <= TARBALL_SIZE_SAFETY_LIMIT, - 'Snap tarball exceeds size limit', - ); - return [tarballResponse.body, targetVersion]; -} - /** * The paths of files within npm tarballs appear to always be prefixed with * "package/".