Skip to content

Commit 9452583

Browse files
committed
revert: undo openabdev#398 (empty allowed_channels denies all)
Reverts the empty-list-means-deny-all behavior from openabdev#398. With explicit allow_all_* flags (next commit), the empty-list hack is no longer needed — the flags become the single source of truth.
1 parent 3a0f938 commit 9452583

10 files changed

Lines changed: 12 additions & 69 deletions

File tree

charts/openab/templates/configmap.yaml

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,6 @@ metadata:
1111
data:
1212
config.toml: |
1313
{{- if ($cfg.discord).enabled }}
14-
{{- if not $cfg.discord.allowedChannels }}
15-
{{- fail (printf "agents.%s.discord.allowedChannels is required — empty means the bot will not respond to any messages. Add at least one channel ID." $name) }}
16-
{{- end }}
1714
[discord]
1815
bot_token = "${DISCORD_BOT_TOKEN}"
1916
{{- range $cfg.discord.allowedChannels }}
@@ -55,9 +52,6 @@ data:
5552
{{- end }}
5653
5754
{{- if and ($cfg.slack).enabled }}
58-
{{- if not (($cfg.slack).allowedChannels) }}
59-
{{- fail (printf "agents.%s.slack.allowedChannels is required — empty means the bot will not respond to any messages. Add at least one channel ID." $name) }}
60-
{{- end }}
6155
[slack]
6256
bot_token = "${SLACK_BOT_TOKEN}"
6357
app_token = "${SLACK_APP_TOKEN}"

charts/openab/tests/adapter-enablement_test.yaml

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,6 @@ tests:
3333
- it: renders [slack] when enabled=true
3434
set:
3535
agents.kiro.slack.enabled: true
36-
agents.kiro.slack.allowedChannels:
37-
- "C0123456789"
3836
asserts:
3937
- matchRegex:
4038
path: data["config.toml"]
@@ -51,8 +49,6 @@ tests:
5149
- it: renders [slack] with placeholder tokens when enabled=true
5250
set:
5351
agents.kiro.slack.enabled: true
54-
agents.kiro.slack.allowedChannels:
55-
- "C0123456789"
5652
asserts:
5753
- matchRegex:
5854
path: data["config.toml"]
@@ -65,8 +61,6 @@ tests:
6561
set:
6662
agents.kiro.discord.enabled: true
6763
agents.kiro.slack.enabled: true
68-
agents.kiro.slack.allowedChannels:
69-
- "C0123456789"
7064
asserts:
7165
- matchRegex:
7266
path: data["config.toml"]

charts/openab/tests/configmap_test.yaml

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -92,27 +92,8 @@ tests:
9292
- it: renders slack allow_user_messages = "multibot-mentions"
9393
set:
9494
agents.kiro.slack.enabled: true
95-
agents.kiro.slack.allowedChannels:
96-
- "C0123456789"
9795
agents.kiro.slack.allowUserMessages: multibot-mentions
9896
asserts:
9997
- matchRegex:
10098
path: data["config.toml"]
10199
pattern: 'allow_user_messages = "multibot-mentions"'
102-
103-
- it: rejects empty discord allowedChannels
104-
set:
105-
agents.kiro.discord.enabled: true
106-
agents.kiro.discord.allowedChannels: []
107-
asserts:
108-
- failedTemplate:
109-
errorPattern: "discord.allowedChannels is required"
110-
111-
- it: rejects empty slack allowedChannels
112-
set:
113-
agents.kiro.discord.enabled: false
114-
agents.kiro.slack.enabled: true
115-
agents.kiro.slack.allowedChannels: []
116-
asserts:
117-
- failedTemplate:
118-
errorPattern: "slack.allowedChannels is required"

charts/openab/values.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ agents:
135135
enabled: false
136136
botToken: "" # Bot User OAuth Token (xoxb-...)
137137
appToken: "" # App-Level Token (xapp-...) for Socket Mode
138-
allowedChannels: [] # required — empty = deny all channels (secure by default)
138+
allowedChannels: [] # empty = allow all channels
139139
allowedUsers: [] # empty = allow all users
140140
# allowBotMessages: "off" (default) | "mentions" | "all"
141141
allowBotMessages: "off"

config.toml.example

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
[discord]
44
bot_token = "${DISCORD_BOT_TOKEN}"
5-
allowed_channels = ["1234567890"] # required — empty or omitted = deny all channels (secure by default)
5+
allowed_channels = ["1234567890"] # empty or omitted = allow all channels
66
# allowed_users = ["<YOUR_DISCORD_USER_ID>"] # empty or omitted = allow all users
77
# allow_bot_messages = "off" # "off" (default) | "mentions" | "all"
88
# "mentions" is recommended for multi-agent collaboration
@@ -14,7 +14,7 @@ allowed_channels = ["1234567890"] # required — empty or omitted = deny a
1414
# [slack]
1515
# bot_token = "${SLACK_BOT_TOKEN}" # Bot User OAuth Token (xoxb-...)
1616
# app_token = "${SLACK_APP_TOKEN}" # App-Level Token (xapp-...) for Socket Mode
17-
# allowed_channels = ["C0123456789"] # required — empty or omitted = deny all channels (secure by default)
17+
# allowed_channels = ["C0123456789"] # empty or omitted = allow all channels
1818
# allowed_users = ["U0123456789"] # empty or omitted = allow all users
1919
# allow_bot_messages = "off" # "off" (default) | "mentions" | "all"
2020
# trusted_bot_ids = [] # empty = any bot (mode permitting); set to restrict

docs/discord.md

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ Complete guide to setting up, configuring, and running OpenAB with Discord.
6464
```toml
6565
[discord]
6666
bot_token = "${DISCORD_BOT_TOKEN}"
67-
allowed_channels = ["123456789"] # channel ID allowlist (empty = deny all)
67+
allowed_channels = ["123456789"] # channel ID allowlist (empty = all)
6868
allowed_users = ["987654321"] # user ID allowlist (empty = all)
6969
allow_bot_messages = "off" # off | mentions | all
7070
allow_user_messages = "involved" # involved | mentions
@@ -75,12 +75,11 @@ trusted_bot_ids = [] # bot user IDs allowed through (empty = an
7575

7676
| `allowed_channels` | `allowed_users` | Result |
7777
|---|---|---|
78-
| empty | empty | **No channels, no users — bot ignores all messages** |
78+
| empty | empty | All users, all channels (default) |
7979
| set | empty | Only these channels, all users |
80-
| empty | set | **Bot ignores all messages** (channels must be configured first) |
80+
| empty | set | All channels, only these users |
8181
| set | set | **AND** — must be in allowed channel AND allowed user |
8282

83-
- Empty `allowed_channels` = bot will not respond anywhere (secure by default)
8483
- Empty `allowed_users` (default) = no user filtering
8584
- Denied users get a 🚫 reaction and no reply
8685

docs/slack-bot-howto.md

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -63,22 +63,10 @@ Add the `[slack]` section to your `config.toml`:
6363
[slack]
6464
bot_token = "${SLACK_BOT_TOKEN}"
6565
app_token = "${SLACK_APP_TOKEN}"
66-
allowed_channels = ["C0123456789"] # required — empty = deny all channels (secure by default)
66+
allowed_channels = [] # empty = allow all channels
6767
# allowed_users = ["U0123456789"] # empty = allow all users
6868
```
6969

70-
### Access control behavior
71-
72-
| `allowed_channels` | `allowed_users` | Result |
73-
|---|---|---|
74-
| empty | empty | **No channels, no users — bot ignores all messages** |
75-
| set | empty | Only these channels, all users |
76-
| empty | set | **Bot ignores all messages** (channels must be configured first) |
77-
| set | set | **AND** — must be in allowed channel AND allowed user |
78-
79-
- Empty `allowed_channels` = bot will not respond anywhere (secure by default)
80-
- Empty `allowed_users` (default) = no user filtering
81-
8270
Set the environment variables:
8371

8472
```bash

src/discord.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -234,11 +234,8 @@ impl EventHandler for Handler {
234234
}).clone();
235235

236236
let channel_id = msg.channel_id.get();
237-
if self.allowed_channels.is_empty() {
238-
debug!("allowed_channels is empty — ignoring message in channel {}", channel_id);
239-
return;
240-
}
241-
let in_allowed_channel = self.allowed_channels.contains(&channel_id);
237+
let in_allowed_channel =
238+
self.allowed_channels.is_empty() || self.allowed_channels.contains(&channel_id);
242239

243240
let is_mentioned = msg.mentions_user_id(bot_id)
244241
|| msg.content.contains(&format!("<@{}>", bot_id));

src/main.rs

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use serenity::prelude::*;
1616
use std::collections::HashSet;
1717
use std::path::PathBuf;
1818
use std::sync::Arc;
19-
use tracing::{error, info, warn};
19+
use tracing::{error, info};
2020

2121
#[derive(Parser)]
2222
#[command(name = "openab")]
@@ -111,9 +111,6 @@ async fn main() -> anyhow::Result<()> {
111111

112112
// Spawn Slack adapter (background task)
113113
let slack_handle = if let Some(slack_cfg) = cfg.slack {
114-
if slack_cfg.allowed_channels.is_empty() {
115-
warn!("no allowed_channels configured for Slack — bot will not respond to any messages. Add at least one channel ID to [slack] allowed_channels.");
116-
}
117114
info!(
118115
channels = slack_cfg.allowed_channels.len(),
119116
users = slack_cfg.allowed_users.len(),
@@ -151,9 +148,6 @@ async fn main() -> anyhow::Result<()> {
151148
if let Some(discord_cfg) = cfg.discord {
152149
let allowed_channels =
153150
parse_id_set(&discord_cfg.allowed_channels, "discord.allowed_channels")?;
154-
if allowed_channels.is_empty() {
155-
warn!("no allowed_channels configured for Discord — bot will not respond to any messages. Add at least one channel ID to [discord] allowed_channels.");
156-
}
157151
let allowed_users = parse_id_set(&discord_cfg.allowed_users, "discord.allowed_users")?;
158152
let trusted_bot_ids = parse_id_set(&discord_cfg.trusted_bot_ids, "discord.trusted_bot_ids")?;
159153
info!(

src/slack.rs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -803,12 +803,8 @@ async fn handle_message(
803803
};
804804
let thread_ts = event["thread_ts"].as_str().map(|s| s.to_string());
805805

806-
// Check allowed channels (empty = deny all)
807-
if allowed_channels.is_empty() {
808-
tracing::debug!("allowed_channels is empty — ignoring message in channel {}", channel_id);
809-
return;
810-
}
811-
if !allowed_channels.contains(&channel_id) {
806+
// Check allowed channels (empty = allow all)
807+
if !allowed_channels.is_empty() && !allowed_channels.contains(&channel_id) {
812808
return;
813809
}
814810

0 commit comments

Comments
 (0)