From 4ffc995f12b32178cea8e23c8430004d3fc3ab59 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Fri, 10 Jul 2026 19:08:44 +0000 Subject: [PATCH] feat(gateway): enforce allowed_channel_ids on outbound emit (with DM exemption) Co-Authored-By: Claude Fable 5 --- apps/agor-daemon/src/services/gateway.test.ts | 105 ++++++++++++++++++ apps/agor-daemon/src/services/gateway.ts | 11 ++ 2 files changed, 116 insertions(+) diff --git a/apps/agor-daemon/src/services/gateway.test.ts b/apps/agor-daemon/src/services/gateway.test.ts index 8fb9a17fc..4167cd6b1 100644 --- a/apps/agor-daemon/src/services/gateway.test.ts +++ b/apps/agor-daemon/src/services/gateway.test.ts @@ -825,6 +825,111 @@ describe('GatewayService outbound emit session branch binding', () => { }); }); +describe('GatewayService outbound emit allowed_channel_ids enforcement', () => { + function makeAllowlistHarness( + args: { config?: Record; connectorExtras?: Record } = {} + ) { + const channel = { + ...slackChannel, + id: 'chan-outbound', + config: { + bot_token: 'xoxb-test', + outbound_enabled: true, + ...args.config, + }, + } as unknown as GatewayChannel; + const { service } = makeGatewayHarness({ channel }); + const sendSlackMessage = vi.fn(async (req: { channel: string }) => ({ + ts: '200.000100', + channel: req.channel, + thread_ts: '200.000100', + permalink: null, + })); + vi.mocked(getConnector).mockReturnValue({ + sendSlackMessage, + ...args.connectorExtras, + } as never); + const outboundRepo = { + create: vi.fn(async (data: Record) => ({ id: 'out-1', ...data })), + findUnconsumedByChannelAndThread: vi.fn(async () => null), + }; + (service as unknown as { outboundRepo: unknown }).outboundRepo = outboundRepo; + return { service, sendSlackMessage, outboundRepo }; + } + + type EmitData = Parameters[0]; + + function emitData(overrides: Partial = {}): EmitData { + return { + gatewayChannelId: 'chan-outbound', + message: 'ship update', + emittedByUserId: 'user-1' as UserID, + userRole: 'admin', + ...overrides, + }; + } + + it('denies a channel-id target outside the allowlist before any Slack send', async () => { + const { service, sendSlackMessage, outboundRepo } = makeAllowlistHarness({ + config: { allowed_channel_ids: ['C123'] }, + }); + + await expect(service.emitMessage(emitData({ target: 'channel:C999' }))).rejects.toThrow( + /allowed_channel_ids/ + ); + expect(sendSlackMessage).not.toHaveBeenCalled(); + expect(outboundRepo.create).not.toHaveBeenCalled(); + }); + + it('denies a channel-name target that resolves to an id outside the allowlist', async () => { + const resolveChannelByName = vi.fn(async () => ({ channel: 'C999', name: 'general' })); + const { service, sendSlackMessage } = makeAllowlistHarness({ + config: { allowed_channel_ids: ['C123'] }, + connectorExtras: { resolveChannelByName }, + }); + + await expect(service.emitMessage(emitData({ target: '#general' }))).rejects.toThrow( + /allowed_channel_ids/ + ); + expect(resolveChannelByName).toHaveBeenCalledWith('general'); + expect(sendSlackMessage).not.toHaveBeenCalled(); + }); + + it('allows an email target resolved to a DM even with an allowlist configured', async () => { + const openDmByEmail = vi.fn(async () => ({ channel: 'D777', user_id: 'U42' })); + const { service, sendSlackMessage } = makeAllowlistHarness({ + config: { allowed_channel_ids: ['C123'] }, + connectorExtras: { openDmByEmail }, + }); + + const result = await service.emitMessage(emitData({ target: 'user@example.com' })); + + expect(result).toMatchObject({ success: true, platform_channel_id: 'D777' }); + expect(openDmByEmail).toHaveBeenCalledWith('user@example.com'); + expect(sendSlackMessage).toHaveBeenCalledWith(expect.objectContaining({ channel: 'D777' })); + }); + + it('allows an allowlisted channel target', async () => { + const { service, sendSlackMessage } = makeAllowlistHarness({ + config: { allowed_channel_ids: ['C123'] }, + }); + + const result = await service.emitMessage(emitData({ target: 'channel:C123' })); + + expect(result).toMatchObject({ success: true, platform_channel_id: 'C123' }); + expect(sendSlackMessage).toHaveBeenCalledWith(expect.objectContaining({ channel: 'C123' })); + }); + + it('allows any channel target when no allowlist is configured', async () => { + const { service, sendSlackMessage } = makeAllowlistHarness(); + + const result = await service.emitMessage(emitData({ target: 'channel:C999' })); + + expect(result).toMatchObject({ success: true, platform_channel_id: 'C999' }); + expect(sendSlackMessage).toHaveBeenCalledWith(expect.objectContaining({ channel: 'C999' })); + }); +}); + describe('GatewayService Slack attachment ingestion', () => { const ingestChannel = { ...slackChannel, diff --git a/apps/agor-daemon/src/services/gateway.ts b/apps/agor-daemon/src/services/gateway.ts index 4c4bdbafe..42d11386e 100644 --- a/apps/agor-daemon/src/services/gateway.ts +++ b/apps/agor-daemon/src/services/gateway.ts @@ -39,6 +39,7 @@ import { formatGatewaySystemPayload, getConnector, hasConnector, + isSlackWriteTargetAllowed, normalizeOutbound, parseGitHubThreadId, } from '@agor/core/gateway'; @@ -1473,6 +1474,16 @@ export class GatewayService { resolvedTargetMetadata.resolved_user_id = resolved.user_id; } + // The allowed_channel_ids whitelist works on concrete conversation ids, + // while `target` may be a channel name or user email — so enforcement + // happens only after resolution. isSlackWriteTargetAllowed exempts DMs, + // so email→DM and D-prefixed targets always pass. + if (!isSlackWriteTargetAllowed(config, resolvedChannel)) { + throw new Error( + `Gateway outbound denied: target ${target} resolves to Slack conversation ${resolvedChannel}, which is not in this gateway channel's allowed_channel_ids whitelist.` + ); + } + let sent: Awaited>; try { sent = await connector.sendSlackMessage({