robertomiranda · March 5, 2012 19:15 · Mar 5, 2012 · Mar 5, 2012 · Mar 5, 2012 · Mar 5, 2012
diff --git a/securing_rails_updates.md b/securing_rails_updates.md
@@ -243,6 +243,8 @@ Author:  Peter Nixey
 Twitter: http://twitter.com/peternixey  
 Blog:    http://peternixey.com  
 
+***PLEASE NOTE I AM NOT A MEMBER OF THE GITHUB TEAM. THIS IS JUST A GIST***
+
 ----
 
 *Further reading:*

diff --git a/securing_rails_updates.md b/securing_rails_updates.md
@@ -227,7 +227,7 @@ Where this should be handled is not an academic design choice, it’s one that
 
 **There are a lot of sites vulnerable and more being built every day**
 
-Rails is a brilliant framework designed by a brilliant team. One of the reasons I like coding in it is that I feel that I always learn from the code I find (in PHP I generally wanted to rewrite it). This weakness has always bugged me though and it doesn't do Rails justice.
+Rails is a brilliant framework designed by brilliant contributors. One of the reasons I like coding in it is that I feel that I always learn from the code I find (in PHP I generally wanted to rewrite it). This one weakness has always bugged me though and I feel it doesn't do Rails justice.
 
 If GitHub, one of the best Rails teams on the planet can be taken out so easily, in so many places by such a simple hack then there is a real and present issue. As Yehuda says, not all security vulnerabilities can be fixed by the framework however this one can and it would make sense to do so.
 

diff --git a/securing_rails_updates.md b/securing_rails_updates.md
@@ -227,6 +227,8 @@ Where this should be handled is not an academic design choice, it’s one that
 
 **There are a lot of sites vulnerable and more being built every day**
 
+Rails is a brilliant framework designed by a brilliant team. One of the reasons I like coding in it is that I feel that I always learn from the code I find (in PHP I generally wanted to rewrite it). This weakness has always bugged me though and it doesn't do Rails justice.
+
 If GitHub, one of the best Rails teams on the planet can be taken out so easily, in so many places by such a simple hack then there is a real and present issue. As Yehuda says, not all security vulnerabilities can be fixed by the framework however this one can and it would make sense to do so.
 
 **A three step suggestion for how the Rails team could address the isssue:**

diff --git a/securing_rails_updates.md b/securing_rails_updates.md
@@ -215,25 +215,24 @@ The argument has been made several times that it is up to the app builder to sec
 
 **If the Rails team are going to stand by the mantra then they also need to accept that the Rails Way to handle updates is conventionally insecure until configured otherwise.**
 
-
 Enforcing that attributes have to be declared `attr_accessible` by default would immediately make things better. 
 
-**The question is not "where should authorization be handled" it is what is the default setting**
+**What should the default authorization setting to be? On or off?**
 
 [Yehuda Katz makes the point](https://gist.github.com/1974187) that this is an authorization issue which is not a framework issue. The question here though is not *“where should authorization be handled”* but *“what should the default setting for authorization be”*. 
 
-In almost everywhere else the default setting for authorization is unauthorized. You can’t even reach a controller method unless you explicitly create a route for it. update_attributes however defaults to authorized. 
+In most other public-facing interfaces in Rails the default setting for authorization is unauthorized. You can’t even reach a controller method unless you explicitly create a route for it. `update_attributes` however defaults to authorized. 
 
-This is not an academic design choice, it’s one that’s carrying a real world cost right now. Arguing over what layer of the app is responsible is like BP blaming Transocean for the Deepwater Horizon. The issue is that oil is leaking.
+Where this should be handled is not an academic design choice, it’s one that’s carrying a real world cost right now. Arguing over what layer of the app is responsible is like BP blaming Transocean for the Deepwater Horizon. The issue is that oil is leaking.
 
-**There are a lot of sites currently vulnerable and more being built**
+**There are a lot of sites vulnerable and more being built every day**
 
-If GitHub, one of the best Rails teams on the planet can be taken out so easily by such a simple hack then there is a real and present issue. As Yehuda says, not all security vulnerabilities can be fixed by the framework however this one can and it would make sense to do so.
+If GitHub, one of the best Rails teams on the planet can be taken out so easily, in so many places by such a simple hack then there is a real and present issue. As Yehuda says, not all security vulnerabilities can be fixed by the framework however this one can and it would make sense to do so.
 
 **A three step suggestion for how the Rails team could address the isssue:**
 
 * On the next release start raising warnings in development when an attempt is made to update an attribute without declaring it `attr_accessible`
-* On the release after raise warnings in production and errors in development
+* On the release after raise errors in development and warnings in production 
 * On the third release make attr_accessible on by default
 
 ------

diff --git a/securing_rails_updates.md b/securing_rails_updates.md
@@ -3,7 +3,9 @@
 
 ----
 
-*Please note: **THIS ARTICLE IS NOT WRITTEN BY THE GITHUB TEAM** or in any way associated with them. It's simply hosted as a [Gist](https://gist.github.com/) because the markdown formatting is excellent and far clearer than anything I could manage on my [personal Tumblr](http://peternixey.com).*
+*Please note: **THIS ARTICLE IS NOT WRITTEN BY THE GITHUB TEAM** or in any way associated with them. It's simply hosted as a [Gist](https://gist.github.com/) because the markdown formatting is excellent and far clearer than anything I could manage on my personal Tumblr at [peternixey.com](http://peternixey.com).*
+
+If you'd like to follow me on twitter my handle is [@peternixey](http://twitter.com/peternixey)
 
 ----
 

diff --git a/securing_rails_updates.md b/securing_rails_updates.md
@@ -3,7 +3,7 @@
 
 ----
 
-*Please note: **THIS ARTICLE IS NOT WRITTEN BY THE GITHUB TEAM** or in any way associated with them. It's simply hosted as a [Gist](https://gist.github.com/) because the markdown formatting is excellent and far clearer than anything I could manage on my personal [Tumblr](http://peternixey.com).*
+*Please note: **THIS ARTICLE IS NOT WRITTEN BY THE GITHUB TEAM** or in any way associated with them. It's simply hosted as a [Gist](https://gist.github.com/) because the markdown formatting is excellent and far clearer than anything I could manage on my [personal Tumblr](http://peternixey.com).*
 
 ----
 

diff --git a/securing_rails_updates.md b/securing_rails_updates.md
@@ -3,7 +3,7 @@
 
 ----
 
-*Please note: **this is not posted by the GitHub team** or in any way associated with them. It's simply hosted as a Gist because the markdown formatting is excellent and far clearer than anything I could manage on my personal [Tumblr](http://peternixey.com).*
+*Please note: **THIS ARTICLE IS NOT WRITTEN BY THE GITHUB TEAM** or in any way associated with them. It's simply hosted as a [Gist](https://gist.github.com/) because the markdown formatting is excellent and far clearer than anything I could manage on my personal [Tumblr](http://peternixey.com).*
 
 ----
 

diff --git a/securing_rails_updates.md b/securing_rails_updates.md
@@ -3,7 +3,7 @@
 
 ----
 
-*Please note **this is not posted by the GitHub team** or in any way associated with them. It's simply hosted as a Gist because the markdown formatting is excellent and far clearer than anything I could manage on my personal [Tumblr](http://peternixey.com).*
+*Please note: **this is not posted by the GitHub team** or in any way associated with them. It's simply hosted as a Gist because the markdown formatting is excellent and far clearer than anything I could manage on my personal [Tumblr](http://peternixey.com).*
 
 ----
 

diff --git a/securing_rails_updates.md b/securing_rails_updates.md
@@ -3,7 +3,7 @@
 
 ----
 
-*Please note **this is not posted by the GitHub team** or in any way associated with them team. It's simply hosted as a Gist because the markdown formatting is excellent and far clearer than anything I could manage on my personal [Tumblr](http://peternixey.com).*
+*Please note **this is not posted by the GitHub team** or in any way associated with them. It's simply hosted as a Gist because the markdown formatting is excellent and far clearer than anything I could manage on my personal [Tumblr](http://peternixey.com).*
 
 ----
 

diff --git a/securing_rails_updates.md b/securing_rails_updates.md
@@ -3,7 +3,7 @@
 
 ----
 
-*Please note this is not posted by GitHub or in any way associated with the GitHub team. It's simply hosted as a Gist because the markdown formatting is excellent and far clearer than anything I could manage on my [Tumblr](http://peternixey.com).*
+*Please note **this is not posted by the GitHub team** or in any way associated with them team. It's simply hosted as a Gist because the markdown formatting is excellent and far clearer than anything I could manage on my personal [Tumblr](http://peternixey.com).*
 
 ----
 

diff --git a/securing_rails_updates.md b/securing_rails_updates.md
@@ -20,7 +20,7 @@ Add the following initializer:
 
     ActiveRecord::Base.send(:attr_accessible, nil)
 
-*(this fix is not without its pitfalls - see later for things to watch for)*
+***(this fix is not without its pitfalls - see later for things to watch for)***
 
 **What the initializer does**
 

diff --git a/securing_rails_updates.md b/securing_rails_updates.md
@@ -3,7 +3,7 @@
 
 ----
 
-*Please note this is not posted by GitHub or in any way an official post. It's simply hosted as a Gist because the markdown formatting is excellent and far clearer than anything I could manage on my [Tumblr](http://peternixey.com).*
+*Please note this is not posted by GitHub or in any way associated with the GitHub team. It's simply hosted as a Gist because the markdown formatting is excellent and far clearer than anything I could manage on my [Tumblr](http://peternixey.com).*
 
 ----
 

diff --git a/securing_rails_updates.md b/securing_rails_updates.md
@@ -1,10 +1,16 @@
 ##How Homakov hacked GitHub and the line of code that could have prevented it
 
+
+----
+
+*Please note this is not posted by GitHub or in any way an official post. It's simply hosted as a Gist because the markdown formatting is excellent and far clearer than anything I could manage on my [Tumblr](http://peternixey.com).*
+
+----
+
 [@homakov’s](http://homakov.blogspot.com/) explot on GitHub was simple and straightforward. Calling it an attack makes it sound malicious whereas the truth was that GitHub bolted its front door but left the hinges on quick release. Homakov released the hinges, walked in and shouted to anyone who would listen that they had a problem.
 
 He was right. The Rails defaults are vulnerable and there’s no better illustration of this than when when one of the best Rails teams in the world is severely compromised.
 
-----
 
 **TL;DR: How to protect your Rails application from the GitHub attack**
 

diff --git a/securing_rails_updates.md b/securing_rails_updates.md
@@ -231,8 +231,8 @@ If GitHub, one of the best Rails teams on the planet can be taken out so easily
 ------
 
 Author:  Peter Nixey  
-Twitter: http://twitter
-Blog:    http://peternixey.com  .com/peternixey
+Twitter: http://twitter.com/peternixey  
+Blog:    http://peternixey.com  
 
 ----
 

diff --git a/securing_rails_updates.md b/securing_rails_updates.md
@@ -49,23 +49,23 @@ and a very simple User class:
     end
 
 
-**Why this `user` is vulnerable**
+**Why the User class is vulnerable**
 
     > u = User.create name: ‘Peter Nixey’, role: :subscriber;
      => #<User id: 1, role: :subscriber, name: "Peter Nixey", 
           created_at: "2012-03-05 09:39:31", updated_at: "2012-03-05 09:39:31"> 
 
 By default, `update_attributes` (which is what you’ll probably use in your update method) updates any attributes that are passed into it - usually via `params[:model_name]`.  It’s wonderfully quick and simple but open to abuse:
 
-`update_params` will for instance happily update not only our name but also our role:
+`update_params` will for instance happily update not only your name but also your role:
 
     > u.update_attributes name: ‘Jenson Button’,  role: :superadmin;
      => #<User id: 1, role: "superadmin", name: "Jenson Button", 
           created_at: "2012-03-05 09:39:31", updated_at: "2012-03-05 09:40:53"> 
 
 *By fiddling with the user update form we just updated our role from subscriber to superadmin.* 
 
-This is not a good thing and is not really what we’d like as our default access setting.
+This is not good.
 
 ### How Homakov used update_attributes to hack the Rails GitHub account 
 
@@ -84,9 +84,9 @@ Homakov assumed (correctly) that GitHub had a table containing users’ public k
       end
     end
 
-Homakov PUT an update to his own existing public key which included a new `user_id`. The user_id he used was that of a member of the Rails repository team. 
+Homakov PUT an update to his own existing public key which included a new `user_id`. The user_id he used was that of a member of the Rails repository members. 
 
-The controller then simply updated all of the parameters Homakov passed it, including the new user_id. With an SSH key on his machine registered to the repository  of a Rails member all he then needed to do was [push](https://github.com/rails/rails/commit/b83965785db1eec019edf1fc272b1aa393e6dc57). This was the same hack he used for [posting from the future](https://github.com/rails/rails/issues/5239).
+The controller then simply updated all of the parameters Homakov passed it, including the new `user_id`. With an SSH key on his machine registered to the repository  of a Rails member all he then needed to do was [push](https://github.com/rails/rails/commit/b83965785db1eec019edf1fc272b1aa393e6dc57). This was the same hack he used for [posting from the future](https://github.com/rails/rails/issues/5239).
 
 **Why don't I just avoid `update_attributes`?**
 
@@ -100,7 +100,7 @@ We could do but it would take five lines where update_attributes only takes one
 
 **How to protect update_attributes: attr_protected (not recommended)**
 
-Everything that happened happened because the user_id attribute should not have been updatable via update_attributes. Rails has a method to prevent exactly this and it’s called `attr_protected`. 
+Everything that happened happened because the `user_id` attribute should not have been updatable via update_attributes. Rails has a method to prevent exactly this and it’s called `attr_protected`. 
 
     class User < ActiveRecord::Base
       attr_protected :role
@@ -113,9 +113,7 @@ With that line added it doesn’t matter whether we pass the role in via a PUT,
     WARNING: Can't mass-assign protected attributes: role
      => true 
 
-The problem is that `attr_protected` only protects us on attributes we actually add it to
-
-The problem with `attr_protected` is that it only works when we remember to add it. If we don’t put it in we don’t get protection. 
+The problem is that `attr_protected` only protects us on attributes we actually declare to be `attr_protected`. It only works when we remember to add it. If we don’t put it in we don’t get protection. 
 
 I prefer to know I’m protected and safe until I chose to be unsafe and that (in theory) is what `attr_accessible` gives us.
 
@@ -129,7 +127,7 @@ Delcaring any attribute as `attr_accessible` implies that all the other attribut
       attr_accessible :name
     end
 
-role is now protected since we haven't declared it `attr_accessible`:
+`:role` is now protected since we haven't declared it `attr_accessible`:
 
     u = User.create name: "Peter Nixey", role: :subscriber;
     u.update_attributes role: :superadmin
@@ -151,60 +149,63 @@ and leave our model unchanged:
       attr_accessible :name
     end
 
-then the `account_type` field is automatically protected for us:
+then the `account_type` field is automatically protected:
 
-    u = User.create name: “Peter Nixey”, account_type: ‘free’
+    u = User.create name: “Peter Nixey”, account_type: ‘free’;
     u.update_attributes account_type: ‘paid’
     WARNING: Can't mass-assign protected attributes: account_type
      => true 
 
 **Either way you can still manually update attributes**
 
-We can still update role directly it’s simply that it’s not vulnerable to an update through our update controller:
+We've not locked ourselves out of our own model. We can still update role directly it’s simply that it’s not vulnerable to being injected during `update_attributes`
 
     u.role = :superadmin
     u.save
        (0.2ms)  UPDATE "users" SET "role" = 'superadmin', 
        "updated_at" = '2012-03-05 10:11:05.042023' WHERE "users"."id" = 1
      => true 
 
-**Even attr_accessible only protects us when we remember it**
+**However, even attr_accessible only protects us when we remember it**
 
 The problem with `attr_protected` was that it only protected us when we remembered to add it to the attribute.
 
 The problem with `attr_accessible` is that it only protects us when we remember to add it to the model. Sometimes (as GitHub showed us) it’s easy to forget to do that.
 
-###The disable_mass_assignment initializer gives security by default
+###The disable_mass_assignment initializer protects us by default
 
+Create a new file:  
 *config/initializers/disable_mass_assignment.rb*
 
     ActiveRecord::Base.send(:attr_accessible, nil)
 
-The beauty of this is that it effectively adds `attr_accessible` to every model we create (actually what it does it take it away by default but it comes to the same thing). No attribute can be updated unless we declare it `attr_accessible`. We’re secure until we decide not to be.
+The beauty of this is that it effectively adds `attr_accessible` to every model we create (actually what it does it take it away by default but it comes to the same thing). No attribute can be updated unless we declare it `attr_accessible`. We’re secure until we decide otherwise.
 
 **Possible issues you might have with the initializer**
 
-If you add `attr_accessible` the first thing you’re going to need to do is to declare the relevant attributes as `attr_accessible`.
+Once you setup the initializer, the first thing you’re going to need to do is declare all relevant attributes as `attr_accessible`.
 
-A good test suite will help a lot here but with or without one, go through each model adding each parameter that you want accessible to your `attr_accessible` arguments:
+A good test suite will help a lot here but either way you need to go through each model adding each parameter that you want to be accessible to your `attr_accessible` arguments:
 
-    class Xyz
-      attr_accessible attribute_a, attribute_b, attribute_c
+    class User
+      attr_accessible :email, :first_name, :last_name, :full_name
     end
 
 You’re going to have some frustrations. There are going to be things that you don’t see coming which will fail silently. Problems I’ve had are:
 
 - Authlogic: you need to remember to make attributes like password, email etc accessible
 - Paperclip: remember to make paperclip attributes accessible
-- Nested attributes: Instructions here: http://api.rubyonrails.org/classes/ActiveRecord/NestedAttributes/ClassMethods.html#label-Using+with+attr_accessible
+- Nested attributes: [Instructions here](http://api.rubyonrails.org/classes/ActiveRecord/NestedAttributes/ClassMethods.html#label-Using+with+attr_accessible) 
 
 I’m sure you’ll hit other issues too but you can generally knock them off by adding attributes one by one to the list of accessible ones.
 
-**How Rails could address this**
+###How Rails could address this
 
 I wouldn’t pretend to have anything like the oversight of the Rails landscape that the Rails core team do. I’ve only built a very few apps and I’m no guru. However...
 
-The argument that it is up to the app builder to secure their own app is however dangerous. Rails’ mantra is convention over configuration. **If the Rails team are going to stand by this mantra then they also need to accept that the Rails Way to handle updates is conventionally insecure until configured otherwise.**
+The argument has been made several times that it is up to the app builder to secure their own app. I don't agree with this though. Rails’ mantra is convention over configuration. 
+
+**If the Rails team are going to stand by the mantra then they also need to accept that the Rails Way to handle updates is conventionally insecure until configured otherwise.**
 
 
 Enforcing that attributes have to be declared `attr_accessible` by default would immediately make things better. 

diff --git a/securing_rails_updates.md b/securing_rails_updates.md
@@ -18,7 +18,7 @@ Add the following initializer:
 
 **What the initializer does**
 
-The initalizer forces you to declare parameters that can be updated via the `update_attributes` method. Rails’ default position is that any attribute on model (except for a few of the ActiveRecord core attributes) is updatable via `update_attributes`. 
+The initalizer forces you to declare parameters that can be updated via the `update_attributes` method. Rails’ default position is that any attribute on a model (except for a few of the ActiveRecord core attributes) is updatable via `update_attributes`. 
 
 If you want to protect attributes from being updated you either need to single them out using `attr_protected` or you can trigger whitelisting on the model by declaring at least one attribute `attr_accessible`.
 
@@ -52,7 +52,7 @@ and a very simple User class:
 **Why this `user` is vulnerable**
 
     > u = User.create name: ‘Peter Nixey’, role: :subscriber;
-     => #<User id: 1, role: :admin, name: "Peter Nixey", 
+     => #<User id: 1, role: :subscriber, name: "Peter Nixey", 
           created_at: "2012-03-05 09:39:31", updated_at: "2012-03-05 09:39:31"> 
 
 By default, `update_attributes` (which is what you’ll probably use in your update method) updates any attributes that are passed into it - usually via `params[:model_name]`.  It’s wonderfully quick and simple but open to abuse:

diff --git a/securing_rails_updates.md b/securing_rails_updates.md
@@ -22,13 +22,13 @@ The initalizer forces you to declare parameters that can be updated via the `upd
 
 If you want to protect attributes from being updated you either need to single them out using `attr_protected` or you can trigger whitelisting on the model by declaring at least one attribute `attr_accessible`.
 
-The initializer switches this round and makes whitelisting the default setting. With the intializer switched on, update_attributes will only update attributes on your models which are declared attr_accessible.
+The initializer switches this round and makes whitelisting the default setting. With the intializer switched on, `update_attributes` will only update attributes on your models which are declared `attr_accessible`.
 
 ----
 
-**What the initializer protects against**
+**Why this is needed**
 
-Let’s take a simple User model:
+Take a simple User model:
 
 *create_users.rb migration:*
 

diff --git a/securing_rails_updates.md b/securing_rails_updates.md
@@ -4,6 +4,8 @@
 
 He was right. The Rails defaults are vulnerable and there’s no better illustration of this than when when one of the best Rails teams in the world is severely compromised.
 
+----
+
 **TL;DR: How to protect your Rails application from the GitHub attack**
 
 Add the following initializer:
@@ -22,8 +24,9 @@ If you want to protect attributes from being updated you either need to single t
 
 The initializer switches this round and makes whitelisting the default setting. With the intializer switched on, update_attributes will only update attributes on your models which are declared attr_accessible.
 
+----
 
-**How update_attributes works by default**
+**What the initializer protects against**
 
 Let’s take a simple User model:
 

diff --git a/securing_rails_updates.md b/securing_rails_updates.md
@@ -1,4 +1,4 @@
-**A line of code that could have protected Github from being hacked**
+##How Homakov hacked GitHub and the line of code that could have prevented it
 
 [@homakov’s](http://homakov.blogspot.com/) explot on GitHub was simple and straightforward. Calling it an attack makes it sound malicious whereas the truth was that GitHub bolted its front door but left the hinges on quick release. Homakov released the hinges, walked in and shouted to anyone who would listen that they had a problem.
 

diff --git a/securing_rails_updates.md b/securing_rails_updates.md
@@ -4,23 +4,24 @@
 
 He was right. The Rails defaults are vulnerable and there’s no better illustration of this than when when one of the best Rails teams in the world is severely compromised.
 
-**Tl;Dr: How to protect your Rails application from the GitHub attack**
+**TL;DR: How to protect your Rails application from the GitHub attack**
 
 Add the following initializer:
 
 *config/initializers/disable_mass_assignment.rb*
 
     ActiveRecord::Base.send(:attr_accessible, nil)
 
+*(this fix is not without its pitfalls - see later for things to watch for)*
+
 **What the initializer does**
 
-The initalizer forces you to declare parameters that can be updated via the update_attributes method. Rails’ default position is that any attribute on model (except for a few of the ActiveRecord core attributes) is updatable via `update_attributes`. 
+The initalizer forces you to declare parameters that can be updated via the `update_attributes` method. Rails’ default position is that any attribute on model (except for a few of the ActiveRecord core attributes) is updatable via `update_attributes`. 
 
 If you want to protect attributes from being updated you either need to single them out using `attr_protected` or you can trigger whitelisting on the model by declaring at least one attribute `attr_accessible`.
 
 The initializer switches this round and makes whitelisting the default setting. With the intializer switched on, update_attributes will only update attributes on your models which are declared attr_accessible.
 
-*(this fix is not without its pitfalls - see later for things to watch for)*
 
 **How update_attributes works by default**
 
@@ -44,19 +45,14 @@ and a very simple User class:
     class User < ActiveRecord::Base
     end
 
-We now have a user with the following attributes:
 
-    User
-    - role
-    - name
-
-**How the default behaviour is vulnerable (& where the whole hoo-ha began)**
+**Why this `user` is vulnerable**
 
     > u = User.create name: ‘Peter Nixey’, role: :subscriber;
      => #<User id: 1, role: :admin, name: "Peter Nixey", 
           created_at: "2012-03-05 09:39:31", updated_at: "2012-03-05 09:39:31"> 
 
-By default, `update_attributes` (which is what you’ll probably use in your update method) updates any attributes that are passed into it - usually via `params[:model_name]`.  It’s wonderfully quick and simple but also vulnerable. 
+By default, `update_attributes` (which is what you’ll probably use in your update method) updates any attributes that are passed into it - usually via `params[:model_name]`.  It’s wonderfully quick and simple but open to abuse:
 
 `update_params` will for instance happily update not only our name but also our role:
 
@@ -66,11 +62,11 @@ By default, `update_attributes` (which is what you’ll probably use in your upd
 
 *By fiddling with the user update form we just updated our role from subscriber to superadmin.* 
 
-This is not a good thing and is not really what we’d like as our default setting.
+This is not a good thing and is not really what we’d like as our default access setting.
 
-# How Homakov used update_attributes to hack the Rails GitHub account 
+### How Homakov used update_attributes to hack the Rails GitHub account 
 
-Since by default, `update_attributes` will update any parameter that’s passed into it, Homakov realised be could could use it to add an SSH key on his machine to the list of keys associated with one of the Rails GitHub account members.
+Since, by default, `update_attributes` will update any parameter that’s passed into it, Homakov realised be could could use it to switch an SSH key for his own account to being one of the list of keys associated with one of the Rails GitHub account members.
 
 Homakov assumed (correctly) that GitHub had a table containing users’ public keys. Each key has a `value` and a `user_id`. Homakov also correctly postulated that he might be able to update his own public key to have the `user_id` of one of the Rails GitHub account members. 
 
@@ -87,7 +83,7 @@ Homakov assumed (correctly) that GitHub had a table containing users’ public k
 
 Homakov PUT an update to his own existing public key which included a new `user_id`. The user_id he used was that of a member of the Rails repository team. 
 
-The controller then simply updated all of the parameters Homakov passed to it, including the new user_id. With an SSH key on his machine registered to the repository  of a Rails member all he then needed to do was [push](https://github.com/rails/rails/commit/b83965785db1eec019edf1fc272b1aa393e6dc57). This was the same hack he used for [posting from the future](https://github.com/rails/rails/issues/5239).
+The controller then simply updated all of the parameters Homakov passed it, including the new user_id. With an SSH key on his machine registered to the repository  of a Rails member all he then needed to do was [push](https://github.com/rails/rails/commit/b83965785db1eec019edf1fc272b1aa393e6dc57). This was the same hack he used for [posting from the future](https://github.com/rails/rails/issues/5239).
 
 **Why don't I just avoid `update_attributes`?**
 
@@ -99,9 +95,9 @@ This way everything would only be updated if we specifically updated it.
 
 We could do but it would take five lines where update_attributes only takes one line. `update_attributes` is also for better or worse, the Rails Way and so it’s a good idea to understand why it’s vulnerable and how to secure it.
 
-**How to protect against this: attr_protected (not recommended)**
+**How to protect update_attributes: attr_protected (not recommended)**
 
-Everything that happened happened because the user_id attribute should not have been updatable via update_attributes. Rails has a method for exactly this and it’s called `attr_protected`. 
+Everything that happened happened because the user_id attribute should not have been updatable via update_attributes. Rails has a method to prevent exactly this and it’s called `attr_protected`. 
 
     class User < ActiveRecord::Base
       attr_protected :role
@@ -114,27 +110,30 @@ With that line added it doesn’t matter whether we pass the role in via a PUT,
     WARNING: Can't mass-assign protected attributes: role
      => true 
 
-however, `attr_protected` only protects us on attributes we actually add it to
+The problem is that `attr_protected` only protects us on attributes we actually add it to
 
 The problem with `attr_protected` is that it only works when we remember to add it. If we don’t put it in we don’t get protection. 
 
 I prefer to know I’m protected and safe until I chose to be unsafe and that (in theory) is what `attr_accessible` gives us.
 
-`attr_accessible` protects all attributes unless we declare otherwise
+**A bit safer protection: attr_accessible**
+
+`attr_accessible` is the recommended method of tackling this problem. It’s actually a little bit of a misnomer since it's less about making an attribute accessible (it already was) and more about making it inaccessible. 
 
-`attr_accessible` is the recommended method of tackling this problem. It’s actually a little bit of a misnomer since its value is not in making an attribute accessible (it already was). Its value is that that adding it implies that all the other attributes are not accessible. Think of its real value less as being `attr_accessible` and more as being *attr_whitelist*
+Delcaring any attribute as `attr_accessible` implies that all the other attributes are not accessible. Think of its real value less as being `attr_accessible` and more as being *attr_whitelist*
 
     class User < ActiveRecord::Base
       attr_accessible :name
     end
 
+role is now protected since we haven't declared it `attr_accessible`:
 
     u = User.create name: "Peter Nixey", role: :subscriber;
     u.update_attributes role: :superadmin
     WARNING: Can't mass-assign protected attributes: role
      => true 
 
-The nice thing about `attr_accessible` is that all new attributes are protected by default. If we add an account type to our database (but leave the model unchanged)
+The nice thing about `attr_accessible` is that all new attributes are protected by default. If we add an account type to our database
 
 
     class AddAccountType < ActiveRecord::Migration
@@ -143,12 +142,13 @@ The nice thing about `attr_accessible` is that all new attributes are protected
       end
     end
 
+and leave our model unchanged:
 
     class User < ActiveRecord::Base
       attr_accessible :name
     end
 
-... the :account_type field is automatically protected
+then the `account_type` field is automatically protected for us:
 
     u = User.create name: “Peter Nixey”, account_type: ‘free’
     u.update_attributes account_type: ‘paid’
@@ -165,25 +165,23 @@ We can still update role directly it’s simply that it’s not vulnerable to an
        "updated_at" = '2012-03-05 10:11:05.042023' WHERE "users"."id" = 1
      => true 
 
-**However, even `attr_accessible` only protects us when we remember it**
+**Even attr_accessible only protects us when we remember it**
 
 The problem with `attr_protected` was that it only protected us when we remembered to add it to the attribute.
 
 The problem with `attr_accessible` is that it only protects us when we remember to add it to the model. Sometimes (as GitHub showed us) it’s easy to forget to do that.
 
-**The disable_mass_assignment initializer gives security by default**
-
-The beauty of the initializer:
+###The disable_mass_assignment initializer gives security by default
 
 *config/initializers/disable_mass_assignment.rb*
 
     ActiveRecord::Base.send(:attr_accessible, nil)
 
-is that it effectively adds `attr_accessible` to every model we create (actually what it does it take it away by default but it comes to the same thing). No attribute can be updated unless we declare it `attr_accessible`. We’re secure until we decide not to be.
+The beauty of this is that it effectively adds `attr_accessible` to every model we create (actually what it does it take it away by default but it comes to the same thing). No attribute can be updated unless we declare it `attr_accessible`. We’re secure until we decide not to be.
 
 **Possible issues you might have with the initializer**
 
-If you add `attr_accessible` the first thing you’re going to need to do is to declare all attributes accessible which are accessible. 
+If you add `attr_accessible` the first thing you’re going to need to do is to declare the relevant attributes as `attr_accessible`.
 
 A good test suite will help a lot here but with or without one, go through each model adding each parameter that you want accessible to your `attr_accessible` arguments:
 
@@ -195,12 +193,13 @@ You’re going to have some frustrations. There are going to be things that you
 
 - Authlogic: you need to remember to make attributes like password, email etc accessible
 - Paperclip: remember to make paperclip attributes accessible
+- Nested attributes: Instructions here: http://api.rubyonrails.org/classes/ActiveRecord/NestedAttributes/ClassMethods.html#label-Using+with+attr_accessible
 
 I’m sure you’ll hit other issues too but you can generally knock them off by adding attributes one by one to the list of accessible ones.
 
 **How Rails could address this**
 
-I wouldn’t pretend to have anything like the oversight of the Rails landscape that the Rails core team do. I’ve only built a very few apps and I’m no guru.
+I wouldn’t pretend to have anything like the oversight of the Rails landscape that the Rails core team do. I’ve only built a very few apps and I’m no guru. However...
 
 The argument that it is up to the app builder to secure their own app is however dangerous. Rails’ mantra is convention over configuration. **If the Rails team are going to stand by this mantra then they also need to accept that the Rails Way to handle updates is conventionally insecure until configured otherwise.**
 
@@ -209,19 +208,19 @@ Enforcing that attributes have to be declared `attr_accessible` by default would
 
 **The question is not "where should authorization be handled" it is what is the default setting**
 
-[Yehuda Katz makes the point](https://gist.github.com/1974187) that access is an authorization issue. The question here though is not *“where should authorization be handled”* but *“what should the default setting for authorization be”*. 
+[Yehuda Katz makes the point](https://gist.github.com/1974187) that this is an authorization issue which is not a framework issue. The question here though is not *“where should authorization be handled”* but *“what should the default setting for authorization be”*. 
 
-In almost everywhere else the default setting for authorization is unauthorized. You can’t even reach a controller method unless you explicitly create a route for it. update_attributes however defaults to authorized. This is not an academic design choice, it’s one that’s carrying a real world cost right now. 
+In almost everywhere else the default setting for authorization is unauthorized. You can’t even reach a controller method unless you explicitly create a route for it. update_attributes however defaults to authorized. 
 
-Arguing over what layer of the app is responsible is like BP blaming Transocean for the Deepwater Horizon. The issues is that oil is leaking.
+This is not an academic design choice, it’s one that’s carrying a real world cost right now. Arguing over what layer of the app is responsible is like BP blaming Transocean for the Deepwater Horizon. The issue is that oil is leaking.
 
 **There are a lot of sites currently vulnerable and more being built**
 
 If GitHub, one of the best Rails teams on the planet can be taken out so easily by such a simple hack then there is a real and present issue. As Yehuda says, not all security vulnerabilities can be fixed by the framework however this one can and it would make sense to do so.
 
 **A three step suggestion for how the Rails team could address the isssue:**
 
-* On the next release start raising warnings in development when an attempt is made to update an attribute without declaring attr_accessible
+* On the next release start raising warnings in development when an attempt is made to update an attribute without declaring it `attr_accessible`
 * On the release after raise warnings in production and errors in development
 * On the third release make attr_accessible on by default
 

diff --git a/securing_rails_updates.md b/securing_rails_updates.md
@@ -53,7 +53,6 @@ We now have a user with the following attributes:
 **How the default behaviour is vulnerable (& where the whole hoo-ha began)**
 
     > u = User.create name: ‘Peter Nixey’, role: :subscriber;
-    > u
      => #<User id: 1, role: :admin, name: "Peter Nixey", 
           created_at: "2012-03-05 09:39:31", updated_at: "2012-03-05 09:39:31"> 
 
@@ -62,37 +61,26 @@ By default, `update_attributes` (which is what you’ll probably use in your upd
 `update_params` will for instance happily update not only our name but also our role:
 
     > u.update_attributes name: ‘Jenson Button’,  role: :superadmin;
-    > u
      => #<User id: 1, role: "superadmin", name: "Jenson Button", 
           created_at: "2012-03-05 09:39:31", updated_at: "2012-03-05 09:40:53"> 
 
 *By fiddling with the user update form we just updated our role from subscriber to superadmin.* 
 
 This is not a good thing and is not really what we’d like as our default setting.
 
-**Do I have to use update_attributes?**
+# How Homakov used update_attributes to hack the Rails GitHub account 
 
-Why all the fuss about `update_attributes`? If it’s so insecure why use it, why not manually update stuff using code such as
-
-    user.name = params[:user][‘name’]
-
-This way everything would only be updated if we specifically updated it.
-
-We could do but it would take five lines where update_attributes only takes one line. `update_attributes` is also for better or worse, the Rails Way and so it’s a good idea to understand why it’s vulnerable and how to secure it.
-
-**Homakov hacked the Rails core account using update_attributes**
-
-Since update_attributes will update any parameter that’s passed into it, Homakov realised be could could use it to add an SSH key on his machine to the list of keys associated with one of the Rails GitHub account members.
+Since by default, `update_attributes` will update any parameter that’s passed into it, Homakov realised be could could use it to add an SSH key on his machine to the list of keys associated with one of the Rails GitHub account members.
 
-Homakov assumed (correctly) that GitHub had a table containing users’ public keys. Each key has a key_value and a `user_id`. Homakov also correctly postulated that he might be able to update his own public key to have the `user_id` of one of the Rails GitHub account members. 
+Homakov assumed (correctly) that GitHub had a table containing users’ public keys. Each key has a `value` and a `user_id`. Homakov also correctly postulated that he might be able to update his own public key to have the `user_id` of one of the Rails GitHub account members. 
 
-*schematic of what the GitHub PublicKey#update method looks like*:
+*schematic of what the GitHub PublicKey#update method might look like*:
 
     class PublicKeyController < ApplicationController
       before_filter :authorize_user
       ...
       def update
-        @current_key = PublicKey.find_by_id params[:key][‘id’]
+        @current_key = PublicKey.find_by_id params[:key]['id']
         @current_key.update_attributes(params[:key])
       end
     end
@@ -101,6 +89,16 @@ Homakov PUT an update to his own existing public key which included a new `user_
 
 The controller then simply updated all of the parameters Homakov passed to it, including the new user_id. With an SSH key on his machine registered to the repository  of a Rails member all he then needed to do was [push](https://github.com/rails/rails/commit/b83965785db1eec019edf1fc272b1aa393e6dc57). This was the same hack he used for [posting from the future](https://github.com/rails/rails/issues/5239).
 
+**Why don't I just avoid `update_attributes`?**
+
+Why all the fuss about `update_attributes`? If it’s so insecure why use it, why not manually update stuff using code such as
+
+    user.name = params[:user][‘name’]
+
+This way everything would only be updated if we specifically updated it.
+
+We could do but it would take five lines where update_attributes only takes one line. `update_attributes` is also for better or worse, the Rails Way and so it’s a good idea to understand why it’s vulnerable and how to secure it.
+
 **How to protect against this: attr_protected (not recommended)**
 
 Everything that happened happened because the user_id attribute should not have been updatable via update_attributes. Rails has a method for exactly this and it’s called `attr_protected`. 

diff --git a/securing_rails_updates.md b/securing_rails_updates.md
@@ -1,24 +1,26 @@
-**The line of code that could have protected Github**
+**A line of code that could have protected Github from being hacked**
 
-@homakov’s attack on GitHub was simple and straightforward. Calling it an attack makes it sound malicious whereas the truth was that GitHub bolted its front door but left the hinges on quick release. Homakov released the hinges, walked in and shouted to anyone who would listen that they had a problem.
+[@homakov’s](http://homakov.blogspot.com/) explot on GitHub was simple and straightforward. Calling it an attack makes it sound malicious whereas the truth was that GitHub bolted its front door but left the hinges on quick release. Homakov released the hinges, walked in and shouted to anyone who would listen that they had a problem.
 
 He was right. The Rails defaults are vulnerable and there’s no better illustration of this than when when one of the best Rails teams in the world is severely compromised.
 
-**How to protect your Rails application from the GitHub attack**
+**Tl;Dr: How to protect your Rails application from the GitHub attack**
 
 Add the following initializer:
 
 *config/initializers/disable_mass_assignment.rb*
 
     ActiveRecord::Base.send(:attr_accessible, nil)
 
-**TL;DR: What the initializer does**
+**What the initializer does**
 
-The initalizer forces all models in your application to whitelist parameters that can be updated via the update_attributes method. Rails’ default position is that any attribute on model is updatable via update_attributes. If you want to protect attributes from being updated you either need to single them out using attr_protected or you trigger whitelisting on the model by declaring at least one attribute to be attr_accessible.
+The initalizer forces you to declare parameters that can be updated via the update_attributes method. Rails’ default position is that any attribute on model (except for a few of the ActiveRecord core attributes) is updatable via `update_attributes`. 
 
-The initializer switches this round and makes whitelisting the default position. With the intializer switched on, update_attributes will only update attributes on your models which are declared attr_accessible.
+If you want to protect attributes from being updated you either need to single them out using `attr_protected` or you can trigger whitelisting on the model by declaring at least one attribute `attr_accessible`.
 
-(also see the last paragraph of this article for for possible issues with enabling this)
+The initializer switches this round and makes whitelisting the default setting. With the intializer switched on, update_attributes will only update attributes on your models which are declared attr_accessible.
+
+*(this fix is not without its pitfalls - see later for things to watch for)*
 
 **How update_attributes works by default**
 
@@ -52,37 +54,39 @@ We now have a user with the following attributes:
 
     > u = User.create name: ‘Peter Nixey’, role: :subscriber;
     > u
-     => #<User id: 1, role: :admin, name: "Peter Nixey", created_at: "2012-03-05 09:39:31",
-          updated_at: "2012-03-05 09:39:31"> 
+     => #<User id: 1, role: :admin, name: "Peter Nixey", 
+          created_at: "2012-03-05 09:39:31", updated_at: "2012-03-05 09:39:31"> 
 
-By default, `update_attributes` (which is what you’ll probably use in your update method) updates any attributes that are passed in via http `params`.  It’s wonderfully quick and simple but also vulnerable. 
+By default, `update_attributes` (which is what you’ll probably use in your update method) updates any attributes that are passed into it - usually via `params[:model_name]`.  It’s wonderfully quick and simple but also vulnerable. 
 
-We can for instance not just update our name but also our role:
+`update_params` will for instance happily update not only our name but also our role:
 
     > u.update_attributes name: ‘Jenson Button’,  role: :superadmin;
     > u
-     => #<User id: 1, role: "superadmin", name: "Jenson Button", created_at: "2012-03-05 09:39:31",
-     updated_at: "2012-03-05 09:40:53"> 
+     => #<User id: 1, role: "superadmin", name: "Jenson Button", 
+          created_at: "2012-03-05 09:39:31", updated_at: "2012-03-05 09:40:53"> 
+
+*By fiddling with the user update form we just updated our role from subscriber to superadmin.* 
 
-We just updated our role from subscriber to superadmin. This is not a good thing and is not really what we’d like as our default setting.
+This is not a good thing and is not really what we’d like as our default setting.
 
 **Do I have to use update_attributes?**
 
 Why all the fuss about `update_attributes`? If it’s so insecure why use it, why not manually update stuff using code such as
 
     user.name = params[:user][‘name’]
 
-This way surely everything would only be updated if we specifically updated it?
+This way everything would only be updated if we specifically updated it.
 
 We could do but it would take five lines where update_attributes only takes one line. `update_attributes` is also for better or worse, the Rails Way and so it’s a good idea to understand why it’s vulnerable and how to secure it.
 
-**`update_attributes` was how Homakov added himself to the Rails GitHub account**
+**Homakov hacked the Rails core account using update_attributes**
 
-Since update_attributes will update any parameter that’s passed into it, Homakov’s could use it to add an SSH key on his machine to the list of keys associated with the Rails GitHub account.
+Since update_attributes will update any parameter that’s passed into it, Homakov realised be could could use it to add an SSH key on his machine to the list of keys associated with one of the Rails GitHub account members.
 
 Homakov assumed (correctly) that GitHub had a table containing users’ public keys. Each key has a key_value and a `user_id`. Homakov also correctly postulated that he might be able to update his own public key to have the `user_id` of one of the Rails GitHub account members. 
 
-outline of what the GitHub update action looks like for the PublicKey controller:
+*schematic of what the GitHub PublicKey#update method looks like*:
 
     class PublicKeyController < ApplicationController
       before_filter :authorize_user
@@ -93,9 +97,9 @@ outline of what the GitHub update action looks like for the PublicKey controller
       end
     end
 
-Homakov PUT an update to his own existing public key which included a new `user_id`. In this case he used the user_id of a member of the Rails repository team. 
+Homakov PUT an update to his own existing public key which included a new `user_id`. The user_id he used was that of a member of the Rails repository team. 
 
-The update controller then simply took the `user_id` attribute that Homakov passed and updated his key. With an SSH key on his machine registered to the repository  of a Rails member all he then needed to do was push.
+The controller then simply updated all of the parameters Homakov passed to it, including the new user_id. With an SSH key on his machine registered to the repository  of a Rails member all he then needed to do was [push](https://github.com/rails/rails/commit/b83965785db1eec019edf1fc272b1aa393e6dc57). This was the same hack he used for [posting from the future](https://github.com/rails/rails/issues/5239).
 
 **How to protect against this: attr_protected (not recommended)**
 
@@ -222,6 +226,7 @@ If GitHub, one of the best Rails teams on the planet can be taken out so easily
 * On the next release start raising warnings in development when an attempt is made to update an attribute without declaring attr_accessible
 * On the release after raise warnings in production and errors in development
 * On the third release make attr_accessible on by default
+
 ------
 
 Author:  Peter Nixey  
@@ -234,4 +239,4 @@ Blog:    http://peternixey.com  .com/peternixey
 
 * [Egor Homakov’s how-to on how he hacked GitHub](http://homakov.blogspot.com/2012/03/how-to.html)
 * [Rails Spike’s “Is your application safe?”](http://railspikes.com/2008/9/22/is-your-rails-application-safe-from-mass-assignment)
-* [Yehuda Katz’s proposal for addressing mass assignment](https://gist.github.com/1974187)
+* [Yehuda Katz’s proposal for addressing mass assignment](https://gist.github.com/1974187)
diff --git a/securing_rails_updates.md b/securing_rails_updates.md
@@ -200,33 +200,38 @@ I’m sure you’ll hit other issues too but you can generally knock them off by
 
 I wouldn’t pretend to have anything like the oversight of the Rails landscape that the Rails core team do. I’ve only built a very few apps and I’m no guru.
 
-The argument that it is up to the app builder to secure their own app is however dangerous. Rails’ mantra is convention over configuration. If the Rails team are going to stand by this mantra then they also need to accept that the Rails Way to handle updates is conventionally insecure until configured otherwise.
+The argument that it is up to the app builder to secure their own app is however dangerous. Rails’ mantra is convention over configuration. **If the Rails team are going to stand by this mantra then they also need to accept that the Rails Way to handle updates is conventionally insecure until configured otherwise.**
 
 
-**This is not an academic architectural issue, it’s an actual security risk**
+Enforcing that attributes have to be declared `attr_accessible` by default would immediately make things better. 
 
-Making parameters `attr_accessible` by default would immediately make things better. Yehuda Katz makes the point that access is an authorization issue I don’t think this reaches that deep though. The questions is not “where should authorization be handled” but “what should the default setting for authorization be”. 
+**The question is not "where should authorization be handled" it is what is the default setting**
 
-In almost everywhere else the default setting for authorization is unauthorized. You can’t even reach a controller method unless you explicitly create a route for it. update_attributes however defaults to authorized. This is not an academic design choice, it’s one that’s carrying a real world cost right now. Arguing over what layer of the app is responsible is like BP blaming Transocean for the Deepwater Horizon. The issues is that oil is leaking.
+[Yehuda Katz makes the point](https://gist.github.com/1974187) that access is an authorization issue. The question here though is not *“where should authorization be handled”* but *“what should the default setting for authorization be”*. 
+
+In almost everywhere else the default setting for authorization is unauthorized. You can’t even reach a controller method unless you explicitly create a route for it. update_attributes however defaults to authorized. This is not an academic design choice, it’s one that’s carrying a real world cost right now. 
+
+Arguing over what layer of the app is responsible is like BP blaming Transocean for the Deepwater Horizon. The issues is that oil is leaking.
+
+**There are a lot of sites currently vulnerable and more being built**
 
 If GitHub, one of the best Rails teams on the planet can be taken out so easily by such a simple hack then there is a real and present issue. As Yehuda says, not all security vulnerabilities can be fixed by the framework however this one can and it would make sense to do so.
 
-**A three step suggestion for how Rails could address the isssue:**
+**A three step suggestion for how the Rails team could address the isssue:**
 
 * On the next release start raising warnings in development when an attempt is made to update an attribute without declaring attr_accessible
 * On the release after raise warnings in production and errors in development
 * On the third release make attr_accessible on by default
+------
+
+Author:  Peter Nixey  
+Twitter: http://twitter
+Blog:    http://peternixey.com  .com/peternixey
+
+----
 
 *Further reading:*
 
 * [Egor Homakov’s how-to on how he hacked GitHub](http://homakov.blogspot.com/2012/03/how-to.html)
-
 * [Rails Spike’s “Is your application safe?”](http://railspikes.com/2008/9/22/is-your-rails-application-safe-from-mass-assignment)
-
 * [Yehuda Katz’s proposal for addressing mass assignment](https://gist.github.com/1974187)
-
-------
-
-Author:  Peter Nixey  
-Blog:    http://peternixey.com  
-Twitter: http://twitter.com/peternixey
diff --git a/securing_rails_updates.md b/securing_rails_updates.md
@@ -211,10 +211,11 @@ In almost everywhere else the default setting for authorization is unauthorized.
 
 If GitHub, one of the best Rails teams on the planet can be taken out so easily by such a simple hack then there is a real and present issue. As Yehuda says, not all security vulnerabilities can be fixed by the framework however this one can and it would make sense to do so.
 
-Three suggestions:
-- On the next release start raising warnings in development when an attempt is made to update an attribute without declaring attr_accessible
-- On the release after raise warnings in production and errors in development
-- On the third release make attr_accessible on by default
+**A three step suggestion for how Rails could address the isssue:**
+
+* On the next release start raising warnings in development when an attempt is made to update an attribute without declaring attr_accessible
+* On the release after raise warnings in production and errors in development
+* On the third release make attr_accessible on by default
 
 *Further reading:*
 
@@ -226,6 +227,6 @@ Three suggestions:
 
 ------
 
-Author:  Peter Nixey
-Blog:    http://peternixey.com
+Author:  Peter Nixey  
+Blog:    http://peternixey.com  
 Twitter: http://twitter.com/peternixey
diff --git a/securing_rails_updates.md b/securing_rails_updates.md
@@ -224,6 +224,8 @@ Three suggestions:
 
 * [Yehuda Katz’s proposal for addressing mass assignment](https://gist.github.com/1974187)
 
-# Author:  Peter Nixey
-# Blog:    http://peternixey.com
-# Twitter: http://twitter.com/peternixey
+------
+
+Author:  Peter Nixey
+Blog:    http://peternixey.com
+Twitter: http://twitter.com/peternixey
diff --git a/securing_rails_updates.md b/securing_rails_updates.md
@@ -217,8 +217,11 @@ Three suggestions:
 - On the third release make attr_accessible on by default
 
 *Further reading:*
+
 * [Egor Homakov’s how-to on how he hacked GitHub](http://homakov.blogspot.com/2012/03/how-to.html)
+
 * [Rails Spike’s “Is your application safe?”](http://railspikes.com/2008/9/22/is-your-rails-application-safe-from-mass-assignment)
+
 * [Yehuda Katz’s proposal for addressing mass assignment](https://gist.github.com/1974187)
 
 # Author:  Peter Nixey

diff --git a/article.md → securing_rails_updates.md b/article.md → securing_rails_updates.md
@@ -217,9 +217,9 @@ Three suggestions:
 - On the third release make attr_accessible on by default
 
 *Further reading:*
-Egor Homakov’s how-to on how he hacked GitHub: http://homakov.blogspot.com/2012/03/how-to.html 
-Rails Spike’s “Is your application safe?” 
-Yehuda Katz’s proposal for signing attributes
+* [Egor Homakov’s how-to on how he hacked GitHub](http://homakov.blogspot.com/2012/03/how-to.html)
+* [Rails Spike’s “Is your application safe?”](http://railspikes.com/2008/9/22/is-your-rails-application-safe-from-mass-assignment)
+* [Yehuda Katz’s proposal for addressing mass assignment](https://gist.github.com/1974187)
 
 # Author:  Peter Nixey
 # Blog:    http://peternixey.com

diff --git a/article.md b/article.md
@@ -59,158 +59,168 @@ By default, `update_attributes` (which is what you’ll probably use in your upd
 
 We can for instance not just update our name but also our role:
 
-> u.update_attributes name: ‘Jenson Button’,  role: :superadmin;
-> u
- => #<User id: 1, role: "superadmin", name: "Jenson Button", created_at: "2012-03-05 09:39:31", updated_at: "2012-03-05 09:40:53"> 
+    > u.update_attributes name: ‘Jenson Button’,  role: :superadmin;
+    > u
+     => #<User id: 1, role: "superadmin", name: "Jenson Button", created_at: "2012-03-05 09:39:31",
+     updated_at: "2012-03-05 09:40:53"> 
 
 We just updated our role from subscriber to superadmin. This is not a good thing and is not really what we’d like as our default setting.
 
-Do I have to use update_attributes?
+**Do I have to use update_attributes?**
 
-Why all the fuss about update_attributes? If it’s so insecure why use it, why not manually update stuff using code such as
+Why all the fuss about `update_attributes`? If it’s so insecure why use it, why not manually update stuff using code such as
 
-user.name = params[:user][‘name’]
+    user.name = params[:user][‘name’]
 
 This way surely everything would only be updated if we specifically updated it?
 
-We could do but it would take five lines where update_attributes only takes one line. update_attributes is also for better or worse, the Rails Way and so it’s a good idea to understand why it’s vulnerable and how to secure it.
+We could do but it would take five lines where update_attributes only takes one line. `update_attributes` is also for better or worse, the Rails Way and so it’s a good idea to understand why it’s vulnerable and how to secure it.
 
-update_attributes was how Homakov added himself to the Rails GitHub account
+**`update_attributes` was how Homakov added himself to the Rails GitHub account**
 
 Since update_attributes will update any parameter that’s passed into it, Homakov’s could use it to add an SSH key on his machine to the list of keys associated with the Rails GitHub account.
 
-Homakov assumed (correctly) that GitHub had a table containing users’ public keys. Each key has a key_value and a user_id. Homakov also correctly postulated that he might be able to update his own public key to have the user_id of one of the Rails GitHub account members. 
+Homakov assumed (correctly) that GitHub had a table containing users’ public keys. Each key has a key_value and a `user_id`. Homakov also correctly postulated that he might be able to update his own public key to have the `user_id` of one of the Rails GitHub account members. 
 
 outline of what the GitHub update action looks like for the PublicKey controller:
 
-class PublicKeyController < ApplicationController
-  before_filter :authorize_user
-  ...
-  def update
-    @current_key = PublicKey.find_by_id params[:key][‘id’]
-    @current_key.update_attributes(params[:key])
-  end
-end
+    class PublicKeyController < ApplicationController
+      before_filter :authorize_user
+      ...
+      def update
+        @current_key = PublicKey.find_by_id params[:key][‘id’]
+        @current_key.update_attributes(params[:key])
+      end
+    end
 
-Homakov PUT an update to his own existing public key which included a new user_id. In this case he used the user_id of a member of the Rails repository team. 
+Homakov PUT an update to his own existing public key which included a new `user_id`. In this case he used the user_id of a member of the Rails repository team. 
 
-The update controller then simply took the :user_id attribute that Homakov passed and updated his key. With an SSH key on his machine registered to the repository  of a Rails member all he then needed to do was push.
+The update controller then simply took the `user_id` attribute that Homakov passed and updated his key. With an SSH key on his machine registered to the repository  of a Rails member all he then needed to do was push.
 
-How to protect against this: attr_protected (not recommended)
+**How to protect against this: attr_protected (not recommended)**
 
-Everything that happened happened because the user_id attribute should not have been updatable via update_attributes. Rails has a method for exactly this and it’s called attr_protected. 
+Everything that happened happened because the user_id attribute should not have been updatable via update_attributes. Rails has a method for exactly this and it’s called `attr_protected`. 
 
-class User < ActiveRecord::Base
-  attr_protected :role
-end
+    class User < ActiveRecord::Base
+      attr_protected :role
+    end
 
 With that line added it doesn’t matter whether we pass the role in via a PUT, it still won’t update:
 
-u = User.create name: "Peter Nixey", role: :subscriber;
-u.update_attributes role: :superadmin
-WARNING: Can't mass-assign protected attributes: role
- => true 
+    u = User.create name: "Peter Nixey", role: :subscriber;
+    u.update_attributes role: :superadmin
+    WARNING: Can't mass-assign protected attributes: role
+     => true 
 
-however, attr_protected only protects us on attributes we actually add it to
+however, `attr_protected` only protects us on attributes we actually add it to
 
-The problem with attr_protected is that it only works when we remember to add it. If we don’t put it in we don’t get protection. 
+The problem with `attr_protected` is that it only works when we remember to add it. If we don’t put it in we don’t get protection. 
 
-I prefer to know I’m protected and safe until I chose to be unsafe and that (in theory) is what attr_accessible gives us.
+I prefer to know I’m protected and safe until I chose to be unsafe and that (in theory) is what `attr_accessible` gives us.
+
+`attr_accessible` protects all attributes unless we declare otherwise
+
+`attr_accessible` is the recommended method of tackling this problem. It’s actually a little bit of a misnomer since its value is not in making an attribute accessible (it already was). Its value is that that adding it implies that all the other attributes are not accessible. Think of its real value less as being `attr_accessible` and more as being *attr_whitelist*
+
+    class User < ActiveRecord::Base
+      attr_accessible :name
+    end
 
-attr_accessible protects all attributes unless we declare otherwise
 
-attr_accessible is the recommended method of tackling this problem. It’s actually a little bit of a misnomer since its value is not in making an attribute accessible (it already was). Its value is that that adding it implies that all the other attributes are not accessible. Think of its real value less as being attr_accessible and more as being attr_whitelist
+    u = User.create name: "Peter Nixey", role: :subscriber;
+    u.update_attributes role: :superadmin
+    WARNING: Can't mass-assign protected attributes: role
+     => true 
 
-class User < ActiveRecord::Base
-  attr_accessible :name
-end
+The nice thing about `attr_accessible` is that all new attributes are protected by default. If we add an account type to our database (but leave the model unchanged)
 
-u = User.create name: "Peter Nixey", role: :subscriber;
-u.update_attributes role: :superadmin
-WARNING: Can't mass-assign protected attributes: role
- => true 
 
-The nice thing about attr_accessible is that all new attributes are protected by default. If we add an account type to our database (but leave the model unchanged)
+    class AddAccountType < ActiveRecord::Migration
+      def change
+        add_column :users, :account_type, :string
+      end
+    end
 
-class AddAccountType < ActiveRecord::Migration
-  def change
-    add_column :users, :account_type, :string
-  end
-end
 
-class User < ActiveRecord::Base
-  attr_accessible :name
-end
+    class User < ActiveRecord::Base
+      attr_accessible :name
+    end
 
 ... the :account_type field is automatically protected
 
-u = User.create name: “Peter Nixey”, account_type: ‘free’
-u.update_attributes account_type: ‘paid’
-WARNING: Can't mass-assign protected attributes: account_type
- => true 
+    u = User.create name: “Peter Nixey”, account_type: ‘free’
+    u.update_attributes account_type: ‘paid’
+    WARNING: Can't mass-assign protected attributes: account_type
+     => true 
 
-Either way you can still manually update attributes
+**Either way you can still manually update attributes**
 
 We can still update role directly it’s simply that it’s not vulnerable to an update through our update controller:
 
-u.role = :superadmin
-u.save
-   (0.2ms)  UPDATE "users" SET "role" = 'superadmin', "updated_at" = '2012-03-05 10:11:05.042023' WHERE "users"."id" = 1
- => true 
+    u.role = :superadmin
+    u.save
+       (0.2ms)  UPDATE "users" SET "role" = 'superadmin', 
+       "updated_at" = '2012-03-05 10:11:05.042023' WHERE "users"."id" = 1
+     => true 
 
-However, even attr_accessible only protects us when we remember it
+**However, even `attr_accessible` only protects us when we remember it**
 
-The problem with attr_protected was that it only protected us when we remembered to add it to the attribute.
+The problem with `attr_protected` was that it only protected us when we remembered to add it to the attribute.
 
-The problem with attr_accessible is that it only protects us when we remember to add it to the model. Sometimes (as GitHub showed us) it’s easy to forget to do that.
+The problem with `attr_accessible` is that it only protects us when we remember to add it to the model. Sometimes (as GitHub showed us) it’s easy to forget to do that.
 
-The disable_mass_assignment initializer gives security by default
+**The disable_mass_assignment initializer gives security by default**
 
 The beauty of the initializer:
 
-config/initializers/disable_mass_assignment.rb
-   ActiveRecord::Base.send(:attr_accessible, nil)
+*config/initializers/disable_mass_assignment.rb*
 
-is that it effectively adds attr_accessible to every model we create (actually what it does it take it away by default but it comes to the same thing). No attribute can be updated unless we declare it attr_accessible. We’re secure until we decide not to be.
+    ActiveRecord::Base.send(:attr_accessible, nil)
 
-Possible issues you might have with the initializer
+is that it effectively adds `attr_accessible` to every model we create (actually what it does it take it away by default but it comes to the same thing). No attribute can be updated unless we declare it `attr_accessible`. We’re secure until we decide not to be.
 
-If you add attr_accessible the first thing you’re going to need to do is to declare all attributes accessible which are accessible. 
+**Possible issues you might have with the initializer**
 
-A good test suite will help a lot here but with or without one, go through each model adding each parameter that you want accessible to your attr_accessible array:
+If you add `attr_accessible` the first thing you’re going to need to do is to declare all attributes accessible which are accessible. 
 
-class Xyz
-  attr_accessible attribute_a, attribute_b, attribute_c
-end
+A good test suite will help a lot here but with or without one, go through each model adding each parameter that you want accessible to your `attr_accessible` arguments:
+
+    class Xyz
+      attr_accessible attribute_a, attribute_b, attribute_c
+    end
 
 You’re going to have some frustrations. There are going to be things that you don’t see coming which will fail silently. Problems I’ve had are:
 
-Authlogic: you need to remember to make attributes like password, email etc accessible
-Paperclip: remember to make paperclip attributes accessible
+- Authlogic: you need to remember to make attributes like password, email etc accessible
+- Paperclip: remember to make paperclip attributes accessible
 
 I’m sure you’ll hit other issues too but you can generally knock them off by adding attributes one by one to the list of accessible ones.
 
-How Rails could address this
+**How Rails could address this**
 
 I wouldn’t pretend to have anything like the oversight of the Rails landscape that the Rails core team do. I’ve only built a very few apps and I’m no guru.
 
 The argument that it is up to the app builder to secure their own app is however dangerous. Rails’ mantra is convention over configuration. If the Rails team are going to stand by this mantra then they also need to accept that the Rails Way to handle updates is conventionally insecure until configured otherwise.
 
 
-This is not an academic architectural issue, it’s an actual security risk
+**This is not an academic architectural issue, it’s an actual security risk**
 
-Making parameters attr_accessible by default would immediately make things better. Yehuda Kat makes the point that access is an authorization issue I don’t think this reaches that deep though. The questions is not “where should authorization be handled” but “what should the default setting for authorization be”. 
+Making parameters `attr_accessible` by default would immediately make things better. Yehuda Katz makes the point that access is an authorization issue I don’t think this reaches that deep though. The questions is not “where should authorization be handled” but “what should the default setting for authorization be”. 
 
 In almost everywhere else the default setting for authorization is unauthorized. You can’t even reach a controller method unless you explicitly create a route for it. update_attributes however defaults to authorized. This is not an academic design choice, it’s one that’s carrying a real world cost right now. Arguing over what layer of the app is responsible is like BP blaming Transocean for the Deepwater Horizon. The issues is that oil is leaking.
 
 If GitHub, one of the best Rails teams on the planet can be taken out so easily by such a simple hack then there is a real and present issue. As Yehuda says, not all security vulnerabilities can be fixed by the framework however this one can and it would make sense to do so.
 
 Three suggestions:
-On the next release start raising warnings in development when an attempt is made to update an attribute without declaring attr_accessible
-On the release after raise warnings in production and errors in development
-On the third release make attr_accessible on by default
+- On the next release start raising warnings in development when an attempt is made to update an attribute without declaring attr_accessible
+- On the release after raise warnings in production and errors in development
+- On the third release make attr_accessible on by default
 
-Further reading:
-Egor Homakov’s how-to on how he hacked GitHub 
+*Further reading:*
+Egor Homakov’s how-to on how he hacked GitHub: http://homakov.blogspot.com/2012/03/how-to.html 
 Rails Spike’s “Is your application safe?” 
-Yehuda Katz’s proposal for signing attributes
+Yehuda Katz’s proposal for signing attributes
+
+# Author:  Peter Nixey
+# Blog:    http://peternixey.com
+# Twitter: http://twitter.com/peternixey
diff --git a/article.md b/article.md
@@ -4,7 +4,7 @@
 
 He was right. The Rails defaults are vulnerable and there’s no better illustration of this than when when one of the best Rails teams in the world is severely compromised.
 
-How to protect your Rails application from the GitHub attack
+**How to protect your Rails application from the GitHub attack**
 
 Add the following initializer:
 
@@ -18,7 +18,7 @@ The initalizer forces all models in your application to whitelist parameters tha
 
 The initializer switches this round and makes whitelisting the default position. With the intializer switched on, update_attributes will only update attributes on your models which are declared attr_accessible.
 
-also see the last paragraph of this article for for possible issues with enabling this
+(also see the last paragraph of this article for for possible issues with enabling this)
 
 **How update_attributes works by default**
 
@@ -50,11 +50,12 @@ We now have a user with the following attributes:
 
 **How the default behaviour is vulnerable (& where the whole hoo-ha began)**
 
-> u = User.create name: ‘Peter Nixey’, role: :subscriber;
-> u
- => #<User id: 1, role: :admin, name: "Peter Nixey", created_at: "2012-03-05 09:39:31", updated_at: "2012-03-05 09:39:31"> 
+    > u = User.create name: ‘Peter Nixey’, role: :subscriber;
+    > u
+     => #<User id: 1, role: :admin, name: "Peter Nixey", created_at: "2012-03-05 09:39:31",
+          updated_at: "2012-03-05 09:39:31"> 
 
-By default, update_attributes (which is what you’ll probably use in your update method) updates any attributes that are passed in via http params.  It’s wonderfully quick and simple but also vulnerable. 
+By default, `update_attributes` (which is what you’ll probably use in your update method) updates any attributes that are passed in via http `params`.  It’s wonderfully quick and simple but also vulnerable. 
 
 We can for instance not just update our name but also our role: