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

Pass excludedPermissions to SnapController #17321

Merged
merged 16 commits into from
Feb 15, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
export const buildSnapEndowmentSpecifications = () =>
Object.values(endowmentPermissionBuilders).reduce(
(allSpecifications, { targetKey, specificationBuilder }) => {
if (!ExcludedSnapEndowments.has(targetKey)) {
if (!Object.keys(ExcludedSnapEndowments).includes(targetKey)) {
hmalik88 marked this conversation as resolved.
Show resolved Hide resolved
allSpecifications[targetKey] = specificationBuilder();
}
return allSpecifications;
Expand All @@ -27,10 +27,10 @@ export const buildSnapEndowmentSpecifications = () =>
* @param {Record<string, Function>} hooks - The hooks for the Snap
* restricted method implementations.
*/
export function buildSnapRestrictedMethodSpecifications(hooks) {
return Object.values(restrictedMethodPermissionBuilders).reduce(
export const buildSnapRestrictedMethodSpecifications = (hooks) =>
Object.values(restrictedMethodPermissionBuilders).reduce(
(specifications, { targetKey, specificationBuilder, methodHooks }) => {
if (!ExcludedSnapPermissions.has(targetKey)) {
if (!Object.keys(ExcludedSnapPermissions).includes(targetKey)) {
hmalik88 marked this conversation as resolved.
Show resolved Hide resolved
specifications[targetKey] = specificationBuilder({
methodHooks: selectHooks(hooks, methodHooks),
});
Expand All @@ -39,4 +39,3 @@ export function buildSnapRestrictedMethodSpecifications(hooks) {
},
{},
);
}
12 changes: 9 additions & 3 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ import {
RestrictedMethods,
///: BEGIN:ONLY_INCLUDE_IN(flask)
EndowmentPermissions,
ExcludedSnapPermissions,
ExcludedSnapEndowments,
///: END:ONLY_INCLUDE_IN
} from '../../shared/constants/permissions';
import { UI_NOTIFICATIONS } from '../../shared/notifications';
Expand All @@ -110,6 +112,9 @@ import { STATIC_MAINNET_TOKEN_LIST } from '../../shared/constants/tokens';
import { getTokenValueParam } from '../../shared/lib/metamask-controller-utils';
import { isManifestV3 } from '../../shared/modules/mv3.utils';
import { hexToDecimal } from '../../shared/modules/conversion.utils';
///: BEGIN:ONLY_INCLUDE_IN(flask)
import { isMain, isFlask } from '../../shared/constants/environment';
///: END:ONLY_INCLUDE_IN
import {
onMessageReceived,
checkForMultipleVersionsRunning,
Expand Down Expand Up @@ -775,11 +780,12 @@ export default class MetamaskController extends EventEmitter {
],
});

const isMain = process.env.METAMASK_BUILD_TYPE === 'main';
const isFlask = process.env.METAMASK_BUILD_TYPE === 'flask';

this.snapController = new SnapController({
environmentEndowmentPermissions: Object.values(EndowmentPermissions),
excludedPermissions: {
...ExcludedSnapPermissions,
...ExcludedSnapEndowments,
},
closeAllConnections: this.removeAllConnections.bind(this),
state: initState.SnapController,
messenger: snapControllerMessenger,
Expand Down
26 changes: 26 additions & 0 deletions lavamoat/browserify/beta/policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -1311,6 +1311,32 @@
"browserify>buffer": true
}
},
"@metamask/rpc-methods>@metamask/key-tree": {
"packages": {
"@metamask/rpc-methods>@metamask/key-tree>@noble/ed25519": true,
"@metamask/rpc-methods>@metamask/key-tree>@noble/secp256k1": true,
"@metamask/scure-bip39": true,
"@metamask/snaps-utils>@noble/hashes": true,
"@metamask/snaps-utils>@scure/base": true,
"@metamask/utils": true
}
},
"@metamask/rpc-methods>@metamask/key-tree>@noble/ed25519": {
"globals": {
"crypto": true
},
"packages": {
"browserify>browser-resolve": true
}
},
"@metamask/rpc-methods>@metamask/key-tree>@noble/secp256k1": {
"globals": {
"crypto": true
},
"packages": {
"browserify>browser-resolve": true
}
},
"@metamask/rpc-methods>nanoid": {
"globals": {
"crypto.getRandomValues": true
Expand Down
26 changes: 26 additions & 0 deletions lavamoat/browserify/flask/policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -1403,6 +1403,32 @@
"browserify>buffer": true
}
},
"@metamask/rpc-methods>@metamask/key-tree": {
"packages": {
"@metamask/rpc-methods>@metamask/key-tree>@noble/ed25519": true,
"@metamask/rpc-methods>@metamask/key-tree>@noble/secp256k1": true,
"@metamask/scure-bip39": true,
"@metamask/snaps-utils>@noble/hashes": true,
"@metamask/snaps-utils>@scure/base": true,
"@metamask/utils": true
}
},
"@metamask/rpc-methods>@metamask/key-tree>@noble/ed25519": {
"globals": {
"crypto": true
},
"packages": {
"browserify>browser-resolve": true
}
},
"@metamask/rpc-methods>@metamask/key-tree>@noble/secp256k1": {
"globals": {
"crypto": true
},
"packages": {
"browserify>browser-resolve": true
}
},
"@metamask/rpc-methods>nanoid": {
"globals": {
"crypto.getRandomValues": true
Expand Down
26 changes: 26 additions & 0 deletions lavamoat/browserify/main/policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -1311,6 +1311,32 @@
"browserify>buffer": true
}
},
"@metamask/rpc-methods>@metamask/key-tree": {
"packages": {
"@metamask/rpc-methods>@metamask/key-tree>@noble/ed25519": true,
"@metamask/rpc-methods>@metamask/key-tree>@noble/secp256k1": true,
"@metamask/scure-bip39": true,
"@metamask/snaps-utils>@noble/hashes": true,
"@metamask/snaps-utils>@scure/base": true,
"@metamask/utils": true
}
},
"@metamask/rpc-methods>@metamask/key-tree>@noble/ed25519": {
"globals": {
"crypto": true
},
"packages": {
"browserify>browser-resolve": true
}
},
"@metamask/rpc-methods>@metamask/key-tree>@noble/secp256k1": {
"globals": {
"crypto": true
},
"packages": {
"browserify>browser-resolve": true
}
},
"@metamask/rpc-methods>nanoid": {
"globals": {
"crypto.getRandomValues": true
Expand Down
2 changes: 2 additions & 0 deletions shared/constants/environment.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export const isMain = process.env.METAMASK_BUILD_TYPE === 'main';
export const isFlask = process.env.METAMASK_BUILD_TYPE === 'flask';
8 changes: 6 additions & 2 deletions shared/constants/permissions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ describe('EndowmentPermissions', () => {
it('has the expected permission keys', () => {
expect(Object.keys(EndowmentPermissions).sort()).toStrictEqual(
Object.keys(endowmentPermissionBuilders)
.filter((targetKey) => !ExcludedSnapEndowments.has(targetKey))
.filter(
(targetKey) =>
!Object.keys(ExcludedSnapEndowments).includes(targetKey),
)
.sort(),
);
});
Expand All @@ -23,7 +26,8 @@ describe('RestrictedMethods', () => {
[
'eth_accounts',
...Object.keys(restrictedMethodPermissionBuilders).filter(
(targetKey) => !ExcludedSnapPermissions.has(targetKey),
(targetKey) =>
!Object.keys(ExcludedSnapPermissions).includes(targetKey),
),
].sort(),
);
Expand Down
18 changes: 15 additions & 3 deletions shared/constants/permissions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,32 @@ export const RestrictedMethods = Object.freeze({
} as const);

///: BEGIN:ONLY_INCLUDE_IN(flask)
/**
* Exclude permissions by code fencing them to avoid any potential usage of excluded permissions at runtime. See: https://github.com/MetaMask/metamask-extension/pull/17321#pullrequestreview-1287014285.
* This is a fix for https://github.com/MetaMask/snaps-monorepo/issues/1103 and https://github.com/MetaMask/snaps-monorepo/issues/990.
* TODO: Disable endowment:long-running and eth_account in stable.
GuillaumeRx marked this conversation as resolved.
Show resolved Hide resolved
*/
export const PermissionNamespaces = Object.freeze({
wallet_snap_: 'wallet_snap_*',
} as const);

export const EndowmentPermissions = Object.freeze({
'endowment:network-access': 'endowment:network-access',
'endowment:long-running': 'endowment:long-running',
'endowment:transaction-insight': 'endowment:transaction-insight',
'endowment:cronjob': 'endowment:cronjob',
'endowment:ethereum-provider': 'endowment:ethereum-provider',
'endowment:rpc': 'endowment:rpc',
'endowment:long-running': 'endowment:long-running',
} as const);

// Methods / permissions in external packages that we are temporarily excluding.
export const ExcludedSnapPermissions = new Set([]);
export const ExcludedSnapEndowments = new Set(['endowment:keyring']);
export const ExcludedSnapPermissions = Object.freeze({
eth_accounts:
'eth_accounts is disabled. For more information please see https://github.com/MetaMask/snaps-monorepo/issues/990.',
});

export const ExcludedSnapEndowments = Object.freeze({
'endowment:keyring':
'This endowment is still in development therefore not available.',
});
///: END:ONLY_INCLUDE_IN