Thoughts on the Github Hack

Over the weekend, a young coder demonstrated a security vulnerability in github.com - one with wide-reaching implications. An early demonstration is at: https://github.com/rails/rails/issues/5239.

Our friend went on to make several updates to github as he experimented/demonstrated the vulnerabilities, got his account suspended, reinstated and set off a firestorm of criticism every which way.

I was ready to put it all into the "someone is wrong" pile, until I ran across this pull request on the Rails core: https://github.com/rails/rails/pull/4062.  That's some mid-90's Microsoft style arrogance right there, and on the off chance that anyone is having trouble seeing it, I figured I might add my breath to the maelstrom.

First of all: it was not right to hack github.  It's not okay to ignore the intent of security, no matter how weak the enforcement.  Much better to have pointed out the vulnerability to github, although for sure the fame wouldn't have been as bright (which is why I'm pointedly not referring to him by name in this post.)  Given github's track record, I think there's a pretty good chance they would have come clean, admitted the fault, as well as crediting its reporter.  But that's complete supposition.

That said, I don't think it's legitimate to consider Github a blameless victim.  The flaw in Rails that was exploited is well known, and well reported, and easy, if irritating, to fix.

The technical aside here is pretty simple.  In a file in config/initializers add:

ActiveRecord::Base.__send__(:attr_accessible, nil)

Then you need to white-list mass-assignable attributes in your models:

attr_accessible :name, :body, :whatever

And keep an eye on your logs for

WARNING: Can't mass-assign protected attributes: :blah

Which is a sign that you might need to add an entry for :blah into the respective model.

All pretty simple.  There are a couple of other notes, like "don't allow reference fields (e.g. :person_id) to be mass assigned" but that's the meat of it.  Put the initializer in your generator (that's much harder) and you never have to think about it again.

So, Github didn't put a simple, well-reported fix into their code.  Is that so bad?  I think so.  Github not only invited the developer community to trust them with the products of their labor, pretty much ousting SourceForge from that position in the process and firming up a development environment choice for open source work (i.e. "use git for version control"), it also invites developers to trust them with secrets.  Specifically, the secret contents of client repositories.  Heck, they get you to pay for the privilege.   So, in short, Github is taking money to keep secrets.  And by not covering a known security hole in a default Rails deploy, they were failing to uphold the trust of their paying customers.

I think Github was letting us down pretty badly.  I think an overzealous coder did a bad thing to bring that to light, but you can't argue that Github should be surprised or is blameless.  Two bad things, no one is blameless.

But the last straw for me was reading the Rails core teams' replies to a pull request to set the default for whitelisting attributes in Rails 4.0 to 'true.'  (After previous discussion concluded that making the change for 3.2 would be "too disruptive.")  That having to do attr_accessible for every model was "a lot of paperwork" - the final commend is @dhh's "I don't like this. -1"  Which is to say: we would rather put unsanitized data into the database than do the bare minimum of manual review.  And that's pretty lame.

Logical Tabs: a Rails helper for persistent HTML tab interfaces

logical_tabs is a Rails plugin that assists with the creation of a tabbed panel interface. It has a number of advantages over other solutions, but the primary one is that the tabs persist: if you reload or revisit a page with a tabbed panel, it will remember which tab you last had open on that page. There is a demo project available that shows how to use it with both ERB and HAML templates.

logical_tabs works with both Prototype and jQuery. Here's a screenshot of it in use on a social networking site:

It's still a little rough and I have future plans for it, but I'm using it in production on three sites so I figured I'd give it an early release while I'm hacking at RailsConf 2010.

Why build a new tool, when others exist?

There are already other solutions for building tabbed interfaces, most notably the jQuery UI Tabs tool. Another solution, which I had used previously, is railstabbedpanel.

The jQuery UI tool still requires you to construct all the HTML yourself which can be tedious, and it doesn't work for the large number of Rails products out there using Prototype rather than jQuery. railstabbedpanel provides HTML helpers, but the rendering code is heavily dependent on the specifics of the ActionView rendering/capturing code and so kept breaking when Rails was upgraded, or when HAML was upgraded. I found it very difficult to keep it working as I upgraded my toolset. Also, RTP's javascript was not unobtrusive.

Moreover, neither solution persists tab selections across page loads. So, I wrote this plugin which provides all of these advantages:

• Simple helpers to make it easy to build semantic HTML structures for the tabbed panels.
• Helper rendering that is isolated from the internals of ActionView and shouldn't break when Rails or HAML are upgraded.
• Javascript for both Prototype and jQuery environments
• Persistence of tab selection across page loads - revisit a page, get the same tab.
• Unobtrusive javascript.

Installing logical_tabs

logical_tabs is currently in version 0.6 and only supports Rails 2.x. I have only tested it in Rails 2.3.x, but it should work in 2.2 and 2.1 ... let me know if it doesn't.

Install the plugin:

> script/plugin install git://github.com/LRDesign/logical_tabs.git

Copy in the appropriate javascript file:

If your project uses Prototype:

> cp vendor/plugins/logical_tabs/javascripts/logical_tabs_prototype.js public/javascripts

If your project uses jQuery:

> cp vendor/plugins/logical_tabs/javascripts/logical_tabs_jquery.js public/javascripts

Copy stylesheets (optional)

logical_tabs includes starter stylesheets in both css and sass, you can find them in vendor/plugins/logical_tabs/stylesheets; copy the appropriate file to your public/stylesheets directory and include it in your application, or create your own.

Using the view helpers

logical tabs gives you a view helper, create_tabbed_panel, which takes a block with an argument - that argument will be a TabbedPanel object, the code for which can be found in vendor/plugins/logical_tabs/lib/logical_tabs/tabbed_panel.rb. This object is responsible for tracking all the tabs and their content, generating appropriate element IDs and the like.

There is example code - both ERB and HAML - in the logical_tabs_demo demo project, but the basic usage looks like this. Individual tab/pane pairs are generated simultaneously and can take their content either as a block or as a :content => "content" option.

<% create_tabbed_panel do |tabbed_panel| %>
  <% tabbed_panel.add_tab("Tab One") do %>
    Content for Tab One
  <% end %>
  <% tabbed_panel.add_tab("Tab Two") do %>
    Content for Tab Two
  <% end %>
  <% tabbed_panel.add_tab("Tab Three", :content => "<p>Content for tab three</p>")  %>
<% end %>

When you close the block for create_tabbed_panel, the object will render the HTML for all tabs you have added. It will generate a div with class="tabbed_panel that contains two UL lists; one for the tabs and one for their associated content panels.

That's about it. If you have the correct javascript file for your environment installed, it should just work.

Other Options

logical_tabs generates a default format for the IDs and classes for generated; it uses this to track multiple panels on the same page, for example. However, you can override the generated IDs if you want them formatted in a specific way for custom javascript you've written:

<%= create_tabbed_panel(:base_id => "my_panel") do |panel| %>
   ...etc...

Will use "my_panel" as the ID of the outer div and as the prefix for the individual IDs of the sub-tabs.

logical_tabs always uses the ID of the outer div as the prefix for all internal components, to guarantee that you don't have duplicate IDs (i.e. two LIs with id='tab_1') if you have multiple panel structures on the page.

Tab Persistence

When using tab interfaces in the past, one of the most common complaints I heard from clients is that the panel would "forget" which tab they had selected when visiting a previous page. The two cases where this was most frequently a problem were:

• When the panels contained forms. If you submit a form but the data fails validation, you may be returned to the same view but with the wrong panel selected, so the user can't see the error messages or the color-coded input fields. This is extremely confusing.
• When the panels list items that the user may want to edit on another page. For example, one client has a tabbed panel in a project management system that lists a project's work orders, action items, and problem reports in separate tabs. If the user clicks to edit a problem report, when finished they may be redirected back to the project page, but now the list of work orders is the active tab - they can't see the list containing the item they just edited. Ouch.

The solution I chose was to store the tab selection in a cookie. logical_tabs stores the current tab selection for each tabbed panel structure (in case you have more than one) along with the url, and at each page load the JS it checks to see if there was previously a tab selected.

Caveats

In some ways, logical_tabs is still a bit of a weekend hack. In particular, I'm not thrilled with the ugly element ID formats, and the jQuery script is not a proper jQuery plugin, doesn't interface with jQueryUI, and isn't themeable with themeroller.

Most of my projects use Prototype, and Logical Reality is only recently beginning to switch to jQuery for new projects. So, the adaptation of the javascript for logical_tabs to jQuery was a very quick hack - my best attempt to clone the Prototype behavior with jQuery selectors. If you think it's ugly, it is, and that's why. :-)

Future Plans

• Clean up the jQuery and make it a properly-behaved jQuery UI plugin skinnable with themeroller.
• Rails 3 compatibility.

Cloning your remote database to local with Capistrano

Handy file: Capistrano recipe for cloning your remote (production) mysql database to your local box, and vice versa.

One of the more useful recipes for Capistrano that I've come across is this one by netzpirat. It's designed to give you simple commands that will dump the mysql database on your production server to a file, scp the file back to your development workstation, and import it into your local mysql database in a single operation. Very sexy, very powerful. (It also has commands for doing the reverse, and for uploading/downloading files as well - useful for things like paperclip attachments.)

Unfortunately, for my purposes, it failed right out of the box in two different ways:

It only pulls your database name, db login, and password from your database.yml, and then (on the remote server) tries to connect to the database on a local socket. Most of my databases are hosted on remote boxes.

It assumes your production database credentials exist in your local database.yml, which they really shouldn't in any decent deployment. Especially not if you are hosting your code in an outside repository.

Slightly improved version

So, I created this improved version, which pulls the remote DB credentials from the database.yml file on the remote server (assuming you've put them in #{shared_path}/config/database.yml), and which will also use the hostname, if any, from your database.yml when it executes mysqldump.

Let me know if you have any trouble with it and I'll correct any errors in the gist.

New LRDesign logo and site!

We've finished our redesign project, and the first version of the new look is up in time for LA Rubyconf!

There's plenty more to do, but we're very happy to have a refreshed look. In can be hard, as a web development company with active clients, to find time to work on our own website!

Back up after hacking incident

This blog, along with a dozen or so other CMS-driven sites I maintain, was compromised by a hacker recently. I've finally gotten this one back up and am working on the others.

When ‘rake spec’ and ‘spec spec’ produce different results

AKA adventures in class loading.

A couple of days ago I did some significant work in authorization in one of my apps, involving creating a Groups class with an HABTM relationship to Person, so I could assign roles to people a group at a time. It all worked out great, and I pushed the product to GitHub. The next day, my collaborator wrote in that my recent contribution broke 119 specs.

I pulled and retested the code, and everything worked perfectly. WTF? After a bit of investigation, I discovered that the specs worked great when I ran 'autotest' or 'spec spec', but that 119 specs broke when I ran the exact same spec suite with 'rake spec'! Double WTF.

Setting constants at class loading

Ultimately, I tracked the problem down to this line and method, in Person.rb:
class Person << ActiveRecord::Base
ADMIN_GROUP = Group.find_by_name('Admin')
def admin?
groups.include? ADMIN_GROUP
end
end


I consider a person an administrator if they are a member of this group, and I was loading it as a constant at the class level in order to avoid having to query the database again every time Person#admin? is called. This worked just fine for me, both in the application, and every time I ran Person#admin?.

But, remarkably, ADMIN_GROUP does not get initialized correctly when I run the tests via rake. I found this via the ruby debugger, running in this particular spec in spec/models/person_spec.rb:


describe Person do
it "should load an admin user from fixture" do
debugger
people(:admin).should be_admin
end
end


When I run the specs and evaluate Person::ADMIN_USER, I get very different results depending on which spec runner I'm using:

Running 'spec spec/models/person_spec.rb':


[11:17:54 CITAlumni]$ spec spec/models/person_spec.rb
spec/models/person_spec.rb:64
people(:admin).should be_admin
(rdb:1) eval Person::ADMIN_GROUP
#


Running 'rake spec SPEC=spec/models/person_spec.rb':


[11:17:13 CITAlumni (48c51f1...)]$ rake spec SPEC=spec/models/person_spec.rb
(in /Users/evan/Development/Ruby/CITAlumni)
FF.....spec/models/person_spec.rb:64
people(:admin).should be_admin
(rdb:1) eval Person::ADMIN_GROUP
nil


How very interesting ... when I use rake, that constant initializes to nil. At some point, I'll actually get around to figuring out why this is so different when the specs are run via rake. In the meantime, the fix was easy:

The Solution

The fix was just to refactor ADMIN_GROUP as a class method with a memoized instance variable. This will at least limit DB queries for the admin group to one per page load; not quite as good as a single DB hit when the class is first loaded, but still a major improvement over querying for the admin group every time Person#admin? is called. I moved it to the Group class at the same time, which was probably the right place for it in the first place:


#app/models/group.rb:
class Group < ActiveRecord::Base
def self.admin_group
@admin_group ||= self.find_by_name('Admin')
end
end

#app/models/person.rb
class Group < ActiveRecord::Base
def admin?
groups.include? Group.admin_group
end
end


And this worked just fine in all environments, solving the problem with 'rake spec'.

The Moral

Be careful with depending on behavior that occurs only during the loading of classes, as it can be environment-dependent!

If anyone out there with uber Ruby skills knows exactly why running specs via rake prevents that class variable from loading correctly, please enlighten us in comments!

Rails fixture strings that are all numbers

I ran into this one today: If you need to specify a string in a YAML file (fixtures or the like) but that string is all digits, put it in quotes.

The problem YAML file looked like this:

spec/fixtures/people.yml (broken)


one:
funky_database_id: 0000012345

two:
funky_database_id: 0000012346


The trouble with this is that yaml interprets those values as integers, not strings, and Person#funky_database_id is a string column. So Ruby conveniently loads the value as an integer and runs to_s on it before inserting. Worse, because these start with 0, they get translated from octal. So people(:one).funky_database_id comes out "5349". Definitely not what I wanted.

This works as expected:

spec/fixtures/people.yml (fixed)


one:
funky_database_id: "0000012345"

two:
funky_database_id: "0000012346"


Single Table Inheritance and RESTful Routes

I'm converting an old, controller/action/id style Rails application to a more RESTful way of doing things, and ran into a brief roadblock: one of my main tables uses single table inheritance to generate three subclasses of items. I never actually use the superclass "task", I only use the three subclasses "action item", "work order", and "problem report".

So, I ran into this little challenge: all three STI subclasses use the same controller, "tasks", because they all have essentially the same behavior and differ only in minor details. But, when I do a resources map:

map.resources :tasks

Then I get errors in much of my code when I say things like redirect_to @task, because if that task happens to be an ActionItem, it's trying to call action_item_path(@task), which doesn't exist.

I googled around a bit to no result. Striking out on my own, it turns out the answer is as simple as mapping each resource independently, and just overriding the controller in map.resources:

In config/routes.rb

map.resources :tasks
map.resources :action_items, :controller => 'tasks'
map.resources :work_orders, :controller => 'tasks'
map.resources :problem_reports, :controller => 'tasks'


Now, redirect_to @task works just fine regardless of which subclass @task happens to be.

Fixing the test setup bug in Rails 2.0.2

Help! My setup method is not running!

Much to my annoyance (and seemingly, very few other developers), Rails 2.0.2 introduced a bug in ActionController::TestCase -- setup methods for tests are not run. It worked in Rails 2.0.1, and broke in Rails 2.0.2. This was fixed in the very next changeset, Revision 8442, but mysteriously it was never rolled out in a 2.0.3. That was five months ago, and given that this basically breaks all tests it's pretty mysterious that there is still no released version of Rails with this bug fixed.

(NOTE: This was true when written. Now, Rails 2.1 is out, and it includes the fixes. You can use the workarounds below if your project depends on Rails 2.0.2, otherwise I recommend that you simply upgrade to 2.1!)

I'm pretty sure I'm not the only Rails developer who writes tests! So I'm surprised there hasn't been more of an outcry about this bug. But the Rails dev process has always been rather mysterious to me. So: we need a workaround. I know of two.

Option one: subclass Test::Unit::TestCase

This is what I've been doing till now: just going back and writing my tests Rails 1.x style. It's annoying and clunky: the whole point of ActionController::TestCase in Rails 2.0 was to eliminate all the redundant and non-DRY junk you had to put in your test setup to establish the instance variables @request, @controller, etc. In any case, it looks like this:


class FooControllerTest < Test::Unit::TestCase
fixtures :tasks, :users, :people

def setup
super
@controller = FooController.new
@request = ActionController::TestRequest.new
@response = ActionController::TestResponse.new
# ... your setup code here ...
end


Option two: use a revision from edge with the bug fixed

Normally, I abhor using unreleased software in a production environment. I definitely don't use EDGE rails in my released software for clients in general ... the hassle is too much to maintain with the constant changesets. I'd rather understand and work with stable software, even if that means I'm not one of the cool Rails edgerati: stable software for my customers is the higher priority.

However, in this case I'm making an exception, and I've started using revision 8442 of Rails as if it were a released 2.0.3, because this bug just annoys me too much. Installing it is simple. If you're using frozen Rails in your application (and you should be!), it's just this at the command-line, in your application root:


$rake rails:freeze:edge REVISION=8442


That should give you a "2.0.3" that fixes this irritating bug and lets you write your tests and setup the way God intended:

class FooControllerTest < ActionController::TestCase
fixtures :tasks, :users, :people

def setup
super
#... your setup code here ...
end


Don't forget the call to super, otherwise you'll clobber the setup method in ActionController::TestCase and your tests won't work.

Important note for subversion users!!

If you are maintaining your project in subversion, re-freezing your rails code with the above command will hose all of your .svn/ directories under vendor/rails. This will give you a few minutes of headaches as you get things straightened back out. I recommend deleting your old rails freeze first, committing, and then re-freezing:


$svn rm vendor/rails
$svn commit -m "removed old rails version"
$rake rails:freeze:edge REVISION=8442
$svn add vendor/rails
$svn commit -m "froze rails to revision 8442"


I maintain most of my projects in Git, now, so this wasn't a problem, but I wasn't thinking when I converted one of my older projects that's still in svn. It was definitely a headache when the working repository broke, so avoid that if you can.

So there you go. I never thought I'd recommend using unreleased software in production, but just hold your nose and pretend that Rev 8442 is "Rails 2.0.3" and you'll probably be fine. I haven't found any problems with it, myself.