Skip to content

Update JSDOM & ESLint to resolve security advisory#8677

Merged
aduth merged 15 commits intomainfrom
aduth-upgrade-eslint-jsdom-optionator
Jun 29, 2023
Merged

Update JSDOM & ESLint to resolve security advisory#8677
aduth merged 15 commits intomainfrom
aduth-upgrade-eslint-jsdom-optionator

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Jun 28, 2023

🛠 Summary of changes

Updates JSDOM & ESLint dependencies (and subdependencies) to resolve a security advisory related to the word-wrap dependency.

Upgrading the top-level dependencies was enough to resolve JSDOM. Upgrading ESLint did not automatically force an update to its optionator dependency, where optionator@0.9.3 swaps the offending package with an alternative, patched version. Removing the optionator block from yarn.lock and rerunning yarn install was enough to force the update.

📜 Testing Plan

The audit_yarn_package exits with a successful exit code:

make audit_yarn_package
echo $?
# 0

@aduth
Copy link
Contributor Author

aduth commented Jun 28, 2023

Upgrading JSDOM will require more work due to some issues with how we manipulate window.location that appears to have changed in newer versions (related issue). It will be nice to upgrade since it drops the dependency chain altogether (escodegen > optionator > word-wrap), but since at least the optionator package is in common, we can update to the newer version of that to resolve.

@aduth
Copy link
Contributor Author

aduth commented Jun 28, 2023

Actually, that might not be true, since escodegen uses an older, unpatched version of optionator 😖

@aduth aduth marked this pull request as draft June 29, 2023 14:28
@aduth aduth force-pushed the aduth-upgrade-eslint-jsdom-optionator branch from e384a2b to 494afd2 Compare June 29, 2023 14:59
@aduth aduth marked this pull request as ready for review June 29, 2023 15:23
@aduth
Copy link
Contributor Author

aduth commented Jun 29, 2023

I managed to work through the JSDOM upgrade, mostly by using a dependency injection approach to parameterizing the redirect logic as an alternative to stubbing the window values directly. This should be ready for another review.

Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM

@aduth aduth merged commit ada558e into main Jun 29, 2023
@aduth aduth deleted the aduth-upgrade-eslint-jsdom-optionator branch June 29, 2023 20:48
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