Merge branch '229974-migrate-to-SAST_EXCLUDED_ANALYZERS' into 'master'

Migrate from SAST_DEFAULT_ANALYZERS to SAST_EXCLUDED_ANALYZERS

See merge request gitlab-org/gitlab!50872

ee/app/services/security/ci_configuration/sast_parser_service.rb

@@ -5,6 +5,8 @@ module Security
     # This class parses SAST template file and .gitlab-ci.yml to populate default and current values into the JSON
     # read from app/validators/json_schemas/security_ci_configuration_schemas/sast_ui_schema.json
     class SastParserService < ::BaseService
+      include Gitlab::Utils::StrongMemoize
+
       SAST_UI_SCHEMA_PATH = 'app/validators/json_schemas/security_ci_configuration_schemas/sast_ui_schema.json'
 
       def initialize(project)
@@ -60,19 +62,35 @@ module Security
 
       def populate_current_value_for_analyzers(result)
         result[:analyzers].each do |analyzer|
-          analyzer[:enabled] = sast_default_analyzers.include?(analyzer[:name])
+          analyzer[:enabled] = analyzer_enabled?(analyzer[:name])
           populate_current_value_for(analyzer, :variables)
         end
       end
 
-      def sast_template_attributes
-        @sast_template_attributes ||= build_sast_attributes(sast_template_content)
+      def analyzer_enabled?(analyzer_name)
+        # Unless explicitly listed in the excluded analyzers, consider it enabled
+        sast_excluded_analyzers.exclude?(analyzer_name)
+      end
+
+      def sast_excluded_analyzers
+        strong_memoize(:sast_excluded_analyzers) do
+          all_analyzers = Security::CiConfiguration::SastBuildActions::SAST_DEFAULT_ANALYZERS.split(', ') rescue []
+          enabled_analyzers = sast_default_analyzers.split(',').map(&:strip) rescue []
+
+          excluded_analyzers = gitlab_ci_yml_attributes["SAST_EXCLUDED_ANALYZERS"] || sast_template_attributes["SAST_EXCLUDED_ANALYZERS"]
+          excluded_analyzers = excluded_analyzers.split(',').map(&:strip) rescue []
+          ((all_analyzers - enabled_analyzers) + excluded_analyzers).uniq
+        end
       end
 
       def sast_default_analyzers
         @sast_default_analyzers ||= gitlab_ci_yml_attributes["SAST_DEFAULT_ANALYZERS"] || sast_template_attributes["SAST_DEFAULT_ANALYZERS"]
       end
 
+      def sast_template_attributes
+        @sast_template_attributes ||= build_sast_attributes(sast_template_content)
+      end
+
       def gitlab_ci_yml_attributes
         @gitlab_ci_yml_attributes ||= begin
           config_content = @project.repository.blob_data_at(@project.repository.root_ref_sha, ci_config_file)

ee/changelogs/unreleased/229974-migrate-to-SAST_EXCLUDED_ANALYZERS.yml

+---
+title: Add support for SAST_EXCLUDED_ANALYZERS
+merge_request: 50872
+author:
+type: changed

ee/lib/security/ci_configuration/sast_build_actions.rb

@@ -51,15 +51,15 @@ module Security
           &.flat_map {|a| a['variables'] }
           &.collect {|v| [v['field'], v[key]] }.to_h
 
-        analyzer_variables['SAST_DEFAULT_ANALYZERS'] = if key == 'value'
-                                                         config['analyzers']
-                                                           &.select {|a| a['enabled'] }
-                                                           &.collect {|a| a['name'] }
-                                                           &.sort
-                                                           &.join(', ')
-                                                       else
-                                                         SAST_DEFAULT_ANALYZERS
-                                                       end
+        analyzer_variables['SAST_EXCLUDED_ANALYZERS'] = if key == 'value'
+                                                          config['analyzers']
+                                                          &.reject {|a| a['enabled'] }
+                                                          &.collect {|a| a['name'] }
+                                                          &.sort
+                                                          &.join(', ')
+                                                        else
+                                                          ''
+                                                        end
 
         analyzer_variables
       end
@@ -155,7 +155,7 @@ module Security
           SAST_ANALYZER_IMAGE_TAG
           SAST_EXCLUDED_PATHS
           SEARCH_MAX_DEPTH
-          SAST_DEFAULT_ANALYZERS
+          SAST_EXCLUDED_ANALYZERS
           SAST_BRAKEMAN_LEVEL
           SAST_BANDIT_EXCLUDED_PATHS
           SAST_FLAWFINDER_LEVEL

ee/spec/lib/security/ci_configuration/sast_build_actions_spec.rb

@@ -140,12 +140,24 @@ RSpec.describe Security::CiConfiguration::SastBuildActions do
           expect(result.first[:content]).to eq(sast_yaml_with_no_variables_set_but_analyzers)
         end
 
+        context 'analyzers are disabled' do
+          let(:gitlab_ci_content) { existing_gitlab_ci_and_single_template_with_sast_and_default_stage }
+
+          subject(:result) { described_class.new(auto_devops_enabled, params_with_analyzer_info, gitlab_ci_content).generate }
+
+          it 'writes SAST_EXCLUDED_ANALYZERS' do
+            stub_const('Security::CiConfiguration::SastBuildActions::SAST_DEFAULT_ANALYZERS', 'bandit, brakeman, flawfinder')
+
+            expect(result.first[:content]).to eq(sast_yaml_with_no_variables_set_but_analyzers)
+          end
+        end
+
         context 'all analyzers are enabled' do
           let(:gitlab_ci_content) { existing_gitlab_ci_and_single_template_with_sast_and_default_stage }
 
           subject(:result) { described_class.new(auto_devops_enabled, params_with_all_analyzers_enabled, gitlab_ci_content).generate }
 
-          it 'does not write SAST_DEFAULT_ANALYZERS' do
+          it 'does not write SAST_DEFAULT_ANALYZERS or SAST_EXCLUDED_ANALYZERS' do
             stub_const('Security::CiConfiguration::SastBuildActions::SAST_DEFAULT_ANALYZERS', 'brakeman, flawfinder')
 
             expect(result.first[:content]).to eq(sast_yaml_with_no_variables_set)
@@ -335,7 +347,7 @@ RSpec.describe Security::CiConfiguration::SastBuildActions do
     - test
     sast:
       variables:
-        SAST_DEFAULT_ANALYZERS: brakeman, flawfinder
+        SAST_EXCLUDED_ANALYZERS: bandit
         SAST_BRAKEMAN_LEVEL: '2'
       stage: test
     include:

ee/spec/services/security/ci_configuration/sast_parser_service_spec.rb

@@ -13,6 +13,7 @@ RSpec.describe Security::CiConfiguration::SastParserService do
     let(:sast_pipeline_stage) { configuration['pipeline'][0] }
     let(:sast_search_max_depth) { configuration['pipeline'][1] }
     let(:brakeman) { configuration['analyzers'][0] }
+    let(:bandit) { configuration['analyzers'][1] }
     let(:sast_brakeman_level) { brakeman['variables'][0] }
 
     it 'parses the configuration for SAST' do
@@ -35,8 +36,27 @@ RSpec.describe Security::CiConfiguration::SastParserService do
           expect(sast_pipeline_stage['value']).to eql('our_custom_security_stage')
           expect(sast_search_max_depth['value']).to eql('8')
           expect(brakeman['enabled']).to be(false)
+          expect(bandit['enabled']).to be(true)
           expect(sast_brakeman_level['value']).to eql('2')
         end
+
+        context 'SAST_DEFAULT_ANALYZERS is set' do
+          it 'enables analyzers correctly' do
+            allow(project.repository).to receive(:blob_data_at).and_return(gitlab_ci_yml_default_analyzers_content)
+
+            expect(brakeman['enabled']).to be(false)
+            expect(bandit['enabled']).to be(true)
+          end
+        end
+
+        context 'SAST_EXCLUDED_ANALYZERS is set' do
+          it 'enables analyzers correctly' do
+            allow(project.repository).to receive(:blob_data_at).and_return(gitlab_ci_yml_excluded_analyzers_content)
+
+            expect(brakeman['enabled']).to be(false)
+            expect(bandit['enabled']).to be(true)
+          end
+        end
       end
 
       context 'when .gitlab-ci.yml is absent' do

lib/gitlab/ci/templates/Security/SAST.gitlab-ci.yml

@@ -10,6 +10,7 @@ variables:
   SECURE_ANALYZERS_PREFIX: "registry.gitlab.com/gitlab-org/security-products/analyzers"
 
   SAST_DEFAULT_ANALYZERS: "bandit, brakeman, gosec, spotbugs, flawfinder, phpcs-security-audit, security-code-scan, nodejs-scan, eslint, sobelow, pmd-apex, kubesec, mobsf"
+  SAST_EXCLUDED_ANALYZERS: ""
   SAST_EXCLUDED_PATHS: "spec, test, tests, tmp"
   SAST_ANALYZER_IMAGE_TAG: 2
   SCAN_KUBERNETES_MANIFESTS: "false"
@@ -44,6 +45,8 @@ bandit-sast:
   rules:
     - if: $SAST_DISABLED
       when: never
+    - if: $SAST_EXCLUDED_ANALYZERS =~ /bandit/
+      when: never
     - if: $CI_COMMIT_BRANCH &&
           $SAST_DEFAULT_ANALYZERS =~ /bandit/
       exists:
@@ -58,6 +61,8 @@ brakeman-sast:
   rules:
     - if: $SAST_DISABLED
       when: never
+    - if: $SAST_EXCLUDED_ANALYZERS =~ /brakeman/
+      when: never
     - if: $CI_COMMIT_BRANCH &&
           $SAST_DEFAULT_ANALYZERS =~ /brakeman/
       exists:
@@ -72,6 +77,8 @@ eslint-sast:
   rules:
     - if: $SAST_DISABLED
       when: never
+    - if: $SAST_EXCLUDED_ANALYZERS =~ /eslint/
+      when: never
     - if: $CI_COMMIT_BRANCH &&
           $SAST_DEFAULT_ANALYZERS =~ /eslint/
       exists:
@@ -90,6 +97,8 @@ flawfinder-sast:
   rules:
     - if: $SAST_DISABLED
       when: never
+    - if: $SAST_EXCLUDED_ANALYZERS =~ /flawfinder/
+      when: never
     - if: $CI_COMMIT_BRANCH &&
           $SAST_DEFAULT_ANALYZERS =~ /flawfinder/
       exists:
@@ -105,6 +114,8 @@ kubesec-sast:
   rules:
     - if: $SAST_DISABLED
       when: never
+    - if: $SAST_EXCLUDED_ANALYZERS =~ /kubesec/
+      when: never
     - if: $CI_COMMIT_BRANCH &&
           $SAST_DEFAULT_ANALYZERS =~ /kubesec/ &&
           $SCAN_KUBERNETES_MANIFESTS == 'true'
@@ -118,6 +129,8 @@ gosec-sast:
   rules:
     - if: $SAST_DISABLED
       when: never
+    - if: $SAST_EXCLUDED_ANALYZERS =~ /gosec/
+      when: never
     - if: $CI_COMMIT_BRANCH &&
           $SAST_DEFAULT_ANALYZERS =~ /gosec/
       exists:
@@ -136,6 +149,8 @@ mobsf-android-sast:
   rules:
     - if: $SAST_DISABLED
       when: never
+    - if: $SAST_EXCLUDED_ANALYZERS =~ /mobsf/
+      when: never
     - if: $CI_COMMIT_BRANCH &&
           $SAST_DEFAULT_ANALYZERS =~ /mobsf/ &&
           $SAST_EXPERIMENTAL_FEATURES == 'true'
@@ -155,6 +170,8 @@ mobsf-ios-sast:
   rules:
     - if: $SAST_DISABLED
       when: never
+    - if: $SAST_EXCLUDED_ANALYZERS =~ /mobsf/
+      when: never
     - if: $CI_COMMIT_BRANCH &&
           $SAST_DEFAULT_ANALYZERS =~ /mobsf/ &&
           $SAST_EXPERIMENTAL_FEATURES == 'true'
@@ -170,6 +187,8 @@ nodejs-scan-sast:
   rules:
     - if: $SAST_DISABLED
       when: never
+    - if: $SAST_EXCLUDED_ANALYZERS =~ /nodejs-scan/
+      when: never
     - if: $CI_COMMIT_BRANCH &&
           $SAST_DEFAULT_ANALYZERS =~ /nodejs-scan/
       exists:
@@ -184,6 +203,8 @@ phpcs-security-audit-sast:
   rules:
     - if: $SAST_DISABLED
       when: never
+    - if: $SAST_EXCLUDED_ANALYZERS =~ /phpcs-security-audit/
+      when: never
     - if: $CI_COMMIT_BRANCH &&
           $SAST_DEFAULT_ANALYZERS =~ /phpcs-security-audit/
       exists:
@@ -198,6 +219,8 @@ pmd-apex-sast:
   rules:
     - if: $SAST_DISABLED
       when: never
+    - if: $SAST_EXCLUDED_ANALYZERS =~ /pmd-apex/
+      when: never
     - if: $CI_COMMIT_BRANCH &&
           $SAST_DEFAULT_ANALYZERS =~ /pmd-apex/
       exists:
@@ -212,6 +235,8 @@ security-code-scan-sast:
   rules:
     - if: $SAST_DISABLED
       when: never
+    - if: $SAST_EXCLUDED_ANALYZERS =~ /security-code-scan/
+      when: never
     - if: $CI_COMMIT_BRANCH &&
           $SAST_DEFAULT_ANALYZERS =~ /security-code-scan/
       exists:
@@ -227,6 +252,8 @@ sobelow-sast:
   rules:
     - if: $SAST_DISABLED
       when: never
+    - if: $SAST_EXCLUDED_ANALYZERS =~ /sobelow/
+      when: never
     - if: $CI_COMMIT_BRANCH &&
           $SAST_DEFAULT_ANALYZERS =~ /sobelow/
       exists:
@@ -239,6 +266,8 @@ spotbugs-sast:
   variables:
     SAST_ANALYZER_IMAGE: "$SECURE_ANALYZERS_PREFIX/spotbugs:$SAST_ANALYZER_IMAGE_TAG"
   rules:
+    - if: $SAST_EXCLUDED_ANALYZERS =~ /spotbugs/
+      when: never
     - if: $SAST_DEFAULT_ANALYZERS =~ /mobsf/ &&
           $SAST_EXPERIMENTAL_FEATURES == 'true'
       exists:

spec/support/gitlab_stubs/gitlab_ci_for_sast.yml

@@ -4,7 +4,8 @@ include:
 variables:
   SECURE_ANALYZERS_PREFIX: "registry.gitlab.com/gitlab-org/security-products/analyzers2"
   SAST_EXCLUDED_PATHS: "spec, executables"
-  SAST_DEFAULT_ANALYZERS: "bandit, gosec"
+  SAST_DEFAULT_ANALYZERS: "bandit, brakeman"
+  SAST_EXCLUDED_ANALYZERS: "brakeman"
 
 stages:
   - our_custom_security_stage

spec/support/gitlab_stubs/gitlab_ci_for_sast_default_analyzers.yml

+include:
+  - template: SAST.gitlab-ci.yml
+
+variables:
+  SECURE_ANALYZERS_PREFIX: "registry.gitlab.com/gitlab-org/security-products/analyzers2"
+  SAST_EXCLUDED_PATHS: "spec, executables"
+  SAST_DEFAULT_ANALYZERS: "bandit, gosec"
+
+stages:
+  - our_custom_security_stage
+sast:
+  stage:  our_custom_security_stage
+  variables:
+    SEARCH_MAX_DEPTH: 8
+    SAST_BRAKEMAN_LEVEL: 2

spec/support/gitlab_stubs/gitlab_ci_for_sast_excluded_analyzers.yml

+include:
+  - template: SAST.gitlab-ci.yml
+
+variables:
+  SECURE_ANALYZERS_PREFIX: "registry.gitlab.com/gitlab-org/security-products/analyzers2"
+  SAST_EXCLUDED_PATHS: "spec, executables"
+  SAST_EXCLUDED_ANALYZERS: "brakeman"
+
+stages:
+  - our_custom_security_stage
+sast:
+  stage:  our_custom_security_stage
+  variables:
+    SEARCH_MAX_DEPTH: 8

spec/support/shared_contexts/read_ci_configuration_shared_context.rb

@@ -5,5 +5,13 @@ RSpec.shared_context 'read ci configuration for sast enabled project' do
     File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci_for_sast.yml'))
   end
 
+  let_it_be(:gitlab_ci_yml_default_analyzers_content) do
+    File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci_for_sast_default_analyzers.yml'))
+  end
+
+  let_it_be(:gitlab_ci_yml_excluded_analyzers_content) do
+    File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci_for_sast_excluded_analyzers.yml'))
+  end
+
   let_it_be(:project) { create(:project, :repository) }
 end