Security Approvals not required when scans removed in MR

Summary

Users can work around Security Approvals groups by removing scanning jobs from the CI config, in the merge request they submit.

Steps to reproduce

• create a project where SAST and Dependency Scanning (DS) is enabled in the master branch
• edit the project settings, and add a security approval group
• create a MR that removes SAST and DS from the CI configuration file

The source branch has no security scans but is not blocked, even though it may introduce no vulnerabilities (compared to the target branch, where the scans are configured).

Example Project

• SAST & DS removed, not blocked
• DS removed, not blocked
• SAST removed, blocked

NOTE: For some reasons the MR is blocked when SAST is removed, which is the expected behavior. That's possibly a side-effect of #37237 (closed).

What is the current bug behavior?

The MR is NOT blocked. The approval widget says No approval required, even though there's a Vulnerability-Check group.

What is the expected correct behavior?

The MR is BLOCKED. Ideally, the error is explicit about the misconfiguration of the security scanning, in the source branch contributors are submitting.

Relevant logs and/or screenshots

Possible fixes

We can compare report types for pipeline from MR and target branch. If there is at least one report type missing in the MR pipeline we will enforce the policy.

diff --git a/lib/gitlab/ci/reports/security/reports.rb b/lib/gitlab/ci/reports/security/reports.rb
index b6372349f68a..2e4c626ed878 100644
--- a/lib/gitlab/ci/reports/security/reports.rb
+++ b/lib/gitlab/ci/reports/security/reports.rb
@@ -23,6 +23,8 @@ def findings
           end
 
           def violates_default_policy_against?(target_reports, vulnerabilities_allowed, severity_levels, vulnerability_states, report_types = [])
+            return true if scan_removed?(target_reports)
+
             unsafe_findings_count(target_reports, severity_levels, vulnerability_states, report_types) > vulnerabilities_allowed
           end
 
@@ -36,6 +38,10 @@ def unsafe_findings_count(target_reports, severity_levels, vulnerability_states,
             new_uuids = unsafe_findings_uuids(severity_levels, report_types) - target_reports&.unsafe_findings_uuids(severity_levels, report_types).to_a
             new_uuids.count
           end
+
+          def scan_removed?(target_reports)
+            (target_reports&.reports&.keys.to_a - reports.keys).any?
+          end
         end
       end
     end

Other ways to solve it?

Changing files related to CI should be restricted and always verified with CODEOWNERS file so this can be solved by properly setting this file.

What kind of issues we might have when we solve it with proposed solution?

• There is still an option to modify .gitlab-ci.yml file to ie. include empty report. Then technically we will have all needed security reports and approval will not be needed, although it should be as the security job was modified.

Implementation plan

• backend in /lib/gitlab/ci/reports/security/reports.rb implement new method scan_removed?(target_reports) to verify if any scan was removed, this method should compare all reports from pipelines between target and source sha,
• backend in /lib/gitlab/ci/reports/security/reports.rb modify violates_default_policy_against? method to return true when scan were removed from the pipeline configuration scan_removed?(target_reports),
• backend feature flag Add feature flag to be able to enable/disable this feature quickly,