Potential fix for code scanning alert no. 123: DOM text reinterpreted as HTML#64
Potential fix for code scanning alert no. 123: DOM text reinterpreted as HTML#64Jackson57279 merged 1 commit intomainfrom
Conversation
… as HTML Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughIntroduces a URL sanitization helper and applies it in the Website Analysis UI to validate hrefs and safely render link text. Adds a new MIN_TITLE_LENGTH constant to input validation alongside existing limits. No changes to exported/public APIs. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Claude finished @otdoges's task —— View job Code Review Complete ✅I've thoroughly reviewed PR #64 which addresses security code scanning alert #123 about DOM text reinterpreted as HTML. Here's my comprehensive analysis: Security Assessment ✅ STRONG✅ Effective XSS ProtectionThe fix implements a robust multi-layer security approach:
Code Quality ✅ EXCELLENT✅ Best Practices Followed
✅ Clean Implementation
Potential Issues
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (6)
src/components/ChatInterface.tsx (6)
850-852: Sanitize 3rd‑party search content with SafeTextTitles and descriptions come from external search and must be treated as untrusted. Wrap them with
SafeText.-<h5 className="font-medium text-sm truncate">{result.title}</h5> -<p className="text-xs text-muted-foreground line-clamp-2 mt-1">{result.description}</p> +<h5 className="font-medium text-sm truncate"><SafeText>{result.title}</SafeText></h5> +<p className="text-xs text-muted-foreground line-clamp-2 mt-1"><SafeText>{result.description}</SafeText></p>
852-861: Sanitize search result links: use getSafeUrl for href and SafeText for link textThis is the same attack surface as the scanner alert. Avoid inserting raw URLs; sanitize the href and escape the visible text.
-<a - href={result.url} +<a + href={getSafeUrl(result.url)} target="_blank" rel="noopener noreferrer" className="text-xs text-primary hover:underline flex items-center gap-1 mt-1" onClick={(e) => e.stopPropagation()} > - {result.url} + <SafeText>{result.url}</SafeText> <ExternalLink className="w-3 h-3" /> </a>
927-931: Sanitize website analysis metadata (title/description) before renderingThese fields are also externally sourced. Wrap with
SafeTextto prevent XSS.-{websiteAnalysis.title && ( - <div><span className="font-medium">Title:</span> {websiteAnalysis.title}</div> -)} -{websiteAnalysis.description && ( - <div><span className="font-medium">Description:</span> {websiteAnalysis.description}</div> -)} +{websiteAnalysis.title && ( + <div><span className="font-medium">Title:</span> <SafeText>{websiteAnalysis.title}</SafeText></div> +)} +{websiteAnalysis.description && ( + <div><span className="font-medium">Description:</span> <SafeText>{websiteAnalysis.description}</SafeText></div> +)}
933-954: Sanitize other website analysis fields (technologies, layout, components)These arrays/strings may originate from untrusted content. Escape them with
SafeText.{websiteAnalysis.technologies && websiteAnalysis.technologies.length > 0 && ( <div> <span className="font-medium">Technologies:</span> <div className="flex flex-wrap gap-1 mt-1"> {websiteAnalysis.technologies.map((tech, index) => ( - <Badge key={index} variant="secondary" className="text-xs">{tech}</Badge> + <Badge key={index} variant="secondary" className="text-xs"><SafeText>{tech}</SafeText></Badge> ))} </div> </div> )} {websiteAnalysis.layout && ( - <div><span className="font-medium">Layout:</span> {websiteAnalysis.layout}</div> + <div><span className="font-medium">Layout:</span> <SafeText>{websiteAnalysis.layout}</SafeText></div> )} {websiteAnalysis.components && websiteAnalysis.components.length > 0 && ( <div> <span className="font-medium">Components:</span> <div className="flex flex-wrap gap-1 mt-1"> {websiteAnalysis.components.map((comp, index) => ( - <Badge key={index} variant="outline" className="text-xs">{comp}</Badge> + <Badge key={index} variant="outline" className="text-xs"><SafeText>{comp}</SafeText></Badge> ))} </div> </div> )}
1008-1013: Sanitize crawled pages links and textCrawl results are untrusted. Sanitize
hrefand escape the text to prevent DOM text being interpreted as HTML.-<a href={p.url} target="_blank" rel="noopener noreferrer" className="text-primary truncate"> - {p.title || p.url} -</a> -<a href={p.url} target="_blank" rel="noopener noreferrer" className="text-muted-foreground"> +<a href={getSafeUrl(p.url)} target="_blank" rel="noopener noreferrer" className="text-primary truncate"> + <SafeText>{p.title || p.url}</SafeText> +</a> +<a href={getSafeUrl(p.url)} target="_blank" rel="noopener noreferrer" className="text-muted-foreground"> <ExternalLink className="w-3 h-3" /> </a>
1-1: Sanitize unsanitized hrefs and correct SafeText usageThe audit uncovered several
<a href={…}>usages that aren’t wrapped ingetSafeUrland oneSafeTextinstance using thetextprop. Please update the following:• src/pages/Settings.tsx
– Line 569:<a href={inv.hosted_invoice_url}…>→href={getSafeUrl(inv.hosted_invoice_url)}
– Line 572:<a href={inv.invoice_pdf}…>→href={getSafeUrl(inv.invoice_pdf)}• src/components/ChatInterface.tsx
– Line 923:<SafeText text={websiteAnalysis.url} />→
<SafeText>{websiteAnalysis.url}</SafeText>
– Lines 1008 & 1011:<a href={p.url}…>→
href={getSafeUrl(p.url)}• src/components/E2BCodeExecution.tsx
– Line 377:<a href={artifact.url}…>→
href={getSafeUrl(artifact.url)}• src/components/AnimatedResultShowcase.tsx
– Line 129:<a href={a.url}…>→
href={getSafeUrl(a.url)}Existing usages of
href={getSafeUrl(…)}(e.g. ChatInterface.tsx 922) are fine.
🧹 Nitpick comments (2)
src/components/ChatInterface.tsx (2)
52-63: Harden URL sanitizer: trim input before parsing and early-return on emptyGood approach restricting to http/https. Minor hardening: trim whitespace/newlines before parsing and bail out on empty inputs to avoid edge cases.
Consider moving this helper to a shared util (e.g.,
@/lib/security/url.ts) since it’s useful in multiple places in this file.-function getSafeUrl(url: string): string { - try { - const urlObj = new URL(url); +function getSafeUrl(url: string): string { + const raw = typeof url === 'string' ? url.trim() : ''; + if (!raw) return '#'; + try { + const urlObj = new URL(raw); if (urlObj.protocol === 'http:' || urlObj.protocol === 'https:') { return urlObj.toString(); } } catch { // Invalid URL } return '#'; }
1311-1314: Preserve message formatting per chat UI patternAdd whitespace preservation to message content for better readability and consistency with patterns.
-<div className="text-sm leading-relaxed text-gray-100"> +<div className="text-sm leading-relaxed text-gray-100 whitespace-pre-wrap break-words"> <SafeText>{getMessageContent(message)}</SafeText> </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/components/ChatInterface.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/{pages,components}/**/*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/authentication-patterns.mdc)
Handle all authentication states in components by showing a loading spinner when loading, a sign-in prompt when unauthenticated, and protected content when authenticated.
Files:
src/components/ChatInterface.tsx
**/*ChatInterface*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/chat-ui-patterns.mdc)
Chat interfaces should follow the specified component structure: state management for selectedChatId, input, isTyping; useQuery for chats and messages; layout with ChatSidebar and ChatArea components.
Files:
src/components/ChatInterface.tsx
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/convex-security.mdc)
**/*.{ts,tsx}: All Convex queries and mutations MUST use proper authentication. Never accept user IDs from client parameters.
Always verify user owns the data before allowing access.
Use the authenticated user's identity.subject for user references.
Implement proper error messages that don't leak information.
Authentication verification in every function.
Authorization checks for data ownership.
Input validation and sanitization.
Error handling without information leakage.
**/*.{ts,tsx}: Use Sonner for toast notifications to provide consistent user feedback, including success, error, and loading states.
Always handle errors gracefully using try-catch blocks in asynchronous functions, providing user feedback and logging errors.
Provide specific, actionable error messages for form validation errors using toast notifications.
Handle common network error scenarios in catch blocks, providing appropriate toast messages for network errors, authentication errors, and unexpected errors.If using TypeScript, use an enum to store flag names.
Strict TypeScript must be used with no 'any' types allowed
**/*.{ts,tsx}: NEVER useanytype - use proper TypeScript types
Useunknownfor truly unknown data types
Implement proper interface definitions
Do not use empty interfaces; use a type alias instead (e.g.,type InputProps = ...instead ofinterface InputProps {})
All function parameters must be typed
All return types should be explicit for public APIs
Use proper generic constraints
Implement discriminated unions for state management
Use proper interface definitions for error handling types (e.g.,interface ValidationResult { isValid: boolean; error?: string; })
**/*.{ts,tsx}: Always sanitize user input before storing or displaying using a sanitization function likesanitizeText.
Implement comprehensive input validation, including length checks and detection of malicious patterns, as shown in thevalidateInputfunction.
Define and use security constants suc...
Files:
src/components/ChatInterface.tsx
**/*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/error-handling.mdc)
**/*.tsx: Always provide loading feedback to users during asynchronous operations.
Use proper error boundaries to handle component crashes and display user-friendly error UI.Use proper React component typing (e.g.,
const MyComponent: React.FC<Props> = ...)
**/*.tsx: Use theSafeTextReact component for all user-generated content to ensure safe text display.
NEVER usedangerouslySetInnerHTMLwith user content.
NEVER use direct string interpolation in HTML with user content.
Files:
src/components/ChatInterface.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/posthog-integration.mdc)
**/*.{js,jsx,ts,tsx}: Use a consistent naming convention for this storage. enum/const object members should be written UPPERCASE_WITH_UNDERSCORE.
If a custom property for a person or event is at any point referenced in two or more files or two or more callsites in the same file, use an enum or const object, as above in feature flags.
Files:
src/components/ChatInterface.tsx
src/components/**/*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/project-overview.mdc)
Input sanitization and validation must be implemented throughout the codebase to provide XSS protection
Files:
src/components/ChatInterface.tsx
src/**/*.tsx
📄 CodeRabbit Inference Engine (CLAUDE.md)
src/**/*.tsx: Use React Hook Form with Zod validation for client-side forms
Prevent XSS by sanitizing any user-generated content before rendering (avoid unsafe HTML, or sanitize it)
Implement proper error handling with typed error boundaries in React
Files:
src/components/ChatInterface.tsx
🧠 Learnings (8)
📚 Learning: 2025-08-09T23:03:33.406Z
Learnt from: CR
PR: otdoges/zapdev#0
File: .cursor/rules/project-overview.mdc:0-0
Timestamp: 2025-08-09T23:03:33.406Z
Learning: Applies to src/components/**/*.tsx : Input sanitization and validation must be implemented throughout the codebase to provide XSS protection
Applied to files:
src/components/ChatInterface.tsx
📚 Learning: 2025-08-09T23:03:47.108Z
Learnt from: CR
PR: otdoges/zapdev#0
File: .cursor/rules/xss-protection.mdc:0-0
Timestamp: 2025-08-09T23:03:47.108Z
Learning: Applies to **/*.tsx : Use the `SafeText` React component for all user-generated content to ensure safe text display.
Applied to files:
src/components/ChatInterface.tsx
📚 Learning: 2025-08-12T03:04:41.034Z
Learnt from: CR
PR: otdoges/zapdev#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-12T03:04:41.034Z
Learning: Applies to src/**/*.tsx : Prevent XSS by sanitizing any user-generated content before rendering (avoid unsafe HTML, or sanitize it)
Applied to files:
src/components/ChatInterface.tsx
📚 Learning: 2025-08-09T23:03:01.775Z
Learnt from: CR
PR: otdoges/zapdev#0
File: .cursor/rules/chat-ui-patterns.mdc:0-0
Timestamp: 2025-08-09T23:03:01.775Z
Learning: Applies to **/*.{tsx} : Render messages safely using a SafeText component inside a div with 'whitespace-pre-wrap' to preserve formatting.
Applied to files:
src/components/ChatInterface.tsx
📚 Learning: 2025-08-09T23:03:01.775Z
Learnt from: CR
PR: otdoges/zapdev#0
File: .cursor/rules/chat-ui-patterns.mdc:0-0
Timestamp: 2025-08-09T23:03:01.775Z
Learning: Applies to **/*.{tsx} : Ensure proper keyboard navigation throughout the chat interface for accessibility.
Applied to files:
src/components/ChatInterface.tsx
📚 Learning: 2025-08-09T23:03:01.775Z
Learnt from: CR
PR: otdoges/zapdev#0
File: .cursor/rules/chat-ui-patterns.mdc:0-0
Timestamp: 2025-08-09T23:03:01.775Z
Learning: Applies to **/*ChatInterface*.tsx : Chat interfaces should follow the specified component structure: state management for selectedChatId, input, isTyping; useQuery for chats and messages; layout with ChatSidebar and ChatArea components.
Applied to files:
src/components/ChatInterface.tsx
📚 Learning: 2025-08-12T03:04:41.034Z
Learnt from: CR
PR: otdoges/zapdev#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-12T03:04:41.034Z
Learning: Applies to src/App.tsx : Maintain routing protection: protect /chat and /settings with AuthGuard; keep public routes unprotected as specified
Applied to files:
src/components/ChatInterface.tsx
📚 Learning: 2025-08-09T23:03:47.108Z
Learnt from: CR
PR: otdoges/zapdev#0
File: .cursor/rules/xss-protection.mdc:0-0
Timestamp: 2025-08-09T23:03:47.108Z
Learning: Applies to **/*.{ts,tsx} : Define and use security constants such as `MAX_MESSAGE_LENGTH`, `MAX_TITLE_LENGTH`, and `MAX_CODE_BLOCK_LENGTH` to enforce input size limits.
Applied to files:
src/components/ChatInterface.tsx
🧬 Code Graph Analysis (1)
src/components/ChatInterface.tsx (1)
src/components/ui/SafeText.tsx (1)
SafeText(13-37)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: claude-review
1 participant
Potential fix for https://github.com/otdoges/zapdev/security/code-scanning/123
To fix the problem, we need to ensure that any user-supplied URL is both validated and sanitized before being used as an
hrefattribute and as link text. The best way is to:httporhttpsprotocols, and ensure the string does not contain any dangerous characters or sequences.SafeText(already imported) to render the URL.hrefattribute, we should use a helper function to validate and sanitize the URL before passing it to the<a>tag. If the URL is invalid or unsafe, we can render a fallback (e.g.,#or nothing).Changes required:
src/components/ChatInterface.tsx, add a helper function (e.g.,getSafeUrl) to validate and sanitize the URL before rendering.SafeTextto render the link text.hrefattribute.Suggested fixes powered by Copilot Autofix. Review carefully before merging.
Summary by CodeRabbit
Bug Fixes
Chores