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: Upgrade Cheerio to patch security vulnerability #228

Merged
merged 1 commit into from Sep 16, 2022

Conversation

Tyharo1
Copy link
Contributor

@Tyharo1 Tyharo1 commented Sep 12, 2022

Upgraded cheerio to latest to resolve ReDos security vulnerability introduced by nth-check@1.0.2.

Vulnerability details can be found here
The dependency chain that introduced this issue is as follows: ember-svg-jar@2.3.4 > cheerio@0.22.0 > css-select@1.2.0 > nth-check@1.0.2

The latest version Cheerio no longer requires css-select.

Note: I ran yarn-deduplicate (package) to clean up the yarn lock file since the project is running yarn v1, this is why there are a few more changes in the yarn-lock files than normal.

@Tyharo1
Copy link
Contributor Author

Tyharo1 commented Sep 12, 2022

@jherdman It appears you're most active on this repo. If you have some time for a review it would be greatly appreciated! If you have any questions feel free to ask away 😃

@jherdman
Copy link
Collaborator

Don't take this the wrong way, but I'm kind of hoping this fails 😆 . I tried to upgrade this dependency in the past and ran into a pile of headaches. Worse yet, I can't seem to find an alternative package that covers our needs.

Fingers crossed this works and I was just too boneheaded to make it work!

@Tyharo1
Copy link
Contributor Author

Tyharo1 commented Sep 12, 2022

Is there an easy way to rerun a job? Kinda odd the windows-latest had a timeout, want to ensure its not just a one off network issue.

@jherdman
Copy link
Collaborator

Yeah, you have a few options:

  1. You can push up an empty commit
  2. Amend your commit and force push

The failure with broc-svg-optimizer is legit though. I had attempted to remove a bunch of these tests in #221 because I don't feel it's our job to smoke test SVGO for breaking changes. I didn't chase down why this wasn't working.

@jherdman
Copy link
Collaborator

Oops! Thought of a third option. Re-running.

@Tyharo1
Copy link
Contributor Author

Tyharo1 commented Sep 12, 2022

Oops! Thought of a third option. Re-running.

I must not have permission to see the re-run button. No worries though, I'll work around it. I thought the broc-svg-optimizer failure was legit but just thought the timeout was odd haha.

@Tyharo1
Copy link
Contributor Author

Tyharo1 commented Sep 13, 2022

@jherdman sorry to bother you again but any chance you would know why the CI tasks didn't kick off after my last commit amend? Trying to see if maybe the yarn-deduplicate maybe causing the dependency install timeout. I rolled back to the main branch and just upgraded cheerio, nothing else.

@jherdman
Copy link
Collaborator

I have to approve and run the workflows. I'll kick it off now. Don't be shy about pinging me on this matter. ❤️

@Tyharo1
Copy link
Contributor Author

Tyharo1 commented Sep 13, 2022

@jherdman Looks like the broccoli and embroider-optimized tests are currently failing today in the main branch. It looks like the ember-cli-fastboot upgrade resolved some currently failing CI tests though! I was using your latest merge as here as a reference for "normal" test failures.

@jherdman
Copy link
Collaborator

I'll give this a whirl tomorrow to smoke test. I think I'm okay with the fails as is. We can sort those out later.

@jherdman
Copy link
Collaborator

I'm happy to bless and merge this PR. Many thanks @Tyharo1 for your work! ❤️

@Tyharo1
Copy link
Contributor Author

Tyharo1 commented Sep 19, 2022

@jherdman Thanks for the help getting this tested and reviewed! 🥇

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.

None yet

2 participants