Repository navigation
Lock down Postgres grants and add an RLS audit before going public - #21
Merged
Merged
Conversation
The repo is about to be made public, which makes two gaps worth closing
first.
CLAUDE.md gains a "Secrets" section, placed ahead of everything else: do
not read local.properties or .env, and never publish their contents into
a commit, PR body, log line or message. Nothing previously forbade it,
and once the repo is public that is a one-way mistake.
On the database side, Supabase ships a standing `alter default
privileges ... grant all on tables to anon, authenticated`. The
point-in-time `revoke all on all tables` in 20260809000000 did not cover
tables created later, so the venue tables have been carrying
INSERT/UPDATE/DELETE grants to anon, with only RLS-and-no-write-policy
stopping the writes. 20260815000200 retracts those default privileges
for both the current role and postgres, re-tightens the venue tables to
SELECT, and retrofits the per-function revokes that set_updated_at and
get_venue_map never had.
Two details are worth knowing, and are documented in the migration and
the README rather than assumed:
* The stock default ACLs exist for supabase_admin as well as postgres.
The former is deliberately untouched — postgres is not a member of
that role, and default privileges follow the creating role, so it
cannot apply to objects these migrations create.
* `alter default privileges` cannot retract PUBLIC's built-in EXECUTE
on new functions; it is merged additively onto the built-in default,
so the function still comes out with proacl null. Verified on PG
17.6. Every new function therefore needs its own `revoke all ...
from public`, and that is the one rule the schema cannot enforce for
us.
supabase/checks/rls-audit.sql makes the invariants checkable instead of
assumed, via `make rls-audit` — RLS-disabled tables, policy-less tables,
grants beyond SELECT, unpinned search_path on security definer
functions, the functions anon can execute, and residual default ACLs.
The connection string is read from `supabase status` at runtime so no
credential lands in the repo.
Verified against a local stack: db reset, both seeds, all must-be-empty
sections empty, section 6 listing only get_venue_map. Over PostgREST as
anon, reads succeed and INSERT/DELETE/import_venue are refused with
42501 at the grant layer. SupabaseIntegrationTest ran for real (3 tests,
0 skipped, 0 failures); make check and make ci pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Section 7 lists supabase_admin rows that are expected, so it is an inventory to read, not a must-be-empty check like 1, 2 and 4. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two pre-publication gaps. First,
CLAUDE.mdgains a Secrets section, placed ahead of everything else: never readlocal.properties/.env, and never publish their contents into a commit, PR body, log line or message — nothing previously forbade it, and once the repo is public that is one-way. Second, Supabase's standingalter default privileges … grant all on tables to anon, authenticatedmeans the point-in-timerevoke all on all tablesin20260809000000never covered the venue tables, which have been carrying INSERT/UPDATE/DELETE grants toanonwith only RLS-and-no-write-policy stopping the writes;20260815000200retracts those defaults, re-tightens the venue tables to SELECT, and retrofits the per-function revokes thatset_updated_atandget_venue_mapnever had.Two findings are documented in the migration and README rather than assumed: the stock default ACLs exist for
supabase_adminas well aspostgres(the former is deliberately untouched —postgresis not a member of it, and default privileges follow the creating role), andalter default privilegescannot retract PUBLIC's built-inEXECUTEon new functions because it is merged additively onto the built-in default (verified on PG 17.6), so every new function needs its ownrevoke all … from public.supabase/checks/rls-audit.sqlplusmake rls-auditmake the invariants checkable instead of assumed, reading the connection string fromsupabase statusat runtime so no credential lands in the repo.Verified against a local stack:
db reset+ both seeds, all must-be-empty sections empty, section 6 listing onlyget_venue_map; over PostgREST asanon, reads succeed while INSERT/DELETE/import_venueare refused with42501at the grant layer.SupabaseIntegrationTestran for real (3 tests, 0 skipped, 0 failures), andmake checkandmake ciboth pass. No production Supabase project exists yet, so this lands cheaply now and is already in place when one is created.🤖 Generated with Claude Code