Security Issues Worth Blocking a Merge

A pull request can be green across every test and still deserve to die. That happens more often than most engineering teams care to admit, and the reason is usually the same: the tests were never built to notice the thing that is wrong. Security review at merge time is not a scanner running in the corner of a dashboard. It is a judgment call, made by a person, about whether this specific change makes the system harder to break than it was yesterday.

The changes worth blocking are not usually dramatic. Nobody merges a backdoor. What gets merged is a small convenience, a shortcut taken under deadline, a logging line added for debugging and forgotten. The security posture of a codebase erodes one reasonable-looking commit at a time.

A New Endpoint That Skips the Authorization Check

The most common shape of a merge-blocking flaw is a route handler that authenticates the caller and then forgets to ask whether that caller is allowed to do the thing. Authentication answers who someone is. Authorization answers what they may touch. A new API endpoint added under deadline pressure frequently gets the first and drops the second.

This is worth blocking because it is invisible in normal testing. The developer's own account is an admin, so every manual check passes. The test suite uses a fixture with full permissions. The hole only appears when a low-privilege account calls the endpoint with someone else's record ID, and nothing in the pipeline ever tries that.

A reviewer looking at a diff that adds a new route should be able to point at the exact line where ownership or role is checked. If that line is not there, the merge waits.

Secrets That Arrive in the Diff

An API key pasted into a config file, a database password hardcoded in a migration script, a private key committed because the local environment needed it for five minutes. These show up in pull requests constantly, and the authors are almost never careless people. They are people working fast in a repository that has no documented way to inject a secret at runtime.

The block here is non-negotiable, and the reason is that git does not forget. Deleting the line in a follow-up commit leaves the string in the history, where it will sit until someone rewrites the branch or the credential is rotated. A four-second mistake becomes an afternoon of cleanup. Rotating the key is the real fix; removing it from the diff is the minimum.

String Concatenation Reaching a Query or a Shell

Injection flaws have been documented for decades, which makes their continued appearance in pull requests more striking, not less. The pattern is almost always the same: a developer builds a query or a command by gluing user input into a string. It works. It reads cleanly. It is wrong.

A reviewer who sees a query assembled with string formatting instead of parameter binding has grounds to block regardless of whether the input is currently validated. Validation at the edge is a good practice and a fragile one; the next developer adds a second call path and forgets. Parameterization at the query itself holds up under refactoring. Shell commands are the same story with higher stakes, because a subprocess call built from concatenated arguments can execute anything the service account can reach.

Deserialization of Untrusted Input

Accepting a serialized object from a client and reconstructing it on the server is a merge-blocking pattern in most languages, and it is worth understanding why rather than memorizing a rule. Deserializers are designed to rebuild arbitrary structures. When the input is attacker-controlled, that flexibility becomes an attack surface, and in several ecosystems it has been the entry point for remote code execution.

The safe alternative is usually boring: accept a narrow schema, validate field by field, construct the object explicitly. It is more code and it is harder to get wrong. A reviewer should block a diff that pipes request data straight into a general-purpose deserializer, and should do so even when the payload currently comes from an internal service, because internal services get exposed eventually.

A Dependency Added Without a Reason in the Description

Every new package is a set of strangers' code running with the application's privileges, plus a supply chain that extends to whoever maintains it. A pull request that adds a dependency and says nothing about why is asking the team to trust an unknown author for an unclear gain.

This does not mean every dependency is suspect. It means the diff should answer three questions: what does this package do, why is the standard library or an existing dependency insufficient, and who maintains it. A single utility function that could be twelve lines of local code is often not worth a permanent supply chain link. The block is not about the package; it is about the missing justification.

Logging That Captures More Than It Should

Debug logging added during an incident has a way of surviving into production. A line that dumps a full request body, including a session token or a password field, turns every log aggregation service into a credential store. Logs are typically retained longer, replicated wider, and accessed by more people than the database itself.

This is a legitimate block when the diff logs an object that plainly contains sensitive fields. The fix is small: log an identifier, redact the payload, or log at a level that stays off in production. Blocking here is cheap, and the alternative is a conversation months later about whether a token was ever exposed.

Disabled Certificate Verification

Somewhere in the diff there is a client configured with verification turned off, and the commit message says the staging certificate is self-signed. This is understandable and it is still a block. The switch that solves a local annoyance has a habit of shipping, and once verification is off, every connection the client makes is open to interception by anyone on the network path.

The right fix belongs in configuration, not in code: trust the staging certificate through an environment-specific setting that cannot follow the build into production. A hardcoded bypass is a decision the reviewer should refuse on the spot.

The Check That Lives Only in the Client

Client-side validation is a courtesy to the user. It is not a security control, because the client belongs to the attacker. A diff that adds a permission check in the frontend and calls the feature protected has added nothing to the server's defenses.

Reviewers catch this by following the request. If the UI hides a button for non-admins, what happens when a non-admin sends the underlying request directly? If the answer is that the server processes it, the change is not finished. The enforcement has to live on the server, and the pull request should show it there.

Error Messages That Describe the Inside of the System

A stack trace returned to the browser, a database error surfaced verbatim, a message that distinguishes "no such user" from "wrong password" on a login form. Each of these hands an attacker a piece of the map: framework versions, table names, query structure, or confirmation that a username exists.

Blocking on error handling feels pedantic until the enumeration attack arrives. The login form that says "wrong password" for a real account and "no such user" for a fake one is a free username oracle. The fix is a generic message to the client and the detail written to the server log where it belongs.

What a Reviewer Owes the Diff

None of these blocks require a security team, a threat model, or a meeting. They require a reviewer who reads the diff as a stranger would, asking what this change lets someone do that they could not do before. That question takes about ninety seconds per pull request and catches most of the list above.

The harder part is cultural. Blocking a merge costs a colleague time, and reviewers learn quickly which objections are welcome and which are treated as obstruction. A team that wants security at merge time has to make the block ordinary, expected, and free of blame. The reviewer who flags the missing authorization check is doing the author a favor, and the author who accepts it without defensiveness is doing the same for everyone downstream.

The pull request that gets blocked today is the incident that does not get written up next quarter. That trade is worth making, one diff at a time.