
Today's Email Incident - buttscicles
https://github.com/blog/1440-today-s-email-incident
======
bhauer
Recognizing that it is not politically correct in these parts to pose this
question, I sometimes wonder if the Github team (which seems like a highly-
skilled, competent team) regrets building on Rails.

It seems they have been able to use their high skill and competence to
laboriously steer a bulky and (dare I say it?) heavily-engineered framework to
serve Github as the modestly fast and fairly reliable site that it is.

But would not they be happier had the efforts exhausted on scale, reliability,
and repair been available to spend building features had a faster, less
confusing platform and framework been selected?

I know, it's antithetical or heretical to imply that Rails is anything but a
model of efficiency.

~~~
danenania
It's become fashionable to hate on rails, and honestly I understand the
motivation--it isn't cool and cutting edge anymore. It's slow. It doesn't do
concurrency well. Ruby is nice for what it is, but it lacks the power and
stability of functional languages, and lacks the performance of go or
javascript on v8.

But rails is just doing what it's always done: trying to optimize developer
productivity for front end web dev and routine web tasks. In this it's far
ahead of the pack, and for many businesses, both small and large, this is
exactly the right thing to optimize. Say what you want about github, but they
push a lot of features and push them fast.

Rails is not for crafting algorithms, but when you need to crank out 6 new
pages with various complex crud views and forms by Friday, and by the way can
it be exportable to cvs and oh can you also do paypal and oauth integration
and user avatars with thumbnails, and keep it all reasonably readable and
organized... the sad fact is that for all of rails' heft and lack of cs cred,
trying the above with any other framework after you know rails will drive you
insane, because despite how great the language is or how many connections your
server can handle, you are again and again spending days on things that take
an hour in rails.

Ultimately, I don't think the way forward from rails is to search for a
partial reimplementation of rails in some other language. Rails is the best
we've got for rapid front end and shallow back end work. So use it for what it
does well. For what it doesn't do well, use something else. Personally, I
think it's silly to have an ORM written in ruby. ActiveRecord is for the most
part a lovely api, but its guts should be a separate service written in go or
something. Building a complex recommendation engine or game AI? Write clojure
or haskell services. But when the data comes back and you need to dice it up
and lay it out as html, with a hundred little ohs and ifs and buts, it's not a
fun time without rails.

~~~
tomjen3
I don't care about cool or cutting edge or even performance. I hate rails
because it is built by people with no understanding of security.

But most of all I hate it because it uses so much magic.

~~~
danenania
Well, the magic is what enables the big productivity boost. It's a higher
risk, higher reward approach. In some domains that added risk is unacceptable,
but for many applications it's a very worthwhile tradeoff.

I think often when people say "I hate rails", they are really saying, "I hate
building the types of apps that rails excels at". Which is fine, and makes
sense, because rails excels at making apps that can be very cool and
innovative on the product side, but are usually pretty boring on the
engineering side--it doesn't really get interesting for the engineers until
it's time to start replacing the lower level tiers of rails with something
else. And while secure best practices are certainly stressed (and included as
defaults), conservatism isn't seen as a valid reason to hold up progress. So
you get both the speed and the occasionally bumpy ride that strategy entails.

~~~
entropy_
That has nothing to do with it whatsoever. They specified why they hate about
rails, the magic. Personally, I simply cannot stand magic in frameworks I use.
That's because I don't become truly productive when using a framework until I
fully understand how everything works.

It is not enough that it does work, I have to understand how it does.
Otherwise, I'm constantly bogged down when implementing every single feature
thinking about possible security and performance issues down the line. The
performance thing might be premature optimisation in some cases, but the
security definitely is not.

That's why I hate when frameworks or IDEs have so much magic -- I'm looking at
you Xcode, I can't count the times I was wondering if a particular statement
would cause a memory leak or not before I finally understood most of the
related magic Xcode and the toolchain for iOS dev do.

For me at least, magic saps my productivity, it doesn't boost it. It also
offers a steeper learning curve to proper mastery to get to that point where
I'm actually productive. So yes, I haven't tried ruby on rails but I can
definitely see myself hating it if it has too much magic.

~~~
manojlds
I can somewhat relate to what you say, and that is perhaps why I prefer
Django, as it has just the right amount of magic for me.

------
pud
I previously assumed the cause was some marketing intern pasting email
addresses into the wrong form. Better training (and a slap on the wrist) would
prevent it from happening again.

But it's somewhat scarier knowing this was due to a bug triggered by a
security patch. Who knows what yet-undiscovered bugs are still in there.

That said, there's no such thing as bug-free software and I still find GitHub
to be trustworthy.

~~~
727374
I don't think anyone doubts github's trustworthiness, but as an enterprise
customer, I do wonder if they have tight enough controls. Marketing emails are
an area that companies should be spending extra time testing as those screwups
tend to be irreversible.

------
ghuntley
Very concerning - they accidentally distributed their entire customer list and
now I have to handle the internal PR fall out re: my email address and that
those of non-technical co-workers (board, accounts, finance, engineering
managers) have been distributed globally and then subsequently pastebined.
Times like this I'm glad that our code is hosted internally and access to it
is controlled internally.

[https://dl.dropbox.com/u/109905580/Screenshots/qg~abn4fl601....](https://dl.dropbox.com/u/109905580/Screenshots/qg~abn4fl601.png)

[https://dl.dropbox.com/u/109905580/Screenshots/73dw6%7Eq_h4h...](https://dl.dropbox.com/u/109905580/Screenshots/73dw6%7Eq_h4hq.png)

~~~
eli
I wouldn't normally consider my email address a secret. I assume the concern
is over spear phishing?

~~~
robryan
If a competitor is looking to market their version control solution to
enterprise this email list is going to be a very good place to start.

~~~
rscale
Atlassian isn't going to spam Stash or BitBucket to the exposed users. They'd
get crucified.

There might be a few users who were already frustrated and switch on their
own, but I think the big problems are an ongoing spearphishing risk and a
bruised ego.

~~~
dubcanada
Come again? Atlassian doesn't email you when you need to repay or when you had
paid?

This wasn't "spam" it was an accident email send. Things pretty much every
single giant website has accidently done at least once.

~~~
rscale
I was responding to my parent comment's concern that GitHub's competitors
(e.g. Atlassian) would use the leaked/pastebinned GitHub emails as a mailing
list.

------
sergiotapia
This is bad - real bad. Goes to show you if the 'golden boys' can make
mistakes such as these, what about the other 96% of companies handling your
info?

~~~
Trufa
I don't see what's bad about it, well except for the bug itself.

Computer software is bound to have bugs, and if you ask me. a company that
discloses those bugs, thoroughly explains the cause, addresses how to avoid
them the future and apologises to their customers is on the right track.

Don't get me wrong, you software should be very stable, but when the
inevitable happens, well this is the way you address it.

------
teyc
A modern mail server should have features to prevent too many users from being
present in the To: field. This will also prevent a lot of the Reply All
problems we see regularly. This can be implemented as part of a Quota
management system.

~~~
pilif
Exim does this since forever ([http://www.exim.org/exim-html-
current/doc/html/spec_html/ch-...](http://www.exim.org/exim-html-
current/doc/html/spec_html/ch-main_configuration.html) \- look for
max_recipients). I would assume other MTAs do so too.

Problem is, options like these feel nice until they annoy you because you do
in fact need more recipients. At that point you build a workaround into your
software to send a new mail for every max_recipient users which brings you
right back to square one with the additional technical dept caused by
potential bugs in your batch sending code (off-by-one errors for example)

Be careful what you wish for :)

~~~
teyc
Just like one can have safe_parse() and unsafe_parse(), there could be an
elevated API that could be safely used when vetted. For instance, when the
SMTP server rejects a request it could put it in a special queue which
requires manual confirmation.

This type of problems happen so regularly. In fact, a Japanese trader
accidentally punched one too many zeros and lost a lot of money for the
company. This type of problems can get very expensive in an instant. As an
industry, we tend to think of input validation in passing because we don't pay
enough attention to the problem domain.

------
erikpukinskis
This is potentially a pretty big security hole for apps, right? For example:

    
    
        posts = current_user ? current_user.posts : Post.public
        posts = posts.where(:user_id => params[:user_id]) if params[:user_id]
    

In the old Rails, that code is guaranteed not to show any private posts. But
with the new Rails it'll show you everything written by a user.

Granted, it's pretty awkward code, but I can imagine situations where people
might be inclined to write something like that.

~~~
pjungwir
I think that sort of code is extremely common. It's a bit less awkward if
`current_user` is never nil, but sometimes a non-saved Guest record, letting
you write code like this:

    
    
        @posts = current_user.posts
        @posts = @posts.where(user_id: params[:user_id]) if params[:user_id]

------
swampthing
In case anyone's interested, I think this is a Github Issue on the same bug:

<https://github.com/rails/rails/issues/9813>

~~~
pjungwir
Thank you for posting that (one of the only informative contributions on this
whole thread). That issue description is worrisome because the problem
actually isn't limited to `default_scope`, which many people avoid, but also
affects just `scope`.

------
jstanley
I'm impressed. I thought it much more likely that somebody carelessly pasted a
bunch of addresses into a "To" box instead of "Bcc".

------
RexM
Would it make sense to just always put the recipients in the BCC field to
ensure you don't accidentally leak email addresses if/when something like this
were to happen again?

~~~
swanson
"putting them in the To: field to enhance deliverability"

~~~
famousactress
Yeah. Is there a strong argument against sending separate emails though?
Selfishly curious because that's one of the things we've been doing to prevent
this sort of mistake.

~~~
sitharus
A lot of enterprise spam filters are triggered by a burst of incoming mail
from a single sender, sending one email with a bunch of to: is less likely to
trigger them.

I've had to deal with too many stupid spam filters in my life.

------
losvedir
Wait, so what exactly is the situation that's at issue here? We upgraded
yesterday, too, and have both scopes and class methods that we append onto
has_many/belongs_to relations, but haven't experienced anything out of the
ordinary. Now I'm terrified we have some unexpected behavior lurking somewhere
that's not tested or something.

> _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._

This doesn't happen in all cases -- all the cases where I've done this appears
to be fine. And a scope just blowing away an association in all cases would be
too obvious to not be caught.

Is it because both the scope and the relation select by the same attribute?
Does that attribute have to be `id` for this bug to occur?

Or does it have to do with the fact that they're nesting a query within the
where clause to determine the IDs? (I don't see why this would matter).

I guess I'm off to a REPL unless anyone here knows more concretely what the
issue is.

EDIT TO ADD:

I can't reproduce this behavior. I just spun up a new Rails 3.2.13 app, and
created those two models. Going through what they do in the console, my
results differ on the potentially dangerous part:

    
    
         1.9.3-p0 :015 > teams = acme.teams.using_octocats_scope
         Team Load (0.3ms)  SELECT "teams".* FROM "teams" 
         WHERE "teams"."organization_id" = 2 
         AND "teams"."organization_id" IN 
           (SELECT id FROM "organizations" 
            WHERE "organizations"."has_octocats" = 't')
         => []
    

I do get an empty array, as is supposed to be the case. I notice that my
nested select remains a SQL statement, rather than being evaluated to '(1)' as
in their example.

So... I don't know if I'm doing something wrong, or if GitHub has other code /
configuration settings interacting strangely here.

Can anyone else reproduce?

EDIT AGAIN:

Here are the four files you need to reproduce yourself.
<https://gist.github.com/losvedir/5202121>

ONE MORE EDIT:

<https://github.com/losvedir/test_github_thing>

There's a repo with my history in the console. Dunno why I'm seeing the
behavior I am.

~~~
pjungwir
I can reproduce it. I'm very worried also because I love using scopes. Here is
a repo that shows the problem:

    
    
        https://github.com/pjungwir/scope-error
    

I think it is actually a mite simpler than the gist in your "EDIT AGAIN."

~~~
grey-area
_SELECT "projects"._ FROM "projects" WHERE "projects"."user_id" = 1 AND
"projects"."user_id"=5

This expected SQL doesn't really make sense to me. It would be better if your
example didn't include selecting twice on the same column (not a join) with an
equals clause, as I'd expect that to be removed (it's redundant and would
always produce zero results). So the resulting sql makes more sense (replace
where clause when new where clause uses same col):

 _SELECT "projects"._ FROM "projects" WHERE "projects"."user_id" = 5*

Looking at the github example, they used xx=1 AND xx IN(1,2,3), which makes
more sense as it could possibly produce results, and I see why they were doing
that.

~~~
pjungwir
I realize those two conditions ensure the query will never return results, but
the point is to let you combine business conditions at a higher level of
abstraction, which may indeed result in zero results. There are many cases
where this sort of thing is useful, and it's important that the conditions be
cumulative.

------
markmac
Bad incident, great transparency and handling. Just don't do it again
please... :)

------
Nazzareno
Sending emails always requires more attention than expected. Everyone, even
Apple in 2007, made some severe mistakes in delivering emails. By using a
professional SMTP relay (disclaimer: we provide SMTP+) this wouldn't happen
since, for example, every A:/CC:/CCN: recipient is splitted from the original
message by default.

------
blarghnox
<http://pastebin.com/JX1PdiMB>

~~~
susi22
You'd think the .edu addresses would go to bitbucket and get free unlimited
hosting.

------
IgorPartola
How would one unit-test an app like this, where you have to talk to an
external database and over SMTP at a specific time of day? Seems like in a
case like this thorough unit testing needs to be very extensive to catch a bug
like this, yet it would have been worthwhile.

~~~
boyter
You would only need to unit test the programmer written ORM code against a
seeded database to be sure you are getting just the data you expected.

You probably wont catch new issues with the ORM (which you should not be
testing) but you will ensure you wont put this bug back into production.

------
ecopoesis
And this is why you shouldn't use ORMs. Magic blackboxes always bite you in
the ass eventually.

~~~
tptacek
Yes I would definitely trust an application much more if each and every one of
its SQL queries was lovingly hand crafted by a human being.

Because if there's one thing computers are truly bad at, it's transforming
high-level things into lower-level things.

~~~
martinced
As an expert on security, what makes you think that in 2013 every webapp out
there should still be backed by SQL and running SQL queries?

I mean is SQL something inherently so secure that all webapp devs should use?

If you're not a DB expert, what would be your rationale for using SQL instead
of other alternatives?

It's not like if every website out there was using SQL. Heck, not even all
sites are using OO languages.

So while I agree that an ORM is nice if you're stuck that particular Java/C# +
SQL hell I fail to see how exactly OP did imply that he recommended objects
and SQL...

He said ORM sucked. Not that he was using "objects" and SQL...

~~~
tptacek
SQL is not intrinsically insecure.

------
isarat
The excuses are on an on. I admire Github guys. They give a super cool
impression to the other outside world and they're doing a great job. But the
recent outages and the problems are giving high attention to Github. It's
giving negative sign for the people who are interested go for Github way of
hosting code. Either their own premise or on github. giving detailed analysis
is a good tech thing... it fires developers heart and we are still going
admire what they did to resolve this issue but definitely this isn't a good
sign for github.

------
pjungwir
I whipped up a quick example Rails project to demonstrate the issue. It's a
lot like the Github post, but a little simpler: for instance, there's no real
reason to talk about class methods:

<https://github.com/pjungwir/scope-error>

I'm pretty concerned about this bug, because scopes are one of my favorite
Rails features.

------
robertsosinski
For quite some time, I have been using Sequel (<http://sequel.rubyforge.org>)
as an ORM with Postgres instead of ActiveRecord, and have never been happier.
Overall, Rails has been a great web-framework, but ActiveRecord has been it's
sore spot, at least for me.

Also, using the sequel_pg (<https://github.com/jeremyevans/sequel_pg>) on
Postgres ensures that even raw SQL queries are type-casted properly, plus it
supports streaming and cursors, while also being very very quick.

If you want to stick with Ruby/Rails and relational databases, but think
ActiveRecord might not be your bag, give Sequel a shot. It also fits nicely
into lighter frameworks such as Sinatra.

------
amadeuspzs
Non-technical companies which do this tend not to apologise - it draws
attention to the mistake which most of their (non-technical) customers never
noticed in the first place.

I guess github doesn't qualify for the non-technical crowd :)

------
jrochkind1
I'm trying to figure out how likely my apps are to be effected by this same
bug/behavior change as Github's, and if I should delay installing the security
update without more investigation.

Not a great situation to be in with a security update.

------
modarts
Seing as how this list of customers are about to run out of their trial for
Github Enterprise, it'd be funny if atlassian or any other hosted Git
providers were to make use these emails for their own gain.

------
chadscira
a little more on the incident
[http://chadscira.com/post/5148b1f0f93e934b550112be/Github-
le...](http://chadscira.com/post/5148b1f0f93e934b550112be/Github-
leaks-1500-emails)

~~~
dbpatterson
atlassian? :)

~~~
jschumacher
Atlassian builds a JIRA connector for Bitbucket and Github:Enterprise. To test
the connector, an installation of Github:Enterprise is required.

Internally we use Stash of course ;)

------
chj
Frameworks are innocent.

------
badgar
Rails security point release breaks apps by changing the ORM.

News at 11.

~~~
stanleydrew
I'm not sure why you found it necessary to post this comment. I found the
original article to be a well-written warning about a rather serious bug that
might affect a lot of people. But your comment seems to be sarcastically
suggesting "move along, nothing to see here."

~~~
jleader
I don't think the GP is criticizing the article, he's using the article to
criticize Rails. Not so much "nothing to see here" as "well, what did you
expect from Rails?".

(I'm explaining what I think the GP meant, not my position; I'm an old Perl
guy, still undecided on the overall trade-offs offered by Rails)

~~~
badgar
Yes, I was pointing out that Rails made backwards-incompatible changes to core
functionality in a point release primarily targeting security. I don't know
how much more succinctly I could have originally put it, and I thought that it
was obvious that this is a Bad Thing. You can't reach everybody.

~~~
michaelfairley
The "backwards-incompatible change" was part of a security fix, not an
unrelated non-security change.

~~~
badgar
Github's code doesn't rely on the security flaw to function. Their code should
keep working after the security fix.

Anyone who has ever consumed an API to build something they don't want to
break understands this.

