From e68622118120dcb2dcbc7713807518e784be9e35 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 10 Aug 2026 12:38:00 +0000 Subject: [PATCH] Enforce username rules on every account-creation path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Username format rules (2-20 chars, [a-zA-Z0-9_] only) were enforced only by POST /api/auth/username/check, which is advisory. The four paths that actually insert users — email registration completion, OAuth completion, and native Apple/Google sign-in — ran only the profanity filter, so a 300-character username containing HTML, newlines, and unicode was accepted and stored. Usernames are rendered directly into push notification bodies ("${user.username} sent you an Oy!"), so a stored username is attacker -controlled lock-screen copy for anyone who receives an Oy. validateUsername in lib.ts is now the single gate: length, character set, and profanity. normalizeUsername (previously dead code) trims and lowercases, matching how usernames are stored and looked up. Both are applied at each creation path, and the check endpoint now delegates to the same function so advisory results match what creation accepts. Also removes fetchUserByUsername, which had no callers. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PvpccXvmDzszQ1fCJnaChX Co-authored-by: jackowayed <18899+jackowayed@users.noreply.github.com> --- tests/worker/email.test.ts | 64 +++++++++++ tests/worker/oauth.test.ts | 203 ++++++++++++++++++++++++++++++++++ tests/worker/username.test.ts | 50 +++++++++ worker/lib.ts | 36 +++--- worker/routes/auth.ts | 21 +--- worker/routes/email.ts | 13 +-- worker/routes/oauth.ts | 30 +++-- 7 files changed, 361 insertions(+), 56 deletions(-) create mode 100644 tests/worker/username.test.ts diff --git a/tests/worker/email.test.ts b/tests/worker/email.test.ts index 875a939..04ced91 100644 --- a/tests/worker/email.test.ts +++ b/tests/worker/email.test.ts @@ -201,6 +201,70 @@ describe("email auth", () => { assert.equal(json.error, "Username contains disallowed language"); }); + it("rejects malformed usernames during email registration completion", async () => { + const { env, kv, db } = createTestEnv(); + const email = "new3@example.com"; + await kv.put( + `email_code:${email}`, + JSON.stringify({ code: "333444", attempts: 0 }), + ); + + const { res } = await jsonRequest(env, "/api/auth/email/verify", { + method: "POST", + body: { email, code: "333444" }, + }); + const pendingId = getCookieValue(res, "email_pending"); + assert.ok(pendingId); + + const hostileUsername = `zed\nOy! tap here 🎉${"a".repeat(300)}`; + const { res: completeRes, json } = await jsonRequest( + env, + "/api/auth/email/complete", + { + method: "POST", + headers: { cookie: `email_pending=${pendingId}` }, + body: { username: hostileUsername }, + }, + ); + + assert.equal(completeRes.status, 400); + assert.equal(json.error, "Username must be 2-20 characters"); + assert.equal(db.users.length, 0); + }); + + it("rejects usernames with disallowed characters during email registration completion", async () => { + const { env, kv, db } = createTestEnv(); + const email = "new4@example.com"; + await kv.put( + `email_code:${email}`, + JSON.stringify({ code: "555666", attempts: 0 }), + ); + + const { res } = await jsonRequest(env, "/api/auth/email/verify", { + method: "POST", + body: { email, code: "555666" }, + }); + const pendingId = getCookieValue(res, "email_pending"); + assert.ok(pendingId); + + const { res: completeRes, json } = await jsonRequest( + env, + "/api/auth/email/complete", + { + method: "POST", + headers: { cookie: `email_pending=${pendingId}` }, + body: { username: "zed\nsent you an Oy!" }, + }, + ); + + assert.equal(completeRes.status, 400); + assert.equal( + json.error, + "Username can only contain letters, numbers, and underscores", + ); + assert.equal(db.users.length, 0); + }); + it("links email for authenticated users", async (t) => { const { env, kv, db } = createTestEnv(); const user = seedUser(db, { username: "Emailer" }); diff --git a/tests/worker/oauth.test.ts b/tests/worker/oauth.test.ts index 3fad494..faca4ef 100644 --- a/tests/worker/oauth.test.ts +++ b/tests/worker/oauth.test.ts @@ -548,4 +548,207 @@ describe("oauth", () => { assert.equal(user.oauth_sub, null); assert.equal(db.sessions.length, 0); }); + + it("rejects malformed usernames during oauth completion", async () => { + const { env, kv, db } = createTestEnv(); + await kv.put( + "oauth_pending:pending-hostile", + JSON.stringify({ + provider: "google", + sub: "google-sub-hostile", + email: "hostile@example.com", + }), + ); + + const res = await request(env, "/api/auth/oauth/complete", { + method: "POST", + headers: { + "content-type": "application/json", + "x-oauth-pending": "pending-hostile", + }, + body: JSON.stringify({ + username: `zed\nOy! tap here 🎉${"a".repeat(300)}`, + }), + }); + + assert.equal(res.status, 400); + const body = (await res.json()) as { error: string }; + assert.equal(body.error, "Username must be 2-20 characters"); + assert.equal(db.users.length, 0); + assert.equal(db.sessions.length, 0); + }); + + it("rejects usernames with disallowed characters during oauth completion", async () => { + const { env, kv, db } = createTestEnv(); + await kv.put( + "oauth_pending:pending-newline", + JSON.stringify({ + provider: "google", + sub: "google-sub-newline", + email: "newline@example.com", + }), + ); + + const res = await request(env, "/api/auth/oauth/complete", { + method: "POST", + headers: { + "content-type": "application/json", + "x-oauth-pending": "pending-newline", + }, + body: JSON.stringify({ username: "zed\nsent you an Oy!" }), + }); + + assert.equal(res.status, 400); + const body = (await res.json()) as { error: string }; + assert.equal( + body.error, + "Username can only contain letters, numbers, and underscores", + ); + assert.equal(db.users.length, 0); + assert.equal(db.sessions.length, 0); + }); + + it("rejects malformed usernames during native google sign-in", async (t) => { + const { env, db } = createTestEnv(); + + const originalFetch = globalThis.fetch; + globalThis.fetch = async (input) => { + const url = typeof input === "string" ? input : input.url; + if (url.startsWith("https://oauth2.googleapis.com/tokeninfo")) { + return { + ok: true, + json: async () => ({ + aud: env.GOOGLE_CLIENT_ID, + sub: "google-native-hostile", + email: "native-hostile@example.com", + email_verified: "true", + }), + } as Response; + } + throw new Error(`Unexpected fetch: ${url}`); + }; + t.after(() => { + globalThis.fetch = originalFetch; + }); + + const res = await request(env, "/api/auth/oauth/google/native", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + idToken: "native-id-token", + username: "zed\nsent you an Oy!", + }), + }); + + assert.equal(res.status, 400); + const body = (await res.json()) as { error: string }; + assert.equal( + body.error, + "Username can only contain letters, numbers, and underscores", + ); + assert.equal(db.users.length, 0); + assert.equal(db.sessions.length, 0); + }); + + it("rejects malformed usernames during native apple sign-in", async (t) => { + const { env, db } = createTestEnv(); + + const { publicKey, privateKey } = await crypto.subtle.generateKey( + { + name: "RSASSA-PKCS1-v1_5", + modulusLength: 2048, + publicExponent: new Uint8Array([1, 0, 1]), + hash: "SHA-256", + }, + true, + ["sign", "verify"], + ); + const jwk = (await crypto.subtle.exportKey("jwk", publicKey)) as JsonWebKey; + jwk.kid = "apple-test-kid-hostile"; + const token = await createAppleIdToken({ + privateKey, + sub: "apple-sub-hostile", + email: "apple-hostile@example.com", + aud: env.APPLE_NATIVE_CLIENT_ID ?? env.APPLE_CLIENT_ID, + kid: String(jwk.kid), + }); + + const originalFetch = globalThis.fetch; + globalThis.fetch = async (input) => { + const url = typeof input === "string" ? input : input.url; + if (url === "https://appleid.apple.com/auth/keys") { + return { + ok: true, + json: async () => ({ keys: [jwk] }), + } as Response; + } + throw new Error(`Unexpected fetch: ${url}`); + }; + t.after(() => { + globalThis.fetch = originalFetch; + }); + + const res = await request(env, "/api/auth/oauth/apple/native", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + idToken: token, + username: `zed\nOy! tap here 🎉${"a".repeat(300)}`, + }), + }); + + assert.equal(res.status, 400); + const body = (await res.json()) as { error: string }; + assert.equal(body.error, "Username must be 2-20 characters"); + assert.equal(db.users.length, 0); + assert.equal(db.sessions.length, 0); + }); + + it("does not create users from malformed signup usernames in the google callback", async (t) => { + const { env, db } = createTestEnv(); + + const originalFetch = globalThis.fetch; + globalThis.fetch = async (input) => { + const url = typeof input === "string" ? input : input.url; + if (url === "https://oauth2.googleapis.com/token") { + return { + ok: true, + json: async () => ({ id_token: "token-callback-hostile" }), + } as Response; + } + if (url.startsWith("https://oauth2.googleapis.com/tokeninfo")) { + return { + ok: true, + json: async () => ({ + aud: env.GOOGLE_CLIENT_ID, + sub: "google-callback-hostile", + email: "callback-hostile@example.com", + email_verified: "true", + }), + } as Response; + } + throw new Error(`Unexpected fetch: ${url}`); + }; + t.after(() => { + globalThis.fetch = originalFetch; + }); + + const hostileUsername = "zed\nsent you an Oy!"; + const startRes = await request( + env, + `/api/auth/oauth/google?username=${encodeURIComponent(hostileUsername)}`, + ); + const startLocation = startRes.headers.get("location") ?? ""; + const state = new URL(startLocation).searchParams.get("state") ?? ""; + + const res = await request( + env, + `/api/auth/oauth/callback?state=${state}&code=auth-code`, + ); + + assert.equal(res.status, 302); + assert.equal(res.headers.get("location"), "/?choose_username=1"); + assert.equal(db.users.length, 0); + assert.equal(db.sessions.length, 0); + }); }); diff --git a/tests/worker/username.test.ts b/tests/worker/username.test.ts new file mode 100644 index 0000000..63a0709 --- /dev/null +++ b/tests/worker/username.test.ts @@ -0,0 +1,50 @@ +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; +import { normalizeUsername, validateUsername } from "../../worker/lib"; + +describe("username validation", () => { + it("normalizes to a trimmed, lowercased string", () => { + assert.equal(normalizeUsername(" Zed "), "zed"); + assert.equal(normalizeUsername(undefined), ""); + assert.equal(normalizeUsername(null), ""); + assert.equal(normalizeUsername(12), "12"); + }); + + it("accepts letters, numbers, and underscores", () => { + assert.equal(validateUsername("zed"), null); + assert.equal(validateUsername("Zed_99"), null); + assert.equal(validateUsername("a".repeat(20)), null); + }); + + it("rejects usernames outside the 2-20 character range", () => { + assert.equal(validateUsername(""), "Username must be 2-20 characters"); + assert.equal(validateUsername("a"), "Username must be 2-20 characters"); + assert.equal( + validateUsername("a".repeat(21)), + "Username must be 2-20 characters", + ); + assert.equal( + validateUsername("a".repeat(300)), + "Username must be 2-20 characters", + ); + }); + + it("rejects HTML, newlines, unicode, and whitespace", () => { + const badCharsError = + "Username can only contain letters, numbers, and underscores"; + assert.equal(validateUsername("zed"), badCharsError); + assert.equal(validateUsername("zed\nOy from evil"), badCharsError); + assert.equal(validateUsername("zed‮gnihsihp"), badCharsError); + assert.equal(validateUsername("zed 🎉"), badCharsError); + assert.equal(validateUsername("zed user"), badCharsError); + assert.equal(validateUsername("zed-user"), badCharsError); + assert.equal(validateUsername("zed@example.com"), badCharsError); + }); + + it("rejects profanity", () => { + assert.equal( + validateUsername("shitname"), + "Username contains disallowed language", + ); + }); +}); diff --git a/worker/lib.ts b/worker/lib.ts index 60f1586..4765a77 100644 --- a/worker/lib.ts +++ b/worker/lib.ts @@ -1,4 +1,5 @@ import { setCookie } from "hono/cookie"; +import { validateCleanUsername } from "./moderation"; import { sendNativePushNotification, sendPushNotification } from "./push"; import type { AppContext, @@ -542,15 +543,32 @@ export function authUserPayload(user: User) { }; } +const USERNAME_MIN_LENGTH = 2; +const USERNAME_MAX_LENGTH = 20; +const USERNAME_PATTERN = /^[a-zA-Z0-9_]+$/; + +// Usernames are stored lowercase; every lookup compares on LOWER(username). export function normalizeUsername(username: unknown) { - return String(username || "").trim(); + return String(username ?? "") + .trim() + .toLowerCase(); } +// The single gate for every username that reaches the users table. Usernames +// are rendered as push notification copy, so anything but a short ASCII +// identifier is rejected here rather than at the advisory check endpoint. export function validateUsername(username: string) { - if (!username || username.length < 2 || username.length > 20) { - return "Username must be 2-20 characters"; + if ( + !username || + username.length < USERNAME_MIN_LENGTH || + username.length > USERNAME_MAX_LENGTH + ) { + return `Username must be ${USERNAME_MIN_LENGTH}-${USERNAME_MAX_LENGTH} characters`; + } + if (!USERNAME_PATTERN.test(username)) { + return "Username can only contain letters, numbers, and underscores"; } - return null; + return validateCleanUsername(username); } export async function createSession(c: AppContext, user: User) { @@ -631,16 +649,6 @@ export async function fetchFriendsByOyRecency( return friends.rows; } -export async function fetchUserByUsername( - c: AppContext, - username: string, -): Promise { - const result = await c - .get("db") - .query("SELECT * FROM users WHERE username ILIKE $1", [username]); - return result.rows[0] ?? null; -} - export async function requireAdmin(c: AppContext) { const user = c.get("user"); if (!user) { diff --git a/worker/routes/auth.ts b/worker/routes/auth.ts index 787517e..db24cb3 100644 --- a/worker/routes/auth.ts +++ b/worker/routes/auth.ts @@ -1,6 +1,5 @@ import { deleteCookie } from "hono/cookie"; -import { authUserPayload, validateUsername } from "../lib"; -import { validateCleanUsername } from "../moderation"; +import { authUserPayload, normalizeUsername, validateUsername } from "../lib"; import type { App, AppContext, User } from "../types"; const DELETE_RATE_PREFIX = "account_delete_rate:"; @@ -30,28 +29,12 @@ export function registerAuthRoutes(app: App) { app.post("/api/auth/username/check", async (c: AppContext) => { const { username } = await c.req.json(); - const trimmed = String(username || "") - .trim() - .toLowerCase(); + const trimmed = normalizeUsername(username); const formatError = validateUsername(trimmed); if (formatError) { return c.json({ available: false, error: formatError }, 400); } - const moderationError = validateCleanUsername(trimmed); - if (moderationError) { - return c.json({ available: false, error: moderationError }, 400); - } - - if (!/^[a-zA-Z0-9_]+$/.test(trimmed)) { - return c.json( - { - available: false, - error: "Username can only contain letters, numbers, and underscores", - }, - 400, - ); - } const existing = await c .get("db") diff --git a/worker/routes/email.ts b/worker/routes/email.ts index a43e0a3..0129abc 100644 --- a/worker/routes/email.ts +++ b/worker/routes/email.ts @@ -2,10 +2,11 @@ import { deleteCookie, getCookie, setCookie } from "hono/cookie"; import { authUserPayload, createSession, + normalizeUsername, setAuthCookies, updateLastSeen, + validateUsername, } from "../lib"; -import { validateCleanUsername } from "../moderation"; import type { App, AppContext, User } from "../types"; const EMAIL_CODE_PREFIX = "email_code:"; @@ -338,16 +339,14 @@ export function registerEmailRoutes(app: App) { const { email } = JSON.parse(pendingData) as { email: string }; const body = await c.req.json(); - const trimmedUsername = String(body.username || "") - .trim() - .toLowerCase(); + const trimmedUsername = normalizeUsername(body.username); if (!trimmedUsername) { return c.json({ error: "Username is required" }, 400); } - const moderationError = validateCleanUsername(trimmedUsername); - if (moderationError) { - return c.json({ error: moderationError }, 400); + const usernameError = validateUsername(trimmedUsername); + if (usernameError) { + return c.json({ error: usernameError }, 400); } // Check if username exists diff --git a/worker/routes/oauth.ts b/worker/routes/oauth.ts index 2c2b53d..d19d63d 100644 --- a/worker/routes/oauth.ts +++ b/worker/routes/oauth.ts @@ -2,10 +2,11 @@ import { deleteCookie, getCookie, setCookie } from "hono/cookie"; import { authUserPayload, createSession, + normalizeUsername, setAuthCookies, updateLastSeen, + validateUsername, } from "../lib"; -import { validateCleanUsername } from "../moderation"; import type { App, AppContext, User } from "../types"; const OAUTH_STATE_PREFIX = "oauth_state:"; @@ -275,9 +276,8 @@ async function tryCreateOAuthUser( sub: string, email: string | undefined, ): Promise { - const trimmed = username.trim().toLowerCase(); - if (!trimmed) return null; - if (validateCleanUsername(trimmed)) return null; + const trimmed = normalizeUsername(username); + if (validateUsername(trimmed)) return null; const existing = await c .get("db") @@ -731,16 +731,14 @@ export function registerOAuthRoutes(app: App) { }; const { username } = await c.req.json(); - const trimmedUsername = String(username || "") - .trim() - .toLowerCase(); + const trimmedUsername = normalizeUsername(username); if (!trimmedUsername) { return c.json({ error: "Username is required" }, 400); } - const moderationError = validateCleanUsername(trimmedUsername); - if (moderationError) { - return c.json({ error: moderationError }, 400); + const usernameError = validateUsername(trimmedUsername); + if (usernameError) { + return c.json({ error: usernameError }, 400); } const result = await tryCreateOAuthUser( @@ -866,9 +864,9 @@ export function registerOAuthRoutes(app: App) { // New user with username provided — try to create if (username) { - const moderationError = validateCleanUsername(username); - if (moderationError) { - return c.json({ error: moderationError }, 400); + const usernameError = validateUsername(normalizeUsername(username)); + if (usernameError) { + return c.json({ error: usernameError }, 400); } const result = await tryCreateOAuthUser( c, @@ -975,9 +973,9 @@ export function registerOAuthRoutes(app: App) { // New user with username provided — try to create if (username) { - const moderationError = validateCleanUsername(username); - if (moderationError) { - return c.json({ error: moderationError }, 400); + const usernameError = validateUsername(normalizeUsername(username)); + if (usernameError) { + return c.json({ error: usernameError }, 400); } const result = await tryCreateOAuthUser( c,