pika-news: auto-refresh, markdown rendering, wider diffs#489
pika-news: auto-refresh, markdown rendering, wider diffs#489justinmoon merged 1 commit intomasterfrom
Conversation
- Auto-reload page every 10s when generation status is pending/generating - Add marked.js for client-side markdown in chat assistant responses - Evidence snippets use pre/code blocks instead of inline code - Add CSS for markdown in panels and chat messages - Remove shell/page-layout max-width constraints for wider diffs - Change d2h-wrapper from overflow:hidden to overflow-x:auto Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughA template file was updated to add Markdown rendering support for assistant messages using marked.js, restructure evidence display to use code blocks instead of inline code, implement client-side auto-refresh logic during generation, and adjust CSS styling for Markdown blocks and layout constraints. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if (role === 'assistant' && window.marked) { | ||
| div.innerHTML = marked.parse(content); | ||
| } else { |
There was a problem hiding this comment.
🔴 Stored XSS via unsanitized marked.parse() output injected with innerHTML
The new addMessage function renders assistant chat messages by passing content through marked.parse(content) and assigning the result to div.innerHTML (line 276) with no HTML sanitization. The marked library does not sanitize HTML — it passes through raw HTML embedded in markdown (e.g., <img onerror="alert(1)">, <script> tags). This contrasts with the server-side rendering path, which uses ammonia sanitization (crates/pika-news/src/web.rs:704-714). An attacker could exploit this via prompt injection: crafting a chat message that causes the Claude assistant to echo back HTML/JS in its response. That response is stored raw in the database (web.rs:631) and re-rendered via loadHistory() (line 310) on every page load, making this a stored XSS vulnerability.
Prompt for agents
In crates/pika-news/templates/detail.html, the addMessage function at line 275-277 uses marked.parse() + innerHTML without sanitization. Either:
1. Add DOMPurify (recommended): Add a script tag for DOMPurify in the head_extra block (line 9 area), e.g. <script src="https://cdn.jsdelivr.net/npm/dompurify@3.2.4/dist/purify.min.js"></script>, then change line 276 from:
div.innerHTML = marked.parse(content);
to:
div.innerHTML = DOMPurify.sanitize(marked.parse(content));
2. Or configure marked to not pass through HTML by setting marked.setOptions({ breaks: true }) and using a custom renderer that escapes HTML tokens, though DOMPurify is the more robust approach.
This must be done to match the server-side sanitization pattern already used in crates/pika-news/src/web.rs:704-714 (markdown_to_safe_html with ammonia).
Was this helpful? React with 👍 or 👎 to provide feedback.
| <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/highlight.js/11.11.1/styles/github.min.css" media="(prefers-color-scheme: light)" /> | ||
| <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/highlight.js/11.11.1/styles/github-dark.min.css" media="(prefers-color-scheme: dark)" /> | ||
| <link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/diff2html@3.4.51/bundles/css/diff2html.min.css" /> | ||
| <script src="https://cdn.jsdelivr.net/npm/marked/marked.min.js"></script> |
There was a problem hiding this comment.
🔴 Unpinned CDN version for marked library creates supply chain risk
The marked script tag loads from https://cdn.jsdelivr.net/npm/marked/marked.min.js without a version pin. This always resolves to the latest published version on npm. All other CDN dependencies on this page are pinned to specific versions (highlight.js 11.11.1, diff2html 3.4.51). An unpinned dependency means a compromised or breaking npm publish would immediately affect all users, and builds are not reproducible. The URL should include a version specifier and ideally an SRI integrity hash.
| <script src="https://cdn.jsdelivr.net/npm/marked/marked.min.js"></script> | |
| <script src="https://cdn.jsdelivr.net/npm/marked@15.0.7/marked.min.js"></script> |
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
pendingorgenerating, stops onceready/failedpre/codeblocks instead of inlinecode(with hljs syntax highlighting)overflow: hiddentooverflow-x: auto(was clipping diff content)Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Improvements