fix: External links without target not being modified

.changeset/serious-trees-whisper.md

@@ -0,0 +1,5 @@
+---
+"@rocket.chat/meteor": patch
+---
+
+Fixed links on settings redirecting to the outside of the chat, now they will open on a new tab

apps/meteor/client/components/MarkdownText.spec.tsx

@@ -1,10 +1,17 @@
 import { mockAppRoot } from '@rocket.chat/mock-providers';
-import { render } from '@testing-library/react';
+import { render, screen } from '@testing-library/react';
 
 import MarkdownText from './MarkdownText';
 
 import '@testing-library/jest-dom';
 
+// Mock getBaseURI from @rocket.chat/ui-client to ensure consistent behavior in tests
+// This will affect the isExternal function imported above.
+jest.mock('@rocket.chat/ui-client', () => ({
+	...jest.requireActual('@rocket.chat/ui-client'), // Import and retain default behavior for other exports
+	getBaseURI: jest.fn(() => 'http://localhost/'), // Mock getBaseURI for consistent test behavior
+}));
+
 const normalizeHtml = (html: any) => {
 	return html.replace(/\s+/g, ' ').trim();
 };
@@ -58,8 +65,17 @@ it('should render html elements as expected using default parser', async () => {
 	expect(normalizedHtml).toContain('<h3>Heading 3</h3>');
 	expect(normalizedHtml).toContain('<ul> <li>List Item 1</li><li>List Item 2</li><li>List Item 3</li><li>List Item 4');
 	expect(normalizedHtml).toContain('<ol> <li>List Item 1</li><li>List Item 2</li><li>List Item 3</li><li>List Item 4');
-	expect(normalizedHtml).toContain('<a title="" rel="nofollow noopener noreferrer" target="_blank">Rocket.Chat</a>');
+
+	expect(normalizedHtml).toContain('<a');
+	expect(normalizedHtml).toContain('title=""');
+	expect(normalizedHtml).toContain('rel="nofollow noopener noreferrer"');
+	expect(normalizedHtml).toContain('target="_blank"');
+	expect(normalizedHtml).toContain('>Rocket.Chat</a>');
+
+	expect(normalizedHtml).toContain('href="mailto:[email protected]"');
+	expect(normalizedHtml).toContain('title="mailto:[email protected]"');
 	expect(normalizedHtml).toContain('[email protected]');
+
 	expect(normalizedHtml).toContain('+55991999999');
 	expect(normalizedHtml).toContain('<code>Inline code</code>');
 	expect(normalizedHtml).toContain('<pre><code class="language-typescript">const test = \'this is code\' </code></pre>');
@@ -87,10 +103,18 @@ it('should render html elements as expected using inline parser', async () => {
 	expect(normalizedHtml).toContain('### Heading 3');
 	expect(normalizedHtml).toContain('<strong>Unordered List</strong> - List Item 1 - List Item 2 - List Item 3 - List Item 4');
 	expect(normalizedHtml).toContain('<strong>Ordered List</strong> 1. List Item 1 2. List Item 2 3. List Item 3 4. List Item 4');
-	expect(normalizedHtml).toContain(`<a title=\"\" rel=\"nofollow noopener noreferrer\" target=\"_blank\">Rocket.Chat</a>`);
-	expect(normalizedHtml).toContain(
-		`<a href=\"mailto:[email protected]\" title=\"mailto:[email protected]\" rel=\"nofollow noopener noreferrer\" target=\"_blank\">[email protected]</a>`,
-	);
+
+	expect(normalizedHtml).toContain('<a');
+	expect(normalizedHtml).toContain('title=""');
+	expect(normalizedHtml).toContain('rel="nofollow noopener noreferrer"');
+	expect(normalizedHtml).toContain('target="_blank"');
+	expect(normalizedHtml).toContain('>Rocket.Chat</a>');
+
+	expect(normalizedHtml).toContain('href="mailto:[email protected]"');
+	expect(normalizedHtml).toContain('title="mailto:[email protected]"');
+	expect(normalizedHtml).toContain('rel="nofollow noopener noreferrer"');
+	expect(normalizedHtml).toContain('target="_blank"');
+
 	expect(normalizedHtml).toContain('+55991999999');
 	expect(normalizedHtml).toContain('Inline code');
 	expect(normalizedHtml).toContain(`typescript const test = 'this is code'`);
@@ -99,3 +123,148 @@ it('should render html elements as expected using inline parser', async () => {
 	expect(normalizedHtml).toContain('<em>Italics within <strong>Bold</strong> text</em>');
 	expect(normalizedHtml).toContain('<em>Italics within <strong>Bold</strong> text with single underscore and asterik</em>');
 });
+
+describe('links handling', () => {
+	it.each([
+		{
+			caseName: 'transform external http',
+			link: 'http://example.com',
+			query: () => screen.getByRole('link', { name: 'Test Link' }),
+			expectedHref: 'http://example.com',
+			expectedRel: 'nofollow noopener noreferrer',
+			expectedTarget: '_blank',
+			expectedTitleAttribute: '',
+		},
+		{
+			caseName: 'transform external https',
+			link: 'https://example.com',
+			query: () => screen.getByRole('link', { name: 'Test Link' }),
+			expectedHref: 'https://example.com',
+			expectedRel: 'nofollow noopener noreferrer',
+			expectedTarget: '_blank',
+			expectedTitleAttribute: '',
+		},
+		{
+			caseName: 'transform notes',
+			link: 'notes://example.com/path',
+			query: () => screen.getByRole('link', { name: 'Test Link' }),
+			expectedHref: 'notes://example.com/path',
+			expectedRel: 'nofollow noopener noreferrer',
+			expectedTarget: '_blank',
+			expectedTitleAttribute: '',
+		},
+		{
+			caseName: 'transform ftp',
+			link: 'ftp://example.com/file.txt',
+			query: () => screen.getByRole('link', { name: 'Test Link' }),
+			expectedHref: 'ftp://example.com/file.txt',
+			expectedRel: 'nofollow noopener noreferrer',
+			expectedTarget: '_blank',
+			expectedTitleAttribute: '',
+		},
+		{
+			caseName: 'transform ftps',
+			link: 'ftps://example.com/file.txt',
+			query: () => screen.getByRole('link', { name: 'Test Link' }),
+			expectedHref: 'ftps://example.com/file.txt',
+			expectedRel: 'nofollow noopener noreferrer',
+			expectedTarget: '_blank',
+			expectedTitleAttribute: '',
+		},
+		{
+			caseName: 'transform tel',
+			link: 'tel:+1234567890',
+			query: () => screen.getByRole('link', { name: 'Test Link' }),
+			expectedHref: 'tel:+1234567890',
+			expectedRel: 'nofollow noopener noreferrer',
+			expectedTarget: '_blank',
+			expectedTitleAttribute: '',
+		},
+		{
+			caseName: 'transform mailto',
+			link: 'mailto:[email protected]',
+			query: () => screen.getByRole('link', { name: 'mailto:[email protected]' }),
+			expectedHref: 'mailto:[email protected]',
+			expectedRel: 'nofollow noopener noreferrer',
+			expectedTarget: '_blank',
+			expectedTitleAttribute: 'mailto:[email protected]',
+		},
+		{
+			caseName: 'transform sms',
+			link: 'sms:+1234567890?body=hello',
+			query: () => screen.getByRole('link', { name: 'Test Link' }),
+			expectedHref: 'sms:+1234567890?body=hello',
+			expectedRel: 'nofollow noopener noreferrer',
+			expectedTarget: '_blank',
+			expectedTitleAttribute: '',
+		},
+		{
+			caseName: 'transform cid',
+			link: 'cid:[email protected]',
+			query: () => screen.getByRole('link', { name: 'Test Link' }),
+			expectedHref: 'cid:[email protected]',
+			expectedRel: 'nofollow noopener noreferrer',
+			expectedTarget: '_blank',
+			expectedTitleAttribute: '',
+		},
+		{
+			caseName: 'filter relative',
+			link: '/channel/general',
+			query: () => screen.getByText('Test Link'),
+			expectedHref: undefined,
+			expectedRel: 'nofollow noopener noreferrer',
+			expectedTarget: '_blank',
+			expectedTitleAttribute: '',
+		},
+		{
+			caseName: 'transform absolute',
+			link: 'http://localhost/another-channel',
+			query: () => screen.getByRole('link', { name: 'Go to: another-channel' }),
+			expectedHref: 'http://localhost/another-channel',
+			expectedRel: undefined,
+			expectedTarget: undefined,
+			expectedTitleAttribute: 'Go to: another-channel',
+		},
+		{
+			caseName: 'filter unknown scheme',
+			link: 'invalid://example.com',
+			query: () => screen.getByText('Test Link'),
+			expectedHref: undefined,
+			expectedRel: 'nofollow noopener noreferrer',
+			expectedTarget: '_blank',
+			expectedTitleAttribute: '',
+		},
+		{
+			caseName: 'filter javascript',
+			link: "javascript:alert('XSS')",
+			query: () => screen.getByText('Test Link'),
+			expectedHref: undefined,
+			expectedRel: 'nofollow noopener noreferrer',
+			expectedTarget: '_blank',
+			expectedTitleAttribute: '',
+		},
+	] as const)('should $caseName links', ({ link, query, expectedHref, expectedRel, expectedTarget, expectedTitleAttribute }) => {
+		const markdownContent = `[Test Link](${link})`;
+		render(<MarkdownText content={markdownContent} variant='document' />, {
+			wrapper: mockAppRoot().withTranslations('en', 'core', { Go_to_href: 'Go to: {{href}}' }).build(),
+		});
+
+		const anchorElement = query();
+
+		expect(anchorElement).toBeInTheDocument();
+		expect(anchorElement.tagName).toBe('A');
+		expect(anchorElement).toHaveTextContent('Test Link');
+
+		if (expectedHref !== undefined) expect(anchorElement).toHaveAttribute('href', expectedHref);
+		else expect(anchorElement).not.toHaveAttribute('href');
+
+		if (expectedRel !== undefined) expect(anchorElement).toHaveAttribute('rel', expectedRel);
+		else expect(anchorElement).not.toHaveAttribute('rel');
+
+		if (expectedTarget !== undefined) expect(anchorElement).toHaveAttribute('target', expectedTarget);
+		else expect(anchorElement).not.toHaveAttribute('target');
+
+		if (expectedTitleAttribute !== undefined) expect(anchorElement).toHaveAttribute('title', expectedTitleAttribute);
+		else expect(anchorElement).not.toHaveAttribute('title');
+	});
+});

apps/meteor/client/components/MarkdownText.tsx

@@ -32,8 +32,9 @@ const walkTokens = (token: marked.Token) => {
 
 marked.use({ walkTokens });
 
-const linkMarked = (href: string | null, _title: string | null, text: string): string =>
-	`<a href="${href}" rel="nofollow noopener noreferrer">${text}</a> `;
+const linkMarked = (href: string | null, _title: string | null, text: string): string => {
+	return `<a href="${href || ''}">${text}</a>`;
+};
 const paragraphMarked = (text: string): string => text;
 const brMarked = (): string => ' ';
 const listItemMarked = (text: string): string => {
@@ -132,20 +133,40 @@ const MarkdownText = ({
 
 		// Add a hook to make all external links open a new window
 		dompurify.addHook('afterSanitizeAttributes', (node) => {
-			if (isElement(node) && 'target' in node) {
-				const href = node.getAttribute('href') || '';
+			if (!isLinkElement(node)) {
+				return;
+			}
 
-				node.setAttribute('title', `${t('Go_to_href', { href: href.replace(getBaseURI(), '') })}`);
+			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');
-				if (isExternal(node.getAttribute('href') || '')) {
-					node.setAttribute('target', '_blank');
-					node.setAttribute('title', href);
-				}
+				// 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:[email protected]" the title would be "mailto:[email protected]"
+				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(schemes) });
-	}, [preserveHtml, sanitizer, content, variant, markedOptions, parseEmoji, t]);
+	}, [preserveHtml, sanitizer, content, variant, markedOptions, parseEmoji, t, schemes]);
 
 	return __html ? (
 		<Box
@@ -158,5 +179,6 @@ const MarkdownText = ({
 };
 
 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;

apps/meteor/client/views/admin/settings/Setting/Setting.tsx

@@ -104,10 +104,7 @@ function Setting({ className = undefined, settingId, sectionChanged }: SettingPr
 	const labelText = (i18n.exists(i18nLabel) && t(i18nLabel)) || (i18n.exists(_id) && t(_id)) || i18nLabel || _id;
 
 	const hint = useMemo(
-		() =>
-			i18nDescription && i18n.exists(i18nDescription) ? (
-				<MarkdownText variant='inline' preserveHtml content={t(i18nDescription)} />
-			) : undefined,
+		() => (i18nDescription && i18n.exists(i18nDescription) ? <MarkdownText variant='inline' content={t(i18nDescription)} /> : undefined),
 		[i18n, i18nDescription, t],
 	);