supabase-security-advisor-remediation

Supabase Security Advisor — remediation & weekly audit

Owner: dev / sys-security · Related: #4645 #4647 #4663 #4664 #4665 #4666 Automation: .github/workflows/weekly-supabase-advisor-audit.yml + .claude/commands/weekly-supabase-advisor-audit.md + state table public.dev_r_supabase_advisor_findings.

This playbook consolidates how to (a) fix a Supabase Advisor finding for good, and (b) operate the weekly audit that catches new findings, regressions, and silently-never-applied fixes.

---

0. Root cause — why three fixes silently never applied (read this first)

Migrations monitoring/supabase/migrations/003_enable_rls_public_tables.sql, 028_security_definer_revoke_anon.sql, and 029_fix_function_search_paths.sql each carried an Applied: 2026-0x-xx header comment but never took effect in production. They live in monitoring/supabase/migrations/ — a folder .github/workflows/apply-supabase-migrations.yml explicitly never applies (its header calls it a “frozen historical record” after a migration there once caused a ~3-hour outage). The gap went unnoticed for ~2 months because nothing re-checked the advisor on a schedule; it surfaced only via a manual dashboard visit (#4645).

Rules that follow from this:

• ✅ Every new migration goes in supabase/migrations/ only (the active folder apply-supabase-migrations.yml applies). Never add remediation SQL to monitoring/supabase/migrations/.
• ✅ Do not trust an “Applied” header comment. Verify against the live advisor (GET /v1/projects/{ref}/advisors/security) — which is exactly what the weekly audit now does.
• ⚠️ public.dev_r_services is a VIEW — never ALTER it in a migration; target the underlying relation or use the IF NOT EXISTS / ELSE UPDATE guard (see docs/playbooks/supabase-migrations.md).

---

1. Fix recipe per lint

Project ref: mwkqmgadqnkkihjdeqsi. After any fix merges + apply-supabase-migrations.yml runs, re-query GET /v1/projects/{ref}/advisors/security and confirm the specific cache_key is gone.

Lint name	Cause	Fix
rls_disabled_in_public	Table in public with RLS off — anon/authenticated can read/write via PostgREST	ALTER TABLE public.<t> ENABLE ROW LEVEL SECURITY; + a service_role FOR ALL policy. Pre-flight the rolbypassrls check (below) and add explicit SELECT policies for any non-bypass consumer (e.g. grafana_readonly) or it silently returns 0 rows. Pattern: 20260730073959_rls_hardening_definer_view_4645.sql.
security_definer_view	View owned by a superuser runs with definer rights, bypassing the caller’s RLS	ALTER VIEW public.<v> SET (security_invoker = on); — not CREATE OR REPLACE VIEW (restates the body, risks drift). Pattern: 20260730080917_rls_hardening_definer_views_4647.sql.
anon_security_definer_function_executable / authenticated_security_definer_function_executable	anon/authenticated hold EXECUTE on a SECURITY DEFINER function (often password/admin helpers)	REVOKE EXECUTE ON FUNCTION public.<fn>(<args>) FROM anon, authenticated;. Regression trap: a later DROP FUNCTION+CREATE FUNCTION resets the ACL to Postgres’ PUBLIC default and re-exposes it — the weekly audit flags this as a regression (#4663).
function_search_path_mutable	Function has no fixed search_path → schema-injection risk	ALTER FUNCTION public.<fn>(<args>) SET search_path = public, extensions, pg_temp; via a dynamic pg_proc loop (re-scans at apply time, so no stale target-list problem). Two corrections to 029 — see §5.1: exclude extension members, and pin public, extensions, pg_temp rather than '' or public, pg_temp. Fixed the 18 findings (#4664).
extension_in_public	Extension installed in public (e.g. vector, pg_trgm, fuzzystrmatch)	ALTER EXTENSION <ext> SET SCHEMA extensions; — never DROP+CREATE (CASCADEs onto typed columns). Sweep dependencies first and re-pin any function with a pinned search_path that calls the extension’s operators — see §5.2 (#4664).
public_bucket_allows_listing	A FOR SELECT TO public policy on storage.objects lets anon enumerate a whole bucket	Re-scope the policy to TO authenticated, service_role. Closes enumeration, not direct fetch if the bucket is public = true — see §5.3 (#4664).
exposed_sensitive_columns / sensitive columns exposed	A sensitive column is readable because RLS is off on its table	Resolved as a side effect of enabling RLS on the table (it was the claude_sessions.session_id case in #4645). No separate fix needed once RLS is on.
auth_leaked_password_protection	HaveIBeenPwned check disabled	Management API, not a migration: PATCH /v1/projects/{ref}/config/auth. Needs ROLE_SECRET_MANAGER_SUPABASE_ACCESS_TOKEN, which dev-worker age keys cannot decrypt — route to secret-manager, do not widen the key. GET first to confirm the field name (unknown keys are silently ignored). Split out of #4664 as #4668; see §5.4.
rls_policy_always_true / rls_enabled_no_policy	Policy is a no-op (USING (true)) or RLS is on with no policy (returns 0 rows)	Investigate per table — either write a real predicate or confirm the permissive policy is intentional (#4665).

RLS / rolbypassrls pre-flight (do this before enabling RLS)

Enabling RLS on a table with no matching policy returns 0 rows silently (no error) for any role that does not bypass RLS. Before the ALTER:

1. List who reads the table and whether they bypass RLS: SELECT rolname, rolbypassrls FROM pg_roles WHERE rolname IN ('postgres','service_role','anon','authenticated','grafana_readonly'); (postgres, service_role, supabase_backup bypass; anon, authenticated, grafana_readonly do not.)
2. For every non-bypass consumer that must keep reading, add an explicit SELECT policy in the same migration. Getting this wrong locks out e.g. Grafana panels or the migration runner’s own ledger.

Verifying security_invoker — the on vs true false-positive trap (#5686)

Postgres stores the security_invoker reloption as the literal token you wrote — SET (security_invoker = on) stores security_invoker=on, = true stores security_invoker=true. It does not normalize them, and both are the same boolean (on/true/1/yes are all TRUE). So a verification query that array-matches one literal misreports every view stored as the other token:

-- ❌ WRONG — misses views stored as '=true', and 'pg_views' has NO reloptions column (errors)
SELECT viewname FROM pg_views WHERE schemaname='public'
  AND NOT (reloptions @> ARRAY['security_invoker=on']);   -- or '=true'
 
-- ✅ CORRECT — parse the option value and test truthiness, off pg_class
SELECT c.relname AS viewname
FROM pg_class c JOIN pg_namespace n ON n.oid=c.relnamespace
WHERE n.nspname='public' AND c.relkind='v'
  AND lower(COALESCE(
        (SELECT split_part(o,'=',2) FROM unnest(COALESCE(c.reloptions,'{}'::text[])) o
          WHERE o LIKE 'security_invoker=%'), 'off')
      ) NOT IN ('on','true','1','yes')
ORDER BY 1;   -- rows here = genuinely unprotected views

In #5686 a single-literal check produced a false “0/32 views fixed” CRITICAL when 31/32 were actually protected (24 stored =true, the audit checked =on). Before treating a security_definer_view sweep as unapplied, run the truthiness-aware query. Also beware the intentional-DEFINER exception: a *_public projection view (e.g. profiles_public) may be DEFINER by design to expose a curated, secret-hiding column subset to a role that has no table grant on the base table — check has_table_privilege('<role>','public.<base>','SELECT') first; if false, flipping to security_invoker breaks it (permission denied for table). Leave it DEFINER and allowlist it, or grant + add a policy first.

---

2. Playbook corrections carried over from #4646 (do not re-derive)

These three documented paths in docs/playbooks/supabase-migrations.md were wrong as written and cost time in the #4645 session — recorded here so nobody re-discovers them a third time:

1. SUPABASE_ACCESS_TOKEN location (Method A). The Management API token is in secrets/administration.env.sops, which the worker age key is NOT a recipient of — so Method A is unusable from an agent worker. The old doc also pointed at monitoring.env.sops, which holds no such key. For the weekly audit we instead use ROLE_SECRET_MANAGER_SUPABASE_ACCESS_TOKEN (secrets/role-secret-manager.env.sops), delivered to the workflow as a GH Actions secret.
2. Method B connection string was wrong. It said -h 127.0.0.1 … -U postgres; vps-i1 has no socat unit — dial bms-4 at 54.36.123.110:15432 instead — and bare -U postgres is rejected by Supavisor (ENOIDENTIFIER) because the IP hop strips SNI. Use the tenant-qualified user postgres.<project_ref>.
3. The “DB password is stale” caveat is out of date. postgres auth succeeds with the value in monitoring/.env / monitoring.env.sops / n8n-bms4.env.sops, and grafana_readonly auth succeeds with SUPABASE_GRAFANA_PASSWORD. This also supersedes the “neither copy authenticates” note in apply-supabase-migrations.yml (#4387 / #4110) — do not treat it as an open blocker.

---

3. How the weekly audit works

weekly-supabase-advisor-audit.yml (cron 0 4 * * 1, self-hosted bms4) runs the weekly-supabase-advisor-audit skill, which:

0. Preflights the required env vars (names only, never values) and fails fast naming any that is missing or empty — otherwise an unset token surfaces as an opaque fetch error in step 1 (#4676).
1. Fetches GET /v1/projects/{ref}/advisors/security and /performance (Bearer ROLE_SECRET_MANAGER_SUPABASE_ACCESS_TOKEN), retrying transient failures 3× with backoff.
2. Diffs every live lint by cache_key against public.dev_r_supabase_advisor_findings:
   • unseen cache_key → NEW → staged via /alert-ingest (source weekly-supabase-advisor)
   • cache_key whose stored status='fixed' reappears → REGRESSION → staged + flagged
   • status='open'/'acknowledged' still present → row refreshed, not staged
3. Verifies resolution — a status='open' row with a linked github_issue_number that has left the live advisor is marked fixed and its issue gets a “verified resolved” comment. This is the check that would have caught the #4645 gap two months earlier.
4. Reports on the pinned [supabase-advisor-weekly] issue: counts by level + new/regressed/fixed, then writes OK … / CRASH … to $ADVISOR_AUDIT_STATUS_FILE — the handshake the workflow gates on.

Staging in step 2 sits behind a flood guard (MAX_STAGE_PER_RUN, default 50): a run that would stage more than the limit stages nothing and files one summary issue instead. See §4b.

Staged findings flow through the existing dev_r_alert_events → batch_stage_alerts() pg_cron → alert-triage-batch pipeline (docs/designs/alert-staging-batch-triage.md), so one issue is filed per correlated lint with dedup for free — no second issue-filing mechanism.

Severity mapping

ERROR → critical (P1) · WARN → warning (P2) · INFO → info (P3).

---

4. Operating the audit

• First run — always dry_run=true: gh workflow run weekly-supabase-advisor-audit.yml --repo radieu/p24-infra -f dry_run=true Inspect the printed report, then backfill the state table with the real cache_keys of the still-open backlog (#4663/#4664/#4665), status='open', linked to their issue numbers, so the first non-dry run does not re-file them. Baseline mode (the skill stages nothing while the state table has no open rows) is the automated backstop if this manual step is skipped.
• Suppress an accepted-risk finding: set its row status='acknowledged' — recorded, never staged.
• Confirm a fix landed: after a remediation PR merges + apply-supabase-migrations.yml runs, either wait for Monday or trigger a manual (non-dry) run; the finding’s cache_key should flip to fixed and its issue get the verified-resolved comment.
• On a REGRESSION alert: treat it as a real re-exposure (e.g. an ACL reset by a DROP+CREATE), not a routine new finding — re-apply the fix and add a guard so the destructive DDL re-REVOKEs.

State table quick queries

-- current open findings by category
SELECT category, level, count(*) FROM public.dev_r_supabase_advisor_findings
WHERE status = 'open' GROUP BY 1,2 ORDER BY 1,2;
 
-- confirm the #4645 seed baseline
SELECT cache_key, status, github_issue_number FROM public.dev_r_supabase_advisor_findings
WHERE github_issue_number = 4645;

---

4b. Audit job failure modes (#4676)

The first real dispatch (run 30547468452, dry_run=true) hit four defects at once. All four are fixed; this section is the diagnosis key if symptoms resembling them come back.

Symptom	Cause	Fix in place
Issue filed: Agent crash: … crashed: fetch security advisors failed, no HTTP status	Get-Advisors was a single-shot 30 s call; the project returns ~900 lints	3 attempts, backoff, -TimeoutSec 120; the message now carries the HTTP status, and 401/403 is terminal and names ROLE_SECRET_MANAGER_SUPABASE_ACCESS_TOKEN
dry_run=true still wrote rows to the state table	The workflow preamble said “take the live actions the skill defines” regardless of DRY_RUN	The preamble is built from the input; DRY_RUN is restated in the skill as a hard gate that outranks the invoking prompt
Job green, audit unfinished, state table half-written	printf … | claude --dangerously-skip-permissions had no -p, so an interactive session returned while the agent’s work was backgrounded	claude --dangerously-skip-permissions -p "$PROMPT" (same as nightly-devops-triage.yml) + an explicit “never background a phase” rule in the skill
Skill’s catch fired but the run’s conclusion was success, so Notify on failure never ran	claude exits 0 whether or not the agent reports a crash	The agent writes OK … / CRASH … to $ADVISOR_AUDIT_STATUS_FILE; the step fails if that file is missing or does not start with OK

Repairing a partial baseline

Baseline mode fires only when the open set is completely empty. A run interrupted mid-Phase-3 leaves some open rows, so the next run is not baseline and treats the rest of the standing backlog as NEW — potentially hundreds of staged alerts.

The flood guard (MAX_STAGE_PER_RUN, default 50) catches that: if a run would stage more than the limit it stages nothing and files/annotates one [supabase-advisor-weekly] staging suppressed … issue. When that fires:

-- how far off is the baseline? compare against the live advisor count in the run log
SELECT status, count(*) FROM public.dev_r_supabase_advisor_findings GROUP BY 1;

Then either backfill the missing cache_keys as status='open' (restore a complete baseline), or clear the partial rows so the next run re-enters baseline mode — and confirm with a dry_run=true dispatch before the next live run. Do not raise MAX_STAGE_PER_RUN to get past the guard.

Left over from run 30547468452: 144 open rows against ~916 live findings. The flood guard is what keeps the next live run from staging the ~772 difference.

---

5. Field notes from #4664 (search_path · extensions · bucket listing)

Everything below was established against the live DB during #4664. Treat it as settled — the point of recording it is that none of it is derivable from the advisor output alone.

5.0 Method — probe, dry-run, assert

Probe access from a worker. secrets/monitoring.env.sops carries a read-only pooler login that works from any worker host (this is the same connection detail §2.2/§2.3 corrected):

eval "$(sops -d --input-type dotenv --output-type dotenv secrets/monitoring.env.sops \
  | grep -E '^SUPABASE_DB_(HOST|PORT|USER|SSLMODE)=')"
export PGPASSWORD=$(sops -d --input-type dotenv --output-type dotenv secrets/monitoring.env.sops \
  | grep -m1 '^SUPABASE_GRAFANA_PASSWORD=' | cut -d= -f2-)
export PGSSLMODE="$SUPABASE_DB_SSLMODE"
psql -h "$SUPABASE_DB_HOST" -p "$SUPABASE_DB_PORT" -U "$SUPABASE_DB_USER" -d postgres -f probe.sql
unset PGPASSWORD

SUPABASE_DB_USER is grafana_readonly — enough for pg_proc, pg_class, pg_policies, pg_index, pg_namespace and extensions.pg_stat_statements, but not storage.* (permission denied on the schema). For storage.buckets / storage.objects, and for any dry run, switch to postgres.mwkqmgadqnkkihjdeqsi with SUPABASE_DB_PASSWORD from the same file.

Dry-run every migration, then roll it back. scripts/apply-supabase-migrations.sh runs each file with --single-transaction + ON_ERROR_STOP=1, so a file is atomic and you can rehearse it exactly:

\set ON_ERROR_STOP on
BEGIN;
\i supabase/migrations/<file>.sql
-- assertions AND smoke tests against the mutated state, e.g. actually CALL the affected function
ROLLBACK;
-- re-query afterwards to prove the live DB is untouched

Run it as postgres.mwkqmgadqnkkihjdeqsi — the role CI applies as. This is the only way to catch privilege surprises (§5.2) and it gives you a free environment to call what the migration touched.

Assert the post-condition inside the migration. A migration that catches its own errors “succeeds” while changing nothing, and the finding stays open until someone re-runs the advisor. End every remediation file with a DO block that re-queries the catalog and RAISE EXCEPTIONs. Assert both directions — that the finding is gone, and that what depended on the old state still works. The second is the one people skip, and it is the one that causes the incident.

5.1 function_search_path_mutable — two corrections to 029

Exclude extension members. A naive pg_proc scan of public returned 174 functions; the advisor reports 18. The other 156 belong to vector/pg_trgm/postgis and are supabase_admin’s to manage. Without this filter the loop attempts ~1400 ALTERs that all raise caught warnings:

AND NOT EXISTS (SELECT 1 FROM pg_depend d
                WHERE d.objid = p.oid AND d.classid = 'pg_proc'::regclass AND d.deptype = 'e')

Pin public, extensions, pg_temp. Not '', and not 029’s public, pg_temp. extensions must be in the path because extensions get relocated there (§5.2) — a function pinned without it that later grows an unqualified extension call breaks with an error that looks unrelated to the move. This is already the value on 120 existing functions and matches the role defaults (postgres → "$user", public, extensions; grafana_readonly → public, extensions).

Including extensions does not weaken the fix: the injection risk is CREATE rights, and neither schema grants them to a reachable role — extensions is {postgres=UC, dashboard_user=UC, anon=U, authenticated=U, service_role=U, grafana_readonly=U}, and PUBLIC holds only USAGE on public. Pin pg_temp last so a caller’s temp schema can never shadow a real object.

5.2 extension_in_public — the two things that bite

Ownership is not what it looks like. vector, pg_trgm and fuzzystrmatch are owned by supabase_admin (a superuser), while CI applies as postgres — not a superuser, not a member of supabase_admin. By the book ALTER EXTENSION … SET SCHEMA should fail with “must be owner of extension”. It does not — Supabase grants postgres the rights. Do not design around either assumption; run the statement inside BEGIN; … ROLLBACK; and find out.

A pinned search_path on a caller is the silent killer. Sweep every dependency (pg_attribute for typed columns, pg_index for opclasses, pg_proc/pg_class for bodies, pg_constraint), then specifically hunt callers whose path is pinned without extensions:

SELECT n.nspname||'.'||p.proname, array_to_string(p.proconfig,' | ')
FROM pg_proc p JOIN pg_namespace n ON p.pronamespace = n.oid
WHERE p.prokind IN ('f','p')
  AND NOT EXISTS (SELECT 1 FROM pg_depend d WHERE d.objid = p.oid
                  AND d.classid = 'pg_proc'::regclass AND d.deptype = 'e')
  AND pg_get_functiondef(p.oid) ~* '(similarity\(|show_trgm|soundex|levenshtein|metaphone|<->|<=>|<#>|::vector)';

In #4664 this returned exactly one object — public.match_documents, pinned public, pg_temp, backing the live n8n/LangChain RAG store. A caller with no pinned path is usually fine (postgres, grafana_readonly, supabase_admin and PostgREST all already carry extensions); a caller with a pinned path that omits it will break and no caller-side setting rescues it. Re-pin it in the same transaction as the move so no broken window exists.

Text screens over-match — a literal % in a RAISE format string and a column merely named after a trgm index both look like hits. Read every matched body before concluding.

Existing indexes survive the opclass relocation (they are OID-bound); assert indisvalid and EXPLAIN a representative query anyway. Add NOTIFY pgrst, 'reload schema'; — extension types appear in RPC signatures in PostgREST’s schema cache.

5.3 public_bucket_allows_listing — prove it, and scope the claim honestly

FOR SELECT TO public includes anon — every unauthenticated request. Prove the exposure with the anon key, and always run the same call against a correctly-scoped sibling bucket as a control — that is what proves the policy is at fault rather than the key being privileged:

curl -s -X POST "$URL/storage/v1/object/list/<bucket>" \
  -H "apikey: $ANON" -H "Authorization: Bearer $ANON" \
  -H "Content-Type: application/json" -d '{"prefix":"","limit":5,"offset":0}'

Tightening the RLS policy closes enumeration, not direct fetch. If storage.buckets.public = true, the route /storage/v1/object/public/<bucket>/<path> is served without consulting RLS at all — a known path stays anonymously fetchable, verified before and after the fix. Closing that means public = false plus migrating consumers to signed URLs, an app-side change. Say so explicitly rather than implying the bucket is now private; enumeration is still the high-value half, since an unknown UUID-suffixed path is not guessable.

Check consumers before narrowing: service_role is rolbypassrls = true so server-side callers are unaffected, and if every object has a non-NULL owner_id there is no anonymous-writer workflow whose reads must stay anonymous.

5.4 auth_leaked_password_protection — a dev worker cannot do this

ROLE_SECRET_MANAGER_SUPABASE_ACCESS_TOKEN lives only in secrets/role-secret-manager.env.sops, which dev-issue / dev-coder worker age keys are deliberately not recipients of (sops -d exits 128 with no identity matched any of the recipients, while 19 other secrets/*.env.sops decrypt fine from the same key). That is role isolation working as designed — route it to secret-manager rather than widening the key. Tracked as #4668.

GET /v1/projects/{ref}/config/auth before any PATCH: the Management API accepts and silently ignores unknown keys, so a PATCH against a guessed field name returns 200 and changes nothing. Confirm the field (expected password_hibp_enabled) in the live GET response, PATCH, then GET again — a 200 is not confirmation. Known blocker: #4626 (P0) reports this token minting keys that return 401; if the GET 401s, that is why.

Enabling HIBP rejects new/changed passwords found in a breach corpus. It does not invalidate existing sessions or stored hashes, so it is safe on a live project.