Functional review: "Does this work?" Security review: "What if the input is wrong, the user is malicious, or the assumption is invalid?"
Checklist
Auth: Endpoint requires auth? Sessions validated? Tokens secure?
Authorisation: User authorised (not just authenticated)? Object-level permissions? Server-side checks?
Input: Schema validated? SQL parameterised? HTML escaped? Commands avoided? Paths sanitised?
Data exposure: Only needed fields? No password hashes? No stack traces?
Crypto: Secrets out of code? Passwords with bcrypt/Argon2? TLS enforced?
Dependencies: Trusted source? Maintained? Known vulns?
Flag immediately
// SQL concatenation
const query = `SELECT * FROM users WHERE id = '${userId}'`;
// dangerouslySetInnerHTML
<div dangerouslySetInnerHTML={{ __html: userInput }} />
// Hardcoded credentials
api_key = "sk-live-abc123def456"
// Missing authorisation
app.delete('/api/users/:id', requireAuth, async (req) => {
await db.deleteUser(req.params.id); // Anyone can delete anyone
});
// Command injection
os.system(f"convert {user_filename} output.png")
// Path traversal
path.join('/uploads', req.params.filename) // ../../../etc/passwd
// Insecure randomness
Math.random().toString(36) // Use crypto.randomBytes
Triaging
| Higher priority | Lower priority |
|---|---|
| Internet-facing | Internal tool |
| Untrusted input | Trusted service |
| PII, creds | Public data |
| Simple exploit | Needs admin + specific conditions |
Communication
Bad: "This is insecure!!!"
Good: Explain risk. Show exploit. Suggest fix. Be specific, constructive.
Automation
Pre-commit hooks (detect-secrets). CI gates (Semgrep, npm audit, trufflehog). Automation catches obvious stuff. Humans focus on logic.
The takeaway
Apply the checklist consistently. Flag SQL concatenation, dangerous HTML, missing auth, hardcoded creds. Triage by exposure and sensitivity. Automate the obvious. Make security normal in every review.