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

Added Permissions validation to SnapManifest #910

Merged
merged 12 commits into from
Nov 11, 2022
Merged
2 changes: 1 addition & 1 deletion packages/examples/examples/insights/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
},
"dependencies": {
"@metamask/abi-utils": "^1.0.0",
"@metamask/utils": "^3.3.0"
"@metamask/utils": "^3.3.1"
},
"devDependencies": {
"@lavamoat/allow-scripts": "^2.0.3",
Expand Down
2 changes: 1 addition & 1 deletion packages/examples/examples/insights/snap.manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"url": "https://github.com/MetaMask/snaps-monorepo.git"
},
"source": {
"shasum": "rWNQVlpSAlUXVByhpVyXxvNRTwEfabK4vS2MU6ANNwc=",
"shasum": "ZGhrs3HV9UT/d8A77+jBM4JtjojE1RGqCEk1HFg4xIk=",
"location": {
"npm": {
"filePath": "dist/bundle.js",
Expand Down
3 changes: 2 additions & 1 deletion packages/multichain-provider/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@
"dependencies": {
"@metamask/safe-event-emitter": "^2.0.0",
"@metamask/snaps-types": "^0.23.0",
"@metamask/utils": "^3.3.0",
"@metamask/snaps-utils": "^0.23.0",
"@metamask/utils": "^3.3.1",
"nanoid": "^3.1.31"
},
"devDependencies": {
Expand Down
41 changes: 41 additions & 0 deletions packages/multichain-provider/src/MultiChainProvider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,47 @@ describe('MultiChainProvider', () => {
expect(result).toBe('foo');
});

it('sends params properly', async () => {
Mrtenz marked this conversation as resolved.
Show resolved Hide resolved
const msg = (params?: any) => ({
method: 'wallet_multiChainRequestHack',
params: {
id: expect.any(String),
jsonrpc: '2.0',
method: 'caip_request',
params: {
chainId: 'eip155:1',
request: {
method: 'asd',
params,
},
},
},
});

const provider = await getProvider();
const request = ethereum.request as jest.MockedFunction<
typeof ethereum.request
>;

request.mockImplementation(async () => 'foo');

await provider.request({
chainId: 'eip155:1',
request: {
method: 'asd',
},
});

await provider.request({
chainId: 'eip155:1',
request: { method: 'asd', params: { prop: 'bar' } },
});

expect(request).toHaveBeenCalledTimes(3);
expect(request).toHaveBeenNthCalledWith(2, msg());
expect(request).toHaveBeenNthCalledWith(3, msg({ prop: 'bar' }));
});

it('generates a random request ID', async () => {
const provider = await getProvider();

Expand Down
14 changes: 10 additions & 4 deletions packages/multichain-provider/src/MultiChainProvider.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import SafeEventEmitter from '@metamask/safe-event-emitter';
import { nanoid } from 'nanoid';
import {
assertIsConnectArguments,
assertIsMetaMaskNotification,
Expand All @@ -12,8 +11,9 @@ import {
RequestNamespace,
Session,
} from '@metamask/snaps-utils';
import { JsonRpcRequest } from '@metamask/utils';
import { JsonRpcRequest, Json } from '@metamask/utils';
import type { SnapProvider } from '@metamask/snaps-types';
import { nanoid } from 'nanoid';
import { Provider } from './Provider';

declare global {
Expand Down Expand Up @@ -126,11 +126,17 @@ export class MultiChainProvider extends SafeEventEmitter implements Provider {

assertIsMultiChainRequest(args);

// We're doing it this way to avoid sentRequest.params = undefined.
const sentRequest: Json = { method: args.request.method };
if (args.request.params !== undefined) {
sentRequest.params = args.request.params;
}

return this.#rpcRequest({
method: 'caip_request',
params: {
chainId: args.chainId,
request: { method: args.request.method, params: args.request.params },
request: sentRequest,
},
});
}
Expand All @@ -152,7 +158,7 @@ export class MultiChainProvider extends SafeEventEmitter implements Provider {
*/
async #rpcRequest(
payload: { method: string } & Partial<
JsonRpcRequest<unknown[] | Record<string, unknown>>
JsonRpcRequest<Record<string, Json> | Json[] | undefined>
>,
) {
return await this.#getProvider().request({
Expand Down
8 changes: 4 additions & 4 deletions packages/rpc-methods/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ module.exports = deepmerge(baseConfig, {
coveragePathIgnorePatterns: ['./src/index.ts'],
coverageThreshold: {
global: {
branches: 85.82,
functions: 80,
lines: 60.24,
statements: 60.24,
branches: 82.85,
functions: 80.85,
lines: 59.77,
statements: 59.77,
},
},
testTimeout: 2500,
Expand Down
2 changes: 1 addition & 1 deletion packages/rpc-methods/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
"@metamask/key-tree": "^6.0.0",
"@metamask/snaps-utils": "^0.23.0",
"@metamask/types": "^1.1.0",
"@metamask/utils": "^3.3.0",
"@metamask/utils": "^3.3.1",
"eth-rpc-errors": "^4.0.2",
"nanoid": "^3.1.31",
"superstruct": "^0.16.7"
Expand Down
80 changes: 6 additions & 74 deletions packages/rpc-methods/src/restricted/getBip32Entropy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,81 +5,11 @@ import {
getBip32EntropyCaveatSpecifications,
getBip32EntropyImplementation,
validateCaveatPaths,
validatePath,
} from './getBip32Entropy';

const TEST_SECRET_RECOVERY_PHRASE =
'test test test test test test test test test test test ball';

describe('validatePath', () => {
it.each([true, false, null, undefined, 'foo', [], new (class {})()])(
'throws if the value is not a plain object',
(value) => {
expect(() => validatePath(value)).toThrow('Expected a plain object.');
},
);

it.each([{}, { path: [] }, { path: 'foo' }])(
'throws if the path is invalid or empty',
() => {
expect(() => validatePath({})).toThrow(
'Invalid "path" parameter. The path must be a non-empty BIP-32 derivation path array.',
);
},
);

it('throws if the path does not start with "m"', () => {
expect(() => validatePath({ path: ["44'", "60'"] })).toThrow(
'Invalid "path" parameter. The path must start with "m".',
);
});

it.each([
{ path: ['m', 'foo'] },
{ path: ['m', '0', 'bar'] },
{ path: ['m', 0] },
])('throws if the path is invalid', (value) => {
expect(() => validatePath(value)).toThrow(
'Invalid "path" parameter. The path must be a valid BIP-32 derivation path array.',
);
});

it.each([{ path: ['m'] }, { path: ['m', "44'"] }])(
'throws if the path has a length of less than three',
(value) => {
expect(() => validatePath(value)).toThrow(
'Invalid "path" parameter. Paths must have a length of at least three.',
);
},
);

it('throws if the curve is invalid', () => {
expect(() =>
validatePath({ path: ['m', "44'", "60'"], curve: 'foo' }),
).toThrow(
'Invalid "curve" parameter. The curve must be "secp256k1" or "ed25519".',
);
});

it('throws if the curve is ed25519 and the path has an unhardened index', () => {
expect(() =>
validatePath({ path: ['m', "44'", "60'", '1'], curve: 'ed25519' }),
).toThrow(
'Invalid "path" parameter. Ed25519 does not support unhardened paths.',
);
});

it('does not throw if the path is valid', () => {
expect(() =>
validatePath({ path: ['m', "44'", "60'"], curve: 'secp256k1' }),
).not.toThrow();

expect(() =>
validatePath({ path: ['m', "44'", "60'"], curve: 'ed25519' }),
).not.toThrow();
});
});

describe('validateCaveatPaths', () => {
it.each([[], null, undefined, 'foo'])(
'throws if the value is not an array or empty',
Expand All @@ -89,7 +19,9 @@ describe('validateCaveatPaths', () => {
type: SnapCaveatType.PermittedDerivationPaths,
value,
}),
).toThrow('Expected non-empty array of paths.');
).toThrow(
/^Invalid BIP-32 entropy caveat: At path: value -- Expected an? array/u,
); // Different error messages for different types
},
);

Expand All @@ -99,7 +31,7 @@ describe('validateCaveatPaths', () => {
type: SnapCaveatType.PermittedDerivationPaths,
value: [{ path: ['foo'], curve: 'secp256k1' }],
}),
).toThrow('Invalid "path" parameter. The path must start with "m".');
).toThrow('At path: value.0.path -- Path must start with "m".');
});
});

Expand Down Expand Up @@ -245,7 +177,7 @@ describe('getBip32EntropyCaveatSpecifications', () => {
// @ts-expect-error Missing other required properties.
})({ params: { ...params, path: [] } }),
).rejects.toThrow(
'Invalid "path" parameter. The path must be a non-empty BIP-32 derivation path array.',
'At path: path -- Path must be a non-empty BIP-32 derivation path array',
);
});

Expand Down Expand Up @@ -275,7 +207,7 @@ describe('getBip32EntropyCaveatSpecifications', () => {
type: SnapCaveatType.PermittedDerivationPaths,
value: [{ path: ['foo'], curve: 'secp256k1' }],
}),
).toThrow('Invalid "path" parameter. The path must start with "m".');
).toThrow('At path: value.0.path -- Path must start with "m".');
});
});
});
Expand Down