fix:

packages/snap-account-service/CHANGELOG.md

@@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- **BREAKING:** `SnapAccountService` now reads `AccountsController` state to filter account data update events by Snap ownership ([#8916](https://github.com/MetaMask/core/pull/8916)).
+    - Filter account data update events (`notify:accountTransactionsUpdated`, `notify:accountBalancesUpdated`, and `notify:accountAssetListUpdated`) to the accounts that the originating Snap actually owns before republishing them
 - Bump `@metamask/utils` from `^11.11.0` to `^11.12.0` ([#10076](https://github.com/MetaMask/core/pull/10076))
 
 ## [2.1.2]

packages/snap-account-service/package.json

@@ -56,6 +56,7 @@
   },
   "dependencies": {
     "@metamask/account-api": "^2.0.0",
+    "@metamask/accounts-controller": "^39.1.0",
     "@metamask/eth-snap-keyring": "^24.0.0",
     "@metamask/keyring-api": "^24.0.0",
     "@metamask/keyring-controller": "^27.1.1",

packages/snap-account-service/src/SnapAccountService.test.ts

@@ -1,4 +1,5 @@
 import type { AccountGroupId } from '@metamask/account-api';
+import type { AccountsControllerState } from '@metamask/accounts-controller';
 import { SNAP_KEYRING_TYPE } from '@metamask/eth-snap-keyring';
 import type { SnapMessage } from '@metamask/eth-snap-keyring';
 import type { SnapKeyring as SnapKeyringV2 } from '@metamask/eth-snap-keyring/v2';
@@ -85,6 +86,10 @@ type Mocks = {
     >;
     getSelectedAccountGroup: jest.MockedFunction<() => AccountGroupId | ''>;
   };
+  // eslint-disable-next-line @typescript-eslint/naming-convention
+  AccountsController: {
+    getState: jest.MockedFunction<() => AccountsControllerState>;
+  };
 };
 
 /**
@@ -126,6 +131,7 @@ function getMessenger(
       'KeyringController:withKeyringV2Unsafe',
       'AccountTreeController:getAccountGroupObject',
       'AccountTreeController:getSelectedAccountGroup',
+      'AccountsController:getState',
     ],
     events: [
       'SnapController:stateChange',
@@ -141,6 +147,7 @@ function getMessenger(
       'AccountTreeController:accountGroupCreated',
       'AccountTreeController:accountGroupUpdated',
       'AccountTreeController:accountGroupRemoved',
+      'AccountsController:stateChanged',
     ],
   });
   return messenger;
@@ -232,6 +239,49 @@ function buildGroup(
   return { id, accounts } as MockAccountGroup as AccountGroupObject;
 }
 
+/**
+ * Builds a minimal `AccountsControllerState` whose `internalAccounts.accounts`
+ * maps each given account ID to an account owned by `snapId` (via
+ * `metadata.snap.id`). Used to seed the service's Snap-ownership cache.
+ *
+ * @param accounts - The accounts to include.
+ * @returns A minimal `AccountsControllerState`.
+ */
+function buildAccountsState(
+  accounts: { id: string; snapId?: string }[],
+): AccountsControllerState {
+  const accountsRecord = Object.fromEntries(
+    accounts.map(({ id, snapId }) => [
+      id,
+      {
+        id,
+        metadata: snapId ? { snap: { id: snapId } } : {},
+      },
+    ]),
+  );
+  return {
+    internalAccounts: { accounts: accountsRecord },
+  } as unknown as AccountsControllerState;
+}
+
+/**
+ * Publishes an `AccountsController:stateChanged` event on the root messenger,
+ * rebuilding the service's Snap-ownership cache from the given accounts.
+ *
+ * @param rootMessenger - The root messenger.
+ * @param accounts - The accounts to include in the new state.
+ */
+function publishAccountsStateChange(
+  rootMessenger: RootMessenger,
+  accounts: { id: string; snapId?: string }[],
+): void {
+  rootMessenger.publish(
+    'AccountsController:stateChanged',
+    buildAccountsState(accounts),
+    [],
+  );
+}
+
 /**
  * Publishes an AccountTreeController accountGroupCreated event on the root
  * messenger.
@@ -403,18 +453,21 @@ function mockWithKeyringV2Unsafe(
  * @param args - The arguments to this function.
  * @param args.snapIsReady - Initial value of `SnapController.isReady`.
  * @param args.runnableSnaps - Snaps returned by `SnapController:getRunnableSnaps`.
+ * @param args.accounts - Initial accounts
  * @param args.config - Optional service config.
  * @param args.captureException - Optional method to capture exceptions in Sentry.
  * @returns The new service, root messenger, service messenger, and mocks.
  */
 async function setup({
   snapIsReady = true,
   runnableSnaps = [],
+  accounts = [],
   config,
   captureException,
 }: {
   snapIsReady?: boolean;
   runnableSnaps?: TruncatedSnap[];
+  accounts?: { id: string; snapId?: string }[];
   config?: SnapAccountServiceOptions['config'];
   captureException?: (error: Error) => void;
 } = {}): Promise<{
@@ -444,6 +497,9 @@ async function setup({
       getAccountGroupObject: jest.fn().mockReturnValue(undefined),
       getSelectedAccountGroup: jest.fn().mockReturnValue(''),
     },
+    AccountsController: {
+      getState: jest.fn().mockReturnValue(buildAccountsState(accounts)),
+    },
   };
 
   rootMessenger.registerActionHandler(
@@ -482,6 +538,10 @@ async function setup({
     'AccountTreeController:getSelectedAccountGroup',
     mocks.AccountTreeController.getSelectedAccountGroup,
   );
+  rootMessenger.registerActionHandler(
+    'AccountsController:getState',
+    mocks.AccountsController.getState,
+  );
 
   const service = new SnapAccountService({ messenger, config });
 
@@ -1182,39 +1242,65 @@ describe('SnapAccountService', () => {
     });
 
     const MOCK_ACCOUNT_ID = '00000000-0000-4000-8000-000000000001';
+    // An account ID that the Snap does NOT own. Updates for this ID must be
+    // stripped before the event is republished, otherwise a Snap could forge
+    // data for accounts owned by another Snap (or for accounts that do not
+    // exist at all).
+    const MOCK_UNOWNED_ACCOUNT_ID = '00000000-0000-4000-8000-000000000002';
 
     it.each([
       [
         KeyringEvent.AccountBalancesUpdated,
         'SnapAccountService:accountBalancesUpdated' as const,
+        'balances' as const,
         {
           balances: {
             [MOCK_ACCOUNT_ID]: {
               'eip155:1/slip44:60': { amount: '1', unit: 'ETH' },
             },
+            [MOCK_UNOWNED_ACCOUNT_ID]: {
+              'eip155:1/slip44:60': { amount: '99', unit: 'ETH' },
+            },
           },
         } satisfies AccountBalancesUpdatedEventPayload,
       ],
       [
         KeyringEvent.AccountAssetListUpdated,
         'SnapAccountService:accountAssetListUpdated' as const,
+        'assets' as const,
         {
           assets: {
             [MOCK_ACCOUNT_ID]: { added: ['eip155:1/slip44:60'], removed: [] },
+            [MOCK_UNOWNED_ACCOUNT_ID]: {
+              added: ['eip155:1/slip44:60'],
+              removed: [],
+            },
           },
         } satisfies AccountAssetListUpdatedEventPayload,
       ],
       [
         KeyringEvent.AccountTransactionsUpdated,
         'SnapAccountService:accountTransactionsUpdated' as const,
+        'transactions' as const,
         {
-          transactions: { [MOCK_ACCOUNT_ID]: [] },
+          transactions: {
+            [MOCK_ACCOUNT_ID]: [],
+            [MOCK_UNOWNED_ACCOUNT_ID]: [],
+          },
         } satisfies AccountTransactionsUpdatedEventPayload,
       ],
     ] as const)(
-      'publishes %s as a service event without touching the keyring',
-      async (method, event, payload) => {
-        const { service, rootMessenger, mocks } = await setup();
+      'filters %s to accounts owned by the Snap before republishing it',
+      async (method, event, key, payload) => {
+        const { service, rootMessenger, mocks } = await setup({
+          accounts: [
+            { id: MOCK_ACCOUNT_ID, snapId: MOCK_SNAP_ID as string },
+            {
+              id: MOCK_UNOWNED_ACCOUNT_ID,
+              snapId: MOCK_OTHER_SNAP_ID as string,
+            },
+          ],
+        });
         const listener = jest.fn();
         rootMessenger.subscribe(event, listener);
 
@@ -1226,13 +1312,100 @@ describe('SnapAccountService', () => {
         } as unknown as SnapMessage);
 
         expect(result).toBeNull();
-        expect(listener).toHaveBeenCalledWith(payload);
+        // Only the owned account survives the ownership filter.
+        const expectedEntry = (
+          payload as Record<string, Record<string, unknown>>
+        )[key][MOCK_ACCOUNT_ID];
+        expect(listener).toHaveBeenCalledTimes(1);
+        expect(listener).toHaveBeenCalledWith({
+          [key]: { [MOCK_ACCOUNT_ID]: expectedEntry },
+        });
+        // The ownership filter is a synchronous AccountsController-state
+        // cache read — it must NOT touch the keyring on the live path. This
+        // assertion previously locked in the bypass (core#8916) and is now
+        // inverted to require the cache, not the keyring.
         expect(
           mocks.KeyringController.withKeyringV2Unsafe,
         ).not.toHaveBeenCalled();
         expect(mocks.KeyringController.withController).not.toHaveBeenCalled();
       },
     );
+
+    it('drops the whole update when no reported account is owned by the Snap (fail closed)', async () => {
+      const { service, rootMessenger } = await setup({
+        accounts: [
+          { id: MOCK_ACCOUNT_ID, snapId: MOCK_OTHER_SNAP_ID as string },
+        ],
+      });
+      const listener = jest.fn();
+      rootMessenger.subscribe(
+        'SnapAccountService:accountBalancesUpdated',
+        listener,
+      );
+
+      const payload = {
+        balances: {
+          [MOCK_ACCOUNT_ID]: {
+            'eip155:1/slip44:60': { amount: '1', unit: 'ETH' },
+          },
+        },
+      } satisfies AccountBalancesUpdatedEventPayload;
+
+      const result = await service.handleKeyringSnapMessage(MOCK_SNAP_ID, {
+        method: KeyringEvent.AccountBalancesUpdated,
+        params: payload,
+      } as unknown as SnapMessage);
+
+      expect(result).toBeNull();
+      expect(listener).not.toHaveBeenCalled();
+    });
+
+    it('picks up ownership changes from AccountsController:stateChanged', async () => {
+      // Initially the Snap does not own the account, so the update is dropped.
+      const { service, rootMessenger } = await setup({
+        accounts: [
+          { id: MOCK_ACCOUNT_ID, snapId: MOCK_OTHER_SNAP_ID as string },
+        ],
+      });
+      const listener = jest.fn();
+      rootMessenger.subscribe(
+        'SnapAccountService:accountBalancesUpdated',
+        listener,
+      );
+
+      const payload = {
+        balances: {
+          [MOCK_ACCOUNT_ID]: {
+            'eip155:1/slip44:60': { amount: '1', unit: 'ETH' },
+          },
+        },
+      } satisfies AccountBalancesUpdatedEventPayload;
+
+      let result = await service.handleKeyringSnapMessage(MOCK_SNAP_ID, {
+        method: KeyringEvent.AccountBalancesUpdated,
+        params: payload,
+      } as unknown as SnapMessage);
+      expect(result).toBeNull();
+      expect(listener).not.toHaveBeenCalled();
+
+      // The account is now transferred to this Snap — the cache rebuilds on
+      // stateChanged and the next update is forwarded.
+      publishAccountsStateChange(rootMessenger, [
+        { id: MOCK_ACCOUNT_ID, snapId: MOCK_SNAP_ID as string },
+      ]);
+
+      result = await service.handleKeyringSnapMessage(MOCK_SNAP_ID, {
+        method: KeyringEvent.AccountBalancesUpdated,
+        params: payload,
+      } as unknown as SnapMessage);
+      expect(result).toBeNull();
+      expect(listener).toHaveBeenCalledTimes(1);
+      expect(listener).toHaveBeenCalledWith({
+        balances: {
+          [MOCK_ACCOUNT_ID]: payload.balances[MOCK_ACCOUNT_ID],
+        },
+      });
+    });
   });
 
   describe('on AccountTreeController:selectedAccountGroupChange', () => {