fix: Prevent initial connections from being revoked as unused (#3729)

Fixes an issue where initial connections between Snaps could be
considered unused when updating preinstalled Snaps. The fix is to treat
`wallet_snap` as a dynamic permission, it should probably have been
marked as one in the past when `wallet_requestSnaps` was unblocked since
that allows dynamically adding to the permission.

This in combination with a previous commit effectively means that any
permissions granted in the manifest that are considered dynamic, will
not be automatically revoked when the permission is removed from the
manifest. This is intentional, but not ideal.

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> Adds `wallet_snap` to dynamic permissions to prevent revoking initial
connections and introduces a test for two-way preinstalled snap
connections.
> 
> - **Controller**:
> - Add `wallet_snap` to default `dynamicPermissions` in
`SnapController` constructor to prevent unintended revocation of initial
connections.
> - **Tests**:
> - Add test ensuring preinstalled snaps with two-way
`initialConnections` do not trigger
`PermissionController:revokePermissions` for `wallet_snap`.
> 
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
3435d5c. 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

@@ -6087,6 +6087,94 @@ describe('SnapController', () => {
       snapController.destroy();
     });
 
+    it('supports preinstalled snaps with two-way initial connections', async () => {
+      const rootMessenger = getControllerMessenger();
+      jest.spyOn(rootMessenger, 'call');
+
+      rootMessenger.registerActionHandler(
+        'PermissionController:getPermissions',
+        (origin) => {
+          if (origin === `${MOCK_SNAP_ID}2`) {
+            return {
+              [WALLET_SNAP_PERMISSION_KEY]: {
+                caveats: [
+                  {
+                    type: SnapCaveatType.SnapIds,
+                    value: {
+                      [MOCK_SNAP_ID]: {},
+                    },
+                  },
+                ],
+                date: 1664187844588,
+                id: 'izn0WGUO8cvq_jqvLQuQP',
+                invoker: MOCK_ORIGIN,
+                parentCapability: WALLET_SNAP_PERMISSION_KEY,
+              },
+            };
+          }
+
+          return {};
+        },
+      );
+
+      const preinstalledSnaps = [
+        {
+          snapId: MOCK_SNAP_ID,
+          manifest: getSnapManifest({
+            initialConnections: {
+              [`${MOCK_SNAP_ID}2`]: {},
+            },
+          }),
+          files: [
+            {
+              path: DEFAULT_SOURCE_PATH,
+              value: stringToBytes(DEFAULT_SNAP_BUNDLE),
+            },
+            {
+              path: DEFAULT_ICON_PATH,
+              value: stringToBytes(DEFAULT_SNAP_ICON),
+            },
+          ],
+        },
+        {
+          snapId: `${MOCK_SNAP_ID}2` as SnapId,
+          manifest: getSnapManifest({
+            initialConnections: {
+              [MOCK_SNAP_ID]: {},
+            },
+          }),
+          files: [
+            {
+              path: DEFAULT_SOURCE_PATH,
+              value: stringToBytes(DEFAULT_SNAP_BUNDLE),
+            },
+            {
+              path: DEFAULT_ICON_PATH,
+              value: stringToBytes(DEFAULT_SNAP_ICON),
+            },
+          ],
+        },
+      ];
+
+      const snapControllerOptions = getSnapControllerWithEESOptions({
+        preinstalledSnaps,
+        rootMessenger,
+      });
+      const [snapController] = getSnapControllerWithEES(snapControllerOptions);
+
+      expect(snapControllerOptions.messenger.call).not.toHaveBeenCalledWith(
+        'PermissionController:revokePermissions',
+        { [MOCK_SNAP_ID]: ['wallet_snap'] },
+      );
+
+      expect(snapControllerOptions.messenger.call).not.toHaveBeenCalledWith(
+        'PermissionController:revokePermissions',
+        { [`${MOCK_SNAP_ID}2`]: ['wallet_snap'] },
+      );
+
+      snapController.destroy();
+    });
+
     it('supports preinstalled snaps with initial connections', async () => {
       const rootMessenger = getControllerMessenger();
       jest.spyOn(rootMessenger, 'call');

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

@@ -936,7 +936,7 @@ export class SnapController extends BaseController<
     closeAllConnections,
     messenger,
     state,
-    dynamicPermissions = ['endowment:caip25'],
+    dynamicPermissions = ['endowment:caip25', 'wallet_snap'],
     environmentEndowmentPermissions = [],
     excludedPermissions = {},
     idleTimeCheckInterval = inMilliseconds(5, Duration.Second),
@@ -4305,6 +4305,10 @@ export class SnapController extends BaseController<
 
         // If the permission doesn't have dependencies, or if at least one of
         // its dependencies is desired, include it in the desired permissions.
+        // NOTE: This effectively means that any permissions granted in the manifest
+        // that are considered dynamic, will not be automatically revoked
+        // when the permission is removed from the manifest.
+        // TODO: Deal with this technical debt.
         if (!hasDependencies || hasDependency) {
           accumulator[permissionName] = oldPermissions[permissionName];
         }