Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 26 commits into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
c93e33c
fix(editor): Replace v-html with custom directive to sanitize content
cstuncsik Sep 13, 2024
4e4c4fd
fix(editor): Remove linter comments and update test snapsho
cstuncsik Sep 13, 2024
e21cd28
fix(editor): Extend unit test
cstuncsik Sep 13, 2024
237859d
Revert "fix(editor): Remove linter comments and update test snapsho"
cstuncsik Sep 13, 2024
27bc0d6
fix(editor): Fix unit test
cstuncsik Sep 13, 2024
14da392
fix(editor): Fix unit test
cstuncsik Sep 13, 2024
dbe9494
fix(editor): Fix unit test
cstuncsik Sep 13, 2024
63ab993
fix(editor): Fix unit test
cstuncsik Sep 16, 2024
0c8e8dd
Merge remote-tracking branch 'origin/master' into replace-v-html
cstuncsik Sep 16, 2024
858ca9b
chore: update lock file after conflicts
cstuncsik Sep 16, 2024
564d3cc
fix(editor): Fix unit test
cstuncsik Sep 17, 2024
892485c
fix(editor): Fix lint
cstuncsik Sep 17, 2024
2a50658
fix(editor): Fix lint
cstuncsik Sep 17, 2024
277615b
fix(editor): Fix typing
cstuncsik Sep 17, 2024
83574a7
fix(editor): Update unit test wording
cstuncsik Sep 17, 2024
ebaca47
fix(editor): Fix unit test
cstuncsik Sep 17, 2024
baeda69
fix(editor): Fix directive
cstuncsik Sep 17, 2024
a1d66d0
Merge remote-tracking branch 'origin/master' into replace-v-html
cstuncsik Sep 17, 2024
7576dc7
Merge remote-tracking branch 'origin/master' into replace-v-html
cstuncsik Sep 17, 2024
1d88fb8
fix(editor): Add usage comment
cstuncsik Sep 17, 2024
c0ca692
Merge remote-tracking branch 'origin/master' into replace-v-html
cstuncsik Sep 17, 2024
fb0bcf2
chore: update lock file after conflicts
cstuncsik Sep 17, 2024
6d5c8ac
Merge remote-tracking branch 'origin/master' into replace-v-html
cstuncsik Sep 17, 2024
d31540f
fix(editor): Enable code tag class when sanitizing
cstuncsik Sep 17, 2024
417b702
fix(editor): Enable data-* attr on links
cstuncsik Sep 17, 2024
6e2282f
Merge branch 'master' into replace-v-html
tomi Sep 18, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion packages/design-system/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'"
Expand Down
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand All @@ -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: [
Expand Down Expand Up @@ -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: [
Expand All @@ -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: [
Expand All @@ -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: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
>
Expand All @@ -145,7 +142,6 @@ exports[`AskAssistantChat > renders chat with messages correctly 1`] = `


</div>

<!--v-if-->
<!--v-if-->
</div>
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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"
>
Expand All @@ -1076,7 +1067,6 @@ exports[`AskAssistantChat > renders end of session chat correctly 1`] = `


</div>

<!--v-if-->
<!--v-if-->
</div>
Expand Down Expand Up @@ -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"
>
Expand All @@ -1325,7 +1312,6 @@ exports[`AskAssistantChat > renders message with code snippet 1`] = `


</div>

<div
class="code-snippet"
data-test-id="assistant-code-snippet"
Expand Down Expand Up @@ -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"
>
Expand All @@ -1568,7 +1551,6 @@ exports[`AskAssistantChat > renders streaming chat correctly 1`] = `


</div>

<!--v-if-->
<!--v-if-->
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand Down
Original file line number Diff line number Diff line change
@@ -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',
},
Expand All @@ -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',
},
Expand All @@ -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',
},
Expand All @@ -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',
Expand Down
2 changes: 1 addition & 1 deletion packages/design-system/src/components/N8nNotice/Notice.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand Down Expand Up @@ -41,6 +42,9 @@ describe('components', () => {
content: '<strong>Hello world!</strong> This is a notice.',
},
global: {
directives: {
n8nHtml,
},
components: {
'n8n-text': N8nText,
},
Expand Down
2 changes: 1 addition & 1 deletion packages/design-system/src/components/N8nSticky/Sticky.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
2 changes: 1 addition & 1 deletion packages/design-system/src/components/N8nTabs/Tabs.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
1 change: 1 addition & 0 deletions packages/design-system/src/directives/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { n8nHtml } from './n8n-html';
45 changes: 45 additions & 0 deletions packages/design-system/src/directives/n8n-html.test.ts
Original file line number Diff line number Diff line change
@@ -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 () => {
cstuncsik marked this conversation as resolved.
Show resolved Hide resolved
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://malicious.com">malicious</a><img alt="Ok" src="./images/logo.svg" />',
cstuncsik marked this conversation as resolved.
Show resolved Hide resolved
},
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>',
);
});
});
Loading