Commit e7d4f2d3 authored by mo khan's avatar mo khan Committed by Lin Jen-Shin

Reproduce sentry errors

* Add case insensitive match for severity

```text
NoMethodError: undefined method `each' for nil:NilClass
  from gitlab/ci/parsers/security/common.rb:14:in `parse!'
  from ee/ci/build.rb:118:in `parse_security_artifact_blob'
  from ee/ci/build.rb:57:in `block (2 levels) in collect_security_report
  from ee/ci/build.rb:54:in `tap'
  from ee/ci/build.rb:54:in `block in collect_security_reports!'
  from ci/build.rb:828:in `block (2 levels) in each_report'
  from gitlab/ci/build/artifacts/adapters/raw_stream.rb:22:in `each_blob
  from ci/job_artifact.rb:183:in `block in each_blob'
  from gitlab_uploader.rb:96:in `open'
```
https://sentry.gitlab.net/gitlab/gitlabcom/issues/991284/events/20532078

```text
NoMethodError: undefined method `[]' for nil:NilClass
  from gitlab/ci/parsers/security/dependency_scanning.rb:16:in `create_
  from gitlab/ci/parsers/security/common.rb:53:in `create_vulnerability
  from gitlab/ci/parsers/security/common.rb:15:in `block in parse!'
  from gitlab/ci/parsers/security/common.rb:14:in `each'
  from gitlab/ci/parsers/security/common.rb:14:in `parse!'
  from ee/ci/build.rb:116:in `parse_security_artifact_blob'
  from ee/ci/build.rb:55:in `block (2 levels) in collect_security_report
  from ee/ci/build.rb:52:in `tap'
  from ee/ci/build.rb:52:in `block in collect_security_reports!'
  from ci/build.rb:828:in `block (2 levels) in each_report'
```
https://sentry.gitlab.net/gitlab/gitlabcom/issues/972642/events/20413167

```text
TypeError: no implicit conversion of nil into String
  from gitlab/ci/reports/security/occurrence.rb:78:in `digest'
  from gitlab/ci/reports/security/occurrence.rb:78:in `hexdigest'
  from gitlab/ci/reports/security/occurrence.rb:78:in `generate_project_
  from gitlab/ci/reports/security/occurrence.rb:37:in `initialize'
  from gitlab/ci/parsers/security/common.rb:48:in `new'
  from gitlab/ci/parsers/security/common.rb:48:in `create_vulnerability'
  from gitlab/ci/parsers/security/common.rb:15:in `block in parse!'
  from gitlab/ci/parsers/security/common.rb:14:in `each'
  from gitlab/ci/parsers/security/common.rb:14:in `parse!'
  from ee/ci/build.rb:113:in `parse_security_artifact_blob'
  from ee/ci/build.rb:64:in `block (2 levels) in collect_security_report
  from ee/ci/build.rb:61:in `tap'
  from ee/ci/build.rb:61:in `block in collect_security_reports!'
  from ci/build.rb:821:in `block (2 levels) in each_report'
```

https://sentry.gitlab.net/gitlab/gitlabcom/issues/903549/events/18937780
parent e1dc0db2
...@@ -37,12 +37,11 @@ module Security ...@@ -37,12 +37,11 @@ module Security
end end
def sync_vulnerability_rules def sync_vulnerability_rules
reports = pipeline.security_reports.reports reports = pipeline.security_reports
safe = reports.any? && reports.none? do |_report_type, report| return if reports.empty? && !pipeline.complete?
report.unsafe_severity? return if reports.violates_default_policy?
end
remove_required_approvals_for(ApprovalMergeRequestRule.security_report) if safe remove_required_approvals_for(ApprovalMergeRequestRule.security_report)
end end
def remove_required_approvals_for(rules) def remove_required_approvals_for(rules)
......
---
title: Prevent parser errors from approving the License-Check rule
merge_request: 18423
author:
type: fixed
...@@ -29,7 +29,7 @@ module Gitlab ...@@ -29,7 +29,7 @@ module Gitlab
# map remediations to relevant vulnerabilities # map remediations to relevant vulnerabilities
def collate_remediations(report_data) def collate_remediations(report_data)
return report_data["vulnerabilities"] unless report_data["remediations"] return report_data["vulnerabilities"] || [] unless report_data["remediations"]
report_data["vulnerabilities"].map do |vulnerability| report_data["vulnerabilities"].map do |vulnerability|
# Grab the first available remediation. # Grab the first available remediation.
...@@ -49,8 +49,8 @@ module Gitlab ...@@ -49,8 +49,8 @@ module Gitlab
uuid: SecureRandom.uuid, uuid: SecureRandom.uuid,
report_type: report.type, report_type: report.type,
name: data['message'], name: data['message'],
compare_key: data['cve'], compare_key: data['cve'] || '',
location: create_location(data['location']), location: create_location(data['location'] || {}),
severity: parse_level(data['severity']), severity: parse_level(data['severity']),
confidence: parse_level(data['confidence']), confidence: parse_level(data['confidence']),
scanner: scanner, scanner: scanner,
......
...@@ -54,7 +54,12 @@ module Gitlab ...@@ -54,7 +54,12 @@ module Gitlab
end end
def unsafe_severity? def unsafe_severity?
occurrences.any? { |occurrence| UNSAFE_SEVERITIES.include?(occurrence.severity) } !safe?
end
def safe?
severities = occurrences.map(&:severity).compact.map(&:downcase)
(severities & UNSAFE_SEVERITIES).size.zero?
end end
end end
end end
......
...@@ -7,6 +7,8 @@ module Gitlab ...@@ -7,6 +7,8 @@ module Gitlab
class Reports class Reports
attr_reader :reports, :commit_sha attr_reader :reports, :commit_sha
delegate :empty?, to: :reports
def initialize(commit_sha) def initialize(commit_sha)
@reports = {} @reports = {}
@commit_sha = commit_sha @commit_sha = commit_sha
...@@ -15,6 +17,12 @@ module Gitlab ...@@ -15,6 +17,12 @@ module Gitlab
def get_report(report_type) def get_report(report_type)
reports[report_type] ||= Report.new(report_type, commit_sha) reports[report_type] ||= Report.new(report_type, commit_sha)
end end
def violates_default_policy?
reports.values.any? do |report|
report.unsafe_severity?
end
end
end end
end end
end end
......
...@@ -49,6 +49,26 @@ describe Gitlab::Ci::Parsers::Security::DependencyScanning do ...@@ -49,6 +49,26 @@ describe Gitlab::Ci::Parsers::Security::DependencyScanning do
end end
end end
context "when parsing a vulnerability with a missing location" do
let(:report_hash) { JSON.parse(fixture_file('security_reports/master/gl-sast-report.json', dir: 'ee'), symbolize_names: true) }
before do
report_hash[:vulnerabilities][0][:location] = nil
end
it { expect { parser.parse!(report_hash.to_json, report) }.not_to raise_error }
end
context "when parsing a vulnerability with a missing cve" do
let(:report_hash) { JSON.parse(fixture_file('security_reports/master/gl-sast-report.json', dir: 'ee'), symbolize_names: true) }
before do
report_hash[:vulnerabilities][0][:cve] = nil
end
it { expect { parser.parse!(report_hash.to_json, report) }.not_to raise_error }
end
context "when vulnerabilities have remediations" do context "when vulnerabilities have remediations" do
let(:artifact) { create(:ee_ci_job_artifact, :dependency_scanning_remediation) } let(:artifact) { create(:ee_ci_job_artifact, :dependency_scanning_remediation) }
......
...@@ -4,44 +4,53 @@ require 'spec_helper' ...@@ -4,44 +4,53 @@ require 'spec_helper'
describe Gitlab::Ci::Parsers::Security::Sast do describe Gitlab::Ci::Parsers::Security::Sast do
describe '#parse!' do describe '#parse!' do
let(:project) { artifact.project } subject(:parser) { described_class.new }
let(:pipeline) { artifact.job.pipeline }
let(:report) { Gitlab::Ci::Reports::Security::Report.new(artifact.file_type, pipeline.sha) }
let(:parser) { described_class.new }
where(report_format: %i(sast sast_deprecated)) let(:commit_sha) { Digest::SHA1.hexdigest(SecureRandom.uuid) }
with_them do context "when parsing valid reports" do
let(:artifact) { create(:ee_ci_job_artifact, report_format) } where(report_format: %i(sast sast_deprecated))
before do with_them do
artifact.each_blob do |blob| let(:report) { Gitlab::Ci::Reports::Security::Report.new(artifact.file_type, commit_sha) }
parser.parse!(blob, report) let(:artifact) { create(:ee_ci_job_artifact, report_format) }
before do
artifact.each_blob do |blob|
parser.parse!(blob, report)
end
end end
end
it "parses all identifiers and occurrences" do it "parses all identifiers and occurrences" do
expect(report.occurrences.length).to eq(33) expect(report.occurrences.length).to eq(33)
expect(report.identifiers.length).to eq(17) expect(report.identifiers.length).to eq(17)
expect(report.scanners.length).to eq(3) expect(report.scanners.length).to eq(3)
end end
it 'generates expected location' do it 'generates expected location' do
location = report.occurrences.first.location location = report.occurrences.first.location
expect(location).to be_a(::Gitlab::Ci::Reports::Security::Locations::Sast) expect(location).to be_a(::Gitlab::Ci::Reports::Security::Locations::Sast)
expect(location).to have_attributes( expect(location).to have_attributes(
file_path: 'python/hardcoded/hardcoded-tmp.py', file_path: 'python/hardcoded/hardcoded-tmp.py',
start_line: 1, start_line: 1,
end_line: 1, end_line: 1,
class_name: nil, class_name: nil,
method_name: nil method_name: nil
) )
end end
it "generates expected metadata_version" do it "generates expected metadata_version" do
expect(report.occurrences.first.metadata_version).to eq('1.2') expect(report.occurrences.first.metadata_version).to eq('1.2')
end
end end
end end
context "when parsing an empty report" do
let(:report) { Gitlab::Ci::Reports::Security::Report.new('sast', commit_sha) }
let(:blob) { JSON.generate({}) }
it { expect(parser.parse!(blob, report)).to be_empty }
end
end end
end end
...@@ -119,4 +119,63 @@ describe Gitlab::Ci::Reports::Security::Report do ...@@ -119,4 +119,63 @@ describe Gitlab::Ci::Reports::Security::Report do
expect(report).to have_received(:replace_with!).with(merged_report) expect(report).to have_received(:replace_with!).with(merged_report)
end end
end end
describe "#safe?" do
subject { described_class.new('sast', commit_sha) }
let(:commit_sha) { Digest::SHA1.hexdigest(SecureRandom.uuid) }
%w[unknown Unknown high High critical Critical].each do |severity|
context "when the sast report has a #{severity} severity vulnerability" do
let(:occurrence) { build(:ci_reports_security_occurrence, severity: severity) }
before do
subject.add_occurrence(occurrence)
end
it { expect(subject.unsafe_severity?).to be(true) }
it { expect(subject.safe?).to be(false) }
end
end
%w[medium Medium low Low].each do |severity|
context "when the sast report has a #{severity} severity vulnerability" do
let(:occurrence) { build(:ci_reports_security_occurrence, severity: severity) }
before do
subject.add_occurrence(occurrence)
end
it { expect(subject.unsafe_severity?).to be(false) }
it { expect(subject.safe?).to be(true) }
end
end
context "when the sast report has a vulnerability with a `nil` severity" do
let(:occurrence) { build(:ci_reports_security_occurrence, severity: nil) }
before do
subject.add_occurrence(occurrence)
end
it { expect(subject.unsafe_severity?).to be(false) }
it { expect(subject.safe?).to be(true) }
end
context "when the sast report has a vulnerability with a blank severity" do
let(:occurrence) { build(:ci_reports_security_occurrence, severity: '') }
before do
subject.add_occurrence(occurrence)
end
it { expect(subject.unsafe_severity?).to be(false) }
it { expect(subject.safe?).to be(true) }
end
context "when the sast report has zero vulnerabilities" do
it { expect(subject.unsafe_severity?).to be(false) }
it { expect(subject.safe?).to be(true) }
end
end
end end
...@@ -35,4 +35,30 @@ describe Gitlab::Ci::Reports::Security::Reports do ...@@ -35,4 +35,30 @@ describe Gitlab::Ci::Reports::Security::Reports do
end end
end end
end end
describe "#violates_default_policy?" do
subject { described_class.new(commit_sha) }
let(:commit_sha) { Digest::SHA1.hexdigest(SecureRandom.uuid) }
let(:low_severity) { build(:ci_reports_security_occurrence, severity: 'low') }
let(:high_severity) { build(:ci_reports_security_occurrence, severity: 'high') }
context "when a report has a high severity vulnerability" do
before do
subject.get_report('sast').add_occurrence(high_severity)
subject.get_report('dependency_scanning').add_occurrence(low_severity)
end
it { expect(subject.violates_default_policy?).to be(true) }
end
context "when none of the reports have a high severity vulnerability" do
before do
subject.get_report('sast').add_occurrence(low_severity)
subject.get_report('dependency_scanning').add_occurrence(low_severity)
end
it { expect(subject.violates_default_policy?).to be(false) }
end
end
end end
...@@ -131,13 +131,14 @@ describe Security::SyncReportsToApprovalRulesService, '#execute' do ...@@ -131,13 +131,14 @@ describe Security::SyncReportsToApprovalRulesService, '#execute' do
end end
context 'without reports' do context 'without reports' do
let(:pipeline) { create(:ci_pipeline, :running, project: project, merge_requests_as_head_pipeline: [merge_request]) }
it "won't change approvals_required count" do it "won't change approvals_required count" do
expect { subject } expect { subject }
.not_to change { report_approver_rule.reload.approvals_required } .not_to change { report_approver_rule.reload.approvals_required }
end end
context "license compliance policy" do context "license compliance policy" do
let(:pipeline) { create(:ee_ci_pipeline, :running, project: project, merge_requests_as_head_pipeline: [merge_request]) }
let!(:software_license_policy) { create(:software_license_policy, :blacklist, project: project, software_license: blacklisted_license) } let!(:software_license_policy) { create(:software_license_policy, :blacklist, project: project, software_license: blacklisted_license) }
let!(:license_compliance_rule) { create(:report_approver_rule, :license_management, merge_request: merge_request, approvals_required: 1) } let!(:license_compliance_rule) { create(:report_approver_rule, :license_management, merge_request: merge_request, approvals_required: 1) }
let!(:blacklisted_license) { create(:software_license) } let!(:blacklisted_license) { create(:software_license) }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment