fix:

README.md

@@ -609,6 +609,7 @@ linkStyle default opacity:0.5
   smart_transactions_controller --> remote_feature_flag_controller;
   smart_transactions_controller --> transaction_controller;
   smart_transactions_controller --> json_rpc_engine;
+  snap_account_service --> accounts_controller;
   snap_account_service --> keyring_controller;
   snap_account_service --> messenger;
   social_controllers --> base_controller;

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 ([#10057](https://github.com/MetaMask/core/pull/10057)).
+  - 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.1",
     "@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,8 @@ function getMessenger(
       'AccountTreeController:accountGroupCreated',
       'AccountTreeController:accountGroupUpdated',
       'AccountTreeController:accountGroupRemoved',
+      'AccountsController:accountsAdded',
+      'AccountsController:accountsRemoved',
     ],
   });
   return messenger;
@@ -232,6 +240,66 @@ 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:accountsAdded` event on the root messenger,
+ * adding the given accounts to the service's Snap-ownership cache.
+ *
+ * @param rootMessenger - The root messenger.
+ * @param accounts - The accounts that were added.
+ */
+function publishAccountsAdded(
+  rootMessenger: RootMessenger,
+  accounts: { id: string; snapId?: string }[],
+): void {
+  rootMessenger.publish(
+    'AccountsController:accountsAdded',
+    accounts.map(({ id, snapId }) => ({
+      id,
+      metadata: snapId ? { snap: { id: snapId } } : {},
+    })),
+  );
+}
+
+/**
+ * Publishes an `AccountsController:accountsRemoved` event on the root
+ * messenger, removing the given account IDs from the service's Snap-ownership
+ * cache.
+ *
+ * @param rootMessenger - The root messenger.
+ * @param accountIds - The IDs of the accounts that were removed.
+ */
+function publishAccountsRemoved(
+  rootMessenger: RootMessenger,
+  accountIds: string[],
+): void {
+  rootMessenger.publish('AccountsController:accountsRemoved', accountIds);
+}
+
 /**
  * Publishes an AccountTreeController accountGroupCreated event on the root
  * messenger.
@@ -403,18 +471,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 +515,9 @@ async function setup({
       getAccountGroupObject: jest.fn().mockReturnValue(undefined),
       getSelectedAccountGroup: jest.fn().mockReturnValue(''),
     },
+    AccountsController: {
+      getState: jest.fn().mockReturnValue(buildAccountsState(accounts)),
+    },
   };
 
   rootMessenger.registerActionHandler(
@@ -482,6 +556,10 @@ async function setup({
     'AccountTreeController:getSelectedAccountGroup',
     mocks.AccountTreeController.getSelectedAccountGroup,
   );
+  rootMessenger.registerActionHandler(
+    'AccountsController:getState',
+    mocks.AccountsController.getState,
+  );
 
   const service = new SnapAccountService({ messenger, config });
 
@@ -1182,39 +1260,71 @@ 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';
+    // An account ID that has no Snap owner. Updates for this ID must be
+    // stripped too — it is owned by nobody.
+    const MOCK_NO_SNAP_ACCOUNT_ID = '00000000-0000-4000-8000-000000000003';
 
     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,
+            },
+            // An account with no Snap owner must be skipped when building the
+            // cache (it is owned by nobody).
+            { id: MOCK_NO_SNAP_ACCOUNT_ID },
+          ],
+        });
         const listener = jest.fn();
         rootMessenger.subscribe(event, listener);
 
@@ -1226,13 +1336,137 @@ 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.each([
+      [
+        KeyringEvent.AccountBalancesUpdated,
+        'SnapAccountService:accountBalancesUpdated' as const,
+        {
+          balances: {
+            [MOCK_ACCOUNT_ID]: {
+              'eip155:1/slip44:60': { amount: '1', unit: 'ETH' },
+            },
+          },
+        } satisfies AccountBalancesUpdatedEventPayload,
+      ],
+      [
+        KeyringEvent.AccountAssetListUpdated,
+        'SnapAccountService:accountAssetListUpdated' as const,
+        {
+          assets: {
+            [MOCK_ACCOUNT_ID]: { added: ['eip155:1/slip44:60'], removed: [] },
+          },
+        } satisfies AccountAssetListUpdatedEventPayload,
+      ],
+      [
+        KeyringEvent.AccountTransactionsUpdated,
+        'SnapAccountService:accountTransactionsUpdated' as const,
+        {
+          transactions: {
+            [MOCK_ACCOUNT_ID]: [],
+          },
+        } satisfies AccountTransactionsUpdatedEventPayload,
+      ],
+    ] as const)(
+      'drops the whole %s update when no reported account is owned by the Snap (fail closed)',
+      async (method, event, payload) => {
+        const { service, rootMessenger } = await setup({
+          accounts: [
+            { id: MOCK_ACCOUNT_ID, snapId: MOCK_OTHER_SNAP_ID as string },
+          ],
+        });
+        const listener = jest.fn();
+        rootMessenger.subscribe(event, listener);
+
+        const result = await service.handleKeyringSnapMessage(MOCK_SNAP_ID, {
+          method,
+          params: payload,
+        } as unknown as SnapMessage);
+
+        expect(result).toBeNull();
+        expect(listener).not.toHaveBeenCalled();
+      },
+    );
+
+    it('picks up added/removed accounts from AccountsController:accountsAdded and :accountsRemoved', 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 added for this Snap — the cache picks it up from
+      // `accountsAdded` and the next update is forwarded. A no-Snap account
+      // is included to verify such accounts are skipped when updating the
+      // cache incrementally.
+      publishAccountsAdded(rootMessenger, [
+        { id: MOCK_ACCOUNT_ID, snapId: MOCK_SNAP_ID as string },
+        { id: MOCK_NO_SNAP_ACCOUNT_ID },
+      ]);
+
+      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],
+        },
+      });
+
+      // The account is removed — the cache drops it and the next update is
+      // dropped again (fail closed).
+      publishAccountsRemoved(rootMessenger, [MOCK_ACCOUNT_ID]);
+
+      listener.mockClear();
+      result = await service.handleKeyringSnapMessage(MOCK_SNAP_ID, {
+        method: KeyringEvent.AccountBalancesUpdated,
+        params: payload,
+      } as unknown as SnapMessage);
+      expect(result).toBeNull();
+      expect(listener).not.toHaveBeenCalled();
+    });
   });
 
   describe('on AccountTreeController:selectedAccountGroupChange', () => {