Skip to content

fix(slack): accept raw bot IDs in trustedBotIds #814

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
thepagent merged 1 commit into from
May 14, 2026
Merged

fix(slack): accept raw bot IDs in trustedBotIds #814

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/config-reference.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ Slack adapter using Socket Mode. Requires both a Bot User OAuth Token and an App
| `allow_all_users` | bool \| omit | auto-detect | Same behavior as Discord. |
| `allowed_users` | string[] | `[]` | Slack user IDs (e.g. `U0123456789`). |
| `allow_bot_messages` | string | `"off"` | Same as Discord. |
| `trusted_bot_ids` | string[] | `[]` | Slack Bot User IDs (`U...`). Find via: click bot profile → Copy member ID. |
| `trusted_bot_ids` | string[] | `[]` | Slack Bot User IDs (`U...`) or Bot IDs (`B...`). `U...` matching resolves event Bot IDs via Slack `bots.info`, so the bot token needs `users:read`. |
| `allow_user_messages` | string | `"involved"` | Same as Discord. |
| `max_bot_turns` | u32 | `100` | Same as Discord. |

Expand Down
2 changes: 1 addition & 1 deletion docs/messaging.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ BotA in thread: here's my analysis
| Key | Type | Default | Description |
|-----|------|---------|-------------|
| `allow_bot_messages` | string | `"off"` | `"off"` — ignore bot messages. `"mentions"` — only process bot messages that @mention this bot. `"all"` — process all bot messages (capped by `max_bot_turns`). |
| `trusted_bot_ids` | string[] | `[]` | Whitelist of bot IDs. When non-empty, only these bots pass the bot gate. Empty = any bot (mode permitting). Ignored when `allow_bot_messages = "off"`. |
| `trusted_bot_ids` | string[] | `[]` | Whitelist of bot IDs. For Slack, entries may be Bot User IDs (`U...`) or Bot IDs (`B...`); `U...` matching requires `users:read` so OpenAB can call `bots.info`. Empty = any bot (mode permitting). Ignored when `allow_bot_messages = "off"`. |
| `max_bot_turns` | u32 | `20` | Max consecutive bot turns per thread before throttling. A human message resets the counter. |

> **Safety:** When `allow_bot_messages = "all"`, a separate hardcoded cap of 10 consecutive bot turns applies regardless of `max_bot_turns`.
Expand Down
86 changes: 77 additions & 9 deletions src/slack.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,10 @@ impl SlackAdapter {
/// Resolve a Bot ID (B...) to Bot User ID (U...) via bots.info API.
/// Cached permanently (bot IDs don't change).
async fn resolve_bot_user_id(&self, bot_id: &str) -> Option<String> {
if bot_id.is_empty() {
return None;
}

{
let cache = self.bot_id_cache.lock().await;
if let Some(user_id) = cache.get(bot_id) {
Expand All @@ -210,6 +214,13 @@ impl SlackAdapter {
let resp = self
.api_post("bots.info", serde_json::json!({ "bot": bot_id }))
.await
.inspect_err(|e| {
warn!(
bot_id,
error = %e,
"failed to resolve Slack bot ID via bots.info"
)
})
.ok()?;
let user_id = resp.get("bot")?.get("user_id")?.as_str()?.to_string();

Expand All @@ -221,6 +232,21 @@ impl SlackAdapter {
Some(user_id)
}

async fn trusted_bot_ids_contains(
&self,
trusted_bot_ids: &HashSet<String>,
event_bot_id: &str,
) -> bool {
if trusted_bot_ids.is_empty() {
return true;
}
if bot_id_matches_trusted(trusted_bot_ids, event_bot_id, None) {
return true;
}
let resolved = self.resolve_bot_user_id(event_bot_id).await;
bot_id_matches_trusted(trusted_bot_ids, event_bot_id, resolved.as_deref())
}

/// Check whether the bot has participated in a Slack thread and whether
/// other bots have also posted in it.
/// Returns `(involved, other_bot_present)`.
Expand Down Expand Up @@ -538,11 +564,11 @@ pub async fn run_slack_adapter(
AllowBots::Mentions | AllowBots::All => {
if !trusted_bot_ids.is_empty() {
let event_bot_id = event["bot_id"].as_str().unwrap_or("");
let resolved = adapter.resolve_bot_user_id(event_bot_id).await;
let is_trusted = resolved.as_ref()
.is_some_and(|uid| trusted_bot_ids.contains(uid.as_str()));
let is_trusted = adapter
.trusted_bot_ids_contains(&trusted_bot_ids, event_bot_id)
.await;
if !is_trusted {
debug!(event_bot_id, resolved = ?resolved, "bot not in trusted_bot_ids, ignoring app_mention");
debug!(event_bot_id, "bot not in trusted_bot_ids, ignoring app_mention");
continue;
}
}
Expand Down Expand Up @@ -713,12 +739,11 @@ pub async fn run_slack_adapter(
}
// Check trusted_bot_ids
if !trusted_bot_ids.is_empty() {
let resolved = adapter.resolve_bot_user_id(event_bot_id).await;
let is_trusted = resolved
.as_ref()
.is_some_and(|uid| trusted_bot_ids.contains(uid.as_str()));
let is_trusted = adapter
.trusted_bot_ids_contains(&trusted_bot_ids, event_bot_id)
.await;
if !is_trusted {
debug!(event_bot_id, resolved = ?resolved, "bot not in trusted_bot_ids, ignoring");
debug!(event_bot_id, "bot not in trusted_bot_ids, ignoring");
continue;
}
}
Expand Down Expand Up @@ -1171,6 +1196,19 @@ fn strip_mime_params(mimetype: &str) -> &str {
mimetype.split(';').next().unwrap_or(mimetype).trim()
}

fn bot_id_matches_trusted(
trusted_bot_ids: &HashSet<String>,
event_bot_id: &str,
resolved_user_id: Option<&str>,
) -> bool {
if event_bot_id.is_empty() {
return false;
}

trusted_bot_ids.contains(event_bot_id)
|| resolved_user_id.is_some_and(|uid| trusted_bot_ids.contains(uid))
}

/// True only when a Slack non-bot event represents a real user message
/// that should reset the bot-turn counter.
///
Expand Down Expand Up @@ -1364,6 +1402,36 @@ mod tests {
assert_eq!(strip_mime_params(" text/plain "), "text/plain");
}

// --- bot_id_matches_trusted tests ---

#[test]
fn trusted_bot_ids_accepts_raw_slack_bot_id() {
let trusted = HashSet::from(["B123BOT".to_string()]);
assert!(bot_id_matches_trusted(&trusted, "B123BOT", None));
}

#[test]
fn trusted_bot_ids_accepts_resolved_bot_user_id() {
let trusted = HashSet::from(["U123BOT".to_string()]);
assert!(bot_id_matches_trusted(
&trusted,
"B123BOT",
Some("U123BOT")
));
}

#[test]
fn trusted_bot_ids_rejects_unknown_bot_when_resolution_fails() {
let trusted = HashSet::from(["U123BOT".to_string()]);
assert!(!bot_id_matches_trusted(&trusted, "B999BOT", None));
}

#[test]
fn trusted_bot_ids_rejects_empty_event_bot_id() {
let trusted = HashSet::from(["".to_string()]);
assert!(!bot_id_matches_trusted(&trusted, "", None));
}

/// Per-thread streaming: ON by default, OFF when another bot is present (#534).
#[test]
fn streaming_per_thread() {
Expand Down
Loading