From 0468945c99f083577c4cc71f671b4b950f6aeb86 Mon Sep 17 00:00:00 2001 From: Csaba Tuncsik Date: Wed, 11 Dec 2024 10:11:51 +0100 Subject: [PATCH] fix(editor): Render sanitized HTML content in toast messages (#12139) --- .../editor-ui/src/components/NpsSurvey.vue | 1 - .../src/components/ParameterInputFull.vue | 1 - .../src/composables/useExecutionDebugging.ts | 2 +- .../src/composables/usePushConnection.test.ts | 1 - .../src/composables/usePushConnection.ts | 1 - .../src/composables/useToast.test.ts | 97 +++++++++++++------ .../editor-ui/src/composables/useToast.ts | 12 +-- .../src/composables/useWorkflowHelpers.ts | 1 - .../editor-ui/src/stores/settings.store.ts | 1 - packages/editor-ui/src/utils/htmlUtils.ts | 4 +- packages/editor-ui/src/views/NodeView.v2.vue | 2 - packages/editor-ui/src/views/NodeView.vue | 4 +- 12 files changed, 71 insertions(+), 56 deletions(-) diff --git a/packages/editor-ui/src/components/NpsSurvey.vue b/packages/editor-ui/src/components/NpsSurvey.vue index 818f7483f8..ce792ba20d 100644 --- a/packages/editor-ui/src/components/NpsSurvey.vue +++ b/packages/editor-ui/src/components/NpsSurvey.vue @@ -96,7 +96,6 @@ async function send() { message: Number(form.value.value) >= 8 ? i18n.baseText('prompts.npsSurvey.reviewUs') : '', type: 'success', duration: 15000, - dangerouslyUseHTMLString: true, }); setTimeout(() => { diff --git a/packages/editor-ui/src/components/ParameterInputFull.vue b/packages/editor-ui/src/components/ParameterInputFull.vue index 84b697f9da..332011b9ea 100644 --- a/packages/editor-ui/src/components/ParameterInputFull.vue +++ b/packages/editor-ui/src/components/ParameterInputFull.vue @@ -167,7 +167,6 @@ function onDrop(newParamValue: string) { title: i18n.baseText('dataMapping.success.title'), message: i18n.baseText('dataMapping.success.moreInfo'), type: 'success', - dangerouslyUseHTMLString: true, }); ndvStore.setMappingOnboarded(); diff --git a/packages/editor-ui/src/composables/useExecutionDebugging.ts b/packages/editor-ui/src/composables/useExecutionDebugging.ts index 632eac195b..19bb660d38 100644 --- a/packages/editor-ui/src/composables/useExecutionDebugging.ts +++ b/packages/editor-ui/src/composables/useExecutionDebugging.ts @@ -76,7 +76,7 @@ export const useExecutionDebugging = () => { type: 'warning', confirmButtonText: i18n.baseText('nodeView.confirmMessage.debug.confirmButtonText'), cancelButtonText: i18n.baseText('nodeView.confirmMessage.debug.cancelButtonText'), - dangerouslyUseHTMLString: true, + customClass: 'matching-pinned-nodes-confirmation', }, ); diff --git a/packages/editor-ui/src/composables/usePushConnection.test.ts b/packages/editor-ui/src/composables/usePushConnection.test.ts index c2901249d6..63592afd79 100644 --- a/packages/editor-ui/src/composables/usePushConnection.test.ts +++ b/packages/editor-ui/src/composables/usePushConnection.test.ts @@ -202,7 +202,6 @@ describe('usePushConnection()', () => { title: 'Problem in node ‘Last Node‘', type: 'error', duration: 0, - dangerouslyUseHTMLString: true, }); expect(result).toBeTruthy(); diff --git a/packages/editor-ui/src/composables/usePushConnection.ts b/packages/editor-ui/src/composables/usePushConnection.ts index 86ffbf358b..6305ba499c 100644 --- a/packages/editor-ui/src/composables/usePushConnection.ts +++ b/packages/editor-ui/src/composables/usePushConnection.ts @@ -397,7 +397,6 @@ export function usePushConnection({ router }: { router: ReturnType { - const original = await vi.importActual('element-plus'); - return { - ...original, - ElNotification: vi.fn(), - ElTooltip: vi.fn(), - }; -}); describe('useToast', () => { let toast: ReturnType; beforeEach(() => { - setActivePinia(createPinia()); + document.body.innerHTML = '
'; + createTestingPinia(); toast = useToast(); }); - afterEach(() => { - vi.restoreAllMocks(); - }); - - it('should show a message', () => { + it('should show a message', async () => { const messageData = { message: 'Test message', title: 'Test title' }; toast.showMessage(messageData); - expect(Notification).toHaveBeenCalledWith( - expect.objectContaining({ - message: 'Test message', - title: 'Test title', - }), - ); + await waitFor(() => { + expect(screen.getByRole('alert')).toBeVisible(); + expect( + within(screen.getByRole('alert')).getByRole('heading', { level: 2 }), + ).toHaveTextContent('Test title'); + expect(screen.getByRole('alert')).toContainHTML('

Test message

'); + }); }); - it('should sanitize message and title', () => { + it('should sanitize message and title', async () => { const messageData = { message: '', title: '', }; + toast.showMessage(messageData); + + await waitFor(() => { + expect(screen.getByRole('alert')).toBeVisible(); + expect( + within(screen.getByRole('alert')).getByRole('heading', { level: 2 }), + ).toHaveTextContent('alert("xss")'); + expect(screen.getByRole('alert')).toContainHTML('

alert("xss")

'); + }); + }); + + it('should sanitize but keep valid, allowed HTML tags', async () => { + const messageData = { + message: + 'Refresh to see the latest status.
More info or go to the Usage and plan settings page.', + title: 'Title', + }; toast.showMessage(messageData); - expect(Notification).toHaveBeenCalledWith( - expect.objectContaining({ - message: 'alert("xss")', - title: 'alert("xss")', - }), - ); + await waitFor(() => { + expect(screen.getByRole('alert')).toBeVisible(); + expect( + within(screen.getByRole('alert')).getByRole('heading', { level: 2 }), + ).toHaveTextContent('Title'); + expect( + within(screen.getByRole('alert')).getByRole('heading', { level: 2 }).querySelectorAll('*'), + ).toHaveLength(0); + expect(screen.getByRole('alert')).toContainHTML( + 'Refresh to see the latest status.
More info or go to the Usage and plan settings page.', + ); + }); + }); + + it('should render component as message, sanitized as well', async () => { + const messageData = { + message: h( + defineComponent({ + template: '

Test content

', + }), + ), + }; + + toast.showMessage(messageData); + + await waitFor(() => { + expect(screen.getByRole('alert')).toBeVisible(); + expect( + within(screen.getByRole('alert')).queryByRole('heading', { level: 2 }), + ).toHaveTextContent(''); + expect( + within(screen.getByRole('alert')).getByRole('heading', { level: 2 }).querySelectorAll('*'), + ).toHaveLength(0); + expect(screen.getByRole('alert')).toContainHTML('

Test content

'); + }); }); }); diff --git a/packages/editor-ui/src/composables/useToast.ts b/packages/editor-ui/src/composables/useToast.ts index 365a72e3fe..764215cf70 100644 --- a/packages/editor-ui/src/composables/useToast.ts +++ b/packages/editor-ui/src/composables/useToast.ts @@ -33,7 +33,7 @@ export function useToast() { const canvasStore = useCanvasStore(); const messageDefaults: Partial> = { - dangerouslyUseHTMLString: false, + dangerouslyUseHTMLString: true, position: 'bottom-right', zIndex: APP_Z_INDEXES.TOASTS, // above NDV and modal overlays offset: settingsStore.isAiAssistantEnabled || workflowsStore.isChatPanelOpen ? 64 : 0, @@ -82,7 +82,6 @@ export function useToast() { customClass?: string; closeOnClick?: boolean; type?: MessageBoxState['type']; - dangerouslyUseHTMLString?: boolean; }) { // eslint-disable-next-line prefer-const let notification: NotificationHandle; @@ -107,7 +106,6 @@ export function useToast() { duration: config.duration, customClass: config.customClass, type: config.type, - dangerouslyUseHTMLString: config.dangerouslyUseHTMLString ?? true, }); return notification; @@ -145,7 +143,6 @@ export function useToast() { ${collapsableDetails(error)}`, type: 'error', duration: 0, - dangerouslyUseHTMLString: true, }, false, ); @@ -165,12 +162,6 @@ export function useToast() { }); } - function showAlert(config: NotificationOptions): NotificationHandle { - return Notification({ - ...config, - }); - } - function causedByCredential(message: string | undefined) { if (!message || typeof message !== 'string') return false; @@ -209,7 +200,6 @@ export function useToast() { showMessage, showToast, showError, - showAlert, clearAllStickyNotifications, showNotificationForViews, }; diff --git a/packages/editor-ui/src/composables/useWorkflowHelpers.ts b/packages/editor-ui/src/composables/useWorkflowHelpers.ts index ca934cf2cf..789540a5bc 100644 --- a/packages/editor-ui/src/composables/useWorkflowHelpers.ts +++ b/packages/editor-ui/src/composables/useWorkflowHelpers.ts @@ -882,7 +882,6 @@ export function useWorkflowHelpers(options: { router: ReturnType { message: i18n.baseText('startupError.message'), type: 'error', duration: 0, - dangerouslyUseHTMLString: true, }); throw e; diff --git a/packages/editor-ui/src/utils/htmlUtils.ts b/packages/editor-ui/src/utils/htmlUtils.ts index cf4bb85c6c..f1d6119adb 100644 --- a/packages/editor-ui/src/utils/htmlUtils.ts +++ b/packages/editor-ui/src/utils/htmlUtils.ts @@ -18,8 +18,8 @@ export function sanitizeHtml(dirtyHtml: string) { } if (ALLOWED_HTML_ATTRIBUTES.includes(name) || name.startsWith('data-')) { - // href is allowed but we need to sanitize certain protocols - if (name === 'href' && !value.match(/^https?:\/\//gm)) { + // href is allowed but we allow only https and relative URLs + if (name === 'href' && !value.match(/^https?:\/\//gm) && !value.startsWith('/')) { return ''; } return `${name}="${escapeAttrValue(value)}"`; diff --git a/packages/editor-ui/src/views/NodeView.v2.vue b/packages/editor-ui/src/views/NodeView.v2.vue index 4f58b52294..e8a4c8b67f 100644 --- a/packages/editor-ui/src/views/NodeView.v2.vue +++ b/packages/editor-ui/src/views/NodeView.v2.vue @@ -587,7 +587,6 @@ async function onClipboardPaste(plainTextData: string): Promise { cancelButtonText: i18n.baseText( 'nodeView.confirmMessage.onClipboardPasteEvent.cancelButtonText', ), - dangerouslyUseHTMLString: true, }, ); @@ -1368,7 +1367,6 @@ function checkIfEditingIsAllowed(): boolean { : 'readOnly.showMessage.executions.message', ), type: 'info', - dangerouslyUseHTMLString: true, }) as unknown as { visible: boolean }; return false; diff --git a/packages/editor-ui/src/views/NodeView.vue b/packages/editor-ui/src/views/NodeView.vue index f802cc3cd1..b200903eac 100644 --- a/packages/editor-ui/src/views/NodeView.vue +++ b/packages/editor-ui/src/views/NodeView.vue @@ -825,7 +825,7 @@ export default defineComponent({ : 'readOnly.showMessage.executions.message', ), type: 'info', - dangerouslyUseHTMLString: true, + onClose: () => { this.readOnlyNotification = null; }, @@ -934,7 +934,6 @@ export default defineComponent({ // Close the creator panel if user clicked on the link if (this.createNodeActive) notice.close(); }, 0), - dangerouslyUseHTMLString: true, }); }, async clearExecutionData() { @@ -1827,7 +1826,6 @@ export default defineComponent({ cancelButtonText: this.i18n.baseText( 'nodeView.confirmMessage.onClipboardPasteEvent.cancelButtonText', ), - dangerouslyUseHTMLString: true, }, );