From fb5be7de6b3bde748bdf2937949acdfa7e5b00af Mon Sep 17 00:00:00 2001 From: Carl Vitullo Date: Mon, 15 Dec 2025 19:27:45 -0500 Subject: [PATCH 1/5] Clear invalid session cookies when detected MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When session cookies are present but contain no valid userId (expired database sessions, corrupted cookies, or tampered data), the server now clears them by calling logout. This prevents clients from repeatedly sending invalid cookies on every request. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- app/models/session.server.ts | 24 +++++++++++++ ...12-15_3_invalid-session-cookie-clearing.md | 35 +++++++++++++++++++ 2 files changed, 59 insertions(+) create mode 100644 notes/2025-12-15_3_invalid-session-cookie-clearing.md diff --git a/app/models/session.server.ts b/app/models/session.server.ts index 15f67a7..a6ec04b 100644 --- a/app/models/session.server.ts +++ b/app/models/session.server.ts @@ -119,9 +119,33 @@ export const DbSessionKeys = { authGuildId: "guildId", } as const; +/** + * Check if a specific cookie is present in the request headers. + */ +function hasCookie(request: Request, cookieName: string): boolean { + const cookieHeader = request.headers.get("Cookie"); + if (!cookieHeader) return false; + // Match cookie name at start of string or after semicolon, followed by = + const regex = new RegExp(`(?:^|;\\s*)${cookieName}=`); + return regex.test(cookieHeader); +} + async function getUserId(request: Request): Promise { const session = await getDbSession(request.headers.get("Cookie")); const userId = session.get(CookieSessionKeys.userId) as string; + + // If session cookies are present but we got no userId, the cookies are + // invalid (e.g., session expired, database session deleted, cookie + // corrupted). Clear them to prevent the client from repeatedly sending + // invalid cookies. + if (!userId) { + const hasSessionCookie = hasCookie(request, "__session"); + const hasClientSessionCookie = hasCookie(request, "__client-session"); + if (hasSessionCookie || hasClientSessionCookie) { + throw await logout(request); + } + } + return userId; } diff --git a/notes/2025-12-15_3_invalid-session-cookie-clearing.md b/notes/2025-12-15_3_invalid-session-cookie-clearing.md new file mode 100644 index 0000000..6608926 --- /dev/null +++ b/notes/2025-12-15_3_invalid-session-cookie-clearing.md @@ -0,0 +1,35 @@ +# Invalid Session Cookie Clearing + +## Problem + +When session cookies were invalid (expired database sessions, corrupted cookies, tampered data), React Router's session utilities (`getCookieSession`, `getDbSession`) silently return new empty sessions without clearing the invalid cookies from the client. This caused: + +- Repeated failed session lookups on every request +- Client kept sending bad cookies indefinitely +- No way for the user to get a clean slate without manually clearing cookies + +## Solution + +Modified `getUserId()` in `session.server.ts` to detect when: +1. Session cookies are present in the request (`__session` or `__client-session`) +2. But no valid userId was found in the session + +When this condition is detected, we call `logout(request)` which: +- Destroys both session cookies (sets them to expire) +- Redirects to `/` + +This clears invalid cookies in a single location that all session-checking code paths flow through (`getUser`, `requireUserId`, `requireUser`). + +## Implementation + +Added `hasCookie(request, cookieName)` helper to check for cookie presence without parsing, then added a check in `getUserId()`: + +```typescript +if (!userId) { + const hasSessionCookie = hasCookie(request, "__session"); + const hasClientSessionCookie = hasCookie(request, "__client-session"); + if (hasSessionCookie || hasClientSessionCookie) { + throw await logout(request); + } +} +``` \ No newline at end of file From 208b233a23a86cf215a2380b48ee19eb75dd39bc Mon Sep 17 00:00:00 2001 From: Carl Vitullo Date: Mon, 15 Dec 2025 21:10:15 -0500 Subject: [PATCH 2/5] Use consolidated SSR Discord client --- app/commands/setupTickets.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/commands/setupTickets.ts b/app/commands/setupTickets.ts index 69b2791..5e1e99c 100644 --- a/app/commands/setupTickets.ts +++ b/app/commands/setupTickets.ts @@ -15,9 +15,8 @@ import { type ChatInputCommandInteraction, } from "discord.js"; -import { REST } from "@discordjs/rest"; - import db from "#~/db.server.js"; +import { ssrDiscordSdk as rest } from "#~/discord/api"; import { quoteMessageContent, type AnyCommand, @@ -25,11 +24,8 @@ import { type ModalCommand, type SlashCommand, } from "#~/helpers/discord"; -import { discordToken } from "#~/helpers/env.server"; import { fetchSettings, SETTINGS } from "#~/models/guilds.server"; -const rest = new REST({ version: "10" }).setToken(discordToken); - const DEFAULT_BUTTON_TEXT = "Open a private ticket with the moderators"; export const Command = [ From 1d295f47c4be2f3ec1afebe59a6f9327770399a3 Mon Sep 17 00:00:00 2001 From: Carl Vitullo Date: Tue, 16 Dec 2025 00:54:14 -0500 Subject: [PATCH 3/5] Fix forwarded message handling in reportUser MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Forwarded messages have empty content - actual content lives in messageSnapshots. This fix properly extracts content from snapshots and adds a "(forwarded)" indicator to logs. Policy: Track the forwarder (who shared), not original author. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- app/helpers/modLog.ts | 56 +++++++++++-------- ...2025-12-16_1_forwarded-message-handling.md | 30 ++++++++++ 2 files changed, 64 insertions(+), 22 deletions(-) create mode 100644 notes/2025-12-16_1_forwarded-message-handling.md diff --git a/app/helpers/modLog.ts b/app/helpers/modLog.ts index 503de94..c560b2b 100644 --- a/app/helpers/modLog.ts +++ b/app/helpers/modLog.ts @@ -2,7 +2,7 @@ import { formatDistanceToNowStrict } from "date-fns"; import { ChannelType, messageLink, - MessageType, + MessageReferenceType, type AnyThreadChannel, type APIEmbed, type Message, @@ -46,6 +46,20 @@ const ReadableReasons: Record = { [ReportReasons.modResolution]: "Mod vote resolved", [ReportReasons.spam]: "detected as spam", }; + +const isForwardedMessage = (message: Message): boolean => { + return message.reference?.type === MessageReferenceType.Forward; +}; + +const getMessageContent = (message: Message): string => { + if (isForwardedMessage(message)) { + // For forwards, content is in the snapshot + const snapshot = message.messageSnapshots.first(); + return snapshot?.content ?? message.content; + } + return message.content; +}; + interface Reported { message: Message; warnings: number; @@ -200,7 +214,7 @@ export const reportUser = async ({ // If it has the data for a poll, use a specialized formatting function const reportedMessage = message.poll ? quoteAndEscapePoll(message.poll) - : quoteAndEscape(message.content).trim(); + : quoteAndEscape(getMessageContent(message)).trim(); // Send the detailed log message to thread const [logMessage] = await Promise.all([ thread.send(logBody), @@ -243,13 +257,13 @@ export const reportUser = async ({ // Send summary to parent channel if possible if (thread.parent?.isSendable()) { - const singleLine = message.cleanContent - .slice(0, 80) - .replaceAll("\n", "\\n "); + // For forwarded messages, cleanContent is empty - use snapshot content instead + const content = isForwardedMessage(message) + ? getMessageContent(message) + : message.cleanContent; + const singleLine = content.slice(0, 80).replaceAll("\n", "\\n "); const truncatedMessage = - message.cleanContent.length > 80 - ? `${singleLine.slice(0, 80)}…` - : singleLine; + singleLine.length > 80 ? `${singleLine.slice(0, 80)}…` : singleLine; try { const stats = await getMessageStats(message); @@ -312,15 +326,7 @@ const constructLog = async ({ const { moderator } = await fetchSettings(lastReport.message.guild.id, [ SETTINGS.moderator, ]); - let { message } = lastReport; - if ( - // If there's a reference and it's not a reply, it's a forwarded message. - // Fetch the reference and track that message. - lastReport.message.type !== MessageType.Reply && - lastReport.message.reference - ) { - message = await message.fetchReference(); - } + const { message } = lastReport; // This should never be possible but we gotta satisfy types if (!moderator) { @@ -330,15 +336,21 @@ const constructLog = async ({ const { content: report, embeds: reactions = [] } = makeReportMessage(lastReport); - const preface = `${report} ${constructDiscordLink(message)} by <@${lastReport.message.author.id}> (${ + // Add indicator if this is forwarded content + const forwardNote = isForwardedMessage(message) ? " (forwarded)" : ""; + const preface = `${report}${forwardNote} ${constructDiscordLink(message)} by <@${lastReport.message.author.id}> (${ lastReport.message.author.username })`; const extra = origExtra ? `${origExtra}\n` : ""; - const embeds = [ - describeAttachments(message.attachments), - ...reactions, - ].filter((e): e is APIEmbed => Boolean(e)); + // For forwarded messages, get attachments from the snapshot + const attachments = isForwardedMessage(message) + ? (message.messageSnapshots.first()?.attachments ?? message.attachments) + : message.attachments; + + const embeds = [describeAttachments(attachments), ...reactions].filter( + (e): e is APIEmbed => Boolean(e), + ); return { content: truncateMessage(`${preface} -# ${extra}${formatDistanceToNowStrict(lastReport.message.createdAt)} ago · `).trim(), diff --git a/notes/2025-12-16_1_forwarded-message-handling.md b/notes/2025-12-16_1_forwarded-message-handling.md new file mode 100644 index 0000000..7bd7de3 --- /dev/null +++ b/notes/2025-12-16_1_forwarded-message-handling.md @@ -0,0 +1,30 @@ +# Forwarded Message Handling Fix + +## Problem + +`reportUser` in `app/helpers/modLog.ts` incorrectly handled Discord forwarded messages: + +1. Forward detection used `message.type !== MessageType.Reply && message.reference` - unreliable heuristic +2. `message.content` is empty for forwards; actual content lives in `messageSnapshots` +3. `constructLog` called `fetchReference()` which contradicted our policy to track forwarders + +## Solution + +Policy decision: Track the **forwarder** (person who shared), not original author. + +### Changes + +1. Added `isForwardedMessage()` helper using `MessageReferenceType.Forward` enum from discord.js +2. Added `getMessageContent()` to extract content from `messageSnapshots` for forwards +3. Updated content extraction in `reportUser` (line 217) to use `getMessageContent()` +4. Updated summary preview (line 260-266) to handle empty `cleanContent` on forwards +5. Removed broken `fetchReference()` block in `constructLog` - we want forwarder attribution +6. Added "(forwarded)" indicator to log preface +7. Attachments now pulled from snapshot for forwarded messages + +## Technical Notes + +- `MessageSnapshot` does NOT contain `author` - only content, attachments, embeds, etc. +- `fetchReference()` makes API call that can fail if original message deleted +- `message.author` on a forwarded message is the forwarder, which aligns with our tracking policy +- Import `MessageReferenceType` from discord.js (not discord-api-types) to satisfy ESLint enum comparison rules From ba65bba540ede114a8d9e3d7ff5e78fb46f597ed Mon Sep 17 00:00:00 2001 From: Carl Vitullo Date: Tue, 16 Dec 2025 02:40:02 -0500 Subject: [PATCH 4/5] Remove CPU limits, remove request from PR preview deployment --- cluster/deployment.yaml | 3 --- cluster/preview/deployment.yaml | 7 ------- 2 files changed, 10 deletions(-) diff --git a/cluster/deployment.yaml b/cluster/deployment.yaml index a575000..b0ea0ab 100644 --- a/cluster/deployment.yaml +++ b/cluster/deployment.yaml @@ -27,9 +27,6 @@ spec: requests: memory: "256Mi" cpu: "50m" - limits: - memory: "512Mi" - cpu: "500m" startupProbe: httpGet: path: /healthcheck diff --git a/cluster/preview/deployment.yaml b/cluster/preview/deployment.yaml index bada152..272ffd9 100644 --- a/cluster/preview/deployment.yaml +++ b/cluster/preview/deployment.yaml @@ -34,13 +34,6 @@ spec: env: - name: ENVIRONMENT value: staging - resources: - requests: - memory: "128Mi" - cpu: "50m" - limits: - memory: "256Mi" - cpu: "500m" startupProbe: httpGet: path: /healthcheck From ad50f55e1bb2ab410bd9cf3aa20594dba0935ca0 Mon Sep 17 00:00:00 2001 From: Carl Vitullo Date: Tue, 16 Dec 2025 03:01:45 -0500 Subject: [PATCH 5/5] Remove created array during honeypot check I spotted some increased CPU usage that was introduced around when this merged, bit of a shot in the dark here. --- app/discord/honeypotTracker.ts | 123 +++++++++++++++++---------------- 1 file changed, 63 insertions(+), 60 deletions(-) diff --git a/app/discord/honeypotTracker.ts b/app/discord/honeypotTracker.ts index 4ce1013..518f3ea 100644 --- a/app/discord/honeypotTracker.ts +++ b/app/discord/honeypotTracker.ts @@ -37,6 +37,7 @@ export async function startHoneypotTracking(client: Client) { throw Error("Missing guild info when tracking honeypot messages"); } let config: HoneypotConfig[]; + const { guild } = msg; const cacheEntry = configCache[msg.guildId]; if (!cacheEntry || cacheEntry.cachedAt + CACHE_TTL_IN_MS < Date.now()) { config = await db @@ -55,69 +56,71 @@ export async function startHoneypotTracking(client: Client) { config = cacheEntry.config; } - const channelIds = config.map((entry) => entry.channel_id); - if (channelIds.includes(msg.channelId)) { - const [member, message] = await Promise.all([ - msg.guild.members.fetch(msg.author.id).catch((_) => undefined), - msg.fetch().catch((_) => undefined), - ]); - if (!member || !message) { - log( - "debug", - "HoneypotTracker", - "unable to resolve member or message for honeypot", - ); - throw Error("unable to resolve member or message for honeypot"); - } - // Get moderator role for permission check - const { moderator: modRoleId } = await fetchSettings(msg.guildId, [ - SETTINGS.moderator, - ]); - if ( - (Array.isArray(member.roles) - ? member.roles.includes(modRoleId) - : member.roles.cache.has(modRoleId)) || - member.permissions.has("Administrator") - ) { - log( - "debug", - "HoneypotTracker", - "Mod posted in Honeypot channel, no action taken", - ); - return; - } - try { - // softban user (ban/unban) to clear all recent messages - await member.ban({ - reason: "honeypot spam detected", - deleteMessageSeconds: 604800, // 7 days - }); - await msg.guild.members.unban(member); - // log action - await reportUser({ - reason: ReportReasons.spam, - message: message, - staff: client.user ?? false, - }); - } catch (e) { - log( - "error", - "HoneypotTracker", - "Failed to softban user in honeypot channel", - { - guildId: msg.guildId, - userId: member.id, - channelId: msg.channelId, - error: e instanceof Error ? e.message : String(e), - }, - ); - await reportUser({ + if (!config.some((c) => c.channel_id === msg.channelId)) { + return; + } + const [member, message] = await Promise.all([ + guild.members.fetch(msg.author.id).catch((_) => undefined), + msg.fetch().catch((_) => undefined), + ]); + if (!member || !message) { + log( + "debug", + "HoneypotTracker", + "unable to resolve member or message for honeypot", + ); + throw Error("unable to resolve member or message for honeypot"); + } + // Get moderator role for permission check + const { moderator: modRoleId } = await fetchSettings(msg.guildId, [ + SETTINGS.moderator, + ]); + if ( + (Array.isArray(member.roles) + ? member.roles.includes(modRoleId) + : member.roles.cache.has(modRoleId)) || + member.permissions.has("Administrator") + ) { + log( + "debug", + "HoneypotTracker", + "Mod posted in Honeypot channel, no action taken", + ); + return; + } + try { + // softban user (ban/unban) to clear all recent messages + await Promise.all([ + member + .ban({ + reason: "honeypot spam detected", + deleteMessageSeconds: 604800, // 7 days + }) + .then(() => guild.members.unban(member)), + reportUser({ reason: ReportReasons.spam, message: message, staff: client.user ?? false, - extra: `Failed to softban user in honeypot channel: ${e instanceof Error ? e.message : String(e)}`, - }); - } + }), + ]); + } catch (e) { + log( + "error", + "HoneypotTracker", + "Failed to softban user in honeypot channel", + { + guildId: msg.guildId, + userId: member.id, + channelId: msg.channelId, + error: e, + }, + ); + await reportUser({ + reason: ReportReasons.spam, + message: message, + staff: client.user ?? false, + extra: `Failed to softban user in honeypot channel: ${e instanceof Error ? e.message : String(e)}`, + }); } }); }