This has been Rails behavior from day 1. Rails seems to assume that people will make some level of effort to secure their application before deploying to production. There are many ways and places to protect models, and mass assignment protection is a blunt tool that would not have worked for github, so the default behavior is not the issue.
This bug could occur in any framework where someone assumed all attributes submitted are writable by the current user. Rails has no internal concept of users or roles, so building that by default into a model makes no sense.
This is a github bug, not a Rails issue. One could argue it's a questionable, but defensible, decision in the Rails framework, to have such an easy way to take every submitted field and apply it to a model. I'd argue that using such a feature in a production app is a fault of the developer for failing to read their own code, because it's rather obvious and clear what the code does:
Given the existence of attr_accessible, attr_protected, a global on/off switch for the default behavior and nested attributes, you cannot tell what that line does without knowing the contents of at least two files.
And Rails 3.1 does have the concept of roles (not necessarily of users), baked into the same mass assignment logic into the model via the :on argument.
I'm not saying that this is all bad or a bug even, but I don't see how this is trivial to the reader either.
I agree there are a number of complex issues here, however that line of code still obviously takes ALL content from the outside world and directly updates a model with it. Any developer who does not spot that as a security issue is probably creating many others as well. Rails can not protect against developers not understanding that form submissions can contain any content and should not be trusted or applied directly to models without understanding what's happening. A cautious developer would slice up the submission to update the model with only the expected or allowed fields.
I just learned of the new role feature for attr_accessible because of this controversy. This seems to solve one of the major issues with attr_accessible - that different controllers and users need to update different attributes, so any somewhat complex app would end up widening it's attr_accessible attributes beyond what they should be.
These are still blunt tools though - what if only superadmin users can update a role column to superadmin, but admins can update it to admin or guest. This requires more extensive logic in the controller than simple attribute filtering, demonstrating why this filtering really belongs outside the model. Despite that, I think the new "role" based attr_accessible probably covers most cases and seems quite useful.
For all we know, GitHub may have been using attr_accessible but have expanded it to include columns updatable by admins.
Thank you for the thoughtful comment - perhaps there is hope that HN hasn't been entirely taken over by people talking out of their asses.
Agreed, this is how most frameworks work. They rely on the developer to put in proper security controls. One of the first things I do after I start a project from scaffolding is go through and filter out values I don't want the user to set such as user_id.
This isn't to say it should be this way, just that it's pretty standard behavior.
Interesting. I work with Rails every day. I understand and explain the source of the bug, and why it has nothing to do with some oversight by Rails. I'm downvoted.
Every other answer, often admittedly, is written by someone who doesn't know anything about Rails, but jumps on the "oh geez Rails has a terrible security hole" bandwagon.
This bug could occur in any framework where someone assumed all attributes submitted are writable by the current user. Rails has no internal concept of users or roles, so building that by default into a model makes no sense.
This is a github bug, not a Rails issue. One could argue it's a questionable, but defensible, decision in the Rails framework, to have such an easy way to take every submitted field and apply it to a model. I'd argue that using such a feature in a production app is a fault of the developer for failing to read their own code, because it's rather obvious and clear what the code does:
@product.update_attributes(params[:product])
Does exactly what you'd expect it to do.