Most engineers think “security review” means a slow, exhaustive process. Threat models. Checklists nobody finishes. A separate security team that shows up too late. Secure Code Review Checklist For Prs: What To Look For In 15 Minutes
That’s not how it actually works in practice.
In my experience, most serious security bugs aren’t subtle crypto failures. They’re logic bugs. Authorization gaps. Missing checks. Bad assumptions. And you can catch a large percentage of them in 15 focused minutes if you know where to look.
I’ve reviewed hundreds of PRs under time pressure. I’ve seen auth bugs ship because a reviewer skimmed past a “small” change. I’ve been on-call for incidents caused by a single missing tenant filter. I’ve also learned what doesn’t matter nearly as much as people think.
This post is a practical checklist for secure PR review. It’s not a compliance guide. It won’t make you perfect. It will help you catch the bugs that actually hurt teams in production.
Who this is for:
-
Engineers who review PRs regularly
-
Tech leads who want fewer 3am incidents
-
Anyone who’s security-minded but time-constrained
What this is not:
-
A full security audit
-
A replacement for threat modeling
-
A guarantee you’ll catch everything
It’s a way to build better instinct.
The 15-minute secure PR review workflow
This is the routine I actually use. It’s not theoretical. It’s optimized for limited attention and real PR queues.
Orient on threat surface & data flow
Before reading code line by line, I ask a few questions:
-
What new capability does this PR add or change?
-
Who can trigger it?
-
What data does it touch?
-
Does it cross a trust boundary?
I skim the PR description, file list, and API routes. I’m building a mental map, not judging code yet. If I can’t explain the feature in one sentence, that’s already a smell.
Minute 2: Identify high-risk changes
Now I scan for red flags:
-
Auth logic
-
Permission checks
-
New endpoints
-
Data access queries
-
External calls
-
Anything with “admin”, “internal”, “debug”, or “override” in the name
If nothing high-risk is present, I relax. If something is, I slow down and spend most of my time there.
Minute 5: Auth & authorization review
This is where most real security bugs live.
I mentally trace:
-
Who is the actor?
-
What object are they acting on?
-
Where is access enforced?
-
What assumptions does the code make about identity or role?
I’m not just checking “is there an auth check?” I’m checking is it the right check, in the right place, with the right scope.
Logging & auditability
Finally, I ask:
-
If this breaks or is abused, will we know?
-
Will logs tell us who did what?
-
Will we regret not logging something?
This is boring until you’re debugging an incident. Then it’s everything.
That’s the loop. Every PR. Same order. Same mindset.
What changes are “high risk”?
Not all PRs deserve the same level of paranoia. The trick is learning which ones do.
Here’s my mental triage rule:
High risk new entry point, or changed who can do what
Everything else is secondary.
Auth, identity, permissions
Any change involving:
-
Login
-
Sessions
-
Tokens
-
Roles
-
Permissions
-
“isAdmin”, “isOwner”, “canAccess”
These deserve slow, careful review. I’ve seen one-line permission changes cause full data leaks.
Example I’ve seen in production:
A PR changed to “support more roles”. That accidentally granted destructive access to roles that were never meant to have it.
Sensitive data & secrets
Watch for:
-
PII
-
Financial data
-
Access tokens
-
API keys
-
Internal IDs exposed externally
I’ve seen “temporary debug logs” leak secrets into centralized logging systems that dozens of people could access.
Inputs that can execute or escape
This includes:
-
SQL query building
-
Template rendering
-
Shell commands
-
File paths
-
Regexes built from user input
-
Deserialization
These bugs still happen, but more often through logic shortcuts than obvious string concatenation.
External calls & integrations
Any time code talks to:
-
Webhooks
-
Third-party APIs
-
User-provided URLs
-
Internal services with trust assumptions
I’ve seen SSRF, data exfiltration, and privilege escalation bugs here because reviewers focused on “business logic” and skipped the integration details.
Infrastructure or security controls
Changes to:
-
Feature flags
-
Rate limits
-
CORS
-
CSP
-
Auth middleware
-
Network rules
These are easy to gloss over and painful to roll back.
If a PR touches any of the above, it gets extra scrutiny. If it touches more than one, I assume it’s dangerous until proven otherwise.
How to review auth changes
This is the part that matters most. And it’s where most reviewers get it wrong.
Authentication vs authorization
Authentication answers: who are you?
Authorization answers: what are you allowed to do?
Most teams are decent at authentication. Libraries help. Frameworks help.
Authorization is where things quietly fall apart.
Why?
Because it’s domain-specific, contextual, and often implicit. The code “looks right” until you think about edge cases.
What not to trust
In reviews, I assume the following are untrusted unless proven otherwise:
-
Client-provided user IDs
-
Role flags from the frontend
-
“Hidden” fields
-
Query parameters
-
Headers added by intermediate services (unless cryptographically protected)
I’ve seen bugs where reviewers thought “this endpoint is internal” until someone found a way to call it externally.
Common dangerous patterns I watch for
Debug or bypass flags
I’ve seen “temporary” debug paths ship and stay for years. If there’s a bypass, it will be abused eventually.
If it truly must exist, it should be:
-
Explicitly scoped
-
Heavily logged
-
Hard to enable accidentally
Trusting client input for authorization
Why is the client allowed to choose it?
IDOR (Insecure Direct Object Reference) bugs almost always look “reasonable” in isolation.
Overbroad admin checks
Admins are not magical.
Ask:
-
Which admin?
-
Admin of what?
-
Global or scoped?
I’ve seen “admin” mean “billing admin”, “org admin”, “support admin”, all with different intended powers.
Confused deputy problems
This happens when a service with high privilege acts on behalf of a low-privilege user without proper checks.
A background job runs with system credentials but accepts a parameter from a request. The job trusts that ID and performs privileged actions for the wrong user.
How these bugs actually slip through
Usually like this:
-
The reviewer checks that some auth exists
-
The happy path works
-
Tests pass
-
No one mentally simulates a malicious user
Under time pressure, it’s tempting to move on.
What I mentally trace during review
I slow down and ask:
-
Who is the actor?
Logged-in user? System? External service?
-
What is the target?
User record? Org resource? File? Job?
-
Where is the check enforced?
Controller? Service layer? Database query?
-
What happens if the check is skipped or mis-scoped?
Cross-tenant access? Data corruption? Privilege escalation?
If I can’t answer these clearly, I leave a comment.
What “good” looks like in real codebases
Good authorization code tends to have these properties:
-
Checks are close to the data access
-
Authorization is explicit, not implied
-
Deny paths exist and are tested
-
Role and scope are clearly separated
-
There’s one obvious place to change the rules
Bad code spreads checks across layers, relies on naming conventions, or assumes callers “did the right thing”.
You don’t need perfection. You need clarity.
What logs should be added with new features?
This is the part most PRs get wrong until there’s an incident.
I’ve debugged production issues at 3am with useless logs. I’ve also been saved by one good log line.
The logging mental model
For security-relevant actions, logs should answer:
Actor → action → target → result
Concretely:
-
Actor
who did it? (user ID, service name)
-
Action
what happened? (delete, update, login, export)
-
Target
what was affected? (resource ID, org ID)
-
Result
success, failure, denied
If a log line doesn’t help reconstruct that, it’s probably noise.
Security events that actually matter
I care about logs for:
-
Auth failures (especially permission denied)
-
Privileged actions
-
Data exports
-
Role or permission changes
-
Cross-tenant access attempts
-
Webhook deliveries and failures
I care less about:
-
Every read
-
Debug spam
-
Framework-level noise
What not to log
Never log:
-
Passwords
-
Tokens
-
Secrets
-
Full request bodies with PII
-
Raw authorization headers
I’ve seen teams leak credentials into logs, then replicate those logs to analytics systems. That turns a small mistake into a systemic risk.
How bad logging hurts incident response
Bad logging means:
-
You don’t know who triggered the issue
-
You can’t scope the blast radius
-
You can’t tell if it’s still happening
-
You can’t explain it to stakeholders
Good logging doesn’t prevent incidents. It makes them survivable.
Optional but valuable additions
If the feature is sensitive, consider:
-
Metrics on auth failures
-
Alerts on repeated denied actions
-
Sampling logs for high-risk endpoints
Not every PR needs this. But some really do.
A copy-paste 15-minute secure PR checklist
You can paste this into a PR template or keep it nearby.
Threat surface
-
Does this add a new endpoint, job, or integration?
-
Does it change who can do what?
Auth & authorization
-
Is the actor clearly identified?
-
Are authorization checks explicit and scoped?
-
Is client input trusted for identity or permissions?
-
Are there bypasses, debug flags, or admin shortcuts?
-
Are deny paths handled and tested?
Data access
-
Is tenant or ownership scoping enforced at query level?
-
Could this access someone else’s data?
Inputs & integrations
-
Are user inputs validated before use?
-
Are external calls constrained and safe?
Logging & auditability
-
Are security-relevant actions logged?
-
Do logs include actor, action, target, result?
-
Are secrets or PII accidentally logged?
If you can’t confidently check a box, leave a comment.
Common PR review comments
These are comments I’ve actually left, or received, in real reviews.
-
“Where is tenant scoping enforced here? I don’t see it in the query.”
-
“This trusts from the request body can we derive it from auth context instead?”
-
“What prevents a non-admin from hitting this path directly?”
-
“Can we add a deny-path test for unauthorized access?”
-
“This logs the full payload, which includes PII. Can we redact?”
-
“Auth check seems to live in the controller what happens if this method is called elsewhere?”
-
“Is this ‘temporary’ bypass safe to ship? How is it disabled in prod?”
If your comments sound like these, you’re doing real security review.
You Might Be Interested In
- Content Security Policy CSP: a starter policy you can deploy safely
- Ai Wont Solve Security It Changes The Battlefield
- Sigstore/cosign Basics: Signing Container Images Without Managing Keys
- Top Cybersecurity Certifications Worth Pursuing in 2025
- Sbom For Beginners: What It Is And How To Generate It In Ci ?
Conclusion
Secure code review isn’t about being paranoid or perfect. It’s about being consistently skeptical in the right places. Most serious security incidents don’t come from exotic attacks they come from ordinary code paths with slightly wrong assumptions, merged on a busy day.
If you take away one thing, let it be this: slow down only where it matters. New entry points. Authorization changes. Data access. Logging. Fifteen intentional minutes on those areas will catch more real-world bugs than hours of unfocused review.
This checklist isn’t a silver bullet, and it won’t eliminate every risk. But used regularly, it builds judgment. It sharpens instincts. And over time, it quietly reduces the number of incidents your team has to explain after the fact. Add it to your PR template, use it until it’s muscle memory, and let security become a habit not a fire drill.
FAQs about Secure Code Review Checklist For Prs: What To Look For In 15 Minutes
What counts as a high-risk PR?
A high-risk PR is any change where a mistake would have security or data-blast consequences, not just a broken feature. In practice, this usually means anything that introduces a new entry point (API endpoint, background job, webhook, internal tool) or changes the rules around who can do what. Even small-looking refactors can be high risk if they subtly alter authorization flow or data access patterns.
In real incidents I’ve seen, the riskiest PRs were not large rewrites. They were “tiny” changes: moving a permission check, reusing a helper, loosening a condition to “support more cases.” If a bug here could let one user see another user’s data, act as an admin, or bypass a safeguard, it deserves extra attention regardless of PR size.
What’s the fastest way to spot auth bugs?
The fastest way is to stop thinking like a normal user and start thinking like a curious attacker. Don’t follow the happy path. Instead, trace what happens if someone tampers with IDs, skips the UI, or calls the endpoint directly. Ask yourself what the code assumes is true, and whether those assumptions are actually enforced anywhere.
In reviews, I mentally change inputs: different user IDs, different tenant IDs, missing fields, unexpected roles. Most auth bugs reveal themselves immediately when you do this. If you only verify that “an auth check exists,” you’ll miss the real failures, which usually come from checks that are present but incomplete, mis-scoped, or placed in the wrong layer.
How do you review multi-tenant access?
I focus almost entirely on where tenant scoping is enforced. The most important question is whether the tenant or ownership constraint lives close to the data access, or whether it’s assumed earlier in the request flow. If it’s enforced only in controllers or service logic, I get nervous, because those assumptions tend to break as code getsreused.
In multi-tenant systems, I’ve seen many incidents caused by a single missing filter in a query. The safest pattern is when every data access clearly includes tenant or ownership conditions, making it impossible to accidentally fetch or modify another tenant’s data. If I have to “trust” that upstream code did the right thing, that’s usually a review comment.
What logs matter most?
The logs that matter most are the ones that help you reconstruct reality during an incident. When something goes wrong, you want to know who triggered the action, what they attempted, which resource was involved, and whether it succeeded or failed. Logs that don’t answer those questions are rarely useful under pressure.
I’ve been in incidents where we knew something bad happened but couldn’t tell who did it or how often. That’s a logging failure. Good logs won’t prevent bugs, but they dramatically reduce mean time to understand, contain, and explain an incident. If a feature adds privileged or sensitive behavior, lack of logging is itself a risk.
What should never be logged?
Anything that would make a bad day worse if it leaked should never be logged. That includes passwords, tokens, secrets, raw authorization headers, and sensitive personal data. Even if logs are “internal,” they tend to spread to analytics systems, debugging tools, and long-term storage far beyond their original scope.
I’ve seen teams accidentally turn a minor auth bug into a serious breach because secrets were logged and then accessed by people who didn’t need them. When reviewing logging changes, I always assume logs live longer and travel farther than intended. If you wouldn’t paste the value into a shared Slack channel, it probably doesn’t belong in logs either.

