From 7fc852df1f0b502b271169789bddd818f82c2194 Mon Sep 17 00:00:00 2001 From: Jim Bethancourt Date: Fri, 2 Oct 2026 07:05:16 -0500 Subject: [PATCH 1/6] Hyperlinking ChartJS Bubbles and the Class Relationship column Hyperlinking ChartJS Bubbles and the Class & Package Relationship columns --- assets/refactor-first-report.mustache | 2 +- lib/report-view.js | 23 +++++++- public/assets/refactor-first-report.mustache | 2 +- tests/integration/report-view.test.jsx | 10 +++- tests/unit/report-view.test.js | 56 ++++++++++++++++++++ 5 files changed, 88 insertions(+), 5 deletions(-) diff --git a/assets/refactor-first-report.mustache b/assets/refactor-first-report.mustache index d30f4b8..5ffffa5 100644 --- a/assets/refactor-first-report.mustache +++ b/assets/refactor-first-report.mustache @@ -469,7 +469,7 @@ {{#classRelationshipsToRemove.relationships}} - {{renderedLabel}} + {{{renderedLabel}}} {{priority}} {{cycleCount}} {{effortRank}} diff --git a/lib/report-view.js b/lib/report-view.js index 3976aaa..4ff06a9 100644 --- a/lib/report-view.js +++ b/lib/report-view.js @@ -532,6 +532,25 @@ 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; + if (url && typeof window !== 'undefined' && typeof window.open === 'function') { + window.open(url, '_blank', 'noopener'); + } + }, + // Pointer affordance while the cursor is over a bubble. + onHover: (event, elements) => { + if (!event || !event.native || !event.native.target) return; + event.native.target.style.cursor = elements && elements.length ? 'pointer' : 'default'; + }, plugins: { legend: { display: true, @@ -565,12 +584,14 @@ 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}` ]; + if (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..5ffffa5 100644 --- a/public/assets/refactor-first-report.mustache +++ b/public/assets/refactor-first-report.mustache @@ -469,7 +469,7 @@ {{#classRelationshipsToRemove.relationships}} - {{renderedLabel}} + {{{renderedLabel}}} {{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(' { 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('shows a pointer cursor while hovering a bubble with a 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' }] + }); + 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, [], created[0]); + 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); From f1e195b6dadbbd94b0d00fc7c6a7a41cd19e0979 Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Fri, 2 Oct 2026 13:08:33 +0000 Subject: [PATCH 2/6] Document initBubbleChart parameters and behavior with JSDoc --- lib/report-view.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/lib/report-view.js b/lib/report-view.js index 4ff06a9..9713e18 100644 --- a/lib/report-view.js +++ b/lib/report-view.js @@ -508,6 +508,20 @@ export function bindThemeChartRedraw() { } } +/** + * 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) { From 2a88c29ef04aefcb230e8781288cb84719eb29f4 Mon Sep 17 00:00:00 2001 From: Jim Bethancourt Date: Fri, 2 Oct 2026 13:04:57 -0500 Subject: [PATCH 3/6] Applied CodeRabbit suggestions: Update mustache partial rendering and link validation for class-relationships source links. --- assets/refactor-first-report.mustache | 2 +- lib/renderer.js | 13 ++++- lib/report-view.js | 25 +++++++++- public/assets/refactor-first-report.mustache | 2 +- tests/unit/renderer.test.js | 52 ++++++++++++++++++++ tests/unit/report-template.test.js | 12 +++++ tests/unit/report-view.test.js | 30 +++++++++++ 7 files changed, 132 insertions(+), 4 deletions(-) diff --git a/assets/refactor-first-report.mustache b/assets/refactor-first-report.mustache index 5ffffa5..77770f9 100644 --- a/assets/refactor-first-report.mustache +++ b/assets/refactor-first-report.mustache @@ -469,7 +469,7 @@ {{#classRelationshipsToRemove.relationships}} - {{{renderedLabel}}} + {{> source-link-cell}} {{priority}} {{cycleCount}} {{effortRank}} diff --git a/lib/renderer.js b/lib/renderer.js index 60f0ca9..0116c2f 100644 --- a/lib/renderer.js +++ b/lib/renderer.js @@ -33,6 +33,17 @@ export function initializeMustache() { let linkSafetyHookRegistered = false; +// The class-relationships source-link cell, rendered from the report data's +// untrusted pre-rendered label. The markup lives here — outside both report +// template copies (assets/ and public/assets/) — and renderTemplate injects +// it as the Mustache partial "source-link-cell", referenced by the templates +// through {{> source-link-cell}}. The trailing CRLF matches the templates' +// line endings, so the partial expands to the exact bytes the templates +// previously embedded inline. +const REPORT_PARTIALS = { + 'source-link-cell': '{{{renderedLabel}}}\r\n' +}; + /** * Registers a DOMPurify hook that protects links opening a new browsing context. * @@ -236,7 +247,7 @@ export function prepareReportData(data, tableStates = {}, config = TABLE_CONFIG) */ export function renderTemplate(template, data) { const mustache = initializeMustache(); - const rendered = mustache.render(template, data); + const rendered = mustache.render(template, data, REPORT_PARTIALS); registerLinkSafetyHook(); // Allow-list is deliberately tight: the bundled template only needs text, // table, popup and chart markup; elements like iframe/object/embed/meta/ diff --git a/lib/report-view.js b/lib/report-view.js index 9713e18..b82f0a5 100644 --- a/lib/report-view.js +++ b/lib/report-view.js @@ -508,6 +508,25 @@ 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 @@ -556,7 +575,11 @@ export function initBubbleChart(canvas, title, chartData) { const url = point && point.data && point.data[hit.index] && point.data[hit.index].raw ? point.data[hit.index].raw.url : null; - if (url && typeof window !== 'undefined' && typeof window.open === 'function') { + // 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'); } }, diff --git a/public/assets/refactor-first-report.mustache b/public/assets/refactor-first-report.mustache index 5ffffa5..77770f9 100644 --- a/public/assets/refactor-first-report.mustache +++ b/public/assets/refactor-first-report.mustache @@ -469,7 +469,7 @@ {{#classRelationshipsToRemove.relationships}} - {{{renderedLabel}}} + {{> source-link-cell}} {{priority}} {{cycleCount}} {{effortRank}} diff --git a/tests/unit/renderer.test.js b/tests/unit/renderer.test.js index 071dd4e..a5eee5b 100644 --- a/tests/unit/renderer.test.js +++ b/tests/unit/renderer.test.js @@ -184,6 +184,58 @@ describe('templating safety (repository-provided templates are untrusted)', () = }); }); +// --------------------------------------------------------------------------- +// Source-link cell partial: the class-relationships cell markup lives in +// lib/renderer.js (outside both report template copies) and is expanded via +// a Mustache partial, preserving the rendered report output byte-for-byte. +// --------------------------------------------------------------------------- + +describe('source-link cell partial (kept outside the report templates)', () => { + const junitFixture = JSON.parse( + readFileSync(path.join(import.meta.dir, '../fixtures/junit4-report.json'), 'utf8') + ); + + it('renders the class-relationships source-link cell through {{> source-link-cell}}', () => { + 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'); + }); + + it('expands the partial to the same bytes the templates embedded inline', () => { + const inline = [ + '', + ' ', + ' ', + ' ', + ' ', + '
{{{renderedLabel}}}{{priority}}
' + ].join('\n') + '\n'; + const viaPartial = [ + '', + ' ', + ' {{> source-link-cell}}', + ' ', + ' ', + '
{{priority}}
' + ].join('\n') + '\n'; + const row = junitFixture.classRelationshipsToRemove.relationships[0]; + const rendered = renderTemplate(viaPartial, row); + expect(rendered).toBe(renderTemplate(inline, row)); + expect(rendered).toContain(''); + expect(rendered).toContain(' sort -> paginate pipeline feeding the report // template with paginated rows and tableUi metadata (plan Phase 3). diff --git a/tests/unit/report-template.test.js b/tests/unit/report-template.test.js index 0cac388..68e7b74 100644 --- a/tests/unit/report-template.test.js +++ b/tests/unit/report-template.test.js @@ -25,4 +25,16 @@ describe('bundled report template', () => { it('the served public/ copy is identical to the source template', () => { expect(loadTemplate('public')).toBe(loadTemplate('assets')); }); + + it('renders the class-relationships source link via the shared partial, not inline', () => { + for (const which of ['assets', 'public']) { + const template = loadTemplate(which); + const start = template.indexOf('{{#classRelationshipsToRemove.relationships}}'); + const end = template.indexOf('{{/classRelationshipsToRemove.relationships}}'); + expect(start).toBeGreaterThan(-1); + const rows = template.slice(start, end); + expect(rows).toContain('{{> source-link-cell}}'); + expect(rows).not.toContain('{{{renderedLabel}}}'); + } + }); }); diff --git a/tests/unit/report-view.test.js b/tests/unit/report-view.test.js index 0c98592..151ebe6 100644 --- a/tests/unit/report-view.test.js +++ b/tests/unit/report-view.test.js @@ -185,6 +185,36 @@ describe('initBubbleChart', () => { 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 a pointer cursor while hovering a bubble with a url', () => { const created = []; window.Chart = function (ctx, config) { created.push(config); }; From 2b7a329b4ac11f7d0cb9d7ee72ee151ff40944d9 Mon Sep 17 00:00:00 2001 From: Jim Bethancourt Date: Fri, 2 Oct 2026 17:53:52 -0500 Subject: [PATCH 4/6] Applied CodeRabbit suggestion: Add tooltip for hover effect only if URL is an HTTP or HTTPS URL --- lib/report-view.js | 6 +++++- tests/unit/report-view.test.js | 17 +++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/lib/report-view.js b/lib/report-view.js index b82f0a5..b3b4292 100644 --- a/lib/report-view.js +++ b/lib/report-view.js @@ -627,7 +627,11 @@ export function initBubbleChart(canvas, title, chartData) { `Effort Rank: ${raw.x}`, `Change Proneness Rank: ${raw.y}` ]; - if (raw.url) lines.push('Click to open source file'); + // 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/tests/unit/report-view.test.js b/tests/unit/report-view.test.js index 151ebe6..2f4851d 100644 --- a/tests/unit/report-view.test.js +++ b/tests/unit/report-view.test.js @@ -215,6 +215,23 @@ describe('initBubbleChart', () => { 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 while hovering a bubble with a url', () => { const created = []; window.Chart = function (ctx, config) { created.push(config); }; From 5c29dae2dc6c1439a1366277dbb265673b84c681 Mon Sep 17 00:00:00 2001 From: Jim Bethancourt Date: Fri, 2 Oct 2026 22:13:17 -0500 Subject: [PATCH 5/6] Applied CodeRabbit suggestions --- assets/refactor-first-report.mustache | 2 +- lib/renderer.js | 13 +------ lib/report-view.js | 18 ++++++++-- public/assets/refactor-first-report.mustache | 2 +- tests/unit/renderer.test.js | 36 ++++---------------- tests/unit/report-template.test.js | 12 ------- tests/unit/report-view.test.js | 16 +++++++-- 7 files changed, 39 insertions(+), 60 deletions(-) diff --git a/assets/refactor-first-report.mustache b/assets/refactor-first-report.mustache index 77770f9..5ffffa5 100644 --- a/assets/refactor-first-report.mustache +++ b/assets/refactor-first-report.mustache @@ -469,7 +469,7 @@ {{#classRelationshipsToRemove.relationships}} - {{> source-link-cell}} + {{{renderedLabel}}} {{priority}} {{cycleCount}} {{effortRank}} diff --git a/lib/renderer.js b/lib/renderer.js index 0116c2f..60f0ca9 100644 --- a/lib/renderer.js +++ b/lib/renderer.js @@ -33,17 +33,6 @@ export function initializeMustache() { let linkSafetyHookRegistered = false; -// The class-relationships source-link cell, rendered from the report data's -// untrusted pre-rendered label. The markup lives here — outside both report -// template copies (assets/ and public/assets/) — and renderTemplate injects -// it as the Mustache partial "source-link-cell", referenced by the templates -// through {{> source-link-cell}}. The trailing CRLF matches the templates' -// line endings, so the partial expands to the exact bytes the templates -// previously embedded inline. -const REPORT_PARTIALS = { - 'source-link-cell': '{{{renderedLabel}}}\r\n' -}; - /** * Registers a DOMPurify hook that protects links opening a new browsing context. * @@ -247,7 +236,7 @@ export function prepareReportData(data, tableStates = {}, config = TABLE_CONFIG) */ export function renderTemplate(template, data) { const mustache = initializeMustache(); - const rendered = mustache.render(template, data, REPORT_PARTIALS); + const rendered = mustache.render(template, data); registerLinkSafetyHook(); // Allow-list is deliberately tight: the bundled template only needs text, // table, popup and chart markup; elements like iframe/object/embed/meta/ diff --git a/lib/report-view.js b/lib/report-view.js index b3b4292..fd13c59 100644 --- a/lib/report-view.js +++ b/lib/report-view.js @@ -583,10 +583,22 @@ export function initBubbleChart(canvas, title, chartData) { window.open(url, '_blank', 'noopener'); } }, - // Pointer affordance while the cursor is over a bubble. - onHover: (event, elements) => { + // 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; - event.native.target.style.cursor = elements && elements.length ? 'pointer' : 'default'; + 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: { diff --git a/public/assets/refactor-first-report.mustache b/public/assets/refactor-first-report.mustache index 77770f9..5ffffa5 100644 --- a/public/assets/refactor-first-report.mustache +++ b/public/assets/refactor-first-report.mustache @@ -469,7 +469,7 @@ {{#classRelationshipsToRemove.relationships}} - {{> source-link-cell}} + {{{renderedLabel}}} {{priority}} {{cycleCount}} {{effortRank}} diff --git a/tests/unit/renderer.test.js b/tests/unit/renderer.test.js index a5eee5b..f78d0d3 100644 --- a/tests/unit/renderer.test.js +++ b/tests/unit/renderer.test.js @@ -185,17 +185,19 @@ describe('templating safety (repository-provided templates are untrusted)', () = }); // --------------------------------------------------------------------------- -// Source-link cell partial: the class-relationships cell markup lives in -// lib/renderer.js (outside both report template copies) and is expanded via -// a Mustache partial, preserving the rendered report output byte-for-byte. +// Source-link cell output: both report templates render the class- +// relationships cell inline ({{{renderedLabel}}}); the rendering layer owns +// the link-safety behavior around it. These guards pin the rendered output +// so template or renderer changes cannot silently drop or weaken the +// source links. // --------------------------------------------------------------------------- -describe('source-link cell partial (kept outside the report templates)', () => { +describe('source-link cell rendering (owned by the report rendering layer)', () => { const junitFixture = JSON.parse( readFileSync(path.join(import.meta.dir, '../fixtures/junit4-report.json'), 'utf8') ); - it('renders the class-relationships source-link cell through {{> source-link-cell}}', () => { + 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' ); @@ -210,30 +212,6 @@ describe('source-link cell partial (kept outside the report templates)', () => { expect(link.getAttribute('href')).toContain('https://github.com/junit-team/junit4/blob/'); expect(link.getAttribute('rel')).toBe('noopener noreferrer'); }); - - it('expands the partial to the same bytes the templates embedded inline', () => { - const inline = [ - '', - ' ', - ' ', - ' ', - ' ', - '
{{{renderedLabel}}}{{priority}}
' - ].join('\n') + '\n'; - const viaPartial = [ - '', - ' ', - ' {{> source-link-cell}}', - ' ', - ' ', - '
{{priority}}
' - ].join('\n') + '\n'; - const row = junitFixture.classRelationshipsToRemove.relationships[0]; - const rendered = renderTemplate(viaPartial, row); - expect(rendered).toBe(renderTemplate(inline, row)); - expect(rendered).toContain(''); - expect(rendered).toContain(' { it('the served public/ copy is identical to the source template', () => { expect(loadTemplate('public')).toBe(loadTemplate('assets')); }); - - it('renders the class-relationships source link via the shared partial, not inline', () => { - for (const which of ['assets', 'public']) { - const template = loadTemplate(which); - const start = template.indexOf('{{#classRelationshipsToRemove.relationships}}'); - const end = template.indexOf('{{/classRelationshipsToRemove.relationships}}'); - expect(start).toBeGreaterThan(-1); - const rows = template.slice(start, end); - expect(rows).toContain('{{> source-link-cell}}'); - expect(rows).not.toContain('{{{renderedLabel}}}'); - } - }); }); diff --git a/tests/unit/report-view.test.js b/tests/unit/report-view.test.js index 2f4851d..805245f 100644 --- a/tests/unit/report-view.test.js +++ b/tests/unit/report-view.test.js @@ -232,19 +232,31 @@ describe('initBubbleChart', () => { expect(promptFor(undefined)).toEqual([]); }); - it('shows a pointer cursor while hovering a bubble with a url', () => { + 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' }] + 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', () => { From 36a1d09d0d72484cec9e526b17a0a60f6815c53c Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Sat, 3 Oct 2026 19:14:06 +0000 Subject: [PATCH 6/6] Fix CodeRabbit issues in PR #17 --- assets/refactor-first-report.mustache | 4 +- lib/renderer.js | 25 +++++++++- public/assets/refactor-first-report.mustache | 4 +- tests/unit/renderer.test.js | 48 +++++++++++++++++++- 4 files changed, 74 insertions(+), 7 deletions(-) diff --git a/assets/refactor-first-report.mustache b/assets/refactor-first-report.mustache index 5ffffa5..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/public/assets/refactor-first-report.mustache b/public/assets/refactor-first-report.mustache index 5ffffa5..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/unit/renderer.test.js b/tests/unit/renderer.test.js index f78d0d3..eccccf5 100644 --- a/tests/unit/renderer.test.js +++ b/tests/unit/renderer.test.js @@ -186,7 +186,7 @@ describe('templating safety (repository-provided templates are untrusted)', () = // --------------------------------------------------------------------------- // Source-link cell output: both report templates render the class- -// relationships cell inline ({{{renderedLabel}}}); the rendering layer owns +// relationships cell inline; the rendering layer owns // the link-safety behavior around it. These guards pin the rendered output // so template or renderer changes cannot silently drop or weaken the // source links. @@ -197,6 +197,52 @@ describe('source-link cell rendering (owned by the report rendering layer)', () 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'