diff --git a/ee/app/controllers/ee/projects/merge_requests_controller.rb b/ee/app/controllers/ee/projects/merge_requests_controller.rb
index ef50d31521afea30a1d93fd46a9146c3b1e93ddd..88ab092b134df1cde21f9bd9bca8bd86543b9abd 100644
--- a/ee/app/controllers/ee/projects/merge_requests_controller.rb
+++ b/ee/app/controllers/ee/projects/merge_requests_controller.rb
@@ -24,9 +24,12 @@ module MergeRequestsController
:metrics_reports]
before_action :authorize_read_licenses!, only: [:license_scanning_reports, :license_scanning_reports_collapsed]
+ before_action :authorize_read_security_reports!, only: [:security_reports]
+
feature_category :vulnerability_management, [:container_scanning_reports, :dependency_scanning_reports,
- :sast_reports, :secret_detection_reports,
- :dast_reports, :coverage_fuzzing_reports, :api_fuzzing_reports]
+ :sast_reports, :secret_detection_reports, :dast_reports,
+ :coverage_fuzzing_reports, :api_fuzzing_reports,
+ :security_reports]
feature_category :metrics, [:metrics_reports]
feature_category :license_compliance, [:license_scanning_reports, :license_scanning_reports_collapsed]
feature_category :code_review_workflow, [:delete_description_version, :description_diff]
@@ -37,7 +40,8 @@ module MergeRequestsController
:secret_detection_reports, :dast_reports,
:coverage_fuzzing_reports, :api_fuzzing_reports,
:metrics_reports, :description_diff,
- :license_scanning_reports, :license_scanning_reports_collapsed]
+ :license_scanning_reports, :license_scanning_reports_collapsed,
+ :security_reports]
end
def can_run_sast_experiments_on?(project)
@@ -76,6 +80,20 @@ def coverage_fuzzing_reports
def api_fuzzing_reports
reports_response(merge_request.compare_api_fuzzing_reports(current_user), head_pipeline)
end
+
+ def security_reports
+ report = ::Security::MergeRequestSecurityReportGenerationService.execute(merge_request, params[:type])
+
+ reports_response(report, head_pipeline)
+ rescue ::Security::MergeRequestSecurityReportGenerationService::InvalidReportTypeError
+ head :bad_request
+ end
+
+ private
+
+ def authorize_read_security_reports!
+ return render_404 unless can?(current_user, :read_security_resource, merge_request.project)
+ end
end
end
end
diff --git a/ee/app/services/security/merge_request_security_report_generation_service.rb b/ee/app/services/security/merge_request_security_report_generation_service.rb
new file mode 100644
index 0000000000000000000000000000000000000000..c4218e44cf41d0aefa52d17a20d59cfed7d5f99f
--- /dev/null
+++ b/ee/app/services/security/merge_request_security_report_generation_service.rb
@@ -0,0 +1,91 @@
+# frozen_string_literal: true
+
+module Security
+ class MergeRequestSecurityReportGenerationService
+ include Gitlab::Utils::StrongMemoize
+
+ DEFAULT_FINDING_STATE = 'detected'
+ FINDING_KEYS = %w[name state severity uuid].freeze
+
+ InvalidReportTypeError = Class.new(ArgumentError)
+
+ def self.execute(merge_request, report_type)
+ new(merge_request, report_type).execute
+ end
+
+ def initialize(merge_request, report_type)
+ @merge_request = merge_request
+ @report_type = report_type
+ end
+
+ def execute
+ return report unless report_available?
+
+ set_states_of!(added_findings)
+ set_states_of!(fixed_findings)
+ remove_redundant_keys_of!(added_findings)
+ remove_redundant_keys_of!(fixed_findings)
+
+ report
+ end
+
+ private
+
+ attr_reader :merge_request, :report_type
+
+ def report_available?
+ report[:status] == :parsed
+ end
+
+ def set_states_of!(findings)
+ findings.each { |finding| finding['state'] = state_for(finding['uuid']) }
+ end
+
+ def remove_redundant_keys_of!(findings)
+ findings.each { |finding| finding.slice!(*FINDING_KEYS) }
+ end
+
+ def state_for(uuid)
+ existing_vulnerabilities[uuid] || DEFAULT_FINDING_STATE
+ end
+
+ def existing_vulnerabilities
+ @existing_vulnerabilities ||= Vulnerability.with_findings_by_uuid(finding_uuids)
+ .pluck(:uuid, :state) # rubocop:disable CodeReuse/ActiveRecord
+ .to_h
+ end
+
+ def finding_uuids
+ (added_findings + fixed_findings).pluck('uuid') # rubocop:disable CodeReuse/ActiveRecord
+ end
+
+ def added_findings
+ @added_findings ||= report.dig(:data, 'added')
+ end
+
+ def fixed_findings
+ @fixed_findings ||= report.dig(:data, 'fixed')
+ end
+
+ strong_memoize_attr def report
+ case report_type
+ when 'sast'
+ merge_request.compare_sast_reports(nil)
+ when 'secret_detection'
+ merge_request.compare_secret_detection_reports(nil)
+ when 'container_scanning'
+ merge_request.compare_container_scanning_reports(nil)
+ when 'dependency_scanning'
+ merge_request.compare_dependency_scanning_reports(nil)
+ when 'dast'
+ merge_request.compare_dast_reports(nil)
+ when 'coverage_fuzzing'
+ merge_request.compare_coverage_fuzzing_reports(nil)
+ when 'api_fuzzing'
+ merge_request.compare_api_fuzzing_reports(nil)
+ else
+ raise InvalidReportTypeError
+ end
+ end
+ end
+end
diff --git a/ee/config/routes/merge_requests.rb b/ee/config/routes/merge_requests.rb
index 5851c29eef3681be5bbdc056a811b837d5630f70..517e8a8075db265dbe5ea3e070502212bb843884 100644
--- a/ee/config/routes/merge_requests.rb
+++ b/ee/config/routes/merge_requests.rb
@@ -14,6 +14,7 @@
get :dast_reports
get :coverage_fuzzing_reports
get :api_fuzzing_reports
+ get :security_reports
post :rebase
end
diff --git a/ee/spec/requests/projects/merge_requests_controller_spec.rb b/ee/spec/requests/projects/merge_requests_controller_spec.rb
index e920ed9fe22a12e7b16914636d5ba6ef92860d1a..8a265a88d4d21b35491b731a8cb4e3b3c4026c29 100644
--- a/ee/spec/requests/projects/merge_requests_controller_spec.rb
+++ b/ee/spec/requests/projects/merge_requests_controller_spec.rb
@@ -136,4 +136,105 @@ def get_index
expect { get_index }.not_to exceed_query_limit(control_count)
end
end
+
+ describe 'security_reports' do
+ let_it_be(:merge_request) { create(:merge_request, :with_head_pipeline) }
+ let_it_be(:user) { create(:user) }
+
+ subject(:request_report) { get security_reports_project_merge_request_path(project, merge_request, type: :sast, format: :json) }
+
+ before do
+ stub_licensed_features(security_dashboard: true)
+ end
+
+ context 'when the user can not read project security resources' do
+ before do
+ project.add_guest(user)
+ end
+
+ it 'responds with 404' do
+ request_report
+
+ expect(response).to have_gitlab_http_status(:not_found)
+ end
+ end
+
+ context 'when the user can read project security resources' do
+ before do
+ project.add_developer(user)
+ end
+
+ context 'when the pipeline is pending' do
+ it 'returns 204 HTTP status along with the `Poll-Interval` header' do
+ request_report
+
+ expect(response).to have_gitlab_http_status(:no_content)
+ expect(response.headers['Poll-Interval']).to eq('3000')
+ end
+ end
+
+ context 'when the pipeline is not pending' do
+ before do
+ merge_request.head_pipeline.reload.succeed!
+ end
+
+ context 'when the given type is invalid' do
+ let(:error) { ::Security::MergeRequestSecurityReportGenerationService::InvalidReportTypeError }
+
+ before do
+ allow(::Security::MergeRequestSecurityReportGenerationService).to receive(:execute).and_raise(error)
+ end
+
+ it 'responds with 400' do
+ request_report
+
+ expect(response).to have_gitlab_http_status(:bad_request)
+ expect(response.header).not_to include('Poll-Interval')
+ end
+ end
+
+ context 'when the given type is valid' do
+ before do
+ allow(::Security::MergeRequestSecurityReportGenerationService)
+ .to receive(:execute).with(an_instance_of(MergeRequest), 'sast').and_return(report_payload)
+ end
+
+ context 'when comparison is being processed' do
+ let(:report_payload) { { status: :parsing } }
+
+ it 'returns 204 HTTP status along with the `Poll-Interval` header' do
+ request_report
+
+ expect(response).to have_gitlab_http_status(:no_content)
+ expect(response.headers['Poll-Interval']).to eq('3000')
+ end
+ end
+
+ context 'when comparison is done' do
+ context 'when the comparison is errored' do
+ let(:report_payload) { { status: :error } }
+
+ it 'responds with 400' do
+ request_report
+
+ expect(response).to have_gitlab_http_status(:bad_request)
+ expect(response.header).not_to include('Poll-Interval')
+ end
+ end
+
+ context 'when the comparision is succeeded' do
+ let(:report_payload) { { status: :parsed, data: { added: ['foo'], fixed: ['bar'] } } }
+
+ it 'responds with 200 along with the report payload' do
+ request_report
+
+ expect(response).to have_gitlab_http_status(:ok)
+ expect(json_response).to eq({ 'added' => ['foo'], 'fixed' => ['bar'] })
+ end
+ end
+ end
+ end
+ end
+ end
+ end
end
diff --git a/ee/spec/services/security/merge_request_security_report_generation_service_spec.rb b/ee/spec/services/security/merge_request_security_report_generation_service_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..c0c02618d4ce01f05174bdc11a3e281ed8fe6772
--- /dev/null
+++ b/ee/spec/services/security/merge_request_security_report_generation_service_spec.rb
@@ -0,0 +1,130 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Security::MergeRequestSecurityReportGenerationService, feature_category: :vulnerability_management do
+ describe '#execute' do
+ let_it_be(:merge_request) { create(:merge_request) }
+
+ let(:service_object) { described_class.new(merge_request, report_type) }
+
+ subject(:report) { service_object.execute }
+
+ context 'when the given report type is invalid' do
+ let(:report_type) { 'foo' }
+
+ it 'raises InvalidReportTypeError' do
+ expect { report }.to raise_error(described_class::InvalidReportTypeError)
+ end
+ end
+
+ context 'when the given report type is valid' do
+ using RSpec::Parameterized::TableSyntax
+
+ let_it_be(:confirmed_finding) { create(:vulnerabilities_finding, :confirmed) }
+ let_it_be(:dismissed_finding) { create(:vulnerabilities_finding, :dismissed) }
+
+ let(:new_uuid) { SecureRandom.uuid }
+ let(:confirmed_uuid) { confirmed_finding.uuid }
+ let(:dismissed_uuid) { dismissed_finding.uuid }
+
+ where(:report_type, :mr_report_method) do
+ 'sast' | :compare_sast_reports
+ 'secret_detection' | :compare_secret_detection_reports
+ 'container_scanning' | :compare_container_scanning_reports
+ 'dependency_scanning' | :compare_dependency_scanning_reports
+ 'dast' | :compare_dast_reports
+ 'coverage_fuzzing' | :compare_coverage_fuzzing_reports
+ 'api_fuzzing' | :compare_api_fuzzing_reports
+ end
+
+ with_them do
+ let(:mock_report) do
+ {
+ status: report_status,
+ data: {
+ 'base_report_created_at' => nil,
+ 'base_report_out_of_date' => false,
+ 'head_report_created_at' => '2023-01-18T11:30:01.035Z',
+ 'added' => [
+ {
+ 'id' => nil,
+ 'name' => 'Test vulnerability 1',
+ 'uuid' => new_uuid,
+ 'severity' => 'critical'
+ },
+ {
+ 'id' => nil,
+ 'name' => 'Test vulnerability 2',
+ 'uuid' => confirmed_uuid,
+ 'severity' => 'critical'
+ }
+ ],
+ 'fixed' => [
+ {
+ 'id' => nil,
+ 'name' => 'Test vulnerability 3',
+ 'uuid' => dismissed_uuid,
+ 'severity' => 'low'
+ }
+ ]
+ }
+ }
+ end
+
+ before do
+ allow(merge_request).to receive(mr_report_method).with(nil).and_return(mock_report)
+ end
+
+ context 'when the report status is `parsing`' do
+ let(:report_status) { :parsing }
+
+ it 'returns the report' do
+ expect(report).to eq(mock_report)
+ end
+ end
+
+ context 'when the report status is `parsed`' do
+ let(:report_status) { :parsed }
+ let(:expected_report) do
+ {
+ status: report_status,
+ data: {
+ 'base_report_created_at' => nil,
+ 'base_report_out_of_date' => false,
+ 'head_report_created_at' => '2023-01-18T11:30:01.035Z',
+ 'added' => [
+ {
+ 'name' => 'Test vulnerability 1',
+ 'uuid' => new_uuid,
+ 'severity' => 'critical',
+ 'state' => 'detected'
+ },
+ {
+ 'name' => 'Test vulnerability 2',
+ 'uuid' => confirmed_uuid,
+ 'severity' => 'critical',
+ 'state' => 'confirmed'
+ }
+ ],
+ 'fixed' => [
+ {
+ 'name' => 'Test vulnerability 3',
+ 'uuid' => dismissed_uuid,
+ 'severity' => 'low',
+ 'state' => 'dismissed'
+ }
+ ]
+ }
+ }
+ end
+
+ it 'returns `title`, `severity`, `uuid`, and `state` of the findings' do
+ expect(report).to eq(expected_report)
+ expect(merge_request).to have_received(mr_report_method).with(nil)
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/ci/reports/security/vulnerability_reports_comparer.rb b/lib/gitlab/ci/reports/security/vulnerability_reports_comparer.rb
index 4be4cf62e7b1858d362af98f94d352856416eb0c..adc666d99872881e42ada4249556837cbbd7dbb5 100644
--- a/lib/gitlab/ci/reports/security/vulnerability_reports_comparer.rb
+++ b/lib/gitlab/ci/reports/security/vulnerability_reports_comparer.rb
@@ -10,6 +10,7 @@ class VulnerabilityReportsComparer
attr_reader :base_report, :head_report
ACCEPTABLE_REPORT_AGE = 1.week
+ MAX_FINDINGS_COUNT = 25
def initialize(project, base_report, head_report)
@base_report = base_report
@@ -40,21 +41,13 @@ def base_report_out_of_date
def added
strong_memoize(:added) do
- if @signatures_enabled
- @added_findings
- else
- head_report.findings - base_report.findings
- end
+ all_added_findings.take(MAX_FINDINGS_COUNT) # rubocop:disable CodeReuse/ActiveRecord (This is Array#take)
end
end
def fixed
strong_memoize(:fixed) do
- if @signatures_enabled
- @fixed_findings
- else
- base_report.findings - head_report.findings
- end
+ all_fixed_findings.take(MAX_FINDINGS_COUNT) # rubocop:disable CodeReuse/ActiveRecord (This is Array#take)
end
end
@@ -89,6 +82,22 @@ def calculate_changes
@added_findings = matcher.unmatched_head_findings.values
end
+
+ def all_added_findings
+ if @signatures_enabled
+ @added_findings
+ else
+ head_report.findings - base_report.findings
+ end
+ end
+
+ def all_fixed_findings
+ if @signatures_enabled
+ @fixed_findings
+ else
+ base_report.findings - head_report.findings
+ end
+ end
end
class FindingMatcher
diff --git a/spec/lib/gitlab/ci/reports/security/vulnerability_reports_comparer_spec.rb b/spec/lib/gitlab/ci/reports/security/vulnerability_reports_comparer_spec.rb
index 6f75e2c55e87126aaaff6d372c0a03c6f25a8fb4..393d65ff1023a10cb6066df14352eea995b88586 100644
--- a/spec/lib/gitlab/ci/reports/security/vulnerability_reports_comparer_spec.rb
+++ b/spec/lib/gitlab/ci/reports/security/vulnerability_reports_comparer_spec.rb
@@ -2,7 +2,7 @@
require 'spec_helper'
-RSpec.describe Gitlab::Ci::Reports::Security::VulnerabilityReportsComparer do
+RSpec.describe Gitlab::Ci::Reports::Security::VulnerabilityReportsComparer, feature_category: :vulnerability_management do
let(:identifier) { build(:ci_reports_security_identifier) }
let_it_be(:project) { create(:project, :repository) }
@@ -90,6 +90,18 @@
expect(subject.added).to eq([vuln, low_vuln])
end
end
+
+ describe 'number of findings' do
+ let(:head_report) { build(:ci_reports_security_aggregated_reports, findings: [head_vulnerability, vuln, low_vuln]) }
+
+ before do
+ stub_const("#{described_class}::MAX_FINDINGS_COUNT", 1)
+ end
+
+ it 'returns no more than `MAX_FINDINGS_COUNT`' do
+ expect(subject.added).to eq([vuln])
+ end
+ end
end
describe '#fixed' do
@@ -123,6 +135,18 @@
expect(subject.fixed).to eq(vul_findings)
end
end
+
+ describe 'number of findings' do
+ let(:base_report) { build(:ci_reports_security_aggregated_reports, findings: [vuln, medium_vuln, base_vulnerability]) }
+
+ before do
+ stub_const("#{described_class}::MAX_FINDINGS_COUNT", 1)
+ end
+
+ it 'returns no more than `MAX_FINDINGS_COUNT`' do
+ expect(subject.fixed).to eq([vuln])
+ end
+ end
end
describe 'with empty vulnerabilities' do