Skip to content
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
21 changes: 21 additions & 0 deletions packages/snaps-controllers/src/snaps/SnapController.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2192,6 +2192,27 @@ describe('SnapController', () => {
});

describe('handleRequest', () => {
it('throws if the Snap is not installed', async () => {
const snapController = getSnapController();

await expect(
snapController.handleRequest({
snapId: 'npm:foo' as SnapId,
origin: METAMASK_ORIGIN,
handler: HandlerType.OnUserInput,
request: {
jsonrpc: '2.0',
method: 'test',
params: { id: MOCK_INTERFACE_ID },
},
}),
).rejects.toThrow(
'The Snap "npm:foo" is not installed. Please install it before invoking it.',
);

snapController.destroy();
});

it.each(
Object.keys(handlerEndowments).filter(
(handler) => handlerEndowments[handler as HandlerType],
Expand Down
11 changes: 9 additions & 2 deletions packages/snaps-controllers/src/snaps/SnapController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3588,6 +3588,13 @@ export class SnapController extends BaseController<
}: SnapRpcHookArgs & { snapId: SnapId }): Promise<unknown> {
this.#assertCanUsePlatform();

const snap = this.get(snapId);

assert(
snap,
`The Snap "${snapId}" is not installed. Please install it before invoking it.`,
);

assert(
origin === METAMASK_ORIGIN || isValidUrl(origin),
"'origin' must be a valid URL or 'metamask'.",
Expand Down Expand Up @@ -3666,11 +3673,11 @@ export class SnapController extends BaseController<
throw new Error(`"${handlerType}" can only be invoked by MetaMask.`);
}

if (!this.state.snaps[snapId].enabled) {
if (!snap.enabled) {
throw new Error(`Snap "${snapId}" is disabled.`);
}

if (this.state.snaps[snapId].status === SnapStatus.Installing) {
if (snap.status === SnapStatus.Installing) {
throw new Error(
`Snap "${snapId}" is currently being installed. Please try again later.`,
);
Expand Down
4 changes: 2 additions & 2 deletions packages/snaps-rpc-methods/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ module.exports = deepmerge(baseConfig, {
],
coverageThreshold: {
global: {
branches: 95.7,
branches: 95.68,
functions: 98.75,
lines: 98.99,
lines: 98.98,
statements: 98.69,
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ describe('wallet_invokeKeyring', () => {
expect(response.error).toStrictEqual({
...rpcErrors
.invalidRequest({
message: `The snap "${MOCK_SNAP_ID}" is not installed. Please install it first, before invoking the snap.`,
message: `The Snap "${MOCK_SNAP_ID}" is not installed. Please install it before invoking it.`,
})
.serialize(),
stack: expect.any(String),
Expand Down
3 changes: 2 additions & 1 deletion packages/snaps-rpc-methods/src/permitted/invokeKeyring.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,9 @@ async function invokeKeyringImplementation(

if (!getSnap(snapId)) {
return end(
// Mirror error message from SnapController.
rpcErrors.invalidRequest({
message: `The snap "${snapId}" is not installed. Please install it first, before invoking the snap.`,
message: `The Snap "${snapId}" is not installed. Please install it before invoking it.`,
}),
);
}
Expand Down
27 changes: 0 additions & 27 deletions packages/snaps-rpc-methods/src/restricted/invokeSnap.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ describe('builder', () => {
targetName: WALLET_SNAP_PERMISSION_KEY,
specificationBuilder: expect.any(Function),
methodHooks: {
getSnap: true,
handleSnapRpcRequest: true,
},
});
Expand All @@ -33,7 +32,6 @@ describe('builder', () => {
expect(
invokeSnapBuilder.specificationBuilder({
methodHooks: {
getSnap: jest.fn(),
handleSnapRpcRequest: jest.fn(),
},
}),
Expand All @@ -53,7 +51,6 @@ describe('builder', () => {
describe('specificationBuilder', () => {
const specification = invokeSnapBuilder.specificationBuilder({
methodHooks: {
getSnap: jest.fn(),
handleSnapRpcRequest: jest.fn(),
},
});
Expand Down Expand Up @@ -90,7 +87,6 @@ describe('implementation', () => {
}) as any;
it('calls handleSnapRpcRequest', async () => {
const hooks = getMockHooks();
hooks.getSnap.mockImplementation(getTruncatedSnap);
const implementation = getInvokeSnapImplementation(hooks);
await implementation({
context: { origin: MOCK_ORIGIN },
Expand All @@ -101,8 +97,6 @@ describe('implementation', () => {
},
});

expect(hooks.getSnap).toHaveBeenCalledTimes(1);
expect(hooks.getSnap).toHaveBeenCalledWith(MOCK_SNAP_ID);
expect(hooks.handleSnapRpcRequest).toHaveBeenCalledWith({
handler: 'onRpcRequest',
origin: MOCK_ORIGIN,
Expand All @@ -113,27 +107,6 @@ describe('implementation', () => {
snapId: MOCK_SNAP_ID,
});
});

it('throws if snap is not installed', async () => {
const hooks = getMockHooks();
const implementation = getInvokeSnapImplementation(hooks);
await expect(
implementation({
context: { origin: MOCK_ORIGIN },
method: WALLET_SNAP_PERMISSION_KEY,
params: {
snapId: MOCK_SNAP_ID,
request: { method: 'hello', params: {} },
},
}),
).rejects.toThrow(
`The snap "${MOCK_SNAP_ID}" is not installed. Please install it first, before invoking the snap.`,
);
expect(hooks.getSnap).toHaveBeenCalledTimes(1);
expect(hooks.getSnap).toHaveBeenCalledWith(MOCK_SNAP_ID);

expect(hooks.handleSnapRpcRequest).not.toHaveBeenCalled();
});
});

describe('handleSnapInstall', () => {
Expand Down
12 changes: 1 addition & 11 deletions packages/snaps-rpc-methods/src/restricted/invokeSnap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import type {
RequestSnapsParams,
RequestSnapsResult,
} from '@metamask/snaps-sdk';
import type { Snap, SnapRpcHookArgs } from '@metamask/snaps-utils';
import type { SnapRpcHookArgs } from '@metamask/snaps-utils';
import { HandlerType, SnapCaveatType } from '@metamask/snaps-utils';
import type { Json, NonEmptyArray } from '@metamask/utils';

Expand All @@ -37,7 +37,6 @@ export type GetPermittedSnaps = {
type AllowedActions = InstallSnaps | GetPermittedSnaps;

export type InvokeSnapMethodHooks = {
getSnap: (snapId: string) => Snap | undefined;
handleSnapRpcRequest: ({
snapId,
origin,
Expand Down Expand Up @@ -139,7 +138,6 @@ const specificationBuilder: PermissionSpecificationBuilder<
};

const methodHooks: MethodHooksObject<InvokeSnapMethodHooks> = {
getSnap: true,
handleSnapRpcRequest: true,
};

Expand All @@ -153,13 +151,11 @@ export const invokeSnapBuilder = Object.freeze({
* Builds the method implementation for `wallet_snap_*`.
*
* @param hooks - The RPC method hooks.
* @param hooks.getSnap - A function that retrieves all information stored about a snap.
* @param hooks.handleSnapRpcRequest - A function that sends an RPC request to a snap's RPC handler or throws if that fails.
* @returns The method implementation which returns the result of `handleSnapRpcRequest`.
* @throws If the params are invalid.
*/
export function getInvokeSnapImplementation({
getSnap,
handleSnapRpcRequest,
}: InvokeSnapMethodHooks) {
return async function invokeSnap(
Expand All @@ -169,12 +165,6 @@ export function getInvokeSnapImplementation({

const { snapId, request } = params as InvokeSnapParams;

if (!getSnap(snapId)) {
throw rpcErrors.invalidRequest({
message: `The snap "${snapId}" is not installed. Please install it first, before invoking the snap.`,
});
}

const { origin } = context;

return (await handleSnapRpcRequest({
Expand Down
Loading