Restrict invite_user_to_org RPC to authenticated callers#1710
Restrict invite_user_to_org RPC to authenticated callers#1710
Conversation
📝 WalkthroughWalkthroughA new SQL function Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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/20260227150000_fix_invite_user_to_org_security.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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c7b781740
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| REVOKE EXECUTE ON FUNCTION public.invite_user_to_org( | ||
| character varying, | ||
| uuid, | ||
| public.user_min_right | ||
| ) FROM "anon"; |
There was a problem hiding this comment.
/organization/members still calls invite_user_to_org for non-RBAC orgs (supabase/functions/_backend/public/organization/members/post.ts, legacy branch) through supabaseApikey, which authenticates with the anon key plus capgkey header (supabase/functions/_backend/utils/supabase.ts). Revoking anon execute here causes those valid API-key requests to fail with function-permission errors, so member invites break for legacy orgs; keep anon execute and rely on the function’s internal get_identity_org_allowed/check_min_rights checks to reject unauthenticated callers.
Useful? React with 👍 / 👎.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/20260227150000_fix_invite_user_to_org_security.sql`:
- Around line 62-63: The 2FA check and audit log are using auth.uid() but
authorization earlier uses calling_user_id from get_identity_org_allowed(), so
update the IF and pg_log call in invite_user_to_org to use calling_user_id
instead of auth.uid(): call public.has_2fa_enabled(calling_user_id) in the IF
condition when org.enforcing_2fa is true, and include 'uid': calling_user_id in
the jsonb_build_object passed to public.pg_log so the correct resolved caller is
checked and recorded; keep all other logic and names (invite_user_to_org,
org.enforcing_2fa, public.has_2fa_enabled, public.pg_log) unchanged.
- Around line 119-135: The function public.invite_user_to_org currently revokes
EXECUTE only from "anon" but inherits EXECUTE from PUBLIC; before the existing
REVOKE on "anon" add an explicit REVOKE EXECUTE ON FUNCTION
public.invite_user_to_org(character varying, uuid, public.user_min_right) FROM
PUBLIC so the PUBLIC grant is removed first, then keep the existing REVOKE FROM
"anon" and the subsequent GRANTs to "authenticated" and "service_role"; adjust
the ACL order around the invite_user_to_org function to ensure PUBLIC cannot
execute it after CREATE OR REPLACE.
| IF org.enforcing_2fa AND NOT public.has_2fa_enabled(auth.uid()) THEN | ||
| PERFORM public.pg_log('deny: SUPER_ADMIN_2FA_REQUIRED', jsonb_build_object('org_id', invite_user_to_org.org_id, 'invite_type', invite_user_to_org.invite_type, 'uid', auth.uid())); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n /dev/null && find . -name "*fix_invite_user_to_org_security.sql" -type fRepository: Cap-go/capgo
Length of output: 128
🏁 Script executed:
find . -path "*/supabase/migrations/*" -name "*fix_invite_user_to_org_security.sql" -type fRepository: Cap-go/capgo
Length of output: 128
🏁 Script executed:
git ls-files | grep -E "supabase/migrations/.*fix_invite_user_to_org_security"Repository: Cap-go/capgo
Length of output: 126
🏁 Script executed:
git ls-files | grep -E "supabase/migrations/.*2026" | head -20Repository: Cap-go/capgo
Length of output: 1409
🏁 Script executed:
ls -la supabase/migrations/ | grep "2026" | head -20Repository: Cap-go/capgo
Length of output: 2029
🏁 Script executed:
fd "fix_invite_user_to_org_security" supabase/migrations/Repository: Cap-go/capgo
Length of output: 126
🏁 Script executed:
ls -lh supabase/migrations/ | tail -30Repository: Cap-go/capgo
Length of output: 3016
🏁 Script executed:
rg -l "fix_invite_user_to_org_security" supabase/migrations/Repository: Cap-go/capgo
Length of output: 38
🏁 Script executed:
git ls-files supabase/migrations/ | tail -10Repository: Cap-go/capgo
Length of output: 745
🏁 Script executed:
rg -l "SUPER_ADMIN_2FA_REQUIRED" supabase/Repository: Cap-go/capgo
Length of output: 221
🏁 Script executed:
rg -B5 -A5 "SUPER_ADMIN_2FA_REQUIRED" supabase/Repository: Cap-go/capgo
Length of output: 6352
🏁 Script executed:
cat -n supabase/migrations/20260227150000_fix_invite_user_to_org_security.sql | head -100Repository: Cap-go/capgo
Length of output: 5007
🏁 Script executed:
cat -n supabase/migrations/20260227150000_fix_invite_user_to_org_security.sqlRepository: Cap-go/capgo
Length of output: 6138
🏁 Script executed:
rg -A10 "CREATE.*FUNCTION.*get_identity_org_allowed" supabase/Repository: Cap-go/capgo
Length of output: 3596
🏁 Script executed:
rg -B2 -A2 "has_2fa_enabled" supabase/ | head -60Repository: Cap-go/capgo
Length of output: 4242
🏁 Script executed:
rg -A15 "CREATE.*FUNCTION.*has_2fa_enabled" supabase/migrations/ | head -80Repository: Cap-go/capgo
Length of output: 2714
🏁 Script executed:
rg -A8 "CREATE OR REPLACE FUNCTION public.has_2fa_enabled" supabase/schemas/prod.sqlRepository: Cap-go/capgo
Length of output: 38
🏁 Script executed:
cat -n supabase/migrations/20260102140000_fix_get_identity_hashed_apikeys.sql | head -100Repository: Cap-go/capgo
Length of output: 4239
🏁 Script executed:
sed -n '100,160p' supabase/migrations/20260102140000_fix_get_identity_hashed_apikeys.sqlRepository: Cap-go/capgo
Length of output: 1957
🏁 Script executed:
rg -B5 "calling_user_id" supabase/migrations/20260227150000_fix_invite_user_to_org_security.sql | head -40Repository: Cap-go/capgo
Length of output: 1620
Use the resolved caller identity for 2FA checks and audit logging.
Authorization is checked against calling_user_id (resolved via get_identity_org_allowed() to handle both JWT and API key auth), but the 2FA check at lines 62-63 switches to auth.uid(). In service-role or API-key-mediated flows, auth.uid() is NULL while calling_user_id contains the actual authenticated user, causing the 2FA check to be bypassed or invalid and audit logging to record NULL instead of the actual caller.
🔧 Proposed fix
- IF org.enforcing_2fa AND NOT public.has_2fa_enabled(auth.uid()) THEN
- PERFORM public.pg_log('deny: SUPER_ADMIN_2FA_REQUIRED', jsonb_build_object('org_id', invite_user_to_org.org_id, 'invite_type', invite_user_to_org.invite_type, 'uid', auth.uid()));
+ IF org.enforcing_2fa AND NOT public.has_2fa_enabled(calling_user_id) THEN
+ PERFORM public.pg_log('deny: SUPER_ADMIN_2FA_REQUIRED', jsonb_build_object('org_id', invite_user_to_org.org_id, 'invite_type', invite_user_to_org.invite_type, 'uid', calling_user_id));
RETURN 'NO_RIGHTS';
END IF;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| IF org.enforcing_2fa AND NOT public.has_2fa_enabled(auth.uid()) THEN | |
| PERFORM public.pg_log('deny: SUPER_ADMIN_2FA_REQUIRED', jsonb_build_object('org_id', invite_user_to_org.org_id, 'invite_type', invite_user_to_org.invite_type, 'uid', auth.uid())); | |
| IF org.enforcing_2fa AND NOT public.has_2fa_enabled(calling_user_id) THEN | |
| PERFORM public.pg_log('deny: SUPER_ADMIN_2FA_REQUIRED', jsonb_build_object('org_id', invite_user_to_org.org_id, 'invite_type', invite_user_to_org.invite_type, 'uid', calling_user_id)); | |
| RETURN 'NO_RIGHTS'; | |
| END IF; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/migrations/20260227150000_fix_invite_user_to_org_security.sql`
around lines 62 - 63, The 2FA check and audit log are using auth.uid() but
authorization earlier uses calling_user_id from get_identity_org_allowed(), so
update the IF and pg_log call in invite_user_to_org to use calling_user_id
instead of auth.uid(): call public.has_2fa_enabled(calling_user_id) in the IF
condition when org.enforcing_2fa is true, and include 'uid': calling_user_id in
the jsonb_build_object passed to public.pg_log so the correct resolved caller is
checked and recorded; keep all other logic and names (invite_user_to_org,
org.enforcing_2fa, public.has_2fa_enabled, public.pg_log) unchanged.
| REVOKE EXECUTE ON FUNCTION public.invite_user_to_org( | ||
| character varying, | ||
| uuid, | ||
| public.user_min_right | ||
| ) FROM "anon"; | ||
|
|
||
| GRANT EXECUTE ON FUNCTION public.invite_user_to_org( | ||
| character varying, | ||
| uuid, | ||
| public.user_min_right | ||
| ) TO "authenticated"; | ||
|
|
||
| GRANT EXECUTE ON FUNCTION public.invite_user_to_org( | ||
| character varying, | ||
| uuid, | ||
| public.user_min_right | ||
| ) TO "service_role"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for the migration file
fd -type f -name "*fix_invite_user_to_org_security.sql"Repository: Cap-go/capgo
Length of output: 226
🏁 Script executed:
# Also check for any existing invite_user_to_org function definitions
rg -n "CREATE.*FUNCTION.*invite_user_to_org|DROP FUNCTION.*invite_user_to_org" supabase/migrations/Repository: Cap-go/capgo
Length of output: 1140
🏁 Script executed:
# Search for patterns of PUBLIC revokes in other migrations
rg -n "REVOKE EXECUTE.*FROM PUBLIC|REVOKE.*FROM \"PUBLIC\"" supabase/migrations/ -A 2 -B 2Repository: Cap-go/capgo
Length of output: 694
🏁 Script executed:
# Read the target migration file to understand full context
cat -n supabase/migrations/20260227150000_fix_invite_user_to_org_security.sqlRepository: Cap-go/capgo
Length of output: 6138
🏁 Script executed:
# Check the previous migration that modified invite_user_to_org
cat -n supabase/migrations/20260203201308_rbac_org_member_no_app_access.sql | head -250Repository: Cap-go/capgo
Length of output: 10909
🏁 Script executed:
# Search for all ACL modifications to invite_user_to_org function
rg -n "GRANT|REVOKE.*invite_user_to_org" supabase/migrations/ -B 2 -A 2Repository: Cap-go/capgo
Length of output: 50369
🏁 Script executed:
# Check if there are any ALTER DEFAULT PRIVILEGES statements affecting functions
rg -n "ALTER DEFAULT PRIVILEGES" supabase/migrations/ -A 2 -B 2Repository: Cap-go/capgo
Length of output: 3833
🌐 Web query:
PostgreSQL CREATE OR REPLACE FUNCTION preserves ACL permissions public role
💡 Result:
In PostgreSQL, CREATE OR REPLACE FUNCTION preserves the existing function’s ownership and permissions (ACL) when it replaces an existing function with the same name + same input argument types—so whatever you previously GRANTed/REVOKEd (including for the PUBLIC pseudo-role) remains in effect. [1]
What can make it look like “PUBLIC got EXECUTE again”:
- If you didn’t actually replace the same function (e.g., you changed the argument types / effectively created a new overloaded function), then you created a new object, and newly created functions default to granting
EXECUTEtoPUBLICunless you revoke it (or change default privileges). [1][2] - If you DROP + CREATE instead of OR REPLACE, it’s a new object, so old grants are not retained. [1]
So: Yes, OR REPLACE preserves ACLs for PUBLIC (and everyone else), as long as it truly replaces the same function signature. [1]
Sources: PostgreSQL CREATE FUNCTION docs on OR REPLACE preserving ownership/permissions [1], PostgreSQL privileges docs on default EXECUTE to PUBLIC for newly created functions [2].
[1] (postgresql.org)
[2] (postgresql.org)
Citations:
- 1: https://www.postgresql.org/docs/current/sql-createfunction.html?utm_source=openai
- 2: https://www.postgresql.org/docs/13/ddl-priv.html?utm_source=openai
Add explicit REVOKE EXECUTE FROM PUBLIC before the anon revoke to fully close inherited access.
The function inherits EXECUTE on PUBLIC from the base migration defaults. Revoking from "anon" alone leaves the PUBLIC role with execute rights. Since CREATE OR REPLACE FUNCTION preserves existing ACLs, the function retains its PUBLIC grant unless explicitly revoked first. Add the PUBLIC revoke to match the pattern used elsewhere for security-critical functions:
Proposed fix
+REVOKE EXECUTE ON FUNCTION public.invite_user_to_org(
+ character varying,
+ uuid,
+ public.user_min_right
+) FROM PUBLIC;
+
REVOKE EXECUTE ON FUNCTION public.invite_user_to_org(
character varying,
uuid,
public.user_min_right
) FROM "anon";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| REVOKE EXECUTE ON FUNCTION public.invite_user_to_org( | |
| character varying, | |
| uuid, | |
| public.user_min_right | |
| ) FROM "anon"; | |
| GRANT EXECUTE ON FUNCTION public.invite_user_to_org( | |
| character varying, | |
| uuid, | |
| public.user_min_right | |
| ) TO "authenticated"; | |
| GRANT EXECUTE ON FUNCTION public.invite_user_to_org( | |
| character varying, | |
| uuid, | |
| public.user_min_right | |
| ) TO "service_role"; | |
| REVOKE EXECUTE ON FUNCTION public.invite_user_to_org( | |
| character varying, | |
| uuid, | |
| public.user_min_right | |
| ) FROM PUBLIC; | |
| REVOKE EXECUTE ON FUNCTION public.invite_user_to_org( | |
| character varying, | |
| uuid, | |
| public.user_min_right | |
| ) FROM "anon"; | |
| GRANT EXECUTE ON FUNCTION public.invite_user_to_org( | |
| character varying, | |
| uuid, | |
| public.user_min_right | |
| ) TO "authenticated"; | |
| GRANT EXECUTE ON FUNCTION public.invite_user_to_org( | |
| character varying, | |
| uuid, | |
| public.user_min_right | |
| ) TO "service_role"; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/migrations/20260227150000_fix_invite_user_to_org_security.sql`
around lines 119 - 135, The function public.invite_user_to_org currently revokes
EXECUTE only from "anon" but inherits EXECUTE from PUBLIC; before the existing
REVOKE on "anon" add an explicit REVOKE EXECUTE ON FUNCTION
public.invite_user_to_org(character varying, uuid, public.user_min_right) FROM
PUBLIC so the PUBLIC grant is removed first, then keep the existing REVOKE FROM
"anon" and the subsequent GRANTs to "authenticated" and "service_role"; adjust
the ACL order around the invite_user_to_org function to ensure PUBLIC cannot
execute it after CREATE OR REPLACE.
1 participant
Summary (AI generated)
public.invite_user_to_orgto remove org existence enumeration by returningNO_RIGHTSfor missingorg_idand unauthenticated callers.anonexecute access while keeping authenticated and service-role grants.OK,ALREADY_INVITED,NO_EMAIL, etc.).Test plan (AI generated)
bun lint:backend.Screenshots (AI generated)
Checklist (AI generated)
bun run lint:backend.Summary by CodeRabbit