The Code Review Checklist That Actually Works on WordPress Diffs

A pull request lands with sixty lines in it. The author added a nonce check, escaped the output with esc_html(), and noted that they read the security handbook. The review takes four minutes and passes. Three weeks later the site shows a message on a page where it was never meant to appear.

Nothing in that diff looked wrong, which is the problem. WordPress is generous with ways to reach the database, the request and the browser, and those paths look tidy in a diff. A reviewer reading for style passes the change that causes the incident.

What follows is the order I use, and it is not a security audit.

Why a WordPress diff can look clean and still be unsafe

Most review advice assumes a routed application: one entry point, a middleware stack, an ORM that parameterises by default, a template layer that escapes by default.

WordPress gives you none of those defaults. It gives you $_GET and $_POST as raw superglobals, $wpdb as a handle you can interpolate strings into, admin-ajax.php as an endpoint any registered action can attach to, and echo as the normal way to produce output. Each is a reasonable decision and each is an escape hatch. A diff that uses all four can still be clean and still be unsafe.

Start at the boundary: every input sanitised, every output escaped

These are two checks, not one, and they happen in different places. Sanitisation belongs where the request enters your code. Escaping belongs where the value leaves it, at the point of rendering.

What you are looking for in the diff is the distance between the two. A handler that reads $_POST['note'] and passes it through sanitize_text_field() before storing it is fine. The same value coming back out of the database six functions later has lost that guarantee, because it may have been written by an import, a WP-CLI command, a REST call, or a previous version with no sanitiser at all. Stored data is untrusted data. A review that checks only the write path has checked half the problem.

The failure to look for is esc_html() used where the value lands in an attribute, or esc_attr() where it lands in text. Both render, both look like escaping, and only one is right for the context. A diff that escapes at all is usually a diff where somebody thought about it; the question is whether they matched the function to the sink.

A nonce is not a capability check

The most common finding in WordPress review is a new admin_post_ or wp_ajax_ handler with a nonce and no capability check. The nonce does one thing: it proves the request came from a page your site rendered to that user. It says nothing about whether that user is allowed to act. A subscriber's session can produce a valid nonce for any action a page renders to them.

So on every new entry point, ask two questions. How is this handler registered, and which capability does the operation require? Changing a setting needs manage_options or a narrower capability. Editing another user's data needs edit_user with the target user ID, not a blanket read. The check has to be in the handler, before the first write. Registering the callback with a capability argument is not a check: that flag controls whether the handler is offered, not whether it can be reached.

REST controllers deserve their own sentence. Look for the permission_callback argument on the route and read what it returns. A route that returns true is public by design, and the description should say so. If it does not, that is the finding.

Every query prepared, and no format string you cannot read

Any call that puts a variable into SQL has to pass through $wpdb->prepare(). There is no useful judgement call here, and the review is mechanical: find the query, find the placeholders, find the argument list. The pattern that keeps appearing is a query built by concatenating a table name or a column name. Placeholders bind values, not identifiers. When a diff concatenates a table name, trace where that name came from. If it came from user input, the change is not fixable by adding a placeholder.

While you are in the query, check the table name. A plugin that writes to wp_posts rather than $wpdb->posts will work on every default install and fail on every install with a custom prefix, which is a support ticket you cannot reproduce locally. It is a one-word fix and it is worth flagging every time.

Filter names, load order, and hooks you do not control

Three smaller checks, each of which takes seconds and each of which causes a puzzling bug class later.

Filter and action names. A new filter called product_price_before_tax will collide with another plugin eventually, and the collision shows up as a value that changes only on sites running both plugins. Prefix everything with the plugin slug. Naming is not style.

What loads where. A stylesheet or script enqueued with no conditional loads on every page of the site, including the ones that never render the component. The same goes for a class instantiated on plugins_loaded that reads an option and registers fifteen hooks to support one admin screen. If the change only matters on an order confirmation page, ask why it is not hooked to something narrower. This is where most plugin performance regressions enter the codebase.

Hook ordering. A callback on init at the default priority that assumes another plugin has already registered its post type is a race the author does not control. Ask what priority the change runs at and what it depends on. If the answer is "whatever the other plugin does", the change is fragile no matter how correct the logic inside it is.

The patterns that catch the most: read backwards, then ask three questions

Reading a diff forwards means following the author's reasoning, and that reasoning contains the assumption you are trying to find. Read it backwards instead. Start at the sink: the echo, the $wpdb->query(), the update_option(). Then walk up the call chain asking where that value came from and what has happened to it since. That direction matches the way data travels, which is why it finds what the forward read misses.

Then three questions.

What happens on a site where the new option does not exist yet? A plugin that installs a default on activation and calls get_option( 'myplugin_settings' )['currency'] in a render path breaks on every site that updated rather than installed. Ask for the default in the call itself. It costs one argument.

What happens when the external call times out? A synchronous wp_remote_get() in a checkout path with no timeout argument inherits a default longer than most people expect, and a request that waits on it holds a PHP worker. The question is not whether the code handles the error, because many of these calls fail silently and correctly. It is whether the failure is safe: does the page still render, and does the order still complete?

Is the change reversible? If it ships and gets reverted, what is already in the database? A change that writes a new option can be reverted cleanly. One that renames a column cannot, and neither can one that transforms data on the first request after deploy. Ask what the rollback actually restores. If the answer is nothing, the migration needs to be additive or the release needs a tested restore path.

One command that finds the diffs worth reading closely

Before the review starts, count the unescaped outputs in the file you were handed. This is a static scan, not a security audit: a nonzero number is a reason to read closely, not a finding.

# echo statements, against those with an escaping call on the same line
grep -c 'echo ' wp-content/plugins/my-plugin/includes/*.php
grep -c 'echo esc_\|echo wp_kses' wp-content/plugins/my-plugin/includes/*.php

When the first number is 40 and the second is 3, read that file line by line before you approve anything else, whatever the description says.

What makes a review useless

Three things, all of them process problems that a longer checklist will not fix.

A diff over a thousand lines. Nobody reviews a thousand lines carefully, and a review that produces one comment on a thousand-line diff is a rubber stamp with extra steps. Split the change, or review it commit by commit, or accept that you are approving rather than reviewing.

A reviewer who only reads the description. The description tells you what the author believes they changed, which is the information you do not need, because the bug is in the part they did not think about.

"Looks good to me" on a change to authentication or capability logic. Those paths get a second reader who did not write the change, a written note about what happens when the new branch is wrong, and a test that exercises the denial case rather than the happy path.

What a diff usually means when it smells

Smell in the diff	What it usually means
A new wp_ajax_ or admin_post_ handler	There is a new entry point and it needs a capability check before anything else in the section.
esc_html() inside an attribute	The author escaped by habit rather than by context. Check every other sink in the file.
A table name written as a literal	The change will break on any install with a custom prefix. Unreproducible locally.
An enqueue call with no condition	The asset now loads site-wide, including pages that never use it.
A direct get_option() with no default	Breaks on upgraded installs and on rows a human deleted. Ask for the default argument.
An unbounded loop over get_posts() or a query with no LIMIT	Fine on the author's dataset, slow on the customer's. Ask what row count it was tested against.
A remove_action() on somebody else's hook	Ordering dependency. It runs only if the other plugin registered first, which is not guaranteed.
A new filter name with no prefix	Collision waiting for a second plugin. A mystery cost that takes two seconds to prevent.

Where the checklist stops, and what to do on Monday

Every item above encodes a mistake I have seen reach production, which is the limit of the method. A checklist surfaces the classes of problem you already know. The bugs that take a site down are the ones nobody wrote a line for: a race between two requests, an assumption about how a third-party endpoint responds under load, a data shape that exists on one customer's install. No list contains the item that has not happened to you yet.

That is why the test matters more than the list. A review confirms that the code does what the author intended. A test confirms what the code does when the intent is wrong, and it keeps confirming it next year, after the reviewer has moved on. A test that fails when I revert the change is worth more than every comment above, because it is the only part of the review that outlives the review.

Concretely, this week:

1. Count the echo statements in your three largest plugin files that no escaping function wraps. That number is your baseline.
2. Add the capability question to your PR template as a required line: which capability does this operation require, and where is it checked?
3. On the next diff under two hundred lines, read it backwards from the sink, then ask the three questions above.
4. Require one test on any change to capability logic, session handling or authentication, and make it assert the denial case.