Commit c25232f9 authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Merge branch '3995-improve-sast' into 'master'

Resolve "Improved visualization of SAST results in MR widget"

Closes #3995 and #4358

See merge request gitlab-org/gitlab-ee!4017
parents 85802afc 1b40dd88
...@@ -801,7 +801,7 @@ ...@@ -801,7 +801,7 @@
.mr-widget-code-quality-list { .mr-widget-code-quality-list {
list-style: none; list-style: none;
padding: 0 2px 0 0; padding: 0 1px;
margin: 0; margin: 0;
line-height: $code_line_height; line-height: $code_line_height;
......
---
title: Adds support to show added, fixed and all vulnerabilties for SAST in merge
request widget
merge_request:
author:
type: changed
...@@ -5,7 +5,7 @@ ...@@ -5,7 +5,7 @@
import issuesBlock from './mr_widget_report_issues.vue'; import issuesBlock from './mr_widget_report_issues.vue';
export default { export default {
name: 'MRWidgetCodeQuality', name: 'MRWidgetCodeQualityCollapsible',
components: { components: {
issuesBlock, issuesBlock,
loadingIcon, loadingIcon,
...@@ -49,10 +49,15 @@ ...@@ -49,10 +49,15 @@
required: false, required: false,
default: () => [], default: () => [],
}, },
allIssues: {
type: Array,
required: false,
default: () => [],
},
infoText: { infoText: {
type: String, type: [String, Boolean],
required: false, required: false,
default: null, default: false,
}, },
hasPriority: { hasPriority: {
type: Boolean, type: Boolean,
...@@ -65,6 +70,7 @@ ...@@ -65,6 +70,7 @@
return { return {
collapseText: __('Expand'), collapseText: __('Expand'),
isCollapsed: true, isCollapsed: true,
isFullReportVisible: false,
}; };
}, },
...@@ -85,7 +91,9 @@ ...@@ -85,7 +91,9 @@
return 'success'; return 'success';
}, },
hasIssues() { hasIssues() {
return this.unresolvedIssues.length || this.resolvedIssues.length; return this.unresolvedIssues.length ||
this.resolvedIssues.length ||
this.allIssues.length;
}, },
}, },
...@@ -96,6 +104,9 @@ ...@@ -96,6 +104,9 @@
const text = this.isCollapsed ? __('Expand') : __('Collapse'); const text = this.isCollapsed ? __('Expand') : __('Collapse');
this.collapseText = text; this.collapseText = text;
}, },
openFullReport() {
this.isFullReportVisible = true;
},
}, },
}; };
</script> </script>
...@@ -168,6 +179,15 @@ ...@@ -168,6 +179,15 @@
:has-priority="hasPriority" :has-priority="hasPriority"
/> />
<issues-block
class="js-mr-code-all-issues"
v-if="isFullReportVisible"
:type="type"
status="failed"
:issues="allIssues"
:has-priority="hasPriority"
/>
<issues-block <issues-block
class="js-mr-code-non-issues" class="js-mr-code-non-issues"
v-if="neutralIssues.length" v-if="neutralIssues.length"
...@@ -185,6 +205,15 @@ ...@@ -185,6 +205,15 @@
:issues="resolvedIssues" :issues="resolvedIssues"
:has-priority="hasPriority" :has-priority="hasPriority"
/> />
<button
v-if="allIssues.length && !isFullReportVisible"
type="button"
class="btn-link btn-blank prepend-left-10 js-expand-full-list"
@click="openFullReport"
>
{{ s__("ciReport|Show complete code vulnerabilities report") }}
</button>
</div> </div>
<div <div
v-else-if="loadingFailed" v-else-if="loadingFailed"
......
...@@ -171,6 +171,7 @@ ...@@ -171,6 +171,7 @@
:href="issue.urlPath" :href="issue.urlPath"
target="_blank" target="_blank"
rel="noopener noreferrer nofollow" rel="noopener noreferrer nofollow"
class="prepend-left-5"
> >
{{ issue.path }}<template v-if="issue.line">:{{ issue.line }}</template> {{ issue.path }}<template v-if="issue.line">:{{ issue.line }}</template>
</a> </a>
......
import { n__, s__, sprintf } from '~/locale'; import { n__, s__, __, sprintf } from '~/locale';
import CEWidgetOptions from '~/vue_merge_request_widget/mr_widget_options'; import CEWidgetOptions from '~/vue_merge_request_widget/mr_widget_options';
import WidgetApprovals from './components/approvals/mr_widget_approvals'; import WidgetApprovals from './components/approvals/mr_widget_approvals';
import GeoSecondaryNode from './components/states/mr_widget_secondary_geo_node'; import GeoSecondaryNode from './components/states/mr_widget_secondary_geo_node';
...@@ -38,7 +38,7 @@ export default { ...@@ -38,7 +38,7 @@ export default {
return performance && performance.head_path && performance.base_path; return performance && performance.head_path && performance.base_path;
}, },
shouldRenderSecurityReport() { shouldRenderSecurityReport() {
return this.mr.sast; return this.mr.sast && this.mr.sast.head_path;
}, },
shouldRenderDockerReport() { shouldRenderDockerReport() {
return this.mr.sastContainer; return this.mr.sastContainer;
...@@ -51,9 +51,9 @@ export default { ...@@ -51,9 +51,9 @@ export default {
const text = []; const text = [];
if (!newIssues.length && !resolvedIssues.length) { if (!newIssues.length && !resolvedIssues.length) {
text.push('No changes to code quality'); text.push(s__('ciReport|No changes to code quality'));
} else if (newIssues.length || resolvedIssues.length) { } else if (newIssues.length || resolvedIssues.length) {
text.push('Code quality'); text.push(s__('ciReport|Code quality'));
if (resolvedIssues.length) { if (resolvedIssues.length) {
text.push(n__( text.push(n__(
...@@ -64,7 +64,7 @@ export default { ...@@ -64,7 +64,7 @@ export default {
} }
if (newIssues.length > 0 && resolvedIssues.length > 0) { if (newIssues.length > 0 && resolvedIssues.length > 0) {
text.push(' and'); text.push(__(' and'));
} }
if (newIssues.length) { if (newIssues.length) {
...@@ -84,9 +84,9 @@ export default { ...@@ -84,9 +84,9 @@ export default {
const text = []; const text = [];
if (!improved.length && !degraded.length) { if (!improved.length && !degraded.length) {
text.push('No changes to performance metrics'); text.push(s__('ciReport|No changes to performance metrics'));
} else if (improved.length || degraded.length) { } else if (improved.length || degraded.length) {
text.push('Performance metrics'); text.push(s__('ciReport|Performance metrics'));
if (improved.length) { if (improved.length) {
text.push(n__( text.push(n__(
...@@ -97,7 +97,7 @@ export default { ...@@ -97,7 +97,7 @@ export default {
} }
if (improved.length > 0 && degraded.length > 0) { if (improved.length > 0 && degraded.length > 0) {
text.push(' and'); text.push(__(' and'));
} }
if (degraded.length) { if (degraded.length) {
...@@ -113,15 +113,36 @@ export default { ...@@ -113,15 +113,36 @@ export default {
}, },
securityText() { securityText() {
if (this.mr.securityReport.length) { const { newIssues, resolvedIssues } = this.mr.securityReport;
return n__( const text = [];
'SAST detected %d security vulnerability',
'SAST detected %d security vulnerabilities', if (!newIssues.length && !resolvedIssues.length) {
this.mr.securityReport.length, text.push(s__('ciReport|SAST detected no security vulnerabilities'));
); } else if (newIssues.length || resolvedIssues.length) {
text.push(s__('ciReport|SAST'));
} }
return 'SAST detected no security vulnerabilities'; if (resolvedIssues.length) {
text.push(n__(
' improved on %d security vulnerability',
' improved on %d security vulnerabilities',
resolvedIssues.length,
));
}
if (newIssues.length > 0 && resolvedIssues.length > 0) {
text.push(__(' and'));
}
if (newIssues.length) {
text.push(n__(
' degraded on %d security vulnerability',
' degraded on %d security vulnerabilities',
newIssues.length,
));
}
return text.join('');
}, },
dockerText() { dockerText() {
...@@ -211,7 +232,7 @@ export default { ...@@ -211,7 +232,7 @@ export default {
}, },
fetchCodeQuality() { fetchCodeQuality() {
const { head_path, head_blob_path, base_path, base_blob_path } = this.mr.codeclimate; const { head_path, base_path } = this.mr.codeclimate;
this.isLoadingCodequality = true; this.isLoadingCodequality = true;
...@@ -220,7 +241,12 @@ export default { ...@@ -220,7 +241,12 @@ export default {
this.service.fetchReport(base_path), this.service.fetchReport(base_path),
]) ])
.then((values) => { .then((values) => {
this.mr.compareCodeclimateMetrics(values[0], values[1], head_blob_path, base_blob_path); this.mr.compareCodeclimateMetrics(
values[0],
values[1],
this.mr.headBlobPath,
this.mr.baseBlobPath,
);
this.isLoadingCodequality = false; this.isLoadingCodequality = false;
}) })
.catch(() => { .catch(() => {
...@@ -247,20 +273,50 @@ export default { ...@@ -247,20 +273,50 @@ export default {
this.loadingPerformanceFailed = true; this.loadingPerformanceFailed = true;
}); });
}, },
/**
* Sast report can either have 2 reports or just 1
* When it has 2 we need to compare them
* When it has 1 we render the output given
*/
fetchSecurity() { fetchSecurity() {
const { path, blob_path } = this.mr.sast; const { sast } = this.mr;
this.isLoadingSecurity = true; this.isLoadingSecurity = true;
this.service.fetchReport(path) if (sast.base_path && sast.head_path) {
.then((data) => { Promise.all([
this.mr.setSecurityReport(data, blob_path); this.service.fetchReport(sast.head_path),
this.isLoadingSecurity = false; this.service.fetchReport(sast.base_path),
}) ])
.catch(() => { .then((values) => {
this.isLoadingSecurity = false; this.handleSecuritySuccess({
this.loadingSecurityFailed = true; head: values[0],
}); headBlobPath: this.mr.headBlobPath,
base: values[1],
baseBlobPath: this.mr.baseBlobPath,
});
})
.catch(() => this.handleSecurityError());
} else if (sast.head_path) {
this.service.fetchReport(sast.head_path)
.then((data) => {
this.handleSecuritySuccess({
head: data,
headBlobPath: this.mr.headBlobPath,
});
})
.catch(() => this.handleSecurityError());
}
},
handleSecuritySuccess(data) {
this.mr.setSecurityReport(data);
this.isLoadingSecurity = false;
},
handleSecurityError() {
this.isLoadingSecurity = false;
this.loadingSecurityFailed = true;
}, },
fetchDockerReport() { fetchDockerReport() {
...@@ -370,7 +426,9 @@ export default { ...@@ -370,7 +426,9 @@ export default {
:loading-text="translateText('security').loading" :loading-text="translateText('security').loading"
:error-text="translateText('security').error" :error-text="translateText('security').error"
:success-text="securityText" :success-text="securityText"
:unresolved-issues="mr.securityReport" :unresolved-issues="mr.securityReport.newIssues"
:resolved-issues="mr.securityReport.resolvedIssues"
:all-issues="mr.securityReport.allIssues"
:has-priority="true" :has-priority="true"
/> />
<collapsible-section <collapsible-section
......
...@@ -4,6 +4,11 @@ import { stripHtml } from '~/lib/utils/text_utility'; ...@@ -4,6 +4,11 @@ import { stripHtml } from '~/lib/utils/text_utility';
export default class MergeRequestStore extends CEMergeRequestStore { export default class MergeRequestStore extends CEMergeRequestStore {
constructor(data) { constructor(data) {
super(data); super(data);
const blobPath = data.blob_path || {};
this.headBlobPath = blobPath.head_path || '';
this.baseBlobPath = blobPath.base_path || '';
this.initCodeclimate(data); this.initCodeclimate(data);
this.initPerformanceReport(data); this.initPerformanceReport(data);
this.initSecurityReport(data); this.initSecurityReport(data);
...@@ -63,7 +68,11 @@ export default class MergeRequestStore extends CEMergeRequestStore { ...@@ -63,7 +68,11 @@ export default class MergeRequestStore extends CEMergeRequestStore {
initSecurityReport(data) { initSecurityReport(data) {
this.sast = data.sast; this.sast = data.sast;
this.securityReport = []; this.securityReport = {
newIssues: [],
resolvedIssues: [],
allIssues: [],
};
} }
initDockerReport(data) { initDockerReport(data) {
...@@ -79,9 +88,47 @@ export default class MergeRequestStore extends CEMergeRequestStore { ...@@ -79,9 +88,47 @@ export default class MergeRequestStore extends CEMergeRequestStore {
this.dast = data.dast; this.dast = data.dast;
this.dastReport = []; this.dastReport = [];
} }
/**
* Security report has 3 types of issues, newIssues, resolvedIssues and allIssues.
*
* When we have both base and head:
* - newIssues = head - base
* - resolvedIssues = base - head
* - allIssues = head - newIssues - resolvedIssues
*
* When we only have head
* - newIssues = head
* - resolvedIssues = 0
* - allIssues = 0
* @param {*} data
*/
setSecurityReport(issues, path) { setSecurityReport(data) {
this.securityReport = MergeRequestStore.parseIssues(issues, path); if (data.base) {
const filterKey = 'cve';
const parsedHead = MergeRequestStore.parseIssues(data.head, data.headBlobPath);
const parsedBase = MergeRequestStore.parseIssues(data.base, data.baseBlobPath);
this.securityReport.newIssues = MergeRequestStore.filterByKey(
parsedHead,
parsedBase,
filterKey,
);
this.securityReport.resolvedIssues = MergeRequestStore.filterByKey(
parsedBase,
parsedHead,
filterKey,
);
// Remove the new Issues and the added issues
this.securityReport.allIssues = MergeRequestStore.filterByKey(
parsedHead,
this.securityReport.newIssues.concat(this.securityReport.resolvedIssues),
filterKey,
);
} else {
this.securityReport.newIssues = MergeRequestStore.parseIssues(data.head, data.headBlobPath);
}
} }
setDockerReport(data = {}) { setDockerReport(data = {}) {
...@@ -130,13 +177,15 @@ export default class MergeRequestStore extends CEMergeRequestStore { ...@@ -130,13 +177,15 @@ export default class MergeRequestStore extends CEMergeRequestStore {
const parsedHeadIssues = MergeRequestStore.parseIssues(headIssues, headBlobPath); const parsedHeadIssues = MergeRequestStore.parseIssues(headIssues, headBlobPath);
const parsedBaseIssues = MergeRequestStore.parseIssues(baseIssues, baseBlobPath); const parsedBaseIssues = MergeRequestStore.parseIssues(baseIssues, baseBlobPath);
this.codeclimateMetrics.newIssues = MergeRequestStore.filterByFingerprint( this.codeclimateMetrics.newIssues = MergeRequestStore.filterByKey(
parsedHeadIssues, parsedHeadIssues,
parsedBaseIssues, parsedBaseIssues,
'fingerprint',
); );
this.codeclimateMetrics.resolvedIssues = MergeRequestStore.filterByFingerprint( this.codeclimateMetrics.resolvedIssues = MergeRequestStore.filterByKey(
parsedBaseIssues, parsedBaseIssues,
parsedHeadIssues, parsedHeadIssues,
'fingerprint',
); );
} }
...@@ -198,7 +247,7 @@ export default class MergeRequestStore extends CEMergeRequestStore { ...@@ -198,7 +247,7 @@ export default class MergeRequestStore extends CEMergeRequestStore {
* @param {array} issues * @param {array} issues
* @return {array} * @return {array}
*/ */
static parseIssues(issues, path) { static parseIssues(issues, path = '') {
return issues.map((issue) => { return issues.map((issue) => {
const parsedIssue = { const parsedIssue = {
name: issue.check_name || issue.message, name: issue.check_name || issue.message,
...@@ -236,8 +285,8 @@ export default class MergeRequestStore extends CEMergeRequestStore { ...@@ -236,8 +285,8 @@ export default class MergeRequestStore extends CEMergeRequestStore {
}); });
} }
static filterByFingerprint(firstArray, secondArray) { static filterByKey(firstArray, secondArray, key) {
return firstArray.filter(item => !secondArray.find(el => el.fingerprint === item.fingerprint)); return firstArray.filter(item => !secondArray.find(el => el[key] === item[key]));
} }
// normalize performance metrics by indexing on performance subject and metric name // normalize performance metrics by indexing on performance subject and metric name
......
...@@ -13,7 +13,8 @@ module EE ...@@ -13,7 +13,8 @@ module EE
delegate :codeclimate_artifact, to: :base_pipeline, prefix: :base, allow_nil: true delegate :codeclimate_artifact, to: :base_pipeline, prefix: :base, allow_nil: true
delegate :performance_artifact, to: :head_pipeline, prefix: :head, allow_nil: true delegate :performance_artifact, to: :head_pipeline, prefix: :head, allow_nil: true
delegate :performance_artifact, to: :base_pipeline, prefix: :base, allow_nil: true delegate :performance_artifact, to: :base_pipeline, prefix: :base, allow_nil: true
delegate :sast_artifact, to: :head_pipeline, allow_nil: true delegate :sast_artifact, to: :head_pipeline, prefix: :head, allow_nil: true
delegate :sast_artifact, to: :base_pipeline, prefix: :base, allow_nil: true
delegate :sast_container_artifact, to: :head_pipeline, allow_nil: true delegate :sast_container_artifact, to: :head_pipeline, allow_nil: true
delegate :dast_artifact, to: :head_pipeline, allow_nil: true delegate :dast_artifact, to: :head_pipeline, allow_nil: true
delegate :sha, to: :head_pipeline, prefix: :head_pipeline, allow_nil: true delegate :sha, to: :head_pipeline, prefix: :head_pipeline, allow_nil: true
...@@ -47,7 +48,11 @@ module EE ...@@ -47,7 +48,11 @@ module EE
end end
def has_sast_data? def has_sast_data?
sast_artifact&.success? head_sast_artifact&.success?
end
def has_base_sast_data?
base_sast_artifact&.success?
end end
def has_sast_container_data? def has_sast_container_data?
......
...@@ -3,6 +3,16 @@ module EE ...@@ -3,6 +3,16 @@ module EE
extend ActiveSupport::Concern extend ActiveSupport::Concern
prepended do prepended do
expose :blob_path do
expose :head_path, if: -> (mr, _) { mr.head_pipeline_sha } do |merge_request|
project_blob_path(merge_request.project, merge_request.head_pipeline_sha)
end
expose :base_path, if: -> (mr, _) { mr.base_pipeline_sha } do |merge_request|
project_blob_path(merge_request.project, merge_request.base_pipeline_sha)
end
end
expose :codeclimate, if: -> (mr, _) { mr.has_codeclimate_data? } do expose :codeclimate, if: -> (mr, _) { mr.has_codeclimate_data? } do
expose :head_path, if: -> (mr, _) { can?(current_user, :read_build, mr.head_codeclimate_artifact) } do |merge_request| expose :head_path, if: -> (mr, _) { can?(current_user, :read_build, mr.head_codeclimate_artifact) } do |merge_request|
raw_project_build_artifacts_url(merge_request.source_project, raw_project_build_artifacts_url(merge_request.source_project,
...@@ -10,19 +20,11 @@ module EE ...@@ -10,19 +20,11 @@ module EE
path: Ci::Build::CODEQUALITY_FILE) path: Ci::Build::CODEQUALITY_FILE)
end end
expose :head_blob_path, if: -> (mr, _) { mr.head_pipeline_sha } do |merge_request|
project_blob_path(merge_request.project, merge_request.head_pipeline_sha)
end
expose :base_path, if: -> (mr, _) { can?(current_user, :read_build, mr.base_codeclimate_artifact) } do |merge_request| expose :base_path, if: -> (mr, _) { can?(current_user, :read_build, mr.base_codeclimate_artifact) } do |merge_request|
raw_project_build_artifacts_url(merge_request.target_project, raw_project_build_artifacts_url(merge_request.target_project,
merge_request.base_codeclimate_artifact, merge_request.base_codeclimate_artifact,
path: Ci::Build::CODEQUALITY_FILE) path: Ci::Build::CODEQUALITY_FILE)
end end
expose :base_blob_path, if: -> (mr, _) { mr.base_pipeline_sha } do |merge_request|
project_blob_path(merge_request.project, merge_request.base_pipeline_sha)
end
end end
expose :performance, if: -> (mr, _) { expose_performance_data?(mr) } do expose :performance, if: -> (mr, _) { expose_performance_data?(mr) } do
...@@ -40,14 +42,16 @@ module EE ...@@ -40,14 +42,16 @@ module EE
end end
expose :sast, if: -> (mr, _) { expose_sast_data?(mr, current_user) } do expose :sast, if: -> (mr, _) { expose_sast_data?(mr, current_user) } do
expose :path do |merge_request| expose :head_path do |merge_request|
raw_project_build_artifacts_url(merge_request.source_project, raw_project_build_artifacts_url(merge_request.source_project,
merge_request.sast_artifact, merge_request.head_sast_artifact,
path: Ci::Build::SAST_FILE) path: Ci::Build::SAST_FILE)
end end
expose :blob_path, if: -> (mr, _) { mr.head_pipeline_sha } do |merge_request| expose :base_path, if: -> (mr, _) { mr.has_base_sast_data? } do |merge_request|
project_blob_path(merge_request.project, merge_request.head_pipeline_sha) raw_project_build_artifacts_url(merge_request.target_project,
merge_request.base_sast_artifact,
path: Ci::Build::SAST_FILE)
end end
end end
...@@ -57,10 +61,6 @@ module EE ...@@ -57,10 +61,6 @@ module EE
merge_request.sast_container_artifact, merge_request.sast_container_artifact,
path: Ci::Build::SAST_CONTAINER_FILE) path: Ci::Build::SAST_CONTAINER_FILE)
end end
expose :blob_path, if: -> (mr, _) { mr.head_pipeline_sha } do |merge_request|
project_blob_path(merge_request.project, merge_request.head_pipeline_sha)
end
end end
expose :dast, if: -> (mr, _) { expose_dast_data?(mr, current_user) } do expose :dast, if: -> (mr, _) { expose_dast_data?(mr, current_user) } do
...@@ -77,7 +77,7 @@ module EE ...@@ -77,7 +77,7 @@ module EE
def expose_sast_data?(mr, current_user) def expose_sast_data?(mr, current_user)
mr.project.feature_available?(:sast) && mr.project.feature_available?(:sast) &&
mr.has_sast_data? && mr.has_sast_data? &&
can?(current_user, :read_build, mr.sast_artifact) can?(current_user, :read_build, mr.head_sast_artifact)
end end
def expose_performance_data?(mr) def expose_performance_data?(mr)
......
...@@ -173,20 +173,34 @@ describe MergeRequest do ...@@ -173,20 +173,34 @@ describe MergeRequest do
end end
end end
describe '#sast_artifact' do describe '#head_sast_artifact' do
it { is_expected.to delegate_method(:sast_artifact).to(:head_pipeline) } it { is_expected.to delegate_method(:sast_artifact).to(:head_pipeline).with_prefix(:head) }
end
describe '#base_sast_artifact' do
it { is_expected.to delegate_method(:sast_artifact).to(:base_pipeline).with_prefix(:base) }
end end
describe '#has_sast_data?' do describe '#has_sast_data?' do
let(:artifact) { double(success?: true) } let(:artifact) { double(success?: true) }
before do before do
allow(merge_request).to receive(:sast_artifact).and_return(artifact) allow(merge_request).to receive(:head_sast_artifact).and_return(artifact)
end end
it { expect(merge_request.has_sast_data?).to be_truthy } it { expect(merge_request.has_sast_data?).to be_truthy }
end end
describe '#has_base_sast_data?' do
let(:artifact) { double(success?: true) }
before do
allow(merge_request).to receive(:base_sast_artifact).and_return(artifact)
end
it { expect(merge_request.has_base_sast_data?).to be_truthy }
end
describe '#sast_container_artifact' do describe '#sast_container_artifact' do
it { is_expected.to delegate_method(:sast_container_artifact).to(:head_pipeline) } it { is_expected.to delegate_method(:sast_container_artifact).to(:head_pipeline) }
end end
......
...@@ -5,11 +5,21 @@ describe MergeRequestWidgetEntity do ...@@ -5,11 +5,21 @@ describe MergeRequestWidgetEntity do
let(:project) { create :project, :repository } let(:project) { create :project, :repository }
let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } let(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
let(:request) { double('request', current_user: user) } let(:request) { double('request', current_user: user) }
let(:pipeline) { create(:ci_empty_pipeline, project: project) }
subject do subject do
described_class.new(merge_request, request: request) described_class.new(merge_request, request: request)
end end
it 'has blob path data' do
allow(merge_request).to receive(:base_pipeline).and_return(pipeline)
allow(merge_request).to receive(:head_pipeline).and_return(pipeline)
expect(subject.as_json).to include(:blob_path)
expect(subject.as_json[:blob_path]).to include(:base_path)
expect(subject.as_json[:blob_path]).to include(:head_path)
end
it 'has performance data' do it 'has performance data' do
build = create(:ci_build, name: 'job') build = create(:ci_build, name: 'job')
...@@ -24,9 +34,13 @@ describe MergeRequestWidgetEntity do ...@@ -24,9 +34,13 @@ describe MergeRequestWidgetEntity do
build = create(:ci_build, name: 'sast') build = create(:ci_build, name: 'sast')
allow(subject).to receive(:expose_sast_data?).and_return(true) allow(subject).to receive(:expose_sast_data?).and_return(true)
allow(merge_request).to receive(:sast_artifact).and_return(build) allow(merge_request).to receive(:has_base_sast_data?).and_return(true)
allow(merge_request).to receive(:base_sast_artifact).and_return(build)
allow(merge_request).to receive(:head_sast_artifact).and_return(build)
expect(subject.as_json).to include(:sast) expect(subject.as_json).to include(:sast)
expect(subject.as_json[:sast]).to include(:head_path)
expect(subject.as_json[:sast]).to include(:base_path)
end end
it 'has sast_container data' do it 'has sast_container data' do
......
...@@ -116,9 +116,11 @@ ...@@ -116,9 +116,11 @@
"approvals_path": { "type": ["string", "null"] }, "approvals_path": { "type": ["string", "null"] },
"codeclimate": { "codeclimate": {
"head_path": { "type": "string" }, "head_path": { "type": "string" },
"head_blob_path": { "type": "string" }, "base_path": { "type": "string" }
"base_path": { "type": "string" }, },
"base_blob_path": { "type": "string" } "blob_path": {
"head_path": { "type": "string" },
"base_path": { "type": "string" }
} }
}, },
"additionalProperties": false "additionalProperties": false
......
...@@ -98,4 +98,78 @@ describe('Merge Request collapsible section', () => { ...@@ -98,4 +98,78 @@ describe('Merge Request collapsible section', () => {
expect(vm.$el.textContent.trim()).toEqual('Failed to load codeclimate report'); expect(vm.$el.textContent.trim()).toEqual('Failed to load codeclimate report');
}); });
}); });
describe('With full report', () => {
beforeEach(() => {
vm = mountComponent(MRWidgetCodeQuality, {
status: 'success',
successText: 'SAST improved on 1 security vulnerability and degraded on 1 security vulnerability',
type: 'security',
errorText: 'Failed to load security report',
hasPriority: true,
loadingText: 'Loading security report',
resolvedIssues: [{
cve: 'CVE-2016-9999',
file: 'Gemfile.lock',
message: 'Test Information Leak Vulnerability in Action View',
name: 'Test Information Leak Vulnerability in Action View',
path: 'Gemfile.lock',
solution: 'upgrade to >= 5.0.0.beta1.1, >= 4.2.5.1, ~> 4.2.5, >= 4.1.14.1, ~> 4.1.14, ~> 3.2.22.1',
tool: 'bundler_audit',
url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/335P1DcLG00',
urlPath: '/Gemfile.lock',
}],
unresolvedIssues: [{
cve: 'CVE-2014-7829',
file: 'Gemfile.lock',
message: 'Arbitrary file existence disclosure in Action Pack',
name: 'Arbitrary file existence disclosure in Action Pack',
path: 'Gemfile.lock',
solution: 'upgrade to ~> 3.2.21, ~> 4.0.11.1, ~> 4.0.12, ~> 4.1.7.1, >= 4.1.8',
tool: 'bundler_audit',
url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/rMTQy4oRCGk',
urlPath: '/Gemfile.lock',
}],
allIssues: [{
cve: 'CVE-2016-0752',
file: 'Gemfile.lock',
message: 'Possible Information Leak Vulnerability in Action View',
name: 'Possible Information Leak Vulnerability in Action View',
path: 'Gemfile.lock',
solution: 'upgrade to >= 5.0.0.beta1.1, >= 4.2.5.1, ~> 4.2.5, >= 4.1.14.1, ~> 4.1.14, ~> 3.2.22.1',
tool: 'bundler_audit',
url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/335P1DcLG00',
urlPath: '/Gemfile.lock',
}],
});
});
it('should render full report section', (done) => {
vm.$el.querySelector('button').click();
Vue.nextTick(() => {
expect(
vm.$el.querySelector('.js-expand-full-list').textContent.trim(),
).toEqual('Show complete code vulnerabilities report');
done();
});
});
it('should expand full list when clicked and hide the show all button', (done) => {
vm.$el.querySelector('button').click();
Vue.nextTick(() => {
vm.$el.querySelector('.js-expand-full-list').click();
Vue.nextTick(() => {
expect(
vm.$el.querySelector('.js-mr-code-all-issues').textContent.trim(),
).toContain('Possible Information Leak Vulnerability in Action View');
done();
});
});
});
});
}); });
...@@ -9,6 +9,7 @@ import mockData, { ...@@ -9,6 +9,7 @@ import mockData, {
headIssues, headIssues,
basePerformance, basePerformance,
headPerformance, headPerformance,
securityIssuesBase,
securityIssues, securityIssues,
dockerReport, dockerReport,
dockerReportParsed, dockerReportParsed,
...@@ -36,8 +37,8 @@ describe('ee merge request widget options', () => { ...@@ -36,8 +37,8 @@ describe('ee merge request widget options', () => {
gl.mrWidgetData = { gl.mrWidgetData = {
...mockData, ...mockData,
sast: { sast: {
path: 'path.json', base_path: 'path.json',
blob_path: 'blob_path', head_path: 'head_path.json',
}, },
}; };
...@@ -59,7 +60,8 @@ describe('ee merge request widget options', () => { ...@@ -59,7 +60,8 @@ describe('ee merge request widget options', () => {
beforeEach(() => { beforeEach(() => {
mock = mock = new MockAdapter(axios); mock = mock = new MockAdapter(axios);
mock.onGet('path.json').reply(200, securityIssues); mock.onGet('path.json').reply(200, securityIssuesBase);
mock.onGet('head_path.json').reply(200, securityIssues);
vm = mountComponent(Component); vm = mountComponent(Component);
}); });
...@@ -71,7 +73,7 @@ describe('ee merge request widget options', () => { ...@@ -71,7 +73,7 @@ describe('ee merge request widget options', () => {
setTimeout(() => { setTimeout(() => {
expect( expect(
vm.$el.querySelector('.js-sast-widget .js-code-text').textContent.trim(), vm.$el.querySelector('.js-sast-widget .js-code-text').textContent.trim(),
).toEqual('SAST detected 2 security vulnerabilities'); ).toEqual('SAST improved on 1 security vulnerability and degraded on 2 security vulnerabilities');
done(); done();
}, 0); }, 0);
}); });
...@@ -83,6 +85,8 @@ describe('ee merge request widget options', () => { ...@@ -83,6 +85,8 @@ describe('ee merge request widget options', () => {
beforeEach(() => { beforeEach(() => {
mock = mock = new MockAdapter(axios); mock = mock = new MockAdapter(axios);
mock.onGet('path.json').reply(200, []); mock.onGet('path.json').reply(200, []);
mock.onGet('head_path.json').reply(200, []);
vm = mountComponent(Component); vm = mountComponent(Component);
}); });
...@@ -106,6 +110,7 @@ describe('ee merge request widget options', () => { ...@@ -106,6 +110,7 @@ describe('ee merge request widget options', () => {
beforeEach(() => { beforeEach(() => {
mock = mock = new MockAdapter(axios); mock = mock = new MockAdapter(axios);
mock.onGet('path.json').reply(500, []); mock.onGet('path.json').reply(500, []);
mock.onGet('head_path.json').reply(500, []);
vm = mountComponent(Component); vm = mountComponent(Component);
}); });
...@@ -373,7 +378,6 @@ describe('ee merge request widget options', () => { ...@@ -373,7 +378,6 @@ describe('ee merge request widget options', () => {
...mockData, ...mockData,
sast_container: { sast_container: {
path: 'gl-sast-container.json', path: 'gl-sast-container.json',
blob_path: 'blob_path',
}, },
}; };
......
...@@ -210,11 +210,13 @@ export default { ...@@ -210,11 +210,13 @@ export default {
"diverged_commits_count": 0, "diverged_commits_count": 0,
"only_allow_merge_if_pipeline_succeeds": false, "only_allow_merge_if_pipeline_succeeds": false,
"commit_change_content_path": "/root/acets-app/merge_requests/22/commit_change_content", "commit_change_content_path": "/root/acets-app/merge_requests/22/commit_change_content",
"codeclimate": { codeclimate: {
"head_path": "head.json", head_path: "head.json",
"head_blob_path": "/root/acets-app/blob/abcdef", base_path: "base.json",
"base_path": "base.json", },
"base_blob_path": "/root/acets-app/blob/abcdef" blob_path: {
base_path: 'blob_path',
head_path: 'blob_path',
}, },
}; };
...@@ -379,55 +381,122 @@ export const securityParsedIssues = [ ...@@ -379,55 +381,122 @@ export const securityParsedIssues = [
}, },
]; ];
export const securityIssues = [ export const securityIssues = [{
{ tool: 'bundler_audit',
tool: 'bundler_audit', message: 'Arbitrary file existence disclosure in Action Pack',
message: 'Arbitrary file existence disclosure in Action Pack', url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/rMTQy4oRCGk',
url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/rMTQy4oRCGk', cve: 'CVE-2014-7829',
cve: 'CVE-2014-7829', file: 'Gemfile.lock',
file: 'Gemfile.lock', solution: 'upgrade to ~> 3.2.21, ~> 4.0.11.1, ~> 4.0.12, ~> 4.1.7.1, >= 4.1.8'
solution: 'upgrade to ~> 3.2.21, ~> 4.0.11.1, ~> 4.0.12, ~> 4.1.7.1, >= 4.1.8', }, {
priority:'High', tool: 'bundler_audit',
line: 12, message: 'Possible Information Leak Vulnerability in Action View',
}, url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/335P1DcLG00',
{ cve: 'CVE-2016-0752',
tool: 'bundler_audit', file: 'Gemfile.lock',
message: 'Possible Information Leak Vulnerability in Action View', solution: 'upgrade to >= 5.0.0.beta1.1, >= 4.2.5.1, ~> 4.2.5, >= 4.1.14.1, ~> 4.1.14, ~> 3.2.22.1'
url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/335P1DcLG00', }, {
cve: 'CVE-2016-0752', tool: 'bundler_audit',
file: 'Gemfile.lock', message: 'Possible Object Leak and Denial of Service attack in Action Pack',
solution: 'upgrade to >= 5.0.0.beta1.1, >= 4.2.5.1, ~> 4.2.5, >= 4.1.14.1, ~> 4.1.14, ~> 3.2.22.1', url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/9oLY_FCzvoc',
priority: 'Medium', cve: 'CVE-2016-0751',
}, file: 'Gemfile.lock',
]; solution: 'upgrade to >= 5.0.0.beta1.1, >= 4.2.5.1, ~> 4.2.5, >= 4.1.14.1, ~> 4.1.14, ~> 3.2.22.1'
}];
export const parsedSecurityIssuesStore = [ export const securityIssuesBase = [{
{ tool: 'bundler_audit',
tool: 'bundler_audit', message: 'Test Information Leak Vulnerability in Action View',
message: 'Arbitrary file existence disclosure in Action Pack', url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/335P1DcLG00',
url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/rMTQy4oRCGk', cve: 'CVE-2016-9999',
cve: 'CVE-2014-7829', file: 'Gemfile.lock',
file: 'Gemfile.lock', solution: 'upgrade to >= 5.0.0.beta1.1, >= 4.2.5.1, ~> 4.2.5, >= 4.1.14.1, ~> 4.1.14, ~> 3.2.22.1'
solution: 'upgrade to ~> 3.2.21, ~> 4.0.11.1, ~> 4.0.12, ~> 4.1.7.1, >= 4.1.8', }, {
priority:'High', tool: 'bundler_audit',
line: 12, message: 'Possible Information Leak Vulnerability in Action View',
name: 'Arbitrary file existence disclosure in Action Pack', url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/335P1DcLG00',
path: 'Gemfile.lock', cve: 'CVE-2016-0752',
urlPath: 'path/Gemfile.lock#L12' file: 'Gemfile.lock',
}, solution: 'upgrade to >= 5.0.0.beta1.1, >= 4.2.5.1, ~> 4.2.5, >= 4.1.14.1, ~> 4.1.14, ~> 3.2.22.1'
{ }];
tool: 'bundler_audit',
message: 'Possible Information Leak Vulnerability in Action View', export const parsedSecurityIssuesStore = [{
url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/335P1DcLG00', tool: 'bundler_audit',
cve: 'CVE-2016-0752', message: 'Arbitrary file existence disclosure in Action Pack',
file: 'Gemfile.lock', url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/rMTQy4oRCGk',
solution: 'upgrade to >= 5.0.0.beta1.1, >= 4.2.5.1, ~> 4.2.5, >= 4.1.14.1, ~> 4.1.14, ~> 3.2.22.1', cve: 'CVE-2014-7829',
priority: 'Medium', file: 'Gemfile.lock',
name: 'Possible Information Leak Vulnerability in Action View', solution: 'upgrade to ~> 3.2.21, ~> 4.0.11.1, ~> 4.0.12, ~> 4.1.7.1, >= 4.1.8',
path: 'Gemfile.lock', name: 'Arbitrary file existence disclosure in Action Pack',
urlPath: 'path/Gemfile.lock', path: 'Gemfile.lock',
}, urlPath: 'path/Gemfile.lock'
]; }, {
tool: 'bundler_audit',
message: 'Possible Information Leak Vulnerability in Action View',
url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/335P1DcLG00',
cve: 'CVE-2016-0752',
file: 'Gemfile.lock',
solution: 'upgrade to >= 5.0.0.beta1.1, >= 4.2.5.1, ~> 4.2.5, >= 4.1.14.1, ~> 4.1.14, ~> 3.2.22.1',
name: 'Possible Information Leak Vulnerability in Action View',
path: 'Gemfile.lock',
urlPath: 'path/Gemfile.lock'
}, {
tool: 'bundler_audit',
message: 'Possible Object Leak and Denial of Service attack in Action Pack',
url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/9oLY_FCzvoc',
cve: 'CVE-2016-0751',
file: 'Gemfile.lock',
solution: 'upgrade to >= 5.0.0.beta1.1, >= 4.2.5.1, ~> 4.2.5, >= 4.1.14.1, ~> 4.1.14, ~> 3.2.22.1',
name: 'Possible Object Leak and Denial of Service attack in Action Pack',
path: 'Gemfile.lock',
urlPath: 'path/Gemfile.lock'
}];
export const parsedSecurityIssuesHead = [{
tool: 'bundler_audit',
message: 'Arbitrary file existence disclosure in Action Pack',
url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/rMTQy4oRCGk',
cve: 'CVE-2014-7829',
file: 'Gemfile.lock',
solution: 'upgrade to ~> 3.2.21, ~> 4.0.11.1, ~> 4.0.12, ~> 4.1.7.1, >= 4.1.8',
name: 'Arbitrary file existence disclosure in Action Pack',
path: 'Gemfile.lock',
urlPath: 'path/Gemfile.lock'
}, {
tool: 'bundler_audit',
message: 'Possible Object Leak and Denial of Service attack in Action Pack',
url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/9oLY_FCzvoc',
cve: 'CVE-2016-0751',
file: 'Gemfile.lock',
solution: 'upgrade to >= 5.0.0.beta1.1, >= 4.2.5.1, ~> 4.2.5, >= 4.1.14.1, ~> 4.1.14, ~> 3.2.22.1',
name: 'Possible Object Leak and Denial of Service attack in Action Pack',
path: 'Gemfile.lock',
urlPath: 'path/Gemfile.lock'
}];
export const parsedSecurityIssuesBaseStore = [{
name: 'Test Information Leak Vulnerability in Action View',
tool: 'bundler_audit',
message: 'Test Information Leak Vulnerability in Action View',
url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/335P1DcLG00',
cve: 'CVE-2016-9999',
file: 'Gemfile.lock',
solution: 'upgrade to >= 5.0.0.beta1.1, >= 4.2.5.1, ~> 4.2.5, >= 4.1.14.1, ~> 4.1.14, ~> 3.2.22.1',
path: 'Gemfile.lock',
urlPath: 'path/Gemfile.lock'
}];
export const allIssuesParsed = [{
name: 'Possible Information Leak Vulnerability in Action View',
tool: 'bundler_audit',
message: 'Possible Information Leak Vulnerability in Action View',
url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/335P1DcLG00',
cve: 'CVE-2016-0752',
file: 'Gemfile.lock',
solution: 'upgrade to >= 5.0.0.beta1.1, >= 4.2.5.1, ~> 4.2.5, >= 4.1.14.1, ~> 4.1.14, ~> 3.2.22.1',
path: 'Gemfile.lock',
urlPath: 'path/Gemfile.lock',
}];
export const dockerReport = { export const dockerReport = {
unapproved: [ unapproved: [
......
...@@ -4,9 +4,13 @@ import mockData, { ...@@ -4,9 +4,13 @@ import mockData, {
headIssues, headIssues,
baseIssues, baseIssues,
securityIssues, securityIssues,
securityIssuesBase,
parsedBaseIssues, parsedBaseIssues,
parsedHeadIssues, parsedHeadIssues,
parsedSecurityIssuesStore, parsedSecurityIssuesStore,
parsedSecurityIssuesBaseStore,
allIssuesParsed,
parsedSecurityIssuesHead,
dockerReport, dockerReport,
dockerReportParsed, dockerReportParsed,
dast, dast,
...@@ -93,10 +97,22 @@ describe('MergeRequestStore', () => { ...@@ -93,10 +97,22 @@ describe('MergeRequestStore', () => {
}); });
describe('setSecurityReport', () => { describe('setSecurityReport', () => {
it('should set security issues', () => { it('should set security issues with head', () => {
store.setSecurityReport(securityIssues, 'path'); store.setSecurityReport({ head: securityIssues, headBlobPath: 'path' });
expect(store.securityReport.newIssues).toEqual(parsedSecurityIssuesStore);
});
it('should set security issues with head and base', () => {
store.setSecurityReport({
head: securityIssues,
headBlobPath: 'path',
base: securityIssuesBase,
baseBlobPath: 'path',
});
expect(store.securityReport).toEqual(parsedSecurityIssuesStore); expect(store.securityReport.newIssues).toEqual(parsedSecurityIssuesHead);
expect(store.securityReport.resolvedIssues).toEqual(parsedSecurityIssuesBaseStore);
expect(store.securityReport.allIssues).toEqual(allIssuesParsed);
}); });
}); });
......
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