Skip to content

Commit cac76dd

Browse files
fix: Skip refetching the registry in handleRegistryUpdate (#3961)
Our tests didn't catch this, but in production, triggering `SnapRegistryController:registryUpdated` triggers `handleRegistryUpdate` which may try to refetch the registry and cause an infinite loop. We can fix this by passing a flag to not refetch the registry. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes registry verification/version-resolution calls to optionally skip refetching, which could alter behavior on allowlist misses and impact snap blocking/auto-update flows if misused. > > **Overview** > Prevents an infinite update loop when handling `SnapRegistryController:registryUpdated` by ensuring `SnapController#handleRegistryUpdate` does not trigger a registry refetch during blocklist checks and preinstalled allowlist version resolution. > > This adds a `skipRefetch` flag plumbed through `SnapRegistryController:get` and `SnapController#resolveAllowlistVersion` (forwarded to `SnapRegistryController:resolveVersion`), and updates the corresponding `SnapController` test to assert the new `get(..., true)` call signature. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit c0401da. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 22f8da9 commit cac76dd

3 files changed

Lines changed: 18 additions & 10 deletions

File tree

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

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10515,16 +10515,19 @@ describe('SnapController', () => {
1051510515
await snapController.updateRegistry();
1051610516

1051710517
// Ensure that CheckSnapBlockListArg is correct
10518-
expect(registry.get).toHaveBeenCalledWith({
10519-
[mockSnapA.id]: {
10520-
version: mockSnapA.manifest.version,
10521-
checksum: mockSnapA.manifest.source.shasum,
10522-
},
10523-
[mockSnapB.id]: {
10524-
version: mockSnapB.manifest.version,
10525-
checksum: mockSnapB.manifest.source.shasum,
10518+
expect(registry.get).toHaveBeenCalledWith(
10519+
{
10520+
[mockSnapA.id]: {
10521+
version: mockSnapA.manifest.version,
10522+
checksum: mockSnapA.manifest.source.shasum,
10523+
},
10524+
[mockSnapB.id]: {
10525+
version: mockSnapB.manifest.version,
10526+
checksum: mockSnapB.manifest.source.shasum,
10527+
},
1052610528
},
10527-
});
10529+
true,
10530+
);
1052810531

1052910532
// A is blocked and disabled
1053010533
expect(snapController.getSnap(mockSnapA.id)?.blocked).toBe(true);

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1471,6 +1471,7 @@ export class SnapController extends BaseController<
14711471
},
14721472
{},
14731473
),
1474+
true,
14741475
);
14751476

14761477
await Promise.all(
@@ -1496,6 +1497,7 @@ export class SnapController extends BaseController<
14961497
const resolvedVersion = await this.#resolveAllowlistVersion(
14971498
snap.id,
14981499
preinstalledVersionRange,
1500+
true,
14991501
);
15001502

15011503
if (
@@ -3100,11 +3102,13 @@ export class SnapController extends BaseController<
31003102
async #resolveAllowlistVersion(
31013103
snapId: SnapId,
31023104
versionRange: SemVerRange,
3105+
skipRefetch = false,
31033106
): Promise<SemVerRange> {
31043107
return await this.messenger.call(
31053108
'SnapRegistryController:resolveVersion',
31063109
snapId,
31073110
versionRange,
3111+
skipRefetch,
31083112
);
31093113
}
31103114

packages/snaps-controllers/src/snaps/registry/SnapRegistryController.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -321,11 +321,12 @@ export class SnapRegistryController extends BaseController<
321321

322322
async get(
323323
snaps: SnapRegistryRequest,
324+
skipRefetch = false,
324325
): Promise<Record<string, SnapRegistryResult>> {
325326
return Object.entries(snaps).reduce<
326327
Promise<Record<string, SnapRegistryResult>>
327328
>(async (previousPromise, [snapId, snapInfo]) => {
328-
const result = await this.#getSingle(snapId, snapInfo);
329+
const result = await this.#getSingle(snapId, snapInfo, skipRefetch);
329330
const acc = await previousPromise;
330331
acc[snapId] = result;
331332
return acc;

0 commit comments

Comments
 (0)