Skip to content

fix/email inconsistency#385

Open
pavsoss wants to merge 3 commits into
Coder-s-OG-s:mainfrom
pavsoss:fix-profile-email
Open

fix/email inconsistency#385
pavsoss wants to merge 3 commits into
Coder-s-OG-s:mainfrom
pavsoss:fix-profile-email

Conversation

@pavsoss

@pavsoss pavsoss commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Description

Fixes the email inconsistency between Supabase Auth and the application profile model by persisting user email addresses on the profiles table.

Background jobs such as help-dispatch.ts cannot access the active Supabase session and therefore need a reliable way to resolve user email addresses. This change introduces an email field on profiles and keeps it synchronized from the authenticated user session during profile bootstrap.

Changes

  • Added a new migration to create the email column on profiles
  • Updated the Drizzle schema to include profiles.email
  • Updated bootstrapProfile to persist user.email from the Supabase Auth session during profile creation/update
  • Established a consistent email source for background workflows and future notification features

Type of Change

  • Bug fix
  • New feature
  • UI / UX improvement
  • Refactor
  • Documentation
  • Other

Related Issue

Closes #365

Checklist

  • My code follows the project structure and conventions
  • I tested this locally (npm run dev)
  • No hardcoded secrets or credentials
  • I have updated documentation if needed

@Ayush-Patel-56 Ayush-Patel-56 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bug fix is correct, but shipping it as-is creates a real PII leak that didn't exist before this PR. Needs the RLS/grant gap closed before merge.

Comment on lines +2 to +3
ALTER TABLE profiles
ADD COLUMN IF NOT EXISTS email text;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

profiles is select-using(true) plus anon has table grant select (migration 0021), so this column is world readable via the REST API with no login. Needs its own RLS-locked table or a column grant restriction before merge.

Comment thread src/app/actions/profile.ts Outdated
github_handle: githubHandle,
avatar_url: avatarUrl ?? null,
display_name: displayName ?? null,
email: user.email ?? null,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fine once the RLS gap above is closed, just flagging it writes the exposed column.

@Ayush-Patel-56 Ayush-Patel-56 added the Needs author reply Author need to reply label Jun 21, 2026
@pavsoss

pavsoss commented Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

@Ayush-Patel-56 Got it, how should i close the RLS gap

  1. Move email into a dedicated RLS-protected private tabl or
  2. Keep it on profiles and restrict access to the email column?

@Ayush-Patel-56

Copy link
Copy Markdown
Collaborator

Separate table. Help-dispatch already reads via service role, which skips RLS regardless of where the column lives, so email never needed to be on a publicly-grantable table in the first place. Column grants on profiles would still fight the existing anon blanket grant and RLS being row-level not column-level.

New table defaults to no anon access (no default-privileges statement in the migrations), so profile_emails with auth.uid() = id just works. Bootstrap writes to both, help-dispatch reads the new one.

@vercel

vercel Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

@pavsoss is attempting to deploy a commit to the codersogs-3057's projects Team on Vercel.

A member of the Team first needs to authorize it.

@jakharmonika364 jakharmonika364 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security design matches what @Ayush-Patel-56 discussed, looks right. Two things before merging: 0022_add_profile_email.sql/0023_profile_emails.sql collide with 0022_ai_pr_detection_setting.sql already on main, needs renumbering (and honestly, squashing into one migration since this PR adds then immediately drops profiles.email itself). Also, can you add a test that mocks profile_emails with a real row and asserts sendHelpDispatchEmail gets called with that address? Nothing currently exercises that path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs author reply Author need to reply

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Background jobs query profiles.email but no email column exists

3 participants