From 12ab71ec0d9291cb00c49af4dc2f63ea19c52bc7 Mon Sep 17 00:00:00 2001
From: Zamir Martins <zfilho@gitlab.com>
Date: Tue, 8 Nov 2022 03:06:44 +0000
Subject: [PATCH] Add notes summary into csv export
for vulnerabilities.
EE: true
Changelog: changed
---
doc/api/vulnerability_exports.md | 14 +++++-----
.../vulnerability_report/index.md | 1 +
ee/app/models/ee/note.rb | 4 +++
ee/app/models/ee/vulnerability.rb | 26 ++++++++++++++++++-
.../vulnerability_exports/export_service.rb | 2 +-
.../exporters/csv_service.rb | 3 ++-
ee/spec/models/ee/vulnerability_spec.rb | 19 ++++++++++++++
ee/spec/models/note_spec.rb | 21 +++++++++++++++
.../export_service_spec.rb | 2 +-
.../exporters/csv_service_spec.rb | 5 ++++
.../helpers/vulnerability_exports_helpers.rb | 2 +-
11 files changed, 87 insertions(+), 12 deletions(-)
diff --git a/doc/api/vulnerability_exports.md b/doc/api/vulnerability_exports.md
index bdc5d2c29e741..8c166c2ba2a3e 100644
--- a/doc/api/vulnerability_exports.md
+++ b/doc/api/vulnerability_exports.md
@@ -198,11 +198,11 @@ The response is `404 Not Found` if the vulnerability export is not finished yet
Example response:
```csv
-Group Name,Project Name,Tool,Scanner Name,Status,Vulnerability,Details,Additional Info,Severity,CVE,CWE,Other Identifiers,Detected At,Location,Activity,
-Gitlab.org,Defend,container_scanning,Trivy,detected,CVE-2019-14697 in musl-utils-1.1.20-r4,"musl libc through 1.1.23 has an x87 floating-point stack adjustment imbalance, related to the math/i386/ directory. In some cases, use of this library could introduce out-of-bounds writes that are not present in an application's source code.",CVE-2019-14697 in musl-utils-1.1.20-r4,critical,CVE-2019-14697,,"",2022-10-07 13:34:41 UTC,"{""image""=>""python:3.4-alpine"", ""dependency""=>{""package""=>{""name""=>""musl-utils""}, ""version""=>""1.1.20-r4""}, ""operating_system""=>""alpine 3.9.2""}",true,
-Gitlab.org,Defend,container_scanning,Trivy,detected,CVE-2019-19242 in sqlite-libs-3.26.0-r3,"SQLite 3.30.1 mishandles pExpr->y.pTab, as demonstrated by the TK_COLUMN case in sqlite3ExprCodeTarget in expr.c.",CVE-2019-19242 in sqlite-libs-3.26.0-r3,medium,CVE-2019-19242,,"",2022-10-07 13:34:41 UTC,"{""image""=>""python:3.4-alpine"", ""dependency""=>{""package""=>{""name""=>""sqlite-libs""}, ""version""=>""3.26.0-r3""}, ""operating_system""=>""alpine 3.9.2""}",true,
-Gitlab.org,Defend,container_scanning,Trivy,detected,CVE-2020-28928 in musl-1.1.20-r4,"In musl libc through 1.2.1, wcsnrtombs mishandles particular combinations of destination buffer size and source character limit, as demonstrated by an invalid write access (buffer overflow).",CVE-2020-28928 in musl-1.1.20-r4,medium,CVE-2020-28928,,"",2022-10-07 13:34:41 UTC,"{""image""=>""python:3.4-alpine"", ""dependency""=>{""package""=>{""name""=>""musl""}, ""version""=>""1.1.20-r4""}, ""operating_system""=>""alpine 3.9.2""}",true,
-Gitlab.org,Defend,dependency_scanning,Gemnasium,detected,Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection') in rack,Carefully crafted requests can cause shell escape sequences to be written to the terminal via Rack's Lint middleware and CommonLogger middleware. These escape sequences can be leveraged to possibly execute commands in the victim's terminal.,Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection') in rack,unknown,Gemfile.lock:rack:gemnasium:60b5a27f-4e4d-4ab4-8ae7-74b4b212e177,,Gemnasium-60b5a27f-4e4d-4ab4-8ae7-74b4b212e177; GHSA-wq4h-7r42-5hrr,2022-10-14 13:16:00 UTC,"{""file""=>""Gemfile.lock"", ""dependency""=>{""package""=>{""name""=>""rack""}, ""version""=>""2.2.3""}}",false,
-Gitlab.org,Defend,dependency_scanning,Gemnasium,detected,Denial of Service Vulnerability in Rack Multipart Parsing in rack,"Carefully crafted multipart POST requests can cause Rack's multipart parser to take much longer than expected, leading to a possible denial of service vulnerability. Impacted code will use Rack's multipart parser to parse multipart posts.",Denial of Service Vulnerability in Rack Multipart Parsing in rack,unknown,Gemfile.lock:rack:gemnasium:20daa17a-47b5-4f79-80c2-cd8f2db9805c,,Gemnasium-20daa17a-47b5-4f79-80c2-cd8f2db9805c; GHSA-hxqx-xwvh-44m2,2022-10-14 13:16:00 UTC,"{""file""=>""Gemfile.lock"", ""dependency""=>{""package""=>{""name""=>""rack""}, ""version""=>""2.2.3""}}",false,
-Gitlab.org,Defend,sast,Brakeman,detected,Possible SQL injection,,Possible SQL injection,medium,e52f23a259cd489168b4313317ac94a3f13bffde57b9635171c1a44a9f329e9a,,"""Brakeman Warning Code 0""",2022-10-13 15:16:36 UTC,"{""file""=>""main.rb"", ""class""=>""User"", ""method""=>""index"", ""start_line""=>3}",false
+Group Name,Project Name,Tool,Scanner Name,Status,Vulnerability,Details,Additional Info,Severity,CVE,CWE,Other Identifiers,Detected At,Location,Activity,Comments,
+Gitlab.org,Defend,container_scanning,Trivy,resolved,CVE-2019-14697 in musl-utils-1.1.20-r4,"musl libc through 1.1.23 has an x87 floating-point stack adjustment imbalance, related to the math/i386/ directory. In some cases, use of this library could introduce out-of-bounds writes that are not present in an application's source code.",CVE-2019-14697 in musl-utils-1.1.20-r4,critical,CVE-2019-14697,,"",2022-10-07 13:34:41 UTC,"{""image""=>""python:3.4-alpine"", ""dependency""=>{""package""=>{""name""=>""musl-utils""}, ""version""=>""1.1.20-r4""}, ""operating_system""=>""alpine 3.9.2""}",true,"2022-10-07 13:41:08 UTC|root|resolved|changed vulnerability status to resolved",
+Gitlab.org,Defend,container_scanning,Trivy,detected,CVE-2019-19242 in sqlite-libs-3.26.0-r3,"SQLite 3.30.1 mishandles pExpr->y.pTab, as demonstrated by the TK_COLUMN case in sqlite3ExprCodeTarget in expr.c.",CVE-2019-19242 in sqlite-libs-3.26.0-r3,medium,CVE-2019-19242,,"",2022-10-07 13:34:41 UTC,"{""image""=>""python:3.4-alpine"", ""dependency""=>{""package""=>{""name""=>""sqlite-libs""}, ""version""=>""3.26.0-r3""}, ""operating_system""=>""alpine 3.9.2""}",true,"",
+Gitlab.org,Defend,container_scanning,Trivy,detected,CVE-2020-28928 in musl-1.1.20-r4,"In musl libc through 1.2.1, wcsnrtombs mishandles particular combinations of destination buffer size and source character limit, as demonstrated by an invalid write access (buffer overflow).",CVE-2020-28928 in musl-1.1.20-r4,medium,CVE-2020-28928,,"",2022-10-07 13:34:41 UTC,"{""image""=>""python:3.4-alpine"", ""dependency""=>{""package""=>{""name""=>""musl""}, ""version""=>""1.1.20-r4""}, ""operating_system""=>""alpine 3.9.2""}",true,"",
+Gitlab.org,Defend,dependency_scanning,Gemnasium,detected,Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection') in rack,Carefully crafted requests can cause shell escape sequences to be written to the terminal via Rack's Lint middleware and CommonLogger middleware. These escape sequences can be leveraged to possibly execute commands in the victim's terminal.,Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection') in rack,unknown,Gemfile.lock:rack:gemnasium:60b5a27f-4e4d-4ab4-8ae7-74b4b212e177,,Gemnasium-60b5a27f-4e4d-4ab4-8ae7-74b4b212e177; GHSA-wq4h-7r42-5hrr,2022-10-14 13:16:00 UTC,"{""file""=>""Gemfile.lock"", ""dependency""=>{""package""=>{""name""=>""rack""}, ""version""=>""2.2.3""}}",false,"",
+Gitlab.org,Defend,dependency_scanning,Gemnasium,detected,Denial of Service Vulnerability in Rack Multipart Parsing in rack,"Carefully crafted multipart POST requests can cause Rack's multipart parser to take much longer than expected, leading to a possible denial of service vulnerability. Impacted code will use Rack's multipart parser to parse multipart posts.",Denial of Service Vulnerability in Rack Multipart Parsing in rack,unknown,Gemfile.lock:rack:gemnasium:20daa17a-47b5-4f79-80c2-cd8f2db9805c,,Gemnasium-20daa17a-47b5-4f79-80c2-cd8f2db9805c; GHSA-hxqx-xwvh-44m2,2022-10-14 13:16:00 UTC,"{""file""=>""Gemfile.lock"", ""dependency""=>{""package""=>{""name""=>""rack""}, ""version""=>""2.2.3""}}",false,"",
+Gitlab.org,Defend,sast,Brakeman,detected,Possible SQL injection,,Possible SQL injection,medium,e52f23a259cd489168b4313317ac94a3f13bffde57b9635171c1a44a9f329e9a,,"""Brakeman Warning Code 0""",2022-10-13 15:16:36 UTC,"{""file""=>""main.rb"", ""class""=>""User"", ""method""=>""index"", ""start_line""=>3}",false,""
```
diff --git a/doc/user/application_security/vulnerability_report/index.md b/doc/user/application_security/vulnerability_report/index.md
index 159445406f031..2b78dde4f6394 100644
--- a/doc/user/application_security/vulnerability_report/index.md
+++ b/doc/user/application_security/vulnerability_report/index.md
@@ -219,6 +219,7 @@ Fields included are:
- Detected At
- Location
- Activity
+- Comments
NOTE:
Full details are available through our
diff --git a/ee/app/models/ee/note.rb b/ee/app/models/ee/note.rb
index b9c628c1dc6b0..e495dd633db0e 100644
--- a/ee/app/models/ee/note.rb
+++ b/ee/app/models/ee/note.rb
@@ -91,6 +91,10 @@ def usage_ping_track_updated_epic_note(user)
)
end
+ def updated_by_or_author
+ last_edited_by || author
+ end
+
private
def system_note_for_epic?
diff --git a/ee/app/models/ee/vulnerability.rb b/ee/app/models/ee/vulnerability.rb
index c95600e5ef4af..18df63cbd86ed 100644
--- a/ee/app/models/ee/vulnerability.rb
+++ b/ee/app/models/ee/vulnerability.rb
@@ -21,6 +21,8 @@ module Vulnerability
MAX_DAYS_OF_HISTORY = 10
ACTIVE_STATES = %w(detected confirmed).freeze
PASSIVE_STATES = %w(dismissed resolved).freeze
+ SUMMARY_DELIMITER = '|'
+ REGEX_CAPTURING_STATUS = /\A[\w+\s]+to\s(?<status>\w+)/.freeze
cache_markdown_field :title, pipeline: :single_line
cache_markdown_field :description, issuable_reference_expansion_enabled: true
@@ -80,7 +82,7 @@ def with_vulnerability_links
scope :with_findings_excluding_uuid, -> (uuid) { joins(:findings).merge(Vulnerabilities::Finding.excluding_uuids(uuid)) }
scope :with_findings_scanner_and_identifiers, -> { includes(findings: [:scanner, :identifiers, finding_identifiers: :identifier]) }
scope :with_created_issue_links_and_issues, -> { includes(created_issue_links: :issue) }
-
+ scope :with_findings_scanner_identifiers_and_notes, -> { with_findings_scanner_and_identifiers.includes(:notes) }
scope :visible_to_user_and_access_level, -> (user, access_level) { where(project_id: ::Project.visible_to_user_and_access_level(user, access_level)) }
scope :for_projects, -> (project_ids) { where(project_id: project_ids) }
scope :with_report_types, -> (report_types) { where(report_type: report_types) }
@@ -195,8 +197,30 @@ def execute_hooks
project.execute_integrations(integration_data, :vulnerability_hooks)
end
+ def notes_summary
+ @notes_summary ||= discussions.map do |discussion|
+ status = extracted_status(discussion.notes.first.note)
+ discussion.notes.map do |note|
+ note_as_string(note.last_edited_at.utc, note.updated_by_or_author.username, status, note.note)
+ end
+ end.join('; ')
+ end
+
private
+ def note_as_string(time, username, status, text)
+ CSV.generate(col_sep: SUMMARY_DELIMITER) do |csv|
+ csv << [time, username, status, text]
+ end
+ end
+
+ def extracted_status(note)
+ match_data = note.match(REGEX_CAPTURING_STATUS)
+ return '' unless match_data
+
+ match_data[:status]
+ end
+
def integration_data
@integration_data ||= ::Gitlab::DataBuilder::Vulnerability.build(self)
end
diff --git a/ee/app/services/vulnerability_exports/export_service.rb b/ee/app/services/vulnerability_exports/export_service.rb
index b3b9046badc1e..2dee21841ba06 100644
--- a/ee/app/services/vulnerability_exports/export_service.rb
+++ b/ee/app/services/vulnerability_exports/export_service.rb
@@ -55,7 +55,7 @@ def exporter
end
def vulnerabilities
- Security::VulnerabilitiesFinder.new(exportable).execute.with_findings_scanner_and_identifiers
+ Security::VulnerabilitiesFinder.new(exportable).execute.with_findings_scanner_identifiers_and_notes
end
def schedule_export_deletion
diff --git a/ee/app/services/vulnerability_exports/exporters/csv_service.rb b/ee/app/services/vulnerability_exports/exporters/csv_service.rb
index 4d51732fac18d..7b00961336c32 100644
--- a/ee/app/services/vulnerability_exports/exporters/csv_service.rb
+++ b/ee/app/services/vulnerability_exports/exporters/csv_service.rb
@@ -20,7 +20,8 @@ class CsvService
'Other Identifiers' => IDENTIFIER_FORMATTER,
'Detected At' => 'created_at',
'Location' => 'location',
- 'Activity' => 'resolved_on_default_branch'
+ 'Activity' => 'resolved_on_default_branch',
+ 'Comments' => 'notes_summary'
}.freeze
attr_reader :vulnerabilities
diff --git a/ee/spec/models/ee/vulnerability_spec.rb b/ee/spec/models/ee/vulnerability_spec.rb
index 9622630185ed7..62d242d61b510 100644
--- a/ee/spec/models/ee/vulnerability_spec.rb
+++ b/ee/spec/models/ee/vulnerability_spec.rb
@@ -867,4 +867,23 @@
expect(subject).not_to include(not_present)
end
end
+
+ describe '#notes_summary' do
+ let(:vulnerability) { create(:vulnerability, project: project) }
+ let!(:note) { create(:note, project: project, noteable: vulnerability, noteable_type: 'Vulnerability', note: 'changed vulnerabilities from detected to confirmed', author: user) }
+
+ subject(:notes_summary) { vulnerability.notes_summary }
+
+ it 'returns summarized version of notes' do
+ expect(notes_summary).to eq("#{note.last_edited_at}|#{user.username}|confirmed|changed vulnerabilities from detected to confirmed\n")
+ end
+
+ context 'with multiple notes' do
+ let!(:additional_note) { create(:note, project: project, noteable: vulnerability, noteable_type: 'Vulnerability', note: 'additional comment by the user', author: user, discussion_id: note.discussion_id) }
+
+ it 'returns summarized version of notes' do
+ expect(notes_summary).to eq("#{note.last_edited_at}|#{user.username}|confirmed|changed vulnerabilities from detected to confirmed\n; #{additional_note.last_edited_at}|#{user.username}|confirmed|additional comment by the user\n")
+ end
+ end
+ end
end
diff --git a/ee/spec/models/note_spec.rb b/ee/spec/models/note_spec.rb
index cc193d50e2688..391d473a38c60 100644
--- a/ee/spec/models/note_spec.rb
+++ b/ee/spec/models/note_spec.rb
@@ -155,4 +155,25 @@
end
end
end
+
+ describe '#updated_by_or_author' do
+ subject(:updated_by_or_author) { note.updated_by_or_author }
+
+ context 'when updated_by is nil' do
+ let(:note) { create(:note, updated_by: nil) }
+
+ it 'returns the author' do
+ expect(updated_by_or_author).to be(note.author)
+ end
+ end
+
+ context 'when updated_by is present' do
+ let(:user) { create(:user) }
+ let(:note) { create(:note, updated_by: user) }
+
+ it 'returns the last user who updated the note' do
+ expect(updated_by_or_author).to be(user)
+ end
+ end
+ end
end
diff --git a/ee/spec/services/vulnerability_exports/export_service_spec.rb b/ee/spec/services/vulnerability_exports/export_service_spec.rb
index 7b6030a6b17e9..688061f911fc8 100644
--- a/ee/spec/services/vulnerability_exports/export_service_spec.rb
+++ b/ee/spec/services/vulnerability_exports/export_service_spec.rb
@@ -100,7 +100,7 @@
context 'when the export format is csv' do
let(:vulnerabilities) { Vulnerability.none }
- let(:mock_relation) { double(:relation, with_findings_scanner_and_identifiers: vulnerabilities) }
+ let(:mock_relation) { double(:relation, with_findings_scanner_identifiers_and_notes: vulnerabilities) }
let(:mock_vulnerability_finder_service_object) { instance_double(Security::VulnerabilitiesFinder, execute: mock_relation) }
let(:exportable_full_path) { 'foo' }
let(:time_suffix) { Time.current.utc.strftime('%FT%H%M') }
diff --git a/ee/spec/services/vulnerability_exports/exporters/csv_service_spec.rb b/ee/spec/services/vulnerability_exports/exporters/csv_service_spec.rb
index 0278155cdad61..a4b02947fe2f7 100644
--- a/ee/spec/services/vulnerability_exports/exporters/csv_service_spec.rb
+++ b/ee/spec/services/vulnerability_exports/exporters/csv_service_spec.rb
@@ -36,6 +36,7 @@
let_it_be(:group) { create(:group) }
let_it_be(:project) { project_with_valid_findings(group: group) }
let_it_be(:vulnerability) { create(:vulnerability, :with_findings, project: project) }
+ let_it_be(:note) { create(:note, project: project, noteable: vulnerability) }
it 'includes proper values for each column type', :aggregate_failures do
expect(csv[0]['Group Name']).to eq group.name
@@ -53,6 +54,7 @@
expect(csv[0]['Detected At']).to eq vulnerability.created_at.to_s
expect(csv[0]['Location']).to eq vulnerability.location.to_s
expect(csv[0]['Activity']).to eq vulnerability.resolved_on_default_branch.to_s
+ expect(csv[0]['Comments']).to include(note.note)
end
end
@@ -60,6 +62,7 @@
let_it_be(:user) { create(:user) }
let_it_be(:project) { project_with_valid_findings(namespace: user.namespace) }
let_it_be(:vulnerability) { create(:vulnerability, :with_findings, project: project) }
+ let_it_be(:note) { create(:note, project: project, noteable: vulnerability) }
it 'includes proper values for each column except group name' do
expect(csv[0]['Group Name']).to be_nil
@@ -77,6 +80,7 @@
expect(csv[0]['Detected At']).to eq vulnerability.created_at.to_s
expect(csv[0]['Location']).to eq vulnerability.location.to_s
expect(csv[0]['Activity']).to eq vulnerability.resolved_on_default_branch.to_s
+ expect(csv[0]['Comments']).to include(note.note)
end
end
end
@@ -102,6 +106,7 @@
expect(csv[0]['Detected At']).to eq vulnerability.created_at.to_s
expect(csv[0]['Location']).to be_nil
expect(csv[0]['Activity']).to eq vulnerability.resolved_on_default_branch.to_s
+ expect(csv[0]['Comments']).to be_empty
end
end
end
diff --git a/ee/spec/support/helpers/vulnerability_exports_helpers.rb b/ee/spec/support/helpers/vulnerability_exports_helpers.rb
index a6007123fbc17..e05e841f9ac50 100644
--- a/ee/spec/support/helpers/vulnerability_exports_helpers.rb
+++ b/ee/spec/support/helpers/vulnerability_exports_helpers.rb
@@ -4,7 +4,7 @@ module VulnerabilityExportsHelpers
def csv_headers
['Group Name', 'Project Name', 'Tool', 'Scanner Name', 'Status',
'Vulnerability', 'Details', 'Additional Info', 'Severity', 'CVE',
- 'CWE', 'Other Identifiers', 'Detected At', 'Location', 'Activity']
+ 'CWE', 'Other Identifiers', 'Detected At', 'Location', 'Activity', 'Comments']
end
def other_identifiers
--
GitLab