Commit a1b61286 authored by Saikat Sarkar's avatar Saikat Sarkar

Parse and persist vulnerability flags

parent 2c314d78
...@@ -47,7 +47,7 @@ module Security ...@@ -47,7 +47,7 @@ module Security
report_finding = report_finding_for(security_finding) report_finding = report_finding_for(security_finding)
return Vulnerabilities::Finding.new unless report_finding return Vulnerabilities::Finding.new unless report_finding
finding_data = report_finding.to_hash.except(:compare_key, :identifiers, :location, :scanner, :links, :signatures) finding_data = report_finding.to_hash.except(:compare_key, :identifiers, :location, :scanner, :links, :signatures, :flags)
identifiers = report_finding.identifiers.map do |identifier| identifiers = report_finding.identifiers.map do |identifier|
Vulnerabilities::Identifier.new(identifier.to_hash) Vulnerabilities::Identifier.new(identifier.to_hash)
end end
...@@ -63,7 +63,9 @@ module Security ...@@ -63,7 +63,9 @@ module Security
finding.scanner = security_finding.scanner finding.scanner = security_finding.scanner
if calculate_false_positive? if calculate_false_positive?
finding.vulnerability_flags = existing_vulnerability_flags.fetch(security_finding.uuid, []) finding.vulnerability_flags = report_finding.flags.map do |flag|
Vulnerabilities::Flag.new(flag)
end
end end
finding.identifiers = identifiers finding.identifiers = identifiers
...@@ -79,10 +81,6 @@ module Security ...@@ -79,10 +81,6 @@ module Security
existing_vulnerabilities.dig(security_finding.scan.scan_type, security_finding.project_fingerprint)&.first existing_vulnerabilities.dig(security_finding.scan.scan_type, security_finding.project_fingerprint)&.first
end end
def existing_vulnerability_flags
@existing_vulnerability_flags ||= project.vulnerability_flags_for(security_findings.map(&:uuid))
end
def calculate_false_positive? def calculate_false_positive?
::Feature.enabled?(:vulnerability_flags, project) && project.licensed_feature_available?(:sast_fp_reduction) ::Feature.enabled?(:vulnerability_flags, project) && project.licensed_feature_available?(:sast_fp_reduction)
end end
......
...@@ -33,8 +33,7 @@ module Security ...@@ -33,8 +33,7 @@ module Security
normalized_findings = normalize_report_findings( normalized_findings = normalize_report_findings(
report.findings, report.findings,
vulnerabilities_by_finding_fingerprint(report), vulnerabilities_by_finding_fingerprint(report))
existing_vulnerability_flags_for(report))
filtered_findings = filter(normalized_findings) filtered_findings = filter(normalized_findings)
...@@ -76,19 +75,15 @@ module Security ...@@ -76,19 +75,15 @@ module Security
.select(:vulnerability_id, :project_fingerprint) .select(:vulnerability_id, :project_fingerprint)
end end
def existing_vulnerability_flags_for(report)
pipeline.project.vulnerability_flags_for(report.findings.map(&:uuid))
end
# This finder is used for fetching vulnerabilities for any pipeline, if we used it to fetch # This finder is used for fetching vulnerabilities for any pipeline, if we used it to fetch
# vulnerabilities for a non-default-branch, the findings will be unpersisted, so we # vulnerabilities for a non-default-branch, the findings will be unpersisted, so we
# coerce the POROs into unpersisted AR records to give them a common object. # coerce the POROs into unpersisted AR records to give them a common object.
# See https://gitlab.com/gitlab-org/gitlab/issues/33588#note_291849433 for more context # See https://gitlab.com/gitlab-org/gitlab/issues/33588#note_291849433 for more context
# on why this happens. # on why this happens.
def normalize_report_findings(report_findings, vulnerabilities, vulnerability_flags) def normalize_report_findings(report_findings, vulnerabilities)
report_findings.map do |report_finding| report_findings.map do |report_finding|
finding_hash = report_finding.to_hash finding_hash = report_finding.to_hash
.except(:compare_key, :identifiers, :location, :scanner, :links, :signatures) .except(:compare_key, :identifiers, :location, :scanner, :links, :signatures, :flags)
finding = Vulnerabilities::Finding.new(finding_hash) finding = Vulnerabilities::Finding.new(finding_hash)
# assigning Vulnerabilities to Findings to enable the computed state # assigning Vulnerabilities to Findings to enable the computed state
...@@ -108,7 +103,9 @@ module Security ...@@ -108,7 +103,9 @@ module Security
end end
if calculate_false_positive? if calculate_false_positive?
finding.vulnerability_flags = vulnerability_flags.fetch(finding.uuid, []) finding.vulnerability_flags = report_finding.flags.map do |flag|
Vulnerabilities::Flag.new(flag)
end
end end
finding finding
......
...@@ -13,5 +13,10 @@ module Vulnerabilities ...@@ -13,5 +13,10 @@ module Vulnerabilities
enum flag_type: { enum flag_type: {
false_positive: 0 false_positive: 0
} }
def initialize(attributes)
attributes = attributes.to_hash if attributes.instance_of?(Gitlab::Ci::Reports::Security::Flag)
super(attributes)
end
end end
end end
...@@ -59,6 +59,10 @@ module Security ...@@ -59,6 +59,10 @@ module Security
update_vulnerabilities_identifiers update_vulnerabilities_identifiers
update_vulnerabilities_finding_identifiers update_vulnerabilities_finding_identifiers
if ::Feature.enabled?(:vulnerability_flags, project) && project.licensed_feature_available?(:sast_fp_reduction)
create_vulnerability_flags_info
end
vulnerability_ids vulnerability_ids
end end
...@@ -75,7 +79,7 @@ module Security ...@@ -75,7 +79,7 @@ module Security
return return
end end
vulnerability_params = finding.to_hash.except(:compare_key, :identifiers, :location, :scanner, :scan, :links, :signatures) vulnerability_params = finding.to_hash.except(:compare_key, :identifiers, :location, :scanner, :scan, :links, :signatures, :flags)
entity_params = Gitlab::Json.parse(vulnerability_params&.dig(:raw_metadata)).slice('description', 'message', 'solution', 'cve', 'location') entity_params = Gitlab::Json.parse(vulnerability_params&.dig(:raw_metadata)).slice('description', 'message', 'solution', 'cve', 'location')
# Vulnerabilities::Finding (`vulnerability_occurrences`) # Vulnerabilities::Finding (`vulnerability_occurrences`)
vulnerability_finding = vulnerability_findings_by_uuid[finding.uuid] || vulnerability_finding = vulnerability_findings_by_uuid[finding.uuid] ||
...@@ -254,6 +258,22 @@ module Security ...@@ -254,6 +258,22 @@ module Security
rescue ActiveRecord::RecordNotUnique rescue ActiveRecord::RecordNotUnique
end end
def create_vulnerability_flags_info
timestamps = { created_at: Time.current, updated_at: Time.current }
vulnerability_finding_to_finding_map.each_slice(BATCH_SIZE) do |vf_to_findings|
records = vf_to_findings.flat_map do |vulnerability_finding, finding|
finding.flags.map { |flag| timestamps.merge(**flag.to_hash, vulnerability_occurrence_id: vulnerability_finding.id) }
end
records.uniq!
Vulnerabilities::Flag.insert_all(records) if records.present?
end
rescue StandardError => e
Gitlab::ErrorTracking.track_exception(e, project_id: project.id, pipeline_id: pipeline.id)
end
def update_vulnerability_links_info def update_vulnerability_links_info
timestamps = { created_at: Time.current, updated_at: Time.current } timestamps = { created_at: Time.current, updated_at: Time.current }
......
...@@ -12,6 +12,16 @@ FactoryBot.define do ...@@ -12,6 +12,16 @@ FactoryBot.define do
end end
end end
trait :sast_with_vulnerability_flags do
file_type { :sast }
file_format { :raw }
after(:build) do |artifact, _|
artifact.file = fixture_file_upload(
Rails.root.join('ee/spec/fixtures/security_reports/master/gl-sast-report-with-vulnerability-flags.json'), 'application/json')
end
end
trait :dast do trait :dast do
file_format { :raw } file_format { :raw }
file_type { :dast } file_type { :dast }
......
...@@ -59,8 +59,6 @@ RSpec.describe Security::FindingsFinder do ...@@ -59,8 +59,6 @@ RSpec.describe Security::FindingsFinder do
deduplicated: true, deduplicated: true,
position: index, position: index,
scan: scan) scan: scan)
create(:vulnerabilities_finding, uuid: finding.uuid, project: pipeline.project)
end end
end end
...@@ -186,8 +184,9 @@ RSpec.describe Security::FindingsFinder do ...@@ -186,8 +184,9 @@ RSpec.describe Security::FindingsFinder do
context 'with some vulnerability flags present' do context 'with some vulnerability flags present' do
before do before do
create(:vulnerabilities_flag, finding: pipeline.project.vulnerability_findings.first) allow_next_instance_of(Gitlab::Ci::Reports::Security::Finding) do |finding|
create(:vulnerabilities_flag, finding: pipeline.project.vulnerability_findings.last) allow(finding).to receive(:flags).and_return([create(:ci_reports_security_flag)]) if finding.report_type == 'sast'
end
end end
it 'has some vulnerability_findings with vulnerability flag' do it 'has some vulnerability_findings with vulnerability flag' do
......
...@@ -140,8 +140,9 @@ RSpec.describe Security::PipelineVulnerabilitiesFinder do ...@@ -140,8 +140,9 @@ RSpec.describe Security::PipelineVulnerabilitiesFinder do
context "false-positive" do context "false-positive" do
before do before do
vulnerability_finding = create(:vulnerabilities_finding, uuid: sast_report_uuids.first, project: pipeline.project) allow_next_instance_of(Gitlab::Ci::Reports::Security::Finding) do |finding|
create(:vulnerabilities_flag, finding: vulnerability_finding) allow(finding).to receive(:flags).and_return([create(:ci_reports_security_flag)]) if finding.report_type == 'sast'
end
end end
it 'includes findings with false-positive' do it 'includes findings with false-positive' do
......
{
"version": "14.0.0",
"vulnerabilities": [
{
"category": "sast",
"name": "Predictable pseudorandom number generator",
"message": "Predictable pseudorandom number generator",
"cve": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy:47:PREDICTABLE_RANDOM",
"severity": "Medium",
"confidence": "Medium",
"scanner": {
"id": "find_sec_bugs",
"name": "Find Security Bugs"
},
"location": {
"file": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy",
"start_line": 47,
"end_line": 47,
"class": "com.gitlab.security_products.tests.App",
"method": "generateSecretToken2"
},
"flags": [
{
"type": "flagged-as-likely-false-positive",
"origin": "vet",
"description": "This vulnerability has been identified as a potential false positive by the VET post-analyzer"
},
{
"type": "flagged-as-likely-false-positive",
"origin": "post analyzer Y",
"description": "integer to sink"
}
],
"identifiers": [
{
"type": "find_sec_bugs_type",
"name": "Find Security Bugs-PREDICTABLE_RANDOM",
"value": "PREDICTABLE_RANDOM",
"url": "https://find-sec-bugs.github.io/bugs.htm#PREDICTABLE_RANDOM"
}
]
},
{
"category": "sast",
"name": "Predictable pseudorandom number generator",
"message": "Predictable pseudorandom number generator",
"cve": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy:41:PREDICTABLE_RANDOM",
"severity": "Low",
"confidence": "Low",
"scanner": {
"id": "find_sec_bugs",
"name": "Find Security Bugs"
},
"location": {
"file": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy",
"start_line": 41,
"end_line": 41,
"class": "com.gitlab.security_products.tests.App",
"method": "generateSecretToken1"
},
"identifiers": [
{
"type": "find_sec_bugs_type",
"name": "Find Security Bugs-PREDICTABLE_RANDOM",
"value": "PREDICTABLE_RANDOM",
"url": "https://find-sec-bugs.github.io/bugs.htm#PREDICTABLE_RANDOM"
}
]
},
{
"category": "sast",
"name": "ECB mode is insecure",
"message": "ECB mode is insecure",
"description": "The cipher uses ECB mode, which provides poor confidentiality for encrypted data",
"cve": "ea0f905fc76f2739d5f10a1fd1e37a10:ECB_MODE:java-maven/src/main/java/com/gitlab/security_products/tests/App.java:29",
"severity": "Medium",
"confidence": "High",
"scanner": {
"id": "find_sec_bugs",
"name": "Find Security Bugs"
},
"location": {
"file": "java-maven/src/main/java/com/gitlab/security_products/tests/App.java",
"start_line": 29,
"end_line": 29,
"class": "com.gitlab.security_products.tests.App",
"method": "insecureCypher"
},
"identifiers": [
{
"type": "find_sec_bugs_type",
"name": "Find Security Bugs-ECB_MODE",
"value": "ECB_MODE",
"url": "https://find-sec-bugs.github.io/bugs.htm#ECB_MODE"
},
{
"type": "cwe",
"name": "CWE-327",
"value": "327",
"url": "https://cwe.mitre.org/data/definitions/327.html"
}
]
},
{
"category": "sast",
"name": "Hard coded key",
"message": "Hard coded key",
"description": "Hard coded cryptographic key found",
"cve": "102ac67e0975ecec02a056008e0faad8:HARD_CODE_KEY:scala-sbt/src/main/scala/example/Main.scala:12",
"severity": "Medium",
"confidence": "High",
"scanner": {
"id": "find_sec_bugs",
"name": "Find Security Bugs"
},
"location": {
"file": "scala-sbt/src/main/scala/example/Main.scala",
"start_line": 12,
"end_line": 12,
"class": "example.Main$",
"method": "getBytes"
},
"identifiers": [
{
"type": "find_sec_bugs_type",
"name": "Find Security Bugs-HARD_CODE_KEY",
"value": "HARD_CODE_KEY",
"url": "https://find-sec-bugs.github.io/bugs.htm#HARD_CODE_KEY"
},
{
"type": "cwe",
"name": "CWE-321",
"value": "321",
"url": "https://cwe.mitre.org/data/definitions/321.html"
}
]
},
{
"category": "sast",
"name": "Cipher with no integrity",
"message": "Cipher with no integrity",
"cve": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy:29:CIPHER_INTEGRITY",
"severity": "Medium",
"confidence": "High",
"scanner": {
"id": "find_sec_bugs",
"name": "Find Security Bugs"
},
"location": {
"file": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy",
"start_line": 29,
"end_line": 29,
"class": "com.gitlab.security_products.tests.App",
"method": "insecureCypher"
},
"identifiers": [
{
"type": "find_sec_bugs_type",
"name": "Find Security Bugs-CIPHER_INTEGRITY",
"value": "CIPHER_INTEGRITY",
"url": "https://find-sec-bugs.github.io/bugs.htm#CIPHER_INTEGRITY"
}
],
"tracking": {
"type": "source",
"items": [
{
"file": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy",
"start_line": 29,
"end_line": 29,
"signatures": [
{
"algorithm": "hash",
"value": "HASHVALUE"
},
{
"algorithm": "scope_offset",
"value": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy:App[0]:insecureCypher[0]:2"
}
]
}
]
}
}
],
"remediations": [],
"scan": {
"scanner": {
"id": "find_sec_bugs",
"name": "Find Security Bugs",
"url": "https://spotbugs.github.io",
"vendor": {
"name": "GitLab"
},
"version": "4.0.2"
},
"type": "sast",
"status": "success",
"start_time": "placeholder-value",
"end_time": "placeholder-value"
}
}
...@@ -55,16 +55,15 @@ RSpec.describe GitlabSchema.types['PipelineSecurityReportFinding'] do ...@@ -55,16 +55,15 @@ RSpec.describe GitlabSchema.types['PipelineSecurityReportFinding'] do
context 'when the vulnerability has a false-positive flag' do context 'when the vulnerability has a false-positive flag' do
before do before do
security_finding = pipeline.security_reports.reports['sast'].findings.first allow_next_instance_of(Gitlab::Ci::Reports::Security::Finding) do |finding|
vulnerability_finding = create(:vulnerabilities_finding, uuid: security_finding.uuid, pipelines: [pipeline], project: pipeline.project) allow(finding).to receive(:flags).and_return([create(:ci_reports_security_flag)]) if finding.report_type == 'sast'
create(:vulnerabilities_flag, finding: vulnerability_finding) end
end end
it 'returns false-positive value' do it 'returns false-positive value' do
vulnerabilities = subject.dig('data', 'project', 'pipeline', 'securityReportFindings', 'nodes') vulnerabilities = subject.dig('data', 'project', 'pipeline', 'securityReportFindings', 'nodes')
expect(vulnerabilities.first['falsePositive']).to be(true) expect(vulnerabilities.first['falsePositive']).to be(true)
expect(vulnerabilities.last['falsePositive']).to be(false)
end end
end end
......
...@@ -15,12 +15,16 @@ RSpec.describe Gitlab::Ci::Reports::Security::Finding do ...@@ -15,12 +15,16 @@ RSpec.describe Gitlab::Ci::Reports::Security::Finding do
let_it_be(:location) { build(:ci_reports_security_locations_sast) } let_it_be(:location) { build(:ci_reports_security_locations_sast) }
let_it_be(:remediation) { build(:ci_reports_security_remediation) } let_it_be(:remediation) { build(:ci_reports_security_remediation) }
let(:flag_1) { build(:ci_reports_security_flag) }
let(:flag_2) { build(:ci_reports_security_flag) }
let(:params) do let(:params) do
{ {
compare_key: 'this_is_supposed_to_be_a_unique_value', compare_key: 'this_is_supposed_to_be_a_unique_value',
confidence: :medium, confidence: :medium,
identifiers: [primary_identifier, other_identifier], identifiers: [primary_identifier, other_identifier],
links: [link], links: [link],
flags: [flag_1, flag_2],
remediations: [remediation], remediations: [remediation],
location: location, location: location,
metadata_version: 'sast:1.0', metadata_version: 'sast:1.0',
...@@ -62,6 +66,7 @@ RSpec.describe Gitlab::Ci::Reports::Security::Finding do ...@@ -62,6 +66,7 @@ RSpec.describe Gitlab::Ci::Reports::Security::Finding do
project_fingerprint: '9a73f32d58d87d94e3dc61c4c1a94803f6014258', project_fingerprint: '9a73f32d58d87d94e3dc61c4c1a94803f6014258',
identifiers: [primary_identifier, other_identifier], identifiers: [primary_identifier, other_identifier],
links: [link], links: [link],
flags: [flag_1, flag_2],
remediations: [remediation], remediations: [remediation],
location: location, location: location,
metadata_version: 'sast:1.0', metadata_version: 'sast:1.0',
...@@ -127,6 +132,7 @@ RSpec.describe Gitlab::Ci::Reports::Security::Finding do ...@@ -127,6 +132,7 @@ RSpec.describe Gitlab::Ci::Reports::Security::Finding do
confidence: occurrence.confidence, confidence: occurrence.confidence,
identifiers: occurrence.identifiers, identifiers: occurrence.identifiers,
links: occurrence.links, links: occurrence.links,
flags: occurrence.flags,
location: occurrence.location, location: occurrence.location,
metadata_version: occurrence.metadata_version, metadata_version: occurrence.metadata_version,
name: occurrence.name, name: occurrence.name,
......
...@@ -16,4 +16,11 @@ RSpec.describe Vulnerabilities::Flag do ...@@ -16,4 +16,11 @@ RSpec.describe Vulnerabilities::Flag do
it { is_expected.to validate_uniqueness_of(:flag_type).scoped_to(:vulnerability_occurrence_id, :origin).ignoring_case_sensitivity } it { is_expected.to validate_uniqueness_of(:flag_type).scoped_to(:vulnerability_occurrence_id, :origin).ignoring_case_sensitivity }
it { is_expected.to define_enum_for(:flag_type).with_values(false_positive: 0) } it { is_expected.to define_enum_for(:flag_type).with_values(false_positive: 0) }
end end
describe '#initialize' do
it 'creates a valid flag with flag_type attribute' do
flag = described_class.new(flag_type: Vulnerabilities::Flag.flag_types[:false_positive], origin: 'post analyzer X', description: 'static string to sink', finding: build(:vulnerabilities_finding))
expect(flag).to be_valid
end
end
end end
...@@ -69,8 +69,7 @@ RSpec.describe API::VulnerabilityFindings do ...@@ -69,8 +69,7 @@ RSpec.describe API::VulnerabilityFindings do
# Threshold is required for the extra query performed in Security::PipelineVulnerabilitiesFinder to load # Threshold is required for the extra query performed in Security::PipelineVulnerabilitiesFinder to load
# the Vulnerabilities providing computed states for the associated Vulnerability::Findings # the Vulnerabilities providing computed states for the associated Vulnerability::Findings
# and all associated vulnerability_flags expect { get api(project_vulnerability_findings_path, user) }.not_to exceed_query_limit(control_count).with_threshold(1)
expect { get api(project_vulnerability_findings_path, user) }.not_to exceed_query_limit(control_count).with_threshold(3)
end end
describe 'using different finders' do describe 'using different finders' do
......
...@@ -26,6 +26,7 @@ RSpec.describe Security::StoreReportService, '#execute' do ...@@ -26,6 +26,7 @@ RSpec.describe Security::StoreReportService, '#execute' do
dependency_scanning: true, dependency_scanning: true,
container_scanning: true, container_scanning: true,
security_dashboard: true, security_dashboard: true,
sast_fp_reduction: true,
vulnerability_finding_signatures: vulnerability_finding_signatures vulnerability_finding_signatures: vulnerability_finding_signatures
) )
allow(Security::AutoFixWorker).to receive(:perform_async) allow(Security::AutoFixWorker).to receive(:perform_async)
...@@ -43,11 +44,12 @@ RSpec.describe Security::StoreReportService, '#execute' do ...@@ -43,11 +44,12 @@ RSpec.describe Security::StoreReportService, '#execute' do
end end
context 'for different security reports' do context 'for different security reports' do
where(:case_name, :trait, :scanners, :identifiers, :findings, :finding_identifiers, :finding_pipelines, :remediations, :signatures, :finding_links) do where(:case_name, :trait, :scanners, :identifiers, :findings, :finding_identifiers, :finding_pipelines, :remediations, :signatures, :finding_links, :finding_flags) do
'with SAST report' | :sast | 1 | 6 | 5 | 7 | 5 | 0 | 2 | 0 'with SAST report' | :sast | 1 | 6 | 5 | 7 | 5 | 0 | 2 | 0 | 0
'with exceeding identifiers' | :with_exceeding_identifiers | 1 | 20 | 1 | 20 | 1 | 0 | 0 | 0 'with exceeding identifiers' | :with_exceeding_identifiers | 1 | 20 | 1 | 20 | 1 | 0 | 0 | 0 | 0
'with Dependency Scanning report' | :dependency_scanning_remediation | 1 | 3 | 2 | 3 | 2 | 1 | 0 | 6 'with Dependency Scanning report' | :dependency_scanning_remediation | 1 | 3 | 2 | 3 | 2 | 1 | 0 | 6 | 0
'with Container Scanning report' | :container_scanning | 1 | 8 | 8 | 8 | 8 | 0 | 0 | 8 'with Container Scanning report' | :container_scanning | 1 | 8 | 8 | 8 | 8 | 0 | 0 | 8 | 0
'with vulnerability flags' | :sast_with_vulnerability_flags | 1 | 6 | 5 | 7 | 5 | 0 | 2 | 0 | 2
end end
with_them do with_them do
...@@ -63,6 +65,22 @@ RSpec.describe Security::StoreReportService, '#execute' do ...@@ -63,6 +65,22 @@ RSpec.describe Security::StoreReportService, '#execute' do
expect { subject }.to change { Vulnerabilities::Finding.count }.by(findings) expect { subject }.to change { Vulnerabilities::Finding.count }.by(findings)
end end
context 'vulnerability flags' do
it 'inserts all finding flags' do
expect { subject }.to change(Vulnerabilities::Flag, :count).by(finding_flags)
end
context 'with vulnerability_flags disabled' do
before do
stub_feature_flags(vulnerability_flags: false)
end
it 'does not insert any vulnerability flag' do
expect { subject }.not_to change(Vulnerabilities::Flag, :count)
end
end
end
it 'inserts all finding links' do it 'inserts all finding links' do
expect { subject }.to change { Vulnerabilities::FindingLink.count }.by(finding_links) expect { subject }.to change { Vulnerabilities::FindingLink.count }.by(finding_links)
end end
......
...@@ -86,6 +86,7 @@ module Gitlab ...@@ -86,6 +86,7 @@ module Gitlab
def create_finding(data, remediations = []) def create_finding(data, remediations = [])
identifiers = create_identifiers(data['identifiers']) identifiers = create_identifiers(data['identifiers'])
flags = create_flags(data['flags'])
links = create_links(data['links']) links = create_links(data['links'])
location = create_location(data['location'] || {}) location = create_location(data['location'] || {})
signatures = create_signatures(tracking_data(data)) signatures = create_signatures(tracking_data(data))
...@@ -111,6 +112,7 @@ module Gitlab ...@@ -111,6 +112,7 @@ module Gitlab
scanner: create_scanner(data['scanner']), scanner: create_scanner(data['scanner']),
scan: report&.scan, scan: report&.scan,
identifiers: identifiers, identifiers: identifiers,
flags: flags,
links: links, links: links,
remediations: remediations, remediations: remediations,
raw_metadata: data.to_json, raw_metadata: data.to_json,
...@@ -205,6 +207,18 @@ module Gitlab ...@@ -205,6 +207,18 @@ module Gitlab
url: identifier['url'])) url: identifier['url']))
end end
def create_flags(flags)
return [] unless flags.is_a?(Array)
flags.map { |flag| create_flag(flag) }.compact
end
def create_flag(flag)
return unless flag.is_a?(Hash)
::Gitlab::Ci::Reports::Security::Flag.new(type: flag['type'], origin: flag['origin'], description: flag['description'])
end
def create_links(links) def create_links(links)
return [] unless links.is_a?(Array) return [] unless links.is_a?(Array)
......
...@@ -10,6 +10,7 @@ module Gitlab ...@@ -10,6 +10,7 @@ module Gitlab
attr_reader :compare_key attr_reader :compare_key
attr_reader :confidence attr_reader :confidence
attr_reader :identifiers attr_reader :identifiers
attr_reader :flags
attr_reader :links attr_reader :links
attr_reader :location attr_reader :location
attr_reader :metadata_version attr_reader :metadata_version
...@@ -30,10 +31,11 @@ module Gitlab ...@@ -30,10 +31,11 @@ module Gitlab
delegate :file_path, :start_line, :end_line, to: :location delegate :file_path, :start_line, :end_line, to: :location
def initialize(compare_key:, identifiers:, links: [], remediations: [], location:, metadata_version:, name:, raw_metadata:, report_type:, scanner:, scan:, uuid:, confidence: nil, severity: nil, details: {}, signatures: [], project_id: nil, vulnerability_finding_signatures_enabled: false) # rubocop:disable Metrics/ParameterLists def initialize(compare_key:, identifiers:, flags: [], links: [], remediations: [], location:, metadata_version:, name:, raw_metadata:, report_type:, scanner:, scan:, uuid:, confidence: nil, severity: nil, details: {}, signatures: [], project_id: nil, vulnerability_finding_signatures_enabled: false) # rubocop:disable Metrics/ParameterLists
@compare_key = compare_key @compare_key = compare_key
@confidence = confidence @confidence = confidence
@identifiers = identifiers @identifiers = identifiers
@flags = flags
@links = links @links = links
@location = location @location = location
@metadata_version = metadata_version @metadata_version = metadata_version
...@@ -58,6 +60,7 @@ module Gitlab ...@@ -58,6 +60,7 @@ module Gitlab
compare_key compare_key
confidence confidence
identifiers identifiers
flags
links links
location location
metadata_version metadata_version
......
# frozen_string_literal: true
module Gitlab
module Ci
module Reports
module Security
class Flag
attr_reader :type, :origin, :description
MAP = { 'flagged-as-likely-false-positive' => :false_positive }.freeze
DEFAULT_FLAG_TYPE = :false_positive
def flag_type
MAP.fetch(type, DEFAULT_FLAG_TYPE)
end
def initialize(type: nil, origin: nil, description: nil)
@type = type
@origin = origin
@description = description
end
def to_hash
{
flag_type: flag_type,
origin: origin,
description: description
}.compact
end
end
end
end
end
end
# frozen_string_literal: true
FactoryBot.define do
factory :ci_reports_security_flag, class: '::Gitlab::Ci::Reports::Security::Flag' do
type { 'flagged-as-likely-false-positive' }
origin { 'post analyzer X' }
description { 'static string to sink' }
skip_create
initialize_with do
::Gitlab::Ci::Reports::Security::Flag.new(**attributes)
end
end
end
...@@ -13,11 +13,18 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Common do ...@@ -13,11 +13,18 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Common do
# The path 'yarn.lock' was initially used by DependencyScanning, it is okay for SAST locations to use it, but this could be made better # The path 'yarn.lock' was initially used by DependencyScanning, it is okay for SAST locations to use it, but this could be made better
let(:location) { ::Gitlab::Ci::Reports::Security::Locations::Sast.new(file_path: 'yarn.lock', start_line: 1, end_line: 1) } let(:location) { ::Gitlab::Ci::Reports::Security::Locations::Sast.new(file_path: 'yarn.lock', start_line: 1, end_line: 1) }
let(:tracking_data) { nil } let(:tracking_data) { nil }
let(:vulnerability_flags_data) do
[
::Gitlab::Ci::Reports::Security::Flag.new(type: 'flagged-as-likely-false-positive', origin: 'post analyzer X', description: 'static string to sink'),
::Gitlab::Ci::Reports::Security::Flag.new(type: 'flagged-as-likely-false-positive', origin: 'post analyzer Y', description: 'integer to sink')
]
end
before do before do
allow_next_instance_of(described_class) do |parser| allow_next_instance_of(described_class) do |parser|
allow(parser).to receive(:create_location).and_return(location) allow(parser).to receive(:create_location).and_return(location)
allow(parser).to receive(:tracking_data).and_return(tracking_data) allow(parser).to receive(:tracking_data).and_return(tracking_data)
allow(parser).to receive(:create_flags).and_return(vulnerability_flags_data)
end end
artifact.each_blob { |blob| described_class.parse!(blob, report, vulnerability_finding_signatures_enabled) } artifact.each_blob { |blob| described_class.parse!(blob, report, vulnerability_finding_signatures_enabled) }
...@@ -231,6 +238,17 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Common do ...@@ -231,6 +238,17 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Common do
end end
end end
describe 'parsing flags' do
it 'returns flags object for each finding' do
flags = report.findings.first.flags
expect(flags).to contain_exactly(
have_attributes(type: 'flagged-as-likely-false-positive', origin: 'post analyzer X', description: 'static string to sink'),
have_attributes(type: 'flagged-as-likely-false-positive', origin: 'post analyzer Y', description: 'integer to sink')
)
end
end
describe 'parsing links' do describe 'parsing links' do
it 'returns links object for each finding', :aggregate_failures do it 'returns links object for each finding', :aggregate_failures do
links = report.findings.flat_map(&:links) links = report.findings.flat_map(&:links)
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::Ci::Reports::Security::Flag do
subject(:security_flag) { described_class.new(type: 'flagged-as-likely-false-positive', origin: 'post analyzer X', description: 'static string to sink') }
describe '#initialize' do
context 'when all params are given' do
it 'initializes an instance' do
expect { subject }.not_to raise_error
expect(subject).to have_attributes(
type: 'flagged-as-likely-false-positive',
origin: 'post analyzer X',
description: 'static string to sink'
)
end
end
describe '#to_hash' do
it 'returns expected hash' do
expect(security_flag.to_hash).to eq(
{
flag_type: :false_positive,
origin: 'post analyzer X',
description: 'static string to sink'
}
)
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment