semgrep
Remove Outdated Rails Rules
Hi folks,
This PR removes some ruby/rails rules we feel are no longer relevant. We maintain a fork of semgrep-rules internally that allowed us to remove these without waiting for upstream changes, but it was suggested to me that I could also open a PR 🙂
- filter-skipping: This rule is meant to detect dynamically generated routes that with specially crafted inputs could allow the client to access other routes, bypassing any filters on them. See this mailing list post from 2011. This only affects two specific versions of Rails which have long since been patched and EOL'd. Brakeman is able to continue including this rule because they do a runtime check of which version of Rails it's running under.
- attr-accessible: This rule checks for models that don't explicitly list which of its parameters can be modified by an external caller. This was a big deal back in 2012 when mass assignment bugs were everywhere (see this classic blog post), but now we've got strong params which moves this enforcement into the controller level in modern rails apps.
I suspect there are other similar rules in this directory that could go this route but I haven't run into them yet 🙂
These rules are really important to us on our Rails versions (with Rails LTS), i think it's important that we find a way to keep them for companies that do still use older Rails versions
Hey @nightpool, that's fair. In that situation rules like these should be opt-in, as opposed to opt-out. e.g. if you need to support a 13 year old version of Rails, you should pull in specific rules as opposed to everyone else being subjected to irrelevant findings 🙂
Well, as an overworked team lead with a lot on my plate (and like 15 GitHub repositories to audit), I'd honestly just appreciate it if Semgrep could figure out my rails version for me 😅
For now we're going to close this PR since we can't determine the version and people still depend on it
People can exclude the rules if they do not want to use them
