From 3d680c141e7734aa559b6704dfe81fb3fea75154 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Fri, 7 Oct 2022 18:09:03 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- config/sidekiq_queues.yml | 2 + doc/development/integrations/secure.md | 25 +++- lib/gitlab/ci/parsers/security/common.rb | 17 ++- lib/gitlab/ci/reports/security/report.rb | 4 + lib/gitlab/ci/reports/security/scanner.rb | 5 +- spec/factories/ci/job_artifacts.rb | 12 ++ .../gl-sast-report-semgrep-for-gosec.json | 7 + ...-report-semgrep-for-multiple-findings.json | 134 ++++++++++++++++++ .../gitlab/ci/parsers/security/sast_spec.rb | 17 ++- .../gitlab/ci/reports/security/report_spec.rb | 18 +++ 10 files changed, 231 insertions(+), 10 deletions(-) create mode 100644 spec/fixtures/security_reports/master/gl-sast-report-semgrep-for-multiple-findings.json diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index aff05cb62b9..b1da557014a 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -499,6 +499,8 @@ - 1 - - upload_checksum - 1 +- - vulnerabilities_mark_dropped_as_resolved + - 1 - - vulnerabilities_statistics_adjustment - 1 - - vulnerability_exports_export diff --git a/doc/development/integrations/secure.md b/doc/development/integrations/secure.md index 2f10d375f84..8d3ce0b7927 100644 --- a/doc/development/integrations/secure.md +++ b/doc/development/integrations/secure.md @@ -434,10 +434,29 @@ The value of the `category` field matches the report type: - `sast` - `dast` -##### Scanner +##### Scan -The `scanner` field is an object that embeds a human-readable `name` and a technical `id`. -The `id` should not collide with any other scanner another integrator would provide. +The `scan` field is an object that embeds meta information about the scan itself: the `analyzer` +and `scanner` that performed the scan, the `start_time` and `end_time` the scan executed, +and `status` of the scan (either "success" or "failure"). + +Both the `analyzer` and `scanner` fields are objects that embeds a human-readable `name` and a technical `id`. +The `id` should not collide with any other analyzers or scanners another integrator would provide. + +##### Scan Primary Identifiers + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/368284) in GitLab 15.4 [with a flag](../../administration/feature_flags.md) named `sec_mark_dropped_findings_as_resolved`. Disabled by default. + +The `scan.primary_identifiers` field is an optional field containing an array of +[primary identifiers](../../user/application_security/terminology/index.md#primary-identifier)). +This is an exhaustive list of all rulesets for which the analyzer performed the scan. + +Even when the [`Vulnerabilities`](#vulnerabilities) array for a given scan may be empty, this optional field +should contain the complete list of potential identifiers in order to inform the Rails application of which +rules were executed. + +When populated, the Rails application will automatically resolve previously detected vulnerabilities as no +longer relevant when their primary identifier is not included. ##### Name, message, and description diff --git a/lib/gitlab/ci/parsers/security/common.rb b/lib/gitlab/ci/parsers/security/common.rb index aa4fd644b2e..0c117d5f214 100644 --- a/lib/gitlab/ci/parsers/security/common.rb +++ b/lib/gitlab/ci/parsers/security/common.rb @@ -200,7 +200,22 @@ module Gitlab external_id: scanner_data['id'], name: scanner_data['name'], vendor: scanner_data.dig('vendor', 'name'), - version: scanner_data.dig('version'))) + version: scanner_data.dig('version'), + primary_identifiers: create_scan_primary_identifiers)) + end + + # TODO: primary_identifiers should be initialized on the + # scan itself but we do not currently parse scans through `MergeReportsService` + def create_scan_primary_identifiers + return unless scan_data.is_a?(Hash) && scan_data.dig('primary_identifiers') + + scan_data.dig('primary_identifiers').map do |identifier| + ::Gitlab::Ci::Reports::Security::Identifier.new( + external_type: identifier['type'], + external_id: identifier['value'], + name: identifier['name'], + url: identifier['url']) + end end def create_identifiers(identifiers) diff --git a/lib/gitlab/ci/reports/security/report.rb b/lib/gitlab/ci/reports/security/report.rb index 70f2919d38d..54b21da5436 100644 --- a/lib/gitlab/ci/reports/security/report.rb +++ b/lib/gitlab/ci/reports/security/report.rb @@ -69,6 +69,10 @@ module Gitlab replace_with!(::Security::MergeReportsService.new(self, other).execute) end + def primary_identifiers + scanners.values.flat_map(&:primary_identifiers).compact + end + def primary_scanner scanners.first&.second end diff --git a/lib/gitlab/ci/reports/security/scanner.rb b/lib/gitlab/ci/reports/security/scanner.rb index 918df163ede..080ed3f834a 100644 --- a/lib/gitlab/ci/reports/security/scanner.rb +++ b/lib/gitlab/ci/reports/security/scanner.rb @@ -16,15 +16,16 @@ module Gitlab "semgrep" => 2 }.freeze - attr_accessor :external_id, :name, :vendor, :version + attr_accessor :external_id, :name, :vendor, :version, :primary_identifiers alias_method :key, :external_id - def initialize(external_id:, name:, vendor:, version:) + def initialize(external_id:, name:, vendor:, version:, primary_identifiers: nil) @external_id = external_id @name = name @vendor = vendor @version = version + @primary_identifiers = primary_identifiers end def to_hash diff --git a/spec/factories/ci/job_artifacts.rb b/spec/factories/ci/job_artifacts.rb index f8b964cf8e0..20306fd8bdc 100644 --- a/spec/factories/ci/job_artifacts.rb +++ b/spec/factories/ci/job_artifacts.rb @@ -352,6 +352,18 @@ FactoryBot.define do end end + # Equivalent Semgrep report for combined :sast_bandit and :sast_gosec reports. + # This report includes signature tracking. + trait :sast_semgrep_for_multiple_findings do + file_type { :sast } + file_format { :raw } + + after(:build) do |artifact, _| + artifact.file = fixture_file_upload( + Rails.root.join('spec/fixtures/security_reports/master/gl-sast-report-semgrep-for-multiple-findings.json'), 'application/json') + end + end + trait :common_security_report do file_format { :raw } file_type { :dependency_scanning } diff --git a/spec/fixtures/security_reports/master/gl-sast-report-semgrep-for-gosec.json b/spec/fixtures/security_reports/master/gl-sast-report-semgrep-for-gosec.json index f01d26a69c9..8fa85c30b56 100644 --- a/spec/fixtures/security_reports/master/gl-sast-report-semgrep-for-gosec.json +++ b/spec/fixtures/security_reports/master/gl-sast-report-semgrep-for-gosec.json @@ -62,6 +62,13 @@ }, "version": "0.82.0" }, + "primary_identifiers": [ + { + "type": "semgrep_id", + "name": "gosec.G106-1", + "value": "gosec.G106-1" + } + ], "type": "sast", "start_time": "2022-03-15T20:36:58", "end_time": "2022-03-15T20:37:05", diff --git a/spec/fixtures/security_reports/master/gl-sast-report-semgrep-for-multiple-findings.json b/spec/fixtures/security_reports/master/gl-sast-report-semgrep-for-multiple-findings.json new file mode 100644 index 00000000000..cbdfdb86f6b --- /dev/null +++ b/spec/fixtures/security_reports/master/gl-sast-report-semgrep-for-multiple-findings.json @@ -0,0 +1,134 @@ +{ + "version": "14.0.4", + "vulnerabilities": [ + { + "id": "985a5666dcae22adef5ac12f8a8a2dacf9b9b481ae5d87cd0ac1712b0fd64864", + "category": "sast", + "message": "Deserialization of Untrusted Data", + "description": "Avoid using `load()`. `PyYAML.load` can create arbitrary Python\nobjects. A malicious actor could exploit this to run arbitrary\ncode. Use `safe_load()` instead.\n", + "cve": "", + "severity": "Critical", + "scanner": { + "id": "semgrep", + "name": "Semgrep" + }, + "location": { + "file": "app/app.py", + "start_line": 39 + }, + "identifiers": [ + { + "type": "semgrep_id", + "name": "bandit.B506", + "value": "bandit.B506", + "url": "https://semgrep.dev/r/gitlab.bandit.B506" + }, + { + "type": "cwe", + "name": "CWE-502", + "value": "502", + "url": "https://cwe.mitre.org/data/definitions/502.html" + }, + { + "type": "bandit_test_id", + "name": "Bandit Test ID B506", + "value": "B506" + } + ], + "tracking": { + "type": "source", + "items": [ + { + "file": "app/app.py", + "line_start": 39, + "line_end": 39, + "signatures": [ + { + "algorithm": "scope_offset", + "value": "app/app.py|yaml_hammer[0]:13" + } + ] + } + ] + } + }, + { + "id": "79f6537b7ec83c7717f5bd1a4f12645916caafefe2e4359148d889855505aa67", + "category": "sast", + "message": "Key Exchange without Entity Authentication", + "description": "Audit the use of ssh.InsecureIgnoreHostKey\n", + "cve": "", + "severity": "Medium", + "scanner": { + "id": "semgrep", + "name": "Semgrep" + }, + "location": { + "file": "og.go", + "start_line": 8 + }, + "identifiers": [ + { + "type": "semgrep_id", + "name": "gosec.G106-1", + "value": "gosec.G106-1" + }, + { + "type": "cwe", + "name": "CWE-322", + "value": "322", + "url": "https://cwe.mitre.org/data/definitions/322.html" + }, + { + "type": "gosec_rule_id", + "name": "Gosec Rule ID G106", + "value": "G106" + } + ], + "tracking": { + "type": "source", + "items": [ + { + "file": "og.go", + "line_start": 8, + "line_end": 8, + "signatures": [ + { + "algorithm": "scope_offset", + "value": "og.go|foo[0]:1" + } + ] + } + ] + } + } + ], + "scan": { + "scanner": { + "id": "semgrep", + "name": "Semgrep", + "url": "https://github.com/returntocorp/semgrep", + "vendor": { + "name": "GitLab" + }, + "version": "0.82.0" + }, + "primary_identifiers": [ + { + "type": "semgrep_id", + "name": "bandit.B506", + "value": "bandit.B506", + "url": "https://semgrep.dev/r/gitlab.bandit.B506" + }, + { + "type": "semgrep_id", + "name": "gosec.G106-1", + "value": "gosec.G106-1" + } + ], + "type": "sast", + "start_time": "2022-03-15T20:36:58", + "end_time": "2022-03-15T20:37:05", + "status": "success" + } +} \ No newline at end of file diff --git a/spec/lib/gitlab/ci/parsers/security/sast_spec.rb b/spec/lib/gitlab/ci/parsers/security/sast_spec.rb index 9c791317262..8a30d70c28c 100644 --- a/spec/lib/gitlab/ci/parsers/security/sast_spec.rb +++ b/spec/lib/gitlab/ci/parsers/security/sast_spec.rb @@ -11,9 +11,12 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Sast do let(:created_at) { 2.weeks.ago } context "when passing valid report" do - where(:report_format, :report_version, :scanner_length, :finding_length, :identifier_length, :file_path, :line) do - :sast | '14.0.0' | 1 | 5 | 6 | 'groovy/src/main/java/com/gitlab/security_products/tests/App.groovy' | 47 + # rubocop: disable Layout/LineLength + where(:report_format, :report_version, :scanner_length, :finding_length, :identifier_length, :file_path, :start_line, :end_line, :primary_identifiers_length) do + :sast | '14.0.0' | 1 | 5 | 6 | 'groovy/src/main/java/com/gitlab/security_products/tests/App.groovy' | 47 | 47 | nil + :sast_semgrep_for_multiple_findings | '14.0.4' | 1 | 2 | 6 | 'app/app.py' | 39 | nil | 2 end + # rubocop: enable Layout/LineLength with_them do let(:report) do @@ -34,6 +37,12 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Sast do expect(report.findings.length).to eq(finding_length) expect(report.identifiers.length).to eq(identifier_length) expect(report.scanners.length).to eq(scanner_length) + + if primary_identifiers_length + expect( + report.scanners.each_value.first.primary_identifiers.length + ).to eq(primary_identifiers_length) + end end it 'generates expected location' do @@ -42,8 +51,8 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Sast do expect(location).to be_a(::Gitlab::Ci::Reports::Security::Locations::Sast) expect(location).to have_attributes( file_path: file_path, - end_line: line, - start_line: line + end_line: end_line, + start_line: start_line ) end diff --git a/spec/lib/gitlab/ci/reports/security/report_spec.rb b/spec/lib/gitlab/ci/reports/security/report_spec.rb index ab0efb90901..d7f967f1c55 100644 --- a/spec/lib/gitlab/ci/reports/security/report_spec.rb +++ b/spec/lib/gitlab/ci/reports/security/report_spec.rb @@ -140,6 +140,24 @@ RSpec.describe Gitlab::Ci::Reports::Security::Report do it { is_expected.to eq(scanner_1) } end + describe '#primary_identifiers' do + it 'returns matching identifiers' do + scanner_with_identifiers = create( + :ci_reports_security_scanner, + external_id: 'external_id_1', + primary_identifiers: [create(:ci_reports_security_identifier, external_id: 'other_id', name: 'other_scanner')] + ) + scanner_without_identifiers = create( + :ci_reports_security_scanner, + external_id: 'external_id_2') + + report.add_scanner(scanner_with_identifiers) + report.add_scanner(scanner_without_identifiers) + + expect(report.primary_identifiers).to eq(scanner_with_identifiers.primary_identifiers) + end + end + describe '#add_error' do context 'when the message is not given' do it 'adds a new error to report with the generic error message' do