fix: Throw if Snap not installed (#3666)

When invoking Snaps directly, e.g. from within MetaMask, we were
currently not validating that Snaps were installed. This means that
misleading errors would be thrown about permissions when the Snap was
not installed in the first place.

Also removes this check from one RPC method, since it is now
unnecessary. We leave the RPC method around in `invokeKeyring` to not
change the error handling behavior.

Closes #3658

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> Adds an installation check to SnapController.handleRequest, removes
redundant snap-installed checks from invokeSnap, aligns error messages,
and updates tests and coverage thresholds.
> 
> - **SnapController**:
> - Validate Snap exists in `handleRequest` and throw a clear error if
not installed.
>   - Refactor subsequent checks to use local `snap` (enabled/status).
> - **RPC Methods**:
> - `invokeSnap`: Remove `getSnap` hook and installed check; rely on
controller for validation.
> - `invokeKeyring`: Align "not installed" error message with
controller.
> - **Tests**:
>   - Add test for non-installed Snap in `SnapController.handleRequest`.
> - Update `invokeSnap` tests to drop `getSnap` usage and non-installed
error case.
>   - Update `invokeKeyring` tests to match new error message.
> - **Config**:
>   - Slightly adjust Jest coverage thresholds in `snaps-rpc-methods`.
> 
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
303b2b9. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

Author: FrederikBolding

packages/snaps-controllers/src/snaps/SnapController.test.tsx

@@ -2192,6 +2192,27 @@ describe('SnapController', () => {
   });
 
   describe('handleRequest', () => {
+    it('throws if the Snap is not installed', async () => {
+      const snapController = getSnapController();
+
+      await expect(
+        snapController.handleRequest({
+          snapId: 'npm:foo' as SnapId,
+          origin: METAMASK_ORIGIN,
+          handler: HandlerType.OnUserInput,
+          request: {
+            jsonrpc: '2.0',
+            method: 'test',
+            params: { id: MOCK_INTERFACE_ID },
+          },
+        }),
+      ).rejects.toThrow(
+        'The Snap "npm:foo" is not installed. Please install it before invoking it.',
+      );
+
+      snapController.destroy();
+    });
+
     it.each(
       Object.keys(handlerEndowments).filter(
         (handler) => handlerEndowments[handler as HandlerType],

packages/snaps-controllers/src/snaps/SnapController.ts

@@ -3588,6 +3588,13 @@ export class SnapController extends BaseController<
   }: SnapRpcHookArgs & { snapId: SnapId }): Promise<unknown> {
     this.#assertCanUsePlatform();
 
+    const snap = this.get(snapId);
+
+    assert(
+      snap,
+      `The Snap "${snapId}" is not installed. Please install it before invoking it.`,
+    );
+
     assert(
       origin === METAMASK_ORIGIN || isValidUrl(origin),
       "'origin' must be a valid URL or 'metamask'.",
@@ -3666,11 +3673,11 @@ export class SnapController extends BaseController<
       throw new Error(`"${handlerType}" can only be invoked by MetaMask.`);
     }
 
-    if (!this.state.snaps[snapId].enabled) {
+    if (!snap.enabled) {
       throw new Error(`Snap "${snapId}" is disabled.`);
     }
 
-    if (this.state.snaps[snapId].status === SnapStatus.Installing) {
+    if (snap.status === SnapStatus.Installing) {
       throw new Error(
         `Snap "${snapId}" is currently being installed. Please try again later.`,
       );

packages/snaps-rpc-methods/jest.config.js

@@ -10,9 +10,9 @@ module.exports = deepmerge(baseConfig, {
   ],
   coverageThreshold: {
     global: {
-      branches: 95.7,
+      branches: 95.68,
       functions: 98.75,
-      lines: 98.99,
+      lines: 98.98,
       statements: 98.69,
     },
   },

packages/snaps-rpc-methods/src/permitted/invokeKeyring.test.ts

@@ -303,7 +303,7 @@ describe('wallet_invokeKeyring', () => {
       expect(response.error).toStrictEqual({
         ...rpcErrors
           .invalidRequest({
-            message: `The snap "${MOCK_SNAP_ID}" is not installed. Please install it first, before invoking the snap.`,
+            message: `The Snap "${MOCK_SNAP_ID}" is not installed. Please install it before invoking it.`,
           })
           .serialize(),
         stack: expect.any(String),

packages/snaps-rpc-methods/src/permitted/invokeKeyring.ts

@@ -107,8 +107,9 @@ async function invokeKeyringImplementation(
 
   if (!getSnap(snapId)) {
     return end(
+      // Mirror error message from SnapController.
       rpcErrors.invalidRequest({
-        message: `The snap "${snapId}" is not installed. Please install it first, before invoking the snap.`,
+        message: `The Snap "${snapId}" is not installed. Please install it before invoking it.`,
       }),
     );
   }

packages/snaps-rpc-methods/src/restricted/invokeSnap.test.ts

@@ -23,7 +23,6 @@ describe('builder', () => {
       targetName: WALLET_SNAP_PERMISSION_KEY,
       specificationBuilder: expect.any(Function),
       methodHooks: {
-        getSnap: true,
         handleSnapRpcRequest: true,
       },
     });
@@ -33,7 +32,6 @@ describe('builder', () => {
     expect(
       invokeSnapBuilder.specificationBuilder({
         methodHooks: {
-          getSnap: jest.fn(),
           handleSnapRpcRequest: jest.fn(),
         },
       }),
@@ -53,7 +51,6 @@ describe('builder', () => {
 describe('specificationBuilder', () => {
   const specification = invokeSnapBuilder.specificationBuilder({
     methodHooks: {
-      getSnap: jest.fn(),
       handleSnapRpcRequest: jest.fn(),
     },
   });
@@ -90,7 +87,6 @@ describe('implementation', () => {
     }) as any;
   it('calls handleSnapRpcRequest', async () => {
     const hooks = getMockHooks();
-    hooks.getSnap.mockImplementation(getTruncatedSnap);
     const implementation = getInvokeSnapImplementation(hooks);
     await implementation({
       context: { origin: MOCK_ORIGIN },
@@ -101,8 +97,6 @@ describe('implementation', () => {
       },
     });
 
-    expect(hooks.getSnap).toHaveBeenCalledTimes(1);
-    expect(hooks.getSnap).toHaveBeenCalledWith(MOCK_SNAP_ID);
     expect(hooks.handleSnapRpcRequest).toHaveBeenCalledWith({
       handler: 'onRpcRequest',
       origin: MOCK_ORIGIN,
@@ -113,27 +107,6 @@ describe('implementation', () => {
       snapId: MOCK_SNAP_ID,
     });
   });
-
-  it('throws if snap is not installed', async () => {
-    const hooks = getMockHooks();
-    const implementation = getInvokeSnapImplementation(hooks);
-    await expect(
-      implementation({
-        context: { origin: MOCK_ORIGIN },
-        method: WALLET_SNAP_PERMISSION_KEY,
-        params: {
-          snapId: MOCK_SNAP_ID,
-          request: { method: 'hello', params: {} },
-        },
-      }),
-    ).rejects.toThrow(
-      `The snap "${MOCK_SNAP_ID}" is not installed. Please install it first, before invoking the snap.`,
-    );
-    expect(hooks.getSnap).toHaveBeenCalledTimes(1);
-    expect(hooks.getSnap).toHaveBeenCalledWith(MOCK_SNAP_ID);
-
-    expect(hooks.handleSnapRpcRequest).not.toHaveBeenCalled();
-  });
 });
 
 describe('handleSnapInstall', () => {

packages/snaps-rpc-methods/src/restricted/invokeSnap.ts

@@ -12,7 +12,7 @@ import type {
   RequestSnapsParams,
   RequestSnapsResult,
 } from '@metamask/snaps-sdk';
-import type { Snap, SnapRpcHookArgs } from '@metamask/snaps-utils';
+import type { SnapRpcHookArgs } from '@metamask/snaps-utils';
 import { HandlerType, SnapCaveatType } from '@metamask/snaps-utils';
 import type { Json, NonEmptyArray } from '@metamask/utils';
 
@@ -37,7 +37,6 @@ export type GetPermittedSnaps = {
 type AllowedActions = InstallSnaps | GetPermittedSnaps;
 
 export type InvokeSnapMethodHooks = {
-  getSnap: (snapId: string) => Snap | undefined;
   handleSnapRpcRequest: ({
     snapId,
     origin,
@@ -139,7 +138,6 @@ const specificationBuilder: PermissionSpecificationBuilder<
 };
 
 const methodHooks: MethodHooksObject<InvokeSnapMethodHooks> = {
-  getSnap: true,
   handleSnapRpcRequest: true,
 };
 
@@ -153,13 +151,11 @@ export const invokeSnapBuilder = Object.freeze({
  * Builds the method implementation for `wallet_snap_*`.
  *
  * @param hooks - The RPC method hooks.
- * @param hooks.getSnap - A function that retrieves all information stored about a snap.
  * @param hooks.handleSnapRpcRequest - A function that sends an RPC request to a snap's RPC handler or throws if that fails.
  * @returns The method implementation which returns the result of `handleSnapRpcRequest`.
  * @throws If the params are invalid.
  */
 export function getInvokeSnapImplementation({
-  getSnap,
   handleSnapRpcRequest,
 }: InvokeSnapMethodHooks) {
   return async function invokeSnap(
@@ -169,12 +165,6 @@ export function getInvokeSnapImplementation({
 
     const { snapId, request } = params as InvokeSnapParams;
 
-    if (!getSnap(snapId)) {
-      throw rpcErrors.invalidRequest({
-        message: `The snap "${snapId}" is not installed. Please install it first, before invoking the snap.`,
-      });
-    }
-
     const { origin } = context;
 
     return (await handleSnapRpcRequest({