diff --git a/apps/accounts/package.json b/apps/accounts/package.json index 07b8beb..d9c82b2 100644 --- a/apps/accounts/package.json +++ b/apps/accounts/package.json @@ -15,6 +15,7 @@ "test": "run-vitest" }, "dependencies": { + "@2toad/profanity": "3.3.0", "@repo/domain": "workspace:*", "@repo/hono-helpers": "workspace:*", "@repo/jwt": "workspace:*", diff --git a/apps/accounts/src/accounts.app.ts b/apps/accounts/src/accounts.app.ts index d00b2dd..c46fa1f 100644 --- a/apps/accounts/src/accounts.app.ts +++ b/apps/accounts/src/accounts.app.ts @@ -448,7 +448,9 @@ const app = new Hono() security: AUTHED, responses: { 200: json(SuccessResponse, 'Updated'), - 400: { description: 'Empty, over 15 characters, or non-alphanumeric (empty body)' }, + 400: { + description: 'Empty, over 15 characters, non-alphanumeric, or profane (empty body)', + }, 401: UNAUTHORIZED_RESPONSE, }, }), @@ -475,7 +477,8 @@ const app = new Hono() tags: ['Profile'], summary: 'Change username', description: [ - 'Letters and digits only, at most 50 characters. Rejects a name taken by another', + 'Letters and digits only, at most 50 characters, and free of profanity (the same', + 'word list as `api`’s `POST /api/sanitize/v1/isPure`). 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).', diff --git a/apps/accounts/src/openapi.ts b/apps/accounts/src/openapi.ts index c9888c4..87bd771 100644 --- a/apps/accounts/src/openapi.ts +++ b/apps/accounts/src/openapi.ts @@ -9,6 +9,11 @@ import { nameRejection, } from '@repo/domain' +// The profanity filter behind `api`'s `POST /api/sanitize/v1/isPure`, imported rather +// than copied so a name is held to the very same word list every other player-typed +// string is. +import { nameContainsSwears } from '../../api/src/sanitize' + import type { OpenAPIV3_1 } from 'openapi-types' /** @@ -158,26 +163,44 @@ export const CreateAccountRequest = z.object({ * 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. */ +/** + * Zod check that defers to the shared name rule, message and all, and then refuses a name + * with a swear in it — the same filter, and the same word list, as `api`'s + * `POST /api/sanitize/v1/isPure`. + * + * Shape first, profanity second: a name that already broke the charset rule gets the one + * sentence that explains it rather than two, and the swear check never sees the + * punctuation the charset rule has already refused. + */ 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 }) + if (rejection !== null) { + ctx.addIssue({ code: 'custom', message: rejection }) + } else if (nameContainsSwears(value)) { + // Deliberately vague about WHICH word: naming it back to the player prints the + // swear in the UI, and the player knows what they typed. + ctx.addIssue({ code: 'custom', message: `Your ${label} can't contain that word.` }) + } }) export const DisplayNameRequest = z.object({ displayName: nameCheck('display name', MAX_DISPLAY_NAME_LENGTH) .min(1) - .describe('Trimmed; letters and digits only, max 15. Empty or invalid is rejected (400)'), + .describe( + 'Trimmed; letters and digits only, max 15, no profanity. Empty or invalid is rejected (400)' + ), }) export const UsernameRequest = z.object({ username: nameCheck('username', MAX_USERNAME_LENGTH) .min(1, 'You must enter a username.') - .describe('Trimmed; letters and digits only, max 50. Must be unique and changes must remain'), + .describe( + 'Trimmed; letters and digits only, max 50, no profanity. Must be unique and changes must remain' + ), }) export const EmailRequest = z.object({ diff --git a/apps/accounts/src/test/integration/api.test.ts b/apps/accounts/src/test/integration/api.test.ts index 33f9b50..a0aef71 100644 --- a/apps/accounts/src/test/integration/api.test.ts +++ b/apps/accounts/src/test/integration/api.test.ts @@ -223,6 +223,32 @@ describe('auth-gated endpoints', () => { expect(((await me.json()) as { displayName: string }).displayName).toBe('laskdjfasdlfkj') }) + test('PUT /account/me/displayname 400s on a name with a swear in it', async () => { + const headers = { + ...(await bearer('895')), + 'Content-Type': 'application/x-www-form-urlencoded', + } + // A name carries no spaces, so the filter has to find the swear at the seam the + // player typed instead of one. + for (const displayName of ['fuck', 'ShitLord', 'Fucker123']) { + const res = await exports.default.fetch(`${ORIGIN}/account/me/displayname`, { + ...form({ displayName }), + headers, + }) + expect(res.status).toBe(400) + } + + // And the words that merely contain one still get through — refusing these is worse + // than missing a swear, because the player can't see why. + for (const displayName of ['Scunthorpe', 'ClassicCar', 'Cumberland']) { + const res = await exports.default.fetch(`${ORIGIN}/account/me/displayname`, { + ...form({ displayName }), + headers, + }) + expect(res.status).toBe(200) + } + }) + test('PUT /account/me/username 401s without a token', async () => { const res = await exports.default.fetch(`${ORIGIN}/account/me/username`, { ...form({ username: 'whoever' }), @@ -244,6 +270,30 @@ describe('auth-gated endpoints', () => { expect(body.value).toBe('') }) + test('PUT /account/me/username refuses a swear without spending a change', async () => { + const headers = { + ...(await bearer('894')), + 'Content-Type': 'application/x-www-form-urlencoded', + } + const res = await exports.default.fetch(`${ORIGIN}/account/me/username`, { + ...form({ username: 'ShitLord' }), + headers, + }) + expect(res.status).toBe(200) + const body = (await res.json()) as { success: boolean; error: string; value: string } + expect(body.success).toBe(false) + // Vague on purpose — the message must not print the swear back at the player. + expect(body.error).toMatch(/can't contain that word/i) + expect(body.value).toBe('') + + // The schema runs before the handler, so a refused name costs none of the account's + // rationed changes. + const me = (await ( + await exports.default.fetch(`${ORIGIN}/account/me`, { headers: await bearer('894') }) + ).json()) as { username: string; availableUsernameChanges: number } + expect(me.availableUsernameChanges).toBe(3) + }) + test('PUT /account/me/username allows three changes, decrements the counter, then blocks', async () => { const headers = { ...(await bearer('892')), diff --git a/apps/api/src/sanitize.ts b/apps/api/src/sanitize.ts index 47e9d86..73f4e6f 100644 --- a/apps/api/src/sanitize.ts +++ b/apps/api/src/sanitize.ts @@ -60,6 +60,40 @@ export function containsSwears(value: string): boolean { return value !== '' && filter.exists(value) } +/** + * Where a space would be in a name if the charset allowed one: a lowercase-to-uppercase + * hop, the last capital of a run before a capitalised word, and either side of a run of + * digits. Applied in that order, so `ShitLord`, `XXFuckYou` and `Fucker123` each come + * apart at the seam a player wrote them with. + */ +const NAME_WORD_BOUNDARIES: Array<[RegExp, string]> = [ + [/([a-z0-9])([A-Z])/g, '$1 $2'], + [/([A-Z]+)([A-Z][a-z])/g, '$1 $2'], + [/([A-Za-z])([0-9])/g, '$1 $2'], +] + +/** + * Whether `value`, read as a NAME, contains a swear. + * + * A username or display name is letters and digits only (`nameRejection`), so it carries + * no spaces — and the filter matches whole words. Handing one to {@link containsSwears} + * as-is therefore only refuses a name that IS a swear and nothing else: `Fucker123` and + * `ShitLord` sail through. So the name is split at the boundaries a player types instead + * of a space, and the pieces are checked as words. + * + * That keeps the library's trade-off rather than reaching for substring matching, which + * is the tempting fix and the wrong one: `Scunthorpe`, `assassin`, `Classic`, + * `Cumberland` and `Shiitake` all contain a swear as a substring, and refusing someone's + * name without being able to say why is worse than missing `Bitchy`. + */ +export function nameContainsSwears(value: string): boolean { + const spaced = NAME_WORD_BOUNDARIES.reduce( + (text, [pattern, replacement]) => text.replace(pattern, replacement), + value + ) + return containsSwears(spaced) +} + /** The mask `POST /api/sanitize/v1` uses when the request names no `ReplacementChar`. */ export const DEFAULT_REPLACEMENT_CHAR = '*' diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 6f88aeb..6645d09 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -53,6 +53,9 @@ importers: apps/accounts: dependencies: + '@2toad/profanity': + specifier: 3.3.0 + version: 3.3.0 '@repo/domain': specifier: workspace:* version: link:../../packages/domain