Section 1: Reviewing code for security
A routine for security focused pull request review
Most code reaches production through pull requests, so that is where security review has the most leverage. Reviewing every change in depth is impossible; reviewing the right changes with a consistent routine is very effective.
Decide how deep to go
Use simple triggers to decide when a change deserves a security focused review:
- It touches authentication, authorisation, sessions or tokens.
- It adds or changes an endpoint, a message consumer or a webhook.
- It introduces raw SQL, command execution, templating with unescaped output, file handling or outbound requests to user supplied addresses.
- It changes cryptography, secrets handling or security relevant configuration.
- It adds a new dependency.
Many teams encode these triggers as path based code owners, so changes to sensitive directories automatically request review from a security champion.
The routine
- Understand the intent. Read the description and tests first. What should this change allow, and for whom?
- Map the entry points. Which new inputs exist? Who can reach them, authenticated or not?
- Follow the data. For each input, trace where it goes. Does it reach a sink? Is the right sanitiser applied for that sink?
- Check the decisions. For each action, where is the authorisation check? Does it check the specific object?
- Look at failure paths. What happens on exceptions, timeouts and invalid input? Does anything fail open?
- Check the tests. Are there tests for the negative cases, such as another user's data being refused?
Writing useful review comments
A good security comment states the risk, the scenario and a concrete fix:
This endpoint loads the project by id without checking that it belongs to the
caller's organisation, so any logged in user could read other organisations'
projects by changing the id. Suggest filtering the query by
organisation_id from the session, and adding a test for the cross
organisation case.
Avoid vague comments such as "is this secure?". They create friction without guidance.
Keep a record
When review finds a real issue, ask whether a rule could catch it automatically next time. That question links manual review to the automation in the rest of this course: humans find new kinds of problems, and tools stop the known kinds from returning.
Pairing with tools
Run automated scanners before human review, so reviewers do not spend attention on things a tool already reports. Reviewers then focus on logic, design and missing checks.