add some basic validation

apps/accounts/src/accounts.app.ts

@@ -1,5 +1,5 @@
 import { Hono } from 'hono'
-import { describeRoute, openAPIRouteHandler } from 'hono-openapi'
+import { describeRoute, openAPIRouteHandler, validator } from 'hono-openapi'
 import { useWorkersLogger } from 'workers-tagged-logger'
 
 import {
@@ -8,11 +8,6 @@ import {
 	getAccount,
 	getAccountByUsername,
 	getAccountsByIds,
-	isValidBio,
-	isValidEmail,
-	MAX_DISPLAY_NAME_LENGTH,
-	MAX_USERNAME_LENGTH,
-	nameRejection,
 	searchAccounts,
 	updateAccount,
 } from '@repo/domain'
@@ -74,12 +69,18 @@ function unauthorized(c: Context<App>) {
 const DEFAULT_USERNAME_CHANGES = 1
 
 /**
- * Username-change result envelope: `{ success, error, value }`, always HTTP 200.
+ * Username-change result envelope: `{ success, error, value }`. On success `value` is
- * On success `value` is the updated account; on error `error` carries the message
+ * the updated account; on a refusal `error` carries the message and `value` is an empty
- * and `value` is an empty string.
+ * string.
+ *
+ * A refusal is a 400. The body shape is unchanged — anything reading `error` still
+ * works — but it used to come back at HTTP 200, which meant a caller keying off the
+ * status read every refusal as a success. That envelope-at-200 was the reference's
+ * (`RecNet`) convention and is kept by `POST /account/create`; here it was traded for a
+ * status a client can actually branch on.
  */
 function usernameResult(c: Context<App>, error = '', value: unknown = '') {
-	return c.json({ success: error === '', error, value })
+	return c.json({ success: error === '', error, value }, error === '' ? 200 : 400)
 }
 
 /** Read a single string field from a form-urlencoded / multipart body. */
@@ -424,26 +425,20 @@ const app = new Hono<App>()
 			summary: 'Set display name',
 			description: 'Persisted and broadcast via an AccountUpdate notification.',
 			security: AUTHED,
-			requestBody: form(DisplayNameRequest, 'The new display name'),
 			responses: {
 				200: json(SuccessResponse, 'Updated'),
 				400: { description: 'Empty, over 15 characters, or non-alphanumeric (empty body)' },
 				401: UNAUTHORIZED_RESPONSE,
 			},
 		}),
+		// An EMPTY 400, which is what this route already answered for an empty name: it
+		// acks with a bare SuccessResponse and has never sent the client a body on
+		// failure, so enforcing the schema doesn't change what a refusal looks like.
+		validator('form', DisplayNameRequest, (r, c) => (r.success ? undefined : c.body(null, 400))),
 		async (c) => {
 			const id = await authedId(c)
 			if (id === null) return unauthorized(c)
-			const displayName = (await formField(c, 'displayName')).trim()
+			const { displayName } = c.req.valid('form')
-			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 })
@@ -465,24 +460,28 @@ const app = new Hono<App>()
 				'(see the UsernameResult envelope).',
 			].join(' '),
 			security: AUTHED,
-			requestBody: form(UsernameRequest, 'The desired username'),
 			responses: {
-				200: json(UsernameResult, 'Result envelope (success or a validation error)'),
+				200: json(UsernameResult, 'The updated account, in the result envelope'),
+				400: json(UsernameResult, 'Refused — `error` carries the reason, `value` is ""'),
 				401: UNAUTHORIZED_RESPONSE,
 			},
 		}),
+		// Shape is checked before the handler runs, so a rejected name costs no D1 read and
+		// — the part that matters — can never spend one of the account's rationed changes.
+		// The message is relayed rather than zod's issue array: `nameRejection` writes the
+		// sentence the player reads, and nothing can render an array of issues.
+		// `c` is annotated so the hook's context matches this app's bindings, and `error` is
+		// Standard Schema's flat issue list rather than a zod error object.
+		validator('form', UsernameRequest, (r, c: Context<App>) =>
+			r.success
+				? undefined
+				: usernameResult(c, r.error[0]?.message ?? 'That username cannot be used.')
+		),
 		async (c) => {
 			const id = await authedId(c)
 			if (id === null) return unauthorized(c)
 
-			const username = (await formField(c, 'username')).trim()
+			const { username } = c.req.valid('form')
-			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)
@@ -514,18 +513,17 @@ const app = new Hono<App>()
 			summary: 'Set email',
 			description: 'Persisted; surfaced only by `/account/me`. Not broadcast.',
 			security: AUTHED,
-			requestBody: form(EmailRequest, 'The new email'),
 			responses: {
 				200: json(SuccessResponse, 'Updated'),
-				400: { description: 'Not a valid address, or over 255 characters (empty body)' },
+				400: { description: 'Not a syntactically valid address (empty body)' },
 				401: UNAUTHORIZED_RESPONSE,
 			},
 		}),
+		validator('form', EmailRequest, (r, c) => (r.success ? undefined : c.body(null, 400))),
 		async (c) => {
 			const id = await authedId(c)
 			if (id === null) return unauthorized(c)
-			const email = (await formField(c, 'email')).trim()
+			const { email } = c.req.valid('form')
-			if (!isValidEmail(email)) return c.body(null, 400)
 			await updateAccount(c.env.DB, id, { email })
 			return c.json({ success: true })
 		}
@@ -539,18 +537,17 @@ const app = new Hono<App>()
 			summary: 'Set phone number',
 			description: 'Persisted on the account row. Not broadcast.',
 			security: AUTHED,
-			requestBody: form(PhoneRequest, 'The new phone number'),
 			responses: {
 				200: json(SuccessResponse, 'Updated'),
 				400: { description: 'Empty phone (empty body)' },
 				401: UNAUTHORIZED_RESPONSE,
 			},
 		}),
+		validator('form', PhoneRequest, (r, c) => (r.success ? undefined : c.body(null, 400))),
 		async (c) => {
 			const id = await authedId(c)
 			if (id === null) return unauthorized(c)
-			const phone = (await formField(c, 'phone')).trim()
+			const { phone } = c.req.valid('form')
-			if (phone === '') return c.body(null, 400)
 			await updateAccount(c.env.DB, id, { phone })
 			return c.json({ success: true })
 		}
@@ -627,20 +624,18 @@ const app = new Hono<App>()
 			summary: 'Set bio',
 			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,
 			},
 		}),
+		// Refused rather than truncated: silently storing half a sentence reads as data loss.
+		validator('form', BioRequest, (r, c) => (r.success ? undefined : c.body(null, 400))),
 		async (c) => {
 			const id = await authedId(c)
 			if (id === null) return unauthorized(c)
-			const bio = await formField(c, 'bio')
+			const { bio } = c.req.valid('form')
-			// 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 })

apps/accounts/src/openapi.ts

@@ -1,20 +1,35 @@
 import { resolver } from 'hono-openapi'
 import { z } from 'zod'
 
+import {
+	isValidBio,
+	isValidEmail,
+	MAX_DISPLAY_NAME_LENGTH,
+	MAX_USERNAME_LENGTH,
+	nameRejection,
+} from '@repo/domain'
+
 import type { OpenAPIV3_1 } from 'openapi-types'
 
 /**
  * OpenAPI schemas for the accounts worker.
  *
- * IMPORTANT: these are DESCRIPTIVE ONLY. They are passed to `describeRoute` to
+ * Most of these are DESCRIPTIVE ONLY: they are passed to `describeRoute` to generate the
- * generate the spec and are never wired into `hono-openapi`'s `validator()`.
+ * spec, and the handler stays lenient. That is deliberate — the Rec Room client is the
+ * real consumer, form fields are read as `typeof value === 'string' ? value : ''`, and
+ * missing or malformed input falls through to a graceful path (or a synthesized default
+ * account) rather than a hard error. A schema that rejected what the client actually
+ * sends would break the game, not protect it.
  *
- * As with the auth worker, this is deliberate. The Rec Room client is the only real
+ * The EXCEPTION is the profile mutations a player types into a box — displayName,
- * consumer and the handlers are intentionally lenient — form fields are read as
+ * username, email, phone, bio. Those carry real rules (see `@repo/domain`), and each is
- * `typeof value === 'string' ? value : ''` and missing/malformed input falls through
+ * wired into `hono-openapi`'s `validator()` per route, with tests, exactly as the older
- * to a graceful path (or a synthesized default account) rather than a hard error.
+ * version of this note prescribed. Wiring one up means the schema both validates the
- * These schemas record what the client is observed to send and what we send back; to
+ * request and generates the spec, so a limit can't be changed in one and not the other —
- * enforce one, do it per-route and land a test with it.
+ * which is precisely how the documented email limit came to disagree with the real one.
+ *
+ * A validated route drops `requestBody: form(...)` from its `describeRoute`: the
+ * validator registers the body itself, and declaring it twice would emit it twice.
  */
 
 /** Emit a zod schema as an `application/json` response body. */
@@ -122,27 +137,54 @@ export const CreateAccountRequest = z.object({
 	platformId: z.string().optional().describe('Parsed for fidelity; currently unused'),
 })
 
-/** Single-string form bodies, one per profile mutation. */
+/**
+ * Single-string form bodies, one per profile mutation.
+ *
+ * These are ENFORCED, not just described: each is handed to hono-openapi's `validator`,
+ * so the same schema both validates the request and generates the spec. Before this they
+ * were documentation only, and the real rule lived in the handler — which meant every
+ * limit had to be edited in two places and nothing caught them disagreeing.
+ *
+ * The rules themselves come from `@repo/domain` so `rooms` and `clubs` can't drift from
+ * `accounts`; `superRefine` is used where the message matters, because `nameRejection`
+ * writes the player-facing sentence and there's no reason to write it twice.
+ */
+
+/** Zod check that defers to the shared name rule, message and all. */
+const nameCheck = (label: string, max: number) =>
+	z.string()
+		.trim()
+		.superRefine((value, ctx) => {
+			const rejection = nameRejection(value, label, max)
+			if (rejection !== null) ctx.addIssue({ code: 'custom', message: rejection })
+		})
+
 export const DisplayNameRequest = z.object({
-	displayName: z
+	displayName: nameCheck('display name', MAX_DISPLAY_NAME_LENGTH)
-		.string()
+		.min(1)
 		.describe('Trimmed; letters and digits only, max 15. Empty or invalid is rejected (400)'),
 })
 
 export const UsernameRequest = z.object({
-	username: z
+	username: nameCheck('username', MAX_USERNAME_LENGTH)
-		.string()
+		.min(1, 'You must enter a username.')
 		.describe(
 			'Trimmed; letters and digits only, max 50. Must be unique and changes must remain'
 		),
 })
 
 export const EmailRequest = z.object({
-	email: z.string().describe('A deliverable-looking address, max 255; otherwise 400'),
+	email: z
+		.string()
+		.trim()
+		.refine(isValidEmail, 'That email address looks wrong.')
+		.describe('A syntactically valid address (RFC 5321/5322, so at most 254); otherwise 400'),
 })
 
 export const PhoneRequest = z.object({
-	phone: z.string().describe('Trimmed; empty is rejected (400)'),
+	// No shape rule on purpose: the client sends E.164 (`+15552223333`), which the name
+	// rule above would reject outright by eating the leading `+`.
+	phone: z.string().trim().min(1).describe('Trimmed; empty is rejected (400)'),
 })
 
 export const IdentityFlagsRequest = z.object({
@@ -154,7 +196,8 @@ export const PronounsRequest = z.object({
 })
 
 export const BioRequest = z.object({
-	bio: z.string().describe('Free text, max 255; empty is allowed'),
+	// Not trimmed — a bio is free text, and leading whitespace is the player's business.
+	bio: z.string().refine(isValidBio).describe('Free text, max 255; empty is allowed'),
 })
 
 export const ProfileImageRequest = z.object({

apps/accounts/src/test/integration/api.test.ts

@@ -220,8 +220,9 @@ describe('auth-gated endpoints', () => {
 			...form({ username: 'Coach' }),
 			headers: { ...(await bearer('893')), 'Content-Type': 'application/x-www-form-urlencoded' },
 		})
-		// Business errors are HTTP 200 with the { success, error, value } envelope.
+		// A refusal is a 400 carrying the same { success, error, value } envelope. It used
-		expect(res.status).toBe(200)
+		// to be HTTP 200, which read as a success to anything branching on the status.
+		expect(res.status).toBe(400)
 		const body = (await res.json()) as { success: boolean; error: string; value: string }
 		expect(body.success).toBe(false)
 		expect(body.error).toMatch(/already taken/i)
@@ -260,7 +261,7 @@ describe('auth-gated endpoints', () => {
 			...form({ username: 'coachy' }),
 			headers,
 		})
-		expect(blocked.status).toBe(200)
+		expect(blocked.status).toBe(400)
 		const blockedBody = (await blocked.json()) as { success: boolean; error: string }
 		expect(blockedBody.success).toBe(false)
 		expect(blockedBody.error).toMatch(/no username changes/i)
@@ -459,6 +460,31 @@ describe('auth-gated endpoints', () => {
 			for (const op of Object.values(ops)) expect(op.summary).toBeTruthy()
 		}
 	})
+
+	// hono-openapi registers a validated form body under `multipart/form-data` only, and
+	// its `media` option can't say otherwise (a precedence bug — see `withCleanSpec`). The
+	// real callers post `application/x-www-form-urlencoded`, so a spec that named only
+	// multipart would tell an integrator to send the one thing nothing here sends.
+	test('GET /openapi.json documents both form content types on validated routes', async () => {
+		const res = await exports.default.fetch(`${ORIGIN}/openapi.json`)
+		const spec = (await res.json()) as {
+			paths: Record<string, Record<string, { requestBody?: { content: Record<string, unknown> } }>>
+		}
+
+		for (const [path, method] of [
+			['/account/me/email', 'post'],
+			['/account/me/username', 'put'],
+			['/account/me/displayname', 'put'],
+			['/account/me/bio', 'put'],
+			['/account/me/phone', 'post'],
+		] as const) {
+			const content = spec.paths[path]?.[method]?.requestBody?.content ?? {}
+			expect(Object.keys(content).sort(), path).toEqual([
+				'application/x-www-form-urlencoded',
+				'multipart/form-data',
+			])
+		}
+	})
 })
 
 // The names a player chooses are alphanumeric and length-capped, by the same rule the
@@ -482,11 +508,13 @@ describe('name, email and bio validation', () => {
 				...form({ username }),
 				headers,
 			})
-			// Still the envelope at HTTP 200, like every other refusal on this route.
+			// Refused by the SCHEMA (see openapi.ts `UsernameRequest`) before the handler
-			expect(res.status).toBe(200)
+			// runs — but still in this route's envelope, because the hook puts it there.
-			const body = (await res.json()) as { success: boolean; error: string }
+			expect(res.status, username).toBe(400)
+			const body = (await res.json()) as { success: boolean; error: string; value: string }
 			expect(body.success, username).toBe(false)
 			expect(body.error).toMatch(/letters and numbers|at most 50 characters/)
+			expect(body.value).toBe('')
 		}
 
 		// The rationed change must NOT be spent by a refusal: an account starts with one,

packages/hono-helpers/src/helpers/openapi.ts

@@ -33,9 +33,44 @@ function addIntegerExamples(node: unknown): void {
 	for (const value of Object.values(obj)) addIntegerExamples(value)
 }
 
+/**
+ * hono-openapi registers a `validator('form', …)` body under `multipart/form-data` ONLY,
+ * and there is no way to ask it for another media type: its media selection reads
+ * `options?.media ?? target === 'json' ? 'application/json' : 'multipart/form-data'`,
+ * which by JS precedence is `((options?.media ?? target) === 'json') ? … : …` — so the
+ * `media` option can never produce anything else.
+ *
+ * That leaves the spec claiming a validated route accepts only multipart, when the real
+ * callers (the Rec Room client and the website) post `application/x-www-form-urlencoded`
+ * and Hono's `parseBody()` reads both. So the urlencoded variant is mirrored back in.
+ *
+ * Safe because nothing here documents a genuinely multipart-only body — there are no file
+ * uploads on these workers, and hand-written form bodies already declare both types. If
+ * one is ever added, it will need to opt out of this.
+ */
+function mirrorFormBodies(node: unknown): void {
+	if (Array.isArray(node)) {
+		for (const item of node) mirrorFormBodies(item)
+		return
+	}
+	if (node === null || typeof node !== 'object') return
+
+	const obj = node as Record<string, unknown>
+	const content = obj.content
+	if (content !== null && typeof content === 'object') {
+		const media = content as Record<string, unknown>
+		const multipart = media['multipart/form-data']
+		if (multipart !== undefined && media['application/x-www-form-urlencoded'] === undefined) {
+			media['application/x-www-form-urlencoded'] = multipart
+		}
+	}
+	for (const value of Object.values(obj)) mirrorFormBodies(value)
+}
+
 /**
  * Wrap `openAPIRouteHandler(...)` so the generated document gets example values for its
- * integer fields. Purely cosmetic — nothing about the documented shapes changes.
+ * integer fields, and so a validated form body documents both content types it really
+ * accepts. Nothing about the runtime behaviour changes — this only corrects the document.
  *
  * ```ts
  * app.get('/openapi.json', describeRoute({ hide: true }), withCleanSpec(openAPIRouteHandler(app, { ... })))
@@ -47,6 +82,7 @@ export function withCleanSpec(handler: Handler | MiddlewareHandler): Handler {
 		if (!(res instanceof Response)) return res as never
 		const spec: unknown = await res.json()
 		addIntegerExamples(spec)
+		mirrorFormBodies(spec)
 		return c.json(spec as Record<string, unknown>)
 	}
 }