Having been a member of the Ruby on Rails community for over 13 years, I’ve seen a lot of application code bases. Starting in 2007, our team began offering Rails Code Audits to entrepreneurs who wanted to get a third-party assessment of their investment.
We have an ever-evolving lengthy list of items to evaluate in every Rails application we audit. At the end of the code audit, our clients receive a document that walks them through our findings with a list of recommended next steps.
In this article, I’m going to share eight of those items (and the rationale behind them) that we check on when we begin auditing a code base. My aim is to help provide you with a list of things to check on and consider cleaning up as a way to tidy up some possible technical debt and/or oversights.
Without further ado. Here are eight things you should check on a code base that doesn’t require you to know or learn a lot about the application’s business logic.
If some of these sound obvious, excellent. You’re on the right track. Keep up the good work.
1. Is there a .ruby-version file?
If you’re using a tool like rbenv or rvm, .ruby-version is a really helpful way of allowing a team of developers to ensure they’re all working off the same version of Ruby. Occasionally, we’ll come across an application that doesn’t specify the version of Ruby within the application’s code base and we have to either a) ask the developers to let us know and/or b) see what version is being run in production environment.
This information can (and should) also exist in the Gemfile, which we’ll touch on further in a moment.
If the file doesn’t exist, then it’s possible that you’re not using rbenv or rvm, which you may want to consider. (We prefer rbenv.)
2. How does the Gemfile look?
How many of you remember managing Rubygem versions prior to Bundler? UFTA. I bet it’s been more than five years since we performed a code audit on a Ruby on Rails application that didn’t include a Gemfile. Glad those days are far behind us.
Having said that, here are a few things that we’d recommend for keeping your Gemfile nicely organized and documented for your peers.
Is the version of Ruby documented at the top?
While we recommend having this in your .ruby-version, we also encourage you to specify this in your Gemfile, too.
Is Ruby on Rails locked to a specific version?
Unless you’re working on a brand new application, your Gemfile should have a locked version of rails specified. The last thing you want to do is accidentally upgrade your Ruby on Rails application during a deployment without having tested it first.
Have you specified versions across all of your gems?
Recently, we inherited a Ruby on Rails application that hadn’t been worked on in nearly six months. It also has poor automated test coverage and the Gemfile didn’t specific versions on most of the required gems. On one of our first changes, I made a small copy change to part of the application, which looked a-okay on development. When I deployed to staging, we quickly noticed that an unrelated--yet primary--feature of their application that dealt with file uploads was not working. It wasn’t caught by the limited test coverage.
It took several hours to track down how it might have broke given that no significant changes had been made in nearly half a year.
What we found was that when the deployment took place, a number of Rubygems had automatically been upgraded. So, we had to compare what Rubygems were installed on our production and staging servers to determine the version differences. We went through each of them, one-by-one, to specify the known working versions on production in our Gemfile. Through this, we determined that one gem (s3_direct_upload) was the culprit as they had changed some configuration syntax at some point.
This is just one example of how this type of version change can bite your future self (and/or another team) in the ass one day.
As a general rule, we try to lock our Rubygems to micro-releases with the following syntax. For example: gem "zencoder", "~> 2.5"
Our development team agreed that we’d like to manually change major versions and allow Bundler to handle micro version updates, automatically.
Are your gems organized in groups?
There isn’t anything wrong with having all your gems installed in every environment. However, there are likely some gems that are only going to 1) take up more disk space and 2) slow your deployments down if you install them in staging and/or production environments. For example, you don’t likely need to install rpsec-rails, cucumber, timecop, database-cleaner, or capybara-webkit on your production servers.
Learn more about Gemfile groups.
What are all these gems used for?
As you scan your gem dependencies, do you remember what they’re all used for? Do any of them seem unfamiliar?
For example, looking at a client’s Gemfile, I find myself asking, “What the hell is the launchy gem? What is the wicked gem used for?” There's no documentation here that indicates what they are being used for. My solution is to have to google them and/or search through the codebase to try and ascertain it.
As a best practice, our team has agreed that if you find yourself having to look up what an uncommon gem provides the application, to add a short note to the Gemfile for future reference. Be kind to your future self and/or peers.
Last but not least, is Rubygems installing over SSL?
Ensure that your Gemfile has:
source 'https://rubygems.org' (https:// vs http://)
3. What warnings does bundler-audit raise?
I recommend installing bundler-audit, which is a Ruby gem that will quickly list out which gem versions specified in your Gemfile.lock have known security vulnerabilities patched. It will also provide you with a suggested version to upgrade to.
Running bundler-audit will give you feedback like this:
$ bundle-audit
{code}
bundle-audit
Name: actionpack
Version: 4.2.6
Advisory: CVE-2015-7576
Criticality: Unknown
URL: https://groups.google.com/forum/#!topic/rubyonrails-security/ANv0HDHEC3k
Title: Timing attack vulnerability in basic authentication in Action Controller.
Solution: upgrade to ~> 5.0.0.beta1.1, ~> 4.2.5.1, ~> 4.1.14.1, ~> 3.2.22.1
Name: actionpack
Version: 4.2.6
Advisory: CVE-2015-7581
Criticality: Unknown
URL: https://groups.google.com/forum/#!topic/rubyonrails-security/dthJ5wL69JE
Title: Object leak vulnerability for wildcard controller routes in Action Pack
Solution: upgrade to ~> 4.2.5.1, ~> 4.1.14.1
Name: actionpack
Version: 4.2.6
Advisory: CVE-2016-0751
Criticality: Unknown
URL: https://groups.google.com/forum/#!topic/rubyonrails-security/9oLY_FCzvoc
Title: Possible Object Leak and Denial of Service attack in Action Pack
Solution: upgrade to ~> 5.0.0.beta1.1, ~> 4.2.5.1, ~> 4.1.14.1, ~> 3.2.22.1
Name: actionview
Version: 4.2.6
Advisory: CVE-2016-0752
Criticality: Unknown
URL: https://groups.google.com/forum/#!topic/rubyonrails-security/335P1DcLG00
Title: Possible Information Leak Vulnerability in Action View
Solution: upgrade to ~> 5.0.0.beta1.1, ~> 4.2.5.1, ~> 4.1.14.1, ~> 3.2.22.1
Name: activemodel
Version: 4.2.6
Advisory: CVE-2016-0753
Criticality: Unknown
URL: https://groups.google.com/forum/#!topic/rubyonrails-security/6jQVC1geukQ
Title: Possible Input Validation Circumvention in Active Model
Solution: upgrade to ~> 5.0.0.beta1.1, ~> 4.2.5.1, ~> 4.1.14.1
Name: activerecord
Version: 4.2.6
Advisory: CVE-2015-7577
Criticality: Unknown
URL: https://groups.google.com/forum/#!topic/rubyonrails-security/cawsWcQ6c8g
Title: Nested attributes rejection proc bypass in Active Record
Solution: upgrade to ~> 5.0.0.beta1.1, ~> 4.2.5.1, ~> 4.1.14.1, ~> 3.2.22.1
Name: devise
Version: 3.4.1
Advisory: CVE-2015-8314
Criticality: Unknown
URL: http://blog.plataformatec.com.br/2016/01/improve-remember-me-cookie-expiration-in-devise/
Title: Devise Gem for Ruby Unauthorized Access Using Remember Me Cookie
Solution: upgrade to >= 3.5.4
Name: rack-attack
Version: 4.2.0
Advisory: 132234
Criticality: Unknown
URL: https://github.com/kickstarter/rack-attack/releases/tag/v4.3.1
Title: rack-attack Gem for Ruby missing normalization before request path
processing
Solution: upgrade to >= 4.3.1
..etc
4. How long has it been since automated tests have been touched?
Before we attempt to run the tests — we want to know how long it has been since a developer has made a change in the test and/or spec directories. Let’s look at the last commit via git.
$ git log --pretty -1 spec/` # and/or `test/`, too
Is that date quite a while ago (compared to work done in app/)?
It’s an indicator that automated testing has not been prioritized. Anecdotally speaking, when we see that tests have been ignored for a long period of time, that the developers seem to fall into one or more of the falling categories.
- The original developer(s) didn’t have many tests; they didn’t get around to adding more of them.
- They upgraded Ruby and/or Rails and the test suite started failing; they didn’t get around to fixing them.
- Solo developer on a project who doesn’t have any peers to work with and feels confident enough that their code “works” as they tested it themselves in a browser.
- The stakeholders haven’t had much budget so the developer has been trying to keep updates quick and dirty--and sees writing tests as “extra” costs (we would argue that they’re still having to manually test, regardless).
I’ve been guilty of skipping the step of writing automated tests from time-to-time. I get it. However, we can’t make this into a long-term bad habit. When you work on projects by yourself, it’s far easier to skip these as you’re not accountable to a teammate.
Regardless, save yourself a bunch of manual browser testing and write automated tests, Robby.
5. Are there security keys that shouldn’t be in the repository?
Look in the config/ directory. Do you see any YAML files? Look through them, do you find staging and/or production credentials? (access keys, usernames, passwords, etc.)
For example, config/environments/production.rb … how many passwords/keys do you see in there?
They shouldn’t be.
These only belong on the server environments and in a password manager vault. We use 1Password for Teams at Planet Argon so that we can control who has access to those.
How do you resolve this?
- Add these files directly to your servers. For example, you might upload those YAML files to your application’s shared/ directory. If it’s a staging environment, don’t have the production environment’s details in there. (and vice versa)
- Note: if you’re using an environment like Heroku, you can move these to config environment variables.
Remove them from your git repository and history. Here's how.
For good measure, you should now go update all of those credentials. We recommend updating your keys and passwords on a quarterly basis.
6. Can we seed a local database?
Before you run the application locally, you might need to load some seed/test data into your local database. Ruby on Rails provides you with some conventions on how to load that up via a db/seeds.rb file that you can run a rake task to load. Learn more about seed data in Rails.
Does anything exist in db/seeds.rb? If not, what data are you relying on? Do you have to manually load data or are you leaning on a production database backup?
If the former, we’d encourage you to take the time to work on building seed data that can be loaded, deleted, and re-loaded.
If the latter, you’re playing with fire. Do you really need to load up everything to work on the code base? Often times, we see the combination of lack of test coverage and no seed data go together…and the developers have been working off of production in a local environment. We’d advocate that you only do that in rare situations and not allow it to become the norm.
7. Are there database indexes?
When we look at an existing database structure, we want to see if attention has been paid to indexes. For example, we can compare the number of foreign keys to the number of indexes with the following.
Approximately, how many foreign keys are there?
$ grep -v 'add_index' db/schema.rb | grep '_id' | wc -l
99
How many indexes are there?
$ grep 'add_index' db/schema.rb | wc -l
95
Of course, foreign keys are not the only thing that you should have an index on…but if these two numbers are drastically different, it’s a hint that they’ve been overlooked in the past.
Quite often, we see a decent performance boost once we mitigate this scenario. Feel free to continue not using indexes because it makes us look good when we show a new client how much quicker we got their application to run. ;-)
8. To-dos: What remains to be done?
I want to leave you with one more quick way to get a sense of what technical debt may exist in the application. Run the following command and see if anything amusing returns:
$ grep -ir 'todo' app/*
These comments are often meant to be temporary reminders to tidy some code up at a later point in time and/or do some further research to figure out what something is.
Quite often, they get forgotten about.
Are these things that should be worked on? Removed?
Closing Thoughts
Regardless of whether your code base is only 2k LOC or 100k LOC, these are a few things that you should consider keeping an eye on within your Ruby on Rails application. They aren’t things that will make or break your application, immediately, but are some best practices that our team has found long-term value in following.
They’re also some of the most common low-hanging fruit recommendations that we make when performing a Ruby on Rails code audit. Of course, we also dive into a number of lower-level details in applications but these are a sampling of things that can benefit yourself and your future teammates.
Do you disagree with any of my recommendations? Do you have any to add? Leave a comment below and let me know.
Top comments (0)