fix(editor): Render sanitized HTML content in toast messages (#12139)

This commit is contained in:
Csaba Tuncsik 2024-12-11 10:11:51 +01:00 committed by GitHub
parent ed359586c8
commit 0468945c99
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 71 additions and 56 deletions

View file

@ -96,7 +96,6 @@ async function send() {
message: Number(form.value.value) >= 8 ? i18n.baseText('prompts.npsSurvey.reviewUs') : '', message: Number(form.value.value) >= 8 ? i18n.baseText('prompts.npsSurvey.reviewUs') : '',
type: 'success', type: 'success',
duration: 15000, duration: 15000,
dangerouslyUseHTMLString: true,
}); });
setTimeout(() => { setTimeout(() => {

View file

@ -167,7 +167,6 @@ function onDrop(newParamValue: string) {
title: i18n.baseText('dataMapping.success.title'), title: i18n.baseText('dataMapping.success.title'),
message: i18n.baseText('dataMapping.success.moreInfo'), message: i18n.baseText('dataMapping.success.moreInfo'),
type: 'success', type: 'success',
dangerouslyUseHTMLString: true,
}); });
ndvStore.setMappingOnboarded(); ndvStore.setMappingOnboarded();

View file

@ -76,7 +76,7 @@ export const useExecutionDebugging = () => {
type: 'warning', type: 'warning',
confirmButtonText: i18n.baseText('nodeView.confirmMessage.debug.confirmButtonText'), confirmButtonText: i18n.baseText('nodeView.confirmMessage.debug.confirmButtonText'),
cancelButtonText: i18n.baseText('nodeView.confirmMessage.debug.cancelButtonText'), cancelButtonText: i18n.baseText('nodeView.confirmMessage.debug.cancelButtonText'),
dangerouslyUseHTMLString: true,
customClass: 'matching-pinned-nodes-confirmation', customClass: 'matching-pinned-nodes-confirmation',
}, },
); );

View file

@ -202,7 +202,6 @@ describe('usePushConnection()', () => {
title: 'Problem in node Last Node', title: 'Problem in node Last Node',
type: 'error', type: 'error',
duration: 0, duration: 0,
dangerouslyUseHTMLString: true,
}); });
expect(result).toBeTruthy(); expect(result).toBeTruthy();

View file

@ -397,7 +397,6 @@ export function usePushConnection({ router }: { router: ReturnType<typeof useRou
message: runDataExecutedErrorMessage, message: runDataExecutedErrorMessage,
type: 'error', type: 'error',
duration: 0, duration: 0,
dangerouslyUseHTMLString: true,
}); });
} }
} }

View file

@ -1,55 +1,90 @@
import { describe, it, expect, vi, beforeEach } from 'vitest'; import { screen, waitFor, within } from '@testing-library/vue';
import { createTestingPinia } from '@pinia/testing';
import { h, defineComponent } from 'vue';
import { useToast } from './useToast'; import { useToast } from './useToast';
import { createPinia, setActivePinia } from 'pinia';
import { ElNotification as Notification } from 'element-plus';
vi.mock('element-plus', async () => {
const original = await vi.importActual('element-plus');
return {
...original,
ElNotification: vi.fn(),
ElTooltip: vi.fn(),
};
});
describe('useToast', () => { describe('useToast', () => {
let toast: ReturnType<typeof useToast>; let toast: ReturnType<typeof useToast>;
beforeEach(() => { beforeEach(() => {
setActivePinia(createPinia()); document.body.innerHTML = '<div id="app-grid"></div>';
createTestingPinia();
toast = useToast(); toast = useToast();
}); });
afterEach(() => { it('should show a message', async () => {
vi.restoreAllMocks();
});
it('should show a message', () => {
const messageData = { message: 'Test message', title: 'Test title' }; const messageData = { message: 'Test message', title: 'Test title' };
toast.showMessage(messageData); toast.showMessage(messageData);
expect(Notification).toHaveBeenCalledWith( await waitFor(() => {
expect.objectContaining({ expect(screen.getByRole('alert')).toBeVisible();
message: 'Test message', expect(
title: 'Test title', within(screen.getByRole('alert')).getByRole('heading', { level: 2 }),
}), ).toHaveTextContent('Test title');
); expect(screen.getByRole('alert')).toContainHTML('<p>Test message</p>');
});
}); });
it('should sanitize message and title', () => { it('should sanitize message and title', async () => {
const messageData = { const messageData = {
message: '<script>alert("xss")</script>', message: '<script>alert("xss")</script>',
title: '<script>alert("xss")</script>', title: '<script>alert("xss")</script>',
}; };
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('<p>alert("xss")</p>');
});
});
it('should sanitize but keep valid, allowed HTML tags', async () => {
const messageData = {
message:
'<a data-action="reload">Refresh</a> to see the <strong>latest status</strong>.<br/> <a href="https://docs.n8n.io/integrations/builtin/core-nodes/n8n-nodes-base.wait/" target="_blank">More info</a> or go to the <a href="/settings/usage">Usage and plan</a> settings page.',
title: '<strong>Title</strong>',
};
toast.showMessage(messageData); toast.showMessage(messageData);
expect(Notification).toHaveBeenCalledWith( await waitFor(() => {
expect.objectContaining({ expect(screen.getByRole('alert')).toBeVisible();
message: 'alert("xss")', expect(
title: 'alert("xss")', 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(
'<a data-action="reload">Refresh</a> to see the <strong>latest status</strong>.<br /> <a href="https://docs.n8n.io/integrations/builtin/core-nodes/n8n-nodes-base.wait/" target="_blank">More info</a> or go to the <a href="/settings/usage">Usage and plan</a> settings page.',
);
});
});
it('should render component as message, sanitized as well', async () => {
const messageData = {
message: h(
defineComponent({
template: '<p>Test <strong>content</strong><script>alert("xss")</script></p>',
}),
),
};
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('<p>Test <strong>content</strong></p>');
});
}); });
}); });

View file

@ -33,7 +33,7 @@ export function useToast() {
const canvasStore = useCanvasStore(); const canvasStore = useCanvasStore();
const messageDefaults: Partial<Omit<NotificationOptions, 'message'>> = { const messageDefaults: Partial<Omit<NotificationOptions, 'message'>> = {
dangerouslyUseHTMLString: false, dangerouslyUseHTMLString: true,
position: 'bottom-right', position: 'bottom-right',
zIndex: APP_Z_INDEXES.TOASTS, // above NDV and modal overlays zIndex: APP_Z_INDEXES.TOASTS, // above NDV and modal overlays
offset: settingsStore.isAiAssistantEnabled || workflowsStore.isChatPanelOpen ? 64 : 0, offset: settingsStore.isAiAssistantEnabled || workflowsStore.isChatPanelOpen ? 64 : 0,
@ -82,7 +82,6 @@ export function useToast() {
customClass?: string; customClass?: string;
closeOnClick?: boolean; closeOnClick?: boolean;
type?: MessageBoxState['type']; type?: MessageBoxState['type'];
dangerouslyUseHTMLString?: boolean;
}) { }) {
// eslint-disable-next-line prefer-const // eslint-disable-next-line prefer-const
let notification: NotificationHandle; let notification: NotificationHandle;
@ -107,7 +106,6 @@ export function useToast() {
duration: config.duration, duration: config.duration,
customClass: config.customClass, customClass: config.customClass,
type: config.type, type: config.type,
dangerouslyUseHTMLString: config.dangerouslyUseHTMLString ?? true,
}); });
return notification; return notification;
@ -145,7 +143,6 @@ export function useToast() {
${collapsableDetails(error)}`, ${collapsableDetails(error)}`,
type: 'error', type: 'error',
duration: 0, duration: 0,
dangerouslyUseHTMLString: true,
}, },
false, false,
); );
@ -165,12 +162,6 @@ export function useToast() {
}); });
} }
function showAlert(config: NotificationOptions): NotificationHandle {
return Notification({
...config,
});
}
function causedByCredential(message: string | undefined) { function causedByCredential(message: string | undefined) {
if (!message || typeof message !== 'string') return false; if (!message || typeof message !== 'string') return false;
@ -209,7 +200,6 @@ export function useToast() {
showMessage, showMessage,
showToast, showToast,
showError, showError,
showAlert,
clearAllStickyNotifications, clearAllStickyNotifications,
showNotificationForViews, showNotificationForViews,
}; };

View file

@ -882,7 +882,6 @@ export function useWorkflowHelpers(options: { router: ReturnType<typeof useRoute
}), }),
i18n.baseText('workflows.concurrentChanges.confirmMessage.title'), i18n.baseText('workflows.concurrentChanges.confirmMessage.title'),
{ {
dangerouslyUseHTMLString: true,
confirmButtonText: i18n.baseText( confirmButtonText: i18n.baseText(
'workflows.concurrentChanges.confirmMessage.confirmButtonText', 'workflows.concurrentChanges.confirmMessage.confirmButtonText',
), ),

View file

@ -278,7 +278,6 @@ export const useSettingsStore = defineStore(STORES.SETTINGS, () => {
message: i18n.baseText('startupError.message'), message: i18n.baseText('startupError.message'),
type: 'error', type: 'error',
duration: 0, duration: 0,
dangerouslyUseHTMLString: true,
}); });
throw e; throw e;

View file

@ -18,8 +18,8 @@ export function sanitizeHtml(dirtyHtml: string) {
} }
if (ALLOWED_HTML_ATTRIBUTES.includes(name) || name.startsWith('data-')) { if (ALLOWED_HTML_ATTRIBUTES.includes(name) || name.startsWith('data-')) {
// href is allowed but we need to sanitize certain protocols // href is allowed but we allow only https and relative URLs
if (name === 'href' && !value.match(/^https?:\/\//gm)) { if (name === 'href' && !value.match(/^https?:\/\//gm) && !value.startsWith('/')) {
return ''; return '';
} }
return `${name}="${escapeAttrValue(value)}"`; return `${name}="${escapeAttrValue(value)}"`;

View file

@ -587,7 +587,6 @@ async function onClipboardPaste(plainTextData: string): Promise<void> {
cancelButtonText: i18n.baseText( cancelButtonText: i18n.baseText(
'nodeView.confirmMessage.onClipboardPasteEvent.cancelButtonText', 'nodeView.confirmMessage.onClipboardPasteEvent.cancelButtonText',
), ),
dangerouslyUseHTMLString: true,
}, },
); );
@ -1368,7 +1367,6 @@ function checkIfEditingIsAllowed(): boolean {
: 'readOnly.showMessage.executions.message', : 'readOnly.showMessage.executions.message',
), ),
type: 'info', type: 'info',
dangerouslyUseHTMLString: true,
}) as unknown as { visible: boolean }; }) as unknown as { visible: boolean };
return false; return false;

View file

@ -825,7 +825,7 @@ export default defineComponent({
: 'readOnly.showMessage.executions.message', : 'readOnly.showMessage.executions.message',
), ),
type: 'info', type: 'info',
dangerouslyUseHTMLString: true,
onClose: () => { onClose: () => {
this.readOnlyNotification = null; this.readOnlyNotification = null;
}, },
@ -934,7 +934,6 @@ export default defineComponent({
// Close the creator panel if user clicked on the link // Close the creator panel if user clicked on the link
if (this.createNodeActive) notice.close(); if (this.createNodeActive) notice.close();
}, 0), }, 0),
dangerouslyUseHTMLString: true,
}); });
}, },
async clearExecutionData() { async clearExecutionData() {
@ -1827,7 +1826,6 @@ export default defineComponent({
cancelButtonText: this.i18n.baseText( cancelButtonText: this.i18n.baseText(
'nodeView.confirmMessage.onClipboardPasteEvent.cancelButtonText', 'nodeView.confirmMessage.onClipboardPasteEvent.cancelButtonText',
), ),
dangerouslyUseHTMLString: true,
}, },
); );