fix: prevent mass assignment in updateProfile#2902
Conversation
Whitelist only safe fields (full_name, avatar_url, phone, company) before passing updates to Supabase. Prevents privilege escalation via role/status/company_id injection. Fixes ritesh-1918#2894
|
@Nancy-3012 is attempting to deploy a commit to the ritesh Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthrough
ChangesFrontend profile update field allowlist
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Frontend/src/store/authStore.js`:
- Around line 252-256: The updateProfile function in authStore.js silently drops
avatar updates because the avatar upload flow in Profile.jsx sends the data with
the key profile_picture, but the ALLOWED_FIELDS allowlist at line 252 expects
avatar_url. Before filtering updates against ALLOWED_FIELDS, add logic to
normalize the legacy key by mapping profile_picture to avatar_url so that avatar
updates are properly included in safeUpdates instead of being filtered out and
causing an early return at line 256.
- Around line 247-265: The updateProfile function implements client-side field
whitelisting which is good practice but insufficient for security. Add Row-Level
Security (RLS) policies to the profiles table in your Supabase database to
enforce server-side authorization that prevents unauthorized users from
modifying other users' profiles. Create an RLS policy that allows users to
update only their own profile records (matching user ID) or allows admin users
to update any profile, ensuring the eq('id', profile.id) condition cannot be
bypassed by direct API calls that circumvent the frontend filtering.
🪄 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
Run ID: 6941364a-e8b8-4c6a-aa75-b1bc6e86fdfa
📒 Files selected for processing (1)
Frontend/src/store/authStore.js
| updateProfile: async (updates) => { | ||
| const { profile } = get(); | ||
| if (!profile?.id) return; | ||
|
|
||
| set({ loading: true }); | ||
| try { | ||
| const { data, error } = await supabase | ||
| .from('profiles') | ||
| .update(updates) | ||
| .eq('id', profile.id) | ||
| .select() | ||
| .single(); | ||
| const { profile } = get(); | ||
| if (!profile?.id) return; | ||
|
|
||
| // Whitelist: only permit safe, user-editable fields | ||
| const ALLOWED_FIELDS = ['full_name', 'avatar_url', 'phone', 'company']; | ||
| const safeUpdates = Object.fromEntries( | ||
| Object.entries(updates).filter(([key]) => ALLOWED_FIELDS.includes(key)) | ||
| ); | ||
| if (Object.keys(safeUpdates).length === 0) return; | ||
|
|
||
| set({ loading: true }); | ||
| try { | ||
| const { data, error } = await supabase | ||
| .from('profiles') | ||
| .update(safeUpdates) | ||
| .eq('id', profile.id) | ||
| .select() | ||
| .single(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Read-only reconnaissance for Supabase RLS/policy evidence in repo
rg -n -C3 "create policy|alter table.*profiles.*enable row level security|using \\(|with check \\(|on profiles|profiles\\)" --type sql
rg -n -C3 "from\\('profiles'\\)\\.update|PATCH\\s+/profiles|updateProfile\\s*:" --type js --type ts --type pyRepository: ritesh-1918/HELPDESK.AI
Length of output: 3926
🏁 Script executed:
# Search for profiles table RLS/policy configuration
find . -name "*.sql" -type f | head -20Repository: ritesh-1918/HELPDESK.AI
Length of output: 562
🏁 Script executed:
# Look for profiles table migration/creation
rg -n "create table.*profiles|alter table.*profiles|enable row level security" --type sql -A 5 -B 2Repository: ritesh-1918/HELPDESK.AI
Length of output: 784
🏁 Script executed:
# Search for any policies on profiles table
rg -n "create policy.*profiles|on.*profiles.*for" --type sql -A 3Repository: ritesh-1918/HELPDESK.AI
Length of output: 49
🏁 Script executed:
# Check the remote schema migration for profiles table definition
cat supabase/migrations/20260330131213_remote_schema.sql | head -100Repository: ritesh-1918/HELPDESK.AI
Length of output: 49
🏁 Script executed:
# Search for profiles table definition and RLS in all migration files
rg -n "profiles" supabase/migrations/ --type sql -B 2 -A 5 | head -150Repository: ritesh-1918/HELPDESK.AI
Length of output: 835
🏁 Script executed:
# Search more broadly for profiles table definition
rg -n "\"profiles\"" --type sql | head -50Repository: ritesh-1918/HELPDESK.AI
Length of output: 49
🏁 Script executed:
# Check all migration files for profiles table structure
for file in supabase/migrations/*.sql; do echo "=== $file ==="; grep -n "profiles" "$file" | head -10; doneRepository: ritesh-1918/HELPDESK.AI
Length of output: 636
🏁 Script executed:
# Look for any RLS/policy mentions in all migration files
rg -n "row level security|enable rls|policy.*profiles" --type sqlRepository: ritesh-1918/HELPDESK.AI
Length of output: 189
🏁 Script executed:
# Check remote_schema migration content
wc -l supabase/migrations/20260330131213_remote_schema.sql
cat supabase/migrations/20260330131213_remote_schema.sqlRepository: ritesh-1918/HELPDESK.AI
Length of output: 122
🏁 Script executed:
# Search for backend authorization checks on profiles
rg -n "profiles.*update|authorization.*profiles|profile.*check" --type js --type ts --type py -A 2 -B 1Repository: ritesh-1918/HELPDESK.AI
Length of output: 1257
🏁 Script executed:
# Check for any function-based security on profiles
rg -n "create.*function|before.*update.*profiles" --type sqlRepository: ritesh-1918/HELPDESK.AI
Length of output: 283
🏁 Script executed:
# Check the full authStore.js to see if there's authorization checking
sed -n '240,280p' Frontend/src/store/authStore.jsRepository: ritesh-1918/HELPDESK.AI
Length of output: 1366
🏁 Script executed:
# Search for any checks ensuring user can only update their own profile
rg -n "auth\\.user|getCurrentUser|user\\.id.*profile\\.id" --type js --type ts -B 2 -A 2 | head -80Repository: ritesh-1918/HELPDESK.AI
Length of output: 1988
🏁 Script executed:
# Check if profiles table might be related to auth.users
rg -n "auth\\.users|user_id.*=.*auth\\.uid" --type sqlRepository: ritesh-1918/HELPDESK.AI
Length of output: 361
Add RLS policies to the profiles table to enforce server-side authorization.
The client-side whitelist is good practice, but it is not a security boundary. The profiles table lacks row-level security (RLS) policies entirely. Direct Supabase API calls can bypass frontend filtering and exploit the eq('id', profile.id) condition. Implement RLS policies enforcing "self or admin only" access to profile updates at the database level to prevent unauthorized modifications.
🤖 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 `@Frontend/src/store/authStore.js` around lines 247 - 265, The updateProfile
function implements client-side field whitelisting which is good practice but
insufficient for security. Add Row-Level Security (RLS) policies to the profiles
table in your Supabase database to enforce server-side authorization that
prevents unauthorized users from modifying other users' profiles. Create an RLS
policy that allows users to update only their own profile records (matching user
ID) or allows admin users to update any profile, ensuring the eq('id',
profile.id) condition cannot be bypassed by direct API calls that circumvent the
frontend filtering.
| const ALLOWED_FIELDS = ['full_name', 'avatar_url', 'phone', 'company']; | ||
| const safeUpdates = Object.fromEntries( | ||
| Object.entries(updates).filter(([key]) => ALLOWED_FIELDS.includes(key)) | ||
| ); | ||
| if (Object.keys(safeUpdates).length === 0) return; |
There was a problem hiding this comment.
Avatar updates are silently dropped due to key mismatch.
At Line 252, the allowlist permits avatar_url, but the avatar upload flow calls updateProfile({ profile_picture: publicUrl }) (Frontend/src/user/pages/Profile.jsx, avatar handler). That makes safeUpdates empty and returns early at Line 256, so the UI can show success while nothing is persisted.
Suggested fix (normalize legacy key before filtering)
- const ALLOWED_FIELDS = ['full_name', 'avatar_url', 'phone', 'company'];
- const safeUpdates = Object.fromEntries(
- Object.entries(updates).filter(([key]) => ALLOWED_FIELDS.includes(key))
- );
+ const normalizedUpdates = { ...updates };
+ if (normalizedUpdates.profile_picture && !normalizedUpdates.avatar_url) {
+ normalizedUpdates.avatar_url = normalizedUpdates.profile_picture;
+ }
+
+ const ALLOWED_FIELDS = ['full_name', 'avatar_url', 'phone', 'company'];
+ const safeUpdates = Object.fromEntries(
+ Object.entries(normalizedUpdates).filter(([key]) => ALLOWED_FIELDS.includes(key))
+ );📝 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.
| const ALLOWED_FIELDS = ['full_name', 'avatar_url', 'phone', 'company']; | |
| const safeUpdates = Object.fromEntries( | |
| Object.entries(updates).filter(([key]) => ALLOWED_FIELDS.includes(key)) | |
| ); | |
| if (Object.keys(safeUpdates).length === 0) return; | |
| const normalizedUpdates = { ...updates }; | |
| if (normalizedUpdates.profile_picture && !normalizedUpdates.avatar_url) { | |
| normalizedUpdates.avatar_url = normalizedUpdates.profile_picture; | |
| } | |
| const ALLOWED_FIELDS = ['full_name', 'avatar_url', 'phone', 'company']; | |
| const safeUpdates = Object.fromEntries( | |
| Object.entries(normalizedUpdates).filter(([key]) => ALLOWED_FIELDS.includes(key)) | |
| ); | |
| if (Object.keys(safeUpdates).length === 0) return; |
🤖 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 `@Frontend/src/store/authStore.js` around lines 252 - 256, The updateProfile
function in authStore.js silently drops avatar updates because the avatar upload
flow in Profile.jsx sends the data with the key profile_picture, but the
ALLOWED_FIELDS allowlist at line 252 expects avatar_url. Before filtering
updates against ALLOWED_FIELDS, add logic to normalize the legacy key by mapping
profile_picture to avatar_url so that avatar updates are properly included in
safeUpdates instead of being filtered out and causing an early return at line
256.
1 participant
Whitelist only safe fields (full_name, avatar_url, phone, company) before passing updates to Supabase. Prevents privilege escalation via role/status/company_id injection.
Fixes #2894
Summary by CodeRabbit