python-code-review

Python Code Review Patterns

This skill provides Python-specific code review guidelines. Use alongside python-style for comprehensive review.

Critical Security Issues

SQL Injection

# VULNERABLE - string formatting in queries
cursor.execute(f"SELECT * FROM users WHERE id = {user_id}")
User.objects.raw(f"SELECT * FROM users WHERE name = '{name}'")

# SAFE - parameterized queries
cursor.execute("SELECT * FROM users WHERE id = %s", [user_id])
User.objects.raw("SELECT * FROM users WHERE name = %s", [name])

Command Injection

# VULNERABLE - unsanitized input in shell commands
os.system(f"grep {user_input} /var/log/app.log")
subprocess.run(f"convert {filename} output.png", shell=True)

# SAFE - use list arguments, avoid shell=True
subprocess.run(["grep", user_input, "/var/log/app.log"])
subprocess.run(["convert", filename, "output.png"], check=True)

Path Traversal

# VULNERABLE - user input directly in path
file_path = f"/uploads/{filename}"
with open(file_path) as f:
    return f.read()

# SAFE - validate and sanitize paths
from pathlib import Path
base_path = Path("/uploads").resolve()
file_path = (base_path / filename).resolve()
if not file_path.is_relative_to(base_path):
    raise ValueError("Invalid path")

Hardcoded Secrets

Flag any of these patterns:

# VULNERABLE
API_KEY = "sk-abc123..."
password = "admin123"
SECRET_KEY = "hardcoded-secret"

# SAFE - use environment variables
API_KEY = os.environ["API_KEY"]
password = os.environ.get("DB_PASSWORD")

High Priority Logic Bugs

Mutable Default Arguments

# BUG - shared mutable default
def add_item(item, items=[]):
    items.append(item)
    return items

# CORRECT
def add_item(item, items=None):
    if items is None:
        items = []
    items.append(item)
    return items

Exception Swallowing

# BUG - silently swallowing errors
try:
    process_data()
except Exception:
    pass

# CORRECT - handle or re-raise
try:
    process_data()
except SpecificError as e:
    logger.warning("Processing failed: %s", e)
    raise