Merge branch 'add_notes_into_csv_export' into 'master'

Add notes summary into csv export for vulnerabilities

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/102270



Merged-by: Mark Chao <mchao@gitlab.com>
Approved-by: Gregory Havenga <11164960-ghavenga@users.noreply.gitlab.com>
Approved-by: Mark Chao <mchao@gitlab.com>
Co-authored-by: Zamir Martins Filho <zfilho@gitlab.com>

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,""
 ```

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

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?

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

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

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

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

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

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') }

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

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