Skip to content

Commit 79ff4ff

Browse files
authored
Merge pull request spacedriveapp#148 from sra/fix/slack-channel-fixes
fix(slack): Slack channel fixes, DM filtering, emoji sanitization, and restore TLS on websocket
2 parents 9a828ad + 3fa734c commit 79ff4ff

6 files changed

Lines changed: 296 additions & 25 deletions

File tree

.githooks/pre-commit

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
#!/bin/sh
1+
#!/usr/bin/env bash
22

33
set -eu
44

Cargo.lock

Lines changed: 30 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: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ async-trait = "0.1"
9393

9494
# Slack
9595
slack-morphism = { version = "2.17", features = ["hyper"] }
96+
emojis = "0.8"
9697

9798
# TLS (shared crypto backend for slack-morphism, reqwest, teloxide)
9899
rustls = { version = "0.23", default-features = false, features = ["ring"] }

src/config.rs

Lines changed: 108 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1162,7 +1162,15 @@ impl SlackPermissions {
11621162
filter
11631163
};
11641164

1165-
let dm_allowed_users = slack.dm_allowed_users.clone();
1165+
let mut dm_allowed_users = slack.dm_allowed_users.clone();
1166+
1167+
for binding in &slack_bindings {
1168+
for id in &binding.dm_allowed_users {
1169+
if !dm_allowed_users.contains(id) {
1170+
dm_allowed_users.push(id.clone());
1171+
}
1172+
}
1173+
}
11661174

11671175
Self {
11681176
workspace_filter,
@@ -4661,6 +4669,105 @@ bind = "127.0.0.1"
46614669
assert_eq!(config.api.bind, "[::]");
46624670
}
46634671

4672+
/// Helper to build a minimal `SlackConfig` for permission tests.
4673+
fn slack_config_with_dm_users(dm_allowed_users: Vec<String>) -> SlackConfig {
4674+
SlackConfig {
4675+
enabled: true,
4676+
bot_token: "xoxb-test".into(),
4677+
app_token: "xapp-test".into(),
4678+
dm_allowed_users,
4679+
commands: vec![],
4680+
}
4681+
}
4682+
4683+
/// Helper to build a Slack binding with optional dm_allowed_users.
4684+
fn slack_binding(workspace_id: Option<&str>, dm_allowed_users: Vec<String>) -> Binding {
4685+
Binding {
4686+
agent_id: "test-agent".into(),
4687+
channel: "slack".into(),
4688+
guild_id: None,
4689+
workspace_id: workspace_id.map(String::from),
4690+
chat_id: None,
4691+
channel_ids: vec![],
4692+
require_mention: false,
4693+
dm_allowed_users,
4694+
}
4695+
}
4696+
4697+
#[test]
4698+
fn slack_permissions_merges_dm_users_from_config_and_bindings() {
4699+
let config = slack_config_with_dm_users(vec!["U001".into(), "U002".into()]);
4700+
let bindings = vec![slack_binding(
4701+
Some("T1"),
4702+
vec!["U003".into(), "U004".into()],
4703+
)];
4704+
let perms = SlackPermissions::from_config(&config, &bindings);
4705+
assert_eq!(perms.dm_allowed_users, vec!["U001", "U002", "U003", "U004"]);
4706+
}
4707+
4708+
#[test]
4709+
fn slack_permissions_deduplicates_dm_users() {
4710+
let config = slack_config_with_dm_users(vec!["U001".into(), "U002".into()]);
4711+
let bindings = vec![slack_binding(
4712+
Some("T1"),
4713+
vec!["U002".into(), "U003".into()],
4714+
)];
4715+
let perms = SlackPermissions::from_config(&config, &bindings);
4716+
// U002 appears in both config and binding — should appear only once
4717+
assert_eq!(perms.dm_allowed_users, vec!["U001", "U002", "U003"]);
4718+
}
4719+
4720+
#[test]
4721+
fn slack_permissions_empty_dm_users_stays_empty() {
4722+
let config = slack_config_with_dm_users(vec![]);
4723+
let bindings = vec![slack_binding(Some("T1"), vec![])];
4724+
let perms = SlackPermissions::from_config(&config, &bindings);
4725+
assert!(perms.dm_allowed_users.is_empty());
4726+
}
4727+
4728+
#[test]
4729+
fn slack_permissions_merges_dm_users_from_multiple_bindings() {
4730+
let config = slack_config_with_dm_users(vec!["U001".into()]);
4731+
let bindings = vec![
4732+
slack_binding(Some("T1"), vec!["U002".into()]),
4733+
slack_binding(Some("T2"), vec!["U003".into()]),
4734+
];
4735+
let perms = SlackPermissions::from_config(&config, &bindings);
4736+
assert_eq!(perms.dm_allowed_users, vec!["U001", "U002", "U003"]);
4737+
}
4738+
4739+
#[test]
4740+
fn slack_permissions_ignores_non_slack_bindings() {
4741+
let config = slack_config_with_dm_users(vec!["U001".into()]);
4742+
let mut discord_binding = slack_binding(Some("T1"), vec!["U099".into()]);
4743+
discord_binding.channel = "discord".into();
4744+
let perms = SlackPermissions::from_config(&config, &[discord_binding]);
4745+
// U099 should not appear — that binding is for discord, not slack
4746+
assert_eq!(perms.dm_allowed_users, vec!["U001"]);
4747+
}
4748+
4749+
#[test]
4750+
fn slack_permissions_workspace_filter_from_bindings() {
4751+
let config = slack_config_with_dm_users(vec![]);
4752+
let bindings = vec![
4753+
slack_binding(Some("T1"), vec![]),
4754+
slack_binding(Some("T2"), vec![]),
4755+
];
4756+
let perms = SlackPermissions::from_config(&config, &bindings);
4757+
assert_eq!(
4758+
perms.workspace_filter,
4759+
Some(vec!["T1".to_string(), "T2".to_string()])
4760+
);
4761+
}
4762+
4763+
#[test]
4764+
fn slack_permissions_no_workspace_filter_when_none_specified() {
4765+
let config = slack_config_with_dm_users(vec![]);
4766+
let bindings = vec![slack_binding(None, vec![])];
4767+
let perms = SlackPermissions::from_config(&config, &bindings);
4768+
assert!(perms.workspace_filter.is_none());
4769+
}
4770+
46644771
#[test]
46654772
fn test_cron_timezone_resolution_precedence() {
46664773
let _lock = env_test_lock()

0 commit comments

Comments
 (0)