diff --git a/app/assets/javascripts/lib/utils/text_utility.js b/app/assets/javascripts/lib/utils/text_utility.js index df512b55480546376de539a0f85a2bb8d6ae7776..51df16ad2d62ef7e3f6c09e232108c2e9cc411bd 100644 --- a/app/assets/javascripts/lib/utils/text_utility.js +++ b/app/assets/javascripts/lib/utils/text_utility.js @@ -384,9 +384,14 @@ export function insertFinalNewline(content, endOfLine = '\n') { return content.slice(-endOfLine.length) !== endOfLine ? `${content}${endOfLine}` : content; } +/** + * Base Markdown config. Use only when paired with sanitized/GFM + * server side validated markdown. When using client-only markdown, + * use `strictMarkdownConfig` instead. + * + * AllowedTags from GitLab's inline HTML guidelines https://docs.gitlab.com/ee/user/markdown.html#inline-html + */ export const markdownConfig = { - // allowedTags from GitLab's inline HTML guidelines - // https://docs.gitlab.com/ee/user/markdown.html#inline-html ALLOWED_TAGS: [ 'a', 'abbr', @@ -442,6 +447,15 @@ export const markdownConfig = { ALLOW_DATA_ATTR: false, }; +/** + * Markdown configuration that does not allow images. Preferred + * when using client-side/non-gfm markdown. + */ +export const strictMarkdownConfig = { + ...markdownConfig, + ALLOWED_TAGS: markdownConfig.ALLOWED_TAGS.filter((tag) => tag !== 'img'), +}; + /** * A regex that matches all single quotes in a string */ diff --git a/app/assets/javascripts/vue_shared/components/markdown/non_gfm_markdown.vue b/app/assets/javascripts/vue_shared/components/markdown/non_gfm_markdown.vue index 8e67bdb89bcccc8268a9cc1dbe48ec27972b9833..984024ea196bd27578d56ab5002595a34c9837a8 100644 --- a/app/assets/javascripts/vue_shared/components/markdown/non_gfm_markdown.vue +++ b/app/assets/javascripts/vue_shared/components/markdown/non_gfm_markdown.vue @@ -11,7 +11,7 @@ import { marked } from 'marked'; import CodeBlockHighlighted from '~/vue_shared/components/code_block_highlighted.vue'; import SafeHtml from '~/vue_shared/directives/safe_html'; import { sanitize } from '~/lib/dompurify'; -import { markdownConfig } from '~/lib/utils/text_utility'; +import { strictMarkdownConfig } from '~/lib/utils/text_utility'; import { __ } from '~/locale'; import SimpleCopyButton from '~/vue_shared/components/simple_copy_button.vue'; @@ -28,12 +28,6 @@ export default { type: String, required: true, }, - /* Including images expose potential security risks. Always use this with controlled and or API sanitized data */ - withImages: { - type: Boolean, - required: false, - default: false, - }, }, data() { return { @@ -70,14 +64,7 @@ export default { }, methods: { getSafeHtml(markdown) { - const markdownConfigWithoutImg = this.withImages - ? markdownConfig - : { - ...markdownConfig, - ALLOWED_TAGS: markdownConfig.ALLOWED_TAGS.filter((tag) => tag !== 'img'), - }; - - return sanitize(marked.parse(markdown), markdownConfigWithoutImg); + return sanitize(marked.parse(markdown), strictMarkdownConfig); }, setHoverOn(key) { this.hoverMap = { ...this.hoverMap, [key]: true }; diff --git a/spec/frontend/lib/utils/text_utility_spec.js b/spec/frontend/lib/utils/text_utility_spec.js index d93f692c911b56348f3b692b95794edd4e5769f5..66938711671bd337d1a1f4c7c6acd78a835ebd24 100644 --- a/spec/frontend/lib/utils/text_utility_spec.js +++ b/spec/frontend/lib/utils/text_utility_spec.js @@ -1,5 +1,9 @@ import * as textUtils from '~/lib/utils/text_utility'; import { stubCrypto } from 'helpers/crypto'; +import { + markdownConfig, + strictMarkdownConfig, +} from '../../../../app/assets/javascripts/lib/utils/text_utility'; describe('text_utility', () => { describe('addDelimiter', () => { @@ -53,6 +57,15 @@ describe('text_utility', () => { }); }); + describe('strictMarkdownConfig', () => { + it('does not allow images', () => { + expect(strictMarkdownConfig).toEqual({ + ...markdownConfig, + ALLOWED_TAGS: expect.not.arrayContaining(['img']), + }); + }); + }); + describe('slugify', () => { it.each` title | input | output diff --git a/spec/frontend/vue_shared/components/markdown/non_gfm_markdown_spec.js b/spec/frontend/vue_shared/components/markdown/non_gfm_markdown_spec.js index e6783fe007459cfd6d83f6b72ad4e320894c66f4..c1c3f9af5350b2197c4d2b449e69765ed5290a33 100644 --- a/spec/frontend/vue_shared/components/markdown/non_gfm_markdown_spec.js +++ b/spec/frontend/vue_shared/components/markdown/non_gfm_markdown_spec.js @@ -32,6 +32,11 @@ describe('NonGitlabMarkdown', () => { const findMarkdownBlock = () => wrapper.findByTestId('non-code-markdown'); const findImageTags = () => wrapper.findAll('img'); + it('should not render image tags', () => { + createComponent({ propsData: { markdown: '![alt text](image.jpg)' } }); + expect(findImageTags()).toHaveLength(0); + }); + describe('rendering markdown without code snippet', () => { beforeEach(() => { createComponent({ propsData: { markdown: nonCodeContent } }); @@ -154,21 +159,5 @@ describe('NonGitlabMarkdown', () => { expect(copyCodeButton.exists()).toBe(false); }); }); - - describe('image tags', () => { - it('should not render image tags by default', () => { - createComponent({ propsData: { markdown: '![alt text](image.jpg)' } }); - expect(findImageTags()).toHaveLength(0); - }); - - describe('when withImages prop is true', () => { - it('should render image tags', () => { - createComponent({ propsData: { markdown: '![alt text](image.jpg)', withImages: true } }); - - expect(findImageTags()).toHaveLength(1); - expect(findImageTags().at(0).attributes('src')).toBe('image.jpg'); - }); - }); - }); }); });