diff --git a/assets/refactor-first-report.mustache b/assets/refactor-first-report.mustache index d30f4b8..0b9c568 100644 --- a/assets/refactor-first-report.mustache +++ b/assets/refactor-first-report.mustache @@ -469,7 +469,7 @@ {{#classRelationshipsToRemove.relationships}} - {{renderedLabel}} + {{{safeRenderedLabel}}} {{priority}} {{cycleCount}} {{effortRank}} @@ -579,7 +579,7 @@ {{#packageRelationshipsToRemove.relationships}} - {{{renderedLabel}}} + {{{safeRenderedLabel}}} {{priority}} {{cycleCount}} {{effortRank}} diff --git a/lib/renderer.js b/lib/renderer.js index 60f0ca9..5227ff7 100644 --- a/lib/renderer.js +++ b/lib/renderer.js @@ -50,6 +50,27 @@ function registerLinkSafetyHook() { linkSafetyHookRegistered = true; } +/** + * Copies a relationship with label markup limited to source links and markers. + * The report-wide sanitizer permits template styles, so labels need their own + * allow-list before interpolation. Never trust a precomputed value from JSON. + * + * @param {object} relationship - Raw relationship data. + * @returns {object} Relationship with a restricted HTML label. + */ +function prepareRelationship(relationship) { + registerLinkSafetyHook(); + return { + ...relationship, + safeRenderedLabel: DOMPurify.sanitize(relationship.renderedLabel ?? '', { + ALLOWED_TAGS: ['a', 'strong'], + ALLOWED_ATTR: ['href', 'target', 'rel'], + ALLOW_DATA_ATTR: false, + ALLOW_ARIA_ATTR: false + }) + }; +} + // --------------------------------------------------------------------------- // Enhanced tables: filter -> sort -> paginate pipeline feeding the template // (plan Phase 3). prepareReportData returns a shallow-copied report object @@ -170,7 +191,7 @@ export function prepareReportData(data, tableStates = {}, config = TABLE_CONFIG) descriptor, data.classRelationshipsToRemove.relationships, stateOf(descriptor.id), config); result.classRelationshipsToRemove = { ...data.classRelationshipsToRemove, - relationships: rows, + relationships: rows.map(prepareRelationship), tableUi: ui }; } @@ -181,7 +202,7 @@ export function prepareReportData(data, tableStates = {}, config = TABLE_CONFIG) descriptor, data.packageRelationshipsToRemove.relationships, stateOf(descriptor.id), config); result.packageRelationshipsToRemove = { ...data.packageRelationshipsToRemove, - relationships: rows, + relationships: rows.map(prepareRelationship), tableUi: ui }; } diff --git a/lib/report-view.js b/lib/report-view.js index 3976aaa..fd13c59 100644 --- a/lib/report-view.js +++ b/lib/report-view.js @@ -508,6 +508,39 @@ export function bindThemeChartRedraw() { } } +/** + * Validates an untrusted URL from the report payload before it reaches + * window.open: only absolute http:/https: URLs pass, so injected schemes + * (javascript:, data:, file:, …) and malformed values are rejected. + * + * @param {*} value - Candidate URL from the fetched report data. + * @returns {boolean} True when value parses to an http: or https: URL. + */ +export function isHttpUrl(value) { + if (typeof value !== 'string') return false; + let parsed; + try { + parsed = new URL(value); + } catch { + return false; + } + return parsed.protocol === 'http:' || parsed.protocol === 'https:'; +} + +/** + * Creates a disharmony bubble chart, replacing any chart on the canvas and + * registering it for theme redraws. Clicking a bubble with a URL opens its + * source in a new tab with noopener. Warns and returns if Chart.js is unavailable. + * + * @param {HTMLCanvasElement} canvas - Canvas on which to render the chart. + * @param {string} title - Dataset label displayed in the legend. + * @param {object} chartData - Report chart data with optional bubbles and axis labels. + * @param {Array} [chartData.bubbles] - Bubble coordinates, radius, colors, + * label, priority, and optional source URL. + * @param {string} [chartData.xaxisLabel] - Overrides the effort axis label. + * @param {string} [chartData.yaxisLabel] - Overrides the relative churn axis label. + * @returns {void} + */ export function initBubbleChart(canvas, title, chartData) { const ChartLib = window.Chart; if (!ChartLib) { @@ -532,6 +565,41 @@ export function initBubbleChart(canvas, title, chartData) { options: { responsive: true, maintainAspectRatio: false, + // Clicking a bubble opens the source file the report links it + // to (disharmonies[].chart.bubbles[].url) in a new tab, with + // noopener so the opened page can't reach back into this one. + onClick: (event, elements, chart) => { + const hit = elements && elements[0]; + if (!hit || !chart || !chart.data || !chart.data.datasets) return; + const point = chart.data.datasets[hit.datasetIndex]; + const url = point && point.data && point.data[hit.index] && point.data[hit.index].raw + ? point.data[hit.index].raw.url + : null; + // The url comes from the fetched report payload, which is + // untrusted: only parsed http(s) URLs may reach + // window.open, so a malicious javascript:/data: value + // can't execute when the bubble is clicked. + if (isHttpUrl(url) && typeof window !== 'undefined' && typeof window.open === 'function') { + window.open(url, '_blank', 'noopener'); + } + }, + // Pointer affordance while the cursor is over a bubble — only + // when the hovered bubble carries a URL the click handler would + // actually open (isHttpUrl), so the hover hint can never + // promise a navigation the click refuses to perform. + onHover: (event, elements, chart) => { + if (!event || !event.native || !event.native.target) return; + const hit = elements && elements[0]; + const dataset = hit && chart && chart.data && chart.data.datasets + ? chart.data.datasets[hit.datasetIndex] + : null; + const raw = dataset && dataset.data && dataset.data[hit.index] + ? dataset.data[hit.index].raw + : null; + event.native.target.style.cursor = hit && isHttpUrl(raw ? raw.url : null) + ? 'pointer' + : 'default'; + }, plugins: { legend: { display: true, @@ -565,12 +633,18 @@ export function initBubbleChart(canvas, title, chartData) { callbacks: { label: context => { const raw = context.raw.raw; - return [ + const lines = [ `File: ${raw.label}`, `Priority: ${raw.priority}`, `Effort Rank: ${raw.x}`, `Change Proneness Rank: ${raw.y}` ]; + // Same gate as the click handler: the prompt + // only appears when the url is actually + // openable (isHttpUrl), never for scheme- + // injected or malformed values. + if (isHttpUrl(raw.url)) lines.push('Click to open source file'); + return lines; } } } diff --git a/public/assets/refactor-first-report.mustache b/public/assets/refactor-first-report.mustache index d30f4b8..0b9c568 100644 --- a/public/assets/refactor-first-report.mustache +++ b/public/assets/refactor-first-report.mustache @@ -469,7 +469,7 @@ {{#classRelationshipsToRemove.relationships}} - {{renderedLabel}} + {{{safeRenderedLabel}}} {{priority}} {{cycleCount}} {{effortRank}} @@ -579,7 +579,7 @@ {{#packageRelationshipsToRemove.relationships}} - {{{renderedLabel}}} + {{{safeRenderedLabel}}} {{priority}} {{cycleCount}} {{effortRank}} diff --git a/tests/integration/report-view.test.jsx b/tests/integration/report-view.test.jsx index 4bf543b..7a8098e 100644 --- a/tests/integration/report-view.test.jsx +++ b/tests/integration/report-view.test.jsx @@ -160,10 +160,16 @@ describe('ReportView', () => { expect(container.querySelector('[role="alert"]')).not.toBeNull(); }); - test('escapes renderedLabel in class relationships', async () => { + test('hyperlinks the Class Relationship column like the Package Relationships table', async () => { respondJsonFor(sampleJson); const { container } = await renderReport(); - expect(container.innerHTML).toContain('<a href=https://github.com/junit-team/junit4'); + const table = container.querySelector('table[data-rf-table="class-relationships"]'); + expect(table).not.toBeNull(); + const link = table.querySelector('tbody a[href^="https://github.com/junit-team/junit4"]'); + expect(link).not.toBeNull(); + expect(link.getAttribute('target')).toBe('_blank'); + expect(link.getAttribute('rel')).toContain('noopener'); + expect(container.innerHTML).not.toContain(' { + const junitFixture = JSON.parse( + readFileSync(path.join(import.meta.dir, '../fixtures/junit4-report.json'), 'utf8') + ); + + for (const templatePath of ['assets', 'public/assets']) { + it(`restricts relationship labels to links and markers in ${templatePath}`, () => { + const template = readFileSync( + path.join(import.meta.dir, `../../${templatePath}/refactor-first-report.mustache`), 'utf8' + ); + const renderedLabel = + '' + + '' + + '* → Target : 1' + + '' + + 'Unsafe'; + const data = structuredClone(junitFixture); + for (const key of ['classRelationshipsToRemove', 'packageRelationshipsToRemove']) { + data[key].relationships = [ + { ...data[key].relationships[0], renderedLabel, + safeRenderedLabel: '' }, + { priority: 2, renderedLabel: null, safeRenderedLabel: 'Forged' } + ]; + } + const original = structuredClone(data); + const html = renderTemplate(template, prepareReportData(data)); + const doc = new JSDOM(html).window.document; + expect(data).toEqual(original); + expect(html).not.toContain('body {'); + for (const table of ['class-relationships', 'package-relationships']) { + const cells = doc.querySelectorAll(`table[data-rf-table="${table}"] tbody tr td:first-child`); + const cell = cells[0]; + expect(cell.textContent).toContain('Source* → Target : 1'); + expect(cell.querySelector('strong')?.textContent).toBe('*'); + expect(cell.querySelector('a')?.getAttribute('href')).toBe('https://example.com/source'); + expect(cell.querySelector('a')?.getAttribute('target')).toBe('_blank'); + expect(cell.querySelector('a')?.getAttribute('rel')).toBe('noopener noreferrer'); + expect(cell.querySelector('a:last-child')?.hasAttribute('href')).toBe(false); + for (const element of cell.querySelectorAll('*')) { + expect(['A', 'STRONG']).toContain(element.tagName); + for (const attribute of element.attributes) { + expect(['href', 'target', 'rel']).toContain(attribute.name); + } + } + expect(cells[1].textContent).toBe(''); + } + }); + } + + it('renders the class-relationships source-link cell with a hardened anchor', () => { + const template = readFileSync( + path.join(import.meta.dir, '../../assets/refactor-first-report.mustache'), 'utf8' + ); + const html = renderTemplate(template, prepareReportData(junitFixture)); + const doc = new JSDOM(html).window.document; + const cell = doc.querySelector( + 'table[data-rf-table="class-relationships"] tbody td.rf-text-left' + ); + expect(cell).not.toBeNull(); + const link = cell.querySelector('a[target="_blank"]'); + expect(link).not.toBeNull(); + expect(link.getAttribute('href')).toContain('https://github.com/junit-team/junit4/blob/'); + expect(link.getAttribute('rel')).toBe('noopener noreferrer'); + }); +}); + // --------------------------------------------------------------------------- // prepareReportData: filter -> sort -> paginate pipeline feeding the report // template with paginated rows and tableUi metadata (plan Phase 3). diff --git a/tests/unit/report-view.test.js b/tests/unit/report-view.test.js index e45b640..805245f 100644 --- a/tests/unit/report-view.test.js +++ b/tests/unit/report-view.test.js @@ -144,6 +144,121 @@ describe('initBubbleChart', () => { delete window.Chart; }); + it('opens the clicked bubble source url in a new tab', () => { + const created = []; + window.Chart = function (ctx, config) { created.push(config); }; + const canvas = document.getElementById('chart_GOD'); + const originalOpen = window.open; + const opens = []; + window.open = (...args) => { opens.push(args); }; + initBubbleChart(canvas, 'God Classes', { + bubbles: [{ x: 1, y: 1, r: 24, label: 'Foo.java', url: 'https://example.com/Foo.java' }] + }); + const stubChart = { + data: { datasets: [{ data: [{ x: 1, y: 1, r: 24, raw: { url: 'https://example.com/Foo.java' } }] }] } + }; + created[0].options.onClick(new window.MouseEvent('click'), [{ datasetIndex: 0, index: 0 }], stubChart); + expect(opens).toEqual([['https://example.com/Foo.java', '_blank', 'noopener']]); + window.open = originalOpen; + }); + + it('ignores bubble clicks away from every bubble and on bubbles without a url', () => { + const created = []; + window.Chart = function (ctx, config) { created.push(config); }; + const canvas = document.getElementById('chart_GOD'); + const originalOpen = window.open; + const opens = []; + window.open = (...args) => { opens.push(args); }; + initBubbleChart(canvas, 'God Classes', { + bubbles: [{ x: 1, y: 1, r: 24, label: 'Foo.java', url: 'https://example.com/Foo.java' }] + }); + const stubChart = { + data: { datasets: [{ data: [{ x: 1, y: 1, r: 24, raw: { url: 'https://example.com/Foo.java' } }] }] } + }; + created[0].options.onClick(new window.MouseEvent('click'), [], stubChart); + created[0].options.onClick( + new window.MouseEvent('click'), + [{ datasetIndex: 0, index: 0 }], + { data: { datasets: [{ data: [{ x: 1, y: 1, r: 24, raw: {} }] }] } } + ); + expect(opens).toEqual([]); + window.open = originalOpen; + }); + + it('never calls window.open for javascript: or other non-http(s) urls', () => { + const created = []; + window.Chart = function (ctx, config) { created.push(config); }; + const canvas = document.getElementById('chart_GOD'); + const originalOpen = window.open; + const opens = []; + window.open = (...args) => { opens.push(args); }; + initBubbleChart(canvas, 'God Classes', { + bubbles: [{ x: 1, y: 1, r: 24, label: 'Foo.java', url: 'https://example.com/Foo.java' }] + }); + const onClick = created[0].options.onClick; + const clickEvent = new window.MouseEvent('click'); + for (const url of [ + 'javascript:alert(1)', + 'JAVASCRIPT:alert(1)', + 'data:text/html,', + 'file:///etc/passwd', + 'ftp://example.com/Foo.java', + '/relative/path', + 'not a url', + { toString: () => 'javascript:alert(1)' } + ]) { + onClick(clickEvent, [{ datasetIndex: 0, index: 0 }], { + data: { datasets: [{ data: [{ x: 1, y: 1, r: 24, raw: { url } }] }] } + }); + } + expect(opens).toEqual([]); + window.open = originalOpen; + }); + + it('shows the click-to-open tooltip prompt only for openable http(s) urls', () => { + const created = []; + window.Chart = function (ctx, config) { created.push(config); }; + const canvas = document.getElementById('chart_GOD'); + initBubbleChart(canvas, 'God Classes', { bubbles: [] }); + const label = created[0].options.plugins.tooltip.callbacks.label; + const promptFor = url => label({ + raw: { raw: { label: 'Foo.java', priority: 1, x: 1, y: 2, url } } + }).filter(line => line === 'Click to open source file'); + expect(promptFor('https://example.com/Foo.java')).toEqual(['Click to open source file']); + expect(promptFor('http://example.com/Foo.java')).toEqual(['Click to open source file']); + expect(promptFor('javascript:alert(1)')).toEqual([]); + expect(promptFor('/relative/path')).toEqual([]); + expect(promptFor('')).toEqual([]); + expect(promptFor(undefined)).toEqual([]); + }); + + it('shows a pointer cursor only while hovering a bubble with an openable http(s) url', () => { + const created = []; + window.Chart = function (ctx, config) { created.push(config); }; + const canvas = document.getElementById('chart_GOD'); + initBubbleChart(canvas, 'God Classes', { + bubbles: [ + { x: 1, y: 1, r: 24, label: 'Foo.java', url: 'https://example.com/Foo.java' }, + { x: 2, y: 2, r: 24, label: 'Bar.java', url: 'javascript:alert(1)' }, + { x: 3, y: 3, r: 24, label: 'Baz.java' } + ] + }); + const onHover = created[0].options.onHover; + const hoverEvent = { native: { target: canvas } }; + onHover(hoverEvent, [{ datasetIndex: 0, index: 0 }], created[0]); + expect(canvas.style.cursor).toBe('pointer'); + onHover(hoverEvent, [{ datasetIndex: 0, index: 1 }], created[0]); + expect(canvas.style.cursor).toBe('default'); + onHover(hoverEvent, [{ datasetIndex: 0, index: 2 }], created[0]); + expect(canvas.style.cursor).toBe('default'); + onHover(hoverEvent, [], created[0]); + expect(canvas.style.cursor).toBe('default'); + // event-target guard preserved: missing native target neither throws + // nor touches the cursor. + expect(() => onHover({ native: {} }, [{ datasetIndex: 0, index: 0 }], created[0])).not.toThrow(); + expect(canvas.style.cursor).toBe('default'); + }); + it('renders legend labels in a theme-resolved color with >= 4.5:1 contrast', () => { for (const [mode, background] of [['light', LIGHT_BG], ['dark', DARK_BG]]) { installThemeRadios(mode);