The Onion:
“Yeah, of course gay men and women can get married. Who gives a shit?” said Chief Justice John Roberts, who interrupted attorney Charles Cooper’s opening statement defending Proposition 8, which rescinded same-sex couples’ right to marry in California. “Why are we even seriously discussing this?”
Earlier today a routine system email was incorrectly sent to many of our GitHub Enterprise customers. In these errant emails, customer email addresses were included in the To:
field, making them visible to anyone who received the message.
We are very sorry about this. We have determined what caused this incident and contacted all affected customers directly.
The incident occured in the Rails application we use to manage trials and customer contact information for GitHub Enterprise, not the product itself. No GitHub Enterprise installations were affected, and no license keys or any other data were exposed. GitHub.com was not affected.
As part of a routine daily process, the system notifies the members of any organization whose license is about to expire about the upcoming need for renewal. The app builds an email message including the addresses of all of the active accounts tied to the given organization, putting them in the To:
field to enhance deliverability. This morning, the email included a great many more addresses than expected.
Yesterday the Rails core team released four security patches (CVE-2013-1854, CVE-2013-1855, CVE-2013-1856, CVE-2013-1857). We immediately reviewed the patches and updated our Rails applications to stay current. Unfortunately one of these security patches included a change that caused certain SQL queries to behave unexpectedly.
Here's an example of this change in behavior:
class Organization has_many :teams attr_accessible :name, :has_octocats scope :has_octocats_scope, lambda { where(:has_octocats => true) } def self.has_octocats_class_method where(:has_octocats => true) end end class Team < ActiveRecord::Base belongs_to :organization attr_accessible :name def self.using_octocats_scope where(:organization_id => Organization.has_octocats_scope.select(:id)) end def self.using_octocats_class_method where(:organization_id => Organization.has_octocats_class_method.select(:id)) end end > github = Organization.create(:name => "GitHub", :has_octocats => true) > acme = Organization.create(:name => "Acme", :has_octocats => false) > github.teams.create(:name => "Supportocats") > acme.teams.create(:name => "Roadrunners") > github.id #=> 1 > acme.id #=> 2
So, an Organization
owns a number of Team
records. We've defined a couple of methods to help us scope queries for teams to only those organizations that have octocats. Ideally, both of these methods will scope to the same thing: only Team
records with an organization_id
of 1, the GitHub Organization
. And prior to this latest Rails release, they did.
But the latest release of Rails introduced a subtle change to this behavior. Let's try to make some queries based on the Organization
's teams:
> teams = github.teams Team Load (0.4ms) SELECT `teams`.* FROM `teams` WHERE `teams`.`organization_id` = 1 > teams.length # => 1 > teams.first.name # => "Supportocats"
Great. Here we've asked for the GitHub organization's teams, and we've gotten the correct one, "Supportocats", back. All is good so far. Now let's use one of our scopes, just to be extra specific:
> teams = github.teams.using_octocats_class_method Team Load (0.4ms) SELECT `teams`.* FROM `teams` WHERE `teams`.`organization_id` = 1 AND `teams`.`organization_id` IN (1) > teams.length # => 1 > teams.first.name # => "Supportocats"
The results are the same, but the query is different. By going through an extra scope, we've added an additional SQL predicate, one that says the returned Team
records must belong to an Organization
that has octocats. Since the GitHub team has them, the result is the same.
Let's try our scope that is restricted to octocat-having teams on the Acme org:
> teams = acme.teams.using_octocats_class_method Team Load (0.4ms) SELECT `teams`.* FROM `teams` WHERE `teams`.`organization_id` = 2 AND `teams`.`organization_id` IN (1) > teams.length # => 0
Here we see a different result, as expected, and a similar query, again asking for all of the Acme organization's teams that also belong to an Organization
that has octocats. The Acme Organization
has none, so no teams are returned.
But now we come to an unexpected difference. In the last couple of examples, we were using an Arel scope on Organization
that was defined as a normal class method. But if we change to using the scope defined with ActiveRecord's scope
method, we get unexpected and potentially dangerous results:
> teams = acme.teams.using_octocats_scope Team Load (0.4ms) SELECT `teams`.* FROM `teams` WHERE `teams`.`organization_id` IN (1) > teams.length # => 1 > teams.first.name # => "Supportocats"
Now the Acme organization is returning the GitHub organization's teams! This is obviously bad behavior. What's happening? In this case, when using the scope
method to define an Arel scope on Organization
, the where
clause of the scope is overriding the condition imposed by the Organization#teams
association. The part of the WHERE
clause meant to restrict the query to Team
records related to the Acme organization was dropped.
We've narrowed down this change in behavior to this commit. We have fixed this issue on our affected applications and are working with the Rails core team to determine if this change was intentional as well as what action other users should take.
We're reviewing every piece of GitHub code that touches email so we can keep this from happening in the future. We're focusing on more stringent automated tests, sanity checks on email recipients, and even more careful review when we upgrade an external dependency like Rails.
Last week on Wait, Wait... Don't Tell Me, we talked about a new yogurt for men, or brogurt, from a company called Powerful Yogurt. Here's what our panelist, comedian Jessi Klein, had to say about it:
"If male yogurt marketing is anywhere near as annoying as female yogurt marketing, you are in for a treat. Every female yogurt commercial is basically like women in a wedding dress just petting a kitten and eating yogurt."
Powerful sent us a crate of the stuff this week. It arrived as all manly products do, carried by a Navy Seal who then punches it into your face.
The first thing you notice about the Powerful Yogurt container is that it has a six pack. Later editions will come with a beer gut and will never take their shirt off at the beach, insisting it's just because they "burn easily."
Peter: I liked the fact there was no lid. You had to smash it on your forehead to get to it.
Ian: I guess this is pretty manly, but not as manly as that Dannon flavor you have to hunt and kill with your bare hands.
Mike: This is good. Like, this is "morning after a night in a Tijuana brothel and I still have both my kidneys" good.
Brogurt doesn't taste so different than regular yogurt. We were sort of hoping for manly flavors, like "Truck" or "Mixed Berry Martial Arts."
Miles: I could really go for some "Essence of Burt Reynolds."
Mike: I like that yogurt flavor titles do not appear on bill.
Robert: I love the effect this is having on everybody. Finally, Eva can sing baritone in my barbershop quartet.
Eva: Could do without this chest hair, though.
[The verdict: not bad. I'm not sure if there's anything in it that actually made me any manlier, but after eating it, I did win seven straight Tours de France.]
This review comes from the folks at Sandwich Monday, a satirical feature from the humorists at Wait Wait ... Don't Tell Me!