fix: empty allowed_channels denies all channels (secure by default)#92
fix: empty allowed_channels denies all channels (secure by default)#92chaodu-agent wants to merge 1 commit intoopenabdev:mainfrom
Conversation
Previously, an empty allowed_channels list would allow the bot to respond in ALL channels. This changes the behavior so that an empty list means the bot will not respond to any messages. Also adds a startup warning log when allowed_channels is empty and a clarifying comment in config.toml.example. Closes openabdev#91
masami-agent
left a comment
There was a problem hiding this comment.
Clean change, well-scoped 👍 Two things to address:
1. Needs rebase on main
PR #108 (allowed_users) has since merged and modified the same area of discord.rs. This PR will likely have conflicts. The allowed_users check also needs the same treatment — after rebase, please verify the user check logic still works correctly alongside this change.
2. Behavioral consistency with allowed_users
After this PR, the semantics would be:
allowed_channels = []→ deny all (this PR)allowed_users = []→ allow all (#108)
This asymmetry could confuse users. Worth discussing: should allowed_users = [] also mean deny all? Or is the current design intentional (channels = required, users = optional filter)?
If intentional, please add a note in the PR description and config.toml.example clarifying the difference. If not, consider aligning the behavior.
Otherwise LGTM — the secure-by-default approach for channels makes sense.
Summary
Implements Option A from #91: empty
allowed_channelsnow means the bot will not respond to any messages (secure by default).Changes
src/discord.rsself.allowed_channels.is_empty() ||from the channel check — empty set now denies allready()whenallowed_channelsis empty:config.toml.example# Required: at least one channel ID. Empty list = bot will not respond to any messages.Before / After
allowed_channels = []allowed_channels = ["123"]Breaking Change
Users who intentionally left
allowed_channelsempty to allow all channels will need to explicitly list their channel IDs. This is a one-line config change.Closes #91