Rich Reader Implementation - #17
Conversation
Implement dedicated SVG sanitization with scoped IDs, local reference validation, size limits, and safe element/attribute allowlists. Add Reader styling and coverage for accessible, malformed, and oversized SVGs.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
readr | 37bc6b8 | Sep 19 2026, 12:15 PM |
There was a problem hiding this comment.
🔵 Needs a closer look
Two moderate SVG safety and layout findings remain unresolved.
Pull request overview
Rich Reader Implementation adds article capability metadata, safe rich-content sanitization, and fixture-driven coverage across extraction, storage, and rendering.
Changes:
- Adds capability detection, persistence, migration, and legacy compatibility.
- Adds inline SVG sanitization and figure/SVG Reader styling.
- Adds rich-content fixtures plus worker, browser, and E2E tests.
File summaries
| File | Summary |
|---|---|
worker/extract.ts |
Detects article capabilities. |
worker/articleContent.ts |
Persists and validates capabilities. |
tests/worker/extract.test.ts |
Tests capability detection. |
tests/worker/capture.test.ts |
Tests persistence and legacy rows. |
tests/worker/articleFixtures.test.ts |
Tests fixture extraction. |
tests/fixtures/articles/video.html |
Video article fixture. |
tests/fixtures/articles/table.html |
Table article fixture. |
tests/fixtures/articles/svg.html |
SVG article fixture. |
tests/fixtures/articles/README.md |
Documents fixture scope. |
tests/fixtures/articles/plain.html |
Plain article fixture. |
tests/fixtures/articles/news.html |
News article fixture. |
tests/fixtures/articles/math.html |
Math article fixture. |
tests/fixtures/articles/hacktron.html |
Rich-content edge-case fixture. |
tests/fixtures/articles/figure.html |
Figure article fixture. |
tests/fixtures/articles/code.html |
Code article fixture. |
tests/fixtures/articles/animated-image.html |
Animated-image article fixture. |
tests/fixtures/articleFixtures.ts |
Defines shared fixtures. |
tests/e2e/reader.spec.ts |
Verifies SVG rendering under CSP. |
tests/browser/sanitizeArticle.test.ts |
Tests SVG and figure sanitization. |
tests/browser/fetchArticleContent.test.ts |
Tests response normalization. |
tests/browser/articleFixtureAudit.test.ts |
Audits sanitized fixture output. |
tests/browser/articleCache.test.ts |
Updates article cache fixtures. |
src/reader/sanitizeSvg.ts |
Sanitizes inline SVG. Moderate finding (1 vote): obfuscated CSS URL functions can retain remote paint references. |
src/reader/sanitizeArticle.ts |
Integrates SVG and figure sanitization. |
src/reader/reader.css |
Styles figures, captions, and SVGs. Moderate finding (1 vote): unbounded SVG aspect ratios can create extremely tall rendered elements. |
src/reader/fetchArticleContent.ts |
Supports legacy response normalization. |
shared/extraction.ts |
Defines and normalizes capability contracts. |
README.md |
Updates migration guidance. |
migrations/0010_article_capabilities.sql |
Adds capability storage. |
docs/rich-reader-implementation-plan.md |
Documents implementation phases. |
docs/rich-reader-fixture-audit.md |
Records fixture findings and decisions. |
Review details
Suppressed comments (2)
src/reader/reader.css:252
- The SVG byte/element limits do not bound its intrinsic aspect ratio, while
width: 100%plusheight: autolets untrustedviewBox/dimension values produce an extremely tall replaced element and a layout/rendering denial of service. Validate or cap the intrinsic dimensions/aspect ratio in the SVG policy, or impose a bounded rendered height before enabling this rule.
width: 100%;
max-width: 100%;
height: auto;
margin: 1.8em auto;
src/reader/sanitizeSvg.ts:206
- The external-reference guard only recognizes a literal
url(spelling. CSS URL functions can be obfuscated with comments/other tokenization, so a value such asfill="url/**/(https://evil.example/fill)"can bypass this branch and remain as a remote paint reference. Detect and reject all URL-function spellings after CSS-token normalization, or restrict these attributes to exact colors and validated localurl(#id)references.
if (LOCAL_REFERENCE_ATTRIBUTES.has(name) && /url\(/i.test(attribute.value)) {
const reference = LOCAL_REFERENCE.exec(attribute.value);
if (reference === null || !identifiers.has(reference[1])) {
return false;
}
- Files reviewed: 31/31 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
No description provided.