Pass `excludedPermissions` to `SnapController` #17321

GuillaumeRx · 2023-01-20T15:41:09Z

Create a record of excluded permission/endowments with their associated error messages and pass it to SnapController. This also checks for the MetaMask version and updates the relevant methods using those excluded permissions.

Fixes: MetaMask/snaps#1103 MetaMask/snaps#990

github-actions · 2023-01-20T15:41:21Z

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

metamaskbot · 2023-01-23T16:07:04Z

Builds ready [548ad1e]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4
- common: 0, 1, 2, 3, 4
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1294 ± 95 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	101	371	133	56	27
		domContentLoaded	997	1645	1292	198	95
		load	997	1646	1294	198	95
		domInteractive	997	1645	1292	198	95

Bundle size diffs [🚀 Bundle size reduced!]

background: -2039 bytes
ui: 0 bytes
common: 0 bytes

metamaskbot · 2023-01-23T17:41:09Z

Builds ready [439a448]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1372 ± 97 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	95	288	134	39	19
		domContentLoaded	1028	1747	1350	184	88
		load	1028	1825	1372	201	97
		domInteractive	1028	1747	1350	184	88

Bundle size diffs [🚀 Bundle size reduced!]

background: -2039 bytes
ui: 0 bytes
common: 0 bytes

shared/constants/permissions.ts

metamaskbot · 2023-01-24T16:29:08Z

Builds ready [d15007e]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1287 ± 121 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	96	169	136	24	11
		domContentLoaded	999	1810	1267	246	118
		load	1017	1827	1287	252	121
		domInteractive	999	1810	1267	246	118

Bundle size diffs

background: 0 bytes
ui: 0 bytes
common: 0 bytes

shared/constants/flask/environment.js

shared/constants/permissions.ts

metamaskbot · 2023-01-26T11:30:52Z

Builds ready [413947d]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1358 ± 109 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	99	157	124	17	8
		domContentLoaded	989	1688	1320	218	105
		load	1064	1695	1358	227	109
		domInteractive	989	1688	1320	218	105

Bundle size diffs

background: 0 bytes
ui: 0 bytes
common: 0 bytes

metamaskbot · 2023-01-30T16:22:29Z

Builds ready [d87186e]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1310 ± 114 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	96	172	136	21	10
		domContentLoaded	1046	1960	1275	231	111
		load	1047	1960	1310	237	114
		domInteractive	1046	1960	1275	231	111

Bundle size diffs

background: 0 bytes
ui: 0 bytes
common: 0 bytes

shared/constants/permissions.ts

metamaskbot · 2023-01-31T18:17:40Z

Builds ready [d1da271]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1230 ± 111 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	89	148	107	14	7
		domContentLoaded	915	1556	1225	225	108
		load	915	1601	1230	231	111
		domInteractive	915	1556	1225	225	108

Bundle size diffs

background: 0 bytes
ui: 0 bytes
common: 0 bytes

shared/constants/permissions.ts

app/scripts/controllers/permissions/flask/snap-permissions.js

ritave

Excluded permissions should not be checked during run-time, but be completely removed during build time using code fencing.

I know this quite a bit of additional work, but this dramatically increases security. If we have a bug in logic, currently a bad actor could still find an exploit to run those permissions in stable.

If there's no code for those permissions at all in the extension, no way to run them in the first place.

As a suggestion, the easiest place to remove them during build time might be the RPC methods and PermissionController set-up, and the list here be used for informational purposes in console logs.

metamaskbot · 2023-02-07T15:06:59Z

Builds ready [e5b1f9c]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1212 ± 92 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	91	178	118	22	11
		domContentLoaded	978	1610	1194	196	94
		load	1000	1610	1212	193	92
		domInteractive	978	1610	1194	196	94

Bundle size diffs

background: 0 bytes
ui: 0 bytes
common: 0 bytes

Dismissing due to code fencing being implemented

metamaskbot · 2023-02-08T14:57:14Z

Builds ready [0de6123]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1388 ± 104 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	110	192	143	23	11
		domContentLoaded	1160	1934	1365	206	99
		load	1160	1934	1388	217	104
		domInteractive	1160	1934	1365	206	99

shared/constants/permissions.ts

brad-decker · 2023-02-14T18:28:00Z

@GuillaumeRx @ritave should this be merged?

GuillaumeRx requested a review from a team as a code owner January 20, 2023 15:41

GuillaumeRx requested review from adonesky1 and removed request for a team January 20, 2023 15:41

FrederikBolding force-pushed the snaps-monorepo@0.28.0 branch from 4261e1e to 64a977a Compare January 23, 2023 09:59

GuillaumeRx force-pushed the gr/pass-excluded-permissions branch 2 times, most recently from ee0312f to 548ad1e Compare January 23, 2023 15:43

FrederikBolding force-pushed the snaps-monorepo@0.28.0 branch from af52878 to 4d08d54 Compare January 23, 2023 16:11

GuillaumeRx force-pushed the gr/pass-excluded-permissions branch from 548ad1e to 439a448 Compare January 23, 2023 17:17

This was linked to issues Jan 23, 2023

Remove endowment:long-running from Snap APIs MetaMask/snaps#945

Closed

Forbid snaps from requesting eth_accounts MetaMask/snaps#990

Closed

Base automatically changed from snaps-monorepo@0.28.0 to develop January 23, 2023 19:41

FrederikBolding requested changes Jan 24, 2023

View reviewed changes

shared/constants/permissions.ts Outdated Show resolved Hide resolved

shared/constants/permissions.ts Outdated Show resolved Hide resolved

shared/constants/permissions.ts Outdated Show resolved Hide resolved

shared/constants/permissions.ts Outdated Show resolved Hide resolved

FrederikBolding removed a link to an issue Jan 24, 2023

Remove endowment:long-running from Snap APIs MetaMask/snaps#945

Closed

GuillaumeRx force-pushed the gr/pass-excluded-permissions branch from 439a448 to 88acbf7 Compare January 24, 2023 15:41

FrederikBolding reviewed Jan 24, 2023

View reviewed changes

shared/constants/flask/environment.js Outdated Show resolved Hide resolved

shared/constants/permissions.ts Outdated Show resolved Hide resolved

FrederikBolding previously approved these changes Jan 25, 2023

View reviewed changes

GuillaumeRx dismissed FrederikBolding’s stale review via d87186e January 30, 2023 15:53

GuillaumeRx force-pushed the gr/pass-excluded-permissions branch from 413947d to d87186e Compare January 30, 2023 15:53

GuillaumeRx force-pushed the gr/pass-excluded-permissions branch from d87186e to d1da271 Compare January 31, 2023 17:42

david0xd reviewed Jan 31, 2023

View reviewed changes

shared/constants/permissions.ts Outdated Show resolved Hide resolved

david0xd reviewed Jan 31, 2023

View reviewed changes

shared/constants/permissions.ts Outdated Show resolved Hide resolved

david0xd previously approved these changes Jan 31, 2023

View reviewed changes

GuillaumeRx dismissed david0xd’s stale review via fd7500c February 1, 2023 16:25

hmalik88 reviewed Feb 6, 2023

View reviewed changes

app/scripts/controllers/permissions/flask/snap-permissions.js Show resolved Hide resolved

hmalik88 reviewed Feb 6, 2023

View reviewed changes

app/scripts/controllers/permissions/flask/snap-permissions.js Show resolved Hide resolved

ritave previously requested changes Feb 7, 2023

View reviewed changes

GuillaumeRx dismissed david0xd’s stale review via e5b1f9c February 7, 2023 14:41

FrederikBolding and others added 16 commits February 8, 2023 15:28

snaps-monorepo@1.28.0

d8e6a2e

Update LavaMoat policies

3a9b190

pass excludedPermissions to SnapController

4492ae7

update useless changes

685b2e1

update yarn.lock

b05340c

fix rebase

ab266ac

fix lavamoat after rebase

08e147d

fix tests

9ff79ef

address requested changes

dd0912c

fix code fencing

77d336d

move environment file

f75d72b

Freeze objects and update messages

7a6fc5d

remove useless freeze

4f90b7a

use code fencing

011ccae

add ref to issues

ae1e914

re-trigger CI

0de6123

GuillaumeRx force-pushed the gr/pass-excluded-permissions branch from 0bda6dc to 0de6123 Compare February 8, 2023 14:29

hmalik88 approved these changes Feb 8, 2023

View reviewed changes

ritave approved these changes Feb 9, 2023

View reviewed changes

shared/constants/permissions.ts Show resolved Hide resolved

GuillaumeRx merged commit ccde549 into develop Feb 15, 2023

GuillaumeRx deleted the gr/pass-excluded-permissions branch February 15, 2023 10:09

github-actions bot locked and limited conversation to collaborators Feb 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pass `excludedPermissions` to `SnapController` #17321

Pass `excludedPermissions` to `SnapController` #17321

GuillaumeRx commented Jan 20, 2023 •

edited by FrederikBolding

Loading

github-actions bot commented Jan 20, 2023

metamaskbot commented Jan 23, 2023

metamaskbot commented Jan 23, 2023

metamaskbot commented Jan 24, 2023

metamaskbot commented Jan 26, 2023

metamaskbot commented Jan 30, 2023

metamaskbot commented Jan 31, 2023

ritave left a comment •

edited

Loading

metamaskbot commented Feb 7, 2023

metamaskbot commented Feb 8, 2023

brad-decker commented Feb 14, 2023

Pass excludedPermissions to SnapController #17321

Pass excludedPermissions to SnapController #17321

Conversation

GuillaumeRx commented Jan 20, 2023 • edited by FrederikBolding Loading

github-actions bot commented Jan 20, 2023

metamaskbot commented Jan 23, 2023

metamaskbot commented Jan 23, 2023

metamaskbot commented Jan 24, 2023

metamaskbot commented Jan 26, 2023

metamaskbot commented Jan 30, 2023

metamaskbot commented Jan 31, 2023

ritave left a comment • edited Loading

Choose a reason for hiding this comment

metamaskbot commented Feb 7, 2023

metamaskbot commented Feb 8, 2023

brad-decker commented Feb 14, 2023

Pass `excludedPermissions` to `SnapController` #17321

Pass `excludedPermissions` to `SnapController` #17321

GuillaumeRx commented Jan 20, 2023 •

edited by FrederikBolding

Loading

ritave left a comment •

edited

Loading