From 5f18720ffc316c135ac00093a33c81716c6c3c9b Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Wed, 20 Mar 2024 14:33:37 +0100 Subject: [PATCH] Remove broken `ethereum` properties (#2296) Removes access to `on` and `removeListener` on the `ethereum` global since these are broken in Snaps anyway. --- .../coverage.json | 8 +++---- .../common/BaseSnapExecutor.test.browser.ts | 7 ++----- .../src/common/BaseSnapExecutor.ts | 20 ++---------------- .../src/common/utils.ts | 21 ++++++------------- packages/snaps-sdk/src/types/provider.test.ts | 4 ++-- packages/snaps-sdk/src/types/provider.ts | 2 +- 6 files changed, 17 insertions(+), 45 deletions(-) diff --git a/packages/snaps-execution-environments/coverage.json b/packages/snaps-execution-environments/coverage.json index 3df0bef368..bb7c7347df 100644 --- a/packages/snaps-execution-environments/coverage.json +++ b/packages/snaps-execution-environments/coverage.json @@ -1,6 +1,6 @@ { - "branches": 80.66, - "functions": 90.19, - "lines": 90.83, - "statements": 90.2 + "branches": 80, + "functions": 90.06, + "lines": 90.75, + "statements": 90.12 } diff --git a/packages/snaps-execution-environments/src/common/BaseSnapExecutor.test.browser.ts b/packages/snaps-execution-environments/src/common/BaseSnapExecutor.test.browser.ts index e51710d6c3..7fdb5f8363 100644 --- a/packages/snaps-execution-environments/src/common/BaseSnapExecutor.test.browser.ts +++ b/packages/snaps-execution-environments/src/common/BaseSnapExecutor.test.browser.ts @@ -390,9 +390,6 @@ describe('BaseSnapExecutor', () => { it('allows direct access to ethereum public properties', async () => { const CODE = ` module.exports.onRpcRequest = () => { - const listener = () => undefined; - ethereum.on('accountsChanged', listener); - ethereum.removeListener('accountsChanged', listener); return ethereum.request({ method: 'eth_blockNumber', params: [] }); }; `; @@ -2415,8 +2412,8 @@ describe('BaseSnapExecutor', () => { hasMethods: { ethereum: { request: true, - on: true, - removeListener: true, + on: false, + removeListener: false, rpcEngine: false, }, snap: { diff --git a/packages/snaps-execution-environments/src/common/BaseSnapExecutor.ts b/packages/snaps-execution-environments/src/common/BaseSnapExecutor.ts index e8a681e1d9..0d4af5db28 100644 --- a/packages/snaps-execution-environments/src/common/BaseSnapExecutor.ts +++ b/packages/snaps-execution-environments/src/common/BaseSnapExecutor.ts @@ -494,23 +494,7 @@ export class BaseSnapExecutor { ); }; - // Proxy target is intentionally set to be an empty object, to ensure - // that access to the prototype chain is not possible. - const snapGlobalProxy = new Proxy( - {}, - { - has(_target: object, prop: string | symbol) { - return typeof prop === 'string' && ['request'].includes(prop); - }, - get(_target, prop: keyof StreamProvider) { - if (prop === 'request') { - return request; - } - - return undefined; - }, - }, - ) as SnapsProvider; + const snapGlobalProxy = proxyStreamProvider(request) as SnapsProvider; return harden(snapGlobalProxy); } @@ -546,7 +530,7 @@ export class BaseSnapExecutor { ); }; - const streamProviderProxy = proxyStreamProvider(provider, request); + const streamProviderProxy = proxyStreamProvider(request); return harden(streamProviderProxy); } diff --git a/packages/snaps-execution-environments/src/common/utils.ts b/packages/snaps-execution-environments/src/common/utils.ts index 2db7693c10..9cdf688897 100644 --- a/packages/snaps-execution-environments/src/common/utils.ts +++ b/packages/snaps-execution-environments/src/common/utils.ts @@ -54,33 +54,24 @@ export async function withTeardown( } /** - * Returns a Proxy that narrows down (attenuates) the fields available on - * the StreamProvider and replaces the request implementation. + * Returns a Proxy that only allows access to a `request` function. + * This is useful for replacing StreamProvider with an attenuated version. * - * @param provider - Instance of a StreamProvider to be limited. - * @param request - Custom attenuated request object. - * @returns Proxy to the StreamProvider instance. + * @param request - Custom attenuated request function. + * @returns Proxy that mimics a StreamProvider instance. */ -export function proxyStreamProvider( - provider: StreamProvider, - request: unknown, -): StreamProvider { +export function proxyStreamProvider(request: unknown): StreamProvider { // Proxy target is intentionally set to be an empty object, to ensure // that access to the prototype chain is not possible. const proxy = new Proxy( {}, { has(_target: object, prop: string | symbol) { - return ( - typeof prop === 'string' && - ['request', 'on', 'removeListener'].includes(prop) - ); + return typeof prop === 'string' && ['request'].includes(prop); }, get(_target, prop: keyof StreamProvider) { if (prop === 'request') { return request; - } else if (['on', 'removeListener'].includes(prop)) { - return provider[prop]; } return undefined; diff --git a/packages/snaps-sdk/src/types/provider.test.ts b/packages/snaps-sdk/src/types/provider.test.ts index 9eba420a2f..fa2c14f107 100644 --- a/packages/snaps-sdk/src/types/provider.test.ts +++ b/packages/snaps-sdk/src/types/provider.test.ts @@ -5,8 +5,8 @@ import type { SnapsEthereumProvider, SnapsProvider } from './provider'; describe('SnapsEthereumProvider', () => { it('only has the expected methods', () => { expectTypeOf().toHaveProperty('request'); - expectTypeOf().toHaveProperty('on'); - expectTypeOf().toHaveProperty('removeListener'); + expectTypeOf().not.toHaveProperty('on'); + expectTypeOf().not.toHaveProperty('removeListener'); expectTypeOf().not.toHaveProperty('isConnected'); }); }); diff --git a/packages/snaps-sdk/src/types/provider.ts b/packages/snaps-sdk/src/types/provider.ts index 2739746209..8aa6f1babd 100644 --- a/packages/snaps-sdk/src/types/provider.ts +++ b/packages/snaps-sdk/src/types/provider.ts @@ -11,7 +11,7 @@ export type SnapsEthereumProvider = Pick< BaseProviderInstance, // Snaps have access to a limited set of methods. This is enforced by the // `proxyStreamProvider` function in `@metamask/snaps-execution-environments`. - 'request' | 'on' | 'removeListener' + 'request' >; /**