beefproject / beef

The Browser Exploitation Framework Project
https://beefproject.com
9.67k stars 2.14k forks source link

Decide on a rubocop version and Resolve Rubocop breaches #1978

Open jcrew99 opened 4 years ago

jcrew99 commented 4 years ago

This is a ticket to decide on a rubocop version to use and then to track progress on resolving the breaches. This will require a large amount of work to view, debug and try and resolve the changes. On -v 0.54.0 i get 20097 breaches... and on -v 0.88.0 i get 21430 breaches so i would like to try and action this.

Lets go with the current .rubocop.yml. I believe we should go with the current release of rubocop -v 0.8.8, use that as a baseline and stick with it. If we need to update we will. The -v 8.8. works with ruby 2.4+ which matches BeEF's requirements. We would add the following to the gemfile: gem 'rubocop', '~> 0.88.0', require: false

I can then action the breaches and focus on removing them. @bcoles, @jackdwalker, @DeezyE Do you have any thoughts or things to keep in mind? Any concerns about using 0.88.0? Anything we should do first?

I'll make the pull request soon with the change.

jackdwalker commented 4 years ago

I'll have a look through - no thoughts off the top of my head @jcrew99. Actually think this a is a great project. Set's a good baseline for acceptable coding practices for the project which I think is really important for the future of BeEF.

jcrew99 commented 4 years ago

1985

jcrew99 commented 3 years ago

1986 was merged, gonna try and do v0.0.2 tomorrow

bcoles commented 3 years ago

The default Rubocop config + Metasploit's might be a good place to start.

https://github.com/rapid7/metasploit-framework/blob/master/.rubocop.yml

jcrew99 commented 3 years ago

updated the .rubocop file a bit to match bcoles suggestion above. Definitely more work to be done there but should improve the cops a bit. Feel free to review and let me know your thoughts if you can. I intend to merge with some more breach fixed and updates.

DeezyE commented 2 years ago

@jcrew99 where did you end up with this?

bcoles commented 2 years ago

I believe we should go with the current release of rubocop -v 0.8.8, use that as a baseline and stick with it. If we need to update we will.

We should keep rubocop up to date with the latest version. We'll get pestered by Dependabot to update anyway.

For reference, we're currently using the latest version at time of writing:

https://github.com/beefproject/beef/blob/dd825469980885aa10df557775b9a91c931e6785/Gemfile.lock#L285 https://github.com/beefproject/beef/blob/dd825469980885aa10df557775b9a91c931e6785/Gemfile#L26

@jcrew99 where did you end up with this?

We've implemented rubocop and explicitly disabled a handful of rules. The disabled rules are almost entirely subjective. ie, cyclomatic complexity, method length, etc. Ideally we should resolve these in the future, where possible. Alternatively, we could sets the threshold very high like we do with LineLength (this should also be reviewed).

https://github.com/beefproject/beef/blob/e75f5a87c20b1301855e63d505c87b1312507575/.rubocop.yml#L10-L12

The entire code base still needs to be updated to comply. The vast majority of this work can be done easily with rubocop -a. However, the changes will need to be reviewed and tested.

I suggest performing the updates piecemeal on one segment of the code base at a time. Preferably an area of the codebase that the person performing the update is familiar with.

bcoles commented 2 years ago

The vast majority of Rubocop violations have been resolved in:

There are currently ~8,000 violations remaining. (My working repo contains some private stuff so that number isn't 100% accurate)

580 files inspected, 8205 offenses detected, 7547 offenses auto-correctable

However, 6,560 of these exist within the Console extension (which hasn't been supported in years) due to the presence of some outdated libraries in extensions/console/lib/.

# rubocop extensions/console/lib/ | tail -n 1
7 files inspected, 6560 offenses detected, 6264 offenses auto-correctable

The remaining violations will require manual review and in many instances will require significant refactoring or architectural changes.