Merge pull request #894 from jcheatham/master

Adds obsolete ignore filter identification functionality

lib/brakeman/report/ignore/config.rb

@@ -11,6 +11,7 @@ def initialize file, new_warnings
       @new_warnings = new_warnings
       @already_ignored = []
       @ignored_fingerprints = Set.new
+      @used_fingerprints = Set.new
       @notes = {}
       @shown_warnings = @ignored_warnings = nil
       @changed = false
@@ -43,6 +44,7 @@ def unignore warning
 
     # Determine if warning should be ignored
     def ignored? warning
+      @used_fingerprints << warning.fingerprint
       @ignored_fingerprints.include? warning.fingerprint
     end
 
@@ -75,6 +77,11 @@ def note_for warning
       nil
     end
 
+    # The set of unused ignore entries
+    def obsolete_fingerprints
+      (@ignored_fingerprints - @used_fingerprints).to_a
+    end
+
     # Read configuration to file
     def read_from_file file = @file
       if File.exist? file

lib/brakeman/report/report_base.rb

@@ -79,6 +79,11 @@ def generate_errors
     render_array('error_overview', ['Error', 'Location'], values, {:tracker => tracker})
   end
 
+  def generate_obsolete
+    values = tracker.unused_fingerprints.collect{|fingerprint| [fingerprint] }
+    render_array('obsolete_ignore_entries', ['fingerprint'], values, {:tracker => tracker})
+  end
+
   def generate_warnings
     render_warnings generic_warnings,
                     :warning,

lib/brakeman/report/report_json.rb

@@ -2,6 +2,8 @@ class Brakeman::Report::JSON < Brakeman::Report::Base
   def generate_report
     errors = tracker.errors.map{|e| { :error => e[:error], :location => e[:backtrace][0] }}
 
+    obsolete = tracker.unused_fingerprints
+
     warnings = convert_to_hashes all_warnings
 
     ignored = convert_to_hashes ignored_warnings
@@ -26,7 +28,8 @@ def generate_report
       :scan_info => scan_info,
       :warnings => warnings,
       :ignored_warnings => ignored,
-      :errors => errors
+      :errors => errors,
+      :obsolete => obsolete
     }
 
     JSON.pretty_generate report_info

lib/brakeman/report/report_table.rb

@@ -25,6 +25,7 @@ def generate_report
       truncate_table(generate_templates.to_s) << "\n"
     end
 
+    output_table("+Obsolete Ignore Entries+", generate_obsolete, out)
     output_table("+Errors+", generate_errors, out)
     output_table("+SECURITY WARNINGS+", generate_warnings, out)
     output_table("Controller Warnings:", generate_controller_warnings, out)

lib/brakeman/tracker.rb

@@ -185,6 +185,11 @@ def filtered_warnings
     end
   end
 
+  def unused_fingerprints
+    return [] unless self.ignored_filter
+    self.ignored_filter.obsolete_fingerprints
+  end
+
   def add_constant name, value, context = nil
     @constants.add name, value, context unless @options[:disable_constant_tracking]
   end

test/apps/rails4/config/brakeman.ignore

@@ -17,6 +17,24 @@
       "user_input": null,
       "confidence": "High",
       "note": "skipping this for a test"
+    },
+    {
+      "warning_type": "Mass Assignment",
+      "warning_code": 60,
+      "fingerprint": "abcdef01234567890ba28050e7faf1d54f218dfa9435c3f65f47cb378c18cf98",
+      "message": "Potentially dangerous attribute available for mass assignment",
+      "file": "app/models/account.rb",
+      "line": null,
+      "link": "http://brakemanscanner.org/docs/warning_types/mass_assignment/",
+      "code": ":admin",
+      "render_path": null,
+      "location": {
+        "type": "model",
+        "model": "Account"
+      },
+      "user_input": null,
+      "confidence": "High",
+      "note": "this error should be detected as obsolete"
     }
   ],
   "updated": "2013-12-20 22:14:42 +0200",

test/tests/json_output.rb

@@ -2,7 +2,7 @@
 
 class JSONOutputTests < Minitest::Test
   def setup
-    @@json ||= JSON.parse(Brakeman.run("#{TEST_PATH}/apps/rails3.2").report.to_json)
+    @@json ||= JSON.parse(Brakeman.run("#{TEST_PATH}/apps/rails4").report.to_json)
   end
 
   def test_for_render_path
@@ -13,7 +13,7 @@ def test_for_render_path
   end
 
   def test_for_expected_keys
-    assert (@@json.keys - ["warnings", "ignored_warnings", "scan_info", "errors"]).empty?
+    assert (@@json.keys - ["warnings", "ignored_warnings", "scan_info", "errors", "obsolete"]).empty?
   end
 
   def test_for_scan_info_keys
@@ -37,6 +37,10 @@ def test_for_errors
     assert @@json["errors"].is_a? Array
   end
 
+  def test_for_obsolete
+    assert_equal ["abcdef01234567890ba28050e7faf1d54f218dfa9435c3f65f47cb378c18cf98"], @@json["obsolete"]
+  end
+
   def test_paths
     assert @@json["warnings"].all? { |w| not w["file"].start_with? "/" }
   end

test/tests/report_generation.rb

@@ -2,7 +2,7 @@
 
 class TestReportGeneration < Minitest::Test
   def setup
-    @@tracker||= Brakeman.run(:app_path => "#{TEST_PATH}/apps/rails3.2", :quiet => true, :report_routes => true)
+    @@tracker||= Brakeman.run(:app_path => "#{TEST_PATH}/apps/rails4", :quiet => true, :report_routes => true)
     @@report ||= @@tracker.report
   end
 
@@ -49,6 +49,13 @@ def test_csv_report_no_warnings
     end
   end
 
+  def test_obsolete_reporting
+    report = @@report.to_s
+
+    assert report.include? "+Obsolete Ignore Entries+"
+    assert report.include? "abcdef01234567890ba28050e7faf1d54f218dfa9435c3f65f47cb378c18cf98"
+  end
+
   def test_tabs_sanity
     report = @@report.to_tabs