chore: update @trezor/connect-web to v9.2.2#23763
Conversation
|
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. |
|
👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎ This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. Ignoring: Next stepsTake a deeper look at the dependencyTake a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev. Remove the packageIf you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency. Mark a package as acceptable riskTo ignore an alert, reply with a comment starting with |
75f4066 to
a4523b6
Compare
@trezor/connect-web to v9.2.2
01fb099 to
50fcea0
Compare
|
@metamaskbot update-policies |
|
Policies updated |
cbff18b to
88abc4b
Compare
|
|
||
| it('Connects to a Hardware wallet for trezor', async function () { | ||
| it('Connects to a Hardware wallet for lattice', async function () { | ||
| if (process.env.ENABLE_MV3) { |
There was a problem hiding this comment.
It looks like this replaces the existing test for Trezor with a test for GridPlus Lattice. Is there a reason to not keep testing for the Trezor-connect case?
There was a problem hiding this comment.
The trezor connect case now requires the user to interact with a browser pop-up before going to the trezor connect screen, and we are unable to do that from the e2e test. However, this e2e test never tested any specific trezor functionality, and just the behaviour of the first steps of the hardware wallet connect flow, which were essentially the same for gridplus and trezor
Co-authored-by: legobeat <109787230+legobeat@users.noreply.github.com>
Builds ready [abfcc55]
Page Load Metrics (1169 ± 590 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #23763 +/- ##
===========================================
- Coverage 67.43% 67.42% -0.01%
===========================================
Files 1290 1290
Lines 50308 50316 +8
Branches 13034 13037 +3
===========================================
+ Hits 33922 33924 +2
- Misses 16386 16392 +6 ☔ View full report in Codecov by Sentry. |
Builds ready [1e820a7]
Page Load Metrics (633 ± 468 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Builds ready [650dc3d]
Page Load Metrics (1390 ± 584 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
|
The implementation looks good to me. Approved ~ |
Builds ready [e167e9a]
Page Load Metrics (506 ± 422 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
|
@SocketSecurity ignore-all All of the dependencies that socket security is warning us about are dependencies of trezor, but not of them are used in our applications code |
|
Missing release label release-11.16.6 on PR. Adding release label release-11.16.6 on PR and removing other release labels(release-11.18.0), as PR was cherry-picked in branch 11.16.6. |
Description
The user experience for trezor connection on mv2 is this:
Before this PR, on mv3, the flow was:
The solution to the problem required two parts.
First, we needed the
connect-webmodule (calledTrezorConnectSDKin the offscreen trezor file) to specify theenvvariable communicated to the trezor connect iframe and popup to be "webextension" and not just "web". This is because the iframe/popup code assumes that if the iframe origin does not match the origin of popup (e.g. metamask vs connect.trezor.io), and if environment is not "webextension", then webusb cannot be available: https://github.com/trezor/trezor-suite/blob/bb2e075024c8d8316fa016b7b20a0421b1a1f7d0/packages/connect-iframe/src/connectSettings.ts#L74-L84.In theory, this could be set when the trezor module is initialized, but if the
envis set to "webextension" in theconnect-webcode, then it attempts to open the popup via chrome apis that are not available in the offscreen document. So the solution is a patch which just tells the iframe and the popup that the environment is "webextension", while leave the environment setting within the trezor code that runs in the offscreen context as "web".With that fix in place, there was another problem of metamask being able to connect to the webusb device and the popup being able to read that connection (or vice-versa). If the
requestDevicecall happens from the popup, then the webusb permission will be granted to the trezor.io origin, and metamask is unable to communicate with those devices. However, if therequestDevicecall happens from metamask, then the devices can be communicated from metamask to the popup, and metamask (in particular the offscreen document) can communicate directly with the devices. Specifically, this code https://github.com/trezor/trezor-suite/blob/bb2e075024c8d8316fa016b7b20a0421b1a1f7d0/packages/transport/src/api/usb.ts#L98, when run within the offscreen document, returns null whenrequestDevicewas called from the trezor popup, but returns a correctly populated array ifrequestDevicewas called from MetaMask. To fix this, we can callrequestDeviceexplicitly within the MetaMask ui flow, instead of having the trezor popup call it for usFinally, and independent of the above two fixes, this PR updates trezor connect packages, which was necessary to get trezor working properly with Snow.
Related issues
Fixes:
Manual testing steps
yarn start:mv3Screenshots/Recordings
trezorworking.mp4
Pre-merge author checklist
Pre-merge reviewer checklist