Code Review for Security

By Davy Rogers

Security bugs are cheapest to fix during review. Here's what to look for.

A security bug caught in review costs a comment. The same bug caught in production costs an incident. That economics is why code review is one of the highest-leverage security activities a team has, and it barely costs anything extra: you're already reviewing the code. The shift is in the question you ask. Functional review asks "does this work?" Security review asks "what happens if the input is hostile, the user is malicious, or the assumption behind this is wrong?"

A checklist to run in your head

You don't need to memorise a framework. Run new code past these categories:

  • Authentication. Does this endpoint require auth? Are sessions validated? Are tokens handled safely?
  • Authorisation. Is the user authorised, not just authenticated? Are object-level permissions checked? Is the check on the server?
  • Input. Is it schema-validated? SQL parameterised? HTML escaped? Shell commands avoided? File paths sanitised?
  • Data exposure. Are only the needed fields returned? No password hashes, no stack traces leaking out?
  • Crypto. Are secrets out of the code? Passwords hashed with bcrypt or Argon2? TLS enforced?
  • Dependencies. Is anything new from a trusted, maintained source with no known vulnerabilities?

Patterns to flag the moment you see them

Some constructs are almost always wrong. Train your eye to stop on these:

// SQL built by string concatenation
const query = `SELECT * FROM users WHERE id = '${userId}'`;

// Raw HTML from user input
<div dangerouslySetInnerHTML={{ __html: userInput }} />

// A hardcoded credential
api_key = "sk-live-abc123def456"

// Authentication without authorisation
app.delete('/api/users/:id', requireAuth, async (req) => {
  await db.deleteUser(req.params.id);  // Any logged-in user can delete any user
});

// Command injection
os.system(f"convert {user_filename} output.png")

// Path traversal
path.join('/uploads', req.params.filename)  // ../../../etc/passwd

// Insecure randomness for something that needs to be unguessable
Math.random().toString(36)  // Use crypto.randomBytes

Triage what you find

Not every finding deserves the same urgency. Weigh exposure against sensitivity:

Higher priorityLower priority
Internet-facingInternal tool
Untrusted inputTrusted service
PII, credentialsPublic data
Trivial to exploitNeeds admin plus specific conditions

How you say it matters

A review comment is also a conversation, and "This is insecure!!!" helps nobody. The version that gets fixed explains the risk, shows how it'd be exploited, and suggests the fix. Be specific and constructive, because the goal is the bug getting fixed and the next one getting prevented, not being right.

Let machines catch the obvious

Automate the boring half so humans can focus on the half that needs judgement. Pre-commit hooks (detect-secrets) and CI gates (Semgrep, npm audit, trufflehog) reliably catch hardcoded secrets and known patterns. That frees your reviewers to think about the logic flaws no scanner will ever spot, which is where the interesting bugs live. Apply the checklist consistently and security review stops being a special event and becomes just how your team reads code.