Commit 0d9f0596 authored by Mark Florian's avatar Mark Florian

Merge branch '216873-use-glsprintf-for-notes' into 'master'

Use GlSprintf instead of v-html for security scanning notes

See merge request gitlab-org/gitlab!34136
parents 912f43fc ad124dd0
<script>
import { escape } from 'lodash';
import { GlSprintf, GlLink } from '@gitlab/ui';
import EventItem from 'ee/vue_shared/security_reports/components/event_item.vue';
import { __, sprintf } from '~/locale';
import { __ } from '~/locale';
export default {
components: {
EventItem,
GlSprintf,
GlLink,
},
props: {
feedback: {
......@@ -19,23 +21,15 @@ export default {
},
},
computed: {
eventText() {
const { project, feedback } = this;
const issueLink = `<a href="${feedback.issue_url}">#${feedback.issue_iid}</a>`;
if (project && project.value && project.url) {
const projectLink = `<a href="${escape(project.url)}">${escape(project.value)}</a>`;
return sprintf(
__('Created issue %{issueLink} at %{projectLink}'),
{
issueLink,
projectLink,
hasProjectUrl() {
return this.project?.value && this.project?.url;
},
false,
);
eventText() {
if (this.hasProjectUrl) {
return __('Created issue %{issueLink} at %{projectLink}');
}
return sprintf(__('Created issue %{issueLink}'), { issueLink }, false);
return __('Created issue %{issueLink}');
},
},
};
......@@ -43,6 +37,13 @@ export default {
<template>
<event-item :author="feedback.author" :created-at="feedback.created_at" icon-name="issue-created">
<div v-html="eventText"></div>
<gl-sprintf :message="eventText">
<template #issueLink>
<gl-link :href="feedback.issue_url">#{{ feedback.issue_iid }}</gl-link>
</template>
<template v-if="hasProjectUrl" #projectLink>
<gl-link :href="project.url">{{ project.value }}</gl-link>
</template>
</gl-sprintf>
</event-item>
</template>
<script>
import { escape } from 'lodash';
import { GlSprintf, GlLink } from '@gitlab/ui';
import EventItem from 'ee/vue_shared/security_reports/components/event_item.vue';
import { __, sprintf } from '~/locale';
import { __ } from '~/locale';
export default {
components: {
EventItem,
GlSprintf,
GlLink,
},
props: {
feedback: {
......@@ -19,23 +21,15 @@ export default {
},
},
computed: {
eventText() {
const { project, feedback } = this;
const mergeRequestLink = `<a href="${feedback.merge_request_path}">!${feedback.merge_request_iid}</a>`;
if (project && project.value && project.url) {
const projectLink = `<a href="${escape(project.url)}">${escape(project.value)}</a>`;
return sprintf(
__('Created merge request %{mergeRequestLink} at %{projectLink}'),
{
mergeRequestLink,
projectLink,
hasProjectUrl() {
return this.project?.value && this.project?.url;
},
false,
);
eventText() {
if (this.hasProjectUrl) {
return __('Created merge request %{mergeRequestLink} at %{projectLink}');
}
return sprintf(__('Created merge request %{mergeRequestLink}'), { mergeRequestLink }, false);
return __('Created merge request %{mergeRequestLink}');
},
},
};
......@@ -43,6 +37,13 @@ export default {
<template>
<event-item :author="feedback.author" :created-at="feedback.created_at" icon-name="merge-request">
<div v-html="eventText"></div>
<gl-sprintf :message="eventText">
<template #mergeRequestLink>
<gl-link :href="feedback.merge_request_path">!{{ feedback.merge_request_iid }}</gl-link>
</template>
<template v-if="hasProjectUrl" #projectLink>
<gl-link :href="project.url">{{ project.value }}</gl-link>
</template>
</gl-sprintf>
</event-item>
</template>
......@@ -84,9 +84,6 @@ export default {
isResolved() {
return Boolean(this.modal.isResolved);
},
hasRemediation() {
return Boolean(this.remediation);
},
project() {
return this.modal.project;
},
......@@ -207,7 +204,6 @@ export default {
:solution="solution"
:remediation="remediation"
:has-mr="vulnerability.hasMergeRequest"
:has-remediation="hasRemediation"
:has-download="canDownloadPatchForThisVulnerability"
:vulnerability-feedback-help-path="vulnerabilityFeedbackHelpPath"
/>
......
......@@ -26,11 +26,6 @@ export default {
default: false,
required: false,
},
hasRemediation: {
type: Boolean,
default: false,
required: false,
},
vulnerabilityFeedbackHelpPath: {
type: String,
default: '',
......@@ -53,13 +48,10 @@ export default {
);
},
showCreateMergeRequestMsg() {
return !this.hasMr && this.hasRemediation && this.hasDownload;
return !this.hasMr && Boolean(this.remediation) && this.hasDownload;
},
showLearnAboutRemedationMsg() {
if (this.hasMr) {
return false;
}
return true;
return !this.hasMr;
},
showMsg() {
return (
......
import { shallowMount } from '@vue/test-utils';
import { GlSprintf } from '@gitlab/ui';
import component from 'ee/vue_shared/security_reports/components/issue_note.vue';
import EventItem from 'ee/vue_shared/security_reports/components/event_item.vue';
......@@ -23,6 +24,7 @@ describe('Issue note', () => {
beforeEach(() => {
wrapper = shallowMount(component, {
stubs: { GlSprintf },
propsData: { feedback },
});
});
......@@ -45,6 +47,7 @@ describe('Issue note', () => {
beforeEach(() => {
wrapper = shallowMount(component, {
stubs: { GlSprintf },
propsData: { feedback, project },
});
});
......@@ -63,6 +66,7 @@ describe('Issue note', () => {
beforeEach(() => {
wrapper = shallowMount(component, {
stubs: { GlSprintf },
propsData: {
feedback,
project: unsafeProject,
......@@ -71,13 +75,8 @@ describe('Issue note', () => {
});
it('should escape the project name', () => {
// Note: We have to check the computed prop here because
// vue test utils unescapes the result of wrapper.text()
expect(wrapper.vm.eventText).not.toContain(project.value);
expect(wrapper.vm.eventText).toContain(
'Foo &lt;script&gt;alert(&quot;XSS&quot;)&lt;/script&gt;',
);
// Test that the XSS text has been rendered literally as safe text.
expect(wrapper.text()).toContain(unsafeProject.value);
});
});
});
import { shallowMount } from '@vue/test-utils';
import { GlSprintf } from '@gitlab/ui';
import component from 'ee/vue_shared/security_reports/components/merge_request_note.vue';
import EventItem from 'ee/vue_shared/security_reports/components/event_item.vue';
......@@ -23,6 +24,7 @@ describe('Merge request note', () => {
beforeEach(() => {
wrapper = shallowMount(component, {
stubs: { GlSprintf },
propsData: { feedback },
});
});
......@@ -45,6 +47,7 @@ describe('Merge request note', () => {
beforeEach(() => {
wrapper = shallowMount(component, {
stubs: { GlSprintf },
propsData: { feedback, project },
});
});
......@@ -65,6 +68,7 @@ describe('Merge request note', () => {
beforeEach(() => {
wrapper = shallowMount(component, {
stubs: { GlSprintf },
propsData: {
feedback,
project: unsafeProject,
......@@ -73,13 +77,8 @@ describe('Merge request note', () => {
});
it('should escape the project name', () => {
// Note: We have to check the computed prop here because
// vue test utils unescapes the result of wrapper.text()
expect(wrapper.vm.eventText).not.toContain(project.value);
expect(wrapper.vm.eventText).toContain(
'Foo &lt;script&gt;alert(&quot;XSS&quot;)&lt;/script&gt;',
);
// Test that the XSS text has been rendered literally as safe text.
expect(wrapper.text()).toContain(unsafeProject.value);
});
});
});
......@@ -37,7 +37,6 @@ describe('Vulnerability Footer', () => {
const solutionInfoProp = {
hasDownload: true,
hasMr: false,
hasRemediation: true,
isStandaloneVulnerability: true,
remediation: {},
solution: 'Upgrade to fixed version.\n',
......
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