fix: add escapeHtml XSS tests and consolidate duplicate test factories (#510)

Add 14 security-focused regression tests for escapeHtml() covering all
HTML entity escapes, XSS vectors (script injection, event handlers,
javascript: protocol), double-escaping, and realistic PR title content.

Consolidate 4 duplicate makePR() factory functions across daily.test.ts,
daily-orchestration.test.ts, and pr-monitor.test.ts to use the shared
makeFetchedPR() from test-utils.ts with thin semantic wrappers.

Closes #486

Author: costajohnt

packages/core/src/commands/daily-orchestration.test.ts

@@ -8,6 +8,7 @@
 
 import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
 import type { FetchedPR, DailyDigest, CommentedIssue } from '../core/types.js';
+import { makeFetchedPR } from '../core/test-utils.js';
 import type { FetchPRsResult } from '../core/pr-monitor.js';
 
 // ---------------------------------------------------------------------------
@@ -98,30 +99,9 @@ import { requireGitHubToken } from '../core/index.js';
 // Test helpers
 // ---------------------------------------------------------------------------
 
-/** Create a minimal FetchedPR for orchestration testing */
-function makePR(overrides: Partial<FetchedPR> & { repo: string; number?: number }): FetchedPR {
-  const num = overrides.number ?? 1;
-  return {
-    id: num,
-    url: `https://github.com/${overrides.repo}/pull/${num}`,
-    number: num,
-    title: 'Test PR',
-    createdAt: '2026-01-01T00:00:00Z',
-    updatedAt: '2026-01-15T00:00:00Z',
-    daysSinceActivity: 5,
-    ciStatus: 'passing',
-    failingCheckNames: [],
-    classifiedChecks: [],
-    hasMergeConflict: false,
-    reviewDecision: 'approved',
-    hasUnrespondedComment: false,
-    hasIncompleteChecklist: false,
-    maintainerActionHints: [],
-    status: 'healthy',
-    displayLabel: '[Healthy]',
-    displayDescription: 'Everything looks good — normal review cycle',
-    ...overrides,
-  };
+/** Create a FetchedPR with repo as required (mirrors orchestration's per-repo grouping). */
+function makePR(overrides: Parameters<typeof makeFetchedPR>[0] & { repo: string }) {
+  return makeFetchedPR({ daysSinceActivity: 5, ...overrides });
 }
 
 /** Build a minimal DailyDigest for mock responses */

packages/core/src/commands/daily.test.ts

@@ -5,33 +5,13 @@
 import { describe, it, expect } from 'vitest';
 import { computeRepoSignals, computeActionMenu, groupPRsByRepo, toShelvedPRRef } from './daily.js';
 import { deduplicateDigest, compactActionableIssues, compactRepoGroups } from '../formatters/json.js';
-import type { FetchedPR, DailyDigest, CommentedIssue } from '../core/types.js';
+import type { DailyDigest, CommentedIssue } from '../core/types.js';
 import type { ActionableIssue, CapacityAssessment } from '../formatters/json.js';
+import { makeFetchedPR } from '../core/test-utils.js';
 
-/** Create a minimal FetchedPR for testing signal computation */
-function makePR(overrides: Partial<FetchedPR> & { repo: string }): FetchedPR {
-  const num = (overrides as { number?: number }).number ?? 1;
-  return {
-    id: num,
-    url: `https://github.com/${overrides.repo}/pull/${num}`,
-    number: num,
-    title: 'Test PR',
-    createdAt: '2026-01-01T00:00:00Z',
-    updatedAt: '2026-01-15T00:00:00Z',
-    daysSinceActivity: 5,
-    ciStatus: 'passing',
-    failingCheckNames: [],
-    classifiedChecks: [],
-    hasMergeConflict: false,
-    reviewDecision: 'approved',
-    hasUnrespondedComment: false,
-    hasIncompleteChecklist: false,
-    maintainerActionHints: [],
-    status: 'healthy',
-    displayLabel: '[Healthy]',
-    displayDescription: 'Everything looks good — normal review cycle',
-    ...overrides,
-  };
+/** Create a FetchedPR with repo as required (mirrors daily command's per-repo grouping). */
+function makePR(overrides: Parameters<typeof makeFetchedPR>[0] & { repo: string }) {
+  return makeFetchedPR({ daysSinceActivity: 5, ...overrides });
 }
 
 /** Create a minimal CapacityAssessment for testing */

packages/core/src/commands/dashboard-formatters.test.ts

@@ -0,0 +1,78 @@
+/**
+ * Tests for dashboard-formatters.ts — HTML escaping and XSS prevention.
+ */
+
+import { describe, it, expect } from 'vitest';
+import { escapeHtml } from './dashboard-formatters.js';
+
+describe('escapeHtml', () => {
+  it('should escape ampersand', () => {
+    expect(escapeHtml('foo & bar')).toBe('foo & bar');
+  });
+
+  it('should escape less-than', () => {
+    expect(escapeHtml('a < b')).toBe('a &lt; b');
+  });
+
+  it('should escape greater-than', () => {
+    expect(escapeHtml('a > b')).toBe('a &gt; b');
+  });
+
+  it('should escape double quotes', () => {
+    expect(escapeHtml('say "hello"')).toBe('say &quot;hello&quot;');
+  });
+
+  it('should escape single quotes', () => {
+    expect(escapeHtml("it's fine")).toBe('it&#39;s fine');
+  });
+
+  it('should escape all special characters at once', () => {
+    expect(escapeHtml('<div class="x" data-val=\'y\'>&</div>')).toBe(
+      '&lt;div class=&quot;x&quot; data-val=&#39;y&#39;&gt;&amp;&lt;/div&gt;',
+    );
+  });
+
+  it('should return empty string unchanged', () => {
+    expect(escapeHtml('')).toBe('');
+  });
+
+  it('should return plain text unchanged', () => {
+    expect(escapeHtml('Hello World 123')).toBe('Hello World 123');
+  });
+
+  // XSS-focused regression tests
+  it('should neutralize script injection', () => {
+    const result = escapeHtml('<script>alert("xss")</script>');
+    expect(result).not.toContain('<script>');
+    expect(result).toBe('&lt;script&gt;alert(&quot;xss&quot;)&lt;/script&gt;');
+  });
+
+  it('should neutralize event handler injection', () => {
+    const result = escapeHtml('<img src=x onerror="alert(1)">');
+    expect(result).not.toContain('<img');
+    expect(result).toBe('&lt;img src=x onerror=&quot;alert(1)&quot;&gt;');
+  });
+
+  it('should neutralize nested HTML tags', () => {
+    const result = escapeHtml('<a href="javascript:void(0)">click</a>');
+    expect(result).not.toContain('<a ');
+    expect(result).toBe('&lt;a href=&quot;javascript:void(0)&quot;&gt;click&lt;/a&gt;');
+  });
+
+  it('should handle double-escaping correctly (already escaped input)', () => {
+    // If input already contains &amp; it should be escaped again
+    expect(escapeHtml('&amp;')).toBe('&amp;amp;');
+  });
+
+  it('should escape PR titles with special characters', () => {
+    // Realistic PR title from GitHub
+    expect(escapeHtml('fix: handle <T> generic in parse() & format()')).toBe(
+      'fix: handle &lt;T&gt; generic in parse() &amp; format()',
+    );
+  });
+
+  it('should escape multi-line content', () => {
+    const input = 'line1 <b>\nline2 "quoted"\nline3 & more';
+    expect(escapeHtml(input)).toBe('line1 &lt;b&gt;\nline2 &quot;quoted&quot;\nline3 &amp; more');
+  });
+});

packages/core/src/core/pr-monitor.test.ts

@@ -45,6 +45,7 @@ const { analyzeChecklist } = await import('./checklist-analysis.js');
 const { extractMaintainerActionHints } = await import('./maintainer-analysis.js');
 const { determineReviewDecision, getLatestChangesRequestedDate, checkUnrespondedComments, isAllSelfReplies } =
   await import('./review-analysis.js');
+const { makeFetchedPR } = await import('./test-utils.js');
 
 describe('PRMonitor CI status deduplication', () => {
   const emptyCombinedStatus = {
@@ -1020,29 +1021,8 @@ describe('PRMonitor getCIStatus auth-gate filtering', () => {
 });
 
 describe('PRMonitor generateDigest', () => {
-  function makeFetchedPR(overrides: Partial<import('./types.js').FetchedPR> = {}): import('./types.js').FetchedPR {
-    return {
-      id: 1,
-      url: 'https://github.com/owner/repo/pull/1',
-      repo: 'owner/repo',
-      number: 1,
-      title: 'Test PR',
-      status: 'healthy',
-      displayLabel: '[Healthy]',
-      displayDescription: 'Everything looks good — normal review cycle',
-      createdAt: '2026-02-01T00:00:00Z',
-      updatedAt: '2026-02-07T00:00:00Z',
-      daysSinceActivity: 1,
-      ciStatus: 'passing',
-      failingCheckNames: [],
-      classifiedChecks: [],
-      hasMergeConflict: false,
-      reviewDecision: 'review_required',
-      hasUnrespondedComment: false,
-      hasIncompleteChecklist: false,
-      maintainerActionHints: [],
-      ...overrides,
-    };
+  function makeDigestPR(overrides: Parameters<typeof makeFetchedPR>[0] = {}) {
+    return makeFetchedPR({ reviewDecision: 'review_required', ...overrides });
   }
 
   beforeEach(() => {
@@ -1064,17 +1044,17 @@ describe('PRMonitor generateDigest', () => {
 
   it('should categorize PRs by status', () => {
     const prs = [
-      makeFetchedPR({ status: 'needs_response', number: 1 }),
-      makeFetchedPR({ status: 'failing_ci', number: 2 }),
-      makeFetchedPR({ status: 'merge_conflict', number: 3 }),
-      makeFetchedPR({ status: 'healthy', number: 4 }),
-      makeFetchedPR({ status: 'dormant', number: 5 }),
-      makeFetchedPR({ status: 'approaching_dormant', number: 6 }),
-      makeFetchedPR({ status: 'needs_changes', number: 7 }),
-      makeFetchedPR({ status: 'changes_addressed', number: 8 }),
-      makeFetchedPR({ status: 'waiting_on_maintainer', number: 9 }),
-      makeFetchedPR({ status: 'incomplete_checklist', number: 10 }),
-      makeFetchedPR({ status: 'waiting', number: 11 }),
+      makeDigestPR({ status: 'needs_response', number: 1 }),
+      makeDigestPR({ status: 'failing_ci', number: 2 }),
+      makeDigestPR({ status: 'merge_conflict', number: 3 }),
+      makeDigestPR({ status: 'healthy', number: 4 }),
+      makeDigestPR({ status: 'dormant', number: 5 }),
+      makeDigestPR({ status: 'approaching_dormant', number: 6 }),
+      makeDigestPR({ status: 'needs_changes', number: 7 }),
+      makeDigestPR({ status: 'changes_addressed', number: 8 }),
+      makeDigestPR({ status: 'waiting_on_maintainer', number: 9 }),
+      makeDigestPR({ status: 'incomplete_checklist', number: 10 }),
+      makeDigestPR({ status: 'waiting', number: 11 }),
     ];
 
     const monitor = new PRMonitor('fake-token');
@@ -1094,18 +1074,18 @@ describe('PRMonitor generateDigest', () => {
 
   it('should calculate totalNeedingAttention correctly', () => {
     const prs = [
-      makeFetchedPR({ status: 'needs_response', number: 1 }),
-      makeFetchedPR({ status: 'needs_changes', number: 2 }),
-      makeFetchedPR({ status: 'failing_ci', number: 3 }),
-      makeFetchedPR({ status: 'merge_conflict', number: 4 }),
-      makeFetchedPR({ status: 'needs_rebase', number: 5 }),
-      makeFetchedPR({ status: 'missing_required_files', number: 6 }),
-      makeFetchedPR({ status: 'incomplete_checklist', number: 7 }),
+      makeDigestPR({ status: 'needs_response', number: 1 }),
+      makeDigestPR({ status: 'needs_changes', number: 2 }),
+      makeDigestPR({ status: 'failing_ci', number: 3 }),
+      makeDigestPR({ status: 'merge_conflict', number: 4 }),
+      makeDigestPR({ status: 'needs_rebase', number: 5 }),
+      makeDigestPR({ status: 'missing_required_files', number: 6 }),
+      makeDigestPR({ status: 'incomplete_checklist', number: 7 }),
       // These should NOT count toward totalNeedingAttention
-      makeFetchedPR({ status: 'healthy', number: 8 }),
-      makeFetchedPR({ status: 'waiting', number: 9 }),
-      makeFetchedPR({ status: 'dormant', number: 10 }),
-      makeFetchedPR({ status: 'waiting_on_maintainer', number: 11 }),
+      makeDigestPR({ status: 'healthy', number: 8 }),
+      makeDigestPR({ status: 'waiting', number: 9 }),
+      makeDigestPR({ status: 'dormant', number: 10 }),
+      makeDigestPR({ status: 'waiting_on_maintainer', number: 11 }),
     ];
 
     const monitor = new PRMonitor('fake-token');
@@ -1764,29 +1744,8 @@ describe('isAcknowledgmentComment (Issue #69)', () => {
 });
 
 describe('computeDisplayLabel (#79)', () => {
-  function makePR(overrides: Partial<import('./types.js').FetchedPR> = {}): import('./types.js').FetchedPR {
-    return {
-      id: 1,
-      url: 'https://github.com/owner/repo/pull/1',
-      repo: 'owner/repo',
-      number: 1,
-      title: 'Test PR',
-      status: 'healthy',
-      displayLabel: '',
-      displayDescription: '',
-      createdAt: '2026-02-01T00:00:00Z',
-      updatedAt: '2026-02-07T00:00:00Z',
-      daysSinceActivity: 1,
-      ciStatus: 'passing',
-      failingCheckNames: [],
-      classifiedChecks: [],
-      hasMergeConflict: false,
-      reviewDecision: 'review_required',
-      hasUnrespondedComment: false,
-      hasIncompleteChecklist: false,
-      maintainerActionHints: [],
-      ...overrides,
-    };
+  function makePR(overrides: Parameters<typeof makeFetchedPR>[0] = {}) {
+    return makeFetchedPR({ displayLabel: '', displayDescription: '', reviewDecision: 'review_required', ...overrides });
   }
 
   it('should return [Healthy] for healthy status', () => {