diff --git a/src/backend/controllers/auth/AuthController.ts b/src/backend/controllers/auth/AuthController.ts index 25b07a932..091a5dbcf 100644 --- a/src/backend/controllers/auth/AuthController.ts +++ b/src/backend/controllers/auth/AuthController.ts @@ -22,7 +22,6 @@ import type { Request, RequestHandler, Response } from 'express'; import crypto from 'node:crypto'; import { posix as pathPosix } from 'node:path'; import { v4 as uuidv4, validate as validateUuid } from 'uuid'; -import validator from 'validator'; import { Controller, Get, Post } from '../../core/http/decorators.js'; import type { HttpErrorOptions } from '../../core/http/HttpError.js'; import { HttpError } from '../../core/http/HttpError.js'; @@ -77,7 +76,11 @@ import { phoneAttemptsKey, } from '../../util/cardFallback.js'; import { sessionCookieFlags } from '../../util/cookieFlags.js'; -import { cleanEmail, isBlockedEmail } from '../../util/email.js'; +import { + cleanEmail, + isBlockedEmail, + isStorableEmail, +} from '../../util/email.js'; import { isGodmodeApp } from '../../util/godmodeApps.js'; import { generate_identifier } from '../../util/identifier.js'; import { parsePhone } from '../../util/phone.js'; @@ -802,7 +805,7 @@ export class AuthController extends PuterController { throw new HttpError(400, 'email must be a string.', { legacyCode: 'bad_request', }); - if (!validator.isEmail(body.email)) + if (!isStorableEmail(body.email)) throw new HttpError( 400, 'Please enter a valid email address.', @@ -2414,7 +2417,7 @@ export class AuthController extends PuterController { if (username) { user = await this.stores.user.getByUsername(username); } else { - if (!validator.isEmail(email)) + if (!isStorableEmail(email)) throw new HttpError(400, 'Invalid email.', { legacyCode: 'bad_request', }); @@ -2808,7 +2811,7 @@ export class AuthController extends PuterController { legacyCode: 'bad_request', }); } - if (!validator.isEmail(new_email)) { + if (!isStorableEmail(new_email)) { throw new HttpError(400, 'Please enter a valid email address.', { legacyCode: 'bad_request', }); @@ -3053,7 +3056,7 @@ export class AuthController extends PuterController { legacyCode: 'username_already_in_use', }); } - if (!email || !validator.isEmail(email)) { + if (!email || !isStorableEmail(email)) { throw new HttpError(400, 'Please enter a valid email address.', { legacyCode: 'bad_request', }); diff --git a/src/backend/services/share/ShareService.ts b/src/backend/services/share/ShareService.ts index feed605a3..7a0e33900 100644 --- a/src/backend/services/share/ShareService.ts +++ b/src/backend/services/share/ShareService.ts @@ -280,7 +280,8 @@ export const DEFAULT_DAILY_SHARE_LIMIT = 200; * Deliverability is the inbox's business, but `a@b` or a pasted sentence must * not become a permanent pending share that spent quota. */ -const EMAIL_SHAPE = /^[^\s@]+@[^\s@]+\.[^\s@]+$/u; +// `"<>` excluded too: legal only in a quoted local part nothing uses. +const EMAIL_SHAPE = /^[^\s@"<>]+@[^\s@"<>]+\.[^\s@"<>]+$/u; /** * Where "refuse shares from everyone" lives on the user row. diff --git a/src/backend/services/team/TeamProvisioning.test.ts b/src/backend/services/team/TeamProvisioning.test.ts index 196b6b2fc..aa08a7250 100644 --- a/src/backend/services/team/TeamProvisioning.test.ts +++ b/src/backend/services/team/TeamProvisioning.test.ts @@ -95,6 +95,25 @@ describe('team account provisioning: home path conflicts', () => { ).resolves.toBeNull(); }); + it('refuses a seat email that could render as markup', async () => { + const owner = await makeUser(); + const team = await makeTeam(owner.id); + const username = `tp_${Math.random().toString(36).slice(2, 10)}`; + + // A quoted local part passes `validator.isEmail`; it must not store. + await expect( + service.provisionAccount(team.uid, owner.id, { + username, + email: '""@example.com', + }), + ).rejects.toMatchObject({ statusCode: 400 }); + + // Refused before any write, so the name is still free. + await expect( + server.stores.user.getByUsername(username, { force: true }), + ).resolves.toBeNull(); + }); + it('does not suggest a name whose home path is occupied', async () => { const owner = await makeUser(); await makeTeam(owner.id); diff --git a/src/backend/services/team/TeamService.ts b/src/backend/services/team/TeamService.ts index 9c7160fc0..3bcad37bb 100644 --- a/src/backend/services/team/TeamService.ts +++ b/src/backend/services/team/TeamService.ts @@ -18,7 +18,6 @@ */ import bcrypt from 'bcrypt'; -import validator from 'validator'; import { v4 as uuidv4 } from 'uuid'; import { USERNAME_MAX_LENGTH, @@ -56,7 +55,7 @@ import { type PageResult, } from '../../util/pagination.js'; import type { UserRow } from '../../stores/user/UserStore'; -import { cleanEmail } from '../../util/email.js'; +import { cleanEmail, isStorableEmail } from '../../util/email.js'; import { generateTemporaryPassword, temporaryPasswordExpiry, @@ -820,18 +819,16 @@ export class TeamService extends PuterService { id === null ? null : (users.get(id)?.username ?? null); return { - items: page.items.map( - (row): MemberActivityEntry => ({ - action: row.action, - reason: row.reason, - created_at: epochSeconds(row.created_at), - username: name(row.user_id_keep), - actor_username: name(row.actor_user_id), - // Only a sign-in carries these; the shape stays uniform. - ip: null, - user_agent: null, - }), - ), + items: page.items.map((row): MemberActivityEntry => ({ + action: row.action, + reason: row.reason, + created_at: epochSeconds(row.created_at), + username: name(row.user_id_keep), + actor_username: name(row.actor_user_id), + // Only a sign-in carries these; the shape stays uniform. + ip: null, + user_agent: null, + })), ...(page.cursor ? { cursor: page.cursor } : {}), }; } @@ -917,7 +914,7 @@ export class TeamService extends PuterService { this.#assertUsableUsername(input.username); const email = typeof input.email === 'string' ? input.email.trim() : ''; - if (email && !validator.isEmail(email)) { + if (email && !isStorableEmail(email)) { throw new HttpError(400, 'Invalid email', { legacyCode: 'bad_request', }); diff --git a/src/backend/util/email.test.ts b/src/backend/util/email.test.ts index c397cc48d..7b7882281 100644 --- a/src/backend/util/email.test.ts +++ b/src/backend/util/email.test.ts @@ -18,7 +18,8 @@ */ import { describe, expect, it } from 'vitest'; -import { cleanEmail, isBlockedEmail } from './email.ts'; +import validator from 'validator'; +import { cleanEmail, isBlockedEmail, isStorableEmail } from './email.ts'; describe('cleanEmail', () => { it('lowercases the whole address', () => { @@ -94,3 +95,28 @@ describe('isBlockedEmail', () => { ).toBe(true); }); }); + +describe('isStorableEmail', () => { + const markup = '""@example.com'; + + it('refuses a quoted local part that carries markup', () => { + // The reason this gate exists: the library on its own says yes. + expect(validator.isEmail(markup)).toBe(true); + expect(isStorableEmail(markup)).toBe(false); + }); + + it('refuses the bare metacharacters too', () => { + for (const bad of ['ab@example.com', 'a"b@example.com']) + expect(isStorableEmail(bad)).toBe(false); + }); + + it('still accepts addresses people actually have', () => { + for (const good of [ + "o'brien@example.com", + 'a&b@example.com', + 'foo.bar+tag@gmail.com', + 'user_name-1@sub.example.co.uk', + ]) + expect(isStorableEmail(good)).toBe(true); + }); +}); diff --git a/src/backend/util/email.ts b/src/backend/util/email.ts index 201fd9af7..aeeb41237 100644 --- a/src/backend/util/email.ts +++ b/src/backend/util/email.ts @@ -25,6 +25,8 @@ * isBlockedEmail('temp@mailinator.com', ['mailinator.com']) === true */ +import validator from 'validator'; + type RuleName = 'dots_dont_matter' | 'remove_subaddressing'; interface Parts { @@ -103,6 +105,11 @@ export function cleanEmail(email: string): string { return `${parts.local}@${parts.domain}`; } +/** Storage gate; `isEmail` alone passes quoted local parts carrying markup. */ +export function isStorableEmail(email: string): boolean { + return !/["<>]/u.test(email) && validator.isEmail(email); +} + /** Strips `+` on any domain. For abuse decisions only, never for identity. */ export function abuseKey(email: string): string { const [local, domain] = cleanEmail(email).split('@');