Brakeman does not catch lack of protect_from_forgery in Rails engines

[tomekr]

It looks like the CheckForgerySetting will only check ApplicationController[1]. This becomes an issue when running the test against a Rails engine as the rails engine generator will throw the ApplicationController into a namespace (i.e. Foobar::ApplicationController) which the check will be unable to find. With help from @jjarmoc, we came up with putting the entire check into a block that grabs all ApplicationControllers via a regex:

  def run_check
    tracker.controllers.keys.select{|key| key =~ /.*ApplicationController/ }.each do |controller_key|
      app_controller = tracker.controllers[controller_key]

      return unless ancestor? app_controller, :"ActionController::Base"
      ...

This should catch CSRF when running brakeman against the Rails engine itself, however it becomes more complicated when attempting to find it in Rails engines that are in other Rails applications. It looks like engines used to be generated in the engines/ directory, but when following the documentation[2], the engine just shows up in the root as a directory with whatever name you give it. A possible way to detect this is via the routes file where AFAIK is how you tell Rails you want to use the engine using something like mount Blorgh::Engine => "/blorgh". This should tell you the Rails engines being used but I'm not sure how you would differentiate between a Rails engine that's a part of the source and a Rails engine that's coming from a gem. Maybe checking for the blorgh directory in the root and if it's not there assume it's coming from a gem?

[1]

brakeman/lib/brakeman/checks/check_forgery_setting.rb

Line 13 in a3dab55

app_controller = tracker.controllers[:ApplicationController]

[2] http://guides.rubyonrails.org/engines.html#generating-an-engine

[presidentbeef]

Hi Tomek,

Brakeman support for engines is quite spotty and I'm sure there is room for improvement. Right now it does load files from the /engines directory. Using the mount call to find the engine seems like a good idea to me. If the path can't be found, then it can't be found and it's not a big deal. The next step would probably be adding an option so you could point it at the engine directory - that would allow for pointing it at gems too.

[presidentbeef]

Fixed with #953 and other referenced PRs.