Skip to content

Commit e3cde1d

Browse files
committed
fix(slack): Slack channel fixes, DM filtering, emoji sanitization, and TLS
Fixes several Slack adapter issues and restores the build after #117. Build fixes: - Restore compile after security middleware changes (Axum State extractor pattern in api_auth_middleware, url::Url → reqwest::Url in browser tool) (this is from unmerged pr #125) Slack DM filtering: - DMs now bypass workspace/channel filters when sender is in dm_allowed_users - dm_allowed_users is merged from both SlackConfig and per-binding configs - Added debug logging for DM permission decisions Emoji reactions: - Sanitize Slack emoji reactions to use shortcodes via the `emojis` crate - Handle edge case where emoji has no shortcode (falls back to name) - Strip colons, normalize whitespace and casing TLS connectivity: - Add tokio-tungstenite with rustls-tls-native-roots feature to fix wss:// connections that broke after the tungstenite 0.28 TLS feature restructure Logging: - Downgrade per-message Slack log from info to debug, matching Discord, Telegram, and Twitch adapters which only use info for lifecycle events Style: - Rename abbreviated `uid` to `sender_id` per style guide - Remove section-divider comments and extra blank lines in imports Tests: - 9 unit tests for sanitize_reaction_name (unicode, shortcodes, fallbacks) - 7 unit tests for SlackPermissions::from_config (merging, dedup, filtering)
1 parent 547d700 commit e3cde1d

6 files changed

Lines changed: 290 additions & 31 deletions

File tree

Cargo.lock

Lines changed: 31 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,9 +91,13 @@ async-trait = "0.1"
9191

9292
# Slack
9393
slack-morphism = { version = "2.17", features = ["hyper"] }
94+
emojis = "0.8"
9495

9596
# TLS (shared crypto backend for slack-morphism, reqwest, teloxide)
9697
rustls = { version = "0.23", default-features = false, features = ["ring"] }
98+
# slack-morphism enables tokio-tungstenite/rustls-native-certs, but tokio-tungstenite 0.28
99+
# restructured features — rustls-tls-native-roots is now needed to activate actual TLS.
100+
tokio-tungstenite = { version = "0.28", features = ["rustls-tls-native-roots"] }
97101

98102
# Telegram
99103
teloxide = { version = "0.17", default-features = false, features = ["rustls"] }

src/api/server.rs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ use super::{
77
};
88

99
use axum::Json;
10-
use axum::extract::Request;
1110
use axum::Router;
11+
use axum::extract::{Request, State};
1212
use axum::http::{StatusCode, Uri, header};
1313
use axum::middleware::{self, Next};
1414
use axum::response::{Html, IntoResponse, Response};
@@ -177,7 +177,11 @@ pub async fn start_http_server(
177177
Ok(handle)
178178
}
179179

180-
async fn api_auth_middleware(state: Arc<ApiState>, request: Request, next: Next) -> Response {
180+
async fn api_auth_middleware(
181+
State(state): State<Arc<ApiState>>,
182+
request: Request,
183+
next: Next,
184+
) -> Response {
181185
let Some(expected_token) = state.auth_token.as_deref() else {
182186
return next.run(request).await;
183187
};
@@ -197,7 +201,11 @@ async fn api_auth_middleware(state: Arc<ApiState>, request: Request, next: Next)
197201
if is_authorized {
198202
next.run(request).await
199203
} else {
200-
(StatusCode::UNAUTHORIZED, Json(json!({"error": "unauthorized"}))).into_response()
204+
(
205+
StatusCode::UNAUTHORIZED,
206+
Json(json!({"error": "unauthorized"})),
207+
)
208+
.into_response()
201209
}
202210
}
203211

src/config.rs

Lines changed: 109 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -877,7 +877,15 @@ impl SlackPermissions {
877877
filter
878878
};
879879

880-
let dm_allowed_users = slack.dm_allowed_users.clone();
880+
let mut dm_allowed_users = slack.dm_allowed_users.clone();
881+
882+
for binding in &slack_bindings {
883+
for id in &binding.dm_allowed_users {
884+
if !dm_allowed_users.contains(id) {
885+
dm_allowed_users.push(id.clone());
886+
}
887+
}
888+
}
881889

882890
Self {
883891
workspace_filter,
@@ -3985,4 +3993,104 @@ bind = "127.0.0.1"
39853993

39863994
assert_eq!(config.api.bind, "[::]");
39873995
}
3996+
3997+
3998+
/// Helper to build a minimal `SlackConfig` for permission tests.
3999+
fn slack_config_with_dm_users(dm_allowed_users: Vec<String>) -> SlackConfig {
4000+
SlackConfig {
4001+
enabled: true,
4002+
bot_token: "xoxb-test".into(),
4003+
app_token: "xapp-test".into(),
4004+
dm_allowed_users,
4005+
commands: vec![],
4006+
}
4007+
}
4008+
4009+
/// Helper to build a Slack binding with optional dm_allowed_users.
4010+
fn slack_binding(workspace_id: Option<&str>, dm_allowed_users: Vec<String>) -> Binding {
4011+
Binding {
4012+
agent_id: "test-agent".into(),
4013+
channel: "slack".into(),
4014+
guild_id: None,
4015+
workspace_id: workspace_id.map(String::from),
4016+
chat_id: None,
4017+
channel_ids: vec![],
4018+
require_mention: false,
4019+
dm_allowed_users,
4020+
}
4021+
}
4022+
4023+
#[test]
4024+
fn slack_permissions_merges_dm_users_from_config_and_bindings() {
4025+
let config = slack_config_with_dm_users(vec!["U001".into(), "U002".into()]);
4026+
let bindings = vec![slack_binding(
4027+
Some("T1"),
4028+
vec!["U003".into(), "U004".into()],
4029+
)];
4030+
let perms = SlackPermissions::from_config(&config, &bindings);
4031+
assert_eq!(perms.dm_allowed_users, vec!["U001", "U002", "U003", "U004"]);
4032+
}
4033+
4034+
#[test]
4035+
fn slack_permissions_deduplicates_dm_users() {
4036+
let config = slack_config_with_dm_users(vec!["U001".into(), "U002".into()]);
4037+
let bindings = vec![slack_binding(
4038+
Some("T1"),
4039+
vec!["U002".into(), "U003".into()],
4040+
)];
4041+
let perms = SlackPermissions::from_config(&config, &bindings);
4042+
// U002 appears in both config and binding — should appear only once
4043+
assert_eq!(perms.dm_allowed_users, vec!["U001", "U002", "U003"]);
4044+
}
4045+
4046+
#[test]
4047+
fn slack_permissions_empty_dm_users_stays_empty() {
4048+
let config = slack_config_with_dm_users(vec![]);
4049+
let bindings = vec![slack_binding(Some("T1"), vec![])];
4050+
let perms = SlackPermissions::from_config(&config, &bindings);
4051+
assert!(perms.dm_allowed_users.is_empty());
4052+
}
4053+
4054+
#[test]
4055+
fn slack_permissions_merges_dm_users_from_multiple_bindings() {
4056+
let config = slack_config_with_dm_users(vec!["U001".into()]);
4057+
let bindings = vec![
4058+
slack_binding(Some("T1"), vec!["U002".into()]),
4059+
slack_binding(Some("T2"), vec!["U003".into()]),
4060+
];
4061+
let perms = SlackPermissions::from_config(&config, &bindings);
4062+
assert_eq!(perms.dm_allowed_users, vec!["U001", "U002", "U003"]);
4063+
}
4064+
4065+
#[test]
4066+
fn slack_permissions_ignores_non_slack_bindings() {
4067+
let config = slack_config_with_dm_users(vec!["U001".into()]);
4068+
let mut discord_binding = slack_binding(Some("T1"), vec!["U099".into()]);
4069+
discord_binding.channel = "discord".into();
4070+
let perms = SlackPermissions::from_config(&config, &[discord_binding]);
4071+
// U099 should not appear — that binding is for discord, not slack
4072+
assert_eq!(perms.dm_allowed_users, vec!["U001"]);
4073+
}
4074+
4075+
#[test]
4076+
fn slack_permissions_workspace_filter_from_bindings() {
4077+
let config = slack_config_with_dm_users(vec![]);
4078+
let bindings = vec![
4079+
slack_binding(Some("T1"), vec![]),
4080+
slack_binding(Some("T2"), vec![]),
4081+
];
4082+
let perms = SlackPermissions::from_config(&config, &bindings);
4083+
assert_eq!(
4084+
perms.workspace_filter,
4085+
Some(vec!["T1".to_string(), "T2".to_string()])
4086+
);
4087+
}
4088+
4089+
#[test]
4090+
fn slack_permissions_no_workspace_filter_when_none_specified() {
4091+
let config = slack_config_with_dm_users(vec![]);
4092+
let bindings = vec![slack_binding(None, vec![])];
4093+
let perms = SlackPermissions::from_config(&config, &bindings);
4094+
assert!(perms.workspace_filter.is_none());
4095+
}
39884096
}

0 commit comments

Comments
 (0)