Teams that decide to "add security to code review" usually start with a forty-item checklist copied from a standard. Within three sprints it is being ticked wholesale, because forty items is not a checklist, it is a form.
This is the short version I leave behind after a training engagement. Seven questions, most of which do not apply to most pull requests, and a rule that any one answered "no" needs a sentence explaining why.
The checklist
- New or changed endpoint: is authorisation enforced server side, per object, and is there a test proving a second account gets 404?
- Input: is the request bound to an explicit set of fields, rather than passed wholesale into a model or query?
- Output: does the response contain only what this caller needs — no internal identifiers, no other users’ data, no debug fields?
- Secrets: does this change introduce a credential, key or token anywhere other than the secret manager?
- Dependencies: does it add one, and if so, who maintains it, and does it touch crypto, storage or networking?
- Logging: does it log anything that would be a problem in a log aggregator — tokens, request bodies, personal data?
- Errors: do failures fail closed, and do error messages avoid leaking internal state to the caller?
Why these seven
They map to what actually turns up in assessments of small and mid-size products, weighted by how often each appears.
| Question | The finding it prevents |
|---|---|
| Authorisation on new endpoints | Broken object level authorisation — the most common critical |
| Explicit input binding | Mass assignment of role, balance or ownership fields |
| Response shaping | Excessive data exposure invisible in the interface |
| Secrets location | Credentials in git history, rotated only after a leak |
| Dependency scrutiny | Supply chain risk in packages touching crypto or storage |
| Logging hygiene | Personal data and tokens spread to every log consumer |
| Fail closed | Auth bypasses that only appear when a service is down |
Make it conditional, and make it visible
- Put it in the pull request template as a collapsible section, with a first line that says to delete the items that do not apply.
- Let the author answer, and the reviewer verify. Two people considering the question beats one person ticking it.
- Wire the mechanical items into CI — secret scanning, dependency checks — so humans spend attention on the four that need judgement.
- Review the list every six months against your actual findings. If an item has never caught anything, delete it; a checklist earns its length.
The rule that makes it stick
One rule, agreed by the team rather than imposed: an unanswered security question is a valid reason to hold a merge, and answering it honestly is never held against the author. Teams where "no, and here is why" is a normal review comment catch things. Teams where the checklist is a compliance ritual catch nothing, and find out later, in a report.