fix: groupPolicy: "open" ignored when channel-specific config exists
## 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
This commit is contained in:
committed by
Peter Steinberger
parent
ae48066d28
commit
72d62a54c6
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user