From d0ee4de3a7892d71520dc96ab7cd53b62570485b Mon Sep 17 00:00:00 2001 From: "S.L" <101876007+slvnlrt@users.noreply.github.com> Date: Fri, 26 Jun 2026 21:01:49 +0200 Subject: [PATCH] feat(messaging): honor "*" allow-all wildcard in DM/user allowlists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Slack, Mattermost, and Twitch allowlists matched user IDs by exact value only, so there was no way to allow every DM/user without enumerating each one — and a list of ["*"] silently matched nobody. The Signal adapter already treats "*" as an explicit allow-all wildcard; this aligns the other string-keyed adapters with that convention. - SlackPermissions::dm_user_allowed / MattermostPermissions::dm_user_allowed (fail-closed: an empty list still blocks all DMs; "*" allows all) - TwitchPermissions::user_allowed (fail-open: an empty list still allows all, preserving existing behavior; "*" allows all; matching is case-insensitive) - the adapters call the new helpers; behavior is unchanged except that "*" now means allow-all instead of matching a literal "*" id Discord and Telegram parse their allowlists into integer id vectors (Vec / Vec), so "*" is not representable there; an allow-all for those would need a separate flag and is out of scope here. Adds unit tests covering each adapter's empty / wildcard / exact-match cases. --- src/config/permissions.rs | 126 ++++++++++++++++++++++++++++++++++++ src/messaging/mattermost.rs | 11 ++-- src/messaging/slack.rs | 2 +- src/messaging/twitch.rs | 4 +- 4 files changed, 132 insertions(+), 11 deletions(-) diff --git a/src/config/permissions.rs b/src/config/permissions.rs index 95323e8c0..e7c9bbce7 100644 --- a/src/config/permissions.rs +++ b/src/config/permissions.rs @@ -191,6 +191,17 @@ impl SlackPermissions { dm_allowed_users, } } + + /// Whether `user_id` may DM the bot. Fail-closed: an empty allowlist + /// blocks all DMs. A `"*"` entry is an explicit allow-all wildcard + /// (consistent with the Signal adapter's convention). + pub fn dm_user_allowed(&self, user_id: &str) -> bool { + !self.dm_allowed_users.is_empty() + && self + .dm_allowed_users + .iter() + .any(|u| u == "*" || u == user_id) + } } /// Hot-reloadable Telegram permission filters. @@ -323,6 +334,18 @@ impl TwitchPermissions { allowed_users, } } + + /// Whether `login` may interact with the bot. Fail-open: an empty + /// allowlist accepts all users (Twitch's existing semantics). A `"*"` + /// entry is an explicit allow-all wildcard; matching is case-insensitive + /// (Twitch logins are case-insensitive). + pub fn user_allowed(&self, login: &str) -> bool { + self.allowed_users.is_empty() + || self + .allowed_users + .iter() + .any(|u| u == "*" || u.eq_ignore_ascii_case(login)) + } } /// Hot-reloadable Signal permission filters. @@ -529,6 +552,17 @@ impl MattermostPermissions { dm_allowed_users, } } + + /// Whether `user_id` may DM the bot. Fail-closed: an empty allowlist + /// blocks all DMs. A `"*"` entry is an explicit allow-all wildcard + /// (consistent with the Signal adapter's convention). + pub fn dm_user_allowed(&self, user_id: &str) -> bool { + !self.dm_allowed_users.is_empty() + && self + .dm_allowed_users + .iter() + .any(|u| u == "*" || u == user_id) + } } fn binding_adapter_selector_matches(binding: &Binding, adapter_selector: Option<&str>) -> bool { @@ -585,3 +619,95 @@ mod base64_tests { assert!(!is_valid_base64(" ")); } } + +#[cfg(test)] +mod dm_wildcard_tests { + use super::*; + + fn slack(users: Vec<&str>) -> SlackPermissions { + SlackPermissions { + workspace_filter: None, + channel_filter: HashMap::new(), + dm_allowed_users: users.into_iter().map(String::from).collect(), + } + } + + fn mattermost(users: Vec<&str>) -> MattermostPermissions { + MattermostPermissions { + team_filter: None, + channel_filter: HashMap::new(), + dm_allowed_users: users.into_iter().map(String::from).collect(), + } + } + + fn twitch(users: Vec<&str>) -> TwitchPermissions { + TwitchPermissions { + channel_filter: None, + allowed_users: users.into_iter().map(String::from).collect(), + } + } + + // --- Slack: fail-closed DM allowlist --- + + #[test] + fn slack_wildcard_allows_any_user() { + let p = slack(vec!["*"]); + assert!(p.dm_user_allowed("U123")); + assert!(p.dm_user_allowed("U-anyone-else")); + } + + #[test] + fn slack_empty_blocks_all() { + assert!(!slack(vec![]).dm_user_allowed("U123")); + } + + #[test] + fn slack_specific_is_exact_match() { + let p = slack(vec!["U123"]); + assert!(p.dm_user_allowed("U123")); + assert!(!p.dm_user_allowed("U999")); + } + + // --- Mattermost: fail-closed DM allowlist (same shape as Slack) --- + + #[test] + fn mattermost_wildcard_allows_any_user() { + assert!(mattermost(vec!["*"]).dm_user_allowed("user-id-abc")); + } + + #[test] + fn mattermost_empty_blocks_all() { + assert!(!mattermost(vec![]).dm_user_allowed("user-id-abc")); + } + + #[test] + fn mattermost_specific_is_exact_match() { + let p = mattermost(vec!["user-id-abc"]); + assert!(p.dm_user_allowed("user-id-abc")); + assert!(!p.dm_user_allowed("other-user")); + } + + // --- Twitch: fail-open user allowlist, case-insensitive --- + + #[test] + fn twitch_empty_allows_all() { + assert!(twitch(vec![]).user_allowed("anyone")); + } + + #[test] + fn twitch_wildcard_allows_all() { + // Before this change, ["*"] would have rejected everyone except a + // literal "*" login — the footgun this wildcard support removes. + let p = twitch(vec!["*"]); + assert!(p.user_allowed("SomeStreamer")); + assert!(p.user_allowed("another_viewer")); + } + + #[test] + fn twitch_specific_is_case_insensitive() { + let p = twitch(vec!["CoolMod"]); + assert!(p.user_allowed("coolmod")); + assert!(p.user_allowed("COOLMOD")); + assert!(!p.user_allowed("someone_else")); + } +} diff --git a/src/messaging/mattermost.rs b/src/messaging/mattermost.rs index b08049394..09a252e3a 100644 --- a/src/messaging/mattermost.rs +++ b/src/messaging/mattermost.rs @@ -1142,13 +1142,10 @@ fn build_message_from_post( } // DM filter: if channel_type is "D", enforce dm_allowed_users (fail-closed) - if post.channel_type.as_deref() == Some("D") { - if context.permissions.dm_allowed_users.is_empty() { - return None; - } - if !context.permissions.dm_allowed_users.contains(&post.user_id) { - return None; - } + if post.channel_type.as_deref() == Some("D") + && !context.permissions.dm_user_allowed(&post.user_id) + { + return None; } // "D" = direct message, "G" = group DM diff --git a/src/messaging/slack.rs b/src/messaging/slack.rs index 4aad68c77..b4e1c0c60 100644 --- a/src/messaging/slack.rs +++ b/src/messaging/slack.rs @@ -205,7 +205,7 @@ async fn handle_message_event( return Ok(()); } if let Some(ref sender_id) = user_id - && !perms.dm_allowed_users.contains(sender_id) + && !perms.dm_user_allowed(sender_id) { tracing::debug!( channel_id, diff --git a/src/messaging/twitch.rs b/src/messaging/twitch.rs index 0974a1326..32ce173f0 100644 --- a/src/messaging/twitch.rs +++ b/src/messaging/twitch.rs @@ -238,9 +238,7 @@ impl Messaging for TwitchAdapter { } // User filter - if !permissions.allowed_users.is_empty() - && !permissions.allowed_users.iter().any(|u| u.eq_ignore_ascii_case(&privmsg.sender.login)) - { + if !permissions.user_allowed(&privmsg.sender.login) { continue; }