feat(keyring-snap-bridge): populate v2 SnapKeyring capabilities from the manifest (#581)

- Populate the v2 `SnapKeyring.capabilities` from the Snap manifest
(`endowment:keyring`) on `deserialize`, via `SnapController:getSnap`,
fulfilling the v2 `Keyring` contract's required `capabilities` field.
Falls back to the empty default when the Snap declares none.
- Guard v2 `SnapKeyring` operations
(`getAccounts`/`getAccount`/`createAccounts`/`deleteAccount`/`submitRequest`)
until `deserialize` has run; they now throw `"SnapKeyring has not been
initialized"` instead of operating on an unbound keyring.
- Bump `@metamask/snaps-utils` to `^12.2.1` (`keyring-snap-bridge` +
`keyring-internal-snap-client`) for the `capabilities` field on the
`endowment:keyring` manifest type.

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Changes snap keyring lifecycle and when account operations are
allowed; capabilities are taken from the manifest without local shape
validation, so struct drift could cause runtime issues.
> 
> **Overview**
> v2 **`SnapKeyring`** now sets **`capabilities`** during
**`deserialize`** by reading the Snap manifest’s **`endowment:keyring`**
entry via **`SnapController:getSnap`**, with **`EMPTY_CAPABILITIES`** as
the typed default when the snap is missing or declares none (including
clearing stale values on re-hydrate).
> 
> **`getAccounts`**, **`getAccount`**, **`createAccounts`**,
**`deleteAccount`**, and **`submitRequest`** are blocked until
**`deserialize`** finishes, throwing *SnapKeyring has not been
initialized* instead of running on an unbound keyring.
> 
> **`@metamask/snaps-utils`** is bumped to **`^12.2.1`** in
**`keyring-snap-bridge`** and **`keyring-internal-snap-client`** for
manifest typing around **`capabilities`**.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
39a8b94. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

Author: hmalik88

packages/keyring-internal-snap-client/package.json

@@ -78,7 +78,7 @@
     "@metamask/auto-changelog": "^6.1.0",
     "@metamask/snaps-controllers": "^19.0.1",
     "@metamask/snaps-sdk": "^11.0.0",
-    "@metamask/snaps-utils": "^12.1.3",
+    "@metamask/snaps-utils": "^12.2.1",
     "@metamask/utils": "^11.11.0",
     "@ts-bridge/cli": "^0.6.3",
     "@types/jest": "^29.5.12",

packages/keyring-snap-bridge/CHANGELOG.md

@@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- Populate v2 `SnapKeyring` `capabilities` from the Snap manifest (`endowment:keyring`) on `deserialize` ([#581](https://github.com/MetaMask/accounts/pull/581))
+- Guard v2 `SnapKeyring` operations until `deserialize` has run (throws "SnapKeyring has not been initialized") ([#581](https://github.com/MetaMask/accounts/pull/581))
+
+### Changed
+
+- Bump `@metamask/snaps-utils` from `^12.1.3` to `^12.2.1` ([#581](https://github.com/MetaMask/accounts/pull/581))
+
 ## [22.3.0]
 
 ### Added

packages/keyring-snap-bridge/package.json

@@ -77,7 +77,7 @@
     "@metamask/messenger": "^1.1.1",
     "@metamask/snaps-controllers": "^19.0.1",
     "@metamask/snaps-sdk": "^11.0.0",
-    "@metamask/snaps-utils": "^12.1.3",
+    "@metamask/snaps-utils": "^12.2.1",
     "@metamask/superstruct": "^3.1.0",
     "@metamask/utils": "^11.11.0",
     "@types/uuid": "^9.0.8",

packages/keyring-snap-bridge/src/v2/SnapKeyring.test.ts

@@ -10,7 +10,7 @@ import type { SnapId } from '@metamask/snaps-sdk';
 
 import type { SnapKeyringMessenger } from '../SnapKeyringMessenger';
 import type { SnapKeyringCallbacks } from './SnapKeyring';
-import { isSnapKeyring, SnapKeyring } from './SnapKeyring';
+import { EMPTY_CAPABILITIES, isSnapKeyring, SnapKeyring } from './SnapKeyring';
 
 const SNAP_ID = 'npm:@metamask/test-snap' as SnapId;
 
@@ -101,6 +101,144 @@ describe('SnapKeyring', () => {
     });
   });
 
+  describe('capabilities', () => {
+    const manifestCapabilities = {
+      scopes: ['solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp'],
+      bip44: { deriveIndex: true, deriveIndexRange: true, discover: true },
+    };
+
+    /**
+     * Build and deserialize a keyring whose messenger returns a snap with the
+     * given `endowment:keyring` permission value from `SnapController:getSnap`.
+     *
+     * @param keyringPermission - The `endowment:keyring` initial-permission value
+     * (or `undefined` to simulate a snap that is not found).
+     * @returns The deserialized keyring.
+     */
+    async function makeKeyringWithSnap(
+      keyringPermission: unknown,
+    ): Promise<SnapKeyring> {
+      const messenger = {
+        call: jest.fn((action: string) =>
+          action === 'SnapController:getSnap' && keyringPermission !== undefined
+            ? {
+                manifest: {
+                  initialPermissions: {
+                    'endowment:keyring': keyringPermission,
+                  },
+                },
+              }
+            : undefined,
+        ),
+        publish: jest.fn(),
+      } as unknown as SnapKeyringMessenger;
+      const keyring = new SnapKeyring({
+        messenger,
+        callbacks: makeMockCallbacks(),
+      });
+      await keyring.deserialize({ snapId: SNAP_ID, accounts: {} });
+      return keyring;
+    }
+
+    it('populates capabilities from the snap manifest on deserialize', async () => {
+      const keyring = await makeKeyringWithSnap({
+        capabilities: manifestCapabilities,
+      });
+      expect(keyring.capabilities).toStrictEqual(manifestCapabilities);
+    });
+
+    it('keeps the empty default when the snap declares no capabilities', async () => {
+      const keyring = await makeKeyringWithSnap({
+        allowedOrigins: ['https://portfolio.metamask.io'],
+      });
+      expect(keyring.capabilities).toStrictEqual(EMPTY_CAPABILITIES);
+    });
+
+    it('keeps the empty default when the snap cannot be found', async () => {
+      const keyring = await makeKeyringWithSnap(undefined);
+      expect(keyring.capabilities).toStrictEqual(EMPTY_CAPABILITIES);
+    });
+
+    it('clears stale capabilities on a later deserialize without capabilities', async () => {
+      const snapWith = (keyringPermission: unknown): unknown => ({
+        manifest: {
+          initialPermissions: { 'endowment:keyring': keyringPermission },
+        },
+      });
+      const mockGetSnap = jest.fn();
+      const messenger = {
+        call: jest.fn((action: string) =>
+          action === 'SnapController:getSnap' ? mockGetSnap() : undefined,
+        ),
+        publish: jest.fn(),
+      } as unknown as SnapKeyringMessenger;
+      const keyring = new SnapKeyring({
+        messenger,
+        callbacks: makeMockCallbacks(),
+      });
+      mockGetSnap.mockReturnValueOnce(
+        snapWith({ capabilities: manifestCapabilities }),
+      );
+      await keyring.deserialize({ snapId: SNAP_ID, accounts: {} });
+      expect(keyring.capabilities).toStrictEqual(manifestCapabilities);
+      // Manifest won't yield any capabilities this time, so we end up
+      // clearing the old ones and fallback to the `EMPTY_CAPABILITIES`.
+      mockGetSnap.mockReturnValueOnce(snapWith({ allowedOrigins: [] }));
+      await keyring.deserialize({ snapId: SNAP_ID, accounts: {} });
+      expect(keyring.capabilities).toStrictEqual(EMPTY_CAPABILITIES);
+    });
+  });
+
+  describe('initialization guard', () => {
+    const ERROR = 'SnapKeyring has not been initialized';
+
+    /**
+     * Build a `SnapKeyring` WITHOUT calling `deserialize`.
+     *
+     * @returns The uninitialized keyring.
+     */
+    function makeUninitializedKeyring(): SnapKeyring {
+      const messenger = {
+        call: jest.fn(),
+        publish: jest.fn(),
+      } as unknown as SnapKeyringMessenger;
+      return new SnapKeyring({ messenger, callbacks: makeMockCallbacks() });
+    }
+
+    it('rejects getAccounts before deserialize is called', async () => {
+      await expect(makeUninitializedKeyring().getAccounts()).rejects.toThrow(
+        ERROR,
+      );
+    });
+
+    it('rejects getAccount before deserialize is called', async () => {
+      await expect(
+        makeUninitializedKeyring().getAccount(account1.id),
+      ).rejects.toThrow(ERROR);
+    });
+
+    it('rejects createAccounts before deserialize is called', async () => {
+      await expect(
+        makeUninitializedKeyring().createAccounts({
+          type: 'bip44:derive-index',
+          entropySource: 'mock-entropy-source',
+          groupIndex: 0,
+        } as CreateAccountOptions),
+      ).rejects.toThrow(ERROR);
+    });
+
+    it('rejects deleteAccount before deserialize is called', async () => {
+      await expect(
+        makeUninitializedKeyring().deleteAccount(account1.id),
+      ).rejects.toThrow(ERROR);
+    });
+
+    it('allows operations after deserialize', async () => {
+      const { keyring } = await makeKeyring();
+      expect(await keyring.getAccounts()).toStrictEqual([]);
+    });
+  });
+
   describe('snapId', () => {
     it('returns the snap ID set during deserialize', async () => {
       const { keyring } = await makeKeyring();

packages/keyring-snap-bridge/src/v2/SnapKeyring.ts

@@ -27,6 +27,15 @@ import type {
 } from '../SnapKeyringV1';
 import { equalsIgnoreCase } from '../util';
 
+/**
+ * Default, empty capabilities used until the snap manifest is read on
+ * `deserialize`. Typed (no cast) so that adding a new required
+ * `KeyringCapabilities` field surfaces here as a compile error.
+ */
+export const EMPTY_CAPABILITIES: KeyringCapabilities = Object.freeze({
+  scopes: [],
+});
+
 /**
  * Superstruct schema for {@link SnapKeyringState}.
  *
@@ -108,6 +117,12 @@ export class SnapKeyring extends SnapKeyringV1 implements Keyring {
    */
   readonly #lock: Mutex;
 
+  /**
+   * Whether `deserialize` has completed. Keyring operations are guarded against
+   * use before the keyring has been bound to a snap and its state loaded.
+   */
+  #initialized = false;
+
   // ──────────────────────────────────────────────
   // Keyring properties
   // ──────────────────────────────────────────────
@@ -127,10 +142,8 @@ export class SnapKeyring extends SnapKeyringV1 implements Keyring {
     this.#callbacks = options.callbacks;
     this.#lock = new Mutex();
 
-    // Default capabilities — parent updates this when snap metadata is available.
-    // We cast here because KeyringCapabilities requires a non-empty scopes array,
-    // but we don't have the snap metadata at construction time (e.g. during deserialization).
-    this.capabilities = { scopes: [] } as unknown as KeyringCapabilities;
+    // Default capabilities; replaced from the snap manifest on `deserialize`.
+    this.capabilities = EMPTY_CAPABILITIES;
   }
 
   /**
@@ -172,6 +185,7 @@ export class SnapKeyring extends SnapKeyringV1 implements Keyring {
    * @returns A promise that resolves to an array of all accounts.
    */
   async getAccounts(): Promise<KeyringAccount[]> {
+    this.#assertInitialized();
     return this.accounts();
   }
 
@@ -182,6 +196,7 @@ export class SnapKeyring extends SnapKeyringV1 implements Keyring {
    * @returns A promise that resolves to the account.
    */
   async getAccount(accountId: AccountId): Promise<KeyringAccount> {
+    this.#assertInitialized();
     const account = this.lookupAccount(accountId);
     if (!account) {
       throw new Error(
@@ -209,6 +224,7 @@ export class SnapKeyring extends SnapKeyringV1 implements Keyring {
   async createAccounts(
     options: CreateAccountOptions,
   ): Promise<KeyringAccount[]> {
+    this.#assertInitialized();
     return this.#withLock(async () => {
       // Keep track of address/account ID part of this batch, to avoid having duplicates.
       const batchAddresses = new Set<string>();
@@ -294,6 +310,7 @@ export class SnapKeyring extends SnapKeyringV1 implements Keyring {
    * @param accountId - ID of the account to delete.
    */
   async deleteAccount(accountId: AccountId): Promise<void> {
+    this.#assertInitialized();
     // Always remove the account from the registry, even if the Snap is going to
     // fail to delete it. removeAccount fires onUnregister to clean #accountIndex.
     this.removeAccount(accountId);
@@ -338,6 +355,7 @@ export class SnapKeyring extends SnapKeyringV1 implements Keyring {
       params?: Json[] | Record<string, Json>;
     };
   }): Promise<Json> {
+    this.#assertInitialized();
     const account = this.lookupAccount(request.account);
     if (!account) {
       throw new Error(
@@ -487,6 +505,12 @@ export class SnapKeyring extends SnapKeyringV1 implements Keyring {
     // mismatch to prevent swapping a keyring's identity via deserialize).
     this.bindSnapId(state.snapId as SnapId);
 
+    // Refresh capabilities from the snap manifest on every deserialize, falling
+    // back to the empty default so a re-hydrate clears any previously-loaded
+    // capabilities when the snap no longer declares them.
+    this.capabilities =
+      this.#resolveKeyringCapabilities() ?? EMPTY_CAPABILITIES;
+
     // Migrate v1 accounts to v2.
     const migratedAccounts: Record<string, KeyringAccount> = {};
     for (const [id, rawAccount] of Object.entries(state.accounts)) {
@@ -508,9 +532,41 @@ export class SnapKeyring extends SnapKeyringV1 implements Keyring {
     for (const account of Object.values(migratedAccounts)) {
       this.setAccount(account);
     }
+
+    this.#initialized = true;
   }
 
   // ──────────────────────────────────────────────
   // Private helpers
   // ──────────────────────────────────────────────
+
+  /**
+   * Assert that the keyring has been initialized.
+   *
+   * @throws An error if the keyring has not been initialized.
+   */
+  #assertInitialized(): void {
+    if (!this.#initialized) {
+      throw new Error(
+        'SnapKeyring has not been initialized: call deserialize() first',
+      );
+    }
+  }
+
+  /**
+   * Resolve the keyring capabilities from the snap manifest.
+   *
+   * @returns The keyring capabilities, or `undefined` if the snap manifest does not declare any capabilities.
+   */
+  #resolveKeyringCapabilities(): KeyringCapabilities | undefined {
+    const snap = this.messenger.call('SnapController:getSnap', this.snapId);
+    // READ THIS CAREFULLY:
+    // We are not validating the shape of the capabilities here, because there is
+    // manifest validation done already on the snaps side, the snaps repo maintains
+    // a copy of the `KeyringCapabilitiesStruct`.
+    // We must ensure that both structs are always in-sync, otherwise the type-cast
+    // could cause runtime issues!
+    return snap?.manifest.initialPermissions['endowment:keyring']
+      ?.capabilities as KeyringCapabilities | undefined;
+  }
 }