Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reporting compress option and sarif filter update #844

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions lib/salus/plugin_manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,12 @@ def register_listener(listener)
@@listners << listener
end

def apply_filter(filter_family, filter_method, data)
def apply_filter(filter_family, filter_method, *data)
result = data&.first
@@filters[filter_family]&.each do |f|
data = f.__send__(filter_method, data) if f.respond_to?(filter_method)
result = f.__send__(filter_method, *data) if f.respond_to?(filter_method)
end
data
result
end

def send_event(event_name, *data)
Expand Down
18 changes: 15 additions & 3 deletions lib/salus/report.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
require 'deepsort'
require 'salus/formatting'
require 'salus/bugsnag'
require 'zlib'

# Adding aliases to prevent deep_sort from failing when comparing symbols and strings
class Symbol
Expand Down Expand Up @@ -63,8 +64,8 @@ def apply_report_hash_filters(report_hash)
Salus::PluginManager.apply_filter(:salus_report, :filter_report_hash, report_hash)
end

def apply_report_sarif_filters(sarif_json)
Salus::PluginManager.apply_filter(:salus_report, :filter_report_sarif, sarif_json)
def apply_report_sarif_filters(sarif_json, config = nil)
Salus::PluginManager.apply_filter(:salus_report, :filter_report_sarif, sarif_json, config)
end

# Syntatical sugar register salus_report filters
Expand Down Expand Up @@ -197,6 +198,8 @@ def to_json
def to_sarif(config = {})
sarif_json = Sarif::SarifReport.new(@scan_reports, config, @repo_path, @config).to_sarif
begin
# This is dangerous in salus as rule mappings use the ruleIndex
# which can change when a deep sort is executed
sorted_sarif = JSON.parse(sarif_json).deep_sort
rescue StandardError => e
bugsnag_notify(e.inspect + "\n" + e.message + "\nResult String: " + to_h.to_s)
Expand All @@ -205,8 +208,9 @@ def to_sarif(config = {})
# We will validate to ensure the applied filter
# doesn't produce any invalid SARIF
sarif_json = JSON.pretty_generate(sorted_sarif)
Sarif::SarifReport.validate_sarif(apply_report_sarif_filters(sarif_json))
Sarif::SarifReport.validate_sarif(apply_report_sarif_filters(sarif_json, config))
rescue StandardError => e
puts "Failure in validing SARIF"
bugsnag_notify(e.class.to_s + " " + e.message + "\nBuild Info:" + @builds.to_s)
end

Expand Down Expand Up @@ -300,6 +304,7 @@ def publish_report(directive)
else
raise ExportReportError, "unknown report format #{directive['format']}"
end

if Salus::Config::REMOTE_URI_SCHEME_REGEX.match?(URI(uri).scheme)
Salus::ReportRequest.send_report(directive, report_body(directive), uri)
else
Expand Down Expand Up @@ -395,12 +400,19 @@ def write_report_to_file(report_file_path, report_string)
"Cannot write file #{report_file_path} - #{e.class}: #{e.message}"
end

def compress(data)
Base64.strict_encode64(Zlib::Deflate.deflate(data))
end

def report_body_hash(config, data)
return data unless config&.key?('post') && config['post'].present?

body_hash = config['post']['additional_params'] || {}
return body_hash unless config['post']['salus_report_param_name']

compress_sarif = config.dig('post', 'salus_report_options', 'gzip-base64')
data = compress(JSON.pretty_generate(data)) if ["true", true].include?(compress_sarif)

body_hash[config['post']['salus_report_param_name']] = data
body_hash
end
Expand Down
2 changes: 2 additions & 0 deletions lib/salus/scanners/brakeman.rb
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ def merged_ignore_file_contents
def ignore_list
return [] unless user_supplied_ignore?

return [] unless File.exist?(@config['ignore'])

data = JSON.parse(File.read(@config['ignore']))
return [] unless data.key?('ignored_warnings')

Expand Down
17 changes: 15 additions & 2 deletions lib/sarif/base_sarif.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
require 'json'
require 'set'
require 'sarif/shared_objects'
require 'deepsort'

module Sarif
class BaseSarif
Expand Down Expand Up @@ -168,13 +169,25 @@ def build_runs_object(supported)

# Salus::ScanReport
invocation = build_invocations(@scan_report, supported)
{
"tool" => build_tool(rules: rules),
runs_object = {
"tool" => build_tool(rules: rules.deep_sort), # we deep sort here as
# our SARIF needs to be deep sorted for easier comparisions
"conversion" => build_conversion,
"results" => results,
"invocations" => [invocation],
"originalUriBaseIds" => uri_info
}
# Ensure our ruleIndex values are correct after the
# prior deep sorting
remap_rule_ids(runs_object)
end

def remap_rule_ids(run)
rules = run['tool'][:driver]['rules']
run['results'].each do |r|
r[:ruleIndex] = rules.index { |rule| rule[:id] == r[:ruleId] }
end
run
end

# Returns the conversion object for the SARIF report
Expand Down
12 changes: 11 additions & 1 deletion spec/fixtures/npm_audit/success_with_exceptions/salus-sarif.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,14 @@ scanner_configs:
advisory_id: "1091018",
changed_by: "joshua.ostrom",
notes: "See https://www.npmjs.com/advisories/48. We're not vulnerable to this because this is a regex dos and we have nothing that puts user input into it. The impact is also minimal.",
}
}
- {
advisory_id: "1091686",
changed_by: "joshua.ostrom",
notes: "WAGMI",
}
- {
advisory_id: "1091710",
changed_by: "joshua.ostrom",
notes: "BTC $26.5K",
}
36 changes: 0 additions & 36 deletions spec/fixtures/sarifs/diff/git_diff_yarn.txt

This file was deleted.

4 changes: 2 additions & 2 deletions spec/fixtures/sorted_results/sorted_sarif.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
"text": "Useless equality test.. Pattern 1 == $X is required but not found."
},
"ruleId": "Required Pattern Not Found",
"ruleIndex": 1
"ruleIndex": 0
},
{
"level": "error",
Expand All @@ -53,7 +53,7 @@
"text": "Syntax error at line /home/spec/fixtures/semgrep/invalid/unparsable_py.py:3:\n `print(\"foo\"` was unexpected"
},
"ruleId": "SAL002",
"ruleIndex": 0
"ruleIndex": 1
}
],
"tool": {
Expand Down
2 changes: 1 addition & 1 deletion spec/lib/sarif/brakeman_sarif_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@

# Check result info
expect(result['ruleId']).to eq('13')
expect(result['ruleIndex']).to eq(0)
expect(result['ruleIndex']).to eq(2)
expect(result['level']).to eq('error')
expect(result['locations'][0]['physicalLocation']['region']['startLine']).to eq(3)
snippet = result['locations'][0]['physicalLocation']['region']['snippet']['text'].to_s
Expand Down
2 changes: 1 addition & 1 deletion spec/lib/sarif/osv/maven_osv_sarif_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ def stub_req_with_valid_response
"severity" => "HIGH"
},
"ruleId" => "CVE-2018-15756",
"ruleIndex" => 5
"ruleIndex" => 0
}
)

Expand Down
2 changes: 1 addition & 1 deletion spec/lib/sarif/pattern_search_sarif_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
expect(results).to include(
{
"ruleId": "Forbidden Pattern Found",
"ruleIndex": 0,
"ruleIndex": 1,
"level": "error",
"message": {
"text": "not important string. Pattern Nerv is forbidden."
Expand Down
2 changes: 1 addition & 1 deletion spec/lib/sarif/semgrep_sarif_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@
expect(result).to include(
{
"ruleId" => "Required Pattern Not Found",
"ruleIndex" => 1,
"ruleIndex" => 0,
"level" => "error",
"message" => {
"text" => "Useless equality test.. Pattern 1 == $X is required but not found."
Expand Down
6 changes: 3 additions & 3 deletions spec/lib/sarif/trufflehog_sarif_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
} }],
"message" => { "text" => "Leaked credential detected" },
"properties" => { "severity" => "high" },
"ruleId" => "FlatIO-PLAIN", "ruleIndex" => 1 }
"ruleId" => "FlatIO-PLAIN", "ruleIndex" => 0 }
expected_vul1 = { "level" => "error",
"locations" => [{ "physicalLocation" => {
"artifactLocation" => { "uri" => "url.txt",
Expand All @@ -40,7 +40,7 @@
} }],
"message" => { "text" => "Leaked credential detected" },
"properties" => { "severity" => "high" },
"ruleId" => "JDBC-PLAIN", "ruleIndex" => 0 }
"ruleId" => "JDBC-PLAIN", "ruleIndex" => 1 }
expected_vul2 = { "level" => "error",
"locations" => [{ "physicalLocation" => {
"artifactLocation" => { "uri" => "url.txt",
Expand All @@ -53,7 +53,7 @@
} }],
"message" => { "text" => "Leaked credential detected" },
"properties" => { "severity" => "high" },
"ruleId" => "JDBC-PLAIN", "ruleIndex" => 0 }
"ruleId" => "JDBC-PLAIN", "ruleIndex" => 1 }
expect(result.size).to eq(3)
[expected_vul0, expected_vul1, expected_vul2].each { |v| expect(result).to include(v) }
end
Expand Down
Loading