Skip to content
37 changes: 37 additions & 0 deletions apps/meteor/client/components/MarkdownText.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -432,3 +432,40 @@ describe('code handling', () => {
expect(screen.getByRole('code').outerHTML).toEqual(expected);
});
});

describe('DOMPurify hook registration', () => {
it('should register hook only once at module level', () => {
// Import the module to trigger hook registration
const dompurify = require('dompurify');
const addHookSpy = jest.spyOn(dompurify, 'addHook');

// Clear any previous calls from module initialization
addHookSpy.mockClear();

const { rerender, unmount } = render(
<MarkdownText content="[Test Link](https://example.com)" variant="document" />,
{
wrapper: mockAppRoot().build(),
}
);

// Hook should NOT be registered during component render (it's registered at module level)
expect(addHookSpy).toHaveBeenCalledTimes(0);

// Re-rendering with different props should not register hook again
rerender(<MarkdownText content="[Another Link](https://example.com)" variant="document" />);
expect(addHookSpy).toHaveBeenCalledTimes(0);

// Rendering another instance should not register hook again
render(<MarkdownText content="[Third Link](https://test.com)" variant="inline" />, {
wrapper: mockAppRoot().build(),
});
expect(addHookSpy).toHaveBeenCalledTimes(0);

// Unmounting should not affect the module-level hook
unmount();
expect(addHookSpy).toHaveBeenCalledTimes(0);

addHookSpy.mockRestore();
});
});
92 changes: 54 additions & 38 deletions apps/meteor/client/components/MarkdownText.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,49 @@ type MarkdownTextProps = Partial<MarkdownTextParams>;

export const supportedURISchemes = ['http', 'https', 'notes', 'ftp', 'ftps', 'tel', 'mailto', 'sms', 'cid'];

const isElement = (node: Node): node is Element => node.nodeType === Node.ELEMENT_NODE;
const isLinkElement = (node: Node): node is HTMLAnchorElement => isElement(node) && node.tagName.toLowerCase() === 'a';

// Special token used to mark internal links that need translation
// This allows the hook to stay at module level while still using contextualized translations
const INTERNAL_LINK_TOKEN = '__INTERNAL_LINK_TITLE__';

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot can we add some runtime generation here? to prevent any enumeration?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added runtime generation in 37a094a. The token is now generated using Math.random().toString(36) at module load time, creating a unique unpredictable string (e.g., __INTERNAL_LINK_TITLE_k3j9f2h8d__) that prevents enumeration attacks.


// Register the DOMPurify hook once at module level to prevent memory leaks
// This hook will be shared by all MarkdownText component instances
dompurify.addHook('afterSanitizeAttributes', (node) => {
if (!isLinkElement(node)) {
return;
}

const href = node.getAttribute('href') || '';
const isExternalLink = isExternal(href);
const isMailto = href.startsWith('mailto:');

// Set appropriate attributes based on link type
if (isExternalLink || isMailto) {
node.setAttribute('rel', 'nofollow noopener noreferrer');
// Enforcing external links to open in new tabs is critical to assure users never navigate away from the chat
// This attribute must be preserved to guarantee users maintain their chat context
node.setAttribute('target', '_blank');
}

// Set appropriate title based on link type
if (isMailto) {
// For mailto links, use the email address as the title for better user experience
// Example: for href "mailto:user@example.com" the title would be "mailto:user@example.com"
node.setAttribute('title', href);
} else if (isExternalLink) {
// For external links, set an empty title to prevent tooltips
// This reduces visual clutter and lets users see the URL in the browser's status bar instead
node.setAttribute('title', '');
} else {
// For internal links, use a token that will be replaced with translated text in the component
// This allows us to use the contextualized translation function
const relativePath = href.replace(getBaseURI(), '');
node.setAttribute('title', `${INTERNAL_LINK_TOKEN}${relativePath}`);
}
});

const MarkdownText = ({
content,
variant = 'document',
Expand Down Expand Up @@ -143,41 +186,17 @@ const MarkdownText = ({
}
})();

// Add a hook to make all external links open a new window
dompurify.addHook('afterSanitizeAttributes', (node) => {
if (!isLinkElement(node)) {
return;
}

const href = node.getAttribute('href') || '';
const isExternalLink = isExternal(href);
const isMailto = href.startsWith('mailto:');

// Set appropriate attributes based on link type
if (isExternalLink || isMailto) {
node.setAttribute('rel', 'nofollow noopener noreferrer');
// Enforcing external links to open in new tabs is critical to assure users never navigate away from the chat
// This attribute must be preserved to guarantee users maintain their chat context
node.setAttribute('target', '_blank');
}

// Set appropriate title based on link type
if (isMailto) {
// For mailto links, use the email address as the title for better user experience
// Example: for href "mailto:user@example.com" the title would be "mailto:user@example.com"
node.setAttribute('title', href);
} else if (isExternalLink) {
// For external links, set an empty title to prevent tooltips
// This reduces visual clutter and lets users see the URL in the browser's status bar instead
node.setAttribute('title', '');
} else {
// For internal links, add a translated title with the relative path
// Example: for href "https://my-server.rocket.chat/channel/general" the title would be "Go to #general"
node.setAttribute('title', `${t('Go_to_href', { href: href.replace(getBaseURI(), '') })}`);
}
});

return preserveHtml ? html : html && sanitizer(html, { ADD_ATTR: ['target'], ALLOWED_URI_REGEXP: getRegexp(supportedURISchemes) });
const sanitizedHtml = preserveHtml ? html : html && sanitizer(html, { ADD_ATTR: ['target'], ALLOWED_URI_REGEXP: getRegexp(supportedURISchemes) });

// Replace internal link tokens with contextualized translations
if (sanitizedHtml && typeof sanitizedHtml === 'string') {
return sanitizedHtml.replace(
new RegExp(`${INTERNAL_LINK_TOKEN}([^"]*)`, 'g'),
(_, href) => t('Go_to_href', { href })
);
}

return sanitizedHtml;
}, [preserveHtml, sanitizer, content, variant, markedOptions, parseEmoji, t]);

return __html ? (
Expand All @@ -190,7 +209,4 @@ const MarkdownText = ({
) : null;
};

const isElement = (node: Node): node is Element => node.nodeType === Node.ELEMENT_NODE;
const isLinkElement = (node: Node): node is HTMLAnchorElement => isElement(node) && node.tagName.toLowerCase() === 'a';

export default MarkdownText;