fix(editor): Replace v-html with custom directive to sanitize html (#…

…10804) Co-authored-by: Tomi Turtiainen <[email protected]>
n8n-io · Sep 23, 2024 · b64e75f · b64e75f
1 parent ca87b8d
commit b64e75f
Show file tree

Hide file tree

Showing 49 changed files with 379 additions and 130 deletions.
diff --git a/packages/design-system/package.json b/packages/design-system/package.json
@@ -29,7 +29,6 @@
     "@types/sanitize-html": "^2.11.0",
     "@vitejs/plugin-vue": "^5.0.4",
     "@vitest/coverage-v8": "catalog:frontend",
-    "@vue/test-utils": "^2.4.3",
     "autoprefixer": "^10.4.19",
     "postcss": "^8.4.38",
     "sass": "^1.64.1",

diff --git a/packages/design-system/src/components/AskAssistantChat/AskAssistantChat.vue b/packages/design-system/src/components/AskAssistantChat/AskAssistantChat.vue
@@ -151,28 +151,28 @@ function growInput() {
 								/>
 							</div>
 							<div :class="$style.blockBody">
-								<!-- eslint-disable-next-line vue/no-v-html -->
-								<span v-html="renderMarkdown(message.content)"></span>
+								<span v-n8n-html="renderMarkdown(message.content)"></span>
 								<BlinkingCursor
 									v-if="streaming && i === messages?.length - 1 && message.title && message.content"
 								/>
 							</div>
 						</div>
 					</div>
 					<div v-else-if="message.type === 'text'" :class="$style.textMessage">
-						<!-- eslint-disable-next-line vue/no-v-html -->
-						<span v-if="message.role === 'user'" v-html="renderMarkdown(message.content)"></span>
-						<!-- eslint-disable-next-line vue/no-v-html -->
+						<span
+							v-if="message.role === 'user'"
+							v-n8n-html="renderMarkdown(message.content)"
+						></span>
 						<div
 							v-else
+							v-n8n-html="renderMarkdown(message.content)"
 							:class="$style.assistantText"
-							v-html="renderMarkdown(message.content)"
 						></div>
 						<div
 							v-if="message?.codeSnippet"
 							:class="$style['code-snippet']"
 							data-test-id="assistant-code-snippet"
-							v-html="renderMarkdown(message.codeSnippet).trim()"
+							v-n8n-html="renderMarkdown(message.codeSnippet).trim()"
 						></div>
 						<BlinkingCursor
 							v-if="streaming && i === messages?.length - 1 && message.role === 'assistant'"

diff --git a/packages/design-system/src/components/AskAssistantChat/__tests__/AskAssistantChat.spec.ts b/packages/design-system/src/components/AskAssistantChat/__tests__/AskAssistantChat.spec.ts
@@ -1,5 +1,6 @@
 import { render } from '@testing-library/vue';
 import AskAssistantChat from '../AskAssistantChat.vue';
+import { n8nHtml } from 'n8n-design-system/directives';
 
 describe('AskAssistantChat', () => {
 	it('renders default placeholder chat correctly', () => {
@@ -12,6 +13,11 @@ describe('AskAssistantChat', () => {
 	});
 	it('renders chat with messages correctly', () => {
 		const { container } = render(AskAssistantChat, {
+			global: {
+				directives: {
+					n8nHtml,
+				},
+			},
 			props: {
 				user: { firstName: 'Kobi', lastName: 'Dog' },
 				messages: [
@@ -86,6 +92,11 @@ describe('AskAssistantChat', () => {
 	});
 	it('renders streaming chat correctly', () => {
 		const { container } = render(AskAssistantChat, {
+			global: {
+				directives: {
+					n8nHtml,
+				},
+			},
 			props: {
 				user: { firstName: 'Kobi', lastName: 'Dog' },
 				messages: [
@@ -105,6 +116,11 @@ describe('AskAssistantChat', () => {
 	});
 	it('renders end of session chat correctly', () => {
 		const { container } = render(AskAssistantChat, {
+			global: {
+				directives: {
+					n8nHtml,
+				},
+			},
 			props: {
 				user: { firstName: 'Kobi', lastName: 'Dog' },
 				messages: [
@@ -130,6 +146,11 @@ describe('AskAssistantChat', () => {
 	});
 	it('renders message with code snippet', () => {
 		const { container } = render(AskAssistantChat, {
+			global: {
+				directives: {
+					n8nHtml,
+				},
+			},
 			props: {
 				user: { firstName: 'Kobi', lastName: 'Dog' },
 				messages: [

diff --git a/...tem/src/components/AskAssistantChat/__tests__/__snapshots__/AskAssistantChat.spec.ts.snap b/...tem/src/components/AskAssistantChat/__tests__/__snapshots__/AskAssistantChat.spec.ts.snap
@@ -129,9 +129,6 @@ exports[`AskAssistantChat > renders chat with messages correctly 1`] = `
           <div
             class="textMessage"
           >
-            <!-- eslint-disable-next-line vue/no-v-html -->
-            
-            <!-- eslint-disable-next-line vue/no-v-html -->
             <div
               class="assistantText"
             >
@@ -145,7 +142,6 @@ exports[`AskAssistantChat > renders chat with messages correctly 1`] = `
               
 
             </div>
-            
             <!--v-if-->
             <!--v-if-->
           </div>
@@ -438,7 +434,6 @@ exports[`AskAssistantChat > renders chat with messages correctly 1`] = `
           <div
             class="textMessage"
           >
-            <!-- eslint-disable-next-line vue/no-v-html -->
             <span>
               <p>
                 Give it to me 
@@ -516,7 +511,6 @@ exports[`AskAssistantChat > renders chat with messages correctly 1`] = `
               <div
                 class="blockBody"
               >
-                <!-- eslint-disable-next-line vue/no-v-html -->
                 <span>
                   <p>
                     Solution steps:
@@ -1060,9 +1054,6 @@ exports[`AskAssistantChat > renders end of session chat correctly 1`] = `
           <div
             class="textMessage"
           >
-            <!-- eslint-disable-next-line vue/no-v-html -->
-            
-            <!-- eslint-disable-next-line vue/no-v-html -->
             <div
               class="assistantText"
             >
@@ -1076,7 +1067,6 @@ exports[`AskAssistantChat > renders end of session chat correctly 1`] = `
               
 
             </div>
-            
             <!--v-if-->
             <!--v-if-->
           </div>
@@ -1309,9 +1299,6 @@ exports[`AskAssistantChat > renders message with code snippet 1`] = `
           <div
             class="textMessage"
           >
-            <!-- eslint-disable-next-line vue/no-v-html -->
-            
-            <!-- eslint-disable-next-line vue/no-v-html -->
             <div
               class="assistantText"
             >
@@ -1325,7 +1312,6 @@ exports[`AskAssistantChat > renders message with code snippet 1`] = `
               
 
             </div>
-            
             <div
               class="code-snippet"
               data-test-id="assistant-code-snippet"
@@ -1552,9 +1538,6 @@ exports[`AskAssistantChat > renders streaming chat correctly 1`] = `
           <div
             class="textMessage"
           >
-            <!-- eslint-disable-next-line vue/no-v-html -->
-            
-            <!-- eslint-disable-next-line vue/no-v-html -->
             <div
               class="assistantText"
             >
@@ -1568,7 +1551,6 @@ exports[`AskAssistantChat > renders streaming chat correctly 1`] = `
               
 
             </div>
-            
             <!--v-if-->
             <!--v-if-->
           </div>

diff --git a/packages/design-system/src/components/N8nActionBox/ActionBox.vue b/packages/design-system/src/components/N8nActionBox/ActionBox.vue
@@ -37,7 +37,7 @@ withDefaults(defineProps<ActionBoxProps>(), {
 		<div :class="$style.description" @click="$emit('descriptionClick', $event)">
 			<N8nText color="text-base">
 				<slot name="description">
-					<span v-html="description"></span>
+					<span v-n8n-html="description"></span>
 				</slot>
 			</N8nText>
 		</div>
@@ -61,7 +61,7 @@ withDefaults(defineProps<ActionBoxProps>(), {
 			:class="$style.callout"
 		>
 			<N8nText color="text-base">
-				<span size="small" v-html="calloutText"></span>
+				<span size="small" v-n8n-html="calloutText"></span>
 			</N8nText>
 		</N8nCallout>
 	</div>

diff --git a/packages/design-system/src/components/N8nInfoAccordion/InfoAccordion.vue b/packages/design-system/src/components/N8nInfoAccordion/InfoAccordion.vue
@@ -75,15 +75,15 @@ const onTooltipClick = (item: string, event: MouseEvent) => emit('tooltipClick',
 				<div v-for="item in items" :key="item.id" :class="$style.accordionItem">
 					<n8n-tooltip :disabled="!item.tooltip">
 						<template #content>
-							<div @click="onTooltipClick(item.id, $event)" v-html="item.tooltip"></div>
+							<div @click="onTooltipClick(item.id, $event)" v-n8n-html="item.tooltip"></div>
 						</template>
 						<N8nIcon :icon="item.icon" :color="item.iconColor" size="small" class="mr-2xs" />
 					</n8n-tooltip>
 					<N8nText size="small" color="text-base">{{ item.label }}</N8nText>
 				</div>
 			</div>
 			<N8nText color="text-base" size="small" align="left">
-				<span v-html="description"></span>
+				<span v-n8n-html="description"></span>
 			</N8nText>
 			<slot name="customContent"></slot>
 		</div>

diff --git a/packages/design-system/src/components/N8nInputLabel/InputLabel.vue b/packages/design-system/src/components/N8nInputLabel/InputLabel.vue
@@ -58,7 +58,7 @@ const addTargetBlank = (html: string) =>
 				<N8nTooltip placement="top" :popper-class="$style.tooltipPopper" :show-after="300">
 					<N8nIcon icon="question-circle" size="small" />
 					<template #content>
-						<div v-html="addTargetBlank(tooltipText)" />
+						<div v-n8n-html="addTargetBlank(tooltipText)" />
 					</template>
 				</N8nTooltip>
 			</span>

diff --git a/packages/design-system/src/components/N8nMarkdown/Markdown.vue b/packages/design-system/src/components/N8nMarkdown/Markdown.vue
@@ -202,7 +202,7 @@ const onCheckboxChange = (index: number) => {
 			@click="onClick"
 			@mousedown="onMouseDown"
 			@change="onChange"
-			v-html="htmlContent"
+			v-n8n-html="htmlContent"
 		/>
 		<div v-else :class="$style.markdown">
 			<div v-for="(_, index) in loadingBlocks" :key="index">

diff --git a/packages/design-system/src/components/N8nMarkdown/__tests__/Markdown.spec.ts b/packages/design-system/src/components/N8nMarkdown/__tests__/Markdown.spec.ts
@@ -1,10 +1,16 @@
 import { render, fireEvent } from '@testing-library/vue';
 import N8nMarkdown from '../Markdown.vue';
+import { n8nHtml } from 'n8n-design-system/directives';
 
 describe('components', () => {
 	describe('N8nMarkdown', () => {
 		it('should render unchecked checkboxes', () => {
 			const wrapper = render(N8nMarkdown, {
+				global: {
+					directives: {
+						n8nHtml,
+					},
+				},
 				props: {
 					content: '__TODO__\n- [ ] Buy milk\n- [ ] Buy socks\n',
 				},
@@ -18,6 +24,11 @@ describe('components', () => {
 
 		it('should render checked checkboxes', () => {
 			const wrapper = render(N8nMarkdown, {
+				global: {
+					directives: {
+						n8nHtml,
+					},
+				},
 				props: {
 					content: '__TODO__\n- [X] Buy milk\n- [X] Buy socks\n',
 				},
@@ -31,6 +42,11 @@ describe('components', () => {
 
 		it('should toggle checkboxes when clicked', async () => {
 			const wrapper = render(N8nMarkdown, {
+				global: {
+					directives: {
+						n8nHtml,
+					},
+				},
 				props: {
 					content: '__TODO__\n- [ ] Buy milk\n- [ ] Buy socks\n',
 				},
@@ -50,6 +66,11 @@ describe('components', () => {
 
 		it('should render inputs as plain text', () => {
 			const wrapper = render(N8nMarkdown, {
+				global: {
+					directives: {
+						n8nHtml,
+					},
+				},
 				props: {
 					content:
 						'__TODO__\n- [X] Buy milk\n- <input type="text" data-testid="text-input" value="Something"/>\n',

diff --git a/packages/design-system/src/components/N8nNotice/Notice.vue b/packages/design-system/src/components/N8nNotice/Notice.vue
@@ -73,7 +73,7 @@ const onClick = (event: MouseEvent) => {
 						:id="`${id}-content`"
 						:class="showFullContent ? $style['expanded'] : $style['truncated']"
 						role="region"
-						v-html="displayContent"
+						v-n8n-html="displayContent"
 					/>
 				</slot>
 			</N8nText>

diff --git a/packages/design-system/src/components/N8nNotice/__tests__/Notice.spec.ts b/packages/design-system/src/components/N8nNotice/__tests__/Notice.spec.ts
@@ -1,6 +1,7 @@
 import { render } from '@testing-library/vue';
 import N8nNotice from '../Notice.vue';
 import { N8nText } from 'n8n-design-system/components';
+import { n8nHtml } from 'n8n-design-system/directives';
 
 describe('components', () => {
 	describe('N8nNotice', () => {
@@ -41,6 +42,9 @@ describe('components', () => {
 							content: '<strong>Hello world!</strong> This is a notice.',
 						},
 						global: {
+							directives: {
+								n8nHtml,
+							},
 							components: {
 								'n8n-text': N8nText,
 							},

diff --git a/packages/design-system/src/components/N8nSticky/Sticky.vue b/packages/design-system/src/components/N8nSticky/Sticky.vue
@@ -116,7 +116,7 @@ const onInputScroll = (event: WheelEvent) => {
 		</div>
 		<div v-if="editMode && shouldShowFooter" :class="$style.footer">
 			<N8nText size="xsmall" align="right">
-				<span v-html="t('sticky.markdownHint')"></span>
+				<span v-n8n-html="t('sticky.markdownHint')"></span>
 			</N8nText>
 		</div>
 	</div>

diff --git a/packages/design-system/src/components/N8nTabs/Tabs.vue b/packages/design-system/src/components/N8nTabs/Tabs.vue
@@ -89,7 +89,7 @@ const scrollRight = () => scroll(50);
 			>
 				<N8nTooltip :disabled="!option.tooltip" placement="bottom">
 					<template #content>
-						<div @click="handleTooltipClick(option.value, $event)" v-html="option.tooltip" />
+						<div @click="handleTooltipClick(option.value, $event)" v-n8n-html="option.tooltip" />
 					</template>
 					<a
 						v-if="option.href"

diff --git a/packages/design-system/src/components/N8nTooltip/Tooltip.vue b/packages/design-system/src/components/N8nTooltip/Tooltip.vue
@@ -42,7 +42,7 @@ defineOptions({
 		<slot />
 		<template #content>
 			<slot name="content">
-				<div v-html="props.content"></div>
+				<div v-n8n-html="props.content"></div>
 			</slot>
 			<div
 				v-if="props.buttons.length"

diff --git a/packages/design-system/src/directives/index.ts b/packages/design-system/src/directives/index.ts
@@ -1 +1,2 @@
 export { n8nTruncate } from './n8n-truncate';
+export { n8nHtml } from './n8n-html';
diff --git a/packages/design-system/src/directives/n8n-html.test.ts b/packages/design-system/src/directives/n8n-html.test.ts
@@ -0,0 +1,45 @@
+import { render } from '@testing-library/vue';
+import { n8nHtml } from './n8n-html';
+
+const TestComponent = {
+	props: {
+		html: {
+			type: String,
+		},
+	},
+	template: '<div v-n8n-html="html"></div>',
+};
+
+describe('Directive n8n-html', () => {
+	it('should sanitize html', async () => {
+		const { html } = render(TestComponent, {
+			props: {
+				html: '<span>text</span><a href="https://malicious.com" onclick="alert(1)">malicious</a><img alt="Ok" src="./images/logo.svg" onerror="alert(2)" /><script>alert(3)</script>',
+			},
+			global: {
+				directives: {
+					n8nHtml,
+				},
+			},
+		});
+		expect(html()).toBe(
+			'<div><span>text</span><a href="https://malicious.com">malicious</a><img alt="Ok" src="./images/logo.svg"></div>',
+		);
+	});
+
+	it('should not touch safe html', async () => {
+		const { html } = render(TestComponent, {
+			props: {
+				html: '<span>text</span><a href="https://safe.com">safe</a><img alt="Ok" src="./images/logo.svg" />',
+			},
+			global: {
+				directives: {
+					n8nHtml,
+				},
+			},
+		});
+		expect(html()).toBe(
+			'<div><span>text</span><a href="https://safe.com">safe</a><img alt="Ok" src="./images/logo.svg"></div>',
+		);
+	});
+});
Original file line number	Diff line number	Diff line change
		@@ -1 +1,2 @@
		export { n8nTruncate } from './n8n-truncate';
		export { n8nHtml } from './n8n-html';