Merge requests shows duplicate container scanning vulnerabilities as detected by dependency scanning
Summary
When running Container Scanning, the detected vulnerabilities are shown twice on the merge request view. Once as "Container Scanning detected x potential vulnerabilities" and a second time as "Dependency Scanning detected x potential vulnerabilities"
Some background: We added the ability for container scanning to output a dependency scanning report so that packages in the container can be shown on the dependency list page (&6698 (closed)). However, this report only populates the dependency_files
field on the security report. The vulnerabilities
field is left empty.
Steps to reproduce
-
Scan an image which detects new vulnerabilities. Ex:
include: - template: Security/Container-Scanning.gitlab-ci.yml container_scanning: variables: CS_DISABLE_DEPENDENCY_SCAN: "false" CS_DISABLE_LANGUAGE_VULNERABILITY_SCAN: "true" GIT_STRATEGY: fetch SECURE_LOG_LEVEL: debug DOCKER_IMAGE: debian:latest
-
Open a merge request
-
Observe that the same vulnerabilities are shown twice
- Download
gl-dependency-scanning-report.json
- It contains no vulnerabilities
Example Project
https://gitlab.com/gitlab-org/protect/demos/container-scanning-test/-/merge_requests/2
Report artifacts: artifacts__5_.zip
What is the current bug behavior?
Vulnerabilities are shown as "Detected by dependency scanning" when gl-dependency-scanning-report.json
contains 0 vulnerabilities.
What is the expected correct behavior?
The vulnerabilities should display only as "Detected by container scanning"
Possible fixes
Ultimately, the issue seems to be pipeline.security_reports
, which is called by PipelineVulnerabilitiesFinder
.
def security_reports(report_types: [])
reports_scope = report_types.empty? ? ::Ci::JobArtifact.security_reports : ::Ci::JobArtifact.security_reports(file_types: report_types)
::Gitlab::Ci::Reports::Security::Reports.new(self).tap do |security_reports|
latest_report_builds(reports_scope).each do |build|
build.collect_security_reports!(security_reports)
end
end
end
The way this method gets reports is to:
- Query for
ci_builds
(jobs) which ran on the latest pipeline and havereport_type = 'dependency_scanning'
. - Parse all the security report files from that job and return them.
This has worked without issue before, because Rails has been operating under the assumption that each job has only one security report, and up until now that has been true.
We should allow collect_security_reports!
to receive an optional parameter which can be used to restrict which report types are collected from the build:
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index ae3ea7aa03f..671d9611fe5 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -1267,10 +1267,11 @@ def merge_train_pipeline?
def security_reports(report_types: [])
reports_scope = report_types.empty? ? ::Ci::JobArtifact.security_reports : ::Ci::JobArtifact.security_reports(file_types: report_types)
+ types_to_collect = report_types.empty? ? ::Ci::JobArtifact::SECURITY_REPORT_FILE_TYPES : report_types
::Gitlab::Ci::Reports::Security::Reports.new(self).tap do |security_reports|
latest_report_builds(reports_scope).each do |build|
- build.collect_security_reports!(security_reports)
+ build.collect_security_reports!(security_reports, report_types: types_to_collect)
end
end
end
diff --git a/ee/app/models/ee/ci/build.rb b/ee/app/models/ee/ci/build.rb
index 5e74521540c..80425b1090c 100644
--- a/ee/app/models/ee/ci/build.rb
+++ b/ee/app/models/ee/ci/build.rb
@@ -99,8 +99,8 @@ def has_security_reports?
job_artifacts.security_reports.any?
end
- def collect_security_reports!(security_reports)
- each_report(::Ci::JobArtifact::SECURITY_REPORT_FILE_TYPES) do |file_type, blob, report_artifact|
+ def collect_security_reports!(security_reports, report_types: ::Ci::JobArtifact::SECURITY_REPORT_FILE_TYPES)
+ each_report(report_types) do |file_type, blob, report_artifact|
security_reports.get_report(file_type, report_artifact).tap do |security_report|
next unless project.feature_available?(LICENSED_PARSER_FEATURES.fetch(file_type))