Skip to content

Commit

Permalink
Merge pull request #1623 from bradparker/revert-file-path-refactor
Browse files Browse the repository at this point in the history
Revert "refactor: IgnoreConfig uses FilePath"
  • Loading branch information
presidentbeef committed Jul 20, 2021
2 parents 53db6f5 + d517c99 commit b6b73a9
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 24 deletions.
10 changes: 4 additions & 6 deletions lib/brakeman.rb
Expand Up @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/brakeman/commandline.rb
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions lib/brakeman/report/ignore/config.rb
Expand Up @@ -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

Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/brakeman/report/report_sarif.rb
Expand Up @@ -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%',
},
},
Expand Down
6 changes: 3 additions & 3 deletions test/tests/brakeman.rb
Expand Up @@ -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
Expand All @@ -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

Expand Down
12 changes: 3 additions & 9 deletions test/tests/ignore.rb
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit b6b73a9

Please sign in to comment.