Skip to content

Lock down internal tables and quota view #381

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
simonsmallchua merged 6 commits into from
May 9, 2026
Merged

Lock down internal tables and quota view #381

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5ecf8fb
Lock down internal tables, quota view
simonsmallchua May 9, 2026
fbe2ab9
Add changelog entry for migration A
simonsmallchua May 9, 2026
239f43c
Revoke anon execute on internal RPCs
simonsmallchua May 9, 2026
66ba2f6
Optimise RLS and function search_path
simonsmallchua May 9, 2026
c356db4
Index unindexed FKs, document advisor decisions
simonsmallchua May 9, 2026
09e4bab
Drop CONCURRENTLY from FK index migration
simonsmallchua May 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 42 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,48 @@ On merge, CI will:

## [Unreleased]

_Add unreleased changes here._
### Security

- Enabled RLS (no policies) and revoked `anon`/`authenticated` grants on
`task_outbox`, `task_outbox_dead`, and `lighthouse_runs`; these tables are
only accessed by the Go server via the service role.
- Switched the `organisation_quota_status` view to `security_invoker = true` so
it honours the caller's RLS rather than the creator's.
- Revoked `anon`/`authenticated` `EXECUTE` on 19 server-internal
`SECURITY DEFINER` functions (OAuth token store/get/delete for Google
Analytics, Slack, and Webflow; vault cleanup helpers; Slack user-link helpers;
`increment_daily_usage`). These RPCs are only called by the Go server via the
service role; the three RLS-helper functions used inside policies
(`user_is_member_of`, `user_organisation_id`, `user_organisations`) remain
callable.

### Performance

- Rewrote 14 RLS policies on `notifications`, `daily_usage`,
`google_analytics_connections`, `google_analytics_accounts`, and
`organisation_domains` to wrap `auth.uid()` in a `(select …)` so it is
evaluated once per query instead of once per row.
- Scoped the `Service role can manage usage` policy on `daily_usage`
`TO service_role` so it no longer fires during anon/authenticated SELECTs,
removing the multiple-permissive-policies overhead.
- Pinned `search_path` on `update_job_queue_counters` and
`get_daily_quota_remaining`.
- Added covering indexes on nine previously-unindexed foreign keys
(`google_analytics_accounts.installing_user_id`,
`google_analytics_connections.installing_user_id`,
`lighthouse_runs.source_task_id`, `organisation_invites.created_by`,
`page_analytics.ga_connection_id`, `platform_org_mappings.created_by`,
`slack_connections.installing_user_id`, `task_outbox_dead.lighthouse_run_id`,
`webflow_connections.installing_user_id`) so cascade deletes and FK joins no
longer fall back to sequential scans.

### Documentation

- Added
[`docs/security/SUPABASE_ADVISORS.md`](docs/security/SUPABASE_ADVISORS.md)
recording the deliberate "won't fix" advisor findings (the three RLS-helper
`SECURITY DEFINER` functions, the empty-policy state of `domain_hosts`) and
deferred items (unused indexes, Auth DB connection strategy).

## Full changelog history

Expand Down
43 changes: 43 additions & 0 deletions docs/security/SUPABASE_ADVISORS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# Supabase Advisor Findings

This file tracks Supabase database linter findings that we have decided not to
remediate (or have deferred), with the reasoning. The companion file
[`SECURITY_REMEDIATION_PLAN.md`](SECURITY_REMEDIATION_PLAN.md) covers code-level
security scanner suppressions; this file covers the database advisors surfaced
under **Supabase Studio → Advisors**.

Last reviewed: 2026-05-09.

## How to use this list

1. When a new advisor finding appears, fix it via a migration if possible.
2. If the finding is a deliberate keep, append a row below with the `cache_key`,
the reason it must remain, and the migration or code that makes it necessary.
3. After landing the corresponding migration, dismiss the matching finding in
the Supabase Studio Advisors UI so the dashboard stays signal-only.

## Deliberate keeps (will keep being flagged)

| Advisor lint | Object | Why we keep it |
| ---------------------------------------------------- | -------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `anon_security_definer_function_executable` | `public.user_is_member_of(uuid)` | Invoked inside RLS policies that need to look up `organisation_members` rows owned by other users. Must remain executable by `authenticated` for those policies to evaluate. Service role bypass alone is not enough — clients call this. |
| `anon_security_definer_function_executable` | `public.user_organisation_id()` | Same reason — used inside RLS policies to derive the caller's primary organisation. |
| `anon_security_definer_function_executable` | `public.user_organisations()` | Same reason — used inside RLS policies for multi-org callers. |
| `authenticated_security_definer_function_executable` | Same three functions | Mirror of the `anon` warnings; same reasoning. |
| `rls_enabled_no_policy` | `public.domain_hosts` | Already deny-all by default. Only the Go server (service role) writes to it. |

## Deferred (not done in this round)

| Advisor lint | Object(s) | Why deferred |
| ------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
| `unused_index` | ~30 indexes across `domains`, `users`, `organisations`, `organisation_invites`, `organisation_members`, `job_share_links`, `task_outbox_dead`, `domain_hosts`, `notifications`, `platform_org_mappings`, `google_analytics_*`, `page_analytics`, `slack_user_links`, `webflow_site_settings` | `pg_stat_user_indexes` resets at upgrade and does not yet reflect a long enough baseline for the recently-added tables. Revisit only if write throughput becomes a bottleneck. |
| `auth_db_connections_absolute` | Auth server (10 connections fixed) | Requires a Supabase dashboard configuration change, not a migration. Switch to percentage-based allocation when the project is next scaled up. |

## Resolved in this PR

| Migration | Findings cleared |
| ------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `20260509101603_lock_down_internal_tables_and_quota_view.sql` | `rls_disabled_in_public` on `task_outbox`, `task_outbox_dead`, `lighthouse_runs`; `security_definer_view` on `organisation_quota_status`. |
| `20260509102622_revoke_anon_execute_on_internal_rpcs.sql` | `anon_security_definer_function_executable` and `authenticated_security_definer_function_executable` for the 19 server-internal RPCs (token store/get/delete, vault cleanup, slack-link, `increment_daily_usage`). |
| `20260509104940_optimise_rls_and_function_search_path.sql` | `auth_rls_initplan` on `notifications`, `daily_usage`, `google_analytics_*`, `organisation_domains`; `multiple_permissive_policies` on `daily_usage`; `function_search_path_mutable` on `update_job_queue_counters`, `get_daily_quota_remaining`. |
| `20260509111837_add_covering_indexes_for_unindexed_fks.sql` | `unindexed_foreign_keys` on `google_analytics_accounts`, `google_analytics_connections`, `lighthouse_runs`, `organisation_invites`, `page_analytics`, `platform_org_mappings`, `slack_connections`, `task_outbox_dead`, `webflow_connections`. |
29 changes: 29 additions & 0 deletions supabase/migrations/20260509101603_lock_down_internal_tables_and_quota_view.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
-- Migration A: lock down server-internal tables and the quota view.
--
-- Background: the Supabase database linter flagged three public tables with
-- RLS disabled (`task_outbox`, `task_outbox_dead`, `lighthouse_runs`) and one
-- public view (`organisation_quota_status`) running with the creator's
-- permissions. All four are accessed exclusively by the Go server using the
-- service role; no frontend or RPC caller references them.
--
-- This migration:
-- 1. Revokes anon/authenticated table grants so PostgREST cannot reach the
-- three internal tables even before RLS is consulted.
-- 2. Enables RLS on each table with no policies, so any non-service-role
-- access is denied by default.
-- 3. Switches `organisation_quota_status` to `security_invoker=true` so
-- the view enforces the caller's RLS instead of the creator's.

-- 1. Revoke client-role privileges on internal tables.
REVOKE ALL ON TABLE public.task_outbox FROM anon, authenticated;
REVOKE ALL ON TABLE public.task_outbox_dead FROM anon, authenticated;
REVOKE ALL ON TABLE public.lighthouse_runs FROM anon, authenticated;

-- 2. Enable RLS with no policies — service role bypasses, all other roles
-- are denied.
ALTER TABLE public.task_outbox ENABLE ROW LEVEL SECURITY;
ALTER TABLE public.task_outbox_dead ENABLE ROW LEVEL SECURITY;
ALTER TABLE public.lighthouse_runs ENABLE ROW LEVEL SECURITY;

-- 3. Make the quota view honour the caller's RLS rather than the creator's.
ALTER VIEW public.organisation_quota_status SET (security_invoker = true);
59 changes: 59 additions & 0 deletions supabase/migrations/20260509102622_revoke_anon_execute_on_internal_rpcs.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
-- Migration B: revoke anon/authenticated EXECUTE on server-internal
-- SECURITY DEFINER functions.
--
-- Background: the Supabase database linter flagged 22 SECURITY DEFINER
-- functions in the public schema as callable by `anon` and `authenticated`
-- via PostgREST's /rpc endpoint. Audit results:
--
-- * Token store/get/delete functions for GA, Slack, and Webflow are only
-- called from the Go server using the service role
-- (internal/db/google_connections.go, slack.go, webflow_connections.go).
-- * `increment_daily_usage` is called from internal/db/batch.go.
-- * `cleanup_*_vault_secret`, `auto_link_slack_user`,
-- `auto_link_existing_slack_users`, and `sync_slack_user_id` have no
-- callers in the application code at all.
--
-- The Go server uses the service role, which bypasses EXECUTE grants, so
-- revoking from `anon`/`authenticated` removes the public attack surface
-- without affecting the server.
--
-- Three SECURITY DEFINER helpers are intentionally **left alone** because
-- they are invoked inside RLS policies and need to remain callable by the
-- `authenticated` role for those policies to evaluate correctly:
-- * public.user_is_member_of(uuid)
-- * public.user_organisation_id()
-- * public.user_organisations()

-- Token store/get/delete (GA connections).
REVOKE EXECUTE ON FUNCTION public.store_ga_token(uuid, text) FROM anon, authenticated;
REVOKE EXECUTE ON FUNCTION public.get_ga_token(uuid) FROM anon, authenticated;
REVOKE EXECUTE ON FUNCTION public.delete_ga_token(uuid) FROM anon, authenticated;

-- Token store/get/delete (GA accounts).
REVOKE EXECUTE ON FUNCTION public.store_ga_account_token(uuid, text) FROM anon, authenticated;
REVOKE EXECUTE ON FUNCTION public.get_ga_account_token(uuid) FROM anon, authenticated;
REVOKE EXECUTE ON FUNCTION public.delete_ga_account_token(uuid) FROM anon, authenticated;

-- Token store/get/delete (Slack).
REVOKE EXECUTE ON FUNCTION public.store_slack_token(uuid, text) FROM anon, authenticated;
REVOKE EXECUTE ON FUNCTION public.get_slack_token(uuid) FROM anon, authenticated;
REVOKE EXECUTE ON FUNCTION public.delete_slack_token(uuid) FROM anon, authenticated;

-- Token store/get/delete (Webflow).
REVOKE EXECUTE ON FUNCTION public.store_webflow_token(uuid, text) FROM anon, authenticated;
REVOKE EXECUTE ON FUNCTION public.get_webflow_token(uuid) FROM anon, authenticated;
REVOKE EXECUTE ON FUNCTION public.delete_webflow_token(uuid) FROM anon, authenticated;

-- Vault cleanup helpers (no application callers; service-role only).
REVOKE EXECUTE ON FUNCTION public.cleanup_ga_vault_secret() FROM anon, authenticated;
REVOKE EXECUTE ON FUNCTION public.cleanup_slack_vault_secret() FROM anon, authenticated;
REVOKE EXECUTE ON FUNCTION public.cleanup_webflow_vault_secret() FROM anon, authenticated;

-- Slack user-link helpers (no application callers; invoked by triggers /
-- service-role admin scripts only).
REVOKE EXECUTE ON FUNCTION public.auto_link_slack_user() FROM anon, authenticated;
REVOKE EXECUTE ON FUNCTION public.auto_link_existing_slack_users() FROM anon, authenticated;
REVOKE EXECUTE ON FUNCTION public.sync_slack_user_id() FROM anon, authenticated;

-- Daily usage counter (server-side batch writer only).
REVOKE EXECUTE ON FUNCTION public.increment_daily_usage(uuid, integer) FROM anon, authenticated;
Loading
Loading