From fccc5c4bcd58d63a328c04c2682a4035c4c7a306 Mon Sep 17 00:00:00 2001 From: Harrison Peters Date: Tue, 21 Oct 2025 01:49:24 -0600 Subject: [PATCH 1/5] Update finding with tracked context through finder --- .../project_tracked_contexts_finder.rb | 57 +++++++++++++++++++ .../security/project_tracked_context.rb | 7 +++ .../vulnerabilities/create_service.rb | 24 ++++++++ 3 files changed, 88 insertions(+) create mode 100644 ee/app/finders/security/project_tracked_contexts_finder.rb 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 00000000000000..5934196e37871e --- /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 d1a21e3cdad3c1..aafcc75a0cad24 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 16e34d72e7f515..967900a1423c5a 100644 --- a/ee/app/services/vulnerabilities/create_service.rb +++ b/ee/app/services/vulnerabilities/create_service.rb @@ -34,6 +34,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 +78,29 @@ 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_id = derive_tracked_context_from_finding(finding) + return unless tracked_context_id + + 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 + tracked_context = Security::ProjectTrackedContextsFinder.new(project, finding.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 -- GitLab From dd6509d0a1a5c5b6a6b76a41767a8b12112bd7ee Mon Sep 17 00:00:00 2001 From: Harrison Peters Date: Tue, 21 Oct 2025 23:34:27 -0600 Subject: [PATCH 2/5] Add specs --- .../project_tracked_contexts_finder_spec.rb | 182 ++++++++++++++++++ .../create_service_tracked_context_spec.rb | 162 ++++++++++++++++ 2 files changed, 344 insertions(+) create mode 100644 ee/spec/finders/security/project_tracked_contexts_finder_spec.rb create mode 100644 ee/spec/services/vulnerabilities/create_service_tracked_context_spec.rb 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 00000000000000..e9d4140ed22f42 --- /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 00000000000000..b1a4ca3e45e347 --- /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(: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 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 -- GitLab From a70699d53fb051c9ae261533a2b4d31ae5be3f6b Mon Sep 17 00:00:00 2001 From: Harrison Peters Date: Wed, 22 Oct 2025 23:35:55 -0600 Subject: [PATCH 3/5] Apply 1 suggestion(s) to 1 file(s) Co-authored-by: DANGER_GITLAB_API_TOKEN --- .../vulnerabilities/create_service_tracked_context_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/spec/services/vulnerabilities/create_service_tracked_context_spec.rb b/ee/spec/services/vulnerabilities/create_service_tracked_context_spec.rb index b1a4ca3e45e347..9de6acb0a32759 100644 --- a/ee/spec/services/vulnerabilities/create_service_tracked_context_spec.rb +++ b/ee/spec/services/vulnerabilities/create_service_tracked_context_spec.rb @@ -8,7 +8,7 @@ end let_it_be(:user) { create(:user) } - let(:project) { create(:project) } + let_it_be(:project) { create(:project) } let(:finding) { create(:vulnerabilities_finding, project: project) } let(:finding_id) { finding.id } -- GitLab From a39656ae91580879d802d8ee22d0e01cd4ce3969 Mon Sep 17 00:00:00 2001 From: Harrison Peters Date: Thu, 23 Oct 2025 00:37:37 -0600 Subject: [PATCH 4/5] Add handling for findings with no pipelines --- .../vulnerabilities/create_service.rb | 36 +++++++++---------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/ee/app/services/vulnerabilities/create_service.rb b/ee/app/services/vulnerabilities/create_service.rb index 967900a1423c5a..31ac400e846f0e 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 @@ -79,15 +71,21 @@ def save_vulnerability(vulnerability, finding) end def update_finding_with_tracked_context(finding) - tracked_context_id = derive_tracked_context_from_finding(finding) - return unless tracked_context_id + tracked_context = if @tracked_context_id.present? + ::Security::ProjectTrackedContext.find(@tracked_context_id) + else + derive_tracked_context_from_finding(finding) + end - finding.update_column(:security_project_tracked_context_id, tracked_context_id) + 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 - tracked_context = Security::ProjectTrackedContextsFinder.new(project, finding.pipeline).by_pipeline + pipeline = finding.latest_finding_pipeline + tracked_context = Security::ProjectTrackedContextsFinder.new(@project, pipeline).by_pipeline unless tracked_context.present? ::Gitlab::AppLogger.warn( -- GitLab From 394e20cd1e93705f7eb79d0b96b2f5bdedf9cc98 Mon Sep 17 00:00:00 2001 From: Harrison Peters Date: Thu, 23 Oct 2025 00:39:14 -0600 Subject: [PATCH 5/5] Add handling for findings with no pipelines --- .../vulnerabilities/create_service_tracked_context_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/spec/services/vulnerabilities/create_service_tracked_context_spec.rb b/ee/spec/services/vulnerabilities/create_service_tracked_context_spec.rb index 9de6acb0a32759..178194334063da 100644 --- a/ee/spec/services/vulnerabilities/create_service_tracked_context_spec.rb +++ b/ee/spec/services/vulnerabilities/create_service_tracked_context_spec.rb @@ -15,7 +15,7 @@ subject(:create_vulnerability) { described_class.new(project, user, finding_id: finding_id).execute } context 'with an authorized user' do - before do + before_all do project.add_maintainer(user) end -- GitLab