Claude/fix article link navigation#7
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request enhances security by implementing XSS protection through DOMPurify sanitization and URL validation. The changes focus on preventing malicious script injection and ensuring only valid HTTP/HTTPS URLs are processed.
- Added DOMPurify library (version 3.0.8) for XSS protection across all HTML files
- Implemented URL scheme validation to prevent javascript: and other malicious protocols
- Applied sanitization to user-generated content including article titles, sources, tags, and chart labels
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| stats.html | Added DOMPurify CDN script and sanitization for chart labels and article data |
| reader.html | Added DOMPurify CDN script, URL validation function, and sanitization for article metadata |
| portfolio.html | Added DOMPurify CDN script, replaced escapeHtml with DOMPurify, added URL validation for repository links |
| README.md | Removed log messages from aggregation process |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| </script> | ||
|
|
||
| <!-- DOMPurify for XSS protection --> | ||
| <script src="https://cdn.jsdelivr.net/npm/dompurify@3.0.8/dist/purify.min.js"></script> |
There was a problem hiding this comment.
There is a version mismatch between the DOMPurify CDN version (3.0.8) being loaded and the package.json dependency version (3.3.1). This inconsistency could lead to unexpected behavior or missing security patches. Consider updating the CDN URL to use version 3.3.1 or use a caret/tilde version specifier to automatically get the latest patch version.
| <script src="https://cdn.jsdelivr.net/npm/dompurify@3.0.8/dist/purify.min.js"></script> | |
| <script src="https://cdn.jsdelivr.net/npm/dompurify@3.3.1/dist/purify.min.js"></script> |
| // Validate URL scheme to prevent XSS | ||
| function isValidHttpUrl(string) { | ||
| if (!string) return false; | ||
| try { | ||
| const url = new URL(string); | ||
| return url.protocol === 'http:' || url.protocol === 'https:'; | ||
| } catch (_) { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| // If article parameters exist and URL is valid, show article view | ||
| if (articleUrl && isValidHttpUrl(articleUrl)) { |
There was a problem hiding this comment.
The URL validation logic only checks for http: and https: protocols, but does not protect against other XSS vectors. While the validation prevents javascript: URLs from being used, the validated articleUrl is still used directly in href and iframe.src assignments (lines 488-489, 496) without further sanitization. The validation should be followed by using the validated URL object's href property to ensure the URL is properly normalized.
| try { | ||
| const urlObj = new URL(repo.html_url); | ||
| if (urlObj.protocol === 'http:' || urlObj.protocol === 'https:') { | ||
| url = DOMPurify.sanitize(repo.html_url); |
There was a problem hiding this comment.
Sanitizing URLs with DOMPurify.sanitize is ineffective for URL validation. DOMPurify is designed to sanitize HTML content, not validate URL schemes. The URL has already been validated by the URL constructor check above. Instead, use the validated URL object's href property directly, or simply use repo.html_url after successful validation without DOMPurify sanitization.
| url = DOMPurify.sanitize(repo.html_url); | |
| url = urlObj.href; |
| </script> | ||
|
|
||
| <!-- DOMPurify for XSS protection --> | ||
| <script src="https://cdn.jsdelivr.net/npm/dompurify@3.0.8/dist/purify.min.js"></script> |
There was a problem hiding this comment.
There is a version mismatch between the DOMPurify CDN version (3.0.8) being loaded and the package.json dependency version (3.3.1). This inconsistency could lead to unexpected behavior or missing security patches. Consider updating the CDN URL to use version 3.3.1 or use a caret/tilde version specifier to automatically get the latest patch version.
| <script src="https://cdn.jsdelivr.net/npm/dompurify@3.0.8/dist/purify.min.js"></script> | |
| <script src="https://cdn.jsdelivr.net/npm/dompurify@3.3.1/dist/purify.min.js"></script> |
| </script> | ||
|
|
||
| <!-- DOMPurify for XSS protection --> | ||
| <script src="https://cdn.jsdelivr.net/npm/dompurify@3.0.8/dist/purify.min.js"></script> |
There was a problem hiding this comment.
There is a version mismatch between the DOMPurify CDN version (3.0.8) being loaded and the package.json dependency version (3.3.1). This inconsistency could lead to unexpected behavior or missing security patches. Consider updating the CDN URL to use version 3.3.1 or use a caret/tilde version specifier to automatically get the latest patch version.
| <script src="https://cdn.jsdelivr.net/npm/dompurify@3.0.8/dist/purify.min.js"></script> | |
| <script src="https://cdn.jsdelivr.net/npm/dompurify@3.3.1/dist/purify.min.js"></script> |
Improve input validation
@claude
Clean logs
@claude
Sync with main
@claude
Improve URL handling
@claude
claude committed 5 minutes ago