Skip to content

docs(adr): identity trust-none default & trust pyramid#1264

Open
chaodu-agent wants to merge 1 commit into
mainfrom
adr/identity-trust-none
Open

docs(adr): identity trust-none default & trust pyramid#1264
chaodu-agent wants to merge 1 commit into
mainfrom
adr/identity-trust-none

Conversation

@chaodu-agent

Copy link
Copy Markdown
Collaborator

Splits the trust/security decision out of #1263 into its own ADR (per discussion — one decision per ADR).

Scope

  • L1/L2/L3 trust pyramid — L1 platform auth (security), L2 channel/group/DM scope (NOT security, default-open), L3 identity (security, default deny-all)
  • Identity trust-none default — flip empty allowed_users from allow-all to deny-all
  • Single router gate at AdapterRouter::handle_message() — un-bypassable
  • TrustConfig + carrier/type changes — extend MessageContext/ChannelRef, no new trait
  • Echo-on-deny request-access flow

Relationship

Doc: docs/adr/identity-trust-none.md

Split out from the per-platform-config ADR. Covers the security/trust
decision independently: L1/L2/L3 trust pyramid, identity trust-none
default at a single router gate, TrustConfig + carrier/type changes,
echo-on-deny. Depends on the first-class per-platform config ADR.
@chaodu-agent chaodu-agent requested a review from thepagent as a code owner June 30, 2026 20:50
chaodu-agent added a commit that referenced this pull request Jun 30, 2026
Split the trust/security decision out into a separate ADR
(docs/adr/identity-trust-none.md, PR #1264). This ADR now covers only
the config schema change: first-class [platform] sections + [gateway]
deprecation + migration.
@chaodu-agent

Copy link
Copy Markdown
Collaborator Author

LGTM ✅ — Well-structured ADR with clear layered trust model and thorough rejected alternatives.

What This PR Does

Introduces a new ADR documenting the identity trust-none default and 3-layer trust pyramid (L1 platform auth → L2 channel scope → L3 identity trust). Flips the default from allow-all to deny-all on the identity layer, with a single un-bypassable router gate.

How It Works

Defines a TrustConfig per platform with L2 (scope, default-open) and L3 (identity, default-deny) checks at AdapterRouter::handle_message(). Untrusted senders receive an echo of their ID for self-service access requests. Removes scattered trust checks from individual adapters into one centralized gate.

Findings

# Severity Finding Location
1 🟢 Excellent separation of concerns — L2 as non-security scoping vs L3 as the actual auth gate is clear and well-argued §3
2 🟢 DM asymmetry explicitly called out — explains why L2 must stay open for the deny UX to work §3 Layer 3
3 🟢 Single router gate eliminates "forgot to implement" class of bugs (vs per-adapter trait) §8 Rejected Alternatives
4 🟢 Migration path is clear — existing deployments just add allow_all_users = true §6
5 🟢 Correct dependency declaration on PR #1263 (first-class-platform-config) Header
What's Good (🟢)
  • Clear trust pyramid with ASCII art architecture diagrams that make the layered model immediately understandable
  • L1 platform auth table covering all 8 platforms with specific mechanism details — useful reference for implementers
  • Echo-on-deny UX is a thoughtful onboarding pattern — untrusted users are not silently dropped, they get actionable guidance
  • Minimal surface area — only MessageContext and ChannelRef carrier types change; no new trait introduced
  • Well-reasoned rejected alternatives — each has a concrete reason for rejection (bypass risk, layer violation, UX breakage)
  • Implementation plan is sequenced correctly (define types → wire gate → remove old checks → add echo → update docs)
Baseline Check
  • PR opened: 2026-06-30
  • Main already has: No identity-trust ADR; existing trust checks are scattered across adapters (discord.rs, slack.rs, gateway.rs)
  • Net-new value: Formalizes the trust-none-by-default decision, defines the 3-layer pyramid, provides implementation blueprint for consolidating trust enforcement into the router
  • Dependency: Requires first-class-platform-config.md (PR docs(adr): first-class per-platform configuration #1263) to land first — per-platform allowed_users lives in those sections

@chaodu-agent

Copy link
Copy Markdown
Collaborator Author

Implementation Plan — all platforms at once (phased)

This plan implements the trust pyramid for every adapter behind one shared gate, then flips the identity default to deny-all as a separate, clearly-flagged breaking change.

Invariant we're enforcing

AdapterRouter::handle_message() becomes the single, un-bypassable point where L2 (scope) + L3 (identity) are evaluated. Adapters keep only structural/trigger logic; no adapter may dispatch to ACP without passing the gate.


Current state (verified)

Platform Where trust runs today Source Default
Discord discord.rs Handler (is_denied_user, allow_all_*) [discord] resolve_allow_all → empty list = allow-all
Slack slack.rs [slack] same
Telegram/LINE/Feishu/WeCom/GChat/Teams gateway.rs::should_skip_event via GatewayEventContext GATEWAY_* env / [gateway] (Telegram now also [telegram]) same

resolve_allow_all(flag, list) = flag.unwrap_or(list.is_empty()) → unset + empty ⇒ trust-all. This is what we flip (L3 only).


What moves to the gate vs stays in adapters

Gate (router) — L2 + L3 only:

  • L2 scope: allow_all_channels / allowed_channels / allow_dm (default OPEN)
  • L3 identity: allow_all_users / allowed_users for human senders (default DENY)

Stays in adapters (structural/trigger — NOT trust):

  • @mention detection, thread detection, multibot detection
  • bot-message policy (allow_bot_messages modes, trusted_bot_ids)
  • allowed_role_ids triggers, bot-turn throttling
  • DM is_dm classification (adapter sets the flag; gate decides allow)

Rationale: adapters answer "is this message a trigger for me?"; the gate answers "is this surface/identity trusted?" Bot admission + mention modes are platform-specific trigger semantics and are intentionally out of TrustConfig (matches the ADR).


Phase 0 — shared infra (behavior-preserving)

  1. Add TrustConfig + PlatformTrustConfigs (registry keyed by platform()), per the ADR. Resolution:
    • allow_all_users = self.allow_all_users.unwrap_or(false) ← L3 deny-default
    • allow_all_channels = self.allow_all_channels.unwrap_or(true), allow_dm.unwrap_or(true) ← L2 open
    • Phase 0 keeps old behavior by feeding the registry through the existing resolve_allow_all (unwrap_or(list.is_empty())) so nothing changes yet.
  2. Carrier changes: MessageContext.sender: SenderContext (replace sender_json); add ChannelRef.is_dm (excluded from Hash/Eq).
  3. Add AdapterRouter::trust: PlatformTrustConfigs + a gate(platform, channel, sender) -> Decision helper. Not yet wired into handle_message.
  4. Unit tests for TrustConfig decision matrix (L2 open/closed × L3 allow/deny × DM/channel).

Phase 1 — route every platform through the gate (still behavior-preserving)

  1. Wire gate() into handle_message(): L2 then L3; on deny → log + (L3-only) echo + return.
  2. Remove scattered checks:
    • discord.rs: channel/user allowlist (keep mention/bot/role/multibot)
    • slack.rs: channel/user allowlist
    • gateway.rs::should_skip_event: drop the user/channel filter (keep bot filter + @mention gating), or have it delegate to the shared gate
  3. Build PlatformTrustConfigs at startup from each [platform] section (Discord/Slack/Telegram first-class; remaining gateway platforms via their [platform] sections per docs(adr): first-class per-platform configuration #1263, env fallback retained).
  4. Per-platform integration tests asserting the old scattered checks are gone and the gate decides.

Phase 2 — echo-on-deny UX

  1. On L3 deny, adapter.send_message() the sender their ID + how to request access.
  2. Echo throttle/dedup (security): echo at most once per (platform, sender_id) per N minutes to prevent the bot being used to spam/amplify. Never echo on L2 deny (scope) or to bots.

Phase 3 — flip the default (the breaking change, isolated PR)

  1. Switch L3 resolution from resolve_allow_all (open) to unwrap_or(false) (deny). L2 stays open.
  2. Migration: allow_all_users = true restores old behavior; document per-platform in release notes with a ⚠️ Breaking Change section.
  3. Optional: a one-time startup WARN when a platform is configured with empty allowed_users and no explicit allow_all_users ("trust-none active; set allowed_users or allow_all_users=true").

Why phased

  • Phases 0–2 are behavior-preserving (mechanical refactor → easy to review, low risk).
  • Phase 3 is the only behavior change — small, isolated diff reviewers can scrutinize, with the migration guide.

Edge cases / risks

  • Bot senders bypass L3 human-identity (bot admission handled in adapters via trusted_bot_ids/allow_bot_messages). Document that L3 allowed_users is human-only.
  • DM path: is_dm=true ⇒ L2 uses allow_dm; L3 still applies (a DM from a stranger → echo + drop).
  • Multibot race: existing cache check stays in adapters; unaffected.
  • ID formats differ per platform — TrustConfig is keyed by platform(), so no cross-platform ID bleed (this is the whole point of docs(adr): first-class per-platform configuration #1263).
  • Echo amplification — mitigated by throttle/dedup (Phase 2).

Test matrix (per platform)

human-allowed / human-denied / DM-from-stranger (echo) / channel-out-of-scope (no echo) / bot-sender (bot policy, not L3) / allow_all_users=true (legacy).

Deliverables

  • PR A: Phase 0 (infra + carriers + tests)
  • PR B: Phase 1 (wire gate + remove scattered checks, all platforms)
  • PR C: Phase 2 (echo + throttle)
  • PR D: Phase 3 (default flip + migration + release note) ← the breaking one

Depends on #1263 (per-platform config) for the per-platform [*] sections that feed PlatformTrustConfigs.

thepagent pushed a commit that referenced this pull request Jul 1, 2026
* feat(trust): Phase 0 — shared TrustConfig + PlatformTrustConfigs (additive)

Defines the shared L2 (scope) + L3 (identity) trust types and pure
decision function per the identity-trust-none ADR (#1264). Purely
additive: not yet wired into AdapterRouter, changes no runtime behavior.

- TrustConfig: L2 (allow_all_channels/allowed_channels/allow_dm, default
  open) + L3 (allow_all_users/allowed_users, default deny-all)
- Decision enum (Allow / DenyScope / DenyIdentity) — only DenyIdentity
  echoes (request-access UX)
- PlatformTrustConfigs registry keyed by platform() (no cross-platform
  ID bleed)
- 11 unit tests covering the L2×L3×DM decision matrix

Wiring + removing scattered per-adapter checks lands in Phase 1; the
trust-none default flip lands in Phase 3.

Refs #1264

* docs(trust): add compact decision-flow diagram to decide() rustdoc

* fix(trust): address #1266 review (F2/F3/F6/F7 + F5 doc)

- F2: #[non_exhaustive] on Decision (avoid future semver break)
- F3: reword DenyScope doc — scope control, not an authorization failure
- F6: normalize platform keys to lowercase (case-insensitive registry)
- F7: empty sender_id is never identity-allowed (fail-closed, even under
  allow_all_users)
- F5: document new() as canonical constructor; allow_all_* takes precedence
- add 3 tests (empty-sender, case-insensitive registry)

---------

Co-authored-by: chaodu-agent <chaodu-agent@users.noreply.github.com>
thepagent pushed a commit that referenced this pull request Jul 1, 2026
…ared gate (#1267)

* feat(trust): Phase 1 (gateway) — route gateway ingress through shared trust gate

Wires the shared L2/L3 gate (#1266) into the unified gateway path:
- AdapterRouter gains a PlatformTrustConfigs registry (via with_trust
  builder — new()'s signature unchanged) + gate_incoming() ingress gate
- process_gateway_event now enforces L2/L3 via router.gate_incoming();
  should_skip_event keeps only bot-filter + @mention gating (its
  channel/user checks are neutered in the unified path)
- registry built at startup from GATEWAY_* env, keyed per gateway platform

Behavior-preserving: registry defaults mirror today's should_skip_event
(allow-all default); is_dm passed false so DMs are evaluated as channels
exactly as today. Discord/Slack routing + dispatch-path privatization +
should_skip_event L2/L3 removal follow in later PRs. Deny-flip is Phase 3.

Refs #1264

* docs(trust): add phase-2 TODO for is_dm carrier at gateway gate (review F2)

---------

Co-authored-by: chaodu-agent <chaodu-agent@users.noreply.github.com>
thepagent pushed a commit that referenced this pull request Jul 1, 2026
…canary before merge] (#1270)

* feat(trust): Phase 1 (discord) — gate L3 identity via shared gate

Routes Discord ingress through AdapterRouter::gate_incoming for the L3
(identity) layer, keyed under "discord" in the trust registry.

- registry "discord" entry: L2 open + allow_dm=true (Discord's richer
  channel/thread/DM logic stays in the adapter — the flat allowed_channels
  model can't express thread-by-parent admission); L3 mirrors resolved
  [discord].allow_all_users/allowed_users
- gate call added at the Discord dispatch spawn, redundant-but-matching
  with Discord's existing pre-dispatch user check → non-regressive by
  construction (cannot deny what already passed)

Behavior-preserving. Phase 1c makes the gate authoritative and removes the
scattered user check; richer Discord L2 modeling + dispatch privatization
tracked in #1269.

Refs #1264 #1269

* fix(trust): Discord gate skips bots + passes real is_dm (review #1270 F1/F2)

F1 (blocker): Discord's is_denied_user has a !is_bot bypass (bot admission
is handled by allow_bot_messages + trusted_bot_ids). The shared L3 gate is
human-identity only, so running it on bots wrongly dropped trusted
bot-to-bot messages when allow_all_users=false (multi-agent). Guard the
gate with !sender.is_bot.

F2: pass the in-scope is_dm instead of hardcoded false (benign today with
the L2-open discord entry, but avoids latent risk).

Note: #1267 (gateway) is unaffected — should_skip_event's user check never
had a bot bypass, so the gateway gate already matched it for bots.

* test(trust): name + test the Discord L3 bot-bypass (review #1270 F4)

Extract the gate's bot-skip into l3_gate_applies(is_bot) and add a
regression test (l3_gate_skips_bots_admits_humans) locking in that bots
bypass the shared L3 gate (mirrors is_denied_user's !is_bot), so
trusted/mode-admitted bots aren't denied when allow_all_users=false.

---------

Co-authored-by: chaodu-agent <chaodu-agent@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(channels): default trust-none policy with UID echo for all channel adapters

1 participant