Add Row Level Security policy to profiles table - #1876
Conversation
|
@anshul23102 is attempting to deploy a commit to the durdana3105's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Thank you for reviewing this security fix! This PR implements critical Row Level Security policies on the profiles table to prevent unauthorized access to user emails and profile metadata. Security Impact:
When ready, please review the changes and consider adding these labels:
Thank you! |
|
Warning Review limit reached
Next review available in: 47 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a Supabase migration that enables RLS on ChangesProfiles RLS enforcement
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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 |
Drop 'Public profiles are viewable by everyone' policy Replace with secure 'profiles_select_own' policy Restrict reads to authenticated user's own profile (auth.uid() = id) Prevents unauthorized email and profile metadata exposure via anon API key
f5ae513 to
5d6213a
Compare
![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
🧹 Nitpick comments (2)
supabase/migrations/20260729000000_add_profiles_rls_policy.sql (2)
34-38: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate INSERT policy; also no
DROP POLICY IF EXISTSguard, unlike prior migrations.
profiles_insert/"Users can insert their own profile."(from the two prior migrations) already enforceauth.uid() = idon INSERT, sousers_create_own_profileis functionally redundant. More importantly, unlike20260518000002_role_management.sqland itsDROP POLICY IF EXISTSpattern, none of the policies in this file are preceded by a drop, so re-applying this migration (e.g., in a fresh environment reset or repair) will fail with "policy already exists."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@supabase/migrations/20260729000000_add_profiles_rls_policy.sql` around lines 34 - 38, Remove the redundant users_create_own_profile INSERT policy, since profiles_insert and "Users can insert their own profile." already enforce the same condition. Add DROP POLICY IF EXISTS guards before each policy created in this migration, following the established role-management migration pattern so reapplication succeeds.
21-32: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueService-role policies are dead code —
service_rolealways bypasses RLS in Supabase.Supabase docs state Also note that adding service_role in RLS policies does nothing. Since the service role never evaluates policies at all,
service_role_read_all_profilesandservice_role_update_profileshave no effect and just add misleading documentation-as-code about the security posture (line 44 comment implies these policies are what grant service-role access).Consider removing these two policies and replacing the comment with a note that service_role access is inherent to the role, not policy-driven.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@supabase/migrations/20260729000000_add_profiles_rls_policy.sql` around lines 21 - 32, Remove the service_role_read_all_profiles and service_role_update_profiles policies from the migration, since service_role access bypasses RLS inherently. Replace their explanatory comments with a concise note stating that service_role access is provided by the role and is not policy-driven.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@supabase/migrations/20260729000000_add_profiles_rls_policy.sql`:
- Around line 8-12: Update the migration containing users_read_own_profile to
drop the existing permissive SELECT policies profiles_select and "Public
profiles are viewable by everyone." with DROP POLICY IF EXISTS on profiles
before or alongside creating the restrictive policy. Preserve
users_read_own_profile as the policy allowing users to select only rows where
auth.uid() = id.
- Around line 14-19: The profiles UPDATE policy must not create a permissive
path around the earlier policy’s column-level WITH CHECK guards. Update the
migration to either drop and replace the existing “Users can update their own
profile.” policy with equivalent ownership and column protections, or omit
users_update_own_profile when the existing policy already permits updates to
owned profiles.
---
Nitpick comments:
In `@supabase/migrations/20260729000000_add_profiles_rls_policy.sql`:
- Around line 34-38: Remove the redundant users_create_own_profile INSERT
policy, since profiles_insert and "Users can insert their own profile." already
enforce the same condition. Add DROP POLICY IF EXISTS guards before each policy
created in this migration, following the established role-management migration
pattern so reapplication succeeds.
- Around line 21-32: Remove the service_role_read_all_profiles and
service_role_update_profiles policies from the migration, since service_role
access bypasses RLS inherently. Replace their explanatory comments with a
concise note stating that service_role access is provided by the role and is not
policy-driven.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 10ecef92-b6b8-44de-ba3b-0b31e5591447
📒 Files selected for processing (1)
supabase/migrations/20260729000000_add_profiles_rls_policy.sql
| -- Policy: Users can read their own profile | ||
| CREATE POLICY "users_read_own_profile" | ||
| ON profiles | ||
| FOR SELECT | ||
| USING (auth.uid() = id); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
This does not fix the exposed-PII vulnerability: old permissive USING (true) SELECT policies are never dropped.
public.profiles already has two permissive SELECT policies from prior migrations — profiles_select (supabase/migrations/20260518000002_role_management.sql) and "Public profiles are viewable by everyone." (supabase/migrations/20260617000000_consolidate_rls_policies.sql) — both USING (true). PostgreSQL combines multiple permissive policies for the same command with OR, so adding users_read_own_profile alongside them changes nothing: any role with SELECT grant (including anon) can still read every row via the pre-existing true policy. This migration must DROP POLICY IF EXISTS on the old permissive SELECT policies before/while adding the new restrictive one, or the anon-key PII exposure from issue #1870 remains unpatched.
🛡️ Proposed fix
+DROP POLICY IF EXISTS "profiles_select" ON public.profiles;
+DROP POLICY IF EXISTS "Public profiles are viewable by everyone." ON public.profiles;
+
-- Policy: Users can read their own profile
+DROP POLICY IF EXISTS "users_read_own_profile" ON public.profiles;
CREATE POLICY "users_read_own_profile"
ON profiles
FOR SELECT
USING (auth.uid() = id);#!/bin/bash
# Confirm the permissive SELECT/UPDATE/INSERT policies from prior migrations still exist and aren't dropped anywhere.
fd -e sql . supabase/migrations | sort
rg -n -A2 'CREATE POLICY' supabase/migrations/20260518000002_role_management.sql supabase/migrations/20260617000000_consolidate_rls_policies.sql🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@supabase/migrations/20260729000000_add_profiles_rls_policy.sql` around lines
8 - 12, Update the migration containing users_read_own_profile to drop the
existing permissive SELECT policies profiles_select and "Public profiles are
viewable by everyone." with DROP POLICY IF EXISTS on profiles before or
alongside creating the restrictive policy. Preserve users_read_own_profile as
the policy allowing users to select only rows where auth.uid() = id.
| -- Policy: Users can update their own profile | ||
| CREATE POLICY "users_update_own_profile" | ||
| ON profiles | ||
| FOR UPDATE | ||
| USING (auth.uid() = id) | ||
| WITH CHECK (auth.uid() = id); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== migration files =="
git ls-files | grep -E 'supabase/migrations/.*(add_profiles_rls_policy|consolidate_rls_policies)\.sql$' || true
echo
echo "== target migration =="
cat -n supabase/migrations/20260729000000_add_profiles_rls_policy.sql
echo
echo "== consolidated RLS migration candidates =="
for f in supabase/migrations/20260617000000_consolidate_rls_policies.sql; do
if [ -f "$f" ]; then
echo "--- $f ---"
cat -n "$f"
fi
done
echo
echo "== all profile UPDATE policies =="
rg -n 'CREATE POLICY .*on profiles|FOR UPDATE|WITH CHECK|profiles' supabase/migrations || trueRepository: durdana3105/peer-learning
Length of output: 1936
🌐 Web query:
PostgreSQL RLS policy multiple WITH CHECK clauses combined with OR UPDATE
💡 Result:
In PostgreSQL, an UPDATE operation relies on both USING and WITH CHECK expressions to manage row access and modification [1][2]. When multiple policies are defined for an UPDATE operation, PostgreSQL combines their expressions according to specific rules based on the type of policy (PERMISSIVE vs. RESTRICTIVE) [1][3]. 1. Understanding USING and WITH CHECK for UPDATE The USING expression determines which existing rows the UPDATE operation can see and modify [1][2]. The WITH CHECK expression defines the constraints that the new (updated) row values must satisfy to be successfully stored back into the table [1][2]. If no explicit WITH CHECK clause is provided for an UPDATE policy, the USING clause is used for both purposes [2][4]. 2. Combining Multiple Policies When multiple policies apply to the same command (such as UPDATE), they are combined as follows: - Permissive vs. Restrictive: PostgreSQL distinguishes between PERMISSIVE policies (the default) and RESTRICTIVE policies [1][3]. - PERMISSIVE Policies: All applicable PERMISSIVE policy expressions for a specific command type are combined using the OR operator [1][3]. - RESTRICTIVE Policies: All applicable RESTRICTIVE policy expressions are combined using the AND operator [1][3]. - Final Combination: The result of the combined PERMISSIVE policies and the combined RESTRICTIVE policies are then joined using the AND operator [1][3]. 3. Combining Different Command Types If an operation requires multiple permissions (e.g., an UPDATE command typically requires both SELECT and UPDATE permissions), PostgreSQL combines the expressions for each command type first using the rules above, and then joins those results using the AND operator [1][3]. In practice, if you define multiple UPDATE policies, the database ensures that at least one PERMISSIVE policy grants access (or evaluates to true) and that all RESTRICTIVE policies are satisfied [1][3]. If you use multiple WITH CHECK clauses across different policies, they effectively become part of the collective set of constraints that must be satisfied for the update to proceed [1][3].
Citations:
- 1: https://www.postgresql.org/docs/current/sql-createpolicy.html
- 2: https://www.postgresql.org/docs/19/sql-createpolicy.html
- 3: https://www.postgresql.org/docs/17/sql-createpolicy.html
- 4: https://www.postgresql.org/docs/14/sql-createpolicy.html
🌐 Web query:
PostgreSQL CREATE POLICY "Users can update their own profile" WITH CHECK profiles auth.uid
💡 Result:
To correctly implement a Row Level Security (RLS) policy that allows users to update only their own profile, it is recommended to use both USING and WITH CHECK clauses [1][2][3]. The USING clause determines which existing rows the user has permission to update, while the WITH CHECK clause ensures that the resulting row after the update still satisfies the policy constraints [1][3][4]. Omitting the USING clause in an UPDATE policy can create a security vulnerability where unauthorized users may be able to modify rows they should not have access to [2]. Below is the recommended SQL syntax for a profiles table: CREATE POLICY "Users can update their own profile" ON profiles FOR UPDATE TO authenticated USING (auth.uid = id) WITH CHECK (auth.uid = id); In this example: - auth.uid() is a function commonly used in Supabase to retrieve the current user's ID from their JWT [5][6][4]. - id should be replaced with the column name used to store the user's ID (e.g., user_id) [4]. - The TO authenticated clause restricts this policy to logged-in users [1][5]. If no WITH CHECK expression is explicitly defined, PostgreSQL defaults to using the USING expression for both checks [1][7]. However, explicitly defining both is considered a best practice to prevent data hijacking and ensure clarity regarding which condition governs row visibility versus row modification [2][3]. Note that for an UPDATE operation to function as expected, a corresponding SELECT policy must typically also be in place [1][8].
Citations:
- 1: https://supabase.com/docs/guides/database/postgres/row-level-security
- 2: https://zenn.dev/cosoado/articles/supabase-rls-with-check-vs-using-pitfall?locale=en
- 3: https://audityour.app/guides/supabase-rls-complete-guide
- 4: https://hrekov.com/blog/rls-policies-supabase
- 5: https://github.com/supabase/supabase/blob/master/examples/prompts/database-rls-policies.md
- 6: https://devcheolu.com/en/posts/79VUi7CkUNJKlZYIATHV
- 7: https://www.postgresql.org/docs/current/ddl-rowsecurity.html
- 8: https://github.com/supabase/supabase/blob/master/apps/docs/content/guides/database/postgres/row-level-security.mdx
Avoid weakening profile UPDATE RLS with a permissive duplicate policy.
If the earlier "Users can update their own profile." UPDATE policy contains column-level WITH CHECK guards, creating another unrestricted permissive UPDATE policy on profiles without dropping or replacing it will let users bypass those guards, because permissive update policies combine with OR. Either drop/replace the prior policy with an equivalent protected version, or omit this new one if the existing policy already allows owned profile updates.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@supabase/migrations/20260729000000_add_profiles_rls_policy.sql` around lines
14 - 19, The profiles UPDATE policy must not create a permissive path around the
earlier policy’s column-level WITH CHECK guards. Update the migration to either
drop and replace the existing “Users can update their own profile.” policy with
equivalent ownership and column protections, or omit users_update_own_profile
when the existing policy already permits updates to owned profiles.
2ad4cfe to
5d6213a
Compare
2 participants
Fixes #1870
Summary
Implements Row Level Security (RLS) policies on the profiles table to prevent unauthorized access to user emails and profile metadata. This closes a critical data exposure vulnerability where any visitor using the exposed VITE_SUPABASE_ANON_KEY could retrieve all user records.
Security Fix
Problem
The profiles table lacked RLS policies, allowing unrestricted read access to all user records via the public anon API key:
Solution
Added comprehensive RLS policies:
auth.uid() = id)Implementation
New migration:
supabase/migrations/20260729000000_add_profiles_rls_policy.sqlPolicies implemented:
users_read_own_profile- SELECT USING (auth.uid() = id)users_update_own_profile- UPDATE with auth.uid() checkusers_create_own_profile- INSERT with auth.uid() checkservice_role_read_all_profiles- Backend accessservice_role_update_profiles- Backend modificationImpact
Testing
Summary by CodeRabbit