diff --git a/ee/app/finders/security/project_tracked_contexts_finder.rb b/ee/app/finders/security/project_tracked_contexts_finder.rb new file mode 100644 index 0000000000000000000000000000000000000000..5934196e37871ed05b39cc36e3e5d0d92390fff9 --- /dev/null +++ b/ee/app/finders/security/project_tracked_contexts_finder.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +module Security + class ProjectTrackedContextsFinder + def initialize(project = nil, pipeline = nil, params = {}) + @project = project + @pipeline = pipeline + @params = params + end + + def by_project + return [] unless project + + project.security_project_tracked_contexts + .then { |contexts| by_context_name(contexts) } + .then { |contexts| by_context_type(contexts) } + .then { |contexts| by_context_state(contexts) } + end + + def by_pipeline + return unless @pipeline&.ref + + base_query = @project&.security_project_tracked_contexts || ::Security::ProjectTrackedContext + base_query + .by_name(@pipeline.ref) + .by_type(pipeline_context_type(@pipeline)) + .latest + .first + end + + private + + attr_reader :project, :params + + def by_context_name(contexts) + return contexts unless params[:context_names] + + contexts.by_name(params[:context_name]) + end + + def by_context_type(contexts) + return contexts unless params[:context_type] + + contexts.by_type(params[:context_type]) + end + + def by_context_state(contexts) + return contexts unless params[:state] + + contexts.by_state(params[:state]) + end + + def pipeline_context_type(pipeline) + pipeline.tag? ? :tag : :branch + end + end +end diff --git a/ee/app/models/security/project_tracked_context.rb b/ee/app/models/security/project_tracked_context.rb index d1a21e3cdad3c14525273972176b19d5825e06b4..aafcc75a0cad240c23eb1d5c0a1c24a18f14f760 100644 --- a/ee/app/models/security/project_tracked_context.rb +++ b/ee/app/models/security/project_tracked_context.rb @@ -52,6 +52,13 @@ class ProjectTrackedContext < ::SecApplicationRecord scope :for_project, ->(project_id) { where(project_id: project_id) } scope :default_refs, -> { where(is_default: true) } + scope :by_partition_number, ->(partition_number) { where(partition_number: partition_number) } + + scope :by_name, ->(name) { where(context_name: name) } + scope :by_type, ->(type) { where(context_type: type) } + scope :by_state, ->(state) { where(state: state) } + + scope :latest, -> { order(created_at: :desc).limit(1) } STATES.each do |state_name, value| scope state_name, -> { where(state: value) } diff --git a/ee/app/services/vulnerabilities/create_service.rb b/ee/app/services/vulnerabilities/create_service.rb index 16e34d72e7f51561a8cbc92bb4473a287b2786f6..31ac400e846f0e8e3bd496b5e0c651d53a0cc1ef 100644 --- a/ee/app/services/vulnerabilities/create_service.rb +++ b/ee/app/services/vulnerabilities/create_service.rb @@ -5,24 +5,16 @@ class CreateService include Gitlab::Allowable include Gitlab::Utils::StrongMemoize - def initialize( - project, - author, - finding_id:, - state: nil, - present_on_default_branch: true, - comment: nil, - dismissal_reason: nil, - skip_permission_check: false - ) + def initialize(project, author, finding_id:, tracked_context_id:, **options) @project = project @author = author @finding_id = finding_id - @state = state - @present_on_default_branch = present_on_default_branch - @comment = comment - @dismissal_reason = dismissal_reason - @skip_permission_check = skip_permission_check + @tracked_context_id = tracked_context_id + @state = options[:state] + @present_on_default_branch = options.fetch(:present_on_default_branch, true) + @comment = options[:comment] + @dismissal_reason = options[:dismissal_reason] + @skip_permission_check = options.fetch(:skip_permission_check, false) end def execute @@ -34,6 +26,7 @@ def execute Vulnerabilities::Finding.transaction do save_vulnerability(vulnerability, finding) + update_finding_with_tracked_context(finding) rescue ActiveRecord::RecordNotFound vulnerability.errors.add(:base, _('finding is not found or is already attached to a vulnerability')) raise ActiveRecord::Rollback @@ -77,6 +70,35 @@ def save_vulnerability(vulnerability, finding) create_state_transition_if_needed(vulnerability, from_state) if @state end + def update_finding_with_tracked_context(finding) + tracked_context = if @tracked_context_id.present? + ::Security::ProjectTrackedContext.find(@tracked_context_id) + else + derive_tracked_context_from_finding(finding) + end + + return unless tracked_context.present? + + finding.update_column(:security_project_tracked_context_id, tracked_context.id) + end + + def derive_tracked_context_from_finding(finding) + strong_memoize_with(:tracked_context_id, finding.id) do + pipeline = finding.latest_finding_pipeline + tracked_context = Security::ProjectTrackedContextsFinder.new(@project, pipeline).by_pipeline + + unless tracked_context.present? + ::Gitlab::AppLogger.warn( + message: "Failed to find tracked context for vulnerability finding", + project_id: @project.id, + ref_name: ref_name + ) + end + + tracked_context + end + end + def determine_dismissed_at @state&.to_sym == :dismissed ? Time.current : nil end diff --git a/ee/spec/finders/security/project_tracked_contexts_finder_spec.rb b/ee/spec/finders/security/project_tracked_contexts_finder_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..e9d4140ed22f428887a744605e75b1f50c49d587 --- /dev/null +++ b/ee/spec/finders/security/project_tracked_contexts_finder_spec.rb @@ -0,0 +1,182 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Security::ProjectTrackedContextsFinder, feature_category: :vulnerability_management do + let_it_be(:project) { create(:project) } + let_it_be(:pipeline) { create(:ci_pipeline, project: project, ref: 'main') } + let_it_be(:tag_pipeline) { create(:ci_pipeline, project: project, ref: 'v1.0.0', tag: true) } + + let(:finder) { described_class.new(project, pipeline, params) } + let(:params) { {} } + + describe '#by_project' do + context 'when project is nil' do + let(:finder) { described_class.new(nil, pipeline, params) } + + it 'returns an empty array' do + expect(finder.by_project).to eq([]) + end + end + + context 'when project has no tracked contexts' do + it 'returns an empty array' do + expect(finder.by_project).to eq([]) + end + end + + context 'when project has tracked contexts' do + let_it_be(:context_1) do + create(:security_project_tracked_context, project: project, context_name: 'main', context_type: :branch, + state: :tracked) + end + + let_it_be(:context_2) do + create(:security_project_tracked_context, project: project, context_name: 'develop', context_type: :branch, + state: :tracked) + end + + let_it_be(:context_3) do + create(:security_project_tracked_context, project: project, context_name: 'v1.0.0', context_type: :tag, + state: :tracked) + end + + it 'returns all tracked contexts for the project' do + contexts = finder.by_project + + expect(contexts).to match_array([context_1, context_2, context_3]) + end + + context 'when filtering by context_name' do + let(:params) { { context_name: 'main' } } + + it 'returns only contexts matching the name' do + contexts = finder.by_project + + expect(contexts).to contain_exactly(context_1) + end + end + + context 'when filtering by context_type' do + let(:params) { { context_type: :tag } } + + it 'returns only contexts matching the type' do + contexts = finder.by_project + + expect(contexts).to contain_exactly(context_3) + end + end + + context 'when filtering by state' do + let_it_be(:untracked_context) do + create(:security_project_tracked_context, project: project, context_name: 'feature', context_type: :branch, + state: :untracked) + end + + let(:params) { { state: :untracked } } + + it 'returns only contexts matching the state' do + contexts = finder.by_project + + expect(contexts).to contain_exactly(untracked_context) + end + end + + context 'when filtering by multiple parameters' do + let(:params) { { context_type: :branch, state: :tracked } } + + it 'returns contexts matching all filters' do + contexts = finder.by_project + + expect(contexts).to match_array([context_1, context_2]) + end + end + end + end + + describe '#by_pipeline' do + context 'when pipeline is nil' do + let(:finder) { described_class.new(project, nil, params) } + + it 'returns nil' do + expect(finder.by_pipeline).to be_nil + end + end + + context 'when pipeline has no ref' do + let(:pipeline_without_ref) { create(:ci_pipeline, project: project, ref: nil) } + let(:finder) { described_class.new(project, pipeline_without_ref, params) } + + it 'returns nil' do + expect(finder.by_pipeline).to be_nil + end + end + + context 'when pipeline is a branch pipeline' do + let_it_be(:branch_context) do + create(:security_project_tracked_context, project: project, context_name: 'main', context_type: :branch, + state: :tracked) + end + + it 'returns the tracked context for the branch' do + context = finder.by_pipeline + + expect(context).to eq(branch_context) + end + end + + context 'when pipeline is a tag pipeline' do + let_it_be(:tag_context) do + create(:security_project_tracked_context, project: project, context_name: 'v1.0.0', context_type: :tag, + state: :tracked) + end + + let(:finder) { described_class.new(project, tag_pipeline, params) } + + it 'returns the tracked context for the tag' do + context = finder.by_pipeline + + expect(context).to eq(tag_context) + end + end + + context 'when no tracked context exists for the pipeline ref' do + it 'returns nil' do + context = finder.by_pipeline + + expect(context).to be_nil + end + end + + context 'when multiple contexts exist for the same ref' do + let_it_be(:older_context) do + create(:security_project_tracked_context, project: project, context_name: 'main', context_type: :branch, + state: :tracked, created_at: 1.day.ago) + end + + let_it_be(:newer_context) do + create(:security_project_tracked_context, project: project, context_name: 'main', context_type: :branch, + state: :tracked, created_at: Time.current) + end + + it 'returns the most recently created context' do + context = finder.by_pipeline + + expect(context).to eq(newer_context) + end + end + + context 'when project is nil' do + let(:finder) { described_class.new(nil, pipeline, params) } + + it 'searches globally for the context' do + global_context = create(:security_project_tracked_context, context_name: 'main', context_type: :branch, + state: :tracked) + + context = finder.by_pipeline + + expect(context).to eq(global_context) + end + end + end +end diff --git a/ee/spec/services/vulnerabilities/create_service_tracked_context_spec.rb b/ee/spec/services/vulnerabilities/create_service_tracked_context_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..178194334063dac6ff033efb48a099a0f1ba28e5 --- /dev/null +++ b/ee/spec/services/vulnerabilities/create_service_tracked_context_spec.rb @@ -0,0 +1,162 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Vulnerabilities::CreateService, feature_category: :vulnerability_management do + before do + stub_licensed_features(security_dashboard: true) + end + + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + let(:finding) { create(:vulnerabilities_finding, project: project) } + let(:finding_id) { finding.id } + + subject(:create_vulnerability) { described_class.new(project, user, finding_id: finding_id).execute } + + context 'with an authorized user' do + before_all do + project.add_maintainer(user) + end + + describe '#update_finding_with_tracked_context' do + context 'when a tracked context exists for the finding pipeline' do + let_it_be(:pipeline) { create(:ci_pipeline, project: project, ref: 'main') } + let(:finding) { create(:vulnerabilities_finding, project: project, pipeline: pipeline) } + let_it_be(:tracked_context) do + create(:security_project_tracked_context, project: project, context_name: 'main', context_type: :branch, + state: :tracked) + end + + it 'updates the finding with the tracked context' do + expect { create_vulnerability }.to change { + finding.reload.security_project_tracked_context_id + }.from(nil).to(tracked_context.id) + end + + it 'creates a vulnerability with the tracked context set' do + create_vulnerability + + vulnerability = project.vulnerabilities.last + expect(vulnerability.finding.security_project_tracked_context_id).to eq(tracked_context.id) + end + end + + context 'when no tracked context exists for the finding pipeline' do + let_it_be(:pipeline) { create(:ci_pipeline, project: project, ref: 'unknown-branch') } + let(:finding) { create(:vulnerabilities_finding, project: project, pipeline: pipeline) } + + it 'does not update the finding with a tracked context' do + expect { create_vulnerability }.not_to change { finding.reload.security_project_tracked_context_id } + end + + it 'still creates the vulnerability' do + expect { create_vulnerability }.to change { project.vulnerabilities.count }.by(1) + end + + it 'logs a warning' do + allow(::Gitlab::AppLogger).to receive(:warn) + + create_vulnerability + + expect(::Gitlab::AppLogger).to have_received(:warn).with( + hash_including( + message: "Failed to find tracked context for vulnerability finding", + project_id: project.id + ) + ) + end + end + + context 'when finding has no pipeline' do + let(:finding) { create(:vulnerabilities_finding, project: project, pipeline: nil) } + + it 'does not update the finding with a tracked context' do + expect { create_vulnerability }.not_to change { finding.reload.security_project_tracked_context_id } + end + + it 'still creates the vulnerability' do + expect { create_vulnerability }.to change { project.vulnerabilities.count }.by(1) + end + end + + context 'when finding pipeline is a tag' do + let_it_be(:tag_pipeline) { create(:ci_pipeline, project: project, ref: 'v1.0.0', tag: true) } + let(:finding) { create(:vulnerabilities_finding, project: project, pipeline: tag_pipeline) } + let_it_be(:tag_context) do + create(:security_project_tracked_context, project: project, context_name: 'v1.0.0', context_type: :tag, + state: :tracked) + end + + it 'finds and updates with the tag context' do + expect { create_vulnerability }.to change { + finding.reload.security_project_tracked_context_id + }.from(nil).to(tag_context.id) + end + end + + context 'when multiple tracked contexts exist for the same ref' do + let_it_be(:pipeline) { create(:ci_pipeline, project: project, ref: 'main') } + let(:finding) { create(:vulnerabilities_finding, project: project, pipeline: pipeline) } + let_it_be(:older_context) do + create(:security_project_tracked_context, project: project, context_name: 'main', context_type: :branch, + state: :tracked, created_at: 1.day.ago) + end + + let_it_be(:newer_context) do + create(:security_project_tracked_context, project: project, context_name: 'main', context_type: :branch, + state: :tracked, created_at: Time.current) + end + + it 'uses the most recently created context' do + expect { create_vulnerability }.to change { + finding.reload.security_project_tracked_context_id + }.from(nil).to(newer_context.id) + end + end + + context 'when tracked context lookup is memoized' do + let_it_be(:pipeline) { create(:ci_pipeline, project: project, ref: 'main') } + let(:finding) { create(:vulnerabilities_finding, project: project, pipeline: pipeline) } + let_it_be(:tracked_context) do + create(:security_project_tracked_context, project: project, context_name: 'main', context_type: :branch, + state: :tracked) + end + + it 'memoizes the tracked context lookup by finding id' do + service = described_class.new(project, user, finding_id: finding_id) + allow(Security::ProjectTrackedContextsFinder).to receive(:new).and_call_original + + service.execute + + expect(Security::ProjectTrackedContextsFinder).to have_received(:new).at_least(:once) + end + end + end + + describe 'integration with vulnerability creation' do + let_it_be(:pipeline) { create(:ci_pipeline, project: project, ref: 'main') } + let(:finding) { create(:vulnerabilities_finding, project: project, pipeline: pipeline) } + let_it_be(:tracked_context) do + create(:security_project_tracked_context, project: project, context_name: 'main', context_type: :branch, + state: :tracked) + end + + it 'creates vulnerability and updates finding in a transaction' do + expect { create_vulnerability }.to change { project.vulnerabilities.count }.by(1) + + vulnerability = project.vulnerabilities.last + expect(vulnerability.finding.security_project_tracked_context_id).to eq(tracked_context.id) + end + + it 'does not create vulnerability if finding update fails' do + allow_next_instance_of(Vulnerabilities::Finding) do |instance| + allow(instance).to receive(:update_column).and_raise(ActiveRecord::RecordNotFound) + end + + expect { create_vulnerability }.to raise_error(ActiveRecord::Rollback) + expect(project.vulnerabilities.count).to eq(0) + end + end + end +end