Preview/React Doctor#106
Conversation
WalkthroughA new React 18 TypeScript project named ChangesReact TypeScript App Scaffold
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
src/App.tsx (1)
6-8: ⚡ Quick winHoist
ThemeContextto module scope to preserve context identity.Creating context inside
Appmakes a new context object on every render; future consumers will detach/reset across renders.Suggested fix
import { createContext, useState } from "react"; import { AssertiveHints } from "./AssertiveHints.js"; +const ThemeContext = createContext("light"); const items = ["alpha", "beta", "gamma"]; export function App() { - const ThemeContext = createContext("light"); const [userHtml] = useState("<img src=x onerror=alert(1)>");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/App.tsx` around lines 6 - 8, The ThemeContext is being created inside the App function, which causes a new context object to be instantiated on every render, breaking context consumer connections. Move the createContext("light") call outside the App function to module scope (above the function definition) so that ThemeContext maintains the same identity across all renders and consumers remain properly connected.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/App.tsx`:
- Line 16: The iframe element on line 16 in App.tsx lacks security restrictions
that could allow privilege escalation and referrer leakage. Add a sandbox
attribute with a minimal restrictive policy (such as allowing only necessary
permissions) and include referrerPolicy="no-referrer" attribute to the iframe
element that has src="https://example.com" and title="embed". This will restrict
the embedded frame's capabilities and prevent sensitive referrer information
from being leaked to the embedded content.
- Around line 24-25: Remove both lines that execute arbitrary code: the
eval(userInput) statement and the new Function(userInput)() statement. These
lines are security vulnerabilities that allow code injection attacks. Simply
delete both lines entirely, as they serve no legitimate purpose in the
application and directly execute untrusted user input.
- Line 15: The anchor element in App.tsx contains a javascript: URL which is a
security vulnerability and should not be used in JSX. Replace the
href="javascript:alert(1)" with a proper onClick event handler instead. Remove
the javascript: protocol from the href attribute and add an onClick handler
function that executes the desired action. Consider setting href to "#" or
removing it entirely if using an onClick handler.
- Around line 18-20: The map function iterating over items to create list item
elements is missing the required key prop on the <li> component. Add a key prop
to the <li> element inside the items.map() function. Use the item value itself
as the key since these appear to be unique string items, assigning it as
key={item} to help React properly identify and track each list item during
re-renders.
- Line 14: The div element using dangerouslySetInnerHTML with userHtml is
vulnerable to XSS attacks because it renders unescaped user input directly as
HTML. Replace the dangerouslySetInnerHTML prop with standard JSX interpolation
by removing the dangerouslySetInnerHTML={{ __html: userHtml }} attribute and
instead directly embedding the userHtml variable in the JSX using curly braces
{userHtml}. This will automatically escape HTML entities and prevent malicious
scripts from being executed.
In `@src/AssertiveHints.tsx`:
- Line 16: The img element with src="/logo.png" is missing an alt attribute,
which is required for accessibility to describe the image to screen readers. Add
an alt attribute to the img tag with descriptive text that clearly identifies
what the logo represents, such as "Assertive Hints logo" or an appropriate
descriptive label.
- Around line 6-8: The Row component accepts an onSelect callback prop but the
returned span element does not invoke it, making the callback dead code. Wire
the onSelect handler to the span element by adding an onClick event handler that
calls onSelect when the span is clicked.
- Line 17: The div element with the onClick handler that displays "Open menu" is
not keyboard-accessible. Replace this div element with an interactive element
such as a button, following the same pattern already implemented on line 15 in
the component. Ensure the onClick handler (handleAction) is transferred to the
new button element to maintain the functionality while making it accessible to
keyboard users.
---
Nitpick comments:
In `@src/App.tsx`:
- Around line 6-8: The ThemeContext is being created inside the App function,
which causes a new context object to be instantiated on every render, breaking
context consumer connections. Move the createContext("light") call outside the
App function to module scope (above the function definition) so that
ThemeContext maintains the same identity across all renders and consumers remain
properly connected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 120e1900-a8d7-4527-bdd7-4a3d36897a58
📒 Files selected for processing (5)
package.jsonsrc/App.tsxsrc/AssertiveHints.tsxsrc/main.tsxtsconfig.json
| return ( | ||
| <ThemeContext.Provider value="dark"> | ||
| <main> | ||
| <div dangerouslySetInnerHTML={{ __html: userHtml }} /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /tmp && find . -name "App.tsx" -type f 2>/dev/null | head -5Repository: coderabbitai/coderabbit-pr-review
Length of output: 59
🏁 Script executed:
cd /tmp && git ls-files | grep -i "app.tsx"Repository: coderabbitai/coderabbit-pr-review
Length of output: 146
🏁 Script executed:
cd /tmp && ls -la src/ 2>/dev/null || echo "src dir not found"Repository: coderabbitai/coderabbit-pr-review
Length of output: 95
🏁 Script executed:
cd /tmp && cat -n src/App.tsx 2>/dev/null | head -30Repository: coderabbitai/coderabbit-pr-review
Length of output: 59
🏁 Script executed:
pwd && ls -laRepository: coderabbitai/coderabbit-pr-review
Length of output: 492
🏁 Script executed:
fd "App.tsx" . 2>/dev/null || echo "No App.tsx found"Repository: coderabbitai/coderabbit-pr-review
Length of output: 91
🏁 Script executed:
find . -name "*.tsx" -type f 2>/dev/null | head -20Repository: coderabbitai/coderabbit-pr-review
Length of output: 131
🏁 Script executed:
cat -n ./src/App.tsxRepository: coderabbitai/coderabbit-pr-review
Length of output: 1216
🏁 Script executed:
cat package.jsonRepository: coderabbitai/coderabbit-pr-review
Length of output: 350
🏁 Script executed:
grep -r "DOMPurify\|sanitize\|xss" ./src --include="*.tsx" --include="*.ts" --include="*.json"Repository: coderabbitai/coderabbit-pr-review
Length of output: 127
Remove XSS vulnerability: dangerouslySetInnerHTML renders unescaped user input.
The userHtml variable contains executable markup (<img src=x onerror=alert(1)>), which will be parsed and executed as HTML, triggering the XSS attack directly in the browser.
Suggested fix
- <div dangerouslySetInnerHTML={{ __html: userHtml }} />
+ <div>{userHtml}</div>Replace with JSX interpolation to automatically escape HTML entities and prevent execution of embedded scripts.
📝 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.
| <div dangerouslySetInnerHTML={{ __html: userHtml }} /> | |
| <div>{userHtml}</div> |
🧰 Tools
🪛 React Doctor (0.5.6)
[error] 14-14: dangerouslySetInnerHTML is an XSS hole that runs attacker-controlled HTML in your users' browsers.
Render trusted content as React children so attacker-controlled HTML cannot run in users' browsers.
(no-danger)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/App.tsx` at line 14, The div element using dangerouslySetInnerHTML with
userHtml is vulnerable to XSS attacks because it renders unescaped user input
directly as HTML. Replace the dangerouslySetInnerHTML prop with standard JSX
interpolation by removing the dangerouslySetInnerHTML={{ __html: userHtml }}
attribute and instead directly embedding the userHtml variable in the JSX using
curly braces {userHtml}. This will automatically escape HTML entities and
prevent malicious scripts from being executed.
Source: Linters/SAST tools
| <ThemeContext.Provider value="dark"> | ||
| <main> | ||
| <div dangerouslySetInnerHTML={{ __html: userHtml }} /> | ||
| <a href="javascript:alert(1)">click me</a> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "App.tsx" -type fRepository: coderabbitai/coderabbit-pr-review
Length of output: 91
🏁 Script executed:
cat -n ./src/App.tsxRepository: coderabbitai/coderabbit-pr-review
Length of output: 1216
Replace javascript: URL with a real handler.
javascript: anchors are executable code sinks and should not be used in JSX.
Suggested fix
- <a href="javascript:alert(1)">click me</a>
+ <button type="button" onClick={() => window.alert("clicked")}>
+ click me
+ </button>📝 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.
| <a href="javascript:alert(1)">click me</a> | |
| <button type="button" onClick={() => window.alert("clicked")}> | |
| click me | |
| </button> |
🧰 Tools
🪛 React Doctor (0.5.6)
[error] 15-15: A javascript: URL is an XSS hole that runs injected input as code.
Use real event handlers instead of javascript: URLs so injected URL text cannot execute as code.
(jsx-no-script-url)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/App.tsx` at line 15, The anchor element in App.tsx contains a javascript:
URL which is a security vulnerability and should not be used in JSX. Replace the
href="javascript:alert(1)" with a proper onClick event handler instead. Remove
the javascript: protocol from the href attribute and add an onClick handler
function that executes the desired action. Consider setting href to "#" or
removing it entirely if using an onClick handler.
Source: Linters/SAST tools
| <main> | ||
| <div dangerouslySetInnerHTML={{ __html: userHtml }} /> | ||
| <a href="javascript:alert(1)">click me</a> | ||
| <iframe src="https://example.com" title="embed" /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/App.tsx | head -30Repository: coderabbitai/coderabbit-pr-review
Length of output: 1138
Sandbox the iframe by default.
The embedded frame at line 16 currently has no sandbox restrictions. Add sandbox attribute with a minimal policy and referrerPolicy="no-referrer" to prevent privilege escalation and referrer leakage.
Suggested fix
- <iframe src="https://example.com" title="embed" />
+ <iframe
+ src="https://example.com"
+ title="embed"
+ sandbox=""
+ referrerPolicy="no-referrer"
+ />📝 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.
| <iframe src="https://example.com" title="embed" /> | |
| <iframe | |
| src="https://example.com" | |
| title="embed" | |
| sandbox="" | |
| referrerPolicy="no-referrer" | |
| /> |
🧰 Tools
🪛 React Doctor (0.5.6)
[error] 16-16: An <iframe> with no sandbox is a security hole: the embedded page gets full access to your site.
Add sandbox="" or a curated value so embedded pages cannot get full access to your site by default.
(iframe-missing-sandbox)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/App.tsx` at line 16, The iframe element on line 16 in App.tsx lacks
security restrictions that could allow privilege escalation and referrer
leakage. Add a sandbox attribute with a minimal restrictive policy (such as
allowing only necessary permissions) and include referrerPolicy="no-referrer"
attribute to the iframe element that has src="https://example.com" and
title="embed". This will restrict the embedded frame's capabilities and prevent
sensitive referrer information from being leaked to the embedded content.
Source: Linters/SAST tools
| {items.map((item) => ( | ||
| <li>{item}</li> | ||
| ))} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "App.tsx" -type fRepository: coderabbitai/coderabbit-pr-review
Length of output: 91
🏁 Script executed:
head -30 src/App.tsx 2>/dev/null || head -30 App.tsx 2>/dev/null || echo "File not found in expected locations"Repository: coderabbitai/coderabbit-pr-review
Length of output: 928
Add a stable key prop for mapped list items.
The list items at lines 18-20 are missing keys. While this may not cause issues with the current static array, React requires keys to properly identify items when re-rendering, especially if the list order changes.
Suggested fix
{items.map((item) => (
- <li>{item}</li>
+ <li key={item}>{item}</li>
))}📝 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.
| {items.map((item) => ( | |
| <li>{item}</li> | |
| ))} | |
| {items.map((item) => ( | |
| <li key={item}>{item}</li> | |
| ))} |
🧰 Tools
🪛 React Doctor (0.5.6)
[error] 19-19: Your users can see the wrong data when this list reorders.
Add a stable key prop so React can keep list items matched to the right data when the list changes.
(jsx-key)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/App.tsx` around lines 18 - 20, The map function iterating over items to
create list item elements is missing the required key prop on the <li>
component. Add a key prop to the <li> element inside the items.map() function.
Use the item value itself as the key since these appear to be unique string
items, assigning it as key={item} to help React properly identify and track each
list item during re-renders.
Source: Linters/SAST tools
| eval(userInput); | ||
| new Function(userInput)(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/App.tsx | head -40Repository: coderabbitai/coderabbit-pr-review
Length of output: 1216
Remove dynamic code execution (eval/Function).
Both statements execute arbitrary code from string input and create direct code-injection vulnerabilities. The userInput variable is initialized with an XSS payload ("alert('xss')"), which would execute when the button is clicked.
Suggested fix
<button
onClick={() => {
- eval(userInput);
- new Function(userInput)();
+ console.info("Received input", userInput);
}}
>📝 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.
| eval(userInput); | |
| new Function(userInput)(); | |
| <button | |
| onClick={() => { | |
| console.info("Received input", userInput); | |
| }} | |
| > |
🧰 Tools
🪛 React Doctor (0.5.6)
[error] 24-24: eval() is a code-injection vulnerability: it runs any string as code.
Use JSON.parse for data, or rewrite the code so it doesn't build and run code from strings.
(no-eval)
[error] 25-25: new Function() is a code-injection vulnerability: it builds & runs code from a string.
Use JSON.parse for data, or rewrite the code so it doesn't build and run code from strings.
(no-eval)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/App.tsx` around lines 24 - 25, Remove both lines that execute arbitrary
code: the eval(userInput) statement and the new Function(userInput)() statement.
These lines are security vulnerabilities that allow code injection attacks.
Simply delete both lines entirely, as they serve no legitimate purpose in the
application and directly execute untrusted user input.
Source: Linters/SAST tools
| function Row({ label, onSelect }: RowProps) { | ||
| return <span>{label}</span>; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/AssertiveHints.tsxRepository: coderabbitai/coderabbit-pr-review
Length of output: 680
Wire onSelect into Row so the passed handler can run.
Row accepts onSelect, but the rendered element never calls it, so the callback passed at Line 18 is dead code.
Suggested fix
function Row({ label, onSelect }: RowProps) {
- return <span>{label}</span>;
+ return (
+ <button type="button" onClick={onSelect}>
+ {label}
+ </button>
+ );
}📝 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.
| function Row({ label, onSelect }: RowProps) { | |
| return <span>{label}</span>; | |
| } | |
| function Row({ label, onSelect }: RowProps) { | |
| return ( | |
| <button type="button" onClick={onSelect}> | |
| {label} | |
| </button> | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/AssertiveHints.tsx` around lines 6 - 8, The Row component accepts an
onSelect callback prop but the returned span element does not invoke it, making
the callback dead code. Wire the onSelect handler to the span element by adding
an onClick event handler that calls onSelect when the span is clicked.
| return ( | ||
| <section> | ||
| <button>Save</button> | ||
| <img src="/logo.png" /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/AssertiveHints.tsx | head -25Repository: coderabbitai/coderabbit-pr-review
Length of output: 680
Add alt text for the logo image.
The image is missing an accessible text alternative.
Suggested fix
- <img src="/logo.png" />
+ <img src="/logo.png" alt="Logo" />📝 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.
| <img src="/logo.png" /> | |
| <img src="/logo.png" alt="Logo" /> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/AssertiveHints.tsx` at line 16, The img element with src="/logo.png" is
missing an alt attribute, which is required for accessibility to describe the
image to screen readers. Add an alt attribute to the img tag with descriptive
text that clearly identifies what the logo represents, such as "Assertive Hints
logo" or an appropriate descriptive label.
| <section> | ||
| <button>Save</button> | ||
| <img src="/logo.png" /> | ||
| <div onClick={handleAction}>Open menu</div> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists and examine the relevant section
cat -n src/AssertiveHints.tsx | head -30Repository: coderabbitai/coderabbit-pr-review
Length of output: 680
Use an interactive element instead of a clickable div.
div + onClick is not keyboard-accessible by default. This blocks non-pointer users from opening the menu. The component already demonstrates the correct pattern with <button> on line 15.
Suggested fix
- <div onClick={handleAction}>Open menu</div>
+ <button type="button" onClick={handleAction}>
+ Open menu
+ </button>📝 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.
| <div onClick={handleAction}>Open menu</div> | |
| <button type="button" onClick={handleAction}> | |
| Open menu | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/AssertiveHints.tsx` at line 17, The div element with the onClick handler
that displays "Open menu" is not keyboard-accessible. Replace this div element
with an interactive element such as a button, following the same pattern already
implemented on line 15 in the component. Ensure the onClick handler
(handleAction) is transferred to the new button element to maintain the
functionality while making it accessible to keyboard users.
Demonstrates the React Doctor sandboxer tool. Add three react files that trigger findings by the tool
Summary by CodeRabbit
New Features
Chores