feat(editor): Implement HTML sanitization for Notification and Message components (#4081)

* feat(editor): Implement HTML sanitization when using `dangerouslyUseHTMLString` option of Notification and Message components

* 🐛 Implement mechanism to allow for A href actions from locale strings

* 🐛 Prevent link action default

* ♻️ Use `xss` library instead of `sanitize-html` to handle sanitization

* 🔥 Remove `onLinkClick` functionality of `$showMessage`

packages/editor-ui/src/App.vue

@@ -30,11 +30,13 @@ import { mapGetters } from 'vuex';
 import { userHelpers } from './components/mixins/userHelpers';
 import { addHeaders, loadLanguage } from './plugins/i18n';
 import { restApi } from '@/components/mixins/restApi';
+import { globalLinkActions } from '@/components/mixins/globalLinkActions';
 
 export default mixins(
 	showMessage,
 	userHelpers,
 	restApi,
+	globalLinkActions,
 ).extend({
 	name: 'App',
 	components: {

packages/editor-ui/src/components/PageAlert.vue

@@ -7,6 +7,7 @@ import mixins from 'vue-typed-mixins';
 
 import { showMessage } from './mixins/showMessage';
 import { ElMessageComponent } from 'element-ui/types/message';
+import { sanitizeHtml } from '@/utils';
 
 export default mixins(
 	showMessage,
@@ -28,7 +29,7 @@ export default mixins(
 	},
 	mounted() {
 		this.alert = this.$showAlert({
-			message: this.message,
+			message: sanitizeHtml(this.message),
 			type: 'warning',
 			duration: 0,
 			showClose: true,

packages/editor-ui/src/components/mixins/globalLinkActions.ts

@@ -0,0 +1,48 @@
+/**
+ * Creates event listeners for `data-action` attribute to allow for actions to be called from locale without using
+ * unsafe onclick attribute
+ */
+ import Vue from 'vue';
+
+ export const globalLinkActions = Vue.extend({
+	data(): {[key: string]: {[key: string]: Function}} {
+		return {
+			customActions: {},
+		};
+	},
+	mounted() {
+		window.addEventListener('click', this.delegateClick);
+		this.$root.$on('registerGlobalLinkAction', this.registerCustomAction);
+	},
+	destroyed() {
+		window.removeEventListener('click', this.delegateClick);
+		this.$root.$off('registerGlobalLinkAction', this.registerCustomAction);
+	},
+	computed: {
+		availableActions(): {[key: string]: Function} {
+			return {
+				reload: this.reload,
+				...this.customActions,
+			};
+		},
+	},
+	methods: {
+		registerCustomAction(key: string, action: Function) {
+			this.customActions[key] = action;
+		},
+		delegateClick(e: MouseEvent) {
+			const clickedElement = e.target;
+			if (!(clickedElement instanceof Element) || clickedElement.tagName !== 'A') return;
+
+			const actionAttribute = clickedElement.getAttribute('data-action');
+			if(actionAttribute && typeof this.availableActions[actionAttribute] === 'function') {
+				e.preventDefault();
+				this.availableActions[actionAttribute]();
+			}
+		},
+		reload() {
+			window.location.reload();
+		},
+	},
+ });
+

packages/editor-ui/src/components/mixins/pushConnection.ts

@@ -233,7 +233,12 @@ export const pushConnection = mixins(
 
 						let action;
 						if (!isSavingExecutions) {
-							action = '<a class="open-settings">Turn on saving manual executions</a> and run again to see what happened after this node.';
+							this.$root.$emit('registerGlobalLinkAction', 'open-settings', async () => {
+								if (this.$store.getters.isNewWorkflow) await this.saveAsNewWorkflow();
+								this.$store.dispatch('ui/openModal', WORKFLOW_SETTINGS_MODAL_KEY);
+							});
+
+							action = '<a data-action="open-settings">Turn on saving manual executions</a> and run again to see what happened after this node.';
 						}
 						else {
 							action = `<a href="/execution/${activeExecutionId}" target="_blank">View the execution</a> to see what happened after this node.`;
@@ -246,14 +251,6 @@ export const pushConnection = mixins(
 							message: `${action} <a href="https://docs.n8n.io/nodes/n8n-nodes-base.wait/" target="_blank">More info</a>`,
 							type: 'success',
 							duration: 0,
-							onLinkClick: async (e: HTMLLinkElement) => {
-								if (e.classList.contains('open-settings')) {
-									if (this.$store.getters.isNewWorkflow) {
-										await this.saveAsNewWorkflow();
-									}
-									this.$store.dispatch('ui/openModal', WORKFLOW_SETTINGS_MODAL_KEY);
-								}
-							},
 						});
 					} else if (runDataExecuted.finished !== true) {
 						this.$titleSet(workflow.name as string, 'ERROR');

packages/editor-ui/src/components/mixins/showMessage.ts

@@ -7,6 +7,7 @@ import { ExecutionError } from 'n8n-workflow';
 import { ElMessageBoxOptions } from 'element-ui/types/message-box';
 import { ElMessage, ElMessageComponent, ElMessageOptions, MessageType } from 'element-ui/types/message';
 import { isChildOf } from './helpers';
+import { sanitizeHtml } from '@/utils';
 
 let stickyNotificationQueue: ElNotificationComponent[] = [];
 
@@ -17,6 +18,8 @@ export const showMessage = mixins(externalHooks).extend({
 			track = true,
 		) {
 			messageData.dangerouslyUseHTMLString = true;
+			messageData.message = messageData.message ? sanitizeHtml(messageData.message) : messageData.message;
+
 			if (messageData.position === undefined) {
 				messageData.position = 'bottom-right';
 			}
@@ -47,7 +50,6 @@ export const showMessage = mixins(externalHooks).extend({
 				duration?: number,
 				customClass?: string,
 				closeOnClick?: boolean,
-				onLinkClick?: (e: HTMLLinkElement) => void,
 				type?: MessageType,
 			}) {
 			// eslint-disable-next-line prefer-const
@@ -64,26 +66,6 @@ export const showMessage = mixins(externalHooks).extend({
 				};
 			}
 
-			if (config.onLinkClick) {
-				const onLinkClick = (e: MouseEvent) => {
-					if (e && e.target && config.onLinkClick && isChildOf(notification.$el, e.target as Element)) {
-						const target = e.target as HTMLElement;
-						if (target && target.tagName === 'A') {
-							config.onLinkClick(e.target as HTMLLinkElement);
-						}
-					}
-				};
-				window.addEventListener('click', onLinkClick);
-
-				const cb = config.onClose;
-				config.onClose = () => {
-					window.removeEventListener('click', onLinkClick);
-					if (cb) {
-						cb();
-					}
-				};
-			}
-
 			notification = this.$showMessage({
 				title: config.title,
 				message: config.message,
@@ -159,7 +141,8 @@ export const showMessage = mixins(externalHooks).extend({
 					...(type && { type }),
 				};
 
-				await this.$confirm(message, headline, options);
+				const sanitizedMessage = sanitizeHtml(message);
+				await this.$confirm(sanitizedMessage, headline, options);
 				return true;
 			} catch (e) {
 				return false;
@@ -176,7 +159,8 @@ export const showMessage = mixins(externalHooks).extend({
 					...(type && { type }),
 				};
 
-				await this.$confirm(message, headline, options);
+				const sanitizedMessage = sanitizeHtml(message);
+				await this.$confirm(sanitizedMessage, headline, options);
 				return 'confirmed';
 			} catch (e) {
 				return e as string;

packages/editor-ui/src/plugins/i18n/locales/en.json

@@ -836,7 +836,7 @@
 	"showMessage.ok": "OK",
 	"showMessage.showDetails": "Show Details",
 	"startupError": "Error connecting to n8n",
-	"startupError.message": "Could not connect to server. <a onclick='window.location.reload(false);'>Refresh</a> to try again",
+	"startupError.message": "Could not connect to server. <a data-action='reload'>Refresh</a> to try again",
 	"tagsDropdown.createTag": "Create tag \"{filter}\"",
 	"tagsDropdown.manageTags": "Manage tags",
 	"tagsDropdown.noMatchingTagsExist": "No matching tags exist",

packages/editor-ui/src/utils.ts

@@ -1,3 +1,5 @@
+import xss, { friendlyAttrValue } from 'xss';
+
 export const omit = (keyToOmit: string, { [keyToOmit]: _, ...remainder }) => remainder;
 
 export function isObjectLiteral(maybeObject: unknown): maybeObject is { [key: string]: string } {
@@ -12,3 +14,33 @@ export function isJsonKeyObject(item: unknown): item is {
 
 	return Object.keys(item).includes('json');
 }
+
+export function sanitizeHtml(dirtyHtml: string) {
+	const allowedAttributes = ['href','name', 'target', 'title', 'class', 'id'];
+	const allowedTags = ['p', 'strong', 'b', 'code', 'a', 'br', 'i', 'em', 'small' ];
+
+	const sanitizedHtml = xss(dirtyHtml, {
+		onTagAttr: (tag, name, value) => {
+			if (tag === 'img' && name === 'src') {
+				// Only allow http requests to supported image files from the `static` directory
+				const isImageFile = value.split('#')[0].match(/\.(jpeg|jpg|gif|png|webp)$/) !== null;
+				const isStaticImageFile = isImageFile && value.startsWith('/static/');
+				if (!value.startsWith('https://') && !isStaticImageFile) {
+					return '';
+				}
+			}
+
+			// Allow `allowedAttributes` and all `data-*` attributes
+			if(allowedAttributes.includes(name) || name.startsWith('data-')) return `${name}="${friendlyAttrValue(value)}"`;
+
+			return;
+			// Return nothing, means keep the default handling measure
+		},
+		onTag: (tag) => {
+			if(!allowedTags.includes(tag)) return '';
+			return;
+		},
+	});
+
+	return sanitizedHtml;
+}

packages/editor-ui/src/views/NodeView.vue

@@ -634,7 +634,7 @@ export default mixins(
 				if ((data as IExecutionsSummary).waitTill) {
 					this.$showMessage({
 						title: this.$locale.baseText('nodeView.thisExecutionHasntFinishedYet'),
-						message: `<a onclick="window.location.reload(false);">${this.$locale.baseText('nodeView.refresh')}</a> ${this.$locale.baseText('nodeView.toSeeTheLatestStatus')}.<br/> <a href="https://docs.n8n.io/nodes/n8n-nodes-base.wait/" target="_blank">${this.$locale.baseText('nodeView.moreInfo')}</a>`,
+						message: `<a data-action="reload">${this.$locale.baseText('nodeView.refresh')}</a> ${this.$locale.baseText('nodeView.toSeeTheLatestStatus')}.<br/> <a href="https://docs.n8n.io/nodes/n8n-nodes-base.wait/" target="_blank">${this.$locale.baseText('nodeView.moreInfo')}</a>`,
 						type: 'warning',
 						duration: 0,
 					});