[codex] collapse duplicate report markdown rendering

e2e/dashboard-pages.spec.ts

@@ -52,3 +52,20 @@ test('history page renders seeded analysis entry', async ({ page }) => {
   await expect(page.getByText('E2E validation PR')).toBeVisible();
   expect(pageErrors).toEqual([]);
 });
+
+test('report page keeps low-signal markdown visible and sanitizes unsafe markdown', async ({ page }) => {
+  const pageErrors = [];
+  page.on('pageerror', (error) => pageErrors.push(error.message));
+
+  await page.goto('/dashboard/reports/9201');
+  await expect(page.getByRole('heading', { name: 'Review 报告 #9201' })).toBeVisible();
+  await expect(page.getByRole('heading', { name: '原始 Markdown 报告' })).toBeVisible();
+  await expect(page.getByText('E2E raw markdown body should be visible by default.')).toBeVisible();
+  await expect(page.getByRole('button', { name: '收起原始报告' })).toBeVisible();
+  await expect(page.getByRole('link', { name: 'unsafe link' })).toHaveAttribute('href', /#$/);
+  await expect(page.locator('script', { hasText: 'window.__codagraphXss' })).toHaveCount(0);
+
+  const xssFlag = await page.evaluate(() => (window as unknown as { __codagraphXss?: boolean }).__codagraphXss);
+  expect(xssFlag).toBeUndefined();
+  expect(pageErrors).toEqual([]);
+});

scripts/seed-e2e-data.js

@@ -12,6 +12,76 @@ const seededAdminPasswordHash = crypto
   .createHash('sha256')
   .update('changeme')
   .digest('hex');
+const seededReportPayload = {
+  generatedAt: new Date().toISOString(),
+  jobId: 9301,
+  riskLevel: 'low',
+  confidence: 'low',
+  reviewMode: 'normal',
+  summary: 'E2E report with low-signal structured metadata keeps markdown visible.',
+  reportMarkdown: [
+    '# E2E Raw Markdown',
+    '',
+    'E2E raw markdown body should be visible by default.',
+    '',
+    '<script>window.__codagraphXss = true</script>',
+    '',
+    '[unsafe link](javascript:alert(1))',
+  ].join('\n'),
+  files: [
+    {
+      path: 'src/e2e-low.ts',
+      status: 'modified',
+      additions: 1,
+      deletions: 0,
+      changes: 1,
+    },
+  ],
+  fileReviews: [
+    {
+      filePath: 'src/e2e-low.ts',
+      status: 'modified',
+      language: 'typescript',
+      fileSummary: 'Metadata-only file context used by report-page E2E.',
+      findings: [],
+      patch: '@@ -1,1 +1,2 @@\n export const value = 1;\n+export const lowSignal = true;\n',
+      semanticContext: {
+        changedSymbols: [],
+        relatedSnippets: [],
+        impactReferences: [],
+        relatedTests: [],
+        contextEngineAvailable: false,
+      },
+      usedFallback: true,
+    },
+  ],
+  findings: [
+    {
+      filePath: 'src/e2e-low.ts',
+      lineNumber: 2,
+      severity: 'low',
+      category: 'style',
+      title: 'Low-signal E2E finding',
+      description: 'This low-severity finding should not collapse the raw markdown by default.',
+      suggestion: 'Keep the raw markdown visible unless high-signal findings exist.',
+      source: 'e2e',
+    },
+  ],
+  coverage: {
+    totalFiles: 1,
+    reviewedFiles: 1,
+    skippedFiles: [],
+    partialReview: false,
+  },
+  nextActions: [],
+  suppressedFindings: [],
+  trace: {
+    mode: 'normal',
+    promptVersion: 'e2e',
+    generatedAt: new Date().toISOString(),
+    entries: [],
+  },
+};
 
 try {
   db.exec(`
@@ -133,6 +203,12 @@ try {
   );
   `);
 
+  db.prepare(`
+    UPDATE analysis
+    SET analysis_result = ?, comment_count = ?, file_count = ?, issue_count = ?
+    WHERE id = 9201
+  `).run(JSON.stringify(seededReportPayload), 3, 1, seededReportPayload.findings.length);
+
   console.log(`Seeded E2E data into ${dbPath}`);
 } finally {
   db.close();

server/src/routes/analysisRoutes.ts

@@ -35,6 +35,7 @@ import {
 import { getOAuthInstallationService } from '../services/OAuthInstallationService';
 import type { ReviewTracePayload } from '../review/reviewTrace';
 import type { ReviewCoverageSummary, ReviewConfidence, SuppressedFinding } from '../review/reviewPrioritization';
+import { sanitizeMarkdownForStorage } from '../utils/markdownSanitizer';
 
 const router = express.Router();
 
@@ -234,7 +235,13 @@ router.get('/pull-requests', async (req: Request, res: Response) => {
 
 function parseReportPayload(raw: string): StoredReviewReportPayload | null {
   try {
-    return JSON.parse(raw) as StoredReviewReportPayload;
+    const payload = JSON.parse(raw) as StoredReviewReportPayload;
+    return {
+      ...payload,
+      reportMarkdown: typeof payload.reportMarkdown === 'string'
+        ? sanitizeMarkdownForStorage(payload.reportMarkdown)
+        : payload.reportMarkdown,
+    };
   } catch {
     return null;
   }

server/src/services/ReviewExecutionService.test.ts

@@ -366,6 +366,44 @@ describe('ReviewExecutionService', () => {
     expect(persistedPayload.reportMarkdown).toContain('- Improve Trace: 1');
   });
 
+  it('sanitizes generated markdown before persisting the report payload', async () => {
+    platformApiClientMock.getPullRequest.mockResolvedValue({
+      number: 42,
+      title: '<img src=x onerror=alert(1)> Improve review engine',
+      head: {
+        sha: 'head-sha',
+        ref: 'feature/review',
+        repo: { full_name: 'mars/lite' },
+      },
+      base: {
+        sha: 'base-sha',
+        ref: 'main',
+        repo: { full_name: 'mars/lite' },
+      },
+    });
+
+    const reviewResult = createDefaultReviewResult();
+    reviewResult.allFindings[0].description = '<script>alert(1)</script> [details](javascript:alert(1))';
+    reviewEngineReviewMock.mockResolvedValueOnce(reviewResult);
+
+    const service = new ReviewExecutionService();
+    await service.execute(1002, JSON.stringify({
+      platform: 'github',
+      repo_name: 'mars/lite',
+      pr_number: '42',
+      repository_id: '7',
+      analysis_id: '13',
+      analysis_job_id: '17',
+    }));
+
+    const persistedPayload = JSON.parse(analysisModelMock.markComplete.mock.calls[0][1]);
+    expect(persistedPayload.reportMarkdown).toContain('&lt;img src=x onerror=alert(1)&gt;');
+    expect(persistedPayload.reportMarkdown).toContain('&lt;script&gt;alert(1)&lt;/script&gt;');
+    expect(persistedPayload.reportMarkdown).toContain('[details](#)');
+    expect(persistedPayload.reportMarkdown).not.toContain('<script>');
+    expect(persistedPayload.reportMarkdown).not.toContain('javascript:alert');
+  });
+
   it('falls back to single inline comments and a summary comment when GitHub batch review fails', async () => {
     commentClientMock.submitReview.mockRejectedValueOnce(new Error('review batch rejected'));
     const service = new ReviewExecutionService();

server/src/services/ReviewExecutionService.ts

@@ -20,6 +20,7 @@ import { getConfig } from '../config';
 import { isAuthenticationFailure } from '../utils/authFailures';
 import { resolveRepositoryCoordinates } from '../utils/repositoryCoordinates';
 import { sanitizeLogText } from '../utils/redactSensitive';
+import { sanitizeMarkdownForStorage } from '../utils/markdownSanitizer';
 import {
   AdvancedReviewEngine,
   type ReviewFileInput,
@@ -679,7 +680,7 @@ export class ReviewExecutionService {
 
     fallbackFindings = dedupeFindings(fallbackFindings);
 
-    const reportMarkdown = buildMarkdownReport(pullRequest, findings, riskLevel, summary, {
+    const reportMarkdown = sanitizeMarkdownForStorage(buildMarkdownReport(pullRequest, findings, riskLevel, summary, {
       mode: advancedReview.mode,
       reviewMode,
       confidence: advancedReview.confidence,
@@ -691,7 +692,7 @@ export class ReviewExecutionService {
       skippedFiles: advancedReview.coverage.skippedFiles.length,
       nextActions: advancedReview.nextActions,
       traceEntryCount: advancedReview.trace?.entries.length || 0,
-    });
+    }));
     const reportPayload = {
       generatedAt: new Date().toISOString(),
       jobId,

server/src/utils/markdownSanitizer.test.ts

@@ -0,0 +1,32 @@
+import { sanitizeMarkdownForStorage, sanitizeMarkdownUrl } from './markdownSanitizer';
+
+describe('markdownSanitizer', () => {
+  it('encodes raw HTML while preserving markdown text', () => {
+    const markdown = [
+      '# Review',
+      '',
+      '- 标题: <img src=x onerror=alert(1)>',
+      '- 描述: <script>alert(1)</script> kept text',
+    ].join('\n');
+
+    const sanitized = sanitizeMarkdownForStorage(markdown);
+
+    expect(sanitized).toContain('&lt;img src=x onerror=alert(1)&gt;');
+    expect(sanitized).toContain('&lt;script&gt;alert(1)&lt;/script&gt; kept text');
+    expect(sanitized).not.toContain('<script>');
+    expect(sanitized).not.toContain('<img');
+  });
+
+  it('replaces unsafe markdown link protocols', () => {
+    expect(sanitizeMarkdownForStorage('[run](javascript:alert(1))')).toBe('[run](#)');
+    expect(sanitizeMarkdownForStorage('[run](JaVaScRiPt%3Aalert(1))')).toBe('[run](#)');
+    expect(sanitizeMarkdownForStorage('[safe](https://example.com/report)')).toBe('[safe](https://example.com/report)');
+  });
+
+  it('allows relative and approved absolute URLs', () => {
+    expect(sanitizeMarkdownUrl('/dashboard/reports/1')).toBe('/dashboard/reports/1');
+    expect(sanitizeMarkdownUrl('../reports/1')).toBe('../reports/1');
+    expect(sanitizeMarkdownUrl('mailto:security@example.com')).toBe('mailto:security@example.com');
+    expect(sanitizeMarkdownUrl('data:text/html,<script>alert(1)</script>')).toBe('#');
+  });
+});

server/src/utils/markdownSanitizer.ts

@@ -0,0 +1,67 @@
+const RAW_HTML_COMMENT_PATTERN = /<!--[\s\S]*?-->/g;
+const RAW_HTML_TAG_PATTERN = /<\/?[A-Za-z][A-Za-z0-9:-]*(?:[^<>]*)?>/g;
+const MARKDOWN_LINK_DESTINATION_PATTERN = /(\]\()((?:[^()\s]+|\([^()\s]*\))+)(\))/g;
+const SAFE_URL_PROTOCOLS = new Set(['http:', 'https:', 'mailto:', 'tel:']);
+
+function encodeHtmlToken(value: string): string {
+  return value
+    .replace(/&/g, '&amp;')
+    .replace(/</g, '&lt;')
+    .replace(/>/g, '&gt;');
+}
+
+function decodeUrlForProtocolCheck(value: string): string {
+  let decoded = value.trim().replace(/[\u0000-\u001F\u007F\s]+/g, '');
+
+  for (let attempt = 0; attempt < 2; attempt += 1) {
+    try {
+      const next = decodeURIComponent(decoded);
+      if (next === decoded) {
+        break;
+      }
+      decoded = next;
+    } catch {
+      break;
+    }
+  }
+
+  return decoded.toLowerCase();
+}
+
+function hasExplicitProtocol(value: string): boolean {
+  const colon = value.indexOf(':');
+  if (colon === -1) {
+    return false;
+  }
+
+  const slash = value.indexOf('/');
+  const questionMark = value.indexOf('?');
+  const hash = value.indexOf('#');
+
+  return !(
+    (slash !== -1 && colon > slash) ||
+    (questionMark !== -1 && colon > questionMark) ||
+    (hash !== -1 && colon > hash)
+  );
+}
+
+export function sanitizeMarkdownUrl(value: string): string {
+  const trimmed = value.trim().replace(/^<|>$/g, '');
+  const normalized = decodeUrlForProtocolCheck(trimmed);
+
+  if (!hasExplicitProtocol(normalized)) {
+    return trimmed;
+  }
+
+  const protocol = normalized.slice(0, normalized.indexOf(':') + 1);
+  return SAFE_URL_PROTOCOLS.has(protocol) ? trimmed : '#';
+}
+
+export function sanitizeMarkdownForStorage(value: string): string {
+  return value
+    .replace(RAW_HTML_COMMENT_PATTERN, (match) => encodeHtmlToken(match))
+    .replace(RAW_HTML_TAG_PATTERN, (match) => encodeHtmlToken(match))
+    .replace(MARKDOWN_LINK_DESTINATION_PATTERN, (_match, open: string, url: string, close: string) => (
+      `${open}${sanitizeMarkdownUrl(url)}${close}`
+    ));
+}