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 = [ 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)}`, + }); } }); } 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/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/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 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 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