From 7cdeb5511524f4e2e4c24209547728e50e5c0b13 Mon Sep 17 00:00:00 2001 From: chaodu-agent Date: Sat, 11 Jul 2026 07:28:10 -0400 Subject: [PATCH 1/3] =?UTF-8?q?feat(trust):=20Phase=201=20(slack)=20?= =?UTF-8?q?=E2=80=94=20L3=20identity=20via=20shared=20gate?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wire Slack into the PlatformTrustConfigs registry and call the shared ingress trust gate from the Slack message path, mirroring the Discord wiring from #1270. Slack was the only configured platform absent from the registry — the gate would have fallen back to the deny-all default had it ever run for slack events. - main.rs: insert "slack" TrustConfig — L2 open (Slack's own channel allowlist stays authoritative in the adapter), L3 mirrors the resolved [slack].allow_all_users/allowed_users, so the gate agrees with Slack's existing user check (behavior-preserving). - slack.rs: thread Arc through run_slack_adapter into handle_message; evaluate gate_incoming after the existing user check (redundant-but-matching, non-regressive). Bots bypass L3 — same rationale as Discord (#1270 review F1): bot admission is allow_bot_messages + trusted_bot_ids, and L3 is human-identity only. - is_dm passed truthfully via Slack conversation-ID prefix (D… = DM), cf. #1270 review F2; decision is identical either way today since the entry is L2-open with allow_dm=true. - tests: pin the L3 bot-bypass and the DM prefix classification. Refs #1361 (first task), umbrella #1356, ADR #1291. --- crates/openab-core/src/slack.rs | 74 +++++++++++++++++++++++++++++++++ src/main.rs | 26 ++++++++++++ 2 files changed, 100 insertions(+) diff --git a/crates/openab-core/src/slack.rs b/crates/openab-core/src/slack.rs index 8e37d5df2..703d6d58a 100644 --- a/crates/openab-core/src/slack.rs +++ b/crates/openab-core/src/slack.rs @@ -803,6 +803,7 @@ fn socket_idle(since_last_inbound: std::time::Duration, timeout: std::time::Dura #[allow(clippy::too_many_arguments)] pub async fn run_slack_adapter( adapter: Arc, + router: Arc, app_token: String, allow_all_channels: bool, allow_all_users: bool, @@ -953,6 +954,7 @@ pub async fn run_slack_adapter( } let event = event.clone(); let adapter = adapter.clone(); + let router = router.clone(); let bot_token = bot_token.clone(); let allowed_channels = allowed_channels.clone(); let allowed_users = allowed_users.clone(); @@ -969,6 +971,7 @@ pub async fn run_slack_adapter( &event, &team_id, &adapter, + &router, &bot_token, allow_all_channels, allow_all_users, @@ -1208,6 +1211,7 @@ pub async fn run_slack_adapter( .to_string(); let event = event.clone(); let adapter = adapter.clone(); + let router = router.clone(); let bot_token = bot_token.clone(); let allowed_channels = allowed_channels.clone(); let allowed_users = allowed_users.clone(); @@ -1220,6 +1224,7 @@ pub async fn run_slack_adapter( &event, &team_id, &adapter, + &router, &bot_token, allow_all_channels, allow_all_users, @@ -1365,11 +1370,32 @@ async fn get_socket_mode_url_with_timeout( .await } +/// Whether the shared L3 identity gate (`AdapterRouter::gate_incoming`) should run +/// for this sender. Bots bypass L3 — mirroring the inline user check's +/// `!is_bot_msg` bypass — because bot admission is a separate concern +/// (`allow_bot_messages` + `trusted_bot_ids`), and L3 (`allowed_users`) is a +/// human-identity allowlist. Running L3 on bots would wrongly deny +/// mode-admitted/trusted bots when `allow_all_users=false` (multi-agent). +/// Same rationale as Discord's bypass (PR #1270 review). +fn l3_gate_applies(is_bot: bool) -> bool { + !is_bot +} + +/// Whether a Slack conversation ID denotes a DM. Slack conversation IDs are +/// prefix-typed: `C…` public channel, `G…` private channel/group, `D…` DM. +/// Only informational for the gate today — Slack's registry entry is L2-open +/// with `allow_dm=true`, so the decision is identical either way — but passing +/// the real surface keeps the gate's inputs truthful (cf. #1270 review F2). +fn is_dm_channel(channel_id: &str) -> bool { + channel_id.starts_with('D') +} + #[allow(clippy::too_many_arguments)] async fn handle_message( event: &serde_json::Value, team_id: &str, adapter: &Arc, + router: &Arc, bot_token: &str, allow_all_channels: bool, allow_all_users: bool, @@ -1422,6 +1448,32 @@ async fn handle_message( return; } + // Shared ingress trust gate (L3 identity). Redundant-but-matching with + // Slack's own user check that already ran above, so it cannot deny anything + // already admitted (non-regressive). L2 (channel allowlist) stays in the + // adapter for Slack — its registry entry is L2-open. + // + // Bots are skipped here: the user check above has the same `!is_bot_msg` + // bypass (bot admission is handled separately by allow_bot_messages + + // trusted_bot_ids), and the shared L3 gate is human-identity only. Running + // it on bots would wrongly drop trusted bot-to-bot messages when + // allow_all_users=false (multi-agent). Same rationale as Discord (#1270 + // review F1). Phase 1c makes this authoritative and removes the scattered + // check (#1361). + if l3_gate_applies(is_bot_msg) { + let decision = + router.gate_incoming("slack", &channel_id, is_dm_channel(&channel_id), &user_id); + if !decision.is_allowed() { + tracing::info!( + user_id, + channel = %channel_id, + ?decision, + "slack message denied by trust gate" + ); + return; + } + } + // Capture the native-streaming recipient for THIS turn, now that the sender has // passed the channel + user allow-list checks above (so denied/unauthorized // senders are never recorded). It rides on the per-turn BufferedMessage to @@ -2048,6 +2100,28 @@ fn build_set_status_body(channel_id: &str, thread_ts: &str, status: &str) -> ser mod tests { use super::*; + // --- trust gate tests --- + + /// Pins the L3 bot-bypass: the shared identity gate must not run for bot + /// senders (bot admission = allow_bot_messages + trusted_bot_ids), matching + /// the inline user check's `!is_bot_msg` bypass. Regression for #1361 + /// (mirrors the Discord test for PR #1270 review F4). + #[test] + fn l3_gate_skips_bots_applies_to_humans() { + assert!(!l3_gate_applies(true)); // bot → gate skipped + assert!(l3_gate_applies(false)); // human → gate applies + } + + /// Pins Slack conversation-ID prefix typing for the gate's `is_dm` input: + /// `D…` = DM; `C…` (public) and `G…` (private/group) are not DMs. + #[test] + fn is_dm_channel_by_prefix() { + assert!(is_dm_channel("D0123456789")); + assert!(!is_dm_channel("C0123456789")); + assert!(!is_dm_channel("G0123456789")); + assert!(!is_dm_channel("")); + } + // --- builder tests --- #[test] diff --git a/src/main.rs b/src/main.rs index 43a7a32f4..e5eaaf62c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -336,6 +336,30 @@ async fn main() -> anyhow::Result<()> { ); } + // Slack: gate L3 (identity) only via the shared gate, mirroring the + // Discord entry above. Slack's L2 (channel allowlist) stays in the + // adapter — its registry entry is L2-open — and L3 mirrors the resolved + // [slack].allow_all_users/allowed_users, so the gate agrees with + // Slack's existing user check (behavior-preserving). Without this + // entry, Slack was the only configured platform absent from the + // registry, falling back to the deny-all default if the gate ever ran + // for it (#1361). + if let Some(s) = &cfg.slack { + reg.insert( + "slack", + TrustConfig::new( + Some(true), // L2 open — Slack's own channel check still applies + Vec::::new(), + Some(true), + Some(config::resolve_allow_all( + s.allow_all_users, + &s.allowed_users, + )), + s.allowed_users.clone(), + ), + ); + } + // Telegram: L3 (identity) mirrors the resolved // [telegram].allow_all_users/allowed_users, so config.toml can // restrict who can message the bot without needing @@ -547,11 +571,13 @@ async fn main() -> anyhow::Result<()> { dispatchers.lock().unwrap().push(slack_dispatcher.clone()); let slack_allowed_users: std::collections::HashSet = slack_cfg.allowed_users.into_iter().collect(); + let slack_router = router.clone(); #[cfg(feature = "filestore")] let slack_filestore = filestore.clone(); Some(tokio::spawn(async move { if let Err(e) = slack::run_slack_adapter( adapter, + slack_router, slack_cfg.app_token, allow_all_channels, allow_all_users, From 7908cb716ccba92098bfc98694c6ff3805fa485b Mon Sep 17 00:00:00 2001 From: chaodu-agent Date: Sat, 11 Jul 2026 08:37:38 -0400 Subject: [PATCH 2/3] refactor(trust): consolidate l3_gate_applies into trust.rs Self-review finding: the Slack wiring copy-pasted l3_gate_applies from discord.rs (identical 3-line fn + doc). Move the single definition next to Decision in trust.rs and import it from both gate call sites. Each adapter keeps its own pinning test against the shared fn. --- crates/openab-core/src/discord.rs | 11 +---------- crates/openab-core/src/slack.rs | 12 +----------- crates/openab-core/src/trust.rs | 11 +++++++++++ 3 files changed, 13 insertions(+), 21 deletions(-) diff --git a/crates/openab-core/src/discord.rs b/crates/openab-core/src/discord.rs index 6cdee8ee2..be0b0a44e 100644 --- a/crates/openab-core/src/discord.rs +++ b/crates/openab-core/src/discord.rs @@ -7,6 +7,7 @@ use crate::dispatch::DispatchTarget; use crate::format; use crate::media; use crate::remind::{self, ReminderStore}; +use crate::trust::l3_gate_applies; use async_trait::async_trait; use serenity::builder::{ CreateActionRow, CreateAttachment, CreateButton, CreateCommand, CreateCommandOption, @@ -2968,16 +2969,6 @@ fn is_denied_user( !is_bot && !allow_all_users && !allowed_users.contains(&user_id) } -/// Whether the shared L3 identity gate (`AdapterRouter::gate_incoming`) should run -/// for this sender. Bots bypass L3 — mirroring [`is_denied_user`]'s `!is_bot` -/// bypass — because bot admission is a separate concern (`allow_bot_messages` + -/// `trusted_bot_ids`), and L3 (`allowed_users`) is a human-identity allowlist. -/// Running L3 on bots would wrongly deny mode-admitted/trusted bots when -/// `allow_all_users=false` (multi-agent). See PR #1270 review. -fn l3_gate_applies(is_bot: bool) -> bool { - !is_bot -} - /// Returns `true` if a bot message should bypass the `allow_bot_messages` mode check. /// A trusted bot that @mentions this bot is treated the same as a human @mention — /// it can pull the bot into a thread regardless of the `allow_bot_messages` setting. diff --git a/crates/openab-core/src/slack.rs b/crates/openab-core/src/slack.rs index 703d6d58a..04af2d9bc 100644 --- a/crates/openab-core/src/slack.rs +++ b/crates/openab-core/src/slack.rs @@ -3,6 +3,7 @@ use crate::adapter::{ChannelRef, ChatAdapter, MessageRef, SenderContext}; use crate::bot_turns::{BotTurnTracker, TurnAction, TurnSeverity}; use crate::config::{AllowBots, AllowUsers, SttConfig}; use crate::media; +use crate::trust::l3_gate_applies; use anyhow::{anyhow, Result}; use async_trait::async_trait; use futures_util::{SinkExt, StreamExt}; @@ -1370,17 +1371,6 @@ async fn get_socket_mode_url_with_timeout( .await } -/// Whether the shared L3 identity gate (`AdapterRouter::gate_incoming`) should run -/// for this sender. Bots bypass L3 — mirroring the inline user check's -/// `!is_bot_msg` bypass — because bot admission is a separate concern -/// (`allow_bot_messages` + `trusted_bot_ids`), and L3 (`allowed_users`) is a -/// human-identity allowlist. Running L3 on bots would wrongly deny -/// mode-admitted/trusted bots when `allow_all_users=false` (multi-agent). -/// Same rationale as Discord's bypass (PR #1270 review). -fn l3_gate_applies(is_bot: bool) -> bool { - !is_bot -} - /// Whether a Slack conversation ID denotes a DM. Slack conversation IDs are /// prefix-typed: `C…` public channel, `G…` private channel/group, `D…` DM. /// Only informational for the gate today — Slack's registry entry is L2-open diff --git a/crates/openab-core/src/trust.rs b/crates/openab-core/src/trust.rs index 603b564fc..5d3ab343b 100644 --- a/crates/openab-core/src/trust.rs +++ b/crates/openab-core/src/trust.rs @@ -50,6 +50,17 @@ impl Decision { } } +/// Whether the shared L3 identity gate (`AdapterRouter::gate_incoming`) should +/// run for this sender. Bots bypass L3 — mirroring the adapters' inline user +/// checks' `!is_bot` bypass — because bot admission is a separate concern +/// (`allow_bot_messages` + `trusted_bot_ids`), and L3 (`allowed_users`) is a +/// human-identity allowlist. Running L3 on bots would wrongly deny +/// mode-admitted/trusted bots when `allow_all_users=false` (multi-agent). +/// See PR #1270 review F1; shared by the Discord and Slack gate call sites. +pub fn l3_gate_applies(is_bot: bool) -> bool { + !is_bot +} + /// Per-platform trust configuration (L2 scope + L3 identity). /// /// Construct via [`TrustConfig::new`], which applies the ADR defaults: From f6606e1332689016a428a681291f52b171b64d62 Mon Sep 17 00:00:00 2001 From: chaodu-agent Date: Sat, 11 Jul 2026 09:51:21 -0400 Subject: [PATCH 3/3] docs(platforms): slack.toml reflects Phase 1 shared-gate wiring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The platform-facts KB (#1295) landed on main after this branch was cut and records 'Slack does NOT use the shared gate' — which this PR makes stale. Update the trust_gate feature entry (partial → implemented, mirroring discord.toml's phrasing) and the Native-trust-divergence quirk. Conformance suite passes against this branch's code refs. --- docs/platforms/schema/slack.toml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/platforms/schema/slack.toml b/docs/platforms/schema/slack.toml index 8e34c46eb..cbcc06e5c 100644 --- a/docs/platforms/schema/slack.toml +++ b/docs/platforms/schema/slack.toml @@ -184,9 +184,9 @@ pr = "" [[openab_features]] feature = "trust_gate" -status = "partial" -note = "Native adapter does NOT use the shared AdapterRouter::gate_incoming (L3 identity trust) — that's wired only for gateway and discord. Slack enforces inline allowlists: allowed_channels and allowed_users, plus trusted_bot_ids resolution (B…→U… via bots.info) for bot senders." -source = ["crates/openab-core/src/slack.rs#allowed_channels", "crates/openab-core/src/slack.rs#trusted_bot_ids_contains"] +status = "implemented" +note = "Two layers, mirroring Discord Phase 1 (#1270): adapter-level inline allowlists (allowed_channels, allowed_users, plus trusted_bot_ids resolution B…→U… via bots.info for bot senders) + shared L3 identity gate router.gate_incoming after the inline check (redundant-but-matching; humans only — bots bypass via trust::l3_gate_applies; is_dm from the D… conversation-ID prefix). Registry entry seeded from resolved [slack] config (L2 open, L3 mirrors allow_all_users/allowed_users)." +source = ["crates/openab-core/src/slack.rs#l3_gate_applies", "crates/openab-core/src/slack.rs#is_dm_channel", "crates/openab-core/src/adapter.rs#gate_incoming", "crates/openab-core/src/trust.rs#l3_gate_applies"] pr = "" [[openab_features]] @@ -273,7 +273,7 @@ source = "crates/openab-core/src/slack.rs#bot_participated_in_thread" [[quirks]] date = "2026-07-04" title = "Native trust divergence" -note = "Unlike the gateway/Discord paths (which call AdapterRouter::gate_incoming), the Slack adapter predates the unified trust gate and enforces access with inline allowed_channels/allowed_users/trusted_bot_ids checks. Deny UX is a 🚫 reaction, not the gateway's DenyIdentity identity-echo. A future consolidation onto AdapterRouter::with_trust would unify this." +note = "The Slack adapter predates the unified trust gate and enforced access only with inline allowed_channels/allowed_users/trusted_bot_ids checks. Since the Slack Phase 1 wiring (#1361/#1363), the shared AdapterRouter::gate_incoming also runs after the inline user check (redundant-but-matching, non-regressive). Deny UX remains the adapter's 🚫 reaction, not the gateway's DenyIdentity identity-echo. Phase 1c makes the shared gate authoritative and removes the inline checks." kind = "openab_decision" source = "crates/openab-core/src/adapter.rs#with_trust"