Skip to content

Commit 24b87d3

Browse files
feat: Allow external images (#3769)
Allow `Image` component to contain external URLs as well as SVGs. This makes it easier for Snaps to show external images, however it is required that the Snap has permission to use the network in order to use this. Additionally allow specifying `height` and `width` for images. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Enable https external images in `Image` JSX with optional width/height, enforcing `endowment:network-access`; add URI struct and update validation, controllers, policies, examples, and tests. > > - **SDK (JSX/UI/Internals)**: > - `Image` now accepts external `https` URLs and optional `width`/`height`; keeps SVG support. > - Add `uri` struct and `UriOptions`; export via `@metamask/snaps-sdk`. > - Validate external images via URI and require network permission. > - Deprecate `getImageComponent` in favor of `<Image src="..." />`. > - **Controllers/Utils**: > - `SnapInterfaceController` validation now checks `PermissionController:hasPermission`; blocks external images without `endowment:network-access`. > - `validateJsxElements` updated to verify external image URLs and permission. > - Consolidate URL validation by reusing SDK `uri` in `snaps-utils` (removed duplicate implementation). > - **Execution Environments (LavaMoat)**: > - Allow `URL` global for `@metamask/snaps-sdk` policies. > - **Examples**: > - Images example snap switched to JSX (`index.tsx`), uses `<Image>` with external URL; manifest shasum updated. > - **Tests/Config**: > - Extensive tests added/updated for dialogs, SVG/PNG, external image permission checks. > - Jest coverage config excludes `test-utils/**/*.tsx`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 394d7ed. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Maarten Zuidhoorn <[email protected]>
1 parent 1e95bcd commit 24b87d3

File tree

26 files changed

+263
-121
lines changed

26 files changed

+263
-121
lines changed

jest.config.base.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ module.exports = {
2929
'!./src/**/*.test.tsx',
3030
'!./src/**/*.test.browser.ts',
3131
'!./src/test-utils/**/*.ts',
32+
'!./src/test-utils/**/*.tsx',
3233
'!./src/**/*.d.ts',
3334
'!./src/**/__test__/**',
3435
'!./src/**/__mocks__/**',

packages/examples/packages/images/snap.config.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import type { SnapConfig } from '@metamask/snaps-cli';
22

33
const config: SnapConfig = {
4-
input: './src/index.ts',
4+
input: './src/index.tsx',
55
server: {
66
port: 8026,
77
},

packages/examples/packages/images/snap.manifest.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
"url": "https://github.com/MetaMask/snaps.git"
88
},
99
"source": {
10-
"shasum": "1naltBL/H7TBXHn+Zxgrq7wz6HUKiUUxFQTYTbp5KkQ=",
10+
"shasum": "A2pzU53b9kEBkckTMFDxcPr9/lbaEsOvNUJK+rNx3/8=",
1111
"location": {
1212
"npm": {
1313
"filePath": "dist/bundle.js",

packages/examples/packages/images/src/index.test.ts renamed to packages/examples/packages/images/src/index.test.tsx

Lines changed: 27 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
import { expect } from '@jest/globals';
2-
import { installSnap } from '@metamask/snaps-jest';
3-
import { DialogType, image, panel, text } from '@metamask/snaps-sdk';
2+
import { installSnap, assertIsAlertDialog } from '@metamask/snaps-jest';
3+
import { Box, Text, Image } from '@metamask/snaps-sdk/jsx';
44
import { renderSVG } from 'uqr';
55

66
describe('onRpcRequest', () => {
77
it('throws an error if the requested method does not exist', async () => {
8-
const { request, close } = await installSnap();
8+
const { request } = await installSnap();
99

1010
const response = await request({
1111
method: 'foo',
@@ -20,8 +20,6 @@ describe('onRpcRequest', () => {
2020
cause: null,
2121
},
2222
});
23-
24-
await close();
2523
});
2624

2725
describe('getQrCode', () => {
@@ -36,52 +34,42 @@ describe('onRpcRequest', () => {
3634
});
3735

3836
const ui = await response.getInterface();
37+
assertIsAlertDialog(ui);
38+
3939
expect(ui).toRender(
40-
panel([
41-
text(`The following is a QR code for the data "Hello, world!":`),
42-
image(renderSVG('Hello, world!')),
43-
]),
40+
<Box>
41+
<Text>
42+
The following is a QR code for the data "{'Hello, world!'}":
43+
</Text>
44+
<Image src={renderSVG('Hello, world!')} />
45+
</Box>,
4446
);
4547

46-
// TODO(ritave): Fix types in SnapInterface
47-
await (ui as any).ok();
48+
await ui.ok();
4849

4950
expect(await response).toRespondWith(null);
5051
});
5152
});
5253

5354
describe('getCat', () => {
54-
// This test is flaky, so we disable it for now.
55-
// eslint-disable-next-line jest/no-disabled-tests
56-
it.skip('shows a cat', async () => {
55+
it('shows a cat using an external URL', async () => {
5756
const { request } = await installSnap();
5857

5958
const response = request({
6059
method: 'getCat',
6160
});
6261

6362
const ui = await response.getInterface();
64-
expect(ui).toStrictEqual(
65-
expect.objectContaining({
66-
type: DialogType.Alert,
67-
content: {
68-
type: 'panel',
69-
children: [
70-
{
71-
type: 'text',
72-
value: 'Enjoy your cat!',
73-
},
74-
{
75-
type: 'image',
76-
value: expect.any(String),
77-
},
78-
],
79-
},
80-
}),
63+
assertIsAlertDialog(ui);
64+
65+
expect(ui).toRender(
66+
<Box>
67+
<Text>Enjoy your cat!</Text>
68+
<Image src="https://cataas.com/cat" height={400} width={400} />
69+
</Box>,
8170
);
8271

83-
// TODO(ritave): Fix types in SnapInterface
84-
await (ui as any).ok();
72+
await ui.ok();
8573

8674
expect(await response).toRespondWith(null);
8775
});
@@ -96,6 +84,8 @@ describe('onRpcRequest', () => {
9684
});
9785

9886
const ui = await response.getInterface();
87+
assertIsAlertDialog(ui);
88+
9989
// eslint-disable-next-line jest/prefer-strict-equal
10090
expect(ui.content).toEqual({
10191
type: 'Box',
@@ -120,8 +110,7 @@ describe('onRpcRequest', () => {
120110
key: null,
121111
});
122112

123-
// TODO(ritave): Fix types in SnapInterface
124-
await (ui as any).ok();
113+
await ui.ok();
125114

126115
expect(await response).toRespondWith(null);
127116
});
@@ -136,6 +125,8 @@ describe('onRpcRequest', () => {
136125
});
137126

138127
const ui = await response.getInterface();
128+
assertIsAlertDialog(ui);
129+
139130
// eslint-disable-next-line jest/prefer-strict-equal
140131
expect(ui.content).toEqual({
141132
type: 'Box',
@@ -160,8 +151,7 @@ describe('onRpcRequest', () => {
160151
key: null,
161152
});
162153

163-
// TODO(ritave): Fix types in SnapInterface
164-
await (ui as any).ok();
154+
await ui.ok();
165155

166156
expect(await response).toRespondWith(null);
167157
});

packages/examples/packages/images/src/index.ts renamed to packages/examples/packages/images/src/index.tsx

Lines changed: 31 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,6 @@
11
import type { OnRpcRequestHandler } from '@metamask/snaps-sdk';
2-
import {
3-
DialogType,
4-
getImageComponent,
5-
image,
6-
panel,
7-
text,
8-
MethodNotFoundError,
9-
} from '@metamask/snaps-sdk';
2+
import { DialogType, MethodNotFoundError } from '@metamask/snaps-sdk';
3+
import { Box, Text, Image } from '@metamask/snaps-sdk/jsx';
104
import { renderSVG } from 'uqr';
115

126
import pngIcon from './images/icon.png';
@@ -26,10 +20,8 @@ type GetQrCodeParams = {
2620
* `wallet_invokeSnap` method. This handler handles two methods:
2721
*
2822
* - `getQrCode`: Show a QR code to the user. The QR code is generated using
29-
* the `uqr` library, and rendered using the `image` component.
30-
* - `getCat`: Show a cat to the user. The cat image is fetched using the
31-
* `getImageComponent` helper. The helper returns an `image` component, which
32-
* can be rendered in a Snap dialog, for example.
23+
* the `uqr` library, and rendered using the `Image` component.
24+
* - `getCat`: Show a cat to the user using an external image.
3325
*
3426
* @param params - The request parameters.
3527
* @param params.request - The JSON-RPC request object.
@@ -44,17 +36,19 @@ export const onRpcRequest: OnRpcRequestHandler = async ({ request }) => {
4436
const { data } = request.params as GetQrCodeParams;
4537

4638
// `renderSVG` returns a `<svg>` element as a string, which can be
47-
// rendered using the `image` component.
39+
// rendered using the `Image` component.
4840
const qr = renderSVG(data);
4941

5042
return await snap.request({
5143
method: 'snap_dialog',
5244
params: {
5345
type: DialogType.Alert,
54-
content: panel([
55-
text(`The following is a QR code for the data "${data}":`),
56-
image(qr),
57-
]),
46+
content: (
47+
<Box>
48+
<Text>The following is a QR code for the data "{data}":</Text>
49+
<Image src={qr} />
50+
</Box>
51+
),
5852
},
5953
});
6054
}
@@ -64,15 +58,12 @@ export const onRpcRequest: OnRpcRequestHandler = async ({ request }) => {
6458
method: 'snap_dialog',
6559
params: {
6660
type: DialogType.Alert,
67-
content: panel([
68-
text('Enjoy your cat!'),
69-
70-
// The `getImageComponent` helper can also be used to fetch an image
71-
// from a URL and render it using the `image` component.
72-
await getImageComponent('https://cataas.com/cat', {
73-
width: 400,
74-
}),
75-
]),
61+
content: (
62+
<Box>
63+
<Text>Enjoy your cat!</Text>
64+
<Image src="https://cataas.com/cat" width={400} height={400} />
65+
</Box>
66+
),
7667
},
7768
});
7869
}
@@ -84,8 +75,13 @@ export const onRpcRequest: OnRpcRequestHandler = async ({ request }) => {
8475
type: DialogType.Alert,
8576

8677
// `.svg` files are imported as strings, so they can be used directly
87-
// with the `image` component.
88-
content: panel([text('Here is an SVG icon:'), image(svgIcon)]),
78+
// with the `Image` component.
79+
content: (
80+
<Box>
81+
<Text>Here is an SVG icon:</Text>
82+
<Image src={svgIcon} />
83+
</Box>
84+
),
8985
},
9086
});
9187
}
@@ -97,8 +93,13 @@ export const onRpcRequest: OnRpcRequestHandler = async ({ request }) => {
9793
type: DialogType.Alert,
9894

9995
// `.png` files are imported as SVGs containing an `<image>` tag,
100-
// so they can be used directly with the `image` component.
101-
content: panel([text('Here is a PNG icon:'), image(pngIcon)]),
96+
// so they can be used directly with the `Image` component.
97+
content: (
98+
<Box>
99+
<Text>Here is a PNG icon:</Text>
100+
<Image src={pngIcon} />
101+
</Box>
102+
),
102103
},
103104
});
104105
}
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
2-
"branches": 94.77,
3-
"functions": 98.03,
2+
"branches": 95.16,
3+
"functions": 99,
44
"lines": 98.63,
55
"statements": 98.43
66
}

packages/snaps-controllers/src/interface/SnapInterfaceController.test.tsx

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -524,6 +524,39 @@ describe('SnapInterfaceController', () => {
524524
).toThrow('A Snap interface context may not be larger than 5 MB');
525525
});
526526

527+
it('throws if the Snap attempts to use external images without permission', async () => {
528+
const rootMessenger = getRootSnapInterfaceControllerMessenger();
529+
const controllerMessenger =
530+
getRestrictedSnapInterfaceControllerMessenger(rootMessenger);
531+
532+
rootMessenger.registerActionHandler(
533+
'PermissionController:hasPermission',
534+
() => false,
535+
);
536+
537+
// eslint-disable-next-line no-new
538+
new SnapInterfaceController({
539+
messenger: controllerMessenger,
540+
});
541+
542+
const element = (
543+
<Box>
544+
<Image src="https://metamask.io/foo.png" />
545+
</Box>
546+
);
547+
548+
expect(() =>
549+
rootMessenger.call(
550+
'SnapInterfaceController:createInterface',
551+
MOCK_SNAP_ID,
552+
element,
553+
{},
554+
),
555+
).toThrow(
556+
'Using external images is only permitted with the `endowment:network-access` permission.',
557+
);
558+
});
559+
527560
it('throws if a link is on the phishing list', async () => {
528561
const rootMessenger = getRootSnapInterfaceControllerMessenger();
529562
const controllerMessenger = getRestrictedSnapInterfaceControllerMessenger(

packages/snaps-controllers/src/interface/SnapInterfaceController.ts

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import type {
88
} from '@metamask/base-controller';
99
import { BaseController } from '@metamask/base-controller';
1010
import type { Messenger } from '@metamask/messenger';
11+
import type { HasPermission } from '@metamask/permission-controller';
1112
import type { TestOrigin } from '@metamask/phishing-controller';
1213
import type {
1314
InterfaceState,
@@ -114,7 +115,8 @@ export type SnapInterfaceControllerAllowedActions =
114115
| MultichainAssetsControllerGetStateAction
115116
| AccountsControllerGetSelectedMultichainAccountAction
116117
| AccountsControllerGetAccountByAddressAction
117-
| AccountsControllerListMultichainAccountsAction;
118+
| AccountsControllerListMultichainAccountsAction
119+
| HasPermission;
118120

119121
export type SnapInterfaceControllerActions =
120122
| CreateInterface
@@ -282,7 +284,7 @@ export class SnapInterfaceController extends BaseController<
282284
contentType?: ContentType,
283285
) {
284286
const element = getJsxInterface(content);
285-
this.#validateContent(element);
287+
this.#validateContent(snapId, element);
286288
validateInterfaceContext(context);
287289

288290
const id = nanoid();
@@ -339,7 +341,7 @@ export class SnapInterfaceController extends BaseController<
339341
) {
340342
this.#validateArgs(snapId, id);
341343
const element = getJsxInterface(content);
342-
this.#validateContent(element);
344+
this.#validateContent(snapId, element);
343345
validateInterfaceContext(context);
344346

345347
const oldState = this.state.interfaces[id].state;
@@ -530,13 +532,22 @@ export class SnapInterfaceController extends BaseController<
530532
return this.messenger.call('SnapController:get', id);
531533
}
532534

535+
#hasPermission(snapId: SnapId, permission: string) {
536+
return this.messenger.call(
537+
'PermissionController:hasPermission',
538+
snapId,
539+
permission,
540+
);
541+
}
542+
533543
/**
534544
* Utility function to validate the components of an interface.
535545
* Throws if something is invalid.
536546
*
547+
* @param snapId - The Snap ID.
537548
* @param element - The JSX element to verify.
538549
*/
539-
#validateContent(element: JSXElement) {
550+
#validateContent(snapId: SnapId, element: JSXElement) {
540551
// We assume the validity of this JSON to be validated by the caller.
541552
// E.g., in the RPC method implementation.
542553
const size = getJsonSizeUnsafe(element);
@@ -549,6 +560,7 @@ export class SnapInterfaceController extends BaseController<
549560
isOnPhishingList: this.#checkPhishingList.bind(this),
550561
getSnap: this.#getSnap.bind(this),
551562
getAccountByAddress: this.#getAccountByAddress.bind(this),
563+
hasPermission: this.#hasPermission.bind(this, snapId),
552564
});
553565
}
554566

0 commit comments

Comments
 (0)