Skip to content
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

fix(security): address CVE-2021-23337 #1820

Merged

Conversation

aldousalvarez
Copy link
Contributor

@aldousalvarez aldousalvarez commented Jan 28, 2022

Fixes #1778 Depends on #1775

Copy link
Contributor

@petermetz petermetz left a comment

Choose a reason for hiding this comment

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

@aldousalvarez Please resolve the conflicts and then the CI should run and we'll see if the changes work or not. I am a bit suspicious there though because you only updated the dependencies but not the imports in the code nor the usage of the methods (which may be fine if it's a drop-in replacement for all the methods that we use but I don't know that for sure either from the top of my head)

@petermetz
Copy link
Contributor

@aldousalvarez @zondervancalvez If this depends on #1816 (or the other way around) then please make sure to declare that dependency in the PR description so that the robot enforces the merge order correctly.

@aldousalvarez aldousalvarez force-pushed the aldousalvarez/issue1778 branch from 346bb36 to 65762dd Compare March 8, 2022 06:28
@aldousalvarez aldousalvarez force-pushed the aldousalvarez/issue1778 branch from 65762dd to 9c776b6 Compare March 8, 2022 06:30
@aldousalvarez aldousalvarez requested a review from petermetz March 8, 2022 06:31
@aldousalvarez aldousalvarez force-pushed the aldousalvarez/issue1778 branch 3 times, most recently from d4354d6 to e271d6c Compare March 10, 2022 09:05
@petermetz petermetz requested review from izuru0 and takeutak and removed request for jonathan-m-hamilton March 11, 2022 02:26
Copy link
Contributor

@petermetz petermetz left a comment

Choose a reason for hiding this comment

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

@aldousalvarez Please make sure to rebase onto the parent PR once the conflicts are fixed there. Then also make sure that the tests here are also passing.

@aldousalvarez aldousalvarez force-pushed the aldousalvarez/issue1778 branch 7 times, most recently from 0d428cb to 60f7292 Compare March 16, 2022 02:19
@aldousalvarez aldousalvarez force-pushed the aldousalvarez/issue1778 branch from 60f7292 to ee8f703 Compare March 28, 2022 07:21
@aldousalvarez aldousalvarez requested a review from petermetz March 30, 2022 05:25
@petermetz petermetz closed this Mar 30, 2022
@petermetz petermetz force-pushed the aldousalvarez/issue1778 branch from ee8f703 to ed71f42 Compare March 30, 2022 21:31
@petermetz
Copy link
Contributor

@aldousalvarez Please add a commit to this branch that updates the resolutions in the root package.json to specify >=4.17.21 as the lodash version

E.g., instead of this:

  "resolutions": {
    "ansi-html": ">0.0.8",
    "glob-parent": "5.1.2",
    "lodash": "4.17.20",
    "minimist": ">=1.2.6",
    "node-forge": ">=1.3.0",
    "underscore": "1.13.2"
  }

We need this:

  "resolutions": {
    "ansi-html": ">0.0.8",
    "glob-parent": "5.1.2",
    "lodash": ">=4.17.21",
    "minimist": ">=1.2.6",
    "node-forge": ">=1.3.0",
    "underscore": "1.13.2"
  }

Once you add that commit to this branch, the PR can be reopened and merged.

Copy link
Contributor

@petermetz petermetz left a comment

Choose a reason for hiding this comment

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

^^ See my above request

@aldousalvarez
Copy link
Contributor Author

Hello @petermetz I have committed the changes that you requested on to this branch aldousalvarez/issue1778. Will the PR be opened again? Thank you.

@petermetz petermetz reopened this Apr 2, 2022
@github-actions github-actions bot removed the dependent label Apr 2, 2022
@github-actions
Copy link

github-actions bot commented Apr 2, 2022

This PR/issue depends on:

Copy link
Contributor

@petermetz petermetz left a comment

Choose a reason for hiding this comment

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

@aldousalvarez Yes, I reopened it just now. LGTM

Fixes hyperledger-cacti#1778

Signed-off-by: aldousalvarez <aldousss.alvarez@gmail.com>
@petermetz petermetz force-pushed the aldousalvarez/issue1778 branch from a68fa49 to 4ce9723 Compare April 2, 2022 02:07
@petermetz petermetz merged commit eccef40 into hyperledger-cacti:main Apr 2, 2022
@petermetz petermetz deleted the aldousalvarez/issue1778 branch April 2, 2022 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fix(security): address CVE-2021-23337
3 participants