Code Review

Review code for issues, improvements, and best practices.

Instructions

When asked to review code, perform a thorough analysis looking for:

1. Security Issues (Critical) - OWASP Top 10

• Injection: SQL, command, LDAP injection via string concatenation
• XSS: innerHTML, dangerouslySetInnerHTML, v-html with user data
• Broken Auth: Hardcoded credentials, weak passwords, missing rate limiting
• Sensitive Data Exposure: Secrets in code, sensitive data in logs/URLs
• Broken Access Control: Missing auth checks, IDOR, privilege escalation
• Security Misconfiguration: Debug mode, default credentials, verbose errors
• Insecure Deserialization: pickle, eval() with user input
• Vulnerable Dependencies: Outdated packages with known CVEs

// VULNERABLE - SQL injection
const query = `SELECT * FROM users WHERE name = '${userInput}'`;

// SAFE - parameterized query
const query = "SELECT * FROM users WHERE name = ?";
db.execute(query, [userInput]);

// VULNERABLE - XSS
element.innerHTML = userInput;

// SAFE
element.textContent = userInput;
// or with sanitization
element.innerHTML = DOMPurify.sanitize(userInput);

2. Error Handling (High)

• Empty catch blocks
• Missing error boundaries
• Unhandled promise rejections
• Missing null/undefined checks
• Silent failures

3. Type Safety (High for TypeScript)

• Usage of any type
• Missing type annotations
• Type assertions that could fail
• Inconsistent types

4. Code Quality (Medium)

• Functions over 50 lines
• Deep nesting (4+ levels)
• Code duplication
• Magic numbers
• Unclear variable names
• Missing or outdated comments

5. Performance (Medium)

• N+1 query patterns
• Missing memoization for expensive operations
• Unnecessary re-renders (React)
• Large bundle imports
• Missing pagination

6. Maintainability (Low)

• Dead code
• Commented-out code
• TODOs that should be addressed
• Inconsistent patterns
• Missing tests

Output Format

Structure your review like this:

## Code Review: [filename or description]

### Summary
[1-2 sentence overview of the code quality]

### Critical Issues
These must be fixed before merging:

1. **[Issue Title]** (Line X)
   - Problem: [What's wrong]
   - Risk: [What could happen]
   - Fix: [How to fix it]

[Code suggestion if applicable]

### Improvements
These should be addressed:

1. **[Issue Title]** (Line X)
- Current: [What it does now]
- Better: [What it should do]
- Why: [Benefit of changing]

### Minor Suggestions
Nice to have, low priority:

1. **[Suggestion]** (Line X)
- [Brief explanation]

### What's Good
[Acknowledge good patterns and practices in the code]

### Verdict
[ ] Ready to merge
[ ] Needs minor changes
[ ] Needs significant changes
[ ] Needs rewrite

Severity Guide

Review Modes

Quick Review

Focus only on critical and high-severity issues:

"Quick review of this code"

Full Review

Check everything:

"Full review of src/api/users.ts"

Security Review (OWASP Top 10)

Deep dive on security - injection, XSS, auth, access control, secrets:

"Security review of the authentication flow" "Check for SQL injection vulnerabilities" "Full security audit of the API endpoints"

Performance Review

Focus on performance:

"Performance review of the dashboard page"

Dependency Audit

Check for vulnerable dependencies:

"Check our dependencies for known vulnerabilities"

Be Constructive

• Explain why something is an issue, not just that it is
• Provide specific suggestions for fixes
• Acknowledge what's done well
• Be respectful - we all make mistakes
• Focus on the code, not the coder