-
Notifications
You must be signed in to change notification settings - Fork 5k
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
Version v12.6.2 RC #28507
Version v12.6.2 RC #28507
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 |
Updates tje changelog for v12.6.2
@SocketSecurity ignore-all This version returns the master branch to the state it was in prior to v12.7.0, plus one code change that did not modify dependencies, so no changes here should pose a security risk. Furthermore, the new authors are known/trusted, and the warnings about gridplus and snaps-utils are to be expected |
Builds ready [91b289b]
Page Load Metrics (1940 ± 79 ms)
|
This is a cherry-pick of #28522 for v12.6.2. Original description: ## **Description** The package `cross-spawn` has been updated to v7.0.6 to address a security advisory. The advisory doesn't impact our usage of this library, but it was easy to update. We had two usages of an older major version of this library in our dependency tree (v5), which were forced to v7 using a resolution. The only breaking changes in v6 and v7 were dropping support for older Node.js versions that are already below our minimum supported version. `cross-spawn` changelog: https://github.com/moxystudio/node-cross-spawn/blob/master/CHANGELOG.md [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/28522?quickstart=1) ## **Related issues** Resolves GHSA-3xgq-45jj-v275 ## **Manual testing steps** N/A ## **Screenshots/Recordings** N/A ## **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/develop/.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/develop/.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.
Builds ready [7fbd4fc]
Page Load Metrics (2138 ± 195 ms)
|
This is a cherry-pick of #28521 for v12.6.2. Original description: ## **Description** The QR scanner is now more strict about the contents it allows to be scanned. If the scanned QR code deviates at all from the supported formats, it will return "unknown" as the result (as it always has for completely unrecognized QR codes). Previously we would accept QR codes with a recognized prefix even if the complete contents did not match our expectations, which has resulted in unexpected behavior. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/28521?quickstart=1) ## **Related issues** Fixes #28527 ## **Manual testing steps** - Open the MetaMask extension and select 'Send' - Click on the QR scanner icon in the "Send To" field and enable webcam - Scan a ERC-20 wallet receive QR from a mobile app, which follows the EIP-681 standard and contains a valid token contract and account address - ERC-20 Token Contract Address, which is the first address in the string, populates the "Send To" field instead of the intended recipient address ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** We didn't record this, but multiple people on the team reproduced the problem. ### **After** https://www.loom.com/share/be8822e872a14ec98a47547cf6198603 ## **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/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - We don't yet have any way to test QR scanning. We will follow up later with tests, and rely on manual testing for now. Later test automation work tracked in #28528 - [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/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [x] 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.
Report too large to display inline |
Builds ready [2167145]
Page Load Metrics (1975 ± 143 ms)
|
Manual test instructions for the only change in this RC are available here: #28521 For the QR code to test with, you can use another wallet (like Coinbase wallet, as was the original report) to generate the QR code, but it may be simpler to generate it yourself with a free QR code generator. You'll want the QR code to resolve to a string of the format Using a code like that, you should see the scan fail on this RC. But previously it would set the recipient to the contract address. |
Testing has been completed against the Version-v12.6.2 with the changes #28521 and could see the proper error message that appears when scanning the code based on the |
📦 🚀