Secure code review checklist for pull requests

Latest Comments

No comments to show.
A professional software review workspace showing a pull request diff and security checklist interface on a laptop screen

Pull request review is one of the most useful security controls a software team can have, but only if reviewers know what to look for. A good review is not just about style, naming, or whether the code compiles. It is about spotting changes that could weaken authentication, bypass access control, expose data, or create a new path for abuse.

For UK SMEs, the challenge is usually not a lack of tools. Most teams already have some mix of static analysis, dependency scanning, unit tests, and maybe a security gate in CI. The gap is often human judgement. Reviewers skim the diff, see that automated checks passed, and approve the change without testing the security assumptions behind it. This article gives you a practical secure code review checklist for pull requests that fits into a real engineering workflow.

Key takeaways

  • Review pull requests for security impact as well as correctness, with extra scrutiny on authentication, access control, input handling, and secrets.
  • Use automation for repeatable checks, but keep human review focused on business logic, abuse cases, and trust boundary changes.
  • Treat dependency updates, configuration changes, and deployment files as security-relevant code, not just operational detail.
  • Document findings clearly with the affected component, the risk, and the recommended fix so the team can learn and move quickly.

Why pull request reviews matter in secure software delivery

A secure review should catch issues that automated tools often miss. That includes business logic flaws, trust boundary mistakes, insecure defaults, and changes that alter how a system behaves under edge conditions. It should also confirm that the code still enforces security controls server-side, rather than relying on the client, the UI, or a hidden assumption in another service.

Pull request review sits alongside, not instead of, other controls. Static application security testing can flag patterns such as injection sinks or unsafe deserialisation. Software composition analysis can highlight vulnerable packages. Test suites can confirm expected behaviour. But a reviewer still needs to ask whether the change introduces a new attack path, weakens a control, or makes an existing control easier to bypass.

If your team is still building the wider secure development process, it helps to treat review as one part of a broader secure SDLC. Our article on Microsoft Secure Development Lifecycle explains how review, testing, and release controls fit together in practice.

How to use this checklist in a real engineering workflow

Do not apply the same depth of review to every change. Focus effort where the risk is highest. A documentation update does not need the same scrutiny as a change to authentication, payment logic, tenant isolation, or infrastructure permissions. In practice, the most effective teams triage pull requests before review and route sensitive changes to a reviewer with the right context.

A simple approach is to classify changes into three bands. Low-risk changes cover text, internal refactoring with no behaviour change, or non-production code. Medium-risk changes include API updates, new features, and changes to data handling. High-risk changes include authentication, authorisation, secrets, cryptography, access to production systems, and anything that affects multiple tenants or trust boundaries. High-risk changes should get a slower, more deliberate review, and often a second pair of eyes.

Human review works best when it is paired with evidence from automation. Before approving, check the output from SAST, dependency scanning, secret scanning, and test runs. If the team uses policy-as-code or security gates in CI, confirm whether the pull request is trying to bypass them. For teams that want to strengthen the pipeline as well as the review process, our guide to integrating SAST, DAST, and SCA tools into CI pipelines is a useful companion.

Authentication and session handling checks

Start with identity-related changes, because weaknesses here often have the widest impact. Review any code that creates, validates, refreshes, revokes, or stores tokens, cookies, or session identifiers. Check whether the implementation uses secure defaults, whether token lifetime is appropriate, and whether the code handles logout, expiry, and revocation consistently across services.

Look for session fixation risks, especially where a session is created before authentication and then reused after login. Confirm that authentication state is not inferred from a client-side flag alone. If the application uses JWTs, check token audience, issuer, expiry, signing algorithm, and revocation strategy. If the code handles API keys or service tokens, confirm that scope is narrow and that secrets are never returned to the client after creation.

Review password reset and account recovery paths carefully. These flows are often less mature than primary login. Check whether reset tokens are single-use, time-limited, and bound to the right account. Confirm that recovery does not allow account takeover through weak knowledge-based questions, predictable links, or inconsistent email verification. If multi-factor authentication is involved, make sure the bypass path is not easier than the normal path.

Authorisation and access control checks

Access control failures are among the easiest mistakes to miss in a pull request because the code can look reasonable at a glance. Every change that reads, updates, or deletes user-owned data should be checked for server-side authorisation. Do not rely on the front end to hide buttons or filter records. The server must enforce the decision every time.

Watch for broken object level authorisation, where a user can access another user’s record by changing an identifier in a request. Check whether object lookups are always followed by a permission check tied to the authenticated principal. For multi-tenant systems, confirm that tenant boundaries are enforced at the data access layer, not just in the UI or service layer. A missing tenant filter in one query can be enough to expose data across customers.

Privilege escalation is another common review target. If the pull request adds a new role, permission, admin endpoint, or background job, ask whether the code allows a lower-privileged user to reach it indirectly. Review any use of role-based access control, claims, or policy engines to make sure the check happens close to the protected action. If your team wants a deeper treatment of role design and least privilege, see designing least privilege access using IAM and RBAC.

Input handling and output safety checks

Any code that accepts input from a user, another service, a file, or a queue should be treated as untrusted until validated. Review whether the code uses allow-list validation where possible, rather than trying to block bad values one by one. Check length limits, type checks, format checks, and canonicalisation where different encodings or representations could bypass validation.

Injection risks often appear in APIs, templates, shell calls, and database access. Review whether the code uses parameterised queries or safe ORM patterns instead of string concatenation. Check whether template output is encoded for the right context, such as HTML, attribute, JavaScript, or URL context. If the code transforms input before validation, confirm that the transformed value is still safe. A common mistake is validating one representation and using another.

For file uploads, look at extension checks, content-type checks, storage location, and downstream processing. For deserialisation, check whether the code accepts only the formats it truly needs. For command execution, confirm that arguments are passed safely and that shell interpretation is avoided where possible. If you need a refresher on the underlying mechanics, our article on input validation, encoding, and output sanitisation techniques is directly relevant.

Secrets, keys, and sensitive data checks

Reviewers should treat secrets as a release blocker if they appear in the diff. That includes API keys, private keys, access tokens, database credentials, and signing material. Check not only the application code, but also test fixtures, example configuration, sample environment files, and migration scripts. Teams often leak secrets into non-production assets because they assume those files are harmless.

Also review logging and error handling for accidental disclosure. A pull request may not contain a secret directly, but it may add debug output that prints headers, tokens, session identifiers, or personal data. Check whether logs are structured, whether sensitive fields are redacted, and whether error messages reveal enough detail to help an attacker. In regulated or customer-facing environments, this matters as much as the code path itself.

Where secrets are loaded from a vault, environment variable, or managed identity, confirm that the code does not fall back to a weaker source if the preferred source fails. Check rotation assumptions too. If a secret is meant to be rotated, the code should not cache it indefinitely or hard-code a value that makes rotation ineffective. For a deeper implementation view, see secrets management using key vaults and HSMs.

Dependency and supply chain checks

Pull requests that add or update dependencies deserve careful review because the risk is not limited to the package you can see. New packages bring transitive dependencies, build-time scripts, and maintenance obligations. Check whether the package is genuinely needed, whether the version is pinned appropriately, and whether the change introduces a dependency with a wider attack surface than the code it replaces.

Review lockfiles and manifest files together. If the lockfile changed unexpectedly, ask why. If the package manager allows post-install scripts or dynamic fetching, confirm that the behaviour is intentional. For containerised builds, check base image updates, digest pinning, and whether the image source is trusted. For build artefacts, confirm that provenance and signing are part of the release process where appropriate.

Dependency review is also a good place to look for namespace confusion, abandoned packages, and packages with excessive privileges in build pipelines. If the team is maturing its supply chain controls, our article on software supply chain assurance controls gives a practical technical baseline.

Security-sensitive configuration checks

Configuration changes can be as risky as code changes. Review any pull request that alters debug flags, feature flags, environment variables, CORS settings, security headers, TLS settings, or cloud permissions. Unsafe defaults often slip through because they look like temporary convenience settings during development and then remain in place.

Check whether debug mode is disabled outside local development. Confirm that CORS is restricted to known origins and does not allow wildcard credentials. Review HTTP security headers where the application is responsible for them. Check TLS configuration for weak protocols, expired certificates, and inconsistent trust settings between environments. If the application runs in cloud infrastructure, review IAM roles, storage permissions, network exposure, and whether the change broadens access unnecessarily.

Environment separation is important here. A pull request that changes configuration for test or staging should not accidentally weaken production controls. Review whether the code or deployment manifests use environment-specific values correctly, and whether defaults are safe if a variable is missing. This is especially important in CI/CD systems where a single mis-scoped variable can affect multiple environments.

Error handling, logging, and monitoring checks

Good error handling supports both security and operations. Review whether errors are specific enough for developers but not so detailed that they expose internals to users. Stack traces, SQL errors, file paths, and upstream service details should not leak into client responses. If the code returns structured errors, check that the response does not reveal whether a username exists, whether a token was valid, or which part of a security check failed.

Logging should be useful for detection and investigation. Ask whether the change creates the right audit trail for security-relevant actions such as login attempts, permission changes, privilege grants, data exports, and administrative actions. Check whether logs include stable identifiers, timestamps, and enough context to support correlation in a SIEM, while still avoiding sensitive content. A log entry that is too vague is almost as unhelpful as no log at all.

Monitoring matters too. If the pull request introduces a new security-sensitive workflow, confirm that alerts or dashboards exist for abnormal behaviour. For example, a new password reset flow should be observable for spikes, failures, and unusual geography. A new admin function should be visible in audit logs. Teams that want to improve this area often benefit from pairing code review with better logging design, as discussed in what security logs you actually need and why.

Business logic and abuse case checks

Some of the most important review findings are not technical in the narrow sense. They are abuse cases. Ask how a malicious or careless user could misuse the feature at scale, bypass intended limits, or exploit an assumption in the workflow. This is where human review adds real value over automated scanning.

Look at rate limits, quotas, retries, refunds, approvals, and state transitions. Can a user repeat an action more times than intended? Can they cancel or replay a request to get a better outcome? Can they manipulate workflow state by sending requests out of order? Can they use a feature for a purpose it was not designed for, such as bulk data extraction through an export function?

Edge cases are often where the risk sits. Reviewers should ask what happens when input is empty, duplicated, delayed, partially complete, or unexpectedly large. They should also ask what happens when a downstream service fails. If the application fails open rather than closed, or if it retries in a way that amplifies abuse, that should be visible in the review.

What to look for in infrastructure and deployment changes

Security review should not stop at application code. Infrastructure as code, pipeline definitions, deployment manifests, and release scripts can all introduce risk. Review whether the change opens new network paths, broadens IAM permissions, disables scanning, or weakens controls in non-production environments that later get copied into production.

For CI/CD changes, check whether the pipeline still protects secrets, uses short-lived credentials where possible, and prevents untrusted pull requests from gaining access to privileged tokens. Review whether build steps are deterministic and whether artefacts are signed or verified before release. If the pull request changes container build logic, confirm that the image is built from a trusted base and that the final runtime image contains only what it needs.

Release gating and rollback also matter. If the change introduces a security control, ask how it will be rolled back safely if it breaks production. If the change removes a control, ask what compensating control exists during the transition. For teams working on pipeline hardening, our article on hardening GitHub Actions against pwn requests and token theft is a useful reference point.

A practical reviewer decision model

Not every concern needs the same response. A useful decision model is to classify findings as approve, request changes, or escalate. Approve means the issue is understood and the control is adequate. Request changes means the risk is real and the code should be revised before merge. Escalate means the reviewer has found a material issue that needs input from a security lead, architect, or product owner.

When documenting concerns, be specific. State the affected component, the risk, the likely abuse path, and the recommended fix. For example, instead of saying the code is insecure, say that the new endpoint lacks server-side tenant checks and could expose another customer’s records if the identifier is changed. That makes it easier for the author to act and for the team to learn from the issue.

It also helps to record recurring patterns. If the same type of issue appears repeatedly, turn it into a checklist item, lint rule, test, or pipeline gate. The goal is not to make reviewers carry every lesson in their heads forever. The goal is to move repeatable checks into automation and reserve human attention for the judgement calls.

Common mistakes teams make during secure code review

The most common mistake is over-relying on tools. Static analysis is valuable, but it will not tell you whether a workflow can be abused, whether a role check is in the wrong place, or whether a change breaks tenant isolation. Another common mistake is shallow approval habits, where reviewers focus on whether the code looks tidy rather than whether it is safe.

Teams also make the mistake of reviewing only the diff and missing the surrounding context. A small change can be risky if it sits on top of weak assumptions elsewhere in the system. Reviewers should understand the feature, the data flow, and the trust boundaries before deciding. If they do not, they should ask for a walkthrough rather than guessing.

Finally, teams sometimes treat security review as a specialist activity that only one person can do. That does not scale well. A better model is to train engineers on the recurring patterns, give them a clear checklist, and reserve specialist review for high-risk changes. That approach is more practical for SMEs and usually produces better coverage over time.

Used well, a secure code review checklist for pull requests improves both security and engineering quality. It helps teams catch issues earlier, reduce rework, and make security part of normal delivery rather than a separate last-minute gate. If you want help adapting this into a lightweight review process, or aligning it with your wider ISMS and development controls, speak to a consultant.

Frequently asked questions

How is secure code review different from functional review?

Functional review checks whether the code does what the feature is supposed to do. Secure code review checks whether it also does so safely, without weakening authentication, access control, input handling, logging, or deployment controls.

How much should be checked manually versus by automation?

Use automation for repeatable checks such as secret scanning, dependency scanning, and common vulnerability patterns. Use manual review for context-sensitive issues such as business logic abuse, tenant isolation, privilege boundaries, and whether a change alters the trust model.

Tags:

Comments are closed