diff --git a/src/frontend/web/von_interface/static/js/chatTab.js b/src/frontend/web/von_interface/static/js/chatTab.js index 99324310..c9b17b18 100644 --- a/src/frontend/web/von_interface/static/js/chatTab.js +++ b/src/frontend/web/von_interface/static/js/chatTab.js @@ -38962,7 +38962,7 @@ function appendMessage(sender, message, turnId, hasLlmDebug = false, isHistory = messageText.className = 'chat-message-text'; messageText.style.cssText = 'color: #333; white-space: pre-wrap; text-align: left; font-weight: 400; overflow-wrap: anywhere; word-break: break-word; min-width: 0;'; const userText = String(message ?? ''); - const shouldRenderUserMarkdown = sender === 'User' && detectMarkdown(userText); + const shouldRenderUserMarkdown = sender !== 'Error' && detectMarkdown(userText); if (shouldRenderUserMarkdown) { messageText.classList.add('markdown-rendered', 'chat-markdown'); messageText.style.whiteSpace = 'normal'; diff --git a/src/frontend/web/von_interface/static/js/markdownUtils.js b/src/frontend/web/von_interface/static/js/markdownUtils.js index 6d5c806d..ab7bcda5 100644 --- a/src/frontend/web/von_interface/static/js/markdownUtils.js +++ b/src/frontend/web/von_interface/static/js/markdownUtils.js @@ -23,6 +23,7 @@ export function detectMarkdown(text) { // Detect clear markdown indicators (conservative patterns) const patterns = [ + /\bhttps?:\/\/[^\s<>]+/i, // Bare web links also need rendering /^#{1,6}\s+.+$/m, // Headers (# ## ### etc.) - must have space after hashes /\*\*[^*\s][^*]*[^*\s]\*\*/, // Bold text (**text**) - must have content without spaces at edges /\*[^*\s][^*]*[^*\s]\*/, // Italic text (*text*) - must have content without spaces at edges @@ -101,6 +102,46 @@ function isExternalHttpLink(href) { return lower.startsWith('http://') || lower.startsWith('https://'); } +// Process only text in escaped/sanitised HTML; preserve anchors and literal code. +function linkifyRenderedHtml(html) { + const template = document.createElement('template'); + template.innerHTML = html; + const walker = document.createTreeWalker(template.content, NodeFilter.SHOW_TEXT); + const nodes = []; + let changed = false; + while (walker.nextNode()) nodes.push(walker.currentNode); + for (const node of nodes) { + if (node.parentElement?.closest('a, code, pre')) continue; + const fragment = document.createDocumentFragment(); + let offset = 0; + for (const match of node.textContent.matchAll(/\bhttps?:\/\/[^\s<>"'`]+/gi)) { + let href = match[0].replace(/[.,;:!?]+$/, ''); + // Retain balanced URL parentheses, excluding sentence delimiters. + while (/[)\]}]$/.test(href)) { + const closing = href.at(-1); + const opening = { ')': '(', ']': '[', '}': '{' }[closing]; + if (href.split(closing).length <= href.split(opening).length) break; + href = href.slice(0, -1).replace(/[.,;:!?]+$/, ''); + } + if (!sanitiseHref(href)) continue; + fragment.append(node.textContent.slice(offset, match.index)); + const anchor = document.createElement('a'); + anchor.href = href; + anchor.textContent = href; + anchor.target = '_blank'; + anchor.rel = 'noopener noreferrer'; + fragment.append(anchor); + offset = match.index + href.length; + } + if (offset) { + changed = true; + fragment.append(node.textContent.slice(offset)); + node.replaceWith(fragment); + } + } + return changed ? template.innerHTML : html; +} + export function simpleMarkdownToHtml(markdown) { if (!markdown || typeof markdown !== 'string') return ''; @@ -121,26 +162,28 @@ export function simpleMarkdownToHtml(markdown) { const orderedListRule = /^ {0,3}\d+\.\s+(.+)$/; const blockquoteRule = /^>\s?(.*)$/; const renderInlineMarkdown = (text) => { - let html = escapeHtml(text ?? ''); - - // Inline code first so markdown markers inside code spans are not re-processed. - html = html.replace(/`([^`]+)`/g, '$1'); - - // Links (sanitise href; never allow javascript: etc.) - html = html.replace(/\[([^\]]+)\]\(([^)\s]+)\)/g, (match, linkText, href) => { + // Protect code and links from subsequent substitutions; escape URLs once. + const tokens = []; + const protect = (html) => `\u0000${tokens.push(html) - 1}\u0000`; + // NUL is reserved for internal placeholders, never user-authored markup. + // eslint-disable-next-line no-control-regex + let html = String(text ?? '').replace(/\u0000/g, '\ufffd'); + html = html.replace(/`([^`]+)`/g, (_, code) => protect(`${escapeHtml(code)}`)); + html = html.replace(/\[([^\]]+)\]\(([^)\s]+)\)/g, (_, label, href) => { const safeHref = sanitiseHref(href); - if (!safeHref) { - return String(linkText); - } + const content = escapeHtml(label); + if (!safeHref) return protect(content); const targetAttr = isExternalHttpLink(safeHref) ? ' target="_blank"' : ''; - return `${String(linkText)}`; + return protect(`${content}`); }); + html = escapeHtml(html); // Bold and italic (conservative ordering). html = html.replace(/\*\*([^*\n][\s\S]*?)\*\*/g, '$1'); html = html.replace(/(^|[^*])\*([^*\n][\s\S]*?)\*([^*]|$)/g, '$1$2$3'); - return html; + // eslint-disable-next-line no-control-regex + return html.replace(/\u0000(\d+)\u0000/g, (_, index) => tokens[Number(index)]); }; const splitTableRow = (line) => { @@ -275,7 +318,7 @@ export function simpleMarkdownToHtml(markdown) { i += 1; } - return blocks.join('\n'); + return linkifyRenderedHtml(blocks.join('\n')); } export function renderSmartText(text, escapeHtmlOutput = true) { @@ -313,7 +356,7 @@ export async function renderMarkdownViaServer(markdown) { } if (typeof data?.html === 'string') { - return data.html; + return linkifyRenderedHtml(data.html); } throw new Error('render_markdown response missing html'); diff --git a/tests/browser/conversationReadability.cjs b/tests/browser/conversationReadability.cjs index 7ab3c117..adbf76be 100644 --- a/tests/browser/conversationReadability.cjs +++ b/tests/browser/conversationReadability.cjs @@ -28,12 +28,21 @@ const actor = '#V#alice', other = '#V#bob'; const row = { session_id: 'messages:fixture', source_kind: 'message_exchange', viewer_id: actor, participant_ids: [actor, other], other_participant_ids: [other], session_name: 'Bob fixture', organisation_concept_id: null, last_message_at: '2026-09-12T10:59:00Z', preview: 'Fixture conversation' }; +const linkText = 'Published https://github.com/Strong-AI-Lab/knowkat/pull/5. ' + + '[Private PR](https://github.com/Strong-AI-Lab/Von-Private/pull/67), ' + + '`https://example.org/literal` [unsafe](javascript:alert) [unsafe](data:text/html,x)'; +async function checkLinks(container) { + await expect(container.locator('a[href="https://github.com/Strong-AI-Lab/knowkat/pull/5"]')).toHaveCount(1); + await expect(container.locator('a[href="https://github.com/Strong-AI-Lab/Von-Private/pull/67"]')).toHaveText('Private PR'); + await expect(container.locator('code')).toHaveText('https://example.org/literal'); + await expect(container.locator('code a, a[href^="javascript:"], a[href^="data:"]')).toHaveCount(0); +} let messages, reads, failRead; function seed() { reads = []; failRead = false; messages = Array.from({ length: 60 }, (_, i) => ({ concept_id: `fixture-${i}`, created_at: `2026-09-12T10:${String(i).padStart(2, '0')}:00Z`, relationships: { '#V#has_sender': [i % 2 ? actor : other], '#V#has_recipient': [i % 2 ? other : actor] }, - concept_data: { read_by: [], content_fallback: `Contribution ${i}. ` + 'A research update with observations, provenance and next steps. '.repeat(5) } })); + concept_data: { read_by: [], content_fallback: linkText + ` Contribution ${i}. ` + 'A research update with observations, provenance and next steps. '.repeat(5) } })); } const unread = () => messages.filter(m => m.relationships['#V#has_recipient'].includes(actor) && !m.concept_data.read_by.includes(actor)); async function setup(page) { @@ -110,6 +119,7 @@ async function measure(page, name) { assert(unread().length > 20, 'Opening cannot clear unseen messages'); assert(unread().some(m => m.concept_id === 'fixture-0'), 'unloaded page stays unread'); await expect(page.locator('.chat-session-tab-unread')).toHaveText(String(unread().length)); + await checkLinks(page.locator('[data-contribution-id="fixture-59"] .message-content')); results.push(await measure(page, `messages-${width}`)); const firstUnread = page.locator('.message-bubble.is-unread').first(); const id = await firstUnread.getAttribute('data-contribution-id'); @@ -144,6 +154,7 @@ async function measure(page, name) { await expect.poll(() => unread().some(m => m.concept_id === 'fixture-0')).toBe(false); const persisted = unread().length; await setup(page); // Simulate page reload against retained canonical fixture state. + await checkLinks(page.locator('[data-contribution-id="fixture-59"] .message-content')); assert(unread().length <= persisted); assert(!unread().some(m => m.concept_id === id)); await page.evaluate(async () => { @@ -158,6 +169,19 @@ async function measure(page, name) { assert(layout[0].x > layout[1].x + 5 && layout[0].x > layout[2].x + 5, 'Old-style participant alignment'); assert(layout.every(item => item.overflow <= 1), 'Old-style controls/content fit'); assert(layout[1].text.includes('Bob')); + for (const history of [false, true]) { + await page.evaluate(async ({ linkText, history }) => { + const chat = await import('/static/js/chatTab.js'); + document.querySelector('#scrollableField').replaceChildren(); + for (const sender of ['User', 'Bob', 'Von']) { + chat.__testOnly_appendMessage(sender, linkText, `links-${sender}`, false, history); + } + }, { linkText, history }); + for (const turn of ['User', 'Bob', 'Von']) { + await checkLinks(page.locator(`[data-turn-id="links-${turn}"] .chat-message-text`)); + } + await page.screenshot({ path: path.join(evidence, `links-chat-${width}-${history}.png`) }); + } await page.evaluate(async () => { const chat = await import('/static/js/chatTab.js'); const field = document.querySelector('#scrollableField'); diff --git a/tests/frontend/markdownUtils.test.js b/tests/frontend/markdownUtils.test.js index b10a0687..cc596ec4 100644 --- a/tests/frontend/markdownUtils.test.js +++ b/tests/frontend/markdownUtils.test.js @@ -104,3 +104,34 @@ describe('markdownUtils', () => { expect(html).not.toContain('
  • '); }); }); + +describe('shared conversation links', () => { + const bare = 'https://github.com/Strong-AI-Lab/knowkat/pull/5'; + const labelled = 'https://github.com/Strong-AI-Lab/Von-Private/pull/67'; + const parse = html => { const el = document.createElement('div'); el.innerHTML = html; return el; }; + test('detects bare links and preserves destinations and trailing punctuation', () => { + expect(detectMarkdown(`Published ${bare}.`)).toBe(true); + const el = parse(simpleMarkdownToHtml(`Published (${bare}), [PR](${labelled}). https://example.org/a(b)?x=1&y=2!`)); + expect([...el.querySelectorAll('a')].map(a => a.getAttribute('href'))).toEqual([bare, labelled, 'https://example.org/a(b)?x=1&y=2']); + expect(el.textContent).toBe(`Published (${bare}), PR. https://example.org/a(b)?x=1&y=2!`); + expect([...el.querySelectorAll('a')].every(a => a.rel === 'noopener noreferrer' && a.target === '_blank')).toBe(true); + }); + test('keeps code literal, rejects executable schemes and escapes injected markup', () => { + const el = parse(simpleMarkdownToHtml('`[literal](https://example.org) https://example.org`\n\n```\nhttps://example.org\n```\n\n[bad](javascript:alert) [bad](data:text/html,x) [bad](vbscript:x) ')); + expect(el.querySelectorAll('a, img, script')).toHaveLength(0); + expect(el.querySelector('code').textContent).toBe('[literal](https://example.org) https://example.org'); + }); + test('labelled query strings are escaped once and existing concept links survive', () => { + const el = parse(simpleMarkdownToHtml('[query](https://example.org/?a=1&b=2) [task](#V#task_example)')); + expect(el.querySelector('a').getAttribute('href')).toBe('https://example.org/?a=1&b=2'); + expect(el.querySelectorAll('a')[1].getAttribute('href')).toBe('#V#task_example'); + }); + test('server-rendered text is linkified without nesting anchors or touching code', async () => { + const { renderMarkdownViaServer } = require('../../src/frontend/web/von_interface/static/js/markdownUtils.js'); + global.fetch = jest.fn().mockResolvedValue({ ok: true, json: async () => ({ html: `

    ${bare}. PR ${bare}

    ` }) }); + const el = parse(await renderMarkdownViaServer(bare)); + expect([...el.querySelectorAll('a')].map(a => a.getAttribute('href'))).toEqual([bare, labelled]); + expect(el.querySelector('code a')).toBeNull(); + delete global.fetch; + }); +});