Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add platform version field to manifest #2803

Merged
merged 11 commits into from
Oct 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions packages/snaps-cli/src/commands/build/implementation.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { getPlatformVersion } from '@metamask/snaps-utils';
import {
DEFAULT_SNAP_BUNDLE,
DEFAULT_SNAP_ICON,
getMockSnapFilesWithUpdatedChecksum,
getPackageJson,
getSnapManifest,
} from '@metamask/snaps-utils/test-utils';
Expand Down Expand Up @@ -61,15 +64,21 @@ jest.mock('../../webpack/utils', () => ({

describe('build', () => {
beforeEach(async () => {
const { manifest } = await getMockSnapFilesWithUpdatedChecksum({
manifest: getSnapManifest({
platformVersion: getPlatformVersion(),
}),
});

await fs.mkdir('/snap');
await fs.writeFile('/snap/input.js', DEFAULT_SNAP_BUNDLE);
await fs.writeFile(
'/snap/snap.manifest.json',
JSON.stringify(getSnapManifest()),
JSON.stringify(manifest.result),
);
await fs.writeFile('/snap/package.json', JSON.stringify(getPackageJson()));
await fs.mkdir('/snap/images');
await fs.writeFile('/snap/images/icon.svg', '<svg></svg>');
await fs.writeFile('/snap/images/icon.svg', DEFAULT_SNAP_ICON);
await fs.mkdir(dirname(BROWSERSLIST_FILE), { recursive: true });
await fs.writeFile(
BROWSERSLIST_FILE,
Expand Down
26 changes: 17 additions & 9 deletions packages/snaps-cli/src/commands/manifest/implementation.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { getPlatformVersion } from '@metamask/snaps-utils';
import {
DEFAULT_SNAP_BUNDLE,
DEFAULT_SNAP_ICON,
getMockSnapFilesWithUpdatedChecksum,
getPackageJson,
getSnapManifest,
} from '@metamask/snaps-utils/test-utils';
Expand Down Expand Up @@ -32,29 +35,33 @@ jest.mock('../../webpack', () => ({

describe('manifest', () => {
beforeEach(async () => {
const { manifest: newManifest } = await getMockSnapFilesWithUpdatedChecksum(
{
manifest: getSnapManifest({
platformVersion: getPlatformVersion(),
}),
},
);

await fs.mkdir('/snap/dist', { recursive: true });
await fs.writeFile('/snap/dist/bundle.js', DEFAULT_SNAP_BUNDLE);
await fs.writeFile(
'/snap/snap.manifest.json',
JSON.stringify(
getSnapManifest({
shasum: 'G/W5b2JZVv+epgNX9pkN63X6Lye9EJVJ4NLSgAw/afc=',
}),
),
JSON.stringify(newManifest.result),
);
await fs.writeFile('/snap/package.json', JSON.stringify(getPackageJson()));
await fs.mkdir('/snap/images');
await fs.writeFile('/snap/images/icon.svg', '<svg></svg>');
await fs.writeFile('/snap/images/icon.svg', DEFAULT_SNAP_ICON);
});

afterEach(async () => {
await fs.rm('/snap', { force: true, recursive: true });
});

it('validates a snap manifest', async () => {
const error = jest.spyOn(console, 'error').mockImplementation();
const error = jest.spyOn(console, 'error');
const warn = jest.spyOn(console, 'warn').mockImplementation();
const log = jest.spyOn(console, 'log').mockImplementation();
const log = jest.spyOn(console, 'log');

const spinner = ora();
const result = await manifest('/snap/snap.manifest.json', false, spinner);
Expand Down Expand Up @@ -157,7 +164,7 @@ describe('manifest', () => {
"url": "https://github.com/MetaMask/example-snap.git"
},
"source": {
"shasum": "d4W7f1lzpVGMj8jjCn1lYhhHmKc/9TSk5QLH5ldKQoI=",
"shasum": "itjh0enng42nO6BxNCEhDH8wm3yl4xlVclfd5LsZ2wA=",
"location": {
"npm": {
"filePath": "dist/bundle.js",
Expand All @@ -172,6 +179,7 @@ describe('manifest', () => {
"chains": ["eip155:1", "eip155:2", "eip155:3"]
}
},
"platformVersion": "1.0.0",
FrederikBolding marked this conversation as resolved.
Show resolved Hide resolved
"manifestVersion": "0.1"
}
"
Expand Down
6 changes: 3 additions & 3 deletions packages/snaps-controllers/coverage.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"branches": 92.63,
"branches": 92.67,
"functions": 96.65,
"lines": 97.97,
"statements": 97.67
"lines": 97.98,
"statements": 97.68
}
2 changes: 2 additions & 0 deletions packages/snaps-controllers/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@
"nanoid": "^3.1.31",
"readable-stream": "^3.6.2",
"readable-web-to-node-stream": "^3.0.2",
"semver": "^7.5.4",
"tar-stream": "^3.1.7"
},
"devDependencies": {
Expand All @@ -125,6 +126,7 @@
"@types/mocha": "^10.0.1",
"@types/node": "18.14.2",
"@types/readable-stream": "^4.0.15",
"@types/semver": "^7.5.0",
"@types/tar-stream": "^3.1.1",
"@typescript-eslint/eslint-plugin": "^5.42.1",
"@typescript-eslint/parser": "^6.21.0",
Expand Down
104 changes: 104 additions & 0 deletions packages/snaps-controllers/src/snaps/SnapController.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import { Text } from '@metamask/snaps-sdk/jsx';
import type { SnapPermissions, RpcOrigins } from '@metamask/snaps-utils';
import {
getPlatformVersion,
DEFAULT_ENDOWMENTS,
DEFAULT_REQUESTED_SNAP_VERSION,
getLocalizedSnapManifest,
Expand Down Expand Up @@ -64,6 +65,7 @@
import fetchMock from 'jest-fetch-mock';
import { pipeline } from 'readable-stream';
import type { Duplex } from 'readable-stream';
import semver from 'semver';

import { setupMultiplex } from '../services';
import type { NodeThreadExecutionService } from '../services/node';
Expand Down Expand Up @@ -1848,7 +1850,7 @@
});

// This isn't stable in CI unfortunately
it.skip('throws if the Snap is terminated while executing', async () => {

Check warning on line 1853 in packages/snaps-controllers/src/snaps/SnapController.test.tsx

View workflow job for this annotation

GitHub Actions / Build, lint, and test / Lint (@metamask/snaps-controllers)

Disabled test
const { manifest, sourceCode, svgIcon } =
await getMockSnapFilesWithUpdatedChecksum({
sourceCode: `
Expand Down Expand Up @@ -5360,6 +5362,108 @@
controller.destroy();
});

it('does not throw an error if the manifest does not specify a platform version', async () => {
const rawManifest = getSnapManifest();
delete rawManifest.platformVersion;

const { manifest } = await getMockSnapFilesWithUpdatedChecksum({
manifest: rawManifest,
});

const messenger = getSnapControllerMessenger();
const controller = getSnapController(
getSnapControllerOptions({
messenger,
detectSnapLocation: loopbackDetect({
manifest: manifest.result,
}),
}),
);

await expect(
controller.installSnaps(MOCK_ORIGIN, {
[MOCK_SNAP_ID]: {},
}),
// eslint-disable-next-line jest/no-restricted-matchers
).resolves.not.toThrow();

controller.destroy();
});

it('throws an error if the specified platform version is newer than the supported platform version', async () => {
const newerVersion = semver.inc(
getPlatformVersion(),
'minor',
) as SemVerVersion;

const { manifest } = await getMockSnapFilesWithUpdatedChecksum({
manifest: getSnapManifest({
platformVersion: newerVersion,
}),
});

const messenger = getSnapControllerMessenger();
const controller = getSnapController(
getSnapControllerOptions({
messenger,
detectSnapLocation: loopbackDetect({
manifest: manifest.result,
}),
}),
);

await expect(
controller.installSnaps(MOCK_ORIGIN, {
[MOCK_SNAP_ID]: {},
}),
).rejects.toThrow(
`The Snap "${MOCK_SNAP_ID}" requires platform version "${newerVersion}" which is greater than the current platform version "${getPlatformVersion()}".`,
);

controller.destroy();
});

it('logs a warning if the specified platform version is newer than the supported platform version and `rejectInvalidPlatformVersion` is disabled', async () => {
const log = jest.spyOn(console, 'warn').mockImplementation();

const newerVersion = semver.inc(
getPlatformVersion(),
'minor',
) as SemVerVersion;

const { manifest } = await getMockSnapFilesWithUpdatedChecksum({
manifest: getSnapManifest({
platformVersion: newerVersion,
}),
});

const messenger = getSnapControllerMessenger();
const controller = getSnapController(
getSnapControllerOptions({
messenger,
featureFlags: {
rejectInvalidPlatformVersion: false,
},
detectSnapLocation: loopbackDetect({
manifest: manifest.result,
}),
}),
);

await expect(
controller.installSnaps(MOCK_ORIGIN, {
[MOCK_SNAP_ID]: {},
}),
// eslint-disable-next-line jest/no-restricted-matchers
).resolves.not.toThrow();

expect(log).toHaveBeenCalledWith(
`The Snap "${MOCK_SNAP_ID}" requires platform version "${newerVersion}" which is greater than the current platform version "${getPlatformVersion()}".`,
);

controller.destroy();
});

it('maps permission caveats to the proper format', async () => {
const initialPermissions = {
// eslint-disable-next-line @typescript-eslint/naming-convention
Expand Down
45 changes: 44 additions & 1 deletion packages/snaps-controllers/src/snaps/SnapController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ import type {
TruncatedSnapFields,
} from '@metamask/snaps-utils';
import {
logWarning,
getPlatformVersion,
assertIsSnapManifest,
assertIsValidSnapId,
DEFAULT_ENDOWMENTS,
Expand Down Expand Up @@ -108,6 +110,7 @@ import type { StateMachine } from '@xstate/fsm';
import { createMachine, interpret } from '@xstate/fsm';
import type { Patch } from 'immer';
import { nanoid } from 'nanoid';
import semver from 'semver';

import { forceStrict, validateMachine } from '../fsm';
import type { CreateInterface, GetInterface } from '../interface';
Expand Down Expand Up @@ -601,6 +604,7 @@ type FeatureFlags = {
requireAllowlist?: boolean;
allowLocalSnaps?: boolean;
disableSnapInstallation?: boolean;
rejectInvalidPlatformVersion?: boolean;
};

type DynamicFeatureFlags = {
Expand Down Expand Up @@ -1332,11 +1336,18 @@ export class SnapController extends BaseController<

async #assertIsInstallAllowed(
snapId: SnapId,
snapInfo: SnapsRegistryInfo & { permissions: SnapPermissions },
{
platformVersion,
...snapInfo
}: SnapsRegistryInfo & {
permissions: SnapPermissions;
platformVersion: string | undefined;
},
) {
const results = await this.messagingSystem.call('SnapsRegistry:get', {
[snapId]: snapInfo,
});

const result = results[snapId];
if (result.status === SnapsRegistryStatus.Blocked) {
throw new Error(
Expand Down Expand Up @@ -1365,6 +1376,8 @@ export class SnapController extends BaseController<
}`,
);
}

this.#validatePlatformVersion(snapId, platformVersion);
}

/**
Expand Down Expand Up @@ -2554,6 +2567,7 @@ export class SnapController extends BaseController<
version: newVersion,
checksum: manifest.source.shasum,
permissions: manifest.initialPermissions,
platformVersion: manifest.platformVersion,
});

const processedPermissions = processSnapPermissions(
Expand Down Expand Up @@ -2739,6 +2753,7 @@ export class SnapController extends BaseController<
version: manifest.version,
checksum: manifest.source.shasum,
permissions: manifest.initialPermissions,
platformVersion: manifest.platformVersion,
});

return this.#set({
Expand Down Expand Up @@ -3010,6 +3025,34 @@ export class SnapController extends BaseController<
);
}

/**
* Validate that the platform version specified in the manifest (if any) is
* compatible with the current platform version.
*
* @param snapId - The ID of the Snap.
* @param platformVersion - The platform version to validate against.
* @throws If the platform version is greater than the current platform
* version.
*/
#validatePlatformVersion(
snapId: SnapId,
platformVersion: string | undefined,
) {
if (platformVersion === undefined) {
return;
}

if (semver.gt(platformVersion, getPlatformVersion())) {
const message = `The Snap "${snapId}" requires platform version "${platformVersion}" which is greater than the current platform version "${getPlatformVersion()}".`;

if (this.#featureFlags.rejectInvalidPlatformVersion) {
throw new Error(message);
}

logWarning(message);
}
}

/**
* Initiates a request for the given snap's initial permissions.
* Must be called in order. See processRequestedSnap.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ const MOCK_DATABASE: SnapsRegistryDatabase = {
// 3. Run the `sign-registry` script.
// 4. Copy the signature from the `signature.json` file.
const MOCK_SIGNATURE =
'0x304402201bfe1a98837631b669643135766de58deb426dc3eeb0a908c8000f85a047db3102207ac621072ea59737287099ac830323b34e59bfc41fb62119b16ce24d0c433f9e';
'0x3045022100fd130773d66931560f199e783c48cf7d8c28d73ea8366add5b64ebcf61f98eca02206f6c56070d5d5899a50fea68add84570d5171c6fae812d4c3a89d5ccdcf396b2';
Mrtenz marked this conversation as resolved.
Show resolved Hide resolved
const MOCK_SIGNATURE_FILE = {
signature: MOCK_SIGNATURE,
curve: 'secp256k1',
Expand Down
5 changes: 4 additions & 1 deletion packages/snaps-controllers/src/test-utils/controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,10 @@ export const getSnapControllerOptions = (
environmentEndowmentPermissions: [],
closeAllConnections: jest.fn(),
messenger: getSnapControllerMessenger(),
featureFlags: { dappsCanUpdateSnaps: true },
featureFlags: {
dappsCanUpdateSnaps: true,
rejectInvalidPlatformVersion: true,
},
state: undefined,
fetchFunction: jest.fn(),
getMnemonic: async () => Promise.resolve(TEST_SECRET_RECOVERY_PHRASE_BYTES),
Expand Down
4 changes: 2 additions & 2 deletions packages/snaps-utils/coverage.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"branches": 99.74,
"functions": 98.92,
"functions": 98.93,
"lines": 99.46,
"statements": 96.32
"statements": 96.36
}
1 change: 1 addition & 0 deletions packages/snaps-utils/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export * from './logging';
export * from './manifest';
export * from './namespace';
export * from './path';
export * from './platform-version';
export * from './snaps';
export * from './strings';
export * from './structs';
Expand Down
Loading
Loading