A security review comment that gets acted on states the problem, the exact location, why it matters, a concrete fix and a severity label. Keep it about the code, not the author, explain the reasoning, and say plainly whether the change is blocked until it is fixed.
Why vague comments stall
"This looks unsafe" forces the author to guess. They do not know which line you mean, what attack you have in mind, or whether you will block the merge. The usual result is a defensive reply, a silent change that misses the point, or a pull request that sits.
Google's public code review guidance gives a good starting rule: make comments about the code, never about the developer, and explain the reasoning. Its example turns an accusing question into a statement about the code and why a change is better.
The template
OWASP's Secure Code Review Cheat Sheet includes a finding report template with fields such as title, severity, location, description, impact, recommendation and references. That is built for formal reports. For a pull request comment, a trimmed version works:
[blocking] SQL built from user input in search_orders (orders/search.py:42)
Why: the term goes into the query string directly, so a crafted value changes the query.
Fix: pass it as a bound parameter, as the other queries in this file do.
Four parts, three lines. The label tells the author what is expected, the location removes the hunt, the reason teaches, and the fix saves a round trip.
Label severity every time
Google's guidance suggests labelling comment severity so authors can tell mandatory changes from suggestions. Its examples are Nit, Optional and FYI. For security work add one more word at the top of the scale.
| Label | Meaning | Merge effect |
|---|---|---|
| blocking | A vulnerability or a removed control | Do not merge until fixed |
| should fix | Weakness with limited reach | Fix here or file a tracked follow up |
| optional | Hardening idea | Author decides |
| FYI | Context for later | No action |
Without labels, authors tend to treat every comment as mandatory, or treat all of them as noise. A label costs one word and removes that guessing.
Point at the problem, then pick the right amount of help
Google's guidance says it is generally the developer's job to fix the change, not the reviewer's, and that pointing out a problem and letting the author choose a solution often produces a better result. For security that has a limit. When the safe pattern is well known, such as parameterised queries or an existing authorization helper, name it. Otherwise the author may "fix" the symptom.
Say what was done well
The same guidance recommends commenting on things you like, with the reason. A reviewer who writes "good that you scoped this lookup to the current user" teaches the habit and makes the next critical comment easier to hear.
Keep a record for patterns
If you leave the same comment three times in a month, it is no longer a comment. It is a missing lint rule, a missing helper or a missing paragraph in your contributing guide. Collect the repeats and fix them at the source.
Try it yourself
Open the demo and look at how a finding is presented with location and fix guidance. Compare it with your last three review comments and see which of the four parts they lack.
Click through it at your own pace, or open it full screen.
Quick check
Should security comments always block the merge?
No. Reserve blocking for vulnerabilities and removed controls. Use labels so that hardening ideas and context do not hold up work.
Is it fine to paste the fix as code?
Yes when the safe pattern is standard in your codebase. Otherwise describe the pattern and let the author write it, which Google's guidance notes often gives a better result.
Sources
- Google Engineering Practices: How to write code review comments
- OWASP Secure Code Review Cheat Sheet
Last reviewed: 19 October 2026.
