From d517c9923fc3c97dab39ea887a2fc263eaee280c Mon Sep 17 00:00:00 2001 From: Brad Parker Date: Tue, 20 Jul 2021 09:12:16 +1000 Subject: [PATCH] Revert "refactor: IgnoreConfig uses FilePath" This reverts commit 52bfc9b187dcc898f631124bc841823de6117ca5. It seems that this might break --interactive-ignore. Specifically on line 39 of lib/brakeman/report/interactive.rb. https://github.com/presidentbeef/brakeman/blob/53db6f585679f648ce761421a9eba2e02e18a4a1/lib/brakeman/report/ignore/interactive.rb#L39 When asking for the ignore file location `@ignore_config.file` is checked for emptiness but Brakeman::FilePath has no `empty?` method and so this fails. --- lib/brakeman.rb | 10 ++++------ lib/brakeman/commandline.rb | 2 +- lib/brakeman/report/ignore/config.rb | 8 ++++---- lib/brakeman/report/report_sarif.rb | 2 +- test/tests/brakeman.rb | 6 +++--- test/tests/ignore.rb | 12 +++--------- 6 files changed, 16 insertions(+), 24 deletions(-) diff --git a/lib/brakeman.rb b/lib/brakeman.rb index b2edc4bb0d..a127b6524b 100644 --- a/lib/brakeman.rb +++ b/lib/brakeman.rb @@ -527,14 +527,12 @@ def self.load_brakeman_dependency name, allow_fail = false # Returns an array of alert fingerprints for any ignored warnings without # notes found in the specified ignore file (if it exists). - def self.ignore_file_entries_with_empty_notes file, options + def self.ignore_file_entries_with_empty_notes file return [] unless file require 'brakeman/report/ignore/config' - app_tree = Brakeman::AppTree.from_options(options) - - config = IgnoreConfig.new(Brakeman::FilePath.from_app_tree(app_tree, file), nil) + config = IgnoreConfig.new(file, nil) config.read_from_file config.already_ignored_entries_with_empty_notes.map { |i| i[:fingerprint] } end @@ -545,9 +543,9 @@ def self.filter_warnings tracker, options app_tree = Brakeman::AppTree.from_options(options) if options[:ignore_file] - file = Brakeman::FilePath.from_app_tree(app_tree, options[:ignore_file]) + file = options[:ignore_file] elsif app_tree.exists? "config/brakeman.ignore" - file = Brakeman::FilePath.from_app_tree(app_tree, "config/brakeman.ignore") + file = app_tree.expand_path("config/brakeman.ignore") elsif not options[:interactive_ignore] return end diff --git a/lib/brakeman/commandline.rb b/lib/brakeman/commandline.rb index 2a585f1d4b..0656db8a16 100644 --- a/lib/brakeman/commandline.rb +++ b/lib/brakeman/commandline.rb @@ -126,7 +126,7 @@ def regular_report options ensure_ignore_notes_failed = false if tracker.options[:ensure_ignore_notes] - fingerprints = Brakeman::ignore_file_entries_with_empty_notes tracker.ignored_filter&.file, options + fingerprints = Brakeman::ignore_file_entries_with_empty_notes tracker.ignored_filter&.file unless fingerprints.empty? ensure_ignore_notes_failed = true diff --git a/lib/brakeman/report/ignore/config.rb b/lib/brakeman/report/ignore/config.rb index 3f3c51c33c..670573bfe2 100644 --- a/lib/brakeman/report/ignore/config.rb +++ b/lib/brakeman/report/ignore/config.rb @@ -100,14 +100,14 @@ def already_ignored_entries_with_empty_notes # Read configuration to file def read_from_file file = @file - if File.exist? file.absolute + if File.exist? file begin @already_ignored = JSON.parse(File.read(file), :symbolize_names => true)[:ignored_warnings] rescue => e - raise e, "\nError[#{e.class}] while reading brakeman ignore file: #{file.relative}\n" + raise e, "\nError[#{e.class}] while reading brakeman ignore file: #{file}\n" end else - Brakeman.notify "[Notice] Could not find ignore configuration in #{file.relative}" + Brakeman.notify "[Notice] Could not find ignore configuration in #{file}" @already_ignored = [] end @@ -134,7 +134,7 @@ def save_to_file warnings, file = @file :brakeman_version => Brakeman::Version } - File.open file.absolute, "w" do |f| + File.open file, "w" do |f| f.puts JSON.pretty_generate(output) end end diff --git a/lib/brakeman/report/report_sarif.rb b/lib/brakeman/report/report_sarif.rb index 54bd3049bb..c6714941a2 100644 --- a/lib/brakeman/report/report_sarif.rb +++ b/lib/brakeman/report/report_sarif.rb @@ -80,7 +80,7 @@ def results :location => { :physicalLocation => { :artifactLocation => { - :uri => @ignore_filter.file.relative, + :uri => Brakeman::FilePath.from_app_tree(@app_tree, @ignore_filter.file).relative, :uriBaseId => '%SRCROOT%', }, }, diff --git a/test/tests/brakeman.rb b/test/tests/brakeman.rb index 8a6460e00e..64654d9cb5 100644 --- a/test/tests/brakeman.rb +++ b/test/tests/brakeman.rb @@ -387,13 +387,13 @@ def test_ensure_latest end def test_ignore_file_entries_with_empty_notes - assert Brakeman.ignore_file_entries_with_empty_notes(nil, {}).empty? + assert Brakeman.ignore_file_entries_with_empty_notes(nil).empty? ignore_file_missing_notes = Tempfile.new('brakeman.ignore') ignore_file_missing_notes.write IGNORE_WITH_MISSING_NOTES_JSON ignore_file_missing_notes.close assert_equal( - Brakeman.ignore_file_entries_with_empty_notes(ignore_file_missing_notes.path, {:app_path => "/tmp" }).to_set, + Brakeman.ignore_file_entries_with_empty_notes(ignore_file_missing_notes.path).to_set, [ '006ac5fe3834bf2e73ee51b67eb111066f618be46e391d493c541ea2a906a82f', ].to_set @@ -403,7 +403,7 @@ def test_ignore_file_entries_with_empty_notes ignore_file_with_notes = Tempfile.new('brakeman.ignore') ignore_file_with_notes.write IGNORE_WITH_NOTES_JSON ignore_file_with_notes.close - assert Brakeman.ignore_file_entries_with_empty_notes(ignore_file_with_notes.path, {:app_path => "/tmp" }).empty? + assert Brakeman.ignore_file_entries_with_empty_notes(ignore_file_with_notes.path).empty? ignore_file_with_notes.unlink end diff --git a/test/tests/ignore.rb b/test/tests/ignore.rb index 7cd2c171b2..45a8bf4010 100644 --- a/test/tests/ignore.rb +++ b/test/tests/ignore.rb @@ -16,8 +16,7 @@ def setup end def make_config file = @config_file.path - app_tree = Brakeman::AppTree.from_options({:app_path => app_path}) - c = Brakeman::IgnoreConfig.new Brakeman::FilePath.from_app_tree(app_tree, file), report.warnings + c = Brakeman::IgnoreConfig.new file, report.warnings c.read_from_file c.filter_ignored c @@ -28,11 +27,7 @@ def teardown end def report - @@report ||= Brakeman.run(app_path) - end - - def app_path - @@app_path ||= File.join(TEST_PATH, "apps", "rails5.2") + @@report ||= Brakeman.run(File.join(TEST_PATH, "apps", "rails5.2")) end def test_sanity @@ -186,8 +181,7 @@ def test_bad_ignore_json_error_message file.write "{[ This is bad json cuz I don't have a closing square bracket, bwahahaha...}" file.close begin - app_tree = Brakeman::AppTree.from_options({:app_path => app_path}) - c = Brakeman::IgnoreConfig.new Brakeman::FilePath.from_app_tree(app_tree, file.path), report.warnings + c = Brakeman::IgnoreConfig.new file.path, report.warnings c.read_from_file rescue => e # The message should clearly show that there was a problem parsing the json