top of page

Spot Security Issues in Review — Code Reviews That Help, Part 5

Shawn West
Jul 8
3 min read

Updated: 2 days ago

Code Reviews That Help · Part 5

Most security bugs don't look dangerous in a diff — they look like ordinary code that trusts input it shouldn't. A reviewer who knows the handful of recurring patterns catches them for free, in the review you're already doing. This walks through spotting the common ones — injection, broken auth, IDOR, leaked secrets — by learning where to look rather than memorizing every exploit.


Most security issues are recurring patterns. Learn the classes; you'll spot them in any codebase.


Step 1: Trust Boundaries (10 min)


Every PR review: identify boundaries.


  • External → Internal — HTTP requests, API webhooks, file uploads

  • User-controlled → Privileged — user IDs becoming admin operations

  • Untrusted JSON → Code — JSON.parse with eval


At each boundary, ask: "Is this input validated?"


Step 2: SQL Injection (10 min)


# Bad
cursor.execute(f"SELECT * FROM users WHERE id = {user_id}")

# Good
cursor.execute("SELECT * FROM users WHERE id = %s", (user_id,))

Any string concatenation or f-string into SQL = red flag.


ORMs usually safe. Raw queries: need parameterization.


Step 3: XSS (Cross-Site Scripting) (10 min)


<!-- Bad: user content rendered as HTML -->
<div>${userBio}</div>

<!-- Good: framework escapes -->
<div>{userBio}</div>   <!-- React, Vue: auto-escape -->

Watch for:


  • dangerouslySetInnerHTML (React)

  • v-html (Vue)

  • innerHTML (vanilla)

  • Server templates without auto-escape


If user content reaches HTML output: escape.


Step 4: Auth Bypasses (15 min)


Patterns that bypass auth:


# Bad — no auth check on this endpoint
@app.get("/admin/users")
def list_users():
    return User.all()

Reviewer asks: "Where's the auth?"


Common bugs:


  • Endpoint missing decorator

  • Decorator on parent class but not override

  • Permission check on wrong object (own resource vs. any resource)


Specifically check: every new endpoint has explicit auth + authorization.


Step 5: IDOR (Insecure Direct Object Reference) (10 min)


@app.get("/orders/{id}")
def get_order(id, user = current_user()):
    return Order.find(id)   # any user can see any order!

Should be:


return Order.where(id=id, user_id=user.id).first()

Always: filter by current user's ownership. Even for "internal" tools.


Step 6: Secret Handling (10 min)


# Bad
API_KEY = "sk_live_abc123"

# Bad
logger.info(f"Authenticating with key: {api_key}")

# Bad
return jsonify(token=user.session_token)

Watch for:


  • Hardcoded secrets

  • Secrets in logs

  • Secrets in error messages

  • Secrets in responses


Use secret managers; don't ship secrets in code.


Step 7: SSRF (Server-Side Request Forgery) (10 min)


def fetch(url):
    return requests.get(url)

# user controls `url`; can hit internal services
fetch("http://169.254.169.254/latest/meta-data/iam/...")  # AWS metadata

Validate user-provided URLs:


  • Block private IP ranges

  • Block 169.254.* (cloud metadata)

  • Whitelist domains where possible


Step 8: Path Traversal (5 min)


@app.get("/files/{name}")
def serve(name):
    return send_file(f"/uploads/{name}")

# user requests: ../../etc/passwd

Sanitize or use safe_join:


path = Path("/uploads") / name
if "/uploads" not in str(path.resolve()):
    abort(403)

Step 9: Race Conditions (10 min)


# Bad
def transfer(from_id, to_id, amount):
    from_acct = Account.find(from_id)
    if from_acct.balance >= amount:
        from_acct.balance -= amount
        Account.find(to_id).balance += amount

Two concurrent transfers can both pass the check. Lost money.


Fix: transaction + atomic update or row lock.


In review: ask "what if two requests race?"


Step 10: Dependencies (5 min)


PRs adding new dependencies:


  • Is the package well-known?

  • How recent? (Recently-published packages can be malicious)

  • Downloads / GitHub stars

  • Maintained?


For npm: npm audit. For Python: pip-audit. For Go: govulncheck.


A typo-squat dep can pwn the project.


What You Just Did


Security in review: trust boundaries, SQL, XSS, auth bypass, IDOR, secrets, SSRF, traversal, races, dependencies. Common patterns to spot.


Review catches what is in the diff. Threat Modeling for Developers, Not Security Teams catches design risks before there is a diff.


Common Failure Modes


No auth check; rubber stamp. Endpoint shipped wide open.


Trust user input in SQL. Injection.


Hardcoded secrets. Repo cloned; secrets leaked.


No dependency review. Supply chain attack.


Custom crypto. Roll your own = wrong.


Generated code adds its own patterns to this list. The AI Failure Mode Register maps sixteen of them to the controls that catch them.


Continue the Code Reviews That Help path



Part of the Code Reviews That Help learning path.

bottom of page