What is a secure code review?
A secure code review reads code with one question in mind: what can a hostile user make this do? It differs from ordinary code review, which asks whether the change is correct, readable and tested. A security reviewer assumes every input is attacker-controlled and looks for the places where the code trusts it anyway.
NIST's Secure Software Development Framework treats it as practice PW.7, "Review and/or Analyze Human-Readable Code," and distinguishes code review (a person reads the code) from code analysis (tools find issues, alone or with a person). The OWASP Secure Code Review Cheat Sheet describes two shapes of work: a baseline review of a whole codebase, for a new application or a compliance need, and a diff-based review of each change as it goes through pull requests. Most teams do the second continuously and the first rarely.
Manual, tool-assisted and AI-assisted review
Each approach finds a different slice of bugs, so mature teams layer them.
| Approach | What it is good at | Where it falls short |
|---|---|---|
| Manual review | Authorization, business logic, intent, design flaws | Slow, inconsistent, depends on the reviewer |
| Tool-assisted (SAST, code search) | Known patterns at scale, taint from input to dangerous call | False positives, blind to logic and access control |
| AI-assisted (LLM reviewers) | Reading intent across files, explaining a finding in plain words | Nondeterministic, can report plausible bugs that do not exist |
Manual review is the only approach that reliably catches a missing ownership check, because spotting one requires knowing that invoice 42 belongs to tenant A. The OWASP Code Review Guide v2.0, published in 2017, runs to over 200 pages on the method: how to scope a review, how to rank code by risk, and "code crawling," searching a codebase for risky APIs such as raw SQL execution or shell calls, per language.
Tool-assisted review means a person working with static analysis output, grep or a query language such as CodeQL. The tool finds candidate lines; the reviewer decides which are exploitable.
AI-assisted review uses a language model, sometimes as an agent that can open other files and run tools, to read the diff and comment. It can follow a request parameter across a controller and a service layer and explain in a sentence why the result is dangerous. It also needs verification: every AI finding should come with a file, a line and a reason a human can check. Agentic code review explained goes into how these reviewers work.
What do reviewers look for?
Reviewers work through bug classes, because each class has a recognizable shape in code. A short checklist:
- Authorization. Every handler that loads a record by ID also checks that the caller may see it. Look for
findById(req.params.id)with no owner or tenant filter, the shape of broken object level authorization. Check that admin-only routes enforce the role on the server. - Injection. Any string built from input and passed to SQL, a shell, a template engine, LDAP or an XPath query. Parameterized queries pass; string concatenation into
query()fails. See SQL injection. - Output encoding. User data written into HTML, JavaScript or attributes without the framework's escaping, including
dangerouslySetInnerHTML,|safeandhtml_safe. - Object binding. A request body passed straight into a model update, letting a client set
roleorplan. - Outbound requests. A URL, hostname or webhook target taken from input, which can point the server at internal addresses.
- Authentication and tokens. JWT verification that accepts the algorithm from the token header, session IDs that survive logout, password reset tokens that do not expire.
- Concurrency. Check-then-act sequences on balances, coupons or invites without a lock or a unique constraint, the setup for a race condition.
- Secrets and configuration. Credentials in code or config files, debug flags, permissive CORS, disabled TLS verification.
- Cryptography. Homemade crypto,
Math.random()for tokens, ECB mode, MD5 or SHA-1 for passwords. - Error handling and logging. Stack traces returned to clients, tokens or passwords written to logs.
How do you review a diff for security?
Start from what the change exposes, then trace data from where it enters to where it is used. A repeatable procedure:
- Read the pull request description and list what is new: routes, parameters, fields, permissions, third-party calls.
- For each new entry point, identify who can reach it (anonymous, any user, admin) and what it accepts.
- Trace each input forward to where it is stored, queried, rendered or sent. Stop at each sink and ask whether it is safe for any value.
- Check what the diff removes. A deleted middleware line or a loosened validation rule is easy to miss because nothing new appears.
- Read the unchanged code around the diff. The missing check is often in a file the change calls but does not modify.
- Compare with sibling code. If every other handler in the folder calls
requireOwner(), the new one that does not is the finding. - Write each finding as: location, what an attacker sends, what they get, and the fix.
A worked example. This handler was added in a pull request:
// routes/projects.js (new in this PR)
router.get("/api/projects/:projectId/export", requireLogin, async (req, res) => {
const project = await Project.findByPk(req.params.projectId);
if (!project) return res.status(404).end();
res.json(await buildExport(project));
});It requires a login, and it has no bug a scanner would flag. Step 6 finds the problem: the neighboring routes call requireProjectMember. Any logged-in user can export any project by changing the ID. The review comment:
routes/projects.js:2 Missing authorization check.
user_a (member of project_1043 only) can GET /api/projects/project_2210/export
and receive project_2210's export. Other routes in this file use
requireProjectMember; add it here, or scope the lookup:
Project.findOne({ where: { id: req.params.projectId, orgId: req.user.orgId } })The IDOR video shows the same flaw from the attacker's side.
Secure code review vs SAST
SAST is one input to a secure code review. A scanner can say that req.query.sort reaches db.query(); only a reviewer can say that the export route above should have checked project membership. Use SAST to cover known patterns on every change, and spend human review time on authorization, logic and anything touching authentication, payments or tenant boundaries.
[ Sources ]
Written by Parameter · Last reviewed
