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

Resolves async to v3.2.3 #1449

Merged
merged 1 commit into from
Apr 15, 2022
Merged

Resolves async to v3.2.3 #1449

merged 1 commit into from
Apr 15, 2022

Conversation

tmarkley
Copy link
Contributor

Description

  • Resolves older versions of async: v0.9.2, v1.5.2, v2.6.3
    • There are known breaking changes, so this manual resolution carries some risk. The mitigation is testing.
    • CHANGELOG
  • Addresses CVE-2021-43138.
  • This requires a manual resolution because @elastic/makelogs@6.1.0, grunt-contrib-clean@2.0.0, ejs@3.1.6, and webpack-dev-server@4.8.1 have downstream dependencies on older versions of async and none of them have newer versions to fix this. I've documented this in Remove unnecessary resolutions from package.json #1298.
  • Bumps getos from v3.1.0 to v3.2.1.
  • Bumps @elastic/makelogs from v6.0.0 to v6.1.0.
  • Bumps archiver from v3.1.1 to v5.3.0 and @types/archiver from v3.1.0 to v5.3.1.
    • Used in the build script, which runs successfully.
    • Breaking changes include removing support for older versions of Node.js as well as no longer supporting absolute path glob patterns, which are not used in our repository.
    • CHANGELOG
  • Bumps webpack-dev-server from v4.7.4 to v4.8.1.
  • Removes grunt-contrib-watch dependency since it is unused.
  • Removes unnecessary @types/react resolution.

Issues Resolved

Resolves #1440

Testing

Ran the build script with no flags as well as with the --release flag and both built successfully.

Check List

  • New functionality includes testing.
    • All tests pass
      • yarn test:jest
      • yarn test:jest_integration
      • yarn test:ftr
  • New functionality has been documented.
  • Commits are signed per the DCO using --signoff

@tmarkley tmarkley added dependencies Pull requests that update a dependency file v2.0.0 cve Security vulnerabilities detected by Dependabot or Mend labels Apr 13, 2022
@tmarkley tmarkley requested a review from a team as a code owner April 13, 2022 15:44
* Resolves older versions: v0.9.2, v1.5.2, v2.6.3
  * There are known breaking changes, so this manual resolution carries
    some risk.
  * [CHANGELOG](https://github.com/caolan/async/blob/v3.2.3/CHANGELOG.md)
* Addresses CVE-2021-43138.
* This requires a manual resolution because `@elastic/makelogs@6.1.0`,
  `grunt-contrib-clean@2.0.0`, `ejs@3.1.6`, and
  `webpack-dev-server@4.8.1` have downstream dependencies on older
  versions of `async` and none of them have newer versions to fix this.
* Bumps `getos` from v3.1.0 to v3.2.1.
* Bumps `@elastic/makelogs` from v6.0.0 to v6.1.0.
* Bumps `archiver` from v3.1.1 to v5.3.0 and `@types/archiver` from
  v3.1.0 to v5.3.1.
  * Used in the build script, which runs successfully.
  * Breaking changes include removing support for older versions of
    Node.js as well as no longer supporting absolute path glob patterns,
    which are not used in our repository.
  * [CHANGELOG](https://github.com/archiverjs/node-archiver/blob/5.3.0/CHANGELOG.md)
* Bumps `webpack-dev-server` from v4.7.4 to v4.8.1.
* Removes `grunt-contrib-watch` dependency since it is unused.
* Removes unnecessary `@types/react` resolution.

Resolves opensearch-project#1440

Signed-off-by: Tommy Markley <markleyt@amazon.com>
@kavilla
Copy link
Member

kavilla commented Apr 13, 2022

Since code freeze for rc1 is a few days away did want to hold of on this mitigation until post rc1? Or look into the impact to plugins and if it's going to cause a breakage for them?

@tmarkley
Copy link
Contributor Author

Since code freeze for rc1 is a few days away did want to hold of on this mitigation until post rc1? Or look into the impact to plugins and if it's going to cause a breakage for them?

@kavilla good question, I think we should try to get it in to give us more time to address any problems that may come up. If it ends up being a blocker for RC1 we can revert it, but we need to address the CVE either way.

@kavilla
Copy link
Member

kavilla commented Apr 13, 2022

@kavilla good question, I think we should try to get it in to give us more time to address any problems that may come up. If it ends up being a blocker for RC1 we can revert it, but we need to address the CVE either way.

I think the CVE was open 4 days ago? I'd rather be more defensive than reactive here unless we know we won't break plugins since the current turn around time from us checking something and plugins reacting is about a week.

@tmarkley
Copy link
Contributor Author

I think the CVE was open 4 days ago? I'd rather be more defensive than reactive here unless we know we won't break plugins since the current turn around time from us checking something and plugins reacting is about a week.

That should give us another reason to push us to resolve these CVEs sooner rather than later, right? We're less than a month away from 2.0, and our goal should be to resolve any CVEs that are found until ~2 weeks before the release date. We can't be walking on eggshells because plugins have a delayed reaction to every change; security should be our top priority.

@kavilla
Copy link
Member

kavilla commented Apr 13, 2022

That should give us another reason to push us to resolve these CVEs sooner rather than later, right? We're less than a month away from 2.0, and our goal should be to resolve any CVEs that are found until ~2 weeks before the release date. We can't be walking on eggshells because plugins have a delayed reaction to every change; security should be our top priority.

I don't think it's walking on eggshells as much as I'd rather we know that this PR won't break plugins days before they should be code frozen for 2.0-rc1. And if we do know it's going to break plugins and needs to go into 2.0-rc1 then we should be raising the red flag that 4/18 will be unlikely.

@tmarkley
Copy link
Contributor Author

I don't think it's walking on eggshells as much as I'd rather we know that this PR won't break plugins days before they should be code frozen for 2.0-rc1. And if we do know it's going to break plugins and needs to go into 2.0-rc1 then we should be raising the red flag that 4/18 will be unlikely.

I think this is a good call-out. I see that some plugin changes are going to occur after RC1 anyways, so plugins should be able to stay on top of anything that needs to be addressed because of this PR.

Copy link
Member

@kavilla kavilla left a comment

Choose a reason for hiding this comment

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

Blocking but if we ensure this doesn't break plugins for rc-1 then I'm okay to merge this in.

@tmarkley tmarkley dismissed kavilla’s stale review April 15, 2022 16:37

We need to merge these CVE PRs in for RC1 to address any breaking changes.

Copy link
Member

@ananzh ananzh left a comment

Choose a reason for hiding this comment

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

LGTM

@tmarkley tmarkley merged commit b854a13 into opensearch-project:main Apr 15, 2022
@tmarkley tmarkley deleted the async branch April 16, 2022 16:41
ananzh added a commit to ananzh/OpenSearch-Dashboards that referenced this pull request Oct 10, 2022
Addresses CVE-2021-43138.

backport PR:
opensearch-project#1449

Signed-off-by: Anan Zhuang <ananzh@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cve Security vulnerabilities detected by Dependabot or Mend dependencies Pull requests that update a dependency file v2.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CVE-2021-43138 (High) detected in multiple libraries
4 participants