-
Notifications
You must be signed in to change notification settings - Fork 0
Feature and dev #8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
…re test artifacts
|
@coderabbitai review |
WalkthroughAdds gitignore entries to prevent test files from being committed, removes content from abc.txt, updates page title formatting in index.html, fixes a prop name in the App component, adds comprehensive test coverage for the App component, and configures Vite server port to 9091. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12–18 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
.gitignore(1 hunks)abc.txt(0 hunks)index.html(1 hunks)src/App.tsx(1 hunks)src/__tests__/App.test.tsx(1 hunks)vite.config.ts(1 hunks)
💤 Files with no reviewable changes (1)
- abc.txt
🧰 Additional context used
🧬 Code graph analysis (2)
src/__tests__/App.test.tsx (1)
src/hooks/useChat.ts (1)
useChat(5-65)
src/App.tsx (5)
src/hooks/useChat.ts (1)
useChat(5-65)src/components/ModelSelector.tsx (1)
ModelSelector(11-73)src/data/models.ts (1)
availableModels(3-39)src/components/ChatArea.tsx (1)
ChatArea(10-55)src/components/ChatInput.tsx (1)
ChatInput(10-78)
🪛 ast-grep (0.40.0)
src/App.tsx
[warning] 66-66: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
src/App.tsx
[error] 67-67: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🔇 Additional comments (6)
vite.config.ts (1)
10-12: LGTM!The server port configuration is correctly implemented. Port 9091 is a valid choice for local development.
.gitignore (1)
26-29: LGTM!Good practice to document the purpose of ignored files. The entries prevent accidental commits of test/junk files.
index.html (1)
7-7: LGTM!The title formatting change improves readability by removing the hyphen.
src/App.tsx (2)
10-21: LGTM!The state management and
lastUserMessagecomputation are correctly implemented. The backward iteration efficiently finds the most recent user message, and memoization prevents unnecessary recalculations.
23-94: Component structure looks good.The layout with header, model selector, sidebar, chat area, and input is well-organized. Props are correctly passed to child components, and the conditional rendering of the clear chat button is appropriate.
src/__tests__/App.test.tsx (1)
1-330: Excellent test coverage!The test suite is comprehensive and well-structured, covering all major functionality including rendering, model selection, message handling, clear chat, last user message display, loading states, and integration flows. The mocking strategy is appropriate and tests are clearly organized.
| it('should handle HTML content in user messages', () => { | ||
| (useChat as any).mockReturnValue({ | ||
| messages: [ | ||
| { role: 'user', content: '<strong>Bold question</strong>' }, | ||
| ], | ||
| isLoading: false, | ||
| sendMessage: mockSendMessage, | ||
| clearChat: mockClearChat, | ||
| }); | ||
|
|
||
| render(<App />); | ||
|
|
||
| const sidebar = screen.getByText('Last Question').parentElement; | ||
| expect(sidebar?.querySelector('strong')).toBeInTheDocument(); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update test after fixing XSS vulnerability.
This test currently validates the unsafe HTML rendering behavior that was flagged as a critical XSS vulnerability in src/App.tsx (line 67). After applying the security fix:
- If using plain text rendering: Update the test to verify that HTML tags are escaped/displayed as text rather than rendered as DOM elements.
- If using DOMPurify sanitization: Update the test to verify that only safe HTML is rendered and malicious scripts are stripped.
Example update for plain text rendering:
- it('should handle HTML content in user messages', () => {
+ it('should escape HTML content in user messages', () => {
(useChat as any).mockReturnValue({
messages: [
{ role: 'user', content: '<strong>Bold question</strong>' },
],
isLoading: false,
sendMessage: mockSendMessage,
clearChat: mockClearChat,
});
render(<App />);
const sidebar = screen.getByText('Last Question').parentElement;
- expect(sidebar?.querySelector('strong')).toBeInTheDocument();
+ expect(sidebar).toHaveTextContent('<strong>Bold question</strong>');
+ expect(sidebar?.querySelector('strong')).not.toBeInTheDocument();
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('should handle HTML content in user messages', () => { | |
| (useChat as any).mockReturnValue({ | |
| messages: [ | |
| { role: 'user', content: '<strong>Bold question</strong>' }, | |
| ], | |
| isLoading: false, | |
| sendMessage: mockSendMessage, | |
| clearChat: mockClearChat, | |
| }); | |
| render(<App />); | |
| const sidebar = screen.getByText('Last Question').parentElement; | |
| expect(sidebar?.querySelector('strong')).toBeInTheDocument(); | |
| }); | |
| it('should escape HTML content in user messages', () => { | |
| (useChat as any).mockReturnValue({ | |
| messages: [ | |
| { role: 'user', content: '<strong>Bold question</strong>' }, | |
| ], | |
| isLoading: false, | |
| sendMessage: mockSendMessage, | |
| clearChat: mockClearChat, | |
| }); | |
| render(<App />); | |
| const sidebar = screen.getByText('Last Question').parentElement; | |
| expect(sidebar).toHaveTextContent('<strong>Bold question</strong>'); | |
| expect(sidebar?.querySelector('strong')).not.toBeInTheDocument(); | |
| }); |
| {lastUserMessage ? ( | ||
| <div | ||
| className="text-gray-100 text-base break-words" | ||
| dangerouslySetInnerHTML={{ __html: lastUserMessage.content }} | ||
| /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical XSS vulnerability: Unsanitized user input rendered as HTML.
Using dangerouslySetInnerHTML with user-provided content (lastUserMessage.content) creates a cross-site scripting (XSS) vulnerability. Malicious users can inject arbitrary JavaScript by submitting messages like <script>alert('XSS')</script> or <img src=x onerror="steal_cookies()">.
Recommended fix: Render as plain text
Apply this diff to safely display user content:
- <div
- className="text-gray-100 text-base break-words"
- dangerouslySetInnerHTML={{ __html: lastUserMessage.content }}
- />
+ <div className="text-gray-100 text-base break-words">
+ {lastUserMessage.content}
+ </div>Alternative fix: Sanitize HTML with DOMPurify (only if HTML rendering is required)
If you need to support formatted content, install and use DOMPurify:
npm install dompurify
npm install --save-dev @types/dompurifyThen sanitize before rendering:
+import DOMPurify from 'dompurify';
+
function App() {
// ... existing code ...
return (
// ... existing JSX ...
<div
className="text-gray-100 text-base break-words"
- dangerouslySetInnerHTML={{ __html: lastUserMessage.content }}
+ dangerouslySetInnerHTML={{ __html: DOMPurify.sanitize(lastUserMessage.content) }}
/>
);
}🧰 Tools
🪛 ast-grep (0.40.0)
[warning] 66-66: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
[error] 67-67: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🤖 Prompt for AI Agents
In src/App.tsx around lines 64-68, the code uses dangerouslySetInnerHTML with
lastUserMessage.content which creates a critical XSS risk; fix it by rendering
the message as plain text (remove dangerouslySetInnerHTML and render the content
as a child or use React's text escaping) and if HTML formatting is required,
sanitize lastUserMessage.content with DOMPurify before passing it to
dangerouslySetInnerHTML (install dompurify and @types/dompurify, import
DOMPurify, call DOMPurify.sanitize(content) and use the sanitized result).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@coderabbitai
Generate a new issue for this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Create a GitHub issue for the XSS vulnerability
gh issue create \
--repo manishcy/chatAI \
--title "Critical XSS vulnerability in src/App.tsx using dangerouslySetInnerHTML" \
--body "## Description
A critical XSS (Cross-Site Scripting) vulnerability has been identified in \`src/App.tsx\` where unsanitized user input is rendered using \`dangerouslySetInnerHTML\`.
## Location
- File: \`src/App.tsx\`
- Lines: 64-68 (approximately)
- Component: App component, Last Question sidebar section
## Issue
The code uses \`dangerouslySetInnerHTML\` with \`lastUserMessage.content\` which comes from user input. This creates a security vulnerability where malicious users can inject arbitrary JavaScript.
## References
- Pull Request: https://github.com/manishcy/chatAI/pull/8
- Review Comment: https://github.com/manishcy/chatAI/pull/8#discussion_r2579965345
- Requested by: @manishcy
## Priority
🔴 Critical - This is a security vulnerability that should be addressed as soon as possible." \
--assignee manishcy
Length of output: 43
@manishcy I've successfully created a new issue for the XSS vulnerability!
Issue created: #9 - Critical XSS vulnerability in src/App.tsx using dangerouslySetInnerHTML
The issue includes:
- Description of the security vulnerability
- File location and affected lines
- References to this PR and review comment
- Priority marked as 🔴 Critical
- Assigned to you
You can track and address this security issue in a separate PR.
🧬 Code Graph Analysis Results
- File: src/data/models.ts (lines 3-39)
export const availableModels: AIModel[] = [
{
id: 'gpt-4-turbo',
name: 'GPT-4 Turbo',
description: 'Most capable model for complex reasoning and analysis',
provider: 'OpenAI',
capabilities: ['Text Generation', 'Code Analysis', 'Mathematical Reasoning', 'Creative Writing']
},
{
id: 'gpt-3.5-turbo',
name: 'GPT-3.5 Turbo',
description: 'Fast and efficient for most conversational tasks',
provider: 'OpenAI',
capabilities: ['Text Generation', 'Basic Coding', 'Q&A', 'Summarization']
},
{
id: 'claude-3-opus',
name: 'Claude 3 Opus',
description: 'Excellent for analysis, math, and creative tasks',
provider: 'Anthropic',
capabilities: ['Advanced Analysis', 'Creative Writing', 'Code Review', 'Research']
},
{
id: 'claude-3-sonnet',
name: 'Claude 3 Sonnet',
description: 'Balanced performance for everyday tasks',
provider: 'Anthropic',
capabilities: ['Text Generation', 'Code Help', 'Analysis', 'Writing']
},
{
id: 'gemini-pro',
name: 'Gemini Pro',
description: 'Google\'s advanced multimodal AI model',
provider: 'Google',
capabilities: ['Text Generation', 'Multimodal', 'Code Generation', 'Reasoning']
}
];
- File: src/hooks/useChat.ts (lines 5-65)
export const useChat = (selectedModel: string) => {
const [messages, setMessages] = useState<Message[]>([]);
const [isLoading, setIsLoading] = useState(false);
const sendMessage = useCallback(async (content: string) => {
if (!selectedModel) {
alert('Please select a model first');
return;
}
const userMessage: Message = {
id: Date.now().toString(),
content,
role: 'user',
timestamp: new Date(),
};
setMessages(prev => [...prev, userMessage]);
setIsLoading(true);
try {
const response = await AIService.generateResponse(
[...messages, userMessage],
selectedModel
);
const assistantMessage: Message = {
id: (Date.now() + 1).toString(),
content: response,
role: 'assistant',
timestamp: new Date(),
model: selectedModel,
};
setMessages(prev => [...prev, assistantMessage]);
} catch (error) {
console.error('Error generating response:', error);
const errorMessage: Message = {
id: (Date.now() + 1).toString(),
content: 'Sorry, I encountered an error while generating a response. Please try again.',
role: 'assistant',
timestamp: new Date(),
model: selectedModel,
};
setMessages(prev => [...prev, errorMessage]);
} finally {
setIsLoading(false);
}
}, [messages, selectedModel]);
const clearChat = useCallback(() => {
setMessages([]);
}, []);
return {
messages,
isLoading,
sendMessage,
clearChat,
};
};
- File: src/components/ChatArea.tsx (lines 10-55)
export const ChatArea: React.FC<ChatAreaProps> = ({ messages, isLoading }) => {
const messagesEndRef = React.useRef<HTMLDivElement>(null);
React.useEffect(() => {
messagesEndRef.current?.scrollIntoView({ behavior: 'smooth' });
}, [messages]);
if (messages.length === 0) {
return (
<div className="flex-1 flex items-center justify-center">
<div className="text-center text-gray-400 max-w-md">
<div className="text-6xl mb-4">🤖</div>
<h3 className="text-xl font-semibold mb-2">Welcome to AI Assistant</h3>
<p className="text-sm">
Select a model above and start a conversation. Ask me anything!
</p>
</div>
</div>
);
}
return (
<div className="flex-1 overflow-y-auto px-4 py-6 space-y-6">
{messages.map((message) => (
<MessageBubble key={message.id} message={message} />
))}
{isLoading && (
<div className="flex gap-3">
<div className="w-8 h-8 rounded-full bg-gray-600 flex items-center justify-center">
<div className="w-2 h-2 bg-white rounded-full animate-pulse" />
</div>
<div className="bg-white/90 backdrop-blur-sm border border-gray-200 rounded-2xl rounded-tl-md px-4 py-3">
<div className="flex gap-1">
<div className="w-2 h-2 bg-gray-400 rounded-full animate-bounce" />
<div className="w-2 h-2 bg-gray-400 rounded-full animate-bounce" style={{ animationDelay: '0.1s' }} />
<div className="w-2 h-2 bg-gray-400 rounded-full animate-bounce" style={{ animationDelay: '0.2s' }} />
</div>
</div>
</div>
)}
<div ref={messagesEndRef} />
</div>
);
};
- File: src/components/ChatInput.tsx (lines 10-78)
export const ChatInput: React.FC<ChatInputProps> = ({
onSendMessage,
disabled = false,
placeholder = "Type your message..."
}) => {
const [message, setMessage] = React.useState('');
const textareaRef = React.useRef<HTMLTextAreaElement>(null);
const handleSubmit = (e: React.FormEvent) => {
e.preventDefault();
if (message.trim() && !disabled) {
onSendMessage(message.trim());
setMessage('');
if (textareaRef.current) {
textareaRef.current.style.height = 'auto';
}
}
};
const handleKeyDown = (e: React.KeyboardEvent) => {
if (e.key === 'Enter' && !e.shiftKey) {
e.preventDefault();
handleSubmit(e);
}
};
const adjustHeight = () => {
const textarea = textareaRef.current;
if (textarea) {
textarea.style.height = 'auto';
textarea.style.height = Math.min(textarea.scrollHeight, 120) + 'px';
}
};
React.useEffect(() => {
adjustHeight();
}, [message]);
return (
<form onSubmit={handleSubmit} className="flex gap-3 items-end">
<div className="flex-1 relative">
<textarea
ref={textareaRef}
value={message}
onChange={(e) => setMessage(e.target.value)}
onKeyDown={handleKeyDown}
placeholder={placeholder}
disabled={disabled}
className="w-full px-4 py-3 pr-12 bg-white/90 backdrop-blur-sm border border-gray-200 rounded-xl resize-none focus:outline-none focus:ring-2 focus:ring-blue-500 focus:border-transparent transition-all duration-200 disabled:opacity-50"
rows={1}
style={{ minHeight: '52px', maxHeight: '120px' }}
/>
</div>
<button
type="submit"
disabled={!message.trim() || disabled}
className="flex-shrink-0 w-12 h-12 bg-blue-500 hover:bg-blue-600 disabled:bg-gray-400 text-white rounded-xl flex items-center justify-center transition-colors duration-200 disabled:cursor-not-allowed"
aria-label="Send message"
>
{disabled ? (
<Loader2 className="w-5 h-5 animate-spin" />
) : (
<Send className="w-5 h-5" />
)}
</button>
</form>
);
};
- File: src/components/ModelSelector.tsx (lines 11-73)
export const ModelSelector: React.FC<ModelSelectorProps> = ({
models,
selectedModel,
onModelChange,
}) => {
const [isOpen, setIsOpen] = React.useState(false);
const selectedModelData = models.find(m => m.id === selectedModel);
return (
<div className="relative">
<button
onClick={() => setIsOpen(!isOpen)}
className="flex items-center gap-3 w-full px-4 py-3 bg-white/10 backdrop-blur-sm border border-white/20 rounded-xl hover:bg-white/15 transition-all duration-200 group"
aria-label="Select AI model"
>
<Bot className="w-5 h-5 text-blue-400" />
<div className="flex-1 text-left">
<div className="font-medium text-white">
{selectedModelData?.name || 'Select Model'}
</div>
<div className="text-sm text-gray-300">
{selectedModelData?.provider}
</div>
</div>
<ChevronDown
className={`w-4 h-4 text-gray-400 transition-transform duration-200 ${
isOpen ? 'rotate-180' : ''
}`}
/>
</button>
{isOpen && (
<div className="absolute top-full left-0 right-0 mt-2 bg-white/95 backdrop-blur-lg border border-gray-200 rounded-xl shadow-2xl overflow-hidden z-50">
{models.map((model) => (
<button
key={model.id}
onClick={() => {
onModelChange(model.id);
setIsOpen(false);
}}
className={`w-full px-4 py-3 text-left hover:bg-blue-50 transition-colors duration-150 border-b border-gray-100 last:border-b-0 ${
selectedModel === model.id ? 'bg-blue-50 border-l-4 border-l-blue-500' : ''
}`}
>
<div className="font-medium text-gray-900">{model.name}</div>
<div className="text-sm text-gray-600 mb-1">{model.description}</div>
<div className="flex gap-1 flex-wrap">
{model.capabilities.slice(0, 3).map((cap) => (
<span
key={cap}
className="text-xs px-2 py-1 bg-gray-200 text-gray-700 rounded-full"
>
{cap}
</span>
))}
</div>
</button>
))}
</div>
)}
</div>
);
};
Review the changes.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.