Commit 97257671 authored by Mehmet Emin INAC's avatar Mehmet Emin INAC

Do not parse Finding#raw_metadata to collect attributes

We can collect the attributes directly from the finding data instead of
serializing the data as string and then parsing the json string.
parent 7020dc6c
......@@ -8,9 +8,7 @@ module Security
# You can think this as the Message object in the pipeline design pattern
# which is passed between tasks.
class FindingMap
FINDING_ATTRIBUTES = %i[confidence metadata_version name raw_metadata report_type severity details].freeze
RAW_METADATA_ATTRIBUTES = %w[description message solution cve location].freeze
RAW_METADATA_PLACEHOLDER = { description: nil, message: nil, solution: nil, cve: nil, location: nil }.freeze
FINDING_ATTRIBUTES = %i[confidence metadata_version name raw_metadata report_type severity details description message cve solution].freeze
attr_reader :security_finding, :report_finding
attr_accessor :finding_id, :vulnerability_id, :new_record, :identifier_ids
......@@ -44,16 +42,16 @@ module Security
end
def to_hash
# This was already an existing problem so we've used it here as well.
# TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/342043
parsed_from_raw_metadata = Gitlab::Json.parse(report_finding.raw_metadata).slice(*RAW_METADATA_ATTRIBUTES).symbolize_keys
report_finding.to_hash
.slice(*FINDING_ATTRIBUTES)
.merge(RAW_METADATA_PLACEHOLDER)
.merge(parsed_from_raw_metadata)
.merge(primary_identifier_id: identifier_ids.first, location_fingerprint: report_finding.location.fingerprint, project_fingerprint: project_fingerprint)
.merge(uuid: uuid, scanner_id: scanner_id)
.merge!(
uuid: uuid,
scanner_id: scanner_id,
project_fingerprint: project_fingerprint,
primary_identifier_id: identifier_ids.first,
location: report_finding.location_data,
location_fingerprint: report_finding.location.fingerprint
)
end
end
end
......
......@@ -29,7 +29,7 @@ RSpec.describe Gitlab::Ci::Reports::Security::Finding do
location: location,
metadata_version: 'sast:1.0',
name: 'Cipher with no integrity',
raw_metadata: 'I am a stringified json object',
original_data: {},
report_type: :sast,
scanner: scanner,
scan: nil,
......@@ -71,7 +71,8 @@ RSpec.describe Gitlab::Ci::Reports::Security::Finding do
location: location,
metadata_version: 'sast:1.0',
name: 'Cipher with no integrity',
raw_metadata: 'I am a stringified json object',
raw_metadata: '{}',
original_data: {},
report_type: :sast,
scanner: scanner,
severity: :high,
......@@ -98,7 +99,7 @@ RSpec.describe Gitlab::Ci::Reports::Security::Finding do
end
end
%i[compare_key identifiers location metadata_version name raw_metadata report_type scanner uuid].each do |attribute|
%i[compare_key identifiers location metadata_version name original_data report_type scanner uuid].each do |attribute|
context "when attribute #{attribute} is missing" do
before do
params.delete(attribute)
......@@ -144,6 +145,10 @@ RSpec.describe Gitlab::Ci::Reports::Security::Finding do
severity: occurrence.severity,
uuid: occurrence.uuid,
details: occurrence.details,
cve: occurrence.compare_key,
description: occurrence.description,
message: occurrence.message,
solution: occurrence.solution,
signatures: []
})
end
......
......@@ -61,7 +61,7 @@ RSpec.describe Security::Ingestion::FindingMap do
description: 'The cipher does not provide data integrity update 1',
solution: 'GCM mode introduces an HMAC into the resulting encrypted data, providing integrity of the result.',
message: nil,
cve: nil,
cve: report_finding.cve,
location: {
"class" => "com.gitlab.security_products.tests.App",
"end_line" => 29,
......
......@@ -114,7 +114,7 @@ module Gitlab
flags: flags,
links: links,
remediations: remediations,
raw_metadata: data.to_json,
original_data: data,
metadata_version: report_version,
details: data['details'] || {},
signatures: signatures,
......
......@@ -17,7 +17,6 @@ module Gitlab
attr_reader :name
attr_reader :old_location
attr_reader :project_fingerprint
attr_reader :raw_metadata
attr_reader :report_type
attr_reader :scanner
attr_reader :scan
......@@ -28,10 +27,13 @@ module Gitlab
attr_reader :details
attr_reader :signatures
attr_reader :project_id
attr_reader :original_data
delegate :file_path, :start_line, :end_line, to: :location
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
alias_method :cve, :compare_key
def initialize(compare_key:, identifiers:, flags: [], links: [], remediations: [], location:, metadata_version:, name:, original_data:, 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
@confidence = confidence
@identifiers = identifiers
......@@ -40,7 +42,7 @@ module Gitlab
@location = location
@metadata_version = metadata_version
@name = name
@raw_metadata = raw_metadata
@original_data = original_data
@report_type = report_type
@scanner = scanner
@scan = scan
......@@ -74,6 +76,10 @@ module Gitlab
uuid
details
signatures
description
message
cve
solution
].each_with_object({}) do |key, hash|
hash[key] = public_send(key) # rubocop:disable GitlabSecurity/PublicSend
end
......@@ -145,6 +151,26 @@ module Gitlab
signatures.present?
end
def raw_metadata
@raw_metadata ||= original_data.to_json
end
def description
original_data['description']
end
def message
original_data['message']
end
def solution
original_data['solution']
end
def location_data
original_data['location']
end
private
def generate_project_fingerprint
......
......@@ -9,7 +9,7 @@ FactoryBot.define do
metadata_version { 'sast:1.0' }
name { 'Cipher with no integrity' }
report_type { :sast }
raw_metadata do
original_data do
{
description: "The cipher does not provide data integrity update 1",
solution: "GCM mode introduces an HMAC into the resulting encrypted data, providing integrity of the result.",
......@@ -26,7 +26,7 @@ FactoryBot.define do
url: "https://crypto.stackexchange.com/questions/31428/pbewithmd5anddes-cipher-does-not-check-for-integrity-first"
}
]
}.to_json
}.deep_stringify_keys
end
scanner factory: :ci_reports_security_scanner
severity { :high }
......
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