Here is a Firestore rule that shipped to production and stayed there for months:
match /devtracker_posts/{postId} {
allow read: if true;
allow create, delete: if request.auth != null;
allow update: if request.auth != null ||
request.resource.data.diff(resource.data).affectedKeys()
.hasOnly(['likes_count', 'views_count']);
}
Read it quickly and the intent is clear. Signed in users can update posts. Anonymous visitors can increment the like and view counters and nothing else.
Read it again.
The pipe
|| short circuits. If the left side is true, the right side never evaluates.
So for any signed in user, the rule reduces to allow update: if true. The field guard, the entire point of the second clause, is unreachable. Any authenticated user in the project could rewrite the body of any post in a public feed.
The author's intent was almost certainly two separate permissions. What they wrote was one permission with a decorative second half.
Why nothing caught it
This is the part worth dwelling on, because the bug itself is a one character fix.
It compiles. The rule is syntactically valid and semantically meaningful. There is nothing for a linter to object to.
It reads correctly. Both clauses are present and both describe real intentions. Skimming it, you see "signed in users" and "only these fields" and your brain assembles the rule you expected to find.
Nothing fails. Every legitimate operation succeeds. The counter increments work. The authenticated updates work. There is no error, no log line, no failing test. The rule does everything it was supposed to do, plus one thing it was not.
Testing it requires adversarial thinking. A test that checks "can a signed in user update a post" passes. To catch this you have to write "can a signed in user update a post they do not own", which means suspecting the bug before you look for it.
The correct shape
Two branches, each self contained:
allow update: if
(
signedIn() && (
resource.data.get('author_uid', '') == request.auth.uid || isSiteAdmin()
)
) || (
request.resource.data.diff(resource.data).affectedKeys()
.hasOnly(['likes_count', 'views_count', 'comments_count'])
);
Now each branch grants a complete, bounded permission. The author or an admin may edit content. Anyone may bump the counters. Neither branch swallows the other.
The migration detail
resource.data.get('author_uid', '') is doing quiet work there.
The author_uid field did not exist when most of those documents were written. A rule reading resource.data.author_uid outright would error on every historical document, which in Firestore means denied. Every old post would become uneditable, including by an admin.
The two argument get returns a default when the field is absent, so old documents fall through to the admin check instead of erroring. They become admin only rather than broken.
That is a deliberate tradeoff and worth stating plainly rather than discovering later: users can no longer delete their own pre migration posts. Fixing that properly needs a backfill.
Two more in the same file
The same review found two others, both in one block:
match /devtracker_whitelist/{userId} {
allow read: if true;
allow write: if request.auth != null;
}
The comments alongside said "app needs to check whitelist on sign-in" and "only admin can manage". Both comments describe reasonable intentions. Neither matches the code.
allow read: if true made every allowed user's email address and GitHub identity readable by anyone on the internet. allow write: if request.auth != null meant any signed in user in the project could add themselves to the allowlist, which is the definition of an access control that does not control access.
The second one is worse than it first appears, because the project hosts more than one application. Any user of any app sharing that Firebase project qualified.
What I would take from this
Security rules are code, and they are code with an unusual property: the failure mode is silent and the success path is indistinguishable from the broken one.
Three things follow.
Comments in a rules file describe intent, not behaviour. When they disagree with the code, the code wins and nobody notices, because the comment is what people read.
Boolean operators in an access rule deserve more suspicion than anywhere else in a codebase. || between a broad condition and a narrow one almost always means the narrow one is dead.
And the test you need is the adversarial one. Not "can the right person do this" but "can the wrong person do this". The first is what you write naturally. The second is the one that finds bugs.