From 72d62a54c613ad45d04bf054f878fe12fb748215 Mon Sep 17 00:00:00 2001 From: Jay Winder Date: Sat, 24 Jan 2026 14:42:54 +0900 Subject: [PATCH] fix: groupPolicy: "open" ignored when channel-specific config exists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Fix Slack `groupPolicy: "open"` to allow unlisted channels even when `channels.slack.channels` contains custom entries. ## Problem When `groupPolicy` is set to `"open"`, the bot should respond in **any channel** it's invited to. However, if `channels.slack.channels` contains *any* entries—even just one channel with a custom system prompt—the open policy is ignored. Only explicitly listed channels receive responses; all others get an ephemeral "This channel is not allowed" error. ### Example config ```json { "channels": { "slack": { "groupPolicy": "open", "channels": { "C0123456789": { "systemPrompt": "Custom prompt for this channel" } } } } } ``` With this config, the bot only responds in `C0123456789`. Messages in any other channel are blocked—even though the policy is `"open"`. ## Root Cause In `src/slack/monitor/context.ts`, `isChannelAllowed()` has two sequential checks: 1. `isSlackChannelAllowedByPolicy()` — correctly returns `true` for open policy 2. A secondary `!channelAllowed` check — was blocking channels when `resolveSlackChannelConfig()` returned `{ allowed: false }` for unlisted channels The second check conflated "channel not in config" with "channel explicitly denied." ## Fix Use `matchSource` to distinguish explicit denial from absence of config: ```ts const hasExplicitConfig = Boolean(channelConfig?.matchSource); if (!channelAllowed && (params.groupPolicy !== "open" || hasExplicitConfig)) { return false; } ``` When `matchSource` is undefined, the channel has no explicit config entry and should be allowed under open policy. ## Behavior After Fix | Scenario | Result | |----------|--------| | `groupPolicy: "open"`, channel unlisted | ✅ Allowed | | `groupPolicy: "open"`, channel explicitly denied (`allow: false`) | ❌ Blocked | | `groupPolicy: "open"`, channel with custom config | ✅ Allowed | | `groupPolicy: "allowlist"`, channel unlisted | ❌ Blocked | ## Test Plan - [x] Open policy + unlisted channel → allowed - [x] Open policy + explicitly denied channel → blocked - [x] Allowlist policy + unlisted channel → blocked - [x] Allowlist policy + listed channel → allowed --- src/slack/monitor/context.test.ts | 58 +++++++++++++++++++++++++++++++ src/slack/monitor/context.ts | 6 +++- 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/src/slack/monitor/context.test.ts b/src/slack/monitor/context.test.ts index c52c5e919..d63b1c71a 100644 --- a/src/slack/monitor/context.test.ts +++ b/src/slack/monitor/context.test.ts @@ -60,3 +60,61 @@ describe("resolveSlackSystemEventSessionKey", () => { ); }); }); + +describe("isChannelAllowed with groupPolicy and channelsConfig", () => { + it("allows unlisted channels when groupPolicy is open even with channelsConfig entries", () => { + // Bug fix: when groupPolicy="open" and channels has some entries, + // unlisted channels should still be allowed (not blocked) + const ctx = createSlackMonitorContext({ + ...baseParams(), + groupPolicy: "open", + channelsConfig: { + C_LISTED: { requireMention: true }, + }, + }); + // Listed channel should be allowed + expect(ctx.isChannelAllowed({ channelId: "C_LISTED", channelType: "channel" })).toBe(true); + // Unlisted channel should ALSO be allowed when policy is "open" + expect(ctx.isChannelAllowed({ channelId: "C_UNLISTED", channelType: "channel" })).toBe(true); + }); + + it("blocks unlisted channels when groupPolicy is allowlist", () => { + const ctx = createSlackMonitorContext({ + ...baseParams(), + groupPolicy: "allowlist", + channelsConfig: { + C_LISTED: { requireMention: true }, + }, + }); + // Listed channel should be allowed + expect(ctx.isChannelAllowed({ channelId: "C_LISTED", channelType: "channel" })).toBe(true); + // Unlisted channel should be blocked when policy is "allowlist" + expect(ctx.isChannelAllowed({ channelId: "C_UNLISTED", channelType: "channel" })).toBe(false); + }); + + it("blocks explicitly denied channels even when groupPolicy is open", () => { + const ctx = createSlackMonitorContext({ + ...baseParams(), + groupPolicy: "open", + channelsConfig: { + C_ALLOWED: { allow: true }, + C_DENIED: { allow: false }, + }, + }); + // Explicitly allowed channel + expect(ctx.isChannelAllowed({ channelId: "C_ALLOWED", channelType: "channel" })).toBe(true); + // Explicitly denied channel should be blocked even with open policy + expect(ctx.isChannelAllowed({ channelId: "C_DENIED", channelType: "channel" })).toBe(false); + // Unlisted channel should be allowed with open policy + expect(ctx.isChannelAllowed({ channelId: "C_UNLISTED", channelType: "channel" })).toBe(true); + }); + + it("allows all channels when groupPolicy is open and channelsConfig is empty", () => { + const ctx = createSlackMonitorContext({ + ...baseParams(), + groupPolicy: "open", + channelsConfig: undefined, + }); + expect(ctx.isChannelAllowed({ channelId: "C_ANY", channelType: "channel" })).toBe(true); + }); +}); diff --git a/src/slack/monitor/context.ts b/src/slack/monitor/context.ts index bd2425103..83acfbae6 100644 --- a/src/slack/monitor/context.ts +++ b/src/slack/monitor/context.ts @@ -327,7 +327,11 @@ export function createSlackMonitorContext(params: { ); return false; } - if (!channelAllowed) { + // When groupPolicy is "open", only block channels that are EXPLICITLY denied + // (i.e., have a matching config entry with allow:false). Channels not in the + // config (matchSource undefined) should be allowed under open policy. + const hasExplicitConfig = Boolean(channelConfig?.matchSource); + if (!channelAllowed && (params.groupPolicy !== "open" || hasExplicitConfig)) { logVerbose(`slack: drop channel ${p.channelId} (${channelMatchMeta})`); return false; }