audit follow-up: burn down lint hotspots and dedupe channel gating flows
This commit is contained in:
@@ -17,7 +17,14 @@ import type {
|
||||
ChannelAdapter,
|
||||
ChannelStatus,
|
||||
} from '../types.js';
|
||||
import { buildResetInboundMessage, normalizeResetCommandText, splitMessage } from '../utils.js';
|
||||
import {
|
||||
allowTrustedOrPairedSender,
|
||||
buildResetInboundMessage,
|
||||
isAllowedByAllowlist,
|
||||
normalizeResetCommandText,
|
||||
shouldIgnoreForMissingMention,
|
||||
splitMessage,
|
||||
} from '../utils.js';
|
||||
import type { PairingManager } from '../pairing.js';
|
||||
|
||||
/** Configuration for the Discord channel adapter. */
|
||||
@@ -98,7 +105,7 @@ export class DiscordAdapter implements ChannelAdapter {
|
||||
|
||||
// ── Message handler — route inbound messages ──
|
||||
this.client.on(Events.MessageCreate, (message: DiscordMessage) => {
|
||||
this.handleMessage(message);
|
||||
void this.handleMessage(message);
|
||||
});
|
||||
|
||||
// Log in and wait for the ready event
|
||||
@@ -162,7 +169,7 @@ export class DiscordAdapter implements ChannelAdapter {
|
||||
}
|
||||
|
||||
/** Internal: process an inbound Discord message. */
|
||||
private handleMessage(message: DiscordMessage): void {
|
||||
private async handleMessage(message: DiscordMessage): Promise<void> {
|
||||
if (!this.messageHandler) {return;}
|
||||
|
||||
// Ignore bot messages
|
||||
@@ -174,42 +181,42 @@ export class DiscordAdapter implements ChannelAdapter {
|
||||
if (!isDM) {
|
||||
// Check allowed guild IDs
|
||||
if (
|
||||
this.config.allowedGuildIds &&
|
||||
this.config.allowedGuildIds.length > 0 &&
|
||||
!this.config.allowedGuildIds.includes(message.guild!.id)
|
||||
!isAllowedByAllowlist(message.guild!.id, this.config.allowedGuildIds)
|
||||
) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Check allowed channel IDs
|
||||
if (
|
||||
this.config.allowedChannelIds &&
|
||||
this.config.allowedChannelIds.length > 0 &&
|
||||
!this.config.allowedChannelIds.includes(message.channelId)
|
||||
) {
|
||||
if (!isAllowedByAllowlist(message.channelId, this.config.allowedChannelIds)) {
|
||||
return;
|
||||
}
|
||||
|
||||
// ── Mention requirement in guild channels ──
|
||||
const requireMention = this.config.requireMention ?? true;
|
||||
if (requireMention && this.client?.user) {
|
||||
if (!message.mentions.has(this.client.user)) {
|
||||
return;
|
||||
}
|
||||
if (this.client?.user && shouldIgnoreForMissingMention({
|
||||
requireMention: this.config.requireMention,
|
||||
defaultRequireMention: true,
|
||||
mentionsBot: message.mentions.has(this.client.user),
|
||||
})) {
|
||||
return;
|
||||
}
|
||||
} else {
|
||||
// DM pairing check — if pairing is enabled, require approval
|
||||
const pm = this.config.pairingManager;
|
||||
if (pm?.enabled && !pm.isApproved('discord', message.channelId)) {
|
||||
const text = message.content.trim();
|
||||
if (text && pm.validateCode('discord', message.channelId, text)) {
|
||||
try {
|
||||
if (this.config.pairingManager?.enabled) {
|
||||
if (!await allowTrustedOrPairedSender({
|
||||
pairingManager: this.config.pairingManager,
|
||||
channel: 'discord',
|
||||
senderId: message.channelId,
|
||||
text: message.content ?? '',
|
||||
isTrusted: false,
|
||||
onPaired: async () => {
|
||||
if ('send' in message.channel) {
|
||||
(message.channel as any).send('Pairing successful! You can now chat with Flynn.');
|
||||
await (message.channel as { send: (content: string) => Promise<unknown> })
|
||||
.send('Pairing successful! You can now chat with Flynn.');
|
||||
}
|
||||
} catch { /* ignore send errors */ }
|
||||
},
|
||||
})) {
|
||||
return;
|
||||
}
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -15,7 +15,14 @@ import type {
|
||||
ChannelAdapter,
|
||||
ChannelStatus,
|
||||
} from '../types.js';
|
||||
import { buildResetInboundMessage, normalizeResetCommandText, splitMessage } from '../utils.js';
|
||||
import {
|
||||
allowTrustedOrPairedSender,
|
||||
buildResetInboundMessage,
|
||||
isAllowedByAllowlist,
|
||||
normalizeResetCommandText,
|
||||
shouldIgnoreForMissingMention,
|
||||
splitMessage,
|
||||
} from '../utils.js';
|
||||
import type { PairingManager } from '../pairing.js';
|
||||
|
||||
/** Configuration for the Slack channel adapter. */
|
||||
@@ -295,46 +302,37 @@ export class SlackAdapter implements ChannelAdapter {
|
||||
if (!channelId) {return;}
|
||||
|
||||
// Check allowed channel IDs
|
||||
if (
|
||||
this.config.allowedChannelIds &&
|
||||
this.config.allowedChannelIds.length > 0 &&
|
||||
!this.config.allowedChannelIds.includes(channelId)
|
||||
) {
|
||||
// Pairing fallback — check if the Slack user is approved or sending a valid code
|
||||
const pm = this.config.pairingManager;
|
||||
const userId = message.user;
|
||||
if (pm?.enabled && userId) {
|
||||
if (pm.isApproved('slack', userId)) {
|
||||
// Approved — fall through to normal message handling
|
||||
} else {
|
||||
const text = (message.text ?? '').trim();
|
||||
if (text && pm.validateCode('slack', userId, text)) {
|
||||
// Code validated — send confirmation via Slack
|
||||
if (this.app) {
|
||||
const threadTs = message.thread_ts ?? message.ts ?? '';
|
||||
try {
|
||||
await this.app.client.chat.postMessage({
|
||||
channel: channelId,
|
||||
text: 'Pairing successful! You can now chat with Flynn.',
|
||||
thread_ts: threadTs || undefined,
|
||||
});
|
||||
} catch { /* ignore send errors */ }
|
||||
}
|
||||
}
|
||||
return;
|
||||
}
|
||||
} else {
|
||||
if (!isAllowedByAllowlist(channelId, this.config.allowedChannelIds)) {
|
||||
const senderId = message.user ?? '';
|
||||
const allowed = await allowTrustedOrPairedSender({
|
||||
pairingManager: this.config.pairingManager,
|
||||
channel: 'slack',
|
||||
senderId,
|
||||
text: message.text ?? '',
|
||||
isTrusted: false,
|
||||
onPaired: async () => {
|
||||
if (!this.app) {return;}
|
||||
const threadTs = message.thread_ts ?? message.ts ?? '';
|
||||
await this.app.client.chat.postMessage({
|
||||
channel: channelId,
|
||||
text: 'Pairing successful! You can now chat with Flynn.',
|
||||
thread_ts: threadTs || undefined,
|
||||
});
|
||||
},
|
||||
});
|
||||
if (!allowed) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
// Mention requirement
|
||||
const requireMention = this.config.requireMention ?? false;
|
||||
if (requireMention && this.botUserId) {
|
||||
const mentionPattern = `<@${this.botUserId}>`;
|
||||
if (!(message.text ?? '').includes(mentionPattern)) {
|
||||
return;
|
||||
}
|
||||
const mentionPattern = this.botUserId ? `<@${this.botUserId}>` : undefined;
|
||||
if (shouldIgnoreForMissingMention({
|
||||
requireMention: mentionPattern ? this.config.requireMention : false,
|
||||
defaultRequireMention: false,
|
||||
mentionsBot: mentionPattern ? (message.text ?? '').includes(mentionPattern) : false,
|
||||
})) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Note: Slack doesn't expose a typing indicator API for bots
|
||||
|
||||
@@ -1,5 +1,13 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { buildResetInboundMessage, normalizeResetCommandText, splitMessage } from './utils.js';
|
||||
import {
|
||||
allowTrustedOrPairedSender,
|
||||
buildResetInboundMessage,
|
||||
isAllowedByAllowlist,
|
||||
normalizeResetCommandText,
|
||||
shouldIgnoreForMissingMention,
|
||||
splitMessage,
|
||||
} from './utils.js';
|
||||
import { PairingManager } from './pairing.js';
|
||||
|
||||
describe('splitMessage', () => {
|
||||
it('returns single chunk for empty string', () => {
|
||||
@@ -133,3 +141,84 @@ describe('buildResetInboundMessage', () => {
|
||||
expect(message.metadata).toEqual({ isCommand: true, command: 'reset' });
|
||||
});
|
||||
});
|
||||
|
||||
describe('isAllowedByAllowlist', () => {
|
||||
it('allows all values when allowlist is missing', () => {
|
||||
expect(isAllowedByAllowlist('C123')).toBe(true);
|
||||
});
|
||||
|
||||
it('allows all values when allowlist is empty', () => {
|
||||
expect(isAllowedByAllowlist('C123', [])).toBe(true);
|
||||
});
|
||||
|
||||
it('filters values when allowlist is present', () => {
|
||||
expect(isAllowedByAllowlist('C123', ['C123', 'C456'])).toBe(true);
|
||||
expect(isAllowedByAllowlist('C999', ['C123', 'C456'])).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('shouldIgnoreForMissingMention', () => {
|
||||
it('ignores when mention is required and missing', () => {
|
||||
expect(shouldIgnoreForMissingMention({
|
||||
requireMention: true,
|
||||
defaultRequireMention: false,
|
||||
mentionsBot: false,
|
||||
})).toBe(true);
|
||||
});
|
||||
|
||||
it('does not ignore when mention is required and present', () => {
|
||||
expect(shouldIgnoreForMissingMention({
|
||||
requireMention: true,
|
||||
defaultRequireMention: false,
|
||||
mentionsBot: true,
|
||||
})).toBe(false);
|
||||
});
|
||||
|
||||
it('uses default when requireMention is undefined', () => {
|
||||
expect(shouldIgnoreForMissingMention({
|
||||
requireMention: undefined,
|
||||
defaultRequireMention: true,
|
||||
mentionsBot: false,
|
||||
})).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('allowTrustedOrPairedSender', () => {
|
||||
it('allows trusted senders immediately', async () => {
|
||||
const pm = new PairingManager({ enabled: true, codeTtl: 60_000, codeLength: 6 });
|
||||
await expect(allowTrustedOrPairedSender({
|
||||
pairingManager: pm,
|
||||
channel: 'slack',
|
||||
senderId: 'U1',
|
||||
text: 'hello',
|
||||
isTrusted: true,
|
||||
})).resolves.toBe(true);
|
||||
});
|
||||
|
||||
it('blocks untrusted senders when pairing is disabled', async () => {
|
||||
const pm = new PairingManager({ enabled: false, codeTtl: 60_000, codeLength: 6 });
|
||||
await expect(allowTrustedOrPairedSender({
|
||||
pairingManager: pm,
|
||||
channel: 'slack',
|
||||
senderId: 'U1',
|
||||
text: 'hello',
|
||||
isTrusted: false,
|
||||
})).resolves.toBe(false);
|
||||
});
|
||||
|
||||
it('consumes valid pairing code and runs onPaired callback', async () => {
|
||||
const pm = new PairingManager({ enabled: true, codeTtl: 60_000, codeLength: 6 });
|
||||
const code = pm.generateCode('test');
|
||||
let paired = false;
|
||||
await expect(allowTrustedOrPairedSender({
|
||||
pairingManager: pm,
|
||||
channel: 'slack',
|
||||
senderId: 'U1',
|
||||
text: code,
|
||||
isTrusted: false,
|
||||
onPaired: () => { paired = true; },
|
||||
})).resolves.toBe(false);
|
||||
expect(paired).toBe(true);
|
||||
expect(pm.isApproved('slack', 'U1')).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
* Shared utilities for channel adapters.
|
||||
*/
|
||||
import type { Attachment, InboundMessage } from './types.js';
|
||||
import type { PairingManager } from './pairing.js';
|
||||
|
||||
/**
|
||||
* Split a long message into chunks that respect a platform's character limit.
|
||||
@@ -44,6 +45,61 @@ export function normalizeResetCommandText(text: string): string {
|
||||
return text;
|
||||
}
|
||||
|
||||
/** Check whether a value is allowed by an optional allowlist (empty/missing list means allow-all). */
|
||||
export function isAllowedByAllowlist(value: string, allowlist?: string[]): boolean {
|
||||
if (!allowlist || allowlist.length === 0) {
|
||||
return true;
|
||||
}
|
||||
return allowlist.includes(value);
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns true when mention gating should ignore the message.
|
||||
* `requireMention` defaults per adapter; if enabled, the message must mention the bot.
|
||||
*/
|
||||
export function shouldIgnoreForMissingMention(params: {
|
||||
requireMention: boolean | undefined;
|
||||
defaultRequireMention: boolean;
|
||||
mentionsBot: boolean;
|
||||
}): boolean {
|
||||
const needsMention = params.requireMention ?? params.defaultRequireMention;
|
||||
return needsMention && !params.mentionsBot;
|
||||
}
|
||||
|
||||
interface PairingGateParams {
|
||||
pairingManager?: PairingManager;
|
||||
channel: InboundMessage['channel'];
|
||||
senderId: string;
|
||||
text: string;
|
||||
/** True for trusted senders/channels that may bypass pairing. */
|
||||
isTrusted: boolean;
|
||||
/** Optional side effect when a pairing code is accepted. */
|
||||
onPaired?: () => Promise<void> | void;
|
||||
}
|
||||
|
||||
/**
|
||||
* Shared pairing gate for channel adapters.
|
||||
* Returns true when the inbound message should continue to normal processing.
|
||||
*/
|
||||
export async function allowTrustedOrPairedSender(params: PairingGateParams): Promise<boolean> {
|
||||
const pm = params.pairingManager;
|
||||
if (params.isTrusted) {
|
||||
return true;
|
||||
}
|
||||
if (!pm?.enabled) {
|
||||
return false;
|
||||
}
|
||||
if (pm.isApproved(params.channel, params.senderId)) {
|
||||
return true;
|
||||
}
|
||||
const code = params.text.trim();
|
||||
if (!code || !pm.validateCode(params.channel, params.senderId, code)) {
|
||||
return false;
|
||||
}
|
||||
await params.onPaired?.();
|
||||
return false;
|
||||
}
|
||||
|
||||
interface ResetMessageParams {
|
||||
id: string;
|
||||
channel: InboundMessage['channel'];
|
||||
|
||||
@@ -17,7 +17,13 @@ import type {
|
||||
ChannelAdapter,
|
||||
ChannelStatus,
|
||||
} from '../types.js';
|
||||
import { buildResetInboundMessage, normalizeResetCommandText, splitMessage } from '../utils.js';
|
||||
import {
|
||||
allowTrustedOrPairedSender,
|
||||
buildResetInboundMessage,
|
||||
normalizeResetCommandText,
|
||||
shouldIgnoreForMissingMention,
|
||||
splitMessage,
|
||||
} from '../utils.js';
|
||||
import type { PairingManager } from '../pairing.js';
|
||||
|
||||
/** Configuration for the WhatsApp channel adapter. */
|
||||
@@ -224,15 +230,15 @@ export class WhatsAppAdapter implements ChannelAdapter {
|
||||
}
|
||||
|
||||
// Mention requirement in group chats
|
||||
const requireMention = this.config.requireMention ?? true;
|
||||
if (requireMention) {
|
||||
if (this.botId && shouldIgnoreForMissingMention({
|
||||
requireMention: this.config.requireMention,
|
||||
defaultRequireMention: true,
|
||||
mentionsBot: message.body?.includes(`@${this.botId.replace(/@c\.us$/, '')}`) ||
|
||||
(message as unknown as { mentionedIds?: string[] }).mentionedIds?.some((id) => id === this.botId) === true,
|
||||
})) {
|
||||
// WhatsApp mentions use @phone_number format in body
|
||||
// Also check for mentions in the message mentionedIds
|
||||
const mentionsBot = this.botId
|
||||
? message.body?.includes(`@${this.botId.replace(/@c\.us$/, '')}`) ||
|
||||
(message as any).mentionedIds?.some((id: string) => id === this.botId)
|
||||
: false;
|
||||
if (!mentionsBot) {return;}
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -244,24 +250,18 @@ export class WhatsAppAdapter implements ChannelAdapter {
|
||||
this.config.allowedNumbers.length > 0 &&
|
||||
!this.config.allowedNumbers.includes(phoneNumber)
|
||||
) {
|
||||
// Pairing fallback — check if the sender is approved or sending a valid code
|
||||
const pm = this.config.pairingManager;
|
||||
if (pm?.enabled) {
|
||||
if (pm.isApproved('whatsapp', phoneNumber)) {
|
||||
// Approved — fall through to normal message handling
|
||||
} else {
|
||||
const text = (message.body ?? '').trim();
|
||||
if (text && pm.validateCode('whatsapp', phoneNumber, text)) {
|
||||
// Code validated — send confirmation via WhatsApp
|
||||
if (this.client) {
|
||||
try {
|
||||
await this.client.sendMessage(from, 'Pairing successful! You can now chat with Flynn.');
|
||||
} catch { /* ignore send errors */ }
|
||||
}
|
||||
}
|
||||
return;
|
||||
}
|
||||
} else {
|
||||
const allowed = await allowTrustedOrPairedSender({
|
||||
pairingManager: this.config.pairingManager,
|
||||
channel: 'whatsapp',
|
||||
senderId: phoneNumber,
|
||||
text: message.body ?? '',
|
||||
isTrusted: false,
|
||||
onPaired: async () => {
|
||||
if (!this.client) {return;}
|
||||
await this.client.sendMessage(from, 'Pairing successful! You can now chat with Flynn.');
|
||||
},
|
||||
});
|
||||
if (!allowed) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user