Skip to content

fix (cherry-pick): Fix audit failures v12.5.1#28187

Merged
danjm merged 2 commits intoVersion-v12.5.1from
fix-audit-failures-v12.5.1
Oct 31, 2024
Merged

fix (cherry-pick): Fix audit failures v12.5.1#28187
danjm merged 2 commits intoVersion-v12.5.1from
fix-audit-failures-v12.5.1

Conversation

@danjm
Copy link
Copy Markdown
Contributor

@danjm danjm commented Oct 30, 2024

Cherry-picks #28145 to v12.5.1, and brings #28177 into v12.5.1 as well. The latter originally targeted v12.6.0, but we need it for a hotfix ahead of that. The PR description for that was:

Description

Forcing resolutions to fix yarn audit warnings and more specifically this issue:

I decided to be very explicit about the resolution itself based on the output of:

$ yarn why secp256k1
├─ ethereum-cryptography@npm:0.1.3
│  └─ secp256k1@npm:4.0.4 (via npm:^4.0.1)

├─ ganache@npm:7.9.2
│  └─ secp256k1@npm:4.0.3 (via npm:4.0.3)

├─ ganache@patch:ganache@npm%3A7.9.2#~/.yarn/patches/ganache-npm-7.9.2-a70dc8da34.patch::version=7.9.2&hash=7d7c66
│  └─ secp256k1@npm:4.0.3 (via npm:4.0.3)

├─ gridplus-sdk@npm:2.5.1
│  └─ secp256k1@npm:4.0.2 (via npm:4.0.2)

└─ hdkey@npm:2.1.0
   └─ secp256k1@npm:4.0.4 (via npm:^4.0.0)

We could also have a more straightforward resolution like:

  ...
  "resolutions": {
    ...
    "secp256k1": "4.0.4"
  }
  ...

But that could also catch version with different major.

Let me know what would be the preferred solution here.

Open in GitHub Codespaces

Related issues

Fixes: GHSA-584q-6j8j-r5pm

Manual testing steps

N/A

Screenshots/Recordings

Before

After

Pre-merge author checklist

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.

ccharly and others added 2 commits October 30, 2024 12:50
## **Description**

To unblock develop, creating this PR that adds `ethereumjs-wallet` to
list of deprecated packages that are ignored when using yarn audit. .

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/28145?quickstart=1)

## **Related issues**

Fixes:

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] 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
- [ ] 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-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.
@danjm danjm requested review from a team as code owners October 30, 2024 15:23
@github-actions
Copy link
Copy Markdown
Contributor

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.

@github-actions github-actions bot added the team-extension-platform Extension Platform team label Oct 30, 2024
@socket-security
Copy link
Copy Markdown

Removed dependencies detected. Learn more about Socket for GitHub ↗︎

🚮 Removed packages: npm/secp256k1@4.0.3

View full report↗︎

Copy link
Copy Markdown
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [9e71592]
Page Load Metrics (1822 ± 71 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint16402289181714569
domContentLoaded16242169177612962
load16422312182214871
domInteractive247943178

@dbrans
Copy link
Copy Markdown
Contributor

dbrans commented Oct 30, 2024

LGTM

@danjm danjm merged commit 7706431 into Version-v12.5.1 Oct 31, 2024
@danjm danjm deleted the fix-audit-failures-v12.5.1 branch October 31, 2024 02:46
@github-actions github-actions bot locked and limited conversation to collaborators Oct 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

team-extension-platform Extension Platform team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants