From 89df1aaa9340f0a359a99f703544a3732aa8b1c2 Mon Sep 17 00:00:00 2001 From: Hitesh Raghuvanshi Date: Wed, 17 Dec 2025 12:25:56 +0530 Subject: [PATCH 1/6] Added check for membership lock before SAST worker trigger EE: true --- ee/app/models/ee/vulnerability.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/ee/app/models/ee/vulnerability.rb b/ee/app/models/ee/vulnerability.rb index fbdb6fdb5589ce..5084b354414504 100644 --- a/ee/app/models/ee/vulnerability.rb +++ b/ee/app/models/ee/vulnerability.rb @@ -394,6 +394,19 @@ def trigger_false_positive_detection user = project.first_owner || author return unless Ability.allowed?(user, :duo_workflow, project) + # This is a temporary check to avoid errors in TriggerFalsePositiveDetectionWorkflowWorker while starting workflow + # Related issue which should remove this check https://gitlab.com/gitlab-org/gitlab/-/issues/584239 + if project.group&.membership_lock || project.root_ancestor&.membership_lock + ::Gitlab::AppLogger.info( + message: 'Project membership locked for SAST vulnerability', + project_id: project.id, + vulnerability_id: id, + group_lock: project.group&.membership_lock, + root_ancestor_lock: project.root_ancestor&.membership_lock + ) + return + end + run_after_commit_or_now do ::Vulnerabilities::TriggerFalsePositiveDetectionWorkflowWorker.perform_async(id) end -- GitLab From b23eb6815134a6ac0e7c8cbcada018acc3532c31 Mon Sep 17 00:00:00 2001 From: Hitesh Raghuvanshi Date: Wed, 17 Dec 2025 14:01:05 +0530 Subject: [PATCH 2/6] Added test cases --- ee/spec/models/ee/vulnerability_spec.rb | 132 ++++++++++++++++++++++++ 1 file changed, 132 insertions(+) diff --git a/ee/spec/models/ee/vulnerability_spec.rb b/ee/spec/models/ee/vulnerability_spec.rb index a709fc30364ca8..fd74117535c581 100644 --- a/ee/spec/models/ee/vulnerability_spec.rb +++ b/ee/spec/models/ee/vulnerability_spec.rb @@ -1624,6 +1624,138 @@ create(:vulnerability, :sast, :high, author: user, project: project) end end + + context 'when membership is locked' do + let_it_be(:group) { create(:group) } + let_it_be(:project_with_group) { create(:project, group: group, duo_sast_fp_detection_enabled: true) } + + before do + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?).with(anything, :duo_workflow, project_with_group).and_return(true) + end + + context 'when project group has membership_lock enabled' do + before do + group.update!(membership_lock: true) + end + + it 'does not trigger false positive detection workflow' do + expect(::Vulnerabilities::TriggerFalsePositiveDetectionWorkflowWorker) + .not_to receive(:perform_async) + + create(:vulnerability, :sast, :high, author: user, project: project_with_group) + end + + it 'logs the membership lock information' do + expect(::Gitlab::AppLogger).to receive(:info).with( + hash_including( + message: 'Project membership locked for SAST vulnerability', + project_id: project_with_group.id, + group_lock: true + ) + ) + + create(:vulnerability, :sast, :high, author: user, project: project_with_group) + end + end + + context 'when project root_ancestor has membership_lock enabled' do + let_it_be(:root_group) { create(:group) } + let_it_be(:subgroup) { create(:group, parent: root_group) } + let_it_be(:project_in_subgroup) { create(:project, group: subgroup, duo_sast_fp_detection_enabled: true) } + + before do + root_group.update!(membership_lock: true) + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?).with(anything, :duo_workflow, project_in_subgroup).and_return(true) + end + + it 'does not trigger false positive detection workflow' do + expect(::Vulnerabilities::TriggerFalsePositiveDetectionWorkflowWorker) + .not_to receive(:perform_async) + + create(:vulnerability, :sast, :critical, author: user, project: project_in_subgroup) + end + + it 'logs the membership lock information with root_ancestor_lock' do + expect(::Gitlab::AppLogger).to receive(:info).with( + hash_including( + message: 'Project membership locked for SAST vulnerability', + project_id: project_in_subgroup.id, + root_ancestor_lock: true + ) + ) + + create(:vulnerability, :sast, :critical, author: user, project: project_in_subgroup) + end + end + + context 'when both group and root_ancestor have membership_lock enabled' do + let_it_be(:root_group) { create(:group, membership_lock: true) } + let_it_be(:subgroup) { create(:group, parent: root_group, membership_lock: true) } + let_it_be(:project_in_subgroup) { create(:project, group: subgroup, duo_sast_fp_detection_enabled: true) } + + before do + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?).with(anything, :duo_workflow, project_in_subgroup).and_return(true) + end + + it 'does not trigger false positive detection workflow' do + expect(::Vulnerabilities::TriggerFalsePositiveDetectionWorkflowWorker) + .not_to receive(:perform_async) + + create(:vulnerability, :sast, :high, author: user, project: project_in_subgroup) + end + + it 'logs both membership locks' do + expect(::Gitlab::AppLogger).to receive(:info).with( + hash_including( + message: 'Project membership locked for SAST vulnerability', + project_id: project_in_subgroup.id, + group_lock: true, + root_ancestor_lock: true + ) + ) + + create(:vulnerability, :sast, :high, author: user, project: project_in_subgroup) + end + end + + context 'when membership_lock is disabled' do + before do + group.update!(membership_lock: false) + end + + it 'triggers false positive detection workflow normally' do + expect_next_instance_of(::Vulnerability) do |instance| + expect(instance).to receive(:run_after_commit_or_now).and_yield + end + expect(::Vulnerabilities::TriggerFalsePositiveDetectionWorkflowWorker) + .to receive(:perform_async).with(anything) + + create(:vulnerability, :sast, :critical, author: user, project: project_with_group) + end + end + + context 'when project has no group' do + let_it_be(:project_without_group) { create(:project, duo_sast_fp_detection_enabled: true) } + + before do + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?).with(anything, :duo_workflow, project_without_group).and_return(true) + end + + it 'triggers false positive detection workflow normally' do + expect_next_instance_of(::Vulnerability) do |instance| + expect(instance).to receive(:run_after_commit_or_now).and_yield + end + expect(::Vulnerabilities::TriggerFalsePositiveDetectionWorkflowWorker) + .to receive(:perform_async).with(anything) + + create(:vulnerability, :sast, :high, author: user, project: project_without_group) + end + end + end end end -- GitLab From acd9ed3314281cfac3ab4ec59f723f348dded25f Mon Sep 17 00:00:00 2001 From: Hitesh Raghuvanshi Date: Wed, 17 Dec 2025 18:21:37 +0530 Subject: [PATCH 3/6] Removed root ancestor check --- ee/app/models/ee/vulnerability.rb | 5 +- ee/spec/models/ee/vulnerability_spec.rb | 62 ------------------------- 2 files changed, 2 insertions(+), 65 deletions(-) diff --git a/ee/app/models/ee/vulnerability.rb b/ee/app/models/ee/vulnerability.rb index 5084b354414504..8dd78ded041078 100644 --- a/ee/app/models/ee/vulnerability.rb +++ b/ee/app/models/ee/vulnerability.rb @@ -396,13 +396,12 @@ def trigger_false_positive_detection # This is a temporary check to avoid errors in TriggerFalsePositiveDetectionWorkflowWorker while starting workflow # Related issue which should remove this check https://gitlab.com/gitlab-org/gitlab/-/issues/584239 - if project.group&.membership_lock || project.root_ancestor&.membership_lock + if project.group&.membership_lock ::Gitlab::AppLogger.info( message: 'Project membership locked for SAST vulnerability', project_id: project.id, vulnerability_id: id, - group_lock: project.group&.membership_lock, - root_ancestor_lock: project.root_ancestor&.membership_lock + group_lock: project.group&.membership_lock ) return end diff --git a/ee/spec/models/ee/vulnerability_spec.rb b/ee/spec/models/ee/vulnerability_spec.rb index fd74117535c581..d7ec018a5c4231 100644 --- a/ee/spec/models/ee/vulnerability_spec.rb +++ b/ee/spec/models/ee/vulnerability_spec.rb @@ -1659,68 +1659,6 @@ end end - context 'when project root_ancestor has membership_lock enabled' do - let_it_be(:root_group) { create(:group) } - let_it_be(:subgroup) { create(:group, parent: root_group) } - let_it_be(:project_in_subgroup) { create(:project, group: subgroup, duo_sast_fp_detection_enabled: true) } - - before do - root_group.update!(membership_lock: true) - allow(Ability).to receive(:allowed?).and_call_original - allow(Ability).to receive(:allowed?).with(anything, :duo_workflow, project_in_subgroup).and_return(true) - end - - it 'does not trigger false positive detection workflow' do - expect(::Vulnerabilities::TriggerFalsePositiveDetectionWorkflowWorker) - .not_to receive(:perform_async) - - create(:vulnerability, :sast, :critical, author: user, project: project_in_subgroup) - end - - it 'logs the membership lock information with root_ancestor_lock' do - expect(::Gitlab::AppLogger).to receive(:info).with( - hash_including( - message: 'Project membership locked for SAST vulnerability', - project_id: project_in_subgroup.id, - root_ancestor_lock: true - ) - ) - - create(:vulnerability, :sast, :critical, author: user, project: project_in_subgroup) - end - end - - context 'when both group and root_ancestor have membership_lock enabled' do - let_it_be(:root_group) { create(:group, membership_lock: true) } - let_it_be(:subgroup) { create(:group, parent: root_group, membership_lock: true) } - let_it_be(:project_in_subgroup) { create(:project, group: subgroup, duo_sast_fp_detection_enabled: true) } - - before do - allow(Ability).to receive(:allowed?).and_call_original - allow(Ability).to receive(:allowed?).with(anything, :duo_workflow, project_in_subgroup).and_return(true) - end - - it 'does not trigger false positive detection workflow' do - expect(::Vulnerabilities::TriggerFalsePositiveDetectionWorkflowWorker) - .not_to receive(:perform_async) - - create(:vulnerability, :sast, :high, author: user, project: project_in_subgroup) - end - - it 'logs both membership locks' do - expect(::Gitlab::AppLogger).to receive(:info).with( - hash_including( - message: 'Project membership locked for SAST vulnerability', - project_id: project_in_subgroup.id, - group_lock: true, - root_ancestor_lock: true - ) - ) - - create(:vulnerability, :sast, :high, author: user, project: project_in_subgroup) - end - end - context 'when membership_lock is disabled' do before do group.update!(membership_lock: false) -- GitLab From 932ef250228940f89378ff9e00dbec314e735e98 Mon Sep 17 00:00:00 2001 From: Hitesh Raghuvanshi Date: Thu, 18 Dec 2025 10:06:40 +0530 Subject: [PATCH 4/6] Adding fix for undercoverage job --- ee/spec/models/ee/vulnerability_spec.rb | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/ee/spec/models/ee/vulnerability_spec.rb b/ee/spec/models/ee/vulnerability_spec.rb index d7ec018a5c4231..1902d86d083fa9 100644 --- a/ee/spec/models/ee/vulnerability_spec.rb +++ b/ee/spec/models/ee/vulnerability_spec.rb @@ -1647,16 +1647,20 @@ end it 'logs the membership lock information' do - expect(::Gitlab::AppLogger).to receive(:info).with( + allow(::Gitlab::AppLogger).to receive(:info).and_call_original + + vulnerability = create(:vulnerability, :sast, :high, author: user, project: project_with_group) + + expect(::Gitlab::AppLogger).to have_received(:info).with( hash_including( message: 'Project membership locked for SAST vulnerability', project_id: project_with_group.id, - group_lock: true + group_lock: true, + vulnerability_id: vulnerability.id ) ) - - create(:vulnerability, :sast, :high, author: user, project: project_with_group) end + end context 'when membership_lock is disabled' do -- GitLab From 40a3f8ef3a636cd8a2a1efd81de0616d436d4918 Mon Sep 17 00:00:00 2001 From: Hitesh Raghuvanshi Date: Thu, 18 Dec 2025 10:33:35 +0530 Subject: [PATCH 5/6] Fixed rubocop --- ee/spec/models/ee/vulnerability_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/ee/spec/models/ee/vulnerability_spec.rb b/ee/spec/models/ee/vulnerability_spec.rb index 1902d86d083fa9..e76e7dfde12084 100644 --- a/ee/spec/models/ee/vulnerability_spec.rb +++ b/ee/spec/models/ee/vulnerability_spec.rb @@ -1660,7 +1660,6 @@ ) ) end - end context 'when membership_lock is disabled' do -- GitLab From b9dd5e6799aacc9dc54eec471a61b151044c51d8 Mon Sep 17 00:00:00 2001 From: Hitesh Raghuvanshi Date: Thu, 18 Dec 2025 11:58:52 +0530 Subject: [PATCH 6/6] Fixed undercoverage --- ee/spec/models/ee/vulnerability_spec.rb | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/ee/spec/models/ee/vulnerability_spec.rb b/ee/spec/models/ee/vulnerability_spec.rb index e76e7dfde12084..b862cf5769da08 100644 --- a/ee/spec/models/ee/vulnerability_spec.rb +++ b/ee/spec/models/ee/vulnerability_spec.rb @@ -1647,18 +1647,15 @@ end it 'logs the membership lock information' do - allow(::Gitlab::AppLogger).to receive(:info).and_call_original - - vulnerability = create(:vulnerability, :sast, :high, author: user, project: project_with_group) - - expect(::Gitlab::AppLogger).to have_received(:info).with( + expect(::Gitlab::AppLogger).to receive(:info).with( hash_including( message: 'Project membership locked for SAST vulnerability', project_id: project_with_group.id, - group_lock: true, - vulnerability_id: vulnerability.id + group_lock: true ) - ) + ).and_call_original + + create(:vulnerability, :sast, :high, author: user, project: project_with_group) end end -- GitLab