Skip to content

Commit

Permalink
Handle unavailable registry more gracefully
Browse files Browse the repository at this point in the history
  • Loading branch information
@FrederikBolding
FrederikBolding committed Mar 8, 2024
1 parent a0bdc87 commit 702fee1
Show file tree
Hide file tree
Showing 6 changed files with 116 additions and 58 deletions.
4 changes: 2 additions & 2 deletions packages/snaps-controllers/coverage.json
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"branches": 91.05,
"functions": 96.56,
"branches": 90.82,
"functions": 96.57,
"lines": 97.8,
"statements": 97.46
}
32 changes: 32 additions & 0 deletions packages/snaps-controllers/src/snaps/SnapController.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -775,6 +775,38 @@ describe('SnapController', () => {
controller.destroy();
});

it('throws an error if the registry is unavailable and allowlisting is required but resolve succeeds', async () => {
const registry = new MockSnapsRegistry();
const rootMessenger = getControllerMessenger(registry);
const messenger = getSnapControllerMessenger(rootMessenger);
const controller = getSnapController(
getSnapControllerOptions({
featureFlags: { requireAllowlist: true },
messenger,
detectSnapLocation: (_location, options) =>
new LoopbackLocation(options),
}),
);

// Mock resolve to succeed, but registry.get() will fail later
registry.resolveVersion.mockReturnValue('1.0.0');
registry.get.mockReturnValue({
[MOCK_SNAP_ID]: { status: SnapsRegistryStatus.Unavailable },
});

await expect(
controller.installSnaps(MOCK_ORIGIN, {
[MOCK_SNAP_ID]: { version: DEFAULT_REQUESTED_SNAP_VERSION },
}),
).rejects.toThrow(
'Cannot install version "1.0.0" of snap "npm:@metamask/example-snap": The registry is temporarily unavailable.',
);

expect(registry.resolveVersion).toHaveBeenCalled();

controller.destroy();
});

it('throws an error if snap is not on allowlist and allowlisting is required', async () => {
const { manifest, sourceCode, svgIcon } =
await getMockSnapFilesWithUpdatedChecksum({
Expand Down
6 changes: 5 additions & 1 deletion packages/snaps-controllers/src/snaps/SnapController.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1234,7 +1234,11 @@ export class SnapController extends BaseController<
result.status !== SnapsRegistryStatus.Verified
) {
throw new Error(
`Cannot install version "${snapInfo.version}" of snap "${snapId}": The snap is not on the allowlist.`,
`Cannot install version "${snapInfo.version}" of snap "${snapId}": ${
result.status === SnapsRegistryStatus.Unavailable
? 'The registry is temporarily unavailable.'
: 'The snap is not on the allowlist.'
}`,
);
}
}
Expand Down
90 changes: 55 additions & 35 deletions packages/snaps-controllers/src/snaps/registry/json.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,11 +242,15 @@ describe('JsonSnapsRegistry', () => {
});
});

it('returns unverified for unavailable database if failOnUnavailableRegistry is set to false', async () => {
it('uses existing state if registry is unavailable', async () => {
fetchMock.mockResponse('', { status: 404 });

const { messenger } = getRegistry({
failOnUnavailableRegistry: false,
refetchOnAllowlistMiss: true,
state: {
lastUpdated: 0,
database: MOCK_DATABASE,
},
});

const result = await messenger.call('SnapsRegistry:get', {
Expand All @@ -258,48 +262,56 @@ describe('JsonSnapsRegistry', () => {

expect(result).toStrictEqual({
[MOCK_SNAP_ID]: {
status: SnapsRegistryStatus.Unverified,
status: SnapsRegistryStatus.Verified,
},
});
});

it('does not verify the signature if no public key is provided', async () => {
fetchMock.mockResponse(JSON.stringify(MOCK_DATABASE));
it('doesnt use existing state if existing state isnt sufficient', async () => {
fetchMock.mockResponse('', { status: 404 });

const { messenger } = getRegistry({
publicKey: undefined,
refetchOnAllowlistMiss: true,
state: {
lastUpdated: 0,
database: MOCK_DATABASE,
},
});

const result = await messenger.call('SnapsRegistry:get', {
[MOCK_SNAP_ID]: {
version: '1.0.0' as SemVerVersion,
version: '1.0.1' as SemVerVersion,
checksum: DEFAULT_SNAP_SHASUM,
},
});

expect(result).toStrictEqual({
[MOCK_SNAP_ID]: {
status: SnapsRegistryStatus.Verified,
status: SnapsRegistryStatus.Unavailable,
},
});
});

it('throws for unavailable database by default', async () => {
it('returns unavailable if the database is unavailable', async () => {
fetchMock.mockResponse('', { status: 404 });

const { messenger } = getRegistry();

await expect(
messenger.call('SnapsRegistry:get', {
[MOCK_SNAP_ID]: {
version: '1.0.0' as SemVerVersion,
checksum: DEFAULT_SNAP_SHASUM,
},
}),
).rejects.toThrow('Snaps registry is unavailable, installation blocked.');
const result = await messenger.call('SnapsRegistry:get', {
[MOCK_SNAP_ID]: {
version: '1.0.0' as SemVerVersion,
checksum: DEFAULT_SNAP_SHASUM,
},
});

expect(result).toStrictEqual({
[MOCK_SNAP_ID]: {
status: SnapsRegistryStatus.Unavailable,
},
});
});

it('throws for unavailable signature', async () => {
it('returns unavailable if signature is unavailable', async () => {
fetchMock
.mockResponseOnce(JSON.stringify(MOCK_DATABASE))
.mockResponseOnce('', {
Expand All @@ -308,17 +320,21 @@ describe('JsonSnapsRegistry', () => {

const { messenger } = getRegistry();

await expect(
messenger.call('SnapsRegistry:get', {
[MOCK_SNAP_ID]: {
version: '1.0.0' as SemVerVersion,
checksum: DEFAULT_SNAP_SHASUM,
},
}),
).rejects.toThrow('Snaps registry is unavailable, installation blocked.');
const result = await messenger.call('SnapsRegistry:get', {
[MOCK_SNAP_ID]: {
version: '1.0.0' as SemVerVersion,
checksum: DEFAULT_SNAP_SHASUM,
},
});

expect(result).toStrictEqual({
[MOCK_SNAP_ID]: {
status: SnapsRegistryStatus.Unavailable,
},
});
});

it('throws for invalid signature', async () => {
it('returns unavailable if signature is invalid', async () => {
fetchMock
.mockResponseOnce(JSON.stringify(MOCK_DATABASE))
.mockResponseOnce(JSON.stringify(MOCK_SIGNATURE_FILE));
Expand All @@ -328,14 +344,18 @@ describe('JsonSnapsRegistry', () => {
'0x034ca27b046507d1a9997bddc991b56d96b93d4adac3a96dfe01ce450bfb661455',
});

await expect(
messenger.call('SnapsRegistry:get', {
[MOCK_SNAP_ID]: {
version: '1.0.0' as SemVerVersion,
checksum: DEFAULT_SNAP_SHASUM,
},
}),
).rejects.toThrow('Snaps registry is unavailable, installation blocked.');
const result = await messenger.call('SnapsRegistry:get', {
[MOCK_SNAP_ID]: {
version: '1.0.0' as SemVerVersion,
checksum: DEFAULT_SNAP_SHASUM,
},
});

expect(result).toStrictEqual({
[MOCK_SNAP_ID]: {
status: SnapsRegistryStatus.Unavailable,
},
});
});

describe('resolveVersion', () => {
Expand Down
40 changes: 21 additions & 19 deletions packages/snaps-controllers/src/snaps/registry/json.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,14 @@ import type {
} from './registry';
import { SnapsRegistryStatus } from './registry';

// TODO: Replace with a Codefi URL
const SNAP_REGISTRY_URL =
'https://cdn.jsdelivr.net/gh/MetaMask/snaps-registry@gh-pages/latest/registry.json';
'https://acl.execution.metamask.io/latest/registry.json';

const SNAP_REGISTRY_SIGNATURE_URL =
'https://cdn.jsdelivr.net/gh/MetaMask/snaps-registry@gh-pages/latest/signature.json';
'https://acl.execution.metamask.io/latest/signature.json';

const DEFAULT_PUBLIC_KEY =
'0x025b65308f0f0fb8bc7f7ff87bfc296e0330eee5d3c1d1ee4a048b2fd6a86fa0a6';

type JsonSnapsRegistryUrl = {
registry: string;
Expand All @@ -40,7 +42,6 @@ export type JsonSnapsRegistryArgs = {
url?: JsonSnapsRegistryUrl;
recentFetchThreshold?: number;
refetchOnAllowlistMiss?: boolean;
failOnUnavailableRegistry?: boolean;
publicKey?: Hex;
};

Expand Down Expand Up @@ -83,13 +84,15 @@ export type SnapsRegistryMessenger = RestrictedControllerMessenger<
export type SnapsRegistryState = {
database: SnapsRegistryDatabase | null;
lastUpdated: number | null;
databaseUnavailable: boolean;
};

const controllerName = 'SnapsRegistry';

const defaultState = {
database: null,
lastUpdated: null,
databaseUnavailable: false,
};

export class JsonSnapsRegistry extends BaseController<
Expand All @@ -99,16 +102,14 @@ export class JsonSnapsRegistry extends BaseController<
> {
#url: JsonSnapsRegistryUrl;

#publicKey?: Hex;
#publicKey: Hex;

#fetchFunction: typeof fetch;

#recentFetchThreshold: number;

#refetchOnAllowlistMiss: boolean;

#failOnUnavailableRegistry: boolean;

#currentUpdate: Promise<void> | null;

constructor({
Expand All @@ -118,17 +119,17 @@ export class JsonSnapsRegistry extends BaseController<
registry: SNAP_REGISTRY_URL,
signature: SNAP_REGISTRY_SIGNATURE_URL,
},
publicKey,
publicKey = DEFAULT_PUBLIC_KEY,

Check warning on line 122 in packages/snaps-controllers/src/snaps/registry/json.ts

View check run for this annotation

Codecov / codecov/patch

packages/snaps-controllers/src/snaps/registry/json.ts#L122 

Added line #L122 was not covered by tests
fetchFunction = globalThis.fetch.bind(globalThis),
recentFetchThreshold = inMilliseconds(5, Duration.Minute),
failOnUnavailableRegistry = true,
refetchOnAllowlistMiss = true,
}: JsonSnapsRegistryArgs) {
super({
messenger,
metadata: {
database: { persist: true, anonymous: false },
lastUpdated: { persist: true, anonymous: false },
databaseUnavailable: { persist: true, anonymous: false },
},
name: controllerName,
state: {
Expand All @@ -141,7 +142,6 @@ export class JsonSnapsRegistry extends BaseController<
this.#fetchFunction = fetchFunction;
this.#recentFetchThreshold = recentFetchThreshold;
this.#refetchOnAllowlistMiss = refetchOnAllowlistMiss;
this.#failOnUnavailableRegistry = failOnUnavailableRegistry;
this.#currentUpdate = null;

this.messagingSystem.registerActionHandler(
Expand Down Expand Up @@ -205,17 +205,19 @@ export class JsonSnapsRegistry extends BaseController<
try {
const database = await this.#safeFetch(this.#url.registry);

if (this.#publicKey) {
const signature = await this.#safeFetch(this.#url.signature);
this.#verifySignature(database, signature);
}
const signature = await this.#safeFetch(this.#url.signature);
this.#verifySignature(database, signature);

this.update((state) => {
state.database = JSON.parse(database);
state.lastUpdated = Date.now();
state.databaseUnavailable = false;
});
} catch {
// Ignore
this.update((state) => {
state.databaseUnavailable = true;
});
}
}

Expand All @@ -224,10 +226,6 @@ export class JsonSnapsRegistry extends BaseController<
await this.#triggerUpdate();
}

// If the database is still null and we require it, throw.
if (this.#failOnUnavailableRegistry && this.state.database === null) {
throw new Error('Snaps registry is unavailable, installation blocked.');
}
return this.state.database;
}

Expand Down Expand Up @@ -266,7 +264,11 @@ export class JsonSnapsRegistry extends BaseController<
await this.#triggerUpdate();
return this.#getSingle(snapId, snapInfo, true);
}
return { status: SnapsRegistryStatus.Unverified };
return {
status: this.state.databaseUnavailable
? SnapsRegistryStatus.Unavailable
: SnapsRegistryStatus.Unverified,
};
}

async #get(
Expand Down
2 changes: 1 addition & 1 deletion packages/snaps-controllers/src/snaps/registry/registry.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ export type SnapsRegistryRequest = Record<SnapId, SnapsRegistryInfo>;
export type SnapsRegistryMetadata =
SnapsRegistryDatabase['verifiedSnaps'][SnapId]['metadata'];

// TODO: Decide on names for these
export enum SnapsRegistryStatus {
Unverified = 0,
Blocked = 1,
Verified = 2,
Unavailable = 3,
}

export type SnapsRegistryResult = {
Expand Down

0 comments on commit 702fee1

Please sign in to comment.