From 6f5fd27fa83358c66e51e2ab93d557541e051150 Mon Sep 17 00:00:00 2001 From: Devin Zuczek Date: Tue, 27 Oct 2026 09:50:34 -0400 Subject: [PATCH] [api] lock down /api/messages/v2/send[multiple] --- apps/api/src/openapi.ts | 6 +- apps/api/src/routes/social.ts | 45 ++++++++++-- apps/api/src/test/integration/api.test.ts | 84 +++++++++++++++-------- packages/domain/src/enums.ts | 1 + 4 files changed, 98 insertions(+), 38 deletions(-) diff --git a/apps/api/src/openapi.ts b/apps/api/src/openapi.ts index 8beea0a..bd05f12 100644 --- a/apps/api/src/openapi.ts +++ b/apps/api/src/openapi.ts @@ -205,8 +205,7 @@ export const SendMessageRequest = z.object({ ToPlayerId: z.string().describe('Account id of the recipient'), Type: z .string() - .optional() - .describe('The Message-model type, e.g. `10`. Passed through unmapped; defaults to 0'), + .describe('The Message-model type, e.g. `10`. Only 1, 2, 10, 11, 30 and 41 are accepted'), Data: z.string().optional().describe('The message payload; often empty'), }) @@ -220,8 +219,7 @@ export const SendMultipleMessagesRequest = z.object({ ToPlayerIds: z.array(z.int()).describe('Account ids of the recipients'), Type: z .int() - .optional() - .describe('The Message-model type, e.g. `20`. Passed through unmapped; defaults to 0'), + .describe('The Message-model type, e.g. `10`. Only 1, 2, 10, 11, 30 and 41 are accepted'), Data: z.string().optional().describe('The message payload; often empty'), }) diff --git a/apps/api/src/routes/social.ts b/apps/api/src/routes/social.ts index 1ea9b42..66f1ace 100644 --- a/apps/api/src/routes/social.ts +++ b/apps/api/src/routes/social.ts @@ -56,6 +56,22 @@ import type { App } from '../context' /** The notifications hub is a single global DO instance (see the `notify` worker). */ const HUB_INSTANCE = 'global' +/** + * The only message types a PLAYER may send through `/api/messages/v2/send` (and its bulk + * form) — the six the client's `RecNet.Messages.SendMessage` wrappers post. Every other + * {@link MessageType} (cheers, friend requests, co-owner/club invites, the Coach, game + * invites themselves) is minted by this server from its own route; accepting them here + * would let any player forge one, from their own id, with a payload of their choosing. + */ +const PLAYER_SENDABLE_MESSAGE_TYPES: ReadonlySet = new Set([ + MessageType.GameInviteDeclined, + MessageType.GameJoinFailed, + MessageType.RequestGameInvite, + MessageType.RequestGameInviteDeclined, + MessageType.TextMessage, + MessageType.FriendCodeUsed, +]) + /** * Push the `MessageReceived` frame for an ALREADY-STORED message, resolving false when the * hub could not be reached. @@ -343,8 +359,11 @@ export const socialRoutes = new Hono({ strict: false }) 'stored id and the recipient reads the same message back from ' + '`GET /api/messages/v2/get` whether or not the push reached them. The sender is ' + 'the caller (from the bearer token), NOT a body field. `Type` is a Message-model ' + - 'type (a different enum from `NotificationType`) passed through unmapped, ' + - 'defaulting to 0; `Data` is the payload and is commonly empty.\n\n' + + 'type (a different enum from `NotificationType`) and is required: only the ' + + 'types the client itself posts are accepted — 1 GameInviteDeclined, 2 ' + + 'GameJoinFailed, 10 RequestGameInvite, 11 RequestGameInviteDeclined, 30 ' + + 'TextMessage, 41 FriendCodeUsed. Every other type is server-originated and ' + + 'refused with a 400. `Data` is the payload and is commonly empty.\n\n' + 'Answers the same `{ success, error }` envelope as the report / warning writes, ' + '`error` an empty string on success. A hub failure is still reported honestly as ' + 'a 500 with `success: false` — the message is in their inbox, but the caller ' + @@ -353,7 +372,7 @@ export const socialRoutes = new Hono({ strict: false }) requestBody: form(SendMessageRequest, 'The message'), responses: { 200: json(SuccessErrorEnvelope, '`{ success: true, error: "" }`'), - 400: json(SuccessErrorEnvelope, 'No `ToPlayerId` in the request'), + 400: json(SuccessErrorEnvelope, 'No `ToPlayerId`, or a missing or server-only `Type`'), 401: UNAUTHORIZED_RESPONSE, 500: json(SuccessErrorEnvelope, 'The notifications hub could not be reached'), }, @@ -373,10 +392,16 @@ export const socialRoutes = new Hono({ strict: false }) // and its id is what goes on the frame. A hub failure after this point leaves // the message in the recipient's inbox, which is the point of the store — they // read it on next login instead of losing it. + // The client always names the type; a missing or server-only one is refused. + const type = Number(str(body.Type) ?? Number.NaN) + if (!PLAYER_SENDABLE_MESSAGE_TYPES.has(type)) { + return c.json({ success: false, error: 'Invalid message type' }, 400) + } + const message = await createNotification(c.env.DB, { FromPlayerId: fromPlayerId, ToPlayerId: toPlayerId, - Type: Number.parseInt(str(body.Type) ?? '', 10) || 0, + Type: type, // `Data` stays a string, empty included — distinct from the null the types // carrying no payload of their own send. Data: str(body.Data) ?? '', @@ -406,7 +431,7 @@ export const socialRoutes = new Hono({ strict: false }) 'sender rule: the caller’s bearer token, never a body field. Stored the same way ' + 'too — one notification per recipient, each with its own id.\n\n' + 'The body is JSON rather than the single send’s form encoding, so `Type` is a ' + - 'number (still an unmapped Message-model type, defaulting to 0) and `Data` a ' + + 'number (limited to the same player-sendable types, 400 otherwise) and `Data` a ' + 'string, commonly empty. Repeated ids are delivered once.\n\n' + 'Answers the same `{ success, error }` envelope. Delivery is attempted for every ' + 'recipient even after one fails, but a hub failure for ANY of them is reported ' + @@ -417,7 +442,10 @@ export const socialRoutes = new Hono({ strict: false }) requestBody: jsonBody(SendMultipleMessagesRequest, 'The message and its recipients'), responses: { 200: json(SuccessErrorEnvelope, '`{ success: true, error: "" }`'), - 400: json(SuccessErrorEnvelope, 'No usable id in `ToPlayerIds`'), + 400: json( + SuccessErrorEnvelope, + 'No usable id in `ToPlayerIds`, or a missing or server-only `Type`' + ), 401: UNAUTHORIZED_RESPONSE, 500: json(SuccessErrorEnvelope, 'The notifications hub could not be reached'), }, @@ -443,7 +471,10 @@ export const socialRoutes = new Hono({ strict: false }) return c.json({ success: false, error: 'ToPlayerIds is required' }, 400) } - const type = typeof body.Type === 'number' ? body.Type : Number(body.Type) || 0 + const type = typeof body.Type === 'number' ? body.Type : Number(body.Type ?? Number.NaN) + if (!PLAYER_SENDABLE_MESSAGE_TYPES.has(type)) { + return c.json({ success: false, error: 'Invalid message type' }, 400) + } const data = typeof body.Data === 'string' ? body.Data : '' // Every recipient is attempted even if an earlier one fails — the reachable diff --git a/apps/api/src/test/integration/api.test.ts b/apps/api/src/test/integration/api.test.ts index 0d32292..3858a91 100644 --- a/apps/api/src/test/integration/api.test.ts +++ b/apps/api/src/test/integration/api.test.ts @@ -7136,19 +7136,18 @@ describe('images', () => { // An unknown id and a non-public one are absent rather than errors or holes, so the // answer can be shorter than the request. - expect((await bulk(`?ids=${one.Id}&ids=999999&ids=${hidden.Id}`)).map((i) => i.SavedImageId)).toEqual([ - one.Id, - ]) + expect( + (await bulk(`?ids=${one.Id}&ids=999999&ids=${hidden.Id}`)).map((i) => i.SavedImageId) + ).toEqual([one.Id]) // No ids at all is an empty array. expect(await bulk('')).toEqual([]) // More ids than D1 will bind in one query (100) — the lookup has to split. const many = [...Array.from({ length: 130 }, (_, i) => 800000 + i), one.Id, two.Id] - expect((await bulk(`?${many.map((id) => `ids=${id}`).join('&')}`)).map((i) => i.SavedImageId)).toEqual([ - one.Id, - two.Id, - ]) + expect( + (await bulk(`?${many.map((id) => `ids=${id}`).join('&')}`)).map((i) => i.SavedImageId) + ).toEqual([one.Id, two.Id]) }) test('POST /api/images/v1/cheer persists, syncs CheerCount, and the bulk lookup reflects it', async () => { @@ -8466,17 +8465,31 @@ describe('messages', () => { expect(inbox[0]).toMatchObject({ Id: frameId, FromPlayerId: 42, Type: 10, Data: '' }) }) - test('POST /api/messages/v2/send defaults Type and Data when omitted', async () => { - const res = await send({ ToPlayerId: '2' }, await bearer('42')) + test('POST /api/messages/v2/send defaults Data when omitted', async () => { + const res = await send({ ToPlayerId: '2', Type: '11' }, await bearer('42')) expect(res.status).toBe(200) expect((await pushed())[0]?.data).toMatchObject({ FromPlayerId: 42, ToPlayerId: 2, - Type: 0, + Type: 11, Data: '', }) }) + test('POST /api/messages/v2/send refuses a missing or server-only Type, storing nothing', async () => { + // 50 PlayerCheer, 100 CoachMessage, 5 VoteToKick, 4 FriendInvite: all minted by the + // server from their own routes; a player posting one here would be forging it. + for (const type of [undefined, '', 'nope', '0', '4', '5', '50', '100']) { + const fields: Record = { ToPlayerId: '9630' } + if (type !== undefined) fields.Type = type + const res = await send(fields, await bearer('42')) + expect(res.status).toBe(400) + expect(await res.json()).toEqual({ success: false, error: 'Invalid message type' }) + expect(await pushed()).toEqual([]) + } + expect(await inboxOf('9630')).toEqual([]) + }) + test('POST /api/messages/v2/send 400s without a recipient, pushing nothing', async () => { const res = await send({ Type: '10' }, await bearer('42')) expect(res.status).toBe(400) @@ -8485,7 +8498,7 @@ describe('messages', () => { }) test('POST /api/messages/v2/send is auth-gated', async () => { - const res = await send({ ToPlayerId: '2' }) + const res = await send({ ToPlayerId: '2', Type: '10' }) expect(res.status).toBe(401) expect(await pushed()).toEqual([]) }) @@ -8502,7 +8515,7 @@ describe('messages', () => { test('POST /api/messages/v1/sendMultiple pushes one frame per recipient', async () => { const res = await sendMultiple( - { ToPlayerIds: [205, 206], Type: 20, Data: 'hi' }, + { ToPlayerIds: [205, 206], Type: 30, Data: 'hi' }, await bearer('42') ) expect(res.status).toBe(200) @@ -8512,8 +8525,8 @@ describe('messages', () => { const sent = await pushed() expect(sent.map((n) => n.playerId)).toEqual([205, 206]) expect(sent.map((n) => n.notificationType)).toEqual([MESSAGE_RECEIVED, MESSAGE_RECEIVED]) - expect(sent[0]?.data).toMatchObject({ FromPlayerId: 42, ToPlayerId: 205, Type: 20, Data: 'hi' }) - expect(sent[1]?.data).toMatchObject({ FromPlayerId: 42, ToPlayerId: 206, Type: 20, Data: 'hi' }) + expect(sent[0]?.data).toMatchObject({ FromPlayerId: 42, ToPlayerId: 205, Type: 30, Data: 'hi' }) + expect(sent[1]?.data).toMatchObject({ FromPlayerId: 42, ToPlayerId: 206, Type: 30, Data: 'hi' }) // One STORED message each, with ids of their own — not one message shared. const ids = sent.map((n) => n.data.Id) @@ -8522,17 +8535,34 @@ describe('messages', () => { expect((await inboxOf('206'))[0]).toMatchObject({ Id: ids[1], Data: 'hi' }) }) - test('POST /api/messages/v1/sendMultiple defaults Type and Data, and de-duplicates ids', async () => { - const res = await sendMultiple({ ToPlayerIds: [205, 205] }, await bearer('42')) + test('POST /api/messages/v1/sendMultiple defaults Data, and de-duplicates ids', async () => { + const res = await sendMultiple({ ToPlayerIds: [205, 205], Type: 10 }, await bearer('42')) expect(res.status).toBe(200) const sent = await pushed() expect(sent).toHaveLength(1) - expect(sent[0]?.data).toMatchObject({ FromPlayerId: 42, ToPlayerId: 205, Type: 0, Data: '' }) + expect(sent[0]?.data).toMatchObject({ FromPlayerId: 42, ToPlayerId: 205, Type: 10, Data: '' }) + }) + + test('POST /api/messages/v1/sendMultiple refuses a missing or server-only Type', async () => { + for (const body of [ + { ToPlayerIds: [205] }, + { ToPlayerIds: [205], Type: 20 }, + { ToPlayerIds: [205], Type: 50 }, + ]) { + const res = await sendMultiple(body, await bearer('42')) + expect(res.status).toBe(400) + expect(await res.json()).toEqual({ success: false, error: 'Invalid message type' }) + expect(await pushed()).toEqual([]) + } }) test('POST /api/messages/v1/sendMultiple 400s with no usable recipient, pushing nothing', async () => { - for (const body of [{ Type: 20 }, { ToPlayerIds: [] }, { ToPlayerIds: ['nope', 0] }]) { + for (const body of [ + { Type: 10 }, + { ToPlayerIds: [], Type: 10 }, + { ToPlayerIds: ['nope', 0], Type: 10 }, + ]) { const res = await sendMultiple(body, await bearer('42')) expect(res.status).toBe(400) expect(await res.json()).toEqual({ success: false, error: 'ToPlayerIds is required' }) @@ -8541,20 +8571,20 @@ describe('messages', () => { }) test('POST /api/messages/v1/sendMultiple is auth-gated', async () => { - const res = await sendMultiple({ ToPlayerIds: [205] }) + const res = await sendMultiple({ ToPlayerIds: [205], Type: 10 }) expect(res.status).toBe(401) expect(await pushed()).toEqual([]) }) test('GET /api/messages/v2/get serves the caller’s inbox, newest first', async () => { // Two messages to 9600, one to somebody else — an inbox is the caller's own. - await send({ ToPlayerId: '9600', Data: 'first' }, await bearer('42')) - await send({ ToPlayerId: '9600', Data: 'second' }, await bearer('43')) - await send({ ToPlayerId: '9601', Data: 'not yours' }, await bearer('42')) + await send({ ToPlayerId: '9600', Type: '30', Data: 'first' }, await bearer('42')) + await send({ ToPlayerId: '9600', Type: '30', Data: 'second' }, await bearer('43')) + await send({ ToPlayerId: '9601', Type: '30', Data: 'not yours' }, await bearer('42')) const inbox = await inboxOf('9600') expect(inbox.map((m) => m.Data)).toEqual(['second', 'first']) - expect(inbox[0]).toMatchObject({ FromPlayerId: 43, ToPlayerId: 9600, Type: 0 }) + expect(inbox[0]).toMatchObject({ FromPlayerId: 43, ToPlayerId: 9600, Type: 30 }) // Every message carries the full Message model, ids and all. expect(Date.parse(inbox[0].SentTime)).not.toBeNaN() expect(inbox.every((m) => m.Id > 0)).toBe(true) @@ -8568,9 +8598,9 @@ describe('messages', () => { }) test('POST /api/messages/v3/delete removes only the caller’s messages', async () => { - await send({ ToPlayerId: '9610', Data: 'keep' }, await bearer('42')) - await send({ ToPlayerId: '9610', Data: 'drop' }, await bearer('42')) - await send({ ToPlayerId: '9611', Data: 'someone else' }, await bearer('42')) + await send({ ToPlayerId: '9610', Type: '30', Data: 'keep' }, await bearer('42')) + await send({ ToPlayerId: '9610', Type: '30', Data: 'drop' }, await bearer('42')) + await send({ ToPlayerId: '9611', Type: '30', Data: 'someone else' }, await bearer('42')) const before = await inboxOf('9610') const drop = before.find((m) => m.Data === 'drop')! @@ -8587,7 +8617,7 @@ describe('messages', () => { }) test('POST /api/messages/v3/delete takes a bare array, and shrugs off the rest', async () => { - await send({ ToPlayerId: '9620', Data: 'bare' }, await bearer('42')) + await send({ ToPlayerId: '9620', Type: '30', Data: 'bare' }, await bearer('42')) const message = (await inboxOf('9620'))[0] // The client posts a bare array; the documented `{ MessageIds }` object works too. diff --git a/packages/domain/src/enums.ts b/packages/domain/src/enums.ts index 957fd61..abd7b15 100644 --- a/packages/domain/src/enums.ts +++ b/packages/domain/src/enums.ts @@ -69,6 +69,7 @@ export enum MessageType { FriendStatusOnline = 20, TextMessage = 30, FriendRequestAccepted = 40, + FriendCodeUsed = 41, PlayerCheer = 50, PlayerCheerAnonymous = 51, RoomCoOwnerAdded = 60,