-
-
Notifications
You must be signed in to change notification settings - Fork 260
feat: decode permissions in SignatureController #6619
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
Conversation
217553e to
4dee17d
Compare
8a636fc to
4b72727
Compare
4b72727 to
21a7c84
Compare
21a7c84 to
b4006dc
Compare
b4006dc to
47460e5
Compare
0ac0fd7 to
b5b431f
Compare
bf157ae to
48d4d7b
Compare
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
31cbe3c to
442a239
Compare
442a239 to
8378da7
Compare
8378da7 to
a3a38b6
Compare
| }; | ||
|
|
||
| /** Metadata use in the signTypedData request when handling EIP-7715 execution permission requests */ | ||
| export type ExecutionPermissionMetadata = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor, this isn't necessarily specific to the permissions but general metadata.
| /** Metadata use in the signTypedData request when handling EIP-7715 execution permission requests */ | ||
| export type ExecutionPermissionMetadata = { | ||
| origin: string; | ||
| justification: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor, JSDoc on properties?
| caveats, | ||
| } as unknown as MessageParamsTyped, | ||
| ], | ||
| [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| await this.#approveAndSignRequest(metadata, traceContext); | ||
| } catch (error) { | ||
| log('Signature request failed', error); | ||
| log('Signature request failed', (error as Error).message); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Error Logging Fails with Non-Error Objects
The updated error logging in SignatureController.ts assumes caught errors are always Error objects. As JavaScript allows throwing any value, accessing .message on non-Error types can lead to undefined being logged or a runtime error, losing valuable debugging context.
## Explanation Releases @metamask/signature-controller@34.0.0 ### Added - Add two new controller state metadata properties: `includeInStateLogs` and `usedInUi` ([#6473](#6473)) - **BREAKING:** Decode delegation permissions using `@metamask/gator-permissions-controller` when calling `newUnsignedTypedMessage`, adds `@metamask/gator-permissions-controller` as a peer dependency. ([#6619](#6619)) ### Changed - Bump `@metamask/controller-utils` from `^11.12.0` to `^11.14.0` ([#6620](#6620), [#6629](#6629)) - Bump `@metamask/base-controller` from `^8.1.0` to `^8.4.0` ([#6355](#6355), [#6465](#6465), [#6632](#6632)) - Bump `@metamask/utils` from `^11.4.2` to `^11.8.0` ([#6588](#6588)) - Bump `@metamask/network-controller` from `^24.1.0` to `^24.2.0` ([#6678](#6678)) - Bump `@metamask/keyring-controller` from `^23.0.0` to `^23.1.0` ([#6590](#6590)) - Bump `@metamask/accounts-controller` from `^33.0.0` to `^33.1.0` ([#6572](#6572))
…ded permission (#36054) ## **Description** This feature is only enabled in Flask. When a dapp requests an EIP-7715 Readable Permission, the user must sign with `eth_signTypedData_v4`, but it's important that the user can understand what is being signed. This PR surfaced the decoded permission information in a new Sign Permission view. When a `eth_signTypedData_v4` request is received by `@metamask/signature-controller` that appears to be a delegation, it will pass this request to `@metamask/gator-permissions-controller` to decode the request into the originating permission. If this is successful, `@metamask/signature-controller` attaches the decoded permission to the request `metadata`. This PR adds a new view showing the details of the permission that the user is signing. This does not implement the final design, and focusses only on introducing the functionality to extension. Further iteration will be made to improve the UX. As such, no internationalization has been included in this change. A couple important points to note: - if `GATOR_PERMISSIONS_ENABLED` is not true, the request will be rejected here (this should never happen, because the method `wallet_requestExecutionPermissions` will not allow requests if the feature is not enabled - if the origin is not the configured Gator Permissions Snap, the permission will not be decoded by the `GatorPermissionsController` - if the request is for a delegation signature, and no permission is decoded, the SignatureController will reject the request - This feature is only being released into Flask at this stage - future PRs will refine the UX before this is released to end-users [](https://codespaces.new/MetaMask/metamask-extension/pull/36054?quickstart=1) ## **Changelog** CHANGELOG entry: presents a Permission confirmation view when a decoded permission exists on signTypedData metadata. Flask only. ## **Related issues** These PRs are related to this feature: - Introduce GatorPermissionsController to mobile MetaMask/metamask-mobile#20006 - Decode permissions in SignatureController MetaMask/core#6619 - Method 'decodePermissionFromPermissionContextForOrigin' is now synchronous MetaMask/core#6656 - Add `decodePermissionFromPermissionContextForOrigin` to `GatorPermissionsController` MetaMask/core#6556 ## **Manual testing steps** 1. Run the 7715 gator snaps locally 2. Build MetaMask flask with `GATOR_PERMISSIONS_ENABLED=true` and `GATOR_PERMISSIONS_PROVIDER_SNAP_ID=local:http://localhost:8082` 2. Open the 7715 gator snap page 3. Install snaps if necessary, by clicking "Install kernel snap", then "Install provider snap" 4. Select the appropriate permission type and parameters 5. Click "Grant Permission" 6. Adjust the requested permission as necessary 7. Click "Grant" Expect to see the sign permission screen: ## **Screenshots/Recordings** <img width="396" height="852" alt="image" src="https://github.com/user-attachments/assets/b45e5a7d-ccaa-4456-be65-4b990bc2ea2d" /> ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
## **Description** Updates the dependency on @metamask/signature-controller to 34.0.0 This version bump introduces a dependency on @metamask/gator-permissions-controller via `GatorPermissionsControllerDecodePermissionFromPermissionContextForOriginAction` which has been added to signature-controller-messenger. ## **Changelog** From SignatureController https://github.com/MetaMask/core/blob/main/packages/signature-controller/CHANGELOG.md: ### @metamask/signature-controller 34.0.0 #### Added - Add two new controller state metadata properties: includeInStateLogs and usedInUi (MetaMask/core#6473) - Decode delegation permissions using `@metamask/gator-permissions-controller` when calling `newUnsignedTypedMessage` ([#6619](MetaMask/core#6619)) #### Changed - Bump @metamask/controller-utils from ^11.12.0 to ^11.14.0 (MetaMask/core#6620, MetaMask/core#6629) - Bump @metamask/base-controller from ^8.1.0 to ^8.4.0 (MetaMask/core#6355, MetaMask/core#6465, MetaMask/core#6632) - Bump @metamask/utils from ^11.4.2 to ^11.8.0 (MetaMask/core#6588) ### @metamask/signature-controller 33.0.0 #### Changed - BREAKING: Bump peer dependency @metamask/accounts-controller from ^32.0.0 to ^33.0.0 (MetaMask/core#6345) - BREAKING: Bump peer dependency @metamask/keyring-controller from ^22.0.0 to ^23.0.0 (MetaMask/core#6345) - Bump @metamask/base-controller from ^8.0.1 to ^8.1.0 (MetaMask/core#6284) - Bump @metamask/controller-utils from ^11.11.0 to ^11.12.0 (MetaMask/core#6303) CHANGELOG entry: null ## **Screenshots/Recordings** ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Upgrade @metamask/signature-controller to ^34 and allow GatorPermissionsController decode action in the signature controller messenger. > > - **Dependencies**: > - Bump `@metamask/signature-controller` to `^34.0.0`. > - **Engine**: > - **`app/core/Engine/messengers/signature-controller-messenger/signature-controller-messenger.ts`**: > - Add `GatorPermissionsControllerDecodePermissionFromPermissionContextForOriginAction` and allow `GatorPermissionsController:decodePermissionFromPermissionContextForOrigin` in `allowedActions`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 657705e. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
Summary
Add execution-permission decoding to
SignatureControllerforeth_signTypedData_v4delegation requests. If a delegation request is unable to be decoded into a coherent Execution Permission, reject external (not fromorigin: metamask) attempts to sign delegations for internal accounts. Adds decoded permission in signature request state for rendering downstream.Why
Readable Permissions requires the ability to sign delegations, only when the delegation can be decoded to a permission that is then shown to the user.
The architecture is described in this document. Decoding has been added to
GatorPermissionsControllerin #6556Note: the request origin is always
@metamask/gator-permission-snap(any other origin is rejected). Because of this,metadatais added to the 712 payload, defining thejustificationandoriginwhich is the original origin of the request (the dapp).