From e87532aa7efdc12d2fe22f17902288a233b09d2f Mon Sep 17 00:00:00 2001 From: Luke Policinski Date: Mon, 28 Sep 2026 15:13:12 -0400 Subject: [PATCH 1/2] bug: harden website chat sends (auth, 2000-char cap, no relay of rejected messages) --- src/chat/chat.gateway.spec.ts | 276 +++++++++++++++++++++ src/chat/chat.gateway.ts | 72 +++++- src/chat/chat.service.spec.ts | 319 +++++++++++++++++++++++++ src/chat/chat.service.ts | 175 ++++++++++---- src/chat/enums/ChatErrorCode.ts | 7 + src/chat/types/ChatMessage.ts | 19 ++ src/chat/types/ChatSendResult.ts | 5 + src/matches/events/ChatMessageEvent.ts | 1 + 8 files changed, 820 insertions(+), 54 deletions(-) create mode 100644 src/chat/chat.gateway.spec.ts create mode 100644 src/chat/enums/ChatErrorCode.ts create mode 100644 src/chat/types/ChatMessage.ts create mode 100644 src/chat/types/ChatSendResult.ts diff --git a/src/chat/chat.gateway.spec.ts b/src/chat/chat.gateway.spec.ts new file mode 100644 index 000000000..acfd41117 --- /dev/null +++ b/src/chat/chat.gateway.spec.ts @@ -0,0 +1,276 @@ +import { ChatGateway } from "./chat.gateway"; +import { ChatService } from "./chat.service"; +import { ChatErrorCode } from "./enums/ChatErrorCode"; +import { ChatLobbyType } from "./enums/ChatLobbyTypes"; + +describe("ChatGateway lobby:chat", () => { + let chat: { sendMessageToChat: jest.Mock; sendChatToServer: jest.Mock }; + let gateway: ChatGateway; + + const client = (user: any = { steam_id: "1", name: "Luke", role: "user" }) => + ({ id: "client-1", user, send: jest.fn() }) as any; + + const sent = (socket: { send: jest.Mock }) => + socket.send.mock.calls.map(([raw]) => JSON.parse(raw)); + + beforeEach(() => { + chat = { + sendMessageToChat: jest.fn().mockResolvedValue({ accepted: true }), + sendChatToServer: jest.fn(), + }; + gateway = new ChatGateway(chat as any); + }); + + describe("input", () => { + it("ignores a socket that has not signed in", async () => { + const socket = client(null); + + await gateway.lobby( + { id: "m-1", type: ChatLobbyType.Match, message: "hi" }, + socket, + ); + + expect(chat.sendMessageToChat).not.toHaveBeenCalled(); + expect(chat.sendChatToServer).not.toHaveBeenCalled(); + }); + + it.each([ + ["a number", 5], + ["an object", { toString: "x" }], + ["an array", ["hi"]], + ["null", null], + ["missing", undefined], + ["only whitespace", " \n "], + ])("ignores a message that is %s", async (_, message) => { + const socket = client(); + + await expect( + gateway.lobby( + { id: "m-1", type: ChatLobbyType.Match, message }, + socket, + ), + ).resolves.toBeUndefined(); + + expect(chat.sendMessageToChat).not.toHaveBeenCalled(); + expect(socket.send).not.toHaveBeenCalled(); + }); + + it("ignores a lobby type it does not know", async () => { + await gateway.lobby( + { id: "m-1", type: "global" as ChatLobbyType, message: "hi" }, + client(), + ); + + expect(chat.sendMessageToChat).not.toHaveBeenCalled(); + }); + + it("ignores a room id that is not a string", async () => { + await gateway.lobby( + { id: { $ne: 1 } as any, type: ChatLobbyType.Match, message: "hi" }, + client(), + ); + + expect(chat.sendMessageToChat).not.toHaveBeenCalled(); + }); + + it("ignores a missing payload", async () => { + await expect( + gateway.lobby(undefined as any, client()), + ).resolves.toBeUndefined(); + + expect(chat.sendMessageToChat).not.toHaveBeenCalled(); + }); + + it("sends the trimmed text", async () => { + await gateway.lobby( + { id: "t-1", type: ChatLobbyType.Tournament, message: " hello " }, + client(), + ); + + expect(chat.sendMessageToChat).toHaveBeenCalledWith( + ChatLobbyType.Tournament, + "t-1", + expect.objectContaining({ steam_id: "1" }), + "hello", + ); + }); + }); + + describe("length", () => { + it("tells the sender a message is too long and never sends it", async () => { + const socket = client(); + + await gateway.lobby( + { + id: "m-1", + type: ChatLobbyType.Match, + message: "a".repeat(ChatService.MAX_MESSAGE_LENGTH + 1), + requestId: "r-1", + }, + socket, + ); + + expect(chat.sendMessageToChat).not.toHaveBeenCalled(); + expect(chat.sendChatToServer).not.toHaveBeenCalled(); + expect(sent(socket)).toEqual([ + { + event: "chat:error", + data: { code: ChatErrorCode.TooLong, max: 2000, requestId: "r-1" }, + }, + ]); + }); + + it("leaves requestId out when the client sent none", async () => { + const socket = client(); + + await gateway.lobby( + { + id: "m-1", + type: ChatLobbyType.Match, + message: "a".repeat(ChatService.MAX_MESSAGE_LENGTH + 1), + }, + socket, + ); + + expect(sent(socket)).toEqual([ + { event: "chat:error", data: { code: "too_long", max: 2000 } }, + ]); + }); + + it("accepts exactly the limit", async () => { + const message = "a".repeat(ChatService.MAX_MESSAGE_LENGTH); + + await gateway.lobby( + { id: "m-1", type: ChatLobbyType.Match, message }, + client(), + ); + + expect(chat.sendMessageToChat).toHaveBeenCalledWith( + ChatLobbyType.Match, + "m-1", + expect.anything(), + message, + ); + }); + }); + + describe("relaying to the game server", () => { + it("never relays a send the room refused", async () => { + // The relay has no membership check of its own: this is the only thing + // stopping any signed-in socket printing into any live match. + chat.sendMessageToChat.mockResolvedValue({ + accepted: false, + code: ChatErrorCode.NotAllowed, + }); + const socket = client(); + + await gateway.lobby( + { + id: "someone-elses-match", + type: ChatLobbyType.Match, + message: "gg", + requestId: "r-2", + }, + socket, + ); + + expect(chat.sendChatToServer).not.toHaveBeenCalled(); + expect(sent(socket)).toEqual([ + { + event: "chat:error", + data: { code: ChatErrorCode.NotAllowed, requestId: "r-2" }, + }, + ]); + }); + + it("says nothing about a refusal that carries no code", async () => { + chat.sendMessageToChat.mockResolvedValue({ accepted: false }); + const socket = client(); + + await gateway.lobby( + { id: "m-1", type: ChatLobbyType.Match, message: "gg" }, + socket, + ); + + expect(chat.sendChatToServer).not.toHaveBeenCalled(); + expect(socket.send).not.toHaveBeenCalled(); + }); + + it("relays an accepted match message", async () => { + await gateway.lobby( + { id: "m-1", type: ChatLobbyType.Match, message: 'say "gg"' }, + client(), + ); + + expect(chat.sendChatToServer).toHaveBeenCalledWith( + "m-1", + "Luke: say 'gg'", + ); + }); + + it("marks an organizer's relayed message", async () => { + await gateway.lobby( + { id: "m-1", type: ChatLobbyType.Match, message: "pause please" }, + client({ steam_id: "1", name: "Luke", role: "administrator" }), + ); + + expect(chat.sendChatToServer).toHaveBeenCalledWith( + "m-1", + "[organizer] Luke: pause please", + ); + }); + + it("never relays a team room", async () => { + await gateway.lobby( + { id: "m-1:l-1", type: ChatLobbyType.MatchTeam, message: "rush b" }, + client(), + ); + + expect(chat.sendMessageToChat).toHaveBeenCalled(); + expect(chat.sendChatToServer).not.toHaveBeenCalled(); + }); + + it.each([ + ChatLobbyType.Direct, + ChatLobbyType.Tournament, + ChatLobbyType.Draft, + ChatLobbyType.MatchMaking, + ChatLobbyType.Organizer, + ])("never relays a %s room", async (type) => { + await gateway.lobby({ id: "x", type, message: "hi" }, client()); + + expect(chat.sendChatToServer).not.toHaveBeenCalled(); + }); + }); + + describe("acknowledgement", () => { + it("acks an accepted send that carried a requestId", async () => { + const socket = client(); + + await gateway.lobby( + { + id: "t-1", + type: ChatLobbyType.Tournament, + message: "hi", + requestId: "r-3", + }, + socket, + ); + + expect(sent(socket)).toEqual([ + { event: "chat:ack", data: { requestId: "r-3" } }, + ]); + }); + + it("stays quiet for an accepted send without one", async () => { + const socket = client(); + + await gateway.lobby( + { id: "t-1", type: ChatLobbyType.Tournament, message: "hi" }, + socket, + ); + + expect(socket.send).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/src/chat/chat.gateway.ts b/src/chat/chat.gateway.ts index 2b7e0e163..d1d2bb4a4 100644 --- a/src/chat/chat.gateway.ts +++ b/src/chat/chat.gateway.ts @@ -7,6 +7,7 @@ import { import { ChatService } from "./chat.service"; import { FiveStackWebSocketClient } from "src/sockets/types/FiveStackWebSocketClient"; import { ChatLobbyType } from "./enums/ChatLobbyTypes"; +import { ChatErrorCode } from "./enums/ChatErrorCode"; import { isRoleAbove } from "@utilities/isRoleAbove"; @WebSocketGateway({ @@ -90,38 +91,95 @@ export class ChatGateway { @MessageBody() data: { id: string; - message: string; + message: unknown; type: ChatLobbyType; + requestId?: string; }, @ConnectedSocket() client: FiveStackWebSocketClient, ) { - if (!data.message) { + if (!client.user) { + return; + } + + if (!ChatGateway.isLobbyType(data?.type) || typeof data.id !== "string") { return; } - data.message = data.message.trim(); + const requestId = + typeof data.requestId === "string" ? data.requestId : undefined; - if (data.message.length === 0) { + const parsed = ChatService.messageText(data.message); + + if ("error" in parsed) { + if (parsed.error === ChatErrorCode.TooLong) { + this.sendError(client, parsed.error, requestId); + } return; } - await this.chat.sendMessageToChat( + const result = await this.chat.sendMessageToChat( data.type, data.id, client.user, - data.message, + parsed.text, ); + // Only a message the room accepted may reach the game server: the relay + // does no membership check of its own, so relaying regardless would let + // any signed-in socket print into any live match. + if (result.accepted === false) { + if (result.code) { + this.sendError(client, result.code, requestId); + } + return; + } + + if (requestId) { + this.sendAck(client, requestId); + } + if (data.type !== ChatLobbyType.Match) { return; } await this.chat.sendChatToServer( data.id, - `${isRoleAbove(client.user.role, "match_organizer") ? `[organizer] ` : ""}${client.user.name}: ${data.message}`.replaceAll( + `${isRoleAbove(client.user.role, "match_organizer") ? `[organizer] ` : ""}${client.user.name}: ${parsed.text}`.replaceAll( `"`, `'`, ), ); } + + private static isLobbyType(value: unknown): value is ChatLobbyType { + return Object.values(ChatLobbyType).includes(value as ChatLobbyType); + } + + private sendError( + client: FiveStackWebSocketClient, + code: ChatErrorCode, + requestId?: string, + ) { + client.send( + JSON.stringify({ + event: "chat:error", + data: { + code, + ...(code === ChatErrorCode.TooLong + ? { max: ChatService.MAX_MESSAGE_LENGTH } + : {}), + ...(requestId ? { requestId } : {}), + }, + }), + ); + } + + private sendAck(client: FiveStackWebSocketClient, requestId: string) { + client.send( + JSON.stringify({ + event: "chat:ack", + data: { requestId }, + }), + ); + } } diff --git a/src/chat/chat.service.spec.ts b/src/chat/chat.service.spec.ts index 8bf632efb..3ded7d99f 100644 --- a/src/chat/chat.service.spec.ts +++ b/src/chat/chat.service.spec.ts @@ -1,6 +1,8 @@ import { ChatService } from "./chat.service"; +import { ChatErrorCode } from "./enums/ChatErrorCode"; import { ChatLobbyType } from "./enums/ChatLobbyTypes"; import { directRoomId } from "./utilities/directRoomId"; +import { HasuraService } from "../hasura/hasura.service"; const ME = "76561198000000001"; const FRIEND = "76561198000000002"; @@ -215,6 +217,10 @@ describe("ChatService direct messages", () => { beforeEach(() => { jest.clearAllMocks(); + // clearAllMocks keeps implementations, so a test that seats someone in a + // room would otherwise leave them seated for every test after it. + redis.hget.mockResolvedValue(null); + redis.get.mockResolvedValue(null); acceptedFriendships = [[ME, FRIEND]]; myMatches = ["m-1"]; tournament = { @@ -426,6 +432,319 @@ describe("ChatService direct messages", () => { 'css_web_chat "x ; quit ; say"', ); }); + + const argument = (command: string) => + command.slice('css_web_chat "'.length, -1); + + it("relays a line at the limit untouched", async () => { + const line = "a".repeat(ChatService.RCON_MESSAGE_MAX_LENGTH); + + expect(argument(await relayed(line))).toBe(line); + }); + + it("cuts a longer line to the limit, ellipsis included", async () => { + const relayedLine = argument( + await relayed("a".repeat(ChatService.MAX_MESSAGE_LENGTH)), + ); + + expect(Array.from(relayedLine)).toHaveLength( + ChatService.RCON_MESSAGE_MAX_LENGTH, + ); + expect(relayedLine.endsWith("a…")).toBe(true); + }); + + it("never splits a character in two", async () => { + const relayedLine = argument(await relayed("πŸ˜€".repeat(300))); + + expect(Array.from(relayedLine)).toHaveLength( + ChatService.RCON_MESSAGE_MAX_LENGTH, + ); + expect(relayedLine).toBe( + `${"πŸ˜€".repeat(ChatService.RCON_MESSAGE_MAX_LENGTH - 1)}…`, + ); + }); + }); + + describe("message text", () => { + it.each([ + ["a number", 42], + ["null", null], + ["undefined", undefined], + ["an object", { message: "hi" }], + ["an array", ["hi"]], + ["empty", ""], + ["only whitespace", " \n\t "], + ])("refuses %s as invalid", (_, raw) => { + expect(ChatService.messageText(raw)).toEqual({ + error: ChatErrorCode.Invalid, + }); + }); + + it("trims what it accepts", () => { + expect(ChatService.messageText(" gg wp \n")).toEqual({ + text: "gg wp", + }); + }); + + it("accepts exactly the limit and refuses one more", () => { + const limit = "a".repeat(ChatService.MAX_MESSAGE_LENGTH); + + expect(ChatService.messageText(limit)).toEqual({ text: limit }); + expect(ChatService.messageText(`${limit}a`)).toEqual({ + error: ChatErrorCode.TooLong, + }); + }); + + it("measures after trimming", () => { + const limit = "a".repeat(ChatService.MAX_MESSAGE_LENGTH); + + expect(ChatService.messageText(` ${limit} `)).toEqual({ + text: limit, + }); + }); + + it("counts UTF-16 code units, as the browser does", () => { + const half = ChatService.MAX_MESSAGE_LENGTH / 2; + + expect(ChatService.messageText("πŸ˜€".repeat(half))).toEqual({ + text: "πŸ˜€".repeat(half), + }); + expect(ChatService.messageText("πŸ˜€".repeat(half + 1))).toEqual({ + error: ChatErrorCode.TooLong, + }); + }); + }); + + describe("sending", () => { + const player = (overrides: Record = {}) => + ({ + steam_id: ME, + name: "Someone", + role: "user", + avatar_url: "avatar", + profile_url: "profile", + ...overrides, + }) as any; + + const seatIn = (steamId: string) => + redis.hget.mockResolvedValue( + JSON.stringify({ user: { steam_id: steamId } }), + ); + + const stored = (key: string) => + redis.hset.mock.calls + .filter(([hash]) => hash === key) + .map(([, , value]) => JSON.parse(value)); + + it("stamps a website message with its source and a string steam id", async () => { + seatIn(ME); + + await expect( + service.sendMessageToChat( + ChatLobbyType.Match, + "m-1", + player(), + "hello", + ), + ).resolves.toEqual({ accepted: true }); + + const [message] = stored("chat_match_m-1"); + + expect(message).toMatchObject({ + message: "hello", + source: "web", + from: { steam_id: ME, name: "Someone", role: "user" }, + }); + expect(typeof message.from.steam_id).toBe("string"); + }); + + it("stamps a line from the game, and stores its steam id as a string", async () => { + await service.sendMessageToChat( + ChatLobbyType.Match, + "m-1", + player({ steam_id: BigInt(ME) }), + "from the server", + true, + "game", + ); + + const [message] = stored("chat_match_m-1"); + + expect(message.source).toBe("game"); + expect(message.from.steam_id).toBe(ME); + }); + + it("refuses a website message over the limit without storing it", async () => { + seatIn(ME); + + await expect( + service.sendMessageToChat( + ChatLobbyType.Match, + "m-1", + player(), + "a".repeat(ChatService.MAX_MESSAGE_LENGTH + 1), + ), + ).resolves.toEqual({ accepted: false, code: ChatErrorCode.TooLong }); + + expect(redis.hset).not.toHaveBeenCalled(); + expect(redis.publish).not.toHaveBeenCalled(); + }); + + it("does not hold a line from the game to the website limit", async () => { + const line = "g".repeat(ChatService.MAX_MESSAGE_LENGTH + 1); + + await expect( + service.sendMessageToChat( + ChatLobbyType.Match, + "m-1", + player(), + line, + true, + "game", + ), + ).resolves.toEqual({ accepted: true }); + + expect(stored("chat_match_m-1").at(0)?.message).toBe(line); + }); + + it("refuses someone who is not in the room", async () => { + await expect( + service.sendMessageToChat( + ChatLobbyType.Match, + "m-2", + player(), + "let me in", + ), + ).resolves.toEqual({ accepted: false, code: ChatErrorCode.NotAllowed }); + + expect(redis.hset).not.toHaveBeenCalled(); + }); + + describe("direct messages", () => { + const room = directRoomId(ME, FRIEND); + + const dmInserts = () => + queries.filter(({ sql }) => + sql.includes("INSERT INTO public.direct_messages"), + ); + + it("delivers to a friend", async () => { + seatIn(ME); + + await expect( + service.sendMessageToChat(ChatLobbyType.Direct, room, player(), "hi"), + ).resolves.toEqual({ accepted: true }); + + expect(dmInserts().at(0)?.bindings.slice(1)).toEqual([room, ME, "hi"]); + + const incoming = redis.publish.mock.calls + .map(([, payload]) => JSON.parse(payload)) + .find(({ event }) => event === "direct:incoming"); + + expect(incoming.steamId).toBe(FRIEND); + expect(incoming.data.message.source).toBe("web"); + }); + + it("stops a conversation the moment the friendship ends", async () => { + // Still seated in the room -- presence outlives the unfriend by up to + // a day, so it cannot be what decides this. + seatIn(ME); + acceptedFriendships = []; + + await expect( + service.sendMessageToChat( + ChatLobbyType.Direct, + room, + player(), + "still there?", + ), + ).resolves.toEqual({ + accepted: false, + code: ChatErrorCode.NotAllowed, + }); + + expect(dmInserts()).toHaveLength(0); + expect(redis.publish).not.toHaveBeenCalled(); + }); + + it("hands back history stamped as website messages", async () => { + postgres.query.mockResolvedValueOnce([ + { + id: "dm-1", + message: "old", + created_at: new Date("2026-01-01T00:00:00Z"), + steam_id: ME, + name: "Someone", + role: "user", + avatar_url: null, + profile_url: null, + }, + ]); + + const [message] = await service["getDirectMessages"](room); + + expect(message).toMatchObject({ + id: "dm-1", + source: "web", + from: { steam_id: ME }, + }); + }); + }); + + describe("who it is from", () => { + const cache = (entries: Record) => + redis.get.mockImplementation(async (key: string) => + key in entries ? JSON.stringify(entries[key]) : null, + ); + + const from = async () => { + await service.sendMessageToChat( + ChatLobbyType.Match, + "m-1", + player({ name: "Fresh Name", role: "user" }), + "hi", + true, + ); + + return stored("chat_match_m-1").at(0).from; + }; + + it("keeps the player's own role when only the name is cached", async () => { + cache({ [HasuraService.PLAYER_NAME_CACHE_KEY(ME)]: "Cached Name" }); + + expect(await from()).toMatchObject({ + name: "Cached Name", + role: "user", + }); + }); + + it("keeps the player's own name when only the role is cached", async () => { + cache({ [HasuraService.PLAYER_ROLE_CACHE_KEY(ME)]: "administrator" }); + + expect(await from()).toMatchObject({ + name: "Fresh Name", + role: "administrator", + }); + }); + + it("prefers both cached values when both are there", async () => { + cache({ + [HasuraService.PLAYER_NAME_CACHE_KEY(ME)]: "Cached Name", + [HasuraService.PLAYER_ROLE_CACHE_KEY(ME)]: "match_organizer", + }); + + expect(await from()).toMatchObject({ + name: "Cached Name", + role: "match_organizer", + }); + }); + + it("falls back when the cache holds null", async () => { + cache({ [HasuraService.PLAYER_ROLE_CACHE_KEY(ME)]: null }); + + expect((await from()).role).toBe("user"); + }); + }); }); describe("rosters", () => { diff --git a/src/chat/chat.service.ts b/src/chat/chat.service.ts index e75d52310..d887f5b2e 100644 --- a/src/chat/chat.service.ts +++ b/src/chat/chat.service.ts @@ -18,6 +18,10 @@ import { PostgresService } from "src/postgres/postgres.service"; import { chatThreadKey } from "src/notifications/push/notification-delivery"; import { SystemSettingName } from "src/system/enums/SystemSettingName"; import { parseDirectRoomId } from "./utilities/directRoomId"; +import { ChatErrorCode } from "./enums/ChatErrorCode"; +import { ChatMessage, ChatMessageSource } from "./types/ChatMessage"; +import { ChatSendResult } from "./types/ChatSendResult"; + @Injectable() export class ChatService { private redis: Redis; @@ -39,6 +43,14 @@ export class ChatService { private static readonly DEFAULT_TTL = 60 * 60; + // Measured in UTF-16 code units, the same unit a textarea's maxlength counts. + public static readonly MAX_MESSAGE_LENGTH = 2000; + + // What a relayed line is cut to in game. The game shows far less than a full + // website message, and 2000 characters of multibyte text can outgrow an rcon + // packet. + public static readonly RCON_MESSAGE_MAX_LENGTH = 240; + // A drafted free agent is on a roster and gets in that way; withdrawn means // they left the pool. private static readonly TOURNAMENT_CHAT_FREE_AGENT_STATUSES: e_tournament_free_agent_statuses_enum[] = @@ -513,57 +525,72 @@ export class ChatService { ); } + // What a player typed on the website, or why it cannot be sent. Lines relayed + // from the game are not held to this: the game has already limited them. + public static messageText( + raw: unknown, + ): { text: string } | { error: ChatErrorCode } { + if (typeof raw !== "string") { + return { error: ChatErrorCode.Invalid }; + } + + const text = raw.trim(); + + if (text.length === 0) { + return { error: ChatErrorCode.Invalid }; + } + + if (text.length > ChatService.MAX_MESSAGE_LENGTH) { + return { error: ChatErrorCode.TooLong }; + } + + return { text }; + } + public async sendMessageToChat( type: ChatLobbyType, id: string, player: User, _message: string, skipCheck = false, - ) { - // verify they are in the lobby - if (skipCheck === false) { - const userData = await this.getUserData(type, id, player.steam_id); - if (!userData) { - return; - } + source: ChatMessageSource = "web", + ): Promise { + let text = _message; - if ( - type === ChatLobbyType.Draft && - !(await this.canSendDraftMessage(id, player)) - ) { - return; - } + if (source === "web") { + const parsed = ChatService.messageText(_message); - // Room membership lives in redis for a day, so leaving the tournament - - // withdrawing from the free agent pool, or being dropped from a roster - - // has to be re-checked here rather than only at join time. - if ( - type === ChatLobbyType.Tournament && - !(await this.canAccessLobby(type, id, player)) - ) { - return; + if ("error" in parsed) { + return { accepted: false, code: parsed.error }; } + + text = parsed.text; + } + + if (skipCheck === false && !(await this.canPostIn(type, id, player))) { + return { accepted: false, code: ChatErrorCode.NotAllowed }; } const name = await this.redis.get( HasuraService.PLAYER_NAME_CACHE_KEY(player.steam_id), ); - const role: e_player_roles_enum = (await this.redis.get( + const role = await this.redis.get( HasuraService.PLAYER_ROLE_CACHE_KEY(player.steam_id), - )) as unknown as e_player_roles_enum; + ); const timestamp = new Date(); - const message = { + const message: ChatMessage = { // Both the history snapshot sent on join and the live broadcast carry the // message, so clients need something stable to recognize it by. id: randomUUID(), - message: _message, + message: text, timestamp: timestamp.toISOString(), + source, from: { - role: name ? JSON.parse(role) : player.role, - name: name ? JSON.parse(name) : player.name, - steam_id: player.steam_id, + role: ChatService.cachedOr(role, player.role), + name: ChatService.cachedOr(name, player.name), + steam_id: String(player.steam_id), avatar_url: player.avatar_url, profile_url: player.profile_url, }, @@ -601,12 +628,47 @@ export class ChatService { id, player, message.from.name, - _message, - ).catch( - (error) => { - this.logger.warn(`unable to notify ${type}:${id} of a message`, error); - }, - ); + text, + ).catch((error) => { + this.logger.warn(`unable to notify ${type}:${id} of a message`, error); + }); + + return { accepted: true }; + } + + // Being present in the room is the baseline. That presence lives in redis + // for a day, so the rooms whose membership can lapse in the meantime are + // re-checked against where it is actually decided: leaving a tournament + // (withdrawing from the free agent pool, being dropped from a roster), and + // an unfriend, which has to end a conversation that is still open. + private async canPostIn( + type: ChatLobbyType, + id: string, + user: User, + ): Promise { + if (!(await this.getUserData(type, id, user.steam_id))) { + return false; + } + + switch (type) { + case ChatLobbyType.Draft: + return await this.canSendDraftMessage(id, user); + case ChatLobbyType.Tournament: + case ChatLobbyType.Direct: + return await this.canAccessLobby(type, id, user); + default: + return true; + } + } + + // The name and role caches are separate keys with separate lifetimes, so + // either can be missing while the other is not. + private static cachedOr(cached: string | null, fallback: T): T { + if (cached === null) { + return fallback; + } + + return (JSON.parse(cached) as T) ?? fallback; } // Notifies the whole roster and lets the delivery gate decide who actually @@ -1198,18 +1260,22 @@ export class ChatService { [roomId], ); - return rows.reverse().map((row) => ({ - id: row.id, - message: row.message, - timestamp: new Date(row.created_at).toISOString(), - from: { - role: row.role, - name: row.name, - steam_id: row.steam_id, - avatar_url: row.avatar_url, - profile_url: row.profile_url, - }, - })); + return rows.reverse().map( + (row): ChatMessage => ({ + id: row.id, + message: row.message, + timestamp: new Date(row.created_at).toISOString(), + // Nothing relays from the game into a DM. + source: "web", + from: { + role: row.role, + name: row.name, + steam_id: row.steam_id, + avatar_url: row.avatar_url, + profile_url: row.profile_url, + }, + }), + ); } // Server-side read state, so unread counts survive a reload instead of @@ -1429,6 +1495,21 @@ export class ChatService { .trim(); } + // Cut on code points rather than code units, so an emoji at the boundary is + // dropped whole instead of leaving half a surrogate pair to be mangled. + private static clampForGame(message: string) { + const characters = Array.from(message); + + if (characters.length <= ChatService.RCON_MESSAGE_MAX_LENGTH) { + return message; + } + + return `${characters + .slice(0, ChatService.RCON_MESSAGE_MAX_LENGTH - 1) + .join("") + .trimEnd()}…`; + } + public async sendChatToServer(matchId: string, message: string) { try { const { matches_by_pk } = await this.hasuraService.query({ @@ -1465,7 +1546,7 @@ export class ChatService { : "sw_web_chat"; return await rcon.send( - `${command} "${ChatService.oneRconArgument(message)}"`, + `${command} "${ChatService.clampForGame(ChatService.oneRconArgument(message))}"`, ); } catch (error) { this.logger.warn( diff --git a/src/chat/enums/ChatErrorCode.ts b/src/chat/enums/ChatErrorCode.ts new file mode 100644 index 000000000..8f3e3b1c6 --- /dev/null +++ b/src/chat/enums/ChatErrorCode.ts @@ -0,0 +1,7 @@ +// Sent to the client as `chat:error { code }`, so these values are a contract +// with the web -- add to it, never rename. +export enum ChatErrorCode { + TooLong = "too_long", + NotAllowed = "not_allowed", + Invalid = "invalid", +} diff --git a/src/chat/types/ChatMessage.ts b/src/chat/types/ChatMessage.ts new file mode 100644 index 000000000..70b4ce468 --- /dev/null +++ b/src/chat/types/ChatMessage.ts @@ -0,0 +1,19 @@ +import { e_player_roles_enum } from "generated"; + +export type ChatMessageSource = "web" | "game"; + +export interface ChatMessage { + id: string; + message: string; + timestamp: string; + // Absent on messages stored before it was recorded. + source?: ChatMessageSource; + from: { + role: e_player_roles_enum; + name: string; + // Always a string: a 17 digit steam id does not survive being a JSON number. + steam_id: string; + avatar_url?: string; + profile_url?: string; + }; +} diff --git a/src/chat/types/ChatSendResult.ts b/src/chat/types/ChatSendResult.ts new file mode 100644 index 000000000..d4b3ed185 --- /dev/null +++ b/src/chat/types/ChatSendResult.ts @@ -0,0 +1,5 @@ +import { ChatErrorCode } from "../enums/ChatErrorCode"; + +export type ChatSendResult = + | { accepted: true } + | { accepted: false; code?: ChatErrorCode }; diff --git a/src/matches/events/ChatMessageEvent.ts b/src/matches/events/ChatMessageEvent.ts index af8d186d9..d0f21764d 100644 --- a/src/matches/events/ChatMessageEvent.ts +++ b/src/matches/events/ChatMessageEvent.ts @@ -31,6 +31,7 @@ export default class ChatMessageEvent extends MatchEventProcessor<{ players_by_pk, this.data.message, true, + "game", ); } } From 705101296afb28ddbf31223e4f63d5b7a13eac0c Mon Sep 17 00:00:00 2001 From: Luke Policinski Date: Mon, 28 Sep 2026 15:27:56 -0400 Subject: [PATCH 2/2] bug: re-check room membership on every chat send, fix the null-organizer match gate --- src/chat/chat.gateway.spec.ts | 6 +- src/chat/chat.gateway.ts | 10 +++- src/chat/chat.service.spec.ts | 100 +++++++++++++++++++++++++++++-- src/chat/chat.service.ts | 66 ++++++++++++-------- src/chat/types/ChatSendResult.ts | 2 +- 5 files changed, 149 insertions(+), 35 deletions(-) diff --git a/src/chat/chat.gateway.spec.ts b/src/chat/chat.gateway.spec.ts index acfd41117..f7e173aad 100644 --- a/src/chat/chat.gateway.spec.ts +++ b/src/chat/chat.gateway.spec.ts @@ -15,7 +15,9 @@ describe("ChatGateway lobby:chat", () => { beforeEach(() => { chat = { - sendMessageToChat: jest.fn().mockResolvedValue({ accepted: true }), + sendMessageToChat: jest + .fn() + .mockResolvedValue({ accepted: true, messageId: "msg-1" }), sendChatToServer: jest.fn(), }; gateway = new ChatGateway(chat as any); @@ -258,7 +260,7 @@ describe("ChatGateway lobby:chat", () => { ); expect(sent(socket)).toEqual([ - { event: "chat:ack", data: { requestId: "r-3" } }, + { event: "chat:ack", data: { requestId: "r-3", messageId: "msg-1" } }, ]); }); diff --git a/src/chat/chat.gateway.ts b/src/chat/chat.gateway.ts index d1d2bb4a4..52cc01ee1 100644 --- a/src/chat/chat.gateway.ts +++ b/src/chat/chat.gateway.ts @@ -135,7 +135,7 @@ export class ChatGateway { } if (requestId) { - this.sendAck(client, requestId); + this.sendAck(client, requestId, result.messageId); } if (data.type !== ChatLobbyType.Match) { @@ -174,11 +174,15 @@ export class ChatGateway { ); } - private sendAck(client: FiveStackWebSocketClient, requestId: string) { + private sendAck( + client: FiveStackWebSocketClient, + requestId: string, + messageId: string, + ) { client.send( JSON.stringify({ event: "chat:ack", - data: { requestId }, + data: { requestId, messageId }, }), ); } diff --git a/src/chat/chat.service.spec.ts b/src/chat/chat.service.spec.ts index 3ded7d99f..82bf9cfac 100644 --- a/src/chat/chat.service.spec.ts +++ b/src/chat/chat.service.spec.ts @@ -53,6 +53,9 @@ describe("ChatService direct messages", () => { // Which matches this player belongs to, by id. let myMatches: string[]; + // Matches this player can see but has no part in. Hasura answers + // is_organizer with NULL, not false, for a match nobody organizes. + let otherMatches: string[]; // The one tournament the fake knows about, and who is attached to it. let tournament: { organizers: string[]; @@ -167,7 +170,19 @@ describe("ChatService direct messages", () => { } if (query.matches_by_pk) { - return myMatches.includes(query.matches_by_pk.__args.id) + const matchId = query.matches_by_pk.__args.id; + + if (otherMatches.includes(matchId)) { + return { + matches_by_pk: { + is_coach: false, + is_organizer: null, + is_in_lineup: false, + }, + }; + } + + return myMatches.includes(matchId) ? { matches_by_pk: { is_coach: false, @@ -223,6 +238,7 @@ describe("ChatService direct messages", () => { redis.get.mockResolvedValue(null); acceptedFriendships = [[ME, FRIEND]]; myMatches = ["m-1"]; + otherMatches = ["mm-1"]; tournament = { organizers: [STRANGER], teamOwners: [], @@ -250,6 +266,20 @@ describe("ChatService direct messages", () => { const joined = () => redis.eval.mock.calls.length > 0; describe("joining", () => { + it("keeps a stranger out of a match nobody organizes", async () => { + // Every matchmaking match: is_organizer comes back NULL, and a strict + // `=== false` once read that as not-a-refusal. + await service.joinMatchLobby(client(ME), ChatLobbyType.Match, "mm-1"); + + expect(joined()).toBe(false); + }); + + it("lets a lineup player into their match", async () => { + await service.joinMatchLobby(client(ME), ChatLobbyType.Match, "m-1"); + + expect(joined()).toBe(true); + }); + it("lets accepted friends into their conversation", async () => { await service.joinMatchLobby( client(ME), @@ -453,6 +483,19 @@ describe("ChatService direct messages", () => { expect(relayedLine.endsWith("a…")).toBe(true); }); + it("never cuts a joined emoji or a flag apart", async () => { + const family = "πŸ‘¨β€πŸ‘©β€πŸ‘§"; + const flag = "πŸ‡ΈπŸ‡ͺ"; + + // 5 and 2 code points each, so neither lands exactly on the limit. + expect(argument(await relayed(family.repeat(100)))).toBe( + `${family.repeat(47)}…`, + ); + expect(argument(await relayed(flag.repeat(200)))).toBe( + `${flag.repeat(119)}…`, + ); + }); + it("never splits a character in two", async () => { const relayedLine = argument(await relayed("πŸ˜€".repeat(300))); @@ -546,7 +589,7 @@ describe("ChatService direct messages", () => { player(), "hello", ), - ).resolves.toEqual({ accepted: true }); + ).resolves.toEqual({ accepted: true, messageId: expect.any(String) }); const [message] = stored("chat_match_m-1"); @@ -602,11 +645,60 @@ describe("ChatService direct messages", () => { true, "game", ), - ).resolves.toEqual({ accepted: true }); + ).resolves.toEqual({ accepted: true, messageId: expect.any(String) }); expect(stored("chat_match_m-1").at(0)?.message).toBe(line); }); + it("answers with the id the message was stored under", async () => { + seatIn(ME); + + const result = await service.sendMessageToChat( + ChatLobbyType.Match, + "m-1", + player(), + "hello", + ); + + expect(result).toEqual({ + accepted: true, + messageId: stored("chat_match_m-1").at(0).id, + }); + }); + + it("refuses a player no longer on the match, though still seated", async () => { + // Presence outlives a lineup change: a player swapped out mid-match + // keeps the page, and the room, open. + seatIn(ME); + myMatches = []; + + await expect( + service.sendMessageToChat( + ChatLobbyType.Match, + "m-1", + player(), + "still here", + ), + ).resolves.toEqual({ accepted: false, code: ChatErrorCode.NotAllowed }); + + expect(redis.hset).not.toHaveBeenCalled(); + }); + + it("refuses a stranger seated in a match nobody organizes", async () => { + seatIn(ME); + + await expect( + service.sendMessageToChat( + ChatLobbyType.Match, + "mm-1", + player(), + "hello from outside", + ), + ).resolves.toEqual({ accepted: false, code: ChatErrorCode.NotAllowed }); + + expect(redis.hset).not.toHaveBeenCalled(); + }); + it("refuses someone who is not in the room", async () => { await expect( service.sendMessageToChat( @@ -633,7 +725,7 @@ describe("ChatService direct messages", () => { await expect( service.sendMessageToChat(ChatLobbyType.Direct, room, player(), "hi"), - ).resolves.toEqual({ accepted: true }); + ).resolves.toEqual({ accepted: true, messageId: expect.any(String) }); expect(dmInserts().at(0)?.bindings.slice(1)).toEqual([room, ME, "hi"]); diff --git a/src/chat/chat.service.ts b/src/chat/chat.service.ts index d887f5b2e..00ad315e7 100644 --- a/src/chat/chat.service.ts +++ b/src/chat/chat.service.ts @@ -207,10 +207,13 @@ export class ChatService { return false; } + // Truthiness, not `=== false`: is_match_organizer is NULL rather than + // false for a match with no organizer (every matchmaking match), and a + // strict comparison let anyone signed in into those rooms. if ( - matches_by_pk.is_coach === false && - matches_by_pk.is_in_lineup === false && - matches_by_pk.is_organizer === false + !matches_by_pk.is_coach && + !matches_by_pk.is_in_lineup && + !matches_by_pk.is_organizer ) { return false; } @@ -633,14 +636,15 @@ export class ChatService { this.logger.warn(`unable to notify ${type}:${id} of a message`, error); }); - return { accepted: true }; + return { accepted: true, messageId: message.id }; } - // Being present in the room is the baseline. That presence lives in redis - // for a day, so the rooms whose membership can lapse in the meantime are - // re-checked against where it is actually decided: leaving a tournament - // (withdrawing from the free agent pool, being dropped from a roster), and - // an unfriend, which has to end a conversation that is still open. + // Presence in the room only says someone joined it once: it lives in redis + // for a day, is refreshed by anyone else joining, and the game server seats + // whoever connects. Membership lapses underneath it -- a player swapped out + // of a live match, a free agent who withdrew, an unfriend -- so every send is + // held to the same rule as joining. Draft has its own, stricter once the + // match has been drafted. private async canPostIn( type: ChatLobbyType, id: string, @@ -650,15 +654,11 @@ export class ChatService { return false; } - switch (type) { - case ChatLobbyType.Draft: - return await this.canSendDraftMessage(id, user); - case ChatLobbyType.Tournament: - case ChatLobbyType.Direct: - return await this.canAccessLobby(type, id, user); - default: - return true; + if (type === ChatLobbyType.Draft) { + return await this.canSendDraftMessage(id, user); } + + return await this.canAccessLobby(type, id, user); } // The name and role caches are separate keys with separate lifetimes, so @@ -1495,19 +1495,35 @@ export class ChatService { .trim(); } - // Cut on code points rather than code units, so an emoji at the boundary is - // dropped whole instead of leaving half a surrogate pair to be mangled. + private static readonly GRAPHEMES = new Intl.Segmenter(undefined, { + granularity: "grapheme", + }); + + // Counted in code points, which bounds the bytes an rcon packet has to carry + // whatever the script. Cut only between graphemes, so a flag or a joined + // emoji at the boundary is dropped whole rather than left in pieces. private static clampForGame(message: string) { - const characters = Array.from(message); + const limit = ChatService.RCON_MESSAGE_MAX_LENGTH; - if (characters.length <= ChatService.RCON_MESSAGE_MAX_LENGTH) { + if (Array.from(message).length <= limit) { return message; } - return `${characters - .slice(0, ChatService.RCON_MESSAGE_MAX_LENGTH - 1) - .join("") - .trimEnd()}…`; + let clamped = ""; + let length = 0; + + for (const { segment } of ChatService.GRAPHEMES.segment(message)) { + const size = Array.from(segment).length; + + if (length + size > limit - 1) { + break; + } + + clamped += segment; + length += size; + } + + return `${clamped.trimEnd()}…`; } public async sendChatToServer(matchId: string, message: string) { diff --git a/src/chat/types/ChatSendResult.ts b/src/chat/types/ChatSendResult.ts index d4b3ed185..ff8e22944 100644 --- a/src/chat/types/ChatSendResult.ts +++ b/src/chat/types/ChatSendResult.ts @@ -1,5 +1,5 @@ import { ChatErrorCode } from "../enums/ChatErrorCode"; export type ChatSendResult = - | { accepted: true } + | { accepted: true; messageId: string } | { accepted: false; code?: ChatErrorCode };