Security code review is the highest-leverage activity most engineering teams aren't doing well. It's the last human checkpoint before vulnerable code ships. Done right, it catches what SAST misses — logic flaws, authorization gaps, insecure design decisions that no automated tool can reliably identify. Done poorly, it produces a trail of "LGTM" comments and a false sense that security was considered.
This is the checklist we use internally and teach to engineering teams. It's not exhaustive — a full OWASP testing guide review is a different kind of engagement. This is what a competent engineer reviewing a PR should be checking without needing to be a security specialist.
Authentication and session management
Authentication code deserves extra scrutiny. Mistakes here have the broadest blast radius.
Check that passwords are hashed with an appropriate algorithm — bcrypt, Argon2, or scrypt. SHA-256 or MD5 password hashing is a finding. If you're looking at code that stores passwords in plaintext or with reversible encryption, stop the review and treat it as critical.
Verify session tokens are cryptographically random, of sufficient length (at minimum 128 bits), and invalidated on logout. It sounds obvious. The number of session management implementations that don't invalidate server-side session state on logout — only clearing a client-side cookie — is higher than you'd expect. CVE-2022-22965 (Spring4Shell) is a reminder that session-adjacent code paths can carry significant risk even when they don't look like auth code at first glance.
Check that multi-factor authentication state is validated server-side, not just client-side. A common pattern: the frontend checks an MFA flag in a JWT and gates the UI, but the backend accepts the JWT as fully authenticated without checking whether the MFA step was completed. That's a complete auth bypass.
Authorization and access control
Broken access control is OWASP A01 — the most common web application vulnerability. It's also the hardest to catch with automated tools because it's architectural and contextual.
For every data access in the code you're reviewing, ask: does this endpoint verify that the authenticated user has permission to access this specific resource? Not just "is the user authenticated" but "is this specific user authorized to see this specific record?" Horizontal privilege escalation — where user A can access user B's data by manipulating an ID parameter — is nearly impossible for SAST to catch, because the code is syntactically correct. Only a human reviewer who understands the data model can catch it.
Look for direct object references. An API endpoint that takes /api/documents/12345 and returns the document without checking that the caller owns document 12345 is a textbook IDOR (Insecure Direct Object Reference). OWASP A01 covers this, but naming it in the review comment helps engineers understand what they're fixing.
Check that authorization is enforced at the service layer, not just the UI layer. If removing a button from the frontend is the only thing preventing unauthorized access to a backend action, that's a finding.
Input handling and injection
OWASP A03 (Injection) covers SQL injection, command injection, LDAP injection, and cross-site scripting. The common thread is unsanitized user input reaching an interpreter.
Check for parameterized queries everywhere SQL is constructed. String concatenation to build SQL queries is a finding, full stop, regardless of apparent input validation elsewhere. Defense in depth applies: sanitization at the edge doesn't protect against injection if parameterization isn't also present.
For XSS: check where user-controlled data is rendered in HTML. Is it properly escaped for its context — HTML body, HTML attribute, JavaScript context, URL? Context-sensitive output encoding is the correct fix. A single htmlspecialchars() call is only right if the data is going into HTML body context. The same data in a JavaScript string needs different encoding. React and modern template frameworks handle most of this by default, but look for dangerouslySetInnerHTML, innerHTML assignments, and v-html usage — all are potential XSS vectors when the input isn't fully controlled.
Cryptography and secrets
Cryptography review has two levels: spotting obviously wrong choices, and spotting absent choices.
Obviously wrong: MD5 or SHA-1 for integrity checks, DES or 3DES for encryption, ECB mode block cipher usage, hard-coded encryption keys, fixed IVs. Any of these is a finding.
Absent: data that should be encrypted at rest isn't. Sensitive data in logs (passwords, tokens, PII). API keys or credentials in code rather than environment variables or secrets management. These are often worse in practice than the obviously-wrong choices because they're invisible — the absence of a security control doesn't generate a warning.
We're not suggesting every code reviewer needs to audit cryptographic implementations at a mathematical level. We're saying that these specific, identifiable patterns — wrong algorithm, wrong mode, missing encryption, plaintext secrets — are things any engineer can spot in a PR review.
Error handling and logging
Error handling code is where a lot of security issues hide. Look for exception handlers that return stack traces to the client — that's information disclosure that tells an attacker about your tech stack, file paths, and internal API structure. Look for log statements that include sensitive data: passwords in "failed login" logs, full credit card numbers in transaction logs, authentication tokens in debug output.
The opposite problem also exists: insufficient logging. Security events — authentication failures, authorization rejections, rate limit hits — should be logged in a way that supports incident response. If a brute-force attack is happening, your logs need to capture enough information to detect the pattern. Review whether new authentication endpoints generate appropriate audit log entries.
What automated tools miss and why this still matters
SAST tools like Semgrep and CodeQL are good at finding syntactic patterns — string concatenation in queries, use of deprecated functions, certain injection patterns. They're poor at finding authorization logic flaws, business logic vulnerabilities, and issues that require understanding the application's data model and user permission structure.
A Semgrep rule can find cursor.execute(f"SELECT * FROM users WHERE id={user_id}"). It cannot find an endpoint that correctly parameterizes its query but fails to check that the requesting user owns the record they're querying. The second finding is often higher impact. It requires a human reviewer who understands the codebase.
Running this checklist on every PR is too slow — you'll create a security bottleneck that makes engineers resent the process. The right scope is new endpoints, authentication/authorization changes, anything touching payment or PII handling, and third-party integrations. For everything else, rely on automated tooling and periodic deeper review cycles. The goal is coverage over critical paths, not coverage over every line of every PR.
