validation in some areas, maybe move this to schema later

2026-09-08 06:31:27 -07:00 · 2026-08-05 12:08:04 -04:00
parent 079c889ccb
commit 6bfd4d9e50
16 changed files with 553 additions and 39 deletions
@@ -8,6 +8,11 @@ import {
 	getAccount,
 	getAccountByUsername,
 	getAccountsByIds,
+	isValidBio,
+	isValidEmail,
+	MAX_DISPLAY_NAME_LENGTH,
+	MAX_USERNAME_LENGTH,
+	nameRejection,
 	searchAccounts,
 	updateAccount,
 } from '@repo/domain'
@@ -422,7 +427,7 @@ const app = new Hono<App>()
 			requestBody: form(DisplayNameRequest, 'The new display name'),
 			responses: {
 				200: json(SuccessResponse, 'Updated'),
-				400: { description: 'Empty display name (empty body)' },
+				400: { description: 'Empty, over 15 characters, or non-alphanumeric (empty body)' },
 				401: UNAUTHORIZED_RESPONSE,
 			},
 		}),
@@ -431,6 +436,14 @@ const app = new Hono<App>()
 			if (id === null) return unauthorized(c)
 			const displayName = (await formField(c, 'displayName')).trim()
 			if (displayName === '') return c.body(null, 400)
+			// Held to the same shape as the username — it's the name other players actually
+			// see, so it can't be the place where spaces and punctuation get in. Refused with
+			// an empty 400 like the empty case above: this route answers a bare
+			// SuccessResponse, and giving it a body the client has never been sent is a
+			// bigger change than the rule is worth.
+			if (nameRejection(displayName, 'display name', MAX_DISPLAY_NAME_LENGTH) !== null) {
+				return c.body(null, 400)
+			}
 			const account = await updateAccount(c.env.DB, id, { displayName })
 			await pushAccountUpdate(c, account)
 			return c.json({ success: true })
@@ -446,9 +459,10 @@ const app = new Hono<App>()
 			tags: ['Profile'],
 			summary: 'Change username',
 			description: [
-				'Rejects a name taken by another account and requires a remaining change; on',
-				'success the name is persisted and the counter decremented. Always HTTP 200 —',
-				'failures carry a message in `error` (see the UsernameResult envelope).',
+				'Letters and digits only, at most 50 characters. Rejects a name taken by another',
+				'account and requires a remaining change; on success the name is persisted and',
+				'the counter decremented. Always HTTP 200 — failures carry a message in `error`',
+				'(see the UsernameResult envelope).',
 			].join(' '),
 			security: AUTHED,
 			requestBody: form(UsernameRequest, 'The desired username'),
@@ -464,6 +478,12 @@ const app = new Hono<App>()
 			const username = (await formField(c, 'username')).trim()
 			if (username === '') return usernameResult(c, 'You must enter a username.')

+			// Shape before availability: a rejected name shouldn't cost a D1 read, and it
+			// must never cost the account one of its rationed changes. The envelope carries
+			// the sentence straight to the player.
+			const rejection = nameRejection(username, 'username', MAX_USERNAME_LENGTH)
+			if (rejection !== null) return usernameResult(c, rejection)
+
 			// Duplicate check first (case-insensitive); keeping your own name is allowed.
 			const existing = await getAccountByUsername(c.env.DB, username)
 			if (existing && existing.accountId !== id) {
@@ -497,7 +517,7 @@ const app = new Hono<App>()
 			requestBody: form(EmailRequest, 'The new email'),
 			responses: {
 				200: json(SuccessResponse, 'Updated'),
-				400: { description: 'Email without an “@” (empty body)' },
+				400: { description: 'Not a valid address, or over 255 characters (empty body)' },
 				401: UNAUTHORIZED_RESPONSE,
 			},
 		}),
@@ -505,7 +525,7 @@ const app = new Hono<App>()
 			const id = await authedId(c)
 			if (id === null) return unauthorized(c)
 			const email = (await formField(c, 'email')).trim()
-			if (!email.includes('@')) return c.body(null, 400)
+			if (!isValidEmail(email)) return c.body(null, 400)
 			await updateAccount(c.env.DB, id, { email })
 			return c.json({ success: true })
 		}
@@ -605,11 +625,12 @@ const app = new Hono<App>()
 		describeRoute({
 			tags: ['Profile'],
 			summary: 'Set bio',
-			description: 'Free text; empty is allowed. Persisted and broadcast.',
+			description: 'Free text up to 255 characters; empty is allowed. Persisted and broadcast.',
 			security: AUTHED,
 			requestBody: form(BioRequest, 'The new bio'),
 			responses: {
 				200: json(SuccessResponse, 'Updated'),
+				400: { description: 'Bio over 255 characters (empty body)' },
 				401: UNAUTHORIZED_RESPONSE,
 			},
 		}),
@@ -617,6 +638,9 @@ const app = new Hono<App>()
 			const id = await authedId(c)
 			if (id === null) return unauthorized(c)
 			const bio = await formField(c, 'bio')
+			// Length only — a bio is free text by design (see the route summary). Refused
+			// rather than truncated: silently storing half a sentence reads as data loss.
+			if (!isValidBio(bio)) return c.body(null, 400)
 			const account = await updateAccount(c.env.DB, id, { bio })
 			await pushAccountUpdate(c, account)
 			return c.json({ success: true })
@@ -124,15 +124,21 @@ export const CreateAccountRequest = z.object({

 /** Single-string form bodies, one per profile mutation. */
 export const DisplayNameRequest = z.object({
-	displayName: z.string().describe('Trimmed; empty is rejected (400)'),
+	displayName: z
+		.string()
+		.describe('Trimmed; letters and digits only, max 15. Empty or invalid is rejected (400)'),
 })

 export const UsernameRequest = z.object({
-	username: z.string().describe('Trimmed; must be unique and changes must remain'),
+	username: z
+		.string()
+		.describe(
+			'Trimmed; letters and digits only, max 50. Must be unique and changes must remain'
+		),
 })

 export const EmailRequest = z.object({
-	email: z.string().describe('Must contain "@"; otherwise 400'),
+	email: z.string().describe('A deliverable-looking address, max 255; otherwise 400'),
 })

 export const PhoneRequest = z.object({
@@ -147,7 +153,9 @@ export const PronounsRequest = z.object({
 	pronounFlags: z.string().describe('Integer string bitmask; non-numeric is 400'),
 })

-export const BioRequest = z.object({ bio: z.string().describe('Free text; empty is allowed') })
+export const BioRequest = z.object({
+	bio: z.string().describe('Free text, max 255; empty is allowed'),
+})

 export const ProfileImageRequest = z.object({
 	imageName: z.string().describe('Avatar object key; empty is rejected (400)'),
@@ -460,3 +460,150 @@ describe('auth-gated endpoints', () => {
 		}
 	})
 })
+
+// The names a player chooses are alphanumeric and length-capped, by the same rule the
+// `rooms` worker applies (see `nameRejection` in @repo/domain). The three limits come
+// from the client's own input boxes rather than a round number, so anything stored is
+// something the game can render and re-edit.
+//
+// Server-generated names go around this deliberately — the seeded "Rec Room" account
+// above has a space in its display name, and dorms are called `@<username>'s Dorm`. The
+// check belongs at the request handler, not in the db helpers.
+describe('name, email and bio validation', () => {
+	const authed = async (sub: string) => ({
+		...(await bearer(sub)),
+		'Content-Type': 'application/x-www-form-urlencoded',
+	})
+
+	test('PUT /account/me/username refuses anything but letters and digits, max 50', async () => {
+		const headers = await authed('8801')
+		for (const username of ['has space', 'under_score', 'punct!', 'café', 'a'.repeat(51)]) {
+			const res = await exports.default.fetch(`${ORIGIN}/account/me/username`, {
+				...form({ username }),
+				headers,
+			})
+			// Still the envelope at HTTP 200, like every other refusal on this route.
+			expect(res.status).toBe(200)
+			const body = (await res.json()) as { success: boolean; error: string }
+			expect(body.success, username).toBe(false)
+			expect(body.error).toMatch(/letters and numbers|at most 50 characters/)
+		}
+
+		// The rationed change must NOT be spent by a refusal: an account starts with one,
+		// and burning it on a typo would leave the player stuck with a name they never had.
+		const me = (await (
+			await exports.default.fetch(`${ORIGIN}/account/me`, { headers: await bearer('8801') })
+		).json()) as { availableUsernameChanges: number }
+		expect(me.availableUsernameChanges).toBe(1)
+
+		// 50 is the client's own cap, so a name that long has to be accepted.
+		const ok = await exports.default.fetch(`${ORIGIN}/account/me/username`, {
+			...form({ username: 'a'.repeat(50) }),
+			headers,
+		})
+		expect(((await ok.json()) as { success: boolean }).success).toBe(true)
+	})
+
+	test('PUT /account/me/displayname refuses anything but letters and digits, max 15', async () => {
+		const headers = await authed('8802')
+		for (const displayName of ['has space', 'punct!', 'a'.repeat(16)]) {
+			const res = await exports.default.fetch(`${ORIGIN}/account/me/displayname`, {
+				...form({ displayName }),
+				headers,
+			})
+			// An empty 400, matching what this route already answers for an empty name —
+			// it acks with a bare `{ success: true }` and has never sent the client a body
+			// on failure.
+			expect(res.status, displayName).toBe(400)
+		}
+
+		// 15 is the client's box, so it must fit.
+		const ok = await exports.default.fetch(`${ORIGIN}/account/me/displayname`, {
+			...form({ displayName: 'a'.repeat(15) }),
+			headers,
+		})
+		expect(ok.status).toBe(200)
+	})
+
+	// Syntax comes from the `isemail` package rather than a pattern written here — this is
+	// a contact address nothing is ever sent to in order to prove it, so a hand-rolled
+	// regex only buys more edge cases to get wrong. It enforces the RFC's own
+	// 254-character maximum, which is why there's no separate length check.
+	test('POST /account/me/email requires a syntactically valid address', async () => {
+		const headers = await authed('8803')
+		const bad = [
+			'nope', // no @ at all — what this route used to be the only check for
+			'@example.com', // nothing to deliver to
+			'someone@', // no domain
+			'someone@example.', // empty last label
+			'two words@example.com', // whitespace
+			`${'a'.repeat(250)}@example.com`, // past the RFC's 254
+		]
+		for (const email of bad) {
+			const res = await exports.default.fetch(`${ORIGIN}/account/me/email`, {
+				...form({ email }),
+				method: 'POST',
+				headers,
+			})
+			expect(res.status, email).toBe(400)
+		}
+
+		// `someone@localhost` is in the ACCEPTED list on purpose: it's valid per the RFC,
+		// and an undeliverable address costs nothing here.
+		for (const email of [
+			'someone@example.com',
+			'first.last+tag@mail.example.co.uk',
+			'someone@localhost',
+		]) {
+			const res = await exports.default.fetch(`${ORIGIN}/account/me/email`, {
+				...form({ email }),
+				method: 'POST',
+				headers,
+			})
+			expect(res.status, email).toBe(200)
+		}
+	})
+
+	test('PUT /account/me/bio caps the stored text at 255 characters', async () => {
+		const headers = await authed('8804')
+
+		const ok = await exports.default.fetch(`${ORIGIN}/account/me/bio`, {
+			...form({ bio: 'b'.repeat(255) }),
+			headers,
+		})
+		expect(ok.status).toBe(200)
+
+		// Refused rather than truncated — storing half a sentence reads as data loss.
+		const tooLong = await exports.default.fetch(`${ORIGIN}/account/me/bio`, {
+			...form({ bio: 'b'.repeat(256) }),
+			headers,
+		})
+		expect(tooLong.status).toBe(400)
+
+		// The refusal changed nothing: the 255-character bio is still what's stored.
+		const me = await exports.default.fetch(`${ORIGIN}/account/8804/bio`)
+		expect(((await me.json()) as { bio: string }).bio).toBe('b'.repeat(255))
+	})
+})
+
+// Phone is deliberately NOT held to the name rule above: the client sends E.164
+// (`+15552223333`), so a letters-and-digits check would reject every real number by
+// eating the leading `+`. Pinned here because this route sits between two that DID just
+// get stricter, and the obvious next "cleanup" is to make it match them.
+test('POST /account/me/phone stores an E.164 number exactly as the client sends it', async () => {
+	const res = await exports.default.fetch(`${ORIGIN}/account/me/phone`, {
+		...form({ phone: '+15552223333' }),
+		method: 'POST',
+		headers: { ...(await bearer('8805')), 'Content-Type': 'application/x-www-form-urlencoded' },
+	})
+	expect(res.status).toBe(200)
+	expect(await res.json()).toEqual({ success: true })
+
+	// Read from the row: phone is stored but not surfaced by any DTO, so there's no
+	// endpoint to check it through.
+	const row = await env.DB.prepare(
+		"SELECT json_extract(data, '$.phone') AS phone FROM account WHERE json_extract(data, '$.accountId') = 8805"
+	).first<{ phone: string }>()
+	// Verbatim — no normalising, no stripping of the +.
+	expect(row?.phone).toBe('+15552223333')
+})