fix(deps): Bump @metamask/eth-json-rpc-middleware to ^14.0.0 (#26143

) Updates `@metamask/eth-json-rpc-middleware` from `^12.1.1` to `^14.0.0`. - This version bump comes with a large number of regressions, most of them type errors. - This is because the package's dependencies are also updated by multiple major versions, and the changes include improved, stricter types (especially in `@metamask/utils`). [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/26143?quickstart=1) - Closes #26287 - Blocks: - MetaMask/MetaMask-planning#2991 - MetaMask/MetaMask-planning#2810 - #25733 - Add and export `PPOMMiddlewareRequest` type for `JsonRpcRequest` types that include the `securityAlertResponse` property. - `securityAlertResponse` is defined as both optional and nullable. - Add `PPOMRequest` type for `eth-sendTransaction` requests. - **BREAKING:** Bump `@metamask/eth-json-rpc-middleware` from `^12.1.1` to `^14.0.0`. - Bump `@trezor/connect-web` from `9.2.2` to `9.3.0`. - **BREAKING:** Narrow `Params` generic parameter of `createPPOMMiddleware` function from `JsonRpcParams` to `(string | { to: string })[]`. - Add `Params` generic parameter to `handleSnapRequest` function, which is constrained by `Record<string, unknown>` and defaults to `JsonRpcParams`. - `handleSnapRequest` can now be typed correctly with any `params` object. - **BREAKING:** Typed signature validation only replaces `0X` prefix with `0x`, and contract address normalization is removed for decimal and octal values. - Threat actors have been manipulating `eth_signTypedData_v4` fields to cause failures in blockaid's detectors. - Extension crashes with an error when performing Malicious permit with a non-0x prefixed integer address. - This fixes an issue where the key value row or petname component disappears if a signed address is prefixed by "0X" instead of "0x". - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: MetaMask Bot <metamaskbot@users.noreply.github.com>
MetaMask · Aug 23, 2024 · 2cf39f1 · 2cf39f1
1 parent e5f4f32
commit 2cf39f1
Show file tree

Hide file tree

Showing 17 changed files with 1,618 additions and 1,074 deletions.
diff --git a/.yarn/patches/@trezor-connect-web-npm-9.3.0-040ab10d9a.patch b/.yarn/patches/@trezor-connect-web-npm-9.3.0-040ab10d9a.patch
@@ -0,0 +1,33 @@
+diff --git a/lib/impl/core-in-iframe.js b/lib/impl/core-in-iframe.js
+index c47cf3bff860d6b1855341c00b80fc6c40f9d6d5..275eb0f312ff396819fa406c154a3562842db49d 100644
+--- a/lib/impl/core-in-iframe.js
++++ b/lib/impl/core-in-iframe.js
+@@ -116,7 +116,9 @@ class CoreInIframe {
+         this._log.enabled = !!this._settings.debug;
+         window.addEventListener('message', this.boundHandleMessage);
+         window.addEventListener('unload', this.boundDispose);
+-        await iframe.init(this._settings);
++        const modifiedSettings = Object.assign({}, this.settings);
++        modifiedSettings.env = 'webextension';
++        await iframe.init(modifiedSettings);
+         if (this._settings.sharedLogger !== false) {
+             iframe.initIframeLogger();
+         }
+diff --git a/lib/popup/index.js b/lib/popup/index.js
+index 9b13c370a5ac8b4e4fc0315ed40cdf615d0bb0cb..4dbd97fc28df49beb73379451974ec48a8a42ea7 100644
+--- a/lib/popup/index.js
++++ b/lib/popup/index.js
+@@ -229,10 +229,12 @@ class PopupManager extends events_1.default {
+         }
+         else if (message.type === events_2.POPUP.LOADED) {
+             this.handleMessage(message);
++            const modifiedSettings = Object.assign({}, this.settings);
++            modifiedSettings.env = 'webextension';
+             this.channel.postMessage({
+                 type: events_2.POPUP.INIT,
+                 payload: {
+-                    settings: this.settings,
++                    settings: modifiedSettings,
+                     useCore: true,
+                 },
+             });
diff --git a/app/scripts/lib/createDupeReqFilterStream.test.ts b/app/scripts/lib/createDupeReqFilterStream.test.ts
@@ -3,7 +3,7 @@ import OurReadableStream from 'readable-stream';
 import ReadableStream2 from 'readable-stream-2';
 import ReadableStream3 from 'readable-stream-3';
 
-import type { JsonRpcRequest } from '@metamask/utils';
+import type { JsonRpcNotification, JsonRpcRequest } from '@metamask/utils';
 import createDupeReqFilterStream, {
   THREE_MINUTES,
 } from './createDupeReqFilterStream';
@@ -26,7 +26,7 @@ function createTestStream(output: JsonRpcRequest[] = [], S = Transform) {
 }
 
 function runStreamTest(
-  requests: JsonRpcRequest[] = [],
+  requests: (JsonRpcRequest | JsonRpcNotification)[] = [],
   advanceTimersTime = 10,
   S = Transform,
 ) {
@@ -54,12 +54,12 @@ describe('createDupeReqFilterStream', () => {
     const requests = [
       { id: 1, method: 'foo' },
       { id: 2, method: 'bar' },
-    ];
+    ].map((request) => ({ ...request, jsonrpc: '2.0' as const }));
 
     const expectedOutput = [
       { id: 1, method: 'foo' },
       { id: 2, method: 'bar' },
-    ];
+    ].map((output) => ({ ...output, jsonrpc: '2.0' }));
 
     const output = await runStreamTest(requests);
     expect(output).toEqual(expectedOutput);
@@ -69,18 +69,25 @@ describe('createDupeReqFilterStream', () => {
     const requests = [
       { id: 1, method: 'foo' },
       { id: 1, method: 'foo' }, // duplicate
-    ];
+    ].map((request) => ({ ...request, jsonrpc: '2.0' as const }));
 
-    const expectedOutput = [{ id: 1, method: 'foo' }];
+    const expectedOutput = [{ id: 1, method: 'foo' }].map((output) => ({
+      ...output,
+      jsonrpc: '2.0',
+    }));
 
     const output = await runStreamTest(requests);
     expect(output).toEqual(expectedOutput);
   });
 
   it("lets through requests if they don't have an id", async () => {
-    const requests = [{ method: 'notify1' }, { method: 'notify2' }];
+    const requests = [{ method: 'notify1' }, { method: 'notify2' }].map(
+      (request) => ({ ...request, jsonrpc: '2.0' as const }),
+    );
 
-    const expectedOutput = [{ method: 'notify1' }, { method: 'notify2' }];
+    const expectedOutput = [{ method: 'notify1' }, { method: 'notify2' }].map(
+      (output) => ({ ...output, jsonrpc: '2.0' }),
+    );
 
     const output = await runStreamTest(requests);
     expect(output).toEqual(expectedOutput);
@@ -95,15 +102,15 @@ describe('createDupeReqFilterStream', () => {
       { method: 'notify2' },
       { id: 2, method: 'bar' },
       { id: 3, method: 'baz' },
-    ];
+    ].map((request) => ({ ...request, jsonrpc: '2.0' as const }));
 
     const expectedOutput = [
       { id: 1, method: 'foo' },
       { method: 'notify1' },
       { id: 2, method: 'bar' },
       { method: 'notify2' },
       { id: 3, method: 'baz' },
-    ];
+    ].map((output) => ({ ...output, jsonrpc: '2.0' }));
 
     const output = await runStreamTest(requests);
     expect(output).toEqual(expectedOutput);

diff --git a/app/scripts/lib/createDupeReqFilterStream.ts b/app/scripts/lib/createDupeReqFilterStream.ts
@@ -54,7 +54,8 @@ export default function createDupeReqFilterStream() {
     transform(chunk: JsonRpcRequest, _, cb) {
       // JSON-RPC notifications have no ids; our only recourse is to let them through.
       const hasNoId = chunk.id === undefined;
-      const requestNotYetSeen = seenRequestIds.add(chunk.id);
+      const requestNotYetSeen =
+        chunk.id !== null && seenRequestIds.add(chunk.id);
 
       if (hasNoId || requestNotYetSeen) {
         cb(null, chunk);

diff --git a/app/scripts/lib/ppom/ppom-middleware.test.ts b/app/scripts/lib/ppom/ppom-middleware.test.ts
@@ -1,16 +1,12 @@
-import {
-  type Hex,
-  JsonRpcRequestStruct,
-  JsonRpcResponseStruct,
-} from '@metamask/utils';
+import { type Hex, JsonRpcResponseStruct } from '@metamask/utils';
 import { CHAIN_IDS } from '../../../../shared/constants/network';
 
 import {
   BlockaidReason,
   BlockaidResultType,
 } from '../../../../shared/constants/security-provider';
 import { flushPromises } from '../../../../test/lib/timer-helpers';
-import { createPPOMMiddleware } from './ppom-middleware';
+import { createPPOMMiddleware, PPOMMiddlewareRequest } from './ppom-middleware';
 import {
   generateSecurityAlertId,
   handlePPOMError,
@@ -28,6 +24,12 @@ const SECURITY_ALERT_RESPONSE_MOCK: SecurityAlertResponse = {
   reason: BlockaidReason.permitFarming,
 };
 
+const REQUEST_MOCK = {
+  params: [],
+  id: '',
+  jsonrpc: '2.0' as const,
+};
+
 const createMiddleware = (
   options: {
     chainId?: Hex;
@@ -48,8 +50,7 @@ const createMiddleware = (
   const preferenceController = {
     store: {
       getState: () => ({
-        securityAlertsEnabled:
-          securityAlertsEnabled === undefined ?? securityAlertsEnabled,
+        securityAlertsEnabled: securityAlertsEnabled ?? true,
       }),
     },
   };
@@ -106,14 +107,14 @@ describe('PPOMMiddleware', () => {
     });
 
     const req = {
-      ...JsonRpcRequestStruct,
+      ...REQUEST_MOCK,
       method: 'eth_sendTransaction',
       securityAlertResponse: undefined,
     };
 
     await middlewareFunction(
       req,
-      { ...JsonRpcResponseStruct },
+      { ...JsonRpcResponseStruct.TYPE },
       () => undefined,
     );
 
@@ -130,20 +131,20 @@ describe('PPOMMiddleware', () => {
   it('adds loading response to confirmation requests while validation is in progress', async () => {
     const middlewareFunction = createMiddleware();
 
-    const req = {
-      ...JsonRpcRequestStruct,
+    const req: PPOMMiddlewareRequest<(string | { to: string })[]> = {
+      ...REQUEST_MOCK,
       method: 'eth_sendTransaction',
       securityAlertResponse: undefined,
     };
 
     await middlewareFunction(
       req,
-      { ...JsonRpcResponseStruct },
+      { ...JsonRpcResponseStruct.TYPE },
       () => undefined,
     );
 
-    expect(req.securityAlertResponse.reason).toBe(BlockaidReason.inProgress);
-    expect(req.securityAlertResponse.result_type).toBe(
+    expect(req.securityAlertResponse?.reason).toBe(BlockaidReason.inProgress);
+    expect(req.securityAlertResponse?.result_type).toBe(
       BlockaidResultType.Loading,
     );
   });
@@ -154,11 +155,12 @@ describe('PPOMMiddleware', () => {
     });
 
     const req = {
-      ...JsonRpcRequestStruct,
+      ...REQUEST_MOCK,
       method: 'eth_sendTransaction',
       securityAlertResponse: undefined,
     };
 
+    // @ts-expect-error Passing in invalid input for testing purposes
     await middlewareFunction(req, undefined, () => undefined);
 
     expect(req.securityAlertResponse).toBeUndefined();
@@ -171,14 +173,14 @@ describe('PPOMMiddleware', () => {
     });
 
     const req = {
-      ...JsonRpcRequestStruct,
+      ...REQUEST_MOCK,
       method: 'eth_sendTransaction',
       securityAlertResponse: undefined,
     };
 
     await middlewareFunction(
       req,
-      { ...JsonRpcResponseStruct },
+      { ...JsonRpcResponseStruct.TYPE },
       () => undefined,
     );
 
@@ -190,14 +192,14 @@ describe('PPOMMiddleware', () => {
     const middlewareFunction = createMiddleware();
 
     const req = {
-      ...JsonRpcRequestStruct,
+      ...REQUEST_MOCK,
       method: 'eth_someRequest',
       securityAlertResponse: undefined,
     };
 
     await middlewareFunction(
       req,
-      { ...JsonRpcResponseStruct },
+      { ...JsonRpcResponseStruct.TYPE },
       () => undefined,
     );
 
@@ -210,8 +212,8 @@ describe('PPOMMiddleware', () => {
     const nextMock = jest.fn();
 
     await middlewareFunction(
-      { ...JsonRpcRequestStruct, method: 'eth_sendTransaction' },
-      { ...JsonRpcResponseStruct },
+      { ...REQUEST_MOCK, method: 'eth_sendTransaction' },
+      { ...JsonRpcResponseStruct.TYPE },
       nextMock,
     );
 
@@ -227,12 +229,12 @@ describe('PPOMMiddleware', () => {
     const middlewareFunction = createMiddleware({ error });
 
     const req = {
-      ...JsonRpcRequestStruct,
+      ...REQUEST_MOCK,
       method: 'eth_sendTransaction',
       securityAlertResponse: undefined,
     };
 
-    await middlewareFunction(req, { ...JsonRpcResponseStruct }, nextMock);
+    await middlewareFunction(req, { ...JsonRpcResponseStruct.TYPE }, nextMock);
 
     expect(req.securityAlertResponse).toStrictEqual(
       SECURITY_ALERT_RESPONSE_MOCK,

diff --git a/app/scripts/lib/ppom/ppom-middleware.ts b/app/scripts/lib/ppom/ppom-middleware.ts
@@ -27,6 +27,12 @@ const CONFIRMATION_METHODS = Object.freeze([
   ...SIGNING_METHODS,
 ]);
 
+export type PPOMMiddlewareRequest<
+  Params extends JsonRpcParams = JsonRpcParams,
+> = Required<JsonRpcRequest<Params>> & {
+  securityAlertResponse?: SecurityAlertResponse | undefined;
+};
+
 /**
  * Middleware function that handles JSON RPC requests.
  * This function will be called for every JSON RPC request.
@@ -44,7 +50,7 @@ const CONFIRMATION_METHODS = Object.freeze([
  * @returns PPOMMiddleware function.
  */
 export function createPPOMMiddleware<
-  Params extends JsonRpcParams,
+  Params extends (string | { to: string })[],
   Result extends Json,
 >(
   ppomController: PPOMController,
@@ -58,7 +64,7 @@ export function createPPOMMiddleware<
   ) => void,
 ) {
   return async (
-    req: JsonRpcRequest<Params>,
+    req: PPOMMiddlewareRequest<Params>,
     _res: JsonRpcResponse<Result>,
     next: () => void,
   ) => {

diff --git a/app/scripts/lib/ppom/ppom-util.test.ts b/app/scripts/lib/ppom/ppom-util.test.ts
@@ -31,6 +31,8 @@ const TRANSACTION_ID_MOCK = '123';
 const REQUEST_MOCK = {
   method: 'eth_signTypedData_v4',
   params: [],
+  id: '',
+  jsonrpc: '2.0' as const,
 };
 
 const SECURITY_ALERT_RESPONSE_MOCK: SecurityAlertResponse = {

diff --git a/app/scripts/lib/ppom/ppom-util.ts b/app/scripts/lib/ppom/ppom-util.ts
@@ -25,6 +25,10 @@ const SECURITY_ALERT_RESPONSE_ERROR = {
   reason: BlockaidReason.errored,
 };
 
+type PPOMRequest = Omit<JsonRpcRequest, 'method' | 'params'> & {
+  method: typeof METHOD_SEND_TRANSACTION;
+  params: [TransactionParams];
+};
 export async function validateRequestWithPPOM({
   ppomController,
   request,
@@ -115,12 +119,18 @@ async function usePPOM(
   }
 }
 
-function normalizePPOMRequest(request: JsonRpcRequest): JsonRpcRequest {
-  if (request.method !== METHOD_SEND_TRANSACTION) {
+function normalizePPOMRequest(
+  request: PPOMRequest | JsonRpcRequest,
+): PPOMRequest | JsonRpcRequest {
+  if (
+    !((req): req is PPOMRequest => req.method === METHOD_SEND_TRANSACTION)(
+      request,
+    )
+  ) {
     return request;
   }
 
-  const transactionParams = (request.params?.[0] || {}) as TransactionParams;
+  const transactionParams = request.params[0];
   const normalizedParams = normalizeTransactionParams(transactionParams);
 
   return {

diff --git a/app/scripts/lib/transaction/util.ts b/app/scripts/lib/transaction/util.ts
@@ -251,11 +251,12 @@ function validateSecurity(request: AddTransactionRequest) {
       params: [
         {
           from,
-          to,
-          value,
-          data,
+          to: to ?? '',
+          value: value ?? '',
+          data: data ?? '',
         },
       ],
+      jsonrpc: '2.0' as const,
     };
 
     const securityAlertId = generateSecurityAlertId();

diff --git a/app/scripts/migrations/119.ts b/app/scripts/migrations/119.ts
@@ -46,13 +46,13 @@ function transformState(state: Record<string, any>) {
         .accounts,
     ).length > 0
   ) {
-    Object.values(accountsController.internalAccounts.accounts).forEach(
-      (internalAccount: InternalAccount) => {
-        if (!internalAccount.metadata?.importTime) {
-          internalAccount.metadata.importTime = Date.now();
-        }
-      },
-    );
+    Object.values<InternalAccount>(
+      accountsController.internalAccounts.accounts,
+    ).forEach((internalAccount) => {
+      if (!internalAccount.metadata?.importTime) {
+        internalAccount.metadata.importTime = Date.now();
+      }
+    });
   }
 
   return {