Commit 231545fd authored by Lucas Charles's avatar Lucas Charles

Exclude secret_detection findings from autoresolution

Secret Detection findings should be treated as distinct from other
finding types as once they are removed from a branch's HEAD that does
not guarantee them to be safe and they require manual resolution, such
as rotation of the leaked token since they remain present in the git
history.

Relates to https://gitlab.com/gitlab-org/gitlab/-/issues/223248

Changelog: changed
EE: true
parent ca3aa6e0
...@@ -2,6 +2,15 @@ ...@@ -2,6 +2,15 @@
module VulnerabilityFindingHelpers module VulnerabilityFindingHelpers
extend ActiveSupport::Concern extend ActiveSupport::Concern
# Manually resolvable report types cannot be considered fixed once removed from the
# target branch due to requiring active triage, such as rotation of an exposed token.
REPORT_TYPES_REQUIRING_MANUAL_RESOLUTION = %w[secret_detection].freeze
def requires_manual_resolution?
REPORT_TYPES_REQUIRING_MANUAL_RESOLUTION.include?(report_type)
end
def matches_signatures(other_signatures, other_uuid) def matches_signatures(other_signatures, other_uuid)
other_signature_types = other_signatures.index_by(&:algorithm_type) other_signature_types = other_signatures.index_by(&:algorithm_type)
......
...@@ -73,6 +73,8 @@ module Security ...@@ -73,6 +73,8 @@ module Security
end end
def mark_as_resolved_except(vulnerability_ids) def mark_as_resolved_except(vulnerability_ids)
return if ::Vulnerabilities::Finding::REPORT_TYPES_REQUIRING_MANUAL_RESOLUTION.include?(report.type)
project.vulnerabilities project.vulnerabilities
.with_report_types(report.type) .with_report_types(report.type)
.id_not_in(vulnerability_ids) .id_not_in(vulnerability_ids)
......
...@@ -477,6 +477,15 @@ RSpec.describe Security::StoreReportService, '#execute', :snowplow do ...@@ -477,6 +477,15 @@ RSpec.describe Security::StoreReportService, '#execute', :snowplow do
end end
end end
context 'when the existing vulnerability requires manual resolution' do
let(:trait) { :secret_detection }
let!(:finding) { create(:vulnerabilities_finding, :with_secret_detection, project: project, pipelines: [pipeline]) }
it 'wont mark the vulnerability as resolved on default branch' do
expect { subject }.not_to change { finding.vulnerability.reload.resolved_on_default_branch }
end
end
context 'when the existing resolved vulnerability is discovered again on the latest report' do context 'when the existing resolved vulnerability is discovered again on the latest report' do
before do before do
vulnerability.update_column(:resolved_on_default_branch, true) vulnerability.update_column(:resolved_on_default_branch, true)
......
...@@ -80,6 +80,8 @@ module Gitlab ...@@ -80,6 +80,8 @@ module Gitlab
matcher = FindingMatcher.new(head_findings) matcher = FindingMatcher.new(head_findings)
base_findings.each do |base_finding| base_findings.each do |base_finding|
next if base_finding.requires_manual_resolution?
matched_head_finding = matcher.find_and_remove_match!(base_finding) matched_head_finding = matcher.find_and_remove_match!(base_finding)
@fixed_findings << base_finding if matched_head_finding.nil? @fixed_findings << base_finding if matched_head_finding.nil?
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe VulnerabilityFindingHelpers do
let(:cls) do
Class.new do
include VulnerabilityFindingHelpers
attr_accessor :report_type
def initialize(report_type)
@report_type = report_type
end
end
end
describe '#requires_manual_resolution?' do
it 'returns false if the finding does not require manual resolution' do
expect(cls.new('sast').requires_manual_resolution?).to eq(false)
end
it 'returns true when the finding requires manual resolution' do
expect(cls.new('secret_detection').requires_manual_resolution?).to eq(true)
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment