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

172 - markdown sanitization in metadata #7206

Merged
merged 20 commits into from
Sep 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
381056e
adds sanitiza-html
mfacar Sep 9, 2024
5c70b62
split html tags
mfacar Sep 9, 2024
804d4bb
conditional sanitization
mfacar Sep 9, 2024
184dde3
Merge branch 'development' of https://github.com/huridocs/uwazi into …
mfacar Sep 10, 2024
95adec4
test coverage
mfacar Sep 10, 2024
2f6082d
Merge branch 'development' of https://github.com/huridocs/uwazi into …
mfacar Sep 10, 2024
d9579d7
update metadata snapshot to extended false
mfacar Sep 10, 2024
465af45
Merge branch 'development' into 172-markdown-sanitization
mfacar Sep 11, 2024
ae5475f
Merge branch 'development' into 172-markdown-sanitization
mfacar Sep 11, 2024
9f15372
renames attribute
mfacar Sep 19, 2024
4e93a46
Merge branch 'development' of https://github.com/huridocs/uwazi into …
mfacar Sep 19, 2024
088a4ed
Merge branch 'development' of https://github.com/huridocs/uwazi into …
mfacar Sep 19, 2024
9e75c3c
Merge branch '172-markdown-sanitization' of https://github.com/hurido…
mfacar Sep 19, 2024
f0925c1
update attrib name
mfacar Sep 19, 2024
81ecd2e
Merge branch 'development' of https://github.com/huridocs/uwazi into …
mfacar Sep 19, 2024
adca0f7
Merge branch 'development' of https://github.com/huridocs/uwazi into …
mfacar Sep 20, 2024
4fdcd10
Merge branch 'development' into 172-markdown-sanitization
konzz Sep 20, 2024
c840c87
Merge branch 'development' of https://github.com/huridocs/uwazi into …
mfacar Sep 20, 2024
b59c758
updates snapshots
mfacar Sep 20, 2024
4887a68
Merge branch '172-markdown-sanitization' of https://github.com/hurido…
mfacar Sep 20, 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
16 changes: 14 additions & 2 deletions app/react/Markdown/MarkdownViewer.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import PropTypes from 'prop-types';
import React, { Component } from 'react';
import sanitizeHtml from 'sanitize-html';
import { risonDecodeOrIgnore } from 'app/utils';
import { Translate } from 'app/I18N';
import { MarkdownLink, SearchBox, MarkdownMedia, ItemList } from './components';
import CustomHookComponents from './CustomHooks';

import markdownToReact from './markdownToReact';
import { ValidatedElement } from './ValidatedElement';
import { visualizationHtmlTags } from './utils';

class MarkdownViewer extends Component {
static errorHtml(index, message) {
Expand Down Expand Up @@ -107,8 +109,15 @@ class MarkdownViewer extends Component {
return false;
}

const sanitizedMarkdown = !this.props.sanitized
? this.props.markdown
: sanitizeHtml(this.props.markdown, {
allowedTags: visualizationHtmlTags,
allowedAttributes: false,
});

const ReactFromMarkdown = markdownToReact(
this.props.markdown,
sanitizedMarkdown,
this.customComponent.bind(this),
this.props.html
);
Expand All @@ -120,7 +129,8 @@ class MarkdownViewer extends Component {
return ValidatedElement(
'div',
{ className: 'markdown-viewer' },
...React.Children.toArray(ReactFromMarkdown)
React.Children.toArray(ReactFromMarkdown),
this.props.sanitized
);
}
}
Expand All @@ -130,13 +140,15 @@ MarkdownViewer.defaultProps = {
markdown: '',
html: false,
compact: false,
sanitized: true,
};

MarkdownViewer.propTypes = {
markdown: PropTypes.string,
lists: PropTypes.arrayOf(PropTypes.object),
html: PropTypes.bool,
compact: PropTypes.bool,
sanitized: PropTypes.bool,
};

export default MarkdownViewer;
20 changes: 14 additions & 6 deletions app/react/Markdown/ValidatedElement.tsx
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
import React from 'react';
import { validHtmlTags } from './utils';
import { extendedHtmlTags, visualizationHtmlTags } from './utils';

const isValidTagName = (tagName: string): boolean => validHtmlTags.has(tagName);
const isValidTagName = (tagName: string, sanitized: boolean): boolean =>
!sanitized ? extendedHtmlTags.includes(tagName) : visualizationHtmlTags.includes(tagName);

const ValidatedElement = (
type: string | React.JSXElementConstructor<any>,
props: (React.Attributes & { children?: React.ReactNode }) | null,
...children: React.ReactNode[]
children: React.ReactNode[],
sanitized = true
): React.ReactElement | null => {
if (typeof type === 'string' && !isValidTagName(type)) {
if (typeof type === 'string' && !isValidTagName(type, sanitized)) {
return React.createElement('div', { className: 'error' }, `Invalid tag: ${type}`);
}

Expand All @@ -17,15 +19,21 @@ const ValidatedElement = (
return child.map(c => {
const childProps = c.props as React.Attributes & { children?: React.ReactNode };
return React.isValidElement(c)
? ValidatedElement(c.type, childProps, ...React.Children.toArray(childProps.children))
? ValidatedElement(
c.type,
childProps,
React.Children.toArray(childProps.children),
sanitized
)
: c;
});
}
if (React.isValidElement(child)) {
return ValidatedElement(
child.type,
child.props as React.Attributes & { children?: React.ReactNode },
...React.Children.toArray(child.props.children)
React.Children.toArray(child.props.children),
sanitized
);
}
return child;
Expand Down
39 changes: 39 additions & 0 deletions app/react/Markdown/specs/MarkdownViewer.spec.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable no-template-curly-in-string */
/* eslint-disable max-statements */
import React, { Component } from 'react';

Expand All @@ -13,6 +14,7 @@ describe('MarkdownViewer', () => {
beforeEach(() => {
props = {
markdown: '## MarkdownContent',
sanitized: false,
};
});

Expand Down Expand Up @@ -213,4 +215,41 @@ describe('MarkdownViewer', () => {
expect(component).toMatchSnapshot();
});
});

describe('limited markdown', () => {
it.each`
type | markdown
${'forbidden tag'} | ${'<div>This <input type="text" /> was cleaned</div>'}
${'forbidden tag'} | ${'<div>This <script language="javascript">console.log(\'dangerous\')</script> was cleaned</div>'}
${'malicious code'} | ${'<img src="javascript:alert(\'forbidden\');" />'}
`('should replace banned tags $type', ({ markdown }) => {
props = {
markdown,
html: true,
sanitized: true,
};
render();

expect(component).toMatchSnapshot();
});

it.each`
type | markdown
${'custom tags'} | ${'<MarkdownLink url="the_url">label</MarkDownLink> and <SearchBox/>\n<div>test</div>'}
${'custom hook'} | ${'<CejilChart002 />'}
${'placeholder'} | ${'<placeholder>$content</placeholder>'}
${'standard tags'} | ${'<Table><tr><th>Title</th></tr><tr><td>value</td></tr> </Table>'}
${'interpolation'} | ${'${template.color}'}
${'media'} | ${'{vimeo}(1234, options)'}
`('should keep allowed tags $type', ({ markdown }) => {
props = {
markdown,
html: true,
sanitized: true,
};
render();

expect(component).toMatchSnapshot();
});
});
});
121 changes: 121 additions & 0 deletions app/react/Markdown/specs/__snapshots__/MarkdownViewer.spec.js.snap
Original file line number Diff line number Diff line change
@@ -1,5 +1,126 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`MarkdownViewer limited markdown should keep allowed tags custom hook 1`] = `
<div
className="markdown-viewer"
>
<p>
<cejilchart002 />
</p>


</div>
`;

exports[`MarkdownViewer limited markdown should keep allowed tags custom tags 1`] = `
<div
className="markdown-viewer"
>
<p>
<MarkdownLink
classname=""
url="the_url"
>
label
</MarkdownLink>
and
</p>


<div>
test
</div>
</div>
`;

exports[`MarkdownViewer limited markdown should keep allowed tags interpolation 1`] = `
<div
className="markdown-viewer"
>
<p>
\${template.color}
</p>


</div>
`;

exports[`MarkdownViewer limited markdown should keep allowed tags media 1`] = `
<div
className="markdown-viewer"
>
<div>
<MarkdownMedia
compact={false}
config="(1234, options)"
/>
</div>


</div>
`;

exports[`MarkdownViewer limited markdown should keep allowed tags placeholder 1`] = `
<div
className="markdown-viewer"
>
<p>
<placeholder>
$content
</placeholder>
</p>


</div>
`;

exports[`MarkdownViewer limited markdown should keep allowed tags standard tags 1`] = `
<div
className="markdown-viewer"
>
<table>
<tr>
<th>
Title
</th>
</tr>
<tr>
<td>
value
</td>
</tr>
</table>
</div>
`;

exports[`MarkdownViewer limited markdown should replace banned tags forbidden tag 1`] = `
<div
className="markdown-viewer"
>
<div>
This was cleaned
</div>
</div>
`;

exports[`MarkdownViewer limited markdown should replace banned tags forbidden tag 2`] = `
<div
className="markdown-viewer"
>
<div>
This was cleaned
</div>
</div>
`;

exports[`MarkdownViewer limited markdown should replace banned tags malicious code 1`] = `
<div
className="markdown-viewer"
>
<img />
</div>
`;

exports[`MarkdownViewer render should be able to render properly custom components not separated by \\n\\n 1`] = `
<div
className="markdown-viewer"
Expand Down
Loading
Loading