From 8b46d218126ec94358b406d9a0552be5ba984bc9 Mon Sep 17 00:00:00 2001 From: Georgy Butaev <41178744+g-but@users.noreply.github.com> Date: Wed, 26 Aug 2026 14:10:02 +0200 Subject: [PATCH] fix(profiles): close the two app-side paths that still minted handles from emails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The trigger fix (#769) was not enough, and the database said so: after applying it, count_email_derived_usernames() returned 77, not the 72 measured while writing it. Five accounts had acquired an email-derived handle in the intervening hour — because profiles are not only created by the handle_new_user trigger. Three sites were still deriving the PUBLIC handle from the address: services/profile/server.ts ensureProfile() creates a profile when one is missing, with username = the sanitized email local part. This is the one that was actively minting them. useProfileEditor.ts pre-filled the username FIELD with the useProfileWizard.ts email local part — one Save from publishing it. Only fires on a half-loaded profile (username is NOT NULL), so: a loaded gun with no purpose. All three now use neutralUsernameFor(userId) or an empty field. The display-name email fallback goes too — a name quietly set to someone's email prefix is the same leak wearing another label. neutralUsernameFor mirrors the SQL in 20260826130000 and says so in both directions, because two independent creation paths must not disagree about what a fresh handle looks like. Also corrects the ratchet baseline 72 -> 77. 72 was measured before the fix landed; a nightly gate set to it would have failed on its first run — a gate red about code that is fine, which is the exact habit this ratchet exists to avoid. 77 is a floor now rather than a moving target: no path mints these any more. The class is closed with a test, not a comment: any line in src/ that mentions a username and splits a string on '@' fails the suite. A comment would not have caught ensureProfile — nothing did, until the count moved. Verified by reintroducing the defect and watching the gate name the line. npm run verify green; 2369 tests pass. Co-Authored-By: Claude Opus 5 --- .../unit/profile/neutral-username.test.ts | 73 +++++++++++++++++++ scripts/check-data-invariants.mjs | 8 +- .../ProfileWizard/hooks/useProfileWizard.ts | 10 +-- .../profile/hooks/useProfileEditor.ts | 10 +-- src/lib/profile/neutral-username.ts | 24 ++++++ src/services/profile/server.ts | 16 ++-- 6 files changed, 122 insertions(+), 19 deletions(-) create mode 100644 __tests__/unit/profile/neutral-username.test.ts create mode 100644 src/lib/profile/neutral-username.ts diff --git a/__tests__/unit/profile/neutral-username.test.ts b/__tests__/unit/profile/neutral-username.test.ts new file mode 100644 index 000000000..a76a106f7 --- /dev/null +++ b/__tests__/unit/profile/neutral-username.test.ts @@ -0,0 +1,73 @@ +/** + * Public handles must not be minted from email addresses. + * + * `/profiles/` is served with no auth and robots.txt has no + * `/profiles` rule. Every profile-creation path used to set the handle to + * `email.split('@')[0]`, which published 77 people's email local parts as + * crawlable identifiers — reported through OrangeCat's own feedback widget. + */ +import { readFileSync, readdirSync, statSync } from 'node:fs'; +import { join } from 'node:path'; +import { neutralUsernameFor } from '@/lib/profile/neutral-username'; +import { isValidUsername } from '@/lib/validation/base'; + +describe('neutralUsernameFor', () => { + const id = '3f2a1b9c-7d4e-4a52-9c8b-0e1f2a3b4c5d'; + + it('carries nothing about the person', () => { + expect(neutralUsernameFor(id)).toBe('user_3f2a1b9c7d4e'); + }); + + it('is accepted by the app own username rules', () => { + expect(isValidUsername(neutralUsernameFor(id))).toBe(true); + }); + + it('mirrors the SQL trigger: user_ + 12 hex of the id, dashes stripped', () => { + // supabase/migrations/20260826130000: 'user_' || left(replace(id::text,'-',''), 12) + expect(neutralUsernameFor(id)).toBe(`user_${id.replace(/-/g, '').slice(0, 12)}`); + }); + + it('differs per user', () => { + expect(neutralUsernameFor(id)).not.toBe( + neutralUsernameFor('99999999-7d4e-4a52-9c8b-0e1f2a3b4c5d') + ); + }); +}); + +/** + * The class, closed. Fixing the DB trigger alone was NOT enough — `ensureProfile` + * and two form pre-fills each derived the handle from the email independently, + * and the count rose 72 -> 77 while the trigger fix was being written. A comment + * would not have caught that; this does. + */ +describe('no code derives a public handle from an email', () => { + const SRC = join(process.cwd(), 'src'); + const ALLOWED = join('lib', 'profile', 'neutral-username.ts'); // documents the bug in prose + + function sourceFiles(dir: string, out: string[] = []): string[] { + for (const name of readdirSync(dir)) { + const full = join(dir, name); + if (statSync(full).isDirectory()) sourceFiles(full, out); + else if (/\.tsx?$/.test(name)) out.push(full); + } + return out; + } + + it('has no site assigning a username from an email local part', () => { + const offenders: string[] = []; + for (const file of sourceFiles(SRC)) { + if (file.endsWith(ALLOWED)) continue; + readFileSync(file, 'utf8') + .split('\n') + .forEach((line, i) => { + const trimmed = line.trim(); + if (trimmed.startsWith('//') || trimmed.startsWith('*')) return; + // Same line mentions a handle AND splits an address on '@'. + if (/username/i.test(line) && /split\(\s*['"]@['"]\s*\)/.test(line)) { + offenders.push(`${file}:${i + 1}: ${trimmed}`); + } + }); + } + expect(offenders).toEqual([]); + }); +}); diff --git a/scripts/check-data-invariants.mjs b/scripts/check-data-invariants.mjs index 809ce0ca6..7083dbd38 100644 --- a/scripts/check-data-invariants.mjs +++ b/scripts/check-data-invariants.mjs @@ -289,7 +289,13 @@ async function checkSilentlyDroppedCatTurns() { * number RISES, which means a new write path started deriving handles from * emails again. */ -const EMAIL_DERIVED_USERNAME_BASELINE = 72; // measured on production 2026-08-26 +// Measured on production 2026-08-26 AFTER the trigger fix landed. The first +// reading taken while writing that fix said 72; five accounts signed up in the +// hour between, so a baseline of 72 would have failed on its very first +// nightly run — a gate red about code that is fine, which is the habit this +// ratchet exists to avoid. The number is a floor now rather than a moving +// target: handle_new_user no longer mints these, so nothing can add to it. +const EMAIL_DERIVED_USERNAME_BASELINE = 77; async function checkEmailDerivedUsernames() { const count = Number(await rpc('count_email_derived_usernames')); diff --git a/src/components/profile/ProfileWizard/hooks/useProfileWizard.ts b/src/components/profile/ProfileWizard/hooks/useProfileWizard.ts index a48ff45cc..ccc798082 100644 --- a/src/components/profile/ProfileWizard/hooks/useProfileWizard.ts +++ b/src/components/profile/ProfileWizard/hooks/useProfileWizard.ts @@ -31,11 +31,11 @@ export function useProfileWizard( resolver: zodResolver(profileSchema), mode: 'onChange', defaultValues: { - username: - profile.username || - (typeof userEmail === 'string' && userEmail.includes('@') - ? userEmail.split('@')[0] - : userEmail || ''), + // No email fallback: username is NOT NULL and always set by a creation + // path, so this only ever fired on a half-loaded profile — and when it + // did it pre-filled the PUBLIC handle field with the user's email local + // part, one Save away from publishing it. + username: profile.username || '', name: profile.name || '', bio: profile.bio || '', location_country: profile.location_country || '', diff --git a/src/components/profile/hooks/useProfileEditor.ts b/src/components/profile/hooks/useProfileEditor.ts index e8629452a..ff706d81f 100644 --- a/src/components/profile/hooks/useProfileEditor.ts +++ b/src/components/profile/hooks/useProfileEditor.ts @@ -73,11 +73,11 @@ export function useProfileEditor({ mode: 'onChange', reValidateMode: 'onChange', defaultValues: { - username: - profile.username || - (typeof userEmail === 'string' && userEmail.includes('@') - ? userEmail.split('@')[0] - : userEmail || ''), + // No email fallback: username is NOT NULL and always set by a creation + // path, so this only ever fired on a half-loaded profile — and when it + // did it pre-filled the PUBLIC handle field with the user's email local + // part, one Save away from publishing it. + username: profile.username || '', name: profile.name || '', bio: profile.bio || '', location_country: profile.location_country || '', diff --git a/src/lib/profile/neutral-username.ts b/src/lib/profile/neutral-username.ts new file mode 100644 index 000000000..150760526 --- /dev/null +++ b/src/lib/profile/neutral-username.ts @@ -0,0 +1,24 @@ +/** + * The handle a new account gets before its owner picks one. + * + * It must carry no personal information. `/profiles/` is served with + * no auth and robots.txt has no `/profiles` rule, so whatever goes here is + * published and crawlable. Deriving it from the email address — as every + * creation path used to — put 77 people's email local parts on the public web, + * reconstructable into real addresses with a handful of common domains. + * + * MIRRORS the SQL in + * supabase/migrations/20260826130000_stop_deriving_usernames_from_email.sql. + * Profiles are created from two independent places — the `handle_new_user` + * trigger on auth.users, and `ensureProfile()` when a profile is missing — and + * they must not disagree about what a fresh handle looks like. If you change + * the shape here, change it there and vice versa; the invariant gate + * (`count_email_derived_usernames`) catches the email case, not a cosmetic + * drift between the two. + */ + +/** `user_` + 12 hex chars of the id. Unique by construction, says nothing about + * the person, and satisfies the app's username pattern (^[a-zA-Z0-9_-]+$). */ +export function neutralUsernameFor(userId: string): string { + return `user_${String(userId).replace(/-/g, '').slice(0, 12)}`; +} diff --git a/src/services/profile/server.ts b/src/services/profile/server.ts index db6d6de5f..c44c0199c 100644 --- a/src/services/profile/server.ts +++ b/src/services/profile/server.ts @@ -13,6 +13,7 @@ import { DATABASE_TABLES } from '@/config/database-tables'; import { getTableName } from '@/config/entity-registry'; import { getOrCreateUserActor } from '@/services/actors/getOrCreateUserActor'; import { STATUS } from '@/config/database-constants'; +import { neutralUsernameFor } from '@/lib/profile/neutral-username'; type ProfileRow = Database['public']['Tables']['profiles']['Row']; type ProfileInsert = Database['public']['Tables']['profiles']['Insert']; @@ -138,18 +139,17 @@ export class ProfileServerService { // Profile doesn't exist, create it const safeEmail = typeof userEmail === 'string' ? userEmail : null; - const emailName = safeEmail && safeEmail.includes('@') ? safeEmail.split('@')[0] : null; - // Sanitize: replace dots and other invalid chars with underscores, keep only letters/numbers/underscores/hyphens - const sanitizedEmailName = emailName ? emailName.replace(/[^a-zA-Z0-9_-]/g, '_') : null; - const username = - sanitizedEmailName && sanitizedEmailName.length > 0 - ? sanitizedEmailName - : `user_${String(userId).slice(0, 8)}`; + // Never the email local part. This path creates profiles independently of + // the handle_new_user trigger, so fixing only the trigger left this one + // still publishing people's email prefixes as crawlable handles — which + // is what took the count from 72 to 77 while the trigger fix was being + // written. The display name gets no email fallback either: a name quietly + // set to someone's email prefix is the same leak wearing another label. + const username = neutralUsernameFor(userId); const name = (userMetadata?.full_name as string | undefined) || (userMetadata?.name as string | undefined) || (userMetadata?.display_name as string | undefined) || - (emailName && emailName.length > 0 ? emailName : null) || 'User'; const profileData: ProfileInsert = {