From d7445a71db174f7748d177326cafa046e63b75b0 Mon Sep 17 00:00:00 2001 From: Filipe Dobreira Date: Fri, 22 Mar 2024 15:13:48 +0000 Subject: [PATCH 1/5] fix: add html ruleset for markdown tiles --- .../common/src/utils/sanitizeHtml.test.ts | 16 +++++++++++++- packages/common/src/utils/sanitizeHtml.ts | 21 ++++++++++++++++--- 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/packages/common/src/utils/sanitizeHtml.test.ts b/packages/common/src/utils/sanitizeHtml.test.ts index 4490cd413029..868c75d26a2c 100644 --- a/packages/common/src/utils/sanitizeHtml.test.ts +++ b/packages/common/src/utils/sanitizeHtml.test.ts @@ -1,4 +1,7 @@ -import { sanitizeHtml } from './sanitizeHtml'; +import { + HTML_SANITIZE_MARKDOWN_TILE_RULES, + sanitizeHtml, +} from './sanitizeHtml'; describe('sanitizeHtml', () => { test('empty input', () => { @@ -29,6 +32,17 @@ describe('sanitizeHtml', () => { ).toEqual('Lightdash'); }); + test('iframe tag with markdown tile rule set', () => { + expect( + sanitizeHtml( + '', + HTML_SANITIZE_MARKDOWN_TILE_RULES, + ), + ).toEqual( + '', + ); + }); + test('style tag in span is preserved', () => { expect( sanitizeHtml('@Foo'), diff --git a/packages/common/src/utils/sanitizeHtml.ts b/packages/common/src/utils/sanitizeHtml.ts index ef505846df95..e1e0f64e2ff2 100644 --- a/packages/common/src/utils/sanitizeHtml.ts +++ b/packages/common/src/utils/sanitizeHtml.ts @@ -4,7 +4,7 @@ import sanitize from 'sanitize-html'; * If you want to modify sanitization settings, be sure to merge them * with the sane defaults pre-included with sanitize-html. */ -const HTML_SANITIZE_OPTIONS: sanitize.IOptions = { +export const HTML_SANITIZE_DEFAULT_RULES: sanitize.IOptions = { ...sanitize.defaults, allowedAttributes: { @@ -14,5 +14,20 @@ const HTML_SANITIZE_OPTIONS: sanitize.IOptions = { }, }; -export const sanitizeHtml = (input: string): string => - sanitize(input, HTML_SANITIZE_OPTIONS); +/** + * Adjusted html sanitization rules for markdown tiles, mainly to + * allow iframes to be used. + */ +export const HTML_SANITIZE_MARKDOWN_TILE_RULES: sanitize.IOptions = { + ...HTML_SANITIZE_DEFAULT_RULES, + allowedTags: [...(HTML_SANITIZE_DEFAULT_RULES.allowedTags || []), 'iframe'], + allowedAttributes: { + ...HTML_SANITIZE_DEFAULT_RULES.allowedAttributes, + iframe: ['width', 'height', 'src', 'name'], + }, +}; + +export const sanitizeHtml = ( + input: string, + ruleSet: sanitize.IOptions = HTML_SANITIZE_DEFAULT_RULES, +): string => sanitize(input, ruleSet); From dac0a93b3503cab67b1049a304629bffbecdf356 Mon Sep 17 00:00:00 2001 From: Filipe Dobreira Date: Fri, 22 Mar 2024 16:17:23 +0000 Subject: [PATCH 2/5] fix: sanitize input to DashboardModel.createVersion for Markdown tiles --- .../backend/src/models/DashboardModel/DashboardModel.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/backend/src/models/DashboardModel/DashboardModel.ts b/packages/backend/src/models/DashboardModel/DashboardModel.ts index 9e4f625bbbd6..d83c17e20c77 100644 --- a/packages/backend/src/models/DashboardModel/DashboardModel.ts +++ b/packages/backend/src/models/DashboardModel/DashboardModel.ts @@ -10,8 +10,10 @@ import { DashboardTileTypes, DashboardUnversionedFields, DashboardVersionedFields, + HTML_SANITIZE_MARKDOWN_TILE_RULES, LightdashUser, NotFoundError, + sanitizeHtml, SavedChart, SessionUser, UnexpectedServerError, @@ -169,7 +171,10 @@ export class DashboardModel { dashboard_version_id: versionId.dashboard_version_id, dashboard_tile_uuid: insertedTile.dashboard_tile_uuid, title: tile.properties.title, - content: tile.properties.content, + content: sanitizeHtml( + tile.properties.content, + HTML_SANITIZE_MARKDOWN_TILE_RULES, + ), }); break; case DashboardTileTypes.LOOM: From e16ae97d15c8d059990b916acb592750c63310e5 Mon Sep 17 00:00:00 2001 From: Filipe Dobreira Date: Fri, 22 Mar 2024 16:34:16 +0000 Subject: [PATCH 3/5] fix: preprocess markdown tile input in frontend --- .../DashboardTiles/TileForms/MarkdownTileForm.tsx | 14 +++++++++++++- .../DashboardTiles/TileForms/TileAddModal.tsx | 14 +++++++++++++- .../DashboardTiles/TileForms/TileUpdateModal.tsx | 14 +++++++++++++- 3 files changed, 39 insertions(+), 3 deletions(-) diff --git a/packages/frontend/src/components/DashboardTiles/TileForms/MarkdownTileForm.tsx b/packages/frontend/src/components/DashboardTiles/TileForms/MarkdownTileForm.tsx index ab6b9744c820..badee7e2b914 100644 --- a/packages/frontend/src/components/DashboardTiles/TileForms/MarkdownTileForm.tsx +++ b/packages/frontend/src/components/DashboardTiles/TileForms/MarkdownTileForm.tsx @@ -1,4 +1,9 @@ -import { type DashboardMarkdownTileProperties } from '@lightdash/common'; +import { + HTML_SANITIZE_MARKDOWN_TILE_RULES, + sanitizeHtml, + type DashboardMarkdownTile, + type DashboardMarkdownTileProperties, +} from '@lightdash/common'; import { Stack, TextInput } from '@mantine/core'; import { type UseFormReturnType } from '@mantine/form'; import MDEditor from '@uiw/react-md-editor'; @@ -7,6 +12,13 @@ interface MarkdownTileFormProps { form: UseFormReturnType; } +export const markdownTileContentTransform = ( + values: DashboardMarkdownTile['properties'], +) => ({ + ...values, + content: sanitizeHtml(values.content, HTML_SANITIZE_MARKDOWN_TILE_RULES), +}); + const MarkdownTileForm = ({ form }: MarkdownTileFormProps) => ( = ({ const form = useForm({ validate: getValidators(), validateInputOnChange: ['title', 'url', 'content'], + transformValues(values) { + if (type === DashboardTileTypes.MARKDOWN) { + return markdownTileContentTransform( + values as DashboardMarkdownTile['properties'], + ); + } + + return values; + }, }); if (!type) return null; diff --git a/packages/frontend/src/components/DashboardTiles/TileForms/TileUpdateModal.tsx b/packages/frontend/src/components/DashboardTiles/TileForms/TileUpdateModal.tsx index acdc4bb52cf4..c65cf673e165 100644 --- a/packages/frontend/src/components/DashboardTiles/TileForms/TileUpdateModal.tsx +++ b/packages/frontend/src/components/DashboardTiles/TileForms/TileUpdateModal.tsx @@ -3,6 +3,7 @@ import { DashboardTileTypes, type Dashboard, type DashboardLoomTileProperties, + type DashboardMarkdownTile, type DashboardMarkdownTileProperties, } from '@lightdash/common'; import { @@ -18,7 +19,9 @@ import { IconMarkdown, IconVideo } from '@tabler/icons-react'; import produce from 'immer'; import MantineIcon from '../../common/MantineIcon'; import LoomTileForm, { getLoomId } from './LoomTileForm'; -import MarkdownTileForm from './MarkdownTileForm'; +import MarkdownTileForm, { + markdownTileContentTransform, +} from './MarkdownTileForm'; type Tile = Dashboard['tiles'][number]; type TileProperties = Tile['properties']; @@ -53,6 +56,15 @@ const TileUpdateModal = ({ initialValues: { ...tile.properties }, validate: getValidators(), validateInputOnChange: ['title', 'url'], + transformValues(values) { + if (tile.type === DashboardTileTypes.MARKDOWN) { + return markdownTileContentTransform( + values as DashboardMarkdownTile['properties'], + ); + } + + return values; + }, }); const handleConfirm = form.onSubmit(({ ...properties }) => { From 16f17439d386c4a3efbf6d84cff8a1732216956c Mon Sep 17 00:00:00 2001 From: Filipe Dobreira Date: Fri, 22 Mar 2024 16:38:56 +0000 Subject: [PATCH 4/5] add comment to markdownTileContentTransform --- .../components/DashboardTiles/TileForms/MarkdownTileForm.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/frontend/src/components/DashboardTiles/TileForms/MarkdownTileForm.tsx b/packages/frontend/src/components/DashboardTiles/TileForms/MarkdownTileForm.tsx index badee7e2b914..d0865e31a17c 100644 --- a/packages/frontend/src/components/DashboardTiles/TileForms/MarkdownTileForm.tsx +++ b/packages/frontend/src/components/DashboardTiles/TileForms/MarkdownTileForm.tsx @@ -12,6 +12,9 @@ interface MarkdownTileFormProps { form: UseFormReturnType; } +/** + * Helper that can be used as a value transformer with Mantine's `useForm` hook. + */ export const markdownTileContentTransform = ( values: DashboardMarkdownTile['properties'], ) => ({ From e1f7428c231962b3a6b2765d0a3b3f3cc067fc4d Mon Sep 17 00:00:00 2001 From: Filipe Dobreira Date: Fri, 22 Mar 2024 16:44:39 +0000 Subject: [PATCH 5/5] allow img tag in markdown tiles --- packages/common/src/utils/sanitizeHtml.test.ts | 16 +++++++++++++++- packages/common/src/utils/sanitizeHtml.ts | 7 ++++++- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/packages/common/src/utils/sanitizeHtml.test.ts b/packages/common/src/utils/sanitizeHtml.test.ts index 868c75d26a2c..a1e450d449f6 100644 --- a/packages/common/src/utils/sanitizeHtml.test.ts +++ b/packages/common/src/utils/sanitizeHtml.test.ts @@ -32,7 +32,7 @@ describe('sanitizeHtml', () => { ).toEqual('Lightdash'); }); - test('iframe tag with markdown tile rule set', () => { + test('markdown tile rule set', () => { expect( sanitizeHtml( '', @@ -41,6 +41,20 @@ describe('sanitizeHtml', () => { ).toEqual( '', ); + + expect( + sanitizeHtml( + '', + HTML_SANITIZE_MARKDOWN_TILE_RULES, + ), + ).toEqual(''); + + expect( + sanitizeHtml( + '', + HTML_SANITIZE_MARKDOWN_TILE_RULES, + ), + ).toEqual(''); }); test('style tag in span is preserved', () => { diff --git a/packages/common/src/utils/sanitizeHtml.ts b/packages/common/src/utils/sanitizeHtml.ts index e1e0f64e2ff2..b207e1d08426 100644 --- a/packages/common/src/utils/sanitizeHtml.ts +++ b/packages/common/src/utils/sanitizeHtml.ts @@ -20,10 +20,15 @@ export const HTML_SANITIZE_DEFAULT_RULES: sanitize.IOptions = { */ export const HTML_SANITIZE_MARKDOWN_TILE_RULES: sanitize.IOptions = { ...HTML_SANITIZE_DEFAULT_RULES, - allowedTags: [...(HTML_SANITIZE_DEFAULT_RULES.allowedTags || []), 'iframe'], + allowedTags: [ + ...(HTML_SANITIZE_DEFAULT_RULES.allowedTags || []), + 'iframe', + 'img', + ], allowedAttributes: { ...HTML_SANITIZE_DEFAULT_RULES.allowedAttributes, iframe: ['width', 'height', 'src', 'name'], + img: ['src', 'width', 'height', 'alt', 'style'], }, };