fix(security): add org membership checks to prevent cross-tenant data leakage in SECURITY DEFINER functions#1738
Conversation
…closure functions Add auth.uid() + org_users membership verification to 8 SECURITY DEFINER functions that accept org_id but never verify the caller belongs to that organization. Any authenticated user could query metrics, plan info, storage stats, and billing data for ANY organization by passing an arbitrary org_id. Affected functions: - get_app_metrics(org_id, start_date, end_date) - get_current_plan_max_org(orgid) - get_current_plan_name_org(orgid) - get_plan_usage_percent_detailed(orgid) [both overloads] - get_total_app_storage_size_orgs(org_id, app_id) - get_total_storage_size_org(org_id) - is_good_plan_v5_org(orgid) Each function now returns empty/null/false if the caller is not a member of the target organization, preventing cross-tenant data leakage.
📝 WalkthroughWalkthroughThis pull request adds org membership verification guards to eight existing PostgreSQL SECURITY DEFINER functions, ensuring that authenticated users can only access metrics, plan details, and storage data for organizations they belong to, preventing cross-tenant information disclosure. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 SQLFluff (4.0.4)supabase/migrations/20260303100000_add_org_membership_checks.sqlUser Error: No dialect was specified. You must configure a dialect or specify one on the command line using --dialect after the command. Available dialects: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@supabase/migrations/20260303100000_add_org_membership_checks.sql`:
- Around line 3-17: Update the migration comment header so the function count
matches the list: change "These 7 info-disclosure functions" to "These 8
info-disclosure functions" (or remove the numeric count entirely) so it
correctly reflects the eight listed functions (get_app_metrics,
get_current_plan_max_org, get_current_plan_name_org,
get_plan_usage_percent_detailed (both overloads count as one or two as
appropriate), get_total_app_storage_size_orgs, get_total_storage_size_org,
is_good_plan_v5_org).
| -- These 7 info-disclosure functions accept an org_id parameter but never verify | ||
| -- that the calling user belongs to that organization. Any authenticated user can | ||
| -- query metrics, plan info, and usage stats for ANY organization by passing an | ||
| -- arbitrary org_id. This migration adds org_users membership checks so only | ||
| -- members of an organization can access its data. | ||
| -- | ||
| -- Affected functions: | ||
| -- 1. get_app_metrics(org_id, start_date, end_date) | ||
| -- 2. get_current_plan_max_org(orgid) | ||
| -- 3. get_current_plan_name_org(orgid) | ||
| -- 4. get_plan_usage_percent_detailed(orgid) | ||
| -- 5. get_plan_usage_percent_detailed(orgid, cycle_start, cycle_end) | ||
| -- 6. get_total_app_storage_size_orgs(org_id, app_id) | ||
| -- 7. get_total_storage_size_org(org_id) | ||
| -- 8. is_good_plan_v5_org(orgid) |
There was a problem hiding this comment.
Fix the header count mismatch in the migration comment.
Line 3 says “7 info-disclosure functions,” but Lines 10-17 list 8.
✏️ Proposed fix
--- a/supabase/migrations/20260303100000_add_org_membership_checks.sql
+++ b/supabase/migrations/20260303100000_add_org_membership_checks.sql
@@
--- These 7 info-disclosure functions accept an org_id parameter but never verify
+-- These 8 info-disclosure functions accept an org_id parameter but never verify🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/migrations/20260303100000_add_org_membership_checks.sql` around
lines 3 - 17, Update the migration comment header so the function count matches
the list: change "These 7 info-disclosure functions" to "These 8 info-disclosure
functions" (or remove the numeric count entirely) so it correctly reflects the
eight listed functions (get_app_metrics, get_current_plan_max_org,
get_current_plan_name_org, get_plan_usage_percent_detailed (both overloads count
as one or two as appropriate), get_total_app_storage_size_orgs,
get_total_storage_size_org, is_good_plan_v5_org).
|
|
AI SPAM |
1 participant
Summary
Add
auth.uid()+org_usersmembership verification to 8SECURITY DEFINERfunctions that accept anorg_idparameter but never check that the caller belongs to the target organization.Any authenticated user could read metrics, plan info, storage stats, and billing data for ANY organization by passing an arbitrary
org_idto these functions.Affected Functions
get_app_metrics(org_id, start_date, end_date)get_current_plan_max_org(orgid)get_current_plan_name_org(orgid)get_plan_usage_percent_detailed(orgid)get_plan_usage_percent_detailed(orgid, cycle_start, cycle_end)get_total_app_storage_size_orgs(org_id, app_id)get_total_storage_size_org(org_id)is_good_plan_v5_org(orgid)Implementation
Each function now includes a cross-tenant guard at the top of its body:
IF NOT EXISTS ( SELECT 1 FROM public.org_users WHERE org_users.org_id = <param> AND org_users.user_id = (SELECT auth.uid()) ) THEN RETURN; -- empty/null/false depending on return type END IF;Migration
20260303100000_add_org_membership_checks.sql—CREATE OR REPLACE FUNCTIONfor all 8 functionsCloses #1737
Test plan
SELECT * FROM get_plan_usage_percent_detailed('org-1-uuid')— should return usage data normally.SELECT * FROM get_plan_usage_percent_detailed('org-1-uuid')— should return empty result set.org_id— all should return empty/null/0/false.Usage.vueandsupabase.tsstill work correctly (they always pass the user's own org_id).get_app_metrics(org_id)single-arg wrapper still works (delegates to the 3-arg version which has the check).Screenshots
N/A — backend-only changes (SQL migration). No UI changes.
Checklist
bun run lint:backend && bun run lint.Summary by CodeRabbit