Commit 2d168684 authored by Craig Smith's avatar Craig Smith

Replace scanned_resources_for_csv with object

Rather than converting the scanned resources array
to an array of OpenStructs, have reports return
an array of ScannedResources objects.
parent 354bfa4f
...@@ -30,14 +30,14 @@ module Projects ...@@ -30,14 +30,14 @@ module Projects
def render_csv def render_csv
CsvBuilders::SingleBatch.new( CsvBuilders::SingleBatch.new(
::Gitlab::Ci::Parsers::Security::ScannedResources.new.scanned_resources_for_csv(@scanned_resources), @scanned_resources,
{ {
'Method': 'request_method', 'Method': 'request_method',
'Scheme': 'scheme', 'Scheme': 'url_scheme',
'Host': 'host', 'Host': 'url_host',
'Port': 'port', 'Port': 'url_port',
'Path': 'path', 'Path': 'url_path',
'Query String': 'query_string' 'Query String': 'url_query'
} }
).render ).render
end end
......
...@@ -21,8 +21,8 @@ module Security ...@@ -21,8 +21,8 @@ module Security
scanned_resources = scanned_resources.first(@limit) if @limit scanned_resources = scanned_resources.first(@limit) if @limit
acc[type] = scanned_resources.map do |resource| acc[type] = scanned_resources.map do |resource|
{ {
'request_method' => resource['method'], 'request_method' => resource.request_method,
'url' => resource['url'] 'url' => resource.request_uri.to_s
} }
end end
end end
......
...@@ -11,7 +11,8 @@ module Gitlab ...@@ -11,7 +11,8 @@ module Gitlab
report_data = parse_report(json_data) report_data = parse_report(json_data)
raise SecurityReportParserError, "Invalid report format" unless report_data.is_a?(Hash) raise SecurityReportParserError, "Invalid report format" unless report_data.is_a?(Hash)
report.scanned_resources = report_data.dig('scan', 'scanned_resources') || [] report.scanned_resources = create_scanned_resources(report_data.dig('scan', 'scanned_resources'))
create_scanner(report, report_data.dig('scan', 'scanner')) create_scanner(report, report_data.dig('scan', 'scanner'))
collate_remediations(report_data).each do |vulnerability| collate_remediations(report_data).each do |vulnerability|
...@@ -26,6 +27,17 @@ module Gitlab ...@@ -26,6 +27,17 @@ module Gitlab
protected protected
def create_scanned_resources(scanned_resources)
return [] unless scanned_resources
scanned_resources.map do |sr|
uri = URI.parse(sr['url'])
::Gitlab::Ci::Reports::Security::ScannedResource.new(uri, sr['method'])
rescue URI::InvalidURIError
nil
end.compact
end
def parse_report(json_data) def parse_report(json_data)
Gitlab::Json.parse!(json_data) Gitlab::Json.parse!(json_data)
end end
......
...@@ -13,21 +13,6 @@ module Gitlab ...@@ -13,21 +13,6 @@ module Gitlab
scanned_resources_sum scanned_resources_sum
end end
def scanned_resources_for_csv(scanned_resources)
scanned_resources.map do |sr|
uri = URI.parse(sr['url'] || '')
OpenStruct.new({
request_method: sr['method'],
scheme: uri.scheme,
host: uri.host,
port: uri.port,
path: uri.path,
query_string: uri.query,
raw_url: sr['url']
})
end
end
private private
def parse_report_json(blob) def parse_report_json(blob)
......
# frozen_string_literal: true
module Gitlab
module Ci
module Reports
module Security
class ScannedResource
include Gitlab::Utils::StrongMemoize
attr_reader :request_method
attr_reader :request_uri
delegate :scheme, :host, :port, :path, :query, to: :request_uri, prefix: :url
def initialize(uri, request_method)
raise ArgumentError unless uri.is_a?(URI)
@request_method = request_method
@request_uri = uri
end
end
end
end
end
end
...@@ -30,13 +30,6 @@ RSpec.describe Projects::Security::ScannedResourcesController do ...@@ -30,13 +30,6 @@ RSpec.describe Projects::Security::ScannedResourcesController do
end end
end end
it 'returns a CSV representation of the scanned resources' do
expect_next_instance_of(::Gitlab::Ci::Parsers::Security::ScannedResources) do |instance|
expect(instance).to receive(:scanned_resources_for_csv).and_return([])
end
expect(subject).to have_gitlab_http_status(:ok)
end
context 'when the pipeline id is missing' do context 'when the pipeline id is missing' do
let_it_be(:action_params) { { project_id: project, namespace_id: project.namespace } } let_it_be(:action_params) { { project_id: project, namespace_id: project.namespace } }
......
...@@ -24,67 +24,135 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Common do ...@@ -24,67 +24,135 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Common do
expect(report.findings.map(&:confidence)).not_to include("undefined") expect(report.findings.map(&:confidence)).not_to include("undefined")
end end
context 'parsing scanned resources' do
describe 'when the URL is invalid' do
let(:raw_json) do
{
"vulnerabilities": [],
"remediations": [],
"dependency_files": [],
"scan": {
"scanned_resources": [
{
"method": "GET",
"type": "url",
"url": "not a URL"
}
]
}
}
end
it 'skips invalid URLs' do
parser.parse!(raw_json.to_json, report)
expect(report.scanned_resources).to be_empty
end
end
describe 'when the URLs are valid' do
let(:raw_json) do
{
"vulnerabilities": [],
"remediations": [],
"dependency_files": [],
"scan": {
"scanned_resources": [
{
"method": "GET",
"type": "url",
"url": "http://example.com/1"
},
{
"method": "POST",
"type": "url",
"url": "http://example.com/2"
},
{
"method": "GET",
"type": "url",
"url": "http://example.com/3"
}
]
}
}
end
it 'creates a scanned resource for each URL' do
parser.parse!(raw_json.to_json, report)
expect(report.scanned_resources.length).to eq(3)
end
it 'converts the JSON to Scanned Resource objects' do
parser.parse!(raw_json.to_json, report)
expect(report.scanned_resources.first).to be_a(::Gitlab::Ci::Reports::Security::ScannedResource)
end
end
end
context 'parsing remediations' do context 'parsing remediations' do
let(:raw_json) do let(:raw_json) do
{ {
"vulnerabilities": [ "vulnerabilities": [
{ {
"category": "dependency_scanning", "category": "dependency_scanning",
"name": "Vulnerabilities in libxml2", "name": "Vulnerabilities in libxml2",
"message": "Vulnerabilities in libxml2 in nokogiri", "message": "Vulnerabilities in libxml2 in nokogiri",
"description": "", "description": "",
"cve": "CVE-1020", "cve": "CVE-1020",
"severity": "High", "severity": "High",
"solution": "Upgrade to latest version.", "solution": "Upgrade to latest version.",
"scanner": { "id": "gemnasium", "name": "Gemnasium" }, "scanner": { "id": "gemnasium", "name": "Gemnasium" },
"location": {}, "location": {},
"identifiers": [], "identifiers": [],
"links": [{ "url": "" }] "links": [{ "url": "" }]
},
{
"id": "bb2fbeb1b71ea360ce3f86f001d4e84823c3ffe1a1f7d41ba7466b14cfa953d3",
"category": "dependency_scanning",
"name": "Regular Expression Denial of Service",
"message": "Regular Expression Denial of Service in debug",
"description": "",
"cve": "CVE-1030",
"severity": "Unknown",
"solution": "Upgrade to latest versions.",
"scanner": {
"id": "gemnasium",
"name": "Gemnasium"
}, },
"location": {}, {
"identifiers": [], "id": "bb2fbeb1b71ea360ce3f86f001d4e84823c3ffe1a1f7d41ba7466b14cfa953d3",
"links": [{ "url": "" }] "category": "dependency_scanning",
}, "name": "Regular Expression Denial of Service",
{ "message": "Regular Expression Denial of Service in debug",
"category": "dependency_scanning", "description": "",
"name": "Authentication bypass via incorrect DOM traversal and canonicalization", "cve": "CVE-1030",
"message": "Authentication bypass via incorrect DOM traversal and canonicalization in saml2-js", "severity": "Unknown",
"description": "", "solution": "Upgrade to latest versions.",
"cve": "yarn/yarn.lock:saml2-js:gemnasium:9952e574-7b5b-46fa-a270-aeb694198a98", "scanner": {
"severity": "Unknown", "id": "gemnasium",
"solution": "Upgrade to fixed version.\r\n", "name": "Gemnasium"
},
"location": {},
"identifiers": [],
"links": [{ "url": "" }]
},
{
"category": "dependency_scanning",
"name": "Authentication bypass via incorrect DOM traversal and canonicalization",
"message": "Authentication bypass via incorrect DOM traversal and canonicalization in saml2-js",
"description": "",
"cve": "yarn/yarn.lock:saml2-js:gemnasium:9952e574-7b5b-46fa-a270-aeb694198a98",
"severity": "Unknown",
"solution": "Upgrade to fixed version.\r\n",
"scanner": {
"id": "gemnasium",
"name": "Gemnasium"
},
"location": {},
"identifiers": [],
"links": [{ "url": "" }, { "url": "" }]
}
],
"remediations": [],
"dependency_files": [],
"scan": {
"scanner": { "scanner": {
"id": "gemnasium", "id": "gemnasium",
"name": "Gemnasium" "name": "Gemnasium",
}, "vendor": { "name": "GitLab" }
"location": {}, }
"identifiers": [],
"links": [{ "url": "" }, { "url": "" }]
}
],
"remediations": [],
"dependency_files": [],
"scan": {
"scanner": {
"id": "gemnasium",
"name": "Gemnasium",
"vendor": { "name": "GitLab" }
} }
} }
}
end end
it 'finds remediation with same cve' do it 'finds remediation with same cve' do
......
...@@ -32,61 +32,4 @@ RSpec.describe Gitlab::Ci::Parsers::Security::ScannedResources do ...@@ -32,61 +32,4 @@ RSpec.describe Gitlab::Ci::Parsers::Security::ScannedResources do
it { is_expected.to be(0) } it { is_expected.to be(0) }
end end
end end
describe '#scanned_resources_for_csv' do
subject { parser.scanned_resources_for_csv(scanned_resources) }
context 'when there are scanned resources' do
let(:scanned_resources) do
[
{ "method" => "GET", "type" => "url", "url" => "http://railsgoat:3001" },
{ "method" => "GET", "type" => "url", "url" => "http://railsgoat:3001/" },
{ "method" => "GET", "type" => "url", "url" => "http://railsgoat:3001/login?foo=bar" },
{ "method" => "POST", "type" => "url", "url" => "http://railsgoat:3001/users" }
]
end
it 'converts the hash to OpenStructs', :aggregate_failures do
expect(subject.length).to eq(4)
resource = subject[2]
expect(resource.request_method).to eq('GET')
expect(resource.scheme).to eq('http')
expect(resource.host).to eq('railsgoat')
expect(resource.port).to eq(3001)
expect(resource.path).to eq('/login')
expect(resource.query_string).to eq('foo=bar')
end
end
context 'when there is an invalid URL' do
let(:scanned_resources) do
[
{ "method" => "GET", "type" => "url", "url" => "http://railsgoat:3001" },
{ "method" => "GET", "type" => "url", "url" => "notaURL" },
{ "method" => "GET", "type" => "url", "url" => "" },
{ "method" => "GET", "type" => "url", "url" => nil },
{ "method" => "GET", "type" => "url", "url" => "http://railsgoat:3001/login?foo=bar" }
]
end
it 'returns a blank object with full URL string', :aggregate_failures do
expect(subject.length).to eq(5)
invalid_url = subject[1]
expect(invalid_url.request_method).to eq('GET')
expect(invalid_url.scheme).to be_nil
expect(invalid_url.raw_url).to eq('notaURL')
blank_url = subject[2]
expect(blank_url.request_method).to eq('GET')
expect(blank_url.scheme).to be_nil
expect(blank_url.raw_url).to eq('')
nil_url = subject[3]
expect(nil_url.request_method).to eq('GET')
expect(nil_url.scheme).to be_nil
expect(nil_url.raw_url).to be_nil
end
end
end
end end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::Ci::Reports::Security::ScannedResource do
let(:url) { 'http://example.com:3001/1?foo=bar' }
let(:request_method) { 'GET' }
context 'when the URI is not a URI' do
subject { ::Gitlab::Ci::Reports::Security::ScannedResource.new(url, request_method) }
it 'raises an error' do
expect { subject }.to raise_error(ArgumentError)
end
end
context 'when the URL is valid' do
subject { ::Gitlab::Ci::Reports::Security::ScannedResource.new(URI.parse(url), request_method) }
it 'sets the URL attributes' do
expect(subject.request_method).to eq(request_method)
expect(subject.request_uri.to_s).to eq(url)
expect(subject.url_scheme).to eq('http')
expect(subject.url_host).to eq('example.com')
expect(subject.url_port).to eq(3001)
expect(subject.url_path).to eq('/1')
expect(subject.url_query).to eq('foo=bar')
end
end
end
...@@ -82,13 +82,29 @@ RSpec.describe Security::MergeReportsService, '#execute' do ...@@ -82,13 +82,29 @@ RSpec.describe Security::MergeReportsService, '#execute' do
let(:report_1_findings) { [finding_id_1, finding_id_2_loc_1, finding_cwe_2, finding_wasc_1] } let(:report_1_findings) { [finding_id_1, finding_id_2_loc_1, finding_cwe_2, finding_wasc_1] }
let(:scanned_resource) do
::Gitlab::Ci::Reports::Security::ScannedResource.new(URI.parse('example.com'), 'GET')
end
let(:scanned_resource_1) do
::Gitlab::Ci::Reports::Security::ScannedResource.new(URI.parse('example.com'), 'POST')
end
let(:scanned_resource_2) do
::Gitlab::Ci::Reports::Security::ScannedResource.new(URI.parse('example.com/2'), 'GET')
end
let(:scanned_resource_3) do
::Gitlab::Ci::Reports::Security::ScannedResource.new(URI.parse('example.com/3'), 'GET')
end
let(:report_1) do let(:report_1) do
build( build(
:ci_reports_security_report, :ci_reports_security_report,
scanners: [scanner_1, scanner_2], scanners: [scanner_1, scanner_2],
findings: report_1_findings, findings: report_1_findings,
identifiers: report_1_findings.flat_map(&:identifiers), identifiers: report_1_findings.flat_map(&:identifiers),
scanned_resources: ['example.com', 'example.com/1', 'example.com/2'] scanned_resources: [scanned_resource, scanned_resource_1, scanned_resource_2]
) )
end end
...@@ -100,7 +116,7 @@ RSpec.describe Security::MergeReportsService, '#execute' do ...@@ -100,7 +116,7 @@ RSpec.describe Security::MergeReportsService, '#execute' do
scanners: [scanner_2], scanners: [scanner_2],
findings: report_2_findings, findings: report_2_findings,
identifiers: finding_id_2_loc_2.identifiers, identifiers: finding_id_2_loc_2.identifiers,
scanned_resources: ['example.com', 'example.com/3'] scanned_resources: [scanned_resource, scanned_resource_1, scanned_resource_3]
) )
end end
...@@ -153,10 +169,10 @@ RSpec.describe Security::MergeReportsService, '#execute' do ...@@ -153,10 +169,10 @@ RSpec.describe Security::MergeReportsService, '#execute' do
it 'deduplicates scanned resources' do it 'deduplicates scanned resources' do
expect(subject.scanned_resources).to( expect(subject.scanned_resources).to(
eq([ eq([
'example.com', scanned_resource,
'example.com/1', scanned_resource_1,
'example.com/2', scanned_resource_2,
'example.com/3' scanned_resource_3
]) ])
) )
end end
......
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