From 768d16b3363c6a4e90647a040d9b0b3e78e23e5c Mon Sep 17 00:00:00 2001
From: "Alan (Maciej) Paruszewski" <mparuszewski@gitlab.com>
Date: Mon, 27 Jun 2022 22:42:11 +0000
Subject: [PATCH] Fix resolving cluster image scanning vulnerabilities
Changelog: fixed
EE: true
---
ee/app/models/ee/vulnerability.rb | 2 +-
ee/app/models/vulnerabilities/finding.rb | 1 +
...starboard_vulnerability_resolve_service.rb | 5 ++++-
ee/spec/factories/vulnerabilities.rb | 20 +++++++++++++++++++
.../models/vulnerabilities/finding_spec.rb | 10 ++++++++++
.../requests/api/internal/kubernetes_spec.rb | 2 +-
...oard_vulnerability_resolve_service_spec.rb | 18 ++++++++++++++++-
7 files changed, 54 insertions(+), 4 deletions(-)
diff --git a/ee/app/models/ee/vulnerability.rb b/ee/app/models/ee/vulnerability.rb
index 43498ae5d0df2..16f5795a22bcc 100644
--- a/ee/app/models/ee/vulnerability.rb
+++ b/ee/app/models/ee/vulnerability.rb
@@ -75,7 +75,7 @@ def with_vulnerability_links
scope :with_findings, -> { includes(:findings) }
scope :with_findings_by_uuid, -> (uuid) { with_findings.where(findings: { uuid: uuid }) }
scope :with_findings_by_uuid_and_state, -> (uuid, state) { with_findings.where(findings: { uuid: uuid }, state: state) }
- scope :with_findings_excluding_uuid, -> (uuid) { with_findings.where.not(findings: { uuid: uuid }) }
+ 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) }
diff --git a/ee/app/models/vulnerabilities/finding.rb b/ee/app/models/vulnerabilities/finding.rb
index c99bd15c5a8bb..720bf8430e05c 100644
--- a/ee/app/models/vulnerabilities/finding.rb
+++ b/ee/app/models/vulnerabilities/finding.rb
@@ -91,6 +91,7 @@ class Finding < ApplicationRecord
scope :by_location_fingerprints, -> (values) { where(location_fingerprint: values) }
scope :by_project_fingerprints, -> (values) { where(project_fingerprint: values) }
scope :by_uuid, -> (uuids) { where(uuid: uuids) }
+ scope :excluding_uuids, -> (uuids) { where.not(uuid: uuids) }
scope :eager_load_comparison_entities, -> { includes(:scanner, :primary_identifier) }
scope :all_preloaded, -> do
diff --git a/ee/app/services/vulnerabilities/starboard_vulnerability_resolve_service.rb b/ee/app/services/vulnerabilities/starboard_vulnerability_resolve_service.rb
index af6b206620997..3030077216dc3 100644
--- a/ee/app/services/vulnerabilities/starboard_vulnerability_resolve_service.rb
+++ b/ee/app/services/vulnerabilities/starboard_vulnerability_resolve_service.rb
@@ -8,11 +8,13 @@ class StarboardVulnerabilityResolveService
STATES = Vulnerability::ACTIVE_STATES
BATCH_SIZE = 250
- attr_reader :project,
+ attr_reader :agent,
+ :project,
:author,
:uuids
def initialize(agent, uuids)
+ @agent = agent
@project = agent.project
@author = agent.created_by_user
@uuids = uuids
@@ -35,6 +37,7 @@ def undetected
.with_states(STATES)
.with_report_types(REPORT_TYPE)
.with_findings_excluding_uuid(uuids)
+ .with_cluster_agent_ids([agent.id.to_s])
end
def authorized?
diff --git a/ee/spec/factories/vulnerabilities.rb b/ee/spec/factories/vulnerabilities.rb
index 91831c859b64d..183a91525a880 100644
--- a/ee/spec/factories/vulnerabilities.rb
+++ b/ee/spec/factories/vulnerabilities.rb
@@ -99,6 +99,26 @@
end
end
+ trait :with_cluster_image_scanning_finding do
+ transient do
+ agent_id { '46357' }
+ end
+
+ after(:build) do |vulnerability, evaluator|
+ finding = build(
+ :vulnerabilities_finding,
+ :identifier,
+ :with_cluster_image_scanning_scanning_metadata,
+ agent_id: evaluator.agent_id,
+ vulnerability: vulnerability,
+ report_type: :cluster_image_scanning,
+ project: vulnerability.project
+ )
+
+ vulnerability.findings = [finding]
+ end
+ end
+
trait :with_remediation do
after(:build) do |vulnerability|
finding = build(
diff --git a/ee/spec/models/vulnerabilities/finding_spec.rb b/ee/spec/models/vulnerabilities/finding_spec.rb
index 90077f5c14c52..cafd1064d3526 100644
--- a/ee/spec/models/vulnerabilities/finding_spec.rb
+++ b/ee/spec/models/vulnerabilities/finding_spec.rb
@@ -1119,4 +1119,14 @@
it { is_expected.to contain_exactly(finding) }
end
+
+ describe '.excluding_uuids' do
+ let(:finding_1) { create(:vulnerabilities_finding) }
+ let(:finding_2) { create(:vulnerabilities_finding) }
+ let(:finding_3) { create(:vulnerabilities_finding) }
+
+ subject { described_class.excluding_uuids([finding_1.uuid, finding_3.uuid]) }
+
+ it { is_expected.to contain_exactly(finding_2) }
+ end
end
diff --git a/ee/spec/requests/api/internal/kubernetes_spec.rb b/ee/spec/requests/api/internal/kubernetes_spec.rb
index fc6c460e6ebad..573fa367935ad 100644
--- a/ee/spec/requests/api/internal/kubernetes_spec.rb
+++ b/ee/spec/requests/api/internal/kubernetes_spec.rb
@@ -291,7 +291,7 @@ def send_request(headers: {}, params: {})
let_it_be(:agent) { agent_token.agent }
let_it_be(:project) { agent.project }
- let_it_be(:existing_vulnerabilities) { create_list(:vulnerability, 4, :detected, :with_finding, project: project, report_type: :cluster_image_scanning) }
+ let_it_be(:existing_vulnerabilities) { create_list(:vulnerability, 4, :detected, :with_cluster_image_scanning_finding, agent_id: agent.id.to_s, project: project, report_type: :cluster_image_scanning) }
let_it_be(:detected_vulnerabilities) { existing_vulnerabilities.first(2) }
let_it_be(:undetected_vulnerabilities) { existing_vulnerabilities - detected_vulnerabilities }
let_it_be(:payload) { { uuids: detected_vulnerabilities.map { |vuln| vuln.finding.uuid } } }
diff --git a/ee/spec/services/vulnerabilities/starboard_vulnerability_resolve_service_spec.rb b/ee/spec/services/vulnerabilities/starboard_vulnerability_resolve_service_spec.rb
index 626404da1bc38..ac7aa7ad00498 100644
--- a/ee/spec/services/vulnerabilities/starboard_vulnerability_resolve_service_spec.rb
+++ b/ee/spec/services/vulnerabilities/starboard_vulnerability_resolve_service_spec.rb
@@ -4,10 +4,17 @@
RSpec.describe Vulnerabilities::StarboardVulnerabilityResolveService do
let_it_be(:agent) { create(:cluster_agent) }
+ let_it_be(:other_agent) { create(:cluster_agent) }
let_it_be(:project) { agent.project }
let_it_be(:user) { agent.created_by_user }
- let_it_be(:existing_vulnerabilities) { create_list(:vulnerability, 4, :detected, :with_finding, project: project, report_type: :cluster_image_scanning) }
+ let_it_be(:existing_vulnerabilities) do
+ create_list(:vulnerability, 4,
+ :detected, :with_cluster_image_scanning_finding,
+ agent_id: agent.id.to_s, project: project, report_type: :cluster_image_scanning
+ )
+ end
+
let_it_be(:detected_vulnerabilities) { existing_vulnerabilities.first(2) }
let_it_be(:undetected_vulnerabilities) { existing_vulnerabilities - detected_vulnerabilities }
let_it_be(:uuids) { detected_vulnerabilities.map(&:finding).map(&:uuid) }
@@ -57,6 +64,15 @@
expect { subject }.not_to change { Vulnerability.resolved.count }
end
+ it "does not resolve vulnerabilities created by other agent" do
+ Vulnerabilities::Finding.all.each do |finding|
+ finding.location['kubernetes_resource']['agent_id'] = other_agent.id.to_s
+ finding.save!
+ end
+
+ expect { subject }.not_to change { Vulnerability.resolved.count }
+ end
+
it 'does not resolve vulnerabilities in passive states' do
EE::Vulnerability::PASSIVE_STATES.each do |state|
Vulnerability.where(id: undetected_vulnerabilities).update_all(state: state)
--
GitLab