Files
prompt-library/examples/development-code-review-example.md
T
xixu-me 3be7d016e7 Remove unused workflows and update markdown files
Deleted 'link-check' and 'spell-check' jobs from the quality-assurance.yml workflow as they are no longer needed. Updated formatting and structure in multiple markdown files to improve readability and consistency, including financial-analyzer.md, bug-hunter.md, lesson-plan-generator.md, quiz-generator.md, examples/README.md, development-code-review-example.md, and technical-documentation.md.
2025-06-01 22:23:19 +08:00

168 lines
5.7 KiB
Markdown

# Code Review Example
## Scenario
A senior developer is reviewing a junior developer's pull request for a user authentication function. The code works but needs improvement for production readiness.
## Input Provided to AI
**Code to Review:**
```python
def login_user(username, password):
users = [
{"username": "admin", "password": "admin123"},
{"username": "user1", "password": "password"},
{"username": "user2", "password": "test123"}
]
for user in users:
if user["username"] == username and user["password"] == password:
return True
return False
def register_user(username, password):
if len(password) < 6:
return "Password too short"
return "User registered successfully"
```
**Focus Areas:** Security, best practices, error handling
## AI Output
### Code Review Analysis
**CRITICAL Security Issues:**
1. **Plaintext Password Storage** (Line 3-7)
- **Issue**: Passwords stored in plaintext arrays
- **Risk**: Complete credential exposure if code is compromised
- **Fix**: Implement proper password hashing with bcrypt/argon2
2. **Hardcoded User Database** (Line 2-6)
- **Issue**: User credentials hardcoded in source code
- **Risk**: Credentials visible in version control
- **Fix**: Move to secure database with environment-based config
**HIGH Priority Issues:**
1. **No Input Validation** (Both functions)
- **Issue**: No validation for None/empty inputs
- **Risk**: Runtime errors and potential injection attacks
- **Fix**: Add comprehensive input sanitization
2. **Weak Password Policy** (Line 2, register_user)
- **Issue**: Only checks length, no complexity requirements
- **Risk**: Vulnerable to brute force attacks
- **Fix**: Implement strong password validation
**MEDIUM Priority Issues:**
1. **No Rate Limiting**
- **Issue**: Unlimited login attempts possible
- **Risk**: Brute force vulnerability
- **Fix**: Add login attempt tracking and temporary lockouts
2. **Poor Error Messages** (register_user)
- **Issue**: Generic error messages
- **Risk**: Poor user experience
- **Fix**: Provide specific, helpful error descriptions
### Recommended Improvements
```python
import bcrypt
import re
from typing import Optional
from datetime import datetime, timedelta
class UserAuthenticator:
def __init__(self, db_connection):
self.db = db_connection
self.failed_attempts = {}
self.lockout_duration = timedelta(minutes=15)
def login_user(self, username: str, password: str) -> dict:
# Input validation
if not username or not password:
return {"success": False, "error": "Username and password required"}
# Check for account lockout
if self._is_account_locked(username):
return {"success": False, "error": "Account temporarily locked"}
# Fetch user from database
user = self.db.get_user_by_username(username)
if not user:
self._record_failed_attempt(username)
return {"success": False, "error": "Invalid credentials"}
# Verify password hash
if bcrypt.checkpw(password.encode('utf-8'), user['password_hash']):
self._clear_failed_attempts(username)
return {"success": True, "user_id": user['id']}
else:
self._record_failed_attempt(username)
return {"success": False, "error": "Invalid credentials"}
def register_user(self, username: str, password: str, email: str) -> dict:
# Comprehensive validation
validation_result = self._validate_registration_data(username, password, email)
if not validation_result["valid"]:
return {"success": False, "errors": validation_result["errors"]}
# Check if user exists
if self.db.user_exists(username, email):
return {"success": False, "error": "User already exists"}
# Hash password and save user
password_hash = bcrypt.hashpw(password.encode('utf-8'), bcrypt.gensalt())
user_id = self.db.create_user(username, password_hash, email)
return {"success": True, "user_id": user_id}
def _validate_registration_data(self, username: str, password: str, email: str) -> dict:
errors = []
if not username or len(username) < 3:
errors.append("Username must be at least 3 characters")
if not self._is_strong_password(password):
errors.append("Password must be at least 8 characters with uppercase, lowercase, number, and special character")
if not re.match(r'^[^@]+@[^@]+\.[^@]+$', email or ''):
errors.append("Valid email address required")
return {"valid": len(errors) == 0, "errors": errors}
```
## Analysis
### What Worked Well
- Clear identification of critical security vulnerabilities
- Prioritized issues by severity level
- Provided specific, actionable fixes
- Included improved code example with proper patterns
### Areas for Improvement
- Could include more specific examples of attack vectors
- Missing discussion of logging and monitoring requirements
- Could suggest specific testing strategies for the fixes
### Key Takeaways
1. Security should always be the top priority in code reviews
2. Provide constructive, specific feedback with examples
3. Explain the "why" behind recommendations
4. Offer concrete solutions, not just problem identification
## Variations
- **Quick Review**: Focus only on critical issues for faster turnaround
- **Mentoring Style**: Include more educational context for junior developers
- **Production Focus**: Emphasize scalability and monitoring aspects
- **Security Audit**: Deep dive into all potential security vulnerabilities