diff --git a/apps/bot/src/features/automation/system/eventBridge.ts b/apps/bot/src/features/automation/system/eventBridge.ts index b05cf647..af33491e 100644 --- a/apps/bot/src/features/automation/system/eventBridge.ts +++ b/apps/bot/src/features/automation/system/eventBridge.ts @@ -211,7 +211,7 @@ export function registerActionEventListeners(client: Client): void { // Message created client.on("messageCreate", (message) => { - if (!message.guildId || message.author.bot) return; + if (!message.guildId || message.author?.bot) return; if (getRulesForEvent(message.guildId, "messageCreated").length === 0) return; const ctx = buildMessageContext("messageCreated", message); processEvent(client, ctx).catch((e) => @@ -229,24 +229,35 @@ export function registerActionEventListeners(client: Client): void { ); }); - // Reaction added/removed - client.on("messageReactionAdd", (reaction, user) => { - if (!reaction.message.guildId || user.bot) return; - if (getRulesForEvent(reaction.message.guildId, "reactionAdded").length === 0) return; - const ctx = buildReactionContext("reactionAdded", reaction, user); - processEvent(client, ctx).catch((e) => - handleError("reactionAdded", e), - ); - }); + // Reaction added/removed. + // + // Partials are enabled on the client, so these fire for messages the bot has + // never cached — which is the normal case for a pinned rules message. Both + // the reaction and the user must be resolved before the context is built, or + // every reaction automation on an older message stays silently broken. + const onReaction = ( + eventType: "reactionAdded" | "reactionRemoved", + ) => async ( + reaction: MessageReaction | PartialMessageReaction, + user: User | PartialUser, + ) => { + try { + if (user.bot) return; + if (reaction.partial) await reaction.fetch(); + if (user.partial) user = await user.fetch(); + const guildId = reaction.message.guildId; + if (!guildId) return; + if (getRulesForEvent(guildId, eventType).length === 0) return; + await processEvent(client, buildReactionContext(eventType, reaction, user)); + } catch (e) { + // A deleted message cannot be fetched back; that is not an error worth + // surfacing, but anything else is. + handleError(eventType, e); + } + }; - client.on("messageReactionRemove", (reaction, user) => { - if (!reaction.message.guildId || user.bot) return; - if (getRulesForEvent(reaction.message.guildId, "reactionRemoved").length === 0) return; - const ctx = buildReactionContext("reactionRemoved", reaction, user); - processEvent(client, ctx).catch((e) => - handleError("reactionRemoved", e), - ); - }); + client.on("messageReactionAdd", onReaction("reactionAdded")); + client.on("messageReactionRemove", onReaction("reactionRemoved")); // Role added/removed + nickname/timeout/boost (all via guildMemberUpdate) client.on("guildMemberUpdate", (oldMember, newMember) => { @@ -379,6 +390,10 @@ export function registerActionEventListeners(client: Client): void { channelId: thread.id, channelName: thread.name, channelMention: `<#${thread.id}>`, + // The thread IS the context channel, so a channel filter would never + // match anything a user could pick. Carry the parent so + // "only threads under #support" is expressible. + parentChannelId: thread.parentId ?? undefined, memberCount: thread.guild.memberCount, timestamp: new Date().toISOString(), extra: { diff --git a/apps/bot/src/features/automation/system/executor.ts b/apps/bot/src/features/automation/system/executor.ts index 6857738a..f81ac029 100644 --- a/apps/bot/src/features/automation/system/executor.ts +++ b/apps/bot/src/features/automation/system/executor.ts @@ -46,32 +46,60 @@ function checkRateLimit(guildId: string, eventType: string): boolean { // --- Condition matching --- +/** + * Trigger filters FAIL CLOSED: a configured filter the event context cannot + * answer means the rule does not fire. + * + * These guards used to read `conditions.excludeRoleIds?.length && + * context.member`, which silently *skipped* an unanswerable filter. The rule + * then fired on exactly the users and channels it was configured to exclude — + * "exclude @Staff" on a Member Banned rule announced every staff ban, because + * a ban context carries no member — while the dashboard counted the filter as + * active. An exclusion that cannot be evaluated is not a permission to + * proceed; it is a reason to stop. + * + * The editor is event-aware (EVENT_CONDITION_SUPPORT) so new rules cannot be + * built with a filter their trigger can never evaluate. + */ function matchesConditions( conditions: ActionConditions, context: EventContext, ): boolean { - // Include filters: if specified, context must match - if (conditions.channelIds?.length && context.channelId) { - if (!conditions.channelIds.includes(context.channelId)) return false; + // Channel filters test the parent too. On threadCreated the context channel + // is the brand-new thread, whose id the user cannot possibly have picked in + // the editor; the parent is the channel they actually chose. + const channelIds = [context.channelId, context.parentChannelId].filter( + (id): id is string => !!id, + ); + + // Include filters: the context must carry the datum AND match it. + if (conditions.channelIds?.length) { + if (channelIds.length === 0) return false; + if (!channelIds.some((id) => conditions.channelIds!.includes(id))) return false; } - if (conditions.userIds?.length && context.userId) { + if (conditions.userIds?.length) { + if (!context.userId) return false; if (!conditions.userIds.includes(context.userId)) return false; } - if (conditions.roleIds?.length && context.member) { + if (conditions.roleIds?.length) { + if (!context.member) return false; const hasMatchingRole = conditions.roleIds.some((id) => context.member!.roles.cache.has(id), ); if (!hasMatchingRole) return false; } - // Exclude filters: if matched, skip - if (conditions.excludeChannelIds?.length && context.channelId) { - if (conditions.excludeChannelIds.includes(context.channelId)) return false; + // Exclude filters: an unanswerable exclusion cannot be cleared, so it blocks. + if (conditions.excludeChannelIds?.length) { + if (channelIds.length === 0) return false; + if (channelIds.some((id) => conditions.excludeChannelIds!.includes(id))) return false; } - if (conditions.excludeUserIds?.length && context.userId) { + if (conditions.excludeUserIds?.length) { + if (!context.userId) return false; if (conditions.excludeUserIds.includes(context.userId)) return false; } - if (conditions.excludeRoleIds?.length && context.member) { + if (conditions.excludeRoleIds?.length) { + if (!context.member) return false; const hasExcludedRole = conditions.excludeRoleIds.some((id) => context.member!.roles.cache.has(id), ); @@ -111,9 +139,21 @@ function evaluateCondition( condition: StepConditionConfig, context: EventContext, ): boolean { - const actual = getContextValue(condition.field, context); const expected = condition.value; + // Role operators are answered entirely by the member's roles — they never + // consume `condition.field`. Checking the field first meant the canonical + // "if member has @Verified" branch always took the else path, because the + // field defaults to channelId and memberJoin never populates one. + if (condition.operator === "hasRole") { + return context.member?.roles.cache.has(expected) ?? false; + } + if (condition.operator === "notHasRole") { + if (!context.member) return false; + return !context.member.roles.cache.has(expected); + } + + const actual = getContextValue(condition.field, context); if (actual === undefined || actual === null) return false; const actualStr = String(actual); @@ -137,10 +177,6 @@ function evaluateCondition( return Number(actual) > Number(expected); case "lessThan": return Number(actual) < Number(expected); - case "hasRole": - return context.member?.roles.cache.has(expected) ?? false; - case "notHasRole": - return !(context.member?.roles.cache.has(expected) ?? true); case "inList": return expected .split(",") @@ -188,12 +224,12 @@ async function executeSteps( try { const executor = getExecutor(step.action.type as ActionType); if (!executor) { - logger.warn( - `Unknown action type: ${step.action.type} in rule "${rule.name}"`, - ); - } else { - await executor(client, context, step.action); + // Must not fall through to the success log below: a rule + // referencing a removed action type would report a clean 100% + // success rate while doing nothing at all. + throw new Error(`Unknown action type: ${step.action.type}`); } + await executor(client, context, step.action); logExecution(rule, step.action.type, true, null).catch(() => {}); } catch (error) { const err = diff --git a/apps/bot/src/features/automation/system/registry.ts b/apps/bot/src/features/automation/system/registry.ts index bbd63691..92708e92 100644 --- a/apps/bot/src/features/automation/system/registry.ts +++ b/apps/bot/src/features/automation/system/registry.ts @@ -32,13 +32,60 @@ function isValidEmoji(value: string): boolean { return UNICODE_EMOJI_REGEX.test(value); } +/** + * Every executor THROWS rather than returning when it cannot do its job. + * + * processEvent cannot distinguish "returned because there was nothing to do" + * from "succeeded", so a silent return was written to the ActionLog as + * `success: true`. A moderator whose auto-role rule never fired saw + * "214 executions, 100% success" and had nothing to debug with. Throwing lets + * the caller's existing catch record the real reason. + */ +function required(value: T | undefined | null, actionType: string, field: string): T { + if (value === undefined || value === null || value === "") { + throw new Error(`${actionType}: ${field} is required`); + } + return value; +} + +/** Fetches a sendable text channel, or throws explaining which one failed. */ +async function sendableChannel(client: Client, channelId: string, actionType: string) { + const channel = await client.channels.fetch(channelId).catch(() => null); + if (!channel?.isTextBased() || !("send" in channel)) { + throw new Error( + `${actionType}: channel ${channelId} is missing, not a text channel, or not visible to the bot`, + ); + } + return channel; +} + +/** + * Resolves the member to act on. Reaction, ban and message contexts often + * carry no member object, which is why addRole/removeRole silently did nothing + * on the single most common automation people build ("react here, get a role"). + */ +async function resolveMember(client: Client, ctx: EventContext, actionType: string) { + if (ctx.member) return ctx.member; + if (!ctx.userId || !ctx.guildId) { + throw new Error(`${actionType}: no member in context and no user/guild to resolve one from`); + } + try { + const guild = await client.guilds.fetch(ctx.guildId); + return await guild.members.fetch(ctx.userId); + } catch (err) { + throw new Error( + `${actionType}: could not resolve member ${ctx.userId} in guild ${ctx.guildId} (${err instanceof Error ? err.message : String(err)})`, + ); + } +} + const executors = new Map(); executors.set("sendMessage", async (client, ctx, config) => { - if (!config.channelId || !config.message) return; - const channel = await client.channels.fetch(config.channelId); - if (!channel?.isTextBased() || !("send" in channel)) return; - const resolved = resolveTemplate(config.message, ctx); + const channelId = required(config.channelId, "sendMessage", "channelId"); + const message = required(config.message, "sendMessage", "message"); + const channel = await sendableChannel(client, channelId, "sendMessage"); + const resolved = resolveTemplate(message, ctx); await channel.send({ content: resolved, allowedMentions: { users: ctx.userId ? [ctx.userId] : [] }, @@ -46,56 +93,55 @@ executors.set("sendMessage", async (client, ctx, config) => { }); executors.set("sendEmbed", async (client, ctx, config) => { - if (!config.channelId || !config.embed) return; - const channel = await client.channels.fetch(config.channelId); - if (!channel?.isTextBased() || !("send" in channel)) return; + const channelId = required(config.channelId, "sendEmbed", "channelId"); + const embedConfig = required(config.embed, "sendEmbed", "embed"); + const channel = await sendableChannel(client, channelId, "sendEmbed"); const embed = new EmbedBuilder().setTimestamp(); - if (config.embed.title) { - embed.setTitle(resolveTemplate(config.embed.title, ctx)); + if (embedConfig.title) { + embed.setTitle(resolveTemplate(embedConfig.title, ctx)); } - if (config.embed.description) { - embed.setDescription(resolveTemplate(config.embed.description, ctx)); + if (embedConfig.description) { + embed.setDescription(resolveTemplate(embedConfig.description, ctx)); } - if (config.embed.color !== undefined) { - embed.setColor(config.embed.color); + if (embedConfig.color !== undefined) { + embed.setColor(embedConfig.color); } - if (config.embed.footer) { - embed.setFooter({ text: resolveTemplate(config.embed.footer, ctx) }); + if (embedConfig.footer) { + embed.setFooter({ text: resolveTemplate(embedConfig.footer, ctx) }); } await channel.send({ embeds: [embed] }); }); executors.set("sendDM", async (client, ctx, config) => { - if (!config.message) return; - const resolved = resolveTemplate(config.message, ctx); - try { - // Prefer ctx.member.user if available, otherwise fetch the user directly - if (ctx.member) { - await ctx.member.user.send(resolved); - } else if (ctx.userId) { - const user = await client.users.fetch(ctx.userId); - await user.send(resolved); - } - } catch (err) { - logger.warn(`sendDM failed for user ${ctx.userId ?? "unknown"}: ${err instanceof Error ? err.message : String(err)}`); + const message = required(config.message, "sendDM", "message"); + const resolved = resolveTemplate(message, ctx); + // A closed DM is a real, reportable failure — the moderator needs to know + // their welcome DM is not arriving, not see it logged as a success. + if (ctx.member) { + await ctx.member.user.send(resolved); + return; } + const userId = required(ctx.userId, "sendDM", "a user to DM"); + const user = await client.users.fetch(userId); + await user.send(resolved); }); -executors.set("addRole", async (_client, ctx, config) => { - if (!ctx.member || !config.roleId) return; - await ctx.member.roles.add(config.roleId); +executors.set("addRole", async (client, ctx, config) => { + const roleId = required(config.roleId, "addRole", "roleId"); + const member = await resolveMember(client, ctx, "addRole"); + await member.roles.add(roleId); }); -executors.set("removeRole", async (_client, ctx, config) => { - if (!ctx.member || !config.roleId) return; - await ctx.member.roles.remove(config.roleId); +executors.set("removeRole", async (client, ctx, config) => { + const roleId = required(config.roleId, "removeRole", "roleId"); + const member = await resolveMember(client, ctx, "removeRole"); + await member.roles.remove(roleId); }); executors.set("logToChannel", async (client, ctx, config) => { - if (!config.channelId) return; - const channel = await client.channels.fetch(config.channelId); - if (!channel?.isTextBased() || !("send" in channel)) return; + const channelId = required(config.channelId, "logToChannel", "channelId"); + const channel = await sendableChannel(client, channelId, "logToChannel"); const eventInfo = EVENT_TYPES[ctx.eventType]; const embed = new EmbedBuilder() @@ -155,28 +201,60 @@ async function isPrivateHost(hostname: string): Promise { } } -executors.set("sendWebhook", async (_client, ctx, config) => { - if (!config.webhook?.url) return; +const MAX_WEBHOOK_REDIRECTS = 3; +const MAX_WEBHOOK_RESPONSE_BYTES = 64 * 1024; +/** Throws unless the URL is https and does not resolve to a private address. */ +async function assertSafeWebhookTarget(raw: string): Promise { let url: URL; try { - url = new URL(config.webhook.url); + url = new URL(raw); } catch { - logger.warn(`Invalid webhook URL in action config: ${config.webhook.url}`); - return; + throw new Error(`sendWebhook: "${raw}" is not a valid URL`); } - if (url.protocol !== "https:") { - logger.warn(`Webhook URL must use HTTPS: ${config.webhook.url}`); - return; + throw new Error(`sendWebhook: URL must use HTTPS, got ${url.protocol.replace(":", "")}`); } - if (await isPrivateHost(url.hostname)) { - logger.warn(`Webhook URL points to private/internal host: ${url.hostname}`); + throw new Error(`sendWebhook: ${url.hostname} resolves to a private/internal address`); + } + return url; +} + +/** + * Reads at most `MAX_WEBHOOK_RESPONSE_BYTES` and then cancels the stream. + * `await response.text()` buffers whatever the remote chooses to send, so a + * hostile or broken endpoint could stream until the bot ran out of memory. We + * only read at all to release the connection; the content is never used. + */ +async function drainCapped(response: { body?: unknown; text?: () => Promise }): Promise { + const body = response.body as + | { getReader(): { read(): Promise<{ done: boolean; value?: { length: number } }>; cancel(): unknown } } + | null + | undefined; + if (!body?.getReader) { + await response.text?.(); return; } + const reader = body.getReader(); + let read = 0; + try { + for (;;) { + const { done, value } = await reader.read(); + if (done) break; + read += value?.length ?? 0; + if (read >= MAX_WEBHOOK_RESPONSE_BYTES) break; + } + } finally { + await reader.cancel(); + } +} + +executors.set("sendWebhook", async (_client, ctx, config) => { + const rawUrl = required(config.webhook?.url, "sendWebhook", "webhook.url"); + let url = await assertSafeWebhookTarget(rawUrl); - let body = config.webhook.bodyTemplate ?? JSON.stringify({ + let body = config.webhook?.bodyTemplate ?? JSON.stringify({ event: ctx.eventType, guild: ctx.guildName, user: ctx.userName, @@ -203,7 +281,7 @@ executors.set("sendWebhook", async (_client, ctx, config) => { ]); const ALLOWED_PREFIX = "x-fluxcore-"; - const userHeaders = config.webhook.headers ?? {}; + const userHeaders = config.webhook?.headers ?? {}; const headers: Record = { "Content-Type": "application/json", }; @@ -219,49 +297,69 @@ executors.set("sendWebhook", async (_client, ctx, config) => { } } - const controller = new AbortController(); - const timeout = setTimeout(() => controller.abort(), 5000); + // Redirects are followed BY HAND so every hop is re-checked against the + // private-address rules. With fetch's automatic following, the SSRF guard ran + // once against the original hostname and a public host answering + // "302 -> http://169.254.169.254/" walked straight into the cloud metadata + // service. + for (let hop = 0; ; hop++) { + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), 5000); + let response: Response; + try { + response = await fetch(url.toString(), { + method: config.webhook?.method ?? "POST", + headers, + body, + signal: controller.signal, + redirect: "manual", + }); + } finally { + clearTimeout(timeout); + } - try { - const response = await fetch(config.webhook.url, { - method: config.webhook.method ?? "POST", - headers, - body, - signal: controller.signal, - }); - // Consume response body to release the connection - await response.text(); - } finally { - clearTimeout(timeout); + if (response.status >= 300 && response.status < 400) { + const location = response.headers.get("location"); + await drainCapped(response); + if (!location) { + throw new Error(`sendWebhook: ${response.status} redirect with no Location header`); + } + if (hop >= MAX_WEBHOOK_REDIRECTS) { + throw new Error(`sendWebhook: too many redirects (>${MAX_WEBHOOK_REDIRECTS})`); + } + url = await assertSafeWebhookTarget(new URL(location, url).toString()); + continue; + } + + await drainCapped(response); + if (response.status < 200 || response.status >= 300) { + throw new Error(`sendWebhook: ${url.hostname} responded ${response.status}`); + } + return; } }); // --- Set Nickname --- executors.set("setNickname", async (client, ctx, config) => { - if (!config.nickname) return; - try { - // Fetch member from guild if not in context - let member = ctx.member; - if (!member && ctx.userId && ctx.guildId) { - const guild = await client.guilds.fetch(ctx.guildId); - member = await guild.members.fetch(ctx.userId); - } - if (!member) return; - const resolved = resolveTemplate(config.nickname, ctx); - await member.setNickname(resolved.slice(0, 32)); - } catch (err) { - logger.warn(`setNickname failed for user ${ctx.userId ?? "unknown"}: ${err instanceof Error ? err.message : String(err)}`); - } + const nickname = required(config.nickname, "setNickname", "nickname"); + const member = await resolveMember(client, ctx, "setNickname"); + const resolved = resolveTemplate(nickname, ctx); + await member.setNickname(resolved.slice(0, 32)); }); // --- Create Thread --- executors.set("createThread", async (client, ctx, config) => { - if (!config.channelId || !config.threadName) return; - const channel = await client.channels.fetch(config.channelId); - if (!channel?.isTextBased() || !("threads" in channel)) return; - const resolved = resolveTemplate(config.threadName, ctx); + const channelId = required(config.channelId, "createThread", "channelId"); + const threadName = required(config.threadName, "createThread", "threadName"); + const channel = await client.channels.fetch(channelId).catch(() => null); + if (!channel?.isTextBased() || !("threads" in channel)) { + throw new Error( + `createThread: channel ${channelId} is missing, not visible to the bot, or does not support threads`, + ); + } + const resolved = resolveTemplate(threadName, ctx); await (channel as TextChannel).threads.create({ name: resolved.slice(0, 100), }); @@ -270,23 +368,18 @@ executors.set("createThread", async (client, ctx, config) => { // --- Add Reaction --- executors.set("addReaction", async (client, ctx, config) => { - if (!config.emoji || !ctx.extra?.["message.id"] || !ctx.channelId) return; - if (!isValidEmoji(config.emoji)) { - logger.warn( - `addReaction skipped: invalid emoji "${config.emoji}" for guild ${ctx.guildId ?? "unknown"}`, - ); - return; + const emoji = required(config.emoji, "addReaction", "emoji"); + if (!isValidEmoji(emoji)) { + throw new Error(`addReaction: "${emoji}" is not a valid unicode or custom emoji`); } - const channel = await client.channels.fetch(ctx.channelId); - if (!channel?.isTextBased() || !("messages" in channel)) return; - try { - const message = await (channel as TextChannel).messages.fetch( - ctx.extra["message.id"], - ); - await message.react(config.emoji); - } catch (err) { - logger.warn(`addReaction failed (emoji: ${config.emoji}, message: ${ctx.extra?.["message.id"] ?? "unknown"}): ${err instanceof Error ? err.message : String(err)}`); + const messageId = required(ctx.extra?.["message.id"], "addReaction", "a triggering message"); + const channelId = required(ctx.channelId, "addReaction", "a channel in the event context"); + const channel = await client.channels.fetch(channelId).catch(() => null); + if (!channel?.isTextBased() || !("messages" in channel)) { + throw new Error(`addReaction: channel ${channelId} is missing or not visible to the bot`); } + const message = await (channel as TextChannel).messages.fetch(messageId); + await message.react(emoji); }); export function getExecutor(actionType: ActionType): ActionExecutor | undefined { diff --git a/apps/bot/src/shared/client/ExtendedClient.ts b/apps/bot/src/shared/client/ExtendedClient.ts index a8e4ffaf..313a3ae0 100644 --- a/apps/bot/src/shared/client/ExtendedClient.ts +++ b/apps/bot/src/shared/client/ExtendedClient.ts @@ -1,4 +1,4 @@ -import { Client, Collection, GatewayIntentBits } from "discord.js"; +import { Client, Collection, GatewayIntentBits, Partials } from "discord.js"; import type { Command } from "@fluxcore/types"; export class ExtendedClient extends Client { @@ -15,6 +15,18 @@ export class ExtendedClient extends Client { GatewayIntentBits.MessageContent, // Privileged: required for message content access GatewayIntentBits.GuildMessageReactions, // Required for reaction events ], + // Without these, discord.js silently DROPS any event whose primary + // structure is not already in cache. The bot only caches messages it has + // seen since its last restart, so reactionAdded / reactionRemoved / + // messageDeleted never fired for an older message — which breaks every + // reaction-role automation attached to a pinned rules message, with no + // error anywhere. Handlers that read a partial must fetch it first. + partials: [ + Partials.Message, + Partials.Reaction, + Partials.Channel, + Partials.User, + ], }); } } diff --git a/apps/bot/tests/features/automation/system/eventBridge-partials.test.ts b/apps/bot/tests/features/automation/system/eventBridge-partials.test.ts new file mode 100644 index 00000000..6b378ba5 --- /dev/null +++ b/apps/bot/tests/features/automation/system/eventBridge-partials.test.ts @@ -0,0 +1,159 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("@fluxcore/config", () => ({ + config: { token: "t", clientId: "c", guildId: undefined, logLevel: "info" }, +})); + +vi.mock("@fluxcore/utils", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + logger: { warn: vi.fn(), error: vi.fn(), info: vi.fn(), debug: vi.fn() }, + }; +}); + +const mockProcessEvent = vi.fn().mockResolvedValue(undefined); +vi.mock("../../../../src/features/automation/system/executor.js", () => ({ + processEvent: (...args: unknown[]) => mockProcessEvent(...args), +})); + +const mockGetRulesForEvent = vi.fn().mockReturnValue([{ id: 1 }]); +vi.mock("@fluxcore/systems/actions/cache", () => ({ + getRulesForEvent: (...args: unknown[]) => mockGetRulesForEvent(...args), +})); + +const { registerActionEventListeners } = await import( + "../../../../src/features/automation/system/eventBridge.js" +); + +/** Captures the handlers registered via client.on so tests can invoke them. */ +function fakeClient() { + const handlers = new Map unknown>(); + return { + client: { on: (name: string, fn: (...args: unknown[]) => unknown) => handlers.set(name, fn) }, + handlers, + }; +} + +function partialReaction(overrides: Record = {}) { + return { + partial: true, + fetch: vi.fn().mockResolvedValue(undefined), + emoji: { toString: () => "🎉", name: "tada" }, + message: { + guildId: "g1", + guild: { name: "G", memberCount: 5 }, + channelId: "c1", + channel: { name: "general" }, + id: "m1", + url: "https://discord.com/x", + }, + ...overrides, + }; +} + +/** + * Partials are now enabled on the client, so reaction events arrive for + * messages the bot has not cached. If the bridge does not fetch them first it + * builds a context out of nulls — or drops the event entirely — and every + * reaction automation on an older message stays silently broken. + */ +describe("eventBridge resolves partials before building a context", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockGetRulesForEvent.mockReturnValue([{ id: 1 }]); + }); + + it("fetches a partial reaction before processing reactionAdded", async () => { + const { client, handlers } = fakeClient(); + registerActionEventListeners(client as never); + const reaction = partialReaction(); + + await handlers.get("messageReactionAdd")!(reaction, { id: "u1", bot: false, username: "ada" }); + + expect(reaction.fetch).toHaveBeenCalled(); + expect(mockProcessEvent).toHaveBeenCalled(); + }); + + it("fetches a partial user so the context carries a real username", async () => { + const { client, handlers } = fakeClient(); + registerActionEventListeners(client as never); + const user = { id: "u1", bot: false, partial: true, fetch: vi.fn().mockResolvedValue({ id: "u1", username: "ada", tag: "ada#1" }) }; + + await handlers.get("messageReactionAdd")!(partialReaction(), user); + + expect(user.fetch).toHaveBeenCalled(); + }); + + it("drops the event rather than throwing when the message is gone", async () => { + const { client, handlers } = fakeClient(); + registerActionEventListeners(client as never); + const reaction = partialReaction({ + fetch: vi.fn().mockRejectedValue(new Error("Unknown Message")), + }); + + await expect( + handlers.get("messageReactionAdd")!(reaction, { id: "u1", bot: false }), + ).resolves.not.toThrow(); + expect(mockProcessEvent).not.toHaveBeenCalled(); + }); + + it("still processes reactionRemoved for a partial reaction", async () => { + const { client, handlers } = fakeClient(); + registerActionEventListeners(client as never); + const reaction = partialReaction(); + + await handlers.get("messageReactionRemove")!(reaction, { id: "u1", bot: false }); + + expect(reaction.fetch).toHaveBeenCalled(); + expect(mockProcessEvent).toHaveBeenCalled(); + }); + + // A deleted message cannot be fetched back, so the bridge must not treat a + // null author as "this is a bot, skip it" — that would drop every + // messageDeleted event for an uncached message. + it("processes messageDeleted for a partial message with no author", async () => { + const { client, handlers } = fakeClient(); + registerActionEventListeners(client as never); + + await handlers.get("messageDelete")!({ + partial: true, + guildId: "g1", + guild: { name: "G", memberCount: 5 }, + channelId: "c1", + channel: { name: "general" }, + id: "m1", + author: null, + content: null, + }); + + expect(mockProcessEvent).toHaveBeenCalled(); + }); +}); + +describe("eventBridge threadCreated matches on the parent channel", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockGetRulesForEvent.mockReturnValue([{ id: 1 }]); + }); + + // context.channelId is the NEW thread, so "only threads under #support" + // could never match. The parent has to travel with the context for a channel + // filter on threadCreated to mean anything. + it("carries the parent channel id in the event context", async () => { + const { client, handlers } = fakeClient(); + registerActionEventListeners(client as never); + + await handlers.get("threadCreate")!({ + guildId: "g1", + guild: { name: "G", memberCount: 5 }, + id: "t1", + name: "help", + ownerId: "u1", + parentId: "parent-1", + }); + + const ctx = mockProcessEvent.mock.calls[0][1]; + expect(ctx.parentChannelId).toBe("parent-1"); + }); +}); diff --git a/apps/bot/tests/features/automation/system/executor-steps.test.ts b/apps/bot/tests/features/automation/system/executor-steps.test.ts new file mode 100644 index 00000000..4215adeb --- /dev/null +++ b/apps/bot/tests/features/automation/system/executor-steps.test.ts @@ -0,0 +1,238 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("@fluxcore/config", () => ({ + config: { token: "t", clientId: "c", guildId: undefined, logLevel: "info" }, +})); + +const mockGetGuildSettingsOrDefault = vi.fn().mockReturnValue({ globalEnabled: true, maxRules: 25 }); +vi.mock("@fluxcore/systems/actions/config", () => ({ + getGuildSettingsOrDefault: (...a: unknown[]) => mockGetGuildSettingsOrDefault(...a), +})); + +const mockGetRulesForEvent = vi.fn().mockReturnValue([]); +vi.mock("@fluxcore/systems/actions/cache", () => ({ + getRulesForEvent: (...a: unknown[]) => mockGetRulesForEvent(...a), +})); + +const mockLogExecution = vi.fn().mockResolvedValue(undefined); +vi.mock("@fluxcore/systems/actions/persistence", () => ({ + logExecution: (...a: unknown[]) => mockLogExecution(...a), +})); + +/** Records which action ran, in order, so branch/order assertions are exact. */ +const ran: string[] = []; +const mockExecutor = vi.fn(async (_c: unknown, _ctx: unknown, config: { type: string; message?: string }) => { + ran.push(config.message ?? config.type); +}); +vi.mock("../../../../src/features/automation/system/registry.js", () => ({ + getExecutor: vi.fn((type: string) => (type === "unknownAction" ? null : mockExecutor)), +})); + +const { processEvent } = await import( + "../../../../src/features/automation/system/executor.js" +); + +const ctx = { + eventType: "memberJoin" as const, + guildId: "g1", + guildName: "G", + userId: "u1", + userName: "ada", + channelId: "c1", + channelName: "general", + memberCount: 42, + timestamp: new Date().toISOString(), +}; + +function action(id: string, message: string, next: string | null) { + return { id, type: "action", action: { type: "sendMessage", message }, next }; +} + +function rule(steps: unknown[], entryStepId: string) { + return [{ name: "graph", enabled: true, conditions: {}, actions: [], steps, entryStepId }]; +} + +/** + * The v2 step graph is what the canvas actually saves once a rule has any + * condition or delay, and the executor prefers it over the flat action list — + * yet none of it was covered. These are the paths a user's branching workflow + * depends on. + */ +describe("executor — v2 step graph", () => { + beforeEach(() => { + vi.clearAllMocks(); + ran.length = 0; + mockGetGuildSettingsOrDefault.mockReturnValue({ globalEnabled: true, maxRules: 25 }); + }); + + it("runs steps in `next` order, not array order", async () => { + mockGetRulesForEvent.mockReturnValueOnce( + rule( + [action("a", "first", "c"), action("b", "third", null), action("c", "second", "b")], + "a", + ), + ); + + await processEvent({} as never, ctx); + + expect(ran).toEqual(["first", "second", "third"]); + }); + + it("prefers the step graph over the flat action list", async () => { + mockGetRulesForEvent.mockReturnValueOnce([ + { + name: "both", + enabled: true, + conditions: {}, + actions: [{ type: "sendMessage", message: "legacy" }], + steps: [action("a", "graph", null)], + entryStepId: "a", + }, + ]); + + await processEvent({} as never, ctx); + + expect(ran).toEqual(["graph"]); + }); + + it("takes the then-branch when the condition matches", async () => { + mockGetRulesForEvent.mockReturnValueOnce( + rule( + [ + { id: "q", type: "condition", condition: { field: "channelName", operator: "equals", value: "general" }, thenNext: "yes", elseNext: "no" }, + action("yes", "matched", null), + action("no", "missed", null), + ], + "q", + ), + ); + + await processEvent({} as never, ctx); + + expect(ran).toEqual(["matched"]); + }); + + it("takes the else-branch when it does not", async () => { + mockGetRulesForEvent.mockReturnValueOnce( + rule( + [ + { id: "q", type: "condition", condition: { field: "channelName", operator: "equals", value: "other" }, thenNext: "yes", elseNext: "no" }, + action("yes", "matched", null), + action("no", "missed", null), + ], + "q", + ), + ); + + await processEvent({} as never, ctx); + + expect(ran).toEqual(["missed"]); + }); + + it("stops cleanly on a null branch", async () => { + mockGetRulesForEvent.mockReturnValueOnce( + rule( + [{ id: "q", type: "condition", condition: { field: "channelName", operator: "equals", value: "nope" }, thenNext: "yes", elseNext: null }, action("yes", "matched", null)], + "q", + ), + ); + + await processEvent({} as never, ctx); + + expect(ran).toEqual([]); + }); + + it("caps a cyclic graph instead of looping forever", async () => { + mockGetRulesForEvent.mockReturnValueOnce( + rule([action("a", "loop", "b"), action("b", "loop", "a")], "a"), + ); + + await processEvent({} as never, ctx); + + // MAX_STEP_ITERATIONS is 20 and every iteration here is an action. + expect(ran.length).toBe(20); + }); + + it("stops when `next` points at a step that does not exist", async () => { + mockGetRulesForEvent.mockReturnValueOnce( + rule([action("a", "only", "ghost")], "a"), + ); + + await expect(processEvent({} as never, ctx)).resolves.not.toThrow(); + expect(ran).toEqual(["only"]); + }); + + it("keeps going after one action fails, and logs the failure", async () => { + mockExecutor.mockRejectedValueOnce(new Error("boom")); + mockGetRulesForEvent.mockReturnValueOnce( + rule([action("a", "bad", "b"), action("b", "good", null)], "a"), + ); + + await processEvent({} as never, ctx); + + expect(ran).toEqual(["good"]); + expect(mockLogExecution).toHaveBeenCalledWith( + expect.anything(), "sendMessage", false, "boom", + ); + }); + + it("runs a delay step and continues", async () => { + vi.useFakeTimers(); + mockGetRulesForEvent.mockReturnValueOnce( + rule([{ id: "d", type: "delay", delayMs: 1000, next: "a" }, action("a", "after", null)], "d"), + ); + + const done = processEvent({} as never, ctx); + await vi.advanceTimersByTimeAsync(1000); + await done; + vi.useRealTimers(); + + expect(ran).toEqual(["after"]); + }); + + it("does not run the graph without an entry step", async () => { + mockGetRulesForEvent.mockReturnValueOnce([ + { name: "no-entry", enabled: true, conditions: {}, actions: [], steps: [action("a", "x", null)] }, + ]); + + await processEvent({} as never, ctx); + + expect(ran).toEqual([]); + }); + + it("skips the whole graph when the rule's trigger filters do not match", async () => { + mockGetRulesForEvent.mockReturnValueOnce([ + { + name: "filtered", + enabled: true, + conditions: { channelIds: ["somewhere-else"] }, + actions: [], + steps: [action("a", "x", null)], + entryStepId: "a", + }, + ]); + + await processEvent({} as never, ctx); + + expect(ran).toEqual([]); + }); + + it("logs each condition evaluation with its outcome", async () => { + mockGetRulesForEvent.mockReturnValueOnce( + rule( + [{ id: "q", type: "condition", condition: { field: "channelName", operator: "equals", value: "general" }, thenNext: null, elseNext: null }], + "q", + ), + ); + + await processEvent({} as never, ctx); + + expect(mockLogExecution).toHaveBeenCalledWith( + expect.anything(), + "condition:channelName", + true, + null, + expect.objectContaining({ result: true }), + ); + }); +}); diff --git a/apps/bot/tests/features/automation/system/executor.test.ts b/apps/bot/tests/features/automation/system/executor.test.ts index 49f4567f..b5d83a34 100644 --- a/apps/bot/tests/features/automation/system/executor.test.ts +++ b/apps/bot/tests/features/automation/system/executor.test.ts @@ -214,3 +214,290 @@ describe("action executor - processEvent", () => { expect(mockSendMessageExecutor).toHaveBeenCalledTimes(2); }); }); + +/** + * Filters used to be guarded on the presence of their own datum + * (`conditions.excludeRoleIds?.length && context.member`), so a filter the + * event context could not answer was skipped rather than failed. The rule then + * fired on exactly the users and channels it had been configured to skip, while + * the dashboard showed the filter as active. Every case below is a rule that + * must NOT fire. + */ +describe("action executor - filters fail closed", () => { + const memberlessContext = { + eventType: "memberBanned" as const, + guildId: "guild-123", + guildName: "Test Guild", + userId: "user-456", + userName: "TestUser", + userTag: "TestUser#0001", + userMention: "<@user-456>", + memberCount: 100, + timestamp: new Date().toISOString(), + }; + + beforeEach(() => { + vi.clearAllMocks(); + mockGetGuildSettingsOrDefault.mockReturnValue({ + globalEnabled: true, + maxRules: 25, + logChannelId: null, + }); + }); + + function ruleWith(conditions: Record) { + return [ + { name: "filtered", enabled: true, conditions, actions: [{ type: "sendMessage" }] }, + ]; + } + + it("does not fire an exclude-role filter when the event carries no member", async () => { + mockGetRulesForEvent.mockReturnValueOnce(ruleWith({ excludeRoleIds: ["staff"] })); + + await processEvent({} as never, memberlessContext); + + expect(mockSendMessageExecutor).not.toHaveBeenCalled(); + }); + + it("does not fire an include-role filter when the event carries no member", async () => { + mockGetRulesForEvent.mockReturnValueOnce(ruleWith({ roleIds: ["verified"] })); + + await processEvent({} as never, memberlessContext); + + expect(mockSendMessageExecutor).not.toHaveBeenCalled(); + }); + + it("does not fire a channel filter when the event carries no channel", async () => { + mockGetRulesForEvent.mockReturnValueOnce(ruleWith({ channelIds: ["general"] })); + + await processEvent({} as never, memberlessContext); + + expect(mockSendMessageExecutor).not.toHaveBeenCalled(); + }); + + it("does not fire an exclude-channel filter when the event carries no channel", async () => { + mockGetRulesForEvent.mockReturnValueOnce(ruleWith({ excludeChannelIds: ["general"] })); + + await processEvent({} as never, memberlessContext); + + expect(mockSendMessageExecutor).not.toHaveBeenCalled(); + }); + + it("does not fire a user filter when the event carries no user", async () => { + mockGetRulesForEvent.mockReturnValueOnce(ruleWith({ userIds: ["someone"] })); + + await processEvent({} as never, { + eventType: "channelCreated" as const, + guildId: "guild-123", + guildName: "Test Guild", + channelId: "channel-789", + memberCount: 100, + timestamp: new Date().toISOString(), + }); + + expect(mockSendMessageExecutor).not.toHaveBeenCalled(); + }); + + it("still fires when the filter is satisfiable and matches", async () => { + const withMember = { + ...memberlessContext, + member: { roles: { cache: new Map([["verified", {}]]) } }, + }; + mockGetRulesForEvent.mockReturnValueOnce(ruleWith({ roleIds: ["verified"] })); + + await processEvent({} as never, withMember as never); + + expect(mockSendMessageExecutor).toHaveBeenCalled(); + }); + + it("still skips when the filter is satisfiable and does not match", async () => { + const withMember = { + ...memberlessContext, + member: { roles: { cache: new Map([["other", {}]]) } }, + }; + mockGetRulesForEvent.mockReturnValueOnce(ruleWith({ roleIds: ["verified"] })); + + await processEvent({} as never, withMember as never); + + expect(mockSendMessageExecutor).not.toHaveBeenCalled(); + }); + + it("leaves unfiltered rules alone", async () => { + mockGetRulesForEvent.mockReturnValueOnce(ruleWith({})); + + await processEvent({} as never, memberlessContext); + + expect(mockSendMessageExecutor).toHaveBeenCalled(); + }); + + // On threadCreated the context channel is the brand-new thread, so a channel + // filter could only ever match an id the user has no way to know. The parent + // is what they actually picked in the editor. + it("matches a channel filter against the parent channel", async () => { + const threadCtx = { + eventType: "threadCreated" as const, + guildId: "guild-123", + guildName: "Test Guild", + channelId: "thread-new", + parentChannelId: "support", + memberCount: 100, + timestamp: new Date().toISOString(), + }; + mockGetRulesForEvent.mockReturnValueOnce(ruleWith({ channelIds: ["support"] })); + + await processEvent({} as never, threadCtx); + + expect(mockSendMessageExecutor).toHaveBeenCalled(); + }); + + it("excludes on the parent channel too", async () => { + const threadCtx = { + eventType: "threadCreated" as const, + guildId: "guild-123", + guildName: "Test Guild", + channelId: "thread-new", + parentChannelId: "support", + memberCount: 100, + timestamp: new Date().toISOString(), + }; + mockGetRulesForEvent.mockReturnValueOnce(ruleWith({ excludeChannelIds: ["support"] })); + + await processEvent({} as never, threadCtx); + + expect(mockSendMessageExecutor).not.toHaveBeenCalled(); + }); +}); + +describe("action executor - the log tells the truth", () => { + const ctx = { + eventType: "memberJoin" as const, + guildId: "guild-123", + guildName: "Test Guild", + userId: "user-456", + memberCount: 100, + timestamp: new Date().toISOString(), + }; + + beforeEach(() => { + vi.clearAllMocks(); + mockGetGuildSettingsOrDefault.mockReturnValue({ globalEnabled: true, maxRules: 25 }); + }); + + // The step-mode branch warned about an unknown action type and then fell + // through to the success log, so a rule referencing a removed action type + // reported a clean 100% success rate forever. + it("does not log success for an unknown action type in step mode", async () => { + mockGetRulesForEvent.mockReturnValueOnce([ + { + name: "unknown-step", + enabled: true, + conditions: {}, + actions: [], + entryStepId: "s0", + steps: [{ id: "s0", type: "action", action: { type: "unknownAction" }, next: null }], + }, + ]); + + await processEvent({} as never, ctx); + + expect(mockLogExecution).not.toHaveBeenCalledWith( + expect.anything(), expect.anything(), true, expect.anything(), + ); + }); + + it("logs a failure for an unknown action type in step mode", async () => { + mockGetRulesForEvent.mockReturnValueOnce([ + { + name: "unknown-step", + enabled: true, + conditions: {}, + actions: [], + entryStepId: "s0", + steps: [{ id: "s0", type: "action", action: { type: "unknownAction" }, next: null }], + }, + ]); + + await processEvent({} as never, ctx); + + expect(mockLogExecution).toHaveBeenCalledWith( + expect.anything(), "unknownAction", false, expect.stringMatching(/unknown action type/i), + ); + }); +}); + +/** + * Condition steps read `getContextValue(condition.field)` and bail when it is + * undefined — BEFORE looking at the operator. hasRole/notHasRole do not consume + * the field at all, so the canonical "if member has @Verified" branch always + * took the else path on memberJoin (whose context has no channelId, the + * default field). + */ +describe("action executor - role operators ignore the unrelated field", () => { + const memberWithRole = { + eventType: "memberJoin" as const, + guildId: "guild-123", + guildName: "Test Guild", + userId: "user-456", + memberCount: 100, + timestamp: new Date().toISOString(), + member: { roles: { cache: new Map([["verified", {}]]) } }, + }; + + beforeEach(() => { + vi.clearAllMocks(); + mockGetGuildSettingsOrDefault.mockReturnValue({ globalEnabled: true, maxRules: 25 }); + }); + + function branchRule(operator: string) { + return [ + { + name: "branch", + enabled: true, + conditions: {}, + actions: [], + entryStepId: "cond", + steps: [ + { + id: "cond", + type: "condition", + // channelId is the default field and memberJoin never populates it. + condition: { field: "channelId", operator, value: "verified" }, + thenNext: "yes", + elseNext: null, + }, + { id: "yes", type: "action", action: { type: "sendMessage" }, next: null }, + ], + }, + ]; + } + + it("takes the then-branch when the member has the role", async () => { + mockGetRulesForEvent.mockReturnValueOnce(branchRule("hasRole")); + + await processEvent({} as never, memberWithRole as never); + + expect(mockSendMessageExecutor).toHaveBeenCalled(); + }); + + it("takes the else-branch when the member lacks the role", async () => { + mockGetRulesForEvent.mockReturnValueOnce(branchRule("hasRole")); + + await processEvent({} as never, { + ...memberWithRole, + member: { roles: { cache: new Map() } }, + } as never); + + expect(mockSendMessageExecutor).not.toHaveBeenCalled(); + }); + + it("notHasRole takes the then-branch when the member lacks the role", async () => { + mockGetRulesForEvent.mockReturnValueOnce(branchRule("notHasRole")); + + await processEvent({} as never, { + ...memberWithRole, + member: { roles: { cache: new Map() } }, + } as never); + + expect(mockSendMessageExecutor).toHaveBeenCalled(); + }); +}); diff --git a/apps/bot/tests/features/automation/system/registry-emoji.test.ts b/apps/bot/tests/features/automation/system/registry-emoji.test.ts index ff4fe285..5cc6ad93 100644 --- a/apps/bot/tests/features/automation/system/registry-emoji.test.ts +++ b/apps/bot/tests/features/automation/system/registry-emoji.test.ts @@ -54,15 +54,17 @@ describe("addReaction emoji validation", () => { const client = mkClient(fetchSpy); const executor = getExecutor("addReaction")!; - await executor(client, baseCtx as never, { - type: "addReaction", - emoji: "not-an-emoji-at-all", - } as never); + // A bad emoji is now a thrown failure rather than a warn-and-return, so + // processEvent records it as `success: false` instead of logging a + // successful execution for an action that never ran. + await expect( + executor(client, baseCtx as never, { + type: "addReaction", + emoji: "not-an-emoji-at-all", + } as never), + ).rejects.toThrow(/not a valid unicode or custom emoji/); expect(fetchSpy).not.toHaveBeenCalled(); - expect(warn).toHaveBeenCalledWith( - expect.stringContaining("invalid emoji"), - ); }); it("accepts a unicode emoji", async () => { @@ -102,10 +104,12 @@ describe("addReaction emoji validation", () => { const fetchSpy = vi.fn(); const client = mkClient(fetchSpy); const executor = getExecutor("addReaction")!; - await executor(client, baseCtx as never, { - type: "addReaction", - emoji: ":smile:", - } as never); + await expect( + executor(client, baseCtx as never, { + type: "addReaction", + emoji: ":smile:", + } as never), + ).rejects.toThrow(/not a valid unicode or custom emoji/); expect(fetchSpy).not.toHaveBeenCalled(); }); }); diff --git a/apps/bot/tests/features/automation/system/registry-failures.test.ts b/apps/bot/tests/features/automation/system/registry-failures.test.ts new file mode 100644 index 00000000..2336a10b --- /dev/null +++ b/apps/bot/tests/features/automation/system/registry-failures.test.ts @@ -0,0 +1,174 @@ +import { describe, it, expect, vi } from "vitest"; + +vi.mock("@fluxcore/config", () => ({ + config: { token: "t", clientId: "c", guildId: undefined, logLevel: "info" }, +})); + +vi.mock("@fluxcore/utils", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + logger: { warn: vi.fn(), error: vi.fn(), info: vi.fn(), debug: vi.fn() }, + }; +}); + +vi.mock("node:dns/promises", () => ({ + lookup: vi.fn().mockResolvedValue({ address: "1.1.1.1", family: 4 }), +})); + +const { getExecutor } = await import( + "../../../../src/features/automation/system/registry.js" +); + +const baseCtx = { + eventType: "messageCreated" as const, + guildId: "g1", + userId: "u1", + userName: "alice", + userTag: "alice#0001", + userMention: "<@u1>", + channelId: "c1", + guildName: "G", + memberCount: 10, + timestamp: new Date().toISOString(), +}; + +/** + * Every executor used to `return` when it could not do its job — no channel + * configured, no member to act on, an unresolvable role. processEvent has no + * way to tell that apart from success, so it wrote `success: true` to the + * ActionLog. A moderator whose auto-role rule never fired saw "214 executions, + * 100% success" and had nothing to debug with. + * + * Every case below must reject, so the caller's existing catch logs it as a + * failure with a readable reason. + */ +describe("action executors fail loudly rather than silently doing nothing", () => { + function clientWithNoChannel() { + return { channels: { fetch: vi.fn().mockResolvedValue(null) } } as never; + } + + it("sendMessage rejects when no channel is configured", async () => { + const run = getExecutor("sendMessage")!; + await expect(run(clientWithNoChannel(), baseCtx, { type: "sendMessage", message: "hi" })) + .rejects.toThrow(/channelId/i); + }); + + it("sendMessage rejects when no message is configured", async () => { + const run = getExecutor("sendMessage")!; + await expect(run(clientWithNoChannel(), baseCtx, { type: "sendMessage", channelId: "c1" })) + .rejects.toThrow(/message/i); + }); + + it("sendMessage rejects when the channel cannot be resolved", async () => { + const run = getExecutor("sendMessage")!; + await expect( + run(clientWithNoChannel(), baseCtx, { type: "sendMessage", channelId: "gone", message: "hi" }), + ).rejects.toThrow(/channel/i); + }); + + it("addRole rejects when no role is configured", async () => { + const run = getExecutor("addRole")!; + await expect(run({} as never, baseCtx, { type: "addRole" })).rejects.toThrow(/roleId/i); + }); + + // The canonical reaction-role automation: "react to this message, get a + // role". A reaction context carries no member, so this silently did nothing + // and reported success for every reaction. + it("addRole resolves the member from the guild when the context has none", async () => { + const add = vi.fn().mockResolvedValue(undefined); + const client = { + guilds: { + fetch: vi.fn().mockResolvedValue({ + members: { fetch: vi.fn().mockResolvedValue({ roles: { add, remove: vi.fn() } }) }, + }), + }, + } as never; + + await getExecutor("addRole")!(client, baseCtx, { type: "addRole", roleId: "r1" }); + + expect(add).toHaveBeenCalledWith("r1"); + }); + + it("addRole rejects when the member cannot be resolved", async () => { + const client = { + guilds: { + fetch: vi.fn().mockResolvedValue({ + members: { fetch: vi.fn().mockRejectedValue(new Error("Unknown Member")) }, + }), + }, + } as never; + + await expect( + getExecutor("addRole")!(client, baseCtx, { type: "addRole", roleId: "r1" }), + ).rejects.toThrow(/member/i); + }); + + it("removeRole resolves the member from the guild when the context has none", async () => { + const remove = vi.fn().mockResolvedValue(undefined); + const client = { + guilds: { + fetch: vi.fn().mockResolvedValue({ + members: { fetch: vi.fn().mockResolvedValue({ roles: { add: vi.fn(), remove } }) }, + }), + }, + } as never; + + await getExecutor("removeRole")!(client, baseCtx, { type: "removeRole", roleId: "r1" }); + + expect(remove).toHaveBeenCalledWith("r1"); + }); + + it("sendDM rejects when the DM cannot be delivered", async () => { + const client = { + users: { fetch: vi.fn().mockResolvedValue({ send: vi.fn().mockRejectedValue(new Error("Cannot send messages to this user")) }) }, + } as never; + + await expect( + getExecutor("sendDM")!(client, baseCtx, { type: "sendDM", message: "hi" }), + ).rejects.toThrow(/cannot send/i); + }); + + it("setNickname rejects when Discord refuses the change", async () => { + const client = { + guilds: { + fetch: vi.fn().mockResolvedValue({ + members: { + fetch: vi.fn().mockResolvedValue({ + setNickname: vi.fn().mockRejectedValue(new Error("Missing Permissions")), + }), + }, + }), + }, + } as never; + + await expect( + getExecutor("setNickname")!(client, baseCtx, { type: "setNickname", nickname: "n" }), + ).rejects.toThrow(/missing permissions/i); + }); + + it("addReaction rejects on an invalid emoji instead of warning and returning", async () => { + const client = { channels: { fetch: vi.fn() } } as never; + + await expect( + getExecutor("addReaction")!( + client, + { ...baseCtx, extra: { "message.id": "m1" } }, + { type: "addReaction", emoji: "not an emoji" }, + ), + ).rejects.toThrow(/emoji/i); + }); + + it("logToChannel rejects when the channel cannot be resolved", async () => { + const run = getExecutor("logToChannel")!; + await expect(run(clientWithNoChannel(), baseCtx, { type: "logToChannel", channelId: "gone" })) + .rejects.toThrow(/channel/i); + }); + + it("createThread rejects when the channel cannot be resolved", async () => { + const run = getExecutor("createThread")!; + await expect( + run(clientWithNoChannel(), baseCtx, { type: "createThread", channelId: "gone", threadName: "t" }), + ).rejects.toThrow(/channel/i); + }); +}); diff --git a/apps/bot/tests/features/automation/system/registry-webhook-safety.test.ts b/apps/bot/tests/features/automation/system/registry-webhook-safety.test.ts new file mode 100644 index 00000000..ef144c2b --- /dev/null +++ b/apps/bot/tests/features/automation/system/registry-webhook-safety.test.ts @@ -0,0 +1,156 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("@fluxcore/config", () => ({ + config: { token: "t", clientId: "c", guildId: undefined, logLevel: "info" }, +})); + +vi.mock("@fluxcore/utils", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + logger: { warn: vi.fn(), error: vi.fn(), info: vi.fn(), debug: vi.fn() }, + }; +}); + +const lookup = vi.hoisted(() => vi.fn()); +vi.mock("node:dns/promises", () => ({ lookup })); + +const { getExecutor } = await import( + "../../../../src/features/automation/system/registry.js" +); + +const ctx = { + eventType: "memberJoin" as const, + guildId: "g1", + guildName: "G", + userId: "u1", + userName: "alice", + memberCount: 1, + timestamp: new Date().toISOString(), +}; + +function cfg(url: string) { + return { type: "sendWebhook", webhook: { url } }; +} + +/** A Response-alike carrying a body of `size` bytes, streamed in 1 KiB chunks. */ +function streamingResponse(size: number, status = 200) { + let sent = 0; + return { + status, + ok: status >= 200 && status < 300, + headers: { get: () => null }, + body: { + getReader() { + return { + async read() { + if (sent >= size) return { done: true, value: undefined }; + const chunk = Math.min(1024, size - sent); + sent += chunk; + return { done: false, value: new Uint8Array(chunk) }; + }, + cancel: vi.fn(), + releaseLock() {}, + }; + }, + }, + text: async () => "x".repeat(size), + }; +} + +describe("sendWebhook safety", () => { + beforeEach(() => { + vi.restoreAllMocks(); + lookup.mockResolvedValue({ address: "1.1.1.1", family: 4 }); + }); + + it("rejects a missing URL rather than returning silently", async () => { + await expect( + getExecutor("sendWebhook")!({} as never, ctx, { type: "sendWebhook" }), + ).rejects.toThrow(/url/i); + }); + + it("rejects plain HTTP", async () => { + await expect( + getExecutor("sendWebhook")!({} as never, ctx, cfg("http://example.com/h")), + ).rejects.toThrow(/https/i); + }); + + it("rejects a private host", async () => { + lookup.mockResolvedValue({ address: "10.0.0.5", family: 4 }); + await expect( + getExecutor("sendWebhook")!({} as never, ctx, cfg("https://internal.example/h")), + ).rejects.toThrow(/private|internal/i); + }); + + // The private-IP check ran once, against the ORIGINAL hostname. A public + // host answering 302 -> http://169.254.169.254/ walked straight past it into + // the cloud metadata service. + it("re-validates the target of a redirect", async () => { + lookup.mockImplementation(async (host: string) => + host === "evil.example" ? { address: "1.1.1.1", family: 4 } : { address: "169.254.169.254", family: 4 }, + ); + vi.stubGlobal( + "fetch", + vi.fn().mockResolvedValue({ + status: 302, + ok: false, + headers: { get: (h: string) => (h.toLowerCase() === "location" ? "https://metadata.internal/latest" : null) }, + body: null, + text: async () => "", + }), + ); + + await expect( + getExecutor("sendWebhook")!({} as never, ctx, cfg("https://evil.example/h")), + ).rejects.toThrow(/private|internal|redirect/i); + }); + + it("does not follow redirects endlessly", async () => { + const f = vi.fn().mockResolvedValue({ + status: 302, + ok: false, + headers: { get: (h: string) => (h.toLowerCase() === "location" ? "https://example.com/loop" : null) }, + body: null, + text: async () => "", + }); + vi.stubGlobal("fetch", f); + + await expect( + getExecutor("sendWebhook")!({} as never, ctx, cfg("https://example.com/h")), + ).rejects.toThrow(/redirect/i); + expect(f.mock.calls.length).toBeLessThanOrEqual(4); + }); + + it("rejects a non-2xx response so the failure is logged", async () => { + vi.stubGlobal("fetch", vi.fn().mockResolvedValue(streamingResponse(10, 500))); + + await expect( + getExecutor("sendWebhook")!({} as never, ctx, cfg("https://example.com/h")), + ).rejects.toThrow(/500/); + }); + + it("stops reading a response body that exceeds the cap", async () => { + const reader = { calls: 0 }; + const oversize = streamingResponse(5_000_000); + const originalGetReader = oversize.body!.getReader.bind(oversize.body); + oversize.body!.getReader = () => { + reader.calls++; + return originalGetReader(); + }; + vi.stubGlobal("fetch", vi.fn().mockResolvedValue(oversize)); + + // Must complete rather than buffering 5 MB into memory. + await getExecutor("sendWebhook")!({} as never, ctx, cfg("https://example.com/h")); + expect(reader.calls).toBe(1); + }); + + it("sends successfully on a 2xx", async () => { + const f = vi.fn().mockResolvedValue(streamingResponse(10)); + vi.stubGlobal("fetch", f); + + await getExecutor("sendWebhook")!({} as never, ctx, cfg("https://example.com/h")); + + expect(f).toHaveBeenCalledOnce(); + }); +}); diff --git a/apps/bot/tests/shared/client/ExtendedClient.test.ts b/apps/bot/tests/shared/client/ExtendedClient.test.ts index 0e660021..9b17e097 100644 --- a/apps/bot/tests/shared/client/ExtendedClient.test.ts +++ b/apps/bot/tests/shared/client/ExtendedClient.test.ts @@ -11,7 +11,7 @@ vi.mock("@fluxcore/config", () => ({ })); import { ExtendedClient } from "../../../src/shared/client/ExtendedClient.js"; -import { Collection } from "discord.js"; +import { Collection, Partials } from "discord.js"; describe("ExtendedClient", () => { it("instantiates without errors", () => { @@ -26,4 +26,21 @@ describe("ExtendedClient", () => { expect(client.commands.size).toBe(0); client.destroy(); }); + + // Without partials, discord.js drops any gateway event whose primary + // structure is not already cached. The bot only caches messages seen since + // its last restart, so reactionAdded / reactionRemoved / messageDeleted + // never reached a handler for an older message — which silently breaks + // every reaction-role automation attached to a pinned rules message. + it("enables the partials the reaction and message-delete triggers need", () => { + const client = new ExtendedClient(); + + const partials = client.options.partials ?? []; + expect(partials).toContain(Partials.Message); + expect(partials).toContain(Partials.Reaction); + expect(partials).toContain(Partials.Channel); + expect(partials).toContain(Partials.User); + + client.destroy(); + }); }); \ No newline at end of file diff --git a/apps/dashboard/src/client/features/automation/components/ActionFields.tsx b/apps/dashboard/src/client/features/automation/components/ActionFields.tsx index d9e13088..ee51a686 100644 --- a/apps/dashboard/src/client/features/automation/components/ActionFields.tsx +++ b/apps/dashboard/src/client/features/automation/components/ActionFields.tsx @@ -1,3 +1,4 @@ +import { useState } from "react"; import { useTranslation } from "react-i18next"; import { Icon } from "../../../shared/components/Icon"; import { SearchableSelect } from "../../../shared/ui/searchable-select"; @@ -15,6 +16,8 @@ import { import { VariableEditor } from "../../../shared/ui/variable-field"; import type { VariableDescriptor } from "../../../shared/ui/variable-field"; import type { ActionFieldDescriptor, Channel, Role } from "../../../shared/lib/schemas"; +import { channelIconName, isMessageableChannel } from "../../../shared/lib/channelTypes"; +import { useAutomationLabels } from "../lib/labels"; const VARIABLE_FIELD_KEYS = new Set([ "message", @@ -33,6 +36,14 @@ interface ActionFieldsProps { channels: Channel[]; roles: Role[]; variables: VariableDescriptor[]; + /** The action type these fields belong to, so labels can be translated. */ + actionType?: string; + /** + * Mark empty required fields invalid with a linked message. A red asterisk + * was the only signal, so a screen-reader user got no feedback at all about + * why Save was disabled. + */ + showErrors?: boolean; } function getNestedValue(obj: Record, path: string): unknown { @@ -45,6 +56,86 @@ function getNestedValue(obj: Record, path: string): unknown { return current; } +/** + * A textarea backed by a JSON value. + * + * The raw text lives in local state so a half-typed object does not have to be + * valid to be typeable; only a successful parse is committed upward. The model + * types `webhook.headers` as Record, and writing the raw string + * there made the rule permanently unsavable ("Expected object, received + * string" — English-only, naming no field). + */ +function JsonField({ + id, + value, + onChange, + placeholder, + maxLength, + invalidLabel, +}: { + id: string; + value: unknown; + onChange: (value: unknown) => void; + placeholder?: string; + maxLength?: number; + invalidLabel: string; +}) { + const serialised = + value === undefined || value === null || value === "" + ? "" + : typeof value === "string" + ? value + : JSON.stringify(value, null, 2); + const [text, setText] = useState(serialised); + const [error, setError] = useState(false); + const errorId = `${id}-json-error`; + + const handle = (next: string) => { + setText(next); + if (next.trim() === "") { + setError(false); + onChange(undefined); + return; + } + try { + const parsed: unknown = JSON.parse(next); + const isStringRecord = + typeof parsed === "object" && + parsed !== null && + !Array.isArray(parsed) && + Object.values(parsed as Record).every((v) => typeof v === "string"); + if (!isStringRecord) { + setError(true); + return; + } + setError(false); + onChange(parsed); + } catch { + setError(true); + } + }; + + return ( + <> +