Skip to content

[security] Close the SECURITY DEFINER PUBLIC-grant class (closes #25) - #41

Merged
Harrolee merged 1 commit into
mainfrom
security/grant-rls-sweep
Aug 16, 2026
Merged

Harrolee merged 1 commit into
mainfrom
security/grant-rls-sweep

Conversation

@Harrolee

Copy link
Copy Markdown
Owner

Closes #25.

Not another instance — the class. Postgres grants EXECUTE on every new function to PUBLIC by default, so the GRANT ... TO service_role sitting next to a SECURITY DEFINER function excluded nobody.

What was actually reachable

Verified on a from-scratch stack over PostgREST using only the anon key — the key that ships in the web bundle. Every one of these returned 200:

Function Result as anon Severity
due_coach_nudges(int) Returned another member's user_id, display_name, coach name and discipline. No auth.uid() filter exists in it at all — it was written as a service_role dispatcher query. High. Discloses that a named person is being coached, and on what.
consume_free_message(uuid,uuid) Incremented messages_used three times over three unauthenticated calls, exhausting a member's free tier. Medium. Unauthenticated write.
has_coach_access(uuid,uuid) Returned true for an arbitrary member id — and the id needed to drive it leaks from due_coach_nudges. Medium. Entitlement oracle.
check_subscription_access, get_trial_days_remaining, can_publish_coaches Answered for an arbitrary email. Low–medium. Enumeration oracle.
create_user_with_trial, get_or_create_user Executed as anon and reached their INSERT. Medium.

Functions whose bodies are auth.uid()-scoped (release_push_device and friends) were callable but no-opped — release_push_device returned 204 and changed nothing. Revoked anyway: "the body happens to check" is not an access control.

All nine now return 42501. get_coach_roster still works logged-out.

Approach

Revokes are computed from the catalog, not a hand-written list, so a function added by a later migration that forgets its own REVOKE is still covered on the next deploy. Grants are then re-applied deliberately.

security_definer_grant_audit() returns any SECURITY DEFINER function still reachable by PUBLIC/anon, minus a two-entry documented allowlist. rls-probe.mjs asserts it is empty, so the next one to forget fails the probes instead of shipping.

has_coach_access also short-circuits when auth.uid() is set and doesn't match p_user_id. Its entitlement logic is otherwise unchanged — the free-tier metering, trialing/grace_period statuses and creator-owns-their-coach branches are all preserved. (My first draft of this rewrite dropped all three; caught before it applied.)

The four inherited probe failures

main was at 44/4, all four failing with permission denied for table user_profiles on the coach detail query.

Root cause: four legacy phone-based policies on coach_profiles that subquery user_profiles. An RLS predicate is evaluated as the invoking role, so their existence forced every reader of coach_profiles — including anon — to hold SELECT on user_profiles.

The tempting fix, granting anon SELECT on user_profiles, would have published every member's email, phone, timezone and reference photo URL. Dropped the policies instead; they're redundant with the uid/email policies the multi-domain work added.

Verification

  • rls-probe.mjs 44/4 → 77/0, with 29 new negative assertions: per-function 42501 checks, per-table anon denials, creator_profiles payout columns unreadable by anon, and cross-member entitlement probing blocked.
  • Other suites unchanged and green: flow 46/0, viz-realtime 33/0, sms-image 55/0, account-deletion 64/0. 275 checks total.
  • Migration is idempotent (re-applies clean) and the full chain cold-applies against supabase-shim.sql with ON_ERROR_STOP=1.
  • tsc --noEmit clean, node --check clean on every function, terraform validate passes.

Note for the next agent

viz-realtime-probe.mjs is flaky — it failed 29/4 on the first run and passed 33/0 on a re-run with no changes in between. The failures are all "the message arrived over realtime — received 0", i.e. the websocket subscription hadn't established before the assertion. Worth a retry/wait rather than an hour of debugging.

docs/grant-matrix.md is the reviewable per-role-per-object table #25 asks for.

🤖 Generated with Claude Code

Postgres grants EXECUTE on every new function to PUBLIC by default, so the
`GRANT ... TO service_role` sitting next to a SECURITY DEFINER function
excluded nobody. Verified on a from-scratch stack with only the anon key --
the key that ships in the web bundle:

  * due_coach_nudges(int) returned another member's user_id, display_name,
    coach name and discipline. It has no auth.uid() filter at all; it was
    written as a service_role dispatcher query. This one discloses that a
    named person is being coached, and on what.
  * consume_free_message() incremented messages_used three times over three
    unauthenticated calls, exhausting a member's free tier.
  * has_coach_access() answered for an arbitrary member id -- and the id it
    needs leaks from due_coach_nudges.
  * check_subscription_access / get_trial_days_remaining / can_publish_coaches
    answered for an arbitrary email.

All nine now return 42501. The logged-out roster still works.

Revokes are computed from the catalog rather than a hand-written list, so a
function added later without its own REVOKE is still covered. Grants are then
re-applied deliberately; docs/grant-matrix.md states the intent per role per
object.

has_coach_access additionally short-circuits when auth.uid() is set and does
not match p_user_id, so a signed-in member cannot probe anyone else. Its
entitlement logic is otherwise byte-for-byte the original -- the free-tier
metering, trialing/grace_period statuses and creator-owns-their-coach branches
all matter and are preserved.

Also drops four legacy phone-based policies on coach_profiles. An RLS predicate
runs as the invoking role, so their subquery into user_profiles forced every
reader of coach_profiles -- including anon -- to hold SELECT on user_profiles.
That is the root cause of the four rls-probe failures inherited on main, and
the tempting fix would have exposed every member's email and phone to the
internet. They are redundant with the uid/email policies.

rls-probe.mjs goes 44/4 -> 77/0, with 29 negative assertions including a drift
detector that fails if a future SECURITY DEFINER function is left PUBLIC.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Harrolee
Harrolee merged commit 286c03e into main Aug 16, 2026
1 check passed
@Harrolee
Harrolee deleted the security/grant-rls-sweep branch August 16, 2026 19:53
@Harrolee
Harrolee restored the security/grant-rls-sweep branch August 16, 2026 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[security] Systemic grant/RLS gaps — four independent agents hit the same class of bug

1 participant