From 842cad0587f7078ee3b0cffafe55b691d770a665 Mon Sep 17 00:00:00 2001 From: chaodu-agent Date: Sat, 4 Jul 2026 13:07:23 +0000 Subject: [PATCH 01/12] docs(adr): revise identity-trust-none to three-layer architecture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Receiver → Trust Gate → Handler replaces the previous 'gate at handle_message()' design. Addresses all findings from the PR #1263 mob review (howie + 3 LLM reviewers). Key changes: - §4.2: Trust Gate is a dedicated ingress layer upstream of Handler - §5: New architecture diagram showing three-layer separation - §7: Implementation plan starts with Receiver/Handler split - Address #1: gate at actual convergence point (not handle_message) - Address #2: trust lookup keys off per-event platform (not adapter) - Address #3: slash commands gated (Handler is downstream of gate) - Address #4: exhaustive scattered-checks inventory - Address #5: explicit empty-vs-missing semantics - Address #6: phased rollout (Phase 0-3) - Address #7: echo rate-limit + bot exclusion + DM-preferred - Address #8: gateway vs first-class section precedence - Address #9: no static HashSet (runtime construction) - Address #10: structured logging on allow + deny - Address #11-#15: minor fixes (Teams ID, bot semantics, etc.) --- docs/adr/identity-trust-none.md | 401 ++++++++++++++++++++++---------- 1 file changed, 274 insertions(+), 127 deletions(-) diff --git a/docs/adr/identity-trust-none.md b/docs/adr/identity-trust-none.md index 98ee26331..2eec46b4c 100644 --- a/docs/adr/identity-trust-none.md +++ b/docs/adr/identity-trust-none.md @@ -1,9 +1,9 @@ # ADR: Identity Trust-None Default & Trust Pyramid -- **Status:** Proposed -- **Date:** 2026-06-30 +- **Status:** Proposed (v2 — revised per PR #1263 review feedback) +- **Date:** 2026-06-30 (revised 2026-07-04) - **Author:** @chaodu-agent -- **Reviewers:** @pahud +- **Reviewers:** @pahud, @howie - **Tracking issues:** #1262 - **Depends on:** [First-Class Per-Platform Configuration](first-class-platform-config.md) — per-platform `allowed_users` live in the first-class `[platform]` sections defined there. @@ -16,8 +16,11 @@ platform's `allowed_users` is empty and `allow_all_users` is not explicitly set `true`, deny all incoming messages and echo the sender their own ID so they can request access. -Trust is enforced at a **single router-level gate** (`AdapterRouter::handle_message()`), -not scattered across adapters. +Trust is enforced at a **dedicated ingress layer** — the Trust Gate — that sits +between the platform Receiver and the per-platform Handler. This is a structural +guarantee: no event reaches any Handler (or the Dispatcher / Agent) without passing +through the gate. The gate is **not** inside any adapter — it is an independent +layer that all adapters are wired through. ## 2. Motivation: trust-all default is insecure @@ -25,6 +28,13 @@ All adapters currently auto-detect: empty `allowed_users` → `allow_all_users = A fresh deployment trusts **everyone** by default. For publicly discoverable bots (e.g. anyone can DM a Telegram bot), this means any stranger can drive the agent. +Additionally, trust checks are currently **scattered** across adapters — each one +implements its own variant (`is_denied_user()` in Discord, `should_skip_event()` in +Gateway, inline allowlist in Slack). This means: +- Different implementations doing the same thing +- A new adapter forgetting the check = fully open bot +- No architectural guarantee that trust is enforced + ## 3. Trust Pyramid (Defense in Depth) Three layers with **clearly separated responsibilities** — only L1 and L3 are @@ -132,54 +142,131 @@ When a message arrives from an untrusted sender: 2. Reply with an echo message showing the sender their own ID 3. Do NOT dispatch to any agent -### 4.2 Trust check at router level (single gate) +**Semantics of `allowed_users`:** +- Missing key = empty list = deny-all (unless `allow_all_users = true`) +- Empty string sender_id = always denied (fail-closed, regardless of `allow_all_users`) +- Startup validation: warn when a platform section has neither `allowed_users` nor + explicit `allow_all_users = true` — helps operators catch misconfiguration. + +### 4.2 Three-layer adapter architecture (Receiver → Trust Gate → Handler) + +Trust enforcement happens in a **dedicated ingress layer** — the Trust Gate — +that is structurally between the Receiver and the Handler. This is NOT inside +any adapter. It is an independent layer that every platform flows through. + +**Architecture: Receiver → Trust Gate → Handler** + +Each adapter is split into two components with the Trust Gate in between: + +| Layer | Responsibility | Per-platform? | +|-------|---------------|---------------| +| **Receiver** | Connect, listen, L1 verify, normalize to `InboundEvent` | Yes | +| **Trust Gate** | L2 scope check + L3 identity check (`decide()`) | **No — unified** | +| **Handler** | Platform-specific interaction logic + dispatch | Yes | -Trust enforcement happens in **one place only**: `AdapterRouter::handle_message()`. -The gateway remains a pure transport layer (L1 only). +**Why this order:** +- Trust Gate is upstream of Handler — Handler never sees untrusted events +- Slash commands (`/reset`, `/cancel`) are in the Handler — they are gated +- No adapter can bypass the gate — it is architecturally mandatory +- New platform = write Receiver + Handler; trust is automatic + +**Trust lookup key:** The gate uses the **per-event platform** from +`InboundEvent.platform` (which maps to `ChannelRef.platform`), NOT +`adapter.platform()`. This correctly handles unified mode where a single +`UnifiedGatewayAdapter` (whose `platform()` returns `"unified"`) multiplexes +events from multiple real platforms. ## 5. Architecture ``` -┌──────────────┐ ┌──────────────┐ ┌──────────────┐ ┌──────────────┐ -│ Telegram │ │ LINE │ │ Feishu │ │ WeCom / GC │ -│ Webhook │ │ Webhook │ │ WebSocket │ │ Webhook │ -└──────┬───────┘ └──────┬───────┘ └──────┬───────┘ └──────┬───────┘ - │ │ │ │ - ▼ ▼ ▼ ▼ -┌─────────────────────────────────────────────────────────────────────┐ -│ openab-gateway — L1: Platform Authentication │ -│ │ -│ ✅ Verify webhook signature / JWT / secret token / IP │ -│ ✅ Normalize → GatewayEvent │ -│ ✅ Forward ALL authenticated events │ -│ ❌ No user filtering (L3 is in core) │ -└────────────────────────────┬────────────────────────────────────────┘ - │ WebSocket - ▼ -┌─────────────────────────────────────────────────────────────────────┐ -│ openab-core — L2 + L3 │ -│ │ -│ ┌───────────┐ ┌───────────┐ ┌─────────────────────────────┐ │ -│ │ Discord │ │ Slack │ │ GatewayAdapter │ │ -│ │ Handler │ │ Handler │ │ (TG/LINE/Feishu/WeCom/GC) │ │ -│ └─────┬─────┘ └─────┬─────┘ └──────────────┬──────────────┘ │ -│ └──────────────┼──────────────────────┘ │ -│ ▼ │ -│ ┌───────────────────────────────────────────────────────────────┐ │ -│ │ 🔒 AdapterRouter::handle_message() │ │ -│ │ │ │ -│ │ L2: scope check (optional, default-open; channel/group/DM) │ │ -│ │ L3: TrustConfig::is_allowed(platform, sender_id) — DENY dflt │ │ -│ │ │ │ -│ │ if denied → log + echo sender ID → RETURN │ │ -│ │ if allowed → dispatch to ACP ✅ │ │ -│ └───────────────────────────────────────────────────────────────┘ │ -│ │ │ -│ ▼ │ -│ ┌─────────────────┐ │ -│ │ ACP Session │ │ -│ └─────────────────┘ │ -└─────────────────────────────────────────────────────────────────────┘ +┌──────────────────────────────────────────────────────────────────────────┐ +│ Platform Sources │ +├──────────────┬───────────────┬────────────────┬─────────────────────────┤ +│ Discord │ Slack │ Gateway │ Cron │ +│ (WebSocket) │ (Socket Mode) │ (TG/LINE/..) │ (timer) │ +└──────┬───────┴───────┬───────┴───────┬────────┴────────────┬────────────┘ + │ │ │ │ + ▼ ▼ ▼ ▼ +┌──────────────────────────────────────────────────────────────────────────┐ +│ Layer 1: Receivers (per-platform transport) │ +│ │ +│ ┌──────────┐ ┌──────────┐ ┌──────────────┐ ┌──────────┐ │ +│ │ Discord │ │ Slack │ │ Gateway │ │ Cron │ │ +│ │ Receiver │ │ Receiver │ │ Receiver │ │ Receiver │ │ +│ └──────────┘ └──────────┘ └──────────────┘ └──────────┘ │ +│ │ +│ Responsibilities: │ +│ • Connect & listen (WebSocket / HTTP webhook / timer) │ +│ • L1 authentication (verify signature / JWT / token) │ +│ • Normalize → InboundEvent { platform, sender_id, channel_id, is_dm } │ +│ │ +│ Does NOT: │ +│ • ❌ Check allowed_users │ +│ • ❌ Handle slash commands │ +│ • ❌ Evaluate @mention / multibot / role logic │ +└──────────────────────────────────┬───────────────────────────────────────┘ + │ + │ InboundEvent (unified format) + ▼ +┌──────────────────────────────────────────────────────────────────────────┐ +│ 🔒 Layer 2+3: TRUST GATE (unified, one implementation) 🔒 │ +│ │ +│ PlatformTrustConfigs::decide( │ +│ event.platform, // per-event platform (not adapter.platform()) │ +│ event.channel_id, │ +│ event.is_dm, │ +│ event.sender_id, │ +│ ) → Decision { Allow | DenyScope | DenyIdentity } │ +│ │ +│ L2 (scope): surface_allowed(channel_id, is_dm) — default OPEN │ +│ L3 (identity): identity_allowed(sender_id) — default DENY-ALL │ +│ │ +│ On DenyIdentity: echo sender ID (with rate-limit) │ +│ On DenyScope: silent drop │ +│ On Allow: pass event to Handler ↓ │ +│ │ +│ Bot messages: if is_bot → skip L3 (bot admission stays in Handler) │ +│ │ +│ 🔑 Architectural guarantee: Handler never receives untrusted events │ +└──────────────────────────────────┬───────────────────────────────────────┘ + │ + │ Only Allow events reach here + ▼ +┌──────────────────────────────────────────────────────────────────────────┐ +│ Layer 4: Handlers (per-platform interaction logic) │ +│ │ +│ ┌────────────┐ ┌────────────┐ ┌─────────────┐ ┌───────────┐ │ +│ │ Discord │ │ Slack │ │ Gateway │ │ Cron │ │ +│ │ Handler │ │ Handler │ │ Handler │ │ Handler │ │ +│ │ │ │ │ │ │ │ │ │ +│ │ • @mention │ │ • thread │ │ • /reset │ │ • format │ │ +│ │ • role │ │ • assist │ │ • /cancel │ │ prompt │ │ +│ │ • multibot │ │ mode │ │ • group │ │ │ │ +│ │ • reaction │ │ • emoji │ │ routing │ │ │ │ +│ │ • channel │ │ │ │ │ │ │ │ +│ └─────┬──────┘ └─────┬──────┘ └──────┬──────┘ └─────┬─────┘ │ +│ │ │ │ │ │ +└────────┼───────────────┼────────────────┼───────────────┼──────────────┘ + │ │ │ │ + ▼ ▼ ▼ ▼ +┌──────────────────────────────────────────────────────────────────────────┐ +│ Dispatcher → dispatch_batch() → ACP Session │ +└──────────────────────────────────────────────────────────────────────────┘ +``` + +### InboundEvent (Receiver output / Trust Gate input) + +```rust +/// Unified inbound event produced by all Receivers. +/// Contains the minimum fields needed for trust evaluation. +pub struct InboundEvent { + pub platform: String, // "discord", "telegram", "line", etc. + pub sender_id: String, // platform-specific sender identifier + pub channel_id: String, // conversation surface + pub is_dm: bool, // DM vs group/channel + pub is_bot: bool, // bot-originated message + pub raw: RawPlatformEvent, // opaque; Handler interprets this +} ``` ### Per-platform TrustConfig @@ -206,123 +293,171 @@ impl TrustConfig { } /// L3: is this identity trusted? (default-deny) - pub fn is_allowed(&self, sender_id: &str) -> bool { + pub fn identity_allowed(&self, sender_id: &str) -> bool { + if sender_id.is_empty() { return false; } // fail-closed on empty ID self.allow_all_users || self.allowed_users.contains(sender_id) } -} - -/// Router holds one TrustConfig per platform -pub struct PlatformTrustConfigs { - configs: HashMap, // keyed by platform name -} -impl PlatformTrustConfigs { - pub fn get(&self, platform: &str) -> &TrustConfig { - self.configs.get(platform).unwrap_or(&DEFAULT) + /// Combined decision: L2 then L3. + pub fn decide(&self, channel_id: &str, is_dm: bool, sender_id: &str) -> Decision { + if !self.surface_allowed(channel_id, is_dm) { + return Decision::DenyScope; + } + if !self.identity_allowed(sender_id) { + return Decision::DenyIdentity; + } + Decision::Allow } } -/// Default: L2 open (act anywhere the platform allows), L3 deny-all. -static DEFAULT: TrustConfig = TrustConfig { - allow_all_channels: true, - allowed_channels: HashSet::new(), - allow_dm: true, - allow_all_users: false, // trust-none on identity - allowed_users: HashSet::new(), -}; +/// Decision outcome. +#[non_exhaustive] +pub enum Decision { + Allow, + DenyScope, // silent drop (L2 — not a security failure) + DenyIdentity, // echo sender ID (L3 — request-access UX) +} ``` -### Trait & Type Changes (no new trait) - -The trust gate is **uniform logic**, not per-platform behavior, so it is a plain -`TrustConfig` + a router method — **not** a `ChatAdapter` method and **not** a new -trait (see Rejected Alternatives). The `ChatAdapter` trait is unchanged: -`platform()` already keys the `TrustConfig` and `send_message()` already performs -the echo. What changes are the **shared data carriers** that feed the router: - -**1. `MessageContext` — carry structured sender identity (not opaque JSON).** -Today the router only receives `sender_json` (a serialized blob); it would have to -parse JSON to read `sender_id`. Pass the `SenderContext` struct so L3 can read -`sender_id` / `is_bot` directly (the router can still serialize it for the agent): +### PlatformTrustConfigs (registry) ```rust -pub struct MessageContext { - pub thread_channel: ChannelRef, - pub sender: SenderContext, // ← was: sender_json: String - pub prompt: String, - pub extra_blocks: Vec, - pub trigger_msg: MessageRef, - pub other_bot_present: bool, +pub struct PlatformTrustConfigs { + configs: HashMap, // keyed by lowercase platform name } -``` -**2. `ChannelRef` — add an `is_dm` flag.** -DM detection is platform-specific *structural* knowledge the adapter already has at -construction time (Discord DM channel vs Telegram private chat vs Slack IM), so it -is a **field the adapter populates**, not a trait method. This lets the router -evaluate `allow_dm` (L2) uniformly: +impl PlatformTrustConfigs { + /// Look up by per-event platform (case-insensitive). + /// Unknown platform → default config (L2 open, L3 deny-all). + pub fn decide(&self, platform: &str, channel_id: &str, is_dm: bool, sender_id: &str) -> Decision { + let config = self.configs + .get(&platform.to_lowercase()) + .unwrap_or(&Self::default_config()); + config.decide(channel_id, is_dm, sender_id) + } -```rust -pub struct ChannelRef { - pub platform: String, - pub channel_id: String, - pub is_dm: bool, // ← new; excluded from Hash/Eq like origin_event_id - pub thread_id: Option, - pub parent_id: Option, - pub origin_event_id: Option, + fn default_config() -> TrustConfig { + // L2 open, L3 deny-all — unknown platform = nobody in. + TrustConfig { + allow_all_channels: true, + allowed_channels: HashSet::new(), + allow_dm: true, + allow_all_users: false, + allowed_users: HashSet::new(), + } + } } ``` -**3. Remove scattered trust checks from adapters.** -The real refactor is deleting the `allowed_channels` / `allowed_users` checks -currently in `discord.rs`, `slack.rs`, and `gateway.rs`, and letting the data flow -into `MessageContext` / `ChannelRef` so the single router gate is the only place -trust is enforced — this is what makes L3 un-bypassable. Structural concerns -(thread detection, @mention gating, multibot detection, bot-ownership) **stay in -the adapters** — they are not trust. +Note: The default config uses a runtime-constructed `TrustConfig` (not a `static` +with `HashSet::new()` which would not compile). The actual implementation uses +`LazyLock` or returns a fresh default; see `trust.rs` on main for the real code. + +### Bot message handling + +Bot messages (where `InboundEvent.is_bot == true`) **bypass L3** at the Trust Gate. +Bot admission is NOT part of the identity trust model — it is platform-specific +structural logic (e.g. `trusted_bot_ids`, `allow_bot_messages`) that stays in the +Handler. The Trust Gate only evaluates human sender identity. ### Echo reply on deny ```rust -// In AdapterRouter::handle_message() -let echo = format!( - "⚠️ You are not in the trusted list.\nYour ID: {}\nPlease ask the admin to add you to [{}].allowed_users.", - msg.sender_id, - adapter.platform() -); -let _ = adapter.send_message(&msg.channel, &echo).await; +// In the Trust Gate layer (not in any adapter) +if decision == Decision::DenyIdentity { + let echo = format!( + "⚠️ You are not in the trusted list.\nYour ID: {}\nPlease ask the admin to add you to [{}].allowed_users.", + event.sender_id, + event.platform, // per-event platform, not adapter.platform() + ); + send_echo(&event, &echo).await; +} ``` +**Echo safeguards:** +- **Rate-limit:** max 1 echo per sender per platform per 5 minutes (prevents spam/DoS amplification) +- **Bot exclusion:** if `is_bot` → silent deny, no echo (prevents infinite reply loops between bots) +- **DM preferred:** in group/channel contexts, prefer DM reply to avoid leaking sender UID publicly; fall back to in-channel if DM unavailable +- **Best-effort:** echo delivery is not guaranteed (e.g. LINE reply tokens expire); this is acceptable — the echo is a UX convenience, not a security mechanism + +**Platform-specific delivery caveats:** +- **LINE:** reply tokens are single-use and short-TTL (~30s). If the echo cannot use the reply token, fall back to push message API (requires separate quota/permission). +- **Other platforms:** no known delivery constraints for the echo use case. + ## 6. Migration -### Breaking change +### Phased rollout (not a hard cutover) -Existing deployments with no `allowed_users` configured will stop accepting messages. +The default flip is phased to avoid silently severing live bots on upgrade: + +| Phase | Behavior | When | +|-------|----------|------| +| **Phase 0** | Types + `decide()` defined, no runtime behavior change. Additive only. | Done (on main) | +| **Phase 1** | Wire Trust Gate into ingress pipeline. **Keep current allow-all default.** Log deprecation warning when relying on implicit allow-all. | Next release | +| **Phase 2** | Require explicit `allow_all_users = true` to preserve old behavior. Deployments without it get a **startup error** (not silent denial). | Pre-GA release | +| **Phase 3** | Flip default: empty `allowed_users` + no `allow_all_users` = **deny-all**. | GA release | ### Migration path ```toml -# Before (implicit trust-all): +# Before (implicit trust-all — works in Phase 0/1, warns in Phase 1, errors in Phase 2): [discord] bot_token = "..." -# After (explicit trust-all to keep old behavior): +# After (explicit trust-all to keep old behavior across all phases): [discord] bot_token = "..." allow_all_users = true + +# Or (recommended — actually configure trust): +[discord] +bot_token = "..." +allowed_users = ["845835116920307722"] ``` +### `[gateway]` vs first-class section precedence + +When both a deprecated `[gateway]` section and a matching first-class section +(e.g. `[telegram]`) exist in config, the **first-class section wins**. The +`[gateway]` entry for that platform is ignored and a deprecation warning is +logged at startup. If only `[gateway]` exists for a platform, it remains +functional. + ## 7. Implementation Plan -1. **Define `TrustConfig` + `PlatformTrustConfigs`** in `openab-core` -2. **Extend carriers** — add `is_dm` to `ChannelRef`; pass `SenderContext` in `MessageContext` -3. **Wire trust gate into `AdapterRouter::handle_message()`** — L2 (scope) then L3 (identity), single check point -4. **Remove scattered trust checks** from: - - `is_denied_user()` in Discord EventHandler - - `should_skip_event()` user/channel filter in `gateway.rs` - - `allowed_users` checks in Slack / Feishu adapters -5. **Add echo reply** on deny using `ChatAdapter::send_message()` -6. **Update `config.toml.example`** and docs; migration guide in release notes +1. **Define `InboundEvent`** — unified event struct that all Receivers produce +2. **Refactor adapters into Receiver + Handler** — starting with Discord and + Gateway (Telegram). The Receiver produces `InboundEvent`; the Handler consumes + only events that passed the Trust Gate. +3. **Wire Trust Gate as the ingress layer** between Receiver and Handler: + - Receives `InboundEvent` from Receiver + - Calls `PlatformTrustConfigs::decide(event.platform, ...)` + - Passes allowed events to Handler + - Echoes + drops denied events +4. **Remove scattered trust checks** — replaced by the unified Trust Gate: + - `is_denied_user()` in Discord EventHandler (`discord.rs:2892`) + - `should_skip_event()` user/channel filter in `gateway.rs` (`:832`, `:1160`) + - Inline user allowlist in Slack (`slack.rs:1224`) + - Feishu L3 check in the gateway crate (`feishu.rs:425`) — must relocate to + core, not just delete (contradicts "gateway = L1 only" model) + - Discord reaction-dispatch gating (`discord.rs:1241`) + - Note: `trusted_bot_ids`, `allow_bot_messages`, `allowed_role_ids` **stay in + Handlers** — they are structural/trigger semantics, not identity trust. +5. **Add echo reply with safeguards** — rate-limit, bot exclusion, DM-preferred +6. **Structured logging** — log sender_id + platform on both deny AND allow + (existing dispatch logs use sender name; add structured sender_id field) +7. **Update `config.toml.example`** and docs; migration guide in release notes + +### What stays in Handlers (NOT moved to Trust Gate) + +These are platform-specific structural concerns, not trust: +- Thread detection and routing +- @mention gating and multibot detection +- Bot-ownership and `trusted_bot_ids` +- `allowed_role_ids` (Discord role-based trigger control) +- Reaction dispatch gating (triggers, not authorization) +- Slash command routing (`/reset`, `/cancel`) — but note these now run AFTER the + Trust Gate, so untrusted senders cannot invoke them. ## 8. Rejected Alternatives @@ -332,7 +467,7 @@ Each adapter implements `is_trusted_sender()`. Rejected because: - Trust logic is identical across all platforms (`allowed_users.contains(id)`) - Forces N identical implementations with no polymorphic benefit - New adapter forgetting to implement = security hole -- Router-level gate is impossible to bypass by construction +- Three-layer architecture makes this impossible to bypass by construction ### Trust check at gateway layer @@ -341,6 +476,18 @@ Gateway adapters filter untrusted senders before forwarding. Rejected because: - Trust config lives in core's `config.toml`, not gateway env vars - Reply capability already wired in core via `ChatAdapter::send_message()` +### Trust gate inside Dispatcher::submit() (downstream of adapter) + +Wire gate into `Dispatcher::submit()` or `AdapterRouter::handle_message()`. +Rejected because: +- Gate is downstream of the Handler — Handler still receives untrusted events +- Slash commands (`/reset`, `/cancel`) processed in the Handler would execute + before the gate is reached +- Does not provide the architectural guarantee that untrusted events are invisible + to platform-specific logic +- The "by construction" safety property requires the gate to be UPSTREAM of any + platform-specific code that acts on events + ### Treating L2 (channel) as a security layer Rejected: the platform already enforces channel/group membership, so L2 is From a4711618f69870675f0b8eb76b629dca21818198 Mon Sep 17 00:00:00 2001 From: chaodu-agent Date: Sat, 4 Jul 2026 13:21:24 +0000 Subject: [PATCH 02/12] docs(adr): address team review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add type-level guarantee (GatedEvent vs InboundEvent) — compile-time enforcement, not just convention (#4) - Clarify Gateway Receiver is one receiver that demuxes by platform (#11) - Fix layer numbering inconsistency — use names, not numbers (#21) - Add sender ID format table with per-platform gotchas (#22, #23, #24) - Clarify is_bot bypass is caller-side, not inside decide() (擺渡-1) - Change echo group fallback to silent drop (avoid UID leakage) (#6) --- docs/adr/identity-trust-none.md | 79 +++++++++++++++++++++++++++++++-- 1 file changed, 75 insertions(+), 4 deletions(-) diff --git a/docs/adr/identity-trust-none.md b/docs/adr/identity-trust-none.md index 2eec46b4c..3579a1477 100644 --- a/docs/adr/identity-trust-none.md +++ b/docs/adr/identity-trust-none.md @@ -170,6 +170,28 @@ Each adapter is split into two components with the Trust Gate in between: - No adapter can bypass the gate — it is architecturally mandatory - New platform = write Receiver + Handler; trust is automatic +**Type-level enforcement (compile-time guarantee):** +The "impossible to bypass" property is enforced via Rust's type system, not just +calling convention. The Trust Gate consumes `InboundEvent` and produces a +**different type** — `GatedEvent` — which is the only type Handler accepts: + +```rust +/// Receiver produces this (untrusted). +pub struct InboundEvent { /* ... */ } + +/// Trust Gate produces this (trusted). Only constructible by the gate. +pub struct GatedEvent { + pub(crate) inner: InboundEvent, // pub(crate) — Handler cannot forge this +} + +/// Handler signature — cannot accept InboundEvent directly. +async fn handle(&self, event: GatedEvent) { /* ... */ } +``` + +A Handler that tries to accept `InboundEvent` directly will not compile. +A Receiver that tries to construct `GatedEvent` will fail (private field). +This makes the bypass impossible at compile time, not just by convention. + **Trust lookup key:** The gate uses the **per-event platform** from `InboundEvent.platform` (which maps to `ChannelRef.platform`), NOT `adapter.platform()`. This correctly handles unified mode where a single @@ -188,7 +210,7 @@ events from multiple real platforms. │ │ │ │ ▼ ▼ ▼ ▼ ┌──────────────────────────────────────────────────────────────────────────┐ -│ Layer 1: Receivers (per-platform transport) │ +│ Receivers (per-platform transport) │ │ │ │ ┌──────────┐ ┌──────────┐ ┌──────────────┐ ┌──────────┐ │ │ │ Discord │ │ Slack │ │ Gateway │ │ Cron │ │ @@ -209,7 +231,7 @@ events from multiple real platforms. │ InboundEvent (unified format) ▼ ┌──────────────────────────────────────────────────────────────────────────┐ -│ 🔒 Layer 2+3: TRUST GATE (unified, one implementation) 🔒 │ +│ 🔒 TRUST GATE (L2 scope + L3 identity, unified) 🔒 │ │ │ │ PlatformTrustConfigs::decide( │ │ event.platform, // per-event platform (not adapter.platform()) │ @@ -233,7 +255,7 @@ events from multiple real platforms. │ Only Allow events reach here ▼ ┌──────────────────────────────────────────────────────────────────────────┐ -│ Layer 4: Handlers (per-platform interaction logic) │ +│ Handlers (per-platform interaction logic) │ │ │ │ ┌────────────┐ ┌────────────┐ ┌─────────────┐ ┌───────────┐ │ │ │ Discord │ │ Slack │ │ Gateway │ │ Cron │ │ @@ -256,6 +278,14 @@ events from multiple real platforms. ### InboundEvent (Receiver output / Trust Gate input) +**Gateway Receiver note:** The Gateway Receiver is a **single receiver** that +connects to the openab-gateway WebSocket and **demultiplexes by platform**. Each +incoming `GatewayEvent` carries a `platform` field (e.g. `"telegram"`, `"line"`, +`"feishu"`); the Receiver uses this to populate `InboundEvent.platform`. It does +NOT spawn per-platform receivers — there is one WS connection, one event loop, +producing `InboundEvent`s tagged with the correct platform. The Trust Gate then +routes the decision to the right platform's `TrustConfig`. + ```rust /// Unified inbound event produced by all Receivers. /// Contains the minimum fields needed for trust evaluation. @@ -360,6 +390,30 @@ Bot admission is NOT part of the identity trust model — it is platform-specifi structural logic (e.g. `trusted_bot_ids`, `allow_bot_messages`) that stays in the Handler. The Trust Gate only evaluates human sender identity. +**Implementation note:** The `is_bot` bypass is implemented at the **Trust Gate +caller level**, not inside `TrustConfig::decide()`. This keeps `decide()` a pure +L2+L3 function with no bot-awareness: + +```rust +// Trust Gate layer (pseudocode) +async fn gate_event(event: InboundEvent, configs: &PlatformTrustConfigs) -> Option { + // Bot bypass — skip L3 entirely; bot admission is Handler's job + if event.is_bot { + return Some(GatedEvent { inner: event }); + } + + let decision = configs.decide(&event.platform, &event.channel_id, event.is_dm, &event.sender_id); + match decision { + Decision::Allow => Some(GatedEvent { inner: event }), + Decision::DenyIdentity => { echo_sender_id(&event).await; None } + Decision::DenyScope => None, // silent drop + } +} +``` + +This means `PlatformTrustConfigs::decide()` does NOT need an `is_bot` parameter — +the bot check happens before `decide()` is called. + ### Echo reply on deny ```rust @@ -377,13 +431,30 @@ if decision == Decision::DenyIdentity { **Echo safeguards:** - **Rate-limit:** max 1 echo per sender per platform per 5 minutes (prevents spam/DoS amplification) - **Bot exclusion:** if `is_bot` → silent deny, no echo (prevents infinite reply loops between bots) -- **DM preferred:** in group/channel contexts, prefer DM reply to avoid leaking sender UID publicly; fall back to in-channel if DM unavailable +- **DM preferred:** in group/channel contexts, prefer DM reply to avoid leaking sender UID publicly; if DM unavailable, **silent drop** (do NOT fall back to in-channel echo, to avoid UID leakage in shared groups) - **Best-effort:** echo delivery is not guaranteed (e.g. LINE reply tokens expire); this is acceptable — the echo is a UX convenience, not a security mechanism **Platform-specific delivery caveats:** - **LINE:** reply tokens are single-use and short-TTL (~30s). If the echo cannot use the reply token, fall back to push message API (requires separate quota/permission). - **Other platforms:** no known delivery constraints for the echo use case. +### Sender ID format notes (for `allowed_users` configuration) + +`InboundEvent.sender_id` is always a `String`. Each platform's native ID is +converted to its string representation. Operators must configure `allowed_users` +using the **exact format** the platform provides in event payloads: + +| Platform | Native type | `allowed_users` format | Example | Gotcha | +|----------|-------------|----------------------|---------|--------| +| Discord | Snowflake (u64) | Numeric string | `"845835116920307722"` | — | +| Slack | String | U-prefix or W-prefix | `"U01ABCDEFGH"` | Enterprise Grid uses `W` prefix; use whichever the event payload provides | +| Telegram | Integer (i64) | Stringified integer | `"123456789"` | ⚠️ Do NOT use `@username` — only numeric ID works | +| LINE | String | U + 32 hex chars | `"U1234567890abcdef0123456789abcdef"` | — | +| Feishu | String | open_id | `"ou_xxxxxxxxxxxxxxxxxxxx"` | ⚠️ `open_id` is **per-app** — same user has different ID in different Feishu apps | +| WeCom | String | UserID | `"zhangsan"` | — | +| Google Chat | String | User resource name | `"users/123456789"` | — | +| MS Teams | String | `activity.from.id` | `"29:1abc..."` | Verify via actual event payload; may differ from AAD Object ID | + ## 6. Migration ### Phased rollout (not a hard cutover) From 67d20564dc77166ab508b7fb589f19e4e28a351f Mon Sep 17 00:00:00 2001 From: chaodu-agent Date: Sat, 4 Jul 2026 13:34:40 +0000 Subject: [PATCH 03/12] docs(adr): add event loop binding design + fix is_bot L2 bypass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add §5 'Event loop binding' section: run_platform generic pipeline, EventReceiver/EventHandler traits, main.rs startup wiring - Gateway platforms: one shared WS, demux by event.platform, fan-out to per-platform Handlers - Fix is_bot bypass: bots skip L3 but STILL enforce L2 scope (擺渡-1 🔴) - Add cross-crate boundary note for Gateway Receiver (擺渡-2 🟡) - Include binding topology summary diagram --- docs/adr/identity-trust-none.md | 198 +++++++++++++++++++++++++++++++- 1 file changed, 192 insertions(+), 6 deletions(-) diff --git a/docs/adr/identity-trust-none.md b/docs/adr/identity-trust-none.md index 3579a1477..f8c730684 100644 --- a/docs/adr/identity-trust-none.md +++ b/docs/adr/identity-trust-none.md @@ -247,7 +247,7 @@ events from multiple real platforms. │ On DenyScope: silent drop │ │ On Allow: pass event to Handler ↓ │ │ │ -│ Bot messages: if is_bot → skip L3 (bot admission stays in Handler) │ +│ Bot messages: if is_bot → skip L3, still enforce L2 scope │ │ │ │ 🔑 Architectural guarantee: Handler never receives untrusted events │ └──────────────────────────────────┬───────────────────────────────────────┘ @@ -286,6 +286,19 @@ NOT spawn per-platform receivers — there is one WS connection, one event loop, producing `InboundEvent`s tagged with the correct platform. The Trust Gate then routes the decision to the right platform's `TrustConfig`. +**Cross-crate boundary:** The Gateway Receiver is actually **two stages** across +two crates: +1. **`openab-gateway` (edge crate):** receives platform webhooks, performs L1 + authentication, normalizes to `GatewayEvent`, forwards over WebSocket. This + crate has NO dependency on `openab-core` and does NOT construct `InboundEvent`. +2. **`openab-core` (core crate):** receives `GatewayEvent` over WebSocket, + wraps it into `InboundEvent { raw: RawPlatformEvent::Gateway(gw_event) }`, + then feeds the Trust Gate. + +This means `InboundEvent` and `GatedEvent` types live in `openab-core` only. +The gateway crate never sees them — it only produces `GatewayEvent` (a simpler +struct defined in a shared types crate or serialized as JSON over the WS). + ```rust /// Unified inbound event produced by all Receivers. /// Contains the minimum fields needed for trust evaluation. @@ -396,15 +409,21 @@ L2+L3 function with no bot-awareness: ```rust // Trust Gate layer (pseudocode) -async fn gate_event(event: InboundEvent, configs: &PlatformTrustConfigs) -> Option { - // Bot bypass — skip L3 entirely; bot admission is Handler's job +async fn gate_event(event: &InboundEvent, configs: &PlatformTrustConfigs) -> Option { + // Bot bypass — skip L3 identity check, but STILL enforce L2 scope. + // Bots must respect channel/DM scope (noise/cost control) even though + // they don't need identity trust. if event.is_bot { - return Some(GatedEvent { inner: event }); + if !configs.surface_allowed(&event.platform, &event.channel_id, event.is_dm) { + return None; // DenyScope — bot in wrong channel, silent drop + } + return Some(GatedEvent { inner: event.clone() }); // L2 pass, skip L3 } + // Human sender — full L2 + L3 evaluation let decision = configs.decide(&event.platform, &event.channel_id, event.is_dm, &event.sender_id); match decision { - Decision::Allow => Some(GatedEvent { inner: event }), + Decision::Allow => Some(GatedEvent { inner: event.clone() }), Decision::DenyIdentity => { echo_sender_id(&event).await; None } Decision::DenyScope => None, // silent drop } @@ -412,7 +431,7 @@ async fn gate_event(event: InboundEvent, configs: &PlatformTrustConfigs) -> Opti ``` This means `PlatformTrustConfigs::decide()` does NOT need an `is_bot` parameter — -the bot check happens before `decide()` is called. +the bot check happens before `decide()` is called. Bots skip L3 but NOT L2. ### Echo reply on deny @@ -455,6 +474,173 @@ using the **exact format** the platform provides in event payloads: | Google Chat | String | User resource name | `"users/123456789"` | — | | MS Teams | String | `activity.from.id` | `"29:1abc..."` | Verify via actual event payload; may differ from AAD Object ID | +### Event loop binding (how platforms wire into the pipeline) + +The ingress pipeline is a **generic function** parameterized by Receiver and +Handler. Each platform spawns one `tokio::spawn` with this pipeline. The Trust +Gate is the same code for all platforms — only Receiver and Handler differ. + +```rust +/// Generic ingress pipeline — all platforms use this. +async fn run_platform(receiver: R, trust: Arc, handler: H) +where + R: EventReceiver, // trait: produces InboundEvent + H: EventHandler, // trait: consumes GatedEvent +{ + let mut rx = receiver.start().await; + + loop { + let event: InboundEvent = match rx.recv().await { + Some(e) => e, + None => break, // connection closed / shutdown + }; + + // 🔒 Trust Gate (unified for all platforms) + let gated = match gate_event(&event, &trust).await { + Some(g) => g, + None => continue, // denied + }; + + // Handler only receives GatedEvent (compile-time enforced) + handler.handle(gated).await; + } +} +``` + +**Traits:** + +```rust +#[async_trait] +trait EventReceiver { + /// Start listening, return a channel of normalized events. + async fn start(&self) -> mpsc::Receiver; +} + +#[async_trait] +trait EventHandler { + /// Process a trusted event (GatedEvent — can only come from Trust Gate). + async fn handle(&self, event: GatedEvent); +} +``` + +**Startup wiring (`main.rs`):** + +```rust +async fn main() { + let trust = Arc::new(PlatformTrustConfigs::from_config(&config)); + let dispatcher = Arc::new(Dispatcher::new(...)); + + // Discord — own WebSocket connection + if config.discord.enabled { + tokio::spawn(run_platform( + DiscordReceiver::new(config.discord.clone()), + trust.clone(), + DiscordHandler::new(config.discord, dispatcher.clone()), + )); + } + + // Slack — own Socket Mode connection + if config.slack.enabled { + tokio::spawn(run_platform( + SlackReceiver::new(config.slack.clone()), + trust.clone(), + SlackHandler::new(config.slack, dispatcher.clone()), + )); + } + + // Gateway platforms — ONE shared WebSocket, demux by platform + if config.has_gateway_platforms() { + tokio::spawn(run_gateway_platforms( + config.gateway_url.clone(), + trust.clone(), + dispatcher.clone(), + config.clone(), + )); + } + + // Cron — timer-based + if config.cron.enabled { + tokio::spawn(run_platform( + CronReceiver::new(config.cron.clone()), + trust.clone(), + CronHandler::new(config.cron, dispatcher.clone()), + )); + } +} +``` + +**Gateway platforms — one WebSocket, fan-out by platform:** + +Gateway-connected platforms (Telegram, LINE, Feishu, WeCom, Google Chat, Teams) +share a **single WebSocket** connection to `openab-gateway`. The gateway has +already performed L1 authentication and normalized events into `GatewayEvent`. +Core receives all events on one connection and **demuxes by `event.platform`**: + +```rust +async fn run_gateway_platforms( + url: String, + trust: Arc, + dispatcher: Arc, + config: Config, +) { + let mut ws = connect_to_gateway(&url).await; + + loop { + let gw_event: GatewayEvent = ws.recv().await; + + // Normalize GatewayEvent → InboundEvent + let event = InboundEvent { + platform: gw_event.platform.clone(), + sender_id: gw_event.sender_id.clone(), + channel_id: gw_event.channel_id.clone(), + is_dm: gw_event.is_dm, + is_bot: gw_event.is_bot, + raw: RawPlatformEvent::Gateway(gw_event.clone()), + }; + + // 🔒 Trust Gate (same logic, keyed by event.platform) + let gated = match gate_event(&event, &trust).await { + Some(g) => g, + None => continue, + }; + + // Fan-out to platform-specific Handler + match event.platform.as_str() { + "telegram" => telegram_handler.handle(gated).await, + "line" => line_handler.handle(gated).await, + "feishu" => feishu_handler.handle(gated).await, + "wecom" => wecom_handler.handle(gated).await, + "googlechat" => googlechat_handler.handle(gated).await, + "teams" => teams_handler.handle(gated).await, + unknown => warn!("unknown gateway platform: {unknown}"), + } + } +} +``` + +**Summary of binding topology:** + +``` +Discord → own WS → DiscordReceiver → 🔒 Trust Gate → DiscordHandler +Slack → own WS → SlackReceiver → 🔒 Trust Gate → SlackHandler +Gateway → one WS → GatewayReceiver → 🔒 Trust Gate → demux by platform + (shared) ├→ TelegramHandler + ├→ LineHandler + ├→ FeishuHandler + ├→ WecomHandler + ├→ GoogleChatHandler + └→ TeamsHandler +Cron → timer → CronReceiver → 🔒 Trust Gate → CronHandler +``` + +**Design choice — one WS for all gateway platforms (not per-platform):** +- Matches current architecture (minimal change) +- Gateway already normalizes all platform events into `GatewayEvent` +- One connection = one reconnect logic, one heartbeat, one backpressure +- Trust Gate uses `event.platform` to look up the correct per-platform config +- If the WS drops, all gateway platforms go offline together — acceptable, + same as current behavior, and gateway is designed for high availability + ## 6. Migration ### Phased rollout (not a hard cutover) From dd963b27d0acf999ebc0e775c5dd07b32ef77271 Mon Sep 17 00:00:00 2001 From: chaodu-agent Date: Sat, 4 Jul 2026 13:40:05 +0000 Subject: [PATCH 04/12] =?UTF-8?q?docs(adr):=20address=20round=203=20findin?= =?UTF-8?q?gs=20=E2=80=94=20tighten=20pseudocode=20precision?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - GatedEvent: private field in narrow module (not pub(crate)), with read-only accessors and module layout diagram (諸葛村夫-1) - gate_event: use configs.get().surface_allowed() to match real API (擺渡-3) - Phase table: add Phase 0.5 for current partially-wired state on main, clarify Phase 2 means 'refuse to start' (諸葛村夫-2) --- docs/adr/identity-trust-none.md | 52 ++++++++++++++++++++++++++------- 1 file changed, 42 insertions(+), 10 deletions(-) diff --git a/docs/adr/identity-trust-none.md b/docs/adr/identity-trust-none.md index f8c730684..286356cdb 100644 --- a/docs/adr/identity-trust-none.md +++ b/docs/adr/identity-trust-none.md @@ -176,21 +176,51 @@ calling convention. The Trust Gate consumes `InboundEvent` and produces a **different type** — `GatedEvent` — which is the only type Handler accepts: ```rust +// In crate::trust::gate (narrow module — only Trust Gate code lives here) + /// Receiver produces this (untrusted). pub struct InboundEvent { /* ... */ } -/// Trust Gate produces this (trusted). Only constructible by the gate. +/// Trust Gate produces this (trusted). +/// The inner field is PRIVATE (not pub(crate)) — only code in this module +/// can construct it. This makes bypass impossible even within openab-core. pub struct GatedEvent { - pub(crate) inner: InboundEvent, // pub(crate) — Handler cannot forge this + inner: InboundEvent, // private — only gate module can construct +} + +impl GatedEvent { + /// Read-only access to the trusted event data. + pub fn event(&self) -> &InboundEvent { &self.inner } + pub fn platform(&self) -> &str { &self.inner.platform } + pub fn sender_id(&self) -> &str { &self.inner.sender_id } + pub fn channel_id(&self) -> &str { &self.inner.channel_id } + pub fn is_dm(&self) -> bool { self.inner.is_dm } + pub fn into_inner(self) -> InboundEvent { self.inner } +} + +/// Only constructible within this module (the Trust Gate). +/// No other module in the crate can call this. +pub(super) fn seal(event: InboundEvent) -> GatedEvent { + GatedEvent { inner: event } } /// Handler signature — cannot accept InboundEvent directly. +/// Cannot forge GatedEvent (private field, no public constructor). async fn handle(&self, event: GatedEvent) { /* ... */ } ``` +**Module layout for enforcement:** +``` +crate::trust +├── mod.rs // PlatformTrustConfigs, TrustConfig, Decision (public) +├── gate.rs // gate_event() + GatedEvent (constructor is pub(super)) +└── (no other module can construct GatedEvent) +``` + A Handler that tries to accept `InboundEvent` directly will not compile. -A Receiver that tries to construct `GatedEvent` will fail (private field). -This makes the bypass impossible at compile time, not just by convention. +Any module outside `crate::trust` that tries to construct `GatedEvent` will fail +(private field, no public constructor). This makes the bypass impossible at +compile time — not `pub(crate)`, but truly module-private. **Trust lookup key:** The gate uses the **per-event platform** from `InboundEvent.platform` (which maps to `ChannelRef.platform`), NOT @@ -408,22 +438,23 @@ caller level**, not inside `TrustConfig::decide()`. This keeps `decide()` a pure L2+L3 function with no bot-awareness: ```rust -// Trust Gate layer (pseudocode) +// Trust Gate layer (pseudocode) — lives in crate::trust::gate async fn gate_event(event: &InboundEvent, configs: &PlatformTrustConfigs) -> Option { // Bot bypass — skip L3 identity check, but STILL enforce L2 scope. // Bots must respect channel/DM scope (noise/cost control) even though // they don't need identity trust. if event.is_bot { - if !configs.surface_allowed(&event.platform, &event.channel_id, event.is_dm) { + let config = configs.get(&event.platform); + if !config.surface_allowed(&event.channel_id, event.is_dm) { return None; // DenyScope — bot in wrong channel, silent drop } - return Some(GatedEvent { inner: event.clone() }); // L2 pass, skip L3 + return Some(seal(event.clone())); // L2 pass, skip L3 } // Human sender — full L2 + L3 evaluation let decision = configs.decide(&event.platform, &event.channel_id, event.is_dm, &event.sender_id); match decision { - Decision::Allow => Some(GatedEvent { inner: event.clone() }), + Decision::Allow => Some(seal(event.clone())), Decision::DenyIdentity => { echo_sender_id(&event).await; None } Decision::DenyScope => None, // silent drop } @@ -650,8 +681,9 @@ The default flip is phased to avoid silently severing live bots on upgrade: | Phase | Behavior | When | |-------|----------|------| | **Phase 0** | Types + `decide()` defined, no runtime behavior change. Additive only. | Done (on main) | -| **Phase 1** | Wire Trust Gate into ingress pipeline. **Keep current allow-all default.** Log deprecation warning when relying on implicit allow-all. | Next release | -| **Phase 2** | Require explicit `allow_all_users = true` to preserve old behavior. Deployments without it get a **startup error** (not silent denial). | Pre-GA release | +| **Phase 0.5** | Partial wiring: `AdapterRouter::gate_incoming`, gateway unified path gating, Discord L3 gate — using existing adapter structure (not yet the Receiver/Handler split). Trust checks co-exist with scattered per-adapter checks during transition. | Done (on main) | +| **Phase 1** | Complete Receiver/Handler split. Wire Trust Gate as the sole ingress layer. **Keep current allow-all default.** Log deprecation warning when relying on implicit allow-all. Remove scattered checks. | Next release | +| **Phase 2** | Require explicit `allow_all_users = true` to preserve old behavior. Deployments without it get a **startup error** (not silent denial — bot refuses to start, operator must explicitly choose). | Pre-GA release | | **Phase 3** | Flip default: empty `allowed_users` + no `allow_all_users` = **deny-all**. | GA release | ### Migration path From 2582ece417022f24df5e6b74e1781454efe99bad Mon Sep 17 00:00:00 2001 From: chaodu-agent Date: Sat, 4 Jul 2026 13:41:26 +0000 Subject: [PATCH 05/12] =?UTF-8?q?docs(adr):=20fix=20seal()=20visibility=20?= =?UTF-8?q?=E2=80=94=20private=20fn,=20not=20pub(super)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit seal() lives in the same module as gate_event(), so it should be a plain private fn. pub(super) would unnecessarily expose it to the parent module. --- docs/adr/identity-trust-none.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/adr/identity-trust-none.md b/docs/adr/identity-trust-none.md index 286356cdb..b0ab44c7c 100644 --- a/docs/adr/identity-trust-none.md +++ b/docs/adr/identity-trust-none.md @@ -200,7 +200,7 @@ impl GatedEvent { /// Only constructible within this module (the Trust Gate). /// No other module in the crate can call this. -pub(super) fn seal(event: InboundEvent) -> GatedEvent { +fn seal(event: InboundEvent) -> GatedEvent { GatedEvent { inner: event } } From 31fe1785a79f2a86780e86d3ed849cff14f2a691 Mon Sep 17 00:00:00 2001 From: chaodu-agent Date: Sat, 4 Jul 2026 16:11:07 +0000 Subject: [PATCH 06/12] =?UTF-8?q?docs(adr):=20fix=20stale=20module=20layou?= =?UTF-8?q?t=20comment=20=E2=80=94=20constructor=20is=20private,=20not=20p?= =?UTF-8?q?ub(super)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/adr/identity-trust-none.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/adr/identity-trust-none.md b/docs/adr/identity-trust-none.md index b0ab44c7c..b80119778 100644 --- a/docs/adr/identity-trust-none.md +++ b/docs/adr/identity-trust-none.md @@ -213,7 +213,7 @@ async fn handle(&self, event: GatedEvent) { /* ... */ } ``` crate::trust ├── mod.rs // PlatformTrustConfigs, TrustConfig, Decision (public) -├── gate.rs // gate_event() + GatedEvent (constructor is pub(super)) +├── gate.rs // gate_event() + GatedEvent (constructor is private) └── (no other module can construct GatedEvent) ``` From d5c420c0510c6963e6ad7e2b42b7d0b562ceba79 Mon Sep 17 00:00:00 2001 From: chaodu-agent Date: Mon, 6 Jul 2026 12:42:17 -0400 Subject: [PATCH 07/12] docs(adr): address LINE/Slack/Feishu review feedback - Replace line-number refs with symbol+semantic descriptions (drift-proof) - Rewrite echo section: platform-specific echo trait (LINE=Reply only, Slack=chat.postEphemeral, Discord=DM); leak-safe content by scope - Add is_bot per-platform derivation table (pinned canonical rules) - Document trusted_bot_ids as shared config (resolves Feishu circular dep) - Clarify slash commands scope (Slack doesn't consume them) - Update Slack sender ID: Enterprise Grid composite key (team_id, sender_id) - Add non-message events section (assistant_thread_started must gate) - Add Slack scope notes (Socket Mode only, MPIM=channel) - Add LINE group policy: open/members dual-mode in decide() - Add LINE @mention pre-filter as documented Receiver exception - Feishu: gateway=L1 only, eliminate double-gating, empty list=deny-all Addresses feedback from: - @luffy-aiagent (LINE platform review) - @antigenius0910 (Slack platform review) - @wangyuyan-agent (Feishu platform review) --- docs/adr/identity-trust-none.md | 151 +++++++++++++++++++++++++++++--- 1 file changed, 139 insertions(+), 12 deletions(-) diff --git a/docs/adr/identity-trust-none.md b/docs/adr/identity-trust-none.md index b80119778..baa63492b 100644 --- a/docs/adr/identity-trust-none.md +++ b/docs/adr/identity-trust-none.md @@ -256,6 +256,12 @@ events from multiple real platforms. │ • ❌ Check allowed_users │ │ • ❌ Handle slash commands │ │ • ❌ Evaluate @mention / multibot / role logic │ +│ │ +│ EXCEPTION — LINE @mention pre-filter: │ +│ • LINE Receiver drops non-@mention group messages BEFORE producing │ +│ InboundEvent (prevents ordinary chatter from hitting Trust Gate │ +│ and triggering deny-echo spam). This is a deliberate exception to │ +│ "Receiver = pure transport." │ └──────────────────────────────────┬───────────────────────────────────────┘ │ │ InboundEvent (unified format) @@ -392,6 +398,42 @@ pub enum Decision { } ``` +### LINE group policy (platform-specific extension to `decide()`) + +LINE groups have a platform-specific challenge: `sender_id` may be `"unknown"` +when a user's privacy settings prevent LINE from revealing their identity. To +handle this, LINE's trust config extends the base `TrustConfig` with a per-group +policy: + +```toml +[line] +allowed_users = ["Uaaa", "Ubbb"] +default_group_policy = "members" # fail-closed default + +[[line.groups]] +id = "C1234567890" +policy = "open" # group-level trust: any @mention sender is allowed + # "unknown" senders permitted; audit at group level only + +[[line.groups]] +id = "C0987654321" +policy = "members" # per-user trust: must be in allowed_users + # "unknown" senders denied; full per-user audit +``` + +**Decision logic for LINE (extends base `decide()`):** +- **1:1 DM:** `sender_id == "unknown"` → Deny; otherwise check `allowed_users` +- **Group not in configured groups** → DenyScope (not in scope) +- **Group `policy = "open"`** → Allow (group-level trust — `"unknown"` permitted) +- **Group `policy = "members"`** → `sender_id == "unknown"` → Deny; otherwise + check `allowed_users` + +**Operator tradeoff (documented):** choosing `"open"` means accepting that: +1. Anonymous/unidentifiable users can use the bot in that group +2. Audit logs only reach group level (no per-user tracking) + +If per-user audit is required, use `"members"` policy. + ### PlatformTrustConfigs (registry) ```rust @@ -433,6 +475,29 @@ Bot admission is NOT part of the identity trust model — it is platform-specifi structural logic (e.g. `trusted_bot_ids`, `allow_bot_messages`) that stays in the Handler. The Trust Gate only evaluates human sender identity. +**`is_bot` per-platform derivation (pinned — each Receiver must use these rules):** + +| Platform | `is_bot` derivation | Notes | +|----------|-------------------|-------| +| Discord | `message.author.bot` flag | Native field from Discord API | +| Slack | `event.bot_id.is_some() \|\| event.subtype == "bot_message"` | Plus `USLACKBOT` always treated as bot | +| LINE | Always `false` | LINE has no bot-to-bot webhook delivery; bot-bypass is a no-op | +| Feishu | `trusted_bot_ids.contains(sender_open_id)` | Feishu marks other bots as `sender_type="user"` — unreliable; must match against known bot IDs | +| Telegram | `message.from.is_bot` flag | Native field from Telegram API | + +**`trusted_bot_ids` is shared config (NOT Handler-only):** + +`trusted_bot_ids` is readable by **all layers** with different purposes: +- **Receiver:** reads the list to compute `InboundEvent.is_bot` (especially for + Feishu where platform signals are unreliable) +- **Trust Gate:** reads `is_bot` flag to bypass L3 (does not need the list itself) +- **Handler:** reads the list for bot admission (which specific bots are allowed + to trigger the agent vs. ignored) + +This resolves the Feishu circular dependency: the Receiver can compute `is_bot` +because it has access to the config, and the Handler independently uses the same +config for admission decisions. + **Implementation note:** The `is_bot` bypass is implemented at the **Trust Gate caller level**, not inside `TrustConfig::decide()`. This keeps `decide()` a pure L2+L3 function with no bot-awareness: @@ -481,12 +546,29 @@ if decision == Decision::DenyIdentity { **Echo safeguards:** - **Rate-limit:** max 1 echo per sender per platform per 5 minutes (prevents spam/DoS amplification) - **Bot exclusion:** if `is_bot` → silent deny, no echo (prevents infinite reply loops between bots) -- **DM preferred:** in group/channel contexts, prefer DM reply to avoid leaking sender UID publicly; if DM unavailable, **silent drop** (do NOT fall back to in-channel echo, to avoid UID leakage in shared groups) -- **Best-effort:** echo delivery is not guaranteed (e.g. LINE reply tokens expire); this is acceptable — the echo is a UX convenience, not a security mechanism +- **Best-effort:** echo delivery is not guaranteed; this is acceptable — the echo is a UX convenience, not a security mechanism + +**Platform-specific echo delivery (via platform echo trait):** + +The Trust Gate makes the **decision** (Allow/Deny) in core. Actual echo delivery +is delegated to a platform-specific **echo trait implementation** — core never +calls platform APIs directly. + +| Platform | Echo mechanism | Rationale | +|----------|---------------|-----------| +| Discord | DM to user | Discord guarantees DM delivery | +| Slack | `chat.postEphemeral` in-channel | Only needs `chat:write` scope; no `im:write` required; visible only to target user; no UID leak | +| LINE | Reply API only; silent drop if token expired | **Never** use Push API for deny-echo (prevents attackers from burning paid Push quota) | +| Telegram | Reply in-chat | Standard reply | +| Feishu | Reply in-chat | Standard reply | + +**Echo content by scope (leak-safe):** +- **DM / 1:1 context:** echo includes sender UID (self-serve — user forwards to admin to request access) +- **Group / channel context:** echo carries **no sender ID** — generic "not authorized, contact admin" only (prevents leaking identity in shared spaces) -**Platform-specific delivery caveats:** -- **LINE:** reply tokens are single-use and short-TTL (~30s). If the echo cannot use the reply token, fall back to push message API (requires separate quota/permission). -- **Other platforms:** no known delivery constraints for the echo use case. +**LINE-specific invariant:** LINE deny-echo uses Reply API only. If the reply +token is expired (~50s TTL) or already consumed, the echo is **silently dropped**. +Push API is never used for deny-echo — this is non-overridable (not a config knob). ### Sender ID format notes (for `allowed_users` configuration) @@ -497,7 +579,7 @@ using the **exact format** the platform provides in event payloads: | Platform | Native type | `allowed_users` format | Example | Gotcha | |----------|-------------|----------------------|---------|--------| | Discord | Snowflake (u64) | Numeric string | `"845835116920307722"` | — | -| Slack | String | U-prefix or W-prefix | `"U01ABCDEFGH"` | Enterprise Grid uses `W` prefix; use whichever the event payload provides | +| Slack | String | U-prefix or W-prefix | `"U01ABCDEFGH"` | Enterprise Grid: use `enterprise_user.id` (stable across workspaces) when available; trust key = `(team_id, sender_id)` for Grid deployments | | Telegram | Integer (i64) | Stringified integer | `"123456789"` | ⚠️ Do NOT use `@username` — only numeric ID works | | LINE | String | U + 32 hex chars | `"U1234567890abcdef0123456789abcdef"` | — | | Feishu | String | open_id | `"ou_xxxxxxxxxxxxxxxxxxxx"` | ⚠️ `open_id` is **per-app** — same user has different ID in different Feishu apps | @@ -724,12 +806,24 @@ functional. - Passes allowed events to Handler - Echoes + drops denied events 4. **Remove scattered trust checks** — replaced by the unified Trust Gate: - - `is_denied_user()` in Discord EventHandler (`discord.rs:2892`) - - `should_skip_event()` user/channel filter in `gateway.rs` (`:832`, `:1160`) - - Inline user allowlist in Slack (`slack.rs:1224`) - - Feishu L3 check in the gateway crate (`feishu.rs:425`) — must relocate to - core, not just delete (contradicts "gateway = L1 only" model) - - Discord reaction-dispatch gating (`discord.rs:1241`) + - `is_denied_user()` call sites in Discord `EventHandler` (forum-post, DM, + and guild-message paths) + - `should_skip_event()` call sites in `run_gateway_adapter` and + `process_gateway_event` (gateway.rs) + - Inline `allowed_users` check in Slack `should_process_message()` + - `allowed_users` + `allowed_groups` filters in Feishu `parse_message_event()` + (gateway crate) — must relocate to core Trust Gate, not just delete + (contradicts "gateway = L1 only" model) + **Feishu double-gating elimination:** Currently Feishu identity is checked + twice — `FEISHU_ALLOWED_USERS`/`FEISHU_ALLOWED_GROUPS` in the gateway crate + AND `[gateway].allowed_users` in core. These can diverge, and the core side + **fails open** when its list is empty (`resolve_allow_all = list.is_empty()`). + Resolution: gateway crate performs L1 only (signature + decrypt); all + `allowed_users` / `allowed_groups` checks move to core Trust Gate. Empty + list = **deny-all** (requires explicit `allow_all_users = true` to open). + Gateway env vars (`FEISHU_ALLOWED_USERS`, `FEISHU_ALLOWED_GROUPS`) are + deprecated in Phase 1 (warn), conflict-error in Phase 2, removed in Phase 3. + - Discord reaction-dispatch gating in `EventHandler` - Note: `trusted_bot_ids`, `allow_bot_messages`, `allowed_role_ids` **stay in Handlers** — they are structural/trigger semantics, not identity trust. 5. **Add echo reply with safeguards** — rate-limit, bot exclusion, DM-preferred @@ -747,6 +841,39 @@ These are platform-specific structural concerns, not trust: - Reaction dispatch gating (triggers, not authorization) - Slash command routing (`/reset`, `/cancel`) — but note these now run AFTER the Trust Gate, so untrusted senders cannot invoke them. + **Scope note:** "slash commands are gated" applies to **gateway-platform + commands** (Telegram `/reset`, `/cancel`) implemented as text-prefix detection + in the Handler. The **Slack adapter does not consume `slash_commands` or + `interactive` envelopes** — thread routing cannot be reconstructed for them. + If Slack slash command support is added in the future, those events must flow + through the full Receiver → Trust Gate → Handler pipeline. + +### Non-message events that MUST flow through the Trust Gate + +Any event type that can **trigger agent state changes** must flow through the +full Receiver → Trust Gate → Handler pipeline. The Trust Gate is not limited to +`message` events: + +| Event | Platform | Must gate? | `InboundEvent` mapping | +|-------|----------|-----------|----------------------| +| `assistant_thread_started` | Slack | **Yes** — untrusted user can establish agent state | `is_dm: true`, `sender_id: event.user` | +| `assistant_thread_context_changed` | Slack | **Yes** — modifies thread context | `is_dm: true`, `sender_id: event.user` | +| `reaction_added` | Slack | Not implemented today | Future: reactor identity (`event.user`) must pass L3 if reactions trigger agent actions | +| Message events | All | **Yes** | Standard mapping | + +**Rule:** if a new event type is added to any Receiver and it can cause the agent +to execute, store state, or respond, its sender identity **must** pass through L3. +Events that are purely informational (e.g. typing indicators) may be excluded. + +### Slack-specific scope notes + +- **L1 authentication:** Slack L1 = Socket Mode `app_token` verification. Events + API HTTP mode (`X-Slack-Signature` HMAC) is out of scope for this ADR. +- **MPIM classification:** `D`-prefix channels = DM (`is_dm = true`). `G`-prefix + channels (MPIM / group DMs) are treated as **channels** (`is_dm = false`) — they + behave more like channels (multiple participants, shared context). If operators + need MPIM-as-DM semantics, a future enhancement can add `conversations.info` + lookup. ## 8. Rejected Alternatives From dcf42649968e2196afd7d2b03d6299aa3e9692ba Mon Sep 17 00:00:00 2001 From: chaodu-agent Date: Mon, 6 Jul 2026 17:06:46 +0000 Subject: [PATCH 08/12] =?UTF-8?q?docs(adr):=20address=209=20review=20findi?= =?UTF-8?q?ngs=20=E2=80=94=20API=20contract=20gaps=20+=20hardening?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes identified during group review: F1+F6: Add workspace_id to InboundEvent; define Slack Enterprise Grid canonical sender_id format and config examples for Grid deployments F2: Replace HashMap with enum PlatformTrustConfig (Base/Line/Slack) — LINE group policy and Slack workspace-scoped trust now have proper type representations F3: Add cron bypass in gate_event() — system-initiated events skip L2/L3 (platform='cron' or sender_id='openab-cron') F4: Add #[cfg(test)] assume_trusted_for_test() constructor for GatedEvent — enables Handler unit testing without full pipeline F5+F9: Change into_inner() to pub(crate); adjust safety claim wording from 'bypass impossible' to 'accidental bypass compile error' F7: Change gate_event() signature to take InboundEvent by value — zero-copy hot path (no .clone() on RawPlatformEvent) F8: Specify bounded LRU cache (max_capacity + TTL) for rate-limit state — prevents OOM from random sender_id flooding --- docs/adr/identity-trust-none.md | 244 ++++++++++++++++++++++++++++---- 1 file changed, 213 insertions(+), 31 deletions(-) diff --git a/docs/adr/identity-trust-none.md b/docs/adr/identity-trust-none.md index baa63492b..30f954e7a 100644 --- a/docs/adr/identity-trust-none.md +++ b/docs/adr/identity-trust-none.md @@ -170,10 +170,12 @@ Each adapter is split into two components with the Trust Gate in between: - No adapter can bypass the gate — it is architecturally mandatory - New platform = write Receiver + Handler; trust is automatic -**Type-level enforcement (compile-time guarantee):** -The "impossible to bypass" property is enforced via Rust's type system, not just -calling convention. The Trust Gate consumes `InboundEvent` and produces a -**different type** — `GatedEvent` — which is the only type Handler accepts: +**Type-level enforcement (compile-time hardening):** +The private constructor makes *accidental* bypass a compile error. The Trust Gate +consumes `InboundEvent` and produces a **different type** — `GatedEvent` — which +is the only type Handler accepts. No code outside `crate::trust::gate` can +construct `GatedEvent`, ensuring untrusted events cannot reach Handlers through +normal code paths: ```rust // In crate::trust::gate (narrow module — only Trust Gate code lives here) @@ -183,7 +185,7 @@ pub struct InboundEvent { /* ... */ } /// Trust Gate produces this (trusted). /// The inner field is PRIVATE (not pub(crate)) — only code in this module -/// can construct it. This makes bypass impossible even within openab-core. +/// can construct it. This makes accidental bypass a compile error. pub struct GatedEvent { inner: InboundEvent, // private — only gate module can construct } @@ -195,7 +197,19 @@ impl GatedEvent { pub fn sender_id(&self) -> &str { &self.inner.sender_id } pub fn channel_id(&self) -> &str { &self.inner.channel_id } pub fn is_dm(&self) -> bool { self.inner.is_dm } - pub fn into_inner(self) -> InboundEvent { self.inner } + + /// Consuming unwrap — restricted to `pub(crate)` to minimize escape paths. + /// Only used by the Dispatcher when it needs ownership of the inner event + /// for session creation. Handlers should use read-only accessors above. + pub(crate) fn into_inner(self) -> InboundEvent { self.inner } + + /// Test-only constructor — allows Handler unit tests to create GatedEvent + /// without wiring the full Trust Gate pipeline. Excluded from production + /// binaries via cfg(test). + #[cfg(test)] + pub(crate) fn assume_trusted_for_test(event: InboundEvent) -> Self { + Self { inner: event } + } } /// Only constructible within this module (the Trust Gate). @@ -219,8 +233,10 @@ crate::trust A Handler that tries to accept `InboundEvent` directly will not compile. Any module outside `crate::trust` that tries to construct `GatedEvent` will fail -(private field, no public constructor). This makes the bypass impossible at -compile time — not `pub(crate)`, but truly module-private. +(private field, no public constructor). This makes accidental bypass a compile +error — the type system enforces the trust boundary structurally. The consuming +unwrap (`into_inner`) is intentionally `pub(crate)` to minimize escape paths +while allowing the Dispatcher to take ownership when creating sessions. **Trust lookup key:** The gate uses the **per-event platform** from `InboundEvent.platform` (which maps to `ChannelRef.platform`), NOT @@ -342,12 +358,18 @@ pub struct InboundEvent { pub platform: String, // "discord", "telegram", "line", etc. pub sender_id: String, // platform-specific sender identifier pub channel_id: String, // conversation surface + pub workspace_id: Option, // workspace/team context (Slack Enterprise Grid: team_id) pub is_dm: bool, // DM vs group/channel pub is_bot: bool, // bot-originated message pub raw: RawPlatformEvent, // opaque; Handler interprets this } ``` +**`workspace_id` usage:** Only populated when the platform has multi-workspace +semantics (currently: Slack Enterprise Grid). Used by `PlatformTrustConfigs::decide()` +to scope trust lookups and by the echo rate-limiter to key per-workspace. For +single-workspace Slack apps and all other platforms, this is `None`. + ### Per-platform TrustConfig ```rust @@ -437,18 +459,60 @@ If per-user audit is required, use `"members"` policy. ### PlatformTrustConfigs (registry) ```rust +/// Platform-specific trust config — supports base config and platform extensions. +pub enum PlatformTrustConfig { + /// Standard config (Discord, Telegram, Feishu, WeCom, Google Chat, Teams). + Base(TrustConfig), + /// LINE — extends base with group policy (open/members/unknown handling). + Line(LineTrustConfig), + /// Slack — extends base with workspace-scoped trust for Enterprise Grid. + Slack(SlackTrustConfig), +} + +/// LINE-specific trust config with per-group policy. +pub struct LineTrustConfig { + pub base: TrustConfig, + pub default_group_policy: GroupPolicy, // "members" (fail-closed default) + pub groups: HashMap, // group_id → policy +} + +/// Slack-specific trust config with workspace-scoped identity. +pub struct SlackTrustConfig { + pub base: TrustConfig, + /// For Enterprise Grid: per-workspace allowed_users override. + /// If empty, falls back to base.allowed_users (single-workspace mode). + pub workspace_users: HashMap>, // team_id → allowed user IDs +} + +#[derive(Clone, Copy, PartialEq)] +pub enum GroupPolicy { + Open, // group-level trust — "unknown" senders permitted + Members, // per-user trust — "unknown" senders denied +} + pub struct PlatformTrustConfigs { - configs: HashMap, // keyed by lowercase platform name + configs: HashMap, // keyed by lowercase platform name } impl PlatformTrustConfigs { - /// Look up by per-event platform (case-insensitive). - /// Unknown platform → default config (L2 open, L3 deny-all). - pub fn decide(&self, platform: &str, channel_id: &str, is_dm: bool, sender_id: &str) -> Decision { + /// Main decision entry point. Dispatches to platform-specific logic. + pub fn decide( + &self, + platform: &str, + channel_id: &str, + is_dm: bool, + sender_id: &str, + workspace_id: Option<&str>, + ) -> Decision { let config = self.configs .get(&platform.to_lowercase()) - .unwrap_or(&Self::default_config()); - config.decide(channel_id, is_dm, sender_id) + .unwrap_or(&PlatformTrustConfig::Base(Self::default_config())); + + match config { + PlatformTrustConfig::Base(c) => c.decide(channel_id, is_dm, sender_id), + PlatformTrustConfig::Line(c) => c.decide(channel_id, is_dm, sender_id), + PlatformTrustConfig::Slack(c) => c.decide(channel_id, is_dm, sender_id, workspace_id), + } } fn default_config() -> TrustConfig { @@ -462,11 +526,100 @@ impl PlatformTrustConfigs { } } } + +impl LineTrustConfig { + pub fn decide(&self, channel_id: &str, is_dm: bool, sender_id: &str) -> Decision { + // L2 scope check first (same as base) + if !self.base.surface_allowed(channel_id, is_dm) { + return Decision::DenyScope; + } + + // 1:1 DM — standard identity check, but "unknown" always denied + if is_dm { + if sender_id == "unknown" || sender_id.is_empty() { + return Decision::DenyIdentity; + } + return if self.base.identity_allowed(sender_id) { + Decision::Allow + } else { + Decision::DenyIdentity + }; + } + + // Group — look up per-group policy + let policy = self.groups + .get(channel_id) + .copied() + .unwrap_or(self.default_group_policy); + + match policy { + GroupPolicy::Open => Decision::Allow, // group-level trust, "unknown" permitted + GroupPolicy::Members => { + if sender_id == "unknown" || sender_id.is_empty() { + Decision::DenyIdentity + } else if self.base.identity_allowed(sender_id) { + Decision::Allow + } else { + Decision::DenyIdentity + } + } + } + } +} + +impl SlackTrustConfig { + pub fn decide( + &self, + channel_id: &str, + is_dm: bool, + sender_id: &str, + workspace_id: Option<&str>, + ) -> Decision { + // L2 scope check + if !self.base.surface_allowed(channel_id, is_dm) { + return Decision::DenyScope; + } + + // L3 identity — workspace-scoped for Enterprise Grid + if sender_id.is_empty() { return Decision::DenyIdentity; } + + // If workspace_users is configured and we have a workspace_id, + // check workspace-scoped allowlist first. + if let Some(ws_id) = workspace_id { + if let Some(ws_users) = self.workspace_users.get(ws_id) { + return if ws_users.contains(sender_id) || self.base.allow_all_users { + Decision::Allow + } else { + Decision::DenyIdentity + }; + } + } + + // Fallback to base config (single-workspace or unscoped) + if self.base.identity_allowed(sender_id) { + Decision::Allow + } else { + Decision::DenyIdentity + } + } +} ``` -Note: The default config uses a runtime-constructed `TrustConfig` (not a `static` -with `HashSet::new()` which would not compile). The actual implementation uses -`LazyLock` or returns a fresh default; see `trust.rs` on main for the real code. +**Slack Enterprise Grid config example:** + +```toml +# Single-workspace Slack (most deployments): +[slack] +allowed_users = ["U01ABCDEFGH", "U09XYZWVUTS"] + +# Enterprise Grid — workspace-scoped (optional): +[slack] +allowed_users = ["E0123456789"] # enterprise_user.id (cross-workspace) + +[slack.workspace_users] +T012345 = ["U01ABCDEFGH", "U01IJKLMNOP"] # workspace-specific overrides +T067890 = ["U09XYZWVUTS"] +``` ### Bot message handling @@ -504,7 +657,15 @@ L2+L3 function with no bot-awareness: ```rust // Trust Gate layer (pseudocode) — lives in crate::trust::gate -async fn gate_event(event: &InboundEvent, configs: &PlatformTrustConfigs) -> Option { +async fn gate_event(event: InboundEvent, configs: &PlatformTrustConfigs) -> Option { + // System-initiated events (e.g., Cron) bypass trust entirely. + // Cron is internal — it has no external sender and targets arbitrary + // platform channels. Gating it against platform L2 scope would incorrectly + // block scheduled jobs to channels not in the human-facing allowlist. + if event.platform == "cron" || event.sender_id == "openab-cron" { + return Some(seal(event)); // fully trusted, no L2/L3 + } + // Bot bypass — skip L3 identity check, but STILL enforce L2 scope. // Bots must respect channel/DM scope (noise/cost control) even though // they don't need identity trust. @@ -513,21 +674,36 @@ async fn gate_event(event: &InboundEvent, configs: &PlatformTrustConfigs) -> Opt if !config.surface_allowed(&event.channel_id, event.is_dm) { return None; // DenyScope — bot in wrong channel, silent drop } - return Some(seal(event.clone())); // L2 pass, skip L3 + return Some(seal(event)); // L2 pass, skip L3 } // Human sender — full L2 + L3 evaluation - let decision = configs.decide(&event.platform, &event.channel_id, event.is_dm, &event.sender_id); + let decision = configs.decide( + &event.platform, + &event.channel_id, + event.is_dm, + &event.sender_id, + event.workspace_id.as_deref(), + ); match decision { - Decision::Allow => Some(seal(event.clone())), + Decision::Allow => Some(seal(event)), Decision::DenyIdentity => { echo_sender_id(&event).await; None } Decision::DenyScope => None, // silent drop } } ``` -This means `PlatformTrustConfigs::decide()` does NOT need an `is_bot` parameter — -the bot check happens before `decide()` is called. Bots skip L3 but NOT L2. +**Note on ownership:** `gate_event` takes `InboundEvent` **by value** (ownership +transfer). On `Allow`, the event is moved into `GatedEvent` with zero copying. +On `Deny`, the event is dropped. This avoids deep-cloning `RawPlatformEvent` +(which may contain the full platform payload) on the hot path. + +**Cron bypass rationale:** Cron tasks are system-initiated (no external sender). +They use `platform = "cron"` and `sender_id = "openab-cron"` — a reserved +synthetic identity that cannot collide with any real platform sender ID (real +platform IDs are alphanumeric, never prefixed with `openab-`). The Cron Receiver +sets these fields; the Trust Gate recognizes them and short-circuits. The CronHandler +then routes the dispatched event to the appropriate target platform/channel. ### Echo reply on deny @@ -545,6 +721,11 @@ if decision == Decision::DenyIdentity { **Echo safeguards:** - **Rate-limit:** max 1 echo per sender per platform per 5 minutes (prevents spam/DoS amplification) +- **Bounded state:** rate-limit cache MUST be bounded (e.g., LRU with max 10,000 + entries + 5-minute TTL eviction). An unbounded `HashMap` + would allow attackers to OOM the process by sending messages with random/spoofed + sender IDs. Implementation: use a TTL-bounded LRU cache (e.g., `moka::sync::Cache` + or equivalent) with `max_capacity` and `time_to_live` configured. - **Bot exclusion:** if `is_bot` → silent deny, no echo (prevents infinite reply loops between bots) - **Best-effort:** echo delivery is not guaranteed; this is acceptable — the echo is a UX convenience, not a security mechanism @@ -579,7 +760,7 @@ using the **exact format** the platform provides in event payloads: | Platform | Native type | `allowed_users` format | Example | Gotcha | |----------|-------------|----------------------|---------|--------| | Discord | Snowflake (u64) | Numeric string | `"845835116920307722"` | — | -| Slack | String | U-prefix or W-prefix | `"U01ABCDEFGH"` | Enterprise Grid: use `enterprise_user.id` (stable across workspaces) when available; trust key = `(team_id, sender_id)` for Grid deployments | +| Slack | String | U-prefix or W-prefix | `"U01ABCDEFGH"` | Enterprise Grid: Receiver MUST set `workspace_id = Some(team_id)` and prefer `enterprise_user.id` as `sender_id` when available (stable across workspaces). Config format for Grid: `allowed_users = ["E0123456789"]` (enterprise_user.id) or `"T012345:U01ABCDEFGH"` (team_id:user_id fallback). Trust key = `(workspace_id, sender_id)` — see `decide()` below. | | Telegram | Integer (i64) | Stringified integer | `"123456789"` | ⚠️ Do NOT use `@username` — only numeric ID works | | LINE | String | U + 32 hex chars | `"U1234567890abcdef0123456789abcdef"` | — | | Feishu | String | open_id | `"ou_xxxxxxxxxxxxxxxxxxxx"` | ⚠️ `open_id` is **per-app** — same user has different ID in different Feishu apps | @@ -608,10 +789,10 @@ where None => break, // connection closed / shutdown }; - // 🔒 Trust Gate (unified for all platforms) - let gated = match gate_event(&event, &trust).await { + // 🔒 Trust Gate (unified for all platforms) — takes ownership of event + let gated = match gate_event(event, &trust).await { Some(g) => g, - None => continue, // denied + None => continue, // denied (event dropped) }; // Handler only receives GatedEvent (compile-time enforced) @@ -706,19 +887,20 @@ async fn run_gateway_platforms( platform: gw_event.platform.clone(), sender_id: gw_event.sender_id.clone(), channel_id: gw_event.channel_id.clone(), + workspace_id: None, // gateway platforms don't use workspace scoping is_dm: gw_event.is_dm, is_bot: gw_event.is_bot, raw: RawPlatformEvent::Gateway(gw_event.clone()), }; - // 🔒 Trust Gate (same logic, keyed by event.platform) - let gated = match gate_event(&event, &trust).await { + // 🔒 Trust Gate (same logic, keyed by event.platform) — takes ownership + let gated = match gate_event(event, &trust).await { Some(g) => g, None => continue, }; // Fan-out to platform-specific Handler - match event.platform.as_str() { + match gated.platform() { "telegram" => telegram_handler.handle(gated).await, "line" => line_handler.handle(gated).await, "feishu" => feishu_handler.handle(gated).await, @@ -883,7 +1065,7 @@ Each adapter implements `is_trusted_sender()`. Rejected because: - Trust logic is identical across all platforms (`allowed_users.contains(id)`) - Forces N identical implementations with no polymorphic benefit - New adapter forgetting to implement = security hole -- Three-layer architecture makes this impossible to bypass by construction +- Three-layer architecture makes accidental bypass a compile error by construction ### Trust check at gateway layer From 3cd9cb0af786d737424a772f4c607a33adda668e Mon Sep 17 00:00:00 2001 From: chaodu-agent Date: Sat, 11 Jul 2026 02:46:13 +0000 Subject: [PATCH 09/12] fix(adr): address review findings F1-F3 on identity-trust-none MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit F1 (critical): Remove sender_id spoofing hole in cron bypass — only check platform == "cron" since WeCom allows freeform UserIDs that could match any synthetic value. Update rationale accordingly. F2: Add WeCom, Google Chat, MS Teams to pinned is_bot derivation and echo-delivery tables (all 8 platforms now covered). F3: Fix pseudocode precision — unwrap_or no longer borrows a temporary; add PlatformTrustConfigs::get() and PlatformTrustConfig::surface_allowed() delegating method used by gate_event. --- docs/adr/identity-trust-none.md | 54 ++++++++++++++++++++++++++++----- 1 file changed, 47 insertions(+), 7 deletions(-) diff --git a/docs/adr/identity-trust-none.md b/docs/adr/identity-trust-none.md index 30f954e7a..960758ef3 100644 --- a/docs/adr/identity-trust-none.md +++ b/docs/adr/identity-trust-none.md @@ -504,9 +504,10 @@ impl PlatformTrustConfigs { sender_id: &str, workspace_id: Option<&str>, ) -> Decision { + let default = PlatformTrustConfig::Base(Self::default_config()); let config = self.configs .get(&platform.to_lowercase()) - .unwrap_or(&PlatformTrustConfig::Base(Self::default_config())); + .unwrap_or(&default); match config { PlatformTrustConfig::Base(c) => c.decide(channel_id, is_dm, sender_id), @@ -515,6 +516,21 @@ impl PlatformTrustConfigs { } } + /// Lookup helper for gate_event — returns the config for a platform. + pub fn get(&self, platform: &str) -> &PlatformTrustConfig { + static DEFAULT: std::sync::LazyLock = + std::sync::LazyLock::new(|| PlatformTrustConfig::Base(TrustConfig { + allow_all_channels: true, + allowed_channels: HashSet::new(), + allow_dm: true, + allow_all_users: false, + allowed_users: HashSet::new(), + })); + self.configs + .get(&platform.to_lowercase()) + .unwrap_or(&DEFAULT) + } + fn default_config() -> TrustConfig { // L2 open, L3 deny-all — unknown platform = nobody in. TrustConfig { @@ -527,6 +543,17 @@ impl PlatformTrustConfigs { } } +impl PlatformTrustConfig { + /// Delegating method — dispatches surface_allowed to the inner TrustConfig. + pub fn surface_allowed(&self, channel_id: &str, is_dm: bool) -> bool { + match self { + PlatformTrustConfig::Base(c) => c.surface_allowed(channel_id, is_dm), + PlatformTrustConfig::Line(c) => c.base.surface_allowed(channel_id, is_dm), + PlatformTrustConfig::Slack(c) => c.base.surface_allowed(channel_id, is_dm), + } + } +} + impl LineTrustConfig { pub fn decide(&self, channel_id: &str, is_dm: bool, sender_id: &str) -> Decision { // L2 scope check first (same as base) @@ -637,6 +664,9 @@ Handler. The Trust Gate only evaluates human sender identity. | LINE | Always `false` | LINE has no bot-to-bot webhook delivery; bot-bypass is a no-op | | Feishu | `trusted_bot_ids.contains(sender_open_id)` | Feishu marks other bots as `sender_type="user"` — unreliable; must match against known bot IDs | | Telegram | `message.from.is_bot` flag | Native field from Telegram API | +| WeCom | `msgtype == "event"` and `event == "enter_agent"`, or `trusted_bot_ids.contains(userid)` | WeCom bot-to-bot uses app callback events; regular messages from bots carry the bot's UserID — match against `trusted_bot_ids` | +| Google Chat | `message.sender.type == "BOT"` | Native field from Chat API event payload | +| MS Teams | `activity.from.role == "bot"` or `trusted_bot_ids.contains(activity.from.id)` | Bot Framework marks bot senders with role field; verify against known bot IDs for reliability | **`trusted_bot_ids` is shared config (NOT Handler-only):** @@ -662,7 +692,11 @@ async fn gate_event(event: InboundEvent, configs: &PlatformTrustConfigs) -> Opti // Cron is internal — it has no external sender and targets arbitrary // platform channels. Gating it against platform L2 scope would incorrectly // block scheduled jobs to channels not in the human-facing allowlist. - if event.platform == "cron" || event.sender_id == "openab-cron" { + // Only `platform` is checked — the Cron Receiver is the sole producer of + // events with platform == "cron", so both fields are trustworthy by construction. + // sender_id is NOT checked here to avoid spoofing: some platforms (e.g. WeCom) + // allow freeform UserIDs that could match any synthetic value. + if event.platform == "cron" { return Some(seal(event)); // fully trusted, no L2/L3 } @@ -699,11 +733,14 @@ On `Deny`, the event is dropped. This avoids deep-cloning `RawPlatformEvent` (which may contain the full platform payload) on the hot path. **Cron bypass rationale:** Cron tasks are system-initiated (no external sender). -They use `platform = "cron"` and `sender_id = "openab-cron"` — a reserved -synthetic identity that cannot collide with any real platform sender ID (real -platform IDs are alphanumeric, never prefixed with `openab-`). The Cron Receiver -sets these fields; the Trust Gate recognizes them and short-circuits. The CronHandler -then routes the dispatched event to the appropriate target platform/channel. +They use `platform = "cron"` and `sender_id = "openab-cron"`. The Trust Gate +recognizes them by **`platform == "cron"` only** — the `sender_id` field is not +part of the bypass condition. This is critical because some platforms (notably +WeCom) allow tenant-admin-assigned freeform UserIDs that could match any string, +including `"openab-cron"`. Since the Cron Receiver is the sole code path that +produces events with `platform = "cron"`, checking the platform field alone is +both necessary and sufficient. The CronHandler then routes the dispatched event +to the appropriate target platform/channel. ### Echo reply on deny @@ -742,6 +779,9 @@ calls platform APIs directly. | LINE | Reply API only; silent drop if token expired | **Never** use Push API for deny-echo (prevents attackers from burning paid Push quota) | | Telegram | Reply in-chat | Standard reply | | Feishu | Reply in-chat | Standard reply | +| WeCom | Reply in-chat via `message.send` API | Uses the application's send-message endpoint; targets the source conversation | +| Google Chat | Reply in-space via `spaces.messages.create` | Replies in the same space; for DMs uses the DM space with the user | +| MS Teams | Reply in-conversation via Bot Framework `sendToConversation` | Uses the Bot Framework connector to reply in the originating conversation | **Echo content by scope (leak-safe):** - **DM / 1:1 context:** echo includes sender UID (self-serve — user forwards to admin to request access) From fae8b0997f49ee8638a34be1b4c33d1fd8c7cc7a Mon Sep 17 00:00:00 2001 From: chaodu-agent Date: Sat, 11 Jul 2026 03:03:08 +0000 Subject: [PATCH 10/12] =?UTF-8?q?fix(adr):=20address=20group=20review=20ro?= =?UTF-8?q?und=202=20=E2=80=94=209=20findings=20from=20B1/B4/B5/B8/B11/B12?= =?UTF-8?q?/B15?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes: - WeCom is_bot: remove enter_agent (user-initiated, not bot); keep only trusted_bot_ids (B5 F1, B12 F1) - SlackTrustConfig::decide(): workspace_users is now strict override (ignore allow_all_users); base fallback supports team_id:user_id composite key for Enterprise Grid (B4 F1, B5 F2, B12 F3) - LINE group policy: fix prose vs code contradiction — unconfigured groups use default_group_policy, not DenyScope (B12 F2) - Reserved platform validation: MUST-level requirement for all external Receivers to reject reserved platform names; cron bypass invariant documented; Phase 1 SHOULD for typed InboundSource enum (B1) - decide() simplified to use self.get() — remove duplicate default logic (B5 F3) - Echo rate-limit key updated to (platform, workspace_id, sender_id) (B12 F4, B15 F2) - Module layout: InboundEvent in mod.rs (public), gate.rs narrow (B12 F5) - into_inner() trust boundary doc: module-level vs crate-level explained, Phase 1 SHOULD for lint/annotation (B11, B15 F1) - Slack Enterprise Grid gotcha: split dense table cell into footnote (B8 F2) --- docs/adr/identity-trust-none.md | 85 ++++++++++++++++++++++----------- 1 file changed, 57 insertions(+), 28 deletions(-) diff --git a/docs/adr/identity-trust-none.md b/docs/adr/identity-trust-none.md index 960758ef3..a2e849ca4 100644 --- a/docs/adr/identity-trust-none.md +++ b/docs/adr/identity-trust-none.md @@ -179,8 +179,10 @@ normal code paths: ```rust // In crate::trust::gate (narrow module — only Trust Gate code lives here) +// Note: InboundEvent is defined in crate::trust (mod.rs) and re-exported publicly. +// GatedEvent and seal() live here in gate.rs — keeping the constructor private. -/// Receiver produces this (untrusted). +/// Receiver produces this (untrusted). Defined in crate::trust::mod.rs (public). pub struct InboundEvent { /* ... */ } /// Trust Gate produces this (trusted). @@ -201,6 +203,14 @@ impl GatedEvent { /// Consuming unwrap — restricted to `pub(crate)` to minimize escape paths. /// Only used by the Dispatcher when it needs ownership of the inner event /// for session creation. Handlers should use read-only accessors above. + /// + /// TRUST BOUNDARY NOTE: The trust enforcement boundary is at the MODULE level + /// (only crate::trust::gate can CONSTRUCT GatedEvent), not the crate level. + /// Any code within openab-core can CONSUME a GatedEvent via into_inner(), + /// but cannot forge one. If the crate grows large or is split, consider + /// tightening this to a trait-based accessor or moving Dispatcher session + /// creation into the trust module. Phase 1 SHOULD add a #[doc(hidden)] + /// annotation or clippy restriction lint to limit call sites. pub(crate) fn into_inner(self) -> InboundEvent { self.inner } /// Test-only constructor — allows Handler unit tests to create GatedEvent @@ -226,7 +236,7 @@ async fn handle(&self, event: GatedEvent) { /* ... */ } **Module layout for enforcement:** ``` crate::trust -├── mod.rs // PlatformTrustConfigs, TrustConfig, Decision (public) +├── mod.rs // PlatformTrustConfigs, TrustConfig, Decision, InboundEvent (public) ├── gate.rs // gate_event() + GatedEvent (constructor is private) └── (no other module can construct GatedEvent) ``` @@ -445,7 +455,8 @@ policy = "members" # per-user trust: must be in allowed_users **Decision logic for LINE (extends base `decide()`):** - **1:1 DM:** `sender_id == "unknown"` → Deny; otherwise check `allowed_users` -- **Group not in configured groups** → DenyScope (not in scope) +- **Group not in configured groups** → apply `default_group_policy` (fail-closed + default: `"members"` — same as explicitly configured `policy = "members"`) - **Group `policy = "open"`** → Allow (group-level trust — `"unknown"` permitted) - **Group `policy = "members"`** → `sender_id == "unknown"` → Deny; otherwise check `allowed_users` @@ -504,12 +515,7 @@ impl PlatformTrustConfigs { sender_id: &str, workspace_id: Option<&str>, ) -> Decision { - let default = PlatformTrustConfig::Base(Self::default_config()); - let config = self.configs - .get(&platform.to_lowercase()) - .unwrap_or(&default); - - match config { + match self.get(platform) { PlatformTrustConfig::Base(c) => c.decide(channel_id, is_dm, sender_id), PlatformTrustConfig::Line(c) => c.decide(channel_id, is_dm, sender_id), PlatformTrustConfig::Slack(c) => c.decide(channel_id, is_dm, sender_id, workspace_id), @@ -517,6 +523,7 @@ impl PlatformTrustConfigs { } /// Lookup helper for gate_event — returns the config for a platform. + /// Returns a static deny-all default for unknown platforms. pub fn get(&self, platform: &str) -> &PlatformTrustConfig { static DEFAULT: std::sync::LazyLock = std::sync::LazyLock::new(|| PlatformTrustConfig::Base(TrustConfig { @@ -530,17 +537,6 @@ impl PlatformTrustConfigs { .get(&platform.to_lowercase()) .unwrap_or(&DEFAULT) } - - fn default_config() -> TrustConfig { - // L2 open, L3 deny-all — unknown platform = nobody in. - TrustConfig { - allow_all_channels: true, - allowed_channels: HashSet::new(), - allow_dm: true, - allow_all_users: false, - allowed_users: HashSet::new(), - } - } } impl PlatformTrustConfig { @@ -610,11 +606,14 @@ impl SlackTrustConfig { // L3 identity — workspace-scoped for Enterprise Grid if sender_id.is_empty() { return Decision::DenyIdentity; } - // If workspace_users is configured and we have a workspace_id, - // check workspace-scoped allowlist first. + // 1. If workspace_users has an entry for this workspace, use STRICT + // per-workspace check — ignore allow_all_users for this workspace. + // Rationale: if an operator configured workspace_users for a specific + // workspace, they expect it to be enforced. allow_all_users only + // applies to workspaces without explicit workspace_users entries. if let Some(ws_id) = workspace_id { if let Some(ws_users) = self.workspace_users.get(ws_id) { - return if ws_users.contains(sender_id) || self.base.allow_all_users { + return if ws_users.contains(sender_id) { Decision::Allow } else { Decision::DenyIdentity @@ -622,8 +621,17 @@ impl SlackTrustConfig { } } - // Fallback to base config (single-workspace or unscoped) - if self.base.identity_allowed(sender_id) { + // 2. Fallback to base config (single-workspace or unscoped). + // Supports both plain sender_id AND composite "team_id:sender_id" + // format for Enterprise Grid deployments that cannot use enterprise_user.id. + let allowed = if let Some(ws_id) = workspace_id { + let trust_key = format!("{}:{}", ws_id, sender_id); + self.base.allowed_users.contains(&trust_key) || self.base.identity_allowed(sender_id) + } else { + self.base.identity_allowed(sender_id) + }; + + if allowed { Decision::Allow } else { Decision::DenyIdentity @@ -664,7 +672,7 @@ Handler. The Trust Gate only evaluates human sender identity. | LINE | Always `false` | LINE has no bot-to-bot webhook delivery; bot-bypass is a no-op | | Feishu | `trusted_bot_ids.contains(sender_open_id)` | Feishu marks other bots as `sender_type="user"` — unreliable; must match against known bot IDs | | Telegram | `message.from.is_bot` flag | Native field from Telegram API | -| WeCom | `msgtype == "event"` and `event == "enter_agent"`, or `trusted_bot_ids.contains(userid)` | WeCom bot-to-bot uses app callback events; regular messages from bots carry the bot's UserID — match against `trusted_bot_ids` | +| WeCom | `trusted_bot_ids.contains(userid)` | WeCom has no reliable native bot flag; match against known bot IDs. Note: `enter_agent` (member-enter event) is user-initiated — do NOT treat as bot | | Google Chat | `message.sender.type == "BOT"` | Native field from Chat API event payload | | MS Teams | `activity.from.role == "bot"` or `trusted_bot_ids.contains(activity.from.id)` | Bot Framework marks bot senders with role field; verify against known bot IDs for reliability | @@ -742,6 +750,17 @@ produces events with `platform = "cron"`, checking the platform field alone is both necessary and sufficient. The CronHandler then routes the dispatched event to the appropriate target platform/channel. +**Reserved platform validation (MUST-level requirement):** + +`"cron"` (and any future internal platform names) are **reserved**. The following +invariants MUST hold: + +| Requirement | Detail | +|---|---| +| Reserved platform rejection | All externally-sourced Receivers (Gateway, Discord, Slack, etc.) MUST reject events whose normalized `platform` field matches a reserved name before constructing `InboundEvent`. Reserved names: `"cron"`, `"internal"`, any value prefixed with `"openab-"`. | +| Cron bypass invariant | Trust Gate may bypass L2/L3 only for events produced by CronReceiver. External payload fields MUST NOT be sufficient to select the cron bypass path. `InboundEvent.platform` MUST be set by the Receiver from a trusted source (hardcoded adapter name or gateway routing config), never derived from external payload content. | +| Phase 1 typed provenance (SHOULD) | Phase 1 implementation SHOULD encode system vs. external provenance with a typed source enum (e.g., `InboundSource::SystemCron` / `InboundSource::External { platform }`) to make the bypass condition impossible to forge at the type level. | + ### Echo reply on deny ```rust @@ -757,7 +776,11 @@ if decision == Decision::DenyIdentity { ``` **Echo safeguards:** -- **Rate-limit:** max 1 echo per sender per platform per 5 minutes (prevents spam/DoS amplification) +- **Rate-limit:** max 1 echo per `(platform, workspace_id, sender_id)` tuple per + 5 minutes (prevents spam/DoS amplification). The `workspace_id` component + ensures Slack Enterprise Grid users get independent rate-limit quotas per + workspace. For platforms without workspace scoping, `workspace_id` is empty/None + and the key effectively becomes `(platform, sender_id)`. - **Bounded state:** rate-limit cache MUST be bounded (e.g., LRU with max 10,000 entries + 5-minute TTL eviction). An unbounded `HashMap` would allow attackers to OOM the process by sending messages with random/spoofed @@ -800,7 +823,13 @@ using the **exact format** the platform provides in event payloads: | Platform | Native type | `allowed_users` format | Example | Gotcha | |----------|-------------|----------------------|---------|--------| | Discord | Snowflake (u64) | Numeric string | `"845835116920307722"` | — | -| Slack | String | U-prefix or W-prefix | `"U01ABCDEFGH"` | Enterprise Grid: Receiver MUST set `workspace_id = Some(team_id)` and prefer `enterprise_user.id` as `sender_id` when available (stable across workspaces). Config format for Grid: `allowed_users = ["E0123456789"]` (enterprise_user.id) or `"T012345:U01ABCDEFGH"` (team_id:user_id fallback). Trust key = `(workspace_id, sender_id)` — see `decide()` below. | +| Slack | String | U-prefix or W-prefix | `"U01ABCDEFGH"` | Enterprise Grid: see below¹ | + +**¹ Slack Enterprise Grid sender ID notes:** +- Receiver MUST set `workspace_id = Some(team_id)` for Grid deployments +- Prefer `enterprise_user.id` as `sender_id` when available (stable across workspaces) +- Config formats: `allowed_users = ["E0123456789"]` (enterprise_user.id, recommended) or `"T012345:U01ABCDEFGH"` (team_id:user_id fallback for non-Grid tokens) +- Trust key = `(workspace_id, sender_id)` — see `SlackTrustConfig::decide()` above | Telegram | Integer (i64) | Stringified integer | `"123456789"` | ⚠️ Do NOT use `@username` — only numeric ID works | | LINE | String | U + 32 hex chars | `"U1234567890abcdef0123456789abcdef"` | — | | Feishu | String | open_id | `"ou_xxxxxxxxxxxxxxxxxxxx"` | ⚠️ `open_id` is **per-app** — same user has different ID in different Feishu apps | From 7a99635440a4a67ed55c533b76859440579ad9f2 Mon Sep 17 00:00:00 2001 From: chaodu-agent Date: Sat, 11 Jul 2026 03:04:10 +0000 Subject: [PATCH 11/12] fix(adr): add platform lowercase invariant to InboundEvent (B14 F2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit InboundEvent.platform MUST be lowercase — Receivers normalize before constructing. This ensures consistency between gate_event's == "cron" check and PlatformTrustConfigs::get()'s to_lowercase() lookup. --- docs/adr/identity-trust-none.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/adr/identity-trust-none.md b/docs/adr/identity-trust-none.md index a2e849ca4..7ab95113b 100644 --- a/docs/adr/identity-trust-none.md +++ b/docs/adr/identity-trust-none.md @@ -366,6 +366,11 @@ struct defined in a shared types crate or serialized as JSON over the WS). /// Contains the minimum fields needed for trust evaluation. pub struct InboundEvent { pub platform: String, // "discord", "telegram", "line", etc. + // INVARIANT: always lowercase. Receivers MUST + // normalize to lowercase before constructing. + // This ensures gate_event's `== "cron"` check + // and PlatformTrustConfigs::get()'s to_lowercase() + // are consistent. pub sender_id: String, // platform-specific sender identifier pub channel_id: String, // conversation surface pub workspace_id: Option, // workspace/team context (Slack Enterprise Grid: team_id) From 6dc0e33bb84da437381f32ef853b451775d2963b Mon Sep 17 00:00:00 2001 From: chaodu-agent Date: Sat, 11 Jul 2026 03:04:35 +0000 Subject: [PATCH 12/12] docs(adr): clarify gw_event.platform source in run_gateway_platforms MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add comment noting platform field is assigned by gateway routing config, not from webhook payload body — satisfies reserved platform invariant. --- docs/adr/identity-trust-none.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/adr/identity-trust-none.md b/docs/adr/identity-trust-none.md index 7ab95113b..d74344c57 100644 --- a/docs/adr/identity-trust-none.md +++ b/docs/adr/identity-trust-none.md @@ -957,6 +957,9 @@ async fn run_gateway_platforms( let gw_event: GatewayEvent = ws.recv().await; // Normalize GatewayEvent → InboundEvent + // Note: gw_event.platform is assigned by gateway routing config (based on + // which webhook endpoint received the request), NOT copied from the platform + // webhook payload body. This satisfies the reserved platform invariant. let event = InboundEvent { platform: gw_event.platform.clone(), sender_id: gw_event.sender_id.clone(),