Proposal: detect read policies whose USING is unconditionally true for unauthenticated sessions (inverted auth.* IS NULL disjunct)

Open
#165 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
5/5
Estimated time
Over a week
Newbie friendliness
45/100
Issue type
Feature
Clarity
Mostly clear
Activity status
Quiet
Tech stack
postgres

Research direction

Start with the /new-lint skill and the existing 0024_rls_policy_always_true lint, then review the standard lint contract and pg_regress test layout. Confirm the fresh lint number and WARN/INFO choice before proceeding. Done means the proposed true positive is detected and the listed negatives—current_user, AND-gated IS NULL, a scoped predicate, and current_setting(name, false)—remain clear in regression tests.

Written by the indexing model from the issue text.

Description

Hi! A suggestion for a new Security lint, related to but framed differently from PR #28 ("feat: add lint to detect RLS policies that allow access to anonymous users").

The gap

0024_rls_policy_always_true deliberately scopes its always-true USING check to command in ('UPDATE','DELETE','ALL') and matches only the literal forms ('true','(true)','1=1','(1=1)') — with the comment that SELECT … USING (true) is often intentional. That's the right call for literal true. But a very common real-world shape slips through entirely:

create policy "read own"
  on public.documents for select to authenticated
  using ( auth.uid() is null or owner_id = auth.uid() );

For any request where auth.uid() is NULL (an anonymous session, or any token-less call), auth.uid() is null is true, so the whole USING is true and every row is readable. It isn't literal-true, and it's a SELECT, so 0024 can't see it — yet it exposes the table.

Why this isn't the PR #28 discussion

This lint does not flag "the policy is reachable by anonymous users" — that's a deliberate design choice and not inherently a bug (which, as I read it, was the substance of the pushback on PR #28). It flags a predicate whose top-level OR short-circuits to true for any session where the auth function returns NULL — a logic error that is wrong whether or not anonymous sign-ins are enabled. (Which sessions those are depends on the policy's roles: for a public/anon policy it's any token-less request; for a TO authenticated policy it's the narrower case of an authenticated token with a NULL sub.) An … IS NULL OR … disjunct on an auth function almost always means the author intended the negation (IS NOT NULL AND …) or a scoped check, and accidentally turned the policy into a no-op. Framing it as a correctness defect (rather than an anonymous-access judgement) keeps it squarely in advisor territory.

Proposed detection

A pure SQL view over pg_policies, standard lint contract. It fires on a predicate shape, not on any project setting, so it needs no conditional execution:

  • permissive policy, cmd in ('SELECT','ALL'), granted to anon / authenticated / public;
  • normalized qual contains a top-level OR disjunct of the form <auth fn>() is null, where <auth fn> is genuinely nullable — auth.uid() / auth.jwt() / auth.role(), or current_setting('…', true).
  • Exclusions, to stay false-positive-clean: never-NULL identity functions (current_user / session_user / current_role / user) — those disjuncts are constant-false dead branches, not leaks; and current_setting(name) / current_setting(name, false), which raise rather than return NULL.
  • Severity: WARN/INFO; remediation: "remove the IS NULL disjunct, or replace it with an explicit deny."

A couple of questions before a PR

  1. Would you prefer a fresh lint number, or to revive/rework PR #28?
  2. Severity preference — WARN or INFO?

Happy to implement it with the /new-lint skill + pg_regress tests covering the true positive above plus negatives (current_user is null disjunct, AND-gated is null, a plain scoped predicate, current_setting(name, false) is null).

Dominant language
PLpgSQL
Stars
275
Forks
102
Avg merge
2d 8h
Merged PRs (30d)
3

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from supabase/splinter

All issues in supabase/splinter

Similar issues

More Databases issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.