fguillen / simplecov-rcov

SimpleCov formatter to generate a simple index.html Rcov style
103 stars 57 forks source link

Vulnerability CVE-2020-8130 exposed in rake dependency #28

Closed dhempy closed 8 months ago

dhempy commented 2 years ago

simplecov-rcov depends on rake.

Rake < 12.3.3 has critical vulnerability that some code inspectors (AWS Inspector) flag as "critical" and blocks deployment.

This PR https://github.com/fguillen/simplecov-rcov/pull/27 fixes the vulnerability by bumping the version of rake.

Please review and approve that PR to allow more people to continue using your awesome gem, @fguillen :)

fguillen commented 2 years ago

Hi @dhempy,

PR accepted and merged. gem version bumped to rubygems.

As you mention in the PR comment, the major version bump in simplecov-rcov could be arguably changed by a minor version bump.. I am satisfied just seeing you also have doubts. It is Sunday and it is not worth it the energy waste in the discussion :)

Thanks for keeping the gem alive and safe!

dhempy commented 2 years ago

As I learn more about gem dependencies, I realize I didn't solve this is the best way. According to https://yehudakatz.com/2010/12/16/clarifying-the-roles-of-the-gemspec-and-gemfile/ , gems (unlike applications) should not include Gemfile.lock at all. I'll submit a fresh PR to follow that path. This will not only resolve this vulnerability today, but will make simplecov-rcov far more resilient into the future without frequent updates.

dhempy commented 2 years ago

I added two PR's, which must be merged separately, and in this order:

pboling commented 1 year ago

@dhempy Actually that post is no longer valid, and best practice has been significantly improved since then. It is now standard, and best, practice to commit Gemfile.lock in every project that has one, with no exceptions. This post really should be updated, but Yehuda has moved on to other things.

Here is the discussion of the current best practice. Rails now includes Gemfile.lock!

fguillen commented 8 months ago

I close it due to the changes in the community best practices

pboling commented 8 months ago

You did merge those PRs removing the Gemfile.lock though, so the Gemfile.lock is not in the project. It isn't a big deal, just want to make sure it is clear what happened.

fguillen commented 8 months ago

Thanks @pboling I add it again, I hope I am not breaking anything now