From 51b623a1cdbe011a6258aea0897ce45ec4d06129 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Caplette?= Date: Tue, 16 Dec 2025 14:28:46 -0500 Subject: [PATCH 1/2] Remove with-images prop from non-gfm markdown This is no longer supported as passing all images processing for markdown should go through GFM server side. --- .../javascripts/lib/utils/text_utility.js | 18 ++++++++++++++-- .../components/markdown/non_gfm_markdown.vue | 17 ++------------- spec/frontend/lib/utils/text_utility_spec.js | 13 ++++++++++++ .../markdown/non_gfm_markdown_spec.js | 21 +++++-------------- 4 files changed, 36 insertions(+), 33 deletions(-) diff --git a/app/assets/javascripts/lib/utils/text_utility.js b/app/assets/javascripts/lib/utils/text_utility.js index df512b55480546..649de04e25bd17 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. Prefered + * 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 8e67bdb89bcccc..984024ea196bd2 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 d93f692c911b56..66938711671bd3 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 e6783fe007459c..c1c3f9af5350b2 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'); - }); - }); - }); }); }); -- GitLab From ea675fd1e1c8f305fa00cb5b3485bdfcbd6da148 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Caplette?= Date: Tue, 16 Dec 2025 14:41:10 -0500 Subject: [PATCH 2/2] Apply 1 suggestion(s) to 1 file(s) Co-authored-by: GitLab Duo --- app/assets/javascripts/lib/utils/text_utility.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/lib/utils/text_utility.js b/app/assets/javascripts/lib/utils/text_utility.js index 649de04e25bd17..51df16ad2d62ef 100644 --- a/app/assets/javascripts/lib/utils/text_utility.js +++ b/app/assets/javascripts/lib/utils/text_utility.js @@ -448,7 +448,7 @@ export const markdownConfig = { }; /** - * Markdown configuration that does not allow images. Prefered + * Markdown configuration that does not allow images. Preferred * when using client-side/non-gfm markdown. */ export const strictMarkdownConfig = { -- GitLab