Merge branch 'integration-token-revocation' into 'master'

Integrate RevocationAPI with BuildFinishedWorker

See merge request gitlab-org/gitlab!46729

ee/app/services/security/token_revocation_service.rb

@@ -36,7 +36,7 @@ module Security
         body: message,
         headers: {
          'Content-Type' => 'application/json',
-         'X-Token' => revocation_api_token
+         'Authorization' => revocation_api_token
         }
       )
     end
@@ -59,7 +59,7 @@ module Security
         token_types_url,
         headers: {
           'Content-Type' => 'application/json',
-          'X-Token' => revocation_api_token
+          'Authorization' => revocation_api_token
         }
       )
       raise RevocationFailedError, 'Failed to get revocation token types' unless response.success?

ee/app/workers/all_queues.yml

@@ -531,6 +531,14 @@
   :weight: 3
   :idempotent:
   :tags: []
+- :name: security_scans:scan_security_report_secrets
+  :feature_category: :static_application_security_testing
+  :has_external_dependencies: true
+  :urgency: :low
+  :resource_boundary: :cpu
+  :weight: 2
+  :idempotent: true
+  :tags: []
 - :name: security_scans:security_store_scans
   :feature_category: :static_application_security_testing
   :has_external_dependencies:

ee/app/workers/ee/build_finished_worker.rb

@@ -8,9 +8,22 @@ module EE
       # and `Namespace#namespace_statistics` will return stale data.
       ::Ci::Minutes::EmailNotificationService.new(build.project.reset).execute if ::Gitlab.com?
 
+      ScanSecurityReportSecretsWorker.perform_async(build.id) if revoke_secret_detection_token?(build)
       RequirementsManagement::ProcessRequirementsReportsWorker.perform_async(build.id)
 
       super
     end
+
+    private
+
+    def revoke_secret_detection_token?(build)
+      ::Gitlab.com? &&
+        ::Gitlab::CurrentSettings.secret_detection_token_revocation_enabled? &&
+        secret_detection_vulnerability_found?(build)
+    end
+
+    def secret_detection_vulnerability_found?(build)
+      build.pipeline.vulnerability_findings.secret_detection.any?
+    end
   end
 end

ee/app/workers/scan_security_report_secrets_worker.rb

+# frozen_string_literal: true
+
+# Worker for triggering events subject to secret_detection security reports
+#
+class ScanSecurityReportSecretsWorker # rubocop:disable Scalability/IdempotentWorker
+  include ApplicationWorker
+  include SecurityScansQueue
+
+  worker_resource_boundary :cpu
+
+  sidekiq_options retry: 20
+
+  worker_has_external_dependencies!
+  idempotent!
+
+  ScanSecurityReportSecretsWorkerError = Class.new(StandardError)
+
+  def perform(build_id)
+    build = Ci::Build.find_by_id(build_id)
+    return unless build
+
+    keys = revocable_keys(build)
+
+    if keys.present?
+      executed_result = Security::TokenRevocationService.new(revocable_keys: keys).execute
+
+      raise ScanSecurityReportSecretsWorkerError, executed_result[:message] if executed_result[:status] == :error
+    end
+  end
+
+  private
+
+  def revocable_keys(build)
+    vulnerability_findings = build.pipeline.vulnerability_findings.report_type(:secret_detection)
+
+    vulnerability_findings.map do |vulnerability_finding|
+      {
+        type: revocation_type(vulnerability_finding),
+        token: vulnerability_finding.metadata['raw_source_code_extract'],
+        location: vulnerability_finding.vulnerability.present.location_link
+      }
+    end
+  end
+
+  def revocation_type(vulnerability_finding)
+    identifier = vulnerability_finding.metadata['identifiers'].first
+    (identifier["type"] + '_' + identifier["value"].tr(' ', '_')).downcase
+  end
+end

ee/changelogs/unreleased/integration-token-revocation.yml

+---
+title: Integrate RevocationAPI with BuildFinishedWorker
+merge_request: 46729
+author:
+type: added

ee/spec/factories/vulnerabilities/findings.rb

@@ -156,6 +156,34 @@ FactoryBot.define do
       end
     end
 
+    trait :with_secret_detection do
+      after(:build) do |finding|
+        finding.severity = "critical"
+        finding.confidence = "unknown"
+        finding.report_type = "secret_detection"
+        finding.name = "AWS API key"
+        finding.metadata_version = "3.0"
+        finding.raw_metadata =
+          { category: "secret_detection",
+            name: "AWS API key",
+            message: "AWS API key",
+            description: "Amazon Web Services API key detected; please remove and revoke it if this is a leak.",
+            cve: "aws-key.py:fac8c3618ca3c0b55431402635743c0d6884016058f696be4a567c4183c66cfd:AWS",
+            severity: "Critical",
+            confidence: "Unknown",
+            raw_source_code_extract: "AKIALALEMEM35243O345",
+            scanner: { id: "gitleaks", name: "Gitleaks" },
+            location: { file: "aws-key.py",
+                        commit: { author: "Analyzer", sha: "d874aae969588eb718e1ed18aa0be73ea69b3539" },
+                        start_line: 5, end_line: 5 },
+            identifiers: [{ type: "gitleaks_rule_id", name: "Gitleaks rule ID AWS", value: "AWS" }] }.to_json
+      end
+
+      after(:create) do |finding|
+        create(:vulnerability, :detected, project: finding.project, findings: [finding])
+      end
+    end
+
     trait :with_remediation do
       after(:build) do |finding|
         raw_metadata = Gitlab::Json.parse(finding.raw_metadata)

ee/spec/workers/build_finished_worker_spec.rb

@@ -20,6 +20,34 @@ RSpec.describe BuildFinishedWorker do
     project.statistics || project.create_statistics(namespace: project.namespace)
   end
 
+  describe '#revoke_secret_detection_token?' do
+    using RSpec::Parameterized::TableSyntax
+
+    where(:dot_com, :token_revocation_enabled, :secret_detection_vulnerability_found, :expected_result) do
+      true  | true  | true  | true
+      true  | true  | false | false
+      true  | false | true  | false
+      true  | false | false | false
+      false | true  | true  | false
+      false | true  | false | false
+      false | false | true  | false
+      false | false | false | false
+    end
+
+    with_them do
+      before do
+        allow(Gitlab).to receive(:com?) { dot_com }
+        stub_application_setting(secret_detection_token_revocation_enabled: token_revocation_enabled)
+
+        allow_next_instance_of(described_class) do |build_finished_worker|
+          allow(build_finished_worker).to receive(:secret_detection_vulnerability_found?) { secret_detection_vulnerability_found }
+        end
+      end
+
+      specify { expect(described_class.new.send(:revoke_secret_detection_token?, build)).to eql(expected_result) }
+    end
+  end
+
   describe '#perform' do
     context 'when on .com' do
       before do
@@ -62,5 +90,33 @@ RSpec.describe BuildFinishedWorker do
 
       subject
     end
+
+    context 'when token revocation is enabled' do
+      before do
+        allow_next_instance_of(described_class) do |build_finished_worker|
+          allow(build_finished_worker).to receive(:revoke_secret_detection_token?) { true }
+        end
+      end
+
+      it 'scans security reports for token revocation' do
+        expect(ScanSecurityReportSecretsWorker).to receive(:perform_async)
+
+        subject
+      end
+    end
+
+    context 'when token revocation is disabled' do
+      before do
+        allow_next_instance_of(described_class) do |build_finished_worker|
+          allow(build_finished_worker).to receive(:revoke_secret_detection_token?) { false }
+        end
+      end
+
+      it 'does not scan security reports for token revocation' do
+        expect(ScanSecurityReportSecretsWorker).not_to receive(:perform_async)
+
+        subject
+      end
+    end
   end
 end

ee/spec/workers/scan_security_report_secrets_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe  ScanSecurityReportSecretsWorker do
+  let(:project) { create(:project, :repository) }
+  let(:pipeline) { create(:ci_pipeline, :success, project: project) }
+
+  let(:secret_detection_build) { create(:ci_build, :secret_detection, pipeline: pipeline) }
+  let(:file) { 'aws-key1.py' }
+  let(:api_key) { 'AKIALALEMEM35243O567' }
+  let(:identifier_type) { 'gitleaks_rule_id' }
+  let(:identifier_value) { 'AWS' }
+  let(:revocation_key_type) { 'gitleaks_rule_id_aws' }
+
+  let(:vulnerability) do
+    create(:vulnerabilities_finding, :with_secret_detection, pipelines: [pipeline], project: project)
+  end
+
+  subject(:worker) { described_class.new }
+
+  before do
+    vulnerability.update!(raw_metadata: { category: 'secret_detection',
+          raw_source_code_extract: api_key,
+          location: { file: file,
+                      start_line: 40, end_line: 45 },
+                      identifiers: [{ type: identifier_type, name: 'Gitleaks rule ID AWS', value: identifier_value }] }.to_json)
+  end
+
+  describe '#perform' do
+    include_examples 'an idempotent worker' do
+      let(:job_args) { [secret_detection_build.id] }
+
+      before do
+        allow_next_instance_of(Security::TokenRevocationService) do |revocation_service|
+          allow(revocation_service).to receive(:execute).and_return({ message: '', status: :success })
+        end
+      end
+
+      it 'executes the service' do
+        expect_next_instance_of(Security::TokenRevocationService) do |revocation_service|
+          expect(revocation_service).to receive(:execute).and_return({ message: '', status: :success })
+        end
+
+        worker.perform(secret_detection_build.id)
+      end
+    end
+
+    context 'with a failure in TokenRevocationService call' do
+      before do
+        allow_next_instance_of(Security::TokenRevocationService) do |revocation_service|
+          allow(revocation_service).to receive(:execute).and_return({ message: 'This is an error', status: :error })
+        end
+      end
+
+      it 'does not execute the service' do
+        expect { worker.perform(secret_detection_build.id) }.to raise_error('This is an error')
+      end
+    end
+  end
+
+  describe '#revocable_keys' do
+    it 'returns a list of revocable_keys' do
+      key = worker.send(:revocable_keys, secret_detection_build).first
+
+      expect(key[:type]).to eql(revocation_key_type)
+      expect(key[:token]).to eql(api_key)
+      expect(key[:location]).to include(file)
+    end
+  end
+end