Absent and unknown are not the same answer

Ask WordPress whether a row exists and you can get back null. Ask it again when the database is refusing connections and you get back null again. Those are two completely different facts sharing one value, and code that decides something important from that value cannot tell them apart.

$wpdb->get_var() returns null for “no such row” and null for “the query failed”. get_post_meta() returns '' for “the key is empty” and '' for “the read did not happen”. An empty $wpdb->last_error means “no error” and also means “the query never ran”. In each pair, the first is a fact about your site and the second is a fact about the database, and the language gives you one word for both.

Version 1.7.5 carries nine consecutive rounds of review against this one shape. Nine, because fixing the places it turned up never closed the habit that produced it. This post is about the shape rather than the list, and about the one instance where it pointed in the dangerous direction.

The worst instance: a failed read that said yes

The plugin asks, before it authorizes an agent, whether you have already consented to this client. The answer decides whether you see the consent screen or whether the flow proceeds straight to minting tokens.

That lookup could fail. Not “find nothing”. Fail, as in the query did not complete. And the code that read the result could not distinguish “this user has never consented to this client” from “I could not find out”. It took the value at face value and read it as consent present.

So with a valid PKCE challenge in hand, the authorize flow would skip the consent screen and mint tokens for a user and client pair that had never been approved. That was reproduced live, returning true for a pairing that did not exist.

This is the direction that matters. A governance control that fails closed is an outage: annoying, visible, and it protects you while it is broken. A governance control that fails open is not an outage at all. Nothing looks wrong. The screen you were supposed to see simply does not appear, and the thing it was guarding happens anyway.

A second instance in the same family: the delete-on-uninstall flag. A failed read of it could come back truthy, which authorizes removal of your configuration, your audit data, and the OAuth tables. Same shape, same direction, different blast radius.

The switch that came back on

Here is one you might actually have hit, and it is the reason the changelog line “a stuck OAuth migration can no longer re-enable a switch you disabled” is not a small note.

When the OAuth toggle moved to its own stored setting, installs that predated the change needed their prior behaviour preserved, so a one-time migration runs on update: if the setting row is absent, write it on, because that matches what the site was doing before. Absent means never migrated. Perfectly reasonable.

But a read that fails also looks absent. Both collapse to “not there”. So on a site where an administrator had deliberately switched OAuth off, a transient read failure at the wrong moment made the migration conclude the row had never existed, and it wrote the setting back to on.

That is an update silently widening access, which is the one thing this plugin says it does not do. It is fixed in 1.7.5: the migration now aborts on any read error instead of deciding from a state it does not know, and tries again on the next request. But it was live in 1.7.2, 1.7.3 and 1.7.4, and it belongs in the open rather than in a one-line changelog bullet.

The fix is to abort on any database error instead of deciding from an unknown state. That is the pattern for the whole release. The bug was never a missing branch or a wrong comparison, it was a design that let an unanswered question and a negative answer arrive in the same variable.

Revocation that reported a success it could not back

The other fix worth its own paragraph is refresh-token reuse detection. Replaying a consumed refresh token is a sign the token has been stolen, so the response is to revoke the entire lineage: the token, its parent, its children, everything.

The walk collected the chain and then deactivated it all in one update at the end. Between the last read and that update there is a gap, and a rotation happening concurrently could mint a successor inside it. That successor was not in the collected list, so it survived, still valid, while the function returned success.

Two fixes shipped for this before the right one. A post-update re-check. Then a five-pass loop that repeated until a pass converged. Both were verified by execution against a real concurrent rotation. Both were still wrong, for the same reason: all three shapes collected identifiers first and deactivated at the end, so convergence was being declared before anything was actually deactivated. A successor minted after the last stable read is invisible no matter how many times you repeat the read.

The third fix stopped patching and inverted the algorithm. The walk now deactivates each node before reading that node’s children, with an unconditional update. The row lock InnoDB takes on that update does the synchronizing, so there are only two orderings and both are safe: either the revocation commits first and the concurrent rotation’s own minting gate matches zero rows and rolls back, or the rotation holds the lock and the revocation waits for it, in which case the successor is committed and visible to the very next read.

It was finally proven with a two-process harness holding a real row lock mid-transaction, and, the part that makes it a proof rather than a hope, the harness was first shown to reproduce the bug against the old code.

There is a governance point buried in that. The function now returns false if any part of the walk could not be completed, and every node it did reach stays deactivated. Partial revocation is strictly better than none, but the caller has to be told that is what it got. A caller that knows the revocation was incomplete can retry. A caller told the chain is revoked has no reason to, which is why reporting a completed revocation you cannot back is worse than admitting a failed one.

A fix that created the next bug

Round seven fixed a stale read in the reaper that deletes abandoned OAuth clients. Round eight found what round seven had left behind: the candidates were scanned, then deleted, with no re-check in between. A client approved inside that window got deleted anyway, orphaning a token issued seconds earlier.

Read-then-mutate is a shape that keeps producing bugs rather than a single bug. The fix moves the abandonment test inside the delete’s own WHERE clause so the database decides atomically, instead of acting on a conclusion that was true when it was drawn and is not any more.

What this means if you are running the plugin

Most of 1.7.5 is invisible until it bites, but one change you may notice: a number of paths moved from failing open to failing closed. An integration that previously received a success on a read that had actually failed now receives an error instead.

That is correct, and an upgrade sweep confirmed none of the new failure branches fire under healthy conditions. But if an agent starts reporting an error on an operation that used to succeed, the honest reading is usually that the operation was failing silently before and you are only now being told.

No default changed in this release. No permission, capability, or ability changed, and the catalog is the same 179 it was in 1.7.4. Read-only mode, the high-risk lock, allowlist and scoping semantics, and what the Activity Log records are all untouched. What changed is whether the plugin tells you the truth about what it did.

That is the whole theme, and it is the same one behind the 1.7.3 object-cache hotfix, where the Activity Log recorded a change as done that had not happened. The practical stake is whether you have to check the agent’s work by hand. If the plugin reports optimistically you do, and at that point the automation has not saved you anything.

Full release history is on the changelog, and the current controls are on security.