Skip to content

Audit dependencies and clean up dependency tree #955

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

Merged
merged 3 commits into from
Nov 16, 2020

Conversation

rumkin
Copy link
Contributor

@rumkin rumkin commented Nov 16, 2020

This PR fixes #952 issue to remove vulnerable dependencies from EthereumJS libraries.

TODOLIST

@ethereumjs/block

Found Fixed
deps 0 0
dev-deps 0 0

@ethereumjs/blockchain

Found Fixed
deps 0 0
dev-deps 0 0

@ethereumjs/common

Found Fixed
deps 0 0
dev-deps 0 0

@ethereumjs/ethash

Found Fixed
deps 0 0
dev-deps 0 0

@ethereumjs/tx

Found Fixed
deps 0 0
dev-deps 9 9
  • Removed redundant contributor package. Reason: dependency is not in use.

@ethereumjs/vm

Found Fixed
deps 0 0
dev-deps 144 0

@codecov
Copy link

codecov bot commented Nov 16, 2020

Codecov Report

Merging #955 (f59e9c5) into master (81fc88b) will decrease coverage by 0.07%.
The diff coverage is n/a.

Impacted file tree graph

Flag Coverage Δ
block 77.41% <ø> (ø)
blockchain 77.39% <ø> (ø)
common 91.87% <ø> (-0.25%) ⬇️
ethash 82.08% <ø> (ø)
tx 86.04% <ø> (ø)
vm 87.21% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@holgerd77
Copy link
Member

@rumkin This looks like a great start on this, cool! 👍

Two notes:

  1. There is the wrong issue linked in the initial comment
  2. @herumi was superquick on Move nyc from dependencies to devDependencies herumi/mcl-wasm#17 (thanks a thousand times for that 😄 😄 ) and already released versions v0.6.0, v0.7.0 and v0.7.1 (this should be it now I hope) 😄 on the mcl-wasm package without the nyc dependency so you can directly take this in here once you are comfortable that other things are working

@rumkin
Copy link
Contributor Author

rumkin commented Nov 16, 2020

@holgerd77 Thanks for reviewing it so quickly!

  1. Wrong linked issue fixed.
  2. I've tested this with mcl-wasm v0.7.0. Now I've updated this to 0.7.1. So npm audit made with fixed dependencies records in mcl-wasm. I'll commit the latest version of the lib in this PR.

@holgerd77
Copy link
Member

Short informational note: I've just merged #953 from @evertonfraga which is bringing the ethereumjs-testing dependency (respectively mainly the ethereum/tests git submodule) into the monorepo. In case if you haven't followed this, if you want to update/rebase your branch on this you need to run:

cd ethereumjs-vm
npm run bootstrap
git submodule init
git submodule update (takes a while since several 100MB of package size)

@rumkin rumkin force-pushed the fix-vulnerable-deps branch from d951949 to 98e20f4 Compare November 16, 2020 10:05
@rumkin
Copy link
Contributor Author

rumkin commented Nov 16, 2020

Rebased changes from #953 through master branch.

@holgerd77
Copy link
Member

@rumkin have given this a "WIP" label, please change to "PR: Ready for review" label once you consider this ready and - important - also drop a note here, since this label change - won't trigger a separate notification by GitHub.

@rumkin
Copy link
Contributor Author

rumkin commented Nov 16, 2020

@holgerd77 Done. It's ready now. I think someone need to reproduce audit independently. I've described actions to be made in the comment to original issue.

Copy link
Member

@holgerd77 holgerd77 left a comment

Choose a reason for hiding this comment

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

LGTM, can confirm the vulnerabilities on the VM going to 0 on a packaged install, thanks Paul!

@holgerd77 holgerd77 merged commit 7765fba into master Nov 16, 2020
@holgerd77 holgerd77 deleted the fix-vulnerable-deps branch November 16, 2020 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Run npm audit checks before v5 rc.1 Releases
2 participants