Commit 69821f27 authored by Kerri Miller's avatar Kerri Miller

Merge branch 'dup-vulns-across-analyzers' into 'master'

Add coverage for replacing SAST analyzers

See merge request gitlab-org/gitlab!84404
parents 449cb21a 4e19824f
......@@ -22,6 +22,16 @@ FactoryBot.define do
end
trait report_type do
options do
{
artifacts: {
reports: {
report_type => Ci::JobArtifact::DEFAULT_FILE_NAMES[report_type]
}
}
}
end
after(:build) do |build|
build.job_artifacts << build(:ee_ci_job_artifact, report_type, job: build)
end
......
......@@ -27,7 +27,7 @@ RSpec.describe StoreSecurityReportsWorker do
end
with_them do
let(:pipeline) { build(:ci_pipeline, project: build(:project, :repository, visibility)) if visibility }
let(:pipeline) { build(:ee_ci_pipeline, project: build(:project, :repository, visibility)) if visibility }
let(:expected_result) { [visibility, token_revocation_enabled, secret_detection_vulnerability_found] == [:public, true, true] }
before do
......@@ -45,11 +45,9 @@ RSpec.describe StoreSecurityReportsWorker do
describe '#perform' do
let(:group) { create(:group) }
let(:project) { create(:project, namespace: group) }
let(:pipeline) { create(:ci_pipeline, ref: 'master', project: project) }
let(:pipeline) { create(:ee_ci_pipeline, ref: 'master', project: project) }
before do
allow(Ci::Pipeline).to receive(:find).with(pipeline.id) { pipeline }
allow(::ScanSecurityReportSecretsWorker).to receive(:perform_async).and_return(nil)
allow_next_instance_of(described_class) do |store_scans_worker|
allow(store_scans_worker).to receive(:revoke_secret_detection_token?) { true }
......@@ -78,6 +76,95 @@ RSpec.describe StoreSecurityReportsWorker do
end
end
context 'when running SAST analyzers that produce duplicate vulnerabilities' do
where(vulnerability_finding_signatures_enabled: [true, false])
with_them do
context 'and properly dedupe vulnerabilities' do
let(:artifact_bandit1) { create(:ee_ci_job_artifact, :sast_bandit, job: bandit1_build) }
let(:bandit1_build) { create(:ci_build, :sast, :success, user: project.creator, pipeline: pipeline, project: project) }
let(:artifact_bandit2) { create(:ee_ci_job_artifact, :sast_bandit, job: bandit2_build) }
let(:artifact_semgrep) { create(:ee_ci_job_artifact, :sast_semgrep_for_bandit, job: semgrep_build) }
let(:pipeline2) { create(:ee_ci_pipeline, ref: 'master', project: project) }
let(:bandit2_build) { create(:ci_build, :sast, :success, user: project.creator, pipeline: pipeline2, project: project) }
let(:semgrep_build) { create(:ci_build, :sast, :success, user: project.creator, pipeline: pipeline2, project: project) }
before do
stub_licensed_features(
sast: true,
vulnerability_finding_signatures: vulnerability_finding_signatures_enabled
)
pipeline.update!(user: bandit1_build.user)
pipeline2.update!(user: bandit2_build.user)
end
it 'does not duplicate vulnerabilities' do
expect do
Security::StoreGroupedScansService.execute([artifact_bandit1])
end.to change { Security::Finding.count }.by(1)
.and change { Security::Scan.count }.by(1)
expect do
described_class.new.perform(pipeline.id)
end.to change { Vulnerabilities::Finding.count }.by(1)
.and change { Vulnerability.count }.by(1)
expect do
Security::StoreGroupedScansService.execute([artifact_bandit2, artifact_semgrep])
end.to change { Security::Finding.count }.by(2)
.and change { Security::Scan.count }.by(2)
expect do
described_class.new.perform(pipeline2.id)
end.to change { Vulnerabilities::Finding.count }.by(0)
.and change { Vulnerability.count }.by(0)
end
end
context 'and improperly dedupe vulnerabilities' do
let(:artifact_gosec1) { create(:ee_ci_job_artifact, :sast_gosec, job: gosec1_build) }
let(:gosec1_build) { create(:ci_build, :sast, :success, user: project.creator, pipeline: pipeline, project: project) }
let(:artifact_gosec2) { create(:ee_ci_job_artifact, :sast_gosec, job: gosec2_build) }
let(:artifact_semgrep) { create(:ee_ci_job_artifact, :sast_semgrep_for_gosec, job: semgrep_build) }
let(:pipeline2) { create(:ee_ci_pipeline, ref: 'master', project: project) }
let(:gosec2_build) { create(:ci_build, :sast, :success, user: project.creator, pipeline: pipeline2, project: project) }
let(:semgrep_build) { create(:ci_build, :sast, :success, user: project.creator, pipeline: pipeline2, project: project) }
before do
stub_licensed_features(
sast: true,
vulnerability_finding_signatures: vulnerability_finding_signatures_enabled
)
pipeline.update!(user: gosec1_build.user)
pipeline2.update!(user: gosec2_build.user)
end
it 'duplicates vulnerabilities' do
expect do
Security::StoreGroupedScansService.execute([artifact_gosec1])
end.to change { Security::Finding.count }.by(1)
.and change { Security::Scan.count }.by(1)
expect do
described_class.new.perform(pipeline.id)
end.to change { Vulnerabilities::Finding.count }.by(1)
.and change { Vulnerability.count }.by(1)
expect do
Security::StoreGroupedScansService.execute([artifact_gosec2, artifact_semgrep])
end.to change { Security::Finding.count }.by(2)
.and change { Security::Scan.count }.by(2)
expect do
described_class.new.perform(pipeline2.id)
end.to change { Vulnerabilities::Finding.count }.by(1)
.and change { Vulnerability.count }.by(1)
end
end
end
end
context "when security reports feature is not available" do
let(:default_branch) { pipeline.ref }
......
......@@ -302,6 +302,56 @@ FactoryBot.define do
end
end
# Bandit reports are correctly de-duplicated when ran in the same pipeline
# as a corresponding semgrep report.
# This report does not include signature tracking.
trait :sast_bandit 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-bandit.json'), 'application/json')
end
end
# Equivalent Semgrep report for :sast_bandit report.
# This report includes signature tracking.
trait :sast_semgrep_for_bandit 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-bandit.json'), 'application/json')
end
end
# Gosec reports are not correctly de-duplicated when ran in the same pipeline
# as a corresponding semgrep report.
# This report includes signature tracking.
trait :sast_gosec 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-gosec.json'), 'application/json')
end
end
# Equivalent Semgrep report for :sast_gosec report.
# This report includes signature tracking.
trait :sast_semgrep_for_gosec 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-gosec.json'), 'application/json')
end
end
trait :common_security_report do
file_format { :raw }
file_type { :dependency_scanning }
......
{
"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": "bandit",
"name": "Bandit"
},
"location": {
"file": "app/app.py",
"start_line": 39
},
"identifiers": [
{
"type": "bandit_test_id",
"name": "Bandit Test ID B506",
"value": "B506"
}
]
}
],
"scan": {
"scanner": {
"id": "bandit",
"name": "Bandit",
"url": "https://github.com/PyCQA/bandit",
"vendor": {
"name": "GitLab"
},
"version": "1.7.1"
},
"type": "sast",
"start_time": "2022-03-11T00:21:49",
"end_time": "2022-03-11T00:21:50",
"status": "success"
}
}
{
"version": "14.0.4",
"vulnerabilities": [
{
"id": "2e5656ff30e2e7cc93c36b4845c8a689ddc47fdbccf45d834c67442fbaa89be0",
"category": "sast",
"name": "Key Exchange without Entity Authentication",
"message": "Use of ssh InsecureIgnoreHostKey should be audited",
"description": "The software performs a key exchange with an actor without verifying the identity of that actor.",
"cve": "og.go:8:7: func foo() {\n8: \t_ = ssh.InsecureIgnoreHostKey()\n9: }\n:CWE-322",
"severity": "Medium",
"confidence": "High",
"raw_source_code_extract": "7: func foo() {\n8: \t_ = ssh.InsecureIgnoreHostKey()\n9: }\n",
"scanner": {
"id": "gosec",
"name": "Gosec"
},
"location": {
"file": "og.go",
"start_line": 8
},
"identifiers": [
{
"type": "gosec_rule_id",
"name": "Gosec Rule ID G106",
"value": "G106"
},
{
"type": "CWE",
"name": "CWE-322",
"value": "322",
"url": "https://cwe.mitre.org/data/definitions/322.html"
}
],
"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": "gosec",
"name": "Gosec",
"url": "https://github.com/securego/gosec",
"vendor": {
"name": "GitLab"
},
"version": "2.10.0"
},
"type": "sast",
"start_time": "2022-03-15T20:33:12",
"end_time": "2022-03-15T20:33:17",
"status": "success"
}
}
{
"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"
}
]
}
]
}
}
],
"scan": {
"scanner": {
"id": "semgrep",
"name": "Semgrep",
"url": "https://github.com/returntocorp/semgrep",
"vendor": {
"name": "GitLab"
},
"version": "0.82.0"
},
"type": "sast",
"start_time": "2022-03-11T18:48:16",
"end_time": "2022-03-11T18:48:22",
"status": "success"
}
}
{
"version": "14.0.4",
"vulnerabilities": [
{
"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"
},
"type": "sast",
"start_time": "2022-03-15T20:36:58",
"end_time": "2022-03-15T20:37:05",
"status": "success"
}
}
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