Every UUID Is Guilty: Notes From the Second Path

Every UUID Is Guilty: Notes From the Second Path

I don't find authorization bugs by reading code carefully. I find them by asking who else can call this, and with what. Careful reading finds the path the author was thinking about. Authorization fails on the other one.

That's the whole finding of this review: multi-tenant authorization almost never fails because somebody forgot security. It fails because a hardened path exists — someone thought hard, wrote the check, left a comment explaining the trust boundary — and then a second path is added later that reaches the same data without passing through it. The second path is written by someone who read the comment and believed it.

One ternary, four criticals

The strongest evidence I have is a single line in a route. Paraphrased, so nobody has to go looking:

// route validates body.resource_id against the caller's org, then:
args: validatedId ? { ...body.args, resource_id: validatedId } : body.args

Read the failure. Supply a resource id at the top level and it gets validated against your org and stamped into the arguments object — the intended path. Omit the field, or supply one that fails validation, and the validated value is undefined, the ternary takes the else branch, and the caller's own arguments object is forwarded downstream verbatim. The guard isn't bypassed by attacking it. It's bypassed by declining to participate in it.

Four separate cross-tenant findings trace to that one expression. Not four bugs — one bug seen from four angles, because four downstream handlers each pulled a tenant-scoped id out of that object and trusted it. Two turned out worse than filed: one is a shared write boundary used by fifteen handlers, and one returns the harvested data straight back in the HTTP response rather than only laundering it through a model.

What converts "unvalidated parameter" into "critical" is that every one of those handlers held a service-role client. Row-level security — the one control that would have caught a foreign id on its way to the database — is off by construction there. An unvalidated id is a bug; an unvalidated id plus a client with no floor under it is a tenancy breach.

The bitter joke is that the route is good. It binds project to org, checks session ownership, caps free text, and its trust-boundary comments are correct. It validates the three parameters it knows about and never imagined those same ids arriving one level deeper, inside a bag of arguments it was only forwarding. Boundaries that enumerate parameters rot the moment somebody adds a field.

The second path always ships later

Once you're looking for it, the pattern is everywhere here, and it is never sloppiness. It's always a good decision that didn't propagate.

In one file, a fetcher carries an explicit org filter with a comment saying, in effect, service-role client, RLS won't do this for us. Its three siblings, same file, same kind of read, omit it. The author knew; the knowledge didn't survive the scroll distance.

A privacy flag is enforced in RLS, re-enforced by hand in the one admin-client loader whose author remembered why, and quietly forgotten in three others. A signing helper carries a header comment in capitals — call this only with server-trusted persisted values, never user input — and three months later a route hands it a string straight out of a request body. A hardened database function takes both a tenant id and an object id, asks whether the caller may act for the tenant, never asks whether the object belongs to it; a dedicated hardening pass re-read it and still only tightened the first question.

There's a rule buried in those. When a feature migrates from an RLS client to a service-role client, it must re-state by hand every predicate RLS was giving it for free: tenant, privacy flag, archival state, ownership. Nobody maintains that checklist, because it isn't a checklist anywhere — it's an understanding, and understandings don't survive refactors.

Prose is not a guard

The single most teachable artifact I read in three days was a comment inside one of the vulnerable handlers, asserting that the route had already validated org ownership.

It had. For a different id.

That comment is a load-bearing assumption written in prose. It cannot be executed, tested, linted or diffed. It was true when written, became false when someone added a field, and nothing noticed — prose has no truth value CI can read.

The database layer has the same disease in sharper form. Three times in one batch a policy comment promised something the SQL underneath did not deliver: owner or admin may delete over a policy any member satisfies; blocked until preflight passes, enforced in code over a column any member can flip directly. Policy names are worse than comments, because they look like structure. I now read the USING and WITH CHECK bodies first and the name second, and I'd tell anyone reviewing a Supabase-shaped app to do the same: the schema's policy lines are the real API contract and the route handlers are a convenience layer over them. The browser holds a public key and a second front door.

A security invariant that lives in a doc comment is a wish. Give the signing helper a bucket-allowlist parameter and the wish becomes a type.

What I got wrong

Roughly half of everything filed at the top two severities in this review did not survive refutation — mine included. That is not a footnote; it's the reason to trust the other half. A review with a hundred percent confirmation rate isn't rigorous, it's unrefuted.

I filed a stored XSS with high confidence: an applicant-supplied URL, unvalidated at intake while the field right beside it was validated, rendered as a link in a staff-facing view. Validated field next to unvalidated field is a genuine tell. The refuter killed it properly — downloaded the actual React DOM build, found sanitizeURL on the href branch in both renderers, and rendered my exact case to confirm React emits its blocked-URL stub instead of the payload. My mental model was several React majors out of date. A plain JSX href is not a sink, and I should have checked the framework instead of pattern-matching the shape.

What saves it from pure noise is that the chain wasn't empty — the real bug was one hop earlier and a different kind. That same attacker-controlled string is handed to a service-role signing helper whose own documentation forbids exactly that. Right chain, wrong sink, wrong class.

The second is less flattering. I found a route where a bare platform-admin flag skips a membership check before a service-role clone, and wrote it up as cross-org exfiltration. The clone lands in the source org; there is no cross-tenant copy. Worse, that admin flag already granted read at the database layer, so the route exposed nothing new. What's left is real but much smaller: a delegated admin using a coarse flag where the codebase's own convention demands a granular permission. I inflated a governance bug into a tenancy bug because tenancy was the shape I was hunting. That is the specific way a specialist reviewer fails, and knowing the failure mode doesn't stop you having it.

Two more of my near-misses were closed by migrations that shipped before the review started. Grade the live schema snapshot, not the migration file — read chronologically, a migration gives you a vulnerability from the past.

What I'd actually change

Not the four handlers. Patch those and the fifth one somebody writes next month is exposed the day it merges. Fix the boundary: validate or allowlist the forwarded argument bag at the route, and make an absent validated value a rejection rather than a fallthrough.

Then convert the lessons into things that fail loudly:

• Every service-role query carries its own tenant predicate. Not a comment justifying the bypass — a predicate. Lintable.
• Every FOR UPDATE policy carries a WITH CHECK. USING alone authorizes who you are and never re-checks the row you produce. Greppable in CI.
• Enqueue paths may only name handlers from a static list, so a caller-supplied action name can't re-enter the whole handler surface under service role.
• Audit function grants by listing grants, not by reading the last migration that mentioned the function. Revoking from PUBLIC alone silently leaves other roles holding execute.

And the sentence I'd hang on the wall, because nearly every failure here answered half of it: authorization is two questions. May this caller act for this tenant, and is this object in that tenant? Findings pile up on the second because the first feels like it finished the job.

Every UUID is guilty until an ownership check is proven, and "the caller already validated this" is not a proof — it's a rumour. This team writes genuinely good defensive code; several objects I read were more carefully threat-modelled than anything I expected. The holes aren't in the pieces. They're in the seams between them, which is exactly where nobody owns the invariant.