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: Fixing one instance of async vulnerability #1142

Merged
merged 2 commits into from
Apr 19, 2022
Merged

Conversation

jlperkins
Copy link
Contributor

@jlperkins jlperkins commented Apr 19, 2022

Details

This PR partially addresses this dependabot alert about using a vulnerable version of "async." Where possible, it updates the dependencies we have that rely on async and bumps them to use a current, secure version of the package. For the case where this is not possible, we are choosing to resolve this dependabot alert without fixing them, as the amount of work required to make a fix is not logical for the low risk that this vulnerability poses for us.

This dependency was updated by removing and re-adding the async@^2.0 entry of the lockfile, since our caret version, when refreshed, would bump up to a secure version of async (2.6.4).

Motivation

Keeping dependencies up-to-date.

Context

This change leaves a few dependencies on async at lower-than-secure versions. Secure versions of async are specifically 2.6.4, and then 3.2.3 and above. However, there are other dependencies of ours which rely on things which rely on things (and so on)... which rely on async at a vulnerable version. Namely, tfx-cli at its latest version relies on async ^1.4.0. Additionally, tfx-cli has a dependency on prompt, which relies on async ~0.9.0, and which also has its own dependency on winston 2.x, which uses async ~1.0.0.

Options considered:

  • Get prompt to use the 3.x version of winston, which relies on an up-to-date version of async. This open issue indicates that the prompt repo owner doesn't intend to do that. That issue and this other issue suggest that the maintainer is also considering archiving the project generally. Looking into winston's changelogs confirm that the work to get prompt to be able to accept a high enough version of winston is significant and not likely something our team wants to take on.
  • Get the 2.x version of winston to rely on a safe version of async. From looking at async's changelogs, this is also a non-trivial amount of work.
  • Tfx-cli also has an open issue to upgrade its dependencies, but since there are other older asks for tfx-cli to update away from vulnerable dependencies that haven't been addressed, and generally the tfx repo doesn't see very frequent changes, it is unlikely that someone else will take care of this for us.
  • Investigation into how tfx-cli is used in our repo shows that it is only a dev dependency for us, and that the input is not user-controlled. Since the dependabot alert is about prototype pollution, this is quite low-risk for us. So, that gives us the option to accept that limited risk and close out the dependabot alert without making any other changes besides the one in this PR; this is the option we are going with.

Pull request checklist

  • [n/a] Addresses an existing issue: Fixes #0000
  • [n/a] Added relevant unit test for your changes. (yarn test)
  • [n/a] Verified code coverage for the changes made. Check coverage report at: <rootDir>/test-results/unit/coverage
  • Ran precheckin (yarn precheckin)

@codecov-commenter
Copy link

Codecov Report

Merging #1142 (f1146ba) into main (b0f8fb5) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1142   +/-   ##
=======================================
  Coverage   90.66%   90.66%           
=======================================
  Files          59       59           
  Lines        1446     1446           
  Branches      172      172           
=======================================
  Hits         1311     1311           
  Misses        135      135           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b0f8fb5...f1146ba. Read the comment docs.

@jlperkins jlperkins marked this pull request as ready for review April 19, 2022 18:46
@jlperkins jlperkins requested a review from a team as a code owner April 19, 2022 18:46
@jlperkins jlperkins merged commit 93fe067 into main Apr 19, 2022
@jlperkins jlperkins deleted the dependabot-alert branch April 19, 2022 19:06
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.

3 participants